fix ref integrity when using pid on resource with forced id

This commit is contained in:
jdar 2024-11-13 10:25:14 -08:00
parent cdb0a9c480
commit fe9cf20271
5 changed files with 124 additions and 20 deletions

View File

@ -136,7 +136,7 @@ public interface IResourceTableDao
* is an object array, where the order matters (the array represents columns returned by the query). Be careful if you change this query in any way.
*/
@Query(
"SELECT t.myResourceType, t.myId, t.myDeleted, t.myPartitionIdValue, t.myPartitionDateValue FROM ResourceTable t WHERE t.myId IN (:pid)")
"SELECT t.myResourceType, t.myId, t.myFhirId, t.myDeleted, t.myPartitionIdValue, t.myPartitionDateValue FROM ResourceTable t WHERE t.myId IN (:pid)")
Collection<Object[]> findLookupFieldsByResourcePid(@Param("pid") List<Long> thePids);
/**
@ -144,7 +144,7 @@ public interface IResourceTableDao
* is an object array, where the order matters (the array represents columns returned by the query). Be careful if you change this query in any way.
*/
@Query(
"SELECT t.myResourceType, t.myId, t.myDeleted, t.myPartitionIdValue, t.myPartitionDateValue FROM ResourceTable t WHERE t.myId IN (:pid) AND t.myPartitionIdValue IN :partition_id")
"SELECT t.myResourceType, t.myId, t.myFhirId, t.myDeleted, t.myPartitionIdValue, t.myPartitionDateValue FROM ResourceTable t WHERE t.myId IN (:pid) AND t.myPartitionIdValue IN :partition_id")
Collection<Object[]> findLookupFieldsByResourcePidInPartitionIds(
@Param("pid") List<Long> thePids, @Param("partition_id") Collection<Integer> thePartitionId);
@ -153,7 +153,7 @@ public interface IResourceTableDao
* is an object array, where the order matters (the array represents columns returned by the query). Be careful if you change this query in any way.
*/
@Query(
"SELECT t.myResourceType, t.myId, t.myDeleted, t.myPartitionIdValue, t.myPartitionDateValue FROM ResourceTable t WHERE t.myId IN (:pid) AND (t.myPartitionIdValue IS NULL OR t.myPartitionIdValue IN :partition_id)")
"SELECT t.myResourceType, t.myId, t.myFhirId, t.myDeleted, t.myPartitionIdValue, t.myPartitionDateValue FROM ResourceTable t WHERE t.myId IN (:pid) AND (t.myPartitionIdValue IS NULL OR t.myPartitionIdValue IN :partition_id)")
Collection<Object[]> findLookupFieldsByResourcePidInPartitionIdsOrNullPartition(
@Param("pid") List<Long> thePids, @Param("partition_id") Collection<Integer> thePartitionId);
@ -162,7 +162,7 @@ public interface IResourceTableDao
* is an object array, where the order matters (the array represents columns returned by the query). Be careful if you change this query in any way.
*/
@Query(
"SELECT t.myResourceType, t.myId, t.myDeleted, t.myPartitionIdValue, t.myPartitionDateValue FROM ResourceTable t WHERE t.myId IN (:pid) AND t.myPartitionIdValue IS NULL")
"SELECT t.myResourceType, t.myId, t.myFhirId, t.myDeleted, t.myPartitionIdValue, t.myPartitionDateValue FROM ResourceTable t WHERE t.myId IN (:pid) AND t.myPartitionIdValue IS NULL")
Collection<Object[]> findLookupFieldsByResourcePidInPartitionNull(@Param("pid") List<Long> thePids);
@Query("SELECT t.myVersion FROM ResourceTable t WHERE t.myId = :pid")

View File

@ -146,6 +146,8 @@ public class IdHelperService implements IIdHelperService<JpaPid> {
* Given a forced ID, convert it to its Long value. Since you are allowed to use string IDs for resources, we need to
* convert those to the underlying Long values that are stored, for lookup and comparison purposes.
* Optionally filters out deleted resources.
* Note that when given a PID for a resource created with a forced ID (client-assigned ID), this method will throw
* @ResourceNotFoundException
*
* @throws ResourceNotFoundException If the ID can not be found
*/
@ -509,10 +511,10 @@ public class IdHelperService implements IIdHelperService<JpaPid> {
}
private Map<String, List<IResourceLookup<JpaPid>>> translateForcedIdToPids(
@Nonnull RequestPartitionId theRequestPartitionId, Collection<IIdType> theId, boolean theExcludeDeleted) {
theId.forEach(id -> Validate.isTrue(id.hasIdPart()));
@Nonnull RequestPartitionId theRequestPartitionId, Collection<IIdType> theIds, boolean theExcludeDeleted) {
theIds.forEach(id -> Validate.isTrue(id.hasIdPart()));
if (theId.isEmpty()) {
if (theIds.isEmpty()) {
return new HashMap<>();
}
@ -520,8 +522,8 @@ public class IdHelperService implements IIdHelperService<JpaPid> {
RequestPartitionId requestPartitionId = replaceDefault(theRequestPartitionId);
if (myStorageSettings.getResourceClientIdStrategy() != JpaStorageSettings.ClientIdStrategyEnum.ANY) {
List<Long> pids = theId.stream()
.filter(t -> isValidPid(t))
List<Long> pids = theIds.stream()
.filter(IdHelperService::isValidPid)
.map(IIdType::getIdPartAsLong)
.collect(Collectors.toList());
if (!pids.isEmpty()) {
@ -530,7 +532,7 @@ public class IdHelperService implements IIdHelperService<JpaPid> {
}
// returns a map of resourcetype->id
ListMultimap<String, String> typeToIds = organizeIdsByResourceType(theId);
ListMultimap<String, String> typeToIds = organizeIdsByResourceType(theIds);
for (Map.Entry<String, Collection<String>> nextEntry : typeToIds.asMap().entrySet()) {
String nextResourceType = nextEntry.getKey();
Collection<String> nextIds = nextEntry.getValue();
@ -583,6 +585,7 @@ public class IdHelperService implements IIdHelperService<JpaPid> {
JpaResourceLookup lookup = new JpaResourceLookup(
resourceType,
resourcePid,
forcedId,
deletedAt,
PartitionablePartitionId.with(partitionId, partitionDate));
retVal.computeIfAbsent(forcedId, id -> new ArrayList<>()).add(lookup);
@ -646,14 +649,19 @@ public class IdHelperService implements IIdHelperService<JpaPid> {
.map(t -> new JpaResourceLookup(
(String) t[0],
(Long) t[1],
(Date) t[2],
PartitionablePartitionId.with((Integer) t[3], (LocalDate) t[4])))
(String) t[2],
(Date) t[3],
PartitionablePartitionId.with((Integer) t[4], (LocalDate) t[5])))
.forEach(t -> {
String id = t.getPersistentId().toString();
if (!theTargets.containsKey(id)) {
theTargets.put(id, new ArrayList<>());
Long pid = t.getPersistentId().getId();
String fhirId = t.getFhirId();
if (!isResolvingPidOfResourceWithForcedId(thePidsToResolve, pid, fhirId)) {
if (!theTargets.containsKey(id)) {
theTargets.put(id, new ArrayList<>());
}
theTargets.get(id).add(t);
}
theTargets.get(id).add(t);
if (!myStorageSettings.isDeleteEnabled()) {
String nextKey = t.getPersistentId().toString();
myMemoryCacheService.putAfterCommit(
@ -663,6 +671,14 @@ public class IdHelperService implements IIdHelperService<JpaPid> {
}
}
/**
* @return true if we were given the PID of a resource that has a forced ID
*/
private boolean isResolvingPidOfResourceWithForcedId(
List<Long> thePidsToResolve, Long theFoundPid, String theFhirId) {
return thePidsToResolve.contains(theFoundPid) && !theFhirId.equals(String.valueOf(theFoundPid));
}
@Override
public PersistentIdToForcedIdMap<JpaPid> translatePidsToForcedIds(Set<JpaPid> theResourceIds) {
assert myDontCheckActiveTransactionForUnitTest || TransactionSynchronizationManager.isSynchronizationActive();
@ -725,7 +741,11 @@ public class IdHelperService implements IIdHelperService<JpaPid> {
if (!myStorageSettings.isDeleteEnabled()) {
JpaResourceLookup lookup = new JpaResourceLookup(
theResourceType, theJpaPid.getId(), theDeletedAt, theJpaPid.getPartitionablePartitionId());
theResourceType,
theJpaPid.getId(),
theForcedId,
theDeletedAt,
theJpaPid.getPartitionablePartitionId());
String nextKey = theJpaPid.toString();
myMemoryCacheService.putAfterCommit(MemoryCacheService.CacheEnum.RESOURCE_LOOKUP, nextKey, lookup);
}

View File

@ -30,14 +30,17 @@ public class JpaResourceLookup implements IResourceLookup<JpaPid> {
private final Long myResourcePid;
private final Date myDeletedAt;
private final PartitionablePartitionId myPartitionablePartitionId;
private final String myFhirId;
public JpaResourceLookup(
String theResourceType,
Long theResourcePid,
String theFhirId,
Date theDeletedAt,
PartitionablePartitionId thePartitionablePartitionId) {
myResourceType = theResourceType;
myResourcePid = theResourcePid;
myFhirId = theFhirId;
myDeletedAt = theDeletedAt;
myPartitionablePartitionId = thePartitionablePartitionId;
}
@ -59,4 +62,8 @@ public class JpaResourceLookup implements IResourceLookup<JpaPid> {
return jpaPid;
}
public String getFhirId() {
return myFhirId;
}
}

View File

@ -7,7 +7,11 @@ import ca.uhn.fhir.jpa.model.config.PartitionSettings;
import ca.uhn.fhir.jpa.model.cross.IResourceLookup;
import ca.uhn.fhir.jpa.model.dao.JpaPid;
import ca.uhn.fhir.jpa.util.MemoryCacheService;
import ca.uhn.fhir.rest.server.exceptions.ResourceNotFoundException;
import org.hl7.fhir.r4.model.Patient;
import static org.junit.jupiter.api.Assertions.fail;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
@ -121,7 +125,7 @@ public class IdHelperServiceTest {
}
@Test
public void resolveResourcePersistenIds_withForcedIdAndDeleteDisabled_returnsMap() {
public void resolveResourcePersistentIds_withForcedIdAndDeleteDisabled_returnsMap() {
RequestPartitionId partitionId = RequestPartitionId.allPartitions();
String resourceType = Patient.class.getSimpleName();
List<String> patientIdsToResolve = new ArrayList<>();
@ -149,8 +153,7 @@ public class IdHelperServiceTest {
for (String id : patientIdsToResolve) {
assertThat(map).containsKey(id);
}
assertThat(map).containsEntry("RED", red);
assertThat(map).containsEntry("BLUE", blue);
assertThat(map).containsExactlyInAnyOrderEntriesOf(Map.of("RED", red, "BLUE", blue));
}
@Test
@ -177,6 +180,50 @@ public class IdHelperServiceTest {
assertEquals(forcedIdView[3], result.getDeleted());
}
@Test
public void testResolveResourceResourceIdentity_withPersistentIdOfResourceWithForcedIdAndDefaultClientIdStrategy_returnsNotFound(){
RequestPartitionId partitionId = RequestPartitionId.fromPartitionIdAndName(1, "partition");
String resourceType = "Patient";
Object[] forcedIdView = new Object[6];
forcedIdView[0] = "Patient";
forcedIdView[1] = 1L;
forcedIdView[2] = "AAA";
forcedIdView[3] = null;
forcedIdView[4] = null;
forcedIdView[5] = null;
Collection<Object[]> testForcedIdViews = new ArrayList<>();
testForcedIdViews.add(forcedIdView);
when(myResourceTableDao.findLookupFieldsByResourcePidInPartitionIds(any(), any())).thenReturn(testForcedIdViews);
try {
// Search by the PID of the resource that has a client assigned FHIR Id
myHelperService.resolveResourceIdentity(partitionId, resourceType, "1");
fail();
} catch(ResourceNotFoundException e) {
assertThat(e.getMessage()).isEqualTo("HAPI-2001: Resource Patient/1 is not known");
}
}
@Test
public void testResolveResourceResourceIdentity_withPersistentIdOfResourceWithForcedIdAndClientIdStrategyAny_returnsNotFound(){
when(myStorageSettings.getResourceClientIdStrategy()).thenReturn(JpaStorageSettings.ClientIdStrategyEnum.ANY);
RequestPartitionId partitionId = RequestPartitionId.fromPartitionIdAndName(1, "partition");
String resourceType = "Patient";
Collection<Object[]> testForcedIdViews = new ArrayList<>();
when(myResourceTableDao.findAndResolveByForcedIdWithNoTypeInPartition(any(), any(), any(), anyBoolean())).thenReturn(testForcedIdViews);
try {
// Search by the PID of the resource that has a client assigned FHIR Id
myHelperService.resolveResourceIdentity(partitionId, resourceType, "1");
fail();
} catch(ResourceNotFoundException e) {
assertThat(e.getMessage()).isEqualTo("HAPI-2001: Resource Patient/1 is not known");
}
}
@Test
public void testResolveResourcePersistentIds_mapDefaultFunctionality(){
RequestPartitionId partitionId = RequestPartitionId.fromPartitionIdAndName(1, "partition");

View File

@ -2,10 +2,14 @@ package ca.uhn.fhir.jpa.dao.r4;
import ca.uhn.fhir.i18n.Msg;
import ca.uhn.fhir.jpa.api.config.JpaStorageSettings;
import ca.uhn.fhir.jpa.api.model.DaoMethodOutcome;
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 static org.assertj.core.api.Assertions.assertThat;
import org.hl7.fhir.instance.model.api.IIdType;
import org.hl7.fhir.r4.model.IdType;
import org.hl7.fhir.r4.model.Observation;
@ -16,6 +20,10 @@ 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.EnumSource;
import static org.junit.jupiter.params.provider.EnumSource.Mode.EXCLUDE;
import org.junit.jupiter.params.provider.MethodSource;
import java.util.stream.Stream;
@ -190,6 +198,29 @@ public class FhirResourceDaoR4ReferentialIntegrityTest extends BaseJpaR4Test {
}
}
@ParameterizedTest
@EnumSource(value = JpaStorageSettings.ClientIdStrategyEnum.class, mode = EXCLUDE, names = {"NOT_ALLOWED"})
public void testReferentialIntegrityOnWrite_withReferenceByPidForClientAssignedIdResource(JpaStorageSettings.ClientIdStrategyEnum theClientIdStrategy) {
myStorageSettings.setResourceClientIdStrategy(theClientIdStrategy);
myStorageSettings.setEnforceReferentialIntegrityOnWrite(true);
Organization o = new Organization();
o.setName("FOO");
o.setId("O1");
DaoMethodOutcome outcome = myOrganizationDao.update(o);
Long organizationPid = (Long) outcome.getEntity().getPersistentId().getId();
Patient p = new Patient();
p.setManagingOrganization(new Reference("Organization/" + organizationPid));
try {
myPatientDao.create(p);
fail();
} catch (InvalidRequestException e) {
assertThat(e.getMessage()).contains("Resource Organization/" + organizationPid + " not found, specified in path: Patient.managingOrganization");
}
}
@Test
public void testDeleteFail() {
Organization o = new Organization();
@ -229,5 +260,4 @@ public class FhirResourceDaoR4ReferentialIntegrityTest extends BaseJpaR4Test {
}
}