diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/cache/ResourceVersionSvcDaoImpl.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/cache/ResourceVersionSvcDaoImpl.java index cb82083e24e..3aa68936218 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/cache/ResourceVersionSvcDaoImpl.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/cache/ResourceVersionSvcDaoImpl.java @@ -24,18 +24,23 @@ 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.jpa.dao.data.IResourceTableDao; +import ca.uhn.fhir.jpa.dao.index.IdHelperService; import ca.uhn.fhir.jpa.model.entity.ResourceTable; import ca.uhn.fhir.jpa.partition.SystemRequestDetails; import ca.uhn.fhir.jpa.searchparam.SearchParameterMap; import ca.uhn.fhir.jpa.util.QueryChunker; import ca.uhn.fhir.rest.api.server.storage.ResourcePersistentId; +import org.hl7.fhir.instance.model.api.IIdType; import org.slf4j.Logger; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Service; import javax.annotation.Nonnull; import java.util.ArrayList; +import java.util.Collection; +import java.util.HashMap; import java.util.List; +import java.util.Optional; import java.util.stream.Collectors; import static org.slf4j.LoggerFactory.getLogger; @@ -52,17 +57,19 @@ public class ResourceVersionSvcDaoImpl implements IResourceVersionSvc { DaoRegistry myDaoRegistry; @Autowired IResourceTableDao myResourceTableDao; + @Autowired + IdHelperService myIdHelperService; @Override @Nonnull - public ResourceVersionMap getVersionMap(String theResourceName, SearchParameterMap theSearchParamMap) { + public ResourceVersionMap getVersionMap(RequestPartitionId theRequestPartitionId, String theResourceName, SearchParameterMap theSearchParamMap) { IFhirResourceDao dao = myDaoRegistry.getResourceDao(theResourceName); if (ourLog.isDebugEnabled()) { ourLog.debug("About to retrieve version map for resource type: {}", theResourceName); } - List matchingIds = dao.searchForIds(theSearchParamMap, new SystemRequestDetails().setRequestPartitionId(RequestPartitionId.allPartitions())).stream() + List matchingIds = dao.searchForIds(theSearchParamMap, new SystemRequestDetails().setRequestPartitionId(theRequestPartitionId)).stream() .map(ResourcePersistentId::getIdAsLong) .collect(Collectors.toList()); @@ -74,4 +81,95 @@ public class ResourceVersionSvcDaoImpl implements IResourceVersionSvc { return ResourceVersionMap.fromResourceTableEntities(allById); } + + @Override + /** + * 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 ResourcePersistentIdMap getLatestVersionIdsForResourceIds(RequestPartitionId theRequestPartitionId, List theIds) { + ResourcePersistentIdMap idToPID = new ResourcePersistentIdMap(); + 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()) { + ResourcePersistentIdMap idAndPID = getIdsOfExistingResources(theRequestPartitionId, + resourceTypeToIds.get(resourceType)); + idToPID.putAll(idAndPID); + } + + return idToPID; + } + + /** + * 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 ResourcePersistentIdMap getIdsOfExistingResources(RequestPartitionId thePartitionId, + Collection theIds) { + // these are the found Ids that were in the db + ResourcePersistentIdMap retval = new ResourcePersistentIdMap(); + + if (theIds == null || theIds.isEmpty()) { + return retval; + } + + 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 -> { + retval.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 retval; + } } 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 58aeae432ca..a8ee4cf93e7 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,7 +30,8 @@ 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.cache.IResourceVersionSvc; +import ca.uhn.fhir.jpa.cache.ResourcePersistentIdMap; import ca.uhn.fhir.jpa.model.cross.IBasePersistedResource; import ca.uhn.fhir.jpa.model.entity.ModelConfig; import ca.uhn.fhir.jpa.model.entity.ResourceTable; @@ -94,7 +95,7 @@ public abstract class BaseStorageDao { @Autowired protected ModelConfig myModelConfig; @Autowired - protected IdHelperService myIdHelperService; + protected IResourceVersionSvc myResourceVersionSvc; @Autowired protected DaoConfig myDaoConfig; @@ -211,7 +212,7 @@ public abstract class BaseStorageDao { IIdType referenceElement = nextReference.getReferenceElement(); if (!referenceElement.hasBaseUrl()) { - Map idToPID = myIdHelperService.getLatestVersionIdsForResourceIds(RequestPartitionId.allPartitions(), + ResourcePersistentIdMap resourceVersionMap = myResourceVersionSvc.getLatestVersionIdsForResourceIds(RequestPartitionId.allPartitions(), Collections.singletonList(referenceElement) ); @@ -220,12 +221,11 @@ public abstract class BaseStorageDao { // 2) no resource exists, but we will create one (eventually). The version is 1 // 3) no resource exists, and none will be made -> throw Long version; - if (idToPID.containsKey(referenceElement)) { + if (resourceVersionMap.containsKey(referenceElement)) { // the resource exists... latest id // will be the value in the ResourcePersistentId - version = idToPID.get(referenceElement).getVersion(); - } - else if (myDaoConfig.isAutoCreatePlaceholderReferenceTargets()) { + version = resourceVersionMap.getResourcePersistentId(referenceElement).getVersion(); + } else if (myDaoConfig.isAutoCreatePlaceholderReferenceTargets()) { // if idToPID doesn't contain object // but autcreateplaceholders is on // then the version will be 1 (the first version) 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 d554b645514..50a683e55f5 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,7 +35,8 @@ 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.cache.IResourceVersionSvc; +import ca.uhn.fhir.jpa.cache.ResourcePersistentIdMap; import ca.uhn.fhir.jpa.dao.tx.HapiTransactionService; import ca.uhn.fhir.jpa.delete.DeleteConflictService; import ca.uhn.fhir.jpa.model.cross.IBasePersistedResource; @@ -54,7 +55,6 @@ import ca.uhn.fhir.rest.api.PreferReturnEnum; import ca.uhn.fhir.rest.api.RestOperationTypeEnum; import ca.uhn.fhir.rest.api.server.RequestDetails; import ca.uhn.fhir.rest.api.server.storage.DeferredInterceptorBroadcasts; -import ca.uhn.fhir.rest.api.server.storage.ResourcePersistentId; import ca.uhn.fhir.rest.api.server.storage.TransactionDetails; import ca.uhn.fhir.rest.param.ParameterUtil; import ca.uhn.fhir.rest.server.RestfulServerUtils; @@ -160,7 +160,7 @@ public abstract class BaseTransactionProcessor { private TaskExecutor myExecutor ; @Autowired - private IdHelperService myIdHelperService; + private IResourceVersionSvc myResourceVersionSvc; @VisibleForTesting public void setDaoConfig(DaoConfig theDaoConfig) { @@ -1271,25 +1271,24 @@ public abstract class BaseTransactionProcessor { // get a map of // existing ids -> PID (for resources that exist in the DB) // should this be allPartitions? - Map idToPID = myIdHelperService.getLatestVersionIdsForResourceIds(RequestPartitionId.allPartitions(), + ResourcePersistentIdMap resourceVersionMap = myResourceVersionSvc.getLatestVersionIdsForResourceIds(RequestPartitionId.allPartitions(), theReferencesToAutoVersion.stream() - .map(ref -> ref.getReferenceElement()).collect(Collectors.toList())); + .map(IBaseReference::getReferenceElement).collect(Collectors.toList())); for (IBaseReference baseRef : theReferencesToAutoVersion) { IIdType id = baseRef.getReferenceElement(); - if (!idToPID.containsKey(id) - && myDaoConfig.isAutoCreatePlaceholderReferenceTargets()) { + if (!resourceVersionMap.containsKey(id) + && myDaoConfig.isAutoCreatePlaceholderReferenceTargets()) { // not in the db, but autocreateplaceholders is true // so the version we'll set is "1" (since it will be // created later) String newRef = id.withVersion("1").getValue(); id.setValue(newRef); - } - else { + } else { // we will add the looked up info to the transaction // for later theTransactionDetails.addResolvedResourceId(id, - idToPID.get(id)); + resourceVersionMap.getResourcePersistentId(id)); } } @@ -1686,19 +1685,19 @@ public abstract class BaseTransactionProcessor { public class BundleTask implements Runnable { - private CountDownLatch myCompletedLatch; - private RequestDetails myRequestDetails; - private IBase myNextReqEntry; - private Map myResponseMap; - private int myResponseOrder; - private boolean myNestedMode; - + private final CountDownLatch myCompletedLatch; + private final RequestDetails myRequestDetails; + private final IBase myNextReqEntry; + private final Map myResponseMap; + private final int myResponseOrder; + private final boolean myNestedMode; + protected BundleTask(CountDownLatch theCompletedLatch, RequestDetails theRequestDetails, Map theResponseMap, int theResponseOrder, IBase theNextReqEntry, boolean theNestedMode) { this.myCompletedLatch = theCompletedLatch; this.myRequestDetails = theRequestDetails; this.myNextReqEntry = theNextReqEntry; - this.myResponseMap = theResponseMap; - this.myResponseOrder = theResponseOrder; + this.myResponseMap = theResponseMap; + this.myResponseOrder = theResponseOrder; this.myNestedMode = theNestedMode; } @@ -1707,7 +1706,7 @@ public abstract class BaseTransactionProcessor { BaseServerResponseExceptionHolder caughtEx = new BaseServerResponseExceptionHolder(); try { IBaseBundle subRequestBundle = myVersionAdapter.createBundle(org.hl7.fhir.r4.model.Bundle.BundleType.TRANSACTION.toCode()); - myVersionAdapter.addEntry(subRequestBundle, (IBase) myNextReqEntry); + myVersionAdapter.addEntry(subRequestBundle, myNextReqEntry); IBaseBundle nextResponseBundle = processTransactionAsSubRequest(myRequestDetails, subRequestBundle, "Batch sub-request", myNestedMode); @@ -1735,7 +1734,7 @@ public abstract class BaseTransactionProcessor { } // checking for the parallelism - ourLog.debug("processing bacth for {} is completed", myVersionAdapter.getEntryRequestUrl((IBase)myNextReqEntry)); + ourLog.debug("processing bacth for {} is completed", myVersionAdapter.getEntryRequestUrl(myNextReqEntry)); myCompletedLatch.countDown(); } } 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 03763d40a0f..91c99df3972 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 @@ -303,7 +303,7 @@ public class IdHelperService { if (forcedId.isPresent()) { retVal.setValue(theResourceType + '/' + forcedId.get()); } else { - retVal.setValue(theResourceType + '/' + theId.toString()); + retVal.setValue(theResourceType + '/' + theId); } return retVal; @@ -530,95 +530,6 @@ 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/main/java/ca/uhn/fhir/jpa/search/elastic/IndexNamePrefixLayoutStrategy.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/elastic/IndexNamePrefixLayoutStrategy.java index fa5721b75dc..f0849bd1b22 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/elastic/IndexNamePrefixLayoutStrategy.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/elastic/IndexNamePrefixLayoutStrategy.java @@ -1,5 +1,25 @@ package ca.uhn.fhir.jpa.search.elastic; +/*- + * #%L + * HAPI FHIR JPA Server + * %% + * Copyright (C) 2014 - 2021 Smile CDR, Inc. + * %% + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * #L% + */ + import ca.uhn.fhir.context.ConfigurationException; import ca.uhn.fhir.jpa.api.config.DaoConfig; import org.apache.commons.lang3.StringUtils; @@ -24,7 +44,7 @@ public class IndexNamePrefixLayoutStrategy implements IndexLayoutStrategy { @Autowired private DaoConfig myDaoConfig; - static final Log log = (Log) LoggerFactory.make(Log.class, MethodHandles.lookup()); + static final Log log = LoggerFactory.make(Log.class, MethodHandles.lookup()); public static final String NAME = "prefix"; public static final Pattern UNIQUE_KEY_EXTRACTION_PATTERN = Pattern.compile("(.*)-\\d{6}"); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/cache/ResourceVersionCacheSvcTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/cache/ResourceVersionCacheSvcTest.java index fb3bfe4c28f..267fdbda222 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/cache/ResourceVersionCacheSvcTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/cache/ResourceVersionCacheSvcTest.java @@ -21,12 +21,12 @@ public class ResourceVersionCacheSvcTest extends BaseJpaR4Test { IIdType patientId = myPatientDao.create(patient).getId(); ResourceVersionMap versionMap = myResourceVersionCacheSvc.getVersionMap("Patient", SearchParameterMap.newSynchronous()); assertEquals(1, versionMap.size()); - assertEquals("1", versionMap.getVersion(patientId)); + assertEquals(1L, versionMap.getVersion(patientId)); patient.setGender(Enumerations.AdministrativeGender.MALE); myPatientDao.update(patient); versionMap = myResourceVersionCacheSvc.getVersionMap("Patient", SearchParameterMap.newSynchronous()); assertEquals(1, versionMap.size()); - assertEquals("2", versionMap.getVersion(patientId)); + assertEquals(2L, versionMap.getVersion(patientId)); } } 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 0bace079a8f..cff84d93d99 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 @@ -5,6 +5,7 @@ import ca.uhn.fhir.context.FhirVersionEnum; import ca.uhn.fhir.interceptor.executor.InterceptorService; import ca.uhn.fhir.jpa.api.config.DaoConfig; import ca.uhn.fhir.jpa.api.dao.DaoRegistry; +import ca.uhn.fhir.jpa.cache.IResourceVersionSvc; import ca.uhn.fhir.jpa.dao.index.IdHelperService; import ca.uhn.fhir.jpa.dao.r4.TransactionProcessorVersionAdapterR4; import ca.uhn.fhir.jpa.dao.tx.HapiTransactionService; @@ -70,6 +71,8 @@ public class TransactionProcessorTest { private MatchUrlService myMatchUrlService; @MockBean private IRequestPartitionHelperSvc myRequestPartitionHelperSvc; + @MockBean + private IResourceVersionSvc myResourceVersionSvc; @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/ResourceVersionSvcTest.java similarity index 77% rename from hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/index/IdHelperServiceTest.java rename to hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/index/ResourceVersionSvcTest.java index 2ca8fea957c..d0b7764729f 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/ResourceVersionSvcTest.java @@ -1,11 +1,12 @@ 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.api.dao.DaoRegistry; +import ca.uhn.fhir.jpa.cache.ResourcePersistentIdMap; +import ca.uhn.fhir.jpa.cache.ResourceVersionSvcDaoImpl; 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; @@ -17,7 +18,6 @@ import org.mockito.Mock; import org.mockito.Mockito; import org.mockito.junit.jupiter.MockitoExtension; -import javax.persistence.EntityManager; import javax.persistence.TypedQuery; import javax.persistence.criteria.CriteriaBuilder; import javax.persistence.criteria.CriteriaQuery; @@ -28,13 +28,14 @@ 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; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.when; @ExtendWith(MockitoExtension.class) -public class IdHelperServiceTest { +public class ResourceVersionSvcTest { // helper class to package up data for helper methods private class ResourceIdPackage { @@ -52,19 +53,15 @@ public class IdHelperServiceTest { } @Mock - private IResourceTableDao myResourceTableDao; - + DaoRegistry myDaoRegistry; @Mock - private DaoConfig myDaoConfig; - + IResourceTableDao myResourceTableDao; @Mock - private MemoryCacheService myMemoryCacheService; - - @Mock - private EntityManager myEntityManager; + IdHelperService myIdHelperService; + // TODO KHS move the methods that use this out to a separate test class @InjectMocks - private IdHelperService myIdHelperService; + private ResourceVersionSvcDaoImpl myResourceVersionSvc; /** * Gets a ResourceTable record for getResourceVersionsForPid @@ -92,20 +89,7 @@ public class IdHelperServiceTest { 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); } /** @@ -130,15 +114,11 @@ public class IdHelperServiceTest { ResourcePersistentId first = resourcePersistentIds.remove(0); if (resourcePersistentIds.isEmpty()) { - Mockito.when(myMemoryCacheService.getIfPresent(Mockito.any(MemoryCacheService.CacheEnum.class), Mockito.anyString())) - .thenReturn(first).thenReturn(null); + when(myIdHelperService.resolveResourcePersistentIdsWithCache(any(), any())).thenReturn(Collections.singletonList(first)); } else { - Mockito.when(myMemoryCacheService.getIfPresent(Mockito.any(MemoryCacheService.CacheEnum.class), Mockito.anyString())) - .thenReturn(first, resourcePersistentIds.toArray()); + when(myIdHelperService.resolveResourcePersistentIdsWithCache(any(), any())).thenReturn(resourcePersistentIds); } - Mockito.when(myResourceTableDao.getResourceVersionsForPid(Mockito.anyList())) - .thenReturn(matches); } @Test @@ -154,7 +134,7 @@ public class IdHelperServiceTest { mockReturnsFor_getIdsOfExistingResources(pack); // test - Map retMap = myIdHelperService.getLatestVersionIdsForResourceIds(RequestPartitionId.allPartitions(), + ResourcePersistentIdMap retMap = myResourceVersionSvc.getLatestVersionIdsForResourceIds(RequestPartitionId.allPartitions(), Collections.singletonList(type)); Assertions.assertTrue(retMap.containsKey(type)); @@ -169,7 +149,7 @@ public class IdHelperServiceTest { mock_resolveResourcePersistentIdsWithCache_toReturnNothing(); // test - Map retMap = myIdHelperService.getLatestVersionIdsForResourceIds(RequestPartitionId.allPartitions(), + ResourcePersistentIdMap retMap = myResourceVersionSvc.getLatestVersionIdsForResourceIds(RequestPartitionId.allPartitions(), Collections.singletonList(type)); Assertions.assertTrue(retMap.isEmpty()); @@ -191,7 +171,7 @@ public class IdHelperServiceTest { mockReturnsFor_getIdsOfExistingResources(pack); // test - Map retMap = myIdHelperService.getLatestVersionIdsForResourceIds( + ResourcePersistentIdMap retMap = myResourceVersionSvc.getLatestVersionIdsForResourceIds( RequestPartitionId.allPartitions(), Arrays.asList(type, type2) ); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/stresstest/GiantTransactionPerfTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/stresstest/GiantTransactionPerfTest.java index d9da4485295..405e2302044 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/stresstest/GiantTransactionPerfTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/stresstest/GiantTransactionPerfTest.java @@ -14,6 +14,7 @@ import ca.uhn.fhir.jpa.cache.ResourceChangeListenerCache; import ca.uhn.fhir.jpa.cache.ResourceChangeListenerCacheFactory; import ca.uhn.fhir.jpa.cache.ResourceChangeListenerCacheRefresherImpl; import ca.uhn.fhir.jpa.cache.ResourceChangeListenerRegistryImpl; +import ca.uhn.fhir.jpa.cache.ResourcePersistentIdMap; import ca.uhn.fhir.jpa.cache.ResourceVersionMap; import ca.uhn.fhir.jpa.dao.JpaResourceDao; import ca.uhn.fhir.jpa.dao.TransactionProcessor; @@ -45,6 +46,7 @@ import com.google.common.collect.Lists; import org.hamcrest.Matchers; import org.hibernate.internal.SessionImpl; import org.hl7.fhir.instance.model.api.IBaseResource; +import org.hl7.fhir.instance.model.api.IIdType; import org.hl7.fhir.r4.model.Bundle; import org.hl7.fhir.r4.model.ExplanationOfBenefit; import org.junit.jupiter.api.AfterEach; @@ -323,10 +325,15 @@ public class GiantTransactionPerfTest { @Nonnull @Override - public ResourceVersionMap getVersionMap(String theResourceName, SearchParameterMap theSearchParamMap) { + public ResourceVersionMap getVersionMap(RequestPartitionId theRequestPartitionId, String theResourceName, SearchParameterMap theSearchParamMap) { myGetVersionMap++; return ResourceVersionMap.fromResources(Lists.newArrayList()); } + + @Override + public ResourcePersistentIdMap getLatestVersionIdsForResourceIds(RequestPartitionId thePartition, List theIds) { + return null; + } } private class MockResourceHistoryTableDao implements IResourceHistoryTableDao { diff --git a/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/cache/IResourceVersionSvc.java b/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/cache/IResourceVersionSvc.java index d53dc45fd94..a13d823d351 100644 --- a/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/cache/IResourceVersionSvc.java +++ b/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/cache/IResourceVersionSvc.java @@ -20,9 +20,12 @@ package ca.uhn.fhir.jpa.cache; * #L% */ +import ca.uhn.fhir.interceptor.model.RequestPartitionId; import ca.uhn.fhir.jpa.searchparam.SearchParameterMap; +import org.hl7.fhir.instance.model.api.IIdType; import javax.annotation.Nonnull; +import java.util.List; /** * This interface is used by the {@link IResourceChangeListenerCacheRefresher} to read resources matching the provided @@ -30,5 +33,12 @@ import javax.annotation.Nonnull; */ public interface IResourceVersionSvc { @Nonnull - ResourceVersionMap getVersionMap(String theResourceName, SearchParameterMap theSearchParamMap); + ResourceVersionMap getVersionMap(RequestPartitionId theRequestPartitionId, String theResourceName, SearchParameterMap theSearchParamMap); + + @Nonnull + default ResourceVersionMap getVersionMap(String theResourceName, SearchParameterMap theSearchParamMap) { + return getVersionMap(RequestPartitionId.allPartitions(), theResourceName, theSearchParamMap); + } + + ResourcePersistentIdMap getLatestVersionIdsForResourceIds(RequestPartitionId thePartition, List theIds); } diff --git a/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/cache/ResourceChangeListenerCacheRefresherImpl.java b/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/cache/ResourceChangeListenerCacheRefresherImpl.java index f809b02a7e4..8bf05bac0c4 100644 --- a/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/cache/ResourceChangeListenerCacheRefresherImpl.java +++ b/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/cache/ResourceChangeListenerCacheRefresherImpl.java @@ -173,8 +173,8 @@ public class ResourceChangeListenerCacheRefresherImpl implements IResourceChange List updatedIds = new ArrayList<>(); for (IIdType id : theNewResourceVersionMap.keySet()) { - String previousValue = theOldResourceVersionCache.put(id, theNewResourceVersionMap.get(id)); - IIdType newId = id.withVersion(theNewResourceVersionMap.get(id)); + Long previousValue = theOldResourceVersionCache.put(id, theNewResourceVersionMap.get(id)); + IIdType newId = id.withVersion(theNewResourceVersionMap.get(id).toString()); if (previousValue == null) { createdIds.add(newId); } else if (!theNewResourceVersionMap.get(id).equals(previousValue)) { diff --git a/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/cache/ResourcePersistentIdMap.java b/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/cache/ResourcePersistentIdMap.java new file mode 100644 index 00000000000..6594bc723b2 --- /dev/null +++ b/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/cache/ResourcePersistentIdMap.java @@ -0,0 +1,67 @@ +package ca.uhn.fhir.jpa.cache; + +/*- + * #%L + * HAPI FHIR Search Parameters + * %% + * Copyright (C) 2014 - 2021 Smile CDR, Inc. + * %% + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * #L% + */ + +import ca.uhn.fhir.rest.api.server.storage.ResourcePersistentId; +import org.hl7.fhir.instance.model.api.IIdType; + +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +public class ResourcePersistentIdMap { + private final Map myMap = new HashMap<>(); + + public static ResourcePersistentIdMap fromResourcePersistentIds(List theResourcePersistentIds) { + ResourcePersistentIdMap retval = new ResourcePersistentIdMap(); + theResourcePersistentIds.forEach(retval::add); + return retval; + } + + private void add(ResourcePersistentId theResourcePersistentId) { + IIdType id = theResourcePersistentId.getAssociatedResourceId(); + myMap.put(id.toUnqualifiedVersionless(), theResourcePersistentId); + } + + public boolean containsKey(IIdType theId) { + return myMap.containsKey(theId.toUnqualifiedVersionless()); + } + + public ResourcePersistentId getResourcePersistentId(IIdType theId) { + return myMap.get(theId.toUnqualifiedVersionless()); + } + + public boolean isEmpty() { + return myMap.isEmpty(); + } + + public int size() { + return myMap.size(); + } + + public void put(IIdType theId, ResourcePersistentId thePid) { + myMap.put(theId, thePid); + } + + public void putAll(ResourcePersistentIdMap theIdAndPID) { + myMap.putAll(theIdAndPID.myMap); + } +} diff --git a/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/cache/ResourceVersionCache.java b/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/cache/ResourceVersionCache.java index b490e6d69a3..3b840aa0b71 100644 --- a/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/cache/ResourceVersionCache.java +++ b/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/cache/ResourceVersionCache.java @@ -32,7 +32,7 @@ import java.util.Set; * detect resources that were modified on remote servers in our cluster. */ public class ResourceVersionCache { - private final Map myVersionMap = new HashMap<>(); + private final Map myVersionMap = new HashMap<>(); public void clear() { myVersionMap.clear(); @@ -43,15 +43,15 @@ public class ResourceVersionCache { * @param theVersion * @return previous value */ - public String put(IIdType theResourceId, String theVersion) { + public Long put(IIdType theResourceId, Long theVersion) { return myVersionMap.put(new IdDt(theResourceId).toVersionless(), theVersion); } - public String getVersionForResourceId(IIdType theResourceId) { + public Long getVersionForResourceId(IIdType theResourceId) { return myVersionMap.get(new IdDt(theResourceId)); } - public String removeResourceId(IIdType theResourceId) { + public Long removeResourceId(IIdType theResourceId) { return myVersionMap.remove(new IdDt(theResourceId)); } diff --git a/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/cache/ResourceVersionMap.java b/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/cache/ResourceVersionMap.java index 3b412e0546c..fbb029b5afc 100644 --- a/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/cache/ResourceVersionMap.java +++ b/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/cache/ResourceVersionMap.java @@ -24,6 +24,8 @@ import ca.uhn.fhir.jpa.model.entity.ResourceTable; import ca.uhn.fhir.model.primitive.IdDt; import org.hl7.fhir.instance.model.api.IBaseResource; import org.hl7.fhir.instance.model.api.IIdType; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import java.util.Collections; import java.util.HashMap; @@ -36,8 +38,10 @@ import java.util.Set; * This immutable map holds a copy of current resource versions read from the repository. */ public class ResourceVersionMap { + private static final Logger ourLog = LoggerFactory.getLogger(ResourceVersionMap.class); private final Set mySourceIds = new HashSet<>(); - private final Map myMap = new HashMap<>(); + // Key versionless id, value version + private final Map myMap = new HashMap<>(); private ResourceVersionMap() {} public static ResourceVersionMap fromResourceTableEntities(List theEntities) { @@ -57,13 +61,17 @@ public class ResourceVersionMap { } private void add(IIdType theId) { + if (theId.getVersionIdPart() == null) { + ourLog.warn("Not storing {} in ResourceVersionMap because it does not have a version.", theId); + return; + } IdDt id = new IdDt(theId); mySourceIds.add(id); - myMap.put(id.toUnqualifiedVersionless(), id.getVersionIdPart()); + myMap.put(id.toUnqualifiedVersionless(), id.getVersionIdPartAsLong()); } - public String getVersion(IIdType theResourceId) { - return myMap.get(new IdDt(theResourceId.toUnqualifiedVersionless())); + public Long getVersion(IIdType theResourceId) { + return get(theResourceId); } public int size() { @@ -78,11 +86,15 @@ public class ResourceVersionMap { return Collections.unmodifiableSet(mySourceIds); } - public String get(IIdType theId) { + public Long get(IIdType theId) { return myMap.get(new IdDt(theId.toUnqualifiedVersionless())); } public boolean containsKey(IIdType theId) { return myMap.containsKey(new IdDt(theId.toUnqualifiedVersionless())); } + + public boolean isEmpty() { + return myMap.isEmpty(); + } } diff --git a/hapi-fhir-server-mdm/src/main/java/ca/uhn/fhir/mdm/api/IMdmBatchJobSubmitterFactory.java b/hapi-fhir-server-mdm/src/main/java/ca/uhn/fhir/mdm/api/IMdmBatchJobSubmitterFactory.java index 6291d89d99a..6dc629d0f05 100644 --- a/hapi-fhir-server-mdm/src/main/java/ca/uhn/fhir/mdm/api/IMdmBatchJobSubmitterFactory.java +++ b/hapi-fhir-server-mdm/src/main/java/ca/uhn/fhir/mdm/api/IMdmBatchJobSubmitterFactory.java @@ -1,5 +1,25 @@ package ca.uhn.fhir.mdm.api; +/*- + * #%L + * HAPI FHIR - Master Data Management + * %% + * Copyright (C) 2014 - 2021 Smile CDR, Inc. + * %% + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * #L% + */ + public interface IMdmBatchJobSubmitterFactory { IMdmClearJobSubmitter getClearJobSubmitter(); } diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/api/server/storage/ResourcePersistentId.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/api/server/storage/ResourcePersistentId.java index 1a45a9c9256..ad04685141e 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/api/server/storage/ResourcePersistentId.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/api/server/storage/ResourcePersistentId.java @@ -33,6 +33,7 @@ import java.util.Optional; * a Long, a String, or something else. */ public class ResourcePersistentId { + private static final String RESOURCE_PID = "RESOURCE_PID"; private Object myId; private Long myVersion; private IIdType myAssociatedResourceId;