From ddd88fbaf34a66d0db2ad9004c9908fdf8d646b7 Mon Sep 17 00:00:00 2001 From: Jesper Kristensen Date: Tue, 9 Jan 2024 16:37:34 +0100 Subject: [PATCH 1/5] #419: First try with tags filter --- config/services.yaml | 6 ++- src/Api/Dto/Event.php | 6 +++ src/Api/Filter/EventTagFilter.php | 52 +++++++++++++++++++ src/Api/State/EventRepresentationProvider.php | 38 +++++++++++++- src/Service/ElasticSearchIndex.php | 18 ++++--- src/Service/IndexInterface.php | 2 +- 6 files changed, 113 insertions(+), 9 deletions(-) create mode 100644 src/Api/Filter/EventTagFilter.php diff --git a/config/services.yaml b/config/services.yaml index 96a83e9..007dfa0 100644 --- a/config/services.yaml +++ b/config/services.yaml @@ -24,11 +24,15 @@ services: # add more service definitions when explicit configuration is needed # please note that last definitions always *replace* previous ones + + App\Api\State\EventRepresentationProvider: + arguments: + $filterLocator: '@api_platform.filter_locator' + App\Command\FixturesLoadCommand: arguments: $appEnv: '%env(string:APP_ENV)%' - Elastic\Elasticsearch\Client: factory: [ '@Elastic\Elasticsearch\ClientBuilder', fromConfig ] arguments: diff --git a/src/Api/Dto/Event.php b/src/Api/Dto/Event.php index b8bff69..4b5ec25 100644 --- a/src/Api/Dto/Event.php +++ b/src/Api/Dto/Event.php @@ -2,10 +2,12 @@ namespace App\Api\Dto; +use ApiPlatform\Metadata\ApiFilter; use ApiPlatform\Metadata\ApiProperty; use ApiPlatform\Metadata\ApiResource; use ApiPlatform\Metadata\Get; use ApiPlatform\Metadata\GetCollection; +use App\Api\Filter\EventTagFilter; use App\Api\State\EventRepresentationProvider; #[ApiResource(operations: [ @@ -35,6 +37,10 @@ provider: EventRepresentationProvider::class, ), ])] +#[ApiFilter( + EventTagFilter::class, + properties: ['tags'] +)] class Event { #[ApiProperty( diff --git a/src/Api/Filter/EventTagFilter.php b/src/Api/Filter/EventTagFilter.php new file mode 100644 index 0000000..725dbc0 --- /dev/null +++ b/src/Api/Filter/EventTagFilter.php @@ -0,0 +1,52 @@ +getProperties($resourceClass); + $terms = [ + "boost" => 1.0, + ]; + foreach ($properties as $property) { + if (empty($context['filters'][$property])) { + continue; + } + $terms[$property] = $context['filters'][$property]; + } + + return ['terms' => $terms]; + } + + public function getDescription(string $resourceClass): array + { + if (!$this->properties) { + return []; + } + + $description = []; + foreach ($this->properties as $filterParameterName => $value) { + $description[$filterParameterName] = [ + 'property' => $filterParameterName, + 'type' => Type::BUILTIN_TYPE_ARRAY, + 'required' => false, + 'description' => 'Filter base on values given', + 'is_collection' => true, + 'openapi' => [ + 'allowReserved' => false, + 'allowEmptyValue' => true, + 'explode' => false, + ], + ]; + } + + return $description; + } +} diff --git a/src/Api/State/EventRepresentationProvider.php b/src/Api/State/EventRepresentationProvider.php index 1637a79..7fbc257 100644 --- a/src/Api/State/EventRepresentationProvider.php +++ b/src/Api/State/EventRepresentationProvider.php @@ -2,11 +2,18 @@ namespace App\Api\State; +use ApiPlatform\Elasticsearch\Filter\FilterInterface; +use ApiPlatform\Elasticsearch\Filter\OrderFilter; use ApiPlatform\Metadata\CollectionOperationInterface; use ApiPlatform\Metadata\Operation; use ApiPlatform\State\ProviderInterface; use App\Model\IndexNames; use App\Service\IndexInterface; +use ApiPlatform\Elasticsearch\Filter\AbstractFilter; +use Psr\Container\ContainerExceptionInterface; +use Psr\Container\ContainerInterface; +use Psr\Container\NotFoundExceptionInterface; + final class EventRepresentationProvider implements ProviderInterface { @@ -15,13 +22,42 @@ final class EventRepresentationProvider implements ProviderInterface public function __construct( private readonly IndexInterface $index, + private readonly ContainerInterface $filterLocator, ) { } + /** + * @throws ContainerExceptionInterface + * @throws NotFoundExceptionInterface + */ public function provide(Operation $operation, array $uriVariables = [], array $context = []): object|array|null { + $resourceFilters = $operation?->getFilters(); + $orderFilters = []; + $outputFilters = []; + + foreach ($resourceFilters as $filterId) { + $filter = $this->filterLocator->has($filterId) ? $this->filterLocator->get($filterId) : null; + + + if ($filter instanceof FilterInterface) { + // Apply the OrderFilter after every. + if ($filter instanceof OrderFilter) { + $orderFilters[$filterId] = $filter; + continue; + } + + $outputFilters[$filterId] = $filter->apply([], IndexNames::Events->value, $operation, $context); + } + } + + foreach ($orderFilters as $filterId => $orderFilter) { + $outputFilters['orderFilters'] ??= []; + $outputFilters['orderFilters'][$filterId] = $orderFilter->apply([], IndexNames::Events->value, $operation, $context); + } + if ($operation instanceof CollectionOperationInterface) { - $data = $this->index->getAll(IndexNames::Events->value, 0, self::PAGE_SIZE); + $data = $this->index->getAll(IndexNames::Events->value, $outputFilters, $context['filters']['page'], self::PAGE_SIZE); return $data['hits']; } diff --git a/src/Service/ElasticSearchIndex.php b/src/Service/ElasticSearchIndex.php index 3b91477..e9432aa 100644 --- a/src/Service/ElasticSearchIndex.php +++ b/src/Service/ElasticSearchIndex.php @@ -52,15 +52,11 @@ public function get(string $indexName, int $id): array return $this->parseResponse($response); } - public function getAll(string $indexName, int $from = 0, int $size = 10): array + public function getAll(string $indexName, array $filters = [], int $from = 0, int $size = 10): array { $params = [ 'index' => $indexName, - 'query' => [ - 'query_string' => [ - 'query' => '*', - ], - ], + 'query' => [], 'size' => $size, 'from' => $from, 'sort' => [ @@ -68,6 +64,16 @@ public function getAll(string $indexName, int $from = 0, int $size = 10): array ], ]; + foreach ($filters as $filter) { + $params['query'] += $filter; + } + + if (empty($params['query'])) { + $params['query'] += [ + "match_all" => [], + ]; + } + $response = $this->client->search($params); $data = $this->parseResponse($response); diff --git a/src/Service/IndexInterface.php b/src/Service/IndexInterface.php index c31a7a5..47049ba 100644 --- a/src/Service/IndexInterface.php +++ b/src/Service/IndexInterface.php @@ -17,5 +17,5 @@ public function indexExists(string $indexName): bool; public function get(string $indexName, int $id): array; - public function getAll(string $indexName, int $from = 0, int $size = 10): array; + public function getAll(string $indexName, array $filters = [], int $from = 0, int $size = 10): array; } From 6b4031ae524fb209ce8f7e0983b3d7facd0aed52 Mon Sep 17 00:00:00 2001 From: Jesper Kristensen Date: Wed, 10 Jan 2024 16:33:23 +0100 Subject: [PATCH 2/5] #419: Added api spec tests to actions --- .github/workflows/pr.yaml | 42 +++++++- CHANGELOG.md | 1 + composer.json | 3 + public/spec.yaml | 214 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 258 insertions(+), 2 deletions(-) create mode 100644 public/spec.yaml diff --git a/.github/workflows/pr.yaml b/.github/workflows/pr.yaml index 1969bbd..5b4fd0f 100644 --- a/.github/workflows/pr.yaml +++ b/.github/workflows/pr.yaml @@ -170,11 +170,49 @@ jobs: - name: markdownlint run: yarn run coding-standards-check - changelog: + apispec: runs-on: ubuntu-latest - name: Changelog should be updated + name: API Specification validation strategy: fail-fast: false + matrix: + php: [ '8.2' ] + steps: + - name: Checkout + uses: actions/checkout@v3 + with: + fetch-depth: 2 + + - name: Setup PHP, with composer and extensions + uses: shivammathur/setup-php@v2 + with: + php-version: ${{ matrix.php}} + extensions: ctype, iconv, imagick, json, redis, soap, xmlreader, zip + coverage: none + + - name: Get composer cache directory + id: composer-cache + run: echo "dir=$(composer config cache-files-dir)" >> $GITHUB_OUTPUT + + - name: Cache dependencies + uses: actions/cache@v3 + with: + path: ${{ steps.composer-cache.outputs.dir }} + key: ${{ runner.os }}-composer-${{ hashFiles('**/composer.lock') }} + restore-keys: ${{ runner.os }}-composer- + + - name: Install Dependencies + run: composer install -q --no-ansi --no-interaction --no-scripts --no-suggest --no-progress --prefer-dist + + - name: Export specifications + run: bin/console api:openapi:export --yaml --output=public/spec.yaml --no-interaction + + - name: Check for changes in specifications + run: git diff --diff-filter=ACMRT --exit-code public/spec.yaml + + changelog: + runs-on: ubuntu-latest + name: Changelog should be updated steps: - name: Checkout uses: actions/checkout@v2 diff --git a/CHANGELOG.md b/CHANGELOG.md index f6335d1..a867d75 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ See [keep a changelog] for information about writing changes to this log. - Added fixture data loader service and command to load fixtures. - Added basic index service. - Basic events DTO added. +- Added tags filter [keep a changelog]: https://keepachangelog.com/en/1.1.0/ [unreleased]: https://github.com/itk-dev/event-database-imports/compare/main...develop diff --git a/composer.json b/composer.json index 0e86e7a..4aa85db 100644 --- a/composer.json +++ b/composer.json @@ -71,6 +71,9 @@ "post-update-cmd": [ "@auto-scripts" ], + "api-spec-export": [ + "bin/console api:openapi:export --yaml --output public/spec.yaml" + ], "coding-standards-apply": [ "PHP_CS_FIXER_IGNORE_ENV=1 vendor/bin/php-cs-fixer fix" ], diff --git a/public/spec.yaml b/public/spec.yaml new file mode 100644 index 0000000..ecca37d --- /dev/null +++ b/public/spec.yaml @@ -0,0 +1,214 @@ +openapi: 3.1.0 +info: + title: 'Event database API' + description: '' + version: 2.0.0 +servers: + - + url: / + description: '' +paths: + /api/v2/events: + get: + operationId: api_events_get_collection + tags: + - Event + responses: + 200: + description: 'Event collection' + content: + application/ld+json: + schema: + type: object + properties: + 'hydra:member': { type: array, items: { $ref: '#/components/schemas/Event.EventRepresentationProvider.jsonld' } } + 'hydra:totalItems': { type: integer, minimum: 0 } + 'hydra:view': { type: object, properties: { '@id': { type: string, format: iri-reference }, '@type': { type: string }, 'hydra:first': { type: string, format: iri-reference }, 'hydra:last': { type: string, format: iri-reference }, 'hydra:previous': { type: string, format: iri-reference }, 'hydra:next': { type: string, format: iri-reference } }, example: { '@id': string, type: string, 'hydra:first': string, 'hydra:last': string, 'hydra:previous': string, 'hydra:next': string } } + 'hydra:search': { type: object, properties: { '@type': { type: string }, 'hydra:template': { type: string }, 'hydra:variableRepresentation': { type: string }, 'hydra:mapping': { type: array, items: { type: object, properties: { '@type': { type: string }, variable: { type: string }, property: { type: [string, 'null'] }, required: { type: boolean } } } } } } + required: + - 'hydra:member' + summary: 'Retrieves the collection of Event resources.' + description: 'Retrieves the collection of Event resources.' + parameters: + - + name: page + in: query + description: 'The collection page number' + required: false + deprecated: false + allowEmptyValue: true + schema: + type: integer + default: 1 + style: form + explode: false + allowReserved: false + - + name: tags + in: query + description: 'Filter base on values given' + required: false + deprecated: false + allowEmptyValue: true + schema: + type: array + items: + type: string + style: deepObject + explode: false + allowReserved: false + deprecated: false + parameters: [] + '/api/v2/events/{id}': + get: + operationId: api_events_id_get + tags: + - Event + responses: + 200: + description: 'Single event' + summary: 'Retrieves a Event resource.' + description: 'Retrieves a Event resource.' + parameters: + - + name: id + in: path + description: '' + required: true + deprecated: false + allowEmptyValue: false + schema: + type: integer + style: simple + explode: false + allowReserved: false + deprecated: false + parameters: [] + /api/v2/organizations: + get: + operationId: api_organizations_get_collection + tags: + - Organization + responses: + 200: + description: 'Organization collection' + content: + application/ld+json: + schema: + type: object + properties: + 'hydra:member': { type: array, items: { $ref: '#/components/schemas/Organization.OrganizationRepresentationProvider.jsonld' } } + 'hydra:totalItems': { type: integer, minimum: 0 } + 'hydra:view': { type: object, properties: { '@id': { type: string, format: iri-reference }, '@type': { type: string }, 'hydra:first': { type: string, format: iri-reference }, 'hydra:last': { type: string, format: iri-reference }, 'hydra:previous': { type: string, format: iri-reference }, 'hydra:next': { type: string, format: iri-reference } }, example: { '@id': string, type: string, 'hydra:first': string, 'hydra:last': string, 'hydra:previous': string, 'hydra:next': string } } + 'hydra:search': { type: object, properties: { '@type': { type: string }, 'hydra:template': { type: string }, 'hydra:variableRepresentation': { type: string }, 'hydra:mapping': { type: array, items: { type: object, properties: { '@type': { type: string }, variable: { type: string }, property: { type: [string, 'null'] }, required: { type: boolean } } } } } } + required: + - 'hydra:member' + summary: 'Retrieves the collection of Organization resources.' + description: 'Retrieves the collection of Organization resources.' + parameters: + - + name: page + in: query + description: 'The collection page number' + required: false + deprecated: false + allowEmptyValue: true + schema: + type: integer + default: 1 + style: form + explode: false + allowReserved: false + deprecated: false + parameters: [] + '/api/v2/organizations/{id}': + get: + operationId: api_organizations_id_get + tags: + - Organization + responses: + 200: + description: 'Single organization' + summary: 'Get single organization base on identifier' + description: 'Retrieves a Organization resource.' + parameters: + - + name: id + in: path + description: '' + required: true + deprecated: false + allowEmptyValue: false + schema: + type: integer + style: simple + explode: false + allowReserved: false + deprecated: false + parameters: [] +components: + schemas: + Event.EventRepresentationProvider.jsonld: + type: object + description: '' + deprecated: false + properties: + '@id': + readOnly: true + type: string + '@type': + readOnly: true + type: string + '@context': + readOnly: true + oneOf: + - + type: string + - + type: object + properties: + '@vocab': + type: string + hydra: + type: string + enum: ['http://www.w3.org/ns/hydra/core#'] + required: + - '@vocab' + - hydra + additionalProperties: true + Organization.OrganizationRepresentationProvider.jsonld: + type: object + description: '' + deprecated: false + properties: + '@id': + readOnly: true + type: string + '@type': + readOnly: true + type: string + '@context': + readOnly: true + oneOf: + - + type: string + - + type: object + properties: + '@vocab': + type: string + hydra: + type: string + enum: ['http://www.w3.org/ns/hydra/core#'] + required: + - '@vocab' + - hydra + additionalProperties: true + responses: { } + parameters: { } + examples: { } + requestBodies: { } + headers: { } + securitySchemes: { } +security: [] +tags: [] From d1ad0ec2f94edcd59d2e0180693abee32c9570e6 Mon Sep 17 00:00:00 2001 From: Jesper Kristensen Date: Wed, 10 Jan 2024 16:49:16 +0100 Subject: [PATCH 3/5] #419: Refactored state providers --- baseline.xml | 5 ++ config/services.yaml | 2 +- src/Api/Filter/EventTagFilter.php | 5 +- src/Api/State/AbstractProvider.php | 49 +++++++++++++++++++ src/Api/State/EventRepresentationProvider.php | 41 ++-------------- .../OrganizationRepresentationProvider.php | 11 ++--- src/Service/ElasticSearchIndex.php | 2 +- 7 files changed, 64 insertions(+), 51 deletions(-) create mode 100644 src/Api/State/AbstractProvider.php diff --git a/baseline.xml b/baseline.xml index 803ab9a..6fb232a 100644 --- a/baseline.xml +++ b/baseline.xml @@ -1,5 +1,10 @@ + + + getFilters + + ProviderInterface diff --git a/config/services.yaml b/config/services.yaml index 007dfa0..f0f9aad 100644 --- a/config/services.yaml +++ b/config/services.yaml @@ -25,7 +25,7 @@ services: # please note that last definitions always *replace* previous ones - App\Api\State\EventRepresentationProvider: + App\Api\State\AbstractProvider: arguments: $filterLocator: '@api_platform.filter_locator' diff --git a/src/Api/Filter/EventTagFilter.php b/src/Api/Filter/EventTagFilter.php index 725dbc0..6ed1d3f 100644 --- a/src/Api/Filter/EventTagFilter.php +++ b/src/Api/Filter/EventTagFilter.php @@ -2,18 +2,17 @@ namespace App\Api\Filter; -use ApiPlatform\Metadata\Operation; use ApiPlatform\Elasticsearch\Filter\AbstractFilter; +use ApiPlatform\Metadata\Operation; use Symfony\Component\PropertyInfo\Type; - final class EventTagFilter extends AbstractFilter { public function apply(array $clauseBody, string $resourceClass, Operation $operation = null, array $context = []): array { $properties = $this->getProperties($resourceClass); $terms = [ - "boost" => 1.0, + 'boost' => 1.0, ]; foreach ($properties as $property) { if (empty($context['filters'][$property])) { diff --git a/src/Api/State/AbstractProvider.php b/src/Api/State/AbstractProvider.php new file mode 100644 index 0000000..fbb1990 --- /dev/null +++ b/src/Api/State/AbstractProvider.php @@ -0,0 +1,49 @@ +getFilters(); + $orderFilters = []; + $outputFilters = []; + + if (!is_null($resourceFilters)) { + foreach ($resourceFilters as $filterId) { + $filter = $this->filterLocator->has($filterId) ? $this->filterLocator->get($filterId) : null; + + if ($filter instanceof FilterInterface) { + // Apply the OrderFilter after every. + if ($filter instanceof OrderFilter) { + $orderFilters[$filterId] = $filter; + continue; + } + + $outputFilters[$filterId] = $filter->apply([], IndexNames::Events->value, $operation, $context); + } + } + + foreach ($orderFilters as $filterId => $orderFilter) { + $outputFilters['orderFilters'] ??= []; + $outputFilters['orderFilters'][$filterId] = $orderFilter->apply([], IndexNames::Events->value, $operation, $context); + } + } + + return $outputFilters; + } +} diff --git a/src/Api/State/EventRepresentationProvider.php b/src/Api/State/EventRepresentationProvider.php index 7fbc257..07d5347 100644 --- a/src/Api/State/EventRepresentationProvider.php +++ b/src/Api/State/EventRepresentationProvider.php @@ -2,62 +2,27 @@ namespace App\Api\State; -use ApiPlatform\Elasticsearch\Filter\FilterInterface; -use ApiPlatform\Elasticsearch\Filter\OrderFilter; use ApiPlatform\Metadata\CollectionOperationInterface; use ApiPlatform\Metadata\Operation; use ApiPlatform\State\ProviderInterface; use App\Model\IndexNames; -use App\Service\IndexInterface; -use ApiPlatform\Elasticsearch\Filter\AbstractFilter; use Psr\Container\ContainerExceptionInterface; -use Psr\Container\ContainerInterface; use Psr\Container\NotFoundExceptionInterface; - -final class EventRepresentationProvider implements ProviderInterface +final class EventRepresentationProvider extends AbstractProvider implements ProviderInterface { // @TODO: should we create enum with 5,10,15,20 public const PAGE_SIZE = 10; - public function __construct( - private readonly IndexInterface $index, - private readonly ContainerInterface $filterLocator, - ) { - } - /** * @throws ContainerExceptionInterface * @throws NotFoundExceptionInterface */ public function provide(Operation $operation, array $uriVariables = [], array $context = []): object|array|null { - $resourceFilters = $operation?->getFilters(); - $orderFilters = []; - $outputFilters = []; - - foreach ($resourceFilters as $filterId) { - $filter = $this->filterLocator->has($filterId) ? $this->filterLocator->get($filterId) : null; - - - if ($filter instanceof FilterInterface) { - // Apply the OrderFilter after every. - if ($filter instanceof OrderFilter) { - $orderFilters[$filterId] = $filter; - continue; - } - - $outputFilters[$filterId] = $filter->apply([], IndexNames::Events->value, $operation, $context); - } - } - - foreach ($orderFilters as $filterId => $orderFilter) { - $outputFilters['orderFilters'] ??= []; - $outputFilters['orderFilters'][$filterId] = $orderFilter->apply([], IndexNames::Events->value, $operation, $context); - } - if ($operation instanceof CollectionOperationInterface) { - $data = $this->index->getAll(IndexNames::Events->value, $outputFilters, $context['filters']['page'], self::PAGE_SIZE); + $filters = $this->getFilters($operation, $context); + $data = $this->index->getAll(IndexNames::Events->value, $filters, $context['filters']['page'] * self::PAGE_SIZE, self::PAGE_SIZE); return $data['hits']; } diff --git a/src/Api/State/OrganizationRepresentationProvider.php b/src/Api/State/OrganizationRepresentationProvider.php index d79bc82..8198907 100644 --- a/src/Api/State/OrganizationRepresentationProvider.php +++ b/src/Api/State/OrganizationRepresentationProvider.php @@ -6,22 +6,17 @@ use ApiPlatform\Metadata\Operation; use ApiPlatform\State\ProviderInterface; use App\Model\IndexNames; -use App\Service\IndexInterface; -final class OrganizationRepresentationProvider implements ProviderInterface +final class OrganizationRepresentationProvider extends AbstractProvider implements ProviderInterface { // @TODO: should we create enum with 5,10,15,20 public const PAGE_SIZE = 10; - public function __construct( - private readonly IndexInterface $index, - ) { - } - public function provide(Operation $operation, array $uriVariables = [], array $context = []): object|array|null { if ($operation instanceof CollectionOperationInterface) { - $data = $this->index->getAll(IndexNames::Organization->value, 0, self::PAGE_SIZE); + $filters = $this->getFilters($operation, $context); + $data = $this->index->getAll(IndexNames::Organization->value, $filters, $context['filters']['page'] * self::PAGE_SIZE, self::PAGE_SIZE); return $data['hits']; } diff --git a/src/Service/ElasticSearchIndex.php b/src/Service/ElasticSearchIndex.php index e9432aa..3f98a84 100644 --- a/src/Service/ElasticSearchIndex.php +++ b/src/Service/ElasticSearchIndex.php @@ -70,7 +70,7 @@ public function getAll(string $indexName, array $filters = [], int $from = 0, in if (empty($params['query'])) { $params['query'] += [ - "match_all" => [], + 'match_all' => [], ]; } From 424514c15c37c608ba09bb9316ee2f9242c54cda Mon Sep 17 00:00:00 2001 From: Jesper Kristensen Date: Wed, 10 Jan 2024 17:49:21 +0100 Subject: [PATCH 4/5] 419: Cleaned up search results --- baseline.xml | 4 ++ config/services.yaml | 8 +++ src/Api/Filter/EventTagFilter.php | 11 ++-- src/Api/State/AbstractProvider.php | 43 ++++++++++++- src/Api/State/EventRepresentationProvider.php | 16 ++--- .../OrganizationRepresentationProvider.php | 4 +- src/Service/ElasticSearchIndex.php | 62 +++++++++++++++---- 7 files changed, 116 insertions(+), 32 deletions(-) diff --git a/baseline.xml b/baseline.xml index 6fb232a..7675662 100644 --- a/baseline.xml +++ b/baseline.xml @@ -6,12 +6,16 @@ + + index->getAll(IndexNames::Events->value, $filters, $from, self::PAGE_SIZE)]]> + ProviderInterface + index->getAll(IndexNames::Organization->value, $filters, $from, self::PAGE_SIZE)]]> index->get(IndexNames::Organization->value, $uriVariables['id'])['_source']]]]> diff --git a/config/services.yaml b/config/services.yaml index f0f9aad..0aca490 100644 --- a/config/services.yaml +++ b/config/services.yaml @@ -29,6 +29,14 @@ services: arguments: $filterLocator: '@api_platform.filter_locator' + App\Api\State\EventRepresentationProvider: + arguments: + $filterLocator: '@api_platform.filter_locator' + + App\Api\State\OrganizationRepresentationProvider: + arguments: + $filterLocator: '@api_platform.filter_locator' + App\Command\FixturesLoadCommand: arguments: $appEnv: '%env(string:APP_ENV)%' diff --git a/src/Api/Filter/EventTagFilter.php b/src/Api/Filter/EventTagFilter.php index 6ed1d3f..d3c780b 100644 --- a/src/Api/Filter/EventTagFilter.php +++ b/src/Api/Filter/EventTagFilter.php @@ -11,17 +11,18 @@ final class EventTagFilter extends AbstractFilter public function apply(array $clauseBody, string $resourceClass, Operation $operation = null, array $context = []): array { $properties = $this->getProperties($resourceClass); - $terms = [ - 'boost' => 1.0, - ]; + $terms = []; + + /** @var string $property */ foreach ($properties as $property) { if (empty($context['filters'][$property])) { + // If no value or empty value is set, skip it. continue; } - $terms[$property] = $context['filters'][$property]; + $terms[$property] = explode(',', $context['filters'][$property]); } - return ['terms' => $terms]; + return empty($terms) ? $terms : ['terms' => $terms + ['boost' => 1.0]]; } public function getDescription(string $resourceClass): array diff --git a/src/Api/State/AbstractProvider.php b/src/Api/State/AbstractProvider.php index fbb1990..c3b740a 100644 --- a/src/Api/State/AbstractProvider.php +++ b/src/Api/State/AbstractProvider.php @@ -9,14 +9,33 @@ use App\Service\IndexInterface; use Psr\Container\ContainerInterface; +/** + * AbstractProvider class. + */ abstract class AbstractProvider { + protected const PAGE_SIZE = 10; + public function __construct( protected readonly IndexInterface $index, protected readonly ContainerInterface $filterLocator, ) { } + /** + * Retrieves the filters to be applied to the operation. + * + * @param operation $operation + * The operation to retrieve the filters for + * @param array $context + * Additional context for filter application + * + * @return array + * An array of filters to be applied to the operation + * + * @throws \Psr\Container\ContainerExceptionInterface + * @throws \Psr\Container\NotFoundExceptionInterface + */ protected function getFilters(Operation $operation, array $context = []): array { $resourceFilters = $operation->getFilters(); @@ -34,16 +53,36 @@ protected function getFilters(Operation $operation, array $context = []): array continue; } - $outputFilters[$filterId] = $filter->apply([], IndexNames::Events->value, $operation, $context); + $data = $filter->apply([], IndexNames::Events->value, $operation, $context); + if (!empty($data)) { + $outputFilters[$filterId] = $data; + } } } foreach ($orderFilters as $filterId => $orderFilter) { $outputFilters['orderFilters'] ??= []; - $outputFilters['orderFilters'][$filterId] = $orderFilter->apply([], IndexNames::Events->value, $operation, $context); + $data = $orderFilter->apply([], IndexNames::Events->value, $operation, $context); + if (!empty($data)) { + $outputFilters['orderFilters'][$filterId] = $data; + } } } return $outputFilters; } + + /** + * Calculates the offset for a paginated result based on the provided context. + * + * @param array $context + * The context containing the pagination filters + * + * @return int + * The calculated offset value + */ + protected function calculatePageOffset(array $context): int + { + return (($context['filters']['page'] ?? 1) - 1) * self::PAGE_SIZE; + } } diff --git a/src/Api/State/EventRepresentationProvider.php b/src/Api/State/EventRepresentationProvider.php index 07d5347..f128f98 100644 --- a/src/Api/State/EventRepresentationProvider.php +++ b/src/Api/State/EventRepresentationProvider.php @@ -6,25 +6,19 @@ use ApiPlatform\Metadata\Operation; use ApiPlatform\State\ProviderInterface; use App\Model\IndexNames; -use Psr\Container\ContainerExceptionInterface; -use Psr\Container\NotFoundExceptionInterface; final class EventRepresentationProvider extends AbstractProvider implements ProviderInterface { - // @TODO: should we create enum with 5,10,15,20 - public const PAGE_SIZE = 10; - - /** - * @throws ContainerExceptionInterface - * @throws NotFoundExceptionInterface - */ public function provide(Operation $operation, array $uriVariables = [], array $context = []): object|array|null { + // @TODO: should we create enum with 5,10,15,20 + // Get page size from context. + if ($operation instanceof CollectionOperationInterface) { $filters = $this->getFilters($operation, $context); - $data = $this->index->getAll(IndexNames::Events->value, $filters, $context['filters']['page'] * self::PAGE_SIZE, self::PAGE_SIZE); + $from = $this->calculatePageOffset($context); - return $data['hits']; + return $this->index->getAll(IndexNames::Events->value, $filters, $from, self::PAGE_SIZE); } return (object) $this->index->get(IndexNames::Events->value, $uriVariables['id'])['_source']; diff --git a/src/Api/State/OrganizationRepresentationProvider.php b/src/Api/State/OrganizationRepresentationProvider.php index 8198907..adc9b7c 100644 --- a/src/Api/State/OrganizationRepresentationProvider.php +++ b/src/Api/State/OrganizationRepresentationProvider.php @@ -16,9 +16,9 @@ public function provide(Operation $operation, array $uriVariables = [], array $c { if ($operation instanceof CollectionOperationInterface) { $filters = $this->getFilters($operation, $context); - $data = $this->index->getAll(IndexNames::Organization->value, $filters, $context['filters']['page'] * self::PAGE_SIZE, self::PAGE_SIZE); + $from = $this->calculatePageOffset($context); - return $data['hits']; + return $this->index->getAll(IndexNames::Organization->value, $filters, $from, self::PAGE_SIZE); } return [$this->index->get(IndexNames::Organization->value, $uriVariables['id'])['_source']]; diff --git a/src/Service/ElasticSearchIndex.php b/src/Service/ElasticSearchIndex.php index 3f98a84..984a7dd 100644 --- a/src/Service/ElasticSearchIndex.php +++ b/src/Service/ElasticSearchIndex.php @@ -17,6 +17,9 @@ public function __construct( ) { } + /** + * @throws IndexException + */ public function indexExists($indexName): bool { try { @@ -52,39 +55,74 @@ public function get(string $indexName, int $id): array return $this->parseResponse($response); } + /** + * @throws ClientResponseException + * @throws ServerResponseException + * @throws \JsonException + */ public function getAll(string $indexName, array $filters = [], int $from = 0, int $size = 10): array { $params = [ 'index' => $indexName, - 'query' => [], - 'size' => $size, - 'from' => $from, - 'sort' => [ - 'entityId', + 'body' => [ + 'query' => [ + 'match_all' => (object) [], + ], + 'size' => $size, + 'from' => $from, + 'sort' => [], ], ]; + $body = []; foreach ($filters as $filter) { - $params['query'] += $filter; + // @TODO: add order filters to sort + $body += $filter; } - if (empty($params['query'])) { - $params['query'] += [ - 'match_all' => [], - ]; + if (!empty($body)) { + $params['body']['query'] = $body; } $response = $this->client->search($params); - $data = $this->parseResponse($response); + $results = $this->parseResponse($response); - return $data['hits']; + return $this->cleanUpHits($results); } /** + * Parses the response from Elasticsearch and returns it as an array. + * + * @param elasticsearch $response + * The Elasticsearch response object + * + * @return array + * Returns the parsed response as an array + * * @throws \JsonException + * Throws a JSON exception if there is an error while parsing the response */ private function parseResponse(Elasticsearch $response): array { return json_decode((string) $response->getBody(), true, 512, JSON_THROW_ON_ERROR); } + + /** + * Cleans up hits by extracting the "_source" field from the given results array. + * + * @param array $results + * The search results from elastic search + * + * @return array + * The cleaned-up hits + */ + private function cleanUpHits(array $results): array + { + $hits = []; + foreach ($results['hits']['hits'] as $hit) { + $hits[] = $hit['_source']; + } + + return $hits; + } } From 1c97a8a345fc67744ee4d5689b1cbce9c0e4a287 Mon Sep 17 00:00:00 2001 From: Jesper Kristensen Date: Thu, 11 Jan 2024 09:45:49 +0100 Subject: [PATCH 5/5] 419: Refactor code in ES services --- baseline.xml | 5 - src/Api/State/AbstractProvider.php | 49 ++++++---- src/Api/State/EventRepresentationProvider.php | 7 ++ .../OrganizationRepresentationProvider.php | 10 +- src/Model/FilterTypes.php | 12 +++ src/Model/IndexNames.php | 3 + src/Service/ElasticSearchIndex.php | 92 +++++++++++++------ src/Service/IndexInterface.php | 34 +++++++ 8 files changed, 159 insertions(+), 53 deletions(-) create mode 100644 src/Model/FilterTypes.php diff --git a/baseline.xml b/baseline.xml index 7675662..fcc8f59 100644 --- a/baseline.xml +++ b/baseline.xml @@ -53,13 +53,8 @@ - $params $params $indexName]]]> - - $response - $response - diff --git a/src/Api/State/AbstractProvider.php b/src/Api/State/AbstractProvider.php index c3b740a..a42e64c 100644 --- a/src/Api/State/AbstractProvider.php +++ b/src/Api/State/AbstractProvider.php @@ -3,8 +3,9 @@ namespace App\Api\State; use ApiPlatform\Elasticsearch\Filter\FilterInterface; -use ApiPlatform\Elasticsearch\Filter\OrderFilter; +use ApiPlatform\Elasticsearch\Filter\SortFilterInterface; use ApiPlatform\Metadata\Operation; +use App\Model\FilterTypes; use App\Model\IndexNames; use App\Service\IndexInterface; use Psr\Container\ContainerInterface; @@ -39,34 +40,27 @@ public function __construct( protected function getFilters(Operation $operation, array $context = []): array { $resourceFilters = $operation->getFilters(); - $orderFilters = []; - $outputFilters = []; + $outputFilters = [ + FilterTypes::Filters => [], + FilterTypes::Sort => [], + ]; if (!is_null($resourceFilters)) { foreach ($resourceFilters as $filterId) { - $filter = $this->filterLocator->has($filterId) ? $this->filterLocator->get($filterId) : null; + $filter = $this->getFilterById($filterId); if ($filter instanceof FilterInterface) { - // Apply the OrderFilter after every. - if ($filter instanceof OrderFilter) { - $orderFilters[$filterId] = $filter; - continue; - } - $data = $filter->apply([], IndexNames::Events->value, $operation, $context); + if (!empty($data)) { - $outputFilters[$filterId] = $data; + if ($filter instanceof SortFilterInterface) { + $outputFilters[FilterTypes::Sort][] = $data; + } else { + $outputFilters[FilterTypes::Filters][] = $data; + } } } } - - foreach ($orderFilters as $filterId => $orderFilter) { - $outputFilters['orderFilters'] ??= []; - $data = $orderFilter->apply([], IndexNames::Events->value, $operation, $context); - if (!empty($data)) { - $outputFilters['orderFilters'][$filterId] = $data; - } - } } return $outputFilters; @@ -85,4 +79,21 @@ protected function calculatePageOffset(array $context): int { return (($context['filters']['page'] ?? 1) - 1) * self::PAGE_SIZE; } + + /** + * Retrieves a filter based on the provided filter ID. + * + * @param string $filterId + * The ID of the filter to retrieve + * + * @return FilterInterface|null + * The filter instance if found, otherwise null + * + * @throws \Psr\Container\ContainerExceptionInterface + * @throws \Psr\Container\NotFoundExceptionInterface + */ + protected function getFilterById(string $filterId): ?FilterInterface + { + return $this->filterLocator->has($filterId) ? $this->filterLocator->get($filterId) : null; + } } diff --git a/src/Api/State/EventRepresentationProvider.php b/src/Api/State/EventRepresentationProvider.php index f128f98..f4f1abc 100644 --- a/src/Api/State/EventRepresentationProvider.php +++ b/src/Api/State/EventRepresentationProvider.php @@ -6,9 +6,16 @@ use ApiPlatform\Metadata\Operation; use ApiPlatform\State\ProviderInterface; use App\Model\IndexNames; +use Psr\Container\ContainerExceptionInterface; +use Psr\Container\NotFoundExceptionInterface; final class EventRepresentationProvider extends AbstractProvider implements ProviderInterface { + /** + * @throws ContainerExceptionInterface + * @throws NotFoundExceptionInterface + * @throws \App\Exception\IndexException + */ public function provide(Operation $operation, array $uriVariables = [], array $context = []): object|array|null { // @TODO: should we create enum with 5,10,15,20 diff --git a/src/Api/State/OrganizationRepresentationProvider.php b/src/Api/State/OrganizationRepresentationProvider.php index adc9b7c..1a3a631 100644 --- a/src/Api/State/OrganizationRepresentationProvider.php +++ b/src/Api/State/OrganizationRepresentationProvider.php @@ -6,12 +6,16 @@ use ApiPlatform\Metadata\Operation; use ApiPlatform\State\ProviderInterface; use App\Model\IndexNames; +use Psr\Container\ContainerExceptionInterface; +use Psr\Container\NotFoundExceptionInterface; final class OrganizationRepresentationProvider extends AbstractProvider implements ProviderInterface { - // @TODO: should we create enum with 5,10,15,20 - public const PAGE_SIZE = 10; - + /** + * @throws ContainerExceptionInterface + * @throws NotFoundExceptionInterface + * @throws \App\Exception\IndexException + */ public function provide(Operation $operation, array $uriVariables = [], array $context = []): object|array|null { if ($operation instanceof CollectionOperationInterface) { diff --git a/src/Model/FilterTypes.php b/src/Model/FilterTypes.php new file mode 100644 index 0000000..f47ba64 --- /dev/null +++ b/src/Model/FilterTypes.php @@ -0,0 +1,12 @@ + $id, ]; - // @TODO: check status codes. - $response = $this->client->get($params); + try { + /** @var Elasticsearch $response */ + $response = $this->client->get($params); + if (Response::HTTP_OK !== $response->getStatusCode()) { + throw new IndexException('Failed to get document from Elasticsearch', $response->getStatusCode()); + } + $result = $this->parseResponse($response); + } catch (ClientResponseException|ServerResponseException|MissingParameterException|\JsonException $e) { + throw new IndexException($e->getMessage(), $e->getCode(), $e); + } + + return $result; + } + + public function getAll(string $indexName, array $filters = [], int $from = 0, int $size = 10): array + { + $params = $this->buildParams($indexName, $filters, $from, $size); + + try { + /** @var Elasticsearch $response */ + $response = $this->client->search($params); + if (Response::HTTP_OK !== $response->getStatusCode()) { + throw new IndexException('Failed to get document from Elasticsearch', $response->getStatusCode()); + } + $results = $this->parseResponse($response); + } catch (ClientResponseException|ServerResponseException|\JsonException $e) { + throw new IndexException($e->getMessage(), $e->getCode(), $e); + } - return $this->parseResponse($response); + return $this->extractSourceFromHits($results); } /** - * @throws ClientResponseException - * @throws ServerResponseException - * @throws \JsonException + * 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 */ - public function getAll(string $indexName, array $filters = [], int $from = 0, int $size = 10): array + private function buildParams(string $indexName, array $filters, int $from, int $size): array { $params = [ 'index' => $indexName, @@ -70,24 +98,36 @@ public function getAll(string $indexName, array $filters = [], int $from = 0, in ], 'size' => $size, 'from' => $from, + // @TODO: add order filters to sort results 'sort' => [], ], ]; - $body = []; - foreach ($filters as $filter) { - // @TODO: add order filters to sort - $body += $filter; - } - + $body = $this->buildBody($filters); if (!empty($body)) { $params['body']['query'] = $body; } - $response = $this->client->search($params); - $results = $this->parseResponse($response); + 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 = []; + foreach ($filters[FilterTypes::Filters] as $filter) { + $body += $filter; + } - return $this->cleanUpHits($results); + return $body; } /** @@ -116,7 +156,7 @@ private function parseResponse(Elasticsearch $response): array * @return array * The cleaned-up hits */ - private function cleanUpHits(array $results): array + private function extractSourceFromHits(array $results): array { $hits = []; foreach ($results['hits']['hits'] as $hit) { diff --git a/src/Service/IndexInterface.php b/src/Service/IndexInterface.php index 47049ba..e25f8b0 100644 --- a/src/Service/IndexInterface.php +++ b/src/Service/IndexInterface.php @@ -2,6 +2,8 @@ namespace App\Service; +use App\Exception\IndexException; + interface IndexInterface { /** @@ -12,10 +14,42 @@ interface IndexInterface * * @return bool * True if the index exists, false otherwise + * + * @throws IndexException */ public function indexExists(string $indexName): bool; + /** + * Retrieves information from the specified index based on the provided ID. + * + * @param string $indexName + * The name of the index to retrieve information from + * @param int $id + * The ID of the document to retrieve + * + * @return array + * The retrieved document as an array + * + * @throws IndexException + */ public function get(string $indexName, int $id): array; + /** + * Retrieves documents from the specified index with optional filters and pagination. + * + * @param string $indexName + * The name of the index to retrieve data from + * @param array $filters + * An array of filters to apply to the retrieved documents keyed by FilterTypes. Default is an empty array. + * @param int $from + * The starting page. Default is 0. + * @param int $size + * The maximum number of records to retrieve. Default is 10. + * + * @return array + * An array containing the retrieved documents + * + * @throws IndexException + */ public function getAll(string $indexName, array $filters = [], int $from = 0, int $size = 10): array; }