Add changelog. Ensure evaluated measure period search takes either both start and end as null or both neither. javadoc. Refactoring. More tests.

hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_6_0/6327-evaluate-measure-use-client-timezone.yaml

@@ -0,0 +1,6 @@
+---
+type: add
+issue: 6267
+title: "$evaluate-measure will now consider the 'Timezone' request header when computing period start and end with
+        the requested date/time offset instead of using the server timezone.  If no 'Timezone' header is provided,
+        $evaluate-measure will default to UTC."

hapi-fhir-storage-cr/src/main/java/ca/uhn/fhir/cr/r4/measure/MeasurePeriodForEvaluation.java

@@ -19,13 +19,17 @@
  */
 package ca.uhn.fhir.cr.r4.measure;
 
+import com.google.common.base.Preconditions;
 import jakarta.annotation.Nullable;
 
 import java.util.Objects;
 import java.util.StringJoiner;
 
-// LUKETODO:  javadoc
+// TODO:  LD:  consider making this a record when hapi-fhir supports JDK 17
-// TODO:  LD:  make this a record when hapi-fhir supports JDK 17
+/**
+ * Simple tuple containing post-conversion String versions of period start and end.
+ * Either both must be null or neither.
+ */
 public class MeasurePeriodForEvaluation {
 	@Nullable
 	private final String myPeriodStart;
@@ -34,6 +38,10 @@ public class MeasurePeriodForEvaluation {
 	private final String myPeriodEnd;
 
 	public MeasurePeriodForEvaluation(@Nullable String thePeriodStart, @Nullable String thePeriodEnd) {
+		// Either both are null or neither
+		Preconditions.checkArgument(
+				(thePeriodStart != null && thePeriodEnd != null) || (thePeriodStart == null && thePeriodEnd == null));
+
 		myPeriodStart = thePeriodStart;
 		myPeriodEnd = thePeriodEnd;
 	}
@@ -49,14 +57,14 @@ public class MeasurePeriodForEvaluation {
 	}
 
 	@Override
-	public boolean equals(Object theO) {
+	public boolean equals(Object theOther) {
-		if (this == theO) {
+		if (this == theOther) {
 			return true;
 		}
-		if (theO == null || getClass() != theO.getClass()) {
+		if (theOther == null || getClass() != theOther.getClass()) {
 			return false;
 		}
-		MeasurePeriodForEvaluation that = (MeasurePeriodForEvaluation) theO;
+		MeasurePeriodForEvaluation that = (MeasurePeriodForEvaluation) theOther;
 		return Objects.equals(myPeriodStart, that.myPeriodStart) && Objects.equals(myPeriodEnd, that.myPeriodEnd);
 	}
 

hapi-fhir-storage-cr/src/main/java/ca/uhn/fhir/cr/r4/measure/MeasureReportPeriodRequestProcessingService.java

@@ -23,6 +23,7 @@ import ca.uhn.fhir.rest.api.Constants;
 import ca.uhn.fhir.rest.api.server.RequestDetails;
 import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException;
 import ca.uhn.fhir.util.DateUtils;
+import jakarta.annotation.Nonnull;
 import org.apache.logging.log4j.util.Strings;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -30,11 +31,28 @@ import org.slf4j.LoggerFactory;
 import java.time.LocalDateTime;
 import java.time.ZoneId;
 import java.time.format.DateTimeFormatter;
+import java.time.temporal.TemporalAccessor;
 import java.util.Map;
 import java.util.Optional;
+import java.util.function.Function;
 
-// LUKETODO: changelog
+/**
-// LUKETODO: javadoc
+ * Used immediately after receiving a REST call by $evaluate-measure and any potential variants to validate and convert
+ * period start and end inputs to timezones with offsets.  The offset is determined from the request header a value for "Timezone".
+ * <p/>
+ * This class takes a fallback timezone that's used in case the request header does not contain a value for "Timezone".
+ * <p/>
+ * The output date/time format is the following to be compatible with clinical-reasoning: yyyy-MM-dd'T'HH:mm:ss.SXXX
+ * ex: 2023-01-01T00:00:00.0-07:00
+ * <p/>
+ * Currently, these are the date/time formats supported:
+ * <ol>
+ *     <li>yyyy</li>
+ *     <li>yyyy-MM</li>
+ *     <li>yyyy-MM-dd</li>
+ *     <li>yyyy-MM-ddTHH:mm:ss</li>
+ * </ol>
+ */
 public class MeasureReportPeriodRequestProcessingService {
 	private static final Logger ourLog = LoggerFactory.getLogger(MeasureReportPeriodRequestProcessingService.class);
 
@@ -43,12 +61,10 @@ public class MeasureReportPeriodRequestProcessingService {
 	private static final DateTimeFormatter DATE_TIME_FORMATTER_YYYY_MM_DD_INPUT = DateTimeFormatter.ISO_DATE;
 	private static final DateTimeFormatter DATE_TIME_FORMATTER_YYYY_MM_DD_HH_MM_SS_INPUT =
 			DateTimeFormatter.ISO_LOCAL_DATE_TIME;
-	// LUKETODO:  what's the winning format here?
+	// This specific format is needed because otherwise clinical-reasoning will error out when parsing the output
-	//	java.time.format.DateTimeParseException: Text '2023-01-01T00:00:00.0-0700'
+	// ex:  2023-01-01T00:00:00.0-07:00
 	private static final DateTimeFormatter DATE_TIME_FORMATTER_YYYY_MM_DD_HH_MM_SS_Z_OUTPUT =
 			DateTimeFormatter.ofPattern("yyyy-MM-dd'T'HH:mm:ss.SXXX");
-	//			DateTimeFormatter.ISO_OFFSET_DATE_TIME;
-	//		DateTimeFormatter.ISO_ZONED_DATE_TIME;
 
 	private static final Map<Integer, DateTimeFormatter> VALID_DATE_TIME_FORMATTERS_BY_FORMAT_LENGTH = Map.of(
 			4, DATE_TIME_FORMATTER_YYYY_INPUT,
@@ -72,7 +88,14 @@ public class MeasureReportPeriodRequestProcessingService {
 	private MeasurePeriodForEvaluation validateInputDates(
 			String thePeriodStart, String thePeriodEnd, ZoneId theZoneId) {
 
-		if (Strings.isBlank(thePeriodStart) || Strings.isBlank(thePeriodEnd)) {
+		if ((Strings.isBlank(thePeriodStart) && !Strings.isBlank(thePeriodEnd))
+				|| (!Strings.isBlank(thePeriodStart) && Strings.isBlank(thePeriodEnd))) {
+			throw new InvalidRequestException(String.format(
+					"Either both period start: [%s] and end: [%s] must be empty or non empty",
+					thePeriodStart, thePeriodEnd));
+		}
+
+		if (Strings.isBlank(thePeriodStart) && Strings.isBlank(thePeriodEnd)) {
 			return new MeasurePeriodForEvaluation(null, null);
 		}
 
@@ -81,48 +104,68 @@ public class MeasureReportPeriodRequestProcessingService {
 					"Period start: %s and end: %s are not the same date/time formats", thePeriodStart, thePeriodEnd));
 		}
 
-		final DateTimeFormatter dateTimeFormatterStart =
+		final DateTimeFormatter dateTimeFormatterStart = validateAndGetDateTimeFormat(thePeriodStart, thePeriodEnd);
-				VALID_DATE_TIME_FORMATTERS_BY_FORMAT_LENGTH.get(thePeriodStart.length());
 
-		if (dateTimeFormatterStart == null) {
+		final LocalDateTime localDateTimeStart = validateAndGetLocalDateTime(
-			throw new InvalidRequestException(String.format(
+				thePeriodStart, dateTimeFormatterStart, DateUtils::extractLocalDateTimeForRangeStartOrEmpty, true);
-					"Unsupported Date/Time format for period start: %s or end: %s", thePeriodStart, thePeriodEnd));
+		final LocalDateTime localDateTimeEnd = validateAndGetLocalDateTime(
-		}
+				thePeriodEnd, dateTimeFormatterStart, DateUtils::extractLocalDateTimeForRangeEndOrEmpty, false);
-
-		final Optional<LocalDateTime> optLocalDateTimeStart = DateUtils.parseDateTimeStringIfValid(
-						thePeriodStart, dateTimeFormatterStart)
-				.flatMap(DateUtils::extractLocalDateTimeForRangeStartOrEmpty);
-
-		final Optional<LocalDateTime> optLocalDateTimeEnd = DateUtils.parseDateTimeStringIfValid(
-						thePeriodEnd, dateTimeFormatterStart)
-				.flatMap(DateUtils::extractLocalDateTimeForRangeEndOrEmpty);
-
-		if (optLocalDateTimeStart.isEmpty() || optLocalDateTimeEnd.isEmpty()) {
-			throw new InvalidRequestException("Either start or end period have an unsupported format");
-		}
-
-		final LocalDateTime localDateTimeStart = optLocalDateTimeStart.get();
-		final LocalDateTime localDateTimeEnd = optLocalDateTimeEnd.get();
-
-		if (localDateTimeStart.isEqual(localDateTimeEnd)) {
-			throw new InvalidRequestException(
-					String.format("Start date: %s is the same as end date: %s", thePeriodStart, thePeriodEnd));
-		}
-
-		if (localDateTimeStart.isAfter(localDateTimeEnd)) {
-			throw new InvalidRequestException(String.format(
-					"Invalid Interval - the ending boundary: %s must be greater than or equal to the starting boundary: %s",
-					thePeriodEnd, thePeriodStart));
-		}
 
+		validateParsedPeriodStartAndEnd(thePeriodStart, thePeriodEnd, localDateTimeStart, localDateTimeEnd);
 
 		final String periodStartFormatted = formatWithTimezone(localDateTimeStart, theZoneId);
 		final String periodEndFormatted = formatWithTimezone(localDateTimeEnd, theZoneId);
 
-		ourLog.info("6560: NEW START: {} formatted: {}, NEW END: {}, formatted: {}", localDateTimeStart, periodStartFormatted, localDateTimeEnd, periodEndFormatted);
+		ourLog.info(
+				"6560: NEW START: {} formatted: {}, NEW END: {}, formatted: {}",
+				localDateTimeStart,
+				periodStartFormatted,
+				localDateTimeEnd,
+				periodEndFormatted);
 
-		return new MeasurePeriodForEvaluation(
+		return new MeasurePeriodForEvaluation(periodStartFormatted, periodEndFormatted);
-			periodStartFormatted, periodEndFormatted);
+	}
+
+	private static void validateParsedPeriodStartAndEnd(
+			String theThePeriodStart,
+			String theThePeriodEnd,
+			LocalDateTime theLocalDateTimeStart,
+			LocalDateTime theLocalDateTimeEnd) {
+		// This should probably never happen
+		if (theLocalDateTimeStart.isEqual(theLocalDateTimeEnd)) {
+			throw new InvalidRequestException(
+					String.format("Start date: %s is the same as end date: %s", theThePeriodStart, theThePeriodEnd));
+		}
+
+		if (theLocalDateTimeStart.isAfter(theLocalDateTimeEnd)) {
+			throw new InvalidRequestException(String.format(
+					"Invalid Interval - the ending boundary: %s must be greater than or equal to the starting boundary: %s",
+					theThePeriodEnd, theThePeriodStart));
+		}
+	}
+
+	private LocalDateTime validateAndGetLocalDateTime(
+			String thePeriod,
+			DateTimeFormatter theDateTimeFormatter,
+			Function<TemporalAccessor, Optional<LocalDateTime>> theTemporalAccessorToLocalDateTimeConverter,
+			boolean isStart) {
+		return DateUtils.parseDateTimeStringIfValid(thePeriod, theDateTimeFormatter)
+				.flatMap(theTemporalAccessorToLocalDateTimeConverter)
+				.orElseThrow(() -> new InvalidRequestException(String.format(
+						"Period %s: %s has an unsupported format", isStart ? "start" : "end", thePeriod)));
+	}
+
+	@Nonnull
+	private static DateTimeFormatter validateAndGetDateTimeFormat(String theThePeriodStart, String theThePeriodEnd) {
+		final DateTimeFormatter dateTimeFormatterStart =
+				VALID_DATE_TIME_FORMATTERS_BY_FORMAT_LENGTH.get(theThePeriodStart.length());
+
+		if (dateTimeFormatterStart == null) {
+			throw new InvalidRequestException(String.format(
+					"Unsupported Date/Time format for period start: %s or end: %s",
+					theThePeriodStart, theThePeriodEnd));
+		}
+		return dateTimeFormatterStart;
 	}
 
 	private String formatWithTimezone(LocalDateTime theLocalDateTime, ZoneId theZoneId) {

hapi-fhir-storage-cr/src/test/java/ca/uhn/fhir/cr/r4/measure/MeasureReportPeriodRequestProcessingServiceTest.java

@@ -5,18 +5,12 @@ import ca.uhn.fhir.rest.api.server.RequestDetails;
 import ca.uhn.fhir.rest.api.server.SystemRequestDetails;
 import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException;
 import jakarta.annotation.Nullable;
-import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.Arguments;
 import org.junit.jupiter.params.provider.CsvSource;
 import org.junit.jupiter.params.provider.MethodSource;
 
-import java.time.LocalDateTime;
-import java.time.Month;
-import java.time.ZoneId;
 import java.time.ZoneOffset;
-import java.time.ZonedDateTime;
-import java.time.format.DateTimeFormatter;
 import java.util.Optional;
 import java.util.stream.Stream;
 
@@ -27,17 +21,6 @@ class MeasureReportPeriodRequestProcessingServiceTest {
 
 	private final MeasureReportPeriodRequestProcessingService myTestSubject = new MeasureReportPeriodRequestProcessingService(ZoneOffset.UTC);
 
-	@Test
-	void dammit() {
-		final String expectedResult = "2020-01-01T00:00:00.0-05:00";
-		final DateTimeFormatter dateTimeFormatter = DateTimeFormatter.ofPattern("yyyy-MM-dd'T'HH:mm:ss.SXXX");
-		final ZonedDateTime input = ZonedDateTime.of(LocalDateTime.of(2020, Month.JANUARY, 1, 0, 0, 0), ZoneId.of("America/Toronto"));
-
-		final String actualResult = input.format(dateTimeFormatter);
-
-		assertThat(actualResult).isEqualTo(expectedResult);
-	}
-
 	// LUKETODO:  what happens if only one is null?
 	@ParameterizedTest
 	@CsvSource( nullValues = {"null"},
@@ -120,6 +103,8 @@ class MeasureReportPeriodRequestProcessingServiceTest {
 	private static Stream<Arguments> errorParams() {
 		return Stream.of(
 			Arguments.of(null, "2024", "2024-01", new InvalidRequestException("Period start: 2024 and end: 2024-01 are not the same date/time formats")),
+			Arguments.of(null, null, "2024-01", new InvalidRequestException("Either both period start: [null] and end: [2024-01] must be empty or non empty")),
+			Arguments.of(null, "2024-01", null, new InvalidRequestException("Either both period start: [2024-01] and end: [null] must be empty or non empty")),
 			Arguments.of(null, "2024-01-01T12", "2024-01-01T12", new InvalidRequestException("Unsupported Date/Time format for period start: 2024-01-01T12 or end: 2024-01-01T12")),
 			Arguments.of(null, "2024-01-02", "2024-01-01", new InvalidRequestException("Invalid Interval - the ending boundary: 2024-01-01 must be greater than or equal to the starting boundary: 2024-01-02")),
 			Arguments.of("Middle-Earth/Combe", "2024-01-02", "2024-01-03", new InvalidRequestException("Invalid value for Timezone header: Middle-Earth/Combe")),