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

Problem with scan and apply custom (extended) attributes #1416

Open
roquie opened this issue Feb 19, 2023 · 15 comments
Open

Problem with scan and apply custom (extended) attributes #1416

roquie opened this issue Feb 19, 2023 · 15 comments
Labels

Comments

@roquie
Copy link

roquie commented Feb 19, 2023

I want to extend default attributes and then use it in the project. I do this like:

# example of a collection attribute
namespace App\Common\Interfaces\Http\Documentation\Property;

use OpenApi\Attributes as OA;
use Str;

#[\Attribute(
    \Attribute::TARGET_METHOD |
    \Attribute::TARGET_PROPERTY |
    \Attribute::TARGET_PARAMETER |
    \Attribute::TARGET_CLASS_CONSTANT |
    \Attribute::IS_REPEATABLE
)]
class Collection extends OA\Property
{
    /** @param class-string $of */
    public function __construct(
        string $of,
        ?string $description = null
    ) {
        $name = Str\uniqueClassShortName($of);

        parent::__construct(
            title: $name,
            description: $description,
            items: new OA\Items(ref: "#/components/schemas/$name")
        );
    }
}
# example of an attribute for typed property item
namespace App\Common\Interfaces\Http\Documentation\Property;

use OpenApi\Attributes as OA;
use Str;

#[\Attribute(
    \Attribute::TARGET_METHOD |
    \Attribute::TARGET_PROPERTY |
    \Attribute::TARGET_PARAMETER |
    \Attribute::TARGET_CLASS_CONSTANT |
    \Attribute::IS_REPEATABLE
)]
class Item extends OA\Property
{
    /** @param class-string $of */
    public function __construct(
        string $of,
        ?string $description = null
    ) {
        $name = Str\uniqueClassShortName($of);

        parent::__construct(
            ref: "#/components/schemas/$name",
            title: $name,
            description: $description,
        );
    }
}
# Example schema attribute
namespace App\Common\Interfaces\Http\Documentation;

use OpenApi\Attributes as OA;
use Str;

#[\Attribute(\Attribute::TARGET_CLASS | \Attribute::TARGET_METHOD | \Attribute::TARGET_PROPERTY | \Attribute::IS_REPEATABLE)]
class Schema extends OA\Schema
{
    /**
     * @param class-string $of
     * @param string|null $description
     * @param array $optional
     * @param int|null $minLength
     * @param int|null $maxLength
     * @throws \ReflectionException
     */
    public function __construct(
        string $of,
        ?string $description = null,
        array $optional = [],
        ?int $minLength = null,
        ?int $maxLength = null,
    ) {
        $name = Str\uniqueClassShortName($of);
        $class = new \ReflectionClass($of);

        $required = null;
        foreach ($class->getProperties(\ReflectionProperty::IS_PUBLIC) as $property) {
            if (\in_array($property->getName(), $optional, true)) {
                continue;
            }

            $required[] = $property->getName();
        }

        parent::__construct(
            schema: $name,
            title: $class->getShortName(),
            description: $description,
            required: $required,
            maxLength: $maxLength,
            minLength: $minLength
        );
    }
}

When I using my attributes I got an "silent error" with nesting DTO classes. DTO structure be like:

#[Schema(of: TargetGroupListDto::class)]
final class TargetGroupListDto
{
    public function __construct(
        /** @var TargetGroupDto[] */
        #[Collection(of: TargetGroupDto::class)]
        public readonly array $targetGroups = []
    ) {}
}

#[Schema(of: TargetGroupDto::class)]
final class TargetGroupDto
{
    public function __construct(
        #[OA\Property] # with my custom attribute #[Item] I also had problems
        public readonly string $groupId,

        #[OA\Property] # with my custom attribute #[Item] I also had problems
        public readonly string $groupName,

        /** @var TargetDto[] */
        #[Collection(of: TargetDto::class)]
//        #[OA\Property( # WORKS
//            items: new OA\Items(ref: '#/components/schemas/TargetDto')
//        )]
        public readonly array $targets = []
    ) {}
}

#[Schema(of: TargetDto::class)]
final class TargetDto
{
    public function __construct(
        #[Item(of: TargetId::class)]
        public readonly string $targetId,
        #[Item(of: TargetType::class)]
        public readonly string $targetType,
        // ...
    ) {}
}

... and for this structure library generate following openapi schema:

{
    "openapi": "3.0.0",
    "info": {
        "title": "API",
        "version": "1.0.0"
    },
    "paths": {
        "/targetGroups": {
            "get": {
                "tags": [
                    "TG"
                ],
                "summary": "List tgs",
                "operationId": "df939917fe568c1cdb0d755db41c5616",
                "responses": {
                    "200": {
                        "description": "Successful response of [TargetGroupListDto]",
                        "content": {
                            "application/json": {
                                "schema": {
                                    "$ref": "#/components/schemas/TargetGroupListDto"
                                }
                            }
                        }
                    }
                }
            }
        }
    },
    "components": {
        "schemas": {
            "TargetDto": {
                "title": "TargetDto",
                "required": [
                    "targetId",
                    "targetType",
                    "targetDestination"
                ],
                "properties": {
                    "targetId": {
                        "$ref": "#/components/schemas/TargetId"
                    },
                    "targetType": {
                        "$ref": "#/components/schemas/TargetType"
                    }
                },
                "type": "object"
            },
            "TargetGroupDto": {
                "title": "TargetGroupDto",
                "required": [
                    "groupId",
                    "groupName",
                    "targets"
                ] # ITEMS NOT EXISTS, #Collection() attribute NOT works fine
            },
            "TargetGroupListDto": {
                "title": "TargetGroupListDto",
                "required": [
                    "targetGroups"
                ],
                "properties": {
                    "targetGroups": {
                        "title": "TargetGroupDto",
                        "type": "array",
                        "items": { # ITEMS EXISTS, #Collection() attribute works fine
                            "$ref": "#/components/schemas/TargetGroupDto"
                        },
                        "nullable": false
                    }
                },
                "type": "object"
            },
            "TargetId": {
                "title": "TargetId"
            },
            "TargetType": {
                "title": "TargetType"
            }
        }
    }
}

Library do not generate for me nested array items if I using my custom #[Collection()] attribute. If I comment my attribute and using #[OA\Property(items: ...)] directly it works fine. But my attribute is a pure-simple and this behavior is unexpected...

@DerManoMann
Copy link
Collaborator

Changes seem reasonable, in particular if the (roughly) same works for other custom attributes.

Would you be able to compress som eof the code into a single file example that reproduces the issue? Multiple classes in the file would be fine, this is just to keep it simple (and perhaps add to the test suite)

@DerManoMann
Copy link
Collaborator

@roquie I've created a PR (#1423) 1423that adds a scratch test that tries to reproduce your custom attributes.

The result so far is looking good to me. If you have a little time to try to add to that so it breaks that would be most helpful.

Without a simple reproducable testcase there isn't a lot I can (or am willing) to do.

@DerManoMann
Copy link
Collaborator

In fact, 4.7.3 has a related change that might also be fixing your problem.

@roquie
Copy link
Author

roquie commented Mar 8, 2023

No, latest release is not fixing this problem. May be problem with scan files feature?

@DerManoMann
Copy link
Collaborator

Ok, back to square one. Would you be able to create a single file repoducer to help debugging?

@roquie
Copy link
Author

roquie commented Mar 15, 2023

Sorry, I'm late...

Filename: test-openapi3-attr.php

<?php

/**
 * @license Apache 2.0
 */

declare(strict_types=1);

require __DIR__ . '/vendor/autoload.php';

use OpenApi\Attributes as OA;
use OpenApi\Generator;

#[OA\Info(version: '1.0.0', title: 'API')]
class App {}

#[\Attribute(
    \Attribute::TARGET_CLASS |
    \Attribute::TARGET_METHOD |
    \Attribute::TARGET_PROPERTY |
    \Attribute::IS_REPEATABLE
)]
class Schema extends OA\Schema
{
    /**
     * @param class-string $of
     * @param string|null $description
     * @param array $optional
     * @param int|null $minLength
     * @param int|null $maxLength
     * @throws \ReflectionException
     */
    public function __construct(
        string $of,
        ?string $description = null,
        array $optional = [],
        ?int $minLength = null,
        ?int $maxLength = null,
    ) {
        $class = new \ReflectionClass($of);

        $required = null;
        foreach ($class->getProperties(\ReflectionProperty::IS_PUBLIC) as $property) {
            if (\in_array($property->getName(), $optional, true)) {
                continue;
            }

            $required[] = $property->getName();
        }

        parent::__construct(
            schema: $of,
            title: $class->getShortName(),
            description: $description,
            required: $required,
            maxLength: $maxLength,
            minLength: $minLength
        );
    }
}



#[\Attribute(
    \Attribute::TARGET_METHOD |
    \Attribute::TARGET_PROPERTY |
    \Attribute::TARGET_PARAMETER |
    \Attribute::TARGET_CLASS_CONSTANT |
    \Attribute::IS_REPEATABLE
)]
class Collection extends OA\Property
{
    /** @param class-string $of */
    public function __construct(
        string $of,
        ?string $description = null
    ) {
        parent::__construct(
            title: $of,
            description: $description,
            items: new OA\Items(ref: "#/components/schemas/$of")
        );
    }
}

#[\Attribute(
    \Attribute::TARGET_METHOD |
    \Attribute::TARGET_PROPERTY |
    \Attribute::TARGET_PARAMETER |
    \Attribute::TARGET_CLASS_CONSTANT |
    \Attribute::IS_REPEATABLE
)]
class Item extends OA\Property
{
    /** @param class-string $of */
    public function __construct(
        string $of,
        ?string $description = null
    ) {
        parent::__construct(
            ref: "#/components/schemas/$of",
            title: $of,
            description: $description,
        );
    }
}


#[\Attribute(
    \Attribute::TARGET_METHOD |
    \Attribute::TARGET_PROPERTY |
    \Attribute::TARGET_PARAMETER |
    \Attribute::TARGET_CLASS_CONSTANT |
    \Attribute::IS_REPEATABLE
)]
class Raw extends OA\Property
{
    public function __construct(
        ?string $title = null,
        ?string $description = null
    ) {
        parent::__construct(
            title: $title,
            description: $description,
        );
    }
}


#[\Attribute(\Attribute::TARGET_CLASS | \Attribute::TARGET_METHOD | \Attribute::IS_REPEATABLE)]
class Successful extends OA\Response
{
    /** @param ?class-string $of */
    public function __construct(
        ?string $of = null,
    ) {
        if ($of === null) {
            parent::__construct(
                response: 200,
                description: 'Operation complete'
            );

            return;
        }

        parent::__construct(
            response: 200,
            description: "Successful response of [$of]",
            content: new OA\JsonContent(
                ref: "#/components/schemas/$of"
            )
        );
    }
}


final class TargetGroupController
{
    #[
        OA\Get(path: '/target_groups', summary: 'List target groups', tags: ['Target groups']),
        Successful(of: TargetGroupListDto::class)
    ]
    public function list(): string {}
}

#[Schema(of: TargetGroupListDto::class)]
final class TargetGroupListDto
{
    public function __construct(
        /** @var TargetGroupDto[] */
        #[Collection(of: TargetGroupDto::class)]
        public readonly array $targetGroups = []
    ) {}
}

#[Schema(of: TargetGroupDto::class)]
final class TargetGroupDto
{
    public function __construct(
        #[OA\Property] # with my custom attribute #[Item] I also had problems
        public readonly string $groupId,

        #[OA\Property] # with my custom attribute #[Item] I also had problems
        public readonly string $groupName,

        /** @var TargetDto[] */
        #[Collection(of: TargetDto::class)]
//        #[OA\Property( # WORKS
//            items: new OA\Items(ref: '#/components/schemas/TargetDto')
//        )]
        public readonly array $targets = []
    ) {}
}

#[Schema(of: TargetDto::class)]
final class TargetDto
{
    public function __construct(
        #[Item(of: TargetId::class)]
        public readonly string $targetId,
        #[Item(of: TargetType::class)]
        public readonly string $targetType,
        // ...
    ) {}
}

#[Schema(of: TargetId::class)]
class TargetId {}

#[Schema(of: TargetType::class)]
class TargetType {}


$generator = Generator::scan([
    __DIR__ . '/test-openapi3-attr.php',
]);

echo $generator->toYaml();

Screenshot 2023-03-15 at 20 58 31

Screenshot 2023-03-15 at 20 59 57

P.S. For main problem you can uncomment 184-186 lines

@roquie
Copy link
Author

roquie commented Mar 15, 2023

PHP 8.2/8.1

name     : zircote/swagger-php
descrip. : swagger-php - Generate interactive documentation for your RESTful API using phpdoc annotations
keywords : api, json, rest, service discovery
versions : * 4.7.3
type     : library
license  : Apache License 2.0 (Apache-2.0) (OSI approved) https://spdx.org/licenses/Apache-2.0.html#licenseText
homepage : https://github.com/zircote/swagger-php/
source   : [git] https://github.com/zircote/swagger-php.git 0299edc47eb4813a2c598a0348eaf205704adc92
dist     : [zip] https://api.github.com/repos/zircote/swagger-php/zipball/0299edc47eb4813a2c598a0348eaf205704adc92 0299edc47eb4813a2c598a0348eaf205704adc92
path     : /Users/roquie/google_drive/projects/spacetab-io/uptimemaster/backend/vendor/zircote/swagger-php
names    : zircote/swagger-php

support
issues : https://github.com/zircote/swagger-php/issues
source : https://github.com/zircote/swagger-php/tree/4.7.3

autoload
psr-4
OpenApi\ => src

requires
doctrine/annotations ^1.7 || ^2.0
ext-json *
php >=7.2
psr/log ^1.1 || ^2.0 || ^3.0
symfony/deprecation-contracts ^2 || ^3
symfony/finder >=2.2
symfony/yaml >=3.3

requires (dev)
composer/package-versions-deprecated ^1.11
friendsofphp/php-cs-fixer ^2.17 || ^3.0
phpstan/phpstan ^1.6
phpunit/phpunit >=8
vimeo/psalm ^4.23

@DerManoMann
Copy link
Collaborator

Sweet, thanks for that.

@DerManoMann
Copy link
Collaborator

Question - is it ok to add your sample code as test case to the project? It would get the same Apache license header ideally....

/**
 * @license Apache 2.0
 */

@roquie
Copy link
Author

roquie commented Mar 16, 2023

Of course, it’s not a problem

@DerManoMann
Copy link
Collaborator

@roquie #1431 should be what you need 🤞

If you could quickly test the branch before I merge that would be nice, although I'll merge in any case I think as this should fix some more subtle issues with derived annotations/attributes.

@roquie
Copy link
Author

roquie commented Mar 17, 2023

Yeah! It works! Tested right now.

@roquie
Copy link
Author

roquie commented Mar 17, 2023

FR: Can you add option to generator to choose enum type values for spec? I can create a new issue for that.

Now, library uses case keys:
Screenshot 2023-03-17 at 11 49 01

use App\Common\Interfaces\Http\Documentation\Schema;

#[Schema(of: HttpMethodType::class)]
enum HttpMethodType: string
{
    case Head = 'HEAD';
    case Get = 'GET';
    case Post = 'POST';
    case Put = 'PUT';
    case Patch = 'PATCH';
    case Delete = 'DELETE';
    case Options = 'OPTIONS';
}

but may be developer wants to use enum case values, like: HEAD, GET ... etc. What you think about this feature?

@DerManoMann
Copy link
Collaborator

Have a look here: https://zircote.github.io/swagger-php/guide/common-techniques.html#enum-cases-as-value

Setting the type explicitly to string should do what you want

@roquie
Copy link
Author

roquie commented Mar 17, 2023

type: 'string' helps me, thx :)

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