Skip to content

Commit 6ecc340

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 9cc5a85 commit 6ecc340

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;
@@ -223,7 +224,10 @@ private QueryFragments createQueryFragments(@Nullable Condition condition, Sort
223224
queryFragments.setReturnExpression(Functions.count(Constants.NAME_OF_TYPED_ROOT_NODE.apply(nodeDescription)), true);
224225
} else {
225226

226-
var theSort = pagingParameter.getSort().and(sort);
227+
var theSort = pagingParameter.getSort();
228+
if (!Objects.equals(theSort, sort)) {
229+
theSort = theSort.and(sort);
230+
}
227231

228232
if (pagingParameter.isUnpaged() && scrollPosition == null && maxResults != null) {
229233
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
@@ -44,6 +44,7 @@
4444
import org.junit.jupiter.api.Tag;
4545
import org.junit.jupiter.api.Test;
4646
import org.junit.jupiter.api.TestMethodOrder;
47+
import org.junit.jupiter.api.extension.ExtendWith;
4748
import org.neo4j.cypherdsl.core.Condition;
4849
import org.neo4j.cypherdsl.core.Cypher;
4950
import org.neo4j.cypherdsl.core.Node;
@@ -182,19 +183,24 @@
182183
import org.springframework.data.neo4j.repository.config.EnableNeo4jRepositories;
183184
import org.springframework.data.neo4j.repository.query.QueryFragmentsAndParameters;
184185
import org.springframework.data.neo4j.test.BookmarkCapture;
186+
import org.springframework.data.neo4j.test.LogbackCapture;
187+
import org.springframework.data.neo4j.test.LogbackCapturingExtension;
185188
import org.springframework.data.neo4j.test.Neo4jImperativeTestConfiguration;
186189
import org.springframework.data.neo4j.test.Neo4jIntegrationTest;
187190
import org.springframework.transaction.PlatformTransactionManager;
188191
import org.springframework.transaction.annotation.EnableTransactionManagement;
189192
import org.springframework.transaction.annotation.Transactional;
190193

194+
import ch.qos.logback.classic.Level;
195+
191196
/**
192197
* @author Michael J. Simons
193198
* @soundtrack Sodom - Sodom
194199
*/
195200
@Neo4jIntegrationTest
196201
@DisplayNameGeneration(SimpleDisplayNameGeneratorWithTags.class)
197202
@TestMethodOrder(MethodOrderer.DisplayName.class)
203+
@ExtendWith(LogbackCapturingExtension.class)
198204
class IssuesIT extends TestBase {
199205

200206
// GH-2210
@@ -1316,6 +1322,20 @@ void shouldSupportGeoResultWithSelfRef(@Autowired LocatedNodeWithSelfRefReposito
13161322
assertSupportedGeoResultBehavior(repository);
13171323
}
13181324

1325+
@Test
1326+
@Tag("GH-2940")
1327+
void shouldNotGenerateDuplicateOrder(@Autowired LocatedNodeRepository repository, LogbackCapture logbackCapture) {
1328+
1329+
try {
1330+
logbackCapture.addLogger("org.springframework.data.neo4j.cypher", Level.DEBUG);
1331+
var nodes = repository.findAllByName("NEO4J_HQ", PageRequest.of(0, 10, Sort.by(Sort.Order.asc("name"))));
1332+
assertThat(nodes).isNotEmpty();
1333+
assertThat(logbackCapture.getFormattedMessages()).noneMatch(l -> l.contains("locatedNode.name, locatedNode.name"));
1334+
} finally {
1335+
logbackCapture.resetLogLevel();
1336+
}
1337+
}
1338+
13191339
@Configuration
13201340
@EnableTransactionManagement
13211341
@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)