GH-2884 - Support composite property in sort.

Should work for map projection and node returns now.

Closes #2884

Author: meistermeier

src/main/java/org/springframework/data/neo4j/repository/query/CypherAdapterUtils.java

@@ -64,25 +64,43 @@ public static Function<Sort.Order, SortItem> sortAdapterFor(NodeDescription<?> n
 		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()) {
+				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'", order.getProperty()));
+				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()) {

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

@@ -278,10 +278,12 @@ protected void prepareIndividual(
 		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 @@ public void testCityModelProjectionPersistence(
 		assertThat(reloaded.getCityEmployees()).hasSize(1);
 	}
 
+	@Test
+	@Tag("GH-2884")
+	void sortByCompositeProperty(@Autowired CityModelRepository repository) {
+		Sort sort = Sort.by(Sort.Order.asc("compositeProperty.language"));
+		List<CityModel> 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<SkuRO> 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,

src/test/java/org/springframework/data/neo4j/integration/issues/TestBase.java

@@ -100,9 +100,10 @@ protected final void beforeEach(@Autowired BookmarkCapture bookmarkCapture) {
 	}
 
 	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();
 		}
 	}
 

src/test/java/org/springframework/data/neo4j/integration/issues/gh2289/SkuRO.java

@@ -20,9 +20,11 @@
 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<RangeRelationRO> rangeRelationsIn = new HashSet<>();
 
+	@CompositeProperty
+	private Map<String, Integer> composite;
+
 	public SkuRO(Long number, String name) {
 		this.number = number;
 		this.name = name;
@@ -67,4 +72,89 @@ public RangeRelationRO rangeRelationTo(SkuRO sku, double minDelta, double maxDel
 		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<RangeRelationRO> getRangeRelationsOut() {
+		return this.rangeRelationsOut;
+	}
+
+	public Set<RangeRelationRO> 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<RangeRelationRO> rangeRelationsOut) {
+		this.rangeRelationsOut = rangeRelationsOut;
+	}
+
+	public void setRangeRelationsIn(Set<RangeRelationRO> 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;
+	}
 }

src/test/java/org/springframework/data/neo4j/integration/issues/gh2474/CityModel.java

@@ -19,8 +19,10 @@
 
 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<String, String> compositeProperty;
 }

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 static void createTestDataWithoutDuplicates(QueryRunner queryRunner) {
 
 	private LocalDateTime c;
 
+	@CompositeProperty
+	private Map<String, String> basicComposite;
+
 	public UUID getId() {
 		return id;
 	}

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 @@ void shouldCombineSortKeysetProper() {
 		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.");
+	}
 }