Skip to content

Conversation

LoicBoursin
Copy link
Contributor

Q A
Branch? 3.1
Tickets N/A
License MIT
Doc PR N/A

This PR fixes an issue when a shortName is set on a resource used as a subresource. The bug was introduced in https://github.com/api-platform/core/pull/4818/files

For example:

<?php

declare(strict_types=1);

namespace App\Api\Resource;

#[ApiResource(
    shortName: 'Review',
    operations: [
        new Get(),
    ],
    provider: ReviewItemStateProvider::class,
    processor: StateProcessor::class,
)]
class ReviewResource
{
    public ?UuidV4 $id = null;

    #[ApiProperty(writable: false, readableLink: true)]
    public ?ReviewAnswerResource $answer = null;
}

has a dependency on ReviewAnswerResource. When this type was checked by the TypeFactory, a schema was generated by JsonSchema/TypeFactory.php:L155. This call to buildSchema is made with the $operation parameter set to null.

In JsonSchema/SchemaFactory.php:L63 we're getting the metadata of the class and retrieving an operation. This operation was an uninitialized HttpOperation set at JsonSchema/SchemaFactory.php:L293. So when we're calling JsonSchema/SchemaFactory.php::buildDefinitionName, we have no operation shortName, hence the reflectionClass shortName is used (ReviewAnswerResource in this example), instead of the shortName set on the ApiResource attribute.

By passing the resource shortName to this uninitialized HttpOperation, the issue is fixed and the generated OpenAPI schema is now the same as in API Platform 2.6

@soyuka
Copy link
Member

soyuka commented Mar 14, 2023

Isnt't ReviewAnswerResource also a Resource? I'm not sure to understand as it looks like you said that the ReviewResource::$answer type is Review but it should be ReviewAnswerResource no?

@LoicBoursin
Copy link
Contributor Author

LoicBoursin commented Mar 15, 2023

@soyuka You're right, I think my message was unclear. Here is a detailed explanation with a before/after.

We have two resources : ReviewResource and ReviewAnswerResource. All of our resources have shortNames.

Here is ReviewResource (much simplified):

#[ApiResource(
    shortName: 'Review',
    operations: [
        new Get(),
    ],
    normalizationContext: [
        'skip_null_values' => true,
    ],
    provider: ReviewItemStateProvider::class,
    processor: StateProcessor::class,
)]
class ReviewResource
{
    public ?UuidV4 $id = null;

    #[ApiProperty(writable: false, readableLink: true)]
    public ?ReviewAnswerResource $answer = null;
}

and ReviewAnswerResource (much simplified):

#[ApiResource(
    shortName: 'ReviewAnswer',
    operations: [
        new Get(),
    ],
    provider: DefaultItemStateProvider::class,
    processor: StateProcessor::class,
)]
class ReviewAnswerResource
{
    public ?UuidV4 $id = null;

    #[ApiProperty(required: true)]
    #[Assert\NotBlank(groups: ['review-answer:write'])]
    #[Assert\Length(min: 10, groups: ['review-answer:write'])]
    public string $comment;
}

Here is the interesting part of our openAPI schema prior to #4818 :

"Review": {
                "type": "object",
                "description": "",
                "properties": {
                    "id": {
                        "type": "string",
                        "format": "uuid",
                        "nullable": true
                    },
                    "answer": {
                        "readOnly": true,
                        "anyOf": [
                            {
                                "$ref": "#/components/schemas/ReviewAnswer"
                            }
                        ],
                        "nullable": true
                    },
                },
            },

And after the update to 3.0 (containing changes in #4818) :

"Review": {
                "type": "object",
                "description": "",
                "deprecated": false,
                "properties": {
                    "id": {
                        "type": "string",
                        "format": "uuid",
                        "nullable": true
                    },
                    "answer": {
                        "readOnly": true,
                        "anyOf": [
                            {
                                "$ref": "#/components/schemas/ReviewAnswerResource"
                            }
                        ],
                        "nullable": true
                    },
                },
            },

Note that the $ref is now ReviewAnswerResource instead of the shortName of that class (ReviewAnswer). This change also generated a new schema named ReviewAnswerResource (we don't want/need it) which is a duplicate of ReviewAnswer's schema.

@soyuka
Copy link
Member

soyuka commented Mar 15, 2023

I see, but why can't we find an operation for the type of Review::answer?

We agree that:

        $subSchema = $this->schemaFactory->buildSchema($className, $format, Schema::TYPE_OUTPUT, null, $subSchema, $serializerContext, false);
       

is called with $className: ReviewAnswerResource::class and no operation, the JsonSchema condition looks wrong at line 273.

Can you try this patch? https://gist.github.com/soyuka/bbd7fa0656fbce2b72383898dd0e22eb

@LoicBoursin
Copy link
Contributor Author

LoicBoursin commented Mar 15, 2023

@soyuka Just tried it and it works perfectly 🎉 with that fix, our 2.6 schema is now ISO with 3.0 👍🏼

@soyuka
Copy link
Member

soyuka commented Mar 17, 2023

we're working on a test at #5469 thanks for the report @LoicBoursin

@soyuka soyuka closed this Mar 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants