Skip to content

Fix various issues with datetime arithmetics #534

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: refactor-datetime-arithmetics
Choose a base branch
from
Open

Fix various issues with datetime arithmetics #534

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
83 changes: 32 additions & 51 deletions core/common/src/Instant.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -450,27 +450,10 @@ public fun String.toInstant(): Instant = Instant.parse(this)
public fun Instant.plus(period: DateTimePeriod, timeZone: TimeZone): Instant = try {
with(period) {
val initialOffset = offsetIn(timeZone)
val initialLdt = toLocalDateTimeFailing(initialOffset)
val instantAfterMonths: Instant
val offsetAfterMonths: UtcOffset
val ldtAfterMonths: LocalDateTime
if (totalMonths != 0L) {
val unresolvedLdtWithMonths = initialLdt.plus(totalMonths, DateTimeUnit.MONTH)
instantAfterMonths = localDateTimeToInstant(unresolvedLdtWithMonths, timeZone, preferred = initialOffset)
offsetAfterMonths = instantAfterMonths.offsetIn(timeZone)
ldtAfterMonths = instantAfterMonths.toLocalDateTime(offsetAfterMonths)
} else {
instantAfterMonths = this@plus
offsetAfterMonths = initialOffset
ldtAfterMonths = initialLdt
}
val instantAfterMonthsAndDays = if (days != 0) {
val unresolvedLdtWithDays = ldtAfterMonths.plus(days, DateTimeUnit.DAY)
localDateTimeToInstant(unresolvedLdtWithDays, timeZone, preferred = offsetAfterMonths)
} else {
instantAfterMonths
}
instantAfterMonthsAndDays
val ldtPlusDate = toLocalDateTimeFailing(initialOffset)
.run { if (totalMonths != 0L) { plus(totalMonths, DateTimeUnit.MONTH) } else { this } }
.run { if (days != 0) { this.plus(days, DateTimeUnit.DAY) } else { this } }
localDateTimeToInstant(ldtPlusDate, timeZone, preferred = initialOffset)
.run { if (totalNanoseconds != 0L) plus(0, totalNanoseconds).check(timeZone) else this }
}.check(timeZone)
} catch (e: ArithmeticException) {
Expand Down Expand Up @@ -522,21 +505,21 @@ public fun Instant.minus(period: DateTimePeriod, timeZone: TimeZone): Instant =
public fun Instant.periodUntil(other: Instant, timeZone: TimeZone): DateTimePeriod {
val initialOffset = offsetIn(timeZone)
val initialLdt = toLocalDateTimeFailing(initialOffset)
val otherLdt = other.toLocalDateTimeFailing(other.offsetIn(timeZone))

val months = initialLdt.until(otherLdt, DateTimeUnit.MONTH) // `until` on dates never fails
val unresolvedLdtWithMonths = initialLdt.plus(months, DateTimeUnit.MONTH)
// won't throw: thisLdt + months <= otherLdt, which is known to be valid
val instantWithMonths = localDateTimeToInstant(unresolvedLdtWithMonths, timeZone, preferred = initialOffset)
val offsetWithMonths = instantWithMonths.offsetIn(timeZone)
val ldtWithMonths = instantWithMonths.toLocalDateTime(offsetWithMonths)
val days = ldtWithMonths.until(otherLdt, DateTimeUnit.DAY) // `until` on dates never fails
val unresolvedLdtWithDays = ldtWithMonths.plus(days, DateTimeUnit.DAY)
val otherLdt = other.toLocalDateTimeFailing(timeZone)
val timeAfterAddingDate =
localDateTimeToInstant(otherLdt.date.atTime(initialLdt.time), timeZone, preferred = initialOffset)
val delta = when {
other > this && timeAfterAddingDate > other -> -1 // addition won't throw: end date - date >= 1
other < this && timeAfterAddingDate < other -> 1 // addition won't throw: date - end date >= 1
else -> 0
}
val endDate = otherLdt.date.plus(delta, DateTimeUnit.DAY) // `endDate` is guaranteed to be valid
val unresolvedLdtWithDays = endDate.atTime(initialLdt.time)
val newInstant = localDateTimeToInstant(unresolvedLdtWithDays, timeZone, preferred = initialOffset)
// won't throw: thisLdt + days <= otherLdt
val nanoseconds = newInstant.until(other, DateTimeUnit.NANOSECOND) // |otherLdt - thisLdt| < 24h

return buildDateTimePeriod(months, days.toInt(), nanoseconds)
val datePeriod = endDate - initialLdt.date
return buildDateTimePeriod(datePeriod.totalMonths, datePeriod.days, nanoseconds)
}

/**
Expand All @@ -555,8 +538,18 @@ public fun Instant.periodUntil(other: Instant, timeZone: TimeZone): DateTimePeri
*/
public fun Instant.until(other: Instant, unit: DateTimeUnit, timeZone: TimeZone): Long =
when (unit) {
is DateTimeUnit.DateBased ->
toLocalDateTimeFailing(offsetIn(timeZone)).until(other.toLocalDateTimeFailing(other.offsetIn(timeZone)), unit)
is DateTimeUnit.DateBased -> {
val start = toLocalDateTimeFailing(timeZone)
val end = other.toLocalDateTimeFailing(timeZone)
val timeAfterAddingDate =
localDateTimeToInstant(end.date.atTime(start.time), timeZone, preferred = this.offsetIn(timeZone))
val delta = when {
other > this && timeAfterAddingDate > other -> -1 // addition won't throw: end date - date >= 1
other < this && timeAfterAddingDate < other -> 1 // addition won't throw: date - end date >= 1
else -> 0
}
start.date.until(end.date.plus(delta, DateTimeUnit.DAY), unit)
}
is DateTimeUnit.TimeBased -> {
check(timeZone); other.check(timeZone)
until(other, unit)
Expand Down Expand Up @@ -890,11 +883,14 @@ private fun Instant.toLocalDateTimeFailing(offset: UtcOffset): LocalDateTime = t
throw DateTimeArithmeticException("Can not convert instant $this to LocalDateTime to perform computations", e)
}

private fun Instant.toLocalDateTimeFailing(timeZone: TimeZone): LocalDateTime =
toLocalDateTimeFailing(offsetIn(timeZone))

/** Check that [Instant] fits in [LocalDateTime].
* This is done on the results of computations for consistency with other platforms.
*/
private fun Instant.check(zone: TimeZone): Instant = this@check.also {
toLocalDateTimeFailing(offsetIn(zone))
toLocalDateTimeFailing(zone)
}

private fun LocalDateTime.plus(value: Long, unit: DateTimeUnit.DateBased) =
Expand All @@ -908,18 +904,3 @@ private fun LocalDateTime.plus(value: Int, unit: DateTimeUnit.DateBased) =
* @throws IllegalArgumentException if the boundaries of Instant are overflown
*/
internal expect fun Instant.plus(secondsToAdd: Long, nanosToAdd: Long): Instant

// org.threeten.bp.LocalDateTime#until
internal fun LocalDateTime.until(other: LocalDateTime, unit: DateTimeUnit.DateBased): Long {
val otherDate = other.date
val delta = when {
otherDate > date && other.time < time -> -1 // addition won't throw: endDate - date >= 1
otherDate < date && other.time > time -> 1 // addition won't throw: date - endDate >= 1
else -> 0
}
val endDate = otherDate.plus(delta, DateTimeUnit.DAY)
return when (unit) {
is DateTimeUnit.MonthBased -> date.until(endDate, DateTimeUnit.MONTH) / unit.months
is DateTimeUnit.DayBased -> date.until(endDate, DateTimeUnit.DAY) / unit.days
}
}
105 changes: 105 additions & 0 deletions core/common/test/InstantTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import kotlin.time.Duration.Companion.hours
import kotlin.time.Duration.Companion.milliseconds
import kotlin.time.Duration.Companion.nanoseconds
import kotlin.time.Duration.Companion.seconds
import kotlin.time.TimeSource

class InstantTest {

Expand Down Expand Up @@ -282,6 +283,104 @@ class InstantTest {
assertEquals(end, end2)
}

@Test
fun periodUntilSameSign() {
val tz = TimeZone.of("Europe/Berlin")
// Same sign when the period is positive but smaller than the non-DST-aware date-based period
assertPeriodSameSign(
LocalDateTime(2025, 3, 29, 2, 30).toInstant(tz).periodUntil(
LocalDateTime(2025, 3, 30, 3, 10).toInstant(tz), tz))
// Same sign when the period is negative but bigger than the non-DST-aware date-based period
assertPeriodSameSign(
Instant.parse("2025-07-27T00:59:00Z").periodUntil(
Instant.parse("2024-10-27T01:00:00Z"), tz))
}

@Test
@Ignore
fun periodUntilSameSignStressTest() {
val tz = TimeZone.of("Europe/Berlin")
val endMoment = TimeSource.Monotonic.markNow() + STRESS_TEST_DURATION
while (endMoment.elapsedNow().isNegative()) {
val start = Instant.fromEpochSeconds(Random.nextLong(1700000000, 1767222000))
val end = Instant.fromEpochSeconds(Random.nextLong(1700000000, 1767222000))
val period = start.periodUntil(end, tz)
assertPeriodSameSign(period)
}
}

@Test
fun untilDays() {
val tz = TimeZone.of("Europe/Berlin")
// No overshooting when the distance is positive but smaller than the non-DST-aware date-based distance
run {
val i1 = LocalDateTime(2025, 3, 29, 2, 30).toInstant(tz)
val i2 = LocalDateTime(2025, 3, 30, 3, 10).toInstant(tz)
val distance = i1.until(i2, DateTimeUnit.DAY, tz)
assertTrue(i2 > i1.plus(distance, DateTimeUnit.DAY, tz))
}
// No overshooting when the distance is negative but bigger than the non-DST-aware date-based distance
run {
val i1 = Instant.parse("2025-07-27T00:59:00Z")
val i2 = Instant.parse("2024-10-27T01:00:00Z")
val distance = i1.until(i2, DateTimeUnit.DAY, tz)
assertTrue(i2 < i1.plus(distance, DateTimeUnit.DAY, tz))
}
}

@Test
@Ignore
fun untilDaysStressTest() {
val tz = TimeZone.of("Europe/Berlin")
val endMoment = TimeSource.Monotonic.markNow() + STRESS_TEST_DURATION
while (endMoment.elapsedNow().isNegative()) {
val start = Instant.fromEpochSeconds(Random.nextLong(1700000000, 1767222000))
val end = Instant.fromEpochSeconds(Random.nextLong(1700000000, 1767222000))
val period = start.until(end, DateTimeUnit.DAY, tz)
val afterAdding = start.plus(period, DateTimeUnit.DAY, tz)
if (afterAdding > end && start < end || afterAdding < end && start > end) {
error("start: $start (${start.toLocalDateTime(tz)}), end: $end (${end.toLocalDateTime(tz)}), period: $period, " +
"afterAdding: $afterAdding (${afterAdding.toLocalDateTime(tz)})")
}
}
}

@Test
fun dateTimePeriodWithGapBetweenMonthsAndDays() {
val zone = TimeZone.of("America/New_York")
// LocalDateTime(2019, 3, 10, 2, 0) is a gap.
// If months and days are not added atomically, the result will be adjusted.
val start = Instant.parse("2019-02-10T02:00:00-05:00")
val expectedEnd = Instant.parse("2019-03-11T02:00:00-04:00")
val end = start.plus(DateTimePeriod(months = 1, days = 1), zone)
// assertEquals(expectedEnd, end)
val period = start.periodUntil(end, zone)
assertEquals(end, start.plus(period, zone))
}

@Test
fun periodUntilWithGapBetweenMonthsAndDays() {
val start = Instant.parse("2024-01-30T01:10:00Z")
val end = Instant.parse("2025-04-01T01:10:00Z")
val tz = TimeZone.of("Europe/Berlin")
val period = start.periodUntil(end, tz)
assertEquals(DateTimePeriod(years = 1, months = 2, days = 2, hours = 1), period)
assertEquals(end, start.plus(period, tz), "start: $start, end: $end, period: $period")
}

@Test
@Ignore
fun periodUntilWithGapBetweenMonthsAndDaysStressTest() {
val tz = TimeZone.of("Europe/Berlin")
val endMoment = TimeSource.Monotonic.markNow() + STRESS_TEST_DURATION
while (endMoment.elapsedNow().isNegative()) {
val start = Instant.fromEpochSeconds(Random.nextLong(1700000000, 1767222000))
val end = Instant.fromEpochSeconds(Random.nextLong(1700000000, 1767222000))
val period = start.periodUntil(end, tz)
assertEquals(end, start.plus(period, tz), "start: $start, end: $end, period: $period")
}
}

@Test
fun diffInvariant() {
repeat(STRESS_TEST_ITERATIONS) {
Expand Down Expand Up @@ -445,6 +544,12 @@ class InstantTest {
assertFalse(Instant.MIN.isDistantFuture)
}

private fun DateTimePeriod.hasSameSign() =
totalMonths >= 0 && days >= 0 && totalNanoseconds >= 0 ||
totalMonths <= 0 && days <= 0 && totalNanoseconds <= 0
private fun assertPeriodSameSign(period: DateTimePeriod) {
assertTrue(period.hasSameSign(), "Period $period has different signs for months, days and nanoseconds")
}
}

class InstantRangeTest {
Expand Down
8 changes: 7 additions & 1 deletion core/common/test/assertions.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import kotlinx.datetime.DateTimeArithmeticException
import kotlinx.datetime.DateTimeFormatException
import kotlin.test.assertFailsWith
import kotlin.test.fail
import kotlin.time.Duration.Companion.minutes

@Suppress("INVISIBLE_REFERENCE", "INVISIBLE_MEMBER")
@kotlin.internal.InlineOnly
Expand Down Expand Up @@ -39,4 +40,9 @@ inline fun <T> assertIllegalArgument(message: String? = null, f: () -> T) {
/**
* The number of iterations to perform in nondeterministic tests.
*/
const val STRESS_TEST_ITERATIONS = 1000
const val STRESS_TEST_ITERATIONS = 1000

/**
* How long to spin nondeterministic tests before giving up.
*/
val STRESS_TEST_DURATION = 5.minutes