[
http://opensource.atlassian.com/projects/hibernate/browse/HV-441?page=com...
]
Gunnar Morling commented on HV-441:
-----------------------------------
Yes, we should double-check all changes to {{BeanMetaDataImpl}}, the pull request workflow
on GitHub seems really valuable here. I think in the end things will be much easier to
understand, with clearly focused classes.
{quote}
If this is the paths we are taking (MethodMetaData) for fields and everything else.
{quote}
I'm not sure I understand. Do you mean we should have a similar type for fields
({{FieldMetaData}})?
My idea for the general approach is that we should have rich model classes which each
encapsulate one single concept/entity. An example is
[
AggregatedMethodMetaData|https://github.com/gunnarmorling/hibernate-valid...]
which represents one given method and all the super type methods which it
overrides/implements. It provides simple to use functionality such as {{isCascaded()}}
etc. which encapsulates the complexity of the hierarchy behind. This makes things much
easier for {{ValidatorImpl}} for instance, which simple can ask the model whether a
cascaded validation is required. Of course any redundancies should be removed by putting
each functionality to the appropriate model class.
As you write the creation process for the model should be separated from the model itself.
So ideally all the annotation retrieval stuff should be removed from BeanMetaDataImpl to a
dedicated builder class dealing with these things. There might be three distinct model
builders (for annotation-based models, the programmatic configuration API and XML).
WDYT, for when should we approach these issues? 4.2, or later? I feel 4.2 would make
pretty much sense, but of course that means that we'll need some more time for this
release.
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....
-
For more information on JIRA, see:
http://www.atlassian.com/software/jira