Skip to content

Commit 8e3152f

Browse files
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
1 parent f1fb0bf commit 8e3152f

File tree

4 files changed

+31
-4
lines changed

4 files changed

+31
-4
lines changed

Diff for: src/main/java/org/springframework/data/neo4j/repository/query/CypherQueryCreator.java

+5-1
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import java.util.LinkedList;
2424
import java.util.List;
2525
import java.util.Map;
26+
import java.util.Objects;
2627
import java.util.Optional;
2728
import java.util.Queue;
2829
import java.util.concurrent.atomic.AtomicInteger;
@@ -220,7 +221,10 @@ private QueryFragments createQueryFragments(@Nullable Condition condition, Sort
220221
queryFragments.setReturnExpression(Cypher.count(Constants.NAME_OF_TYPED_ROOT_NODE.apply(nodeDescription)), true);
221222
} else {
222223

223-
var theSort = pagingParameter.getSort().and(sort);
224+
var theSort = pagingParameter.getSort();
225+
if (!Objects.equals(theSort, sort)) {
226+
theSort = theSort.and(sort);
227+
}
224228

225229
if (pagingParameter.isUnpaged() && scrollPosition == null && maxResults != null) {
226230
queryFragments.setLimit(limitModifier.apply(maxResults.intValue()));

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

+20
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747
import org.junit.jupiter.api.Tag;
4848
import org.junit.jupiter.api.Test;
4949
import org.junit.jupiter.api.TestMethodOrder;
50+
import org.junit.jupiter.api.extension.ExtendWith;
5051
import org.neo4j.cypherdsl.core.Condition;
5152
import org.neo4j.cypherdsl.core.Cypher;
5253
import org.neo4j.cypherdsl.core.LabelExpression;
@@ -200,19 +201,24 @@
200201
import org.springframework.data.neo4j.repository.config.EnableNeo4jRepositories;
201202
import org.springframework.data.neo4j.repository.query.QueryFragmentsAndParameters;
202203
import org.springframework.data.neo4j.test.BookmarkCapture;
204+
import org.springframework.data.neo4j.test.LogbackCapture;
205+
import org.springframework.data.neo4j.test.LogbackCapturingExtension;
203206
import org.springframework.data.neo4j.test.Neo4jImperativeTestConfiguration;
204207
import org.springframework.data.neo4j.test.Neo4jIntegrationTest;
205208
import org.springframework.transaction.PlatformTransactionManager;
206209
import org.springframework.transaction.annotation.EnableTransactionManagement;
207210
import org.springframework.transaction.annotation.Transactional;
208211

212+
import ch.qos.logback.classic.Level;
213+
209214
/**
210215
* @author Michael J. Simons
211216
* @soundtrack Sodom - Sodom
212217
*/
213218
@Neo4jIntegrationTest
214219
@DisplayNameGeneration(SimpleDisplayNameGeneratorWithTags.class)
215220
@TestMethodOrder(MethodOrderer.DisplayName.class)
221+
@ExtendWith(LogbackCapturingExtension.class)
216222
class IssuesIT extends TestBase {
217223

218224
// GH-2210
@@ -1657,6 +1663,20 @@ void shouldSupportGeoResultWithSelfRef(@Autowired LocatedNodeWithSelfRefReposito
16571663
assertSupportedGeoResultBehavior(repository);
16581664
}
16591665

1666+
@Test
1667+
@Tag("GH-2940")
1668+
void shouldNotGenerateDuplicateOrder(@Autowired LocatedNodeRepository repository, LogbackCapture logbackCapture) {
1669+
1670+
try {
1671+
logbackCapture.addLogger("org.springframework.data.neo4j.cypher", Level.DEBUG);
1672+
var nodes = repository.findAllByName("NEO4J_HQ", PageRequest.of(0, 10, Sort.by(Sort.Order.asc("name"))));
1673+
assertThat(nodes).isNotEmpty();
1674+
assertThat(logbackCapture.getFormattedMessages()).noneMatch(l -> l.contains("locatedNode.name, locatedNode.name"));
1675+
} finally {
1676+
logbackCapture.resetLogLevel();
1677+
}
1678+
}
1679+
16601680
@Configuration
16611681
@EnableTransactionManagement
16621682
@EnableNeo4jRepositories(namedQueriesLocation = "more-custom-queries.properties")

Diff for: src/test/java/org/springframework/data/neo4j/integration/issues/gh2908/LocatedNodeRepository.java

+4
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,15 @@
1515
*/
1616
package org.springframework.data.neo4j.integration.issues.gh2908;
1717

18+
import org.springframework.data.domain.Page;
19+
import org.springframework.data.domain.PageRequest;
1820
import org.springframework.data.neo4j.repository.Neo4jRepository;
1921

2022
/**
2123
* Repository spotting all supported Geo* results.
2224
* @author Michael J. Simons
2325
*/
2426
public interface LocatedNodeRepository extends HasNameAndPlaceRepository<LocatedNode>, Neo4jRepository<LocatedNode, String> {
27+
28+
Page<LocatedNode> findAllByName(String whatever, PageRequest name);
2529
}

Diff for: src/test/java/org/springframework/data/neo4j/test/LogbackCapture.java

+2-3
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@
2323
import java.util.HashMap;
2424
import java.util.List;
2525
import java.util.Map;
26-
import java.util.stream.Collectors;
2726

2827
import org.junit.jupiter.api.extension.ExtensionContext;
2928

@@ -54,7 +53,7 @@ public void addLogger(String loggerName, Level level) {
5453
}
5554

5655
public List<String> getFormattedMessages() {
57-
return listAppender.list.stream().map(e -> e.getFormattedMessage()).collect(Collectors.toList());
56+
return listAppender.list.stream().map(ILoggingEvent::getFormattedMessage).toList();
5857
}
5958

6059
void start() {
@@ -75,6 +74,6 @@ public void close() {
7574
}
7675

7776
public void resetLogLevel() {
78-
this.additionalLoggers.entrySet().forEach(entry -> entry.getKey().setLevel(entry.getValue()));
77+
this.additionalLoggers.forEach(Logger::setLevel);
7978
}
8079
}

0 commit comments

Comments
 (0)