fix(jsonschema): generation of non-LD+JSON distinct schema formats

[GwendolenLynch]

Q	A
Branch?	3.2
Tickets	Closes #6228
License	MIT

I've not figured out a good way to CI test this, so guidance will be gratefully appreciated.

To reproduce the base issue, apply the following to the 3.2 branch and run vendor/bin/simple-phpunit --filter JsonSchemaGenerateCommandTest::testSubSchemaJsonLd

diff --git a/src/Hydra/JsonSchema/SchemaFactory.php b/src/Hydra/JsonSchema/SchemaFactory.php
index b04ba616b8..43478de74d 100644
--- a/src/Hydra/JsonSchema/SchemaFactory.php
+++ b/src/Hydra/JsonSchema/SchemaFactory.php
@@ -186,7 +186,7 @@ public function buildSchema(string $className, string $format = 'jsonld', string
 
     public function addDistinctFormat(string $format): void
     {
-        if ($this->schemaFactory instanceof BaseSchemaFactory) {
+        if (method_exists($this->schemaFactory, 'addDistinctFormat')) {
             $this->schemaFactory->addDistinctFormat($format);
         }
     }
diff --git a/src/JsonApi/JsonSchema/SchemaFactory.php b/src/JsonApi/JsonSchema/SchemaFactory.php
new file mode 100644
index 0000000000..b9747c281d
--- /dev/null
+++ b/src/JsonApi/JsonSchema/SchemaFactory.php
@@ -0,0 +1,41 @@
+<?php
+
+declare(strict_types=1);
+
+namespace ApiPlatform\JsonApi\JsonSchema;
+
+use ApiPlatform\JsonSchema\Schema;
+use ApiPlatform\JsonSchema\SchemaFactoryAwareInterface;
+use ApiPlatform\JsonSchema\SchemaFactoryInterface;
+use ApiPlatform\Metadata\Operation;
+
+final class SchemaFactory implements SchemaFactoryInterface
+{
+    public function __construct(private SchemaFactoryInterface $schemaFactory)
+    {
+        $this->addDistinctFormat('jsonapi');
+        if ($this->schemaFactory instanceof SchemaFactoryAwareInterface) {
+            $this->schemaFactory->setSchemaFactory($this);
+        }
+    }
+
+    public function buildSchema(string $className, string $format = 'jsonapi', string $type = Schema::TYPE_OUTPUT, ?Operation $operation = null, ?Schema $schema = null, ?array $serializerContext = null, bool $forceCollection = false): Schema
+    {
+        $schema = $this->schemaFactory->buildSchema($className, $format, $type, $operation, $schema, $serializerContext, $forceCollection);
+
+        if ('jsonapi' !== $format) {
+            return $schema;
+        }
+
+        // Do schema updates here …
+
+        return $schema;
+    }
+
+    public function addDistinctFormat(string $format): void
+    {
+        if (method_exists($this->schemaFactory, 'addDistinctFormat')) {
+            $this->schemaFactory->addDistinctFormat($format);
+        }
+    }
+}
diff --git a/src/Symfony/Bundle/Resources/config/jsonapi.xml b/src/Symfony/Bundle/Resources/config/jsonapi.xml
index 0a84066acc..98f1b08bc6 100644
--- a/src/Symfony/Bundle/Resources/config/jsonapi.xml
+++ b/src/Symfony/Bundle/Resources/config/jsonapi.xml
@@ -5,6 +5,10 @@
            xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd">
 
     <services>
+        <service id="api_platform.jsonapi.json_schema.schema_factory" class="ApiPlatform\JsonApi\JsonSchema\SchemaFactory" decorates="api_platform.json_schema.schema_factory">
+            <argument type="service" id="api_platform.jsonapi.json_schema.schema_factory.inner" />
+        </service>
+
         <service id="api_platform.jsonapi.encoder" class="ApiPlatform\Serializer\JsonEncoder" public="false">
             <argument>jsonapi</argument>
 

[soyuka]

something's bothering me here I need to check that, I don't understand why we put array in the first place.

[GwendolenLynch]

Introduced in #4357, as was addDistinctFormat()

[soyuka]

https://datatracker.ietf.org/doc/html/draft-kelly-json-hal#section-4.1.2

It is an object whose property names are link relation types (as defined by [RFC5988]) and values are either a Resource Object or an array of Resource Objects.

can you revert this change? I think that our HAL representation always uses an array and we use items as key.

[GwendolenLynch]

Doesn't that make the behaviour in ApiPlatform\Hal\Serializer\CollectionNormalizer incorrect?

core/src/Hal/Serializer/CollectionNormalizer.php

Lines 86 to 100 in 874e4d6

	protected function getItemsData(iterable $object, ?string $format = null, array $context = []): array
	{
	$data = [];
	foreach ($object as $obj) {
	$item = $this->normalizer->normalize($obj, $format, $context);
	if (!\is_array($item)) {
	throw new UnexpectedValueException('Expected item to be an array');
	}
	$data['_embedded']['item'][] = $item;
	$data['_links']['item'][] = $item['_links']['self'] ?? null;
	}
	return $data;
	}

The changes in ApiTestCaseTest (testing jsonld and jsonhal as formats) trigger a failure condition without this change:

1) ApiPlatform\Tests\Symfony\Bundle\Test\ApiTestCaseTest::testAssertMatchesResourceCollectionJsonSchema with data set "jsonhal" ('jsonhal', 'application/hal+json')
Failed asserting that Array &0 (
    '_links' => Array &1 (
        'self' => Array &2 (
            'href' => '/resource_interfaces'
        )
        'item' => Array &3 (
            0 => Array &4 (
                'href' => '/resource_interfaces/item1'
            )
            1 => Array &5 (
                'href' => '/resource_interfaces/item2'
            )
        )
    )
    '_embedded' => Array &6 (
        'item' => Array &7 (
            0 => Array &8 (
                '_links' => Array &9 (
                    'self' => Array &10 (
                        'href' => '/resource_interfaces/item1'
                    )
                )
                'foo' => 'item1'
                'fooz' => 'fooz'
            )
            1 => Array &11 (
                '_links' => Array &12 (
                    'self' => Array &13 (
                        'href' => '/resource_interfaces/item2'
                    )
                )
                'foo' => 'item2'
                'fooz' => 'fooz'
            )
        )
    )
) matches the provided JSON Schema.
_embedded: Object value found, but an array is required

2) ApiPlatform\Tests\Symfony\Bundle\Test\ApiTestCaseTest::testAssertMatchesResourceCollectionJsonSchemaKeepSerializationContext with data set "jsonhal" ('jsonhal', 'application/hal+json')
Failed asserting that Array &0 (
    '_links' => Array &1 (
        'self' => Array &2 (
            'href' => '/issue-6146-parents'
        )
        'item' => Array &3 (
            0 => Array &4 (
                'href' => '/issue-6146-parents/1'
            )
        )
    )
    'totalItems' => 1
    'itemsPerPage' => 3
    '_embedded' => Array &5 (
        'item' => Array &6 (
            0 => Array &7 (
                '_links' => Array &8 (
                    'self' => Array &9 (
                        'href' => '/issue-6146-parents/1'
                    )
                )
            )
        )
    )
) matches the provided JSON Schema.
_embedded: Object value found, but an array is required

3) ApiPlatform\Tests\Symfony\Bundle\Test\ApiTestCaseTest::testAssertMatchesResourceItemAndCollectionJsonSchemaOutputWithContext with data set "jsonhal" ('jsonhal', 'application/hal+json')
Failed asserting that Array &0 (
    '_links' => Array &1 (
        'self' => Array &2 (
            'href' => '/users-with-groups'
        )
        'item' => Array &3 (
            0 => Array &4 (
                'href' => '/users/1'
            )
        )
    )
    'totalItems' => 1
    'itemsPerPage' => 3
    '_embedded' => Array &5 (
        'item' => Array &6 (
            0 => Array &7 (
                '_links' => Array &8 (
                    'self' => Array &9 (
                        'href' => '/users/1'
                    )
                )
                'fullname' => 'Grégoire'
            )
        )
    )
) matches the provided JSON Schema.
_embedded: Object value found, but an array is required

[GwendolenLynch]

From https://datatracker.ietf.org/doc/html/draft-kelly-json-hal#section-6

{
  "_links": {
    "self": { "href": "/orders" },
    "next": { "href": "/orders?page=2" },
    "find": { "href": "/orders{?id}", "templated": true }
  },
  "_embedded": {
    "orders": [{
        "_links": {
          "self": { "href": "/orders/123" },
          "basket": { "href": "/baskets/98712" },
          "customer": { "href": "/customers/7809" }
        },
        "total": 30.00,
        "currency": "USD",
        "status": "shipped",
      },{
        "_links": {
          "self": { "href": "/orders/124" },
          "basket": { "href": "/baskets/97213" },
          "customer": { "href": "/customers/12369" }
        },
        "total": 20.00,
        "currency": "USD",
        "status": "processing"
    }]
  },
  "currentlyProcessing": 14,
  "shippedToday": 20
}

Which also comes back to a previous question as to the key(s) of _embedded … internally items is used but the examples in the draft seem to use the resource name.

[soyuka]

To be correct the schema should be anyOf object or array. We force item at

core/src/Hal/Serializer/CollectionNormalizer.php

Line 95 in 874e4d6

$data['_embedded']['item'][] = $item;

therefore we need the key at item as I see it.

[GwendolenLynch]

As in how I did it originally in 97f7dff?

[GwendolenLynch]

Ugh, never mind, it clicked what you meant … now (I think) I get you 😇

[GwendolenLynch]

Looking too hard at the same thing for too long and got my brain in a twist.

[soyuka]

Right now we may have (as this is what we use for our collections and):

'anyOf' => [
    [
        // see features/hal/collection.feature
        'type' => 'object',
        'properties' => [
            'item' => [
                'type' => 'array',
                'items' => $items,
            ],
        ],
    ],
    // see features/hal/hal.feature
    [ 'type' => 'object' ]
],

Although the spec says that it can also be

// according to what I understand from the spec this is also valid:
  [
        'type' => 'array'
  ],

I'm okay with adding it if you want

[GwendolenLynch]

@soyuka I have also updated more of the tests in ApiTestCaseTest to use the formats data provider. I'm a bit nervous about coverage for non-LD+JSON formats.

If you want to suggest other test locations that I can expand on, I'll happily work away on them.

[soyuka]

took the liberty of adding the correct schema in a new commit I want to check that the CI is green

[soyuka]

Thanks @GwendolenLynch ! Feel free to propose the array schema in another PR if you think we should add it, I wanted this patch in the next release so I took the liberty of adding back the anyOf.