From 803f1d86428ce3b653276d72207e5a0009b57d16 Mon Sep 17 00:00:00 2001 From: Rob Bygrave Date: Mon, 20 Nov 2023 22:18:53 +1300 Subject: [PATCH] Refactor tidy internals of DbPrimary, DatabaseFactory, ContainerConfig, ClusterManager --- .../main/java/io/ebean/DatabaseFactory.java | 8 +--- .../src/main/java/io/ebean/DbPrimary.java | 15 +------- .../java/io/ebean/config/ContainerConfig.java | 38 +++++++------------ .../src/test/java/io/ebean/DbPrimaryTest.java | 5 --- .../server/cluster/ClusterManager.java | 25 ++---------- .../server/core/DefaultContainer.java | 20 +++++++--- .../xtest/config/ContainerConfigTest.java | 25 ++++-------- 7 files changed, 44 insertions(+), 92 deletions(-) diff --git a/ebean-api/src/main/java/io/ebean/DatabaseFactory.java b/ebean-api/src/main/java/io/ebean/DatabaseFactory.java index f887c1416..85dd2cbb6 100644 --- a/ebean-api/src/main/java/io/ebean/DatabaseFactory.java +++ b/ebean-api/src/main/java/io/ebean/DatabaseFactory.java @@ -3,10 +3,9 @@ package io.ebean; import io.ebean.config.ContainerConfig; import io.ebean.service.SpiContainer; import io.ebean.service.SpiContainerFactory; - import jakarta.persistence.PersistenceException; + import java.util.Iterator; -import java.util.Properties; import java.util.ServiceLoader; import java.util.concurrent.locks.ReentrantLock; @@ -146,12 +145,9 @@ public final class DatabaseFactory { if (container != null) { return container; } - if (containerConfig == null) { // effectively load configuration from ebean.properties - Properties properties = DbPrimary.getProperties(); containerConfig = new ContainerConfig(); - containerConfig.loadFromProperties(properties); } container = createContainer(containerConfig); return container; @@ -160,7 +156,7 @@ public final class DatabaseFactory { /** * Create the container instance using the configuration. */ - protected static SpiContainer createContainer(ContainerConfig containerConfig) { + private static SpiContainer createContainer(ContainerConfig containerConfig) { Iterator factories = ServiceLoader.load(SpiContainerFactory.class).iterator(); if (factories.hasNext()) { return factories.next().create(containerConfig); diff --git a/ebean-api/src/main/java/io/ebean/DbPrimary.java b/ebean-api/src/main/java/io/ebean/DbPrimary.java index ea1980de6..48b1d5f9c 100644 --- a/ebean-api/src/main/java/io/ebean/DbPrimary.java +++ b/ebean-api/src/main/java/io/ebean/DbPrimary.java @@ -44,25 +44,12 @@ final class DbPrimary { * Return the default database name. */ static String getDefaultServerName() { - lock.lock(); - try { - getProperties(); - return defaultServerName; - } finally { - lock.unlock(); - } - } - - /** - * Return the default configuration Properties. - */ - static Properties getProperties() { lock.lock(); try { if (defaultServerName == null) { defaultServerName = determineDefaultServerName(); } - return Config.asProperties(); + return defaultServerName; } finally { lock.unlock(); } diff --git a/ebean-api/src/main/java/io/ebean/config/ContainerConfig.java b/ebean-api/src/main/java/io/ebean/config/ContainerConfig.java index 00cf8f8bf..20c05c27d 100644 --- a/ebean-api/src/main/java/io/ebean/config/ContainerConfig.java +++ b/ebean-api/src/main/java/io/ebean/config/ContainerConfig.java @@ -1,5 +1,8 @@ package io.ebean.config; +import io.avaje.config.Config; +import io.avaje.config.Configuration; + import java.util.Properties; /** @@ -15,8 +18,17 @@ public class ContainerConfig { private String namespace; private String podName; private int port; - private Properties properties; + private Configuration configuration; + + public ContainerConfig() { + this.configuration = Config.asConfiguration(); + this.active = configuration.getBool("ebean.cluster.active", active); + this.serviceName = configuration.getNullable("ebean.cluster.serviceName", serviceName); + this.namespace = configuration.getNullable("ebean.cluster.namespace", namespace); + this.podName = configuration.getNullable("ebean.cluster.podName", podName); + this.port = configuration.getInt("ebean.cluster.port", 0); + } /** * Return the service name. @@ -92,7 +104,7 @@ public class ContainerConfig { * Return the deployment properties. */ public Properties getProperties() { - return properties; + return properties != null ? properties : configuration.asProperties(); } /** @@ -102,26 +114,4 @@ public class ContainerConfig { this.properties = properties; } - /** - * Load the settings from properties. - */ - public void loadFromProperties(Properties properties) { - this.properties = properties; - this.active = getProperty(properties, "ebean.cluster.active", active); - this.serviceName = properties.getProperty("ebean.cluster.serviceName", serviceName); - this.namespace = properties.getProperty("ebean.cluster.namespace", namespace); - this.podName = properties.getProperty("ebean.cluster.podName", podName); - String portParam = properties.getProperty("ebean.cluster.port"); - if (portParam != null) { - this.port = Integer.parseInt(portParam); - } - } - - /** - * Return the boolean property setting. - */ - protected boolean getProperty(Properties properties, String key, boolean defaultValue) { - return "true".equalsIgnoreCase(properties.getProperty(key, Boolean.toString(defaultValue))); - } - } diff --git a/ebean-api/src/test/java/io/ebean/DbPrimaryTest.java b/ebean-api/src/test/java/io/ebean/DbPrimaryTest.java index 38da9f413..418f57ecd 100644 --- a/ebean-api/src/test/java/io/ebean/DbPrimaryTest.java +++ b/ebean-api/src/test/java/io/ebean/DbPrimaryTest.java @@ -21,9 +21,4 @@ class DbPrimaryTest { assertEquals("h2", DbPrimary.getDefaultServerName()); } - @Test - void testLoadProperties() { - Properties properties = DbPrimary.getProperties(); - assertFalse(properties.isEmpty()); - } } diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/cluster/ClusterManager.java b/ebean-core/src/main/java/io/ebeaninternal/server/cluster/ClusterManager.java index e370534d9..142e652f1 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/cluster/ClusterManager.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/cluster/ClusterManager.java @@ -5,7 +5,6 @@ import io.ebean.Database; import io.ebean.config.ContainerConfig; import io.ebeaninternal.server.transaction.RemoteTransactionEvent; -import java.util.Iterator; import java.util.ServiceLoader; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.locks.ReentrantLock; @@ -20,38 +19,22 @@ public class ClusterManager implements ServerLookup { private static final System.Logger clusterLogger = AppLog.getLogger("io.ebean.Cluster"); private final ReentrantLock lock = new ReentrantLock(); - private final ConcurrentHashMap serverMap = new ConcurrentHashMap<>(); - - private final Object monitor = new Object(); - private final ClusterBroadcast broadcast; - private boolean started; - private boolean shutdown; public ClusterManager(ContainerConfig config) { - ClusterBroadcastFactory factory = createFactory(); - if (factory != null && config.isActive()) { - broadcast = factory.create(this, config); + if (config.isActive()) { + ClusterBroadcastFactory factory = createFactory(); + broadcast = factory != null ? factory.create(this, config) : null; } else { broadcast = null; } } - /** - * Return the ClusterTransportFactory via ServiceLoader. - */ private ClusterBroadcastFactory createFactory() { - - ServiceLoader load = ServiceLoader.load(ClusterBroadcastFactory.class); - ClusterBroadcastFactory factory = null; - Iterator iterator = load.iterator(); - if (iterator.hasNext()) { - factory = iterator.next(); - } - return factory; + return ServiceLoader.load(ClusterBroadcastFactory.class).findFirst().orElse(null); } public void registerServer(Database server) { diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/core/DefaultContainer.java b/ebean-core/src/main/java/io/ebeaninternal/server/core/DefaultContainer.java index 30817020a..23147762a 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/core/DefaultContainer.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/core/DefaultContainer.java @@ -13,11 +13,13 @@ import io.ebeaninternal.server.cluster.ClusterManager; import io.ebeaninternal.server.core.bootup.BootupClassPathSearch; import io.ebeaninternal.server.core.bootup.BootupClasses; import io.ebeaninternal.server.executor.DefaultBackgroundExecutor; - import jakarta.persistence.PersistenceException; + import java.net.URL; import java.sql.Connection; import java.sql.SQLException; +import java.util.ArrayList; +import java.util.List; import java.util.ServiceLoader; import java.util.Set; import java.util.concurrent.locks.ReentrantLock; @@ -33,12 +35,22 @@ public final class DefaultContainer implements SpiContainer { private final ReentrantLock lock = new ReentrantLock(); private final ClusterManager clusterManager; + private final List entityClassRegisters; public DefaultContainer(ContainerConfig containerConfig) { this.clusterManager = new ClusterManager(containerConfig); // register so that we can shutdown any Ebean wide // resources such as clustering ShutdownManager.registerContainer(this); + entityClassRegisters = initEntityRegisters(); + } + + private List initEntityRegisters() { + var entityClassRegisters = new ArrayList(); + for (EntityClassRegister entityClassRegister : ServiceLoader.load(EntityClassRegister.class)) { + entityClassRegisters.add(entityClassRegister); + } + return entityClassRegisters; } @Override @@ -126,12 +138,10 @@ public final class DefaultContainer implements SpiContainer { } if (config.isLoadModuleInfo()) { // auto register entity classes - boolean found = false; - for (EntityClassRegister loader : ServiceLoader.load(EntityClassRegister.class)) { + for (EntityClassRegister loader : entityClassRegisters) { config.addAll(loader.classesFor(config.getName(), config.isDefaultServer())); - found = true; } - if (!found) { + if (entityClassRegisters.isEmpty()) { checkMissingModulePathProvides(); } } diff --git a/ebean-test/src/test/java/io/ebean/xtest/config/ContainerConfigTest.java b/ebean-test/src/test/java/io/ebean/xtest/config/ContainerConfigTest.java index 2ae4f24e2..6fba07abe 100644 --- a/ebean-test/src/test/java/io/ebean/xtest/config/ContainerConfigTest.java +++ b/ebean-test/src/test/java/io/ebean/xtest/config/ContainerConfigTest.java @@ -5,28 +5,19 @@ import org.junit.jupiter.api.Test; import java.util.Properties; +import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertEquals; -public class ContainerConfigTest { +class ContainerConfigTest { @Test - public void loadFromProperties() { - - Properties p = new Properties(); - p.setProperty("ebean.cluster.active", "true"); - p.setProperty("ebean.cluster.serviceName", "a"); - p.setProperty("ebean.cluster.namespace", "b"); - p.setProperty("ebean.cluster.podName", "c"); - p.setProperty("ebean.cluster.port", "42"); - - + void loadFromProperties() { ContainerConfig containerConfig = new ContainerConfig(); - containerConfig.loadFromProperties(p); - assertEquals(true, containerConfig.isActive()); - assertEquals("a", containerConfig.getServiceName()); - assertEquals("b", containerConfig.getNamespace()); - assertEquals("c", containerConfig.getPodName()); - assertEquals(42, containerConfig.getPort()); + assertThat(containerConfig.isActive()).isFalse(); + assertThat(containerConfig.getServiceName()).isNull(); + assertThat(containerConfig.getNamespace()).isNull(); + assertThat(containerConfig.getPodName()).isNull(); + assertThat(containerConfig.getPort()).isEqualTo(0); } }