Backport HSEARCH-4565 + HSEARCH-4611 to branch 6.1 - Cyclic dependency detection for IndexingDependency(derivedFrom = ...) does not detect "buried" cycles

engine/src/main/java/org/hibernate/search/engine/logging/impl/Log.java

@@ -187,7 +187,10 @@ SearchException securityManagerLoadingError(@FormatWith(ClassFormatter.class) Cl
 	@Message(id = ID_OFFSET + 44, value = "Invalid type '%1$s': missing constructor. The type must expose a public constructor with a single parameter of type Map.")
 	SearchException noPublicMapArgConstructor(@FormatWith(ClassFormatter.class) Class<?> classToLoad);
 
-	@Message(id = ID_OFFSET + 46, value = "Infinite @IndexedEmbedded recursion involving path '%1$s' on type '%2$s'.")
+	@Message(id = ID_OFFSET + 46, value = "Cyclic @IndexedEmbedded recursion starting from type '%2$s'."
+			+ " Path starting from that type and ending with a cycle: '%1$s'."
+			+ " A type cannot declare an unrestricted @IndexedEmbedded to itself, even indirectly."
+			+ " To break the cycle, you should consider adding filters to your @IndexedEmbedded: includePaths, includeDepth, ...")
 	SearchException indexedEmbeddedCyclicRecursion(String cyclicRecursionPath,
 			@FormatWith(MappableTypeModelFormatter.class) MappableTypeModel parentTypeModel);
 

engine/src/main/java/org/hibernate/search/engine/reporting/spi/RootFailureCollector.java

@@ -219,11 +219,7 @@ void appendContextTo(StringJoiner joiner) {
 		synchronized void appendFailuresTo(ToStringTreeBuilder builder) {
 			builder.startObject( context.render() );
 			if ( !failureMessages.isEmpty() ) {
-				builder.startList( EngineEventContextMessages.INSTANCE.failureReportFailures() );
-				for ( String failureMessage : failureMessages ) {
-					builder.value( failureMessage );
-				}
-				builder.endList();
+				builder.attribute( EngineEventContextMessages.INSTANCE.failureReportFailures(), failureMessages );
 			}
 			appendChildrenFailuresTo( builder );
 			builder.endObject();

engine/src/test/java/org/hibernate/search/engine/mapper/mapping/building/impl/ConfiguredIndexSchemaManagerNestingContextTest.java

@@ -139,10 +139,8 @@ level1Definition, new IndexedEmbeddedPathTracker( level1Definition ),
 				);
 		} )
 				.isInstanceOf( SearchException.class )
-				.hasMessageContainingAll(
-						"Infinite @IndexedEmbedded recursion",
-						"path 'level1.prefix1_level1.prefix1_'",
-						"type '" + typeModel1Mock.toString() + "'"
+				.hasMessageContainingAll( "Cyclic @IndexedEmbedded recursion starting from type '" + typeModel1Mock.toString() + "'",
+						"Path starting from that type and ending with a cycle: 'level1.prefix1_level1.prefix1_'"
 				);
 		verifyNoOtherInteractionsAndReset();
 	}
@@ -172,10 +170,8 @@ level2Definition, new IndexedEmbeddedPathTracker( level2Definition ),
 			);
 		} )
 				.isInstanceOf( SearchException.class )
-				.hasMessageContainingAll(
-						"Infinite @IndexedEmbedded recursion",
-						"path 'level1.prefix1_level2.prefix2_level1.prefix1_'",
-						"type '" + typeModel1Mock.toString() + "'"
+				.hasMessageContainingAll( "Cyclic @IndexedEmbedded recursion starting from type '" + typeModel1Mock.toString() + "'",
+						"Path starting from that type and ending with a cycle: 'level1.prefix1_level2.prefix2_level1.prefix1_'"
 				);
 		verifyNoOtherInteractionsAndReset();
 	}

integrationtest/mapper/pojo-base/src/test/java/org/hibernate/search/integrationtest/mapper/pojo/mapping/definition/DependencyIT.java

@@ -683,14 +683,91 @@ public String getDerivedC() {
 				.satisfies( FailureReportUtils.hasFailureReport()
 						.typeContext( DerivedFromCycle.A.class.getName() )
 						.pathContext( ".derivedA<no value extractors>" )
-						.failure( "Unable to resolve dependencies of a derived property:"
-								+ " there is a cyclic dependency involving path '.derivedA<no value extractors>'"
-								+ " on type '" + DerivedFromCycle.A.class.getName() + "'",
+						.multilineFailure( "Unable to resolve dependencies of a derived property:"
+										+ " there is a cyclic dependency starting from type '" + DerivedFromCycle.A.class.getName() + "'",
+								"Derivation chain starting from that type and ending with a cycle:\n"
+										+ "- " + DerivedFromCycle.A.class.getName() + "#.b<default value extractors>.derivedB<default value extractors>\n"
+										+ "- " + DerivedFromCycle.B.class.getName() + "#.c<default value extractors>.derivedC<default value extractors>\n"
+										+ "- " + DerivedFromCycle.C.class.getName() + "#.a<default value extractors>.derivedA<default value extractors>\n",
 								"A derived property cannot be marked as derived from itself",
 								"you should consider disabling automatic reindexing"
 						) );
 	}
 
+	@Test
+	@TestForIssue(jiraKey = "HSEARCH-4565")
+	public void derivedFrom_error_cycle_buried() {
+		class DerivedFromCycle {
+			@Indexed
+			class Zero {
+				@DocumentId
+				Integer id;
+				A a;
+				@GenericField
+				@IndexingDependency(derivedFrom = @ObjectPath({
+						@PropertyValue(propertyName = "a"),
+						@PropertyValue(propertyName = "derivedA")
+				}))
+				public String getDerivedZero() {
+					throw new UnsupportedOperationException( "Should not be called" );
+				}
+			}
+			class A {
+				B b;
+				@GenericField
+				@IndexingDependency(derivedFrom = @ObjectPath({
+						@PropertyValue(propertyName = "b"),
+						@PropertyValue(propertyName = "derivedB")
+				}))
+				public String getDerivedA() {
+					throw new UnsupportedOperationException( "Should not be called" );
+				}
+			}
+			class B {
+				C c;
+				@GenericField
+				@IndexingDependency(derivedFrom = @ObjectPath({
+						@PropertyValue(propertyName = "c"),
+						@PropertyValue(propertyName = "derivedC")
+				}))
+				public String getDerivedB() {
+					throw new UnsupportedOperationException( "Should not be called" );
+				}
+			}
+			class C {
+				A a;
+				@GenericField
+				@IndexingDependency(derivedFrom = @ObjectPath({
+						@PropertyValue(propertyName = "a"),
+						@PropertyValue(propertyName = "derivedA")
+				}))
+				public String getDerivedC() {
+					throw new UnsupportedOperationException( "Should not be called" );
+				}
+			}
+		}
+		assertThatThrownBy(
+				() -> setupHelper.start()
+						.withAnnotatedEntityTypes( DerivedFromCycle.Zero.class )
+						.withAnnotatedTypes( DerivedFromCycle.A.class, DerivedFromCycle.B.class, DerivedFromCycle.C.class )
+						.setup()
+		)
+				.isInstanceOf( SearchException.class )
+				.satisfies( FailureReportUtils.hasFailureReport()
+						.typeContext( DerivedFromCycle.Zero.class.getName() )
+						.pathContext( ".derivedZero<no value extractors>" )
+						.multilineFailure( "Unable to resolve dependencies of a derived property:"
+										+ " there is a cyclic dependency starting from type '" + DerivedFromCycle.A.class.getName() + "'",
+								"Derivation chain starting from that type and ending with a cycle:\n"
+										+ "- " + DerivedFromCycle.A.class.getName() + "#.b<default value extractors>.derivedB<default value extractors>\n"
+										+ "- " + DerivedFromCycle.B.class.getName() + "#.c<default value extractors>.derivedC<default value extractors>\n"
+										+ "- " + DerivedFromCycle.C.class.getName() + "#.a<default value extractors>.derivedA<default value extractors>\n",
+								"A derived property cannot be marked as derived from itself",
+								"you should consider disabling automatic reindexing"
+						)
+				);
+	}
+
 	@Test
 	@TestForIssue(jiraKey = "HSEARCH-4423")
 	public void derivedFrom_cycleFalsePositive() {

integrationtest/mapper/pojo-base/src/test/java/org/hibernate/search/integrationtest/mapper/pojo/mapping/definition/IndexedEmbeddedBaseIT.java

@@ -1548,6 +1548,73 @@ class IndexedEntity {
 								+ " (@GenericField, @FullTextField, custom bridges, ...) is defined for that type." ) );
 	}
 
+	@Test
+	public void cycle() {
+		class Model {
+			@Indexed(index = INDEX_NAME)
+			class EntityA {
+				@DocumentId
+				Integer id;
+				@IndexedEmbedded
+				EntityB b;
+			}
+			class EntityB {
+				Integer id;
+				@IndexedEmbedded
+				EntityA a;
+			}
+		}
+
+		assertThatThrownBy( () -> setupHelper.start()
+				.withAnnotatedEntityTypes( Model.EntityA.class )
+				.setup() )
+				.isInstanceOf( SearchException.class )
+				.satisfies( FailureReportUtils.hasFailureReport()
+						.typeContext( Model.EntityA.class.getName() )
+						.pathContext( ".b<no value extractors>.a<no value extractors>.b" )
+						.failure( "Cyclic @IndexedEmbedded recursion starting from type '" + Model.EntityA.class.getName() + "'",
+								"Path starting from that type and ending with a cycle: 'b.a.b.'",
+								"A type cannot declare an unrestricted @IndexedEmbedded to itself, even indirectly",
+								"To break the cycle, you should consider adding filters to your @IndexedEmbedded: includePaths, includeDepth, ..." )
+				);
+	}
+
+	@Test
+	public void cycle_nonRoot() {
+		class Model {
+			@Indexed(index = INDEX_NAME)
+			class EntityA {
+				@DocumentId
+				Integer id;
+				@IndexedEmbedded
+				EntityB b;
+			}
+			class EntityB {
+				Integer id;
+				@IndexedEmbedded
+				EntityC c;
+			}
+			class EntityC {
+				Integer id;
+				@IndexedEmbedded
+				EntityB b;
+			}
+		}
+
+		assertThatThrownBy( () -> setupHelper.start()
+				.withAnnotatedEntityTypes( Model.EntityA.class )
+				.setup() )
+				.isInstanceOf( SearchException.class )
+				.satisfies( FailureReportUtils.hasFailureReport()
+						.typeContext( Model.EntityA.class.getName() )
+						.pathContext( ".b<no value extractors>.c<no value extractors>.b<no value extractors>.c" )
+						.failure( "Cyclic @IndexedEmbedded recursion starting from type '" + Model.EntityB.class.getName() + "'",
+								"Path starting from that type and ending with a cycle: 'c.b.c.'",
+								"A type cannot declare an unrestricted @IndexedEmbedded to itself, even indirectly",
+								"To break the cycle, you should consider adding filters to your @IndexedEmbedded: includePaths, includeDepth, ..." )
+				);
+	}
+
 	private <E> void doTestEmbeddedRuntime(SearchMapping mapping,
 			Function<Integer, E> newEntityFunction,
 			Consumer<StubDocumentNode.Builder> expectedDocumentContributor) {

mapper/pojo-base/src/main/java/org/hibernate/search/mapper/pojo/automaticindexing/building/impl/AbstractPojoIndexingDependencyCollectorDirectValueNode.java

@@ -25,6 +25,7 @@
 import org.hibernate.search.mapper.pojo.model.spi.PojoRawTypeModel;
 import org.hibernate.search.mapper.pojo.model.spi.PojoTypeModel;
 import org.hibernate.search.util.common.AssertionFailure;
+import org.hibernate.search.util.common.data.impl.LinkedNode;
 import org.hibernate.search.util.common.logging.impl.LoggerFactory;
 
 /**
@@ -92,8 +93,7 @@ public <U> PojoIndexingDependencyCollectorTypeNode<? extends U> castedType(PojoR
 
 	public abstract void collectDependency();
 
-	abstract void doCollectDependency(
-			PojoIndexingDependencyCollectorMonomorphicDirectValueNode<?, ?> initialNodeCollectingDependency);
+	abstract void doCollectDependency(LinkedNode<DerivedDependencyWalkingInfo> derivedDependencyPath);
 
 	@Override
 	final PojoIndexingDependencyCollectorTypeNode<?> lastEntityNode() {

mapper/pojo-base/src/main/java/org/hibernate/search/mapper/pojo/automaticindexing/building/impl/DerivedDependencyWalkingInfo.java

@@ -0,0 +1,28 @@
+/*
+ * 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 <http://www.gnu.org/licenses/lgpl-2.1.html>.
+ */
+package org.hibernate.search.mapper.pojo.automaticindexing.building.impl;
+
+import org.hibernate.search.mapper.pojo.model.path.PojoModelPath;
+import org.hibernate.search.mapper.pojo.model.spi.PojoRawTypeModel;
+
+public final class DerivedDependencyWalkingInfo {
+	public final PojoIndexingDependencyCollectorMonomorphicDirectValueNode<?, ?> node;
+	public final PojoRawTypeModel<?> definingTypeModel;
+	public final PojoModelPath derivedFromPath;
+
+	public DerivedDependencyWalkingInfo(PojoIndexingDependencyCollectorMonomorphicDirectValueNode<?, ?> node,
+			PojoModelPath derivedFromPath) {
+		this.node = node;
+		this.definingTypeModel = node.parentNode.parentNode().typeModel().rawType();
+		this.derivedFromPath = derivedFromPath;
+	}
+
+	@Override
+	public String toString() {
+		return definingTypeModel.name() + "#" + derivedFromPath.toPathString();
+	}
+}