Skip to content

Commit 203195c

Browse files
meistermeiermeistermeier
committed
GH-2639 - Fix relationship detection in converter.
In an inheritance situation, SDN might not search in the result for the right name of the list of relationships from the generic query. Closes #2639
1 parent 111af50 commit 203195c

File tree

13 files changed

+532
-22
lines changed

13 files changed

+532
-22
lines changed

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

+23-21
Original file line numberDiff line numberDiff line change
@@ -299,10 +299,10 @@ private static MapAccessor mergeRootNodeWithRecord(Node node, MapAccessor record
299299
private <ET> ET map(MapAccessor queryResult, MapAccessor allValues, Neo4jPersistentEntity<ET> nodeDescription) {
300300
Collection<Relationship> relationshipsFromResult = extractRelationships(allValues);
301301
Collection<Node> nodesFromResult = extractNodes(allValues);
302-
return map(queryResult, nodeDescription, null, null, relationshipsFromResult, nodesFromResult);
302+
return map(queryResult, nodeDescription, nodeDescription, null, null, relationshipsFromResult, nodesFromResult);
303303
}
304304

305-
private <ET> ET map(MapAccessor queryResult, Neo4jPersistentEntity<ET> nodeDescription,
305+
private <ET> ET map(MapAccessor queryResult, Neo4jPersistentEntity<ET> nodeDescription, NodeDescription<?> genericTargetNodeDescription,
306306
@Nullable Object lastMappedEntity, @Nullable RelationshipDescription relationshipDescription, Collection<Relationship> relationshipsFromResult, Collection<Node> nodesFromResult) {
307307

308308
// prior to SDN 7 local `getInternalId` didn't check relationships, so in that case, they have never been a known
@@ -330,7 +330,7 @@ private <ET> ET map(MapAccessor queryResult, Neo4jPersistentEntity<ET> nodeDescr
330330
Neo4jPersistentEntity<ET> concreteNodeDescription = (Neo4jPersistentEntity<ET>) nodeDescriptionAndLabels
331331
.getNodeDescription();
332332

333-
ET instance = instantiate(concreteNodeDescription, queryResult,
333+
ET instance = instantiate(concreteNodeDescription, genericTargetNodeDescription, queryResult,
334334
nodeDescriptionAndLabels.getDynamicLabels(), lastMappedEntity, relationshipsFromResult, nodesFromResult);
335335

336336
knownObjects.removeFromInCreation(internalId);
@@ -451,7 +451,7 @@ private boolean containsOnePlainNode(MapAccessor queryResult) {
451451
.filter(value -> value.hasType(nodeType)).count() == 1L;
452452
}
453453

454-
private <ET> ET instantiate(Neo4jPersistentEntity<ET> nodeDescription, MapAccessor values,
454+
private <ET> ET instantiate(Neo4jPersistentEntity<ET> nodeDescription, NodeDescription<?> genericNodeDescription, MapAccessor values,
455455
Collection<String> surplusLabels, @Nullable Object lastMappedEntity,
456456
Collection<Relationship> relationshipsFromResult, Collection<Node> nodesFromResult) {
457457

@@ -472,12 +472,12 @@ public <T> T getParameterValue(Parameter<T, Neo4jPersistentProperty> parameter)
472472
// If we cannot find any value it does not mean that there isn't any.
473473
// The result set might contain associations not named CONCRETE_TYPE_TARGET but ABSTRACT_TYPE_TARGET.
474474
// For this we bubble up the hierarchy of NodeDescriptions.
475-
result = createInstanceOfRelationships(matchingProperty, values, relationshipDescription, nodeDescription, relationshipsFromResult, nodesFromResult)
475+
result = createInstanceOfRelationships(matchingProperty, values, relationshipDescription, nodeDescription, genericNodeDescription, relationshipsFromResult, nodesFromResult)
476476
.orElseGet(() -> {
477477
NodeDescription<?> parentNodeDescription = nodeDescription.getParentNodeDescription();
478478
T resultValue = null;
479479
while (parentNodeDescription != null) {
480-
Optional<Object> value = createInstanceOfRelationships(matchingProperty, values, relationshipDescription, parentNodeDescription, relationshipsFromResult, nodesFromResult);
480+
Optional<Object> value = createInstanceOfRelationships(matchingProperty, values, relationshipDescription, parentNodeDescription, parentNodeDescription, relationshipsFromResult, nodesFromResult);
481481
if (value.isPresent()) {
482482
resultValue = (T) value.get();
483483
break;
@@ -570,7 +570,7 @@ private AssociationHandler<Neo4jPersistentProperty> populateFrom(MapAccessor que
570570
&& propertyValueNotNull;
571571

572572
if (populatedCollection) {
573-
createInstanceOfRelationships(persistentProperty, queryResult, (RelationshipDescription) association, baseDescription, relationshipsFromResult, nodesFromResult, false)
573+
createInstanceOfRelationships(persistentProperty, queryResult, (RelationshipDescription) association, baseDescription, baseDescription, relationshipsFromResult, nodesFromResult, false)
574574
.ifPresent(value -> {
575575
Collection<?> providedCollection = (Collection<?>) value;
576576
Collection<?> existingValue = (Collection<?>) propertyValue;
@@ -593,7 +593,7 @@ private AssociationHandler<Neo4jPersistentProperty> populateFrom(MapAccessor que
593593
return;
594594
}
595595

596-
createInstanceOfRelationships(persistentProperty, queryResult, (RelationshipDescription) association, baseDescription, relationshipsFromResult, nodesFromResult)
596+
createInstanceOfRelationships(persistentProperty, queryResult, (RelationshipDescription) association, baseDescription, baseDescription, relationshipsFromResult, nodesFromResult)
597597
.ifPresent(value -> propertyAccessor.setProperty(persistentProperty, value));
598598
};
599599
}
@@ -617,13 +617,13 @@ private void mergeCollections(RelationshipDescription relationshipDescription, C
617617
}
618618

619619
private Optional<Object> createInstanceOfRelationships(Neo4jPersistentProperty persistentProperty, MapAccessor values,
620-
RelationshipDescription relationshipDescription, NodeDescription<?> baseDescription, Collection<Relationship> relationshipsFromResult,
620+
RelationshipDescription relationshipDescription, NodeDescription<?> baseDescription, NodeDescription<?> genericNodeDescription, Collection<Relationship> relationshipsFromResult,
621621
Collection<Node> nodesFromResult) {
622-
return createInstanceOfRelationships(persistentProperty, values, relationshipDescription, baseDescription, relationshipsFromResult, nodesFromResult, true);
622+
return createInstanceOfRelationships(persistentProperty, values, relationshipDescription, baseDescription, genericNodeDescription, relationshipsFromResult, nodesFromResult, true);
623623
}
624624

625625
private Optional<Object> createInstanceOfRelationships(Neo4jPersistentProperty persistentProperty, MapAccessor values,
626-
RelationshipDescription relationshipDescription, NodeDescription<?> baseDescription, Collection<Relationship> relationshipsFromResult,
626+
RelationshipDescription relationshipDescription, NodeDescription<?> baseDescription, NodeDescription<?> genericNodeDescription, Collection<Relationship> relationshipsFromResult,
627627
Collection<Node> nodesFromResult, boolean fetchMore) {
628628

629629
String typeOfRelationship = relationshipDescription.getType();
@@ -657,7 +657,7 @@ private Optional<Object> createInstanceOfRelationships(Neo4jPersistentProperty p
657657
mappedObjectHandler = (type, mappedObject) -> value.add(mappedObject);
658658
}
659659

660-
String collectionName = relationshipDescription.generateRelatedNodesCollectionName(baseDescription);
660+
String collectionName = relationshipDescription.generateRelatedNodesCollectionName(genericNodeDescription);
661661

662662
Value list = values.get(collectionName);
663663

@@ -705,23 +705,24 @@ private Optional<Object> createInstanceOfRelationships(Neo4jPersistentProperty p
705705
if (fetchMore) {
706706
mappedObject = sourceNodeId != null && sourceNodeId.equals(targetNodeId)
707707
? knownObjects.getObject("N" + sourceNodeId)
708-
: map(possibleValueNode, concreteTargetNodeDescription, null, null, relationshipsFromResult, nodesFromResult);
708+
: map(possibleValueNode, concreteTargetNodeDescription, genericNodeDescription, null, null, relationshipsFromResult, nodesFromResult);
709709
} else {
710710
Object objectFromStore = knownObjects.getObject("N" + targetNodeId);
711711
mappedObject = objectFromStore != null
712712
? objectFromStore
713-
: map(possibleValueNode, concreteTargetNodeDescription, null, null, relationshipsFromResult, nodesFromResult);
713+
: map(possibleValueNode, concreteTargetNodeDescription, genericNodeDescription, null, null, relationshipsFromResult, nodesFromResult);
714714
}
715715

716716
if (relationshipDescription.hasRelationshipProperties()) {
717717
Object relationshipProperties;
718+
Neo4jPersistentEntity<?> relationshipPropertiesEntity = (Neo4jPersistentEntity<?>) relationshipDescription.getRelationshipPropertiesEntity();
718719
if (fetchMore) {
719-
relationshipProperties = map(possibleRelationship, (Neo4jPersistentEntity<?>) relationshipDescription.getRelationshipPropertiesEntity(), mappedObject, relationshipDescription, relationshipsFromResult, nodesFromResult);
720+
relationshipProperties = map(possibleRelationship, relationshipPropertiesEntity, relationshipPropertiesEntity, mappedObject, relationshipDescription, relationshipsFromResult, nodesFromResult);
720721
} else {
721722
Object objectFromStore = knownObjects.getObject(IdentitySupport.getInternalId(possibleRelationship, relationshipDescription.getDirection().name()));
722723
relationshipProperties = objectFromStore != null
723724
? objectFromStore
724-
: map(possibleRelationship, (Neo4jPersistentEntity<?>) relationshipDescription.getRelationshipPropertiesEntity(), mappedObject, relationshipDescription, relationshipsFromResult, nodesFromResult);
725+
: map(possibleRelationship, relationshipPropertiesEntity, relationshipPropertiesEntity, mappedObject, relationshipDescription, relationshipsFromResult, nodesFromResult);
725726
}
726727
relationshipsAndProperties.add(relationshipProperties);
727728
mappedObjectHandler.accept(possibleRelationship.type(), relationshipProperties);
@@ -741,29 +742,30 @@ private Optional<Object> createInstanceOfRelationships(Neo4jPersistentProperty p
741742

742743
Object valueEntry;
743744
if (fetchMore) {
744-
valueEntry = map(relatedEntity, concreteTargetNodeDescription, null, null, relationshipsFromResult, nodesFromResult);
745+
valueEntry = map(relatedEntity, concreteTargetNodeDescription, genericNodeDescription, null, null, relationshipsFromResult, nodesFromResult);
745746
} else {
746747
Object objectFromStore = knownObjects.getObject(IdentitySupport.getInternalId(relatedEntity, null));
747748
valueEntry = objectFromStore != null
748749
? objectFromStore
749-
: map(relatedEntity, concreteTargetNodeDescription, null, null, relationshipsFromResult, nodesFromResult);
750+
: map(relatedEntity, concreteTargetNodeDescription, genericNodeDescription, null, null, relationshipsFromResult, nodesFromResult);
750751
}
751752

752753
if (relationshipDescription.hasRelationshipProperties()) {
753-
String sourceLabel = relationshipDescription.getSource().getMostAbstractParentLabel(baseDescription);
754+
String sourceLabel = relationshipDescription.getSource().getMostAbstractParentLabel(genericNodeDescription);
754755
String relationshipSymbolicName = sourceLabel
755756
+ RelationshipDescription.NAME_OF_RELATIONSHIP + targetLabel;
756757
Relationship relatedEntityRelationship = relatedEntity.get(relationshipSymbolicName)
757758
.asRelationship();
758759

759760
Object relationshipProperties;
761+
Neo4jPersistentEntity<?> relationshipPropertiesEntity = (Neo4jPersistentEntity<?>) relationshipDescription.getRelationshipPropertiesEntity();
760762
if (fetchMore) {
761-
relationshipProperties = map(relatedEntityRelationship, (Neo4jPersistentEntity<?>) relationshipDescription.getRelationshipPropertiesEntity(), valueEntry, relationshipDescription, relationshipsFromResult, nodesFromResult);
763+
relationshipProperties = map(relatedEntityRelationship, relationshipPropertiesEntity, relationshipPropertiesEntity, valueEntry, relationshipDescription, relationshipsFromResult, nodesFromResult);
762764
} else {
763765
Object objectFromStore = knownObjects.getObject(IdentitySupport.getInternalId(relatedEntityRelationship, relationshipDescription.getDirection().name()));
764766
relationshipProperties = objectFromStore != null
765767
? objectFromStore
766-
: map(relatedEntityRelationship, (Neo4jPersistentEntity<?>) relationshipDescription.getRelationshipPropertiesEntity(), valueEntry, relationshipDescription, relationshipsFromResult, nodesFromResult);
768+
: map(relatedEntityRelationship, relationshipPropertiesEntity, relationshipPropertiesEntity, valueEntry, relationshipDescription, relationshipsFromResult, nodesFromResult);
767769
}
768770

769771
relationshipsAndProperties.add(relationshipProperties);

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

+55
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
2020
import static org.assertj.core.api.Assertions.tuple;
2121

22+
import java.util.ArrayList;
2223
import java.util.Arrays;
2324
import java.util.Collection;
2425
import java.util.Collections;
@@ -127,6 +128,15 @@
127128
import org.springframework.data.neo4j.integration.issues.gh2579.TableRepository;
128129
import org.springframework.data.neo4j.integration.issues.gh2583.GH2583Node;
129130
import org.springframework.data.neo4j.integration.issues.gh2583.GH2583Repository;
131+
import org.springframework.data.neo4j.integration.issues.gh2639.Company;
132+
import org.springframework.data.neo4j.integration.issues.gh2639.CompanyPerson;
133+
import org.springframework.data.neo4j.integration.issues.gh2639.CompanyRepository;
134+
import org.springframework.data.neo4j.integration.issues.gh2639.Developer;
135+
import org.springframework.data.neo4j.integration.issues.gh2639.Enterprise;
136+
import org.springframework.data.neo4j.integration.issues.gh2639.Individual;
137+
import org.springframework.data.neo4j.integration.issues.gh2639.LanguageRelationship;
138+
import org.springframework.data.neo4j.integration.issues.gh2639.ProgrammingLanguage;
139+
import org.springframework.data.neo4j.integration.issues.gh2639.Sales;
130140
import org.springframework.data.neo4j.integration.misc.ConcreteImplementationTwo;
131141
import org.springframework.data.neo4j.repository.config.EnableNeo4jRepositories;
132142
import org.springframework.data.neo4j.repository.query.QueryFragmentsAndParameters;
@@ -936,6 +946,51 @@ void mapStandardCustomQueryWithLotsOfRelationshipsProperly(@Autowired GH2583Repo
936946
assertThat(nodes).hasSize(2);
937947
}
938948

949+
@Test
950+
@Tag("GH-2639")
951+
void relationshipsOfGenericRelationshipsGetResolvedCorrectly(@Autowired CompanyRepository companyRepository) {
952+
CompanyPerson greg = new Sales("Greg");
953+
CompanyPerson roy = new Sales("Roy");
954+
CompanyPerson craig = new Sales("Craig");
955+
956+
ProgrammingLanguage java = new ProgrammingLanguage("java", "1.5");
957+
java.inventor = new Enterprise("Sun", ";(");
958+
ProgrammingLanguage perl = new ProgrammingLanguage("perl", "6.0");
959+
perl.inventor = new Individual("Larry Wall", "larryW");
960+
961+
List<LanguageRelationship> languageRelationships = new ArrayList<>();
962+
LanguageRelationship javaRelationship = new LanguageRelationship(5, java);
963+
LanguageRelationship perlRelationship = new LanguageRelationship(2, perl);
964+
languageRelationships.add(javaRelationship);
965+
languageRelationships.add(perlRelationship);
966+
967+
Developer harry = new Developer("Harry", languageRelationships);
968+
List<CompanyPerson> team = Arrays.asList(greg, roy, craig, harry);
969+
Company acme = new Company("ACME", team);
970+
companyRepository.save(acme);
971+
972+
Company loadedAcme = companyRepository.findByName("ACME");
973+
974+
Developer loadedHarry = loadedAcme.getEmployees().stream()
975+
.filter(e -> e instanceof Developer)
976+
.map(e -> (Developer) e)
977+
.filter(developer -> developer.getName().equals("Harry"))
978+
.findFirst().get();
979+
980+
List<LanguageRelationship> programmingLanguages = loadedHarry.getProgrammingLanguages();
981+
assertThat(programmingLanguages)
982+
.isNotEmpty()
983+
.extracting("score")
984+
.containsExactlyInAnyOrder(5, 2);
985+
986+
assertThat(programmingLanguages)
987+
.extracting("language")
988+
.extracting("inventor")
989+
.containsExactlyInAnyOrder(
990+
new Individual("Larry Wall", "larryW"), new Enterprise("Sun", ";(")
991+
);
992+
}
993+
939994
@Configuration
940995
@EnableTransactionManagement
941996
@EnableNeo4jRepositories(namedQueriesLocation = "more-custom-queries.properties")

‎src/test/java/org/springframework/data/neo4j/integration/issues/gh2639/Company.java

+62
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
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.gh2639;
17+
18+
import org.springframework.data.neo4j.core.schema.GeneratedValue;
19+
import org.springframework.data.neo4j.core.schema.Id;
20+
import org.springframework.data.neo4j.core.schema.Node;
21+
import org.springframework.data.neo4j.core.schema.Relationship;
22+
23+
import java.util.List;
24+
import java.util.StringJoiner;
25+
26+
/**
27+
* Root node
28+
*/
29+
@Node
30+
public class Company {
31+
32+
@Id
33+
@GeneratedValue
34+
private Long id;
35+
36+
private final String name;
37+
@Relationship(type = "EMPLOYEE")
38+
private final List<CompanyPerson> employees;
39+
40+
41+
public Company(String name, List<CompanyPerson> employees) {
42+
this.name = name;
43+
this.employees = employees;
44+
}
45+
46+
public void addEmployee(CompanyPerson person) {
47+
employees.add(person);
48+
}
49+
50+
public List<CompanyPerson> getEmployees() {
51+
return employees;
52+
}
53+
54+
@Override
55+
public String toString() {
56+
return new StringJoiner(", ", Company.class.getSimpleName() + "[", "]")
57+
.add("id=" + id)
58+
.add("name='" + name + "'")
59+
.add("employees=" + employees)
60+
.toString();
61+
}
62+
}

‎src/test/java/org/springframework/data/neo4j/integration/issues/gh2639/CompanyPerson.java

+32
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
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.gh2639;
17+
18+
import org.springframework.data.neo4j.core.schema.GeneratedValue;
19+
import org.springframework.data.neo4j.core.schema.Id;
20+
import org.springframework.data.neo4j.core.schema.Node;
21+
22+
/**
23+
* Abstract base class for relationships from company to specific persons.
24+
*/
25+
@Node
26+
public abstract class CompanyPerson {
27+
28+
@Id
29+
@GeneratedValue
30+
private Long id;
31+
32+
}

‎src/test/java/org/springframework/data/neo4j/integration/issues/gh2639/CompanyRepository.java

+25
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
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.gh2639;
17+
18+
import org.springframework.data.neo4j.repository.Neo4jRepository;
19+
20+
/**
21+
* @author Gerrit Meier
22+
*/
23+
public interface CompanyRepository extends Neo4jRepository<Company, Long> {
24+
Company findByName(String name);
25+
}

0 commit comments

Comments
 (0)