Making content type resolution more robust

[marceloverdijk]

Similar as micronaut-projects/micronaut-graphql#7 this makes the content type resolution more robust. E.g. case insensitive and ignoring content type parameters.

To do so I upgraded the Spring Boot + Spring minor versions to use equalsTypeAndSubtype introduced in Spring 5.1.4.

This also caused I had to change some integration tests using WebTestClient because of a bug in 5.1.2 which resulted in different behaviour.

[andimarek]

see my comment about wrong content types.

Also: At least one more test would be good.

[andimarek]

why are we ignoring wrong content types here? is that a real case we need to deal with?

[marceloverdijk]

If the parsing fails at that point we could throw also a throw new ResponseStatusException(HttpStatus.UNPROCESSABLE_ENTITY, "Could not process GraphQL request");

[marceloverdijk]

Note we do the same at the end in that method. Do you prefer to add that?

You want a test case with an invalid content type? I can add that one yes.

[marceloverdijk]

@andimarek I can do something like throw new ResponseStatusException(HttpStatus.BAD_REQUEST, "Invalid Content-Type: " + contentType, e);

Testing will be difficult as MockMvc does not allow to send invalid media types:
https://github.com/spring-projects/spring-framework/blob/master/spring-test/src/main/java/org/springframework/test/web/servlet/request/MockHttpServletRequestBuilder.java#L648

So effectively the test will never get in the controller itself (I just tried).

Let me know if you prefer the 400 I suggested above or keep as it and fail later (HTTP 422 at end of controller)?

[symm]

I've run into this same issue whilst trying to write some integration tests using MockMvc e.g.

def request = post("/graphql")
        .content(content)
        .contentType(MediaType.APPLICATION_JSON_VALUE)

mockMvc.perform(request).andExpect(status().isOk())

The request fails because content type gets sent over as: application/json;charset=UTF-8 and doesn't match the strict check of application/json.

Is there a workaround or anything that can be done to get this fix merged in?