Skip to content

Commit

Permalink
fix: Update Oracle schema to use timezone-aware type (#457)
Browse files Browse the repository at this point in the history
* Update Oracle schema to use timezone-aware type
* Adds tests verifying that persisted instants are not affected by
changes to JVM timezone
* Add configuration option to `JdbcCustomization` to allow users to
explicitly choose to persist timestamps in UTC
  • Loading branch information
kagkarlsson committed Jan 13, 2024
1 parent a27383a commit 4d3b5bb
Show file tree
Hide file tree
Showing 22 changed files with 220 additions and 35 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,11 @@ public class AutodetectJdbcCustomization implements JdbcCustomization {
private final JdbcCustomization jdbcCustomization;

public AutodetectJdbcCustomization(DataSource dataSource) {
JdbcCustomization detectedCustomization = new DefaultJdbcCustomization();
this(dataSource, false);
}

public AutodetectJdbcCustomization(DataSource dataSource, boolean persistTimestampInUTC) {
JdbcCustomization detectedCustomization = new DefaultJdbcCustomization(persistTimestampInUTC);

LOG.debug("Detecting database...");
try (Connection c = dataSource.getConnection()) {
Expand All @@ -44,25 +48,25 @@ public AutodetectJdbcCustomization(DataSource dataSource) {

if (databaseProductName.equals(MICROSOFT_SQL_SERVER)) {
LOG.info("Using MSSQL jdbc-overrides.");
detectedCustomization = new MssqlJdbcCustomization();
detectedCustomization = new MssqlJdbcCustomization(persistTimestampInUTC);
} else if (databaseProductName.equals(POSTGRESQL)) {
LOG.info("Using PostgreSQL jdbc-overrides.");
detectedCustomization = new PostgreSqlJdbcCustomization();
detectedCustomization = new PostgreSqlJdbcCustomization(false, persistTimestampInUTC);
} else if (databaseProductName.contains(ORACLE)) {
LOG.info("Using Oracle jdbc-overrides.");
detectedCustomization = new OracleJdbcCustomization();
detectedCustomization = new OracleJdbcCustomization(persistTimestampInUTC);
} else if (databaseProductName.contains(MARIADB)) {
LOG.info("Using MariaDB jdbc-overrides.");
detectedCustomization = new MariaDBJdbcCustomization();
detectedCustomization = new MariaDBJdbcCustomization(persistTimestampInUTC);
} else if (databaseProductName.contains(MYSQL)) {
int databaseMajorVersion = c.getMetaData().getDatabaseMajorVersion();
String dbVersion = c.getMetaData().getDatabaseProductVersion();
if (databaseMajorVersion >= 8) {
LOG.info("Using MySQL jdbc-overrides version 8 and later. (v {})", dbVersion);
detectedCustomization = new MySQL8JdbcCustomization();
detectedCustomization = new MySQL8JdbcCustomization(persistTimestampInUTC);
} else {
LOG.info("Using MySQL jdbc-overrides for version older than 8. (v {})", dbVersion);
detectedCustomization = new MySQLJdbcCustomization();
detectedCustomization = new MySQLJdbcCustomization(persistTimestampInUTC);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,19 +19,41 @@
import java.sql.SQLException;
import java.sql.Timestamp;
import java.time.Instant;
import java.util.Calendar;
import java.util.GregorianCalendar;
import java.util.List;
import java.util.Optional;
import java.util.TimeZone;

public class DefaultJdbcCustomization implements JdbcCustomization {
private static final Calendar UTC = GregorianCalendar.getInstance(TimeZone.getTimeZone("CET"));
private final boolean persistTimestampInUTC;

public DefaultJdbcCustomization(boolean persistTimestampInUTC) {

this.persistTimestampInUTC = persistTimestampInUTC;
}

@Override
public void setInstant(PreparedStatement p, int index, Instant value) throws SQLException {
p.setTimestamp(index, value != null ? Timestamp.from(value) : null);
if (persistTimestampInUTC) {
p.setTimestamp(index, value != null ? Timestamp.from(value) : null, UTC);
} else {
p.setTimestamp(index, value != null ? Timestamp.from(value) : null);
}
}

@Override
public Instant getInstant(ResultSet rs, String columnName) throws SQLException {
return Optional.ofNullable(rs.getTimestamp(columnName)).map(Timestamp::toInstant).orElse(null);
if (persistTimestampInUTC) {
return Optional.ofNullable(rs.getTimestamp(columnName, UTC))
.map(Timestamp::toInstant)
.orElse(null);
} else {
return Optional.ofNullable(rs.getTimestamp(columnName))
.map(Timestamp::toInstant)
.orElse(null);
}
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,12 @@
import java.util.List;
import java.util.function.Supplier;

class JdbcTaskRepositoryContext {
final TaskResolver taskResolver;
final String tableName;
final SchedulerName schedulerName;
final JdbcRunner jdbcRunner;
final Supplier<ResultSetMapper<List<Execution>>> resultSetMapper;
public class JdbcTaskRepositoryContext {
public final TaskResolver taskResolver;
public final String tableName;
public final SchedulerName schedulerName;
public final JdbcRunner jdbcRunner;
public final Supplier<ResultSetMapper<List<Execution>>> resultSetMapper;

JdbcTaskRepositoryContext(
TaskResolver taskResolver,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@

public class MariaDBJdbcCustomization extends DefaultJdbcCustomization {

public MariaDBJdbcCustomization(boolean persistTimestampInUTC) {
super(persistTimestampInUTC);
}

@Override
public String getName() {
return "MariaDB";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,14 @@

public class MssqlJdbcCustomization extends DefaultJdbcCustomization {

public MssqlJdbcCustomization() {
super(false);
}

public MssqlJdbcCustomization(boolean persistTimestampInUTC) {
super(persistTimestampInUTC);
}

@Override
public String getName() {
return "MSSQL";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@

public class MySQL8JdbcCustomization extends DefaultJdbcCustomization {

public MySQL8JdbcCustomization(boolean persistTimestampInUTC) {
super(persistTimestampInUTC);
}

@Override
public String getName() {
return "MySQL => v8";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@

public class MySQLJdbcCustomization extends DefaultJdbcCustomization {

public MySQLJdbcCustomization(boolean persistTimestampInUTC) {
super(persistTimestampInUTC);
}

@Override
public String getName() {
return "MySQL < v8";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@

public class OracleJdbcCustomization extends DefaultJdbcCustomization {

public OracleJdbcCustomization(boolean persistTimestampInUTC) {
super(persistTimestampInUTC);
}

@Override
public String getName() {
return "Oracle";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,12 @@ public class PostgreSqlJdbcCustomization extends DefaultJdbcCustomization {
private final boolean useGenericLockAndFetch;

public PostgreSqlJdbcCustomization() {
this(false);
this(false, false);
}

public PostgreSqlJdbcCustomization(boolean useGenericLockAndFetch) {
public PostgreSqlJdbcCustomization(
boolean useGenericLockAndFetch, boolean persistTimestampInUTC) {
super(persistTimestampInUTC);
this.useGenericLockAndFetch = useGenericLockAndFetch;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ public ManualScheduler build() {
new JdbcTaskRepository(
dataSource,
true,
new DefaultJdbcCustomization(),
new DefaultJdbcCustomization(false),
tableName,
taskResolver,
new SchedulerName.Fixed("manual"),
Expand All @@ -85,7 +85,7 @@ public ManualScheduler build() {
new JdbcTaskRepository(
dataSource,
commitWhenAutocommitDisabled,
new DefaultJdbcCustomization(),
new DefaultJdbcCustomization(false),
tableName,
taskResolver,
new SchedulerName.Fixed("manual"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,10 @@
import java.time.Duration;
import java.time.Instant;
import java.util.ArrayList;
import java.util.GregorianCalendar;
import java.util.List;
import java.util.Optional;
import java.util.TimeZone;
import javax.sql.DataSource;
import org.hamcrest.collection.IsCollectionWithSize;
import org.junit.jupiter.api.AfterEach;
Expand All @@ -56,6 +58,7 @@ public abstract class CompatibilityTest {

private static final int POLLING_LIMIT = 10_000;
private final boolean supportsSelectForUpdate;
private final boolean shouldHavePersistentTimezone;

@RegisterExtension public StopSchedulerExtension stopScheduler = new StopSchedulerExtension();

Expand All @@ -68,8 +71,9 @@ public abstract class CompatibilityTest {
private TestableRegistry testableRegistry;
private ExecutionCompletedCondition completed12Condition;

public CompatibilityTest(boolean supportsSelectForUpdate) {
public CompatibilityTest(boolean supportsSelectForUpdate, boolean shouldHavePersistentTimezone) {
this.supportsSelectForUpdate = supportsSelectForUpdate;
this.shouldHavePersistentTimezone = shouldHavePersistentTimezone;
}

public abstract DataSource getDataSource();
Expand Down Expand Up @@ -216,6 +220,48 @@ public void test_jdbc_repository_select_for_update_compatibility() {
assertThat(jdbcTaskRepository.pick(picked.get(0), now), OptionalMatchers.empty());
}

@Test
public void test_has_peristent_time_zone() {
if (!shouldHavePersistentTimezone) {
return;
}
TaskResolver defaultTaskResolver = new TaskResolver(StatsRegistry.NOOP, new ArrayList<>());
defaultTaskResolver.addTask(oneTime);

JdbcTaskRepository winterTaskRepo =
new JdbcTaskRepository(
getDataSource(),
commitWhenAutocommitDisabled(),
new ZoneSpecificJdbcCustomization(
getJdbcCustomization().orElse(new AutodetectJdbcCustomization(getDataSource())),
GregorianCalendar.getInstance(TimeZone.getTimeZone("CET"))),
DEFAULT_TABLE_NAME,
defaultTaskResolver,
new SchedulerName.Fixed("scheduler1"),
new SystemClock());

JdbcTaskRepository summerTaskRepo =
new JdbcTaskRepository(
getDataSource(),
commitWhenAutocommitDisabled(),
new ZoneSpecificJdbcCustomization(
getJdbcCustomization().orElse(new AutodetectJdbcCustomization(getDataSource())),
GregorianCalendar.getInstance(TimeZone.getTimeZone("CEST"))),
DEFAULT_TABLE_NAME,
defaultTaskResolver,
new SchedulerName.Fixed("scheduler1"),
new SystemClock());

Instant noonFirstJan = Instant.parse("2020-01-01T12:00:00.00Z");

TaskInstance<String> instance1 = oneTime.instance("future1");
winterTaskRepo.createIfNotExists(SchedulableInstance.of(instance1, noonFirstJan));

assertThat(
winterTaskRepo.getExecution(instance1).get().executionTime,
is(summerTaskRepo.getExecution(instance1).get().executionTime));
}

private void doJDBCRepositoryCompatibilityTestUsingData(String data) {
TaskResolver taskResolver = new TaskResolver(StatsRegistry.NOOP, new ArrayList<>());
taskResolver.addTask(oneTime);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ public class MariaDB103CompatibilityTest extends CompatibilityTest {
private static HikariDataSource pooledDatasource;

public MariaDB103CompatibilityTest() {
super(false); // FIXLATER: fix syntax and enable
super(false, false); // FIXLATER: fix syntax and enable
}

@BeforeAll
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public class MssqlCompatibilityTest extends CompatibilityTest {
private static DataSource pooledDatasource;

public MssqlCompatibilityTest() {
super(true);
super(true, true);
}

@BeforeAll
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ public class Mysql5CompatibilityTest extends CompatibilityTest {
private static HikariDataSource pooledDatasource;

public Mysql5CompatibilityTest() {
super(false);
super(false, false);
}

@BeforeAll
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ public class Mysql8CompatibilityTest extends CompatibilityTest {
private static HikariDataSource pooledDatasource;

public Mysql8CompatibilityTest() {
super(false); // FIXLATER: fix syntax and enable
super(false, false); // FIXLATER: fix syntax and enable
}

@BeforeAll
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ public class NoAutoCommitPostgresqlCompatibilityTest extends CompatibilityTest {
private static HikariDataSource pooledDatasource;

public NoAutoCommitPostgresqlCompatibilityTest() {
super(true);
super(true, true);
}

@BeforeAll
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ public class Oracle11gCompatibilityTest extends CompatibilityTest {
private static HikariDataSource pooledDatasource;

public Oracle11gCompatibilityTest() {
super(false); // FIXLATER: fix syntax and enable
super(false, true); // FIXLATER: fix syntax and enable
}

@BeforeAll
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ public class PostgresqlCompatibilityTest extends CompatibilityTest {
// );

public PostgresqlCompatibilityTest() {
super(true);
super(true, true);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ public class PostgresqlGenericFetchAndLockCompatibilityTest extends Compatibilit
public EmbeddedPostgresqlExtension postgres = new EmbeddedPostgresqlExtension();

public PostgresqlGenericFetchAndLockCompatibilityTest() {
super(true);
super(true, true);
}

@Override
Expand All @@ -28,6 +28,6 @@ public boolean commitWhenAutocommitDisabled() {

@Override
public Optional<JdbcCustomization> getJdbcCustomization() {
return Optional.of(new PostgreSqlJdbcCustomization(true));
return Optional.of(new PostgreSqlJdbcCustomization(true, false));
}
}

0 comments on commit 4d3b5bb

Please sign in to comment.