diff --git a/api-tests/src/test/java/org/openmrs/module/reporting/common/ResultSetIteratorTest.java b/api-tests/src/test/java/org/openmrs/module/reporting/common/ResultSetIteratorTest.java index a4a1afb0c..27e18c091 100644 --- a/api-tests/src/test/java/org/openmrs/module/reporting/common/ResultSetIteratorTest.java +++ b/api-tests/src/test/java/org/openmrs/module/reporting/common/ResultSetIteratorTest.java @@ -4,7 +4,6 @@ import org.junit.Before; import org.junit.Test; import org.openmrs.module.reporting.dataset.DataSetRow; -import org.openmrs.module.reporting.dataset.definition.evaluator.SqlFileDataSetEvaluator; import javax.sql.rowset.RowSetMetaDataImpl; import java.sql.ResultSet; @@ -21,23 +20,19 @@ public class ResultSetIteratorTest { - //@Mock - private SqlFileDataSetEvaluator mockEvaluator; - - //@Mock private ResultSet mockResultSet; - //@Mock private Statement mockStatement; private ResultSetIterator resultSetIterator; @Before public void setUp() throws SQLException { - mockEvaluator = mock(SqlFileDataSetEvaluator.class); mockResultSet = mock(ResultSet.class); mockStatement = mock(Statement.class); - resultSetIterator = new ResultSetIterator(createMetadata(), mockResultSet, mockEvaluator, mockStatement); + when(mockResultSet.getStatement()).thenReturn(mockStatement); + when(mockResultSet.getMetaData()).thenReturn(createMetadata()); + resultSetIterator = new ResultSetIterator(mockResultSet); mockResultSet(); } diff --git a/api/src/main/java/org/openmrs/module/reporting/common/ResultSetIterator.java b/api/src/main/java/org/openmrs/module/reporting/common/ResultSetIterator.java index a0468abe0..2f8b77c5d 100644 --- a/api/src/main/java/org/openmrs/module/reporting/common/ResultSetIterator.java +++ b/api/src/main/java/org/openmrs/module/reporting/common/ResultSetIterator.java @@ -3,7 +3,10 @@ import org.openmrs.api.APIException; import org.openmrs.module.reporting.dataset.DataSetColumn; import org.openmrs.module.reporting.dataset.DataSetRow; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import java.sql.Connection; import java.sql.ResultSet; import java.sql.ResultSetMetaData; import java.sql.SQLException; @@ -13,25 +16,22 @@ import java.util.List; import java.util.NoSuchElementException; -import org.openmrs.module.reporting.dataset.definition.evaluator.SqlFileDataSetEvaluator; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - public class ResultSetIterator implements Iterator { + + private static final Logger log = LoggerFactory.getLogger(ResultSetIterator.class); + private ResultSet resultSet; private List columns; + private Connection connection; private Statement statement; - private SqlFileDataSetEvaluator evaluator; - private static final Logger log = LoggerFactory.getLogger(ResultSetIterator.class); private boolean isNextUsed = true; private boolean hasNext = true; - - public ResultSetIterator(ResultSetMetaData metadata, ResultSet resultSet, SqlFileDataSetEvaluator evaluator, Statement statement) throws SQLException { + public ResultSetIterator(ResultSet resultSet) throws SQLException { this.resultSet = resultSet; - this.evaluator = evaluator; - this.statement = statement; - this.createDataSetColumns(metadata); + this.statement = resultSet.getStatement(); + this.connection = resultSet.getStatement().getConnection(); + this.createDataSetColumns(resultSet.getMetaData()); } @Override @@ -95,7 +95,9 @@ public void closeConnection() { if (statement != null) { statement.close(); } - evaluator.closeConnection(); + if (connection != null) { + connection.close(); + } } catch (Exception ex) { log.error("Failed to close ResultSetIterator connection.", ex); } diff --git a/api/src/main/java/org/openmrs/module/reporting/common/SqlRunner.java b/api/src/main/java/org/openmrs/module/reporting/common/SqlRunner.java index 39b8dca10..a79f1e798 100644 --- a/api/src/main/java/org/openmrs/module/reporting/common/SqlRunner.java +++ b/api/src/main/java/org/openmrs/module/reporting/common/SqlRunner.java @@ -15,11 +15,11 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.openmrs.OpenmrsObject; -import org.openmrs.module.reporting.dataset.definition.evaluator.SqlFileDataSetEvaluator; import org.openmrs.module.reporting.report.util.ReportUtil; import java.io.File; import java.io.IOException; +import java.sql.Connection; import java.sql.ResultSet; import java.sql.ResultSetMetaData; import java.sql.Statement; @@ -43,17 +43,17 @@ public class SqlRunner { //*********** INSTANCE PROPERTIES ****************** - private SqlFileDataSetEvaluator evaluator; + private Connection connection; private String delimiter = ";"; //*********** CONSTRUCTORS ****************** - public SqlRunner(SqlFileDataSetEvaluator evaluator) { - this.evaluator = evaluator; + public SqlRunner(Connection connection) { + this.connection = connection; } - public SqlRunner(SqlFileDataSetEvaluator evaluator, String delimiter) { - this(evaluator); + public SqlRunner(Connection connection, String delimiter) { + this(connection); this.delimiter = delimiter; } @@ -114,13 +114,13 @@ public SqlResult executeSql(String sql, Map parameterValues) { Boolean originalAutoCommit = null; try { - originalAutoCommit = evaluator.getConnection().getAutoCommit(); - evaluator.getConnection().setAutoCommit(false); + originalAutoCommit = connection.getAutoCommit(); + connection.setAutoCommit(false); for (String sqlStatement : sqlStatements) { Statement statement = null; try { - statement = evaluator.getConnection().createStatement(); + statement = connection.createStatement(); log.debug("Executing: " + sqlStatement); statement.execute(sqlStatement); ResultSet resultSet = statement.getResultSet(); @@ -181,13 +181,13 @@ public ResultSetIterator executeSqlToIterator(String sql, Map pa Boolean originalAutoCommit = null; try { - originalAutoCommit = evaluator.getConnection().getAutoCommit(); - evaluator.getConnection().setAutoCommit(false); + originalAutoCommit = connection.getAutoCommit(); + connection.setAutoCommit(false); for (String sqlStatement : sqlStatements) { Statement statement = null; try { - statement = evaluator.getConnection().createStatement(); + statement = connection.createStatement(); // If is the last statement set setFetchSize if (sqlStatement.equals(sqlStatements.get(sqlStatements.size() - 1))) { statement.setFetchSize(10); @@ -197,8 +197,7 @@ public ResultSetIterator executeSqlToIterator(String sql, Map pa ResultSet resultSet = statement.getResultSet(); if (resultSet != null) { - ResultSetMetaData rsmd = resultSet.getMetaData(); - iterator = new ResultSetIterator(rsmd, resultSet, evaluator, statement); + iterator = new ResultSetIterator(resultSet); } } catch (Exception e) { closeStatement(statement); @@ -227,7 +226,7 @@ protected void closeStatement(Statement statement) { protected void commit() { try { - evaluator.getConnection().commit(); + connection.commit(); } catch (Exception e) { log.warn("An error occurred while trying to commit", e); @@ -236,7 +235,7 @@ protected void commit() { protected void rollback() { try { - evaluator.getConnection().rollback(); + connection.rollback(); } catch (Exception e) { log.warn("An error occurred while trying to rollback a connection", e); @@ -246,7 +245,7 @@ protected void rollback() { protected void resetAutocommit(Boolean autocommit) { try { if (autocommit != null) { - evaluator.getConnection().setAutoCommit(autocommit); + connection.setAutoCommit(autocommit); } } catch (Exception e) { @@ -373,6 +372,15 @@ protected String getNewDelimiter(String line) { } //************* PROPERTY ACCESS ******************* + + public Connection getConnection() { + return connection; + } + + public void setConnection(Connection connection) { + this.connection = connection; + } + public String getDelimiter() { return delimiter; } diff --git a/api/src/main/java/org/openmrs/module/reporting/dataset/definition/evaluator/DataSetEvaluator.java b/api/src/main/java/org/openmrs/module/reporting/dataset/definition/evaluator/DataSetEvaluator.java index eea2489bd..64e0011ef 100644 --- a/api/src/main/java/org/openmrs/module/reporting/dataset/definition/evaluator/DataSetEvaluator.java +++ b/api/src/main/java/org/openmrs/module/reporting/dataset/definition/evaluator/DataSetEvaluator.java @@ -30,7 +30,7 @@ public interface DataSetEvaluator extends DefinitionEvaluator * Evaluate a DataSet for the given EvaluationContext * * @param dataSetDefinition - * @param inputCohortencounter_datetime + * @param evalContext * @return the evaluated DataSet */ public DataSet evaluate(DataSetDefinition dataSetDefinition, EvaluationContext evalContext) throws EvaluationException; diff --git a/api/src/main/java/org/openmrs/module/reporting/dataset/definition/evaluator/IterableSqlDataSetEvaluator.java b/api/src/main/java/org/openmrs/module/reporting/dataset/definition/evaluator/IterableSqlDataSetEvaluator.java index 808c2a05b..05ea94500 100644 --- a/api/src/main/java/org/openmrs/module/reporting/dataset/definition/evaluator/IterableSqlDataSetEvaluator.java +++ b/api/src/main/java/org/openmrs/module/reporting/dataset/definition/evaluator/IterableSqlDataSetEvaluator.java @@ -10,11 +10,9 @@ package org.openmrs.module.reporting.dataset.definition.evaluator; import org.apache.commons.lang.StringUtils; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import org.openmrs.annotation.Handler; -import org.openmrs.api.context.Context; import org.openmrs.module.reporting.common.ObjectUtil; +import org.openmrs.module.reporting.common.ResultSetIterator; import org.openmrs.module.reporting.common.SqlRunner; import org.openmrs.module.reporting.dataset.DataSet; import org.openmrs.module.reporting.dataset.IterableSqlDataSet; @@ -22,12 +20,11 @@ import org.openmrs.module.reporting.dataset.definition.IterableSqlDataSetDefinition; import org.openmrs.module.reporting.evaluation.EvaluationContext; import org.openmrs.module.reporting.evaluation.EvaluationException; -import org.openmrs.module.reporting.common.ResultSetIterator; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import java.io.File; import java.sql.Connection; -import java.sql.DriverManager; -import java.util.Iterator; import java.util.Map; import java.util.Properties; @@ -41,12 +38,6 @@ public class IterableSqlDataSetEvaluator extends SqlFileDataSetEvaluator { private static final Logger log = LoggerFactory.getLogger(IterableSqlDataSetEvaluator.class); - /** - * Public constructor - */ - public IterableSqlDataSetEvaluator() { - } - /** * @should evaluate a IterableSqlDataSetDefinition * @should evaluate a IterableSqlDataSetDefinition with parameters @@ -58,17 +49,12 @@ public IterableSqlDataSetEvaluator() { public DataSet evaluate(DataSetDefinition dataSetDefinition, EvaluationContext context) throws EvaluationException { context = ObjectUtil.nvl(context, new EvaluationContext()); - IterableSqlDataSetDefinition sqlDsd = (IterableSqlDataSetDefinition) dataSetDefinition; - String sqlQuery = sqlDsd.getSql(); - IterableSqlDataSetDefinition definition = (IterableSqlDataSetDefinition) dataSetDefinition; - Properties connectionProperties = getConnectionProperties(definition.getConnectionPropertyFile()); - Iterator iterator = null; - Connection connection = null; - + ResultSetIterator iterator; + Connection connection; try { - createConnection(connectionProperties); - SqlRunner runner = new SqlRunner(this); + connection = getConnection(definition, context); + SqlRunner runner = new SqlRunner(connection); Map parameterValues = constructParameterValues(definition, context); if (StringUtils.isNotBlank(definition.getSqlFile())) { @@ -87,32 +73,27 @@ public DataSet evaluate(DataSetDefinition dataSetDefinition, EvaluationContext c } else { throw new EvaluationException("A SqlFileDataSetDefinition must define either a SQL File or SQL Resource"); } - - } catch (EvaluationException ee) { + } + catch (EvaluationException ee) { throw ee; - } catch (Exception e) { + } + catch (Exception e) { throw new EvaluationException("An error occurred while evaluating a SqlFileDataSetDefinition", e); } - return new IterableSqlDataSet(context, sqlDsd, (ResultSetIterator) iterator); + return new IterableSqlDataSet(context, definition, iterator); } /** * @return a new connection given a set of connection properties */ @Override - protected void createConnection(Properties connectionProperties) throws EvaluationException { - try { - String driver = connectionProperties.getProperty("connection.driver_class", "com.mysql.jdbc.Driver"); - String url = connectionProperties.getProperty("connection.url"); - url += "&useCursorFetch=true"; - String user = connectionProperties.getProperty("connection.username"); - String password = connectionProperties.getProperty("connection.password"); - Context.loadClass(driver); - this.connection = DriverManager.getConnection(url, user, password); - } catch (Exception e) { - throw new EvaluationException("Unable to create a new connection to the database", e); + protected Properties getConnectionProperties(String connectionPropertyFile) throws EvaluationException { + Properties p = super.getConnectionProperties(connectionPropertyFile); + String url = p.getProperty("connection.url"); + if (url != null && !url.contains("useCursorFetch")) { + p.setProperty("connection.url", url + "&useCursorFetch=true"); } + return p; } - } \ No newline at end of file diff --git a/api/src/main/java/org/openmrs/module/reporting/dataset/definition/evaluator/SqlFileDataSetEvaluator.java b/api/src/main/java/org/openmrs/module/reporting/dataset/definition/evaluator/SqlFileDataSetEvaluator.java index 6728630ff..6c9d0de44 100644 --- a/api/src/main/java/org/openmrs/module/reporting/dataset/definition/evaluator/SqlFileDataSetEvaluator.java +++ b/api/src/main/java/org/openmrs/module/reporting/dataset/definition/evaluator/SqlFileDataSetEvaluator.java @@ -50,8 +50,6 @@ public class SqlFileDataSetEvaluator implements DataSetEvaluator { protected Log log = LogFactory.getLog(this.getClass()); - protected Connection connection; - /** * @see DataSetEvaluator#evaluate(DataSetDefinition, EvaluationContext) */ @@ -62,11 +60,10 @@ public DataSet evaluate(DataSetDefinition dataSetDefinition, EvaluationContext c SqlFileDataSetDefinition dsd = (SqlFileDataSetDefinition) dataSetDefinition; - Properties connectionProperties = getConnectionProperties(dsd.getConnectionPropertyFile()); + Connection connection = null; try { - createConnection(connectionProperties); - - SqlRunner runner = new SqlRunner(this); + connection = getConnection(dsd, context); + SqlRunner runner = new SqlRunner(connection); SqlResult resultData = null; Map parameterValues = constructParameterValues(dsd, context); @@ -104,12 +101,21 @@ public DataSet evaluate(DataSetDefinition dataSetDefinition, EvaluationContext c } data.addRow(row); } - } catch (EvaluationException ee) { + } + catch (EvaluationException ee) { throw ee; - } catch (Exception e) { + } + catch (Exception e) { throw new EvaluationException("An error occurred while evaluating a SqlFileDataSetDefinition", e); - } finally { - closeConnection(); + } + finally { + try { + if (connection != null) { + connection.close(); + } + } catch (Exception e) { + log.warn("Error closing the database connection for SqlFileDataSetEvaluator", e); + } } return data; @@ -139,15 +145,17 @@ protected Properties getConnectionProperties(String connectionPropertyFile) thro /** * @return a new connection given a set of connection properties */ - protected void createConnection(Properties connectionProperties) throws EvaluationException { + protected Connection getConnection(SqlFileDataSetDefinition dsd, EvaluationContext context) throws EvaluationException { + Properties connectionProperties = getConnectionProperties(dsd.getConnectionPropertyFile()); try { String driver = connectionProperties.getProperty("connection.driver_class", "com.mysql.jdbc.Driver"); String url = connectionProperties.getProperty("connection.url"); String user = connectionProperties.getProperty("connection.username"); String password = connectionProperties.getProperty("connection.password"); Context.loadClass(driver); - connection = DriverManager.getConnection(url, user, password); - } catch (Exception e) { + return DriverManager.getConnection(url, user, password); + } + catch (Exception e) { throw new EvaluationException("Unable to create a new connection to the database", e); } } @@ -172,18 +180,4 @@ protected Map constructParameterValues(SqlFileDataSetDefinition } return ret; } - - public void closeConnection() { - try { - if (connection != null) { - connection.close(); - } - } catch (Exception e) { - log.warn("Error closing the database connection for SqlFileDataSetEvaluator", e); - } - } - - public Connection getConnection() { - return connection; - } }