From d0d63fc5b4da81e82df1d9d39ab198508c7cb13c Mon Sep 17 00:00:00 2001 From: jdar Date: Mon, 3 Mar 2025 17:06:06 -0800 Subject: [PATCH 1/6] initial failing test --- .../provider/r4/MultitenantServerR4Test.java | 85 +++++++++++++++++++ 1 file changed, 85 insertions(+) diff --git a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/provider/r4/MultitenantServerR4Test.java b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/provider/r4/MultitenantServerR4Test.java index c8c053962ae..31885226bce 100644 --- a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/provider/r4/MultitenantServerR4Test.java +++ b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/provider/r4/MultitenantServerR4Test.java @@ -5,13 +5,19 @@ import ca.uhn.fhir.batch2.model.JobInstance; import ca.uhn.fhir.batch2.model.StatusEnum; import ca.uhn.fhir.i18n.Msg; +import ca.uhn.fhir.jpa.api.config.JpaStorageSettings; import ca.uhn.fhir.jpa.api.model.BulkExportJobResults; import ca.uhn.fhir.jpa.batch.models.Batch2JobStartResponse; import ca.uhn.fhir.jpa.bulk.export.model.BulkExportResponseJson; +import ca.uhn.fhir.jpa.dao.r4.FhirResourceDaoR4ConcurrentWriteTest; import ca.uhn.fhir.jpa.entity.PartitionEntity; +import ca.uhn.fhir.jpa.interceptor.TransactionConcurrencySemaphoreInterceptor; import ca.uhn.fhir.jpa.model.config.PartitionSettings; +import ca.uhn.fhir.jpa.model.dao.JpaPid; import ca.uhn.fhir.jpa.model.entity.ResourceTable; +import ca.uhn.fhir.jpa.model.entity.StorageSettings; import ca.uhn.fhir.jpa.model.util.JpaConstants; +import ca.uhn.fhir.jpa.util.MemoryCacheService; import ca.uhn.fhir.model.api.Include; import ca.uhn.fhir.rest.api.Constants; import ca.uhn.fhir.rest.api.server.RequestDetails; @@ -25,9 +31,20 @@ import ca.uhn.fhir.rest.server.provider.ProviderConstants; import ca.uhn.fhir.rest.server.servlet.ServletRequestDetails; import ca.uhn.fhir.test.utilities.ITestDataBuilder; +import ca.uhn.fhir.util.BundleBuilder; import ca.uhn.fhir.util.JsonUtil; import com.google.common.collect.Sets; +import jakarta.annotation.Nonnull; import jakarta.servlet.http.HttpServletResponse; + +import java.util.ArrayList; +import java.util.Map; +import java.util.TreeMap; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; + import org.apache.commons.io.IOUtils; import org.apache.http.client.methods.CloseableHttpResponse; import org.apache.http.client.methods.HttpGet; @@ -35,12 +52,16 @@ import org.hl7.fhir.instance.model.api.IIdType; import org.hl7.fhir.r4.model.Bundle; import org.hl7.fhir.r4.model.CapabilityStatement; +import org.hl7.fhir.r4.model.CodeType; import org.hl7.fhir.r4.model.Condition; +import org.hl7.fhir.r4.model.DateType; +import org.hl7.fhir.r4.model.Encounter; import org.hl7.fhir.r4.model.IdType; import org.hl7.fhir.r4.model.IntegerType; import org.hl7.fhir.r4.model.Organization; import org.hl7.fhir.r4.model.Parameters; import org.hl7.fhir.r4.model.Patient; +import org.hl7.fhir.r4.model.Reference; import org.hl7.fhir.r4.model.Resource; import org.hl7.fhir.r4.model.StringType; import org.junit.jupiter.api.AfterEach; @@ -50,8 +71,18 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ValueSource; +import org.mockito.ArgumentCaptor; + +import static org.mockito.ArgumentMatchers.eq; + +import org.mockito.Captor; import org.mockito.InjectMocks; import org.mockito.Mock; + +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.reset; +import static org.mockito.Mockito.verify; + import org.springframework.mock.web.MockHttpServletRequest; import java.io.IOException; @@ -79,12 +110,17 @@ @SuppressWarnings("Duplicates") public class MultitenantServerR4Test extends BaseMultitenantResourceProviderR4Test implements ITestDataBuilder { + @Captor + private ArgumentCaptor myMatchUrlCacheValueCaptor; @Override @AfterEach public void after() throws Exception { super.after(); + JpaStorageSettings defaultStorageSettings = new JpaStorageSettings(); + myPartitionSettings.setAllowReferencesAcrossPartitions(PartitionSettings.CrossPartitionReferenceMode.NOT_ALLOWED); + myStorageSettings.setMatchUrlCacheEnabled(defaultStorageSettings.isMatchUrlCacheEnabled()); assertFalse(myPartitionSettings.isAllowUnqualifiedCrossPartitionReference()); } @@ -510,6 +546,54 @@ public void testTransactionPut_withSearchNarrowingInterceptor_createsPatient() { assertEquals("Family", patient1.getName().get(0).getFamily()); } + + @Test + public void testUpdate_withConditionalReferenceAndMatchUrlCache_sameMatchUrlInDifferentPartitionShouldNotBeFound() { + // Given + myStorageSettings.setMatchUrlCacheEnabled(true); + + IBaseResource patientA = buildPatient(withTenant(TENANT_A), withActiveTrue(), withId("1234a"), + withFamily("Family"), withGiven("Given")); + myClient.update().resource(patientA).execute(); + + SystemRequestDetails requestDetails = new SystemRequestDetails(); + requestDetails.setTenantId(TENANT_A); + JpaPid patientPid = (JpaPid) myPatientDao.readEntity(new IdType("1234a"), requestDetails).getPersistentId(); + + Encounter encounter = new Encounter(); + encounter.setId("1234b"); + String thePatientMatchUrl = "Patient?_id=1234a"; + encounter.setSubject(new Reference(thePatientMatchUrl)); + + // When + myTenantClientInterceptor.setTenantId(TENANT_A); + myClient.update().resource(encounter).execute(); + + // Then: ensure the Encounter is updated + Encounter encounter1 = myEncounterDao.read(new IdType("Encounter/1234b"), requestDetails); + assertEquals("Patient/1234a", encounter1.getSubject().getReference()); + + // Also ensure that the match URL cache is populated after resolving + verify(myMemoryCacheService).putAfterCommit(eq(MemoryCacheService.CacheEnum.MATCH_URL), eq(thePatientMatchUrl), myMatchUrlCacheValueCaptor.capture()); + JpaPid actualCachedPid = myMatchUrlCacheValueCaptor.getValue(); + assertThat(actualCachedPid.getId()).isEqualTo(patientPid.getId()); + assertThat(actualCachedPid.getPartitionId()).isEqualTo(TENANT_A_ID); + + reset(myMemoryCacheService); + encounter.setId("1234c"); + + try { + // When: try to update the Encounter again with the same Patient match url, but on Partition B + myTenantClientInterceptor.setTenantId(TENANT_B); + myClient.update().resource(encounter).execute(); + fail(); + } catch (ResourceNotFoundException e) { + // Then: the match URL should fail to resolve + assertThat(e.getMessage()).contains("Invalid match URL \"" + thePatientMatchUrl + "\" - No resources match this search"); + verify(myMemoryCacheService, never()).putAfterCommit(eq(MemoryCacheService.CacheEnum.MATCH_URL), eq(thePatientMatchUrl), any()); + } + } + @ParameterizedTest @ValueSource(strings = {"Patient/1234a", "TENANT-B/Patient/1234a"}) public void testTransactionGet_withSearchNarrowingInterceptor_retrievesPatient(String theEntryUrl) { @@ -813,4 +897,5 @@ private String buildExportUrl(String createInPartition, String jobId) { + JpaConstants.PARAM_EXPORT_POLL_STATUS_JOB_ID + "=" + jobId; } } + } From cb31d059c605e92b5b9e34f102d5815f06f8a28d Mon Sep 17 00:00:00 2001 From: jdar Date: Tue, 4 Mar 2025 09:51:14 -0800 Subject: [PATCH 2/6] fix and a bunch of other tests --- .../java/ca/uhn/fhir/util/BundleBuilder.java | 24 ++- .../fhir/jpa/dao/BaseHapiFhirResourceDao.java | 9 +- .../fhir/jpa/dao/TransactionProcessor.java | 29 ++- ...actionConcurrencySemaphoreInterceptor.java | 2 +- .../jpa/dao/MatchResourceUrlServiceTest.java | 123 ++++++++++++ .../bulk/imprt2/ConsumeFilesStepR4Test.java | 3 +- .../jpa/dao/r4/BasePartitioningR4Test.java | 25 ++- .../dao/r4/PartitioningSearchCacheR4Test.java | 45 +++++ .../provider/r4/MultitenantServerR4Test.java | 175 ++++++++++++++++++ .../ca/uhn/fhir/jpa/dao/r5/BaseJpaR5Test.java | 5 + .../dao/r5/CrossPartitionReferencesTest.java | 78 ++++++++ .../ca/uhn/fhir/jpa/test/BaseJpaR4Test.java | 3 +- .../fhir/jpa/dao/BaseStorageResourceDao.java | 20 +- .../fhir/jpa/dao/MatchResourceUrlService.java | 23 ++- ...rchParamWithInlineReferencesExtractor.java | 14 +- .../ca/uhn/fhir/util/BundleBuilderR4Test.java | 20 ++ 16 files changed, 565 insertions(+), 33 deletions(-) create mode 100644 hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/MatchResourceUrlServiceTest.java diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/BundleBuilder.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/BundleBuilder.java index 2da6dc27a23..8d9607e9216 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/BundleBuilder.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/BundleBuilder.java @@ -27,6 +27,7 @@ import ca.uhn.fhir.model.primitive.IdDt; import jakarta.annotation.Nonnull; import jakarta.annotation.Nullable; +import org.apache.commons.lang3.StringUtils; import org.apache.commons.lang3.Validate; import org.hl7.fhir.instance.model.api.IBase; import org.hl7.fhir.instance.model.api.IBaseBackboneElement; @@ -194,13 +195,26 @@ public PatchBuilder addTransactionFhirPatchEntry(IBaseParameters thePatch) { * @param theResource The resource to update */ public UpdateBuilder addTransactionUpdateEntry(IBaseResource theResource) { + return addTransactionUpdateEntry(theResource, null); + } + + /** + * Adds an entry containing an update (PUT) request. + * Also sets the Bundle.type value to "transaction" if it is not already set. + * + * @param theResource The resource to update + * @param theRequestUrl The url to attach to the Bundle.entry.request.url. If null, will default to the resource ID. + */ + public UpdateBuilder addTransactionUpdateEntry(IBaseResource theResource, String theRequestUrl) { Validate.notNull(theResource, "theResource must not be null"); IIdType id = getIdTypeForUpdate(theResource); - String requestUrl = id.toUnqualifiedVersionless().getValue(); String fullUrl = id.getValue(); String verb = "PUT"; + String requestUrl = StringUtils.isBlank(theRequestUrl) + ? id.toUnqualifiedVersionless().getValue() + : theRequestUrl; IPrimitiveType url = addAndPopulateTransactionBundleEntryRequest(theResource, fullUrl, requestUrl, verb); @@ -215,10 +229,7 @@ private IPrimitiveType addAndPopulateTransactionBundleEntryRequest( IBase request = addEntryAndReturnRequest(theResource, theFullUrl); // Bundle.entry.request.url - IPrimitiveType url = - (IPrimitiveType) myContext.getElementDefinition("uri").newInstance(); - url.setValueAsString(theRequestUrl); - myEntryRequestUrlChild.getMutator().setValue(request, url); + IPrimitiveType url = addRequestUrl(request, theRequestUrl); // Bundle.entry.request.method addRequestMethod(request, theHttpVerb); @@ -416,11 +427,12 @@ private void addFullUrl(IBase theEntry, String theFullUrl) { myEntryFullUrlChild.getMutator().setValue(theEntry, fullUrl); } - private void addRequestUrl(IBase request, String theRequestUrl) { + private IPrimitiveType addRequestUrl(IBase request, String theRequestUrl) { IPrimitiveType url = (IPrimitiveType) myContext.getElementDefinition("uri").newInstance(); url.setValueAsString(theRequestUrl); myEntryRequestUrlChild.getMutator().setValue(request, url); + return url; } private void addRequestMethod(IBase theRequest, String theMethod) { diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java index 437dc33e9b8..d929f05342c 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java @@ -302,6 +302,11 @@ protected IDeleteExpungeJobSubmitter getDeleteExpungeJobSubmitter() { return myDeleteExpungeJobSubmitter; } + @Override + protected IRequestPartitionHelperSvc getRequestPartitionHelperService() { + return myRequestPartitionHelperService; + } + /** * @deprecated Use {@link #create(T, RequestDetails)} instead */ @@ -425,7 +430,7 @@ private DaoMethodOutcome doCreateForPostOrPut( if (isNotBlank(theMatchUrl) && theProcessMatchUrl) { Set match = myMatchResourceUrlService.processMatchUrl( - theMatchUrl, myResourceType, theTransactionDetails, theRequest); + theMatchUrl, myResourceType, theTransactionDetails, theRequest, theRequestPartitionId); ourLog.trace("Resolving match URL {} found: {}", theMatchUrl, match); if (match.size() > 1) { String msg = getContext() @@ -2393,7 +2398,7 @@ private DaoMethodOutcome doUpdate( if (isNotBlank(theMatchUrl)) { // Validate that the supplied resource matches the conditional. Set match = myMatchResourceUrlService.processMatchUrl( - theMatchUrl, myResourceType, theTransactionDetails, theRequest, theResource); + theMatchUrl, myResourceType, theTransactionDetails, theRequest, theResource, theRequestPartitionId); if (match.size() > 1) { String msg = getContext() .getLocalizer() diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/TransactionProcessor.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/TransactionProcessor.java index 16837edded6..709365ec1bd 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/TransactionProcessor.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/TransactionProcessor.java @@ -358,10 +358,21 @@ private void preFetchConditionalUrls( resourceType = myFhirContext.getResourceType(resource); } if (("PUT".equals(verb) || "PATCH".equals(verb)) && requestUrl != null && requestUrl.contains("?")) { - preFetchConditionalUrl(resourceType, requestUrl, true, idsToPreFetch, searchParameterMapsToResolve); + preFetchConditionalUrl( + resourceType, + requestUrl, + true, + idsToPreFetch, + searchParameterMapsToResolve, + theRequestPartitionId); } else if ("POST".equals(verb) && requestIfNoneExist != null && requestIfNoneExist.contains("?")) { preFetchConditionalUrl( - resourceType, requestIfNoneExist, false, idsToPreFetch, searchParameterMapsToResolve); + resourceType, + requestIfNoneExist, + false, + idsToPreFetch, + searchParameterMapsToResolve, + theRequestPartitionId); } if (myStorageSettings.isAllowInlineMatchUrlReferences()) { @@ -374,7 +385,12 @@ private void preFetchConditionalUrls( String refResourceType = determineResourceTypeInResourceUrl(myFhirContext, referenceUrl); if (refResourceType != null) { preFetchConditionalUrl( - refResourceType, referenceUrl, false, idsToPreFetch, searchParameterMapsToResolve); + refResourceType, + referenceUrl, + false, + idsToPreFetch, + searchParameterMapsToResolve, + theRequestPartitionId); } } } @@ -546,14 +562,17 @@ private void preFetchSearchParameterMapsToken( * @param theShouldPreFetchResourceBody Should we also fetch the actual resource body, or just figure out the PID associated with it. See the method javadoc above for some context. * @param theOutputIdsToPreFetch This will be populated with any resource PIDs that need to be pre-fetched * @param theOutputSearchParameterMapsToResolve This will be populated with any {@link SearchParameterMap} instances corresponding to match URLs we need to resolve + * @param thePartitionId The partition ID of the associated resource (can be null) */ private void preFetchConditionalUrl( String theResourceType, String theRequestUrl, boolean theShouldPreFetchResourceBody, List theOutputIdsToPreFetch, - List theOutputSearchParameterMapsToResolve) { - JpaPid cachedId = myMatchResourceUrlService.processMatchUrlUsingCacheOnly(theResourceType, theRequestUrl); + List theOutputSearchParameterMapsToResolve, + RequestPartitionId thePartitionId) { + JpaPid cachedId = + myMatchResourceUrlService.processMatchUrlUsingCacheOnly(theResourceType, theRequestUrl, thePartitionId); if (cachedId != null) { if (theShouldPreFetchResourceBody) { theOutputIdsToPreFetch.add(cachedId.getId()); diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/interceptor/TransactionConcurrencySemaphoreInterceptor.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/interceptor/TransactionConcurrencySemaphoreInterceptor.java index 20045a9b1fd..719f9c33cff 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/interceptor/TransactionConcurrencySemaphoreInterceptor.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/interceptor/TransactionConcurrencySemaphoreInterceptor.java @@ -180,7 +180,7 @@ public void post(TransactionDetails theTransactionDetails) { } /** - * Clear all semaphors from the list. This is really mostly intended for testing scenarios. + * Clear all semaphores from the list. This is really mostly intended for testing scenarios. */ public void clearSemaphores() { mySemaphoreCache.invalidateAll(); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/MatchResourceUrlServiceTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/MatchResourceUrlServiceTest.java new file mode 100644 index 00000000000..9811aeb5bc5 --- /dev/null +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/MatchResourceUrlServiceTest.java @@ -0,0 +1,123 @@ +package ca.uhn.fhir.jpa.dao; + +import ca.uhn.fhir.context.FhirContext; +import ca.uhn.fhir.interceptor.model.RequestPartitionId; +import ca.uhn.fhir.jpa.api.config.JpaStorageSettings; +import ca.uhn.fhir.jpa.api.dao.DaoRegistry; +import ca.uhn.fhir.jpa.api.dao.IFhirResourceDao; +import ca.uhn.fhir.jpa.model.dao.JpaPid; +import ca.uhn.fhir.jpa.searchparam.MatchUrlService; +import ca.uhn.fhir.jpa.searchparam.SearchParameterMap; +import ca.uhn.fhir.jpa.util.MemoryCacheService; + +import ca.uhn.fhir.rest.api.server.RequestDetails; +import ca.uhn.fhir.rest.api.server.storage.TransactionDetails; + +import ca.uhn.fhir.rest.param.DateRangeParam; + +import java.util.List; + +import static org.assertj.core.api.Assertions.assertThat; + +import org.hl7.fhir.r4.model.Patient; + +import static org.junit.jupiter.api.Assertions.*; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; + +import static org.mockito.ArgumentMatchers.any; + +import org.mockito.InjectMocks; +import org.mockito.Mock; + +import static org.mockito.Mockito.when; + +import org.mockito.Spy; +import org.mockito.junit.jupiter.MockitoExtension; + +@ExtendWith(MockitoExtension.class) +class MatchResourceUrlServiceTest { + @Spy + private JpaStorageSettings myStorageSettings = new JpaStorageSettings(); + @Spy + private MemoryCacheService myMemoryCacheService = new MemoryCacheService(myStorageSettings); + + @Mock + private TransactionDetails myTransactionDetails; + + @Mock + private RequestDetails myRequestDetails; + + @Mock + private DaoRegistry myDaoRegistry; + + @Mock + private IFhirResourceDao myFhirResourceDao; + + @Mock + private FhirContext myCtx = FhirContext.forR4(); + + @Mock + private MatchUrlService myMatchUrlSvc; + + @InjectMocks + private MatchResourceUrlService myMatchResourceUrlSvc = new MatchResourceUrlService(); + + @BeforeEach + public void beforeEach() { + myMemoryCacheService.invalidateAllCaches(); + } + + @Test + void testProcessMatchUrlUsingCacheOnly_shouldNotReturnPidsFromWrongPartition() { + myStorageSettings.setMatchUrlCacheEnabled(true); + + String matchUrl = "Patient?identifier=test|123"; + final int partitionId = 1; + JpaPid cachedPid = JpaPid.fromId(1L); + cachedPid.setPartitionId(partitionId); + + myMatchResourceUrlSvc.matchUrlResolved(myTransactionDetails, "Patient", matchUrl, cachedPid); + + JpaPid pid = myMatchResourceUrlSvc.processMatchUrlUsingCacheOnly("Patient", matchUrl, RequestPartitionId.fromPartitionId(1)); + assertNotNull(pid); + assertThat(pid.getPartitionId()).isEqualTo(partitionId); + assertThat(pid.getId()).isEqualTo(1L); + + pid = myMatchResourceUrlSvc.processMatchUrlUsingCacheOnly("Patient", matchUrl, RequestPartitionId.allPartitions()); + assertNotNull(pid); + assertThat(pid.getPartitionId()).isEqualTo(partitionId); + + pid = myMatchResourceUrlSvc.processMatchUrlUsingCacheOnly("Patient", matchUrl, RequestPartitionId.fromPartitionId(2)); + assertNull(pid); + + pid = myMatchResourceUrlSvc.processMatchUrlUsingCacheOnly("Patient", matchUrl, RequestPartitionId.fromPartitionId(null)); + assertNull(pid); + } + + @Test + void testProcessMatchUrl_storesFoundMatchInCache() { + myStorageSettings.setMatchUrlCacheEnabled(true); + + String matchUrl = "Patient?identifier=test|123"; + final int partitionId = 1; + JpaPid cachedPid = JpaPid.fromId(1L); + cachedPid.setPartitionId(partitionId); + + SearchParameterMap sp = new SearchParameterMap(); + sp.setLastUpdated(new DateRangeParam().setLowerBound("2024").setUpperBound("2025")); + + when(myDaoRegistry.getResourceDao(Patient.class)).thenReturn(myFhirResourceDao); + when(myFhirResourceDao.searchForIds(any(), any(), any())).thenReturn(List.of(cachedPid)); + when(myMatchUrlSvc.translateMatchUrl(any(), any())).thenReturn(sp); + + myMatchResourceUrlSvc.processMatchUrl(matchUrl, Patient.class, myTransactionDetails, myRequestDetails, RequestPartitionId.fromPartitionId(1)); + + JpaPid pid = myMatchResourceUrlSvc.processMatchUrlUsingCacheOnly("Patient", matchUrl, RequestPartitionId.fromPartitionId(1)); + assertNotNull(pid); + assertThat(pid.getPartitionId()).isEqualTo(partitionId); + assertThat(pid.getId()).isEqualTo(1L); + } +} diff --git a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/bulk/imprt2/ConsumeFilesStepR4Test.java b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/bulk/imprt2/ConsumeFilesStepR4Test.java index 282d62feb5d..f7bf2d1cdc7 100644 --- a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/bulk/imprt2/ConsumeFilesStepR4Test.java +++ b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/bulk/imprt2/ConsumeFilesStepR4Test.java @@ -102,8 +102,7 @@ public void testAlreadyExisting_WithChanges(boolean partitionEnabled) { if (partitionEnabled) { myPartitionSettings.setPartitioningEnabled(true); myPartitionSettings.setIncludePartitionInSearchHashes(true); - addCreatePartition(1); - addCreatePartition(1); + addCreatePartitionNTimes(1, 2); } Patient patient = new Patient(); patient.setId("A"); diff --git a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/BasePartitioningR4Test.java b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/BasePartitioningR4Test.java index b7da51350fe..7a547a758c1 100644 --- a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/BasePartitioningR4Test.java +++ b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/BasePartitioningR4Test.java @@ -61,17 +61,20 @@ public abstract class BasePartitioningR4Test extends BaseJpaR4SystemTest { public void after() { assertNoRemainingPartitionIds(); - myPartitionSettings.setIncludePartitionInSearchHashes(new PartitionSettings().isIncludePartitionInSearchHashes()); - myPartitionSettings.setPartitioningEnabled(new PartitionSettings().isPartitioningEnabled()); - myPartitionSettings.setAllowReferencesAcrossPartitions(new PartitionSettings().getAllowReferencesAcrossPartitions()); - myPartitionSettings.setDefaultPartitionId(new PartitionSettings().getDefaultPartitionId()); + PartitionSettings defaultPartitionSettings = new PartitionSettings(); + JpaStorageSettings defaultStorageSettings = new JpaStorageSettings(); + + myPartitionSettings.setIncludePartitionInSearchHashes(defaultPartitionSettings.isIncludePartitionInSearchHashes()); + myPartitionSettings.setPartitioningEnabled(defaultPartitionSettings.isPartitioningEnabled()); + myPartitionSettings.setAllowReferencesAcrossPartitions(defaultPartitionSettings.getAllowReferencesAcrossPartitions()); + myPartitionSettings.setDefaultPartitionId(defaultPartitionSettings.getDefaultPartitionId()); mySrdInterceptorService.unregisterInterceptorsIf(t -> t instanceof MyReadWriteInterceptor); - myStorageSettings.setIndexMissingFields(new JpaStorageSettings().getIndexMissingFields()); - myStorageSettings.setAutoCreatePlaceholderReferenceTargets(new JpaStorageSettings().isAutoCreatePlaceholderReferenceTargets()); - myStorageSettings.setMassIngestionMode(new JpaStorageSettings().isMassIngestionMode()); - myStorageSettings.setMatchUrlCacheEnabled(new JpaStorageSettings().getMatchUrlCache()); + myStorageSettings.setIndexMissingFields(defaultStorageSettings.getIndexMissingFields()); + myStorageSettings.setAutoCreatePlaceholderReferenceTargets(defaultStorageSettings.isAutoCreatePlaceholderReferenceTargets()); + myStorageSettings.setMassIngestionMode(defaultStorageSettings.isMassIngestionMode()); + myStorageSettings.setMatchUrlCacheEnabled(defaultStorageSettings.isMatchUrlCacheEnabled()); if (myRegisteredSearchParamValidatingInterceptor) { myInterceptorRegistry.unregisterInterceptor(mySearchParamValidatingInterceptor); @@ -238,6 +241,12 @@ protected void dropForcedIdUniqueConstraint() { myHaveDroppedForcedIdUniqueConstraint = true; } + protected void addCreatePartitionNTimes(Integer thePartitionId, Integer theNumberOfTimes) { + for (int i = 0; i < theNumberOfTimes; i++) { + addCreatePartition(thePartitionId, null); + } + } + protected void addCreatePartition(Integer thePartitionId) { addCreatePartition(thePartitionId, null); } diff --git a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/PartitioningSearchCacheR4Test.java b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/PartitioningSearchCacheR4Test.java index 0f4673fdb47..5668cabb0c1 100644 --- a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/PartitioningSearchCacheR4Test.java +++ b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/PartitioningSearchCacheR4Test.java @@ -1,5 +1,16 @@ package ca.uhn.fhir.jpa.dao.r4; +import ca.uhn.fhir.util.BundleBuilder; + +import org.hl7.fhir.r4.model.Bundle; +import org.hl7.fhir.r4.model.CodeableConcept; +import org.hl7.fhir.r4.model.HumanName; +import org.hl7.fhir.r4.model.Identifier; +import org.hl7.fhir.r4.model.OperationOutcome; +import org.hl7.fhir.r4.model.Patient; + +import org.hl7.fhir.r4.model.StringType; + import static org.junit.jupiter.api.Assertions.assertEquals; import ca.uhn.fhir.jpa.api.dao.PatientEverythingParameters; @@ -155,4 +166,38 @@ public void testSearch_MultiplePartitions_UseCache() { } + @Test + public void testConditionalCreate_withMultiplePartitionsAndMatchUrlCache_NoCachePartitionConflicts() { + myStorageSettings.setMatchUrlCacheEnabled(true); + myPartitionSettings.setConditionalCreateDuplicateIdentifiersEnabled(true); + + Bundle responseBundle1 = mySystemDao.transaction(mySrd, createPatientWithConditionalUrlOnPartition(1)); + assertResourceCreated(responseBundle1); + + Bundle responseBundle2 = mySystemDao.transaction(mySrd, createPatientWithConditionalUrlOnPartition(2)); + assertResourceCreated(responseBundle2); + } + + private Bundle createPatientWithConditionalUrlOnPartition(Integer thePartitionId) { + BundleBuilder bb = new BundleBuilder(myFhirContext); + + Patient p = new Patient(); + p.setIdentifier(List.of(new Identifier().setSystem("foo").setValue("bar"))); + p.setActive(true); + p.setName(List.of(new HumanName().setFamily("ABC").setGiven(List.of(new StringType("DEF"))))); + bb.addTransactionUpdateEntry(p, "Patient?identifier=foo|bar"); + addCreatePartitionNTimes(thePartitionId, 3); + + return (Bundle) bb.getBundle(); + } + + private static void assertResourceCreated(Bundle responseBundle1) { + assertThat(responseBundle1.getEntry()).hasSize(1); + Bundle.BundleEntryResponseComponent response1 = responseBundle1.getEntryFirstRep().getResponse(); + assertThat(response1.getStatus()).isEqualTo("201 Created"); + OperationOutcome oo1 = (OperationOutcome) response1.getOutcome(); + assertThat(oo1.getIssue()).hasSize(1); + CodeableConcept details1 = oo1.getIssueFirstRep().getDetails(); + assertThat(details1.getCodingFirstRep().getCode()).isEqualTo("SUCCESSFUL_UPDATE_NO_CONDITIONAL_MATCH"); + } } diff --git a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/provider/r4/MultitenantServerR4Test.java b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/provider/r4/MultitenantServerR4Test.java index 31885226bce..a5bcbb96942 100644 --- a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/provider/r4/MultitenantServerR4Test.java +++ b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/provider/r4/MultitenantServerR4Test.java @@ -546,6 +546,72 @@ public void testTransactionPut_withSearchNarrowingInterceptor_createsPatient() { assertEquals("Family", patient1.getName().get(0).getFamily()); } + @Test + public void testTransactionPatch_withConditionalAndMatchUrlCache_sameMatchUrlInDifferentPartitionShouldNotBeFound() { + // Given + myStorageSettings.setMatchUrlCacheEnabled(true); + + IBaseResource patientToCreate = buildPatient(withTenant(TENANT_A), withActiveTrue(), withId("1234a"), + withFamily("Family"), withGiven("Given"), withBirthdate("1970-01-01")); + myClient.update().resource(patientToCreate).execute(); + + SystemRequestDetails requestDetails = new SystemRequestDetails(); + requestDetails.setTenantId(TENANT_A); + JpaPid patientPid = (JpaPid) myPatientDao.readEntity(new IdType("1234a"), requestDetails).getPersistentId(); + + String thePatientMatchUrl = "Patient?_id=1234a"; + Bundle patchBundle1 = createPatchBundleReplaceBirthDateOnPatient("2000-01-01", thePatientMatchUrl); + + // When + myTenantClientInterceptor.setTenantId(TENANT_A); + myClient.transaction().withBundle(patchBundle1).execute(); + Patient patient1 = myPatientDao.read(new IdType("Patient/1234a"), requestDetails); + + // Then: the Patch is successful and the match url is cached + assertEquals("Family", patient1.getName().get(0).getFamily()); + assertThat(patient1.getBirthDateElement().getValueAsString()).isEqualTo("2000-01-01"); + + // Verify that the entry is put into cache + verify(myMemoryCacheService).putAfterCommit(eq(MemoryCacheService.CacheEnum.MATCH_URL), eq(thePatientMatchUrl), myMatchUrlCacheValueCaptor.capture()); + JpaPid actualCachedPid = myMatchUrlCacheValueCaptor.getValue(); + assertThat(actualCachedPid.getId()).isEqualTo(patientPid.getId()); + assertThat(actualCachedPid.getPartitionId()).isEqualTo(TENANT_A_ID); + + reset(myMemoryCacheService); + Bundle patchBundle2 = createPatchBundleReplaceBirthDateOnPatient("2025-01-01", thePatientMatchUrl); + + try { + // When: Perform Patch with the same match url, but in another partition + myTenantClientInterceptor.setTenantId(TENANT_B); + myClient.transaction().withBundle(patchBundle2).execute(); + fail(); + } catch (ResourceNotFoundException e) { + // Then: the match URl cache should not be resolved + assertThat(e.getMessage()).contains("Invalid match URL \"" + thePatientMatchUrl + "\" - No resources match this search"); + Patient patientInDb = myPatientDao.read(new IdType("Patient/1234a"), requestDetails); + assertEquals("Family", patientInDb.getName().get(0).getFamily()); + assertThat(patientInDb.getBirthDateElement().getValueAsString()).isEqualTo("2000-01-01"); + verify(myMemoryCacheService, never()).putAfterCommit(eq(MemoryCacheService.CacheEnum.MATCH_URL), eq(thePatientMatchUrl), any()); + } + } + + private static Bundle createPatchBundleReplaceBirthDateOnPatient(String theDate, String thePatientUrl) { + Parameters patch = new Parameters(); + Parameters.ParametersParameterComponent operation = patch.addParameter(); + operation.setName("operation"); + operation.addPart().setName("type").setValue(new CodeType("replace")); + operation.addPart().setName("path").setValue(new CodeType("Patient.birthDate")); + operation.addPart().setName("value").setValue(new DateType(theDate)); + + Bundle input = new Bundle(); + input.setType(Bundle.BundleType.TRANSACTION); + input.addEntry() + .setFullUrl(thePatientUrl) + .setResource(patch) + .getRequest().setUrl(thePatientUrl) + .setMethod(Bundle.HTTPVerb.PATCH); + return input; + } @Test public void testUpdate_withConditionalReferenceAndMatchUrlCache_sameMatchUrlInDifferentPartitionShouldNotBeFound() { @@ -898,4 +964,113 @@ private String buildExportUrl(String createInPartition, String jobId) { } } + /** + * This is a partitioned version of the same test in + * {@link FhirResourceDaoR4ConcurrentWriteTest#testTransactionCreates_WithConcurrencySemaphore_DontLockOnCachedMatchUrlsForConditionalCreate()} + */ + @Nested + class TestConcurrencyInterceptorInPartitioningMode { + private static final int THREAD_COUNT = 10; + + private ExecutorService myExecutor; + private TransactionConcurrencySemaphoreInterceptor myConcurrencySemaphoreInterceptor; + + @BeforeEach + public void before() { + myExecutor = Executors.newFixedThreadPool(THREAD_COUNT); + myConcurrencySemaphoreInterceptor = new TransactionConcurrencySemaphoreInterceptor(myMemoryCacheService); + + RestfulServer server = new RestfulServer(myFhirContext); + when(mySrd.getServer()).thenReturn(server); + } + + @AfterEach + public void after() { + myExecutor.shutdown(); + myInterceptorRegistry.unregisterInterceptor(myConcurrencySemaphoreInterceptor); + } + + @Test + public void testTransactionCreates_WithConcurrencySemaphore_DontLockOnCachedMatchUrlsForConditionalCreate() throws ExecutionException, InterruptedException { + myStorageSettings.setMatchUrlCacheEnabled(true); + myPartitionSettings.setConditionalCreateDuplicateIdentifiersEnabled(true); + myInterceptorRegistry.registerInterceptor(myConcurrencySemaphoreInterceptor); + myConcurrencySemaphoreInterceptor.setLogWaits(true); + + Runnable creatorForPartitionA = ()->{ + Bundle input = createBundleForRunnableTransaction(); + SystemRequestDetails requestDetails = new SystemRequestDetails(); + requestDetails.setTenantId(TENANT_A); + mySystemDao.transaction(requestDetails, input); + }; + + Runnable creatorForPartitionB = ()->{ + Bundle input = createBundleForRunnableTransaction(); + SystemRequestDetails requestDetails = new SystemRequestDetails(); + requestDetails.setTenantId(TENANT_B); + mySystemDao.transaction(requestDetails, input); + }; + + for (int set = 0; set < 3; set++) { + myConcurrencySemaphoreInterceptor.clearSemaphores(); + + List> futures = new ArrayList<>(); + for (int j = 0; j < 10; j++) { + if (j % 2 == 0) { + futures.add(myExecutor.submit(creatorForPartitionA)); + } else { + futures.add(myExecutor.submit(creatorForPartitionB)); + } + + } + + for (Future next : futures) { + next.get(); + } + + // Only a thread from the first iteration (set) will acquire the semaphores for the match urls + // Since the match URLs will still be present in the Match URL cache for the remaining of the test + if (set == 0) { + assertEquals(2, myConcurrencySemaphoreInterceptor.countSemaphores()); + } else { + assertEquals(0, myConcurrencySemaphoreInterceptor.countSemaphores()); + } + } + + runInTransaction(() -> { + Map counts = getResourceCountMap(); + assertEquals(4, counts.get("Patient"), counts.toString()); + }); + } + + private Bundle createBundleForRunnableTransaction() { + BundleBuilder bb = new BundleBuilder(myFhirContext); + + Patient patient1 = new Patient(); + patient1.addIdentifier().setSystem("http://foo").setValue("1"); + bb.addTransactionCreateEntry(patient1).conditional("Patient?identifier=http://foo|1"); + + Patient patient2 = new Patient(); + patient2.addIdentifier().setSystem("http://foo").setValue("2"); + bb.addTransactionCreateEntry(patient2).conditional("Patient?identifier=http://foo|2"); + + return (Bundle) bb.getBundle(); + } + + @Nonnull + private Map getResourceCountMap() { + Map counts = new TreeMap<>(); + myResourceTableDao + .findAll() + .stream() + .forEach(t -> { + counts.putIfAbsent(t.getResourceType(), 0); + int value = counts.get(t.getResourceType()); + value++; + counts.put(t.getResourceType(), value); + }); + ourLog.info("Counts: {}", counts); + return counts; + } + } } diff --git a/hapi-fhir-jpaserver-test-r5/src/test/java/ca/uhn/fhir/jpa/dao/r5/BaseJpaR5Test.java b/hapi-fhir-jpaserver-test-r5/src/test/java/ca/uhn/fhir/jpa/dao/r5/BaseJpaR5Test.java index d1b670004c7..cf0bc6826c0 100644 --- a/hapi-fhir-jpaserver-test-r5/src/test/java/ca/uhn/fhir/jpa/dao/r5/BaseJpaR5Test.java +++ b/hapi-fhir-jpaserver-test-r5/src/test/java/ca/uhn/fhir/jpa/dao/r5/BaseJpaR5Test.java @@ -64,6 +64,7 @@ import ca.uhn.fhir.jpa.test.BaseJpaTest; import ca.uhn.fhir.jpa.test.PreventDanglingInterceptorsExtension; import ca.uhn.fhir.jpa.test.config.TestR5Config; +import ca.uhn.fhir.jpa.util.MemoryCacheService; import ca.uhn.fhir.jpa.util.ResourceCountCache; import ca.uhn.fhir.parser.StrictErrorHandler; import ca.uhn.fhir.rest.server.BasePagingProvider; @@ -126,6 +127,7 @@ import org.junit.jupiter.api.extension.RegisterExtension; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Qualifier; +import org.springframework.boot.test.mock.mockito.SpyBean; import org.springframework.context.ApplicationContext; import org.springframework.test.context.ContextConfiguration; import org.springframework.test.context.junit.jupiter.SpringExtension; @@ -383,6 +385,8 @@ public abstract class BaseJpaR5Test extends BaseJpaTest implements ITestDataBuil protected ITermConceptMapDao myTermConceptMapDao; @Autowired protected ITermConceptMapGroupElementTargetDao myTermConceptMapGroupElementTargetDao; + @SpyBean + protected MemoryCacheService myMemoryCacheService; @Autowired protected ICacheWarmingSvc myCacheWarmingSvc; @Autowired @@ -429,6 +433,7 @@ public void afterCleanupDao() { myStorageSettings.setReuseCachedSearchResultsForMillis(defaults.getReuseCachedSearchResultsForMillis()); myStorageSettings.setSuppressUpdatesWithNoChange(defaults.isSuppressUpdatesWithNoChange()); myStorageSettings.setAutoCreatePlaceholderReferenceTargets(defaults.isAutoCreatePlaceholderReferenceTargets()); + myStorageSettings.setMatchUrlCacheEnabled(defaults.isMatchUrlCacheEnabled()); myPagingProvider.setDefaultPageSize(BasePagingProvider.DEFAULT_DEFAULT_PAGE_SIZE); myPagingProvider.setMaximumPageSize(BasePagingProvider.DEFAULT_MAX_PAGE_SIZE); diff --git a/hapi-fhir-jpaserver-test-r5/src/test/java/ca/uhn/fhir/jpa/dao/r5/CrossPartitionReferencesTest.java b/hapi-fhir-jpaserver-test-r5/src/test/java/ca/uhn/fhir/jpa/dao/r5/CrossPartitionReferencesTest.java index 2719deb84a5..732fef35879 100644 --- a/hapi-fhir-jpaserver-test-r5/src/test/java/ca/uhn/fhir/jpa/dao/r5/CrossPartitionReferencesTest.java +++ b/hapi-fhir-jpaserver-test-r5/src/test/java/ca/uhn/fhir/jpa/dao/r5/CrossPartitionReferencesTest.java @@ -15,6 +15,7 @@ import ca.uhn.fhir.jpa.searchparam.extractor.CrossPartitionReferenceDetails; import ca.uhn.fhir.jpa.searchparam.extractor.IResourceLinkResolver; import ca.uhn.fhir.jpa.util.JpaHapiTransactionService; +import ca.uhn.fhir.jpa.util.MemoryCacheService; import ca.uhn.fhir.rest.api.Constants; import ca.uhn.fhir.rest.api.server.IBundleProvider; import ca.uhn.fhir.rest.param.TokenParam; @@ -29,9 +30,16 @@ import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import org.mockito.ArgumentCaptor; import org.mockito.Captor; import org.mockito.Mock; + +import static org.mockito.Mockito.never; + +import static org.mockito.Mockito.reset; + import org.springframework.beans.factory.annotation.Autowired; import org.springframework.transaction.annotation.Propagation; @@ -62,6 +70,8 @@ public class CrossPartitionReferencesTest extends BaseJpaR5Test { private ICrossPartitionReferenceDetectedHandler myCrossPartitionReferencesDetectedInterceptor; @Captor private ArgumentCaptor myCrossPartitionReferenceDetailsCaptor; + @Captor + private ArgumentCaptor myMatchUrlCacheValueCaptor; @Override @BeforeEach @@ -189,6 +199,74 @@ public void testCrossPartitionReference_Create() { assertEquals(patientId.getValue(), referenceDetails.getPathAndRef().getRef().getReferenceElement().getValue()); } + @ParameterizedTest + @ValueSource(booleans = {true, false}) + public void testCrossPartitionReference_CreateWithConditionalUrl(boolean theMatchUrlCacheEnabled) { + myStorageSettings.setMatchUrlCacheEnabled(theMatchUrlCacheEnabled); + + // Setup + Patient p = new Patient(); + p.setActive(true); + IIdType patientId = myPatientDao.create(p, mySrd).getId().toUnqualifiedVersionless(); + ourLog.info("Patient ID: {}", patientId); + runInTransaction(() -> { + ResourceTable resourceTable = myResourceTableDao.findById(patientId.getIdPartAsLong()).orElseThrow(); + assertEquals(1, resourceTable.getPartitionId().getPartitionId()); + }); + + initializeCrossReferencesInterceptor(); + + // Test + Observation o = new Observation(); + o.setStatus(Enumerations.ObservationStatus.FINAL); + String thePatientMatchUrl = "Patient/?_id=" + patientId.getIdPart(); + o.setSubject(new Reference(thePatientMatchUrl)); + myCaptureQueriesListener.clear(); + IIdType observationId = myObservationDao.create(o, mySrd).getId().toUnqualifiedVersionless(); + + // Verify + // 3 queries: Search to resolve PID from Match URL, search to resolve reference, create the resource + assertEquals(3, myCaptureQueriesListener.countCommits()); + assertEquals(0, myCaptureQueriesListener.countRollbacks()); + + runInTransaction(() -> { + ResourceTable resourceTable = myResourceTableDao.findById(observationId.getIdPartAsLong()).orElseThrow(); + assertEquals(2, resourceTable.getPartitionId().getPartitionId()); + }); + + verify(myCrossPartitionReferencesDetectedInterceptor, times(1)).handle(eq(Pointcut.JPA_RESOLVE_CROSS_PARTITION_REFERENCE), myCrossPartitionReferenceDetailsCaptor.capture()); + CrossPartitionReferenceDetails referenceDetails = myCrossPartitionReferenceDetailsCaptor.getValue(); + assertEquals(PARTITION_OBSERVATION, referenceDetails.getSourceResourcePartitionId()); + assertEquals(patientId.getValue(), referenceDetails.getPathAndRef().getRef().getReferenceElement().getValue()); + + if (theMatchUrlCacheEnabled) { + // Verify that the entry is put into cache + verify(myMemoryCacheService).putAfterCommit(eq(MemoryCacheService.CacheEnum.MATCH_URL), eq(thePatientMatchUrl), myMatchUrlCacheValueCaptor.capture()); + JpaPid actualCachedPid = myMatchUrlCacheValueCaptor.getValue(); + assertThat(actualCachedPid.getId()).isEqualTo(patientId.getIdPartAsLong()); + assertThat(actualCachedPid.getPartitionId()).isEqualTo(PARTITION_PATIENT.getFirstPartitionIdOrNull()); + } + + // Test again (the match URL should be in cache now) + myCaptureQueriesListener.clear(); + reset(myMemoryCacheService); + IIdType observationId2 = myObservationDao.create(o, mySrd).getId().toUnqualifiedVersionless(); + + // Verify + assertEquals(2, myCaptureQueriesListener.countCommits()); + assertEquals(0, myCaptureQueriesListener.countRollbacks()); + + runInTransaction(() -> { + ResourceTable resourceTable = myResourceTableDao.findById(observationId2.getIdPartAsLong()).orElseThrow(); + assertEquals(2, resourceTable.getPartitionId().getPartitionId()); + }); + + if (theMatchUrlCacheEnabled) { + verify(myMemoryCacheService, never()).putAfterCommit(eq(MemoryCacheService.CacheEnum.MATCH_URL), eq(thePatientMatchUrl), any()); + } + + } + private void initializeCrossReferencesInterceptor() { when(myCrossPartitionReferencesDetectedInterceptor.handle(any(), any())).thenAnswer(t -> { CrossPartitionReferenceDetails theDetails = t.getArgument(1, CrossPartitionReferenceDetails.class); diff --git a/hapi-fhir-jpaserver-test-utilities/src/main/java/ca/uhn/fhir/jpa/test/BaseJpaR4Test.java b/hapi-fhir-jpaserver-test-utilities/src/main/java/ca/uhn/fhir/jpa/test/BaseJpaR4Test.java index eaed24c66db..e7807eee971 100644 --- a/hapi-fhir-jpaserver-test-utilities/src/main/java/ca/uhn/fhir/jpa/test/BaseJpaR4Test.java +++ b/hapi-fhir-jpaserver-test-utilities/src/main/java/ca/uhn/fhir/jpa/test/BaseJpaR4Test.java @@ -203,6 +203,7 @@ import org.slf4j.event.Level; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Qualifier; +import org.springframework.boot.test.mock.mockito.SpyBean; import org.springframework.context.ApplicationContext; import org.springframework.data.domain.Pageable; import org.springframework.test.context.ContextConfiguration; @@ -527,7 +528,7 @@ public abstract class BaseJpaR4Test extends BaseJpaTest implements ITestDataBuil protected ITermConceptMapDao myTermConceptMapDao; @Autowired protected ITermConceptMapGroupElementTargetDao myTermConceptMapGroupElementTargetDao; - @Autowired + @SpyBean protected MemoryCacheService myMemoryCacheService; @Autowired protected ICacheWarmingSvc myCacheWarmingSvc; diff --git a/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/dao/BaseStorageResourceDao.java b/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/dao/BaseStorageResourceDao.java index 8eacb2d3820..6db537cdef5 100644 --- a/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/dao/BaseStorageResourceDao.java +++ b/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/dao/BaseStorageResourceDao.java @@ -20,12 +20,14 @@ package ca.uhn.fhir.jpa.dao; import ca.uhn.fhir.i18n.Msg; +import ca.uhn.fhir.interceptor.model.RequestPartitionId; import ca.uhn.fhir.jpa.api.dao.IFhirResourceDao; import ca.uhn.fhir.jpa.api.dao.IJpaDao; import ca.uhn.fhir.jpa.api.model.DaoMethodOutcome; import ca.uhn.fhir.jpa.api.model.DeleteMethodOutcome; import ca.uhn.fhir.jpa.dao.tx.HapiTransactionService; import ca.uhn.fhir.jpa.model.cross.IBasePersistedResource; +import ca.uhn.fhir.jpa.partition.IRequestPartitionHelperSvc; import ca.uhn.fhir.jpa.patch.FhirPatch; import ca.uhn.fhir.jpa.patch.JsonPatchUtils; import ca.uhn.fhir.jpa.patch.XmlPatchUtils; @@ -74,6 +76,14 @@ public abstract class BaseStorageResourceDao extends Ba @Autowired protected abstract IDeleteExpungeJobSubmitter getDeleteExpungeJobSubmitter(); + @Autowired + private IRequestPartitionHelperSvc myRequestPartitionHelperSvc; + + protected IRequestPartitionHelperSvc getRequestPartitionHelperService() { + // TODO JD change this to an abstract autowired on the next bump + return myRequestPartitionHelperSvc; + } + @Override public DaoMethodOutcome patch( IIdType theId, @@ -114,8 +124,16 @@ public DaoMethodOutcome patchInTransaction( IIdType resourceId; if (isNotBlank(theConditionalUrl)) { + RequestPartitionId theRequestPartitionId = getRequestPartitionHelperService() + .determineReadPartitionForRequestForSearchType(theRequestDetails, theId.getResourceType()); + Set match = getMatchResourceUrlService() - .processMatchUrl(theConditionalUrl, getResourceType(), theTransactionDetails, theRequestDetails); + .processMatchUrl( + theConditionalUrl, + getResourceType(), + theTransactionDetails, + theRequestDetails, + theRequestPartitionId); if (match.size() > 1) { String msg = getContext() .getLocalizer() diff --git a/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/dao/MatchResourceUrlService.java b/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/dao/MatchResourceUrlService.java index 916186dcf5d..017691412a3 100644 --- a/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/dao/MatchResourceUrlService.java +++ b/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/dao/MatchResourceUrlService.java @@ -25,6 +25,7 @@ import ca.uhn.fhir.interceptor.api.HookParams; import ca.uhn.fhir.interceptor.api.IInterceptorBroadcaster; import ca.uhn.fhir.interceptor.api.Pointcut; +import ca.uhn.fhir.interceptor.model.RequestPartitionId; import ca.uhn.fhir.jpa.api.config.JpaStorageSettings; import ca.uhn.fhir.jpa.api.dao.DaoRegistry; import ca.uhn.fhir.jpa.api.dao.IFhirResourceDao; @@ -88,8 +89,9 @@ public Set processMatchUrl( String theMatchUrl, Class theResourceType, TransactionDetails theTransactionDetails, - RequestDetails theRequest) { - return processMatchUrl(theMatchUrl, theResourceType, theTransactionDetails, theRequest, null); + RequestDetails theRequest, + RequestPartitionId thePartitionId) { + return processMatchUrl(theMatchUrl, theResourceType, theTransactionDetails, theRequest, null, thePartitionId); } /** @@ -100,7 +102,8 @@ public Set processMatchUrl( Class theResourceType, TransactionDetails theTransactionDetails, RequestDetails theRequest, - IBaseResource theConditionalOperationTargetOrNull) { + IBaseResource theConditionalOperationTargetOrNull, + RequestPartitionId thePartitionId) { Set retVal = null; String resourceType = myContext.getResourceType(theResourceType); @@ -117,7 +120,8 @@ public Set processMatchUrl( } } - T resolvedInCache = processMatchUrlUsingCacheOnly(resourceType, matchUrl); + T resolvedInCache = processMatchUrlUsingCacheOnly(resourceType, matchUrl, thePartitionId); + ourLog.debug("Resolving match URL from cache {} found: {}", theMatchUrl, resolvedInCache); if (resolvedInCache != null) { retVal = Collections.singleton(resolvedInCache); @@ -203,11 +207,18 @@ private String massageForStorage(String theResourceType, String theMatchUrl) { } @Nullable - public T processMatchUrlUsingCacheOnly(String theResourceType, String theMatchUrl) { + public T processMatchUrlUsingCacheOnly( + String theResourceType, String theMatchUrl, RequestPartitionId thePartitionId) { T existing = null; if (myStorageSettings.isMatchUrlCacheEnabled()) { String matchUrl = massageForStorage(theResourceType, theMatchUrl); - existing = myMemoryCacheService.getIfPresent(MemoryCacheService.CacheEnum.MATCH_URL, matchUrl); + T potentialMatch = myMemoryCacheService.getIfPresent(MemoryCacheService.CacheEnum.MATCH_URL, matchUrl); + if (potentialMatch != null + && (thePartitionId.isAllPartitions() + || (thePartitionId.hasPartitionIds() + && thePartitionId.hasPartitionId(potentialMatch.getPartitionId())))) { + existing = potentialMatch; + } } return existing; } diff --git a/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/searchparam/extractor/BaseSearchParamWithInlineReferencesExtractor.java b/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/searchparam/extractor/BaseSearchParamWithInlineReferencesExtractor.java index c383deaf1e7..08b198d3f00 100644 --- a/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/searchparam/extractor/BaseSearchParamWithInlineReferencesExtractor.java +++ b/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/searchparam/extractor/BaseSearchParamWithInlineReferencesExtractor.java @@ -22,12 +22,14 @@ import ca.uhn.fhir.context.FhirContext; import ca.uhn.fhir.context.RuntimeResourceDefinition; import ca.uhn.fhir.i18n.Msg; +import ca.uhn.fhir.interceptor.model.RequestPartitionId; import ca.uhn.fhir.jpa.api.config.JpaStorageSettings; import ca.uhn.fhir.jpa.api.svc.IIdHelperService; import ca.uhn.fhir.jpa.dao.BaseStorageDao; import ca.uhn.fhir.jpa.dao.MatchResourceUrlService; import ca.uhn.fhir.jpa.dao.index.DaoResourceLinkResolver; import ca.uhn.fhir.jpa.model.cross.IBasePersistedResource; +import ca.uhn.fhir.jpa.partition.IRequestPartitionHelperSvc; import ca.uhn.fhir.jpa.util.MemoryCacheService; import ca.uhn.fhir.rest.api.server.RequestDetails; import ca.uhn.fhir.rest.api.server.storage.IResourcePersistentId; @@ -67,6 +69,9 @@ public abstract class BaseSearchParamWithInlineReferencesExtractor myIdHelperService; + @Autowired + private IRequestPartitionHelperSvc myPartitionHelperSvc; + @Override public void extractInlineReferences( RequestDetails theRequestDetails, IBaseResource theResource, TransactionDetails theTransactionDetails) { @@ -116,8 +121,15 @@ public void extractInlineReferences( if (resolvedMatch != null && !IResourcePersistentId.NOT_FOUND.equals(resolvedMatch)) { matches = Set.of(resolvedMatch); } else { + RequestPartitionId theRequestPartitionId = + myPartitionHelperSvc.determineReadPartitionForRequestForSearchType( + theRequestDetails, resourceTypeString); matches = myMatchResourceUrlService.processMatchUrl( - nextIdText, matchResourceType, theTransactionDetails, theRequestDetails); + nextIdText, + matchResourceType, + theTransactionDetails, + theRequestDetails, + theRequestPartitionId); } T match; diff --git a/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/util/BundleBuilderR4Test.java b/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/util/BundleBuilderR4Test.java index 7cf91d2d03f..8b00fe279ef 100644 --- a/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/util/BundleBuilderR4Test.java +++ b/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/util/BundleBuilderR4Test.java @@ -105,6 +105,26 @@ public void testAddEntryUpdate() { assertEquals(Bundle.HTTPVerb.PUT, bundle.getEntry().get(0).getRequest().getMethod()); } + @Test + public void testAddEntryUpdate_withCustomRequestUrl() { + BundleBuilder builder = new BundleBuilder(myFhirContext); + + Patient patient = new Patient(); + patient.setId("http://foo/Patient/123"); + patient.setActive(true); + builder.addTransactionUpdateEntry(patient, "Patient?identifier=test|123"); + + Bundle bundle = (Bundle) builder.getBundle(); + ourLog.debug("Bundle:\n{}", myFhirContext.newJsonParser().setPrettyPrint(true).encodeResourceToString(bundle)); + + assertEquals(Bundle.BundleType.TRANSACTION, bundle.getType()); + assertThat(bundle.getEntry()).hasSize(1); + assertThat(bundle.getEntry().get(0).getResource()).isSameAs(patient); + assertEquals("http://foo/Patient/123", bundle.getEntry().get(0).getFullUrl()); + assertEquals("Patient?identifier=test|123", bundle.getEntry().get(0).getRequest().getUrl()); + assertEquals(Bundle.HTTPVerb.PUT, bundle.getEntry().get(0).getRequest().getMethod()); + } + @Test public void testNewPrimitive() { BundleBuilder builder = new BundleBuilder(myFhirContext); From b2b5cbc2675ac4032f02d27214d5d6136521579b Mon Sep 17 00:00:00 2001 From: jdar Date: Tue, 4 Mar 2025 11:36:53 -0800 Subject: [PATCH 3/6] fix tests and changelog --- .../8_2_0/6767-fix-match-url-cache-and-partitions.yaml | 7 +++++++ .../uhn/fhir/jpa/provider/r4/MultitenantServerR4Test.java | 3 +++ .../test/java/ca/uhn/fhir/jpa/dao/r5/BaseJpaR5Test.java | 2 -- .../uhn/fhir/jpa/dao/r5/CrossPartitionReferencesTest.java | 3 +++ .../src/main/java/ca/uhn/fhir/jpa/test/BaseJpaR4Test.java | 2 +- .../java/ca/uhn/fhir/jpa/dao/BaseStorageResourceDao.java | 2 +- 6 files changed, 15 insertions(+), 4 deletions(-) create mode 100644 hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/8_2_0/6767-fix-match-url-cache-and-partitions.yaml diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/8_2_0/6767-fix-match-url-cache-and-partitions.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/8_2_0/6767-fix-match-url-cache-and-partitions.yaml new file mode 100644 index 00000000000..ac17295190f --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/8_2_0/6767-fix-match-url-cache-and-partitions.yaml @@ -0,0 +1,7 @@ +--- +type: fix +issue: 6767 +jira: SMILE-7942 +title: "Previously, when both Match URL cache and Partitioning mode were enabled, the Match URL cache would sometimes +return the incorrect resource while resolving a conditional URL that matches to a resource that exists in multiple +partitions. This has now been fixed." diff --git a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/provider/r4/MultitenantServerR4Test.java b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/provider/r4/MultitenantServerR4Test.java index a5bcbb96942..538eb778818 100644 --- a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/provider/r4/MultitenantServerR4Test.java +++ b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/provider/r4/MultitenantServerR4Test.java @@ -83,6 +83,7 @@ import static org.mockito.Mockito.reset; import static org.mockito.Mockito.verify; +import org.springframework.boot.test.mock.mockito.SpyBean; import org.springframework.mock.web.MockHttpServletRequest; import java.io.IOException; @@ -112,6 +113,8 @@ public class MultitenantServerR4Test extends BaseMultitenantResourceProviderR4Test implements ITestDataBuilder { @Captor private ArgumentCaptor myMatchUrlCacheValueCaptor; + @SpyBean + private MemoryCacheService myMemoryCacheService; @Override @AfterEach diff --git a/hapi-fhir-jpaserver-test-r5/src/test/java/ca/uhn/fhir/jpa/dao/r5/BaseJpaR5Test.java b/hapi-fhir-jpaserver-test-r5/src/test/java/ca/uhn/fhir/jpa/dao/r5/BaseJpaR5Test.java index cf0bc6826c0..694dc6b57e0 100644 --- a/hapi-fhir-jpaserver-test-r5/src/test/java/ca/uhn/fhir/jpa/dao/r5/BaseJpaR5Test.java +++ b/hapi-fhir-jpaserver-test-r5/src/test/java/ca/uhn/fhir/jpa/dao/r5/BaseJpaR5Test.java @@ -385,8 +385,6 @@ public abstract class BaseJpaR5Test extends BaseJpaTest implements ITestDataBuil protected ITermConceptMapDao myTermConceptMapDao; @Autowired protected ITermConceptMapGroupElementTargetDao myTermConceptMapGroupElementTargetDao; - @SpyBean - protected MemoryCacheService myMemoryCacheService; @Autowired protected ICacheWarmingSvc myCacheWarmingSvc; @Autowired diff --git a/hapi-fhir-jpaserver-test-r5/src/test/java/ca/uhn/fhir/jpa/dao/r5/CrossPartitionReferencesTest.java b/hapi-fhir-jpaserver-test-r5/src/test/java/ca/uhn/fhir/jpa/dao/r5/CrossPartitionReferencesTest.java index 732fef35879..92c454c7fdf 100644 --- a/hapi-fhir-jpaserver-test-r5/src/test/java/ca/uhn/fhir/jpa/dao/r5/CrossPartitionReferencesTest.java +++ b/hapi-fhir-jpaserver-test-r5/src/test/java/ca/uhn/fhir/jpa/dao/r5/CrossPartitionReferencesTest.java @@ -41,6 +41,7 @@ import static org.mockito.Mockito.reset; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.mock.mockito.SpyBean; import org.springframework.transaction.annotation.Propagation; import java.util.List; @@ -66,6 +67,8 @@ public class CrossPartitionReferencesTest extends BaseJpaR5Test { private IHapiTransactionService myTransactionService; @Autowired private IResourceLinkResolver myResourceLinkResolver; + @SpyBean + protected MemoryCacheService myMemoryCacheService; @Mock private ICrossPartitionReferenceDetectedHandler myCrossPartitionReferencesDetectedInterceptor; @Captor diff --git a/hapi-fhir-jpaserver-test-utilities/src/main/java/ca/uhn/fhir/jpa/test/BaseJpaR4Test.java b/hapi-fhir-jpaserver-test-utilities/src/main/java/ca/uhn/fhir/jpa/test/BaseJpaR4Test.java index e7807eee971..82e75505355 100644 --- a/hapi-fhir-jpaserver-test-utilities/src/main/java/ca/uhn/fhir/jpa/test/BaseJpaR4Test.java +++ b/hapi-fhir-jpaserver-test-utilities/src/main/java/ca/uhn/fhir/jpa/test/BaseJpaR4Test.java @@ -528,7 +528,7 @@ public abstract class BaseJpaR4Test extends BaseJpaTest implements ITestDataBuil protected ITermConceptMapDao myTermConceptMapDao; @Autowired protected ITermConceptMapGroupElementTargetDao myTermConceptMapGroupElementTargetDao; - @SpyBean + @Autowired protected MemoryCacheService myMemoryCacheService; @Autowired protected ICacheWarmingSvc myCacheWarmingSvc; diff --git a/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/dao/BaseStorageResourceDao.java b/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/dao/BaseStorageResourceDao.java index 6db537cdef5..5a5ea7039a5 100644 --- a/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/dao/BaseStorageResourceDao.java +++ b/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/dao/BaseStorageResourceDao.java @@ -125,7 +125,7 @@ public DaoMethodOutcome patchInTransaction( if (isNotBlank(theConditionalUrl)) { RequestPartitionId theRequestPartitionId = getRequestPartitionHelperService() - .determineReadPartitionForRequestForSearchType(theRequestDetails, theId.getResourceType()); + .determineReadPartitionForRequestForSearchType(theRequestDetails, getResourceType().getTypeName()); Set match = getMatchResourceUrlService() .processMatchUrl( From 7492ca86befcde53ee9e7ddfc86d725c7bc0627c Mon Sep 17 00:00:00 2001 From: jdar Date: Tue, 4 Mar 2025 11:39:20 -0800 Subject: [PATCH 4/6] fix tests and changelog --- .../main/java/ca/uhn/fhir/jpa/dao/BaseStorageResourceDao.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/dao/BaseStorageResourceDao.java b/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/dao/BaseStorageResourceDao.java index 5a5ea7039a5..7938e0685cb 100644 --- a/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/dao/BaseStorageResourceDao.java +++ b/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/dao/BaseStorageResourceDao.java @@ -125,7 +125,8 @@ public DaoMethodOutcome patchInTransaction( if (isNotBlank(theConditionalUrl)) { RequestPartitionId theRequestPartitionId = getRequestPartitionHelperService() - .determineReadPartitionForRequestForSearchType(theRequestDetails, getResourceType().getTypeName()); + .determineReadPartitionForRequestForSearchType( + theRequestDetails, getResourceType().getTypeName()); Set match = getMatchResourceUrlService() .processMatchUrl( From db0e492c3ba830a2bb0cf1fa370f3e29a1757ebd Mon Sep 17 00:00:00 2001 From: jdar Date: Wed, 5 Mar 2025 14:14:16 -0800 Subject: [PATCH 5/6] address code review comments --- .../jpa/batch2/BulkDataErrorAbuseTest.java | 35 +- .../bulk/imprt2/ConsumeFilesStepR4Test.java | 6 +- .../jpa/dao/r4/BasePartitioningR4Test.java | 75 ++-- ...itioningNonNullDefaultPartitionR4Test.java | 40 +- .../dao/r4/PartitioningSearchCacheR4Test.java | 23 +- .../jpa/dao/r4/PartitioningSqlR4Test.java | 372 +++++++++--------- .../provider/r4/MultitenantServerR4Test.java | 44 ++- .../fhir/jpa/util/ConcurrencyTestUtil.java | 50 +++ 8 files changed, 334 insertions(+), 311 deletions(-) create mode 100644 hapi-fhir-jpaserver-test-utilities/src/main/java/ca/uhn/fhir/jpa/util/ConcurrencyTestUtil.java diff --git a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/batch2/BulkDataErrorAbuseTest.java b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/batch2/BulkDataErrorAbuseTest.java index ae6ac62d086..4111032ecf8 100644 --- a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/batch2/BulkDataErrorAbuseTest.java +++ b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/batch2/BulkDataErrorAbuseTest.java @@ -1,7 +1,7 @@ package ca.uhn.fhir.jpa.batch2; +import static ca.uhn.fhir.jpa.util.ConcurrencyTestUtil.executeFutures; import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.assertNotNull; import ca.uhn.fhir.batch2.api.IJobCoordinator; import ca.uhn.fhir.batch2.model.JobInstance; @@ -78,7 +78,7 @@ void afterEach() { } @Test - public void testGroupBulkExportNotInGroup_DoesNotShowUp() throws InterruptedException, ExecutionException { + public void testGroupBulkExportNotInGroup_DoesNotShowUp() { duAbuseTest(100); } @@ -93,7 +93,7 @@ public void testGroupBulkExportNotInGroup_DoesNotShowUp() throws InterruptedExce */ @Test @Disabled("for manual debugging") - public void testNonStopAbuseBatch2BulkExportStressTest() throws InterruptedException, ExecutionException { + public void testNonStopAbuseBatch2BulkExportStressTest() { duAbuseTest(Integer.MAX_VALUE); } @@ -190,35 +190,6 @@ private void duAbuseTest(int taskExecutions) { ourLog.info("Finished task execution"); } - private void executeFutures(CompletionService theCompletionService, int theTotal) { - List errors = new ArrayList<>(); - int count = 0; - - while (count + errors.size() < theTotal) { - try { - Future future = theCompletionService.take(); - boolean r = future.get(); - assertTrue(r); - count++; - } catch (Exception ex) { - // we will run all the threads to completion, even if we have errors; - // this is so we don't have background threads kicking around with - // partial changes. - // we either do this, or shutdown the completion service in an - // "inelegant" manner, dropping all threads (which we aren't doing) - ourLog.error("Failed after checking " + count + " futures"); - String[] frames = ExceptionUtils.getRootCauseStackTrace(ex); - errors.add(ex + "\n" + String.join("\n ", frames)); - } - } - - if (!errors.isEmpty()) { - fail(String.format("Failed to execute futures. Found %d errors :\n", errors.size()) - + String.join(", ", errors)); - } - } - - private void verifyBulkExportResults(String theInstanceId, List theContainedList, List theExcludedList) { // Iterate over the files JobInstance jobInfo = myJobCoordinator.getInstance(theInstanceId); diff --git a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/bulk/imprt2/ConsumeFilesStepR4Test.java b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/bulk/imprt2/ConsumeFilesStepR4Test.java index f7bf2d1cdc7..cc321135460 100644 --- a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/bulk/imprt2/ConsumeFilesStepR4Test.java +++ b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/bulk/imprt2/ConsumeFilesStepR4Test.java @@ -102,7 +102,7 @@ public void testAlreadyExisting_WithChanges(boolean partitionEnabled) { if (partitionEnabled) { myPartitionSettings.setPartitioningEnabled(true); myPartitionSettings.setIncludePartitionInSearchHashes(true); - addCreatePartitionNTimes(1, 2); + addNextTargetPartitionNTimesForCreate(1, 2); } Patient patient = new Patient(); patient.setId("A"); @@ -131,8 +131,8 @@ public void testAlreadyExisting_WithChanges(boolean partitionEnabled) { myMemoryCacheService.invalidateAllCaches(); myCaptureQueriesListener.clear(); if (partitionEnabled) { - addReadPartition(1); - addReadPartition(1); + addNextTargetPartitionsForRead(1); + addNextTargetPartitionsForRead(1); mySvc.storeResources(resources, myRequestPartitionId); } else { mySvc.storeResources(resources, null); diff --git a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/BasePartitioningR4Test.java b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/BasePartitioningR4Test.java index 7a547a758c1..4b167b384bf 100644 --- a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/BasePartitioningR4Test.java +++ b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/BasePartitioningR4Test.java @@ -115,7 +115,7 @@ public void before() throws Exception { myStorageSettings.setIndexMissingFields(JpaStorageSettings.IndexEnabledEnum.ENABLED); // Ensure the partition names are resolved - myPartitionInterceptor.addReadPartition(RequestPartitionId.fromPartitionNames(JpaConstants.DEFAULT_PARTITION_NAME, PARTITION_1, PARTITION_2, PARTITION_3, PARTITION_4)); + myPartitionInterceptor.addNextTargetReadPartition(RequestPartitionId.fromPartitionNames(JpaConstants.DEFAULT_PARTITION_NAME, PARTITION_1, PARTITION_2, PARTITION_3, PARTITION_4)); myPatientDao.search(new SearchParameterMap().setLoadSynchronous(true), mySrd); // Pre-fetch the partitions by ID @@ -147,8 +147,8 @@ public void afterPurgeDatabase() { } protected void createUniqueComboSp() { - addCreateDefaultPartition(); - addReadDefaultPartition(); // one for search param validation + addNextTargetPartitionForCreateDefaultPartition(); + addNextTargetPartitionForReadDefaultPartition(); // one for search param validation SearchParameter sp = new SearchParameter(); sp.setId("SearchParameter/patient-gender"); sp.setType(Enumerations.SearchParamType.TOKEN); @@ -158,8 +158,8 @@ protected void createUniqueComboSp() { sp.addBase("Patient"); mySearchParameterDao.update(sp, mySrd); - addCreateDefaultPartition(); - addReadDefaultPartition(); // one for search param validation + addNextTargetPartitionForCreateDefaultPartition(); + addNextTargetPartitionForReadDefaultPartition(); // one for search param validation sp = new SearchParameter(); sp.setId("SearchParameter/patient-family"); sp.setType(Enumerations.SearchParamType.STRING); @@ -169,7 +169,7 @@ protected void createUniqueComboSp() { sp.addBase("Patient"); mySearchParameterDao.update(sp, mySrd); - addCreateDefaultPartition(); + addNextTargetPartitionForCreateDefaultPartition(); sp = new SearchParameter(); sp.setId("SearchParameter/patient-gender-family-unique"); sp.setType(Enumerations.SearchParamType.COMPOSITE); @@ -192,8 +192,8 @@ protected void createUniqueComboSp() { } protected void createNonUniqueComboSp() { - addCreateDefaultPartition(); - addReadDefaultPartition(); // one for search param validation + addNextTargetPartitionForCreateDefaultPartition(); + addNextTargetPartitionForReadDefaultPartition(); // one for search param validation SearchParameter sp = new SearchParameter(); sp.setId("SearchParameter/patient-family"); sp.setType(Enumerations.SearchParamType.STRING); @@ -203,8 +203,8 @@ protected void createNonUniqueComboSp() { sp.addBase("Patient"); mySearchParameterDao.update(sp, mySrd); - addCreateDefaultPartition(); - addReadDefaultPartition(); // one for search param validation + addNextTargetPartitionForCreateDefaultPartition(); + addNextTargetPartitionForReadDefaultPartition(); // one for search param validation sp = new SearchParameter(); sp.setId("SearchParameter/patient-managingorg"); sp.setType(Enumerations.SearchParamType.REFERENCE); @@ -214,7 +214,7 @@ protected void createNonUniqueComboSp() { sp.addBase("Patient"); mySearchParameterDao.update(sp, mySrd); - addCreateDefaultPartition(); + addNextTargetPartitionForCreateDefaultPartition(); sp = new SearchParameter(); sp.setId("SearchParameter/patient-family-and-org"); sp.setType(Enumerations.SearchParamType.COMPOSITE); @@ -241,48 +241,48 @@ protected void dropForcedIdUniqueConstraint() { myHaveDroppedForcedIdUniqueConstraint = true; } - protected void addCreatePartitionNTimes(Integer thePartitionId, Integer theNumberOfTimes) { + protected void addNextTargetPartitionNTimesForCreate(Integer thePartitionId, Integer theNumberOfTimes) { for (int i = 0; i < theNumberOfTimes; i++) { - addCreatePartition(thePartitionId, null); + addNextTargetPartitionForCreate(thePartitionId, null); } } - protected void addCreatePartition(Integer thePartitionId) { - addCreatePartition(thePartitionId, null); + protected void addNextTargetPartitionForCreate(Integer thePartitionId) { + addNextTargetPartitionForCreate(thePartitionId, null); } - protected void addCreatePartition(Integer thePartitionId, LocalDate thePartitionDate) { + protected void addNextTargetPartitionForCreate(Integer thePartitionId, LocalDate thePartitionDate) { Validate.notNull(thePartitionId); RequestPartitionId requestPartitionId = RequestPartitionId.fromPartitionId(thePartitionId, thePartitionDate); - myPartitionInterceptor.addCreatePartition(requestPartitionId); + myPartitionInterceptor.addNextTargetPartitionForCreate(requestPartitionId); } - protected void addCreateDefaultPartition() { - myPartitionInterceptor.addCreatePartition(RequestPartitionId.defaultPartition()); + protected void addNextTargetPartitionForCreateDefaultPartition() { + myPartitionInterceptor.addNextTargetPartitionForCreate(RequestPartitionId.defaultPartition()); } - protected void addCreateDefaultPartition(LocalDate thePartitionDate) { + protected void addNextTargetPartitionForCreateDefaultPartition(LocalDate thePartitionDate) { RequestPartitionId requestPartitionId = RequestPartitionId.fromPartitionId(null, thePartitionDate); - myPartitionInterceptor.addCreatePartition(requestPartitionId); + myPartitionInterceptor.addNextTargetPartitionForCreate(requestPartitionId); } - protected void addReadPartition(Integer... thePartitionId) { + protected void addNextTargetPartitionsForRead(Integer... thePartitionId) { Validate.notNull(thePartitionId); - myPartitionInterceptor.addReadPartition(RequestPartitionId.fromPartitionIds(thePartitionId)); + myPartitionInterceptor.addNextTargetReadPartition(RequestPartitionId.fromPartitionIds(thePartitionId)); } - protected void addReadPartitions(String... thePartitionNames) { + protected void addNextTargetPartitionsForRead(String... thePartitionNames) { Validate.notNull(thePartitionNames); Validate.isTrue(thePartitionNames.length > 0); - myPartitionInterceptor.addReadPartition(RequestPartitionId.fromPartitionNames(thePartitionNames)); + myPartitionInterceptor.addNextTargetReadPartition(RequestPartitionId.fromPartitionNames(thePartitionNames)); } - protected void addReadDefaultPartition() { - myPartitionInterceptor.addReadPartition(RequestPartitionId.defaultPartition()); + protected void addNextTargetPartitionForReadDefaultPartition() { + myPartitionInterceptor.addNextTargetReadPartition(RequestPartitionId.defaultPartition()); } - protected void addReadAllPartitions() { - myPartitionInterceptor.addReadPartition(RequestPartitionId.allPartitions()); + protected void addNextTargetPartitionForReadAllPartitions() { + myPartitionInterceptor.addNextTargetReadPartition(RequestPartitionId.allPartitions()); } public void createRequestId() { @@ -292,9 +292,9 @@ public void createRequestId() { protected ICreationArgument withPartition(Integer thePartitionId) { return t -> { if (thePartitionId != null) { - addCreatePartition(thePartitionId, null); + addNextTargetPartitionForCreate(thePartitionId, null); } else { - addCreateDefaultPartition(); + addNextTargetPartitionForCreateDefaultPartition(); } }; } @@ -302,11 +302,11 @@ protected ICreationArgument withPartition(Integer thePartitionId) { protected ICreationArgument withReadWritePartitions(Integer thePartitionId) { return t -> { if (thePartitionId != null) { - addReadPartition(thePartitionId); - addCreatePartition(thePartitionId, null); + addNextTargetPartitionsForRead(thePartitionId); + addNextTargetPartitionForCreate(thePartitionId, null); } else { - addReadDefaultPartition(); - addCreateDefaultPartition(); + addNextTargetPartitionForReadDefaultPartition(); + addNextTargetPartitionForCreateDefaultPartition(); } }; } @@ -317,7 +317,7 @@ public static class MyReadWriteInterceptor extends MyWriteInterceptor { private final List myReadRequestPartitionIds = new ArrayList<>(); - public void addReadPartition(RequestPartitionId theRequestPartitionId) { + public void addNextTargetReadPartition(RequestPartitionId theRequestPartitionId) { myReadRequestPartitionIds.add(theRequestPartitionId); ourLog.info("Adding partition {} for read (not have {})", theRequestPartitionId, myReadRequestPartitionIds.size()); } @@ -358,10 +358,9 @@ public void assertNoRemainingIds() { @Interceptor public static class MyWriteInterceptor { - private final List myCreateRequestPartitionIds = new ArrayList<>(); - public void addCreatePartition(RequestPartitionId theRequestPartitionId) { + public void addNextTargetPartitionForCreate(RequestPartitionId theRequestPartitionId) { myCreateRequestPartitionIds.add(theRequestPartitionId); } diff --git a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/PartitioningNonNullDefaultPartitionR4Test.java b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/PartitioningNonNullDefaultPartitionR4Test.java index 8aec4b349fb..973d213039c 100644 --- a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/PartitioningNonNullDefaultPartitionR4Test.java +++ b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/PartitioningNonNullDefaultPartitionR4Test.java @@ -49,10 +49,10 @@ public void after() { @Test public void testCreateAndSearch_NonPartitionable() { - addCreateDefaultPartition(); + addNextTargetPartitionForCreateDefaultPartition(); // we need two read partition accesses for when the creation of the SP triggers a reindex of Patient - addReadDefaultPartition(); // one for search param validation - addReadDefaultPartition(); // and one for the reindex job + addNextTargetPartitionForReadDefaultPartition(); // one for search param validation + addNextTargetPartitionForReadDefaultPartition(); // and one for the reindex job SearchParameter sp = new SearchParameter(); sp.addBase("Patient"); sp.setStatus(Enumerations.PublicationStatus.ACTIVE); @@ -68,12 +68,12 @@ public void testCreateAndSearch_NonPartitionable() { }); // Search on Token - addReadDefaultPartition(); + addNextTargetPartitionForReadDefaultPartition(); List outcome = toUnqualifiedVersionlessIdValues(mySearchParameterDao.search(SearchParameterMap.newSynchronous().add("code", new TokenParam("extpatorg")), mySrd)); assertThat(outcome).containsExactly("SearchParameter/" + id); // Search on All Resources - addReadDefaultPartition(); + addNextTargetPartitionForReadDefaultPartition(); outcome = toUnqualifiedVersionlessIdValues(mySearchParameterDao.search(SearchParameterMap.newSynchronous(), mySrd)); assertThat(outcome).containsExactly("SearchParameter/" + id); @@ -81,10 +81,10 @@ public void testCreateAndSearch_NonPartitionable() { @Test public void testCreateAndSearch_NonPartitionable_ForcedId() { - addCreateDefaultPartition(); + addNextTargetPartitionForCreateDefaultPartition(); // we need two read partition accesses for when the creation of the SP triggers a reindex of Patient - addReadDefaultPartition(); // one for search param validation - addReadDefaultPartition(); // and one for the reindex job + addNextTargetPartitionForReadDefaultPartition(); // one for search param validation + addNextTargetPartitionForReadDefaultPartition(); // and one for the reindex job SearchParameter sp = new SearchParameter(); sp.setId("SearchParameter/A"); sp.addBase("Patient"); @@ -101,12 +101,12 @@ public void testCreateAndSearch_NonPartitionable_ForcedId() { }); // Search on Token - addReadDefaultPartition(); + addNextTargetPartitionForReadDefaultPartition(); List outcome = toUnqualifiedVersionlessIdValues(mySearchParameterDao.search(SearchParameterMap.newSynchronous().add("code", new TokenParam("extpatorg")), mySrd)); assertThat(outcome).containsExactly("SearchParameter/A"); // Search on All Resources - addReadDefaultPartition(); + addNextTargetPartitionForReadDefaultPartition(); outcome = toUnqualifiedVersionlessIdValues(mySearchParameterDao.search(SearchParameterMap.newSynchronous(), mySrd)); assertThat(outcome).containsExactly("SearchParameter/A"); @@ -114,7 +114,7 @@ public void testCreateAndSearch_NonPartitionable_ForcedId() { @Test public void testCreateAndSearch_Partitionable_ForcedId() { - addCreateDefaultPartition(); + addNextTargetPartitionForCreateDefaultPartition(); Patient patient = new Patient(); patient.setId("A"); patient.addIdentifier().setSystem("http://foo").setValue("123"); @@ -127,12 +127,12 @@ public void testCreateAndSearch_Partitionable_ForcedId() { }); // Search on Token - addReadDefaultPartition(); + addNextTargetPartitionForReadDefaultPartition(); List outcome = toUnqualifiedVersionlessIdValues(myPatientDao.search(SearchParameterMap.newSynchronous().add("identifier", new TokenParam("http://foo", "123")), mySrd)); assertThat(outcome).containsExactly("Patient/A"); // Search on All Resources - addReadDefaultPartition(); + addNextTargetPartitionForReadDefaultPartition(); outcome = toUnqualifiedVersionlessIdValues(myPatientDao.search(SearchParameterMap.newSynchronous(), mySrd)); assertThat(outcome).containsExactly("Patient/A"); @@ -141,7 +141,7 @@ public void testCreateAndSearch_Partitionable_ForcedId() { @Test public void testCreateAndSearch_Partitionable() { - addCreateDefaultPartition(); + addNextTargetPartitionForCreateDefaultPartition(); Patient patient = new Patient(); patient.getMeta().addTag().setSystem("http://foo").setCode("TAG"); patient.addIdentifier().setSystem("http://foo").setValue("123"); @@ -156,17 +156,17 @@ public void testCreateAndSearch_Partitionable() { }); // Search on Token - addReadDefaultPartition(); + addNextTargetPartitionForReadDefaultPartition(); List outcome = toUnqualifiedVersionlessIdValues(myPatientDao.search(SearchParameterMap.newSynchronous().add("identifier", new TokenParam("http://foo", "123")), mySrd)); assertThat(outcome).containsExactly("Patient/" + id); // Search on Tag - addReadDefaultPartition(); + addNextTargetPartitionForReadDefaultPartition(); outcome = toUnqualifiedVersionlessIdValues(myPatientDao.search(SearchParameterMap.newSynchronous().add("_tag", new TokenParam("http://foo", "TAG")), mySrd)); assertThat(outcome).containsExactly("Patient/" + id); // Search on All Resources - addReadDefaultPartition(); + addNextTargetPartitionForReadDefaultPartition(); outcome = toUnqualifiedVersionlessIdValues(myPatientDao.search(SearchParameterMap.newSynchronous(), mySrd)); assertThat(outcome).containsExactly("Patient/" + id); @@ -176,19 +176,19 @@ public void testCreateAndSearch_Partitionable() { @Test public void testRead_Partitionable() { - addCreateDefaultPartition(); + addNextTargetPartitionForCreateDefaultPartition(); Patient patient = new Patient(); patient.getMeta().addTag().setSystem("http://foo").setCode("TAG"); patient.addIdentifier().setSystem("http://foo").setValue("123"); patient.setActive(true); Long id = myPatientDao.create(patient, mySrd).getId().getIdPartAsLong(); - addReadDefaultPartition(); + addNextTargetPartitionForReadDefaultPartition(); patient = myPatientDao.read(new IdType("Patient/" + id), mySrd); assertTrue(patient.getActive()); // Wrong partition - addReadPartition(2); + addNextTargetPartitionsForRead(2); try { myPatientDao.read(new IdType("Patient/" + id), mySrd); fail(); diff --git a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/PartitioningSearchCacheR4Test.java b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/PartitioningSearchCacheR4Test.java index 5668cabb0c1..f962581df2f 100644 --- a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/PartitioningSearchCacheR4Test.java +++ b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/PartitioningSearchCacheR4Test.java @@ -13,25 +13,16 @@ import static org.junit.jupiter.api.Assertions.assertEquals; -import ca.uhn.fhir.jpa.api.dao.PatientEverythingParameters; -import ca.uhn.fhir.jpa.binary.provider.BinaryAccessProvider; -import ca.uhn.fhir.jpa.rp.r4.PatientResourceProvider; import ca.uhn.fhir.jpa.search.PersistedJpaBundleProvider; import ca.uhn.fhir.jpa.search.cache.SearchCacheStatusEnum; import ca.uhn.fhir.jpa.searchparam.SearchParameterMap; import ca.uhn.fhir.jpa.util.SqlQuery; -import ca.uhn.fhir.rest.api.server.IBundleProvider; -import ca.uhn.fhir.rest.api.server.SystemRequestDetails; import org.apache.commons.lang3.StringUtils; -import org.hl7.fhir.instance.model.api.IBaseResource; import org.hl7.fhir.instance.model.api.IIdType; -import org.hl7.fhir.r4.model.IntegerType; import org.junit.jupiter.api.Test; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import org.springframework.beans.factory.annotation.Autowired; -import java.util.ArrayList; import java.util.List; import java.util.stream.Collectors; @@ -53,7 +44,7 @@ public void testSearch_OnePartition_UseCache() { { myCaptureQueriesListener.clear(); - addReadPartition(1); + addNextTargetPartitionsForRead(1); PersistedJpaBundleProvider outcome = (PersistedJpaBundleProvider) myPatientDao.search(new SearchParameterMap(), mySrd); assertEquals(SearchCacheStatusEnum.MISS, outcome.getCacheStatus()); assertEquals(2, outcome.sizeOrThrowNpe(), ()-> "Resources:\n * " + runInTransaction(()->myResourceTableDao.findAll().stream().map(t->t.toString()).collect(Collectors.joining("\n * "))) + @@ -72,7 +63,7 @@ public void testSearch_OnePartition_UseCache() { // Try from a different partition { myCaptureQueriesListener.clear(); - addReadPartition(2); + addNextTargetPartitionsForRead(2); PersistedJpaBundleProvider outcome = (PersistedJpaBundleProvider) myPatientDao.search(new SearchParameterMap(), mySrd); assertEquals(SearchCacheStatusEnum.MISS, outcome.getCacheStatus()); assertEquals(2, outcome.sizeOrThrowNpe()); @@ -89,7 +80,7 @@ public void testSearch_OnePartition_UseCache() { // Try from the first partition, should be a cache hit this time { myCaptureQueriesListener.clear(); - addReadPartition(2); + addNextTargetPartitionsForRead(2); PersistedJpaBundleProvider outcome = (PersistedJpaBundleProvider) myPatientDao.search(new SearchParameterMap(), mySrd); assertEquals(SearchCacheStatusEnum.HIT, outcome.getCacheStatus()); assertEquals(2, outcome.sizeOrThrowNpe()); @@ -116,7 +107,7 @@ public void testSearch_MultiplePartitions_UseCache() { { myCaptureQueriesListener.clear(); - addReadPartition(1, null); + addNextTargetPartitionsForRead(1, null); PersistedJpaBundleProvider outcome = (PersistedJpaBundleProvider) myPatientDao.search(new SearchParameterMap(), mySrd); assertEquals(SearchCacheStatusEnum.MISS, outcome.getCacheStatus()); assertEquals(4, outcome.sizeOrThrowNpe()); @@ -133,7 +124,7 @@ public void testSearch_MultiplePartitions_UseCache() { // Try from a different partition { myCaptureQueriesListener.clear(); - addReadPartition(2, 1); + addNextTargetPartitionsForRead(2, 1); PersistedJpaBundleProvider outcome = (PersistedJpaBundleProvider) myPatientDao.search(new SearchParameterMap(), mySrd); assertEquals(SearchCacheStatusEnum.MISS, outcome.getCacheStatus()); assertEquals(4, outcome.sizeOrThrowNpe()); @@ -150,7 +141,7 @@ public void testSearch_MultiplePartitions_UseCache() { // Try from the first partition, should be a cache hit this time { myCaptureQueriesListener.clear(); - addReadPartition(1, null); + addNextTargetPartitionsForRead(1, null); PersistedJpaBundleProvider outcome = (PersistedJpaBundleProvider) myPatientDao.search(new SearchParameterMap(), mySrd); assertEquals(SearchCacheStatusEnum.HIT, outcome.getCacheStatus()); assertEquals(4, outcome.sizeOrThrowNpe()); @@ -186,7 +177,7 @@ private Bundle createPatientWithConditionalUrlOnPartition(Integer thePartitionId p.setActive(true); p.setName(List.of(new HumanName().setFamily("ABC").setGiven(List.of(new StringType("DEF"))))); bb.addTransactionUpdateEntry(p, "Patient?identifier=foo|bar"); - addCreatePartitionNTimes(thePartitionId, 3); + addNextTargetPartitionNTimesForCreate(thePartitionId, 3); return (Bundle) bb.getBundle(); } diff --git a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/PartitioningSqlR4Test.java b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/PartitioningSqlR4Test.java index 41822721c4a..b2d12f7e182 100644 --- a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/PartitioningSqlR4Test.java +++ b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/PartitioningSqlR4Test.java @@ -14,10 +14,8 @@ import ca.uhn.fhir.interceptor.model.RequestPartitionId; import ca.uhn.fhir.jpa.api.config.JpaStorageSettings; import ca.uhn.fhir.jpa.batch.models.Batch2JobStartResponse; -import ca.uhn.fhir.jpa.dao.BaseHapiFhirDao; import ca.uhn.fhir.jpa.model.config.PartitionSettings; import ca.uhn.fhir.jpa.model.dao.JpaPid; -import ca.uhn.fhir.jpa.model.dao.JpaPidFk; import ca.uhn.fhir.jpa.model.entity.EntityIndexStatusEnum; import ca.uhn.fhir.jpa.model.entity.PartitionablePartitionId; import ca.uhn.fhir.jpa.model.entity.ResourceHistoryTable; @@ -32,7 +30,6 @@ import ca.uhn.fhir.jpa.model.entity.SearchParamPresentEntity; import ca.uhn.fhir.jpa.model.util.JpaConstants; import ca.uhn.fhir.jpa.searchparam.SearchParameterMap; -import ca.uhn.fhir.jpa.searchparam.submit.interceptor.SearchParamValidatingInterceptor; import ca.uhn.fhir.jpa.util.SqlQuery; import ca.uhn.fhir.rest.api.Constants; import ca.uhn.fhir.rest.api.server.IBundleProvider; @@ -60,7 +57,6 @@ import org.hl7.fhir.instance.model.api.IIdType; import org.hl7.fhir.r4.model.Bundle; import org.hl7.fhir.r4.model.CodeSystem; -import org.hl7.fhir.r4.model.ConceptMap; import org.hl7.fhir.r4.model.DateTimeType; import org.hl7.fhir.r4.model.Enumerations; import org.hl7.fhir.r4.model.IdType; @@ -137,8 +133,8 @@ public void afterEach() { @Test public void testCreateSearchParameter_DefaultPartition() { - addCreateDefaultPartition(); - addReadDefaultPartition(); // one for search param validation + addNextTargetPartitionForCreateDefaultPartition(); + addNextTargetPartitionForReadDefaultPartition(); // one for search param validation SearchParameter sp = new SearchParameter(); sp.addBase("Patient"); sp.setStatus(Enumerations.PublicationStatus.ACTIVE); @@ -160,13 +156,13 @@ public void testCreate_CrossPartitionReference_ByPid_Allowed() { myStorageSettings.setIndexMissingFields(JpaStorageSettings.IndexEnabledEnum.DISABLED); // Create patient in partition 1 - addCreatePartition(myPartitionId, myPartitionDate); + addNextTargetPartitionForCreate(myPartitionId, myPartitionDate); Patient patient = new Patient(); patient.setActive(true); IIdType patientId = myPatientDao.create(patient, mySrd).getId().toUnqualifiedVersionless(); // Create observation in partition 2 - addCreatePartition(myPartitionId2, myPartitionDate2); + addNextTargetPartitionForCreate(myPartitionId2, myPartitionDate2); Observation obs = new Observation(); obs.getSubject().setReference(patientId.getValue()); @@ -195,13 +191,13 @@ public void testCreate_CrossPartitionReference_ByPid_Allowed() { public void testCreate_CrossPartitionReference_ByPid_NotAllowed() { // Create patient in partition 1 - addCreatePartition(myPartitionId, myPartitionDate); + addNextTargetPartitionForCreate(myPartitionId, myPartitionDate); Patient patient = new Patient(); patient.setActive(true); IIdType patientId = myPatientDao.create(patient, mySrd).getId().toUnqualifiedVersionless(); // Create observation in partition 2 - addCreatePartition(myPartitionId2, myPartitionDate2); + addNextTargetPartitionForCreate(myPartitionId2, myPartitionDate2); Observation obs = new Observation(); obs.getSubject().setReference(patientId.getValue()); @@ -219,14 +215,14 @@ public void testCreate_CrossPartitionReference_ByForcedId_Allowed() { myPartitionSettings.setAllowReferencesAcrossPartitions(PartitionSettings.CrossPartitionReferenceMode.ALLOWED_UNQUALIFIED); // Create patient in partition 1 - addCreatePartition(myPartitionId, myPartitionDate); + addNextTargetPartitionForCreate(myPartitionId, myPartitionDate); Patient patient = new Patient(); patient.setId("ONE"); patient.setActive(true); IIdType patientId = myPatientDao.update(patient, mySrd).getId().toUnqualifiedVersionless(); // Create observation in partition 2 - addCreatePartition(myPartitionId2, myPartitionDate2); + addNextTargetPartitionForCreate(myPartitionId2, myPartitionDate2); Observation obs = new Observation(); obs.getSubject().setReference(patientId.getValue()); IIdType obsId = myObservationDao.create(obs, mySrd).getId().toUnqualifiedVersionless(); @@ -244,14 +240,14 @@ public void testCreate_CrossPartitionReference_ByForcedId_Allowed() { public void testCreate_CrossPartitionReference_ByForcedId_NotAllowed() { // Create patient in partition 1 - addCreatePartition(myPartitionId, myPartitionDate); + addNextTargetPartitionForCreate(myPartitionId, myPartitionDate); Patient patient = new Patient(); patient.setId("ONE"); patient.setActive(true); IIdType patientId = myPatientDao.update(patient, mySrd).getId().toUnqualifiedVersionless(); // Create observation in partition 2 - addCreatePartition(myPartitionId2, myPartitionDate2); + addNextTargetPartitionForCreate(myPartitionId2, myPartitionDate2); Observation obs = new Observation(); obs.getSubject().setReference(patientId.getValue()); @@ -267,13 +263,13 @@ public void testCreate_CrossPartitionReference_ByForcedId_NotAllowed() { @Test public void testCreate_SamePartitionReference_DefaultPartition_ByPid() { // Create patient in partition NULL - addCreateDefaultPartition(myPartitionDate); + addNextTargetPartitionForCreateDefaultPartition(myPartitionDate); Patient patient = new Patient(); patient.setActive(true); IIdType patientId = myPatientDao.create(patient, mySrd).getId().toUnqualifiedVersionless(); // Create observation in partition NULL - addCreateDefaultPartition(myPartitionDate); + addNextTargetPartitionForCreateDefaultPartition(myPartitionDate); Observation obs = new Observation(); obs.getSubject().setReference(patientId.getValue()); IIdType obsId = myObservationDao.create(obs, mySrd).getId().toUnqualifiedVersionless(); @@ -290,14 +286,14 @@ public void testCreate_SamePartitionReference_DefaultPartition_ByPid() { @Test public void testCreate_SamePartitionReference_DefaultPartition_ByForcedId() { // Create patient in partition NULL - addCreateDefaultPartition(myPartitionDate); + addNextTargetPartitionForCreateDefaultPartition(myPartitionDate); Patient patient = new Patient(); patient.setId("ONE"); patient.setActive(true); IIdType patientId = myPatientDao.update(patient, mySrd).getId().toUnqualifiedVersionless(); // Create observation in partition NULL - addCreateDefaultPartition(myPartitionDate); + addNextTargetPartitionForCreateDefaultPartition(myPartitionDate); Observation obs = new Observation(); obs.getSubject().setReference(patientId.getValue()); IIdType obsId = myObservationDao.create(obs, mySrd).getId().toUnqualifiedVersionless(); @@ -313,8 +309,8 @@ public void testCreate_SamePartitionReference_DefaultPartition_ByForcedId() { @Test public void testCreateSearchParameter_DefaultPartitionWithDate() { - addCreateDefaultPartition(myPartitionDate); - addReadDefaultPartition(); // one for search param validation + addNextTargetPartitionForCreateDefaultPartition(myPartitionDate); + addNextTargetPartitionForReadDefaultPartition(); // one for search param validation SearchParameter sp = new SearchParameter(); sp.addBase("Patient"); @@ -338,7 +334,7 @@ public void testCreateSearchParameter_DefaultPartitionWithDate() { @Test public void testCreateSearchParameter_NonDefaultPartition() { - addCreatePartition(myPartitionId, myPartitionDate); + addNextTargetPartitionForCreate(myPartitionId, myPartitionDate); SearchParameter sp = new SearchParameter(); sp.addBase("Patient"); @@ -359,24 +355,24 @@ public void testCreateSearchParameter_NonDefaultPartition() { public void testCreate_AutoCreatePlaceholderTargets() { myStorageSettings.setAutoCreatePlaceholderReferenceTargets(true); - addCreatePartition(1, null); - addCreatePartition(1, null); + addNextTargetPartitionForCreate(1, null); + addNextTargetPartitionForCreate(1, null); IIdType patientId1 = createPatient(withOrganization(new IdType("Organization/FOO"))); assertNoRemainingPartitionIds(); - addReadPartition(1); + addNextTargetPartitionsForRead(1); IdType gotId1 = myPatientDao.read(patientId1, mySrd).getIdElement().toUnqualifiedVersionless(); assertEquals(patientId1, gotId1); - addReadPartition(1); + addNextTargetPartitionsForRead(1); IdType gotIdOrg = myOrganizationDao.read(new IdType("Organization/FOO"), mySrd).getIdElement().toUnqualifiedVersionless(); assertEquals("Organization/FOO", gotIdOrg.toUnqualifiedVersionless().getValue()); } @Test public void testCreate_UnknownPartition() { - addCreatePartition(99, null); + addNextTargetPartitionForCreate(99, null); Patient patient = new Patient(); patient.addIdentifier().setSystem("system").setValue("value"); @@ -392,7 +388,7 @@ public void testCreate_UnknownPartition() { @Test public void testCreate_ServerId_NoPartition() { - addCreateDefaultPartition(); + addNextTargetPartitionForCreateDefaultPartition(); Patient patient = new Patient(); patient.addIdentifier().setSystem("system").setValue("value"); @@ -411,12 +407,12 @@ public void testCreate_ServerId_WithPartition() { createUniqueComboSp(); createRequestId(); - addCreatePartition(myPartitionId, myPartitionDate); + addNextTargetPartitionForCreate(myPartitionId, myPartitionDate); Organization org = new Organization(); org.setName("org"); IIdType orgId = myOrganizationDao.create(org, mySrd).getId().toUnqualifiedVersionless(); - addCreatePartition(myPartitionId, myPartitionDate); + addNextTargetPartitionForCreate(myPartitionId, myPartitionDate); Patient p = new Patient(); p.getMeta().addTag("http://system", "code", "diisplay"); p.addName().setFamily("FAM"); @@ -496,12 +492,12 @@ public void testCreate_ServerId_DefaultPartition() { createUniqueComboSp(); createRequestId(); - addCreateDefaultPartition(myPartitionDate); + addNextTargetPartitionForCreateDefaultPartition(myPartitionDate); Organization org = new Organization(); org.setName("org"); IIdType orgId = myOrganizationDao.create(org, mySrd).getId().toUnqualifiedVersionless(); - addCreateDefaultPartition(myPartitionDate); + addNextTargetPartitionForCreateDefaultPartition(myPartitionDate); Patient p = new Patient(); p.getMeta().addTag("http://system", "code", "diisplay"); p.addName().setFamily("FAM"); @@ -576,13 +572,13 @@ public void testCreate_ServerId_DefaultPartition() { @Test public void testCreate_ForcedId_WithPartition() { - addCreatePartition(myPartitionId, myPartitionDate); + addNextTargetPartitionForCreate(myPartitionId, myPartitionDate); Organization org = new Organization(); org.setId("org"); org.setName("org"); IIdType orgId = myOrganizationDao.update(org, mySrd).getId().toUnqualifiedVersionless(); - addCreatePartition(myPartitionId, myPartitionDate); + addNextTargetPartitionForCreate(myPartitionId, myPartitionDate); Patient p = new Patient(); p.setId("pat"); p.getManagingOrganization().setReferenceElement(orgId); @@ -602,13 +598,13 @@ public void testCreate_ForcedId_WithPartition() { @Test public void testCreate_ForcedId_NoPartition() { - addCreateDefaultPartition(); + addNextTargetPartitionForCreateDefaultPartition(); Organization org = new Organization(); org.setId("org"); org.setName("org"); IIdType orgId = myOrganizationDao.update(org, mySrd).getId().toUnqualifiedVersionless(); - addCreateDefaultPartition(); + addNextTargetPartitionForCreateDefaultPartition(); Patient p = new Patient(); p.setId("pat"); p.getManagingOrganization().setReferenceElement(orgId); @@ -626,13 +622,13 @@ public void testCreate_ForcedId_NoPartition() { @Test public void testCreate_ForcedId_DefaultPartition() { - addCreateDefaultPartition(myPartitionDate); + addNextTargetPartitionForCreateDefaultPartition(myPartitionDate); Organization org = new Organization(); org.setId("org"); org.setName("org"); IIdType orgId = myOrganizationDao.update(org, mySrd).getId().toUnqualifiedVersionless(); - addCreateDefaultPartition(myPartitionDate); + addNextTargetPartitionForCreateDefaultPartition(myPartitionDate); Patient p = new Patient(); p.setId("pat"); p.getManagingOrganization().setReferenceElement(orgId); @@ -656,12 +652,12 @@ public void testCreateInTransaction_ServerId_WithPartition() { createUniqueComboSp(); createRequestId(); - addCreatePartition(myPartitionId, myPartitionDate); - addCreatePartition(myPartitionId, myPartitionDate); - addCreatePartition(myPartitionId, myPartitionDate); - addCreatePartition(myPartitionId, myPartitionDate); - addCreatePartition(myPartitionId, myPartitionDate); - addCreatePartition(myPartitionId, myPartitionDate); + addNextTargetPartitionForCreate(myPartitionId, myPartitionDate); + addNextTargetPartitionForCreate(myPartitionId, myPartitionDate); + addNextTargetPartitionForCreate(myPartitionId, myPartitionDate); + addNextTargetPartitionForCreate(myPartitionId, myPartitionDate); + addNextTargetPartitionForCreate(myPartitionId, myPartitionDate); + addNextTargetPartitionForCreate(myPartitionId, myPartitionDate); Bundle input = new Bundle(); input.setType(Bundle.BundleType.TRANSACTION); @@ -696,25 +692,25 @@ public void testCreateInTransaction_ServerId_WithPartition() { public void testDeleteExpunge_Cascade() { myPartitionSettings.setPartitioningEnabled(true); - addCreatePartition(myPartitionId, myPartitionDate); - addCreatePartition(myPartitionId, myPartitionDate); + addNextTargetPartitionForCreate(myPartitionId, myPartitionDate); + addNextTargetPartitionForCreate(myPartitionId, myPartitionDate); IIdType p1 = createPatient(withActiveTrue()); IIdType o1 = createObservation(withSubject(p1)); - addCreatePartition(myPartitionId2, myPartitionDate); - addCreatePartition(myPartitionId2, myPartitionDate); + addNextTargetPartitionForCreate(myPartitionId2, myPartitionDate); + addNextTargetPartitionForCreate(myPartitionId2, myPartitionDate); IIdType p2 = createPatient(withActiveTrue()); IIdType o2 = createObservation(withSubject(p2)); assertNoRemainingPartitionIds(); // validate precondition - addReadAllPartitions(); - addReadAllPartitions(); + addNextTargetPartitionForReadAllPartitions(); + addNextTargetPartitionForReadAllPartitions(); assertEquals(2, myPatientDao.search(SearchParameterMap.newSynchronous(), mySrd).size()); assertEquals(2, myObservationDao.search(SearchParameterMap.newSynchronous(), mySrd).size()); - addReadPartition(myPartitionId); - addReadPartition(myPartitionId); + addNextTargetPartitionsForRead(myPartitionId); + addNextTargetPartitionsForRead(myPartitionId); assertEquals(1, myPatientDao.search(SearchParameterMap.newSynchronous(), mySrd).size()); assertEquals(1, myObservationDao.search(SearchParameterMap.newSynchronous(), mySrd).size()); @@ -737,13 +733,13 @@ public void testDeleteExpunge_Cascade() { // Validate JobInstance outcome = myBatch2JobHelper.awaitJobCompletion(startResponse); assertEquals(2, outcome.getCombinedRecordsProcessed()); - addReadAllPartitions(); + addNextTargetPartitionForReadAllPartitions(); assertDoesntExist(p1); - addReadAllPartitions(); + addNextTargetPartitionForReadAllPartitions(); assertDoesntExist(o1); - addReadAllPartitions(); + addNextTargetPartitionForReadAllPartitions(); assertNotGone(p2); - addReadAllPartitions(); + addNextTargetPartitionForReadAllPartitions(); assertNotGone(o2); } @@ -761,7 +757,7 @@ public void testUpdateResourceWithPartition() { createRequestId(); // Create a resource - addCreatePartition(myPartitionId, myPartitionDate); + addNextTargetPartitionForCreate(myPartitionId, myPartitionDate); Patient patient = new Patient(); patient.getMeta().addTag("http://system", "code", "display"); patient.setActive(true); @@ -769,7 +765,7 @@ public void testUpdateResourceWithPartition() { assertPersistedPartitionIdMatches(patientId); // Update that resource - addCreatePartition(myPartitionId); + addNextTargetPartitionForCreate(myPartitionId); patient = new Patient(); patient.setId("Patient/" + patientId.getId()); patient.setActive(false); @@ -812,8 +808,8 @@ public void testUpdateConditionalInPartition() { createRequestId(); // Create a resource - addCreatePartition(myPartitionId, myPartitionDate); - addReadPartition(myPartitionId); + addNextTargetPartitionForCreate(myPartitionId, myPartitionDate); + addNextTargetPartitionsForRead(myPartitionId); Patient p = new Patient(); p.setActive(false); p.addIdentifier().setValue("12345"); @@ -833,8 +829,8 @@ public void testUpdateConditionalInPartition() { }); // Update that resource - addReadPartition(myPartitionId); - addCreatePartition(myPartitionId); + addNextTargetPartitionsForRead(myPartitionId); + addNextTargetPartitionForCreate(myPartitionId); p = new Patient(); p.setActive(true); p.addIdentifier().setValue("12345"); @@ -871,7 +867,7 @@ public void testRead_PidId_AllPartitions() { IIdType patientId2 = createPatient(withPartition(2), withActiveTrue()); { - addReadAllPartitions(); + addNextTargetPartitionForReadAllPartitions(); myCaptureQueriesListener.clear(); IdType gotId1 = myPatientDao.read(patientId1, mySrd).getIdElement().toUnqualifiedVersionless(); assertEquals(patientId1, gotId1); @@ -884,7 +880,7 @@ public void testRead_PidId_AllPartitions() { assertEquals(1, StringUtils.countMatches(searchSql, "PARTITION_ID"), searchSql); } { - addReadAllPartitions(); + addNextTargetPartitionForReadAllPartitions(); IdType gotId2 = myPatientDao.read(patientId2, mySrd).getIdElement().toUnqualifiedVersionless(); assertEquals(patientId2, gotId2); @@ -906,7 +902,7 @@ public void testRead_PidId_SpecificPartition() { // Read in correct Partition { myCaptureQueriesListener.clear(); - addReadPartition(1); + addNextTargetPartitionsForRead(1); IdType gotId1 = myPatientDao.read(patientId1, mySrd).getIdElement().toUnqualifiedVersionless(); assertEquals(patientId1, gotId1); @@ -920,7 +916,7 @@ public void testRead_PidId_SpecificPartition() { // Read in null Partition { - addReadPartition(1); + addNextTargetPartitionsForRead(1); try { myPatientDao.read(patientIdNull, mySrd).getIdElement().toUnqualifiedVersionless(); fail(); @@ -931,7 +927,7 @@ public void testRead_PidId_SpecificPartition() { // Read in wrong Partition { - addReadPartition(1); + addNextTargetPartitionsForRead(1); try { myPatientDao.read(patientId2, mySrd).getIdElement().toUnqualifiedVersionless(); fail(); @@ -954,7 +950,7 @@ public void testRead_PidId_MultiplePartitionNames() { { myCaptureQueriesListener.clear(); assertNoRemainingPartitionIds(); - myPartitionInterceptor.addReadPartition(RequestPartitionId.fromPartitionNames(PARTITION_1, PARTITION_2)); + myPartitionInterceptor.addNextTargetReadPartition(RequestPartitionId.fromPartitionNames(PARTITION_1, PARTITION_2)); IdType gotId1 = myPatientDao.read(patientId1, mySrd).getIdElement().toUnqualifiedVersionless(); assertEquals(patientId1, gotId1); @@ -968,7 +964,7 @@ public void testRead_PidId_MultiplePartitionNames() { // Two partitions including default - Found { myCaptureQueriesListener.clear(); - myPartitionInterceptor.addReadPartition(RequestPartitionId.fromPartitionNames(PARTITION_1, JpaConstants.DEFAULT_PARTITION_NAME)); + myPartitionInterceptor.addNextTargetReadPartition(RequestPartitionId.fromPartitionNames(PARTITION_1, JpaConstants.DEFAULT_PARTITION_NAME)); IdType gotId1; try { gotId1 = myPatientDao.read(patientIdNull, mySrd).getIdElement().toUnqualifiedVersionless(); @@ -987,7 +983,7 @@ public void testRead_PidId_MultiplePartitionNames() { // Two partitions - Not Found { - myPartitionInterceptor.addReadPartition(RequestPartitionId.fromPartitionNames(PARTITION_1, PARTITION_2)); + myPartitionInterceptor.addNextTargetReadPartition(RequestPartitionId.fromPartitionNames(PARTITION_1, PARTITION_2)); try { myPatientDao.read(patientId3, mySrd); fail(); @@ -995,7 +991,7 @@ public void testRead_PidId_MultiplePartitionNames() { // good } - myPartitionInterceptor.addReadPartition(RequestPartitionId.fromPartitionNames(PARTITION_1, PARTITION_2)); + myPartitionInterceptor.addNextTargetReadPartition(RequestPartitionId.fromPartitionNames(PARTITION_1, PARTITION_2)); try { myPatientDao.read(patientIdNull, mySrd); fail(); @@ -1016,7 +1012,7 @@ public void testRead_PidId_MultiplePartitionIds() { // Two partitions - Found { myCaptureQueriesListener.clear(); - myPartitionInterceptor.addReadPartition(RequestPartitionId.fromPartitionIds(1, 2)); + myPartitionInterceptor.addNextTargetReadPartition(RequestPartitionId.fromPartitionIds(1, 2)); IdType gotId1 = myPatientDao.read(patientId1, mySrd).getIdElement().toUnqualifiedVersionless(); assertEquals(patientId1, gotId1); @@ -1035,7 +1031,7 @@ public void testRead_PidId_MultiplePartitionIds() { { myCaptureQueriesListener.clear(); myPartitionInterceptor.assertNoRemainingIds(); - myPartitionInterceptor.addReadPartition(RequestPartitionId.fromPartitionIds(1, null)); + myPartitionInterceptor.addNextTargetReadPartition(RequestPartitionId.fromPartitionIds(1, null)); IdType gotId1 = myPatientDao.read(patientIdNull, mySrd).getIdElement().toUnqualifiedVersionless(); assertEquals(patientIdNull, gotId1); @@ -1047,7 +1043,7 @@ public void testRead_PidId_MultiplePartitionIds() { // Two partitions - Not Found { - myPartitionInterceptor.addReadPartition(RequestPartitionId.fromPartitionNames(PARTITION_1, PARTITION_2)); + myPartitionInterceptor.addNextTargetReadPartition(RequestPartitionId.fromPartitionNames(PARTITION_1, PARTITION_2)); try { myPatientDao.read(patientId3, mySrd); fail(); @@ -1055,7 +1051,7 @@ public void testRead_PidId_MultiplePartitionIds() { // good } - myPartitionInterceptor.addReadPartition(RequestPartitionId.fromPartitionNames(PARTITION_1, PARTITION_2)); + myPartitionInterceptor.addNextTargetReadPartition(RequestPartitionId.fromPartitionNames(PARTITION_1, PARTITION_2)); try { myPatientDao.read(patientIdNull, mySrd); fail(); @@ -1075,7 +1071,7 @@ public void testRead_PidId_DefaultPartition() { // Read in correct Partition { myCaptureQueriesListener.clear(); - addReadDefaultPartition(); + addNextTargetPartitionForReadDefaultPartition(); IdType gotId1 = myPatientDao.read(patientIdNull, mySrd).getIdElement().toUnqualifiedVersionless(); assertEquals(patientIdNull, gotId1); @@ -1089,7 +1085,7 @@ public void testRead_PidId_DefaultPartition() { // Read in wrong Partition { - addReadDefaultPartition(); + addNextTargetPartitionForReadDefaultPartition(); try { myPatientDao.read(patientId1, mySrd).getIdElement().toUnqualifiedVersionless(); fail(); @@ -1104,7 +1100,7 @@ public void testRead_PidId_DefaultPartition() { public void testRead_PidId_UnknownResourceId() { // Read in specific Partition { - addReadPartition(1); + addNextTargetPartitionsForRead(1); try { myPatientDao.read(new IdType("Patient/1"), mySrd); fail(); @@ -1115,7 +1111,7 @@ public void testRead_PidId_UnknownResourceId() { // Read in null Partition { - addReadDefaultPartition(); + addNextTargetPartitionForReadDefaultPartition(); try { myPatientDao.read(new IdType("Patient/1"), mySrd); fail(); @@ -1130,7 +1126,7 @@ public void testRead_PidId_ResourceIdOnlyExistsInDifferentPartition() { IIdType id = createPatient(withPartition(2), withActiveTrue()); // Read in specific Partition { - addReadPartition(1); + addNextTargetPartitionsForRead(1); try { myPatientDao.read(id, mySrd); fail(); @@ -1141,7 +1137,7 @@ public void testRead_PidId_ResourceIdOnlyExistsInDifferentPartition() { // Read in null Partition { - addReadDefaultPartition(); + addNextTargetPartitionForReadDefaultPartition(); try { myPatientDao.read(id, mySrd); fail(); @@ -1158,12 +1154,12 @@ public void testRead_ForcedId_SpecificPartition() { IIdType patientId2 = createPatient(withPartition(2), withActiveTrue(), withId("TWO")); // Read in correct Partition - addReadPartition(1); + addNextTargetPartitionsForRead(1); IdType gotId1 = myPatientDao.read(patientId1, mySrd).getIdElement().toUnqualifiedVersionless(); assertEquals(patientId1, gotId1); // Read in null Partition - addReadPartition(1); + addNextTargetPartitionsForRead(1); try { myPatientDao.read(patientIdNull, mySrd).getIdElement().toUnqualifiedVersionless(); fail(); @@ -1172,7 +1168,7 @@ public void testRead_ForcedId_SpecificPartition() { } // Read in wrong Partition - addReadPartition(1); + addNextTargetPartitionsForRead(1); try { myPatientDao.read(patientId2, mySrd).getIdElement().toUnqualifiedVersionless(); fail(); @@ -1181,7 +1177,7 @@ public void testRead_ForcedId_SpecificPartition() { } // Read in wrong Partition - addReadPartition(2); + addNextTargetPartitionsForRead(2); try { myPatientDao.read(patientId1, mySrd).getIdElement().toUnqualifiedVersionless(); fail(); @@ -1190,7 +1186,7 @@ public void testRead_ForcedId_SpecificPartition() { } // Read in correct Partition - addReadPartition(2); + addNextTargetPartitionsForRead(2); IdType gotId2 = myPatientDao.read(patientId2, mySrd).getIdElement().toUnqualifiedVersionless(); assertEquals(patientId2, gotId2); @@ -1203,12 +1199,12 @@ public void testRead_ForcedId_DefaultPartition() { IIdType patientId2 = createPatient(withPartition(2), withActiveTrue(), withId("TWO")); // Read in correct Partition - addReadDefaultPartition(); + addNextTargetPartitionForReadDefaultPartition(); IdType gotId1 = myPatientDao.read(patientIdNull, mySrd).getIdElement().toUnqualifiedVersionless(); assertEquals(patientIdNull, gotId1); // Read in null Partition - addReadDefaultPartition(); + addNextTargetPartitionForReadDefaultPartition(); try { myPatientDao.read(patientId1, mySrd).getIdElement().toUnqualifiedVersionless(); fail(); @@ -1217,7 +1213,7 @@ public void testRead_ForcedId_DefaultPartition() { } // Read in wrong Partition - addReadDefaultPartition(); + addNextTargetPartitionForReadDefaultPartition(); try { myPatientDao.read(patientId2, mySrd).getIdElement().toUnqualifiedVersionless(); fail(); @@ -1232,12 +1228,12 @@ public void testRead_ForcedId_AllPartition() { IIdType patientId1 = createPatient(withPartition(1), withActiveTrue(), withId("ONE")); createPatient(withPartition(2), withActiveTrue(), withId("TWO")); { - addReadAllPartitions(); + addNextTargetPartitionForReadAllPartitions(); IdType gotId1 = myPatientDao.read(patientIdNull, mySrd).getIdElement().toUnqualifiedVersionless(); assertEquals(patientIdNull, gotId1); } { - addReadAllPartitions(); + addNextTargetPartitionForReadAllPartitions(); IdType gotId1 = myPatientDao.read(patientId1, mySrd).getIdElement().toUnqualifiedVersionless(); assertEquals(patientId1, gotId1); } @@ -1253,7 +1249,7 @@ public void testRead_ForcedId_AllPartition_WithDuplicate() { assertEquals(patientIdNull, patientId2); { - addReadAllPartitions(); + addNextTargetPartitionForReadAllPartitions(); try { myPatientDao.read(patientIdNull, mySrd); fail(); @@ -1276,7 +1272,7 @@ public void testSearch_IdParamOnly_PidId_SpecificPartition() { // Read in correct Partition { myCaptureQueriesListener.clear(); - addReadPartition(1); + addNextTargetPartitionsForRead(1); SearchParameterMap map = SearchParameterMap.newSynchronous(IAnyResource.SP_RES_ID, new TokenParam(patientId1.toUnqualifiedVersionless().getValue())); IBundleProvider searchOutcome = myPatientDao.search(map, mySrd); @@ -1295,7 +1291,7 @@ public void testSearch_IdParamOnly_PidId_SpecificPartition() { // Read in null Partition { - addReadPartition(1); + addNextTargetPartitionsForRead(1); SearchParameterMap map = SearchParameterMap.newSynchronous(IAnyResource.SP_RES_ID, new TokenParam(patientIdNull.toUnqualifiedVersionless().getValue())); IBundleProvider searchOutcome = myPatientDao.search(map, mySrd); @@ -1304,7 +1300,7 @@ public void testSearch_IdParamOnly_PidId_SpecificPartition() { // Read in wrong Partition { - addReadPartition(1); + addNextTargetPartitionsForRead(1); SearchParameterMap map = SearchParameterMap.newSynchronous(IAnyResource.SP_RES_ID, new TokenParam(patientId2.toUnqualifiedVersionless().getValue())); IBundleProvider searchOutcome = myPatientDao.search(map, mySrd); @@ -1327,7 +1323,7 @@ public void testSearch_IdParamSecond_PidId_SpecificPartition() { // Read in correct Partition { myCaptureQueriesListener.clear(); - addReadPartition(1); + addNextTargetPartitionsForRead(1); SearchParameterMap map = SearchParameterMap.newSynchronous() .add(Patient.SP_ACTIVE, new TokenParam("true")) @@ -1348,7 +1344,7 @@ public void testSearch_IdParamSecond_PidId_SpecificPartition() { // Read in null Partition { - addReadPartition(1); + addNextTargetPartitionsForRead(1); SearchParameterMap map = SearchParameterMap.newSynchronous() .add(Patient.SP_ACTIVE, new TokenParam("true")) @@ -1359,7 +1355,7 @@ public void testSearch_IdParamSecond_PidId_SpecificPartition() { // Read in wrong Partition { - addReadPartition(1); + addNextTargetPartitionsForRead(1); SearchParameterMap map = SearchParameterMap.newSynchronous() .add(Patient.SP_ACTIVE, new TokenParam("true")) @@ -1384,7 +1380,7 @@ public void testSearch_IdParamOnly_ForcedId_SpecificPartition() { // Read in correct Partition { myCaptureQueriesListener.clear(); - addReadPartition(1); + addNextTargetPartitionsForRead(1); SearchParameterMap map = SearchParameterMap.newSynchronous(IAnyResource.SP_RES_ID, new TokenParam(patientId1.toUnqualifiedVersionless().getValue())); IBundleProvider searchOutcome = myPatientDao.search(map, mySrd); @@ -1403,7 +1399,7 @@ public void testSearch_IdParamOnly_ForcedId_SpecificPartition() { // Read in null Partition { - addReadPartition(1); + addNextTargetPartitionsForRead(1); SearchParameterMap map = SearchParameterMap.newSynchronous(IAnyResource.SP_RES_ID, new TokenParam(patientIdNull.toUnqualifiedVersionless().getValue())); IBundleProvider searchOutcome = myPatientDao.search(map, mySrd); @@ -1412,7 +1408,7 @@ public void testSearch_IdParamOnly_ForcedId_SpecificPartition() { // Read in wrong Partition { - addReadPartition(1); + addNextTargetPartitionsForRead(1); SearchParameterMap map = SearchParameterMap.newSynchronous(IAnyResource.SP_RES_ID, new TokenParam(patientId2.toUnqualifiedVersionless().getValue())); IBundleProvider searchOutcome = myPatientDao.search(map, mySrd); @@ -1437,7 +1433,7 @@ public void testSearch_IdParamSecond_ForcedId_SpecificPartition() { // Read in correct Partition { myCaptureQueriesListener.clear(); - addReadPartition(1); + addNextTargetPartitionsForRead(1); SearchParameterMap map = SearchParameterMap.newSynchronous() .add(Patient.SP_ACTIVE, new TokenParam("true")) @@ -1457,7 +1453,7 @@ public void testSearch_IdParamSecond_ForcedId_SpecificPartition() { // Read in null Partition { - addReadPartition(1); + addNextTargetPartitionsForRead(1); SearchParameterMap map = SearchParameterMap.newSynchronous() .add(Patient.SP_ACTIVE, new TokenParam("true")) @@ -1468,7 +1464,7 @@ public void testSearch_IdParamSecond_ForcedId_SpecificPartition() { // Read in wrong Partition { - addReadPartition(1); + addNextTargetPartitionsForRead(1); SearchParameterMap map = SearchParameterMap.newSynchronous() .add(Patient.SP_ACTIVE, new TokenParam("true")) @@ -1488,7 +1484,7 @@ public void testSearch_MissingParamString_SearchAllPartitions() { // :missing=true { - addReadAllPartitions(); + addNextTargetPartitionForReadAllPartitions(); myCaptureQueriesListener.clear(); SearchParameterMap map = new SearchParameterMap(); map.add(Patient.SP_ACTIVE, new StringParam().setMissing(true)); @@ -1505,7 +1501,7 @@ public void testSearch_MissingParamString_SearchAllPartitions() { // :missing=false { - addReadAllPartitions(); + addNextTargetPartitionForReadAllPartitions(); myCaptureQueriesListener.clear(); SearchParameterMap map = new SearchParameterMap(); map.add(Patient.SP_FAMILY, new StringParam().setMissing(false)); @@ -1530,7 +1526,7 @@ public void testSearch_MissingParamString_SearchOnePartition() { // :missing=true { - addReadPartition(1); + addNextTargetPartitionsForRead(1); myCaptureQueriesListener.clear(); SearchParameterMap map = new SearchParameterMap(); map.add(Patient.SP_ACTIVE, new StringParam().setMissing(true)); @@ -1547,7 +1543,7 @@ public void testSearch_MissingParamString_SearchOnePartition() { // :missing=false { - addReadPartition(1); + addNextTargetPartitionsForRead(1); myCaptureQueriesListener.clear(); SearchParameterMap map = new SearchParameterMap(); map.add(Patient.SP_FAMILY, new StringParam().setMissing(false)); @@ -1571,7 +1567,7 @@ public void testSearch_MissingParamString_SearchDefaultPartition() { // :missing=true { - addReadDefaultPartition(); + addNextTargetPartitionForReadDefaultPartition(); myCaptureQueriesListener.clear(); SearchParameterMap map = new SearchParameterMap(); map.add(Patient.SP_ACTIVE, new StringParam().setMissing(true)); @@ -1588,7 +1584,7 @@ public void testSearch_MissingParamString_SearchDefaultPartition() { // :missing=false { - addReadDefaultPartition(); + addNextTargetPartitionForReadDefaultPartition(); myCaptureQueriesListener.clear(); SearchParameterMap map = new SearchParameterMap(); map.add(Patient.SP_FAMILY, new StringParam().setMissing(false)); @@ -1614,7 +1610,7 @@ public void testSearch_MissingParamReference_SearchAllPartitions() { // :missing=true { - addReadAllPartitions(); + addNextTargetPartitionForReadAllPartitions(); myCaptureQueriesListener.clear(); SearchParameterMap map = new SearchParameterMap(); map.add(Patient.SP_GENERAL_PRACTITIONER, new StringParam().setMissing(true)); @@ -1641,7 +1637,7 @@ public void testSearch_MissingParamReference_SearchOnePartition_IncludePartition // :missing=true { - addReadPartition(1); + addNextTargetPartitionsForRead(1); myCaptureQueriesListener.clear(); SearchParameterMap map = new SearchParameterMap(); map.add(Patient.SP_GENERAL_PRACTITIONER, new StringParam().setMissing(true)); @@ -1669,7 +1665,7 @@ public void testSearch_MissingParamReference_SearchOnePartition_DontIncludeParti // :missing=true { - addReadPartition(1); + addNextTargetPartitionsForRead(1); myCaptureQueriesListener.clear(); SearchParameterMap map = new SearchParameterMap(); map.add(Patient.SP_GENERAL_PRACTITIONER, new StringParam().setMissing(true)); @@ -1695,7 +1691,7 @@ public void testSearch_MissingParamReference_SearchDefaultPartition() { // :missing=true { - addReadDefaultPartition(); + addNextTargetPartitionForReadDefaultPartition(); myCaptureQueriesListener.clear(); SearchParameterMap map = new SearchParameterMap(); map.add(Patient.SP_GENERAL_PRACTITIONER, new StringParam().setMissing(true)); @@ -1720,7 +1716,7 @@ public void testSearch_NoParams_SearchAllPartitions() { IIdType patientId1 = createPatient(withPartition(1), withActiveTrue()); IIdType patientId2 = createPatient(withPartition(2), withActiveTrue()); - addReadAllPartitions(); + addNextTargetPartitionForReadAllPartitions(); myCaptureQueriesListener.clear(); SearchParameterMap map = new SearchParameterMap(); @@ -1740,7 +1736,7 @@ public void testSearch_NoParams_SearchOnePartition() { IIdType patientId1 = createPatient(withPartition(1), withActiveTrue()); createPatient(withPartition(2), withActiveTrue()); - addReadPartition(1); + addNextTargetPartitionsForRead(1); myCaptureQueriesListener.clear(); SearchParameterMap map = new SearchParameterMap(); @@ -1761,7 +1757,7 @@ public void testSearch_NoParams_SearchMultiplePartitionsByName_NoDefault() { IIdType patientId2 = createPatient(withPartition(2), withActiveTrue()); createPatient(withPartition(3), withActiveTrue()); - addReadPartitions(PARTITION_1, PARTITION_2); + addNextTargetPartitionsForRead(PARTITION_1, PARTITION_2); myCaptureQueriesListener.clear(); SearchParameterMap map = new SearchParameterMap(); @@ -1782,7 +1778,7 @@ public void testSearch_NoParams_SearchMultiplePartitionsByName_WithDefault() { IIdType patientId2 = createPatient(withPartition(2), withActiveTrue()); createPatient(withPartition(3), withActiveTrue()); - addReadPartitions(JpaConstants.DEFAULT_PARTITION_NAME, PARTITION_2); + addNextTargetPartitionsForRead(JpaConstants.DEFAULT_PARTITION_NAME, PARTITION_2); myCaptureQueriesListener.clear(); SearchParameterMap map = new SearchParameterMap(); @@ -1810,7 +1806,7 @@ public void testSearch_DateParam_SearchAllPartitions() { // Date param - addReadAllPartitions(); + addNextTargetPartitionForReadAllPartitions(); myCaptureQueriesListener.clear(); SearchParameterMap map = new SearchParameterMap(); map.add(Patient.SP_BIRTHDATE, new DateParam("2020-04-20")); @@ -1826,7 +1822,7 @@ public void testSearch_DateParam_SearchAllPartitions() { // Date OR param - addReadAllPartitions(); + addNextTargetPartitionForReadAllPartitions(); myCaptureQueriesListener.clear(); map = new SearchParameterMap(); map.add(Patient.SP_BIRTHDATE, new DateOrListParam().addOr(new DateParam("2020-04-20")).addOr(new DateParam("2020-04-22"))); @@ -1842,7 +1838,7 @@ public void testSearch_DateParam_SearchAllPartitions() { // Date AND param - addReadAllPartitions(); + addNextTargetPartitionForReadAllPartitions(); myCaptureQueriesListener.clear(); map = new SearchParameterMap(); map.add(Patient.SP_BIRTHDATE, new DateAndListParam().addAnd(new DateOrListParam().addOr(new DateParam("2020"))).addAnd(new DateOrListParam().addOr(new DateParam("2020-04-20")))); @@ -1858,7 +1854,7 @@ public void testSearch_DateParam_SearchAllPartitions() { // DateRangeParam - addReadAllPartitions(); + addNextTargetPartitionForReadAllPartitions(); myCaptureQueriesListener.clear(); map = new SearchParameterMap(); map.add(Patient.SP_BIRTHDATE, new DateRangeParam(new DateParam("2020-01-01"), new DateParam("2020-04-25"))); @@ -1891,7 +1887,7 @@ public void testSearch_DateParam_SearchSpecificPartitions() { runInTransaction(() -> { ourLog.info("Date indexes:\n * {}", myResourceIndexedSearchParamDateDao.findAll().stream().map(ResourceIndexedSearchParamDate::toString).collect(Collectors.joining("\n * "))); }); - addReadPartition(1); + addNextTargetPartitionsForRead(1); myCaptureQueriesListener.clear(); SearchParameterMap map = new SearchParameterMap(); map.add(Patient.SP_BIRTHDATE, new DateParam("2020-04-20")); @@ -1909,7 +1905,7 @@ public void testSearch_DateParam_SearchSpecificPartitions() { // Date OR param - addReadPartition(1); + addNextTargetPartitionsForRead(1); myCaptureQueriesListener.clear(); map = new SearchParameterMap(); map.add(Patient.SP_BIRTHDATE, new DateOrListParam().addOr(new DateParam("2020-04-20")).addOr(new DateParam("2020-04-22"))); @@ -1925,7 +1921,7 @@ public void testSearch_DateParam_SearchSpecificPartitions() { // Date AND param - addReadPartition(1); + addNextTargetPartitionsForRead(1); myCaptureQueriesListener.clear(); map = new SearchParameterMap(); map.add(Patient.SP_BIRTHDATE, new DateAndListParam().addAnd(new DateOrListParam().addOr(new DateParam("2020"))).addAnd(new DateOrListParam().addOr(new DateParam("2020-04-20")))); @@ -1941,7 +1937,7 @@ public void testSearch_DateParam_SearchSpecificPartitions() { // DateRangeParam - addReadPartition(1); + addNextTargetPartitionsForRead(1); myCaptureQueriesListener.clear(); map = new SearchParameterMap(); map.add(Patient.SP_BIRTHDATE, new DateRangeParam(new DateParam("2020-01-01"), new DateParam("2020-04-25"))); @@ -1973,7 +1969,7 @@ public void testSearch_DateParam_SearchDefaultPartitions() { // Date param - addReadDefaultPartition(); + addNextTargetPartitionForReadDefaultPartition(); myCaptureQueriesListener.clear(); SearchParameterMap map = new SearchParameterMap(); map.add(Patient.SP_BIRTHDATE, new DateParam("2020-04-20")); @@ -1989,7 +1985,7 @@ public void testSearch_DateParam_SearchDefaultPartitions() { // Date OR param - addReadDefaultPartition(); + addNextTargetPartitionForReadDefaultPartition(); myCaptureQueriesListener.clear(); map = new SearchParameterMap(); map.add(Patient.SP_BIRTHDATE, new DateOrListParam().addOr(new DateParam("2020-04-20")).addOr(new DateParam("2020-04-22"))); @@ -2005,7 +2001,7 @@ public void testSearch_DateParam_SearchDefaultPartitions() { // Date AND param - addReadDefaultPartition(); + addNextTargetPartitionForReadDefaultPartition(); myCaptureQueriesListener.clear(); map = new SearchParameterMap(); map.add(Patient.SP_BIRTHDATE, new DateAndListParam().addAnd(new DateOrListParam().addOr(new DateParam("2020"))).addAnd(new DateOrListParam().addOr(new DateParam("2020-04-20")))); @@ -2021,7 +2017,7 @@ public void testSearch_DateParam_SearchDefaultPartitions() { // DateRangeParam - addReadDefaultPartition(); + addNextTargetPartitionForReadDefaultPartition(); myCaptureQueriesListener.clear(); map = new SearchParameterMap(); map.add(Patient.SP_BIRTHDATE, new DateRangeParam(new DateParam("2020-01-01"), new DateParam("2020-04-25"))); @@ -2052,7 +2048,7 @@ public void testSearch_DateParam_SearchDefaultPartitions_NonNullDefaultPartition createPatient(withPartition(2), withBirthdate("2021-04-20")); // Date param - addReadDefaultPartition(); + addNextTargetPartitionForReadDefaultPartition(); myCaptureQueriesListener.clear(); SearchParameterMap map = new SearchParameterMap(); map.add(Patient.SP_BIRTHDATE, new DateParam("2020-04-20")); @@ -2070,26 +2066,26 @@ public void testSearch_DateParam_SearchDefaultPartitions_NonNullDefaultPartition @Test public void testSearch_HasParam_SearchOnePartition() { - addCreatePartition(1, null); + addNextTargetPartitionForCreate(1, null); Organization org = new Organization(); org.setId("ORG"); org.setName("ORG"); myOrganizationDao.update(org, mySrd); - addCreatePartition(1, null); + addNextTargetPartitionForCreate(1, null); Practitioner practitioner = new Practitioner(); practitioner.setId("PRACT"); practitioner.addName().setFamily("PRACT"); myPractitionerDao.update(practitioner, mySrd); - addCreatePartition(1, null); + addNextTargetPartitionForCreate(1, null); PractitionerRole role = new PractitionerRole(); role.setId("ROLE"); role.getPractitioner().setReference("Practitioner/PRACT"); role.getOrganization().setReference("Organization/ORG"); myPractitionerRoleDao.update(role, mySrd); - addReadPartition(1); + addNextTargetPartitionsForRead(1); SearchParameterMap params = SearchParameterMap.newSynchronous(); HasAndListParam value = new HasAndListParam(); value.addAnd(new HasOrListParam().addOr(new HasParam("PractitionerRole", "practitioner", "_id", "ROLE"))); @@ -2114,7 +2110,7 @@ public void testSearch_StringParam_SearchAllPartitions() { IIdType patientId1 = createPatient(withPartition(1), withFamily("FAMILY")); IIdType patientId2 = createPatient(withPartition(2), withFamily("FAMILY")); - addReadAllPartitions(); + addNextTargetPartitionForReadAllPartitions(); myCaptureQueriesListener.clear(); SearchParameterMap map = new SearchParameterMap(); @@ -2136,7 +2132,7 @@ public void testSearch_StringParam_SearchDefaultPartition() { createPatient(withPartition(1), withFamily("FAMILY")); createPatient(withPartition(2), withFamily("FAMILY")); - addReadDefaultPartition(); + addNextTargetPartitionForReadDefaultPartition(); myCaptureQueriesListener.clear(); SearchParameterMap map = new SearchParameterMap(); @@ -2160,7 +2156,7 @@ public void testSearch_StringParam_SearchOnePartition() { IIdType patientId1 = createPatient(withPartition(1), withFamily("FAMILY")); createPatient(withPartition(2), withFamily("FAMILY")); - addReadPartition(1); + addNextTargetPartitionsForRead(1); myCaptureQueriesListener.clear(); SearchParameterMap map = new SearchParameterMap(); @@ -2196,7 +2192,7 @@ public void testSearch_StringParam_SearchMultiplePartitions() { // Match two partitions { - addReadPartition(1, 2); + addNextTargetPartitionsForRead(1, 2); myCaptureQueriesListener.clear(); IBundleProvider results = myPatientDao.search(map, mySrd); @@ -2211,7 +2207,7 @@ public void testSearch_StringParam_SearchMultiplePartitions() { // Match two partitions including null { - addReadPartition(1, null); + addNextTargetPartitionsForRead(1, null); myCaptureQueriesListener.clear(); IBundleProvider results = myPatientDao.search(map, mySrd); @@ -2235,7 +2231,7 @@ public void testSearch_StringParam_SearchMultiplePartitions_IncludePartitionInHa map.add(Patient.SP_FAMILY, new StringParam("FAMILY")); map.setLoadSynchronous(true); - addReadPartition(1, 2); + addNextTargetPartitionsForRead(1, 2); try { myPatientDao.search(map, mySrd); fail(); @@ -2248,7 +2244,7 @@ public void testSearch_StringParam_SearchMultiplePartitions_IncludePartitionInHa public void testSearch_StringParam_SearchAllPartitions_IncludePartitionInHashes() { myPartitionSettings.setIncludePartitionInSearchHashes(true); - addReadAllPartitions(); + addNextTargetPartitionForReadAllPartitions(); myCaptureQueriesListener.clear(); SearchParameterMap map = new SearchParameterMap(); @@ -2271,7 +2267,7 @@ public void testSearch_StringParam_SearchDefaultPartition_IncludePartitionInHash createPatient(withPartition(1), withFamily("FAMILY")); createPatient(withPartition(2), withFamily("FAMILY")); - addReadDefaultPartition(); + addNextTargetPartitionForReadDefaultPartition(); myCaptureQueriesListener.clear(); SearchParameterMap map = new SearchParameterMap(); @@ -2299,7 +2295,7 @@ public void testSearch_StringParam_SearchOnePartition_IncludePartitionInHashes() logAllStringIndexes(); - addReadPartition(1); + addNextTargetPartitionsForRead(1); myCaptureQueriesListener.clear(); SearchParameterMap map = new SearchParameterMap(); @@ -2322,21 +2318,21 @@ public void testSearch_StringParam_SearchOnePartition_IncludePartitionInHashes() @Test @Disabled public void testSearch_StringParam_SearchOnePartition_AddRevIncludes() { - addReadPartition(1); - addCreatePartition(1, null); + addNextTargetPartitionsForRead(1); + addNextTargetPartitionForCreate(1, null); Organization org = new Organization(); org.setName("FOO"); org.setId("FOO-ORG"); myOrganizationDao.update(org, mySrd); for (int i = 0; i < 50; i++) { - addCreatePartition(1, null); + addNextTargetPartitionForCreate(1, null); PractitionerRole pr = new PractitionerRole(); pr.getOrganization().setReference("Organization/FOO-ORG"); myPractitionerRoleDao.create(pr, mySrd); } - addReadPartition(1); + addNextTargetPartitionsForRead(1); myCaptureQueriesListener.clear(); SearchParameterMap map = new SearchParameterMap(); @@ -2358,7 +2354,7 @@ public void testSearch_TagNotParam_SearchAllPartitions() { createPatient(withPartition(1), withActiveTrue(), withTag("http://system", "code2")); createPatient(withPartition(2), withActiveTrue(), withTag("http://system", "code2")); - addReadAllPartitions(); + addNextTargetPartitionForReadAllPartitions(); myCaptureQueriesListener.clear(); SearchParameterMap map = new SearchParameterMap(); map.add(Constants.PARAM_TAG, new TokenParam("http://system", "code2").setModifier(TokenParamModifier.NOT)); @@ -2374,7 +2370,7 @@ public void testSearch_TagNotParam_SearchAllPartitions() { // And with another param - addReadAllPartitions(); + addNextTargetPartitionForReadAllPartitions(); myCaptureQueriesListener.clear(); map = new SearchParameterMap(); map.add(Constants.PARAM_TAG, new TokenParam("http://system", "code2").setModifier(TokenParamModifier.NOT)); @@ -2399,7 +2395,7 @@ public void testSearch_TagNotParam_SearchDefaultPartition() { createPatient(withPartition(1), withActiveTrue(), withTag("http://system", "code")); createPatient(withPartition(2), withActiveTrue(), withTag("http://system", "code")); - addReadDefaultPartition(); + addNextTargetPartitionForReadDefaultPartition(); myCaptureQueriesListener.clear(); SearchParameterMap map = new SearchParameterMap(); @@ -2426,7 +2422,7 @@ public void testSearch_TagNotParam_SearchOnePartition() { createPatient(withPartition(1), withActiveTrue(), withTag("http://system", "code2")); createPatient(withPartition(2), withActiveTrue(), withTag("http://system", "code2")); - addReadPartition(1); + addNextTargetPartitionsForRead(1); myCaptureQueriesListener.clear(); SearchParameterMap map = new SearchParameterMap(); @@ -2448,7 +2444,7 @@ public void testSearch_TagParam_SearchAllPartitions() { IIdType patientId1 = createPatient(withPartition(1), withActiveTrue(), withTag("http://system", "code")); IIdType patientId2 = createPatient(withPartition(2), withActiveTrue(), withTag("http://system", "code")); - addReadAllPartitions(); + addNextTargetPartitionForReadAllPartitions(); myCaptureQueriesListener.clear(); SearchParameterMap map = new SearchParameterMap(); @@ -2470,7 +2466,7 @@ public void testSearch_TagParam_SearchOnePartition() { IIdType patientId1 = createPatient(withPartition(1), withActiveTrue(), withTag("http://system", "code")); createPatient(withPartition(2), withActiveTrue(), withTag("http://system", "code")); - addReadPartition(1); + addNextTargetPartitionsForRead(1); myCaptureQueriesListener.clear(); SearchParameterMap map = new SearchParameterMap(); @@ -2499,7 +2495,7 @@ public void testSearch_TagParamNot_SearchAllPartitions() { createPatient(withPartition(1), withActiveTrue(), withTag("http://system", "code"), withTag("http://system", "code2")); createPatient(withPartition(2), withActiveTrue(), withTag("http://system", "code"), withTag("http://system", "code2")); - addReadAllPartitions(); + addNextTargetPartitionForReadAllPartitions(); myCaptureQueriesListener.clear(); SearchParameterMap map = new SearchParameterMap(); @@ -2524,7 +2520,7 @@ public void testSearch_TagParamNot_SearchOnePartition() { createPatient(withPartition(1), withActiveTrue(), withTag("http://system", "code"), withTag("http://system", "code2")); createPatient(withPartition(2), withActiveTrue(), withTag("http://system", "code"), withTag("http://system", "code2")); - addReadPartition(1); + addNextTargetPartitionsForRead(1); logAllResources(); logAllResourceVersions(); @@ -2661,7 +2657,7 @@ public void testSearch_UniqueParam_SearchAllPartitions() { IIdType id = createPatient(withPartition(1), withGender("male"), withFamily("FAM")); - addReadAllPartitions(); + addNextTargetPartitionForReadAllPartitions(); myCaptureQueriesListener.clear(); SearchParameterMap map = new SearchParameterMap(); @@ -2687,7 +2683,7 @@ public void testSearch_UniqueParam_SearchOnePartition() { IIdType id = createPatient(withPartition(1), withGender("male"), withFamily("FAM")); - addReadPartition(1); + addNextTargetPartitionsForRead(1); myCaptureQueriesListener.clear(); SearchParameterMap map = new SearchParameterMap(); map.add(Patient.SP_FAMILY, new StringParam("FAM")); @@ -2704,7 +2700,7 @@ public void testSearch_UniqueParam_SearchOnePartition() { assertThat(searchSql).containsOnlyOnce("IDX_STRING = 'Patient?family=FAM&gender=male'"); // Same query, different partition - addReadPartition(2); + addNextTargetPartitionsForRead(2); myCaptureQueriesListener.clear(); map = new SearchParameterMap(); map.add(Patient.SP_GENDER, new TokenParam(null, "male")); @@ -2737,13 +2733,13 @@ public void testSearch_NonUniqueComboParam(String theReadPartitions) { switch (theReadPartitions) { case "ALL": - addReadAllPartitions(); + addNextTargetPartitionForReadAllPartitions(); break; case "ONE": - addReadPartition(myPartitionId); + addNextTargetPartitionsForRead(myPartitionId); break; case "MANY": - addReadPartition(myPartitionId, myPartitionId4); + addNextTargetPartitionsForRead(myPartitionId, myPartitionId4); break; default: throw new IllegalStateException(); @@ -2787,7 +2783,7 @@ public void testSearch_RefParam_TargetPid_SearchOnePartition() { IIdType patientId = createPatient(withPartition(myPartitionId), withGender("male")); IIdType observationId = createObservation(withPartition(myPartitionId), withSubject(patientId)); - addReadPartition(myPartitionId); + addNextTargetPartitionsForRead(myPartitionId); myCaptureQueriesListener.clear(); SearchParameterMap map = new SearchParameterMap(); map.add(Observation.SP_SUBJECT, new ReferenceParam(patientId)); @@ -2805,7 +2801,7 @@ public void testSearch_RefParam_TargetPid_SearchOnePartition() { assertThat(StringUtils.countMatches(searchSql, "PARTITION_ID")).as(searchSql).isEqualTo(2); // Same query, different partition - addReadPartition(2); + addNextTargetPartitionsForRead(2); myCaptureQueriesListener.clear(); map = new SearchParameterMap(); map.add(Observation.SP_SUBJECT, new ReferenceParam(patientId)); @@ -2824,7 +2820,7 @@ public void testSearch_RefParam_TargetPid_SearchDefaultPartition() { IIdType patientId = createPatient(withPartition(null), withGender("male")); IIdType observationId = createObservation(withPartition(null), withSubject(patientId)); - addReadDefaultPartition(); + addNextTargetPartitionForReadDefaultPartition(); myCaptureQueriesListener.clear(); SearchParameterMap map = new SearchParameterMap(); @@ -2842,7 +2838,7 @@ public void testSearch_RefParam_TargetPid_SearchDefaultPartition() { assertEquals(2, StringUtils.countMatches(searchSql, "PARTITION_ID")); // Same query, different partition - addReadPartition(2); + addNextTargetPartitionsForRead(2); myCaptureQueriesListener.clear(); map = new SearchParameterMap(); map.add(Observation.SP_SUBJECT, new ReferenceParam(patientId)); @@ -2861,7 +2857,7 @@ public void testSearch_RefParam_TargetForcedId_SearchOnePartition() { IIdType patientId = createPatient(withPartition(myPartitionId), withId("ONE"), withGender("male")); IIdType observationId = createObservation(withPartition(myPartitionId), withSubject(patientId)); - addReadPartition(myPartitionId); + addNextTargetPartitionsForRead(myPartitionId); myCaptureQueriesListener.clear(); SearchParameterMap map = new SearchParameterMap(); map.add(Observation.SP_SUBJECT, new ReferenceParam(patientId)); @@ -2877,7 +2873,7 @@ public void testSearch_RefParam_TargetForcedId_SearchOnePartition() { assertThat(StringUtils.countMatches(searchSql, "PARTITION_ID")).as(searchSql).isEqualTo(2); // Same query, different partition - addReadPartition(2); + addNextTargetPartitionsForRead(2); myCaptureQueriesListener.clear(); map = new SearchParameterMap(); map.add(Observation.SP_SUBJECT, new ReferenceParam(patientId)); @@ -2914,7 +2910,7 @@ public void testSearch_TokenParam_CodeInValueSet() { logAllTokenIndexes(); myCaptureQueriesListener.clear(); - addReadPartitions(PARTITION_1); + addNextTargetPartitionsForRead(PARTITION_1); SearchParameterMap map = SearchParameterMap.newSynchronous("code", new TokenParam("http://vs").setModifier(TokenParamModifier.IN)); IBundleProvider outcome = myObservationDao.search(map, mySrd); List actual = toUnqualifiedVersionlessIdValues(outcome); @@ -2931,7 +2927,7 @@ public void testSearch_RefParam_TargetForcedId_SearchDefaultPartition() { IIdType patientId = createPatient(withPartition(null), withId("ONE"), withGender("male")); IIdType observationId = createObservation(withPartition(null), withSubject(patientId)); - addReadDefaultPartition(); + addNextTargetPartitionForReadDefaultPartition(); myCaptureQueriesListener.clear(); SearchParameterMap map = new SearchParameterMap(); @@ -2947,7 +2943,7 @@ public void testSearch_RefParam_TargetForcedId_SearchDefaultPartition() { assertThat(StringUtils.countMatches(searchSql, "PARTITION_ID")).as(searchSql).isEqualTo(2); // Same query, different partition - addReadPartition(2); + addNextTargetPartitionsForRead(2); myCaptureQueriesListener.clear(); map = new SearchParameterMap(); map.add(Observation.SP_SUBJECT, new ReferenceParam(patientId)); @@ -3010,7 +3006,7 @@ public void testTransaction_MultipleConditionalUpdates() { ourLog.info("About to start transaction"); for (int i = 0; i < 60; i++) { - addCreatePartition(1, null); + addNextTargetPartitionForCreate(1, null); } // Pre-fetch the partition ID from the partition lookup table @@ -3145,7 +3141,7 @@ public void testTransactionWithManyInlineMatchUrls() throws IOException { public void testUpdate_ResourcePreExistsInWrongPartition() { IIdType patientId = createPatient(withPartition(null), withId("ONE"), withBirthdate("2020-01-01")); - addCreatePartition(1); + addNextTargetPartitionForCreate(1); Patient patient = new Patient(); patient.setId(patientId.toUnqualifiedVersionless()); @@ -3158,13 +3154,13 @@ public void testHistory_Instance_CorrectPartition() { IIdType id = createPatient(withPartition(1), withBirthdate("2020-01-01")); // Update the patient - addCreatePartition(myPartitionId); + addNextTargetPartitionForCreate(myPartitionId); Patient patient = new Patient(); patient.setActive(false); patient.setId(id); myPatientDao.update(patient, mySrd); - addReadPartition(1); + addNextTargetPartitionsForRead(1); myCaptureQueriesListener.clear(); IBundleProvider results = myPatientDao.history(id, null, null, null, mySrd); assertEquals(2, results.sizeOrThrowNpe()); @@ -3194,13 +3190,13 @@ public void testHistory_Instance_WrongPartition() { IIdType id = createPatient(withPartition(1), withBirthdate("2020-01-01")); // Update the patient - addCreatePartition(myPartitionId); + addNextTargetPartitionForCreate(myPartitionId); Patient p = new Patient(); p.setActive(false); p.setId(id); myPatientDao.update(p, mySrd); - addReadPartition(2); + addNextTargetPartitionsForRead(2); try { myPatientDao.history(id, null, null, null, mySrd); fail(); @@ -3214,7 +3210,7 @@ public void testHistory_Instance_DefaultPartition() { IIdType id = createPatient(withPartition(null), withBirthdate("2020-01-01")); // Update the patient - addCreateDefaultPartition(); + addNextTargetPartitionForCreateDefaultPartition(); Patient patient = new Patient(); patient.setActive(false); patient.setId(id); @@ -3222,7 +3218,7 @@ public void testHistory_Instance_DefaultPartition() { logAllResourceVersions(); - addReadDefaultPartition(); + addNextTargetPartitionForReadDefaultPartition(); myCaptureQueriesListener.clear(); IBundleProvider results = myPatientDao.history(id, null, null, null, mySrd); int size = results.sizeOrThrowNpe(); @@ -3254,13 +3250,13 @@ public void testHistory_Instance_AllPartitions() { IIdType id = createPatient(withPartition(1), withBirthdate("2020-01-01")); // Update the patient - addCreatePartition(myPartitionId); + addNextTargetPartitionForCreate(myPartitionId); Patient patient = new Patient(); patient.setActive(false); patient.setId(id); myPatientDao.update(patient, mySrd); - addReadAllPartitions(); + addNextTargetPartitionForReadAllPartitions(); myCaptureQueriesListener.clear(); IBundleProvider results = myPatientDao.history(id, null, null, null, mySrd); assertEquals(2, results.sizeOrThrowNpe()); @@ -3270,7 +3266,7 @@ public void testHistory_Instance_AllPartitions() { @Test public void testHistory_Server() { - addReadAllPartitions(); + addNextTargetPartitionForReadAllPartitions(); try { mySystemDao.history(null, null, null, mySrd).size(); fail(); @@ -3289,7 +3285,7 @@ public void testHistory_Server_SpecificPartition() { sleepAtLeast(10); createPatient(withPartition(2), withBirthdate("2020-01-01")); - addReadPartition(1); + addNextTargetPartitionsForRead(1); myCaptureQueriesListener.clear(); IBundleProvider results = mySystemDao.history(null, null, null, mySrd); assertEquals(2, results.sizeOrThrowNpe()); @@ -3321,7 +3317,7 @@ public void testHistory_Server_DefaultPartition() { sleepAtLeast(10); createPatient(withPartition(2), withBirthdate("2020-01-01")); - addReadDefaultPartition(); + addNextTargetPartitionForReadDefaultPartition(); myCaptureQueriesListener.clear(); IBundleProvider results = mySystemDao.history(null, null, null, mySrd); assertEquals(2, results.sizeOrThrowNpe()); @@ -3358,7 +3354,7 @@ public void testHistory_Server_MultiplePartitions() { // Multiple Partitions { - addReadPartition(2, null); + addNextTargetPartitionsForRead(2, null); myCaptureQueriesListener.clear(); IBundleProvider results = mySystemDao.history(null, null, null, mySrd); assertEquals(4, results.sizeOrThrowNpe()); @@ -3368,7 +3364,7 @@ public void testHistory_Server_MultiplePartitions() { // Multiple Partitions With Null { - addReadPartition(2, 3); + addNextTargetPartitionsForRead(2, 3); myCaptureQueriesListener.clear(); IBundleProvider results = mySystemDao.history(null, null, null, mySrd); assertEquals(4, results.sizeOrThrowNpe()); @@ -3380,7 +3376,7 @@ public void testHistory_Server_MultiplePartitions() { @Test public void testHistory_Type_AllPartitions() { - addReadAllPartitions(); + addNextTargetPartitionForReadAllPartitions(); try { myPatientDao.history(null, null, null, mySrd).size(); fail(); @@ -3399,7 +3395,7 @@ public void testHistory_Type_SpecificPartition() { sleepAtLeast(10); createPatient(withPartition(2), withBirthdate("2020-01-01")); - addReadPartition(1); + addNextTargetPartitionsForRead(1); myCaptureQueriesListener.clear(); IBundleProvider results = myPatientDao.history(null, null, null, mySrd); assertEquals(2, results.sizeOrThrowNpe()); @@ -3431,7 +3427,7 @@ public void testHistory_Type_DefaultPartition() { sleepAtLeast(10); createPatient(withPartition(2), withBirthdate("2020-01-01")); - addReadDefaultPartition(); + addNextTargetPartitionForReadDefaultPartition(); myCaptureQueriesListener.clear(); IBundleProvider results = myPatientDao.history(null, null, null, mySrd); assertEquals(2, results.sizeOrThrowNpe()); diff --git a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/provider/r4/MultitenantServerR4Test.java b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/provider/r4/MultitenantServerR4Test.java index 538eb778818..fb2f76d93a2 100644 --- a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/provider/r4/MultitenantServerR4Test.java +++ b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/provider/r4/MultitenantServerR4Test.java @@ -15,8 +15,10 @@ import ca.uhn.fhir.jpa.model.config.PartitionSettings; import ca.uhn.fhir.jpa.model.dao.JpaPid; import ca.uhn.fhir.jpa.model.entity.ResourceTable; -import ca.uhn.fhir.jpa.model.entity.StorageSettings; import ca.uhn.fhir.jpa.model.util.JpaConstants; + +import static ca.uhn.fhir.jpa.util.ConcurrencyTestUtil.executeFutures; + import ca.uhn.fhir.jpa.util.MemoryCacheService; import ca.uhn.fhir.model.api.Include; import ca.uhn.fhir.rest.api.Constants; @@ -24,6 +26,7 @@ import ca.uhn.fhir.rest.api.server.SystemRequestDetails; import ca.uhn.fhir.rest.api.server.bulk.BulkExportJobParameters; import ca.uhn.fhir.rest.server.RestfulServer; +import ca.uhn.fhir.rest.server.exceptions.InternalErrorException; import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; import ca.uhn.fhir.rest.server.exceptions.MethodNotAllowedException; import ca.uhn.fhir.rest.server.exceptions.ResourceNotFoundException; @@ -40,14 +43,19 @@ import java.util.ArrayList; import java.util.Map; import java.util.TreeMap; +import java.util.concurrent.Callable; +import java.util.concurrent.CompletionService; import java.util.concurrent.ExecutionException; +import java.util.concurrent.ExecutorCompletionService; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.Future; import org.apache.commons.io.IOUtils; +import org.apache.commons.lang3.exception.ExceptionUtils; import org.apache.http.client.methods.CloseableHttpResponse; import org.apache.http.client.methods.HttpGet; + import org.hl7.fhir.instance.model.api.IBaseResource; import org.hl7.fhir.instance.model.api.IIdType; import org.hl7.fhir.r4.model.Bundle; @@ -88,7 +96,6 @@ import java.io.IOException; import java.nio.charset.StandardCharsets; -import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.Date; @@ -976,11 +983,13 @@ class TestConcurrencyInterceptorInPartitioningMode { private static final int THREAD_COUNT = 10; private ExecutorService myExecutor; + CompletionService myCompletionService; private TransactionConcurrencySemaphoreInterceptor myConcurrencySemaphoreInterceptor; @BeforeEach public void before() { myExecutor = Executors.newFixedThreadPool(THREAD_COUNT); + myCompletionService = new ExecutorCompletionService<>(myExecutor); myConcurrencySemaphoreInterceptor = new TransactionConcurrencySemaphoreInterceptor(myMemoryCacheService); RestfulServer server = new RestfulServer(myFhirContext); @@ -994,42 +1003,38 @@ public void after() { } @Test - public void testTransactionCreates_WithConcurrencySemaphore_DontLockOnCachedMatchUrlsForConditionalCreate() throws ExecutionException, InterruptedException { + public void testTransactionCreates_WithConcurrencySemaphore_DontLockOnCachedMatchUrlsForConditionalCreate() { myStorageSettings.setMatchUrlCacheEnabled(true); myPartitionSettings.setConditionalCreateDuplicateIdentifiersEnabled(true); myInterceptorRegistry.registerInterceptor(myConcurrencySemaphoreInterceptor); myConcurrencySemaphoreInterceptor.setLogWaits(true); - Runnable creatorForPartitionA = ()->{ + Callable creatorForPartitionA = () -> { Bundle input = createBundleForRunnableTransaction(); SystemRequestDetails requestDetails = new SystemRequestDetails(); requestDetails.setTenantId(TENANT_A); - mySystemDao.transaction(requestDetails, input); + return executeTransactionOrThrow(requestDetails, input); }; - Runnable creatorForPartitionB = ()->{ + Callable creatorForPartitionB = () -> { Bundle input = createBundleForRunnableTransaction(); SystemRequestDetails requestDetails = new SystemRequestDetails(); requestDetails.setTenantId(TENANT_B); - mySystemDao.transaction(requestDetails, input); + return executeTransactionOrThrow(requestDetails, input); }; for (int set = 0; set < 3; set++) { myConcurrencySemaphoreInterceptor.clearSemaphores(); - List> futures = new ArrayList<>(); for (int j = 0; j < 10; j++) { if (j % 2 == 0) { - futures.add(myExecutor.submit(creatorForPartitionA)); + myCompletionService.submit(creatorForPartitionA); } else { - futures.add(myExecutor.submit(creatorForPartitionB)); + myCompletionService.submit(creatorForPartitionB); } - } - for (Future next : futures) { - next.get(); - } + executeFutures(myCompletionService, 10); // Only a thread from the first iteration (set) will acquire the semaphores for the match urls // Since the match URLs will still be present in the Match URL cache for the remaining of the test @@ -1046,6 +1051,17 @@ public void testTransactionCreates_WithConcurrencySemaphore_DontLockOnCachedMatc }); } + private boolean executeTransactionOrThrow(SystemRequestDetails requestDetails, Bundle input) { + try { + mySystemDao.transaction(requestDetails, input); + return true; + } catch (Throwable theError) { + String bundleAsString = myFhirContext.newJsonParser().encodeResourceToString(input); + ourLog.error("Caught an error during processing instance {}", bundleAsString, theError); + throw new InternalErrorException("Caught an error during processing instance " + bundleAsString, theError); + } + } + private Bundle createBundleForRunnableTransaction() { BundleBuilder bb = new BundleBuilder(myFhirContext); diff --git a/hapi-fhir-jpaserver-test-utilities/src/main/java/ca/uhn/fhir/jpa/util/ConcurrencyTestUtil.java b/hapi-fhir-jpaserver-test-utilities/src/main/java/ca/uhn/fhir/jpa/util/ConcurrencyTestUtil.java new file mode 100644 index 00000000000..1499386781f --- /dev/null +++ b/hapi-fhir-jpaserver-test-utilities/src/main/java/ca/uhn/fhir/jpa/util/ConcurrencyTestUtil.java @@ -0,0 +1,50 @@ +package ca.uhn.fhir.jpa.util; + +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.CompletionService; +import java.util.concurrent.Future; + +import org.apache.commons.lang3.exception.ExceptionUtils; + +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class ConcurrencyTestUtil { + + private static final Logger ourLog = LoggerFactory.getLogger(ConcurrencyTestUtil.class); + + private ConcurrencyTestUtil() {} + + public static void executeFutures(CompletionService theCompletionService, int theTotal) { + List errors = new ArrayList<>(); + int count = 0; + + while (count + errors.size() < theTotal) { + try { + // svc.take() will process the next future that is ready + Future future = theCompletionService.take(); + boolean r = future.get(); + assertTrue(r); + count++; + } catch (Exception ex) { + // we will run all the threads to completion, even if we have errors + // this is so we don't have background threads kicking around with + // partial changes. + // we either do this, or shutdown the completion service in an + // "inelegant" manner, dropping all threads (which we aren't doing) + ourLog.error("Failed after checking {} futures", count); + String[] frames = ExceptionUtils.getRootCauseStackTrace(ex); + errors.add(ex + "\n" + String.join("\n ", frames)); + } + } + + if (!errors.isEmpty()) { + fail(String.format("Failed to execute futures. Found %d errors :%n", errors.size()) + + String.join(", ", errors)); + } + } +} From 6196969af2c268a9597c08d15d1b2163442b3b30 Mon Sep 17 00:00:00 2001 From: jdar Date: Wed, 5 Mar 2025 14:16:10 -0800 Subject: [PATCH 6/6] address code review comments --- .../ca/uhn/fhir/jpa/util/ConcurrencyTestUtil.java | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/hapi-fhir-jpaserver-test-utilities/src/main/java/ca/uhn/fhir/jpa/util/ConcurrencyTestUtil.java b/hapi-fhir-jpaserver-test-utilities/src/main/java/ca/uhn/fhir/jpa/util/ConcurrencyTestUtil.java index 1499386781f..f1433ca5144 100644 --- a/hapi-fhir-jpaserver-test-utilities/src/main/java/ca/uhn/fhir/jpa/util/ConcurrencyTestUtil.java +++ b/hapi-fhir-jpaserver-test-utilities/src/main/java/ca/uhn/fhir/jpa/util/ConcurrencyTestUtil.java @@ -1,18 +1,17 @@ package ca.uhn.fhir.jpa.util; +import org.apache.commons.lang3.exception.ExceptionUtils; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + import java.util.ArrayList; import java.util.List; import java.util.concurrent.CompletionService; import java.util.concurrent.Future; -import org.apache.commons.lang3.exception.ExceptionUtils; - import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - public class ConcurrencyTestUtil { private static final Logger ourLog = LoggerFactory.getLogger(ConcurrencyTestUtil.class); @@ -44,7 +43,7 @@ public static void executeFutures(CompletionService theCompletionServic if (!errors.isEmpty()) { fail(String.format("Failed to execute futures. Found %d errors :%n", errors.size()) - + String.join(", ", errors)); + + String.join(", ", errors)); } } }