GH-2574 - Ensure consistent behaviour of org.springframework.data.neo4j.core.mapping.NodeDescription#getChildNodeDescriptionsInHierarchy.

This fixes #2574.

Author: michael-simons

src/main/java/org/springframework/data/neo4j/core/mapping/DefaultNeo4jPersistentEntity.java

@@ -513,13 +513,13 @@ public Collection<GraphPropertyDescription> getGraphPropertiesInHierarchy() {
 	public void addChildNodeDescription(NodeDescription<?> child) {
 		this.childNodeDescriptions.add(child);
 		updateChildNodeDescriptionCache();
-		if (this.parentNodeDescription != null) {
-			((DefaultNeo4jPersistentEntity<?>) this.parentNodeDescription).updateChildNodeDescriptionCache();
-		}
 	}
 
 	private void updateChildNodeDescriptionCache() {
 		this.childNodeDescriptionsInHierarchy = computeChildNodeDescriptionInHierarchy();
+		if (this.parentNodeDescription != null) {
+			((DefaultNeo4jPersistentEntity<?>) this.parentNodeDescription).updateChildNodeDescriptionCache();
+		}
 	}
 
 	@Override

src/test/java/org/springframework/data/neo4j/core/mapping/Neo4jMappingContextTest.java

@@ -25,13 +25,18 @@
 import java.util.Collections;
 import java.util.Comparator;
 import java.util.HashSet;
+import java.util.IdentityHashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Set;
+import java.util.TreeSet;
 import java.util.UUID;
+import java.util.stream.Collectors;
 
 import org.assertj.core.api.Assertions;
 import org.junit.jupiter.api.Nested;
+import org.junit.jupiter.api.RepeatedTest;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
 import org.junit.jupiter.params.ParameterizedTest;
@@ -54,6 +59,7 @@
 import org.springframework.data.neo4j.core.mapping.datagraph1446.R2;
 import org.springframework.data.neo4j.core.mapping.datagraph1448.A_S3;
 import org.springframework.data.neo4j.core.mapping.datagraph1448.RelatedThing;
+import org.springframework.data.neo4j.core.mapping.gh2574.Model;
 import org.springframework.data.neo4j.core.schema.CompositeProperty;
 import org.springframework.data.neo4j.core.schema.GeneratedValue;
 import org.springframework.data.neo4j.core.schema.Id;
@@ -532,6 +538,60 @@ void dontFailOnCyclicRelationshipProperties() { // GH-2398
 		assertThat(persistentEntity.getGraphProperties()).hasSize(3);
 	}
 
+	private static Set<Class<?>> scanAndShuffle(String basePackage) throws ClassNotFoundException {
+
+		Comparator<Class<?>> pseudoRandomComparator = new Comparator<Class<?>>() {
+			private final Map<Object, UUID> uniqueIds = new IdentityHashMap<>();
+
+			@Override
+			public int compare(Class<?> o1, Class<?> o2) {
+				UUID e1 = uniqueIds.computeIfAbsent(o1, k -> UUID.randomUUID());
+				UUID e2 = uniqueIds.computeIfAbsent(o2, k -> UUID.randomUUID());
+				return e1.compareTo(e2);
+			}
+		};
+
+		Set<Class<?>> scanResult = Neo4jEntityScanner.get().scan(basePackage);
+		Set<Class<?>> initialEntities = new TreeSet<>(pseudoRandomComparator);
+		initialEntities.addAll(scanResult);
+		return initialEntities;
+	}
+
+
+	@RepeatedTest(10) // GH-2574
+	void hierarchyMustBeConsistentlyReportedWithIntermediateConcreteClasses() throws ClassNotFoundException {
+
+		Neo4jMappingContext neo4jMappingContext = new Neo4jMappingContext();
+		neo4jMappingContext.setStrict(true);
+		neo4jMappingContext.setInitialEntitySet(scanAndShuffle("org.springframework.data.neo4j.core.mapping.gh2574"));
+		neo4jMappingContext.initialize();
+
+		Neo4jPersistentEntity<?> b1 = Objects.requireNonNull(neo4jMappingContext.getPersistentEntity(Model.B1.class));
+		List<String> children = b1.getChildNodeDescriptionsInHierarchy()
+				.stream().map(NodeDescription::getPrimaryLabel)
+				.sorted()
+				.collect(Collectors.toList());
+
+		assertThat(children).containsExactly("B2", "B2a", "B3", "B3a");
+	}
+
+	@Test // GH-2574
+	void hierarchyMustBeConsistentlyReported() throws ClassNotFoundException {
+
+		Neo4jMappingContext neo4jMappingContext = new Neo4jMappingContext();
+		neo4jMappingContext.setStrict(true);
+		neo4jMappingContext.setInitialEntitySet(scanAndShuffle("org.springframework.data.neo4j.core.mapping.gh2574"));
+		neo4jMappingContext.initialize();
+
+		Neo4jPersistentEntity<?> a1 = Objects.requireNonNull(neo4jMappingContext.getPersistentEntity(Model.A1.class));
+		List<String> children = a1.getChildNodeDescriptionsInHierarchy()
+				.stream().map(NodeDescription::getPrimaryLabel)
+				.sorted()
+				.collect(Collectors.toList());
+
+		assertThat(children).containsExactly("A2", "A3", "A4");
+	}
+
 	static class DummyIdGenerator implements IdGenerator<Void> {
 
 		@Override

src/test/java/org/springframework/data/neo4j/core/mapping/gh2574/Model.java

@@ -0,0 +1,97 @@
+/*
+ * Copyright 2011-2022 the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.springframework.data.neo4j.core.mapping.gh2574;
+
+import org.springframework.data.neo4j.core.schema.Id;
+import org.springframework.data.neo4j.core.schema.Node;
+
+/**
+ * A hierachical model with and without intermediate abstract classes.
+ *
+ * @author Michael J. Simons
+ */
+public abstract class Model {
+
+	/**
+	 * Shut up checkstyle.
+	 */
+	@Node
+	public abstract static class A1 {
+		@Id
+		String id;
+	}
+
+	/**
+	 * Shut up checkstyle.
+	 */
+	@Node
+	public abstract static class A2 extends A1 {
+	}
+
+	/**
+	 * Shut up checkstyle.
+	 */
+	@Node
+	public abstract static class A3 extends A2 {
+	}
+
+	/**
+	 * Shut up checkstyle.
+	 */
+	@Node
+	public static class A4 extends A3 {
+	}
+
+	/**
+	 * Shut up checkstyle.
+	 */
+	@Node
+	public abstract static class B1 {
+		@Id
+		String id;
+	}
+
+	/**
+	 * Shut up checkstyle.
+	 */
+	@Node
+	public abstract static class B2 extends B1 {
+	}
+
+	/**
+	 * Shut up checkstyle.
+	 */
+	@Node
+	public static class B2a extends B2 {
+	}
+
+	/**
+	 * Shut up checkstyle.
+	 */
+	@Node
+	public abstract static class B3 extends B2 {
+	}
+
+	/**
+	 * Shut up checkstyle.
+	 */
+	@Node
+	public static class B3a extends B3 {
+	}
+
+	private Model() {
+	}
+}