From 072f3a422ab350850d3edd9394f670adcaa25761 Mon Sep 17 00:00:00 2001 From: James Agnew Date: Mon, 29 Jun 2020 18:22:27 -0400 Subject: [PATCH] Use read partition for finding update candidate on upsert (#1945) * Use read partition for finding update candidate on upsert * Add changelog --- ...45-use-read-partition-for-upsert-read.yaml | 7 ++ .../fhir/jpa/dao/BaseHapiFhirResourceDao.java | 3 +- .../fhir/jpa/dao/r4/PartitioningR4Test.java | 71 ++++++++++++++----- 3 files changed, 61 insertions(+), 20 deletions(-) create mode 100644 hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_1_0/1945-use-read-partition-for-upsert-read.yaml diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_1_0/1945-use-read-partition-for-upsert-read.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_1_0/1945-use-read-partition-for-upsert-read.yaml new file mode 100644 index 00000000000..bf70447be81 --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_1_0/1945-use-read-partition-for-upsert-read.yaml @@ -0,0 +1,7 @@ +--- +type: change +issue: 1945 +title: "When performing a JPA 'upsert' (a PUT to an ID that may or may not already exist) on a partitioned system, + the partition interceptor will now be called once to determine the READ partition in order to find the candidate + resource to update, and possibly a second time to determine the CREATE partition if a new row is actually being + created. Previously only the CREATE partition was checked and used to perform the initial read." 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 79bffd0d49c..144c498bbde 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 @@ -1331,10 +1331,11 @@ public abstract class BaseHapiFhirResourceDao extends B */ resourceId = theResource.getIdElement(); - RequestPartitionId requestPartitionId = myRequestPartitionHelperService.determineCreatePartitionForRequest(theRequest, theResource, getResourceName()); + RequestPartitionId requestPartitionId = myRequestPartitionHelperService.determineReadPartitionForRequest(theRequest, getResourceName()); try { entity = readEntityLatestVersion(resourceId, requestPartitionId); } catch (ResourceNotFoundException e) { + requestPartitionId = myRequestPartitionHelperService.determineCreatePartitionForRequest(theRequest, theResource, getResourceName()); return doCreate(theResource, null, thePerformIndexing, theTransactionDetails, theRequest, requestPartitionId); } } diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/PartitioningR4Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/PartitioningR4Test.java index 44713d59019..d36b4596e1a 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/PartitioningR4Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/PartitioningR4Test.java @@ -231,6 +231,7 @@ public class PartitioningR4Test extends BaseJpaR4SystemTest { myPartitionSettings.setAllowReferencesAcrossPartitions(PartitionSettings.CrossPartitionReferenceMode.ALLOWED_UNQUALIFIED); // Create patient in partition 1 + addReadPartition(myPartitionId); addCreatePartition(myPartitionId, myPartitionDate); Patient patient = new Patient(); patient.setId("ONE"); @@ -256,6 +257,7 @@ public class PartitioningR4Test extends BaseJpaR4SystemTest { public void testCreate_CrossPartitionReference_ByForcedId_NotAllowed() { // Create patient in partition 1 + addReadPartition(myPartitionId); addCreatePartition(myPartitionId, myPartitionDate); Patient patient = new Patient(); patient.setId("ONE"); @@ -302,6 +304,7 @@ public class PartitioningR4Test extends BaseJpaR4SystemTest { @Test public void testCreate_SamePartitionReference_DefaultPartition_ByForcedId() { // Create patient in partition NULL + addReadDefaultPartition(); addCreateDefaultPartition(myPartitionDate); Patient patient = new Patient(); patient.setId("ONE"); @@ -567,12 +570,14 @@ public class PartitioningR4Test extends BaseJpaR4SystemTest { @Test public void testCreate_ForcedId_WithPartition() { + addReadPartition(myPartitionId); addCreatePartition(myPartitionId, myPartitionDate); Organization org = new Organization(); org.setId("org"); org.setName("org"); IIdType orgId = myOrganizationDao.update(org).getId().toUnqualifiedVersionless(); + addReadPartition(myPartitionId); addCreatePartition(myPartitionId, myPartitionDate); Patient p = new Patient(); p.setId("pat"); @@ -593,12 +598,14 @@ public class PartitioningR4Test extends BaseJpaR4SystemTest { @Test public void testCreate_ForcedId_NoPartition() { + addReadDefaultPartition(); addCreateDefaultPartition(); Organization org = new Organization(); org.setId("org"); org.setName("org"); IIdType orgId = myOrganizationDao.update(org).getId().toUnqualifiedVersionless(); + addReadDefaultPartition(); addCreateDefaultPartition(); Patient p = new Patient(); p.setId("pat"); @@ -617,12 +624,14 @@ public class PartitioningR4Test extends BaseJpaR4SystemTest { @Test public void testCreate_ForcedId_DefaultPartition() { + addReadDefaultPartition(); addCreateDefaultPartition(myPartitionDate); Organization org = new Organization(); org.setId("org"); org.setName("org"); IIdType orgId = myOrganizationDao.update(org).getId().toUnqualifiedVersionless(); + addReadDefaultPartition(); addCreateDefaultPartition(myPartitionDate); Patient p = new Patient(); p.setId("pat"); @@ -691,7 +700,6 @@ public class PartitioningR4Test extends BaseJpaR4SystemTest { // Create a resource addCreatePartition(myPartitionId, myPartitionDate); - addCreatePartition(myPartitionId, myPartitionDate); Patient p = new Patient(); p.getMeta().addTag("http://system", "code", "diisplay"); p.setActive(true); @@ -704,6 +712,7 @@ public class PartitioningR4Test extends BaseJpaR4SystemTest { }); // Update that resource + addReadPartition(myPartitionId); p = new Patient(); p.setId("Patient/" + patientId); p.setActive(false); @@ -949,9 +958,9 @@ public class PartitioningR4Test extends BaseJpaR4SystemTest { @Test public void testRead_ForcedId_SpecificPartition() { - IIdType patientIdNull = createPatient(withPartition(null), withActiveTrue(), withId("NULL")); - IIdType patientId1 = createPatient(withPartition(1), withActiveTrue(), withId("ONE")); - IIdType patientId2 = createPatient(withPartition(2), withActiveTrue(), withId("TWO")); + IIdType patientIdNull = createPatient(withPutPartition(null), withActiveTrue(), withId("NULL")); + IIdType patientId1 = createPatient(withPutPartition(1), withActiveTrue(), withId("ONE")); + IIdType patientId2 = createPatient(withPutPartition(2), withActiveTrue(), withId("TWO")); // Read in correct Partition addReadPartition(1); @@ -979,9 +988,9 @@ public class PartitioningR4Test extends BaseJpaR4SystemTest { @Test public void testRead_ForcedId_DefaultPartition() { - IIdType patientIdNull = createPatient(withPartition(null), withActiveTrue(), withId("NULL")); - IIdType patientId1 = createPatient(withPartition(1), withActiveTrue(), withId("ONE")); - IIdType patientId2 = createPatient(withPartition(2), withActiveTrue(), withId("TWO")); + IIdType patientIdNull = createPatient(withPutPartition(null), withActiveTrue(), withId("NULL")); + IIdType patientId1 = createPatient(withPutPartition(1), withActiveTrue(), withId("ONE")); + IIdType patientId2 = createPatient(withPutPartition(2), withActiveTrue(), withId("TWO")); // Read in correct Partition addReadDefaultPartition(); @@ -1009,9 +1018,9 @@ public class PartitioningR4Test extends BaseJpaR4SystemTest { @Test public void testRead_ForcedId_AllPartition() { - IIdType patientIdNull = createPatient(withPartition(null), withActiveTrue(), withId("NULL")); - IIdType patientId1 = createPatient(withPartition(1), withActiveTrue(), withId("ONE")); - createPatient(withPartition(2), withActiveTrue(), withId("TWO")); + IIdType patientIdNull = createPatient(withPutPartition(null), withActiveTrue(), withId("NULL")); + IIdType patientId1 = createPatient(withPutPartition(1), withActiveTrue(), withId("ONE")); + createPatient(withPutPartition(2), withActiveTrue(), withId("TWO")); { addReadAllPartitions(); IdType gotId1 = myPatientDao.read(patientIdNull, mySrd).getIdElement().toUnqualifiedVersionless(); @@ -1027,9 +1036,9 @@ public class PartitioningR4Test extends BaseJpaR4SystemTest { @Test public void testRead_ForcedId_AllPartition_WithDuplicate() { dropForcedIdUniqueConstraint(); - IIdType patientIdNull = createPatient(withPartition(null), withActiveTrue(), withId("FOO")); - IIdType patientId1 = createPatient(withPartition(1), withActiveTrue(), withId("FOO")); - IIdType patientId2 = createPatient(withPartition(2), withActiveTrue(), withId("FOO")); + IIdType patientIdNull = createPatient(withPutPartition(null), withActiveTrue(), withId("FOO")); + IIdType patientId1 = createPatient(withPutPartition(1), withActiveTrue(), withId("FOO")); + IIdType patientId2 = createPatient(withPutPartition(2), withActiveTrue(), withId("FOO")); assertEquals(patientIdNull, patientId1); assertEquals(patientIdNull, patientId2); @@ -2005,7 +2014,7 @@ public class PartitioningR4Test extends BaseJpaR4SystemTest { public void testSearch_RefParam_TargetForcedId_SearchOnePartition() { createUniqueCompositeSp(); - IIdType patientId = createPatient(withPartition(myPartitionId), withId("ONE"), withBirthdate("2020-01-01")); + IIdType patientId = createPatient(withPutPartition(myPartitionId), withId("ONE"), withBirthdate("2020-01-01")); IIdType observationId = createObservation(withPartition(myPartitionId), withSubject(patientId)); addReadPartition(myPartitionId); @@ -2041,7 +2050,7 @@ public class PartitioningR4Test extends BaseJpaR4SystemTest { public void testSearch_RefParam_TargetForcedId_SearchDefaultPartition() { createUniqueCompositeSp(); - IIdType patientId = createPatient(withPartition(null), withId("ONE"), withBirthdate("2020-01-01")); + IIdType patientId = createPatient(withPutPartition(null), withId("ONE"), withBirthdate("2020-01-01")); IIdType observationId = createObservation(withPartition(null), withSubject(patientId)); addReadDefaultPartition(); @@ -2073,12 +2082,24 @@ public class PartitioningR4Test extends BaseJpaR4SystemTest { } + @Test + public void testUpdate_ResourcePreExistsInWrongPartition() { + IIdType patientId = createPatient(withPutPartition(null), withId("ONE"), withBirthdate("2020-01-01")); + + addReadAllPartitions(); + + Patient p = new Patient(); + p.setId(patientId.toUnqualifiedVersionless()); + p.setGender(Enumerations.AdministrativeGender.MALE); + myPatientDao.update(p); + } + @Test public void testHistory_Instance_CorrectPartition() { IIdType id = createPatient(withPartition(1), withBirthdate("2020-01-01")); // Update the patient - addCreatePartition(myPartitionId, myPartitionDate); + addReadPartition(myPartitionId); Patient p = new Patient(); p.setActive(false); p.setId(id); @@ -2119,7 +2140,7 @@ public class PartitioningR4Test extends BaseJpaR4SystemTest { IIdType id = createPatient(withPartition(1), withBirthdate("2020-01-01")); // Update the patient - addCreatePartition(myPartitionId, myPartitionDate); + addReadPartition(myPartitionId); Patient p = new Patient(); p.setActive(false); p.setId(id); @@ -2139,7 +2160,7 @@ public class PartitioningR4Test extends BaseJpaR4SystemTest { IIdType id = createPatient(withPartition(null), withBirthdate("2020-01-01")); // Update the patient - addCreateDefaultPartition(); + addReadDefaultPartition(); Patient p = new Patient(); p.setActive(false); p.setId(id); @@ -2180,7 +2201,7 @@ public class PartitioningR4Test extends BaseJpaR4SystemTest { IIdType id = createPatient(withPartition(1), withBirthdate("2020-01-01")); // Update the patient - addCreatePartition(myPartitionId, myPartitionDate); + addReadPartition(myPartitionId); Patient p = new Patient(); p.setActive(false); p.setId(id); @@ -2454,6 +2475,18 @@ public class PartitioningR4Test extends BaseJpaR4SystemTest { }; } + private Consumer withPutPartition(Integer thePartitionId) { + return t -> { + if (thePartitionId != null) { + addReadPartition(thePartitionId); + addCreatePartition(thePartitionId, null); + } else { + addReadDefaultPartition(); + addCreateDefaultPartition(); + } + }; + } + @Interceptor public static class MyReadWriteInterceptor extends MyWriteInterceptor {