From 14253dd0e3112721692fb98a94a54922a4177ac4 Mon Sep 17 00:00:00 2001 From: MykolaMedynskyiSCDR <110495727+MykolaMedynskyiSCDR@users.noreply.github.com> Date: Mon, 19 Sep 2022 10:17:09 -0400 Subject: [PATCH] 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 --- ...-a-hash-the-hash-value-is-overwritten.yaml | 5 + .../ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java | 16 +- ...ava => IResourceHistoryProvenanceDao.java} | 9 +- .../dao/expunge/ResourceExpungeService.java | 4 +- .../dao/r4/FhirResourceDaoR4FilterTest.java | 4 +- .../provider/r4/ResourceProviderR4Test.java | 229 ++++++++++++++++++ .../ca/uhn/fhir/jpa/api/config/DaoConfig.java | 26 ++ 7 files changed, 276 insertions(+), 17 deletions(-) create mode 100644 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 rename hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/{IResourceProvenanceDao.java => IResourceHistoryProvenanceDao.java} (76%) diff --git a/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 b/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 new file mode 100644 index 00000000000..c0bffd03005 --- /dev/null +++ b/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." diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java index 31c904cd915..52565ce2cb3 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java +++ b/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 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 extends BaseStora + defaultString(provenanceRequestId); MetaUtil.setSource(myContext, retVal, sourceString); - } // 7. Add partition information @@ -1626,7 +1622,7 @@ public abstract class BaseHapiFhirDao 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 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 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())) { diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/IResourceProvenanceDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/IResourceHistoryProvenanceDao.java similarity index 76% rename from hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/IResourceProvenanceDao.java rename to hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/IResourceHistoryProvenanceDao.java index 4b77937676d..921dbe942fc 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/IResourceProvenanceDao.java +++ b/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, IHapiFhirJpaRepository { +public interface IResourceHistoryProvenanceDao extends JpaRepository, IHapiFhirJpaRepository { @Modifying @Query("DELETE FROM ResourceHistoryProvenanceEntity t WHERE t.myId = :pid") diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/expunge/ResourceExpungeService.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/expunge/ResourceExpungeService.java index 17ddf220008..1fa5665e57c 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/expunge/ResourceExpungeService.java +++ b/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 diff --git a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4FilterTest.java b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4FilterTest.java index 2c12e5a3a0f..c85101bd150 100644 --- a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4FilterTest.java +++ b/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() { diff --git a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderR4Test.java b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderR4Test.java index d3120f969fb..15acc7292cb 100644 --- a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderR4Test.java +++ b/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"; diff --git a/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/api/config/DaoConfig.java b/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/api/config/DaoConfig.java index 45a461152f2..6da8e161e27 100644 --- a/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/api/config/DaoConfig.java +++ b/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),