From 5bc571d97d5da3137045330f563efd356d9ea977 Mon Sep 17 00:00:00 2001 From: Ken Stevens Date: Thu, 9 Sep 2021 01:00:12 -0400 Subject: [PATCH] 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; - } }