- * When {@link #setRegister(boolean)} is set to true, and a database with the same - * name is already registered, this may return the existing registered database - * rather than creating a new one. + * When {@link #setRegister(boolean)} is set to true (the default), and a database + * with the same name is already registered, this throws an {@link IllegalStateException}. + * Use a unique name, or use {@link #setRegister(boolean)} with {@code false} if the + * Database instance is not intended to be registered/looked up by name. */ Database build(); diff --git a/ebean-api/src/main/java/io/ebean/DatabaseFactory.java b/ebean-api/src/main/java/io/ebean/DatabaseFactory.java index e3044854f..888c3b41a 100644 --- a/ebean-api/src/main/java/io/ebean/DatabaseFactory.java +++ b/ebean-api/src/main/java/io/ebean/DatabaseFactory.java @@ -7,8 +7,6 @@ import jakarta.persistence.PersistenceException; import java.util.concurrent.locks.ReentrantLock; -import static java.lang.System.Logger.Level.WARNING; - /** * Low-level factory for creating {@link Database} instances. *
@@ -81,10 +79,10 @@ public final class DatabaseFactory { // We're explicitly creating a database to be registered, so avoid // triggering DbContext static initialisation to auto-create a default one. DbPrimary.setSkip(true); - Database existing = DbContext.getInstance().getRegistered(name); - if (existing != null) { - EbeanVersion.log.log(WARNING, "Using existing database with name:{0}", name); - return existing; + if (DbContext.getInstance().contains(name)) { + throw new IllegalStateException("A Database with name [" + name + "] is already registered." + + " Use a unique DatabaseConfig name, or set DatabaseConfig.setRegister(false)" + + " if this Database instance is not intended to be registered/looked up by name."); } } Database server = createInternal(config); @@ -123,6 +121,24 @@ public final class DatabaseFactory { } } + /** + * Remove the registration of this Database. + *
+ * This is invoked when a Database is shutdown so that its registered name + * becomes available again for a subsequently created Database with the same name. + */ + public static void unregister(Database server) { + lock.lock(); + try { + DbContext.getInstance().deregister(server); + if (server.name().equals(defaultServerName)) { + defaultServerName = null; + } + } finally { + lock.unlock(); + } + } + /** * Shutdown gracefully all Database instances cleaning up any resources as required. *
diff --git a/ebean-api/src/main/java/io/ebean/DbContext.java b/ebean-api/src/main/java/io/ebean/DbContext.java index 19a444848..be32c02ae 100644 --- a/ebean-api/src/main/java/io/ebean/DbContext.java +++ b/ebean-api/src/main/java/io/ebean/DbContext.java @@ -4,7 +4,6 @@ import io.ebean.config.BeanNotEnhancedException; import io.ebean.datasource.DataSourceConfigurationException; import jakarta.persistence.PersistenceException; -import org.jspecify.annotations.Nullable; import java.util.HashMap; import java.util.concurrent.ConcurrentHashMap; @@ -77,9 +76,8 @@ final class DbContext { return defaultDatabase; } - @Nullable - Database getRegistered(String name) { - return concMap.get(name); + boolean contains(String name) { + return concMap.containsKey(name); } /** @@ -122,6 +120,27 @@ final class DbContext { registerWithName(server.name(), server, isDefault); } + /** + * Remove the registration for this Database (typically on shutdown) so that + * its name becomes available again for a subsequently created Database. + *
+ * Only removes the registration if it currently maps to this exact instance + * (avoids removing a different Database subsequently registered with the same name). + */ + void deregister(Database server) { + lock.lock(); + try { + String name = server.name(); + concMap.remove(name, server); + syncMap.remove(name, server); + if (defaultDatabase == server) { + defaultDatabase = null; + } + } finally { + lock.unlock(); + } + } + private void registerWithName(String name, Database server, boolean isDefault) { lock.lock(); try { diff --git a/ebean-api/src/main/java/io/ebean/event/ShutdownManager.java b/ebean-api/src/main/java/io/ebean/event/ShutdownManager.java index 58c2eb903..b3416f129 100644 --- a/ebean-api/src/main/java/io/ebean/event/ShutdownManager.java +++ b/ebean-api/src/main/java/io/ebean/event/ShutdownManager.java @@ -1,6 +1,7 @@ package io.ebean.event; import io.ebean.Database; +import io.ebean.DatabaseFactory; import io.ebean.EbeanVersion; import io.ebean.service.SpiContainer; @@ -188,6 +189,7 @@ public final class ShutdownManager { */ public static void unregisterDatabase(Database server) { databases.remove(server); + DatabaseFactory.unregister(server); } private static class ShutdownHook extends Thread { diff --git a/ebean-redis/src/test/java/org/integration/ClusterTest.java b/ebean-redis/src/test/java/org/integration/ClusterTest.java index b5d4edb6d..0d3986cfd 100644 --- a/ebean-redis/src/test/java/org/integration/ClusterTest.java +++ b/ebean-redis/src/test/java/org/integration/ClusterTest.java @@ -5,17 +5,23 @@ import io.ebean.Database; import io.ebean.redis.DuelCache; import org.domain.Person; import org.domain.query.QPerson; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; -import javax.sql.DataSource; - import static org.assertj.core.api.Assertions.assertThat; class ClusterTest { - private Database createOther(DataSource dataSource) { - return Database.builder() - .dataSource(dataSource) + private static Database db; + private static Database other; + + @BeforeAll + static void setup() { + // ensure the default server exists first + db = DB.getDefault(); + other = Database.builder() + .dataSource(db.pluginApi().dataSource()) .loadFromProperties() .defaultDatabase(false) .name("other") @@ -24,12 +30,13 @@ class ClusterTest { .build(); } + @AfterAll + static void tearDown() { + other.shutdown(false, false); + } + @Test void testBothNear() throws InterruptedException { - // ensure the default server exists first - final Database db = DB.getDefault(); - Database other = createOther(db.pluginApi().dataSource()); - new QPerson() .name.eq("Someone") .delete(); @@ -59,10 +66,6 @@ class ClusterTest { @Test void test() throws InterruptedException { - // ensure the default server exists first - final Database db = DB.getDefault(); - Database other = createOther(db.pluginApi().dataSource()); - for (int i = 0; i < 10; i++) { Person foo = new Person("name " + i); foo.save(); diff --git a/ebean-redisson/src/test/java/org/integration/ClusterTest.java b/ebean-redisson/src/test/java/org/integration/ClusterTest.java index 67fa2c566..4784d8439 100644 --- a/ebean-redisson/src/test/java/org/integration/ClusterTest.java +++ b/ebean-redisson/src/test/java/org/integration/ClusterTest.java @@ -5,17 +5,23 @@ import io.ebean.Database; import io.ebean.redisson.DuelCache; import org.domain.Person; import org.domain.query.QPerson; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; -import javax.sql.DataSource; - import static org.assertj.core.api.Assertions.assertThat; class ClusterTest { - private Database createOther(DataSource dataSource) { - return Database.builder() - .dataSource(dataSource) + private static Database db; + private static Database other; + + @BeforeAll + static void setup() { + // ensure the default server exists first + db = DB.getDefault(); + other = Database.builder() + .dataSource(db.pluginApi().dataSource()) .loadFromProperties() .defaultDatabase(false) .name("other") @@ -24,12 +30,13 @@ class ClusterTest { .build(); } + @AfterAll + static void tearDown() { + other.shutdown(false, false); + } + @Test void testBothNear() throws InterruptedException { - // ensure the default server exists first - final Database db = DB.getDefault(); - Database other = createOther(db.pluginApi().dataSource()); - new QPerson() .name.eq("Someone") .delete(); @@ -59,10 +66,6 @@ class ClusterTest { @Test void test() throws InterruptedException { - // ensure the default server exists first - final Database db = DB.getDefault(); - Database other = createOther(db.pluginApi().dataSource()); - for (int i = 0; i < 10; i++) { Person foo = new Person("name " + i); foo.save(); diff --git a/ebean-test/src/test/java/io/ebean/xtest/base/EbeanServerFactory_ServerConfigStart_Test.java b/ebean-test/src/test/java/io/ebean/xtest/base/EbeanServerFactory_ServerConfigStart_Test.java index 02ae305be..d3e081979 100644 --- a/ebean-test/src/test/java/io/ebean/xtest/base/EbeanServerFactory_ServerConfigStart_Test.java +++ b/ebean-test/src/test/java/io/ebean/xtest/base/EbeanServerFactory_ServerConfigStart_Test.java @@ -15,6 +15,7 @@ import java.util.Set; import java.util.concurrent.atomic.AtomicInteger; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; public class EbeanServerFactory_ServerConfigStart_Test { @@ -83,7 +84,7 @@ public class EbeanServerFactory_ServerConfigStart_Test { } @Test - public void create_registeredDatabase_twice_returnsExistingInstance() { + public void create_registeredDatabase_twice_throwsException() { DatabaseBuilder config = new DatabaseConfig(); config.setName("h2"); @@ -100,12 +101,15 @@ public class EbeanServerFactory_ServerConfigStart_Test { config.addServerConfigStartup(serverConfig -> startupCount.incrementAndGet()); Database db = DatabaseFactory.create(config); - Database existing = DatabaseFactory.create(config); + try { + assertThatThrownBy(() -> DatabaseFactory.create(config)) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("already registered"); - assertThat(existing).isSameAs(db); - assertThat(startupCount.get()).isEqualTo(1); - - db.shutdown(true, false); + assertThat(startupCount.get()).isEqualTo(1); + } finally { + db.shutdown(true, false); + } } @Test