OpenAPI is missing additionalProperties:false (when allow_extra_attributes=false)

[guilliamxavier]

API Platform version(s) affected: 2.6.4

Description
(extracted from #4215 (comment))
With default config + denormalizationContext={"allow_extra_attributes": false}, the generated OpenAPI schemas don't have "additionalProperties": false.

How to reproduce
Config:

api_platform:
    defaults:
        denormalization_context:
            allow_extra_attributes: false

and some dummy API resource.
Command (in a Symfony app): php bin/console api:openapi:export

Possible Solution
Remove this line:

            $schema->setDefinitions($schemas);

(introduced by a commit titled "Improve openapi performances").

Additional Context
Even #4247 doesn't fix it.
But if I customize the config by adding this:

    formats:
        jsonld: ['application/ld+json']
    patch_formats:
        json: ['application/merge-patch+json']

then [well, I lose /docs.json and Swagger UI, but I can still run the command, and] the generated schemas do have "additionalProperties": false 🤔

[guilliamxavier]

Originally posted by @soyuka in #4215 (comment):

Weird as

core/src/JsonSchema/SchemaFactory.php

Lines 120 to 124 in c81eaae

	// additionalProperties are allowed by default, so it does not need to be set explicitly, unless allow_extra_attributes is false
	// See https://json-schema.org/understanding-json-schema/reference/object.html#properties
	if (false === ($serializerContext[AbstractNormalizer::ALLOW_EXTRA_ATTRIBUTES] ?? true)) {
	$definition['additionalProperties'] = false;
	}

should work your way.

For the patch format to me the only lines modifying something are here: https://github.com/api-platform/core/blob/main/src/Serializer/SerializerContextBuilder.php#L104-L116

From what I know having $schema->setDefinitions($schemas); allows to re-use previously generated schemas (as this is really time-consuming for properties). Maybe that the context is lost in the way?

[guilliamxavier]

Originally posted by @guilliamxavier in #4215 (comment):

Weird indeed 😔 Speaking of context, 49ce534 replaced $context with null (and added , $forceSchemaCollection) in both calls to $this->jsonSchemaFactory->buildSchema(/*...*/) in the body of private function collectPaths, making the array $context parameter not used anymore; but I have tried reverting these null back to $context and it doesn't seem to fix anything, worse actually it cancels the effect of removing $schema->setDefinitions($schemas); 😕

Edit: and also cancels the effect of customizing the config

[guilliamxavier]

Some more digging: After adding some debugging:

diff --git a/src/JsonSchema/SchemaFactory.php b/src/JsonSchema/SchemaFactory.php
--- a/src/JsonSchema/SchemaFactory.php	(revision 7d9916150522a292749ed4f6b5615169d8a17e72)
+++ b/src/JsonSchema/SchemaFactory.php	(date 1621954945820)
@@ -69,6 +69,7 @@
      */
     public function buildSchema(string $className, string $format = 'json', string $type = Schema::TYPE_OUTPUT, ?string $operationType = null, ?string $operationName = null, ?Schema $schema = null, ?array $serializerContext = null, bool $forceCollection = false): Schema
     {
+        echo implode(' | ', [$className, $format, $type, $operationType, $operationName]).' | ';
         $schema = $schema ? clone $schema : new Schema();
         if (null === $metadata = $this->getMetadata($className, $type, $operationType, $operationName, $serializerContext)) {
             return $schema;
@@ -81,6 +82,7 @@
 
         $version = $schema->getVersion();
         $definitionName = $this->buildDefinitionName($className, $format, $inputOrOutputClass, $resourceMetadata, $serializerContext);
+        echo $definitionName.' | ';
 
         if (null === $operationType || null === $operationName) {
             $method = Schema::TYPE_INPUT === $type ? 'POST' : 'GET';
@@ -107,6 +109,7 @@
         $definitions = $schema->getDefinitions();
         if (isset($definitions[$definitionName])) {
             // Already computed
+            echo 'Already computed: '; var_dump($definitions[$definitionName]['additionalProperties'] ?? null);
             return $schema;
         }
 
@@ -122,6 +125,7 @@
         if (false === ($serializerContext[AbstractNormalizer::ALLOW_EXTRA_ATTRIBUTES] ?? true)) {
             $definition['additionalProperties'] = false;
         }
+        var_dump($definition['additionalProperties'] ?? null);
 
         // see https://github.com/json-schema-org/json-schema-spec/pull/737
         if (

then:

1. default config:

   • current:

     class	fmt	type	op. type	op.	definition	additionalProperties
     App\Entity\Dummy	jsonld	output	item	get	Dummy.jsonld	NULL
     App\Entity\Dummy	json	output	item	get	Dummy	NULL
     App\Entity\Dummy	html	output	item	get	Dummy	Already computed: NULL
     App\Entity\Dummy	jsonld	output	item	delete	Dummy.jsonld	Already computed: NULL
     App\Entity\Dummy	json	output	item	delete	Dummy	Already computed: NULL
     App\Entity\Dummy	html	output	item	delete	Dummy	Already computed: NULL
     App\Entity\Dummy	jsonld	output	item	put	Dummy.jsonld	Already computed: NULL
     App\Entity\Dummy	json	output	item	put	Dummy	Already computed: NULL
     App\Entity\Dummy	html	output	item	put	Dummy	Already computed: NULL
     App\Entity\Dummy	jsonld	input	item	put	Dummy.jsonld	Already computed: NULL
     App\Entity\Dummy	json	input	item	put	Dummy	Already computed: NULL
     App\Entity\Dummy	html	input	item	put	Dummy	Already computed: NULL
     App\Entity\Dummy	jsonld	output	collection	get	Dummy.jsonld	Already computed: NULL
     App\Entity\Dummy	json	output	collection	get	Dummy	Already computed: NULL
     App\Entity\Dummy	html	output	collection	get	Dummy	Already computed: NULL
     App\Entity\Dummy	jsonld	output	collection	post	Dummy.jsonld	Already computed: NULL
     App\Entity\Dummy	json	output	collection	post	Dummy	Already computed: NULL
     App\Entity\Dummy	html	output	collection	post	Dummy	Already computed: NULL
     App\Entity\Dummy	jsonld	input	collection	post	Dummy.jsonld	Already computed: NULL
     App\Entity\Dummy	json	input	collection	post	Dummy	Already computed: NULL
     App\Entity\Dummy	html	input	collection	post	Dummy	Already computed: NULL

     => no "additionalProperties": false

   • after removing $schema->setDefinitions($schemas);:

     class	fmt	type	op. type	op.	definition	additionalProperties
     App\Entity\Dummy	jsonld	output	item	get	Dummy.jsonld	NULL
     App\Entity\Dummy	json	output	item	get	Dummy	NULL
     App\Entity\Dummy	html	output	item	get	Dummy	NULL
     App\Entity\Dummy	jsonld	output	item	delete	Dummy.jsonld	NULL
     App\Entity\Dummy	json	output	item	delete	Dummy	NULL
     App\Entity\Dummy	html	output	item	delete	Dummy	NULL
     App\Entity\Dummy	jsonld	output	item	put	Dummy.jsonld	NULL
     App\Entity\Dummy	json	output	item	put	Dummy	NULL
     App\Entity\Dummy	html	output	item	put	Dummy	NULL
     App\Entity\Dummy	jsonld	input	item	put	Dummy.jsonld	bool(false)
     App\Entity\Dummy	json	input	item	put	Dummy	bool(false)
     App\Entity\Dummy	html	input	item	put	Dummy	bool(false)
     App\Entity\Dummy	jsonld	output	collection	get	Dummy.jsonld	NULL
     App\Entity\Dummy	json	output	collection	get	Dummy	NULL
     App\Entity\Dummy	html	output	collection	get	Dummy	NULL
     App\Entity\Dummy	jsonld	output	collection	post	Dummy.jsonld	NULL
     App\Entity\Dummy	json	output	collection	post	Dummy	NULL
     App\Entity\Dummy	html	output	collection	post	Dummy	NULL
     App\Entity\Dummy	jsonld	input	collection	post	Dummy.jsonld	bool(false)
     App\Entity\Dummy	json	input	collection	post	Dummy	bool(false)
     App\Entity\Dummy	html	input	collection	post	Dummy	bool(false)

     => "additionalProperties": false in both schemas

2. customized config:

   • current:

     class	fmt	type	op. type	op.	definition	additionalProperties
     App\Entity\Dummy	jsonld	output	item	get	Dummy.jsonld	NULL
     App\Entity\Dummy	jsonld	output	item	delete	Dummy.jsonld	Already computed: NULL
     App\Entity\Dummy	jsonld	output	item	put	Dummy.jsonld	Already computed: NULL
     App\Entity\Dummy	jsonld	input	item	put	Dummy.jsonld	Already computed: NULL
     App\Entity\Dummy	jsonld	output	item	patch	Dummy.jsonld	Already computed: NULL
     App\Entity\Dummy	json	input	item	patch	Dummy	bool(false)
     App\Entity\Dummy	jsonld	output	collection	get	Dummy.jsonld	Already computed: NULL
     App\Entity\Dummy	jsonld	output	collection	post	Dummy.jsonld	Already computed: NULL
     App\Entity\Dummy	jsonld	input	collection	post	Dummy.jsonld	Already computed: NULL

     => "additionalProperties": false in Dummy but not in Dummy.jsonld

   • after removing $schema->setDefinitions($schemas);:

     class	fmt	type	op. type	op.	definition	additionalProperties
     App\Entity\Dummy	jsonld	output	item	get	Dummy.jsonld	NULL
     App\Entity\Dummy	jsonld	output	item	delete	Dummy.jsonld	NULL
     App\Entity\Dummy	jsonld	output	item	put	Dummy.jsonld	NULL
     App\Entity\Dummy	jsonld	input	item	put	Dummy.jsonld	bool(false)
     App\Entity\Dummy	jsonld	output	item	patch	Dummy.jsonld	NULL
     App\Entity\Dummy	json	input	item	patch	Dummy	bool(false)
     App\Entity\Dummy	jsonld	output	collection	get	Dummy.jsonld	NULL
     App\Entity\Dummy	jsonld	output	collection	post	Dummy.jsonld	NULL
     App\Entity\Dummy	jsonld	input	collection	post	Dummy.jsonld	bool(false)

     => "additionalProperties": false in both schemas

Quick analysis: $serializerContext[AbstractNormalizer::ALLOW_EXTRA_ATTRIBUTES] is false only for input operations, but at that point the definition was already computed for the first output operation.

[guilliamxavier]

Workaround (without patching vendor code): config diff:

 api_platform:
     defaults:
         denormalization_context: { allow_extra_attributes: false }
+        normalization_context: { allow_extra_attributes: false }

but doesn't really make sense 😕

Maybe additionalProperties should be "merged" into the already computed definition? (and maybe other things too?)

[stale]

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]

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.