Merge branch 'rel_8_0' into 6673-fixing-search-builder-with-fulltext-…

…search

hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_8_0/6689-replace-references-operation-skip-versioned-refs.yaml

@@ -0,0 +1,4 @@
+---
+type: change
+issue: 6689
+title: "$hapi.fhir.replace-references operation has been changed to not replace versioned references"

hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/provider/JpaSystemProvider.java

@@ -182,7 +182,7 @@ public IBaseParameters replaceReferences(
 		startRequest(theServletRequest);
 
 		try {
-			validateReplaceReferencesParams(theSourceId.getValue(), theTargetId.getValue());
+			validateReplaceReferencesParams(theSourceId, theTargetId);
 
 			int resourceLimit = MergeResourceHelper.setResourceLimitFromParameter(myStorageSettings, theResourceLimit);
 
@@ -205,13 +205,14 @@ public IBaseParameters replaceReferences(
 		}
 	}
 
-	private static void validateReplaceReferencesParams(String theSourceId, String theTargetId) {
-		if (isBlank(theSourceId)) {
+	private static void validateReplaceReferencesParams(
+			IPrimitiveType<String> theSourceId, IPrimitiveType<String> theTargetId) {
+		if (theSourceId == null || isBlank(theSourceId.getValue())) {
 			throw new InvalidRequestException(Msg.code(2583) + "Parameter '"
 					+ OPERATION_REPLACE_REFERENCES_PARAM_SOURCE_REFERENCE_ID + "' is blank");
 		}
 
-		if (isBlank(theTargetId)) {
+		if (theTargetId == null || isBlank(theTargetId.getValue())) {
 			throw new InvalidRequestException(Msg.code(2584) + "Parameter '"
 					+ OPERATION_REPLACE_REFERENCES_PARAM_TARGET_REFERENCE_ID + "' is blank");
 		}

hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/provider/r4/ReplaceReferencesR4Test.java

@@ -4,21 +4,28 @@
 import ca.uhn.fhir.jpa.api.config.JpaStorageSettings;
 import ca.uhn.fhir.jpa.provider.BaseResourceProviderR4Test;
 import ca.uhn.fhir.jpa.replacereferences.ReplaceReferencesTestHelper;
+import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException;
 import ca.uhn.fhir.rest.server.exceptions.PreconditionFailedException;
 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.Observation;
 import org.hl7.fhir.r4.model.Parameters;
+import org.hl7.fhir.r4.model.Practitioner;
+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.Task;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.NullSource;
 import org.junit.jupiter.params.provider.ValueSource;
 
 import java.util.List;
+import java.util.Set;
 
 import static ca.uhn.fhir.jpa.provider.ReplaceReferencesSvcImpl.RESOURCE_TYPES_SYSTEM;
 import static ca.uhn.fhir.jpa.replacereferences.ReplaceReferencesTestHelper.EXPECTED_SMALL_BATCHES;
@@ -29,6 +36,7 @@
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
 public class ReplaceReferencesR4Test extends BaseResourceProviderR4Test {
@@ -159,7 +167,127 @@ void testReplaceReferencesSmallTransactionEntriesSize() {
 		myTestHelper.assertAllReferencesUpdated();
 	}
 
-	// TODO ED we should add some tests for the invalid request error cases (and assert 4xx status code)
+	@ParameterizedTest
+	@ValueSource(strings = {""})
+	@NullSource
+	void testReplaceReferences_MissingSourceId_ThrowsInvalidRequestException(String theSourceId) {
+		InvalidRequestException exception = assertThrows(InvalidRequestException.class, () -> {
+			myTestHelper.callReplaceReferencesWithResourceLimit(myClient, theSourceId, "target-id", false, null);
+		});
+		assertThat(exception.getMessage()).contains("HAPI-2583: Parameter 'source-reference-id' is blank");
+	}
+
+	@ParameterizedTest
+	@ValueSource(strings = {""})
+	@NullSource
+	void testReplaceReferences_MissingTargetId_ThrowsInvalidRequestException(String theTargetId) {
+		InvalidRequestException exception = assertThrows(InvalidRequestException.class, () -> {
+			myTestHelper.callReplaceReferencesWithResourceLimit(myClient, "source-id", theTargetId, false, null);
+		});
+		assertThat(exception.getMessage()).contains("HAPI-2584: Parameter 'target-reference-id' is blank");
+	}
+
+
+	@Test
+	void testReplaceReferences_WhenReplacingAHighCardinalityReferenceElement_OnlyReplacesMatchingReferences() {
+		//This test uses an Observation resource with multiple Practitioner references in the 'performer' element.
+		// Create Practitioners
+		IIdType practitionerId1 = myClient.create().resource(new Practitioner()).execute().getId().toUnqualifiedVersionless();
+		IIdType practitionerId2 = myClient.create().resource(new Practitioner()).execute().getId().toUnqualifiedVersionless();
+		IIdType practitionerId3 = myClient.create().resource(new Practitioner()).execute().getId().toUnqualifiedVersionless();
+
+		// Create observation with references in the performer field
+		IIdType observationId = createObservationWithPerformers(practitionerId1, practitionerId2).toUnqualifiedVersionless();
+
+		// Call $replace-references operation to replace practitionerId1 with practitionerId3
+		Parameters outParams = myTestHelper.callReplaceReferencesWithResourceLimit(myClient,
+			practitionerId1.toString(),
+			practitionerId3.toString(),
+			false,
+			null);
+
+		// Assert operation outcome
+		Bundle patchResultBundle = (Bundle) outParams.getParameter(OPERATION_REPLACE_REFERENCES_OUTPUT_PARAM_OUTCOME).getResource();
+
+		ReplaceReferencesTestHelper.validatePatchResultBundle(patchResultBundle,
+			1, List.of(
+				"Observation"));
+
+		// Fetch and validate updated observation
+		Observation updatedObservation = myClient
+			.read()
+			.resource(Observation.class)
+			.withId(observationId)
+			.execute();
+
+		// Extract the performer references from the updated Observation
+		List<String> actualPerformerIds = updatedObservation.getPerformer().stream()
+			.map(ref -> ref.getReferenceElement().toString())
+			.toList();
+
+		// Assert that the performer references match the expected values
+		assertThat(actualPerformerIds).containsExactly(practitionerId3.toString(), practitionerId2.toString());
+	}
+
+	@Test
+	void testReplaceReferences_ShouldNotReplaceVersionedReferences() {
+		// this configuration makes preserve versioned references in the Provenance.target
+		// so that we can test that the versioned reference was not replaced
+		// but keep a copy of the original configuration to restore it after the test
+		Set<String> originalNotStrippedPaths =
+			myFhirContext.getParserOptions().getDontStripVersionsFromReferencesAtPaths();
+		myFhirContext.getParserOptions().setDontStripVersionsFromReferencesAtPaths("Provenance.target");
+		try {
+
+			IIdType practitionerId1 = myClient.create().resource(new Practitioner()).execute().getId().toUnqualified();
+			IIdType practitionerId2 = myClient.create().resource(new Practitioner()).execute().getId().toUnqualified();
+
+			Provenance provenance = new Provenance();
+			provenance.addTarget(new Reference(practitionerId1));
+			IIdType provenanceId = myClient.create().resource(provenance).execute().getId();
+			// Call $replace-references operation to replace practitionerId1 with practitionerId3
+			myTestHelper.callReplaceReferencesWithResourceLimit(myClient,
+				practitionerId1.toVersionless().toString(),
+				practitionerId2.toVersionless().toString(),
+				false,
+				null);
+
+			// Fetch and validate the provenance
+			Provenance provenanceAfterOperation = myClient
+				.read()
+				.resource(Provenance.class)
+				.withId(provenanceId.toUnqualifiedVersionless())
+				.execute();
+
+			// Extract the target references from the Provenance
+			List<String> actualTargetIds = provenanceAfterOperation.getTarget().stream()
+				.map(ref -> ref.getReferenceElement().toString())
+				.toList();
+
+			// Assert that the versioned reference in the Provenance  was not replaced
+			assertThat(actualTargetIds).containsExactly(practitionerId1.toString());
+
+		} finally {
+			myFhirContext.getParserOptions().setDontStripVersionsFromReferencesAtPaths(originalNotStrippedPaths);
+		}
+	}
+
+	private IIdType createObservationWithPerformers(IIdType... performerIds) {
+		// Create a new Observation resource
+		Observation observation = new Observation();
+
+		// Add references to performers
+		for (IIdType performerId : performerIds) {
+			observation.addPerformer(new Reference(performerId.toUnqualifiedVersionless()));
+		}
+
+		// Store the observation resource via the FHIR client
+		return myClient.create().resource(observation).execute().getId();
+
+	}
+
+
+
 
 	@Override
 	protected boolean verboseClientLogging() {

hapi-fhir-jpaserver-test-utilities/src/main/java/ca/uhn/fhir/jpa/replacereferences/ReplaceReferencesTestHelper.java

@@ -234,17 +234,31 @@ public Parameters callReplaceReferences(IGenericClient theFhirClient, boolean th
 
 	public Parameters callReplaceReferencesWithResourceLimit(
 			IGenericClient theFhirClient, boolean theIsAsync, Integer theResourceLimit) {
+		return callReplaceReferencesWithResourceLimit(
+				theFhirClient,
+				mySourcePatientId.getValue(),
+				myTargetPatientId.getValue(),
+				theIsAsync,
+				theResourceLimit);
+	}
+
+	public Parameters callReplaceReferencesWithResourceLimit(
+			IGenericClient theFhirClient,
+			String theSourceId,
+			String theTargetId,
+			boolean theIsAsync,
+			Integer theResourceLimit) {
 		IOperationUntypedWithInputAndPartialOutput<Parameters> request = theFhirClient
 				.operation()
 				.onServer()
 				.named(OPERATION_REPLACE_REFERENCES)
 				.withParameter(
 						Parameters.class,
 						ProviderConstants.OPERATION_REPLACE_REFERENCES_PARAM_SOURCE_REFERENCE_ID,
-						new StringType(mySourcePatientId.getValue()))
+						new StringType(theSourceId))
 				.andParameter(
 						ProviderConstants.OPERATION_REPLACE_REFERENCES_PARAM_TARGET_REFERENCE_ID,
-						new StringType(myTargetPatientId.getValue()));
+						new StringType(theTargetId));
 		if (theResourceLimit != null) {
 			request.andParameter(
 					ProviderConstants.OPERATION_REPLACE_REFERENCES_RESOURCE_LIMIT, new IntegerType(theResourceLimit));

hapi-fhir-storage/src/main/java/ca/uhn/fhir/replacereferences/ReplaceReferencesPatchBundleSvc.java

@@ -81,7 +81,6 @@ private Bundle buildPatchBundle(
 			List<IdDt> theResourceIds,
 			RequestDetails theRequestDetails) {
 		BundleBuilder bundleBuilder = new BundleBuilder(myFhirContext);
-
 		theResourceIds.forEach(referencingResourceId -> {
 			IFhirResourceDao<?> dao = myDaoRegistry.getResourceDao(referencingResourceId.getResourceType());
 			IBaseResource resource = dao.read(referencingResourceId, theRequestDetails);
@@ -101,7 +100,7 @@ private Bundle buildPatchBundle(
 						refInfo,
 						theReplaceReferencesRequest.sourceId)) // We only care about references to our source resource
 				.map(refInfo -> createReplaceReferencePatchOperation(
-						referencingResource.fhirType() + "." + refInfo.getName(),
+						getFhirPathForPatch(referencingResource, refInfo),
 						new Reference(theReplaceReferencesRequest.targetId.getValueAsString())))
 				.forEach(params::addParameter); // Add each operation to parameters
 		return params;
@@ -110,11 +109,33 @@ private Bundle buildPatchBundle(
 	private static boolean matches(ResourceReferenceInfo refInfo, IIdType theSourceId) {
 		return refInfo.getResourceReference()
 				.getReferenceElement()
-				.toUnqualifiedVersionless()
+				.toUnqualified()
 				.getValueAsString()
 				.equals(theSourceId.getValueAsString());
 	}
 
+	private String getFhirPathForPatch(IBaseResource theReferencingResource, ResourceReferenceInfo theRefInfo) {
+		// construct the path to the element containing the reference in the resource, e.g. "Observation.subject"
+		String path = theReferencingResource.fhirType() + "." + theRefInfo.getName();
+		// check the allowed cardinality of the element containing the reference
+		int maxCardinality = myFhirContext
+				.newTerser()
+				.getDefinition(theReferencingResource.getClass(), path)
+				.getMax();
+		if (maxCardinality != 1) {
+			// if the element allows high cardinality, specify the exact reference to replace by appending a where
+			// filter to the path. If we don't do this, all the existing references in the element would be lost as a
+			// result of getting replaced with the new reference, and that is not the behaviour we want.
+			// e.g. "Observation.performer.where(reference='Practitioner/123')"
+			return String.format(
+					"%s.where(reference='%s')",
+					path,
+					theRefInfo.getResourceReference().getReferenceElement().getValueAsString());
+		}
+		// the element allows max cardinality of 1, so the whole element can be safely replaced
+		return path;
+	}
+
 	@Nonnull
 	private Parameters.ParametersParameterComponent createReplaceReferencePatchOperation(
 			String thePath, Type theValue) {