From 263757aa86cb75c3955f0545bb8e0ac06b3c8a69 Mon Sep 17 00:00:00 2001 From: Roland Praml Date: Wed, 26 Jan 2022 11:46:02 +0100 Subject: [PATCH] Refactor UUID generator. Add uuid-property to serverConfig (#57) Co-authored-by: Roland Praml --- .../java/io/ebean/config/DatabaseConfig.java | 42 +++++ .../deploy/meta/DeployBeanDescriptor.java | 2 +- .../server/idgen/UuidV1IdGenerator.java | 175 ++++++++++++------ .../server/idgen/TestUuidGenerator.java | 133 ++++++++++++- 4 files changed, 290 insertions(+), 62 deletions(-) diff --git a/ebean-api/src/main/java/io/ebean/config/DatabaseConfig.java b/ebean-api/src/main/java/io/ebean/config/DatabaseConfig.java index 6db8b4ca7..2e61d2755 100644 --- a/ebean-api/src/main/java/io/ebean/config/DatabaseConfig.java +++ b/ebean-api/src/main/java/io/ebean/config/DatabaseConfig.java @@ -377,6 +377,33 @@ public class DatabaseConfig { */ private String uuidStateFile; + /** + * The node id (=mac address) for Version 1 UUIDs. There are several options: + * + * Note: It is possible that multiple servers are sharing the same state file as + * long as they are in the same JVM/ClassLoader scope. In this case it is + * recommended to use the same uuidNodeId configuration. + * + * If you have multiple servers in different JVMs, do not share the state + * files! + */ + private String uuidNodeId; + /** * The clock used for setting the timestamps (e.g. @UpdatedTimestamp) on objects. */ @@ -2050,6 +2077,20 @@ public class DatabaseConfig { public void setUuidStateFile(String uuidStateFile) { this.uuidStateFile = uuidStateFile; } + + /** + * Returns the V1-UUID-NodeId + */ + public String getUuidNodeId() { + return uuidNodeId; + } + + /** + * Sets the V1-UUID-NodeId. + */ + public void setUuidNodeId(String uuidNodeId) { + this.uuidNodeId = uuidNodeId; + } /** * Return true if LocalTime should be persisted with nanos precision. @@ -2930,6 +2971,7 @@ public class DatabaseConfig { uuidVersion = p.getEnum(UuidVersion.class, "uuidVersion", uuidVersion); uuidStateFile = p.get("uuidStateFile", uuidStateFile); + uuidNodeId = p.get("uuidNodeId", uuidNodeId); localTimeWithNanos = p.getBoolean("localTimeWithNanos", localTimeWithNanos); jodaLocalTimeMode = p.get("jodaLocalTimeMode", jodaLocalTimeMode); diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/deploy/meta/DeployBeanDescriptor.java b/ebean-core/src/main/java/io/ebeaninternal/server/deploy/meta/DeployBeanDescriptor.java index 2b5783aaa..7d1f2a1ae 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/deploy/meta/DeployBeanDescriptor.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/deploy/meta/DeployBeanDescriptor.java @@ -786,7 +786,7 @@ public class DeployBeanDescriptor implements DeployBeanDescriptorMeta { this.identityMode.setIdType(IdType.EXTERNAL); switch (config.getUuidVersion()) { case VERSION1: - this.idGenerator = UuidV1IdGenerator.getInstance(config.getUuidStateFile()); + this.idGenerator = UuidV1IdGenerator.getInstance(config.getUuidStateFile(), config.getUuidNodeId()); break; case VERSION1RND: this.idGenerator = UuidV1RndIdGenerator.INSTANCE; diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/idgen/UuidV1IdGenerator.java b/ebean-core/src/main/java/io/ebeaninternal/server/idgen/UuidV1IdGenerator.java index cdd99ea44..083e08df2 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/idgen/UuidV1IdGenerator.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/idgen/UuidV1IdGenerator.java @@ -12,7 +12,8 @@ import java.util.concurrent.ConcurrentHashMap; * It extends the UuidV1RndIdGenerator so that it can generate rfc4122 compliant Type 1 UUIDs. * * This generator produces real Type 1 UUIDs (best for sqlserver) - You should use this generator only, - * if you can guarantee, that mac addess is uniqe. + * if you can guarantee, that mac addess is uniqe or specify 'ebean.uuidNodeId' in your config file. + * */ public class UuidV1IdGenerator extends UuidV1RndIdGenerator { @@ -25,38 +26,36 @@ public class UuidV1IdGenerator extends UuidV1RndIdGenerator { /** * Returns an instance for given file. */ - public static UuidV1IdGenerator getInstance(String file) { - return INSTANCES.computeIfAbsent(new File(file), UuidV1IdGenerator::new); + public static UuidV1IdGenerator getInstance(String file, String nodeId) { + return getInstance(new File(file), nodeId); } /** * Returns an instance for given file. */ - public static UuidV1IdGenerator getInstance(File file) { - return INSTANCES.computeIfAbsent(file, UuidV1IdGenerator::new); + public static UuidV1IdGenerator getInstance(File file, String nodeId) { + return INSTANCES.computeIfAbsent(file, f ->new UuidV1IdGenerator(f, nodeId == null ? null : nodeId.toLowerCase())); } /** * Returns an alternative node id - set with the 'ebean.uuid.nodeId' system property. */ - private static byte[] getAlternativeNodeId() { + private static byte[] parseAlternativeNodeId(String altNodeId) { + String[] components = altNodeId.split("-"); + if (components.length != 6) { + throw new IllegalArgumentException(altNodeId + " is invalid. Expected format: xx-xx-xx-xx-xx-xx"); + } try { - String altNodeId = System.getProperty("ebean.uuid.nodeId"); - if (altNodeId != null) { - String[] components = altNodeId.split("-"); - if (components.length != 5) { - throw new IllegalArgumentException("Invalid nodeId string: " + altNodeId); - } - byte[] nodeId = new byte[6]; - for (int i=0; i<5; i++) { - nodeId[i] = Byte.decode("0x"+components[i]).byteValue(); - } - return nodeId; + byte[] nodeId = new byte[6]; + for (int i = 0; i < 6; i++) { + // do not use Byte.parseByte + // https://bugs.java.com/bugdatabase/view_bug.do?bug_id=6259307 + nodeId[i] = (byte) Integer.parseInt(components[i], 16); } - } catch (SecurityException se) { - // ignore + return nodeId; + } catch (IllegalArgumentException iae) { + throw new IllegalArgumentException(altNodeId + " is invalid.", iae); } - return null; } /** @@ -108,43 +107,85 @@ public class UuidV1IdGenerator extends UuidV1RndIdGenerator { * Creates a new instance of UuidGenerator. Note that there should not be more * than one instance per stateFile. */ - private UuidV1IdGenerator(final File stateFile) { + private UuidV1IdGenerator(final File stateFile, String altNodeId) { super(); this.stateFile = stateFile; try { - // See, if there is an alternative MAC address set. - nodeId = getAlternativeNodeId(); - if (nodeId != null) { - log.info("Using alternative MAC {} to generate Type 1 UUIDs", getNodeIdentifier()); + if (altNodeId == null) { + // using hardware mode + tryHardwareId(); + } else if (altNodeId.equals("generate")) { + tryGenerateMode(); + } else if (altNodeId.equals("random")) { + useRandomMode(); } else { - nodeId = getHardwareId(); - log.info("Using MAC {} to generate Type 1 UUIDs", getNodeIdentifier()); + // See, if there is an alternative MAC address set. + nodeId = parseAlternativeNodeId(altNodeId); + restoreState(); + log.info("Explicitly using ID {} to generate Type 1 UUIDs", getNodeIdentifier()); } - if (nodeId == null) { - canSaveState = false; - // RFC 4.5 use random portion for node - nodeId = super.getNodeIdBytes(); - log.error("Have to fall back to random node identifier {} (Reason: No suitable network interface found)", getNodeIdentifier()); + UUID uuid = nextId(null); + long ts = timeStamp.get(); + ts -= UUID_EPOCH_OFFSET; + ts /= MILLIS_TO_UUID; - } else { - boolean flag = restoreState(); - UUID uuid = nextId(null); - long ts = timeStamp.get(); - ts -= UUID_EPOCH_OFFSET; - ts /= MILLIS_TO_UUID; - - saveState(); - log.debug("RestoreState: {}, ClockSeq {}, Timestamp {}, uuid {}, stateFile: {})", flag, clockSeq.get(), - new Date(ts), uuid, stateFile); - } + saveState(); + log.debug("Saved state: clockSeq {}, timestamp {}, uuid {}, stateFile: {})", clockSeq.get(), new Date(ts), uuid, stateFile); } catch (IOException e) { - canSaveState = false; + log.error("There was a problem while detecting the nodeId. Falling back to random mode. Try using to specify 'ebean.uuidNodeId' property", e); + useRandomMode(); + } + } + + /** + * Tries to initialize the generator by retrieving the MAC address from + * hardware. If there is no suitable network interface found, it will fall back + * to "generate" mode. + * + * @throws IOException if state file is not readable or tryGenerateMode also + * fails. + */ + private void tryHardwareId() throws IOException { + try { + nodeId = getHardwareId(); + } catch (IOException e) { + log.error("Error while reading MAC address. Fall back to 'generate' mode", e); + tryGenerateMode(); + } + + if (nodeId != null) { + restoreState(); + log.info("Using MAC {} to generate Type 1 UUIDs", getNodeIdentifier()); + return; + } + log.warn("No suitable network interface found. Fall back to 'generate' mode"); + tryGenerateMode(); + } + + /** + * Tries the "generate" mode. A nodeId is generated once and saved to the state + * file + */ + private void tryGenerateMode() throws IOException { + if (restoreState()) { + log.info("Using recently generated nodeId {} to generate Type 1 UUIDs", getNodeIdentifier()); + } else { // RFC 4.5 use random portion for node nodeId = super.getNodeIdBytes(); - log.error("Have to fall back to random node identifier {} (Reason: {} )", getNodeIdentifier(), e.getMessage()); + log.info("Using a newly generated nodeId {} to generate Type 1 UUIDs", getNodeIdentifier()); } } + /** + * Random mode. A nodeId is generated every start up. no state file is + * maintained. This should always work. + */ + private void useRandomMode() { + canSaveState = false; + nodeId = super.getNodeIdBytes(); + log.info("Explicitly using a new random ID {} to generate Type 1 UUIDs", getNodeIdentifier()); + } + /** * Returns the Node-identifier (=MAC address) as string */ @@ -164,22 +205,38 @@ public class UuidV1IdGenerator extends UuidV1RndIdGenerator { */ private boolean restoreState() throws IOException { Properties prop = new Properties(); - if (stateFile.exists()) { - try (InputStream is = new FileInputStream(stateFile)) { - prop.load(is); - } + if (!stateFile.exists()) { + log.debug("State file '{}' does not exist", stateFile); + return false; } - if (getNodeIdentifier().equals(prop.getProperty("nodeId"))) { - try { - Integer seq = Integer.valueOf(prop.getProperty("clockSeq")) & 0x3FFF; - Long ts = Long.valueOf(prop.getProperty("timeStamp")); - clockSeq.set(seq); - timeStamp.set(ts); - log.debug("Restored state from '{}'", stateFile); - return true; - } catch (NumberFormatException nfe) { - // nop + try (InputStream is = new FileInputStream(stateFile)) { + prop.load(is); + } + + String propNodeId = prop.getProperty("nodeId"); + if (propNodeId == null || propNodeId.isEmpty()) { + log.warn("State file '{}' is incomplete", stateFile); + return false; // we cannot restore + } + try { + if (nodeId == null) { + nodeId = parseAlternativeNodeId(propNodeId); + } else if (!getNodeIdentifier().equals(propNodeId)) { + log.warn( + "The nodeId in the state file '{}' has changed from {} to {}. " + + "This can happen when MAC address changes or when two containers share the same state file", + stateFile, propNodeId, getNodeIdentifier()); + return false; } + Integer seq = Integer.valueOf(prop.getProperty("clockSeq")) & 0x3FFF; + Long ts = Long.valueOf(prop.getProperty("timeStamp")); + clockSeq.set(seq); + timeStamp.set(ts); + log.debug("State successfully restored: {}", prop); + return true; + + } catch (IllegalArgumentException nfe) { + log.error("State file '{}' is corrupt", stateFile, nfe); } return false; } diff --git a/ebean-test/src/test/java/io/ebeaninternal/server/idgen/TestUuidGenerator.java b/ebean-test/src/test/java/io/ebeaninternal/server/idgen/TestUuidGenerator.java index 50f8e81b9..9bed5a1d0 100644 --- a/ebean-test/src/test/java/io/ebeaninternal/server/idgen/TestUuidGenerator.java +++ b/ebean-test/src/test/java/io/ebeaninternal/server/idgen/TestUuidGenerator.java @@ -5,14 +5,26 @@ package io.ebeaninternal.server.idgen; import io.ebean.config.dbplatform.PlatformIdGenerator; + +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import java.io.File; +import java.io.FileInputStream; +import java.io.FileOutputStream; +import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; import java.util.Map; +import java.util.Properties; import java.util.UUID; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.atomic.AtomicBoolean; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.assertj.core.api.Assertions.within; /** * Run some simple tests for the UUID generator. @@ -52,12 +64,129 @@ public class TestUuidGenerator { } } + private File stateFile; + + @BeforeEach + void beforeEach() throws IOException { + stateFile = new File("target/" + UUID.randomUUID() + ".state"); + } + + @AfterEach + void afterEach() { + stateFile.delete(); + } + + private void writePropertyFile(String nodeId, String clockSeq, String timestamp) throws IOException { + Properties prop = new Properties(); + prop.setProperty("nodeId", nodeId); + prop.setProperty("clockSeq", clockSeq); + prop.setProperty("timeStamp", timestamp); + try (OutputStream os = new FileOutputStream(stateFile)) { + prop.store(os, "ebean uuid state file"); + } + } + + private Properties readPropertyFile() throws IOException { + Properties prop = new Properties(); + try (InputStream is = new FileInputStream(stateFile)) { + prop.load(is); + } + return prop; + } + /** + * Test takes ~0.3 sec + */ + @Test + public void testUuidFixedMac() throws Exception { + UuidV1IdGenerator gen = UuidV1IdGenerator.getInstance(stateFile, "01-02-03-04-05-06"); + UUID uuid = gen.nextId(null); + assertThat(uuid.node()).isEqualTo(0x010203040506L); + Properties props = readPropertyFile(); + assertThat(props) + .containsEntry("nodeId", "01-02-03-04-05-06") + .containsEntry("clockSeq", String.valueOf(uuid.clockSequence())); + assertThat(Long.parseLong(props.getProperty("timeStamp"))) + .isCloseTo(uuid.timestamp(), within(2_000_000L)); + } + + @Test + public void testUuidInvalidMac() throws Exception { + assertThatThrownBy(()-> UuidV1IdGenerator.getInstance(stateFile, "01-02-03-04-05")) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("01-02-03-04-05 is invalid. Expected format: xx-xx-xx-xx-xx-xx"); + assertThatThrownBy(()-> UuidV1IdGenerator.getInstance(stateFile, "01-02-03-04-05-GG")) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("01-02-03-04-05-gg is invalid.") + .hasRootCauseInstanceOf(NumberFormatException.class) + .hasRootCauseMessage("For input string: \"gg\""); + assertThat(stateFile).doesNotExist(); + } + + @Test + public void testUuidGenerate() throws Exception { + UuidV1IdGenerator gen = UuidV1IdGenerator.getInstance(stateFile, "generate"); + UUID uuid = gen.nextId(null); + assertThat(uuid).isNotNull(); + assertThat(stateFile).exists(); + } + + @Test + public void testUuidRandom() throws Exception { + UuidV1IdGenerator gen = UuidV1IdGenerator.getInstance(stateFile, "random"); + UUID uuid = gen.nextId(null); + assertThat(uuid).isNotNull(); + assertThat(stateFile).doesNotExist(); + } + + @Test + public void testUuidFile() throws Exception { + writePropertyFile("12-34-56-78-90-AB", "4252", "0"); + UuidV1IdGenerator gen = UuidV1IdGenerator.getInstance(stateFile, "generate"); + UUID uuid = gen.nextId(null); + assertThat(uuid.node()).isEqualTo(0x1234567890ABL); + assertThat(uuid.clockSequence()).isEqualTo(4252); + } + + @Test + public void testUuidFileInvalidNodeId() throws Exception { + writePropertyFile("AB-CD-EF-GH-IJ-KL", "4252", "0"); + UuidV1IdGenerator gen = UuidV1IdGenerator.getInstance(stateFile, "generate"); + // a error message will be printed + UUID uuid = gen.nextId(null); + assertThat(uuid).isNotNull(); + } + + @Test + public void testInvalidFileName() throws Exception { + UuidV1IdGenerator gen = UuidV1IdGenerator.getInstance("/", null); + UUID uuid = gen.nextId(null); + assertThat(uuid).isNotNull(); + } + + @Test + public void testInvalidStateFile() throws Exception { + writePropertyFile("", "", ""); + UuidV1IdGenerator gen = UuidV1IdGenerator.getInstance(stateFile, null); + UUID uuid = gen.nextId(null); + assertThat(uuid).isNotNull(); + } + + @Test + public void testMacChange() throws Exception { + writePropertyFile("01-02-03-04-05", "1234", "1234"); + UuidV1IdGenerator gen = UuidV1IdGenerator.getInstance(stateFile, null); + UUID uuid = gen.nextId(null); + assertThat(uuid).isNotNull(); + // MAC must be updated with HW/ID + assertThat(readPropertyFile()).doesNotContainEntry("nodeId", "01-02-03-04-05"); + } + /** * Test takes ~0.3 sec */ @Test public void testUuidV1SingleThread() throws Exception { - testGenerator(1, 500_000, UuidV1IdGenerator.getInstance("ebean-test-uuid.state")); + testGenerator(1, 500_000, UuidV1IdGenerator.getInstance(stateFile, null)); } /** @@ -65,7 +194,7 @@ public class TestUuidGenerator { */ @Test public void testUuidV1MultiThread() throws Exception { - testGenerator(10, 50_000, UuidV1IdGenerator.getInstance("ebean-test-uuid.state")); + testGenerator(10, 50_000, UuidV1IdGenerator.getInstance(stateFile, null)); } /**