diff --git a/CHANGELOG.md b/CHANGELOG.md index 651abee..7231369 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,8 @@ See [keep a changelog] for information about writing changes to this log. ## [Unreleased] +- [PR-38](https://github.com/itk-dev/event-database-api/pull/38) + Extract SearchParamsBuilder from ElasticSearchIndex and unit-test the query DSL - [PR-37](https://github.com/itk-dev/event-database-api/pull/37) Upload test coverage to Codecov in CI - [PR-36](https://github.com/itk-dev/event-database-api/pull/36) diff --git a/src/Service/ElasticSearch/ElasticSearchIndex.php b/src/Service/ElasticSearch/ElasticSearchIndex.php index 0341676..5344c52 100644 --- a/src/Service/ElasticSearch/ElasticSearchIndex.php +++ b/src/Service/ElasticSearch/ElasticSearchIndex.php @@ -3,8 +3,6 @@ namespace App\Service\ElasticSearch; use App\Exception\IndexException; -use App\Model\FilterType; -use App\Model\IndexName; use App\Model\SearchResults; use App\Service\IndexInterface; use Elastic\Elasticsearch\Client; @@ -18,6 +16,7 @@ class ElasticSearchIndex implements IndexInterface { public function __construct( private readonly Client $client, + private readonly SearchParamsBuilder $paramsBuilder, ) { } @@ -113,7 +112,7 @@ private function getByCustomIdField(string $indexName, int|string $id, string $i public function getAll(string $indexName, array $filters = [], int $from = 0, int $size = 10): SearchResults { - $params = $this->buildParams($indexName, $filters, $from, $size); + $params = $this->paramsBuilder->buildParams($indexName, $filters, $from, $size); try { /** @var Elasticsearch $response */ @@ -132,79 +131,6 @@ public function getAll(string $indexName, array $filters = [], int $from = 0, in ); } - /** - * Builds the parameters for the Elasticsearch search request. - * - * @param string $indexName - * The name of the index to search in - * @param array $filters - * An array of filters to apply to the search query - * @param int $from - * The starting offset for the search results - * @param int $size - * The maximum number of search results to return - * - * @return array - * The built parameters for the Elasticsearch search request - */ - private function buildParams(string $indexName, array $filters, int $from, int $size): array - { - $params = [ - 'index' => $indexName, - 'body' => [ - 'query' => [ - 'match_all' => (object) [], - ], - 'size' => $size, - 'from' => $from, - // @TODO: make a proper sort filter to allow client to set sort direction - 'sort' => $this->getSort($indexName), - ], - ]; - - $body = $this->buildBody($filters); - if ([] !== $body) { - $params['body']['query'] = $body; - } - - return $params; - } - - /** - * Builds the body for Elasticsearch request using the given filters. - * - * @param array $filters - * The filters to be included in the body - * - * @return array - * The built body for Elasticsearch request - */ - private function buildBody(array $filters): array - { - $body = []; - $combined = (bool) count($filters[FilterType::Filters->value]); - foreach ($filters[FilterType::Filters->value] as $filter) { - if ($combined) { - if (!array_key_exists('bool', $body)) { - $body['bool'] = ['must' => []]; - } - // Ensure that associative arrays and lists are not combined with keys "0","1" etc. in the final json. - // So we need to loop over lists to ensure keys are "reset" in the final body statement. - if (array_is_list($filter)) { - foreach ($filter as $val) { - $body['bool']['must'][] = $val; - } - } else { - $body['bool']['must'][] = $filter; - } - } else { - $body += $filter; - } - } - - return $body; - } - /** * Parses the response from Elasticsearch and returns it as an array. * @@ -254,50 +180,4 @@ private function getTotalHits(array $data): int { return $data['hits']['total']['value'] ?? 0; } - - /** - * Get the sorting configuration for a specific index. - * - * This method returns an array containing the sorting configuration based on the given index name. - * If the index name matches one of the predefined index names, a specific sorting configuration will be returned. - * Otherwise, an empty array will be returned indicating no sorting is required. - * - * @param string $indexName the name of the index - * - * @return array the sorting configuration - */ - private function getSort(string $indexName): array - { - // Translates a string or int into the corresponding Enum case, if any. - // If there is no matching case defined, it will return null. - $indexName = IndexName::tryFrom($indexName); - - return match ($indexName) { - IndexName::Events => [ - '_score', - [ - 'title.keyword' => [ - 'order' => 'asc', - ], - ], - ], - IndexName::DailyOccurrences, IndexName::Occurrences => [ - 'start' => [ - 'order' => 'asc', - 'format' => 'strict_date_optional_time_nanos', - ], - ], - IndexName::Tags, IndexName::Vocabularies,IndexName::Locations, IndexName::Organizations => [ - '_score', - [ - 'name.keyword' => [ - 'order' => 'asc', - ], - ], - ], - default => [ - '_score', - ], - }; - } } diff --git a/src/Service/ElasticSearch/SearchParamsBuilder.php b/src/Service/ElasticSearch/SearchParamsBuilder.php new file mode 100644 index 0000000..44e8746 --- /dev/null +++ b/src/Service/ElasticSearch/SearchParamsBuilder.php @@ -0,0 +1,100 @@ +> $filters compiled clauses keyed by FilterType + * + * @return array + */ + public function buildParams(string $indexName, array $filters, int $from, int $size): array + { + $params = [ + 'index' => $indexName, + 'body' => [ + 'query' => [ + 'match_all' => (object) [], + ], + 'size' => $size, + 'from' => $from, + // @TODO: make a proper sort filter to allow client to set sort direction + 'sort' => $this->buildSort($indexName), + ], + ]; + + $body = $this->buildBody($filters); + if ([] !== $body) { + $params['body']['query'] = $body; + } + + return $params; + } + + /** + * Combines the filter clauses into a single `bool`/`must` query. + * + * @param array> $filters + * + * @return array + */ + private function buildBody(array $filters): array + { + $body = []; + foreach ($filters[FilterType::Filters->value] as $filter) { + if (!array_key_exists('bool', $body)) { + $body['bool'] = ['must' => []]; + } + // Ensure that associative arrays and lists are not combined with keys "0","1" etc. in the final json. + // So we need to loop over lists to ensure keys are "reset" in the final body statement. + if (array_is_list($filter)) { + foreach ($filter as $val) { + $body['bool']['must'][] = $val; + } + } else { + $body['bool']['must'][] = $filter; + } + } + + return $body; + } + + /** + * The per-index sort configuration. + * + * @return array + */ + private function buildSort(string $indexName): array + { + return match (IndexName::tryFrom($indexName)) { + IndexName::Events => [ + '_score', + ['title.keyword' => ['order' => 'asc']], + ], + IndexName::DailyOccurrences, IndexName::Occurrences => [ + 'start' => [ + 'order' => 'asc', + 'format' => 'strict_date_optional_time_nanos', + ], + ], + IndexName::Tags, IndexName::Vocabularies, IndexName::Locations, IndexName::Organizations => [ + '_score', + ['name.keyword' => ['order' => 'asc']], + ], + default => [ + '_score', + ], + }; + } +} diff --git a/tests/Unit/Service/ElasticSearch/ElasticIndexExceptionTest.php b/tests/Unit/Service/ElasticSearch/ElasticIndexExceptionTest.php new file mode 100644 index 0000000..271df2e --- /dev/null +++ b/tests/Unit/Service/ElasticSearch/ElasticIndexExceptionTest.php @@ -0,0 +1,37 @@ +". + public function test400ParsesElasticErrorMessage(): void + { + $raw = '400 Bad Request: {"error":{"root_cause":[{"type":"parse_exception",' + .'"reason":"failed to parse date field [2004-02-12T15:19:21+0000]: [details]"}]}}'; + + $exception = new ElasticIndexException($raw, 400); + + self::assertSame( + 'Parse exception: failed to parse date field [2004-02-12T15:19:21+0000]', + $exception->getMessage(), + ); + self::assertSame(400, $exception->getCode()); + } + + // Goal: non-400 codes collapse to a generic message (no ES JSON to parse). + public function testNon400CollapsesToBadRequest(): void + { + $exception = new ElasticIndexException('500 Internal Server Error: something', 500); + + self::assertSame('Bad Request', $exception->getMessage()); + } +} diff --git a/tests/Unit/Service/ElasticSearch/SearchParamsBuilderTest.php b/tests/Unit/Service/ElasticSearch/SearchParamsBuilderTest.php new file mode 100644 index 0000000..c9eb3da --- /dev/null +++ b/tests/Unit/Service/ElasticSearch/SearchParamsBuilderTest.php @@ -0,0 +1,108 @@ +value => [], FilterType::Sort->value => []]; + } + + /** + * @param array $clauses + */ + private function withFilters(array $clauses): array + { + return [FilterType::Filters->value => $clauses, FilterType::Sort->value => []]; + } + + // Goal: with no filters the query defaults to match_all and pagination/sort are set. + public function testDefaultsToMatchAllWithPagination(): void + { + $params = (new SearchParamsBuilder())->buildParams(IndexName::Events->value, $this->noFilters(), 20, 5); + + self::assertSame(IndexName::Events->value, $params['index']); + self::assertEquals(['match_all' => (object) []], $params['body']['query']); + self::assertSame(5, $params['body']['size']); + self::assertSame(20, $params['body']['from']); + self::assertArrayHasKey('sort', $params['body']); + } + + // Goal: a single associative clause (e.g. a MatchFilter hit) is wrapped in bool/must. + public function testSingleClauseWrappedInBoolMust(): void + { + $params = (new SearchParamsBuilder())->buildParams( + IndexName::Events->value, + $this->withFilters([['match' => ['title' => 'x']]]), + 0, + 10, + ); + + self::assertSame(['bool' => ['must' => [['match' => ['title' => 'x']]]]], $params['body']['query']); + } + + // Goal: a list-shaped clause (e.g. IdFilter output) is flattened into must, not + // nested under numeric keys. + public function testListClauseIsFlattenedIntoMust(): void + { + $params = (new SearchParamsBuilder())->buildParams( + IndexName::Events->value, + $this->withFilters([[['terms' => ['organizer.entityId' => ['9'], 'boost' => 1.0]]]]), + 0, + 10, + ); + + self::assertSame( + ['bool' => ['must' => [['terms' => ['organizer.entityId' => ['9'], 'boost' => 1.0]]]]], + $params['body']['query'], + ); + } + + // Goal: multiple clauses accumulate under a single bool/must. + public function testMultipleClausesCombine(): void + { + $params = (new SearchParamsBuilder())->buildParams( + IndexName::Events->value, + $this->withFilters([ + ['match' => ['title' => 'x']], + ['terms' => ['tags' => ['aros'], 'boost' => 1.0]], + ]), + 0, + 10, + ); + + self::assertSame([ + ['match' => ['title' => 'x']], + ['terms' => ['tags' => ['aros'], 'boost' => 1.0]], + ], $params['body']['query']['bool']['must']); + } + + // Goal: each index gets its documented sort; unknown indexes fall back to _score. + #[DataProvider('sortProvider')] + public function testSortPerIndex(string $index, array $expectedSort): void + { + $params = (new SearchParamsBuilder())->buildParams($index, $this->noFilters(), 0, 10); + + self::assertSame($expectedSort, $params['body']['sort']); + } + + public static function sortProvider(): iterable + { + yield 'events' => [IndexName::Events->value, ['_score', ['title.keyword' => ['order' => 'asc']]]]; + yield 'occurrences' => [IndexName::Occurrences->value, ['start' => ['order' => 'asc', 'format' => 'strict_date_optional_time_nanos']]]; + yield 'tags' => [IndexName::Tags->value, ['_score', ['name.keyword' => ['order' => 'asc']]]]; + yield 'unknown → _score' => ['not_an_index', ['_score']]; + } +}