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.68k stars 6.55k forks source link

[BUG] [PYTHON] Malformed type hints on 6.3.0-SNAPSHOT with 303 status response #14009

Open syntaxaire opened 1 year ago

syntaxaire commented 1 year ago

Bug Report Checklist

Description

Clients are generated with syntax errors using the spec provided.

Sample of code with syntax errors:

class ThingDownload(BaseApi):
    # this class is used by api classes that refer to endpoints with operationId fn names

    @typing.overload
    def thing_download(
        self,
        path_params: RequestPathParams = frozendict.frozendict(),
        accept_content_types: typing.Tuple[str] = _all_accept_content_types,
        stream: bool = False,
        timeout: typing.Optional[typing.Union[int, typing.Tuple]] = None,
        skip_deserialization: typing_extensions.Literal[False] = ...,
    ) -> typing.Union[
    ]: ...

It is not valid for this typing.Union to be empty, as shown at import time:

E       ) -> typing.Union[
E                        ^
E   SyntaxError: expected ':'
openapi-generator version
openapi-generator-cli 6.3.0-SNAPSHOT
  commit : 188c39d
  built  : 2022-11-11T16:58:19-05:00
  source : https://github.com/openapitools/openapi-generator
  docs   : https://openapi-generator.tech/
OpenAPI declaration file content or url

Gist: https://gist.github.com/syntaxaire/9e584c63ac4ce71c7eda882a4d6837dc

Generation Details

java -jar openapi-generator-cli.jar generate --global-property skipFormModel=false -i empty-union.json -g python -o empty-union

Steps to reproduce
syntaxaire commented 1 year ago

Copy of the minimal reproduction from the gist:

{
    "openapi": "3.0.0",
    "info": {
        "title": "API Documentation",
        "description": "Test documentation",
        "contact": {
            "name": "Support",
            "email": "support@example.com"
        },
        "version": "v4",
        "x-logo": {
            "url": "https://example.com/logo.png"
        }
    },
    "servers": [
        {
            "url": "https://app.example.com/api/v4",
            "description": "Server"
        }
    ],
    "paths": {
        "/things/{id}/download.json": {
            "get": {
                "tags": [
                    "Things"
                ],
                "summary": "Download the Thing",
                "operationId": "Thing#download",
                "description": "Download the completed Thing",
                "parameters": [
                    {
                        "name": "id",
                        "in": "path",
                        "description": "The unique identifier for the Thing",
                        "required": true,
                        "schema": {
                            "type": "integer",
                            "format": "int64"
                        }
                    }
                ],
                "responses": {
                    "303": {
                        "description": "See Other"
                    },
                    "400": {
                        "description": "Bad Request",
                        "content": {
                            "application/json": {}
                        }
                    },
                    "401": {
                        "description": "Unauthorized",
                        "content": {
                            "application/json": {}
                        }
                    },
                    "404": {
                        "description": "Not Found",
                        "content": {
                            "application/json": {}
                        }
                    },
                    "429": {
                        "description": "Too Many Requests",
                        "content": {
                            "application/json": {}
                        }
                    }
                }
            }
        }
    },
    "tags": [
        {
            "name": "Things",
            "description": "A bunch of things"
        }
    ]
}
spacether commented 1 year ago

Hi there. This is happening because this endpoint does not have any 2XX response status code. Per the spec here: https://github.com/OAI/OpenAPI-Specification/blob/main/versions/3.0.3.md#responsesObject The Responses Object MUST contain at least one response code, and it SHOULD be the response for a successful operation call. Does this endpoint only return 303 and 4XX http status codes?

Some ways to fix this are:

Some more context here: The openapi spec isn't clear about how redirects should be handled per: https://github.com/OAI/OpenAPI-Specification/issues/2512

We have open issues on redirects in other generators also:

syntaxaire commented 1 year ago

Thanks spacether, I'm reaching out to the upstream API provider to find out whether this is an oversight or whether there is really no 2xx response for this endpoint.

spacether commented 1 year ago

Welcome :) Glad to hear it. Even if a 2XX is never returned, adding a dummy 2XX response with no content schema info is an option because it will generate working code.

syntaxaire commented 1 year ago

I heard back from the API provider. 303 is the appropriate code for these operations. The 303 response provides an AWS URL which the client must follow to receive the requested download. AWS then provides a 200 response code when the file is downloaded.

spacether commented 1 year ago

Okay, so do you want to add a dummy 200 response, or write a PR to handle the 3XX responses? If you add the dummy 2XX response, you will still need to extract the response header from the urllib3 raw response api_response.response to find out where the 303 wants you to go and act on it accordingly.

Note: The 3XX response issue is already tracked in my separate repo also, https://github.com/openapi-json-schema-tools/openapi-json-schema-generator/issues/23

syntaxaire commented 1 year ago

This is for work, so unfortunately I don't think it's realistic for me to change scope into Java to write a PR. The upstream spec changes daily, so I can't just write a patch for it that will stay current. The solution for me might be to freeze one of the dailies of the spec, edit in the responses, and only update when absolutely necessary.

spacether commented 1 year ago

You could write a python file that opens the spec, finds the path and response and writes a new spec with the additional 200 response. Or you can write a post processing program that fixes the client and pass it in to the generate command as a post-processing step

abhi1693 commented 1 year ago

@spacether I have the exact same issue but my upstream API has 204 instead of 200

image

spacether commented 1 year ago

@abhi1693 can you please include a minimal spec to reproduce your issue? When I generate this endpoint:

  /fake/responseWithOnly204Status:
    get:
      operationId: responseWithOnly204Status
      summary: response with only 204 status
      tags:
        - fake
      responses:
        204:
          description: success
          content:
            application/json:
              schema: {}

I get this code:

_response_for_204 = api_client.OpenApiResponse(
    response_cls=ApiResponseFor204,
    content={
        'application/json': api_client.MediaType(
            schema=SchemaFor204ResponseBodyApplicationJson),
    },
)
_status_code_to_response = {
    '204': _response_for_204,
}

class BaseApi(api_client.Api):
    @typing.overload
    def _response_with_only204_status_oapg(
        self,
        accept_content_types: typing.Tuple[str] = _all_accept_content_types,
        stream: bool = False,
        timeout: typing.Optional[typing.Union[int, typing.Tuple]] = None,
        skip_deserialization: typing_extensions.Literal[False] = ...,
    ) -> typing.Union[
        ApiResponseFor204,
    ]: ...

    @typing.overload
    def _response_with_only204_status_oapg(
        self,
        skip_deserialization: typing_extensions.Literal[True],
        accept_content_types: typing.Tuple[str] = _all_accept_content_types,
        stream: bool = False,
        timeout: typing.Optional[typing.Union[int, typing.Tuple]] = None,
    ) -> api_client.ApiResponseWithoutDeserialization: ...

    @typing.overload
    def _response_with_only204_status_oapg(
        self,
        accept_content_types: typing.Tuple[str] = _all_accept_content_types,
        stream: bool = False,
        timeout: typing.Optional[typing.Union[int, typing.Tuple]] = None,
        skip_deserialization: bool = ...,
    ) -> typing.Union[
        ApiResponseFor204,
        api_client.ApiResponseWithoutDeserialization,
    ]: ...

    def _response_with_only204_status_oapg(
        self,
        accept_content_types: typing.Tuple[str] = _all_accept_content_types,
        stream: bool = False,
        timeout: typing.Optional[typing.Union[int, typing.Tuple]] = None,
        skip_deserialization: bool = False,
    ):

Using the current master branch. So I am unable to replicate your issue.

abhi1693 commented 1 year ago

@spacether You can use the spec defined at https://demo.netbox.dev/api/docs/?format=openapi. My command looks like

config.json

{
  "generateAliasAsModel": "true",
  "packageAuthor": "Abhimanyu Saharan",
  "packageDescription": "NetBox Client is a Python library for interacting with the NetBox API.",
  "packageName": "netbox_client",
  "packageVersion": "0.0.1",
  "useInlineModelResolver": "true"
}
curl -s -o ./openapi.json https://demo.netbox.dev/api/docs/?format=openapi
openapi-generator-cli generate --config config.json --generator-name python --input-spec openapi.json

One of class that generated the incorrect type

@dataclass
class ApiResponseFor204(api_client.ApiResponse):
    response: urllib3.HTTPResponse
    body: typing.Union[
    ]
    headers: schemas.Unset = schemas.unset
spacether commented 1 year ago

FYI this is now working in the v2 release of openapi-json-schema-generator. That is where development of the python generator is continuing. See this endpoint that has a redirection response:

  /fake/redirection:
    get:
      operationId: redirection
      summary: operation with redirection responses
      tags:
        - fake
      responses:
        303:
          description: see other
        3XX:
          description: 3XX response

See a working test of http status code 303 response deserialization here

    @patch.object(urllib3.PoolManager, 'request')
    def test_non_wildcard_response(self, mock_request):
        mock_request.return_value = self.response(b'', status=303)

        api_response = self.api.get()
        self.assert_pool_manager_request_called_with(
            mock_request,
            f'http://petstore.swagger.io:80/v2/fake/redirection',
            body=None,
            method='GET',
            content_type=None,
            accept_content_type=None,
        )

        assert isinstance(api_response, get.response_303.ResponseFor303.response_cls)
        assert isinstance(api_response.response, urllib3.HTTPResponse)
        assert isinstance(api_response.body, schemas.Unset)
        assert isinstance(api_response.headers, schemas.Unset)
        assert api_response.response.status == 303

Note: ranged response redirection deserialization also works for 3XX per this other test