From 101203a0f1f7b0f11e2e548288f6147430d25e4e Mon Sep 17 00:00:00 2001 From: Enrico Risa Date: Thu, 28 Sep 2023 10:31:54 +0200 Subject: [PATCH] feat(Pool): autovacuum fix + refactor (#3479) * feat(Pool): disable test connection on return, adds rollback if it's not autocommit, config refactor * docs: adds new pool configuration values in README.md * feat: removes types with var --- .../sql-pool-apache-commons/README.md | 41 ++- .../pool/commons/CommonsConnectionPool.java | 60 ++-- .../commons/CommonsConnectionPoolConfig.java | 2 +- .../CommonsConnectionPoolConfigKeys.java | 49 ++- ...CommonsConnectionPoolServiceExtension.java | 94 +++--- ...onsConnectionPoolServiceExtensionTest.java | 54 ---- .../CommonsConnectionPoolConfigTest.java | 57 ++-- ...onsConnectionPoolServiceExtensionTest.java | 291 ++++++++---------- .../commons/CommonsConnectionPoolTest.java | 207 +++++++------ .../DriverManagerConnectionFactoryTest.java | 61 ++++ 10 files changed, 494 insertions(+), 422 deletions(-) delete mode 100644 extensions/common/sql/sql-pool/sql-pool-apache-commons/src/test/java/org/eclipse/edc/sql/pool/commons/AbstractCommonsConnectionPoolServiceExtensionTest.java create mode 100644 extensions/common/sql/sql-pool/sql-pool-apache-commons/src/test/java/org/eclipse/edc/sql/pool/commons/DriverManagerConnectionFactoryTest.java diff --git a/extensions/common/sql/sql-pool/sql-pool-apache-commons/README.md b/extensions/common/sql/sql-pool/sql-pool-apache-commons/README.md index 8a08b1ca008..95cbea414a0 100644 --- a/extensions/common/sql/sql-pool/sql-pool-apache-commons/README.md +++ b/extensions/common/sql/sql-pool/sql-pool-apache-commons/README.md @@ -5,17 +5,32 @@ the `org.eclipse.edc.transaction.datasource.spi.DataSourceRegistry` capable of pooling `java.sql.Connection`s. The pooling mechanism is backed by the [Apache Commons Pool library](https://commons.apache.org/proper/commons-pool/). -## Configuration +## Old Configuration (Deprecated since 0.3.1) -| Key | Description | Mandatory | -|:---|:---|---| -| edc.datasource..url | JDBC driver url | X | -| edc.datasource..pool.maxIdleConnections | The maximum amount of idling connections maintained by the pool | | -| edc.datasource..pool.maxTotalConnections | The maximum amount of total connections maintained by the pool | | -| edc.datasource..pool.minIdleConnections | The minimum amount of idling connections maintained by the pool | | -| edc.datasource..pool.testConnectionOnBorrow | Flag to define whether connections will be validated when a connection has been obtained from the pool | | -| edc.datasource..pool.testConnectionOnCreate | Flag to define whether connections will be validated when a connection has been established | | -| edc.datasource..pool.testConnectionOnReturn | Flag to define whether connections will be validated when a connection has been returned to the pool | | -| edc.datasource..pool.testConnectionWhileIdle | Flag to define whether idling connections will be validated | | -| edc.datasource..pool.testQuery | Test query to validate a connection maintained by the pool | | -| edc.datasource.. | JDBC driver specific configuration properties | | +| Key | Description | Mandatory | +|:--------------------------------------------------------------|:-------------------------------------------------------------------------------------------------------|-----------| +| edc.datasource..url | JDBC driver url | X | +| edc.datasource..pool.maxIdleConnections | The maximum amount of idling connections maintained by the pool | | +| edc.datasource..pool.maxTotalConnections | The maximum amount of total connections maintained by the pool | | +| edc.datasource..pool.minIdleConnections | The minimum amount of idling connections maintained by the pool | | +| edc.datasource..pool.testConnectionOnBorrow | Flag to define whether connections will be validated when a connection has been obtained from the pool | | +| edc.datasource..pool.testConnectionOnCreate | Flag to define whether connections will be validated when a connection has been established | | +| edc.datasource..pool.testConnectionOnReturn | Flag to define whether connections will be validated when a connection has been returned to the pool | | +| edc.datasource..pool.testConnectionWhileIdle | Flag to define whether idling connections will be validated | | +| edc.datasource..pool.testQuery | Test query to validate a connection maintained by the pool | | +| edc.datasource.. | JDBC driver specific configuration properties | | + +## New Configuration (since 0.3.1) + +| Key | Description | Mandatory | +|:-----------------------------------------------------------------|:-------------------------------------------------------------------------------------------------------|-----------| +| edc.datasource..url | JDBC driver url | X | +| edc.datasource..pool.connections.max-idle | The maximum amount of idling connections maintained by the pool | | +| edc.datasource..pool.connections.max-total | The maximum amount of total connections maintained by the pool | | +| edc.datasource..pool.connections.min-idle | The minimum amount of idling connections maintained by the pool | | +| edc.datasource..pool.connection.test.on-borrow | Flag to define whether connections will be validated when a connection has been obtained from the pool | | +| edc.datasource..pool.connection.test.on-create | Flag to define whether connections will be validated when a connection has been established | | +| edc.datasource..pool.connection.test.on-return | Flag to define whether connections will be validated when a connection has been returned to the pool | | +| edc.datasource..pool.connection.test.while-idle | Flag to define whether idling connections will be validated | | +| edc.datasource..pool.connection.test.query | Test query to validate a connection maintained by the pool | | +| edc.datasource.. | JDBC driver specific configuration properties | | \ No newline at end of file diff --git a/extensions/common/sql/sql-pool/sql-pool-apache-commons/src/main/java/org/eclipse/edc/sql/pool/commons/CommonsConnectionPool.java b/extensions/common/sql/sql-pool/sql-pool-apache-commons/src/main/java/org/eclipse/edc/sql/pool/commons/CommonsConnectionPool.java index 5d5ed719b23..3f9da8e8418 100644 --- a/extensions/common/sql/sql-pool/sql-pool-apache-commons/src/main/java/org/eclipse/edc/sql/pool/commons/CommonsConnectionPool.java +++ b/extensions/common/sql/sql-pool/sql-pool-apache-commons/src/main/java/org/eclipse/edc/sql/pool/commons/CommonsConnectionPool.java @@ -20,6 +20,7 @@ import org.apache.commons.pool2.impl.DefaultPooledObject; import org.apache.commons.pool2.impl.GenericObjectPool; import org.apache.commons.pool2.impl.GenericObjectPoolConfig; +import org.eclipse.edc.spi.monitor.Monitor; import org.eclipse.edc.spi.persistence.EdcPersistenceException; import org.eclipse.edc.sql.pool.ConnectionPool; import org.jetbrains.annotations.NotNull; @@ -32,13 +33,15 @@ public final class CommonsConnectionPool implements ConnectionPool, AutoCloseable { private final GenericObjectPool connectionObjectPool; + private final CommonsConnectionPoolConfig poolConfig; - public CommonsConnectionPool(DataSource dataSource, CommonsConnectionPoolConfig commonsConnectionPoolConfig) { + public CommonsConnectionPool(DataSource dataSource, CommonsConnectionPoolConfig commonsConnectionPoolConfig, Monitor monitor) { + this.poolConfig = commonsConnectionPoolConfig; Objects.requireNonNull(dataSource, "connectionFactory"); Objects.requireNonNull(commonsConnectionPoolConfig, "commonsConnectionPoolConfig"); this.connectionObjectPool = new GenericObjectPool<>( - new PooledConnectionObjectFactory(dataSource, commonsConnectionPoolConfig.getTestQuery()), + new PooledConnectionObjectFactory(dataSource, commonsConnectionPoolConfig.getTestQuery(), monitor), getGenericObjectPoolConfig(commonsConnectionPoolConfig)); } @@ -81,13 +84,20 @@ public void close() { connectionObjectPool.close(); } + public CommonsConnectionPoolConfig getPoolConfig() { + return poolConfig; + } + private static class PooledConnectionObjectFactory extends BasePooledObjectFactory { private final String testQuery; private final DataSource dataSource; - PooledConnectionObjectFactory(@NotNull DataSource dataSource, @NotNull String testQuery) { + private final Monitor monitor; + + PooledConnectionObjectFactory(@NotNull DataSource dataSource, @NotNull String testQuery, Monitor monitor) { this.dataSource = Objects.requireNonNull(dataSource); this.testQuery = Objects.requireNonNull(testQuery); + this.monitor = monitor; } @Override @@ -96,32 +106,37 @@ public Connection create() throws SQLException { } @Override - public void destroyObject(PooledObject pooledObject, DestroyMode destroyMode) throws Exception { + public boolean validateObject(PooledObject pooledObject) { if (pooledObject == null) { - return; + return false; } Connection connection = pooledObject.getObject(); - - if (connection != null && !connection.isClosed()) { - connection.close(); + if (connection == null) { + return false; } - pooledObject.invalidate(); + return isConnectionValid(connection); } @Override - public boolean validateObject(PooledObject pooledObject) { + public PooledObject wrap(Connection connection) { + return new DefaultPooledObject<>(connection); + } + + @Override + public void destroyObject(PooledObject pooledObject, DestroyMode destroyMode) throws Exception { if (pooledObject == null) { - return false; + return; } Connection connection = pooledObject.getObject(); - if (connection == null) { - return false; + + if (connection != null && !connection.isClosed()) { + connection.close(); } - return isConnectionValid(connection); + pooledObject.invalidate(); } private boolean isConnectionValid(Connection connection) { @@ -131,16 +146,25 @@ private boolean isConnectionValid(Connection connection) { } try (PreparedStatement preparedStatement = connection.prepareStatement(testQuery)) { - return preparedStatement.execute(); + preparedStatement.execute(); + return rollbackIfNeeded(connection); } } catch (Exception e) { // any exception thrown indicates invalidity of the connection return false; } } - @Override - public PooledObject wrap(Connection connection) { - return new DefaultPooledObject<>(connection); + private boolean rollbackIfNeeded(Connection connection) { + try { + if (!connection.getAutoCommit()) { + connection.rollback(); + } + return true; + } catch (SQLException e) { + monitor.debug("Failed to rollback transaction", e); + } + return false; } + } } diff --git a/extensions/common/sql/sql-pool/sql-pool-apache-commons/src/main/java/org/eclipse/edc/sql/pool/commons/CommonsConnectionPoolConfig.java b/extensions/common/sql/sql-pool/sql-pool-apache-commons/src/main/java/org/eclipse/edc/sql/pool/commons/CommonsConnectionPoolConfig.java index 533d4b53a73..e9d539d29ee 100644 --- a/extensions/common/sql/sql-pool/sql-pool-apache-commons/src/main/java/org/eclipse/edc/sql/pool/commons/CommonsConnectionPoolConfig.java +++ b/extensions/common/sql/sql-pool/sql-pool-apache-commons/src/main/java/org/eclipse/edc/sql/pool/commons/CommonsConnectionPoolConfig.java @@ -90,7 +90,7 @@ public static final class Builder { private int minIdleConnections = 1; private boolean testConnectionOnBorrow = true; private boolean testConnectionOnCreate = true; - private boolean testConnectionOnReturn = true; + private boolean testConnectionOnReturn = false; private boolean testConnectionWhileIdle = false; private String testQuery = "SELECT 1;"; diff --git a/extensions/common/sql/sql-pool/sql-pool-apache-commons/src/main/java/org/eclipse/edc/sql/pool/commons/CommonsConnectionPoolConfigKeys.java b/extensions/common/sql/sql-pool/sql-pool-apache-commons/src/main/java/org/eclipse/edc/sql/pool/commons/CommonsConnectionPoolConfigKeys.java index 0597db7ab74..2767362b593 100644 --- a/extensions/common/sql/sql-pool/sql-pool-apache-commons/src/main/java/org/eclipse/edc/sql/pool/commons/CommonsConnectionPoolConfigKeys.java +++ b/extensions/common/sql/sql-pool/sql-pool-apache-commons/src/main/java/org/eclipse/edc/sql/pool/commons/CommonsConnectionPoolConfigKeys.java @@ -16,32 +16,65 @@ import org.eclipse.edc.runtime.metamodel.annotation.Setting; +import java.util.Map; + interface CommonsConnectionPoolConfigKeys { + @Deprecated(since = "0.3.1") @Setting(required = false) - String POOL_MAX_IDLE_CONNECTIONS = "pool.maxIdleConnections"; + String DEPRACATED_POOL_MAX_IDLE_CONNECTIONS = "pool.maxIdleConnections"; + String POOL_CONNECTIONS_MAX_IDLE = "pool.connections.max-idle"; + @Deprecated(since = "0.3.1") @Setting(required = false) - String POOL_MAX_TOTAL_CONNECTIONS = "pool.maxTotalConnections"; + String DEPRACATED_POOL_MAX_TOTAL_CONNECTIONS = "pool.maxTotalConnections"; + + String POOL_CONNECTIONS_MAX_TOTAL = "pool.connections.max-total"; + @Deprecated(since = "0.3.1") @Setting(required = false) - String POOL_MIN_IDLE_CONNECTIONS = "pool.minIdleConnections"; + String DEPRACATED_POOL_MIN_IDLE_CONNECTIONS = "pool.minIdleConnections"; + String POOL_CONNECTIONS_MIN_IDLE = "pool.connections.min-idle"; + @Deprecated(since = "0.3.1") @Setting(required = false) - String POOL_TEST_CONNECTION_ON_BORROW = "pool.testConnectionOnBorrow"; + String DEPRACATED_POOL_TEST_CONNECTION_ON_BORROW = "pool.testConnectionOnBorrow"; + String POOL_CONNECTION_TEST_ON_BORROW = "pool.connection.test.on-borrow"; + @Deprecated(since = "0.3.1") @Setting(required = false) - String POOL_TEST_CONNECTION_ON_CREATE = "pool.testConnectionOnCreate"; + String DEPRACATED_POOL_TEST_CONNECTION_ON_CREATE = "pool.testConnectionOnCreate"; + String POOL_CONNECTION_TEST_ON_CREATE = "pool.connection.test.on-create"; + + @Deprecated(since = "0.3.1") @Setting(required = false) - String POOL_TEST_CONNECTION_ON_RETURN = "pool.testConnectionOnReturn"; + String DEPRACATED_POOL_TEST_CONNECTION_ON_RETURN = "pool.testConnectionOnReturn"; + + String POOL_CONNECTION_TEST_ON_RETURN = "pool.connection.test.on-return"; + @Deprecated(since = "0.3.1") @Setting(required = false) - String POOL_TEST_CONNECTION_WHILE_IDLE = "pool.testConnectionWhileIdle"; + String DEPRACATED_POOL_TEST_CONNECTION_WHILE_IDLE = "pool.testConnectionWhileIdle"; + String POOL_CONNECTION_TEST_WHILE_IDLE = "pool.connection.test.while-idle"; + @Deprecated(since = "0.3.1") @Setting(required = false) - String POOL_TEST_QUERY = "pool.testQuery"; + String DEPRACATED_POOL_TEST_QUERY = "pool.testQuery"; + + String POOL_CONNECTION_TEST_QUERY = "pool.connection.test.query"; @Setting(required = true) String URL = "url"; + + Map CONFIGURATION_MAPPING = Map.of( + POOL_CONNECTIONS_MAX_IDLE, DEPRACATED_POOL_MAX_IDLE_CONNECTIONS, + POOL_CONNECTIONS_MIN_IDLE, DEPRACATED_POOL_MIN_IDLE_CONNECTIONS, + POOL_CONNECTIONS_MAX_TOTAL, DEPRACATED_POOL_MAX_TOTAL_CONNECTIONS, + POOL_CONNECTION_TEST_ON_BORROW, DEPRACATED_POOL_TEST_CONNECTION_ON_BORROW, + POOL_CONNECTION_TEST_ON_CREATE, DEPRACATED_POOL_TEST_CONNECTION_ON_CREATE, + POOL_CONNECTION_TEST_ON_RETURN, DEPRACATED_POOL_TEST_CONNECTION_ON_RETURN, + POOL_CONNECTION_TEST_WHILE_IDLE, DEPRACATED_POOL_TEST_CONNECTION_WHILE_IDLE, + POOL_CONNECTION_TEST_QUERY, DEPRACATED_POOL_TEST_QUERY); + } 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 index 547ca9ed084..7cf213e3d4e 100644 --- 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 @@ -16,6 +16,7 @@ import org.eclipse.edc.runtime.metamodel.annotation.Extension; import org.eclipse.edc.runtime.metamodel.annotation.Inject; +import org.eclipse.edc.spi.monitor.Monitor; import org.eclipse.edc.spi.system.ServiceExtension; import org.eclipse.edc.spi.system.ServiceExtensionContext; import org.eclipse.edc.spi.system.configuration.Config; @@ -30,43 +31,32 @@ import java.util.Map; import java.util.Objects; import java.util.Properties; +import java.util.function.BiFunction; import java.util.function.Consumer; -import java.util.stream.Collectors; import javax.sql.DataSource; +import static java.lang.String.format; +import static org.eclipse.edc.sql.pool.commons.CommonsConnectionPoolConfigKeys.CONFIGURATION_MAPPING; +import static org.eclipse.edc.sql.pool.commons.CommonsConnectionPoolConfigKeys.POOL_CONNECTIONS_MAX_IDLE; +import static org.eclipse.edc.sql.pool.commons.CommonsConnectionPoolConfigKeys.POOL_CONNECTIONS_MAX_TOTAL; +import static org.eclipse.edc.sql.pool.commons.CommonsConnectionPoolConfigKeys.POOL_CONNECTIONS_MIN_IDLE; +import static org.eclipse.edc.sql.pool.commons.CommonsConnectionPoolConfigKeys.POOL_CONNECTION_TEST_ON_BORROW; +import static org.eclipse.edc.sql.pool.commons.CommonsConnectionPoolConfigKeys.POOL_CONNECTION_TEST_ON_CREATE; +import static org.eclipse.edc.sql.pool.commons.CommonsConnectionPoolConfigKeys.POOL_CONNECTION_TEST_ON_RETURN; +import static org.eclipse.edc.sql.pool.commons.CommonsConnectionPoolConfigKeys.POOL_CONNECTION_TEST_QUERY; +import static org.eclipse.edc.sql.pool.commons.CommonsConnectionPoolConfigKeys.POOL_CONNECTION_TEST_WHILE_IDLE; +import static org.eclipse.edc.sql.pool.commons.CommonsConnectionPoolConfigKeys.URL; + @Extension(value = CommonsConnectionPoolServiceExtension.NAME) public class CommonsConnectionPoolServiceExtension implements ServiceExtension { public static final String NAME = "Commons Connection Pool"; - static final String EDC_DATASOURCE_PREFIX = "edc.datasource"; + public static final String EDC_DATASOURCE_PREFIX = "edc.datasource"; private final List commonsConnectionPools = new LinkedList<>(); @Inject private DataSourceRegistry dataSourceRegistry; - private static void setIfProvidedString(String key, Consumer setter, Config config) { - var value = config.getString(key, null); - if (value == null) { - return; - } - setter.accept(value); - } - - private static void setIfProvidedInt(String key, Consumer setter, Config config) { - var value = config.getInteger(key, null); - if (value == null) { - return; - } - - setter.accept(value); - } - - private static void setIfProvidedBoolean(String key, Consumer setter, Config config) { - var value = config.getString(key, null); - if (value == null) { - return; - } - - setter.accept(Boolean.parseBoolean(value)); - } + @Inject + private Monitor monitor; @Override public String name() { @@ -95,9 +85,37 @@ public void shutdown() { } } + public List getCommonsConnectionPools() { + return commonsConnectionPools; + } + + private void setIfProvidedString(String key, Consumer setter, Config config) { + setIfProvided(key, setter, config::getString); + } + + private void setIfProvidedBoolean(String key, Consumer setter, Config config) { + setIfProvided(key, setter, config::getBoolean); + } + + private void setIfProvidedInt(String key, Consumer setter, Config config) { + setIfProvided(key, setter, config::getInteger); + } + + private void setIfProvided(String key, Consumer setter, BiFunction getter) { + var oldKey = CONFIGURATION_MAPPING.get(key); + var oldValue = getter.apply(oldKey, null); + if (oldValue != null) { + monitor.warning(format("Configuration setting %s has been deprecated, please use %s instead", oldKey, key)); + } + var value = getter.apply(key, oldValue); + if (value != null) { + setter.accept(value); + } + } + private Map createConnectionPools(Config parent) { Map commonsConnectionPools = new HashMap<>(); - for (Config config : parent.partition().collect(Collectors.toList())) { + for (Config config : parent.partition().toList()) { String dataSourceName = config.currentNode(); DataSource dataSource = createDataSource(config); @@ -109,7 +127,7 @@ private Map createConnectionPools(Config parent) } private DataSource createDataSource(Config config) { - String jdbcUrl = Objects.requireNonNull(config.getString(CommonsConnectionPoolConfigKeys.URL)); + String jdbcUrl = Objects.requireNonNull(config.getString(URL)); Properties properties = new Properties(); properties.putAll(config.getRelativeEntries()); @@ -122,15 +140,15 @@ private DataSource createDataSource(Config config) { private CommonsConnectionPool createConnectionPool(DataSource unPooledDataSource, Config config) { CommonsConnectionPoolConfig.Builder builder = CommonsConnectionPoolConfig.Builder.newInstance(); - setIfProvidedInt(CommonsConnectionPoolConfigKeys.POOL_MAX_IDLE_CONNECTIONS, builder::maxIdleConnections, config); - setIfProvidedInt(CommonsConnectionPoolConfigKeys.POOL_MAX_TOTAL_CONNECTIONS, builder::maxTotalConnections, config); - setIfProvidedInt(CommonsConnectionPoolConfigKeys.POOL_MIN_IDLE_CONNECTIONS, builder::minIdleConnections, config); - setIfProvidedBoolean(CommonsConnectionPoolConfigKeys.POOL_TEST_CONNECTION_ON_BORROW, builder::testConnectionOnBorrow, config); - setIfProvidedBoolean(CommonsConnectionPoolConfigKeys.POOL_TEST_CONNECTION_ON_CREATE, builder::testConnectionOnCreate, config); - setIfProvidedBoolean(CommonsConnectionPoolConfigKeys.POOL_TEST_CONNECTION_ON_RETURN, builder::testConnectionOnReturn, config); - setIfProvidedBoolean(CommonsConnectionPoolConfigKeys.POOL_TEST_CONNECTION_WHILE_IDLE, builder::testConnectionWhileIdle, config); - setIfProvidedString(CommonsConnectionPoolConfigKeys.POOL_TEST_QUERY, builder::testQuery, config); + setIfProvidedInt(POOL_CONNECTIONS_MAX_IDLE, builder::maxIdleConnections, config); + setIfProvidedInt(POOL_CONNECTIONS_MAX_TOTAL, builder::maxTotalConnections, config); + setIfProvidedInt(POOL_CONNECTIONS_MIN_IDLE, builder::minIdleConnections, config); + setIfProvidedBoolean(POOL_CONNECTION_TEST_ON_BORROW, builder::testConnectionOnBorrow, config); + setIfProvidedBoolean(POOL_CONNECTION_TEST_ON_CREATE, builder::testConnectionOnCreate, config); + setIfProvidedBoolean(POOL_CONNECTION_TEST_ON_RETURN, builder::testConnectionOnReturn, config); + setIfProvidedBoolean(POOL_CONNECTION_TEST_WHILE_IDLE, builder::testConnectionWhileIdle, config); + setIfProvidedString(POOL_CONNECTION_TEST_QUERY, builder::testQuery, config); - return new CommonsConnectionPool(unPooledDataSource, builder.build()); + return new CommonsConnectionPool(unPooledDataSource, builder.build(), monitor); } } diff --git a/extensions/common/sql/sql-pool/sql-pool-apache-commons/src/test/java/org/eclipse/edc/sql/pool/commons/AbstractCommonsConnectionPoolServiceExtensionTest.java b/extensions/common/sql/sql-pool/sql-pool-apache-commons/src/test/java/org/eclipse/edc/sql/pool/commons/AbstractCommonsConnectionPoolServiceExtensionTest.java deleted file mode 100644 index 69738634539..00000000000 --- a/extensions/common/sql/sql-pool/sql-pool-apache-commons/src/test/java/org/eclipse/edc/sql/pool/commons/AbstractCommonsConnectionPoolServiceExtensionTest.java +++ /dev/null @@ -1,54 +0,0 @@ -/* - * Copyright (c) 2021 Daimler TSS GmbH - * - * This program and the accompanying materials are made available under the - * terms of the Apache License, Version 2.0 which is available at - * https://www.apache.org/licenses/LICENSE-2.0 - * - * SPDX-License-Identifier: Apache-2.0 - * - * Contributors: - * Daimler TSS GmbH - Initial Extension Test - * - */ - -package org.eclipse.edc.sql.pool.commons; - -import org.eclipse.edc.junit.extensions.EdcExtension; -import org.eclipse.edc.spi.system.ServiceExtension; -import org.eclipse.edc.spi.system.ServiceExtensionContext; -import org.eclipse.edc.transaction.datasource.spi.DataSourceRegistry; -import org.eclipse.edc.transaction.spi.TransactionContext; -import org.junit.jupiter.api.AfterEach; -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.extension.ExtendWith; - -import java.util.concurrent.atomic.AtomicReference; - -@ExtendWith(EdcExtension.class) -abstract class AbstractCommonsConnectionPoolServiceExtensionTest { - - private final AtomicReference contextRef = new AtomicReference<>(); - - @BeforeEach - void before(EdcExtension extension) { - extension.registerSystemExtension(ServiceExtension.class, new ServiceExtension() { - public void initialize(ServiceExtensionContext context) { - contextRef.set(context); - } - }); - } - - @AfterEach - void afterTestExecution() { - contextRef.set(null); - } - - protected DataSourceRegistry getDataSourceRegistry() { - return contextRef.get().getService(DataSourceRegistry.class); - } - - protected TransactionContext getTransactionContext() { - return contextRef.get().getService(TransactionContext.class); - } -} diff --git a/extensions/common/sql/sql-pool/sql-pool-apache-commons/src/test/java/org/eclipse/edc/sql/pool/commons/CommonsConnectionPoolConfigTest.java b/extensions/common/sql/sql-pool/sql-pool-apache-commons/src/test/java/org/eclipse/edc/sql/pool/commons/CommonsConnectionPoolConfigTest.java index 43cf0c6b658..2242ca049fe 100644 --- a/extensions/common/sql/sql-pool/sql-pool-apache-commons/src/test/java/org/eclipse/edc/sql/pool/commons/CommonsConnectionPoolConfigTest.java +++ b/extensions/common/sql/sql-pool/sql-pool-apache-commons/src/test/java/org/eclipse/edc/sql/pool/commons/CommonsConnectionPoolConfigTest.java @@ -14,37 +14,40 @@ package org.eclipse.edc.sql.pool.commons; -import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + class CommonsConnectionPoolConfigTest { @Test void testDefaults() { - CommonsConnectionPoolConfig commonsConnectionPoolConfig = CommonsConnectionPoolConfig.Builder.newInstance().build(); + var commonsConnectionPoolConfig = CommonsConnectionPoolConfig.Builder.newInstance().build(); - Assertions.assertEquals(4, commonsConnectionPoolConfig.getMaxIdleConnections()); - Assertions.assertEquals(8, commonsConnectionPoolConfig.getMaxTotalConnections()); - Assertions.assertEquals(1, commonsConnectionPoolConfig.getMinIdleConnections()); - Assertions.assertTrue(commonsConnectionPoolConfig.getTestConnectionOnBorrow()); - Assertions.assertTrue(commonsConnectionPoolConfig.getTestConnectionOnCreate()); - Assertions.assertTrue(commonsConnectionPoolConfig.getTestConnectionOnReturn()); - Assertions.assertFalse(commonsConnectionPoolConfig.getTestConnectionWhileIdle()); - Assertions.assertEquals("SELECT 1;", commonsConnectionPoolConfig.getTestQuery()); + assertEquals(4, commonsConnectionPoolConfig.getMaxIdleConnections()); + assertEquals(8, commonsConnectionPoolConfig.getMaxTotalConnections()); + assertEquals(1, commonsConnectionPoolConfig.getMinIdleConnections()); + assertTrue(commonsConnectionPoolConfig.getTestConnectionOnBorrow()); + assertTrue(commonsConnectionPoolConfig.getTestConnectionOnCreate()); + assertFalse(commonsConnectionPoolConfig.getTestConnectionOnReturn()); + assertFalse(commonsConnectionPoolConfig.getTestConnectionWhileIdle()); + assertEquals("SELECT 1;", commonsConnectionPoolConfig.getTestQuery()); } @Test void test() { - int minIdleConnections = 1; - int maxIdleConnections = 2; - int maxTotalConnections = 3; - boolean testConnectionOnBorrow = true; - boolean testConnectionOnCreate = false; - boolean testConnectionWhileIdle = true; - boolean testConnectionOnReturn = false; - String testQuery = "testquery"; + var minIdleConnections = 1; + var maxIdleConnections = 2; + var maxTotalConnections = 3; + var testConnectionOnBorrow = true; + var testConnectionOnCreate = false; + var testConnectionWhileIdle = true; + var testConnectionOnReturn = false; + var testQuery = "testquery"; - CommonsConnectionPoolConfig commonsConnectionPoolConfig = CommonsConnectionPoolConfig.Builder.newInstance() + var commonsConnectionPoolConfig = CommonsConnectionPoolConfig.Builder.newInstance() .maxIdleConnections(maxIdleConnections) .maxTotalConnections(maxTotalConnections) .minIdleConnections(minIdleConnections) @@ -55,13 +58,13 @@ void test() { .testQuery(testQuery) .build(); - Assertions.assertEquals(maxIdleConnections, commonsConnectionPoolConfig.getMaxIdleConnections()); - Assertions.assertEquals(maxTotalConnections, commonsConnectionPoolConfig.getMaxTotalConnections()); - Assertions.assertEquals(minIdleConnections, commonsConnectionPoolConfig.getMinIdleConnections()); - Assertions.assertEquals(testConnectionOnBorrow, commonsConnectionPoolConfig.getTestConnectionOnBorrow()); - Assertions.assertEquals(testConnectionOnCreate, commonsConnectionPoolConfig.getTestConnectionOnCreate()); - Assertions.assertEquals(testConnectionOnReturn, commonsConnectionPoolConfig.getTestConnectionOnReturn()); - Assertions.assertEquals(testConnectionWhileIdle, commonsConnectionPoolConfig.getTestConnectionWhileIdle()); - Assertions.assertEquals(testQuery, commonsConnectionPoolConfig.getTestQuery()); + assertEquals(maxIdleConnections, commonsConnectionPoolConfig.getMaxIdleConnections()); + assertEquals(maxTotalConnections, commonsConnectionPoolConfig.getMaxTotalConnections()); + assertEquals(minIdleConnections, commonsConnectionPoolConfig.getMinIdleConnections()); + assertEquals(testConnectionOnBorrow, commonsConnectionPoolConfig.getTestConnectionOnBorrow()); + assertEquals(testConnectionOnCreate, commonsConnectionPoolConfig.getTestConnectionOnCreate()); + assertEquals(testConnectionOnReturn, commonsConnectionPoolConfig.getTestConnectionOnReturn()); + assertEquals(testConnectionWhileIdle, commonsConnectionPoolConfig.getTestConnectionWhileIdle()); + assertEquals(testQuery, commonsConnectionPoolConfig.getTestQuery()); } } 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 index 87cbd5d8f4d..ee55649a6ed 100644 --- 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 @@ -14,194 +14,147 @@ package org.eclipse.edc.sql.pool.commons; -import org.eclipse.edc.transaction.local.DataSourceResource; -import org.junit.jupiter.api.AfterEach; -import org.junit.jupiter.api.Assertions; +import org.assertj.core.api.ThrowingConsumer; +import org.eclipse.edc.junit.extensions.DependencyInjectionExtension; +import org.eclipse.edc.spi.system.ServiceExtensionContext; +import org.eclipse.edc.spi.system.configuration.Config; +import org.eclipse.edc.spi.system.configuration.ConfigFactory; +import org.eclipse.edc.transaction.datasource.spi.DataSourceRegistry; import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Disabled; -import org.junit.jupiter.api.DisplayName; -import org.junit.jupiter.api.Test; -import org.mockito.MockedStatic; -import org.mockito.Mockito; - -import java.sql.Connection; -import java.sql.DriverManager; -import java.sql.PreparedStatement; -import java.sql.SQLException; -import java.util.HashMap; +import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.api.extension.ExtensionContext; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.ArgumentsProvider; +import org.junit.jupiter.params.provider.ArgumentsSource; + import java.util.Map; -import java.util.Properties; -import java.util.stream.IntStream; -import javax.sql.DataSource; +import java.util.stream.Collectors; +import java.util.stream.Stream; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.eclipse.edc.sql.pool.commons.CommonsConnectionPoolConfigKeys.DEPRACATED_POOL_MAX_IDLE_CONNECTIONS; +import static org.eclipse.edc.sql.pool.commons.CommonsConnectionPoolConfigKeys.DEPRACATED_POOL_MAX_TOTAL_CONNECTIONS; +import static org.eclipse.edc.sql.pool.commons.CommonsConnectionPoolConfigKeys.DEPRACATED_POOL_MIN_IDLE_CONNECTIONS; +import static org.eclipse.edc.sql.pool.commons.CommonsConnectionPoolConfigKeys.DEPRACATED_POOL_TEST_CONNECTION_ON_BORROW; +import static org.eclipse.edc.sql.pool.commons.CommonsConnectionPoolConfigKeys.DEPRACATED_POOL_TEST_CONNECTION_ON_CREATE; +import static org.eclipse.edc.sql.pool.commons.CommonsConnectionPoolConfigKeys.DEPRACATED_POOL_TEST_CONNECTION_ON_RETURN; +import static org.eclipse.edc.sql.pool.commons.CommonsConnectionPoolConfigKeys.DEPRACATED_POOL_TEST_CONNECTION_WHILE_IDLE; +import static org.eclipse.edc.sql.pool.commons.CommonsConnectionPoolConfigKeys.DEPRACATED_POOL_TEST_QUERY; +import static org.eclipse.edc.sql.pool.commons.CommonsConnectionPoolConfigKeys.POOL_CONNECTIONS_MAX_IDLE; +import static org.eclipse.edc.sql.pool.commons.CommonsConnectionPoolConfigKeys.POOL_CONNECTIONS_MAX_TOTAL; +import static org.eclipse.edc.sql.pool.commons.CommonsConnectionPoolConfigKeys.POOL_CONNECTIONS_MIN_IDLE; +import static org.eclipse.edc.sql.pool.commons.CommonsConnectionPoolConfigKeys.POOL_CONNECTION_TEST_ON_BORROW; +import static org.eclipse.edc.sql.pool.commons.CommonsConnectionPoolConfigKeys.POOL_CONNECTION_TEST_ON_CREATE; +import static org.eclipse.edc.sql.pool.commons.CommonsConnectionPoolConfigKeys.POOL_CONNECTION_TEST_ON_RETURN; +import static org.eclipse.edc.sql.pool.commons.CommonsConnectionPoolConfigKeys.POOL_CONNECTION_TEST_QUERY; +import static org.eclipse.edc.sql.pool.commons.CommonsConnectionPoolConfigKeys.POOL_CONNECTION_TEST_WHILE_IDLE; +import static org.eclipse.edc.sql.pool.commons.CommonsConnectionPoolServiceExtension.EDC_DATASOURCE_PREFIX; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; //sometimes hangs and causes the test to never finish. -@Disabled -class CommonsConnectionPoolServiceExtensionTest extends AbstractCommonsConnectionPoolServiceExtensionTest { - private static final String SQL_QUERY = "SELECT 1"; +@ExtendWith(DependencyInjectionExtension.class) +class CommonsConnectionPoolServiceExtensionTest { private static final String DS_1_NAME = "ds1"; - private static final String DS_2_NAME = "ds2"; - - private final Map systemProperties = new HashMap<>() { - { - put("edc.datasource." + DS_1_NAME + ".url", DS_1_NAME); - put("edc.datasource." + DS_1_NAME + "." + CommonsConnectionPoolConfigKeys.POOL_TEST_CONNECTION_ON_CREATE, "false"); - put("edc.datasource." + DS_1_NAME + "." + CommonsConnectionPoolConfigKeys.POOL_TEST_CONNECTION_ON_RETURN, "false"); - put("edc.datasource." + DS_1_NAME + "." + CommonsConnectionPoolConfigKeys.POOL_TEST_CONNECTION_ON_BORROW, "false"); - put("edc.datasource." + DS_1_NAME + "." + CommonsConnectionPoolConfigKeys.POOL_TEST_CONNECTION_WHILE_IDLE, "false"); - put("edc.datasource." + DS_2_NAME + ".url", DS_2_NAME); - put("edc.datasource." + DS_2_NAME + "." + CommonsConnectionPoolConfigKeys.POOL_TEST_CONNECTION_ON_CREATE, "false"); - put("edc.datasource." + DS_2_NAME + "." + CommonsConnectionPoolConfigKeys.POOL_TEST_CONNECTION_ON_RETURN, "false"); - put("edc.datasource." + DS_2_NAME + "." + CommonsConnectionPoolConfigKeys.POOL_TEST_CONNECTION_ON_BORROW, "false"); - put("edc.datasource." + DS_2_NAME + "." + CommonsConnectionPoolConfigKeys.POOL_TEST_CONNECTION_WHILE_IDLE, "false"); - } - }; - - // mocks - private Connection dataSource1Connection; - private Connection dataSource2Connection; + private final DataSourceRegistry dataSourceRegistry = mock(); @BeforeEach - void setUp() { - dataSource1Connection = Mockito.mock(Connection.class); - dataSource2Connection = Mockito.mock(Connection.class); - - systemProperties.forEach(System::setProperty); - } - - @AfterEach - void tearDown() { - systemProperties.keySet().forEach(System::clearProperty); - } - - @Test - @DisplayName("DataSource Registry contains defined DataSources") - void testDataSourceRegistryContainsDataSources() { - DataSource dataSource = getDataSourceRegistry().resolve(DS_1_NAME); - - Assertions.assertNotNull(dataSource); - Assertions.assertInstanceOf(DataSourceResource.class, dataSource); - - dataSource = getDataSourceRegistry().resolve(DS_2_NAME); - Assertions.assertNotNull(dataSource); - Assertions.assertInstanceOf(DataSourceResource.class, dataSource); + void setUp(ServiceExtensionContext context) { + context.registerService(DataSourceRegistry.class, dataSourceRegistry); } - @Test - @DisplayName("Used DataSource is scoped by the TransactionContext") - void testUsedDataSourceIsScopedByTransactionContext() throws SQLException { - PreparedStatement preparedStatementMock = Mockito.mock(PreparedStatement.class); - - try (MockedStatic driverManagerMock = Mockito.mockStatic(DriverManager.class)) { - driverManagerMock.when(() -> DriverManager.getConnection(Mockito.eq(DS_1_NAME), Mockito.any(Properties.class))).thenReturn(dataSource1Connection); - - Mockito.when(dataSource1Connection.prepareStatement(SQL_QUERY)).thenReturn(preparedStatementMock); - - getTransactionContext().execute(() -> { - DataSource dataSource = getDataSourceRegistry().resolve(DS_1_NAME); - try (Connection connection = dataSource.getConnection()) { - try (PreparedStatement statement = connection.prepareStatement(SQL_QUERY)) { - statement.execute(); - } - } catch (Exception exception) { - throw new RuntimeException(exception); - } - }); - - driverManagerMock.verify(() -> DriverManager.getConnection(Mockito.eq(DS_1_NAME), Mockito.any(Properties.class)), Mockito.times(1)); - driverManagerMock.verify(() -> DriverManager.getConnection(Mockito.eq(DS_2_NAME), Mockito.any(Properties.class)), Mockito.never()); + @ParameterizedTest + @ArgumentsSource(ConfigProvider.class) + void initialize_withConfig(Map configuration, ThrowingConsumer checker, + boolean isEnv, + CommonsConnectionPoolServiceExtension extension, ServiceExtensionContext context) { + Config config; + if (isEnv) { + config = ConfigFactory.fromEnvironment(configuration); + } else { + config = ConfigFactory.fromMap(configuration); } + when(context.getConfig(EDC_DATASOURCE_PREFIX)).thenReturn(config); - Mockito.verify(dataSource1Connection, Mockito.times(1)).setAutoCommit(false); + extension.initialize(context); - Mockito.verify(preparedStatementMock, Mockito.times(1)).execute(); + verify(dataSourceRegistry).register(eq(DS_1_NAME), any()); - Mockito.verify(preparedStatementMock, Mockito.times(1)).close(); - - Mockito.verify(dataSource1Connection, Mockito.times(1)).commit(); + assertThat(extension.getCommonsConnectionPools()).hasSize(1).first() + .extracting(CommonsConnectionPool::getPoolConfig) + .satisfies(checker); } - @Test - @DisplayName("DataSourcePool reuses issued Connection") - void testDataSourcePoolReusesConnections() throws SQLException { - PreparedStatement preparedStatementMock = Mockito.mock(PreparedStatement.class); - - final int iterations = 3; - try (MockedStatic driverManagerMock = Mockito.mockStatic(DriverManager.class)) { - driverManagerMock.when(() -> DriverManager.getConnection(Mockito.eq(DS_1_NAME), Mockito.any(Properties.class))).thenReturn(dataSource1Connection); - - Mockito.when(dataSource1Connection.prepareStatement(SQL_QUERY)).thenReturn(preparedStatementMock); - - IntStream.rangeClosed(1, iterations).forEach((iteration) -> { - getTransactionContext().execute(() -> { - DataSource dataSource = getDataSourceRegistry().resolve(DS_1_NAME); - try (Connection connection = dataSource.getConnection()) { - try (PreparedStatement statement = connection.prepareStatement(SQL_QUERY)) { - statement.execute(); - } - } catch (Exception exception) { - throw new RuntimeException(exception); - } - }); - }); - - driverManagerMock.verify(() -> DriverManager.getConnection(Mockito.eq(DS_1_NAME), Mockito.any(Properties.class)), Mockito.times(1)); - driverManagerMock.verify(() -> DriverManager.getConnection(Mockito.eq(DS_2_NAME), Mockito.any(Properties.class)), Mockito.never()); - } - - Mockito.verify(dataSource1Connection, Mockito.times(iterations)).setAutoCommit(false); - - Mockito.verify(preparedStatementMock, Mockito.times(iterations)).execute(); - - Mockito.verify(preparedStatementMock, Mockito.times(iterations)).close(); - Mockito.verify(dataSource1Connection, Mockito.times(iterations)).commit(); - } - - @Test - @DisplayName("All used DataSources are scoped by the TransactionContext") - void testAllDataSourceAreScopedByTransactionContext() throws SQLException { - PreparedStatement preparedStatementMock1 = Mockito.mock(PreparedStatement.class); - PreparedStatement preparedStatementMock2 = Mockito.mock(PreparedStatement.class); - - try (MockedStatic driverManagerMock = Mockito.mockStatic(DriverManager.class)) { - driverManagerMock.when(() -> DriverManager.getConnection(Mockito.eq(DS_1_NAME), Mockito.any(Properties.class))).thenReturn(dataSource1Connection); - driverManagerMock.when(() -> DriverManager.getConnection(Mockito.eq(DS_2_NAME), Mockito.any(Properties.class))).thenReturn(dataSource2Connection); - - Mockito.when(dataSource1Connection.prepareStatement(SQL_QUERY)).thenReturn(preparedStatementMock1); - Mockito.when(dataSource2Connection.prepareStatement(SQL_QUERY)).thenReturn(preparedStatementMock2); - - getTransactionContext().execute(() -> { - DataSource dataSource1 = getDataSourceRegistry().resolve(DS_1_NAME); - try (Connection connection = dataSource1.getConnection()) { - try (PreparedStatement statement = connection.prepareStatement(SQL_QUERY)) { - statement.execute(); - } - } catch (Exception exception) { - throw new RuntimeException(exception); - } - DataSource dataSource2 = getDataSourceRegistry().resolve(DS_2_NAME); - try (Connection connection = dataSource2.getConnection()) { - try (PreparedStatement statement = connection.prepareStatement(SQL_QUERY)) { - statement.execute(); - } - } catch (Exception exception) { - throw new RuntimeException(exception); - } - }); - - driverManagerMock.verify(() -> DriverManager.getConnection(Mockito.eq(DS_1_NAME), Mockito.any(Properties.class)), Mockito.times(1)); - driverManagerMock.verify(() -> DriverManager.getConnection(Mockito.eq(DS_2_NAME), Mockito.any(Properties.class)), Mockito.times(1)); + static class ConfigProvider implements ArgumentsProvider { + + private final Map defaultConfig = Map.of(DS_1_NAME + ".url", DS_1_NAME); + + private final Map 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"); + + + private final Map deprecatedConfig = Map.of( + DS_1_NAME + ".url", DS_1_NAME, + DS_1_NAME + "." + DEPRACATED_POOL_TEST_CONNECTION_ON_CREATE, "false", + DS_1_NAME + "." + DEPRACATED_POOL_TEST_CONNECTION_ON_BORROW, "false", + DS_1_NAME + "." + DEPRACATED_POOL_TEST_CONNECTION_ON_RETURN, "true", + DS_1_NAME + "." + DEPRACATED_POOL_TEST_CONNECTION_WHILE_IDLE, "true", + DS_1_NAME + "." + DEPRACATED_POOL_TEST_QUERY, "SELECT foo FROM bar;", + DS_1_NAME + "." + DEPRACATED_POOL_MIN_IDLE_CONNECTIONS, "10", + DS_1_NAME + "." + DEPRACATED_POOL_MAX_IDLE_CONNECTIONS, "10", + DS_1_NAME + "." + DEPRACATED_POOL_MAX_TOTAL_CONNECTIONS, "10"); + + @Override + public Stream provideArguments(ExtensionContext context) { + ThrowingConsumer checkDefault = this::checkDefault; + ThrowingConsumer checkWithConfig = this::checkWithConfig; + + var envConfiguration = configuration.entrySet().stream() + .map(it -> Map.entry(it.getKey().toUpperCase().replace(".", "_"), it.getValue())) + .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)); + return Stream.of( + Arguments.of(defaultConfig, checkDefault, false), + Arguments.of(configuration, checkWithConfig, false), + Arguments.of(envConfiguration, checkWithConfig, true), + Arguments.of(deprecatedConfig, checkWithConfig, false) + ); } - Mockito.verify(dataSource1Connection, Mockito.times(1)).setAutoCommit(false); - Mockito.verify(dataSource2Connection, Mockito.times(1)).setAutoCommit(false); + private void checkDefault(CommonsConnectionPoolConfig cfg) { + assertThat(cfg.getTestConnectionOnCreate()).isTrue(); + assertThat(cfg.getTestConnectionOnBorrow()).isTrue(); + assertThat(cfg.getTestConnectionOnReturn()).isFalse(); + assertThat(cfg.getTestConnectionWhileIdle()).isFalse(); + assertThat(cfg.getTestQuery()).isEqualTo("SELECT 1;"); + assertThat(cfg.getMinIdleConnections()).isEqualTo(1); + assertThat(cfg.getMaxIdleConnections()).isEqualTo(4); + assertThat(cfg.getMaxTotalConnections()).isEqualTo(8); + } - Mockito.verify(preparedStatementMock1, Mockito.times(1)).execute(); - Mockito.verify(preparedStatementMock1, Mockito.times(1)).close(); - Mockito.verify(preparedStatementMock2, Mockito.times(1)).execute(); - Mockito.verify(preparedStatementMock2, Mockito.times(1)).close(); + private void checkWithConfig(CommonsConnectionPoolConfig cfg) { + assertThat(cfg.getTestConnectionOnCreate()).isFalse(); + assertThat(cfg.getTestConnectionOnBorrow()).isFalse(); + assertThat(cfg.getTestConnectionOnReturn()).isTrue(); + assertThat(cfg.getTestConnectionWhileIdle()).isTrue(); + assertThat(cfg.getTestQuery()).isEqualTo("SELECT foo FROM bar;"); + assertThat(cfg.getMinIdleConnections()).isEqualTo(10); + assertThat(cfg.getMaxIdleConnections()).isEqualTo(10); + assertThat(cfg.getMaxTotalConnections()).isEqualTo(10); + } - Mockito.verify(dataSource1Connection, Mockito.times(1)).commit(); - Mockito.verify(dataSource2Connection, Mockito.times(1)).commit(); } + } diff --git a/extensions/common/sql/sql-pool/sql-pool-apache-commons/src/test/java/org/eclipse/edc/sql/pool/commons/CommonsConnectionPoolTest.java b/extensions/common/sql/sql-pool/sql-pool-apache-commons/src/test/java/org/eclipse/edc/sql/pool/commons/CommonsConnectionPoolTest.java index c3b7da8eae7..dd9ccd8d6a4 100644 --- a/extensions/common/sql/sql-pool/sql-pool-apache-commons/src/test/java/org/eclipse/edc/sql/pool/commons/CommonsConnectionPoolTest.java +++ b/extensions/common/sql/sql-pool/sql-pool-apache-commons/src/test/java/org/eclipse/edc/sql/pool/commons/CommonsConnectionPoolTest.java @@ -14,175 +14,194 @@ package org.eclipse.edc.sql.pool.commons; +import org.eclipse.edc.spi.monitor.Monitor; import org.eclipse.edc.spi.persistence.EdcPersistenceException; -import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; -import org.mockito.Mockito; import java.sql.Connection; import java.sql.PreparedStatement; import java.sql.SQLException; import javax.sql.DataSource; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.Mockito.anyString; +import static org.mockito.Mockito.atLeastOnce; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + class CommonsConnectionPoolTest { + private final Monitor monitor = mock(); + @Test void getConnection() throws SQLException { - Connection connection = Mockito.mock(Connection.class); - PreparedStatement testQueryPreparedStatement = Mockito.mock(PreparedStatement.class); - DataSource dataSource = Mockito.mock(DataSource.class); - CommonsConnectionPoolConfig commonsConnectionPoolConfig = CommonsConnectionPoolConfig.Builder.newInstance().build(); - CommonsConnectionPool connectionPool = new CommonsConnectionPool(dataSource, commonsConnectionPoolConfig); + var connection = mock(Connection.class); + var testQueryPreparedStatement = mock(PreparedStatement.class); + var dataSource = mock(DataSource.class); + var commonsConnectionPoolConfig = CommonsConnectionPoolConfig.Builder.newInstance().build(); + var connectionPool = new CommonsConnectionPool(dataSource, commonsConnectionPoolConfig, monitor); - Mockito.when(testQueryPreparedStatement.execute()).thenReturn(true); - Mockito.when(connection.prepareStatement(Mockito.anyString())).thenReturn(testQueryPreparedStatement); - Mockito.when(dataSource.getConnection()).thenReturn(connection); + when(testQueryPreparedStatement.execute()).thenReturn(true); + when(connection.prepareStatement(anyString())).thenReturn(testQueryPreparedStatement); + when(dataSource.getConnection()).thenReturn(connection); - Connection result = connectionPool.getConnection(); + var result = connectionPool.getConnection(); - Assertions.assertNotNull(connection); - Assertions.assertEquals(connection, result); + assertNotNull(connection); + assertEquals(connection, result); - Mockito.verify(dataSource, Mockito.atLeastOnce()).getConnection(); - Mockito.verify(connection, Mockito.atLeastOnce()).isClosed(); - Mockito.verify(connection, Mockito.atLeastOnce()).prepareStatement(Mockito.anyString()); - Mockito.verify(testQueryPreparedStatement, Mockito.atLeastOnce()).execute(); + verify(dataSource, atLeastOnce()).getConnection(); + verify(connection, atLeastOnce()).isClosed(); + verify(connection, atLeastOnce()).prepareStatement(anyString()); + verify(testQueryPreparedStatement, atLeastOnce()).execute(); } @Test void getConnectionAnyExceptionThrownThrowsSqlException() throws SQLException { - DataSource dataSource = Mockito.mock(DataSource.class); - CommonsConnectionPoolConfig commonsConnectionPoolConfig = CommonsConnectionPoolConfig.Builder.newInstance().build(); - CommonsConnectionPool connectionPool = new CommonsConnectionPool(dataSource, commonsConnectionPoolConfig); - RuntimeException causingRuntimeException = new RuntimeException("intended to be thrown"); + var dataSource = mock(DataSource.class); + var commonsConnectionPoolConfig = CommonsConnectionPoolConfig.Builder.newInstance().build(); + var connectionPool = new CommonsConnectionPool(dataSource, commonsConnectionPoolConfig, monitor); + var causingRuntimeException = new RuntimeException("intended to be thrown"); - Mockito.when(dataSource.getConnection()).thenThrow(causingRuntimeException); + when(dataSource.getConnection()).thenThrow(causingRuntimeException); - EdcPersistenceException exceptionWrappingRuntimeException = Assertions.assertThrows(EdcPersistenceException.class, connectionPool::getConnection); + var exceptionWrappingRuntimeException = assertThrows(EdcPersistenceException.class, connectionPool::getConnection); - Assertions.assertNotNull(exceptionWrappingRuntimeException.getCause()); - Assertions.assertEquals(causingRuntimeException, exceptionWrappingRuntimeException.getCause()); + assertNotNull(exceptionWrappingRuntimeException.getCause()); + assertEquals(causingRuntimeException, exceptionWrappingRuntimeException.getCause()); - Mockito.verify(dataSource, Mockito.atLeastOnce()).getConnection(); + verify(dataSource, atLeastOnce()).getConnection(); } @Test void getConnectionSqlExceptionThrownThrowsSame() throws SQLException { - DataSource dataSource = Mockito.mock(DataSource.class); - CommonsConnectionPoolConfig commonsConnectionPoolConfig = CommonsConnectionPoolConfig.Builder.newInstance().build(); - CommonsConnectionPool connectionPool = new CommonsConnectionPool(dataSource, commonsConnectionPoolConfig); - SQLException causingSqlException = new SQLException("intended to be thrown"); + var dataSource = mock(DataSource.class); + var commonsConnectionPoolConfig = CommonsConnectionPoolConfig.Builder.newInstance().build(); + var connectionPool = new CommonsConnectionPool(dataSource, commonsConnectionPoolConfig, monitor); + var causingSqlException = new SQLException("intended to be thrown"); - Mockito.when(dataSource.getConnection()).thenThrow(causingSqlException); + when(dataSource.getConnection()).thenThrow(causingSqlException); - EdcPersistenceException sqlException = Assertions.assertThrows(EdcPersistenceException.class, connectionPool::getConnection); + var sqlException = assertThrows(EdcPersistenceException.class, connectionPool::getConnection); - Assertions.assertNotNull(sqlException.getCause()); - Assertions.assertEquals(causingSqlException, sqlException.getCause()); + assertNotNull(sqlException.getCause()); + assertEquals(causingSqlException, sqlException.getCause()); - Mockito.verify(dataSource, Mockito.atLeastOnce()).getConnection(); + verify(dataSource, atLeastOnce()).getConnection(); } @Test void returnConnectionNullThrowsNullPointerException() { - DataSource dataSource = Mockito.mock(DataSource.class); - CommonsConnectionPoolConfig commonsConnectionPoolConfig = CommonsConnectionPoolConfig.Builder.newInstance().build(); - CommonsConnectionPool connectionPool = new CommonsConnectionPool(dataSource, commonsConnectionPoolConfig); + var dataSource = mock(DataSource.class); + var commonsConnectionPoolConfig = CommonsConnectionPoolConfig.Builder.newInstance().build(); + var connectionPool = new CommonsConnectionPool(dataSource, commonsConnectionPoolConfig, monitor); - Assertions.assertThrows(NullPointerException.class, () -> connectionPool.returnConnection(null)); + assertThrows(NullPointerException.class, () -> connectionPool.returnConnection(null)); } @Test void returnConnectionUnknownThrowsIllegalStateException() { - DataSource dataSource = Mockito.mock(DataSource.class); - CommonsConnectionPoolConfig commonsConnectionPoolConfig = CommonsConnectionPoolConfig.Builder.newInstance().build(); - CommonsConnectionPool connectionPool = new CommonsConnectionPool(dataSource, commonsConnectionPoolConfig); + var dataSource = mock(DataSource.class); + var commonsConnectionPoolConfig = CommonsConnectionPoolConfig.Builder.newInstance().build(); + var connectionPool = new CommonsConnectionPool(dataSource, commonsConnectionPoolConfig, monitor); // a connection unmanaged by the pool - Connection connection = Mockito.mock(Connection.class); + var connection = mock(Connection.class); - Assertions.assertThrows(IllegalStateException.class, () -> connectionPool.returnConnection(connection)); + assertThrows(IllegalStateException.class, () -> connectionPool.returnConnection(connection)); } @Test void returnConnection() throws SQLException { - Connection connection = Mockito.mock(Connection.class); - PreparedStatement testQueryPreparedStatement = Mockito.mock(PreparedStatement.class); - DataSource dataSource = Mockito.mock(DataSource.class); - CommonsConnectionPoolConfig commonsConnectionPoolConfig = CommonsConnectionPoolConfig.Builder.newInstance().build(); - CommonsConnectionPool connectionPool = new CommonsConnectionPool(dataSource, commonsConnectionPoolConfig); + var connection = mock(Connection.class); + PreparedStatement testQueryPreparedStatement = mock(PreparedStatement.class); + var dataSource = mock(DataSource.class); + var commonsConnectionPoolConfig = CommonsConnectionPoolConfig.Builder.newInstance().build(); + var connectionPool = new CommonsConnectionPool(dataSource, commonsConnectionPoolConfig, monitor); - Mockito.when(testQueryPreparedStatement.execute()).thenReturn(true); - Mockito.when(connection.prepareStatement(Mockito.anyString())).thenReturn(testQueryPreparedStatement); - Mockito.when(dataSource.getConnection()).thenReturn(connection); + when(testQueryPreparedStatement.execute()).thenReturn(true); + when(connection.prepareStatement(anyString())).thenReturn(testQueryPreparedStatement); + when(dataSource.getConnection()).thenReturn(connection); - Connection result = connectionPool.getConnection(); + var result = connectionPool.getConnection(); - Assertions.assertNotNull(connection); - Assertions.assertEquals(connection, result); + assertNotNull(connection); + assertEquals(connection, result); connectionPool.returnConnection(result); - Mockito.verify(dataSource, Mockito.atLeastOnce()).getConnection(); - Mockito.verify(connection, Mockito.atLeastOnce()).isClosed(); - Mockito.verify(connection, Mockito.atLeastOnce()).prepareStatement(Mockito.anyString()); - Mockito.verify(testQueryPreparedStatement, Mockito.atLeastOnce()).execute(); - } + verify(dataSource, atLeastOnce()).getConnection(); + verify(connection, atLeastOnce()).isClosed(); + verify(connection, atLeastOnce()).prepareStatement(anyString()); + verify(testQueryPreparedStatement, atLeastOnce()).execute(); + verify(connection, atLeastOnce()).rollback(); + } + @Test - void returnConnectionProperlyClosed() throws SQLException { - Connection connection = Mockito.mock(Connection.class); - PreparedStatement testQueryPreparedStatement = Mockito.mock(PreparedStatement.class); - Mockito.when(testQueryPreparedStatement.execute()).thenReturn(false); - Mockito.when(connection.prepareStatement(Mockito.anyString())).thenReturn(testQueryPreparedStatement); - DataSource dataSource = Mockito.mock(DataSource.class); - Mockito.when(dataSource.getConnection()).thenReturn(connection); - CommonsConnectionPoolConfig commonsConnectionPoolConfig = CommonsConnectionPoolConfig.Builder.newInstance() + void returnConnection_shouldInvalidateConnection_rollbackFailure() throws SQLException { + var connection = mock(Connection.class); + var testQueryPreparedStatement = mock(PreparedStatement.class); + when(testQueryPreparedStatement.execute()).thenReturn(true); + when(connection.prepareStatement(anyString())).thenReturn(testQueryPreparedStatement); + var dataSource = mock(DataSource.class); + when(dataSource.getConnection()).thenReturn(connection); + var commonsConnectionPoolConfig = CommonsConnectionPoolConfig.Builder.newInstance() .testConnectionOnCreate(false) .testConnectionOnBorrow(false) + .testConnectionOnReturn(true) .build(); - CommonsConnectionPool connectionPool = new CommonsConnectionPool(dataSource, commonsConnectionPoolConfig); + var connectionPool = new CommonsConnectionPool(dataSource, commonsConnectionPoolConfig, monitor); - Connection result = connectionPool.getConnection(); + var result = connectionPool.getConnection(); - Assertions.assertNotNull(connection); - Assertions.assertEquals(connection, result); + assertNotNull(connection); + assertEquals(connection, result); - Mockito.when(connection.isClosed()).thenReturn(false); + when(connection.isClosed()).thenReturn(false); + + doThrow(new SQLException()).when(connection).rollback(); connectionPool.returnConnection(connection); - Mockito.verify(dataSource, Mockito.atLeastOnce()).getConnection(); - Mockito.verify(connection, Mockito.atLeastOnce()).isClosed(); - Mockito.verify(connection, Mockito.atLeastOnce()).prepareStatement(Mockito.anyString()); - Mockito.verify(testQueryPreparedStatement, Mockito.atLeastOnce()).execute(); - Mockito.verify(connection, Mockito.atLeastOnce()).close(); + verify(dataSource, atLeastOnce()).getConnection(); + verify(connection, atLeastOnce()).isClosed(); + verify(connection, atLeastOnce()).prepareStatement(anyString()); + verify(testQueryPreparedStatement, atLeastOnce()).execute(); + verify(connection, atLeastOnce()).close(); + + verify(connection).rollback(); + } @Test void closeProperlyClosesManagedConnections() throws SQLException { - Connection connection = Mockito.mock(Connection.class); - PreparedStatement testQueryPreparedStatement = Mockito.mock(PreparedStatement.class); - Mockito.when(testQueryPreparedStatement.execute()).thenReturn(true); - Mockito.when(connection.prepareStatement(Mockito.anyString())).thenReturn(testQueryPreparedStatement); - DataSource dataSource = Mockito.mock(DataSource.class); - Mockito.when(dataSource.getConnection()).thenReturn(connection); - CommonsConnectionPoolConfig commonsConnectionPoolConfig = CommonsConnectionPoolConfig.Builder.newInstance().build(); - CommonsConnectionPool connectionPool = new CommonsConnectionPool(dataSource, commonsConnectionPoolConfig); + var connection = mock(Connection.class); + var testQueryPreparedStatement = mock(PreparedStatement.class); + when(testQueryPreparedStatement.execute()).thenReturn(true); + when(connection.prepareStatement(anyString())).thenReturn(testQueryPreparedStatement); + var dataSource = mock(DataSource.class); + when(dataSource.getConnection()).thenReturn(connection); + var commonsConnectionPoolConfig = CommonsConnectionPoolConfig.Builder.newInstance().build(); + var connectionPool = new CommonsConnectionPool(dataSource, commonsConnectionPoolConfig, monitor); - Connection result = connectionPool.getConnection(); + var result = connectionPool.getConnection(); - Assertions.assertNotNull(connection); - Assertions.assertEquals(connection, result); + assertNotNull(connection); + assertEquals(connection, result); connectionPool.returnConnection(connection); connectionPool.close(); - Mockito.verify(dataSource, Mockito.atLeastOnce()).getConnection(); - Mockito.verify(connection, Mockito.atLeastOnce()).isClosed(); - Mockito.verify(connection, Mockito.atLeastOnce()).prepareStatement(Mockito.anyString()); - Mockito.verify(testQueryPreparedStatement, Mockito.atLeastOnce()).execute(); - Mockito.verify(connection, Mockito.atLeastOnce()).close(); + verify(dataSource, atLeastOnce()).getConnection(); + verify(connection, atLeastOnce()).isClosed(); + verify(connection, atLeastOnce()).prepareStatement(anyString()); + verify(testQueryPreparedStatement, atLeastOnce()).execute(); + verify(connection, atLeastOnce()).close(); } } diff --git a/extensions/common/sql/sql-pool/sql-pool-apache-commons/src/test/java/org/eclipse/edc/sql/pool/commons/DriverManagerConnectionFactoryTest.java b/extensions/common/sql/sql-pool/sql-pool-apache-commons/src/test/java/org/eclipse/edc/sql/pool/commons/DriverManagerConnectionFactoryTest.java new file mode 100644 index 00000000000..f963a6d4d04 --- /dev/null +++ b/extensions/common/sql/sql-pool/sql-pool-apache-commons/src/test/java/org/eclipse/edc/sql/pool/commons/DriverManagerConnectionFactoryTest.java @@ -0,0 +1,61 @@ +/* + * Copyright (c) 2023 Bayerische Motoren Werke Aktiengesellschaft (BMW AG) + * + * This program and the accompanying materials are made available under the + * terms of the Apache License, Version 2.0 which is available at + * https://www.apache.org/licenses/LICENSE-2.0 + * + * SPDX-License-Identifier: Apache-2.0 + * + * Contributors: + * Bayerische Motoren Werke Aktiengesellschaft (BMW AG) - initial API and implementation + * + */ + +package org.eclipse.edc.sql.pool.commons; + +import org.eclipse.edc.spi.persistence.EdcPersistenceException; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import java.sql.Connection; +import java.sql.DriverManager; +import java.sql.SQLException; +import java.util.Properties; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.Mockito.any; +import static org.mockito.Mockito.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.mockStatic; + +public class DriverManagerConnectionFactoryTest { + private static final String DS_NAME = "datasource"; + private final Connection connection = mock(); + private DriverManagerConnectionFactory factory; + + @BeforeEach + void setup() { + factory = new DriverManagerConnectionFactory(DS_NAME, new Properties()); + } + + @Test + void create() throws SQLException { + try (var driverManagerMock = mockStatic(DriverManager.class)) { + driverManagerMock.when(() -> DriverManager.getConnection(eq(DS_NAME), any(Properties.class))).thenReturn(connection); + try (var conn = factory.create()) { + assertThat(conn).isEqualTo(connection); + } + } + } + + @Test + void create_shouldThrowException() { + try (var driverManagerMock = mockStatic(DriverManager.class)) { + driverManagerMock.when(() -> DriverManager.getConnection(eq(DS_NAME), any(Properties.class))) + .thenThrow(SQLException.class); + assertThatThrownBy(() -> factory.create()).isInstanceOf(EdcPersistenceException.class); + } + } +}