From 0a009901b392fb9974c8835d0472bbbe592cfc33 Mon Sep 17 00:00:00 2001 From: Michael Buckley Date: Wed, 15 May 2024 18:10:32 -0400 Subject: [PATCH] Add some more sql syntax tests for Postgres, Oracle, and Sql Server (#5923) Also: - fix total parsing in internal MatchUrl service. - Add more syntax tests for our docker database tests - Change DaoTestDataBuilder to use a bundle with delete entries for teardown. --- .../fhir/jpa/searchparam/MatchUrlService.java | 16 ++++++--- .../jpa/searchparam/MatchUrlServiceTest.java | 21 +++++++++++ .../database/BaseDatabaseVerificationIT.java | 35 ++++++++++--------- .../fhir/storage/test/DaoTestDataBuilder.java | 22 +++++++----- 4 files changed, 65 insertions(+), 29 deletions(-) diff --git a/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/MatchUrlService.java b/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/MatchUrlService.java index 362d1cbeb1a..49f48f74433 100644 --- a/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/MatchUrlService.java +++ b/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/MatchUrlService.java @@ -127,12 +127,18 @@ public class MatchUrlService { && !paramList.isEmpty() && !paramList.get(0).isEmpty()) { String totalModeEnumStr = paramList.get(0).get(0); - try { - paramMap.setSearchTotalMode(SearchTotalModeEnum.valueOf(totalModeEnumStr)); - } catch (IllegalArgumentException e) { - throw new InvalidRequestException(Msg.code(2078) + "Invalid " - + Constants.PARAM_SEARCH_TOTAL_MODE + " value: " + totalModeEnumStr); + SearchTotalModeEnum searchTotalMode = SearchTotalModeEnum.fromCode(totalModeEnumStr); + if (searchTotalMode == null) { + // We had an oops here supporting the UPPER CASE enum instead of the FHIR code for _total. + // Keep supporting it in case someone is using it. + try { + paramMap.setSearchTotalMode(SearchTotalModeEnum.valueOf(totalModeEnumStr)); + } catch (IllegalArgumentException e) { + throw new InvalidRequestException(Msg.code(2078) + "Invalid " + + Constants.PARAM_SEARCH_TOTAL_MODE + " value: " + totalModeEnumStr); + } } + paramMap.setSearchTotalMode(searchTotalMode); } } else if (Constants.PARAM_OFFSET.equals(nextParamName)) { if (paramList != null diff --git a/hapi-fhir-jpaserver-test-dstu3/src/test/java/ca/uhn/fhir/jpa/searchparam/MatchUrlServiceTest.java b/hapi-fhir-jpaserver-test-dstu3/src/test/java/ca/uhn/fhir/jpa/searchparam/MatchUrlServiceTest.java index 41f7226825b..9b46664d817 100644 --- a/hapi-fhir-jpaserver-test-dstu3/src/test/java/ca/uhn/fhir/jpa/searchparam/MatchUrlServiceTest.java +++ b/hapi-fhir-jpaserver-test-dstu3/src/test/java/ca/uhn/fhir/jpa/searchparam/MatchUrlServiceTest.java @@ -6,6 +6,7 @@ import ca.uhn.fhir.i18n.Msg; import ca.uhn.fhir.jpa.searchparam.util.Dstu3DistanceHelper; import ca.uhn.fhir.jpa.test.BaseJpaTest; import ca.uhn.fhir.jpa.test.config.TestDstu3Config; +import ca.uhn.fhir.rest.api.SearchTotalModeEnum; import ca.uhn.fhir.rest.param.QuantityParam; import ca.uhn.fhir.rest.param.ReferenceParam; import ca.uhn.fhir.rest.server.util.ISearchParamRegistry; @@ -97,6 +98,26 @@ public class MatchUrlServiceTest extends BaseJpaTest { } } + @Test + void testTotal_fromStandardLowerCase() { + // given + // when + var map = myMatchUrlService.translateMatchUrl("Patient?family=smith&_total=none", ourCtx.getResourceDefinition("Patient")); + + // then + assertEquals(SearchTotalModeEnum.NONE, map.getSearchTotalMode()); + } + + @Test + void testTotal_fromUpperCase() { + // given + // when + var map = myMatchUrlService.translateMatchUrl("Patient?family=smith&_total=none", ourCtx.getResourceDefinition("Patient")); + + // then + assertEquals(SearchTotalModeEnum.NONE, map.getSearchTotalMode()); + } + @Override protected FhirContext getFhirContext() { return ourCtx; diff --git a/hapi-fhir-jpaserver-test-r5/src/test/java/ca/uhn/fhir/jpa/dao/r5/database/BaseDatabaseVerificationIT.java b/hapi-fhir-jpaserver-test-r5/src/test/java/ca/uhn/fhir/jpa/dao/r5/database/BaseDatabaseVerificationIT.java index beeef22ff91..2298bf75de7 100644 --- a/hapi-fhir-jpaserver-test-r5/src/test/java/ca/uhn/fhir/jpa/dao/r5/database/BaseDatabaseVerificationIT.java +++ b/hapi-fhir-jpaserver-test-r5/src/test/java/ca/uhn/fhir/jpa/dao/r5/database/BaseDatabaseVerificationIT.java @@ -11,11 +11,9 @@ import ca.uhn.fhir.jpa.migrate.SchemaMigrator; import ca.uhn.fhir.jpa.migrate.dao.HapiMigrationDao; import ca.uhn.fhir.jpa.migrate.tasks.HapiFhirJpaMigrationTasks; import ca.uhn.fhir.jpa.search.DatabaseBackedPagingProvider; -import ca.uhn.fhir.jpa.searchparam.SearchParameterMap; import ca.uhn.fhir.jpa.test.BaseJpaTest; import ca.uhn.fhir.jpa.test.config.TestR5Config; import ca.uhn.fhir.rest.api.EncodingEnum; -import ca.uhn.fhir.rest.api.SortSpec; import ca.uhn.fhir.rest.api.server.SystemRequestDetails; import ca.uhn.fhir.rest.client.api.IGenericClient; import ca.uhn.fhir.rest.server.exceptions.ResourceGoneException; @@ -36,6 +34,7 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.junit.jupiter.api.extension.RegisterExtension; import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; import org.junit.jupiter.params.provider.ValueSource; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -57,6 +56,7 @@ import java.util.Set; import static ca.uhn.fhir.jpa.model.util.JpaConstants.OPERATION_EVERYTHING; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsInAnyOrder; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -160,22 +160,23 @@ public abstract class BaseDatabaseVerificationIT extends BaseJpaTest implements assertThat(values.toString(), values, containsInAnyOrder(expectedIds.toArray(new String[0]))); } - @Test - void testChainedSort() { - // given - - // when - SearchParameterMap map = SearchParameterMap - .newSynchronous() - .setSort(new SortSpec("Practitioner:general-practitioner.family")); - myCaptureQueriesListener.clear(); - myPatientDao.search(map, mySrd); - + @ParameterizedTest + @CsvSource(textBlock = """ + query string, Patient?name=smith + query date, Observation?date=2021 + query token, Patient?active=true + sort string, Patient?_sort=name + sort date, Observation?_sort=date + sort token, Patient?_sort=active + sort chained date, Observation?_sort=patient.birthdate + sort chained string, Observation?_sort=patient.name + sort chained qualified, Patient?_sort=Practitioner:general-practitioner.family + sort chained token, Observation?_sort=patient.active + """) + void testSyntaxForVariousQueries(String theMessage, String theQuery) { + assertDoesNotThrow(()->myTestDaoSearch.searchForBundleProvider(theQuery), theMessage); } - - - @Configuration public static class TestConfig extends TestR5Config { @@ -238,11 +239,13 @@ public abstract class BaseDatabaseVerificationIT extends BaseJpaTest implements } } + @SuppressWarnings("unchecked") @Override public IIdType doCreateResource(IBaseResource theResource) { return myDaoRegistry.getResourceDao(myFhirContext.getResourceType(theResource)).create(theResource, new SystemRequestDetails()).getId(); } + @SuppressWarnings("unchecked") @Override public IIdType doUpdateResource(IBaseResource theResource) { return myDaoRegistry.getResourceDao(myFhirContext.getResourceType(theResource)).update(theResource, new SystemRequestDetails()).getId(); diff --git a/hapi-fhir-storage-test-utilities/src/main/java/ca/uhn/fhir/storage/test/DaoTestDataBuilder.java b/hapi-fhir-storage-test-utilities/src/main/java/ca/uhn/fhir/storage/test/DaoTestDataBuilder.java index 483736324b2..cd7f6019568 100644 --- a/hapi-fhir-storage-test-utilities/src/main/java/ca/uhn/fhir/storage/test/DaoTestDataBuilder.java +++ b/hapi-fhir-storage-test-utilities/src/main/java/ca/uhn/fhir/storage/test/DaoTestDataBuilder.java @@ -25,6 +25,7 @@ import ca.uhn.fhir.jpa.api.dao.IFhirResourceDao; import ca.uhn.fhir.rest.api.server.RequestDetails; import ca.uhn.fhir.rest.api.server.SystemRequestDetails; import ca.uhn.fhir.test.utilities.ITestDataBuilder; +import ca.uhn.fhir.util.BundleBuilder; import com.google.common.collect.HashMultimap; import com.google.common.collect.SetMultimap; import org.hl7.fhir.instance.model.api.IBaseResource; @@ -97,21 +98,26 @@ public class DaoTestDataBuilder implements ITestDataBuilder.WithSupport, ITestDa } /** - * Delete anything created + * Delete anything created by this builder since the last cleanup(). */ public void cleanup() { ourLog.info("cleanup {}", myIds); - myIds.keySet().stream() - .sorted() // Hack to ensure Patients are deleted before Practitioners. This may need to be refined. - .forEach(nextType->{ - // todo do this in a bundle for perf. - IFhirResourceDao dao = myDaoRegistry.getResourceDao(nextType); - myIds.get(nextType).forEach(dao::delete); - }); + var builder = new BundleBuilder(myFhirCtx); + myIds.values() + .forEach(builder::addTransactionDeleteEntry); + var bundle = builder.getBundle(); + + ourLog.trace("Deleting in bundle {}", myFhirCtx.newJsonParser().encodeToString(bundle)); + //noinspection unchecked + myDaoRegistry.getSystemDao().transaction(mySrd, bundle); + myIds.clear(); } + /** + * Tear down and cleanup any Resources created during execution. + */ @Override public void afterEach(ExtensionContext context) throws Exception { cleanup();