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
This commit is contained in:
Michael Simons
2024-08-20 16:43:15 +02:00
parent af59d65955
commit 79d8aafad1
4 changed files with 31 additions and 4 deletions

View File

@@ -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()));

View File

@@ -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")

View File

@@ -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);
}

View File

@@ -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);
}
}