From e031ad203e139c104e5e72710c476b715960c25b Mon Sep 17 00:00:00 2001 From: Ken Stevens Date: Thu, 23 Jan 2020 19:12:05 -0500 Subject: [PATCH] FIXME --- .../uhn/fhir/jpa/bulk/BulkDataExportSvcImpl.java | 5 +---- .../java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java | 4 ++-- .../uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java | 3 +-- .../src/main/java/ca/uhn/fhir/jpa/dao/IDao.java | 2 +- .../java/ca/uhn/fhir/jpa/dao/ISearchBuilder.java | 2 -- .../java/ca/uhn/fhir/jpa/dao/SearchBuilder.java | 15 +++++---------- .../ca/uhn/fhir/jpa/dao/SearchBuilderFactory.java | 3 ++- .../fhir/jpa/dao/predicate/PredicateBuilder.java | 2 +- .../dao/predicate/PredicateBuilderReference.java | 1 - .../ca/uhn/fhir/jpa/dao/predicate/QueryRoot.java | 2 +- .../fhir/jpa/dao/predicate/QueryRootEntry.java | 4 ++-- .../jpa/search/PersistedJpaBundleProvider.java | 4 +--- .../fhir/jpa/search/SearchCoordinatorSvcImpl.java | 6 ++---- .../ca/uhn/fhir/jpa/dao/SearchBuilderTest.java | 2 +- .../jpa/search/SearchCoordinatorSvcImplTest.java | 4 ++-- 15 files changed, 22 insertions(+), 37 deletions(-) diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/bulk/BulkDataExportSvcImpl.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/bulk/BulkDataExportSvcImpl.java index 83c58acc11a..83fc0eb4707 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/bulk/BulkDataExportSvcImpl.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/bulk/BulkDataExportSvcImpl.java @@ -213,9 +213,8 @@ public class BulkDataExportSvcImpl implements IBulkDataExportSvc { ourLog.info("Bulk export assembling export of type {} for job {}", nextType, theJobUuid); - ISearchBuilder sb = dao.newSearchBuilder(); Class nextTypeClass = myContext.getResourceDefinition(nextType).getImplementingClass(); - sb.setType(nextTypeClass, nextType); + ISearchBuilder sb = dao.newSearchBuilder(nextType, nextTypeClass); SearchParameterMap map = new SearchParameterMap(); map.setLoadSynchronous(true); @@ -225,8 +224,6 @@ public class BulkDataExportSvcImpl implements IBulkDataExportSvc { IResultIterator resultIterator = sb.createQuery(map, new SearchRuntimeDetails(null, theJobUuid), null); storeResultsToFiles(nextCollection, sb, resultIterator, jobResourceCounter, jobStopwatch); - - } job.setStatus(BulkJobStatusEnum.COMPLETE); diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java index bc507738736..f150865d643 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java @@ -456,8 +456,8 @@ public abstract class BaseHapiFhirDao extends BaseStora // TODO KHS inject a searchBuilderFactory into callers of this method and delete this method @Override - public SearchBuilder newSearchBuilder() { - return mySearchBuilderFactory.newSearchBuilder(this); + public SearchBuilder newSearchBuilder(String theResourceName, Class theResourceType) { + return mySearchBuilderFactory.newSearchBuilder(this, theResourceName, theResourceType); } public void notifyInterceptors(RestOperationTypeEnum theOperationType, ActionRequestDetails theRequestDetails) { diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java index 56bf3b703a0..b179788743b 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java @@ -1126,8 +1126,7 @@ public abstract class BaseHapiFhirResourceDao extends B @Override public Set searchForIds(SearchParameterMap theParams, RequestDetails theRequest) { - SearchBuilder builder = newSearchBuilder(); - builder.setType(getResourceType(), getResourceName()); + SearchBuilder builder = newSearchBuilder(getResourceName(), getResourceType()); // FIXME: fail if too many results diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/IDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/IDao.java index 376269a17ba..a3ce82c8ca3 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/IDao.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/IDao.java @@ -48,7 +48,7 @@ public interface IDao { */ void injectDependenciesIntoBundleProvider(PersistedJpaBundleProvider theProvider); - ISearchBuilder newSearchBuilder(); + ISearchBuilder newSearchBuilder(String theResourceName, Class theResourceType); IBaseResource toResource(BaseHasResource theEntity, boolean theForHistoryOperation); diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/ISearchBuilder.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/ISearchBuilder.java index 97eb73abbba..22e99cd783c 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/ISearchBuilder.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/ISearchBuilder.java @@ -54,8 +54,6 @@ public interface ISearchBuilder { */ void setFetchSize(int theFetchSize); - void setType(Class theResourceType, String theResourceName); - void setPreviouslyAddedResourcePids(List thePreviouslyAddedResourcePids); } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/SearchBuilder.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/SearchBuilder.java index 212f9c2ad9c..28c44aa0926 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/SearchBuilder.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/SearchBuilder.java @@ -127,21 +127,23 @@ public class SearchBuilder implements ISearchBuilder { private CriteriaBuilder myBuilder; private BaseHapiFhirDao myCallingDao; private SearchParameterMap myParams; - private String myResourceName; - private Class myResourceType; private String mySearchUuid; private int myFetchSize; private Integer myMaxResultsToFetch; private Set myPidSet; private PredicateBuilder myPredicateBuilder; private final QueryRoot myQueryRoot = new QueryRoot(); + private final String myResourceName; + private final Class myResourceType; /** * Constructor */ - SearchBuilder(BaseHapiFhirDao theDao) { + SearchBuilder(BaseHapiFhirDao theDao, String theResourceName, Class theResourceType) { myCallingDao = theDao; myDaoConfig = theDao.getConfig(); + myResourceName = theResourceName; + myResourceType = theResourceType; myDontUseHashesForSearch = myDaoConfig.getDisableHashBasedSearches(); } @@ -860,13 +862,6 @@ public class SearchBuilder implements ISearchBuilder { myFetchSize = theFetchSize; } - // FIXME KHS move this into constructor and make these final - @Override - public void setType(Class theResourceType, String theResourceName) { - myResourceType = theResourceType; - myResourceName = theResourceName; - } - @VisibleForTesting void setParamsForUnitTest(SearchParameterMap theParams) { myParams = theParams; diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/SearchBuilderFactory.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/SearchBuilderFactory.java index 61a38aa8c01..8c7ec79e3c6 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/SearchBuilderFactory.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/SearchBuilderFactory.java @@ -20,11 +20,12 @@ package ca.uhn.fhir.jpa.dao; * #L% */ +import org.hl7.fhir.instance.model.api.IBaseResource; import org.springframework.beans.factory.annotation.Lookup; import org.springframework.stereotype.Service; @Service public abstract class SearchBuilderFactory { @Lookup - public abstract SearchBuilder newSearchBuilder(BaseHapiFhirDao theBaseHapiFhirResourceDao); + public abstract SearchBuilder newSearchBuilder(BaseHapiFhirDao theBaseHapiFhirResourceDao, String theResourceName, Class theResourceType); } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/predicate/PredicateBuilder.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/predicate/PredicateBuilder.java index 14320931288..8bcf8e89b47 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/predicate/PredicateBuilder.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/predicate/PredicateBuilder.java @@ -36,7 +36,7 @@ public class PredicateBuilder { } void addPredicateCoords(String theResourceName, String theParamName, List theNextAnd) { - myPredicateBuilderCoords.addPredicate(theResourceName, theParamName, theNextAnd); + myPredicateBuilderCoords.addPredicate(theResourceName, theParamName, theNextAnd, null); } Predicate addPredicateDate(String theResourceName, String theParamName, List theNextAnd, SearchFilterParser.CompareOperation theOperation) { diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/predicate/PredicateBuilderReference.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/predicate/PredicateBuilderReference.java index 7272bccf4f5..5ce41ac770c 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/predicate/PredicateBuilderReference.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/predicate/PredicateBuilderReference.java @@ -502,7 +502,6 @@ class PredicateBuilderReference extends BasePredicateBuilder { // TODO: we clear the predicates below because the filter builds up // its own collection of predicates. It'd probably be good at some // point to do something more fancy... - // FIXME KHS ArrayList holdPredicates = new ArrayList<>(myQueryRoot.getPredicates()); Predicate filterPredicate = processFilter(filter, theResourceName, theRequest); diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/predicate/QueryRoot.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/predicate/QueryRoot.java index 1539edbf72a..5427eb14dc0 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/predicate/QueryRoot.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/predicate/QueryRoot.java @@ -4,6 +4,7 @@ import ca.uhn.fhir.jpa.model.entity.ResourceIndexedSearchParamDate; import ca.uhn.fhir.jpa.model.entity.ResourceTable; import javax.persistence.criteria.*; +import java.util.Collections; import java.util.List; import java.util.Stack; @@ -60,7 +61,6 @@ public class QueryRoot { top().clearPredicates(); } - // FIXME KHS don't leak List getPredicates() { return top().getPredicates(); } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/predicate/QueryRootEntry.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/predicate/QueryRootEntry.java index 4d65ce27cd2..319702b059a 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/predicate/QueryRootEntry.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/predicate/QueryRootEntry.java @@ -5,6 +5,7 @@ import ca.uhn.fhir.jpa.model.entity.ResourceTable; import javax.persistence.criteria.*; import java.util.ArrayList; +import java.util.Collections; import java.util.List; public class QueryRootEntry { @@ -54,9 +55,8 @@ public class QueryRootEntry { myPredicates.clear(); } - // FIXME KHS don't leak List getPredicates() { - return myPredicates; + return Collections.unmodifiableList(myPredicates); } public void where(Predicate theAnd) { diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/PersistedJpaBundleProvider.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/PersistedJpaBundleProvider.java index 8e989884a0d..c561b54aafc 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/PersistedJpaBundleProvider.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/PersistedJpaBundleProvider.java @@ -170,11 +170,9 @@ public class PersistedJpaBundleProvider implements IBundleProvider { // No resources to fetch (e.g. we did a _summary=count search) return Collections.emptyList(); } - final ISearchBuilder sb = myDao.newSearchBuilder(); - String resourceName = mySearchEntity.getResourceType(); Class resourceType = myContext.getResourceDefinition(resourceName).getImplementingClass(); - sb.setType(resourceType, resourceName); + final ISearchBuilder sb = myDao.newSearchBuilder(resourceName, resourceType); final List pidsSubList = mySearchCoordinatorSvc.getResources(myUuid, theFromIndex, theToIndex, myRequest); diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/SearchCoordinatorSvcImpl.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/SearchCoordinatorSvcImpl.java index ca1e6de2187..8a5e046827a 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/SearchCoordinatorSvcImpl.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/SearchCoordinatorSvcImpl.java @@ -286,8 +286,7 @@ public class SearchCoordinatorSvcImpl implements ISearchCoordinatorSvc { ourLog.debug("Registering new search {}", searchUuid); Class resourceTypeClass = myContext.getResourceDefinition(theResourceType).getImplementingClass(); - final ISearchBuilder sb = theCallingDao.newSearchBuilder(); - sb.setType(resourceTypeClass, theResourceType); + final ISearchBuilder sb = theCallingDao.newSearchBuilder(theResourceType, resourceTypeClass); sb.setFetchSize(mySyncSize); final Integer loadSynchronousUpTo = getLoadSynchronousUpToOrNull(theCacheControlDirective); @@ -645,8 +644,7 @@ public class SearchCoordinatorSvcImpl implements ISearchCoordinatorSvc { private ISearchBuilder newSearchBuilder() { Class resourceTypeClass = myContext.getResourceDefinition(myResourceType).getImplementingClass(); - ISearchBuilder sb = myCallingDao.newSearchBuilder(); - sb.setType(resourceTypeClass, myResourceType); + ISearchBuilder sb = myCallingDao.newSearchBuilder(myResourceType, resourceTypeClass); return sb; } diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/SearchBuilderTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/SearchBuilderTest.java index 96d27cb2899..e63f84989c4 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/SearchBuilderTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/SearchBuilderTest.java @@ -28,7 +28,7 @@ public class SearchBuilderTest { public void testIncludeIterator() { BaseHapiFhirDao mockDao = mock(BaseHapiFhirDao.class); when(mockDao.getConfig()).thenReturn(new DaoConfig()); - SearchBuilder searchBuilder = new SearchBuilder(mockDao); + SearchBuilder searchBuilder = new SearchBuilder(mockDao, null, null); searchBuilder.setParamsForUnitTest(new SearchParameterMap()); EntityManager mockEntityManager = mock(EntityManager.class); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/search/SearchCoordinatorSvcImplTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/search/SearchCoordinatorSvcImplTest.java index 67e050ebcaa..dc0084e8fe9 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/search/SearchCoordinatorSvcImplTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/search/SearchCoordinatorSvcImplTest.java @@ -77,7 +77,7 @@ public class SearchCoordinatorSvcImplTest { @After public void after() { System.clearProperty(SearchCoordinatorSvcImpl.UNIT_TEST_CAPTURE_STACK); - verify(myCallingDao, atMost(myExpectedNumberOfSearchBuildersCreated)).newSearchBuilder(); + verify(myCallingDao, atMost(myExpectedNumberOfSearchBuildersCreated)).newSearchBuilder(any(), any()); } @Before @@ -97,7 +97,7 @@ public class SearchCoordinatorSvcImplTest { DaoConfig daoConfig = new DaoConfig(); mySvc.setDaoConfigForUnitTest(daoConfig); - when(myCallingDao.newSearchBuilder()).thenReturn(mySearchBuilder); + when(myCallingDao.newSearchBuilder(any(), any())).thenReturn(mySearchBuilder); when(myTxManager.getTransaction(any())).thenReturn(mock(TransactionStatus.class));