Skip to content

Commit 081a434

Browse files
emilienbevclaude
andcommitted
Fix result truncation, options mutation and count() row-drain issues in findBySearch
- all() now applies a default limit of 10,000 when none is set; the FTS service default of 10 hits silently truncated results. - User-supplied SearchOptions are no longer mutated per subscription (sorts accumulated on retry/resubscribe and fluent siblings shared state); combining withOptions() with the individual with* methods now throws IllegalArgumentException. - count()/exists() drain rows before reading the metadata trailer (the SDK only completes metaData() once rows are consumed) and request limit 0 when options are owned by the template. - Stale-hit skip logging now fires on actual KV misses (findById maps DocumentNotFoundException to empty, so the previous onErrorResume was dead code); hydration uses flatMapSequential instead of sequential concatMap. - Blocking count() throws CouchbaseQueryExecutionException instead of NPE on a null count. - @Search combined with @query on one method now throws instead of silently preferring @Search. - FTS integration tests skip when the cluster lacks the SEARCH capability; index-readiness wait loop no longer hot-loops or NPEs on exceptions without a message. Signed-off-by: Emilien Bevierre <emilien.bevierre@couchbase.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 4265e6a commit 081a434

9 files changed

Lines changed: 77 additions & 24 deletions

src/main/java/org/springframework/data/couchbase/core/ExecutableFindBySearchOperation.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,9 @@ default Optional<T> first() {
9191

9292
/**
9393
* Get all matching elements, hydrated as entities via KV GET.
94+
* <p>
95+
* If no limit is specified via {@code withLimit(...)} or {@code withOptions(...)}, a default limit of 10,000 is
96+
* applied (the FTS service would otherwise return only its default of 10 hits).
9497
*
9598
* @return never {@literal null}.
9699
*/

src/main/java/org/springframework/data/couchbase/core/ExecutableFindBySearchOperationSupport.java

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,11 @@ public Stream<T> stream() {
120120

121121
@Override
122122
public long count() {
123-
return reactiveSupport.count().block();
123+
Long count = reactiveSupport.count().block();
124+
if (count == null) {
125+
throw new CouchbaseQueryExecutionException("search count query did not return a count, index: " + indexName);
126+
}
127+
return count;
124128
}
125129

126130
@Override

src/main/java/org/springframework/data/couchbase/core/ReactiveFindBySearchOperation.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,9 @@ interface TerminatingFindBySearch<T> {
7272

7373
/**
7474
* Get all matching elements, hydrated as entities via KV GET.
75+
* <p>
76+
* If no limit is specified via {@code withLimit(...)} or {@code withOptions(...)}, a default limit of 10,000 is
77+
* applied (the FTS service would otherwise return only its default of 10 hits).
7578
*
7679
* @return never {@literal null}.
7780
*/
@@ -120,6 +123,9 @@ interface FindBySearchWithQuery<T> extends TerminatingFindBySearch<T>, WithSearc
120123

121124
/**
122125
* Fluent method to specify options.
126+
* <p>
127+
* The given {@link SearchOptions} are passed to the SDK as provided (aside from collection routing) and must not be
128+
* combined with the individual {@code with*} configuration methods of this fluent API.
123129
*
124130
* @param <T> the entity type to use.
125131
*/

src/main/java/org/springframework/data/couchbase/core/ReactiveFindBySearchOperationSupport.java

Lines changed: 44 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,12 @@ public class ReactiveFindBySearchOperationSupport implements ReactiveFindBySearc
4848
private final ReactiveCouchbaseTemplate template;
4949
private static final Logger LOG = LoggerFactory.getLogger(ReactiveFindBySearchOperationSupport.class);
5050

51+
/**
52+
* Limit applied when neither {@code withLimit(...)} nor {@code withOptions(...)} is used. Without an explicit size
53+
* the FTS service returns only its default of 10 hits, which would silently truncate {@code all()} results.
54+
*/
55+
static final int DEFAULT_LIMIT = 10_000;
56+
5157
public ReactiveFindBySearchOperationSupport(final ReactiveCouchbaseTemplate template) {
5258
this.template = template;
5359
}
@@ -223,7 +229,7 @@ public Flux<T> all() {
223229
return TransactionalSupport.verifyNotInTransaction("findBySearch")
224230
.thenMany(executeSearch()
225231
.flatMapMany(ReactiveSearchResult::rows))
226-
.concatMap(row -> hydrateRow(row))
232+
.flatMapSequential(this::hydrateRow)
227233
.onErrorMap(throwable -> {
228234
if (throwable instanceof RuntimeException) {
229235
return template.potentiallyConvertRuntimeException((RuntimeException) throwable);
@@ -240,9 +246,10 @@ public Mono<Long> count() {
240246
Assert.notNull(indexName, "Index name must be specified via withIndex()");
241247
Assert.notNull(searchRequest, "SearchRequest must be specified via matching()");
242248

249+
// rows must be drained before the SDK emits the metadata trailer, even with limit 0
243250
return TransactionalSupport.verifyNotInTransaction("findBySearch")
244-
.then(executeSearch())
245-
.flatMap(ReactiveSearchResult::metaData)
251+
.then(executeSearch(true))
252+
.flatMap(result -> result.rows().then(result.metaData()))
246253
.map(metaData -> metaData.metrics().totalRows())
247254
.onErrorMap(throwable -> {
248255
if (throwable instanceof RuntimeException) {
@@ -309,7 +316,7 @@ private Mono<SearchResult<T>> collectFullResult(ReactiveSearchResult reactiveRes
309316
java.util.Map<String, SearchFacetResult> facetResults = tuple.getT3();
310317

311318
return Flux.fromIterable(searchRows)
312-
.concatMap(row -> hydrateRow(row))
319+
.flatMapSequential(this::hydrateRow)
313320
.collectList()
314321
.map(entities -> new SearchResult<>(entities, searchRows, metaData, facetResults));
315322
});
@@ -323,19 +330,24 @@ private Mono<SearchResult<T>> collectFullResult(ReactiveSearchResult reactiveRes
323330
* misses typically indicate an out-of-sync FTS index.
324331
*/
325332
private Mono<T> hydrateRow(SearchRow row) {
333+
// findById already maps DocumentNotFoundException to an empty Mono, so misses surface as emptiness
326334
return template.findById(returnType)
327335
.inScope(scope)
328336
.inCollection(collection)
329337
.one(row.id())
330-
.onErrorResume(com.couchbase.client.core.error.DocumentNotFoundException.class, ex -> {
338+
.switchIfEmpty(Mono.defer(() -> {
331339
LOG.warn("Skipping stale FTS result for document id '{}': document not found in KV "
332340
+ "(index '{}' may be out of sync)", row.id(), indexName);
333341
return Mono.empty();
334-
});
342+
}));
335343
}
336344

337345
private Mono<ReactiveSearchResult> executeSearch() {
338-
SearchOptions opts = buildSearchOptions();
346+
return executeSearch(false);
347+
}
348+
349+
private Mono<ReactiveSearchResult> executeSearch(boolean metadataOnly) {
350+
SearchOptions opts = buildSearchOptions(metadataOnly);
339351
if (scope != null) {
340352
return template.getCouchbaseClientFactory()
341353
.withScope(scope).getScope().reactive()
@@ -347,8 +359,19 @@ private Mono<ReactiveSearchResult> executeSearch() {
347359
}
348360
}
349361

350-
private SearchOptions buildSearchOptions() {
351-
SearchOptions opts = options != null ? options : SearchOptions.searchOptions();
362+
private SearchOptions buildSearchOptions(boolean metadataOnly) {
363+
if (options != null) {
364+
if (hasFluentOptions()) {
365+
throw new IllegalArgumentException("withOptions() cannot be combined with withConsistency(), withSort(), "
366+
+ "withHighlight(), withFacets(), withFields(), withLimit() or withSkip(); "
367+
+ "set those directly on the SearchOptions instead");
368+
}
369+
if (collection != null) {
370+
options.collections(collection);
371+
}
372+
return options;
373+
}
374+
SearchOptions opts = SearchOptions.searchOptions();
352375
if (scanConsistency != null) {
353376
opts.scanConsistency(scanConsistency);
354377
}
@@ -371,15 +394,24 @@ private SearchOptions buildSearchOptions() {
371394
if (fields != null && fields.length > 0) {
372395
opts.fields(fields);
373396
}
374-
if (limitSkip != null) {
375-
if (limitSkip[0] != null) {
397+
if (metadataOnly) {
398+
opts.limit(0);
399+
} else {
400+
if (limitSkip != null && limitSkip[0] != null) {
376401
opts.limit(limitSkip[0]);
402+
} else {
403+
opts.limit(DEFAULT_LIMIT);
377404
}
378-
if (limitSkip[1] != null) {
405+
if (limitSkip != null && limitSkip[1] != null) {
379406
opts.skip(limitSkip[1]);
380407
}
381408
}
382409
return opts;
383410
}
411+
412+
private boolean hasFluentOptions() {
413+
return scanConsistency != null || (sort != null && sort.length > 0) || highlightStyle != null
414+
|| (facets != null && !facets.isEmpty()) || (fields != null && fields.length > 0) || limitSkip != null;
415+
}
384416
}
385417
}

src/main/java/org/springframework/data/couchbase/repository/support/CouchbaseRepositoryFactory.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,10 @@ public RepositoryQuery resolveQuery(final Method method, final RepositoryMetadat
168168
CouchbaseQueryMethod queryMethod = new CouchbaseQueryMethod(method, metadata, factory, mappingContext);
169169

170170
if (queryMethod.hasSearchAnnotation()) {
171+
if (queryMethod.hasN1qlAnnotation()) {
172+
throw new IllegalArgumentException(
173+
"Method " + method + " must not be annotated with both @Search and @Query");
174+
}
171175
return new SearchBasedCouchbaseQuery(queryMethod, couchbaseOperations);
172176
} else if (queryMethod.hasN1qlAnnotation()) {
173177
return new StringBasedCouchbaseQuery(queryMethod, couchbaseOperations,

src/main/java/org/springframework/data/couchbase/repository/support/ReactiveCouchbaseRepositoryFactory.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,10 @@ public RepositoryQuery resolveQuery(Method method, RepositoryMetadata metadata,
157157
mappingContext);
158158

159159
if (queryMethod.hasSearchAnnotation()) {
160+
if (queryMethod.hasN1qlAnnotation()) {
161+
throw new IllegalArgumentException(
162+
"Method " + method + " must not be annotated with both @Search and @Query");
163+
}
160164
return new ReactiveSearchBasedCouchbaseQuery(queryMethod, couchbaseOperations);
161165
} else if (queryMethod.hasN1qlAnnotation()) {
162166
return new ReactiveStringBasedCouchbaseQuery(queryMethod, couchbaseOperations,

src/test/java/org/springframework/data/couchbase/core/CouchbaseTemplateFtsIntegrationTests.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
import org.springframework.data.couchbase.core.mapping.CouchbaseMappingContext;
3333
import org.springframework.data.couchbase.repository.Collection;
3434
import org.springframework.data.couchbase.repository.Scope;
35+
import org.springframework.data.couchbase.util.Capabilities;
3536
import org.springframework.data.couchbase.util.ClusterType;
3637
import org.springframework.data.couchbase.util.IgnoreWhen;
3738
import org.springframework.data.couchbase.util.JavaIntegrationTests;
@@ -58,7 +59,7 @@
5859
* @author Emilien Bevierre
5960
* @since 6.2
6061
*/
61-
@IgnoreWhen(clusterTypes = ClusterType.MOCKED)
62+
@IgnoreWhen(clusterTypes = ClusterType.MOCKED, missesCapabilities = Capabilities.SEARCH)
6263
class CouchbaseTemplateFtsIntegrationTests extends JavaIntegrationTests {
6364

6465
private static final String INDEX_NAME = "sd-fts-airport-idx";

src/test/java/org/springframework/data/couchbase/core/CouchbaseTemplateSearchIntegrationTests.java

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import org.springframework.beans.factory.annotation.Autowired;
2828
import org.springframework.data.couchbase.domain.Config;
2929
import org.springframework.data.couchbase.domain.User;
30+
import org.springframework.data.couchbase.util.Capabilities;
3031
import org.springframework.data.couchbase.util.ClusterType;
3132
import org.springframework.data.couchbase.util.IgnoreWhen;
3233
import org.springframework.data.couchbase.util.JavaIntegrationTests;
@@ -51,7 +52,7 @@
5152
*
5253
* @since 6.2
5354
*/
54-
@IgnoreWhen(clusterTypes = ClusterType.MOCKED)
55+
@IgnoreWhen(clusterTypes = ClusterType.MOCKED, missesCapabilities = Capabilities.SEARCH)
5556
@SpringJUnitConfig(Config.class)
5657
@DirtiesContext
5758
class CouchbaseTemplateSearchIntegrationTests extends JavaIntegrationTests {
@@ -341,16 +342,13 @@ private static void waitForFtsIndex(Cluster cluster, String indexName) {
341342
cluster.searchQuery(indexName, SearchQuery.queryString("*"));
342343
return;
343344
} catch (Exception ex) {
344-
if (i < maxRetries - 1 && (ex.getMessage().contains("no planPIndexes")
345-
|| ex.getMessage().contains("pindex_consistency")
346-
|| ex.getMessage().contains("pindex not available")
347-
|| ex.getMessage().contains("index not found"))) {
348-
sleepMs(2000);
349-
continue;
350-
}
351-
if (i >= maxRetries - 1) {
345+
String msg = ex.getMessage() != null ? ex.getMessage() : "";
346+
boolean retryable = msg.contains("no planPIndexes") || msg.contains("pindex_consistency")
347+
|| msg.contains("pindex not available") || msg.contains("index not found");
348+
if (!retryable || i >= maxRetries - 1) {
352349
throw new RuntimeException("FTS index " + indexName + " did not become ready in time", ex);
353350
}
351+
sleepMs(2000);
354352
}
355353
}
356354
}

src/test/java/org/springframework/data/couchbase/repository/ReactiveSearchRepositoryFtsIntegrationTests.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import org.springframework.data.couchbase.core.mapping.CouchbaseMappingContext;
3030
import org.springframework.data.couchbase.repository.config.ReactiveRepositoryOperationsMapping;
3131
import org.springframework.data.couchbase.repository.support.ReactiveCouchbaseRepositoryFactory;
32+
import org.springframework.data.couchbase.util.Capabilities;
3233
import org.springframework.data.couchbase.util.ClusterType;
3334
import org.springframework.data.couchbase.util.IgnoreWhen;
3435
import org.springframework.data.couchbase.util.JavaIntegrationTests;
@@ -55,7 +56,7 @@
5556
* @author Emilien Bevierre
5657
* @since 6.2
5758
*/
58-
@IgnoreWhen(clusterTypes = ClusterType.MOCKED)
59+
@IgnoreWhen(clusterTypes = ClusterType.MOCKED, missesCapabilities = Capabilities.SEARCH)
5960
class ReactiveSearchRepositoryFtsIntegrationTests extends JavaIntegrationTests {
6061

6162
private static final String INDEX_NAME = "sd-fts-airport-idx";

0 commit comments

Comments
 (0)