Skip to content

Commit 71e3fd7

Browse files
committed
GH-2622 - Throw exception on immutable self-reference.
This slipped through the check before: A has dependency on A defined within the constructor. Because of a "same" entity check, we skipped the `inCreation` check and did not throw the MappingException. Closes #2622
1 parent bcba1e5 commit 71e3fd7

File tree

2 files changed

+156
-8
lines changed

2 files changed

+156
-8
lines changed

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

+8-8
Original file line numberDiff line numberDiff line change
@@ -310,14 +310,6 @@ private <ET> ET map(MapAccessor queryResult, Neo4jPersistentEntity<ET> nodeDescr
310310
String internalId = getInternalId(queryResult, direction);
311311

312312
Supplier<ET> mappedObjectSupplier = () -> {
313-
if (knownObjects.isInCreation(internalId)) {
314-
throw new MappingException(
315-
String.format(
316-
"The node with id %s has a logical cyclic mapping dependency. " +
317-
"Its creation caused the creation of another node that has a reference to this.",
318-
internalId.substring(1))
319-
);
320-
}
321313
knownObjects.setInCreation(internalId);
322314

323315
List<String> allLabels = getLabels(queryResult, nodeDescription);
@@ -962,6 +954,14 @@ private Object getObject(@Nullable String internalId) {
962954
read.lock();
963955

964956
Object knownEntity = internalIdStore.get(internalId);
957+
if (isInCreation(internalId)) {
958+
throw new MappingException(
959+
String.format(
960+
"The node with id %s has a logical cyclic mapping dependency. " +
961+
"Its creation caused the creation of another node that has a reference to this.",
962+
internalId.substring(1))
963+
);
964+
}
965965

966966
if (knownEntity != null) {
967967
return knownEntity;
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,148 @@
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.gh2622;
17+
18+
import org.assertj.core.api.InstanceOfAssertFactories;
19+
import org.junit.jupiter.api.BeforeEach;
20+
import org.junit.jupiter.api.Test;
21+
import org.neo4j.driver.Driver;
22+
import org.neo4j.driver.Session;
23+
import org.springframework.beans.factory.annotation.Autowired;
24+
import org.springframework.context.annotation.Bean;
25+
import org.springframework.context.annotation.Configuration;
26+
import org.springframework.data.mapping.MappingException;
27+
import org.springframework.data.neo4j.core.DatabaseSelectionProvider;
28+
import org.springframework.data.neo4j.core.schema.GeneratedValue;
29+
import org.springframework.data.neo4j.core.schema.Id;
30+
import org.springframework.data.neo4j.core.schema.Node;
31+
import org.springframework.data.neo4j.core.schema.Relationship;
32+
import org.springframework.data.neo4j.core.transaction.Neo4jBookmarkManager;
33+
import org.springframework.data.neo4j.core.transaction.Neo4jTransactionManager;
34+
import org.springframework.data.neo4j.repository.Neo4jRepository;
35+
import org.springframework.data.neo4j.repository.config.EnableNeo4jRepositories;
36+
import org.springframework.data.neo4j.test.BookmarkCapture;
37+
import org.springframework.data.neo4j.test.Neo4jExtension;
38+
import org.springframework.data.neo4j.test.Neo4jImperativeTestConfiguration;
39+
import org.springframework.data.neo4j.test.Neo4jIntegrationTest;
40+
import org.springframework.transaction.PlatformTransactionManager;
41+
import org.springframework.transaction.annotation.EnableTransactionManagement;
42+
43+
import java.util.ArrayList;
44+
import java.util.List;
45+
import java.util.Objects;
46+
47+
import static org.assertj.core.api.Assertions.as;
48+
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
49+
50+
/**
51+
* @author Gerrit Meier
52+
*/
53+
@Neo4jIntegrationTest
54+
class SelfReferenceLoadingIT {
55+
56+
protected static Neo4jExtension.Neo4jConnectionSupport neo4jConnectionSupport;
57+
58+
@BeforeEach
59+
void setupData(@Autowired Driver driver, @Autowired BookmarkCapture bookmarkCapture) {
60+
61+
try (Session session = driver.session(bookmarkCapture.createSessionConfig())) {
62+
session.run("MATCH (n) DETACH DELETE n").consume();
63+
bookmarkCapture.seedWith(session.lastBookmark());
64+
}
65+
}
66+
67+
@Test // GH-2622
68+
void throwCyclicMappingDependencyExceptionOnSelfReference(@Autowired GH2622Repository repository) {
69+
MePointingTowardsMe entity = new MePointingTowardsMe("me", new ArrayList<>());
70+
entity.others.add(entity);
71+
repository.save(entity);
72+
73+
assertThatExceptionOfType(MappingException.class).isThrownBy(repository::findAll)
74+
.withRootCauseInstanceOf(MappingException.class)
75+
.extracting(Throwable::getCause, as(InstanceOfAssertFactories.THROWABLE))
76+
.hasMessageContaining("has a logical cyclic mapping dependency");
77+
78+
}
79+
80+
interface GH2622Repository extends Neo4jRepository<MePointingTowardsMe, Long> {
81+
}
82+
83+
@Node
84+
static class MePointingTowardsMe {
85+
86+
@Id
87+
@GeneratedValue
88+
Long id;
89+
90+
final String name;
91+
92+
@Relationship
93+
final List<MePointingTowardsMe> others;
94+
95+
MePointingTowardsMe(String name, List<MePointingTowardsMe> others) {
96+
this.name = name;
97+
this.others = others;
98+
}
99+
100+
@Override
101+
public boolean equals(Object o) {
102+
if (this == o) {
103+
return true;
104+
}
105+
if (o == null || getClass() != o.getClass()) {
106+
return false;
107+
}
108+
MePointingTowardsMe that = (MePointingTowardsMe) o;
109+
return name.equals(that.name);
110+
}
111+
112+
@Override
113+
public int hashCode() {
114+
return Objects.hash(name);
115+
}
116+
}
117+
118+
@Configuration
119+
@EnableTransactionManagement
120+
@EnableNeo4jRepositories(considerNestedRepositories = true)
121+
static class Config extends Neo4jImperativeTestConfiguration {
122+
123+
@Bean
124+
public BookmarkCapture bookmarkCapture() {
125+
return new BookmarkCapture();
126+
}
127+
128+
@Override
129+
public PlatformTransactionManager transactionManager(
130+
Driver driver, DatabaseSelectionProvider databaseNameProvider) {
131+
132+
BookmarkCapture bookmarkCapture = bookmarkCapture();
133+
return new Neo4jTransactionManager(driver, databaseNameProvider,
134+
Neo4jBookmarkManager.create(bookmarkCapture));
135+
}
136+
137+
@Bean
138+
public Driver driver() {
139+
140+
return neo4jConnectionSupport.getDriver();
141+
}
142+
143+
@Override
144+
public boolean isCypher5Compatible() {
145+
return neo4jConnectionSupport.isCypher5SyntaxCompatible();
146+
}
147+
}
148+
}

0 commit comments

Comments
 (0)