zircote/swagger-php

Possible BC Break in 4.10.1

bastianschwarz opened this issue · 4 comments

Hey There,

our CI jus failed for Dependabot with the latest release. Since it's a patch it should not happen.

image

I will investigate as soon as I have the time, will take couple of days as I'm chasing deadline. Just that you are aware of the possibility. :)

Hmm, nothing in that particular release that I could think of. From what version where you upgrading from?

I'm updating from 4.10.0

I saw the 4.10.2 release and tried that but same error. I might have some time today to take a closer look.

Ok, I played around a bit. It was something I struggled to get working back when I built it and got it to work with a workaround. Now that seems to break most likely to something very unrelated.

I have a custom path parameter property like this:

#[Attribute]
#[PathParameter(
    parameter: 'uuid',
    name: 'uuid',
    description: 'The UUID of the entity',
    required: true,
    schema: new Schema(ref: Uuid4::class),
)]
final class UuidPathParameter extends PathParameter
{

    public function __construct(
        string $name = 'uuid',
        string $description = 'The UUID of the entity',
        Schema $schema = new Schema(ref: Uuid4::class),
    )
    {
        parent::__construct(
            name: $name,
            description: $description,
            required: true,
            ref: '#/components/parameters/uuid',
            schema: $schema,
        );
    }
}

I can't rememeber why I added the PathParameter attribute to the class itself but I think it was because otherwise it wouldn't show up.

I use it in my controller:

    public function book(
        #[UuidPathParameter] Uuid $uuid,
        Request $request,
        InquiryLookupInterface $inquiryLookup,
        BatchAwareActivityLoggerInterface $activityLogger,
        WithBookingRequestFacadeInterface $withBookingRequestFacade,
        ClockInterface $clock,
    ): JsonSerializable
    {

This renders or rather rendered correctly the way I want it to

image

I also remember adding the ref as string was the only way to make it behave.

Now ... for the parameter. When I add it to the parent constructor the generator complains about the already existing name. Makes sense. But when I remove the PathParameter from the Attribute class, I get an empty parameter

#[Attribute]
final class UuidPathParameter extends PathParameter
{

    public function __construct(
        string $name = 'uuid',
        string $description = 'The UUID of the entity',
        Schema $schema = new Schema(ref: Uuid4::class),
    )
    {
        parent::__construct(
            parameter: 'uuid',
            name: $name,
            description: $description,
            required: true,
            ref: '#/components/parameters/uuid',
            schema: $schema,
        );
    }
}

image

Also kind of makes sense because there's nothing to ref anymore.

When I remove the ref and parameter again and leave the attribute from class as well it works as intented which is what I originally tried to do.

#[Attribute]
final class UuidPathParameter extends PathParameter
{

    public function __construct(
        string $name = 'uuid',
        string $description = 'The UUID of the entity',
        Schema $schema = new Schema(ref: Uuid4::class),
    )
    {
        parent::__construct(
            name: $name,
            description: $description,
            required: true,
            schema: $schema,
        );
    }
}

So bottom line:

I guess you fixed the original problem for something unrelated, I can now use it without the workaround ... so all good 😆

Still not great it happened on a patch version but hey, that's what CI is for right? 😉

So I guess I'm closing this.

By the way: Thanks for doing this. I spent quite some time working through different OpenAPI/Swagger packages and this is by far the best one.

Thanks. Sounds like quite the process; glad you found a solution.