DarkaOnLine / L5-Swagger

OpenApi or Swagger integration to Laravel
https://github.com/DarkaOnLine/L5-Swagger
MIT License
2.64k stars 394 forks source link

Properties in traits loaded in parent class are not generated #499

Closed rickgoemans closed 1 year ago

rickgoemans commented 2 years ago

Description:

I have a small setup for models that prevent me from writing duplicate properties, but when I add properties via a trait to the parent (abstract) model class, they are not generated in the children models.

Steps To Reproduce:

Make the following 3 files (of course correct the namespace to your situation);

<?php

namespace App\Http\Swagger\Models;

use OpenApi\Annotations as OA;

trait HasId
{
    /**
     * @OA\Property(
     *     format="int64",
     *     readOnly=true,
     * )
     *
     * @var int
     */
    public int $id;
}
<?php

namespace App\Http\Swagger\Models;

use OpenApi\Annotations as OA;

abstract class Model
{
    use HasId;
}
<?php

namespace App\Http\Swagger\Models;

use OpenApi\Annotations as OA;

/**
 * @OA\Schema(
 *     required={"street",},
 *     @OA\Xml(
 *         name="Address"
 *     )
 * )
 */
class Address extends Model
{
    /** @OA\Property */
    public string $street;
}

Here's the result of the JSON file:

"Address": {
    "required": [
        "street"
    ],
    "properties": {
        "street": {
            "type": "string"
        }
    },
    "type": "object",
    "xml": {
        "name": "Address"
    }
},

Here's the result of Swagger UI: Screenshot 2022-10-12 at 13 10 50

If you then include the HasId trait within the Address model, it will be shown correctly.

Of course I can move the id property to the Model class, but the general thought is that I have some properties that cannot be placed within just one file. Anybody else experiencing similar issues and/or maybe know a good solution? I can make a very complex class extending structure but that is not a realistic solution.

rickgoemans commented 2 years ago

For additional informatie, it does not matter wether the parent class is abstract or a normal class and wether the child models have properties (or other traits) or not.

DerManoMann commented 2 years ago

This is an issue with swagger-php. Its an edge case since neither Model nor HasId have a /** @OA\Schema() */ annotation. That means swagger-pph will try to merge annotations rather than inheritance, where you end up with something like


allOf:
        -
          $ref: '#/components/schemas/Model'
``
Looks like the merge code does not lookup properties recursively through used traits in the (not schema) parent class.

Looks like there is also a bug involved because even if you annotate both `HasId` and `Model` with `/** @OA\Schema() */` the `Model` class is missing a `allOf` ref to `HasId`.
rickgoemans commented 2 years ago

This is an issue with swagger-php. Its an edge case since neither Model nor HasId have a /** @OA\Schema() */ annotation. That means swagger-pph will try to merge annotations rather than inheritance, where you end up with something like

allOf:
        -
          $ref: '#/components/schemas/Model'
``
Looks like the merge code does not lookup properties recursively through used traits in the (not schema) parent class.

Looks like there is also a bug involved because even if you annotate both `HasId` and `Model` with `/** @OA\Schema() */` the `Model` class is missing a `allOf` ref to `HasId`.

Thanks for the clarification and opening an issue at the source repo! Let's see what they're suggesting or coming up with related to a fix.

DerManoMann commented 2 years ago

@rickgoemans There is a PR ready that addresses your issue: https://github.com/zircote/swagger-php/pull/1331

Would you be able to try this and let me know if this fixes things for you? It sure does fix your reproducer :)

rickgoemans commented 2 years ago

It is fixed, thanks for the update and the fix!

rickgoemans commented 2 years ago

I just noticed that if you use a trait in both the parent (in my example the abstract Model) and the children (in my example Address), the properties in the traits of ONLY the parent class are loaded.

rickgoemans commented 2 years ago

Since the title of this issue does not fully cover the new situation, should I create a new issue?

My new setup would include another trait called HasTimestamps which contains two properties (created_at and updated_at) and is used in the Address model/schema.

Here's the trait:

<?php

namespace ...

use Carbon\Carbon;
use OpenApi\Annotations as OA;

trait HasTimestamps
{
    /**
     * @OA\Property(
     *     format="date-time",
     *     type="string",
     *     readOnly=true,
     * )
     */
    public Carbon $created_at;

    /**
     * @OA\Property(
     *     format="date-time",
     *     type="string",
     *     readOnly=true,
     * )
     */
    public Carbon $updated_at;
}
DerManoMann commented 2 years ago

Not a new issue, but since this is purely swagger-php related we should move this over to https://github.com/zircote/swagger-php/issues/1325 and close this one.