api-platform / core

The server component of API Platform: hypermedia and GraphQL APIs in minutes
https://api-platform.com
MIT License
2.45k stars 874 forks source link

GetCollection ignores ApiFilter required properties #6734

Open JercSi opened 2 weeks ago

JercSi commented 2 weeks ago

API Platform version(s) affected: 4.0.3, 4.0.4 (Symfony)

Description
API Filters required parameters seems to be lost with the upgrade from v3.2.26 to 4.0.4 (Symfony). In previous version http reponse code was 400 with a description that query parameter is required. In 4.0.4 response is 200 with the data. OpenAPI specification (api/docs) shows the query parameter field as required in both versions.

How to reproduce
Have a resource with getCollection (with custom data provider) and ApiFilter which defines one property as required.
Expected behavior is to get the http reponse 400 with an description that property is required instead of http 200. (Response examples are below)

Additional Context
Resource example:

namespace App\Test;

use ApiPlatform\Metadata\ApiFilter;
use ApiPlatform\Metadata\ApiResource;
use ApiPlatform\Metadata\GetCollection;

#[ApiResource(
    operations: [
        new GetCollection(
            provider: Provider::class,
        ),
    ],
    paginationEnabled: false,
)]
#[ApiFilter(
    filterClass: Filter::class,
    properties: [
        'prop' => 'filter_a',
    ]
)]
class SimpleResource
{ }

Collection provider example:

namespace App\Test;

use ApiPlatform\Metadata\Operation;
use ApiPlatform\State\ProviderInterface;

class Provider implements ProviderInterface
{
    public function provide(Operation $operation, array $uriVariables = [], array $context = []): object|array|null
    {
        return [];
    }
}

Filter:

namespace App\Test;

use ApiPlatform\Elasticsearch\Filter\ConstantScoreFilterInterface;
use ApiPlatform\Metadata\Operation;

final class Filter implements ConstantScoreFilterInterface
{
    private array $properties;
    public function __construct(array $properties = [])
    {
        $this->properties = $properties;
    }

    public function apply(array $clauseBody, string $resourceClass, ?Operation $operation = null, array $context = []): array
    {
        return $clauseBody;
    }

    public function getDescription(string $resourceClass): array
    {
        $description = [];

        $properties = $this->properties;

        foreach ($properties as $property => $strategy) {
            $description[$property] = [
                'property' => $property,
                'type' => 'integer',
                'required' => true,
                'is_collection' => false, // or true, result is the same
                'openapi' => [
                    'type' => 'integer',
                ],
            ];
        }

        return $description;
    }
}

image

Response from 3.2.26:

{
  "@context": "\/api\/contexts\/Error",
  "@type": "hydra:Error",
  "hydra:title": "An error occurred",
  "hydra:description": "Query parameter \"prop\" is required",
  "trace": []
}

Response from 4.0.4:

{
  "@context": "\/api\/contexts\/SimpleResource",
  "@id": "\/api\/simple_resources",
  "@type": "Collection",
  "totalItems": 0,
  "member": [],
  "search": {
    "@type": "IriTemplate",
    "template": "\/api\/simple_resources{?prop}",
    "variableRepresentation": "BasicRepresentation",
    "mapping": [
      {
        "@type": "IriTemplateMapping",
        "variable": "prop",
        "property": "prop",
        "required": true
      }
    ]
  }
}
f-jost commented 2 weeks ago

Hi, have you checked the documentation? https://api-platform.com/docs/core/filters/#parameter-validation

You can declare parameters on a Resource or an Operation through the parameters property.

In your example:

new GetCollection(
    provider: Provider::class,
    parameters: ['prop' => new QueryParameter(required: true)],
),

A GET /simple_resources gives you:

{
    "@context": "/api/contexts/ConstraintViolationList",
    "@id": "/api/validation_errors/ad32d13f-c3d4-423b-909a-857b961eb720",
    "@type": "ConstraintViolationList",
    "status": 422,
    "violations": [
        {
            "propertyPath": "prop",
            "message": "The parameter \"prop\" is required.",
            "code": "ad32d13f-c3d4-423b-909a-857b961eb720"
        }
    ],
    "detail": "prop: The parameter \"prop\" is required.",
    "description": "prop: The parameter \"prop\" is required.",
    "type": "/validation_errors/ad32d13f-c3d4-423b-909a-857b961eb720",
    "title": "An error occurred"
}
JercSi commented 2 weeks ago

Your suggestion is almost perfect. If i do any changes on the filter (e.g. property is no longer required) I would need to apply this change on all resources where my filter is used.

Current solution is, to set alias to the filter and use it parameters list. Downside of this solution is that I need to set on all resources a parameters and link them with my filter.

#[ApiResource(
    operations: [
        new GetCollection(
            provider: Provider::class,
            parameters: [
                'prop' => new QueryParameter(filter: 'MyFilterClass')
            ]
        ),
    ],
    paginationEnabled: false,
)]
#[ApiFilter(
    filterClass: Filter::class,
    properties: [
        'prop' => 'filter_a',
    ],
    alias: 'MyFilterClass',
)]