core icon indicating copy to clipboard operation
core copied to clipboard

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

Open IcarusSosie opened this issue 1 year ago • 4 comments

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.

IcarusSosie avatar Jul 25 '24 16:07 IcarusSosie

would you be able to propose a patch? Thanks!

soyuka avatar Aug 08 '24 14:08 soyuka

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.

IcarusSosie avatar Aug 13 '24 08:08 IcarusSosie

Nice lmk if you need help

soyuka avatar Aug 29 '24 21:08 soyuka

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?

soyuka avatar Oct 17 '24 12:10 soyuka

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Dec 18 '24 04:12 stale[bot]