Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

MockMvc ignores MultipartFile registrations when both files and parts are registered #26166

Closed
ThanksForAllTheFish opened this issue Nov 27, 2020 · 3 comments
Assignees
Labels
in: test Issues in the test module in: web Issues in web modules (web, webmvc, webflux, websocket) type: regression A bug that is also a regression
Milestone

Comments

@ThanksForAllTheFish
Copy link

In my code I am doing something like:

mockMvc.multipart(DOCUMENTS, uuid) {
      file(MockMultipartFile("file", "test.txt", "text/plain", "Test".toByteArray()))
      part(MockPart("data", """{"node":"node"}""".toByteArray()))
}

which is supposed to create a request with both a part, named data, and a file, named file.

This was supported in Spring 5.2, but it is not supported anymore in Spring 5.3 since #25602

I am not sure if it was a "wrong" use case, not meant to be working, or if the fix introduced in the linked PR is a regression, since TestDispatcherServlet simply creates a StandardMultipartHttpServletRequest without copying the files over, as MockMultipartHttpServletRequestBuilder was originally doing.

https://github.com/spring-projects/spring-framework/blob/master/spring-web/src/main/java/org/springframework/web/multipart/support/StandardMultipartHttpServletRequest.java#L95 simply copies the parts, but not the files

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Nov 27, 2020
@rstoyanchev
Copy link
Contributor

Can you provide more details around the failure and the target controller method? Also have you tried providing the file as a part, something like this (headers can also be set via setters):

part(MockPart("file", "test.txt", "Test".toByteArray()))
part(MockPart("data", """{"node":"node"}""".toByteArray()))

@rstoyanchev rstoyanchev added in: test Issues in the test module in: web Issues in web modules (web, webmvc, webflux, websocket) status: waiting-for-feedback We need additional information before we can continue labels Nov 30, 2020
@ThanksForAllTheFish
Copy link
Author

@rstoyanchev yes, I indeed tried with multiple MockParts and that passes to my controller method the expected values. I could definitely use MockPart instead of MockMultipartFile.

As for the controller method I expect to be invoked:

@PostMapping(consumes = [MediaType.MULTIPART_FORM_DATA_VALUE, MediaType.APPLICATION_JSON_VALUE])
fun uploadDocuments(
    @RequestPart("file") file: MultipartFile
)

this is it. Since #25602, file is null when both part and file are used in the test.

Just to be clear, it is absolutely not a problem to use MockPart, it just felt weird that the two can be combined but then file ends up being discarded and wanted to double check if the change in the PR really led to the wanted result.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Dec 2, 2020
@rstoyanchev rstoyanchev self-assigned this Dec 3, 2020
@rstoyanchev
Copy link
Contributor

Indeed, this was not intended. In MockMultipartHttpServletRequestBuilder, when both files and parts are registered, we can turn the files to parts with a filename and effectively register everything as parts.

@rstoyanchev rstoyanchev added type: regression A bug that is also a regression and removed status: feedback-provided Feedback has been provided status: waiting-for-triage An issue we've not yet triaged or decided on labels Dec 3, 2020
@rstoyanchev rstoyanchev added this to the 5.3.2 milestone Dec 3, 2020
@rstoyanchev rstoyanchev changed the title Failed to create a mock request with file and parts MockMvc ignores MultipartFile registrations when both files and parts are registered Dec 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: test Issues in the test module in: web Issues in web modules (web, webmvc, webflux, websocket) type: regression A bug that is also a regression
Projects
None yet
Development

No branches or pull requests

3 participants