Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

New issue

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

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

Already on GitHub? # to your account

Support multiple content types for a single endpoint #453

Closed
dpursehouse opened this issue Jul 11, 2021 · 11 comments
Closed

Support multiple content types for a single endpoint #453

dpursehouse opened this issue Jul 11, 2021 · 11 comments
Labels
✨ enhancement New feature or improvement 🍭 OpenAPI Compliance Supporting a new bit of the OpenAPI spec

Comments

@dpursehouse
Copy link

With an openapi yaml schema that has this content:

      requestBody:
        content:
          application/json:
            schema:
              type: array
              items:
                $ref: '#/components/schemas/REDACTED'
          application/x-www-form-urlencoded:
            schema:
              type: array
              items:
                $ref: '#/components/schemas/REDACTED'
          multipart/form-data:
            schema:
              type: array
              items:
                $ref: '#/components/schemas/REDACTED'
        required: true

The generated client has this method:

def _get_kwargs(
    *,
    client: AuthenticatedClient,
    multipart_data: List[REDACTED],
    json_body: List[REDACTED],
) -> Dict[str, Any]:
    url = "{}/api/redacted/".format(client.base_url)

    headers: Dict[str, Any] = client.get_headers()
    cookies: Dict[str, Any] = client.get_cookies()

    json_json_body = []
    for json_body_item_data in json_body:
        json_body_item = json_body_item_data.to_dict()

        json_json_body.append(json_body_item)

    multipart_multipart_data = []
    for multipart_data_item_data in multipart_data:
        multipart_data_item = multipart_data_item_data.to_dict()

        multipart_multipart_data.append(multipart_data_item)

    return {
        "url": url,
        "headers": headers,
        "cookies": cookies,
        "timeout": client.get_timeout(),
        "files": multipart_multipart_data,
    }

The diff from the client generated by version 0.9.2 is that it now takes a multipart_data parameter which is now used to populate the files field on the returned dict.

However the json field was removed and the json_body parameter is thus unused. This breaks clients that are passing the json_body field.

Adding it back fixes it, i.e.

    return {
        "url": url,
        "headers": headers,
        "cookies": cookies,
        "timeout": client.get_timeout(),
        "json": json_json_body,          # <--- HERE
        "files": multipart_multipart_data,
    }
@dbanty
Copy link
Collaborator

dbanty commented Jul 12, 2021

Interesting, I don’t think we’ve ever added (at least intentionally) support for multiple content types in one endpoint. How do you think this would be most ergonomic in the generated client? Only one content type can actually be sent so the function definition should indicate that somehow.

Should there be a separate function per content type? Or maybe function overloading?

@dbanty dbanty added ✨ enhancement New feature or improvement and removed 🐞bug Something isn't working labels Jul 12, 2021
@dbanty dbanty changed the title json field is omitted from returned dict in _get_kwargs Support multiple content types for a single endpoint Jul 12, 2021
@dpursehouse
Copy link
Author

Maybe make both parameters optional, and then raise an exception if both are set, or if neither is set?

@dpursehouse
Copy link
Author

@dbanty it turns out that this could be solved by explicitly setting parser_classes = [JSONParser] in the Django view, which prevents the schema generated by drf-spectacular from including the multiple content types.

@dbanty
Copy link
Collaborator

dbanty commented Aug 23, 2021

Glad you found a solution @dpursehouse! Would still love to add support for this some day, I'm sure there are others who would benefit from it.

@johnthagen
Copy link
Collaborator

johnthagen commented Nov 9, 2021

@dpursehouse

it turns out that this could be solved by explicitly setting parser_classes = [JSONParser] in the Django view,

Do you know if this breaks the DRF Web browsable API? (e.g https://www.django-rest-framework.org/ | Web Browsable API: https://restframework.herokuapp.com/) I assumed that the form_data portion was used by the browsable API, and disabling form_data could have unexpected consequences?

@johnthagen
Copy link
Collaborator

johnthagen commented Nov 9, 2021

However the json field was removed and the json_body parameter is thus unused. This breaks clients that are passing the json_body field.

@dbanty It seems like this portion might be a bug in its own right.

When I wanted to send json_body information in a Patched request (all fields are optional and default to UNSET in the generated client code), this didn't work:

        t_partial_update.sync(
            metal.id,
            client=client,
            json_body=T(type="RED"),
            form_data=T(),
            multipart_data=T(),
        )

I opened up Wireshark on localhost interface to confirm and all of the PATCH requests that are sent are blank (no data).

I had to move it to the form_data parameter to get the data to be sent, but this seems a bit misleading/counter intuitive:

        t_partial_update.sync(
            metal.id,
            client=client,
            json_body=T(),
            form_data=T(type="RED"),
            multipart_data=T(),
        )

@dbanty
Copy link
Collaborator

dbanty commented Nov 9, 2021

I assume this is a limitation in httpx but it'd take some testing to be sure. Sending both a form and JSON is quite unusual. I'm guessing it just picks one to send.

@johnthagen

This comment has been minimized.

@johnthagen
Copy link
Collaborator

drf-spectacular 0.21.0 has a new feature to support excluding certain parsers (content types) to help work around this issue:

SPECTACULAR_SETTINGS = {
    "PARSER_WHITELIST": ["rest_framework.parsers.JSONParser"],
 }

@davidlizeng
Copy link

We have an API endpoint where we support uploading a file either as multipart/form-data or by specifying an application/json body that includes urls to the files. Sample spec:

{
  "openapi": "3.0.0",
  "paths": {
    "/api/files/upload": {
      "post": {
        "operationId": "uploadFile",
        "summary": "Upload a file by multipart or by url",
        "requestBody": {
          "content": {
            "multipart/form-data": {
              "schema": {
                "type": "object",
                "properties": {
                  "file": {
                    "type": "string",
                    "format": "binary"
                  }
                }
              }
            },
            "application/json": {
              "schema": {
                "$ref": "#/components/schemas/UrlUploadDto"
              }
            }
          },
          "required": true
        },
        "parameters": [],
        "responses": {
          "201": {
            "description": "Returns some random string",
            "content": {
              "application/json": {
                "schema": {
                  "type": "string"
                }
              }
            }
          }
        },
        "tags": [
          "files"
        ]
      }
    }
  },
  "info": {
    "title": "Single Upload",
    "description": "Test spec for files in multipart/form-data or application/json",
    "version": "0.0.1",
    "contact": {}
  },
  "tags": [],
  "servers": [],
  "components": {
    "schemas": {
      "UrlUploadDto": {
        "type": "object",
        "properties": {
          "url": {
            "type": "string",
            "description": "URL to download the file from using a GET request"
          },
          "fileName": {
            "type": "string",
            "description": "Filename to save for the file. If empty, will use the final portion of the URL path"
          },
          "mimeType": {
            "type": "string",
            "description": "Mime type of file. If empty, will use the response content type of the download"
          }
        },
        "required": [
          "url"
        ]
      }
    }
  }
}

Would love to have a feature where the codegen client would be able to support functions for both the multipart/form-data request as well as the application/json request. The current behavior will handle the multipart/form-data correctly, but the application/json requests will not work as the json body is dropped during _get_kwargs.

In my case, having entirely separate functions for each request would be preferred, as it feels like the interface is strongly typed and most clear. It would mean we'd need to generate names that may look a little long:

  • upload_file_by_multipart_form_data.py
  • upload_file_by_application_json.py
    The general pattern would be to roughly look like: f"{operation_id}_by_{request_content_type}".

This would also mean that we no longer have a 1:1 relationship between the api files and the actual paths in the OpenAPI spec. I'm not sure if there is anything that relies on such an assumption?

Would be happy to contribute this feature, but did want to see if it the approach of separate functions makes sense, and is something that the community would be interested in having?

@edmundsj
Copy link

edmundsj commented Nov 23, 2022

EDIT: While the below suggestion provides a potential fix, I see this as more of an issue with drf-spectacular generating invalid schemas than an issue with openapi-generator. If we want openapi-generator to compensate for somewhat-nonsensical schemas, then this fix makes sense to me.

I have also run facefirst into this issue multiple times. I have a schema that is being generated by drf-spectacular for the purposes of uploading images, and it has multiple content types in the request body:

      requestBody:
        content:
          application/json:
            schema:
              $ref: '#/components/schemas/DOImageRequest'
          application/x-www-form-urlencoded:
            schema:
              $ref: '#/components/schemas/DOImageRequest'
          multipart/form-data:
            schema:
              $ref: '#/components/schemas/DOImageRequest'
        required: true

Unfortunately, openapi-generator only uses the first of these definitions, so I am stuck with this auto-generated client function:

async communityImagesCreateRaw(requestParameters: CommunityImagesCreateRequest, initOverrides?: RequestInit | runtime.InitOverrideFunction): Promise<runtime.ApiResponse<DOImage>> {
        if (requestParameters.dOImageRequest === null || requestParameters.dOImageRequest === undefined) {
            throw new runtime.RequiredError('dOImageRequest','Required parameter requestParameters.dOImageRequest was null or undefined when calling communityImagesCreate.');
        }

        const queryParameters: any = {};

        const headerParameters: runtime.HTTPHeaders = {};

        headerParameters['Content-Type'] = 'application/json';

        const response = await this.request({
            path: `/api/community/images/`,
            method: 'POST',
            headers: headerParameters,
            query: queryParameters,
            body: DOImageRequestToJSON(requestParameters.dOImageRequest),
        }, initOverrides);

        return new runtime.JSONApiResponse(response, (jsonValue) => DOImageFromJSON(jsonValue));
    }

Doh! It's trying to convert an image into JSON - clearly not what I am looking for. I have had to avoid using my auto-generated client any time I perform image uploads for this reason and manually submit data via formData. It's the only place I am not using the generated client and is a glaring violation of DRY.

However, most of the content request bodies in my schema have multiple entries in the "content" section, and I only want to specify that formData be uploaded for the image not the myriad of other models I have for which JSON works just fine.

The most elegant way I can think to do this without breaking basic functionality is this algorithm:

  1. When multiple content-types are chosen, choose the first one (current algorithm) UNLESS
  2. The component schema referred to contains a property whose "format" is "binary" and the first content-type does not support this (i.e. application/json). In that case, look for the first content-type which DOES support a binary format in the data (i.e multipart/form-data).

@dbanty dbanty added the 🍭 OpenAPI Compliance Supporting a new bit of the OpenAPI spec label Aug 13, 2023
@openapi-generators openapi-generators locked and limited conversation to collaborators Aug 13, 2023
@dbanty dbanty converted this issue into discussion #822 Aug 13, 2023

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
✨ enhancement New feature or improvement 🍭 OpenAPI Compliance Supporting a new bit of the OpenAPI spec
Projects
None yet
Development

No branches or pull requests

5 participants