Skip to content

Commit 53c2212

Browse files
authored
fix: add limit support for findTop/findFirst repository methods (#631)
- Add PartTree limit checking in RediSearchQuery.processPartTree() - Methods like findTop2ByOrderByNameAsc now correctly pass limit to Redis - Fixes issue where all methods returned default 10000 results - Add comprehensive tests for various limiting scenarios Fixes #630
1 parent 9c63e8e commit 53c2212

File tree

4 files changed

+185
-0
lines changed

4 files changed

+185
-0
lines changed

redis-om-spring/src/main/java/com/redis/om/spring/repository/query/RediSearchQuery.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -380,6 +380,11 @@ private void processPartTree(PartTree pt, List<String> nullParamNames, List<Stri
380380
sortBy = order.getProperty();
381381
sortAscending = order.isAscending();
382382
}
383+
384+
// Handle limiting queries (findTop, findFirst, etc.)
385+
if (pt.isLimiting()) {
386+
this.limit = pt.getMaxResults();
387+
}
383388
}
384389

385390
private List<Pair<String, QueryClause>> extractQueryFields(Class<?> type, Part part, List<PropertyPath> path) {

tests/src/test/java/com/redis/om/spring/fixtures/document/repository/CompanyRepository.java

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,4 +61,15 @@ public interface CompanyRepository extends RedisDocumentRepository<Company, Stri
6161
List<Company> findByYearFoundedGreaterThanOrderByNameDesc(int year);
6262

6363
List<Company> findByYearFoundedBetweenOrderByNameAsc(int start, int end);
64+
65+
// Methods for testing findTop/findFirst limiting functionality
66+
Optional<Company> findTopByOrderByYearFoundedAsc();
67+
68+
List<Company> findTop2ByOrderByYearFoundedAsc();
69+
70+
List<Company> findTop3ByPubliclyListedOrderByYearFoundedDesc(boolean publiclyListed);
71+
72+
Optional<Company> findFirstByOrderByNameAsc();
73+
74+
List<Company> findFirst5ByPubliclyListedOrderByNameDesc(boolean publiclyListed);
6475
}
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
package com.redis.om.spring.fixtures.document.repository;
2+
3+
import com.redis.om.spring.fixtures.document.model.Company;
4+
import com.redis.om.spring.repository.RedisDocumentRepository;
5+
6+
import java.util.List;
7+
import java.util.Optional;
8+
9+
/**
10+
* Repository interface to test findTop/findFirst limiting functionality.
11+
*/
12+
public interface CompanyRepositoryWithLimiting extends RedisDocumentRepository<Company, String> {
13+
Optional<Company> findFirstByTags(String tag);
14+
Optional<Company> findFirstByOrderByYearFoundedAsc();
15+
Optional<Company> findTopByOrderByYearFoundedDesc();
16+
List<Company> findTop2ByOrderByYearFoundedAsc();
17+
Optional<Company> findFirstByTagsOrderByYearFoundedAsc(String tag);
18+
19+
// Additional test cases for various limiting scenarios
20+
List<Company> findTop5ByOrderByNameAsc();
21+
List<Company> findFirst3ByOrderByEmailDesc();
22+
Optional<Company> findTopByTagsOrderByNameAsc(String tag);
23+
List<Company> findTop10ByYearFoundedGreaterThanOrderByNameAsc(int year);
24+
}
Lines changed: 145 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,145 @@
1+
package com.redis.om.spring.repository.query;
2+
3+
import com.redis.om.spring.AbstractBaseDocumentTest;
4+
import com.redis.om.spring.fixtures.document.model.Company;
5+
import com.redis.om.spring.fixtures.document.repository.CompanyRepositoryWithLimiting;
6+
import org.junit.jupiter.api.BeforeEach;
7+
import org.junit.jupiter.api.Test;
8+
import org.springframework.beans.factory.annotation.Autowired;
9+
import org.springframework.data.geo.Point;
10+
11+
import java.time.LocalDate;
12+
import java.util.List;
13+
import java.util.Optional;
14+
import java.util.Set;
15+
16+
import static org.assertj.core.api.Assertions.assertThat;
17+
18+
/**
19+
* Tests to verify that findTop/findFirst repository methods properly limit results.
20+
* This tests the issue where findTopByTeamOrderByDueDateAsc returns LIMIT 0 10000 instead of LIMIT 0 1.
21+
*/
22+
class FindTopLimitingTest extends AbstractBaseDocumentTest {
23+
24+
@Autowired
25+
CompanyRepositoryWithLimiting repository;
26+
27+
@BeforeEach
28+
void setup() {
29+
repository.deleteAll();
30+
31+
// Create test data
32+
Company company1 = Company.of("RedisInc", 2011, LocalDate.of(2011, 5, 1),
33+
new Point(-122.066540, 37.377690), "[email protected]");
34+
company1.setTags(Set.of("fast", "scalable", "reliable"));
35+
36+
Company company2 = Company.of("Microsoft", 1975, LocalDate.of(1975, 4, 4),
37+
new Point(-122.124500, 47.640160), "[email protected]");
38+
company2.setTags(Set.of("software", "cloud", "enterprise"));
39+
40+
Company company3 = Company.of("Tesla", 2003, LocalDate.of(2003, 7, 1),
41+
new Point(-122.145800, 37.396400), "[email protected]");
42+
company3.setTags(Set.of("electric", "automotive", "innovative"));
43+
44+
repository.saveAll(List.of(company1, company2, company3));
45+
}
46+
47+
@Test
48+
void testFindFirstByTags() {
49+
// Test findFirst with a tag filter
50+
Optional<Company> result = repository.findFirstByTags("software");
51+
52+
assertThat(result).isPresent();
53+
assertThat(result.get().getName()).isEqualTo("Microsoft");
54+
}
55+
56+
@Test
57+
void testFindFirstByOrderBy() {
58+
// Test findFirst with ordering
59+
Optional<Company> result = repository.findFirstByOrderByYearFoundedAsc();
60+
61+
assertThat(result).isPresent();
62+
assertThat(result.get().getName()).isEqualTo("Microsoft");
63+
assertThat(result.get().getYearFounded()).isEqualTo(1975);
64+
}
65+
66+
@Test
67+
void testFindTopByOrderBy() {
68+
// Test findTop with ordering
69+
Optional<Company> result = repository.findTopByOrderByYearFoundedDesc();
70+
71+
assertThat(result).isPresent();
72+
assertThat(result.get().getName()).isEqualTo("RedisInc");
73+
assertThat(result.get().getYearFounded()).isEqualTo(2011);
74+
}
75+
76+
@Test
77+
void testFindTop2ByOrderBy() {
78+
// Test findTop with explicit number
79+
List<Company> results = repository.findTop2ByOrderByYearFoundedAsc();
80+
81+
// This test will likely fail because Redis OM Spring doesn't properly parse the limit from method names
82+
assertThat(results).hasSize(2);
83+
assertThat(results.get(0).getName()).isEqualTo("Microsoft");
84+
assertThat(results.get(1).getName()).isEqualTo("Tesla");
85+
}
86+
87+
@Test
88+
void testFindFirstByTagsOrderBy() {
89+
// Test the reported issue pattern: findTopByTeamOrderByDueDateAsc
90+
Optional<Company> result = repository.findFirstByTagsOrderByYearFoundedAsc("automotive");
91+
92+
assertThat(result).isPresent();
93+
assertThat(result.get().getName()).isEqualTo("Tesla");
94+
}
95+
96+
@Test
97+
void testFindTop5ByOrderBy() {
98+
// Add more test data to verify limit 5
99+
Company company4 = Company.of("Apple", 1976, LocalDate.of(1976, 4, 1),
100+
new Point(-122.030000, 37.330000), "[email protected]");
101+
Company company5 = Company.of("Amazon", 1994, LocalDate.of(1994, 7, 5),
102+
new Point(-122.329200, 47.614900), "[email protected]");
103+
Company company6 = Company.of("Google", 1998, LocalDate.of(1998, 9, 4),
104+
new Point(-122.084000, 37.422000), "[email protected]");
105+
repository.saveAll(List.of(company4, company5, company6));
106+
107+
List<Company> results = repository.findTop5ByOrderByNameAsc();
108+
109+
assertThat(results).hasSize(5);
110+
assertThat(results.get(0).getName()).isEqualTo("Amazon");
111+
assertThat(results.get(1).getName()).isEqualTo("Apple");
112+
assertThat(results.get(2).getName()).isEqualTo("Google");
113+
assertThat(results.get(3).getName()).isEqualTo("Microsoft");
114+
assertThat(results.get(4).getName()).isEqualTo("RedisInc");
115+
}
116+
117+
@Test
118+
void testFindFirst3ByOrderBy() {
119+
List<Company> results = repository.findFirst3ByOrderByEmailDesc();
120+
121+
assertThat(results).hasSize(3);
122+
assertThat(results.get(0).getEmail()).isEqualTo("[email protected]");
123+
assertThat(results.get(1).getEmail()).isEqualTo("[email protected]");
124+
assertThat(results.get(2).getEmail()).isEqualTo("[email protected]");
125+
}
126+
127+
@Test
128+
void testFindTopByTagsOrderBy() {
129+
Optional<Company> result = repository.findTopByTagsOrderByNameAsc("software");
130+
131+
assertThat(result).isPresent();
132+
assertThat(result.get().getName()).isEqualTo("Microsoft");
133+
}
134+
135+
@Test
136+
void testFindTop10WithCriteria() {
137+
// This should return only the companies founded after 2000, limited to 10
138+
List<Company> results = repository.findTop10ByYearFoundedGreaterThanOrderByNameAsc(2000);
139+
140+
assertThat(results).hasSize(2); // Only RedisInc (2011) and Tesla (2003) match
141+
assertThat(results.get(0).getName()).isEqualTo("RedisInc");
142+
assertThat(results.get(1).getName()).isEqualTo("Tesla");
143+
}
144+
145+
}

0 commit comments

Comments
 (0)