api-platform / core

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

Regression in `jsonopenapi` serialization of references caused by #4019 #6496

Open IcarusSosie opened 3 months ago

IcarusSosie commented 3 months ago

API Platform version(s) affected: 2.6.2 to 3.3.6

Description
PR #4019 introduces an anonymous service argument in openapi.xml for the service api_platform.openapi.normalizer. Whilst the change does fix an issue with incorrect OpenAPI document serialization when the default name converter is changed, it also has a secondary effect : OpenApiNormalizer now does not have access to Attribute metadata when serializing the OpenAPI definition. Since ApiPlatform\OpenApi\Model\Reference declares a #[SerializedName('$ref')] attribute on its getRef() method, any reference created by the OpenApiFactory will be serialized as

{ "ref" : "#/some/ref" }

instead of

{ "$ref" : "#/some/ref" }

causing SwaggerUI to render incorrectly. For example, a reference to a path parameter will show up as an empty field with no info or context.

As far as I know, the default OpenApiFactory does not generate references except for JSON schemas, which are not affected, but any reference added to the OpenApi object by a decorator of OpenApiFactory will serialize incorrectly.

How to reproduce
In an API-Platform project, decorate OpenApiFactory :

<?php

namespace App\OpenApi;

use ApiPlatform\OpenApi\Model\Parameter;
use ApiPlatform\OpenApi\Model\Paths;
use ApiPlatform\OpenApi\Model\Reference;
use ArrayObject;
use ApiPlatform\OpenApi\Factory\OpenApiFactoryInterface;
use ApiPlatform\OpenApi\Model\PathItem;
use ApiPlatform\OpenApi\OpenApi;
use Symfony\Component\DependencyInjection\Attribute\AsDecorator;
use Symfony\Component\DependencyInjection\Attribute\AutowireDecorated;

#[AsDecorator(
    decorates: 'api_platform.openapi.factory',
)]
readonly class OpenApiFactoryDemo implements OpenApiFactoryInterface
{
    public function __construct(
        #[AutowireDecorated]
        private OpenApiFactoryInterface $decorated,
    )
    {
    }

    /**
     * {@inheritdoc}
     */
    public function __invoke(array $context = []) : OpenApi
    {
        $openApi = ($this->decorated)($context);

        $parameters       = $openApi->getComponents()->getParameters();
        $editedComponents = $openApi->getComponents()->withParameters(
            new ArrayObject(
                array_merge(
                    $parameters?->getArrayCopy() ?? [],
                    [
                        'MyNewParameter' => new Parameter(
                            name: 'MyNewParameter',
                            in: 'header',
                            description: <<<Markdown
                                Lorem Ipsum dolor sir amet
                                Markdown,
                            required: false,
                            schema: ['type' => 'string'],
                            example: 'test',
                        ),
                    ],
                ),
            ),
        );

        $paths       = $openApi->getPaths();
        $editedPaths = new Paths();

        /** @var PathItem $item */
        foreach ($paths->getPaths() as $path => $item) {
            $editedPaths->addPath(
                $path,
                $item->withParameters([
                    ...$item->getParameters(),
                    new Reference('#/components/parameters/MyNewParameter'),
                ])
            );
        }

        return $openApi
            ->withComponents($editedComponents)
            ->withPaths($editedPaths);
    }
}

Then, access the swagger docs. All operations will have a dud input corresponding to the invalid reference.

Possible Solution
Currently, I'm solving this by overriding api_platform.openapi.normalizer in services.yaml :

services:

  # [...]

  api_platform.openapi.normalizer:
    class: ApiPlatform\OpenApi\Serializer\OpenApiNormalizer
    public: true
    arguments:
      - !service
        class: Symfony\Component\Serializer\Serializer
        arguments:
          - - !service
              class: Symfony\Component\Serializer\Normalizer\ObjectNormalizer
              arguments:
                - '@api_platform.serializer.mapping.class_metadata_factory'
                - !service
                  class: Symfony\Component\Serializer\NameConverter\MetadataAwareNameConverter
                  arguments:
                    - '@api_platform.serializer.mapping.class_metadata_factory'
                - '@api_platform.property_accessor'
                - '@api_platform.property_info'
          - [ '@serializer.encoder.json' ]
    tags:
      - name: 'serializer.normalizer'
        priority: -795

The solution is quite similar to the fix in #4019, using anonymous services to not override other services declarations using the same underlying class. I'm just adding the MetadataAwareNameConverter with no fallback, which I think might avoid issues if I decide to specify a default name converter in the future.

Additional Context
I'm not a Symfony expert by any means, so I might be missing some other quicker solution or workaround. My API-Platform setup is also quite custom, which might affect serializer functionality, although that would be surprising.

soyuka commented 2 months ago

would you be able to propose a patch? Thanks!

IcarusSosie commented 2 months ago

Hi, I just got back from vacation :smile:

I can absolutely create the PR. I'm assuming editing the relevant XML configuration file should suffice for the behavior to be fixed, but I'll have to take a closer look to tests to add a test case with OpenAPI references.

soyuka commented 1 month ago

Nice lmk if you need help

soyuka commented 1 week ago

Not sure I understand the bug actually I could not reproduce, you can override the name converter by specifying the service name inside api_platform.name_converter and we do the proper aliasing to always use the MetadataAwareNameConverter. Can you provide a proper reproducer?