AF-1149: Dashbuilder not closing ResultSets and Statements leading to…

… ORA-01000 error
dashbuilder · Apr 19, 2018 · 7bee975 · 7bee975
1 parent ae4c145
commit 7bee975
Show file tree

Hide file tree

Showing 9 changed files with 323 additions and 82 deletions.
diff --git a/dashbuilder-backend/dashbuilder-dataset-sql/pom.xml b/dashbuilder-backend/dashbuilder-dataset-sql/pom.xml
@@ -89,6 +89,17 @@
       <scope>test</scope>
     </dependency>
 
+    <dependency>
+      <groupId>org.powermock</groupId>
+      <artifactId>powermock-module-junit4</artifactId>
+      <scope>test</scope>
+    </dependency>
+
+    <dependency>
+      <groupId>org.powermock</groupId>
+      <artifactId>powermock-api-mockito</artifactId>
+      <scope>test</scope>
+    </dependency>
   </dependencies>
 
 </project>
diff --git a/...end/dashbuilder-dataset-sql/src/main/java/org/dashbuilder/dataprovider/sql/JDBCUtils.java b/...end/dashbuilder-dataset-sql/src/main/java/org/dashbuilder/dataprovider/sql/JDBCUtils.java
@@ -15,12 +15,7 @@
  */
 package org.dashbuilder.dataprovider.sql;
 
-import java.sql.Connection;
-import java.sql.DatabaseMetaData;
-import java.sql.ResultSet;
-import java.sql.ResultSetMetaData;
-import java.sql.SQLException;
-import java.sql.Types;
+import java.sql.*;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
@@ -58,23 +53,28 @@ public class JDBCUtils {
     private static final Logger log = LoggerFactory.getLogger(JDBCUtils.class);
 
     public static void execute(Connection connection, String sql) throws SQLException {
+        Statement stmt = connection.createStatement();
         try {
             if (log.isDebugEnabled()) {
                 log.debug(sql);
             }
-            connection.createStatement().execute(sql);
+            stmt.execute(sql);
         } catch (SQLException e) {
             log.error(sql);
             throw e;
+        } finally {
+            stmt.close();
         }
     }
 
-    public static ResultSet executeQuery(Connection connection, String sql) throws SQLException {
+    public static ResultSetHandler executeQuery(Connection connection, String sql) throws SQLException {
         try {
             if (log.isDebugEnabled()) {
                 log.debug(sql);
             }
-            return connection.createStatement().executeQuery(sql);
+            Statement statement = connection.createStatement();
+            ResultSet resultSet = statement.executeQuery(sql);
+            return new ResultSetHandler(resultSet, statement);
         } catch (SQLException e) {
             log.error(sql);
             throw e;
@@ -161,28 +161,32 @@ public static Dialect dialect(String dbName) {
         return DEFAULT;
     }
 
-    public static List<Column> getColumns(ResultSet resultSet, String[] exclude) throws SQLException {
-        List<Column> columnList = new ArrayList<Column>();
-        List<String> columnExcluded = exclude == null ? new ArrayList<String>() : Arrays.asList(exclude);
+    public static List<Column> getColumns(ResultSet resultSet, String[] exclude) {
+        try {
+            List<Column> columnList = new ArrayList<>();
+            List<String> columnExcluded = exclude == null ? new ArrayList<String>() : Arrays.asList(exclude);
 
-        ResultSetMetaData meta = resultSet.getMetaData();
-        for (int i = 1; i <= meta.getColumnCount(); i++) {
-            String name = meta.getColumnName(i);
-            String alias = meta.getColumnLabel(i);
-            if (alias != null && !alias.trim().isEmpty()) {
-                name = alias.trim();
-            }
+            ResultSetMetaData meta = resultSet.getMetaData();
+            for (int i = 1; i <= meta.getColumnCount(); i++) {
+                String name = meta.getColumnName(i);
+                String alias = meta.getColumnLabel(i);
+                if (alias != null && !alias.trim().isEmpty()) {
+                    name = alias.trim();
+                }
 
-            if (!columnExcluded.contains(name) && !columnExcluded.contains(alias)) {
-                ColumnType type = JDBCUtils.calculateType(meta.getColumnType(i));
-                if (type != null) {
-                    int size = meta.getColumnDisplaySize(i);
-                    Column column = SQLFactory.column(name, type, size);
-                    columnList.add(column);
+                if (!columnExcluded.contains(name) && !columnExcluded.contains(alias)) {
+                    ColumnType type = JDBCUtils.calculateType(meta.getColumnType(i));
+                    if (type != null) {
+                        int size = meta.getColumnDisplaySize(i);
+                        Column column = SQLFactory.column(name, type, size);
+                        columnList.add(column);
+                    }
                 }
             }
+            return columnList;
+        } catch (SQLException e) {
+            throw new RuntimeException(e);
         }
-        return columnList;
     }
 
     public static String fixCase(Connection connection, String id) {
@@ -203,7 +207,7 @@ public static String fixCase(Connection connection, String id) {
     public static final String[] QUOTES = new String[]{"\"", "'", "`", "´"};
 
     public static List<String> getWordsBetweenQuotes(String s) {
-        List<String> result = new ArrayList<String>();
+        List<String> result = new ArrayList<>();
         if (s != null) {
             for (int i = 0; i < QUOTES.length; i++) {
                 String quote = QUOTES[i];

diff --git a/...builder-dataset-sql/src/main/java/org/dashbuilder/dataprovider/sql/ResultSetConsumer.java b/...builder-dataset-sql/src/main/java/org/dashbuilder/dataprovider/sql/ResultSetConsumer.java
@@ -0,0 +1,31 @@
+/**
+ * Copyright 2018 Red Hat, Inc. and/or its affiliates.
+ *
+ * Licensed 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.
+ */
+package org.dashbuilder.dataprovider.sql;
+
+import java.sql.ResultSet;
+
+@FunctionalInterface
+public interface ResultSetConsumer<R> {
+
+    /**
+     * Consumes the given data set and produces a result
+     *
+     * @param resultSet The {@link ResultSet} to consume
+     * @return R The resulting object
+     */
+    R consume(ResultSet resultSet);
+
+}
diff --git a/...hbuilder-dataset-sql/src/main/java/org/dashbuilder/dataprovider/sql/ResultSetHandler.java b/...hbuilder-dataset-sql/src/main/java/org/dashbuilder/dataprovider/sql/ResultSetHandler.java
@@ -0,0 +1,44 @@
+/**
+ * Copyright 2018 Red Hat, Inc. and/or its affiliates.
+ *
+ * Licensed 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.
+ */
+package org.dashbuilder.dataprovider.sql;
+
+import java.sql.ResultSet;
+import java.sql.SQLException;
+import java.sql.Statement;
+
+public class ResultSetHandler {
+
+    private ResultSet resultSet;
+    private Statement statement;
+
+    public ResultSetHandler(ResultSet resultSet, Statement statement) {
+        this.resultSet = resultSet;
+        this.statement = statement;
+    }
+
+    public ResultSet getResultSet() {
+        return resultSet;
+    }
+
+    public Statement getStatement() {
+        return statement;
+    }
+
+    public void close() throws SQLException {
+        resultSet.close();
+        statement.close();
+    }
+}
diff --git a/...uilder-dataset-sql/src/main/java/org/dashbuilder/dataprovider/sql/SQLDataSetProvider.java b/...uilder-dataset-sql/src/main/java/org/dashbuilder/dataprovider/sql/SQLDataSetProvider.java
@@ -17,7 +17,6 @@
 
 import java.sql.Connection;
 import java.sql.ResultSet;
-import java.sql.SQLException;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Date;
@@ -407,16 +406,15 @@ protected DataSetMetadata _getDataSetMetadata(SQLDataSetDef def, Connection conn
         return result.metadata;
     }
 
-    protected List<Column> _getColumns(SQLDataSetDef def, Connection conn) throws Exception {
-        Dialect dialect = JDBCUtils.dialect(conn);
-        if (!StringUtils.isBlank(def.getDbSQL())) {
-            Select query = SQLFactory.select(conn).from(def.getDbSQL()).limit(1);
-            return JDBCUtils.getColumns(logSQL(query).fetch(), dialect.getExcludedColumns());
-        }
-        else {
-            Select query = SQLFactory.select(conn).from(_createTable(def)).limit(1);
-            return JDBCUtils.getColumns(logSQL(query).fetch(), dialect.getExcludedColumns());
-        }
+    protected List<Column> _getColumns(SQLDataSetDef def, Connection conn) {
+        final Dialect dialect = JDBCUtils.dialect(conn);
+        Select q = SQLFactory.select(conn);
+        q = (!StringUtils.isBlank(def.getDbSQL()) ? q.from(def.getDbSQL()) : q.from(_createTable(def))).limit(1);
+        return logSQL(q).fetch(new ResultSetConsumer<List<Column>>() {
+            public List<Column> consume(ResultSet _rs) {
+                return JDBCUtils.getColumns(_rs, dialect.getExcludedColumns());
+            }
+        });
     }
 
     protected int _getRowCount(DataSetMetadata metadata, SQLDataSetDef def, Connection conn) throws Exception {
@@ -670,13 +668,8 @@ public DataSet run() throws Exception {
                     }
 
                     // Fetch the results and build the data set
-                    ResultSet _results = logSQL(_query).fetch();
                     List<DataColumn> columns = calculateColumns(null);
-                    DataSet dataSet = _buildDataSet(columns, _results);
-                    if (trim && postProcessingOps.isEmpty()) {
-                        dataSet.setRowCountNonTrimmed(totalRows);
-                    }
-                    return dataSet;
+                    return buildDataSet(columns, trim, totalRows);
                 }
                 // ... or a list of operations.
                 else {
@@ -737,19 +730,30 @@ public DataSet run() throws Exception {
                     }
 
                     // Fetch the results and build the data set
-                    ResultSet _results = logSQL(_query).fetch();
                     List<DataColumn> columns = calculateColumns(groupOp);
-                    DataSet dataSet = _buildDataSet(columns, _results);
-                    if (trim && postProcessingOps.isEmpty()) {
-                        dataSet.setRowCountNonTrimmed(totalRows);
-                    }
-                    return dataSet;
+                    return buildDataSet(columns, trim, totalRows);
                 }
             } finally {
                 conn.close();
             }
         }
 
+        protected DataSet buildDataSet(final List<DataColumn> columns, boolean trim, int totalRows) throws Exception {
+            DataSet dataSet = logSQL(_query).fetch(new ResultSetConsumer<DataSet>() {
+                public DataSet consume(ResultSet _rs) {
+                    try {
+                        return _buildDataSet(columns, _rs);
+                    } catch (Exception e) {
+                        throw new RuntimeException(e);
+                    }
+                }
+            });
+            if (trim && postProcessingOps.isEmpty()) {
+                dataSet.setRowCountNonTrimmed(totalRows);
+            }
+            return dataSet;
+        }
+
         protected DateIntervalType calculateDateInterval(ColumnGroup cg) {
             if (dateIntervalType != null) {
                 return dateIntervalType;
@@ -793,22 +797,20 @@ protected Date calculateDateLimit(String dateColumnId, boolean min) {
                 _appendIntervalSelection(intervalSelect, _limitsQuery);
             }
 
-            try {
-                // Fetch the date
-                ResultSet rs = logSQL(_limitsQuery
-                        .where(_dateColumn.notNull())
-                        .orderBy(min ? _dateColumn.asc() : _dateColumn.desc())
-                        .limit(1)).fetch();
-
-                if (!rs.next()) {
-                    return null;
-                } else {
-                    return rs.getDate(1);
+            _limitsQuery = _limitsQuery.where(_dateColumn.notNull())
+                    .orderBy(min ? _dateColumn.asc() : _dateColumn.desc())
+                    .limit(1);
+
+            return logSQL(_limitsQuery).fetch(new ResultSetConsumer<Date>() {
+                public Date consume(ResultSet rs) {
+                    try {
+                        return rs.next() ? rs.getDate(1) : null;
+                    } catch (Exception e) {
+                        log.error("Error reading date limit from query results", e);
+                        return null;
+                    }
                 }
-            } catch (SQLException e) {
-                log.error("Error reading date limit from query results", e);
-                return null;
-            }
+            });
         }
 
         protected List<DataColumn> calculateColumns(DataSetGroup gOp) {

diff --git a/...-dataset-sql/src/main/java/org/dashbuilder/dataprovider/sql/dialect/SQLServerDialect.java b/...-dataset-sql/src/main/java/org/dashbuilder/dataprovider/sql/dialect/SQLServerDialect.java
@@ -15,14 +15,15 @@
  */
 package org.dashbuilder.dataprovider.sql.dialect;
 
-import java.sql.SQLException;
+import java.sql.ResultSet;
 import java.text.SimpleDateFormat;
 import java.util.Collections;
 import java.util.Date;
 import java.util.List;
 import java.util.ArrayList;
 
 import org.dashbuilder.dataprovider.sql.JDBCUtils;
+import org.dashbuilder.dataprovider.sql.ResultSetConsumer;
 import org.dashbuilder.dataprovider.sql.model.Column;
 import org.dashbuilder.dataprovider.sql.model.Select;
 import org.dashbuilder.dataprovider.sql.model.SortColumn;
@@ -122,10 +123,15 @@ public List<Column> fetchColumns(Select select) {
         try {
             // Disable limits & fetch results
             select.limit(0).offset(0);
-            return JDBCUtils.getColumns(select.fetch(), null);
-        }
-        catch (SQLException e) {
-            return Collections.emptyList();
+            return select.fetch(new ResultSetConsumer<List<Column>>() {
+                public List<Column> consume(ResultSet rs) {
+                    try {
+                        return JDBCUtils.getColumns(rs, null);
+                    } catch (Exception e) {
+                        return Collections.emptyList();
+                    }
+                }
+            });
         }
         finally {
             // Restore original limits