nelmio / NelmioApiDocBundle

Generates documentation for your REST API from annotations
MIT License
2.22k stars 833 forks source link

Duplicate schemas when referencing with `Attributes\Model` #2218

Open oriolvinals opened 7 months ago

oriolvinals commented 7 months ago

I have an entity called Contact with a serialization group interface named ContactAll

<?php

namespace App\Entity;

use Doctrine\ORM\Mapping as ORM;
use Symfony\Component\Serializer\Annotation as Serializer;

#[ORM\Entity]
class Contact 
{
      #[ORM\Id]
      #[ORM\GeneratedValue]
      #[ORM\Column(type: 'integer', options: ['unsigned' => true])]
      #[Serializer\Groups([SerializationGroup::ContactAll])]
      private ?int $id = null;

      // Other variables, getters and setters...
}

On Controller, I use the open api attributes with model from Nelmio\ApiDocBundle\Annotation\Model

<?php

namespace App\Controller;

use OpenApi\Attributes as OA;

class ContactController extends BaseController 
{
   #[OA\Response(
       response: 200, 
       description: 'Success',
       content: new OA\JsonContent(
           type: 'array', 
           items: OA\Items(ref: new Model(Contact::class, groups: [SerializationGroup::ContactAll]))
       )
    )]
   #[Route(...)]
   public function list(){}

   #[OA\Response(
       response: 200, 
       description: 'Success',
       content: new OA\JsonContent(
           ref: new Model(Contact::class, groups: [SerializationGroup::ContactAll])
       )
    )]
   #[Route(...)]
   public function show(){}

   #[OA\Response(
       response: 201, 
       description: 'Success',
       content: new OA\JsonContent(
           ref: new Model(Contact::class, groups: [SerializationGroup::ContactAll])
       )
    )]
   #[Route(...)]
   public function create(){}

   #[OA\Response(
       response: 200, 
       description: 'Success',
       content: new OA\JsonContent(
           ref: new Model(Contact::class, groups: [SerializationGroup::ContactAll])
       )
    )]
   #[Route(...)]
   public function update(){}
}

When I go to the OpenApi schemas I have 4 same schemas with names Contact, Contact2, Contact3 and Contact4.

There's a way to not duplicate schemas with the same serialization groups?

DjordyKoert commented 7 months ago

I cannot seem to reproduce it. I do see a few typo's in your attributes though.

Incorrect Model usage

 new Model(Contact::class, groups: [SerializationGroup::ContactAll])

Should be

 new Model(type: Contact::class, groups: [SerializationGroup::ContactAll])

Missing new on OA\Items

items: OA\Items(ref: new Model(Contact::class, groups: [SerializationGroup::ContactAll]))

Should be

items: new OA\Items(ref: new Model(type: Contact::class, groups: [SerializationGroup::ContactAll]))
oriolvinals commented 7 months ago

@DjordyKoert sorry for the mistakes, I made them without looking at the code, but this is not the problem I really have.

I'm going to start over, since it's harder to reproduce.

I have 3 entities: Member, Contact i Event

Member

<?php

namespace App\Entity;

use Doctrine\ORM\Mapping as ORM;
use Symfony\Component\Serializer\Annotation as Serializer;

#[ORM\Entity]
class Member 
{
      #[ORM\Id]
      #[ORM\GeneratedValue]
      #[ORM\Column(type: 'integer', options: ['unsigned' => true])]
      #[Serializer\Groups([SerializationGroup::MemberAll])]
      private ?int $id = null;

      // Other variables, constructor, getters and setters...
}

Contact

<?php

namespace App\Entity;

use Doctrine\ORM\Mapping as ORM;
use Symfony\Component\Serializer\Annotation as Serializer;

#[ORM\Entity]
class Contact 
{
      #[ORM\Id]
      #[ORM\GeneratedValue]
      #[ORM\Column(type: 'integer', options: ['unsigned' => true])]
      #[Serializer\Groups([SerializationGroup::ContactAll])]
      private ?int $id = null;

      #[ORM\ManyToOne(targetEntity: Member::class)]
      #[Serializer\Ignore]
      private Member $member;

      // Other variables, constructor, getters and setters...
}

Event

<?php

namespace App\Entity;

use Doctrine\ORM\Mapping as ORM;
use Symfony\Component\Serializer\Annotation as Serializer;

#[ORM\Entity]
class Event 
{
      #[ORM\Id]
      #[ORM\GeneratedValue]
      #[ORM\Column(type: 'integer', options: ['unsigned' => true])]
      #[Serializer\Groups([SerializationGroup::EventAll])]
      private ?int $id = null;

      #[ORM\ManyToOne(targetEntity: Member::class)]
      #[Serializer\Ignore]
      private Member $member;

      // Other variables, constructor, getters and setters...
}

Now i create 2 Outputs called ContactOutput i EventOutput with another serialization groups

ContactOutput

<?php

namespace App\Dto\Output;

use App\Entity\Contact;
use App\Entity\Member;
use App\Serialization\Groups\SerializationGroup;
use Symfony\Component\Serializer\Annotation as Serializer;

class ContactOutput
{
    #[Serializer\Groups([SerializationGroup::ContactOutputAll])]
    private readonly Contact $contact;

    #[Serializer\Groups([SerializationGroup::ContactOutputAll])]
    private readonly Member $member;

    public function __construct(Contact $contact, Member $member)
    {
        $this->contact = $contact;
    }

    public function getContact(): Contact
    {
        return $this->contact;
    }

    public function getMember(): Member
    {
        return $this->member;
    }
}

EventOutput

<?php

namespace App\Dto\Output;

use App\Entity\Event;
use App\Entity\Member;
use App\Serialization\Groups\SerializationGroup;
use Symfony\Component\Serializer\Annotation as Serializer;

class EventOutput
{
    #[Serializer\Groups([SerializationGroup::EventOutputAll])]
    private readonly Event $event;

    #[Serializer\Groups([SerializationGroup::EventOutputAll])]
    private readonly Member $member;

    public function __construct(Event $event, Member $member)
    {
        $this->event = $event;
    }

    public function getEventt(): Event
    {
        return $this->event;
    }

    public function getMember(): Member
    {
        return $this->member;
    }
}

Now I want to use this outputs on the contact and event controllers

<?php

namespace App\Controller;

use OpenApi\Attributes as OA;

class ContactController extends BaseController 
{
   #[OA\Response(
       response: 200, 
       description: 'Success',
       content: new OA\JsonContent(
           type: 'array', 
           items: new OA\Items(ref: new Model(type: ContactOutput::class, groups: [
               SerializationGroup::ContactOutputAll,
               SerializationGroup::ContactAll,
               SerializationGroup::MemberAll
           ]))
       )
    )]
   #[Route(...)]
   public function list(){}

   #[OA\Response(
       response: 200, 
       description: 'Success',
       content: new OA\JsonContent(
           ref: new Model(type: ContactOutput::class, groups: [
               SerializationGroup::ContactOutputAll,
               SerializationGroup::ContactAll,
               SerializationGroup::MemberAll
           ])
       )
    )]
   #[Route(...)]
   public function show(){}

   #[OA\Response(
       response: 201, 
       description: 'Success',
       content: new OA\JsonContent(
           ref: new Model(type: ContactOutput::class, groups: [
               SerializationGroup::ContactOutputAll,
               SerializationGroup::ContactAll,
               SerializationGroup::MemberAll
           ])
       )
    )]
   #[Route(...)]
   public function create(){}

   #[OA\Response(
       response: 200, 
       description: 'Success',
       content: new OA\JsonContent(
           ref: new Model(type: ContactOutput::class, groups: [
               SerializationGroup::ContactOutputAll,
               SerializationGroup::ContactAll,
               SerializationGroup::MemberAll
           ])
       )
    )]
   #[Route(...)]
   public function update(){}
}
<?php

namespace App\Controller;

use OpenApi\Attributes as OA;

class EventController extends BaseController 
{
   #[OA\Response(
       response: 200, 
       description: 'Success',
       content: new OA\JsonContent(
           type: 'array', 
           items: new OA\Items(ref: new Model(type: EventOutput::class, groups: [
               SerializationGroup::EventOutputAll,
               SerializationGroup::EventAll,
               SerializationGroup::MemberAll
           ]))
       )
    )]
   #[Route(...)]
   public function list(){}

   #[OA\Response(
       response: 200, 
       description: 'Success',
       content: new OA\JsonContent(
           ref: new Model(type: EventOutput::class, groups: [
               SerializationGroup::EventOutputAll,
               SerializationGroup::EventAll,
               SerializationGroup::MemberAll
           ])
       )
    )]
   #[Route(...)]
   public function show(){}

   #[OA\Response(
       response: 201, 
       description: 'Success',
       content: new OA\JsonContent(
           ref: new Model(type: EventOutput::class, groups: [
               SerializationGroup::EventOutputAll,
               SerializationGroup::EventAll,
               SerializationGroup::MemberAll
           ])
       )
    )]
   #[Route(...)]
   public function create(){}

   #[OA\Response(
       response: 200, 
       description: 'Success',
       content: new OA\JsonContent(
           ref: new Model(type: EventOutput::class, groups: [
               SerializationGroup::EventOutputAll,
               SerializationGroup::EventAll,
               SerializationGroup::MemberAll
           ])
       )
    )]
   #[Route(...)]
   public function update(){}
}

Now in schemas there's a duplicated schemas image

emhovis commented 7 months ago

I have been dealing with the same issue- also using a response object to wraps my entities. I just assumed it was just incorrect usage somewhere else on my end. Coincidentally, this was the root cause for me writing up #2099

image

DjordyKoert commented 7 months ago

Ah I see why this is happening at least. This happens because Member class is used in both the ContactOutput & EventOutput. Both of these "output" classes get different serialization groups applied: 1.SerializationGroup::ContactOutputAll, SerializationGroup::ContactAll & SerializationGroup::MemberAll

  1. SerializationGroup::EventOutputAll, SerializationGroup::EventAll & SerializationGroup::MemberAll

These groups also get passed to the child classes (the Member class in this example). Technically speaking the different groups used could result in a different schema for the Member class, which is the reason why you get a Member & Member2 schema. It just so happens that both of these Schema's are the exact same. https://github.com/DjordyKoert/NelmioApiDocBundle/blob/52b297059b2ccf4bf1db3c8e80b5234f672fe4af/Tests/Functional/Fixtures/Issue2218.json#L216

You can see the setup that I used to replicate it in https://github.com/nelmio/NelmioApiDocBundle/pull/2221. I am not yet sure what the proper way would be to change this behaviour or if this should stay as it is.

mymain commented 7 months ago

@DjordyKoert Same issue on my side.

aprat84 commented 6 months ago

@DjordyKoert Maybe an approach would be to only consider the groups that actually affect the model for creating the hash, and therefore unique models.

In the example above, even though we are using 3 groups, the Contact submodel is actually only affected by one of them.

Of course we still need to keep track of the root groups for nested models down the serialization tree.

I tried to look at the code, but it won't be an easy task and sadly now I don't have the time :(

DjordyKoert commented 6 months ago

Maybe creating an optional swagger-php processor which removes duplicate schemas is an option?

spasat commented 3 weeks ago

Maybe there is a possibility to make a hash from the schema fingerprint not from the type and context https://github.com/nelmio/NelmioApiDocBundle/blob/master/src/Model/Model.php#L68

vpshvd commented 3 weeks ago

I got rid of the extensive logging by adding the aliases to the config

    models:
        use_jms: false
        names:
            - { alias: AddressDTO_Read, type: App\DTO\AddressDTO, groups: [ user:read ] }
            - { alias: AddressDTO_Sign, type: App\DTO\AddressDTO, groups: [ user:sign ] }
            - { alias: AddressDTO_SignResponse, type: App\DTO\AddressDTO, groups: [ user:sign:response ] }
            - { alias: AddressDTO_Write, type: App\DTO\AddressDTO, groups: [ user:write ] }

Obviously, this doesn't solve the problem of the bundle creating extra schemes in the doc though

oriolvinals commented 2 weeks ago

I got rid of the extensive logging by adding the aliases to the config

    models:
        use_jms: false
        names:
            - { alias: AddressDTO_Read, type: App\DTO\AddressDTO, groups: [ user:read ] }
            - { alias: AddressDTO_Sign, type: App\DTO\AddressDTO, groups: [ user:sign ] }
            - { alias: AddressDTO_SignResponse, type: App\DTO\AddressDTO, groups: [ user:sign:response ] }
            - { alias: AddressDTO_Write, type: App\DTO\AddressDTO, groups: [ user:write ] }

Obviously, this doesn't solve the problem of the bundle creating extra schemes in the doc though

Thanks @vpshvd,

I already knew that way to do it. But when the project have +150 entities, +50 outputs, +50 enums and other classes it is hard to maintain the all schemas correctly.