From 19af4d94448d0bca7cd1688f5e331a4080fa2daa Mon Sep 17 00:00:00 2001 From: Gerrit Meier Date: Tue, 9 Apr 2024 17:29:53 +0200 Subject: [PATCH] GH-2884 - Support composite property in sort. Should work for map projection and node returns now. Closes #2884 --- .../repository/query/CypherAdapterUtils.java | 30 +++++-- .../neo4j/integration/issues/IssuesIT.java | 25 ++++++ .../neo4j/integration/issues/TestBase.java | 5 +- .../integration/issues/gh2289/SkuRO.java | 90 +++++++++++++++++++ .../integration/issues/gh2474/CityModel.java | 5 ++ .../shared/common/ScrollingEntity.java | 5 ++ .../query/CypherAdapterUtilsTest.java | 22 +++++ 7 files changed, 174 insertions(+), 8 deletions(-) diff --git a/src/main/java/org/springframework/data/neo4j/repository/query/CypherAdapterUtils.java b/src/main/java/org/springframework/data/neo4j/repository/query/CypherAdapterUtils.java index 2cff1b5f9..336e95ed1 100644 --- a/src/main/java/org/springframework/data/neo4j/repository/query/CypherAdapterUtils.java +++ b/src/main/java/org/springframework/data/neo4j/repository/query/CypherAdapterUtils.java @@ -64,25 +64,43 @@ public final class CypherAdapterUtils { return order -> { String domainProperty = order.getProperty(); - boolean propertyIsQualified = domainProperty.contains("."); + boolean propertyIsQualifiedOrComposite = domainProperty.contains("."); SymbolicName root; - if (!propertyIsQualified) { + if (!propertyIsQualifiedOrComposite) { root = Constants.NAME_OF_TYPED_ROOT_NODE.apply(nodeDescription); } else { - int indexOfSeparator = domainProperty.indexOf("."); - root = Cypher.name(domainProperty.substring(0, indexOfSeparator)); - domainProperty = domainProperty.substring(indexOfSeparator + 1); + // need to check first if this is really a qualified name or the "qualifier" is a composite property + if (nodeDescription.getGraphProperty(domainProperty.split("\\.")[0]).isEmpty()) { + int indexOfSeparator = domainProperty.indexOf("."); + root = Cypher.name(domainProperty.substring(0, indexOfSeparator)); + domainProperty = domainProperty.substring(indexOfSeparator + 1); + } else { + root = Constants.NAME_OF_TYPED_ROOT_NODE.apply(nodeDescription); + } } var optionalGraphProperty = nodeDescription.getGraphProperty(domainProperty); + // try to resolve if this is a composite property if (optionalGraphProperty.isEmpty()) { - throw new IllegalStateException(String.format("Cannot order by the unknown graph property: '%s'", order.getProperty())); + var domainPropertyPrefix = domainProperty.split("\\.")[0]; + optionalGraphProperty = nodeDescription.getGraphProperty(domainPropertyPrefix); + } + if (optionalGraphProperty.isEmpty()) { + throw new IllegalStateException(String.format("Cannot order by the unknown graph property: '%s'", domainProperty)); } var graphProperty = optionalGraphProperty.get(); Expression expression; if (graphProperty.isInternalIdProperty()) { // Not using the id expression here, as the root will be referring to the constructed map being returned. expression = property(root, Constants.NAME_OF_INTERNAL_ID); + } else if (graphProperty.isComposite() && !domainProperty.contains(".")) { + throw new IllegalStateException(String.format("Cannot order by composite property: '%s'. Only ordering by its nested fields is allowed.", domainProperty)); + } else if (graphProperty.isComposite()) { + if (nodeDescription.containsPossibleCircles(rpp -> true)) { + expression = property(root, domainProperty); + } else { + expression = property(root, Constants.NAME_OF_ALL_PROPERTIES, domainProperty); + } } else { expression = property(root, graphProperty.getPropertyName()); if (order.isIgnoreCase()) { diff --git a/src/test/java/org/springframework/data/neo4j/integration/issues/IssuesIT.java b/src/test/java/org/springframework/data/neo4j/integration/issues/IssuesIT.java index 339e3e31b..163197eaf 100644 --- a/src/test/java/org/springframework/data/neo4j/integration/issues/IssuesIT.java +++ b/src/test/java/org/springframework/data/neo4j/integration/issues/IssuesIT.java @@ -278,10 +278,12 @@ class IssuesIT extends TestBase { CityModel aachen = new CityModel(); aachen.setName("Aachen"); aachen.setExoticProperty("Cars"); + aachen.setCompositeProperty(Map.of("language", "German")); CityModel utrecht = new CityModel(); utrecht.setName("Utrecht"); utrecht.setExoticProperty("Bikes"); + utrecht.setCompositeProperty(Map.of("language", "Dutch")); cityModelRepository.saveAll(List.of(aachen, utrecht)); } @@ -732,6 +734,29 @@ class IssuesIT extends TestBase { assertThat(reloaded.getCityEmployees()).hasSize(1); } + @Test + @Tag("GH-2884") + void sortByCompositeProperty(@Autowired CityModelRepository repository) { + Sort sort = Sort.by(Sort.Order.asc("compositeProperty.language")); + List models = repository.findAll(sort); + + assertThat(models).extracting("name").containsExactly("Utrecht", "Aachen"); + + Sort sortDesc = Sort.by(Sort.Order.desc("compositeProperty.language")); + models = repository.findAll(sortDesc); + + assertThat(models).extracting("name").containsExactly("Aachen", "Utrecht"); + } + + + @Test + @Tag("GH-2884") + void sortByCompositePropertyForCyclicDomainReturn(@Autowired SkuRORepository repository) { + List result = repository.findAll(Sort.by("composite.a")); + + assertThat(result).extracting("number").containsExactly(3L, 2L, 1L, 0L); + } + @Test @Tag("GH-2493") void saveOneShouldWork(@Autowired Driver driver, @Autowired BookmarkCapture bookmarkCapture, diff --git a/src/test/java/org/springframework/data/neo4j/integration/issues/TestBase.java b/src/test/java/org/springframework/data/neo4j/integration/issues/TestBase.java index d1c8bcf2d..a4f6a9d5b 100644 --- a/src/test/java/org/springframework/data/neo4j/integration/issues/TestBase.java +++ b/src/test/java/org/springframework/data/neo4j/integration/issues/TestBase.java @@ -100,9 +100,10 @@ abstract class TestBase { } protected static void setupGH2289(QueryRunner queryRunner) { + queryRunner.run("MATCH (s:SKU_RO) DETACH DELETE s").consume(); for (int i = 0; i < 4; ++i) { - queryRunner.run("CREATE (s:SKU_RO {number: $i, name: $n})", - Values.parameters("i", i, "n", new String(new char[] { (char) ('A' + i) }))).consume(); + queryRunner.run("CREATE (s:SKU_RO {number: $i, name: $n, `composite.a`: $a})", + Values.parameters("i", i, "n", new String(new char[] { (char) ('A' + i)}), "a", 10 - i)).consume(); } } diff --git a/src/test/java/org/springframework/data/neo4j/integration/issues/gh2289/SkuRO.java b/src/test/java/org/springframework/data/neo4j/integration/issues/gh2289/SkuRO.java index 4fedb2d01..20f649a67 100644 --- a/src/test/java/org/springframework/data/neo4j/integration/issues/gh2289/SkuRO.java +++ b/src/test/java/org/springframework/data/neo4j/integration/issues/gh2289/SkuRO.java @@ -20,9 +20,11 @@ import lombok.Getter; import lombok.Setter; import java.util.HashSet; +import java.util.Map; import java.util.Set; import org.springframework.data.annotation.ReadOnlyProperty; +import org.springframework.data.neo4j.core.schema.CompositeProperty; import org.springframework.data.neo4j.core.schema.GeneratedValue; import org.springframework.data.neo4j.core.schema.Id; import org.springframework.data.neo4j.core.schema.Node; @@ -57,6 +59,9 @@ public class SkuRO { @Relationship(type = "RANGE_RELATION_TO", direction = Relationship.Direction.INCOMING) private Set rangeRelationsIn = new HashSet<>(); + @CompositeProperty + private Map composite; + public SkuRO(Long number, String name) { this.number = number; this.name = name; @@ -67,4 +72,89 @@ public class SkuRO { rangeRelationsOut.add(relationOut); return relationOut; } + + public Long getId() { + return this.id; + } + + public Long getNumber() { + return this.number; + } + + public String getName() { + return this.name; + } + + public Set getRangeRelationsOut() { + return this.rangeRelationsOut; + } + + public Set getRangeRelationsIn() { + return this.rangeRelationsIn; + } + + public void setId(Long id) { + this.id = id; + } + + public void setNumber(Long number) { + this.number = number; + } + + public void setName(String name) { + this.name = name; + } + + public void setRangeRelationsOut(Set rangeRelationsOut) { + this.rangeRelationsOut = rangeRelationsOut; + } + + public void setRangeRelationsIn(Set rangeRelationsIn) { + this.rangeRelationsIn = rangeRelationsIn; + } + + public boolean equals(final Object o) { + if (o == this) { + return true; + } + if (!(o instanceof SkuRO)) { + return false; + } + final SkuRO other = (SkuRO) o; + if (!other.canEqual((Object) this)) { + return false; + } + final Object this$id = this.getId(); + final Object other$id = other.getId(); + if (this$id == null ? other$id != null : !this$id.equals(other$id)) { + return false; + } + final Object this$number = this.getNumber(); + final Object other$number = other.getNumber(); + if (this$number == null ? other$number != null : !this$number.equals(other$number)) { + return false; + } + final Object this$name = this.getName(); + final Object other$name = other.getName(); + if (this$name == null ? other$name != null : !this$name.equals(other$name)) { + return false; + } + return true; + } + + protected boolean canEqual(final Object other) { + return other instanceof SkuRO; + } + + public int hashCode() { + final int PRIME = 59; + int result = 1; + final Object $id = this.getId(); + result = result * PRIME + ($id == null ? 43 : $id.hashCode()); + final Object $number = this.getNumber(); + result = result * PRIME + ($number == null ? 43 : $number.hashCode()); + final Object $name = this.getName(); + result = result * PRIME + ($name == null ? 43 : $name.hashCode()); + return result; + } } diff --git a/src/test/java/org/springframework/data/neo4j/integration/issues/gh2474/CityModel.java b/src/test/java/org/springframework/data/neo4j/integration/issues/gh2474/CityModel.java index 1fb3e0bfc..fbeb1b10b 100644 --- a/src/test/java/org/springframework/data/neo4j/integration/issues/gh2474/CityModel.java +++ b/src/test/java/org/springframework/data/neo4j/integration/issues/gh2474/CityModel.java @@ -19,8 +19,10 @@ import lombok.Data; import java.util.ArrayList; import java.util.List; +import java.util.Map; import java.util.UUID; +import org.springframework.data.neo4j.core.schema.CompositeProperty; import org.springframework.data.neo4j.core.schema.GeneratedValue; import org.springframework.data.neo4j.core.schema.Id; import org.springframework.data.neo4j.core.schema.Node; @@ -50,4 +52,7 @@ public class CityModel { @Property("exotic.property") private String exoticProperty; + + @CompositeProperty + private Map compositeProperty; } diff --git a/src/test/java/org/springframework/data/neo4j/integration/shared/common/ScrollingEntity.java b/src/test/java/org/springframework/data/neo4j/integration/shared/common/ScrollingEntity.java index acea96999..b0e7226ed 100644 --- a/src/test/java/org/springframework/data/neo4j/integration/shared/common/ScrollingEntity.java +++ b/src/test/java/org/springframework/data/neo4j/integration/shared/common/ScrollingEntity.java @@ -16,10 +16,12 @@ package org.springframework.data.neo4j.integration.shared.common; import java.time.LocalDateTime; +import java.util.Map; import java.util.UUID; import org.neo4j.driver.QueryRunner; import org.springframework.data.domain.Sort; +import org.springframework.data.neo4j.core.schema.CompositeProperty; import org.springframework.data.neo4j.core.schema.GeneratedValue; import org.springframework.data.neo4j.core.schema.Id; import org.springframework.data.neo4j.core.schema.Node; @@ -78,6 +80,9 @@ public class ScrollingEntity { private LocalDateTime c; + @CompositeProperty + private Map basicComposite; + public UUID getId() { return id; } diff --git a/src/test/java/org/springframework/data/neo4j/repository/query/CypherAdapterUtilsTest.java b/src/test/java/org/springframework/data/neo4j/repository/query/CypherAdapterUtilsTest.java index 1bfcab551..b3f5ccdf0 100644 --- a/src/test/java/org/springframework/data/neo4j/repository/query/CypherAdapterUtilsTest.java +++ b/src/test/java/org/springframework/data/neo4j/repository/query/CypherAdapterUtilsTest.java @@ -16,6 +16,7 @@ package org.springframework.data.neo4j.repository.query; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatIllegalStateException; import java.time.LocalDateTime; import java.util.Map; @@ -62,4 +63,25 @@ class CypherAdapterUtilsTest { assertThat(Renderer.getRenderer(Configuration.prettyPrinting()).render(Cypher.match(Cypher.anyNode(n)).where(condition).returning(n).build())) .isEqualTo(expected); } + + @Test + void sortByCompositePropertyField() { + var mappingContext = new Neo4jMappingContext(); + var entity = mappingContext.getPersistentEntity(ScrollingEntity.class); + + var sortItem = CypherAdapterUtils.sortAdapterFor(entity).apply(Sort.Order.asc("basicComposite.blubb")); + var node = Cypher.anyNode("scrollingEntity"); + var statement = Cypher.match(node).returning(node).orderBy(sortItem).build(); + assertThat(Renderer.getDefaultRenderer().render(statement)) + .isEqualTo("MATCH (scrollingEntity) RETURN scrollingEntity ORDER BY scrollingEntity.__allProperties__.`basicComposite.blubb`"); + } + + @Test + void failOnDirectCompositePropertyAccess() { + var mappingContext = new Neo4jMappingContext(); + var entity = mappingContext.getPersistentEntity(ScrollingEntity.class); + + assertThatIllegalStateException().isThrownBy(() -> CypherAdapterUtils.sortAdapterFor(entity).apply(Sort.Order.asc("basicComposite"))) + .withMessage("Cannot order by composite property: 'basicComposite'. Only ordering by its nested fields is allowed."); + } }