[bv-dev] Cross parameter validation convergence

[Matt Benson]

On Wed, Sep 5, 2012 at 2:51 AM, Gunnar Morling <gunnar at hibernate.org> wrote:
>
> Am 04.09.2012 00:55 schrieb "Emmanuel Bernard" <emmanuel at hibernate.org>:
>
>
>>
>> On Sat, Sep 01, 2012 at 06:38:58PM +0200, Gunnar Morling wrote:
>> > Based on our discussions around the issue, I'm wondering whether we
>> > should put cross-parameter validation into BV 1.1 at all.
>> >
>> > AFAIK none of the BV implementations provides such a feature, so with
>> > this we'd be pretty much on the "innovating things" side of what a JSR
>> > can do, opposed to the "standardization" side of the spectrum. IMO it
>> > would be better to see cross-parameter validation be implemented by at
>> > least one BV provider to gather some experiences and then standardize.
>> >
>>
>> I personally would feel that we are missing a big and well defined set of
>> use cases by leaving this unsolved. Look at how much complain we already
>> have for cross field validations where we have *an actual solution*.
>>
>> Plus by not addressing it we might have APIs that won't support the
>> notion down the road.
>>
>> > Note that a user can work around the lack of cross-parameter
>> > validation by pulling the concerned parameters into a parameter
>> > object, putting a class-level constraint on it and annotating the
>> > parameter with @Valid.
>>
>> We can't realistically force people to change the natural signature of
>> their method just to use Bean Validation. Do you really want to be the
>> one in front of a 200 people crowd telling them that they need to change
>> their method signature on the ground that you did not feel like
>> supporting cross parameter validation? :)
>
> Sounds like an interesting experience, being thrown at with rotten
> vegetables by a furious crowd complaining about cross-parameter validation
> not being supported ;)
>
>>
>> > If we really want to add this to BV 1.1, IMO we should only add the
>> > generic approach and let implementors experiment with the type-safe
>> > one.
>>
>> What is your reasoning for not wanting the type safe approach in?
>
> It's just the number of open questions around this which I'm a bit concerned
> about. For the other method validation things we could more or less rely on
> the experiences from the existing implementations or added smaller things
> like ParameterNameProvider.
>
> Compared to that, there are several questions around (type-safe)
> cross-parameter validation we'd be answering in the spec for the first time.
> But it may be I'm just over-cautious here.
>
>>
>> > > We can do it on the same CrossParameterConstraintValidator
>> > > implementation [...] If the method validated has a matching signature, we
>> > > use it, otherwise we use the generic method.
>> >
>> > Wouldn't this contradict the idea of type-safety? If the validated
>> > method signature gets changed without adapting the validator, silently
>> > the generic method would be used, and also an annotation processor
>> > couldn't raise an error since there is the generic method.
>>
>> That's a very good point and the solution needs to address that.
>> The easiest solution I can see here is to have the following interfaces
>>
>>     interface CrossParameterConstraintValidator<A extends Annotation> {
>>         void initialize(A constraintAnnotation);
>>     }
>>
>>     interface GenericCrossParameterConstraintValidator<A extends
>> Annotation> {
>>         boolean isValid(Object[] parameterValues,
>> ConstraintValidatorContext context);
>>     }
>>
>> That brings back an idea Matt had that I forgot to reply to. Should we
>> enforce at most one `isValid` method per
>> `CrossParameterConstraintValidator`? With that problem in mind I am
>> tempted to think that this is a good rule.
>>
>> People can still add several validators per cross parameter constraint.
>
> Does it really make a difference whether two (typed) isValid() methods are
> declared on one or two validators? I see this a bit similar to the
> invocation of overloaded methods. As long as there is exactly one matching
> signature, we can invoke it.

It had escaped me that apparently the idea I had got mutated at some
point.  I was thinking more along the lines that a ValidationException
should be thrown should more than one *generic* validation method be
found for any particular cross-parameter validation constraint in a
given ValidatorFactory.

Matt

>
> It may be challenging though to find the right isValid() method on several
> validators in case of overloaded methods. In this respect it might even be
> easier to allow for only one cross-parameter validator per constraint with
> possibly several isValid() methods.
>
>>
>> That leaves one question: should we allow the mix of
>> `GenericCrossParameterConstraintValidator` and non
>> `GenericCrossParameterConstraintValidator` in a single constraint?
>> That would indeed defeat the type-safety belt. On the other hands, it
>> looks like a useful thing to be able to mix them when adding validators
>> via the XML descriptor.
>
> So far I don't see a good way around the problem of a transparent fallback
> to the generic method. Safest (but most restrictive) would be to allow to
> have either the generic method or exactly one matching type-safe one over
> all validators for a constraint.
>
>>
>> > Thinking more about it, I'm wondering how we would represent
>> > cross-parameter constraints in ConstraintViolations. What should be
>> > returned by getInvalidValue()/getLeafBean()?
>>
>> I agree that this is not intuitive but we could return the `Object[]` of
>> parameters when `getInvalidValue()` is called.
>>
>> The more general problem is that cross-parameter constraints (or
>> should it be constraint validators) do not return the actual parameters
>> being considered in violation.
>
> Right. I guess we could either return the concerned indices from isValid()
> (returning an empty list meaning no violation) or require isValid()
> implementors to create the violation themselves.
>
>>
>> I imagine we could have a way to return parameter indexes as part of the
>> `ConstraintViolation` or the `ParametersDescriptor` (see below).
>>
>> But do we want such a feature? And if yes, should it be statically defined
>> or dynamically
>> defined. And if static, should it be hosted on the cross parameters
>> constraint or the cross parameters constraint validator implementation?
>>
>> Note that if we manage to crack this nut, that's a step towards better
>> support for cross field validations.
>>
>> > getPropertyPath() might
>> > be an issue, too. For single parameters we have one node representing
>> > the concerned parameter at the end of the violation's path, but this
>> > wouldn't work here. Perhaps the path could just end with the method
>> > node in that case.
>>
>> I think we just need to introduce a `ParametersDescriptor` that would
>> represent this particular case. That seems the most natural approach.
>
> Yepp. I think best is to try some things out in code and look how it works.
> The use case could be an interceptor throwing an exception for violated
> cross parameter constraints which prints out all required information in a
> comprehensible manner.
>>
>> Emmanuel
>
> --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
>