2

Background

The problem came up on my day job while implementing a POST multipart/form-data endpoint for a file upload with some meta information. I am not an expert in the Spring Boot ecosystem; it is likely that the problem is solved by a simple fix and that I am just missing the right term to search for.

Problem statement

To implement an endpoint for a file-upload with additional meta information, I wrote the following @RestController:

@RestController
@RequestMapping(Resource.ROOT)
@AllArgsConstructor(onConstructor = @__({@Inject}))
public class Resource {
  public static final String ROOT = "/test";

  private final Logger logger;

  @PostMapping(consumes = MediaType.MULTIPART_FORM_DATA, produces = MediaType.APPLICATION_JSON)
  public ResponseEntity<Void> test(@Valid final Request request) {
    logger.info("request = {}", request);
    return ResponseEntity.ok().build();
  }
}

With Request being specified as:

@Value
@AllArgsConstructor
public class Request {

  @NotNull
  String name;

  @NotNull
  MultipartFile file;
}

And a small happy path test:

@SpringBootTest
@AutoConfigureMockMvc
class TestCase {

  @Autowired
  private MockMvc mockMvc;

  @Test
  void shouldReturnOk() throws Exception {
    // GIVEN
    final byte[] content = Files.readAllBytes(Path.of(".", "src/test/resources/PenPen.png"));
    final String name = "name";

    // WHEN
    // @formatter:off
    mockMvc.perform(MockMvcRequestBuilders
        .multipart(Resource.ROOT)
            .file("file", content)
            .param("name", name))

    // THEN
        .andExpect(status().isOk());
    // @formatter:on
  }
}

A complete MRE can be found on Bitbucket, branch problem-with-immutable-request.

When running the test (./mvnw test), it fails with the endpoint returning a 400 BAD REQUEST instead of 200 OK. Reading the logs reveals that request parameter file is null:

...
     Content type = text/plain;charset=UTF-8
             Body = file: must not be null.
...

I partially understand why it is null. With this partial knowledge, I was able to circumvent the problem by making the field file in Request mutable:

@ToString
@Getter
@AllArgsConstructor
public class Request {

  @NotNull
  private final String name;

  @Setter
  @NotNull
  private MultipartFile file;
}

The code "fixing" the problem can be found on Bitbucket, branch problem-solved-by-making-field-mutable.

This, however, makes the Request mutable, which I would like to prevent. To further investigate, I unrolled the lombok annotations on Request and added some logging:

public class Request {

  private static final Logger LOGGER = LoggerFactory.getLogger(Request.class);

  @NotNull
  private final String name;

  @NotNull
  private MultipartFile file;

  public Request(final String name, final MultipartFile file) {
    this.name = name;
    this.setFile(file);
  }

  public @NotNull String getName() {
    return this.name;
  }

  public @NotNull MultipartFile getFile() {
    return this.file;
  }

  public String toString() {
    return "Request(name=" + this.getName() + ", file=" + this.getFile() + ")";
  }

  public void setFile(final MultipartFile file) {
    LOGGER.info("file = {}", file);
    this.file = file;
  }
}

Code of unrolled version can be found on Bitbucket, branch lombok-unrolled-for-debugging.

When looking at the log statements of the now successful test, we can see that Request::setFile is called twice:

2020-09-05 09:42:31.049  INFO 11012 --- [           main] d.turing85.springboot.multipart.Request  : file = null
2020-09-05 09:42:31.056  INFO 11012 --- [           main] d.turing85.springboot.multipart.Request  : file = org.springframework.mock.web

The first call comes from the constructor invocation. The second call, I imagine, comes from somewhere within Spring's mapping mechanism for the form parameters.

I know that there is the possibility to define the form parameters individually on the endpoint and constructing the Request instance within the method:

public class Resource {
  ...
  @PostMapping(consumes = MediaType.MULTIPART_FORM_DATA, produces = MediaType.APPLICATION_JSON)
  public ResponseEntity<Void> test(
      @RequestPart(name = "name") final String name,
      @RequestPart(name = "file") final MultipartFile file) {
    final Request request = new Request(name, file);
    logger.info("request = {}", request);
    return ResponseEntity.ok().build();
  }
}

This will, however, result in other problems. For example, we would have to add an additional exception mapper for MissingServletRequestPartException and align the returned HTTP response with the existing response for BindException. I would like to avoid this if possible.

A search on the topic turned up Spring Boot controller - Upload Multipart and JSON to DTO. The solution, however, did not work for me since I do not use MVC (I think).

Question

Is there a possibility to keep Request immutable such that Spring is able to pass the MultipartFile to the all args constructor instead of setting it through the setter afterwards? Writing a custom mapper/converter is acceptable, but I did not find a possibility to write a mapper for either a specific endpoint or a specific type.

Turing85
  • 18,217
  • 7
  • 33
  • 58
  • is it necessary to wrap the `MultipartFile`? seems like if you will separate it you will have no mutability problems – limido Sep 05 '20 at 10:29
  • I tried with ModelAttribute and it works but you have to define setter for multipart data else it will be null. – Gurkan İlleez Sep 05 '20 at 10:30
  • @limido Yes, it is. I gave an explanation as to why I want it to be in `Request` in my post. – Turing85 Sep 05 '20 at 11:20
  • @Gurkanİlleez defining setters defeats the point. I have already demonstrated that providing a setter for `file` "fixes" the problem, without adding `@ModelAttribute`. – Turing85 Sep 05 '20 at 11:21
  • Ofcourse it defeats the point but converter is using setter to convert multipart file data – Gurkan İlleez Sep 06 '20 at 14:56

1 Answers1

0
 @PostMapping(consumes = MediaType.MULTIPART_FORM_DATA_VALUE, produces = MediaType.APPLICATION_JSON_VALUE)
 public ResponseEntity<Void> test(@Valid @ModelAttribute final RequestDto request) {
     return ResponseEntity.ok().build();
 }

It is still working with rest api call. But i really do not get immutability concern of yours.

If you define setter the multipart data you can use ModelAttribute.

@SpringBootTest
@AutoConfigureMockMvc
class FileUploadControllerIT {

    @Autowired
    private MockMvc mockMvc;

    @Test
    void shouldReturnOk() throws Exception {
        // GIVEN
        final byte[] content = Files.readAllBytes(Paths.get(Thread.currentThread().getContextClassLoader().getResource("text.txt").toURI()));
        final String name = "name";

        // WHEN
        // @formatter:off
        mockMvc.perform(MockMvcRequestBuilders
                .multipart("/context/api/v1")
                .file("multipartFile", content)
                .param("name", name))

                // THEN
                .andExpect(status().isOk());
        // @formatter:on
    }
}

The above code works with ModelAttribute.

Also you are giving absolute path, i guess it is wrong. You can get file with classloader.

Gurkan İlleez
  • 1,503
  • 1
  • 10
  • 12
  • For me, the test still fails with `@ModelAttriibute`. This does not answer the question. As to why I want immutability: with JSON requests, we have the possibility to annotate the DTO such that a bulder is used. For form data, this seems to not be the case. This in return leads to a heterogenus structure of DTOs, which in return introduces unnecessary complexity. – Turing85 Sep 05 '20 at 11:29