From 69287b6bc3d9307ac8201e9cfcdb762f387412c3 Mon Sep 17 00:00:00 2001 From: leif stawnyczy Date: Tue, 7 Sep 2021 11:52:11 -0400 Subject: [PATCH] issue-2901 review fixes - moving methods to idhelperservice --- .../fhir/jpa/api/dao/IFhirResourceDao.java | 9 - .../ca/uhn/fhir/jpa/config/BaseConfig.java | 7 - .../jpa/dao/AutoVersioningServiceImpl.java | 44 ---- .../fhir/jpa/dao/BaseHapiFhirResourceDao.java | 49 ----- .../ca/uhn/fhir/jpa/dao/BaseStorageDao.java | 5 +- .../jpa/dao/BaseTransactionProcessor.java | 9 +- .../fhir/jpa/dao/IAutoVersioningService.java | 25 --- .../fhir/jpa/dao/index/IdHelperService.java | 89 ++++++++ .../dao/AutoVersioningServiceImplTests.java | 97 --------- .../jpa/dao/BaseHapiFhirResourceDaoTest.java | 166 -------------- .../jpa/dao/TransactionProcessorTest.java | 2 - .../jpa/dao/index/IdHelperServiceTest.java | 204 +++++++++++++++++- .../ca/uhn/fhir/jpa/dao/r4/BaseJpaR4Test.java | 3 - 13 files changed, 300 insertions(+), 409 deletions(-) delete mode 100644 hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/AutoVersioningServiceImpl.java delete mode 100644 hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/IAutoVersioningService.java delete mode 100644 hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/AutoVersioningServiceImplTests.java diff --git a/hapi-fhir-jpaserver-api/src/main/java/ca/uhn/fhir/jpa/api/dao/IFhirResourceDao.java b/hapi-fhir-jpaserver-api/src/main/java/ca/uhn/fhir/jpa/api/dao/IFhirResourceDao.java index 065149eef76..577faa0aee5 100644 --- a/hapi-fhir-jpaserver-api/src/main/java/ca/uhn/fhir/jpa/api/dao/IFhirResourceDao.java +++ b/hapi-fhir-jpaserver-api/src/main/java/ca/uhn/fhir/jpa/api/dao/IFhirResourceDao.java @@ -162,15 +162,6 @@ public interface IFhirResourceDao extends IDao { */ T read(IIdType theId); - /** - * Helper method to determine if some resources exist in the DB (without throwing). - * Returns a set that contains the IIdType for every resource found. - * If it's not found, it won't be included in the set. - * @param theIds - list of IIdType ids (for the same resource) - * @return - */ - Map getIdsOfExistingResources(RequestPartitionId partitionId, Collection theIds); - /** * Read a resource by its internal PID */ diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/config/BaseConfig.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/config/BaseConfig.java index fb58b1912e0..2aebff2b539 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/config/BaseConfig.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/config/BaseConfig.java @@ -27,11 +27,9 @@ import ca.uhn.fhir.jpa.bulk.imprt.api.IBulkDataImportSvc; import ca.uhn.fhir.jpa.bulk.imprt.svc.BulkDataImportSvcImpl; import ca.uhn.fhir.jpa.cache.IResourceVersionSvc; import ca.uhn.fhir.jpa.cache.ResourceVersionSvcDaoImpl; -import ca.uhn.fhir.jpa.dao.AutoVersioningServiceImpl; import ca.uhn.fhir.jpa.dao.DaoSearchParamProvider; import ca.uhn.fhir.jpa.dao.HistoryBuilder; import ca.uhn.fhir.jpa.dao.HistoryBuilderFactory; -import ca.uhn.fhir.jpa.dao.IAutoVersioningService; import ca.uhn.fhir.jpa.dao.ISearchBuilder; import ca.uhn.fhir.jpa.dao.LegacySearchBuilder; import ca.uhn.fhir.jpa.dao.MatchResourceUrlService; @@ -232,11 +230,6 @@ public abstract class BaseConfig { public void initSettings() { } - @Bean - public IAutoVersioningService autoVersioningService() { - return new AutoVersioningServiceImpl(); - } - public void setSearchCoordCorePoolSize(Integer searchCoordCorePoolSize) { this.searchCoordCorePoolSize = searchCoordCorePoolSize; } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/AutoVersioningServiceImpl.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/AutoVersioningServiceImpl.java deleted file mode 100644 index 151c9eb1191..00000000000 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/AutoVersioningServiceImpl.java +++ /dev/null @@ -1,44 +0,0 @@ -package ca.uhn.fhir.jpa.dao; - -import ca.uhn.fhir.interceptor.model.RequestPartitionId; -import ca.uhn.fhir.jpa.api.dao.DaoRegistry; -import ca.uhn.fhir.jpa.api.dao.IFhirResourceDao; -import ca.uhn.fhir.rest.api.server.storage.ResourcePersistentId; -import org.hl7.fhir.instance.model.api.IIdType; -import org.springframework.beans.factory.annotation.Autowired; - -import java.util.ArrayList; -import java.util.Collection; -import java.util.HashMap; -import java.util.List; -import java.util.Map; - -public class AutoVersioningServiceImpl implements IAutoVersioningService { - - @Autowired - private DaoRegistry myDaoRegistry; - - @Override - public Map getExistingAutoversionsForIds(RequestPartitionId theRequestPartitionId, Collection theIds) { - HashMap idToPID = new HashMap<>(); - HashMap> resourceTypeToIds = new HashMap<>(); - - for (IIdType id : theIds) { - String resourceType = id.getResourceType(); - if (!resourceTypeToIds.containsKey(resourceType)) { - resourceTypeToIds.put(resourceType, new ArrayList<>()); - } - resourceTypeToIds.get(resourceType).add(id); - } - - for (String resourceType : resourceTypeToIds.keySet()) { - IFhirResourceDao dao = myDaoRegistry.getResourceDao(resourceType); - - Map idAndPID = dao.getIdsOfExistingResources(theRequestPartitionId, - resourceTypeToIds.get(resourceType)); - idToPID.putAll(idAndPID); - } - - return idToPID; - } -} diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java index 8a19b774710..458c728c62e 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java @@ -1158,55 +1158,6 @@ public abstract class BaseHapiFhirResourceDao extends B return myTransactionService.execute(theRequest, transactionDetails, tx -> doRead(theId, theRequest, theDeletedOk)); } - @Override - public Map getIdsOfExistingResources(RequestPartitionId thePartitionId, - Collection theIds) { - // these are the found Ids that were in the db - HashMap collected = new HashMap<>(); - - if (theIds == null || theIds.isEmpty()) { - return collected; - } - - List resourcePersistentIds = myIdHelperService.resolveResourcePersistentIdsWithCache(thePartitionId, - theIds.stream().collect(Collectors.toList())); - - // we'll use this map to fetch pids that require versions - HashMap pidsToVersionToResourcePid = new HashMap<>(); - - // fill in our map - for (ResourcePersistentId pid : resourcePersistentIds) { - if (pid.getVersion() == null) { - pidsToVersionToResourcePid.put(pid.getIdAsLong(), pid); - } - Optional idOp = theIds.stream() - .filter(i -> i.getIdPart().equals(pid.getAssociatedResourceId().getIdPart())) - .findFirst(); - // this should always be present - // since it was passed in. - // but land of optionals... - idOp.ifPresent(id -> { - collected.put(id, pid); - }); - } - - // set any versions we don't already have - if (!pidsToVersionToResourcePid.isEmpty()) { - Collection resourceEntries = myResourceTableDao - .getResourceVersionsForPid(new ArrayList<>(pidsToVersionToResourcePid.keySet())); - - for (Object[] record : resourceEntries) { - // order matters! - Long retPid = (Long)record[0]; - String resType = (String)record[1]; - Long version = (Long)record[2]; - pidsToVersionToResourcePid.get(retPid).setVersion(version); - } - } - - return collected; - } - public T doRead(IIdType theId, RequestDetails theRequest, boolean theDeletedOk) { assert TransactionSynchronizationManager.isActualTransactionActive(); diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseStorageDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseStorageDao.java index 6df1d8fe44d..58aeae432ca 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseStorageDao.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseStorageDao.java @@ -30,6 +30,7 @@ import ca.uhn.fhir.jpa.api.config.DaoConfig; import ca.uhn.fhir.jpa.api.dao.DaoRegistry; import ca.uhn.fhir.jpa.api.model.DaoMethodOutcome; import ca.uhn.fhir.jpa.api.model.LazyDaoMethodOutcome; +import ca.uhn.fhir.jpa.dao.index.IdHelperService; import ca.uhn.fhir.jpa.model.cross.IBasePersistedResource; import ca.uhn.fhir.jpa.model.entity.ModelConfig; import ca.uhn.fhir.jpa.model.entity.ResourceTable; @@ -93,7 +94,7 @@ public abstract class BaseStorageDao { @Autowired protected ModelConfig myModelConfig; @Autowired - protected IAutoVersioningService myAutoVersioningService; + protected IdHelperService myIdHelperService; @Autowired protected DaoConfig myDaoConfig; @@ -210,7 +211,7 @@ public abstract class BaseStorageDao { IIdType referenceElement = nextReference.getReferenceElement(); if (!referenceElement.hasBaseUrl()) { - Map idToPID = myAutoVersioningService.getExistingAutoversionsForIds(RequestPartitionId.allPartitions(), + Map idToPID = myIdHelperService.getLatestVersionIdsForResourceIds(RequestPartitionId.allPartitions(), Collections.singletonList(referenceElement) ); diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseTransactionProcessor.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseTransactionProcessor.java index f8db7e92b11..d554b645514 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseTransactionProcessor.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseTransactionProcessor.java @@ -35,6 +35,7 @@ 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.api.model.DeleteMethodOutcome; +import ca.uhn.fhir.jpa.dao.index.IdHelperService; import ca.uhn.fhir.jpa.dao.tx.HapiTransactionService; import ca.uhn.fhir.jpa.delete.DeleteConflictService; import ca.uhn.fhir.jpa.model.cross.IBasePersistedResource; @@ -159,7 +160,7 @@ public abstract class BaseTransactionProcessor { private TaskExecutor myExecutor ; @Autowired - private IAutoVersioningService myAutoVersioningService; + private IdHelperService myIdHelperService; @VisibleForTesting public void setDaoConfig(DaoConfig theDaoConfig) { @@ -696,7 +697,7 @@ public abstract class BaseTransactionProcessor { * * @param theEntries */ - private void consolidateDuplicateConditionalCreates(List theEntries) { + private void consolidateDuplicateConditionals(List theEntries) { final HashMap keyToUuid = new HashMap<>(); for (int index = 0, originalIndex = 0; index < theEntries.size(); index++, originalIndex++) { IBase nextReqEntry = theEntries.get(index); @@ -832,7 +833,7 @@ public abstract class BaseTransactionProcessor { /* * Look for duplicate conditional creates and consolidate them */ - consolidateDuplicateConditionalCreates(theEntries); + consolidateDuplicateConditionals(theEntries); /* * Loop through the request and process any entries of type @@ -1270,7 +1271,7 @@ public abstract class BaseTransactionProcessor { // get a map of // existing ids -> PID (for resources that exist in the DB) // should this be allPartitions? - Map idToPID = myAutoVersioningService.getExistingAutoversionsForIds(RequestPartitionId.allPartitions(), + Map idToPID = myIdHelperService.getLatestVersionIdsForResourceIds(RequestPartitionId.allPartitions(), theReferencesToAutoVersion.stream() .map(ref -> ref.getReferenceElement()).collect(Collectors.toList())); diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/IAutoVersioningService.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/IAutoVersioningService.java deleted file mode 100644 index 80d327eb2d0..00000000000 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/IAutoVersioningService.java +++ /dev/null @@ -1,25 +0,0 @@ -package ca.uhn.fhir.jpa.dao; - -import ca.uhn.fhir.interceptor.model.RequestPartitionId; -import ca.uhn.fhir.rest.api.server.storage.ResourcePersistentId; -import org.hl7.fhir.instance.model.api.IIdType; - -import java.util.Collection; -import java.util.Map; - -public interface IAutoVersioningService { - /** - * Takes in a list of IIdType and returns a map of - * IIdType -> ResourcePersistentId. - * ResourcePersistentId will contain the history version - * but the returned IIdType will not (this is to allow consumers - * to use their passed in IIdType as a lookup value). - * - * If the returned map does not return an IIdType -> ResourcePersistentId - * then it means that it is a non-existing resource in the DB - * @param theIds - * @param thePartitionId - the partition id - * @return - */ - Map getExistingAutoversionsForIds(RequestPartitionId thePartitionId, Collection theIds); -} 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 b667e3a1375..03763d40a0f 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 @@ -530,6 +530,95 @@ public class IdHelperService { return retVal; } + /** + * Helper method to determine if some resources exist in the DB (without throwing). + * Returns a set that contains the IIdType for every resource found. + * If it's not found, it won't be included in the set. + * @param theIds - list of IIdType ids (for the same resource) + * @return + */ + private Map getIdsOfExistingResources(RequestPartitionId thePartitionId, + Collection theIds) { + // these are the found Ids that were in the db + HashMap collected = new HashMap<>(); + + if (theIds == null || theIds.isEmpty()) { + return collected; + } + + List resourcePersistentIds = resolveResourcePersistentIdsWithCache(thePartitionId, + theIds.stream().collect(Collectors.toList())); + + // we'll use this map to fetch pids that require versions + HashMap pidsToVersionToResourcePid = new HashMap<>(); + + // fill in our map + for (ResourcePersistentId pid : resourcePersistentIds) { + if (pid.getVersion() == null) { + pidsToVersionToResourcePid.put(pid.getIdAsLong(), pid); + } + Optional idOp = theIds.stream() + .filter(i -> i.getIdPart().equals(pid.getAssociatedResourceId().getIdPart())) + .findFirst(); + // this should always be present + // since it was passed in. + // but land of optionals... + idOp.ifPresent(id -> { + collected.put(id, pid); + }); + } + + // set any versions we don't already have + if (!pidsToVersionToResourcePid.isEmpty()) { + Collection resourceEntries = myResourceTableDao + .getResourceVersionsForPid(new ArrayList<>(pidsToVersionToResourcePid.keySet())); + + for (Object[] record : resourceEntries) { + // order matters! + Long retPid = (Long)record[0]; + String resType = (String)record[1]; + Long version = (Long)record[2]; + pidsToVersionToResourcePid.get(retPid).setVersion(version); + } + } + + return collected; + } + + /** + * Retrieves the latest versions for any resourceid that are found. + * If they are not found, they will not be contained in the returned map. + * The key should be the same value that was passed in to allow + * consumer to look up the value using the id they already have. + * + * This method should not throw, so it can safely be consumed in + * transactions. + * + * @param theRequestPartitionId - request partition id + * @param theIds - list of IIdTypes for resources of interest. + * @return + */ + public Map getLatestVersionIdsForResourceIds(RequestPartitionId theRequestPartitionId, Collection theIds) { + HashMap idToPID = new HashMap<>(); + HashMap> resourceTypeToIds = new HashMap<>(); + + for (IIdType id : theIds) { + String resourceType = id.getResourceType(); + if (!resourceTypeToIds.containsKey(resourceType)) { + resourceTypeToIds.put(resourceType, new ArrayList<>()); + } + resourceTypeToIds.get(resourceType).add(id); + } + + for (String resourceType : resourceTypeToIds.keySet()) { + Map idAndPID = getIdsOfExistingResources(theRequestPartitionId, + resourceTypeToIds.get(resourceType)); + idToPID.putAll(idAndPID); + } + + return idToPID; + } + /** * @deprecated This method doesn't take a partition ID as input, so it is unsafe. It * should be reworked to include the partition ID before any new use is incorporated diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/AutoVersioningServiceImplTests.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/AutoVersioningServiceImplTests.java deleted file mode 100644 index 206b6c968ec..00000000000 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/AutoVersioningServiceImplTests.java +++ /dev/null @@ -1,97 +0,0 @@ -package ca.uhn.fhir.jpa.dao; - -import ca.uhn.fhir.interceptor.model.RequestPartitionId; -import ca.uhn.fhir.jpa.api.dao.DaoRegistry; -import ca.uhn.fhir.jpa.api.dao.IFhirResourceDao; -import ca.uhn.fhir.model.primitive.IdDt; -import ca.uhn.fhir.rest.api.server.storage.ResourcePersistentId; -import org.hl7.fhir.instance.model.api.IIdType; -import org.junit.jupiter.api.Assertions; -import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.extension.ExtendWith; -import org.mockito.InjectMocks; -import org.mockito.Mock; -import org.mockito.Mockito; -import org.mockito.junit.jupiter.MockitoExtension; - -import java.util.Arrays; -import java.util.Collections; -import java.util.HashMap; -import java.util.Map; - -@ExtendWith(MockitoExtension.class) -public class AutoVersioningServiceImplTests { - - @Mock - private DaoRegistry daoRegistry; - - @InjectMocks - private AutoVersioningServiceImpl myAutoversioningService; - - @Test - public void getAutoversionsForIds_whenResourceExists_returnsMapWithPIDAndVersion() { - IIdType type = new IdDt("Patient/RED"); - ResourcePersistentId pid = new ResourcePersistentId(1); - pid.setVersion(2L); - HashMap map = new HashMap<>(); - map.put(type, pid); - - IFhirResourceDao daoMock = Mockito.mock(IFhirResourceDao.class); - Mockito.when(daoMock.getIdsOfExistingResources(Mockito.any(RequestPartitionId.class), - Mockito.anyList())) - .thenReturn(map); - Mockito.when(daoRegistry.getResourceDao(Mockito.anyString())) - .thenReturn(daoMock); - - Map retMap = myAutoversioningService.getExistingAutoversionsForIds(RequestPartitionId.allPartitions(), - Collections.singletonList(type)); - - Assertions.assertTrue(retMap.containsKey(type)); - Assertions.assertEquals(pid.getVersion(), map.get(type).getVersion()); - } - - @Test - public void getAutoversionsForIds_whenResourceDoesNotExist_returnsEmptyMap() { - IIdType type = new IdDt("Patient/RED"); - - IFhirResourceDao daoMock = Mockito.mock(IFhirResourceDao.class); - Mockito.when(daoMock.getIdsOfExistingResources(Mockito.any(RequestPartitionId.class), - Mockito.anyList())) - .thenReturn(new HashMap<>()); - Mockito.when(daoRegistry.getResourceDao(Mockito.anyString())) - .thenReturn(daoMock); - - Map retMap = myAutoversioningService.getExistingAutoversionsForIds(RequestPartitionId.allPartitions(), - Collections.singletonList(type)); - - Assertions.assertTrue(retMap.isEmpty()); - } - - @Test - public void getAutoversionsForIds_whenSomeResourcesDoNotExist_returnsOnlyExistingElements() { - IIdType type = new IdDt("Patient/RED"); - ResourcePersistentId pid = new ResourcePersistentId(1); - pid.setVersion(2L); - HashMap map = new HashMap<>(); - map.put(type, pid); - IIdType type2 = new IdDt("Patient/BLUE"); - - // when - IFhirResourceDao daoMock = Mockito.mock(IFhirResourceDao.class); - Mockito.when(daoMock.getIdsOfExistingResources(Mockito.any(RequestPartitionId.class), Mockito.anyList())) - .thenReturn(map); - Mockito.when(daoRegistry.getResourceDao(Mockito.anyString())) - .thenReturn(daoMock); - - // test - Map retMap = myAutoversioningService.getExistingAutoversionsForIds( - RequestPartitionId.allPartitions(), - Arrays.asList(type, type2) - ); - - // verify - Assertions.assertEquals(map.size(), retMap.size()); - Assertions.assertTrue(retMap.containsKey(type)); - Assertions.assertFalse(retMap.containsKey(type2)); - } -} diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDaoTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDaoTest.java index dd9cff05d3a..4936631f818 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDaoTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDaoTest.java @@ -1,188 +1,22 @@ package ca.uhn.fhir.jpa.dao; -import ca.uhn.fhir.context.FhirContext; -import ca.uhn.fhir.context.RuntimeResourceDefinition; -import ca.uhn.fhir.interceptor.model.RequestPartitionId; import ca.uhn.fhir.jpa.api.config.DaoConfig; -import ca.uhn.fhir.jpa.dao.data.IForcedIdDao; -import ca.uhn.fhir.jpa.dao.data.IResourceTableDao; -import ca.uhn.fhir.jpa.dao.index.IdHelperService; import ca.uhn.fhir.jpa.partition.SystemRequestDetails; -import ca.uhn.fhir.model.primitive.IdDt; import ca.uhn.fhir.rest.api.server.RequestDetails; -import ca.uhn.fhir.rest.api.server.storage.ResourcePersistentId; import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; import ca.uhn.fhir.rest.server.exceptions.ResourceNotFoundException; import ca.uhn.fhir.rest.server.servlet.ServletRequestDetails; -import org.hl7.fhir.instance.model.api.IIdType; import org.hl7.fhir.r4.model.Patient; -import org.hl7.fhir.r4.model.Request; -import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; -import org.mockito.InjectMocks; -import org.mockito.Mock; -import org.mockito.Mockito; -import org.mockito.junit.MockitoTestRule; import org.mockito.junit.jupiter.MockitoExtension; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collection; -import java.util.Collections; -import java.util.List; -import java.util.Map; - import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.fail; @ExtendWith(MockitoExtension.class) class BaseHapiFhirResourceDaoTest { - // our simple concrete test class for BaseHapiFhirResourceDao - private class SimpleTestDao extends BaseHapiFhirResourceDao { - public SimpleTestDao() { - super(); - // post inject hooks - setResourceType(Patient.class); - RuntimeResourceDefinition resourceDefinition = Mockito.mock(RuntimeResourceDefinition.class); - Mockito.when(resourceDefinition.getName()).thenReturn("Patient"); - FhirContext myFhirContextMock = Mockito.mock(FhirContext.class); - Mockito.when(myFhirContextMock.getResourceDefinition(Mockito.any(Class.class))) - .thenReturn(resourceDefinition); - setContext(myFhirContextMock); - postConstruct(); - } - - } - - @InjectMocks - private BaseHapiFhirResourceDao myBaseHapiFhirResourceDao = new SimpleTestDao(); - -// @Mock -// private IForcedIdDao myIForcedIdDao; - - @Mock - private IdHelperService myIdHelperService; - - @Mock - private IResourceTableDao myResourceTableDao; - - //TODO - all other dependency mocks - - - private ResourcePersistentId getResourcePersistentIdFromResource(IIdType theId, long thePid) { - ResourcePersistentId id = new ResourcePersistentId(thePid); - String idPortion = theId.getIdPart(); - IIdType newId = new IdDt(theId.getResourceType(), idPortion); - id.setAssociatedResourceId(newId); - return id; - } - - /** - * Gets a ResourceTable record for getResourceVersionsForPid - * Order matters! - * @param resourceType - * @param pid - * @param version - * @return - */ - private Object[] getResourceTableRecordForResourceTypeAndPid(String resourceType, long pid, long version) { - return new Object[] { - pid, // long - resourceType, // string - version // long - }; - } - - @Test - public void getIdsOfExistingResources_forExistingResources_returnsMapOfIdToPIDWithVersion() { - // setup - IIdType patientIdAndType = new IdDt("Patient/RED"); - long patientPID = 1L; - long patientResourceVersion = 2L; - ResourcePersistentId pat1ResourcePID = getResourcePersistentIdFromResource(patientIdAndType, patientPID); - IIdType patient2IdAndType = new IdDt("Patient/BLUE"); - long patient2PID = 3L; - long patient2ResourceVersion = 4L; - ResourcePersistentId pat2ResourcePID = getResourcePersistentIdFromResource(patient2IdAndType, patient2PID); - List inputList = new ArrayList<>(); - inputList.add(patientIdAndType); - inputList.add(patient2IdAndType); - - Collection matches = Arrays.asList( - getResourceTableRecordForResourceTypeAndPid(patientIdAndType.getResourceType(), patientPID, patientResourceVersion), - getResourceTableRecordForResourceTypeAndPid(patient2IdAndType.getResourceType(), patient2PID, patient2ResourceVersion) - ); - - // when - Mockito.when(myIdHelperService.resolveResourcePersistentIdsWithCache(Mockito.any(RequestPartitionId.class), - Mockito.anyList())) - .thenReturn(Arrays.asList(pat1ResourcePID, pat2ResourcePID)); - Mockito.when(myResourceTableDao.getResourceVersionsForPid(Mockito.anyList())) - .thenReturn(matches); - - Map idToPIDOfExistingResources = myBaseHapiFhirResourceDao.getIdsOfExistingResources(RequestPartitionId.allPartitions(), - inputList); - - Assertions.assertEquals(inputList.size(), idToPIDOfExistingResources.size()); - Assertions.assertTrue(idToPIDOfExistingResources.containsKey(patientIdAndType)); - Assertions.assertTrue(idToPIDOfExistingResources.containsKey(patient2IdAndType)); - - Assertions.assertEquals(patientPID, idToPIDOfExistingResources.get(patientIdAndType).getIdAsLong()); - Assertions.assertEquals(patient2PID, idToPIDOfExistingResources.get(patient2IdAndType).getIdAsLong(), patient2PID); - Assertions.assertEquals(patientResourceVersion, idToPIDOfExistingResources.get(patientIdAndType).getVersion()); - Assertions.assertEquals(patient2ResourceVersion, idToPIDOfExistingResources.get(patient2IdAndType).getVersion()); - } - - @Test - public void getIdsOfExistingResources_forNonExistentResources_returnsEmptyMap() { - //setup - IIdType patient = new IdDt("Patient/RED"); - - // when - Mockito.when(myIdHelperService.resolveResourcePersistentIdsWithCache(Mockito.any(RequestPartitionId.class), - Mockito.anyList())) - .thenReturn(new ArrayList<>()); - - Map map = myBaseHapiFhirResourceDao.getIdsOfExistingResources(RequestPartitionId.allPartitions(), - Collections.singletonList(patient)); - - Assertions.assertTrue(map.isEmpty()); - } - - @Test - public void getIdsOfExistingResources_whenSomeResourcesExist_returnsOnlyExistingResourcesInMap() { - // setup - IIdType patientIdAndType = new IdDt("Patient/RED"); - long patientPID = 1L; - long patientResourceVersion = 2L; - IIdType patient2IdAndType = new IdDt("Patient/BLUE"); - List inputList = new ArrayList<>(); - inputList.add(patientIdAndType); - inputList.add(patient2IdAndType); - - // when - Mockito.when(myIdHelperService - .resolveResourcePersistentIdsWithCache(Mockito.any(RequestPartitionId.class), - Mockito.anyList())) - .thenReturn(Collections.singletonList(getResourcePersistentIdFromResource(patientIdAndType, patientPID))); - Mockito.when(myResourceTableDao.getResourceVersionsForPid(Mockito.anyList())) - .thenReturn(Collections - .singletonList(getResourceTableRecordForResourceTypeAndPid(patientIdAndType.getResourceType(), patientPID, patientResourceVersion))); - - Map map = myBaseHapiFhirResourceDao.getIdsOfExistingResources(RequestPartitionId.allPartitions(), - inputList); - - // verify - Assertions.assertFalse(map.isEmpty()); - Assertions.assertEquals(inputList.size() - 1, map.size()); - Assertions.assertTrue(map.containsKey(patientIdAndType)); - Assertions.assertFalse(map.containsKey(patient2IdAndType)); - } - - /*******************/ - TestResourceDao mySvc = new TestResourceDao(); @Test diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/TransactionProcessorTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/TransactionProcessorTest.java index 907650ba59b..0bace079a8f 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/TransactionProcessorTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/TransactionProcessorTest.java @@ -70,8 +70,6 @@ public class TransactionProcessorTest { private MatchUrlService myMatchUrlService; @MockBean private IRequestPartitionHelperSvc myRequestPartitionHelperSvc; - @MockBean - private IAutoVersioningService myAutoVersioningService; @MockBean(answer = Answers.RETURNS_DEEP_STUBS) private SessionImpl mySession; diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/index/IdHelperServiceTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/index/IdHelperServiceTest.java index 20e100065f1..ad9fbc78c44 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/index/IdHelperServiceTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/index/IdHelperServiceTest.java @@ -1,13 +1,215 @@ package ca.uhn.fhir.jpa.dao.index; import ca.uhn.fhir.interceptor.model.RequestPartitionId; +import ca.uhn.fhir.jpa.api.config.DaoConfig; +import ca.uhn.fhir.jpa.dao.data.IResourceTableDao; import ca.uhn.fhir.jpa.model.config.PartitionSettings; +import ca.uhn.fhir.jpa.model.entity.ForcedId; +import ca.uhn.fhir.jpa.util.MemoryCacheService; +import ca.uhn.fhir.model.primitive.IdDt; +import ca.uhn.fhir.rest.api.server.storage.ResourcePersistentId; +import org.hl7.fhir.instance.model.api.IIdType; +import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.Mockito; +import org.mockito.junit.jupiter.MockitoExtension; -import static org.junit.jupiter.api.Assertions.*; +import javax.persistence.EntityManager; +import javax.persistence.TypedQuery; +import javax.persistence.criteria.CriteriaBuilder; +import javax.persistence.criteria.CriteriaQuery; +import javax.persistence.criteria.Path; +import javax.persistence.criteria.Root; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertSame; + +@ExtendWith(MockitoExtension.class) public class IdHelperServiceTest { + // helper class to package up data for helper methods + private class ResourceIdPackage { + public IIdType MyResourceId; + public ResourcePersistentId MyPid; + public Long MyVersion; + + public ResourceIdPackage(IIdType id, + ResourcePersistentId pid, + Long version) { + MyResourceId = id; + MyPid = pid; + MyVersion = version; + } + } + + @Mock + private IResourceTableDao myResourceTableDao; + + @Mock + private DaoConfig myDaoConfig; + + @Mock + private MemoryCacheService myMemoryCacheService; + + @Mock + private EntityManager myEntityManager; + + @InjectMocks + private IdHelperService myIdHelperService; + + private ResourcePersistentId getResourcePersistentIdFromResource(IIdType theId, long thePid) { + ResourcePersistentId id = new ResourcePersistentId(thePid); + String idPortion = theId.getIdPart(); + IIdType newId = new IdDt(theId.getResourceType(), idPortion); + id.setAssociatedResourceId(newId); + return id; + } + + /** + * Gets a ResourceTable record for getResourceVersionsForPid + * Order matters! + * @param resourceType + * @param pid + * @param version + * @return + */ + private Object[] getResourceTableRecordForResourceTypeAndPid(String resourceType, long pid, long version) { + return new Object[] { + pid, // long + resourceType, // string + version // long + }; + } + + /** + * Helper function to mock out resolveResourcePersistentIdsWithCache + * to return empty lists (as if no resources were found). + */ + private void mock_resolveResourcePersistentIdsWithCache_toReturnNothing() { + CriteriaBuilder cb = Mockito.mock(CriteriaBuilder.class); + CriteriaQuery criteriaQuery = Mockito.mock(CriteriaQuery.class); + Root from = Mockito.mock(Root.class); + Path path = Mockito.mock(Path.class); + + Mockito.when(cb.createQuery(Mockito.any(Class.class))) + .thenReturn(criteriaQuery); + Mockito.when(criteriaQuery.from(Mockito.any(Class.class))) + .thenReturn(from); + Mockito.when(from.get(Mockito.anyString())) + .thenReturn(path); + + TypedQuery queryMock = Mockito.mock(TypedQuery.class); + Mockito.when(queryMock.getResultList()).thenReturn(new ArrayList<>()); // not found + + Mockito.when(myEntityManager.getCriteriaBuilder()) + .thenReturn(cb); + Mockito.when(myEntityManager.createQuery(Mockito.any(CriteriaQuery.class))) + .thenReturn(queryMock); + } + + /** + * Helper function to mock out getIdsOfExistingResources + * to return the matches and resources matching those provided + * by parameters. + * @param theResourcePacks + */ + private void mockReturnsFor_getIdsOfExistingResources(ResourceIdPackage... theResourcePacks) { + List resourcePersistentIds = new ArrayList<>(); + List matches = new ArrayList<>(); + + for (ResourceIdPackage pack : theResourcePacks) { + resourcePersistentIds.add(pack.MyPid); + + matches.add(getResourceTableRecordForResourceTypeAndPid( + pack.MyResourceId.getResourceType(), + pack.MyPid.getIdAsLong(), + pack.MyVersion + )); + } + + ResourcePersistentId first = resourcePersistentIds.remove(0); + if (resourcePersistentIds.isEmpty()) { + Mockito.when(myMemoryCacheService.getIfPresent(Mockito.any(MemoryCacheService.CacheEnum.class), Mockito.anyString())) + .thenReturn(first).thenReturn(null); + } + else { + Mockito.when(myMemoryCacheService.getIfPresent(Mockito.any(MemoryCacheService.CacheEnum.class), Mockito.anyString())) + .thenReturn(first, resourcePersistentIds.toArray()); + } + Mockito.when(myResourceTableDao.getResourceVersionsForPid(Mockito.anyList())) + .thenReturn(matches); + } + + @Test + public void getLatestVersionIdsForResourceIds_whenResourceExists_returnsMapWithPIDAndVersion() { + IIdType type = new IdDt("Patient/RED"); + ResourcePersistentId pid = new ResourcePersistentId(1L); + pid.setAssociatedResourceId(type); + HashMap map = new HashMap<>(); + map.put(type, pid); + ResourceIdPackage pack = new ResourceIdPackage(type, pid, 2L); + + // when + mockReturnsFor_getIdsOfExistingResources(pack); + + // test + Map retMap = myIdHelperService.getLatestVersionIdsForResourceIds(RequestPartitionId.allPartitions(), + Collections.singletonList(type)); + + Assertions.assertTrue(retMap.containsKey(type)); + Assertions.assertEquals(pid.getVersion(), map.get(type).getVersion()); + } + + @Test + public void getLatestVersionIdsForResourceIds_whenResourceDoesNotExist_returnsEmptyMap() { + IIdType type = new IdDt("Patient/RED"); + + // when + mock_resolveResourcePersistentIdsWithCache_toReturnNothing(); + + // test + Map retMap = myIdHelperService.getLatestVersionIdsForResourceIds(RequestPartitionId.allPartitions(), + Collections.singletonList(type)); + + Assertions.assertTrue(retMap.isEmpty()); + } + + @Test + public void getLatestVersionIdsForResourceIds_whenSomeResourcesDoNotExist_returnsOnlyExistingElements() { + // resource to be found + IIdType type = new IdDt("Patient/RED"); + ResourcePersistentId pid = new ResourcePersistentId(1L); + pid.setAssociatedResourceId(type); + ResourceIdPackage pack = new ResourceIdPackage(type, pid, 2L); + + // resource that won't be found + IIdType type2 = new IdDt("Patient/BLUE"); + + // when + mock_resolveResourcePersistentIdsWithCache_toReturnNothing(); + mockReturnsFor_getIdsOfExistingResources(pack); + + // test + Map retMap = myIdHelperService.getLatestVersionIdsForResourceIds( + RequestPartitionId.allPartitions(), + Arrays.asList(type, type2) + ); + + // verify + Assertions.assertEquals(1, retMap.size()); + Assertions.assertTrue(retMap.containsKey(type)); + Assertions.assertFalse(retMap.containsKey(type2)); + } + @Test public void testReplaceDefault_AllPartitions() { diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/BaseJpaR4Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/BaseJpaR4Test.java index 39660fe7399..95a9732fed5 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/BaseJpaR4Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/BaseJpaR4Test.java @@ -20,7 +20,6 @@ import ca.uhn.fhir.jpa.binstore.BinaryStorageInterceptor; import ca.uhn.fhir.jpa.bulk.export.api.IBulkDataExportSvc; import ca.uhn.fhir.jpa.config.TestR4Config; import ca.uhn.fhir.jpa.dao.BaseJpaTest; -import ca.uhn.fhir.jpa.dao.IAutoVersioningService; import ca.uhn.fhir.jpa.dao.IFulltextSearchSvc; import ca.uhn.fhir.jpa.dao.data.IForcedIdDao; import ca.uhn.fhir.jpa.dao.data.IMdmLinkDao; @@ -499,8 +498,6 @@ public abstract class BaseJpaR4Test extends BaseJpaTest implements ITestDataBuil protected ValidationSettings myValidationSettings; @Autowired protected IMdmLinkDao myMdmLinkDao; - @Autowired - protected IAutoVersioningService myAutoVersioningService; @AfterEach() public void afterCleanupDao() {