From 5fae355f6671c070cfee7b749437e9410ccc29aa Mon Sep 17 00:00:00 2001 From: Fanis <93339170+tharropoulos@users.noreply.github.com> Date: Wed, 25 Sep 2024 19:11:47 +0300 Subject: [PATCH] Fix Typesense pagination issue when using query callback (#867) * fix(typesense): fix per_page error when paginating with query callbacks - Implement performPaginatedSearch for large result sets - Add maxTotalResults property to limit total fetched results - Adjust search method to use maxPerPage as default limit - Ensure search respects both maxPerPage and maxTotalResults limits This ensures that when the `getTotalCount` builder function is called with a query callback, the resulting request to the server will be broken down batches of 250 results per page, Typesense's maximum. * feat(typesense): addmax total results configuration for typesense - Introduce a new 'max_total_results' configuration option for Typesense in scout.php. - Update TypesenseEngine constructor and EngineManager to utilize this new parameter, providing better control over the maximum number of searchable results. This ensures that the resulting requests to the server when broken up to batches are handled by the user, not defaulting to Typesense's total records, as that can introduce perfomance issues for larger datasets. * test(typesense): add test for paginated search with empty query callback - Implement new test method in TypesenseSearchableTest to verify pagination behavior with an empty query callback. This ensures the search functionality works correctly when no additional query constraints are applied. * style: ci linting errors * fix(test): fix missing constructor args in typesense unit test * style: more ci linting errors * formatting * formatting --------- Co-authored-by: Taylor Otwell --- config/scout.php | 1 + src/EngineManager.php | 3 +- src/Engines/TypesenseEngine.php | 70 ++++++++++++++++++- tests/Integration/SearchableTests.php | 8 +++ tests/Integration/TypesenseSearchableTest.php | 8 +++ tests/Unit/TypesenseEngineTest.php | 2 +- 6 files changed, 88 insertions(+), 4 deletions(-) diff --git a/config/scout.php b/config/scout.php index ac1a88ea..68218c2f 100644 --- a/config/scout.php +++ b/config/scout.php @@ -173,6 +173,7 @@ 'num_retries' => env('TYPESENSE_NUM_RETRIES', 3), 'retry_interval_seconds' => env('TYPESENSE_RETRY_INTERVAL_SECONDS', 1), ], + // 'max_total_results' => env('TYPESENSE_MAX_TOTAL_RESULTS', 1000), 'model-settings' => [ // User::class => [ // 'collection-schema' => [ diff --git a/src/EngineManager.php b/src/EngineManager.php index 08d73f92..10b1be8e 100644 --- a/src/EngineManager.php +++ b/src/EngineManager.php @@ -153,9 +153,10 @@ protected function ensureMeilisearchClientIsInstalled() */ public function createTypesenseDriver() { + $config = config('scout.typesense'); $this->ensureTypesenseClientIsInstalled(); - return new TypesenseEngine(new Typesense(config('scout.typesense.client-settings'))); + return new TypesenseEngine(new Typesense($config['client-settings']), $config['max_total_results'] ?? 1000); } /** diff --git a/src/Engines/TypesenseEngine.php b/src/Engines/TypesenseEngine.php index 18146989..72861390 100644 --- a/src/Engines/TypesenseEngine.php +++ b/src/Engines/TypesenseEngine.php @@ -29,14 +29,29 @@ class TypesenseEngine extends Engine */ protected array $searchParameters = []; + /** + * The maximum number of results that can be fetched per page. + * + * @var int + */ + private int $maxPerPage = 250; + + /** + * The maximum number of results that can be fetched during pagination. + * + * @var int + */ + protected int $maxTotalResults; + /** * Create new Typesense engine instance. * * @param Typesense $typesense */ - public function __construct(Typesense $typesense) + public function __construct(Typesense $typesense, int $maxTotalResults) { $this->typesense = $typesense; + $this->maxTotalResults = $maxTotalResults; } /** @@ -186,9 +201,14 @@ protected function deleteDocument(TypesenseCollection $collectionIndex, $modelId */ public function search(Builder $builder) { + // If the limit exceeds Typesense's capabilities, perform a paginated search... + if ($builder->limit >= $this->maxPerPage) { + return $this->performPaginatedSearch($builder); + } + return $this->performSearch( $builder, - $this->buildSearchParameters($builder, 1, $builder->limit) + $this->buildSearchParameters($builder, 1, $builder->limit ?? $this->maxPerPage) ); } @@ -232,6 +252,52 @@ protected function performSearch(Builder $builder, array $options = []): mixed return $documents->search($options); } + /** + * Perform a paginated search on the engine. + * + * @param \Laravel\Scout\Builder $builder + * @return mixed + * + * @throws \Http\Client\Exception + * @throws \Typesense\Exceptions\TypesenseClientError + */ + protected function performPaginatedSearch(Builder $builder) + { + $page = 1; + $limit = min($builder->limit ?? $this->maxPerPage, $this->maxPerPage, $this->maxTotalResults); + $remainingResults = min($builder->limit ?? $this->maxTotalResults, $this->maxTotalResults); + + $results = new Collection; + + while ($remainingResults > 0) { + $searchResults = $this->performSearch( + $builder, + $this->buildSearchParameters($builder, $page, $limit) + ); + + $results = $results->concat($searchResults['hits'] ?? []); + + if ($page === 1) { + $totalFound = $searchResults['found'] ?? 0; + } + + $remainingResults -= $limit; + $page++; + + if (count($searchResults['hits'] ?? []) < $limit) { + break; + } + } + + return [ + 'hits' => $results->all(), + 'found' => $results->count(), + 'out_of' => $totalFound, + 'page' => 1, + 'request_params' => $this->buildSearchParameters($builder, 1, $builder->limit ?? $this->maxPerPage), + ]; + } + /** * Build the search parameters for a given Scout query builder. * diff --git a/tests/Integration/SearchableTests.php b/tests/Integration/SearchableTests.php index 9856f719..ef25c85f 100644 --- a/tests/Integration/SearchableTests.php +++ b/tests/Integration/SearchableTests.php @@ -104,4 +104,12 @@ protected function itCanUsePaginatedSearchWithQueryCallback() User::search('lar')->take(10)->query($queryCallback)->paginate(5, 'page', 2), ]; } + + protected function itCanUsePaginatedSearchWithEmptyQueryCallback() + { + $queryCallback = function ($query) { + }; + + return User::search('*')->query($queryCallback)->paginate(); + } } diff --git a/tests/Integration/TypesenseSearchableTest.php b/tests/Integration/TypesenseSearchableTest.php index 30da065e..c6405f25 100644 --- a/tests/Integration/TypesenseSearchableTest.php +++ b/tests/Integration/TypesenseSearchableTest.php @@ -164,6 +164,14 @@ public function test_it_can_use_paginated_search_with_query_callback() ], $page2->pluck('name', 'id')->all()); } + public function test_it_can_usePaginatedSearchWithEmptyQueryCallback() + { + $res = $this->itCanUsePaginatedSearchWithEmptyQueryCallback(); + + $this->assertSame($res->total(), 44); + $this->assertSame($res->lastPage(), 3); + } + protected static function scoutDriver(): string { return 'typesense'; diff --git a/tests/Unit/TypesenseEngineTest.php b/tests/Unit/TypesenseEngineTest.php index 9f360a9b..93f3a5dc 100644 --- a/tests/Unit/TypesenseEngineTest.php +++ b/tests/Unit/TypesenseEngineTest.php @@ -27,7 +27,7 @@ protected function setUp(): void // Mock the Typesense client and pass it to the engine constructor $typesenseClient = $this->createMock(TypesenseClient::class); $this->engine = $this->getMockBuilder(TypesenseEngine::class) - ->setConstructorArgs([$typesenseClient]) + ->setConstructorArgs([$typesenseClient, 1000]) ->onlyMethods(['getOrCreateCollectionFromModel', 'buildSearchParameters']) ->getMock(); }