openapi-generator icon indicating copy to clipboard operation
openapi-generator copied to clipboard

[BUG][JavaSpring] Multipart request regression in 6.x

Open cc-ju opened this issue 2 years ago • 11 comments

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

It seems the PR #11449 breaks with some multipart requests.

With an endpoint:

  • Consuming multipart/form-data
  • File as parameter
  • A second parameter consisting of an object

and the generator settings:

  • skipFormModel set to false to use the second parameter
  • interfaceOnly set to true to only generate models & interfaces

The generated API causes an error while spring tries to convert the request part to the object:

Resolved [org.springframework.web.method.annotation.MethodArgumentConversionNotSupportedException: Failed to convert value of type 'java.lang.String' to required type 'com.example.demo.api.model.TestObjectPart'; nested exception is java.lang.IllegalStateException: Cannot convert value of type 'java.lang.String' to required type 'com.example.demo.api.model.TestObjectPart': no matching editors or conversion strategy found]
openapi-generator version

It's a regression introduced with openapi-generator 6.x. With the 5.x version the generated code behaves as expected.

OpenAPI declaration file content or url

Or see the reproduction repo: https://github.com/cc-ju/openapi-genrator-spring-error-reproduction/blob/main/openapi.yaml

openapi: 3.0.2
info:
  version: 1.0.0
  title: test

paths:
  /test:
    post:
      requestBody:
        content:
          multipart/form-data:
            encoding:
              file:
                contentType: "application/octet-stream"
              content:
                contentType: "application/json"
            schema:
              type: object
              required:
                - file
                - content
              properties:
                file:
                  type: string
                  format: binary
                content:
                  $ref: "#/components/schemas/testObjectPart"
      responses:
        200:
          description: OK

components:
  schemas:
    testObjectPart:
      type: object
      required:
        - foo
      properties:
        foo:
          type: string
        bar:
          type: number
Generation Details
Actual output
    default ResponseEntity<Void> testPost(
        @Parameter(name = "file", description = "", required = true) @RequestPart(value = "file", required = true) MultipartFile file,
        @Parameter(name = "content", description = "", required = true) @Valid @RequestParam(value = "content", required = true) TestObjectPart content
    ) {
        return new ResponseEntity<>(HttpStatus.NOT_IMPLEMENTED);

    }
Expected output

Note that this would re-introduce the problem #11449 tried to fix

    default ResponseEntity<Void> testPost(
        @Parameter(name = "file", description = "", required = true) @RequestPart(value = "file", required = true) MultipartFile file,
        @Parameter(name = "content", description = "", required = true) @Valid @RequestPart(value = "content", required = true) TestObjectPart content
    ) {
        return new ResponseEntity<>(HttpStatus.NOT_IMPLEMENTED);

    }
Steps to reproduce

check out the reproduction repo: https://github.com/cc-ju/openapi-genrator-spring-error-reproduction

run ./gradlew check --info

To switch to the older generator just change:

  • https://github.com/cc-ju/openapi-genrator-spring-error-reproduction/blob/main/build.gradle#L3-L4
  • https://github.com/cc-ju/openapi-genrator-spring-error-reproduction/blob/main/build.gradle#L21-L24
Related issues/PRs

As mentioned above this regression is caused by #11449

Suggest a fix

Obviously #11449 resolved another problem. But broke this use case. Currently I don't have a solution which does not break either this use case or the use case from the PR.

cc-ju avatar May 31 '22 17:05 cc-ju

I am not sure what's "right" but we were able to get past this issue by implementing a custom org.springframework.core.convert.converter.GenericConverter. Not ideal as I have to maintain a small class that previously happened automatically for me but is what it is.

import com.fasterxml.jackson.databind.ObjectMapper;

import org.springframework.core.convert.TypeDescriptor;
import org.springframework.core.convert.converter.ConditionalGenericConverter;
import org.springframework.lang.Nullable;
import org.springframework.web.multipart.MultipartFile;

import java.io.IOException;
import java.io.InputStreamReader;
import java.io.Reader;
import java.io.StringReader;
import java.util.HashSet;
import java.util.Set;

public class DTOConverter implements ConditionalGenericConverter {
	ObjectMapper mapper = new ObjectMapper();

	@Override
	public boolean matches( TypeDescriptor sourceType, TypeDescriptor targetType ) {
		if ( targetType.getType().getPackage().getName().equals( "my.namespace.dto" )
				&& ( sourceType.isAssignableTo( TypeDescriptor.valueOf( MultipartFile.class ) )
						|| sourceType.getType().equals( String.class ) ) ) {
			return true;
		}
		return false;
	}

	@Override
	public Set<ConvertiblePair> getConvertibleTypes() {
		HashSet<ConvertiblePair> pairs = new HashSet<>();
		pairs.add( new ConvertiblePair( MultipartFile.class, Object.class ) );
		pairs.add( new ConvertiblePair( String.class, Object.class ) );
		return pairs;
	}

	@Override
	@Nullable
	public Object convert( @Nullable Object source, TypeDescriptor sourceType, TypeDescriptor targetType ) {
		if ( source == null ) {
			return null;
		}
		Object converted = null;
		try {
			converted = mapper.readValue( getSource( source, sourceType ), targetType.getType() );
		} catch ( IOException e ) {
			e.printStackTrace();
		}
		return converted;
	}

	private Reader getSource( Object source, TypeDescriptor sourceType ) throws IOException {
		if ( sourceType.isAssignableTo( TypeDescriptor.valueOf( MultipartFile.class ) ) ) {
			return new InputStreamReader( ( ( MultipartFile ) source ).getInputStream() );
		} else if ( sourceType.getType().equals( String.class ) ) {
			return new StringReader( ( String ) source );
		}
		throw new IllegalArgumentException( "Unsupported source type: " + sourceType.getType().getCanonicalName() );
	}

}

which can be registered in spring boot as follows

@EnableAsync
@Configuration
@EnableScheduling
@EnableTransactionManagement
public class WarSpringConfiguration implements WebMvcConfigurer {

	public WarSpringConfiguration() {
	}

	@Override
	public void addFormatters( FormatterRegistry registry ) {
		registry.addConverter( new DTOConverter() );
		WebMvcConfigurer.super.addFormatters( registry );
	}
}

jej2003 avatar Jun 01 '22 19:06 jej2003

Noticed the same today, the file is mapped as @RequestPart, and companion object is @RequestParam.

JSON definition:

"requestBody": {
          "content": {
            "multipart/form-data": {
              "schema": {
                "type": "object",
                "required": [
                  "documentDescription",
                  "document"
                ],
                "properties": {
                  "documentDescription": {
                    "$ref": "#/components/schemas/DocumentDescription"
                  },
                  "document": {
                    "type": "string",
                    "format": "binary",
                    "description": "A document.                      \nCurrently only **application/pdf** Content-Type is supported.\n"
                  }
                }
              },
              "encoding": {
                "documentDescription": {
                  "contentType": "application/json"
                },
                "document": {
                  "contentType": "application/pdf"
                }
              }
            }
          }
        }

Generated code:

default Mono<ResponseEntity<Void>> endpoint1(
...
        @Parameter(name = "documentDescription", description = "", required = true) @Valid @RequestParam(value = "documentDescription", required = true) DocumentDescription documentDescription,
        @Parameter(name = "document", description = "A document.                       Currently only **application/pdf** Content-Type is supported. ", required = true) @RequestPart(value = "document", required = true) Flux<Part> document,
...
    ) {

Tried to test it:

    @Test
    public void test() {
        DocumentDescription documentDescription = ...
        MultipartBodyBuilder multipartBodyBuilder = new MultipartBodyBuilder();
        multipartBodyBuilder.part("document", new ClassPathResource("/pdf/testFile.pdf"), MediaType.APPLICATION_PDF);
        multipartBodyBuilder.part("documentDescription", documentDescription, MediaType.APPLICATION_JSON);

        testClient.post()
                .uri(uriBuilder -> uriBuilder
                        .path(REQUEST_URL)
                        .queryParam("requestId", "1234")
                        .build()
                )
                .header("header-1", "1")
                .contentType(MediaType.MULTIPART_FORM_DATA)
                .bodyValue(multipartBodyBuilder.build())
                .exchange()
                .expectStatus().isAccepted();
    }

And was getting:

2022-08-12 17:44:04,479 [jetty-http@25f7cc38-35] DEBUG o.s.web.method.HandlerMethod - [3e4b6cd7] Could not resolve parameter [2] in public reactor.core.publisher.Mono<org.springframework.http.ResponseEntity<java.lang.Void>> xxx.GeneratedController.endpoint1(java.lang.String,java.lang.String,xxx.DocumentDescription,reactor.core.publisher.Flux<org.springframework.http.codec.multipart.Part>,org.springframework.web.server.ServerWebExchange): 400 BAD_REQUEST "Required DocumentDescription parameter 'documentDescription' is not present"

After changing the @RequestParam to @RequestPart for documentDescription param - everything is working well for above WebClient tests.

bskorka avatar Aug 12 '22 15:08 bskorka

I have solved the problem by customizing the formParams.mustache template to revert the change.

For that, I have copied the file from the repo (make sure to use the appropriate branch, here 6.1.x!) and recreated it in templates/formParams.mustache in my project. I have then reverted the change from #11449 by changing @RequestParam to @RequestPart in the template.

The custom template can then be registered in the openApiGenerate task in gradle by adding the templateDir option.

openApiGenerate {
  generatorName = "spring"
  library = "spring-boot"
  ...
  templateDir = "$projectDir/templates"
}

FieteO avatar Sep 20 '22 12:09 FieteO

Apparently, reverting changes in the the formParams.mustache template is not a solution as it breaks the generated code for content type application/x-www-form-urlencoded

The template have to take into account isMultipart value when generating @RequestParam annotation string.

To solve the issue and seamlessly support following content types in spring generated code

  • application/x-www-form-urlencoded
  • multipart/*

The string @RequestParam in template should be substituted by

{{#isMultipart}}@RequestPart{{/isMultipart}}{{^isMultipart}}@RequestParam{{/isMultipart}}

skduma avatar Nov 30 '22 15:11 skduma

Ping!

I am also affected by this bug. Any news here?

lsh-silpion avatar Jan 25 '23 16:01 lsh-silpion

@lsh-silpion If you update to 6.3.0-SNAPSHOT it should be fixed. It's a temporary solution at least until 6.3.0 releases.

rayalex avatar Jan 25 '23 16:01 rayalex

I enabled snapshots by adding

    <repositories>
        <repository>
            <id>sonatype-snapshots</id>
            <url>https://oss.sonatype.org/content/repositories/snapshots</url>
            <snapshots>
                <enabled>true</enabled>
            </snapshots>
            <releases>
                <enabled>false</enabled>
            </releases>
        </repository>
    </repositories>

    <pluginRepositories>
        <pluginRepository>
            <id>sonatype-snapshots</id>
            <url>https://oss.sonatype.org/content/repositories/snapshots</url>
            <snapshots>
                <enabled>true</enabled>
            </snapshots>
            <releases>
                <enabled>false</enabled>
            </releases>
        </pluginRepository>
    </pluginRepositories>

and then used

        <openapi-generator-version>6.3.0-20230125.125049-89</openapi-generator-version>

specifially (because 6.3.0-SNAPSHOT did not seem to work) and I am still getting

[ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.8.1:compile (default-compile) on project specification: Compilation failure
[ERROR] /Users/lars/Documents/Projects/akquinet/TIMREF/messenger-testtreiber-api/specification/target/generated-sources/openapi/src/main/java/de/akquinet/timref/testdriver/api/DevicesApi.java:[1744,54] Inkompatible Typen: org.springframework.core.io.Resource kann nicht in org.springframework.web.multipart.MultipartFile konvertiert werden

I am missing something?

lsh-silpion avatar Jan 25 '23 19:01 lsh-silpion

Hi, today I also stumpled upon this bug in version 6.5.0 of the spring generator. Is there any update?

bjpe avatar May 05 '23 14:05 bjpe

Hi, the bug still exists in version 7.0.0.

bjpe avatar Sep 08 '23 10:09 bjpe

I can confirm that this is still reproducible in 7.0.0. Is there any ETA on fixing the regression bug?

yddimitrov avatar Sep 13 '23 13:09 yddimitrov

Hi, the bug still exists in version 7.2.0.

vitaliykushneruk avatar Apr 27 '24 14:04 vitaliykushneruk

Hi, same in 7.3.0

chucktesejme avatar May 30 '24 05:05 chucktesejme