HSEARCH-4162 Have the SearchPredicateFactory expect field paths relat…

…ive to the supporting object field in named predicates
hibernate · Jun 17, 2021 · 64c15e7 · 64c15e7
1 parent 2b4ff4c
commit 64c15e7
Show file tree

Hide file tree

Showing 9 changed files with 12 additions and 101 deletions.
diff --git a/...rnate/search/backend/elasticsearch/search/predicate/impl/ElasticsearchNamedPredicate.java b/...rnate/search/backend/elasticsearch/search/predicate/impl/ElasticsearchNamedPredicate.java
@@ -151,11 +151,5 @@ public Optional<Object> paramOptional(String name) {
 			Contracts.assertNotNull( name, "name" );
 			return Optional.ofNullable( params.get( name ) );
 		}
-
-		@Override
-		public String absolutePath(String relativeFieldPath) {
-			Contracts.assertNotNull( relativeFieldPath, "relativeFieldPath" );
-			return field.absolutePath( relativeFieldPath );
-		}
 	}
 }
diff --git a/.../java/org/hibernate/search/backend/lucene/search/predicate/impl/LuceneNamedPredicate.java b/.../java/org/hibernate/search/backend/lucene/search/predicate/impl/LuceneNamedPredicate.java
@@ -147,11 +147,5 @@ public Optional<Object> paramOptional(String name) {
 			Contracts.assertNotNull( name, "name" );
 			return Optional.ofNullable( params.get( name ) );
 		}
-
-		@Override
-		public String absolutePath(String relativeFieldPath) {
-			Contracts.assertNotNull( relativeFieldPath, "relativeFieldPath" );
-			return field.absolutePath( relativeFieldPath );
-		}
 	}
 }
diff --git a/...entation/src/main/asciidoc/reference/mapper-orm-bridge-named-predicate.asciidoc b/...entation/src/main/asciidoc/reference/mapper-orm-bridge-named-predicate.asciidoc
@@ -55,9 +55,9 @@ Alternatively it is possible to use `paramOptional` to get the `java.util.Option
 <4> The provider uses the predicate factory to create predicates.
 In this example, this implementation transforms a pattern with a custom format into three patterns,
 one for each field populated by the bridge.
-<5> Be careful: the search predicate factory expects absolute paths.
-The context can turn relative paths into absolute paths;
-here the path `departmentCode` will be transformed to `<path to the object field>.departmentCode`.
+<5> Be careful: the search predicate factory expects paths
+relative to the object field where the named predicate was registered.
+Here the path `departmentCode` will be understood as `<path to the object field>.departmentCode`.
 <6> Do not forget to call `toPredicate()` to return a `SearchPredicate` instance.
 
 [source, JAVA, indent=0, subs="+callouts"]

diff --git a/.../hibernate/search/documentation/mapper/orm/bridge/namedpredicate/SkuIdentifierBinder.java b/.../hibernate/search/documentation/mapper/orm/bridge/namedpredicate/SkuIdentifierBinder.java
@@ -101,17 +101,17 @@ public SearchPredicate create(NamedPredicateProviderContext context) {
 				String[] patternParts = pattern.split( "\\." );
 				if ( patternParts.length > 0 ) {
 					b.must( f.wildcard()
-							.field( context.absolutePath( "departmentCode" ) ) // <5>
+							.field( "departmentCode" ) // <5>
 							.matching( patternParts[0] ) );
 				}
 				if ( patternParts.length > 1 ) {
 					b.must( f.wildcard()
-							.field( context.absolutePath( "collectionCode" ) )
+							.field( "collectionCode" )
 							.matching( patternParts[1] ) );
 				}
 				if ( patternParts.length > 2 ) {
 					b.must( f.wildcard()
-							.field( context.absolutePath( "itemCode" ) )
+							.field( "itemCode" )
 							.matching( patternParts[2] ) );
 				}
 			} ).toPredicate(); // <6>

diff --git a/.../org/hibernate/search/engine/search/predicate/dsl/impl/NamedPredicateOptionsStepImpl.java b/.../org/hibernate/search/engine/search/predicate/dsl/impl/NamedPredicateOptionsStepImpl.java
@@ -29,7 +29,7 @@ public NamedPredicateOptionsStepImpl(SearchPredicateFactory predicateFactory,
 		SearchQueryElementTypeKey<NamedPredicateBuilder> key = PredicateTypeKeys.named( predicateName );
 		this.builder = absoluteFieldPath == null ? scope.rootQueryElement( key )
 				: scope.fieldQueryElement( absoluteFieldPath, key );
-		builder.factory( predicateFactory );
+		builder.factory( absoluteFieldPath == null ? predicateFactory : predicateFactory.withRoot( absoluteFieldPath ) );
 	}
 
 	@Override

diff --git a/...org/hibernate/search/engine/search/predicate/factories/NamedPredicateProviderContext.java b/...org/hibernate/search/engine/search/predicate/factories/NamedPredicateProviderContext.java
@@ -22,6 +22,8 @@ public interface NamedPredicateProviderContext {
 
 	/**
 	 * @return A predicate factory.
+	 * If the named predicate was registered on an object field,
+	 * this factory expects field paths to be provided relative to that same object field.
 	 * This factory is only valid in the present context and must not be used after
 	 * {@link NamedPredicateProvider#create(NamedPredicateProviderContext)} returns.
 	 * @see SearchPredicateFactory
@@ -44,9 +46,4 @@ public interface NamedPredicateProviderContext {
 	 */
 	Optional<Object> paramOptional(String name);
 
-	/**
-	 * @param relativeFieldPath The path a field, relative to the element to which the named predicate was assigned.
-	 * @return The absolute path of the field. Note the path is returned even if the field doesn't exist.
-	 */
-	String absolutePath(String relativeFieldPath);
 }
diff --git a/...hibernate/search/integrationtest/backend/lucene/testsupport/util/LuceneTckTestRunner.java b/...hibernate/search/integrationtest/backend/lucene/testsupport/util/LuceneTckTestRunner.java
@@ -19,7 +19,7 @@
 @RunWith(ClasspathSuite.class)
 @ClasspathSuite.ClassnameFilters({
 		"!.*\\$.*", // Exclude nested tests, typically used with the Enclosed runner
-		".*\\.tck\\..*"
+		".*\\.tck\\..*search.*"
 })
 public class LuceneTckTestRunner {
 }
diff --git a/...g/hibernate/search/integrationtest/backend/tck/search/predicate/NamedPredicateBaseIT.java b/...g/hibernate/search/integrationtest/backend/tck/search/predicate/NamedPredicateBaseIT.java
@@ -237,14 +237,12 @@ public TestNamedPredicateProvider(String field1Name, String field2Name) {
 
 		@Override
 		public SearchPredicate create(NamedPredicateProviderContext context) {
-			String field1Path = context.absolutePath( field1Name );
-			String field2Path = context.absolutePath( field2Name );
 			String word1 = (String) context.param( "value1" );
 			String word2 = (String) context.param( "value2" );
 			SearchPredicateFactory f = context.predicate();
 			return f.bool()
-					.must( f.match().field( field1Path ).matching( word1 ) )
-					.must( f.match().field( field2Path ).matching( word2 ) )
+					.must( f.match().field( field1Name ).matching( word1 ) )
+					.must( f.match().field( field2Name ).matching( word2 ) )
 					.toPredicate();
 		}
 	}

diff --git a/.../search/integrationtest/backend/tck/search/predicate/NamedPredicateProviderContextIT.java b/.../search/integrationtest/backend/tck/search/predicate/NamedPredicateProviderContextIT.java
@@ -112,78 +112,6 @@ public void missingParam() {
 						"Param with name 'missing' has not been defined for the named predicate 'stub-predicate'." );
 	}
 
-	@Test
-	public void absolutePath_root() {
-		Object[] actualResolved = new Object[2];
-
-		assertThatQuery( index.query()
-				.where( f -> f.named( "stub-predicate" )
-						.param( "impl", (NamedPredicateProvider) context -> {
-							actualResolved[0] = context.absolutePath( "foo" );
-							actualResolved[1] = context.absolutePath( "foo.bar" );
-							return context.predicate().matchAll().toPredicate();
-						} ) ) )
-				.hasDocRefHitsAnyOrder( index.typeName(), DOCUMENT_1 );
-
-		assertThat( actualResolved ).containsExactly( "foo", "foo.bar" );
-	}
-
-	@Test
-	public void absolutePath_nested() {
-		Object[] actualResolved = new Object[2];
-
-		assertThatQuery( index.query()
-				.where( f -> f.named( "nested.stub-predicate" )
-						.param( "impl", (NamedPredicateProvider) context -> {
-							actualResolved[0] = context.absolutePath( "foo" );
-							actualResolved[1] = context.absolutePath( "foo.bar" );
-							return context.predicate().matchAll().toPredicate();
-						} ) ) )
-				.hasNoHits();
-
-		assertThat( actualResolved ).containsExactly( "nested.foo", "nested.foo.bar" );
-	}
-
-	@Test
-	public void absolutePath_flattened() {
-		Object[] actualResolved = new Object[2];
-
-		assertThatQuery( index.query()
-				.where( f -> f.named( "flattened.stub-predicate" )
-						.param( "impl", (NamedPredicateProvider) context -> {
-							actualResolved[0] = context.absolutePath( "foo" );
-							actualResolved[1] = context.absolutePath( "foo.bar" );
-							return context.predicate().matchAll().toPredicate();
-						} ) ) )
-				.hasDocRefHitsAnyOrder( index.typeName(), DOCUMENT_1 );
-
-		assertThat( actualResolved ).containsExactly( "flattened.foo", "flattened.foo.bar" );
-	}
-
-	@Test
-	public void absolutePath_root_null() {
-		assertThatThrownBy( () -> index.createScope().predicate().named( "stub-predicate" )
-				.param( "impl", (NamedPredicateProvider) context -> {
-					context.absolutePath( null );
-					return context.predicate().matchAll().toPredicate();
-				} )
-				.toPredicate() )
-				.isInstanceOf( IllegalArgumentException.class )
-				.hasMessageContaining( "'relativeFieldPath' must not be null" );
-	}
-
-	@Test
-	public void absolutePath_objectField_null() {
-		assertThatThrownBy( () -> index.createScope().predicate().named( "nested.stub-predicate" )
-						.param( "impl", (NamedPredicateProvider) context -> {
-							context.absolutePath( null );
-							return context.predicate().matchAll().toPredicate();
-						} )
-						.toPredicate() )
-				.isInstanceOf( IllegalArgumentException.class )
-				.hasMessageContaining( "'relativeFieldPath' must not be null" );
-	}
-
 	private static class IndexBinding {
 		final SimpleFieldModel<String> field1;
 		final ObjectFieldBinding nested;