Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug]: Generated open api spec is unpredictable and often includes null and undefined even if not true #2469

Open
crtl opened this issue Apr 1, 2025 · 4 comments
Labels

Comments

@crtl
Copy link

crtl commented Apr 1, 2025

Version

4.36.1

Description

It is very unpredictable what types are gnerated for properties and how, for example given the following class:

class Positioning extends AbstractEntity {
  protected BusinessModel $businessModel;

  public function __construct(?BusinessModel $businessModel) {
   $this->businessModel = $businessModel ?? new BusinessModel($this);
  }

  public function getBusinessModel(): BusinessModel {
    return $this->businessModel;
  }
}

Will produce the following type in open api json:

"Positioning": {
    "required": [
        "guid",
    ],
    "properties": {
        "businessModel": {
            "oneOf": [
                {
                    "$ref": "#/components/schemas/PositioningBusinessModel"
                }
            ],
            "nullable": true,
            "title": "Business Model",
            "default": null
        },
    ]
}

But there is no reference to null anywhere in its type. So why are all the properties nullable and also why are they not required.
I have to manualy add each property to OA\Schema required list.
Instead it should be the other way around, properties are normally blacklisted.

Edit

AbstractPositioningModel

<?php

namespace App\Entity\Positioning;

use App\Entity\Positioning;
use Symfony\Component\Serializer\Attribute\Ignore;

abstract class AbstractPositioningModel
{

    #[Ignore]
    protected Positioning $positioning;


    protected function getPositioningValuesForProgress(): array
    {
        return [];
    }

    public function __construct(Positioning $positioning)
    {
        $this->positioning = $positioning;
    }

    /**
     * Checks if model is complete (properties are not null and arrays non-empty)
     * @return bool
     */
    public function isComplete(): bool
    {
        return $this->getProgress() === 1.0;
    }

    /**
     * Returns the progress for this model between 0 and 1 (complete)
     * @return float
     */
    public function getProgress(): float
    {
return 1.0;
    }

    public function setPositioning(Positioning $positioning): self
    {
        $this->positioning = $positioning;
        return $this;
    }

    public function getPositioning(): Positioning
    {
        return $this->positioning;
    }

}

BusinessModel

<?php

namespace App\Entity\Positioning;

use Doctrine\ORM\Mapping\Embeddable;
use OpenApi\Attributes as OA;
use Symfony\Component\Serializer\Attribute\SerializedName;

#[OA\Schema(
    required: [
        "progress",
        "complete"
    ]
)]
class BusinessModel extends AbstractPositioningModel
{

    /**
     *
     * @return float
     */
    #[SerializedName("progress")]
    public function getProgress(): float
    {
        // ....
return 1.0;
    }


}
@crtl crtl added the bug label Apr 1, 2025
@mario-fehr
Copy link

mario-fehr commented Apr 1, 2025

@crtl could you update the example with all classes like businessModel and PositioningBusinessModel please?

@crtl
Copy link
Author

crtl commented Apr 1, 2025

@mario-fehr I have added the BusinessModel class and its parent AbstractPositonioningModel class, but shouldnt be this irrelevant? Those are implementation details shouldnt the type in Positioning be the only thing that matters?
I also have several other embeddables which are also documented as nullable.

@mario-fehr
Copy link

mario-fehr commented Apr 2, 2025

@crtl sorry it was late at night yesterday and my brain was stoping to work properly. You are right does not matter much.

I created a reproducer for you with

  • symfony 7.2
  • NelmioApiDocBundle 5
  • Type Info 7.2 (tested with and without)

Only thing I changed was I removed the circular reference because of this error:

Circular reference detected for service "App\Response\BusinessModel", path: "App\Response\BusinessModel -> App\Response\Positioning -> App\Response\BusinessModel".

And could not reproduce your situation. You can finde the api.json in docs/api.json. The only way was lucky to get the same result was changing the constructore to this:

public function __construct(?BusinessModel $businessModel = null){
        $this->businessModel = $businessModel ?? new BusinessModel();
}

Maybe you could give us a bit more info about what versions you are using.

@crtl
Copy link
Author

crtl commented Apr 10, 2025

@mario-fehr sry for the late reply.

Ive created a reproduction here crtl/nelmioApiDocBundle-reproducer

it seems the problem is that doctrine mapping metadata and class constructor signatures override the type.

I have created several entities in the demo:

  • TestEntity Shows how nullable constructor args will result in nullable OA property despite constructor impl. making sure its non-nullable
  • MappedEntity showing how doctrine mappings affect field types and also constructor nullable issues

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants