From 2d924058a0c92abf7fe63fee51581254b2631e32 Mon Sep 17 00:00:00 2001 From: Emre Dincturk Date: Fri, 3 Jan 2025 15:17:46 -0500 Subject: [PATCH 1/3] create merge provenance resource --- .../provider/merge/ResourceMergeService.java | 18 +- .../merge/ResourceMergeServiceTest.java | 156 ++++++++++++------ .../jpa/provider/merge/MergeBatchTest.java | 7 + .../jpa/provider/r4/PatientMergeR4Test.java | 11 +- .../ReplaceReferencesTestHelper.java | 58 ++++++- .../jobs/merge/MergeResourceHelper.java | 64 ++++++- .../merge/MergeUpdateTaskReducerStep.java | 12 +- 7 files changed, 254 insertions(+), 72 deletions(-) diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/provider/merge/ResourceMergeService.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/provider/merge/ResourceMergeService.java index 1c445d7bdde..b968ea5217a 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/provider/merge/ResourceMergeService.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/provider/merge/ResourceMergeService.java @@ -39,10 +39,13 @@ import org.hl7.fhir.instance.model.api.IBase; import org.hl7.fhir.instance.model.api.IBaseOperationOutcome; import org.hl7.fhir.r4.model.Patient; +import org.hl7.fhir.r4.model.Provenance; import org.hl7.fhir.r4.model.Task; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import java.util.Date; + import static ca.uhn.fhir.batch2.jobs.merge.MergeAppCtx.JOB_MERGE; import static ca.uhn.fhir.rest.api.Constants.STATUS_HTTP_200_OK; import static ca.uhn.fhir.rest.api.Constants.STATUS_HTTP_202_ACCEPTED; @@ -84,13 +87,18 @@ public ResourceMergeService( myBatch2TaskHelper = theBatch2TaskHelper; myFhirContext = myPatientDao.getContext(); myHapiTransactionService = theHapiTransactionService; - myMergeResourceHelper = new MergeResourceHelper(myPatientDao); + IFhirResourceDao provenanceDao = theDaoRegistry.getResourceDao(Provenance.class); + myMergeResourceHelper = new MergeResourceHelper(myPatientDao, provenanceDao); myMergeValidationService = new MergeValidationService(myFhirContext, theDaoRegistry); } /** - * Perform the $merge operation. If the number of resources to be changed exceeds the provided batch size, - * then switch to async mode. See the Patient $merge spec + * Perform the $merge operation. Operation can be performed synchronously or asynchronously depending on + * the prefer-async request header. + * If the operation is requested to be performed synchronously and the number of + * resources to be changed exceeds the provided batch size, + * and error is returned indicating that operation needs to be performed asynchronously. See the + * Patient $merge spec * for details on what the difference is between synchronous and asynchronous mode. * * @param theMergeOperationParameters the merge operation parameters @@ -211,6 +219,7 @@ private void doMergeSync( MergeOperationOutcome theMergeOutcome, RequestPartitionId partitionId) { + Date startTime = new Date(); ReplaceReferencesRequest replaceReferencesRequest = new ReplaceReferencesRequest( theSourceResource.getIdElement(), theTargetResource.getIdElement(), @@ -225,7 +234,8 @@ private void doMergeSync( theTargetResource, (Patient) theMergeOperationParameters.getResultResource(), theMergeOperationParameters.getDeleteSource(), - theRequestDetails); + theRequestDetails, + startTime); theMergeOutcome.setUpdatedTargetResource(updatedTarget); String detailsText = "Merge operation completed successfully."; diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/merge/ResourceMergeServiceTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/merge/ResourceMergeServiceTest.java index 093f7c466d3..9131823ea30 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/merge/ResourceMergeServiceTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/merge/ResourceMergeServiceTest.java @@ -23,11 +23,13 @@ import ca.uhn.fhir.rest.server.exceptions.ResourceNotFoundException; import ca.uhn.fhir.util.CanonicalIdentifier; import org.hl7.fhir.instance.model.api.IBaseResource; +import org.hl7.fhir.r4.model.Coding; import org.hl7.fhir.r4.model.IdType; import org.hl7.fhir.r4.model.Identifier; import org.hl7.fhir.r4.model.OperationOutcome; import org.hl7.fhir.r4.model.Parameters; import org.hl7.fhir.r4.model.Patient; +import org.hl7.fhir.r4.model.Provenance; import org.hl7.fhir.r4.model.Reference; import org.hl7.fhir.r4.model.Task; import org.junit.jupiter.api.BeforeEach; @@ -42,6 +44,7 @@ import org.mockito.stubbing.OngoingStubbing; import org.testcontainers.shaded.org.checkerframework.checker.nullness.qual.Nullable; +import java.time.Instant; import java.util.Collections; import java.util.List; @@ -89,6 +92,9 @@ public class ResourceMergeServiceTest { @Mock IFhirResourceDaoPatient myTaskDaoMock; + @Mock + IFhirResourceDaoPatient myProvenanceDaoMock; + @Mock IReplaceReferencesSvc myReplaceReferencesSvcMock; @@ -125,6 +131,7 @@ public class ResourceMergeServiceTest { void setup() { when(myDaoRegistryMock.getResourceDao(eq(Patient.class))).thenReturn(myPatientDaoMock); when(myDaoRegistryMock.getResourceDao(eq(Task.class))).thenReturn(myTaskDaoMock); + when(myDaoRegistryMock.getResourceDao(eq(Provenance.class))).thenReturn(myProvenanceDaoMock); when(myPatientDaoMock.getContext()).thenReturn(myFhirContext); myResourceMergeService = new ResourceMergeService( myStorageSettingsMock, @@ -154,8 +161,8 @@ void testMerge_WithoutResultResource_Success() { targetPatient.addIdentifier(new Identifier().setSystem("sysTarget").setValue("valT1")); setupDaoMockForSuccessfulRead(sourcePatient); setupDaoMockForSuccessfulRead(targetPatient); - setupDaoMockForSuccessfulSourcePatientUpdate(sourcePatient, new Patient()); - Patient patientReturnedFromDaoAfterTargetUpdate = new Patient(); + setupDaoMockForSuccessfulSourcePatientUpdate(sourcePatient, createPatient(SOURCE_PATIENT_TEST_ID_WITH_VERSION_2)); + Patient patientReturnedFromDaoAfterTargetUpdate = createPatient(TARGET_PATIENT_TEST_ID_WITH_VERSION_2); setupDaoMockForSuccessfulTargetPatientUpdate(targetPatient, patientReturnedFromDaoAfterTargetUpdate); setupTransactionServiceMock(); setupReplaceReferencesForSuccessForSync(); @@ -173,7 +180,8 @@ void testMerge_WithoutResultResource_Success() { new Identifier().setSystem("sysSource").setValue("valS1").setUse(Identifier.IdentifierUse.OLD), new Identifier().setSystem("sysSource").setValue("valS2").setUse(Identifier.IdentifierUse.OLD)); verifyUpdatedTargetPatient(true, expectedIdentifiers); - verifyNoMoreInteractions(myPatientDaoMock); + verifyProvenanceCreated(false); + verifyNoMoreInteractions(myPatientDaoMock, myTaskDaoMock, myProvenanceDaoMock, myBatch2TaskHelperMock); } @@ -188,8 +196,8 @@ void testMerge_WithoutResultResource_TargetSetToActiveExplicitly_Success() { targetPatient.setActive(true); setupDaoMockForSuccessfulRead(sourcePatient); setupDaoMockForSuccessfulRead(targetPatient); - setupDaoMockForSuccessfulSourcePatientUpdate(sourcePatient, new Patient()); - Patient patientReturnedFromDaoAfterTargetUpdate = new Patient(); + setupDaoMockForSuccessfulSourcePatientUpdate(sourcePatient, createPatient(SOURCE_PATIENT_TEST_ID_WITH_VERSION_2)); + Patient patientReturnedFromDaoAfterTargetUpdate = createPatient(TARGET_PATIENT_TEST_ID_WITH_VERSION_2); setupDaoMockForSuccessfulTargetPatientUpdate(targetPatient, patientReturnedFromDaoAfterTargetUpdate); setupTransactionServiceMock(); setupReplaceReferencesForSuccessForSync(); @@ -201,7 +209,8 @@ void testMerge_WithoutResultResource_TargetSetToActiveExplicitly_Success() { verifySuccessfulOutcomeForSync(mergeOutcome, patientReturnedFromDaoAfterTargetUpdate); verifyUpdatedSourcePatient(); verifyUpdatedTargetPatient(true, Collections.emptyList()); - verifyNoMoreInteractions(myPatientDaoMock); + verifyProvenanceCreated(false); + verifyNoMoreInteractions(myPatientDaoMock, myTaskDaoMock, myProvenanceDaoMock, myBatch2TaskHelperMock); } @Test @@ -222,8 +231,8 @@ void testMerge_WithResultResource_Success() { setupDaoMockForSuccessfulRead(sourcePatient); setupDaoMockForSuccessfulRead(targetPatient); - setupDaoMockForSuccessfulSourcePatientUpdate(sourcePatient, new Patient()); - Patient patientToBeReturnedFromDaoAfterTargetUpdate = new Patient(); + setupDaoMockForSuccessfulSourcePatientUpdate(sourcePatient, createPatient(SOURCE_PATIENT_TEST_ID_WITH_VERSION_2)); + Patient patientToBeReturnedFromDaoAfterTargetUpdate = createPatient(TARGET_PATIENT_TEST_ID_WITH_VERSION_2); setupDaoMockForSuccessfulTargetPatientUpdate(resultPatient, patientToBeReturnedFromDaoAfterTargetUpdate); setupTransactionServiceMock(); setupReplaceReferencesForSuccessForSync(); @@ -235,7 +244,8 @@ void testMerge_WithResultResource_Success() { verifySuccessfulOutcomeForSync(mergeOutcome, patientToBeReturnedFromDaoAfterTargetUpdate); verifyUpdatedSourcePatient(); verifyUpdatedTargetPatient(true, Collections.emptyList()); - verifyNoMoreInteractions(myPatientDaoMock); + verifyProvenanceCreated(false); + verifyNoMoreInteractions(myPatientDaoMock, myTaskDaoMock, myProvenanceDaoMock, myBatch2TaskHelperMock); } @@ -259,8 +269,8 @@ void testMerge_WithResultResource_ResultHasAllTargetIdentifiers_Success() { setupDaoMockForSuccessfulRead(sourcePatient); setupDaoMockSearchForIdentifiers(List.of(List.of(targetPatient))); - setupDaoMockForSuccessfulSourcePatientUpdate(sourcePatient, new Patient()); - Patient patientToBeReturnedFromDaoAfterTargetUpdate = new Patient(); + setupDaoMockForSuccessfulSourcePatientUpdate(sourcePatient, createPatient(SOURCE_PATIENT_TEST_ID_WITH_VERSION_2)); + Patient patientToBeReturnedFromDaoAfterTargetUpdate = createPatient(TARGET_PATIENT_TEST_ID_WITH_VERSION_2); setupDaoMockForSuccessfulTargetPatientUpdate(resultPatient, patientToBeReturnedFromDaoAfterTargetUpdate); setupTransactionServiceMock(); setupReplaceReferencesForSuccessForSync(); @@ -277,7 +287,8 @@ void testMerge_WithResultResource_ResultHasAllTargetIdentifiers_Success() { new Identifier().setSystem("sys").setValue("val2") ); verifyUpdatedTargetPatient(true, expectedIdentifiers); - verifyNoMoreInteractions(myPatientDaoMock); + verifyProvenanceCreated(false); + verifyNoMoreInteractions(myPatientDaoMock, myTaskDaoMock, myProvenanceDaoMock, myBatch2TaskHelperMock); } @Test @@ -293,7 +304,7 @@ void testMerge_WithDeleteSourceTrue_Success() { setupDaoMockForSuccessfulRead(targetPatient); when(myPatientDaoMock.delete(new IdType(SOURCE_PATIENT_TEST_ID_WITH_VERSION_1), myRequestDetailsMock)).thenReturn(new DaoMethodOutcome()); - Patient patientToBeReturnedFromDaoAfterTargetUpdate = new Patient(); + Patient patientToBeReturnedFromDaoAfterTargetUpdate = createPatient(TARGET_PATIENT_TEST_ID_WITH_VERSION_2); setupDaoMockForSuccessfulTargetPatientUpdate(targetPatient, patientToBeReturnedFromDaoAfterTargetUpdate); setupTransactionServiceMock(); setupReplaceReferencesForSuccessForSync(); @@ -305,7 +316,8 @@ void testMerge_WithDeleteSourceTrue_Success() { // Then verifySuccessfulOutcomeForSync(mergeOutcome, patientToBeReturnedFromDaoAfterTargetUpdate); verifyUpdatedTargetPatient(false, Collections.emptyList()); - verifyNoMoreInteractions(myPatientDaoMock); + verifyProvenanceCreated(true); + verifyNoMoreInteractions(myPatientDaoMock, myTaskDaoMock, myProvenanceDaoMock, myBatch2TaskHelperMock); } @@ -324,7 +336,7 @@ void testMerge_WithDeleteSourceTrue_And_WithResultResource_Success() { setupDaoMockForSuccessfulRead(targetPatient); when(myPatientDaoMock.delete(new IdType(SOURCE_PATIENT_TEST_ID_WITH_VERSION_1), myRequestDetailsMock)).thenReturn(new DaoMethodOutcome()); - Patient patientToBeReturnedFromDaoAfterTargetUpdate = new Patient(); + Patient patientToBeReturnedFromDaoAfterTargetUpdate = createPatient(TARGET_PATIENT_TEST_ID_WITH_VERSION_2); setupDaoMockForSuccessfulTargetPatientUpdate(resultPatient, patientToBeReturnedFromDaoAfterTargetUpdate); setupTransactionServiceMock(); setupReplaceReferencesForSuccessForSync(); @@ -336,7 +348,8 @@ void testMerge_WithDeleteSourceTrue_And_WithResultResource_Success() { // Then verifySuccessfulOutcomeForSync(mergeOutcome, patientToBeReturnedFromDaoAfterTargetUpdate); verifyUpdatedTargetPatient(false, Collections.emptyList()); - verifyNoMoreInteractions(myPatientDaoMock); + verifyProvenanceCreated(true); + verifyNoMoreInteractions(myPatientDaoMock, myTaskDaoMock, myProvenanceDaoMock, myBatch2TaskHelperMock); } @Test @@ -367,7 +380,7 @@ void testMerge_WithPreviewTrue_Success() { assertThat(issue.getDetails().getText()).contains("Preview only merge operation - no issues detected"); assertThat(issue.getDiagnostics()).contains("Merge would update 12 resources"); - verifyNoMoreInteractions(myPatientDaoMock); + verifyNoMoreInteractions(myPatientDaoMock, myTaskDaoMock, myProvenanceDaoMock, myBatch2TaskHelperMock); } @Test @@ -380,8 +393,8 @@ void testMerge_ResolvesResourcesByReferenceThatHasVersions_CurrentResourceVersio Patient targetPatient = createPatient(TARGET_PATIENT_TEST_ID_WITH_VERSION_2); setupDaoMockForSuccessfulRead(sourcePatient); setupDaoMockForSuccessfulRead(targetPatient); - setupDaoMockForSuccessfulSourcePatientUpdate(sourcePatient, new Patient()); - Patient patientToBeReturnedFromDaoAfterTargetUpdate = new Patient(); + setupDaoMockForSuccessfulSourcePatientUpdate(sourcePatient, createPatient(SOURCE_PATIENT_TEST_ID_WITH_VERSION_2)); + Patient patientToBeReturnedFromDaoAfterTargetUpdate = createPatient(TARGET_PATIENT_TEST_ID_WITH_VERSION_2); setupDaoMockForSuccessfulTargetPatientUpdate(targetPatient, patientToBeReturnedFromDaoAfterTargetUpdate); setupTransactionServiceMock(); setupReplaceReferencesForSuccessForSync(); @@ -393,7 +406,8 @@ void testMerge_ResolvesResourcesByReferenceThatHasVersions_CurrentResourceVersio verifySuccessfulOutcomeForSync(mergeOutcome, patientToBeReturnedFromDaoAfterTargetUpdate); verifyUpdatedSourcePatient(); verifyUpdatedTargetPatient(true, Collections.emptyList()); - verifyNoMoreInteractions(myPatientDaoMock); + verifyProvenanceCreated(false); + verifyNoMoreInteractions(myPatientDaoMock, myTaskDaoMock, myProvenanceDaoMock, myBatch2TaskHelperMock); } @@ -431,10 +445,10 @@ void testMerge_AsyncBecauseOfPreferHeader_Success(boolean theWithResultResource, verifySuccessfulOutcomeForAsync(mergeOutcome, task); verifyBatch2JobTaskHelperMockInvocation(resultResource, theWithDeleteSource); - - verifyNoMoreInteractions(myPatientDaoMock); + verifyNoMoreInteractions(myPatientDaoMock, myTaskDaoMock, myProvenanceDaoMock, myBatch2TaskHelperMock); } + // ERROR CASES @ParameterizedTest @CsvSource({ "true, false", @@ -442,8 +456,8 @@ void testMerge_AsyncBecauseOfPreferHeader_Success(boolean theWithResultResource, "true, true", "false, false" }) - void testMerge_AsyncBecauseOfLargeNumberOfRefs_Success(boolean theWithResultResource, - boolean theWithDeleteSource) { + void testMerge_AsyncBecauseOfLargeNumberOfRefs_PreconditionFailed(boolean theWithResultResource, + boolean theWithDeleteSource) { // Given BaseMergeOperationInputParameters mergeOperationParameters = new PatientMergeOperationInputParameters(PAGE_SIZE); mergeOperationParameters.setSourceResource(new Reference(SOURCE_PATIENT_TEST_ID)); @@ -466,7 +480,7 @@ void testMerge_AsyncBecauseOfLargeNumberOfRefs_Success(boolean theWithResultReso MergeOperationOutcome mergeOutcome = myResourceMergeService.merge(mergeOperationParameters, myRequestDetailsMock); verifyFailedOutcome(mergeOutcome); - verifyNoMoreInteractions(myPatientDaoMock); + verifyNoMoreInteractions(myPatientDaoMock, myTaskDaoMock, myProvenanceDaoMock, myBatch2TaskHelperMock); } private void verifyFailedOutcome(MergeOperationOutcome theMergeOutcome) { @@ -476,7 +490,6 @@ private void verifyFailedOutcome(MergeOperationOutcome theMergeOutcome) { assertThat(operationOutcome.getIssueFirstRep().getDiagnostics()).isEqualTo(PRECONDITION_FAILED_MESSAGE); } - // ERROR CASES @ParameterizedTest @ValueSource(booleans = {true, false}) void testMerge_UnhandledServerResponseExceptionThrown_UsesStatusCodeOfTheException(boolean thePreview) { @@ -501,7 +514,7 @@ void testMerge_UnhandledServerResponseExceptionThrown_UsesStatusCodeOfTheExcepti assertThat(issue.getDiagnostics()).contains("this is the exception message"); assertThat(issue.getCode().toCode()).isEqualTo("exception"); - verifyNoMoreInteractions(myPatientDaoMock); + verifyNoMoreInteractions(myPatientDaoMock, myTaskDaoMock, myProvenanceDaoMock, myBatch2TaskHelperMock); } @ParameterizedTest @@ -528,7 +541,7 @@ void testMerge_UnhandledExceptionThrown_Uses500StatusCode(boolean thePreview) { assertThat(issue.getDiagnostics()).contains("this is the exception message"); assertThat(issue.getCode().toCode()).isEqualTo("exception"); - verifyNoMoreInteractions(myPatientDaoMock); + verifyNoMoreInteractions(myPatientDaoMock, myTaskDaoMock, myProvenanceDaoMock, myBatch2TaskHelperMock); } @ParameterizedTest @@ -552,7 +565,7 @@ void testMerge_ValidatesInputParameters_MissingSourcePatientParams_ReturnsErrorW assertThat(issue.getDiagnostics()).contains(MISSING_SOURCE_PARAMS_MSG); assertThat(issue.getCode().toCode()).isEqualTo("required"); - verifyNoMoreInteractions(myPatientDaoMock); + verifyNoMoreInteractions(myPatientDaoMock, myTaskDaoMock, myProvenanceDaoMock, myBatch2TaskHelperMock); } @@ -578,7 +591,7 @@ void testMerge_ValidatesInputParameters_MissingTargetPatientParams_ReturnsErrorW assertThat(issue.getDiagnostics()).contains(MISSING_TARGET_PARAMS_MSG); assertThat(issue.getCode().toCode()).isEqualTo("required"); - verifyNoMoreInteractions(myPatientDaoMock); + verifyNoMoreInteractions(myPatientDaoMock, myTaskDaoMock, myProvenanceDaoMock, myBatch2TaskHelperMock); } @ParameterizedTest @@ -605,7 +618,7 @@ void testMerge_ValidatesInputParameters_MissingBothSourceAndTargetPatientParams_ assertThat(issue2.getDiagnostics()).contains(MISSING_TARGET_PARAMS_MSG); assertThat(issue2.getCode().toCode()).isEqualTo("required"); - verifyNoMoreInteractions(myPatientDaoMock); + verifyNoMoreInteractions(myPatientDaoMock, myTaskDaoMock, myProvenanceDaoMock, myBatch2TaskHelperMock); } @ParameterizedTest @@ -632,7 +645,7 @@ void testMerge_ValidatesInputParameters_BothSourceResourceAndSourceIdentifierPar assertThat(issue.getCode().toCode()).isEqualTo("required"); - verifyNoMoreInteractions(myPatientDaoMock); + verifyNoMoreInteractions(myPatientDaoMock, myTaskDaoMock, myProvenanceDaoMock, myBatch2TaskHelperMock); } @@ -659,7 +672,7 @@ void testMerge_ValidatesInputParameters_BothTargetResourceAndTargetIdentifiersPa assertThat(issue.getDiagnostics()).contains(BOTH_TARGET_PARAMS_PROVIDED_MSG); assertThat(issue.getCode().toCode()).isEqualTo("required"); - verifyNoMoreInteractions(myPatientDaoMock); + verifyNoMoreInteractions(myPatientDaoMock, myTaskDaoMock, myProvenanceDaoMock, myBatch2TaskHelperMock); } @@ -686,7 +699,7 @@ void testMerge_ValidatesInputParameters_SourceResourceParamHasNoReferenceElement assertThat(issue.getDiagnostics()).contains("Reference specified in 'source-patient' parameter does not have a reference element."); assertThat(issue.getCode().toCode()).isEqualTo("required"); - verifyNoMoreInteractions(myPatientDaoMock); + verifyNoMoreInteractions(myPatientDaoMock, myTaskDaoMock, myProvenanceDaoMock, myBatch2TaskHelperMock); } @@ -714,7 +727,7 @@ void testMerge_ValidatesInputParameters_TargetResourceParamHasNoReferenceElement "a reference element."); assertThat(issue.getCode().toCode()).isEqualTo("required"); - verifyNoMoreInteractions(myPatientDaoMock); + verifyNoMoreInteractions(myPatientDaoMock, myTaskDaoMock, myProvenanceDaoMock, myBatch2TaskHelperMock); } @ParameterizedTest @@ -740,7 +753,7 @@ void testMerge_ResolvesSourceResourceByReference_ResourceNotFound_ReturnsErrorWi assertThat(issue.getDiagnostics()).contains("Resource not found for the reference specified in 'source-patient'"); assertThat(issue.getCode().toCode()).isEqualTo("not-found"); - verifyNoMoreInteractions(myPatientDaoMock); + verifyNoMoreInteractions(myPatientDaoMock, myTaskDaoMock, myProvenanceDaoMock, myBatch2TaskHelperMock); } @ParameterizedTest @@ -768,7 +781,7 @@ void testMerge_ResolvesTargetResourceByReference_ResourceNotFound_ReturnsErrorWi assertThat(issue.getDiagnostics()).contains("Resource not found for the reference specified in 'target-patient'"); assertThat(issue.getCode().toCode()).isEqualTo("not-found"); - verifyNoMoreInteractions(myPatientDaoMock); + verifyNoMoreInteractions(myPatientDaoMock, myTaskDaoMock, myProvenanceDaoMock, myBatch2TaskHelperMock); } @ParameterizedTest @@ -798,7 +811,7 @@ void testMerge_ResolvesSourceResourceByIdentifiers_NoMatchFound_ReturnsErrorWith assertThat(issue.getDiagnostics()).contains("No resources found matching the identifier(s) specified in 'source-patient-identifier'"); assertThat(issue.getCode().toCode()).isEqualTo("not-found"); - verifyNoMoreInteractions(myPatientDaoMock); + verifyNoMoreInteractions(myPatientDaoMock, myTaskDaoMock, myProvenanceDaoMock, myBatch2TaskHelperMock); } @@ -832,7 +845,7 @@ void testMerge_ResolvesSourceResourceByIdentifiers_MultipleMatchesFound_ReturnsE " 'source-patient-identifier'"); assertThat(issue.getCode().toCode()).isEqualTo("multiple-matches"); - verifyNoMoreInteractions(myPatientDaoMock); + verifyNoMoreInteractions(myPatientDaoMock, myTaskDaoMock, myProvenanceDaoMock, myBatch2TaskHelperMock); } @@ -866,7 +879,7 @@ void testMerge_ResolvesTargetResourceByIdentifiers_NoMatchFound_ReturnsErrorWith "'target-patient-identifier'"); assertThat(issue.getCode().toCode()).isEqualTo("not-found"); - verifyNoMoreInteractions(myPatientDaoMock); + verifyNoMoreInteractions(myPatientDaoMock, myTaskDaoMock, myProvenanceDaoMock, myBatch2TaskHelperMock); } @ParameterizedTest @@ -901,7 +914,7 @@ void testMerge_ResolvesTargetResourceByIdentifiers_MultipleMatchesFound_ReturnsE assertThat(issue.getDiagnostics()).contains("Multiple resources found matching the identifier(s) specified in 'target-patient-identifier'"); assertThat(issue.getCode().toCode()).isEqualTo("multiple-matches"); - verifyNoMoreInteractions(myPatientDaoMock); + verifyNoMoreInteractions(myPatientDaoMock, myTaskDaoMock, myProvenanceDaoMock, myBatch2TaskHelperMock); } @ParameterizedTest @@ -929,7 +942,7 @@ void testMerge_ResolvesSourceResourceByReferenceThatHasVersion_CurrentResourceVe assertThat(issue.getDiagnostics()).contains("The reference in 'source-patient' parameter has a version specified, but it is not the latest version of the resource"); assertThat(issue.getCode().toCode()).isEqualTo("conflict"); - verifyNoMoreInteractions(myPatientDaoMock); + verifyNoMoreInteractions(myPatientDaoMock, myTaskDaoMock, myProvenanceDaoMock, myBatch2TaskHelperMock); } @ParameterizedTest @@ -960,7 +973,7 @@ void testMerge_ResolvesTargetResourceByReferenceThatHasVersion_CurrentResourceVe "specified, but it is not the latest version of the resource"); assertThat(issue.getCode().toCode()).isEqualTo("conflict"); - verifyNoMoreInteractions(myPatientDaoMock); + verifyNoMoreInteractions(myPatientDaoMock, myTaskDaoMock, myProvenanceDaoMock, myBatch2TaskHelperMock); } @ParameterizedTest @@ -989,8 +1002,7 @@ void testMerge_SourceAndTargetResolvesToSameResource_ReturnsErrorWith422Status(b assertThat(issue.getSeverity()).isEqualTo(OperationOutcome.IssueSeverity.ERROR); assertThat(issue.getDiagnostics()).contains("Source and target resources are the same resource."); - //TODO: enable this - //verifyNoMoreInteractions(myDaoMock); + verifyNoMoreInteractions(myPatientDaoMock, myTaskDaoMock, myProvenanceDaoMock, myBatch2TaskHelperMock); } @ParameterizedTest @@ -1019,7 +1031,7 @@ void testMerge_TargetResourceIsInactive_ReturnsErrorWith422Status(boolean thePre assertThat(issue.getSeverity()).isEqualTo(OperationOutcome.IssueSeverity.ERROR); assertThat(issue.getDiagnostics()).contains("Target resource is not active, it must be active to be the target of a merge operation"); - verifyNoMoreInteractions(myPatientDaoMock); + verifyNoMoreInteractions(myPatientDaoMock, myTaskDaoMock, myProvenanceDaoMock, myBatch2TaskHelperMock); } @ParameterizedTest @@ -1050,7 +1062,7 @@ void testMerge_TargetResourceWasPreviouslyReplacedByAnotherResource_ReturnsError "reference 'Patient/replacing-res-id', it is " + "not a suitable target for merging."); - verifyNoMoreInteractions(myPatientDaoMock); + verifyNoMoreInteractions(myPatientDaoMock, myTaskDaoMock, myProvenanceDaoMock, myBatch2TaskHelperMock); } @ParameterizedTest @@ -1080,7 +1092,7 @@ void testMerge_SourceResourceWasPreviouslyReplacedByAnotherResource_ReturnsError assertThat(issue.getDiagnostics()).contains("Source resource was previously replaced by a resource with " + "reference 'Patient/replacing-res-id', it is not a suitable source for merging."); - verifyNoMoreInteractions(myPatientDaoMock); + verifyNoMoreInteractions(myPatientDaoMock, myTaskDaoMock, myProvenanceDaoMock, myBatch2TaskHelperMock); } @ParameterizedTest @@ -1114,7 +1126,7 @@ void testMerge_ValidatesResultResource_ResultResourceHasDifferentIdThanTargetRes "as the actual" + " resolved target resource 'Patient/not-the-target-id'. The actual resolved target resource's id is: '" + TARGET_PATIENT_TEST_ID +"'"); - verifyNoMoreInteractions(myPatientDaoMock); + verifyNoMoreInteractions(myPatientDaoMock, myTaskDaoMock, myProvenanceDaoMock, myBatch2TaskHelperMock); } @@ -1153,7 +1165,7 @@ void testMerge_ValidatesResultResource_ResultResourceDoesNotHaveAllIdentifiersPr assertThat(issue.getSeverity()).isEqualTo(OperationOutcome.IssueSeverity.ERROR); assertThat(issue.getDiagnostics()).contains("'result-patient' must have all the identifiers provided in target-patient-identifier"); - verifyNoMoreInteractions(myPatientDaoMock); + verifyNoMoreInteractions(myPatientDaoMock, myTaskDaoMock, myProvenanceDaoMock, myBatch2TaskHelperMock); } @@ -1185,7 +1197,7 @@ void testMerge_ValidatesResultResource_ResultResourceHasNoReplacesLinkAtAll_Retu assertThat(issue.getSeverity()).isEqualTo(OperationOutcome.IssueSeverity.ERROR); assertThat(issue.getDiagnostics()).contains("'result-patient' must have a 'replaces' link to the source resource."); - verifyNoMoreInteractions(myPatientDaoMock); + verifyNoMoreInteractions(myPatientDaoMock, myTaskDaoMock, myProvenanceDaoMock, myBatch2TaskHelperMock); } @ParameterizedTest @@ -1218,7 +1230,7 @@ void testMerge_ValidatesResultResource_ResultResourceHasNoReplacesLinkToSource_R assertThat(issue.getSeverity()).isEqualTo(OperationOutcome.IssueSeverity.ERROR); assertThat(issue.getDiagnostics()).contains("'result-patient' must have a 'replaces' link to the source resource."); - verifyNoMoreInteractions(myPatientDaoMock); + verifyNoMoreInteractions(myPatientDaoMock, myTaskDaoMock, myProvenanceDaoMock, myBatch2TaskHelperMock); } @ParameterizedTest @@ -1251,7 +1263,7 @@ void testMerge_ValidatesResultResource_ResultResourceHasReplacesLinkAndDeleteSou assertThat(issue.getSeverity()).isEqualTo(OperationOutcome.IssueSeverity.ERROR); assertThat(issue.getDiagnostics()).contains("'result-patient' must not have a 'replaces' link to the source resource when the source resource will be deleted, as the link may prevent deleting the source resource."); - verifyNoMoreInteractions(myPatientDaoMock); + verifyNoMoreInteractions(myPatientDaoMock, myTaskDaoMock, myProvenanceDaoMock, myBatch2TaskHelperMock); } @ParameterizedTest @@ -1286,7 +1298,7 @@ void testMerge_ValidatesResultResource_ResultResourceHasRedundantReplacesLinksTo assertThat(issue.getSeverity()).isEqualTo(OperationOutcome.IssueSeverity.ERROR); assertThat(issue.getDiagnostics()).contains("'result-patient' has multiple 'replaces' links to the source resource. There should be only one."); - verifyNoMoreInteractions(myPatientDaoMock); + verifyNoMoreInteractions(myPatientDaoMock, myTaskDaoMock, myProvenanceDaoMock, myBatch2TaskHelperMock); } private void verifySuccessfulOutcomeForSync(MergeOperationOutcome theMergeOutcome, Patient theExpectedTargetResource) { @@ -1391,6 +1403,7 @@ private void setupDaoMockForSuccessfulSourcePatientUpdate(Patient thePatientExpe DaoMethodOutcome outcome = new DaoMethodOutcome(); outcome.setResource(thePatientToReturnInDaoOutcome); + thePatientExpectedAsInput.setId(thePatientToReturnInDaoOutcome.getIdElement()); return outcome; }); } @@ -1415,6 +1428,41 @@ private void verifyUpdatedTargetPatient(boolean theExpectLinkToSourcePatient, Li } + private void verifyProvenanceCreated(boolean theDeleteSource) { + + ArgumentCaptor captor = ArgumentCaptor.forClass(Provenance.class); + verify(myProvenanceDaoMock).create(captor.capture(), eq(myRequestDetailsMock)); + + Provenance provenance = captor.getValue(); + //assert targets + assertThat(provenance.getTarget()).hasSize(theDeleteSource ? 1 : 2); + // the first target reference should be the target patient + String targetPatientReference = provenance.getTarget().get(0).getReference(); + assertThat(provenance.getTarget().get(0).getReference()).isEqualTo(TARGET_PATIENT_TEST_ID_WITH_VERSION_2); + if (!theDeleteSource) { + // the second target reference should be the source patient, if it wasn't deleted + String sourcePatientReference = provenance.getTarget().get(1).getReference(); + assertThat(sourcePatientReference).isEqualTo(SOURCE_PATIENT_TEST_ID_WITH_VERSION_2); + } + + assertThat(provenance.getRecorded()).isCloseTo(Instant.now(), 60000); + + // validate provenance.reason + assertThat(provenance.getReason()).hasSize(1); + Coding reasonCoding = provenance.getReason().get(0).getCodingFirstRep(); + assertThat(reasonCoding).isNotNull(); + assertThat(reasonCoding.getSystem()).isEqualTo("http://terminology.hl7.org/CodeSystem/v3-ActReason"); + assertThat(reasonCoding.getCode()).isEqualTo("PATADMIN"); + + //validate provenance.activity + Coding activityCoding = provenance.getActivity().getCodingFirstRep(); + assertThat(activityCoding).isNotNull(); + assertThat(activityCoding.getSystem()).isEqualTo("http://terminology.hl7.org/CodeSystem/iso-21089-lifecycle"); + assertThat(activityCoding.getCode()).isEqualTo("merge"); + } + + + private void setupReplaceReferencesForSuccessForSync() { // set the count to less that the page size for sync processing when(myReplaceReferencesSvcMock.replaceReferences(isA(ReplaceReferencesRequest.class), diff --git a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/provider/merge/MergeBatchTest.java b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/provider/merge/MergeBatchTest.java index 196fb407462..15ca8e78b56 100644 --- a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/provider/merge/MergeBatchTest.java +++ b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/provider/merge/MergeBatchTest.java @@ -46,6 +46,11 @@ public void before() throws Exception { myTestHelper = new ReplaceReferencesTestHelper(myFhirContext, myDaoRegistry); myTestHelper.beforeEach(); + // keep the version on Provenance.target fields to verify that Provenance resources were saved + // with versioned target references + myFhirContext.getParserOptions() + .setDontStripVersionsFromReferencesAtPaths("Provenance.target"); + mySrd.setRequestPartitionId(RequestPartitionId.allPartitions()); } @@ -84,6 +89,8 @@ public void testHappyPath(boolean theDeleteSource, boolean theWithResultResource myTestHelper.assertSourcePatientUpdatedOrDeleted(theDeleteSource); myTestHelper.assertTargetPatientUpdated(theDeleteSource, myTestHelper.getExpectedIdentifiersForTargetAfterMerge(theWithResultResource)); + + myTestHelper.assertMergeProvenance(theDeleteSource); } @Test diff --git a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/provider/r4/PatientMergeR4Test.java b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/provider/r4/PatientMergeR4Test.java index f781f54befc..66046047183 100644 --- a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/provider/r4/PatientMergeR4Test.java +++ b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/provider/r4/PatientMergeR4Test.java @@ -78,12 +78,16 @@ public void before() throws Exception { myStorageSettings.setReuseCachedSearchResultsForMillis(null); myStorageSettings.setAllowMultipleDelete(true); myFhirContext.setParserErrorHandler(new StrictErrorHandler()); + // keep the version on Provenance.target fields to verify that Provenance resources were saved + // with versioned target references + myFhirContext.getParserOptions() + .setDontStripVersionsFromReferencesAtPaths("Provenance.target"); myTestHelper = new ReplaceReferencesTestHelper(myFhirContext, myDaoRegistry); myTestHelper.beforeEach(); } - @ParameterizedTest + @ParameterizedTest(name = "{index}: deleteSource={0}, resultPatient={1}, preview={2}, async={3}") @CsvSource({ // withDelete, withInputResultPatient, withPreview, isAsync "true, true, true, false", @@ -106,7 +110,6 @@ public void before() throws Exception { }) public void testMerge(boolean withDelete, boolean withInputResultPatient, boolean withPreview, boolean isAsync) { // setup - ReplaceReferencesTestHelper.PatientMergeInputParameters inParams = new ReplaceReferencesTestHelper.PatientMergeInputParameters(); myTestHelper.setSourceAndTarget(inParams); inParams.deleteSource = withDelete; @@ -225,6 +228,7 @@ public void testMerge(boolean withDelete, boolean withInputResultPatient, boolea myTestHelper.assertAllReferencesUpdated(withDelete); myTestHelper.assertSourcePatientUpdatedOrDeleted(withDelete); myTestHelper.assertTargetPatientUpdated(withDelete, expectedIdentifiersOnTargetAfterMerge); + myTestHelper.assertMergeProvenance(withDelete); } } @@ -361,8 +365,7 @@ private Parameters callMergeOperation(Parameters inParameters, boolean isAsync) class MyExceptionHandler implements TestExecutionExceptionHandler { @Override public void handleTestExecutionException(ExtensionContext theExtensionContext, Throwable theThrowable) throws Throwable { - if (theThrowable instanceof BaseServerResponseException) { - BaseServerResponseException ex = (BaseServerResponseException) theThrowable; + if (theThrowable instanceof BaseServerResponseException ex) { String message = extractFailureMessage(ex); throw ex.getClass().getDeclaredConstructor(String.class, Throwable.class).newInstance(message, ex); } diff --git a/hapi-fhir-jpaserver-test-utilities/src/main/java/ca/uhn/fhir/jpa/replacereferences/ReplaceReferencesTestHelper.java b/hapi-fhir-jpaserver-test-utilities/src/main/java/ca/uhn/fhir/jpa/replacereferences/ReplaceReferencesTestHelper.java index 856b092af07..bd449ee50ea 100644 --- a/hapi-fhir-jpaserver-test-utilities/src/main/java/ca/uhn/fhir/jpa/replacereferences/ReplaceReferencesTestHelper.java +++ b/hapi-fhir-jpaserver-test-utilities/src/main/java/ca/uhn/fhir/jpa/replacereferences/ReplaceReferencesTestHelper.java @@ -26,11 +26,13 @@ import ca.uhn.fhir.jpa.api.dao.IFhirResourceDao; import ca.uhn.fhir.jpa.api.dao.IFhirResourceDaoPatient; import ca.uhn.fhir.jpa.api.dao.PatientEverythingParameters; +import ca.uhn.fhir.jpa.searchparam.SearchParameterMap; import ca.uhn.fhir.model.primitive.IdDt; import ca.uhn.fhir.rest.api.server.IBundleProvider; import ca.uhn.fhir.rest.api.server.SystemRequestDetails; import ca.uhn.fhir.rest.client.api.IGenericClient; import ca.uhn.fhir.rest.gclient.IOperationUntypedWithInputAndPartialOutput; +import ca.uhn.fhir.rest.param.ReferenceParam; import ca.uhn.fhir.rest.server.exceptions.ResourceGoneException; import ca.uhn.fhir.rest.server.provider.ProviderConstants; import ca.uhn.fhir.util.JsonUtil; @@ -49,6 +51,8 @@ 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.Period; +import org.hl7.fhir.r4.model.Provenance; import org.hl7.fhir.r4.model.Reference; import org.hl7.fhir.r4.model.Resource; import org.hl7.fhir.r4.model.StringType; @@ -57,6 +61,8 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import java.time.Instant; +import java.time.temporal.ChronoUnit; import java.util.ArrayList; import java.util.List; import java.util.Set; @@ -96,6 +102,7 @@ public class ReplaceReferencesTestHelper { private final IFhirResourceDao myEncounterDao; private final IFhirResourceDao myCarePlanDao; private final IFhirResourceDao myObservationDao; + private final IFhirResourceDao myProvenanceDao; private IIdType myOrgId; private IIdType mySourcePatientId; @@ -117,6 +124,7 @@ public ReplaceReferencesTestHelper(FhirContext theFhirContext, DaoRegistry theDa myEncounterDao = theDaoRegistry.getResourceDao(Encounter.class); myCarePlanDao = theDaoRegistry.getResourceDao(CarePlan.class); myObservationDao = theDaoRegistry.getResourceDao(Observation.class); + myProvenanceDao = theDaoRegistry.getResourceDao(Provenance.class); } public void beforeEach() throws Exception { @@ -203,6 +211,52 @@ public IIdType getTargetPatientId() { return myTargetPatientId; } + public List searchProvenance(String targetId) { + SearchParameterMap map = new SearchParameterMap(); + map.add("target", new ReferenceParam(targetId)); + IBundleProvider searchBundle = myProvenanceDao.search(map, mySrd); + return searchBundle.getAllResources(); + } + + public void assertMergeProvenance(boolean theDeleteSource) { + List provenances = searchProvenance(myTargetPatientId.getValue()); + assertThat(provenances).hasSize(1); + Provenance provenance = (Provenance) provenances.get(0); + + // assert targets + assertThat(provenance.getTarget()).hasSize(theDeleteSource ? 1 : 2); + // the first target reference should be the target patient + String targetPatientReference = provenance.getTarget().get(0).getReference(); + assertThat(targetPatientReference).isEqualTo(myTargetPatientId.getValue() + "/_history/2"); + if (!theDeleteSource) { + // the second target reference should be the source patient, if it wasn't deleted + String sourcePatientReference = provenance.getTarget().get(1).getReference(); + assertThat(sourcePatientReference).isEqualTo(mySourcePatientId.getValue() + "/_history/2"); + } + + Instant now = Instant.now(); + Instant oneMinuteAgo = now.minus(1, ChronoUnit.MINUTES); + assertThat(provenance.getRecorded()).isBetween(oneMinuteAgo, now); + + Period period = provenance.getOccurredPeriod(); + assertThat(period.getStart()).isBefore(period.getEnd()); + assertThat(period.getStart()).isBetween(oneMinuteAgo, now); + assertThat(period.getEnd()).isEqualTo(provenance.getRecorded()); + + // validate provenance.reason + assertThat(provenance.getReason()).hasSize(1); + Coding reasonCoding = provenance.getReason().get(0).getCodingFirstRep(); + assertThat(reasonCoding).isNotNull(); + assertThat(reasonCoding.getSystem()).isEqualTo("http://terminology.hl7.org/CodeSystem/v3-ActReason"); + assertThat(reasonCoding.getCode()).isEqualTo("PATADMIN"); + + // validate provenance.activity + Coding activityCoding = provenance.getActivity().getCodingFirstRep(); + assertThat(activityCoding).isNotNull(); + assertThat(activityCoding.getSystem()).isEqualTo("http://terminology.hl7.org/CodeSystem/iso-21089-lifecycle"); + assertThat(activityCoding.getCode()).isEqualTo("merge"); + } + private Set getTargetEverythingResourceIds() { PatientEverythingParameters everythingParams = new PatientEverythingParameters(); everythingParams.setCount(new IntegerType(100)); @@ -432,7 +486,7 @@ private void validateJobReport(JobInstance theJobInstance, IIdType theTaskId) { public List getExpectedIdentifiersForTargetAfterMerge(boolean theWithInputResultPatient) { - List expectedIdentifiersOnTargetAfterMerge = null; + List expectedIdentifiersOnTargetAfterMerge; if (theWithInputResultPatient) { expectedIdentifiersOnTargetAfterMerge = List.of(new Identifier().setSystem("SYS1A").setValue("VAL1A")); @@ -450,7 +504,7 @@ public List getExpectedIdentifiersForTargetAfterMerge(boolean theWit public void assertSourcePatientUpdatedOrDeleted(boolean withDelete) { if (withDelete) { - assertThrows(ResourceGoneException.class, () -> readSourcePatient()); + assertThrows(ResourceGoneException.class, this::readSourcePatient); } else { Patient source = readSourcePatient(); assertThat(source.getLink()).hasSize(1); diff --git a/hapi-fhir-storage-batch2-jobs/src/main/java/ca/uhn/fhir/batch2/jobs/merge/MergeResourceHelper.java b/hapi-fhir-storage-batch2-jobs/src/main/java/ca/uhn/fhir/batch2/jobs/merge/MergeResourceHelper.java index 74645cba581..45064c8ec27 100644 --- a/hapi-fhir-storage-batch2-jobs/src/main/java/ca/uhn/fhir/batch2/jobs/merge/MergeResourceHelper.java +++ b/hapi-fhir-storage-batch2-jobs/src/main/java/ca/uhn/fhir/batch2/jobs/merge/MergeResourceHelper.java @@ -23,15 +23,20 @@ import ca.uhn.fhir.jpa.api.dao.IFhirResourceDao; import ca.uhn.fhir.jpa.api.model.DaoMethodOutcome; import ca.uhn.fhir.jpa.dao.tx.IHapiTransactionService; +import ca.uhn.fhir.model.api.TemporalPrecisionEnum; import ca.uhn.fhir.rest.api.server.RequestDetails; import ca.uhn.fhir.rest.server.provider.ProviderConstants; import jakarta.annotation.Nullable; import org.hl7.fhir.instance.model.api.IIdType; import org.hl7.fhir.instance.model.api.IPrimitiveType; +import org.hl7.fhir.r4.model.CodeableConcept; import org.hl7.fhir.r4.model.Identifier; import org.hl7.fhir.r4.model.Patient; +import org.hl7.fhir.r4.model.Period; +import org.hl7.fhir.r4.model.Provenance; import org.hl7.fhir.r4.model.Reference; +import java.util.Date; import java.util.List; import java.util.concurrent.atomic.AtomicReference; @@ -43,10 +48,17 @@ */ public class MergeResourceHelper { + private static final String ACTIVITY_CODE_SYSTEM = "http://terminology.hl7.org/CodeSystem/iso-21089-lifecycle"; + private static final String ACTIVITY_CODE_MERGE = "merge"; + private static final String ACT_REASON_CODE_SYSTEM = "http://terminology.hl7.org/CodeSystem/v3-ActReason"; + private static final String ACT_REASON_PATIENT_ADMINISTRATION_CODE = "PATADMIN"; + private final IFhirResourceDao myPatientDao; + private final IFhirResourceDao myProvenceDao; - public MergeResourceHelper(IFhirResourceDao theDao) { - myPatientDao = theDao; + public MergeResourceHelper(IFhirResourceDao thePatientDao, IFhirResourceDao theProvenanceDao) { + myPatientDao = thePatientDao; + myProvenceDao = theProvenanceDao; } public static int setResourceLimitFromParameter( @@ -66,7 +78,8 @@ public void updateMergedResourcesAfterReferencesReplaced( IIdType theTargetResourceId, @Nullable Patient theResultResource, boolean theDeleteSource, - RequestDetails theRequestDetails) { + RequestDetails theRequestDetails, + Date theStartTime) { Patient sourceResource = myPatientDao.read(theSourceResourceId, theRequestDetails); Patient targetResource = myPatientDao.read(theTargetResourceId, theRequestDetails); @@ -76,7 +89,8 @@ public void updateMergedResourcesAfterReferencesReplaced( targetResource, theResultResource, theDeleteSource, - theRequestDetails); + theRequestDetails, + theStartTime); } public Patient updateMergedResourcesAfterReferencesReplaced( @@ -85,7 +99,8 @@ public Patient updateMergedResourcesAfterReferencesReplaced( Patient theTargetResource, @Nullable Patient theResultResource, boolean theDeleteSource, - RequestDetails theRequestDetails) { + RequestDetails theRequestDetails, + Date theStartTime) { AtomicReference targetPatientAfterUpdate = new AtomicReference<>(); myHapiTransactionService.withRequest(theRequestDetails).execute(() -> { @@ -100,6 +115,13 @@ public Patient updateMergedResourcesAfterReferencesReplaced( prepareSourcePatientForUpdate(theSourceResource, theTargetResource); updateResource(theSourceResource, theRequestDetails); } + + createProvenance( + theSourceResource, + targetPatientAfterUpdate.get(), + theDeleteSource, + theRequestDetails, + theStartTime); }); return targetPatientAfterUpdate.get(); @@ -184,4 +206,36 @@ private Patient updateResource(Patient theResource, RequestDetails theRequestDet private void deleteResource(Patient theResource, RequestDetails theRequestDetails) { myPatientDao.delete(theResource.getIdElement(), theRequestDetails); } + + private void createProvenance( + Patient theSourcePatient, + Patient theTargetPatient, + boolean theDeleteSource, + RequestDetails theRequestDetails, + Date theStartTime) { + + Provenance provenance = new Provenance(); + provenance.addTarget().setReference(theTargetPatient.getIdElement().getValue()); + if (!theDeleteSource) { + provenance.addTarget().setReference(theSourcePatient.getIdElement().getValue()); + } + Date now = new Date(); + provenance.setOccurred(new Period() + .setStart(theStartTime, TemporalPrecisionEnum.MILLI) + .setEnd(now, TemporalPrecisionEnum.MILLI)); + provenance.setRecorded(now); + CodeableConcept activityCodeableConcept = new CodeableConcept(); + activityCodeableConcept.addCoding().setSystem(ACTIVITY_CODE_SYSTEM).setCode(ACTIVITY_CODE_MERGE); + provenance.setActivity(activityCodeableConcept); + + CodeableConcept activityReasonCodeableConcept = new CodeableConcept(); + activityReasonCodeableConcept + .addCoding() + .setSystem(ACT_REASON_CODE_SYSTEM) + .setCode(ACT_REASON_PATIENT_ADMINISTRATION_CODE); + provenance.addReason(activityReasonCodeableConcept); + + // TODO Emre: should we add agent + myProvenceDao.create(provenance, theRequestDetails); + } } diff --git a/hapi-fhir-storage-batch2-jobs/src/main/java/ca/uhn/fhir/batch2/jobs/merge/MergeUpdateTaskReducerStep.java b/hapi-fhir-storage-batch2-jobs/src/main/java/ca/uhn/fhir/batch2/jobs/merge/MergeUpdateTaskReducerStep.java index d57050994e7..0597afe5904 100644 --- a/hapi-fhir-storage-batch2-jobs/src/main/java/ca/uhn/fhir/batch2/jobs/merge/MergeUpdateTaskReducerStep.java +++ b/hapi-fhir-storage-batch2-jobs/src/main/java/ca/uhn/fhir/batch2/jobs/merge/MergeUpdateTaskReducerStep.java @@ -32,6 +32,9 @@ import ca.uhn.fhir.rest.api.server.SystemRequestDetails; import jakarta.annotation.Nonnull; import org.hl7.fhir.r4.model.Patient; +import org.hl7.fhir.r4.model.Provenance; + +import java.util.Date; public class MergeUpdateTaskReducerStep extends ReplaceReferenceUpdateTaskReducerStep { private final IHapiTransactionService myHapiTransactionService; @@ -48,6 +51,8 @@ public RunOutcome run( @Nonnull IJobDataSink theDataSink) throws JobExecutionFailedException { + Date startTime = theStepExecutionDetails.getInstance().getStartTime(); + MergeJobParameters mergeJobParameters = theStepExecutionDetails.getParameters(); SystemRequestDetails requestDetails = SystemRequestDetails.forRequestPartitionId(mergeJobParameters.getPartitionId()); @@ -59,8 +64,8 @@ public RunOutcome run( } IFhirResourceDao patientDao = myDaoRegistry.getResourceDao(Patient.class); - - MergeResourceHelper helper = new MergeResourceHelper(patientDao); + IFhirResourceDao provenanceDao = myDaoRegistry.getResourceDao(Provenance.class); + MergeResourceHelper helper = new MergeResourceHelper(patientDao, provenanceDao); helper.updateMergedResourcesAfterReferencesReplaced( myHapiTransactionService, @@ -68,7 +73,8 @@ public RunOutcome run( mergeJobParameters.getTargetId().asIdDt(), resultResource, mergeJobParameters.getDeleteSource(), - requestDetails); + requestDetails, + startTime); return super.run(theStepExecutionDetails, theDataSink); } From 6c1331edd387da50c4371d3a61d7a56992451976 Mon Sep 17 00:00:00 2001 From: Emre Dincturk Date: Thu, 9 Jan 2025 16:25:16 -0500 Subject: [PATCH 2/3] added provenance to replace-references and added provenance tests for merge as well --- .../fhir/jpa/provider/JpaSystemProvider.java | 2 +- .../provider/ReplaceReferencesSvcImpl.java | 15 +++ .../provider/merge/ResourceMergeService.java | 28 +++- .../merge/ResourceMergeServiceTest.java | 10 +- .../jpa/provider/r4/PatientMergeR4Test.java | 57 +++++++- .../provider/r4/ReplaceReferencesR4Test.java | 6 + .../ReplaceReferencesTestHelper.java | 95 ++++++++++++- .../fhir/batch2/jobs/merge/MergeAppCtx.java | 20 ++- .../batch2/jobs/merge/MergeProvenanceSvc.java | 41 ++++++ .../jobs/merge/MergeResourceHelper.java | 82 ++++-------- .../merge/MergeUpdateTaskReducerStep.java | 24 ++-- ...ReplaceReferenceUpdateTaskReducerStep.java | 68 +++++++--- .../ReplaceReferencesAppCtx.java | 11 +- .../ReplaceReferencesJobParameters.java | 7 +- .../ReplaceReferencesProvenanceSvc.java | 125 ++++++++++++++++++ .../ReplaceReferencesRequest.java | 6 +- 16 files changed, 487 insertions(+), 110 deletions(-) create mode 100644 hapi-fhir-storage-batch2-jobs/src/main/java/ca/uhn/fhir/batch2/jobs/merge/MergeProvenanceSvc.java create mode 100644 hapi-fhir-storage-batch2-jobs/src/main/java/ca/uhn/fhir/batch2/jobs/replacereferences/ReplaceReferencesProvenanceSvc.java diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/provider/JpaSystemProvider.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/provider/JpaSystemProvider.java index 6b917aa531e..83b27345ad0 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/provider/JpaSystemProvider.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/provider/JpaSystemProvider.java @@ -191,7 +191,7 @@ public IBaseParameters replaceReferences( RequestPartitionId partitionId = myRequestPartitionHelperSvc.determineReadPartitionForRequest( theServletRequest, ReadPartitionIdRequestDetails.forRead(targetId)); ReplaceReferencesRequest replaceReferencesRequest = - new ReplaceReferencesRequest(sourceId, targetId, resourceLimit, partitionId); + new ReplaceReferencesRequest(sourceId, targetId, resourceLimit, partitionId, true); IBaseParameters retval = getReplaceReferencesSvc().replaceReferences(replaceReferencesRequest, theServletRequest); if (ParametersUtil.getNamedParameter(getContext(), retval, OPERATION_REPLACE_REFERENCES_OUTPUT_PARAM_TASK) diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/provider/ReplaceReferencesSvcImpl.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/provider/ReplaceReferencesSvcImpl.java index 8a14538abdf..915fe6078a0 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/provider/ReplaceReferencesSvcImpl.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/provider/ReplaceReferencesSvcImpl.java @@ -21,6 +21,7 @@ import ca.uhn.fhir.batch2.api.IJobCoordinator; import ca.uhn.fhir.batch2.jobs.replacereferences.ReplaceReferencesJobParameters; +import ca.uhn.fhir.batch2.jobs.replacereferences.ReplaceReferencesProvenanceSvc; import ca.uhn.fhir.batch2.util.Batch2TaskHelper; import ca.uhn.fhir.i18n.Msg; import ca.uhn.fhir.jpa.api.config.JpaStorageSettings; @@ -42,6 +43,8 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import java.util.Date; +import java.util.List; import java.util.stream.Stream; import static ca.uhn.fhir.batch2.jobs.replacereferences.ReplaceReferencesAppCtx.JOB_REPLACE_REFERENCES; @@ -58,6 +61,7 @@ public class ReplaceReferencesSvcImpl implements IReplaceReferencesSvc { private final ReplaceReferencesPatchBundleSvc myReplaceReferencesPatchBundleSvc; private final Batch2TaskHelper myBatch2TaskHelper; private final JpaStorageSettings myStorageSettings; + private final ReplaceReferencesProvenanceSvc myReplaceReferencesProvenanceSvc; public ReplaceReferencesSvcImpl( DaoRegistry theDaoRegistry, @@ -74,6 +78,7 @@ public ReplaceReferencesSvcImpl( myReplaceReferencesPatchBundleSvc = theReplaceReferencesPatchBundleSvc; myBatch2TaskHelper = theBatch2TaskHelper; myStorageSettings = theStorageSettings; + myReplaceReferencesProvenanceSvc = new ReplaceReferencesProvenanceSvc(theDaoRegistry); } @Override @@ -123,6 +128,7 @@ private IBaseParameters replaceReferencesPreferAsync( private IBaseParameters replaceReferencesPreferSync( ReplaceReferencesRequest theReplaceReferencesRequest, RequestDetails theRequestDetails) { + Date startTime = new Date(); // TODO KHS get partition from request StopLimitAccumulator accumulator = myHapiTransactionService .withRequest(theRequestDetails) @@ -139,6 +145,15 @@ private IBaseParameters replaceReferencesPreferSync( Bundle result = myReplaceReferencesPatchBundleSvc.patchReferencingResources( theReplaceReferencesRequest, accumulator.getItemList(), theRequestDetails); + if (theReplaceReferencesRequest.createProvenance) { + myReplaceReferencesProvenanceSvc.createProvenance( + theReplaceReferencesRequest.targetId, + theReplaceReferencesRequest.sourceId, + List.of(result), + startTime, + theRequestDetails); + } + Parameters retval = new Parameters(); retval.addParameter() .setName(OPERATION_REPLACE_REFERENCES_OUTPUT_PARAM_OUTCOME) diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/provider/merge/ResourceMergeService.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/provider/merge/ResourceMergeService.java index b968ea5217a..d3ee4931449 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/provider/merge/ResourceMergeService.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/provider/merge/ResourceMergeService.java @@ -21,6 +21,7 @@ import ca.uhn.fhir.batch2.api.IJobCoordinator; import ca.uhn.fhir.batch2.jobs.merge.MergeJobParameters; +import ca.uhn.fhir.batch2.jobs.merge.MergeProvenanceSvc; import ca.uhn.fhir.batch2.jobs.merge.MergeResourceHelper; import ca.uhn.fhir.batch2.util.Batch2TaskHelper; import ca.uhn.fhir.context.FhirContext; @@ -36,20 +37,24 @@ import ca.uhn.fhir.rest.api.server.RequestDetails; import ca.uhn.fhir.rest.server.exceptions.BaseServerResponseException; import ca.uhn.fhir.util.OperationOutcomeUtil; +import ca.uhn.fhir.util.ParametersUtil; import org.hl7.fhir.instance.model.api.IBase; import org.hl7.fhir.instance.model.api.IBaseOperationOutcome; +import org.hl7.fhir.instance.model.api.IBaseParameters; +import org.hl7.fhir.r4.model.Bundle; import org.hl7.fhir.r4.model.Patient; -import org.hl7.fhir.r4.model.Provenance; import org.hl7.fhir.r4.model.Task; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import java.util.Date; +import java.util.List; import static ca.uhn.fhir.batch2.jobs.merge.MergeAppCtx.JOB_MERGE; import static ca.uhn.fhir.rest.api.Constants.STATUS_HTTP_200_OK; import static ca.uhn.fhir.rest.api.Constants.STATUS_HTTP_202_ACCEPTED; import static ca.uhn.fhir.rest.api.Constants.STATUS_HTTP_500_INTERNAL_ERROR; +import static ca.uhn.fhir.rest.server.provider.ProviderConstants.OPERATION_REPLACE_REFERENCES_OUTPUT_PARAM_OUTCOME; /** * Service for the FHIR $merge operation. Currently only supports Patient/$merge. The plan is to expand to other resource types. @@ -68,6 +73,7 @@ public class ResourceMergeService { private final MergeResourceHelper myMergeResourceHelper; private final Batch2TaskHelper myBatch2TaskHelper; private final MergeValidationService myMergeValidationService; + private final MergeProvenanceSvc myMergeProvenanceSvc; public ResourceMergeService( JpaStorageSettings theStorageSettings, @@ -87,8 +93,8 @@ public ResourceMergeService( myBatch2TaskHelper = theBatch2TaskHelper; myFhirContext = myPatientDao.getContext(); myHapiTransactionService = theHapiTransactionService; - IFhirResourceDao provenanceDao = theDaoRegistry.getResourceDao(Provenance.class); - myMergeResourceHelper = new MergeResourceHelper(myPatientDao, provenanceDao); + myMergeProvenanceSvc = new MergeProvenanceSvc(theDaoRegistry); + myMergeResourceHelper = new MergeResourceHelper(theDaoRegistry, myMergeProvenanceSvc); myMergeValidationService = new MergeValidationService(myFhirContext, theDaoRegistry); } @@ -224,18 +230,28 @@ private void doMergeSync( theSourceResource.getIdElement(), theTargetResource.getIdElement(), theMergeOperationParameters.getResourceLimit(), - partitionId); + partitionId, + // don't create provenance as part of replace-references, + // we create it after updating source and target for merge + false); - myReplaceReferencesSvc.replaceReferences(replaceReferencesRequest, theRequestDetails); + IBaseParameters outParams = + myReplaceReferencesSvc.replaceReferences(replaceReferencesRequest, theRequestDetails); - Patient updatedTarget = myMergeResourceHelper.updateMergedResourcesAfterReferencesReplaced( + Bundle patchResultBundle = (Bundle) ParametersUtil.getNamedParameterResource( + myFhirContext, outParams, OPERATION_REPLACE_REFERENCES_OUTPUT_PARAM_OUTCOME) + .orElseThrow(); + + Patient updatedTarget = myMergeResourceHelper.updateMergedResourcesAndCreateProvenance( myHapiTransactionService, theSourceResource, theTargetResource, + List.of(patchResultBundle), (Patient) theMergeOperationParameters.getResultResource(), theMergeOperationParameters.getDeleteSource(), theRequestDetails, startTime); + theMergeOutcome.setUpdatedTargetResource(updatedTarget); String detailsText = "Merge operation completed successfully."; diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/merge/ResourceMergeServiceTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/merge/ResourceMergeServiceTest.java index 9131823ea30..f67dfd35ae0 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/merge/ResourceMergeServiceTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/merge/ResourceMergeServiceTest.java @@ -23,6 +23,7 @@ import ca.uhn.fhir.rest.server.exceptions.ResourceNotFoundException; import ca.uhn.fhir.util.CanonicalIdentifier; import org.hl7.fhir.instance.model.api.IBaseResource; +import org.hl7.fhir.r4.model.Bundle; import org.hl7.fhir.r4.model.Coding; import org.hl7.fhir.r4.model.IdType; import org.hl7.fhir.r4.model.Identifier; @@ -1464,9 +1465,14 @@ private void verifyProvenanceCreated(boolean theDeleteSource) { private void setupReplaceReferencesForSuccessForSync() { - // set the count to less that the page size for sync processing + Parameters parameters = new Parameters(); + Parameters.ParametersParameterComponent outcomeParameter = new Parameters.ParametersParameterComponent(); + outcomeParameter.setName("outcome"); + outcomeParameter.setResource(new Bundle()); + parameters.addParameter(outcomeParameter); + when(myReplaceReferencesSvcMock.replaceReferences(isA(ReplaceReferencesRequest.class), - eq(myRequestDetailsMock))).thenReturn(new Parameters()); + eq(myRequestDetailsMock))).thenReturn(parameters); } private void setupBatch2JobTaskHelperMock(Task theTaskToReturn) { diff --git a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/provider/r4/PatientMergeR4Test.java b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/provider/r4/PatientMergeR4Test.java index 66046047183..fa3c33462bc 100644 --- a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/provider/r4/PatientMergeR4Test.java +++ b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/provider/r4/PatientMergeR4Test.java @@ -12,6 +12,7 @@ import ca.uhn.fhir.rest.server.exceptions.UnprocessableEntityException; import jakarta.annotation.Nonnull; import jakarta.servlet.http.HttpServletResponse; +import org.hl7.fhir.instance.model.api.IIdType; import org.hl7.fhir.r4.model.Bundle; import org.hl7.fhir.r4.model.Coding; import org.hl7.fhir.r4.model.Encounter; @@ -32,7 +33,9 @@ import org.junit.jupiter.params.provider.CsvSource; import org.springframework.beans.factory.annotation.Autowired; +import java.util.Collections; import java.util.List; +import java.util.Set; import java.util.stream.Collectors; import static ca.uhn.fhir.jpa.provider.ReplaceReferencesSvcImpl.RESOURCE_TYPES_SYSTEM; @@ -143,6 +146,7 @@ public void testMerge(boolean withDelete, boolean withInputResultPatient, boolea List expectedIdentifiersOnTargetAfterMerge = myTestHelper.getExpectedIdentifiersForTargetAfterMerge(withInputResultPatient); + // Assert Task inAsync mode, unless it is preview in which case we don't return a task if (isAsync && !withPreview) { assertThat(getLastHttpStatusCode()).isEqualTo(HttpServletResponse.SC_ACCEPTED); @@ -246,6 +250,55 @@ void testMerge_smallResourceLimit() { .satisfies(ex -> assertThat(extractFailureMessage((BaseServerResponseException) ex)).isEqualTo("HAPI-2597: Number of resources with references to "+ myTestHelper.getSourcePatientId() + " exceeds the resource-limit 5. Submit the request asynchronsly by adding the HTTP Header 'Prefer: respond-async'.")); } + @ParameterizedTest(name = "{index}: deleteSource={0}, async={1}") + @CsvSource({ + "true, false", + "false, false", + "true, true", + "false, true", + }) + void testMerge_resourcesWithNoReferences(boolean theDeleteSource, boolean theAsync) { + + Patient sourcePatient = new Patient(); + sourcePatient = (Patient )myPatientDao.create(sourcePatient, mySrd).getResource(); + + + Patient targetPatient = new Patient(); + targetPatient = (Patient) myPatientDao.create(targetPatient, mySrd).getResource(); + + ReplaceReferencesTestHelper.PatientMergeInputParameters inParams = new ReplaceReferencesTestHelper.PatientMergeInputParameters(); + inParams.sourcePatient = new Reference(sourcePatient.getIdElement().toVersionless()); + inParams.targetPatient = new Reference(targetPatient.getIdElement().toVersionless()); + if (theDeleteSource) { + inParams.deleteSource = true; + } + + Parameters outParams = callMergeOperation(inParams.asParametersResource(), theAsync); + + if (theAsync) { + assertThat(getLastHttpStatusCode()).isEqualTo(HttpServletResponse.SC_ACCEPTED); + Task task = (Task) outParams.getParameter(OPERATION_MERGE_OUTPUT_PARAM_TASK).getResource(); + assertNull(task.getIdElement().getVersionIdPart()); + ourLog.info("Got task {}", task.getId()); + String jobId = myTestHelper.getJobIdFromTask(task); + myBatch2JobHelper.awaitJobCompletion(jobId); + } + + IIdType theExpectedTargetIdWithVersion = targetPatient.getIdElement().withVersion("2"); + if (theDeleteSource) { + // when the source resource is being deleted and since there is no identifiers to copy over to the target + // in this test, the target is not actually updated, so its version will remain the same + theExpectedTargetIdWithVersion = targetPatient.getIdElement().withVersion("1"); + } + + myTestHelper.assertMergeProvenance(theDeleteSource, + sourcePatient.getIdElement().withVersion("2"), + theExpectedTargetIdWithVersion, + 0, + Collections.EMPTY_SET); + } + + @Test void testMerge_SourceResourceCannotBeDeletedBecauseAnotherResourceReferencingSourceWasAddedWhileJobIsRunning_JobFails() { ReplaceReferencesTestHelper.PatientMergeInputParameters inParams = new ReplaceReferencesTestHelper.PatientMergeInputParameters(); @@ -284,7 +337,7 @@ void testMerge_SourceResourceCannotBeDeletedBecauseAnotherResourceReferencingSou assertThat(taskAfterJobFailure.getStatus()).isEqualTo(Task.TaskStatus.FAILED); } - @ParameterizedTest + @ParameterizedTest(name = "{index}: deleteSource={0}, resultPatient={1}, preview={2}") @CsvSource({ // withDelete, withInputResultPatient, withPreview "true, true, true", @@ -305,7 +358,7 @@ public void testMultipleTargetMatchesFails(boolean withDelete, boolean withInput } - @ParameterizedTest + @ParameterizedTest(name = "{index}: deleteSource={0}, resultPatient={1}, preview={2}") @CsvSource({ // withDelete, withInputResultPatient, withPreview "true, true, true", diff --git a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/provider/r4/ReplaceReferencesR4Test.java b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/provider/r4/ReplaceReferencesR4Test.java index 92b140ac95c..b09b3f91845 100644 --- a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/provider/r4/ReplaceReferencesR4Test.java +++ b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/provider/r4/ReplaceReferencesR4Test.java @@ -45,6 +45,10 @@ public void after() throws Exception { @BeforeEach public void before() throws Exception { super.before(); + // keep the version on Provenance.target fields to verify that Provenance resources were saved + // with versioned target references + myFhirContext.getParserOptions() + .setDontStripVersionsFromReferencesAtPaths("Provenance.target"); myTestHelper = new ReplaceReferencesTestHelper(myFhirContext, myDaoRegistry); myTestHelper.beforeEach(); @@ -81,6 +85,7 @@ void testReplaceReferences(boolean isAsync) { // Check that the linked resources were updated myTestHelper.assertAllReferencesUpdated(); + myTestHelper.assertReplaceReferencesProvenance(); } private JobInstance awaitJobCompletion(Task task) { @@ -157,6 +162,7 @@ void testReplaceReferencesSmallTransactionEntriesSize() { // Check that the linked resources were updated myTestHelper.assertAllReferencesUpdated(); + myTestHelper.assertReplaceReferencesProvenance(); } // TODO ED we should add some tests for the invalid request error cases (and assert 4xx status code) diff --git a/hapi-fhir-jpaserver-test-utilities/src/main/java/ca/uhn/fhir/jpa/replacereferences/ReplaceReferencesTestHelper.java b/hapi-fhir-jpaserver-test-utilities/src/main/java/ca/uhn/fhir/jpa/replacereferences/ReplaceReferencesTestHelper.java index bd449ee50ea..d439370cf2e 100644 --- a/hapi-fhir-jpaserver-test-utilities/src/main/java/ca/uhn/fhir/jpa/replacereferences/ReplaceReferencesTestHelper.java +++ b/hapi-fhir-jpaserver-test-utilities/src/main/java/ca/uhn/fhir/jpa/replacereferences/ReplaceReferencesTestHelper.java @@ -64,6 +64,7 @@ import java.time.Instant; import java.time.temporal.ChronoUnit; import java.util.ArrayList; +import java.util.HashSet; import java.util.List; import java.util.Set; import java.util.regex.Pattern; @@ -218,22 +219,86 @@ public List searchProvenance(String targetId) { return searchBundle.getAllResources(); } + public void assertReplaceReferencesProvenance() { + List provenances = + searchProvenance(myTargetPatientId.toVersionless().getIdPart()); + assertThat(provenances).hasSize(1); + Provenance provenance = (Provenance) provenances.get(0); + + // assert targets + int expectedNumberOfProvenanceTargets = TOTAL_EXPECTED_PATCHES + 2; + assertThat(provenance.getTarget()).hasSize(expectedNumberOfProvenanceTargets); + // the first target reference should be the target patient + String targetPatientReferenceInProvenance = + provenance.getTarget().get(0).getReference(); + assertThat(targetPatientReferenceInProvenance.toString()).isEqualTo(myTargetPatientId.toString()); + // the second target reference should be the source patient, if it wasn't deleted + String sourcePatientReference = provenance.getTarget().get(1).getReference(); + assertThat(sourcePatientReference.toString()).isEqualTo(mySourcePatientId.toString()); + + Set allActualTargets = extractResourceIdsFromProvenanceTarget(provenance.getTarget()); + assertThat(allActualTargets).containsAll(getExpectedProvenanceTargetsForPatchedResources()); + + Instant now = Instant.now(); + Instant oneMinuteAgo = now.minus(1, ChronoUnit.MINUTES); + assertThat(provenance.getRecorded()).isBetween(oneMinuteAgo, now); + + Period period = provenance.getOccurredPeriod(); + assertThat(period.getStart()).isBefore(period.getEnd()); + assertThat(period.getStart()).isBetween(oneMinuteAgo, now); + assertThat(period.getEnd()).isEqualTo(provenance.getRecorded()); + + // validate provenance.reason + assertThat(provenance.getReason()).hasSize(1); + Coding reasonCoding = provenance.getReason().get(0).getCodingFirstRep(); + assertThat(reasonCoding).isNotNull(); + assertThat(reasonCoding.getSystem()).isEqualTo("http://terminology.hl7.org/CodeSystem/v3-ActReason"); + assertThat(reasonCoding.getCode()).isEqualTo("PATADMIN"); + + // FIXME KHS: assert provenance activity code for replace-references + ; + } + public void assertMergeProvenance(boolean theDeleteSource) { - List provenances = searchProvenance(myTargetPatientId.getValue()); + assertMergeProvenance( + theDeleteSource, + mySourcePatientId.withVersion("2"), + myTargetPatientId.withVersion("2"), + TOTAL_EXPECTED_PATCHES, + getExpectedProvenanceTargetsForPatchedResources()); + } + + public void assertMergeProvenance( + boolean theDeleteSource, + IIdType theSourcePatientIdWithExpectedVersion, + IIdType theTargetPatientIdWithExpectedVersion, + int theExpectedPatches, + Set theExpectedProvenanceTargetsForPatchedResources) { + + List provenances = searchProvenance( + theTargetPatientIdWithExpectedVersion.toVersionless().getIdPart()); assertThat(provenances).hasSize(1); Provenance provenance = (Provenance) provenances.get(0); // assert targets - assertThat(provenance.getTarget()).hasSize(theDeleteSource ? 1 : 2); + int expectedNumberOfProvenanceTargets = theExpectedPatches; + // target patient and source patient if not deleted + expectedNumberOfProvenanceTargets += theDeleteSource ? 1 : 2; + assertThat(provenance.getTarget()).hasSize(expectedNumberOfProvenanceTargets); // the first target reference should be the target patient - String targetPatientReference = provenance.getTarget().get(0).getReference(); - assertThat(targetPatientReference).isEqualTo(myTargetPatientId.getValue() + "/_history/2"); + String targetPatientReferenceInProvenance = + provenance.getTarget().get(0).getReference(); + assertThat(targetPatientReferenceInProvenance.toString()) + .isEqualTo(theTargetPatientIdWithExpectedVersion.toString()); if (!theDeleteSource) { // the second target reference should be the source patient, if it wasn't deleted String sourcePatientReference = provenance.getTarget().get(1).getReference(); - assertThat(sourcePatientReference).isEqualTo(mySourcePatientId.getValue() + "/_history/2"); + assertThat(sourcePatientReference.toString()).isEqualTo(theSourcePatientIdWithExpectedVersion.toString()); } + Set allActualTargets = extractResourceIdsFromProvenanceTarget(provenance.getTarget()); + assertThat(allActualTargets).containsAll(theExpectedProvenanceTargetsForPatchedResources); + Instant now = Instant.now(); Instant oneMinuteAgo = now.minus(1, ChronoUnit.MINUTES); assertThat(provenance.getRecorded()).isBetween(oneMinuteAgo, now); @@ -257,6 +322,18 @@ public void assertMergeProvenance(boolean theDeleteSource) { assertThat(activityCoding.getCode()).isEqualTo("merge"); } + private Set getExpectedProvenanceTargetsForPatchedResources() { + Set allExpectedTargets = new HashSet<>(); + + allExpectedTargets.add(mySourceEncId1.withVersion("2").toString()); + allExpectedTargets.add(mySourceEncId2.withVersion("2").toString()); + allExpectedTargets.add(mySourceCarePlanId.withVersion("2").toString()); + allExpectedTargets.addAll(mySourceObsIds.stream() + .map(obsId -> obsId.withVersion("2").toString()) + .toList()); + return allExpectedTargets; + } + private Set getTargetEverythingResourceIds() { PatientEverythingParameters everythingParams = new PatientEverythingParameters(); everythingParams.setCount(new IntegerType(100)); @@ -534,4 +611,12 @@ public void assertIdentifiers(List theActualIdentifiers, List extractResourceIdsFromProvenanceTarget(List theTargets) { + return theTargets.stream() + .map(Reference::getReference) + .map(IdDt::new) + .map(IdDt::toString) + .collect(Collectors.toSet()); + } } diff --git a/hapi-fhir-storage-batch2-jobs/src/main/java/ca/uhn/fhir/batch2/jobs/merge/MergeAppCtx.java b/hapi-fhir-storage-batch2-jobs/src/main/java/ca/uhn/fhir/batch2/jobs/merge/MergeAppCtx.java index 243c276a103..de9bc449d7f 100644 --- a/hapi-fhir-storage-batch2-jobs/src/main/java/ca/uhn/fhir/batch2/jobs/merge/MergeAppCtx.java +++ b/hapi-fhir-storage-batch2-jobs/src/main/java/ca/uhn/fhir/batch2/jobs/merge/MergeAppCtx.java @@ -85,10 +85,26 @@ public ReplaceReferenceUpdateStep mergeUpdateStep( return new ReplaceReferenceUpdateStep<>(theFhirContext, theReplaceReferencesPatchBundleSvc); } + @Bean + public MergeProvenanceSvc mergeProvenanceSvc(DaoRegistry theDaoRegistry) { + return new MergeProvenanceSvc(theDaoRegistry); + } + + @Bean + public MergeResourceHelper mergeResourceHelper( + DaoRegistry theDaoRegistry, MergeProvenanceSvc theMergeProvenanceSvc) { + + return new MergeResourceHelper(theDaoRegistry, theMergeProvenanceSvc); + } + @Bean public MergeUpdateTaskReducerStep mergeUpdateTaskStep( - DaoRegistry theDaoRegistry, IHapiTransactionService theHapiTransactionService) { - return new MergeUpdateTaskReducerStep(theDaoRegistry, theHapiTransactionService); + DaoRegistry theDaoRegistry, + IHapiTransactionService theHapiTransactionService, + MergeResourceHelper theMergeResourceHelper, + MergeProvenanceSvc theMergeProvenanceSvc) { + return new MergeUpdateTaskReducerStep( + theDaoRegistry, theHapiTransactionService, theMergeResourceHelper, theMergeProvenanceSvc); } @Bean diff --git a/hapi-fhir-storage-batch2-jobs/src/main/java/ca/uhn/fhir/batch2/jobs/merge/MergeProvenanceSvc.java b/hapi-fhir-storage-batch2-jobs/src/main/java/ca/uhn/fhir/batch2/jobs/merge/MergeProvenanceSvc.java new file mode 100644 index 00000000000..b039d02284f --- /dev/null +++ b/hapi-fhir-storage-batch2-jobs/src/main/java/ca/uhn/fhir/batch2/jobs/merge/MergeProvenanceSvc.java @@ -0,0 +1,41 @@ +/*- + * #%L + * hapi-fhir-storage-batch2-jobs + * %% + * Copyright (C) 2014 - 2025 Smile CDR, Inc. + * %% + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * #L% + */ +package ca.uhn.fhir.batch2.jobs.merge; + +import ca.uhn.fhir.batch2.jobs.replacereferences.ReplaceReferencesProvenanceSvc; +import ca.uhn.fhir.jpa.api.dao.DaoRegistry; +import org.hl7.fhir.r4.model.CodeableConcept; + +public class MergeProvenanceSvc extends ReplaceReferencesProvenanceSvc { + + private static final String ACTIVITY_CODE_SYSTEM = "http://terminology.hl7.org/CodeSystem/iso-21089-lifecycle"; + private static final String ACTIVITY_CODE_MERGE = "merge"; + + public MergeProvenanceSvc(DaoRegistry theDaoRegistry) { + super(theDaoRegistry); + } + + @Override + protected CodeableConcept getActivityCodeableConcept() { + CodeableConcept retVal = new CodeableConcept(); + retVal.addCoding().setSystem(ACTIVITY_CODE_SYSTEM).setCode(ACTIVITY_CODE_MERGE); + return retVal; + } +} diff --git a/hapi-fhir-storage-batch2-jobs/src/main/java/ca/uhn/fhir/batch2/jobs/merge/MergeResourceHelper.java b/hapi-fhir-storage-batch2-jobs/src/main/java/ca/uhn/fhir/batch2/jobs/merge/MergeResourceHelper.java index 45064c8ec27..017a55142d0 100644 --- a/hapi-fhir-storage-batch2-jobs/src/main/java/ca/uhn/fhir/batch2/jobs/merge/MergeResourceHelper.java +++ b/hapi-fhir-storage-batch2-jobs/src/main/java/ca/uhn/fhir/batch2/jobs/merge/MergeResourceHelper.java @@ -20,20 +20,18 @@ package ca.uhn.fhir.batch2.jobs.merge; 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.api.model.DaoMethodOutcome; import ca.uhn.fhir.jpa.dao.tx.IHapiTransactionService; -import ca.uhn.fhir.model.api.TemporalPrecisionEnum; +import ca.uhn.fhir.model.primitive.IdDt; import ca.uhn.fhir.rest.api.server.RequestDetails; import ca.uhn.fhir.rest.server.provider.ProviderConstants; import jakarta.annotation.Nullable; -import org.hl7.fhir.instance.model.api.IIdType; import org.hl7.fhir.instance.model.api.IPrimitiveType; -import org.hl7.fhir.r4.model.CodeableConcept; +import org.hl7.fhir.r4.model.Bundle; import org.hl7.fhir.r4.model.Identifier; import org.hl7.fhir.r4.model.Patient; -import org.hl7.fhir.r4.model.Period; -import org.hl7.fhir.r4.model.Provenance; import org.hl7.fhir.r4.model.Reference; import java.util.Date; @@ -48,17 +46,12 @@ */ public class MergeResourceHelper { - private static final String ACTIVITY_CODE_SYSTEM = "http://terminology.hl7.org/CodeSystem/iso-21089-lifecycle"; - private static final String ACTIVITY_CODE_MERGE = "merge"; - private static final String ACT_REASON_CODE_SYSTEM = "http://terminology.hl7.org/CodeSystem/v3-ActReason"; - private static final String ACT_REASON_PATIENT_ADMINISTRATION_CODE = "PATADMIN"; - private final IFhirResourceDao myPatientDao; - private final IFhirResourceDao myProvenceDao; + private final MergeProvenanceSvc myProvenanceSvc; - public MergeResourceHelper(IFhirResourceDao thePatientDao, IFhirResourceDao theProvenanceDao) { - myPatientDao = thePatientDao; - myProvenceDao = theProvenanceDao; + public MergeResourceHelper(DaoRegistry theDaoRegistry, MergeProvenanceSvc theMergeProvenanceSvc) { + myPatientDao = theDaoRegistry.getResourceDao(Patient.class); + myProvenanceSvc = theMergeProvenanceSvc; } public static int setResourceLimitFromParameter( @@ -72,10 +65,11 @@ public static int setResourceLimitFromParameter( return retval; } - public void updateMergedResourcesAfterReferencesReplaced( + public void updateMergedResourcesAndCreateProvenance( IHapiTransactionService myHapiTransactionService, - IIdType theSourceResourceId, - IIdType theTargetResourceId, + IdDt theSourceResourceId, + IdDt theTargetResourceId, + List thePatchResultBundles, @Nullable Patient theResultResource, boolean theDeleteSource, RequestDetails theRequestDetails, @@ -83,20 +77,22 @@ public void updateMergedResourcesAfterReferencesReplaced( Patient sourceResource = myPatientDao.read(theSourceResourceId, theRequestDetails); Patient targetResource = myPatientDao.read(theTargetResourceId, theRequestDetails); - updateMergedResourcesAfterReferencesReplaced( + updateMergedResourcesAndCreateProvenance( myHapiTransactionService, sourceResource, targetResource, + thePatchResultBundles, theResultResource, theDeleteSource, theRequestDetails, theStartTime); } - public Patient updateMergedResourcesAfterReferencesReplaced( + public Patient updateMergedResourcesAndCreateProvenance( IHapiTransactionService myHapiTransactionService, Patient theSourceResource, Patient theTargetResource, + List thePatchResultBundles, @Nullable Patient theResultResource, boolean theDeleteSource, RequestDetails theRequestDetails, @@ -108,20 +104,20 @@ public Patient updateMergedResourcesAfterReferencesReplaced( theTargetResource, theSourceResource, theResultResource, theDeleteSource); targetPatientAfterUpdate.set(updateResource(patientToUpdate, theRequestDetails)); - + Patient sourcePatientAfterUpdate = null; if (theDeleteSource) { deleteResource(theSourceResource, theRequestDetails); } else { prepareSourcePatientForUpdate(theSourceResource, theTargetResource); - updateResource(theSourceResource, theRequestDetails); + sourcePatientAfterUpdate = updateResource(theSourceResource, theRequestDetails); } - createProvenance( - theSourceResource, - targetPatientAfterUpdate.get(), - theDeleteSource, - theRequestDetails, - theStartTime); + myProvenanceSvc.createProvenance( + targetPatientAfterUpdate.get().getIdElement(), + theDeleteSource ? null : sourcePatientAfterUpdate.getIdElement(), + thePatchResultBundles, + theStartTime, + theRequestDetails); }); return targetPatientAfterUpdate.get(); @@ -206,36 +202,4 @@ private Patient updateResource(Patient theResource, RequestDetails theRequestDet private void deleteResource(Patient theResource, RequestDetails theRequestDetails) { myPatientDao.delete(theResource.getIdElement(), theRequestDetails); } - - private void createProvenance( - Patient theSourcePatient, - Patient theTargetPatient, - boolean theDeleteSource, - RequestDetails theRequestDetails, - Date theStartTime) { - - Provenance provenance = new Provenance(); - provenance.addTarget().setReference(theTargetPatient.getIdElement().getValue()); - if (!theDeleteSource) { - provenance.addTarget().setReference(theSourcePatient.getIdElement().getValue()); - } - Date now = new Date(); - provenance.setOccurred(new Period() - .setStart(theStartTime, TemporalPrecisionEnum.MILLI) - .setEnd(now, TemporalPrecisionEnum.MILLI)); - provenance.setRecorded(now); - CodeableConcept activityCodeableConcept = new CodeableConcept(); - activityCodeableConcept.addCoding().setSystem(ACTIVITY_CODE_SYSTEM).setCode(ACTIVITY_CODE_MERGE); - provenance.setActivity(activityCodeableConcept); - - CodeableConcept activityReasonCodeableConcept = new CodeableConcept(); - activityReasonCodeableConcept - .addCoding() - .setSystem(ACT_REASON_CODE_SYSTEM) - .setCode(ACT_REASON_PATIENT_ADMINISTRATION_CODE); - provenance.addReason(activityReasonCodeableConcept); - - // TODO Emre: should we add agent - myProvenceDao.create(provenance, theRequestDetails); - } } diff --git a/hapi-fhir-storage-batch2-jobs/src/main/java/ca/uhn/fhir/batch2/jobs/merge/MergeUpdateTaskReducerStep.java b/hapi-fhir-storage-batch2-jobs/src/main/java/ca/uhn/fhir/batch2/jobs/merge/MergeUpdateTaskReducerStep.java index 0597afe5904..611937dde01 100644 --- a/hapi-fhir-storage-batch2-jobs/src/main/java/ca/uhn/fhir/batch2/jobs/merge/MergeUpdateTaskReducerStep.java +++ b/hapi-fhir-storage-batch2-jobs/src/main/java/ca/uhn/fhir/batch2/jobs/merge/MergeUpdateTaskReducerStep.java @@ -27,21 +27,25 @@ import ca.uhn.fhir.batch2.jobs.replacereferences.ReplaceReferenceResultsJson; import ca.uhn.fhir.batch2.jobs.replacereferences.ReplaceReferenceUpdateTaskReducerStep; import ca.uhn.fhir.jpa.api.dao.DaoRegistry; -import ca.uhn.fhir.jpa.api.dao.IFhirResourceDao; import ca.uhn.fhir.jpa.dao.tx.IHapiTransactionService; import ca.uhn.fhir.rest.api.server.SystemRequestDetails; import jakarta.annotation.Nonnull; import org.hl7.fhir.r4.model.Patient; -import org.hl7.fhir.r4.model.Provenance; import java.util.Date; public class MergeUpdateTaskReducerStep extends ReplaceReferenceUpdateTaskReducerStep { private final IHapiTransactionService myHapiTransactionService; + private final MergeResourceHelper myMergeResourceHelper; - public MergeUpdateTaskReducerStep(DaoRegistry theDaoRegistry, IHapiTransactionService theHapiTransactionService) { - super(theDaoRegistry); + public MergeUpdateTaskReducerStep( + DaoRegistry theDaoRegistry, + IHapiTransactionService theHapiTransactionService, + MergeResourceHelper theMergeResourceHelper, + MergeProvenanceSvc theMergeProvenanceSvc) { + super(theDaoRegistry, theMergeProvenanceSvc); this.myHapiTransactionService = theHapiTransactionService; + myMergeResourceHelper = theMergeResourceHelper; } @Nonnull @@ -63,19 +67,19 @@ public RunOutcome run( myFhirContext.newJsonParser().parseResource(Patient.class, mergeJobParameters.getResultResource()); } - IFhirResourceDao patientDao = myDaoRegistry.getResourceDao(Patient.class); - IFhirResourceDao provenanceDao = myDaoRegistry.getResourceDao(Provenance.class); - MergeResourceHelper helper = new MergeResourceHelper(patientDao, provenanceDao); - - helper.updateMergedResourcesAfterReferencesReplaced( + myMergeResourceHelper.updateMergedResourcesAndCreateProvenance( myHapiTransactionService, mergeJobParameters.getSourceId().asIdDt(), mergeJobParameters.getTargetId().asIdDt(), + getPatchOutputBundles(), resultResource, mergeJobParameters.getDeleteSource(), requestDetails, startTime); - return super.run(theStepExecutionDetails, theDataSink); + // setting createProvenance to false, because the provenance resource for merge has been created in the helper + // method above. The reason for that is merge updates target and source resources, unlike replace references, + // and we would like the merge provenance to reference the target and source versions after the update. + return super.run(theStepExecutionDetails, theDataSink, false); } } diff --git a/hapi-fhir-storage-batch2-jobs/src/main/java/ca/uhn/fhir/batch2/jobs/replacereferences/ReplaceReferenceUpdateTaskReducerStep.java b/hapi-fhir-storage-batch2-jobs/src/main/java/ca/uhn/fhir/batch2/jobs/replacereferences/ReplaceReferenceUpdateTaskReducerStep.java index 538b8f416e7..cf79cad3f5f 100644 --- a/hapi-fhir-storage-batch2-jobs/src/main/java/ca/uhn/fhir/batch2/jobs/replacereferences/ReplaceReferenceUpdateTaskReducerStep.java +++ b/hapi-fhir-storage-batch2-jobs/src/main/java/ca/uhn/fhir/batch2/jobs/replacereferences/ReplaceReferenceUpdateTaskReducerStep.java @@ -25,10 +25,12 @@ import ca.uhn.fhir.batch2.api.JobExecutionFailedException; import ca.uhn.fhir.batch2.api.RunOutcome; import ca.uhn.fhir.batch2.api.StepExecutionDetails; +import ca.uhn.fhir.batch2.jobs.chunk.FhirIdJson; import ca.uhn.fhir.batch2.model.ChunkOutcome; import ca.uhn.fhir.context.FhirContext; import ca.uhn.fhir.jpa.api.dao.DaoRegistry; import ca.uhn.fhir.jpa.api.dao.IFhirResourceDao; +import ca.uhn.fhir.rest.api.server.RequestDetails; import ca.uhn.fhir.rest.api.server.SystemRequestDetails; import jakarta.annotation.Nonnull; import org.hl7.fhir.r4.model.Bundle; @@ -46,13 +48,16 @@ public class ReplaceReferenceUpdateTaskReducerStep myTaskDao; + private final ReplaceReferencesProvenanceSvc myProvenanceSvc; private List myPatchOutputBundles = new ArrayList<>(); - public ReplaceReferenceUpdateTaskReducerStep(DaoRegistry theDaoRegistry) { + public ReplaceReferenceUpdateTaskReducerStep( + DaoRegistry theDaoRegistry, ReplaceReferencesProvenanceSvc theProvenanceSvc) { myDaoRegistry = theDaoRegistry; myTaskDao = myDaoRegistry.getResourceDao(Task.class); myFhirContext = theDaoRegistry.getFhirContext(); + myProvenanceSvc = theProvenanceSvc; } @Nonnull @@ -71,28 +76,29 @@ public RunOutcome run( @Nonnull StepExecutionDetails theStepExecutionDetails, @Nonnull IJobDataSink theDataSink) throws JobExecutionFailedException { + return run(theStepExecutionDetails, theDataSink, true); + } + + protected RunOutcome run( + @Nonnull StepExecutionDetails theStepExecutionDetails, + @Nonnull IJobDataSink theDataSink, + boolean theCreateProvenance) + throws JobExecutionFailedException { try { ReplaceReferencesJobParameters params = theStepExecutionDetails.getParameters(); SystemRequestDetails requestDetails = SystemRequestDetails.forRequestPartitionId(params.getPartitionId()); - Task task = myTaskDao.read(params.getTaskId().asIdDt(), requestDetails); - - task.setStatus(Task.TaskStatus.COMPLETED); - // TODO KHS this Task will probably be too large for large jobs. Revisit this model once we support - // Provenance - // resources. - myPatchOutputBundles.forEach(outputBundle -> { - Task.TaskOutputComponent output = task.addOutput(); - Coding coding = output.getType().getCodingFirstRep(); - coding.setSystem(RESOURCE_TYPES_SYSTEM); - coding.setCode("Bundle"); - Reference outputBundleReference = - new Reference("#" + outputBundle.getIdElement().getIdPart()); - output.setValue(outputBundleReference); - task.addContained(outputBundle); - }); - - myTaskDao.update(task, requestDetails); + + updateTask(params.getTaskId(), requestDetails); + + if (theCreateProvenance) { + myProvenanceSvc.createProvenance( + params.getTargetId().asIdDt(), + params.getSourceId().asIdDt(), + myPatchOutputBundles, + theStepExecutionDetails.getInstance().getStartTime(), + requestDetails); + } ReplaceReferenceResultsJson result = new ReplaceReferenceResultsJson(); result.setTaskId(params.getTaskId()); @@ -107,4 +113,28 @@ public RunOutcome run( myPatchOutputBundles.clear(); } } + + protected void updateTask(FhirIdJson theTaskId, RequestDetails theRequestDetails) { + Task task = myTaskDao.read(theTaskId.asIdDt(), theRequestDetails); + task.setStatus(Task.TaskStatus.COMPLETED); + + // TODO KHS this Task will probably be too large for large jobs. Revisit this model once we support + // Provenance resources. + myPatchOutputBundles.forEach(outputBundle -> { + Task.TaskOutputComponent output = task.addOutput(); + Coding coding = output.getType().getCodingFirstRep(); + coding.setSystem(RESOURCE_TYPES_SYSTEM); + coding.setCode("Bundle"); + Reference outputBundleReference = + new Reference("#" + outputBundle.getIdElement().getIdPart()); + output.setValue(outputBundleReference); + task.addContained(outputBundle); + }); + + myTaskDao.update(task, theRequestDetails); + } + + protected List getPatchOutputBundles() { + return myPatchOutputBundles; + } } diff --git a/hapi-fhir-storage-batch2-jobs/src/main/java/ca/uhn/fhir/batch2/jobs/replacereferences/ReplaceReferencesAppCtx.java b/hapi-fhir-storage-batch2-jobs/src/main/java/ca/uhn/fhir/batch2/jobs/replacereferences/ReplaceReferencesAppCtx.java index 62416d818a4..f490aeb6a0f 100644 --- a/hapi-fhir-storage-batch2-jobs/src/main/java/ca/uhn/fhir/batch2/jobs/replacereferences/ReplaceReferencesAppCtx.java +++ b/hapi-fhir-storage-batch2-jobs/src/main/java/ca/uhn/fhir/batch2/jobs/replacereferences/ReplaceReferencesAppCtx.java @@ -31,6 +31,7 @@ import org.hl7.fhir.r4.model.Task; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.Primary; @Configuration public class ReplaceReferencesAppCtx { @@ -82,8 +83,8 @@ public ReplaceReferenceUpdateStep replaceReferen @Bean public ReplaceReferenceUpdateTaskReducerStep replaceReferenceUpdateTaskStep( - DaoRegistry theDaoRegistry) { - return new ReplaceReferenceUpdateTaskReducerStep<>(theDaoRegistry); + DaoRegistry theDaoRegistry, ReplaceReferencesProvenanceSvc theReplaceReferencesProvenanceSvc) { + return new ReplaceReferenceUpdateTaskReducerStep<>(theDaoRegistry, theReplaceReferencesProvenanceSvc); } @Bean @@ -92,4 +93,10 @@ public ReplaceReferencesErrorHandler replaceRefe IFhirResourceDao taskDao = theDaoRegistry.getResourceDao(Task.class); return new ReplaceReferencesErrorHandler<>(theBatch2TaskHelper, taskDao); } + + @Primary + @Bean + public ReplaceReferencesProvenanceSvc replaceReferencesProvenanceSvc(DaoRegistry theDaoRegistry) { + return new ReplaceReferencesProvenanceSvc(theDaoRegistry); + } } diff --git a/hapi-fhir-storage-batch2-jobs/src/main/java/ca/uhn/fhir/batch2/jobs/replacereferences/ReplaceReferencesJobParameters.java b/hapi-fhir-storage-batch2-jobs/src/main/java/ca/uhn/fhir/batch2/jobs/replacereferences/ReplaceReferencesJobParameters.java index 75dff6080ad..3615aff5172 100644 --- a/hapi-fhir-storage-batch2-jobs/src/main/java/ca/uhn/fhir/batch2/jobs/replacereferences/ReplaceReferencesJobParameters.java +++ b/hapi-fhir-storage-batch2-jobs/src/main/java/ca/uhn/fhir/batch2/jobs/replacereferences/ReplaceReferencesJobParameters.java @@ -44,6 +44,9 @@ public class ReplaceReferencesJobParameters extends BatchJobParametersWithTaskId @JsonProperty("partitionId") private RequestPartitionId myPartitionId; + @JsonProperty(value = "createProvenance", defaultValue = "true", required = false) + private boolean myCreateProvenance; + public ReplaceReferencesJobParameters() {} public ReplaceReferencesJobParameters(ReplaceReferencesRequest theReplaceReferencesRequest, int theBatchSize) { @@ -53,6 +56,7 @@ public ReplaceReferencesJobParameters(ReplaceReferencesRequest theReplaceReferen // async case. myBatchSize = theBatchSize; myPartitionId = theReplaceReferencesRequest.partitionId; + myCreateProvenance = theReplaceReferencesRequest.createProvenance; } public FhirIdJson getSourceId() { @@ -91,6 +95,7 @@ public void setPartitionId(RequestPartitionId thePartitionId) { } public ReplaceReferencesRequest asReplaceReferencesRequest() { - return new ReplaceReferencesRequest(mySourceId.asIdDt(), myTargetId.asIdDt(), myBatchSize, myPartitionId); + return new ReplaceReferencesRequest( + mySourceId.asIdDt(), myTargetId.asIdDt(), myBatchSize, myPartitionId, myCreateProvenance); } } diff --git a/hapi-fhir-storage-batch2-jobs/src/main/java/ca/uhn/fhir/batch2/jobs/replacereferences/ReplaceReferencesProvenanceSvc.java b/hapi-fhir-storage-batch2-jobs/src/main/java/ca/uhn/fhir/batch2/jobs/replacereferences/ReplaceReferencesProvenanceSvc.java new file mode 100644 index 00000000000..8fb99a8c913 --- /dev/null +++ b/hapi-fhir-storage-batch2-jobs/src/main/java/ca/uhn/fhir/batch2/jobs/replacereferences/ReplaceReferencesProvenanceSvc.java @@ -0,0 +1,125 @@ +/*- + * #%L + * hapi-fhir-storage-batch2-jobs + * %% + * Copyright (C) 2014 - 2025 Smile CDR, Inc. + * %% + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * #L% + */ +package ca.uhn.fhir.batch2.jobs.replacereferences; + +import ca.uhn.fhir.jpa.api.dao.DaoRegistry; +import ca.uhn.fhir.jpa.api.dao.IFhirResourceDao; +import ca.uhn.fhir.model.api.TemporalPrecisionEnum; +import ca.uhn.fhir.rest.api.server.RequestDetails; +import jakarta.annotation.Nullable; +import org.hl7.fhir.instance.model.api.IIdType; +import org.hl7.fhir.r4.model.Bundle; +import org.hl7.fhir.r4.model.CodeableConcept; +import org.hl7.fhir.r4.model.Period; +import org.hl7.fhir.r4.model.Provenance; +import org.hl7.fhir.r4.model.Reference; + +import java.util.ArrayList; +import java.util.Date; +import java.util.List; + +public class ReplaceReferencesProvenanceSvc { + + private static final String ACT_REASON_CODE_SYSTEM = "http://terminology.hl7.org/CodeSystem/v3-ActReason"; + private static final String ACT_REASON_PATIENT_ADMINISTRATION_CODE = "PATADMIN"; + private final IFhirResourceDao myProvenanceDao; + + public ReplaceReferencesProvenanceSvc(DaoRegistry theDaoRegistry) { + myProvenanceDao = theDaoRegistry.getResourceDao(Provenance.class); + } + + @Nullable + protected CodeableConcept getActivityCodeableConcept() { + // FIXME KHS: return a codeable concepp suitable for replace-references + return null; + } + + protected Provenance createProvenanceObject( + Reference theTargetReference, + @Nullable Reference theSourceReference, + List theUpdatedReferencingResources, + Date theStartTime) { + Provenance provenance = new Provenance(); + + // FIXME KHS: add agent to the provenance + + Date now = new Date(); + provenance.setOccurred(new Period() + .setStart(theStartTime, TemporalPrecisionEnum.MILLI) + .setEnd(now, TemporalPrecisionEnum.MILLI)); + provenance.setRecorded(now); + + CodeableConcept activityCodeableConcept = getActivityCodeableConcept(); + if (activityCodeableConcept != null) { + provenance.setActivity(activityCodeableConcept); + } + CodeableConcept activityReasonCodeableConcept = new CodeableConcept(); + activityReasonCodeableConcept + .addCoding() + .setSystem(ACT_REASON_CODE_SYSTEM) + .setCode(ACT_REASON_PATIENT_ADMINISTRATION_CODE); + + provenance.addReason(activityReasonCodeableConcept); + + provenance.addTarget(theTargetReference); + if (theSourceReference != null) { + provenance.addTarget(theSourceReference); + } + + theUpdatedReferencingResources.forEach(provenance::addTarget); + return provenance; + } + + public void createProvenance( + IIdType theTargetId, + @Nullable IIdType theSourceId, + List thePatchResultBundles, + Date theStartTime, + RequestDetails theRequestDetails) { + + // FIXME KHS: should we be using the version specific source and target ID if the source and target ids + // passed in are not version specific? Currently the source and target ids passed in are not version specific + // for the replace-references current, but they are version specific for merge + Reference targetReference = new Reference(theTargetId); + Reference sourceReference = null; + if (theSourceId != null) { + sourceReference = new Reference(theSourceId); + } + List references = extractUpdatedResourceReferences(thePatchResultBundles); + Provenance provenance = createProvenanceObject(targetReference, sourceReference, references, theStartTime); + myProvenanceDao.create(provenance, theRequestDetails); + } + + protected List extractUpdatedResourceReferences(List thePatchBundles) { + List patchedResourceReferences = new ArrayList<>(); + thePatchBundles.forEach(outputBundle -> { + outputBundle.getEntry().forEach(entry -> { + if (entry.getResponse() != null && entry.getResponse().hasLocation()) { + // FIXME KHS: should we check here the patch result wasn't a no-op patch, and + // not include it if it was a no-op? It could be no-op patch because some other concurrent request + // updated the reference to the same reference that replace-references was supposed to update to. + Reference reference = new Reference(entry.getResponse().getLocation()); + patchedResourceReferences.add(reference); + } + }); + }); + return patchedResourceReferences; + } +} diff --git a/hapi-fhir-storage/src/main/java/ca/uhn/fhir/replacereferences/ReplaceReferencesRequest.java b/hapi-fhir-storage/src/main/java/ca/uhn/fhir/replacereferences/ReplaceReferencesRequest.java index 02bb8f783f2..25ee1427bfe 100644 --- a/hapi-fhir-storage/src/main/java/ca/uhn/fhir/replacereferences/ReplaceReferencesRequest.java +++ b/hapi-fhir-storage/src/main/java/ca/uhn/fhir/replacereferences/ReplaceReferencesRequest.java @@ -46,15 +46,19 @@ public class ReplaceReferencesRequest { public final RequestPartitionId partitionId; + public boolean createProvenance = true; + public ReplaceReferencesRequest( @Nonnull IIdType theSourceId, @Nonnull IIdType theTargetId, int theResourceLimit, - RequestPartitionId thePartitionId) { + RequestPartitionId thePartitionId, + boolean theCreateProvenance) { sourceId = theSourceId.toUnqualifiedVersionless(); targetId = theTargetId.toUnqualifiedVersionless(); resourceLimit = theResourceLimit; partitionId = thePartitionId; + createProvenance = theCreateProvenance; } public void validateOrThrowInvalidParameterException() { From b699e441736266eadf9922ce2c55b3eaffd416c4 Mon Sep 17 00:00:00 2001 From: Emre Dincturk Date: Fri, 10 Jan 2025 16:16:41 -0500 Subject: [PATCH 3/3] minor code cleanup --- .../merge/ResourceMergeServiceTest.java | 18 ++++++++++++++---- .../jpa/provider/r4/PatientMergeR4Test.java | 4 ++-- .../ReplaceReferencesBatchTest.java | 6 ++++++ .../ReplaceReferencesTestHelper.java | 12 +++++------- .../jobs/merge/MergeUpdateTaskReducerStep.java | 7 ++++--- 5 files changed, 31 insertions(+), 16 deletions(-) diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/merge/ResourceMergeServiceTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/merge/ResourceMergeServiceTest.java index f67dfd35ae0..d08340e13ac 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/merge/ResourceMergeServiceTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/merge/ResourceMergeServiceTest.java @@ -30,6 +30,7 @@ import org.hl7.fhir.r4.model.OperationOutcome; import org.hl7.fhir.r4.model.Parameters; import org.hl7.fhir.r4.model.Patient; +import org.hl7.fhir.r4.model.Period; import org.hl7.fhir.r4.model.Provenance; import org.hl7.fhir.r4.model.Reference; import org.hl7.fhir.r4.model.Task; @@ -46,6 +47,7 @@ import org.testcontainers.shaded.org.checkerframework.checker.nullness.qual.Nullable; import java.time.Instant; +import java.time.temporal.ChronoUnit; import java.util.Collections; import java.util.List; @@ -457,8 +459,8 @@ void testMerge_AsyncBecauseOfPreferHeader_Success(boolean theWithResultResource, "true, true", "false, false" }) - void testMerge_AsyncBecauseOfLargeNumberOfRefs_PreconditionFailed(boolean theWithResultResource, - boolean theWithDeleteSource) { + void testMerge_SyncRequest_ReplaceReferencesThrowsPreconditionFailedException_TheExceptionReturnedToClientInOutcome(boolean theWithResultResource, + boolean theWithDeleteSource) { // Given BaseMergeOperationInputParameters mergeOperationParameters = new PatientMergeOperationInputParameters(PAGE_SIZE); mergeOperationParameters.setSourceResource(new Reference(SOURCE_PATIENT_TEST_ID)); @@ -1439,14 +1441,22 @@ private void verifyProvenanceCreated(boolean theDeleteSource) { assertThat(provenance.getTarget()).hasSize(theDeleteSource ? 1 : 2); // the first target reference should be the target patient String targetPatientReference = provenance.getTarget().get(0).getReference(); - assertThat(provenance.getTarget().get(0).getReference()).isEqualTo(TARGET_PATIENT_TEST_ID_WITH_VERSION_2); + assertThat(targetPatientReference).isEqualTo(TARGET_PATIENT_TEST_ID_WITH_VERSION_2); if (!theDeleteSource) { // the second target reference should be the source patient, if it wasn't deleted String sourcePatientReference = provenance.getTarget().get(1).getReference(); assertThat(sourcePatientReference).isEqualTo(SOURCE_PATIENT_TEST_ID_WITH_VERSION_2); } - assertThat(provenance.getRecorded()).isCloseTo(Instant.now(), 60000); + Instant now = Instant.now(); + Instant oneMinuteAgo = now.minus(1, ChronoUnit.MINUTES); + assertThat(provenance.getRecorded()).isBetween(oneMinuteAgo, now, true, true); + + Period period = provenance.getOccurredPeriod(); + // since this is unit test and the test runs fast, the start time could be same as the end time + assertThat(period.getStart()).isBeforeOrEqualTo(period.getEnd()); + assertThat(period.getStart()).isBetween(oneMinuteAgo, now, true, true); + assertThat(period.getEnd()).isEqualTo(provenance.getRecorded()); // validate provenance.reason assertThat(provenance.getReason()).hasSize(1); diff --git a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/provider/r4/PatientMergeR4Test.java b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/provider/r4/PatientMergeR4Test.java index fa3c33462bc..6bf019dad67 100644 --- a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/provider/r4/PatientMergeR4Test.java +++ b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/provider/r4/PatientMergeR4Test.java @@ -257,10 +257,10 @@ void testMerge_smallResourceLimit() { "true, true", "false, true", }) - void testMerge_resourcesWithNoReferences(boolean theDeleteSource, boolean theAsync) { + void testMerge_sourceResourceWithoutAnyReference(boolean theDeleteSource, boolean theAsync) { Patient sourcePatient = new Patient(); - sourcePatient = (Patient )myPatientDao.create(sourcePatient, mySrd).getResource(); + sourcePatient = (Patient)myPatientDao.create(sourcePatient, mySrd).getResource(); Patient targetPatient = new Patient(); diff --git a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/replacereferences/ReplaceReferencesBatchTest.java b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/replacereferences/ReplaceReferencesBatchTest.java index 6779dfef9ac..af2b2d3cb0e 100644 --- a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/replacereferences/ReplaceReferencesBatchTest.java +++ b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/replacereferences/ReplaceReferencesBatchTest.java @@ -41,6 +41,11 @@ public class ReplaceReferencesBatchTest extends BaseJpaR4Test { public void before() throws Exception { super.before(); + // keep the version on Provenance.target fields to verify that Provenance resources were saved + // with versioned target references + myFhirContext.getParserOptions() + .setDontStripVersionsFromReferencesAtPaths("Provenance.target"); + myTestHelper = new ReplaceReferencesTestHelper(myFhirContext, myDaoRegistry); myTestHelper.beforeEach(); @@ -65,6 +70,7 @@ public void testHappyPath() { "Observation", "Encounter", "CarePlan")); myTestHelper.assertAllReferencesUpdated(); + myTestHelper.assertReplaceReferencesProvenance(); } diff --git a/hapi-fhir-jpaserver-test-utilities/src/main/java/ca/uhn/fhir/jpa/replacereferences/ReplaceReferencesTestHelper.java b/hapi-fhir-jpaserver-test-utilities/src/main/java/ca/uhn/fhir/jpa/replacereferences/ReplaceReferencesTestHelper.java index d439370cf2e..31be574ba02 100644 --- a/hapi-fhir-jpaserver-test-utilities/src/main/java/ca/uhn/fhir/jpa/replacereferences/ReplaceReferencesTestHelper.java +++ b/hapi-fhir-jpaserver-test-utilities/src/main/java/ca/uhn/fhir/jpa/replacereferences/ReplaceReferencesTestHelper.java @@ -231,10 +231,10 @@ public void assertReplaceReferencesProvenance() { // the first target reference should be the target patient String targetPatientReferenceInProvenance = provenance.getTarget().get(0).getReference(); - assertThat(targetPatientReferenceInProvenance.toString()).isEqualTo(myTargetPatientId.toString()); - // the second target reference should be the source patient, if it wasn't deleted + assertThat(targetPatientReferenceInProvenance).isEqualTo(myTargetPatientId.toString()); + // the second target reference should be the source patient String sourcePatientReference = provenance.getTarget().get(1).getReference(); - assertThat(sourcePatientReference.toString()).isEqualTo(mySourcePatientId.toString()); + assertThat(sourcePatientReference).isEqualTo(mySourcePatientId.toString()); Set allActualTargets = extractResourceIdsFromProvenanceTarget(provenance.getTarget()); assertThat(allActualTargets).containsAll(getExpectedProvenanceTargetsForPatchedResources()); @@ -256,7 +256,6 @@ public void assertReplaceReferencesProvenance() { assertThat(reasonCoding.getCode()).isEqualTo("PATADMIN"); // FIXME KHS: assert provenance activity code for replace-references - ; } public void assertMergeProvenance(boolean theDeleteSource) { @@ -288,12 +287,11 @@ public void assertMergeProvenance( // the first target reference should be the target patient String targetPatientReferenceInProvenance = provenance.getTarget().get(0).getReference(); - assertThat(targetPatientReferenceInProvenance.toString()) - .isEqualTo(theTargetPatientIdWithExpectedVersion.toString()); + assertThat(targetPatientReferenceInProvenance).isEqualTo(theTargetPatientIdWithExpectedVersion.toString()); if (!theDeleteSource) { // the second target reference should be the source patient, if it wasn't deleted String sourcePatientReference = provenance.getTarget().get(1).getReference(); - assertThat(sourcePatientReference.toString()).isEqualTo(theSourcePatientIdWithExpectedVersion.toString()); + assertThat(sourcePatientReference).isEqualTo(theSourcePatientIdWithExpectedVersion.toString()); } Set allActualTargets = extractResourceIdsFromProvenanceTarget(provenance.getTarget()); diff --git a/hapi-fhir-storage-batch2-jobs/src/main/java/ca/uhn/fhir/batch2/jobs/merge/MergeUpdateTaskReducerStep.java b/hapi-fhir-storage-batch2-jobs/src/main/java/ca/uhn/fhir/batch2/jobs/merge/MergeUpdateTaskReducerStep.java index 611937dde01..a02be2c1445 100644 --- a/hapi-fhir-storage-batch2-jobs/src/main/java/ca/uhn/fhir/batch2/jobs/merge/MergeUpdateTaskReducerStep.java +++ b/hapi-fhir-storage-batch2-jobs/src/main/java/ca/uhn/fhir/batch2/jobs/merge/MergeUpdateTaskReducerStep.java @@ -77,9 +77,10 @@ public RunOutcome run( requestDetails, startTime); - // setting createProvenance to false, because the provenance resource for merge has been created in the helper - // method above. The reason for that is merge updates target and source resources, unlike replace references, - // and we would like the merge provenance to reference the target and source versions after the update. + // Setting createProvenance to false. Because the provenance resource for merge has been created in the helper + // method above. The reason is that the merge operation updates the target and source resources, unlike replace + // references, and we would like the merge provenance to reference the target and source versions after the + // update. return super.run(theStepExecutionDetails, theDataSink, false); } }