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, ?>>.