Skip to content

Commit 6452de1

Browse files
michael-simonsmichael-simons
committed
GH-2500 - Use actual id values when present to be stored in the relationship statemachine.
When checking if a relationship has already been processed, the actual fromId is always the one returned by the persistent entity: This is always an internal generated id when no id generators are present, but when a user runs their own, it’s not. Therefor, when the internal id is always used in the statemachine, a processed relationship would have not been seen and known and deleted in a 2nd go. This fixes #2500.
1 parent 7915c5c commit 6452de1

File tree

7 files changed

+390
-10
lines changed

7 files changed

+390
-10
lines changed

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

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -794,21 +794,23 @@ private <T> T processNestedRelations(
794794
}
795795
}
796796

797+
Neo4jPersistentProperty requiredIdProperty = targetEntity.getRequiredIdProperty();
797798
PersistentPropertyAccessor<?> targetPropertyAccessor = targetEntity.getPropertyAccessor(newRelatedObject);
799+
Object actualRelatedId = targetPropertyAccessor.getProperty(requiredIdProperty);
798800
// if an internal id is used this must be set to link this entity in the next iteration
799801
if (targetEntity.isUsingInternalIds()) {
800-
Neo4jPersistentProperty requiredIdProperty = targetEntity.getRequiredIdProperty();
801-
if (relatedInternalId == null && targetPropertyAccessor.getProperty(requiredIdProperty) != null) {
802+
if (relatedInternalId == null && actualRelatedId != null) {
802803
relatedInternalId = (Long) targetPropertyAccessor.getProperty(requiredIdProperty);
803-
} else if (targetPropertyAccessor.getProperty(requiredIdProperty) == null) {
804+
} else if (actualRelatedId == null) {
804805
targetPropertyAccessor.setProperty(requiredIdProperty, relatedInternalId);
805806
}
806807
}
807808
if (savedEntity != null) {
808809
TemplateSupport.updateVersionPropertyIfPossible(targetEntity, targetPropertyAccessor, savedEntity);
809810
}
810811
stateMachine.markValueAsProcessedAs(relatedObjectBeforeCallbacksApplied, targetPropertyAccessor.getBean());
811-
stateMachine.markRelationshipAsProcessed(relatedInternalId, relationshipDescription.getRelationshipObverse());
812+
stateMachine.markRelationshipAsProcessed(actualRelatedId == null ? relatedInternalId : actualRelatedId,
813+
relationshipDescription.getRelationshipObverse());
812814

813815
Object idValue = idProperty != null
814816
? relationshipContext

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

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -930,22 +930,23 @@ private <T> Mono<T> processNestedRelations(Neo4jPersistentEntity<?> sourceEntity
930930
return queryOrSave.flatMap(idAndEntity -> {
931931
Long relatedInternalId = idAndEntity.getT1().get();
932932
Entity savedEntity = idAndEntity.getT2().get();
933-
// if an internal id is used this must be set to link this entity in the next iteration
933+
Neo4jPersistentProperty requiredIdProperty = targetEntity.getRequiredIdProperty();
934934
PersistentPropertyAccessor<?> targetPropertyAccessor = targetEntity.getPropertyAccessor(newRelatedObject);
935+
Object actualRelatedId = targetPropertyAccessor.getProperty(requiredIdProperty);
936+
// if an internal id is used this must be set to link this entity in the next iteration
935937
if (targetEntity.isUsingInternalIds()) {
936-
Neo4jPersistentProperty requiredIdProperty = targetEntity.getRequiredIdProperty();
937-
if (relatedInternalId == null
938-
&& targetPropertyAccessor.getProperty(requiredIdProperty) != null) {
938+
if (relatedInternalId == null && actualRelatedId != null) {
939939
relatedInternalId = (Long) targetPropertyAccessor.getProperty(requiredIdProperty);
940-
} else if (targetPropertyAccessor.getProperty(requiredIdProperty) == null) {
940+
} else if (actualRelatedId == null) {
941941
targetPropertyAccessor.setProperty(requiredIdProperty, relatedInternalId);
942942
}
943943
}
944944
if (savedEntity != null) {
945945
TemplateSupport.updateVersionPropertyIfPossible(targetEntity, targetPropertyAccessor, savedEntity);
946946
}
947947
stateMachine.markValueAsProcessedAs(relatedObjectBeforeCallbacksApplied, targetPropertyAccessor.getBean());
948-
stateMachine.markRelationshipAsProcessed(relatedInternalId, relationshipDescription.getRelationshipObverse());
948+
stateMachine.markRelationshipAsProcessed(actualRelatedId == null ? relatedInternalId : actualRelatedId,
949+
relationshipDescription.getRelationshipObverse());
949950

950951
Object idValue = idProperty != null
951952
? relationshipContext

‎src/test/java/org/springframework/data/neo4j/integration/issues/gh2500/Device.java‎

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
/*
2+
* Copyright 2011-2022 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.gh2500;
17+
18+
import lombok.Getter;
19+
import lombok.Setter;
20+
21+
import java.util.LinkedHashSet;
22+
import java.util.Set;
23+
24+
import org.springframework.data.annotation.Version;
25+
import org.springframework.data.neo4j.core.schema.Id;
26+
import org.springframework.data.neo4j.core.schema.Node;
27+
import org.springframework.data.neo4j.core.schema.Relationship;
28+
29+
/**
30+
* @author Michael J. Simons
31+
*/
32+
@Node
33+
@Getter
34+
@Setter
35+
public class Device {
36+
37+
@Id
38+
private Long id;
39+
40+
@Version
41+
private Long version;
42+
43+
private String name;
44+
45+
@Relationship(type = "BELONGS_TO", direction = Relationship.Direction.OUTGOING)
46+
private Set<Group> groups = new LinkedHashSet<>();
47+
48+
@Override
49+
public boolean equals(Object o) {
50+
if (this == o) {
51+
return true;
52+
}
53+
if (o == null || getClass() != o.getClass()) {
54+
return false;
55+
}
56+
57+
Device device = (Device) o;
58+
59+
return id.equals(device.id);
60+
}
61+
62+
@Override
63+
public int hashCode() {
64+
int result = id != null ? id.hashCode() : 0;
65+
result = 31 * result + (name != null ? name.hashCode() : 0);
66+
return result;
67+
}
68+
}

‎src/test/java/org/springframework/data/neo4j/integration/issues/gh2500/GH2500IT.java‎

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
/*
2+
* Copyright 2011-2022 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.gh2500;
17+
18+
import static org.assertj.core.api.Assertions.assertThat;
19+
20+
import java.util.HashMap;
21+
import java.util.Map;
22+
23+
import org.junit.jupiter.api.Test;
24+
import org.neo4j.driver.Driver;
25+
import org.neo4j.driver.Session;
26+
import org.springframework.beans.factory.annotation.Autowired;
27+
import org.springframework.context.annotation.Bean;
28+
import org.springframework.context.annotation.Configuration;
29+
import org.springframework.data.neo4j.config.AbstractNeo4jConfig;
30+
import org.springframework.data.neo4j.core.DatabaseSelectionProvider;
31+
import org.springframework.data.neo4j.core.Neo4jTemplate;
32+
import org.springframework.data.neo4j.core.transaction.Neo4jBookmarkManager;
33+
import org.springframework.data.neo4j.core.transaction.Neo4jTransactionManager;
34+
import org.springframework.data.neo4j.test.BookmarkCapture;
35+
import org.springframework.data.neo4j.test.Neo4jIntegrationTest;
36+
import org.springframework.transaction.PlatformTransactionManager;
37+
import org.springframework.transaction.annotation.EnableTransactionManagement;
38+
39+
/**
40+
* @author Michael J. Simons
41+
*/
42+
@Neo4jIntegrationTest
43+
public class GH2500IT extends TestBase {
44+
45+
@Test // GH-2498
46+
void shouldNotDeleteFreshlyCreatedRelationships(@Autowired Driver driver, @Autowired Neo4jTemplate template) {
47+
48+
Group group = new Group();
49+
group.setName("test");
50+
group.getDevices().add(template.findById(1L, Device.class).get());
51+
52+
template.save(group);
53+
54+
try (Session session = driver.session()) {
55+
Map<String, Object> parameters = new HashMap<>();
56+
parameters.put("name", group.getName());
57+
parameters.put("deviceId", 1L);
58+
long cnt = session.run(
59+
"MATCH (g:Group {name: $name}) <-[:BELONGS_TO]- (d:Device {id: $deviceId}) RETURN count(*)",
60+
parameters)
61+
.single().get(0).asLong();
62+
assertThat(cnt).isOne();
63+
}
64+
}
65+
66+
@Configuration
67+
@EnableTransactionManagement
68+
static class Config extends AbstractNeo4jConfig {
69+
70+
@Bean
71+
public BookmarkCapture bookmarkCapture() {
72+
return new BookmarkCapture();
73+
}
74+
75+
@Override
76+
public PlatformTransactionManager transactionManager(
77+
Driver driver, DatabaseSelectionProvider databaseNameProvider) {
78+
79+
BookmarkCapture bookmarkCapture = bookmarkCapture();
80+
return new Neo4jTransactionManager(driver, databaseNameProvider,
81+
Neo4jBookmarkManager.create(bookmarkCapture));
82+
}
83+
84+
@Bean
85+
public Driver driver() {
86+
87+
return neo4jConnectionSupport.getDriver();
88+
}
89+
}
90+
}

‎src/test/java/org/springframework/data/neo4j/integration/issues/gh2500/Group.java‎

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
/*
2+
* Copyright 2011-2022 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.gh2500;
17+
18+
import lombok.Getter;
19+
import lombok.Setter;
20+
21+
import java.util.LinkedHashSet;
22+
import java.util.Set;
23+
24+
import org.springframework.data.annotation.Version;
25+
import org.springframework.data.neo4j.core.schema.GeneratedValue;
26+
import org.springframework.data.neo4j.core.schema.Id;
27+
import org.springframework.data.neo4j.core.schema.Node;
28+
import org.springframework.data.neo4j.core.schema.Relationship;
29+
import org.springframework.data.neo4j.core.support.UUIDStringGenerator;
30+
import org.springframework.lang.NonNull;
31+
32+
/**
33+
* @author Michael J. Simons
34+
*/
35+
@Node
36+
@Getter
37+
@Setter
38+
public class Group {
39+
40+
@Id
41+
@GeneratedValue(generatorClass = UUIDStringGenerator.class)
42+
private String id;
43+
44+
@Version
45+
private Long version;
46+
47+
@NonNull
48+
private String name;
49+
50+
@Relationship(type = "BELONGS_TO", direction = Relationship.Direction.INCOMING)
51+
private Set<Device> devices = new LinkedHashSet<>();
52+
53+
@Relationship(type = "GROUP_LINK")
54+
private Set<Group> groups = new LinkedHashSet<>();
55+
56+
@Override
57+
public boolean equals(Object o) {
58+
if (this == o) {
59+
return true;
60+
}
61+
if (o == null || getClass() != o.getClass()) {
62+
return false;
63+
}
64+
65+
Group group = (Group) o;
66+
67+
if (!id.equals(group.id)) {
68+
return false;
69+
}
70+
return name.equals(group.name);
71+
}
72+
73+
@Override
74+
public int hashCode() {
75+
int result = 7;
76+
result = 31 * result + id.hashCode();
77+
result = 31 * result + name.hashCode();
78+
return result;
79+
}
80+
}

‎src/test/java/org/springframework/data/neo4j/integration/issues/gh2500/ReactiveGH2500IT.java‎

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
/*
2+
* Copyright 2011-2022 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.gh2500;
17+
18+
import static org.assertj.core.api.Assertions.assertThat;
19+
20+
import reactor.test.StepVerifier;
21+
22+
import java.util.HashMap;
23+
import java.util.Map;
24+
25+
import org.junit.jupiter.api.Test;
26+
import org.neo4j.driver.Driver;
27+
import org.neo4j.driver.Session;
28+
import org.springframework.beans.factory.annotation.Autowired;
29+
import org.springframework.context.annotation.Bean;
30+
import org.springframework.context.annotation.Configuration;
31+
import org.springframework.data.neo4j.config.AbstractReactiveNeo4jConfig;
32+
import org.springframework.data.neo4j.core.ReactiveDatabaseSelectionProvider;
33+
import org.springframework.data.neo4j.core.ReactiveNeo4jTemplate;
34+
import org.springframework.data.neo4j.core.transaction.Neo4jBookmarkManager;
35+
import org.springframework.data.neo4j.core.transaction.ReactiveNeo4jTransactionManager;
36+
import org.springframework.data.neo4j.test.BookmarkCapture;
37+
import org.springframework.data.neo4j.test.Neo4jIntegrationTest;
38+
import org.springframework.transaction.ReactiveTransactionManager;
39+
import org.springframework.transaction.annotation.EnableTransactionManagement;
40+
41+
/**
42+
* @author Michael J. Simons
43+
*/
44+
@Neo4jIntegrationTest
45+
public class ReactiveGH2500IT extends TestBase {
46+
47+
@Test // GH-2498
48+
void shouldNotDeleteFreshlyCreatedRelationships(@Autowired Driver driver, @Autowired
49+
ReactiveNeo4jTemplate template) {
50+
51+
Group group = new Group();
52+
group.setName("test");
53+
template.findById(1L, Device.class)
54+
.flatMap(d -> {
55+
group.getDevices().add(d);
56+
return template.save(group);
57+
})
58+
.as(StepVerifier::create)
59+
.expectNextCount(1L)
60+
.verifyComplete();
61+
62+
63+
try (Session session = driver.session()) {
64+
Map<String, Object> parameters = new HashMap<>();
65+
parameters.put("name", group.getName());
66+
parameters.put("deviceId", 1L);
67+
long cnt = session.run(
68+
"MATCH (g:Group {name: $name}) <-[:BELONGS_TO]- (d:Device {id: $deviceId}) RETURN count(*)",
69+
parameters)
70+
.single().get(0).asLong();
71+
assertThat(cnt).isOne();
72+
}
73+
}
74+
75+
@Configuration
76+
@EnableTransactionManagement
77+
static class Config extends AbstractReactiveNeo4jConfig {
78+
79+
@Bean
80+
public BookmarkCapture bookmarkCapture() {
81+
return new BookmarkCapture();
82+
}
83+
84+
@Override
85+
public ReactiveTransactionManager reactiveTransactionManager(Driver driver,
86+
ReactiveDatabaseSelectionProvider databaseSelectionProvider) {
87+
BookmarkCapture bookmarkCapture = bookmarkCapture();
88+
return new ReactiveNeo4jTransactionManager(driver, databaseSelectionProvider,
89+
Neo4jBookmarkManager.create(bookmarkCapture));
90+
}
91+
92+
@Bean
93+
public Driver driver() {
94+
95+
return neo4jConnectionSupport.getDriver();
96+
}
97+
}
98+
}

0 commit comments

Comments
 (0)