From b299d0b63f759e5231227d946d6f383c6e502122 Mon Sep 17 00:00:00 2001 From: Tadgh Date: Fri, 6 Mar 2020 11:39:39 -0800 Subject: [PATCH] Fix Typos, reduce code duplication * Change Two streams into one. * Fix typos. * Begin Writing docstrings * Rename variables for clarity --- .../ca/uhn/fhir/jpa/dao/SearchBuilder.java | 50 +++++++++---------- .../fhir/jpa/dao/index/IdHelperService.java | 28 +++++------ 2 files changed, 39 insertions(+), 39 deletions(-) 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 008b0a8fe7e..107a5eab81b 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 @@ -128,7 +128,7 @@ public class SearchBuilder implements ISearchBuilder { @Autowired private PredicateBuilderFactory myPredicateBuilderFactory; private List myAlsoIncludePids; - private CriteriaBuilder myBuilder; + private CriteriaBuilder myCriteriaBuilder; private IDao myCallingDao; private SearchParameterMap myParams; private String mySearchUuid; @@ -214,10 +214,10 @@ public class SearchBuilder implements ISearchBuilder { return new QueryIterator(theSearchRuntimeDetails, theRequest); } - private void init(SearchParameterMap theParams, String theTheSearchUuid) { + private void init(SearchParameterMap theParams, String theSearchUuid) { myParams = theParams; - myBuilder = myEntityManager.getCriteriaBuilder(); - mySearchUuid = theTheSearchUuid; + myCriteriaBuilder = myEntityManager.getCriteriaBuilder(); + mySearchUuid = theSearchUuid; myPredicateBuilder = new PredicateBuilder(this, myPredicateBuilderFactory); } @@ -232,50 +232,50 @@ public class SearchBuilder implements ISearchBuilder { if (sort != null) { assert !theCount; - outerQuery = myBuilder.createQuery(Long.class); + outerQuery = myCriteriaBuilder.createQuery(Long.class); myQueryRoot.push(outerQuery); if (theCount) { - outerQuery.multiselect(myBuilder.countDistinct(myQueryRoot.getRoot())); + outerQuery.multiselect(myCriteriaBuilder.countDistinct(myQueryRoot.getRoot())); } else { outerQuery.multiselect(myQueryRoot.get("myId").as(Long.class)); } List orders = Lists.newArrayList(); - createSort(myBuilder, myQueryRoot, sort, orders); + createSort(myCriteriaBuilder, myQueryRoot, sort, orders); if (orders.size() > 0) { outerQuery.orderBy(orders); } } else { - outerQuery = myBuilder.createQuery(Long.class); + outerQuery = myCriteriaBuilder.createQuery(Long.class); myQueryRoot.push(outerQuery); if (theCount) { - outerQuery.multiselect(myBuilder.countDistinct(myQueryRoot.getRoot())); + outerQuery.multiselect(myCriteriaBuilder.countDistinct(myQueryRoot.getRoot())); } else { outerQuery.multiselect(myQueryRoot.get("myId").as(Long.class)); // KHS This distinct call is causing performance issues in large installations // outerQuery.distinct(true); } - } if (myParams.getEverythingMode() != null) { Join join = myQueryRoot.join("myResourceLinks", JoinType.LEFT); if (myParams.get(IAnyResource.SP_RES_ID) != null) { - StringParam idParm = (StringParam) myParams.get(IAnyResource.SP_RES_ID).get(0).get(0); - ResourcePersistentId pid = myIdHelperService.translateForcedIdToPid(myResourceName, idParm.getValue(), theRequest); + StringParam idParam = (StringParam) myParams.get(IAnyResource.SP_RES_ID).get(0).get(0); + ResourcePersistentId pid = myIdHelperService.translateForcedIdToPid(myResourceName, idParam.getValue(), theRequest); if (myAlsoIncludePids == null) { myAlsoIncludePids = new ArrayList<>(1); } myAlsoIncludePids.add(pid); - myQueryRoot.addPredicate(myBuilder.equal(join.get("myTargetResourcePid").as(Long.class), pid.getIdAsLong())); + //Won't this fail on non-long IDs? Or does IdHelperService massage this? + myQueryRoot.addPredicate(myCriteriaBuilder.equal(join.get("myTargetResourcePid").as(Long.class), pid.getIdAsLong())); } else { - Predicate targetTypePredicate = myBuilder.equal(join.get("myTargetResourceType").as(String.class), myResourceName); - Predicate sourceTypePredicate = myBuilder.equal(myQueryRoot.get("myResourceType").as(String.class), myResourceName); - myQueryRoot.addPredicate(myBuilder.or(sourceTypePredicate, targetTypePredicate)); + Predicate targetTypePredicate = myCriteriaBuilder.equal(join.get("myTargetResourceType").as(String.class), myResourceName); + Predicate sourceTypePredicate = myCriteriaBuilder.equal(myQueryRoot.get("myResourceType").as(String.class), myResourceName); + myQueryRoot.addPredicate(myCriteriaBuilder.or(sourceTypePredicate, targetTypePredicate)); } } else { @@ -318,17 +318,17 @@ public class SearchBuilder implements ISearchBuilder { */ if (!myQueryRoot.hasIndexJoins()) { if (myParams.getEverythingMode() == null) { - myQueryRoot.addPredicate(myBuilder.equal(myQueryRoot.get("myResourceType"), myResourceName)); + myQueryRoot.addPredicate(myCriteriaBuilder.equal(myQueryRoot.get("myResourceType"), myResourceName)); } - myQueryRoot.addPredicate(myBuilder.isNull(myQueryRoot.get("myDeleted"))); + myQueryRoot.addPredicate(myCriteriaBuilder.isNull(myQueryRoot.get("myDeleted"))); } // Last updated DateRangeParam lu = myParams.getLastUpdated(); - List lastUpdatedPredicates = createLastUpdatedPredicates(lu, myBuilder, myQueryRoot.getRoot()); + List lastUpdatedPredicates = createLastUpdatedPredicates(lu, myCriteriaBuilder, myQueryRoot.getRoot()); myQueryRoot.addPredicates(lastUpdatedPredicates); - myQueryRoot.where(myBuilder.and(myQueryRoot.getPredicateArray())); + myQueryRoot.where(myCriteriaBuilder.and(myQueryRoot.getPredicateArray())); /* * Now perform the search @@ -850,7 +850,7 @@ public class SearchBuilder implements ISearchBuilder { private void addPredicateCompositeStringUnique(@Nonnull SearchParameterMap theParams, String theIndexedString) { myQueryRoot.setHasIndexJoins(true); Join join = myQueryRoot.join("myParamsCompositeStringUnique", JoinType.LEFT); - Predicate predicate = myBuilder.equal(join.get("myIndexString"), theIndexedString); + Predicate predicate = myCriteriaBuilder.equal(join.get("myIndexString"), theIndexedString); myQueryRoot.addPredicate(predicate); // Remove any empty parameters remaining after this @@ -877,7 +877,7 @@ public class SearchBuilder implements ISearchBuilder { } public CriteriaBuilder getBuilder() { - return myBuilder; + return myCriteriaBuilder; } public QueryRoot getQueryRoot() { @@ -954,7 +954,7 @@ public class SearchBuilder implements ISearchBuilder { private final SearchRuntimeDetails mySearchRuntimeDetails; private final RequestDetails myRequest; private final boolean myHaveRawSqlHooks; - private final boolean myHavePerftraceFoundIdHook; + private final boolean myHavePerfTraceFoundIdHook; private boolean myFirst = true; private IncludesIterator myIncludesIterator; private ResourcePersistentId myNext; @@ -975,7 +975,7 @@ public class SearchBuilder implements ISearchBuilder { myStillNeedToFetchIncludes = true; } - myHavePerftraceFoundIdHook = JpaInterceptorBroadcaster.hasHooks(Pointcut.JPA_PERFTRACE_SEARCH_FOUND_ID, myInterceptorBroadcaster, myRequest); + myHavePerfTraceFoundIdHook = JpaInterceptorBroadcaster.hasHooks(Pointcut.JPA_PERFTRACE_SEARCH_FOUND_ID, myInterceptorBroadcaster, myRequest); myHaveRawSqlHooks = JpaInterceptorBroadcaster.hasHooks(Pointcut.JPA_PERFTRACE_RAW_SQL, myInterceptorBroadcaster, myRequest); } @@ -1017,7 +1017,7 @@ public class SearchBuilder implements ISearchBuilder { if (myNext == null) { while (myResultsIterator.hasNext()) { Long nextLong = myResultsIterator.next(); - if (myHavePerftraceFoundIdHook) { + if (myHavePerfTraceFoundIdHook) { HookParams params = new HookParams() .add(Integer.class, System.identityHashCode(this)) .add(Object.class, nextLong); 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 a8e7cc82bcd..b93c94a8a25 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 @@ -43,6 +43,7 @@ import org.springframework.stereotype.Service; import javax.annotation.Nonnull; import java.util.*; +import java.util.stream.Stream; import static org.apache.commons.lang3.StringUtils.isBlank; @@ -68,6 +69,9 @@ public class IdHelperService { } /** + * Given a forced ID, convert it to it's Long value. Since you are allowed to use string IDs for resources, we need to + * convert those to the underlying Long values that are stored, for lookup and comparison purposes. + * * @throws ResourceNotFoundException If the ID can not be found */ @Nonnull @@ -96,6 +100,8 @@ public class IdHelperService { List retVal = new ArrayList<>(); ListMultimap typeToIds = MultimapBuilder.hashKeys().arrayListValues().build(); + //If the PID is fully numeric and we aren't in ClientIdStrategyEnum.ANY, add it to return value. + //Otherwise, add it to a map of resource type -> ID. for (IIdType nextId : theId) { if (theDaoConfig.getResourceClientIdStrategy() != DaoConfig.ClientIdStrategyEnum.ANY && isValidPid(nextId)) { retVal.add(new ResourcePersistentId(nextId.getIdPartAsLong())); @@ -108,9 +114,11 @@ public class IdHelperService { } } + // For every resource type, fetch all of the requested ids against the forcedPidDao, and add them to the return value. for (Map.Entry> nextEntry : typeToIds.asMap().entrySet()) { String nextResourceType = nextEntry.getKey(); Collection nextIds = nextEntry.getValue(); + List convertedPidStream; if (isBlank(nextResourceType)) { StorageProcessingMessage msg = new StorageProcessingMessage() @@ -121,23 +129,15 @@ public class IdHelperService { .add(StorageProcessingMessage.class, msg); JpaInterceptorBroadcaster.doCallHooks(theInterceptorBroadcaster, theRequest, Pointcut.JPA_PERFTRACE_WARNING, params); - theForcedIdDao - .findByForcedId(nextIds) - .stream() - .map(t->new ResourcePersistentId(t)) - .forEach(t->retVal.add(t)); - + convertedPidStream = theForcedIdDao.findByForcedId(nextIds); } else { - - theForcedIdDao - .findByTypeAndForcedId(nextResourceType, nextIds) - .stream() - .map(t->new ResourcePersistentId(t)) - .forEach(t->retVal.add(t)); - + convertedPidStream = theForcedIdDao.findByTypeAndForcedId(nextResourceType, nextIds); } + convertedPidStream + .stream() + .map(ResourcePersistentId::new) + .forEach(retVal::add); } - return retVal; }