Skip to content

Commit fe34e72

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 d192a80 commit fe34e72

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
@@ -264,10 +264,10 @@ private static MapAccessor mergeRootNodeWithRecord(Node node, MapAccessor record
264264
private <ET> ET map(MapAccessor queryResult, MapAccessor allValues, Neo4jPersistentEntity<ET> nodeDescription) {
265265
Collection<Relationship> relationshipsFromResult = extractRelationships(allValues);
266266
Collection<Node> nodesFromResult = extractNodes(allValues);
267-
return map(queryResult, nodeDescription, null, null, relationshipsFromResult, nodesFromResult);
267+
return map(queryResult, nodeDescription, nodeDescription, null, null, relationshipsFromResult, nodesFromResult);
268268
}
269269

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

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

298-
ET instance = instantiate(concreteNodeDescription, queryResult,
298+
ET instance = instantiate(concreteNodeDescription, genericTargetNodeDescription, queryResult,
299299
nodeDescriptionAndLabels.getDynamicLabels(), lastMappedEntity, relationshipsFromResult, nodesFromResult);
300300

301301
knownObjects.removeFromInCreation(internalId);
@@ -416,7 +416,7 @@ private boolean containsOnePlainNode(MapAccessor queryResult) {
416416
.filter(value -> value.hasType(nodeType)).count() == 1L;
417417
}
418418

419-
private <ET> ET instantiate(Neo4jPersistentEntity<ET> nodeDescription, MapAccessor values,
419+
private <ET> ET instantiate(Neo4jPersistentEntity<ET> nodeDescription, NodeDescription<?> genericNodeDescription, MapAccessor values,
420420
Collection<String> surplusLabels, @Nullable Object lastMappedEntity,
421421
Collection<Relationship> relationshipsFromResult, Collection<Node> nodesFromResult) {
422422

@@ -437,12 +437,12 @@ public <T> T getParameterValue(Parameter<T, Neo4jPersistentProperty> parameter)
437437
// If we cannot find any value it does not mean that there isn't any.
438438
// The result set might contain associations not named CONCRETE_TYPE_TARGET but ABSTRACT_TYPE_TARGET.
439439
// For this we bubble up the hierarchy of NodeDescriptions.
440-
result = createInstanceOfRelationships(matchingProperty, values, relationshipDescription, nodeDescription, relationshipsFromResult, nodesFromResult)
440+
result = createInstanceOfRelationships(matchingProperty, values, relationshipDescription, nodeDescription, genericNodeDescription, relationshipsFromResult, nodesFromResult)
441441
.orElseGet(() -> {
442442
NodeDescription<?> parentNodeDescription = nodeDescription.getParentNodeDescription();
443443
T resultValue = null;
444444
while (parentNodeDescription != null) {
445-
Optional<Object> value = createInstanceOfRelationships(matchingProperty, values, relationshipDescription, parentNodeDescription, relationshipsFromResult, nodesFromResult);
445+
Optional<Object> value = createInstanceOfRelationships(matchingProperty, values, relationshipDescription, parentNodeDescription, parentNodeDescription, relationshipsFromResult, nodesFromResult);
446446
if (value.isPresent()) {
447447
resultValue = (T) value.get();
448448
break;
@@ -535,7 +535,7 @@ private AssociationHandler<Neo4jPersistentProperty> populateFrom(MapAccessor que
535535
&& propertyValueNotNull;
536536

537537
if (populatedCollection) {
538-
createInstanceOfRelationships(persistentProperty, queryResult, (RelationshipDescription) association, baseDescription, relationshipsFromResult, nodesFromResult, false)
538+
createInstanceOfRelationships(persistentProperty, queryResult, (RelationshipDescription) association, baseDescription, baseDescription, relationshipsFromResult, nodesFromResult, false)
539539
.ifPresent(value -> {
540540
Collection<?> providedCollection = (Collection<?>) value;
541541
Collection<?> existingValue = (Collection<?>) propertyValue;
@@ -558,7 +558,7 @@ private AssociationHandler<Neo4jPersistentProperty> populateFrom(MapAccessor que
558558
return;
559559
}
560560

561-
createInstanceOfRelationships(persistentProperty, queryResult, (RelationshipDescription) association, baseDescription, relationshipsFromResult, nodesFromResult)
561+
createInstanceOfRelationships(persistentProperty, queryResult, (RelationshipDescription) association, baseDescription, baseDescription, relationshipsFromResult, nodesFromResult)
562562
.ifPresent(value -> propertyAccessor.setProperty(persistentProperty, value));
563563
};
564564
}
@@ -582,13 +582,13 @@ private void mergeCollections(RelationshipDescription relationshipDescription, C
582582
}
583583

584584
private Optional<Object> createInstanceOfRelationships(Neo4jPersistentProperty persistentProperty, MapAccessor values,
585-
RelationshipDescription relationshipDescription, NodeDescription<?> baseDescription, Collection<Relationship> relationshipsFromResult,
585+
RelationshipDescription relationshipDescription, NodeDescription<?> baseDescription, NodeDescription<?> genericNodeDescription, Collection<Relationship> relationshipsFromResult,
586586
Collection<Node> nodesFromResult) {
587-
return createInstanceOfRelationships(persistentProperty, values, relationshipDescription, baseDescription, relationshipsFromResult, nodesFromResult, true);
587+
return createInstanceOfRelationships(persistentProperty, values, relationshipDescription, baseDescription, genericNodeDescription, relationshipsFromResult, nodesFromResult, true);
588588
}
589589

590590
private Optional<Object> createInstanceOfRelationships(Neo4jPersistentProperty persistentProperty, MapAccessor values,
591-
RelationshipDescription relationshipDescription, NodeDescription<?> baseDescription, Collection<Relationship> relationshipsFromResult,
591+
RelationshipDescription relationshipDescription, NodeDescription<?> baseDescription, NodeDescription<?> genericNodeDescription, Collection<Relationship> relationshipsFromResult,
592592
Collection<Node> nodesFromResult, boolean fetchMore) {
593593

594594
String typeOfRelationship = relationshipDescription.getType();
@@ -622,7 +622,7 @@ private Optional<Object> createInstanceOfRelationships(Neo4jPersistentProperty p
622622
mappedObjectHandler = (type, mappedObject) -> value.add(mappedObject);
623623
}
624624

625-
String collectionName = relationshipDescription.generateRelatedNodesCollectionName(baseDescription);
625+
String collectionName = relationshipDescription.generateRelatedNodesCollectionName(genericNodeDescription);
626626

627627
Value list = values.get(collectionName);
628628

@@ -670,23 +670,24 @@ private Optional<Object> createInstanceOfRelationships(Neo4jPersistentProperty p
670670
if (fetchMore) {
671671
mappedObject = sourceNodeId != null && sourceNodeId.equals(targetNodeId)
672672
? knownObjects.getObject("N" + sourceNodeId)
673-
: map(possibleValueNode, concreteTargetNodeDescription, null, null, relationshipsFromResult, nodesFromResult);
673+
: map(possibleValueNode, concreteTargetNodeDescription, genericNodeDescription, null, null, relationshipsFromResult, nodesFromResult);
674674
} else {
675675
Object objectFromStore = knownObjects.getObject("N" + targetNodeId);
676676
mappedObject = objectFromStore != null
677677
? objectFromStore
678-
: map(possibleValueNode, concreteTargetNodeDescription, null, null, relationshipsFromResult, nodesFromResult);
678+
: map(possibleValueNode, concreteTargetNodeDescription, genericNodeDescription, null, null, relationshipsFromResult, nodesFromResult);
679679
}
680680

681681
if (relationshipDescription.hasRelationshipProperties()) {
682682
Object relationshipProperties;
683+
Neo4jPersistentEntity<?> relationshipPropertiesEntity = (Neo4jPersistentEntity<?>) relationshipDescription.getRelationshipPropertiesEntity();
683684
if (fetchMore) {
684-
relationshipProperties = map(possibleRelationship, (Neo4jPersistentEntity<?>) relationshipDescription.getRelationshipPropertiesEntity(), mappedObject, relationshipDescription, relationshipsFromResult, nodesFromResult);
685+
relationshipProperties = map(possibleRelationship, relationshipPropertiesEntity, relationshipPropertiesEntity, mappedObject, relationshipDescription, relationshipsFromResult, nodesFromResult);
685686
} else {
686687
Object objectFromStore = knownObjects.getObject(IdentitySupport.getInternalId(possibleRelationship, relationshipDescription.getDirection().name()));
687688
relationshipProperties = objectFromStore != null
688689
? objectFromStore
689-
: map(possibleRelationship, (Neo4jPersistentEntity<?>) relationshipDescription.getRelationshipPropertiesEntity(), mappedObject, relationshipDescription, relationshipsFromResult, nodesFromResult);
690+
: map(possibleRelationship, relationshipPropertiesEntity, relationshipPropertiesEntity, mappedObject, relationshipDescription, relationshipsFromResult, nodesFromResult);
690691
}
691692
relationshipsAndProperties.add(relationshipProperties);
692693
mappedObjectHandler.accept(possibleRelationship.type(), relationshipProperties);
@@ -706,29 +707,30 @@ private Optional<Object> createInstanceOfRelationships(Neo4jPersistentProperty p
706707

707708
Object valueEntry;
708709
if (fetchMore) {
709-
valueEntry = map(relatedEntity, concreteTargetNodeDescription, null, null, relationshipsFromResult, nodesFromResult);
710+
valueEntry = map(relatedEntity, concreteTargetNodeDescription, genericNodeDescription, null, null, relationshipsFromResult, nodesFromResult);
710711
} else {
711712
Object objectFromStore = knownObjects.getObject(IdentitySupport.getInternalId(relatedEntity, null));
712713
valueEntry = objectFromStore != null
713714
? objectFromStore
714-
: map(relatedEntity, concreteTargetNodeDescription, null, null, relationshipsFromResult, nodesFromResult);
715+
: map(relatedEntity, concreteTargetNodeDescription, genericNodeDescription, null, null, relationshipsFromResult, nodesFromResult);
715716
}
716717

717718
if (relationshipDescription.hasRelationshipProperties()) {
718-
String sourceLabel = relationshipDescription.getSource().getMostAbstractParentLabel(baseDescription);
719+
String sourceLabel = relationshipDescription.getSource().getMostAbstractParentLabel(genericNodeDescription);
719720
String relationshipSymbolicName = sourceLabel
720721
+ RelationshipDescription.NAME_OF_RELATIONSHIP + targetLabel;
721722
Relationship relatedEntityRelationship = relatedEntity.get(relationshipSymbolicName)
722723
.asRelationship();
723724

724725
Object relationshipProperties;
726+
Neo4jPersistentEntity<?> relationshipPropertiesEntity = (Neo4jPersistentEntity<?>) relationshipDescription.getRelationshipPropertiesEntity();
725727
if (fetchMore) {
726-
relationshipProperties = map(relatedEntityRelationship, (Neo4jPersistentEntity<?>) relationshipDescription.getRelationshipPropertiesEntity(), valueEntry, relationshipDescription, relationshipsFromResult, nodesFromResult);
728+
relationshipProperties = map(relatedEntityRelationship, relationshipPropertiesEntity, relationshipPropertiesEntity, valueEntry, relationshipDescription, relationshipsFromResult, nodesFromResult);
727729
} else {
728730
Object objectFromStore = knownObjects.getObject(IdentitySupport.getInternalId(relatedEntityRelationship, relationshipDescription.getDirection().name()));
729731
relationshipProperties = objectFromStore != null
730732
? objectFromStore
731-
: map(relatedEntityRelationship, (Neo4jPersistentEntity<?>) relationshipDescription.getRelationshipPropertiesEntity(), valueEntry, relationshipDescription, relationshipsFromResult, nodesFromResult);
733+
: map(relatedEntityRelationship, relationshipPropertiesEntity, relationshipPropertiesEntity, valueEntry, relationshipDescription, relationshipsFromResult, nodesFromResult);
732734
}
733735

734736
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)