5:58 p.m.
Hi and happy new year to everyone!
We discussed a couple of things today with Gunnar and we have a couple of
questions for you all regarding the improved java.time support we committed
before the holidays.
1. Support for partials
================
The initial proposal of Michael included the support for partials such as
YearMonth or MonthDay (ie types which do not strictly represent a point in
time).
While @Past and @Future make sense for YearMonth, it's a bit more difficult
to define them for MonthDay.
The implementation we committed indeed supports MonthDay by building a
MonthDay from the Clock provided by the ClockProvider and comparing them
using MonthDay.compare(...).
We tried to think of a use case but couldn't think of any.
Partials currently supported are:
- LocalTime
- MonthDay
- OffsetTime
- Year
- YearMonth
I think Year and YearMonth can be considered OK without further discussion.
LocalTime and OffsetTime, I can think of use cases. MonthDay, well, I
didn't find any potential use case and it looks just weird.
That being said, it does not cost us anything to have it for consistency.
So, any thoughts on this? Use cases, votes for removing it/keeping it?
2. Resolution
==========
In the comments of https://hibernate.atlassian.net/browse/HV-820, someone
suggested the idea of resolution.
You could have something like:
@Future(resolution = ChronoUnit.MONTHS)
private LocalDate paymentDate;
This would truncate the LocalDates to ChronoUnit.MONTHS before doing the
comparison, ensuring that the date should be at least next month.
It might also be useful to reduce the resolution of ZonedDateTimes to days
before doing the comparison, especially when using the orPresent option.
Note that it's only useful if you use a type more precise than what you
require for the validation so it might be a very narrow use case not worth
adding additional complexity to the standard constraints.
Do you see any use cases for that? Have you ever used custom constraints
for that?
3. orPresent
=========
The current implementation adds orPresent support as an option of the
preexisting @Past/@Future annotations:
@Past(orPresent = true) / @Future(orPresent = true)
Gunnar suggested this afternoon that it might be more readable to have
@PastOrPresent/@FutureOrPresent, especially if we have more than one
options for @Past/@Future.
Something like @Past(resolution = DAY, orPresent = true) would be less
readable than @PastOrPresent(resolution = DAY).
Personally, I prefer having one annotation with an option rather than 2
different annotations - I think it has a better semantic - but I can see
the rationale behind Gunnar's proposal.
Thoughts?
Thanks.
--
Guillaume
7:17 p.m.
Hi Guillaume,
Let me see if I can help:
On Thu, Jan 5, 2017 at 2:58 PM, Guillaume Smet <guillaume.smet(a)gmail.com>
wrote:
1. Support for partials
================
The initial proposal of Michael included the support for partials such as
YearMonth or MonthDay (ie types which do not strictly represent a point in
time).
While @Past and @Future make sense for YearMonth, it's a bit more
difficult to define them for MonthDay.
The implementation we committed indeed supports MonthDay by building a
MonthDay from the Clock provided by the ClockProvider and comparing them
using MonthDay.compare(...).
We tried to think of a use case but couldn't think of any.
When you are using a calendar year for fiscal purposes (in Brazil, its
January-December), you could use MonthDay to represent a point in time
separately from the year. If a MonthDay represents a holiday, there might
be other use cases related to the current year.
That being said, it does not cost us anything to have it for consistency.
Consistency for me is a good argument in itself. If we couldn't think of
the use case, maybe it's just because we're temporarily blind. Being
consistent doesn't hurt, especially given the effort is minimum :-)
2. Resolution
==========
In the comments of https://hibernate.atlassian.net/browse/HV-820, someone
suggested the idea of resolution.
You could have something like:
@Future(resolution = ChronoUnit.MONTHS)
private LocalDate paymentDate;
This would truncate the LocalDates to ChronoUnit.MONTHS before doing the
comparison, ensuring that the date should be at least next month.
While this is a valid use case, it seems a little bit specific to me in the
sense you have a reference for "now" as a comparison (a use case I
mentioned before is validating a temporal object against an arbitrary
reference) and in this case converting it. This also limits the
"truncation" (which is just a specific case of conversion) to ChronoUnit,
when other types of conversion might be needed.
This can be handled by doing:
@Future
private YearMonth paymentYearMonth() {
return YearMonth.from(paymentDate);
}
Which is trivial.
A more interesting use case that remains unsupported, of which the previous
is kind of a specialization is: I have a startDate and an endDate and there
is no simple way to express that endDate must be greater or equal than
startDate. That would apply to any Comparable, by the way :-( If we are to
pursue something more advanced, that's the use case I'd suggest going after.
Note that it's only useful if you use a type more precise than
what you
require for the validation so it might be a very narrow use case not worth
adding additional complexity to the standard constraints.
That's my opinion, too narrow. A broader one is relative comparable
validation.
3. orPresent
=========
The current implementation adds orPresent support as an option of the
preexisting @Past/@Future annotations:
@Past(orPresent = true) / @Future(orPresent = true)
Gunnar suggested this afternoon that it might be more readable to have
@PastOrPresent/@FutureOrPresent, especially if we have more than one
options for @Past/@Future.
Something like @Past(resolution = DAY, orPresent = true) would be less
readable than @PastOrPresent(resolution = DAY).
Personally, I prefer having one annotation with an option rather than 2
different annotations - I think it has a better semantic - but I can see
the rationale behind Gunnar's proposal.
I vote for one annotation.
PS: sorry for my long absence. My work load got too intense these days, I
hope it will cool off soon...
Regards,
Michael
12:43 p.m.
Hey all,
see my thoughts inline..
1. Support for partials
> ================
>
[....]
>
> We tried to think of a use case but couldn't think of any.
>
When you are using a calendar year for fiscal purposes (in Brazil, its
January-December), you could use MonthDay to represent a point in time
separately from the year. If a MonthDay represents a holiday, there might
be other use cases related to the current year.
>
> That being said, it does not cost us anything to have it for consistency.
>
Consistency for me is a good argument in itself. If we couldn't think of
the use case, maybe it's just because we're temporarily blind. Being
consistent doesn't hurt, especially given the effort is minimum :-)
I agree with Michael. I could imagine use cases especially in the context
of financials (just think of a fiscal year for example). So I would prefer
to be consistent and just support it.
2. Resolution
> ==========
> [...]
>
> This would truncate the LocalDates to ChronoUnit.MONTHS before doing the
> comparison, ensuring that the date should be at least next month.
>
>
While this is a valid use case, it seems a little bit specific to me in
the sense you have a reference for "now" as a comparison (a use case I
mentioned before is validating a temporal object against an arbitrary
reference) and in this case converting it. This also limits the
"truncation" (which is just a specific case of conversion) to ChronoUnit,
when other types of conversion might be needed.
I don't think this is a very common use case. So I currently don't see a
good reason to support it. But may be I'm missing a common use case here?
> 3. orPresent
> =========
> [...]
>
> Personally, I prefer having one annotation with an option rather than 2
> different annotations - I think it has a better semantic - but I can see
> the rationale behind Gunnar's proposal.
>
I vote for one annotation.
I would also prefer one annotation. I think even something
like @Past(resolution = DAY, orPresent = true) is quite good readable.
Christian
--
Christian Kaltepoth
Blog: http://blog.kaltepoth.de/
Twitter: http://twitter.com/chkal
GitHub: https://github.com/chkal
9:12 a.m.
Thanks a lot for your feedback!
On 1., I was initially a bit skeptical about @Future for partial types such
as LocalTime or MonthDay, which don't denote "one point in time" but a
recurring one. But after playing a bit with those types in the RI, having
support for them felt alright.
On 2., agreed. People can always put their constraint to a derived member
with the right resolution as pointed out by Michael, so I don't think it
warrants explicit support.
On 3., ok, works for me.
Based on this feedback and the original proposal [1]. I've sent a pull
request for updating the spec (https://github.com/
beanvalidation/beanvalidation-spec/pull/122). Note that you need to take a
look at the API project, too, in order to get the full picture (as we
include API types directly from there).
Any comments on this change are welcome. I'll leave the PR open until the
end of the week end then merge it if nothing more comes up.
This change does not include support for range data types, I've opened
BVAL-553 [2] for that one. @Michael, maybe you are up for writing a
proposal for that one, too?
Thanks,
--Gunnar
[1] http://beanvalidation.org/proposals/BVAL-496/
[2] https://hibernate.atlassian.net/browse/BVAL-553
2017-01-08 12:43 GMT+01:00 Christian Kaltepoth <christian(a)kaltepoth.de>:
Hey all,
see my thoughts inline..
1. Support for partials
>> ================
>>
>
> [....]
>>
>> We tried to think of a use case but couldn't think of any.
>>
>
> When you are using a calendar year for fiscal purposes (in Brazil, its
> January-December), you could use MonthDay to represent a point in time
> separately from the year. If a MonthDay represents a holiday, there might
> be other use cases related to the current year.
>
>>
>> That being said, it does not cost us anything to have it for consistency.
>>
>
> Consistency for me is a good argument in itself. If we couldn't think of
> the use case, maybe it's just because we're temporarily blind. Being
> consistent doesn't hurt, especially given the effort is minimum :-)
>
I agree with Michael. I could imagine use cases especially in the context
of financials (just think of a fiscal year for example). So I would prefer
to be consistent and just support it.
> 2. Resolution
>> ==========
>> [...]
>>
>> This would truncate the LocalDates to ChronoUnit.MONTHS before doing the
>> comparison, ensuring that the date should be at least next month.
>>
>>
> While this is a valid use case, it seems a little bit specific to me in
> the sense you have a reference for "now" as a comparison (a use case I
> mentioned before is validating a temporal object against an arbitrary
> reference) and in this case converting it. This also limits the
> "truncation" (which is just a specific case of conversion) to ChronoUnit,
> when other types of conversion might be needed.
>
I don't think this is a very common use case. So I currently don't see a
good reason to support it. But may be I'm missing a common use case here?
>
>
>> 3. orPresent
>> =========
>> [...]
>>
>> Personally, I prefer having one annotation with an option rather than 2
>> different annotations - I think it has a better semantic - but I can see
>> the rationale behind Gunnar's proposal.
>>
>
> I vote for one annotation.
>
I would also prefer one annotation. I think even something
like @Past(resolution = DAY, orPresent = true) is quite good readable.
Christian
--
Christian Kaltepoth
Blog: http://blog.kaltepoth.de/
Twitter: http://twitter.com/chkal
GitHub: https://github.com/chkal
_______________________________________________
beanvalidation-dev mailing list
beanvalidation-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/beanvalidation-dev
11:46 a.m.
Hi,
On Tue, Jan 10, 2017 at 9:12 AM, Gunnar Morling <gunnar(a)hibernate.org>
wrote:
Based on this feedback and the original proposal [1]. I've sent a
pull
request for updating the spec (https://github.com/beanvalida
tion/beanvalidation-spec/pull/122). Note that you need to take a look at
the API project, too, in order to get the full picture (as we include API
types directly from there).
Any comments on this change are welcome. I'll leave the PR open until the
end of the week end then merge it if nothing more comes up.
I merged this PR by mistake just now but feel free to add comment on it,
I'll ajust what needs to be adjusted.
Thanks!
12:46 p.m.
Just closed this PR as there was no further feedback. Thanks all who
worked on this change, glad to see it done!
2017-01-10 11:46 GMT+01:00 Guillaume Smet <guillaume.smet(a)gmail.com>:
Hi,
On Tue, Jan 10, 2017 at 9:12 AM, Gunnar Morling <gunnar(a)hibernate.org>
wrote:
>
> Based on this feedback and the original proposal [1]. I've sent a pull
> request for updating the spec
> (https://github.com/beanvalidation/beanvalidation-spec/pull/122). Note that
> you need to take a look at the API project, too, in order to get the full
> picture (as we include API types directly from there).
>
> Any comments on this change are welcome. I'll leave the PR open until the
> end of the week end then merge it if nothing more comes up.
I merged this PR by mistake just now but feel free to add comment on it,
I'll ajust what needs to be adjusted.
Thanks!
_______________________________________________
beanvalidation-dev mailing list
beanvalidation-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/beanvalidation-dev
9:38 a.m.
Hi Michael,
A more interesting use case that remains unsupported, of which the
previous is kind
of a specialization is: I have a startDate and an endDate and there is no simple way to
express that endDate must be greater or equal than startDate. That would apply to
any Comparable, by the way
It's an interesting request, but I'm not sure whether it pulls its
weight. We'd probably end up with something looking like that:
@Compare(
member1 = "dateOfEntry",
comparison = ComparisonType.LESS_THAN,
member2 = "dateOfCancelation"
)
public class Customer {
public LocalDateTime dateOfEntry;
public LocalDateTime dateOfCancelation;
}
I find it doesn't read overly well. It feels a bit too much like
"programming in annotations" to me. It also raises the question how to
retrieve the values for member1 and member2, getter or field?
But there are simple ways for implementing your requirement. Using
@AssertTrue, you can do it by writing actual, safe Java code:
public class Customer {
public LocalDateTime dateOfEntry;
public LocalDateTime dateOfCancelation;
@AssertTrue
public boolean isEntryAndCancelationDatesConsistent() {
return dateOfEntry.compareTo( dateOfCancelation ) < 0;
}
}
An alternative is the RI's @ScriptAssert. It even discusses your very
example in the docs:
@ScriptAssert(lang = "jexl", script = "_.startDate <
_.endDate",
alias = "_")
public class CalendarEvent {
private Date startDate;
private Date endDate;
//...
}
Would either approach help to address your requirements?
--Gunnar
2650
days inactive
2661
days old
beanvalidation-dev@lists.jboss.org
6 comments
4 participants
participants (4)
-
Christian Kaltepoth -
Guillaume Smet -
Gunnar Morling -
Michael Nascimento