3960 when creating a resource patient where patientmetasource is provided with a hash the hash value is overwritten (#3965)

* Adding failing test.

* What was done:
-Meta source id is not replaced if provided.
-Added change log.

* Fixed code

* More tests for meta source

* Overwriting meta source can be configured.

* Test naming convention uplift and renaming of overwrite flag along with setter/getter

* Fixed when the source did not return if there was no resourceID

* Test naming convention uplift and renaming of overwrite flag along with setter/getter

* WIP in getting search on _source to be returning the correct number of patients.

* Custom requestId now stored on its own column

* Simplified getRequestId method in BaseHapiFhirDao

* Simplified getRequestId method in BaseHapiFhirDao

* An unused variable has been removed

Co-authored-by: peartree <etienne.poirier@smilecdr.com>

hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_2_0/3960-when-creating-a-resource-patient-where-patientmetasource-is-provided-with-a-hash-the-hash-value-is-overwritten.yaml

@@ -0,0 +1,5 @@
+---
+type: fix
+issue: 3960
+jira: SMILE-4679
+title: "Previously, when a client would provide a requestId within the source uri of a Meta.source, the provided requestId would get discarded and replaced by an id generated by the system. This has been corrected. And now it depends on configuration."

hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java

@@ -20,7 +20,6 @@ import ca.uhn.fhir.jpa.api.svc.IIdHelperService;
 import ca.uhn.fhir.jpa.api.svc.ISearchCoordinatorSvc;
 import ca.uhn.fhir.jpa.dao.data.IForcedIdDao;
 import ca.uhn.fhir.jpa.dao.data.IResourceHistoryTableDao;
-import ca.uhn.fhir.jpa.dao.data.IResourceProvenanceDao;
 import ca.uhn.fhir.jpa.dao.data.IResourceTableDao;
 import ca.uhn.fhir.jpa.dao.data.IResourceTagDao;
 import ca.uhn.fhir.jpa.dao.expunge.ExpungeService;
@@ -205,8 +204,6 @@ public abstract class BaseHapiFhirDao<T extends IBaseResource> extends BaseStora
 	@Autowired
 	protected IForcedIdDao myForcedIdDao;
 	@Autowired
-	protected IResourceProvenanceDao myResourceProvenanceDao;
-	@Autowired
 	protected ISearchCoordinatorSvc mySearchCoordinatorSvc;
 	@Autowired
 	protected ITermReadSvc myTerminologySvc;
@@ -1207,7 +1204,6 @@ public abstract class BaseHapiFhirDao<T extends IBaseResource> extends BaseStora
 				+ defaultString(provenanceRequestId);
 
 			MetaUtil.setSource(myContext, retVal, sourceString);
-
 		}
 
 		// 7. Add partition information
@@ -1626,7 +1622,7 @@ public abstract class BaseHapiFhirDao<T extends IBaseResource> extends BaseStora
 
 		// Save resource source
 		String source = null;
-		String requestId = theRequest != null ? theRequest.getRequestId() : null;
+
 		if (theResource != null) {
 			if (myContext.getVersion().getVersion().isEqualOrNewerThan(FhirVersionEnum.R4)) {
 				IBaseMetaType meta = theResource.getMeta();
@@ -1644,6 +1640,9 @@ public abstract class BaseHapiFhirDao<T extends IBaseResource> extends BaseStora
 			}
 		}
 
+		String requestId = getRequestId(theRequest, source);
+		source = cleanProvenanceSourceUri(source);
+
 		boolean haveSource = isNotBlank(source) && myConfig.getStoreMetaSourceInformation().isStoreSourceUri();
 		boolean haveRequestId = isNotBlank(requestId) && myConfig.getStoreMetaSourceInformation().isStoreRequestId();
 		if (haveSource || haveRequestId) {
@@ -1661,6 +1660,13 @@ public abstract class BaseHapiFhirDao<T extends IBaseResource> extends BaseStora
 		}
 	}
 
+	private String getRequestId(RequestDetails theRequest, String theSource) {
+		if (myConfig.isPreserveRequestIdInResourceBody()) {
+			return StringUtils.substringAfter(theSource, "#");
+		}
+		return theRequest != null ? theRequest.getRequestId() : null;
+	}
+
 	private void validateIncomingResourceTypeMatchesExisting(IBaseResource theResource, BaseHasResource entity) {
 		String resourceType = myContext.getResourceType(theResource);
 		if (!resourceType.equals(entity.getResourceType())) {

hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/IResourceHistoryProvenanceDao.java

@@ -1,18 +1,11 @@
 package ca.uhn.fhir.jpa.dao.data;
 
 import ca.uhn.fhir.jpa.model.entity.ResourceHistoryProvenanceEntity;
-import ca.uhn.fhir.jpa.model.entity.ResourceTable;
-import org.springframework.data.domain.Pageable;
-import org.springframework.data.domain.Slice;
 import org.springframework.data.jpa.repository.JpaRepository;
 import org.springframework.data.jpa.repository.Modifying;
 import org.springframework.data.jpa.repository.Query;
 import org.springframework.data.repository.query.Param;
 
-import java.util.Date;
-import java.util.List;
-import java.util.Map;
-
 /*
  * #%L
  * HAPI FHIR JPA Server
@@ -33,7 +26,7 @@ import java.util.Map;
  * #L%
  */
 
-public interface IResourceProvenanceDao extends JpaRepository<ResourceHistoryProvenanceEntity, Long>, IHapiFhirJpaRepository {
+public interface IResourceHistoryProvenanceDao extends JpaRepository<ResourceHistoryProvenanceEntity, Long>, IHapiFhirJpaRepository {
 
 	@Modifying
 	@Query("DELETE FROM ResourceHistoryProvenanceEntity t WHERE t.myId = :pid")

hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/expunge/ResourceExpungeService.java

@@ -41,7 +41,7 @@ import ca.uhn.fhir.jpa.dao.data.IResourceIndexedSearchParamStringDao;
 import ca.uhn.fhir.jpa.dao.data.IResourceIndexedSearchParamTokenDao;
 import ca.uhn.fhir.jpa.dao.data.IResourceIndexedSearchParamUriDao;
 import ca.uhn.fhir.jpa.dao.data.IResourceLinkDao;
-import ca.uhn.fhir.jpa.dao.data.IResourceProvenanceDao;
+import ca.uhn.fhir.jpa.dao.data.IResourceHistoryProvenanceDao;
 import ca.uhn.fhir.jpa.dao.data.IResourceTableDao;
 import ca.uhn.fhir.jpa.dao.data.IResourceTagDao;
 import ca.uhn.fhir.jpa.dao.data.ISearchParamPresentDao;
@@ -116,7 +116,7 @@ public class ResourceExpungeService implements IResourceExpungeService {
 	@Autowired
 	private DaoRegistry myDaoRegistry;
 	@Autowired
-	private IResourceProvenanceDao myResourceHistoryProvenanceTableDao;
+	private IResourceHistoryProvenanceDao myResourceHistoryProvenanceTableDao;
 	@Autowired
 	private ISearchParamPresentDao mySearchParamPresentDao;
 	@Autowired

hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4FilterTest.java

@@ -2,7 +2,7 @@ package ca.uhn.fhir.jpa.dao.r4;
 
 import ca.uhn.fhir.i18n.Msg;
 import ca.uhn.fhir.jpa.api.config.DaoConfig;
-import ca.uhn.fhir.jpa.dao.data.IResourceProvenanceDao;
+import ca.uhn.fhir.jpa.dao.data.IResourceHistoryProvenanceDao;
 import ca.uhn.fhir.jpa.searchparam.SearchParameterMap;
 import ca.uhn.fhir.jpa.test.BaseJpaR4Test;
 import ca.uhn.fhir.rest.api.Constants;
@@ -38,7 +38,7 @@ public class FhirResourceDaoR4FilterTest extends BaseJpaR4Test {
 
 	private static final Logger ourLog = LoggerFactory.getLogger(FhirResourceDaoR4FilterTest.class);
 	@Autowired
-	private IResourceProvenanceDao myResourceProvenanceDao;
+	private IResourceHistoryProvenanceDao myResourceProvenanceDao;
 
 	@AfterEach
 	public void after() {

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

@@ -2,9 +2,11 @@ package ca.uhn.fhir.jpa.provider.r4;
 
 import ca.uhn.fhir.i18n.Msg;
 import ca.uhn.fhir.jpa.api.config.DaoConfig;
+import ca.uhn.fhir.jpa.dao.data.IResourceHistoryProvenanceDao;
 import ca.uhn.fhir.jpa.dao.data.ISearchDao;
 import ca.uhn.fhir.jpa.entity.Search;
 import ca.uhn.fhir.jpa.model.entity.NormalizedQuantitySearchLevel;
+import ca.uhn.fhir.jpa.model.entity.ResourceHistoryProvenanceEntity;
 import ca.uhn.fhir.jpa.model.entity.ResourceHistoryTable;
 import ca.uhn.fhir.jpa.model.util.JpaConstants;
 import ca.uhn.fhir.jpa.model.util.UcumServiceUtil;
@@ -245,6 +247,8 @@ public class ResourceProviderR4Test extends BaseResourceProviderR4Test {
 
 		myClient.unregisterInterceptor(myCapturingInterceptor);
 		myDaoConfig.setUpdateWithHistoryRewriteEnabled(false);
+		myDaoConfig.setPreserveRequestIdInResourceBody(false);
+
 	}
 
 	@BeforeEach
@@ -7092,6 +7096,231 @@ public class ResourceProviderR4Test extends BaseResourceProviderR4Test {
 		}
 	}
 
+	@Test
+	public void createResource_withPreserveRequestIdEnabled_requestIdIsPreserved(){
+		myDaoConfig.setPreserveRequestIdInResourceBody(true);
+
+		String expectedMetaSource = "mySource#345676";
+		String patientId = "1234a";
+		Patient patient = new Patient();
+		patient.getMeta().setSource(expectedMetaSource);
+
+		patient.setId(patientId);
+		patient.addName().addGiven("Phil").setFamily("Sick");
+
+		MethodOutcome outcome = myClient.update().resource(patient).execute();
+
+		IIdType iIdType = outcome.getId();
+
+		Patient returnedPatient = myClient.read().resource(Patient.class).withId(iIdType).execute();
+
+		String returnedPatientMetaSource = returnedPatient.getMeta().getSource();
+
+		assertEquals(expectedMetaSource, returnedPatientMetaSource);
+	}
+
+	@Test
+	public void createResource_withPreserveRequestIdEnabledAndRequestIdLengthGT16_requestIdIsPreserved(){
+		myDaoConfig.setPreserveRequestIdInResourceBody(true);
+
+		String metaSource = "mySource#123456789012345678901234567890";
+		String expectedMetaSource = "mySource#1234567890123456";
+		String patientId = "1234a";
+		Patient patient = new Patient();
+		patient.getMeta().setSource(metaSource);
+
+		patient.setId(patientId);
+		patient.addName().addGiven("Phil").setFamily("Sick");
+
+		MethodOutcome outcome = myClient.update().resource(patient).execute();
+
+		IIdType iIdType = outcome.getId();
+
+		Patient returnedPatient = myClient.read().resource(Patient.class).withId(iIdType).execute();
+
+		String returnedPatientMetaSource = returnedPatient.getMeta().getSource();
+
+		assertEquals(expectedMetaSource, returnedPatientMetaSource);
+	}
+
+	@Test
+	public void createResource_withPreserveRequestIdDisabled_RequestIdIsOverwritten(){
+		String sourceURL = "mySource";
+		String requestId = "#345676";
+		String patientId = "1234a";
+		Patient patient = new Patient();
+		patient.getMeta().setSource(sourceURL + requestId);
+
+		patient.setId(patientId);
+		patient.addName().addGiven("Phil").setFamily("Sick");
+
+		MethodOutcome outcome = myClient.update().resource(patient).execute();
+
+		IIdType iIdType = outcome.getId();
+
+		Patient returnedPatient = myClient.read().resource(Patient.class).withId(iIdType).execute();
+
+		String returnedPatientMetaSource = returnedPatient.getMeta().getSource();
+
+		assertTrue(returnedPatientMetaSource.startsWith(sourceURL));
+		assertFalse(returnedPatientMetaSource.endsWith(requestId));
+	}
+
+	@Test
+	public void searchResource_bySourceAndRequestIdWithPreserveRequestIdEnabled_isSuccess(){
+		myDaoConfig.setPreserveRequestIdInResourceBody(true);
+
+		String sourceUri = "mySource";
+		String requestId = "345676";
+		String expectedSourceUrl = sourceUri + "#" + requestId;
+
+		Patient patient = new Patient();
+		patient.getMeta().setSource(expectedSourceUrl);
+
+		myClient
+			.create()
+			.resource(patient)
+			.execute();
+
+		Bundle results = myClient
+			.search()
+			.byUrl(ourServerBase + "/Patient?_source=" + sourceUri + "%23" + requestId)
+			.returnBundle(Bundle.class)
+			.execute();
+
+		Patient returnedPatient = (Patient) results.getEntry().get(0).getResource();
+		String returnedPatientMetaSource = returnedPatient.getMeta().getSource();
+
+		assertEquals(1, results.getEntry().size());
+		assertEquals(expectedSourceUrl, returnedPatientMetaSource);
+	}
+
+	@Test
+	public void searchResource_bySourceAndRequestIdWithPreserveRequestIdDisabled_fails(){
+		String sourceURI = "mySource";
+		String requestId = "345676";
+
+		Patient patient = new Patient();
+		patient.getMeta().setSource(sourceURI + "#" + requestId);
+
+		myClient
+			.create()
+			.resource(patient)
+			.execute();
+
+		Bundle results = myClient
+			.search()
+			.byUrl(ourServerBase + "/Patient?_source=" + sourceURI + "%23" + requestId)
+			.returnBundle(Bundle.class)
+			.execute();
+
+		assertEquals(0, results.getEntry().size());
+	}
+
+	@Test
+	public void searchResource_bySourceWithPreserveRequestIdDisabled_isSuccess() {
+		String sourceUri = "http://acme.org";
+		String requestId = "my-fragment";
+
+		Patient p1 = new Patient();
+		p1.getMeta().setSource(sourceUri + "#" + requestId);
+
+		myClient
+			.create()
+			.resource(p1)
+			.execute();
+
+		Bundle results = myClient
+			.search()
+			.byUrl(ourServerBase + "/Patient?_source=" + sourceUri)
+			.returnBundle(Bundle.class)
+			.execute();
+
+		Patient returnedPatient = (Patient) results.getEntry().get(0).getResource();
+		String returnedPatientMetaSource = returnedPatient.getMeta().getSource();
+
+		assertEquals(1, results.getEntry().size());
+		assertTrue(returnedPatientMetaSource.startsWith(sourceUri));
+		assertFalse(returnedPatientMetaSource.endsWith(requestId));
+	}
+
+	@Test
+	public void searchResource_bySourceWithPreserveRequestIdEnabled_isSuccess() {
+		myDaoConfig.setPreserveRequestIdInResourceBody(true);
+		String sourceUri = "http://acme.org";
+		String requestId = "my-fragment";
+		String expectedSourceUrl = sourceUri + "#" + requestId;
+
+		Patient p1 = new Patient();
+		p1.getMeta().setSource(expectedSourceUrl);
+
+		myClient
+			.create()
+			.resource(p1)
+			.execute();
+
+		Bundle results = myClient
+			.search()
+			.byUrl(ourServerBase + "/Patient?_source=" + sourceUri)
+			.returnBundle(Bundle.class)
+			.execute();
+
+		Patient returnedPatient = (Patient) results.getEntry().get(0).getResource();
+		String returnedPatientMetaSource = returnedPatient.getMeta().getSource();
+
+		assertEquals(1, results.getEntry().size());
+		assertEquals(expectedSourceUrl, returnedPatientMetaSource);
+	}
+
+	@Test
+	public void searchResource_byRequestIdWithPreserveRequestIdEnabled_isSuccess() {
+		myDaoConfig.setPreserveRequestIdInResourceBody(true);
+		String sourceUri = "http://acme.org";
+		String requestId = "my-fragment";
+		String expectedSourceUrl = sourceUri + "#" + requestId;
+
+		Patient patient = new Patient();
+		patient.getMeta().setSource(expectedSourceUrl);
+
+		myClient
+			.create()
+			.resource(patient)
+			.execute();
+
+		Bundle results = myClient
+			.search()
+			.byUrl(ourServerBase + "/Patient?_source=%23" + requestId)
+			.returnBundle(Bundle.class)
+			.execute();
+
+		assertEquals(1, results.getEntry().size());
+
+		Patient returnedPatient = (Patient) results.getEntry().get(0).getResource();
+		String returnedPatientMetaSource = returnedPatient.getMeta().getSource();
+
+		assertEquals(expectedSourceUrl, returnedPatientMetaSource);
+	}
+
+	@Test
+	public void searchResource_bySourceAndWrongRequestIdWithPreserveRequestIdEnabled_fails() {
+		myDaoConfig.setPreserveRequestIdInResourceBody(true);
+		Patient patient = new Patient();
+		patient.getMeta().setSource("urn:source:0#my-fragment123");
+
+		myClient
+			.create()
+			.resource(patient)
+			.execute();
+
+		Bundle results = myClient
+			.search()
+			.byUrl(ourServerBase + "/Patient?_source=%23my-fragment000")
+			.returnBundle(Bundle.class)
+			.execute();
+
+		assertEquals(0, results.getEntry().size());
+	}
+
 	@Nonnull
 	private IIdType createNewPatientWithHistory() {
 		String TEST_SYSTEM_NAME = "testHistoryRewrite";

hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/api/config/DaoConfig.java

@@ -319,6 +319,11 @@ public class DaoConfig {
 	 */
 	private boolean myUpdateWithHistoryRewriteEnabled = false;
 
+	/**
+	 * Since 6.2.0
+	 */
+	private boolean myPreserveRequestIdInResourceBody = false;
+
 	/**
 	 * Constructor
 	 */
@@ -2911,6 +2916,27 @@ public class DaoConfig {
 		myUpdateWithHistoryRewriteEnabled = theUpdateWithHistoryRewriteEnabled;
 	}
 
+	/**
+	 * This setting indicate whether a providedResource.meta.source requestID (source#requestID)
+	 * should be preserved or overwritten.
+	 *
+	 * @since 6.2.0
+	 */
+	public boolean isPreserveRequestIdInResourceBody() {
+		return myPreserveRequestIdInResourceBody;
+	}
+
+	/**
+	 * This setting indicate whether a providedResource.meta.source requestID (source#requestID)
+	 * should be preserved or overwritten.
+	 * Default is false. This means that a client provided requestId will be overwritten.
+	 *
+	 * @since 6.2.0
+	 */
+	public void setPreserveRequestIdInResourceBody(boolean thePreserveRequestIdInResourceBody) {
+		myPreserveRequestIdInResourceBody = thePreserveRequestIdInResourceBody;
+	}
+
 
 	public enum StoreMetaSourceInformationEnum {
 		NONE(false, false),