core-api / python-openapi-codec

An OpenAPI codec for Core API.
Other
33 stars 35 forks source link

OpenAPI data types are all named "data" #28

Open paultiplady opened 7 years ago

paultiplady commented 7 years ago

I'm trying to use swagger-codegen to generate a Python client from my django-rest-swagger schema (django-rest-swagger==2.0.7, djangorestframework==3.5.3).

I am getting a lot of models named Data*, and no client models related to my actual API models.

Here's an example OpenAPI path:

  "paths" : {
    "/accounts/{id}/" : {
      "put" : {
        "tags" : [ "accounts" ],
        "summary" : "The accounts for all Users.",
        "description" : "The accounts for all Users.",
        "operationId" : "accounts_update",
        "consumes" : [ "application/json" ],
        "parameters" : [ {
          "name" : "id",
          "in" : "path",
          "description" : "",
          "required" : true,
          "type" : "string"
        }, {
          "in" : "body",
          "name" : "data",
          "required" : false,
          "schema" : {
            "type" : "object",
            "required" : [ "account_number"],
            "properties" : {
              "account_number" : {
                "type" : "string",
                "description" : ""
              },
              ...
            }
          }
        } ],
        "responses" : {
          "200" : {
            "description" : ""
          }
        }
      },
      ...

Note the "in": "body", "name": "data" in the generated swagger JSON.

The model generated by swagger-codegen has the correct fields, but incorrect name:

class Data1(object):
    """
    NOTE: This class is auto generated by the swagger code generator program.
    Do not edit the class manually.
    """
    def __init__(self, account_number=None, ...):
        """
        Data1 - a model defined in Swagger

        :param dict swaggerTypes: The key is attribute name
                                  and the value is attribute type.
        :param dict attributeMap: The key is attribute name
                                  and the value is json key in definition.
        """
        self.swagger_types = {
            'account_number': 'str',
            ...
        }
        ...

Per the swagger-codegen client, it's expected that the "name" field here should actually describe the resource.

@marcgibbons over at django-rest-swagger pointed out in the original issue (https://github.com/marcgibbons/django-rest-swagger/issues/595) that the name field is hardcoded to data here: https://github.com/core-api/python-openapi-codec/blob/master/openapi_codec/encode.py#L167. Seems like it should either be the Serializer name, or the Serializer.Meta.model. (Configurable would be better; should be simple to delegate to an overridable function which gets passed the Serializer and returns the resource name.).

Thoughts on whether this sounds like the right approach, and where best to put this logic if it's acceptable?

tomchristie commented 7 years ago

Yup I think this is pretty valid point. We don't properly name the field in those cases because we don't have a serializer field on which to base that name. However, using the serializer class name or similar as a more meaningful name would make a lot of sense.

I'm working through all the Core API integration in detail at the moment as part of introducing a live-documentation tool for REST framework, so addressing this is absolutely on my backlog right now.

jmbowman1107 commented 7 years ago

Hi! Any update on when this issue plans on being addressed? This issue is a blocker for my project. I am using autorest to create a C# client, (it simply uses the last model with same name (e.g. 'Data')) So of course this makes my client completely useless.

tomchristie commented 7 years ago

Noted. It's possible that it'll be resolved as part of the 3.6 release (planned for Feb/Mar)

wrsulliv commented 7 years ago

Hey! Any further updates on this issue? I'm also looking to use swagger-codegen with descriptive model names (instead of "DataX").

wrsulliv commented 6 years ago

I needed a temporary solution, so I made the two pull requests above. They enable a user to specify a "title" field on the view (or viewset) which will then be used to generate a custom name on the swagger JSON (instead of "data").