zircote / swagger-php

A php swagger annotation and parsing library
http://zircote.github.io/swagger-php/
Apache License 2.0
5.1k stars 934 forks source link

Merging properties from non-schema parent should include traits #1325

Closed DerManoMann closed 2 years ago

DerManoMann commented 2 years ago

Given the following:

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

abstract class Model
{
    use HasId;
}

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

The ExpandClasses processor should merge not just direct properties properties from Model into Address, but also properties inherited from HasId.

Related to https://github.com/DarkaOnLine/L5-Swagger/issues/499

DerManoMann commented 2 years ago

@rickgoemans Not sure if you actually tried the PR orjust looked at the code changes - either way, I added a test that looks at both parent and own class traits and I believe things do work as expected in both cases...

rickgoemans commented 2 years ago

I've just tested it in another test project since I've already started implementing it my main project.

My setup is:

HasID (trait)

<?php

namespace App\Http\Swagger\Models;

use OpenApi\Annotations as OA;

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

HasTimestamps (trait)

<?php

namespace App\Http\Swagger\Models;

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;
}

HasSoftDelete (trait)

<?php

namespace App\Http\Swagger\Models;

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

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

Model (abstract model that is inherited)

<?php

namespace App\Http\Swagger\Models;

use App\Models\Model as BaseModel;
use OpenApi\Annotations as OA;

/**
 * @OA\Schema(
 *     description="This model can be ignored, it is just used for inheritance."
 * )
 *
 * @see BaseModel
 */
abstract class Model
{
    use HasId;
    use HasTimestamps;
}

Product (an actual model)

<?php

namespace App\Http\Swagger\Models;

use App\Models\Product as ProductModel;
use OpenApi\Annotations as OA;

/**
 * @OA\Schema (
 *     description="Product",
 *     required={
 *         "number",
 *         "name",
 *     },
 *     @OA\Xml(
 *         name="Product",
 *     ),
 * )
 *
 * @see ProductModel
 */
class Product extends Model
{
    use HasSoftDelete;

    /** @OA\Property */
    public string $number;

    /** @OA\Property */
    public string $name;

}

Here's the JSON result:

{
    "openapi": "3.0.0",
    "info": {
        "title": "Laravel Scout - API Documentation",
        "description": "Laravel Scout - API Documentation",
        "termsOfService": "https://swagger.io/terms",
        "contact": {
            "name": "Rick Goemans",
            "email": "<LEFT OUT OF DEMO>"
        },
        "license": {
            "name": "Apache 2.0",
            "url": "https://www.apache.org/licenses/LICENSE-2.0.html"
        },
        "version": "0.0.1"
    },
    "paths": {
            // LEFT OUT FOR FOR DEMO
        }
    },
    "components": {
        "schemas": {
            "Model": {
                "description": "This model can be ignored, it is just used for inheritance.",
                "properties": {
                    "id": {
                        "type": "integer",
                        "format": "int64",
                        "readOnly": true
                    },
                    "created_at": {
                        "type": "string",
                        "format": "date-time",
                        "readOnly": true
                    },
                    "updated_at": {
                        "type": "string",
                        "format": "date-time",
                        "readOnly": true
                    }
                },
                "type": "object"
            },
            "Product": {
                "description": "Product",
                "required": [
                    "number",
                    "name"
                ],
                "type": "object",
                "xml": {
                    "name": "Product"
                },
                "allOf": [
                    {
                        "$ref": "#/components/schemas/Model"
                    },
                    {
                        "properties": {
                            "number": {
                                "type": "string"
                            },
                            "name": {
                                "type": "string"
                            }
                        }
                    }
                ]
            }
        },
        "securitySchemes": {
            "passport": {
                "type": "oauth2",
                "description": "Laravel passport oauth2 security.",
                "in": "header",
                "scheme": "http",
                "flows": {
                    "password": {
                        "authorizationUrl": "http://localhost/oauth/authorize",
                        "tokenUrl": "http://localhost/oauth/token",
                        "refreshUrl": "http://localhost/token/refresh",
                        "scopes": []
                    }
                }
            }
        }
    },
    "externalDocs": {
        "description": "Find out more about Swagger",
        "url": "https://swagger.io"
    }
}

As you can see, the properties of HasSoftDelete are not shown.

DerManoMann commented 2 years ago

Phew! This got a bit bigger than expected.

I've updated the PR; includes some code cleanup and shuffling but your last example now passes for me :)

rickgoemans commented 2 years ago

Are you referring this PR https://github.com/zircote/swagger-php/pull/1333/files? I see it is merged but there is no release yet so I can't test it.

DerManoMann commented 2 years ago

Nah, #1331. You'd have to pull from the PR repo and use that version. Hmm, I guess I'll merge and create a release. Pretty sure this is it :)