Skip to content

Commit

Permalink
Fix DateTime initialization to always use offset from State and never…
Browse files Browse the repository at this point in the history
… from null (#1269)

* First commit.

* More changes with experiments.

* Small change to test.

* Add logging to try to figure out why the test passes on the github pipeline.

* More logging.

* Even more logging.

* Refactor DateTime constructor code to enable unit tests using custom timezones and default offset dates.

* Get rid of all known code that relies on the default timezone and instead rely on either the default offsetdatetime.

* Run CqlInternalTypeRepresentationSuiteTest with multiple current times and timezones.

* Throw Exceptions for null offsets in DateTime.  Pass in offsets from State whenever possible.  Fix most units with non-null offsets.  There are still test failures to investigate.  Lots of cleanup needs to be done afterward.

* All tests pass.  Lots of code and TODOs to cleanup.  Need to test on another machine on a DST date.

* Add DST and non-DST "now" logic to TypeConverter tests to replicate test failures on machine set to DST date.

* Try a fix for ***TypeConverter tests that I'm not sure about...

* Fix one use case for DateTimeType conversion that failed on Nov 1st.

* Try latest fix for fhir date time conversion.

* Ensure tests work on Nov 1st.

* Start cleaning up TODOs and commented out and abandoned code.

* Enhanced some tests and clean up more TODOs.

* Enhance more tests.

* Enhance even more tests.

* Enhance all TypeConverter tests.

* More cleanup and refactoring and removing TODOs.

* Last set of tweaks to simplify the implementation.

* Add tests for equals

---------

Co-authored-by: JP <jonathan.i.percival@gmail.com>
  • Loading branch information
lukedegruchy and JPercival authored Nov 28, 2023
1 parent 2985f47 commit 75e55e9
Show file tree
Hide file tree
Showing 29 changed files with 841 additions and 415 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -729,6 +729,11 @@ public void testIfConditionalReturnTypes() throws IOException {
assertTrue("Expected return types are String and Boolean: ", actualChoiceTypes.equals(expectedChoiceTypes));
}

@Test
public void testIssue863() throws IOException {
final CqlTranslator translator = TestUtils.runSemanticTest("Issue863.cql", 0);
}

private CqlTranslator runSemanticTest(String testFileName) throws IOException {
return runSemanticTest(testFileName, 0);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
library Issue863

parameter "Measurement Period" Interval<DateTime> default Interval[@2021-01-01T00:00:00.000, @2022-01-01T00:00:00.000)

define "the interval": //Interval[2020-07-01T00:00:00.000, 2021-07-01T00:00:00.000)
Interval[start of "Measurement Period" - 6 months, start of "Measurement Period" + 6 months)

define "in interval 1": //false
@2020-07-01 in Interval[start of "Measurement Period" - 6 months, start of "Measurement Period" + 6 months)

define "in interval 2": //true
@2020-07-01T01:00:00.000 in Interval[start of "Measurement Period" - 6 months, start of "Measurement Period" + 6 months)

define "in interval 3": //false
@2020-07-01T00:59:59.999 in Interval[start of "Measurement Period" - 6 months, start of "Measurement Period" + 6 months)

define "in interval 4": //true
@2020-07-01 in Interval[@2020-07-01T00:00:00.000, @2021-07-01T00:00:00.000)

define "in interval 5": //true
@2020-07-01T00:00:00.000 in Interval[@2020-07-01T00:00:00.000, @2021-07-01T00:00:00.000)

define "in interval 6": //false
@2020-07-01 in "the interval"
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
package org.opencds.cqf.cql.engine.fhir.converter;

import java.math.BigDecimal;
import java.time.Instant;
import java.time.format.DateTimeFormatter;
import java.util.Calendar;
import java.util.GregorianCalendar;
import java.util.stream.Collectors;
Expand All @@ -12,8 +10,6 @@
import org.opencds.cqf.cql.engine.runtime.Quantity;
import org.opencds.cqf.cql.engine.runtime.Ratio;

import ca.uhn.fhir.model.api.TemporalPrecisionEnum;

import org.apache.commons.lang3.NotImplementedException;
import org.hl7.fhir.instance.model.api.IBase;
import org.hl7.fhir.instance.model.api.IBaseCoding;
Expand Down Expand Up @@ -74,7 +70,7 @@ public IPrimitiveType<java.util.Date> toFhirDateTime(DateTime value) {
return null;
}

var result = new DateTimeType(value.getDateTime().format(DateTimeFormatter.ISO_OFFSET_DATE_TIME));
final DateTimeType result = new DateTimeType(value.toDateString());
result.setPrecision(toFhirPrecision(value.getPrecision()));
return result;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -600,7 +600,8 @@ public void setPrimitiveValue(Object value, IPrimitiveType target) {
switch (simpleName) {
case "DateTimeType":
case "InstantType":
target.setValue(((DateTime) value).toJavaDate());
// Ensure offset is taken into account from the ISO datetime String instead of the default timezone
target.setValueAsString(((DateTime) value).toDateString());
setCalendarConstant((BaseDateTimeType) target, (BaseTemporal) value);
break;
case "DateType":
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
package org.opencds.cqf.cql.engine.fhir.converter;

import org.testng.annotations.DataProvider;

import java.time.LocalDateTime;
import java.time.Month;
import java.time.format.DateTimeFormatter;

public class ConverterTestUtils {
static final LocalDateTime DST_2022_11_01 = LocalDateTime.of(2022, Month.NOVEMBER, 1, 0, 0, 0);
static final LocalDateTime DST_2023_11_01 = LocalDateTime.of(2023, Month.NOVEMBER, 1, 0, 0, 0);
static final LocalDateTime DST_2023_11_03 = LocalDateTime.of(2023, Month.NOVEMBER, 3, 0, 0, 0);
static final LocalDateTime NON_DST_2022_01_01 = LocalDateTime.of(2022, Month.JANUARY, 1, 0, 0, 0);
static final LocalDateTime NON_DST_2023_01_01 = LocalDateTime.of(2023, Month.JANUARY, 1, 0, 0, 0);
static final LocalDateTime NON_DST_2022_11_10 = LocalDateTime.of(2022, Month.NOVEMBER, 10, 0, 0, 0);
static final LocalDateTime NON_DST_2023_11_10 = LocalDateTime.of(2023, Month.NOVEMBER, 10, 0, 0, 0);
static final LocalDateTime NON_DST_2023_11_14 = LocalDateTime.of(2023, Month.NOVEMBER, 14, 0, 0, 0);
static final DateTimeFormatter YYYY_MM_DD = DateTimeFormatter.ofPattern("yyyy-MM-dd");

@DataProvider
static Object[][] dateTimes() {
return new Object[][] {{DST_2023_11_01, DST_2023_11_01, DST_2023_11_03}, {NON_DST_2023_11_14, NON_DST_2023_11_10, NON_DST_2023_11_14}, {NON_DST_2023_11_14, DST_2023_11_01, DST_2023_11_03}, {DST_2023_11_01, NON_DST_2023_11_10, NON_DST_2023_11_14}};
}

@DataProvider
static Object[][] startAndEndTimes() {
return new Object[][] {{DST_2023_11_01, DST_2023_11_03}, {NON_DST_2023_11_10, NON_DST_2023_11_14}, {DST_2023_11_01, DST_2023_11_03}, {NON_DST_2023_11_10, NON_DST_2023_11_14}, {DST_2022_11_01, DST_2023_11_03}, {NON_DST_2022_11_10, NON_DST_2023_11_10}, {NON_DST_2022_01_01, NON_DST_2023_01_01}};
}

@DataProvider
static Object[][] startAndEndYears() {
return new Object[][] {{DST_2022_11_01, 2019, 2020}, {NON_DST_2023_11_14, 2019, 2020},{DST_2022_11_01, 2018, 2022}, {NON_DST_2023_11_14, 2018, 2022}};
}
@DataProvider
static Object[][] nowsAndEvaluationTimes() {
return new Object[][] {{NON_DST_2022_01_01, NON_DST_2023_01_01}, {DST_2022_11_01, NON_DST_2023_01_01}, {NON_DST_2022_11_10, NON_DST_2023_01_01}};
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,15 @@

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.instanceOf;
import static org.opencds.cqf.cql.engine.fhir.converter.ConverterTestUtils.*;
import static org.testng.Assert.assertEquals;
import static org.testng.Assert.assertFalse;
import static org.testng.Assert.assertNull;
import static org.testng.Assert.assertTrue;

import java.math.BigDecimal;
import java.time.ZoneOffset;
import java.time.*;
import java.time.format.DateTimeFormatter;
import java.util.ArrayList;
import java.util.Calendar;
import java.util.Iterator;
Expand Down Expand Up @@ -37,18 +39,9 @@
import org.hl7.fhir.instance.model.api.ICompositeType;
import org.hl7.fhir.instance.model.api.IIdType;
import org.hl7.fhir.instance.model.api.IPrimitiveType;
import org.opencds.cqf.cql.engine.runtime.Code;
import org.opencds.cqf.cql.engine.runtime.Concept;
import org.opencds.cqf.cql.engine.runtime.CqlType;
import org.opencds.cqf.cql.engine.runtime.Date;
import org.opencds.cqf.cql.engine.runtime.DateTime;
import org.opencds.cqf.cql.engine.runtime.Interval;
import org.opencds.cqf.cql.engine.runtime.Precision;
import org.opencds.cqf.cql.engine.runtime.Quantity;
import org.opencds.cqf.cql.engine.runtime.Ratio;
import org.opencds.cqf.cql.engine.runtime.Time;
import org.opencds.cqf.cql.engine.runtime.Tuple;
import org.opencds.cqf.cql.engine.runtime.*;
import org.testng.annotations.BeforeClass;
import org.testng.annotations.DataProvider;
import org.testng.annotations.Test;

import ca.uhn.fhir.model.api.TemporalPrecisionEnum;
Expand Down Expand Up @@ -243,24 +236,37 @@ public void TestDateToFhirDate() {
assertEquals(expectedDate.getValue(), actualDate.getValue());
}

@Test
public void TestDateTimeToFhirDateTime() {
IPrimitiveType<java.util.Date> expectedDate = new DateTimeType("2019-02-03");

@DataProvider
private static Object[][] nowsAndEvaluationTimes() {
return ConverterTestUtils.nowsAndEvaluationTimes();
}

@Test(dataProvider = "nowsAndEvaluationTimes")
public void TestDateTimeToFhirDateTime(LocalDateTime now, LocalDateTime evaluationTime) {
final ZonedDateTime zonedDateTime = ZonedDateTime.of(now, ZoneId.systemDefault());
final ZoneOffset defaultOffset = zonedDateTime.getOffset();

final String evalTimeWithOffset = DateTimeFormatter.ISO_OFFSET_DATE_TIME.format(evaluationTime.atOffset(defaultOffset));
final String evalDate = DateTimeFormatter.ISO_DATE.format(evaluationTime);

var expectedDate = new DateTimeType(evalTimeWithOffset);
IPrimitiveType<java.util.Date> actualDate = this.typeConverter
.toFhirDateTime(new DateTime("2019-02-03", null));
.toFhirDateTime(new DateTime(evalDate, defaultOffset));
assertEquals(expectedDate.getValue(), actualDate.getValue());

expectedDate = new DateTimeType("2019");
actualDate = this.typeConverter.toFhirDateTime(new DateTime("2019", null));
expectedDate = new DateTimeType(evalTimeWithOffset);
actualDate = this.typeConverter.toFhirDateTime(new DateTime(""+evaluationTime.getYear(), defaultOffset));
expectedDate.setPrecision(TemporalPrecisionEnum.YEAR);
assertEquals(expectedDate.getValue(), actualDate.getValue());

expectedDate = new DateTimeType("2019");
actualDate = this.typeConverter.toFhirDateTime(new DateTime("2019", null));
assertEquals(expectedDate.getValueAsString(), actualDate.getValueAsString());
}

expectedDate = new DateTimeType("2019-10-10T01:00:00-06:00");
@Test
public void TestDateTimeToFhirDateTime_Timezones() {
var expectedDate = new DateTimeType("2019-10-10T01:00:00-06:00");
((DateTimeType) expectedDate).setTimeZone(TimeZone.getTimeZone("MST"));
actualDate = this.typeConverter.toFhirDateTime(new DateTime("2019-10-10T00:00:00", ZoneOffset.ofHours(-7)));
var actualDate = this.typeConverter.toFhirDateTime(new DateTime("2019-10-10T00:00:00", ZoneOffset.ofHours(-7)));
assertEquals(expectedDate.getValueAsString(), actualDate.getValueAsString());

expectedDate = new DateTimeType("2019-10-10T19:35:53.000Z");
Expand Down Expand Up @@ -333,21 +339,69 @@ public void TestConceptToFhirCodeableConcept() {
assertNull(expected);
}

@Test
public void TestIntervalToFhirPeriod() {
Period expected = new Period().setStartElement(new DateTimeType("2019-02-03"))
.setEndElement(new DateTimeType("2019-02-05"));
Period actual = (Period) this.typeConverter
.toFhirPeriod(new Interval(new Date("2019-02-03"), true, new Date("2019-02-05"), true));
@DataProvider
private static Object[][] startAndEndTimes() {
return ConverterTestUtils.startAndEndTimes();
}

@Test(dataProvider = "startAndEndTimes")
public void TestIntervalToFhirPeriod_yyyyMMdd(LocalDateTime startTime, LocalDateTime endTime) {
final String startTime_yyyyMMdd = YYYY_MM_DD.format(startTime);
final String endTime_yyyyMMdd = YYYY_MM_DD.format(endTime);

final Period expected = new Period().setStartElement(new DateTimeType(startTime_yyyyMMdd))
.setEndElement(new DateTimeType(endTime_yyyyMMdd));
final Period actual = (Period) this.typeConverter
.toFhirPeriod(new Interval(new Date(startTime_yyyyMMdd), true, new Date(endTime_yyyyMMdd), true));
assertTrue(expected.equalsDeep(actual));
}

@DataProvider
private static Object[][] dateTimes() {
return ConverterTestUtils.dateTimes();
}

@Test(dataProvider = "dateTimes")
public void TestIntervalToFhirPeriod_timestampWithOffsets(LocalDateTime now, LocalDateTime startTime, LocalDateTime endTime) {
final ZonedDateTime zonedDateTime = ZonedDateTime.of(now, ZoneId.systemDefault());
final ZoneOffset defaultOffset = zonedDateTime.getOffset();

final String startTimeWithOffset = DateTimeFormatter.ISO_OFFSET_DATE_TIME.format(startTime.atOffset(defaultOffset));
final String endTimeWithOffset = DateTimeFormatter.ISO_OFFSET_DATE_TIME.format(endTime.atOffset(defaultOffset));
final String startTimeNoOffset = DateTimeFormatter.ISO_DATE_TIME.format(startTime.atOffset(defaultOffset));
final String endTimeNoOffset = DateTimeFormatter.ISO_DATE_TIME.format(endTime.atOffset(defaultOffset));

final DateTimeType dateTimeTypeStart = new DateTimeType(startTimeWithOffset);
final DateTimeType dateTimeTypeEnd = new DateTimeType(endTimeWithOffset);
var expected = new Period().setStartElement(dateTimeTypeStart).setEndElement(dateTimeTypeEnd);

final DateTime dateTimeStart = new DateTime(startTimeNoOffset, defaultOffset);
final DateTime dateTimeEnd = new DateTime(endTimeNoOffset, defaultOffset);
final Interval intervalStartEnd = new Interval(dateTimeStart, true, dateTimeEnd, true);
var actual = (Period) this.typeConverter.toFhirPeriod(intervalStartEnd);

assertTrue(expected.equalsDeep(actual));
}

@DataProvider
private static Object[][] startAndEndYears() {
return ConverterTestUtils.startAndEndYears();
}

@Test(dataProvider = "startAndEndYears")
public void TestIntervalToFhirPeriod_startAndEndYears(LocalDateTime now, int startYear, int endYear) {
final ZonedDateTime zonedDateTime = ZonedDateTime.of(now, ZoneId.systemDefault());
final ZoneOffset defaultOffset = zonedDateTime.getOffset();

expected = new Period().setStartElement(new DateTimeType("2019")).setEndElement(new DateTimeType("2020"));
actual = (Period) this.typeConverter.toFhirPeriod(
new Interval(new DateTime("2019", null), true, new DateTime("2020", null), true));
final Period expected = new Period().setStartElement(new DateTimeType(startYear+"-01-01T00:00:00"+defaultOffset)).setEndElement(new DateTimeType(endYear+"-01-01T00:00:00"+defaultOffset));
final Period actual = (Period) this.typeConverter.toFhirPeriod(
new Interval(new DateTime(""+startYear, defaultOffset), true, new DateTime(""+endYear, defaultOffset), true));
assertTrue(expected.equalsDeep(actual));
}

actual = (Period) this.typeConverter.toFhirPeriod(null);
assertNull(null);
@Test
public void TestIntervalToFhirPeriod_null() {
assertNull(this.typeConverter.toFhirPeriod(null));
}

@Test(expectedExceptions = IllegalArgumentException.class)
Expand Down
Loading

0 comments on commit 75e55e9

Please sign in to comment.