Merge pull request #737 from cqframework/fix-library-reference-scope-…

…issue

Fixed an issue with data requirements processing resulting in a NullP…

Src/java/elm-fhir/src/main/java/org/cqframework/cql/elm/requirements/fhir/DataRequirementsProcessor.java

@@ -4,6 +4,8 @@
 import org.cqframework.cql.cql2elm.LibraryManager;
 import org.cqframework.cql.cql2elm.NamespaceManager;
 import org.cqframework.cql.cql2elm.model.TranslatedLibrary;
+import org.cqframework.cql.elm.tracking.TrackBack;
+import org.cqframework.cql.elm.tracking.Trackable;
 import org.hl7.cql.model.IntervalType;
 import org.hl7.cql.model.ListType;
 import org.hl7.cql.model.NamedType;
@@ -518,6 +520,36 @@ private String toFHIRTypeCode(org.hl7.cql.model.DataType dataType, AtomicBoolean
         return "Any";
     }
 
+    /**
+     * TODO: This function is used to determine the library identifier in which the reference element was declared
+     * This is only possible if the ELM includes trackbacks, which are typically only available in ELM coming straight from the translator
+     * (i.e. de-compiled ELM won't have this)
+     * The issue is that when code filter expressions are distributed, the references may cross declaration contexts (i.e. a code filter
+     * expression from the library in which it was first expressed may be evaluated in the context of a data requirement inferred
+     * from a retrieve in a different library. If the library aliases are consistent, this isn't an issue, but if the library aliases
+     * are different, this will result in a failure to resolve the reference (or worse, an incorrect reference).
+     * This is being reported as a warning currently, but it is really an issue with the data requirement distribution, it should be
+     * rewriting references as it distributes (or better yet, ELM should have a library identifier that is fully resolved, rather
+     * than relying on library-specific aliases for library referencing elements.
+     * @param trackable
+     * @param libraryIdentifier
+     * @return
+     */
+    private VersionedIdentifier getDeclaredLibraryIdentifier(Trackable trackable, VersionedIdentifier libraryIdentifier) {
+        if (trackable.getTrackbacks() != null) {
+            for (TrackBack tb : trackable.getTrackbacks()) {
+                if (tb.getLibrary() != null) {
+                    return tb.getLibrary();
+                }
+            }
+        }
+
+        validationMessages.add(new ValidationMessage(ValidationMessage.Source.Publisher, ValidationMessage.IssueType.PROCESSING, "Data requirements processing",
+                String.format("Library referencing element (%s) is potentially being resolved in a different context than it was declared. Ensure library aliases are consistent", trackable.getClass().getSimpleName()), ValidationMessage.IssueSeverity.WARNING));
+
+        return libraryIdentifier;
+    }
+
     private org.hl7.fhir.r5.model.DataRequirement.DataRequirementCodeFilterComponent toCodeFilterComponent(ElmRequirementsContext context, VersionedIdentifier libraryIdentifier, String property, Expression value) {
         org.hl7.fhir.r5.model.DataRequirement.DataRequirementCodeFilterComponent cfc =
                 new org.hl7.fhir.r5.model.DataRequirement.DataRequirementCodeFilterComponent();
@@ -528,7 +560,8 @@ private org.hl7.fhir.r5.model.DataRequirement.DataRequirementCodeFilterComponent
 
         if (value instanceof ValueSetRef) {
             ValueSetRef vsr = (ValueSetRef)value;
-            cfc.setValueSet(toReference(context.resolveValueSetRef(libraryIdentifier, vsr)));
+            VersionedIdentifier declaredLibraryIdentifier = getDeclaredLibraryIdentifier(vsr, libraryIdentifier);
+            cfc.setValueSet(toReference(context.resolveValueSetRef(declaredLibraryIdentifier, vsr)));
         }
 
         if (value instanceof org.hl7.elm.r1.ToList) {
@@ -950,7 +983,8 @@ private void resolveCodeFilterCodes(ElmRequirementsContext context, VersionedIde
                                         Expression e) {
         if (e instanceof org.hl7.elm.r1.CodeRef) {
             CodeRef cr = (CodeRef)e;
-            cfc.addCode(toCoding(context, libraryIdentifier, context.toCode(context.resolveCodeRef(libraryIdentifier, cr))));
+            VersionedIdentifier declaredLibraryIdentifier = getDeclaredLibraryIdentifier(cr, libraryIdentifier);
+            cfc.addCode(toCoding(context, libraryIdentifier, context.toCode(context.resolveCodeRef(declaredLibraryIdentifier, cr))));
         }
 
         if (e instanceof org.hl7.elm.r1.Code) {
@@ -959,8 +993,9 @@ private void resolveCodeFilterCodes(ElmRequirementsContext context, VersionedIde
 
         if (e instanceof org.hl7.elm.r1.ConceptRef) {
             ConceptRef cr = (ConceptRef)e;
+            VersionedIdentifier declaredLibraryIdentifier = getDeclaredLibraryIdentifier(cr, libraryIdentifier);
             org.hl7.fhir.r5.model.CodeableConcept c = toCodeableConcept(context, libraryIdentifier,
-                    context.toConcept(libraryIdentifier, context.resolveConceptRef(libraryIdentifier, cr)));
+                    context.toConcept(libraryIdentifier, context.resolveConceptRef(declaredLibraryIdentifier, cr)));
             for (org.hl7.fhir.r5.model.Coding code : c.getCoding()) {
                 cfc.addCode(code);
             }
@@ -972,10 +1007,16 @@ private void resolveCodeFilterCodes(ElmRequirementsContext context, VersionedIde
                 cfc.addCode(code);
             }
         }
+
+        if (e instanceof org.hl7.elm.r1.Literal) {
+            org.hl7.elm.r1.Literal literal = (org.hl7.elm.r1.Literal)e;
+            cfc.addCode().setCode(literal.getValue());
+        }
     }
 
     private org.hl7.fhir.r5.model.Coding toCoding(ElmRequirementsContext context, VersionedIdentifier libraryIdentifier, Code code) {
-        CodeSystemDef codeSystemDef = context.resolveCodeSystemRef(libraryIdentifier, code.getSystem());
+        VersionedIdentifier declaredLibraryIdentifier = getDeclaredLibraryIdentifier(code.getSystem(), libraryIdentifier);
+        CodeSystemDef codeSystemDef = context.resolveCodeSystemRef(declaredLibraryIdentifier, code.getSystem());
         org.hl7.fhir.r5.model.Coding coding = new org.hl7.fhir.r5.model.Coding();
         coding.setCode(code.getCode());
         coding.setDisplay(code.getDisplay());

Src/java/elm-fhir/src/test/java/org/cqframework/cql/elm/requirements/fhir/DataRequirementsProcessorTest.java

@@ -1406,20 +1406,36 @@ public void TestHEDISBCSE() throws IOException {
         assertNotNull(moduleDefinitionLibrary);
     }
 
+    private void assertEqualToExpectedModuleDefinitionLibrary(org.hl7.fhir.r5.model.Library actualModuleDefinitionLibrary, String pathToExpectedModuleDefinitionLibrary) {
+        FhirContext context =  getFhirContext();
+        IParser parser = context.newJsonParser();
+        org.hl7.fhir.r5.model.Library expectedModuleDefinitionLibrary = (org.hl7.fhir.r5.model.Library)parser.parseResource(DataRequirementsProcessorTest.class.getResourceAsStream(pathToExpectedModuleDefinitionLibrary));
+        assertNotNull(expectedModuleDefinitionLibrary);
+        actualModuleDefinitionLibrary.setDate(null);
+        expectedModuleDefinitionLibrary.setDate(null);
+        assertTrue(actualModuleDefinitionLibrary.equalsDeep(expectedModuleDefinitionLibrary));
+    }
+
     @Test
     public void TestEXMLogic() throws IOException {
         CqlTranslatorOptions translatorOptions = getTranslatorOptions();
         translatorOptions.setAnalyzeDataRequirements(false);
         CqlTranslator translator = setupDataRequirementsAnalysis("EXMLogic/EXMLogic.cql", translatorOptions);
         org.hl7.fhir.r5.model.Library moduleDefinitionLibrary = getModuleDefinitionLibrary(translator, translatorOptions);
         assertNotNull(moduleDefinitionLibrary);
-        FhirContext context =  getFhirContext();
-        IParser parser = context.newJsonParser();
-        org.hl7.fhir.r5.model.Library expectedModuleDefinitionLibrary = (org.hl7.fhir.r5.model.Library)parser.parseResource(DataRequirementsProcessorTest.class.getResourceAsStream("EXMLogic/Library-EXMLogic-data-requirements.json"));
-        assertNotNull(expectedModuleDefinitionLibrary);
-        moduleDefinitionLibrary.setDate(null);
-        expectedModuleDefinitionLibrary.setDate(null);
-        assertTrue(moduleDefinitionLibrary.equalsDeep(expectedModuleDefinitionLibrary));
+        assertEqualToExpectedModuleDefinitionLibrary(moduleDefinitionLibrary, "EXMLogic/Library-EXMLogic-data-requirements.json");
+
+        //outputModuleDefinitionLibrary(moduleDefinitionLibrary);
+    }
+
+    @Test
+    public void TestWithDependencies() throws IOException {
+        CqlTranslatorOptions translatorOptions = getTranslatorOptions();
+        translatorOptions.setAnalyzeDataRequirements(false);
+        CqlTranslator translator = setupDataRequirementsAnalysis("WithDependencies/BSElements.cql", translatorOptions);
+        org.hl7.fhir.r5.model.Library moduleDefinitionLibrary = getModuleDefinitionLibrary(translator, translatorOptions);
+        assertNotNull(moduleDefinitionLibrary);
+        assertEqualToExpectedModuleDefinitionLibrary(moduleDefinitionLibrary, "WithDependencies/Library-BSElements-data-requirements.json");
 
         //outputModuleDefinitionLibrary(moduleDefinitionLibrary);
     }

Src/java/elm-fhir/src/test/resources/org/cqframework/cql/elm/requirements/fhir/WithDependencies/BSElements.cql

@@ -0,0 +1,17 @@
+/* Context-independent Data Elements. (e.g., the Retrieves, to be used in more than one context.) */
+
+library BSElements version '1.0.000'
+
+using FHIR version '4.0.1'
+
+include FHIRCommon version '4.0.1' called FHIRCommon
+include FHIRHelpers version '4.0.1' called FHIRHelpers
+include CommonConcepts version '1.0.000' called CommonCx
+include CommonElements version '1.0.000' called CommonEl
+
+include Ind2E31A37EB104A7D1 version '1.0.000' called Ind2E31A37EB104A7D1
+
+context Patient
+
+define "4. Biliopancreatic bypass with duodenal switch in patients ages greater than or equal to 18 years with BMI greater than or equal to 50 kg/(meter squared)":
+  Ind2E31A37EB104A7D1."4. Biliopancreatic bypass with duodenal switch in patients ages greater than or equal to 18 years with BMI greater than or equal to 50 kg/(meter squared)"

Src/java/elm-fhir/src/test/resources/org/cqframework/cql/elm/requirements/fhir/WithDependencies/CommonConcepts.cql

@@ -0,0 +1,67 @@
+/* Common Terminology */
+library CommonConcepts version '1.0.000'
+
+using FHIR version '4.0.1'
+
+codesystem "ConditionVerificationStatusCodes": 'http://terminology.hl7.org/CodeSystem/condition-ver-status'
+
+valueset "Severe Pain Valueset": 'http://example.com/fhir/ValueSet/rc-severePain'
+valueset "Functional Disability Valueset": 'http://example.com/fhir/ValueSet/rc_functionalDisabilityDJD'
+valueset "Active Condition": 'http://fhir.org/guides/cqf/common/ValueSet/active-condition'
+
+valueset "ro_bodyMassIndex_kg_per_m2": 'http://example.com/fhir/ValueSet/ro_bodyMassIndex_kg_per_m2'
+valueset "ro_weight_kg": 'http://example.com/fhir/ValueSet/ro_weight_kg'
+valueset "ro_weight_lb": 'http://example.com/fhir/ValueSet/ro_weight_lb'
+valueset "ro_height_in": 'http://example.com/fhir/ValueSet/ro_height_in'
+valueset "ro_height_cm": 'http://example.com/fhir/ValueSet/ro_height_cm'
+valueset "rc_diabetesMellitusChronic": 'http://example.com/fhir/ValueSet/rc_diabetesMellitusChronic'
+valueset "rc_coronaryArteryDisease": 'http://example.com/fhir/ValueSet/rc_coronaryArteryDisease'
+valueset "rc_hypertension_cambia": 'http://example.com/fhir/ValueSet/rc_hypertension_cambia'
+valueset "rc_obstructiveSleepApnea": 'http://example.com/fhir/ValueSet/rc_obstructiveSleepApnea'
+valueset "rc_perforationOfStomach_cambia": 'http://example.com/fhir/ValueSet/rc_perforationOfStomach_cambia'
+valueset "rc_gastricMucosalErosion_cambia": 'http://example.com/fhir/ValueSet/rc_gastricMucosalErosion_cambia'
+valueset "rp_sleeveGastrectomy_cambia": 'http://example.com/fhir/ValueSet/rp_sleeveGastrectomy_cambia'
+valueset "rp_initialAdjustableGastricBanding_cambia": 'http://example.com/fhir/ValueSet/rp_initialAdjustableGastricBanding_cambia'
+valueset "rp_initialGastricBypassRouxEnYAnastomosis_cambia": 'http://example.com/fhir/ValueSet/rp_initialGastricBypassRouxEnYAnastomosis_cambia'
+valueset "rc_gastricBandComplication_cambia": 'http://example.com/fhir/ValueSet/rc_gastricBandComplication_cambia'
+valueset "rp_gastricBandAdjustment_cambia": 'http://example.com/fhir/ValueSet/rp_gastricBandAdjustment_cambia'
+valueset "rc_gastricFistula_cambia": 'http://example.com/fhir/ValueSet/rc_gastricFistula_cambia'
+valueset "rc_gastricSmallBowelObstructionAcquired_cambia": 'http://example.com/fhir/ValueSet/rc_gastricSmallBowelObstructionAcquired_cambia'
+valueset "rc_gastricBandInfection_cambia": 'http://example.com/fhir/ValueSet/rc_gastricBandInfection_cambia'
+valueset "rc_esophagitis_cambia": 'http://example.com/fhir/ValueSet/rc_esophagitis_cambia'
+valueset "rc_barrettEsophagus": 'http://example.com/fhir/ValueSet/rc_barrettEsophagus'
+valueset "rc_gastrojejunalUlcer_cambia": 'http://example.com/fhir/ValueSet/rc_gastrojejunalUlcer_cambia'
+valueset "rp_biliopancreaticDiversionWithDuodenalSwitch_cambia": 'http://example.com/fhir/ValueSet/rp_biliopancreaticDiversionWithDuodenalSwitch_cambia'
+
+/* Duplicates */
+/*
+valueset "rp_sleeveGastrectomy_cambia": 'http://example.com/fhir/ValueSet/rp_sleeveGastrectomy_cambia'
+valueset "rp_initialAdjustableGastricBanding_cambia": 'http://example.com/fhir/ValueSet/rp_initialAdjustableGastricBanding_cambia'
+valueset "rp_initialGastricBypassRouxEnYAnastomosis_cambia": 'http://example.com/fhir/ValueSet/rp_initialGastricBypassRouxEnYAnastomosis_cambia'
+valueset "rp_initialGastricBypassRouxEnYAnastomosis_cambia": 'http://example.com/fhir/ValueSet/rp_initialGastricBypassRouxEnYAnastomosis_cambia'
+valueset "rp_sleeveGastrectomy_cambia": 'http://example.com/fhir/ValueSet/rp_sleeveGastrectomy_cambia'
+valueset "rp_initialAdjustableGastricBanding_cambia": 'http://example.com/fhir/ValueSet/rp_initialAdjustableGastricBanding_cambia'
+valueset "rp_initialGastricBypassRouxEnYAnastomosis_cambia": 'http://example.com/fhir/ValueSet/rp_initialGastricBypassRouxEnYAnastomosis_cambia'
+*/
+/* End of Duplicates */
+
+/*
+valueset "": 'http://example.com/fhir/ValueSet/'
+valueset "": 'http://example.com/fhir/ValueSet/'
+valueset "": 'http://example.com/fhir/ValueSet/'
+valueset "": 'http://example.com/fhir/ValueSet/'
+valueset "": 'http://example.com/fhir/ValueSet/'
+valueset "": 'http://example.com/fhir/ValueSet/'
+valueset "": 'http://example.com/fhir/ValueSet/'
+valueset "": 'http://example.com/fhir/ValueSet/'
+valueset "": 'http://example.com/fhir/ValueSet/'
+valueset "": 'http://example.com/fhir/ValueSet/'
+valueset "": 'http://example.com/fhir/ValueSet/'
+valueset "": 'http://example.com/fhir/ValueSet/'
+valueset "": 'http://example.com/fhir/ValueSet/'
+valueset "": 'http://example.com/fhir/ValueSet/'
+valueset "": 'http://example.com/fhir/ValueSet/'
+valueset "": 'http://example.com/fhir/ValueSet/'
+*/
+
+code "condition-confirmed": 'confirmed' from "ConditionVerificationStatusCodes" display 'confirmed'

Src/java/elm-fhir/src/test/resources/org/cqframework/cql/elm/requirements/fhir/WithDependencies/CommonElements.cql

@@ -0,0 +1,22 @@
+/* Context-independent Data Elements. (e.g., the Retrieves, to be used in more than one context.) */
+library CommonElements version '1.0.000'
+
+using FHIR version '4.0.1'
+
+include FHIRHelpers version '4.0.1' called FHIRHelpers
+include CommonConcepts version '1.0.000' called Cx
+
+context Patient
+
+/* Potentially "Common" elements */
+define "sVitalSigns":
+  [Observation: category in 'vital-sign']
+
+define function "Get Active Confirmed Conditions" (value List<Condition>) returns List<Condition>:
+  value C
+    where C.clinicalStatus in Cx."Active Condition"
+      and C.verificationStatus ~ ToConcept(Cx."condition-confirmed")
+
+define function "Get Qualified Observations" (value List<Observation>) returns List<Observation>:
+  value O
+    where O.status in { 'final', 'amended', 'corrected', 'appended' }