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

[PHP] nullable attribute with value null gets removed #2199

Open
githubERIK opened this issue Feb 20, 2019 · 6 comments
Open

[PHP] nullable attribute with value null gets removed #2199

githubERIK opened this issue Feb 20, 2019 · 6 comments

Comments

@githubERIK
Copy link

githubERIK commented Feb 20, 2019

Description

Given a nullable field in OpenAPI object

    SomeObject:
      type: object
      properties:
        someProperty:
          type: string
          nullable: true

When HTTP Request has the object and it includes someProperty=null.

Then the generated php client code does not include someProperty in the response.

openapi-generator version

master branch (version 3.3.4)
EDIT: also version 4.0.3

OpenAPI declaration file content or url

See the description

Command line used for generation

openapi-generator generate -g php

Steps to reproduce
Related issues/PRs
Suggest a fix/enhancement

Remove null-check if-block if ($value !== null) from https://github.com/OpenAPITools/openapi-generator/blob/master/modules/openapi-generator/src/main/resources/php/ObjectSerializer.mustache#L68

by changing this

if ($value !== null) {
    $values[$data::attributeMap()[$property]] = self::sanitizeForSerialization($value, $openAPIType, $formats[$property]);
}

to this

$values[$data::attributeMap()[$property]] = self::sanitizeForSerialization($value, $openAPIType, $formats[$property]);
realerikrani pushed a commit to messente/messente-phonebook-php that referenced this issue Feb 20, 2019
@wing328
Copy link
Member

wing328 commented Feb 27, 2019

@githubERIK thanks for reporting the issue and sharing the workaround.

But I think in certain use cases, the workaround may introduce additional null values in the payload, which are not present before.

I think we may need to maintain another associative array similar to openAPITypes to indicate if the property is nullable

realerikrani pushed a commit to messente/messente-api-php that referenced this issue Apr 24, 2019
realerikrani pushed a commit to messente/messente-api-php that referenced this issue May 22, 2019
@wing328
Copy link
Member

wing328 commented Jul 26, 2019

I think we may need to maintain another associative array similar to openAPITypes to indicate if the property is nullable

@githubERIK does my comment earlier make sense to you?

@githubERIK
Copy link
Author

githubERIK commented Jul 26, 2019

The comment makes sense.

For nullables in model_generic.mustache,

protected static $openAPINullables = [
    {{#vars}}'{{name}}' => {{#isNullable}}true{{/isNullable}}{{^isNullable}}false{{/isNullable}}{{#hasMore}},
    {{/hasMore}}{{/vars}}
];

could be added and then accessed.

@githubERIK
Copy link
Author

githubERIK commented Jul 26, 2019

Things to keep in mind.

When a request goes out

  • field is nullable and value is null
    • when somebody set the value to null on purpose then this should not be filtered out by ObjectSerializer
    • when somebody did not specify the value when using the constructor (causing the constructor to automatically set the value to null) then that null value should be filtered out

When a response comes in

  • field is nullable and value is null
    • echo $response should show all null values.

@jorissteyn
Copy link

jorissteyn commented Feb 26, 2020

API platform supports partial PUT operations: omitting a field in the request does not clear the value on PUT operations. So I'm wondering, the change discussed here, how would the client distinguish nullable fields explicitly set to null versus fields omitted in the payload?

I believe this question is especially important because the client does not support PATCH requests.

@gavrochelegnou
Copy link

Hi !
Is there any workaround for this issue ?
I'm still having problems forcing a field to null.
Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants