[BUG] PHP Client inconsistent unserialize behavior between object and array

[nadar]

Description

We have created a PHP SDK using the OpenAPI Generator (openapi-generator-cli generate -i https://api.flyo.cloud/nitro/v1/openapi -g php), and we have noticed inconsistent behavior regarding the parsed results. The results for accessing data is sometimes either an object or an array. It is possible that this inconsistency is related to how the OpenAPI description is defined, although we are not entirely sure.

openapi-generator version

6.6.0

OpenAPI declaration file content or url

block:
      title: Block
      type: object
      properties:
        items:
          title: BlockItems
          type: array
          items: {}
          description: Mapping Items
          example:
            - title: Hello
              teaser: Great!
            - title: Hallo
              teaser: Toll!
        content:
          title: BlockContent
          type: object
          description: Text Contents
          example:
            title: Hello World

I cut out the relevant part, but here is the full reference: https://api.flyo.cloud/nitro/v1/openapi

Generation Details

The screenshot below demonstrates the inconsistency, with the content section sometimes being represented as an array and sometimes as an object:

As observed in the OpenAPI file, the content section is defined with type: object. Therefore, one would expect to access its values using $block->getContent()->someVariable. However, the actual access method is $block->getContent()['someVariable']. On the other hand, in the items property, which is an array of objects, the array access method works as expected: foreach ($block->getContents() as $item) { $item->someVariable }.

Suggested Fix

To ensure consistent behavior, it is recommended to either use objects (stdClass in the PHP world) or arrays consistently throughout the SDK.

[wing328]

To ensure consistent behavior, it is recommended to either use objects (stdClass in the PHP world) or arrays consistently throughout the SDK.

Thanks for reporting the issue. I agree with consistency throughout the SDK.

Is that something you can help us with by filing a PR with the suggested fix?

[nadar]

I am not sure where to start, i assume somewhere here https://github.com/OpenAPITools/openapi-generator/blob/master/modules/openapi-generator/src/main/resources/php/ObjectSerializer.mustache#L394

I guess someone needs to point me into the right direction in order to send a PR. Also maybe there is a reason why this has not been done already?

[wing328]

cc PHP Technical committee to see if they've a clue.

@jebentier (2017/07), @dkarlovi (2017/07), @mandrean (2017/08), @jfastnacht (2017/09), @ybelenko (2018/07), @renepardon (2018/12)

[dkarlovi]

When decoding JSON in PHP, you decide what - the object element is by passing the second arg to json_decode. But, arrays will always be arrays, meaning JSON [] will get decoded as a PHP array, with this setting you control what happens to {}. Verify your input data which gets deserialized.

[nadar]

hi @dkarlovi thanks for taking time on this. I do agree with you, of course an array without keys will always be an array an object (assoc array) an object :-) https://3v4l.org/pEKaA

But as you can see from the example code i can access the key someVariable in $block->getContent()['someVariable'] so this MUST be an assoc array (which should be resolved as object (https://3v4l.org/pEKaA).

I will take a look at the response data and post this here.

[nadar]

Here you can see the json response for those two definitions getItems() and getContent() (i have removed not relevant data):

{
  "items": [
    {
      "title": "Foo Bar"
    },
    {
      "title": "Bar Foo"
    }
  ],
  "content": {
    "title": "Super duper title",
    "_empty": false
  }
}

as you can see here https://3v4l.org/E9gVd those would be all objects, but the sdks generates (for whatever reason) the getContent() as array output instead of an object. Here you see what the Schema Model result looks like btw: https://github.com/flyocloud/nitro-php-sdk/blob/main/lib/Model/Block.php

Based on the data above i would expect $block->getContent()->title and not $block->getContent()['title'].

btw, if we try to use $block->getContent()->title we get Attempt to read property "title" on array so i assume that for whatever reason sometimes the https://www.php.net/json_decode $associative is true and sometimes false. But since i could not see that $assoc param somewhere (expecting always an object which is true), there must be some "rewrite" happen based on certain circumstances. I saw things like that, not sure what happens, generating assoc arrays and not keeping the object annotation:

• https://github.com/OpenAPITools/openapi-generator/blob/master/modules/openapi-generator/src/main/resources/php/ObjectSerializer.mustache#L424
• https://github.com/OpenAPITools/openapi-generator/blob/master/modules/openapi-generator/src/main/resources/php/ObjectSerializer.mustache#L410
• https://github.com/OpenAPITools/openapi-generator/blob/master/modules/openapi-generator/src/main/resources/php/ObjectSerializer.mustache#L75

[nadar]

@ybelenko @dkarlovi @wing328 The problem is here: https://github.com/OpenAPITools/openapi-generator/blob/master/modules/openapi-generator/src/main/resources/php/ObjectSerializer.mustache#L431

This changes the type from object to array and therefore an array is returned instead of an object.

• Why this has been introduced? What's the reason to do so?
• If someone defines an object, i would say an object is expected, and not an array.

[ybelenko]

@ybelenko @dkarlovi @wing328 The problem is here: https://github.com/OpenAPITools/openapi-generator/blob/master/modules/openapi-generator/src/main/resources/php/ObjectSerializer.mustache#L431

This changes the type from object to array and therefore an array is returned instead of an object.

• Why this has been introduced? What's the reason to do so?

• If someone defines an object, i would say an object is expected, and not an array.

Can't say about that particular line, I'm not the author of the template.

My opinion it's just common php developers experience. Since we cannot initiate object in { foobar: "baz" } way, then we usually write [ "foobar" => "baz" ], so we don't use objects in php at all. That's why json_decode option has been introduced. I don't see any problems when objects serialized as arrays.

If you don't like it and want to check whether you have object or array at the runtime you can change the template, build new client lib and try again. That's the point of templates, everybody can change generator to fit his personal needs. That's why I don't like generator cli options by the way, it makes output unpredictable since you have fully customisable templates and conditions from cli flags at the same time. Hard to maintain.

[nadar]

I don't see any problems when objects are serialized as arrays.

I agree, BUT only if it's consistent throughout the SDK, which is not the case. It's a bad developer experience if sometimes you get arrays and sometimes stdClass objects, as demonstrated by the code above. Either always use an associative array representation for objects or always use objects.

If you don't like it and want to check whether you have an object or array at runtime, you can change the template, build a new client library, and try again.

Then I would have to patch the library every time we provide new updates. I don't think that should be the case. Isn't it what makes this OpenAPI generator such a strong and amazing tool? Everybody can benefit from updates made in the vendor generator just by calling the generate command. You have the latest code from the generator (vendor) and the latest updates you have made to your OpenAPI file. But that's maybe a personal preference, or you need to point me in the right direction regarding what you mean by templates. 👍

What really makes me curious is that nobody has struggled with the same issue here. I always need to var_dump() the getter methods and see "ah, it's an array" to use getContent()['var'], or "ah, it's an object" to use getContent()->var, instead of having a straightforward developer path.

I would suggest adding a flag to enable strict object generation. If enabled, the option I pointed out would be removed from the ObjectSerializer; otherwise, it should be kept as it is.

I think this is a real issue that should be improved or, at the very least, explained to people what is expected and when. Even the generated PHPDocs are wrong (https://github.com/flyocloud/nitro-php-sdk/blob/main/lib/Model/Block.php#L364 => it's not an object, it's an array).

I would also be willing to sponsor something to get this properly fixed.

[denyszrazhevskiy]

Problem got worse after adding return types. As an example from issue description When we create Block model we have setContent(object $object), but ObjectSerializer return array and error occurs

[denyszrazhevskiy]

I think @nadar is right in his comment and we need remove

 if ($class === 'object') {
            settype($data, 'array');
            return $data;
        } else

[nadar]

@denyszrazhevskiy Yes, it's indeed still a problem and should be fixed in the core, thanks for helping me vote for the fix :+1: . Temporarily, we have made use of @ybelenko's suggested 'template fix,' where all the files are now part of the repository and there we have fixed the bug. The problem is obviously that we don't benefit from further updates of the library.

This is how we generate and replace the code:

• https://github.com/flyocloud/nitro-php-sdk/blob/main/generate.sh
• The replaced fixed file: https://github.com/flyocloud/nitro-php-sdk/blob/main/templates/ObjectSerializer.mustache#L431-L432