#272 setRequired for Range, Min and not nullable

[Florian Hof]

OpenXava considers as required a property with a minimal value greater than zero. For me (and for the Hibernate implementation at http://grepcode.com/file/repo1.maven.org/maven2/org.hibernate/hibernate-validator/3.1.0.GA/org/hibernate/validator/RangeValidator.java#RangeValidator\), a minimal value or a range can in all case be optional. In my case I have optional swiss coordinates, which are (if defined) always greater than 0. If I want a property to be mandatory, I either specify it with nullable=false on @Column or with OpenXava's @Required.

OpenXava determines if a property is required in AnnotatedClassParser.processAnnotations. The case @Column(nullable=false) is not covered, that would be nice to detect. The @Min from javax, @Min from Hibernate and @Range from Hibernate should IMHO never be considered as required. @Length > 0 can of course be considered as required.

Instead of:
// required
if (element.isAnnotationPresent(Required.class)) {
property.setRequired(true);
}
else if (element.isAnnotationPresent(javax.validation.constraints.Min.class)) {
javax.validation.constraints.Min min = element.getAnnotation(javax.validation.constraints.Min.class);
if (min.value() > 0) property.setRequired(true);
}
else if (element.isAnnotationPresent(org.hibernate.validator.Min.class)) {
org.hibernate.validator.Min min = element.getAnnotation(org.hibernate.validator.Min.class);
if (min.value() > 0) property.setRequired(true);
}
else if (element.isAnnotationPresent(org.hibernate.validator.Range.class)) {
org.hibernate.validator.Range range = element.getAnnotation(org.hibernate.validator.Range.class);
if (range.min() > 0) property.setRequired(true);
}
else if (element.isAnnotationPresent(org.hibernate.validator.Length.class)) {
org.hibernate.validator.Length length = element.getAnnotation(org.hibernate.validator.Length.class);
if (length.min() > 0) property.setRequired(true);
}

I would propose:
// required
if (element.isAnnotationPresent(Required.class)) {
property.setRequired(true);
}
else if (element.isAnnotationPresent(javax.persistence.Column.class)) {
javax.persistence.Column column = element.getAnnotation(javax.persistence.Column.class);
if (!column.nullable()) property.setRequired(true);
}
else if (element.isAnnotationPresent(org.hibernate.validator.Length.class)) {
org.hibernate.validator.Length length = element.getAnnotation(org.hibernate.validator.Length.class);
if (length.min() > 0) property.setRequired(true);
}

[Javier Paniza]

Hi Florian,

> The @Min from javax, @Min from Hibernate and @Range from Hibernate should IMHO never be considered as required
So, you have a case like this:
@Min(18)
int age
where if specified the age must be at least 18, but it's not mandatary to specify the age. Have you this case in your application?

> @Column(nullable=false) is not covered, that would be nice to detect
Uhhm! But null is not the same as empty string, and if you can put a entry String the value is not mandatory for an end user perspective.

[Florian Hof]

Hi Javier,
Thanks for your quick response.

I've a case like:
@Range(min=18, max=65) // -> org.hibernate.validator.Range
int age
and OpenXava displays the small image for required fields. I cannot save the entity if this age is empty.

You are right, an empty string and null is not the same ...
But the users probably don't see the difference (and cannot with a single string field). On the opposite side, some databases like Oracle treats empty strings as null. So, apart in the business layer, the difference between nulls and empty strings is tiny.

[Javier Paniza]

Hi Florian,

> I've a case like:
> @Range(min=18, max=65) // -> org.hibernate.validator.Range
> int age
> and OpenXava displays the small image for required fields. I cannot save
> the entity if this age is empty.
OK. You're right. You're case is legitime and is not well supported currently, so we'll follow your advice and we'll remove the @Min, @Max and @Range as synonymous of @Required. It can produces some backward incompatibilities, but I think it's better as you say.
Moreover, it would be in keeping with this comment from Bean Validation specification:
"While not mandatory, it is considered a good practice to split the core constraint validation from the not
null constraint validation (for example, an @Email constraint will return true on a null object, i.e. will not
take care of the @NotNull validation) can have multiple meanings but is commonly used to express that a value does not make sense, is not
available or is simply unknown. Those constraints on the value are orthogonal in most cases to other con-
straints. For example a String, if present, must be an email but can be null. Separating both concerns is a
good practice."
We can apply the same reasoning to @Required.