revamp monotonic api surface and tests

Author: daphil19

kulid/src/commonMain/kotlin/dev/phillipslabs/kulid/ULID.kt

@@ -10,28 +10,27 @@ import kotlin.jvm.JvmInline
 import kotlin.math.absoluteValue
 import kotlin.random.Random
 
-// some of these are marked as internal so that tests can be conducted easier
+// these are marked as internal so that tests can be conducted easier
 // ideally, they would all be private unless there's a compelling reason to expose these
 
-private const val TIMESTAMP_BIT_SIZE = 48
+internal const val TIMESTAMP_BIT_SIZE = 48
 
-// this is really just used for testing
 internal const val TIMESTAMP_BYTE_SIZE = TIMESTAMP_BIT_SIZE / 8
 internal const val MAX_TIME = (1L shl TIMESTAMP_BIT_SIZE) - 1 // 2^48 - 1
-private const val RANDOM_BIT_SIZE = 80
-private const val ULID_BIT_SIZE = TIMESTAMP_BIT_SIZE + RANDOM_BIT_SIZE
+internal const val RANDOM_BIT_SIZE = 80
+internal const val ULID_BIT_SIZE = TIMESTAMP_BIT_SIZE + RANDOM_BIT_SIZE
 
 internal const val ULID_BYTE_SIZE = ULID_BIT_SIZE / 8
-private const val RANDOM_BYTE_SIZE = RANDOM_BIT_SIZE / 8
+internal const val RANDOM_BYTE_SIZE = RANDOM_BIT_SIZE / 8
 
-private const val ENCODED_ULID_BYTE_SIZE = 26
+internal const val ENCODED_ULID_BYTE_SIZE = 26
 
 // ULIDs don't saturate the entirety of their encoding space, and zero-pad the bits they don't use at the front
 // as a result, we need to shift around the bit we're looking at a little bit more when placing it
-private const val ULID_ENCODING_FRONT_PADDING = ENCODED_ULID_BYTE_SIZE * 5 - ULID_BIT_SIZE
+internal const val ULID_ENCODING_FRONT_PADDING = ENCODED_ULID_BYTE_SIZE * 5 - ULID_BIT_SIZE
 
 // Crockford's base32 alphabet
-private val ENCODING_CHARS = "0123456789ABCDEFGHJKMNPQRSTVWXYZ".toCharArray()
+internal val ENCODING_CHARS = "0123456789ABCDEFGHJKMNPQRSTVWXYZ".toCharArray()
 
 /**
  * A Universally Unique Lexicographically Sortable Identifier (ULID).
@@ -90,22 +89,27 @@ public value class ULID private constructor(
      */
     override fun toString(): String = encodeCrockfordBase32(value)
 
-    // we actually do need a class bc we have to keep track of the generator's state
-    // ... then again, do we really? This at least will make things faster than ripping it out
-    public class MonotonicGenerator(
-        private val secureRandom: Boolean = true,
+    public class MonotonicGenerator internal constructor(
+        private val randomProvider: (ByteArray) -> Unit,
+        private val clock: () -> Long,
     ) {
+        public constructor(secureRandom: Boolean = true) : this(
+            randomProvider = { randomBytes(secureRandom, it) },
+            clock = { Clock.System.now().toEpochMilliseconds() },
+        )
+
+        // these are internal so that tests can access them
         private var lastTimestamp = 0L
         private var lastRandom = ByteArray(RANDOM_BYTE_SIZE)
 
         public fun next(): ULID {
-            val timestamp = Clock.System.now().toEpochMilliseconds()
+            val timestamp = clock()
 
             if (timestamp == lastTimestamp) {
                 incrementRandom()
             } else {
                 lastTimestamp = timestamp
-                randomBytes(secureRandom, lastRandom)
+                randomProvider(lastRandom)
             }
 
             return ULID(lastTimestamp, lastRandom)
@@ -273,20 +277,20 @@ public value class ULID private constructor(
                     }
                 }
             }
+    }
+}
 
-        private fun randomBytes(
-            secure: Boolean,
-            buf: ByteArray? = null,
-        ): ByteArray {
-            val buffer = buf ?: ByteArray(RANDOM_BYTE_SIZE)
-            if (secure) {
-                CryptoRand.Default.nextBytes(
-                    buffer,
-                )
-            } else {
-                Random.nextBytes(buffer)
-            }
-            return buffer
-        }
+internal fun randomBytes(
+    secure: Boolean,
+    buf: ByteArray? = null,
+): ByteArray {
+    val buffer = buf ?: ByteArray(RANDOM_BYTE_SIZE)
+    if (secure) {
+        CryptoRand.Default.nextBytes(
+            buffer,
+        )
+    } else {
+        Random.nextBytes(buffer)
     }
+    return buffer
 }

kulid/src/commonTest/kotlin/dev/phillipslabs/kulid/TestULIDMonotonic.kt

@@ -1,12 +1,14 @@
 package dev.phillipslabs.kulid
 
+import kotlinx.datetime.Clock
 import kotlin.test.Test
+import kotlin.test.assertFailsWith
 import kotlin.test.assertTrue
 
 class TestULIDMonotonic {
     @Test
     fun generatorProducesNonDecreasingSequence() {
-        val gen = ULID.MonotonicGenerator(secureRandom = true)
+        val gen = ULID.MonotonicGenerator()
         var prev = gen.next()
         // Generate a reasonably large sequence to likely span multiple milliseconds
         repeat(2_000) {
@@ -21,52 +23,51 @@ class TestULIDMonotonic {
 
     @Test
     fun ulidsWithinSameMillisecondAreStrictlyIncreasing() {
-        val gen = ULID.MonotonicGenerator(secureRandom = true)
+        val staticTime = Clock.System.now().toEpochMilliseconds()
+        // insecure random is faster for tests
+        val gen = ULID.MonotonicGenerator(randomProvider = { randomBytes(false, it) }, clock = { staticTime })
 
-        // Keep trying to capture a batch generated within the same millisecond.
-        // We will iterate up to some upper bound to avoid an infinite loop on very slow environments.
-        var attempts = 0
-        var foundBatch = false
-        while (attempts++ < 50 && !foundBatch) {
-            // Start a new batch
-            val first = gen.next()
-            val tsPrefix = first.toString().take(10) // timestamp portion in Crockford Base32
+        val first = gen.next()
+        val second = gen.next()
 
-            val prev = first
+        assertTrue(
+            second > first,
+            "ULIDs within same millisecond must be strictly increasing: first=$first second=$second",
+        )
 
-            // Collect additional ULIDs while still in the same millisecond (same timestamp prefix)
-            val candidate = gen.next()
-            if (candidate.toString().startsWith(tsPrefix)) {
-                // Within same millisecond, should be strictly increasing by randomness
-                assertTrue(
-                    prev < candidate,
-                    "ULIDs within same millisecond must be strictly increasing: prev=$prev next=$candidate",
-                )
-
-                assertTrue(
-                    candidate.value
+        assertTrue(
+            second.value
+                .toByteArray()
+                .drop(TIMESTAMP_BYTE_SIZE)
+                .reversed()
+                .zip(
+                    first.value
                         .toByteArray()
                         .drop(TIMESTAMP_BYTE_SIZE)
-                        .reversed()
-                        .zip(
-                            prev.value
-                                .toByteArray()
-                                .drop(TIMESTAMP_BYTE_SIZE)
-                                .reversed(),
-                        )
-                        // any short circuits once we've found a true value to the predicate
-                        // since we only have the random bytes and they're in reverse order,
-                        // we should find the byte that incremented
-                        .any { (next, prev) ->
-                            // max value check to help avoid an overflow error
-                            prev != 0xff.toByte() && (prev + 1).toByte() == next
-                        },
+                        .reversed(),
                 )
-                foundBatch = true
-                break
-            }
-        }
+                // any short circuits once we've found a true value to the predicate
+                // since we only have the random bytes and they're in reverse order,
+                // we should find the byte that incremented
+                .any { (next, prev) ->
+                    // max value check to help avoid an overflow error
+                    prev != 0xff.toByte() && (prev + 1).toByte() == next
+                },
+        )
+    }
 
-        assertTrue(foundBatch, "Failed to observe two or more ULIDs within the same millisecond to validate strict increase")
+    @Test
+    fun ulidThrowsErrorWhenRandomIsAtMax() {
+        val staticTime = Clock.System.now().toEpochMilliseconds()
+        val gen =
+            ULID.MonotonicGenerator(
+                randomProvider = { it.fill(0xff.toByte()) },
+                clock = { staticTime },
+            )
+        // first ULID should be ok, as it uses timestamp and random to make a max ulid
+        gen.next()
+        assertFailsWith<IllegalStateException>("Failed to throw at a max-random ULID increment attempt") {
+            gen.next()
+        }
     }
 }