openapi-generator icon indicating copy to clipboard operation
openapi-generator copied to clipboard

[BUG] oneOf(List,Object) not correctly working with to_dict

Open FrankEssenberger opened this issue 1 year ago • 3 comments

Bug Report Checklist

  • [x] Have you provided a full/minimal spec to reproduce the issue?
  • [x] Have you validated the input using an OpenAPI validator (example)?
  • [x] Have you tested with the latest master to confirm the issue still exists?
  • [x] Have you searched for related issues/PRs?
  • [x] What's the actual output vs expected output? ~~- [ ] [Optional] Sponsorship to speed up the bug fix or feature request (example)~~
Description

When you have a spec containing a oneOf with a list of objects and a single object the generated to_dict looks like:

        if hasattr(self.actual_instance, "to_dict") and callable(self.actual_instance.to_dict):
            return self.actual_instance.to_dict()     
        else:
            # primitive type
            return self.actual_instance

However, the list object does not have a to_dict so the individual elements do not get serialized in case the object is the list type. Something like this should be there:

        if hasattr(self.actual_instance, "to_dict") and callable(self.actual_instance.to_dict):
            return self.actual_instance.to_dict()
        elif isinstance(self.actual_instance, list):
            return [item.to_dict() for item in self.actual_instance]
        else:
            # primitive type
            return self.actual_instance

Actual behavior with the properties not converted: image Expected behavior with the dicts: image

openapi-generator version

Version 7.4.0 exetued

OpenAPI declaration file content or url
openapi: 3.0.0
info:
  title: Sample API
  version: 0.0.1
servers:
  - url: http://api.example.com/v1
paths:
  /users:
    get:
      responses:
        '200':    # status code
          description: A JSON array of user names
          content:
            application/json:
                schema:
                    $ref: "#/components/schemas/output"

components:
  schemas:
    list_prop:
      type: array
      items:
        $ref: "#/components/schemas/object_prop"

    object_prop:
      type: object
      properties:
        name:
          type: string

    output:
      oneOf:
      - $ref: "#/components/schemas/object_prop"
      - $ref: "#/components/schemas/list_prop"
Generation Details

We use the docker container to run the generator:

docker run --rm -v ${PWD}/src:/src openapitools/openapi-generator-cli:v7.4.0 generate -i /src/spec/minimal.yaml -g python -o /src/tmp --package-name test.v1.generated --additional-properties=generateAliasAsModel=false
Steps to reproduce

Take the spec and generate then execute the following:

import json
from v1.generated.models.output import Output

o = Output.from_json(json.dumps([{"name":'foo'},{"name":'bar'}]))
o.to_dict() #shows the unconverted dict entries
Related issues/PRs

I found an issue for the parsing: https://github.com/OpenAPITools/openapi-generator/issues/14986 but want to use not the from_jsonbut the to_dict.

Suggest a fix

If there is a union type like Union[List[SomeObject],SomeOtherObject] do in the to_dict() method add a check if the current object is of type list. If so loop over the items and call the to_dict() on each individual item (see above). Potentially in the loop one should also check once more if the iten is a primitive type, then one should just put it in the list of course.

FrankEssenberger avatar Mar 14 '24 15:03 FrankEssenberger

@FrankEssenberger I just came across the same issue, however I would solve it in a different place: The problem is that to_dict() is called from api_client.py in the method sanitize_for_serialization. There should be the extra case for lists, like this:

    def sanitize_for_serialization(self, obj):
        """Builds a JSON POST object.

        If obj is None, return None.
        If obj is SecretStr, return obj.get_secret_value()
        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 decimal.Decimal return string representation.
        If obj is list, sanitize each element in the list.     <<< NOTE: this already does the right thing for a normal 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 isinstance(obj_dict, list):
            # here we handle instances that can either be a list or something else, and only became a real list by calling to_dict()
            return self.sanitize_for_serialization(obj_dict)

        return {
            key: self.sanitize_for_serialization(val)
            for key, val in obj_dict.items()
        }

My addition to the method goes in here: https://github.com/OpenAPITools/openapi-generator/blob/c2472b03b63fa61a3d7e5b086f36c1bd48d0ee4c/modules/openapi-generator/src/main/resources/python/api_client.mustache#L393-L396

I will try to open a PR soon, if some of the maintainers don´t fix it before I can :)

karge-itestra avatar Aug 20 '24 15:08 karge-itestra

I actually did not see the sanitize_for_serialization method and since other checks are already in there I agree that this is a more fitting place to keep the sanatize logic together.

FrankEssenberger avatar Aug 21 '24 09:08 FrankEssenberger

I created the pull request, if you want to take a look: https://github.com/OpenAPITools/openapi-generator/pull/19405

karge-itestra avatar Aug 22 '24 09:08 karge-itestra

@karge-itestra I have added a comment to your PR. Thank you for finishing this work in advance.

denys999 avatar Mar 18 '25 13:03 denys999