On 11 déc. 2012, at 14:43, Gunnar Morling <gunnar@hibernate.org> wrote:

> @DecimalMax(value = "0.0", exclusive = true)

Can we name it "inclusive" rather than "exclusive"? Personally I prefer positive variable/method names as I think they read better, in particular when it comes to negation.

I like inclusive as well

> When dealing with decimal number such a flag can indeed simplify things,
> however it creates a problem when dealing with error messages.

I think selecting the right message within the validator impl is sufficient. The spec could just describe which text is to be chosen in which case. Being able to make use of more advanced formatting logic (conditionals etc.) in messages is something which we should discuss separately IMO.

Yes and no. Should we postpone this issue until the formatting logic is powerful enough? Or should we go for a solution we will want to deprecate down the road?

What I wanted to say is that we don't necessarily need the EL feature in order to support the @DecimalMax use case. I think a description in the spec should do the trick, but if we manage to get the EL feature completed, it would of course make sense to use it here, too.

In case we go for a non-EL based solution in 1.1 and come up with EL support in a later revision I think it should be possible to adapt @DecimalMax without effecting clients.

A possible alternative would be to make use of constraint composition. E.g. @DecimalMin could be a composed constraint like this:

@Target({ METHOD, FIELD, ANNOTATION_TYPE, CONSTRUCTOR, PARAMETER })
@Retention(RUNTIME)
@Documented
@DecimalMinExclusive("")
@DecimalMinInclusive("")
@Constraint(validatedBy = { })
public @interface DecimalMin {

String message() default "";
Class<?>[] groups() default { };
Class<? extends Payload>[] payload() default { };

@OverridesAttribute.List( {
@OverridesAttribute(constraint=DecimalMinInclusive.class, name="value"),
@OverridesAttribute(constraint=DecimalMinExclusive.class, name="value")
} )
String value();

@OverridesAttribute.List( {
@OverridesAttribute(constraint=DecimalMinInclusive.class, name="inclusive"),
@OverridesAttribute(constraint=DecimalMinExclusive.class, name="inclusive")
} )
boolean inclusive() default true;
}

Depending on the value of the "inclusive" attribute, only one of the validators for @DecimalMinExclusive and @DecimalMinInclusive would validate the given value and potentially raise an error. By decomposing the validation into two distinct constraints we could make use of plain error messages:

javax.validation.constraints.DecimalMinInclusive.message = must be greater than or equal to {value}
javax.validation.constraints.DecimalMinExclusive.message = must be greater than {value}

The main issue with that approach is that DecimalMinInclusive etc. would be part of the API, although they ideally shouldn't be directly used by BV end users.

I don't like this approach very much. From a user pov, it's as annoying as the double message and pollute the API with annotations they should never use.

> I also like the approach taken in OVal. There you can create/change message
> variables by overriding 'Map<String, String> createMessageVariables()'. Something

> like this could for example be added to ConstraintValidatorContext.

A while ago I created BVAL-339 [2] which I think goes into this direction. It suggests to add a method setValue() to ConstraintValidatorContext, allowing to set arbitrary message attributes:

context
.buildConstraintViolationWithTemplate( "Must be after {now}" )
.setValue( "now", now )
.addConstraintViolation();

Independent from the originally discussed issue, I think adding this would makes sense in any case. If no one objects, I can create pull requests for this.

I see this as less useful than the formatting improvements reusing the EL engine. So if I had to prioritize I'd put this one after.
Do you have use cases by the way? The example with now is not really convincing :)

I think this would be helpful where validation is based on some sort of context which is not expressed in the constraint annotation attributes:

* "Password must be at least {min} characters long for users in role {role}" (role is retrieved from the current user)

* "Order number invalid. Reason: {reason}" (reason is one of several possible ones checked by the validator)

One could work around this by creating the message completely by yourself, but I think it would be nice to leave retrieval of the template etc. to BV and just put custom variables in. In combination with the EL feature, you could also put in variables evaluated in the EL expression.

_______________________________________________
beanvalidation-dev mailing list
beanvalidation-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/beanvalidation-dev