From 7e76c1893f50d470fa049b3d1b0c569ccf059cfa Mon Sep 17 00:00:00 2001 From: Diederik Muylwyk Date: Mon, 9 Sep 2019 18:41:00 -0400 Subject: [PATCH] Fixed bug for large ValueSet expansion with URL parameter; return 404 for unknown ValueSets. --- .../jpa/term/BaseHapiTerminologySvcImpl.java | 5 +- .../r4/ResourceProviderR4ValueSetTest.java | 49 ++++++++++++++++--- .../jpa/term/TerminologySvcImplR4Test.java | 9 +--- 3 files changed, 44 insertions(+), 19 deletions(-) diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/BaseHapiTerminologySvcImpl.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/BaseHapiTerminologySvcImpl.java index fe562f4298c..ea2ee08dd6e 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/BaseHapiTerminologySvcImpl.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/BaseHapiTerminologySvcImpl.java @@ -488,7 +488,7 @@ public abstract class BaseHapiTerminologySvcImpl implements IHapiTerminologySvc, } if (!optionalTermValueSet.isPresent()) { - ourLog.warn("ValueSet is not present in terminology tables. Will perform in-memory expansion without parameters. Will schedule this ValueSet for pre-expansion. {}", getValueSetInfo(theValueSetToExpand)); + ourLog.warn("ValueSet is not present in terminology tables. Will perform in-memory expansion without parameters. {}", getValueSetInfo(theValueSetToExpand)); return expandValueSet(theValueSetToExpand); // In-memory expansion. } @@ -947,8 +947,7 @@ public abstract class BaseHapiTerminologySvcImpl implements IHapiTerminologySvc, Optional optionalTermValueSet = myValueSetDao.findByResourcePid(valueSetResourcePid); if (!optionalTermValueSet.isPresent()) { - ourLog.warn("ValueSet is not present in terminology tables. Will perform in-memory code validation. Will schedule this ValueSet for pre-expansion. {}", getValueSetInfo(theValueSet)); - myDeferredValueSets.add(theValueSet); + ourLog.warn("ValueSet is not present in terminology tables. Will perform in-memory code validation. {}", getValueSetInfo(theValueSet)); return false; } diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderR4ValueSetTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderR4ValueSetTest.java index e29ae2a327b..207e54474d2 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderR4ValueSetTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderR4ValueSetTest.java @@ -9,6 +9,7 @@ import ca.uhn.fhir.jpa.entity.TermConceptParentChildLink.RelationshipTypeEnum; import ca.uhn.fhir.jpa.model.entity.ResourceTable; import ca.uhn.fhir.jpa.term.IHapiTerminologySvc; import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; +import ca.uhn.fhir.rest.server.exceptions.ResourceNotFoundException; import ca.uhn.fhir.rest.server.servlet.ServletRequestDetails; import ca.uhn.fhir.util.TestUtil; import ca.uhn.fhir.util.UrlUtil; @@ -311,8 +312,7 @@ public class ResourceProviderR4ValueSetTest extends BaseResourceProviderR4Test { .operation() .onType(ValueSet.class) .named("expand") -// .withParameter(Parameters.class, "url", new UriType("http://www.healthintersections.com.au/fhir/ValueSet/extensional-case-2")) //FIXME: DM - .withParameter(Parameters.class, "url", new UriType("http://www.healthintersections.com.au/fhir/ValueSet/bogus")) + .withParameter(Parameters.class, "url", new UriType("http://www.healthintersections.com.au/fhir/ValueSet/extensional-case-2")) .execute(); ValueSet expanded = (ValueSet) respParam.getParameter().get(0).getResource(); @@ -325,21 +325,34 @@ public class ResourceProviderR4ValueSetTest extends BaseResourceProviderR4Test { } @Test - public void testExpandByUrlWithPreExpansion() throws Exception {//FIXME: DM + public void testExpandByUrlWithBogusUrl() throws Exception { + loadAndPersistCodeSystemAndValueSet(HttpVerb.POST); + + try { + ourClient + .operation() + .onType(ValueSet.class) + .named("expand") + .withParameter(Parameters.class, "url", new UriType("http://www.healthintersections.com.au/fhir/ValueSet/bogus")) + .execute(); + } catch (ResourceNotFoundException e) { + assertEquals(404, e.getStatusCode()); + assertEquals("HTTP 404 Not Found: Unknown ValueSet: http%3A%2F%2Fwww.healthintersections.com.au%2Ffhir%2FValueSet%2Fbogus", e.getMessage()); + } + } + + @Test + public void testExpandByUrlWithPreExpansion() throws Exception { myDaoConfig.setPreExpandValueSetsExperimental(true); - ourLog.info("DIEDERIK - before loading CD and VS"); loadAndPersistCodeSystemAndValueSet(HttpVerb.POST); - ourLog.info("DIEDERIK - after loading CD and VS; before pre-expansion"); myTermSvc.preExpandDeferredValueSetsToTerminologyTables(); - ourLog.info("DIEDERIK - after pre-expansion"); 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")) //FIXME: DM - .withParameter(Parameters.class, "url", new UriType("http://www.healthintersections.com.au/fhir/ValueSet/bogus")) + .withParameter(Parameters.class, "url", new UriType("http://www.healthintersections.com.au/fhir/ValueSet/extensional-case-2")) .execute(); ValueSet expanded = (ValueSet) respParam.getParameter().get(0).getResource(); @@ -351,6 +364,26 @@ public class ResourceProviderR4ValueSetTest extends BaseResourceProviderR4Test { } + @Test + public void testExpandByUrlWithPreExpansionAndBogusUrl() throws Exception { + myDaoConfig.setPreExpandValueSetsExperimental(true); + + loadAndPersistCodeSystemAndValueSet(HttpVerb.POST); + myTermSvc.preExpandDeferredValueSetsToTerminologyTables(); + + try { + Parameters respParam = ourClient + .operation() + .onType(ValueSet.class) + .named("expand") + .withParameter(Parameters.class, "url", new UriType("http://www.healthintersections.com.au/fhir/ValueSet/bogus")) + .execute(); + } catch (ResourceNotFoundException e) { + assertEquals(404, e.getStatusCode()); + assertEquals("HTTP 404 Not Found: Unknown ValueSet: http%3A%2F%2Fwww.healthintersections.com.au%2Ffhir%2FValueSet%2Fbogus", e.getMessage()); + } + } + @Test public void testExpandByValueSet() throws IOException { loadAndPersistCodeSystem(HttpVerb.POST); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/term/TerminologySvcImplR4Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/term/TerminologySvcImplR4Test.java index e108f2b8457..a4613d1efb3 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/term/TerminologySvcImplR4Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/term/TerminologySvcImplR4Test.java @@ -885,19 +885,12 @@ public class TerminologySvcImplR4Test extends BaseJpaR4Test { assertEquals("Systolic blood pressure 8 hour minimum", containsComponent.getDisplay()); assertFalse(containsComponent.hasDesignation()); - myTermSvc.saveDeferred(); - myTermSvc.preExpandDeferredValueSetsToTerminologyTables(); - expandedValueSet = myTermSvc.expandValueSet(valueSet, myDaoConfig.getPreExpandValueSetsDefaultOffsetExperimental(), myDaoConfig.getPreExpandValueSetsDefaultCountExperimental()); ourLog.info("Expanded ValueSet:\n" + myFhirCtx.newJsonParser().setPrettyPrint(true).encodeResourceToString(expandedValueSet)); assertEquals(codeSystem.getConcept().size(), expandedValueSet.getExpansion().getTotal()); assertEquals(myDaoConfig.getPreExpandValueSetsDefaultOffsetExperimental(), expandedValueSet.getExpansion().getOffset()); - assertEquals(2, expandedValueSet.getExpansion().getParameter().size()); - assertEquals("offset", expandedValueSet.getExpansion().getParameter().get(0).getName()); - assertEquals(0, expandedValueSet.getExpansion().getParameter().get(0).getValueIntegerType().getValue().intValue()); - assertEquals("count", expandedValueSet.getExpansion().getParameter().get(1).getName()); - assertEquals(1000, expandedValueSet.getExpansion().getParameter().get(1).getValueIntegerType().getValue().intValue()); + assertEquals(0, expandedValueSet.getExpansion().getParameter().size()); assertEquals(codeSystem.getConcept().size(), expandedValueSet.getExpansion().getContains().size());