From 8e3152fc9c83d4dbeeaa5d6845537866f44b75c5 Mon Sep 17 00:00:00 2001
From: Michael Simons <michael@simons.ac>
Date: Tue, 20 Aug 2024 16:43:15 +0200
Subject: [PATCH] fix: Check if the pageable sort already contains the
 additional sort.

If it does or is equal to, don't add it a second time.

Fixes #2940
---
 .../repository/query/CypherQueryCreator.java  |  6 +++++-
 .../neo4j/integration/issues/IssuesIT.java    | 20 +++++++++++++++++++
 .../issues/gh2908/LocatedNodeRepository.java  |  4 ++++
 .../data/neo4j/test/LogbackCapture.java       |  5 ++---
 4 files changed, 31 insertions(+), 4 deletions(-)

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 fdc73ed62..9097f0c05 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
@@ -23,6 +23,7 @@ import java.util.Iterator;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Optional;
 import java.util.Queue;
 import java.util.concurrent.atomic.AtomicInteger;
@@ -220,7 +221,10 @@ final class CypherQueryCreator extends AbstractQueryCreator<QueryFragmentsAndPar
 			queryFragments.setReturnExpression(Cypher.count(Constants.NAME_OF_TYPED_ROOT_NODE.apply(nodeDescription)), true);
 		} else {
 
-			var theSort = pagingParameter.getSort().and(sort);
+			var theSort = pagingParameter.getSort();
+			if (!Objects.equals(theSort, sort)) {
+				theSort = theSort.and(sort);
+			}
 
 			if (pagingParameter.isUnpaged() && scrollPosition == null && maxResults != null) {
 				queryFragments.setLimit(limitModifier.apply(maxResults.intValue()));
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 61761e1a1..f2b9e7d35 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
@@ -47,6 +47,7 @@ import org.junit.jupiter.api.RepeatedTest;
 import org.junit.jupiter.api.Tag;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.TestMethodOrder;
+import org.junit.jupiter.api.extension.ExtendWith;
 import org.neo4j.cypherdsl.core.Condition;
 import org.neo4j.cypherdsl.core.Cypher;
 import org.neo4j.cypherdsl.core.LabelExpression;
@@ -200,12 +201,16 @@ import org.springframework.data.neo4j.integration.misc.ConcreteImplementationTwo
 import org.springframework.data.neo4j.repository.config.EnableNeo4jRepositories;
 import org.springframework.data.neo4j.repository.query.QueryFragmentsAndParameters;
 import org.springframework.data.neo4j.test.BookmarkCapture;
+import org.springframework.data.neo4j.test.LogbackCapture;
+import org.springframework.data.neo4j.test.LogbackCapturingExtension;
 import org.springframework.data.neo4j.test.Neo4jImperativeTestConfiguration;
 import org.springframework.data.neo4j.test.Neo4jIntegrationTest;
 import org.springframework.transaction.PlatformTransactionManager;
 import org.springframework.transaction.annotation.EnableTransactionManagement;
 import org.springframework.transaction.annotation.Transactional;
 
+import ch.qos.logback.classic.Level;
+
 /**
  * @author Michael J. Simons
  * @soundtrack Sodom - Sodom
@@ -213,6 +218,7 @@ import org.springframework.transaction.annotation.Transactional;
 @Neo4jIntegrationTest
 @DisplayNameGeneration(SimpleDisplayNameGeneratorWithTags.class)
 @TestMethodOrder(MethodOrderer.DisplayName.class)
+@ExtendWith(LogbackCapturingExtension.class)
 class IssuesIT extends TestBase {
 
 	// GH-2210
@@ -1657,6 +1663,20 @@ class IssuesIT extends TestBase {
 		assertSupportedGeoResultBehavior(repository);
 	}
 
+	@Test
+	@Tag("GH-2940")
+	void shouldNotGenerateDuplicateOrder(@Autowired LocatedNodeRepository repository, LogbackCapture logbackCapture) {
+
+		try {
+			logbackCapture.addLogger("org.springframework.data.neo4j.cypher", Level.DEBUG);
+			var nodes = repository.findAllByName("NEO4J_HQ", PageRequest.of(0, 10, Sort.by(Sort.Order.asc("name"))));
+			assertThat(nodes).isNotEmpty();
+			assertThat(logbackCapture.getFormattedMessages()).noneMatch(l -> l.contains("locatedNode.name, locatedNode.name"));
+		} finally {
+			logbackCapture.resetLogLevel();
+		}
+	}
+
 	@Configuration
 	@EnableTransactionManagement
 	@EnableNeo4jRepositories(namedQueriesLocation = "more-custom-queries.properties")
diff --git a/src/test/java/org/springframework/data/neo4j/integration/issues/gh2908/LocatedNodeRepository.java b/src/test/java/org/springframework/data/neo4j/integration/issues/gh2908/LocatedNodeRepository.java
index 0e0d19c54..e948167b6 100644
--- a/src/test/java/org/springframework/data/neo4j/integration/issues/gh2908/LocatedNodeRepository.java
+++ b/src/test/java/org/springframework/data/neo4j/integration/issues/gh2908/LocatedNodeRepository.java
@@ -15,6 +15,8 @@
  */
 package org.springframework.data.neo4j.integration.issues.gh2908;
 
+import org.springframework.data.domain.Page;
+import org.springframework.data.domain.PageRequest;
 import org.springframework.data.neo4j.repository.Neo4jRepository;
 
 /**
@@ -22,4 +24,6 @@ import org.springframework.data.neo4j.repository.Neo4jRepository;
  * @author Michael J. Simons
  */
 public interface LocatedNodeRepository extends HasNameAndPlaceRepository<LocatedNode>, Neo4jRepository<LocatedNode, String> {
+
+	Page<LocatedNode> findAllByName(String whatever, PageRequest name);
 }
diff --git a/src/test/java/org/springframework/data/neo4j/test/LogbackCapture.java b/src/test/java/org/springframework/data/neo4j/test/LogbackCapture.java
index 998af5f84..12c1b43c5 100644
--- a/src/test/java/org/springframework/data/neo4j/test/LogbackCapture.java
+++ b/src/test/java/org/springframework/data/neo4j/test/LogbackCapture.java
@@ -23,7 +23,6 @@ import ch.qos.logback.core.read.ListAppender;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
-import java.util.stream.Collectors;
 
 import org.junit.jupiter.api.extension.ExtensionContext;
 
@@ -54,7 +53,7 @@ public final class LogbackCapture implements ExtensionContext.Store.CloseableRes
 	}
 
 	public List<String> getFormattedMessages() {
-		return listAppender.list.stream().map(e -> e.getFormattedMessage()).collect(Collectors.toList());
+		return listAppender.list.stream().map(ILoggingEvent::getFormattedMessage).toList();
 	}
 
 	void start() {
@@ -75,6 +74,6 @@ public final class LogbackCapture implements ExtensionContext.Store.CloseableRes
 	}
 
 	public void resetLogLevel() {
-		this.additionalLoggers.entrySet().forEach(entry -> entry.getKey().setLevel(entry.getValue()));
+		this.additionalLoggers.forEach(Logger::setLevel);
 	}
 }