From 19676a01b728ba983e04338ec699d400080699d3 Mon Sep 17 00:00:00 2001 From: Diederik Muylwyk Date: Mon, 9 Sep 2019 17:24:01 -0400 Subject: [PATCH] Incremental progress on large ValueSet expansion with URL parameter. --- .../java/ca/uhn/fhir/jpa/dao/DaoConfig.java | 6 +- .../fhir/jpa/term/HapiTerminologySvcR4.java | 2 +- .../jpa/term/ValueSetConceptAccumulator.java | 2 +- .../r4/ResourceProviderR4ValueSetTest.java | 245 +++++++++++++++--- 4 files changed, 218 insertions(+), 37 deletions(-) diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/DaoConfig.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/DaoConfig.java index 996949bb7c3..32a8f556bb9 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/DaoConfig.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/DaoConfig.java @@ -146,12 +146,12 @@ public class DaoConfig { private boolean myEnableInMemorySubscriptionMatching = true; private boolean myEnforceReferenceTargetTypes = true; private ClientIdStrategyEnum myResourceClientIdStrategy = ClientIdStrategyEnum.ALPHANUMERIC; + private boolean myFilterParameterEnabled = false; + private StoreMetaSourceInformationEnum myStoreMetaSourceInformation = StoreMetaSourceInformationEnum.SOURCE_URI_AND_REQUEST_ID; /** * EXPERIMENTAL - Do not use in production! Do not change default of {@code false}! */ - private boolean myPreExpandValueSetsExperimental = true; // FIXME: DM 2019-09-03 - Return to false before merging into master. - private boolean myFilterParameterEnabled = false; - private StoreMetaSourceInformationEnum myStoreMetaSourceInformation = StoreMetaSourceInformationEnum.SOURCE_URI_AND_REQUEST_ID; + private boolean myPreExpandValueSetsExperimental = false; /** * EXPERIMENTAL - Do not use in production! Do not change default of {@code 0}! */ diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/HapiTerminologySvcR4.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/HapiTerminologySvcR4.java index a7e3d60e8e7..e9498856f14 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/HapiTerminologySvcR4.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/HapiTerminologySvcR4.java @@ -121,7 +121,7 @@ public class HapiTerminologySvcR4 extends BaseHapiTerminologySvcImpl implements } @Override - public List expandValueSet(String theValueSet) { + public List expandValueSet(String theValueSet) {//FIXME: DM COWABUNGA ValueSet vs = myValidationSupport.fetchResource(myContext, ValueSet.class, theValueSet); if (vs == null) { return Collections.emptyList(); diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/ValueSetConceptAccumulator.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/ValueSetConceptAccumulator.java index 0cbb6ff4e69..55a8d2e5b8f 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/ValueSetConceptAccumulator.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/ValueSetConceptAccumulator.java @@ -156,7 +156,7 @@ public class ValueSetConceptAccumulator implements IValueSetConceptAccumulator { for (Long conceptId : conceptIds) { myValueSetConceptDao.updateOrderById(conceptId, order++); } - ourLog.info("Have remove gaps from concept order for {} concepts in ValueSet[{}]", conceptIds.size(), myTermValueSet.getUrl()); + ourLog.info("Have removed gaps from concept order for {} concepts in ValueSet[{}]", conceptIds.size(), myTermValueSet.getUrl()); return true; } 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 7a69fd83b55..e29ae2a327b 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 @@ -1,5 +1,6 @@ package ca.uhn.fhir.jpa.provider.r4; +import ca.uhn.fhir.jpa.dao.DaoConfig; import ca.uhn.fhir.jpa.dao.IFhirResourceDao; import ca.uhn.fhir.jpa.dao.data.IResourceTableDao; import ca.uhn.fhir.jpa.entity.TermCodeSystemVersion; @@ -24,10 +25,13 @@ import org.hl7.fhir.r4.model.CodeSystem.CodeSystemContentMode; import org.hl7.fhir.r4.model.CodeSystem.ConceptDefinitionComponent; import org.hl7.fhir.r4.model.ValueSet.ConceptSetComponent; import org.hl7.fhir.r4.model.ValueSet.FilterOperator; +import org.hl7.fhir.r4.model.codesystems.HttpVerb; +import org.junit.After; import org.junit.AfterClass; -import org.junit.Before; import org.junit.Test; -import org.springframework.transaction.annotation.Transactional; +import org.springframework.transaction.TransactionStatus; +import org.springframework.transaction.support.TransactionCallbackWithoutResult; +import org.springframework.transaction.support.TransactionTemplate; import java.io.IOException; import java.nio.charset.StandardCharsets; @@ -40,18 +44,86 @@ import static org.junit.Assert.*; public class ResourceProviderR4ValueSetTest extends BaseResourceProviderR4Test { private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(ResourceProviderR4ValueSetTest.class); + private IIdType myExtensionalCsId; private IIdType myExtensionalVsId; private IIdType myLocalValueSetId; + private Long myExtensionalCsIdOnResourceTable; + private Long myExtensionalVsIdOnResourceTable; private ValueSet myLocalVs; - @Before - @Transactional - public void before02() throws IOException { - CodeSystem cs = loadResourceFromClasspath(CodeSystem.class, "/extensional-case-3-cs.xml"); - myCodeSystemDao.create(cs, mySrd); +// @Before +// @Transactional +// public void before02() throws IOException { +// CodeSystem cs = loadResourceFromClasspath(CodeSystem.class, "/extensional-case-3-cs.xml"); +// myCodeSystemDao.create(cs, mySrd); +// +// ValueSet upload = loadResourceFromClasspath(ValueSet.class, "/extensional-case-3-vs.xml"); +// myExtensionalVsId = myValueSetDao.create(upload, mySrd).getId().toUnqualifiedVersionless(); +// } - ValueSet upload = loadResourceFromClasspath(ValueSet.class, "/extensional-case-3-vs.xml"); - myExtensionalVsId = myValueSetDao.create(upload, mySrd).getId().toUnqualifiedVersionless(); + private void loadAndPersistCodeSystemAndValueSet(HttpVerb theVerb) throws IOException { + loadAndPersistCodeSystem(theVerb); + loadAndPersistValueSet(theVerb); + } + + private void loadAndPersistCodeSystem(HttpVerb theVerb) throws IOException { + CodeSystem codeSystem = loadResourceFromClasspath(CodeSystem.class, "/extensional-case-3-cs.xml"); + codeSystem.setId("CodeSystem/cs"); + persistCodeSystem(codeSystem, theVerb); + } + + private void persistCodeSystem(CodeSystem theCodeSystem, HttpVerb theVerb) { + switch (theVerb) { + case POST: + new TransactionTemplate(myTxManager).execute(new TransactionCallbackWithoutResult() { + @Override + protected void doInTransactionWithoutResult(TransactionStatus theStatus) { + myExtensionalCsId = myCodeSystemDao.create(theCodeSystem, mySrd).getId().toUnqualifiedVersionless(); + } + }); + break; + case PUT: + new TransactionTemplate(myTxManager).execute(new TransactionCallbackWithoutResult() { + @Override + protected void doInTransactionWithoutResult(TransactionStatus theStatus) { + myExtensionalCsId = myCodeSystemDao.update(theCodeSystem, mySrd).getId().toUnqualifiedVersionless(); + } + }); + break; + default: + throw new IllegalArgumentException("HTTP verb is not supported: " + theVerb); + } + myExtensionalCsIdOnResourceTable = myCodeSystemDao.readEntity(myExtensionalCsId, null).getId(); + } + + private void loadAndPersistValueSet(HttpVerb theVerb) throws IOException { + ValueSet valueSet = loadResourceFromClasspath(ValueSet.class, "/extensional-case-3-vs.xml"); + valueSet.setId("ValueSet/vs"); + persistValueSet(valueSet, theVerb); + } + + private void persistValueSet(ValueSet theValueSet, HttpVerb theVerb) { + switch (theVerb) { + case POST: + new TransactionTemplate(myTxManager).execute(new TransactionCallbackWithoutResult() { + @Override + protected void doInTransactionWithoutResult(TransactionStatus theStatus) { + myExtensionalVsId = myValueSetDao.create(theValueSet, mySrd).getId().toUnqualifiedVersionless(); + } + }); + break; + case PUT: + new TransactionTemplate(myTxManager).execute(new TransactionCallbackWithoutResult() { + @Override + protected void doInTransactionWithoutResult(TransactionStatus theStatus) { + myExtensionalVsId = myValueSetDao.update(theValueSet, mySrd).getId().toUnqualifiedVersionless(); + } + }); + break; + default: + throw new IllegalArgumentException("HTTP verb is not supported: " + theVerb); + } + myExtensionalVsIdOnResourceTable = myValueSetDao.readEntity(myExtensionalVsId, null).getId(); } private CodeSystem createExternalCs() { @@ -128,8 +200,9 @@ public class ResourceProviderR4ValueSetTest extends BaseResourceProviderR4Test { } @Test - public void testExpandById() { - //@formatter:off + public void testExpandById() throws Exception { + loadAndPersistCodeSystemAndValueSet(HttpVerb.POST); + Parameters respParam = ourClient .operation() .onInstance(myExtensionalVsId) @@ -137,7 +210,6 @@ public class ResourceProviderR4ValueSetTest extends BaseResourceProviderR4Test { .withNoParameters(Parameters.class) .execute(); ValueSet expanded = (ValueSet) respParam.getParameter().get(0).getResource(); - //@formatter:on String resp = myFhirCtx.newXmlParser().setPrettyPrint(true).encodeResourceToString(expanded); ourLog.info(resp); @@ -158,9 +230,42 @@ public class ResourceProviderR4ValueSetTest extends BaseResourceProviderR4Test { } @Test - public void testExpandByIdWithFilter() { + public void testExpandByIdWithPreExpansion() throws Exception { + myDaoConfig.setPreExpandValueSetsExperimental(true); + + loadAndPersistCodeSystemAndValueSet(HttpVerb.POST); + myTermSvc.preExpandDeferredValueSetsToTerminologyTables(); + + Parameters respParam = ourClient + .operation() + .onInstance(myExtensionalVsId) + .named("expand") + .withNoParameters(Parameters.class) + .execute(); + ValueSet expanded = (ValueSet) respParam.getParameter().get(0).getResource(); + + String resp = myFhirCtx.newXmlParser().setPrettyPrint(true).encodeResourceToString(expanded); + ourLog.info(resp); + assertThat(resp, containsString("")); + assertThat(resp, containsString("")); + assertThat(resp, containsString("")); + assertThat(resp, containsString("")); + assertThat(resp, containsString("")); + assertThat(resp, containsString("")); + assertThat(resp, containsString("")); + assertThat(resp, containsString("")); + assertThat(resp, containsString("")); + assertThat(resp, containsString("")); + assertThat(resp, containsString("")); + assertThat(resp, containsString("")); + assertThat(resp, containsString("")); + + } + + @Test + public void testExpandByIdWithFilter() throws Exception { + loadAndPersistCodeSystemAndValueSet(HttpVerb.POST); - //@formatter:off Parameters respParam = ourClient .operation() .onInstance(myExtensionalVsId) @@ -168,7 +273,6 @@ public class ResourceProviderR4ValueSetTest extends BaseResourceProviderR4Test { .withParameter(Parameters.class, "filter", new StringType("first")) .execute(); ValueSet expanded = (ValueSet) respParam.getParameter().get(0).getResource(); - //@formatter:on String resp = myFhirCtx.newXmlParser().setPrettyPrint(true).encodeResourceToString(expanded); ourLog.info(resp); @@ -178,12 +282,64 @@ public class ResourceProviderR4ValueSetTest extends BaseResourceProviderR4Test { } @Test - public void testExpandByUrl() { + public void testExpandByIdWithFilterWithPreExpansion() throws Exception { + myDaoConfig.setPreExpandValueSetsExperimental(true); + + loadAndPersistCodeSystemAndValueSet(HttpVerb.POST); + myTermSvc.preExpandDeferredValueSetsToTerminologyTables(); + + Parameters respParam = ourClient + .operation() + .onInstance(myExtensionalVsId) + .named("expand") + .withParameter(Parameters.class, "filter", new StringType("first")) + .execute(); + ValueSet expanded = (ValueSet) respParam.getParameter().get(0).getResource(); + + String resp = myFhirCtx.newXmlParser().setPrettyPrint(true).encodeResourceToString(expanded); + ourLog.info(resp); + assertThat(resp, containsString("")); + assertThat(resp, not(containsString(""))); + + } + + @Test + public void testExpandByUrl() throws Exception { + loadAndPersistCodeSystemAndValueSet(HttpVerb.POST); + 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")) +// .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")) + .execute(); + ValueSet expanded = (ValueSet) respParam.getParameter().get(0).getResource(); + + String resp = myFhirCtx.newXmlParser().setPrettyPrint(true).encodeResourceToString(expanded); + ourLog.info(resp); + assertThat(resp, stringContainsInOrder( + "", + "")); + + } + + @Test + public void testExpandByUrlWithPreExpansion() throws Exception {//FIXME: DM + 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")) .execute(); ValueSet expanded = (ValueSet) respParam.getParameter().get(0).getResource(); @@ -197,6 +353,8 @@ public class ResourceProviderR4ValueSetTest extends BaseResourceProviderR4Test { @Test public void testExpandByValueSet() throws IOException { + loadAndPersistCodeSystem(HttpVerb.POST); + ValueSet toExpand = loadResourceFromClasspath(ValueSet.class, "/extensional-case-3-vs.xml"); Parameters respParam = ourClient @@ -215,13 +373,36 @@ public class ResourceProviderR4ValueSetTest extends BaseResourceProviderR4Test { } + @Test + public void testExpandByValueSetWithPreExpansion() throws IOException { + myDaoConfig.setPreExpandValueSetsExperimental(true); + + loadAndPersistCodeSystem(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) + .execute(); + ValueSet expanded = (ValueSet) respParam.getParameter().get(0).getResource(); + + String resp = myFhirCtx.newXmlParser().setPrettyPrint(true).encodeResourceToString(expanded); + ourLog.info(resp); + assertThat(resp, stringContainsInOrder( + "", + "")); + + } @Test public void testExpandInlineVsAgainstBuiltInCs() { createLocalVsPointingAtBuiltInCodeSystem(); assertNotNull(myLocalValueSetId); - //@formatter:off Parameters respParam = ourClient .operation() .onType(ValueSet.class) @@ -229,7 +410,6 @@ public class ResourceProviderR4ValueSetTest extends BaseResourceProviderR4Test { .withParameter(Parameters.class, "valueSet", myLocalVs) .execute(); ValueSet expanded = (ValueSet) respParam.getParameter().get(0).getResource(); - //@formatter:on String resp = myFhirCtx.newXmlParser().setPrettyPrint(true).encodeResourceToString(expanded); ourLog.info(resp); @@ -243,7 +423,6 @@ public class ResourceProviderR4ValueSetTest extends BaseResourceProviderR4Test { assertNotNull(myLocalVs); myLocalVs.setId(""); - //@formatter:off Parameters respParam = ourClient .operation() .onType(ValueSet.class) @@ -251,7 +430,6 @@ public class ResourceProviderR4ValueSetTest extends BaseResourceProviderR4Test { .withParameter(Parameters.class, "valueSet", myLocalVs) .execute(); ValueSet expanded = (ValueSet) respParam.getParameter().get(0).getResource(); - //@formatter:on String resp = myFhirCtx.newXmlParser().setPrettyPrint(true).encodeResourceToString(expanded); ourLog.info(resp); @@ -263,7 +441,9 @@ public class ResourceProviderR4ValueSetTest extends BaseResourceProviderR4Test { } @Test - public void testExpandInvalidParams() throws IOException { + public void testExpandInvalidParams() throws Exception { + loadAndPersistCodeSystemAndValueSet(HttpVerb.POST); + try { ourClient .operation() @@ -334,7 +514,6 @@ public class ResourceProviderR4ValueSetTest extends BaseResourceProviderR4Test { createLocalVsPointingAtBuiltInCodeSystem(); assertNotNull(myLocalValueSetId); - //@formatter:off Parameters respParam = ourClient .operation() .onInstance(myLocalValueSetId) @@ -342,7 +521,6 @@ public class ResourceProviderR4ValueSetTest extends BaseResourceProviderR4Test { .withNoParameters(Parameters.class) .execute(); ValueSet expanded = (ValueSet) respParam.getParameter().get(0).getResource(); - //@formatter:on String resp = myFhirCtx.newXmlParser().setPrettyPrint(true).encodeResourceToString(expanded); ourLog.info(resp); @@ -355,7 +533,6 @@ public class ResourceProviderR4ValueSetTest extends BaseResourceProviderR4Test { createExternalCsAndLocalVs(); assertNotNull(myLocalValueSetId); - //@formatter:off Parameters respParam = ourClient .operation() .onInstance(myLocalValueSetId) @@ -363,7 +540,6 @@ public class ResourceProviderR4ValueSetTest extends BaseResourceProviderR4Test { .withNoParameters(Parameters.class) .execute(); ValueSet expanded = (ValueSet) respParam.getParameter().get(0).getResource(); - //@formatter:on String resp = myFhirCtx.newXmlParser().setPrettyPrint(true).encodeResourceToString(expanded); ourLog.info(resp); @@ -401,7 +577,6 @@ public class ResourceProviderR4ValueSetTest extends BaseResourceProviderR4Test { createExternalCsAndLocalVsWithUnknownCode(); assertNotNull(myLocalValueSetId); - //@formatter:off try { ourClient .operation() @@ -412,7 +587,6 @@ public class ResourceProviderR4ValueSetTest extends BaseResourceProviderR4Test { } catch (InvalidRequestException e) { assertEquals("HTTP 400 Bad Request: Invalid filter criteria - code does not exist: {http://example.com/my_code_system}childFOOOOOOO", e.getMessage()); } - //@formatter:on } /** @@ -438,8 +612,9 @@ public class ResourceProviderR4ValueSetTest extends BaseResourceProviderR4Test { } @Test - public void testValidateCodeOperationByCodeAndSystemInstance() { - //@formatter:off + public void testValidateCodeOperationByCodeAndSystemInstance() throws Exception { + loadAndPersistCodeSystemAndValueSet(HttpVerb.POST); + Parameters respParam = ourClient .operation() .onInstance(myExtensionalVsId) @@ -498,8 +673,9 @@ public class ResourceProviderR4ValueSetTest extends BaseResourceProviderR4Test { } @Test - public void testValidateCodeOperationByCodeAndSystemType() { - //@formatter:off + public void testValidateCodeOperationByCodeAndSystemType() throws Exception { + loadAndPersistCodeSystemAndValueSet(HttpVerb.POST); + Parameters respParam = ourClient .operation() .onType(ValueSet.class) @@ -585,6 +761,11 @@ public class ResourceProviderR4ValueSetTest extends BaseResourceProviderR4Test { } + @After + public void afterResetPreExpansionDefault() { + myDaoConfig.setPreExpandValueSetsExperimental(new DaoConfig().isPreExpandValueSetsExperimental()); + } + @AfterClass public static void afterClassClearContext() { TestUtil.clearAllStaticFieldsForUnitTest();