Some feedback reading the spec, and having used OVal (which I'm surprised to not see mentioned that much).

1) All parameters and return types should document whether they accept or return null. If null is accepted its meaning should be described. The JSR-310 code uses:



2) 2.1 - I find the naming of Constraint confusing. The constraint should be the annotation itself, such as @Size. The class which performs the check should be named Check (as in OVal) or Validator (but this clashes). Thus rename Constaint to Check, and rename ConstraintValidator to Constraint.

3) @NotNull is confusing as it is not the same as JSR305/308. It may well be desirable to annotate a field with both annotations, resulting in one being fully qualified, which is very messy.

Could you consider prefixing all constraints? This would aid clarity and readability in general:


Possible prefixes are 'Check', 'Validate' or 'Val'. For example, @CheckValid and @CheckUpperCase are a lot more expressive than @Valid or @UpperCase.

4) 2.4 - The current Constraint interface has a major flaw in that it does not pass in enough data. Oval passes in four parameters, including the object being validated:


In my work use of OVal we use the validatedObject parameter to perform cross-field validation:


This is very neat and would be a major loss. In addition, JSR303 doesn't provide access to the Validator, which is the bootstrap object.

5) 2.4 - ConstraintContext should not be an interface. Using an interface here means that the key validation API that will be widely implemented cannot be extended in a version 2 of JSR303. I recommend using a class (possibly abstract) instead.

6) 3.4 - Default should be named DefaultGroup. Default is rather meaningless by itself.

7) 3.4.2 - This seems to indicate that constraints are not evaluated in order. This is not acceptable. By default annotations on a field need to be evaluated in the order that they are declared. This is because webpages may only be able to display one error at a time, and thus will just select the first returned error for each field. This needs to be predictable. The group sequencing is way too complex for this simple requirement.

8) I found the sections on group sequences difficult to follow. I don't know why I'd need them or how I'd use them. They obviously make the spec a lot more complex, so I'd ask if you really need them? (ie. just let the user call the Default group followed by the Detailed group. Don't over-complicate)

9) 4 - Validator should not be an interface. As an interface, you won't be able to extend it in a version 2 of the spec. Similarly, ConstraintViolation.

10) Much of the info from ConstraintViolation should be made available to implementors of constraint validators (root bean, leaf bean, property path, groups, descriptor...)

11) The bootstrap process has builders, factories, factories of factories, and more. This seems remarkably complicated (I don't have time to study it). OVal has one class - Validator - which is thread-safe. Shouldn't this spec simply provide a simple bean-style API, and allow other specs to integrate it how they want? Maybe I'm missing something...

Again, I'd note that since the key participants are interfaces, you're not able to extend in a second version of the spec.

12) 5.5 - OVal allows us to get the constraint validation implementation. We use this to get hold of the maximum length of a field which can go in HTML. The metadata API seems quite tricky to use for simple cases like this (eg. find the MaxLength annotation on this property and get the configured values)

Overall, the spec is well written, and I'm reasonably happy with it. However, I'm not yet convinced that I would receommend migration from OVal, especially wrt item #4 above.

Stephen Colebourne
co-spec lead JSR-310

Sebastian (OVal fame) is part of the EG and joined rather recently but he is contributing actively. Part of the spec has been heavily influenced by his contribution.



noted the point.


Agreed we renamed things in the latest version. check out http://in.relation.to/10679.lace



Well I have hard time understanding why the java namespace mechanism is a bad thing. The problem with the prefix, say "Check", is that it equally can apply to a 305 constraint. So it does not really solve any of the confusion problem. I opened a discussion with Roberto (Sun EE) and Bill P on the subject of 305 / 303, @NotNull / @Nonnull is really the main clashing name. The rest seems fine.



It does, you need to use a class level constraint which accept the java bean instance. you can then access sub properties. There were various problems in allowing cross property validation from a property level constraint, and class-level validation does solves that elegantly (IMO).


How is that not be extendable in future versions? This interface is implemented by a Bean VAlidation provider, not by a client.



well it's in the group package :) We discussed the point, there were no clear agreement. We ended up choosing the smallest name.



Web page don't accept more than one error per field? JSF might not, but fore sure that's not a hard limitation.

The problem with ordering is that the order in which annotations are retrieved for an element are not guaranteed to be predictable by the JVM / compiler, AFAIK. If it's not the case (and annotations are retrieved in predictable order), then I am ok with adding this ordering constraint (ie for a given element (field, method, class), constraints are validated in the order they are retrieved by the reflection API.
let me know.


People vehemently wanted a way to order constraints, hence the introduction of group sequence.
You approach might be a solution but it forces every client to remember the ordering, leading to duplication.


Same remark. Look at JPA, EntityManager has new methods in JPA 2, this will be backward compatible.



Can you describe a use case for each?


I renamed a lot of the bootstrap elements in the latest version, it should be more readable http://in.relation.to/10679.lace

The complexity (comparing with Hibernate Validator legacy and OVal) is due to:
- support for multiple implementations of the spec
- using the fluent API design simplify the API usage (but complexify its design and implementation)

A default user will do



Same remark


I don't follow you, can you detail the use case for me?



Generally speaking the spec supports extensibility. So Oval implementing Bean Validation can offer these extra functionalities.
I don't think they are needed though (at least in a spec).
We will see :)

Thanks for your feedback.

BTW I wanted to add that groups is a rather advanced feature that will not be needed 80% of the time, sequence even less. So the complexity will only show up when complexity is needed.

So, how can a class level constraint check two pairs of start/end dates?



You can only have one annotation of a given type at the class level. This also isn't natural, as the validation is really part of the field, as I want to associate the error message with the field, not the class.





In the Java I work with, published interfaces don't get changed. If you are adopting a strategy of interfaces changing with revisions, then this point is irrelevant. I'd prefer to see fixed interfaces.




OVal manages to return them in a predictable order, so I'd suggest asking them. Predictability like this is pretty important.




I'd ask the opposite question - why hide the information? The info will be useful to someone sometime.




In the web framework, you can write



This ties the validation to the web page (its a simple way to access the meta-data)
[/code]

We have a construct in the spec allowing multiple constraints of the same type.



And a bean level constraint can if it makes sense to apply messages on specific properties ( see ConstraintValidatorContext.addError(String property, String message) passed to isValid(Object, ConstraintValidatorContext) )




I will ask them but frankly, the compiler/VM does not guarantee the order (some implementations might) so I don't see how they can do better.



root bean: root bean is not always available esp when we validate individual values. So validator implementations cannot rely on it.

leaf bean, same as root bean.

propertyPath: we might be able to pass this info but I don't see why this is useful. I can't seem to find a use case for a validation dependent on the "parent" context. If we turn out to be wrong this is something we can add to the ConstraintValidatorContext in a future rev of the spec.


In BV that would be


Granted it's more complex but:
- it's due to the fact that BV supports the same constraint multiple times
- it's due to the fact that BV supports the notion of groups
- this kind of code will be handled by the Web framework in the future (not the user)

That gave me some ideas on smoothing a bit the API though (more generics etc).

What I don't understand though is why you want to tie the constraint validator implementation (ie the one tied to a given constraint annotation) with your web framework?

Well, this is a significantly worse API for a user to deal with. I want annotations that affect a field to be located on the field, not on the class. I appreciate that you've got a clever way to add multiple annotations, but that doesn't solve the usability part. Would it really be that hard to add the validated object as an argument to the validation code?



Well, that code is definitely not attractive (bear in mind that not all of your users like existing web frameworks, and some choose to write their own).

The use cases for this are pretty obvious. I create a JavaBean, such as PersonSearchRequirements, that takes the relevant input data. Each field is annotated with maximum length, minimum length and whether it is mandatory or not. The framework can interpret this input and add the html maxlength attribute to text fields, javascript for allowed characters and mandatory, and css styles to indicate mandatory. Everything driven from one centralised annotation.

The key point is that it does not affects one field but two fields.
It also poses significant problems for systems that want to test the validity of a given value before it is actually pushed to the model (JSF is one case, but any binder would use that too). because you know a field/getter level constraint applies on the field/getter only, you can validate it in isolation. This is no longer the case if you allow a constraint to sneak into other contexts (in your case an other property).
That would make BV fail instead of having a "significantly worse API to deal with" - your call by the way :)


Yes I understand, but we are talking about one class on the entire web framework that needs to deal with the metadata API. This is not a widely used API (LOC wise). Simplifying the API would mean simplifying the number of use cases BV supports.



OK. That's the point of Bean Validation so we are in phase here.

So you are not advocating to pass javax.validation.Validator into the ConstraintValidator API (ie the logic validating a value for a given constraint). To solve your use case, you need a Validator accessible in your web framework.
In the Bean Validation API model, the constraint validator implementation (say Size for collections) does not understand any particular web framework nor update the web component metadata. Instead, the web framework query the BV metadata API and bind constraints (in our case @Size) to a web framework metadata.
Remember BV is agnostic to other layers (web framework, persistence framework etc). But other layers can rely on a standard Bean Validation metadata API.