Introduce container ordinal column in the search parameter index tables and populate it

hapi-fhir-base/src/main/java/ca/uhn/fhir/util/FhirTerser.java

@@ -38,7 +38,6 @@
 import ca.uhn.fhir.model.api.ISupportsUndeclaredExtensions;
 import ca.uhn.fhir.model.base.composite.BaseContainedDt;
 import ca.uhn.fhir.model.base.composite.BaseResourceReferenceDt;
-import ca.uhn.fhir.model.primitive.IdDt;
 import ca.uhn.fhir.model.primitive.StringDt;
 import ca.uhn.fhir.parser.DataFormatException;
 import com.google.common.collect.Lists;
@@ -77,7 +76,6 @@
 import static org.apache.commons.lang3.StringUtils.defaultString;
 import static org.apache.commons.lang3.StringUtils.isBlank;
 import static org.apache.commons.lang3.StringUtils.isNotBlank;
-import static org.apache.commons.lang3.StringUtils.substring;
 
 public class FhirTerser {
 
@@ -1330,6 +1328,19 @@ private void visit(
 		theStack.remove(theElement);
 	}
 
+	/**
+	 * Returns the resources embedded in the provided resource.
+	 * @param theResource the resource to scan
+	 * @param theRecurse true if recursion should be applied when discovering embedded resources
+	 * @return the embedded resources object
+	 */
+	public EmbeddedResources getEmbeddedResources(IBaseResource theResource, boolean theRecurse) {
+		Collection<IBaseResource> resourceList = getAllEmbeddedResources(theResource, theRecurse);
+		EmbeddedResources embeddedResources = new EmbeddedResources();
+		resourceList.forEach(embeddedResources::addResource);
+		return embeddedResources;
+	}
+
 	/**
 	 * Returns all embedded resources that are found embedded within <code>theResource</code>.
 	 * An embedded resource is a resource that can be found as a direct child within a resource,
@@ -1348,31 +1359,15 @@ public Collection<IBaseResource> getAllEmbeddedResources(IBaseResource theResour
 		Validate.notNull(theResource, "theResource must not be null");
 		ArrayList<IBaseResource> retVal = new ArrayList<>();
 
-		visit(theResource, new IModelVisitor2() {
-			@Override
-			public boolean acceptElement(
-					IBase theElement,
-					List<IBase> theContainingElementPath,
-					List<BaseRuntimeChildDefinition> theChildDefinitionPath,
-					List<BaseRuntimeElementDefinition<?>> theElementDefinitionPath) {
-				if (theElement == theResource) {
-					return true;
-				}
-				if (theElement instanceof IBaseResource) {
-					retVal.add((IBaseResource) theElement);
-					return theRecurse;
-				}
+		visit(theResource, (theElement, theContainingElementPath, theChildDefinitionPath, theElementDefinitionPath) -> {
+			if (theElement == theResource) {
 				return true;
 			}
-
-			@Override
-			public boolean acceptUndeclaredExtension(
-					IBaseExtension<?, ?> theNextExt,
-					List<IBase> theContainingElementPath,
-					List<BaseRuntimeChildDefinition> theChildDefinitionPath,
-					List<BaseRuntimeElementDefinition<?>> theElementDefinitionPath) {
-				return true;
+			if (theElement instanceof IBaseResource) {
+				retVal.add((IBaseResource) theElement);
+				return theRecurse;
 			}
+			return true;
 		});
 
 		return retVal;
@@ -1408,25 +1403,9 @@ public boolean acceptUndeclaredExtension(
 		});
 	}
 
-	private void containResourcesForEncoding(
+	private void containReferenceTargetsWithNoId(
 			ContainedResources theContained, IBaseResource theResource, boolean theModifyResource) {
-		List<IBaseReference> allReferences = getAllPopulatedChildElementsOfType(theResource, IBaseReference.class);
-		for (IBaseReference next : allReferences) {
-			IBaseResource resource = next.getResource();
-			if (resource == null && next.getReferenceElement().isLocal()) {
-				if (theContained.hasExistingIdToContainedResource()) {
-					IBaseResource potentialTarget = theContained
-							.getExistingIdToContainedResource()
-							.remove(next.getReferenceElement().getValue());
-					if (potentialTarget != null) {
-						theContained.addContained(next.getReferenceElement(), potentialTarget);
-						containResourcesForEncoding(theContained, potentialTarget, theModifyResource);
-					}
-				}
-			}
-		}
-
-		for (IBaseReference next : allReferences) {
+		for (IBaseReference next : getAllPopulatedChildElementsOfType(theResource, IBaseReference.class)) {
 			IBaseResource resource = next.getResource();
 			if (resource != null) {
 				if (resource.getIdElement().isEmpty() || resource.getIdElement().isLocal()) {
@@ -1439,11 +1418,6 @@ private void containResourcesForEncoding(
 						getContainedResourceList(theResource).add(resource);
 						next.setReference(id.getValue());
 					}
-					if (resource.getIdElement().isLocal() && theContained.hasExistingIdToContainedResource()) {
-						theContained
-								.getExistingIdToContainedResource()
-								.remove(resource.getIdElement().getValue());
-					}
 				}
 			}
 		}
@@ -1492,7 +1466,7 @@ public ContainedResources containResources(IBaseResource theResource, OptionsEnu
 		}
 
 		if (myContext.getParserOptions().isAutoContainReferenceTargetsWithNoId()) {
-			containResourcesForEncoding(contained, theResource, modifyResource);
+			containReferenceTargetsWithNoId(contained, theResource, modifyResource);
 		}
 
 		if (storeAndReuse) {
@@ -1760,129 +1734,104 @@ private interface ICompartmentOwnerVisitor {
 		boolean consume(IIdType theCompartmentOwner);
 	}
 
-	public static class ContainedResources {
-		private long myNextContainedId = 1;
-
+	public static class EmbeddedResources {
 		private List<IBaseResource> myResourceList;
-		private IdentityHashMap<IBaseResource, IIdType> myResourceToIdMap;
-		private Map<String, IBaseResource> myExistingIdToContainedResourceMap;
+		private Map<IBaseResource, IIdType> myResourceToIdMap;
+		private Map<IIdType, IBaseResource> myIdToResourceMap;
 
-		public Map<String, IBaseResource> getExistingIdToContainedResource() {
-			if (myExistingIdToContainedResourceMap == null) {
-				myExistingIdToContainedResourceMap = new HashMap<>();
-			}
-			return myExistingIdToContainedResourceMap;
+		void addResource(IBaseResource theResource) {
+			addResource(theResource.getIdElement(), theResource);
 		}
 
-		public IIdType addContained(IBaseResource theResource) {
-			IIdType existing = getResourceToIdMap().get(theResource);
-			if (existing != null) {
-				return existing;
-			}
-
-			IIdType newId = theResource.getIdElement();
-			if (isBlank(newId.getValue())) {
-				newId.setValue("#" + myNextContainedId++);
-			} else {
-				// Avoid auto-assigned contained IDs colliding with pre-existing ones
-				String idPart = newId.getValue();
-				if (substring(idPart, 0, 1).equals("#")) {
-					idPart = idPart.substring(1);
-					if (StringUtils.isNumeric(idPart)) {
-						myNextContainedId = Long.parseLong(idPart) + 1;
-					}
-				}
-			}
-
-			getResourceToIdMap().put(theResource, newId);
+		void addResource(IIdType theId, IBaseResource theResource) {
 			getOrCreateResourceList().add(theResource);
-			return newId;
+			getOrCreateResourceToIdMap().put(theResource, theId);
+			getOrCreateIdToResourceMap().put(theId, theResource);
 		}
 
-		public void addContained(IIdType theId, IBaseResource theResource) {
-			if (!getResourceToIdMap().containsKey(theResource)) {
-				getResourceToIdMap().put(theResource, theId);
-				getOrCreateResourceList().add(theResource);
-			}
-		}
-
-		public List<IBaseResource> getContainedResources() {
-			if (getResourceToIdMap() == null) {
-				return Collections.emptyList();
-			}
-			return getOrCreateResourceList();
+		public IIdType getResourceId(IBaseResource theNext) {
+			return getOrCreateResourceToIdMap().get(theNext);
 		}
 
-		public IIdType getResourceId(IBaseResource theNext) {
-			if (getResourceToIdMap() == null) {
-				return null;
-			}
-			return getResourceToIdMap().get(theNext);
+		public IBaseResource getResource(IIdType theId) {
+			return getOrCreateIdToResourceMap().get(theId);
 		}
 
-		private List<IBaseResource> getOrCreateResourceList() {
+		protected List<IBaseResource> getOrCreateResourceList() {
 			if (myResourceList == null) {
 				myResourceList = new ArrayList<>();
 			}
 			return myResourceList;
 		}
 
-		private IdentityHashMap<IBaseResource, IIdType> getResourceToIdMap() {
+		protected Map<IBaseResource, IIdType> getOrCreateResourceToIdMap() {
 			if (myResourceToIdMap == null) {
 				myResourceToIdMap = new IdentityHashMap<>();
 			}
 			return myResourceToIdMap;
 		}
 
-		public boolean isEmpty() {
-			if (myResourceToIdMap == null) {
-				return true;
+		protected Map<IIdType, IBaseResource> getOrCreateIdToResourceMap() {
+			if (myIdToResourceMap == null) {
+				myIdToResourceMap = new HashMap<>();
 			}
-			return myResourceToIdMap.isEmpty();
+			return myIdToResourceMap;
 		}
 
-		public boolean hasExistingIdToContainedResource() {
-			return myExistingIdToContainedResourceMap != null;
+		public boolean isEmpty() {
+			return myResourceList == null || myResourceList.isEmpty();
 		}
+	}
 
-		public void assignIdsToContainedResources() {
-
-			if (!getContainedResources().isEmpty()) {
+	public static class ContainedResources extends EmbeddedResources {
+		private long myNextContainedId = 1;
+		private Map<IBaseResource, Short> myResourceToOrdMap;
 
-				/*
-				 * The idea with the code block below:
-				 *
-				 * We want to preserve any IDs that were user-assigned, so that if it's really
-				 * important to someone that their contained resource have the ID of #FOO
-				 * or #1 we will keep that.
-				 *
-				 * For any contained resources where no ID was assigned by the user, we
-				 * want to manually create an ID but make sure we don't reuse an existing ID.
-				 */
+		@Override
+		protected void addResource(IIdType theId, IBaseResource theResource) {
+			super.addResource(theId, theResource);
+			short ordinal = Integer.valueOf(getOrCreateResourceList().size()).shortValue();
+			getOrCreateResourceToOrdMap().put(theResource, ordinal);
+		}
 
-				Set<String> ids = new HashSet<>();
+		public IIdType addContained(IBaseResource theResource) {
+			IIdType id = assignIdIfNeeded(theResource);
+			addResource(id, theResource);
+			return id;
+		}
 
-				// Gather any user assigned IDs
-				for (IBaseResource nextResource : getContainedResources()) {
-					if (getResourceToIdMap().get(nextResource) != null) {
-						ids.add(getResourceToIdMap().get(nextResource).getValue());
-					}
-				}
+		private IIdType assignIdIfNeeded(IBaseResource theResource) {
+			/*
+			 * We want to preserve any IDs that were user-assigned, so that if it's really
+			 * important to someone that their contained resource have the ID of #FOO
+			 * or #1 we will keep that.
+			 * For any contained resources where no ID was assigned by the user, we
+			 * want to manually create an ID but make sure we don't reuse an existing ID.
+			 */
+			IIdType newId = theResource.getIdElement();
+			while (newId.isEmpty() || getOrCreateIdToResourceMap().containsKey(newId)) {
+				newId.setValue("#" + myNextContainedId++);
+			}
 
-				// Automatically assign IDs to the rest
-				for (IBaseResource nextResource : getContainedResources()) {
+			return newId;
+		}
 
-					while (getResourceToIdMap().get(nextResource) == null) {
-						String nextCandidate = "#" + myNextContainedId;
-						myNextContainedId++;
-						if (!ids.add(nextCandidate)) {
-							continue;
-						}
+		public List<IBaseResource> getContainedResources() {
+			if (getOrCreateResourceToIdMap() == null) {
+				return Collections.emptyList();
+			}
+			return getOrCreateResourceList();
+		}
 
-						getResourceToIdMap().put(nextResource, new IdDt(nextCandidate));
-					}
-				}
+		private Map<IBaseResource, Short> getOrCreateResourceToOrdMap() {
+			if (myResourceToOrdMap == null) {
+				myResourceToOrdMap = new IdentityHashMap<>();
 			}
+			return myResourceToOrdMap;
+		}
+
+		public Short getContainedResourceOrd(IBaseResource theResource) {
+			return getOrCreateResourceToOrdMap().get(theResource);
 		}
 	}
 }

hapi-fhir-cli/hapi-fhir-cli-api/src/test/java/ca/uhn/fhir/cli/HapiFlywayMigrateDatabaseCommandTest.java

@@ -121,7 +121,7 @@ public void testMigrateFrom340() throws IOException, SQLException {
 			assertEquals("identifier", values.get(0).get("SP_NAME"));
 			assertEquals("12345678", values.get(0).get("SP_VALUE"));
 			assertTrue(values.get(0).keySet().contains("HASH_IDENTITY"));
-			assertEquals(7001889285610424179L, values.get(0).get("HASH_IDENTITY"));
+			assertEquals(792023395537367244L, values.get(0).get("HASH_IDENTITY"));
 			return null;
 		});
 	}

hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/config/JpaConfig.java

@@ -79,8 +79,11 @@
 import ca.uhn.fhir.jpa.interceptor.JpaConsentContextServices;
 import ca.uhn.fhir.jpa.interceptor.OverridePathBasedReferentialIntegrityForDeletesInterceptor;
 import ca.uhn.fhir.jpa.interceptor.validation.RepositoryValidatingRuleBuilder;
+import ca.uhn.fhir.jpa.model.config.PartitionSettings;
 import ca.uhn.fhir.jpa.model.dao.JpaPid;
+import ca.uhn.fhir.jpa.model.entity.StorageSettings;
 import ca.uhn.fhir.jpa.model.sched.ISchedulerService;
+import ca.uhn.fhir.jpa.model.search.hash.ResourceIndexHasher;
 import ca.uhn.fhir.jpa.packages.IHapiPackageCacheManager;
 import ca.uhn.fhir.jpa.packages.IPackageInstallerSvc;
 import ca.uhn.fhir.jpa.packages.JpaPackageCache;
@@ -876,4 +879,10 @@ public ResourceHistoryCalculator resourceHistoryCalculator(
 			FhirContext theFhirContext, HibernatePropertiesProvider theHibernatePropertiesProvider) {
 		return new ResourceHistoryCalculator(theFhirContext, theHibernatePropertiesProvider.isOracleDialect());
 	}
+
+	@Bean
+	public ResourceIndexHasher myResourceIndexHasher(
+			PartitionSettings thePartitionSettings, StorageSettings theStorageSettings) {
+		return new ResourceIndexHasher(thePartitionSettings, theStorageSettings);
+	}
 }

hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/TransactionProcessor.java

@@ -31,6 +31,7 @@
 import ca.uhn.fhir.jpa.model.dao.JpaPid;
 import ca.uhn.fhir.jpa.model.entity.ResourceIndexedSearchParamToken;
 import ca.uhn.fhir.jpa.model.entity.StorageSettings;
+import ca.uhn.fhir.jpa.model.search.hash.ResourceIndexHasher;
 import ca.uhn.fhir.jpa.partition.IRequestPartitionHelperSvc;
 import ca.uhn.fhir.jpa.searchparam.MatchUrlService;
 import ca.uhn.fhir.jpa.searchparam.SearchParameterMap;
@@ -115,6 +116,9 @@ public class TransactionProcessor extends BaseTransactionProcessor {
 	@Autowired
 	private IRequestPartitionHelperSvc myRequestPartitionSvc;
 
+	@Autowired
+	private ResourceIndexHasher myResourceIndexHasher;
+
 	public void setEntityManagerForUnitTest(EntityManager theEntityManager) {
 		myEntityManager = theEntityManager;
 	}
@@ -502,17 +506,15 @@ private void buildHashPredicateFromTokenParam(
 			Set<Long> theSysAndValuePredicates,
 			Set<Long> theValuePredicates) {
 		if (isNotBlank(theTokenParam.getValue()) && isNotBlank(theTokenParam.getSystem())) {
-			theMatchUrl.myHashSystemAndValue = ResourceIndexedSearchParamToken.calculateHashSystemAndValue(
-					myPartitionSettings,
+			theMatchUrl.myHashSystemAndValue = myResourceIndexHasher.hash(
 					theRequestPartitionId,
 					theMatchUrl.myResourceDefinition.getName(),
 					theMatchUrl.myMatchUrlSearchMap.keySet().iterator().next(),
 					theTokenParam.getSystem(),
 					theTokenParam.getValue());
 			theSysAndValuePredicates.add(theMatchUrl.myHashSystemAndValue);
 		} else if (isNotBlank(theTokenParam.getValue())) {
-			theMatchUrl.myHashValue = ResourceIndexedSearchParamToken.calculateHashValue(
-					myPartitionSettings,
+			theMatchUrl.myHashValue = myResourceIndexHasher.hash(
 					theRequestPartitionId,
 					theMatchUrl.myResourceDefinition.getName(),
 					theMatchUrl.myMatchUrlSearchMap.keySet().iterator().next(),

hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/IResourceIndexedSearchParamDateDao.java

@@ -34,5 +34,5 @@ public interface IResourceIndexedSearchParamDateDao
 	void deleteByResourceId(@Param("resid") Long theResourcePid);
 
 	@Query("SELECT t FROM ResourceIndexedSearchParamDate t WHERE t.myResourcePid = :resId")
-	List<ResourceIndexedSearchParamDate> findAllForResourceId(@Param("resId") Long thePatientId);
+	List<ResourceIndexedSearchParamDate> findAllForResourceId(@Param("resId") Long theResourcePid);
 }

hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/IResourceIndexedSearchParamStringDao.java

@@ -35,5 +35,5 @@ public interface IResourceIndexedSearchParamStringDao
 	void deleteByResourceId(@Param("resId") Long theResourcePid);
 
 	@Query("SELECT t FROM ResourceIndexedSearchParamString t WHERE t.myResourcePid = :resId")
-	List<ResourceIndexedSearchParamString> findAllForResourceId(@Param("resId") Long thePatientId);
+	List<ResourceIndexedSearchParamString> findAllForResourceId(@Param("resId") Long theResourcePid);
 }