diff --git a/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/topic/SubscriptionTopicCanonicalizer.java b/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/topic/SubscriptionTopicCanonicalizer.java index c48391cdb55..dcec70eb0bc 100644 --- a/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/topic/SubscriptionTopicCanonicalizer.java +++ b/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/topic/SubscriptionTopicCanonicalizer.java @@ -31,7 +31,6 @@ public final class SubscriptionTopicCanonicalizer { private SubscriptionTopicCanonicalizer() { } - // WIP STR5 use elsewhere public static SubscriptionTopic canonicalizeTopic(FhirContext theFhirContext, IBaseResource theSubscriptionTopic) { switch (theFhirContext.getVersion().getVersion()) { case R4B: diff --git a/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/topic/SubscriptionTriggerMatcher.java b/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/topic/SubscriptionTriggerMatcher.java index cfb7d710e6e..14d6b3dfc79 100644 --- a/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/topic/SubscriptionTriggerMatcher.java +++ b/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/topic/SubscriptionTriggerMatcher.java @@ -24,14 +24,15 @@ import ca.uhn.fhir.jpa.searchparam.matcher.InMemoryMatchResult; import ca.uhn.fhir.jpa.subscription.model.ResourceModifiedMessage; import ca.uhn.fhir.rest.api.server.SystemRequestDetails; import ca.uhn.fhir.rest.server.messaging.BaseResourceMessage; +import ca.uhn.fhir.storage.PreviousVersionReader; import ca.uhn.fhir.util.Logs; import org.hl7.fhir.instance.model.api.IBaseResource; -import org.hl7.fhir.instance.model.api.IIdType; import org.hl7.fhir.r5.model.Enumeration; import org.hl7.fhir.r5.model.SubscriptionTopic; import org.slf4j.Logger; import java.util.List; +import java.util.Optional; public class SubscriptionTriggerMatcher { private static final Logger ourLog = Logs.getSubscriptionTopicLog(); @@ -42,6 +43,7 @@ public class SubscriptionTriggerMatcher { private final String myResourceName; private final IBaseResource myResource; private final IFhirResourceDao myDao; + private final PreviousVersionReader myPreviousVersionReader; private final SystemRequestDetails mySrd; public SubscriptionTriggerMatcher(SubscriptionTopicSupport theSubscriptionTopicSupport, ResourceModifiedMessage theMsg, SubscriptionTopic.SubscriptionTopicResourceTriggerComponent theTrigger) { @@ -51,6 +53,7 @@ public class SubscriptionTriggerMatcher { myResourceName = myResource.fhirType(); myDao = mySubscriptionTopicSupport.getDaoRegistry().getResourceDao(myResourceName); myTrigger = theTrigger; + myPreviousVersionReader = new PreviousVersionReader(myDao); mySrd = new SystemRequestDetails(); } @@ -83,12 +86,10 @@ public class SubscriptionTriggerMatcher { if (previousCriteria != null) { if (myOperation == ResourceModifiedMessage.OperationTypeEnum.UPDATE || myOperation == ResourceModifiedMessage.OperationTypeEnum.DELETE) { - Long currentVersion = myResource.getIdElement().getVersionIdPartAsLong(); - if (currentVersion > 1) { - IIdType previousVersionId = myResource.getIdElement().withVersion("" + (currentVersion - 1)); - // WIP STR5 should we use the partition id from the resource? Ideally we should have a "previous version" service we can use for this - IBaseResource previousVersion = myDao.read(previousVersionId, new SystemRequestDetails()); - previousMatches = matchResource(previousVersion, previousCriteria); + + Optional oPreviousVersion = myPreviousVersionReader.readPreviousVersion(myResource); + if (oPreviousVersion.isPresent()) { + previousMatches = matchResource(oPreviousVersion.get(), previousCriteria); } else { ourLog.warn("Resource {} has a version of 1, which should not be the case for a create or delete operation", myResource.getIdElement().toUnqualifiedVersionless()); } diff --git a/hapi-fhir-jpaserver-subscription/src/test/java/ca/uhn/fhir/jpa/topic/SubscriptionTriggerMatcherTest.java b/hapi-fhir-jpaserver-subscription/src/test/java/ca/uhn/fhir/jpa/topic/SubscriptionTriggerMatcherTest.java index a376a64c389..ae47a026bb7 100644 --- a/hapi-fhir-jpaserver-subscription/src/test/java/ca/uhn/fhir/jpa/topic/SubscriptionTriggerMatcherTest.java +++ b/hapi-fhir-jpaserver-subscription/src/test/java/ca/uhn/fhir/jpa/topic/SubscriptionTriggerMatcherTest.java @@ -18,6 +18,7 @@ import org.mockito.junit.jupiter.MockitoExtension; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -119,7 +120,7 @@ class SubscriptionTriggerMatcherTest { IFhirResourceDao mockEncounterDao = mock(IFhirResourceDao.class); when(myDaoRegistry.getResourceDao("Encounter")).thenReturn(mockEncounterDao); Encounter encounterPreviousVersion = new Encounter(); - when(mockEncounterDao.read(any(), any())).thenReturn(encounterPreviousVersion); + when(mockEncounterDao.read(any(), any(), eq(false))).thenReturn(encounterPreviousVersion); when(mySearchParamMatcher.match(any(), any(), any())).thenReturn(InMemoryMatchResult.successfulMatch()); // run diff --git a/hapi-fhir-jpaserver-test-r5/src/test/java/ca/uhn/fhir/jpa/subscription/resthook/RestHookTestR5IT.java b/hapi-fhir-jpaserver-test-r5/src/test/java/ca/uhn/fhir/jpa/subscription/resthook/RestHookTestR5IT.java index cad0d9464e3..007bda87cb7 100644 --- a/hapi-fhir-jpaserver-test-r5/src/test/java/ca/uhn/fhir/jpa/subscription/resthook/RestHookTestR5IT.java +++ b/hapi-fhir-jpaserver-test-r5/src/test/java/ca/uhn/fhir/jpa/subscription/resthook/RestHookTestR5IT.java @@ -77,7 +77,6 @@ public class RestHookTestR5IT extends BaseSubscriptionsR5Test { createObservationSubscriptionTopic(OBS_CODE2); waitForRegisteredSubscriptionTopicCount(2); - // WIP STR5 will likely require matching TopicSubscription Subscription subscription1 = newTopicSubscription(SUBSCRIPTION_TOPIC_TEST_URL + OBS_CODE, Constants.CT_FHIR_XML_NEW); Subscription subscription = postSubscription(subscription1); diff --git a/hapi-fhir-jpaserver-test-r5/src/test/java/ca/uhn/fhir/storage/PreviousVersionReaderPartitionedTest.java b/hapi-fhir-jpaserver-test-r5/src/test/java/ca/uhn/fhir/storage/PreviousVersionReaderPartitionedTest.java new file mode 100644 index 00000000000..cd27ab666dc --- /dev/null +++ b/hapi-fhir-jpaserver-test-r5/src/test/java/ca/uhn/fhir/storage/PreviousVersionReaderPartitionedTest.java @@ -0,0 +1,147 @@ +package ca.uhn.fhir.storage; + +import ca.uhn.fhir.interceptor.model.RequestPartitionId; +import ca.uhn.fhir.jpa.api.dao.DaoRegistry; +import ca.uhn.fhir.jpa.dao.SimplePartitionTestHelper; +import ca.uhn.fhir.jpa.dao.r5.BaseJpaR5Test; +import ca.uhn.fhir.rest.api.server.SystemRequestDetails; +import org.hl7.fhir.r5.model.Enumerations; +import org.hl7.fhir.r5.model.IdType; +import org.hl7.fhir.r5.model.Patient; +import org.jetbrains.annotations.NotNull; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; + +import java.util.Optional; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +public class PreviousVersionReaderPartitionedTest extends BaseJpaR5Test { + PreviousVersionReader mySvc; + SystemRequestDetails mySrd; + @Autowired + DaoRegistry myDaoRegistry; + SimplePartitionTestHelper mySimplePartitionTestHelper; + + @BeforeEach + public void before() throws Exception { + super.before(); + + mySimplePartitionTestHelper = new SimplePartitionTestHelper(myPartitionSettings, myPartitionConfigSvc, myInterceptorRegistry); + mySimplePartitionTestHelper.beforeEach(null); + + mySvc = new PreviousVersionReader<>(myPatientDao); + mySrd = new SystemRequestDetails(); + RequestPartitionId part1 = RequestPartitionId.fromPartitionId(SimplePartitionTestHelper.TEST_PARTITION_ID); + mySrd.setRequestPartitionId(part1); + } + + @AfterEach + public void after() throws Exception { + mySimplePartitionTestHelper.afterEach(null); + } + + @Test + void readPreviousVersion() { + // setup + Patient patient = createMale(); + patient.setGender(Enumerations.AdministrativeGender.FEMALE); + myPatientDao.update(patient, mySrd); + assertEquals(Enumerations.AdministrativeGender.FEMALE, myPatientDao.read(patient.getIdElement(), mySrd).getGenderElement().getValue()); + + // execute + Optional oPreviousPatient = mySvc.readPreviousVersion(patient); + + // verify + assertTrue(oPreviousPatient.isPresent()); + Patient previousPatient = oPreviousPatient.get(); + assertEquals(Enumerations.AdministrativeGender.MALE, previousPatient.getGenderElement().getValue()); + } + + private Patient createMale() { + Patient male = new Patient(); + male.setGender(Enumerations.AdministrativeGender.MALE); + return (Patient) myPatientDao.create(male, mySrd).getResource(); + } + + @Test + void noPrevious() { + // setup + Patient patient = createMale(); + + // execute + Optional oPreviousPatient = mySvc.readPreviousVersion(patient); + + // verify + assertFalse(oPreviousPatient.isPresent()); + } + + @Test + void currentDeleted() { + // setup + Patient patient = createMale(); + IdType patientId = patient.getIdElement().toVersionless(); + myPatientDao.delete(patientId, mySrd); + + Patient currentDeletedVersion = myPatientDao.read(patientId, mySrd, true); + + // execute + Optional oPreviousPatient = mySvc.readPreviousVersion(currentDeletedVersion); + + // verify + assertTrue(oPreviousPatient.isPresent()); + Patient previousPatient = oPreviousPatient.get(); + assertEquals(Enumerations.AdministrativeGender.MALE, previousPatient.getGenderElement().getValue()); + } + + @Test + void previousDeleted() { + // setup + Patient latestUndeletedVersion = setupPreviousDeletedResource(); + + // execute + Optional oDeletedPatient = mySvc.readPreviousVersion(latestUndeletedVersion); + assertFalse(oDeletedPatient.isPresent()); + } + + @Test + void previousDeletedDeletedOk() { + // setup + Patient latestUndeletedVersion = setupPreviousDeletedResource(); + + // execute + Optional oPreviousPatient = mySvc.readPreviousVersion(latestUndeletedVersion, true); + + // verify + assertTrue(oPreviousPatient.isPresent()); + Patient previousPatient = oPreviousPatient.get(); + assertTrue(previousPatient.isDeleted()); + } + + @NotNull + private Patient setupPreviousDeletedResource() { + Patient patient = createMale(); + assertEquals(1L, patient.getIdElement().getVersionIdPartAsLong()); + IdType patientId = patient.getIdElement().toVersionless(); + myPatientDao.delete(patientId, mySrd); + + Patient currentDeletedVersion = myPatientDao.read(patientId, mySrd, true); + assertEquals(2L, currentDeletedVersion.getIdElement().getVersionIdPartAsLong()); + + currentDeletedVersion.setGender(Enumerations.AdministrativeGender.FEMALE); + currentDeletedVersion.setId(currentDeletedVersion.getIdElement().toVersionless()); + myPatientDao.update(currentDeletedVersion, mySrd); + + Patient latestUndeletedVersion = myPatientDao.read(patientId, mySrd); + assertEquals(3L, latestUndeletedVersion.getIdElement().getVersionIdPartAsLong()); + + assertFalse(latestUndeletedVersion.isDeleted()); + assertEquals(Enumerations.AdministrativeGender.FEMALE, latestUndeletedVersion.getGenderElement().getValue()); + return latestUndeletedVersion; + } + +} diff --git a/hapi-fhir-jpaserver-test-utilities/src/main/java/ca/uhn/fhir/jpa/dao/SimplePartitionTestHelper.java b/hapi-fhir-jpaserver-test-utilities/src/main/java/ca/uhn/fhir/jpa/dao/SimplePartitionTestHelper.java new file mode 100644 index 00000000000..6afa6a06422 --- /dev/null +++ b/hapi-fhir-jpaserver-test-utilities/src/main/java/ca/uhn/fhir/jpa/dao/SimplePartitionTestHelper.java @@ -0,0 +1,39 @@ +package ca.uhn.fhir.jpa.dao; + +import ca.uhn.fhir.interceptor.api.IInterceptorService; +import ca.uhn.fhir.jpa.entity.PartitionEntity; +import ca.uhn.fhir.jpa.interceptor.ex.PartitionInterceptorReadAllPartitions; +import ca.uhn.fhir.jpa.model.config.PartitionSettings; +import ca.uhn.fhir.jpa.partition.IPartitionLookupSvc; +import org.junit.jupiter.api.extension.AfterEachCallback; +import org.junit.jupiter.api.extension.BeforeEachCallback; +import org.junit.jupiter.api.extension.ExtensionContext; + +public class SimplePartitionTestHelper implements BeforeEachCallback, AfterEachCallback { + public static final int TEST_PARTITION_ID = 17; + private static final String TEST_PARTITION_NAME = "test-partition-17"; + private final PartitionSettings myPartitionSettings; + private final IPartitionLookupSvc myPartitionConfigSvc; + private final IInterceptorService myInterceptorRegistry; + private final PartitionInterceptorReadAllPartitions myInterceptor = new PartitionInterceptorReadAllPartitions(); + + public SimplePartitionTestHelper(PartitionSettings thePartitionSettings, IPartitionLookupSvc thePartitionConfigSvc, IInterceptorService theInterceptorRegistry) { + myPartitionSettings = thePartitionSettings; + myPartitionConfigSvc = thePartitionConfigSvc; + myInterceptorRegistry = theInterceptorRegistry; + } + + @Override + public void beforeEach(ExtensionContext context) throws Exception { + myPartitionSettings.setPartitioningEnabled(true); + myPartitionConfigSvc.createPartition(new PartitionEntity().setId(TEST_PARTITION_ID).setName(TEST_PARTITION_NAME), null); + myInterceptorRegistry.registerInterceptor(myInterceptor); + } + + @Override + public void afterEach(ExtensionContext context) throws Exception { + myInterceptorRegistry.unregisterInterceptor(myInterceptor); + myPartitionConfigSvc.deletePartition(TEST_PARTITION_ID); + myPartitionSettings.setPartitioningEnabled(false); + } +} diff --git a/hapi-fhir-storage/src/main/java/ca/uhn/fhir/storage/PreviousVersionReader.java b/hapi-fhir-storage/src/main/java/ca/uhn/fhir/storage/PreviousVersionReader.java new file mode 100644 index 00000000000..acf7db9c306 --- /dev/null +++ b/hapi-fhir-storage/src/main/java/ca/uhn/fhir/storage/PreviousVersionReader.java @@ -0,0 +1,36 @@ +package ca.uhn.fhir.storage; + +import ca.uhn.fhir.jpa.api.dao.IFhirResourceDao; +import ca.uhn.fhir.rest.api.server.SystemRequestDetails; +import ca.uhn.fhir.rest.server.exceptions.ResourceGoneException; +import org.hl7.fhir.instance.model.api.IBaseResource; +import org.hl7.fhir.instance.model.api.IIdType; + +import java.util.Optional; + +public class PreviousVersionReader { + private final IFhirResourceDao myDao; + + public PreviousVersionReader(IFhirResourceDao theDao) { + myDao = theDao; + } + + public Optional readPreviousVersion(T theResource) { + return readPreviousVersion(theResource, false); + } + + public Optional readPreviousVersion(T theResource, boolean theDeletedOk) { + Long currentVersion = theResource.getIdElement().getVersionIdPartAsLong(); + if (currentVersion == null || currentVersion == 1L) { + return Optional.empty(); + } + long previousVersion = currentVersion - 1L; + IIdType previousId = theResource.getIdElement().withVersion(Long.toString(previousVersion)); + try { + return Optional.ofNullable(myDao.read(previousId, new SystemRequestDetails(), theDeletedOk)); + } catch (ResourceGoneException e) { + // This will only happen in the case where theDeleteOk = false + return Optional.empty(); + } + } +}