From 4c0a71f70360e89b25689ed8c7bb8f8acdac9688 Mon Sep 17 00:00:00 2001 From: leif stawnyczy Date: Thu, 23 Jan 2025 15:35:02 -0500 Subject: [PATCH 1/5] allow deleting resources if references are all versioned only --- .../delete/DeleteConflictFinderService.java | 8 +- .../delete/DeleteConflictServiceR4Test.java | 150 +++++++++++++++++- 2 files changed, 151 insertions(+), 7 deletions(-) diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/delete/DeleteConflictFinderService.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/delete/DeleteConflictFinderService.java index 9816f786e513..0008cd1c6419 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/delete/DeleteConflictFinderService.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/delete/DeleteConflictFinderService.java @@ -35,10 +35,12 @@ public class DeleteConflictFinderService { protected EntityManager myEntityManager; List findConflicts(ResourceTable theEntity, int maxResults) { - TypedQuery query = myEntityManager.createQuery( - "SELECT l FROM ResourceLink l WHERE l.myTargetResource.myPid = :target_pid", ResourceLink.class); + String queryStr = "SELECT l FROM ResourceLink l WHERE l.myTargetResource.myPid = :target_pid AND (l.myTargetResourceVersion IS NULL)"; + TypedQuery query = myEntityManager.createQuery(queryStr, ResourceLink.class); query.setParameter("target_pid", theEntity.getId()); query.setMaxResults(maxResults); - return query.getResultList(); + List resourceLinks = query.getResultList(); + + return resourceLinks; } } diff --git a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/delete/DeleteConflictServiceR4Test.java b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/delete/DeleteConflictServiceR4Test.java index cbb90a688fd3..c15a7476ffcf 100644 --- a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/delete/DeleteConflictServiceR4Test.java +++ b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/delete/DeleteConflictServiceR4Test.java @@ -1,40 +1,49 @@ package ca.uhn.fhir.jpa.delete; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertNotNull; import ca.uhn.fhir.i18n.Msg; import ca.uhn.fhir.interceptor.api.Hook; import ca.uhn.fhir.interceptor.api.Pointcut; +import ca.uhn.fhir.jpa.api.model.DaoMethodOutcome; import ca.uhn.fhir.jpa.api.model.DeleteConflict; import ca.uhn.fhir.jpa.api.model.DeleteConflictList; import ca.uhn.fhir.jpa.test.BaseJpaR4Test; import ca.uhn.fhir.model.primitive.IdDt; +import ca.uhn.fhir.rest.api.server.SystemRequestDetails; import ca.uhn.fhir.rest.api.server.storage.TransactionDetails; import ca.uhn.fhir.rest.server.exceptions.ResourceVersionConflictException; import org.hl7.fhir.instance.model.api.IIdType; +import org.hl7.fhir.r4.model.Coding; import org.hl7.fhir.r4.model.Condition; +import org.hl7.fhir.r4.model.Encounter; +import org.hl7.fhir.r4.model.Observation; +import org.hl7.fhir.r4.model.OperationOutcome; import org.hl7.fhir.r4.model.Organization; import org.hl7.fhir.r4.model.Patient; import org.hl7.fhir.r4.model.Reference; 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.ValueSource; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import java.util.ArrayList; +import java.util.Date; import java.util.List; import java.util.function.Function; import static org.assertj.core.api.Assertions.assertThat; -import static org.junit.jupiter.api.Assertions.fail; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; public class DeleteConflictServiceR4Test extends BaseJpaR4Test { private static final Logger ourLog = LoggerFactory.getLogger(DeleteConflictServiceR4Test.class); - private final DeleteConflictInterceptor myDeleteInterceptor = new DeleteConflictInterceptor(); private int myInterceptorDeleteCount; @@ -204,6 +213,139 @@ public void testBadInterceptorNoInfiniteLoop() { assertEquals(1 + DeleteConflictService.MAX_RETRY_ATTEMPTS, myDeleteInterceptor.myCallCount); } + private void setupResourceReferenceTests() { + // we don't need this + myInterceptorRegistry.unregisterInterceptor(myDeleteInterceptor); + + // we have to allow versioned references; by default we do not + myFhirContext.getParserOptions() + .setStripVersionsFromReferences(false); + } + + @ParameterizedTest + @ValueSource(booleans = { true, false }) + public void delete_resourceReferencedByVersionedReferenceOnly_succeeds(boolean theUpdateObs) { + // setup + SystemRequestDetails requestDetails = new SystemRequestDetails(); + DaoMethodOutcome outcome; + + setupResourceReferenceTests(); + + Observation observation = new Observation(); + observation.setStatus(Observation.ObservationStatus.FINAL); + outcome = myObservationDao.create(observation, requestDetails); + IIdType obsId = outcome.getId(); + + // create encounter that references Observation by versioned reference only + Encounter encounter = new Encounter(); + encounter.setStatus(Encounter.EncounterStatus.FINISHED); + Coding coding = new Coding(); + coding.setSystem("http://terminology.hl7.org/ValueSet/v3-ActEncounterCode"); + coding.setCode("AMB"); + encounter.setClass_(coding); + encounter.addReasonReference() + .setReference(obsId.getValue()); // versioned reference + outcome = myEncounterDao.create(encounter, requestDetails); + assertTrue(outcome.getCreated()); + + if (theUpdateObs) { + observation.setId(obsId.toUnqualifiedVersionless()); + observation.setIssued(new Date()); + myObservationDao.update(observation, requestDetails); + } + + // test + outcome = myObservationDao.delete(obsId.toUnqualifiedVersionless(), requestDetails); + + // validate + assertTrue(outcome.getOperationOutcome() instanceof OperationOutcome); + OperationOutcome oo = (OperationOutcome) outcome.getOperationOutcome(); + assertFalse(oo.getIssue().isEmpty()); + assertTrue(oo.getIssue().stream().anyMatch(i -> i.getDiagnostics().contains("Successfully deleted 1 resource(s)"))); + } + + @ParameterizedTest + @ValueSource(booleans = { true, false }) + public void delete_resourceReferencedByNonVersionReferenceOnly_fails(boolean theUpdateObservation) { + // setup + SystemRequestDetails requestDetails = new SystemRequestDetails(); + DaoMethodOutcome outcome; + + setupResourceReferenceTests(); + + Observation observation = new Observation(); + observation.setStatus(Observation.ObservationStatus.FINAL); + outcome = myObservationDao.create(observation, requestDetails); + IIdType obsId = outcome.getId(); + + // create encounter that references Observation by versioned reference only + Encounter encounter = new Encounter(); + encounter.setStatus(Encounter.EncounterStatus.FINISHED); + Coding coding = new Coding(); + coding.setSystem("http://terminology.hl7.org/ValueSet/v3-ActEncounterCode"); + coding.setCode("AMB"); + encounter.setClass_(coding); + encounter.addReasonReference() + .setReference(obsId.toUnqualifiedVersionless().getValue()); // versionless reference + outcome = myEncounterDao.create(encounter, requestDetails); + assertTrue(outcome.getCreated()); + + if (theUpdateObservation) { + observation.setId(obsId.toUnqualifiedVersionless()); + observation.setIssued(new Date()); + myObservationDao.update(observation, requestDetails); + } + + // test + try { + myObservationDao.delete(obsId.toUnqualifiedVersionless(), requestDetails); + fail("Deletion of resource referenced by versionless id should fail."); + } catch (ResourceVersionConflictException ex) { + assertTrue(ex.getLocalizedMessage().contains("Unable to delete") + && ex.getLocalizedMessage().contains("at least one resource has a reference to this resource"), + ex.getLocalizedMessage()); + } + } + + @Test + public void delete_resourceReferencedByNonVersionedAndVersionedReferences_fails() { + // setup + SystemRequestDetails requestDetails = new SystemRequestDetails(); + DaoMethodOutcome outcome; + + setupResourceReferenceTests(); + + Observation observation = new Observation(); + observation.setStatus(Observation.ObservationStatus.FINAL); + outcome = myObservationDao.create(observation, requestDetails); + IIdType obsId = outcome.getId(); + + // create 2 encounters; one referenced with a versionless id, one referenced with a versioned id + for (String id : new String[] { obsId.getValue(), obsId.toUnqualifiedVersionless().getValue() }) { + Encounter encounter = new Encounter(); + encounter.setStatus(Encounter.EncounterStatus.FINISHED); + Coding coding = new Coding(); + coding.setSystem("http://terminology.hl7.org/ValueSet/v3-ActEncounterCode"); + coding.setCode("AMB"); + encounter.setClass_(coding); + encounter.addReasonReference() + .setReference(id); + outcome = myEncounterDao.create(encounter, requestDetails); + assertTrue(outcome.getCreated()); + } + + // test + try { + // versionless delete + myObservationDao.delete(obsId.toUnqualifiedVersionless(), requestDetails); + fail("Should not be able to delete observations referenced by versionless id because it is referenced by version AND by versionless."); + } catch (ResourceVersionConflictException ex) { + assertTrue(ex.getLocalizedMessage().contains("Unable to delete") + && ex.getLocalizedMessage().contains("at least one resource has a reference to this resource"), + ex.getLocalizedMessage()); + } + } + @Test public void testNoDuplicateConstraintReferences() { Patient patient = new Patient(); From f13e532011acd7ad817941a4ba15628d40d5942f Mon Sep 17 00:00:00 2001 From: leif stawnyczy Date: Thu, 23 Jan 2025 15:53:14 -0500 Subject: [PATCH 2/5] adding a changelog --- ...leting-resources-if-only-references-are-versioned.yaml | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_8_0/6644-allow-deleting-resources-if-only-references-are-versioned.yaml diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_8_0/6644-allow-deleting-resources-if-only-references-are-versioned.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_8_0/6644-allow-deleting-resources-if-only-references-are-versioned.yaml new file mode 100644 index 000000000000..df2ae259180e --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_8_0/6644-allow-deleting-resources-if-only-references-are-versioned.yaml @@ -0,0 +1,8 @@ +--- +type: add +issue: 6644 +jira: SMILE-9604 +title: "If references are allowed to contain versioned references, + and a resource is only referenced using versioned references, + DELETE actions will be permitted. +" From 36144eb92ce7f8668da7ed1c74feb90437a29947 Mon Sep 17 00:00:00 2001 From: leif stawnyczy Date: Thu, 23 Jan 2025 16:28:51 -0500 Subject: [PATCH 3/5] spotless --- .../ca/uhn/fhir/jpa/delete/DeleteConflictFinderService.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/delete/DeleteConflictFinderService.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/delete/DeleteConflictFinderService.java index 0008cd1c6419..e5101be84405 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/delete/DeleteConflictFinderService.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/delete/DeleteConflictFinderService.java @@ -35,7 +35,8 @@ public class DeleteConflictFinderService { protected EntityManager myEntityManager; List findConflicts(ResourceTable theEntity, int maxResults) { - String queryStr = "SELECT l FROM ResourceLink l WHERE l.myTargetResource.myPid = :target_pid AND (l.myTargetResourceVersion IS NULL)"; + String queryStr = + "SELECT l FROM ResourceLink l WHERE l.myTargetResource.myPid = :target_pid AND (l.myTargetResourceVersion IS NULL)"; TypedQuery query = myEntityManager.createQuery(queryStr, ResourceLink.class); query.setParameter("target_pid", theEntity.getId()); query.setMaxResults(maxResults); From 4b7ab5bfd6ac1723cd0c1dbe0e1c20c0fc76bd54 Mon Sep 17 00:00:00 2001 From: leif stawnyczy Date: Fri, 24 Jan 2025 09:53:36 -0500 Subject: [PATCH 4/5] reiew points --- ...eting-resources-if-only-references-are-versioned.yaml | 9 ++++++--- .../uhn/fhir/jpa/delete/DeleteConflictFinderService.java | 3 +-- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_8_0/6644-allow-deleting-resources-if-only-references-are-versioned.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_8_0/6644-allow-deleting-resources-if-only-references-are-versioned.yaml index df2ae259180e..229c34650779 100644 --- a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_8_0/6644-allow-deleting-resources-if-only-references-are-versioned.yaml +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_8_0/6644-allow-deleting-resources-if-only-references-are-versioned.yaml @@ -2,7 +2,10 @@ type: add issue: 6644 jira: SMILE-9604 -title: "If references are allowed to contain versioned references, - and a resource is only referenced using versioned references, - DELETE actions will be permitted. +title: "By default, referential integrity is enforced on deletes. + This change introduces support for an exceptional case such + that deletion of a given resource will not be blocked if all + references to that resource are versioned. If there is at + least one unversioned reference to the resource, deletion will + still be blocked. " diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/delete/DeleteConflictFinderService.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/delete/DeleteConflictFinderService.java index e5101be84405..97058e29d753 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/delete/DeleteConflictFinderService.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/delete/DeleteConflictFinderService.java @@ -40,8 +40,7 @@ List findConflicts(ResourceTable theEntity, int maxResults) { TypedQuery query = myEntityManager.createQuery(queryStr, ResourceLink.class); query.setParameter("target_pid", theEntity.getId()); query.setMaxResults(maxResults); - List resourceLinks = query.getResultList(); - return resourceLinks; + return query.getResultList(); } } From 921bd87e4f6a8acee8331a383a7af4db62360b26 Mon Sep 17 00:00:00 2001 From: leif stawnyczy Date: Fri, 24 Jan 2025 09:57:44 -0500 Subject: [PATCH 5/5] removing unneeded stub --- .../uhn/fhir/rest/server/interceptor/ConsentInterceptorTest.java | 1 - 1 file changed, 1 deletion(-) diff --git a/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/interceptor/ConsentInterceptorTest.java b/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/interceptor/ConsentInterceptorTest.java index 73a1f9ba1609..dee950281f5f 100644 --- a/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/interceptor/ConsentInterceptorTest.java +++ b/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/interceptor/ConsentInterceptorTest.java @@ -139,7 +139,6 @@ public void testOutcomeSuccess() throws IOException { when(myConsentSvc.startOperation(any(), any())).thenReturn(ConsentOutcome.PROCEED); when(myConsentSvc.canSeeResource(any(), any(), any())).thenReturn(ConsentOutcome.PROCEED); when(myConsentSvc.willSeeResource(any(), any(), any())).thenReturn(ConsentOutcome.PROCEED); - doNothing().when(myConsentSvc).completeOperationSuccess(any(), any()); HttpGet httpGet = new HttpGet("http://localhost:" + myPort + "/Patient");