DoclerLabs / api-client-generator

API client generator is a console application capable of generating an API client based on OpenAPI(Swagger) specification.
MIT License
31 stars 19 forks source link

Optional and nullable properties won't be really optional in generated request class #80

Closed szhajdu closed 2 years ago

szhajdu commented 2 years ago
openapi: 3.0.1

info:
  title: Test Api
  version: '1'
  description: Test Api

paths:
  '/examples':
    patch:
      requestBody:
        required: true
        content:
          application/json:
            schema:
              type: object
              properties:
                foo:
                  type: integer
                  nullable: true
                bar:
                  type: integer
                  nullable: true
            examples:
              Example 1A:
                value:
                  foo: 1
              Example 1B:
                value:
                  foo: null
              Example 2A:
                value:
                  bar: 2
              Example 2B:
                value:
                  bar: null
              Example 3A:
                value:
                  foo: 1
                  bar: 2
              Example 3B:
                value:
                  foo: null
                  bar: null
      responses:
        204:
          description: Updated successfully

The toArray method in generated request body class will always send each optional properties to the endpoint:

public function toArray(): array
{
    $fields        = [];
    $fields['foo'] = $this->foo;
    $fields['bar'] = $this->bar;

    return $fields;
}

If I remove the nullable: true modifier, it will generate optional properties, but it won't be nullable anymore:

public function toArray(): array
{
    $fields        = [];
    if ($this->foo !== null) {
        $fields['foo'] = $this->foo;
    }
    if ($this->bar !== null) {
        $fields['bar'] = $this->bar;
    }

    return $fields;
}

I think the request object should know if a property has been set or not instead of checking default null values. Maybe maintain flags or use some specific value for it.

vsouz4 commented 2 years ago

I understand your point, but:

szhajdu commented 2 years ago

Most probably this is a design issue, but there are legacy APIs and I can imagine that something is depends on this functionality. The optional nullable properties are part of openapi specification and it should be supported by the generator as well, or at least marked as unsupported feature and generation should fail in these situations, otherwise the generated code won't match with the specification and it could lead to unexpected issues on the user side.

vsouz4 commented 2 years ago

it's misleading indeed, added PR with your suggestion, thx

Screen Shot 2022-06-27 at 08 17 15