Skip to content

Commit

Permalink
Remove all remnants of old ZoneId code. Add more unit tests.
Browse files Browse the repository at this point in the history
  • Loading branch information
@lukedegruchy
lukedegruchy committed Oct 27, 2023
1 parent bac08ff commit 82ad161
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 139 deletions.
135 changes: 12 additions & 123 deletions Src/java/engine/src/main/java/org/opencds/cqf/cql/engine/runtime/DateTime.java
View file
Original file line number Diff line number Diff line change
@@ -1,24 +1,17 @@
package org.opencds.cqf.cql.engine.runtime;

import org.opencds.cqf.cql.engine.exception.InvalidDateTime;

import java.math.BigDecimal;
import java.math.RoundingMode;
import java.time.*;
import java.time.temporal.ChronoField;
import java.time.LocalDateTime;
import java.time.OffsetDateTime;
import java.time.ZoneOffset;
import java.util.Calendar;
import java.util.Date;
import java.util.Set;
import java.util.TimeZone;
import java.util.function.Predicate;
import java.util.stream.Collectors;

import org.opencds.cqf.cql.engine.exception.InvalidDateTime;

public class DateTime extends BaseTemporal {
private final ZoneId zoneId;
private ZoneOffset zoneOffset = null;

// Performance optimization
private final Set<ZoneId> allZoneIds = getAllZoneIds();
private final ZoneOffset zoneOffset;

private OffsetDateTime dateTime;
public OffsetDateTime getDateTime() {
Expand Down Expand Up @@ -47,19 +40,18 @@ public DateTime withPrecision(Precision precision) {
public DateTime(OffsetDateTime dateTime) {
setDateTime(dateTime);
this.precision = Precision.MILLISECOND;

zoneId = toZoneId(dateTime);
zoneOffset = toZoneOffset(dateTime);
}

public DateTime(OffsetDateTime dateTime, Precision precision) {
setDateTime(dateTime);
this.precision = precision;
zoneId = toZoneId(dateTime);
zoneOffset = toZoneOffset(dateTime);
}

public DateTime(String dateString, ZoneOffset offset) {
zoneOffset = offset;

// Handles case when Tz is not complete (T02:04:59.123+01)
if (dateString.matches("T[0-2]\\d:[0-5]\\d:[0-5]\\d\\.\\d{3}(\\+|-)\\d{2}$")) {
dateString += ":00";
Expand Down Expand Up @@ -103,16 +95,15 @@ public DateTime(String dateString, ZoneOffset offset) {
else {
setDateTime(TemporalHelper.toOffsetDateTime(LocalDateTime.parse(dateString)));
}

zoneId = toZoneId(offset);
zoneOffset = offset;
}

public DateTime(BigDecimal offset, int ... dateElements) {
if (dateElements.length == 0) {
throw new InvalidDateTime("DateTime must include a year");
}

zoneOffset = toZoneOffset(offset);

StringBuilder dateString = new StringBuilder();
String[] stringElements = TemporalHelper.normalizeDateTimeElements(dateElements);

Expand Down Expand Up @@ -143,20 +134,12 @@ else if (i == 6) {
// Otherwise, parse as a LocalDateTime and then interpret that in the evaluation timezone

if (offset != null) {
// dateString.append(ZoneOffset.ofHoursMinutes(offset.intValue(), new BigDecimal("60").multiply(offset.remainder(BigDecimal.ONE)).intValue()).getId());
dateString.append(toZoneOffset(offset).getId());
setDateTime(OffsetDateTime.parse(dateString.toString()));
}
else {
setDateTime(TemporalHelper.toOffsetDateTime(LocalDateTime.parse(dateString.toString())));
}

zoneId = toZoneId(offset);
zoneOffset = toZoneOffset(offset);
}

public ZoneId getZoneId() {
return zoneId;
}

public ZoneOffset getZoneOffset() {
Expand Down Expand Up @@ -231,26 +214,12 @@ public Integer compare(BaseTemporal other, boolean forSort) {
}

public OffsetDateTime getNormalized(Precision precision) {
// return getNormalized(precision, zoneId);
return getNormalized(precision, zoneOffset);
}

public OffsetDateTime getNormalized(Precision precision, ZoneId nullableZoneId) {
if (precision.toDateTimeIndex() > Precision.DAY.toDateTimeIndex()) {
if (nullableZoneId != null) {
return dateTime.atZoneSameInstant(nullableZoneId).toOffsetDateTime();
}

return dateTime.atZoneSameInstant(TimeZone.getDefault().toZoneId()).toOffsetDateTime();
}

return dateTime;
}

public OffsetDateTime getNormalized(Precision precision, ZoneOffset nullableZoneOffset) {
if (precision.toDateTimeIndex() > Precision.DAY.toDateTimeIndex()) {
if (nullableZoneOffset != null) {
// return dateTime.atZoneSameInstant(nullableZoneId).toOffsetDateTime();
return dateTime.withOffsetSameInstant(nullableZoneOffset);
}

Expand All @@ -267,7 +236,6 @@ public Integer compareToPrecision(BaseTemporal other, Precision thePrecision) {

// adjust dates to evaluation offset
OffsetDateTime leftDateTime = this.getNormalized(thePrecision);
// OffsetDateTime rightDateTime = ((DateTime) other).getNormalized(thePrecision, getZoneId());
OffsetDateTime rightDateTime = ((DateTime) other).getNormalized(thePrecision, getZoneOffset());

if (!leftMeetsPrecisionRequirements || !rightMeetsPrecisionRequirements) {
Expand Down Expand Up @@ -334,26 +302,6 @@ public static DateTime fromJavaDate(Date date) {
return new DateTime(OffsetDateTime.ofInstant(calendar.toInstant(), calendar.getTimeZone().toZoneId()), Precision.MILLISECOND);
}

private ZoneId toZoneId(OffsetDateTime offsetDateTime) {
return toZoneId(offsetDateTime.getOffset());
}

private ZoneId toZoneId(ZoneOffset offset) {
if (offset == null) {
return null;
}

return toZoneId(zoneId -> isZoneEquivalentToOffset(zoneId, offset));
}

private ZoneId toZoneId(BigDecimal offset) {
if (offset == null) {
return null;
}

return toZoneId(zoneId -> isZoneEquivalentToOffset(zoneId, offset));
}

private ZoneOffset toZoneOffset(OffsetDateTime offsetDateTime) {
return offsetDateTime.getOffset();
}
Expand All @@ -365,66 +313,7 @@ private ZoneOffset toZoneOffset(BigDecimal offsetAsBigDecimal) {

return ZoneOffset.ofHoursMinutes(offsetAsBigDecimal.intValue(),
new BigDecimal(60)
.multiply(offsetAsBigDecimal.remainder(BigDecimal.ONE)).intValue());

// return allZoneIds.stream()
// .map(zoneId -> LocalDateTime.now().atZone(zoneId).getOffset())
// .filter(aZoneOffset -> {
// final long offsetSeconds = aZoneOffset.getLong(ChronoField.OFFSET_SECONDS);
// final BigDecimal offsetMinutes = BigDecimal.valueOf(offsetSeconds)
// .divide(BigDecimal.valueOf(60), 2, RoundingMode.CEILING);
// final BigDecimal offsetHours = offsetMinutes
// .divide(BigDecimal.valueOf(60), 2, RoundingMode.CEILING);
//
// return offsetHours.doubleValue() == offsetAsBigDecimal.doubleValue();
// })
// .findFirst()
// .orElse(null);
}

private ZoneId toZoneId(Predicate<ZoneId> predicate) {
// final Set<String> allDstOffsets = allZoneIds.stream().
// map(zoneId -> zoneId.getId() + zoneId.getRules().getStandardOffset(LocalDateTime.of(2023, Month.JUNE, 25, 5, 15, 0).atZone(zoneId).toInstant()).toString())
// .collect(Collectors.toSet());
//
// final Set<String> allNonDstOffsets = allZoneIds.stream().
// map(zoneId -> zoneId.getId() + zoneId.getRules().getStandardOffset(LocalDateTime.of(2023, Month.DECEMBER, 25, 5, 15, 0).atZone(zoneId).toInstant()).toString())
// .collect(Collectors.toSet());
//
// final Set<String> dstMontreal = allDstOffsets.stream().filter(offset -> offset.contains("Montreal")).collect(Collectors.toSet());
// final Set<String> nonDstMontreal = allNonDstOffsets.stream().filter(offset -> offset.contains("Montreal")).collect(Collectors.toSet());
//
// final Set<String> dstNewfoundland = allDstOffsets.stream().filter(offset -> offset.contains("St_Johns")).collect(Collectors.toSet());
// final Set<String> nonDstNewfoundland = allNonDstOffsets.stream().filter(offset -> offset.contains("St_Johns")).collect(Collectors.toSet());

// LUKETODO: there is a performance issue with CQL and it's probably because we don't inline enough here:
return allZoneIds.stream()
.filter(predicate)
.findFirst()
.orElse(null);
}

private static Set<ZoneId> getAllZoneIds() {
return ZoneId.getAvailableZoneIds()
.stream()
.map(ZoneId::of)
.collect(Collectors.toSet());
}

private static boolean isZoneEquivalentToOffset(ZoneId zoneId, ZoneOffset zoneOffset) {
final ZoneOffset zoneIdOffset = LocalDateTime.now().atZone(zoneId).getOffset();

return zoneIdOffset.equals(zoneOffset);
}

private static boolean isZoneEquivalentToOffset(ZoneId zoneId, BigDecimal offset) {
final ZoneOffset zoneOffset = LocalDateTime.now().atZone(zoneId).getOffset();
final long offsetSeconds = zoneOffset.getLong(ChronoField.OFFSET_SECONDS);
final BigDecimal offsetMinutes = BigDecimal.valueOf(offsetSeconds)
.divide(BigDecimal.valueOf(60), 2, RoundingMode.CEILING);
final BigDecimal offsetHours = offsetMinutes
.divide(BigDecimal.valueOf(60), 2, RoundingMode.CEILING);

return offsetHours.doubleValue() == offset.doubleValue();
.multiply(offsetAsBigDecimal.remainder(BigDecimal.ONE))
.intValue());
}
}
1 change: 0 additions & 1 deletion Src/java/engine/src/test/java/org/opencds/cqf/cql/engine/execution/CqlPerformanceIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@

import static org.testng.Assert.assertTrue;

// LUKETODO: this test suite is failing with performance problems
public class CqlPerformanceIT extends CqlTestBase {

private static final Integer ITERATIONS = 200;
Expand Down
34 changes: 19 additions & 15 deletions Src/java/engine/src/test/java/org/opencds/cqf/cql/engine/runtime/DateTimeTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ public class DateTimeTest {
private static final ZoneOffset DST_OFFSET_NORTH_AMERICA_MOUNTAIN = ZoneOffset.of("-06:00");
private static final ZoneOffset NON_DST_OFFSET_NORTH_AMERICA_MOUNTAIN = ZoneOffset.of("-07:00");
private static final ZoneOffset DST_OFFSET_NORTH_AMERICA_NEWFOUNDLAND = ZoneOffset.of("-02:30");
// This offset doesn't exist for an ZoneID
private static final ZoneOffset NON_DST_OFFSET_NORTH_AMERICA_NEWFOUNDLAND = ZoneOffset.of("-03:30");

private static final BigDecimal DST_BIG_DECIMAL_OFFSET_NORTH_AMERICA_EASTERN = toBigDecimal(DST_OFFSET_NORTH_AMERICA_EASTERN);
Expand Down Expand Up @@ -95,20 +96,34 @@ void testDateStrings(String dateString, ZoneOffset zoneOffset, Precision precisi
assertEquals(normalizedDateTime, dateTime.getDateTime());
}

// LUKETODO: more test cases
@DataProvider
private static Object[][] dateStringsOtherZoneId() {
return new Object[][]{
{DST_2023_10_26_22_12_0, DST_OFFSET_NORTH_AMERICA_EASTERN, DST_OFFSET_NORTH_AMERICA_MOUNTAIN}
{DST_2023_10_26_22_12_0, DST_OFFSET_NORTH_AMERICA_EASTERN, DST_OFFSET_NORTH_AMERICA_MOUNTAIN, Precision.HOUR},
{DST_2023_10_26_22_12_0, DST_OFFSET_NORTH_AMERICA_MOUNTAIN, DST_OFFSET_NORTH_AMERICA_MOUNTAIN, Precision.HOUR},
{DST_2023_10_26_22_12_0, DST_OFFSET_NORTH_AMERICA_EASTERN, DST_OFFSET_NORTH_AMERICA_NEWFOUNDLAND, Precision.HOUR},
{DST_2023_10_26_22_12_0, DST_OFFSET_NORTH_AMERICA_NEWFOUNDLAND, DST_OFFSET_NORTH_AMERICA_EASTERN, Precision.HOUR},
{NON_DST_2024_02_27_07_28_0, NON_DST_OFFSET_NORTH_AMERICA_EASTERN, NON_DST_OFFSET_NORTH_AMERICA_MOUNTAIN, Precision.HOUR},
{NON_DST_2024_02_27_07_28_0, NON_DST_OFFSET_NORTH_AMERICA_MOUNTAIN, NON_DST_OFFSET_NORTH_AMERICA_MOUNTAIN, Precision.HOUR},
{NON_DST_2024_02_27_07_28_0, NON_DST_OFFSET_NORTH_AMERICA_EASTERN, NON_DST_OFFSET_NORTH_AMERICA_NEWFOUNDLAND, Precision.HOUR},
{NON_DST_2024_02_27_07_28_0, NON_DST_OFFSET_NORTH_AMERICA_NEWFOUNDLAND, NON_DST_OFFSET_NORTH_AMERICA_EASTERN, Precision.HOUR},
{DST_2023_10_26_22_12_0, DST_OFFSET_NORTH_AMERICA_EASTERN, DST_OFFSET_NORTH_AMERICA_MOUNTAIN, Precision.MILLISECOND},
{DST_2023_10_26_22_12_0, DST_OFFSET_NORTH_AMERICA_MOUNTAIN, DST_OFFSET_NORTH_AMERICA_MOUNTAIN, Precision.MILLISECOND},
{DST_2023_10_26_22_12_0, DST_OFFSET_NORTH_AMERICA_EASTERN, DST_OFFSET_NORTH_AMERICA_NEWFOUNDLAND, Precision.MILLISECOND},
{DST_2023_10_26_22_12_0, DST_OFFSET_NORTH_AMERICA_NEWFOUNDLAND, DST_OFFSET_NORTH_AMERICA_EASTERN, Precision.MILLISECOND},
{NON_DST_2024_02_27_07_28_0, NON_DST_OFFSET_NORTH_AMERICA_EASTERN, NON_DST_OFFSET_NORTH_AMERICA_MOUNTAIN, Precision.MILLISECOND},
{NON_DST_2024_02_27_07_28_0, NON_DST_OFFSET_NORTH_AMERICA_MOUNTAIN, NON_DST_OFFSET_NORTH_AMERICA_MOUNTAIN, Precision.MILLISECOND},
{NON_DST_2024_02_27_07_28_0, NON_DST_OFFSET_NORTH_AMERICA_EASTERN, NON_DST_OFFSET_NORTH_AMERICA_NEWFOUNDLAND, Precision.MILLISECOND},
{NON_DST_2024_02_27_07_28_0, NON_DST_OFFSET_NORTH_AMERICA_NEWFOUNDLAND, NON_DST_OFFSET_NORTH_AMERICA_EASTERN, Precision.MILLISECOND},
};
}

@Test(dataProvider = "dateStringsOtherZoneId")
void testDateStringsOtherZoneId(LocalDateTime localDateTime, ZoneOffset zoneOffsetInit, ZoneOffset zonedOffsetGetNormalized) {
void testDateStringsOtherZoneId(LocalDateTime localDateTime, ZoneOffset zoneOffsetInit, ZoneOffset zonedOffsetGetNormalized, Precision precision) {
final OffsetDateTime expectedOffsetDateTime = OffsetDateTime.of(localDateTime.minusSeconds(zoneOffsetInit.getTotalSeconds() - zonedOffsetGetNormalized.getTotalSeconds()), zonedOffsetGetNormalized);
final DateTime dateTime = new DateTime(FORMATTER.format(localDateTime), zoneOffsetInit);

final OffsetDateTime normalizedDateTime = dateTime.getNormalized(Precision.HOUR, zonedOffsetGetNormalized);
final OffsetDateTime normalizedDateTime = dateTime.getNormalized(precision, zonedOffsetGetNormalized);

assertEquals(normalizedDateTime, expectedOffsetDateTime);
}
Expand Down Expand Up @@ -165,7 +180,6 @@ private static Object[][] bigDecimals() {
{BigDecimal.ZERO, Precision.HOUR, NON_DST_2024_02_27_07_28_0_INTS},
{NON_DST_BIG_DECIMAL_OFFSET_NORTH_AMERICA_EASTERN, Precision.HOUR, NON_DST_2024_02_27_07_28_0_INTS},
{NON_DST_BIG_DECIMAL_OFFSET_NORTH_AMERICA_MOUNTAIN, Precision.HOUR, NON_DST_2024_02_27_07_28_0_INTS},
// LUKETODO: This fails with an offset of "-3.50"
{NON_DST_BIG_DECIMAL_OFFSET_NORTH_AMERICA_NEWFOUNDLAND, Precision.HOUR, NON_DST_2024_02_27_07_28_0_INTS},
{null, Precision.MILLISECOND, DST_2024_06_15_23_32_0_INTS},
{BigDecimal.ZERO, Precision.MILLISECOND, DST_2024_06_15_23_32_0_INTS},
Expand All @@ -184,14 +198,4 @@ void testBigDecimal(BigDecimal offset, Precision precision, List<Integer> dateEl

assertEquals(normalizedDateTime, dateTime.getDateTime());
}

@Test
void testNewfoundlandNonDst() {
final int[] dateElementsArray = NON_DST_2024_02_27_07_28_0_INTS.stream().mapToInt(anInt -> anInt).toArray();
final DateTime dateTime = new DateTime(NON_DST_BIG_DECIMAL_OFFSET_NORTH_AMERICA_NEWFOUNDLAND, dateElementsArray);

final OffsetDateTime normalizedDateTime = dateTime.getNormalized(Precision.HOUR);

assertEquals(normalizedDateTime, dateTime.getDateTime());
}
}

0 comments on commit 82ad161

Please sign in to comment.