[bv-dev] Method constraints and TraversableResolver contract
Gunnar Morling
gunnar at hibernate.org
Tue Feb 5 05:43:26 EST 2013
+1 for option #1 as well as it is in line with the current behavior.
I think BV providers could offer option #4 on top if they want to (of
course binding implementors of such a contract). If that proves useful, we
could standardize option #4 in a future revision.
--Gunnar
2013/1/31 Hardy Ferentschik <hardy at hibernate.org>
> Hi,
>
> +1 for option 1. It is imo inline with the current behaviour of Bean
> Validation. Besides we are
> talking about not passing the actual parameter / return value to the
> traversable resolver. As we
> cascade validation into the parameters / return values we still would call
> the traversable resolver
> as in standard bean validation. In this case the parameter resp. return
> value become the 'traversableObject'
> parameter for the isReachable and isCascadable calls.
>
> I am not a big fan of introducing artificial wrappers and I don't like
> using the traversable resolver interface in
> a non intended way, for that reason I would chose option 4 as my second
> choice.
>
> --Hardy
>
>
> On 31 Jan 2013, at 1:07 PM, Emmanuel Bernard <emmanuel at hibernate.org>
> wrote:
>
> > 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
> > _______________________________________________
> > 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.jboss.org/pipermail/beanvalidation-dev/attachments/20130205/8d4f5f4e/attachment-0001.html
More information about the beanvalidation-dev
mailing list