Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
5 of 6 tasks
syntaxaire opened this issue Nov 14, 2022 · 12 comments
Open
5 of 6 tasks

Comments

@syntaxaire
Copy link

syntaxaire commented Nov 14, 2022

Bug Report Checklist

  • Have you provided a full/minimal spec to reproduce the issue?
  • Have you validated the input using an OpenAPI validator (example)?
  • Have you tested with the latest master to confirm the issue still exists?
  • Have you searched for related issues/PRs?
  • What's the actual output vs expected output?
  • [Optional] Sponsorship to speed up the bug fix or feature request (example)
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
  • Download the spec from the provided gist.
  • Use the latest git version with the command line above to generate the client
  • Install the client and run its unit tests, the above syntax error will be shown when imported
@syntaxaire
Copy link
Author

Copy of the minimal reproduction from the gist:

{
    "openapi": "3.0.0",
    "info": {
        "title": "API Documentation",
        "description": "Test documentation",
        "contact": {
            "name": "Support",
            "email": "[email protected]"
        },
        "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
Copy link
Contributor

spacether commented Nov 14, 2022

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:

  • submit a PR adding the ability to detect no response body for 3XX and handling 3XX responses
  • add a 2XX response to your spec with no content in it (this will fix the type hint)

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

We have open issues on redirects in other generators also:

@syntaxaire
Copy link
Author

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
Copy link
Contributor

spacether commented Nov 14, 2022

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
Copy link
Author

syntaxaire commented Nov 15, 2022

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
Copy link
Contributor

spacether commented Nov 16, 2022

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, openapi-json-schema-tools/openapi-json-schema-generator#23

@syntaxaire
Copy link
Author

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
Copy link
Contributor

spacether commented Nov 16, 2022

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

@spacether spacether changed the title [BUG] [PYTHON] Malformed type hints on 6.3.0-SNAPSHOT Python generator [BUG] [PYTHON] Malformed type hints on 6.3.0-SNAPSHOT with 303 status response Nov 18, 2022
@abhi1693
Copy link

abhi1693 commented Dec 4, 2022

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

image

@spacether
Copy link
Contributor

@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
Copy link

@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
Copy link
Contributor

spacether commented Apr 11, 2023

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants