Add PreviousVersion service with tests (#4902)

* started writing PreviousVersionReader

* started writing PreviousVersionReader

* moar tests

* add partitioning to the test

* switch subscription to use previous version reader

---------

Co-authored-by: Ken Stevens <ken@smilecdr.com>

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:

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<IBaseResource> 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());
 				}

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

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);

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<Patient> 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<Patient> 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<Patient> 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<Patient> 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<Patient> oDeletedPatient = mySvc.readPreviousVersion(latestUndeletedVersion);
+		assertFalse(oDeletedPatient.isPresent());
+	}
+
+	@Test
+	void previousDeletedDeletedOk() {
+		// setup
+		Patient latestUndeletedVersion = setupPreviousDeletedResource();
+
+		// execute
+		Optional<Patient> 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;
+	}
+
+}

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);
+	}
+}

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<T extends IBaseResource> {
+	private final IFhirResourceDao<T> myDao;
+
+	public PreviousVersionReader(IFhirResourceDao<T> theDao) {
+		myDao = theDao;
+	}
+
+	public Optional<T> readPreviousVersion(T theResource) {
+		return readPreviousVersion(theResource, false);
+	}
+
+	public Optional<T> 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();
+		}
+	}
+}