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

Generating required fields while using DTOs in response #2111

Closed
mrazek-honza opened this issue Jun 30, 2023 · 11 comments
Closed

Generating required fields while using DTOs in response #2111

mrazek-honza opened this issue Jun 30, 2023 · 11 comments

Comments

@mrazek-honza
Copy link

Hello I encountered a minor inconvenience / issue while trying to generate TypeScript types from JSON documentation (using this package) and Nelmio Api Doc bundle version 4.11.1. I have the following class that is serialized to JSON and sent in response body

class ExampleResponse
{
    public function __construct(
        public readonly int $id,
        public readonly string $name,
        public readonly bool $active
    ){
    }
}

Route that is sending this response has the following code

    #[OA\Get(
        summary: 'Example response',
        responses: [
            new OA\Response(
                response: 200,
                description: 'Example',
                content: new Model(type: ExampleResponse::class),
            ),
        ]
    )]
    #[Route('/example', name: 'example', methods: ['GET'])]
    public function testDoc(): JsonResponse
    {
        return $this->json(
            data: new ExampleResponse(1, 'string', true),
        );
    }

This generates following path and component in JSON documentation

"\/api\/project\/example": {
    "get": {
        "summary": "Example response",
        "responses": {
            "200": {
                "description": "Example",
                "content": {
                    "application\/json": {
                        "schema": {
                            "$ref": "#\/components\/schemas\/ExampleResponse"
                        }
                    }
                }
            }
        }
    }
}
"ExampleResponse": {
    "properties": {
        "id": {
            "type": "integer"
        },
        "name": {
            "type": "string",
            "nullable": true
        },
        "active": {
            "type": "boolean"
        }
    },
    "type": "object"
},

However when I tried to generate the types using it generates all properties from ExampleResponse as optional

interface ExampleResponse {
    id?: number;
    name?: string | null;
    active?: boolean;
};

The only way I managed to generate the desired JSON output is below mentioned solution:
Add a schema to the ExampleResponse class

use OpenApi\Attributes as OA;

#[OA\Schema(
    required: [
        'id',
        'name',
        'active',
    ]
)]
class ExampleResponse
{
    public function __construct(
        public readonly int $id,
        public readonly ?string $name,
        public readonly bool $active
    ){
    }
}

Which generates required fields in response schema

"ExampleResponse": {
    "required": [
        "id",
        "name",
        "active"
    ],
    "properties": {
        "id": {
            "type": "integer"
        },
        "name": {
            "type": "string",
            "nullable": true
        },
        "active": {
            "type": "boolean"
        }
    },
    "type": "object"
},

Question is - is this an intended behavior? I read the docs and I found out that using the Model should generate these fields as required by default. Please note that I am not using any Asserts nor Serializer groups like mentioned in docs (I do not assume it makes any difference). If it is intended, is there any way how to generate the desired output while not having to explicitly define all required parameters? Something like: treat all properties of this class as required when generating the JSON schema?

Thank you for response

@palgalik
Copy link

I have exactly the same issue.

@egonolieux
Copy link

I encountered the exact same issue. I would expect the properties to be required by default, unless a default value was specified.

@mrazek-honza
Copy link
Author

As it turns out, adding an attribute assert not blank to a property makes it non optional in the generated schema.

use Symfony\Component\Validator\Constraints as Assert;

#[Assert\NotBlank]
public readonly int $id;

However adding this attribute to every property of every DTO is an inconvenience, not to mention a thing that you have to keep in mind every time you add a property

@palgalik
Copy link

palgalik commented Aug 22, 2023

Also you can add Schema attribute to the class, and list all the required properties.

use OpenApi\Attributes as OA;

#[OA\Schema(required: ['property1', 'property2'])]
readonly class Foo {
    public function __construct(
        public string         $property1,
        public string         $property2,
    ) {
    }
}

I'm looking for a solution, where the generator automatically interprets the constructor non nullable arguments as required properties.

@mrazek-honza
Copy link
Author

Yes that is also a possibility. In the current situation I prefer the assert version, because you can just copy paste it and it is not sensitive to renaming properties.

@Autsider666
Copy link

I came here with the hope for a solution to this as well. Having to put #[Assert\NotBlank] everywhere works, but it's quite easy to just forget because adding asserts to response DTOs isn't really an obvious step.

I've tried decorating some services, but everything that's really useful to any of this is initialized inside classes like Nelmio\ApiDocBundle\ModelDescriber\ObjectModelDescriber

@DjordyKoert
Copy link
Collaborator

I have made a PR to fix this issue: #2141

Would be helpful if someone could test this branch of mine on their own project and see if any issues occur and if everything is as they expected.

I have tested it on our own project and it seems to work perfect (also checked it with the openapi-typescript package which was mentioned in the ticket).

How to test

Simply add my branch as a vcs repo inside the composer.json.

        {
            "name": "nelmio/api-doc-bundle",
            "type": "vcs",
            "url": "[email protected]:DjordyKoert/NelmioApiDocBundle.git"
        }

And update your nelmio/api-doc-bundle require to this:

        "nelmio/api-doc-bundle": "dev-feat/2111-set-required-fields-on-object",

Followed by a composer update nelmio/api-doc-bundle

@mrazek-honza
Copy link
Author

Hello @DjordyKoert, sorry for inactivity. I have a lot on my plate currently. I will get to this maybe next week, and give you feedback :) Thank you anyways for your activity and awesome work you are putting into this.

@mrazek-honza
Copy link
Author

@DjordyKoert finally had a chance to test this, seems to work well in my project :)

@DjordyKoert
Copy link
Collaborator

@mrazek-honza glad to hear, I have merged the PR :)

@darius-v
Copy link

darius-v commented Nov 9, 2024

As it turns out, adding an attribute assert not blank to a property makes it non optional in the generated schema.

use Symfony\Component\Validator\Constraints as Assert;

#[Assert\NotBlank]
public readonly int $id;

However adding this attribute to every property of every DTO is an inconvenience, not to mention a thing that you have to keep in mind every time you add a property

As it turns out, adding an attribute assert not blank to a property makes it non optional in the generated schema.

use Symfony\Component\Validator\Constraints as Assert;

#[Assert\NotBlank]
public readonly int $id;

However adding this attribute to every property of every DTO is an inconvenience, not to mention a thing that you have to keep in mind every time you add a property

Invonvenience would be small problem.
But what if property is nullable but still required? This makes it code reading as it should be not blank but actually it can be blank.
https://stackoverflow.com/a/45577763/6737670
And it probably even validates and gives error if that becomes null.

Only way I know is to use this

#[OA\Schema(
    required: [
        'id',
        'name',
        'active',
    ]
)]

but that looks weird that it generates automatically but incorrectly. Because I often need to put only some properties into schema required array. And when I am writing this way, I am doubting if that is the correct way to do.

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

6 participants