Hi,
compliments for the great draft, it looks really promising and I think, beans validation will be a great success. As I read through the spec, the following questions/ideas came to my mind:
- The question was raised before, but I didn't find an answer: Couldn't the Constraint interface be parametrized with the field type validatable by a concrete implementation as follows:
Code:
public interface Constraint<A extends Annotation, T> {
void initialize(A constraintAnnotation);
boolean isValid(T object, ConstraintContext constraintContext);
}
That way, implementations could be created in a type-safe manner. In case a constraint annotation is allowed at multiple types, this would require a separate Constraint implementation for each type, but I don't think, this is a big issue. Actually, it would lead to cleaner code without "if-instanceof-...-else-if-instanceof-..." cascades.
To specify, which validator should be used for a certain field type supported by a given constraint annotation, an interface ValidatorDescriptor could be introduced:
Code:
public interface ValidatorDescriptor<A extends Annotation, T, V extends Constraint<A, T>> {
Class<T> getType();
Class<V> getValidatorClass();
}
Following this approach, the ConstraintValidator meta annotation would refer to an array of such validator descriptors:
Code:
public @interface ConstraintValidator {
public abstract Class<? extends ValidatorDescriptor<?, ?, ?>>[] value();
}
Beside a safer implementation of validators, this approach would enable another use case: compile-time checking for misplaced constraint annotations by implementing an annotation processor, which could not only check for proper usage of the standard annotations, but also - by evaluating the validator descriptors - of any custom constraint annotations.
- The interface name "Constraint" appears irritating to me. I would regard Min, Max, Size etc. as "constraints" and classes that check for such constraints as "validators". Throughout the spec, that nomenclature is also used several times (so the JavaDoc of ConstraintFactory says: "This class manages the creation of constraint validators."). And in fact, the package, in which the constraint annotations live, is named javax.validation.constraints.
Therefore I suggest to rename the Constraint interface to "Validator" (the current Validator interface might be named "BeansValidator" or similar) or "ConstraintValidator" (the current ConstraintValidator interface could e.g. be named "ValidatedBy" then).
- The class comments of the Past and Future annotations say that the JVM's current time will be used for validity checking. This could be an issue because "now" is not in every case defined as the JVM's current time.
In my code for instance, rather than calling new Date() or similar, I prefer the usage of a simple time service: Date myDate = DateService.getDate(). This service might return new Date() normally, but for instance yesterday's date in case a failed batch run shall be re-executed. Using a date set by some IoC container as "now", might be another scenario.
Either way, validation of a Date field might fail, if the constraint validator for Past and Future strictly relies on JVM time. So I think, a way to let that validator know, what "now" means, is required. I'm not sure though, how to do that. One option might be to allow the usage of an expression language (unified EL?) to specify the comparison value:
Code:
...
private Date now;
@Future(currentDate="${now}")
private Date futureDate;
...
Being able to use an EL to specify comparison values would also be useful for other constraints such as Min, Max or Size. That way, some scenarios, for which now a custom class level constraint is required, could be realized leveraging the standard annotations (e.g. one number field required to be larger than another one). Alternatively, to provide a simple solution just for the Past/Future problem, some setter for the current time at the Validator API might be considered.
- Could the Size constraint also be allowed for attributes of type Map? The class comment of Size mentions String, Collection and Array as allowed types, leaving Map out, as it doesn't inherit from Collection.
- At the Min/Max constraints the comment says, that you are not sure wether to allow them at byte variables or not. What would be the rationale for excluding byte-typed fields?
Thanks for any answers in advance. Keep up the good work and with kind regards,
Gunnar
Login
FAQ
Search
