[bv-dev] Method constraints and TraversableResolver contract

Emmanuel Bernard emmanuel at hibernate.org
Thu Jan 31 07:07:00 EST 2013 

Said a bit differently, here are the consequences.

In the current model, parameter and return values are not going through
the filter of TraversableResolver. This means that if a parameter or
return value is a JPA proxy (lazy object), Bean Validation will by side
effect trigger the loading of the proxy.

That is not an ideal situation IMO but as HAryd points out it's
consistent with the behavior on beans passed to the validate() methods.

There are alternatives to solve this problem but they are not 100%
satisfactory. What would you prefer:

1. not use traversableResolver for parameter and return values and
risking the excessive object loading
2. use traversable resolver for parameters and return values with the
existing methods. This creates not so obvious use of the TR API but the
impact on TR implementors should be minimal. OTOH it's a bit
inconsistent with the bean objects behavior.
3. use traversable resolver for params, return values and beans passed
to the validate methods.  This creates not so obvious use of the TR API.
This makes for a consistent experience but will break TraversableResolver
implementations. Compatibility issues will arise
4. use traversable resolver for params, return values and beans passed
to the validate methods but using a new dedicated method on TR. This
makes for a consistent experience, will be natural contract wise but
will break TraversableResolver implementation. Compatibility issues will
arise.

What is your preference? Ideally with some arguments for it.

Emmanuel

On Wed 2013-01-30 19:06, Hardy Ferentschik wrote:
> Hi,
> 
> after a chat with Emmanuel and Gunnar I would like to elaborate a little on my previous email.
> I still think that not calling TraversableResolver#isReachable and TraversableResolver#isCascadable is the most 
> consistent behaviour and it does not effect existing TraversableResolver implementations. 
> 
> There are a few alternatives though. For completeness here are the two methods from the TraversableResolver interface:
> 
> 	boolean isReachable(Object traversableObject,
> 						Path.Node traversableProperty,
> 						Class<?> rootBeanType,
> 						Path pathToTraversableObject,
> 						ElementType elementType);
> 
> 
> 	boolean isCascadable(Object traversableObject,
> 						 Path.Node traversableProperty,
> 						 Class<?> rootBeanType,
> 						 Path pathToTraversableObject,
> 						 ElementType elementType);
> 
> 
> Note that the actual object passed (traversableObject) is not the object which needs to be validated. It is the object hosting the property
> to be validated and we only get the Path.Node element for the value to be validated. How would that work out for let's say parameter validation?
> The traversable object would be probably null and the Node would be something like "arg0". Hardly valuable information to make a useful decision 
> in a TraversableResolver implementation. Passing the actual parameter as traversable object on the other hand violates the contract of the interface.
> 
> A potential solution would be to introduce a ValueHolder wrapper for parameters and return values and pass the validated value via this wrapper.
> The wrapper acts in this case as traversableObject. Emmauel sketched this approach in this gist: https://gist.github.com/4673863
> Introducing such a wrapper feels of course contrived. It also raises the question we we don't use this approach for normal bean validation as well. 
> In fact for consistency reasons we should probably do that. As a result we would break existing TraversableResolver implementations though. 
> 
> A last solution would be to introduce a whole new method on TraversbleResolver which would be used for parameter and return value validation. Of course
> it would raise the question again whether it should be used for default bean validation as well. This solution obviously breaks completely existing
> implementations.
> 
> Anyways, I just wanted to list the different options. Does anyone have any thoughts on this?
> 
> --Hardy
> 
> 
> On 30 Jan 2013, at 12:13 PM, Hardy Ferentschik <hardy at hibernate.org> wrote:
> 
> > Hi,
> > 
> > Do you really want to call isReachable/isCascadable for the return value and the actual parameters.
> > What would be the traversableObject in this case? null?
> > 
> > Let's have a look at a concrete example - TraversableResolverTest from the TCK
> > 
> > This is the existing test (ignoring for a second that is makes use of the toString representation of pathToTraversableObject which needs to be changed):
> > https://github.com/hferentschik/beanvalidation-tck/blob/HV-673/tests/src/main/java/org/hibernate/beanvalidation/tck/tests/traversableresolver/TraversableResolverTest.java#L66
> > 
> > Now adding a test for parameter and return value validation could look like this:
> > https://github.com/hferentschik/beanvalidation-tck/blob/HV-673/tests/src/main/java/org/hibernate/beanvalidation/tck/tests/traversableresolver/TraversableResolverTest.java#L101
> > https://github.com/hferentschik/beanvalidation-tck/blob/HV-673/tests/src/main/java/org/hibernate/beanvalidation/tck/tests/traversableresolver/TraversableResolverTest.java#L144
> > 
> > IMO the actual number of calls to isReachable and isCascadable is the same. But if I understand your previous mail correctly you expect actual calls for the method parameter and
> > return value itself. Or is there a misunderstanding?
> > 
> > --Hardy
> > 
> > On 10 Jan 2013, at 1:13 PM, Emmanuel Bernard <emmanuel at hibernate.org> wrote:
> > 
> >> Damn it. I changed my mind and thought while writing the email. Forget anything after my signature on the last post. 
> >> Your example is what I had in mind indeed. 
> >> 
> >> On 10 janv. 2013, at 10:30, Gunnar Morling <gunnar at hibernate.org> wrote:
> >> 
> >>> I've created https://hibernate.onjira.com/browse/BVAL-357
> >>> 
> >>>> I think you should call isReachable and isCascadable for params and return values.
> >>> 
> >>>> Of the top of my head I cannot think of a reason why we would need to use isReachable on a parameter
> >>> 
> >>> So do you think isReachable() *is* required or not? Regarding your entity example, did you mean it like this:
> >>> 
> >>> @Entity
> >>> public class Customer {}
> >>> 
> >>> public class CustomerService {
> >>>    public void updateCustomer(@RetailCustomer @Valid Customer customer) {
> >>>    }
> >>> }
> >>> 
> >>> Then I guess it would indeed make sense to call isReachable() and isCascadable() when validating the "customer" parameter.
> >>> 
> >>> --Gunnar
> >>> 
> >>> 
> >>> 
> >>> 2013/1/10 Emmanuel Bernard <emmanuel at hibernate.org>
> >>> That's an interesting question. I think you should call isReachable and isCascadable for params and return values.
> >>> Imagine a constraint validating a JPA entity. You don't want it to be validated if the entity is a proxy. This constraint could access a few of the entity state properties. And that's before cascading.
> >>> 
> >>> isCascadable was a contract added specifically so that the same entity would not be validated over and over if it happened to be referenced by several other entities in a dirty persistence context.
> >>> 
> >>> Open an issue because we need to clarify all that in the spec.
> >>> 
> >>> Emmanuel
> >>> 
> >>> Of the top of my head I cannot think of a reason why we would need to use isReachable on a parameter
> >>> 
> >>> On 9 janv. 2013, at 15:42, Gunnar Morling <gunnar at hibernate.org> wrote:
> >>> 
> >>>> Hi all,
> >>>> 
> >>>> working on the TCK, I'm wondering whether a BV provider should use TraversableResolver#isReachable() and isCascadable()) to check whether a validated method parameter or return value may be accessed/traversed.
> >>>> 
> >>>> I think checking for cascadability might make sense, but I'm not so sure about checking for reachability; can e.g. be a parameter not reachable?
> >>>> 
> >>>> If any of the checks shall be done for method validation, we need to update the TraversableResolver contract (section 4.6.3) which currently explicitly speaks about properties and is limited to the element types FIELD and METHOD.
> >>>> 
> >>>> WDYT?
> >>>> 
> >>>> --Gunnar
> >>>> 
> >>>> 
> >>>> 
> >>>> 
> >>>> 
> >>>> 
> >>>> _______________________________________________
> >>>> beanvalidation-dev mailing list
> >>>> beanvalidation-dev at lists.jboss.org
> >>>> https://lists.jboss.org/mailman/listinfo/beanvalidation-dev
> >>> 
> >>> _______________________________________________
> >>> beanvalidation-dev mailing list
> >>> beanvalidation-dev at lists.jboss.org
> >>> https://lists.jboss.org/mailman/listinfo/beanvalidation-dev
> >>> 
> >>> _______________________________________________
> >>> beanvalidation-dev mailing list
> >>> beanvalidation-dev at lists.jboss.org
> >>> https://lists.jboss.org/mailman/listinfo/beanvalidation-dev
> >> _______________________________________________
> >> beanvalidation-dev mailing list
> >> beanvalidation-dev at lists.jboss.org
> >> https://lists.jboss.org/mailman/listinfo/beanvalidation-dev
> > 
> > 
> > _______________________________________________
> > beanvalidation-dev mailing list
> > beanvalidation-dev at lists.jboss.org
> > https://lists.jboss.org/mailman/listinfo/beanvalidation-dev
> 
> 
> _______________________________________________
> beanvalidation-dev mailing list
> beanvalidation-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/beanvalidation-dev

More information about the beanvalidation-dev mailing list