You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by th...@apache.org on 2023/02/14 03:38:50 UTC

[commons-dbutils] 02/03: Promote optimization as default for fetching the parameter metadata only once Clean up duplicated tests

This is an automated email from the ASF dual-hosted git repository.

thecarlhall pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-dbutils.git

commit de4a1224dbfae557abdf3e237ee29df43e8d145b
Author: Carl Hall <th...@apache.org>
AuthorDate: Mon Feb 13 22:30:40 2023 -0500

    Promote optimization as default for fetching the parameter metadata only once
    Clean up duplicated tests
---
 .../commons/dbutils/AbstractQueryRunner.java       |  31 +++--
 .../apache/commons/dbutils/QueryRunnerTest.java    | 145 ++-------------------
 2 files changed, 25 insertions(+), 151 deletions(-)

diff --git a/src/main/java/org/apache/commons/dbutils/AbstractQueryRunner.java b/src/main/java/org/apache/commons/dbutils/AbstractQueryRunner.java
index f740823..fd433e3 100644
--- a/src/main/java/org/apache/commons/dbutils/AbstractQueryRunner.java
+++ b/src/main/java/org/apache/commons/dbutils/AbstractQueryRunner.java
@@ -295,7 +295,15 @@ public abstract class AbstractQueryRunner {
     public void fillStatement(final PreparedStatement stmt, final Object... params) throws SQLException {
         ParameterMetaData pmd = null;
         if (!pmdKnownBroken) {
-            pmd = this.getParameterMetaData(stmt);
+            try {
+                pmd = this.getParameterMetaData(stmt);
+                if (pmd == null) { // can be returned by implementations that don't support the method
+                    pmdKnownBroken = true;
+                }
+            } catch (final SQLFeatureNotSupportedException ex) {
+                pmdKnownBroken = true;
+            }
+            // TODO see DBUTILS-117: would it make sense to catch any other SQLEx types here?
         }
         fillStatement(stmt, pmd, params);
     }
@@ -318,23 +326,14 @@ public abstract class AbstractQueryRunner {
             throws SQLException {
 
         // check the parameter count, if we can
-        if (!pmdKnownBroken) {
-            try {
-                if (pmd == null) { // can be returned by implementations that don't support the method
-                    pmdKnownBroken = true;
-                } else {
-                    final int stmtCount = pmd.getParameterCount();
-                    final int paramsCount = params == null ? 0 : params.length;
+        if (!pmdKnownBroken && pmd != null) {
+            final int stmtCount = pmd.getParameterCount();
+            final int paramsCount = params == null ? 0 : params.length;
 
-                    if (stmtCount != paramsCount) {
-                        throw new SQLException("Wrong number of parameters: expected "
-                                + stmtCount + ", was given " + paramsCount);
-                    }
-                }
-            } catch (final SQLFeatureNotSupportedException ex) {
-                pmdKnownBroken = true;
+            if (stmtCount != paramsCount) {
+                throw new SQLException("Wrong number of parameters: expected "
+                        + stmtCount + ", was given " + paramsCount);
             }
-            // TODO see DBUTILS-117: would it make sense to catch any other SQLEx types here?
         }
 
         // nothing to do here
diff --git a/src/test/java/org/apache/commons/dbutils/QueryRunnerTest.java b/src/test/java/org/apache/commons/dbutils/QueryRunnerTest.java
index bfc33ae..d408398 100644
--- a/src/test/java/org/apache/commons/dbutils/QueryRunnerTest.java
+++ b/src/test/java/org/apache/commons/dbutils/QueryRunnerTest.java
@@ -47,7 +47,6 @@ import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mockito.Mock;
-import org.mockito.MockitoAnnotations;
 import org.mockito.invocation.InvocationOnMock;
 import org.mockito.junit.MockitoJUnitRunner;
 import org.mockito.stubbing.Answer;
@@ -97,16 +96,6 @@ public class QueryRunnerTest {
         when(meta.getParameterCount()).thenReturn(2);
         runner.batch(conn, "select * from blah where ? = ?", params);
 
-        verify(prepStmt, times(2)).addBatch();
-        verify(prepStmt, times(1)).executeBatch();
-        verify(prepStmt, times(1)).close();    // make sure we closed the statement
-        verify(conn, times(0)).close();    // make sure we do not close the connection, since QueryRunner.batch(Connection, String, Object[][]) does not close connections
-    }
-
-    private void callGoodBatchPrefetchPmdTrue(final Connection conn, final Object[][] params) throws Exception {
-        when(meta.getParameterCount()).thenReturn(2);
-        runner.batch(conn, "select * from blah where ? = ?", true, params);
-
         verify(prepStmt, times(1)).getParameterMetaData();
         verify(prepStmt, times(2)).addBatch();
         verify(prepStmt, times(1)).executeBatch();
@@ -115,18 +104,12 @@ public class QueryRunnerTest {
     }
 
     private void callGoodBatch(final Object[][] params) throws Exception {
-        when(meta.getParameterCount()).thenReturn(2);
-        runner.batch("select * from blah where ? = ?", params);
-
-        verify(prepStmt, times(2)).addBatch();
-        verify(prepStmt, times(1)).executeBatch();
-        verify(prepStmt, times(1)).close();    // make sure we closed the statement
-        verify(conn, times(1)).close();    // make sure we closed the connection
+        callGoodBatch(params, true);
     }
 
-    private void callGoodBatchPrefetchPmdTrue(final Object[][] params, boolean pmdCheck) throws Exception {
+    private void callGoodBatch(final Object[][] params, boolean pmdCheck) throws Exception {
         when(meta.getParameterCount()).thenReturn(2);
-        runner.batch("select * from blah where ? = ?", true, params);
+        runner.batch("select * from blah where ? = ?", params);
 
         verify(prepStmt, times(pmdCheck ? 1 : 0)).getParameterMetaData();
         verify(prepStmt, times(2)).addBatch();
@@ -143,30 +126,14 @@ public class QueryRunnerTest {
         callGoodBatch(params);
     }
 
-    @Test
-    public void testGoodBatchPrefetchPmdTrue() throws Exception {
-        final String[][] params = new String[][] { { "unit", "unit" }, { "test", "test" } };
-
-        callGoodBatchPrefetchPmdTrue(params, true);
-    }
-
     @Test
     public void testGoodBatchPmdTrue() throws Exception {
         runner = new QueryRunner(dataSource, true);
         final String[][] params = new String[][] { { "unit", "unit" }, { "test", "test" } };
 
-        callGoodBatch(params);
-    }
-
-    @Test
-    public void testGoodBatchPmdTrueAndPrefetchPmdTrue() throws Exception {
-        runner = new QueryRunner(dataSource, true);
-        final String[][] params = new String[][] { { "unit", "unit" }, { "test", "test" } };
-
-        callGoodBatchPrefetchPmdTrue(params, false);
+        callGoodBatch(params, false);
     }
 
-
     @Test
     public void testGoodBatchDefaultConstructor() throws Exception {
         runner = new QueryRunner();
@@ -175,15 +142,6 @@ public class QueryRunnerTest {
         callGoodBatch(conn, params);
     }
 
-    @Test
-    public void testGoodBatchDefaultConstructorPrefetchPmdTrue() throws Exception {
-        runner = new QueryRunner();
-        final String[][] params = new String[][] { { "unit", "unit" }, { "test", "test" } };
-
-        callGoodBatchPrefetchPmdTrue(conn, params);
-    }
-
-
     @Test
     public void testNullParamsBatch() throws Exception {
         final String[][] params = new String[][] { { null, "unit" }, { "test", null } };
@@ -191,37 +149,10 @@ public class QueryRunnerTest {
         callGoodBatch(params);
     }
 
-    @Test
-    public void testNullParamsBatchPrefetchPmdTrue() throws Exception {
-        final String[][] params = new String[][] { { null, "unit" }, { "test", null } };
-
-        callGoodBatchPrefetchPmdTrue(params, true);
-    }
-
-
 
     // helper method for calling batch when an exception is expected
     private void callBatchWithException(final String sql, final Object[][] params) throws Exception {
-        boolean caught = false;
-
-        try {
-            runner.batch(sql, params);
-
-            verify(prepStmt, times(2)).addBatch();
-            verify(prepStmt, times(1)).executeBatch();
-            verify(prepStmt, times(1)).close();    // make sure the statement is closed
-            verify(conn, times(1)).close();    // make sure the connection is closed
-        } catch(final SQLException e) {
-            caught = true;
-        }
-
-        if(!caught) {
-            fail("Exception never thrown, but expected");
-        }
-    }
-
-    // helper method for calling batch when an exception is expected
-    private void callBatchWithExceptionPrefetchPmd(final String sql, final Object[][] params) throws Exception {
+        when(meta.getParameterCount()).thenReturn(2);
         boolean caught = false;
 
         try {
@@ -233,6 +164,8 @@ public class QueryRunnerTest {
             verify(prepStmt, times(1)).close();    // make sure the statement is closed
             verify(conn, times(1)).close();    // make sure the connection is closed
         } catch(final SQLException e) {
+            System.out.println("[TEST] The following exception is expected:");
+            System.out.println(e);
             caught = true;
         }
 
@@ -241,7 +174,6 @@ public class QueryRunnerTest {
         }
     }
 
-
     @Test
     public void testTooFewParamsBatch() throws Exception {
         final String[][] params = new String[][] { { "unit" }, { "test" } };
@@ -256,21 +188,6 @@ public class QueryRunnerTest {
         callBatchWithException("select * from blah where ? = ?", params);
     }
 
-    @Test
-    public void testTooFewParamsBatchPrefetchPmd() throws Exception {
-        final String[][] params = new String[][] { { "unit" }, { "test" } };
-
-        callBatchWithExceptionPrefetchPmd("select * from blah where ? = ?", params);
-    }
-
-    @Test
-    public void testTooManyParamsBatchPrefetchPmd() throws Exception {
-        final String[][] params = new String[][] { { "unit", "unit", "unit" }, { "test", "test", "test" } };
-
-        callBatchWithExceptionPrefetchPmd("select * from blah where ? = ?", params);
-    }
-
-
     @Test(expected=SQLException.class)
     public void testNullConnectionBatch() throws Exception {
         final String[][] params = new String[][] { { "unit", "unit" }, { "test", "test" } };
@@ -280,16 +197,6 @@ public class QueryRunnerTest {
         runner.batch("select * from blah where ? = ?", params);
     }
 
-    @Test(expected=SQLException.class)
-    public void testNullConnectionBatchPrefetchPmd() throws Exception {
-        final String[][] params = new String[][] { { "unit", "unit" }, { "test", "test" } };
-
-        when(meta.getParameterCount()).thenReturn(2);
-        when(dataSource.getConnection()).thenReturn(null);
-
-        runner.batch("select * from blah where ? = ?", true, params);
-    }
-
 
     @Test(expected=SQLException.class)
     public void testNullSqlBatch() throws Exception {
@@ -298,60 +205,28 @@ public class QueryRunnerTest {
         runner.batch(null, params);
     }
 
-    @Test(expected=SQLException.class)
-    public void testNullSqlBatchPrefetchPmd() throws Exception {
-        final String[][] params = new String[][] { { "unit", "unit" }, { "test", "test" } };
-
-        when(meta.getParameterCount()).thenReturn(2);
-
-        runner.batch(null, true, params);
-    }
-
 
     @Test(expected=SQLException.class)
     public void testNullParamsArgBatch() throws Exception {
         runner.batch("select * from blah where ? = ?", null);
     }
 
-    @Test(expected=SQLException.class)
-    public void testNullParamsArgBatchPrefetchPmd() throws Exception {
-        when(meta.getParameterCount()).thenReturn(2);
-
-        runner.batch("select * from blah where ? = ?", true, null);
-    }
-
-
     @Test
-    public void testAddBatchException() throws Exception {
-        final String[][] params = new String[][] { { "unit", "unit" }, { "test", "test" } };
-
-        callBatchWithException("select * from blah where ? = ?", params);
-    }
-
-    @Test
-    public void testAddBatchExceptionPrefetchPmd() throws Exception {
+    public void testAddBatchExceptionOnAdd() throws Exception {
         final String[][] params = new String[][] { { "unit", "unit" }, { "test", "test" } };
 
         doThrow(new SQLException()).when(prepStmt).addBatch();
 
-        callBatchWithExceptionPrefetchPmd("select * from blah where ? = ?", params);
-    }
-
-
-    @Test
-    public void testExecuteBatchException() throws Exception {
-        final String[][] params = new String[][] { { "unit", "unit" }, { "test", "test" } };
-
         callBatchWithException("select * from blah where ? = ?", params);
     }
 
     @Test
-    public void testExecuteBatchExceptionPrefetchPmd() throws Exception {
+    public void testExecuteBatchExceptionOnExec() throws Exception {
         final String[][] params = new String[][] { { "unit", "unit" }, { "test", "test" } };
 
         doThrow(new SQLException()).when(prepStmt).executeBatch();
 
-        callBatchWithExceptionPrefetchPmd("select * from blah where ? = ?", params);
+        callBatchWithException("select * from blah where ? = ?", params);
     }