fix(jsonapi): add missing "included" schema parts

[GwendolenLynch]

Q	A
Branch?	main
Tickets	Related #6250
License	MIT

Per https://jsonapi.org/format/#fetching-includes which the serializer already handles beautifully.

[GwendolenLynch]

@soyuka I have had to proof-of-concept a slightly different approach in a junk commit for review & discussion on where to go from here 🌞

Problem
Both the base and JSON:API schema factories needs to be able to reliably know the schema component "definition name" for a resource. That requires both of the getMetadata() & buildDefinitionName methods from ApiPlatform\JsonSchema\SchemaFactory.

Approach

1. I have moved buildDefinitionName and its dependant methods to ApiPlatform\JsonSchema\DefinitionNameFactory and making that a container service api_platform.json_schema.definition_name_factory that injects into the schema factories as needed.
2. getMetadata() and friends I put into ApiPlatform\JsonSchema\ResourceMetadataTrait

Questions

1. Is this (generally) an acceptable direction in this case?
2. What namespaces would you like those two new files in?

Advice and guidance warmly welcomed!

Edit: I've started breaking out more of the logic in the trait/

[soyuka]

Nice refactoring actually I think that it's a good idea to have a Trait to share the logic, not sure about adding a new interface though as buildDefinitionName is private anyways. Can that work only using the Trait?

Also don't put another author then yourself (no author phpdoc is also fine) and I think that the trait should be @internal so that we can make updates to it in the future without breaking userland.

[soyuka]

why would you reset the context?

[GwendolenLynch]

I was hoping you'd bring this up 👍

I haven't mapped it out fully yet, but the JSON:API serializer seems to mostly ignore normalization groups (c.f. include query parameter.

So if it is not a jsonapi format, just pass it on as-is, else generate the schema without groups. Hadn't gotten past that so far. I got distracted with enums.

[soyuka]

can you mention the json:api specification in a comment (e.g: the JSON:API serializer ignores normalization groups and uses an include parameter)?

side note I've come accross:

core/src/OpenApi/Factory/OpenApiFactory.php

Line 402 in 93f8b5f

$operationInputSchema = $this->jsonSchemaFactory->buildSchema($resourceClass, $operationFormat, Schema::TYPE_INPUT, $operation, $schema, null, $forceSchemaCollection);

Looks like we don't send the context from the openapi factory but I'm not sure why.

[GwendolenLynch]

[…] not sure about adding a new interface though as buildDefinitionName is private anyways. Can that work only using the Trait?

It can. I chose to externalise it as a guess as to what you'd choose … plus the thought that definition name generation becomes implementation override-able.

Your call 👍

[soyuka]

Indeed but as long as you use the shortName the extension point exists no? If you are in a weird case you need to decorate the SchemaFactory.

[GwendolenLynch]

As I understand it, it depends on how formats: config is set-up.

Assume you have the resource Foo.

If you set, for example, one format in the formats key:

api_platform:
    formats:
        jsonapi:  ['application/vnd.api+json']

… then patch_formats will default to application/merge-patch+json so you will end up with two definitions:

• Foo (write operations)
• Foo.jsonapi (read operations)

However, if you configure a non application/merge-patch+json in patch_formats:

api_platform:
    formats:
        jsonapi:  ['application/vnd.api+json']
    patch_formats:
        jsonapi:  ['application/vnd.api+json']

… you will end up with one definition:

• Foo.jsonapi

Now if you add (de)normalization groups all bets are off. Hence the refactoring in this PR.

I did not know all that a couple of weeks ago, so yeah 😇

[GwendolenLynch]

One other thing that came to mind with having it separate is unit testing. It makes that part of the process distinctly testable.

Disclaimer: I can be overly obsessive about unit tests and the "…one thing well" principle. 😊

[soyuka]

nice, few leftovers

[GwendolenLynch]

Thanks @soyuka!