Backport of HSEARCH-5255 to 7.2 - Change how projection cardinality check is done on "request"

backend/elasticsearch/src/main/java/org/hibernate/search/backend/elasticsearch/search/projection/impl/ElasticsearchFieldProjection.java

@@ -80,8 +80,7 @@ public String toString() {
 	@Override
 	public ValueFieldExtractor<?> request(JsonObject requestBody, ProjectionRequestContext context) {
 		ProjectionRequestContext innerContext = context.forField( absoluteFieldPath, absoluteFieldPathComponents );
-		if ( requiredContextAbsoluteFieldPath != null
-				&& !requiredContextAbsoluteFieldPath.equals( context.absoluteCurrentFieldPath() ) ) {
+		if ( !context.projectionCardinalityCorrectlyAddressed( requiredContextAbsoluteFieldPath ) ) {
 			throw log.invalidSingleValuedProjectionOnValueFieldInMultiValuedObjectField(
 					absoluteFieldPath, requiredContextAbsoluteFieldPath );
 		}

backend/elasticsearch/src/main/java/org/hibernate/search/backend/elasticsearch/search/projection/impl/FieldProjectionRequestContext.java

@@ -86,4 +86,16 @@ public NamedValues queryParameters() {
 		return root().queryParameters();
 	}
 
+	@Override
+	public boolean projectionCardinalityCorrectlyAddressed(String requiredContextAbsoluteFieldPath) {
+		// requiredContextAbsoluteFieldPath generally speaking should be the path from root to the closest multi-valued field
+		// (i.e. the very last multi-valued field in the current path)
+		//
+		// Hence if we have the context that includes that path or matches it exactly then we can assume that the cardinality was correctly handled:
+		return requiredContextAbsoluteFieldPath == null
+				|| requiredContextAbsoluteFieldPath.equals( absoluteCurrentFieldPath )
+				|| ( absoluteCurrentFieldPath != null
+						&& absoluteCurrentFieldPath.startsWith( requiredContextAbsoluteFieldPath + "." ) );
+	}
+
 }

backend/elasticsearch/src/main/java/org/hibernate/search/backend/elasticsearch/search/projection/impl/ProjectionRequestContext.java

@@ -23,4 +23,5 @@ public interface ProjectionRequestContext {
 
 	NamedValues queryParameters();
 
+	boolean projectionCardinalityCorrectlyAddressed(String requiredContextAbsoluteFieldPath);
 }

backend/elasticsearch/src/main/java/org/hibernate/search/backend/elasticsearch/search/query/impl/ElasticsearchSearchQueryRequestContext.java

@@ -126,6 +126,11 @@ public NamedValues queryParameters() {
 		return parameters;
 	}
 
+	@Override
+	public boolean projectionCardinalityCorrectlyAddressed(String requiredContextAbsoluteFieldPath) {
+		return requiredContextAbsoluteFieldPath == null;
+	}
+
 	@Override
 	public ElasticsearchSearchHighlighter highlighter(String highlighterName) {
 		if ( highlighterName == null ) {

backend/lucene/src/main/java/org/hibernate/search/backend/lucene/search/projection/impl/LuceneDistanceToFieldProjection.java

@@ -94,8 +94,7 @@ public Extractor<?, P> request(ProjectionRequestContext context) {
 		}
 		else {
 			context.checkValidField( absoluteFieldPath );
-			if ( requiredContextAbsoluteFieldPath != null
-					&& !requiredContextAbsoluteFieldPath.equals( context.absoluteCurrentNestedFieldPath() ) ) {
+			if ( !context.projectionCardinalityCorrectlyAddressed( requiredContextAbsoluteFieldPath ) ) {
 				throw log.invalidSingleValuedProjectionOnValueFieldInMultiValuedObjectField(
 						absoluteFieldPath, requiredContextAbsoluteFieldPath );
 			}

backend/lucene/src/main/java/org/hibernate/search/backend/lucene/search/projection/impl/LuceneFieldProjection.java

@@ -77,8 +77,7 @@ public String toString() {
 	@Override
 	public ValueFieldExtractor<?> request(ProjectionRequestContext context) {
 		context.checkValidField( absoluteFieldPath );
-		if ( requiredContextAbsoluteFieldPath != null
-				&& !requiredContextAbsoluteFieldPath.equals( context.absoluteCurrentNestedFieldPath() ) ) {
+		if ( !context.projectionCardinalityCorrectlyAddressed( requiredContextAbsoluteFieldPath ) ) {
 			throw log.invalidSingleValuedProjectionOnValueFieldInMultiValuedObjectField(
 					absoluteFieldPath, requiredContextAbsoluteFieldPath );
 		}

backend/lucene/src/main/java/org/hibernate/search/backend/lucene/search/projection/impl/LuceneObjectProjection.java

@@ -76,8 +76,7 @@ public String toString() {
 	@Override
 	public Extractor<?, P> request(ProjectionRequestContext context) {
 		ProjectionRequestContext innerContext = context.forField( absoluteFieldPath, nested );
-		if ( requiredContextAbsoluteFieldPath != null
-				&& !requiredContextAbsoluteFieldPath.equals( context.absoluteCurrentNestedFieldPath() ) ) {
+		if ( !context.projectionCardinalityCorrectlyAddressed( requiredContextAbsoluteFieldPath ) ) {
 			throw log.invalidSingleValuedProjectionOnValueFieldInMultiValuedObjectField(
 					absoluteFieldPath, requiredContextAbsoluteFieldPath );
 		}

backend/lucene/src/main/java/org/hibernate/search/backend/lucene/search/projection/impl/ProjectionRequestContext.java

@@ -95,6 +95,15 @@ public String absoluteCurrentNestedFieldPath() {
 		return absoluteCurrentNestedFieldPath;
 	}
 
+	public boolean projectionCardinalityCorrectlyAddressed(String requiredContextAbsoluteFieldPath) {
+		String absoluteCurrentNestedFieldPath = absoluteCurrentNestedFieldPath();
+		return requiredContextAbsoluteFieldPath == null
+				|| requiredContextAbsoluteFieldPath.equals( absoluteCurrentNestedFieldPath )
+				|| ( absoluteCurrentNestedFieldPath != null
+						&& absoluteCurrentNestedFieldPath.startsWith( requiredContextAbsoluteFieldPath + "." ) );
+
+	}
+
 	public String absoluteCurrentFieldPath() {
 		return absoluteCurrentFieldPath;
 	}

integrationtest/backend/tck/src/main/java/org/hibernate/search/integrationtest/backend/tck/search/projection/FieldProjectionBaseIT.java

@@ -4,24 +4,33 @@
  */
 package org.hibernate.search.integrationtest.backend.tck.search.projection;
 
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
 import java.math.BigDecimal;
 import java.util.ArrayList;
 import java.util.Comparator;
 import java.util.List;
 
+import org.hibernate.search.engine.backend.document.DocumentElement;
+import org.hibernate.search.engine.backend.document.model.dsl.IndexSchemaElement;
+import org.hibernate.search.engine.backend.document.model.dsl.IndexSchemaObjectField;
 import org.hibernate.search.engine.backend.types.ObjectStructure;
+import org.hibernate.search.engine.backend.types.Projectable;
 import org.hibernate.search.engine.backend.types.dsl.SearchableProjectableIndexFieldTypeOptionsStep;
 import org.hibernate.search.engine.search.projection.dsl.ProjectionFinalStep;
 import org.hibernate.search.engine.search.projection.dsl.SearchProjectionFactory;
 import org.hibernate.search.integrationtest.backend.tck.testsupport.types.FieldTypeDescriptor;
 import org.hibernate.search.integrationtest.backend.tck.testsupport.types.StandardFieldTypeDescriptor;
 import org.hibernate.search.integrationtest.backend.tck.testsupport.util.TckConfiguration;
 import org.hibernate.search.integrationtest.backend.tck.testsupport.util.extension.SearchSetupHelper;
+import org.hibernate.search.util.common.SearchException;
 import org.hibernate.search.util.impl.integrationtest.mapper.stub.BulkIndexer;
 import org.hibernate.search.util.impl.integrationtest.mapper.stub.SimpleMappedIndex;
 
 import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.Nested;
+import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.RegisterExtension;
 import org.junit.jupiter.params.provider.Arguments;
 
@@ -52,7 +61,9 @@ static void setup() {
 						InObjectProjectionConfigured.missingLevel2SingleValuedFieldIndex,
 						InvalidFieldConfigured.index,
 						ProjectableConfigured.projectableDefaultIndex, ProjectableConfigured.projectableYesIndex,
-						ProjectableConfigured.projectableNoIndex )
+						ProjectableConfigured.projectableNoIndex,
+						CardinalityCheckConfigured.index,
+						CardinalityCheckConfigured.containing )
 				.setup();
 
 		BulkIndexer compositeForEachMainIndexer = InObjectProjectionConfigured.mainIndex.bulkIndexer();
@@ -73,7 +84,12 @@ static void setup() {
 
 		compositeForEachMainIndexer.join( compositeForEachMissingLevel1Indexer,
 				compositeForEachMissingLevel1SingleValuedFieldIndexer, compositeForEachMissingLevel2Indexer,
-				compositeForEachMissingLevel2SingleValuedFieldIndexer );
+				compositeForEachMissingLevel2SingleValuedFieldIndexer,
+				CardinalityCheckConfigured.containing.binding()
+						.contribute( CardinalityCheckConfigured.containing.bulkIndexer() ),
+				CardinalityCheckConfigured.index.binding()
+						.contribute( CardinalityCheckConfigured.index.bulkIndexer() )
+		);
 	}
 
 	@Nested
@@ -210,4 +226,208 @@ protected String projectionTrait() {
 			return "projection:field";
 		}
 	}
+
+	@Nested
+	class CardinalityCheckIT extends CardinalityCheckConfigured {
+		// JDK 11 does not allow static fields in non-static inner class and JUnit does not allow running @Nested tests in static inner classes...
+	}
+
+	abstract static class CardinalityCheckConfigured {
+		private static final SimpleMappedIndex<IndexContaining> containing =
+				SimpleMappedIndex.of( IndexContaining::new ).name( "cardinality-containing" );
+		private static final SimpleMappedIndex<Index> index = SimpleMappedIndex.of( Index::new ).name( "cardinality-index" );
+
+		/**
+		 * In this case we build a projection with object/fields projections recreating the entity tree structure.
+		 * Hence, it should be close to what a projection constructor is doing.
+		 */
+		@Test
+		void testAsIfItWasProjectionConstructors() {
+			List<List<?>> hits = index.createScope().query()
+					.select( f -> f.composite(
+							f.id(),
+							f.field( "string" ),
+							f.object( "contained" ).from(
+									f.field( "contained.id" ),
+									f.field( "contained.containedString" ),
+									f.object( "contained.deeperContained" ).from(
+											f.field( "contained.deeperContained.id" ),
+											f.field( "contained.deeperContained.deeperContainedString" )
+									).asList()
+							).asList().multi()
+					) )
+					.where( f -> f.matchAll() )
+					.fetchAllHits();
+
+			assertThat( hits ).hasSize( 1 )
+					.contains(
+							List.of(
+									"1",
+									"string",
+									List.of(
+											List.of( "10", "containedString1",
+													List.of( "100", "deeperContainedString1" ) ),
+											List.of( "20", "containedString2",
+													List.of( "200", "deeperContainedString2" ) )
+									)
+							)
+					);
+		}
+
+		/**
+		 * We want to make sure that cardinality is correctly checked when we request a projection for a single field with a "long" path
+		 * containing both single and multi fields in it.
+		 * <p>
+		 * Since there is a multi-`contained` the resulting caridnality of the projected field is expected to be also multi
+		 */
+		@Test
+		void testFieldProjectionLongPath_correctCardinality() {
+			List<List<Object>> hits = index.createScope().query()
+					.select( f -> f.field( "contained.deeperContained.id" ).multi() )
+					.where( f -> f.matchAll() )
+					.fetchAllHits();
+
+			assertThat( hits ).hasSize( 1 )
+					.contains( List.of( "100", "200" ) );
+		}
+
+		@Test
+		void testFieldProjectionLongPath_incorrectCardinality() {
+			assertThatThrownBy( () -> index.createScope().query()
+					.select( f -> f.field( "contained.deeperContained.id" ) )
+					.where( f -> f.matchAll() )
+					.fetchAllHits() )
+					.isInstanceOf( SearchException.class )
+					.hasMessageContaining( "Invalid cardinality for projection on field 'contained.deeperContained.id'" );
+		}
+
+		@Test
+		void testFieldProjectionLongPath_correctCardinality_multiAtDifferentLevelInPath() {
+			assertThat( containing.createScope().query()
+					.select( f -> f.field( "single.contained.deeperContained.id" ).multi() )
+					.where( f -> f.matchAll() )
+					.fetchAllHits()
+			).hasSize( 1 )
+					.contains( List.of( "100", "200" ) );
+		}
+
+		@Test
+		void testFieldProjectionLongPath_correctCardinality_multiAtDifferentLevelInPath_multipleMultis() {
+			assertThat( containing.createScope().query()
+					.select( f -> f.field( "list.contained.deeperContained.id" ).multi() )
+					.where( f -> f.matchAll() )
+					.fetchAllHits() ).hasSize( 1 )
+					.contains( List.of( "100", "200" ) );
+		}
+
+		/**
+		 * Here we take all multi fields as object projections with expected cardinalities and then
+		 * add the "deepest" field as a simple single-valued field:
+		 */
+		@Test
+		void testFieldProjectionLongPath_correctCardinality_multiFieldsAsObjects() {
+			assertThat( containing.createScope().query()
+					.select( f -> f.object( "list" )
+							.from( f.object( "list.contained" )
+									.from( f.field( "list.contained.deeperContained.id" ) ).asList().multi() )
+							.asList().multi() )
+					.where( f -> f.matchAll() )
+					.fetchAllHits() ).hasSize( 1 )
+					.contains(
+							List.of( List.of( List.of( List.of( "100" ), List.of( "200" ) ) ) ) );
+		}
+
+		@Test
+		void testFieldProjectionLongPath_incorrectCardinality_multiAtDifferentLevelInPath_multipleMultis() {
+			assertThatThrownBy( () -> containing.createScope().query()
+					.select( f -> f.field( "list.contained.deeperContained.id" ) )
+					.where( f -> f.matchAll() )
+					.fetchAllHits() )
+					.isInstanceOf( SearchException.class )
+					.hasMessageContaining(
+							"Invalid cardinality for projection on field 'list.contained.deeperContained.id'" );
+		}
+
+		@Test
+		void testFieldProjectionLongPath_incorrectCardinality_multiAtDifferentLevelInPath() {
+			assertThatThrownBy( () -> containing.createScope().query()
+					.select( f -> f.field( "single.contained.deeperContained.id" ) )
+					.where( f -> f.matchAll() )
+					.fetchAllHits() )
+					.isInstanceOf( SearchException.class )
+					.hasMessageContaining(
+							"Invalid cardinality for projection on field 'single.contained.deeperContained.id'" );
+		}
+
+		public static final class IndexContaining extends IndexBase {
+			public IndexContaining(IndexSchemaElement root) {
+				IndexSchemaObjectField single = root.objectField( "single", ObjectStructure.NESTED );
+				addIndexFields( single );
+				single.toReference();
+
+				IndexSchemaObjectField list = root.objectField( "list", ObjectStructure.NESTED ).multiValued();
+				addIndexFields( list );
+				list.toReference();
+			}
+
+			BulkIndexer contribute(BulkIndexer indexer) {
+				indexer.add( "1", d -> {
+					contribute( d.addObject( "single" ) );
+					contribute( d.addObject( "list" ) );
+				} );
+				return indexer;
+			}
+		}
+
+		public static class Index extends IndexBase {
+
+			public Index(IndexSchemaElement root) {
+				addIndexFields( root );
+			}
+
+			BulkIndexer contribute(BulkIndexer indexer) {
+				indexer.add( "1", this::contribute );
+				return indexer;
+			}
+		}
+
+		public abstract static class IndexBase {
+			protected void addIndexFields(IndexSchemaElement el) {
+				el.field( "id", f -> f.asString().projectable( Projectable.YES ) ).toReference();
+				el.field( "string", f -> f.asString().projectable( Projectable.YES ) ).toReference();
+
+				IndexSchemaObjectField contained = el.objectField( "contained", ObjectStructure.NESTED ).multiValued();
+				contained.field( "id", f -> f.asString().projectable( Projectable.YES ) ).toReference();
+				contained.field( "containedString", f -> f.asString().projectable( Projectable.YES ) ).toReference();
+
+				IndexSchemaObjectField deeperContained = contained.objectField( "deeperContained", ObjectStructure.NESTED );
+				deeperContained.field( "id", f -> f.asString().projectable( Projectable.YES ) ).toReference();
+				deeperContained.field( "deeperContainedString", f -> f.asString().projectable( Projectable.YES ) )
+						.toReference();
+
+				deeperContained.toReference();
+				contained.toReference();
+			}
+
+			protected void contribute(DocumentElement element) {
+				element.addValue( "id", "1" );
+				element.addValue( "string", "string" );
+
+				DocumentElement contained = element.addObject( "contained" );
+				contained.addValue( "id", "10" );
+				contained.addValue( "containedString", "containedString1" );
+				DocumentElement deeperContained = contained.addObject( "deeperContained" );
+				deeperContained.addValue( "id", "100" );
+				deeperContained.addValue( "deeperContainedString", "deeperContainedString1" );
+
+				contained = element.addObject( "contained" );
+				contained.addValue( "id", "20" );
+				contained.addValue( "containedString", "containedString2" );
+				deeperContained = contained.addObject( "deeperContained" );
+				deeperContained.addValue( "id", "200" );
+				deeperContained.addValue( "deeperContainedString", "deeperContainedString2" );
+
+			}
+		}
+	}
 }