From b55caff43f1f76fa64b12e989c53e09b92f4ecb9 Mon Sep 17 00:00:00 2001 From: Tom H Anderson Date: Sun, 4 Oct 2026 19:25:35 -0600 Subject: [PATCH] Add _or to filters A filter's _or is a list of branches, any of which a row matches. Each branch is a filter whose own filters are all matched, of a FilterBranch_ type: the same filters less sort and sortPriority, with an _or of its own. Collection filters and computed fields, with their args, have it too. - Filter\QueryBuilder's filter methods return their conditions rather than adding them; apply() adds the top-level conditions one by one, so a filter without _or builds the same DQL. A branch with no conditions is 1 = 1, keeping the parameters other branches bound; _or: [] matches nothing, as in: [] does. - The filterDepth (default 3) and filterConditions (default 100) config limit _or before any DQL is built, with a client error. Only conditions inside _or are counted; 0 or null is unlimited. - A field, alias or computed field named _or throws Exception\Metadata. - A field given null, such as { name: null }, raised a PHP warning; it is not applied, as a filter given null is not. Co-Authored-By: Claude Opus 5.5 --- CLAUDE.md | 3 + docs/driver.rst | 25 + docs/queries.rst | 41 +- docs/versions.rst | 2 + src/Config.php | 50 +- src/ConfigBuilder.php | 46 +- src/Filter/FilterFactory.php | 83 +++- src/Filter/QueryBuilder.php | 310 ++++++++++--- src/Resolve/ResolveCollectionFactory.php | 3 +- src/Resolve/ResolveEntityFactory.php | 5 +- src/Type/Entity/Entity.php | 13 +- .../ComputedField/ComputedExpressionData.php | 54 +++ .../ComputedFieldExpressionTest.php | 37 +- .../Filter/InverseOneToOneFilterTest.php | 4 +- test/Feature/Filter/OrFilterTest.php | 426 ++++++++++++++++++ .../Filter/ToOneAssociationFiltersTest.php | 2 +- test/Feature/Metadata/EmbeddedTest.php | 2 +- 17 files changed, 971 insertions(+), 135 deletions(-) create mode 100644 test/Feature/ComputedField/ComputedExpressionData.php create mode 100644 test/Feature/Filter/OrFilterTest.php diff --git a/CLAUDE.md b/CLAUDE.md index 7451f54..231cc6d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -120,6 +120,7 @@ Filters are auto-generated for all exposed fields and associations (src/Filter/) - Date and time filter values are bound as the field's Doctrine type - Filters are context-aware based on field type - Can be excluded globally via Config or per-entity/field via attributes +- A filter's `_or` is a list of branches, any of which a row matches; a branch is of a `FilterBranch_` type, the same filters less `sort` and `sortPriority`, whose own `_or` is a list of itself. `Filter\QueryBuilder` builds each filter's condition as a string (binding its parameters) and the caller ANDs them, or ORs the branches; a branch with no conditions is `1 = 1` and `_or: []` is `1 = 0`. `filterDepth` and `filterConditions` limit `_or` before any DQL is built; only conditions inside `_or` are counted. No field may be named or aliased `_or` ### Resolution and Hydration @@ -150,6 +151,8 @@ Filters are auto-generated for all exposed fields and associations (src/Filter/) - `useHydratorCache` - Cache hydrator results for as long as the entity exists - `useQueryResultCache` - Cache query results for identical SQL and parameters until the entity manager is cleared or flushed (`Cache\QueryResultCacheListener`) - `limit` - Default limit on the rows of a connection (default: 1000); an entity's `limit`, then an association's, replaces it, even when larger +- `filterDepth` - How deeply a filter's `_or` may nest (default 3); 0 or null is unlimited +- `filterConditions` - The most conditions inside a filter's `_or` branches (default 100); 0 or null is unlimited - `extractByValue` - Extract by value vs reference for every entity, overriding the `extractByValue` argument of the `#[Entity]` attribute - `entityPrefix` - Remove prefix from type names - `sortFields` - Sort fields alphabetically diff --git a/docs/driver.rst b/docs/driver.rst index b0fc0f6..09ed6c3 100644 --- a/docs/driver.rst +++ b/docs/driver.rst @@ -29,6 +29,8 @@ Creating a Driver with all config options 'useNonNullTypes' => true, 'useQueryResultCache' => true, 'excludeFilters' => [Filters::CONTAINS], + 'filterConditions' => 100, + 'filterDepth' => 3, 'formatJsonAs' => JsonFormat::Object, ]); @@ -104,6 +106,29 @@ extract by reference. When not set the individual entity attribute value is used and that is, by default, extract by value. +filterConditions +---------------- + +The most filters the branches of a filter's `_or `_ may have +in all, including those of nested ``_or``. A filter given ``null`` is not +counted, nor are a computed field's ``args``; neither are the filters outside +``_or``, so a filter without one is never limited. An ``in`` list is one +filter whatever its length. Beyond it, the query is a client error. It must +be at least 0; 0 or ``null`` is unlimited. Default is 100. + + +filterDepth +----------- + +How deeply a filter's `_or `_ may nest: an ``_or`` of the +filter is at depth 1, and an ``_or`` of one of its branches at depth 2. +Beyond it, the query is a client error. It must be at least 0; 0 or ``null`` +is unlimited. Default is 3. + +GraphQL's query depth and complexity rules count the fields a query selects, +not its arguments, so they do not limit a filter. + + formatJsonAs ------------ diff --git a/docs/queries.rst b/docs/queries.rst index 3396883..b310ee6 100644 --- a/docs/queries.rst +++ b/docs/queries.rst @@ -170,8 +170,40 @@ To select a list of years } -All filters are **AND** filters. For **OR** support use multiple -queries and aggregate them. +Or +-- + +The filters of a ``filter`` are all matched: they are **AND** filters. Its +``_or`` is a list of filters, any of which a row matches: + +.. code-block:: js + + filter: { + id: { gt: 1 } + _or: [ + { name: { startswith: "G" } } + { name: { eq: "Phish" }, year: { lt: 1990 } } + ] + } + +This matches rows whose ``id`` is greater than 1 and which either have a name +beginning with G, or are named Phish with a year before 1990. Each branch is a +filter whose own filters are all matched, and which may have an ``_or`` of +its own. A branch has no ``sort`` or ``sortPriority``, which mean nothing in +it; its type is named ``FilterBranch_`` followed by the entity's type name, as +the filter's is named ``Filter_``. + +A branch with no filters, or whose filters are all given ``null``, matches +every row, so the whole ``_or`` does. An ``_or`` of no branches, ``_or: []``, +matches no row, as ``in: []`` does; ``_or: null`` is not applied. + +How deeply ``_or`` may nest, and how many filters its branches may have in +all, are limited by the `filterDepth and filterConditions +`_ config; beyond either is an error. + +A field may not be named ``_or``, nor aliased as it: one which is throws +``ApiSkeletons\Doctrine\ORM\GraphQL\Exception\Metadata`` when its type is +built. Pagination @@ -396,7 +428,10 @@ Filters have a cost too, which these rules do not count. ``contains``, ``endswith`` begin with a wildcard, so no ordinary index serves them and each reads every row; whether an index serves ``startswith`` depends on the database. A ``sort`` on a column without an index sorts every matching row -before the page is taken. An ``in`` or ``notin`` list binds a parameter for +before the page is taken. An ``_or`` of filters on different columns often +stops the database using a single index; PostgreSQL's bitmap OR and MySQL's +``index_merge`` can combine indexes, but neither is certain. The +``filterDepth`` and ``filterConditions`` config limit the size of an ``_or``. An ``in`` or ``notin`` list binds a parameter for each of its values, and databases limit them: Oracle allows 1000 values in a list and SQL Server 2100 parameters in a query. Index the columns clients filter and sort on, and leave out the filters a large table should not offer diff --git a/docs/versions.rst b/docs/versions.rst index 484661a..8bac4f6 100644 --- a/docs/versions.rst +++ b/docs/versions.rst @@ -61,6 +61,8 @@ ORM versions do not support ``doctrine/persistence`` 4. each parameter of its method - 14.1: a computed field with an ``expression``, its value in DQL, is filtered and sorted by it; see `computed fields `_ +- 14.1: a filter's ``_or`` matches any of its branches, limited by the ``filterDepth`` + and ``filterConditions`` config; see `or `_ - Metadata is read as typed value objects, and cached metadata carries its format version and the config it was built with - Configuration mistakes, such as a misplaced attribute, an invalid name or a field diff --git a/src/Config.php b/src/Config.php index 31a91b0..45e7778 100644 --- a/src/Config.php +++ b/src/Config.php @@ -72,6 +72,18 @@ final class Config /** @var int A hard limit for fetching any collection within the schema */ protected readonly int $limit; + /** + * @var int|null How deeply a filter's _or may nest: a top-level _or is at + * depth 1. 0 or null is unlimited. + */ + protected readonly int|null $filterDepth; + + /** + * @var int|null The most conditions a filter may have inside its _or + * branches. 0 or null is unlimited. + */ + protected readonly int|null $filterConditions; + /** * @var bool|null When set to true, all entities will be extracted by value * across all hydrators in the driver. When set to false, @@ -125,6 +137,8 @@ final class Config 'batchAssociations' => ['bool'], 'batchLimit' => ['int'], 'limit' => ['int'], + 'filterDepth' => ['int', 'null'], + 'filterConditions' => ['int', 'null'], 'extractByValue' => ['bool', 'null'], 'entityPrefix' => ['string', 'null'], 'sortFields' => ['bool'], @@ -144,6 +158,8 @@ public function __construct(array $config = []) 'batchAssociations' => true, 'batchLimit' => 1000, 'limit' => 1000, + 'filterDepth' => 3, + 'filterConditions' => 100, 'extractByValue' => null, 'entityPrefix' => null, 'sortFields' => false, @@ -173,6 +189,8 @@ public function __construct(array $config = []) $this->batchAssociations = $mergedConfig['batchAssociations']; $this->batchLimit = $mergedConfig['batchLimit']; $this->limit = $mergedConfig['limit']; + $this->filterDepth = $mergedConfig['filterDepth']; + $this->filterConditions = $mergedConfig['filterConditions']; $this->extractByValue = $mergedConfig['extractByValue']; $this->entityPrefix = $mergedConfig['entityPrefix']; $this->sortFields = $mergedConfig['sortFields']; @@ -187,7 +205,7 @@ public function __construct(array $config = []) * * @param array $config * - * @return array{group: string, groupSuffix: string|null, useHydratorCache: bool, useQueryResultCache: bool, batchAssociations: bool, batchLimit: int, limit: int, extractByValue: bool|null, entityPrefix: string|null, sortFields: bool, excludeFilters: Filters[], formatJsonAs: JsonFormat, useNonNullTypes: bool} + * @return array{group: string, groupSuffix: string|null, useHydratorCache: bool, useQueryResultCache: bool, batchAssociations: bool, batchLimit: int, limit: int, filterDepth: int|null, filterConditions: int|null, extractByValue: bool|null, entityPrefix: string|null, sortFields: bool, excludeFilters: Filters[], formatJsonAs: JsonFormat, useNonNullTypes: bool} * * @throws ConfigurationException */ @@ -222,6 +240,17 @@ private function validate(array $config): array ); } + foreach (['filterDepth', 'filterConditions'] as $field) { + $value = $config[$field]; + assert(is_int($value) || $value === null); + + if ($value !== null && $value < 0) { + throw new ConfigurationException( + 'Invalid configuration value for ' . $field . ': it must be at least 0, got ' . $value . '.', + ); + } + } + $excludeFilters = []; /** @psalm-suppress MixedAssignment Each filter is checked */ foreach ($config['excludeFilters'] as $filter) { @@ -247,7 +276,7 @@ private function validate(array $config): array ); } - /** @var array{group: string, groupSuffix: string|null, useHydratorCache: bool, useQueryResultCache: bool, batchAssociations: bool, batchLimit: int, limit: int, extractByValue: bool|null, entityPrefix: string|null, sortFields: bool, excludeFilters: Filters[], formatJsonAs: JsonFormat, useNonNullTypes: bool} $validated */ + /** @var array{group: string, groupSuffix: string|null, useHydratorCache: bool, useQueryResultCache: bool, batchAssociations: bool, batchLimit: int, limit: int, filterDepth: int|null, filterConditions: int|null, extractByValue: bool|null, entityPrefix: string|null, sortFields: bool, excludeFilters: Filters[], formatJsonAs: JsonFormat, useNonNullTypes: bool} $validated */ $validated = $config; return $validated; @@ -288,6 +317,23 @@ public function getLimit(): int return $this->limit; } + /** + * How deeply a filter's _or may nest, or null when it is unlimited + */ + public function getFilterDepth(): int|null + { + return $this->filterDepth === 0 ? null : $this->filterDepth; + } + + /** + * The most conditions a filter may have inside its _or branches, or null + * when it is unlimited + */ + public function getFilterConditions(): int|null + { + return $this->filterConditions === 0 ? null : $this->filterConditions; + } + public function getExtractByValue(): bool|null { return $this->extractByValue; diff --git a/src/ConfigBuilder.php b/src/ConfigBuilder.php index ea232f3..9eeacd5 100644 --- a/src/ConfigBuilder.php +++ b/src/ConfigBuilder.php @@ -22,16 +22,18 @@ */ final class ConfigBuilder { - private string $group = 'default'; - private string|null $groupSuffix = null; - private bool $useHydratorCache = false; - private bool $useQueryResultCache = false; - private bool $batchAssociations = true; - private int $batchLimit = 1000; - private int $limit = 1000; - private bool|null $extractByValue = null; - private string|null $entityPrefix = null; - private bool $sortFields = false; + private string $group = 'default'; + private string|null $groupSuffix = null; + private bool $useHydratorCache = false; + private bool $useQueryResultCache = false; + private bool $batchAssociations = true; + private int $batchLimit = 1000; + private int $limit = 1000; + private int|null $filterDepth = 3; + private int|null $filterConditions = 100; + private bool|null $extractByValue = null; + private string|null $entityPrefix = null; + private bool $sortFields = false; /** @var Filters[] */ private array $excludeFilters = []; @@ -146,6 +148,28 @@ public function withLimit(int $limit): self return $this; } + /** + * Set how deeply a filter's _or may nest: a top-level _or is at depth 1. + * 0 or null is unlimited. + */ + public function withFilterDepth(int|null $filterDepth): self + { + $this->filterDepth = $filterDepth; + + return $this; + } + + /** + * Set the most conditions a filter may have inside its _or branches. 0 + * or null is unlimited. + */ + public function withFilterConditions(int|null $filterConditions): self + { + $this->filterConditions = $filterConditions; + + return $this; + } + /** * Set global extraction strategy * @@ -253,6 +277,8 @@ public function build(): Config 'batchAssociations' => $this->batchAssociations, 'batchLimit' => $this->batchLimit, 'limit' => $this->limit, + 'filterDepth' => $this->filterDepth, + 'filterConditions' => $this->filterConditions, 'extractByValue' => $this->extractByValue, 'entityPrefix' => $this->entityPrefix, 'sortFields' => $this->sortFields, diff --git a/src/Filter/FilterFactory.php b/src/Filter/FilterFactory.php index 29121e7..20ea5c2 100644 --- a/src/Filter/FilterFactory.php +++ b/src/Filter/FilterFactory.php @@ -11,6 +11,7 @@ use ApiSkeletons\Doctrine\ORM\GraphQL\Metadata\AssociationMetadata; use ApiSkeletons\Doctrine\ORM\GraphQL\Type\Entity\Entity; use ApiSkeletons\Doctrine\ORM\GraphQL\Type\TypeContainer; +use Closure; use Doctrine\Common\Collections\ArrayCollection; use Doctrine\ORM\EntityManager; use GraphQL\Type\Definition\InputObjectType as GraphQLInputObjectType; @@ -52,6 +53,10 @@ public function __construct( * Return an InputObjectType of filters for the target entity. For a * collection, the owning entity, association name and the association's * metadata are given. + * + * Its _or is a list of branches, any of which a row matches. A branch is + * of a type of the same filters but the sorts, which mean nothing in a + * branch, and has an _or of its own. */ public function get( Entity $targetEntity, @@ -59,9 +64,10 @@ public function get( string|null $associationName = null, AssociationMetadata|null $associationMetadata = null, ): GraphQLInputObjectType { - $typeName = $owningEntity ? - 'Filter_' . $owningEntity->getTypeName() . '_' . ucwords((string) $associationName) - : 'Filter_' . $targetEntity->getTypeName(); + $baseName = $owningEntity + ? $owningEntity->getTypeName() . '_' . ucwords((string) $associationName) + : $targetEntity->getTypeName(); + $typeName = 'Filter_' . $baseName; if ($this->typeContainer->has($typeName)) { $filter = $this->typeContainer->get($typeName); @@ -93,25 +99,72 @@ public function get( } /** @psalm-suppress MixedArgumentTypeCoercion */ - $fields = $this->addFields($targetEntity, $allowedFilters); - /** @psalm-suppress MixedArgumentTypeCoercion */ - $fields = array_merge($fields, $this->addAssociations($targetEntity, $allowedFilters)); - /** @psalm-suppress MixedArgumentTypeCoercion */ - $fields = array_merge($fields, $this->addComputedFields($targetEntity, $allowedFilters)); + $branchFilters = array_values(array_filter( + $allowedFilters, + static fn (Filters $filter): bool => ! in_array($filter, [Filters::SORT, Filters::SORTPRIORITY], true), + )); + + // The branch type's _or is a list of the branch type itself + /** @psalm-suppress MixedArgumentTypeCoercion The allowed filters are Filters */ + $branch = $this->filterType( + 'FilterBranch_' . $baseName, + $this->filterFields($targetEntity, $branchFilters), + static fn (GraphQLInputObjectType $branch): GraphQLInputObjectType => $branch, + ); + + /** @psalm-suppress MixedArgumentTypeCoercion The allowed filters are Filters */ + $inputObject = $this->filterType( + $typeName, + $this->filterFields($targetEntity, $allowedFilters), + static fn (): GraphQLInputObjectType => $branch, + ); + + $this->typeContainer->set($typeName, $inputObject); + + return $inputObject; + } + + /** + * The fields of a filter type of the allowed filters, without its _or + * + * @param Filters[] $allowedFilters + * + * @return array + */ + private function filterFields(Entity $targetEntity, array $allowedFilters): array + { + return array_merge( + $this->addFields($targetEntity, $allowedFilters), + $this->addAssociations($targetEntity, $allowedFilters), + $this->addComputedFields($targetEntity, $allowedFilters), + ); + } + + /** + * A filter type of the fields and an _or, a list of the type the function + * gives. The function is given the filter type itself, as a branch type's + * _or is a list of itself. + * + * @param array $fields + * @param Closure(GraphQLInputObjectType): GraphQLInputObjectType $branch + */ + private function filterType(string $name, array $fields, Closure $branch): GraphQLInputObjectType + { + return (new ReflectionClass(GraphQLInputObjectType::class)) + ->newLazyGhost(static function (GraphQLInputObjectType $object) use ($name, $fields, $branch): void { + $fields[QueryBuilder::OR] = [ + 'name' => QueryBuilder::OR, + 'type' => static fn (): Type => Type::listOf(Type::nonNull($branch($object))), + 'description' => 'Filters any of which a row matches; the filters of each are all matched', + ]; - $inputObject = (new ReflectionClass(GraphQLInputObjectType::class)) - ->newLazyGhost(static function (GraphQLInputObjectType $object) use ($typeName, $fields): void { /** @psalm-suppress PossiblyInvalidArgument */ /** @psalm-suppress DirectConstructorCall */ $object->__construct([ // @phpstan-ignore argument.type - 'name' => $typeName, + 'name' => $name, 'fields' => static fn () => $fields, ]); }); - - $this->typeContainer->set($typeName, $inputObject); - - return $inputObject; } /** diff --git a/src/Filter/QueryBuilder.php b/src/Filter/QueryBuilder.php index e0f672e..a9a1fb0 100644 --- a/src/Filter/QueryBuilder.php +++ b/src/Filter/QueryBuilder.php @@ -21,6 +21,7 @@ use function array_flip; use function array_key_exists; use function assert; +use function count; use function in_array; use function is_array; use function is_int; @@ -101,6 +102,9 @@ final class QueryBuilder Filters::NOTIN, ]; + /** The filter of a filter's branches, any of which a row matches */ + public const string OR = '_or'; + /** The filters which sort */ private const array SORTS = [Filters::SORT, Filters::SORTPRIORITY]; @@ -133,6 +137,16 @@ final class QueryBuilder /** The number of uses of computed field expressions, whose aliases are numbered by it */ private int $expressionCount = 0; + /** + * @param int|null $filterDepth How deeply _or may nest, or null when it is unlimited + * @param int|null $filterConditions The most conditions inside _or branches, or null when it is unlimited + */ + public function __construct( + private readonly int|null $filterDepth = null, + private readonly int|null $filterConditions = null, + ) { + } + /** * Add where clauses to a QueryBuilder based on the FilterType of the entity * @@ -140,7 +154,7 @@ final class QueryBuilder * @param bool $sort Whether to apply the sorts. A count is * not sorted. * - * @psalm-suppress MixedAssignment, MixedArgument + * @throws FilterException When a filter cannot be applied, or _or is beyond the limits. */ public function apply( array $filterTypes, @@ -148,12 +162,56 @@ public function apply( Entity $entity, bool $sort = true, ): void { + $this->assertWithinLimits($filterTypes); + + foreach ($this->conditions($filterTypes, $queryBuilder, $entity, $sort) as $condition) { + $queryBuilder->andWhere($condition); + } + + $this->applySort($queryBuilder); + } + + /** + * The conditions of a filter, each of which a row must match: one for + * each field filter given a value, and one for its _or + * + * @param array> $filterTypes + * + * @return list + * + * @psalm-suppress MixedAssignment, MixedArgument + */ + private function conditions( + array $filterTypes, + DoctrineQueryBuilder $queryBuilder, + Entity $entity, + bool $sort, + ): array { + $conditions = []; + foreach ($filterTypes as $fieldName => $filters) { + if ($fieldName === self::OR) { + if ($filters !== null) { + assert(is_array($filters)); + $conditions[] = $this->orCondition($filters, $queryBuilder, $entity); + } + + continue; + } + + // A field given null is not applied, as a filter given null is not + if ($filters === null) { + continue; + } + // A computed field is filtered by its expression. Each field of the // type has a unique name, so a computed field is not a field's alias. $computedField = $entity->getEntityMetadata()->computedFields[$fieldName] ?? null; if ($computedField instanceof ComputedFieldMetadata && $computedField->expression !== null) { - $this->applyComputedField($fieldName, $computedField, $filters, $queryBuilder, $sort); + $conditions = [ + ...$conditions, + ...$this->computedFieldConditions($fieldName, $computedField, $filters, $queryBuilder, $sort), + ]; continue; } @@ -188,35 +246,150 @@ public function apply( ); } - $this->addFilter($filter, $queryBuilderField, $value, $queryBuilder); + $condition = $this->condition($filter, $queryBuilderField, $value, $queryBuilder); + if ($condition === null) { + continue; + } + + $conditions[] = $condition; } } - $this->applySort($queryBuilder); + return $conditions; + } + + /** + * The condition of an _or: a row matches one of its branches, each of + * which is a filter without sorts. A branch which has no conditions + * matches every row, and an _or of no branches matches none. Each branch + * binds its own parameters, so a branch is given a condition even when it + * matches every row. + * + * @param array $branches + */ + private function orCondition(array $branches, DoctrineQueryBuilder $queryBuilder, Entity $entity): string + { + if ($branches === []) { + return '1 = 0'; + } + + $expr = $queryBuilder->expr(); + $parts = []; + + /** @psalm-suppress MixedAssignment Each branch is an input object */ + foreach ($branches as $branch) { + assert(is_array($branch)); + + /** @psalm-suppress MixedArgumentTypeCoercion A branch is a filter */ + $conditions = $this->conditions($branch, $queryBuilder, $entity, false); + + $parts[] = match (count($conditions)) { + 0 => '1 = 1', + 1 => $conditions[0], + default => (string) $expr->andX(...$conditions), + }; + } + + return count($parts) === 1 ? $parts[0] : (string) $expr->orX(...$parts); + } + + /** + * _or may nest at most filterDepth deep, a top-level _or being at depth + * 1, and its branches may have at most filterConditions conditions in + * all. Only conditions inside _or are counted, so a filter without one is + * never limited. They are checked before any condition is built. + * + * @param array> $filterTypes + * + * @throws FilterException + */ + private function assertWithinLimits(array $filterTypes): void + { + $conditions = 0; + $this->countBranches($filterTypes[self::OR] ?? null, 1, $conditions); + + if ($this->filterConditions !== null && $conditions > $this->filterConditions) { + throw new FilterException( + 'A filter may have at most ' . $this->filterConditions . " conditions in '" . self::OR . "'.", + ); + } } /** - * Add the filters of a computed field, each comparing its expression. A - * sort orders by the expression, selected as a hidden result variable, as - * DQL does not order by a subquery. + * Count the conditions of _or branches, the filters given a value, + * checking the depth of each _or + * + * @throws FilterException + */ + private function countBranches(mixed $branches, int $depth, int &$conditions): void + { + if ($branches === null) { + return; + } + + if ($this->filterDepth !== null && $depth > $this->filterDepth) { + throw new FilterException( + "A filter may nest '" . self::OR . "' at most " . $this->filterDepth . ' deep.', + ); + } + + assert(is_array($branches)); + + /** @psalm-suppress MixedAssignment Each branch is an input object */ + foreach ($branches as $branch) { + assert(is_array($branch)); + + /** @psalm-suppress MixedAssignment Each field's filters are an input object */ + foreach ($branch as $fieldName => $filters) { + if ($fieldName === self::OR) { + $this->countBranches($filters, $depth + 1, $conditions); + + continue; + } + + if ($filters === null) { + continue; + } + + assert(is_array($filters)); + + /** @psalm-suppress MixedAssignment A filter's value may be of any type */ + foreach ($filters as $filter => $value) { + if ($filter === 'args' || $value === null) { + continue; + } + + $conditions++; + } + } + } + } + + /** + * The conditions of a computed field's filters, each comparing its + * expression. A sort orders by the expression, selected as a hidden + * result variable, as DQL does not order by a subquery. * * @param array $filters The filters, and the args of the field's arguments * + * @return list + * * @psalm-suppress MixedAssignment, MixedArgument */ - private function applyComputedField( + private function computedFieldConditions( string $fieldName, ComputedFieldMetadata $computedField, array $filters, DoctrineQueryBuilder $queryBuilder, bool $sort, - ): void { + ): array { $args = $filters['args'] ?? []; $args = is_array($args) ? $args : []; unset($filters['args']); - $fieldType = self::computedFieldValueType($computedField->type); - $sortAlias = null; + $fieldType = self::computedFieldValueType($computedField->type); + $sortAlias = null; + $conditions = []; foreach ($filters as $filter => $value) { $filter = Filters::from($filter); @@ -240,7 +413,7 @@ private function applyComputedField( $this->fieldNames[$sortAlias] = $fieldName; } - $this->addFilter($filter, $sortAlias, $value, $queryBuilder); + $this->condition($filter, $sortAlias, $value, $queryBuilder); continue; } @@ -258,13 +431,13 @@ private function applyComputedField( if ($filter === Filters::BETWEEN) { assert(is_array($value)); - $this->compare( + $conditions[] = $this->compare( Filters::GTE, $this->expression($computedField, $args, $queryBuilder, $fieldName), $value['from'], $queryBuilder, ); - $this->compare( + $conditions[] = $this->compare( Filters::LTE, $this->expression($computedField, $args, $queryBuilder, $fieldName), $value['to'], @@ -274,13 +447,18 @@ private function applyComputedField( continue; } - $this->addFilter( + $condition = $this->condition( $filter, $this->expression($computedField, $args, $queryBuilder, $fieldName), $value, $queryBuilder, ); + assert($condition !== null); + + $conditions[] = $condition; } + + return $conditions; } /** @@ -401,13 +579,30 @@ private function validateFilter( } /** - * Add a filter to the QueryBuilder + * The condition of a filter, binding its parameters, or null for a sort, + * which is recorded to order the query by * * @psalm-suppress MixedArgument The value is of the filter's GraphQL type */ - private function addFilter(Filters $filter, string $field, mixed $value, DoctrineQueryBuilder $queryBuilder): void - { - match ($filter) { + private function condition( + Filters $filter, + string $field, + mixed $value, + DoctrineQueryBuilder $queryBuilder, + ): string|null { + if ($filter === Filters::SORT) { + $this->sort($field, $value); + + return null; + } + + if ($filter === Filters::SORTPRIORITY) { + $this->sortPriority($field, $value); + + return null; + } + + return match ($filter) { Filters::EQ, Filters::NEQ, Filters::LT, @@ -421,76 +616,69 @@ private function addFilter(Filters $filter, string $field, mixed $value, Doctrin Filters::STARTSWITH => $this->startsWith($field, $value, $queryBuilder), Filters::ENDSWITH => $this->endsWith($field, $value, $queryBuilder), Filters::ISNULL => $this->isnull($field, $value, $queryBuilder), - Filters::SORT => $this->sort($field, $value, $queryBuilder), - Filters::SORTPRIORITY => $this->sortPriority($field, $value, $queryBuilder), }; } /** * Compare the field with the value */ - private function compare(Filters $filter, string $field, mixed $value, DoctrineQueryBuilder $queryBuilder): void + private function compare(Filters $filter, string $field, mixed $value, DoctrineQueryBuilder $queryBuilder): string { $parameter = $this->parameter($queryBuilder); $placeholder = ':' . $parameter; $expr = $queryBuilder->expr(); - $queryBuilder - ->andWhere(match ($filter) { - Filters::NEQ => $expr->neq($field, $placeholder), - Filters::LT => $expr->lt($field, $placeholder), - Filters::LTE => $expr->lte($field, $placeholder), - Filters::GT => $expr->gt($field, $placeholder), - Filters::GTE => $expr->gte($field, $placeholder), - Filters::IN => $expr->in($field, $placeholder), - Filters::NOTIN => $expr->notIn($field, $placeholder), - default => $expr->eq($field, $placeholder), - }) - ->setParameter($parameter, $value); + $queryBuilder->setParameter($parameter, $value); + + return (string) match ($filter) { + Filters::NEQ => $expr->neq($field, $placeholder), + Filters::LT => $expr->lt($field, $placeholder), + Filters::LTE => $expr->lte($field, $placeholder), + Filters::GT => $expr->gt($field, $placeholder), + Filters::GTE => $expr->gte($field, $placeholder), + Filters::IN => $expr->in($field, $placeholder), + Filters::NOTIN => $expr->notIn($field, $placeholder), + default => $expr->eq($field, $placeholder), + }; } /** @param array $value */ - private function between(string $field, array $value, DoctrineQueryBuilder $queryBuilder): void + private function between(string $field, array $value, DoctrineQueryBuilder $queryBuilder): string { $from = $this->parameter($queryBuilder); $to = $this->parameter($queryBuilder); $queryBuilder - ->andWhere( - $queryBuilder->expr()->between( - $field, - ':' . $from, - ':' . $to, - ), - ) ->setParameter($from, $value['from']) ->setParameter($to, $value['to']); + + return $queryBuilder->expr()->between($field, ':' . $from, ':' . $to); } - private function contains(string $field, string $value, DoctrineQueryBuilder $queryBuilder): void + private function contains(string $field, string $value, DoctrineQueryBuilder $queryBuilder): string { - $this->like($field, '%' . $this->escapeLike($value) . '%', $queryBuilder); + return $this->like($field, '%' . $this->escapeLike($value) . '%', $queryBuilder); } - private function startsWith(string $field, string $value, DoctrineQueryBuilder $queryBuilder): void + private function startsWith(string $field, string $value, DoctrineQueryBuilder $queryBuilder): string { - $this->like($field, $this->escapeLike($value) . '%', $queryBuilder); + return $this->like($field, $this->escapeLike($value) . '%', $queryBuilder); } - private function endsWith(string $field, string $value, DoctrineQueryBuilder $queryBuilder): void + private function endsWith(string $field, string $value, DoctrineQueryBuilder $queryBuilder): string { - $this->like($field, '%' . $this->escapeLike($value), $queryBuilder); + return $this->like($field, '%' . $this->escapeLike($value), $queryBuilder); } /** * Match a LIKE pattern. Wildcards in the value are escaped, so it matches * only itself. */ - private function like(string $field, string $pattern, DoctrineQueryBuilder $queryBuilder): void + private function like(string $field, string $pattern, DoctrineQueryBuilder $queryBuilder): string { $parameter = $this->parameter($queryBuilder); - $queryBuilder - ->andWhere($field . ' LIKE :' . $parameter . " ESCAPE '" . self::LIKE_ESCAPE . "'") - ->setParameter($parameter, $pattern); + $queryBuilder->setParameter($parameter, $pattern); + + return $field . ' LIKE :' . $parameter . " ESCAPE '" . self::LIKE_ESCAPE . "'"; } /** @@ -601,17 +789,11 @@ private function escapeLike(string $value): string ]); } - private function isnull(string $field, bool $value, DoctrineQueryBuilder $queryBuilder): void + private function isnull(string $field, bool $value, DoctrineQueryBuilder $queryBuilder): string { - if ($value === true) { - $queryBuilder->andWhere( - $queryBuilder->expr()->isNull($field), - ); - } else { - $queryBuilder->andWhere( - $queryBuilder->expr()->isNotNull($field), - ); - } + return $value + ? $queryBuilder->expr()->isNull($field) + : $queryBuilder->expr()->isNotNull($field); } /** @@ -730,7 +912,7 @@ private static function isDbalType(string $type): bool return isset(self::$dbalTypes[$type]); } - private function sort(string $field, string $direction, DoctrineQueryBuilder $queryBuilder): void + private function sort(string $field, string $direction): void { if (! isset($this->sortFields[$field])) { $this->sortFields[$field] = []; @@ -742,7 +924,7 @@ private function sort(string $field, string $direction, DoctrineQueryBuilder $qu $this->sortFields[$field]['direction'] = $direction; } - private function sortPriority(string $field, int $priority, DoctrineQueryBuilder $queryBuilder): void + private function sortPriority(string $field, int $priority): void { if (! isset($this->sortFields[$field])) { $this->sortFields[$field] = []; diff --git a/src/Resolve/ResolveCollectionFactory.php b/src/Resolve/ResolveCollectionFactory.php index 99588bc..c89b745 100644 --- a/src/Resolve/ResolveCollectionFactory.php +++ b/src/Resolve/ResolveCollectionFactory.php @@ -734,7 +734,8 @@ private function applyFilters(QueryBuilder $queryBuilder, array $args, Entity $e } /** @psalm-suppress MixedArgument */ - (new QueryBuilderFilter())->apply($args['filter'], $queryBuilder, $entity, $sort); + (new QueryBuilderFilter($this->config->getFilterDepth(), $this->config->getFilterConditions())) + ->apply($args['filter'], $queryBuilder, $entity, $sort); } /** diff --git a/src/Resolve/ResolveEntityFactory.php b/src/Resolve/ResolveEntityFactory.php index c2bf13b..c5ee8e9 100644 --- a/src/Resolve/ResolveEntityFactory.php +++ b/src/Resolve/ResolveEntityFactory.php @@ -40,7 +40,10 @@ public function get(Entity $entity, string|null $eventName): Closure { return function (mixed $objectValue, array $args, mixed $context, ResolveInfo $info) use ($entity, $eventName) { $entityClass = $entity->getEntityClass(); - $queryBuilderFilter = new QueryBuilderFilter(); + $queryBuilderFilter = new QueryBuilderFilter( + $this->config->getFilterDepth(), + $this->config->getFilterConditions(), + ); $queryBuilder = $this->entityManager->createQueryBuilder(); $queryBuilder->select('entity') diff --git a/src/Type/Entity/Entity.php b/src/Type/Entity/Entity.php index 5bcddae..48ae344 100644 --- a/src/Type/Entity/Entity.php +++ b/src/Type/Entity/Entity.php @@ -9,6 +9,7 @@ use ApiSkeletons\Doctrine\ORM\GraphQL\Exception\Hydrator as HydratorException; use ApiSkeletons\Doctrine\ORM\GraphQL\Exception\Metadata as MetadataException; use ApiSkeletons\Doctrine\ORM\GraphQL\Filter\FilterFactory; +use ApiSkeletons\Doctrine\ORM\GraphQL\Filter\QueryBuilder as QueryBuilderFilter; use ApiSkeletons\Doctrine\ORM\GraphQL\Hydrator\HydratorContainer; use ApiSkeletons\Doctrine\ORM\GraphQL\Metadata\ComputedFieldMetadata; use ApiSkeletons\Doctrine\ORM\GraphQL\Metadata\EntityMetadata; @@ -130,7 +131,8 @@ public function getEntityClass(): string * Every field of the type, whether a field, an association or a computed * field, named by its alias if it has one, must have a unique name. * Otherwise one would silently replace another in the type and in the - * hydrator's extraction. + * hydrator's extraction. No field may be named _or, the name of the + * filters' branches. * * @return array * @@ -159,6 +161,15 @@ public function getExtractionMap(): array $alias = $fieldMetadata instanceof ComputedFieldMetadata ? null : $fieldMetadata->alias; $name = $alias ?? $fieldName; + // The filters of a type have an _or of their own + if ($name === QueryBuilderFilter::OR) { + throw new MetadataException( + 'The ' . $kind . ' ' . $fieldName . ($alias !== null ? ' aliased as "' . $alias . '"' : '') + . ' of entity ' . $this->getEntityClass() . ' is named "' . $name . '", which is the name of ' + . 'the filter of branches any of which a row matches. Give it another name.', + ); + } + if (isset($names[$name])) { throw new MetadataException( 'Duplicate field name "' . $name . '" in entity ' . $this->getEntityClass() . ': ' diff --git a/test/Feature/ComputedField/ComputedExpressionData.php b/test/Feature/ComputedField/ComputedExpressionData.php new file mode 100644 index 0000000..ee35d58 --- /dev/null +++ b/test/Feature/ComputedField/ComputedExpressionData.php @@ -0,0 +1,54 @@ +persist($labels[$name]); + } + + // Each artist's recordings, by the date of their release + $artists = [ + ['Phish', 'Elektra', ['2002-05-14', '2002-11-05', '2003-10-14', '2004-06-15']], + ['Ween', 'Elektra', ['2003-07-15', '2007-10-23']], + ['moe.', 'Elektra', []], + ["Gov't Mule", 'Relix', ['2001-06-12', '2003-09-23', '2004-08-17']], + ['Grateful Dead', 'Relix', ['1977-05-08']], + ]; + + foreach ($artists as [$name, $label, $releases]) { + $artist = (new ComputedExpressionArtist($name))->setLabel($labels[$label]); + $entityManager->persist($artist); + + foreach ($releases as $i => $released) { + $entityManager->persist(new ComputedExpressionRecording( + $name . ' ' . $i, + new DateTimeImmutable($released), + $artist, + )); + } + } + + $entityManager->flush(); + $entityManager->clear(); + } +} diff --git a/test/Feature/ComputedField/ComputedFieldExpressionTest.php b/test/Feature/ComputedField/ComputedFieldExpressionTest.php index 02e85fb..094c0f1 100644 --- a/test/Feature/ComputedField/ComputedFieldExpressionTest.php +++ b/test/Feature/ComputedField/ComputedFieldExpressionTest.php @@ -12,9 +12,7 @@ use ApiSkeletons\Doctrine\ORM\GraphQL\Metadata; use ApiSkeletonsTest\Doctrine\ORM\GraphQL\Entity\ComputedExpressionArtist; use ApiSkeletonsTest\Doctrine\ORM\GraphQL\Entity\ComputedExpressionLabel; -use ApiSkeletonsTest\Doctrine\ORM\GraphQL\Entity\ComputedExpressionRecording; use ApiSkeletonsTest\Doctrine\ORM\GraphQL\TestCase; -use DateTimeImmutable; use GraphQL\Error\DebugFlag; use GraphQL\GraphQL; use GraphQL\Type\Definition\InputObjectType; @@ -36,42 +34,13 @@ */ class ComputedFieldExpressionTest extends TestCase { + use ComputedExpressionData; + public function setUp(): void { parent::setUp(); - $entityManager = $this->getEntityManager(); - - $labels = []; - foreach (['Elektra', 'Relix'] as $name) { - $labels[$name] = new ComputedExpressionLabel($name); - $entityManager->persist($labels[$name]); - } - - // Each artist's recordings, by the date of their release - $artists = [ - ['Phish', 'Elektra', ['2002-05-14', '2002-11-05', '2003-10-14', '2004-06-15']], - ['Ween', 'Elektra', ['2003-07-15', '2007-10-23']], - ['moe.', 'Elektra', []], - ["Gov't Mule", 'Relix', ['2001-06-12', '2003-09-23', '2004-08-17']], - ['Grateful Dead', 'Relix', ['1977-05-08']], - ]; - - foreach ($artists as [$name, $label, $releases]) { - $artist = (new ComputedExpressionArtist($name))->setLabel($labels[$label]); - $entityManager->persist($artist); - - foreach ($releases as $i => $released) { - $entityManager->persist(new ComputedExpressionRecording( - $name . ' ' . $i, - new DateTimeImmutable($released), - $artist, - )); - } - } - - $entityManager->flush(); - $entityManager->clear(); + $this->populateComputedExpressionData($this->getEntityManager()); } /** @param array $config */ diff --git a/test/Feature/Filter/InverseOneToOneFilterTest.php b/test/Feature/Filter/InverseOneToOneFilterTest.php index 3395f0e..b9ca2a8 100644 --- a/test/Feature/Filter/InverseOneToOneFilterTest.php +++ b/test/Feature/Filter/InverseOneToOneFilterTest.php @@ -38,10 +38,10 @@ public function testInverseSideHasNoFilter(): void $driver = $this->driver(); $this->assertSame( - ['id', 'name', 'nickname', 'requiredArtist', 'optionalArtist', 'defaultArtist'], + ['id', 'name', 'nickname', 'requiredArtist', 'optionalArtist', 'defaultArtist', '_or'], array_keys($driver->filter(TestNonNullTypes::class)->getFields()), ); - $this->assertSame(['id', 'owner'], array_keys($driver->filter(TestNonNullTypesDetail::class)->getFields())); + $this->assertSame(['id', 'owner', '_or'], array_keys($driver->filter(TestNonNullTypesDetail::class)->getFields())); } public function testOwningSideIsFiltered(): void diff --git a/test/Feature/Filter/OrFilterTest.php b/test/Feature/Filter/OrFilterTest.php new file mode 100644 index 0000000..5006a4f --- /dev/null +++ b/test/Feature/Filter/OrFilterTest.php @@ -0,0 +1,426 @@ +populateComputedExpressionData($this->getEntityManager()); + } + + /** @param array $config */ + private function getDriver(array $config = []): Driver + { + return new Driver( + $this->getEntityManager(), + new Config(['group' => 'ComputedExpression'] + $config), + ); + } + + /** @return array */ + private function execute(Driver $driver, string $query): array + { + $this->getEntityManager()->clear(); + + $schema = new Schema([ + 'query' => new ObjectType([ + 'name' => 'query', + 'fields' => [ + 'artists' => $driver->completeConnection(ComputedExpressionArtist::class), + 'labels' => $driver->completeConnection(ComputedExpressionLabel::class), + ], + ]), + ]); + + return GraphQL::executeQuery($schema, $query)->toArray(DebugFlag::INCLUDE_DEBUG_MESSAGE); + } + + /** + * The names of the artists a filter selects, in order, and their count + * + * @param array $config + * + * @return array{int, list} + */ + private function artists(string $filter, array $config = []): array + { + $result = $this->execute( + $this->getDriver($config), + '{ artists (filter: ' . $filter . ') { totalCount edges { node { name } } } }', + ); + + $this->assertArrayNotHasKey('errors', $result, $filter . json_encode($result['errors'] ?? null)); + + return [ + $result['data']['artists']['totalCount'], + array_column(array_column($result['data']['artists']['edges'], 'node'), 'name'), + ]; + } + + /** @return array}> */ + public static function filterProvider(): array + { + $all = ['Phish', 'Ween', 'moe.', "Gov't Mule", 'Grateful Dead']; + + return [ + 'two branches' => ['{ _or: [{ name: { eq: "Phish" } }, { name: { eq: "Ween" } }] }', ['Phish', 'Ween']], + 'one branch' => ['{ _or: [{ name: { eq: "Phish" } }] }', ['Phish']], + 'with a field' => [ + '{ id: { gt: 1 }, _or: [{ name: { startswith: "G" } }, { recordingCount: { gt: 3 } }] }', + ["Gov't Mule", 'Grateful Dead'], + ], + 'a branch of two fields' => [ + '{ _or: [{ name: { startswith: "G" }, recordingCount: { gt: 2 } }, { name: { eq: "moe." } }] }', + ['moe.', "Gov't Mule"], + ], + 'a branch of two filters on a field' => [ + '{ _or: [{ id: { gt: 1, lt: 3 } }, { name: { eq: "Grateful Dead" } }] }', + ['Ween', 'Grateful Dead'], + ], + 'nested' => [ + '{ _or: [{ _or: [{ name: { eq: "Phish" } }, { name: { eq: "Ween" } }] }, ' + . '{ recordingCount: { eq: 1 } }] }', + ['Phish', 'Ween', 'Grateful Dead'], + ], + 'every filter' => [ + '{ _or: [{ id: { in: [1, 2] } }, { id: { notin: [1, 2, 3, 5] } }, { id: { between: { from: 5, to: 9 } } }, ' + . '{ name: { endswith: "Phish", isnull: true } }, { name: { contains: "e.", neq: "Ween" } }] }', + ['Phish', 'Ween', 'moe.', "Gov't Mule", 'Grateful Dead'], + ], + 'a computed field' => [ + '{ _or: [{ recordingCount: { between: { from: 2, to: 2 } } }, { upperName: { startswith: "MOE" } }] }', + ['Ween', 'moe.'], + ], + 'a computed field with arguments' => [ + '{ _or: [{ recordingCountSince: { args: { year: 2004 }, gt: 0 } }, { name: { eq: "moe." } }] }', + ['Phish', 'Ween', 'moe.', "Gov't Mule"], + ], + // A branch with no condition matches every row + 'an empty branch' => ['{ _or: [{ name: { eq: "Phish" } }, {}] }', $all], + 'a branch whose filter is given null' => ['{ _or: [{ name: { eq: "Phish" } }, { name: { contains: null } }] }', $all], + 'a branch whose field is given null' => ['{ _or: [{ name: { eq: "Phish" } }, { name: null }] }', $all], + 'a branch whose _or is given null' => ['{ _or: [{ name: { eq: "Phish" } }, { _or: null }] }', $all], + // An _or of no branches matches no row + 'no branches' => ['{ _or: [] }', []], + 'null' => ['{ _or: null }', $all], + 'a field given null' => ['{ name: null }', $all], + ]; + } + + /** @param list $names */ + #[DataProvider('filterProvider')] + public function testFilter(string $filter, array $names): void + { + $this->assertSame([count($names), $names], $this->artists($filter)); + } + + public function testSort(): void + { + $this->assertSame( + [3, ['Phish', "Gov't Mule", 'Ween']], + $this->artists('{ recordingCount: { sort: DESC }, _or: [{ id: { lt: 3 } }, { id: { eq: 4 } }] }'), + ); + } + + /** + * A branch is of a type of the same filters but the sorts, with an _or of + * its own + */ + public function testFilterType(): void + { + $filter = $this->getDriver()->filter(ComputedExpressionArtist::class); + $branch = self::branchType($filter); + + $this->assertSame('FilterBranch_' . self::typeName(), $branch->name()); + $this->assertSame($branch, self::branchType($branch)); + $this->assertSame(array_keys($filter->getFields()), array_keys($branch->getFields())); + + foreach (['name', 'recordingCount', 'recordingCountSince'] as $fieldName) { + $fields = $filter->getField($fieldName)->getType(); + assert($fields instanceof InputObjectType); + $this->assertArrayHasKey('sort', $fields->getFields()); + + $branchFields = $branch->getField($fieldName)->getType(); + assert($branchFields instanceof InputObjectType); + $this->assertArrayNotHasKey('sort', $branchFields->getFields()); + $this->assertArrayNotHasKey('sortPriority', $branchFields->getFields()); + } + + // A computed field's arguments are the same in a branch + $since = $branch->getField('recordingCountSince')->getType(); + assert($since instanceof InputObjectType); + $args = $since->getField('args')->getType(); + assert($args instanceof NonNull); + $this->assertSame('FilterArgs_' . self::typeName() . '_recordingCountSince', $args->getWrappedType()->name()); + } + + public function testSortInBranchIsInvalid(): void + { + $result = $this->execute( + $this->getDriver(), + '{ artists (filter: { _or: [{ name: { sort: ASC } }] }) { edges { node { name } } } }', + ); + + $this->assertStringContainsString('"sort"', $result['errors'][0]['message']); + } + + /** The type of the branches of a filter's _or */ + private static function branchType(InputObjectType $filter): InputObjectType + { + $or = $filter->getField('_or')->getType(); + assert($or instanceof ListOfType); + $branch = $or->getWrappedType(); + assert($branch instanceof NonNull); + $type = $branch->getWrappedType(); + assert($type instanceof InputObjectType); + + return $type; + } + + private static function typeName(): string + { + return 'ApiSkeletonsTest_Doctrine_ORM_GraphQL_Entity_ComputedExpressionArtist_ComputedExpression'; + } + + /** @return array}> */ + public static function collectionProvider(): array + { + return [ + 'batched' => [[]], + 'over the batch limit' => [['batchLimit' => 1]], + 'not batched' => [['batchAssociations' => false]], + ]; + } + + /** + * A collection's filter has an _or, however the collection is fetched + * + * @param array $config + */ + #[DataProvider('collectionProvider')] + public function testCollection(array $config): void + { + $result = $this->execute( + $this->getDriver($config), + '{ labels { edges { node { name artists (filter: { _or: [{ name: { startswith: "G" } }, ' + . '{ recordingCountSince: { args: { year: 2004 }, gt: 0 } }] }) { totalCount edges { node { name } } } } } } }', + ); + + $this->assertArrayNotHasKey('errors', $result, json_encode($result['errors'] ?? null)); + + $labels = []; + foreach ($result['data']['labels']['edges'] as $label) { + $labels[$label['node']['name']] = [ + $label['node']['artists']['totalCount'], + array_column(array_column($label['node']['artists']['edges'], 'node'), 'name'), + ]; + } + + $this->assertSame( + [ + 'Elektra' => [2, ['Phish', 'Ween']], + 'Relix' => [2, ["Gov't Mule", 'Grateful Dead']], + ], + $labels, + ); + } + + /** A filter whose _or nests to a depth */ + private static function nested(int $depth): string + { + return str_repeat('{ _or: [', $depth) . '{ name: { eq: "Phish" } }' . str_repeat('] }', $depth); + } + + /** A filter whose _or has a number of conditions */ + private static function conditions(int $count): string + { + return '{ _or: [' . implode(', ', array_fill(0, $count, '{ name: { eq: "Phish" } }')) . '] }'; + } + + /** @return array, string|null}> */ + public static function limitProvider(): array + { + $depth = "A filter may nest '_or' at most 3 deep."; + $conditions = "A filter may have at most 100 conditions in '_or'."; + + return [ + 'the default depth' => [self::nested(3), [], null], + 'beyond the default depth' => [self::nested(4), [], $depth], + 'beyond a depth' => [self::nested(2), ['filterDepth' => 1], "A filter may nest '_or' at most 1 deep."], + 'an unlimited depth' => [self::nested(10), ['filterDepth' => null], null], + 'a depth of 0' => [self::nested(10), ['filterDepth' => 0], null], + 'the default conditions' => [self::conditions(100), [], null], + 'beyond the default conditions' => [self::conditions(101), [], $conditions], + 'beyond conditions' => [ + '{ _or: [{ name: { eq: "Phish", neq: "Ween" } }, { id: { eq: 2 } }] }', + ['filterConditions' => 2], + "A filter may have at most 2 conditions in '_or'.", + ], + 'conditions in nested branches' => [ + '{ _or: [{ _or: [{ id: { eq: 1 } }, { id: { eq: 2 } }] }, { id: { eq: 3 } }] }', + ['filterConditions' => 2], + "A filter may have at most 2 conditions in '_or'.", + ], + 'unlimited conditions' => [self::conditions(101), ['filterConditions' => null], null], + 'conditions of 0' => [self::conditions(101), ['filterConditions' => 0], null], + // Neither the args, nor a filter given null, nor a field given null, is a condition + 'not conditions' => [ + '{ _or: [{ recordingCountSince: { args: { year: 2004 }, gt: 0 }, name: { contains: null }, id: null }] }', + ['filterConditions' => 1], + null, + ], + // Conditions outside _or are not counted + 'outside _or' => [ + '{ id: { gt: 0, lt: 9 }, name: { neq: "x" }, _or: [{ id: { eq: 1 } }] }', + ['filterConditions' => 1], + null, + ], + ]; + } + + /** @param array $config */ + #[DataProvider('limitProvider')] + public function testLimits(string $filter, array $config, string|null $message): void + { + $result = $this->execute( + $this->getDriver($config), + '{ artists (filter: ' . $filter . ') { edges { node { name } } } }', + ); + + if ($message === null) { + $this->assertArrayNotHasKey('errors', $result, json_encode($result['errors'] ?? null)); + + return; + } + + // The error is the client's + $this->assertSame($message, $result['errors'][0]['message']); + } + + /** + * A collection's filter is limited too + */ + public function testCollectionLimits(): void + { + $result = $this->execute( + $this->getDriver(), + '{ labels { edges { node { artists (filter: ' . self::nested(4) . ') { edges { node { name } } } } } } }', + ); + + $this->assertSame("A filter may nest '_or' at most 3 deep.", $result['errors'][0]['message']); + } + + public function testConfig(): void + { + $config = new Config(); + $this->assertSame(3, $config->getFilterDepth()); + $this->assertSame(100, $config->getFilterConditions()); + + $config = new Config(['filterDepth' => 0, 'filterConditions' => 0]); + $this->assertNull($config->getFilterDepth()); + $this->assertNull($config->getFilterConditions()); + + $config = ConfigBuilder::create()->withFilterDepth(5)->withFilterConditions(null)->build(); + $this->assertSame(5, $config->getFilterDepth()); + $this->assertNull($config->getFilterConditions()); + } + + /** @return array */ + public static function negativeLimitProvider(): array + { + return [ + 'filterDepth' => ['filterDepth'], + 'filterConditions' => ['filterConditions'], + ]; + } + + #[DataProvider('negativeLimitProvider')] + public function testNegativeLimit(string $option): void + { + $this->expectException(ConfigurationException::class); + $this->expectExceptionMessage('Invalid configuration value for ' . $option . ': it must be at least 0, got -1.'); + + new Config([$option => -1]); + } + + /** @return array */ + public static function reservedNameProvider(): array + { + return [ + 'a field' => ['fields', 'The field name aliased as "_or"'], + 'a computed field' => ['computedFields', 'The computed field _or'], + ]; + } + + /** + * No field may be named _or, the name of a filter's branches + */ + #[DataProvider('reservedNameProvider')] + public function testReservedName(string $kind, string $message): void + { + $driver = $this->getDriver(); + $driver->get(EventDispatcher::class)->subscribeTo( + 'metadata.build', + static function (MetadataEvent $event) use ($kind): void { + $metadata = $event->getMetadata(); + $artist = $metadata[ComputedExpressionArtist::class]; + assert(is_array($artist) && is_array($artist['fields']) && is_array($artist['computedFields'])); + + if ($kind === 'fields') { + assert(is_array($artist['fields']['name'])); + $artist['fields']['name']['alias'] = '_or'; + } else { + $artist['computedFields']['_or'] = $artist['computedFields']['displayName']; + unset($artist['computedFields']['displayName']); + } + + $metadata[ComputedExpressionArtist::class] = $artist; + }, + ); + + $this->expectException(MetadataException::class); + $this->expectExceptionMessage($message . ' of entity ' . ComputedExpressionArtist::class . ' is named "_or"'); + + $driver->filter(ComputedExpressionArtist::class); + } +} diff --git a/test/Feature/Filter/ToOneAssociationFiltersTest.php b/test/Feature/Filter/ToOneAssociationFiltersTest.php index 7f9e4a6..7de4b19 100644 --- a/test/Feature/Filter/ToOneAssociationFiltersTest.php +++ b/test/Feature/Filter/ToOneAssociationFiltersTest.php @@ -31,7 +31,7 @@ public function testAssociationLimitsItsFilters(): void $filter = $this->driver()->filter(TestToOneFilters::class); // An association left with no filter has no filter field - $this->assertSame(['id', 'excluded', 'included', 'every'], array_keys($filter->getFields())); + $this->assertSame(['id', 'excluded', 'included', 'every', '_or'], array_keys($filter->getFields())); $filters = static function (string $fieldName) use ($filter): array { $type = $filter->getField($fieldName)->getType(); diff --git a/test/Feature/Metadata/EmbeddedTest.php b/test/Feature/Metadata/EmbeddedTest.php index ffcc0b0..7bd163d 100644 --- a/test/Feature/Metadata/EmbeddedTest.php +++ b/test/Feature/Metadata/EmbeddedTest.php @@ -86,7 +86,7 @@ public function testEmbeddedFieldsAreNotExposed(): void $this->assertInstanceOf(ObjectType::class, $objectType); $this->assertSame(['id', 'name', 'city'], array_keys($objectType->getFields())); - $this->assertSame(['id', 'name'], array_keys($driver->filter(TestEntityWithEmbedded::class)->getFields())); + $this->assertSame(['id', 'name', '_or'], array_keys($driver->filter(TestEntityWithEmbedded::class)->getFields())); $this->assertSame(['name'], array_keys($driver->input(TestEntityWithEmbedded::class)->getFields())); }