From 4dbd227d5dbfca1f5823d52c68b01f030c1b99b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yoann=20Rodi=C3=A8re?= Date: Mon, 29 Oct 2018 17:56:01 +0100 Subject: [PATCH] HSEARCH-3290 Introduce toSort() and toPredicate() methods in the DSLs They can be used wherever end() can be used, but they always return a SearchSort/SearchPredicate, unlike end() that returns a context-specific type. This adds in particular the ability to cache predicates at any level of the DSL tree, not just the root like we used to. --- ...csearchSearchSortContainerContextImpl.java | 13 +-- .../LuceneSearchSortContainerContextImpl.java | 13 +-- .../SearchPredicateTerminalContext.java | 10 ++ .../BooleanJunctionPredicateContextImpl.java | 8 +- .../impl/MatchAllPredicateContextImpl.java | 7 +- .../impl/MultiFieldPredicateCommonState.java | 8 +- .../impl/NestedPredicateContextImpl.java | 7 +- .../RangePredicateFieldSetContextImpl.java | 4 +- .../RootSearchPredicateDslContextImpl.java | 53 ++------- ...ectCreatingSearchPredicateContributor.java | 81 ++++++++++++++ .../dsl/sort/SearchSortTerminalContext.java | 10 ++ .../sort/impl/DistanceSortContextImpl.java | 24 ++-- .../dsl/sort/impl/FieldSortContextImpl.java | 21 +--- .../sort/impl/NonEmptySortContextImpl.java | 31 ------ .../impl/RootSearchSortDslContextImpl.java | 7 +- .../dsl/sort/impl/ScoreSortContextImpl.java | 24 ++-- .../impl/SearchSortContainerContextImpl.java | 7 +- ...archSortContainerExtensionContextImpl.java | 1 + .../dsl/sort/spi/NonEmptySortContextImpl.java | 36 ++++++ .../dsl/sort/spi/SearchSortDslContext.java | 10 ++ .../search/predicate/SearchPredicateIT.java | 105 ++++++++++++------ .../backend/tck/search/sort/SearchSortIT.java | 4 +- 22 files changed, 284 insertions(+), 200 deletions(-) create mode 100644 engine/src/main/java/org/hibernate/search/engine/search/dsl/predicate/spi/AbstractObjectCreatingSearchPredicateContributor.java delete mode 100644 engine/src/main/java/org/hibernate/search/engine/search/dsl/sort/impl/NonEmptySortContextImpl.java create mode 100644 engine/src/main/java/org/hibernate/search/engine/search/dsl/sort/spi/NonEmptySortContextImpl.java diff --git a/backend/elasticsearch/src/main/java/org/hibernate/search/backend/elasticsearch/search/dsl/sort/impl/ElasticsearchSearchSortContainerContextImpl.java b/backend/elasticsearch/src/main/java/org/hibernate/search/backend/elasticsearch/search/dsl/sort/impl/ElasticsearchSearchSortContainerContextImpl.java index 397c4fe31ad..4087a01a88a 100644 --- a/backend/elasticsearch/src/main/java/org/hibernate/search/backend/elasticsearch/search/dsl/sort/impl/ElasticsearchSearchSortContainerContextImpl.java +++ b/backend/elasticsearch/src/main/java/org/hibernate/search/backend/elasticsearch/search/dsl/sort/impl/ElasticsearchSearchSortContainerContextImpl.java @@ -12,6 +12,7 @@ import org.hibernate.search.engine.search.dsl.sort.NonEmptySortContext; import org.hibernate.search.engine.search.dsl.sort.SearchSortContainerContext; import org.hibernate.search.engine.search.dsl.sort.spi.DelegatingSearchSortContainerContextImpl; +import org.hibernate.search.engine.search.dsl.sort.spi.NonEmptySortContextImpl; import org.hibernate.search.engine.search.dsl.sort.spi.SearchSortDslContext; @@ -38,16 +39,6 @@ public NonEmptySortContext fromJsonString(String jsonString) { } private NonEmptySortContext nonEmptyContext() { - return new NonEmptySortContext() { - @Override - public SearchSortContainerContext then() { - return ElasticsearchSearchSortContainerContextImpl.this; - } - - @Override - public N end() { - return dslContext.getNextContext(); - } - }; + return new NonEmptySortContextImpl<>( this, dslContext ); } } diff --git a/backend/lucene/src/main/java/org/hibernate/search/backend/lucene/search/dsl/sort/impl/LuceneSearchSortContainerContextImpl.java b/backend/lucene/src/main/java/org/hibernate/search/backend/lucene/search/dsl/sort/impl/LuceneSearchSortContainerContextImpl.java index 71de7291a77..3f8de3fe9d4 100644 --- a/backend/lucene/src/main/java/org/hibernate/search/backend/lucene/search/dsl/sort/impl/LuceneSearchSortContainerContextImpl.java +++ b/backend/lucene/src/main/java/org/hibernate/search/backend/lucene/search/dsl/sort/impl/LuceneSearchSortContainerContextImpl.java @@ -14,6 +14,7 @@ import org.hibernate.search.engine.search.dsl.sort.NonEmptySortContext; import org.hibernate.search.engine.search.dsl.sort.SearchSortContainerContext; import org.hibernate.search.engine.search.dsl.sort.spi.DelegatingSearchSortContainerContextImpl; +import org.hibernate.search.engine.search.dsl.sort.spi.NonEmptySortContextImpl; import org.hibernate.search.engine.search.dsl.sort.spi.SearchSortDslContext; @@ -46,16 +47,6 @@ public NonEmptySortContext fromLuceneSort(Sort luceneSort) { } private NonEmptySortContext nonEmptyContext() { - return new NonEmptySortContext() { - @Override - public SearchSortContainerContext then() { - return LuceneSearchSortContainerContextImpl.this; - } - - @Override - public N end() { - return dslContext.getNextContext(); - } - }; + return new NonEmptySortContextImpl<>( this, dslContext ); } } diff --git a/engine/src/main/java/org/hibernate/search/engine/search/dsl/predicate/SearchPredicateTerminalContext.java b/engine/src/main/java/org/hibernate/search/engine/search/dsl/predicate/SearchPredicateTerminalContext.java index ee88d29b79d..ad8cd97837c 100644 --- a/engine/src/main/java/org/hibernate/search/engine/search/dsl/predicate/SearchPredicateTerminalContext.java +++ b/engine/src/main/java/org/hibernate/search/engine/search/dsl/predicate/SearchPredicateTerminalContext.java @@ -7,6 +7,8 @@ package org.hibernate.search.engine.search.dsl.predicate; +import org.hibernate.search.engine.search.SearchPredicate; + /** * The terminal context of the predicate DSL. * @@ -21,4 +23,12 @@ public interface SearchPredicateTerminalContext { */ N end(); + /** + * Create a {@link SearchPredicate} instance + * matching the definition given in the previous DSL steps. + * + * @return The {@link SearchPredicate} resulting from the previous DSL steps. + */ + SearchPredicate toPredicate(); + } diff --git a/engine/src/main/java/org/hibernate/search/engine/search/dsl/predicate/impl/BooleanJunctionPredicateContextImpl.java b/engine/src/main/java/org/hibernate/search/engine/search/dsl/predicate/impl/BooleanJunctionPredicateContextImpl.java index f3f5a55dc85..07fe9f90413 100644 --- a/engine/src/main/java/org/hibernate/search/engine/search/dsl/predicate/impl/BooleanJunctionPredicateContextImpl.java +++ b/engine/src/main/java/org/hibernate/search/engine/search/dsl/predicate/impl/BooleanJunctionPredicateContextImpl.java @@ -15,6 +15,7 @@ import org.hibernate.search.engine.search.dsl.predicate.BooleanJunctionPredicateContext; import org.hibernate.search.engine.search.dsl.predicate.MinimumShouldMatchContext; import org.hibernate.search.engine.search.dsl.predicate.SearchPredicateContainerContext; +import org.hibernate.search.engine.search.dsl.predicate.spi.AbstractObjectCreatingSearchPredicateContributor; import org.hibernate.search.engine.search.dsl.predicate.spi.SearchPredicateContributor; import org.hibernate.search.engine.search.dsl.predicate.spi.SearchPredicateDslContext; import org.hibernate.search.engine.search.predicate.spi.BooleanJunctionPredicateBuilder; @@ -22,10 +23,9 @@ class BooleanJunctionPredicateContextImpl + extends AbstractObjectCreatingSearchPredicateContributor implements BooleanJunctionPredicateContext, SearchPredicateContributor { - private final SearchPredicateFactory factory; - private final Supplier nextContextProvider; private final BooleanJunctionPredicateBuilder builder; @@ -39,7 +39,7 @@ class BooleanJunctionPredicateContextImpl BooleanJunctionPredicateContextImpl(SearchPredicateFactory factory, Supplier nextContextProvider) { - this.factory = factory; + super( factory ); this.nextContextProvider = nextContextProvider; this.builder = factory.bool(); this.must = new OccurContext(); @@ -112,7 +112,7 @@ public BooleanJunctionPredicateContext minimumShouldMatch( } @Override - public B contribute() { + protected B doContribute() { must.contribute( builder::must ); mustNot.contribute( builder::mustNot ); should.contribute( builder::should ); diff --git a/engine/src/main/java/org/hibernate/search/engine/search/dsl/predicate/impl/MatchAllPredicateContextImpl.java b/engine/src/main/java/org/hibernate/search/engine/search/dsl/predicate/impl/MatchAllPredicateContextImpl.java index e2ef5bd4dce..6dc610b0785 100644 --- a/engine/src/main/java/org/hibernate/search/engine/search/dsl/predicate/impl/MatchAllPredicateContextImpl.java +++ b/engine/src/main/java/org/hibernate/search/engine/search/dsl/predicate/impl/MatchAllPredicateContextImpl.java @@ -14,6 +14,7 @@ import org.hibernate.search.engine.search.SearchPredicate; import org.hibernate.search.engine.search.dsl.predicate.MatchAllPredicateContext; import org.hibernate.search.engine.search.dsl.predicate.SearchPredicateContainerContext; +import org.hibernate.search.engine.search.dsl.predicate.spi.AbstractObjectCreatingSearchPredicateContributor; import org.hibernate.search.engine.search.dsl.predicate.spi.SearchPredicateContributor; import org.hibernate.search.engine.search.dsl.predicate.spi.SearchPredicateDslContext; import org.hibernate.search.engine.search.predicate.spi.BooleanJunctionPredicateBuilder; @@ -22,9 +23,9 @@ class MatchAllPredicateContextImpl + extends AbstractObjectCreatingSearchPredicateContributor implements MatchAllPredicateContext, SearchPredicateContributor { - private final SearchPredicateFactory factory; private final Supplier nextContextProvider; private final MatchAllPredicateBuilder matchAllBuilder; @@ -32,7 +33,7 @@ class MatchAllPredicateContextImpl MatchAllPredicateContextImpl(SearchPredicateFactory factory, Supplier nextContextProvider) { - this.factory = factory; + super( factory ); this.nextContextProvider = nextContextProvider; this.matchAllBuilder = factory.matchAll(); } @@ -56,7 +57,7 @@ public MatchAllPredicateContext except(Consumer> + extends AbstractObjectCreatingSearchPredicateContributor implements SearchPredicateContributor { - private final SearchPredicateFactory factory; - private final Supplier nextContextProvider; private final List fieldSetContexts = new ArrayList<>(); MultiFieldPredicateCommonState(SearchPredicateFactory factory, Supplier nextContextProvider) { - this.factory = factory; + super( factory ); this.nextContextProvider = nextContextProvider; } @@ -46,7 +46,7 @@ List getFieldSetContexts() { } @Override - public B contribute() { + protected B doContribute() { List predicateBuilders = new ArrayList<>(); for ( F fieldSetContext : fieldSetContexts ) { fieldSetContext.contributePredicateBuilders( predicateBuilders::add ); diff --git a/engine/src/main/java/org/hibernate/search/engine/search/dsl/predicate/impl/NestedPredicateContextImpl.java b/engine/src/main/java/org/hibernate/search/engine/search/dsl/predicate/impl/NestedPredicateContextImpl.java index 281ebb60469..64580e26893 100644 --- a/engine/src/main/java/org/hibernate/search/engine/search/dsl/predicate/impl/NestedPredicateContextImpl.java +++ b/engine/src/main/java/org/hibernate/search/engine/search/dsl/predicate/impl/NestedPredicateContextImpl.java @@ -16,6 +16,7 @@ import org.hibernate.search.engine.search.dsl.predicate.NestedPredicateContext; import org.hibernate.search.engine.search.dsl.predicate.NestedPredicateFieldContext; import org.hibernate.search.engine.search.dsl.predicate.SearchPredicateContainerContext; +import org.hibernate.search.engine.search.dsl.predicate.spi.AbstractObjectCreatingSearchPredicateContributor; import org.hibernate.search.engine.search.dsl.predicate.spi.SearchPredicateContributor; import org.hibernate.search.engine.search.dsl.predicate.spi.SearchPredicateDslContext; import org.hibernate.search.engine.search.predicate.spi.NestedPredicateBuilder; @@ -24,12 +25,12 @@ class NestedPredicateContextImpl + extends AbstractObjectCreatingSearchPredicateContributor implements NestedPredicateContext, NestedPredicateFieldContext, SearchPredicateTerminalContext, SearchPredicateDslContext, SearchPredicateContributor { private static final Log log = LoggerFactory.make( Log.class, MethodHandles.lookup() ); - private final SearchPredicateFactory factory; private final Supplier nextContextProvider; private final SearchPredicateContainerContext containerContext; @@ -37,7 +38,7 @@ class NestedPredicateContextImpl private SearchPredicateContributor childPredicateContributor; NestedPredicateContextImpl(SearchPredicateFactory factory, Supplier nextContextProvider) { - this.factory = factory; + super( factory ); this.nextContextProvider = nextContextProvider; this.containerContext = new SearchPredicateContainerContextImpl<>( factory, this ); } @@ -79,7 +80,7 @@ public N getNextContext() { } @Override - public B contribute() { + protected B doContribute() { builder.nested( childPredicateContributor.contribute() ); return builder.toImplementation(); } diff --git a/engine/src/main/java/org/hibernate/search/engine/search/dsl/predicate/impl/RangePredicateFieldSetContextImpl.java b/engine/src/main/java/org/hibernate/search/engine/search/dsl/predicate/impl/RangePredicateFieldSetContextImpl.java index 7c0caac0f45..11dd540455d 100644 --- a/engine/src/main/java/org/hibernate/search/engine/search/dsl/predicate/impl/RangePredicateFieldSetContextImpl.java +++ b/engine/src/main/java/org/hibernate/search/engine/search/dsl/predicate/impl/RangePredicateFieldSetContextImpl.java @@ -88,10 +88,10 @@ static class CommonState extends MultiFieldPredicateCommonState from(Object value, RangeBoundInclusion inclusion) { diff --git a/engine/src/main/java/org/hibernate/search/engine/search/dsl/predicate/impl/RootSearchPredicateDslContextImpl.java b/engine/src/main/java/org/hibernate/search/engine/search/dsl/predicate/impl/RootSearchPredicateDslContextImpl.java index 4777bccf97a..fb14e2063b9 100644 --- a/engine/src/main/java/org/hibernate/search/engine/search/dsl/predicate/impl/RootSearchPredicateDslContextImpl.java +++ b/engine/src/main/java/org/hibernate/search/engine/search/dsl/predicate/impl/RootSearchPredicateDslContextImpl.java @@ -12,11 +12,11 @@ import org.hibernate.search.engine.backend.index.spi.IndexSearchTarget; import org.hibernate.search.engine.logging.impl.Log; import org.hibernate.search.engine.search.SearchPredicate; +import org.hibernate.search.engine.search.dsl.predicate.spi.AbstractObjectCreatingSearchPredicateContributor; import org.hibernate.search.engine.search.dsl.predicate.spi.SearchPredicateContributor; import org.hibernate.search.engine.search.dsl.predicate.spi.SearchPredicateDslContext; import org.hibernate.search.engine.search.dsl.query.SearchQueryResultContext; import org.hibernate.search.engine.search.predicate.spi.SearchPredicateFactory; -import org.hibernate.search.util.AssertionFailure; import org.hibernate.search.util.impl.common.LoggerFactory; /** @@ -26,23 +26,20 @@ * (in which case the lambda may retrieve the resulting {@link SearchPredicate} object and cache it). */ public final class RootSearchPredicateDslContextImpl + extends AbstractObjectCreatingSearchPredicateContributor implements SearchPredicateDslContext { private static final Log log = LoggerFactory.make( Log.class, MethodHandles.lookup() ); - private final SearchPredicateFactory factory; - private SearchPredicateContributor singlePredicateContributor; - private boolean usedContributor = false; - private SearchPredicate predicateResult; public RootSearchPredicateDslContextImpl(SearchPredicateFactory factory) { - this.factory = factory; + super( factory ); } @Override public void addChild(SearchPredicateContributor child) { - if ( usedContributor ) { + if ( isContributed() ) { throw log.cannotAddPredicateToUsedContext(); } if ( this.singlePredicateContributor != null ) { @@ -53,43 +50,15 @@ public void addChild(SearchPredicateContributor child) { @Override public SearchPredicate getNextContext() { - if ( predicateResult == null ) { - if ( usedContributor ) { - // HSEARCH-3207: we must never call a contribution twice. Contributions may have side-effects. - throw new AssertionFailure( - "A predicate object was requested after the corresponding information was contributed to the DSL." + - " There is a bug in Hibernate Search, please report it." - ); - } - predicateResult = factory.toSearchPredicate( getResultingBuilder() ); - } - return predicateResult; + return toPredicate(); } public B getResultingBuilder() { - if ( predicateResult != null ) { - /* - * If the SearchPredicate object was already created, - * we can't use the builder collected by the aggregator anymore: it might be single-use. - * We just ask the factory to convert the SearchPredicate object back to a builder. - * If the builders can be used multiple times, the factory can optimize this. - */ - return factory.toImplementation( predicateResult ); - } - else { - if ( usedContributor ) { - // HSEARCH-3207: we must never call a contribution twice. Contributions may have side-effects. - throw new AssertionFailure( - "A predicate contributor was called twice. There is a bug in Hibernate Search, please report it." - ); - } - usedContributor = true; - /* - * Optimization: we know the user will not be able to request a SearchPredicate object anymore, - * so we don't need to build a SearchPredicate object in this case, - * we can just use the builder collected by the aggregator directly. - */ - return singlePredicateContributor.contribute(); - } + return contribute(); + } + + @Override + protected B doContribute() { + return singlePredicateContributor.contribute(); } } diff --git a/engine/src/main/java/org/hibernate/search/engine/search/dsl/predicate/spi/AbstractObjectCreatingSearchPredicateContributor.java b/engine/src/main/java/org/hibernate/search/engine/search/dsl/predicate/spi/AbstractObjectCreatingSearchPredicateContributor.java new file mode 100644 index 00000000000..29fa8548edb --- /dev/null +++ b/engine/src/main/java/org/hibernate/search/engine/search/dsl/predicate/spi/AbstractObjectCreatingSearchPredicateContributor.java @@ -0,0 +1,81 @@ +/* + * Hibernate Search, full-text search for your domain model + * + * License: GNU Lesser General Public License (LGPL), version 2.1 or later + * See the lgpl.txt file in the root directory or . + */ +package org.hibernate.search.engine.search.dsl.predicate.spi; + +import org.hibernate.search.engine.search.SearchPredicate; +import org.hibernate.search.engine.search.predicate.spi.SearchPredicateFactory; +import org.hibernate.search.util.AssertionFailure; + +/** + * An abstract base for {@link SearchPredicateContributor} implementation that can also be requested + * to create a {@link SearchPredicate} object, independently from being asked to {@link #contribute()} a builder. + *

+ * This is especially useful when implementing DSL contexts that implement + * {@link org.hibernate.search.engine.search.dsl.predicate.SearchPredicateTerminalContext}. + * + * @param The implementation type of builders, see {@link SearchPredicateContributor}. + */ +public abstract class AbstractObjectCreatingSearchPredicateContributor implements SearchPredicateContributor { + + protected final SearchPredicateFactory factory; + + private boolean contributed = false; + private SearchPredicate predicateResult; + + public AbstractObjectCreatingSearchPredicateContributor(SearchPredicateFactory factory) { + this.factory = factory; + } + + @Override + public final B contribute() { + if ( predicateResult != null ) { + /* + * If the SearchPredicate object was already created, + * we can't use the builder collected by the aggregator anymore: it might be single-use. + * We just ask the factory to convert the SearchPredicate object back to a builder. + * If the builders can be used multiple times, the factory can optimize this. + */ + return factory.toImplementation( predicateResult ); + } + else { + if ( contributed ) { + // HSEARCH-3207: we must never call a contribution twice. Contributions may have side-effects. + throw new AssertionFailure( + "A predicate contributor was called twice. There is a bug in Hibernate Search, please report it." + ); + } + contributed = true; + /* + * Optimization: we know the user will not be able to request a SearchPredicate object anymore, + * so we don't need to build a SearchPredicate object in this case, + * we can just use the builder collected by the aggregator directly. + */ + return doContribute(); + } + } + + public SearchPredicate toPredicate() { + if ( predicateResult == null ) { + if ( contributed ) { + // HSEARCH-3207: we must never call a contribution twice. Contributions may have side-effects. + throw new AssertionFailure( + "A predicate object was requested after the corresponding information was contributed to the DSL." + + " There is a bug in Hibernate Search, please report it." + ); + } + contributed = true; + predicateResult = factory.toSearchPredicate( doContribute() ); + } + return predicateResult; + } + + protected abstract B doContribute(); + + protected boolean isContributed() { + return contributed; + } +} diff --git a/engine/src/main/java/org/hibernate/search/engine/search/dsl/sort/SearchSortTerminalContext.java b/engine/src/main/java/org/hibernate/search/engine/search/dsl/sort/SearchSortTerminalContext.java index 961555fa398..bececb609ec 100644 --- a/engine/src/main/java/org/hibernate/search/engine/search/dsl/sort/SearchSortTerminalContext.java +++ b/engine/src/main/java/org/hibernate/search/engine/search/dsl/sort/SearchSortTerminalContext.java @@ -7,6 +7,8 @@ package org.hibernate.search.engine.search.dsl.sort; +import org.hibernate.search.engine.search.SearchSort; + /** * The terminal context of the sort DSL. * @@ -21,4 +23,12 @@ public interface SearchSortTerminalContext { */ N end(); + /** + * Create a {@link SearchSort} instance + * matching the definition given in the previous DSL steps. + * + * @return The {@link SearchSort} resulting from the previous DSL steps. + */ + SearchSort toSort(); + } diff --git a/engine/src/main/java/org/hibernate/search/engine/search/dsl/sort/impl/DistanceSortContextImpl.java b/engine/src/main/java/org/hibernate/search/engine/search/dsl/sort/impl/DistanceSortContextImpl.java index 1742bbf5b7c..39dd3aad947 100644 --- a/engine/src/main/java/org/hibernate/search/engine/search/dsl/sort/impl/DistanceSortContextImpl.java +++ b/engine/src/main/java/org/hibernate/search/engine/search/dsl/sort/impl/DistanceSortContextImpl.java @@ -7,27 +7,27 @@ package org.hibernate.search.engine.search.dsl.sort.impl; import java.util.function.Consumer; -import java.util.function.Supplier; import org.hibernate.search.engine.search.dsl.sort.DistanceSortContext; import org.hibernate.search.engine.search.dsl.sort.SearchSortContainerContext; import org.hibernate.search.engine.search.dsl.sort.SortOrder; +import org.hibernate.search.engine.search.dsl.sort.spi.NonEmptySortContextImpl; import org.hibernate.search.engine.search.dsl.sort.spi.SearchSortContributor; +import org.hibernate.search.engine.search.dsl.sort.spi.SearchSortDslContext; import org.hibernate.search.engine.search.sort.spi.DistanceSortBuilder; import org.hibernate.search.engine.search.sort.spi.SearchSortFactory; import org.hibernate.search.engine.spatial.GeoPoint; -class DistanceSortContextImpl implements DistanceSortContext, SearchSortContributor { +class DistanceSortContextImpl + extends NonEmptySortContextImpl + implements DistanceSortContext, SearchSortContributor { - private final SearchSortContainerContext containerContext; - private final Supplier nextContextProvider; private final DistanceSortBuilder builder; DistanceSortContextImpl(SearchSortContainerContext containerContext, - SearchSortFactory factory, Supplier nextContextProvider, + SearchSortFactory factory, SearchSortDslContext dslContext, String absoluteFieldPath, GeoPoint location) { - this.containerContext = containerContext; - this.nextContextProvider = nextContextProvider; + super( containerContext, dslContext ); this.builder = factory.distance( absoluteFieldPath, location ); } @@ -37,16 +37,6 @@ public DistanceSortContext order(SortOrder order) { return this; } - @Override - public SearchSortContainerContext then() { - return containerContext; - } - - @Override - public N end() { - return nextContextProvider.get(); - } - @Override public void contribute(Consumer collector) { collector.accept( builder.toImplementation() ); diff --git a/engine/src/main/java/org/hibernate/search/engine/search/dsl/sort/impl/FieldSortContextImpl.java b/engine/src/main/java/org/hibernate/search/engine/search/dsl/sort/impl/FieldSortContextImpl.java index 0d1256e9049..c2ed7606950 100644 --- a/engine/src/main/java/org/hibernate/search/engine/search/dsl/sort/impl/FieldSortContextImpl.java +++ b/engine/src/main/java/org/hibernate/search/engine/search/dsl/sort/impl/FieldSortContextImpl.java @@ -7,28 +7,27 @@ package org.hibernate.search.engine.search.dsl.sort.impl; import java.util.function.Consumer; -import java.util.function.Supplier; import org.hibernate.search.engine.search.dsl.sort.FieldSortContext; import org.hibernate.search.engine.search.dsl.sort.FieldSortMissingValueContext; import org.hibernate.search.engine.search.dsl.sort.SearchSortContainerContext; import org.hibernate.search.engine.search.dsl.sort.SortOrder; +import org.hibernate.search.engine.search.dsl.sort.spi.NonEmptySortContextImpl; import org.hibernate.search.engine.search.dsl.sort.spi.SearchSortContributor; +import org.hibernate.search.engine.search.dsl.sort.spi.SearchSortDslContext; import org.hibernate.search.engine.search.sort.spi.FieldSortBuilder; import org.hibernate.search.engine.search.sort.spi.SearchSortFactory; class FieldSortContextImpl + extends NonEmptySortContextImpl implements FieldSortContext, FieldSortMissingValueContext>, SearchSortContributor { - private final SearchSortContainerContext containerContext; - private final Supplier nextContextProvider; private final FieldSortBuilder builder; FieldSortContextImpl(SearchSortContainerContext containerContext, - SearchSortFactory factory, Supplier nextContextProvider, + SearchSortFactory factory, SearchSortDslContext dslContext, String absoluteFieldPath) { - this.containerContext = containerContext; - this.nextContextProvider = nextContextProvider; + super( containerContext, dslContext ); this.builder = factory.field( absoluteFieldPath ); } @@ -61,16 +60,6 @@ public FieldSortContext use(Object value) { return this; } - @Override - public SearchSortContainerContext then() { - return containerContext; - } - - @Override - public N end() { - return nextContextProvider.get(); - } - @Override public void contribute(Consumer collector) { collector.accept( builder.toImplementation() ); diff --git a/engine/src/main/java/org/hibernate/search/engine/search/dsl/sort/impl/NonEmptySortContextImpl.java b/engine/src/main/java/org/hibernate/search/engine/search/dsl/sort/impl/NonEmptySortContextImpl.java deleted file mode 100644 index 3a554808859..00000000000 --- a/engine/src/main/java/org/hibernate/search/engine/search/dsl/sort/impl/NonEmptySortContextImpl.java +++ /dev/null @@ -1,31 +0,0 @@ -/* - * Hibernate Search, full-text search for your domain model - * - * License: GNU Lesser General Public License (LGPL), version 2.1 or later - * See the lgpl.txt file in the root directory or . - */ -package org.hibernate.search.engine.search.dsl.sort.impl; - -import org.hibernate.search.engine.search.dsl.sort.NonEmptySortContext; -import org.hibernate.search.engine.search.dsl.sort.SearchSortContainerContext; -import org.hibernate.search.engine.search.dsl.sort.spi.SearchSortDslContext; - -final class NonEmptySortContextImpl implements NonEmptySortContext { - private final SearchSortContainerContext parent; - private final SearchSortDslContext dslContext; - - NonEmptySortContextImpl(SearchSortContainerContext parent, SearchSortDslContext dslContext) { - this.parent = parent; - this.dslContext = dslContext; - } - - @Override - public SearchSortContainerContext then() { - return parent; - } - - @Override - public N end() { - return dslContext.getNextContext(); - } -} diff --git a/engine/src/main/java/org/hibernate/search/engine/search/dsl/sort/impl/RootSearchSortDslContextImpl.java b/engine/src/main/java/org/hibernate/search/engine/search/dsl/sort/impl/RootSearchSortDslContextImpl.java index e8831189380..f1d342ea99a 100644 --- a/engine/src/main/java/org/hibernate/search/engine/search/dsl/sort/impl/RootSearchSortDslContextImpl.java +++ b/engine/src/main/java/org/hibernate/search/engine/search/dsl/sort/impl/RootSearchSortDslContextImpl.java @@ -52,12 +52,17 @@ public void addChild(SearchSortContributor child) { @Override public SearchSort getNextContext() { + return toSort(); + } + + @Override + public SearchSort toSort() { if ( sortResult == null ) { if ( usedContributors ) { // HSEARCH-3207: we must never call a contribution twice. Contributions may have side-effects. throw new AssertionFailure( "A sort object was requested after the corresponding information was contributed to the DSL." + - " There is a bug in Hibernate Search, please report it." + " There is a bug in Hibernate Search, please report it." ); } List builderResult = getResultingBuilders(); diff --git a/engine/src/main/java/org/hibernate/search/engine/search/dsl/sort/impl/ScoreSortContextImpl.java b/engine/src/main/java/org/hibernate/search/engine/search/dsl/sort/impl/ScoreSortContextImpl.java index 7b6f9011a28..ee83f9dfac7 100644 --- a/engine/src/main/java/org/hibernate/search/engine/search/dsl/sort/impl/ScoreSortContextImpl.java +++ b/engine/src/main/java/org/hibernate/search/engine/search/dsl/sort/impl/ScoreSortContextImpl.java @@ -7,25 +7,25 @@ package org.hibernate.search.engine.search.dsl.sort.impl; import java.util.function.Consumer; -import java.util.function.Supplier; import org.hibernate.search.engine.search.dsl.sort.ScoreSortContext; import org.hibernate.search.engine.search.dsl.sort.SearchSortContainerContext; import org.hibernate.search.engine.search.dsl.sort.SortOrder; +import org.hibernate.search.engine.search.dsl.sort.spi.NonEmptySortContextImpl; import org.hibernate.search.engine.search.dsl.sort.spi.SearchSortContributor; +import org.hibernate.search.engine.search.dsl.sort.spi.SearchSortDslContext; import org.hibernate.search.engine.search.sort.spi.ScoreSortBuilder; import org.hibernate.search.engine.search.sort.spi.SearchSortFactory; -class ScoreSortContextImpl implements ScoreSortContext, SearchSortContributor { +class ScoreSortContextImpl + extends NonEmptySortContextImpl + implements ScoreSortContext, SearchSortContributor { - private final SearchSortContainerContext containerContext; - private final Supplier nextContextProvider; private final ScoreSortBuilder builder; ScoreSortContextImpl(SearchSortContainerContext containerContext, - SearchSortFactory factory, Supplier nextContextProvider) { - this.containerContext = containerContext; - this.nextContextProvider = nextContextProvider; + SearchSortFactory factory, SearchSortDslContext dslContext) { + super( containerContext, dslContext ); this.builder = factory.score(); } @@ -35,16 +35,6 @@ public ScoreSortContext order(SortOrder order) { return this; } - @Override - public SearchSortContainerContext then() { - return containerContext; - } - - @Override - public N end() { - return nextContextProvider.get(); - } - @Override public void contribute(Consumer collector) { collector.accept( builder.toImplementation() ); diff --git a/engine/src/main/java/org/hibernate/search/engine/search/dsl/sort/impl/SearchSortContainerContextImpl.java b/engine/src/main/java/org/hibernate/search/engine/search/dsl/sort/impl/SearchSortContainerContextImpl.java index 4f4d64546af..9b175a491f0 100644 --- a/engine/src/main/java/org/hibernate/search/engine/search/dsl/sort/impl/SearchSortContainerContextImpl.java +++ b/engine/src/main/java/org/hibernate/search/engine/search/dsl/sort/impl/SearchSortContainerContextImpl.java @@ -15,6 +15,7 @@ import org.hibernate.search.engine.search.dsl.sort.SearchSortContainerContext; import org.hibernate.search.engine.search.dsl.sort.SearchSortContainerContextExtension; import org.hibernate.search.engine.search.dsl.sort.SearchSortContainerExtensionContext; +import org.hibernate.search.engine.search.dsl.sort.spi.NonEmptySortContextImpl; import org.hibernate.search.engine.search.dsl.sort.spi.SearchSortDslContext; import org.hibernate.search.engine.search.sort.spi.SearchSortFactory; import org.hibernate.search.engine.spatial.GeoPoint; @@ -39,7 +40,7 @@ public NonEmptySortContext by(SearchSort sort) { @Override public ScoreSortContext byScore() { - ScoreSortContextImpl child = new ScoreSortContextImpl<>( this, factory, dslContext::getNextContext ); + ScoreSortContextImpl child = new ScoreSortContextImpl<>( this, factory, dslContext ); dslContext.addChild( child ); return child; } @@ -53,7 +54,7 @@ public NonEmptySortContext byIndexOrder() { @Override public FieldSortContext byField(String absoluteFieldPath) { FieldSortContextImpl child = new FieldSortContextImpl<>( - this, factory, dslContext::getNextContext, absoluteFieldPath + this, factory, dslContext, absoluteFieldPath ); dslContext.addChild( child ); return child; @@ -62,7 +63,7 @@ public FieldSortContext byField(String absoluteFieldPath) { @Override public DistanceSortContext byDistance(String absoluteFieldPath, GeoPoint location) { DistanceSortContextImpl child = new DistanceSortContextImpl<>( - this, factory, dslContext::getNextContext, absoluteFieldPath, location + this, factory, dslContext, absoluteFieldPath, location ); dslContext.addChild( child ); return child; diff --git a/engine/src/main/java/org/hibernate/search/engine/search/dsl/sort/impl/SearchSortContainerExtensionContextImpl.java b/engine/src/main/java/org/hibernate/search/engine/search/dsl/sort/impl/SearchSortContainerExtensionContextImpl.java index 7906f619f11..6e60f11d38b 100644 --- a/engine/src/main/java/org/hibernate/search/engine/search/dsl/sort/impl/SearchSortContainerExtensionContextImpl.java +++ b/engine/src/main/java/org/hibernate/search/engine/search/dsl/sort/impl/SearchSortContainerExtensionContextImpl.java @@ -13,6 +13,7 @@ import org.hibernate.search.engine.search.dsl.sort.SearchSortContainerContext; import org.hibernate.search.engine.search.dsl.sort.SearchSortContainerContextExtension; import org.hibernate.search.engine.search.dsl.sort.SearchSortContainerExtensionContext; +import org.hibernate.search.engine.search.dsl.sort.spi.NonEmptySortContextImpl; import org.hibernate.search.engine.search.dsl.sort.spi.SearchSortDslContext; import org.hibernate.search.engine.search.sort.spi.SearchSortFactory; diff --git a/engine/src/main/java/org/hibernate/search/engine/search/dsl/sort/spi/NonEmptySortContextImpl.java b/engine/src/main/java/org/hibernate/search/engine/search/dsl/sort/spi/NonEmptySortContextImpl.java new file mode 100644 index 00000000000..7b366cf12fe --- /dev/null +++ b/engine/src/main/java/org/hibernate/search/engine/search/dsl/sort/spi/NonEmptySortContextImpl.java @@ -0,0 +1,36 @@ +/* + * Hibernate Search, full-text search for your domain model + * + * License: GNU Lesser General Public License (LGPL), version 2.1 or later + * See the lgpl.txt file in the root directory or . + */ +package org.hibernate.search.engine.search.dsl.sort.spi; + +import org.hibernate.search.engine.search.SearchSort; +import org.hibernate.search.engine.search.dsl.sort.NonEmptySortContext; +import org.hibernate.search.engine.search.dsl.sort.SearchSortContainerContext; + +public class NonEmptySortContextImpl implements NonEmptySortContext { + private final SearchSortContainerContext containerContext; + protected final SearchSortDslContext dslContext; + + public NonEmptySortContextImpl(SearchSortContainerContext containerContext, SearchSortDslContext dslContext) { + this.containerContext = containerContext; + this.dslContext = dslContext; + } + + @Override + public final SearchSortContainerContext then() { + return containerContext; + } + + @Override + public final N end() { + return dslContext.getNextContext(); + } + + @Override + public final SearchSort toSort() { + return dslContext.toSort(); + } +} diff --git a/engine/src/main/java/org/hibernate/search/engine/search/dsl/sort/spi/SearchSortDslContext.java b/engine/src/main/java/org/hibernate/search/engine/search/dsl/sort/spi/SearchSortDslContext.java index 36adc1c7610..30613622796 100644 --- a/engine/src/main/java/org/hibernate/search/engine/search/dsl/sort/spi/SearchSortDslContext.java +++ b/engine/src/main/java/org/hibernate/search/engine/search/dsl/sort/spi/SearchSortDslContext.java @@ -6,6 +6,8 @@ */ package org.hibernate.search.engine.search.dsl.sort.spi; +import org.hibernate.search.engine.search.SearchSort; + /** * Represents the current context in the search DSL, * i.e. the current position in the sort tree. @@ -35,4 +37,12 @@ default void addChild(B builder) { */ N getNextContext(); + /** + * Create a {@link SearchSort} instance + * matching the definition given in the previous DSL steps. + * + * @return The {@link SearchSort} instance. + */ + SearchSort toSort(); + } diff --git a/integrationtest/backend/tck/src/main/java/org/hibernate/search/integrationtest/backend/tck/search/predicate/SearchPredicateIT.java b/integrationtest/backend/tck/src/main/java/org/hibernate/search/integrationtest/backend/tck/search/predicate/SearchPredicateIT.java index e209619cc21..4fb8c13596e 100644 --- a/integrationtest/backend/tck/src/main/java/org/hibernate/search/integrationtest/backend/tck/search/predicate/SearchPredicateIT.java +++ b/integrationtest/backend/tck/src/main/java/org/hibernate/search/integrationtest/backend/tck/search/predicate/SearchPredicateIT.java @@ -43,13 +43,12 @@ public class SearchPredicateIT { private static final String INDEX_NAME = "IndexName"; - private static final String MATCHING_ID = "matching"; - private static final String NON_MATCHING_ID = "nonMatching"; - private static final String EMPTY_ID = "empty"; + private static final String DOCUMENT_1 = "doc1"; + private static final String DOCUMENT_2 = "doc2"; + private static final String EMPTY = "empty"; - private static final String MATCHING_STRING = "Irving"; - - private static final String NON_MATCHING_STRING = "Auster"; + private static final String STRING_1 = "Irving"; + private static final String STRING_2 = "Auster"; @Rule public SearchSetupHelper setupHelper = new SearchSetupHelper(); @@ -77,18 +76,18 @@ public void match_fluid() { SearchQuery query = searchTarget.query( sessionContext ) .asReferences() - .predicate( root -> root.match().onField( "string" ).matching( MATCHING_STRING ) ) + .predicate( root -> root.match().onField( "string" ).matching( STRING_1 ) ) .build(); DocumentReferencesSearchResultAssert.assertThat( query ) - .hasReferencesHitsAnyOrder( INDEX_NAME, MATCHING_ID ); + .hasReferencesHitsAnyOrder( INDEX_NAME, DOCUMENT_1 ); } @Test public void match_search_predicate() { IndexSearchTarget searchTarget = indexManager.createSearchTarget().build(); - SearchPredicate predicate = searchTarget.predicate().match().onField( "string" ).matching( MATCHING_STRING ).end(); + SearchPredicate predicate = searchTarget.predicate().match().onField( "string" ).matching( STRING_1 ).end(); SearchQuery query = searchTarget.query( sessionContext ) .asReferences() @@ -96,7 +95,7 @@ public void match_search_predicate() { .build(); DocumentReferencesSearchResultAssert.assertThat( query ) - .hasReferencesHitsAnyOrder( INDEX_NAME, MATCHING_ID ); + .hasReferencesHitsAnyOrder( INDEX_NAME, DOCUMENT_1 ); } @Test @@ -105,22 +104,22 @@ public void match_lambda() { SearchQuery query = searchTarget.query( sessionContext ) .asReferences() - .predicate( root -> root.match().onField( "string" ).matching( MATCHING_STRING ) ) + .predicate( root -> root.match().onField( "string" ).matching( STRING_1 ) ) .build(); DocumentReferencesSearchResultAssert.assertThat( query ) - .hasReferencesHitsAnyOrder( INDEX_NAME, MATCHING_ID ); + .hasReferencesHitsAnyOrder( INDEX_NAME, DOCUMENT_1 ); } @Test - public void match_lambda_caching() { + public void match_lambda_caching_root() { IndexSearchTarget searchTarget = indexManager.createSearchTarget().build(); AtomicReference cache = new AtomicReference<>(); - Consumer> cachingContributor = c -> { + Consumer> cachingContributor = c -> { if ( cache.get() == null ) { - SearchPredicate result = c.match().onField( "string" ).matching( MATCHING_STRING ).end(); + SearchPredicate result = c.match().onField( "string" ).matching( STRING_1 ).toPredicate(); cache.set( result ); } else { @@ -136,7 +135,7 @@ public void match_lambda_caching() { .build(); DocumentReferencesSearchResultAssert.assertThat( query ) - .hasReferencesHitsAnyOrder( INDEX_NAME, MATCHING_ID ); + .hasReferencesHitsAnyOrder( INDEX_NAME, DOCUMENT_1 ); Assertions.assertThat( cache ).doesNotHaveValue( null ); @@ -146,7 +145,47 @@ public void match_lambda_caching() { .build(); DocumentReferencesSearchResultAssert.assertThat( query ) - .hasReferencesHitsAnyOrder( INDEX_NAME, MATCHING_ID ); + .hasReferencesHitsAnyOrder( INDEX_NAME, DOCUMENT_1 ); + } + + @Test + public void match_lambda_caching_nonRoot() { + IndexSearchTarget searchTarget = indexManager.createSearchTarget().build(); + + AtomicReference cache = new AtomicReference<>(); + + Consumer> cachingContributor = c -> { + if ( cache.get() == null ) { + SearchPredicate result = c.match().onField( "string" ).matching( STRING_1 ).toPredicate(); + cache.set( result ); + } + else { + c.predicate( cache.get() ); + } + }; + + Assertions.assertThat( cache ).hasValue( null ); + + SearchQuery query = searchTarget.query( sessionContext ) + .asReferences() + .predicate( root -> root.bool().must( cachingContributor ) ) + .build(); + + DocumentReferencesSearchResultAssert.assertThat( query ) + .hasReferencesHitsAnyOrder( INDEX_NAME, DOCUMENT_1 ); + + Assertions.assertThat( cache ).doesNotHaveValue( null ); + + query = searchTarget.query( sessionContext ) + .asReferences() + .predicate( root -> root.bool() + .should( cachingContributor ) + .should( c -> c.match().onField( "string" ).matching( STRING_2 ) ) + ) + .build(); + + DocumentReferencesSearchResultAssert.assertThat( query ) + .hasReferencesHitsAnyOrder( INDEX_NAME, DOCUMENT_1, DOCUMENT_2 ); } @Test @@ -158,11 +197,11 @@ public void extension() { query = searchTarget.query( sessionContext ) .asReferences() .predicate( root -> root.extension( new SupportedExtension<>() ) - .match().onField( "string" ).matching( MATCHING_STRING ) + .match().onField( "string" ).matching( STRING_1 ) ) .build(); DocumentReferencesSearchResultAssert.assertThat( query ) - .hasReferencesHitsAnyOrder( INDEX_NAME, MATCHING_ID ); + .hasReferencesHitsAnyOrder( INDEX_NAME, DOCUMENT_1 ); // Conditional extensions with orElse - two, both supported query = searchTarget.query( sessionContext ) @@ -171,7 +210,7 @@ public void extension() { // FIXME find some way to forbid using the context passed to the consumers twice... ? .ifSupported( new SupportedExtension<>(), - c -> c.match().onField( "string" ).matching( MATCHING_STRING ).end() + c -> c.match().onField( "string" ).matching( STRING_1 ).end() ) .ifSupported( new SupportedExtension<>(), @@ -181,7 +220,7 @@ public void extension() { ) .build(); DocumentReferencesSearchResultAssert.assertThat( query ) - .hasReferencesHitsAnyOrder( INDEX_NAME, MATCHING_ID ); + .hasReferencesHitsAnyOrder( INDEX_NAME, DOCUMENT_1 ); // Conditional extensions with orElse - two, second supported query = searchTarget.query( sessionContext ) @@ -193,7 +232,7 @@ public void extension() { ) .ifSupported( new SupportedExtension<>(), - c -> c.match().onField( "string" ).matching( MATCHING_STRING ).end() + c -> c.match().onField( "string" ).matching( STRING_1 ).end() ) .orElse( ignored -> Assert.fail( "This should not be called" ) @@ -201,7 +240,7 @@ public void extension() { ) .build(); DocumentReferencesSearchResultAssert.assertThat( query ) - .hasReferencesHitsAnyOrder( INDEX_NAME, MATCHING_ID ); + .hasReferencesHitsAnyOrder( INDEX_NAME, DOCUMENT_1 ); // Conditional extensions with orElse - two, both unsupported query = searchTarget.query( sessionContext ) @@ -216,12 +255,12 @@ public void extension() { ignored -> Assert.fail( "This should not be called" ) ) .orElse( - c -> c.match().onField( "string" ).matching( MATCHING_STRING ).end() + c -> c.match().onField( "string" ).matching( STRING_1 ).end() ) ) .build(); DocumentReferencesSearchResultAssert.assertThat( query ) - .hasReferencesHitsAnyOrder( INDEX_NAME, MATCHING_ID ); + .hasReferencesHitsAnyOrder( INDEX_NAME, DOCUMENT_1 ); // Conditional extensions without orElse - one, unsupported query = searchTarget.query( sessionContext ) @@ -234,24 +273,24 @@ public void extension() { ) ) .must( - c -> c.match().onField( "string" ).matching( MATCHING_STRING ).end() + c -> c.match().onField( "string" ).matching( STRING_1 ).end() ) .end() ) .build(); DocumentReferencesSearchResultAssert.assertThat( query ) - .hasReferencesHitsAnyOrder( INDEX_NAME, MATCHING_ID ); + .hasReferencesHitsAnyOrder( INDEX_NAME, DOCUMENT_1 ); } private void initData() { IndexWorkPlan workPlan = indexManager.createWorkPlan( sessionContext ); - workPlan.add( referenceProvider( MATCHING_ID ), document -> { - indexAccessors.string.write( document, MATCHING_STRING ); + workPlan.add( referenceProvider( DOCUMENT_1 ), document -> { + indexAccessors.string.write( document, STRING_1 ); } ); - workPlan.add( referenceProvider( NON_MATCHING_ID ), document -> { - indexAccessors.string.write( document, NON_MATCHING_STRING ); + workPlan.add( referenceProvider( DOCUMENT_2 ), document -> { + indexAccessors.string.write( document, STRING_2 ); } ); - workPlan.add( referenceProvider( EMPTY_ID ), document -> { } ); + workPlan.add( referenceProvider( EMPTY ), document -> { } ); workPlan.execute().join(); @@ -261,7 +300,7 @@ private void initData() { .asReferences() .predicate( root -> root.matchAll() ) .build(); - assertThat( query ).hasReferencesHitsAnyOrder( INDEX_NAME, MATCHING_ID, NON_MATCHING_ID, EMPTY_ID ); + assertThat( query ).hasReferencesHitsAnyOrder( INDEX_NAME, DOCUMENT_1, DOCUMENT_2, EMPTY ); } private static class IndexAccessors { diff --git a/integrationtest/backend/tck/src/main/java/org/hibernate/search/integrationtest/backend/tck/search/sort/SearchSortIT.java b/integrationtest/backend/tck/src/main/java/org/hibernate/search/integrationtest/backend/tck/search/sort/SearchSortIT.java index 2d04fb89bac..dd4e57ab3c6 100644 --- a/integrationtest/backend/tck/src/main/java/org/hibernate/search/integrationtest/backend/tck/search/sort/SearchSortIT.java +++ b/integrationtest/backend/tck/src/main/java/org/hibernate/search/integrationtest/backend/tck/search/sort/SearchSortIT.java @@ -208,9 +208,9 @@ public void separateSort() { public void lambda_caching() { AtomicReference cache = new AtomicReference<>(); - Consumer> cachingContributor = c -> { + Consumer> cachingContributor = c -> { if ( cache.get() == null ) { - SearchSort result = c.byField( "string" ).onMissingValue().sortLast().end(); + SearchSort result = c.byField( "string" ).onMissingValue().sortLast().toSort(); cache.set( result ); } else {