OpenAPITools / openapi-generator

OpenAPI Generator allows generation of API client libraries (SDK generation), server stubs, documentation and configuration automatically given an OpenAPI Spec (v2, v3)
https://openapi-generator.tech
Apache License 2.0
21.75k stars 6.56k forks source link

Multiple use of allOf not working in python #6796

Open mwilby opened 4 years ago

mwilby commented 4 years ago

Bug Report Checklist

Description

The use of allOf more than once, is not working in python. Specifically, models that use an allOf import on a model that also uses allOf are not correctly constructed.

using

address:      # Can be referenced via '#/components/schemas/address'
  type: object
  properties:
    al:
      type: array
      items:
        $ref: 'https://host014.etsit.upm.es/schemas/test/maitred/msg/net_point.yaml#/network_point'
    did:
      type: string
    sid:
      type: integer
      format: int32

to build

header:
  type: object
  allOf:
    - $ref: 'https://host014.etsit.upm.es/schemas/test/maitred/base/address.yaml#/address'
  properties:
    tag:
      type: string
    f:
      type: integer
      format: int32
      enum:
        - -1
        - 0
        - 1
        - 2
        - 3
        - 4
      default: 0
    ts:
      type: integer
      format: int64

The corresponding generator header class results in

class Header(Model):

    def __init__(self, tag=None, f=0, ts=None, al=None, did=None, sid=None):  # noqa: E501

Everything is fine, including the enumerator.

Now, if it is then used to build

message:
  type: object
  allOf:
    - $ref: 'https://host014.etsit.upm.es/schemas/test/maitred/base/message_header.yaml#/header'
  properties:
    pl:
      type: object

The resulting class results is

class Message(Model):

    def __init__(self, al=None, did=None, sid=None, pl=None):  # noqa: E501

Only the address field are included and components native to header have disappeared.

Its not just a simple template mistake in not including the inheritance of the base class. There is a problem in including the properties in the base model in the derived class. Something is going wrong with the generation of the vars and their variations.

Also, if inheritance was the target design, then there is an ambiguity with the imports. The message model has

from maitred.base.models.header import Header
from maitred.base.models.network_point import NetworkPoint

Where the NetworkPoint should not be necessary at this class level.

A rethink in terms of the constructor arguments will also be needed.

Note, the schema generates a model that, so far works fine in Java.

openapi-generator version

4.3.1-SNAPSHOT and 4.3.0

OpenAPI declaration file content or url

The corresponding full model files are published as:

network_point address header message

Command line used for generation

N/A

Steps to reproduce

N/A

Related issues/PRs

N/A

Suggest a fix

The simplest hack would be to modify the template to inherite the parent class and live with the fact that the base properties on the deepest model will be overloaded, but clarifying the var list would be nice.

mwilby commented 4 years ago

We had to fix this, it was preventing us merging two projects.

We modified the python-flask model.mustache. We don't use the python template because it does not handle the type's correctly.

The solution we used is the one suggested. Its very ugly, and almost certainly slower than it needs to be, but we needed something that worked. So now the Message class looks like this:

class Message(Header):
    def __init__(self, pl=None, al=None, did=None, sid=None, **oapi_ud_param_list):  # noqa: E501
        super(Message, self).__init__(oapi_ud_param_list=oapi_ud_param_list)

        self.openapi_types['pl'] = object
        self.openapi_types['al'] = List[NetworkPoint]
        self.openapi_types['did'] = str
        self.openapi_types['sid'] = int

        self.attribute_map['pl'] = 'pl'
        self.attribute_map['al'] = 'al'
        self.attribute_map['did'] = 'did'
        self.attribute_map['sid'] = 'sid'

        self._pl = pl
        self._al = al
        self._did = did
        self._sid = sid

The generated code ran inside a fairly complex app with no problems.

We went back and checked the python generator and it has similar problems. But in that case the vars list is correct, i.e. it is only the locally defined properties. So a full fix is possible just with the template.

I have posted the template (I think I took out all our vendor specific stuff) at

model.mustache

It could be used as a model for all the server specific generators.

There are additional issues due to the fact that the bundle structure in python is different to that of the server generators. The problem of having 2 branches of the python language definitions, are only going to get worse. The failed attempt to address this in #6065 is now completely defunct, because merging the PythonCodegen with server abstract class is more involved than was addressed, because the bundles generated differ dramatically. Its going to need a convergence mechanism, if you don't want the divergence to needlesly grow.

mwilby commented 4 years ago

Because we set the parameters after instantiation, we didn't notice, but the assignment of parameters to the super class wasn't working correctly. We corrected the template, now the variable assignment looks like.

        oapi_ud_param_list = oapi_ud_param_list.get('oapi_ud_param_list')

        if oapi_ud_param_list is None:
            self._al = al
            self._did = did
            self._sid = sid
        else:
            if oapi_ud_param_list.get('al') is None:
                self._al = al
            else:
                self._al = oapi_ud_param_list.get('al')
            if oapi_ud_param_list.get('did') is None:
                self._did = did
            else:
                self._did = oapi_ud_param_list.get('did')
            if oapi_ud_param_list.get('sid') is None:
                self._sid = sid
            else:
                self._sid = oapi_ud_param_list.get('sid')

As I said, its ugly, I am sure there is a better and more efficient way of doing it, but we haent time t play with it.