Skip to content

Commit

Permalink
AF-1149: Dashbuilder not closing ResultSets and Statements leading to…
Browse files Browse the repository at this point in the history 
… ORA-01000 error
  • Loading branch information
@dgutierr
dgutierr committed Apr 11, 2018
1 parent ae4c145 commit 19361df
Show file tree
Hide file tree
Showing 7 changed files with 203 additions and 45 deletions.
11 changes: 11 additions & 0 deletions dashbuilder-backend/dashbuilder-dataset-sql/pom.xml
View file
Expand Up @@ -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>
16 changes: 7 additions & 9 deletions ...end/dashbuilder-dataset-sql/src/main/java/org/dashbuilder/dataprovider/sql/JDBCUtils.java
View file
Expand Up @@ -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;
Expand Down Expand Up @@ -58,14 +53,17 @@ 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();
}
}

Expand Down Expand Up @@ -162,7 +160,7 @@ public static Dialect dialect(String dbName) {
}

public static List<Column> getColumns(ResultSet resultSet, String[] exclude) throws SQLException {
List<Column> columnList = new ArrayList<Column>();
List<Column> columnList = new ArrayList<>();
List<String> columnExcluded = exclude == null ? new ArrayList<String>() : Arrays.asList(exclude);

ResultSetMetaData meta = resultSet.getMetaData();
Expand Down Expand Up @@ -203,7 +201,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];
Expand Down
53 changes: 34 additions & 19 deletions ...uilder-dataset-sql/src/main/java/org/dashbuilder/dataprovider/sql/SQLDataSetProvider.java
View file
Expand Up @@ -409,13 +409,20 @@ protected DataSetMetadata _getDataSetMetadata(SQLDataSetDef def, Connection conn

protected List<Column> _getColumns(SQLDataSetDef def, Connection conn) throws Exception {
Dialect dialect = JDBCUtils.dialect(conn);
ResultSet _rs = null;
if (!StringUtils.isBlank(def.getDbSQL())) {
Select query = SQLFactory.select(conn).from(def.getDbSQL()).limit(1);
return JDBCUtils.getColumns(logSQL(query).fetch(), dialect.getExcludedColumns());
_rs = logSQL(query).fetch();
}
else {
Select query = SQLFactory.select(conn).from(_createTable(def)).limit(1);
return JDBCUtils.getColumns(logSQL(query).fetch(), dialect.getExcludedColumns());
_rs = logSQL(query).fetch();
}
try {
return JDBCUtils.getColumns(_rs, dialect.getExcludedColumns());
} finally {
_rs.getStatement().close();
_rs.close();
}
}

Expand Down Expand Up @@ -670,13 +677,9 @@ 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;
ResultSet _results = logSQL(_query).fetch();
return buildDataSet(columns, _results, trim, totalRows);
}
// ... or a list of operations.
else {
Expand Down Expand Up @@ -737,19 +740,28 @@ 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;
ResultSet _results = logSQL(_query).fetch();
return buildDataSet(columns, _results, trim, totalRows);
}
} finally {
conn.close();
}
}

protected DataSet buildDataSet(List<DataColumn> columns, ResultSet _results, boolean trim, int totalRows) throws Exception {
try {
DataSet dataSet = _buildDataSet(columns, _results);
if (trim && postProcessingOps.isEmpty()) {
dataSet.setRowCountNonTrimmed(totalRows);
}
return dataSet;
} finally {
_results.getStatement().close();
_results.close();
}
}

protected DateIntervalType calculateDateInterval(ColumnGroup cg) {
if (dateIntervalType != null) {
return dateIntervalType;
Expand Down Expand Up @@ -800,12 +812,15 @@ protected Date calculateDateLimit(String dateColumnId, boolean min) {
.orderBy(min ? _dateColumn.asc() : _dateColumn.desc())
.limit(1)).fetch();

if (!rs.next()) {
return null;
} else {
return rs.getDate(1);
try {
return rs.next() ? rs.getDate(1) : null;
}
} catch (SQLException e) {
finally {
rs.getStatement().close();
rs.close();
}
}
catch (SQLException e) {
log.error("Error reading date limit from query results", e);
return null;
}
Expand Down
27 changes: 18 additions & 9 deletions ...-dataset-sql/src/main/java/org/dashbuilder/dataprovider/sql/dialect/SQLServerDialect.java
View file
Expand Up @@ -15,6 +15,7 @@
*/
package org.dashbuilder.dataprovider.sql.dialect;

import java.sql.ResultSet;
import java.sql.SQLException;
import java.text.SimpleDateFormat;
import java.util.Collections;
Expand Down Expand Up @@ -117,20 +118,28 @@ public String getSQL(Select select) {
}

public List<Column> fetchColumns(Select select) {
int offset = select.getOffset();
int limit = select.getLimit();
try {
// Disable limits & fetch results
select.limit(0).offset(0);
return JDBCUtils.getColumns(select.fetch(), null);
int offset = select.getOffset();
int limit = select.getLimit();
ResultSet rs = null;
try {
// Disable limits & fetch results
select.limit(0).offset(0);
rs = select.fetch();
return JDBCUtils.getColumns(rs, null);
}
finally {
// Restore original limits
select.limit(limit).offset(offset);
if (rs != null) {
rs.getStatement().close();
rs.close();
}
}
}
catch (SQLException e) {
return Collections.emptyList();
}
finally {
// Restore original limits
select.limit(limit).offset(offset);
}
}

@Override
Expand Down
9 changes: 5 additions & 4 deletions .../dashbuilder-dataset-sql/src/main/java/org/dashbuilder/dataprovider/sql/model/Select.java
View file
Expand Up @@ -171,10 +171,11 @@ public String toString() {
public int fetchCount() throws SQLException {
String countSql = dialect.getCountQuerySQL(this);
ResultSet _rs = JDBCUtils.executeQuery(connection, countSql);
if (_rs.next()) {
return _rs.getInt(1);
} else {
return 0;
try {
return _rs.next() ? _rs.getInt(1) : 0;
} finally {
_rs.getStatement().close();
_rs.close();
}
}

Expand Down
20 changes: 16 additions & 4 deletions ...dashbuilder-dataset-sql/src/test/java/org/dashbuilder/dataprovider/sql/JDBCUtilsTest.java
View file
Expand Up @@ -15,9 +15,7 @@
*/
package org.dashbuilder.dataprovider.sql;

import java.sql.ResultSet;
import java.sql.ResultSetMetaData;
import java.sql.Types;
import java.sql.*;
import java.util.List;

import org.dashbuilder.dataprovider.sql.JDBCUtils;
Expand All @@ -36,16 +34,30 @@
public class JDBCUtilsTest {

@Mock
ResultSet resultSet;
Connection connection;

@Mock
Statement statement;

@Mock
ResultSet resultSet;

@Mock
ResultSetMetaData metaData;

@Before
public void setUp() throws Exception {
when(connection.createStatement()).thenReturn(statement);
when(resultSet.getMetaData()).thenReturn(metaData);
}

@Test
public void testStatementClose() throws Exception {
JDBCUtils.execute(connection, "sql");
verify(statement).execute("sql");
verify(statement).close();
}

@Test
public void testFixSQLCase() throws Exception {
String sql = "SELECT \"ID\" FROM TABLE";
Expand Down
112 changes: 112 additions & 0 deletions ...der-dataset-sql/src/test/java/org/dashbuilder/dataprovider/sql/SQLCloseResourcesTest.java
View file
@@ -0,0 +1,112 @@
/**
* 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 org.dashbuilder.dataprovider.sql.dialect.Dialect;
import org.dashbuilder.dataprovider.sql.model.Column;
import org.dashbuilder.dataprovider.sql.model.Select;
import org.dashbuilder.dataset.DataSetLookup;
import org.dashbuilder.dataset.DataSetLookupFactory;
import org.dashbuilder.dataset.def.SQLDataSetDef;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.Mock;
import org.powermock.api.mockito.PowerMockito;
import org.powermock.core.classloader.annotations.PrepareForTest;
import org.powermock.modules.junit4.PowerMockRunner;

import javax.sql.DataSource;
import java.sql.Connection;
import java.sql.ResultSet;
import java.sql.ResultSetMetaData;
import java.sql.Statement;
import java.util.Arrays;

import static org.mockito.Matchers.any;
import static org.mockito.Matchers.anyString;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

@RunWith(PowerMockRunner.class)
@PrepareForTest(JDBCUtils.class)
public class SQLCloseResourcesTest {

@Mock
Connection connection;

@Mock
Statement statement;

@Mock
ResultSet resultSet;

@Mock
Dialect dialect;

@Mock
Select select;

@Mock
SQLDataSourceLocator dataSourceLocator;

@Mock
DataSource dataSource;

@Mock
ResultSetMetaData metaData;

SQLDataSetProvider sqlDataSetProvider;
SQLDataSetDef dataSetDef = new SQLDataSetDef();

@Before
public void setUp() throws Exception {
dataSetDef.setDataSource("test");
dataSetDef.setDbSQL("test");

sqlDataSetProvider = SQLDataSetProvider.get();
sqlDataSetProvider.setDataSourceLocator(dataSourceLocator);
when(dataSourceLocator.lookup(any(SQLDataSetDef.class))).thenReturn(dataSource);
when(dataSource.getConnection()).thenReturn(connection);

PowerMockito.mockStatic(JDBCUtils.class);
when(JDBCUtils.dialect(connection)).thenReturn(dialect);
when(JDBCUtils.executeQuery(any(Connection.class), anyString())).thenReturn(resultSet);
when(resultSet.getStatement()).thenReturn(statement);
when(select.fetch()).thenReturn(resultSet);
}

@Test
public void testGetColumns() throws Exception {
sqlDataSetProvider._getColumns(dataSetDef, connection);
verify(statement).close();
verify(resultSet).close();
}

@Test
public void testLookup() throws Exception {
DataSetLookup dataSetLookup = DataSetLookupFactory.newDataSetLookupBuilder()
.column("column1")
.buildLookup();

when(JDBCUtils.getColumns(resultSet, null)).thenReturn(Arrays.asList(new Column("column1")));
sqlDataSetProvider.lookupDataSet(dataSetDef, dataSetLookup);
verify(resultSet, times(3)).close();
verify(statement, times(3)).close();
verify(connection).close();
}
}

0 comments on commit 19361df

Please sign in to comment.