Refactor UUID generator. Add uuid-property to serverConfig (#57)

Co-authored-by: Roland Praml <roland.praml@foconis.de>

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:
+   * <ul>
+   * <li><code>null</code> (default) The generator tries to get the hardwarwe MAC
+   * address. If this fails, it will fall back to 'generate' mode.</li>
+   * <li><code>"generate"</code> Hardware detection is skipped. It generates a
+   * random identifier and tries to persist this to the state file. This nodeId
+   * will be reused on next start. If persisting to the state file will fail also,
+   * it will fall back to 'random' mode.<br>
+   * This mode is good, if the MAC address is not reliable, e.g. if you run
+   * multiple ebean instances on the same machine.</li>
+   * <li><code>"random"</code> In this mode, a random node id is generated on each
+   * start. No stateFile is used. it will generate a new nodeId on each
+   * application start.<br>
+   * This mode is good, if you have no write access to save the state file.</li>
+   * <li><code>"xx-xx-xx-xx-xx-xx"</code> When an explicit nodeId is specified,
+   * this one is used.</li>
+   * </ul>
+   * Note: It is possible that multiple servers are sharing the same state file as
+   * long as they are in the <b>same</b> JVM/ClassLoader scope. In this case it is
+   * recommended to use the same uuidNodeId configuration.
+   * 
+   * If you have multiple servers in different JVMs, do <b>not</b> 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);

ebean-core/src/main/java/io/ebeaninternal/server/deploy/meta/DeployBeanDescriptor.java

@@ -786,7 +786,7 @@ public class DeployBeanDescriptor<T> 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;

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;
   }

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));
   }
 
   /**