diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_1_0/1895-fix-jpa-validation-performance-regression.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_1_0/1895-fix-jpa-validation-performance-regression.yaml index d7c082340b9..a6f7ebe888d 100644 --- a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_1_0/1895-fix-jpa-validation-performance-regression.yaml +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_1_0/1895-fix-jpa-validation-performance-regression.yaml @@ -1,5 +1,5 @@ --- type: fix issue: 1895 -title: "HAPI FHIR 5.0.0 introduced a regressin in JPA validator performance, where a number of unnecessary database lookups +title: "HAPI FHIR 5.0.0 introduced a regression in JPA validator performance, where a number of unnecessary database lookups were introduced. This has been corrected." diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_1_0/1933-fix-valueset-expand-with-offset-and-count-parameters.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_1_0/1933-fix-valueset-expand-with-offset-and-count-parameters.yaml new file mode 100644 index 00000000000..4e9d7a3aaf0 --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_1_0/1933-fix-valueset-expand-with-offset-and-count-parameters.yaml @@ -0,0 +1,6 @@ +--- +type: fix +issue: 1933 +title: "`BaseValidationSupportWrapper.expandValueSet(...)` and `ValidationSupportChain.expandValueSet(...)` were + incorrectly replacing expansion options (i.e. offset and count) with `null`, causing these parameters to be + ignored when invoking the `ValueSet$expand` operation. This has been corrected." diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r5/ResourceProviderR5ValueSetTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r5/ResourceProviderR5ValueSetTest.java index b7dd83f9dc9..58a6bf69e5d 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r5/ResourceProviderR5ValueSetTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r5/ResourceProviderR5ValueSetTest.java @@ -301,6 +301,103 @@ public class ResourceProviderR5ValueSetTest extends BaseResourceProviderR5Test { } + @Test + public void testExpandByIdWithPreExpansionWithOffset() throws Exception { + myDaoConfig.setPreExpandValueSets(true); + + loadAndPersistCodeSystemAndValueSet(HTTPVerb.POST); + myTermSvc.preExpandDeferredValueSetsToTerminologyTables(); + + Parameters respParam = ourClient + .operation() + .onInstance(myExtensionalVsId) + .named("expand") + .withParameter(Parameters.class, "offset", new IntegerType(1)) + .execute(); + ValueSet expanded = (ValueSet) respParam.getParameter().get(0).getResource(); + + String resp = myFhirCtx.newXmlParser().setPrettyPrint(true).encodeResourceToString(expanded); + ourLog.info(resp); + + assertEquals(24, expanded.getExpansion().getTotal()); + assertEquals(1, expanded.getExpansion().getOffset()); + assertEquals("offset", expanded.getExpansion().getParameter().get(0).getName()); + assertEquals(1, expanded.getExpansion().getParameter().get(0).getValueIntegerType().getValue().intValue()); + assertEquals("count", expanded.getExpansion().getParameter().get(1).getName()); + assertEquals(1000, expanded.getExpansion().getParameter().get(1).getValueIntegerType().getValue().intValue()); + assertEquals(23, expanded.getExpansion().getContains().size()); + assertEquals("http://acme.org", expanded.getExpansion().getContains().get(0).getSystem()); + assertEquals("11378-7", expanded.getExpansion().getContains().get(0).getCode()); + assertEquals("Systolic blood pressure at First encounter", expanded.getExpansion().getContains().get(0).getDisplay()); + assertEquals("http://acme.org", expanded.getExpansion().getContains().get(1).getSystem()); + assertEquals("8493-9", expanded.getExpansion().getContains().get(1).getCode()); + assertEquals("Systolic blood pressure 10 hour minimum", expanded.getExpansion().getContains().get(1).getDisplay()); + + } + + @Test + public void testExpandByIdWithPreExpansionWithCount() throws Exception { + myDaoConfig.setPreExpandValueSets(true); + + loadAndPersistCodeSystemAndValueSet(HTTPVerb.POST); + myTermSvc.preExpandDeferredValueSetsToTerminologyTables(); + + Parameters respParam = ourClient + .operation() + .onInstance(myExtensionalVsId) + .named("expand") + .withParameter(Parameters.class, "count", new IntegerType(1)) + .execute(); + ValueSet expanded = (ValueSet) respParam.getParameter().get(0).getResource(); + + String resp = myFhirCtx.newXmlParser().setPrettyPrint(true).encodeResourceToString(expanded); + ourLog.info(resp); + + assertEquals(24, expanded.getExpansion().getTotal()); + assertEquals(0, expanded.getExpansion().getOffset()); + assertEquals("offset", expanded.getExpansion().getParameter().get(0).getName()); + assertEquals(0, expanded.getExpansion().getParameter().get(0).getValueIntegerType().getValue().intValue()); + assertEquals("count", expanded.getExpansion().getParameter().get(1).getName()); + assertEquals(1, expanded.getExpansion().getParameter().get(1).getValueIntegerType().getValue().intValue()); + assertEquals(1, expanded.getExpansion().getContains().size()); + assertEquals("http://acme.org", expanded.getExpansion().getContainsFirstRep().getSystem()); + assertEquals("8450-9", expanded.getExpansion().getContainsFirstRep().getCode()); + assertEquals("Systolic blood pressure--expiration", expanded.getExpansion().getContainsFirstRep().getDisplay()); + + } + + @Test + public void testExpandByIdWithPreExpansionWithOffsetAndCount() throws Exception { + myDaoConfig.setPreExpandValueSets(true); + + loadAndPersistCodeSystemAndValueSet(HTTPVerb.POST); + myTermSvc.preExpandDeferredValueSetsToTerminologyTables(); + + Parameters respParam = ourClient + .operation() + .onInstance(myExtensionalVsId) + .named("expand") + .withParameter(Parameters.class, "offset", new IntegerType(1)) + .andParameter("count", new IntegerType(1)) + .execute(); + ValueSet expanded = (ValueSet) respParam.getParameter().get(0).getResource(); + + String resp = myFhirCtx.newXmlParser().setPrettyPrint(true).encodeResourceToString(expanded); + ourLog.info(resp); + + assertEquals(24, expanded.getExpansion().getTotal()); + assertEquals(1, expanded.getExpansion().getOffset()); + assertEquals("offset", expanded.getExpansion().getParameter().get(0).getName()); + assertEquals(1, expanded.getExpansion().getParameter().get(0).getValueIntegerType().getValue().intValue()); + assertEquals("count", expanded.getExpansion().getParameter().get(1).getName()); + assertEquals(1, expanded.getExpansion().getParameter().get(1).getValueIntegerType().getValue().intValue()); + assertEquals(1, expanded.getExpansion().getContains().size()); + assertEquals("http://acme.org", expanded.getExpansion().getContainsFirstRep().getSystem()); + assertEquals("11378-7", expanded.getExpansion().getContainsFirstRep().getCode()); + assertEquals("Systolic blood pressure at First encounter", expanded.getExpansion().getContainsFirstRep().getDisplay()); + + } + @Test public void testExpandByIdWithFilter() throws Exception { loadAndPersistCodeSystemAndValueSet(HTTPVerb.POST); @@ -402,6 +499,106 @@ public class ResourceProviderR5ValueSetTest extends BaseResourceProviderR5Test { } + @Test + public void testExpandByUrlWithPreExpansionWithOffset() throws Exception { + myDaoConfig.setPreExpandValueSets(true); + + loadAndPersistCodeSystemAndValueSet(HTTPVerb.POST); + myTermSvc.preExpandDeferredValueSetsToTerminologyTables(); + + Parameters respParam = ourClient + .operation() + .onType(ValueSet.class) + .named("expand") + .withParameter(Parameters.class, "url", new UriType("http://www.healthintersections.com.au/fhir/ValueSet/extensional-case-2")) + .andParameter("offset", new IntegerType(1)) + .execute(); + ValueSet expanded = (ValueSet) respParam.getParameter().get(0).getResource(); + + String resp = myFhirCtx.newXmlParser().setPrettyPrint(true).encodeResourceToString(expanded); + ourLog.info(resp); + + assertEquals(24, expanded.getExpansion().getTotal()); + assertEquals(1, expanded.getExpansion().getOffset()); + assertEquals("offset", expanded.getExpansion().getParameter().get(0).getName()); + assertEquals(1, expanded.getExpansion().getParameter().get(0).getValueIntegerType().getValue().intValue()); + assertEquals("count", expanded.getExpansion().getParameter().get(1).getName()); + assertEquals(1000, expanded.getExpansion().getParameter().get(1).getValueIntegerType().getValue().intValue()); + assertEquals(23, expanded.getExpansion().getContains().size()); + assertEquals("http://acme.org", expanded.getExpansion().getContains().get(0).getSystem()); + assertEquals("11378-7", expanded.getExpansion().getContains().get(0).getCode()); + assertEquals("Systolic blood pressure at First encounter", expanded.getExpansion().getContains().get(0).getDisplay()); + assertEquals("http://acme.org", expanded.getExpansion().getContains().get(1).getSystem()); + assertEquals("8493-9", expanded.getExpansion().getContains().get(1).getCode()); + assertEquals("Systolic blood pressure 10 hour minimum", expanded.getExpansion().getContains().get(1).getDisplay()); + + } + + @Test + public void testExpandByUrlWithPreExpansionWithCount() throws Exception { + myDaoConfig.setPreExpandValueSets(true); + + loadAndPersistCodeSystemAndValueSet(HTTPVerb.POST); + myTermSvc.preExpandDeferredValueSetsToTerminologyTables(); + + Parameters respParam = ourClient + .operation() + .onType(ValueSet.class) + .named("expand") + .withParameter(Parameters.class, "url", new UriType("http://www.healthintersections.com.au/fhir/ValueSet/extensional-case-2")) + .andParameter("count", new IntegerType(1)) + .execute(); + ValueSet expanded = (ValueSet) respParam.getParameter().get(0).getResource(); + + String resp = myFhirCtx.newXmlParser().setPrettyPrint(true).encodeResourceToString(expanded); + ourLog.info(resp); + + assertEquals(24, expanded.getExpansion().getTotal()); + assertEquals(0, expanded.getExpansion().getOffset()); + assertEquals("offset", expanded.getExpansion().getParameter().get(0).getName()); + assertEquals(0, expanded.getExpansion().getParameter().get(0).getValueIntegerType().getValue().intValue()); + assertEquals("count", expanded.getExpansion().getParameter().get(1).getName()); + assertEquals(1, expanded.getExpansion().getParameter().get(1).getValueIntegerType().getValue().intValue()); + assertEquals(1, expanded.getExpansion().getContains().size()); + assertEquals("http://acme.org", expanded.getExpansion().getContainsFirstRep().getSystem()); + assertEquals("8450-9", expanded.getExpansion().getContainsFirstRep().getCode()); + assertEquals("Systolic blood pressure--expiration", expanded.getExpansion().getContainsFirstRep().getDisplay()); + + } + + @Test + public void testExpandByUrlWithPreExpansionWithOffsetAndCount() throws Exception { + myDaoConfig.setPreExpandValueSets(true); + + loadAndPersistCodeSystemAndValueSet(HTTPVerb.POST); + myTermSvc.preExpandDeferredValueSetsToTerminologyTables(); + + Parameters respParam = ourClient + .operation() + .onType(ValueSet.class) + .named("expand") + .withParameter(Parameters.class, "url", new UriType("http://www.healthintersections.com.au/fhir/ValueSet/extensional-case-2")) + .andParameter("offset", new IntegerType(1)) + .andParameter("count", new IntegerType(1)) + .execute(); + ValueSet expanded = (ValueSet) respParam.getParameter().get(0).getResource(); + + String resp = myFhirCtx.newXmlParser().setPrettyPrint(true).encodeResourceToString(expanded); + ourLog.info(resp); + + assertEquals(24, expanded.getExpansion().getTotal()); + assertEquals(1, expanded.getExpansion().getOffset()); + assertEquals("offset", expanded.getExpansion().getParameter().get(0).getName()); + assertEquals(1, expanded.getExpansion().getParameter().get(0).getValueIntegerType().getValue().intValue()); + assertEquals("count", expanded.getExpansion().getParameter().get(1).getName()); + assertEquals(1, expanded.getExpansion().getParameter().get(1).getValueIntegerType().getValue().intValue()); + assertEquals(1, expanded.getExpansion().getContains().size()); + assertEquals("http://acme.org", expanded.getExpansion().getContainsFirstRep().getSystem()); + assertEquals("11378-7", expanded.getExpansion().getContainsFirstRep().getCode()); + assertEquals("Systolic blood pressure at First encounter", expanded.getExpansion().getContainsFirstRep().getDisplay()); + + } + @Test public void testExpandByUrlWithPreExpansionAndBogusUrl() throws Exception { myDaoConfig.setPreExpandValueSets(true); @@ -448,7 +645,7 @@ public class ResourceProviderR5ValueSetTest extends BaseResourceProviderR5Test { public void testExpandByValueSetWithPreExpansion() throws IOException { myDaoConfig.setPreExpandValueSets(true); - loadAndPersistCodeSystem(HTTPVerb.POST); + loadAndPersistCodeSystemAndValueSet(HTTPVerb.POST); myTermSvc.preExpandDeferredValueSetsToTerminologyTables(); ValueSet toExpand = loadResourceFromClasspath(ValueSet.class, "/extensional-case-3-vs.xml"); @@ -469,6 +666,112 @@ public class ResourceProviderR5ValueSetTest extends BaseResourceProviderR5Test { } + @Test + public void testExpandByValueSetWithPreExpansionWithOffset() throws IOException { + myDaoConfig.setPreExpandValueSets(true); + + loadAndPersistCodeSystemAndValueSet(HTTPVerb.POST); + myTermSvc.preExpandDeferredValueSetsToTerminologyTables(); + + ValueSet toExpand = loadResourceFromClasspath(ValueSet.class, "/extensional-case-3-vs.xml"); + + Parameters respParam = ourClient + .operation() + .onType(ValueSet.class) + .named("expand") + .withParameter(Parameters.class, "valueSet", toExpand) + .andParameter("offset", new IntegerType(1)) + .execute(); + ValueSet expanded = (ValueSet) respParam.getParameter().get(0).getResource(); + + String resp = myFhirCtx.newXmlParser().setPrettyPrint(true).encodeResourceToString(expanded); + ourLog.info(resp); + + assertEquals(24, expanded.getExpansion().getTotal()); + assertEquals(1, expanded.getExpansion().getOffset()); + assertEquals("offset", expanded.getExpansion().getParameter().get(0).getName()); + assertEquals(1, expanded.getExpansion().getParameter().get(0).getValueIntegerType().getValue().intValue()); + assertEquals("count", expanded.getExpansion().getParameter().get(1).getName()); + assertEquals(1000, expanded.getExpansion().getParameter().get(1).getValueIntegerType().getValue().intValue()); + assertEquals(23, expanded.getExpansion().getContains().size()); + assertEquals("http://acme.org", expanded.getExpansion().getContains().get(0).getSystem()); + assertEquals("11378-7", expanded.getExpansion().getContains().get(0).getCode()); + assertEquals("Systolic blood pressure at First encounter", expanded.getExpansion().getContains().get(0).getDisplay()); + assertEquals("http://acme.org", expanded.getExpansion().getContains().get(1).getSystem()); + assertEquals("8493-9", expanded.getExpansion().getContains().get(1).getCode()); + assertEquals("Systolic blood pressure 10 hour minimum", expanded.getExpansion().getContains().get(1).getDisplay()); + + } + + @Test + public void testExpandByValueSetWithPreExpansionWithCount() throws IOException { + myDaoConfig.setPreExpandValueSets(true); + + loadAndPersistCodeSystemAndValueSet(HTTPVerb.POST); + myTermSvc.preExpandDeferredValueSetsToTerminologyTables(); + + ValueSet toExpand = loadResourceFromClasspath(ValueSet.class, "/extensional-case-3-vs.xml"); + + Parameters respParam = ourClient + .operation() + .onType(ValueSet.class) + .named("expand") + .withParameter(Parameters.class, "valueSet", toExpand) + .andParameter("count", new IntegerType(1)) + .execute(); + ValueSet expanded = (ValueSet) respParam.getParameter().get(0).getResource(); + + String resp = myFhirCtx.newXmlParser().setPrettyPrint(true).encodeResourceToString(expanded); + ourLog.info(resp); + + assertEquals(24, expanded.getExpansion().getTotal()); + assertEquals(0, expanded.getExpansion().getOffset()); + assertEquals("offset", expanded.getExpansion().getParameter().get(0).getName()); + assertEquals(0, expanded.getExpansion().getParameter().get(0).getValueIntegerType().getValue().intValue()); + assertEquals("count", expanded.getExpansion().getParameter().get(1).getName()); + assertEquals(1, expanded.getExpansion().getParameter().get(1).getValueIntegerType().getValue().intValue()); + assertEquals(1, expanded.getExpansion().getContains().size()); + assertEquals("http://acme.org", expanded.getExpansion().getContainsFirstRep().getSystem()); + assertEquals("8450-9", expanded.getExpansion().getContainsFirstRep().getCode()); + assertEquals("Systolic blood pressure--expiration", expanded.getExpansion().getContainsFirstRep().getDisplay()); + + } + + @Test + public void testExpandByValueSetWithPreExpansionWithOffsetAndCount() throws IOException { + myDaoConfig.setPreExpandValueSets(true); + + loadAndPersistCodeSystemAndValueSet(HTTPVerb.POST); + myTermSvc.preExpandDeferredValueSetsToTerminologyTables(); + + ValueSet toExpand = loadResourceFromClasspath(ValueSet.class, "/extensional-case-3-vs.xml"); + + Parameters respParam = ourClient + .operation() + .onType(ValueSet.class) + .named("expand") + .withParameter(Parameters.class, "valueSet", toExpand) + .andParameter("offset", new IntegerType(1)) + .andParameter("count", new IntegerType(1)) + .execute(); + ValueSet expanded = (ValueSet) respParam.getParameter().get(0).getResource(); + + String resp = myFhirCtx.newXmlParser().setPrettyPrint(true).encodeResourceToString(expanded); + ourLog.info(resp); + + assertEquals(24, expanded.getExpansion().getTotal()); + assertEquals(1, expanded.getExpansion().getOffset()); + assertEquals("offset", expanded.getExpansion().getParameter().get(0).getName()); + assertEquals(1, expanded.getExpansion().getParameter().get(0).getValueIntegerType().getValue().intValue()); + assertEquals("count", expanded.getExpansion().getParameter().get(1).getName()); + assertEquals(1, expanded.getExpansion().getParameter().get(1).getValueIntegerType().getValue().intValue()); + assertEquals(1, expanded.getExpansion().getContains().size()); + assertEquals("http://acme.org", expanded.getExpansion().getContainsFirstRep().getSystem()); + assertEquals("11378-7", expanded.getExpansion().getContainsFirstRep().getCode()); + assertEquals("Systolic blood pressure at First encounter", expanded.getExpansion().getContainsFirstRep().getDisplay()); + + } + @Test public void testExpandInlineVsAgainstBuiltInCs() { createLocalVsPointingAtBuiltInCodeSystem(); diff --git a/hapi-fhir-validation/src/main/java/org/hl7/fhir/common/hapi/validation/support/BaseValidationSupportWrapper.java b/hapi-fhir-validation/src/main/java/org/hl7/fhir/common/hapi/validation/support/BaseValidationSupportWrapper.java index e18e1918f14..cc04a447d10 100644 --- a/hapi-fhir-validation/src/main/java/org/hl7/fhir/common/hapi/validation/support/BaseValidationSupportWrapper.java +++ b/hapi-fhir-validation/src/main/java/org/hl7/fhir/common/hapi/validation/support/BaseValidationSupportWrapper.java @@ -70,7 +70,7 @@ public class BaseValidationSupportWrapper extends BaseValidationSupport { @Override public IValidationSupport.ValueSetExpansionOutcome expandValueSet(ValidationSupportContext theValidationSupportContext, ValueSetExpansionOptions theExpansionOptions, IBaseResource theValueSetToExpand) { - return myWrap.expandValueSet(theValidationSupportContext, null, theValueSetToExpand); + return myWrap.expandValueSet(theValidationSupportContext, theExpansionOptions, theValueSetToExpand); } @Override diff --git a/hapi-fhir-validation/src/main/java/org/hl7/fhir/common/hapi/validation/support/ValidationSupportChain.java b/hapi-fhir-validation/src/main/java/org/hl7/fhir/common/hapi/validation/support/ValidationSupportChain.java index 3076d31826d..20a667e0b96 100644 --- a/hapi-fhir-validation/src/main/java/org/hl7/fhir/common/hapi/validation/support/ValidationSupportChain.java +++ b/hapi-fhir-validation/src/main/java/org/hl7/fhir/common/hapi/validation/support/ValidationSupportChain.java @@ -134,7 +134,7 @@ public class ValidationSupportChain implements IValidationSupport { public ValueSetExpansionOutcome expandValueSet(ValidationSupportContext theValidationSupportContext, ValueSetExpansionOptions theExpansionOptions, IBaseResource theValueSetToExpand) { for (IValidationSupport next : myChain) { // TODO: test if code system is supported? - ValueSetExpansionOutcome expanded = next.expandValueSet(theValidationSupportContext, null, theValueSetToExpand); + ValueSetExpansionOutcome expanded = next.expandValueSet(theValidationSupportContext, theExpansionOptions, theValueSetToExpand); if (expanded != null) { return expanded; }