[PHP] Serializing maps

[pminten]

Description

When serializing a map object (type object with additionalProperties) in a generated PHP client additional keys not explicitly declared in the spec are ignored.

Swagger-codegen version

2.2.2, the problem also exists in the master branch at abffdca (current tip of master).

Swagger declaration file content or url

This is a definition which I stole from from another issue but it nicely demonstrates the problem.

{
  "swagger": "2.0",
  "info": {
    "version": "0.0.1",
    "title": "test of Swagger with boolean additionalProperties in schema definition"
  },
  "paths": {
    "/": {
      "get": {
        "produces": [ "application/json" ],
        "description": "xxx",
        "responses": {
          "200": {
            "schema": { "$ref": "#/definitions/Root" }
          }
        }
      }
    }
  },
  "definitions": {
    "Root": {
      "type": "object",
      "properties": {
        "foo": {
          "type": "string"
        }
      },
      "additionalProperties": false
    }
  }
}

Command line used for generation

java -jar $SWAGGER_CODEGEN_CHECKOUT/modules/swagger-codegen-cli/target/swagger-codegen-cli.jar generate -l php -i input.json -o generated

Steps to reproduce

I have a simple PHP file:

<?php
require 'SwaggerClient-php/autoload.php';
$serializer = new Swagger\Client\ObjectSerializer();
var_dump($serializer->sanitizeForSerialization(new \Swagger\Client\Model\Root(["a" => "b", "c" => "d", "foo" => "bar"])));

Running this (no arguments or anything needed) produces:

object(stdClass)#4 (1) {
  ["foo"]=>
  string(3) "bar"
}

I would expect a mapping type to contain all keys and values passed to it (keys "a" and "c" and associated values). Because it doesn't all key/value pairs not declared explicitly in the spec will not be encoded and thus not be sent to the server, defeating the point of additionalProperties.

Related issues

Couldn't find any.

Suggest a Fix

Based on how ObjectSerializer works it would seem sensible to have the generated model class simply store all key/value pairs somewhere and make them visible through swaggerTypes() and getters()/setters(). However there are a few problems with that. One is that setters returns an array which is typically immediately indexed into, that will cause problems if you define a new key. This can be solved by having setters return an array-like object instead with some magic behavior. The second problem is that ObjectSerializer will again use swaggerTypes() to figure out which properties to set upon deserialization. Of course swaggerTypes can't know all the properties beforehand on a map so this will miss stuff on deserialization.

[wing328]

@pminten can you show us what the JSON payload looks like? I think you may not have defined Root in your example correctly.

[pminten]

The JSON the api expects is something like (usually with a whole lot more question_NUM keys):

{"complete": false, "question_1": "a", "question_2": 2}

I've tried this with several variations of the Root definition, all drop the question_* entries

    "Root": {
      "type": "object",
      "properties": {
        "changed": {
          "type": "string"
        }
      },
      "additionalProperties": true
    }

    "Root": {
      "type": "object",
      "additionalProperties": true
    }

And the same with additionalProperties false (which after reading the json-schema spec is indeed incorrect). Setting additionalProperties to {"type": "string"} (which is actually incorrect for our schema but I wanted to test it anyway) doesn't do the trick either.

[wing328]

Instead of defining "$ref": "#/definitions/Root" inside "schema", what using the following instead?

{
  "schema": {
    "type": "object",
    "additionalProperties": {
      "type": "string"
    }
  }
}

[pminten]

I've tested that with the following spec:

{
  "swagger": "2.0",
  "paths": {
    "/": {
      "get": {
        "operationId": "getIt",
        "produces": [ "application/json" ],
        "responses": {
          "200": {
            "schema": {
              "type": "object",
              "additionalProperties": {"type": "string"}
            }
          }
        }
      },
      "post": {
        "operationId": "postIt",
        "consumes": ["application/json"],
        "parameters": [{
          "name": "body",
          "in": "body",
          "required": true,
          "schema": {
            "type": "string",
            "additionalProperties": {"type": "string"}
          }
        }]
      }
    }
  }
}

The spec is intended to work with a trivial php page which I run with php -S localhost:9001 server.php:

<?php
if ($_SERVER['REQUEST_METHOD'] == 'GET') {
        echo '{"question_1": 1, "question_2": "b"}';
} else {
        error_log(file_get_contents("php://input")); // log post body
}

The driver is a client program:

<?php
<?php
require "SwaggerClient-php/autoload.php";
$apiClient = new Swagger\Client\ApiClient((new Swagger\Client\Configuration())->setHost("http://localhost:9001"));
$api = new Swagger\Client\Api\DefaultApi($apiClient);
var_dump($api->getIt());

echo "-----\n";

// Keep as array to get data correctly encoded
$api->postIt(["question_1" => 1, "question_2" => "a"]);

echo "-----\n";

// Cast to object to match type expressed in phpdoc comment of api
$api->postIt((object)["question_1" => 1, "question_2" => "a"]);

This is the result:

array(2) {
  ["question_1"]=>
  string(1) "1"
  ["question_2"]=>
  string(1) "b"
}
-----
-----
PHP Fatal error:  Uncaught Error: Call to undefined method stdClass::swaggerTypes() in /tmp/generated4/SwaggerClient-php/lib/ObjectSerializer.php:62
Stack trace:
#0 /tmp/generated4/SwaggerClient-php/lib/ApiClient.php(156): Swagger\Client\ObjectSerializer::sanitizeForSerialization(Object(stdClass))
#1 /tmp/generated4/SwaggerClient-php/lib/Api/DefaultApi.php(221): Swagger\Client\ApiClient->callApi('/', 'POST', Array, Object(stdClass), Array, NULL, '/')
#2 /tmp/generated4/SwaggerClient-php/lib/Api/DefaultApi.php(169): Swagger\Client\Api\DefaultApi->postItWithHttpInfo(Object(stdClass))
#3 /tmp/generated4/client.php(15): Swagger\Client\Api\DefaultApi->postIt(Object(stdClass))
#4 {main}
  thrown in /tmp/generated4/SwaggerClient-php/lib/ObjectSerializer.php on line 62

On the server the first post call gives an incoming post body of {"question_1":1,"question_2":"a"}.

So the get call works correctly, the post call with array works (but is incorrect per the PHPDoc comment, which leads to annoying IDE highlighting) and the post call with object fails.

Note that there's a warning from the generator: [main] WARN io.swagger.codegen.languages.PhpClientCodegen - Type object not handled properly in setParameterExampleValue

[wing328]

cc @arnested

[msample]

This issue recently affected a project I'm involved with. Patching the generated code to skip json decode works but is a bit grim for the robustness of the build process.

~Unrelated - other patch solutions we've resorted to were to enable explicit nulls in json (we were defining Swagger for an existing service). Per-operation codegen features enabled by operation tagging or something would be handy.

[wing328]

@msample instead of patching the auto-generated code, what about customizing the templates?

https://github.com/swagger-api/swagger-codegen/wiki/FAQ#how-can-i-customize-the-template-to-add-headerfooter-to-the-auto-generated-code

[msample]

@wing328 thanks for the suggestion. I considered it but just one of many endpoints had this issue, and it was a one line change. I didn't have the time to look at conditionals in the template to treat empty objects differently and even then it seems like a higher level data structure mapping problem - ie how to handle KVs that are not explicitly declared in objects such that they are accessible in the deserialized form, and subsequently re-serialized if req'd.

[ackintosh]

👀

[wing328]

#6685 has been merged into master. Thanks for the fix by @ackintosh

@pminten please pull the latest master to give it a try.