GH-2621: Recursively apply DtoInstantiating converter for fluent ops. (#2667)

This allows to use Dto based projections to be used in a nested fashion when using the fluent save operations.

This does not solve - or attempt to solve - the issue that we don’t have any information what concrete projection class to be used when projection in all cases.

In the test `nestedProjectWithFluentOpsShouldWork` we could argue that the concrete value can be used to determine the type information. However, while that can be made to work with heterogeneous collections, it would bring a big performance penalty, getting new persistent entity instances and property accessors all the time.

Thus, this change now makes sure no exception is thrown when using such inherited projections, but does explicitly not bring support for deriving the projected properties from them accross an inheritance hierachy.

For more information, please read the comments in org.springframework.data.neo4j.integration.imperative.ProjectionIT#nestedProjectWithFluentOpsShouldWork and org.springframework.data.neo4j.integration.imperative.ProjectionIT#nestedProjectWithFluentOpsShouldWork2.

This closes #2621.

Author: michael-simons

src/main/java/org/springframework/data/neo4j/core/Neo4jTemplate.java

@@ -347,6 +347,10 @@ public <T, R> R saveAs(T instance, Class<R> resultType) {
 				projectionFactory, neo4jMappingContext);
 
 		T savedInstance = saveImpl(instance, pps, null);
+		if (!resultType.isInterface()) {
+			@SuppressWarnings("unchecked") R result = (R) new DtoInstantiatingConverter(resultType, neo4jMappingContext).convertDirectly(savedInstance);
+			return result;
+		}
 		if (projectionInformation.isClosed()) {
 			return projectionFactory.createProjection(resultType, savedInstance);
 		}

src/main/java/org/springframework/data/neo4j/core/ReactiveNeo4jTemplate.java

@@ -346,6 +346,14 @@ public <T, R> Mono<R> saveAs(T instance, Class<R> resultType) {
 				projectionFactory, neo4jMappingContext);
 
 		Mono<T> savingPublisher = saveImpl(instance, pps, null);
+
+		if (!resultType.isInterface()) {
+			return savingPublisher.map(savedInstance -> {
+				@SuppressWarnings("unchecked")
+				R result = (R) (new DtoInstantiatingConverter(resultType, neo4jMappingContext).convertDirectly(savedInstance));
+				return result;
+			});
+		}
 		if (projectionInformation.isClosed()) {
 			return savingPublisher.map(savedInstance -> projectionFactory.createProjection(resultType, savedInstance));
 		}

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

@@ -109,7 +109,12 @@ Object getPropertyValueDirectlyFor(PersistentProperty<?> targetProperty, Persist
 		if (sourceProperty == null) {
 			return null;
 		}
-		return sourceAccessor.getProperty(sourceProperty);
+
+		Object result = sourceAccessor.getProperty(sourceProperty);
+		if (targetProperty.isEntity() && !targetProperty.getTypeInformation().isAssignableFrom(sourceProperty.getTypeInformation())) {
+			return new DtoInstantiatingConverter(targetProperty.getType(), this.context).convertDirectly(result);
+		}
+		return result;
 	}
 
 	@Override

src/test/java/org/springframework/data/neo4j/integration/imperative/ProjectionIT.java

@@ -49,6 +49,7 @@
 import org.springframework.data.domain.Sort;
 import org.springframework.data.neo4j.core.Neo4jOperations;
 import org.springframework.data.neo4j.integration.shared.common.DoritoEatingPerson;
+import org.springframework.data.neo4j.integration.shared.common.GH2621Domain;
 import org.springframework.data.neo4j.test.Neo4jImperativeTestConfiguration;
 import org.springframework.data.neo4j.core.DatabaseSelectionProvider;
 import org.springframework.data.neo4j.core.Neo4jTemplate;
@@ -79,6 +80,7 @@
 import org.springframework.data.repository.query.Param;
 import org.springframework.transaction.PlatformTransactionManager;
 import org.springframework.transaction.annotation.EnableTransactionManagement;
+import org.springframework.transaction.support.TransactionTemplate;
 
 /**
  * @author Gerrit Meier
@@ -459,6 +461,57 @@ public void projectionRespected(@Autowired Neo4jOperations neo4jOperations) {
 		assertThat(saved).hasValueSatisfying(it -> assertThat(it.getFriends()).isEmpty());
 	}
 
+	@Test // GH-2621
+	public void nestedProjectWithFluentOpsShouldWork(@Autowired TransactionTemplate transactionTemplate, @Autowired Neo4jTemplate neo4jTemplate) {
+
+		GH2621Domain.FooProjection fooProjection = transactionTemplate.execute(tx -> {
+			final GH2621Domain.BarBarProjection barBarProjection = new GH2621Domain.BarBarProjection("v1", "v2");
+			return neo4jTemplate.save(GH2621Domain.Foo.class).one(new GH2621Domain.FooProjection(barBarProjection));
+		});
+
+		assertThat(fooProjection.getBar()).isNotNull();
+		assertThat(fooProjection.getBar().getValue1()).isEqualTo("v1");
+		// There is no way to deduce from a `BarProjection` field the correlation from `BarBarProjection to `BarBar`
+		// without throwing a dice and we are not going to try this
+		assertThat(fooProjection.getBar()).isInstanceOf(GH2621Domain.BarProjection.class);
+
+		// The result above is reflected in the graph
+		try (Session session = driver.session(bookmarkCapture.createSessionConfig())) {
+			Record result = session.run("MATCH (n:GH2621Bar) RETURN n").single();
+			assertThat(result.get("n").asNode().get("value1").asString()).isEqualTo("v1");
+		}
+	}
+
+	@Test // GH-2621
+	public void nestedProjectWithFluentOpsShouldWork2(@Autowired TransactionTemplate transactionTemplate, @Autowired Neo4jTemplate neo4jTemplate) {
+
+		GH2621Domain.FooProjection fooProjection = transactionTemplate.execute(tx -> {
+			GH2621Domain.Foo foo = new GH2621Domain.Foo(new GH2621Domain.BarBar("v1", "v2"));
+			return neo4jTemplate.saveAs(foo, GH2621Domain.FooProjection.class);
+		});
+
+		assertThat(fooProjection.getBar()).isNotNull();
+		assertThat(fooProjection.getBar().getValue1()).isEqualTo("v1");
+		// There is no way to deduce from a `BarProjection` field the correlation from `BarBarProjection to `BarBar`
+		// without throwing a dice and we are not going to try this
+		assertThat(fooProjection.getBar()).isInstanceOf(GH2621Domain.BarProjection.class);
+
+		// This is a different here as the concrete dto was used during save ops, so the
+		try (Session session = driver.session(bookmarkCapture.createSessionConfig())) {
+			Record result = session.run("MATCH (n:GH2621Bar:GH2621BarBar) RETURN n").single();
+			org.neo4j.driver.types.Node node = result.get("n").asNode();
+			assertThat(node.get("value1").asString()).isEqualTo("v1");
+			// This is a limitation of the Spring Data Commons support for the DTO projections
+			// when we reach org/springframework/data/neo4j/core/PropertyFilterSupport.java:141 we call
+			// org.springframework.data.projection.ProjectionFactory.getProjectionInformation and we only
+			// have the concrete type information at hand, in the domain example FooProjection#bar, which points
+			// to BarProjection, without any clue that we do want a BarBarProjection being used during saving.
+			// So with the example in the ticket, saving value2 (or anything on the BarBarProjection) won't
+			// be possible
+			assertThat(node.get("value2").isNull()).isTrue();
+		}
+	}
+
 	private static void projectedEntities(PersonDepartmentQueryResult personAndDepartment) {
 		assertThat(personAndDepartment.getPerson()).extracting(PersonEntity::getId).isEqualTo("p1");
 		assertThat(personAndDepartment.getPerson()).extracting(PersonEntity::getEmail).isEqualTo("p1@dep1.org");
@@ -621,5 +674,10 @@ public PlatformTransactionManager transactionManager(Driver driver, DatabaseSele
 		public boolean isCypher5Compatible() {
 			return neo4jConnectionSupport.isCypher5SyntaxCompatible();
 		}
+
+		@Bean
+		TransactionTemplate transactionTemplate(PlatformTransactionManager transactionManager) {
+			return new TransactionTemplate(transactionManager);
+		}
 	}
 }

src/test/java/org/springframework/data/neo4j/integration/shared/common/GH2621Domain.java

@@ -0,0 +1,143 @@
+/*
+ * Copyright 2011-2023 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.integration.shared.common;
+
+import java.util.UUID;
+
+import org.springframework.data.neo4j.core.schema.GeneratedValue;
+import org.springframework.data.neo4j.core.schema.Id;
+import org.springframework.data.neo4j.core.schema.Node;
+
+/**
+ * Container for a bunch of domain classes.
+ */
+public final class GH2621Domain {
+
+	/**
+	 * A node.
+	 */
+	@Node("GH2621Foo")
+	public static class Foo {
+		@Id
+		@GeneratedValue
+		private UUID id;
+
+		private final Bar bar;
+
+		public Foo(Bar bar) {
+			this.bar = bar;
+		}
+
+		public UUID getId() {
+			return id;
+		}
+
+		public Bar getBar() {
+			return bar;
+		}
+	}
+
+	/**
+	 * A node.
+	 */
+	@Node("GH2621Bar")
+	public static class Bar {
+		@Id
+		@GeneratedValue
+		private UUID id;
+
+		private final String value1;
+
+		public Bar(String value1) {
+			this.value1 = value1;
+		}
+
+		public UUID getId() {
+			return id;
+		}
+
+		public String getValue1() {
+			return value1;
+		}
+	}
+
+	/**
+	 * A node.
+	 */
+	@Node("GH2621BarBar")
+	public static class BarBar extends Bar {
+		private final String value2;
+
+		public BarBar(String value1, String value2) {
+			super(value1);
+			this.value2 = value2;
+		}
+
+		public String getValue2() {
+			return value2;
+		}
+	}
+
+	/**
+	 * Projects {@link Foo}
+	 */
+	public static class FooProjection {
+		private final BarProjection bar;
+
+		public FooProjection(BarProjection bar) {
+			this.bar = bar;
+		}
+
+		public BarProjection getBar() {
+			return bar;
+		}
+	}
+
+	/**
+	 * Projects {@link Bar} and {@link BarBar}
+	 */
+	public static class BarProjection {
+		private final String value1;
+
+		public BarProjection(String value1) {
+			this.value1 = value1;
+		}
+
+		public String getValue1() {
+			return value1;
+		}
+	}
+
+	/**
+	 * Projects {@link Bar} and {@link BarBar}
+	 */
+	public static class BarBarProjection extends BarProjection {
+		private final String value2;
+
+		public BarBarProjection(String value1, String value2) {
+			super(value1);
+			this.value2 = value2;
+		}
+
+		public String getValue2() {
+			return value2;
+		}
+	}
+
+
+	private GH2621Domain() {
+	}
+}