honeymoose/hapi-fhir
1
0
Fork 0
mirror of https://github.com/hapifhir/hapi-fhir.git synced 2025-03-25 01:18:37 +00:00
Code Issues Packages Projects Releases Wiki Activity

Disabling enforce referential integrity on write still checks references (#6219)

Browse Source 
* fix

* tests, changelog, spotless

* document tests better

* address review comments

* reset settings so other tests don't fail

* unused import

---------

Co-authored-by: jdar <justin.dar@smiledigitalhealth.com>
This commit is contained in:
jdar8 2024-09-06 10:16:49 -07:00 committed by GitHub
parent b4607c3628
commit 12765ac397
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 194 additions and 37 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_6_0/6218-fix-ref-integrity-on-write-disabled-still-checks-refs.yaml Normal file
View File

@ -0,0 +1,6 @@
---
type: fix
issue: 6218
jira: SMILE-8370
title: "Previously, when disabling the 'Enforce Referential Integrity on Write' setting, referential integrity was still
partially enforced when posting a resource with an invalid reference. This has now been fixed."

168
hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4ReferentialIntegrityTest.java
View File

@ -1,19 +1,26 @@
package ca.uhn.fhir.jpa.dao.r4;
import static org.junit.jupiter.api.Assertions.assertEquals;
import ca.uhn.fhir.i18n.Msg;
import ca.uhn.fhir.jpa.api.config.JpaStorageSettings;
import ca.uhn.fhir.jpa.test.BaseJpaR4Test;
import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException;
import ca.uhn.fhir.rest.server.exceptions.ResourceVersionConflictException;
import ca.uhn.fhir.rest.server.exceptions.UnprocessableEntityException;
import org.hl7.fhir.instance.model.api.IIdType;
import org.hl7.fhir.r4.model.IdType;
import org.hl7.fhir.r4.model.Observation;
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.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;
import static org.assertj.core.api.Assertions.assertThat;
import java.util.stream.Stream;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.fail;
public class FhirResourceDaoR4ReferentialIntegrityTest extends BaseJpaR4Test {
@ -23,51 +30,168 @@ public class FhirResourceDaoR4ReferentialIntegrityTest extends BaseJpaR4Test {
myStorageSettings.setEnforceReferentialIntegrityOnWrite(new JpaStorageSettings().isEnforceReferentialIntegrityOnWrite());
myStorageSettings.setEnforceReferentialIntegrityOnDelete(new JpaStorageSettings().isEnforceReferentialIntegrityOnDelete());
myStorageSettings.setEnforceReferenceTargetTypes(new JpaStorageSettings().isEnforceReferenceTargetTypes());
myStorageSettings.setResourceClientIdStrategy(new JpaStorageSettings().getResourceClientIdStrategy());
}
@ParameterizedTest
@MethodSource("paramsProvider_withResourceType")
public void referentialIntegrityOnWriteSetting_unknownIds_fullScopeTest(boolean theIsEnforceRefIntegrityEnabled,
JpaStorageSettings.ClientIdStrategyEnum theClientIdStrategy,
String theReferenceId) {
// Given
myStorageSettings.setEnforceReferentialIntegrityOnWrite(theIsEnforceRefIntegrityEnabled);
myStorageSettings.setEnforceReferenceTargetTypes(theIsEnforceRefIntegrityEnabled);
myStorageSettings.setResourceClientIdStrategy(theClientIdStrategy);
Patient p = new Patient();
p.setManagingOrganization(new Reference(theReferenceId));
if (theIsEnforceRefIntegrityEnabled) {
try {
// When
myPatientDao.create(p);
fail();
} catch (InvalidRequestException e) {
// Then
assertEquals(Msg.code(1094) + "Resource " + theReferenceId + " not found, specified in path: Patient.managingOrganization", e.getMessage());
}
} else {
// Disabled ref integrity on write case: all POSTs should succeed
// When
IIdType id = myPatientDao.create(p).getId().toUnqualifiedVersionless();
// Then
p = myPatientDao.read(id);
assertEquals(theReferenceId, p.getManagingOrganization().getReference());
}
}
private static Stream<Arguments> paramsProvider_withResourceType() {
// theIsEnforceRefIntegrityEnabled, theClientIdStrategy, theReferenceId
// note: client ID is tested since resolving the resource reference is different depending on the strategy
return Stream.of(
Arguments.of(true, JpaStorageSettings.ClientIdStrategyEnum.ALPHANUMERIC, "Organization/123"),
Arguments.of(true, JpaStorageSettings.ClientIdStrategyEnum.ALPHANUMERIC, "Organization/A123"),
Arguments.of(true, JpaStorageSettings.ClientIdStrategyEnum.ANY, "Organization/123"),
Arguments.of(true, JpaStorageSettings.ClientIdStrategyEnum.ANY, "Organization/A123"),
Arguments.of(false, JpaStorageSettings.ClientIdStrategyEnum.ALPHANUMERIC, "Organization/123"),
Arguments.of(false, JpaStorageSettings.ClientIdStrategyEnum.ALPHANUMERIC, "Organization/A123"),
Arguments.of(false, JpaStorageSettings.ClientIdStrategyEnum.ANY, "Organization/123"),
Arguments.of(false, JpaStorageSettings.ClientIdStrategyEnum.ANY, "Organization/A123")
);
}
@Test
public void testCreateUnknownReferenceFail() throws Exception {
public void testRefIntegrityOnWrite_withReferenceIdOfAnotherResourceType() {
// Given
myStorageSettings.setResourceClientIdStrategy(JpaStorageSettings.ClientIdStrategyEnum.ALPHANUMERIC);
// Create Observation with some ID...
Observation obs = new Observation();
IIdType obsId = myObservationDao.create(obs, mySrd).getId().toUnqualifiedVersionless();
// And reference a non-existing Organization resource with the same ID as the Observation
Patient p = new Patient();
p.setManagingOrganization(new Reference("Organization/AAA"));
p.setManagingOrganization(new Reference(new IdType("Organization", obsId.getIdPart())));
try {
// When
myPatientDao.create(p);
fail();
} catch (InvalidRequestException e) {
assertEquals(Msg.code(1094) + "Resource Organization/AAA not found, specified in path: Patient.managingOrganization", e.getMessage());
} catch (UnprocessableEntityException e) {
// Then: identify that it is the wrong resource type, since ref integrity is enabled
assertEquals(Msg.code(1095) + "Resource contains reference to unknown resource ID Organization/" + obsId.getIdPart(), e.getMessage());
}
// Given: now disable referential integrity on write
myStorageSettings.setEnforceReferentialIntegrityOnWrite(false);
// When
IIdType id = myPatientDao.create(p).getId().toUnqualifiedVersionless();
// Then: this should now succeed
p = myPatientDao.read(id);
assertEquals("Organization/" + obsId.getIdPart(), p.getManagingOrganization().getReference());
}
@Test
public void testCreateUnknownReferenceAllowed() {
myStorageSettings.setEnforceReferentialIntegrityOnWrite(false);
@ParameterizedTest
@MethodSource("paramsProvider_noResourceType")
public void testRefIntegrityOnWrite_withValidReferenceId_shouldAlwaysSucceed(boolean theIsEnforceRefIntegrityEnabled,
JpaStorageSettings.ClientIdStrategyEnum theClientIdStrategy,
String theReferenceId) {
// Given
myStorageSettings.setEnforceReferentialIntegrityOnWrite(theIsEnforceRefIntegrityEnabled);
myStorageSettings.setEnforceReferenceTargetTypes(theIsEnforceRefIntegrityEnabled);
myStorageSettings.setResourceClientIdStrategy(theClientIdStrategy);
Organization obs = new Organization();
obs.setId(theReferenceId);
myOrganizationDao.update(obs, mySrd);
String qualifiedReferenceId = "Organization/" + theReferenceId;
Patient p = new Patient();
p.setManagingOrganization(new Reference("Organization/AAA"));
p.setManagingOrganization(new Reference(qualifiedReferenceId));
// When
IIdType id = myPatientDao.create(p).getId().toUnqualifiedVersionless();
// Then
p = myPatientDao.read(id);
assertEquals("Organization/AAA", p.getManagingOrganization().getReference());
assertEquals(qualifiedReferenceId, p.getManagingOrganization().getReference());
}
@Test
public void testCreateUnknownReferenceAllowed_NumericId() {
myStorageSettings.setEnforceReferentialIntegrityOnWrite(false);
myStorageSettings.setEnforceReferenceTargetTypes(false);
private static Stream<Arguments> paramsProvider_noResourceType() {
// theIsEnforceRefIntegrityEnabled, theClientIdStrategy, theReferenceId
// note: client ID is tested since resolving the resource reference is different depending on the strategy
return Stream.of(
Arguments.of(true, JpaStorageSettings.ClientIdStrategyEnum.ALPHANUMERIC, "A123"),
Arguments.of(true, JpaStorageSettings.ClientIdStrategyEnum.ANY, "123"),
Arguments.of(true, JpaStorageSettings.ClientIdStrategyEnum.ANY, "A123"),
Arguments.of(false, JpaStorageSettings.ClientIdStrategyEnum.ALPHANUMERIC, "A123"),
Arguments.of(false, JpaStorageSettings.ClientIdStrategyEnum.ANY, "123"),
Arguments.of(false, JpaStorageSettings.ClientIdStrategyEnum.ANY, "A123")
);
}
@ParameterizedTest
@MethodSource("paramsProvider_noResourceType")
public void testRefIntegrityOnWrite_withReferenceIdOfDeletedResource(boolean theIsEnforceRefIntegrityEnabled,
JpaStorageSettings.ClientIdStrategyEnum theClientIdStrategy,
String theReferenceId) {
// Given
myStorageSettings.setEnforceReferentialIntegrityOnWrite(theIsEnforceRefIntegrityEnabled);
myStorageSettings.setEnforceReferenceTargetTypes(theIsEnforceRefIntegrityEnabled);
myStorageSettings.setResourceClientIdStrategy(theClientIdStrategy);
Organization obs = new Organization();
obs.setId(theReferenceId);
IIdType obsId = myOrganizationDao.update(obs, mySrd).getId();
String qualifiedReferenceId = "Organization/" + theReferenceId;
myOrganizationDao.delete(obsId, mySrd);
Patient p = new Patient();
p.setManagingOrganization(new Reference("Organization/123"));
IIdType id = myPatientDao.create(p).getId().toUnqualifiedVersionless();
p.setManagingOrganization(new Reference(qualifiedReferenceId));
p = myPatientDao.read(id);
assertEquals("Organization/123", p.getManagingOrganization().getReference());
if (theIsEnforceRefIntegrityEnabled) {
try {
// When
myPatientDao.create(p);
fail();
} catch (InvalidRequestException e) {
// Then
assertEquals(Msg.code(1096) + "Resource " + qualifiedReferenceId + " is deleted, specified in path: Patient.managingOrganization", e.getMessage());
}
} else {
// Disabled ref integrity on write case: all POSTs should succeed
// When
IIdType id = myPatientDao.create(p).getId().toUnqualifiedVersionless();
// Then
p = myPatientDao.read(id);
assertEquals(qualifiedReferenceId, p.getManagingOrganization().getReference());
}
}
@Test
public void testDeleteFail() throws Exception {
public void testDeleteFail() {
Organization o = new Organization();
o.setName("FOO");
IIdType oid = myOrganizationDao.create(o).getId().toUnqualifiedVersionless();
@ -89,7 +213,7 @@ public class FhirResourceDaoR4ReferentialIntegrityTest extends BaseJpaR4Test {
}
@Test
public void testDeleteAllow() throws Exception {
public void testDeleteAllow() {
myStorageSettings.setEnforceReferentialIntegrityOnDelete(false);
Organization o = new Organization();

57
hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/dao/index/DaoResourceLinkResolver.java
View File

@ -134,7 +134,7 @@ public class DaoResourceLinkResolver<T extends IResourcePersistentId> implements
type, targetReference, idPart, theRequest, theTransactionDetails);
if (!createdTableOpt.isPresent()) {
if (myStorageSettings.isEnforceReferentialIntegrityOnWrite() == false) {
if (!myStorageSettings.isEnforceReferentialIntegrityOnWrite()) {
return null;
}
@ -150,20 +150,8 @@ public class DaoResourceLinkResolver<T extends IResourcePersistentId> implements
"Resolved resource of type {} as PID: {}",
resolvedResource.getResourceType(),
resolvedResource.getPersistentId());
if (!resourceType.equals(resolvedResource.getResourceType())) {
ourLog.error(
"Resource with PID {} was of type {} and wanted {}",
resolvedResource.getPersistentId(),
resourceType,
resolvedResource.getResourceType());
throw new UnprocessableEntityException(Msg.code(1095)
+ "Resource contains reference to unknown resource ID " + targetResourceId.getValue());
}
if (resolvedResource.getDeleted() != null) {
String resName = resolvedResource.getResourceType();
throw new InvalidRequestException(Msg.code(1096) + "Resource " + resName + "/" + idPart
+ " is deleted, specified in path: " + sourcePath);
if (!validateResolvedResourceOrThrow(resourceType, resolvedResource, targetResourceId, idPart, sourcePath)) {
return null;
}
if (persistentId == null) {
@ -182,6 +170,45 @@ public class DaoResourceLinkResolver<T extends IResourcePersistentId> implements
return resolvedResource;
}
/**
* Validates the resolved resource.
* If 'Enforce Referential Integrity on Write' is enabled:
* Throws <code>UnprocessableEntityException</code> when resource types do not match
* Throws <code>InvalidRequestException</code> when the resolved resource was deleted
* <p>
* Otherwise, return false when resource types do not match or resource was deleted
* and return true if the resolved resource is valid.
*/
private boolean validateResolvedResourceOrThrow(
String resourceType,
IResourceLookup resolvedResource,
IIdType targetResourceId,
String idPart,
String sourcePath) {
if (!resourceType.equals(resolvedResource.getResourceType())) {
ourLog.error(
"Resource with PID {} was of type {} and wanted {}",
resolvedResource.getPersistentId(),
resourceType,
resolvedResource.getResourceType());
if (!myStorageSettings.isEnforceReferentialIntegrityOnWrite()) {
return false;
}
throw new UnprocessableEntityException(Msg.code(1095)
+ "Resource contains reference to unknown resource ID " + targetResourceId.getValue());
}
if (resolvedResource.getDeleted() != null) {
if (!myStorageSettings.isEnforceReferentialIntegrityOnWrite()) {
return false;
}
String resName = resolvedResource.getResourceType();
throw new InvalidRequestException(Msg.code(1096) + "Resource " + resName + "/" + idPart
+ " is deleted, specified in path: " + sourcePath);
}
return true;
}
@Nullable
@Override
public IBaseResource loadTargetResource(