From fa6a1aaede8373dc5041326f1163c51b53d8de55 Mon Sep 17 00:00:00 2001 From: Michael Simons Date: Mon, 9 Nov 2020 11:47:18 +0100 Subject: [PATCH] DATAGRAPH-1424 - Make derived queries target the correct path with multiple relationships to the same target node. --- .../repository/query/CypherQueryCreator.java | 125 +++++++++--------- .../imperative/RelationshipsIT.java | 35 +++++ .../shared/Multiple1O1Relationships.java | 71 ++++++++++ 3 files changed, 167 insertions(+), 64 deletions(-) create mode 100644 src/test/java/org/springframework/data/neo4j/integration/shared/Multiple1O1Relationships.java diff --git a/src/main/java/org/springframework/data/neo4j/repository/query/CypherQueryCreator.java b/src/main/java/org/springframework/data/neo4j/repository/query/CypherQueryCreator.java index 46d08ffa7..2d28b26bb 100644 --- a/src/main/java/org/springframework/data/neo4j/repository/query/CypherQueryCreator.java +++ b/src/main/java/org/springframework/data/neo4j/repository/query/CypherQueryCreator.java @@ -147,17 +147,15 @@ final class CypherQueryCreator extends AbstractQueryCreator> propertyPathList; + private final PersistentPropertyPath propertyPath; PropertyPathWrapper(int index, PersistentPropertyPath propertyPath) { this.index = index; - propertyPathList = (List>) propertyPath.toList(); - this.leafProperty = (Neo4jPersistentProperty) propertyPath.getRequiredLeafProperty(); + this.propertyPath = propertyPath; } - private Neo4jPersistentProperty getLeafProperty() { - return this.leafProperty; + public PersistentPropertyPath getPropertyPath() { + return propertyPath; } private String getNodeName() { @@ -168,18 +166,11 @@ final class CypherQueryCreator extends AbstractQueryCreator persistentProperty) { - - // size - 1 = last index - // size - 2 = property on last node - // size - 3 = last node itself - return propertyPathList.indexOf(persistentProperty) > propertyPathList.size() - 3; - } - private ExposesRelationships createRelationshipChain(ExposesRelationships existingRelationshipChain) { ExposesRelationships cypherRelationship = existingRelationshipChain; - for (PersistentProperty persistentProperty : propertyPathList) { + int cnt = 0; + for (PersistentProperty persistentProperty : propertyPath) { RelationshipDescription relationshipDescription = (RelationshipDescription) persistentProperty.getAssociation(); @@ -193,7 +184,10 @@ final class CypherQueryCreator extends AbstractQueryCreator targetEntity = relationshipDescription.getTarget(); Node relatedNode = Cypher.node(targetEntity.getPrimaryLabel(), targetEntity.getAdditionalLabels()); - boolean lastNode = isLastNode(persistentProperty); + // length - 1 = last index + // length - 2 = property on last node + // length - 3 = last node itself + boolean lastNode = cnt++ > (propertyPath.getLength() - 3); if (lastNode || hasTargetNode) { relatedNode = relatedNode.named(getNodeName()); } @@ -229,7 +223,7 @@ final class CypherQueryCreator extends AbstractQueryCreator 1; + return this.propertyPath.getLength() > 1; } } @@ -326,69 +320,69 @@ final class CypherQueryCreator extends AbstractQueryCreator path, String parameterName, boolean ignoreCase) { String regexOptions = ignoreCase ? "(?i)" : ""; - return toCypherProperty(persistentProperty, false).matches( + return toCypherProperty(path, false).matches( Cypher.literalOf(regexOptions + ".*").concat(Cypher.parameter(parameterName)).concat(Cypher.literalOf(".*"))); } - private Condition betweenCondition(Neo4jPersistentProperty persistentProperty, Iterator actualParameters, + private Condition betweenCondition(PersistentPropertyPath path, Iterator actualParameters, boolean ignoreCase) { - Parameter lowerBoundOrRange = nextRequiredParameter(actualParameters, persistentProperty); + Neo4jPersistentProperty leafProperty = path.getLeafProperty(); + Parameter lowerBoundOrRange = nextRequiredParameter(actualParameters, leafProperty); - Expression property = toCypherProperty(persistentProperty, ignoreCase); + Expression property = toCypherProperty(path, ignoreCase); if (lowerBoundOrRange.value instanceof Range) { return createRangeConditionForProperty(property, lowerBoundOrRange); } else { - Parameter upperBound = nextRequiredParameter(actualParameters, persistentProperty); + Parameter upperBound = nextRequiredParameter(actualParameters, leafProperty); return property.gte(toCypherParameter(lowerBoundOrRange, ignoreCase)) .and(property.lte(toCypherParameter(upperBound, ignoreCase))); } } - private Condition createNearCondition(Neo4jPersistentProperty persistentProperty, Iterator actualParameters) { + private Condition createNearCondition(PersistentPropertyPath path, Iterator actualParameters) { - Parameter p1 = nextRequiredParameter(actualParameters, persistentProperty); - Optional p2 = nextOptionalParameter(actualParameters, persistentProperty); + Neo4jPersistentProperty leafProperty = path.getRequiredLeafProperty(); + Parameter p1 = nextRequiredParameter(actualParameters, leafProperty); + Optional p2 = nextOptionalParameter(actualParameters, leafProperty); Expression referencePoint; @@ -456,7 +452,7 @@ final class CypherQueryCreator extends AbstractQueryCreator p.hasValueOfType(Distance.class)).isPresent()) { return distanceFunction.lte(toCypherParameter(other.get(), false)); @@ -473,15 +469,16 @@ final class CypherQueryCreator extends AbstractQueryCreator actualParameters) { - Parameter area = nextRequiredParameter(actualParameters, persistentProperty); + private Condition createWithinCondition(PersistentPropertyPath path, Iterator actualParameters) { + + Neo4jPersistentProperty leafProperty = path.getRequiredLeafProperty(); + Parameter area = nextRequiredParameter(actualParameters, leafProperty); if (area.hasValueOfType(Circle.class)) { // We don't know the CRS of the point, so we assume the same as the reference toCypherProperty Expression referencePoint = point(Cypher.mapOf("x", createCypherParameter(area.nameOrIndex + ".x", false), "y", createCypherParameter(area.nameOrIndex + ".y", false), "srid", - Cypher.property(toCypherProperty(persistentProperty, false), "srid"))); - Expression distanceFunction = Functions.distance(toCypherProperty(persistentProperty, false), referencePoint); + Cypher.property(toCypherProperty(path, false), "srid"))); + Expression distanceFunction = Functions.distance(toCypherProperty(path, false), referencePoint); return distanceFunction.lte(createCypherParameter(area.nameOrIndex + ".radius", false)); } else if (area.hasValueOfType(BoundingBox.class) || area.hasValueOfType(Box.class)) { Expression llx = createCypherParameter(area.nameOrIndex + ".llx", false); @@ -489,8 +486,8 @@ final class CypherQueryCreator extends AbstractQueryCreator path, boolean addToLower) { - Neo4jPersistentEntity owner = (Neo4jPersistentEntity) persistentProperty.getOwner(); + Neo4jPersistentProperty leafProperty = path.getRequiredLeafProperty(); + Neo4jPersistentEntity owner = (Neo4jPersistentEntity) leafProperty.getOwner(); Expression expression; if (owner.equals(this.nodeDescription)) { - expression = Cypher.property(Constants.NAME_OF_ROOT_NODE, persistentProperty.getPropertyName()); + expression = Cypher.property(Constants.NAME_OF_ROOT_NODE, leafProperty.getPropertyName()); } else { PropertyPathWrapper propertyPathWrapper = propertyPathWrappers.stream() - .filter(rp -> rp.getLeafProperty().equals(persistentProperty)).findFirst().get(); - + .filter(rp -> rp.getPropertyPath().equals(path)).findFirst().get(); String cypherElementName; // this "entity" is a representation of a relationship with properties if (owner.isRelationshipPropertiesEntity()) { @@ -549,7 +546,7 @@ final class CypherQueryCreator extends AbstractQueryCreator (p1)\n" + + "CREATE (m1) - [:REL_2] -> (p2)\n" + + "CREATE (m2) - [:REL_1] -> (p1)\n" + + "CREATE (m2) - [:REL_2] -> (p3)"); + tx.commit(); + } + + List objects = repository.findAllByPerson1NameAndPerson2Name("val1", "val2"); + + assertThat(objects).hasSize(1).first() + .satisfies(m -> { + assertThat(m.getName()).isEqualTo("m1"); + assertThat(m.getPerson1().getName()).isEqualTo("val1"); + assertThat(m.getPerson2().getName()).isEqualTo("val2"); + }); + } + interface MultipleRelationshipsThingRepository extends CrudRepository {} + interface Multiple1O1RelationshipsRepository extends CrudRepository { + + List findAllByPerson1NameAndPerson2Name(String name1, String name2); + } + @Configuration @EnableTransactionManagement @EnableNeo4jRepositories(considerNestedRepositories = true) diff --git a/src/test/java/org/springframework/data/neo4j/integration/shared/Multiple1O1Relationships.java b/src/test/java/org/springframework/data/neo4j/integration/shared/Multiple1O1Relationships.java new file mode 100644 index 000000000..d4367d1c2 --- /dev/null +++ b/src/test/java/org/springframework/data/neo4j/integration/shared/Multiple1O1Relationships.java @@ -0,0 +1,71 @@ +/* + * Copyright 2011-2020 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.data.neo4j.integration.shared; + +import org.springframework.data.neo4j.core.schema.GeneratedValue; +import org.springframework.data.neo4j.core.schema.Id; +import org.springframework.data.neo4j.core.schema.Node; +import org.springframework.data.neo4j.core.schema.Relationship; + +/** + * @author Michael J. Simons + * @soundtrack Dream Theater - Scenes From A Memory + */ +@Node +public class Multiple1O1Relationships { + + @Id @GeneratedValue private Long id; + + private String name; + + @Relationship("REL_1") + private AltPerson person1; + + @Relationship("REL_2") + private AltPerson person2; + + public Long getId() { + return id; + } + + public void setId(Long id) { + this.id = id; + } + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + + public AltPerson getPerson1() { + return person1; + } + + public void setPerson1(AltPerson person1) { + this.person1 = person1; + } + + public AltPerson getPerson2() { + return person2; + } + + public void setPerson2(AltPerson person2) { + this.person2 = person2; + } +}