Work at SourceForge, help us to make it a better place! We have an immediate need for a Support Technician in our San Francisco or Denver office.

Close

#272 setRequired for Range, Min and not nullable

Not planned
open
nobody
None
1
2013-10-03
2012-11-13
Florian Hof
No

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);
}

Discussion

  • Javier Paniza
    Javier Paniza
    2012-11-13

    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
    Florian Hof
    2012-11-13

    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
    Javier Paniza
    2012-11-14

    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.

     
  • Javier Paniza
    Javier Paniza
    2012-11-14

    • priority: 5 --> 6
     
  • Javier Paniza
    Javier Paniza
    2013-10-02

    • Group: --> v4.9
    • Priority: 6 --> 1
     
  • Javier Paniza
    Javier Paniza
    2013-10-03

    • Group: v4.9 --> Not planned