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 29c993d2109..4a961a979a8 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 @@ -1125,6 +1125,7 @@ public abstract class BaseTransactionProcessor { IIdType targetId = resourceReference.getResource().getIdElement(); if (targetId.getValue() == null || targetId.getValue().startsWith("#")) { // This means it's a contained resource + ourLog.error("THIS THING ISN'T CONTAINED! WHY IS IT CONTAINED!!! "); continue; } else if (theIdSubstitutions.containsValue(targetId)) { newId = targetId; diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/TransactionProcessor.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/TransactionProcessor.java index 45c04b47be7..95a9f9d78b8 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/TransactionProcessor.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/TransactionProcessor.java @@ -41,11 +41,13 @@ import ca.uhn.fhir.rest.param.TokenParam; import ca.uhn.fhir.util.StopWatch; import com.google.common.annotations.VisibleForTesting; import org.apache.commons.lang3.Validate; +import org.apache.jena.ext.com.google.common.base.Optional; import org.hibernate.internal.SessionImpl; import org.hl7.fhir.instance.model.api.IBase; import org.hl7.fhir.instance.model.api.IBaseBundle; import org.hl7.fhir.instance.model.api.IBaseResource; import org.hl7.fhir.instance.model.api.IIdType; +import org.jetbrains.annotations.Nullable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; @@ -61,6 +63,7 @@ import javax.persistence.criteria.Predicate; import javax.persistence.criteria.Root; import java.util.ArrayList; import java.util.Collection; +import java.util.HashMap; import java.util.HashSet; import java.util.IdentityHashMap; import java.util.List; @@ -218,18 +221,10 @@ public class TransactionProcessor extends BaseTransactionProcessor { IQueryParameterType param = andList.get(0).get(0); if (param instanceof TokenParam) { - TokenParam tokenParam = (TokenParam) param; - Predicate hashPredicate = null; - if (isNotBlank(tokenParam.getValue()) && isNotBlank(tokenParam.getSystem())) { - next.myHashSystemAndValue = ResourceIndexedSearchParamToken.calculateHashSystemAndValue(myPartitionSettings, requestPartitionId, next.myResourceDefinition.getName(), next.myMatchUrlSearchMap.keySet().iterator().next(), tokenParam.getSystem(), tokenParam.getValue()); - hashPredicate = cb.equal(from.get("myHashSystemAndValue").as(Long.class), next.myHashSystemAndValue); - } else if (isNotBlank(tokenParam.getValue())) { - next.myHashValue = ResourceIndexedSearchParamToken.calculateHashValue(myPartitionSettings, requestPartitionId, next.myResourceDefinition.getName(), next.myMatchUrlSearchMap.keySet().iterator().next(), tokenParam.getValue()); - hashPredicate = cb.equal(from.get("myHashValue").as(Long.class), next.myHashValue); - } + Predicate hashPredicate = buildHashPredicateFromTokenParam((TokenParam)param, requestPartitionId, cb, from, next); + //Some partition witchcraft if (hashPredicate != null) { - if (myPartitionSettings.isPartitioningEnabled() && !myPartitionSettings.isIncludePartitionInSearchHashes()) { if (requestPartitionId.isDefaultPartition()) { Predicate partitionIdCriteria = cb.isNull(from.get("myPartitionIdValue").as(Integer.class)); @@ -250,35 +245,28 @@ public class TransactionProcessor extends BaseTransactionProcessor { if (orPredicates.size() > 1) { cq.where(cb.or(orPredicates.toArray(EMPTY_PREDICATE_ARRAY))); + Map hashToSearchMap = buildHashToSearchMap(searchParameterMapsToResolve); + TypedQuery query = myEntityManager.createQuery(cq); List results = query.getResultList(); + for (ResourceIndexedSearchParamToken nextResult : results) { + Optional matchedSearch = Optional + .fromNullable(hashToSearchMap.get(nextResult.getHashSystemAndValue())) + .or(Optional.fromNullable(hashToSearchMap.get(nextResult.getHashValue()))); - for (MatchUrlToResolve nextSearchParameterMap : searchParameterMapsToResolve) { - if (nextSearchParameterMap.myHashSystemAndValue != null && nextSearchParameterMap.myHashSystemAndValue.equals(nextResult.getHashSystemAndValue())) { - idsToPreFetch.add(nextResult.getResourcePid()); - myMatchResourceUrlService.matchUrlResolved(theTransactionDetails, nextSearchParameterMap.myResourceDefinition.getName(), nextSearchParameterMap.myRequestUrl, new ResourcePersistentId(nextResult.getResourcePid())); - theTransactionDetails.addResolvedMatchUrl(nextSearchParameterMap.myRequestUrl, new ResourcePersistentId(nextResult.getResourcePid())); - nextSearchParameterMap.myResolved = true; - } - if (nextSearchParameterMap.myHashValue != null && nextSearchParameterMap.myHashValue.equals(nextResult.getHashValue())) { - idsToPreFetch.add(nextResult.getResourcePid()); - myMatchResourceUrlService.matchUrlResolved(theTransactionDetails, nextSearchParameterMap.myResourceDefinition.getName(), nextSearchParameterMap.myRequestUrl, new ResourcePersistentId(nextResult.getResourcePid())); - theTransactionDetails.addResolvedMatchUrl(nextSearchParameterMap.myRequestUrl, new ResourcePersistentId(nextResult.getResourcePid())); - nextSearchParameterMap.myResolved = true; - } - + if (matchedSearch.isPresent()) { + setSearchToResolvedAndPrefetchFoundResourcePid(theTransactionDetails, idsToPreFetch, nextResult, matchedSearch.get()); } - } - - for (MatchUrlToResolve nextSearchParameterMap : searchParameterMapsToResolve) { + //For each SP Map which did not return a result, tag it as not found. + searchParameterMapsToResolve.stream() // No matches - if (!nextSearchParameterMap.myResolved) { - theTransactionDetails.addResolvedMatchUrl(nextSearchParameterMap.myRequestUrl, TransactionDetails.NOT_FOUND); - } - } - + .filter(match -> !match.myResolved) + .forEach(match -> { + ourLog.warn("Was unable to match url {} from database", match.myRequestUrl); + theTransactionDetails.addResolvedMatchUrl(match.myRequestUrl, TransactionDetails.NOT_FOUND); + }); } } @@ -320,6 +308,45 @@ public class TransactionProcessor extends BaseTransactionProcessor { return super.doTransactionWriteOperations(theRequest, theActionName, theTransactionDetails, theAllIds, theIdSubstitutions, theIdToPersistedOutcome, theResponse, theOriginalRequestOrder, theEntries, theTransactionStopWatch); } + /** + * Given a token parameter, build the query predicate based on its hash. Uses system and value if both are available, otherwise just value. + * If neither are available, it returns null. + */ + @Nullable + private Predicate buildHashPredicateFromTokenParam(TokenParam theTokenParam, RequestPartitionId theRequestPartitionId, CriteriaBuilder cb, Root from, MatchUrlToResolve theMatchUrl) { + Predicate hashPredicate = null; + if (isNotBlank(theTokenParam.getValue()) && isNotBlank(theTokenParam.getSystem())) { + theMatchUrl.myHashSystemAndValue = ResourceIndexedSearchParamToken.calculateHashSystemAndValue(myPartitionSettings, theRequestPartitionId, theMatchUrl.myResourceDefinition.getName(), theMatchUrl.myMatchUrlSearchMap.keySet().iterator().next(), theTokenParam.getSystem(), theTokenParam.getValue()); + hashPredicate = cb.equal(from.get("myHashSystemAndValue").as(Long.class), theMatchUrl.myHashSystemAndValue); + } else if (isNotBlank(theTokenParam.getValue())) { + theMatchUrl.myHashValue = ResourceIndexedSearchParamToken.calculateHashValue(myPartitionSettings, theRequestPartitionId, theMatchUrl.myResourceDefinition.getName(), theMatchUrl.myMatchUrlSearchMap.keySet().iterator().next(), theTokenParam.getValue()); + hashPredicate = cb.equal(from.get("myHashValue").as(Long.class), theMatchUrl.myHashValue); + } + return hashPredicate; + } + + private Map buildHashToSearchMap(List searchParameterMapsToResolve) { + Map hashToSearch = new HashMap<>(); + //Build a lookup map so we don't have to iterate over the searches repeatedly. + for (MatchUrlToResolve nextSearchParameterMap : searchParameterMapsToResolve) { + if (nextSearchParameterMap.myHashSystemAndValue != null) { + hashToSearch.put(nextSearchParameterMap.myHashSystemAndValue, nextSearchParameterMap); + } + if (nextSearchParameterMap.myHashValue!= null) { + hashToSearch.put(nextSearchParameterMap.myHashValue, nextSearchParameterMap); + } + } + return hashToSearch; + } + + private void setSearchToResolvedAndPrefetchFoundResourcePid(TransactionDetails theTransactionDetails, List idsToPreFetch, ResourceIndexedSearchParamToken nextResult, MatchUrlToResolve nextSearchParameterMap) { + ourLog.warn("Matched url {} from database", nextSearchParameterMap.myRequestUrl); + idsToPreFetch.add(nextResult.getResourcePid()); + myMatchResourceUrlService.matchUrlResolved(theTransactionDetails, nextSearchParameterMap.myResourceDefinition.getName(), nextSearchParameterMap.myRequestUrl, new ResourcePersistentId(nextResult.getResourcePid())); + theTransactionDetails.addResolvedMatchUrl(nextSearchParameterMap.myRequestUrl, new ResourcePersistentId(nextResult.getResourcePid())); + nextSearchParameterMap.setResolved(true); + } + private List preFetchIndexes(List ids, String typeDesc, String fieldName) { TypedQuery query = myEntityManager.createQuery("FROM ResourceTable r LEFT JOIN FETCH r." + fieldName + " WHERE r.myId IN ( :IDS )", ResourceTable.class); query.setParameter("IDS", ids); @@ -379,5 +406,8 @@ public class TransactionProcessor extends BaseTransactionProcessor { myMatchUrlSearchMap = theMatchUrlSearchMap; myResourceDefinition = theResourceDefinition; } + public void setResolved(boolean theResolved) { + myResolved = theResolved; + } } } diff --git a/hapi-fhir-jpaserver-base/src/test/resources/logback-test.xml b/hapi-fhir-jpaserver-base/src/test/resources/logback-test.xml index ac75e0d04be..36a83af6599 100644 --- a/hapi-fhir-jpaserver-base/src/test/resources/logback-test.xml +++ b/hapi-fhir-jpaserver-base/src/test/resources/logback-test.xml @@ -42,7 +42,7 @@ - +