6404 lucene search fulltext fix (#6517)

hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_8_0/6404-fulltext-search-with-lastupdate.yaml

@@ -0,0 +1,8 @@
+---
+type: fix
+issue: 6404
+title: "Searches using fulltext search that combined `_lastUpdated` query parameter with
+        any other (supported) fulltext query parameter would find no matches, even
+        if matches existed.
+        This has been corrected.
+"

hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_8_0/upgrade.md

@@ -0,0 +1,4 @@
+# Fulltext Search with _lastUpdated Filter
+
+Fulltext searches have been updated to support `_lastUpdated` search parameter. A reindexing of Search Parameters
+is required to migrate old data to support the `_lastUpdated` search parameter.

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

@@ -1689,7 +1689,7 @@ public void populateFullTextFields(
 			theEntity.setContentText(parseContentTextIntoWords(theContext, theResource));
 			if (myStorageSettings.isAdvancedHSearchIndexing()) {
 				ExtendedHSearchIndexData hSearchIndexData =
-						myFulltextSearchSvc.extractLuceneIndexData(theResource, theNewParams);
+						myFulltextSearchSvc.extractLuceneIndexData(theResource, theEntity, theNewParams);
 				theEntity.setLuceneIndexData(hSearchIndexData);
 			}
 		}

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

@@ -135,13 +135,13 @@ public FulltextSearchSvcImpl() {
 
 	@Override
 	public ExtendedHSearchIndexData extractLuceneIndexData(
-			IBaseResource theResource, ResourceIndexedSearchParams theNewParams) {
+			IBaseResource theResource, ResourceTable theEntity, ResourceIndexedSearchParams theNewParams) {
 		String resourceType = myFhirContext.getResourceType(theResource);
 		ResourceSearchParams activeSearchParams = mySearchParamRegistry.getActiveSearchParams(
 				resourceType, ISearchParamRegistry.SearchParamLookupContextEnum.SEARCH);
 		ExtendedHSearchIndexExtractor extractor = new ExtendedHSearchIndexExtractor(
 				myStorageSettings, myFhirContext, activeSearchParams, mySearchParamExtractor);
-		return extractor.extract(theResource, theNewParams);
+		return extractor.extract(theResource, theEntity, theNewParams);
 	}
 
 	@Override

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

@@ -89,7 +89,7 @@ <T extends IResourcePersistentId> List<T> everything(
 	boolean isDisabled();
 
 	ExtendedHSearchIndexData extractLuceneIndexData(
-			IBaseResource theResource, ResourceIndexedSearchParams theNewParams);
+			IBaseResource theResource, ResourceTable theEntity, ResourceIndexedSearchParams theNewParams);
 
 	/**
 	 * Returns true if the parameter map can be handled for hibernate search.

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

@@ -392,7 +392,7 @@ public void addReferenceUnchainedSearch(
 
 	/**
 	 * Create date clause from date params. The date lower and upper bounds are taken
-	 * into considertion when generating date query ranges
+	 * into consideration when generating date query ranges
 	 *
 	 * <p>Example 1 ('eq' prefix/empty): <code>http://fhirserver/Observation?date=eq2020</code>
 	 * would generate the following search clause

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

@@ -25,6 +25,8 @@
 import ca.uhn.fhir.jpa.model.entity.ResourceIndexedSearchParamDate;
 import ca.uhn.fhir.jpa.model.entity.ResourceIndexedSearchParamQuantity;
 import ca.uhn.fhir.jpa.model.entity.ResourceLink;
+import ca.uhn.fhir.jpa.model.entity.ResourceTable;
+import ca.uhn.fhir.jpa.model.entity.TagDefinition;
 import ca.uhn.fhir.jpa.model.search.CompositeSearchIndexData;
 import ca.uhn.fhir.jpa.model.search.DateSearchIndexData;
 import ca.uhn.fhir.jpa.model.search.ExtendedHSearchIndexData;
@@ -37,6 +39,7 @@
 import ca.uhn.fhir.util.MetaUtil;
 import com.google.common.base.Strings;
 import jakarta.annotation.Nonnull;
+import org.apache.commons.lang3.ObjectUtils;
 import org.hl7.fhir.instance.model.api.IBase;
 import org.hl7.fhir.instance.model.api.IBaseCoding;
 import org.hl7.fhir.instance.model.api.IBaseResource;
@@ -74,8 +77,10 @@ public ExtendedHSearchIndexExtractor(
 	}
 
 	@Nonnull
-	public ExtendedHSearchIndexData extract(IBaseResource theResource, ResourceIndexedSearchParams theNewParams) {
-		ExtendedHSearchIndexData retVal = new ExtendedHSearchIndexData(myContext, myJpaStorageSettings, theResource);
+	public ExtendedHSearchIndexData extract(
+			IBaseResource theResource, ResourceTable theEntity, ResourceIndexedSearchParams theNewParams) {
+		ExtendedHSearchIndexData retVal =
+				new ExtendedHSearchIndexData(myContext, myJpaStorageSettings, theResource, theEntity);
 
 		if (myJpaStorageSettings.isStoreResourceInHSearchIndex()) {
 			retVal.setRawResourceData(myContext.newJsonParser().encodeResourceToString(theResource));
@@ -113,11 +118,27 @@ public ExtendedHSearchIndexData extract(IBaseResource theResource, ResourceIndex
 				.filter(nextParam -> !nextParam.isMissing())
 				.forEach(nextParam -> retVal.addUriIndexData(nextParam.getParamName(), nextParam.getUri()));
 
-		theResource.getMeta().getTag().forEach(tag -> retVal.addTokenIndexData("_tag", tag));
-
-		theResource.getMeta().getSecurity().forEach(sec -> retVal.addTokenIndexData("_security", sec));
-
-		theResource.getMeta().getProfile().forEach(prof -> retVal.addUriIndexData("_profile", prof.getValue()));
+		theEntity.getTags().forEach(tag -> {
+			TagDefinition td = tag.getTag();
+
+			IBaseCoding coding = (IBaseCoding) myContext.getVersion().newCodingDt();
+			coding.setVersion(td.getVersion());
+			coding.setDisplay(td.getDisplay());
+			coding.setCode(td.getCode());
+			coding.setSystem(td.getSystem());
+			coding.setUserSelected(ObjectUtils.defaultIfNull(td.getUserSelected(), false));
+			switch (td.getTagType()) {
+				case TAG:
+					retVal.addTokenIndexData("_tag", coding);
+					break;
+				case PROFILE:
+					retVal.addUriIndexData("_profile", coding.getCode());
+					break;
+				case SECURITY_LABEL:
+					retVal.addTokenIndexData("_security", coding);
+					break;
+			}
+		});
 
 		String source = MetaUtil.getSource(myContext, theResource.getMeta());
 		if (isNotBlank(source)) {
@@ -127,20 +148,14 @@ public ExtendedHSearchIndexData extract(IBaseResource theResource, ResourceIndex
 		theNewParams.myCompositeParams.forEach(nextParam ->
 				retVal.addCompositeIndexData(nextParam.getSearchParamName(), buildCompositeIndexData(nextParam)));
 
-		if (theResource.getMeta().getLastUpdated() != null) {
-			int ordinal = ResourceIndexedSearchParamDate.calculateOrdinalValue(
-							theResource.getMeta().getLastUpdated())
+		if (theEntity.getUpdated() != null && !theEntity.getUpdated().isEmpty()) {
+			int ordinal = ResourceIndexedSearchParamDate.calculateOrdinalValue(theEntity.getUpdatedDate())
 					.intValue();
 			retVal.addDateIndexData(
-					"_lastUpdated",
-					theResource.getMeta().getLastUpdated(),
-					ordinal,
-					theResource.getMeta().getLastUpdated(),
-					ordinal);
+					"_lastUpdated", theEntity.getUpdatedDate(), ordinal, theEntity.getUpdatedDate(), ordinal);
 		}
 
 		if (!theNewParams.myLinks.isEmpty()) {
-
 			// awkwardly, links are indexed by jsonpath, not by search param.
 			// so we re-build the linkage.
 			Map<String, List<String>> linkPathToParamName = new HashMap<>();

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

@@ -28,6 +28,7 @@
 import ca.uhn.fhir.rest.api.SearchContainedModeEnum;
 import ca.uhn.fhir.rest.param.CompositeParam;
 import ca.uhn.fhir.rest.param.DateParam;
+import ca.uhn.fhir.rest.param.DateRangeParam;
 import ca.uhn.fhir.rest.param.NumberParam;
 import ca.uhn.fhir.rest.param.QuantityParam;
 import ca.uhn.fhir.rest.param.ReferenceParam;
@@ -89,19 +90,28 @@ public boolean illegalForHibernateSearch(String theParamName, ResourceSearchPara
 	 * be inaccurate and wrong.
 	 */
 	public boolean canUseHibernateSearch(
-			String theResourceType, SearchParameterMap myParams, ISearchParamRegistry theSearchParamRegistry) {
+			String theResourceType, SearchParameterMap theParams, ISearchParamRegistry theSearchParamRegistry) {
 		boolean canUseHibernate = false;
 
 		ResourceSearchParams resourceActiveSearchParams = theSearchParamRegistry.getActiveSearchParams(
 				theResourceType, ISearchParamRegistry.SearchParamLookupContextEnum.SEARCH);
-		for (String paramName : myParams.keySet()) {
+		for (String paramName : theParams.keySet()) {
+			// special SearchParam handling:
+			// _lastUpdated
+			if (theParams.getLastUpdated() != null) {
+				canUseHibernate = !illegalForHibernateSearch(Constants.PARAM_LASTUPDATED, resourceActiveSearchParams);
+				if (!canUseHibernate) {
+					return false;
+				}
+			}
+
 			// is this parameter supported?
 			if (illegalForHibernateSearch(paramName, resourceActiveSearchParams)) {
 				canUseHibernate = false;
 			} else {
 				// are the parameter values supported?
 				canUseHibernate =
-						myParams.get(paramName).stream()
+						theParams.get(paramName).stream()
 								.flatMap(Collection::stream)
 								.collect(Collectors.toList())
 								.stream()
@@ -136,6 +146,7 @@ public boolean isSupportsAllOf(SearchParameterMap myParams) {
 
 				// not yet supported in HSearch
 				myParams.getSearchContainedMode() == SearchContainedModeEnum.FALSE
+				&& supportsLastUpdated(myParams)
 				&& // ???
 				myParams.entrySet().stream()
 						.filter(e -> !ourUnsafeSearchParmeters.contains(e.getKey()))
@@ -145,6 +156,19 @@ public boolean isSupportsAllOf(SearchParameterMap myParams) {
 						.allMatch(this::isParamTypeSupported);
 	}
 
+	private boolean supportsLastUpdated(SearchParameterMap theMap) {
+		if (theMap.getLastUpdated() == null || theMap.getLastUpdated().isEmpty()) {
+			return true;
+		}
+
+		DateRangeParam lastUpdated = theMap.getLastUpdated();
+
+		return lastUpdated.getLowerBound() != null
+				&& isParamTypeSupported(lastUpdated.getLowerBound())
+				&& lastUpdated.getUpperBound() != null
+				&& isParamTypeSupported(lastUpdated.getUpperBound());
+	}
+
 	/**
 	 * Do we support this query param type+modifier?
 	 * <p>

hapi-fhir-jpaserver-elastic-test-utilities/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchWithElasticSearchIT.java

@@ -294,6 +294,20 @@ public List<StorageProcessingMessage> getMessages() {
 		}
 	}
 
+	@Test
+	public void testNoOpUpdateDoesNotModifyLastUpdated() throws InterruptedException {
+		myStorageSettings.setAdvancedHSearchIndexing(true);
+		Patient patient = new Patient();
+		patient.getNameFirstRep().setFamily("graham").addGiven("gary");
+
+		patient = (Patient) myPatientDao.create(patient).getResource();
+		Date originalLastUpdated = patient.getMeta().getLastUpdated();
+
+		patient = (Patient) myPatientDao.update(patient).getResource();
+		Date newLastUpdated = patient.getMeta().getLastUpdated();
+
+		assertThat(originalLastUpdated).isEqualTo(newLastUpdated);
+	}
 
 	@Test
 	public void testFullTextSearchesArePerformanceLogged() {
@@ -1804,68 +1818,52 @@ public void resetResourceStorage() {
 
 		@Test
 		public void eq() {
-			myCaptureQueriesListener.clear();
 			List<String> allIds = myTestDaoSearch.searchForIds("/Observation?_lastUpdated=eq" + myOldLastUpdatedDateTime);
 
-			assertThat(myCaptureQueriesListener.getSelectQueriesForCurrentThread().size()).as("we build the bundle with no sql").isEqualTo(0);
 			assertThat(allIds).containsExactly(myOldObsId);
 		}
 
 		@Test
 		public void eqLessPrecisionRequest() {
-			myCaptureQueriesListener.clear();
 			List<String> allIds = myTestDaoSearch.searchForIds("/Observation?_lastUpdated=eq2017-03-24");
 
-			assertThat(myCaptureQueriesListener.getSelectQueriesForCurrentThread().size()).as("we build the bundle with no sql").isEqualTo(0);
 			assertThat(allIds).containsExactly(myOldObsId);
 		}
 
 		@Test
 		public void ne() {
-			myCaptureQueriesListener.clear();
 			List<String> allIds = myTestDaoSearch.searchForIds("/Observation?_lastUpdated=ne" + myOldLastUpdatedDateTime);
 
-			assertThat(myCaptureQueriesListener.getSelectQueriesForCurrentThread().size()).as("we build the bundle with no sql").isEqualTo(0);
 			assertThat(allIds).containsExactly(myNewObsId);
 		}
 
 		@Test
 		void gt() {
-			myCaptureQueriesListener.clear();
 			List<String> allIds = myTestDaoSearch.searchForIds("/Observation?_lastUpdated=gt2018-01-01");
 
-			assertThat(myCaptureQueriesListener.getSelectQueriesForCurrentThread().size()).as("we build the bundle with no sql").isEqualTo(0);
 			assertThat(allIds).containsExactly(myNewObsId);
 		}
 
 		@Test
 		public void ge() {
-			myCaptureQueriesListener.clear();
 			List<String> allIds = myTestDaoSearch.searchForIds("/Observation?_lastUpdated=ge" + myOldLastUpdatedDateTime);
 
-			assertThat(myCaptureQueriesListener.getSelectQueriesForCurrentThread().size()).as("we build the bundle with no sql").isEqualTo(0);
 			assertThat(allIds).containsExactly(myOldObsId, myNewObsId);
 		}
 
 		@Test
 		void lt() {
-			myCaptureQueriesListener.clear();
 			List<String> allIds = myTestDaoSearch.searchForIds("/Observation?_lastUpdated=lt2018-01-01");
 
-			assertThat(myCaptureQueriesListener.getSelectQueriesForCurrentThread().size()).as("we build the bundle with no sql").isEqualTo(0);
 			assertThat(allIds).containsExactly(myOldObsId);
 		}
 
 		@Test
 		public void le() {
-			myCaptureQueriesListener.clear();
 			List<String> allIds = myTestDaoSearch.searchForIds("/Observation?_lastUpdated=le" + myOldLastUpdatedDateTime);
 
-			assertThat(myCaptureQueriesListener.getSelectQueriesForCurrentThread().size()).as("we build the bundle with no sql").isEqualTo(0);
 			assertThat(allIds).containsExactly(myOldObsId);
 		}
-
-
 	}
 
 	@Nested

hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/search/ExtendedHSearchIndexData.java

@@ -20,6 +20,7 @@
 package ca.uhn.fhir.jpa.model.search;
 
 import ca.uhn.fhir.context.FhirContext;
+import ca.uhn.fhir.jpa.model.entity.ResourceTable;
 import ca.uhn.fhir.jpa.model.entity.StorageSettings;
 import ca.uhn.fhir.model.dstu2.composite.CodingDt;
 import com.google.common.collect.HashMultimap;
@@ -57,12 +58,17 @@ public class ExtendedHSearchIndexData {
 	private String myForcedId;
 	private String myResourceJSON;
 	private IBaseResource myResource;
+	private ResourceTable myEntity;
 
 	public ExtendedHSearchIndexData(
-			FhirContext theFhirContext, StorageSettings theStorageSettings, IBaseResource theResource) {
+			FhirContext theFhirContext,
+			StorageSettings theStorageSettings,
+			IBaseResource theResource,
+			ResourceTable theEntity) {
 		this.myFhirContext = theFhirContext;
 		this.myStorageSettings = theStorageSettings;
 		myResource = theResource;
+		myEntity = theEntity;
 	}
 
 	private <V> BiConsumer<String, V> ifNotContained(BiConsumer<String, V> theIndexWriter) {