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

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

Closed
rickgoemans opened this issue Oct 12, 2022 · 8 comments
Closed

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

rickgoemans opened this issue Oct 12, 2022 · 8 comments

Comments

@rickgoemans
Copy link

rickgoemans commented Oct 12, 2022

  • L5-Swagger Version: 8.4.1 (composer show | grep l5-swagger)
  • Laravel version: 9.34.0
  • PHP Version (php -v): 8.1.10
  • OS: Mac OS

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
Copy link
Author

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
Copy link

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
Copy link
Author

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
Copy link

@rickgoemans There is a PR ready that addresses your issue: zircote/swagger-php#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
Copy link
Author

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

@rickgoemans
Copy link
Author

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
Copy link
Author

rickgoemans commented Oct 19, 2022

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
Copy link

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

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

No branches or pull requests

3 participants