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

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}