[BUG][KOTLIN-SPRING] Multipart file upload generate wrong type

notizklotz commented 3 years ago

Bug Report Checklist

[x] Have you provided a full/minimal spec to reproduce the issue?
[x] Have you validated the input using an OpenAPI validator (example)?
[x] Have you tested with the latest master to confirm the issue still exists?
[x] Have you searched for related issues/PRs?
[x] What's the actual output vs expected output?
[ ] [Optional] Sponsorship to speed up the bug fix or feature request (example)

Description

The kotlin-spring generator generates multipart file upload attributes as org.springframework.core.io.Resource which actually cannot be handled by Spring Boot. org.springframework.web.multipart.MultipartFile should be generated instead: Uploading files guide

The spring generator handles this case correctly by applying some special case handling in the Mustache templates.

openapi-generator version

openapi-generator-maven-plugin:5.0.0

OpenAPI declaration file content or url

Multipart file upload sample spec

Steps to reproduce

Generate server code using the kotlin-spring generator
Generate server code using the spring generator
Note that the spring generator generates the file paramter as List<MultipartFile> while the kotlin-spring generator generates List<Resource>

Related issues/PRs

Suggest a fix

Apply the same special case handling in the templates like the spring generator

wing328 commented 3 years ago

@notizklotz thanks for reporting the issue. Can you please file a PR with the suggested fix in the spring generator when you've time? We'll review accordingly

notizklotz commented 3 years ago

Hi, yes I'll prepare a PR

sylvainmoindron commented 3 years ago

back in 2019 the générator switched to Resources (my bad https://github.com/OpenAPITools/openapi-generator/pull/2455) Since MultipartFile don't exist in reactive spring.

we should use MultipartFile whith spring web and FilePart with webflux

koscejev commented 2 years ago

I stumbled upon the same problem for reactive. Indeed, FilePart can be provided by Spring's org.springframework.http.codec.multipart.DefaultPartHttpMessageReader, but this reader actually implements HttpMessageReader<Part> and it provides FilePart only if the Content-Disposition header contains filename. So the API must use Part, not FilePart and actual implementation logic (outside of the generator scope) can check whether it's FilePart.

Next, the problem is with the mentioned type - Part instead of Resource is already used correctly by Java Spring generator (except for arrays of files, where it uses List<Flux<Part>> for some reason - but that's a separate bug). But Kotlin Spring generator currently just uses Resource, which has no way of working. Because of suspend usage, the type should be Part for a single and Flow<Part> for multiple. But Flow<Part> doesn't work for some strange reason (it fails on some internal Spring lookup of the correct Reader and I don't understand the underlying problem in detail unfortunately), while Flux<Part> actually works (verified on our project via customizing templates).

So I propose changing it (at least for now) to Flux<Part> (even though Kotlin's Flow would normally be used here) when dealing with an array of file parts, and just Part when dealing with single file parts. There are still some unknowns - I haven't verified some more advanced usages, such as multiple multipart file fields (I've only verified single array file field).

@wing328 If this is OK, I can maybe try introducing the proper fix (not just template change) that would change Resource to Part and use Flux<Part> for an array of files - and submit PR to 6.0.x (since it'll be a breaking change).

m-i-k-e-e commented 1 year ago

Hey Guys,

It works with resources but you have to adapt the RequestPart parameter. By default it generates @RequestPart("file") but using _@RequesPart("my_paramname") worked for me.

moehringn commented 1 year ago

I have the same issue, trying to upload multiple files within the same request. kotlin-spring generator does not respect the property name for the file:

      CreateMediaRequest:
        type: object
        properties:
          name:
            type: string
          type:
            $ref: '#/components/schemas/MediaType'
          file:
            type: string
            format: binary
          thumbnail:
            type: string
            format: binary

  fun createMediaPost(
    @Parameter(
      description = ""
    ) @RequestParam(value = "name", required = true) name: kotlin.String,
    @Parameter(
      description = "",
    ) @RequestParam(value = "type", required = true) type: MediaType,
    @Parameter(
      description = "file detail"
    ) @Valid @RequestPart("file") thumbnail: org.springframework.core.io.Resource,        
    @Parameter(
      description = "file detail"
    ) @Valid @RequestPart("file") file: org.springframework.core.io.Resource?
  ): ResponseEntity<MediaView> {
    return getDelegate().createMediaPost(name, type, thumbnail, file)
  }

As you can see, both @RequestPart("file") have the value "file" instead of "file" and "thumbnail"

@m-i-k-e-e how did you manage to set "my_param_name" in there?

pbabu0192 commented 11 months ago

I ran into the same issue with multipart and kotlin-spring generator @moehringn did you manage to fix this issue? @m-i-k-e-e Could you please elaborate your solution?

dds-pereihug commented 3 months ago

one suggestion as a fix, could be:

on formParams.mustache {{#isFormParam}} {{^isFile}}{{#swagger2AnnotationLibrary}}@Parameter(description = "{{{description}}}"{{#required}}, required = true{{/required}}{{#allowableValues}}{{#defaultValue}}, schema = Schema(allowableValues = [{{#values}}"{{{.}}}"{{^-last}}, {{/-last}}{{/values}}]{{^isContainer}}, defaultValue = {{^isString}}"{{/isString}}{{#isString}}{{#isEnum}}"{{/isEnum}}{{/isString}}{{{defaultValue}}}{{^isString}}"{{/isString}}{{#isString}}{{#isEnum}}"{{/isEnum}}{{/isString}}{{/isContainer}}){{/defaultValue}}{{/allowableValues}}{{#allowableValues}}{{^defaultValue}}, schema = Schema(allowableValues = [{{#values}}"{{{.}}}"{{^-last}}, {{/-last}}{{/values}}]){{/defaultValue}}{{/allowableValues}}{{^allowableValues}}{{#defaultValue}}{{^isContainer}}, schema = Schema(defaultValue = {{^isString}}"{{/isString}}{{#isString}}{{#isEnum}}"{{/isEnum}}{{/isString}}{{{defaultValue}}}{{^isString}}"{{/isString}}{{#isString}}{{#isEnum}}"{{/isEnum}}{{/isString}}){{/isContainer}}{{/defaultValue}}{{/allowableValues}}){{/swagger2AnnotationLibrary}}{{#swagger1AnnotationLibrary}}@ApiParam(value = "{{{description}}}"{{#required}}, required = true{{/required}}{{#allowableValues}}, allowableValues = "{{#values}}{{{.}}}{{^-last}}, {{/-last}}{{/values}}"{{/allowableValues}}{{#defaultValue}}, defaultValue = "{{{.}}}"{{/defaultValue}}){{/swagger1AnnotationLibrary}} @RequestParam(value = "{{baseName}}"{{#required}}, required = true{{/required}}{{^required}}, required = false{{/required}}) {{{paramName}}}: {{>optionalDataType}} {{/isFile}} {{#isFile}}{{#swagger2AnnotationLibrary}}@Parameter(description = "{{{description}}}"){{/swagger2AnnotationLibrary}}{{#swagger1AnnotationLibrary}}@ApiParam(value = "file detail"){{/swagger1AnnotationLibrary}} {{#useBeanValidation}}@Valid{{/useBeanValidation}} @RequestPart("{{baseName}}"{{#required}}, required = true{{/required}}{{^required}}, required = false{{/required}}) {{{paramName}}}: MultipartFile{{/isFile}} {{/isFormParam}}
on apiInterface.mustache, just add the following import import org.springframework.web.multipart.MultipartFile

OlgaErmolaeva commented 2 months ago

@dds-pereihug Thank you so much for your solution. It works. But I didn't add import org.springframework.web.multipart.MultipartFile in apiInterface.mustache. Instead I used the fully qualified class name in the formParams.mustache.

{{#isFormParam}} {{^isFile}}{{#swagger2AnnotationLibrary}}@Parameter(description = "{{{description}}}"{{#required}}, required = true{{/required}}{{#allowableValues}}{{#defaultValue}}, schema = Schema(allowableValues = [{{#values}}"{{{.}}}"{{^-last}}, {{/-last}}{{/values}}]{{^isContainer}}, defaultValue = {{^isString}}"{{/isString}}{{#isString}}{{#isEnum}}"{{/isEnum}}{{/isString}}{{{defaultValue}}}{{^isString}}"{{/isString}}{{#isString}}{{#isEnum}}"{{/isEnum}}{{/isString}}{{/isContainer}}){{/defaultValue}}{{/allowableValues}}{{#allowableValues}}{{^defaultValue}}, schema = Schema(allowableValues = [{{#values}}"{{{.}}}"{{^-last}}, {{/-last}}{{/values}}]){{/defaultValue}}{{/allowableValues}}{{^allowableValues}}{{#defaultValue}}{{^isContainer}}, schema = Schema(defaultValue = {{^isString}}"{{/isString}}{{#isString}}{{#isEnum}}"{{/isEnum}}{{/isString}}{{{defaultValue}}}{{^isString}}"{{/isString}}{{#isString}}{{#isEnum}}"{{/isEnum}}{{/isString}}){{/isContainer}}{{/defaultValue}}{{/allowableValues}}){{/swagger2AnnotationLibrary}}{{#swagger1AnnotationLibrary}}@ApiParam(value = "{{{description}}}"{{#required}}, required = true{{/required}}{{#allowableValues}}, allowableValues = "{{#values}}{{{.}}}{{^-last}}, {{/-last}}{{/values}}"{{/allowableValues}}{{#defaultValue}}, defaultValue = "{{{.}}}"{{/defaultValue}}){{/swagger1AnnotationLibrary}} @RequestParam(value = "{{baseName}}"{{#required}}, required = true{{/required}}{{^required}}, required = false{{/required}}) {{{paramName}}}: {{>optionalDataType}} {{/isFile}} {{#isFile}}{{#swagger2AnnotationLibrary}}@Parameter(description = "{{{description}}}"){{/swagger2AnnotationLibrary}}{{#swagger1AnnotationLibrary}}@ApiParam(value = "file detail"){{/swagger1AnnotationLibrary}} {{#useBeanValidation}}@Valid{{/useBeanValidation}} @RequestPart("{{baseName}}"{{#required}}, required = true{{/required}}{{^required}}, required = false{{/required}}) {{{paramName}}}: org.springframework.web.multipart.MultipartFile{{/isFile}} {{/isFormParam}}

dds-pereihug commented 2 months ago

Glad i could help @OlgaErmolaeva.

@wing328 dont know if there was a pr for this, but if u want i can provide my suggestion as form as PR.

mkeller75 commented 4 weeks ago

Hi @wing328 I would also be very interested in a solution for the faulty multipart generation. Is it already foreseeable when the proposed fix from @dds-pereihug / @OlgaErmolaeva will be integrated?

wing328 commented 4 weeks ago

please kindly submit a PR if none has been submitted so far

we will review accordingly and try to get it merged before next release

varminas commented 4 weeks ago

I truly appreciate the time and effort you've already put into addressing this matter. This solution is very important to us.

tomasbudnikov-ext90569 commented 3 weeks ago

Can you please let me know when the next release with this fix will be available? 🤞 Thank you for your effort.

OpenAPITools / openapi-generator