From 922cae44f932f80f28153fcb9445d94e9eec00e9 Mon Sep 17 00:00:00 2001
From: kprotty <kbutcher6200@gmail.com>
Date: Thu, 14 Dec 2023 11:21:11 -0600
Subject: [PATCH] java: cleanup ULID overflow & comparison

---
 .../main/java/com/tigerbeetle/UInt128.java    | 18 +++++---
 .../java/com/tigerbeetle/UInt128Test.java     | 42 ++++---------------
 2 files changed, 21 insertions(+), 39 deletions(-)

diff --git a/src/clients/java/src/main/java/com/tigerbeetle/UInt128.java b/src/clients/java/src/main/java/com/tigerbeetle/UInt128.java
index 9da77c6032..b74066bd05 100644
--- a/src/clients/java/src/main/java/com/tigerbeetle/UInt128.java
+++ b/src/clients/java/src/main/java/com/tigerbeetle/UInt128.java
@@ -184,9 +184,10 @@ public static byte[] asBytes(final BigInteger value) {
      *
      * The ULID returned always increases monotonically and is thread-safe.
      *
+     * @throws ArithmeticException as per ULID specs, if the 80 bits of randomness overflows.
      * @return an array of 16 bytes representing the 128-bit value.
      */
-    public static byte[] ULID() throws ArithmeticException {
+    public static byte[] ULID() {
         long randomLo;
         short randomHi;
         long timestamp = System.currentTimeMillis();
@@ -205,12 +206,19 @@ public static byte[] ULID() throws ArithmeticException {
             randomLo = random.getLong();
             randomHi = random.getShort();
 
-            // Increment lo. If that overflows, increment hi. If that overflows, throw error.
-            // ULID spec says to throw error upon 80-bit overflow (not relative to initial random).
-            if (randomLo++ == Long.MAX_VALUE && randomHi++ == 0xffff) {
-                throw new ArithmeticException("random bits overflow");
+            // If randomLo will overflow from increment, then increment randomHi as carry.
+            // If randomHi will overflow on increment, throw error on 80-bit random overflow.
+            if (randomLo == 0xFFFFFFFFFFFFFFFFL) {
+                if (randomHi == 0xffff) {
+                    throw new ArithmeticException("random bits overflow on monotonic increment");
+                } else {
+                    randomHi += 1;
+                }
             }
 
+            // Wrapping increment on randomLo. Java allows overflowing arithmetic by default.
+            randomLo += 1;
+
             random.flip();
             random.putLong(randomLo);
             random.putShort(randomHi);
diff --git a/src/clients/java/src/test/java/com/tigerbeetle/UInt128Test.java b/src/clients/java/src/test/java/com/tigerbeetle/UInt128Test.java
index 17192451fe..09833fe57b 100644
--- a/src/clients/java/src/test/java/com/tigerbeetle/UInt128Test.java
+++ b/src/clients/java/src/test/java/com/tigerbeetle/UInt128Test.java
@@ -1,8 +1,8 @@
 package com.tigerbeetle;
 
 import static org.junit.Assert.assertArrayEquals;
-import static org.junit.Assert.assertNotEquals;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.assertSame;
 import java.util.concurrent.CountDownLatch;
 import java.math.BigInteger;
@@ -172,44 +172,18 @@ public void testAsBigIntegerZero() {
 
     @Test
     public void testULID() throws Exception {
-        class DecodedULID {
-            public final long timestamp;
-            public final BigInteger random;
-
-            public DecodedULID(byte[] ulid) {
-                var buffer = ByteBuffer.wrap(ulid).order(ByteOrder.BIG_ENDIAN);
-                assertEquals(ulid.length, UInt128.SIZE);
-
-                var timestampHi = buffer.getInt();
-                var timestampLo = buffer.getShort();
-                timestamp = (((long) timestampHi) << 32) | timestampLo;
-
-                var randomLo = buffer.getLong();
-                var randomHi = buffer.getShort();
-                random = UInt128.asBigInteger(randomLo, randomHi);
-            }
-
-            public void assertMonotonicSince(final DecodedULID earlier) {
-                var compareEarlier = Long.compare(earlier.timestamp, timestamp);
-                if (compareEarlier == 0) {
-                    compareEarlier = earlier.random.compareTo(random);
-                }
-
-                // earlier NOT greater than this
-                assertNotEquals(compareEarlier, 1);
-            }
-        }
+        // Java's BigInteger uses bytes in big-endian, which should allow direct casting from ULID.
 
         {
             // Generate ULIDs, sleeping for ~1ms after a few to test intra-millisecond monotonicity.
-            var ulidA = new DecodedULID(UInt128.ULID());
+            var ulidA = new BigInteger(UInt128.ULID());
             for (int i = 0; i < 10_000_000; i++) {
                 if (i % 10_000 == 0) {
                     Thread.sleep(1);
                 }
 
-                var ulidB = new DecodedULID(UInt128.ULID());
-                ulidB.assertMonotonicSince(ulidA);
+                var ulidB = new BigInteger(UInt128.ULID());
+                assertTrue(ulidB.compareTo(ulidA) > 0);
 
                 // Use the generated ULID as the new reference point for the next loop.
                 ulidA = ulidB;
@@ -229,14 +203,14 @@ public void assertMonotonicSince(final DecodedULID earlier) {
                     latchStart.await();
 
                     // Same as serial test above, but with smaller bounds.
-                    var ulidA = new DecodedULID(UInt128.ULID());
+                    var ulidA = new BigInteger(UInt128.ULID());
                     for (int j = 0; j < 10_000; j++) {
                         if (j % 1000 == 0) {
                             Thread.sleep(1);
                         }
 
-                        var ulidB = new DecodedULID(UInt128.ULID());
-                        ulidB.assertMonotonicSince(ulidA);
+                        var ulidB = new BigInteger(UInt128.ULID());
+                        assertTrue(ulidB.compareTo(ulidA) > 0);
                         ulidA = ulidB;
                     }