From 92db526786ebeb9cde434483c5695ce8a5f4db2d Mon Sep 17 00:00:00 2001 From: michaelabuckley Date: Tue, 22 Mar 2022 18:41:21 -0400 Subject: [PATCH] Skip database query when hibernate search fully satisfies search (#3478) If a query is completely satisfied by Hibernate Search, skip the database. Co-authored-by: Jaison Baskaran --- .../6_0_0/3478-fast-hibernate-search.yaml | 6 + .../ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java | 3 + .../fhir/jpa/dao/BaseHapiFhirResourceDao.java | 1 - .../fhir/jpa/dao/FulltextSearchSvcImpl.java | 59 ++--- .../uhn/fhir/jpa/dao/IFulltextSearchSvc.java | 17 +- .../uhn/fhir/jpa/dao/LegacySearchBuilder.java | 30 ++- .../search/ExtendedLuceneIndexExtractor.java | 2 + .../ExtendedLuceneResourceProjection.java | 34 +++ .../uhn/fhir/jpa/dao/search/package-info.java | 6 + .../ValueSetAutocompleteSearch.java | 2 + .../search/builder/ISearchQueryExecutor.java | 12 ++ .../jpa/search/builder/SearchBuilder.java | 201 +++++++++++++----- .../builder/sql/SearchQueryExecutor.java | 5 +- .../ca/uhn/fhir/jpa/dao/TestDaoSearch.java | 26 ++- .../r4/FhirResourceDaoR4SearchLastNIT.java | 14 +- ...SearchLastNUsingExtendedLuceneIndexIT.java | 43 +++- ...esourceDaoR4SearchWithElasticSearchIT.java | 148 ++++++++++++- ...rResourceDaoR4StandardQueriesNoFTTest.java | 6 +- .../ExtendedLuceneResourceProjectionTest.java | 40 ++++ .../TokenAutocompleteElasticsearchIT.java | 3 +- .../fhir/jpa/model/entity/ResourceTable.java | 11 + .../model/search/ExtendedLuceneIndexData.java | 17 ++ .../search/SearchParamTextPropertyBinder.java | 7 + 23 files changed, 580 insertions(+), 113 deletions(-) create mode 100644 hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_0_0/3478-fast-hibernate-search.yaml create mode 100644 hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/search/ExtendedLuceneResourceProjection.java create mode 100644 hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/ISearchQueryExecutor.java create mode 100644 hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/search/ExtendedLuceneResourceProjectionTest.java diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_0_0/3478-fast-hibernate-search.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_0_0/3478-fast-hibernate-search.yaml new file mode 100644 index 00000000000..86c76933e16 --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_0_0/3478-fast-hibernate-search.yaml @@ -0,0 +1,6 @@ +--- +type: perf +issue: 3478 +title: "When using JPA persistence with Hibernate Search (Lucene or Elasticsearch), simple FHIR queries that can be + satisfied completely by Hibernate Search no longer query the database. Before, every search involved the database, + even when not needed. This is faster when there are many results." diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java index 42a03498c58..f4b47fb80e0 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java @@ -1750,6 +1750,9 @@ public abstract class BaseHapiFhirDao extends BaseStora if (myDaoConfig.isAdvancedLuceneIndexing()) { ExtendedLuceneIndexData luceneIndexData = myFulltextSearchSvc.extractLuceneIndexData(theResource, theNewParams); theEntity.setLuceneIndexData(luceneIndexData); + if(myDaoConfig.isStoreResourceInLuceneIndex()) { + theEntity.setRawResourceData(theContext.newJsonParser().encodeResourceToString(theResource)); + } } } } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java index c36ce45386d..e7f17993087 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java @@ -816,7 +816,6 @@ public abstract class BaseHapiFhirResourceDao extends B IBaseResource oldVersion = toResource(theEntity, false); - List tags = toTagList(theMetaDel); for (TagDefinition nextDef : tags) { diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/FulltextSearchSvcImpl.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/FulltextSearchSvcImpl.java index c714a0a693d..0a905885c65 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/FulltextSearchSvcImpl.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/FulltextSearchSvcImpl.java @@ -23,9 +23,10 @@ package ca.uhn.fhir.jpa.dao; import ca.uhn.fhir.context.FhirContext; import ca.uhn.fhir.i18n.Msg; import ca.uhn.fhir.jpa.api.config.DaoConfig; -import ca.uhn.fhir.jpa.dao.data.IForcedIdDao; +import ca.uhn.fhir.jpa.api.svc.IIdHelperService; import ca.uhn.fhir.jpa.dao.search.ExtendedLuceneClauseBuilder; import ca.uhn.fhir.jpa.dao.search.ExtendedLuceneIndexExtractor; +import ca.uhn.fhir.jpa.dao.search.ExtendedLuceneResourceProjection; import ca.uhn.fhir.jpa.dao.search.ExtendedLuceneSearchBuilder; import ca.uhn.fhir.jpa.dao.search.LastNOperation; import ca.uhn.fhir.jpa.model.entity.ResourceTable; @@ -36,11 +37,9 @@ import ca.uhn.fhir.jpa.searchparam.SearchParameterMap; import ca.uhn.fhir.jpa.searchparam.extractor.ISearchParamExtractor; import ca.uhn.fhir.jpa.searchparam.extractor.ResourceIndexedSearchParams; import ca.uhn.fhir.model.api.IQueryParameterType; +import ca.uhn.fhir.parser.IParser; import ca.uhn.fhir.rest.api.Constants; -import ca.uhn.fhir.rest.api.server.RequestDetails; import ca.uhn.fhir.rest.api.server.storage.ResourcePersistentId; -import ca.uhn.fhir.rest.param.StringParam; -import ca.uhn.fhir.rest.param.TokenParam; import ca.uhn.fhir.rest.server.util.ISearchParamRegistry; import ca.uhn.fhir.rest.server.util.ResourceSearchParams; import org.hibernate.search.backend.elasticsearch.ElasticsearchExtension; @@ -48,7 +47,6 @@ import org.hibernate.search.mapper.orm.Search; import org.hibernate.search.mapper.orm.session.SearchSession; import org.hibernate.search.mapper.orm.work.SearchIndexingPlan; import org.hibernate.search.util.common.SearchException; -import org.hl7.fhir.instance.model.api.IAnyResource; import org.hl7.fhir.instance.model.api.IBaseResource; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.transaction.PlatformTransactionManager; @@ -59,6 +57,7 @@ import javax.annotation.Nonnull; import javax.persistence.EntityManager; import javax.persistence.PersistenceContext; import javax.persistence.PersistenceContextType; +import java.util.Collection; import java.util.List; import java.util.stream.Collectors; @@ -66,8 +65,6 @@ import static org.apache.commons.lang3.StringUtils.isNotBlank; public class FulltextSearchSvcImpl implements IFulltextSearchSvc { private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(FulltextSearchSvcImpl.class); - @Autowired - protected IForcedIdDao myForcedIdDao; @PersistenceContext(type = PersistenceContextType.TRANSACTION) private EntityManager myEntityManager; @Autowired @@ -80,6 +77,9 @@ public class FulltextSearchSvcImpl implements IFulltextSearchSvc { private DaoConfig myDaoConfig; @Autowired ISearchParamExtractor mySearchParamExtractor; + @Autowired + IIdHelperService myIdHelperService; + final private ExtendedLuceneSearchBuilder myAdvancedIndexQueryBuilder = new ExtendedLuceneSearchBuilder(); private Boolean ourDisabled; @@ -140,7 +140,7 @@ public class FulltextSearchSvcImpl implements IFulltextSearchSvc { /* * Handle _text parameter (resource narrative content) * - * Positerity: + * Posterity: * We do not want the HAPI-FHIR dao's to process the * _text parameter, so we remove it from the map here */ @@ -158,7 +158,7 @@ public class FulltextSearchSvcImpl implements IFulltextSearchSvc { /* * Handle other supported parameters */ - if (myDaoConfig.isAdvancedLuceneIndexing()) { + if (myDaoConfig.isAdvancedLuceneIndexing() && theParams.getEverythingMode() == null) { myAdvancedIndexQueryBuilder.addAndConsumeAdvancedQueryClauses(builder, theResourceType, theParams, mySearchParamRegistry); } //DROP EARLY HERE IF BOOL IS EMPTY? @@ -181,26 +181,12 @@ public class FulltextSearchSvcImpl implements IFulltextSearchSvc { } @Override - public List everything(String theResourceName, SearchParameterMap theParams, RequestDetails theRequest) { + public List everything(String theResourceName, SearchParameterMap theParams, ResourcePersistentId theReferencingPid) { - ResourcePersistentId pid = null; - if (theParams.get(IAnyResource.SP_RES_ID) != null) { - String idParamValue; - IQueryParameterType idParam = theParams.get(IAnyResource.SP_RES_ID).get(0).get(0); - if (idParam instanceof TokenParam) { - TokenParam idParm = (TokenParam) idParam; - idParamValue = idParm.getValue(); - } else { - StringParam idParm = (StringParam) idParam; - idParamValue = idParm.getValue(); - } -// pid = myIdHelperService.translateForcedIdToPid_(theResourceName, idParamValue, theRequest); - } - ResourcePersistentId referencingPid = pid; - List retVal = doSearch(null, theParams, referencingPid); - if (referencingPid != null) { - retVal.add(referencingPid); + List retVal = doSearch(null, theParams, theReferencingPid); + if (theReferencingPid != null) { + retVal.add(theReferencingPid); } return retVal; } @@ -271,4 +257,23 @@ public class FulltextSearchSvcImpl implements IFulltextSearchSvc { return convertLongsToResourcePersistentIds(pidList); } + @Override + public List getResources(Collection thePids) { + SearchSession session = getSearchSession(); + List rawResourceDataList = session.search(ResourceTable.class) + .select( + f -> f.composite( + (pid, forcedId, resource)-> new ExtendedLuceneResourceProjection(pid, forcedId, resource), + f.field("myId", Long.class), + f.field("myForcedId", String.class), + f.field("myRawResource", String.class)) + ) + .where( + f -> f.id().matchingAny(thePids) // matches '_id' from resource index + ).fetchAllHits(); + IParser parser = myFhirContext.newJsonParser(); + return rawResourceDataList.stream() + .map(p -> p.toResource(parser)) + .collect(Collectors.toList()); + } } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/IFulltextSearchSvc.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/IFulltextSearchSvc.java index 794b71080fb..02a4cadbdbf 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/IFulltextSearchSvc.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/IFulltextSearchSvc.java @@ -20,18 +20,17 @@ package ca.uhn.fhir.jpa.dao; * #L% */ -import java.util.List; - -import ca.uhn.fhir.context.FhirContext; import ca.uhn.fhir.jpa.model.entity.ResourceTable; import ca.uhn.fhir.jpa.model.search.ExtendedLuceneIndexData; import ca.uhn.fhir.jpa.search.autocomplete.ValueSetAutocompleteOptions; import ca.uhn.fhir.jpa.searchparam.SearchParameterMap; import ca.uhn.fhir.jpa.searchparam.extractor.ResourceIndexedSearchParams; -import ca.uhn.fhir.rest.api.server.RequestDetails; import ca.uhn.fhir.rest.api.server.storage.ResourcePersistentId; import org.hl7.fhir.instance.model.api.IBaseResource; +import java.util.Collection; +import java.util.List; + public interface IFulltextSearchSvc { @@ -52,7 +51,7 @@ public interface IFulltextSearchSvc { */ IBaseResource tokenAutocompleteValueSetSearch(ValueSetAutocompleteOptions theOptions); - List everything(String theResourceName, SearchParameterMap theParams, RequestDetails theRequest); + List everything(String theResourceName, SearchParameterMap theParams, ResourcePersistentId theReferencingPid); boolean isDisabled(); @@ -72,4 +71,12 @@ public interface IFulltextSearchSvc { List lastN(SearchParameterMap theParams, Integer theMaximumResults); + /** + * Returns inlined resource stored along with index mappings for matched identifiers + * + * @param thePids raw pids - we dont support versioned references + * @return Resources list or empty if nothing found + */ + List getResources(Collection thePids); + } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/LegacySearchBuilder.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/LegacySearchBuilder.java index 18948221a62..ce960bd2250 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/LegacySearchBuilder.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/LegacySearchBuilder.java @@ -20,12 +20,12 @@ package ca.uhn.fhir.jpa.dao; * #L% */ -import ca.uhn.fhir.i18n.Msg; import ca.uhn.fhir.context.ComboSearchParamType; import ca.uhn.fhir.context.FhirContext; import ca.uhn.fhir.context.FhirVersionEnum; import ca.uhn.fhir.context.RuntimeResourceDefinition; import ca.uhn.fhir.context.RuntimeSearchParam; +import ca.uhn.fhir.i18n.Msg; import ca.uhn.fhir.interceptor.api.HookParams; import ca.uhn.fhir.interceptor.api.IInterceptorBroadcaster; import ca.uhn.fhir.interceptor.api.Pointcut; @@ -35,7 +35,6 @@ import ca.uhn.fhir.jpa.api.dao.IDao; import ca.uhn.fhir.jpa.api.svc.IIdHelperService; import ca.uhn.fhir.jpa.dao.data.IResourceSearchViewDao; import ca.uhn.fhir.jpa.dao.data.IResourceTagDao; -import ca.uhn.fhir.jpa.dao.index.IdHelperService; import ca.uhn.fhir.jpa.dao.predicate.PredicateBuilder; import ca.uhn.fhir.jpa.dao.predicate.PredicateBuilderFactory; import ca.uhn.fhir.jpa.dao.predicate.SearchBuilderJoinEnum; @@ -54,13 +53,10 @@ import ca.uhn.fhir.jpa.model.search.StorageProcessingMessage; import ca.uhn.fhir.jpa.search.DatabaseBackedPagingProvider; import ca.uhn.fhir.jpa.search.lastn.IElasticsearchSvc; import ca.uhn.fhir.jpa.searchparam.SearchParameterMap; -import ca.uhn.fhir.rest.param.TokenParam; -import ca.uhn.fhir.rest.server.util.ISearchParamRegistry; import ca.uhn.fhir.jpa.searchparam.util.Dstu3DistanceHelper; import ca.uhn.fhir.jpa.searchparam.util.LastNParameterHelper; import ca.uhn.fhir.jpa.util.BaseIterator; import ca.uhn.fhir.jpa.util.CurrentThreadCaptureQueriesListener; -import ca.uhn.fhir.rest.server.util.CompositeInterceptorBroadcaster; import ca.uhn.fhir.jpa.util.QueryChunker; import ca.uhn.fhir.jpa.util.ScrollableResultsIterator; import ca.uhn.fhir.jpa.util.SqlQueryList; @@ -80,8 +76,11 @@ import ca.uhn.fhir.rest.api.server.storage.ResourcePersistentId; import ca.uhn.fhir.rest.param.DateRangeParam; import ca.uhn.fhir.rest.param.ReferenceParam; import ca.uhn.fhir.rest.param.StringParam; +import ca.uhn.fhir.rest.param.TokenParam; import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; import ca.uhn.fhir.rest.server.servlet.ServletRequestDetails; +import ca.uhn.fhir.rest.server.util.CompositeInterceptorBroadcaster; +import ca.uhn.fhir.rest.server.util.ISearchParamRegistry; import ca.uhn.fhir.util.StopWatch; import ca.uhn.fhir.util.UrlUtil; import com.google.common.annotations.VisibleForTesting; @@ -288,7 +287,7 @@ public class LegacySearchBuilder implements ISearchBuilder { } if (myParams.getEverythingMode() != null) { - pids = myFulltextSearchSvc.everything(myResourceName, myParams, theRequest); + pids = queryHibernateSearchForEverythingPids(); } else { pids = myFulltextSearchSvc.search(myResourceName, myParams); } @@ -333,6 +332,25 @@ public class LegacySearchBuilder implements ISearchBuilder { return myQueries; } + private List queryHibernateSearchForEverythingPids() { + ResourcePersistentId pid = null; + if (myParams.get(IAnyResource.SP_RES_ID) != null) { + String idParamValue; + IQueryParameterType idParam = myParams.get(IAnyResource.SP_RES_ID).get(0).get(0); + if (idParam instanceof TokenParam) { + TokenParam idParm = (TokenParam) idParam; + idParamValue = idParm.getValue(); + } else { + StringParam idParm = (StringParam) idParam; + idParamValue = idParm.getValue(); + } + + pid = myIdHelperService.resolveResourcePersistentIds(myRequestPartitionId, myResourceName, idParamValue); + } + List pids = myFulltextSearchSvc.everything(myResourceName, myParams, pid); + return pids; + } + private void doCreateChunkedQueries(List thePids, SortSpec sort, Integer theOffset, boolean theCount, RequestDetails theRequest, ArrayList> theQueries) { if(thePids.size() < getMaximumPageSize()) { normalizeIdListForLastNInClause(thePids); diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/search/ExtendedLuceneIndexExtractor.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/search/ExtendedLuceneIndexExtractor.java index beb9099c533..dd2bc2cc68b 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/search/ExtendedLuceneIndexExtractor.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/search/ExtendedLuceneIndexExtractor.java @@ -61,6 +61,8 @@ public class ExtendedLuceneIndexExtractor { public ExtendedLuceneIndexData extract(IBaseResource theResource, ResourceIndexedSearchParams theNewParams) { ExtendedLuceneIndexData retVal = new ExtendedLuceneIndexData(myContext); + retVal.setForcedId(theResource.getIdElement().getIdPart()); + extractAutocompleteTokens(theResource, retVal); theNewParams.myStringParams.forEach(nextParam -> diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/search/ExtendedLuceneResourceProjection.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/search/ExtendedLuceneResourceProjection.java new file mode 100644 index 00000000000..5b22510b149 --- /dev/null +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/search/ExtendedLuceneResourceProjection.java @@ -0,0 +1,34 @@ +package ca.uhn.fhir.jpa.dao.search; + +import ca.uhn.fhir.model.primitive.IdDt; +import ca.uhn.fhir.parser.IParser; +import org.hl7.fhir.instance.model.api.IBaseResource; + +/** + * Query result when fetching full resources from Hibernate Search. + */ +public class ExtendedLuceneResourceProjection { + final long myPid; + final String myForcedId; + final String myResourceString; + + public ExtendedLuceneResourceProjection(long thePid, String theForcedId, String theResourceString) { + myPid = thePid; + myForcedId = theForcedId; + myResourceString = theResourceString; + } + + public IBaseResource toResource(IParser theParser) { + IBaseResource result = theParser.parseResource(myResourceString); + + IdDt id; + if (myForcedId != null) { + id = new IdDt(myForcedId); + } else { + id = new IdDt(myPid); + } + result.setId(id); + + return result; + } +} diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/search/package-info.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/search/package-info.java index 826bcd700b8..57ff53a2723 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/search/package-info.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/search/package-info.java @@ -16,6 +16,12 @@ * This pid list is used as a narrowing where clause against the remaining unprocessed search parameters in a jdbc query. * The actual queries for the different search types (e.g. token, string, modifiers, etc.) are * generated in {@link ca.uhn.fhir.jpa.dao.search.ExtendedLuceneSearchBuilder}. + *

+ * Full resource bodies can be stored in the Hibernate Search index. + * The {@link ca.uhn.fhir.jpa.dao.search.ExtendedLuceneResourceProjection} is used to extract these. + * This is currently restricted to LastN, and misses tag changes from $meta-add and $meta-delete since those don't + * update Hibernate Search. + *

* *

Operation

* During startup, Hibernate Search uses {@link ca.uhn.fhir.jpa.model.search.SearchParamTextPropertyBinder} to generate a schema. diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/autocomplete/ValueSetAutocompleteSearch.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/autocomplete/ValueSetAutocompleteSearch.java index 497ccb72a19..9d7a20390b7 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/autocomplete/ValueSetAutocompleteSearch.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/autocomplete/ValueSetAutocompleteSearch.java @@ -24,6 +24,7 @@ import ca.uhn.fhir.context.FhirContext; import ca.uhn.fhir.rest.param.TokenParam; import org.hibernate.search.mapper.orm.session.SearchSession; import org.hl7.fhir.instance.model.api.IBaseResource; +import org.hl7.fhir.r4.model.Enumerations; import org.hl7.fhir.r4.model.ValueSet; import java.util.List; @@ -47,6 +48,7 @@ public class ValueSetAutocompleteSearch { ValueSet result = new ValueSet(); ValueSet.ValueSetExpansionComponent expansion = new ValueSet.ValueSetExpansionComponent(); result.setExpansion(expansion); + result.setStatus(Enumerations.PublicationStatus.ACTIVE); aggEntries.stream() .map(this::makeCoding) .forEach(expansion::addContains); diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/ISearchQueryExecutor.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/ISearchQueryExecutor.java new file mode 100644 index 00000000000..b577e5f8105 --- /dev/null +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/ISearchQueryExecutor.java @@ -0,0 +1,12 @@ +package ca.uhn.fhir.jpa.search.builder; + +import java.io.Closeable; +import java.util.Iterator; + +public interface ISearchQueryExecutor extends Iterator, Closeable { + /** + * Narrow the signature - no IOException allowed. + */ + @Override + void close(); +} diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/SearchBuilder.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/SearchBuilder.java index 3a6417b78a3..b305d261c2b 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/SearchBuilder.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/SearchBuilder.java @@ -43,7 +43,6 @@ import ca.uhn.fhir.jpa.dao.IResultIterator; import ca.uhn.fhir.jpa.dao.ISearchBuilder; import ca.uhn.fhir.jpa.dao.data.IResourceSearchViewDao; import ca.uhn.fhir.jpa.dao.data.IResourceTagDao; -import ca.uhn.fhir.jpa.dao.index.IdHelperService; import ca.uhn.fhir.jpa.entity.ResourceSearchView; import ca.uhn.fhir.jpa.interceptor.JpaPreResourceAccessDetails; import ca.uhn.fhir.jpa.model.config.PartitionSettings; @@ -111,7 +110,6 @@ import javax.persistence.EntityManager; import javax.persistence.PersistenceContext; import javax.persistence.PersistenceContextType; import javax.persistence.Query; -import javax.persistence.SqlResultSetMapping; import javax.persistence.Tuple; import javax.persistence.TypedQuery; import javax.persistence.criteria.CriteriaBuilder; @@ -278,11 +276,11 @@ public class SearchBuilder implements ISearchBuilder { init(theParams, theSearchUuid, theRequestPartitionId); - ArrayList queries = createQuery(myParams, null, null, null, true, theRequest, null); + List queries = createQuery(myParams, null, null, null, true, theRequest, null); if (queries.isEmpty()) { return Collections.emptyIterator(); } - try (SearchQueryExecutor queryExecutor = queries.get(0)) { + try (ISearchQueryExecutor queryExecutor = queries.get(0)) { return Lists.newArrayList(queryExecutor.next()).iterator(); } } @@ -317,20 +315,24 @@ public class SearchBuilder implements ISearchBuilder { myRequestPartitionId = theRequestPartitionId; } - private ArrayList createQuery(SearchParameterMap theParams, SortSpec sort, Integer theOffset, Integer theMaximumResults, boolean theCount, RequestDetails theRequest, - SearchRuntimeDetails theSearchRuntimeDetails) { + private List createQuery(SearchParameterMap theParams, SortSpec sort, Integer theOffset, Integer theMaximumResults, boolean theCount, RequestDetails theRequest, + SearchRuntimeDetails theSearchRuntimeDetails) { - List pids = new ArrayList<>(); + ArrayList queries = new ArrayList<>(); if (checkUseHibernateSearch()) { + // we're going to run at least part of the search against the Fulltext service. + List fulltextMatchIds; if (myParams.isLastN()) { - pids = executeLastNAgainstIndex(theMaximumResults); + fulltextMatchIds = executeLastNAgainstIndex(theMaximumResults); + } else if (myParams.getEverythingMode() != null) { + fulltextMatchIds = queryHibernateSearchForEverythingPids(); } else { - pids = queryLuceneForPIDs(theRequest); + fulltextMatchIds = myFulltextSearchSvc.search(myResourceName, myParams); } if (theSearchRuntimeDetails != null) { - theSearchRuntimeDetails.setFoundIndexMatchesCount(pids.size()); + theSearchRuntimeDetails.setFoundIndexMatchesCount(fulltextMatchIds.size()); HookParams params = new HookParams() .add(RequestDetails.class, theRequest) .addIfMatchesType(ServletRequestDetails.class, theRequest) @@ -338,20 +340,39 @@ public class SearchBuilder implements ISearchBuilder { CompositeInterceptorBroadcaster.doCallHooks(myInterceptorBroadcaster, theRequest, Pointcut.JPA_PERFTRACE_INDEXSEARCH_QUERY_COMPLETE, params); } - if (pids.isEmpty()) { - // Will never match - pids = Collections.singletonList(new ResourcePersistentId(-1L)); + List rawPids = ResourcePersistentId.toLongList(fulltextMatchIds); + + // can we skip the database entirely and return the pid list from here? + boolean canSkipDatabase = + // if we processed an AND clause, and it returned nothing, then nothing can match. + rawPids.isEmpty() || + // Our hibernate search query doesn't respect partitions yet + (!myPartitionSettings.isPartitioningEnabled() && + // were there AND terms left? Then we still need the db. + theParams.isEmpty() && + // not every param is a param. :-( + theParams.getNearDistanceParam() == null && + theParams.getLastUpdated() == null && + theParams.getEverythingMode() == null && + theParams.getOffset() == null && + // or sorting? + theParams.getSort() == null + // todo MB Ugh - review with someone else + //theParams.toNormalizedQueryString(myContext).length() <= 1 && + ); + + if (canSkipDatabase) { + queries.add(ResolvedSearchQueryExecutor.from(rawPids)); + } else { + // Finish the query in the database for the rest of the search parameters, sorting, partitioning, etc. + // We break the pids into chunks that fit in the 1k limit for jdbc bind params. + new QueryChunker() + .chunk(rawPids, t -> doCreateChunkedQueries(theParams, t, theOffset, sort, theCount, theRequest, queries)); } - - } - - ArrayList queries = new ArrayList<>(); - - if (!pids.isEmpty()) { - new QueryChunker().chunk(ResourcePersistentId.toLongList(pids), t -> doCreateChunkedQueries(theParams, t, theOffset, sort, theCount, theRequest, queries)); } else { + // do everything in the database. Optional query = createChunkedQuery(theParams, sort, theOffset, theMaximumResults, theCount, theRequest, null); - query.ifPresent(t -> queries.add(t)); + query.ifPresent(queries::add); } return queries; @@ -371,8 +392,10 @@ public class SearchBuilder implements ISearchBuilder { failIfUsed(Constants.PARAM_CONTENT); } - // TODO MB someday we'll want a query planner to figure out if we _should_ use the ft index, not just if we can. - return fulltextEnabled && myFulltextSearchSvc.supportsSomeOf(myParams); + // TODO MB someday we'll want a query planner to figure out if we _should_ or _must_ use the ft index, not just if we can. + return fulltextEnabled && + myParams.getSearchContainedMode() == SearchContainedModeEnum.FALSE && + myFulltextSearchSvc.supportsSomeOf(myParams); } private void failIfUsed(String theParamName) { @@ -401,19 +424,26 @@ public class SearchBuilder implements ISearchBuilder { } } - private List queryLuceneForPIDs(RequestDetails theRequest) { - validateFullTextSearchIsEnabled(); + private List queryHibernateSearchForEverythingPids() { + ResourcePersistentId pid = null; + if (myParams.get(IAnyResource.SP_RES_ID) != null) { + String idParamValue; + IQueryParameterType idParam = myParams.get(IAnyResource.SP_RES_ID).get(0).get(0); + if (idParam instanceof TokenParam) { + TokenParam idParm = (TokenParam) idParam; + idParamValue = idParm.getValue(); + } else { + StringParam idParm = (StringParam) idParam; + idParamValue = idParm.getValue(); + } - List pids; - if (myParams.getEverythingMode() != null) { - pids = myFulltextSearchSvc.everything(myResourceName, myParams, theRequest); - } else { - pids = myFulltextSearchSvc.search(myResourceName, myParams); + pid = myIdHelperService.resolveResourcePersistentIds(myRequestPartitionId, myResourceName, idParamValue); } + List pids = myFulltextSearchSvc.everything(myResourceName, myParams, pid); return pids; } - private void doCreateChunkedQueries(SearchParameterMap theParams, List thePids, Integer theOffset, SortSpec sort, boolean theCount, RequestDetails theRequest, ArrayList theQueries) { + private void doCreateChunkedQueries(SearchParameterMap theParams, List thePids, Integer theOffset, SortSpec sort, boolean theCount, RequestDetails theRequest, ArrayList theQueries) { if (thePids.size() < getMaximumPageSize()) { normalizeIdListForLastNInClause(thePids); } @@ -820,14 +850,19 @@ public class SearchBuilder implements ISearchBuilder { idList.add(resource.getId()); } + return getPidToTagMap(idList); + } + + @Nonnull + private Map> getPidToTagMap(List thePidList) { Map> tagMap = new HashMap<>(); //-- no tags - if (idList.size() == 0) + if (thePidList.size() == 0) return tagMap; //-- get all tags for the idList - Collection tagList = myResourceTagDao.findByResourceIds(idList); + Collection tagList = myResourceTagDao.findByResourceIds(thePidList); //-- build the map, key = resourceId, value = list of ResourceTag ResourcePersistentId resourceId; @@ -865,23 +900,55 @@ public class SearchBuilder implements ISearchBuilder { theResourceListToPopulate.add(null); } - List pids = new ArrayList<>(thePids); // Can we fast track this loading by checking elastic search? - if (isLoadingFromElasticSearchSupported(theIncludedPids.isEmpty())) { - theResourceListToPopulate.addAll(loadObservationResourcesFromElasticSearch(thePids)); + if (isLoadingFromElasticSearchSupported(theIncludedPids)) { + theResourceListToPopulate.addAll(loadResourcesFromElasticSearch(thePids)); } else { // We only chunk because some jdbc drivers can't handle long param lists. - new QueryChunker().chunk(pids, t -> doLoadPids(t, theIncludedPids, theResourceListToPopulate, theForHistoryOperation, position)); + new QueryChunker().chunk(thePids, t -> doLoadPids(t, theIncludedPids, theResourceListToPopulate, theForHistoryOperation, position)); } } - private boolean isLoadingFromElasticSearchSupported(boolean noIncludePids) { - return noIncludePids && !Objects.isNull(myParams) && myParams.isLastN() && myDaoConfig.isStoreResourceInLuceneIndex() - && myContext.getVersion().getVersion().isEqualOrNewerThan(FhirVersionEnum.DSTU3); + /** + * Check if we can load the resources from Hibernate Search instead of the database. + * We assume this is faster. + * + * Hibernate Search only stores the current version, and only if enabled. + * @param theIncludedPids the _include target to check for versioned ids + * @return can we fetch from Hibernate Search? + */ + private boolean isLoadingFromElasticSearchSupported(Collection theIncludedPids) { + // todo mb we can be smarter here. + // todo check if theIncludedPids has any with version not null. + + // is storage enabled? + return myDaoConfig.isStoreResourceInLuceneIndex() && + // only support lastN for now. + myParams.isLastN() && + // do we need to worry about versions? + theIncludedPids.isEmpty() && + // skip the complexity for metadata in dstu2 + myContext.getVersion().getVersion().isEqualOrNewerThan(FhirVersionEnum.DSTU3); } - private List loadObservationResourcesFromElasticSearch(Collection thePids) { - return myIElasticsearchSvc.getObservationResources(thePids); + private List loadResourcesFromElasticSearch(Collection thePids) { + // Do we use the fulltextsvc via hibernate-search to load resources or be backwards compatible with older ES only impl + // to handle lastN? + if (myDaoConfig.isAdvancedLuceneIndexing() && myDaoConfig.isStoreResourceInLuceneIndex()) { + List pidList = thePids.stream().map(ResourcePersistentId::getIdAsLong).collect(Collectors.toList()); + + // todo need to inject metadata - use profile to build resource, tags, and security labels + //Map> pidToTagMap = getPidToTagMap(pidList); + List resources = myFulltextSearchSvc.getResources(pidList); + return resources; + } else if (!Objects.isNull(myParams) && myParams.isLastN()) { + // legacy LastN implementation + return myIElasticsearchSvc.getObservationResources(thePids); + } else { + // TODO I wonder if we should drop this path, and only support the new Hibernate Search path. + Validate.isTrue(false, "Unexpected"); + return Collections.emptyList(); + } } /** @@ -1323,6 +1390,41 @@ public class SearchBuilder implements ISearchBuilder { myDaoConfig = theDaoConfig; } + /** + * Adapt simple Iterator to our internal query interface. + */ + static class ResolvedSearchQueryExecutor implements ISearchQueryExecutor { + private final Iterator myIterator; + + ResolvedSearchQueryExecutor(Iterable theIterable) { + this(theIterable.iterator()); + } + + ResolvedSearchQueryExecutor(Iterator theIterator) { + myIterator = theIterator; + } + + @Nonnull + public static ResolvedSearchQueryExecutor from(List rawPids) { + return new ResolvedSearchQueryExecutor(rawPids); + } + + @Override + public boolean hasNext() { + return myIterator.hasNext(); + } + + @Override + public Long next() { + return myIterator.next(); + } + + @Override + public void close() { + // empty + } + } + public class IncludesIterator extends BaseIterator implements Iterator { private final RequestDetails myRequest; @@ -1381,11 +1483,11 @@ public class SearchBuilder implements ISearchBuilder { private boolean myFirst = true; private IncludesIterator myIncludesIterator; private ResourcePersistentId myNext; - private SearchQueryExecutor myResultsIterator; + private ISearchQueryExecutor myResultsIterator; private boolean myStillNeedToFetchIncludes; private int mySkipCount = 0; private int myNonSkipCount = 0; - private ArrayList myQueryList = new ArrayList<>(); + private List myQueryList = new ArrayList<>(); private QueryIterator(SearchRuntimeDetails theSearchRuntimeDetails, RequestDetails theRequest) { mySearchRuntimeDetails = theSearchRuntimeDetails; @@ -1668,15 +1770,4 @@ public class SearchBuilder implements ISearchBuilder { return thePredicates.toArray(new Predicate[0]); } - private void validateFullTextSearchIsEnabled() { - if (myFulltextSearchSvc == null) { - if (myParams.containsKey(Constants.PARAM_TEXT)) { - throw new InvalidRequestException(Msg.code(1200) + "Fulltext search is not enabled on this service, can not process parameter: " + Constants.PARAM_TEXT); - } else if (myParams.containsKey(Constants.PARAM_CONTENT)) { - throw new InvalidRequestException(Msg.code(1201) + "Fulltext search is not enabled on this service, can not process parameter: " + Constants.PARAM_CONTENT); - } else { - throw new InvalidRequestException(Msg.code(1202) + "Fulltext search is not enabled on this service, can not process qualifier :text"); - } - } - } } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/sql/SearchQueryExecutor.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/sql/SearchQueryExecutor.java index 2cd75ece598..d94131321e0 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/sql/SearchQueryExecutor.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/sql/SearchQueryExecutor.java @@ -21,6 +21,7 @@ package ca.uhn.fhir.jpa.search.builder.sql; */ import ca.uhn.fhir.i18n.Msg; +import ca.uhn.fhir.jpa.search.builder.ISearchQueryExecutor; import ca.uhn.fhir.jpa.util.ScrollableResultsIterator; import ca.uhn.fhir.rest.server.exceptions.InternalErrorException; import ca.uhn.fhir.util.IoUtil; @@ -35,12 +36,10 @@ import javax.persistence.EntityManager; import javax.persistence.PersistenceContext; import javax.persistence.PersistenceContextType; import javax.persistence.Query; -import java.io.Closeable; import java.sql.Connection; import java.sql.PreparedStatement; -import java.util.Iterator; -public class SearchQueryExecutor implements Iterator, Closeable { +public class SearchQueryExecutor implements ISearchQueryExecutor { private static final Long NO_MORE = -1L; private static final SearchQueryExecutor NO_VALUE_EXECUTOR = new SearchQueryExecutor(); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/TestDaoSearch.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/TestDaoSearch.java index 8280b4a6f65..32601274276 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/TestDaoSearch.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/TestDaoSearch.java @@ -10,6 +10,7 @@ import ca.uhn.fhir.jpa.searchparam.SearchParameterMap; import ca.uhn.fhir.rest.api.SortSpec; import ca.uhn.fhir.rest.api.server.IBundleProvider; import ca.uhn.fhir.rest.server.method.SortParameter; +import org.hl7.fhir.instance.model.api.IBaseResource; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; @@ -45,6 +46,11 @@ public class TestDaoSearch { myFhirCtx = theFhirCtx; } + public List searchForResources(String theQueryUrl) { + IBundleProvider result = searchForBundleProvider(theQueryUrl); + return result.getAllResources(); + } + public List searchForIds(String theQueryUrl) { // fake out the server url parsing IBundleProvider result = searchForBundleProvider(theQueryUrl); @@ -55,19 +61,31 @@ public class TestDaoSearch { public IBundleProvider searchForBundleProvider(String theQueryUrl) { ResourceSearch search = myMatchUrlService.getResourceSearch(theQueryUrl); + IFhirResourceDao dao = myDaoRegistry.getResourceDao(search.getResourceName()); + SearchParameterMap map = search.getSearchParameterMap(); map.setLoadSynchronous(true); - SystemRequestDetails request = fakeRequestDetailsFromUrl(theQueryUrl); - SortSpec sort = (SortSpec) new SortParameter(myFhirCtx).translateQueryParametersIntoServerArgument(request, null); + SortSpec sort = (SortSpec) new SortParameter(myFhirCtx).translateQueryParametersIntoServerArgument(fakeRequestDetailsFromUrl(theQueryUrl), null); if (sort != null) { map.setSort(sort); } - IFhirResourceDao dao = myDaoRegistry.getResourceDao(search.getResourceName()); - IBundleProvider result = dao.search(map, request); + IBundleProvider result = dao.search(map, fakeRequestDetailsFromUrl(theQueryUrl)); return result; } + public SearchParameterMap toSearchParameters(String theQueryUrl) { + ResourceSearch search = myMatchUrlService.getResourceSearch(theQueryUrl); + + SearchParameterMap map = search.getSearchParameterMap(); + map.setLoadSynchronous(true); + SortSpec sort = (SortSpec) new SortParameter(myFhirCtx).translateQueryParametersIntoServerArgument(fakeRequestDetailsFromUrl(theQueryUrl), null); + if (sort != null) { + map.setSort(sort); + } + return map; + } + @Nonnull private SystemRequestDetails fakeRequestDetailsFromUrl(String theQueryUrl) { SystemRequestDetails request = new SystemRequestDetails(); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchLastNIT.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchLastNIT.java index 3ea92a85c7d..5886124af45 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchLastNIT.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchLastNIT.java @@ -24,7 +24,6 @@ import java.util.UUID; import java.util.stream.Collectors; import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.contains; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.matchesPattern; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -34,7 +33,6 @@ import static org.mockito.Mockito.when; @ExtendWith(SpringExtension.class) public class FhirResourceDaoR4SearchLastNIT extends BaseR4SearchLastN { - @AfterEach public void reset() { SearchBuilder.setMaxPageSize50ForTest(false); @@ -127,16 +125,22 @@ public class FhirResourceDaoR4SearchLastNIT extends BaseR4SearchLastN { when(mySrd.getParameters()).thenReturn(requestParameters); List results = toUnqualifiedVersionlessIdValues(myObservationDao.observationsLastN(params, mockSrd(), null)); + verifyResourcesLoadedFromElastic(observationIds, results); + + } + + void verifyResourcesLoadedFromElastic(List theObservationIds, List theResults) { List expectedArgumentPids = ResourcePersistentId.fromLongList( - observationIds.stream().map(IIdType::getIdPartAsLong).collect(Collectors.toList()) + theObservationIds.stream().map(IIdType::getIdPartAsLong).collect(Collectors.toList()) ); ArgumentCaptor> actualPids = ArgumentCaptor.forClass(List.class); verify(myElasticsearchSvc, times(1)).getObservationResources(actualPids.capture()); assertThat(actualPids.getValue(), is(expectedArgumentPids)); - List expectedObservationList = observationIds.stream() + List expectedObservationList = theObservationIds.stream() .map(id -> id.toUnqualifiedVersionless().getValue()).collect(Collectors.toList()); - assertEquals(results, expectedObservationList); + assertEquals(expectedObservationList, theResults); + } } diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchLastNUsingExtendedLuceneIndexIT.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchLastNUsingExtendedLuceneIndexIT.java index d3a241034eb..863c9ea6e1a 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchLastNUsingExtendedLuceneIndexIT.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchLastNUsingExtendedLuceneIndexIT.java @@ -1,17 +1,37 @@ package ca.uhn.fhir.jpa.dao.r4; import ca.uhn.fhir.jpa.api.config.DaoConfig; +import ca.uhn.fhir.jpa.dao.IFulltextSearchSvc; +import org.hl7.fhir.instance.model.api.IIdType; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.ArgumentCaptor; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.mock.mockito.SpyBean; import org.springframework.test.context.junit.jupiter.SpringExtension; +import java.util.List; +import java.util.stream.Collectors; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; + /** * Run entire @see {@link FhirResourceDaoR4SearchLastNIT} test suite this time - * using Extended Lucene index as search target + * using Extended Lucene index as search target. + * + * The other implementation is obsolete, and we can merge these someday. */ @ExtendWith(SpringExtension.class) public class FhirResourceDaoR4SearchLastNUsingExtendedLuceneIndexIT extends FhirResourceDaoR4SearchLastNIT { + // awkward override so we can spy + @SpyBean + @Autowired(required = false) + IFulltextSearchSvc myFulltestSearchSvc; @BeforeEach public void enableAdvancedLuceneIndexing() { @@ -23,4 +43,25 @@ public class FhirResourceDaoR4SearchLastNUsingExtendedLuceneIndexIT extends Fhir myDaoConfig.setAdvancedLuceneIndexing(new DaoConfig().isAdvancedLuceneIndexing()); } + /** + * We pull the resources from Hibernate Search when LastN uses Hibernate Search. + * Override the test verification + */ + @Override + void verifyResourcesLoadedFromElastic(List theObservationIds, List theResults) { + List expectedArgumentPids = + theObservationIds.stream().map(IIdType::getIdPartAsLong).collect(Collectors.toList()); + + ArgumentCaptor> actualPids = ArgumentCaptor.forClass(List.class); + + verify(myFulltestSearchSvc, times(1)).getResources(actualPids.capture()); + assertThat(actualPids.getValue(), is(expectedArgumentPids)); + + // we don't include the type in the id returned from Hibernate Search for now. + List expectedObservationList = theObservationIds.stream() + .map(id -> id.toUnqualifiedVersionless().getIdPart()).collect(Collectors.toList()); + assertEquals(expectedObservationList, theResults); + + } + } diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchWithElasticSearchIT.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchWithElasticSearchIT.java index 4ed6515a325..45cb714e7eb 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchWithElasticSearchIT.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchWithElasticSearchIT.java @@ -10,12 +10,12 @@ import ca.uhn.fhir.jpa.api.dao.IFhirSystemDao; import ca.uhn.fhir.jpa.api.model.DaoMethodOutcome; import ca.uhn.fhir.jpa.api.svc.ISearchCoordinatorSvc; import ca.uhn.fhir.jpa.bulk.export.api.IBulkDataExportJobSchedulingHelper; -import ca.uhn.fhir.jpa.bulk.export.api.IBulkDataExportSvc; import ca.uhn.fhir.jpa.config.TestHibernateSearchAddInConfig; import ca.uhn.fhir.jpa.config.TestR4Config; import ca.uhn.fhir.jpa.dao.BaseDateSearchDaoTests; import ca.uhn.fhir.jpa.dao.BaseJpaTest; import ca.uhn.fhir.jpa.dao.DaoTestDataBuilder; +import ca.uhn.fhir.jpa.dao.TestDaoSearch; import ca.uhn.fhir.jpa.dao.data.IResourceTableDao; import ca.uhn.fhir.jpa.entity.TermCodeSystemVersion; import ca.uhn.fhir.jpa.entity.TermConcept; @@ -37,10 +37,14 @@ import ca.uhn.fhir.rest.param.StringParam; import ca.uhn.fhir.rest.param.TokenParam; import ca.uhn.fhir.rest.param.TokenParamModifier; import ca.uhn.fhir.rest.server.util.ISearchParamRegistry; +import ca.uhn.fhir.test.utilities.ITestDataBuilder; import ca.uhn.fhir.test.utilities.docker.RequiresDocker; import ca.uhn.fhir.validation.FhirValidator; import ca.uhn.fhir.validation.ValidationResult; import org.hamcrest.Matchers; +import org.hl7.fhir.instance.model.api.IBaseCoding; +import org.hl7.fhir.instance.model.api.IBaseMetaType; +import org.hl7.fhir.instance.model.api.IBaseResource; import org.hl7.fhir.instance.model.api.IIdType; import org.hl7.fhir.r4.model.Bundle; import org.hl7.fhir.r4.model.CodeSystem; @@ -75,14 +79,22 @@ import java.util.List; import java.util.stream.Collectors; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.contains; import static org.hamcrest.Matchers.containsInAnyOrder; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.stringContainsInOrder; import static org.junit.jupiter.api.Assertions.assertEquals; @ExtendWith(SpringExtension.class) @RequiresDocker -@ContextConfiguration(classes = {TestR4Config.class, TestHibernateSearchAddInConfig.Elasticsearch.class}) +@ContextConfiguration(classes = { + TestR4Config.class, + TestHibernateSearchAddInConfig.Elasticsearch.class, + DaoTestDataBuilder.Config.class, + TestDaoSearch.Config.class +}) @DirtiesContext(classMode = DirtiesContext.ClassMode.AFTER_CLASS) public class FhirResourceDaoR4SearchWithElasticSearchIT extends BaseJpaTest { public static final String URL_MY_CODE_SYSTEM = "http://example.com/my_code_system"; @@ -132,6 +144,11 @@ public class FhirResourceDaoR4SearchWithElasticSearchIT extends BaseJpaTest { private ITermCodeSystemStorageSvc myTermCodeSystemStorageSvc; @Autowired private DaoRegistry myDaoRegistry; + @Autowired + ITestDataBuilder myTestDataBuilder; + @Autowired + TestDaoSearch myTestDaoSearch; + @BeforeEach public void beforePurgeDatabase() { @@ -159,6 +176,7 @@ public class FhirResourceDaoR4SearchWithElasticSearchIT extends BaseJpaTest { DaoConfig defaultConfig = new DaoConfig(); myDaoConfig.setAllowContainsSearches(defaultConfig.isAllowContainsSearches()); myDaoConfig.setAdvancedLuceneIndexing(defaultConfig.isAdvancedLuceneIndexing()); + myDaoConfig.setStoreResourceInLuceneIndex(defaultConfig.isStoreResourceInLuceneIndex()); } @Test @@ -422,6 +440,10 @@ public class FhirResourceDaoR4SearchWithElasticSearchIT extends BaseJpaTest { map.add("code", new TokenParam("Bodum").setModifier(TokenParamModifier.TEXT)); assertObservationSearchMatchesNothing("search with shared prefix does not match", map); } + + { + assertObservationSearchMatches("empty params finds everything", "Observation?", id1, id2, id3, id4); + } } @Test @@ -515,6 +537,11 @@ public class FhirResourceDaoR4SearchWithElasticSearchIT extends BaseJpaTest { assertThat(message, toUnqualifiedVersionlessIdValues(myObservationDao.search(map)), containsInAnyOrder(toValues(iIdTypes))); } + private void assertObservationSearchMatches(String theMessage, String theSearch, IIdType... theIds) { + SearchParameterMap map = myTestDaoSearch.toSearchParameters(theSearch); + assertObservationSearchMatches(theMessage, map, theIds); + } + @Nested public class WithContainedIndexingIT { @BeforeEach @@ -762,4 +789,121 @@ public class FhirResourceDaoR4SearchWithElasticSearchIT extends BaseJpaTest { } } + /** + * Some queries can be satisfied directly from Hibernate Search. + * We still need at least one query to fetch the resources. + */ + @Nested + public class FastPath { + @BeforeEach + public void enableResourceStorage() { + myDaoConfig.setStoreResourceInLuceneIndex(false); + } + + @AfterEach + public void resetResourceStorage() { + myDaoConfig.setStoreResourceInLuceneIndex(new DaoConfig().isStoreResourceInLuceneIndex()); + } + + + @Test + public void simpleTokenSkipsSql() { + + IIdType id = myTestDataBuilder.createObservation(myTestDataBuilder.withObservationCode("http://example.com/", "theCode")); + myCaptureQueriesListener.clear(); + + List ids = myTestDaoSearch.searchForIds("Observation?code=theCode"); + myCaptureQueriesListener.logSelectQueriesForCurrentThread(); + + assertThat(ids, hasSize(1)); + assertThat(ids, contains(id.getIdPart())); + assertEquals(1, myCaptureQueriesListener.getSelectQueriesForCurrentThread().size(), "sql just to fetch resources"); + } + + @Test + public void sortStillRequiresSql() { + + IIdType id = myTestDataBuilder.createObservation(myTestDataBuilder.withObservationCode("http://example.com/", "theCode")); + myCaptureQueriesListener.clear(); + + List ids = myTestDaoSearch.searchForIds("Observation?code=theCode&_sort=code"); + myCaptureQueriesListener.logSelectQueriesForCurrentThread(); + + assertThat(ids, hasSize(1)); + assertThat(ids, contains(id.getIdPart())); + + assertEquals(2, myCaptureQueriesListener.getSelectQueriesForCurrentThread().size(), "the pids come from elastic, but we use sql to sort, and fetch resources"); + } + + @Test + public void deletedResourceNotFound() { + + IIdType id = myTestDataBuilder.createObservation(myTestDataBuilder.withObservationCode("http://example.com/", "theCode")); + myObservationDao.delete(id); + myCaptureQueriesListener.clear(); + + List ids = myTestDaoSearch.searchForIds("Observation?code=theCode&_sort=code"); + myCaptureQueriesListener.logSelectQueriesForCurrentThread(); + + assertThat(ids, hasSize(0)); + + assertEquals(0, myCaptureQueriesListener.getSelectQueriesForCurrentThread().size(), "the pids come from elastic, and nothing to fetch"); + } + + @Test + public void forcedIdSurvivesWithNoSql() { + IIdType id = myTestDataBuilder.createObservation( + myTestDataBuilder.withObservationCode("http://example.com/", "theCode"), + myTestDataBuilder.withId("forcedid")); + assertThat(id.getIdPart(), equalTo("forcedid")); + myCaptureQueriesListener.clear(); + + List ids = myTestDaoSearch.searchForIds("Observation?code=theCode"); + myCaptureQueriesListener.logSelectQueriesForCurrentThread(); + + assertThat(ids, hasSize(1)); + assertThat(ids, contains(id.getIdPart())); + + assertEquals(1, myCaptureQueriesListener.getSelectQueriesForCurrentThread().size(), "just 1 to fetch the resources"); + } + + /** + * A paranoid test to make sure tags stay with the resource. + * + * Tags live outside the resource, and can be modified by + * Since we lost the id, also check tags in case someone changes metadata processing during ingestion. + */ + @Test + public void tagsSurvive() { + IIdType id = myTestDataBuilder.createObservation( + myTestDataBuilder.withObservationCode("http://example.com/", "theCode"), + myTestDataBuilder.withTag("http://example.com", "aTag")); + + myCaptureQueriesListener.clear(); + List observations = myTestDaoSearch.searchForResources("Observation?code=theCode"); + + assertThat(observations, hasSize(1)); + List tags = observations.get(0).getMeta().getTag(); + assertThat(tags, hasSize(1)); + assertThat(tags.get(0).getSystem(), equalTo("http://example.com")); + assertThat(tags.get(0).getCode(), equalTo("aTag")); + + Meta meta = new Meta(); + meta.addTag().setSystem("tag_scheme1").setCode("tag_code1"); + meta.addProfile("http://profile/1"); + meta.addSecurity().setSystem("seclabel_sys1").setCode("seclabel_code1"); + myObservationDao.metaAddOperation(id, meta, mySrd); + + observations = myTestDaoSearch.searchForResources("Observation?code=theCode"); + + assertThat(observations, hasSize(1)); + IBaseMetaType newMeta = observations.get(0).getMeta(); + assertThat(newMeta.getProfile(), hasSize(1)); + assertThat(newMeta.getSecurity(), hasSize(1)); + assertThat(newMeta.getTag(), hasSize(2)); + } + + + } + } diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4StandardQueriesNoFTTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4StandardQueriesNoFTTest.java index e6cbbf408ac..5f19303a8d5 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4StandardQueriesNoFTTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4StandardQueriesNoFTTest.java @@ -190,7 +190,7 @@ public class FhirResourceDaoR4StandardQueriesNoFTTest extends BaseJpaTest { withRiskAssessmentWithProbabilty(0.6); assertNotFind("when gt", "/RiskAssessment?probability=0.5"); - // fixme we break the spec here. + // TODO we break the spec here. Default search should be approx // assertFind("when a little gt - default is approx", "/RiskAssessment?probability=0.599"); // assertFind("when a little lt - default is approx", "/RiskAssessment?probability=0.601"); assertFind("when eq", "/RiskAssessment?probability=0.6"); @@ -301,10 +301,10 @@ public class FhirResourceDaoR4StandardQueriesNoFTTest extends BaseJpaTest { assertNotFind("when gt", "/Observation?value-quantity=0.5||mmHg"); assertNotFind("when gt unitless", "/Observation?value-quantity=0.5"); - // fixme we break the spec here. + // TODO we break the spec here. Default search should be approx // assertFind("when a little gt - default is approx", "/Observation?value-quantity=0.599"); // assertFind("when a little lt - default is approx", "/Observation?value-quantity=0.601"); - // fixme we don't seem to support "units", only "code". + // TODO we don't seem to support "units", only "code". assertFind("when eq with units", "/Observation?value-quantity=0.6||mm[Hg]"); assertFind("when eq unitless", "/Observation?value-quantity=0.6"); assertNotFind("when lt", "/Observation?value-quantity=0.7||mmHg"); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/search/ExtendedLuceneResourceProjectionTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/search/ExtendedLuceneResourceProjectionTest.java new file mode 100644 index 00000000000..6423ad04b62 --- /dev/null +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/search/ExtendedLuceneResourceProjectionTest.java @@ -0,0 +1,40 @@ +package ca.uhn.fhir.jpa.dao.search; + +import ca.uhn.fhir.context.FhirContext; +import ca.uhn.fhir.parser.IParser; +import org.hl7.fhir.instance.model.api.IBaseResource; +import org.hl7.fhir.r4.model.Observation; +import org.junit.jupiter.api.Test; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.instanceOf; + +class ExtendedLuceneResourceProjectionTest { + final FhirContext myFhirContext = FhirContext.forR4(); + final IParser myParser = myFhirContext.newJsonParser(); + ExtendedLuceneResourceProjection myProjection; + IBaseResource myResource; + + @Test + public void basicBodyReceivesId() { + myProjection = new ExtendedLuceneResourceProjection(22, null, "{ \"resourceType\":\"Observation\"}"); + + myResource = myProjection.toResource(myParser); + + assertThat(myResource, instanceOf(Observation.class)); + assertThat(myResource.getIdElement().getIdPart(), equalTo("22")); + } + + @Test + public void forcedIdOverridesPid() { + myProjection = new ExtendedLuceneResourceProjection(22, "force-id", "{ \"resourceType\":\"Observation\"}"); + + myResource = myProjection.toResource(myParser); + + assertThat(myResource, instanceOf(Observation.class)); + assertThat(myResource.getIdElement().getIdPart(), equalTo("force-id")); + } + + +} diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/search/autocomplete/TokenAutocompleteElasticsearchIT.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/search/autocomplete/TokenAutocompleteElasticsearchIT.java index c7da5f80bae..a6010a65b08 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/search/autocomplete/TokenAutocompleteElasticsearchIT.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/search/autocomplete/TokenAutocompleteElasticsearchIT.java @@ -20,6 +20,7 @@ import org.hamcrest.Description; import org.hamcrest.Matcher; import org.hamcrest.TypeSafeDiagnosingMatcher; import org.hibernate.search.mapper.orm.Search; +import org.hl7.fhir.instance.model.api.IBaseCoding; import org.hl7.fhir.instance.model.api.IIdType; import org.hl7.fhir.r4.model.Coding; import org.hl7.fhir.r4.model.Observation; @@ -193,7 +194,7 @@ public class TokenAutocompleteElasticsearchIT extends BaseJpaTest{ } @Nonnull - private Matcher matchingSystemAndCode(Coding theCoding) { + private Matcher matchingSystemAndCode(IBaseCoding theCoding) { return new TypeSafeDiagnosingMatcher() { private final String mySystemAndCode = theCoding.getSystem() + "|" + theCoding.getCode(); diff --git a/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ResourceTable.java b/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ResourceTable.java index b877194bae0..80fff289346 100644 --- a/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ResourceTable.java +++ b/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ResourceTable.java @@ -140,6 +140,13 @@ public class ResourceTable extends BaseHasResource implements Serializable, IBas @PropertyBinding(binder = @PropertyBinderRef(type = SearchParamTextPropertyBinder.class)) private ExtendedLuceneIndexData myLuceneIndexData; + // todo mb move this to ExtendedLuceneIndexData + @Transient + @GenericField(name="myRawResource", projectable = Projectable.YES, searchable = Searchable.NO) + @IndexingDependency(derivedFrom = @ObjectPath(@PropertyValue(propertyName = "myVersion"))) + @OptimisticLock(excluded = true) + private String myRawResourceData; + @OneToMany(mappedBy = "myResource", cascade = {}, fetch = FetchType.LAZY, orphanRemoval = false) @OptimisticLock(excluded = true) private Collection myParamsCoords; @@ -775,4 +782,8 @@ public class ResourceTable extends BaseHasResource implements Serializable, IBas public void setLuceneIndexData(ExtendedLuceneIndexData theLuceneIndexData) { myLuceneIndexData = theLuceneIndexData; } + + public void setRawResourceData(String theResourceData) { + myRawResourceData = theResourceData; + } } diff --git a/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/search/ExtendedLuceneIndexData.java b/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/search/ExtendedLuceneIndexData.java index c05e6867170..c723df6512e 100644 --- a/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/search/ExtendedLuceneIndexData.java +++ b/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/search/ExtendedLuceneIndexData.java @@ -45,6 +45,7 @@ public class ExtendedLuceneIndexData { final SetMultimap mySearchParamTokens = HashMultimap.create(); final SetMultimap mySearchParamLinks = HashMultimap.create(); final SetMultimap mySearchParamDates = HashMultimap.create(); + String myForcedId; public ExtendedLuceneIndexData(FhirContext theFhirContext) { this.myFhirContext = theFhirContext; @@ -58,11 +59,20 @@ public class ExtendedLuceneIndexData { } }; } + + /** + * Write the index document. + * + * Keep this in sync with the schema defined in {@link SearchParamTextPropertyBinder} + * @param theDocument + */ public void writeIndexElements(DocumentElement theDocument) { HibernateSearchIndexWriter indexWriter = HibernateSearchIndexWriter.forRoot(myFhirContext, theDocument); ourLog.debug("Writing JPA index to Hibernate Search"); + theDocument.addValue("myForcedId", myForcedId); + mySearchParamStrings.forEach(ifNotContained(indexWriter::writeStringIndex)); mySearchParamTokens.forEach(ifNotContained(indexWriter::writeTokenIndex)); mySearchParamLinks.forEach(ifNotContained(indexWriter::writeReferenceIndex)); @@ -97,4 +107,11 @@ public class ExtendedLuceneIndexData { mySearchParamDates.put(theSpName, new DateSearchIndexData(theLowerBound, theLowerBoundOrdinal, theUpperBound, theUpperBoundOrdinal)); } + public void setForcedId(String theForcedId) { + myForcedId = theForcedId; + } + + public String getForcedId() { + return myForcedId; + } } diff --git a/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/search/SearchParamTextPropertyBinder.java b/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/search/SearchParamTextPropertyBinder.java index 91b0e9b1233..903db94a487 100644 --- a/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/search/SearchParamTextPropertyBinder.java +++ b/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/search/SearchParamTextPropertyBinder.java @@ -100,11 +100,18 @@ public class SearchParamTextPropertyBinder implements PropertyBinder, PropertyBr .projectable(Projectable.NO) .sortable(Sortable.YES); + StringIndexFieldTypeOptionsStep forcedIdType = indexFieldTypeFactory.asString() + .projectable(Projectable.YES) + .aggregable(Aggregable.NO); + // the old style for _text and _contains indexSchemaElement .fieldTemplate("SearchParamText", standardAnalyzer) .matchingPathGlob(SEARCH_PARAM_TEXT_PREFIX + "*"); + + indexSchemaElement.field("myForcedId", forcedIdType).toReference(); + // The following section is a bit ugly. We need to enforce order and dependency or the object matches will be too big. { IndexSchemaObjectField spfield = indexSchemaElement.objectField(HibernateSearchIndexWriter.SEARCH_PARAM_ROOT, ObjectStructure.FLATTENED);