[hibernate-issues] [Hibernate-JIRA] Commented: (HV-441) Refactor BeanMetaDataImpl

[Hardy Ferentschik (JIRA)]

    [ http://opensource.atlassian.com/projects/hibernate/browse/HV-441?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=40061#action_40061 ] 

Hardy Ferentschik commented on HV-441:
--------------------------------------

The intend was always to hide (abstract) whether we deal with a field or property as much as possible. Something like a {{AggregatedPropertyMetaData}} sounds reasonable and is inline w/ what I mentioned about unification. 

Regarding the builders we have to see how separate we can keep them, since there are dependencies between them. For example the XML configuration can mark certain annotations to be ignored. I guess that's what you mean with chaining. I also like the idea of having the final model class immutable. Initially this was the intend of the {{BeanMetaData}}, but got broken by introducing additional configuration means (programmatic, XML).

Even though it would be nice to get this into 4.2 it is no requirement. This is not anything the user gets affected by. I don't have time right now to start this refactoring and you are on the critical path for the 4.2 release. It feels like it makes more sense to get 4.2 out and start 4.2.x with a refactoring of the meta model. 

> Refactor BeanMetaDataImpl
> -------------------------
>
>                 Key: HV-441
>                 URL: http://opensource.atlassian.com/projects/hibernate/browse/HV-441
>             Project: Hibernate Validator
>          Issue Type: Task
>          Components: engine
>            Reporter: Hardy Ferentschik
>             Fix For: 4.x
>
>
> Due to the introduction of method level validation the {{BeanMetaDataImpl}} abstraction started leaking. For example parts of the configuration data is now held at multiple places. We have to start addressing this and pull things together again.
> I started a discussion around this with Gunnar. My original question was:
> {quote}
> Could you have a look at my branch https://github.com/hferentschik/hibernate-validator/commits/HV-433. The tests for this issue are passing (CascadingWithConstraintMappingTest), in fact the whole test suite is passing. The problem I encountered was around MethodMetaData and its cascading flag. In the case of the test case it was not properly set. The code was just looking for the @Valid annotation, but also has to check whether the method is explicitly (xml or programmatic api) configured for cascading.
> If you have a look at the branch you see how I changed that. The question which came up in my mind is, whether there is not duplication between MethodMetaData and BeanMetaData (at least regarding the cascading).
> {quote}
> Gunnar's response:
> {quote}
> Yes, that's certainly right. Actually I'm not quite happy with BeanMetaDataImpl altogether atm. It is pretty complex code, is quite huge and I think we should tidy up/restructure it for 4.3. 
> When implementing method validation I found that the meta model was not expressive enough for the new method stuff, which was why I came up with more powerful model classes accompanying BeanMetaData such as MethodMetaData and ParameterMetaData. The idea was that the engine (ValidatorImpl) could simply ask a MethodMetaData whether it requires cascaded validation or not for instance. At the same time I tried to to modify existing functionality in BeanMetaDataImpl not more than necessary. So currently the situation is, that method validation is based on MethodMetaData#isCascading(), while standard bean/property validation relies on BeanMetaDataImpl#cascadingMembers. In BMDI's constructor both structures are maintained. 
> Actually I'm proceeding on that path for HV-371. There I introduced AggregatedMethodMetaData which represents a given method *and* all the methods up in the hierarchy which it overrides/implements (basically this replaces the Map<Class<?>, Map<Method, MethodMetaData>> methodMetaConstraints with Map<Method, AggregatedMethodMetaData> methodMetaData. This should make things much easier in ValidatorImpl, which now can deal with one AggregatedMethodMetaData object instead of iterating over the complete inheritance hierarchy (So for instance AMMD#isCascading() answers whether a cascaded validation is required for the given method no matter where in the hierarchy it was marked with @Valid).
> So I definitely think we should get these things more aligned again but I've got the feeling we can live with the situation temporarily. WDYT?
> {quote}

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://opensource.atlassian.com/projects/hibernate/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira