From 8bc66fcd79c8fa89187fba7b1bf454cc2ce5a35b Mon Sep 17 00:00:00 2001 From: Gerrit Meier Date: Thu, 4 Apr 2024 16:02:32 +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 | 4 +-- .../integration/issues/gh2289/SkuRO.java | 13 ++++++++ .../integration/issues/gh2474/CityModel.java | 13 ++++++++ .../shared/common/ScrollingEntity.java | 5 ++++ .../query/CypherAdapterUtilsTest.java | 22 ++++++++++++++ 7 files changed, 104 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 70941baa2..f25f4b2c5 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 @@ -281,10 +281,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)); } @@ -736,6 +738,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 3540c5e53..7c633b061 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 @@ -102,8 +102,8 @@ 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 60f8d12a8..0b19550e1 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 @@ -16,6 +16,7 @@ package org.springframework.data.neo4j.integration.issues.gh2289; 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; @@ -23,6 +24,7 @@ import org.springframework.data.neo4j.core.schema.Property; import org.springframework.data.neo4j.core.schema.Relationship; import java.util.HashSet; +import java.util.Map; import java.util.Set; /** @@ -48,6 +50,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; @@ -99,6 +104,14 @@ public class SkuRO { this.rangeRelationsIn = rangeRelationsIn; } + public Map getComposite() { + return composite; + } + + public void setComposite(Map composite) { + this.composite = composite; + } + public boolean equals(final Object o) { if (o == this) { return true; 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 7b0c396b5..aa23dc1fe 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 @@ -15,6 +15,7 @@ */ package org.springframework.data.neo4j.integration.issues.gh2474; +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; @@ -23,6 +24,7 @@ import org.springframework.data.neo4j.core.schema.Relationship; import java.util.ArrayList; import java.util.List; +import java.util.Map; import java.util.UUID; /** @@ -48,6 +50,9 @@ public class CityModel { @Property("exotic.property") private String exoticProperty; + @CompositeProperty + private Map compositeProperty; + public CityModel() { } @@ -99,6 +104,14 @@ public class CityModel { this.exoticProperty = exoticProperty; } + public Map getCompositeProperty() { + return compositeProperty; + } + + public void setCompositeProperty(Map compositeProperty) { + this.compositeProperty = compositeProperty; + } + public boolean equals(final Object o) { if (o == this) { return true; 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."); + } }