Properties with multiple type unions generate incorrect OpenAPI schema

[GwendolenLynch]

API Platform version(s) affected: 3.2

Description
Currently (3.2.16) with the model below the owner property will generate the schema:

"owner": {
    "anyOf": [
        {
            "$ref": "#/components/schemas/Wren"
        },
        {
            "type": "null"
        }
    ]
}

Instead of:

"owner": {
    "anyOf": [
        {
            "$ref": "#/components/schemas/Wren"
        },
        {
            "$ref": "#/components/schemas/Robin"
        },
        {
            "type": "null"
        }
    ]
}

How to reproduce

#[ApiResource]
#[ORM\Entity]
class Nest
{
    #[ORM\Id]
    #[ORM\GeneratedValue]
    #[ORM\Column]
    private ?int $id = null;

    #[ORM\Column(type: 'bird')]
    private ?Bird $owner;

    public function getId(): ?int
    {
        return $this->id;
    }

    public function getOwner(): ?Bird
    {
        return $this->owner;
    }

    public function setOwner(Wren|Robin|null $owner): static
    {
        $this->owner = $owner;

        return $this;
    }
}

interface Bird
{
    public function getName(): ?string;

    public function getAge(): ?int;
}

final class Robin implements Bird
{
    public ?string $name = null;
    public ?int $age = null;

    public function getName(): ?string
    {
        return $this->name;
    }

    public function getAge(): ?int
    {
        return $this->age;
    }
}

final class Wren implements Bird
{
    public ?string $name = null;
    public ?int $age = null;
    public ?int $weight = null;

    public function getName(): ?string
    {
        return $this->name;
    }

    public function getAge(): ?int
    {
        return $this->age;
    }
}

Possible Solution
I dug in a bit and it stems from handling in SchemaFactory::buildPropertySchema. Simply removing the final break here make this particular problem disappear but breaks tests. So I hacked up a more robust proof-of-concept fix/patch below that addresses this issue (and passes existing tests).

diff --git a/src/JsonSchema/SchemaFactory.php b/src/JsonSchema/SchemaFactory.php
index a128a8968..9a21d47d3 100644
--- a/src/JsonSchema/SchemaFactory.php
+++ b/src/JsonSchema/SchemaFactory.php
@@ -196,10 +196,13 @@
         // property schema is created in SchemaPropertyMetadataFactory, but it cannot build resource reference ($ref)
         // complete property schema with resource reference ($ref) only if it's related to an object
         $version = $schema->getVersion();
-        $subSchema = new Schema($version);
-        $subSchema->setDefinitions($schema->getDefinitions()); // Populate definitions of the main schema
+        $refs = [];
+        $isNullable = null;
+
+        foreach ($types as $type) {
+            $subSchema = new Schema($version);
+            $subSchema->setDefinitions($schema->getDefinitions()); // Populate definitions of the main schema

-        foreach ($types as $type) {
             // TODO: in 3.3 add trigger_deprecation() as type factories are not used anymore, we moved this logic to SchemaPropertyMetadataFactory so that it gets cached
             if ($typeFromFactory = $this->typeFactory?->getType($type, 'jsonschema', $propertyMetadata->isReadableLink(), $serializerContext)) {
                 $propertySchema = $typeFromFactory;
@@ -230,14 +233,25 @@
                 break;
             }

-            if ($type->isNullable()) {
-                $propertySchema['anyOf'] = [['$ref' => $subSchema['$ref']], ['type' => 'null']];
-            } else {
-                $propertySchema['$ref'] = $subSchema['$ref'];
+            $isNullable = $isNullable ?? $type->isNullable();
+            $refs[$subSchema['$ref']] = '$ref';
+        }
+
+        if (\count($refs) > 1) {
+            $anyOf = [];
+            foreach (array_keys($refs) as $ref) {
+                $anyOf[] = ['$ref' => $ref];
+            }
+            $propertySchema['anyOf'] = $anyOf;
+
+            if ($isNullable) {
+                $propertySchema['anyOf'][] = ['type' => 'null'];
             }

             unset($propertySchema['type']);
-            break;
+        } elseif (\count($refs) === 1) {
+            $propertySchema['$ref'] = array_keys($refs)[0];
+            unset($propertySchema['type']);
         }

         $schema->getDefinitions()[$definitionName]['properties'][$normalizedPropertyName] = new \ArrayObject($propertySchema);

[soyuka]

Hi could you open a PR?

[GwendolenLynch]

Fixed in #6223

[GwendolenLynch]

@soyuka I've found more issues related to this that are probably worth fixing before the next release.

I've created a branch that I'll use to PR with two unit tests to illustrate the problem(s): https://github.com/GwendolenLynch/api-platform-core/tree/fix/issues/6212

These two will produce incorrect schema output:

    #[ApiProperty]
    public ?Book $reference;

    // The integer is stored in the database, and a state provider transforms the integer into the Species class
    #[ApiProperty]
    public int|Species|null $species = null;

                "species": {
                    "oneOf": [
                        {
                            "type": [
                                "string",
                                "null"
                            ],
                            "format": "iri-reference",
                            "example": "https://example.com/"
                        },
                        {
                            "type": [
                                "integer",
                                "null"
                            ]
                        }
                    ]
                },
                "reference": {
                    "type": [
                        "string",
                        "null"
                    ],
                    "format": "iri-reference",
                    "example": "https://example.com/"
                }

[soyuka]

Looks okay to me

[GwendolenLynch]

Looks okay to me

Me too! :face_with_spiral_eyes:

I was trying to "simplify" the use/test case that I had, but went too far … The "looking at the problem too long" thing again.

However, putting the serialization group back in shows the problem I am trying to illustrate:

    // The integer is stored in the database, and a state provider transforms the integer into the Species class
    #[ApiProperty]
    #[Groups(['read'])] // also applied to the properties in the Species class
    public int|Species|null $species = null;

Snippet of the output. Note the presence of unknown_type:

                "species": {
                    "oneOf": [
                        {
                            "type": [
                                "unknown_type",
                                "null"
                            ]
                        },
                        {
                            "type": [
                                "integer",
                                "null"
                            ]
                        }
                    ]
                }