fix(jsonschema): make all required properties optional in PATCH operation with 'json' format

[ttskch]

Note

Is this a new feature? If so, we can close this PR and go to #6395.

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

In the schema for the request body of application/merge-patch+json PATCH operation, all required properties should be output as optional.

See here: https://datatracker.ietf.org/doc/html/rfc7386#section-3

This PR fixes the SchemaFactory for the 'json' format to output required properties as optional only for input context of PATCH operations.

For example, for the following entity, the output of the OpenAPI document will change as follows:

#[ORM\Entity(repositoryClass: TodoRepository::class)]
#[ApiResource]
class Todo
{
    #[ORM\Id]
    #[ORM\GeneratedValue]
    #[ORM\Column]
    private ?int $id = null;

    #[ORM\Column(length: 255)]
    #[Assert\NotBlank]
    #[Assert\Length(max: 255)]
    private ?string $title = null;

    #[ORM\Column(type: Types::TEXT, nullable: true)]
    private ?string $description = null;

    #[ORM\Column]
    #[Assert\NotNull]
    private ?bool $done = null;

	Before	After
PATCH Operation
Schemas

[soyuka]

I think that we can make this a bug fix as it won't break any current implementations

[soyuka]

Actually we need to revert this as we found it was breaking some implementations but we'll introduce this in 3.4. can you re-open this for 3.4 ? We also need to add a compatibility flag, my suggestion is to add schema to the Metadata class that needs to be specified for Patch operations in 4.x or will be left empty. Basically the changes in the SchemaFactory should be:

diff --git a/src/JsonSchema/SchemaFactory.php b/src/JsonSchema/SchemaFactory.php
index f9c75720f..4ba48b894 100644
--- a/src/JsonSchema/SchemaFactory.php
+++ b/src/JsonSchema/SchemaFactory.php
@@ -115,7 +115,7 @@ public function buildSchema(string $className, string $format = 'json', string $
         }
 
         /** @var \ArrayObject<string, mixed> $definition */
-        $definition = new \ArrayObject(['type' => 'object']);
+        $definition = new \ArrayObject(($operation->getSchema() ?? []) + ['type' => 'object']);
         $definitions[$definitionName] = $definition;
         $definition['description'] = $operation ? ($operation->getDescription() ?? '') : '';
 
@@ -139,6 +139,12 @@ public function buildSchema(string $className, string $format = 'json', string $
         }
 
         $options = ['schema_type' => $type] + $this->getFactoryOptions($serializerContext, $validationGroups, $operation instanceof HttpOperation ? $operation : null);
+
+        $hasRequiredProperties = isset($definition['required']);
+        if ($isJsonMergePatch && !$hasRequiredProperties) {
+            trigger_deprecation('api-platform/core', '3.4', "Specify the Patch(schema: ['required' => []]) properties if needed as api-platform 4 will force an empty array.");
+        }
+
         foreach ($this->propertyNameCollectionFactory->create($inputOrOutputClass, $options) as $propertyName) {
             $propertyMetadata = $this->propertyMetadataFactory->create($inputOrOutputClass, $propertyName, $options);
             if (!$propertyMetadata->isReadable() && !$propertyMetadata->isWritable()) {
@@ -146,7 +152,7 @@ public function buildSchema(string $className, string $format = 'json', string $
             }
 
             $normalizedPropertyName = $this->nameConverter ? $this->nameConverter->normalize($propertyName, $inputOrOutputClass, $format, $serializerContext) : $propertyName;
-            if ($propertyMetadata->isRequired() && !$isJsonMergePatch) {
+            if (!$hasRequiredProperties && $propertyMetadata->isRequired()) {
                 $definition['required'][] = $normalizedPropertyName;
             }

[ttskch]

@soyuka Sorry for this. Of course, after #6476 is merged and 3.3 is merged into 3.4, I can create a new PR for 3.4.

Actually we need to revert this as we found it was breaking some implementations

But, for reference, could you tell me what the problem was specifically? Or, if there are any related issues, please let me know.

I still think that it is reasonable for required properties to be automatically changed to optional in the schema for JSON Merge Patch.

It is inconvenient for users to have to specify #[Patch(schema: ['required' => []])] every time.

[soyuka]

Currently some users have required fields on their Patch endpoints. We need a flag to avoid breaking implementations and a way to force keeping required fields. I'll take care of porting this to 3.4.

[soyuka]

see for now #6478