api-platform / core

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

Filter Parameters only get documented on GetCollection operations in OpenAPI document #6498

Closed IcarusSosie closed 1 month ago

IcarusSosie commented 1 month ago

API Platform version(s) affected: tested on main, but the code mentioned here is also present on 3.3.11

Description
Query/header parameter documentation generated as a result of adding a filter to an ApiResource, either via the ApiFilter attribute, or the filters parameter on ApiResource/Metadata, seem to only show up in swagger for endpoints corresponding to a GetCollection operation.

I've tested this on my API-Platform project, with both a custom filter and PropertyFilter.

I'm honestly not sure this isn't the expected behavior, but I've followed the generation of the OpenApi document via debugger and it seems there's something missing. Either behavior-wise or documentation-wise.

First, in ParameterResourceMetadataCollectionFactory::addFilterValidation, for each filter on the current operation metadata is being collected for, a simple QueryParameter gets added with the sole purpose of containing the constraints associated with the filter's definition. A comment mentions :

// we disable openapi and hydra on purpose as their docs comes from filters see the condition for addFilterValidation above.

This comment leaves me confused as to where the docs actually come from. Is it some other ResourceMetadataCollectionFactoryInterface, is it OpenApiFactory itself ? A link to the code actually creating the documentation for this parameter would be welcome here.

Second, in OpenApiFactory, the getFiltersParameters method is responsible for returning an array of filters applicable to an operation. According to the method signature, it takes either an HttpOperation or a CollectionOperationInterface. This looks to me like the method is meant to be able to act on both types of operations. However, this method is used only in collectPaths, in this context :

if ($operation instanceof CollectionOperationInterface && 'POST' !== $method) {
    foreach (array_merge($this->getPaginationParameters($operation), $this->getFiltersParameters($operation)) as $parameter) {
        if ($operationParameter = $this->hasParameter($openapiOperation, $parameter)) {
            continue;
        }

        $openapiOperation = $openapiOperation->withParameter($parameter);
    }
}

So far this seems like the only place where the aforementioned docs generation happens, but it's limited to GetCollection operations.

Whilst most filters really only operate to remove elements or reorder elements in a list of results, currently filters are generic enough in behavior to apply to all types of operations. PropertyFilter and GroupFilter are two filters that can apply to a unique item.

How to reproduce
Add an #[ApiFilter(PropertyFilter::class)] attribute on an ApiResource that has all operations. Access the swagger documentation. Only the GetCollection endpoints have the properties[] parameter documented.

Possible Solution
I'm currently thinking of adding a step to my decoration of OpenApiFactory to "fix" this behavior and test if that has any side-effects.

Additional context

As an aside : the docs website does encourage users to use Vulcain instead of PropertyFilter. I did try to follow this recommendation at first, but honestly a few reasons make me hesitant to actually use Vulcain :

  • A few areas in the docs mention using HTTP/2 Push, and Vulcain touts support for this feature. But, only after experiencing it not working on my own project, and some research online, did I discover that Push was disabled on chrome, which is effectively equivalent to the feature being killed;
  • Vulcain does make the claim that performance will still be better with Preload headers as opposed to embedding the data, but does not back up that claim;
  • Same issue for cache hit rates, I'd like an explanation of why cache will be hit more with Vulcain than without, as it's not entirely clear to me;
  • Using Preload/Early Hints/Push changes the way you interact with the data. Yeah in essence the cost of querying preloaded data is nil, but the consumer might be using a reactive framework that handles futures in such a way that it makes the experience of querying preloads a pain. My experiences with strictly-typed languages and reactivity (Dart+Flutter, Rust+Leptos) only reinforce this assumption.

I 'm aware this isn't the place to put this. I started a draft issue locally addressing this subject but I found it too confrontational. I know API-Platform, FrankenPHP, Vulcain and Mercure originate from the same people, but I find the relationship between the projects confusing. I'm more comfortable with an aside here for now, but if needed I'll create a separate issue.

soyuka commented 1 month ago

Parameters and Filters are two different things, you should probably try to use https://api-platform.com/docs/core/filters/#parameters directly. Filters are only working on collection operations.

IcarusSosie commented 1 month ago

Haha I really needed this vacation. Sorry about that !

I'll take a look at parameters and open another issue if I'm still having trouble. Closing this for now.