[bv-dev] BVAL-192 'exclusive' flag for @DecimalMin/@DecimalMax

Gunnar Morling gunnar at hibernate.org
Wed Dec 12 15:20:39 EST 2012


2012/12/12 Emmanuel Bernard <emmanuel at hibernate.org>

>
>
> On 11 déc. 2012, at 14:43, Gunnar Morling <gunnar at 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 at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/beanvalidation-dev
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.jboss.org/pipermail/beanvalidation-dev/attachments/20121212/1362ebb1/attachment.html 


More information about the beanvalidation-dev mailing list