Hi,

2016-09-26 14:00 GMT+02:00 Michael Nascimento <misterm@gmail.com>:
On Mon, Sep 26, 2016 at 5:18 AM, Gunnar Morling <gunnar@hibernate.org> wrote:
> Great proposal! I like your scheme for capturing all types to be supported
> and how you describe the validation routine.

Nice!

> * Why is it that you use compareTo() over isAfter()/isBefore()? I reckon the
> latter are not as widely supported amongst JSR 310 types?

Unfortunately, it's not in any interface though. So we have to stick
to what the interfaces provide... :-)

Yes, we could say something about these methods explicitly, but using the Comparable contract definitely simplifies things.

> * What are the shortcomings meant by "2.1.1. Working around limitations in
> ConstraintValidator that prevent the above strategy"?

All those sections without content are still to be written. In this
case, according to the spec, there is no way to write a
PastTemporalAccessorComparableConstraintValidator extends
ConstraintValidator<Past, ? extends TemporalAccessor & Comparable<?>>
or something like it, which is the natural choice. Making it just
"extends ConstraintValidator<TemporalAccessor>" won't make it fail at
the right phase if @Past is applied to a non-Comparable
TemporalAccessor. I have a few ideas on how to address that, but maybe
now you can get thinking too :-)

Ok, got you.

That'd only be a problem though if you wanted to write a single validator implementation for all of them, right? I'm wondering whether one wouldn't use several more specific implementations anyways, given one needs to invoke a specific static now() method?
 
> * What do you have in mind with regards to "2.4. "Simple" TemporalAmount
> implementations support"?

These are TemporalAmount(s) with a single unit. For these, @Max and
@Min support should work for the single unit they support.

Ah, seeing now that Duration and Period implement these. I don't think I like @Max/@Min for these as it's not clear what the unit of the expected value is. New constraints may be more useful here:

    public void save(@MaxDuration(value=60, unit=ChronoUnit.SECONDS) Duration d) { ... }

> * We need a way to expose the ClockProvider to ConstraintValidator
> implementations (injection via CDI may be used, but BV must be usable
> without CDI, too). Adding a new method
> ConstraintValidatorContext#getClockProvider() seems the most obvious choice
> (currently the RI provides
> HibernateConstraintValidatorContext#getTimeProvider() for that purpose).

Sure, did I remove that? :-)

No, I think it was not yet part of the proposal :)

> * Regarding Duration/Period, how about handling this separately? It seems we
> could add this in a second step, allowing to align quickly on the
> @Future/@Past additions and add support for it to the RI.

Your call :-) I was just trying to consolidate everything about
JSR-310 in a single proposal; there's nothing wrong with splitting it.

Ok, I'd say if think you can come up with something for this quickly, go for it. Otherwise let's keep it separately. It'd be nice to have a first cut of stuff we can take and provide support for in the RI.

Thanks a lot for your effort, Michael, that's much appreciated!

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