From 7ec2a7cc98cbc6ca599d274570b46e6acfe0aeaf Mon Sep 17 00:00:00 2001 From: Fabio Lima Date: Sun, 23 Feb 2020 23:55:34 -0300 Subject: [PATCH] Throw exception when 2^80 requests are made In the previous implementation an exception was thrown when the random part reached 2^80. It meant that if the first random generated within a millisecond was (2^80 - 1000), the generator could thrown the exception when only 1000 requests was made. It was very difficult, but possible. Now it only occurs in the extremely unlikely event that you manage to generate more than 2^80 ULIDs within the same millisecond. --- .../com/github/f4b6a3/ulid/UlidCreator.java | 4 +- .../github/f4b6a3/ulid/guid/GuidCreator.java | 71 +++++++++++-------- .../f4b6a3/ulid/guid/GuidCreatorMock.java | 14 +++- .../f4b6a3/ulid/guid/GuidCreatorTest.java | 44 ++++++------ 4 files changed, 76 insertions(+), 57 deletions(-) diff --git a/src/main/java/com/github/f4b6a3/ulid/UlidCreator.java b/src/main/java/com/github/f4b6a3/ulid/UlidCreator.java index 9cbe579..5ee9b2e 100644 --- a/src/main/java/com/github/f4b6a3/ulid/UlidCreator.java +++ b/src/main/java/com/github/f4b6a3/ulid/UlidCreator.java @@ -108,10 +108,10 @@ public class UlidCreator { } private static class GuidCreatorLazyHolder { - static final GuidCreator INSTANCE = getGuidCreator().withoutOverflowException(); + static final GuidCreator INSTANCE = getGuidCreator().withoutOverrunException(); } private static class FastGuidCreatorLazyHolder { - static final GuidCreator INSTANCE = getGuidCreator().withFastRandomGenerator().withoutOverflowException(); + static final GuidCreator INSTANCE = getGuidCreator().withFastRandomGenerator().withoutOverrunException(); } } diff --git a/src/main/java/com/github/f4b6a3/ulid/guid/GuidCreator.java b/src/main/java/com/github/f4b6a3/ulid/guid/GuidCreator.java index cd55936..90db56c 100644 --- a/src/main/java/com/github/f4b6a3/ulid/guid/GuidCreator.java +++ b/src/main/java/com/github/f4b6a3/ulid/guid/GuidCreator.java @@ -44,18 +44,20 @@ import com.github.f4b6a3.ulid.timestamp.DefaultTimestampStrategy; */ public class GuidCreator { - protected static final long MAX_LOW = 0xffffffffffffffffL; // unsigned - protected static final long MAX_HIGH = 0x000000000000ffffL; - - protected long previousTimestamp; - protected boolean enableOverflowException = true; - - protected Random random; - protected long low; protected long high; - protected static final String OVERFLOW_MESSAGE = "The system caused an overflow in the generator by requesting too many IDs."; + protected long firstLow; + protected long firstHigh; + + protected long previousTimestamp; + protected boolean enableOverrunException = true; + + protected Random random; + + protected static final long MASK_UNSIGNED_SHORT = 0x000000000000ffffL; // 2^16 + + protected static final String OVERRUN_MESSAGE = "The system overran the generator by requesting too many GUIDs."; protected TimestampStrategy timestampStrategy; @@ -76,11 +78,13 @@ public class GuidCreator { * 2. A part of 80 bits that has a random value generated a secure random * generator. * + * The random part is reset to a new value every time the millisecond part + * changes. + * * If more than one GUID is generated within the same millisecond, the * random part is incremented by one. * - * The random part is reset to a new value every time the millisecond part - * changes. + * The maximum GUIDs that can be generated per millisecond is 2^80. * * ### Specification of Universally Unique Lexicographically Sortable ID * @@ -110,15 +114,14 @@ public class GuidCreator { * significant bit position (with carrying). * * If, in the extremely unlikely event that, you manage to generate more - * than 280 ULIDs within the same millisecond, or cause the random component - * to overflow with less, the generation will fail. + * than 2^80 ULIDs within the same millisecond, or cause the random + * component to overflow with less, the generation will fail. * * @return {@link UUID} a UUID value * * @throws UlidCreatorException - * an overflow exception if too many requests within the same - * millisecond causes an overflow when incrementing the random - * bits of the GUID. + * an overrun exception if too many requests are made within the + * same millisecond. */ public synchronized UUID create() { @@ -129,7 +132,7 @@ public class GuidCreator { return new UUID(msb, lsb); } - + /** * Return a ULID. * @@ -139,7 +142,7 @@ public class GuidCreator { UUID guid = create(); return UlidUtil.fromUuidToUlid(guid); } - + /** * Return a ULID as byte sequence. * @@ -173,27 +176,35 @@ public class GuidCreator { * Reset the random part of the GUID. */ protected synchronized void reset() { + + // Get random values if (random == null) { this.low = SecureRandomLazyHolder.INSTANCE.nextLong(); - this.high = SecureRandomLazyHolder.INSTANCE.nextLong() & MAX_HIGH; + this.high = SecureRandomLazyHolder.INSTANCE.nextInt() & MASK_UNSIGNED_SHORT; } else { this.low = random.nextLong(); - this.high = random.nextLong() & MAX_HIGH; + this.high = random.nextInt() & MASK_UNSIGNED_SHORT; } + + // Save the random values + this.firstLow = this.low; + this.firstHigh = this.high; } /** * Increment the random part of the GUID. * + * An exception is thrown when more than 2^80 increment operations are made. + * * @throws UlidCreatorException - * if an overflow happens. + * if an overrun happens. */ protected synchronized void increment() { - if ((this.low++ == MAX_LOW) && (this.high++ == MAX_HIGH)) { - this.high = 0L; + if ((++this.low == this.firstLow) && (++this.high == this.firstHigh)) { + this.reset(); // Too many requests - if (enableOverflowException) { - throw new UlidCreatorException(OVERFLOW_MESSAGE); + if (enableOverrunException) { + throw new UlidCreatorException(OVERRUN_MESSAGE); } } } @@ -251,16 +262,16 @@ public class GuidCreator { } /** - * Used to disable the overflow exception. + * Used to disable the overrun exception. * - * An exception thrown when too many requests within the same millisecond - * causes an overflow while incrementing the random bits of the GUID. + * An exception is thrown when too many requests are made within the same + * millisecond. * * @return {@link GuidCreator} */ @SuppressWarnings("unchecked") - public synchronized T withoutOverflowException() { - this.enableOverflowException = false; + public synchronized T withoutOverrunException() { + this.enableOverrunException = false; return (T) this; } diff --git a/src/test/java/com/github/f4b6a3/ulid/guid/GuidCreatorMock.java b/src/test/java/com/github/f4b6a3/ulid/guid/GuidCreatorMock.java index 539a653..7c1799f 100644 --- a/src/test/java/com/github/f4b6a3/ulid/guid/GuidCreatorMock.java +++ b/src/test/java/com/github/f4b6a3/ulid/guid/GuidCreatorMock.java @@ -1,12 +1,22 @@ package com.github.f4b6a3.ulid.guid; -import com.github.f4b6a3.ulid.guid.GuidCreator; - class GuidCreatorMock extends GuidCreator { + public GuidCreatorMock(long low, long high, long previousTimestamp) { + this(low, high, low, high, previousTimestamp); + } + + public GuidCreatorMock(long low, long high, long firstLow, long firstHigh, long previousTimestamp) { super(); + + // Set initial values this.low = low; this.high = high; + + // Save the initial values + this.firstLow = firstLow; + this.firstHigh = firstHigh; + this.previousTimestamp = previousTimestamp; } } \ No newline at end of file diff --git a/src/test/java/com/github/f4b6a3/ulid/guid/GuidCreatorTest.java b/src/test/java/com/github/f4b6a3/ulid/guid/GuidCreatorTest.java index 10496c0..caaec03 100644 --- a/src/test/java/com/github/f4b6a3/ulid/guid/GuidCreatorTest.java +++ b/src/test/java/com/github/f4b6a3/ulid/guid/GuidCreatorTest.java @@ -6,7 +6,6 @@ import java.util.UUID; import org.junit.Test; import com.github.f4b6a3.ulid.exception.UlidCreatorException; -import com.github.f4b6a3.ulid.guid.GuidCreator; import com.github.f4b6a3.ulid.random.Xorshift128PlusRandom; import com.github.f4b6a3.ulid.timestamp.FixedTimestampStretegy; @@ -14,19 +13,17 @@ import static org.junit.Assert.*; public class GuidCreatorTest { - private static final long DEFAULT_LOOP_MAX = 100_000; + private static final long DEFAULT_LOOP_MAX = 1_000_000; private static final long TIMESTAMP = System.currentTimeMillis(); - private static final long MAX_LOW = GuidCreator.MAX_LOW; - private static final long MAX_HIGH = GuidCreator.MAX_HIGH; private static final Random RANDOM = new Xorshift128PlusRandom(); @Test public void testRandomMostSignificantBits() { - long low = RANDOM.nextInt(); - long high = RANDOM.nextInt(Short.MAX_VALUE); + long low = RANDOM.nextLong(); + long high = (short) (RANDOM.nextInt()); GuidCreatorMock creator = new GuidCreatorMock(low, high, TIMESTAMP); creator.withTimestampStrategy(new FixedTimestampStretegy(TIMESTAMP)); @@ -34,7 +31,7 @@ public class GuidCreatorTest { UUID uuid = creator.create(); long firstMsb = (short) uuid.getMostSignificantBits(); long lastMsb = 0; - for (int i = 0; i <= DEFAULT_LOOP_MAX; i++) { + for (int i = 0; i < DEFAULT_LOOP_MAX; i++) { uuid = creator.create(); lastMsb = (short) uuid.getMostSignificantBits(); } @@ -74,8 +71,8 @@ public class GuidCreatorTest { @Test public void testIncrementOfRandomLeastSignificantBits() { - long low = RANDOM.nextInt(); - long high = RANDOM.nextInt(Short.MAX_VALUE); + long low = RANDOM.nextLong(); + long high = (short) RANDOM.nextInt(); GuidCreatorMock creator = new GuidCreatorMock(low, high, TIMESTAMP); creator.withTimestampStrategy(new FixedTimestampStretegy(TIMESTAMP)); @@ -85,16 +82,16 @@ public class GuidCreatorTest { uuid = creator.create(); } - long expected = low + DEFAULT_LOOP_MAX; + long expectedLsb = low + DEFAULT_LOOP_MAX; long randomLsb = uuid.getLeastSignificantBits(); - assertEquals(String.format("The LSB should be iqual to %s.", expected), expected, randomLsb); + assertEquals(String.format("The LSB should be iqual to %s.", expectedLsb), expectedLsb, randomLsb); } @Test public void testIncrementOfRandomMostSignificantBits() { - long low = MAX_LOW; - long high = RANDOM.nextInt(Short.MAX_VALUE); + long low = RANDOM.nextLong(); + long high = (short) (RANDOM.nextInt()); GuidCreatorMock creator = new GuidCreatorMock(low, high, TIMESTAMP); creator.withTimestampStrategy(new FixedTimestampStretegy(TIMESTAMP)); @@ -104,18 +101,21 @@ public class GuidCreatorTest { uuid = creator.create(); } - long expected = high + 1; - long randomMsb = uuid.getMostSignificantBits() & MAX_HIGH; + long expected = high; + long randomMsb = (short) (uuid.getMostSignificantBits()); assertEquals(String.format("The MSB should be iqual to %s.", expected), expected, randomMsb); } @Test(expected = UlidCreatorException.class) public void testShouldThrowOverflowException() { - long low = MAX_LOW - DEFAULT_LOOP_MAX; - long high = MAX_HIGH; + long startLow = RANDOM.nextInt() + DEFAULT_LOOP_MAX; + long startHigh = (short) (RANDOM.nextInt() + 1); - GuidCreatorMock creator = new GuidCreatorMock(low, high, TIMESTAMP); + long low = startLow - DEFAULT_LOOP_MAX; + long high = (short) (startHigh - 1); + + GuidCreatorMock creator = new GuidCreatorMock(low, high, startLow, startHigh, TIMESTAMP); creator.withTimestampStrategy(new FixedTimestampStretegy(TIMESTAMP)); UUID uuid = new UUID(0, 0); @@ -123,13 +123,11 @@ public class GuidCreatorTest { uuid = creator.create(); } - long expected = MAX_LOW; long randomLsb = uuid.getLeastSignificantBits(); - assertEquals(String.format("The LSB should be iqual to %s.", expected), expected, randomLsb); + assertEquals(String.format("The LSB should be iqual to %s.", startLow), startLow, randomLsb); - expected = MAX_HIGH; - long randomMsb = uuid.getMostSignificantBits() & MAX_HIGH; - assertEquals(String.format("The MSB should be iqual to %s.", expected), expected, randomMsb); + long randomMsb = (short) (uuid.getMostSignificantBits()); + assertEquals(String.format("The MSB should be iqual to %s.", startHigh), startHigh, randomMsb); creator.create(); fail("It should throw an overflow exception.");