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

[Python] Client code is wrong when an endpoint accepts multiple MediaTypes #440

Closed
raubel opened this issue Jul 3, 2018 · 12 comments · Fixed by #7427
Closed

[Python] Client code is wrong when an endpoint accepts multiple MediaTypes #440

raubel opened this issue Jul 3, 2018 · 12 comments · Fixed by #7427

Comments

@raubel
Copy link

raubel commented Jul 3, 2018

Description

My OpenApi 3.0 specification contains an endpoint for which the response media type can be application/octet-stream or, if the query cannot be completed, application/json.
The generated Python client code builds a query with a MediaType being the concatenation of both: application/octet-streamapplication/json

openapi-generator version

3.0.3

OpenAPI declaration file content or url

Below is my (simplified) OpenApi specification:

openapi: 3.0.0

servers:
- url: http://localhost/myapi

info:
  version: "1.0"
  title: My API

security:
- basicAuth: []

paths:

  /download:
    get:
      operationId: download
      responses:
        200:
          description: OK
          content:
            application/octet-stream:
              schema:
                type: string
                format: binary
        500:
          description: Error
          content:
            application/json:
              schema:
                type: string
Command line used for generation

I call the generator directly from my Java application:

	CodegenConfigurator configurator = new CodegenConfigurator();
	configurator.setInputSpec("spec.yml");
	configurator.setGeneratorName("python");
	configurator.setOutputDir("out");
	new DefaultGenerator().opts(configurator.toClientOptInput()).generate();
Python test file

Running this script with the generated openapi_client

import unittest
import openapi_client

configuration = openapi_client.Configuration()
configuration.debug = True

# create an instance of the API class
api_instance = openapi_client.DefaultApi(openapi_client.ApiClient(configuration))

class TestModels(unittest.TestCase):
    def test_download(self):
        api_instance.download()

if __name__ == '__main__':
    unittest.main()

results in the following output:

2018-07-02 15:17:38,841 DEBUG Starting new HTTP connection (1): localhost
send: 'GET /myapi HTTP/1.1\r\nHost: localhost:10080\r\nAccept-Encoding: identity\r\nContent-Type: application/json\r\nAccept: application/octet-streamapplication/json\r\nAuthorization: Basic c2VtYWRtaW46c2VtYWRtaW4=\r\nUser-Agent: OpenAPI-Generator/1.0.0/python\r\n\r\n'
reply: 'HTTP/1.1 406 Not Acceptable\r\n'
...
@ackintosh
Copy link
Contributor

@raubel Thanks for reporting the issue!

It has fixed in latest release v3.1.0, so please give it a try with latest version. 😉

(Related PR: #445 📝 )

@raubel
Copy link
Author

raubel commented Jul 9, 2018

@ackintosh, unfortunately, it does not seem to be fixed in 3.1.0. I still have an Accept header being application/octet-streamapplication/json.

@ackintosh
Copy link
Contributor

ackintosh commented Jul 9, 2018

Oh... 😢

Could you please check the generated file default_api.py?

(Below is part of files generated from the simplified spec)

├── openapi_client
│   ├── __init__.py
│   ├── api
│   │   ├── __init__.py
│   │   ├── default_api.py <<====
│   ├── api_client.py
│   ├── configuration.py
│   ├── models
│   │   ├── __init__.py
│   ├── rest.py

There should be the difference between 3.0.3 and 3.1.0 on default_api.py like below:

3.0.3

L96

        body_params = None
        # HTTP header `Accept`
        header_params['Accept'] = self.api_client.select_header_accept(
            ['application/octet-stream''application/json'])  # noqa: E501

(A comma to be present between 'application/octet-stream' and 'application/json' is missing)

3.1.0

L96

        body_params = None
        # HTTP header `Accept`
        header_params['Accept'] = self.api_client.select_header_accept(
            ['application/octet-stream', 'application/json'])  # noqa: E501

What about your default_api.py?

@raubel
Copy link
Author

raubel commented Jul 9, 2018

The generated code seems correct in default_api.py.

However, the code in api_client.py is suspicious:

    def select_header_accept(self, accepts):
        """Returns `Accept` based on an array of accepts provided.

        :param accepts: List of headers.
        :return: Accept (e.g. application/json).
        """
        if not accepts:
            return

        accepts = [x.lower() for x in accepts]

        if 'application/json' in accepts:
            return 'application/json'
        else:
            return ', '.join(accepts)

It removes all media types except application/json. It this correct?

That's why I still have a 406 (Not accepted) status code.
(BTW, I'm not able to retrieve the application/octet-streamapplication/json (I was probably looking at the wrong log file).)

@ackintosh
Copy link
Contributor

@raubel
Copy link
Author

raubel commented Jul 24, 2018

Any update?

@ackintosh
Copy link
Contributor

Sorry for late reply. 💦

It removes all media types except application/json. It this correct?

Maybe it is not good. (but currently most of the generated clients (even other language) has same issue.)

My understanding about why the API client removes all media types except application/json is...

API client treats the response data as JSON so API client sets application/json to Accept header preferentially.

My idea

I think Quality values will be good solution.

The following Accept header allows all media types and request that the response be returned as application/json.

Accept: application/json,application/octet-stream;q=0.9

The solution requires some works (template improvement), I'll file a PR if the idea gets someone's 👍 .

@cbornet
Copy link
Member

cbornet commented Oct 31, 2018

Instead of removing mediatypes from Accept, we should just put application/json in first position.
Then when we get the response, check if the content-type is application/json and only deserialize if it's the case.

@jirikuncar
Copy link
Contributor

jirikuncar commented Dec 19, 2019

There is a same problem in Go(-experimental).

// selectHeaderAccept join all accept types and return
func selectHeaderAccept(accepts []string) string {
	if len(accepts) == 0 {
		return ""
	}

	if contains(accepts, "application/json") {
		return "application/json"
	}

	return strings.Join(accepts, ",")
}

when using a param with Accept: application/json; param=foo only the application/json type from 300+ responses gets picked up.

      responses:
        "200":
          description: OK
          content:
            application/json;param=foo:
              schema:
                $ref: "#/components/schemas/OK"
        "400":
          description: Bad Request
          content:
            application/json:
              schema:
                $ref: "#/components/schemas/Error400"

I would propose to remove the special treatment for application/json.

@ymilki
Copy link

ymilki commented Dec 23, 2021

@spacether I don't believe #7427 fixes this issue as reported. #7427 fixes the python data type of the return value of responses (e.g. Is the return value a primitive type or model).

This issue addresses the content-type format of the raw response (e.g. is the response "application/json" or "text/plain"). This format has implications in response deserialization. @ackintosh in #728 identifies this issue from the spec perspective (the Accept header should not be only "application/json"), but as mentioned earlier in #440 (comment), the deserialization code must also understand how to read the response.

@ymilki
Copy link

ymilki commented Jan 5, 2022

I see #10978 explicitly passes the response content-type around which provides the framework to the client code to handle different content-types.

@spacether
Copy link
Contributor

spacether commented Jan 5, 2022

This code has been cleaned up in python-experimental. For client endpoints:

  • one can input content_type to control the serialization of transmitted request bodies to the server
  • one can input a tuple of custom accept_content_types to control which response type(s) the server should send back

One can now use python-experimental; it was added in #8325

nilskuhn pushed a commit to nilskuhn/openapi-generator that referenced this issue Apr 6, 2023
chore(deps): update dependency @types/jest to v27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants
@raubel @jirikuncar @ackintosh @spacether @ymilki @cbornet and others