From d71a0f3fabe77c0f87ef0aa00d1468ff368f74d4 Mon Sep 17 00:00:00 2001 From: Luke deGruchy Date: Wed, 29 Apr 2026 16:09:35 -0400 Subject: [PATCH 01/13] Centralize ExpressionResult.getValue handling in CqlExpressionValue Introduce CqlExpressionValue, a wrapper around the raw Object returned by ExpressionResult.getValue(), to consolidate the null / Boolean / Iterable / scalar normalization that was previously duplicated across measure-evaluation call sites. Phase 1 adopts the wrapper at three boundaries: - MeasureEvaluator.evaluatePopulationCriteria / evaluateSupportingCriteria - FunctionEvaluationHandler.getResultIterable - PopulationBasisValidator's two-stage null checks The wrapper's internal raw field is intentionally typed as Object so a future migration to the upstream sealed CQL Value type touches only this class, not its callers. CqlExpressionValueException (plain RuntimeException) replaces a HAPI InternalErrorException previously thrown on the subject-context lookup path. Out of scope for Phase 1: collapsing CriteriaResult and StratumValueWrapper, and pushing the wrapper through the Def-class signatures. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../cr/measure/common/CqlExpressionValue.java | 175 +++++++++++ .../common/CqlExpressionValueException.java | 12 + .../common/FunctionEvaluationHandler.java | 28 +- .../cr/measure/common/MeasureEvaluator.java | 49 +-- .../common/PopulationBasisValidator.java | 12 +- .../common/CqlExpressionValueTest.java | 279 ++++++++++++++++++ 6 files changed, 477 insertions(+), 78 deletions(-) create mode 100644 cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/CqlExpressionValue.java create mode 100644 cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/CqlExpressionValueException.java create mode 100644 cqf-fhir-cr/src/test/java/org/opencds/cqf/fhir/cr/measure/common/CqlExpressionValueTest.java diff --git a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/CqlExpressionValue.java b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/CqlExpressionValue.java new file mode 100644 index 0000000000..4ee900e77e --- /dev/null +++ b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/CqlExpressionValue.java @@ -0,0 +1,175 @@ +package org.opencds.cqf.fhir.cr.measure.common; + +import jakarta.annotation.Nullable; +import java.util.Collection; +import java.util.Collections; +import java.util.Map; +import java.util.Optional; +import java.util.Set; +import org.opencds.cqf.cql.engine.execution.EvaluationResult; +import org.opencds.cqf.cql.engine.execution.ExpressionResult; + +/** + * Wrapper around the raw {@code Object} returned by + * {@link ExpressionResult#getValue()}. + *

+ * Centralizes the null / Boolean / Iterable / scalar normalization that was previously + * scattered across measure-evaluation call sites, so the contract between the CQL engine + * and the measure pipeline is testable in one place. The internal {@code raw} field is + * intentionally typed as {@code Object} so a future migration to the upstream sealed + * {@code Value} type touches only this class, not its callers. + */ +public final class CqlExpressionValue { + + private static final CqlExpressionValue EMPTY = new CqlExpressionValue(null, Collections.emptySet()); + + private final @Nullable Object raw; + private final Set evaluatedResources; + + private CqlExpressionValue(@Nullable Object raw, Set evaluatedResources) { + this.raw = raw; + this.evaluatedResources = evaluatedResources; + } + + /** + * Wraps an {@link ExpressionResult}. Accepts a null result and yields an empty wrapper. + */ + public static CqlExpressionValue of(@Nullable ExpressionResult result) { + if (result == null) { + return EMPTY; + } + Set resources = result.getEvaluatedResources(); + return new CqlExpressionValue(result.getValue(), resources != null ? resources : Collections.emptySet()); + } + + /** + * Wraps a raw value plus its evaluated-resource set directly. Useful for tests and + * for callers that already hold the underlying value. + */ + public static CqlExpressionValue ofRaw(@Nullable Object value, @Nullable Set evaluatedResources) { + return new CqlExpressionValue(value, evaluatedResources != null ? evaluatedResources : Collections.emptySet()); + } + + /** + * Returns a wrapper whose value is null and whose evaluated-resources set is empty. + */ + public static CqlExpressionValue empty() { + return EMPTY; + } + + public boolean isNull() { + return raw == null; + } + + public boolean isBoolean() { + return raw instanceof Boolean; + } + + public boolean isTrue() { + return Boolean.TRUE.equals(raw); + } + + public boolean isIterable() { + return raw instanceof Iterable; + } + + /** + * True when the underlying value is null, an empty {@link Iterable} (or {@link Collection}), + * or an empty {@link Map}. + */ + public boolean isEmpty() { + if (raw == null) { + return true; + } + if (raw instanceof Collection collection) { + return collection.isEmpty(); + } + if (raw instanceof Map map) { + return map.isEmpty(); + } + if (raw instanceof Iterable iterable) { + return !iterable.iterator().hasNext(); + } + return false; + } + + public Optional asBoolean() { + return raw instanceof Boolean b ? Optional.of(b) : Optional.empty(); + } + + /** + * Normalizes the value to an {@link Iterable}: null becomes an empty list, an existing + * iterable is returned as-is, and a scalar is wrapped in a single-element list. + */ + @SuppressWarnings("unchecked") + public Iterable asIterable() { + if (raw == null) { + return Collections.emptyList(); + } + if (raw instanceof Iterable) { + return (Iterable) raw; + } + return Collections.singletonList(raw); + } + + /** + * Like {@link #asIterable()} but preserves a true-null result rather than coercing to + * an empty list. Mirrors the legacy {@code evaluateSupportingCriteria} contract where + * a null indicates "no result evaluated" and is meaningful to downstream consumers. + */ + @SuppressWarnings("unchecked") + public @Nullable Iterable asIterableOrNull() { + if (raw == null) { + return null; + } + if (raw instanceof Iterable) { + return (Iterable) raw; + } + return Collections.singletonList(raw); + } + + /** + * Resolves a population-criterion value to an iterable of population members. + *

+ *

    + *
  • If the value is null, returns an empty list.
    • + *
  • If the value is {@link Boolean#TRUE}, looks up {@code subjectType} in the + * provided {@link EvaluationResult} and returns its single resolved value + * (the subject context resource).
    • + *
  • If the value is {@link Boolean#FALSE}, returns an empty list.
    • + *
  • Otherwise, normalizes via {@link #asIterable()}.
    • + *
+ * Throws {@link CqlExpressionValueException} when the {@code subjectType} lookup + * yields no expression result for a {@code Boolean.TRUE} criterion. + */ + public Iterable resolveForPopulation(String subjectType, EvaluationResult evaluationResult) { + if (raw == null) { + return Collections.emptyList(); + } + if (raw instanceof Boolean b) { + if (!b) { + return Collections.emptyList(); + } + ExpressionResult subjectResult = evaluationResult.get(subjectType); + if (subjectResult == null) { + throw new CqlExpressionValueException( + "expression result is null for subject type: %s".formatted(subjectType)); + } + return Collections.singletonList(subjectResult.getValue()); + } + return asIterable(); + } + + public Set evaluatedResources() { + return evaluatedResources; + } + + /** + * Escape hatch for callers that still need the underlying {@link Object}. Preserved + * during the migration from the legacy {@code Object}-typed pipeline to the eventual + * sealed {@code Value} type. + */ + public @Nullable Object raw() { + return raw; + } +} diff --git a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/CqlExpressionValueException.java b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/CqlExpressionValueException.java new file mode 100644 index 0000000000..55a0e07c28 --- /dev/null +++ b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/CqlExpressionValueException.java @@ -0,0 +1,12 @@ +package org.opencds.cqf.fhir.cr.measure.common; + +/** + * Thrown when a value extracted from a CQL {@code ExpressionResult} cannot be normalized + * by {@link CqlExpressionValue} into the shape the caller expects (for example, when a + * Boolean criterion's resolved subject-context lookup returns no result). + */ +public class CqlExpressionValueException extends RuntimeException { + public CqlExpressionValueException(String message) { + super(message); + } +} diff --git a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/FunctionEvaluationHandler.java b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/FunctionEvaluationHandler.java index 8d6eb05c80..e8409ed314 100644 --- a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/FunctionEvaluationHandler.java +++ b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/FunctionEvaluationHandler.java @@ -4,7 +4,6 @@ import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; import java.util.ArrayList; import java.util.Collection; -import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.List; @@ -600,32 +599,7 @@ private static Optional tryGetExpressionResult( private static Iterable getResultIterable( EvaluationResult evaluationResult, ExpressionResult expressionResult, String subjectTypePart) { - if (expressionResult.getValue() instanceof Boolean) { - if ((Boolean.TRUE.equals(expressionResult.getValue()))) { - // if Boolean, returns context by SubjectType - var expressionResultForSubjectId = evaluationResult.get(subjectTypePart); - - if (expressionResultForSubjectId == null) { - throw new InternalErrorException( - "expression result is null for subject type: %s".formatted(subjectTypePart)); - } - - Object booleanResult = expressionResultForSubjectId.getValue(); - - // remove evaluated resources - return Collections.singletonList(booleanResult); - } else { - // false result shows nothing - return Collections.emptyList(); - } - } - - Object value = expressionResult.getValue(); - if (value instanceof Iterable iterable) { - return iterable; - } else { - return Collections.singletonList(value); - } + return CqlExpressionValue.of(expressionResult).resolveForPopulation(subjectTypePart, evaluationResult); } private static List getFunctionArguments(GroupDef groupDef, Object result) { diff --git a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureEvaluator.java b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureEvaluator.java index 45bb8e6df1..9475760f92 100644 --- a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureEvaluator.java +++ b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureEvaluator.java @@ -14,7 +14,6 @@ import ca.uhn.fhir.rest.server.exceptions.InternalErrorException; import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; import jakarta.annotation.Nullable; -import java.util.Collections; import java.util.Iterator; import java.util.List; import java.util.Map; @@ -112,59 +111,19 @@ protected MeasureDef evaluateSubject( return measureDef; } - @SuppressWarnings("unchecked") protected Iterable evaluatePopulationCriteria( String subjectType, ExpressionResult expressionResult, EvaluationResult evaluationResult, Set outEvaluatedResources) { - if (expressionResult != null - && !expressionResult.getEvaluatedResources().isEmpty()) { - outEvaluatedResources.addAll(expressionResult.getEvaluatedResources()); - } - - if (expressionResult == null || expressionResult.getValue() == null) { - return Collections.emptyList(); - } - - if (expressionResult.getValue() instanceof Boolean) { - if ((Boolean.TRUE.equals(expressionResult.getValue()))) { - // if Boolean, returns context by SubjectType - Object booleanResult = evaluationResult.get(subjectType).getValue(); - // remove evaluated resources - return Collections.singletonList(booleanResult); - } else { - // false result shows nothing - return Collections.emptyList(); - } - } - - Object value = expressionResult.getValue(); - if (value instanceof Iterable) { - return (Iterable) value; - } else { - return Collections.singletonList(value); - } + var wrapper = CqlExpressionValue.of(expressionResult); + outEvaluatedResources.addAll(wrapper.evaluatedResources()); + return wrapper.resolveForPopulation(subjectType, evaluationResult); } - @SuppressWarnings("unchecked") protected Iterable evaluateSupportingCriteria(ExpressionResult expressionResult) { - - // Case 1 — true null - if (expressionResult == null || expressionResult.getValue() == null) { - return null; // need to preserve result - } - - Object value = expressionResult.getValue(); - - // Case 2 — list - if (value instanceof Iterable) { - return (Iterable) value; // may be empty or not - } - - // Case 3 — scalar - return List.of(value); + return CqlExpressionValue.of(expressionResult).asIterableOrNull(); } protected PopulationDef evaluatePopulationMembership( diff --git a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/PopulationBasisValidator.java b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/PopulationBasisValidator.java index 520ad0a1b6..8840685354 100644 --- a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/PopulationBasisValidator.java +++ b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/PopulationBasisValidator.java @@ -112,13 +112,13 @@ private void validateGroupPopulationBasisType( return; } - var expressionResult = evaluationResult.get(populationExpression); + var wrapper = CqlExpressionValue.of(evaluationResult.get(populationExpression)); - if (expressionResult == null || expressionResult.getValue() == null) { + if (wrapper.isNull()) { return; } - var resultClasses = StratifierUtils.extractClassesFromSingleOrListResult(expressionResult.getValue()); + var resultClasses = StratifierUtils.extractClassesFromSingleOrListResult(wrapper.raw()); var groupPopulationBasisCode = groupDef.getPopulationBasis().code(); var optResourceClass = extractResourceType(groupPopulationBasisCode); @@ -161,13 +161,13 @@ private void validateExpressionResultType( EvaluationResult evaluationResult, String url) { - var expressionResult = evaluationResult.get(expression); + var wrapper = CqlExpressionValue.of(evaluationResult.get(expression)); - if (expressionResult == null || expressionResult.getValue() == null) { + if (wrapper.isNull()) { return; } - var resultClasses = StratifierUtils.extractClassesFromSingleOrListResult(expressionResult.getValue()); + var resultClasses = StratifierUtils.extractClassesFromSingleOrListResult(wrapper.raw()); var groupPopulationBasisCode = groupDef.getPopulationBasis().code(); if (stratifierDef.isCriteriaStratifier()) { diff --git a/cqf-fhir-cr/src/test/java/org/opencds/cqf/fhir/cr/measure/common/CqlExpressionValueTest.java b/cqf-fhir-cr/src/test/java/org/opencds/cqf/fhir/cr/measure/common/CqlExpressionValueTest.java new file mode 100644 index 0000000000..a24986d755 --- /dev/null +++ b/cqf-fhir-cr/src/test/java/org/opencds/cqf/fhir/cr/measure/common/CqlExpressionValueTest.java @@ -0,0 +1,279 @@ +package org.opencds.cqf.fhir.cr.measure.common; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.math.BigDecimal; +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.stream.StreamSupport; +import org.hl7.fhir.r4.model.Encounter; +import org.hl7.fhir.r4.model.Patient; +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.MethodSource; +import org.opencds.cqf.cql.engine.execution.EvaluationExpressionRef; +import org.opencds.cqf.cql.engine.execution.EvaluationResult; +import org.opencds.cqf.cql.engine.execution.ExpressionResult; + +class CqlExpressionValueTest { + + @Test + void of_nullExpressionResult_returnsEmpty() { + CqlExpressionValue wrapper = CqlExpressionValue.of(null); + + assertTrue(wrapper.isNull()); + assertTrue(wrapper.isEmpty()); + assertSame(CqlExpressionValue.empty(), wrapper); + assertEquals(Set.of(), wrapper.evaluatedResources()); + } + + @Test + void of_expressionResult_propagatesValueAndResources() { + Patient patient = new Patient(); + patient.setId("p1"); + Set resources = new HashSet<>(List.of(patient)); + ExpressionResult result = new ExpressionResult(patient, resources); + + CqlExpressionValue wrapper = CqlExpressionValue.of(result); + + assertSame(patient, wrapper.raw()); + assertEquals(resources, wrapper.evaluatedResources()); + } + + @Test + void of_expressionResultWithNullResources_substitutesEmptySet() { + ExpressionResult result = new ExpressionResult("v", null); + + CqlExpressionValue wrapper = CqlExpressionValue.of(result); + + assertEquals(Set.of(), wrapper.evaluatedResources()); + } + + @Test + void ofRaw_acceptsNullResources() { + CqlExpressionValue wrapper = CqlExpressionValue.ofRaw(42, null); + + assertEquals(42, wrapper.raw()); + assertEquals(Set.of(), wrapper.evaluatedResources()); + } + + // -- predicates -------------------------------------------------------------- + + @Test + void isBooleanAndIsTrue_onlyForBooleanRawValues() { + assertTrue(CqlExpressionValue.ofRaw(true, null).isBoolean()); + assertTrue(CqlExpressionValue.ofRaw(true, null).isTrue()); + assertTrue(CqlExpressionValue.ofRaw(false, null).isBoolean()); + assertFalse(CqlExpressionValue.ofRaw(false, null).isTrue()); + assertFalse(CqlExpressionValue.ofRaw("string", null).isBoolean()); + assertFalse(CqlExpressionValue.ofRaw(null, null).isTrue()); + } + + @Test + void isIterable_trueForCollectionsAndOtherIterables() { + assertTrue(CqlExpressionValue.ofRaw(List.of(1, 2), null).isIterable()); + assertTrue(CqlExpressionValue.ofRaw(Set.of(1, 2), null).isIterable()); + assertFalse(CqlExpressionValue.ofRaw("string", null).isIterable()); + assertFalse(CqlExpressionValue.ofRaw(null, null).isIterable()); + } + + @ParameterizedTest + @MethodSource("emptyValues") + void isEmpty_recognizesNullEmptyIterableEmptyMap(Object raw) { + assertTrue(CqlExpressionValue.ofRaw(raw, null).isEmpty()); + } + + static java.util.stream.Stream emptyValues() { + return java.util.stream.Stream.of(null, Collections.emptyList(), Collections.emptySet(), Map.of()); + } + + @Test + void isEmpty_falseForNonEmptyContainersAndScalars() { + assertFalse(CqlExpressionValue.ofRaw(List.of(1), null).isEmpty()); + assertFalse(CqlExpressionValue.ofRaw(Map.of("k", "v"), null).isEmpty()); + assertFalse(CqlExpressionValue.ofRaw("anything", null).isEmpty()); + assertFalse(CqlExpressionValue.ofRaw(false, null).isEmpty()); + } + + // -- asBoolean --------------------------------------------------------------- + + @Test + void asBoolean_presentOnlyForBooleanValues() { + assertEquals( + java.util.Optional.of(true), + CqlExpressionValue.ofRaw(true, null).asBoolean()); + assertEquals( + java.util.Optional.of(false), + CqlExpressionValue.ofRaw(false, null).asBoolean()); + assertEquals( + java.util.Optional.empty(), + CqlExpressionValue.ofRaw("not-bool", null).asBoolean()); + assertEquals( + java.util.Optional.empty(), CqlExpressionValue.ofRaw(null, null).asBoolean()); + } + + // -- asIterable / asIterableOrNull ------------------------------------------ + + @ParameterizedTest + @MethodSource("asIterableCases") + void asIterable_normalizesAllShapes(Object raw, List expected) { + Iterable actual = CqlExpressionValue.ofRaw(raw, null).asIterable(); + assertEquals(expected, toList(actual)); + } + + static java.util.stream.Stream asIterableCases() { + Patient patient = new Patient(); + patient.setId("p1"); + Encounter encounter = new Encounter(); + encounter.setId("e1"); + return java.util.stream.Stream.of( + Arguments.of(null, List.of()), + Arguments.of(true, List.of(true)), + Arguments.of(false, List.of(false)), + Arguments.of(List.of(), List.of()), + Arguments.of(List.of(patient), List.of(patient)), + Arguments.of(List.of(patient, encounter), List.of(patient, encounter)), + Arguments.of("string", List.of("string")), + Arguments.of(BigDecimal.ONE, List.of(BigDecimal.ONE)), + Arguments.of(42, List.of(42)), + Arguments.of(Map.of("k", "v"), List.of(Map.of("k", "v")))); + } + + @Test + void asIterable_passesThroughIterableInstance() { + ArrayList source = new ArrayList<>(List.of("a", "b")); + Iterable result = CqlExpressionValue.ofRaw(source, null).asIterable(); + + // Same iterable instance is returned (no copying) + assertSame(source, result); + } + + @Test + void asIterableOrNull_preservesNullForTrueNullValue() { + assertNull(CqlExpressionValue.ofRaw(null, null).asIterableOrNull()); + } + + @Test + void asIterableOrNull_normalizesScalarToSingletonList() { + Iterable result = CqlExpressionValue.ofRaw("scalar", null).asIterableOrNull(); + + assertNotNull(result); + assertEquals(List.of("scalar"), toList(result)); + } + + @Test + void asIterableOrNull_passesThroughIterable() { + List source = List.of("a", "b"); + assertSame(source, CqlExpressionValue.ofRaw(source, null).asIterableOrNull()); + } + + // -- resolveForPopulation ---------------------------------------------------- + + @Test + void resolveForPopulation_nullValueReturnsEmpty() { + EvaluationResult evaluationResult = new EvaluationResult(); + + Iterable result = + CqlExpressionValue.ofRaw(null, null).resolveForPopulation("Patient", evaluationResult); + + assertEquals(List.of(), toList(result)); + } + + @Test + void resolveForPopulation_falseReturnsEmpty() { + EvaluationResult evaluationResult = new EvaluationResult(); + evaluationResult.set(new EvaluationExpressionRef("Patient"), new ExpressionResult(new Patient(), Set.of())); + + Iterable result = + CqlExpressionValue.ofRaw(false, null).resolveForPopulation("Patient", evaluationResult); + + assertEquals(List.of(), toList(result)); + } + + @Test + void resolveForPopulation_trueLooksUpSubjectContextValue() { + Patient patient = new Patient(); + patient.setId("p1"); + EvaluationResult evaluationResult = new EvaluationResult(); + evaluationResult.set(new EvaluationExpressionRef("Patient"), new ExpressionResult(patient, Set.of())); + + Iterable result = + CqlExpressionValue.ofRaw(true, null).resolveForPopulation("Patient", evaluationResult); + + List resolved = toList(result); + assertEquals(1, resolved.size()); + assertSame(patient, resolved.get(0)); + } + + @Test + void resolveForPopulation_trueButNoSubjectResultThrows() { + EvaluationResult evaluationResult = new EvaluationResult(); + + CqlExpressionValueException ex = assertThrows( + CqlExpressionValueException.class, + () -> CqlExpressionValue.ofRaw(true, null).resolveForPopulation("Patient", evaluationResult)); + + assertTrue(ex.getMessage().contains("Patient")); + } + + @Test + void resolveForPopulation_iterableReturnedAsIs() { + Patient p1 = new Patient(); + p1.setId("p1"); + Patient p2 = new Patient(); + p2.setId("p2"); + List source = List.of(p1, p2); + + Iterable result = + CqlExpressionValue.ofRaw(source, null).resolveForPopulation("Patient", new EvaluationResult()); + + assertSame(source, result); + } + + @Test + void resolveForPopulation_scalarWrappedInSingletonList() { + Encounter encounter = new Encounter(); + encounter.setId("e1"); + + Iterable result = + CqlExpressionValue.ofRaw(encounter, null).resolveForPopulation("Patient", new EvaluationResult()); + + assertEquals(List.of(encounter), toList(result)); + } + + // -- evaluatedResources / raw ------------------------------------------------ + + @Test + void evaluatedResources_returnsTheBackingSet() { + Set resources = new HashSet<>(List.of("r1", "r2")); + CqlExpressionValue wrapper = CqlExpressionValue.ofRaw("v", resources); + + assertSame(resources, wrapper.evaluatedResources()); + } + + @Test + void raw_returnsUnderlyingObject() { + Map accumulator = new HashMap<>(); + accumulator.put("k", 1); + + CqlExpressionValue wrapper = CqlExpressionValue.ofRaw(accumulator, null); + + assertSame(accumulator, wrapper.raw()); + } + + private static List toList(Iterable it) { + return StreamSupport.stream(it.spliterator(), false).toList(); + } +} From dfe96d755d5ee5e62c8836b0899d4debbe1c419b Mon Sep 17 00:00:00 2001 From: Luke deGruchy Date: Wed, 29 Apr 2026 16:42:50 -0400 Subject: [PATCH 02/13] Absorb CriteriaResult into CqlExpressionValue Phase 2 of the wrapper migration. Adds valueAsSet() and nonNullValues() to CqlExpressionValue (matching the existing CriteriaResult contract, including HashSetForFhirResourcesAndCqlTypes identity semantics for valueAsSet) and switches the per-subject result maps on SdeDef, StratifierDef, and StratifierComponentDef from CriteriaResult to CqlExpressionValue. Updates the only external consumers (MeasureMultiSubjectEvaluator and Dstu3MeasureReportBuilder) to call raw() / valueAsSet() / nonNullValues() on the new wrapper. The CriteriaResult class is now redundant and is removed; its unused NULL_VALUE / EMPTY_RESULT sentinels go with it. The public StratifierDef.getAllCriteriaResultValues() method retains its name to avoid disturbing call sites; only its return type semantics (now backed by CqlExpressionValue) change. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../cr/measure/common/CqlExpressionValue.java | 37 +++++++++ .../cr/measure/common/CriteriaResult.java | 77 ------------------- .../common/MeasureMultiSubjectEvaluator.java | 22 +++--- .../cqf/fhir/cr/measure/common/SdeDef.java | 6 +- .../common/StratifierComponentDef.java | 6 +- .../fhir/cr/measure/common/StratifierDef.java | 10 ++- .../dstu3/Dstu3MeasureReportBuilder.java | 2 +- .../common/CqlExpressionValueTest.java | 63 +++++++++++++++ 8 files changed, 124 insertions(+), 99 deletions(-) delete mode 100644 cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/CriteriaResult.java diff --git a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/CqlExpressionValue.java b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/CqlExpressionValue.java index 4ee900e77e..44e25bb535 100644 --- a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/CqlExpressionValue.java +++ b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/CqlExpressionValue.java @@ -1,11 +1,15 @@ package org.opencds.cqf.fhir.cr.measure.common; import jakarta.annotation.Nullable; +import java.util.ArrayList; import java.util.Collection; import java.util.Collections; +import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Optional; import java.util.Set; +import java.util.stream.StreamSupport; import org.opencds.cqf.cql.engine.execution.EvaluationResult; import org.opencds.cqf.cql.engine.execution.ExpressionResult; @@ -160,6 +164,39 @@ public Iterable resolveForPopulation(String subjectType, EvaluationResul return asIterable(); } + /** + * Returns the underlying value(s) as a {@link Set} that uses FHIR-resource and CQL-type + * identity semantics: scalars are wrapped in a single-element set, iterables are flattened + * into the set, and a null value yields an empty set. + */ + @SuppressWarnings({"unchecked", "rawtypes"}) + public Set valueAsSet() { + if (raw == null) { + return new HashSetForFhirResourcesAndCqlTypes<>(); + } + if (raw instanceof Iterable) { + return new HashSetForFhirResourcesAndCqlTypes<>((Iterable) raw); + } + return new HashSetForFhirResourcesAndCqlTypes<>(raw); + } + + /** + * Returns the underlying value(s) as a {@link List} with nulls filtered out. A scalar + * becomes a single-element list, an iterable is flattened (preserving order, dropping + * nulls), and a null value yields an empty list. + */ + public List nonNullValues() { + if (raw == null) { + return Collections.emptyList(); + } + if (raw instanceof Iterable iterable) { + return StreamSupport.stream(iterable.spliterator(), false) + .filter(Objects::nonNull) + .collect(ArrayList::new, ArrayList::add, ArrayList::addAll); + } + return List.of(raw); + } + public Set evaluatedResources() { return evaluatedResources; } diff --git a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/CriteriaResult.java b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/CriteriaResult.java deleted file mode 100644 index 1a6b1de6e3..0000000000 --- a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/CriteriaResult.java +++ /dev/null @@ -1,77 +0,0 @@ -package org.opencds.cqf.fhir.cr.measure.common; - -import java.util.ArrayList; -import java.util.Collections; -import java.util.HashSet; -import java.util.List; -import java.util.Objects; -import java.util.Set; -import java.util.stream.StreamSupport; - -public class CriteriaResult { - private final Object value; - private final Set evaluatedResources; - - public static final Object NULL_VALUE = new Object(); - - public static final CriteriaResult EMPTY_RESULT = new CriteriaResult(NULL_VALUE, Collections.emptySet()); - - public CriteriaResult(Object value, Set evaluatedResources) { - this.value = value; - this.evaluatedResources = new HashSet<>(evaluatedResources); - } - - public Object rawValue() { - return this.value; - } - - @SuppressWarnings({"unchecked", "rawtypes"}) - public Iterable iterableValue() { - if (this.rawValue() instanceof Iterable) { - return (Iterable) this.rawValue(); - } else if (this.rawValue() == null) { - return Collections.emptyList(); - } else { - return Collections.singletonList(this.rawValue()); - } - } - - public Set valueAsSet() { - if (this.rawValue() instanceof Iterable) { - return buildSet(unsafeCast(this.rawValue())); - } else if (this.rawValue() == null) { - return new HashSetForFhirResourcesAndCqlTypes<>(); - } else { - return new HashSetForFhirResourcesAndCqlTypes<>(this.rawValue()); - } - } - - public Set evaluatedResources() { - return this.evaluatedResources; - } - - /** - * Returns the value(s) as a list with nulls filtered out. - * Handles single values, iterables, and null values uniformly. - */ - public List nonNullValues() { - if (this.value == null) { - return Collections.emptyList(); - } - if (this.value instanceof Iterable iterable) { - return StreamSupport.stream(iterable.spliterator(), false) - .filter(Objects::nonNull) - .collect(ArrayList::new, ArrayList::add, ArrayList::addAll); - } - return List.of(this.value); - } - - private Set buildSet(Iterable iterable) { - return new HashSetForFhirResourcesAndCqlTypes<>(iterable); - } - - @SuppressWarnings("unchecked") - private static T unsafeCast(Object object) { - return (T) object; - } -} diff --git a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureMultiSubjectEvaluator.java b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureMultiSubjectEvaluator.java index af9b50971c..8da2379e24 100644 --- a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureMultiSubjectEvaluator.java +++ b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureMultiSubjectEvaluator.java @@ -496,8 +496,8 @@ private static Map> collectFunctionRowKeys( for (StratifierComponentDef componentDef : componentDefs) { for (var entry : componentDef.getResults().entrySet()) { String subjectId = entry.getKey(); - CriteriaResult result = entry.getValue(); - Object rawValue = result == null ? null : result.rawValue(); + CqlExpressionValue result = entry.getValue(); + Object rawValue = result == null ? null : result.raw(); // Only process function results (Map values) if (rawValue instanceof Map functionResults) { @@ -528,10 +528,10 @@ private static List mapToListOfTableEntries( private record StratumTableRow(StratifierRowKey stratifierRowKey, StratumValueWrapper stratumValueWrapper) {} private static List mapToListOfTableEntries( - String subjectId, CriteriaResult result, Map> functionRowKeysBySubject) { + String subjectId, CqlExpressionValue result, Map> functionRowKeysBySubject) { final String qualifiedSubject = FhirResourceUtils.addPatientQualifier(subjectId); - final Object rawValue = result == null ? null : result.rawValue(); + final Object rawValue = result == null ? null : result.raw(); if (rawValue instanceof Map functionResults) { return addFunctionResultRows(qualifiedSubject, functionResults); @@ -749,19 +749,19 @@ private static Map, List> groupSubjectsBy *

Intersection rules: *

    *
  • If the stratifier result is {@code Map}, intersect using {@code map.keySet()} (the input params)
    • - *
  • Otherwise, intersect using {@link CriteriaResult#valueAsSet()}
    • + *
  • Otherwise, intersect using {@link CqlExpressionValue#valueAsSet()}
    • *
*/ private static Set calculateCriteriaStratifierIntersection( StratifierDef stratifierDef, PopulationDef populationDef) { - final Map stratifierResultsBySubject = stratifierDef.getResults(); + final Map stratifierResultsBySubject = stratifierDef.getResults(); final List allPopulationStratumIntersectingResources = new ArrayList<>(); // For each subject, we intersect between the population and stratifier results - for (Entry stratifierEntryBySubject : stratifierResultsBySubject.entrySet()) { + for (Entry stratifierEntryBySubject : stratifierResultsBySubject.entrySet()) { final Set stratifierResultsPerSubject = - criteriaResultAsIntersectionSet(stratifierEntryBySubject.getValue()); + stratifierResultAsIntersectionSet(stratifierEntryBySubject.getValue()); final Set populationResultsPerSubject = populationDef.getResourcesForSubject(stratifierEntryBySubject.getKey()); @@ -775,17 +775,17 @@ private static Set calculateCriteriaStratifierIntersection( } /** - * Convert a CriteriaResult into the set that should be used for intersection. + * Convert a stratifier result into the set that should be used for intersection. * *

For Map-based results (Map), the input parameters (map keys) * are the intersectable items. */ - private static Set criteriaResultAsIntersectionSet(CriteriaResult result) { + private static Set stratifierResultAsIntersectionSet(CqlExpressionValue result) { if (result == null) { return Set.of(); } - Object raw = result.rawValue(); + Object raw = result.raw(); if (raw instanceof Map m) { return new HashSet<>(m.keySet()); } diff --git a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/SdeDef.java b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/SdeDef.java index 3b3c87a13a..c324c13bdb 100644 --- a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/SdeDef.java +++ b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/SdeDef.java @@ -13,7 +13,7 @@ public class SdeDef { private final ConceptDef code; private final String expression; private final String description; - private final Map results = new HashMap<>(); + private final Map results = new HashMap<>(); // Pre-accumulated state (populated by MeasureMultiSubjectEvaluator) private final Map accumulatedValues = new HashMap<>(); @@ -47,7 +47,7 @@ public String description() { } public void putResult(String subject, Object value, Set evaluatedResources) { - this.results.put(subject, new CriteriaResult(value, evaluatedResources)); + this.results.put(subject, CqlExpressionValue.ofRaw(value, evaluatedResources)); } public Map getAccumulatedValues() { @@ -64,7 +64,7 @@ public Set getAllEvaluatedResources() { */ public void accumulate() { // Merge all evaluated resources across subjects - for (CriteriaResult result : results.values()) { + for (CqlExpressionValue result : results.values()) { allEvaluatedResources.addAll(result.evaluatedResources()); } diff --git a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/StratifierComponentDef.java b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/StratifierComponentDef.java index 53a7a01590..6346b5a289 100644 --- a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/StratifierComponentDef.java +++ b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/StratifierComponentDef.java @@ -9,7 +9,7 @@ public class StratifierComponentDef { private final ConceptDef code; private final String expression; - private Map results; + private Map results; public StratifierComponentDef(String id, ConceptDef code, String expression) { this.id = id; @@ -30,10 +30,10 @@ public ConceptDef code() { } public void putResult(String subject, Object value, Set evaluatedResources) { - this.getResults().put(subject, new CriteriaResult(value, evaluatedResources)); + this.getResults().put(subject, CqlExpressionValue.ofRaw(value, evaluatedResources)); } - public Map getResults() { + public Map getResults() { if (this.results == null) { this.results = new HashMap<>(); } diff --git a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/StratifierDef.java b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/StratifierDef.java index 27ca2bd965..26b82347f2 100644 --- a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/StratifierDef.java +++ b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/StratifierDef.java @@ -23,7 +23,7 @@ public class StratifierDef { private final List stratum = new ArrayList<>(); @Nullable - private Map results; + private Map results; public StratifierDef(String id, ConceptDef code, String expression, MeasureStratifierType stratifierType) { this(id, code, expression, stratifierType, Collections.emptyList()); @@ -76,10 +76,12 @@ public List components() { public void putResult(String subject, Object value, Set evaluatedResources) { this.getResults() - .put(subject, new CriteriaResult(value, new HashSetForFhirResourcesAndCqlTypes<>(evaluatedResources))); + .put( + subject, + CqlExpressionValue.ofRaw(value, new HashSetForFhirResourcesAndCqlTypes<>(evaluatedResources))); } - public Map getResults() { + public Map getResults() { if (this.results == null) { this.results = new HashMap<>(); } @@ -90,7 +92,7 @@ public Map getResults() { // Ensure we handle FHIR resource identity properly public Set getAllCriteriaResultValues() { return new HashSetForFhirResourcesAndCqlTypes<>(this.getResults().values().stream() - .map(CriteriaResult::rawValue) + .map(CqlExpressionValue::raw) .map(this::toSet) .flatMap(Collection::stream) .collect(Collectors.toUnmodifiableSet())); diff --git a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/dstu3/Dstu3MeasureReportBuilder.java b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/dstu3/Dstu3MeasureReportBuilder.java index dbdaf15a77..481ee887e5 100644 --- a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/dstu3/Dstu3MeasureReportBuilder.java +++ b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/dstu3/Dstu3MeasureReportBuilder.java @@ -202,7 +202,7 @@ protected void buildStratifier( // the StratumValueWrapper does it for them. Map> subjectsByValue = subjectValues.keySet().stream() .collect(Collectors.groupingBy( - x -> new StratumValueWrapper(subjectValues.get(x).rawValue()))); + x -> new StratumValueWrapper(subjectValues.get(x).raw()))); for (Map.Entry> stratValue : subjectsByValue.entrySet()) { buildStratum( diff --git a/cqf-fhir-cr/src/test/java/org/opencds/cqf/fhir/cr/measure/common/CqlExpressionValueTest.java b/cqf-fhir-cr/src/test/java/org/opencds/cqf/fhir/cr/measure/common/CqlExpressionValueTest.java index a24986d755..83dabd3b15 100644 --- a/cqf-fhir-cr/src/test/java/org/opencds/cqf/fhir/cr/measure/common/CqlExpressionValueTest.java +++ b/cqf-fhir-cr/src/test/java/org/opencds/cqf/fhir/cr/measure/common/CqlExpressionValueTest.java @@ -253,6 +253,69 @@ void resolveForPopulation_scalarWrappedInSingletonList() { assertEquals(List.of(encounter), toList(result)); } + // -- valueAsSet -------------------------------------------------------------- + + @Test + void valueAsSet_nullValueYieldsEmptySet() { + Set set = CqlExpressionValue.ofRaw(null, null).valueAsSet(); + + assertTrue(set instanceof HashSetForFhirResourcesAndCqlTypes); + assertTrue(set.isEmpty()); + } + + @Test + void valueAsSet_scalarYieldsSingletonSet() { + Patient patient = new Patient(); + patient.setId("p1"); + + Set set = CqlExpressionValue.ofRaw(patient, null).valueAsSet(); + + assertTrue(set instanceof HashSetForFhirResourcesAndCqlTypes); + assertEquals(1, set.size()); + assertTrue(set.contains(patient)); + } + + @Test + void valueAsSet_iterableFlattensIntoSet() { + Patient p1 = new Patient(); + p1.setId("p1"); + Patient p2 = new Patient(); + p2.setId("p2"); + + Set set = CqlExpressionValue.ofRaw(List.of(p1, p2), null).valueAsSet(); + + assertTrue(set instanceof HashSetForFhirResourcesAndCqlTypes); + assertEquals(2, set.size()); + } + + // -- nonNullValues ----------------------------------------------------------- + + @Test + void nonNullValues_nullValueYieldsEmptyList() { + assertEquals(List.of(), CqlExpressionValue.ofRaw(null, null).nonNullValues()); + } + + @Test + void nonNullValues_scalarYieldsSingletonList() { + assertEquals(List.of("v"), CqlExpressionValue.ofRaw("v", null).nonNullValues()); + } + + @Test + void nonNullValues_iterableFiltersOutNullElements() { + ArrayList source = new ArrayList<>(); + source.add("a"); + source.add(null); + source.add("b"); + source.add(null); + + assertEquals(List.of("a", "b"), CqlExpressionValue.ofRaw(source, null).nonNullValues()); + } + + @Test + void nonNullValues_emptyIterableYieldsEmptyList() { + assertEquals(List.of(), CqlExpressionValue.ofRaw(List.of(), null).nonNullValues()); + } + // -- evaluatedResources / raw ------------------------------------------------ @Test From 9cf0957f221c77586da04a9992c799b091ff89ec Mon Sep 17 00:00:00 2001 From: Luke deGruchy Date: Wed, 29 Apr 2026 17:35:15 -0400 Subject: [PATCH 03/13] Push CqlExpressionValue into observation-accumulator handling Extends CqlExpressionValue with isMap() / asMap() and replaces every remaining instanceof Map / Map.class::isInstance / unsafe-cast site that handles MEASUREOBSERVATION accumulators (Map) so the lone unchecked cast lives in one tested place. Also fixes a latent bug in MeasureEvaluator.retainObservationResources InPopulation: the previous loop modified the same Set it was iterating over (working only by reference-aliasing accident). Replaced with the collect-then-removeAll pattern. Sites migrated: - MeasureEvaluator: retainObservationSubjectResourcesInPopulation, retainObservationResourcesInPopulation (+ bug fix), removeObservatorySubjectResource - MeasureMultiSubjectEvaluator: collectFunctionRowKeys, mapToListOfTableEntries, stratifierResultAsIntersectionSet, getPopulationResourceKeySet - MeasureReportDefScorer: getResultsForStratumByResourceIds - PopulationDef: countObservations, removeExcludedMeasureObservation Resource PopulationDef.subjectResources field type is unchanged; that's a larger architectural change reserved for a future phase. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../cr/measure/common/CqlExpressionValue.java | 15 +++++ .../cr/measure/common/MeasureEvaluator.java | 57 +++++++++++-------- .../common/MeasureMultiSubjectEvaluator.java | 43 +++++++------- .../common/MeasureReportDefScorer.java | 4 +- .../fhir/cr/measure/common/PopulationDef.java | 17 +++--- .../common/CqlExpressionValueTest.java | 47 +++++++++++++++ 6 files changed, 131 insertions(+), 52 deletions(-) diff --git a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/CqlExpressionValue.java b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/CqlExpressionValue.java index 44e25bb535..4546817fa2 100644 --- a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/CqlExpressionValue.java +++ b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/CqlExpressionValue.java @@ -77,6 +77,10 @@ public boolean isIterable() { return raw instanceof Iterable; } + public boolean isMap() { + return raw instanceof Map; + } + /** * True when the underlying value is null, an empty {@link Iterable} (or {@link Collection}), * or an empty {@link Map}. @@ -101,6 +105,17 @@ public Optional asBoolean() { return raw instanceof Boolean b ? Optional.of(b) : Optional.empty(); } + /** + * Returns the underlying value as a typed {@link Map} when it is one, otherwise empty. + * The single unchecked cast is localized here so call sites do not have to repeat it. + * Used for measure-observation accumulators where the CQL engine produces + * {@code Map}. + */ + @SuppressWarnings("unchecked") + public Optional> asMap() { + return raw instanceof Map map ? Optional.of((Map) map) : Optional.empty(); + } + /** * Normalizes the value to an {@link Iterable}: null becomes an empty list, an existing * iterable is returned as-is, and a scalar is wrapped in a single-element list. diff --git a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureEvaluator.java b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureEvaluator.java index 9475760f92..97ceed9ebe 100644 --- a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureEvaluator.java +++ b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureEvaluator.java @@ -14,6 +14,7 @@ import ca.uhn.fhir.rest.server.exceptions.InternalErrorException; import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; import jakarta.annotation.Nullable; +import java.util.ArrayList; import java.util.Iterator; import java.util.List; import java.util.Map; @@ -426,7 +427,6 @@ protected void evaluateContinuousVariable( * Keeps Measure-Observation values found in measurePopulation * are not found in the corresponding measurePopulation set. */ - @SuppressWarnings("unchecked") public void retainObservationSubjectResourcesInPopulation( Map> measurePopulation, Map> measureObservation) { @@ -439,9 +439,7 @@ public void retainObservationSubjectResourcesInPopulation( it.hasNext(); ) { Map.Entry> entry = it.next(); String subjectId = entry.getKey(); - - // Cast subject's observation set to the expected type - Set> obsSet = (Set>) (Set) entry.getValue(); + Set obsSet = entry.getValue(); // get valid population values for this subject Set validPopulation = measurePopulation.get(subjectId); @@ -452,8 +450,13 @@ public void retainObservationSubjectResourcesInPopulation( continue; } - // remove observations not matching population values - obsSet.removeIf(obsMap -> { + // remove observation accumulators whose keys aren't all in the valid population + obsSet.removeIf(item -> { + Map obsMap = + CqlExpressionValue.ofRaw(item, null).asMap().orElse(null); + if (obsMap == null) { + return false; // not an observation accumulator, leave alone + } for (Object key : obsMap.keySet()) { if (!validPopulation.contains(key)) { return true; // remove this observation map @@ -475,23 +478,26 @@ protected void retainObservationResourcesInPopulation( PopulationDef measurePopulationDef, // MeasurePopulationType.MEASUREOBSERVATION PopulationDef measureObservationDef) { - for (Object populationResource : measureObservationDef.getResourcesForSubject(subjectId)) { - if (populationResource instanceof Map measureObservationResourceAsMap) { - for (Entry measureObservationResourceMapEntry : measureObservationResourceAsMap.entrySet()) { - final Object measureObservationSubjectResourceMapKey = measureObservationResourceMapEntry.getKey(); - if (measurePopulationDef != null) { - final Set measurePopulationResourcesForSubject = - measurePopulationDef.getResourcesForSubject(subjectId); - if (!measurePopulationResourcesForSubject.contains(measureObservationSubjectResourceMapKey)) { - // remove observation results not found in measure population - measureObservationDef - .getResourcesForSubject(subjectId) - .remove(populationResource); - } - } + if (measurePopulationDef == null) { + return; + } + Set resourcesForSubject = measureObservationDef.getResourcesForSubject(subjectId); + Set measurePopulationResourcesForSubject = measurePopulationDef.getResourcesForSubject(subjectId); + List toRemove = new ArrayList<>(); + for (Object populationResource : resourcesForSubject) { + Map obsMap = + CqlExpressionValue.ofRaw(populationResource, null).asMap().orElse(null); + if (obsMap == null) { + continue; + } + for (Object key : obsMap.keySet()) { + if (!measurePopulationResourcesForSubject.contains(key)) { + toRemove.add(populationResource); + break; } } } + resourcesForSubject.removeAll(toRemove); } /** @@ -535,12 +541,12 @@ private void removeObservatorySubjectResource( } final Object firstEntryValue = entryValue.iterator().next(); - if (!(firstEntryValue instanceof Map)) { + if (!CqlExpressionValue.ofRaw(firstEntryValue, null).isMap()) { throw new InternalErrorException("Expected a Map but was not: %s".formatted(firstEntryValue)); } @SuppressWarnings("unchecked") - Set> obsSet = (Set>) entryValue; + Set obsSet = (Set) entryValue; // population values for this subject Set populationValues = measurePopulation.get(subjectId); @@ -552,7 +558,12 @@ private void removeObservatorySubjectResource( } // Remove observations that *do* match population values - obsSet.removeIf(obsMap -> { + obsSet.removeIf(item -> { + Map obsMap = + CqlExpressionValue.ofRaw(item, null).asMap().orElse(null); + if (obsMap == null) { + return false; + } for (Object key : obsMap.keySet()) { if (populationValues.contains(key)) { // This observation map is backed by a population resource -> remove iterator diff --git a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureMultiSubjectEvaluator.java b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureMultiSubjectEvaluator.java index 8da2379e24..48fe0932ef 100644 --- a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureMultiSubjectEvaluator.java +++ b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureMultiSubjectEvaluator.java @@ -497,18 +497,22 @@ private static Map> collectFunctionRowKeys( for (var entry : componentDef.getResults().entrySet()) { String subjectId = entry.getKey(); CqlExpressionValue result = entry.getValue(); - Object rawValue = result == null ? null : result.raw(); // Only process function results (Map values) - if (rawValue instanceof Map functionResults) { - String qualifiedSubject = FhirResourceUtils.addPatientQualifier(subjectId); - Set rowKeys = - functionRowKeysBySubject.computeIfAbsent(qualifiedSubject, k -> new HashSet<>()); - - for (Object key : functionResults.keySet()) { - String normalizedKey = normalizeResourceKey(key); - rowKeys.add(StratifierRowKey.withInput(qualifiedSubject, normalizedKey)); - } + if (result == null) { + continue; + } + Map functionResults = result.asMap().orElse(null); + if (functionResults == null) { + continue; + } + String qualifiedSubject = FhirResourceUtils.addPatientQualifier(subjectId); + Set rowKeys = + functionRowKeysBySubject.computeIfAbsent(qualifiedSubject, k -> new HashSet<>()); + + for (Object key : functionResults.keySet()) { + String normalizedKey = normalizeResourceKey(key); + rowKeys.add(StratifierRowKey.withInput(qualifiedSubject, normalizedKey)); } } } @@ -533,11 +537,11 @@ private static List mapToListOfTableEntries( final String qualifiedSubject = FhirResourceUtils.addPatientQualifier(subjectId); final Object rawValue = result == null ? null : result.raw(); - if (rawValue instanceof Map functionResults) { - return addFunctionResultRows(qualifiedSubject, functionResults); - - } else if (rawValue instanceof Iterable iterableValue) { - return addIterableValueRows(qualifiedSubject, iterableValue); + if (result != null && result.isMap()) { + return addFunctionResultRows(qualifiedSubject, result.asMap().orElseThrow()); + } + if (result != null && result.isIterable()) { + return addIterableValueRows(qualifiedSubject, (Iterable) rawValue); } // Scalar value: check if we need to expand to match function row keys @@ -785,8 +789,8 @@ private static Set stratifierResultAsIntersectionSet(CqlExpressionValue return Set.of(); } - Object raw = result.raw(); - if (raw instanceof Map m) { + Map m = result.asMap().orElse(null); + if (m != null) { return new HashSet<>(m.keySet()); } @@ -924,8 +928,9 @@ private static Set getPopulationResourceKeySet( if (populationDef.type() == MeasurePopulationType.MEASUREOBSERVATION) { // MEASUREOBSERVATION always deals with FHIR resources, so no subject qualification needed resources.stream() - .filter(Map.class::isInstance) - .map(m -> (Map) m) + .map(item -> + CqlExpressionValue.ofRaw(item, null).asMap()) + .flatMap(java.util.Optional::stream) .flatMap(m -> m.keySet().stream()) .map(MeasureMultiSubjectEvaluator::normalizePopulationKey) .filter(java.util.Objects::nonNull) diff --git a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureReportDefScorer.java b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureReportDefScorer.java index 190cf6551a..89bf44d615 100644 --- a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureReportDefScorer.java +++ b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureReportDefScorer.java @@ -542,8 +542,8 @@ private static Collection getResultsForStratumByResourceIds( if (populationDef.type() == MeasurePopulationType.MEASUREOBSERVATION) { return populationDef.getSubjectResources().values().stream() .flatMap(Collection::stream) - .filter(Map.class::isInstance) - .map(m -> (Map) m) + .map(item -> CqlExpressionValue.ofRaw(item, null).asMap()) + .flatMap(java.util.Optional::stream) .map(map -> { // Filter the map to only include entries matching stratum resource IDs Map filteredMap = new java.util.HashMap<>(); diff --git a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/PopulationDef.java b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/PopulationDef.java index 98a2ac86f0..fba4cae5a9 100644 --- a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/PopulationDef.java +++ b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/PopulationDef.java @@ -129,14 +129,15 @@ public void removeExcludedMeasureObservationResource(String subjectId, Object me } // Remove the key from all inner maps - resourcesForSubject.forEach(element -> { - if (element instanceof Map innerMap) { - innerMap.remove(measureObservationResourceKey); - } - }); + resourcesForSubject.forEach(element -> CqlExpressionValue.ofRaw(element, null) + .asMap() + .ifPresent(innerMap -> innerMap.remove(measureObservationResourceKey))); // Remove empty inner maps - critical for correct counting - resourcesForSubject.removeIf(element -> element instanceof Map m && m.isEmpty()); + resourcesForSubject.removeIf(element -> CqlExpressionValue.ofRaw(element, null) + .asMap() + .map(Map::isEmpty) + .orElse(false)); // If the subject's resource set is now empty, remove the subject from the map entirely if (resourcesForSubject.isEmpty()) { @@ -193,8 +194,8 @@ public int countObservations() { } return this.getAllSubjectResources().stream() - .filter(Map.class::isInstance) - .map(Map.class::cast) + .map(item -> CqlExpressionValue.ofRaw(item, null).asMap()) + .flatMap(java.util.Optional::stream) .mapToInt(Map::size) .sum(); } diff --git a/cqf-fhir-cr/src/test/java/org/opencds/cqf/fhir/cr/measure/common/CqlExpressionValueTest.java b/cqf-fhir-cr/src/test/java/org/opencds/cqf/fhir/cr/measure/common/CqlExpressionValueTest.java index 83dabd3b15..a8b1c120fb 100644 --- a/cqf-fhir-cr/src/test/java/org/opencds/cqf/fhir/cr/measure/common/CqlExpressionValueTest.java +++ b/cqf-fhir-cr/src/test/java/org/opencds/cqf/fhir/cr/measure/common/CqlExpressionValueTest.java @@ -253,6 +253,53 @@ void resolveForPopulation_scalarWrappedInSingletonList() { assertEquals(List.of(encounter), toList(result)); } + // -- isMap / asMap ----------------------------------------------------------- + + @Test + void isMap_trueOnlyForMapValues() { + assertTrue(CqlExpressionValue.ofRaw(Map.of("k", "v"), null).isMap()); + assertTrue(CqlExpressionValue.ofRaw(Map.of(), null).isMap()); + assertFalse(CqlExpressionValue.ofRaw(List.of(), null).isMap()); + assertFalse(CqlExpressionValue.ofRaw("string", null).isMap()); + assertFalse(CqlExpressionValue.ofRaw(null, null).isMap()); + } + + @Test + void asMap_emptyOptionalForNonMapInputs() { + assertEquals( + java.util.Optional.empty(), CqlExpressionValue.ofRaw(null, null).asMap()); + assertEquals( + java.util.Optional.empty(), + CqlExpressionValue.ofRaw("scalar", null).asMap()); + assertEquals( + java.util.Optional.empty(), + CqlExpressionValue.ofRaw(List.of(1, 2), null).asMap()); + } + + @Test + void asMap_returnsTypedMapForMapInputs() { + Patient patient = new Patient(); + patient.setId("p1"); + Map source = new HashMap<>(); + source.put(patient, 42); + + java.util.Optional> opt = + CqlExpressionValue.ofRaw(source, null).asMap(); + + assertTrue(opt.isPresent()); + assertSame(source, opt.get()); + assertEquals(42, opt.get().get(patient)); + } + + @Test + void asMap_emptyMapInputYieldsEmptyMap() { + java.util.Optional> opt = + CqlExpressionValue.ofRaw(Map.of(), null).asMap(); + + assertTrue(opt.isPresent()); + assertTrue(opt.get().isEmpty()); + } + // -- valueAsSet -------------------------------------------------------------- @Test From 01e0ea3b45561287af54f610cfa08141c826014e Mon Sep 17 00:00:00 2001 From: Luke deGruchy Date: Wed, 29 Apr 2026 18:00:17 -0400 Subject: [PATCH 04/13] Tidy CqlExpressionValue follow-ups across the wrapper-migration sites A handful of small post-Phase-3a refinements that all match the wrapper-migration theme and get the remaining instanceof Map / raw Object passthroughs out of the way: - MeasureEvaluator.retainObservationResourcesInPopulation: switch from collect-then-removeAll(List) to Set.removeIf, silencing a static analyzer warning about Set.removeAll(List) being O(n*m) in the worst case and lining this method up with its sibling. - MeasureObservationHandler.removeObservationResourcesInPopulation: drop instanceof Map in favour of CqlExpressionValue.asMap(). - MeasureScoreCalculator.collectQuantities: replace the filter(instanceof Map).map(cast) chain with the wrapper's asMap() flatMap. Method signature unchanged. - StratifierUtils.extractClassesFromSingleOrListResult: change the parameter from Object to CqlExpressionValue and dispatch via isNull/isIterable/asIterable. The two callers in PopulationBasisValidator drop their .raw() passthrough. - StratifierDef: delete the private toSet helper and route through CqlExpressionValue.valueAsSet() directly. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../fhir/cr/measure/common/MeasureEvaluator.java | 14 +++++--------- .../common/MeasureObservationHandler.java | 8 ++++---- .../measure/common/MeasureScoreCalculator.java | 5 +++-- .../measure/common/PopulationBasisValidator.java | 4 ++-- .../fhir/cr/measure/common/StratifierDef.java | 16 +--------------- .../fhir/cr/measure/common/StratifierUtils.java | 13 +++++++------ 6 files changed, 22 insertions(+), 38 deletions(-) diff --git a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureEvaluator.java b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureEvaluator.java index 97ceed9ebe..b6455f6fa6 100644 --- a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureEvaluator.java +++ b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureEvaluator.java @@ -14,7 +14,6 @@ import ca.uhn.fhir.rest.server.exceptions.InternalErrorException; import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; import jakarta.annotation.Nullable; -import java.util.ArrayList; import java.util.Iterator; import java.util.List; import java.util.Map; @@ -481,23 +480,20 @@ protected void retainObservationResourcesInPopulation( if (measurePopulationDef == null) { return; } - Set resourcesForSubject = measureObservationDef.getResourcesForSubject(subjectId); Set measurePopulationResourcesForSubject = measurePopulationDef.getResourcesForSubject(subjectId); - List toRemove = new ArrayList<>(); - for (Object populationResource : resourcesForSubject) { + measureObservationDef.getResourcesForSubject(subjectId).removeIf(populationResource -> { Map obsMap = CqlExpressionValue.ofRaw(populationResource, null).asMap().orElse(null); if (obsMap == null) { - continue; + return false; } for (Object key : obsMap.keySet()) { if (!measurePopulationResourcesForSubject.contains(key)) { - toRemove.add(populationResource); - break; + return true; } } - } - resourcesForSubject.removeAll(toRemove); + return false; + }); } /** diff --git a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureObservationHandler.java b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureObservationHandler.java index 400c9fc303..1fe6f5ce0d 100644 --- a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureObservationHandler.java +++ b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureObservationHandler.java @@ -58,10 +58,10 @@ static void removeObservationResourcesInPopulation( // Iterate over observation resources (which are Maps) and remove matching keys for (Object observationResource : observationResourcesCopy) { - if (observationResource instanceof Map observationMap) { - removeMatchingKeysFromObservationMap( - observationMap, exclusionResources, measureObservationDef, subjectId); - } + CqlExpressionValue.ofRaw(observationResource, null) + .asMap() + .ifPresent(observationMap -> removeMatchingKeysFromObservationMap( + observationMap, exclusionResources, measureObservationDef, subjectId)); } } diff --git a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureScoreCalculator.java b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureScoreCalculator.java index 24093a7555..1f52905e9e 100644 --- a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureScoreCalculator.java +++ b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureScoreCalculator.java @@ -9,6 +9,7 @@ import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.Optional; /** * Pure mathematical functions for measure scoring calculations. @@ -247,8 +248,8 @@ public static BigDecimal aggregateContinuousVariableBigDecimal( */ public static List collectQuantities(Collection resources) { var mapValues = resources.stream() - .filter(x -> x instanceof Map) - .map(x -> (Map) x) + .map(x -> CqlExpressionValue.ofRaw(x, null).asMap()) + .flatMap(Optional::stream) .map(Map::values) .flatMap(Collection::stream) .toList(); diff --git a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/PopulationBasisValidator.java b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/PopulationBasisValidator.java index 8840685354..c9c2117cc5 100644 --- a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/PopulationBasisValidator.java +++ b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/PopulationBasisValidator.java @@ -118,7 +118,7 @@ private void validateGroupPopulationBasisType( return; } - var resultClasses = StratifierUtils.extractClassesFromSingleOrListResult(wrapper.raw()); + var resultClasses = StratifierUtils.extractClassesFromSingleOrListResult(wrapper); var groupPopulationBasisCode = groupDef.getPopulationBasis().code(); var optResourceClass = extractResourceType(groupPopulationBasisCode); @@ -167,7 +167,7 @@ private void validateExpressionResultType( return; } - var resultClasses = StratifierUtils.extractClassesFromSingleOrListResult(wrapper.raw()); + var resultClasses = StratifierUtils.extractClassesFromSingleOrListResult(wrapper); var groupPopulationBasisCode = groupDef.getPopulationBasis().code(); if (stratifierDef.isCriteriaStratifier()) { diff --git a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/StratifierDef.java b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/StratifierDef.java index 26b82347f2..354980b751 100644 --- a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/StratifierDef.java +++ b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/StratifierDef.java @@ -9,7 +9,6 @@ import java.util.Map; import java.util.Set; import java.util.stream.Collectors; -import java.util.stream.StreamSupport; import org.opencds.cqf.fhir.cr.measure.MeasureStratifierType; public class StratifierDef { @@ -92,8 +91,7 @@ public Map getResults() { // Ensure we handle FHIR resource identity properly public Set getAllCriteriaResultValues() { return new HashSetForFhirResourcesAndCqlTypes<>(this.getResults().values().stream() - .map(CqlExpressionValue::raw) - .map(this::toSet) + .map(CqlExpressionValue::valueAsSet) .flatMap(Collection::stream) .collect(Collectors.toUnmodifiableSet())); } @@ -101,16 +99,4 @@ public Set getAllCriteriaResultValues() { public MeasureStratifierType getStratifierType() { return stratifierType; } - - private Set toSet(Object value) { - if (value == null) { - return Set.of(); - } - - if (value instanceof Iterable iterable) { - return StreamSupport.stream(iterable.spliterator(), false).collect(Collectors.toUnmodifiableSet()); - } else { - return Set.of(value); - } - } } diff --git a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/StratifierUtils.java b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/StratifierUtils.java index bb47e9c394..fceb7bae77 100644 --- a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/StratifierUtils.java +++ b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/StratifierUtils.java @@ -15,22 +15,23 @@ private StratifierUtils() { // Static utility class } - public static List> extractClassesFromSingleOrListResult(Object result) { - if (result == null) { + public static List> extractClassesFromSingleOrListResult(CqlExpressionValue value) { + if (value.isNull()) { return Collections.emptyList(); } - if (result instanceof Class clazz) { + Object raw = value.raw(); + if (raw instanceof Class clazz) { return List.of(clazz); } - if (!(result instanceof Iterable iterable)) { - return List.of(result.getClass()); + if (!value.isIterable()) { + return List.of(raw.getClass()); } // Need to this to return List> and get rid of Sonar warnings. final Stream> classStream = - getStream(iterable).filter(Objects::nonNull).map(Object::getClass); + getStream(value.asIterable()).filter(Objects::nonNull).map(Object::getClass); return classStream.toList(); } From 5d5ed27186ec9e1ebe61b0bd88e0c6770c23b051 Mon Sep 17 00:00:00 2001 From: Luke deGruchy Date: Thu, 30 Apr 2026 09:18:02 -0400 Subject: [PATCH 05/13] Route remaining instanceof Iterable/Map sites through CqlExpressionValue Tidies up the satellite call sites that did the same null / Iterable / Map dispatch as the wrapper but in their own raw form. No behaviour change; consistent vocabulary across the package. - EvaluationResultFormatter.formatValue / printValue: wrapper-based isIterable / asMap dispatch. The {empty} sentinel for empty Maps is preserved. - StratumValueWrapper: delete the private isEmptyCollection helper (it was a near-duplicate of wrapper.isEmpty()) and route its three callers through the wrapper. Drops the java.util.Collection and java.util.Map imports along with it. - R4SupportingEvidenceExtension.classifyValue / collectLeavesInto: wrapper-based dispatch for the recursive evidence-flattening paths. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../common/EvaluationResultFormatter.java | 43 ++++++++----------- .../measure/common/StratumValueWrapper.java | 36 +++++----------- .../r4/R4SupportingEvidenceExtension.java | 23 ++++++---- 3 files changed, 43 insertions(+), 59 deletions(-) diff --git a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/EvaluationResultFormatter.java b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/EvaluationResultFormatter.java index c76e8727d7..31a5aaff06 100644 --- a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/EvaluationResultFormatter.java +++ b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/EvaluationResultFormatter.java @@ -136,26 +136,25 @@ public static String formatExpressionValue(Object value) { * @return formatted string representation */ private static String formatValue(Object value) { - if (value == null) { + var wrapper = CqlExpressionValue.ofRaw(value, null); + if (wrapper.isNull()) { return "null"; } // Handle iterables and collections - if (value instanceof Iterable iterable) { - String items = StreamSupport.stream(iterable.spliterator(), false) + if (wrapper.isIterable()) { + String items = StreamSupport.stream(wrapper.asIterable().spliterator(), false) .map(EvaluationResultFormatter::formatSingleValue) .collect(Collectors.joining(", ")); return "[" + items + "]"; } - if (value instanceof Map map) { - return map.entrySet().stream() - .map(entry -> "%s -> %s" - .formatted(formatSingleValue(entry.getKey()), formatSingleValue(entry.getValue()))) - .collect(Collectors.joining(", ")); - } - - return formatSingleValue(value); + return wrapper.asMap() + .map(map -> map.entrySet().stream() + .map(entry -> "%s -> %s" + .formatted(formatSingleValue(entry.getKey()), formatSingleValue(entry.getValue()))) + .collect(Collectors.joining(", "))) + .orElseGet(() -> formatSingleValue(value)); } /** @@ -289,18 +288,14 @@ public static String printValue(Object value) { return resource.getIdElement().getValueAsString(); } - if (value instanceof Map map) { - final String toString = map.entrySet().stream() - .map(entry -> printValue(entry.getKey()) + " -> " + printValue(entry.getValue())) - .collect(Collectors.joining(", ")); - - if (StringUtils.isBlank(toString)) { - return "{empty}"; - } - - return toString; - } - - return value.toString(); + return CqlExpressionValue.ofRaw(value, null) + .asMap() + .map(map -> { + final String toString = map.entrySet().stream() + .map(entry -> printValue(entry.getKey()) + " -> " + printValue(entry.getValue())) + .collect(Collectors.joining(", ")); + return StringUtils.isBlank(toString) ? "{empty}" : toString; + }) + .orElseGet(value::toString); } } diff --git a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/StratumValueWrapper.java b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/StratumValueWrapper.java index 5cd9f5f10c..2f191e6b9c 100644 --- a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/StratumValueWrapper.java +++ b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/StratumValueWrapper.java @@ -2,8 +2,6 @@ import ca.uhn.fhir.context.FhirVersionEnum; import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; -import java.util.Collection; -import java.util.Map; import java.util.StringJoiner; import java.util.stream.Collectors; import java.util.stream.StreamSupport; @@ -77,13 +75,14 @@ public String toString() { private static final String EMPTY_STRATUM_VALUE = "empty"; public String getKey() { + var wrapper = CqlExpressionValue.ofRaw(value, null); // Handle null values - group them into a special "null" stratum - if (value == null) { + if (wrapper.isNull()) { return NULL_STRATUM_VALUE; } // Handle empty collections - group them into a special "empty" stratum - if (isEmptyCollection(value)) { + if (wrapper.isEmpty()) { return EMPTY_STRATUM_VALUE; } @@ -124,10 +123,11 @@ public String getValueAsString() { } public String getDescription() { - if (value == null) { + var wrapper = CqlExpressionValue.ofRaw(value, null); + if (wrapper.isNull()) { return NULL_STRATUM_VALUE; } - if (isEmptyCollection(value)) { + if (wrapper.isEmpty()) { return EMPTY_STRATUM_VALUE; } if (value instanceof IBaseCoding) { @@ -169,29 +169,13 @@ private String joinValues(String... elements) { return String.join("-", elements); } - /** - * Check if the value is an empty collection (List, Set, Map, or other Iterable). - * CQL's empty list "{}" evaluates to an empty collection, which should be treated - * as a distinct stratum value rather than causing an error. - */ - private static boolean isEmptyCollection(Object value) { - if (value instanceof Collection collection) { - return collection.isEmpty(); - } - if (value instanceof Map map) { - return map.isEmpty(); - } - if (value instanceof Iterable iterable) { - return !iterable.iterator().hasNext(); - } - return false; - } - private String getValueAsString(Object valueInner) { - if (valueInner == null) { + var wrapper = CqlExpressionValue.ofRaw(valueInner, null); + if (wrapper.isNull()) { return NULL_STRATUM_VALUE; } - if (isEmptyCollection(valueInner)) { + // CQL's empty list "{}" should be a distinct stratum value, not an error + if (wrapper.isEmpty()) { return EMPTY_STRATUM_VALUE; } if (valueInner instanceof IBaseCoding) { diff --git a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/r4/R4SupportingEvidenceExtension.java b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/r4/R4SupportingEvidenceExtension.java index 4650ea3627..f63386c4c5 100644 --- a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/r4/R4SupportingEvidenceExtension.java +++ b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/r4/R4SupportingEvidenceExtension.java @@ -22,6 +22,7 @@ import org.opencds.cqf.cql.engine.runtime.Tuple; import org.opencds.cqf.fhir.cr.measure.common.CodeDef; import org.opencds.cqf.fhir.cr.measure.common.ConceptDef; +import org.opencds.cqf.fhir.cr.measure.common.CqlExpressionValue; import org.opencds.cqf.fhir.cr.measure.common.SupportingEvidenceDef; import org.opencds.cqf.fhir.cr.measure.r4.utils.R4DateHelper; @@ -194,15 +195,16 @@ private enum ValueKind { * - NORMAL: everything else */ private static ValueKind classifyValue(Object value) { - if (value == null) { + var wrapper = CqlExpressionValue.ofRaw(value, null); + if (wrapper.isNull()) { return ValueKind.NULL_RESULT; } - if (value instanceof Iterable it) { + if (wrapper.isIterable()) { boolean sawAny = false; boolean sawNonNull = false; - for (Object o : it) { + for (Object o : wrapper.asIterable()) { sawAny = true; if (o != null) { sawNonNull = true; @@ -220,8 +222,8 @@ private static ValueKind classifyValue(Object value) { return ValueKind.NORMAL; } - if (value instanceof Map m) { - return m.isEmpty() ? ValueKind.EMPTY_LIST : ValueKind.NORMAL; + if (wrapper.isMap()) { + return wrapper.isEmpty() ? ValueKind.EMPTY_LIST : ValueKind.NORMAL; } return ValueKind.NORMAL; @@ -273,17 +275,20 @@ private static void collectLeavesInto(Object value, List out, int depth) return; } + var wrapper = CqlExpressionValue.ofRaw(value, null); + // Flatten lists & sets - if (value instanceof Iterable it) { - for (Object item : it) { + if (wrapper.isIterable()) { + for (Object item : wrapper.asIterable()) { collectLeavesInto(item, out, depth + 1); } return; } // Optional: flatten map values (if you still want) - if (value instanceof Map map) { - for (Object v : map.values()) { + var asMap = wrapper.asMap(); + if (asMap.isPresent()) { + for (Object v : asMap.get().values()) { collectLeavesInto(v, out, depth + 1); } return; From d68933139d46387aec4cd9f46540cb9f46a5d118 Mon Sep 17 00:00:00 2001 From: Luke deGruchy Date: Thu, 30 Apr 2026 09:49:39 -0400 Subject: [PATCH 06/13] Type collectQuantities on CqlExpressionValue Switches MeasureScoreCalculator.collectQuantities from Collection to Collection. The body simplifies to a single stream chain over CqlExpressionValue::asMap, and the only production caller (MeasureReportDefScorer.calculate ContinuousVariableAggregateQuantity) wraps at the boundary using CqlExpressionValue.ofRaw. Tests gain a small wrap(...) helper. The boundary wrap is a transient shim: when PopulationDef.subject Resources eventually moves to typed storage, the wrap goes away (captured in a MIGRATION-NOTE at that boundary). Co-Authored-By: Claude Opus 4.7 (1M context) --- .../common/MeasureReportDefScorer.java | 14 +++++++++-- .../common/MeasureScoreCalculator.java | 23 ++++++++----------- .../common/MeasureScoreCalculatorTest.java | 18 ++++++++++----- 3 files changed, 34 insertions(+), 21 deletions(-) diff --git a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureReportDefScorer.java b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureReportDefScorer.java index 89bf44d615..de7609cd06 100644 --- a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureReportDefScorer.java +++ b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureReportDefScorer.java @@ -610,8 +610,18 @@ private static QuantityDef calculateContinuousVariableAggregateQuantity( @Nullable private static QuantityDef calculateContinuousVariableAggregateQuantity( ContinuousVariableObservationAggregateMethod aggregateMethod, Collection qualifyingResources) { - // Delegate to MeasureScoreCalculator for collection and aggregation - var observationQuantity = MeasureScoreCalculator.collectQuantities(qualifyingResources); + // MIGRATION-NOTE (typed-subjectResources): qualifyingResources is sourced via the + // popDefToResources Function from PopulationDef.getAllSubjectResources() (List) or + // getResultsForStratum (List). When PopulationDef returns typed wrappers, this + // boundary wrap and the popDefToResources signature both update: Function>. The wrap-and-toList step here goes away. + // + // Test focus: continuous-variable scoring (group-level + stratum-level, both proportion + // and ratio variants) — those are the only consumers of this aggregate path. + var wrapped = qualifyingResources.stream() + .map(o -> CqlExpressionValue.ofRaw(o, null)) + .toList(); + var observationQuantity = MeasureScoreCalculator.collectQuantities(wrapped); return MeasureScoreCalculator.aggregateContinuousVariable(observationQuantity, aggregateMethod); } diff --git a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureScoreCalculator.java b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureScoreCalculator.java index 1f52905e9e..18776627a0 100644 --- a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureScoreCalculator.java +++ b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureScoreCalculator.java @@ -7,7 +7,6 @@ import java.math.RoundingMode; import java.util.Collection; import java.util.List; -import java.util.Map; import java.util.Objects; import java.util.Optional; @@ -228,10 +227,11 @@ public static BigDecimal aggregateContinuousVariableBigDecimal( } /** - * Collect QuantityDef objects from nested Map structures in resources. + * Collect QuantityDef objects from observation-accumulator wrappers. * - *

Helper for continuous variable scoring. Extracts QuantityDef values from - * resources that contain {@code Map} structures with QuantityDef values. + *

Helper for continuous variable scoring. Each {@link CqlExpressionValue} that + * wraps a {@code Map} accumulator contributes its + * QuantityDef-typed values; non-Map and non-QuantityDef entries are filtered out. * *

Usage Pattern: * 

@@ -243,18 +243,15 @@ public static BigDecimal aggregateContinuousVariableBigDecimal(
      *     quantities, ContinuousVariableObservationAggregateMethod.SUM);
      *
* - * @param resources Collection of objects that may contain Maps with QuantityDef values + * @param resources Collection of CqlExpressionValue wrappers that may contain + * observation-accumulator Maps with QuantityDef values * @return List of QuantityDef objects found */ - public static List collectQuantities(Collection resources) { - var mapValues = resources.stream() - .map(x -> CqlExpressionValue.ofRaw(x, null).asMap()) + public static List collectQuantities(Collection resources) { + return resources.stream() + .map(CqlExpressionValue::asMap) .flatMap(Optional::stream) - .map(Map::values) - .flatMap(Collection::stream) - .toList(); - - return mapValues.stream() + .flatMap(map -> map.values().stream()) .filter(QuantityDef.class::isInstance) .map(QuantityDef.class::cast) .toList(); diff --git a/cqf-fhir-cr/src/test/java/org/opencds/cqf/fhir/cr/measure/common/MeasureScoreCalculatorTest.java b/cqf-fhir-cr/src/test/java/org/opencds/cqf/fhir/cr/measure/common/MeasureScoreCalculatorTest.java index ef5134cea8..536f0429de 100644 --- a/cqf-fhir-cr/src/test/java/org/opencds/cqf/fhir/cr/measure/common/MeasureScoreCalculatorTest.java +++ b/cqf-fhir-cr/src/test/java/org/opencds/cqf/fhir/cr/measure/common/MeasureScoreCalculatorTest.java @@ -310,7 +310,7 @@ void testCollectQuantities_ValidMaps() { Map map2 = new HashMap<>(); map2.put("key3", new QuantityDef(30.0)); - Collection resources = List.of(map1, map2); + Collection resources = wrap(map1, map2); List quantities = MeasureScoreCalculator.collectQuantities(resources); @@ -323,7 +323,7 @@ void testCollectQuantities_ValidMaps() { @Test void testCollectQuantities_EmptyCollection() { - Collection resources = new ArrayList<>(); + Collection resources = new ArrayList<>(); List quantities = MeasureScoreCalculator.collectQuantities(resources); @@ -334,7 +334,7 @@ void testCollectQuantities_EmptyCollection() { @Test void testCollectQuantities_NoMaps() { // Collection with non-Map objects - Collection resources = List.of("string", 42, new Object()); + Collection resources = wrap("string", 42, new Object()); List quantities = MeasureScoreCalculator.collectQuantities(resources); @@ -348,7 +348,7 @@ void testCollectQuantities_MapsWithoutQuantityDef() { map1.put("key1", "not a quantity"); map1.put("key2", 123); - Collection resources = List.of(map1); + Collection resources = wrap(map1); List quantities = MeasureScoreCalculator.collectQuantities(resources); @@ -366,7 +366,7 @@ void testCollectQuantities_MixedContent() { Map map2 = new HashMap<>(); map2.put("key3", 123); - Collection resources = List.of(map1, "string", map2, new QuantityDef(20.0)); + Collection resources = wrap(map1, "string", map2, new QuantityDef(20.0)); List quantities = MeasureScoreCalculator.collectQuantities(resources); @@ -375,6 +375,12 @@ void testCollectQuantities_MixedContent() { assertEquals(10.0, quantities.get(0).value(), 0.0001); } + private static List wrap(Object... items) { + return java.util.Arrays.stream(items) + .map(item -> CqlExpressionValue.ofRaw(item, null)) + .toList(); + } + // ========== Edge Cases and Integration Tests ========== @Test @@ -425,7 +431,7 @@ void testFullWorkflow_CollectAggregateContinuousVariable() { Map resource2 = new HashMap<>(); resource2.put("obs3", new QuantityDef(30.0)); - Collection resources = List.of(resource1, resource2); + Collection resources = wrap(resource1, resource2); // Collect quantities List quantities = MeasureScoreCalculator.collectQuantities(resources); From a3ff630db63a52c6579111fa599949076184df40 Mon Sep 17 00:00:00 2001 From: Luke deGruchy Date: Thu, 30 Apr 2026 09:49:53 -0400 Subject: [PATCH 07/13] Mark migration sites for typed PopulationDef.subjectResources Adds MIGRATION-NOTE comments at the major call sites that will need to change when PopulationDef.subjectResources moves from Map> to a typed container (Set or a new PopulationResultSet). Each note captures: what changes at that site, the equality-semantics consideration where relevant, and test-focus areas for that flavour of measure. Sites covered: - PopulationDef: the field declaration (central design note) and addResource (single insertion point) - MeasureEvaluator: three observation methods that walk the Set - MeasureMultiSubjectEvaluator: getPopulationResourceKeySet (three branches by population basis) and stratifierResultAsIntersectionSet (load-bearing for Sets.intersection equality) - MeasureObservationHandler: HashSetForFhirResourcesAndCqlTypes copy - R4MeasureReportBuilder / Dstu3MeasureReportBuilder: the FHIR-build consumers - HashSetForFhirResourcesAndCqlTypes: javadoc note on the two options for wrapper identity Searchable via grep "MIGRATION-NOTE (typed-subjectResources)". Co-Authored-By: Claude Opus 4.7 (1M context) --- .../HashSetForFhirResourcesAndCqlTypes.java | 14 +++++++++ .../cr/measure/common/MeasureEvaluator.java | 26 ++++++++++++++++ .../common/MeasureMultiSubjectEvaluator.java | 23 ++++++++++++++ .../common/MeasureObservationHandler.java | 9 ++++++ .../fhir/cr/measure/common/PopulationDef.java | 30 +++++++++++++++++++ .../dstu3/Dstu3MeasureReportBuilder.java | 8 +++++ .../cr/measure/r4/R4MeasureReportBuilder.java | 8 +++++ 7 files changed, 118 insertions(+) diff --git a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/HashSetForFhirResourcesAndCqlTypes.java b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/HashSetForFhirResourcesAndCqlTypes.java index 75723bc2e6..2fff029805 100644 --- a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/HashSetForFhirResourcesAndCqlTypes.java +++ b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/HashSetForFhirResourcesAndCqlTypes.java @@ -18,6 +18,20 @@ *

* This class exists strictly to compensate for the fact that FHIR resource classes and CQL types * do not implement equals() and hashCode(). + * + *

MIGRATION-NOTE (typed-subjectResources): when PopulationDef.subjectResources moves to + * Set<CqlExpressionValue>, this Set's identity contract becomes the load-bearing decision + * point. Two options: + *

    + *
  1. Add equals/hashCode to CqlExpressionValue that delegate to FhirResourceAndCqlTypeUtils + * .areObjectsEqual on raw(). Pollutes a generic wrapper; affects every other Set/Map of + * wrappers project-wide.
    2. + *
  2. Build a parallel HashSetForCqlExpressionValues that overrides contains/remove etc. to + * compare on element.raw() via the same FhirResourceAndCqlTypeUtils helpers. Keeps the + * wrapper generic; localizes the special-case storage to the population-results pipeline.
    3. + *
+ * Test focus when migrating: every retainAll / removeAll / removeIf path that operates on + * subjectResources, especially observation-exclusion filtering and stratifier intersection. * @param the type of elements in this set, which may or may not be a {@link IBaseResource} * or a {@link CqlType} */ diff --git a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureEvaluator.java b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureEvaluator.java index b6455f6fa6..ee1475f468 100644 --- a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureEvaluator.java +++ b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureEvaluator.java @@ -426,6 +426,15 @@ protected void evaluateContinuousVariable( * Keeps Measure-Observation values found in measurePopulation * are not found in the corresponding measurePopulation set. */ + // MIGRATION-NOTE (typed-subjectResources): both parameters are PopulationDef.subjectResources + // map references handed in from callers. When the field type changes, both signatures here + // (and in removeObservationSubjectResourcesInPopulation / removeObservatorySubjectResource) + // change in lockstep — Map>. The asMap() inside the removeIf + // already operates on a wrapper view, so the body stays nearly identical: drop the per-item + // CqlExpressionValue.ofRaw(...) and call item.asMap() directly. + // + // Test focus: ratio measures (numerator-and-denominator filtering); continuous-variable + // measures with measure-population-exclusion (exercises retain + remove flow). public void retainObservationSubjectResourcesInPopulation( Map> measurePopulation, Map> measureObservation) { @@ -471,6 +480,14 @@ public void retainObservationSubjectResourcesInPopulation( } } + // MIGRATION-NOTE (typed-subjectResources): walks the observation accumulator set directly via + // measureObservationDef.getResourcesForSubject(subjectId). When that returns Set, the lambda becomes (wrapper) -> wrapper.asMap()... — drop the local ofRaw wrap. The + // measurePopulationResourcesForSubject contains() check on map keys still operates on raw FHIR + // resources (the keys, not the values), so its semantics don't change. + // + // Test focus: continuous-variable measures with both numerator and denominator observations, + // where some observation keys aren't in the corresponding population (exclusion flow). protected void retainObservationResourcesInPopulation( String subjectId, // MeasurePopulationType.MEASUREPOPULATION @@ -526,6 +543,15 @@ public void removeObservationSubjectResourcesInPopulation( } } + // MIGRATION-NOTE (typed-subjectResources): the unsafe (Set) cast at the body's + // `obsSet` line goes away when `entryValue` is already Set. The + // `firstEntryValue.isMap()` check stays — it's a structural sanity check, not a type + // discrimination. Note: the InternalErrorException for "expected a Map but wasn't" is left + // in place by convention; converting to a domain exception is the responsibility of the + // separate exception-handling pass. + // + // Test focus: ratio measures with denominator-exclusion populations (the "remove if matches" + // inverse flow); empty observation sets after filtering. private void removeObservatorySubjectResource( Map> measurePopulation, Set entryValue, diff --git a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureMultiSubjectEvaluator.java b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureMultiSubjectEvaluator.java index 48fe0932ef..ca86c7646e 100644 --- a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureMultiSubjectEvaluator.java +++ b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureMultiSubjectEvaluator.java @@ -784,6 +784,18 @@ private static Set calculateCriteriaStratifierIntersection( *

For Map-based results (Map), the input parameters (map keys) * are the intersectable items. */ + // MIGRATION-NOTE (typed-subjectResources): the calling site + // calculateCriteriaStratifierIntersection runs Sets.intersection(populationResultsPerSubject, + // stratifierResultsPerSubject) where populationResultsPerSubject comes from + // populationDef.getResourcesForSubject(...) — a Set today, Set + // post-migration. The intersection MUST use FHIR-identity equality between the two sides. + // Either: (a) wrap the population side too and ensure the wrapper's equals delegates to FHIR + // identity, or (b) rebuild stratifierResultsPerSubject as a Set of raw resources by + // unwrapping. The current method already returns Set, so option (b) is the smaller + // change but loses the "everything is a wrapper" invariant the migration is trying to achieve. + // + // Test focus: criteria-based stratifiers where stratifier and population results overlap + // partially (the typical intersection case); all-Map and all-iterable stratifier results. private static Set stratifierResultAsIntersectionSet(CqlExpressionValue result) { if (result == null) { return Set.of(); @@ -910,6 +922,17 @@ private static List getResourcesForSubjects( *

For MEASUREOBSERVATION populations, the subjectResources contain Set<Map<inputResource, outputValue>> * so we extract the keys (input resources) from those maps. */ + // MIGRATION-NOTE (typed-subjectResources): three branches all consume Set from + // populationDef.getSubjectResources(). When typed: + // - MEASUREOBSERVATION branch: drop the per-item ofRaw(...) wrap; call item.asMap() directly. + // - isResourceType branch: normalizePopulationKey(item.raw()) — or extend the wrapper with an + // overload that takes a wrapper. Using raw() keeps the existing key-derivation semantics. + // - primitive branch: obj.raw().toString() — the toString fallback is unchanged. + // Note that resourceKeys is a plain HashSet of SubjectResourceKey records, so wrapper equality + // doesn't affect this method directly; the wrapper concern is purely on the input side. + // + // Test focus: stratifiers across all three population-basis dimensions (boolean, FHIR resource, + // primitive); cross-subject duplicate-resource handling for non-resource basis. private static Set getPopulationResourceKeySet( FhirContext fhirContext, GroupDef groupDef, PopulationDef populationDef) { final String resourceType = FhirResourceUtils.determineFhirResourceTypeOrNull(fhirContext, groupDef); diff --git a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureObservationHandler.java b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureObservationHandler.java index 1fe6f5ce0d..bbdeb94b46 100644 --- a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureObservationHandler.java +++ b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureObservationHandler.java @@ -29,6 +29,15 @@ private MeasureObservationHandler() { * @param measurePopulationExclusionDef population containing resources to exclude (e.g., cancelled encounters) * @param measureObservationDef population containing observation maps to filter */ + // MIGRATION-NOTE (typed-subjectResources): consumes both populationDef.getResourcesForSubject + // (Set today) and the HashSetForFhirResourcesAndCqlTypes copy. When subjectResources + // is typed, observationResources becomes Set; the copy constructor needs + // a wrapper-aware HashSet variant that preserves FHIR-identity dedup on the underlying raw + // resources. The lambda inside ifPresent already operates on a Map view, so the body holds. + // + // Test focus: continuous-variable measures with measure-population-exclusion that filters + // observations by FHIR resource identity (different Java instances of the same FHIR resource + // must still match — the existing HashSetForFhirResourcesAndCqlTypes guarantee). static void removeObservationResourcesInPopulation( String subjectId, PopulationDef measurePopulationExclusionDef, PopulationDef measureObservationDef) { diff --git a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/PopulationDef.java b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/PopulationDef.java index fba4cae5a9..261091417c 100644 --- a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/PopulationDef.java +++ b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/PopulationDef.java @@ -29,6 +29,27 @@ public class PopulationDef { private Double aggregationResult; protected Set evaluatedResources; + + // MIGRATION-NOTE (typed-subjectResources): the central data model for population results. + // For most population types each item is a FHIR resource / CQL value; for MEASUREOBSERVATION + // populations each item is actually a Map accumulator. The Set + // is a HashSetForFhirResourcesAndCqlTypes — its identity semantics (resource type + logical + // ID for IBaseResource, CQL .equal for CqlType) are load-bearing for retainAll / removeAll + // / removeIf in observation filtering and stratifier intersection. + // + // To migrate to Map> (or a new PopulationResultSet container): + // 1. Decide the wrapper's equals/hashCode story — delegating to FhirResourceAndCqlTypeUtils + // .areObjectsEqual is the obvious path but pollutes a generic wrapper. The alternative is + // a new HashSet variant that compares on raw() instead of on the wrapper. + // 2. Update every accessor below (addResource, getResourcesForSubject, getAllSubjectResources, + // getSubjectResources, retainAllResources, removeAllResources, + // removeExcludedMeasureObservationResource, countObservations, getCount). + // 3. Update every consumer — MeasureEvaluator observation methods, MeasureMultiSubjectEvaluator, + // MeasureReportDefScorer, MeasureObservationHandler, R4/Dstu3/R5 MeasureReportBuilders. + // + // Test focus: ratio + continuous-variable measures (exercise observation accumulators end-to-end); + // measures with stratifiers that intersect populations (exercise FHIR-identity equality on + // retainAll/removeAll); measures with duplicate resources across subjects (countObservations). protected Map> subjectResources = new HashMap<>(); public PopulationDef( @@ -219,6 +240,15 @@ public Set getResourcesForSubject(String subjectId) { } // Add an element to Set under a key (Creates a new set if key is missing) + // + // MIGRATION-NOTE (typed-subjectResources): the only entry point for inserting into + // subjectResources. Once the field is typed, this is where Object → CqlExpressionValue + // wrapping happens (via CqlExpressionValue.ofRaw(value, null), preserving the lack of + // evaluatedResources at this granularity). Callers in MeasureEvaluator.evaluatePopulation + // Membership and FunctionEvaluationHandler.aggregateFunctionResults still pass raw Object; + // the wrap should land here, not at every caller. + // + // Test focus: any measure that lands resources in a population — every flavour exercises this. public void addResource(String key, Object value) { subjectResources .computeIfAbsent(key, k -> new HashSetForFhirResourcesAndCqlTypes<>()) diff --git a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/dstu3/Dstu3MeasureReportBuilder.java b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/dstu3/Dstu3MeasureReportBuilder.java index 481ee887e5..b4c7dd42e7 100644 --- a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/dstu3/Dstu3MeasureReportBuilder.java +++ b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/dstu3/Dstu3MeasureReportBuilder.java @@ -280,6 +280,14 @@ protected void buildPopulation( reportPopulation.setCode(measurePopulation.getCode()); reportPopulation.setId(measurePopulation.getId()); + // MIGRATION-NOTE (typed-subjectResources): the .size() call only needs the count, so the + // typed migration is transparent here. The MEASUREOBSERVATION branch below at line ~320 + // hands getAllSubjectResources() into buildMeasureObservations(...), which expects raw + // Object items — that signature needs to change to consume CqlExpressionValue or unwrap + // via .stream().map(CqlExpressionValue::raw). + // + // Test focus: DSTU3 measure reports for continuous-variable measures (the only ones that + // hit buildMeasureObservations); DSTU3 patient-list reports. if (!measureDef.groups().isEmpty() && !measureDef.groups().get(0).isBooleanBasis()) { reportPopulation.setCount(populationDef.getAllSubjectResources().size()); } else { diff --git a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/r4/R4MeasureReportBuilder.java b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/r4/R4MeasureReportBuilder.java index dcfb1da01d..86358003bc 100644 --- a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/r4/R4MeasureReportBuilder.java +++ b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/r4/R4MeasureReportBuilder.java @@ -263,6 +263,14 @@ private void buildPopulation( // This is a temporary list carried forward to stratifiers // subjectResult set defined by basis of Measure + // MIGRATION-NOTE (typed-subjectResources): when getAllSubjectResources() returns + // List, the filter chain becomes + // .map(CqlExpressionValue::raw).filter(Resource.class::isInstance) — preserve the + // existing "non-Resource entries are silently dropped" behaviour. The stratifier path + // downstream consumes populationSet as Set, so wrapper boundary stops here. + // + // Test focus: subject-list reports (where populationSet drives subject references); ratio + // measures (which carry FHIR Resource lookups across both numerator and denominator). Set populationSet; if (groupDef.isBooleanBasis()) { populationSet = populationDef.getSubjects().stream() From cc3f77b7b0b5e2d6bbac5f4558d0ad019a58e612 Mon Sep 17 00:00:00 2001 From: Luke deGruchy Date: Thu, 30 Apr 2026 11:02:06 -0400 Subject: [PATCH 08/13] Type PopulationDef.subjectResources on CqlExpressionValue Promotes PopulationDef.subjectResources from Map> to Map>, eliminating the last raw-Object storage in the population-results pipeline. Equality strategy: a new sister type HashSetForCqlExpressionValues mirrors HashSetForFhirResourcesAndCqlTypes but unwraps each element via .raw() before applying FHIR-resource / CQL-type identity rules. The wrapper itself stays generic (no equals/hashCode override), and per-subject sets remain small enough that linear-time identity checks are acceptable. Migrated: - PopulationDef accessors (addResource, getResourcesForSubject, getAllSubjectResources, getSubjectResources, countObservations, removeExcludedMeasureObservationResource) all speak in CqlExpressionValue. addResource is the single wrap point. - MeasureEvaluator's three observation methods (retainObservation SubjectResourcesInPopulation, retainObservationResourcesInPopulation, removeObservatorySubjectResource) take typed parameters and drop per-item ofRaw() wraps and the legacy Set casts. - MeasureMultiSubjectEvaluator: getPopulationResourceKeySet, the resourceIds builder, and calculateCriteriaStratifierIntersection (now manually intersects via raw() rather than Sets.intersection across mismatched element types). - MeasureReportDefScorer: calculateContinuousVariableAggregateQuantity drops the boundary-wrap shim; getResultsForStratum and getResultsForStratumByResourceIds return wrapped collections. - MeasureObservationHandler: copy uses HashSetForCqlExpressionValues; exclusion lookup unwraps to raw via wrapper.raw() at one site. - R4MeasureReportBuilder + Dstu3MeasureReportBuilder: builder consumers unwrap via .raw() at the FHIR-resource-id boundary. - EvaluationResultFormatter.printSubjectResources: unwraps wrappers before formatting. Tests: PopulationDefTest's helper unwraps wrappers; the assertions that used to call getAllSubjectResources().contains(rawValue) route through the FHIR-identity helper. MeasureObservationHandlerTest switches from direct subjectResources.put() to the public addResource() API everywhere. All MIGRATION-NOTE breadcrumbs left for this phase are now removed. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../common/EvaluationResultFormatter.java | 9 +- .../common/HashSetForCqlExpressionValues.java | 165 ++++++++++++++++++ .../HashSetForFhirResourcesAndCqlTypes.java | 15 +- .../cr/measure/common/MeasureEvaluator.java | 80 +++------ .../common/MeasureMultiSubjectEvaluator.java | 55 +++--- .../common/MeasureObservationHandler.java | 34 ++-- .../common/MeasureReportDefScorer.java | 33 ++-- .../fhir/cr/measure/common/PopulationDef.java | 77 +++----- .../dstu3/Dstu3MeasureReportBuilder.java | 11 +- .../cr/measure/r4/R4MeasureReportBuilder.java | 18 +- .../common/MeasureObservationHandlerTest.java | 127 +++++--------- .../cr/measure/common/PopulationDefTest.java | 5 +- 12 files changed, 325 insertions(+), 304 deletions(-) create mode 100644 cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/HashSetForCqlExpressionValues.java diff --git a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/EvaluationResultFormatter.java b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/EvaluationResultFormatter.java index 31a5aaff06..6a2c897ce8 100644 --- a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/EvaluationResultFormatter.java +++ b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/EvaluationResultFormatter.java @@ -254,14 +254,17 @@ public static Object printSubjectResources(PopulationDef populationDef, String s return "{empty}"; } - final Set resources = populationDef.getSubjectResources().get(subjectId); + final Set resources = + populationDef.getSubjectResources().get(subjectId); if (CollectionUtils.isEmpty(resources)) { return subjectId + ": {empty}"; } - final String toString = - resources.stream().map(EvaluationResultFormatter::printValue).collect(Collectors.joining(", ")); + final String toString = resources.stream() + .map(CqlExpressionValue::raw) + .map(EvaluationResultFormatter::printValue) + .collect(Collectors.joining(", ")); if (StringUtils.isBlank(toString)) { return subjectId + ": {empty}"; diff --git a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/HashSetForCqlExpressionValues.java b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/HashSetForCqlExpressionValues.java new file mode 100644 index 0000000000..da01c08e08 --- /dev/null +++ b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/HashSetForCqlExpressionValues.java @@ -0,0 +1,165 @@ +package org.opencds.cqf.fhir.cr.measure.common; + +import jakarta.annotation.Nonnull; +import java.util.Collection; +import java.util.HashSet; +import java.util.Iterator; +import java.util.Objects; +import java.util.stream.Collectors; +import org.hl7.fhir.instance.model.api.IBaseResource; + +/** + * A {@link HashSet} of {@link CqlExpressionValue} that compares elements by the FHIR-resource and + * CQL-type identity rules of their underlying value (via {@link FhirResourceAndCqlTypeUtils}). + *

+ * Sister type to {@link HashSetForFhirResourcesAndCqlTypes} for use when the population pipeline + * stores wrappers rather than raw {@link Object}s. Two wrappers around FHIR resources with the + * same resource type and logical ID are considered equal, even if the wrappers (or the underlying + * resource instances) are different object instances. Same applies to CQL types via + * {@link org.opencds.cqf.cql.engine.runtime.CqlType#equal}. + *

+ * Bucket placement still uses the wrapper's default {@code Object.hashCode()} (the wrapper + * doesn't implement {@code equals} / {@code hashCode}), so {@code add} / {@code remove} / + * {@code contains} / {@code retainAll} fall through to linear-time identity checks via + * {@link FhirResourceAndCqlTypeUtils#areObjectsEqual}. This is acceptable — per-subject + * population sets are small. + */ +@SuppressWarnings("squid:S3776") +public class HashSetForCqlExpressionValues extends HashSet { + + public HashSetForCqlExpressionValues() { + super(); + } + + public HashSetForCqlExpressionValues(Collection collection) { + super(); + for (CqlExpressionValue value : collection) { + this.add(value); + } + } + + public HashSetForCqlExpressionValues(Iterable iterable) { + super(); + for (CqlExpressionValue value : iterable) { + this.add(value); + } + } + + /** + * Linear-search check that any wrapper in this set has an underlying value equal — by FHIR + * resource / CQL type identity — to {@code other}. Accepts either a {@link CqlExpressionValue} + * (the typical case) or a raw object (so callers can ask "does this set contain a wrapper + * around resource X?" directly). + */ + @Override + public boolean contains(Object other) { + return containsByIdentity(this, unwrap(other)); + } + + /** + * Adds {@code newElement} only if no existing wrapper in this set has an underlying value + * equal to {@code newElement.raw()} by FHIR identity. + */ + @Override + public boolean add(CqlExpressionValue newElement) { + if (newElement == null) { + return super.add(null); + } + Object newRaw = newElement.raw(); + if (newRaw == null + || (FhirResourceAndCqlTypeUtils.castToResourceIfApplicable(newRaw) == null + && FhirResourceAndCqlTypeUtils.castToCqlTypeIfApplicable(newRaw) == null)) { + return super.add(newElement); + } + for (CqlExpressionValue existing : this) { + if (existing != null && FhirResourceAndCqlTypeUtils.areObjectsEqual(existing.raw(), newRaw)) { + return false; + } + } + return super.add(newElement); + } + + /** + * Removes the wrapper whose underlying value matches {@code removalCandidate} by FHIR + * identity. {@code removalCandidate} may be a {@link CqlExpressionValue} or a raw resource. + */ + @Override + public boolean remove(Object removalCandidate) { + Object targetRaw = unwrap(removalCandidate); + if (targetRaw == null) { + return super.remove(removalCandidate); + } + if (FhirResourceAndCqlTypeUtils.castToResourceIfApplicable(targetRaw) == null + && FhirResourceAndCqlTypeUtils.castToCqlTypeIfApplicable(targetRaw) == null) { + return super.remove(removalCandidate); + } + for (CqlExpressionValue existing : this) { + if (existing != null && FhirResourceAndCqlTypeUtils.areObjectsEqual(existing.raw(), targetRaw)) { + return super.remove(existing); + } + } + return false; + } + + @Override + public boolean retainAll(@Nonnull Collection otherCollection) { + Objects.requireNonNull(otherCollection); + + if (otherCollection instanceof HashSetForCqlExpressionValues) { + return super.retainAll(otherCollection); + } + + boolean modified = false; + Iterator it = iterator(); + while (it.hasNext()) { + CqlExpressionValue next = it.next(); + if (!otherContains(otherCollection, next)) { + it.remove(); + modified = true; + } + } + return modified; + } + + private static boolean otherContains(Collection collection, CqlExpressionValue value) { + Object raw = value == null ? null : value.raw(); + for (Object other : collection) { + Object otherRaw = unwrap(other); + if (FhirResourceAndCqlTypeUtils.areObjectsEqual(raw, otherRaw)) { + return true; + } + } + return false; + } + + private static boolean containsByIdentity(Iterable elements, Object targetRaw) { + for (CqlExpressionValue existing : elements) { + Object existingRaw = existing == null ? null : existing.raw(); + if (FhirResourceAndCqlTypeUtils.areObjectsEqual(existingRaw, targetRaw)) { + return true; + } + } + return false; + } + + private static Object unwrap(Object o) { + return o instanceof CqlExpressionValue v ? v.raw() : o; + } + + @Override + public String toString() { + if (isEmpty()) { + return "[]"; + } + Object firstRaw = iterator().next() == null ? null : iterator().next().raw(); + if (firstRaw instanceof IBaseResource) { + return stream() + .map(CqlExpressionValue::raw) + .filter(IBaseResource.class::isInstance) + .map(IBaseResource.class::cast) + .map(r -> r.getIdElement().getValueAsString()) + .collect(Collectors.joining(",", "[", "]")); + } + return super.toString(); + } +} diff --git a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/HashSetForFhirResourcesAndCqlTypes.java b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/HashSetForFhirResourcesAndCqlTypes.java index 2fff029805..7d436c3093 100644 --- a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/HashSetForFhirResourcesAndCqlTypes.java +++ b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/HashSetForFhirResourcesAndCqlTypes.java @@ -19,19 +19,8 @@ * This class exists strictly to compensate for the fact that FHIR resource classes and CQL types * do not implement equals() and hashCode(). * - *

MIGRATION-NOTE (typed-subjectResources): when PopulationDef.subjectResources moves to - * Set<CqlExpressionValue>, this Set's identity contract becomes the load-bearing decision - * point. Two options: - *

    - *
  1. Add equals/hashCode to CqlExpressionValue that delegate to FhirResourceAndCqlTypeUtils - * .areObjectsEqual on raw(). Pollutes a generic wrapper; affects every other Set/Map of - * wrappers project-wide.
    2. - *
  2. Build a parallel HashSetForCqlExpressionValues that overrides contains/remove etc. to - * compare on element.raw() via the same FhirResourceAndCqlTypeUtils helpers. Keeps the - * wrapper generic; localizes the special-case storage to the population-results pipeline.
    3. - *
- * Test focus when migrating: every retainAll / removeAll / removeIf path that operates on - * subjectResources, especially observation-exclusion filtering and stratifier intersection. + *

For a wrapper-aware sister type used by {@code PopulationDef.subjectResources}, see + * {@link HashSetForCqlExpressionValues}. * @param the type of elements in this set, which may or may not be a {@link IBaseResource} * or a {@link CqlType} */ diff --git a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureEvaluator.java b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureEvaluator.java index ee1475f468..4cd7d0e5de 100644 --- a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureEvaluator.java +++ b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureEvaluator.java @@ -426,31 +426,23 @@ protected void evaluateContinuousVariable( * Keeps Measure-Observation values found in measurePopulation * are not found in the corresponding measurePopulation set. */ - // MIGRATION-NOTE (typed-subjectResources): both parameters are PopulationDef.subjectResources - // map references handed in from callers. When the field type changes, both signatures here - // (and in removeObservationSubjectResourcesInPopulation / removeObservatorySubjectResource) - // change in lockstep — Map>. The asMap() inside the removeIf - // already operates on a wrapper view, so the body stays nearly identical: drop the per-item - // CqlExpressionValue.ofRaw(...) and call item.asMap() directly. - // - // Test focus: ratio measures (numerator-and-denominator filtering); continuous-variable - // measures with measure-population-exclusion (exercises retain + remove flow). public void retainObservationSubjectResourcesInPopulation( - Map> measurePopulation, Map> measureObservation) { + Map> measurePopulation, + Map> measureObservation) { if (measurePopulation == null || measureObservation == null) { return; } - for (Iterator>> it = + for (Iterator>> it = measureObservation.entrySet().iterator(); it.hasNext(); ) { - Map.Entry> entry = it.next(); + Map.Entry> entry = it.next(); String subjectId = entry.getKey(); - Set obsSet = entry.getValue(); + Set obsSet = entry.getValue(); // get valid population values for this subject - Set validPopulation = measurePopulation.get(subjectId); + Set validPopulation = measurePopulation.get(subjectId); if (validPopulation == null || validPopulation.isEmpty()) { // no population for this subject -> drop the whole subject @@ -460,8 +452,7 @@ public void retainObservationSubjectResourcesInPopulation( // remove observation accumulators whose keys aren't all in the valid population obsSet.removeIf(item -> { - Map obsMap = - CqlExpressionValue.ofRaw(item, null).asMap().orElse(null); + Map obsMap = item.asMap().orElse(null); if (obsMap == null) { return false; // not an observation accumulator, leave alone } @@ -480,14 +471,6 @@ public void retainObservationSubjectResourcesInPopulation( } } - // MIGRATION-NOTE (typed-subjectResources): walks the observation accumulator set directly via - // measureObservationDef.getResourcesForSubject(subjectId). When that returns Set, the lambda becomes (wrapper) -> wrapper.asMap()... — drop the local ofRaw wrap. The - // measurePopulationResourcesForSubject contains() check on map keys still operates on raw FHIR - // resources (the keys, not the values), so its semantics don't change. - // - // Test focus: continuous-variable measures with both numerator and denominator observations, - // where some observation keys aren't in the corresponding population (exclusion flow). protected void retainObservationResourcesInPopulation( String subjectId, // MeasurePopulationType.MEASUREPOPULATION @@ -497,10 +480,10 @@ protected void retainObservationResourcesInPopulation( if (measurePopulationDef == null) { return; } - Set measurePopulationResourcesForSubject = measurePopulationDef.getResourcesForSubject(subjectId); + Set measurePopulationResourcesForSubject = + measurePopulationDef.getResourcesForSubject(subjectId); measureObservationDef.getResourcesForSubject(subjectId).removeIf(populationResource -> { - Map obsMap = - CqlExpressionValue.ofRaw(populationResource, null).asMap().orElse(null); + Map obsMap = populationResource.asMap().orElse(null); if (obsMap == null) { return false; } @@ -518,22 +501,22 @@ protected void retainObservationResourcesInPopulation( * @param measurePopulation population results that you would like to exclude from measureObservation * @param measureObservation population results that will have items excluded from it, if found in measurePopulation */ - @SuppressWarnings("unchecked") public void removeObservationSubjectResourcesInPopulation( - Map> measurePopulation, Map> measureObservation) { + Map> measurePopulation, + Map> measureObservation) { if (measurePopulation == null || measureObservation == null) { return; } - for (Iterator>> it = + for (Iterator>> it = measureObservation.entrySet().iterator(); it.hasNext(); ) { - Map.Entry> entry = it.next(); + Map.Entry> entry = it.next(); String subjectId = entry.getKey(); - final Set entryValue = entry.getValue(); + final Set entryValue = entry.getValue(); if (CollectionUtils.isEmpty(entryValue)) { continue; @@ -543,35 +526,23 @@ public void removeObservationSubjectResourcesInPopulation( } } - // MIGRATION-NOTE (typed-subjectResources): the unsafe (Set) cast at the body's - // `obsSet` line goes away when `entryValue` is already Set. The - // `firstEntryValue.isMap()` check stays — it's a structural sanity check, not a type - // discrimination. Note: the InternalErrorException for "expected a Map but wasn't" is left - // in place by convention; converting to a domain exception is the responsibility of the - // separate exception-handling pass. - // - // Test focus: ratio measures with denominator-exclusion populations (the "remove if matches" - // inverse flow); empty observation sets after filtering. private void removeObservatorySubjectResource( - Map> measurePopulation, - Set entryValue, + Map> measurePopulation, + Set entryValue, String subjectId, - Iterator>> iterator) { + Iterator>> iterator) { if (entryValue.isEmpty()) { // Nothing to do return; } - final Object firstEntryValue = entryValue.iterator().next(); + final CqlExpressionValue firstEntryValue = entryValue.iterator().next(); - if (!CqlExpressionValue.ofRaw(firstEntryValue, null).isMap()) { - throw new InternalErrorException("Expected a Map but was not: %s".formatted(firstEntryValue)); + if (!firstEntryValue.isMap()) { + throw new InternalErrorException("Expected a Map but was not: %s".formatted(firstEntryValue.raw())); } - @SuppressWarnings("unchecked") - Set obsSet = (Set) entryValue; - // population values for this subject - Set populationValues = measurePopulation.get(subjectId); + Set populationValues = measurePopulation.get(subjectId); // If there is no population for this subject, there is nothing "to remove because iterator matches", // so leave the observation set as-is. @@ -580,9 +551,8 @@ private void removeObservatorySubjectResource( } // Remove observations that *do* match population values - obsSet.removeIf(item -> { - Map obsMap = - CqlExpressionValue.ofRaw(item, null).asMap().orElse(null); + entryValue.removeIf(item -> { + Map obsMap = item.asMap().orElse(null); if (obsMap == null) { return false; } @@ -596,7 +566,7 @@ private void removeObservatorySubjectResource( }); // If no observations remain for this subject, remove the subject entry entirely - if (obsSet.isEmpty()) { + if (entryValue.isEmpty()) { iterator.remove(); } } diff --git a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureMultiSubjectEvaluator.java b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureMultiSubjectEvaluator.java index ca86c7646e..33a93878e1 100644 --- a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureMultiSubjectEvaluator.java +++ b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureMultiSubjectEvaluator.java @@ -762,16 +762,23 @@ private static Set calculateCriteriaStratifierIntersection( final Map stratifierResultsBySubject = stratifierDef.getResults(); final List allPopulationStratumIntersectingResources = new ArrayList<>(); - // For each subject, we intersect between the population and stratifier results + // For each subject, we intersect between the population (Set) and + // stratifier results (Set of raw resources). Iterate the population side and + // delegate to the stratifier set's contains(): for Map-based stratifier results that's + // plain Object.equals on map keys; for non-Map results that's FHIR-identity equality + // via HashSetForFhirResourcesAndCqlTypes. for (Entry stratifierEntryBySubject : stratifierResultsBySubject.entrySet()) { final Set stratifierResultsPerSubject = stratifierResultAsIntersectionSet(stratifierEntryBySubject.getValue()); - - final Set populationResultsPerSubject = + final Set populationResultsPerSubject = populationDef.getResourcesForSubject(stratifierEntryBySubject.getKey()); - allPopulationStratumIntersectingResources.addAll( - Sets.intersection(populationResultsPerSubject, stratifierResultsPerSubject)); + for (CqlExpressionValue wrapper : populationResultsPerSubject) { + Object raw = wrapper == null ? null : wrapper.raw(); + if (stratifierResultsPerSubject.contains(raw)) { + allPopulationStratumIntersectingResources.add(raw); + } + } } // We add up all the results of the intersections here: @@ -784,18 +791,6 @@ private static Set calculateCriteriaStratifierIntersection( *

For Map-based results (Map), the input parameters (map keys) * are the intersectable items. */ - // MIGRATION-NOTE (typed-subjectResources): the calling site - // calculateCriteriaStratifierIntersection runs Sets.intersection(populationResultsPerSubject, - // stratifierResultsPerSubject) where populationResultsPerSubject comes from - // populationDef.getResourcesForSubject(...) — a Set today, Set - // post-migration. The intersection MUST use FHIR-identity equality between the two sides. - // Either: (a) wrap the population side too and ensure the wrapper's equals delegates to FHIR - // identity, or (b) rebuild stratifierResultsPerSubject as a Set of raw resources by - // unwrapping. The current method already returns Set, so option (b) is the smaller - // change but loses the "everything is a wrapper" invariant the migration is trying to achieve. - // - // Test focus: criteria-based stratifiers where stratifier and population results overlap - // partially (the typical intersection case); all-Map and all-iterable stratifier results. private static Set stratifierResultAsIntersectionSet(CqlExpressionValue result) { if (result == null) { return Set.of(); @@ -892,15 +887,19 @@ private static List getResourcesForSubjects( continue; } - Set resources = entry.getValue(); + Set resources = entry.getValue(); if (resources != null) { if (isResourceType) { resources.stream() + .map(CqlExpressionValue::raw) .map(MeasureMultiSubjectEvaluator::normalizePopulationKey) .filter(java.util.Objects::nonNull) .forEach(resourceIds::add); } else { - resources.stream().map(Object::toString).forEach(resourceIds::add); + resources.stream() + .map(CqlExpressionValue::raw) + .map(Object::toString) + .forEach(resourceIds::add); } } } @@ -922,17 +921,6 @@ private static List getResourcesForSubjects( *

For MEASUREOBSERVATION populations, the subjectResources contain Set<Map<inputResource, outputValue>> * so we extract the keys (input resources) from those maps. */ - // MIGRATION-NOTE (typed-subjectResources): three branches all consume Set from - // populationDef.getSubjectResources(). When typed: - // - MEASUREOBSERVATION branch: drop the per-item ofRaw(...) wrap; call item.asMap() directly. - // - isResourceType branch: normalizePopulationKey(item.raw()) — or extend the wrapper with an - // overload that takes a wrapper. Using raw() keeps the existing key-derivation semantics. - // - primitive branch: obj.raw().toString() — the toString fallback is unchanged. - // Note that resourceKeys is a plain HashSet of SubjectResourceKey records, so wrapper equality - // doesn't affect this method directly; the wrapper concern is purely on the input side. - // - // Test focus: stratifiers across all three population-basis dimensions (boolean, FHIR resource, - // primitive); cross-subject duplicate-resource handling for non-resource basis. private static Set getPopulationResourceKeySet( FhirContext fhirContext, GroupDef groupDef, PopulationDef populationDef) { final String resourceType = FhirResourceUtils.determineFhirResourceTypeOrNull(fhirContext, groupDef); @@ -944,15 +932,14 @@ private static Set getPopulationResourceKeySet( String subjectId = entry.getKey(); // Qualify the subject ID to match the format used in StratifierRowKey (only needed for primitive types) String qualifiedSubject = FhirResourceUtils.addPatientQualifier(subjectId); - Set resources = entry.getValue(); + Set resources = entry.getValue(); if (resources != null) { // For MEASUREOBSERVATION, resources are Map // We need to extract the keys (input resources) if (populationDef.type() == MeasurePopulationType.MEASUREOBSERVATION) { // MEASUREOBSERVATION always deals with FHIR resources, so no subject qualification needed resources.stream() - .map(item -> - CqlExpressionValue.ofRaw(item, null).asMap()) + .map(CqlExpressionValue::asMap) .flatMap(java.util.Optional::stream) .flatMap(m -> m.keySet().stream()) .map(MeasureMultiSubjectEvaluator::normalizePopulationKey) @@ -962,6 +949,7 @@ private static Set getPopulationResourceKeySet( } else if (isResourceType) { // FHIR resource types have globally unique IDs - no subject qualification needed resources.stream() + .map(CqlExpressionValue::raw) .map(MeasureMultiSubjectEvaluator::normalizePopulationKey) .filter(java.util.Objects::nonNull) .map(SubjectResourceKey::resourceOnly) @@ -969,6 +957,7 @@ private static Set getPopulationResourceKeySet( } else { // Primitive types (like Date) - include subject context to preserve duplicates resources.stream() + .map(CqlExpressionValue::raw) .map(obj -> SubjectResourceKey.of(qualifiedSubject, obj.toString())) .forEach(resourceKeys::add); } diff --git a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureObservationHandler.java b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureObservationHandler.java index bbdeb94b46..d53f5a5db3 100644 --- a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureObservationHandler.java +++ b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureObservationHandler.java @@ -29,15 +29,6 @@ private MeasureObservationHandler() { * @param measurePopulationExclusionDef population containing resources to exclude (e.g., cancelled encounters) * @param measureObservationDef population containing observation maps to filter */ - // MIGRATION-NOTE (typed-subjectResources): consumes both populationDef.getResourcesForSubject - // (Set today) and the HashSetForFhirResourcesAndCqlTypes copy. When subjectResources - // is typed, observationResources becomes Set; the copy constructor needs - // a wrapper-aware HashSet variant that preserves FHIR-identity dedup on the underlying raw - // resources. The lambda inside ifPresent already operates on a Map view, so the body holds. - // - // Test focus: continuous-variable measures with measure-population-exclusion that filters - // observations by FHIR resource identity (different Java instances of the same FHIR resource - // must still match — the existing HashSetForFhirResourcesAndCqlTypes guarantee). static void removeObservationResourcesInPopulation( String subjectId, PopulationDef measurePopulationExclusionDef, PopulationDef measureObservationDef) { @@ -45,12 +36,13 @@ static void removeObservationResourcesInPopulation( return; } - final Set exclusionResources = measurePopulationExclusionDef.getResourcesForSubject(subjectId); + final Set exclusionResources = + measurePopulationExclusionDef.getResourcesForSubject(subjectId); if (CollectionUtils.isEmpty(exclusionResources)) { return; } - final Set observationResources = measureObservationDef.getResourcesForSubject(subjectId); + final Set observationResources = measureObservationDef.getResourcesForSubject(subjectId); if (CollectionUtils.isEmpty(observationResources)) { return; } @@ -63,11 +55,12 @@ static void removeObservationResourcesInPopulation( // Make a copy to avoid ConcurrentModificationException when removeExcludedMeasureObservationResource // removes empty maps from the original set - final Set observationResourcesCopy = new HashSetForFhirResourcesAndCqlTypes<>(observationResources); + final Set observationResourcesCopy = + new HashSetForCqlExpressionValues(observationResources); // Iterate over observation resources (which are Maps) and remove matching keys - for (Object observationResource : observationResourcesCopy) { - CqlExpressionValue.ofRaw(observationResource, null) + for (CqlExpressionValue observationResource : observationResourcesCopy) { + observationResource .asMap() .ifPresent(observationMap -> removeMatchingKeysFromObservationMap( observationMap, exclusionResources, measureObservationDef, subjectId)); @@ -82,30 +75,31 @@ static void removeObservationResourcesInPopulation( * map keys may be separate Java object instances representing the same FHIR resource. * * @param observationMap observation map containing Resource -> QuantityDef entries - * @param exclusionResources set of resources to exclude + * @param exclusionResources set of resources to exclude (wrapped) * @param measureObservationDef the observation population definition * @param subjectId the subject ID */ private static void removeMatchingKeysFromObservationMap( Map observationMap, - Set exclusionResources, + Set exclusionResources, PopulationDef measureObservationDef, String subjectId) { // Find observation map keys that match any exclusion resource - for (Object exclusionResource : exclusionResources) { + for (CqlExpressionValue exclusionResource : exclusionResources) { + Object exclusionRaw = exclusionResource.raw(); // Check if this exclusion resource matches any key in the observation map // Must use custom equality that compares FHIR resource identity, not object instance boolean matchFound = observationMap.keySet().stream() - .anyMatch(mapKey -> FhirResourceAndCqlTypeUtils.areObjectsEqual(mapKey, exclusionResource)); + .anyMatch(mapKey -> FhirResourceAndCqlTypeUtils.areObjectsEqual(mapKey, exclusionRaw)); if (matchFound) { logger.debug( "Removing observation for excluded resource: {}", - EvaluationResultFormatter.formatResource(exclusionResource)); + EvaluationResultFormatter.formatResource(exclusionRaw)); // Remove the entry from the inner map using the PopulationDef's removal method // This ensures proper handling of the Map>> structure - measureObservationDef.removeExcludedMeasureObservationResource(subjectId, exclusionResource); + measureObservationDef.removeExcludedMeasureObservationResource(subjectId, exclusionRaw); } } } diff --git a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureReportDefScorer.java b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureReportDefScorer.java index de7609cd06..cd6b6d9326 100644 --- a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureReportDefScorer.java +++ b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureReportDefScorer.java @@ -489,7 +489,7 @@ private StratumPopulationDef getStratumPopDefFromPopDef(StratumDef stratumDef, P * @param stratumPopulationDef the stratum population to filter by * @return collection of resources belonging to this stratum */ - private static Collection getResultsForStratum( + private static Collection getResultsForStratum( PopulationDef populationDef, StratumPopulationDef stratumPopulationDef) { if (stratumPopulationDef == null || populationDef == null || populationDef.getSubjectResources() == null) { @@ -531,7 +531,7 @@ private static Collection getResultsForStratum( * @param stratumPopulationDef the stratum population containing resource IDs * @return collection of resources/observations matching the stratum's resource IDs */ - private static Collection getResultsForStratumByResourceIds( + private static Collection getResultsForStratumByResourceIds( PopulationDef populationDef, StratumPopulationDef stratumPopulationDef) { Set stratumResourceIds = stratumPopulationDef.resourceIdsAsSet(); @@ -542,8 +542,8 @@ private static Collection getResultsForStratumByResourceIds( if (populationDef.type() == MeasurePopulationType.MEASUREOBSERVATION) { return populationDef.getSubjectResources().values().stream() .flatMap(Collection::stream) - .map(item -> CqlExpressionValue.ofRaw(item, null).asMap()) - .flatMap(java.util.Optional::stream) + .map(CqlExpressionValue::asMap) + .flatMap(Optional::stream) .map(map -> { // Filter the map to only include entries matching stratum resource IDs Map filteredMap = new java.util.HashMap<>(); @@ -562,14 +562,16 @@ private static Collection getResultsForStratumByResourceIds( return filteredMap; }) .filter(map -> !map.isEmpty()) // Only include non-empty filtered maps + .map(filteredMap -> CqlExpressionValue.ofRaw(filteredMap, null)) .collect(Collectors.toList()); } // For non-MEASUREOBSERVATION populations, filter resources directly return populationDef.getSubjectResources().values().stream() .flatMap(Collection::stream) - .filter(resource -> { - if (resource instanceof IBaseResource baseResource) { + .filter(wrapper -> { + Object raw = wrapper.raw(); + if (raw instanceof IBaseResource baseResource) { String resourceId = baseResource.getIdElement().toVersionless().getValue(); return stratumResourceIds.contains(resourceId); @@ -589,7 +591,8 @@ private static Collection getResultsForStratumByResourceIds( */ @Nullable private static QuantityDef calculateContinuousVariableAggregateQuantity( - @Nullable PopulationDef populationDef, Function> popDefToResources) { + @Nullable PopulationDef populationDef, + Function> popDefToResources) { if (populationDef == null) { return null; @@ -609,19 +612,9 @@ private static QuantityDef calculateContinuousVariableAggregateQuantity( */ @Nullable private static QuantityDef calculateContinuousVariableAggregateQuantity( - ContinuousVariableObservationAggregateMethod aggregateMethod, Collection qualifyingResources) { - // MIGRATION-NOTE (typed-subjectResources): qualifyingResources is sourced via the - // popDefToResources Function from PopulationDef.getAllSubjectResources() (List) or - // getResultsForStratum (List). When PopulationDef returns typed wrappers, this - // boundary wrap and the popDefToResources signature both update: Function>. The wrap-and-toList step here goes away. - // - // Test focus: continuous-variable scoring (group-level + stratum-level, both proportion - // and ratio variants) — those are the only consumers of this aggregate path. - var wrapped = qualifyingResources.stream() - .map(o -> CqlExpressionValue.ofRaw(o, null)) - .toList(); - var observationQuantity = MeasureScoreCalculator.collectQuantities(wrapped); + ContinuousVariableObservationAggregateMethod aggregateMethod, + Collection qualifyingResources) { + var observationQuantity = MeasureScoreCalculator.collectQuantities(qualifyingResources); return MeasureScoreCalculator.aggregateContinuousVariable(observationQuantity, aggregateMethod); } diff --git a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/PopulationDef.java b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/PopulationDef.java index 261091417c..18c82d61c2 100644 --- a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/PopulationDef.java +++ b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/PopulationDef.java @@ -30,27 +30,14 @@ public class PopulationDef { protected Set evaluatedResources; - // MIGRATION-NOTE (typed-subjectResources): the central data model for population results. - // For most population types each item is a FHIR resource / CQL value; for MEASUREOBSERVATION - // populations each item is actually a Map accumulator. The Set - // is a HashSetForFhirResourcesAndCqlTypes — its identity semantics (resource type + logical - // ID for IBaseResource, CQL .equal for CqlType) are load-bearing for retainAll / removeAll - // / removeIf in observation filtering and stratifier intersection. - // - // To migrate to Map> (or a new PopulationResultSet container): - // 1. Decide the wrapper's equals/hashCode story — delegating to FhirResourceAndCqlTypeUtils - // .areObjectsEqual is the obvious path but pollutes a generic wrapper. The alternative is - // a new HashSet variant that compares on raw() instead of on the wrapper. - // 2. Update every accessor below (addResource, getResourcesForSubject, getAllSubjectResources, - // getSubjectResources, retainAllResources, removeAllResources, - // removeExcludedMeasureObservationResource, countObservations, getCount). - // 3. Update every consumer — MeasureEvaluator observation methods, MeasureMultiSubjectEvaluator, - // MeasureReportDefScorer, MeasureObservationHandler, R4/Dstu3/R5 MeasureReportBuilders. - // - // Test focus: ratio + continuous-variable measures (exercise observation accumulators end-to-end); - // measures with stratifiers that intersect populations (exercise FHIR-identity equality on - // retainAll/removeAll); measures with duplicate resources across subjects (countObservations). - protected Map> subjectResources = new HashMap<>(); + /** + * Per-subject results from CQL evaluation, stored as wrappers so the FHIR-identity / CQL-type + * equality rules live in one place ({@link HashSetForCqlExpressionValues}). For most + * population types each wrapper holds a FHIR resource or CQL value; for + * {@link MeasurePopulationType#MEASUREOBSERVATION} populations each wrapper holds a + * {@code Map} accumulator. + */ + protected Map> subjectResources = new HashMap<>(); public PopulationDef( String id, @@ -144,21 +131,18 @@ public void removeExcludedMeasureObservationResource(String subjectId, Object me return; } - final Set resourcesForSubject = subjectResources.get(subjectId); + final Set resourcesForSubject = subjectResources.get(subjectId); if (resourcesForSubject == null) { return; } // Remove the key from all inner maps - resourcesForSubject.forEach(element -> CqlExpressionValue.ofRaw(element, null) - .asMap() - .ifPresent(innerMap -> innerMap.remove(measureObservationResourceKey))); + resourcesForSubject.forEach( + element -> element.asMap().ifPresent(innerMap -> innerMap.remove(measureObservationResourceKey))); // Remove empty inner maps - critical for correct counting - resourcesForSubject.removeIf(element -> CqlExpressionValue.ofRaw(element, null) - .asMap() - .map(Map::isEmpty) - .orElse(false)); + resourcesForSubject.removeIf( + element -> element.asMap().map(Map::isEmpty).orElse(false)); // If the subject's resource set is now empty, remove the subject from the map entirely if (resourcesForSubject.isEmpty()) { @@ -201,7 +185,7 @@ public void removeAllSubjects(PopulationDef otherPopulationDef) { * * */ - public List getAllSubjectResources() { + public List getAllSubjectResources() { return subjectResources.values().stream() .flatMap(Collection::stream) .filter(Objects::nonNull) @@ -210,13 +194,9 @@ public List getAllSubjectResources() { // Extracted from R4MeasureReportBuilder.countObservations() by Claude Sonnet 4.5 public int countObservations() { - if (this.getAllSubjectResources() == null) { - return 0; - } - return this.getAllSubjectResources().stream() - .map(item -> CqlExpressionValue.ofRaw(item, null).asMap()) - .flatMap(java.util.Optional::stream) + .map(CqlExpressionValue::asMap) + .flatMap(Optional::stream) .mapToInt(Map::size) .sum(); } @@ -231,28 +211,23 @@ public String expression() { } // Getter method - public Map> getSubjectResources() { + public Map> getSubjectResources() { return subjectResources; } - public Set getResourcesForSubject(String subjectId) { - return subjectResources.getOrDefault(subjectId, new HashSetForFhirResourcesAndCqlTypes<>()); + public Set getResourcesForSubject(String subjectId) { + return subjectResources.getOrDefault(subjectId, new HashSetForCqlExpressionValues()); } - // Add an element to Set under a key (Creates a new set if key is missing) - // - // MIGRATION-NOTE (typed-subjectResources): the only entry point for inserting into - // subjectResources. Once the field is typed, this is where Object → CqlExpressionValue - // wrapping happens (via CqlExpressionValue.ofRaw(value, null), preserving the lack of - // evaluatedResources at this granularity). Callers in MeasureEvaluator.evaluatePopulation - // Membership and FunctionEvaluationHandler.aggregateFunctionResults still pass raw Object; - // the wrap should land here, not at every caller. - // - // Test focus: any measure that lands resources in a population — every flavour exercises this. + /** + * The single insertion point for population results. Wraps raw {@link Object} in a + * {@link CqlExpressionValue} so the underlying Set ({@link HashSetForCqlExpressionValues}) + * can dedupe by FHIR-resource / CQL-type identity rather than Java object identity. + */ public void addResource(String key, Object value) { subjectResources - .computeIfAbsent(key, k -> new HashSetForFhirResourcesAndCqlTypes<>()) - .add(value); + .computeIfAbsent(key, k -> new HashSetForCqlExpressionValues()) + .add(CqlExpressionValue.ofRaw(value, null)); } @Nullable diff --git a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/dstu3/Dstu3MeasureReportBuilder.java b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/dstu3/Dstu3MeasureReportBuilder.java index b4c7dd42e7..7dc740fa40 100644 --- a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/dstu3/Dstu3MeasureReportBuilder.java +++ b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/dstu3/Dstu3MeasureReportBuilder.java @@ -35,6 +35,7 @@ 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.fhir.cr.measure.common.CqlExpressionValue; import org.opencds.cqf.fhir.cr.measure.common.GroupDef; import org.opencds.cqf.fhir.cr.measure.common.MeasureDef; import org.opencds.cqf.fhir.cr.measure.common.MeasureInfo; @@ -280,14 +281,6 @@ protected void buildPopulation( reportPopulation.setCode(measurePopulation.getCode()); reportPopulation.setId(measurePopulation.getId()); - // MIGRATION-NOTE (typed-subjectResources): the .size() call only needs the count, so the - // typed migration is transparent here. The MEASUREOBSERVATION branch below at line ~320 - // hands getAllSubjectResources() into buildMeasureObservations(...), which expects raw - // Object items — that signature needs to change to consume CqlExpressionValue or unwrap - // via .stream().map(CqlExpressionValue::raw). - // - // Test focus: DSTU3 measure reports for continuous-variable measures (the only ones that - // hit buildMeasureObservations); DSTU3 patient-list reports. if (!measureDef.groups().isEmpty() && !measureDef.groups().get(0).isBooleanBasis()) { reportPopulation.setCount(populationDef.getAllSubjectResources().size()); } else { @@ -332,7 +325,7 @@ protected void buildPopulation( } } - protected void buildMeasureObservations(String observationName, Collection resources) { + protected void buildMeasureObservations(String observationName, Collection resources) { for (int i = 0; i < resources.size(); i++) { // TODO: Do something with the resource... Observation observation = diff --git a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/r4/R4MeasureReportBuilder.java b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/r4/R4MeasureReportBuilder.java index 86358003bc..d053aba6ee 100644 --- a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/r4/R4MeasureReportBuilder.java +++ b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/r4/R4MeasureReportBuilder.java @@ -41,6 +41,7 @@ import org.opencds.cqf.cql.engine.runtime.Interval; import org.opencds.cqf.fhir.cr.measure.common.CodeDef; import org.opencds.cqf.fhir.cr.measure.common.ConceptDef; +import org.opencds.cqf.fhir.cr.measure.common.CqlExpressionValue; import org.opencds.cqf.fhir.cr.measure.common.FhirResourceUtils; import org.opencds.cqf.fhir.cr.measure.common.GroupDef; import org.opencds.cqf.fhir.cr.measure.common.MeasureDef; @@ -197,7 +198,8 @@ private void buildGroup( if (docPopDef != null && docPopDef.getAllSubjectResources() != null && !docPopDef.getAllSubjectResources().isEmpty()) { - var docValue = docPopDef.getAllSubjectResources().iterator().next(); + var docValue = + docPopDef.getAllSubjectResources().iterator().next().raw(); if (docValue != null) { assert docValue instanceof Interval; Interval docInterval = (Interval) docValue; @@ -227,8 +229,8 @@ private void addMeasureDescription(MeasureReportGroupComponent reportGroup, Meas } } - private String getPopulationResourceIds(Object resourceObject) { - if (resourceObject instanceof IBaseResource resource) { + private String getPopulationResourceIds(CqlExpressionValue wrapper) { + if (wrapper.raw() instanceof IBaseResource resource) { return resource.getIdElement().toVersionless().getValueAsString(); } return null; @@ -263,14 +265,6 @@ private void buildPopulation( // This is a temporary list carried forward to stratifiers // subjectResult set defined by basis of Measure - // MIGRATION-NOTE (typed-subjectResources): when getAllSubjectResources() returns - // List, the filter chain becomes - // .map(CqlExpressionValue::raw).filter(Resource.class::isInstance) — preserve the - // existing "non-Resource entries are silently dropped" behaviour. The stratifier path - // downstream consumes populationSet as Set, so wrapper boundary stops here. - // - // Test focus: subject-list reports (where populationSet drives subject references); ratio - // measures (which carry FHIR Resource lookups across both numerator and denominator). Set populationSet; if (groupDef.isBooleanBasis()) { populationSet = populationDef.getSubjects().stream() @@ -278,7 +272,7 @@ private void buildPopulation( .collect(Collectors.toSet()); } else { populationSet = populationDef.getAllSubjectResources().stream() - .filter(Resource.class::isInstance) + .filter(wrapper -> wrapper.raw() instanceof Resource) .map(this::getPopulationResourceIds) .collect(Collectors.toSet()); } diff --git a/cqf-fhir-cr/src/test/java/org/opencds/cqf/fhir/cr/measure/common/MeasureObservationHandlerTest.java b/cqf-fhir-cr/src/test/java/org/opencds/cqf/fhir/cr/measure/common/MeasureObservationHandlerTest.java index 37801655ec..30b3742754 100644 --- a/cqf-fhir-cr/src/test/java/org/opencds/cqf/fhir/cr/measure/common/MeasureObservationHandlerTest.java +++ b/cqf-fhir-cr/src/test/java/org/opencds/cqf/fhir/cr/measure/common/MeasureObservationHandlerTest.java @@ -76,10 +76,8 @@ void removeObservationResourcesInPopulation_removesMatchingResources_withSeparat ContinuousVariableObservationAggregateMethod.SUM, List.of()); - Set observationResources = new HashSetForFhirResourcesAndCqlTypes<>(); - observationResources.add(observationMap1); - observationResources.add(observationMap2); - measureObservationDef.subjectResources.put(SUBJECT_ID_1, observationResources); + measureObservationDef.addResource(SUBJECT_ID_1, observationMap1); + measureObservationDef.addResource(SUBJECT_ID_1, observationMap2); // Create MEASUREPOPULATIONEXCLUSION population with encounter to exclude measurePopulationExclusionDef = new PopulationDef( @@ -90,21 +88,20 @@ void removeObservationResourcesInPopulation_removesMatchingResources_withSeparat codeDef, List.of()); - Set exclusionResources = new HashSetForFhirResourcesAndCqlTypes<>(); - exclusionResources.add(encounter1InExclusion); // This should match encounter1InObservation by ID - measurePopulationExclusionDef.subjectResources.put(SUBJECT_ID_1, exclusionResources); + // This encounter should match encounter1InObservation by ID + measurePopulationExclusionDef.addResource(SUBJECT_ID_1, encounter1InExclusion); // When: Remove observation resources that match exclusions MeasureObservationHandler.removeObservationResourcesInPopulation( SUBJECT_ID_1, measurePopulationExclusionDef, measureObservationDef); // Then: The empty map should be removed, leaving only 1 map - Set remainingObservations = measureObservationDef.getResourcesForSubject(SUBJECT_ID_1); + Set remainingObservations = measureObservationDef.getResourcesForSubject(SUBJECT_ID_1); assertThat( "Should have 1 observation map remaining after empty map removal", remainingObservations, hasSize(1)); // Verify encounter-1's map was removed and only encounter-2 remains - Map remainingMap = (Map) remainingObservations.iterator().next(); + Map remainingMap = remainingObservations.iterator().next().asMap().orElseThrow(); assertThat("Remaining map should have 1 entry", remainingMap.size(), is(1)); assertTrue(remainingMap.containsKey(encounter2InObservation), "Should contain encounter-2"); var quantityFromMapForEncounter = remainingMap.get(encounter2InObservation); @@ -137,29 +134,25 @@ void removeObservationResourcesInPopulation_removesMultipleMatchingResources() { obsMap3.put(enc3Obs, new QuantityDef(300.0)); measureObservationDef = createMeasureObservationDef(); - Set observationResources = new HashSetForFhirResourcesAndCqlTypes<>(); - observationResources.add(obsMap1); - observationResources.add(obsMap2); - observationResources.add(obsMap3); - measureObservationDef.subjectResources.put(SUBJECT_ID_1, observationResources); + measureObservationDef.addResource(SUBJECT_ID_1, obsMap1); + measureObservationDef.addResource(SUBJECT_ID_1, obsMap2); + measureObservationDef.addResource(SUBJECT_ID_1, obsMap3); // Create exclusions measurePopulationExclusionDef = createMeasurePopulationExclusionDef(); - Set exclusionResources = new HashSetForFhirResourcesAndCqlTypes<>(); - exclusionResources.add(enc1Excl); - exclusionResources.add(enc2Excl); - measurePopulationExclusionDef.subjectResources.put(SUBJECT_ID_1, exclusionResources); + measurePopulationExclusionDef.addResource(SUBJECT_ID_1, enc1Excl); + measurePopulationExclusionDef.addResource(SUBJECT_ID_1, enc2Excl); // When MeasureObservationHandler.removeObservationResourcesInPopulation( SUBJECT_ID_1, measurePopulationExclusionDef, measureObservationDef); // Then: The two empty maps (obsMap1 and obsMap2) should be removed, leaving only obsMap3 - Set remainingObservations = measureObservationDef.getResourcesForSubject(SUBJECT_ID_1); + Set remainingObservations = measureObservationDef.getResourcesForSubject(SUBJECT_ID_1); assertThat("Should have 1 observation map remaining", remainingObservations, hasSize(1)); // Verify only enc3 remains - Map remainingMap = (Map) remainingObservations.iterator().next(); + Map remainingMap = remainingObservations.iterator().next().asMap().orElseThrow(); assertThat("Map should have 1 entry", remainingMap.size(), is(1)); assertTrue(remainingMap.containsKey(enc3Obs), "Should contain encounter-3"); var quantityFromMapForEncounter = remainingMap.get(enc3Obs); @@ -187,23 +180,19 @@ void removeObservationResourcesInPopulation_noMatchingExclusions_allObservations obsMap2.put(enc2Obs, new QuantityDef(200.0)); measureObservationDef = createMeasureObservationDef(); - Set observationResources = new HashSetForFhirResourcesAndCqlTypes<>(); - observationResources.add(obsMap1); - observationResources.add(obsMap2); - measureObservationDef.subjectResources.put(SUBJECT_ID_1, observationResources); + measureObservationDef.addResource(SUBJECT_ID_1, obsMap1); + measureObservationDef.addResource(SUBJECT_ID_1, obsMap2); measurePopulationExclusionDef = createMeasurePopulationExclusionDef(); - Set exclusionResources = new HashSetForFhirResourcesAndCqlTypes<>(); - exclusionResources.add(enc3Excl); - exclusionResources.add(enc4Excl); - measurePopulationExclusionDef.subjectResources.put(SUBJECT_ID_1, exclusionResources); + measurePopulationExclusionDef.addResource(SUBJECT_ID_1, enc3Excl); + measurePopulationExclusionDef.addResource(SUBJECT_ID_1, enc4Excl); // When MeasureObservationHandler.removeObservationResourcesInPopulation( SUBJECT_ID_1, measurePopulationExclusionDef, measureObservationDef); // Then: All observations should remain - Set remainingObservations = measureObservationDef.getResourcesForSubject(SUBJECT_ID_1); + Set remainingObservations = measureObservationDef.getResourcesForSubject(SUBJECT_ID_1); assertThat("Should have 2 observation maps remaining", remainingObservations, hasSize(2)); } @@ -218,19 +207,17 @@ void removeObservationResourcesInPopulation_emptyExclusions_allObservationsRemai obsMap1.put(enc1, new QuantityDef(100.0)); measureObservationDef = createMeasureObservationDef(); - Set observationResources = new HashSetForFhirResourcesAndCqlTypes<>(); - observationResources.add(obsMap1); - measureObservationDef.subjectResources.put(SUBJECT_ID_1, observationResources); + measureObservationDef.addResource(SUBJECT_ID_1, obsMap1); measurePopulationExclusionDef = createMeasurePopulationExclusionDef(); - measurePopulationExclusionDef.subjectResources.put(SUBJECT_ID_1, new HashSetForFhirResourcesAndCqlTypes<>()); + // No exclusions for this subject — getResourcesForSubject returns an empty default set // When MeasureObservationHandler.removeObservationResourcesInPopulation( SUBJECT_ID_1, measurePopulationExclusionDef, measureObservationDef); // Then: All observations should remain - Set remainingObservations = measureObservationDef.getResourcesForSubject(SUBJECT_ID_1); + Set remainingObservations = measureObservationDef.getResourcesForSubject(SUBJECT_ID_1); assertThat("Should have 1 observation map remaining", remainingObservations, hasSize(1)); } @@ -241,9 +228,7 @@ void removeObservationResourcesInPopulation_emptyExclusions_allObservationsRemai void removeObservationResourcesInPopulation_nullMeasureObservation_noExceptionThrown() { // Given: Null measure observation def measurePopulationExclusionDef = createMeasurePopulationExclusionDef(); - Set exclusionResources = new HashSetForFhirResourcesAndCqlTypes<>(); - exclusionResources.add(createEncounter("encounter-1")); - measurePopulationExclusionDef.subjectResources.put(SUBJECT_ID_1, exclusionResources); + measurePopulationExclusionDef.addResource(SUBJECT_ID_1, createEncounter("encounter-1")); // When/Then: Should not throw exception MeasureObservationHandler.removeObservationResourcesInPopulation( @@ -257,16 +242,14 @@ void removeObservationResourcesInPopulation_nullMeasureObservation_noExceptionTh void removeObservationResourcesInPopulation_nullExclusionDef_noExceptionThrown() { // Given: Null exclusion def measureObservationDef = createMeasureObservationDef(); - Set observationResources = new HashSetForFhirResourcesAndCqlTypes<>(); Map obsMap1 = new HashMapForFhirResourcesAndCqlTypes<>(); obsMap1.put(createEncounter("encounter-1"), new QuantityDef(100.0)); - observationResources.add(obsMap1); - measureObservationDef.subjectResources.put(SUBJECT_ID_1, observationResources); + measureObservationDef.addResource(SUBJECT_ID_1, obsMap1); // When/Then: Should not throw exception, all observations remain MeasureObservationHandler.removeObservationResourcesInPopulation(SUBJECT_ID_1, null, measureObservationDef); - Set remainingObservations = measureObservationDef.getResourcesForSubject(SUBJECT_ID_1); + Set remainingObservations = measureObservationDef.getResourcesForSubject(SUBJECT_ID_1); assertThat("Should have 1 observation map remaining", remainingObservations, hasSize(1)); } @@ -281,21 +264,17 @@ void removeObservationResourcesInPopulation_subjectNotInExclusions_allObservatio obsMap1.put(enc1, new QuantityDef(100.0)); measureObservationDef = createMeasureObservationDef(); - Set observationResources = new HashSetForFhirResourcesAndCqlTypes<>(); - observationResources.add(obsMap1); - measureObservationDef.subjectResources.put(SUBJECT_ID_1, observationResources); + measureObservationDef.addResource(SUBJECT_ID_1, obsMap1); measurePopulationExclusionDef = createMeasurePopulationExclusionDef(); - Set exclusionResources = new HashSetForFhirResourcesAndCqlTypes<>(); - exclusionResources.add(createEncounter("encounter-1")); - measurePopulationExclusionDef.subjectResources.put(SUBJECT_ID_2, exclusionResources); + measurePopulationExclusionDef.addResource(SUBJECT_ID_2, createEncounter("encounter-1")); // When: Try to remove for subject-1 (no exclusions) MeasureObservationHandler.removeObservationResourcesInPopulation( SUBJECT_ID_1, measurePopulationExclusionDef, measureObservationDef); // Then: All observations should remain - Set remainingObservations = measureObservationDef.getResourcesForSubject(SUBJECT_ID_1); + Set remainingObservations = measureObservationDef.getResourcesForSubject(SUBJECT_ID_1); assertThat("Should have 1 observation map remaining", remainingObservations, hasSize(1)); } @@ -321,27 +300,23 @@ void removeObservationResourcesInPopulation_mixedCancelledAndNonCancelled_onlyCa obsMapActive.put(nonCancelledEncObs, new QuantityDef(420.0)); measureObservationDef = createMeasureObservationDef(); - Set observationResources = new HashSetForFhirResourcesAndCqlTypes<>(); - observationResources.add(obsMapCancelled); - observationResources.add(obsMapActive); - measureObservationDef.subjectResources.put(SUBJECT_ID_3, observationResources); + measureObservationDef.addResource(SUBJECT_ID_3, obsMapCancelled); + measureObservationDef.addResource(SUBJECT_ID_3, obsMapActive); // Only the cancelled encounter is in exclusions measurePopulationExclusionDef = createMeasurePopulationExclusionDef(); - Set exclusionResources = new HashSetForFhirResourcesAndCqlTypes<>(); - exclusionResources.add(cancelledEncExcl); - measurePopulationExclusionDef.subjectResources.put(SUBJECT_ID_3, exclusionResources); + measurePopulationExclusionDef.addResource(SUBJECT_ID_3, cancelledEncExcl); // When MeasureObservationHandler.removeObservationResourcesInPopulation( SUBJECT_ID_3, measurePopulationExclusionDef, measureObservationDef); // Then: The empty obsMapCancelled should be removed, leaving only obsMapActive - Set remainingObservations = measureObservationDef.getResourcesForSubject(SUBJECT_ID_3); + Set remainingObservations = measureObservationDef.getResourcesForSubject(SUBJECT_ID_3); assertThat("Should have 1 observation map remaining", remainingObservations, hasSize(1)); // Verify only the non-cancelled encounter remains - Map remainingMap = (Map) remainingObservations.iterator().next(); + Map remainingMap = remainingObservations.iterator().next().asMap().orElseThrow(); assertThat("Map should have 1 entry", remainingMap.size(), is(1)); assertTrue(remainingMap.containsKey(nonCancelledEncObs), "Should contain non-cancelled encounter"); QuantityDef activeQuantity = (QuantityDef) remainingMap.get(nonCancelledEncObs); @@ -363,9 +338,7 @@ void removeObservationResourcesInPopulation_allEntriesRemoved_subjectNotCounted( obsMap1.put(enc1Obs, new QuantityDef(100.0)); measureObservationDef = createMeasureObservationDef(); - Set observationResources = new HashSetForFhirResourcesAndCqlTypes<>(); - observationResources.add(obsMap1); - measureObservationDef.subjectResources.put(SUBJECT_ID_1, observationResources); + measureObservationDef.addResource(SUBJECT_ID_1, obsMap1); // Initial count should include the subject assertEquals(1, measureObservationDef.getCount(), "Initial count should be 1"); @@ -373,9 +346,7 @@ void removeObservationResourcesInPopulation_allEntriesRemoved_subjectNotCounted( // Create exclusion with the same encounter measurePopulationExclusionDef = createMeasurePopulationExclusionDef(); - Set exclusionResources = new HashSetForFhirResourcesAndCqlTypes<>(); - exclusionResources.add(enc1Excl); - measurePopulationExclusionDef.subjectResources.put(SUBJECT_ID_1, exclusionResources); + measurePopulationExclusionDef.addResource(SUBJECT_ID_1, enc1Excl); // When: Remove observation resources MeasureObservationHandler.removeObservationResourcesInPopulation( @@ -405,9 +376,7 @@ void removeObservationResourcesInPopulation_partialRemoval_subjectStillCounted() obsMap.put(enc2, new QuantityDef(200.0)); measureObservationDef = createMeasureObservationDef(); - Set observationResources = new HashSetForFhirResourcesAndCqlTypes<>(); - observationResources.add(obsMap); - measureObservationDef.subjectResources.put(SUBJECT_ID_1, observationResources); + measureObservationDef.addResource(SUBJECT_ID_1, obsMap); // Initial count should be 2 (two observation entries) assertEquals(2, measureObservationDef.getCount(), "Initial count should be 2"); @@ -415,9 +384,7 @@ void removeObservationResourcesInPopulation_partialRemoval_subjectStillCounted() // Create exclusion with only one encounter measurePopulationExclusionDef = createMeasurePopulationExclusionDef(); - Set exclusionResources = new HashSetForFhirResourcesAndCqlTypes<>(); - exclusionResources.add(enc1Excl); - measurePopulationExclusionDef.subjectResources.put(SUBJECT_ID_1, exclusionResources); + measurePopulationExclusionDef.addResource(SUBJECT_ID_1, enc1Excl); // When: Remove observation resources MeasureObservationHandler.removeObservationResourcesInPopulation( @@ -455,17 +422,9 @@ void removeObservationResourcesInPopulation_multipleSubjects_countsCorrectly() { obsMap3.put(enc4, new QuantityDef(400.0)); measureObservationDef = createMeasureObservationDef(); - Set obs1 = new HashSetForFhirResourcesAndCqlTypes<>(); - obs1.add(obsMap1); - measureObservationDef.subjectResources.put(SUBJECT_ID_1, obs1); - - Set obs2 = new HashSetForFhirResourcesAndCqlTypes<>(); - obs2.add(obsMap2); - measureObservationDef.subjectResources.put(SUBJECT_ID_2, obs2); - - Set obs3 = new HashSetForFhirResourcesAndCqlTypes<>(); - obs3.add(obsMap3); - measureObservationDef.subjectResources.put(SUBJECT_ID_3, obs3); + measureObservationDef.addResource(SUBJECT_ID_1, obsMap1); + measureObservationDef.addResource(SUBJECT_ID_2, obsMap2); + measureObservationDef.addResource(SUBJECT_ID_3, obsMap3); // Initial: 1 + 2 + 1 = 4 total observations, 3 subjects assertEquals(4, measureObservationDef.getCount(), "Initial count should be 4"); @@ -475,14 +434,10 @@ void removeObservationResourcesInPopulation_multipleSubjects_countsCorrectly() { measurePopulationExclusionDef = createMeasurePopulationExclusionDef(); // Subject 1: Exclude all (enc1) - Set excl1 = new HashSetForFhirResourcesAndCqlTypes<>(); - excl1.add(createEncounter("encounter-1")); - measurePopulationExclusionDef.subjectResources.put(SUBJECT_ID_1, excl1); + measurePopulationExclusionDef.addResource(SUBJECT_ID_1, createEncounter("encounter-1")); // Subject 2: Exclude one (enc2) - Set excl2 = new HashSetForFhirResourcesAndCqlTypes<>(); - excl2.add(createEncounter("encounter-2")); - measurePopulationExclusionDef.subjectResources.put(SUBJECT_ID_2, excl2); + measurePopulationExclusionDef.addResource(SUBJECT_ID_2, createEncounter("encounter-2")); // When: Remove for each subject MeasureObservationHandler.removeObservationResourcesInPopulation( diff --git a/cqf-fhir-cr/src/test/java/org/opencds/cqf/fhir/cr/measure/common/PopulationDefTest.java b/cqf-fhir-cr/src/test/java/org/opencds/cqf/fhir/cr/measure/common/PopulationDefTest.java index d13ce6ead1..8cfbebcaff 100644 --- a/cqf-fhir-cr/src/test/java/org/opencds/cqf/fhir/cr/measure/common/PopulationDefTest.java +++ b/cqf-fhir-cr/src/test/java/org/opencds/cqf/fhir/cr/measure/common/PopulationDefTest.java @@ -30,7 +30,7 @@ void setHandlingStrings() { popDef1.retainAllResources("subj1", popDef2); assertEquals(1, popDef1.getAllSubjectResources().size()); - assertTrue(popDef1.getAllSubjectResources().contains("string1")); + assertTrue(getResourcesDistinctAcrossAllSubjects(popDef1).contains("string1")); } @Test @@ -47,7 +47,7 @@ void setHandlingIntegers() { popDef1.retainAllResources("subj1", popDef2); assertEquals(1, popDef1.getAllSubjectResources().size()); - assertTrue(popDef1.getAllSubjectResources().contains(123)); + assertTrue(getResourcesDistinctAcrossAllSubjects(popDef1).contains(123)); } @Test @@ -77,6 +77,7 @@ private Set getResourcesDistinctAcrossAllSubjects(PopulationDef popDef) return new HashSetForFhirResourcesAndCqlTypes<>(popDef.getSubjectResources().values().stream() .flatMap(Collection::stream) .filter(Objects::nonNull) + .map(CqlExpressionValue::raw) .collect(Collectors.toUnmodifiableSet())); } From 03e3ef995fc569167155b1cf3df5085b9a3cb628 Mon Sep 17 00:00:00 2001 From: Luke deGruchy Date: Thu, 30 Apr 2026 11:33:55 -0400 Subject: [PATCH 09/13] Tighten null-safety on CqlExpressionValue iteration sites After the typed-subjectResources migration, several spots dereferenced .raw() or called wrapper methods without first guarding against null wrappers (which the underlying HashSet permits in principle). This adds defensive null filters / null-checks at the iteration boundaries. Spots tightened: - MeasureEvaluator: removeIf lambdas in retainObservation SubjectResourcesInPopulation, retainObservationResourcesInPopulation, and removeObservatorySubjectResource skip null wrappers; the removeObservatorySubjectResource null-then-isMap check no longer NPEs on a null first element. - MeasureMultiSubjectEvaluator: stream chains over Set add Objects::nonNull filters before .map(CqlExpressionValue:: raw / asMap), and the criteria-stratifier intersection skips null wrappers / null raw values explicitly. - MeasureObservationHandler: removeMatchingKeysFromObservationMap skips null exclusion wrappers. - MeasureReportDefScorer: getResultsForStratumByResourceIds adds Objects::nonNull after the flatMap before consuming wrappers. - PopulationDef: removeExcludedMeasureObservationResource null-guards the forEach and removeIf elements. - R4MeasureReportBuilder: getPopulationResourceIds and the populationSet filter null-guard the wrapper before calling .raw(); the date-of- compliance pull splits the iterator chain so the wrapper itself can be null-checked separately. - Dstu3MeasureReportBuilder: stratifier groupingBy null-guards the wrapper looked up via Map.get before calling .raw(). - EvaluationResultFormatter: printSubjectResources filters null wrappers before mapping to .raw(). Co-Authored-By: Claude Opus 4.7 (1M context) --- .../common/EvaluationResultFormatter.java | 2 ++ .../cr/measure/common/MeasureEvaluator.java | 17 ++++++++++++--- .../common/MeasureMultiSubjectEvaluator.java | 21 ++++++++++++++----- .../common/MeasureObservationHandler.java | 3 +++ .../common/MeasureReportDefScorer.java | 3 +++ .../fhir/cr/measure/common/PopulationDef.java | 9 +++++--- .../dstu3/Dstu3MeasureReportBuilder.java | 6 ++++-- .../cr/measure/r4/R4MeasureReportBuilder.java | 9 ++++---- 8 files changed, 53 insertions(+), 17 deletions(-) diff --git a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/EvaluationResultFormatter.java b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/EvaluationResultFormatter.java index 6a2c897ce8..5b2a3693a9 100644 --- a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/EvaluationResultFormatter.java +++ b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/EvaluationResultFormatter.java @@ -7,6 +7,7 @@ import java.util.Collection; import java.util.Date; import java.util.Map; +import java.util.Objects; import java.util.Set; import java.util.stream.Collectors; import java.util.stream.StreamSupport; @@ -262,6 +263,7 @@ public static Object printSubjectResources(PopulationDef populationDef, String s } final String toString = resources.stream() + .filter(Objects::nonNull) .map(CqlExpressionValue::raw) .map(EvaluationResultFormatter::printValue) .collect(Collectors.joining(", ")); diff --git a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureEvaluator.java b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureEvaluator.java index 4cd7d0e5de..f19dccdd41 100644 --- a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureEvaluator.java +++ b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureEvaluator.java @@ -452,6 +452,9 @@ public void retainObservationSubjectResourcesInPopulation( // remove observation accumulators whose keys aren't all in the valid population obsSet.removeIf(item -> { + if (item == null) { + return false; + } Map obsMap = item.asMap().orElse(null); if (obsMap == null) { return false; // not an observation accumulator, leave alone @@ -483,6 +486,9 @@ protected void retainObservationResourcesInPopulation( Set measurePopulationResourcesForSubject = measurePopulationDef.getResourcesForSubject(subjectId); measureObservationDef.getResourcesForSubject(subjectId).removeIf(populationResource -> { + if (populationResource == null) { + return false; + } Map obsMap = populationResource.asMap().orElse(null); if (obsMap == null) { return false; @@ -537,8 +543,9 @@ private void removeObservatorySubjectResource( } final CqlExpressionValue firstEntryValue = entryValue.iterator().next(); - if (!firstEntryValue.isMap()) { - throw new InternalErrorException("Expected a Map but was not: %s".formatted(firstEntryValue.raw())); + if (firstEntryValue == null || !firstEntryValue.isMap()) { + throw new InternalErrorException("Expected a Map but was not: %s" + .formatted(firstEntryValue == null ? "null" : firstEntryValue.raw())); } // population values for this subject @@ -552,12 +559,16 @@ private void removeObservatorySubjectResource( // Remove observations that *do* match population values entryValue.removeIf(item -> { + if (item == null) { + return false; + } Map obsMap = item.asMap().orElse(null); if (obsMap == null) { return false; } for (Object key : obsMap.keySet()) { - if (populationValues.contains(key)) { + if (key instanceof CqlExpressionValue expressionValueKey + && populationValues.contains(expressionValueKey)) { // This observation map is backed by a population resource -> remove iterator return true; } diff --git a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureMultiSubjectEvaluator.java b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureMultiSubjectEvaluator.java index 33a93878e1..d23cbe81bf 100644 --- a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureMultiSubjectEvaluator.java +++ b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureMultiSubjectEvaluator.java @@ -11,6 +11,7 @@ import java.util.List; import java.util.Map; import java.util.Map.Entry; +import java.util.Objects; import java.util.Set; import java.util.stream.Collector; import java.util.stream.Collectors; @@ -774,8 +775,11 @@ private static Set calculateCriteriaStratifierIntersection( populationDef.getResourcesForSubject(stratifierEntryBySubject.getKey()); for (CqlExpressionValue wrapper : populationResultsPerSubject) { - Object raw = wrapper == null ? null : wrapper.raw(); - if (stratifierResultsPerSubject.contains(raw)) { + if (wrapper == null) { + continue; + } + Object raw = wrapper.raw(); + if (raw != null && stratifierResultsPerSubject.contains(raw)) { allPopulationStratumIntersectingResources.add(raw); } } @@ -891,13 +895,16 @@ private static List getResourcesForSubjects( if (resources != null) { if (isResourceType) { resources.stream() + .filter(Objects::nonNull) .map(CqlExpressionValue::raw) .map(MeasureMultiSubjectEvaluator::normalizePopulationKey) - .filter(java.util.Objects::nonNull) + .filter(Objects::nonNull) .forEach(resourceIds::add); } else { resources.stream() + .filter(Objects::nonNull) .map(CqlExpressionValue::raw) + .filter(Objects::nonNull) .map(Object::toString) .forEach(resourceIds::add); } @@ -939,25 +946,29 @@ private static Set getPopulationResourceKeySet( if (populationDef.type() == MeasurePopulationType.MEASUREOBSERVATION) { // MEASUREOBSERVATION always deals with FHIR resources, so no subject qualification needed resources.stream() + .filter(Objects::nonNull) .map(CqlExpressionValue::asMap) .flatMap(java.util.Optional::stream) .flatMap(m -> m.keySet().stream()) .map(MeasureMultiSubjectEvaluator::normalizePopulationKey) - .filter(java.util.Objects::nonNull) + .filter(Objects::nonNull) .map(SubjectResourceKey::resourceOnly) .forEach(resourceKeys::add); } else if (isResourceType) { // FHIR resource types have globally unique IDs - no subject qualification needed resources.stream() + .filter(Objects::nonNull) .map(CqlExpressionValue::raw) .map(MeasureMultiSubjectEvaluator::normalizePopulationKey) - .filter(java.util.Objects::nonNull) + .filter(Objects::nonNull) .map(SubjectResourceKey::resourceOnly) .forEach(resourceKeys::add); } else { // Primitive types (like Date) - include subject context to preserve duplicates resources.stream() + .filter(Objects::nonNull) .map(CqlExpressionValue::raw) + .filter(Objects::nonNull) .map(obj -> SubjectResourceKey.of(qualifiedSubject, obj.toString())) .forEach(resourceKeys::add); } diff --git a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureObservationHandler.java b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureObservationHandler.java index d53f5a5db3..1b1d3f0127 100644 --- a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureObservationHandler.java +++ b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureObservationHandler.java @@ -87,6 +87,9 @@ private static void removeMatchingKeysFromObservationMap( // Find observation map keys that match any exclusion resource for (CqlExpressionValue exclusionResource : exclusionResources) { + if (exclusionResource == null) { + continue; + } Object exclusionRaw = exclusionResource.raw(); // Check if this exclusion resource matches any key in the observation map // Must use custom equality that compares FHIR resource identity, not object instance diff --git a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureReportDefScorer.java b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureReportDefScorer.java index cd6b6d9326..ebe20284f1 100644 --- a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureReportDefScorer.java +++ b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureReportDefScorer.java @@ -5,6 +5,7 @@ import java.util.Collection; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Optional; import java.util.Set; import java.util.function.Function; @@ -542,6 +543,7 @@ private static Collection getResultsForStratumByResourceIds( if (populationDef.type() == MeasurePopulationType.MEASUREOBSERVATION) { return populationDef.getSubjectResources().values().stream() .flatMap(Collection::stream) + .filter(Objects::nonNull) .map(CqlExpressionValue::asMap) .flatMap(Optional::stream) .map(map -> { @@ -569,6 +571,7 @@ private static Collection getResultsForStratumByResourceIds( // For non-MEASUREOBSERVATION populations, filter resources directly return populationDef.getSubjectResources().values().stream() .flatMap(Collection::stream) + .filter(Objects::nonNull) .filter(wrapper -> { Object raw = wrapper.raw(); if (raw instanceof IBaseResource baseResource) { diff --git a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/PopulationDef.java b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/PopulationDef.java index 18c82d61c2..b1f51b6a98 100644 --- a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/PopulationDef.java +++ b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/PopulationDef.java @@ -137,12 +137,15 @@ public void removeExcludedMeasureObservationResource(String subjectId, Object me } // Remove the key from all inner maps - resourcesForSubject.forEach( - element -> element.asMap().ifPresent(innerMap -> innerMap.remove(measureObservationResourceKey))); + resourcesForSubject.forEach(element -> { + if (element != null) { + element.asMap().ifPresent(innerMap -> innerMap.remove(measureObservationResourceKey)); + } + }); // Remove empty inner maps - critical for correct counting resourcesForSubject.removeIf( - element -> element.asMap().map(Map::isEmpty).orElse(false)); + element -> element != null && element.asMap().map(Map::isEmpty).orElse(false)); // If the subject's resource set is now empty, remove the subject from the map entirely if (resourcesForSubject.isEmpty()) { diff --git a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/dstu3/Dstu3MeasureReportBuilder.java b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/dstu3/Dstu3MeasureReportBuilder.java index 7dc740fa40..6f7f3b0ab7 100644 --- a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/dstu3/Dstu3MeasureReportBuilder.java +++ b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/dstu3/Dstu3MeasureReportBuilder.java @@ -202,8 +202,10 @@ protected void buildStratifier( // equals // the StratumValueWrapper does it for them. Map> subjectsByValue = subjectValues.keySet().stream() - .collect(Collectors.groupingBy( - x -> new StratumValueWrapper(subjectValues.get(x).raw()))); + .collect(Collectors.groupingBy(x -> { + var wrapper = subjectValues.get(x); + return new StratumValueWrapper(wrapper == null ? null : wrapper.raw()); + })); for (Map.Entry> stratValue : subjectsByValue.entrySet()) { buildStratum( diff --git a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/r4/R4MeasureReportBuilder.java b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/r4/R4MeasureReportBuilder.java index d053aba6ee..a2c10f0493 100644 --- a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/r4/R4MeasureReportBuilder.java +++ b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/r4/R4MeasureReportBuilder.java @@ -198,8 +198,9 @@ private void buildGroup( if (docPopDef != null && docPopDef.getAllSubjectResources() != null && !docPopDef.getAllSubjectResources().isEmpty()) { - var docValue = - docPopDef.getAllSubjectResources().iterator().next().raw(); + var firstWrapper = + docPopDef.getAllSubjectResources().iterator().next(); + var docValue = firstWrapper == null ? null : firstWrapper.raw(); if (docValue != null) { assert docValue instanceof Interval; Interval docInterval = (Interval) docValue; @@ -230,7 +231,7 @@ private void addMeasureDescription(MeasureReportGroupComponent reportGroup, Meas } private String getPopulationResourceIds(CqlExpressionValue wrapper) { - if (wrapper.raw() instanceof IBaseResource resource) { + if (wrapper != null && wrapper.raw() instanceof IBaseResource resource) { return resource.getIdElement().toVersionless().getValueAsString(); } return null; @@ -272,7 +273,7 @@ private void buildPopulation( .collect(Collectors.toSet()); } else { populationSet = populationDef.getAllSubjectResources().stream() - .filter(wrapper -> wrapper.raw() instanceof Resource) + .filter(wrapper -> wrapper != null && wrapper.raw() instanceof Resource) .map(this::getPopulationResourceIds) .collect(Collectors.toSet()); } From 2251ae0c98b1e9a883b399165b4352a6d2d40b93 Mon Sep 17 00:00:00 2001 From: Luke deGruchy Date: Fri, 1 May 2026 14:39:24 -0400 Subject: [PATCH 10/13] Replace measure-observation Map with typed ObservationAccumulator The MEASUREOBSERVATION population accumulator was a Map with implicit semantics (FHIR-identity keys, QuantityDef values). Replace it with two records: ObservationEntry(inputResource, observation) and ObservationAccumulator wrapping a List. The accumulator wraps the list in a non-Iterable record so the upstream asIterable() path doesn't unroll it into individual entries. CqlExpressionValue gains asObservationAccumulator() alongside asMap(); generic asMap() is kept because supporting-evidence and formatting code still consumes arbitrary CQL Maps that are not observation accumulators. This is commit A of three. Commit B will give the same treatment to the stratifier function-result Map; commit C will delete HashMapForFhirResourcesAndCqlTypes once both are migrated. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../cr/measure/common/CqlExpressionValue.java | 16 +- .../common/FunctionEvaluationHandler.java | 21 +- .../cr/measure/common/MeasureEvaluator.java | 37 +-- .../common/MeasureMultiSubjectEvaluator.java | 9 +- .../common/MeasureObservationHandler.java | 37 ++- .../common/MeasureReportDefScorer.java | 37 ++- .../common/MeasureScoreCalculator.java | 8 +- .../common/ObservationAccumulator.java | 27 +++ .../cr/measure/common/ObservationEntry.java | 21 ++ .../fhir/cr/measure/common/PopulationDef.java | 36 ++- .../common/CqlExpressionValueTest.java | 45 ++++ .../common/MeasureObservationHandlerTest.java | 134 ++++++----- .../common/MeasureReportDefScorerTest.java | 227 ++++++------------ .../common/MeasureScoreCalculatorTest.java | 36 ++- .../cr/measure/common/PopulationDefTest.java | 46 ++-- 15 files changed, 379 insertions(+), 358 deletions(-) create mode 100644 cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/ObservationAccumulator.java create mode 100644 cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/ObservationEntry.java diff --git a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/CqlExpressionValue.java b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/CqlExpressionValue.java index 4546817fa2..47e2167471 100644 --- a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/CqlExpressionValue.java +++ b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/CqlExpressionValue.java @@ -108,14 +108,26 @@ public Optional asBoolean() { /** * Returns the underlying value as a typed {@link Map} when it is one, otherwise empty. * The single unchecked cast is localized here so call sites do not have to repeat it. - * Used for measure-observation accumulators where the CQL engine produces - * {@code Map}. + * Used for arbitrary CQL Map values (e.g. supporting evidence, formatting). For + * measure-observation accumulators produced by + * {@code FunctionEvaluationHandler.processMeasureObservation}, prefer + * {@link #asObservationAccumulator()}. */ @SuppressWarnings("unchecked") public Optional> asMap() { return raw instanceof Map map ? Optional.of((Map) map) : Optional.empty(); } + /** + * Returns the underlying value as the {@link ObservationAccumulator} produced by + * {@code FunctionEvaluationHandler.processMeasureObservation}, or empty otherwise. + * The accumulator wraps a {@code List} in a non-Iterable record so the + * upstream {@link #asIterable()} path doesn't unroll it into individual entries. + */ + public Optional asObservationAccumulator() { + return raw instanceof ObservationAccumulator acc ? Optional.of(acc) : Optional.empty(); + } + /** * Normalizes the value to an {@link Iterable}: null becomes an empty list, an existing * iterable is returned as-is, and a scalar is wrapped in a single-element list. diff --git a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/FunctionEvaluationHandler.java b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/FunctionEvaluationHandler.java index e8409ed314..d437300467 100644 --- a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/FunctionEvaluationHandler.java +++ b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/FunctionEvaluationHandler.java @@ -269,9 +269,12 @@ private static EvaluationResult processMeasureObservation( // this will be used in MeasureEvaluator var expressionName = criteriaPopulationId + "-" + observationExpression; - // VERY IMPORTANT: We need a custom Map to ensure remove by FHIR resource key does not - // use object identity (AKA ==) - final Map functionResults = new HashMapForFhirResourcesAndCqlTypes<>(); + // Each entry pairs an input from the population with the QuantityDef that the observation + // function produced for it. Consumers (PopulationDef, MeasureEvaluator, MeasureScoreCalculator, + // MeasureObservationHandler) iterate this list and apply FHIR-identity comparisons via + // FhirResourceAndCqlTypeUtils.areObjectsEqual where needed; nothing in the downstream pipeline + // does random-access lookup by input, so a List is sufficient and self-documenting. + final List functionResults = new ArrayList<>(); final Set evaluatedResources = new HashSet<>(); final String exceptionMessageIfNotFunction = """ @@ -291,17 +294,11 @@ private static EvaluationResult processMeasureObservation( exceptionMessageIfNotFunction); var quantity = convertCqlResultToQuantityDef(observationResult.getValue()); - // add function results to existing EvaluationResult under new expression - // name - // need a way to capture input parameter here too, otherwise we have no way - // to connect input objects related to output object - // key= input parameter to function - // value= the output Observation resource containing calculated value - functionResults.put(result, quantity); + functionResults.add(new ObservationEntry(result, quantity)); Optional.ofNullable(observationResult.getEvaluatedResources()).ifPresent(evaluatedResources::addAll); } - return buildEvaluationResult(expressionName, functionResults, evaluatedResources); + return buildEvaluationResult(expressionName, new ObservationAccumulator(functionResults), evaluatedResources); } /** @@ -650,7 +647,7 @@ private static boolean hasNonSubValueStratifier(MeasureDef measureDef) { } private static EvaluationResult buildEvaluationResult( - String expressionName, Map functionResults, Set evaluatedResources) { + String expressionName, Object functionResults, Set evaluatedResources) { final EvaluationResult evaluationResultToReturn = new EvaluationResult(); diff --git a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureEvaluator.java b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureEvaluator.java index f19dccdd41..d0515074c7 100644 --- a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureEvaluator.java +++ b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureEvaluator.java @@ -450,18 +450,18 @@ public void retainObservationSubjectResourcesInPopulation( continue; } - // remove observation accumulators whose keys aren't all in the valid population + // remove observation accumulators whose inputs aren't all in the valid population obsSet.removeIf(item -> { if (item == null) { return false; } - Map obsMap = item.asMap().orElse(null); - if (obsMap == null) { + ObservationAccumulator acc = item.asObservationAccumulator().orElse(null); + if (acc == null) { return false; // not an observation accumulator, leave alone } - for (Object key : obsMap.keySet()) { - if (!validPopulation.contains(key)) { - return true; // remove this observation map + for (ObservationEntry obsEntry : acc.entries()) { + if (!validPopulation.contains(obsEntry.inputResource())) { + return true; // remove this observation accumulator } } return false; @@ -489,12 +489,13 @@ protected void retainObservationResourcesInPopulation( if (populationResource == null) { return false; } - Map obsMap = populationResource.asMap().orElse(null); - if (obsMap == null) { + ObservationAccumulator acc = + populationResource.asObservationAccumulator().orElse(null); + if (acc == null) { return false; } - for (Object key : obsMap.keySet()) { - if (!measurePopulationResourcesForSubject.contains(key)) { + for (ObservationEntry entry : acc.entries()) { + if (!measurePopulationResourcesForSubject.contains(entry.inputResource())) { return true; } } @@ -543,8 +544,9 @@ private void removeObservatorySubjectResource( } final CqlExpressionValue firstEntryValue = entryValue.iterator().next(); - if (firstEntryValue == null || !firstEntryValue.isMap()) { - throw new InternalErrorException("Expected a Map but was not: %s" + if (firstEntryValue == null + || firstEntryValue.asObservationAccumulator().isEmpty()) { + throw new InternalErrorException("Expected an observation accumulator but was not: %s" .formatted(firstEntryValue == null ? "null" : firstEntryValue.raw())); } @@ -562,14 +564,13 @@ private void removeObservatorySubjectResource( if (item == null) { return false; } - Map obsMap = item.asMap().orElse(null); - if (obsMap == null) { + ObservationAccumulator acc = item.asObservationAccumulator().orElse(null); + if (acc == null) { return false; } - for (Object key : obsMap.keySet()) { - if (key instanceof CqlExpressionValue expressionValueKey - && populationValues.contains(expressionValueKey)) { - // This observation map is backed by a population resource -> remove iterator + for (ObservationEntry entry : acc.entries()) { + if (populationValues.contains(entry.inputResource())) { + // This observation accumulator is backed by a population resource -> drop it return true; } } diff --git a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureMultiSubjectEvaluator.java b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureMultiSubjectEvaluator.java index d23cbe81bf..1a88b0e9f2 100644 --- a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureMultiSubjectEvaluator.java +++ b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureMultiSubjectEvaluator.java @@ -941,15 +941,16 @@ private static Set getPopulationResourceKeySet( String qualifiedSubject = FhirResourceUtils.addPatientQualifier(subjectId); Set resources = entry.getValue(); if (resources != null) { - // For MEASUREOBSERVATION, resources are Map - // We need to extract the keys (input resources) + // For MEASUREOBSERVATION, resources hold ObservationAccumulator entries. + // Extract the input resource of each entry to drive stratification keys. if (populationDef.type() == MeasurePopulationType.MEASUREOBSERVATION) { // MEASUREOBSERVATION always deals with FHIR resources, so no subject qualification needed resources.stream() .filter(Objects::nonNull) - .map(CqlExpressionValue::asMap) + .map(CqlExpressionValue::asObservationAccumulator) .flatMap(java.util.Optional::stream) - .flatMap(m -> m.keySet().stream()) + .flatMap(acc -> acc.entries().stream()) + .map(ObservationEntry::inputResource) .map(MeasureMultiSubjectEvaluator::normalizePopulationKey) .filter(Objects::nonNull) .map(SubjectResourceKey::resourceOnly) diff --git a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureObservationHandler.java b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureObservationHandler.java index 1b1d3f0127..091c5665ae 100644 --- a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureObservationHandler.java +++ b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureObservationHandler.java @@ -1,6 +1,5 @@ package org.opencds.cqf.fhir.cr.measure.common; -import java.util.Map; import java.util.Set; import org.apache.commons.collections4.CollectionUtils; import org.slf4j.Logger; @@ -58,50 +57,42 @@ static void removeObservationResourcesInPopulation( final Set observationResourcesCopy = new HashSetForCqlExpressionValues(observationResources); - // Iterate over observation resources (which are Maps) and remove matching keys + // Iterate observation accumulators and drop entries whose input matches any exclusion for (CqlExpressionValue observationResource : observationResourcesCopy) { observationResource - .asMap() - .ifPresent(observationMap -> removeMatchingKeysFromObservationMap( - observationMap, exclusionResources, measureObservationDef, subjectId)); + .asObservationAccumulator() + .ifPresent(acc -> removeMatchingEntriesFromObservationAccumulator( + acc, exclusionResources, measureObservationDef, subjectId)); } } /** - * Removes keys from an observation map that match exclusion resources. + * Drops entries from an observation accumulator whose input matches an exclusion resource. *

- * This method uses FHIR resource identity (resource type + logical ID) for matching - * rather than object instance equality, since the exclusion resources and observation - * map keys may be separate Java object instances representing the same FHIR resource. - * - * @param observationMap observation map containing Resource -> QuantityDef entries - * @param exclusionResources set of resources to exclude (wrapped) - * @param measureObservationDef the observation population definition - * @param subjectId the subject ID + * Uses FHIR resource identity (resource type + logical ID) for matching rather than object + * instance equality, since the exclusion resources and observation entry inputs may be + * separate Java object instances representing the same FHIR resource. */ - private static void removeMatchingKeysFromObservationMap( - Map observationMap, + private static void removeMatchingEntriesFromObservationAccumulator( + ObservationAccumulator accumulator, Set exclusionResources, PopulationDef measureObservationDef, String subjectId) { - // Find observation map keys that match any exclusion resource for (CqlExpressionValue exclusionResource : exclusionResources) { if (exclusionResource == null) { continue; } Object exclusionRaw = exclusionResource.raw(); - // Check if this exclusion resource matches any key in the observation map - // Must use custom equality that compares FHIR resource identity, not object instance - boolean matchFound = observationMap.keySet().stream() - .anyMatch(mapKey -> FhirResourceAndCqlTypeUtils.areObjectsEqual(mapKey, exclusionRaw)); + boolean matchFound = accumulator.entries().stream() + .anyMatch( + entry -> FhirResourceAndCqlTypeUtils.areObjectsEqual(entry.inputResource(), exclusionRaw)); if (matchFound) { logger.debug( "Removing observation for excluded resource: {}", EvaluationResultFormatter.formatResource(exclusionRaw)); - // Remove the entry from the inner map using the PopulationDef's removal method - // This ensures proper handling of the Map>> structure + // Delegate to PopulationDef so empty accumulators get purged from the subject set measureObservationDef.removeExcludedMeasureObservationResource(subjectId, exclusionRaw); } } diff --git a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureReportDefScorer.java b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureReportDefScorer.java index ebe20284f1..899cfb6ba9 100644 --- a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureReportDefScorer.java +++ b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureReportDefScorer.java @@ -537,34 +537,25 @@ private static Collection getResultsForStratumByResourceIds( Set stratumResourceIds = stratumPopulationDef.resourceIdsAsSet(); - // For MEASUREOBSERVATION, subjectResources contains Set> - // MeasureScoreCalculator.collectQuantities expects Map objects and extracts values from them. - // We need to return filtered Maps (not the values directly) so collectQuantities can process them. + // For MEASUREOBSERVATION, subjectResources contains observation accumulators. Filter + // each accumulator to only the entries whose input resource ID matches a stratum + // resource ID, drop empties, and re-wrap so MeasureScoreCalculator.collectQuantities + // sees one wrapped accumulator per subject. if (populationDef.type() == MeasurePopulationType.MEASUREOBSERVATION) { return populationDef.getSubjectResources().values().stream() .flatMap(Collection::stream) .filter(Objects::nonNull) - .map(CqlExpressionValue::asMap) + .map(CqlExpressionValue::asObservationAccumulator) .flatMap(Optional::stream) - .map(map -> { - // Filter the map to only include entries matching stratum resource IDs - Map filteredMap = new java.util.HashMap<>(); - for (var entry : map.entrySet()) { - Object key = entry.getKey(); - if (key instanceof IBaseResource baseResource) { - String resourceId = baseResource - .getIdElement() - .toVersionless() - .getValue(); - if (stratumResourceIds.contains(resourceId)) { - filteredMap.put(key, entry.getValue()); - } - } - } - return filteredMap; - }) - .filter(map -> !map.isEmpty()) // Only include non-empty filtered maps - .map(filteredMap -> CqlExpressionValue.ofRaw(filteredMap, null)) + .map(acc -> acc.entries().stream() + .filter(entry -> entry.inputResource() instanceof IBaseResource baseResource + && stratumResourceIds.contains(baseResource + .getIdElement() + .toVersionless() + .getValue())) + .toList()) + .filter(entries -> !entries.isEmpty()) + .map(entries -> CqlExpressionValue.ofRaw(new ObservationAccumulator(entries), null)) .collect(Collectors.toList()); } diff --git a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureScoreCalculator.java b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureScoreCalculator.java index 18776627a0..f6d3079ec8 100644 --- a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureScoreCalculator.java +++ b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureScoreCalculator.java @@ -249,11 +249,11 @@ public static BigDecimal aggregateContinuousVariableBigDecimal( */ public static List collectQuantities(Collection resources) { return resources.stream() - .map(CqlExpressionValue::asMap) + .map(CqlExpressionValue::asObservationAccumulator) .flatMap(Optional::stream) - .flatMap(map -> map.values().stream()) - .filter(QuantityDef.class::isInstance) - .map(QuantityDef.class::cast) + .flatMap(acc -> acc.entries().stream()) + .map(ObservationEntry::observation) + .filter(Objects::nonNull) .toList(); } diff --git a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/ObservationAccumulator.java b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/ObservationAccumulator.java new file mode 100644 index 0000000000..05f85d2a3d --- /dev/null +++ b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/ObservationAccumulator.java @@ -0,0 +1,27 @@ +package org.opencds.cqf.fhir.cr.measure.common; + +import java.util.List; + +/** + * The bag of {@link ObservationEntry} produced for one subject by one MEASUREOBSERVATION + * population's observation function. Conceptually a single composite value (one accumulator per + * subject), which is why this is a record rather than a {@code List} directly: + * a List is {@link Iterable}, and the upstream evaluation pipeline ( + * {@code MeasureEvaluator.evaluatePopulationCriteria} → {@code CqlExpressionValue.asIterable}) + * unrolls Iterables when stashing values into {@code PopulationDef.subjectResources}. Wrapping in + * a non-Iterable record keeps the whole accumulator as one stored value. + */ +public record ObservationAccumulator(List entries) { + + public ObservationAccumulator { + entries = List.copyOf(entries); + } + + public boolean isEmpty() { + return entries.isEmpty(); + } + + public int size() { + return entries.size(); + } +} diff --git a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/ObservationEntry.java b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/ObservationEntry.java new file mode 100644 index 0000000000..facda615fd --- /dev/null +++ b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/ObservationEntry.java @@ -0,0 +1,21 @@ +package org.opencds.cqf.fhir.cr.measure.common; + +import jakarta.annotation.Nullable; + +/** + * One row of a {@code MEASUREOBSERVATION} accumulator: an input from the population paired with + * the {@link QuantityDef} produced by evaluating the observation function against it. + *

+ * Used in place of {@code Map} so the data flow is self-documenting, + * the value type is statically guaranteed (no {@code QuantityDef::isInstance} filtering downstream), + * and the consumer sites that previously iterated {@code map.keySet()} / {@code map.values()} / + * {@code map.entrySet()} can iterate a typed {@code List} instead. + *

+ * {@code inputResource} is typed as {@link Object} rather than {@link org.hl7.fhir.instance.model.api.IBaseResource} + * because measure-observation population basis is not constrained to FHIR resource types — + * primitive bases (Date, Integer, etc.) are valid and the input there is a CQL value, not a FHIR + * resource. Consumers handle this via the existing {@code FhirResourceAndCqlTypeUtils.areObjectsEqual} + * helper and {@code instanceof IBaseResource} guards where they need to extract resource IDs. + */ +public record ObservationEntry( + @Nullable Object inputResource, @Nullable QuantityDef observation) {} diff --git a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/PopulationDef.java b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/PopulationDef.java index b1f51b6a98..4887f751f2 100644 --- a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/PopulationDef.java +++ b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/PopulationDef.java @@ -136,16 +136,30 @@ public void removeExcludedMeasureObservationResource(String subjectId, Object me return; } - // Remove the key from all inner maps - resourcesForSubject.forEach(element -> { - if (element != null) { - element.asMap().ifPresent(innerMap -> innerMap.remove(measureObservationResourceKey)); + // Drop accumulators whose entries all match (or, after filtering, none remain). + // Each ObservationAccumulator is immutable, so we replace its containing wrapper with a + // freshly-constructed one carrying the filtered entries; if the filtered list is empty, + // we drop the wrapper entirely so the count stays correct. + Set rebuilt = new HashSetForCqlExpressionValues(); + for (CqlExpressionValue element : resourcesForSubject) { + if (element == null) { + continue; } - }); - - // Remove empty inner maps - critical for correct counting - resourcesForSubject.removeIf( - element -> element != null && element.asMap().map(Map::isEmpty).orElse(false)); + ObservationAccumulator acc = element.asObservationAccumulator().orElse(null); + if (acc == null) { + rebuilt.add(element); // not an observation accumulator, leave alone + continue; + } + List filtered = acc.entries().stream() + .filter(e -> !FhirResourceAndCqlTypeUtils.areObjectsEqual( + e.inputResource(), measureObservationResourceKey)) + .toList(); + if (!filtered.isEmpty()) { + rebuilt.add(CqlExpressionValue.ofRaw(new ObservationAccumulator(filtered), null)); + } + } + resourcesForSubject.clear(); + resourcesForSubject.addAll(rebuilt); // If the subject's resource set is now empty, remove the subject from the map entirely if (resourcesForSubject.isEmpty()) { @@ -198,9 +212,9 @@ public List getAllSubjectResources() { // Extracted from R4MeasureReportBuilder.countObservations() by Claude Sonnet 4.5 public int countObservations() { return this.getAllSubjectResources().stream() - .map(CqlExpressionValue::asMap) + .map(CqlExpressionValue::asObservationAccumulator) .flatMap(Optional::stream) - .mapToInt(Map::size) + .mapToInt(ObservationAccumulator::size) .sum(); } diff --git a/cqf-fhir-cr/src/test/java/org/opencds/cqf/fhir/cr/measure/common/CqlExpressionValueTest.java b/cqf-fhir-cr/src/test/java/org/opencds/cqf/fhir/cr/measure/common/CqlExpressionValueTest.java index a8b1c120fb..f6b86fa8e4 100644 --- a/cqf-fhir-cr/src/test/java/org/opencds/cqf/fhir/cr/measure/common/CqlExpressionValueTest.java +++ b/cqf-fhir-cr/src/test/java/org/opencds/cqf/fhir/cr/measure/common/CqlExpressionValueTest.java @@ -300,6 +300,51 @@ void asMap_emptyMapInputYieldsEmptyMap() { assertTrue(opt.get().isEmpty()); } + // -- asObservationAccumulator ------------------------------------------------ + + @Test + void asObservationAccumulator_emptyOptionalForNonAccumulatorInputs() { + assertEquals( + java.util.Optional.empty(), CqlExpressionValue.ofRaw(null, null).asObservationAccumulator()); + assertEquals( + java.util.Optional.empty(), + CqlExpressionValue.ofRaw("scalar", null).asObservationAccumulator()); + assertEquals( + java.util.Optional.empty(), + CqlExpressionValue.ofRaw(Map.of("k", "v"), null).asObservationAccumulator()); + assertEquals( + java.util.Optional.empty(), + CqlExpressionValue.ofRaw(List.of(), null).asObservationAccumulator()); + } + + @Test + void asObservationAccumulator_returnsAccumulatorWhenWrapped() { + Encounter enc = new Encounter(); + enc.setId("Encounter/1"); + ObservationAccumulator acc = + new ObservationAccumulator(List.of(new ObservationEntry(enc, new QuantityDef(42.0)))); + + java.util.Optional opt = + CqlExpressionValue.ofRaw(acc, null).asObservationAccumulator(); + + assertTrue(opt.isPresent()); + assertSame(acc, opt.get()); + assertEquals(1, opt.get().size()); + assertSame(enc, opt.get().entries().get(0).inputResource()); + } + + @Test + void asObservationAccumulator_emptyAccumulatorYieldsEmptyAccumulator() { + ObservationAccumulator empty = new ObservationAccumulator(List.of()); + + java.util.Optional opt = + CqlExpressionValue.ofRaw(empty, null).asObservationAccumulator(); + + assertTrue(opt.isPresent()); + assertTrue(opt.get().isEmpty()); + assertEquals(0, opt.get().size()); + } + // -- valueAsSet -------------------------------------------------------------- @Test diff --git a/cqf-fhir-cr/src/test/java/org/opencds/cqf/fhir/cr/measure/common/MeasureObservationHandlerTest.java b/cqf-fhir-cr/src/test/java/org/opencds/cqf/fhir/cr/measure/common/MeasureObservationHandlerTest.java index 30b3742754..36738587c9 100644 --- a/cqf-fhir-cr/src/test/java/org/opencds/cqf/fhir/cr/measure/common/MeasureObservationHandlerTest.java +++ b/cqf-fhir-cr/src/test/java/org/opencds/cqf/fhir/cr/measure/common/MeasureObservationHandlerTest.java @@ -6,13 +6,11 @@ import static org.hamcrest.Matchers.is; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertInstanceOf; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNotSame; import static org.junit.jupiter.api.Assertions.assertTrue; import java.util.List; -import java.util.Map; import java.util.Set; import org.hl7.fhir.r4.model.Encounter; import org.junit.jupiter.api.Test; @@ -58,12 +56,11 @@ void removeObservationResourcesInPopulation_removesMatchingResources_withSeparat encounter1InExclusion.getIdElement(), "Encounter IDs should be equal"); - // Create measure observation map: Map - Map observationMap1 = new HashMapForFhirResourcesAndCqlTypes<>(); - observationMap1.put(encounter1InObservation, new QuantityDef(120.0)); - - Map observationMap2 = new HashMapForFhirResourcesAndCqlTypes<>(); - observationMap2.put(encounter2InObservation, new QuantityDef(180.0)); + // Create observation accumulators + var observationMap1 = new ObservationAccumulator( + List.of(new ObservationEntry(encounter1InObservation, new QuantityDef(120.0)))); + var observationMap2 = new ObservationAccumulator( + List.of(new ObservationEntry(encounter2InObservation, new QuantityDef(180.0)))); // Create MEASUREOBSERVATION population with the maps measureObservationDef = new PopulationDef( @@ -100,14 +97,20 @@ void removeObservationResourcesInPopulation_removesMatchingResources_withSeparat assertThat( "Should have 1 observation map remaining after empty map removal", remainingObservations, hasSize(1)); - // Verify encounter-1's map was removed and only encounter-2 remains - Map remainingMap = remainingObservations.iterator().next().asMap().orElseThrow(); - assertThat("Remaining map should have 1 entry", remainingMap.size(), is(1)); - assertTrue(remainingMap.containsKey(encounter2InObservation), "Should contain encounter-2"); - var quantityFromMapForEncounter = remainingMap.get(encounter2InObservation); - assertInstanceOf(QuantityDef.class, quantityFromMapForEncounter); - var quantityFromMap = (QuantityDef) quantityFromMapForEncounter; - assertQuantityEquals(180.0, quantityFromMap, "Encounter-2 should have correct quantity"); + // Verify encounter-1's entry was removed and only encounter-2 remains + List remainingEntries = remainingObservations + .iterator() + .next() + .asObservationAccumulator() + .orElseThrow() + .entries(); + assertThat("Remaining accumulator should have 1 entry", remainingEntries, hasSize(1)); + ObservationEntry remainingEntry = remainingEntries.get(0); + assertTrue( + FhirResourceAndCqlTypeUtils.areObjectsEqual(remainingEntry.inputResource(), encounter2InObservation), + "Should contain encounter-2"); + assertNotNull(remainingEntry.observation()); + assertQuantityEquals(180.0, remainingEntry.observation(), "Encounter-2 should have correct quantity"); } /** @@ -123,15 +126,10 @@ void removeObservationResourcesInPopulation_removesMultipleMatchingResources() { Encounter enc1Excl = createEncounter("encounter-1"); Encounter enc2Excl = createEncounter("encounter-2"); - // Create observation maps - Map obsMap1 = new HashMapForFhirResourcesAndCqlTypes<>(); - obsMap1.put(enc1Obs, new QuantityDef(100.0)); - - Map obsMap2 = new HashMapForFhirResourcesAndCqlTypes<>(); - obsMap2.put(enc2Obs, new QuantityDef(200.0)); - - Map obsMap3 = new HashMapForFhirResourcesAndCqlTypes<>(); - obsMap3.put(enc3Obs, new QuantityDef(300.0)); + // Create observation accumulators + var obsMap1 = new ObservationAccumulator(List.of(new ObservationEntry(enc1Obs, new QuantityDef(100.0)))); + var obsMap2 = new ObservationAccumulator(List.of(new ObservationEntry(enc2Obs, new QuantityDef(200.0)))); + var obsMap3 = new ObservationAccumulator(List.of(new ObservationEntry(enc3Obs, new QuantityDef(300.0)))); measureObservationDef = createMeasureObservationDef(); measureObservationDef.addResource(SUBJECT_ID_1, obsMap1); @@ -152,14 +150,19 @@ void removeObservationResourcesInPopulation_removesMultipleMatchingResources() { assertThat("Should have 1 observation map remaining", remainingObservations, hasSize(1)); // Verify only enc3 remains - Map remainingMap = remainingObservations.iterator().next().asMap().orElseThrow(); - assertThat("Map should have 1 entry", remainingMap.size(), is(1)); - assertTrue(remainingMap.containsKey(enc3Obs), "Should contain encounter-3"); - var quantityFromMapForEncounter = remainingMap.get(enc3Obs); - assertInstanceOf(QuantityDef.class, quantityFromMapForEncounter); - var quantityFromMap = (QuantityDef) quantityFromMapForEncounter; - - assertQuantityEquals(300.0, quantityFromMap, "Encounter-3 should have correct quantity"); + List remainingEntries = remainingObservations + .iterator() + .next() + .asObservationAccumulator() + .orElseThrow() + .entries(); + assertThat("Accumulator should have 1 entry", remainingEntries, hasSize(1)); + ObservationEntry remainingEntry = remainingEntries.get(0); + assertTrue( + FhirResourceAndCqlTypeUtils.areObjectsEqual(remainingEntry.inputResource(), enc3Obs), + "Should contain encounter-3"); + assertNotNull(remainingEntry.observation()); + assertQuantityEquals(300.0, remainingEntry.observation(), "Encounter-3 should have correct quantity"); } /** @@ -173,11 +176,8 @@ void removeObservationResourcesInPopulation_noMatchingExclusions_allObservations Encounter enc3Excl = createEncounter("encounter-3"); Encounter enc4Excl = createEncounter("encounter-4"); - Map obsMap1 = new HashMapForFhirResourcesAndCqlTypes<>(); - obsMap1.put(enc1Obs, new QuantityDef(100.0)); - - Map obsMap2 = new HashMapForFhirResourcesAndCqlTypes<>(); - obsMap2.put(enc2Obs, new QuantityDef(200.0)); + var obsMap1 = new ObservationAccumulator(List.of(new ObservationEntry(enc1Obs, new QuantityDef(100.0)))); + var obsMap2 = new ObservationAccumulator(List.of(new ObservationEntry(enc2Obs, new QuantityDef(200.0)))); measureObservationDef = createMeasureObservationDef(); measureObservationDef.addResource(SUBJECT_ID_1, obsMap1); @@ -203,8 +203,7 @@ void removeObservationResourcesInPopulation_noMatchingExclusions_allObservations void removeObservationResourcesInPopulation_emptyExclusions_allObservationsRemain() { // Given: Observations but no exclusions Encounter enc1 = createEncounter("encounter-1"); - Map obsMap1 = new HashMapForFhirResourcesAndCqlTypes<>(); - obsMap1.put(enc1, new QuantityDef(100.0)); + var obsMap1 = new ObservationAccumulator(List.of(new ObservationEntry(enc1, new QuantityDef(100.0)))); measureObservationDef = createMeasureObservationDef(); measureObservationDef.addResource(SUBJECT_ID_1, obsMap1); @@ -242,8 +241,8 @@ void removeObservationResourcesInPopulation_nullMeasureObservation_noExceptionTh void removeObservationResourcesInPopulation_nullExclusionDef_noExceptionThrown() { // Given: Null exclusion def measureObservationDef = createMeasureObservationDef(); - Map obsMap1 = new HashMapForFhirResourcesAndCqlTypes<>(); - obsMap1.put(createEncounter("encounter-1"), new QuantityDef(100.0)); + var obsMap1 = new ObservationAccumulator( + List.of(new ObservationEntry(createEncounter("encounter-1"), new QuantityDef(100.0)))); measureObservationDef.addResource(SUBJECT_ID_1, obsMap1); // When/Then: Should not throw exception, all observations remain @@ -260,8 +259,7 @@ void removeObservationResourcesInPopulation_nullExclusionDef_noExceptionThrown() void removeObservationResourcesInPopulation_subjectNotInExclusions_allObservationsRemain() { // Given: Observations for subject-1, exclusions for different subject Encounter enc1 = createEncounter("encounter-1"); - Map obsMap1 = new HashMapForFhirResourcesAndCqlTypes<>(); - obsMap1.put(enc1, new QuantityDef(100.0)); + var obsMap1 = new ObservationAccumulator(List.of(new ObservationEntry(enc1, new QuantityDef(100.0)))); measureObservationDef = createMeasureObservationDef(); measureObservationDef.addResource(SUBJECT_ID_1, obsMap1); @@ -292,12 +290,11 @@ void removeObservationResourcesInPopulation_mixedCancelledAndNonCancelled_onlyCa Encounter cancelledEncExcl = createEncounter("patient-3-encounter-cancelled"); - // Create observation maps for both encounters - Map obsMapCancelled = new HashMapForFhirResourcesAndCqlTypes<>(); - obsMapCancelled.put(cancelledEncObs, new QuantityDef(100.0)); - - Map obsMapActive = new HashMapForFhirResourcesAndCqlTypes<>(); - obsMapActive.put(nonCancelledEncObs, new QuantityDef(420.0)); + // Create observation accumulators for both encounters + var obsMapCancelled = + new ObservationAccumulator(List.of(new ObservationEntry(cancelledEncObs, new QuantityDef(100.0)))); + var obsMapActive = + new ObservationAccumulator(List.of(new ObservationEntry(nonCancelledEncObs, new QuantityDef(420.0)))); measureObservationDef = createMeasureObservationDef(); measureObservationDef.addResource(SUBJECT_ID_3, obsMapCancelled); @@ -316,10 +313,18 @@ void removeObservationResourcesInPopulation_mixedCancelledAndNonCancelled_onlyCa assertThat("Should have 1 observation map remaining", remainingObservations, hasSize(1)); // Verify only the non-cancelled encounter remains - Map remainingMap = remainingObservations.iterator().next().asMap().orElseThrow(); - assertThat("Map should have 1 entry", remainingMap.size(), is(1)); - assertTrue(remainingMap.containsKey(nonCancelledEncObs), "Should contain non-cancelled encounter"); - QuantityDef activeQuantity = (QuantityDef) remainingMap.get(nonCancelledEncObs); + List remainingEntries = remainingObservations + .iterator() + .next() + .asObservationAccumulator() + .orElseThrow() + .entries(); + assertThat("Accumulator should have 1 entry", remainingEntries, hasSize(1)); + ObservationEntry remainingEntry = remainingEntries.get(0); + assertTrue( + FhirResourceAndCqlTypeUtils.areObjectsEqual(remainingEntry.inputResource(), nonCancelledEncObs), + "Should contain non-cancelled encounter"); + QuantityDef activeQuantity = remainingEntry.observation(); assertNotNull(activeQuantity); assertThat("Active encounter should have correct quantity", activeQuantity.value(), is(closeTo(420.0, 0.01))); } @@ -334,8 +339,7 @@ void removeObservationResourcesInPopulation_allEntriesRemoved_subjectNotCounted( Encounter enc1Obs = createEncounter("encounter-1"); Encounter enc1Excl = createEncounter("encounter-1"); - Map obsMap1 = new HashMapForFhirResourcesAndCqlTypes<>(); - obsMap1.put(enc1Obs, new QuantityDef(100.0)); + var obsMap1 = new ObservationAccumulator(List.of(new ObservationEntry(enc1Obs, new QuantityDef(100.0)))); measureObservationDef = createMeasureObservationDef(); measureObservationDef.addResource(SUBJECT_ID_1, obsMap1); @@ -371,9 +375,9 @@ void removeObservationResourcesInPopulation_partialRemoval_subjectStillCounted() Encounter enc2 = createEncounter("encounter-2"); Encounter enc1Excl = createEncounter("encounter-1"); - Map obsMap = new HashMapForFhirResourcesAndCqlTypes<>(); - obsMap.put(enc1, new QuantityDef(100.0)); - obsMap.put(enc2, new QuantityDef(200.0)); + var obsMap = new ObservationAccumulator(List.of( + new ObservationEntry(enc1, new QuantityDef(100.0)), + new ObservationEntry(enc2, new QuantityDef(200.0)))); measureObservationDef = createMeasureObservationDef(); measureObservationDef.addResource(SUBJECT_ID_1, obsMap); @@ -406,20 +410,18 @@ void removeObservationResourcesInPopulation_multipleSubjects_countsCorrectly() { // Given: Three subjects with different exclusion scenarios // Subject 1: All entries excluded (1 encounter) Encounter enc1 = createEncounter("encounter-1"); - Map obsMap1 = new HashMapForFhirResourcesAndCqlTypes<>(); - obsMap1.put(enc1, new QuantityDef(100.0)); + var obsMap1 = new ObservationAccumulator(List.of(new ObservationEntry(enc1, new QuantityDef(100.0)))); // Subject 2: Partial exclusion (2 encounters, 1 excluded) Encounter enc2 = createEncounter("encounter-2"); Encounter enc3 = createEncounter("encounter-3"); - Map obsMap2 = new HashMapForFhirResourcesAndCqlTypes<>(); - obsMap2.put(enc2, new QuantityDef(200.0)); - obsMap2.put(enc3, new QuantityDef(300.0)); + var obsMap2 = new ObservationAccumulator(List.of( + new ObservationEntry(enc2, new QuantityDef(200.0)), + new ObservationEntry(enc3, new QuantityDef(300.0)))); // Subject 3: No exclusions (1 encounter) Encounter enc4 = createEncounter("encounter-4"); - Map obsMap3 = new HashMapForFhirResourcesAndCqlTypes<>(); - obsMap3.put(enc4, new QuantityDef(400.0)); + var obsMap3 = new ObservationAccumulator(List.of(new ObservationEntry(enc4, new QuantityDef(400.0)))); measureObservationDef = createMeasureObservationDef(); measureObservationDef.addResource(SUBJECT_ID_1, obsMap1); diff --git a/cqf-fhir-cr/src/test/java/org/opencds/cqf/fhir/cr/measure/common/MeasureReportDefScorerTest.java b/cqf-fhir-cr/src/test/java/org/opencds/cqf/fhir/cr/measure/common/MeasureReportDefScorerTest.java index 984fdfaf0e..254fc13151 100644 --- a/cqf-fhir-cr/src/test/java/org/opencds/cqf/fhir/cr/measure/common/MeasureReportDefScorerTest.java +++ b/cqf-fhir-cr/src/test/java/org/opencds/cqf/fhir/cr/measure/common/MeasureReportDefScorerTest.java @@ -2,9 +2,7 @@ import static org.junit.jupiter.api.Assertions.*; -import java.util.HashMap; import java.util.List; -import java.util.Map; import java.util.Set; import org.junit.jupiter.api.Test; import org.opencds.cqf.fhir.cr.measure.MeasureStratifierType; @@ -299,16 +297,13 @@ void testScoreGroup_ContinuousVariable_SumAggregation() { null); // Add QuantityDef observations for each subject - Map obs1 = new HashMap<>(); - obs1.put("obs-1", new QuantityDef(10.0)); + var obs1 = new ObservationAccumulator(List.of(new ObservationEntry("obs-1", new QuantityDef(10.0)))); measureObsPop.addResource("p1", obs1); - Map obs2 = new HashMap<>(); - obs2.put("obs-2", new QuantityDef(20.0)); + var obs2 = new ObservationAccumulator(List.of(new ObservationEntry("obs-2", new QuantityDef(20.0)))); measureObsPop.addResource("p2", obs2); - Map obs3 = new HashMap<>(); - obs3.put("obs-3", new QuantityDef(30.0)); + var obs3 = new ObservationAccumulator(List.of(new ObservationEntry("obs-3", new QuantityDef(30.0)))); measureObsPop.addResource("p3", obs3); GroupDef groupDef = new GroupDef( @@ -359,16 +354,13 @@ void testScoreGroup_ContinuousVariable_AvgAggregation() { ContinuousVariableObservationAggregateMethod.AVG, null); - Map obs1 = new HashMap<>(); - obs1.put("obs-1", new QuantityDef(10.0)); + var obs1 = new ObservationAccumulator(List.of(new ObservationEntry("obs-1", new QuantityDef(10.0)))); measureObsPop.addResource("p1", obs1); - Map obs2 = new HashMap<>(); - obs2.put("obs-2", new QuantityDef(20.0)); + var obs2 = new ObservationAccumulator(List.of(new ObservationEntry("obs-2", new QuantityDef(20.0)))); measureObsPop.addResource("p2", obs2); - Map obs3 = new HashMap<>(); - obs3.put("obs-3", new QuantityDef(30.0)); + var obs3 = new ObservationAccumulator(List.of(new ObservationEntry("obs-3", new QuantityDef(30.0)))); measureObsPop.addResource("p3", obs3); GroupDef groupDef = new GroupDef( @@ -419,16 +411,13 @@ void testScoreGroup_ContinuousVariable_MinAggregation() { ContinuousVariableObservationAggregateMethod.MIN, null); - Map obs1 = new HashMap<>(); - obs1.put("obs-1", new QuantityDef(10.0)); + var obs1 = new ObservationAccumulator(List.of(new ObservationEntry("obs-1", new QuantityDef(10.0)))); measureObsPop.addResource("p1", obs1); - Map obs2 = new HashMap<>(); - obs2.put("obs-2", new QuantityDef(20.0)); + var obs2 = new ObservationAccumulator(List.of(new ObservationEntry("obs-2", new QuantityDef(20.0)))); measureObsPop.addResource("p2", obs2); - Map obs3 = new HashMap<>(); - obs3.put("obs-3", new QuantityDef(30.0)); + var obs3 = new ObservationAccumulator(List.of(new ObservationEntry("obs-3", new QuantityDef(30.0)))); measureObsPop.addResource("p3", obs3); GroupDef groupDef = new GroupDef( @@ -479,16 +468,13 @@ void testScoreGroup_ContinuousVariable_MaxAggregation() { ContinuousVariableObservationAggregateMethod.MAX, null); - Map obs1 = new HashMap<>(); - obs1.put("obs-1", new QuantityDef(10.0)); + var obs1 = new ObservationAccumulator(List.of(new ObservationEntry("obs-1", new QuantityDef(10.0)))); measureObsPop.addResource("p1", obs1); - Map obs2 = new HashMap<>(); - obs2.put("obs-2", new QuantityDef(20.0)); + var obs2 = new ObservationAccumulator(List.of(new ObservationEntry("obs-2", new QuantityDef(20.0)))); measureObsPop.addResource("p2", obs2); - Map obs3 = new HashMap<>(); - obs3.put("obs-3", new QuantityDef(30.0)); + var obs3 = new ObservationAccumulator(List.of(new ObservationEntry("obs-3", new QuantityDef(30.0)))); measureObsPop.addResource("p3", obs3); GroupDef groupDef = new GroupDef( @@ -932,16 +918,13 @@ void testScoreGroup_RatioWithObservations_GroupLevel() { null); // Add numerator observations - Map numObs1 = new HashMap<>(); - numObs1.put("obs-num-1", new QuantityDef(10.0)); + var numObs1 = new ObservationAccumulator(List.of(new ObservationEntry("obs-num-1", new QuantityDef(10.0)))); numeratorMeasureObs.addResource("p1", numObs1); - Map numObs2 = new HashMap<>(); - numObs2.put("obs-num-2", new QuantityDef(20.0)); + var numObs2 = new ObservationAccumulator(List.of(new ObservationEntry("obs-num-2", new QuantityDef(20.0)))); numeratorMeasureObs.addResource("p2", numObs2); - Map numObs3 = new HashMap<>(); - numObs3.put("obs-num-3", new QuantityDef(30.0)); + var numObs3 = new ObservationAccumulator(List.of(new ObservationEntry("obs-num-3", new QuantityDef(30.0)))); numeratorMeasureObs.addResource("p3", numObs3); // Create denominator MEASUREOBSERVATION with criteriaReference to denominator @@ -957,16 +940,13 @@ void testScoreGroup_RatioWithObservations_GroupLevel() { null); // Add denominator observations - Map denObs1 = new HashMap<>(); - denObs1.put("obs-den-1", new QuantityDef(5.0)); + var denObs1 = new ObservationAccumulator(List.of(new ObservationEntry("obs-den-1", new QuantityDef(5.0)))); denominatorMeasureObs.addResource("p1", denObs1); - Map denObs2 = new HashMap<>(); - denObs2.put("obs-den-2", new QuantityDef(10.0)); + var denObs2 = new ObservationAccumulator(List.of(new ObservationEntry("obs-den-2", new QuantityDef(10.0)))); denominatorMeasureObs.addResource("p2", denObs2); - Map denObs3 = new HashMap<>(); - denObs3.put("obs-den-3", new QuantityDef(15.0)); + var denObs3 = new ObservationAccumulator(List.of(new ObservationEntry("obs-den-3", new QuantityDef(15.0)))); denominatorMeasureObs.addResource("p3", denObs3); // Create GroupDef with RATIO scoring @@ -1092,16 +1072,13 @@ void testScoreGroup_RatioWithObservations_NullNumeratorPopulation() { null); // Add denominator observations - Map denObs1 = new HashMap<>(); - denObs1.put("obs-den-1", new QuantityDef(5.0)); + var denObs1 = new ObservationAccumulator(List.of(new ObservationEntry("obs-den-1", new QuantityDef(5.0)))); denominatorMeasureObs.addResource("p1", denObs1); - Map denObs2 = new HashMap<>(); - denObs2.put("obs-den-2", new QuantityDef(10.0)); + var denObs2 = new ObservationAccumulator(List.of(new ObservationEntry("obs-den-2", new QuantityDef(10.0)))); denominatorMeasureObs.addResource("p2", denObs2); - Map denObs3 = new HashMap<>(); - denObs3.put("obs-den-3", new QuantityDef(15.0)); + var denObs3 = new ObservationAccumulator(List.of(new ObservationEntry("obs-den-3", new QuantityDef(15.0)))); denominatorMeasureObs.addResource("p3", denObs3); GroupDef groupDef = new GroupDef( @@ -1151,16 +1128,13 @@ void testScoreGroup_RatioWithObservations_NullDenominatorPopulation() { null); // Add numerator observations - Map numObs1 = new HashMap<>(); - numObs1.put("obs-num-1", new QuantityDef(10.0)); + var numObs1 = new ObservationAccumulator(List.of(new ObservationEntry("obs-num-1", new QuantityDef(10.0)))); numeratorMeasureObs.addResource("p1", numObs1); - Map numObs2 = new HashMap<>(); - numObs2.put("obs-num-2", new QuantityDef(20.0)); + var numObs2 = new ObservationAccumulator(List.of(new ObservationEntry("obs-num-2", new QuantityDef(20.0)))); numeratorMeasureObs.addResource("p2", numObs2); - Map numObs3 = new HashMap<>(); - numObs3.put("obs-num-3", new QuantityDef(30.0)); + var numObs3 = new ObservationAccumulator(List.of(new ObservationEntry("obs-num-3", new QuantityDef(30.0)))); numeratorMeasureObs.addResource("p3", numObs3); GroupDef groupDef = new GroupDef( @@ -1264,8 +1238,7 @@ void testScoreGroup_RatioWithObservations_NullAggregateForNumerator() { ContinuousVariableObservationAggregateMethod.SUM, null); - Map denObs1 = new HashMap<>(); - denObs1.put("obs-den-1", new QuantityDef(10.0)); + var denObs1 = new ObservationAccumulator(List.of(new ObservationEntry("obs-den-1", new QuantityDef(10.0)))); denominatorMeasureObs.addResource("p1", denObs1); GroupDef groupDef = new GroupDef( @@ -1320,16 +1293,13 @@ void testScoreGroup_RatioWithObservations_AvgAggregation() { ContinuousVariableObservationAggregateMethod.AVG, null); // AVG instead of SUM - Map numObs1 = new HashMap<>(); - numObs1.put("obs-num-1", new QuantityDef(10.0)); + var numObs1 = new ObservationAccumulator(List.of(new ObservationEntry("obs-num-1", new QuantityDef(10.0)))); numeratorMeasureObs.addResource("p1", numObs1); - Map numObs2 = new HashMap<>(); - numObs2.put("obs-num-2", new QuantityDef(20.0)); + var numObs2 = new ObservationAccumulator(List.of(new ObservationEntry("obs-num-2", new QuantityDef(20.0)))); numeratorMeasureObs.addResource("p2", numObs2); - Map numObs3 = new HashMap<>(); - numObs3.put("obs-num-3", new QuantityDef(30.0)); + var numObs3 = new ObservationAccumulator(List.of(new ObservationEntry("obs-num-3", new QuantityDef(30.0)))); numeratorMeasureObs.addResource("p3", numObs3); // Create denominator MEASUREOBSERVATION with AVG aggregation @@ -1344,16 +1314,13 @@ void testScoreGroup_RatioWithObservations_AvgAggregation() { ContinuousVariableObservationAggregateMethod.AVG, null); // AVG instead of SUM - Map denObs1 = new HashMap<>(); - denObs1.put("obs-den-1", new QuantityDef(5.0)); + var denObs1 = new ObservationAccumulator(List.of(new ObservationEntry("obs-den-1", new QuantityDef(5.0)))); denominatorMeasureObs.addResource("p1", denObs1); - Map denObs2 = new HashMap<>(); - denObs2.put("obs-den-2", new QuantityDef(10.0)); + var denObs2 = new ObservationAccumulator(List.of(new ObservationEntry("obs-den-2", new QuantityDef(10.0)))); denominatorMeasureObs.addResource("p2", denObs2); - Map denObs3 = new HashMap<>(); - denObs3.put("obs-den-3", new QuantityDef(15.0)); + var denObs3 = new ObservationAccumulator(List.of(new ObservationEntry("obs-den-3", new QuantityDef(15.0)))); denominatorMeasureObs.addResource("p3", denObs3); GroupDef groupDef = new GroupDef( @@ -1424,25 +1391,20 @@ void testScoreStratifier_RatioWithObservations_StratumLevel() { null); // Male numerator observations: 10 + 15 + 15 = 40 - Map numObs1 = new HashMap<>(); - numObs1.put("p1", new QuantityDef(10.0)); + var numObs1 = new ObservationAccumulator(List.of(new ObservationEntry("p1", new QuantityDef(10.0)))); numeratorMeasureObs.addResource("p1", numObs1); - Map numObs2 = new HashMap<>(); - numObs2.put("p2", new QuantityDef(15.0)); + var numObs2 = new ObservationAccumulator(List.of(new ObservationEntry("p2", new QuantityDef(15.0)))); numeratorMeasureObs.addResource("p2", numObs2); - Map numObs3 = new HashMap<>(); - numObs3.put("p3", new QuantityDef(15.0)); + var numObs3 = new ObservationAccumulator(List.of(new ObservationEntry("p3", new QuantityDef(15.0)))); numeratorMeasureObs.addResource("p3", numObs3); // Female numerator observations: 10 + 20 = 30 - Map numObs4 = new HashMap<>(); - numObs4.put("p4", new QuantityDef(10.0)); + var numObs4 = new ObservationAccumulator(List.of(new ObservationEntry("p4", new QuantityDef(10.0)))); numeratorMeasureObs.addResource("p4", numObs4); - Map numObs5 = new HashMap<>(); - numObs5.put("p5", new QuantityDef(20.0)); + var numObs5 = new ObservationAccumulator(List.of(new ObservationEntry("p5", new QuantityDef(20.0)))); numeratorMeasureObs.addResource("p5", numObs5); // Create denominator MEASUREOBSERVATION @@ -1458,25 +1420,20 @@ void testScoreStratifier_RatioWithObservations_StratumLevel() { null); // Male denominator observations: 5 + 7 + 8 = 20 - Map denObs1 = new HashMap<>(); - denObs1.put("p1", new QuantityDef(5.0)); + var denObs1 = new ObservationAccumulator(List.of(new ObservationEntry("p1", new QuantityDef(5.0)))); denominatorMeasureObs.addResource("p1", denObs1); - Map denObs2 = new HashMap<>(); - denObs2.put("p2", new QuantityDef(7.0)); + var denObs2 = new ObservationAccumulator(List.of(new ObservationEntry("p2", new QuantityDef(7.0)))); denominatorMeasureObs.addResource("p2", denObs2); - Map denObs3 = new HashMap<>(); - denObs3.put("p3", new QuantityDef(8.0)); + var denObs3 = new ObservationAccumulator(List.of(new ObservationEntry("p3", new QuantityDef(8.0)))); denominatorMeasureObs.addResource("p3", denObs3); // Female denominator observations: 4 + 6 = 10 - Map denObs4 = new HashMap<>(); - denObs4.put("p4", new QuantityDef(4.0)); + var denObs4 = new ObservationAccumulator(List.of(new ObservationEntry("p4", new QuantityDef(4.0)))); denominatorMeasureObs.addResource("p4", denObs4); - Map denObs5 = new HashMap<>(); - denObs5.put("p5", new QuantityDef(6.0)); + var denObs5 = new ObservationAccumulator(List.of(new ObservationEntry("p5", new QuantityDef(6.0)))); denominatorMeasureObs.addResource("p5", denObs5); // Create stratum populations for Male stratum @@ -1610,20 +1567,15 @@ void testScoreStratifier_RatioWithObservations_PerStratumAggregationResults() { ContinuousVariableObservationAggregateMethod.SUM, null); - Map numObs1 = new HashMap<>(); - numObs1.put("p1", new QuantityDef(10.0)); + var numObs1 = new ObservationAccumulator(List.of(new ObservationEntry("p1", new QuantityDef(10.0)))); numeratorMeasureObs.addResource("p1", numObs1); - Map numObs2 = new HashMap<>(); - numObs2.put("p2", new QuantityDef(15.0)); + var numObs2 = new ObservationAccumulator(List.of(new ObservationEntry("p2", new QuantityDef(15.0)))); numeratorMeasureObs.addResource("p2", numObs2); - Map numObs3 = new HashMap<>(); - numObs3.put("p3", new QuantityDef(15.0)); + var numObs3 = new ObservationAccumulator(List.of(new ObservationEntry("p3", new QuantityDef(15.0)))); numeratorMeasureObs.addResource("p3", numObs3); - Map numObs4 = new HashMap<>(); - numObs4.put("p4", new QuantityDef(10.0)); + var numObs4 = new ObservationAccumulator(List.of(new ObservationEntry("p4", new QuantityDef(10.0)))); numeratorMeasureObs.addResource("p4", numObs4); - Map numObs5 = new HashMap<>(); - numObs5.put("p5", new QuantityDef(20.0)); + var numObs5 = new ObservationAccumulator(List.of(new ObservationEntry("p5", new QuantityDef(20.0)))); numeratorMeasureObs.addResource("p5", numObs5); // Denominator MEASUREOBSERVATION with SUM @@ -1638,20 +1590,15 @@ void testScoreStratifier_RatioWithObservations_PerStratumAggregationResults() { ContinuousVariableObservationAggregateMethod.SUM, null); - Map denObs1 = new HashMap<>(); - denObs1.put("p1", new QuantityDef(5.0)); + var denObs1 = new ObservationAccumulator(List.of(new ObservationEntry("p1", new QuantityDef(5.0)))); denominatorMeasureObs.addResource("p1", denObs1); - Map denObs2 = new HashMap<>(); - denObs2.put("p2", new QuantityDef(7.0)); + var denObs2 = new ObservationAccumulator(List.of(new ObservationEntry("p2", new QuantityDef(7.0)))); denominatorMeasureObs.addResource("p2", denObs2); - Map denObs3 = new HashMap<>(); - denObs3.put("p3", new QuantityDef(8.0)); + var denObs3 = new ObservationAccumulator(List.of(new ObservationEntry("p3", new QuantityDef(8.0)))); denominatorMeasureObs.addResource("p3", denObs3); - Map denObs4 = new HashMap<>(); - denObs4.put("p4", new QuantityDef(4.0)); + var denObs4 = new ObservationAccumulator(List.of(new ObservationEntry("p4", new QuantityDef(4.0)))); denominatorMeasureObs.addResource("p4", denObs4); - Map denObs5 = new HashMap<>(); - denObs5.put("p5", new QuantityDef(6.0)); + var denObs5 = new ObservationAccumulator(List.of(new ObservationEntry("p5", new QuantityDef(6.0)))); denominatorMeasureObs.addResource("p5", denObs5); // Male stratum populations @@ -1768,22 +1715,17 @@ void testScoreStratifier_ContinuousVariable_PerStratumAggregationResults() { null); // Male observations: 10, 20, 30 - Map obs1 = new HashMap<>(); - obs1.put("obs-1", new QuantityDef(10.0)); + var obs1 = new ObservationAccumulator(List.of(new ObservationEntry("obs-1", new QuantityDef(10.0)))); measureObsPop.addResource("p1", obs1); - Map obs2 = new HashMap<>(); - obs2.put("obs-2", new QuantityDef(20.0)); + var obs2 = new ObservationAccumulator(List.of(new ObservationEntry("obs-2", new QuantityDef(20.0)))); measureObsPop.addResource("p2", obs2); - Map obs3 = new HashMap<>(); - obs3.put("obs-3", new QuantityDef(30.0)); + var obs3 = new ObservationAccumulator(List.of(new ObservationEntry("obs-3", new QuantityDef(30.0)))); measureObsPop.addResource("p3", obs3); // Female observations: 5, 15 - Map obs4 = new HashMap<>(); - obs4.put("obs-4", new QuantityDef(5.0)); + var obs4 = new ObservationAccumulator(List.of(new ObservationEntry("obs-4", new QuantityDef(5.0)))); measureObsPop.addResource("p4", obs4); - Map obs5 = new HashMap<>(); - obs5.put("obs-5", new QuantityDef(15.0)); + var obs5 = new ObservationAccumulator(List.of(new ObservationEntry("obs-5", new QuantityDef(15.0)))); measureObsPop.addResource("p5", obs5); // Male stratum @@ -2189,24 +2131,19 @@ void testScoreGroup_ContinuousVariable_MedianAggregation() { null); // Add observations in non-sorted order - Map obs1 = new HashMap<>(); - obs1.put("obs-1", new QuantityDef(20.0)); + var obs1 = new ObservationAccumulator(List.of(new ObservationEntry("obs-1", new QuantityDef(20.0)))); measureObsPop.addResource("p1", obs1); - Map obs2 = new HashMap<>(); - obs2.put("obs-2", new QuantityDef(5.0)); + var obs2 = new ObservationAccumulator(List.of(new ObservationEntry("obs-2", new QuantityDef(5.0)))); measureObsPop.addResource("p2", obs2); - Map obs3 = new HashMap<>(); - obs3.put("obs-3", new QuantityDef(25.0)); + var obs3 = new ObservationAccumulator(List.of(new ObservationEntry("obs-3", new QuantityDef(25.0)))); measureObsPop.addResource("p3", obs3); - Map obs4 = new HashMap<>(); - obs4.put("obs-4", new QuantityDef(10.0)); + var obs4 = new ObservationAccumulator(List.of(new ObservationEntry("obs-4", new QuantityDef(10.0)))); measureObsPop.addResource("p4", obs4); - Map obs5 = new HashMap<>(); - obs5.put("obs-5", new QuantityDef(15.0)); + var obs5 = new ObservationAccumulator(List.of(new ObservationEntry("obs-5", new QuantityDef(15.0)))); measureObsPop.addResource("p5", obs5); GroupDef groupDef = new GroupDef( @@ -2259,18 +2196,18 @@ void testScoreGroup_ContinuousVariable_CountAggregation() { null); // Patient 1 has 4 observations - Map obs1 = new HashMap<>(); - obs1.put("obs-1", new QuantityDef(10.0)); - obs1.put("obs-2", new QuantityDef(20.0)); - obs1.put("obs-3", new QuantityDef(30.0)); - obs1.put("obs-4", new QuantityDef(40.0)); + var obs1 = new ObservationAccumulator(List.of( + new ObservationEntry("obs-1", new QuantityDef(10.0)), + new ObservationEntry("obs-2", new QuantityDef(20.0)), + new ObservationEntry("obs-3", new QuantityDef(30.0)), + new ObservationEntry("obs-4", new QuantityDef(40.0)))); measureObsPop.addResource("p1", obs1); // Patient 2 has 3 observations - Map obs2 = new HashMap<>(); - obs2.put("obs-5", new QuantityDef(50.0)); - obs2.put("obs-6", new QuantityDef(60.0)); - obs2.put("obs-7", new QuantityDef(70.0)); + var obs2 = new ObservationAccumulator(List.of( + new ObservationEntry("obs-5", new QuantityDef(50.0)), + new ObservationEntry("obs-6", new QuantityDef(60.0)), + new ObservationEntry("obs-7", new QuantityDef(70.0)))); measureObsPop.addResource("p2", obs2); GroupDef groupDef = new GroupDef( @@ -2333,11 +2270,9 @@ void testScoreStratifier_RatioWithObservations_PartialNullStratum_NumeratorOnly( ContinuousVariableObservationAggregateMethod.SUM, null); - Map numObs1 = new HashMap<>(); - numObs1.put("p1", new QuantityDef(10.0)); + var numObs1 = new ObservationAccumulator(List.of(new ObservationEntry("p1", new QuantityDef(10.0)))); numeratorMeasureObs.addResource("p1", numObs1); - Map numObs2 = new HashMap<>(); - numObs2.put("p2", new QuantityDef(20.0)); + var numObs2 = new ObservationAccumulator(List.of(new ObservationEntry("p2", new QuantityDef(20.0)))); numeratorMeasureObs.addResource("p2", numObs2); // Denominator MEASUREOBSERVATION with NO observations for p1 or p2 @@ -2354,8 +2289,7 @@ void testScoreStratifier_RatioWithObservations_PartialNullStratum_NumeratorOnly( null); // Only add denominator obs for p3 (NOT in this stratum) - Map denObs3 = new HashMap<>(); - denObs3.put("p3", new QuantityDef(50.0)); + var denObs3 = new ObservationAccumulator(List.of(new ObservationEntry("p3", new QuantityDef(50.0)))); denominatorMeasureObs.addResource("p3", denObs3); // Stratum populations: subjects p1, p2 @@ -2449,8 +2383,7 @@ void testScoreStratifier_RatioWithObservations_PartialNullStratum_DenominatorOnl null); // Only add numerator obs for p3 (NOT in this stratum) - Map numObs3 = new HashMap<>(); - numObs3.put("p3", new QuantityDef(99.0)); + var numObs3 = new ObservationAccumulator(List.of(new ObservationEntry("p3", new QuantityDef(99.0)))); numeratorMeasureObs.addResource("p3", numObs3); // Denominator MEASUREOBSERVATION with observations for p1 and p2 @@ -2465,11 +2398,9 @@ void testScoreStratifier_RatioWithObservations_PartialNullStratum_DenominatorOnl ContinuousVariableObservationAggregateMethod.SUM, null); - Map denObs1 = new HashMap<>(); - denObs1.put("p1", new QuantityDef(15.0)); + var denObs1 = new ObservationAccumulator(List.of(new ObservationEntry("p1", new QuantityDef(15.0)))); denominatorMeasureObs.addResource("p1", denObs1); - Map denObs2 = new HashMap<>(); - denObs2.put("p2", new QuantityDef(25.0)); + var denObs2 = new ObservationAccumulator(List.of(new ObservationEntry("p2", new QuantityDef(25.0)))); denominatorMeasureObs.addResource("p2", denObs2); // Stratum populations: subjects p1, p2 @@ -2561,8 +2492,7 @@ void testScoreStratifier_RatioWithObservations_PartialNullStratum_BothNull() { null); // Only add numerator obs for p3 (NOT in stratum) - Map numObs3 = new HashMap<>(); - numObs3.put("p3", new QuantityDef(99.0)); + var numObs3 = new ObservationAccumulator(List.of(new ObservationEntry("p3", new QuantityDef(99.0)))); numeratorMeasureObs.addResource("p3", numObs3); // Denominator MEASUREOBSERVATION with NO observations for stratum subjects @@ -2578,8 +2508,7 @@ void testScoreStratifier_RatioWithObservations_PartialNullStratum_BothNull() { null); // Only add denominator obs for p3 (NOT in stratum) - Map denObs3 = new HashMap<>(); - denObs3.put("p3", new QuantityDef(50.0)); + var denObs3 = new ObservationAccumulator(List.of(new ObservationEntry("p3", new QuantityDef(50.0)))); denominatorMeasureObs.addResource("p3", denObs3); // Stratum populations: subjects p1, p2 diff --git a/cqf-fhir-cr/src/test/java/org/opencds/cqf/fhir/cr/measure/common/MeasureScoreCalculatorTest.java b/cqf-fhir-cr/src/test/java/org/opencds/cqf/fhir/cr/measure/common/MeasureScoreCalculatorTest.java index 536f0429de..f2d9043a88 100644 --- a/cqf-fhir-cr/src/test/java/org/opencds/cqf/fhir/cr/measure/common/MeasureScoreCalculatorTest.java +++ b/cqf-fhir-cr/src/test/java/org/opencds/cqf/fhir/cr/measure/common/MeasureScoreCalculatorTest.java @@ -302,15 +302,14 @@ void testAggregateContinuousVariable_SingleValue() { @Test void testCollectQuantities_ValidMaps() { - // Create resources with nested maps containing QuantityDef values - Map map1 = new HashMap<>(); - map1.put("key1", new QuantityDef(10.0)); - map1.put("key2", new QuantityDef(20.0)); + // Create observation accumulators with QuantityDef values + var acc1 = new ObservationAccumulator(List.of( + new ObservationEntry("key1", new QuantityDef(10.0)), + new ObservationEntry("key2", new QuantityDef(20.0)))); - Map map2 = new HashMap<>(); - map2.put("key3", new QuantityDef(30.0)); + var acc2 = new ObservationAccumulator(List.of(new ObservationEntry("key3", new QuantityDef(30.0)))); - Collection resources = wrap(map1, map2); + Collection resources = wrap(acc1, acc2); List quantities = MeasureScoreCalculator.collectQuantities(resources); @@ -343,7 +342,7 @@ void testCollectQuantities_NoMaps() { } @Test - void testCollectQuantities_MapsWithoutQuantityDef() { + void testCollectQuantities_NonAccumulatorInputsIgnored() { Map map1 = new HashMap<>(); map1.put("key1", "not a quantity"); map1.put("key2", 123); @@ -358,15 +357,13 @@ void testCollectQuantities_MapsWithoutQuantityDef() { @Test void testCollectQuantities_MixedContent() { - // Mix of maps with and without QuantityDef, and non-map objects - Map map1 = new HashMap<>(); - map1.put("key1", new QuantityDef(10.0)); - map1.put("key2", "not a quantity"); + // Mix of accumulators with non-null/null observation values, and non-accumulator objects + var acc1 = new ObservationAccumulator( + List.of(new ObservationEntry("key1", new QuantityDef(10.0)), new ObservationEntry("key2", null))); - Map map2 = new HashMap<>(); - map2.put("key3", 123); + var acc2 = new ObservationAccumulator(List.of(new ObservationEntry("key3", null))); - Collection resources = wrap(map1, "string", map2, new QuantityDef(20.0)); + Collection resources = wrap(acc1, "string", acc2, new QuantityDef(20.0)); List quantities = MeasureScoreCalculator.collectQuantities(resources); @@ -424,12 +421,11 @@ void testRatioScoreIntegration_ContinuousVariable() { @Test void testFullWorkflow_CollectAggregateContinuousVariable() { // Full workflow: collect quantities -> aggregate -> score - Map resource1 = new HashMap<>(); - resource1.put("obs1", new QuantityDef(10.0)); - resource1.put("obs2", new QuantityDef(20.0)); + var resource1 = new ObservationAccumulator(List.of( + new ObservationEntry("obs1", new QuantityDef(10.0)), + new ObservationEntry("obs2", new QuantityDef(20.0)))); - Map resource2 = new HashMap<>(); - resource2.put("obs3", new QuantityDef(30.0)); + var resource2 = new ObservationAccumulator(List.of(new ObservationEntry("obs3", new QuantityDef(30.0)))); Collection resources = wrap(resource1, resource2); diff --git a/cqf-fhir-cr/src/test/java/org/opencds/cqf/fhir/cr/measure/common/PopulationDefTest.java b/cqf-fhir-cr/src/test/java/org/opencds/cqf/fhir/cr/measure/common/PopulationDefTest.java index 8cfbebcaff..cd76c5ec36 100644 --- a/cqf-fhir-cr/src/test/java/org/opencds/cqf/fhir/cr/measure/common/PopulationDefTest.java +++ b/cqf-fhir-cr/src/test/java/org/opencds/cqf/fhir/cr/measure/common/PopulationDefTest.java @@ -5,7 +5,7 @@ import static org.junit.jupiter.api.Assertions.assertTrue; import java.util.Collection; -import java.util.Map; +import java.util.List; import java.util.Objects; import java.util.Set; import java.util.stream.Collectors; @@ -205,17 +205,16 @@ void testGetCount_MeasureObservation_CountsObservations() { PopulationDef popDef = new PopulationDef( "pop-obs", null, MeasurePopulationType.MEASUREOBSERVATION, "MeasureObservation", booleanBasis, null); - // Add observations (Maps) for subjects - // Each observation is a Map with key-value pairs - Map obs1 = Map.of("value", 10.0); - Map obs2 = Map.of("value", 20.0, "unit", "mg"); - Map obs3 = Map.of("value", 30.0); + // Add observation accumulators for subjects, one entry each + var obs1 = new ObservationAccumulator(List.of(new ObservationEntry("Patient/1", new QuantityDef(10.0)))); + var obs2 = new ObservationAccumulator(List.of(new ObservationEntry("Patient/2", new QuantityDef(20.0)))); + var obs3 = new ObservationAccumulator(List.of(new ObservationEntry("Patient/3", new QuantityDef(30.0)))); popDef.addResource("Patient/1", obs1); popDef.addResource("Patient/2", obs2); popDef.addResource("Patient/3", obs3); - // For MEASUREOBSERVATION, getCount() should count subjects + // For MEASUREOBSERVATION, getCount() should count observation entries across accumulators assertEquals(3, popDef.getCount(), "MEASUREOBSERVATION should count observation entries"); } @@ -243,16 +242,13 @@ void testRemoveExcludedMeasureObservationResource_BooleanBasis_RemovesEmptyMaps( Encounter enc3 = (Encounter) new Encounter().setId("Encounter/3"); // Add observations for three subjects - Map obsMap1 = new HashMapForFhirResourcesAndCqlTypes<>(); - obsMap1.put(enc1, new QuantityDef(100.0)); + var obsMap1 = new ObservationAccumulator(List.of(new ObservationEntry(enc1, new QuantityDef(100.0)))); popDef.addResource("Patient/1", obsMap1); - Map obsMap2 = new HashMapForFhirResourcesAndCqlTypes<>(); - obsMap2.put(enc2, new QuantityDef(200.0)); + var obsMap2 = new ObservationAccumulator(List.of(new ObservationEntry(enc2, new QuantityDef(200.0)))); popDef.addResource("Patient/2", obsMap2); - Map obsMap3 = new HashMapForFhirResourcesAndCqlTypes<>(); - obsMap3.put(enc3, new QuantityDef(300.0)); + var obsMap3 = new ObservationAccumulator(List.of(new ObservationEntry(enc3, new QuantityDef(300.0)))); popDef.addResource("Patient/3", obsMap3); // Initial count should be 3 @@ -296,13 +292,12 @@ void testRemoveExcludedMeasureObservationResource_EncounterBasis_PartialRemoval( Encounter enc2 = (Encounter) new Encounter().setId("Encounter/2"); Encounter enc3 = (Encounter) new Encounter().setId("Encounter/3"); - Map obsMapSubject1 = new HashMapForFhirResourcesAndCqlTypes<>(); - obsMapSubject1.put(enc1, new QuantityDef(100.0)); - obsMapSubject1.put(enc2, new QuantityDef(200.0)); + var obsMapSubject1 = new ObservationAccumulator(List.of( + new ObservationEntry(enc1, new QuantityDef(100.0)), + new ObservationEntry(enc2, new QuantityDef(200.0)))); popDef.addResource("Patient/1", obsMapSubject1); - Map obsMapSubject2 = new HashMapForFhirResourcesAndCqlTypes<>(); - obsMapSubject2.put(enc3, new QuantityDef(300.0)); + var obsMapSubject2 = new ObservationAccumulator(List.of(new ObservationEntry(enc3, new QuantityDef(300.0)))); popDef.addResource("Patient/2", obsMapSubject2); // Initial: Patient/1 has 2 entries, Patient/2 has 1 entry = 3 total @@ -341,9 +336,9 @@ void testRemoveExcludedMeasureObservationResource_RemovesAllEntries_SubjectNotCo Encounter enc2 = (Encounter) new Encounter().setId("Encounter/2"); // Patient/1 has 2 observations - Map obsMap1 = new HashMapForFhirResourcesAndCqlTypes<>(); - obsMap1.put(enc1, new QuantityDef(100.0)); - obsMap1.put(enc2, new QuantityDef(200.0)); + var obsMap1 = new ObservationAccumulator(List.of( + new ObservationEntry(enc1, new QuantityDef(100.0)), + new ObservationEntry(enc2, new QuantityDef(200.0)))); popDef.addResource("Patient/1", obsMap1); assertEquals(2, popDef.getCount(), "Should have 2 observations initially"); @@ -690,14 +685,13 @@ void testCountObservations_WithMultipleMaps() { Encounter enc3 = (Encounter) new Encounter().setId("Encounter/3"); // Patient/1 has 2 observations - Map obsMap1 = new HashMapForFhirResourcesAndCqlTypes<>(); - obsMap1.put(enc1, new QuantityDef(100.0)); - obsMap1.put(enc2, new QuantityDef(200.0)); + var obsMap1 = new ObservationAccumulator(List.of( + new ObservationEntry(enc1, new QuantityDef(100.0)), + new ObservationEntry(enc2, new QuantityDef(200.0)))); popDef.addResource("Patient/1", obsMap1); // Patient/2 has 1 observation - Map obsMap2 = new HashMapForFhirResourcesAndCqlTypes<>(); - obsMap2.put(enc3, new QuantityDef(300.0)); + var obsMap2 = new ObservationAccumulator(List.of(new ObservationEntry(enc3, new QuantityDef(300.0)))); popDef.addResource("Patient/2", obsMap2); assertEquals(3, popDef.countObservations(), "Should count all observation entries across all maps"); From e29bba8c1424dde92c524c813b62f1e2b566fd9d Mon Sep 17 00:00:00 2001 From: Luke deGruchy Date: Fri, 1 May 2026 14:50:35 -0400 Subject: [PATCH 11/13] Replace stratifier function-result Map with FunctionResultAccumulator Mirrors the previous commit for non-subject-value stratifier function results. FunctionEvaluationHandler.processNonSubValueStratifier now produces a FunctionResultAccumulator wrapping List instead of Map. The accumulator is a non-Iterable record so the upstream asIterable() path doesn't unroll it. CqlExpressionValue gains asFunctionResultAccumulator() alongside asObservationAccumulator(). MeasureMultiSubjectEvaluator's three function- result consumers (collectFunctionRowKeys, mapToListOfTableEntries via addFunctionResultRows, stratifierResultAsIntersectionSet) read entries through the typed accessor. asMap() / isMap() are kept since R4SupportingEvidenceExtension and EvaluationResultFormatter still process arbitrary CQL Map values that are not function-result accumulators. After this commit HashMapForFhirResourcesAndCqlTypes is unused by any production code; commit C will remove it. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../cr/measure/common/CqlExpressionValue.java | 10 ++++ .../common/FunctionEvaluationHandler.java | 22 +++----- .../common/FunctionResultAccumulator.java | 24 +++++++++ .../measure/common/FunctionResultEntry.java | 17 ++++++ .../common/MeasureMultiSubjectEvaluator.java | 53 +++++++++++-------- .../common/CqlExpressionValueTest.java | 45 ++++++++++++++++ 6 files changed, 135 insertions(+), 36 deletions(-) create mode 100644 cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/FunctionResultAccumulator.java create mode 100644 cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/FunctionResultEntry.java diff --git a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/CqlExpressionValue.java b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/CqlExpressionValue.java index 47e2167471..e8450749dd 100644 --- a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/CqlExpressionValue.java +++ b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/CqlExpressionValue.java @@ -128,6 +128,16 @@ public Optional asObservationAccumulator() { return raw instanceof ObservationAccumulator acc ? Optional.of(acc) : Optional.empty(); } + /** + * Returns the underlying value as the {@link FunctionResultAccumulator} produced by + * {@code FunctionEvaluationHandler.processNonSubValueStratifier}, or empty otherwise. + * Mirrors {@link #asObservationAccumulator()}: non-Iterable record so the upstream + * {@link #asIterable()} path doesn't unroll it into individual entries. + */ + public Optional asFunctionResultAccumulator() { + return raw instanceof FunctionResultAccumulator acc ? Optional.of(acc) : Optional.empty(); + } + /** * Normalizes the value to an {@link Iterable}: null becomes an empty list, an existing * iterable is returned as-is, and a scalar is wrapped in a single-element list. diff --git a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/FunctionEvaluationHandler.java b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/FunctionEvaluationHandler.java index d437300467..32cd81f445 100644 --- a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/FunctionEvaluationHandler.java +++ b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/FunctionEvaluationHandler.java @@ -4,7 +4,6 @@ import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; import java.util.ArrayList; import java.util.Collection; -import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; @@ -407,7 +406,7 @@ private static void processNonSubValueStratifier( // make new expression name for uniquely extracting results // this will be used in MeasureEvaluator (Criteria population Id and Stratifier Expression) var expressionName = popDef.id() + "-" + stratifierExpression; - final Map functionResults = new HashMap<>(); + final List functionResults = new ArrayList<>(); final Set evaluatedResources = new HashSet<>(); for (Object result : resultsIter) { @@ -417,13 +416,10 @@ private static void processNonSubValueStratifier( stratifierExpression, getFunctionArguments(groupDef, result), exceptionMessageIfNotFunction); - // add function results to existing EvaluationResult under new expression - // name - // need a way to capture input parameter here too, otherwise we have no way - // to connect input objects related to output object - // key= input parameter to function - // value= the output Observation resource containing calculated value - functionResults.put(result, functionResult.getValue()); + // Each entry pairs the input parameter passed to the stratifier function with the + // heterogeneous CQL value the function returned. Iteration order is the order + // populationDef results were iterated. + functionResults.add(new FunctionResultEntry(result, functionResult.getValue())); Set evaluated = functionResult.getEvaluatedResources(); if (evaluated == null) { throw new IllegalStateException("CQL function '" + stratifierExpression @@ -433,7 +429,8 @@ private static void processNonSubValueStratifier( evaluatedResources.addAll(functionResult.getEvaluatedResources()); } // add to EvaluationResult - addToEvaluationResult(evalResult, expressionName, functionResults, evaluatedResources); + addToEvaluationResult( + evalResult, expressionName, new FunctionResultAccumulator(functionResults), evaluatedResources); } } @@ -658,10 +655,7 @@ private static EvaluationResult buildEvaluationResult( } private static void addToEvaluationResult( - EvaluationResult result, - String expressionName, - Map functionResults, - Set evaluatedResources) { + EvaluationResult result, String expressionName, Object functionResults, Set evaluatedResources) { result.set( new EvaluationExpressionRef(expressionName), new ExpressionResult(functionResults, evaluatedResources)); diff --git a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/FunctionResultAccumulator.java b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/FunctionResultAccumulator.java new file mode 100644 index 0000000000..0e71ea6a82 --- /dev/null +++ b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/FunctionResultAccumulator.java @@ -0,0 +1,24 @@ +package org.opencds.cqf.fhir.cr.measure.common; + +import java.util.List; + +/** + * The bag of {@link FunctionResultEntry} produced for one subject by one NON_SUBJECT_VALUE + * stratifier component's function. A non-Iterable record wrapping the entries list so the + * upstream {@link CqlExpressionValue#asIterable()} path doesn't unroll it; mirrors + * {@link ObservationAccumulator}. + */ +public record FunctionResultAccumulator(List entries) { + + public FunctionResultAccumulator { + entries = List.copyOf(entries); + } + + public boolean isEmpty() { + return entries.isEmpty(); + } + + public int size() { + return entries.size(); + } +} diff --git a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/FunctionResultEntry.java b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/FunctionResultEntry.java new file mode 100644 index 0000000000..fd6d078f27 --- /dev/null +++ b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/FunctionResultEntry.java @@ -0,0 +1,17 @@ +package org.opencds.cqf.fhir.cr.measure.common; + +import jakarta.annotation.Nullable; + +/** + * One row of a non-subject-value stratifier function-result accumulator: an input parameter + * passed to the stratifier function paired with the heterogeneous CQL value the function returned. + *

+ * Used in place of {@code Map} so the data flow is self-documenting and + * downstream consumers iterate a typed {@code List} instead of {@code Map.Entry}. + *

+ * Both {@code input} and {@code output} are typed as {@link Object}: the input may be a FHIR resource + * or a primitive (depending on population basis), and the output is whatever the CQL function produced + * (typically a String or Number, but the contract permits any CQL value). + */ +public record FunctionResultEntry( + @Nullable Object input, @Nullable Object output) {} diff --git a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureMultiSubjectEvaluator.java b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureMultiSubjectEvaluator.java index 1a88b0e9f2..dcd3cb83c5 100644 --- a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureMultiSubjectEvaluator.java +++ b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureMultiSubjectEvaluator.java @@ -499,11 +499,12 @@ private static Map> collectFunctionRowKeys( String subjectId = entry.getKey(); CqlExpressionValue result = entry.getValue(); - // Only process function results (Map values) + // Only process function results (FunctionResultAccumulator) if (result == null) { continue; } - Map functionResults = result.asMap().orElse(null); + FunctionResultAccumulator functionResults = + result.asFunctionResultAccumulator().orElse(null); if (functionResults == null) { continue; } @@ -511,8 +512,8 @@ private static Map> collectFunctionRowKeys( Set rowKeys = functionRowKeysBySubject.computeIfAbsent(qualifiedSubject, k -> new HashSet<>()); - for (Object key : functionResults.keySet()) { - String normalizedKey = normalizeResourceKey(key); + for (FunctionResultEntry fnEntry : functionResults.entries()) { + String normalizedKey = normalizeResourceKey(fnEntry.input()); rowKeys.add(StratifierRowKey.withInput(qualifiedSubject, normalizedKey)); } } @@ -538,8 +539,12 @@ private static List mapToListOfTableEntries( final String qualifiedSubject = FhirResourceUtils.addPatientQualifier(subjectId); final Object rawValue = result == null ? null : result.raw(); - if (result != null && result.isMap()) { - return addFunctionResultRows(qualifiedSubject, result.asMap().orElseThrow()); + if (result != null) { + FunctionResultAccumulator functionResults = + result.asFunctionResultAccumulator().orElse(null); + if (functionResults != null) { + return addFunctionResultRows(qualifiedSubject, functionResults); + } } if (result != null && result.isIterable()) { return addIterableValueRows(qualifiedSubject, (Iterable) rawValue); @@ -577,18 +582,20 @@ private static List expandScalarToMatchFunctionRowKeys( } /** - * Adds rows for non-subject value stratifiers with function results (Map<inputResource, outputValue>). + * Adds rows for non-subject value stratifiers with function results (one entry per inputResource + * paired with the outputValue the stratifier function produced). * - *

For each entry in the map: + *

For each entry: *

    *
  • Build composite row key: "Patient/xxx|Resource/yyy"
    • *
  • The output value becomes the stratum value (what's displayed)
    • *
  • Null values are allowed - they will be grouped into a special "null" stratum
    • *
*/ - private static List addFunctionResultRows(String qualifiedSubject, Map functionResults) { + private static List addFunctionResultRows( + String qualifiedSubject, FunctionResultAccumulator functionResults) { - return functionResults.entrySet().stream() + return functionResults.entries().stream() .map(entry -> // The output value becomes the stratum value (what's displayed) // Null values are allowed - they will be grouped into a special "null" stratum @@ -596,8 +603,8 @@ private static List addFunctionResultRows(String qualifiedSubje StratifierRowKey.withInput( qualifiedSubject, // Build composite row key: "Patient/xxx|Resource/yyy" - normalizeResourceKey(entry.getKey())), - new StratumValueWrapper(entry.getValue()))) + normalizeResourceKey(entry.input())), + new StratumValueWrapper(entry.output()))) .toList(); } @@ -765,9 +772,9 @@ private static Set calculateCriteriaStratifierIntersection( // For each subject, we intersect between the population (Set) and // stratifier results (Set of raw resources). Iterate the population side and - // delegate to the stratifier set's contains(): for Map-based stratifier results that's - // plain Object.equals on map keys; for non-Map results that's FHIR-identity equality - // via HashSetForFhirResourcesAndCqlTypes. + // delegate to the stratifier set's contains(): for FunctionResultAccumulator-based + // stratifier results that's plain Object.equals on the entry inputs; for non-accumulator + // results that's FHIR-identity equality via HashSetForFhirResourcesAndCqlTypes. for (Entry stratifierEntryBySubject : stratifierResultsBySubject.entrySet()) { final Set stratifierResultsPerSubject = stratifierResultAsIntersectionSet(stratifierEntryBySubject.getValue()); @@ -792,17 +799,19 @@ private static Set calculateCriteriaStratifierIntersection( /** * Convert a stratifier result into the set that should be used for intersection. * - *

For Map-based results (Map), the input parameters (map keys) - * are the intersectable items. + *

For function-result accumulators (one entry per input parameter / produced value), the + * input parameters are the intersectable items. */ private static Set stratifierResultAsIntersectionSet(CqlExpressionValue result) { if (result == null) { return Set.of(); } - Map m = result.asMap().orElse(null); - if (m != null) { - return new HashSet<>(m.keySet()); + FunctionResultAccumulator acc = result.asFunctionResultAccumulator().orElse(null); + if (acc != null) { + return acc.entries().stream() + .map(FunctionResultEntry::input) + .collect(java.util.stream.Collectors.toCollection(HashSet::new)); } return result.valueAsSet(); @@ -925,8 +934,8 @@ private static List getResourcesForSubjects( * (e.g., "patient1"), but StratifierRowKey uses QUALIFIED IDs (e.g., "Patient/patient1"). * For primitive types, this method qualifies the subject ID to ensure proper matching. * - *

For MEASUREOBSERVATION populations, the subjectResources contain Set<Map<inputResource, outputValue>> - * so we extract the keys (input resources) from those maps. + *

For MEASUREOBSERVATION populations, the subjectResources hold {@link ObservationAccumulator} + * instances; we extract each entry's {@code inputResource} to drive stratification keys. */ private static Set getPopulationResourceKeySet( FhirContext fhirContext, GroupDef groupDef, PopulationDef populationDef) { diff --git a/cqf-fhir-cr/src/test/java/org/opencds/cqf/fhir/cr/measure/common/CqlExpressionValueTest.java b/cqf-fhir-cr/src/test/java/org/opencds/cqf/fhir/cr/measure/common/CqlExpressionValueTest.java index f6b86fa8e4..5904b26163 100644 --- a/cqf-fhir-cr/src/test/java/org/opencds/cqf/fhir/cr/measure/common/CqlExpressionValueTest.java +++ b/cqf-fhir-cr/src/test/java/org/opencds/cqf/fhir/cr/measure/common/CqlExpressionValueTest.java @@ -345,6 +345,51 @@ void asObservationAccumulator_emptyAccumulatorYieldsEmptyAccumulator() { assertEquals(0, opt.get().size()); } + // -- asFunctionResultAccumulator --------------------------------------------- + + @Test + void asFunctionResultAccumulator_emptyOptionalForNonAccumulatorInputs() { + assertEquals( + java.util.Optional.empty(), CqlExpressionValue.ofRaw(null, null).asFunctionResultAccumulator()); + assertEquals( + java.util.Optional.empty(), + CqlExpressionValue.ofRaw("scalar", null).asFunctionResultAccumulator()); + assertEquals( + java.util.Optional.empty(), + CqlExpressionValue.ofRaw(Map.of("k", "v"), null).asFunctionResultAccumulator()); + assertEquals( + java.util.Optional.empty(), + CqlExpressionValue.ofRaw(List.of(), null).asFunctionResultAccumulator()); + } + + @Test + void asFunctionResultAccumulator_returnsAccumulatorWhenWrapped() { + Encounter enc = new Encounter(); + enc.setId("Encounter/1"); + FunctionResultAccumulator acc = + new FunctionResultAccumulator(List.of(new FunctionResultEntry(enc, "stratum-value"))); + + java.util.Optional opt = + CqlExpressionValue.ofRaw(acc, null).asFunctionResultAccumulator(); + + assertTrue(opt.isPresent()); + assertSame(acc, opt.get()); + assertEquals(1, opt.get().size()); + assertSame(enc, opt.get().entries().get(0).input()); + assertEquals("stratum-value", opt.get().entries().get(0).output()); + } + + @Test + void asFunctionResultAccumulator_isNotConfusedWithObservationAccumulator() { + ObservationAccumulator obsAcc = + new ObservationAccumulator(List.of(new ObservationEntry("k", new QuantityDef(1.0)))); + + // Same wrapper held only as the OTHER accumulator type returns the right narrowing + CqlExpressionValue wrapper = CqlExpressionValue.ofRaw(obsAcc, null); + assertTrue(wrapper.asObservationAccumulator().isPresent()); + assertEquals(java.util.Optional.empty(), wrapper.asFunctionResultAccumulator()); + } + // -- valueAsSet -------------------------------------------------------------- @Test From 9893b65feb201c07a92e7b4ebc7537a163d3d3fe Mon Sep 17 00:00:00 2001 From: Luke deGruchy Date: Fri, 1 May 2026 15:29:02 -0400 Subject: [PATCH 12/13] Fix sonar. --- .../cr/measure/common/CqlExpressionValue.java | 4 ++-- .../common/MeasureMultiSubjectEvaluator.java | 21 ++++++++++++------- .../common/CqlExpressionValueTest.java | 4 +++- 3 files changed, 19 insertions(+), 10 deletions(-) diff --git a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/CqlExpressionValue.java b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/CqlExpressionValue.java index e8450749dd..9849f3df6a 100644 --- a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/CqlExpressionValue.java +++ b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/CqlExpressionValue.java @@ -187,8 +187,8 @@ public Iterable resolveForPopulation(String subjectType, EvaluationResul if (raw == null) { return Collections.emptyList(); } - if (raw instanceof Boolean b) { - if (!b) { + if (raw instanceof Boolean aBoolean) { + if (Boolean.FALSE.equals(aBoolean)) { return Collections.emptyList(); } ExpressionResult subjectResult = evaluationResult.get(subjectType); diff --git a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureMultiSubjectEvaluator.java b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureMultiSubjectEvaluator.java index dcd3cb83c5..bfe9ca63e1 100644 --- a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureMultiSubjectEvaluator.java +++ b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureMultiSubjectEvaluator.java @@ -12,6 +12,7 @@ import java.util.Map; import java.util.Map.Entry; import java.util.Objects; +import java.util.Optional; import java.util.Set; import java.util.stream.Collector; import java.util.stream.Collectors; @@ -500,19 +501,17 @@ private static Map> collectFunctionRowKeys( CqlExpressionValue result = entry.getValue(); // Only process function results (FunctionResultAccumulator) - if (result == null) { - continue; - } - FunctionResultAccumulator functionResults = - result.asFunctionResultAccumulator().orElse(null); - if (functionResults == null) { + final Optional optFunctionResults = getFunctionResultAccumulator(result); + + if (optFunctionResults.isEmpty()) { continue; } + String qualifiedSubject = FhirResourceUtils.addPatientQualifier(subjectId); Set rowKeys = functionRowKeysBySubject.computeIfAbsent(qualifiedSubject, k -> new HashSet<>()); - for (FunctionResultEntry fnEntry : functionResults.entries()) { + for (FunctionResultEntry fnEntry : optFunctionResults.get().entries()) { String normalizedKey = normalizeResourceKey(fnEntry.input()); rowKeys.add(StratifierRowKey.withInput(qualifiedSubject, normalizedKey)); } @@ -522,6 +521,14 @@ private static Map> collectFunctionRowKeys( return functionRowKeysBySubject; } + private static Optional getFunctionResultAccumulator(CqlExpressionValue result) { + if (result == null) { + return Optional.empty(); + } + + return result.asFunctionResultAccumulator(); + } + private static List mapToListOfTableEntries( StratifierComponentDef componentDef, Map> functionRowKeysBySubject) { diff --git a/cqf-fhir-cr/src/test/java/org/opencds/cqf/fhir/cr/measure/common/CqlExpressionValueTest.java b/cqf-fhir-cr/src/test/java/org/opencds/cqf/fhir/cr/measure/common/CqlExpressionValueTest.java index 5904b26163..f8784f7234 100644 --- a/cqf-fhir-cr/src/test/java/org/opencds/cqf/fhir/cr/measure/common/CqlExpressionValueTest.java +++ b/cqf-fhir-cr/src/test/java/org/opencds/cqf/fhir/cr/measure/common/CqlExpressionValueTest.java @@ -221,9 +221,11 @@ void resolveForPopulation_trueLooksUpSubjectContextValue() { void resolveForPopulation_trueButNoSubjectResultThrows() { EvaluationResult evaluationResult = new EvaluationResult(); + final CqlExpressionValue cqlExpressionValue = CqlExpressionValue.ofRaw(true, null); + CqlExpressionValueException ex = assertThrows( CqlExpressionValueException.class, - () -> CqlExpressionValue.ofRaw(true, null).resolveForPopulation("Patient", evaluationResult)); + () -> cqlExpressionValue.resolveForPopulation("Patient", evaluationResult)); assertTrue(ex.getMessage().contains("Patient")); } From d1ee0376a75133b803bf0710234bef76b10d3270 Mon Sep 17 00:00:00 2001 From: Luke deGruchy Date: Fri, 1 May 2026 15:47:42 -0400 Subject: [PATCH 13/13] Fix sonar. --- .../common/MeasureObservationHandler.java | 2 +- .../fhir/cr/measure/common/PopulationDef.java | 35 +++++++++++-------- 2 files changed, 21 insertions(+), 16 deletions(-) diff --git a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureObservationHandler.java b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureObservationHandler.java index 091c5665ae..2addf529db 100644 --- a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureObservationHandler.java +++ b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureObservationHandler.java @@ -89,7 +89,7 @@ private static void removeMatchingEntriesFromObservationAccumulator( entry -> FhirResourceAndCqlTypeUtils.areObjectsEqual(entry.inputResource(), exclusionRaw)); if (matchFound) { - logger.debug( + logger.atDebug().log( "Removing observation for excluded resource: {}", EvaluationResultFormatter.formatResource(exclusionRaw)); // Delegate to PopulationDef so empty accumulators get purged from the subject set diff --git a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/PopulationDef.java b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/PopulationDef.java index 4887f751f2..2f9042153f 100644 --- a/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/PopulationDef.java +++ b/cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/PopulationDef.java @@ -142,21 +142,7 @@ public void removeExcludedMeasureObservationResource(String subjectId, Object me // we drop the wrapper entirely so the count stays correct. Set rebuilt = new HashSetForCqlExpressionValues(); for (CqlExpressionValue element : resourcesForSubject) { - if (element == null) { - continue; - } - ObservationAccumulator acc = element.asObservationAccumulator().orElse(null); - if (acc == null) { - rebuilt.add(element); // not an observation accumulator, leave alone - continue; - } - List filtered = acc.entries().stream() - .filter(e -> !FhirResourceAndCqlTypeUtils.areObjectsEqual( - e.inputResource(), measureObservationResourceKey)) - .toList(); - if (!filtered.isEmpty()) { - rebuilt.add(CqlExpressionValue.ofRaw(new ObservationAccumulator(filtered), null)); - } + processSingleCqlExpressionValue(measureObservationResourceKey, element, rebuilt); } resourcesForSubject.clear(); resourcesForSubject.addAll(rebuilt); @@ -167,6 +153,25 @@ public void removeExcludedMeasureObservationResource(String subjectId, Object me } } + private static void processSingleCqlExpressionValue( + Object measureObservationResourceKey, CqlExpressionValue element, Set rebuilt) { + if (element == null) { + return; + } + ObservationAccumulator acc = element.asObservationAccumulator().orElse(null); + if (acc == null) { + rebuilt.add(element); // not an observation accumulator, leave alone + return; + } + List filtered = acc.entries().stream() + .filter(e -> + !FhirResourceAndCqlTypeUtils.areObjectsEqual(e.inputResource(), measureObservationResourceKey)) + .toList(); + if (!filtered.isEmpty()) { + rebuilt.add(CqlExpressionValue.ofRaw(new ObservationAccumulator(filtered), null)); + } + } + public void retainAllResources(String subjectId, PopulationDef otherPopulationDef) { getResourcesForSubject(subjectId).retainAll(otherPopulationDef.getResourcesForSubject(subjectId)); }