REPORT-895 - Remove use of instance variable for connection object (#240

)

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();
  }
 

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<DataSetRow> {
+
+ private static final Logger log = LoggerFactory.getLogger(ResultSetIterator.class);
+
  private ResultSet resultSet;
  private List<DataSetColumn> 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);
  }

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<String, Object> 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<String, Object> 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<String, Object> 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;
  }

api/src/main/java/org/openmrs/module/reporting/dataset/definition/evaluator/DataSetEvaluator.java

@@ -30,7 +30,7 @@ public interface DataSetEvaluator extends DefinitionEvaluator<DataSetDefinition>
  * Evaluate a DataSet for the given EvaluationContext
  * 
  * @param dataSetDefinition
- * @param inputCohortencounter_datetime
+ * @param evalContext
  * @return the evaluated <code>DataSet</code>
  */
  public DataSet evaluate(DataSetDefinition dataSetDefinition, EvaluationContext evalContext) throws EvaluationException;

api/src/main/java/org/openmrs/module/reporting/dataset/definition/evaluator/IterableSqlDataSetEvaluator.java

@@ -10,24 +10,21 @@
 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;
 import org.openmrs.module.reporting.dataset.definition.DataSetDefinition;
 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<String, Object> 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;
  }
-
 }