I've been making good use of the declarative HTTP interface feature that was added in the latest spring release. I would like HTTP interface to be a bit more versatile and support different encoding methods for @RequestParam.

Currently, the encoding method for @RequestParam of type collection seems to be repeating the same key with &, which seems to be related to the way RequestParamArgumentResolver is implemented.

interface TestRepository {
    @GetExchange("/test")
    Repository test(@RequestParam List<String> param);
}

@Service
@RequiredArgsConstructor
public class TestService {
        private TestRepository repository;

        public void test() {
                List<String> param = List.of("1", "2", "3");
                repository.test(param); // Generated URI path -> GET /test?param=1&param=2&param=3
        }
}

Of course Modern web frameworks expect @RequestParam to be deserialized to collection type automatically, either by repeating the same key value as &, or by parsing strings joined by certain delimiters. However, if this is not possible due to limitations of older web frameworks or API specs, we have to join the strings with a specific delimiter before the request and then pass the strings as @RequestParam, which is not cool.

It would be nice to have the ability to encode @RequestParam via @CollectionFormat, which is supported by Spring Cloud OpenFeign. I think users who use declarative HTTP interface will have experience with Spring Cloud OpenFeign and will expect a smooth transition.

interface TestRepository {
    @GetExchange("/test1")
    Repository test1(@RequestParam @CollectionFormat(CSV) List<String> param);

    @GetExchange("/test2")
    Repository test2(@RequestParam @CollectionFormat(SSV) List<String> param);
}

@Service
@RequiredArgsConstructor
public class TestService {
        private TestRepository repository;

        public void test() {
                List<String> param = List.of("1", "2", "3");
                repository.test1(param); // Generated URI path -> GET /test1?param=1,2,3
                repository.test2(param); // Generated URI path -> GET /test2?param=1 2 3
        }
}

I'm curious to know what the maintainers think about this πŸ™‚

Comment From: doljae

BTW, I created a PR that simply adds the @CollectionFormat feature πŸ™‚ - https://github.com/spring-projects/spring-framework/pull/33155

Comment From: rstoyanchev

The example is with HTTP GET parameters in the query, but @RequestParam also applies to HTTP POST with form data in the request body. Is this intended to apply to form data too?

On the server side we apply the ConversionService to method parameters, and it makes it possible to inject comma-separated values into a List. We could do something similar on the client side as well. In fact, RequestParamArgumentResolver already has a ConversionService but it's not actually used because the multiValued argument here is set to true. If I set that to false, then the ConversionService is applied and comma-separated works as expected. We could expose this as a configuration property on RequestParamArgumentResolver, e.g. boolean formatAsSingleValue or something similar, and you could configure the resolver.

Comment From: doljae

@rstoyanchev Thanks for the explanation πŸ™‚

Is this intended to apply to form data too?

It is not intended to extend to form data. It's a bit embarrassing, but I just realized that we can bind form data with a combination of @PostMapping and @RequestParam πŸ˜…

This issue and the PR I created were initially thought of from a client side perspective only. We often concatenated collection type parameters into strings with a pre-request delimiter to meet certain API specifications, which I thought was uncool, and Spring Cloud OpenFeign is proposing to migrate to the Spring Framework's declarative HTTP interface as feature-complete.

So my personal opinion is that it wouldn't be a bad idea to look at it from a client-side perspective, i.e. to support this feature only on HTTP interfaces using HTTP clients, including RestClient, as a starting point.

Comment From: doljae

Also, @RequestParam is a very stable, high-impact annotation that's used in a lot of places. So I thought it would have fewer side effects to add a new annotation and have the RequestParamArgumentResolver do a little extra logic if it has this annotation.

First of all, I identified that it would be nice to have such a feature as an issue and PR which is not perfect, but it can be implemented without much modification.

If you have any ideas for a different, more generalized approach to implementing this feature (which you've outlined), and if it's possible, I'd be happy to work on it πŸ™‚

Comment From: rstoyanchev

HTTP POST of form data with @RequestParam is supported on both the client and server side, and generally the intent with @HttExchange methods is to keep the programming model neutral to client or server.

I propose that we start by making RequestParamArgumentResolver customizable so that it formats collection values to a String through the ConversionService. An application can then configure that as a custom argument resolver on the HttpServiceProxyFactory.Builder. If you want to start from there. We also need to make sure that this is not done for form data nor multipart requests. One way to do it is to check the HttpExchange annotation contentType attribute.

Comment From: doljae

@rstoyanchev

I analyzed the code according to your guide and understood the logic behind the integration of RequestParamArgumentResolver and ConversionService.

I have a few questions before I start working on it.

  1. How to trigger the multiValued field of the NamedValueInfo returned by RequestParamArgumentResolver.createNamedValueInfo() to be false, using the @CollectionFormat annotation I mentioned before? Please let me know if you have any other methods in mind πŸ™‚

  2. HttpExchange.contentType() can be read directly from the XXXExchange annotation, but I think it should also take into account the value of the HttpExchange annotation in the method, class, etc. Is this the way you intended to work? Seems HttpExchangeReflectiveProcessor is related to this logic and it determine the contentType value from various contentType() from @HttpExchange πŸ€”


@HttpExchange(contentType = "application/json")
private interface TestApiClient {

    @HttpExchange(contentType = "application/x-www-form-urlencoded;charset=UTF-8")
    @PostExchange(contentType = "application/json")
    void test();
}
  1. How would we enable a converter that supports more delimiters than just commas? CollectionToStringConverter seems to be the core class for turning a collection into a string with a comma delimiter. Based on your answers to questions 1 and 2, it seems like it could be implemented to support for comma delimiters. But how would we extend the Converter class to read @CollectionFormat and convert to that delimiter?

Comment From: rstoyanchev

@doljae I am suggesting that we start with the ConversionService approach as a transparent, automated feature, without introducing an @CollectionFormat annotation. In other words, for a given HttpServiceProxyFactory you choose whether to have collection values formatted or not. If this is sufficient then it's easier than having to repeat the same annotation on every request parameter.

Comment From: doljae

@rstoyanchev you mean that make some modification so that we can use new feature like this way?

@Configuration
class RestClientConfiguration {
    @Bean
    fun testApiClient(conversionService: ConversionService): TestApiClient {
        val restClient =
            RestClient.builder()
                .baseUrl("http://127.0.0.1:8080")
                .build()
        val adapter = RestClientAdapter.create(restClient)
        val factory = HttpServiceProxyFactory.builderFor(adapter)
            .customArgumentResolver(RequestParamArgumentResolver(conversionService, true)) // new boolean field, formatAsSingleValue
            .build()

        return factory.createClient(TestApiClient::class.java)
    }
}

Comment From: doljae

@rstoyanchev

I make draft PR. Please check I'm working in the right direction πŸ™‚ - https://github.com/spring-projects/spring-framework/pull/33220

Comment From: rstoyanchev

I'll close this as superseded by #33220. Let's continue there.