From 519244a2ea5763e75ebb79f04a3d91b5b42bc004 Mon Sep 17 00:00:00 2001 From: Ken Stevens Date: Wed, 8 Sep 2021 19:14:50 -0400 Subject: [PATCH 1/7] unit test passes --- .../jpa/cache/ResourceVersionSvcDaoImpl.java | 17 +++- .../ca/uhn/fhir/jpa/dao/BaseStorageDao.java | 14 +-- .../jpa/dao/BaseTransactionProcessor.java | 41 ++++----- .../fhir/jpa/dao/index/IdHelperService.java | 91 +------------------ ...eTest.java => ResourceVersionSvcTest.java} | 52 ++++------- .../stresstest/GiantTransactionPerfTest.java | 8 +- .../fhir/jpa/cache/IResourceVersionSvc.java | 12 ++- .../fhir/jpa/cache/ResourceVersionMap.java | 24 ++++- 8 files changed, 100 insertions(+), 159 deletions(-) rename hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/index/{IdHelperServiceTest.java => ResourceVersionSvcTest.java} (77%) 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..aac577d9d85 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,11 +24,13 @@ 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; @@ -52,17 +54,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 +78,13 @@ public class ResourceVersionSvcDaoImpl implements IResourceVersionSvc { return ResourceVersionMap.fromResourceTableEntities(allById); } + + @Override + public ResourceVersionMap getLatestVersionIdsForResourceIds(RequestPartitionId thePartitionId, List theIds) { + + List resourcePersistentIds = myIdHelperService.resolveResourcePersistentIdsWithCache(thePartitionId, + new ArrayList<>(theIds)); + + return ResourceVersionMap.fromResourcePersistentIds(resourcePersistentIds); + } } 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..afc8b324062 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.ResourceVersionMap; 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(), + ResourceVersionMap 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.getVersionAsLong(referenceElement); + } 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..04352bc2464 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.ResourceVersionMap; 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(), + ResourceVersionMap 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/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..f6fa0f9d798 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.ResourceVersionMap; +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(), + ResourceVersionMap 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(), + ResourceVersionMap 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( + ResourceVersionMap 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..57c5b39a76a 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 @@ -45,6 +45,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 +324,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 ResourceVersionMap 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..d7566044ccd 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); + } + + ResourceVersionMap getLatestVersionIdsForResourceIds(RequestPartitionId thePartition, List theIds); } 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..3760039a63d 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 @@ -22,6 +22,7 @@ package ca.uhn.fhir.jpa.cache; import ca.uhn.fhir.jpa.model.entity.ResourceTable; import ca.uhn.fhir.model.primitive.IdDt; +import ca.uhn.fhir.rest.api.server.storage.ResourcePersistentId; import org.hl7.fhir.instance.model.api.IBaseResource; import org.hl7.fhir.instance.model.api.IIdType; @@ -37,6 +38,7 @@ import java.util.Set; */ public class ResourceVersionMap { private final Set mySourceIds = new HashSet<>(); + // Key versionless id, value version private final Map myMap = new HashMap<>(); private ResourceVersionMap() {} @@ -56,6 +58,12 @@ public class ResourceVersionMap { return new ResourceVersionMap(); } + public static ResourceVersionMap fromResourcePersistentIds(List theResourcePersistentIds) { + ResourceVersionMap retval = new ResourceVersionMap(); + theResourcePersistentIds.forEach(resource -> retval.add(resource.getAssociatedResourceId())); + return retval; + } + private void add(IIdType theId) { IdDt id = new IdDt(theId); mySourceIds.add(id); @@ -63,7 +71,7 @@ public class ResourceVersionMap { } public String getVersion(IIdType theResourceId) { - return myMap.get(new IdDt(theResourceId.toUnqualifiedVersionless())); + return get(theResourceId); } public int size() { @@ -85,4 +93,18 @@ public class ResourceVersionMap { public boolean containsKey(IIdType theId) { return myMap.containsKey(new IdDt(theId.toUnqualifiedVersionless())); } + + public ResourcePersistentId getResourcePersistentId(IIdType theId) { + String idPart = theId.getIdPart(); + Long version = getVersionAsLong(theId); + return new ResourcePersistentId(idPart, version); + } + + public Long getVersionAsLong(IIdType theId) { + return Long.valueOf(getVersion(theId)); + } + + public boolean isEmpty() { + return myMap.isEmpty(); + } } From 81abe26cdaedf47b752fa7cde2302289352cbac0 Mon Sep 17 00:00:00 2001 From: Ken Stevens Date: Wed, 8 Sep 2021 20:08:06 -0400 Subject: [PATCH 2/7] improve api --- .../main/java/ca/uhn/fhir/jpa/cache/ResourceVersionMap.java | 2 +- .../fhir/rest/api/server/storage/ResourcePersistentId.java | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) 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 3760039a63d..b01a140f8b6 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 @@ -60,7 +60,7 @@ public class ResourceVersionMap { public static ResourceVersionMap fromResourcePersistentIds(List theResourcePersistentIds) { ResourceVersionMap retval = new ResourceVersionMap(); - theResourcePersistentIds.forEach(resource -> retval.add(resource.getAssociatedResourceId())); + theResourcePersistentIds.forEach(resourcePersistentId -> retval.add(resourcePersistentId.getAssociatedResourceId())); return retval; } 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..801d83b9b47 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 @@ -125,4 +125,10 @@ public class ResourcePersistentId { } return retVal; } + + public static ResourcePersistentId fromIIdType(IIdType theId) { + ResourcePersistentId retval = new ResourcePersistentId(theId); + retval.setAssociatedResourceId(theId); + return retval; + } } From 67c8216d7331bcae8c08bc258e118e032501a3a3 Mon Sep 17 00:00:00 2001 From: Ken Stevens Date: Wed, 8 Sep 2021 20:17:14 -0400 Subject: [PATCH 3/7] cleanup --- .../main/java/ca/uhn/fhir/jpa/cache/ResourceVersionMap.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) 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 b01a140f8b6..3a6ab214906 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 @@ -95,9 +95,9 @@ public class ResourceVersionMap { } public ResourcePersistentId getResourcePersistentId(IIdType theId) { - String idPart = theId.getIdPart(); - Long version = getVersionAsLong(theId); - return new ResourcePersistentId(idPart, version); + ResourcePersistentId retval = ResourcePersistentId.fromIIdType(theId); + retval.setVersion(getVersionAsLong(theId)); + return retval; } public Long getVersionAsLong(IIdType theId) { From 5bc571d97d5da3137045330f563efd356d9ea977 Mon Sep 17 00:00:00 2001 From: Ken Stevens Date: Thu, 9 Sep 2021 01:00:12 -0400 Subject: [PATCH 4/7] fix test --- .../jpa/cache/ResourceVersionSvcDaoImpl.java | 4 +- .../ca/uhn/fhir/jpa/dao/BaseStorageDao.java | 6 +-- .../jpa/dao/BaseTransactionProcessor.java | 4 +- .../cache/ResourceVersionCacheSvcTest.java | 4 +- .../jpa/dao/TransactionProcessorTest.java | 3 ++ .../jpa/dao/index/ResourceVersionSvcTest.java | 8 ++-- .../stresstest/GiantTransactionPerfTest.java | 3 +- .../fhir/jpa/cache/IResourceVersionSvc.java | 2 +- ...ourceChangeListenerCacheRefresherImpl.java | 4 +- .../jpa/cache/ResourcePersistentIdMap.java | 39 +++++++++++++++++++ .../fhir/jpa/cache/ResourceVersionCache.java | 8 ++-- .../fhir/jpa/cache/ResourceVersionMap.java | 32 ++++++--------- .../server/storage/ResourcePersistentId.java | 6 --- 13 files changed, 75 insertions(+), 48 deletions(-) create mode 100644 hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/cache/ResourcePersistentIdMap.java 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 aac577d9d85..c9ca8e0255a 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 @@ -80,11 +80,11 @@ public class ResourceVersionSvcDaoImpl implements IResourceVersionSvc { } @Override - public ResourceVersionMap getLatestVersionIdsForResourceIds(RequestPartitionId thePartitionId, List theIds) { + public ResourcePersistentIdMap getLatestVersionIdsForResourceIds(RequestPartitionId thePartitionId, List theIds) { List resourcePersistentIds = myIdHelperService.resolveResourcePersistentIdsWithCache(thePartitionId, new ArrayList<>(theIds)); - return ResourceVersionMap.fromResourcePersistentIds(resourcePersistentIds); + return ResourcePersistentIdMap.fromResourcePersistentIds(resourcePersistentIds); } } 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 afc8b324062..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 @@ -31,7 +31,7 @@ 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.cache.IResourceVersionSvc; -import ca.uhn.fhir.jpa.cache.ResourceVersionMap; +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; @@ -212,7 +212,7 @@ public abstract class BaseStorageDao { IIdType referenceElement = nextReference.getReferenceElement(); if (!referenceElement.hasBaseUrl()) { - ResourceVersionMap resourceVersionMap = myResourceVersionSvc.getLatestVersionIdsForResourceIds(RequestPartitionId.allPartitions(), + ResourcePersistentIdMap resourceVersionMap = myResourceVersionSvc.getLatestVersionIdsForResourceIds(RequestPartitionId.allPartitions(), Collections.singletonList(referenceElement) ); @@ -224,7 +224,7 @@ public abstract class BaseStorageDao { if (resourceVersionMap.containsKey(referenceElement)) { // the resource exists... latest id // will be the value in the ResourcePersistentId - version = resourceVersionMap.getVersionAsLong(referenceElement); + version = resourceVersionMap.getResourcePersistentId(referenceElement).getVersion(); } else if (myDaoConfig.isAutoCreatePlaceholderReferenceTargets()) { // if idToPID doesn't contain object // but autcreateplaceholders is on 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 04352bc2464..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 @@ -36,7 +36,7 @@ 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.cache.IResourceVersionSvc; -import ca.uhn.fhir.jpa.cache.ResourceVersionMap; +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; @@ -1271,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? - ResourceVersionMap resourceVersionMap = myResourceVersionSvc.getLatestVersionIdsForResourceIds(RequestPartitionId.allPartitions(), + ResourcePersistentIdMap resourceVersionMap = myResourceVersionSvc.getLatestVersionIdsForResourceIds(RequestPartitionId.allPartitions(), theReferencesToAutoVersion.stream() .map(IBaseReference::getReferenceElement).collect(Collectors.toList())); 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/ResourceVersionSvcTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/index/ResourceVersionSvcTest.java index f6fa0f9d798..d0b7764729f 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/index/ResourceVersionSvcTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/index/ResourceVersionSvcTest.java @@ -2,7 +2,7 @@ package ca.uhn.fhir.jpa.dao.index; import ca.uhn.fhir.interceptor.model.RequestPartitionId; import ca.uhn.fhir.jpa.api.dao.DaoRegistry; -import ca.uhn.fhir.jpa.cache.ResourceVersionMap; +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; @@ -134,7 +134,7 @@ public class ResourceVersionSvcTest { mockReturnsFor_getIdsOfExistingResources(pack); // test - ResourceVersionMap retMap = myResourceVersionSvc.getLatestVersionIdsForResourceIds(RequestPartitionId.allPartitions(), + ResourcePersistentIdMap retMap = myResourceVersionSvc.getLatestVersionIdsForResourceIds(RequestPartitionId.allPartitions(), Collections.singletonList(type)); Assertions.assertTrue(retMap.containsKey(type)); @@ -149,7 +149,7 @@ public class ResourceVersionSvcTest { mock_resolveResourcePersistentIdsWithCache_toReturnNothing(); // test - ResourceVersionMap retMap = myResourceVersionSvc.getLatestVersionIdsForResourceIds(RequestPartitionId.allPartitions(), + ResourcePersistentIdMap retMap = myResourceVersionSvc.getLatestVersionIdsForResourceIds(RequestPartitionId.allPartitions(), Collections.singletonList(type)); Assertions.assertTrue(retMap.isEmpty()); @@ -171,7 +171,7 @@ public class ResourceVersionSvcTest { mockReturnsFor_getIdsOfExistingResources(pack); // test - ResourceVersionMap retMap = myResourceVersionSvc.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 57c5b39a76a..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; @@ -330,7 +331,7 @@ public class GiantTransactionPerfTest { } @Override - public ResourceVersionMap getLatestVersionIdsForResourceIds(RequestPartitionId thePartition, List theIds) { + public ResourcePersistentIdMap getLatestVersionIdsForResourceIds(RequestPartitionId thePartition, List theIds) { return null; } } 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 d7566044ccd..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 @@ -40,5 +40,5 @@ public interface IResourceVersionSvc { return getVersionMap(RequestPartitionId.allPartitions(), theResourceName, theSearchParamMap); } - ResourceVersionMap getLatestVersionIdsForResourceIds(RequestPartitionId thePartition, List theIds); + 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..6a723db0606 --- /dev/null +++ b/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/cache/ResourcePersistentIdMap.java @@ -0,0 +1,39 @@ +package ca.uhn.fhir.jpa.cache; + +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(); + } +} 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 3a6ab214906..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 @@ -22,9 +22,10 @@ package ca.uhn.fhir.jpa.cache; import ca.uhn.fhir.jpa.model.entity.ResourceTable; import ca.uhn.fhir.model.primitive.IdDt; -import ca.uhn.fhir.rest.api.server.storage.ResourcePersistentId; 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; @@ -37,9 +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<>(); // Key versionless id, value version - private final Map myMap = new HashMap<>(); + private final Map myMap = new HashMap<>(); private ResourceVersionMap() {} public static ResourceVersionMap fromResourceTableEntities(List theEntities) { @@ -58,19 +60,17 @@ public class ResourceVersionMap { return new ResourceVersionMap(); } - public static ResourceVersionMap fromResourcePersistentIds(List theResourcePersistentIds) { - ResourceVersionMap retval = new ResourceVersionMap(); - theResourcePersistentIds.forEach(resourcePersistentId -> retval.add(resourcePersistentId.getAssociatedResourceId())); - return retval; - } - 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) { + public Long getVersion(IIdType theResourceId) { return get(theResourceId); } @@ -86,7 +86,7 @@ public class ResourceVersionMap { return Collections.unmodifiableSet(mySourceIds); } - public String get(IIdType theId) { + public Long get(IIdType theId) { return myMap.get(new IdDt(theId.toUnqualifiedVersionless())); } @@ -94,16 +94,6 @@ public class ResourceVersionMap { return myMap.containsKey(new IdDt(theId.toUnqualifiedVersionless())); } - public ResourcePersistentId getResourcePersistentId(IIdType theId) { - ResourcePersistentId retval = ResourcePersistentId.fromIIdType(theId); - retval.setVersion(getVersionAsLong(theId)); - return retval; - } - - public Long getVersionAsLong(IIdType theId) { - return Long.valueOf(getVersion(theId)); - } - public boolean isEmpty() { return myMap.isEmpty(); } 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 801d83b9b47..1a45a9c9256 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 @@ -125,10 +125,4 @@ public class ResourcePersistentId { } return retVal; } - - public static ResourcePersistentId fromIIdType(IIdType theId) { - ResourcePersistentId retval = new ResourcePersistentId(theId); - retval.setAssociatedResourceId(theId); - return retval; - } } From e77d84d008a6228fdfa3084b704dc7599ed1d679 Mon Sep 17 00:00:00 2001 From: Ken Stevens Date: Thu, 9 Sep 2021 01:35:35 -0400 Subject: [PATCH 5/7] fix test --- .../rest/api/server/storage/ResourcePersistentId.java | 9 +++++++++ 1 file changed, 9 insertions(+) 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..79a622e268f 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 @@ -21,6 +21,7 @@ package ca.uhn.fhir.rest.api.server.storage; */ import ca.uhn.fhir.util.ObjectUtil; +import org.hl7.fhir.instance.model.api.IBaseResource; import org.hl7.fhir.instance.model.api.IIdType; import java.util.ArrayList; @@ -125,4 +126,12 @@ public class ResourcePersistentId { } return retVal; } + + public static ResourcePersistentId fromResource(IBaseResource theResource) { + IIdType id = theResource.getIdElement(); + ResourcePersistentId retval = new ResourcePersistentId(id.getIdPart()); + retval.setAssociatedResourceId(id); + retval.setVersion(id.getVersionIdPartAsLong()); + return retval; + } } From 661ca02a6a2045a93d54e9e43534fc3d3c5ebf3c Mon Sep 17 00:00:00 2001 From: Ken Stevens Date: Thu, 9 Sep 2021 09:28:01 -0400 Subject: [PATCH 6/7] revert clobbered code --- .../jpa/cache/ResourceVersionSvcDaoImpl.java | 91 ++++++++++++++++++- .../IndexNamePrefixLayoutStrategy.java | 22 ++++- .../jpa/cache/ResourcePersistentIdMap.java | 28 ++++++ .../mdm/api/IMdmBatchJobSubmitterFactory.java | 20 ++++ .../server/storage/ResourcePersistentId.java | 7 +- 5 files changed, 161 insertions(+), 7 deletions(-) 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 c9ca8e0255a..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 @@ -37,7 +37,10 @@ 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; @@ -80,11 +83,93 @@ public class ResourceVersionSvcDaoImpl implements IResourceVersionSvc { } @Override - public ResourcePersistentIdMap getLatestVersionIdsForResourceIds(RequestPartitionId thePartitionId, List theIds) { + /** + * 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, - new ArrayList<>(theIds)); + theIds.stream().collect(Collectors.toList())); - return ResourcePersistentIdMap.fromResourcePersistentIds(resourcePersistentIds); + // 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/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-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 index 6a723db0606..6594bc723b2 100644 --- 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 @@ -1,5 +1,25 @@ 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; @@ -36,4 +56,12 @@ public class ResourcePersistentIdMap { 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-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 79a622e268f..020c043d866 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 @@ -21,7 +21,7 @@ package ca.uhn.fhir.rest.api.server.storage; */ import ca.uhn.fhir.util.ObjectUtil; -import org.hl7.fhir.instance.model.api.IBaseResource; +import org.hl7.fhir.instance.model.api.IAnyResource; import org.hl7.fhir.instance.model.api.IIdType; import java.util.ArrayList; @@ -34,6 +34,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; @@ -127,9 +128,9 @@ public class ResourcePersistentId { return retVal; } - public static ResourcePersistentId fromResource(IBaseResource theResource) { + public static ResourcePersistentId fromResource(IAnyResource theResource) { IIdType id = theResource.getIdElement(); - ResourcePersistentId retval = new ResourcePersistentId(id.getIdPart()); + ResourcePersistentId retval = new ResourcePersistentId(theResource.getUserData(RESOURCE_PID)); retval.setAssociatedResourceId(id); retval.setVersion(id.getVersionIdPartAsLong()); return retval; From 4ab0f7076a607d98569ee0fc7669ae3521cfa6db Mon Sep 17 00:00:00 2001 From: Ken Stevens Date: Thu, 9 Sep 2021 09:29:45 -0400 Subject: [PATCH 7/7] remove problematic method --- .../rest/api/server/storage/ResourcePersistentId.java | 9 --------- 1 file changed, 9 deletions(-) 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 020c043d866..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 @@ -21,7 +21,6 @@ package ca.uhn.fhir.rest.api.server.storage; */ import ca.uhn.fhir.util.ObjectUtil; -import org.hl7.fhir.instance.model.api.IAnyResource; import org.hl7.fhir.instance.model.api.IIdType; import java.util.ArrayList; @@ -127,12 +126,4 @@ public class ResourcePersistentId { } return retVal; } - - public static ResourcePersistentId fromResource(IAnyResource theResource) { - IIdType id = theResource.getIdElement(); - ResourcePersistentId retval = new ResourcePersistentId(theResource.getUserData(RESOURCE_PID)); - retval.setAssociatedResourceId(id); - retval.setVersion(id.getVersionIdPartAsLong()); - return retval; - } }