Author: markt
Date: Mon Nov 27 12:53:37 2017
New Revision: 1816449
URL: http://svn.apache.org/viewvc?rev=1816449&view=rev
Log:
Fix SpotBugs issues (up to rank 16) in JDBC pool
Modified:
tomcat/trunk/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/MultipleCloseTest.java
tomcat/trunk/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/StarvationTest.java
tomcat/trunk/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/TestException.java
tomcat/trunk/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/TestStatementCache.java
tomcat/trunk/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/TestTimeout.java
tomcat/trunk/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/TestValidationQueryTimeout.java
tomcat/trunk/res/findbugs/filter-false-positives.xml
Modified:
tomcat/trunk/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/MultipleCloseTest.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/MultipleCloseTest.java?rev=1816449&r1=1816448&r2=1816449&view=diff
==============================================================================
---
tomcat/trunk/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/MultipleCloseTest.java
(original)
+++
tomcat/trunk/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/MultipleCloseTest.java
Mon Nov 27 12:53:37 2017
@@ -61,9 +61,11 @@ public class MultipleCloseTest extends D
Assert.assertTrue(con1.isClosed());
// Open a new connection (This will re-use the previous pooled
connection)
- datasource.getConnection();
+ Connection con2 = datasource.getConnection();
// A connection, once closed, should stay closed
Assert.assertTrue(con1.isClosed());
+
+ con2.close();
}
}
Modified:
tomcat/trunk/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/StarvationTest.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/StarvationTest.java?rev=1816449&r1=1816448&r2=1816449&view=diff
==============================================================================
---
tomcat/trunk/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/StarvationTest.java
(original)
+++
tomcat/trunk/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/StarvationTest.java
Mon Nov 27 12:53:37 2017
@@ -82,6 +82,7 @@ public class StarvationTest extends Defa
}finally {
if (con2!=null) con2.close();
}
+ con1.close();
}
@Test
@@ -104,5 +105,6 @@ public class StarvationTest extends Defa
}finally {
if (con2!=null) con2.close();
}
+ con1.close();
}
}
Modified:
tomcat/trunk/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/TestException.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/TestException.java?rev=1816449&r1=1816448&r2=1816449&view=diff
==============================================================================
---
tomcat/trunk/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/TestException.java
(original)
+++
tomcat/trunk/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/TestException.java
Mon Nov 27 12:53:37 2017
@@ -17,7 +17,9 @@
package org.apache.tomcat.jdbc.test;
import java.sql.Connection;
+import java.sql.Statement;
+import org.junit.Assert;
import org.junit.Test;
import org.apache.tomcat.jdbc.pool.ConnectionPool;
@@ -30,11 +32,11 @@ public class TestException extends Defau
public void testException() throws Exception {
datasource.getPoolProperties().setJdbcInterceptors(TestInterceptor.class.getName());
Connection con = datasource.getConnection();
- try {
- con.createStatement();
- }catch (Exception x) {
- // Ignore
+ try (Statement s = con.createStatement()){
+ } catch (Exception x) {
+ Assert.fail();
}
+ con.close();
}
@@ -42,7 +44,7 @@ public class TestException extends Defau
@Override
public void reset(ConnectionPool parent, PooledConnection con) {
- // TODO Auto-generated method stub
+ // NO-OP
}
}
}
Modified:
tomcat/trunk/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/TestStatementCache.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/TestStatementCache.java?rev=1816449&r1=1816448&r2=1816449&view=diff
==============================================================================
---
tomcat/trunk/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/TestStatementCache.java
(original)
+++
tomcat/trunk/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/TestStatementCache.java
Mon Nov 27 12:53:37 2017
@@ -88,6 +88,7 @@ public class TestStatementCache extends
ps3.close();
Assert.assertTrue(ps3.isClosed());
Assert.assertEquals(1,interceptor.getCacheSize().get());
+ con.close();
}
@Test
@@ -108,6 +109,7 @@ public class TestStatementCache extends
ps3.close();
Assert.assertTrue(ps3.isClosed());
Assert.assertEquals(0,interceptor.getCacheSize().get());
+ con.close();
}
@Test
Modified:
tomcat/trunk/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/TestTimeout.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/TestTimeout.java?rev=1816449&r1=1816448&r2=1816449&view=diff
==============================================================================
---
tomcat/trunk/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/TestTimeout.java
(original)
+++
tomcat/trunk/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/TestTimeout.java
Mon Nov 27 12:53:37 2017
@@ -16,6 +16,9 @@
*/
package org.apache.tomcat.jdbc.test;
+import java.sql.Connection;
+import java.util.HashSet;
+import java.util.Set;
import java.util.concurrent.atomic.AtomicInteger;
import org.junit.Assert;
@@ -27,6 +30,7 @@ public class TestTimeout extends Default
@Test
public void testCheckoutTimeout() throws Exception {
+ Set<Connection> cons = new HashSet<>();
try {
this.datasource.getPoolProperties().setTestWhileIdle(true);
this.datasource.getPoolProperties().setTestOnBorrow(false);
@@ -42,20 +46,24 @@ public class TestTimeout extends Default
System.out.println("About to test connection pool:"+datasource);
for (int i = 0; i < 21; i++) {
long now = System.currentTimeMillis();
- this.datasource.getConnection();
+ cons.add(this.datasource.getConnection());
long delta = System.currentTimeMillis()-now;
System.out.println("Got connection #"+i+" in "+delta+" ms.");
}
- Assert.assertTrue(false);
+ Assert.fail();
} catch ( Exception x ) {
- Assert.assertTrue(true);
+ // Expected on 21st checkout
}finally {
Thread.sleep(2000);
}
+ for (Connection c : cons) {
+ c.close();
+ }
}
@Test
public void testCheckoutTimeoutFair() throws Exception {
+ Set<Connection> cons = new HashSet<>();
try {
this.datasource.getPoolProperties().setFairQueue(true);
this.datasource.getPoolProperties().setTestWhileIdle(true);
@@ -72,15 +80,18 @@ public class TestTimeout extends Default
System.out.println("About to test connection pool:"+datasource);
for (int i = 0; i < 21; i++) {
long now = System.currentTimeMillis();
- this.datasource.getConnection();
+ cons.add(this.datasource.getConnection());
long delta = System.currentTimeMillis()-now;
System.out.println("Got connection #"+i+" in "+delta+" ms.");
}
- Assert.assertTrue(false);
+ Assert.fail();
} catch ( Exception x ) {
- Assert.assertTrue(true);
+ // Expected on 21st checkout
}finally {
Thread.sleep(2000);
}
+ for (Connection c : cons) {
+ c.close();
+ }
}
}
Modified:
tomcat/trunk/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/TestValidationQueryTimeout.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/TestValidationQueryTimeout.java?rev=1816449&r1=1816448&r2=1816449&view=diff
==============================================================================
---
tomcat/trunk/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/TestValidationQueryTimeout.java
(original)
+++
tomcat/trunk/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/TestValidationQueryTimeout.java
Mon Nov 27 12:53:37 2017
@@ -67,8 +67,9 @@ public class TestValidationQueryTimeout
@Test
public void testValidationQueryTimeoutEnabled() throws Exception {
// because testOnBorrow is true, this triggers the validation query
- this.datasource.getConnection();
+ Connection con = this.datasource.getConnection();
Assert.assertTrue(isTimeoutSet);
+ con.close();
}
@Test
@@ -76,8 +77,9 @@ public class TestValidationQueryTimeout
this.datasource.setValidationQueryTimeout(-1);
// because testOnBorrow is true, this triggers the validation query
- this.datasource.getConnection();
+ Connection con = this.datasource.getConnection();
Assert.assertFalse(isTimeoutSet);
+ con.close();
}
@Test
@@ -109,8 +111,9 @@ public class TestValidationQueryTimeout
// pull another connection and check it
TIMEOUT = 10;
isTimeoutSet = false;
- this.datasource.getConnection();
+ Connection con2 = this.datasource.getConnection();
Assert.assertTrue(isTimeoutSet);
+ con2.close();
}
// this test depends on the execution time of the validation query
Modified: tomcat/trunk/res/findbugs/filter-false-positives.xml
URL:
http://svn.apache.org/viewvc/tomcat/trunk/res/findbugs/filter-false-positives.xml?rev=1816449&r1=1816448&r2=1816449&view=diff
==============================================================================
--- tomcat/trunk/res/findbugs/filter-false-positives.xml (original)
+++ tomcat/trunk/res/findbugs/filter-false-positives.xml Mon Nov 27 12:53:37
2017
@@ -1223,6 +1223,15 @@
<Bug code="ST" />
</Match>
<Match>
+ <!-- The name shadowing is deliberate -->
+ <Or>
+ <Class name="org.apache.tomcat.jdbc.test.driver.Connection" />
+ <Class name="org.apache.tomcat.jdbc.test.driver.Driver" />
+ <Class name="org.apache.tomcat.jdbc.test.driver.ResultSet" />
+ </Or>
+ <Bug pattern="NM_SAME_SIMPLE_NAME_AS_INTERFACE" />
+ </Match>
+ <Match>
<!-- The call with the ignored return value is used to ensure the pool -->
<!-- thinks the connection is being used. -->
<Class name="org.apache.tomcat.jdbc.test.AbandonPercentageTest" />
@@ -1230,6 +1239,32 @@
<Bug pattern="RV_RETURN_VALUE_IGNORED" />
</Match>
<Match>
+ <!-- A number of the tests incude performance tests -->
+ <Class name="org.apache.tomcat.jdbc.test.DefaultTestCase" />
+ <Method name="tearDown" />
+ <Bug pattern="DM_GC" />
+ </Match>
+ <Match>
+ <!-- Test does not explicitly close statement deliberately -->
+ <Class name="org.apache.tomcat.jdbc.test.StatementFinalizerTest" />
+ <Method name="testStatementFinalization" />
+ <Bug pattern="ODR_OPEN_DATABASE_RESOURCE"/>
+ </Match>
+ <Match>
+ <!-- Choice of name is deliberate -->
+ <Class name="org.apache.tomcat.jdbc.test.TestException" />
+ <Bug pattern="NM_CLASS_NOT_EXCEPTION" />
+ </Match>
+ <Match>
+ <!-- Testing auto-close so connections not explicitly closed -->
+ <Class name="org.apache.tomcat.jdbc.test.TestGCClose" />
+ <Or>
+ <Method name="testGCStop" />
+ <Method name="testClose" />
+ </Or>
+ <Bug pattern="ODR_OPEN_DATABASE_RESOURCE" />
+ </Match>
+ <Match>
<!-- SQL is from config so is considered safe -->
<Class name="org.apache.tomcat.jdbc.test.TestSlowQueryReport" />
<Method name="testFastSql" />
@@ -1248,6 +1283,16 @@
<Bug pattern="SQL_PREPARED_STATEMENT_GENERATED_FROM_NONCONSTANT_STRING" />
</Match>
<Match>
+ <!-- Tests throw exceptions so connections are never created -->
+ <Class name="org.apache.tomcat.jdbc.test.TestValidationQueryTimeout" />
+ <Or>
+ <Method name="testValidationQueryTimeoutOnConnection" />
+ <Method name="testValidationInvalidOnConnection" />
+ <Method name="testValidationQueryTimeoutOnBorrow" />
+ </Or>
+ <Bug pattern="ODR_OPEN_DATABASE_RESOURCE" />
+ </Match>
+ <Match>
<Class name="org.apache.tomcat.jdbc.test.TwoDataSources" />
<Method name="testTwoDataSources" />
<Or>
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
