From 9564541e5e3e4af0976ce3661f67e257ca1835bd Mon Sep 17 00:00:00 2001 From: James Agnew Date: Thu, 4 Jul 2024 10:47:51 -0400 Subject: [PATCH] Test fix --- .../uhn/fhir/rest/param/DateAndListParam.java | 19 +- .../uhn/fhir/jpa/util/PermutationBuilder.java | 3 - .../fhir/jpa/util/PermutationBuilderTest.java | 25 +++ ...hirResourceDaoR4ComboUniqueParamTest.java} | 173 ++++++++++++------ 4 files changed, 157 insertions(+), 63 deletions(-) rename hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/{FhirResourceDaoR4ComboUniqueParamIT.java => FhirResourceDaoR4ComboUniqueParamTest.java} (94%) diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/DateAndListParam.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/DateAndListParam.java index de11981a64c..6f458973f40 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/DateAndListParam.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/DateAndListParam.java @@ -19,7 +19,7 @@ */ package ca.uhn.fhir.rest.param; -import ca.uhn.fhir.util.CoverageIgnore; +import org.apache.commons.lang3.Validate; public class DateAndListParam extends BaseAndListParam { @@ -28,10 +28,25 @@ public class DateAndListParam extends BaseAndListParam { return new DateOrListParam(); } - @CoverageIgnore @Override public DateAndListParam addAnd(DateOrListParam theValue) { addValue(theValue); return this; } + + /** + * @param theValue The OR values + * @return Returns a reference to this for convenient chaining + * @since 7.4.0 + */ + public DateAndListParam addAnd(DateParam... theValue) { + Validate.notNull(theValue, "theValue must not be null"); + DateOrListParam orListParam = new DateOrListParam(); + for (DateParam next : theValue) { + orListParam.add(next); + } + addValue(orListParam); + return this; + } + } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/util/PermutationBuilder.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/util/PermutationBuilder.java index 0cb78d47ce8..5394e245027 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/util/PermutationBuilder.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/util/PermutationBuilder.java @@ -45,9 +45,6 @@ public class PermutationBuilder { * @since 7.4.0 */ public static List> calculatePermutations(List> theInput) { - if (theInput.size() == 1) { - return theInput; - } List> listToPopulate = new ArrayList<>(calculatePermutationCount(theInput)); int[] indices = new int[theInput.size()]; doCalculatePermutationsIntoIndicesArrayAndPopulateList(0, indices, theInput, listToPopulate); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/util/PermutationBuilderTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/util/PermutationBuilderTest.java index 85016bb9bb7..caf45f9b94f 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/util/PermutationBuilderTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/util/PermutationBuilderTest.java @@ -36,6 +36,31 @@ class PermutationBuilderTest { ); } + @Test + public void testCalculatePermutations_OneDimensonalInputOnX() { + List> input = List.of( + List.of("A0", "A1") + ); + + List> output = PermutationBuilder.calculatePermutations(input); + assertThat(output).containsExactlyInAnyOrder( + List.of("A0"), + List.of("A1") + ); + } + + @Test + public void testCalculatePermutations_OneDimensonalInputOnY() { + List> input = List.of( + List.of("A0"), + List.of("B0") + ); + + List> output = PermutationBuilder.calculatePermutations(input); + assertThat(output).containsExactlyInAnyOrder( + List.of("A0", "B0") + ); + } @Test public void testCalculatePermutationCount() { diff --git a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4ComboUniqueParamIT.java b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4ComboUniqueParamTest.java similarity index 94% rename from hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4ComboUniqueParamIT.java rename to hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4ComboUniqueParamTest.java index 7c3726df589..f8e293a3007 100644 --- a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4ComboUniqueParamIT.java +++ b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4ComboUniqueParamTest.java @@ -13,6 +13,7 @@ import ca.uhn.fhir.jpa.model.entity.ResourceTable; import ca.uhn.fhir.jpa.searchparam.SearchParameterMap; import ca.uhn.fhir.jpa.searchparam.util.JpaParamUtil; import ca.uhn.fhir.rest.api.server.IBundleProvider; +import ca.uhn.fhir.rest.param.DateAndListParam; import ca.uhn.fhir.rest.param.DateOrListParam; import ca.uhn.fhir.rest.param.DateParam; import ca.uhn.fhir.rest.param.ReferenceParam; @@ -22,6 +23,7 @@ import ca.uhn.fhir.rest.server.exceptions.ResourceVersionConflictException; import ca.uhn.fhir.rest.server.exceptions.UnprocessableEntityException; import ca.uhn.fhir.util.HapiExtensions; import jakarta.annotation.Nonnull; +import org.apache.commons.lang3.tuple.Pair; import org.hl7.fhir.instance.model.api.IIdType; import org.hl7.fhir.r4.model.BooleanType; import org.hl7.fhir.r4.model.Bundle; @@ -61,9 +63,9 @@ import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; -public class FhirResourceDaoR4ComboUniqueParamIT extends BaseComboParamsR4Test { +public class FhirResourceDaoR4ComboUniqueParamTest extends BaseComboParamsR4Test { - private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(FhirResourceDaoR4ComboUniqueParamIT.class); + private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(FhirResourceDaoR4ComboUniqueParamTest.class); @Autowired private IJobCoordinator myJobCoordinator; @@ -383,9 +385,7 @@ public class FhirResourceDaoR4ComboUniqueParamIT extends BaseComboParamsR4Test { IIdType id = myPatientDao.create(pt, mySrd).getId().toUnqualifiedVersionless(); myCaptureQueriesListener.logInsertQueries(); - List values = runInTransaction(()->{ - return myResourceIndexedComboStringUniqueDao.findAllForResourceIdForUnitTest(id.getIdPartAsLong()); - }); + List values = runInTransaction(()-> myResourceIndexedComboStringUniqueDao.findAllForResourceIdForUnitTest(id.getIdPartAsLong())); assertEquals(2, values.size()); values.sort(Comparator.comparing(ResourceIndexedComboStringUnique::getIndexString)); assertEquals("Patient?identifier=urn%7C111", values.get(0).getIndexString()); @@ -418,7 +418,115 @@ public class FhirResourceDaoR4ComboUniqueParamIT extends BaseComboParamsR4Test { } @Test - public void testDoubleMatchingOnAnd_Search() { + public void testDoubleMatchingOnAnd_Search_TwoAndValues() { + Pair ids = prepareDoubleMatchingSearchParameterAndPatient(); + String id1 = ids.getLeft(); + + // Two AND values + myCaptureQueriesListener.clear(); + SearchParameterMap sp = new SearchParameterMap(); + sp.setLoadSynchronous(true); + sp.add("identifier", + new TokenAndListParam() + .addAnd(new TokenParam("urn", "111")) + .addAnd(new TokenParam("urn", "222")) + ); + IBundleProvider outcome = myPatientDao.search(sp, mySrd); + myCaptureQueriesListener.logFirstSelectQueryForCurrentThread(); + String unformattedSql = myCaptureQueriesListener.getSelectQueriesForCurrentThread().get(0).getSql(true, false); + assertThat(unformattedSql).containsSubsequence( + "IDX_STRING = 'Patient?identifier=urn%7C111'", + "IDX_STRING = 'Patient?identifier=urn%7C222'" + ); + assertThat(unformattedSql).doesNotContain(("HFJ_SPIDX_TOKEN")); + assertThat(unformattedSql).doesNotContain(("RES_DELETED_AT")); + assertThat(unformattedSql).doesNotContain(("RES_TYPE")); + assertThat(toUnqualifiedVersionlessIdValues(outcome)).containsExactlyInAnyOrder(id1); + } + + + // FIXME: AAAAAAAAAAA + @Test + public void testDoubleMatchingOnAnd_Search_TwoOrValues() { + Pair ids = prepareDoubleMatchingSearchParameterAndPatient(); + String id1 = ids.getLeft(); + + // Two OR values on the same resource - Currently composite SPs don't work for this + myCaptureQueriesListener.clear(); + SearchParameterMap sp = new SearchParameterMap(); + sp.setLoadSynchronous(true); + sp.add("identifier", + new TokenAndListParam() + .addAnd(new TokenParam("urn", "111"), new TokenParam("urn", "222")) + ); + IBundleProvider outcome = myPatientDao.search(sp, mySrd); + myCaptureQueriesListener.logFirstSelectQueryForCurrentThread(); + assertThat(toUnqualifiedVersionlessIdValues(outcome)).containsExactlyInAnyOrder(id1); + String unformattedSql = myCaptureQueriesListener.getSelectQueriesForCurrentThread().get(0).getSql(true, false); + assertEquals("SELECT t0.RES_ID FROM HFJ_IDX_CMP_STRING_UNIQ t0 WHERE (t0.IDX_STRING IN ('Patient?identifier=urn%7C111','Patient?identifier=urn%7C222') )", unformattedSql); + + } + + + @Test + public void testDoubleMatchingOnAnd_Search_TwoAndOrValues() { + myStorageSettings.setUniqueIndexesCheckedBeforeSave(false); + + createUniqueBirthdateAndGenderSps(); + + Patient pt1 = new Patient(); + pt1.setGender(Enumerations.AdministrativeGender.MALE); + pt1.setBirthDateElement(new DateType("2011-01-01")); + String id1 = myPatientDao.create(pt1, mySrd).getId().toUnqualifiedVersionless().getValue(); + + // Two OR values on the same resource - Currently composite SPs don't work for this + myCaptureQueriesListener.clear(); + SearchParameterMap sp = new SearchParameterMap(); + sp.setLoadSynchronous(true); + sp.add(Patient.SP_GENDER, + new TokenAndListParam() + .addAnd(new TokenParam("http://hl7.org/fhir/administrative-gender","male"), new TokenParam( "http://hl7.org/fhir/administrative-gender","female")) + ); + sp.add(Patient.SP_BIRTHDATE, + new DateAndListParam() + .addAnd(new DateParam("2011-01-01"), new DateParam( "2011-02-02")) + ); + IBundleProvider outcome = myPatientDao.search(sp, mySrd); + myCaptureQueriesListener.logFirstSelectQueryForCurrentThread(); + assertThat(toUnqualifiedVersionlessIdValues(outcome)).containsExactlyInAnyOrder(id1); + String unformattedSql = myCaptureQueriesListener.getSelectQueriesForCurrentThread().get(0).getSql(true, false); + assertEquals("SELECT t0.RES_ID FROM HFJ_IDX_CMP_STRING_UNIQ t0 WHERE (t0.IDX_STRING IN ('Patient?birthdate=2011-01-01&gender=http%3A%2F%2Fhl7.org%2Ffhir%2Fadministrative-gender%7Cfemale','Patient?birthdate=2011-01-01&gender=http%3A%2F%2Fhl7.org%2Ffhir%2Fadministrative-gender%7Cmale','Patient?birthdate=2011-02-02&gender=http%3A%2F%2Fhl7.org%2Ffhir%2Fadministrative-gender%7Cfemale','Patient?birthdate=2011-02-02&gender=http%3A%2F%2Fhl7.org%2Ffhir%2Fadministrative-gender%7Cmale') )", unformattedSql); + } + + + + @Test + public void testDoubleMatchingOnAnd_Search_NonMatching() { + Pair ids = prepareDoubleMatchingSearchParameterAndPatient(); + String id1 = ids.getLeft(); + String id2 = ids.getRight(); + + String unformattedSql; + + // Not matching the composite SP at all + myCaptureQueriesListener.clear(); + SearchParameterMap sp = new SearchParameterMap(); + sp.setLoadSynchronous(true); + sp.add("active", + new TokenAndListParam() + .addAnd(new TokenParam(null, "true")) + ); + IBundleProvider outcome = myPatientDao.search(sp, mySrd); + myCaptureQueriesListener.logFirstSelectQueryForCurrentThread(); + assertThat(toUnqualifiedVersionlessIdValues(outcome)).containsExactlyInAnyOrder(id1, id2); + unformattedSql = myCaptureQueriesListener.getSelectQueriesForCurrentThread().get(0).getSql(true, false); + assertThat(unformattedSql).doesNotContain(("IDX_STRING")); + assertThat(unformattedSql).doesNotContain(("RES_DELETED_AT")); + assertThat(unformattedSql).doesNotContain(("RES_TYPE")); + + } + + private Pair prepareDoubleMatchingSearchParameterAndPatient() { myStorageSettings.setAdvancedHSearchIndexing(false); createUniqueIndexPatientIdentifier(); @@ -438,58 +546,7 @@ public class FhirResourceDaoR4ComboUniqueParamIT extends BaseComboParamsR4Test { pt.addIdentifier().setSystem("urn").setValue("444"); myPatientDao.create(pt, mySrd); - String unformattedSql; - - // Two AND values - myCaptureQueriesListener.clear(); - SearchParameterMap sp = new SearchParameterMap(); - sp.setLoadSynchronous(true); - sp.add("identifier", - new TokenAndListParam() - .addAnd(new TokenParam("urn", "111")) - .addAnd(new TokenParam("urn", "222")) - ); - IBundleProvider outcome = myPatientDao.search(sp, mySrd); - myCaptureQueriesListener.logFirstSelectQueryForCurrentThread(); - unformattedSql = myCaptureQueriesListener.getSelectQueriesForCurrentThread().get(0).getSql(true, false); - assertThat(unformattedSql).containsSubsequence( - "IDX_STRING = 'Patient?identifier=urn%7C111'", - "HASH_SYS_AND_VALUE = '-3122824860083758210'" - ); - assertThat(unformattedSql).doesNotContain(("RES_DELETED_AT")); - assertThat(unformattedSql).doesNotContain(("RES_TYPE")); - assertThat(toUnqualifiedVersionlessIdValues(outcome)).containsExactlyInAnyOrder(id1); - - // Two OR values on the same resource - Currently composite SPs don't work for this - myCaptureQueriesListener.clear(); - sp = new SearchParameterMap(); - sp.setLoadSynchronous(true); - sp.add("identifier", - new TokenAndListParam() - .addAnd(new TokenParam("urn", "111"), new TokenParam("urn", "222")) - ); - outcome = myPatientDao.search(sp, mySrd); - myCaptureQueriesListener.logFirstSelectQueryForCurrentThread(); - assertThat(toUnqualifiedVersionlessIdValues(outcome)).containsExactlyInAnyOrder(id1); - unformattedSql = myCaptureQueriesListener.getSelectQueriesForCurrentThread().get(0).getSql(true, false); - assertEquals("SELECT t0.RES_ID FROM HFJ_IDX_CMP_STRING_UNIQ t0 WHERE (t0.IDX_STRING IN ('Patient?identifier=urn%7C111','Patient?identifier=urn%7C222') )", unformattedSql); - - // Not matching the composite SP at all - myCaptureQueriesListener.clear(); - sp = new SearchParameterMap(); - sp.setLoadSynchronous(true); - sp.add("active", - new TokenAndListParam() - .addAnd(new TokenParam(null, "true")) - ); - outcome = myPatientDao.search(sp, mySrd); - myCaptureQueriesListener.logFirstSelectQueryForCurrentThread(); - assertThat(toUnqualifiedVersionlessIdValues(outcome)).containsExactlyInAnyOrder(id1, id2); - unformattedSql = myCaptureQueriesListener.getSelectQueriesForCurrentThread().get(0).getSql(true, false); - assertThat(unformattedSql).doesNotContain(("IDX_STRING")); - assertThat(unformattedSql).doesNotContain(("RES_DELETED_AT")); - assertThat(unformattedSql).doesNotContain(("RES_TYPE")); - + return Pair.of(id1, id2); } @Test