[bv-dev] Cross parameter validation convergence

[Emmanuel Bernard]

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? :)

> 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?

> > 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.

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.

> 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.

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.

Emmanuel