Skip to content

Commit

Permalink
Merge pull request #2748 from IBM/issue-2747-utc-calendar
Browse files Browse the repository at this point in the history
issue-2747 - thread safety for JDBC get/set timestamp UTC
  • Loading branch information
lmsurpre authored Sep 8, 2021
2 parents ad00a02 + 514bb92 commit af70782
Show file tree
Hide file tree
Showing 18 changed files with 130 additions and 50 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
* for looser coupling.
*/
public class BindVisitor implements BindMarkerNodeVisitor {
private static final Calendar UTC = Calendar.getInstance(TimeZone.getTimeZone("UTC"));
private static final Logger logger = Logger.getLogger(BindVisitor.class.getName());

// the statement to bind values to
Expand All @@ -40,6 +39,9 @@ public class BindVisitor implements BindMarkerNodeVisitor {
// Keep track of the parameter index
private int parameterIndex = 1;

// Do not make static - Calendar is not thread-safe
private final Calendar UTC = Calendar.getInstance(TimeZone.getTimeZone("UTC"));

/**
* Public constructor
* @param ps
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,4 @@ public class SchemaConstants {
public static final String OBJECT_TYPE = "OBJECT_TYPE";
public static final String OBJECT_NAME = "OBJECT_NAME";
public static final String SCHEMA_NAME = "SCHEMA_NAME";

public static final Calendar UTC = Calendar.getInstance(TimeZone.getTimeZone("UTC"));

}
Original file line number Diff line number Diff line change
Expand Up @@ -119,9 +119,11 @@ public class JDBCConstants {
public static final String DEFAULT_TOKEN_SYSTEM = "default-token-system";

/**
* Calendar object to use while inserting Timestamp objects into the database.
* This Calendar object is not thread-safe! Use CalendarHelper#getCalendarForUTC() instead.
*/
public static final Calendar UTC = Calendar.getInstance(TimeZone.getTimeZone("UTC"));
// DO NOT USE
// public static final Calendar UTC = Calendar.getInstance(TimeZone.getTimeZone("UTC"));
// DO NOT USE

/**
* Maps search parameter types to the currently supported list of modifiers for that type.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import com.ibm.fhir.persistence.jdbc.dao.impl.ResourceDAOImpl;
import com.ibm.fhir.persistence.jdbc.dto.ExtractedParameterValue;
import com.ibm.fhir.persistence.jdbc.impl.ParameterTransactionDataImpl;
import com.ibm.fhir.persistence.jdbc.util.CalendarHelper;
import com.ibm.fhir.persistence.jdbc.util.ParameterTableSupport;

/**
Expand Down Expand Up @@ -148,7 +149,7 @@ protected ResourceIndexRecord getResource(Instant reindexTstamp, Long logicalRes
if (logicalResourceId != null) {
// specific resource by logical resource ID (primary key)
stmt.setLong(1, logicalResourceId);
stmt.setTimestamp(2, Timestamp.from(reindexTstamp));
stmt.setTimestamp(2, Timestamp.from(reindexTstamp), CalendarHelper.getCalendarForUTC());
}
ResultSet rs = stmt.executeQuery();
if (rs.next()) {
Expand Down Expand Up @@ -208,13 +209,13 @@ protected ResourceIndexRecord getNextResource(SecureRandom random, Instant reind
if (resourceTypeId != null && logicalId != null) {
stmt.setInt(1, resourceTypeId);
stmt.setString(2, logicalId);
stmt.setTimestamp(3, Timestamp.from(reindexTstamp));
stmt.setTimestamp(3, Timestamp.from(reindexTstamp), CalendarHelper.getCalendarForUTC());
} else if (resourceTypeId != null) {
stmt.setInt(1, resourceTypeId);
stmt.setTimestamp(2, Timestamp.from(reindexTstamp));
stmt.setTimestamp(2, Timestamp.from(reindexTstamp), CalendarHelper.getCalendarForUTC());
stmt.setInt(3, offset);
} else {
stmt.setTimestamp(1, Timestamp.from(reindexTstamp));
stmt.setTimestamp(1, Timestamp.from(reindexTstamp), CalendarHelper.getCalendarForUTC());
stmt.setInt(2, offset);
}
ResultSet rs = stmt.executeQuery();
Expand All @@ -237,7 +238,7 @@ protected ResourceIndexRecord getNextResource(SecureRandom random, Instant reind
+ " AND reindex_txid = ? "; // make sure we have the txid we selected above

try (PreparedStatement stmt = connection.prepareStatement(UPDATE)) {
stmt.setTimestamp(1, Timestamp.from(reindexTstamp));
stmt.setTimestamp(1, Timestamp.from(reindexTstamp), CalendarHelper.getCalendarForUTC());
stmt.setLong(2, result.getTransactionId() + 1L);
stmt.setLong(3, result.getLogicalResourceId());
stmt.setLong(4, result.getTransactionId());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import com.ibm.fhir.persistence.jdbc.exception.FHIRPersistenceDBCleanupException;
import com.ibm.fhir.persistence.jdbc.exception.FHIRPersistenceDBConnectException;
import com.ibm.fhir.persistence.jdbc.exception.FHIRPersistenceDataAccessException;
import com.ibm.fhir.persistence.jdbc.util.CalendarHelper;

/**
* This class is a root Data Access Object for managing JDBC access to the FHIR database.
Expand Down Expand Up @@ -200,7 +201,7 @@ protected List<Resource> runQuery(String sql, Object... searchArgs)
// Inject arguments into the prepared stmt.
for (int i = 0; i < searchArgs.length; i++) {
if (searchArgs[i] instanceof Timestamp) {
stmt.setTimestamp(i + 1, (Timestamp) searchArgs[i], JDBCConstants.UTC);
stmt.setTimestamp(i + 1, (Timestamp) searchArgs[i], CalendarHelper.getCalendarForUTC());
} else {
stmt.setObject(i + 1, searchArgs[i]);
}
Expand Down Expand Up @@ -263,7 +264,7 @@ protected int runCountQuery(String sql, Object... searchArgs)
// Inject arguments into the prepared stmt.
for (int i = 0; i < searchArgs.length; i++) {
if (searchArgs[i] instanceof Timestamp) {
stmt.setTimestamp(i + 1, (Timestamp) searchArgs[i], JDBCConstants.UTC);
stmt.setTimestamp(i + 1, (Timestamp) searchArgs[i], CalendarHelper.getCalendarForUTC());
} else {
stmt.setObject(i + 1, searchArgs[i]);
}
Expand Down Expand Up @@ -474,7 +475,7 @@ protected List<String> runQuery_STR_VALUES(String sql, Object... searchArgs)
// Inject arguments into the prepared stmt.
for (int i = 0; i < searchArgs.length; i++) {
if (searchArgs[i] instanceof Timestamp) {
stmt.setTimestamp(i + 1, (Timestamp) searchArgs[i], JDBCConstants.UTC);
stmt.setTimestamp(i + 1, (Timestamp) searchArgs[i], CalendarHelper.getCalendarForUTC());
} else {
stmt.setObject(i + 1, searchArgs[i]);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@

import com.ibm.fhir.persistence.ResourcePayload;
import com.ibm.fhir.persistence.exception.FHIRPersistenceException;
import com.ibm.fhir.persistence.jdbc.util.CalendarHelper;

/**
* DAO to fetch the payload objects for a list of resource ids
Expand Down Expand Up @@ -107,7 +108,7 @@ public void run(Connection c) throws FHIRPersistenceException {
// we can save a ton of CPU. The stream is closed by ResultSet (according to the docs). ResultSet
// will be closed when the PreparedStatement is closed
String logicalId = rs.getString(1);
Instant lastUpdated = Instant.ofEpochMilli(rs.getTimestamp(2).getTime());
Instant lastUpdated = rs.getTimestamp(2, CalendarHelper.getCalendarForUTC()).toInstant();
long resourceId = rs.getLong(3);
InputStream is = new GZIPInputStream(rs.getBinaryStream(4));
ResourcePayload rp = new ResourcePayload(logicalId, lastUpdated, resourceId, is);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,13 @@
import com.ibm.fhir.persistence.ResourceChangeLogRecord;
import com.ibm.fhir.persistence.ResourceChangeLogRecord.ChangeType;
import com.ibm.fhir.persistence.exception.FHIRPersistenceException;
import com.ibm.fhir.persistence.jdbc.util.CalendarHelper;

/**
* Simple DAO to read records from the RESOURCE_CHANGE_LOG table
*/
public class FetchResourceChangesDAO {
private static final Logger logger = Logger.getLogger(FetchResourceChangesDAO.class.getName());
private static final Calendar UTC_CALENDAR = Calendar.getInstance(TimeZone.getTimeZone("UTC"));

private final IDatabaseTranslator translator;
private final String schemaName;
Expand Down Expand Up @@ -114,7 +114,7 @@ public List<ResourceChangeLogRecord> run(Connection c) throws FHIRPersistenceExc
try (PreparedStatement ps = c.prepareStatement(SQL)) {
int a = 1;
if (this.fromTstamp != null) {
ps.setTimestamp(a++, Timestamp.from(this.fromTstamp), UTC_CALENDAR);
ps.setTimestamp(a++, Timestamp.from(this.fromTstamp), CalendarHelper.getCalendarForUTC());
}

if (this.afterResourceId != null) {
Expand All @@ -135,7 +135,7 @@ public List<ResourceChangeLogRecord> run(Connection c) throws FHIRPersistenceExc
default:
throw new FHIRPersistenceException("Invalid ChangeType in change log"); // DBA can find the bad row if it ever happens
}
ResourceChangeLogRecord record = new ResourceChangeLogRecord(rs.getString(2), rs.getString(3), rs.getInt(5), rs.getLong(1), rs.getTimestamp(4, UTC_CALENDAR).toInstant(), ct);
ResourceChangeLogRecord record = new ResourceChangeLogRecord(rs.getString(2), rs.getString(3), rs.getInt(5), rs.getLong(1), rs.getTimestamp(4, CalendarHelper.getCalendarForUTC()).toInstant(), ct);
result.add(record);
}
} catch (SQLException x) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import com.ibm.fhir.persistence.ResourcePayload;
import com.ibm.fhir.persistence.exception.FHIRPersistenceException;
import com.ibm.fhir.persistence.jdbc.exception.FHIRPersistenceDataAccessException;
import com.ibm.fhir.persistence.jdbc.util.CalendarHelper;

/**
* DAO to fetch resource ids using a time range and optional current resource id as a filter.
Expand All @@ -36,8 +37,6 @@
public class FetchResourcePayloadsDAO {
private static final Logger logger = Logger.getLogger(FetchResourcePayloadsDAO.class.getName());

private static final Calendar UTC_CALENDAR = Calendar.getInstance(TimeZone.getTimeZone("UTC"));

// The FHIR data schema name
private final String schemaName;

Expand Down Expand Up @@ -118,19 +117,19 @@ public ResourcePayload run(Connection c) throws FHIRPersistenceException {

// Set the variables marking the start point of the scan
if (this.fromLastUpdated != null) {
ps.setTimestamp(a++, Timestamp.from(this.fromLastUpdated), UTC_CALENDAR);
ps.setTimestamp(a++, Timestamp.from(this.fromLastUpdated), CalendarHelper.getCalendarForUTC());
}

// And where we want the scan to stop (e.g. exporting a limited time range)
if (this.toLastUpdated != null) {
ps.setTimestamp(a++, Timestamp.from(this.toLastUpdated), UTC_CALENDAR);
ps.setTimestamp(a++, Timestamp.from(this.toLastUpdated), CalendarHelper.getCalendarForUTC());
}

ResultSet rs = ps.executeQuery();
while (rs.next()) {
// make sure we get the timestamp as a UTC value
String logicalId = rs.getString(1);
Instant lastUpdated = rs.getTimestamp(2, UTC_CALENDAR).toInstant();
Instant lastUpdated = rs.getTimestamp(2, CalendarHelper.getCalendarForUTC()).toInstant();
long resourceId = rs.getLong(3);
InputStream is = new GZIPInputStream(rs.getBinaryStream(4));
result = new ResourcePayload(logicalId, lastUpdated, resourceId, is);
Expand Down Expand Up @@ -192,12 +191,12 @@ public int count(Connection c) throws FHIRPersistenceException {

// Set the variables marking the start point of the scan
if (this.fromLastUpdated != null) {
ps.setTimestamp(a++, Timestamp.from(fromLastUpdated), UTC_CALENDAR);
ps.setTimestamp(a++, Timestamp.from(fromLastUpdated), CalendarHelper.getCalendarForUTC());
}

// And where we want the scan to stop (e.g. exporting a limited time range)
if (this.toLastUpdated != null) {
ps.setTimestamp(a++, Timestamp.from(this.toLastUpdated), UTC_CALENDAR);
ps.setTimestamp(a++, Timestamp.from(this.toLastUpdated), CalendarHelper.getCalendarForUTC());
}

ResultSet rs = ps.executeQuery();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
package com.ibm.fhir.persistence.jdbc.dao.impl;

import static com.ibm.fhir.config.FHIRConfiguration.PROPERTY_SEARCH_ENABLE_LEGACY_WHOLE_SYSTEM_SEARCH_PARAMS;
import static com.ibm.fhir.persistence.jdbc.JDBCConstants.UTC;
import static com.ibm.fhir.search.SearchConstants.PROFILE;
import static com.ibm.fhir.search.SearchConstants.SECURITY;
import static com.ibm.fhir.search.SearchConstants.TAG;
Expand All @@ -19,6 +18,7 @@
import java.sql.Timestamp;
import java.sql.Types;
import java.util.ArrayList;
import java.util.Calendar;
import java.util.List;
import java.util.logging.Level;
import java.util.logging.Logger;
Expand All @@ -40,6 +40,7 @@
import com.ibm.fhir.persistence.jdbc.dto.TokenParmVal;
import com.ibm.fhir.persistence.jdbc.exception.FHIRPersistenceDataAccessException;
import com.ibm.fhir.persistence.jdbc.impl.ParameterTransactionDataImpl;
import com.ibm.fhir.persistence.jdbc.util.CalendarHelper;
import com.ibm.fhir.persistence.jdbc.util.CanonicalSupport;
import com.ibm.fhir.schema.control.FhirSchemaConstants;
import com.ibm.fhir.search.util.ReferenceValue;
Expand Down Expand Up @@ -415,6 +416,7 @@ public void visit(DateParmVal param) throws FHIRPersistenceException {
}

private void setDateParms(PreparedStatement insert, int parameterNameId, Timestamp dateStart, Timestamp dateEnd) throws SQLException {
final Calendar UTC = CalendarHelper.getCalendarForUTC();
insert.setInt(1, parameterNameId);
insert.setTimestamp(2, dateStart, UTC);
insert.setTimestamp(3, dateEnd, UTC);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@

import static com.ibm.fhir.persistence.jdbc.JDBCConstants.END;
import static com.ibm.fhir.persistence.jdbc.JDBCConstants.THEN;
import static com.ibm.fhir.persistence.jdbc.JDBCConstants.UTC;
import static com.ibm.fhir.persistence.jdbc.JDBCConstants.WHEN;

import java.sql.CallableStatement;
Expand Down Expand Up @@ -48,6 +47,7 @@
import com.ibm.fhir.persistence.jdbc.exception.FHIRPersistenceDataAccessException;
import com.ibm.fhir.persistence.jdbc.exception.FHIRPersistenceFKVException;
import com.ibm.fhir.persistence.jdbc.impl.ParameterTransactionDataImpl;
import com.ibm.fhir.persistence.jdbc.util.CalendarHelper;
import com.ibm.fhir.persistence.jdbc.util.ResourceTypesCache;
import com.ibm.fhir.persistence.jdbc.util.ResourceTypesCacheUpdater;
import com.ibm.fhir.persistence.jdbc.util.SqlQueryData;
Expand Down Expand Up @@ -262,7 +262,7 @@ protected Resource createDTO(ResultSet resultSet) throws FHIRPersistenceDataAcce
}
resource.setId(resultSet.getLong(IDX_RESOURCE_ID));
resource.setLogicalResourceId(resultSet.getLong(IDX_LOGICAL_RESOURCE_ID));
resource.setLastUpdated(resultSet.getTimestamp(IDX_LAST_UPDATED));
resource.setLastUpdated(resultSet.getTimestamp(IDX_LAST_UPDATED, CalendarHelper.getCalendarForUTC()));
resource.setLogicalId(resultSet.getString(IDX_LOGICAL_ID));
resource.setVersionId(resultSet.getInt(IDX_VERSION_ID));
resource.setDeleted(resultSet.getString(IDX_IS_DELETED).equals("Y") ? true : false);
Expand Down Expand Up @@ -466,7 +466,7 @@ public List<Long> searchForIds(SqlQueryData queryData) throws FHIRPersistenceDat
for (int i = 0; i < queryData.getBindVariables().size(); i++) {
Object object = queryData.getBindVariables().get(i);
if (object instanceof Timestamp) {
stmt.setTimestamp(i + 1, (Timestamp) object, JDBCConstants.UTC);
stmt.setTimestamp(i + 1, (Timestamp) object, CalendarHelper.getCalendarForUTC());
} else {
stmt.setObject(i + 1, object);
}
Expand Down Expand Up @@ -582,7 +582,7 @@ public Resource insert(Resource resource, List<ExtractedParameterValue> paramete
}

lastUpdated = resource.getLastUpdated();
stmt.setTimestamp(4, lastUpdated, UTC);
stmt.setTimestamp(4, lastUpdated, CalendarHelper.getCalendarForUTC());
stmt.setString(5, resource.isDeleted() ? "Y": "N");
stmt.setInt(6, resource.getVersionId());
stmt.setString(7, parameterHashB64);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,13 @@
import com.ibm.fhir.database.utils.api.IDatabaseTranslator;
import com.ibm.fhir.persistence.exception.FHIRPersistenceException;
import com.ibm.fhir.persistence.jdbc.FHIRPersistenceJDBCCache;
import com.ibm.fhir.persistence.jdbc.util.CalendarHelper;

/**
* Simple DAO to retrieve index IDs (i.e. logical resource IDs) from the LOGICAL_RESOURCES table.
*/
public class RetrieveIndexDAO {
private static final Logger logger = Logger.getLogger(RetrieveIndexDAO.class.getName());
private static final Calendar UTC_CALENDAR = Calendar.getInstance(TimeZone.getTimeZone("UTC"));

private final IDatabaseTranslator translator;
private final String schemaName;
private final String resourceTypeName;
Expand Down Expand Up @@ -109,7 +108,7 @@ public List<Long> run(Connection c) throws FHIRPersistenceException {
ps.setString(i++, resourceTypeName);
}
if (notModifiedAfter != null) {
ps.setTimestamp(i++, Timestamp.from(notModifiedAfter), UTC_CALENDAR);
ps.setTimestamp(i++, Timestamp.from(notModifiedAfter), CalendarHelper.getCalendarForUTC());
}
if (afterLogicalResourceId != null) {
ps.setLong(i++, afterLogicalResourceId);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,14 @@

package com.ibm.fhir.persistence.jdbc.derby;

import static com.ibm.fhir.persistence.jdbc.JDBCConstants.UTC;

import java.io.InputStream;
import java.sql.Connection;
import java.sql.PreparedStatement;
import java.sql.ResultSet;
import java.sql.SQLException;
import java.sql.SQLIntegrityConstraintViolationException;
import java.sql.Timestamp;
import java.util.Calendar;
import java.util.List;
import java.util.UUID;
import java.util.logging.Level;
Expand All @@ -40,6 +39,7 @@
import com.ibm.fhir.persistence.jdbc.exception.FHIRPersistenceDataAccessException;
import com.ibm.fhir.persistence.jdbc.exception.FHIRPersistenceFKVException;
import com.ibm.fhir.persistence.jdbc.impl.ParameterTransactionDataImpl;
import com.ibm.fhir.persistence.jdbc.util.CalendarHelper;
import com.ibm.fhir.persistence.jdbc.util.ParameterTableSupport;
import com.ibm.fhir.persistence.jdbc.util.ResourceTypesCache;

Expand Down Expand Up @@ -207,6 +207,7 @@ public Resource insert(Resource resource, List<ExtractedParameterValue> paramet
public long storeResource(String tablePrefix, List<ExtractedParameterValue> parameters, String p_logical_id, InputStream p_payload, Timestamp p_last_updated, boolean p_is_deleted,
String p_source_key, Integer p_version, String p_parameterHashB64, Connection conn, ParameterDAO parameterDao) throws Exception {

final Calendar UTC = CalendarHelper.getCalendarForUTC();
final String METHODNAME = "storeResource() for " + tablePrefix + " resource";
logger.entering(CLASSNAME, METHODNAME);

Expand Down
Loading

0 comments on commit af70782

Please sign in to comment.