spring-restdocs Duplicate File Parts Generated in MockMvc using MockPart

MockMultipartHttpServletRequest and MockMultipartHttpServletRequestBuilder Behavior have been found with the behavior of MockMultipartHttpServletRequest (spring-test) and MockMultipartHttpServletRequestBuilder (spring-test).

MockMultipartHttpServletRequest internally manages multipartFiles as separate file types. When using MockMultipartHttpServletRequestBuilder to create a mock multipart request, files can be added using either the .part method to add a MockPart or the .file method to add a MockMultipartFile.

this.mockMvc.perform(multipart("/upload")
        .file(new MockMultipartFile("file", "file.pdf", "application/pdf", "sample content".getBytes()))
        .part(new MockPart("file", "test.pdf", "contents".getBytes(), MediaType.APPLICATION_PDF))
);

The actual servlet creation process is implemented as follows:

protected final MockHttpServletRequest createServletRequest(ServletContext servletContext) {
    MockMultipartHttpServletRequest request = new MockMultipartHttpServletRequest(servletContext);
    Charset defaultCharset = (request.getCharacterEncoding() != null ?
            Charset.forName(request.getCharacterEncoding()) : StandardCharsets.UTF_8);

    this.files.forEach(request::addFile);
    this.parts.values().stream().flatMap(Collection::stream).forEach(part -> {
        request.addPart(part);
        try {
            String name = part.getName();
            String filename = part.getSubmittedFileName();
            InputStream is = part.getInputStream();
            if (filename != null) {
                request.addFile(new MockMultipartFile(name, filename, part.getContentType(), is));
            } else {
                InputStreamReader reader = new InputStreamReader(is, getCharsetOrDefault(part, defaultCharset));
                String value = FileCopyUtils.copyToString(reader);
                request.addParameter(part.getName(), value);
            }
        } catch (IOException ex) {
            throw new IllegalStateException("Failed to read content for part " + part.getName(), ex);
        }
    });
}

As can be seen in the code, files added using the .file method are only processed using addFile. However, files added using the .part method are processed using both addPart and addFile, resulting in duplicate data being included in the servlet creation.

Spring Server and Multipart Data Processing

Spring servers can process multipart data in two ways:

@RequestParam("file") MultipartFile file
request.multipartData().getFirst("file")

In particular, when using Functional Endpoints, only the second method is available. An example is shown below:

@RestController
class FileUploadController {
    @PostMapping("/upload")
    public ReturnObject handleFileUpload(@RequestParam("file") MultipartFile file) {
        System.out.println("Received file: " + file.getOriginalFilename());
        return new ReturnObject("File uploaded successfully");
    }
}

@Component
class FileUploadFunctionalEndpoint {

    @Bean
    public RouterFunction<ServerResponse> fileUploadRoute() {
        return RouterFunctions.route(
                POST("/upload"),
                request -> {
                    var filePart = request.multipartData().getFirst("file");
                    var file = (Part) filePart;
                    System.out.println("Received file: " + file.getSubmittedFileName());
                    return ServerResponse.ok().body(new ReturnObject("File uploaded successfully"));
                }
        );
    }
}

Spring RestDocs and MockPart Issues

When using RestDocs to test, files added using the .part method result in the following output:

this.mockMvc.perform(multipart("/upload")
        .part(new MockPart("file", "test.pdf", "contents".getBytes(), MediaType.APPLICATION_PDF)))
    .andDo(document("upload",
        responseFields(
            fieldWithPath("message").description("A message")
        )
    ));

$ curl 'http://localhost:8080/upload' -i -X POST \
    -H 'Content-Type: multipart/form-data;charset=UTF-8' \
    -F '[email protected];type=application/pdf' \
    -F '[email protected];type=application/pdf'

As shown in the output, even though only one file was added using the .part method, the file is included twice in the request.

Review Points

When using .file to build a MockMultipartFile, the request's part data is empty, making it impossible to read the data using Functional Endpoints. Is it appropriate to add part data to the request in tests as well?
As far as i know HTTP Multipart requests use Part normally. Therefore, it seems more consistent to fill in the part data for mock objects as well.
I think, This issue is not limited to RestDocs alone but appears to be a problem stemming from the subtle differences in behavior between various modules (Spring, spring-test(MockMVC), spring-restdocs, etc.).

Proposed Solution

Currently, MockMvcRequestConverter (Spring RestDocs) dependency that inspects and processes MockMultipartHttpServletRequest instances. to process "file" because parts is emply when you use .file() for uploading Multipart

IMO, this seems to be the case when using the common Spring method @RequestParam("file") MultipartFile file to upload files.

Solution:

Handle within RestDocs Since MockMvcRequestConverter is already dependent on MockMultipartHttpServletRequest, it would be more suitable to handle this issue within RestDocs rather than modifying other modules and potentially breaking backward compatibility.
Remove duplicates Since MockMultipartHttpServletRequestBuilder always adds both File and Part when addPart with filename is called, When the same file is added to both File and Part, remove the duplicate.
Duplicate validation There may be cases where multiple files with the same name need to be included intentionally. Therefore, the duplicate removal logic should only apply when File and Part exist as a pair.

By implementing this solution, the issue of duplicate file parts being generated in MockMultipartHttpServletRequest can be resolved, ensuring that RestDocs generates accurate API documentation.

Jan 06 '25 10:01 Lee-WonJun

This is my sample code to make adoc

package com.example.demo;

import com.fasterxml.jackson.databind.ObjectMapper;
import jakarta.servlet.http.Part;
import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.autoconfigure.restdocs.AutoConfigureRestDocs;
import org.springframework.boot.test.autoconfigure.web.servlet.WebMvcTest;
import org.springframework.context.annotation.Bean;
import org.springframework.http.MediaType;
import org.springframework.mock.web.MockMultipartFile;
import org.springframework.mock.web.MockPart;
import org.springframework.stereotype.Component;
import org.springframework.test.web.servlet.MockMvc;
import org.springframework.web.bind.annotation.PostMapping;
import org.springframework.web.bind.annotation.RequestParam;
import org.springframework.web.bind.annotation.RestController;
import org.springframework.web.multipart.MultipartFile;
import org.springframework.web.multipart.MultipartHttpServletRequest;
import org.springframework.web.servlet.function.RouterFunction;
import org.springframework.web.servlet.function.RouterFunctions;
import org.springframework.web.servlet.function.ServerResponse;

import static org.springframework.restdocs.mockmvc.MockMvcRestDocumentation.document;
import static org.springframework.restdocs.payload.PayloadDocumentation.fieldWithPath;
import static org.springframework.restdocs.payload.PayloadDocumentation.responseFields;
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.multipart;
import static org.springframework.web.servlet.function.RequestPredicates.POST;
import static org.springframework.web.servlet.function.RequestPredicates.accept;

record ReturnObject(String message) {}


@RestController
class FileUploadController {
	@PostMapping("/upload")
	public ReturnObject handleFileUpload(
			@RequestParam("file") MultipartFile file) {
		System.out.println("Received file: " + file.getOriginalFilename());

		return new ReturnObject("File uploaded successfully");
	}
}

@Component
class FileUploadFunctionalEndpoint {

	@Bean
	public RouterFunction<ServerResponse> fileUploadRoute() {
		return RouterFunctions.route(
				POST("/upload"),
				request -> {
					var filePart = request.multipartData().getFirst("file");
					var file = (Part) filePart;
					System.out.println("Received file: " + file.getSubmittedFileName());
					return ServerResponse.ok().body(new ReturnObject("File uploaded successfully"));
				}
		);
	}
}

@WebMvcTest(FileUploadController.class)
@AutoConfigureRestDocs // 1
class FileUploadControllerTest {

	@Autowired
	private MockMvc mockMvc;

	@Autowired
	private ObjectMapper objectMapper;

	@Test
	void test() throws Exception {
		this.mockMvc.perform(multipart("/upload")
						//.file(new MockMultipartFile("file", "file.pdf", "application/pdf", "sample content".getBytes()))
						.part(new MockPart("file", "test.pdf", "contents".getBytes(), MediaType.APPLICATION_PDF))
				)
				.andDo(document("upload", // 2
						responseFields(
								fieldWithPath("message").description("A message")
						)
				));

	}
}

@WebMvcTest(FileUploadFunctionalEndpoint.class)
@AutoConfigureRestDocs
class FileUploadFunctionalEndpointTest {

	@Autowired
	private MockMvc mockMvc;

	@Autowired
	private ObjectMapper objectMapper;

	@Test
	void test() throws Exception {
		MockPart filePart = new MockPart("file", "test.pdf", "contents".getBytes(), MediaType.APPLICATION_PDF);
		this.mockMvc.perform(multipart("/upload")
						.part(filePart)
						//.file(new MockMultipartFile("file", "file.pdf", "application/pdf", "sample content".getBytes()))
				)
				.andDo(document("upload-functional-endpoint",
						responseFields(
								fieldWithPath("message").description("A message")
						)
				));
	}
}

Jan 06 '25 10:01 Lee-WonJun

I wrote #954 as a reference.

Jan 06 '25 10:01 Lee-WonJun

Thanks for the report, but I think this should be considered in Spring Framework in the first instance. As things stand, only MockMultipartHttpServletRequestBuilder can know with certainty whether a file and part with the same name was intended by the user or exists purely because a part was added to the request and the builder duplicated it as a file. Ideally, MockMvc would address this by offering a single de-duplicated view of things. Anything else is likely to be complicated at best and, worse, may result in false positives when identifying duplicates.

Please open a Spring Framework issue, comment here with a link to it, and we can take things from there.

Jan 06 '25 11:01 wilkinsona

In my initial assessment, I thought that restdocs-mockmvc already performed checks on MockMultipartHttpServletRequest (for adding files). Therefore, my modifications were focused on deeper integration with MockMultipartHttpServletRequest. To minimize compatibility issues and avoid extensive code changes, I adopted this approach.

I fully agree with your perspective, and that's precisely why I included this point in the Review Points. However, I don't have a clear solution to address this situation comprehensively, as I'm not familiar with all the dependencies within the Spring Framework. I would greatly appreciate your suggestions on how to effectively resolve this issue.

I will open an issue in the link you provided and include a comment there.

Jan 06 '25 13:01 Lee-WonJun

I would greatly appreciate your suggestions on how to effectively resolve this issue

The Framework team are best-placed to suggest something here. My hope is that they can provide a mechanism by which the files and parts can be retrieved without any duplication.

Jan 06 '25 13:01 wilkinsona

I would greatly appreciate your suggestions on how to effectively resolve this issue

The Framework team are best-placed to suggest something here. My hope is that they can provide a mechanism by which the files and parts can be retrieved without any duplication.

I agree, I hope so too.

I created an Issue in spring-framework Is there something else I should do?

Jan 06 '25 13:01 Lee-WonJun

Not at this time. We need to wait for an update from the Framework team.

Jan 23 '25 11:01 wilkinsona