Hi<div><br></div><div>I have very few concerns with the generic approach, I can't see why it couldn't be stable for some time, we can provide it now and it should cover all use cases albeit without type safety? It's the type safe approach I'd have more concerns about a future spec. wanting to update and finding itself hamstrung by a decision now, perhaps it is biting off more than a JSR should. Happy to be persuaded otherwise though.</div>
<div><br></div><div>Another +1 for not validating parameters and return values in a single constraint.</div><div><br></div><div>Regards</div><div>Rich</div><div><br><br><div class="gmail_quote">On 1 September 2012 17:38, Gunnar Morling <span dir="ltr"><<a href="mailto:gunnar@hibernate.org" target="_blank">gunnar@hibernate.org</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi,<br>
<br>
Based on our discussions around the issue, I'm wondering whether we<br>
should put cross-parameter validation into BV 1.1 at all.<br>
<br>
AFAIK none of the BV implementations provides such a feature, so with<br>
this we'd be pretty much on the "innovating things" side of what a JSR<br>
can do, opposed to the "standardization" side of the spectrum. IMO it<br>
would be better to see cross-parameter validation be implemented by at<br>
least one BV provider to gather some experiences and then standardize.<br>
<br>
Note that a user can work around the lack of cross-parameter<br>
validation by pulling the concerned parameters into a parameter<br>
object, putting a class-level constraint on it and annotating the<br>
parameter with @Valid.<br>
<br>
If we really want to add this to BV 1.1, IMO we should only add the<br>
generic approach and let implementors experiment with the type-safe<br>
one.<br>
<br>
> 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.<br>
<br>
Wouldn't this contradict the idea of type-safety? If the validated<br>
method signature gets changed without adapting the validator, silently<br>
the generic method would be used, and also an annotation processor<br>
couldn't raise an error since there is the generic method.<br>
<br>
Thinking more about it, I'm wondering how we would represent<br>
cross-parameter constraints in ConstraintViolations. What should be<br>
returned by getInvalidValue()/getLeafBean()? getPropertyPath() might<br>
be an issue, too. For single parameters we have one node representing<br>
the concerned parameter at the end of the violation's path, but this<br>
wouldn't work here. Perhaps the path could just end with the method<br>
node in that case.<br>
<br>
Thoughts?<br>
<br>
--Gunnar<br>
<br>
<br>
2012/8/29 Matt Benson <<a href="mailto:mbenson@apache.org">mbenson@apache.org</a>>:<br>
<div class="HOEnZb"><div class="h5">> On Wed, Aug 29, 2012 at 4:03 AM, Emmanuel Bernard<br>
> <<a href="mailto:emmanuel@hibernate.org">emmanuel@hibernate.org</a>> wrote:<br>
>> I would like to bring home cross parameter validation which is one of the main open issues in method validation.<br>
>> We have been discussing it for literally 11 months, it's time to converge. If you could give your feedback quickly, Gunnar<br>
>> might be able to add it to the spec in the next few days. I am already pushing him harder than I should to<br>
>> move BV forward so give him some help :)<br>
>><br>
>> The following is a less readable copy of what you can find at <a href="http://beanvalidation.org/proposals/BVAL-232/" target="_blank">http://beanvalidation.org/proposals/BVAL-232/</a><br>
>><br>
>> ### Cross parameter validation and return value<br>
>><br>
>> Do we agree that validating both the parameters and the return value in a single<br>
>> constraint validator is not a use case we want to cover?<br>
>> To me that would be awkward to support it as we would need to execute the method<br>
>> to validate it.<br>
>><br>
><br>
> +1, yet nothing should be done to preclude adding this in a later<br>
> version of the spec.<br>
><br>
>> ### Where to host cross-parameter constraints<br>
>><br>
>> We use the method as host to the return value constraints and possibly `@Valid`.<br>
>> That is unfortunately also the natural place for cross parameter constraints.<br>
>><br>
>> I cannot think of another place to put them. There is also no easy way to add a<br>
>> visual cue and differentiate a regular constraint from a cross param constraint<br>
>> except by its meta annotation. We would rely on the user adding a visual cue in<br>
>> the constraint name. Which kind of cue? Put `param` in the constraint name?<br>
><br>
> In the end this is not something that should be enforced anyway, so I<br>
> suspect far too much thought has been given to it already. ;) Using<br>
> the CheckRetypedPasswordParameter example, I would personally prefer<br>
> CheckRetypedPasswordParams and extrapolate that to "ends with<br>
> 'Params'". Find an example or two everyone is comfortable with,<br>
> extrapolate a *recommendation* as in the example of .@List for<br>
> multiple constraints and move on. :)<br>
><br>
>><br>
>> Any one can think of a better approach?<br>
><br>
> Nothing from me.<br>
><br>
>><br>
>> #### Bean Validation class<br>
>><br>
>> @interface CrossParameterConstraint {<br>
>> public Class<? extends CrossParameterConstraintValidator<?, ?>>[] validatedBy();<br>
>> }<br>
><br>
> Why <?, ?> on the CPCV signature? I assume this is an error as it's<br>
> not consistent with the CheckRetypedPasswordValidator example below.<br>
><br>
>><br>
>> interface CrossParameterConstraintValidator<A extends Annotation> {<br>
>> void initialize(A constraintAnnotation);<br>
>> [...]<br>
>> }<br>
>><br>
>>> Question: does these different annotations/interfaces affect the metadata API?<br>
><br>
> I haven't looked at the metadata API in depth where it applies to<br>
> method validation, but it might be nice to differentiate<br>
> cross-parameter constraints from return value constraints at the<br>
> metadata level to save users the trouble.<br>
><br>
>>><br>
>>> Question: can the same constraint annotation be annotated by both<br>
>>> `@Constraint` and `@CrossParameterConstraint`: `@ScriptAssert` is a good candidate<br>
>>> Note: how does composition plays into this?<br>
>><br>
>> #### Constraint coder class<br>
>><br>
>> @CrossParameterConstraint(validatedBy=CheckRetypedPasswordValidator.class)<br>
>> @interface CheckRetypedPasswordParameter {<br>
>> String message() default "...";<br>
>> Class<?>[] groups() default {};<br>
>> class<? extends Payload>[] payload();<br>
>> }<br>
>><br>
>> class CheckRetypedPasswordValidator implements<br>
>> CrossParameterConstraintValidator<CheckRetypedPasswordParameter> {<br>
>> ...<br>
>> }<br>
>><br>
>> #### User code<br>
>><br>
>> class AccountService {<br>
>> //cross param constraints<br>
>> @CheckRetypedPasswordParameter<br>
>> //return value constraints<br>
>> @Valid @NotNull<br>
>> User createUser(@NotEmpty String username, @Email String email, String password, String retypedPassword);<br>
>> }<br>
>><br>
>> ### What is the cross parameter constraint validator contract?<br>
>><br>
>> There has been two leading proposals. the others are described in the<br>
>> [previous proposal][previous proposal].<br>
>><br>
>> #### Generic approach<br>
>><br>
>> interface CrossParameterConstraintValidator<A extends Annotations> {<br>
>> void initialize(...) { ... }<br>
>> boolean isValid(Object[] parameterValues, ConstraintValidatorContext context);<br>
>> }<br>
>><br>
>> #### Type-safe approach (with annotation processors)<br>
>><br>
>> A more type-safe approach is to reuse the parameters signature of the method to match.<br>
>> While the Java compiler cannot discover problems, both an annotation processor and the bean validation provider at runtime<br>
>> can detect inconsistent contracts and raise respectively compilation errors and deployment time exception.<br>
>><br>
>> class CheckRetypedPasswordValidator implements<br>
>> CrossParameterConstraintValidator<CheckRetypedPasswordParameter> {<br>
>> void initialize(...) { ... }<br>
>> boolean isValid(String username, String email, String password, String retypedPassword,<br>
>> ConstraintValidatorContext context) {<br>
>> ...<br>
>> }<br>
>> }<br>
>><br>
>> #### Discussions<br>
>><br>
>> I think we must put the generic approach in because that's the only way to write non<br>
>> method specific cross parameter constraints. Two examples of such constraints are<br>
>><br>
>> - script based constraints<br>
>> - generic password retype checks based on the parameter indexes<br>
>> `@AreEqual(indexes={2,3}, message="Passwords must be identical")`<br>
>><br>
>> So the remaining question is do we also support the type-safe approach in parallel?<br>
>> I am inclined to think yes. We can do it on the same `CrossParameterConstraintValidator`<br>
>> implementation:<br>
>><br>
>><br>
>> class CheckRetypedPasswordValidator implements<br>
>> CrossParameterConstraintValidator<CheckRetypedPasswordParameter> {<br>
>> void initialize(...) { ... }<br>
>><br>
>> boolean isValid(Object[] parameterValues, ConstraintValidatorContext context);<br>
>><br>
>> boolean isValid(String username, String email, String password, String retypedPassword,<br>
>> ConstraintValidatorContext context) {<br>
>> ...<br>
>> }<br>
>> }<br>
>><br>
>> If the method validated has a matching signature, we use it, otherwise we use the generic<br>
>> method.<br>
>><br>
><br>
> I am inclined to think this can be done as an implementation detail of<br>
> a given CPCV. However, is it intended that multiple CPCV impls might<br>
> be enabled for a given CPC type, each presumably handling a different<br>
> method signature? It might actually be simpler in the long run to<br>
> promote the type-safe approach such that at most one CPCV with no<br>
> typesafe declarations could be enabled. Perhaps the spec should<br>
> provide an abstract TypesafeCrossParameterConstraintValidator that<br>
> makes the correct typesafe call. Perhaps this class could provide a<br>
> method to check whether a given signature is supported, or this method<br>
> could be specified on the CPCV interface.<br>
><br>
>> Do we keep the generic `isValid` method on the interface, thus forcing people to implement<br>
>> it? Or is that an optional non constrained contract?<br>
>> I am tempted to think that forcing is a better approach (the implementation can raise an<br>
>> exception). Thoughts?<br>
><br>
> +1<br>
><br>
> Matt<br>
><br>
>><br>
>> [jira]: <a href="https://hibernate.onjira.com/browse/BVAL-232" target="_blank">https://hibernate.onjira.com/browse/BVAL-232</a><br>
>> [previous proposal]: /proposals/BVAL-241/#cross_parameter<br>
>><br>
>><br>
>> _______________________________________________<br>
>> beanvalidation-dev mailing list<br>
>> <a href="mailto:beanvalidation-dev@lists.jboss.org">beanvalidation-dev@lists.jboss.org</a><br>
>> <a href="https://lists.jboss.org/mailman/listinfo/beanvalidation-dev" target="_blank">https://lists.jboss.org/mailman/listinfo/beanvalidation-dev</a><br>
> _______________________________________________<br>
> beanvalidation-dev mailing list<br>
> <a href="mailto:beanvalidation-dev@lists.jboss.org">beanvalidation-dev@lists.jboss.org</a><br>
> <a href="https://lists.jboss.org/mailman/listinfo/beanvalidation-dev" target="_blank">https://lists.jboss.org/mailman/listinfo/beanvalidation-dev</a><br>
_______________________________________________<br>
beanvalidation-dev mailing list<br>
<a href="mailto:beanvalidation-dev@lists.jboss.org">beanvalidation-dev@lists.jboss.org</a><br>
<a href="https://lists.jboss.org/mailman/listinfo/beanvalidation-dev" target="_blank">https://lists.jboss.org/mailman/listinfo/beanvalidation-dev</a><br>
</div></div></blockquote></div><br></div>