refactor: switch pool config to the new format (#5667)

ndr-brt · web-flow · commit cf23f6bebd28 · 2026-04-14T15:39:27.000+02:00
diff --git a/extensions/common/sql/sql-pool/sql-pool-apache-commons/src/main/java/org/eclipse/edc/sql/pool/commons/CommonsConnectionPoolServiceExtension.java b/extensions/common/sql/sql-pool/sql-pool-apache-commons/src/main/java/org/eclipse/edc/sql/pool/commons/CommonsConnectionPoolServiceExtension.java
@@ -14,9 +14,12 @@
 
 package org.eclipse.edc.sql.pool.commons;
 
+import org.eclipse.edc.runtime.metamodel.annotation.Configuration;
 import org.eclipse.edc.runtime.metamodel.annotation.Extension;
 import org.eclipse.edc.runtime.metamodel.annotation.Inject;
 import org.eclipse.edc.runtime.metamodel.annotation.Setting;
+import org.eclipse.edc.runtime.metamodel.annotation.SettingContext;
+import org.eclipse.edc.runtime.metamodel.annotation.Settings;
 import org.eclipse.edc.spi.EdcException;
 import org.eclipse.edc.spi.monitor.Monitor;
 import org.eclipse.edc.spi.security.Vault;
@@ -30,10 +33,11 @@
 
 import java.util.LinkedList;
 import java.util.List;
+import java.util.Map;
 import java.util.Optional;
 import java.util.Properties;
-import java.util.function.BiFunction;
 import java.util.function.Consumer;
+import java.util.function.Supplier;
 import javax.sql.DataSource;
 
 import static java.util.Optional.ofNullable;
@@ -44,41 +48,28 @@ public class CommonsConnectionPoolServiceExtension implements ServiceExtension {
     public static final String NAME = "Commons Connection Pool";
 
     public static final String EDC_DATASOURCE_PREFIX = "edc.datasource";
-    private static final String EDC_DATASOURCE_CONFIG_CONTEXT = EDC_DATASOURCE_PREFIX + ".<name>";
-
-    @Setting(description = "JDBC url", required = true, context = EDC_DATASOURCE_CONFIG_CONTEXT)
     public static final String URL = "url";
-    @Setting(description = "Username to be used for the JDBC connection. Can be omitted if not required, or if the user is encoded in the JDBC url.", context = EDC_DATASOURCE_CONFIG_CONTEXT)
     public static final String USER = "user";
-    @Setting(description = "Username to be used for the JDBC connection. Can be omitted if not required, or if the password is encoded in the JDBC url.", context = EDC_DATASOURCE_CONFIG_CONTEXT)
     public static final String PASSWORD = "password";
-
-    @Setting(description = "Pool max idle connections", context = EDC_DATASOURCE_CONFIG_CONTEXT)
     public static final String POOL_CONNECTIONS_MAX_IDLE = "pool.connections.max-idle";
-    @Setting(description = "Pool max total connections",  context = EDC_DATASOURCE_CONFIG_CONTEXT)
     public static final String POOL_CONNECTIONS_MAX_TOTAL = "pool.connections.max-total";
-    @Setting(description = "Pool min idle connections",  context = EDC_DATASOURCE_CONFIG_CONTEXT)
     public static final String POOL_CONNECTIONS_MIN_IDLE = "pool.connections.min-idle";
-    @Setting(description = "Pool test on borrow", context = EDC_DATASOURCE_CONFIG_CONTEXT)
     public static final String POOL_CONNECTION_TEST_ON_BORROW = "pool.connection.test.on-borrow";
-    @Setting(description = "Pool test on create", context = EDC_DATASOURCE_CONFIG_CONTEXT)
     public static final String POOL_CONNECTION_TEST_ON_CREATE = "pool.connection.test.on-create";
-    @Setting(description = "Pool test on return", context = EDC_DATASOURCE_CONFIG_CONTEXT)
     public static final String POOL_CONNECTION_TEST_ON_RETURN = "pool.connection.test.on-return";
-    @Setting(description = "Pool test while idle", context = EDC_DATASOURCE_CONFIG_CONTEXT)
     public static final String POOL_CONNECTION_TEST_WHILE_IDLE = "pool.connection.test.while-idle";
-    @Setting(description = "Pool test query", context = EDC_DATASOURCE_CONFIG_CONTEXT)
     public static final String POOL_CONNECTION_TEST_QUERY = "pool.connection.test.query";
 
+    @SettingContext(EDC_DATASOURCE_PREFIX)
+    @Configuration
+    private Map<String, DatasourceConfiguration> datasources;
+
     @Inject
     private DataSourceRegistry dataSourceRegistry;
-
     @Inject
     private Monitor monitor;
-
     @Inject
     private ConnectionFactory connectionFactory;
-
     @Inject
     private Vault vault;
 
@@ -91,13 +82,13 @@ public String name() {
 
     @Override
     public void initialize(ServiceExtensionContext context) {
-        context.getConfig(EDC_DATASOURCE_PREFIX).partition().forEach(config -> {
-            var dataSourceName = config.currentNode();
-            var dataSource = createDataSource(config);
-            var connectionPool = createConnectionPool(dataSource, config);
+        datasources.forEach((name, configuration) -> {
+            var rootPath = EDC_DATASOURCE_PREFIX + "." + name;
+            var dataSource = createDataSource(rootPath, configuration, context.getConfig(rootPath));
+            var connectionPool = createConnectionPool(dataSource, configuration);
             commonsConnectionPools.add(connectionPool);
             var connectionPoolDataSource = new ConnectionPoolDataSource(connectionPool);
-            dataSourceRegistry.register(dataSourceName, connectionPoolDataSource);
+            dataSourceRegistry.register(name, connectionPoolDataSource);
         });
     }
 
@@ -106,51 +97,111 @@ public void cleanup() {
         commonsConnectionPools.forEach(CommonsConnectionPool::close);
     }
 
-    private DataSource createDataSource(Config config) {
-        var rootPath = EDC_DATASOURCE_PREFIX + "." + config.currentNode();
-
-        var jdbcUrl = getSecretOrSetting(rootPath, URL, config)
-                .orElseThrow(() -> new EdcException("Mandatory url for datasource '%s' not found. Please provide a value for it, either as a secret in the vault or an application property.".formatted(config.currentNode())));
-        var jdbcUser = getSecretOrSetting(rootPath, USER, config);
-        var jdbcPassword = getSecretOrSetting(rootPath, PASSWORD, config);
+    private DataSource createDataSource(String rootPath, DatasourceConfiguration configuration, Config currentNode) {
+        var jdbcUrl = getSecretOrSetting(rootPath, URL, configuration.url())
+                .orElseThrow(() -> new EdcException("Mandatory url for datasource '%s' not found. Please provide a value for it, either as a secret in the vault or an application property.".formatted(currentNode.currentNode())));
+        var jdbcUser = getSecretOrSetting(rootPath, USER, configuration.user());
+        var jdbcPassword = getSecretOrSetting(rootPath, PASSWORD, configuration.password());
 
         var properties = new Properties();
-        properties.putAll(config.getRelativeEntries());
+        properties.putAll(currentNode.getRelativeEntries());
 
         jdbcUser.ifPresent(u -> properties.put(USER, u));
         jdbcPassword.ifPresent(p -> properties.put(PASSWORD, p));
 
         return new ConnectionFactoryDataSource(connectionFactory, jdbcUrl, properties);
     }
 
-    private Optional<String> getSecretOrSetting(String rootPath, String key, Config config) {
+    private Optional<String> getSecretOrSetting(String rootPath, String key, String configValue) {
         var fullKey = rootPath + "." + key;
         return ofNullable(vault.resolveSecret(fullKey))
                 .or(() -> {
                     monitor.warning("Datasource configuration value '%s' not found in vault, will fall back to Config. Please consider putting datasource configuration into the vault.".formatted(fullKey));
-                    return Optional.ofNullable(config.getString(key, null));
+                    return Optional.ofNullable(configValue);
                 });
     }
 
-    private CommonsConnectionPool createConnectionPool(DataSource unPooledDataSource, Config config) {
+    private CommonsConnectionPool createConnectionPool(DataSource dataSource, DatasourceConfiguration configuration) {
         var builder = CommonsConnectionPoolConfig.Builder.newInstance();
 
-        setIfProvided(POOL_CONNECTIONS_MAX_IDLE, config::getInteger, builder::maxIdleConnections);
-        setIfProvided(POOL_CONNECTIONS_MAX_TOTAL, config::getInteger, builder::maxTotalConnections);
-        setIfProvided(POOL_CONNECTIONS_MIN_IDLE, config::getInteger, builder::minIdleConnections);
-        setIfProvided(POOL_CONNECTION_TEST_ON_BORROW, config::getBoolean, builder::testConnectionOnBorrow);
-        setIfProvided(POOL_CONNECTION_TEST_ON_CREATE, config::getBoolean, builder::testConnectionOnCreate);
-        setIfProvided(POOL_CONNECTION_TEST_ON_RETURN, config::getBoolean, builder::testConnectionOnReturn);
-        setIfProvided(POOL_CONNECTION_TEST_WHILE_IDLE, config::getBoolean, builder::testConnectionWhileIdle);
-        setIfProvided(POOL_CONNECTION_TEST_QUERY, config::getString, builder::testQuery);
+        setIfProvided(configuration::poolConnectionsMaxIdle, builder::maxIdleConnections);
+        setIfProvided(configuration::poolConnectionsMaxTotal, builder::maxTotalConnections);
+        setIfProvided(configuration::poolConnectionsMinIdle, builder::minIdleConnections);
+        setIfProvided(configuration::poolConnectionTestOnBorrow, builder::testConnectionOnBorrow);
+        setIfProvided(configuration::poolConnectionTestOnCreate, builder::testConnectionOnCreate);
+        setIfProvided(configuration::poolConnectionTestOnReturn, builder::testConnectionOnReturn);
+        setIfProvided(configuration::poolConnectionTestWhileIdle, builder::testConnectionWhileIdle);
+        setIfProvided(configuration::poolConnectionTestQuery, builder::testQuery);
 
-        return new CommonsConnectionPool(unPooledDataSource, builder.build(), monitor);
+        return new CommonsConnectionPool(dataSource, builder.build(), monitor);
     }
 
-    private <T> void setIfProvided(String key, BiFunction<String, T, T> getter, Consumer<T> setter) {
-        var value = getter.apply(key, null);
+    private <T> void setIfProvided(Supplier<T> supplier, Consumer<T> setter) {
+        var value = supplier.get();
         if (value != null) {
             setter.accept(value);
         }
     }
+
+    @Settings
+    private record DatasourceConfiguration(
+            @Setting(
+                    key = URL,
+                    description = "JDBC url",
+                    required = false)
+            String url,
+            @Setting(
+                    key = USER,
+                    description = "Username to be used for the JDBC connection. Can be omitted if not required, or if the user is encoded in the JDBC url.",
+                    required = false)
+            String user,
+            @Setting(
+                    key = PASSWORD,
+                    description = "Username to be used for the JDBC connection. Can be omitted if not required, or if the password is encoded in the JDBC url.",
+                    required = false)
+            String password,
+
+            @Setting(
+                    key = POOL_CONNECTIONS_MAX_IDLE,
+                    description = "Pool max idle connections",
+                    required = false)
+            Integer poolConnectionsMaxIdle,
+            @Setting(
+                    key = POOL_CONNECTIONS_MAX_TOTAL,
+                    description = "Pool max total connections",
+                    required = false)
+            Integer poolConnectionsMaxTotal,
+            @Setting(
+                    key = POOL_CONNECTIONS_MIN_IDLE,
+                    description = "Pool min idle connections",
+                    required = false)
+            Integer poolConnectionsMinIdle,
+            @Setting(
+                    key = POOL_CONNECTION_TEST_ON_BORROW,
+                    description = "Pool test on borrow",
+                    required = false)
+            Boolean poolConnectionTestOnBorrow,
+            @Setting(
+                    key = POOL_CONNECTION_TEST_ON_CREATE,
+                    description = "Pool test on create",
+                    required = false)
+            Boolean poolConnectionTestOnCreate,
+            @Setting(
+                    key = POOL_CONNECTION_TEST_ON_RETURN,
+                    description = "Pool test on return",
+                    required = false)
+            Boolean poolConnectionTestOnReturn,
+            @Setting(
+                    key = POOL_CONNECTION_TEST_WHILE_IDLE,
+                    description = "Pool test while idle",
+                    required = false)
+            Boolean poolConnectionTestWhileIdle,
+            @Setting(
+                    key = POOL_CONNECTION_TEST_QUERY,
+                    description = "Pool test query",
+                    required = false)
+            String poolConnectionTestQuery
+    ) {
+
+    }
 }
diff --git a/extensions/common/sql/sql-pool/sql-pool-apache-commons/src/test/java/org/eclipse/edc/sql/pool/commons/CommonsConnectionPoolServiceExtensionTest.java b/extensions/common/sql/sql-pool/sql-pool-apache-commons/src/test/java/org/eclipse/edc/sql/pool/commons/CommonsConnectionPoolServiceExtensionTest.java
@@ -15,11 +15,12 @@
 package org.eclipse.edc.sql.pool.commons;
 
 import org.assertj.core.api.ThrowingConsumer;
+import org.eclipse.edc.boot.system.injection.ObjectFactory;
 import org.eclipse.edc.boot.vault.InMemoryVault;
 import org.eclipse.edc.junit.extensions.DependencyInjectionExtension;
+import org.eclipse.edc.junit.extensions.TestExtensionContext;
 import org.eclipse.edc.spi.EdcException;
 import org.eclipse.edc.spi.security.Vault;
-import org.eclipse.edc.spi.system.ServiceExtensionContext;
 import org.eclipse.edc.spi.system.configuration.ConfigFactory;
 import org.eclipse.edc.sql.ConnectionFactory;
 import org.eclipse.edc.sql.datasource.ConnectionPoolDataSource;
@@ -54,7 +55,6 @@
 import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
-import static org.mockito.Mockito.when;
 
 @ExtendWith(DependencyInjectionExtension.class)
 class CommonsConnectionPoolServiceExtensionTest {
@@ -63,7 +63,7 @@ class CommonsConnectionPoolServiceExtensionTest {
     private final ConnectionFactory connectionFactory = mock();
 
     @BeforeEach
-    void setUp(ServiceExtensionContext context) {
+    void setUp(TestExtensionContext context) {
         context.registerService(DataSourceRegistry.class, dataSourceRegistry);
         context.registerService(Vault.class, new InMemoryVault(mock(), null));
         context.registerService(ConnectionFactory.class, connectionFactory);
@@ -73,9 +73,10 @@ void setUp(ServiceExtensionContext context) {
     @ArgumentsSource(ConfigProvider.class)
     void initialize_withConfig(Map<String, String> configuration, ThrowingConsumer<CommonsConnectionPoolConfig> checker,
                                boolean isEnv,
-                               CommonsConnectionPoolServiceExtension extension, ServiceExtensionContext context) {
+                               ObjectFactory factory, TestExtensionContext context) {
         var config = isEnv ? ConfigFactory.fromEnvironment(configuration) : ConfigFactory.fromMap(configuration);
-        when(context.getConfig(EDC_DATASOURCE_PREFIX)).thenReturn(config);
+        context.setConfig(config);
+        var extension = factory.constructInstance(CommonsConnectionPoolServiceExtension.class);
 
         extension.initialize(context);
 
@@ -87,14 +88,14 @@ void initialize_withConfig(Map<String, String> configuration, ThrowingConsumer<C
     }
 
     @Test
-    void initialize_fromVault(CommonsConnectionPoolServiceExtension extension, ServiceExtensionContext context) {
-        when(context.getConfig(EDC_DATASOURCE_PREFIX))
-                .thenReturn(ConfigFactory.fromMap(Map.of("ds1.name", "ds1")));
+    void initialize_fromVault(ObjectFactory factory, TestExtensionContext context) {
+        context.setConfig(ConfigFactory.fromMap(Map.of(EDC_DATASOURCE_PREFIX + ".ds1.name", "ds1")));
         var vault = context.getService(Vault.class);
 
         vault.storeSecret("edc.datasource." + DS_1_NAME + ".user", "test-user");
         vault.storeSecret("edc.datasource." + DS_1_NAME + ".password", "test-pwd");
         vault.storeSecret("edc.datasource." + DS_1_NAME + ".url", "jdbc://whatever");
+        var extension = factory.constructInstance(CommonsConnectionPoolServiceExtension.class);
 
         extension.initialize(context);
 
@@ -112,17 +113,17 @@ void initialize_fromVault(CommonsConnectionPoolServiceExtension extension, Servi
     }
 
     @Test
-    void initialize_fromVault_shouldOverrideConfig(CommonsConnectionPoolServiceExtension extension, ServiceExtensionContext context) {
-        when(context.getConfig(EDC_DATASOURCE_PREFIX))
-                .thenReturn(ConfigFactory.fromMap(
-                        Map.of("ds1.name", "ds1",
-                                "ds1.user", "this-should-be-ignored",
-                                "ds1.password", "this-as-well")));
+    void initialize_fromVault_shouldOverrideConfig(ObjectFactory factory, TestExtensionContext context) {
+        context.setConfig(ConfigFactory.fromMap(Map.of(
+                EDC_DATASOURCE_PREFIX + ".ds1.name", "ds1",
+                EDC_DATASOURCE_PREFIX + ".ds1.user", "this-should-be-ignored",
+                EDC_DATASOURCE_PREFIX + ".ds1.password", "this-as-well")));
         var vault = context.getService(Vault.class);
 
         vault.storeSecret("edc.datasource." + DS_1_NAME + ".user", "test-user");
         vault.storeSecret("edc.datasource." + DS_1_NAME + ".password", "test-pwd");
         vault.storeSecret("edc.datasource." + DS_1_NAME + ".url", "jdbc://whatever");
+        var extension = factory.constructInstance(CommonsConnectionPoolServiceExtension.class);
 
         extension.initialize(context);
 
@@ -141,18 +142,18 @@ void initialize_fromVault_shouldOverrideConfig(CommonsConnectionPoolServiceExten
 
     static class ConfigProvider implements ArgumentsProvider {
 
-        private final Map<String, String> defaultConfig = Map.of(DS_1_NAME + ".url", DS_1_NAME);
+        private final Map<String, String> defaultConfig = Map.of(EDC_DATASOURCE_PREFIX + "." + DS_1_NAME + ".url", DS_1_NAME);
 
         private final Map<String, String> configuration = Map.of(
-                DS_1_NAME + ".url", DS_1_NAME,
-                DS_1_NAME + "." + POOL_CONNECTION_TEST_ON_CREATE, "false",
-                DS_1_NAME + "." + POOL_CONNECTION_TEST_ON_BORROW, "false",
-                DS_1_NAME + "." + POOL_CONNECTION_TEST_ON_RETURN, "true",
-                DS_1_NAME + "." + POOL_CONNECTION_TEST_WHILE_IDLE, "true",
-                DS_1_NAME + "." + POOL_CONNECTION_TEST_QUERY, "SELECT foo FROM bar;",
-                DS_1_NAME + "." + POOL_CONNECTIONS_MIN_IDLE, "10",
-                DS_1_NAME + "." + POOL_CONNECTIONS_MAX_IDLE, "10",
-                DS_1_NAME + "." + POOL_CONNECTIONS_MAX_TOTAL, "10");
+                EDC_DATASOURCE_PREFIX + "." + DS_1_NAME + ".url", DS_1_NAME,
+                EDC_DATASOURCE_PREFIX + "." + DS_1_NAME + "." + POOL_CONNECTION_TEST_ON_CREATE, "false",
+                EDC_DATASOURCE_PREFIX + "." + DS_1_NAME + "." + POOL_CONNECTION_TEST_ON_BORROW, "false",
+                EDC_DATASOURCE_PREFIX + "." + DS_1_NAME + "." + POOL_CONNECTION_TEST_ON_RETURN, "true",
+                EDC_DATASOURCE_PREFIX + "." + DS_1_NAME + "." + POOL_CONNECTION_TEST_WHILE_IDLE, "true",
+                EDC_DATASOURCE_PREFIX + "." + DS_1_NAME + "." + POOL_CONNECTION_TEST_QUERY, "SELECT foo FROM bar;",
+                EDC_DATASOURCE_PREFIX + "." + DS_1_NAME + "." + POOL_CONNECTIONS_MIN_IDLE, "10",
+                EDC_DATASOURCE_PREFIX + "." + DS_1_NAME + "." + POOL_CONNECTIONS_MAX_IDLE, "10",
+                EDC_DATASOURCE_PREFIX + "." + DS_1_NAME + "." + POOL_CONNECTIONS_MAX_TOTAL, "10");
 
 
         @Override
