From cbfcc02a920881c4f2a1e0fef843451fc9161575 Mon Sep 17 00:00:00 2001 From: leif stawnyczy Date: Thu, 2 Sep 2021 11:07:06 -0400 Subject: [PATCH] issue-2901 removing cruft and cleaning up --- .../jpa/dao/AutoVersioningServiceImpl.java | 2 +- .../ca/uhn/fhir/jpa/dao/BaseStorageDao.java | 2 +- .../jpa/dao/BaseTransactionProcessor.java | 3 +- .../fhir/jpa/dao/IAutoVersioningService.java | 2 +- .../uhn/fhir/jpa/dao/data/IForcedIdDao.java | 9 +++++ .../dao/AutoVersioningServiceImplTests.java | 35 +++++++++++++++++-- .../jpa/dao/BaseHapiFhirResourceDaoTest.java | 9 +++++ ...irResourceDaoCreatePlaceholdersR4Test.java | 6 ---- 8 files changed, 54 insertions(+), 14 deletions(-) diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/AutoVersioningServiceImpl.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/AutoVersioningServiceImpl.java index 1953273dbed..0df25f61e27 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/AutoVersioningServiceImpl.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/AutoVersioningServiceImpl.java @@ -18,7 +18,7 @@ public class AutoVersioningServiceImpl implements IAutoVersioningService { private DaoRegistry myDaoRegistry; @Override - public Map getAutoversionsForIds(Collection theIds) { + public Map getExistingAutoversionsForIds(Collection theIds) { HashMap idToPID = new HashMap<>(); HashMap> resourceTypeToIds = new HashMap<>(); 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 2ad93b1d994..b56f0547dd2 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 @@ -209,7 +209,7 @@ public abstract class BaseStorageDao { IIdType referenceElement = nextReference.getReferenceElement(); if (!referenceElement.hasBaseUrl()) { - Map idToPID = myAutoVersioningService.getAutoversionsForIds( + Map idToPID = myAutoVersioningService.getExistingAutoversionsForIds( Collections.singletonList(referenceElement) ); diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseTransactionProcessor.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseTransactionProcessor.java index 724aedc31cd..bf9bbf779ac 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 @@ -117,7 +117,6 @@ import java.util.Map; import java.util.Optional; import java.util.Set; import java.util.TreeSet; -import java.util.concurrent.Callable; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; @@ -1269,7 +1268,7 @@ public abstract class BaseTransactionProcessor { } else { // get a map of // existing ids -> PID (for resources that exist in the DB) - Map idToPID = myAutoVersioningService.getAutoversionsForIds(theReferencesToAutoVersion.stream() + Map idToPID = myAutoVersioningService.getExistingAutoversionsForIds(theReferencesToAutoVersion.stream() .map(ref -> ref.getReferenceElement()).collect(Collectors.toList())); for (IBaseReference baseRef : theReferencesToAutoVersion) { diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/IAutoVersioningService.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/IAutoVersioningService.java index 720edc41478..312c6902a9f 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/IAutoVersioningService.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/IAutoVersioningService.java @@ -19,5 +19,5 @@ public interface IAutoVersioningService { * @param theIds * @return */ - Map getAutoversionsForIds(Collection theIds); + Map getExistingAutoversionsForIds(Collection theIds); } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/IForcedIdDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/IForcedIdDao.java index 0676f124e90..4854fe421e1 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/IForcedIdDao.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/IForcedIdDao.java @@ -111,6 +111,15 @@ public interface IForcedIdDao extends JpaRepository { "WHERE f.myResourceType = :resource_type AND f.myForcedId IN ( :forced_id )") Collection findAndResolveByForcedIdWithNoType(@Param("resource_type") String theResourceType, @Param("forced_id") Collection theForcedIds); + /** + * This method returns a collection where eah row is an element in the collection. + * Each element in the collection is an object array where order matters. + * The returned order of each object array element is: + * ResourceType (Patient, etc - String), ForcedId (String), ResourcePID (Long), Version (Long) + * @param theResourceType + * @param theForcedIds + * @return + */ @Query("" + "SELECT " + " f.myResourceType, f.myForcedId, f.myResourcePid, t.myVersion " + diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/AutoVersioningServiceImplTests.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/AutoVersioningServiceImplTests.java index 6d8d848b01b..da32e0cbc35 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/AutoVersioningServiceImplTests.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/AutoVersioningServiceImplTests.java @@ -13,7 +13,7 @@ import org.mockito.Mock; import org.mockito.Mockito; import org.mockito.junit.jupiter.MockitoExtension; -import java.util.Collection; +import java.util.Arrays; import java.util.Collections; import java.util.HashMap; import java.util.Map; @@ -38,8 +38,10 @@ public class AutoVersioningServiceImplTests { IFhirResourceDao daoMock = Mockito.mock(IFhirResourceDao.class); Mockito.when(daoMock.getIdsOfExistingResources(Mockito.anyList())) .thenReturn(map); + Mockito.when(daoRegistry.getResourceDao(Mockito.anyString())) + .thenReturn(daoMock); - Map retMap = myAutoversioningService.getAutoversionsForIds(Collections.singletonList(type)); + Map retMap = myAutoversioningService.getExistingAutoversionsForIds(Collections.singletonList(type)); Assertions.assertTrue(retMap.containsKey(type)); Assertions.assertEquals(pid.getVersion(), map.get(type).getVersion()); @@ -52,11 +54,38 @@ public class AutoVersioningServiceImplTests { IFhirResourceDao daoMock = Mockito.mock(IFhirResourceDao.class); Mockito.when(daoMock.getIdsOfExistingResources(Mockito.anyList())) .thenReturn(new HashMap<>()); + Mockito.when(daoRegistry.getResourceDao(Mockito.anyString())) + .thenReturn(daoMock); - Map retMap = myAutoversioningService.getAutoversionsForIds(Collections.singletonList(type)); + Map retMap = myAutoversioningService.getExistingAutoversionsForIds(Collections.singletonList(type)); Assertions.assertTrue(retMap.isEmpty()); } + @Test + public void getAutoversionsForIds_whenSomeResourcesDoNotExist_returnsOnlyExistingElements() { + IIdType type = new IdDt("Patient/RED"); + ResourcePersistentId pid = new ResourcePersistentId(1); + pid.setVersion(2L); + HashMap map = new HashMap<>(); + map.put(type, pid); + IIdType type2 = new IdDt("Patient/BLUE"); + // when + IFhirResourceDao daoMock = Mockito.mock(IFhirResourceDao.class); + Mockito.when(daoMock.getIdsOfExistingResources(Mockito.anyList())) + .thenReturn(map); + Mockito.when(daoRegistry.getResourceDao(Mockito.anyString())) + .thenReturn(daoMock); + + // test + Map retMap = myAutoversioningService.getExistingAutoversionsForIds( + Arrays.asList(type, type2) + ); + + // verify + Assertions.assertEquals(map.size(), retMap.size()); + Assertions.assertTrue(retMap.containsKey(type)); + Assertions.assertFalse(retMap.containsKey(type2)); + } } diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDaoTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDaoTest.java index 8292ce448fa..951c8a97a38 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDaoTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDaoTest.java @@ -34,6 +34,7 @@ import static org.junit.jupiter.api.Assertions.fail; @ExtendWith(MockitoExtension.class) class BaseHapiFhirResourceDaoTest { + // our simple concrete test class for BaseHapiFhirResourceDao private class SimpleTestDao extends BaseHapiFhirResourceDao { public SimpleTestDao() { super(); @@ -58,6 +59,14 @@ class BaseHapiFhirResourceDaoTest { //TODO - all other dependency mocks + /** + * Creates a match entry to be returned by myIForcedIdDao. + * This ordering matters (see IForcedIdDao) + * @param theId + * @param thePID + * @param theResourceVersion + * @return + */ private Object[] createMatchEntryForGetIdsOfExistingResources(IIdType theId, long thePID, long theResourceVersion) { Object[] arr = new Object[] { theId.getResourceType(), diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoCreatePlaceholdersR4Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoCreatePlaceholdersR4Test.java index 0451769f045..7945896fa60 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoCreatePlaceholdersR4Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoCreatePlaceholdersR4Test.java @@ -25,7 +25,6 @@ import org.hl7.fhir.r4.model.Reference; import org.hl7.fhir.r4.model.Task; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Assertions; -import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; import java.util.List; @@ -598,9 +597,7 @@ public class FhirResourceDaoCreatePlaceholdersR4Test extends BaseJpaR4Test { // create Patient patient = new Patient(); -// patient.setId(patientId); patient.setIdElement(new IdType(patientId)); -// patient.setActive(true); myPatientDao.update(patient); // update @@ -624,7 +621,4 @@ public class FhirResourceDaoCreatePlaceholdersR4Test extends BaseJpaR4Test { Observation retObservation = myObservationDao.read(obs.getIdElement()); Assertions.assertTrue(retObservation != null); } - - // always work with the put - // conditional create (replace put with conditional create?) }