[bv-dev] Cross parameter validation convergence

[Emmanuel Bernard]

On Mer 2012-09-05  9:51, Gunnar Morling wrote:
> > > 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.

That is a legitimate fear but, I'd rather we give all that we've got and
try to build the feature even if we decide to pull it out in the last
month before the release.

My reasoning is that we will have see the option(s) we have for the
future and won't cut us into a corner.

> >
> > > > 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 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 can be challenging and I don't think having all methods hosted on
the same class solve the problem.

We can decide to honor exact method + parameter matching in which case
the resolution is quite simple. Or we can try and apply the full method
overloading mechanism as defined by the JLS.

I have a hard time to see a use case requiring the full method
overloading approach.

One open question is generics and how we plan to address that problem.

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

The natural way would be to have`isValid()` implementors indeed build a
violation report 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.

Yes exactly. We should even have early working prototypes to see how
people react to the feature.

Emmanuel