From ace3fccf31dc6883a6ffcefed2287925c3568945 Mon Sep 17 00:00:00 2001 From: Tadgh Date: Mon, 5 Feb 2024 14:39:04 -0800 Subject: [PATCH] Descendent fix (#5669) * Fix "is-a" ValueSet expansion, add "descendent-of" support * Fixed tests in DSTU3 and R5 * Trigger new build * Revert "Trigger new build" This reverts commit 46c672b338db1f85c4f438344bfa828bfc723a37. * fix default partition setting on resource (#5617) * fix default partition setting on resource * changelog * Handle DEFAULT partition in rule checker. * Fix spotless --------- Co-authored-by: Michael Buckley Co-authored-by: James Agnew * pom bump, doc add, version enum add (#5616) Co-authored-by: Long Ma * fix default partition setting on resource (#5618) * fix default partition setting on resource * Handle DEFAULT partition in rule checker. Co-authored-by: Ken Stevens * Add setting to make broker not use JacksonMessageConverter (#5611) * Add setting to make broker not use JacksonMessageConverter * Add changelog * Implement suggestions --------- Co-authored-by: juan.marchionatto * Fix version * add changelog, add attribution, remove accidental bring-overs * add changelog, add attribution, remove accidental bring-overs * Finish jira section --------- Co-authored-by: Ole Hedegaard Co-authored-by: Ken Stevens Co-authored-by: Michael Buckley Co-authored-by: James Agnew Co-authored-by: longma1 <32119004+longma1@users.noreply.github.com> Co-authored-by: Long Ma Co-authored-by: jmarchionatto <60409882+jmarchionatto@users.noreply.github.com> Co-authored-by: juan.marchionatto --- .../7_0_0/5603-is-a-descendent-of.yaml | 6 ++++ .../uhn/hapi/fhir/changelog/7_2_0/upgrade.md | 0 .../hapi/fhir/changelog/7_2_0/version.yaml | 3 ++ .../ca/uhn/fhir/jpa/term/TermReadSvcImpl.java | 10 ++++++ ...esourceDaoR4SearchWithElasticSearchIT.java | 2 +- .../FhirResourceDaoDstu3TerminologyTest.java | 10 +++--- ...rceProviderDstu3ValueSetVersionedTest.java | 2 +- .../r4/FhirResourceDaoR4TerminologyTest.java | 36 ++++++++++++++++--- ...rceProviderR4ValueSetNoVerCSNoVerTest.java | 4 +-- ...ourceProviderR4ValueSetVerCSNoVerTest.java | 2 +- ...esourceProviderR4ValueSetVerCSVerTest.java | 2 +- .../r5/ResourceProviderR5ValueSetTest.java | 2 +- ...sourceProviderR5ValueSetVersionedTest.java | 2 +- .../channel/api/BaseChannelSettings.java | 3 ++ 14 files changed, 67 insertions(+), 17 deletions(-) create mode 100644 hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_0_0/5603-is-a-descendent-of.yaml create mode 100644 hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_2_0/upgrade.md create mode 100644 hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_2_0/version.yaml diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_0_0/5603-is-a-descendent-of.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_0_0/5603-is-a-descendent-of.yaml new file mode 100644 index 00000000000..1c8244a07ee --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_0_0/5603-is-a-descendent-of.yaml @@ -0,0 +1,6 @@ +--- +type: fix +issue: 5603 +jira: SMILE-8000 +title: "Previously, the semantics of `is-a` were incorrect in Valueset Expansion. The implementation previously used the behaviour of `descendent-of`, which means that `A is-a A` was not being considered as true. This has been corrected. In addition, +`descendent-of` is now supported, which compares for strict descendency, and does not include itself. Thanks to Ole Hedegaard (@ohetrifork) for the fix." diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_2_0/upgrade.md b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_2_0/upgrade.md new file mode 100644 index 00000000000..e69de29bb2d diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_2_0/version.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_2_0/version.yaml new file mode 100644 index 00000000000..9e4b8d89de0 --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_2_0/version.yaml @@ -0,0 +1,3 @@ +--- +release-date: "2023-05-18" +codename: "Borealis" diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/TermReadSvcImpl.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/TermReadSvcImpl.java index 4c3420ea4ea..86235e0403b 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/TermReadSvcImpl.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/TermReadSvcImpl.java @@ -1598,6 +1598,16 @@ public class TermReadSvcImpl implements ITermReadSvc, IHasScheduledJobs { TermConcept code = findCodeForFilterCriteria(theSystem, theFilter); if (theFilter.getOp() == ValueSet.FilterOperator.ISA) { + ourLog.debug( + " * Filtering on specific code and codes with a parent of {}/{}/{}", + code.getId(), + code.getCode(), + code.getDisplay()); + + b.must(f.bool() + .should(f.match().field("myParentPids").matching("" + code.getId())) + .should(f.match().field("myId").matching(code.getId()))); + } else if (theFilter.getOp() == ValueSet.FilterOperator.DESCENDENTOF) { ourLog.debug( " * Filtering on codes with a parent of {}/{}/{}", code.getId(), code.getCode(), code.getDisplay()); diff --git a/hapi-fhir-jpaserver-elastic-test-utilities/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchWithElasticSearchIT.java b/hapi-fhir-jpaserver-elastic-test-utilities/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchWithElasticSearchIT.java index a17f61fe13e..615f5d9e165 100644 --- a/hapi-fhir-jpaserver-elastic-test-utilities/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchWithElasticSearchIT.java +++ b/hapi-fhir-jpaserver-elastic-test-utilities/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchWithElasticSearchIT.java @@ -786,7 +786,7 @@ public class FhirResourceDaoR4SearchWithElasticSearchIT extends BaseJpaTest impl logAndValidateValueSet(result); ArrayList codes = toCodesContains(result.getExpansion().getContains()); - assertThat(codes, containsInAnyOrder("childAAA", "childAAB")); + assertThat(codes, containsInAnyOrder("childAA", "childAAA", "childAAB")); } @Test diff --git a/hapi-fhir-jpaserver-test-dstu3/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoDstu3TerminologyTest.java b/hapi-fhir-jpaserver-test-dstu3/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoDstu3TerminologyTest.java index d5c500e4b17..c5ac179edbe 100644 --- a/hapi-fhir-jpaserver-test-dstu3/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoDstu3TerminologyTest.java +++ b/hapi-fhir-jpaserver-test-dstu3/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoDstu3TerminologyTest.java @@ -313,7 +313,7 @@ public class FhirResourceDaoDstu3TerminologyTest extends BaseJpaDstu3Test { .setSystem(codeSystem.getUrl()) .addFilter() .setProperty("concept") - .setOp(FilterOperator.ISA) + .setOp(FilterOperator.DESCENDENTOF) .setValue("dogs"); myValueSetDao.create(valueSet, mySrd); @@ -504,7 +504,7 @@ public class FhirResourceDaoDstu3TerminologyTest extends BaseJpaDstu3Test { logAndValidateValueSet(result); ArrayList codes = toCodesContains(result.getExpansion().getContains()); - assertThat(codes, containsInAnyOrder("childAAA", "childAAB")); + assertThat(codes, containsInAnyOrder("childAA", "childAAA", "childAAB")); } @@ -535,7 +535,7 @@ public class FhirResourceDaoDstu3TerminologyTest extends BaseJpaDstu3Test { logAndValidateValueSet(result); ArrayList codes = toCodesContains(result.getExpansion().getContains()); - assertThat(codes, containsInAnyOrder("childAAA", "childAAB")); + assertThat(codes, containsInAnyOrder("childAA", "childAAA", "childAAB")); } @@ -650,7 +650,7 @@ public class FhirResourceDaoDstu3TerminologyTest extends BaseJpaDstu3Test { ValueSet vs = new ValueSet(); ConceptSetComponent include = vs.getCompose().addInclude(); include.setSystem(URL_MY_CODE_SYSTEM); - include.addFilter().setProperty("concept").setOp(FilterOperator.ISA).setValue("ParentA"); + include.addFilter().setProperty("concept").setOp(FilterOperator.DESCENDENTOF).setValue("ParentA"); ValueSet result = myValueSetDao.expand(vs, null); logAndValidateValueSet(result); @@ -669,7 +669,7 @@ public class FhirResourceDaoDstu3TerminologyTest extends BaseJpaDstu3Test { vs = new ValueSet(); include = vs.getCompose().addInclude(); include.setSystem(URL_MY_CODE_SYSTEM); - include.addFilter().setProperty("concept").setOp(FilterOperator.ISA).setValue("ParentA"); + include.addFilter().setProperty("concept").setOp(FilterOperator.DESCENDENTOF).setValue("ParentA"); result = myValueSetDao.expand(vs, null); logAndValidateValueSet(result); diff --git a/hapi-fhir-jpaserver-test-dstu3/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/ResourceProviderDstu3ValueSetVersionedTest.java b/hapi-fhir-jpaserver-test-dstu3/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/ResourceProviderDstu3ValueSetVersionedTest.java index aeb4cc7328c..98dd6fb4ec1 100644 --- a/hapi-fhir-jpaserver-test-dstu3/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/ResourceProviderDstu3ValueSetVersionedTest.java +++ b/hapi-fhir-jpaserver-test-dstu3/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/ResourceProviderDstu3ValueSetVersionedTest.java @@ -230,7 +230,7 @@ public class ResourceProviderDstu3ValueSetVersionedTest extends BaseResourceProv ConceptSetComponent include = myLocalVs.getCompose().addInclude(); include.setSystem(theCodeSystemUrl); include.setVersion(theValueSetVersion); - include.addFilter().setProperty("concept").setOp(FilterOperator.ISA).setValue("ParentA"); + include.addFilter().setProperty("concept").setOp(FilterOperator.DESCENDENTOF).setValue("ParentA"); return myLocalVs; } diff --git a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4TerminologyTest.java b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4TerminologyTest.java index 048e242d700..789e11b60f1 100644 --- a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4TerminologyTest.java +++ b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4TerminologyTest.java @@ -353,7 +353,7 @@ public class FhirResourceDaoR4TerminologyTest extends BaseJpaR4Test { .setSystem(codeSystem.getUrl()) .addFilter() .setProperty("concept") - .setOp(FilterOperator.ISA) + .setOp(FilterOperator.DESCENDENTOF) .setValue("dogs"); myValueSetDao.create(valueSet, mySrd); @@ -584,7 +584,7 @@ public class FhirResourceDaoR4TerminologyTest extends BaseJpaR4Test { logAndValidateValueSet(result); ArrayList codes = toCodesContains(result.getExpansion().getContains()); - assertThat(codes, containsInAnyOrder("childAAA", "childAAB")); + assertThat(codes, containsInAnyOrder("childAA", "childAAA", "childAAB")); } @@ -610,6 +610,34 @@ public class FhirResourceDaoR4TerminologyTest extends BaseJpaR4Test { logAndValidateValueSet(result); ArrayList codes = toCodesContains(result.getExpansion().getContains()); + assertEquals(3, codes.size()); + assertThat(codes, containsInAnyOrder("childAA", "childAAA", "childAAB")); + + } + + @Test + public void testExpandWithDescendentOfInExternalValueSetReindex() { + TermReindexingSvcImpl.setForceSaveDeferredAlwaysForUnitTest(true); + + createExternalCsAndLocalVs(); + + myResourceReindexingSvc.markAllResourcesForReindexing(); + myResourceReindexingSvc.forceReindexingPass(); + myResourceReindexingSvc.forceReindexingPass(); + myTerminologyDeferredStorageSvc.saveDeferred(); + myTerminologyDeferredStorageSvc.saveDeferred(); + myTerminologyDeferredStorageSvc.saveDeferred(); + + ValueSet vs = new ValueSet(); + ConceptSetComponent include = vs.getCompose().addInclude(); + include.setSystem(TermTestUtil.URL_MY_CODE_SYSTEM); + include.addFilter().setOp(FilterOperator.DESCENDENTOF).setValue("childAA").setProperty("concept"); + + ValueSet result = myValueSetDao.expand(vs, null); // breakpoint + logAndValidateValueSet(result); + + ArrayList codes = toCodesContains(result.getExpansion().getContains()); + assertEquals(2, codes.size()); assertThat(codes, containsInAnyOrder("childAAA", "childAAB")); } @@ -795,7 +823,7 @@ public class FhirResourceDaoR4TerminologyTest extends BaseJpaR4Test { ValueSet vs = new ValueSet(); ConceptSetComponent include = vs.getCompose().addInclude(); include.setSystem(TermTestUtil.URL_MY_CODE_SYSTEM); - include.addFilter().setProperty("concept").setOp(FilterOperator.ISA).setValue("ParentA"); + include.addFilter().setProperty("concept").setOp(FilterOperator.DESCENDENTOF).setValue("ParentA"); ValueSet result = myValueSetDao.expand(vs, null); logAndValidateValueSet(result); @@ -814,7 +842,7 @@ public class FhirResourceDaoR4TerminologyTest extends BaseJpaR4Test { vs = new ValueSet(); include = vs.getCompose().addInclude(); include.setSystem(TermTestUtil.URL_MY_CODE_SYSTEM); - include.addFilter().setProperty("concept").setOp(FilterOperator.ISA).setValue("ParentA"); + include.addFilter().setProperty("concept").setOp(FilterOperator.DESCENDENTOF).setValue("ParentA"); result = myValueSetDao.expand(vs, null); logAndValidateValueSet(result); diff --git a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderR4ValueSetNoVerCSNoVerTest.java b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderR4ValueSetNoVerCSNoVerTest.java index 2ab6dd76bf7..a3e9614ff32 100644 --- a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderR4ValueSetNoVerCSNoVerTest.java +++ b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderR4ValueSetNoVerCSNoVerTest.java @@ -202,7 +202,7 @@ public class ResourceProviderR4ValueSetNoVerCSNoVerTest extends BaseResourceProv myLocalVs.setUrl(URL_MY_VALUE_SET); ConceptSetComponent include = myLocalVs.getCompose().addInclude(); include.setSystem(codeSystem.getUrl()); - include.addFilter().setProperty("concept").setOp(FilterOperator.ISA).setValue("ParentA"); + include.addFilter().setProperty("concept").setOp(FilterOperator.DESCENDENTOF).setValue("ParentA"); myLocalValueSetId = myValueSetDao.create(myLocalVs, mySrd).getId().toUnqualifiedVersionless(); } @@ -1199,7 +1199,7 @@ public class ResourceProviderR4ValueSetNoVerCSNoVerTest extends BaseResourceProv .setSystem(URL_MY_CODE_SYSTEM) .addFilter() .setProperty("concept") - .setOp(FilterOperator.ISA) + .setOp(FilterOperator.DESCENDENTOF) .setValue("A"); myLocalVs .getCompose() diff --git a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderR4ValueSetVerCSNoVerTest.java b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderR4ValueSetVerCSNoVerTest.java index 7b56df6dbc8..cb8de80537b 100644 --- a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderR4ValueSetVerCSNoVerTest.java +++ b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderR4ValueSetVerCSNoVerTest.java @@ -167,7 +167,7 @@ public class ResourceProviderR4ValueSetVerCSNoVerTest extends BaseResourceProvid myLocalVs.setUrl(URL_MY_VALUE_SET); ConceptSetComponent include = myLocalVs.getCompose().addInclude(); include.setSystem(codeSystem.getUrl()); - include.addFilter().setProperty("concept").setOp(FilterOperator.ISA).setValue("ParentA"); + include.addFilter().setProperty("concept").setOp(FilterOperator.DESCENDENTOF).setValue("ParentA"); myLocalValueSetId = myValueSetDao.create(myLocalVs, mySrd).getId().toUnqualifiedVersionless(); } diff --git a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderR4ValueSetVerCSVerTest.java b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderR4ValueSetVerCSVerTest.java index cc8d09d8ff7..f10efb5b1f6 100644 --- a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderR4ValueSetVerCSVerTest.java +++ b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderR4ValueSetVerCSVerTest.java @@ -196,7 +196,7 @@ public class ResourceProviderR4ValueSetVerCSVerTest extends BaseResourceProvider ConceptSetComponent include = myLocalVs.getCompose().addInclude(); include.setSystem(theCodeSystemUrl); include.setVersion(theValueSetVersion); - include.addFilter().setProperty("concept").setOp(FilterOperator.ISA).setValue("ParentA"); + include.addFilter().setProperty("concept").setOp(FilterOperator.DESCENDENTOF).setValue("ParentA"); return myLocalVs; } diff --git a/hapi-fhir-jpaserver-test-r5/src/test/java/ca/uhn/fhir/jpa/provider/r5/ResourceProviderR5ValueSetTest.java b/hapi-fhir-jpaserver-test-r5/src/test/java/ca/uhn/fhir/jpa/provider/r5/ResourceProviderR5ValueSetTest.java index f5f882e38ff..9df23b88314 100644 --- a/hapi-fhir-jpaserver-test-r5/src/test/java/ca/uhn/fhir/jpa/provider/r5/ResourceProviderR5ValueSetTest.java +++ b/hapi-fhir-jpaserver-test-r5/src/test/java/ca/uhn/fhir/jpa/provider/r5/ResourceProviderR5ValueSetTest.java @@ -208,7 +208,7 @@ public class ResourceProviderR5ValueSetTest extends BaseResourceProviderR5Test { myLocalVs.setUrl(URL_MY_VALUE_SET); ConceptSetComponent include = myLocalVs.getCompose().addInclude(); include.setSystem(codeSystem.getUrl()); - include.addFilter().setProperty("concept").setOp(Enumerations.FilterOperator.ISA).setValue("ParentA"); + include.addFilter().setProperty("concept").setOp(Enumerations.FilterOperator.DESCENDENTOF).setValue("ParentA"); myLocalValueSetId = myValueSetDao.create(myLocalVs, mySrd).getId().toUnqualifiedVersionless(); } diff --git a/hapi-fhir-jpaserver-test-r5/src/test/java/ca/uhn/fhir/jpa/provider/r5/ResourceProviderR5ValueSetVersionedTest.java b/hapi-fhir-jpaserver-test-r5/src/test/java/ca/uhn/fhir/jpa/provider/r5/ResourceProviderR5ValueSetVersionedTest.java index 824384e57c4..c7af44cab70 100644 --- a/hapi-fhir-jpaserver-test-r5/src/test/java/ca/uhn/fhir/jpa/provider/r5/ResourceProviderR5ValueSetVersionedTest.java +++ b/hapi-fhir-jpaserver-test-r5/src/test/java/ca/uhn/fhir/jpa/provider/r5/ResourceProviderR5ValueSetVersionedTest.java @@ -231,7 +231,7 @@ public class ResourceProviderR5ValueSetVersionedTest extends BaseResourceProvide ConceptSetComponent include = myLocalVs.getCompose().addInclude(); include.setSystem(theCodeSystemUrl); include.setVersion(theValueSetVersion); - include.addFilter().setProperty("concept").setOp(FilterOperator.ISA).setValue("ParentA"); + include.addFilter().setProperty("concept").setOp(FilterOperator.DESCENDENTOF).setValue("ParentA"); return myLocalVs; } diff --git a/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/subscription/channel/api/BaseChannelSettings.java b/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/subscription/channel/api/BaseChannelSettings.java index 29a4b6d086e..0776d1b8bb3 100644 --- a/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/subscription/channel/api/BaseChannelSettings.java +++ b/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/subscription/channel/api/BaseChannelSettings.java @@ -26,6 +26,9 @@ public abstract class BaseChannelSettings implements IChannelSettings { private ChannelRetryConfiguration myRetryConfigurationParameters; + // init true to match previous behaviour + private boolean myUseJacksonMessageConverter = true; + /** * Default true. Used by IChannelNamer to decide how to qualify the channel name. */