Skip to content

Commit f533810

Browse files
authored
fix(graphql): accept FilterInterface instance in QueryParameter (#7972)
| Q | A | ------------- | --- | Branch? | 4.3 | Tickets | refs #7966 | License | MIT | Doc PR | ∅ FieldsBuilder called filterLocator->has() with whatever QueryParameter::$filter returned, crashing on object-form filters (new SortFilter()) when the parameter key contained '['. A resolveFilter() helper now handles both string service ids and FilterInterface instances, applied at every locator site.
1 parent bf3fded commit f533810

3 files changed

Lines changed: 126 additions & 16 deletions

File tree

src/GraphQl/Type/FieldsBuilder.php

Lines changed: 32 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -512,20 +512,16 @@ private function getParameterArgs(Operation $operation, array $args = []): array
512512
$name = key($leafs);
513513

514514
$filterLeafs = [];
515-
if (($filterId = $parameter->getFilter()) && $this->filterLocator->has($filterId)) {
516-
$filter = $this->filterLocator->get($filterId);
517-
518-
if ($filter instanceof FilterInterface) {
519-
$property = $parameter->getProperty() ?? $name;
520-
$property = str_replace('.', $this->nestingSeparator, $property);
521-
$description = $filter->getDescription($operation->getClass());
522-
523-
foreach ($description as $descKey => $descValue) {
524-
$descKey = str_replace('.', $this->nestingSeparator, $descKey);
525-
parse_str($descKey, $descValues);
526-
if (isset($descValues[$property]) && \is_array($descValues[$property])) {
527-
$filterLeafs = array_merge($filterLeafs, $descValues[$property]);
528-
}
515+
if ($filter = $this->resolveFilter($parameter->getFilter())) {
516+
$property = $parameter->getProperty() ?? $name;
517+
$property = str_replace('.', $this->nestingSeparator, $property);
518+
$description = $filter->getDescription($operation->getClass());
519+
520+
foreach ($description as $descKey => $descValue) {
521+
$descKey = str_replace('.', $this->nestingSeparator, $descKey);
522+
parse_str($descKey, $descValues);
523+
if (isset($descValues[$property]) && \is_array($descValues[$property])) {
524+
$filterLeafs = array_merge($filterLeafs, $descValues[$property]);
529525
}
530526
}
531527
}
@@ -612,12 +608,12 @@ private function getFilterArgs(array $args, ?string $resourceClass, string $root
612608
}
613609

614610
foreach ($resourceOperation->getFilters() ?? [] as $filterId) {
615-
if (!$this->filterLocator->has($filterId)) {
611+
if (!($filter = $this->resolveFilter($filterId))) {
616612
continue;
617613
}
618614

619615
$entityClass = $this->getStateOptionsClass($resourceOperation, $resourceOperation->getClass());
620-
foreach ($this->filterLocator->get($filterId)->getDescription($entityClass) as $key => $description) {
616+
foreach ($filter->getDescription($entityClass) as $key => $description) {
621617
$filterType = \in_array($description['type'], TypeIdentifier::values(), true) ? Type::builtin($description['type']) : Type::object($description['type']);
622618
if (!($description['required'] ?? false)) {
623619
$filterType = Type::nullable($filterType);
@@ -751,4 +747,24 @@ private function normalizePropertyName(string $property, string $resourceClass):
751747

752748
return $this->nameConverter->normalize($property, $resourceClass);
753749
}
750+
751+
/**
752+
* Resolves a filter reference to a {@see FilterInterface} instance, supporting
753+
* both a string service id (legacy/locator path) and an object form
754+
* (`new QueryParameter(filter: new SortFilter())`).
755+
*/
756+
private function resolveFilter(mixed $filter): ?FilterInterface
757+
{
758+
if ($filter instanceof FilterInterface) {
759+
return $filter;
760+
}
761+
762+
if (\is_string($filter) && $this->filterLocator->has($filter)) {
763+
$resolved = $this->filterLocator->get($filter);
764+
765+
return $resolved instanceof FilterInterface ? $resolved : null;
766+
}
767+
768+
return null;
769+
}
754770
}
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the API Platform project.
5+
*
6+
* (c) Kévin Dunglas <dunglas@gmail.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
declare(strict_types=1);
13+
14+
namespace ApiPlatform\Tests\Fixtures\TestBundle\ApiResource\Issue7966;
15+
16+
use ApiPlatform\Doctrine\Orm\Filter\SortFilter;
17+
use ApiPlatform\Metadata\ApiResource;
18+
use ApiPlatform\Metadata\GraphQl\QueryCollection;
19+
use ApiPlatform\Metadata\QueryParameter;
20+
21+
#[ApiResource(
22+
operations: [],
23+
graphQlOperations: [
24+
new QueryCollection(
25+
provider: [self::class, 'provide'],
26+
paginationEnabled: false,
27+
parameters: [
28+
'order[:property]' => new QueryParameter(filter: new SortFilter()),
29+
],
30+
),
31+
],
32+
)]
33+
final class SortFilterParameterDummy
34+
{
35+
public ?string $id = null;
36+
public ?string $name = null;
37+
38+
public static function provide(): array
39+
{
40+
return [];
41+
}
42+
}
Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the API Platform project.
5+
*
6+
* (c) Kévin Dunglas <dunglas@gmail.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
declare(strict_types=1);
13+
14+
namespace ApiPlatform\Tests\Functional\GraphQl;
15+
16+
use ApiPlatform\Symfony\Bundle\Test\ApiTestCase;
17+
use ApiPlatform\Tests\Fixtures\TestBundle\ApiResource\Issue7966\SortFilterParameterDummy;
18+
use ApiPlatform\Tests\SetupClassResourcesTrait;
19+
20+
/**
21+
* Object-form filter (FilterInterface instance) combined with a bracketed parameter
22+
* key crashed the GraphQL schema build because `filterLocator->has()` was called
23+
* with the instance rather than a string service id.
24+
*
25+
* @see https://github.com/api-platform/core/issues/7966
26+
*/
27+
final class Issue7966Test extends ApiTestCase
28+
{
29+
use SetupClassResourcesTrait;
30+
31+
protected static ?bool $alwaysBootKernel = false;
32+
33+
/**
34+
* @return class-string[]
35+
*/
36+
public static function getResources(): array
37+
{
38+
return [SortFilterParameterDummy::class];
39+
}
40+
41+
public function testSchemaBuildsWithObjectFormFilterAndBracketedKey(): void
42+
{
43+
$response = self::createClient()->request('POST', '/graphql', ['json' => [
44+
'query' => '{ __type(name: "SortFilterParameterDummy") { name } }',
45+
]]);
46+
47+
$this->assertResponseIsSuccessful();
48+
$json = $response->toArray(false);
49+
$this->assertArrayNotHasKey('errors', $json, json_encode($json['errors'] ?? null));
50+
$this->assertSame('SortFilterParameterDummy', $json['data']['__type']['name']);
51+
}
52+
}

0 commit comments

Comments
 (0)