Add LocalTime.[from/to]MillisecondOfDay (#213)

dkhalanskyjb · web-flow · commit 3dbe18809d88 · 2022-06-01T15:30:27.000+03:00
* The documentation for existing similar methods is improved.
* `ThreeTenBpLocalTimeTest` is no longer needed due to more
  comprenensive testing in common code.
* A constant introduced for the number of iterations in tests that
  repeatedly generate random data to test functionality.
diff --git a/core/common/src/LocalTime.kt b/core/common/src/LocalTime.kt
@@ -41,16 +41,42 @@ public expect class LocalTime : Comparable<LocalTime> {
         public fun parse(isoString: String): LocalTime
 
         /**
-         * Returns a LocalTime with the specified [secondOfDay]. The nanosecond field will be set to zero.
+         * Constructs a [LocalTime] that represents the specified number of seconds since the start of a calendar day.
+         * The fractional parts of the second will be zero.
          *
-         * @throws IllegalArgumentException if the boundaries of [secondOfDay] are exceeded.
+         * @throws IllegalArgumentException if [secondOfDay] is outside the `0 until 86400` range,
+         * with 86400 being the number of seconds in a calendar day.
+         *
+         * @see LocalTime.toSecondOfDay
+         * @see LocalTime.fromMillisecondOfDay
+         * @see LocalTime.fromNanosecondOfDay
          */
         public fun fromSecondOfDay(secondOfDay: Int): LocalTime
 
         /**
-         * Returns a LocalTime with the specified [nanosecondOfDay].
+         * Constructs a [LocalTime] that represents the specified number of milliseconds since the start of
+         * a calendar day.
+         * The sub-millisecond parts of the `LocalTime` will be zero.
+         *
+         * @throws IllegalArgumentException if [millisecondOfDay] is outside the `0 until 86400 * 1_000` range,
+         * with 86400 being the number of seconds in a calendar day.
+         *
+         * @see LocalTime.fromSecondOfDay
+         * @see LocalTime.toMillisecondOfDay
+         * @see LocalTime.fromNanosecondOfDay
+         */
+        public fun fromMillisecondOfDay(millisecondOfDay: Int): LocalTime
+
+        /**
+         * Constructs a [LocalTime] that represents the specified number of nanoseconds since the start of
+         * a calendar day.
          *
-         * @throws IllegalArgumentException if the boundaries of [nanosecondOfDay] are exceeded.
+         * @throws IllegalArgumentException if [nanosecondOfDay] is outside the `0 until 86400 * 1_000_000_000` range,
+         * with 86400 being the number of seconds in a calendar day.
+         *
+         * @see LocalTime.fromSecondOfDay
+         * @see LocalTime.fromMillisecondOfDay
+         * @see LocalTime.toNanosecondOfDay
          */
         public fun fromNanosecondOfDay(nanosecondOfDay: Long): LocalTime
 
@@ -80,10 +106,13 @@ public expect class LocalTime : Comparable<LocalTime> {
     /** Returns the nanosecond-of-second time component of this time value. */
     public val nanosecond: Int
 
-    /** Returns the time as a second of a day, from 0 to 24 * 60 * 60 - 1. */
+    /** Returns the time as a second of a day, in `0 until 24 * 60 * 60`. */
     public fun toSecondOfDay(): Int
 
-    /** Returns the time as a nanosecond of a day, from 0 to 24 * 60 * 60 * 1_000_000_000 - 1. */
+    /** Returns the time as a millisecond of a day, in `0 until 24 * 60 * 60 * 1_000`. */
+    public fun toMillisecondOfDay(): Int
+
+    /** Returns the time as a nanosecond of a day, in `0 until 24 * 60 * 60 * 1_000_000_000`. */
     public fun toNanosecondOfDay(): Long
 
     /**
diff --git a/core/common/src/math.kt b/core/common/src/math.kt
@@ -12,9 +12,27 @@ internal fun Long.clampToInt(): Int =
             else -> toInt()
         }
 
+internal const val SECONDS_PER_HOUR = 60 * 60
+
+internal const val SECONDS_PER_MINUTE = 60
+
+internal const val MINUTES_PER_HOUR = 60
+
+internal const val HOURS_PER_DAY = 24
+
+internal const val SECONDS_PER_DAY: Int = SECONDS_PER_HOUR * HOURS_PER_DAY
+
+internal const val NANOS_PER_ONE = 1_000_000_000
 internal const val NANOS_PER_MILLI = 1_000_000
 internal const val MILLIS_PER_ONE = 1_000
-internal const val NANOS_PER_ONE = 1_000_000_000
+
+internal const val NANOS_PER_DAY: Long = NANOS_PER_ONE * SECONDS_PER_DAY.toLong()
+
+internal const val NANOS_PER_MINUTE: Long = NANOS_PER_ONE * SECONDS_PER_MINUTE.toLong()
+
+internal const val NANOS_PER_HOUR = NANOS_PER_ONE * SECONDS_PER_HOUR.toLong()
+
+internal const val MILLIS_PER_DAY: Int = SECONDS_PER_DAY * MILLIS_PER_ONE
 
 internal expect fun safeMultiply(a: Long, b: Long): Long
 internal expect fun safeMultiply(a: Int, b: Int): Int
diff --git a/core/common/test/InstantTest.kt b/core/common/test/InstantTest.kt
@@ -263,7 +263,7 @@ class InstantTest {
 
     @Test
     fun diffInvariant() {
-        repeat(1000) {
+        repeat(STRESS_TEST_ITERATIONS) {
             val millis1 = Random.nextLong(2_000_000_000_000L)
             val millis2 = Random.nextLong(2_000_000_000_000L)
             val instant1 = Instant.fromEpochMilliseconds(millis1)
@@ -279,7 +279,7 @@ class InstantTest {
 
     @Test
     fun diffInvariantSameAsDate() {
-        repeat(1000) {
+        repeat(STRESS_TEST_ITERATIONS) {
             val millis1 = Random.nextLong(2_000_000_000_000L)
             val millis2 = Random.nextLong(2_000_000_000_000L)
             with(TimeZone.UTC) TZ@ {
diff --git a/core/common/test/LocalDateTest.kt b/core/common/test/LocalDateTest.kt
@@ -119,7 +119,7 @@ class LocalDateTest {
         assertEquals(origin, origin.plus(0, DateTimeUnit.DAY))
         assertEquals(origin, origin.plus(DatePeriod(days = 0)))
 
-        repeat(1000) {
+        repeat(STRESS_TEST_ITERATIONS) {
             val days1 = Random.nextInt(-3652..3652)
             val days2 = Random.nextInt(-3652..3652)
             val ldtBefore = origin + DatePeriod(days = days1)
diff --git a/core/common/test/LocalTimeTest.kt b/core/common/test/LocalTimeTest.kt
@@ -6,10 +6,12 @@
 package kotlinx.datetime.test
 
 import kotlinx.datetime.*
+import kotlin.math.*
+import kotlin.random.*
 import kotlin.test.*
 
 class LocalTimeTest {
-    
+
     @Test
     fun localTimeParsing() {
         fun checkParsedComponents(value: String, hour: Int, minute: Int, second: Int, nanosecond: Int) {
@@ -63,7 +65,8 @@ class LocalTimeTest {
             Pair(LocalTime(0, 0, 0, 9999), "00:00:00.000009999"),
             Pair(LocalTime(0, 0, 0, 999), "00:00:00.000000999"),
             Pair(LocalTime(0, 0, 0, 99), "00:00:00.000000099"),
-            Pair(LocalTime(0, 0, 0, 9), "00:00:00.000000009"))
+            Pair(LocalTime(0, 0, 0, 9), "00:00:00.000000009"),
+        )
         for ((time, str) in data) {
             assertEquals(str, time.toString())
             assertEquals(time, LocalTime.parse(str))
@@ -89,9 +92,18 @@ class LocalTimeTest {
             0L to LocalTime(0, 0),
             5000000001L to LocalTime(0, 0, 5, 1),
             44105123456789L to LocalTime(12, 15, 5, 123456789),
-            86399999999999L to LocalTime(23, 59, 59, 999999999),
-        )
-
+            NANOS_PER_DAY - 1 to LocalTime(23, 59, 59, 999999999),
+        ) + buildMap {
+            repeat(STRESS_TEST_ITERATIONS) {
+                val hour = Random.nextInt(24)
+                val minute = Random.nextInt(60)
+                val second = Random.nextInt(60)
+                val nanosecond = Random.nextInt(1_000_000_000)
+                val nanosecondOfDay =
+                    hour * NANOS_PER_HOUR + minute * NANOS_PER_MINUTE + second * NANOS_PER_ONE.toLong() + nanosecond
+                put(nanosecondOfDay, LocalTime(hour, minute, second, nanosecond))
+            }
+        }
         data.forEach { (nanosecondOfDay, localTime) ->
             assertEquals(nanosecondOfDay, localTime.toNanosecondOfDay())
             assertEquals(localTime, LocalTime.fromNanosecondOfDay(nanosecondOfDay))
@@ -101,30 +113,69 @@ class LocalTimeTest {
     @Test
     fun fromNanosecondOfDayInvalid() {
         assertFailsWith<IllegalArgumentException> { LocalTime.fromNanosecondOfDay(-1) }
-        assertFailsWith<IllegalArgumentException> { LocalTime.fromNanosecondOfDay(86400000000000L) }
-        assertFailsWith<IllegalArgumentException> { LocalTime.fromNanosecondOfDay(Long.MAX_VALUE) }
+        assertFailsWith<IllegalArgumentException> { LocalTime.fromNanosecondOfDay(NANOS_PER_DAY) }
+        repeat(STRESS_TEST_ITERATIONS) {
+            assertFailsWith<IllegalArgumentException> {
+                LocalTime.fromNanosecondOfDay(NANOS_PER_DAY + Random.nextLong().absoluteValue)
+            }
+        }
     }
 
     @Test
-    fun fromSecondOfDay() {
+    fun fromMillisecondOfDay() {
         val data = mapOf(
             0 to LocalTime(0, 0),
-            5 to LocalTime(0, 0, 5),
-            44105 to LocalTime(12, 15, 5),
-            86399 to LocalTime(23, 59, 59),
-        )
+            5001 to LocalTime(0, 0, 5, 1000000),
+            44105123 to LocalTime(12, 15, 5, 123000000),
+            MILLIS_PER_DAY - 1 to LocalTime(23, 59, 59, 999000000),
+        ) + buildMap {
+            repeat(STRESS_TEST_ITERATIONS) {
+                val hour = Random.nextInt(24)
+                val minute = Random.nextInt(60)
+                val second = Random.nextInt(60)
+                val millisecond = Random.nextInt(1000)
+                val millisecondOfDay =
+                    (hour * SECONDS_PER_HOUR + minute * SECONDS_PER_MINUTE + second) * MILLIS_PER_ONE +
+                        millisecond
+                put(millisecondOfDay, LocalTime(hour, minute, second, millisecond * NANOS_PER_MILLI))
+            }
+        }
+        data.forEach { (millisecondOfDay, localTime) ->
+            assertEquals(millisecondOfDay, localTime.toMillisecondOfDay())
+            assertEquals(localTime, LocalTime.fromMillisecondOfDay(millisecondOfDay))
+        }
+    }
 
-        data.forEach { (secondOfDay, localTime) ->
-            assertEquals(secondOfDay, localTime.toSecondOfDay())
-            assertEquals(localTime, LocalTime.fromSecondOfDay(secondOfDay))
+    @Test
+    fun fromMillisecondOfDayInvalid() {
+        assertFailsWith<IllegalArgumentException> { LocalTime.fromMillisecondOfDay(-1) }
+        assertFailsWith<IllegalArgumentException> { LocalTime.fromMillisecondOfDay(MILLIS_PER_DAY) }
+        repeat(STRESS_TEST_ITERATIONS) {
+            assertFailsWith<IllegalArgumentException> {
+                LocalTime.fromMillisecondOfDay(MILLIS_PER_DAY + Random.nextInt().absoluteValue)
+            }
+        }
+    }
+
+    @Test
+    fun fromSecondOfDay() {
+        var t = LocalTime(0, 0, 0, 0)
+        for (i in 0 until SECONDS_PER_DAY) {
+            assertEquals(i, t.toSecondOfDay())
+            assertEquals(t, LocalTime.fromSecondOfDay(t.toSecondOfDay()))
+            t = t.plusSeconds(1)
         }
     }
 
     @Test
     fun fromSecondOfDayInvalid() {
         assertFailsWith<IllegalArgumentException> { LocalTime.fromSecondOfDay(-1) }
-        assertFailsWith<IllegalArgumentException> { LocalTime.fromSecondOfDay(86400) }
-        assertFailsWith<IllegalArgumentException> { LocalTime.fromSecondOfDay(Int.MAX_VALUE) }
+        assertFailsWith<IllegalArgumentException> { LocalTime.fromSecondOfDay(SECONDS_PER_DAY) }
+        repeat(STRESS_TEST_ITERATIONS) {
+            assertFailsWith<IllegalArgumentException> {
+                LocalTime.fromSecondOfDay(SECONDS_PER_DAY + Random.nextInt().absoluteValue)
+            }
+        }
     }
 
     @Test
@@ -166,3 +217,18 @@ fun checkEquals(expected: LocalTime, actual: LocalTime) {
     assertEquals(expected.hashCode(), actual.hashCode())
     assertEquals(expected.toString(), actual.toString())
 }
+
+private fun LocalTime.plusSeconds(secondsToAdd: Long): LocalTime {
+    if (secondsToAdd == 0L) {
+        return this
+    }
+    val sofd: Int = hour * SECONDS_PER_HOUR + minute * SECONDS_PER_MINUTE + second
+    val newSofd: Int = ((secondsToAdd % SECONDS_PER_DAY).toInt() + sofd + SECONDS_PER_DAY) % SECONDS_PER_DAY
+    if (sofd == newSofd) {
+        return this
+    }
+    val newHour: Int = newSofd / SECONDS_PER_HOUR
+    val newMinute: Int = newSofd / SECONDS_PER_MINUTE % MINUTES_PER_HOUR
+    val newSecond: Int = newSofd % SECONDS_PER_MINUTE
+    return LocalTime(newHour, newMinute, newSecond, nanosecond)
+}
diff --git a/core/common/test/MultiplyAndDivideTest.kt b/core/common/test/MultiplyAndDivideTest.kt
@@ -1,9 +1,9 @@
 /*
- * Copyright 2019-2020 JetBrains s.r.o.
+ * Copyright 2019-2022 JetBrains s.r.o.
  * Use of this source code is governed by the Apache 2.0 License that can be found in the LICENSE.txt file.
  */
 
-package kotlinx.datetime.test.math
+package kotlinx.datetime.test
 import kotlin.random.*
 import kotlin.test.*
 import kotlinx.datetime.*
@@ -55,7 +55,7 @@ class MultiplyAndDivideTest {
 
     @Test
     fun randomProductFitsInLong() {
-        repeat(1000) {
+        repeat(STRESS_TEST_ITERATIONS) {
             val a = Random.nextInt().toLong()
             val b = Random.nextInt().toLong()
             val m = Random.nextInt(1, Int.MAX_VALUE).toLong()
diff --git a/core/common/test/assertions.kt b/core/common/test/assertions.kt
@@ -34,4 +34,9 @@ inline fun <T> assertIllegalArgument(message: String? = null, f: () -> T) {
         val result = f()
         fail(result.toString())
     }
-}
+}
+
+/**
+ * The number of iterations to perform in nondeterministic tests.
+ */
+const val STRESS_TEST_ITERATIONS = 1000
diff --git a/core/darwin/test/ConvertersTest.kt b/core/darwin/test/ConvertersTest.kt
@@ -31,7 +31,7 @@ class ConvertersTest {
         val gregorianCalendarStart = Instant.parse("1582-10-15T00:00:00Z").toEpochMilliseconds()
         val minBoundMillis = (NSDate.distantPast.timeIntervalSince1970 * 1000 + 0.5).toLong()
         val maxBoundMillis = (NSDate.distantFuture.timeIntervalSince1970 * 1000 - 0.5).toLong()
-        repeat (1000) {
+        repeat(STRESS_TEST_ITERATIONS) {
             val millis = Random.nextLong(minBoundMillis, maxBoundMillis)
             val instant = Instant.fromEpochMilliseconds(millis)
             val date = instant.toNSDate()
diff --git a/core/js/src/LocalTime.kt b/core/js/src/LocalTime.kt
@@ -27,6 +27,7 @@ public actual class LocalTime internal constructor(internal val value: jtLocalTi
     public actual val second: Int get() = value.second().toInt()
     public actual val nanosecond: Int get() = value.nano().toInt()
     public actual fun toSecondOfDay(): Int = value.toSecondOfDay().toInt()
+    public actual fun toMillisecondOfDay(): Int = (value.toNanoOfDay().toDouble() / NANOS_PER_MILLI).toInt()
     public actual fun toNanosecondOfDay(): Long = value.toNanoOfDay().toLong()
 
     override fun equals(other: Any?): Boolean =
@@ -52,8 +53,15 @@ public actual class LocalTime internal constructor(internal val value: jtLocalTi
             throw IllegalArgumentException(e)
         }
 
+        public actual fun fromMillisecondOfDay(millisecondOfDay: Int): LocalTime = try {
+            jtLocalTime.ofNanoOfDay(millisecondOfDay * 1_000_000.0).let(::LocalTime)
+        } catch (e: Throwable) {
+            throw IllegalArgumentException(e)
+        }
+
         public actual fun fromNanosecondOfDay(nanosecondOfDay: Long): LocalTime = try {
-            jtLocalTime.ofNanoOfDay(nanosecondOfDay).let(::LocalTime)
+            // number of nanoseconds in a day is much less than `Number.MAX_SAFE_INTEGER`.
+            jtLocalTime.ofNanoOfDay(nanosecondOfDay.toDouble()).let(::LocalTime)
         } catch (e: Throwable) {
             throw IllegalArgumentException(e)
         }
diff --git a/core/jvm/src/LocalTime.kt b/core/jvm/src/LocalTime.kt
@@ -30,6 +30,7 @@ public actual class LocalTime internal constructor(internal val value: jtLocalTi
     public actual val second: Int get() = value.second
     public actual val nanosecond: Int get() = value.nano
     public actual fun toSecondOfDay(): Int = value.toSecondOfDay()
+    public actual fun toMillisecondOfDay(): Int = (value.toNanoOfDay() / NANOS_PER_MILLI).toInt()
     public actual fun toNanosecondOfDay(): Long = value.toNanoOfDay()
 
     override fun equals(other: Any?): Boolean =
@@ -54,6 +55,12 @@ public actual class LocalTime internal constructor(internal val value: jtLocalTi
             throw IllegalArgumentException(e)
         }
 
+        public actual fun fromMillisecondOfDay(millisecondOfDay: Int): LocalTime = try {
+            jtLocalTime.ofNanoOfDay(millisecondOfDay * 1_000_000L).let(::LocalTime)
+        } catch (e: Throwable) {
+            throw IllegalArgumentException(e)
+        }
+
         public actual fun fromNanosecondOfDay(nanosecondOfDay: Long): LocalTime = try {
             jtLocalTime.ofNanoOfDay(nanosecondOfDay).let(::LocalTime)
         } catch (e: DateTimeException) {
diff --git a/core/jvm/test/ConvertersTest.kt b/core/jvm/test/ConvertersTest.kt
@@ -30,7 +30,7 @@ class ConvertersTest {
             assertEquals(jtInstant, ktInstant.toString().let(JTInstant::parse))
         }
 
-        repeat(1000) {
+        repeat(STRESS_TEST_ITERATIONS) {
             val seconds = Random.nextLong(1_000_000_000_000)
             val nanos = Random.nextInt()
             test(seconds, nanos)
@@ -70,7 +70,7 @@ class ConvertersTest {
             assertEquals(jtDateTime, ktDateTime.toString().let(JTLocalDateTime::parse))
         }
 
-        repeat(1000) {
+        repeat(STRESS_TEST_ITERATIONS) {
             test(randomDateTime())
         }
     }
@@ -87,7 +87,7 @@ class ConvertersTest {
             assertEquals(jtTime, ktTime.toString().let(JTLocalTime::parse))
         }
 
-        repeat(1000) {
+        repeat(STRESS_TEST_ITERATIONS) {
             test(randomTime())
         }
     }
@@ -104,7 +104,7 @@ class ConvertersTest {
             assertEquals(jtDate, ktDate.toString().let(JTLocalDate::parse))
         }
 
-        repeat(1000) {
+        repeat(STRESS_TEST_ITERATIONS) {
             test(randomDate())
         }
     }
@@ -128,7 +128,7 @@ class ConvertersTest {
             assertJtPeriodNormalizedEquals(jtPeriod, ktPeriod.toString().let(JTPeriod::parse))
         }
 
-        repeat(1000) {
+        repeat(STRESS_TEST_ITERATIONS) {
             test(Random.nextInt(-1000, 1000), Random.nextInt(-1000, 1000), Random.nextInt(-1000, 1000))
         }
     }
diff --git a/core/native/src/LocalDateTime.kt b/core/native/src/LocalDateTime.kt
diff --git a/core/native/src/LocalTime.kt b/core/native/src/LocalTime.kt
diff --git a/core/native/src/mathNative.kt b/core/native/src/mathNative.kt
diff --git a/core/native/test/ThreeTenBpLocalTimeTest.kt b/core/native/test/ThreeTenBpLocalTimeTest.kt
