diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/IResourceTableDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/IResourceTableDao.java index a081668cdae..5f37f70bf3b 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/IResourceTableDao.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/IResourceTableDao.java @@ -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 findLookupFieldsByResourcePid(@Param("pid") List 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 findLookupFieldsByResourcePidInPartitionIds( @Param("pid") List thePids, @Param("partition_id") Collection 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 findLookupFieldsByResourcePidInPartitionIdsOrNullPartition( @Param("pid") List thePids, @Param("partition_id") Collection 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 findLookupFieldsByResourcePidInPartitionNull(@Param("pid") List thePids); @Query("SELECT t.myVersion FROM ResourceTable t WHERE t.myId = :pid") diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/index/IdHelperService.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/index/IdHelperService.java index 9e81e87f598..6dd7e9a2fb7 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/index/IdHelperService.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/index/IdHelperService.java @@ -146,6 +146,8 @@ public class IdHelperService implements IIdHelperService { * 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 { } private Map>> translateForcedIdToPids( - @Nonnull RequestPartitionId theRequestPartitionId, Collection theId, boolean theExcludeDeleted) { - theId.forEach(id -> Validate.isTrue(id.hasIdPart())); + @Nonnull RequestPartitionId theRequestPartitionId, Collection 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 { RequestPartitionId requestPartitionId = replaceDefault(theRequestPartitionId); if (myStorageSettings.getResourceClientIdStrategy() != JpaStorageSettings.ClientIdStrategyEnum.ANY) { - List pids = theId.stream() - .filter(t -> isValidPid(t)) + List pids = theIds.stream() + .filter(IdHelperService::isValidPid) .map(IIdType::getIdPartAsLong) .collect(Collectors.toList()); if (!pids.isEmpty()) { @@ -530,7 +532,7 @@ public class IdHelperService implements IIdHelperService { } // returns a map of resourcetype->id - ListMultimap typeToIds = organizeIdsByResourceType(theId); + ListMultimap typeToIds = organizeIdsByResourceType(theIds); for (Map.Entry> nextEntry : typeToIds.asMap().entrySet()) { String nextResourceType = nextEntry.getKey(); Collection nextIds = nextEntry.getValue(); @@ -583,6 +585,7 @@ public class IdHelperService implements IIdHelperService { 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 { .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 { } } + /** + * @return true if we were given the PID of a resource that has a forced ID + */ + private boolean isResolvingPidOfResourceWithForcedId( + List thePidsToResolve, Long theFoundPid, String theFhirId) { + return thePidsToResolve.contains(theFoundPid) && !theFhirId.equals(String.valueOf(theFoundPid)); + } + @Override public PersistentIdToForcedIdMap translatePidsToForcedIds(Set theResourceIds) { assert myDontCheckActiveTransactionForUnitTest || TransactionSynchronizationManager.isSynchronizationActive(); @@ -725,7 +741,11 @@ public class IdHelperService implements IIdHelperService { 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); } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/model/cross/JpaResourceLookup.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/model/cross/JpaResourceLookup.java index 891617aa007..82ae36eb40b 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/model/cross/JpaResourceLookup.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/model/cross/JpaResourceLookup.java @@ -30,14 +30,17 @@ public class JpaResourceLookup implements IResourceLookup { 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 { return jpaPid; } + + public String getFhirId() { + return myFhirId; + } } diff --git a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/index/IdHelperServiceTest.java b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/index/IdHelperServiceTest.java index fa0870d4e67..7466fe34620 100644 --- a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/index/IdHelperServiceTest.java +++ b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/index/IdHelperServiceTest.java @@ -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 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 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 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"); diff --git a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4ReferentialIntegrityTest.java b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4ReferentialIntegrityTest.java index 8020f0556ea..07a93ceb650 100644 --- a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4ReferentialIntegrityTest.java +++ b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4ReferentialIntegrityTest.java @@ -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 { } - }