-
-
Notifications
You must be signed in to change notification settings - Fork 429
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Relative functions for DateTimeType #4276
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Gaël L'hopital <gael@lhopital.org>
This reverts commit cdfceb1.
…sameDay) to DateTimeType Signed-off-by: Gaël L'hopital <gael@lhopital.org>
Given that some of the languages do not expose and have you work with the raw Java DateTimeType Objects adding these will not make them available to all rules languages. Some of the stuff like this though has been added to JS Scripting already so I think this PR could be more ambitious if youi wanted to be. There you will find:
So you can do stuff like this:
Note that The real magic (IMO) with the JS Scripting approach is that in many of these conversion and comparison functions you can use anything that can be converted by
In addition to In JS you can monkeypatch these into the ZonedDateTime class but I'm pretty sure that's not an option on the Java side of things. But a series of utility functions on the DateTimeType to not only compare but also convert "stuff" to ZonedDateTimes. There is also a lot of convenience stuff built into jRuby which might prove some inspiration as well. Once I implemented a lot of the above (originally as part of OHRT and then I was able to get most of them accepted into the openhab-js library) all the complexities of working with DateTimes kind of just disappeared. It's just an idea and may be more than you're willing to take on, but I can say this has greatly simplified JS Scripting rules. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is a handy addition, especially for rulesdsl. There is currently not a direct equivalent in jruby for what this adds, and when this is added, it will be automatically usable from within jruby without any modifications to the jruby helper library. However it's currently already quite easy to do this in jruby
# dt1 and dt2 are DateTimeType objects, e.g. A_DateTime_Item.state
# Checking for same day
dt1.to_date == dt2.to_date # to_date creates a Date object, stripping off the time data
# to check for today
dt1.to_date == Date.today
# to check for tomorrow
dt1.to_date == Date.today + 1
# to check for yesterday
dt1.to_date == Date.today - 1
After this PR, JRuby will be able to call:
dt1.today? #jruby automatically translates isToday to ruby method naming convention `today?`
dt1.yesterday?
dt1.tomorrow?
dt1.same_day(dt2) # The Ruby way would be dt1 === dt2
However, in jruby we can add these to ZonedDateTime directly, thus making it universally usable in many more places, including DateTimeType, because in jruby a DateTimeType delegates its methods to its ZonedDateTime value.
@ccutrer wdyt?
assertTrue(new DateTimeType().isToday()); | ||
|
||
DateTimeType now = new DateTimeType(); | ||
DateTimeType tomorrow = new DateTimeType(now.getZonedDateTime().plusHours(24)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DateTimeType tomorrow = new DateTimeType(now.getZonedDateTime().plusHours(24)); | |
DateTimeType tomorrow = new DateTimeType(now.getZonedDateTime().plusDays(1)); |
I know this is an edge case, but the length of the day isn't always precisely 24h.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wanted to avoid using the same method than what's used in the object itself.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed
|
||
DateTimeType now = new DateTimeType(); | ||
DateTimeType tomorrow = new DateTimeType(now.getZonedDateTime().plusHours(24)); | ||
DateTimeType yesterday = new DateTimeType(now.getZonedDateTime().minusHours(24)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DateTimeType yesterday = new DateTimeType(now.getZonedDateTime().minusHours(24)); | |
DateTimeType yesterday = new DateTimeType(now.getZonedDateTime().minusdays(1)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed
|
||
public boolean sameDay(ZonedDateTime other) { | ||
return zonedDateTime.truncatedTo(ChronoUnit.DAYS) | ||
.isEqual(other.withZoneSameLocal(zonedDateTime.getZone()).truncatedTo(ChronoUnit.DAYS)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this use witZoneSameInstant
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed
@Test | ||
public void relativeTest() { | ||
DateTimeType dt1 = new DateTimeType("2019-06-12T17:30:00Z"); | ||
DateTimeType dt2 = new DateTimeType("2019-06-12T00:00:00+0000"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To ensure that this checks for the same instant:
dt1 = "2019-06-13T01:10:00+02"
dt2 = "2019-06-12T23:00:00Z"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed
Signed-off-by: Gaël L'hopital <gael@lhopital.org>
Signed-off-by: Gaël L'hopital <gael@lhopital.org>
return zonedDateTime.truncatedTo(ChronoUnit.DAYS) | ||
.isEqual(other.withZoneSameInstant(zonedDateTime.getZone()).truncatedTo(ChronoUnit.DAYS)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return zonedDateTime.truncatedTo(ChronoUnit.DAYS) | |
.isEqual(other.withZoneSameInstant(zonedDateTime.getZone()).truncatedTo(ChronoUnit.DAYS)); | |
return zonedDateTime.truncatedTo(ChronoUnit.DAYS).isEqual(other.truncatedTo(ChronoUnit.DAYS)); |
isEqual already compares the instant so it's not necessary to equalise the timezone prior to comparison.
jshell> var d1 = java.time.ZonedDateTime.parse("2024-01-01T00:00:00Z")
d1 ==> 2024-01-01T00:00Z
jshell> var d2 = java.time.ZonedDateTime.parse("2024-01-01T10:00:00+10:00")
d2 ==> 2024-01-01T10:00+10:00
jshell> d1.isEqual(d2)
$17 ==> true
jshell> d1.getZone()
$18 ==> Z
jshell> d2.getZone()
$19 ==> +10:00
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With the truncation to ChronoUnit.DAYS, the comparison is false if they are not moved in the same time zone.
public boolean isAfter(DateTimeType other) { | ||
return zonedDateTime.isAfter(other.zonedDateTime); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We might also need the versions of isBefore and isAfter that accept a ZonedDateTime
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These already exists, reason why I thought it wasn't needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Basically it's for convenience, so you can do DateTimeItem.state.isBefore(zdt) instead of DateTimeItem.state.getZonedDateTime().isBefore(zdt), and also for uniformity since you are adding the ability to do
DateTimeItem.state.isBefore(anotherDateTimeItem), but, I'll leave the decision whether to have it to the maintainers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added
public boolean isBeforeDate(ZonedDateTime other) { | ||
return zonedDateTime.truncatedTo(ChronoUnit.DAYS).isBefore(other.truncatedTo(ChronoUnit.DAYS)); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am against the introduction of isBeforeDate, isBeforeTime, etc. because their meaning is ambiguous depending on the time zones of the two objects involved.
Take two zdt objects containing the same date but within two different time zones. They would fail the given comparison.
jshell> var d1 = java.time.ZonedDateTime.parse("2024-01-01T00:00:00-11:00")
d1 ==> 2024-01-01T00:00-11:00
jshell> var d2 = java.time.ZonedDateTime.parse("2024-01-01T10:00:00+11:00")
d2 ==> 2024-01-01T10:00+11:00
jshell> d2.withZoneSameInstant(d1.getZone())
$22 ==> 2023-12-31T12:00-11:00
jshell> d1.truncatedTo(java.time.temporal.ChronoUnit.DAYS).isBefore(d2.truncatedTo(java.time.temporal.ChronoUnit.DAYS))
$23 ==> false
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So do you recommend I should align Timezones before comparison, rename the methods or drop them ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would prefer to drop them, but that's just an opinion. Again, I defer to the maintainers to decide.
return new DateTimeType(zonedDateTime.withYear(now.getYear()).withMonth(now.getMonthValue()) | ||
.withDayOfMonth(now.getDayOfMonth())); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This also has the same ambiguity issue related to differences in time zones between what's in zonedDateTime
vs the system time zone.
jshell> var d1 = java.time.ZonedDateTime.parse("2024-01-01T00:00:00+11:00")
d1 ==> 2024-01-01T00:00+11:00
jshell> var now = java.time.ZonedDateTime.parse("2025-05-01T00:00:00-11:00")
now ==> 2025-05-01T00:00-11:00
jshell> d1.withYear(now.getYear()).withMonth(now.getMonthValue())
...> .withDayOfMonth(now.getDayOfMonth()).withZoneSameInstant(now.getZone())
$38 ==> 2025-04-30T02:00-11:00
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Made it relative to zonedDateTime
zone's.
This PR will be obsoleted by #3583 if accepted. |
As from one scripting language to the other, dealing with Dates is not always so easy, I propose to add these small functions that I use very often while scripting.
Here are the methods added to
DateTimeType
:isBefore
/isAfter
anotherDateTimeType
sameDay
compared to anotherDateTimeType
or aZonedDateTime
isToday
/isTomorrow
/isYesterday
toToday
,toTomorrow
,toYesterday
changes day/month/year accordingly, preserving time part of theDateTimeType
isBeforeDate
/isAfterDate
compared to anotherDateTimeType
or aZonedDateTime
ignoring the time componentisBeforeTime
/isAfterTime
compared to anotherDateTimeType
or aZonedDateTime
ignoring the date component