diff --git a/velocity-engine-core/pom.xml b/velocity-engine-core/pom.xml index 27e705910..8f57641d9 100644 --- a/velocity-engine-core/pom.xml +++ b/velocity-engine-core/pom.xml @@ -251,6 +251,24 @@ test ${test.jdbc.driver.classifier} + + org.apache.commons + commons-dbcp2 + 2.13.0 + test + + + org.apache.tomcat + tomcat-jdbc + 9.0.104 + test + + + com.zaxxer + HikariCP + 4.0.3 + test + org.slf4j slf4j-simple 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 21e1a1e0a..b7b75963d 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 @@ -165,8 +165,14 @@ public void close() throws IOException super.close(); try { - resultSet.close(); - factory.releaseStatement(templateSQL, (PreparedStatement)resultSet.getStatement()); + PreparedStatement statement = (PreparedStatement) resultSet.getStatement(); + try { + if (!resultSet.isClosed()) { + resultSet.close(); + } + } finally { + factory.releaseStatement(templateSQL, statement); + } } catch (RuntimeException re) { @@ -295,7 +301,7 @@ public synchronized Reader getResourceReader(final String name, String encoding) throw new ResourceNotFoundException("DataSourceResourceLoader: Template name was empty or null"); } - ResultSet rs = null; + ResultSet rs; try { PreparedStatement statement = factory.prepareStatement(templateSQL); @@ -314,6 +320,11 @@ public synchronized Reader getResourceReader(final String name, String encoding) } else { + try { + closeResultSet(rs); + } finally { + factory.releaseStatement(templateSQL, statement); + } throw new ResourceNotFoundException("DataSourceResourceLoader: " + "could not find resource '" + name + "'"); @@ -408,7 +419,9 @@ private void closeResultSet(final ResultSet rs) { try { - rs.close(); + if (!rs.isClosed()) { + rs.close(); + } } catch (RuntimeException re) { 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 6edc61245..91384a028 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 @@ -35,7 +35,7 @@ public void init(DataSource dataSource, ExtProperties properties) public PreparedStatement prepareStatement(String sql) throws SQLException { Connection connection = dataSource.getConnection(); - return connection.prepareStatement(sql); + return PreparedStatementProxy.newInstance(connection.prepareStatement(sql), connection); } /** @@ -49,11 +49,15 @@ public void releaseStatement(String sql, PreparedStatement stmt) throws SQLExcep Connection connection = stmt.getConnection(); try { - stmt.close(); + if (!stmt.isClosed()) { + stmt.close(); + } } finally { - connection.close(); + if (!connection.isClosed()) { + connection.close(); + } } } } diff --git a/velocity-engine-core/src/main/java/org/apache/velocity/runtime/resource/loader/PreparedStatementProxy.java b/velocity-engine-core/src/main/java/org/apache/velocity/runtime/resource/loader/PreparedStatementProxy.java new file mode 100644 index 000000000..8c114431f --- /dev/null +++ b/velocity-engine-core/src/main/java/org/apache/velocity/runtime/resource/loader/PreparedStatementProxy.java @@ -0,0 +1,69 @@ +package org.apache.velocity.runtime.resource.loader; +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import java.lang.reflect.InvocationHandler; +import java.lang.reflect.Method; +import java.lang.reflect.Proxy; +import java.sql.Connection; +import java.sql.PreparedStatement; +import java.sql.ResultSet; +import java.sql.SQLException; + +/** + * Proxy for {@link java.sql.PreparedStatement} that guarantees getConnection will always return the DataSource + * provided wrapped {@link java.sql.Connection} and not the underlying database specific connection. + * Also overrides executeQuery to return a proxy for {@link java.sql.ResultSet} see {@link ResultSetProxy} + */ +public class PreparedStatementProxy implements InvocationHandler { + private final PreparedStatement wrappedPreparedStatement; + private PreparedStatement proxyPreparedStatement; + private final Connection wrappedConnection; + + public PreparedStatementProxy(PreparedStatement wrappedPreparedStatement, Connection wrappedConnection) { + this.wrappedPreparedStatement = wrappedPreparedStatement; + this.wrappedConnection = wrappedConnection; + } + + @Override + public Object invoke(Object proxy, Method method, Object[] args) throws Throwable { + Object result; + if (method.getName().equals("getConnection")) { + result = wrappedConnection; + } else if (method.getName().equals("executeQuery")) { + return ResultSetProxy.newInstance((ResultSet) method.invoke(wrappedPreparedStatement, args), proxyPreparedStatement); + } else { + result = method.invoke(wrappedPreparedStatement, args); + } + return result; + } + + public static PreparedStatement newInstance(PreparedStatement ps, Connection con) throws SQLException { + PreparedStatementProxy invocationHandler = new PreparedStatementProxy(ps, con); + PreparedStatement proxyPreparedStatement = (PreparedStatement) Proxy.newProxyInstance(ps.getClass().getClassLoader(), + ps.getClass().getInterfaces(), + invocationHandler); + invocationHandler.setProxyPreparedStatement(proxyPreparedStatement); + return proxyPreparedStatement; + } + + public void setProxyPreparedStatement(PreparedStatement proxyPreparedStatement) { + this.proxyPreparedStatement = proxyPreparedStatement; + } +} diff --git a/velocity-engine-core/src/main/java/org/apache/velocity/runtime/resource/loader/ResultSetProxy.java b/velocity-engine-core/src/main/java/org/apache/velocity/runtime/resource/loader/ResultSetProxy.java new file mode 100644 index 000000000..7f068c005 --- /dev/null +++ b/velocity-engine-core/src/main/java/org/apache/velocity/runtime/resource/loader/ResultSetProxy.java @@ -0,0 +1,58 @@ +package org.apache.velocity.runtime.resource.loader; +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import java.lang.reflect.InvocationHandler; +import java.lang.reflect.Method; +import java.lang.reflect.Proxy; +import java.sql.ResultSet; +import java.sql.SQLException; +import java.sql.Statement; + +/** + * Proxy for {@link java.sql.ResultSet} that guarantees getStatement will return the original wrapped {@link java.sql.Statement} + * and will not throw an exception or return null if the ResultSet is closed. + */ +public class ResultSetProxy implements InvocationHandler { + private final ResultSet wrappedResultSet; + private final Statement wrappedStatement; + + public ResultSetProxy(ResultSet wrappedResultSet, Statement wrappedStatement) { + this.wrappedResultSet = wrappedResultSet; + this.wrappedStatement = wrappedStatement; + } + + @Override + public Object invoke(Object proxy, Method method, Object[] args) throws Throwable { + Object result; + if (method.getName().equals("getStatement")) { + result = wrappedStatement; + } else { + result = method.invoke(wrappedResultSet, args); + } + return result; + } + + public static ResultSet newInstance(ResultSet rs, Statement ps) throws SQLException { + return (ResultSet) Proxy.newProxyInstance(rs.getClass().getClassLoader(), + rs.getClass().getInterfaces(), + new ResultSetProxy(rs, ps)); + } + +} diff --git a/velocity-engine-core/src/test/java/org/apache/velocity/test/sql/DataSourceResourceLoaderTestCase.java b/velocity-engine-core/src/test/java/org/apache/velocity/test/sql/DataSourceResourceLoaderTestCase.java index d4333b73a..dd0b9eae9 100644 --- a/velocity-engine-core/src/test/java/org/apache/velocity/test/sql/DataSourceResourceLoaderTestCase.java +++ b/velocity-engine-core/src/test/java/org/apache/velocity/test/sql/DataSourceResourceLoaderTestCase.java @@ -19,11 +19,14 @@ * under the License. */ +import com.zaxxer.hikari.HikariDataSource; import junit.framework.Test; import junit.framework.TestSuite; +import org.apache.commons.dbcp2.BasicDataSource; import org.apache.velocity.Template; import org.apache.velocity.VelocityContext; import org.apache.velocity.app.Velocity; +import org.apache.velocity.exception.ResourceNotFoundException; import org.apache.velocity.runtime.RuntimeInstance; import org.apache.velocity.runtime.resource.loader.DataSourceResourceLoader; import org.apache.velocity.test.misc.TestLogger; @@ -69,6 +72,18 @@ public class DataSourceResourceLoaderTestCase /* engine with VARCHAR templates data source */ private RuntimeInstance varcharTemplatesEngine = null; + /* engine with VARCHAR templates data source for testing connection counts with DBCP2 data source*/ + private RuntimeInstance varcharTemplatesDBCP2ConnectionCountTestEngine = null; + private BasicDataSource dbcp2ConnectionCountDataSource = null; + + /* engine with VARCHAR templates data source for testing connection counts with Tomcat JDBC data source*/ + private RuntimeInstance varcharTemplatesTomcatJDBCConnectionCountTestEngine = null; + private org.apache.tomcat.jdbc.pool.DataSource tomcatJDBCConnectionCountDataSource = null; + + /* engine with VARCHAR templates data source for testing connection counts with Tomcat JDBC data source*/ + private RuntimeInstance varcharTemplatesHikariConnectionCountTestEngine = null; + private HikariDataSource hikariConnectionCountDataSource = null; + /* engine with CLOB templates data source */ private RuntimeInstance clobTemplatesEngine = null; @@ -93,25 +108,71 @@ public void setUp() DataSource ds1 = new TestDataSource(TEST_JDBC_DRIVER_CLASS, TEST_JDBC_URI, TEST_JDBC_LOGIN, TEST_JDBC_PASSWORD); DataSourceResourceLoader rl1 = new DataSourceResourceLoader(); rl1.setDataSource(ds1); - - DataSource ds2 = new TestDataSource(TEST_JDBC_DRIVER_CLASS, TEST_JDBC_URI, TEST_JDBC_LOGIN, TEST_JDBC_PASSWORD); - DataSourceResourceLoader rl2 = new DataSourceResourceLoader(); - rl2.setDataSource(ds2); - ExtProperties props = getResourceLoaderProperties(); props.setProperty( "ds.resource.loader.instance", rl1); props.setProperty( "ds.resource.loader.resource.table", "velocity_template_varchar"); - varcharTemplatesEngine = new RuntimeInstance(); varcharTemplatesEngine.setConfiguration(props); varcharTemplatesEngine.init(); + DataSource ds2 = new TestDataSource(TEST_JDBC_DRIVER_CLASS, TEST_JDBC_URI, TEST_JDBC_LOGIN, TEST_JDBC_PASSWORD); + DataSourceResourceLoader rl2 = new DataSourceResourceLoader(); + rl2.setDataSource(ds2); ExtProperties props2 = (ExtProperties)props.clone(); props2.setProperty( "ds.resource.loader.instance", rl2); props2.setProperty( "ds.resource.loader.resource.table", "velocity_template_clob"); clobTemplatesEngine = new RuntimeInstance(); clobTemplatesEngine.setConfiguration(props2); clobTemplatesEngine.init(); + + BasicDataSource ds3 = new BasicDataSource(); + ds3.setDriverClassName(TEST_JDBC_DRIVER_CLASS); + ds3.setUrl(TEST_JDBC_URI); + ds3.setUsername(TEST_JDBC_LOGIN); + ds3.setPassword(TEST_JDBC_PASSWORD); + ds3.setMaxTotal(10); + DataSourceResourceLoader rl3 = new DataSourceResourceLoader(); + rl3.setDataSource(ds3); + ExtProperties props3 = getResourceLoaderProperties(); + props3.setProperty( "ds.resource.loader.instance", rl3); + props3.setProperty( "ds.resource.loader.resource.table", "velocity_template_varchar"); + varcharTemplatesDBCP2ConnectionCountTestEngine = new RuntimeInstance(); + varcharTemplatesDBCP2ConnectionCountTestEngine.setConfiguration(props3); + varcharTemplatesDBCP2ConnectionCountTestEngine.init(); + dbcp2ConnectionCountDataSource = ds3; + + org.apache.tomcat.jdbc.pool.DataSource ds4 = new org.apache.tomcat.jdbc.pool.DataSource(); + ds4.setDriverClassName(TEST_JDBC_DRIVER_CLASS); + ds4.setUrl(TEST_JDBC_URI); + ds4.setUsername(TEST_JDBC_LOGIN); + ds4.setPassword(TEST_JDBC_PASSWORD); + ds4.setMaxActive(10); + DataSourceResourceLoader rl4 = new DataSourceResourceLoader(); + rl4.setDataSource(ds4); + ExtProperties props4 = getResourceLoaderProperties(); + props4.setProperty( "ds.resource.loader.instance", rl4); + props4.setProperty( "ds.resource.loader.resource.table", "velocity_template_varchar"); + varcharTemplatesTomcatJDBCConnectionCountTestEngine = new RuntimeInstance(); + varcharTemplatesTomcatJDBCConnectionCountTestEngine.setConfiguration(props4); + varcharTemplatesTomcatJDBCConnectionCountTestEngine.init(); + tomcatJDBCConnectionCountDataSource = ds4; + + HikariDataSource ds5 = new HikariDataSource(); + ds5.setDriverClassName(TEST_JDBC_DRIVER_CLASS); + ds5.setJdbcUrl(TEST_JDBC_URI); + ds5.setUsername(TEST_JDBC_LOGIN); + ds5.setPassword(TEST_JDBC_PASSWORD); + ds5.setMaximumPoolSize(10); + DataSourceResourceLoader rl5 = new DataSourceResourceLoader(); + rl5.setDataSource(ds5); + ExtProperties props5 = getResourceLoaderProperties(); + props5.setProperty( "ds.resource.loader.instance", rl5); + props5.setProperty( "ds.resource.loader.resource.table", "velocity_template_varchar"); + varcharTemplatesHikariConnectionCountTestEngine = new RuntimeInstance(); + varcharTemplatesHikariConnectionCountTestEngine.setConfiguration(props5); + varcharTemplatesHikariConnectionCountTestEngine.init(); + hikariConnectionCountDataSource = ds5; + } protected ExtProperties getResourceLoaderProperties() @@ -129,13 +190,62 @@ protected ExtProperties getResourceLoaderProperties() * Tests loading and rendering of a simple template. If that works, we are able to get data * from the database. */ + public void testSimpleTemplate() throws Exception { Template t = executeTest("testTemplate1", varcharTemplatesEngine); assertFalse("Timestamp is 0", 0 == t.getLastModified()); t = executeTest("testTemplate1", clobTemplatesEngine); - assertFalse("Timestamp is 0", 0 == t.getLastModified()); } + assertFalse("Timestamp is 0", 0 == t.getLastModified()); + } + /** + * Tests loading and rendering of a simple template and checks that there are no connection leaks. Uses DBCP2 Data data source + */ + public void testDBCP2DataSourceForConnectionLeaks() + throws Exception + { + executeTest("testTemplate1", varcharTemplatesDBCP2ConnectionCountTestEngine); + try { + varcharTemplatesDBCP2ConnectionCountTestEngine.getTemplate("fakeTemplate"); + fail("Should have thrown exception ResourceNotFoundException"); + } catch (ResourceNotFoundException e) { + //continue + } + assertEquals("Open connection count is greater then 0", 0, this.dbcp2ConnectionCountDataSource.getConnectionPool().getNumActive()); + } + + /** + * Tests loading and rendering of a simple template and checks that there are no connection leaks. Uses Tomcat JDBC data source + */ + public void testTomcatJDBCDataSourceForConnectionLeaks() + throws Exception + { + executeTest("testTemplate1", varcharTemplatesTomcatJDBCConnectionCountTestEngine); + try { + varcharTemplatesTomcatJDBCConnectionCountTestEngine.getTemplate("fakeTemplate"); + fail("Should have thrown exception ResourceNotFoundException"); + } catch (ResourceNotFoundException e) { + //continue + } + assertEquals("Open connection count is greater then 0", 0, this.tomcatJDBCConnectionCountDataSource.getActive()); + } + + /** + * Tests loading and rendering of a simple template and checks that there are no connection leaks. Uses Tomcat JDBC data source + */ + public void testHikariCPDataSourceForConnectionLeaks() + throws Exception + { + executeTest("testTemplate1", varcharTemplatesHikariConnectionCountTestEngine); + try { + varcharTemplatesHikariConnectionCountTestEngine.getTemplate("fakeTemplate"); + fail("Should have thrown exception ResourceNotFoundException"); + } catch (ResourceNotFoundException e) { + //continue + } + assertEquals("Open connection count is greater then 0", 0, this.hikariConnectionCountDataSource.getHikariPoolMXBean().getActiveConnections()); + } public void testUnicode(RuntimeInstance engine) throws Exception @@ -161,6 +271,7 @@ public void testUnicode(RuntimeInstance engine) * Now we have a more complex example. Run a very simple tool. * from the database. */ + public void testRenderTool() throws Exception { @@ -173,17 +284,20 @@ public void testRenderTool() /** * Will a NULL timestamp choke the loader? */ + public void testNullTimestamp() throws Exception { Template t = executeTest("testTemplate3", varcharTemplatesEngine); assertEquals("Timestamp is not 0", 0, t.getLastModified()); t = executeTest("testTemplate3", clobTemplatesEngine); - assertEquals("Timestamp is not 0", 0, t.getLastModified()); } + assertEquals("Timestamp is not 0", 0, t.getLastModified()); + } /** * Does it load the global Macros from the DB? */ + public void testMacroInvocation() throws Exception {