This is an automated email from the ASF dual-hosted git repository. cbrisson pushed a commit to branch VELOCITY-965 in repository https://gitbox.apache.org/repos/asf/velocity-engine.git
commit 5d87ee8f88fea6a7c7ffe24872d5972d6fb8c55b Author: Claude Brisson <[email protected]> AuthorDate: Wed Aug 28 19:58:34 2024 +0200 Various code cleanup to DataSourceResourceLoader --- .../loader/CachingDatabaseObjectsFactory.java | 128 ++++++++++++++------- .../resource/loader/DataSourceResourceLoader.java | 10 +- .../resource/loader/DatabaseObjectsFactory.java | 9 +- .../loader/DefaultDatabaseObjectsFactory.java | 10 +- 4 files changed, 105 insertions(+), 52 deletions(-) diff --git a/velocity-engine-core/src/main/java/org/apache/velocity/runtime/resource/loader/CachingDatabaseObjectsFactory.java b/velocity-engine-core/src/main/java/org/apache/velocity/runtime/resource/loader/CachingDatabaseObjectsFactory.java index 86a87a40..aadc219e 100644 --- a/velocity-engine-core/src/main/java/org/apache/velocity/runtime/resource/loader/CachingDatabaseObjectsFactory.java +++ b/velocity-engine-core/src/main/java/org/apache/velocity/runtime/resource/loader/CachingDatabaseObjectsFactory.java @@ -1,19 +1,18 @@ package org.apache.velocity.runtime. resource.loader; -import org.apache.commons.pool2.BasePooledObjectFactory; +import org.apache.commons.pool2.BaseKeyedPooledObjectFactory; +import org.apache.commons.pool2.KeyedObjectPool; import org.apache.commons.pool2.PooledObject; import org.apache.commons.pool2.impl.DefaultPooledObject; -import org.apache.commons.pool2.impl.GenericObjectPool; -import org.apache.commons.pool2.impl.GenericObjectPoolConfig; +import org.apache.commons.pool2.impl.GenericKeyedObjectPool; +import org.apache.commons.pool2.impl.GenericKeyedObjectPoolConfig; import org.apache.velocity.util.ExtProperties; +import org.slf4j.Logger; import javax.sql.DataSource; import java.sql.Connection; import java.sql.PreparedStatement; import java.sql.SQLException; -import java.util.HashMap; -import java.util.Map; -import java.util.Optional; /** * <p>Database objects factory which will keep a single connection to be able to cache statements preparation, by means @@ -24,6 +23,7 @@ import java.util.Optional; * <groupId>org.apache.commons</groupId> * <artifactId>commons-pool2</artifactId> * <version>2.12.0</version> + * <scope>runtime</scope> * </dependency> * </code></pre> * <p>To use this class, you must add the following property to the example configuration described in @@ -48,19 +48,13 @@ public class CachingDatabaseObjectsFactory implements DatabaseObjectsFactory { private DataSource dataSource; private Connection connection; private int poolsMaxSize; - private Map<String, GenericObjectPool<PreparedStatement>> statementsCache = new HashMap<>(); + private KeyedObjectPool<String, PreparedStatement> statementsPool; + protected Logger log = null; - private class PreparedStatementFactory extends BasePooledObjectFactory<PreparedStatement> + private class PreparedStatementFactory extends BaseKeyedPooledObjectFactory<String, PreparedStatement> { - private final String sql; - - PreparedStatementFactory(String sql) - { - this.sql = sql; - } - @Override - public PreparedStatement create() throws Exception { + public PreparedStatement create(String sql) throws Exception { checkConnection(); return connection.prepareStatement(sql); } @@ -71,7 +65,20 @@ public class CachingDatabaseObjectsFactory implements DatabaseObjectsFactory { } @Override - public void destroyObject(final PooledObject<PreparedStatement> p) throws Exception { + public boolean validateObject(String key, PooledObject<PreparedStatement> obj) + { + try + { + return !obj.getObject().isClosed() && obj.getObject().getConnection().isValid(0); + } + catch (SQLException sqle) + { + return false; + } + } + + @Override + public void destroyObject(String key, PooledObject<PreparedStatement> p) throws Exception { p.getObject().close(); } } @@ -85,7 +92,22 @@ public class CachingDatabaseObjectsFactory implements DatabaseObjectsFactory { { this.dataSource = dataSource; this.connection = dataSource.getConnection(); - this.poolsMaxSize = Optional.ofNullable(properties.getInt(STATEMENTS_POOL_MAX_SIZE)).orElse(STATEMENTS_POOL_MAX_SIZE_DEFAULT); + this.poolsMaxSize = properties.getInt(STATEMENTS_POOL_MAX_SIZE, STATEMENTS_POOL_MAX_SIZE_DEFAULT); + createStatementsPool(); + } + + private void createStatementsPool() + { + GenericKeyedObjectPoolConfig<PreparedStatement> poolConfig = new GenericKeyedObjectPoolConfig<>(); + poolConfig.setMaxTotal(poolsMaxSize); + poolConfig.setTestOnBorrow(true); + statementsPool = new GenericKeyedObjectPool<>(new PreparedStatementFactory(), poolConfig); + } + + @Override + public void setLogger(Logger log) + { + this.log = log; } /** @@ -94,19 +116,11 @@ public class CachingDatabaseObjectsFactory implements DatabaseObjectsFactory { * @return prepared statement */ @Override - public synchronized PreparedStatement prepareStatement(String sql) throws SQLException + public PreparedStatement prepareStatement(String sql) throws SQLException { - GenericObjectPool<PreparedStatement> pool = statementsCache.computeIfAbsent(sql, (String key) -> { - GenericObjectPoolConfig<PreparedStatement> poolConfig = new GenericObjectPoolConfig<>(); - poolConfig.setMaxTotal(poolsMaxSize); - return new GenericObjectPool<>( - new PreparedStatementFactory(sql), - poolConfig - ); - }); try { - return pool.borrowObject(); + return statementsPool.borrowObject(sql); } catch (SQLException sqle) { @@ -118,16 +132,22 @@ public class CachingDatabaseObjectsFactory implements DatabaseObjectsFactory { } } - private void checkConnection() throws SQLException + protected synchronized void checkConnection() throws SQLException { if (!connection.isValid(0)) { - // refresh connection - connection = dataSource.getConnection(); - statementsCache = new HashMap<>(); + reset(); } } + private void reset() throws SQLException + { + clear(); + // refresh connection and statements pool + connection = dataSource.getConnection(); + createStatementsPool(); + } + /** * Releases a prepared statement * @param sql original sql query @@ -136,26 +156,48 @@ public class CachingDatabaseObjectsFactory implements DatabaseObjectsFactory { @Override public void releaseStatement(String sql, PreparedStatement stmt) throws SQLException { - GenericObjectPool<PreparedStatement> pool = statementsCache.get(sql); - if (pool == null) + try { - throw new SQLException("statement is not pooled"); + statementsPool.returnObject(sql, stmt); + } + catch (Exception e) + { + if (log != null) + { + log.debug("could not return statement to the pool", e); + } } - pool.returnObject(stmt); } + /** + * Free resources + */ @Override - public void destroy() + public void clear() { - statementsCache.values().forEach(pool -> + statementsPool.close(); + try { - pool.close(); - pool.clear(); - }); + statementsPool.clear(); + } + catch (Exception e) + { + if (log != null) + { + log.debug("statements pool clearing gave an exception", e); + } + } try { connection.close(); } - catch (SQLException sqle) {} - }; + catch (SQLException sqle) + { + log.debug("connection closing gave an exception", sqle); + } + finally + { + connection = null; + } + } } diff --git a/velocity-engine-core/src/main/java/org/apache/velocity/runtime/resource/loader/DataSourceResourceLoader.java b/velocity-engine-core/src/main/java/org/apache/velocity/runtime/resource/loader/DataSourceResourceLoader.java index 85ca3a1e..cd08d59c 100644 --- a/velocity-engine-core/src/main/java/org/apache/velocity/runtime/resource/loader/DataSourceResourceLoader.java +++ b/velocity-engine-core/src/main/java/org/apache/velocity/runtime/resource/loader/DataSourceResourceLoader.java @@ -19,7 +19,6 @@ package org.apache.velocity.runtime.resource.loader; * under the License. */ -import org.apache.commons.pool2.ObjectPool; import org.apache.velocity.exception.ResourceNotFoundException; import org.apache.velocity.exception.VelocityException; import org.apache.velocity.runtime.resource.Resource; @@ -152,8 +151,7 @@ public class DataSourceResourceLoader extends ResourceLoader private class SelfCleaningReader extends FilterReader { - private ResultSet resultSet; - private ObjectPool<PreparedStatement> statementPool; + private final ResultSet resultSet; public SelfCleaningReader(Reader reader, ResultSet resultSet) { @@ -393,7 +391,7 @@ public class DataSourceResourceLoader extends ResourceLoader catch (SQLException sqle) { // just log, don't throw - log.error("DataSourceResourceLoader: error releasing prepared statement", sqle); + log.debug("DataSourceResourceLoader: error releasing prepared statement", sqle); } } } @@ -472,11 +470,11 @@ public class DataSourceResourceLoader extends ResourceLoader /** * Frees all resources. */ - public void destroy() + public void clear() { if (factory != null) { - factory.destroy(); + factory.clear(); } } diff --git a/velocity-engine-core/src/main/java/org/apache/velocity/runtime/resource/loader/DatabaseObjectsFactory.java b/velocity-engine-core/src/main/java/org/apache/velocity/runtime/resource/loader/DatabaseObjectsFactory.java index 34c9d8dc..39eac7e7 100644 --- a/velocity-engine-core/src/main/java/org/apache/velocity/runtime/resource/loader/DatabaseObjectsFactory.java +++ b/velocity-engine-core/src/main/java/org/apache/velocity/runtime/resource/loader/DatabaseObjectsFactory.java @@ -1,6 +1,7 @@ package org.apache.velocity.runtime.resource.loader; import org.apache.velocity.util.ExtProperties; +import org.slf4j.Logger; import javax.sql.DataSource; import java.sql.PreparedStatement; @@ -18,6 +19,12 @@ public interface DatabaseObjectsFactory { */ void init(DataSource dataSource, ExtProperties properties) throws SQLException; + /** + * Set the logger to be used by the factory + * @param log + */ + default void setLogger(Logger log) {} + /** * Prepare a statement * @param sql Statement SQL @@ -35,5 +42,5 @@ public interface DatabaseObjectsFactory { /** * Free resources */ - default void destroy() {}; + default void clear() {}; } diff --git a/velocity-engine-core/src/main/java/org/apache/velocity/runtime/resource/loader/DefaultDatabaseObjectsFactory.java b/velocity-engine-core/src/main/java/org/apache/velocity/runtime/resource/loader/DefaultDatabaseObjectsFactory.java index ed52d5a2..6edc6124 100644 --- a/velocity-engine-core/src/main/java/org/apache/velocity/runtime/resource/loader/DefaultDatabaseObjectsFactory.java +++ b/velocity-engine-core/src/main/java/org/apache/velocity/runtime/resource/loader/DefaultDatabaseObjectsFactory.java @@ -47,7 +47,13 @@ public class DefaultDatabaseObjectsFactory implements DatabaseObjectsFactory { public void releaseStatement(String sql, PreparedStatement stmt) throws SQLException { Connection connection = stmt.getConnection(); - stmt.close(); - connection.close(); + try + { + stmt.close(); + } + finally + { + connection.close(); + } } }
