Support Map in FormHttpMessageConverter

[xenoterracide]

Affects: 6.1.6

This feels like it should work. I get there are scenarios that it can't, and I don't care if it does on response.

    var restClient = RestClient.builder()
      .requestFactory(new HttpComponentsClientHttpRequestFactory())
      .baseUrl("http://localhost:" + this.port)
      .messageConverters(converters -> {
        converters.addFirst(new OAuth2AccessTokenResponseHttpMessageConverter());
        //  converters.add(new FormHttpMessageConverter()); // doesn't change anything
      })
      .build();

    var login = restClient
      .post()
      .uri("/login")
      .contentType(MediaType.APPLICATION_FORM_URLENCODED)
      .body(Map.of("username", this.user, "password", this.pass))
      .retrieve()
      .toEntity(String.class);

No HttpMessageConverter for java.util.ImmutableCollections$MapN and content type "application/x-www-form-urlencoded"
org.springframework.web.client.RestClientException: No HttpMessageConverter for java.util.ImmutableCollections$MapN and content type "application/x-www-form-urlencoded"
	at org.springframework.web.client.DefaultRestClient$DefaultRequestBodyUriSpec.writeWithMessageConverters(DefaultRestClient.java:422)
	at org.springframework.web.client.DefaultRestClient$DefaultRequestBodyUriSpec.lambda$body$0(DefaultRestClient.java:381)
	at org.springframework.web.client.DefaultRestClient$DefaultRequestBodyUriSpec.exchangeInternal(DefaultRestClient.java:471)
	at org.springframework.web.client.DefaultRestClient$DefaultRequestBodyUriSpec.retrieve(DefaultRestClient.java:444)
	at com.xenoterracide.test.authorization.server.AuthorizationServerTest.authn(AuthorizationServerTest.java:98)
	at java.base/java.lang.reflect.Method.invoke(Method.java:580)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)

[poutsma]

The underlying issue here is that the FormHttpMessageConverter only accepts MultiValueMap types; not Map types. The following does work:

var login = restClient
  .post()
  .uri("/login")
  .contentType(MediaType.APPLICATION_FORM_URLENCODED)
  .body(CollectionUtils.toMultiValueMap(Map.of("username", List.of("foo"), "password", List.of("bar"))))
  .retrieve()
  .toEntity(String.class);

which is definitely less elegant.

To fix this, we would have to change FormHttpMessageConverter from a HttpMessageConverter<MultiValueMap<String, ?>> to a HttpMessageConverter<Map<String, ?>> which can determine at runtime whether it's dealing with a single or multi value map. However, this is a breaking change. Could we make such a change in 6.2, @jhoeller?

Alternatively, we can introduce a new SingleValueFormHttpMessageConverter which implements HttpMessageConverter<Map<String, ?>> and simply delegates to a (the existing?) FormHttpMessageConverter.

I definitely prefer doing the former, as having duplicate form converters seems unnecessary and complicated, also in terms of configuration.

[xenoterracide]

Maybe a couple of other alternatives. I'm not sure how possible the first one is.

1. Could the map be converted to a multi-map under the hood without changing the interface? I'm not looking at The converter as I say this.
2. Not really the same answer as I wrote the bug for but perhaps there could be a static factory on The multi-map interface that matches the map.of contract. Also I would consider adding a regular of that matches the collection utils method that you mentioned. As I certainly didn't look for it there these days 😉

[poutsma]

1. Could the map be converted to a multi-map under the hood without changing the interface? I'm not looking at The converter as I say this.

Not really, the interface defines what types are accepted for reading and writing.

2. Not really the same answer as I wrote the bug for but perhaps there could be a static factory on The multi-map interface that matches the map.of contract. Also I would consider adding a regular of that matches the collection utils method that you mentioned. As I certainly didn't look for it there these days 😉

We can certainly consider introducing similar convenience methods. I created a separate issue for that.

[poutsma]

After team discussion, we decided that the best way to go forward is to change the FormHttpMessageConverter from a HttpMessageConverter<MultiValueMap<String, ?>> to a HttpMessageConverter<Map<String, ?>>.

[poutsma]

Unfortunately, supporting plain maps in the FormHttpMessageConverter breaks applications that expect to read or write a map as JSON, but do not specify any content type. Before 80faa94 this worked fine, but now these applications read or write their maps as forms.

We did consider other ways to solve this problem (see discussion on #32917), but in the end could not find any, leaving us no other choice but to revert 80faa94, and close this issue as not planned. Fortunately, there are still the enhancements in #32832, which we can keep.