[hibernate-dev] Feedback on Bean Validation JAX-RS integration

[Emmanuel Bernard]

FYI my reply

Hi guys,
This is a very good proposal and quite aligned with Bean Validation (BV) ideas and with future plans we have for Bean Validation 1.1.
In particular, we plan to add method validations (somewhat close to how we have designed it in Hibernate Validator (HV) http://docs.jboss.org/hibernate/validator/4.2/reference/en-US/html_single/#validator-customoptions-methodvalidation ). We probably should discuss so that JAX-RS (and maybe other specs) could reuse the same way of describing these constraints on method parameters and return value to gain consistency.

I/we have reserves wrt client validation. I've detailed them below. The rest is feedback or comments on specific issues.

Users need to be able to disable validation (even if it's on default) and select the groups they want to see validated (even if the Default group is used by default).
@Validate(of=Extended.class)
public void registerUser( @NotNull @FormParam("firstName") String firstName ) { ... }

@Validate(DISABLED)
public void registerUser( @NotNull @FormParam("firstName") String firstName ) { ... }

Such a system will have to be standardized I think for the method validation feature we plan to add to BV 1.1.

Listening 2 got me confused: do you really want to put the constraint annotation on the setter instead of the getter? I don't think that's totally inline with BV and is not consistent: at the very least the @Email annotation should be a parameter annotation public void setEmail(@Email String email) { }

* Entity Validation

All of these approaches to declare constraints on method parameters (and return values) will be explored and hopefully standardized in Bean Validation.
What I see is that you also have a need to ensure that upon a method call, attributes of the hosting class should be validated. I need to think about that. Intuitively, I don't think this should be standardized in BV but rather left to the integrating framework.

But imagine if we manage to standardize this approach in Bean Validation for all frameworks  in need for lifecycle driven validation (at least the declaration approach), we would get a huge consistency across the platform.

There is "@Provider" in one of your examples. What's the meaning for JAX-RS, why would a constraint validator need to be a provider as well? (It's usually an "implementation detail" attached to a @Constraint and is never seen by client libraries.

"JAX-RS implementations MUST report an error if the type of the validation does not match the return type of the resource method. In the example above, the validator for the @CheckUser annotation must be defined for the type User."
=> Bean Validation must do that already, JAX-RS can delegate the work to BV.

BV need to support Constructor validation (something I forgot)

"The order in which validations are checked is as follows: constraint validations on fields and properties are checked first, on resource classes are checked next and on method parameters are checked last."
What's the reasoning behind enforcing an order? In BV so far, we say that we will return all violations but that the order is undefined.

"The rule for inheritance of constraint annotations is the same as that for all the other JAX-RS annotations (see Section 3.6). Namely, constraint annotations on methods and method parameters are inherited from interfaces and super-classes, with the latter taking precedence over the former when sharing common methods. "
Interesting, we need to investigate that. Where do these rules come from? 
In Hibernate Validator we have stricter rules. In HV we decided against your approach, as preconditions shouldn't be strengthened in sub-types in order to obey to the Liskov substitution principle. Instead of the very conservative approach currently taken by HV (not allowing parameter constraints for one method in several places in a hierarchy) one could also OR-join the constraints from a method hierarchy causing the "weakest" contract in the hierarchy becoming effective. That's the way taken by most DbC frameworks.

s/ConstraintValidationException/ConstraintViolationException/

"A constraint violation error reported for the return type of a resource method should not return a 400 (Bad Request) HTTP code. Instead, this condition should be reported as a 500 (Internal Server Error) response. However, it isn't clear how to write an exception mapper that can discern request vs. response violations."
I'm not sure I follow. I agree that input violations should be 400 and output violations should be 500. Why do you say that writing an exception mapper would be hard? Is exception mapper a specific feature of JAX-RS?

Client side validation
On the client side, you don't know the server contract right, ie the client and the server don't share the contract in a type-safe way?, ie you don't proxy a method call from the client via some REST call and call the same method on the server side with an actual implementation? If that was the case we could reuse the metadata without having to instantiate the constraints manually.
If we cannot share the constraint declaration, we see very little value in the client side validation.

Otherwise, I prefer the option 'Binding using Annotation Proxies' but again my main concern is duplication of constraints declarations between the client and the server which is what BV has be built to avoid :(
Note that we have in Hibernate Validator the notion of programmatic mapping API that lets you associate constraints to properties etc. It uses a slightly different approach to instantiate constraints that could work here too. We create XYZDef classes for each @XYZ constraint class which are a read-write counterpart  and also a GenericConstraintDef somehwat similar to the annotations proxies approaches (see http://docs.jboss.org/hibernate/validator/4.2/reference/en-US/html_single/#programmaticapi ).

"Direct Binding using Validators". because initialize() won't received data, I don't think it's a good idea.

On 8 juil. 2011, at 20:29, Emmanuel Bernard wrote:

> Many thanks guys.
> The good news is that we all are very much aligned in what we think. I am writing an email to send this feedback to the JAX-RS team.
> 
> Emmanuel
> 
> On 1 juil. 2011, at 14:55, Hardy Ferentschik wrote:
> 
>> On Thu, 30 Jun 2011 22:04:39 +0200, Gunnar Morling <gunnar.morling at googlemail.com> wrote:
>> 
>>> Some minor remarks:
>>> 
>>> * In listing 2 setEmail() instead of a getter is annotated.
>>> 
>>> I'm not sure whether this is intentionally, otherwise it would make
>>> sense to use getters as in BV.
>> 
>> Good catch. I was assuming getters. Need to check whether this is really
>> intentionally.
>> 
>> 
>>> * "The order in which validations are checked is as follows:
>>> constraint validations on fields and properties are checked first, on
>>> resource classes are checked next"
>>> 
>>> I don't think we could currently ensure such an order in HV but I
>>> guess it could be implemented. The question is whether this is really
>>> required.
>> 
>> I understood this to be more of a JAX-RS thing. I thought that there is a
>> way for same in the framework to distinguish between form parameter processing
>> and "resource class populating". If so it is just a question where and how
>> to call the validator. After re-reading the examples I am not so sure
>> anymore. The examples just use MyResourceClass, but still seem to distinguish
>> between resource class and non resource class.
>> 
>> 
>>> * "If the getUser() method in [the implementing class] MyResourceClass
>>> is decorated with any annotation ... all of the annotations in the
>>> [implemented] interface will be ignored.
>> 
>>> I think this might require some more consideration. In HV we decided
>>> against such an approach, as preconditions shouldn't be strengthened
>>> in sub-types in order to obey to the Liskov substitution principle
>>> [1].
>>> Instead of the very conservative approach currently taken by HV (not
>>> allowing parameter constraints for one method in several places in a
>>> hierarchy)
>> 
>> I wonder what they think about this approach?
>> 
>> --Hardy
> 
> 
> _______________________________________________
> hibernate-dev mailing list
> hibernate-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/hibernate-dev