Fix issue with SearchPagedFlux and SearchPagedIterable #46990
Conversation
alzimmermsft
requested review from
a team,
JonathanGiles,
Netyyyy,
moarychan,
rujche,
saragluna and
srnagar
as code owners
There was a problem hiding this comment.
Pull Request Overview
This PR fixes an issue where SearchPagedFlux and SearchPagedIterable would return the same first page for every subsequent iteration/subscription. The fix removes logic that was caching the first page response and deprecates non-iterating APIs on these classes in favor of using equivalent APIs on SearchPagedResponse.
Key Changes
- Removed caching logic from search clients that caused duplicate first page results
- Made
SearchPagedResponsemethods public and deprecated equivalent methods on pageable types - Consolidated search response creation into a shared utility method
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
SearchClient.java |
Removed first page caching logic and consolidated response creation |
SearchAsyncClient.java |
Removed first page caching logic and added shared response mapping utility |
SearchPagedResponse.java |
Made accessor methods public and updated constructors with deprecations |
SearchPagedIterable.java |
Deprecated metadata access methods with guidance to use response methods |
SearchPagedFlux.java |
Deprecated metadata access methods with guidance to use response methods |
SemanticSearchResults.java |
Updated constructor to accept parameters directly instead of SearchPagedResponse |
| Test files | Updated to use new public methods instead of helper classes |
| Helper classes | Removed SearchPagedResponseAccessHelper and updated SemanticSearchResultsAccessHelper |
| * @param facets Facets contained in the search. | ||
| * @param count Total number of documents available as a result for the search. | ||
| * @param coverage Percent of the index used in the search operation. | ||
| * @deprecated Use {@link SearchPagedResponse#SearchPagedResponse(Response, String, Map, Long, Double, List, SemanticErrorReason, SemanticSearchResultsType, DebugInfo, SemanticQueryRewritesResultType)} |
There was a problem hiding this comment.
The deprecated constructor reference is incorrect. The actual constructor being referenced has different parameter ordering and types. Should reference the correct constructor signature with SemanticSearchResults parameter.
Copilot uses AI. Check for mistakes.
| * @param queryAnswers Answers contained in the search. | ||
| * @param semanticErrorReason Reason that a partial response was returned for a semantic search request. | ||
| * @param semanticSearchResultsType Type of the partial response returned for a semantic search request. | ||
| * @deprecated Use {@link SearchPagedResponse#SearchPagedResponse(Response, String, Map, Long, Double, List, SemanticErrorReason, SemanticSearchResultsType, DebugInfo, SemanticQueryRewritesResultType)} |
There was a problem hiding this comment.
The deprecated constructor reference is incorrect. The actual constructor being referenced has different parameter ordering and types. Should reference the correct constructor signature with SemanticSearchResults parameter.
Copilot uses AI. Check for mistakes.
API Change CheckAPIView identified API level changes in this PR and created the following API reviews |
1 participant
Description
Fixes #43095
Removes the logic where
SearchClientandSearchAsyncClientwill return the same first page for every subsequent iteration / subscription on theSearchPagedIterableandSearchPagedFluxreturned.All non-iterating / subscription APIs on
SearchPagedIterableandSearchPagedFluxhave been deprecated and the equivalent APIs inSearchPagedResponseshould be used.All SDK Contribution checklist:
General Guidelines and Best Practices
Testing Guidelines