[PYTHON] Generated code disregards nullable flag

[noamgat]

Description

I generated a data structure that has a nullable property, yet the python code that is generated does not see it.

Swagger-codegen version

https://repo1.maven.org/maven2/io/swagger/swagger-codegen-cli/2.4.16/swagger-codegen-cli-2.4.16.jar

Swagger declaration file content or url

"ProjectProfile": {
            "type": "object",
            "properties": {
                "id": {
                    "type": "number"
                },
                "result": {
                    "type": "object",
                    "description": "An object from the JSON field in the database",
                    "nullable": true,
                    "default": {}
                }
            },
            "required": ["id", "result"]
        },

Command line used for generation

java -jar swagger-codegen-cli.jar generate -i http://localhost:3001/api-docs-json -l python -o swapp_system_client_package -DpackageName=swapp_system_client

Steps to reproduce

run codegen with json snippet, this is how the python will look for result setter:

@result.setter
    def result(self, result):
        """Sets the result of this ProjectProfile.

        An object from the JSON field in the database  # noqa: E501

        :param result: The result of this ProjectProfile.  # noqa: E501
        :type: object
        """
        if result is None:
            raise ValueError("Invalid value for `result`, must not be `None`")  # noqa: E501

        self._result = result

Related issues/PRs

Suggest a fix/enhancement

It seems like nullable flag does not affect the setter code generation in python.

[noamgat]

Looking at the source code in this file:
https://github.com/swagger-api/swagger-codegen/blob/5a6d68716bc5a5f6fda10017ff380c382183d0c0/modules/swagger-codegen/src/main/resources/python/model.mustache
it looks like the snippet is surrounded by {{#required}} {{/required}}
Shouldn't this check be dropped in case of #nullable ? (Is there such a thing?)

[rvinzent]

(Is there such a thing?)

yes, it is entirely possible to require a nullable field to be present. This is causing issues for me as well

[rvinzent]

I believe I've found the offending code:

    def sanitize_for_serialization(self, obj):
        """Builds a JSON POST object.
        If obj is None, return None.
        If obj is str, int, long, float, bool, return directly.
        If obj is datetime.datetime, datetime.date
            convert to string in iso8601 format.
        If obj is list, sanitize each element in the list.
        If obj is dict, return the dict.
        If obj is OpenAPI model, return the properties dict.
        :param obj: The data to serialize.
        :return: The serialized form of data.
        """
        if obj is None:
            return None
        elif isinstance(obj, self.PRIMITIVE_TYPES):
            return obj
        elif isinstance(obj, list):
            return [self.sanitize_for_serialization(sub_obj)
                    for sub_obj in obj]
        elif isinstance(obj, tuple):
            return tuple(self.sanitize_for_serialization(sub_obj)
                         for sub_obj in obj)
        elif isinstance(obj, (datetime.datetime, datetime.date)):
            return obj.isoformat()
        if isinstance(obj, dict):
            obj_dict = obj
        else:
            # Convert model obj to dict except
            # attributes `openapi_types`, `attribute_map`
            # and attributes which value is not None.
            # Convert attribute name to json key in
            # model definition for request.
            obj_dict = {obj.attribute_map[attr]: getattr(obj, attr)
                        for attr, _ in six.iteritems(obj.openapi_types)
                        if getattr(obj, attr) is not None}
        return {key: self.sanitize_for_serialization(val)
                for key, val in six.iteritems(obj_dict)}

In the else block, None values are being filtered out. This code should probably be replaced with a .to_dict() call on the model which accurately produces a dictionary with appropriate None values.

Update:

just kidding, using to_dict() is also broken because it includes the required fields even when they are left unspecified. It sounds like the solution is probably to use an empty sentinel value to enable the client to distinguish between null and unspecified.

[MalteEbner]

@noamgat, @rvinzent: Could you check if my PR solves the issue also for you? If yes, do you know how to get it merged?

[rvinzent]

@noamgat, @rvinzent: Could you check if my PR solves the issue also for you? If yes, do you know how to get it merged?

I don't think the logic in that PR is quite right because if the field is nullable then it won't be required.

There should be a distinction between null and unset, but the library treats both cases as the same, so it can't implement a required, nullable field.

[ericboucher]

Any update on this? It would be quite useful to have the library handle that on its own. What solution/hack have you implemented in the meantime @MalteEbner @rvinzent?

[bonnici]

@ericboucher I posted a workaround in the PR: #10906 (comment)

[thibgc]

Note: Two other issues #8710 #12021 are marked as closed even though the corresponding PR #10906 has never been merged.