Fully fleshed test, partial refactor

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;

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 = buildHashPredicateFromTokenParam((TokenParam)param, requestPartitionId, cb, from, next);
-							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);
-							}
 
+							//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<Long, MatchUrlToResolve> hashToSearchMap = buildHashToSearchMap(searchParameterMapsToResolve);
+
 					TypedQuery<ResourceIndexedSearchParamToken> query = myEntityManager.createQuery(cq);
 					List<ResourceIndexedSearchParamToken> results = query.getResultList();
+
 					for (ResourceIndexedSearchParamToken nextResult : results) {
+						Optional<MatchUrlToResolve> matchedSearch = Optional
+							.fromNullable(hashToSearchMap.get(nextResult.getHashSystemAndValue()))
+							.or(Optional.fromNullable(hashToSearchMap.get(nextResult.getHashValue())));
 
-						for (MatchUrlToResolve nextSearchParameterMap : searchParameterMapsToResolve) {
+						if (matchedSearch.isPresent()) {
-							if (nextSearchParameterMap.myHashSystemAndValue != null && nextSearchParameterMap.myHashSystemAndValue.equals(nextResult.getHashSystemAndValue())) {
+							setSearchToResolvedAndPrefetchFoundResourcePid(theTransactionDetails, idsToPreFetch, nextResult, matchedSearch.get());
-								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;
-							}
-
 						}
-
 					}
-
+					//For each SP Map which did not return a result, tag it as not found.
-					for (MatchUrlToResolve nextSearchParameterMap : searchParameterMapsToResolve) {
+					searchParameterMapsToResolve.stream()
 						// No matches
-						if (!nextSearchParameterMap.myResolved) {
+						.filter(match -> !match.myResolved)
-							theTransactionDetails.addResolvedMatchUrl(nextSearchParameterMap.myRequestUrl, TransactionDetails.NOT_FOUND);
+						.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<ResourceIndexedSearchParamToken> 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<Long, MatchUrlToResolve> buildHashToSearchMap(List<MatchUrlToResolve> searchParameterMapsToResolve) {
+		Map<Long, MatchUrlToResolve> 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<Long> 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<ResourceTable> preFetchIndexes(List<Long> ids, String typeDesc, String fieldName) {
 		TypedQuery<ResourceTable> 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;
+		}
 	}
 }

hapi-fhir-jpaserver-base/src/test/resources/logback-test.xml

@@ -42,7 +42,7 @@
 		<appender-ref ref="STDOUT" />
 	</logger>
 
-	<logger name="ca.uhn.fhir.jpa.dao" additivity="false" level="info">
+	<logger name="ca.uhn.fhir.jpa.dao" additivity="false" level="debug">
 		<appender-ref ref="STDOUT" />
 	</logger>