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.57k stars 6.52k forks source link

[BUG][Typescript Axios] Content-type not set #2694

Open julaudo opened 5 years ago

julaudo commented 5 years ago
Description

Typescript axios generators does not handle correctly multipart requests. Content-Type header is never set.

openapi-generator version

4.0.0-beta3

OpenAPI declaration file content or url
openapi: 3.0.0

paths:
  'upload':
    post:
      summary: Upload File
      description: ''
      operationId: uploadFile
      requestBody:
        content:
          multipart/form-data:
            schema:
              $ref: '#/components/schemas/logoForCreation'
            encoding:
              medicalCertificateFileName:
                contentType: image/png, image/jpeg

        required:
          true
        description: body
      responses:
        201:
          description: Created or updated club's logo
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/photo'
components:
  schemas:
    logoForCreation:
      type: object
      required:
        - logoFileName
      properties:
        logoFileName:
          type: string
          format: binary
    photo:
      required:
        - location
      properties:
        location:
          type: string
Command line used for generation

openapi-generator generate -i swagger.yaml -g typescript-axios -o src/generated --skip-validate-spec

Steps to reproduce

Run the above command with the included yaml Notice that the following expected line is missing: localVarHeaderParameter['Content-Type'] = 'multipart/form-data';

Suggest a fix

multipartFormData mustache is used without vendorExtensions so it's not detected

auto-labeler[bot] commented 5 years ago

👍 Thanks for opening this issue! 🏷 I have applied any labels matching special text in your issue.

The team will review the labels and make any necessary changes.

vellengs commented 5 years ago

Before this issue PR, My test case is correct, but after that, I got an error of

error:"Internal Server Error"
message:"Failed to parse multipart servlet request; nested exception is java.io.IOException: org.apache.tomcat.util.http.fileupload.FileUploadException: the request was rejected because no multipart boundary was found"
path:"/rest/s/oem-configure/update"
status:500
timestamp:"2019-08-13 04:21:44"

There are someone saids the headers will auto set; https://stackoverflow.com/questions/17415084/multipart-data-post-using-python-requests-no-multipart-boundary-was-found

macjohnny commented 5 years ago

the SO answer relates to python. In https://stackoverflow.com/questions/41878838/how-do-i-set-multipart-in-axios-with-react the and https://github.com/axios/axios/issues/318#issuecomment-218948420 content type is automatically set when using FormData @vellengs would you like to file a PR that removes https://github.com/OpenAPITools/openapi-generator/pull/2695/files#diff-d270de95051055e6c877f904c360ecadR175 ? @nicokoenig can you help here?

macjohnny commented 5 years ago

this one is important to consider: https://github.com/axios/axios/issues/318#issuecomment-444827876

vellengs commented 5 years ago

if on nodejs side should use form.getHeaders() otherwise browser side will auto set.

because correct headers should like

'content-type':
      'multipart/form-data; boundary=--------------------------671856582217525463534855'

https://github.com/axios/axios/issues/1006#issuecomment-320165427

vellengs commented 5 years ago

Might use

    if((localVarFormParams as any).getHeaders){
      localVarHeaderParameter['Content-Type'] = (localVarFormParams as any).getHeaders();
    }

instead of

    localVarHeaderParameter['Content-Type'] = 'multipart/form-data';
sherlock1982 commented 3 years ago

Currently it should be like this:

        if((localVarFormParams as any).getHeaders){
            Object.assign(localVarHeaderParameter, (<any>localVarFormParams).getHeaders());
        }
        else {
            localVarHeaderParameter['Content-Type'] = 'multipart/form-data';
        }
ysfaran commented 2 years ago

Is there really no fix yet after 2 years?

The solution as mentioned by @vellengs / @sherlock1982 would also fix it in my case.

Any workaround? This is really bad. I would also like to pass a FormData object myself into the api method, which would allow for more flexibility in general..

macjohnny commented 2 years ago

@ysfaran do you want to provide a fix?

ysfaran commented 2 years ago

I actually digged a bit deeper into this topic and tried to add this feature, just to find out that there is already animplementation to fix this.

TL;DR

You can set the currently undocumented multipartFormData option in additionalProperies to true: https://github.com/OpenAPITools/openapi-generator/blob/9f3fac53c1689b82bf4c99b664e10e4a5decfb8e/bin/configs/typescript-axios-with-node-imports.yaml#L1-L7 withNodeImports is just relevant for NodeJS and you also need to install form-data package manually.

Detailed Findings

So while I was trying to add the proposed solution toe the mustache template I came across this line: https://github.com/OpenAPITools/openapi-generator/blob/9f3fac53c1689b82bf4c99b664e10e4a5decfb8e/modules/openapi-generator/src/main/resources/typescript-axios/apiInner.mustache#L199

which eventually generates a line of typescript if multipartFormData is set to true (as already mentioned above):

localVarRequestOptions.headers = {...localVarHeaderParameter, ...headersFromBaseOptions, ...(localVarFormParams as any).getHeaders?.(), ...options.headers};

Essentially it does the same thing as the proposed solution.

So I thought that there is just no documentation for multipartFormData and I can just file a PR to mention it in the README of typescript-axios. While trying to do so I noticed that this should actually be happening automatically already:

https://github.com/OpenAPITools/openapi-generator/blob/9f3fac53c1689b82bf4c99b664e10e4a5decfb8e/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/TypeScriptAxiosClientCodegen.java#L149-L163

The important part is this one:

operations.stream() 
                 .filter(op -> op.hasConsumes) 
                 .filter(op -> op.consumes.stream().anyMatch(opc -> opc.values().stream().anyMatch("multipart/form-data"::equals))) 
                 .forEach(op -> op.vendorExtensions.putIfAbsent("multipartFormData", true)); 

Here you can see that multipartFormData is automatically set to true if the corresponding operation from the input spec specified a consumes property with a value of multipart/form-data. An example input spec could contain following code:

paths:
   /upload:
     post:
       summary: Uploads a file.
       consumes:
         - multipart/form-data # the would automatically enable multipartFormData
       # ..

I noticed that this is an example for the OpenAPI spec 2.x but for 3.x (which is also what I use in my project) it would look like this:

 /upload:
    post:
      summary: Uploads a file.
      requestBody:
        content:
          multipart/form-data:
            schema:
            # ..

The fundamental difference here is that, OpenAPI 3.x doesn't use a consumes statement here anymore but requestBody instead.

My assumption here it that OpenAPI v3 is currently not supported, which is why I tried to add another check by traversing the properties of requestBody.content but I unfortunately couldn't find a way to get the requestBody of an operation similar to op.consumes (see current code). I am not sure if I was on the right track here though.

I'd happy to file a PR if someone could guide me here, since this is my first PR to the repo and I couldn't find a solution on my own.

macjohnny commented 2 years ago

Thanks for sharing this analysis! @wing328 do you have an idea?