Skip to content

Commit d890a65

Browse files
committed
GH-2728 - Check current Cypher-DSL dialect before issuing queries.
Make sure we ignore invalid dialect config (i.e. we actually have neo4j 5, but are on 4 dialect in a crucial mapping path.
1 parent 4dbbb0b commit d890a65

14 files changed

+557
-4
lines changed

pom.xml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919
<parent>
2020
<groupId>org.springframework.data.build</groupId>
2121
<artifactId>spring-data-parent</artifactId>
22-
<version>3.1.1-SNAPSHOT</version>
22+
<version>3.1.0</version>
2323
</parent>
2424

2525
<groupId>org.springframework.data</groupId>
@@ -115,7 +115,7 @@
115115
<skipUnitTests>${skipTests}</skipUnitTests>
116116

117117
<spring-data-commons-docs.dir>../../../../spring-data-commons/src/main/asciidoc</spring-data-commons-docs.dir>
118-
<springdata.commons>3.1.1-SNAPSHOT</springdata.commons>
118+
<springdata.commons>3.1.0</springdata.commons>
119119
</properties>
120120

121121
<dependencyManagement>

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -814,7 +814,7 @@ private <T> T processNestedRelations(
814814
relatedInternalId = stateMachine.getObjectId(relatedValueToStore);
815815
} else {
816816
savedEntity = saveRelatedNode(newRelatedObject, targetEntity, includeProperty, currentPropertyPath);
817-
relatedInternalId = IdentitySupport.getElementId(savedEntity);
817+
relatedInternalId = TemplateSupport.rendererCanUseElementIdIfPresent(renderer) ? savedEntity.elementId() : Long.toString(savedEntity.id());
818818
stateMachine.markEntityAsProcessed(relatedValueToStore, relatedInternalId);
819819
if (relatedValueToStore instanceof MappingSupport.RelationshipPropertiesWithEntityHolder) {
820820
Object entity = ((MappingSupport.RelationshipPropertiesWithEntityHolder) relatedValueToStore).getRelatedEntity();

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -952,7 +952,7 @@ private <T> Mono<T> processNestedRelations(Neo4jPersistentEntity<?> sourceEntity
952952
queryOrSave = Mono.just(Tuples.of(relatedInternalId, new AtomicReference<>()));
953953
} else {
954954
queryOrSave = saveRelatedNode(newRelatedObject, targetEntity, includeProperty, currentPropertyPath)
955-
.map(entity -> Tuples.of(new AtomicReference<>(IdentitySupport.getElementId(entity)), new AtomicReference<>(entity)))
955+
.map(entity -> Tuples.of(new AtomicReference<>(TemplateSupport.rendererCanUseElementIdIfPresent(renderer) ? entity.elementId() : Long.toString(entity.id())), new AtomicReference<>(entity)))
956956
.doOnNext(t -> {
957957
var relatedInternalId = t.getT1().get();
958958
stateMachine.markEntityAsProcessed(relatedValueToStore, relatedInternalId);

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
import org.neo4j.cypherdsl.core.Node;
4040
import org.neo4j.cypherdsl.core.Relationship;
4141
import org.neo4j.cypherdsl.core.Statement;
42+
import org.neo4j.cypherdsl.core.renderer.Renderer;
4243
import org.neo4j.driver.types.Entity;
4344
import org.neo4j.driver.types.MapAccessor;
4445
import org.neo4j.driver.types.TypeSystem;
@@ -410,6 +411,15 @@ static <T> String retrieveOrSetRelatedId(
410411
return Objects.requireNonNull(relatedInternalId);
411412
}
412413

414+
/**
415+
* Checks if the renderer is configured in such a way that it will use element id or apply toString(id(n)) workaround.
416+
* @return {@literal true} if renderer will use elementId
417+
*/
418+
static boolean rendererCanUseElementIdIfPresent(Renderer renderer) {
419+
return renderer.render(Cypher.returning(Functions.elementId(Cypher.anyNode("n"))).build())
420+
.equals("RETURN elementId(n)");
421+
}
422+
413423
private TemplateSupport() {
414424
}
415425
}
Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
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.issues.gh2728;
17+
18+
import org.junit.jupiter.api.Assertions;
19+
import org.junit.jupiter.api.Test;
20+
import org.neo4j.driver.Driver;
21+
import org.springframework.beans.factory.annotation.Autowired;
22+
import org.springframework.context.annotation.Bean;
23+
import org.springframework.data.neo4j.core.ReactiveDatabaseSelectionProvider;
24+
import org.springframework.data.neo4j.core.transaction.Neo4jBookmarkManager;
25+
import org.springframework.data.neo4j.core.transaction.ReactiveNeo4jTransactionManager;
26+
import org.springframework.data.neo4j.repository.ReactiveNeo4jRepository;
27+
import org.springframework.data.neo4j.test.BookmarkCapture;
28+
import org.springframework.data.neo4j.test.Neo4jExtension;
29+
import org.springframework.data.neo4j.test.Neo4jIntegrationTest;
30+
import org.springframework.data.neo4j.test.Neo4jReactiveTestConfiguration;
31+
import org.springframework.transaction.ReactiveTransactionManager;
32+
33+
/**
34+
* @author Michael J. Simons
35+
*/
36+
@Neo4jIntegrationTest
37+
public abstract class AbstractReactiveTestBase {
38+
39+
protected static Neo4jExtension.Neo4jConnectionSupport neo4jConnectionSupport;
40+
41+
@Autowired
42+
private TestEntityWithGeneratedDeprecatedId1Repository generatedDeprecatedIdRepository;
43+
44+
@Autowired
45+
private TestEntityWithAssignedId1Repository assignedIdRepository;
46+
47+
@Test
48+
public void testGeneratedDeprecatedIds() {
49+
TestEntityWithGeneratedDeprecatedId2 t2 = new TestEntityWithGeneratedDeprecatedId2(null, "v2");
50+
TestEntityWithGeneratedDeprecatedId1 t1 = new TestEntityWithGeneratedDeprecatedId1(null, "v1", t2);
51+
52+
TestEntityWithGeneratedDeprecatedId1 result = generatedDeprecatedIdRepository.save(t1).block();
53+
54+
TestEntityWithGeneratedDeprecatedId1 freshRetrieved = generatedDeprecatedIdRepository.findById(result.getId()).block();
55+
56+
Assertions.assertNotNull(result.getRelatedEntity());
57+
Assertions.assertNotNull(freshRetrieved.getRelatedEntity());
58+
}
59+
60+
/**
61+
* This is a test to ensure if the fix for the failing test above will continue to work for
62+
* assigned ids. For broader test cases please return false for isCypher5Compatible in (Reactive)RepositoryIT
63+
*/
64+
@Test
65+
public void testAssignedIds() {
66+
TestEntityWithAssignedId2 t2 = new TestEntityWithAssignedId2("second", "v2");
67+
TestEntityWithAssignedId1 t1 = new TestEntityWithAssignedId1("first", "v1", t2);
68+
69+
TestEntityWithAssignedId1 result = assignedIdRepository.save(t1).block();
70+
71+
TestEntityWithAssignedId1 freshRetrieved = assignedIdRepository.findById(result.getAssignedId()).block();
72+
73+
Assertions.assertNotNull(result.getRelatedEntity());
74+
Assertions.assertNotNull(freshRetrieved.getRelatedEntity());
75+
}
76+
77+
interface TestEntityWithGeneratedDeprecatedId1Repository extends ReactiveNeo4jRepository<TestEntityWithGeneratedDeprecatedId1, Long> {
78+
}
79+
80+
interface TestEntityWithAssignedId1Repository extends ReactiveNeo4jRepository<TestEntityWithAssignedId1, String> {
81+
}
82+
83+
abstract static class Config extends Neo4jReactiveTestConfiguration {
84+
85+
@Bean
86+
public Driver driver() {
87+
88+
return neo4jConnectionSupport.getDriver();
89+
}
90+
91+
@Bean
92+
public BookmarkCapture bookmarkCapture() {
93+
return new BookmarkCapture();
94+
}
95+
96+
@Override
97+
public ReactiveTransactionManager reactiveTransactionManager(Driver driver, ReactiveDatabaseSelectionProvider databaseSelectionProvider) {
98+
99+
BookmarkCapture bookmarkCapture = bookmarkCapture();
100+
return new ReactiveNeo4jTransactionManager(driver, databaseSelectionProvider, Neo4jBookmarkManager.create(bookmarkCapture));
101+
}
102+
}
103+
}
Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,104 @@
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.issues.gh2728;
17+
18+
import org.junit.jupiter.api.Assertions;
19+
import org.junit.jupiter.api.Test;
20+
import org.neo4j.driver.Driver;
21+
import org.springframework.beans.factory.annotation.Autowired;
22+
import org.springframework.context.annotation.Bean;
23+
import org.springframework.data.neo4j.core.DatabaseSelectionProvider;
24+
import org.springframework.data.neo4j.core.transaction.Neo4jBookmarkManager;
25+
import org.springframework.data.neo4j.core.transaction.Neo4jTransactionManager;
26+
import org.springframework.data.neo4j.repository.Neo4jRepository;
27+
import org.springframework.data.neo4j.test.BookmarkCapture;
28+
import org.springframework.data.neo4j.test.Neo4jExtension;
29+
import org.springframework.data.neo4j.test.Neo4jImperativeTestConfiguration;
30+
import org.springframework.data.neo4j.test.Neo4jIntegrationTest;
31+
import org.springframework.transaction.PlatformTransactionManager;
32+
33+
/**
34+
* @author Gerrit Meier
35+
*/
36+
@Neo4jIntegrationTest
37+
public abstract class AbstractTestBase {
38+
39+
protected static Neo4jExtension.Neo4jConnectionSupport neo4jConnectionSupport;
40+
41+
@Autowired
42+
private TestEntityWithGeneratedDeprecatedId1Repository generatedDeprecatedIdRepository;
43+
44+
@Autowired
45+
private TestEntityWithAssignedId1Repository assignedIdRepository;
46+
47+
@Test
48+
public void testGeneratedDeprecatedIds() {
49+
TestEntityWithGeneratedDeprecatedId2 t2 = new TestEntityWithGeneratedDeprecatedId2(null, "v2");
50+
TestEntityWithGeneratedDeprecatedId1 t1 = new TestEntityWithGeneratedDeprecatedId1(null, "v1", t2);
51+
52+
TestEntityWithGeneratedDeprecatedId1 result = generatedDeprecatedIdRepository.save(t1);
53+
54+
TestEntityWithGeneratedDeprecatedId1 freshRetrieved = generatedDeprecatedIdRepository.findById(result.getId()).get();
55+
56+
Assertions.assertNotNull(result.getRelatedEntity());
57+
Assertions.assertNotNull(freshRetrieved.getRelatedEntity());
58+
}
59+
60+
/**
61+
* This is a test to ensure if the fix for the failing test above will continue to work for
62+
* assigned ids. For broader test cases please return false for isCypher5Compatible in (Reactive)RepositoryIT
63+
*/
64+
@Test
65+
public void testAssignedIds() {
66+
TestEntityWithAssignedId2 t2 = new TestEntityWithAssignedId2("second", "v2");
67+
TestEntityWithAssignedId1 t1 = new TestEntityWithAssignedId1("first", "v1", t2);
68+
69+
TestEntityWithAssignedId1 result = assignedIdRepository.save(t1);
70+
71+
TestEntityWithAssignedId1 freshRetrieved = assignedIdRepository.findById(result.getAssignedId()).get();
72+
73+
Assertions.assertNotNull(result.getRelatedEntity());
74+
Assertions.assertNotNull(freshRetrieved.getRelatedEntity());
75+
}
76+
77+
interface TestEntityWithGeneratedDeprecatedId1Repository extends Neo4jRepository<TestEntityWithGeneratedDeprecatedId1, Long> {
78+
}
79+
80+
interface TestEntityWithAssignedId1Repository extends Neo4jRepository<TestEntityWithAssignedId1, String> {
81+
}
82+
83+
abstract static class Config extends Neo4jImperativeTestConfiguration {
84+
85+
@Bean
86+
public Driver driver() {
87+
88+
return neo4jConnectionSupport.getDriver();
89+
}
90+
91+
@Bean
92+
public BookmarkCapture bookmarkCapture() {
93+
return new BookmarkCapture();
94+
}
95+
96+
@Override
97+
public PlatformTransactionManager transactionManager(Driver driver,
98+
DatabaseSelectionProvider databaseNameProvider) {
99+
BookmarkCapture bookmarkCapture = bookmarkCapture();
100+
return new Neo4jTransactionManager(driver, databaseNameProvider,
101+
Neo4jBookmarkManager.create(bookmarkCapture));
102+
}
103+
}
104+
}
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
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.issues.gh2728;
17+
18+
import org.springframework.context.annotation.Configuration;
19+
import org.springframework.data.neo4j.repository.config.EnableNeo4jRepositories;
20+
import org.springframework.data.neo4j.test.Neo4jIntegrationTest;
21+
import org.springframework.transaction.annotation.EnableTransactionManagement;
22+
23+
/**
24+
* @author Michael J. Simons
25+
*/
26+
@Neo4jIntegrationTest
27+
public class CorrectConfigIT extends AbstractTestBase {
28+
29+
@Configuration
30+
@EnableTransactionManagement
31+
@EnableNeo4jRepositories(considerNestedRepositories = true)
32+
static class Config extends AbstractTestBase.Config {
33+
@Override
34+
public boolean isCypher5Compatible() {
35+
return AbstractTestBase.neo4jConnectionSupport.isCypher5SyntaxCompatible();
36+
}
37+
}
38+
}
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
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.issues.gh2728;
17+
18+
import org.springframework.context.annotation.Configuration;
19+
import org.springframework.data.neo4j.repository.config.EnableReactiveNeo4jRepositories;
20+
import org.springframework.data.neo4j.test.Neo4jIntegrationTest;
21+
import org.springframework.transaction.annotation.EnableTransactionManagement;
22+
23+
/**
24+
* @author Michael J. Simons
25+
*/
26+
@Neo4jIntegrationTest
27+
public class CorrectReactiveConfigIT extends AbstractReactiveTestBase {
28+
29+
@Configuration
30+
@EnableTransactionManagement
31+
@EnableReactiveNeo4jRepositories(considerNestedRepositories = true)
32+
static class Config extends AbstractReactiveTestBase.Config {
33+
@Override
34+
public boolean isCypher5Compatible() {
35+
return AbstractReactiveTestBase.neo4jConnectionSupport.isCypher5SyntaxCompatible();
36+
}
37+
}
38+
}
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
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.issues.gh2728;
17+
18+
import org.springframework.data.neo4j.core.schema.Id;
19+
import org.springframework.data.neo4j.core.schema.Node;
20+
import org.springframework.data.neo4j.core.schema.Property;
21+
import org.springframework.data.neo4j.core.schema.Relationship;
22+
23+
/**
24+
* @author Gerrit Meier
25+
*/
26+
@Node
27+
public class TestEntityWithAssignedId1 {
28+
29+
@Id
30+
private String assignedId;
31+
32+
@Property("value_one")
33+
private String valueOne;
34+
35+
@Relationship("related_to")
36+
private TestEntityWithAssignedId2 relatedEntity;
37+
38+
public TestEntityWithAssignedId1(String assignedId, String valueOne, TestEntityWithAssignedId2 relatedEntity) {
39+
this.assignedId = assignedId;
40+
this.valueOne = valueOne;
41+
this.relatedEntity = relatedEntity;
42+
}
43+
public String getAssignedId() {
44+
return assignedId;
45+
}
46+
47+
public TestEntityWithAssignedId2 getRelatedEntity() {
48+
return relatedEntity;
49+
}
50+
}

0 commit comments

Comments
 (0)