Skip to content

Commit

Permalink
fix(clients): update browse iterator (#4058)
Browse files Browse the repository at this point in the history
  • Loading branch information
@Fluf22
Fluf22 authored Oct 31, 2024
1 parent cc160af commit 6254217
Show file tree
Hide file tree
Showing 10 changed files with 46 additions and 129 deletions.
4 changes: 2 additions & 2 deletions clients/algoliasearch-client-csharp/algoliasearch/Utils/SearchClientExtensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -344,7 +344,7 @@ public async Task<IEnumerable<Rule>> BrowseRulesAsync(string indexName, SearchRu
var page = prevResp?.Item2 ?? 0;
var searchSynonymsResponse = await SearchRulesAsync(indexName, searchRulesParams, requestOptions);
return new Tuple<SearchRulesResponse, int>(searchSynonymsResponse, page + 1);
}, resp => resp?.Item1 is { NbHits: < hitsPerPage }).ConfigureAwait(false);
}, resp => resp?.Item1 is { Hits.Count: < hitsPerPage }).ConfigureAwait(false);

return all.SelectMany(u => u.Item1.Hits);
}
Expand All @@ -367,7 +367,7 @@ public async Task<IEnumerable<SynonymHit>> BrowseSynonymsAsync(string indexName,
var searchSynonymsResponse = await SearchSynonymsAsync(indexName, synonymsParams, requestOptions);
page = page + 1;
return new Tuple<SearchSynonymsResponse, int>(searchSynonymsResponse, page);
}, resp => resp?.Item1 is { NbHits: < hitsPerPage }).ConfigureAwait(false);
}, resp => resp?.Item1 is { Hits.Count: < hitsPerPage }).ConfigureAwait(false);

return all.SelectMany(u => u.Item1.Hits);
}
Expand Down
2 changes: 1 addition & 1 deletion clients/algoliasearch-client-php/lib/Iterators/RuleIterator.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ protected function fetchNextPage()
{
if (
is_array($this->response)
&& $this->key >= $this->response['nbHits']
&& $this->key >= count($this->response['hits'])
) {
return;
}
Expand Down
2 changes: 1 addition & 1 deletion clients/algoliasearch-client-php/lib/Iterators/SynonymIterator.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ protected function fetchNextPage()
{
if (
is_array($this->response)
&& $this->key >= $this->response['nbHits']
&& $this->key >= count($this->response['hits'])
) {
return;
}
Expand Down
12 changes: 8 additions & 4 deletions clients/algoliasearch-client-swift/Sources/Search/Extra/SearchClientExtension.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -243,9 +243,13 @@ public extension SearchClient {
aggregator: @escaping (BrowseResponse<T>) -> Void,
requestOptions: RequestOptions? = nil
) async throws -> BrowseResponse<T> {
try await createIterable(
var updatedBrowseParams = browseParams
if updatedBrowseParams.hitsPerPage == nil {
updatedBrowseParams.hitsPerPage = 1000
}

return try await createIterable(
execute: { previousResponse in
var updatedBrowseParams = browseParams
if let previousResponse {
updatedBrowseParams.cursor = previousResponse.cursor
}
Expand Down Expand Up @@ -298,7 +302,7 @@ public extension SearchClient {
)
},
validate: validate ?? { response in
response.nbHits < hitsPerPage
response.hits.count < hitsPerPage
},
aggregator: aggregator
)
Expand Down Expand Up @@ -341,7 +345,7 @@ public extension SearchClient {
)
},
validate: validate ?? { response in
response.nbHits < hitsPerPage
response.hits.count < hitsPerPage
},
aggregator: aggregator
)
Expand Down
8 changes: 6 additions & 2 deletions templates/go/search_helpers.mustache
View file
Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,10 @@ func (c *APIClient) BrowseObjects(
browseParams BrowseParamsObject,
opts ...IterableOption,
) error {
if browseParams.HitsPerPage == nil {
browseParams.HitsPerPage = utils.ToPtr(int32(1000))
}

_, err := CreateIterable( //nolint:wrapcheck
func(previousResponse *BrowseResponse, previousErr error) (*BrowseResponse, error) {
if previousResponse != nil {
Expand Down Expand Up @@ -363,7 +367,7 @@ func (c *APIClient) BrowseRules(
)
},
func(response *SearchRulesResponse, err error) (bool, error) {
return err != nil || (response != nil && response.NbHits < hitsPerPage), err
return err != nil || (response != nil && len(response.Hits) < int(hitsPerPage)), err
},
opts...,
)
Expand Down Expand Up @@ -409,7 +413,7 @@ func (c *APIClient) BrowseSynonyms(
)
},
func(response *SearchSynonymsResponse, err error) (bool, error) {
return err != nil || (response != nil && response.NbHits < hitsPerPage), err
return err != nil || (response != nil && len(response.Hits) < int(hitsPerPage)), err
},
opts...,
)
Expand Down
8 changes: 6 additions & 2 deletions templates/java/api_helpers.mustache
View file
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,10 @@ public GetApiKeyResponse waitForApiKey(String key, ApiKeyOperation operation) {
public <T> Iterable<T> browseObjects(String indexName, BrowseParamsObject params, Class<T> innerType, RequestOptions requestOptions) {
final Holder<String> currentCursor = new Holder<>();
if (params.getHitsPerPage() == null) {
params.setHitsPerPage(1000);
}
return AlgoliaIterableHelper.createIterable(
() -> {
BrowseResponse<T> response = this.browse(indexName, params, innerType, requestOptions);
Expand Down Expand Up @@ -347,7 +351,7 @@ public Iterable<SynonymHit> browseSynonyms(String indexName, SearchSynonymsParam
return AlgoliaIterableHelper.createIterable(
() -> {
SearchSynonymsResponse response = this.searchSynonyms(indexName, params, requestOptions);
currentPage.value = response.getNbHits() < params.getHitsPerPage() ? null : currentPage.value + 1;
currentPage.value = response.getHits().size() < params.getHitsPerPage() ? null : currentPage.value + 1;
return response.getHits().iterator();
},
() -> currentPage.value != null
Expand Down Expand Up @@ -389,7 +393,7 @@ public Iterable<Rule> browseRules(String indexName, SearchRulesParams params, Re
return AlgoliaIterableHelper.createIterable(
() -> {
SearchRulesResponse response = this.searchRules(indexName, params.setPage(currentPage.value), requestOptions);
currentPage.value = response.getNbHits() < hitsPerPage ? null : currentPage.value + 1;
currentPage.value = response.getHits().size() < hitsPerPage ? null : currentPage.value + 1;
return response.getHits().iterator();
},
() -> currentPage.value != null
Expand Down
5 changes: 3 additions & 2 deletions templates/javascript/clients/client/api/helpers.mustache
View file
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,7 @@ browseObjects<T>(
indexName,
browseParams: {
cursor: previousResponse ? previousResponse.cursor : undefined,
hitsPerPage: 1000,
...browseParams,
},
},
Expand Down Expand Up @@ -226,7 +227,7 @@ browseRules(
requestOptions
);
},
validate: (response) => response.nbHits < params.hitsPerPage,
validate: (response) => response.hits.length < params.hitsPerPage,
...browseRulesOptions,
});
},
Expand Down Expand Up @@ -271,7 +272,7 @@ browseSynonyms(
params.page += 1;
return resp;
},
validate: (response) => response.nbHits < params.hitsPerPage,
validate: (response) => response.hits.length < params.hitsPerPage,
...browseSynonymsOptions,
});
},
Expand Down
6 changes: 4 additions & 2 deletions templates/python/search_helpers.mustache
View file
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,8 @@
"""
Helper: Iterate on the `browse` method of the client to allow aggregating objects of an index.
"""
browse_params.hits_per_page = browse_params.hits_per_page or 1000

{{^isSyncClient}}async {{/isSyncClient}}def _func(_prev: Optional[BrowseResponse]) -> BrowseResponse:
if _prev is not None and _prev.cursor is not None:
browse_params.cursor = _prev.cursor
Expand Down Expand Up @@ -167,7 +169,7 @@
)
return {{^isSyncClient}}await {{/isSyncClient}}create_iterable{{#isSyncClient}}_sync{{/isSyncClient}}(
func=_func,
validate=lambda _resp: _resp.nb_hits < hits_per_page,
validate=lambda _resp: len(_resp.hits) < hits_per_page,
aggregator=aggregator,
)

Expand Down Expand Up @@ -197,7 +199,7 @@
return resp
return {{^isSyncClient}}await {{/isSyncClient}}create_iterable{{#isSyncClient}}_sync{{/isSyncClient}}(
func=_func,
validate=lambda _resp: _resp.nb_hits < hits_per_page,
validate=lambda _resp: len(_resp.hits) < hits_per_page,
aggregator=aggregator,
)

Expand Down
6 changes: 4 additions & 2 deletions templates/ruby/search_helpers.mustache
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,8 @@ end
# @param request_options [Hash] the requestOptions to send along with the query, they will be forwarded to the `browse` method.
# @param block [Proc] the block to execute on each object of the index.
def browse_objects(index_name, browse_params = Search::BrowseParamsObject.new, request_options = {}, &block)
browse_params[:hits_per_page] = browse_params[:hits_per_page] || 1000

hits = []
loop do
res = browse(index_name, browse_params, request_options)
Expand Down Expand Up @@ -130,7 +132,7 @@ def browse_rules(index_name, search_rules_params = Search::SearchRulesParams.new
rules.concat(res.hits)
end
search_rules_params.page += 1
break if res.nb_hits < search_rules_params.hits_per_page
break if res.hits.length < search_rules_params.hits_per_page
end

rules unless block_given?
Expand All @@ -154,7 +156,7 @@ def browse_synonyms(index_name, search_synonyms_params = Search::SearchSynonymsP
synonyms.concat(res.hits)
end
search_synonyms_params.page += 1
break if res.nb_hits < search_synonyms_params.hits_per_page
break if res.hits.length < search_synonyms_params.hits_per_page
end

synonyms unless block_given?
Expand Down
122 changes: 11 additions & 111 deletions tests/output/csharp/src/ClientExtensionsTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ public async Task ShouldBrowseSynonyms()
It.IsAny<CancellationToken>()
)
)
// First call return 1000 Hits
// Only one call since it will take the hit list length and conclude there isn't more
.Returns(
Task.FromResult(
new AlgoliaHttpResponse
Expand All @@ -228,56 +228,8 @@ public async Task ShouldBrowseSynonyms()
{
new() { ObjectID = "XXX", Type = SynonymType.Altcorrection1 },
new() { ObjectID = "XXX", Type = SynonymType.Altcorrection1 },
}, // Not 1000 but it doesn't matter
NbHits = 1000,
}
)
)
),
}
)
)
// Second call return again 1000 Hits
.Returns(
Task.FromResult(
new AlgoliaHttpResponse
{
HttpStatusCode = 200,
Body = new MemoryStream(
Encoding.UTF8.GetBytes(
serializer.Serialize(
new SearchSynonymsResponse()
{
Hits = new List<SynonymHit>()
{
new() { ObjectID = "XXX", Type = SynonymType.Altcorrection1 },
new() { ObjectID = "XXX", Type = SynonymType.Altcorrection1 },
}, // Not 1000 but it doesn't matter
NbHits = 1000,
}
)
)
),
}
)
)
// Third call return 999 Hits
.Returns(
Task.FromResult(
new AlgoliaHttpResponse
{
HttpStatusCode = 200,
Body = new MemoryStream(
Encoding.UTF8.GetBytes(
serializer.Serialize(
new SearchSynonymsResponse
{
Hits = new List<SynonymHit>
{
new() { ObjectID = "XXX", Type = SynonymType.Altcorrection1 },
new() { ObjectID = "XXX", Type = SynonymType.Altcorrection1 },
}, // Not 1000 but it doesn't matter
NbHits = 999,
},
NbHits = 2,
}
)
)
Expand All @@ -302,10 +254,10 @@ public async Task ShouldBrowseSynonyms()
It.IsAny<TimeSpan>(),
It.IsAny<CancellationToken>()
),
Times.Exactly(3)
Times.Exactly(1)
);

Assert.Equal(6, browseSynonymsAsync.Count());
Assert.Equal(2, browseSynonymsAsync.Count());
}

[Fact]
Expand All @@ -323,59 +275,7 @@ public async Task ShouldBrowseRules()
It.IsAny<CancellationToken>()
)
)
// First call return 1000 Hits
.Returns(
Task.FromResult(
new AlgoliaHttpResponse
{
HttpStatusCode = 200,
Body = new MemoryStream(
Encoding.UTF8.GetBytes(
serializer.Serialize(
new SearchRulesResponse
{
Page = 0,
NbPages = 2,
Hits = new List<Rule>
{
new() { ObjectID = "XXX" },
new() { ObjectID = "XXX" },
}, // Not 1000 but it doesn't matter
NbHits = 1000,
}
)
)
),
}
)
)
// Second call return again 1000 Hits
.Returns(
Task.FromResult(
new AlgoliaHttpResponse
{
HttpStatusCode = 200,
Body = new MemoryStream(
Encoding.UTF8.GetBytes(
serializer.Serialize(
new SearchRulesResponse
{
Page = 0,
NbPages = 2,
Hits = new List<Rule>
{
new() { ObjectID = "XXX" },
new() { ObjectID = "XXX" },
}, // Not 1000 but it doesn't matter
NbHits = 1000,
}
)
)
),
}
)
)
// Third call return 999 Hits
// Only one call since it will take the hit list length and conclude there isn't more
.Returns(
Task.FromResult(
new AlgoliaHttpResponse
Expand All @@ -387,13 +287,13 @@ public async Task ShouldBrowseRules()
new SearchRulesResponse
{
Page = 0,
NbPages = 2,
NbPages = 1,
Hits = new List<Rule>
{
new() { ObjectID = "XXX" },
new() { ObjectID = "XXX" },
}, // Not 1000 but it doesn't matter
NbHits = 999,
},
NbHits = 2,
}
)
)
Expand All @@ -416,10 +316,10 @@ public async Task ShouldBrowseRules()
It.IsAny<TimeSpan>(),
It.IsAny<CancellationToken>()
),
Times.Exactly(3)
Times.Exactly(1)
);

Assert.Equal(6, browseSynonymsAsync.Count());
Assert.Equal(2, browseSynonymsAsync.Count());
}

[Fact]
Expand Down

0 comments on commit 6254217

Please sign in to comment.