Skip to content

Commit 5d1241e

Browse files
michael-simonsmichael-simons
authored
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.
1 parent ec5cb85 commit 5d1241e

File tree

5 files changed

+219
-1
lines changed

5 files changed

+219
-1
lines changed

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

+4
Original file line numberDiff line numberDiff line change
@@ -375,6 +375,10 @@ public <T, R> R saveAs(T instance, Class<R> resultType) {
375375
projectionFactory, neo4jMappingContext);
376376

377377
T savedInstance = saveImpl(instance, pps, null);
378+
if (!resultType.isInterface()) {
379+
@SuppressWarnings("unchecked") R result = (R) new DtoInstantiatingConverter(resultType, neo4jMappingContext).convertDirectly(savedInstance);
380+
return result;
381+
}
378382
if (projectionInformation.isClosed()) {
379383
return projectionFactory.createProjection(resultType, savedInstance);
380384
}

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

+8
Original file line numberDiff line numberDiff line change
@@ -360,6 +360,14 @@ public <T, R> Mono<R> saveAs(T instance, Class<R> resultType) {
360360
projectionFactory, neo4jMappingContext);
361361

362362
Mono<T> savingPublisher = saveImpl(instance, pps, null);
363+
364+
if (!resultType.isInterface()) {
365+
return savingPublisher.map(savedInstance -> {
366+
@SuppressWarnings("unchecked")
367+
R result = (R) (new DtoInstantiatingConverter(resultType, neo4jMappingContext).convertDirectly(savedInstance));
368+
return result;
369+
});
370+
}
363371
if (projectionInformation.isClosed()) {
364372
return savingPublisher.map(savedInstance -> projectionFactory.createProjection(resultType, savedInstance));
365373
}

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

+6-1
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,12 @@ Object getPropertyValueDirectlyFor(PersistentProperty<?> targetProperty, Persist
109109
if (sourceProperty == null) {
110110
return null;
111111
}
112-
return sourceAccessor.getProperty(sourceProperty);
112+
113+
Object result = sourceAccessor.getProperty(sourceProperty);
114+
if (targetProperty.isEntity() && !targetProperty.getTypeInformation().isAssignableFrom(sourceProperty.getTypeInformation())) {
115+
return new DtoInstantiatingConverter(targetProperty.getType(), this.context).convertDirectly(result);
116+
}
117+
return result;
113118
}
114119

115120
@Override

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

+58
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@
4949
import org.springframework.data.domain.Sort;
5050
import org.springframework.data.neo4j.core.Neo4jOperations;
5151
import org.springframework.data.neo4j.integration.shared.common.DoritoEatingPerson;
52+
import org.springframework.data.neo4j.integration.shared.common.GH2621Domain;
5253
import org.springframework.data.neo4j.test.Neo4jImperativeTestConfiguration;
5354
import org.springframework.data.neo4j.core.DatabaseSelectionProvider;
5455
import org.springframework.data.neo4j.core.Neo4jTemplate;
@@ -79,6 +80,7 @@
7980
import org.springframework.data.repository.query.Param;
8081
import org.springframework.transaction.PlatformTransactionManager;
8182
import org.springframework.transaction.annotation.EnableTransactionManagement;
83+
import org.springframework.transaction.support.TransactionTemplate;
8284

8385
/**
8486
* @author Gerrit Meier
@@ -459,6 +461,57 @@ public void projectionRespected(@Autowired Neo4jOperations neo4jOperations) {
459461
assertThat(saved).hasValueSatisfying(it -> assertThat(it.getFriends()).isEmpty());
460462
}
461463

464+
@Test // GH-2621
465+
public void nestedProjectWithFluentOpsShouldWork(@Autowired TransactionTemplate transactionTemplate, @Autowired Neo4jTemplate neo4jTemplate) {
466+
467+
GH2621Domain.FooProjection fooProjection = transactionTemplate.execute(tx -> {
468+
final GH2621Domain.BarBarProjection barBarProjection = new GH2621Domain.BarBarProjection("v1", "v2");
469+
return neo4jTemplate.save(GH2621Domain.Foo.class).one(new GH2621Domain.FooProjection(barBarProjection));
470+
});
471+
472+
assertThat(fooProjection.getBar()).isNotNull();
473+
assertThat(fooProjection.getBar().getValue1()).isEqualTo("v1");
474+
// There is no way to deduce from a `BarProjection` field the correlation from `BarBarProjection to `BarBar`
475+
// without throwing a dice and we are not going to try this
476+
assertThat(fooProjection.getBar()).isInstanceOf(GH2621Domain.BarProjection.class);
477+
478+
// The result above is reflected in the graph
479+
try (Session session = driver.session(bookmarkCapture.createSessionConfig())) {
480+
Record result = session.run("MATCH (n:GH2621Bar) RETURN n").single();
481+
assertThat(result.get("n").asNode().get("value1").asString()).isEqualTo("v1");
482+
}
483+
}
484+
485+
@Test // GH-2621
486+
public void nestedProjectWithFluentOpsShouldWork2(@Autowired TransactionTemplate transactionTemplate, @Autowired Neo4jTemplate neo4jTemplate) {
487+
488+
GH2621Domain.FooProjection fooProjection = transactionTemplate.execute(tx -> {
489+
GH2621Domain.Foo foo = new GH2621Domain.Foo(new GH2621Domain.BarBar("v1", "v2"));
490+
return neo4jTemplate.saveAs(foo, GH2621Domain.FooProjection.class);
491+
});
492+
493+
assertThat(fooProjection.getBar()).isNotNull();
494+
assertThat(fooProjection.getBar().getValue1()).isEqualTo("v1");
495+
// There is no way to deduce from a `BarProjection` field the correlation from `BarBarProjection to `BarBar`
496+
// without throwing a dice and we are not going to try this
497+
assertThat(fooProjection.getBar()).isInstanceOf(GH2621Domain.BarProjection.class);
498+
499+
// This is a different here as the concrete dto was used during save ops, so the
500+
try (Session session = driver.session(bookmarkCapture.createSessionConfig())) {
501+
Record result = session.run("MATCH (n:GH2621Bar:GH2621BarBar) RETURN n").single();
502+
org.neo4j.driver.types.Node node = result.get("n").asNode();
503+
assertThat(node.get("value1").asString()).isEqualTo("v1");
504+
// This is a limitation of the Spring Data Commons support for the DTO projections
505+
// when we reach org/springframework/data/neo4j/core/PropertyFilterSupport.java:141 we call
506+
// org.springframework.data.projection.ProjectionFactory.getProjectionInformation and we only
507+
// have the concrete type information at hand, in the domain example FooProjection#bar, which points
508+
// to BarProjection, without any clue that we do want a BarBarProjection being used during saving.
509+
// So with the example in the ticket, saving value2 (or anything on the BarBarProjection) won't
510+
// be possible
511+
assertThat(node.get("value2").isNull()).isTrue();
512+
}
513+
}
514+
462515
private static void projectedEntities(PersonDepartmentQueryResult personAndDepartment) {
463516
assertThat(personAndDepartment.getPerson()).extracting(PersonEntity::getId).isEqualTo("p1");
464517
assertThat(personAndDepartment.getPerson()).extracting(PersonEntity::getEmail).isEqualTo("[email protected]");
@@ -621,5 +674,10 @@ public PlatformTransactionManager transactionManager(Driver driver, DatabaseSele
621674
public boolean isCypher5Compatible() {
622675
return neo4jConnectionSupport.isCypher5SyntaxCompatible();
623676
}
677+
678+
@Bean
679+
TransactionTemplate transactionTemplate(PlatformTransactionManager transactionManager) {
680+
return new TransactionTemplate(transactionManager);
681+
}
624682
}
625683
}

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

+143
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,143 @@
1+
/*
2+
* Copyright 2011-2023 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package org.springframework.data.neo4j.integration.shared.common;
17+
18+
import java.util.UUID;
19+
20+
import org.springframework.data.neo4j.core.schema.GeneratedValue;
21+
import org.springframework.data.neo4j.core.schema.Id;
22+
import org.springframework.data.neo4j.core.schema.Node;
23+
24+
/**
25+
* Container for a bunch of domain classes.
26+
*/
27+
public final class GH2621Domain {
28+
29+
/**
30+
* A node.
31+
*/
32+
@Node("GH2621Foo")
33+
public static class Foo {
34+
@Id
35+
@GeneratedValue
36+
private UUID id;
37+
38+
private final Bar bar;
39+
40+
public Foo(Bar bar) {
41+
this.bar = bar;
42+
}
43+
44+
public UUID getId() {
45+
return id;
46+
}
47+
48+
public Bar getBar() {
49+
return bar;
50+
}
51+
}
52+
53+
/**
54+
* A node.
55+
*/
56+
@Node("GH2621Bar")
57+
public static class Bar {
58+
@Id
59+
@GeneratedValue
60+
private UUID id;
61+
62+
private final String value1;
63+
64+
public Bar(String value1) {
65+
this.value1 = value1;
66+
}
67+
68+
public UUID getId() {
69+
return id;
70+
}
71+
72+
public String getValue1() {
73+
return value1;
74+
}
75+
}
76+
77+
/**
78+
* A node.
79+
*/
80+
@Node("GH2621BarBar")
81+
public static class BarBar extends Bar {
82+
private final String value2;
83+
84+
public BarBar(String value1, String value2) {
85+
super(value1);
86+
this.value2 = value2;
87+
}
88+
89+
public String getValue2() {
90+
return value2;
91+
}
92+
}
93+
94+
/**
95+
* Projects {@link Foo}
96+
*/
97+
public static class FooProjection {
98+
private final BarProjection bar;
99+
100+
public FooProjection(BarProjection bar) {
101+
this.bar = bar;
102+
}
103+
104+
public BarProjection getBar() {
105+
return bar;
106+
}
107+
}
108+
109+
/**
110+
* Projects {@link Bar} and {@link BarBar}
111+
*/
112+
public static class BarProjection {
113+
private final String value1;
114+
115+
public BarProjection(String value1) {
116+
this.value1 = value1;
117+
}
118+
119+
public String getValue1() {
120+
return value1;
121+
}
122+
}
123+
124+
/**
125+
* Projects {@link Bar} and {@link BarBar}
126+
*/
127+
public static class BarBarProjection extends BarProjection {
128+
private final String value2;
129+
130+
public BarBarProjection(String value1, String value2) {
131+
super(value1);
132+
this.value2 = value2;
133+
}
134+
135+
public String getValue2() {
136+
return value2;
137+
}
138+
}
139+
140+
141+
private GH2621Domain() {
142+
}
143+
}

0 commit comments

Comments
 (0)