Skip to content

Commit 85ff14f

Browse files
committed
Fix tests and minor bugs
1 parent 9b15f15 commit 85ff14f

File tree

2 files changed

+18
-11
lines changed

2 files changed

+18
-11
lines changed

src/main/java/redis/clients/jedis/search/aggr/AggregateIterator.java

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,11 @@
2727
* try (AggregateIterator iterator = new AggregateIterator(provider, "myindex", aggr)) {
2828
* while (iterator.hasNext()) {
2929
* AggregationResult batch = iterator.next();
30+
*
31+
* if (batch.isEmpty()) {
32+
* break; // FT.AGGREGATE returned empty result set
33+
* }
34+
*
3035
* // Process batch - access rows via batch.getRows()
3136
*
3237
* // Optionally terminate early and free server resources
@@ -72,9 +77,6 @@ public AggregateIterator(ConnectionProvider connectionProvider, String indexName
7277

7378
@Override
7479
public boolean hasNext() {
75-
// If aggrCommandResult is not null, we have initial results from FT.AGGREGATE that haven't been returned yet
76-
// For subsequent batches, check if there are more cursor results available
77-
// If cursor has been removed (cursorId <= 0), no more results are available
7880
return aggrCommandResult != null || cursorId > 0;
7981
}
8082

@@ -111,6 +113,8 @@ public Long getCursorId() {
111113

112114
@Override
113115
public void remove() {
116+
aggrCommandResult = null;
117+
114118
if (cursorId == null || cursorId <= 0) {
115119
// Cursor is already closed or not initialized, nothing to do
116120
return;

src/test/java/redis/clients/jedis/search/aggr/AggregateIteratorTest.java renamed to src/test/java/redis/clients/jedis/modules/search/AggregateIteratorTest.java

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
package redis.clients.jedis.search.aggr;
1+
package redis.clients.jedis.modules.search;
22

33
import static org.junit.jupiter.api.Assertions.assertEquals;
44
import static org.junit.jupiter.api.Assertions.assertFalse;
@@ -28,7 +28,7 @@
2828
import redis.clients.jedis.search.Document;
2929
import redis.clients.jedis.search.IndexOptions;
3030
import redis.clients.jedis.search.Schema;
31-
31+
import redis.clients.jedis.search.aggr.*;
3232

3333
@ParameterizedClass
3434
@MethodSource("redis.clients.jedis.commands.CommandsTestsParameters#respVersions")
@@ -190,7 +190,8 @@ public void testAggregateIteratorFirstBatchReturnsInitialResults() {
190190
assertEquals(10, rows.get(1).getLong("sum"));
191191

192192
// Should be no more batches since we got all results in first batch
193-
assertFalse(iterator.hasNext());
193+
AggregationResult secondBatch = iterator.next();
194+
assertEquals(0, secondBatch.getRows().size());
194195
}
195196
}
196197

@@ -210,7 +211,7 @@ public void testAggregateIteratorEmptyResult() {
210211
// Test the iterator with empty results using the integrated method
211212
try (AggregateIterator iterator = client.ftAggregateIterator(index, aggr)) {
212213
// Should have no results
213-
assertFalse(iterator.hasNext());
214+
assertTrue(iterator.next().isEmpty());
214215
}
215216
}
216217

@@ -269,11 +270,12 @@ public void testAggregateIteratorRemoveBeforeNext() {
269270
client.ftCreate(index, IndexOptions.defaultOptions(), sc);
270271

271272
addDocument(new Document("data1").set("name", "abc").set("count", 10));
273+
addDocument(new Document("data2").set("name", "cde").set("count", 8));
272274

273275
// Create aggregation with cursor
274276
AggregationBuilder aggr = new AggregationBuilder()
275277
.groupBy("@name", Reducers.sum("@count").as("sum"))
276-
.cursor(10, 10000);
278+
.cursor(1, 10000);
277279

278280
// Test calling remove() before next() - should work since cursor is initialized
279281
try (AggregateIterator iterator = client.ftAggregateIterator(index, aggr)) {
@@ -324,11 +326,12 @@ public void testAggregateIteratorRemoveMultipleTimes() {
324326
client.ftCreate(index, IndexOptions.defaultOptions(), sc);
325327

326328
addDocument(new Document("data1").set("name", "abc").set("count", 10));
329+
addDocument(new Document("data2").set("name", "cde").set("count", 3));
327330

328331
// Create aggregation with cursor
329332
AggregationBuilder aggr = new AggregationBuilder()
330333
.groupBy("@name", Reducers.sum("@count").as("sum"))
331-
.cursor(10, 10000);
334+
.cursor(1, 10000);
332335

333336
// Test calling remove() multiple times
334337
try (AggregateIterator iterator = client.ftAggregateIterator(index, aggr)) {
@@ -337,11 +340,11 @@ public void testAggregateIteratorRemoveMultipleTimes() {
337340
// First remove should work
338341
iterator.remove();
339342
assertFalse(iterator.hasNext());
340-
assertEquals(Long.valueOf(-1), iterator.getCursorId());
343+
assertEquals(-1L, iterator.getCursorId());
341344

342345
// Second remove should not throw exception, just return silently
343346
iterator.remove(); // Should not throw
344-
assertEquals(Long.valueOf(-1), iterator.getCursorId());
347+
assertEquals(-1L, iterator.getCursorId());
345348
}
346349
}
347350
}

0 commit comments

Comments
 (0)