From 76a31365fce4febe4c9faeed496f2db2a1fb4953 Mon Sep 17 00:00:00 2001 From: Bryn Rhodes Date: Tue, 27 Sep 2022 12:31:06 -0600 Subject: [PATCH 1/2] #508,#552: Added support for profile validation in the retrieve #520: Fixed incorrect handling of null enumerations and primitives for FHIR-based models --- .../engine/fhir/model/FhirModelResolver.java | 14 ++++++ .../fhir/retrieve/FhirBundleCursor.java | 46 +++++++++++++++++-- .../fhir/model/TestDstu2ModelResolver.java | 31 +++++++++++-- .../fhir/model/TestDstu3ModelResolver.java | 27 ++++++++++- .../fhir/model/TestR4ModelResolver.java | 32 ++++++++++--- .../cql/engine/data/SystemDataProvider.java | 4 ++ pom.xml | 2 +- 7 files changed, 139 insertions(+), 17 deletions(-) diff --git a/engine.fhir/src/main/java/org/opencds/cqf/cql/engine/fhir/model/FhirModelResolver.java b/engine.fhir/src/main/java/org/opencds/cqf/cql/engine/fhir/model/FhirModelResolver.java index ab3c41d46..a81cc0a8b 100644 --- a/engine.fhir/src/main/java/org/opencds/cqf/cql/engine/fhir/model/FhirModelResolver.java +++ b/engine.fhir/src/main/java/org/opencds/cqf/cql/engine/fhir/model/FhirModelResolver.java @@ -415,6 +415,16 @@ protected Object resolveProperty(Object target, String path) { return null; } + // If the instance is a primitive (including (or even especially an enumeration), and it has no value, return null + if (child instanceof RuntimeChildPrimitiveDatatypeDefinition) { + IBase value = values.get(0); + if (value instanceof IPrimitiveType) { + if (!((IPrimitiveType)value).hasValue()) { + return null; + } + } + } + if (child instanceof RuntimeChildChoiceDefinition && !child.getElementName().equalsIgnoreCase(path)) { if (!values.get(0).getClass().getSimpleName() .equalsIgnoreCase(child.getChildByName(path).getImplementingClass().getSimpleName())) { @@ -630,6 +640,10 @@ public TemporalPrecisionEnum toTemporalPrecisionEnum(Precision precision) { */ public Object toJavaPrimitive(Object result, Object source) { + if (source instanceof IPrimitiveType && !((IPrimitiveType)source).hasValue()) { + return null; + } + String simpleName = source.getClass().getSimpleName(); switch (simpleName) { case "InstantType": diff --git a/engine.fhir/src/main/java/org/opencds/cqf/cql/engine/fhir/retrieve/FhirBundleCursor.java b/engine.fhir/src/main/java/org/opencds/cqf/cql/engine/fhir/retrieve/FhirBundleCursor.java index a5787103b..61d4c4883 100644 --- a/engine.fhir/src/main/java/org/opencds/cqf/cql/engine/fhir/retrieve/FhirBundleCursor.java +++ b/engine.fhir/src/main/java/org/opencds/cqf/cql/engine/fhir/retrieve/FhirBundleCursor.java @@ -1,10 +1,12 @@ package org.opencds.cqf.cql.engine.fhir.retrieve; +import java.util.ArrayList; import java.util.Iterator; import java.util.List; import org.hl7.fhir.instance.model.api.IBaseBundle; import org.hl7.fhir.instance.model.api.IBaseResource; +import org.hl7.fhir.instance.model.api.IPrimitiveType; import org.opencds.cqf.cql.engine.fhir.exception.UnknownElement; import ca.uhn.fhir.rest.client.api.IGenericClient; @@ -14,19 +16,25 @@ public class FhirBundleCursor implements Iterable { public FhirBundleCursor(IGenericClient fhirClient, IBaseBundle results) { - this(fhirClient, results, null); + this(fhirClient, results, null, null); } + public FhirBundleCursor(IGenericClient fhirClient, IBaseBundle results, String dataType) { this(fhirClient, results, null, null); } + // This constructor filters the bundle based on dataType - public FhirBundleCursor(IGenericClient fhirClient, IBaseBundle results, String dataType) { + // If templateId is provided, this is a trusted cursor, meaning that it will only return results + // for resources that declare they conform to the given profile + public FhirBundleCursor(IGenericClient fhirClient, IBaseBundle results, String dataType, String templateId) { this.fhirClient = fhirClient; this.results = results; this.dataType = dataType; + this.templateId = templateId; } private IGenericClient fhirClient; private IBaseBundle results; private String dataType; + private String templateId; /** @@ -35,15 +43,21 @@ public FhirBundleCursor(IGenericClient fhirClient, IBaseBundle results, String d * @return an Iterator. */ public Iterator iterator() { - return new FhirBundleIterator(fhirClient, results, dataType); + return new FhirBundleIterator(fhirClient, results, dataType, templateId); } private class FhirBundleIterator implements Iterator { - public FhirBundleIterator(IGenericClient fhirClient, IBaseBundle results, String dataType) { + public FhirBundleIterator(IGenericClient fhirClient, IBaseBundle results, String dataType, String templateId) { this.fhirClient = fhirClient; this.results = results; this.current = -1; this.dataType = dataType; + this.templateId = templateId; + + // Do not test templateId for base resource "profiles" + if (this.templateId != null && this.templateId.startsWith(String.format("http://hl7.org/fhir/StructureDefinition/%s", dataType))) { + this.templateId = null; + } if (dataType != null) { this.dataTypeClass = this.fhirClient.getFhirContext().getResourceDefinition(this.dataType).getImplementingClass(); @@ -56,6 +70,7 @@ public FhirBundleIterator(IGenericClient fhirClient, IBaseBundle results, String private IBaseBundle results; private int current; private String dataType; + private String templateId; private Class dataTypeClass; private List currentEntry; @@ -74,13 +89,33 @@ public boolean hasNext() { private List getEntry() { if (this.dataTypeClass != null) { - return BundleUtil.toListOfResourcesOfType(this.fhirClient.getFhirContext(), this.results, this.dataTypeClass); + List entries = BundleUtil.toListOfResourcesOfType(this.fhirClient.getFhirContext(), this.results, this.dataTypeClass); + if (templateId != null) { + return getTrustedEntries(entries, templateId); + } + else { + return entries; + } } else { return BundleUtil.toListOfResources(this.fhirClient.getFhirContext(), this.results); } } + private List getTrustedEntries(List entries, String templateId) { + List trustedEntries = new ArrayList(); + for (IBaseResource entry : entries) { + if (entry.getMeta() != null && entry.getMeta().getProfile() != null) { + for (IPrimitiveType profile : entry.getMeta().getProfile()) { + if (profile.hasValue() && profile.getValueAsString().equals(templateId)) { + trustedEntries.add(entry); + } + } + } + } + return trustedEntries; + } + private String getLink() { return BundleUtil.getLinkUrlOfType(this.fhirClient.getFhirContext(), this.results, IBaseBundle.LINK_NEXT); } @@ -105,6 +140,7 @@ public Object next() { } // TODO: It would be possible to get here if the next link was present, but the returned page had 0 entries... + // NOTE: This is especially true if the page has only data that is not conformant to the given profile throw new UnknownElement("The iteration has no more elements."); } } diff --git a/engine.fhir/src/test/java/org/opencds/cqf/cql/engine/fhir/model/TestDstu2ModelResolver.java b/engine.fhir/src/test/java/org/opencds/cqf/cql/engine/fhir/model/TestDstu2ModelResolver.java index 72e9fe32b..49ef9c193 100644 --- a/engine.fhir/src/test/java/org/opencds/cqf/cql/engine/fhir/model/TestDstu2ModelResolver.java +++ b/engine.fhir/src/test/java/org/opencds/cqf/cql/engine/fhir/model/TestDstu2ModelResolver.java @@ -5,16 +5,18 @@ import static org.testng.Assert.assertTrue; import java.lang.reflect.Field; +import java.math.BigDecimal; import java.util.ArrayList; import java.util.List; + import org.cqframework.cql.cql2elm.ModelManager; import org.cqframework.cql.cql2elm.model.Model; import org.hl7.elm.r1.VersionedIdentifier; import org.hl7.elm_modelinfo.r1.ClassInfo; import org.hl7.elm_modelinfo.r1.TypeInfo; -import org.hl7.fhir.dstu2.model.EnumFactory; -import org.hl7.fhir.dstu2.model.Enumeration; + +import org.hl7.fhir.dstu2.model.*; import org.hl7.fhir.dstu2.model.Enumerations.AdministrativeGender; import org.hl7.fhir.dstu2.model.Enumerations.AgeUnits; import org.hl7.fhir.dstu2.model.Enumerations.BindingStrength; @@ -29,7 +31,6 @@ import org.hl7.fhir.dstu2.model.Enumerations.ResourceType; import org.hl7.fhir.dstu2.model.Enumerations.SearchParamType; import org.hl7.fhir.dstu2.model.Enumerations.SpecialValues; -import org.hl7.fhir.dstu2.model.Patient; import org.opencds.cqf.cql.engine.fhir.exception.UnknownType; import org.opencds.cqf.cql.engine.model.ModelResolver; import org.testng.annotations.Test; @@ -232,4 +233,28 @@ public void resolveMissingPropertyReturnsNull() { Object result = resolver.resolvePath(p, "not-a-path"); assertNull(result); } + + + //@Test + public void resolveNullEnumerationReturnsNull() { + FhirModelResolver resolver = new Dstu2FhirModelResolver(); + + Quantity q = new Quantity(); + q.setValue(new BigDecimal("10.0")); + q.setUnit("1"); + SimpleQuantity sq = resolver.castToSimpleQuantity(q); + + Object result = resolver.resolvePath(sq, "comparator"); + assertNull(result); + } + + //@Test + public void resolveNullPrimitiveReturnsNull() { + FhirModelResolver resolver = new Dstu2FhirModelResolver(); + + DateTimeType dt = new DateTimeType(); + + Object result = resolver.resolvePath(dt, "value"); + assertNull(result); + } } diff --git a/engine.fhir/src/test/java/org/opencds/cqf/cql/engine/fhir/model/TestDstu3ModelResolver.java b/engine.fhir/src/test/java/org/opencds/cqf/cql/engine/fhir/model/TestDstu3ModelResolver.java index c59419dee..87f82c23f 100644 --- a/engine.fhir/src/test/java/org/opencds/cqf/cql/engine/fhir/model/TestDstu3ModelResolver.java +++ b/engine.fhir/src/test/java/org/opencds/cqf/cql/engine/fhir/model/TestDstu3ModelResolver.java @@ -4,6 +4,7 @@ import static org.testng.Assert.assertNull; import static org.testng.Assert.assertTrue; +import java.math.BigDecimal; import java.util.ArrayList; import java.util.List; @@ -12,7 +13,7 @@ import org.hl7.elm.r1.VersionedIdentifier; import org.hl7.elm_modelinfo.r1.ClassInfo; import org.hl7.elm_modelinfo.r1.TypeInfo; -import org.hl7.fhir.dstu3.model.Enumeration; +import org.hl7.fhir.dstu3.model.*; import org.hl7.fhir.dstu3.model.Enumerations.AbstractType; import org.hl7.fhir.dstu3.model.Enumerations.AdministrativeGender; import org.hl7.fhir.dstu3.model.Enumerations.AgeUnits; @@ -30,7 +31,6 @@ import org.hl7.fhir.dstu3.model.Enumerations.ResourceType; import org.hl7.fhir.dstu3.model.Enumerations.SearchParamType; import org.hl7.fhir.dstu3.model.Enumerations.SpecialValues; -import org.hl7.fhir.dstu3.model.Patient; import org.opencds.cqf.cql.engine.fhir.exception.UnknownType; import org.opencds.cqf.cql.engine.model.ModelResolver; import org.testng.annotations.Test; @@ -262,4 +262,27 @@ public void resolveMissingPropertyReturnsNull() { Object result = resolver.resolvePath(p, "not-a-path"); assertNull(result); } + + @Test + public void resolveNullEnumerationReturnsNull() { + FhirModelResolver resolver = new Dstu3FhirModelResolver(); + + Quantity q = new Quantity(); + q.setValue(new BigDecimal("10.0")); + q.setUnit("1"); + SimpleQuantity sq = resolver.castToSimpleQuantity(q); + + Object result = resolver.resolvePath(sq, "comparator"); + assertNull(result); + } + + @Test + public void resolveNullPrimitiveReturnsNull() { + FhirModelResolver resolver = new Dstu3FhirModelResolver(); + + DateTimeType dt = new DateTimeType(); + + Object result = resolver.resolvePath(dt, "value"); + assertNull(result); + } } diff --git a/engine.fhir/src/test/java/org/opencds/cqf/cql/engine/fhir/model/TestR4ModelResolver.java b/engine.fhir/src/test/java/org/opencds/cqf/cql/engine/fhir/model/TestR4ModelResolver.java index 20db16962..5b3a7c9c7 100644 --- a/engine.fhir/src/test/java/org/opencds/cqf/cql/engine/fhir/model/TestR4ModelResolver.java +++ b/engine.fhir/src/test/java/org/opencds/cqf/cql/engine/fhir/model/TestR4ModelResolver.java @@ -6,6 +6,7 @@ import static org.testng.Assert.assertNull; import static org.testng.Assert.assertTrue; +import java.math.BigDecimal; import java.util.ArrayList; import java.util.Date; import java.util.GregorianCalendar; @@ -16,8 +17,7 @@ import org.hl7.elm.r1.VersionedIdentifier; import org.hl7.elm_modelinfo.r1.ClassInfo; import org.hl7.elm_modelinfo.r1.TypeInfo; -import org.hl7.fhir.r4.model.DateTimeType; -import org.hl7.fhir.r4.model.Enumeration; +import org.hl7.fhir.r4.model.*; import org.hl7.fhir.r4.model.Enumerations.AbstractType; import org.hl7.fhir.r4.model.Enumerations.AdministrativeGender; import org.hl7.fhir.r4.model.Enumerations.AgeUnits; @@ -40,9 +40,6 @@ import org.hl7.fhir.r4.model.Enumerations.ResourceType; import org.hl7.fhir.r4.model.Enumerations.SearchParamType; import org.hl7.fhir.r4.model.Enumerations.SpecialValues; -import org.hl7.fhir.r4.model.IdType; -import org.hl7.fhir.r4.model.Patient; -import org.hl7.fhir.r4.model.VisionPrescription; import org.opencds.cqf.cql.engine.fhir.exception.UnknownType; import org.opencds.cqf.cql.engine.model.ModelResolver; import org.testng.annotations.Test; @@ -164,7 +161,7 @@ public void modelInfoSpecialCaseTests() { public void modelInfo400Tests() { ModelResolver resolver = new R4FhirModelResolver(); ModelManager mm = new ModelManager(); - Model m = mm.resolveModel(new VersionedIdentifier().withId("FHIR").withVersion("4.0.0")); + Model m = mm.resolveModel(new VersionedIdentifier().withId("FHIR").withVersion("4.0.1")); List typeInfos = m.getModelInfo().getTypeInfo(); @@ -378,4 +375,27 @@ public void resolveDateTimeProviderReturnsDate() { assertNotNull(result); assertThat(result, is(dateTimeType)); } + + @Test + public void resolveNullEnumerationReturnsNull() { + FhirModelResolver resolver = new R4FhirModelResolver(); + + Quantity q = new Quantity(); + q.setValue(new BigDecimal("10.0")); + q.setUnit("1"); + SimpleQuantity sq = resolver.castToSimpleQuantity(q); + + Object result = resolver.resolvePath(sq, "comparator"); + assertNull(result); + } + + @Test + public void resolveNullPrimitiveReturnsNull() { + FhirModelResolver resolver = new R4FhirModelResolver(); + + DateTimeType dt = new DateTimeType(); + + Object result = resolver.resolvePath(dt, "value"); + assertNull(result); + } } diff --git a/engine/src/main/java/org/opencds/cqf/cql/engine/data/SystemDataProvider.java b/engine/src/main/java/org/opencds/cqf/cql/engine/data/SystemDataProvider.java index a66f64ebf..dcc005f51 100644 --- a/engine/src/main/java/org/opencds/cqf/cql/engine/data/SystemDataProvider.java +++ b/engine/src/main/java/org/opencds/cqf/cql/engine/data/SystemDataProvider.java @@ -41,6 +41,10 @@ private Field getProperty(Class clazz, String path) { Field field = clazz.getDeclaredField(path); return field; } catch (NoSuchFieldException e) { + if (clazz.getSuperclass() != null) { + Field field = getProperty(clazz.getSuperclass(), path); + return field; + } throw new IllegalArgumentException(String.format("Could not determine field for path %s of type %s", path, clazz.getSimpleName())); } } diff --git a/pom.xml b/pom.xml index 7e4b0ac52..83aeb1f96 100644 --- a/pom.xml +++ b/pom.xml @@ -50,7 +50,7 @@ UTF-8 2.13.2 2.13.2.1 - 1.5.11 + 1.5.12-SNAPSHOT 5.6.3 1.7.29 From c83ba6031b979ce953ee5011cf474c3349ac252e Mon Sep 17 00:00:00 2001 From: Bryn Rhodes Date: Tue, 27 Sep 2022 12:47:53 -0600 Subject: [PATCH 2/2] Add v15 branch to release scripting --- .travis.yml | 2 +- scripts/deploy.sh | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.travis.yml b/.travis.yml index 7fe9beb72..26287fe33 100644 --- a/.travis.yml +++ b/.travis.yml @@ -13,7 +13,7 @@ script: before_cache: - rm -rf $HOME/.m2/repository/org/opencds/cqf/cql after_success: -- test $TRAVIS_BRANCH = "master" && test $TRAVIS_PULL_REQUEST = "false" && ./scripts/deploy.sh +- test ($TRAVIS_BRANCH = "master" || $TRAVIS_BRANCH = "v15") && test $TRAVIS_PULL_REQUEST = "false" && ./scripts/deploy.sh deploy: provider: releases api_key: diff --git a/scripts/deploy.sh b/scripts/deploy.sh index 82c4f6ad5..d3e22ab0a 100755 --- a/scripts/deploy.sh +++ b/scripts/deploy.sh @@ -4,9 +4,9 @@ set -euxo pipefail bash -n "$0" -if [[ "$TRAVIS_BRANCH" != "master" && -z "$TRAVIS_TAG" ]] +if [[ "$TRAVIS_BRANCH" != "master" && "$TRAVIS_BRANCH" != "v15" && -z "$TRAVIS_TAG" ]] then - echo "Not on the master branch or a git tag. Skipping deploy." + echo "Not on the master or v15 branches or a git tag. Skipping deploy." exit 0 fi