From 72dd6b2922b205c00d57767df0e46a21dd912de1 Mon Sep 17 00:00:00 2001 From: Diederik Muylwyk Date: Thu, 22 Aug 2019 14:00:08 -0400 Subject: [PATCH] Incremental work on large ValueSet expansion support; fixed broken deletion for TermValueSetConceptDesignation. --- .../jpa/dao/data/ITermValueSetConceptDao.java | 2 +- .../ITermValueSetConceptDesignationDao.java | 8 +-- .../TermValueSetConceptDesignation.java | 38 +++++++++++++ .../jpa/term/BaseHapiTerminologySvcImpl.java | 43 ++++++++++++++ .../jpa/term/ValueSetConceptAccumulator.java | 1 + .../jpa/term/TerminologySvcImplR4Test.java | 56 +++++++++++++++---- 6 files changed, 132 insertions(+), 16 deletions(-) diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/ITermValueSetConceptDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/ITermValueSetConceptDao.java index bfec0626c1e..e225719ed4e 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/ITermValueSetConceptDao.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/ITermValueSetConceptDao.java @@ -32,7 +32,7 @@ import java.util.Optional; public interface ITermValueSetConceptDao extends JpaRepository { - @Query("SELECT COUNT(vsc) FROM TermValueSetConcept vsc WHERE vsc.myValueSet.myId = :pid") + @Query("SELECT COUNT(*) FROM TermValueSetConcept vsc WHERE vsc.myValueSet.myId = :pid") Integer countByTermValueSetId(@Param("pid") Long theValueSetId); @Query("DELETE FROM TermValueSetConcept vsc WHERE vsc.myValueSet.myId = :pid") diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/ITermValueSetConceptDesignationDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/ITermValueSetConceptDesignationDao.java index ced9e67b5f3..1792b8496e0 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/ITermValueSetConceptDesignationDao.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/ITermValueSetConceptDesignationDao.java @@ -30,13 +30,13 @@ import org.springframework.data.repository.query.Param; public interface ITermValueSetConceptDesignationDao extends JpaRepository { - @Query("SELECT COUNT(vscd) FROM TermValueSetConceptDesignation vscd WHERE vscd.myConcept.myId = :pid") - Integer countByTermValueSetConceptId(@Param("pid") Long theValueSetConceptId); + @Query("SELECT COUNT(vscd) FROM TermValueSetConceptDesignation vscd WHERE vscd.myValueSet.myId = :pid") + Integer countByTermValueSetId(@Param("pid") Long theValueSetId); - @Query("DELETE FROM TermValueSetConceptDesignation vscd WHERE vscd.myConcept.myValueSet.myId = :pid") + @Query("DELETE FROM TermValueSetConceptDesignation vscd WHERE vscd.myValueSet.myId = :pid") @Modifying void deleteByTermValueSetId(@Param("pid") Long theValueSetId); - @Query("SELECT vscd from TermValueSetConceptDesignation vscd WHERE vscd.myConcept.myId = :pid") + @Query("SELECT vscd FROM TermValueSetConceptDesignation vscd WHERE vscd.myConcept.myId = :pid") Slice findByTermValueSetConceptId(Pageable thePage, @Param("pid") Long theValueSetConceptId); } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/TermValueSetConceptDesignation.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/TermValueSetConceptDesignation.java index b1ab2cff713..e3aa0c5196a 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/TermValueSetConceptDesignation.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/TermValueSetConceptDesignation.java @@ -52,6 +52,16 @@ public class TermValueSetConceptDesignation implements Serializable { @JoinColumn(name = "VALUESET_CONCEPT_PID", referencedColumnName = "PID", nullable = false, foreignKey = @ForeignKey(name = "FK_TRM_VALUESET_CONCEPT_PID")) private TermValueSetConcept myConcept; + @ManyToOne() + @JoinColumn(name = "VALUESET_PID", referencedColumnName = "PID", nullable = false, foreignKey = @ForeignKey(name = "FK_TRM_VSCD_VS_PID")) + private TermValueSet myValueSet; + + @Transient + private String myValueSetUrl; + + @Transient + private String myValueSetName; + @Column(name = "LANG", nullable = true, length = MAX_LENGTH) private String myLanguage; @@ -80,6 +90,31 @@ public class TermValueSetConceptDesignation implements Serializable { return this; } + public TermValueSet getValueSet() { + return myValueSet; + } + + public TermValueSetConceptDesignation setValueSet(TermValueSet theValueSet) { + myValueSet = theValueSet; + return this; + } + + public String getValueSetUrl() { + if (myValueSetUrl == null) { + myValueSetUrl = getValueSet().getUrl(); + } + + return myValueSetUrl; + } + + public String getValueSetName() { + if (myValueSetName == null) { + myValueSetName = getValueSet().getName(); + } + + return myValueSetName; + } + public String getLanguage() { return myLanguage; } @@ -167,6 +202,9 @@ public class TermValueSetConceptDesignation implements Serializable { return new ToStringBuilder(this, ToStringStyle.SHORT_PREFIX_STYLE) .append("myId", myId) .append(myConcept != null ? ("myConcept - id=" + myConcept.getId()) : ("myConcept=(null)")) + .append(myValueSet != null ? ("myValueSet - id=" + myValueSet.getId()) : ("myValueSet=(null)")) + .append("myValueSetUrl", this.getValueSetUrl()) + .append("myValueSetName", this.getValueSetName()) .append("myLanguage", myLanguage) .append("myUseSystem", myUseSystem) .append("myUseCode", myUseCode) 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 a3e3723ebfe..bf1b6f0bf2d 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 @@ -612,6 +612,7 @@ public abstract class BaseHapiTerminologySvcImpl implements IHapiTerminologySvc, // Handle includes ourLog.debug("Handling includes"); for (ValueSet.ConceptSetComponent include : theValueSetToExpand.getCompose().getInclude()) { + ourLog.info("Working with " + identifyValueSetForLogging(theValueSetToExpand)); boolean add = true; expandValueSetHandleIncludeOrExclude(theValueSetCodeAccumulator, addedCodes, include, add, theCodeCounter); } @@ -619,11 +620,48 @@ public abstract class BaseHapiTerminologySvcImpl implements IHapiTerminologySvc, // Handle excludes ourLog.debug("Handling excludes"); for (ValueSet.ConceptSetComponent exclude : theValueSetToExpand.getCompose().getExclude()) { + ourLog.info("Working with " + identifyValueSetForLogging(theValueSetToExpand)); boolean add = false; expandValueSetHandleIncludeOrExclude(theValueSetCodeAccumulator, addedCodes, exclude, add, theCodeCounter); } } + private String identifyValueSetForLogging(ValueSet theValueSet) { + StringBuilder sb = new StringBuilder(); + boolean isIdentified = false; + sb + .append("ValueSet:"); + if (theValueSet.hasId()) { + isIdentified = true; + sb + .append(" ValueSet.id[") + .append(theValueSet.getId()) + .append("]"); + } + if (theValueSet.hasUrl()) { + isIdentified = true; + sb + .append(" ValueSet.url[") + .append(theValueSet.getUrl()) + .append("]"); + } + if (theValueSet.hasIdentifier()) { + isIdentified = true; + sb + .append(" ValueSet.identifier[") + .append(theValueSet.getIdentifierFirstRep().getSystem()) + .append("|") + .append(theValueSet.getIdentifierFirstRep().getValue()) + .append("]"); + } + + if (!isIdentified) { + sb.append(" None of ValueSet.id, ValueSet.url, and ValueSet.identifier are provided."); + } + + return sb.toString(); + } + protected List expandValueSetAndReturnVersionIndependentConcepts(org.hl7.fhir.r4.model.ValueSet theValueSetToExpandR4) { org.hl7.fhir.r4.model.ValueSet.ValueSetExpansionComponent expandedR4 = expandValueSet(theValueSetToExpandR4).getExpansion(); @@ -768,6 +806,11 @@ public abstract class BaseHapiTerminologySvcImpl implements IHapiTerminologySvc, */ FullTextQuery jpaQuery = em.createFullTextQuery(luceneQuery, TermConcept.class); + /* + * DM 2019-08-21 - Processing slows after any ValueSets with many codes explicitly identified. This might + * be due to the dark arts that is memory management. Will monitor but not do anything about this right now. + */ + BooleanQuery.setMaxClauseCount(10000); StopWatch sw = new StopWatch(); AtomicInteger count = new AtomicInteger(0); 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 a072fb52251..4d74d1ed24d 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 @@ -116,6 +116,7 @@ public class ValueSetConceptAccumulator implements IValueSetConceptAccumulator { TermValueSetConceptDesignation designation = new TermValueSetConceptDesignation(); designation.setConcept(theConcept); + designation.setValueSet(myTermValueSet); designation.setLanguage(theDesignation.getLanguage()); if (isNoneBlank(theDesignation.getUseSystem(), theDesignation.getUseCode())) { designation.setUseSystem(theDesignation.getUseSystem()); 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 cd6a1b46811..c80329d070b 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 @@ -562,6 +562,40 @@ public class TerminologySvcImplR4Test extends BaseJpaR4Test { } + @Test + public void testDeleteValueSet() throws Exception { + myDaoConfig.setPreExpandValueSetsExperimental(true); + + loadAndPersistCodeSystemAndValueSetWithDesignations(); + + CodeSystem codeSystem = myCodeSystemDao.read(myExtensionalCsId); + ourLog.info("CodeSystem:\n" + myFhirCtx.newJsonParser().setPrettyPrint(true).encodeResourceToString(codeSystem)); + + ValueSet valueSet = myValueSetDao.read(myExtensionalVsId); + ourLog.info("ValueSet:\n" + myFhirCtx.newJsonParser().setPrettyPrint(true).encodeResourceToString(valueSet)); + + myTermSvc.preExpandValueSetToTerminologyTables(); + + ValueSet expandedValueSet = myTermSvc.expandValueSet(valueSet, myDaoConfig.getPreExpandValueSetsDefaultOffsetExperimental(), myDaoConfig.getPreExpandValueSetsDefaultCountExperimental()); + ourLog.info("Expanded ValueSet:\n" + myFhirCtx.newJsonParser().setPrettyPrint(true).encodeResourceToString(expandedValueSet)); + + Long termValueSetId = myTermValueSetDao.findByResourcePid(valueSet.getIdElement().toUnqualifiedVersionless().getIdPartAsLong()).get().getId(); + assertEquals(3, myTermValueSetConceptDesignationDao.countByTermValueSetId(termValueSetId).intValue()); + assertEquals(24, myTermValueSetConceptDao.countByTermValueSetId(termValueSetId).intValue()); + + new TransactionTemplate(myTxManager).execute(new TransactionCallbackWithoutResult() { + @Override + protected void doInTransactionWithoutResult(TransactionStatus theStatus) { + myTermValueSetConceptDesignationDao.deleteByTermValueSetId(termValueSetId); + assertEquals(0, myTermValueSetConceptDesignationDao.countByTermValueSetId(termValueSetId).intValue()); + myTermValueSetConceptDao.deleteByTermValueSetId(termValueSetId); + assertEquals(0, myTermValueSetConceptDao.countByTermValueSetId(termValueSetId).intValue()); + myTermValueSetDao.deleteByTermValueSetId(termValueSetId); + assertFalse(myTermValueSetDao.findByResourcePid(valueSet.getIdElement().toUnqualifiedVersionless().getIdPartAsLong()).isPresent()); + } + }); + } + @Test public void testDuplicateCodeSystemUrls() throws Exception { loadAndPersistCodeSystem(); @@ -895,17 +929,6 @@ public class TerminologySvcImplR4Test extends BaseJpaR4Test { verify(myValueSetCodeAccumulator, times(9)).includeConceptWithDesignations(anyString(), anyString(), nullable(String.class), anyCollection()); } - @Test - public void testValidateCode() { - createCodeSystem(); - - IValidationSupport.CodeValidationResult validation = myTermSvc.validateCode(myFhirCtx, CS_URL, "ParentWithNoChildrenA", null); - assertEquals(true, validation.isOk()); - - validation = myTermSvc.validateCode(myFhirCtx, CS_URL, "ZZZZZZZ", null); - assertEquals(false, validation.isOk()); - } - @Test public void testStoreTermCodeSystemAndChildren() throws Exception { loadAndPersistCodeSystemWithDesignations(); @@ -2579,6 +2602,17 @@ public class TerminologySvcImplR4Test extends BaseJpaR4Test { }); } + @Test + public void testValidateCode() { + createCodeSystem(); + + IValidationSupport.CodeValidationResult validation = myTermSvc.validateCode(myFhirCtx, CS_URL, "ParentWithNoChildrenA", null); + assertEquals(true, validation.isOk()); + + validation = myTermSvc.validateCode(myFhirCtx, CS_URL, "ZZZZZZZ", null); + assertEquals(false, validation.isOk()); + } + @AfterClass public static void afterClassClearContext() { TestUtil.clearAllStaticFieldsForUnitTest();