-
1. Re: Child object validation
gavin.king Feb 12, 2006 10:16 PM (in response to andrew.rw.robinson)I'll point Emmanuel this way.
-
2. Re: Child object validation
epbernard Feb 13, 2006 1:10 PM (in response to andrew.rw.robinson)Year I noticed that.
http://opensource2.atlassian.com/projects/hibernate/browse/ANN-208
This is probably a 1h30 work test included, if you're willing to contribute :-) -
3. Re: Child object validation
andrew.rw.robinson Feb 13, 2006 1:41 PM (in response to andrew.rw.robinson)I am really busy, but will consider it :-)
-
4. Re: Child object validation
andrew.rw.robinson Feb 17, 2006 12:19 AM (in response to andrew.rw.robinson)Okay, I just spent some time looking into the code and found this is not a seam issue, but a hibernate issue. The ClassValidator does not support collections. In both the initialization method as well as the actual validation method, it applies the validation to the field class and the method return class. The short comming is that it doesn't check to see if the field type or property type is a collection.
So what happens is that the parent object is parsed and the fields. The field type is a List/Set/Map (either generic or not). The class validator validates "List", "Set" and "Map" instead of the objects that the collection contains.
Proposal Initialize changes:
* Change the ClassValidator to check for collections.
* If property/field is a collection, then check the generic type to see what type the collection contains.
* Recurse through as normal
Proposal Validate changes:
* Check for collections when the property/field value is retrieved
* If a collection, iterate over the members (values for map)
* validate each item in the collection (TODO: decide the path to the object, should it include some kind of index identifier? This would not work for map though)
Once this is done for hibernate-annotations, then seam would need an update. The trick would be how to map the validation issues to component IDs. This isn't so easy. The dataTable reders children IDs with the index:
myObjectId:0:myCollectionMemberId
where 0 is the index. That means to attempt to find the invalid component, the path dataTable would have to have an ID equal to the collection name, then somehow identify the index (would be good if the classvalidator did this) and then try to find the component.
Example:
class User
@Valid
List addresses;
May result in a validation issue with path like:
user.addresses[5].street
The ID's in JSF would be something like:
myForm:addresses:5:street
Where myForm is the form component id,
addresses is the ID of the dataTable component
5 is the index in the collection
street is the input text component ID with the invalid value
Gavin, looks like you are the author of the ClassValidator, what impact would this cause on the validator? I think this should be acceptable, if the developer does not want to propagate validation into the collection, they simply should not mark the field/property as "@Valid".
Without this code, the only way to validate children is to have to hand code a validate method for every property on every entity that has a collection to validate, definately more effort than to incorporate it in the classvalidator.
I would really like to have this before middle March, and may be willing to help to speed this up (probably by submitting a patch if necessary rather than trying to get CVS all setup).
Thanks,
Andrew -
5. Re: Child object validation
andrew.rw.robinson Feb 17, 2006 12:22 AM (in response to andrew.rw.robinson)HTML got messed up, "addresses" was supposed to be a generic list of type Address in the above example
-
6. Re: Child object validation
andrew.rw.robinson Feb 18, 2006 6:57 PM (in response to andrew.rw.robinson)I'm posting the patch to the hibernate-annotations, now just need to patch the seam side so that it can try to find the component (right now all child validation errors end up in the global messages only).
-Andrew -
7. Re: Child object validation
epbernard Feb 18, 2006 7:53 PM (in response to andrew.rw.robinson)Fantastic!
Where did you post the validator patch?
Can you do it in http://opensource.atlassian.com/projects/hibernate/secure/Dashboard.jspa
BTW, I'm the maintainer of Hibernate Validator.
Some comments:
Do not check the generic type in collection but the actual object type (collection might not be generic) plus sub element might need extra validation compared to superelements
Map.Key should also be validated I think
For the path, List and Map should get their key parent.children[3] or parent.children["Elder"]. For Set and Collection that does not really make sense, since ordering is not guarantied (I can't remember how OGNL describe navigations in Sets and Collection). -
8. Re: Child object validation
andrew.rw.robinson Feb 18, 2006 9:37 PM (in response to andrew.rw.robinson)I put the patch in the JIRA issue:
http://opensource2.atlassian.com/projects/hibernate/browse/ANN-208
I used generic collection refection as hibernate annotations are java5, so thought it was a safe assuption.
This version is working for me, and I really can't spend more time on it. This version should be okay for now, although maybe not as nice as you may like. I wish I could help, but I am getting dangerously close to breaking my deadline. I need to see if I can get seam to find the component now (tricky since each row is not a separate set of components, but the same ones rendered multiple times). -
9. Re: Child object validation
andrew.rw.robinson Feb 18, 2006 11:32 PM (in response to andrew.rw.robinson)I can see what you mean about the validation of the keys and the index issue. I am mostly thinkin on this from a Seam perspective, where I need to make sense of the path. I begs an interesting question of how to handle complex keys and how to represent what element in invalid in the collection.
I know the set iterator and map values iterator will not always produce the correct index. For set, there really is no other choice except maybe the object's id. For Map, the map key would be fine as well in the path. The problem for key and set item ID is that if the objects are complex, then they will not translate into a string path.
As I mentioned, I thought this would be a good "initial" release. Hopefully there is enough in the patch to make it easier for you or another hibernate developer to get a good head start to completing the functionality.
I am about to finish the patch for seam assuming that this code is not changed drastically.
-Andrew -
10. Re: Child object validation
andrew.rw.robinson Feb 25, 2006 5:56 PM (in response to andrew.rw.robinson)If the hibernate-annotations patch I submitted is incorporated, here is the patch to seam for this fix:
Index: ValidationInterceptor.java =================================================================== RCS file: /cvsroot/jboss/jboss-seam/src/main/org/jboss/seam/interceptors/ValidationInterceptor.java,v retrieving revision 1.10 diff -r1.10 ValidationInterceptor.java 5a6,8 > import java.util.StringTokenizer; > import java.util.regex.Matcher; > import java.util.regex.Pattern; 36a40 > @SuppressWarnings("unchecked") 92c96,162 < String clientId = getClientId( facesContext.getViewRoot(), iv.getPropertyName(), facesContext); --- > Pattern indexPattern = Pattern.compile("([^\\.]+?)\\[(\\d)\\]"); > boolean hasIndexes = false; > > UIComponent comp = facesContext.getViewRoot(); > StringTokenizer tokens = new StringTokenizer(iv.getPropertyPath(), "."); > > boolean first = true; > while (tokens.hasMoreTokens()) > { > String property = tokens.nextToken(); > Matcher match = indexPattern.matcher(property); > > if (match.find()) > { > hasIndexes = true; > property = match.group(1); > } > > comp = findComponent(comp, property, facesContext); > > if (comp == null && first) > { > // if the first wasn't found, don't sweat it, it just means the bean wasn't found > // (Usually the JSF content will not have a component for the bean) > if (tokens.hasMoreTokens()) > comp = facesContext.getViewRoot(); > else > break; > } > else if (comp == null) > break; > > first = false; > } > > String clientId = (comp == null) ? > null : comp.getClientId(facesContext); > > if (clientId != null && hasIndexes) > { > // apply the indexes > StringBuilder indexedId = new StringBuilder(); > Matcher match = indexPattern.matcher(iv.getPropertyPath()); > int previousIndex = 0; > while (match.find()) > { > String propName = match.group(1); > String index = match.group(2); > int idIndex = clientId.indexOf(":" + propName + ":", previousIndex); > if (idIndex == -1) > { > indexedId = null; > break; > } > indexedId.append(clientId.substring(previousIndex, idIndex + propName.length() + 2)); > indexedId.append(index); > indexedId.append(":"); > previousIndex = idIndex + propName.length() + 2; > } > > if (indexedId != null) > { > indexedId.append(clientId.substring(previousIndex)); > clientId = indexedId.toString(); > } > } > 97c167 < private static String getClientId(UIComponent component, String id, FacesContext facesContext) --- > private static UIComponent findComponent(UIComponent parent, String id, FacesContext facesContext) 99c169 < String componentId = component.getId(); --- > String componentId = parent.getId(); 102c172 < return component.getClientId(facesContext); --- > return parent; 106c176 < Iterator iter = component.getFacetsAndChildren(); --- > Iterator iter = parent.getFacetsAndChildren(); 110,111c180,181 < String clientId = getClientId(child, id, facesContext); < if (clientId!=null) return clientId; --- > UIComponent result = findComponent(child, id, facesContext); > if (result != null) return result; 116,117d185 < <
-
11. Re: Child object validation
gavin.king Mar 1, 2006 6:20 PM (in response to andrew.rw.robinson)please add it to JIRA, thanks
-
12. Re: Child object validation
andrew.rw.robinson Mar 1, 2006 11:03 PM (in response to andrew.rw.robinson)Sorry to be an idiot, do you want me to create a new JBoss-Seam JIRA issue or just attach the seam patch to the hibernate-annotations JIRA that is already open
-
13. Re: Child object validation
andrew.rw.robinson Mar 2, 2006 12:00 AM (in response to andrew.rw.robinson)I went ahead and added a new JIRA issue as it wouldn't make sense in the hibernate-annotations issue:
http://jira.jboss.com/jira/browse/JBSEAM-144