Merge pull request #3286 from yvasyliev/feature/form-urlencoded-collection-format

feat(form): enhance `UrlencodedFormContentProcessor` to support `CollectionFormats` in key-value pairs

Author: velo

CHANGELOG.md

@@ -1,3 +1,8 @@
+### Version 13.12
+
+* `UrlencodedFormContentProcessor` now honors `CollectionFormat` from `@RequestLine`/`RequestTemplate` for array and
+  collection values in `application/x-www-form-urlencoded` bodies.
+
 ### Version 11.9
 
 * `OkHttpClient` now implements `AsyncClient`

form/src/main/java/feign/form/UrlencodedFormContentProcessor.java

@@ -17,14 +17,19 @@
 
 import static feign.form.ContentType.URLENCODED;
 
+import feign.CollectionFormat;
 import feign.RequestTemplate;
 import feign.codec.EncodeException;
 import java.net.URLEncoder;
 import java.nio.charset.Charset;
+import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.Map;
 import java.util.Map.Entry;
+import java.util.Objects;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
 import lombok.SneakyThrows;
 import lombok.val;
 
@@ -55,7 +60,7 @@ public void process(RequestTemplate template, Charset charset, Map<String, Objec
       if (bodyData.length() > 0) {
         bodyData.append(QUERY_DELIMITER);
       }
-      bodyData.append(createKeyValuePair(entry, charset));
+      bodyData.append(createKeyValuePair(template.collectionFormat(), entry, charset));
     }
 
     val contentTypeValue =
@@ -77,16 +82,19 @@ public ContentType getSupportedContentType() {
     return URLENCODED;
   }
 
-  private String createKeyValuePair(Entry<String, Object> entry, Charset charset) {
+  private CharSequence createKeyValuePair(
+      CollectionFormat collectionFormat, Entry<String, Object> entry, Charset charset) {
     String encodedKey = encode(entry.getKey(), charset);
     Object value = entry.getValue();
 
     if (value == null) {
       return encodedKey;
     } else if (value.getClass().isArray()) {
-      return createKeyValuePairFromArray(encodedKey, value, charset);
+      return createKeyValuePair(
+          collectionFormat, encodedKey, Arrays.stream((Object[]) value), charset);
     } else if (value instanceof Collection) {
-      return createKeyValuePairFromCollection(encodedKey, value, charset);
+      return createKeyValuePair(
+          collectionFormat, encodedKey, ((Collection<?>) value).stream(), charset);
     }
     return new StringBuilder()
         .append(encodedKey)
@@ -95,28 +103,10 @@ private String createKeyValuePair(Entry<String, Object> entry, Charset charset)
         .toString();
   }
 
-  private String createKeyValuePairFromCollection(String key, Object values, Charset charset) {
-    val collection = (Collection<?>) values;
-    val array = collection.toArray(new Object[0]);
-    return createKeyValuePairFromArray(key, array, charset);
-  }
-
-  private String createKeyValuePairFromArray(String key, Object values, Charset charset) {
-    val result = new StringBuilder();
-    val array = (Object[]) values;
-
-    for (int index = 0; index < array.length; index++) {
-      val value = array[index];
-      if (value == null) {
-        continue;
-      }
-
-      if (index > 0) {
-        result.append(QUERY_DELIMITER);
-      }
-
-      result.append(key).append(EQUAL_SIGN).append(encode(value, charset));
-    }
-    return result.toString();
+  private CharSequence createKeyValuePair(
+      CollectionFormat collectionFormat, String key, Stream<?> values, Charset charset) {
+    val stringValues =
+        values.filter(Objects::nonNull).map(Object::toString).collect(Collectors.toList());
+    return collectionFormat.join(key, stringValues, charset);
   }
 }

form/src/test/java/feign/form/UrlencodedFormContentProcessorTest.java

@@ -0,0 +1,162 @@
+/*
+ * Copyright © 2012 The Feign Authors (feign@commonhaus.dev)
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package feign.form;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import feign.CollectionFormat;
+import feign.Feign;
+import feign.Headers;
+import feign.RequestLine;
+import feign.form.utils.UndertowServer;
+import feign.jackson.JacksonEncoder;
+import io.undertow.server.HttpServerExchange;
+import java.util.Arrays;
+import java.util.LinkedHashMap;
+import java.util.Map;
+import java.util.function.BiFunction;
+import org.junit.jupiter.api.Test;
+
+class UrlencodedFormContentProcessorTest {
+
+  @Test
+  void arrayValueUsesDefaultExplodedCollectionFormat() {
+    assertEncodedBody(
+        "from=%2B987654321&to=%2B123456789&tags=one&tags=two",
+        new String[] {"one", "two"}, Client::map);
+  }
+
+  @Test
+  void collectionValueUsesDefaultExplodedCollectionFormat() {
+    assertEncodedBody(
+        "from=%2B987654321&to=%2B123456789&tags=one&tags=two",
+        Arrays.asList("one", "two"), Client::map);
+  }
+
+  @Test
+  void arrayValueUsesCsvCollectionFormat() {
+    assertEncodedBody(
+        "from=%2B987654321&to=%2B123456789&tags=one%2Ctwo",
+        new String[] {"one", "two"}, Client::mapCsv);
+  }
+
+  @Test
+  void collectionValueUsesCsvCollectionFormat() {
+    assertEncodedBody(
+        "from=%2B987654321&to=%2B123456789&tags=one%2Ctwo",
+        Arrays.asList("one", "two"), Client::mapCsv);
+  }
+
+  @Test
+  void arrayValueUsesSsvCollectionFormat() {
+    assertEncodedBody(
+        "from=%2B987654321&to=%2B123456789&tags=one%20two",
+        new String[] {"one", "two"}, Client::mapSsv);
+  }
+
+  @Test
+  void collectionValueUsesSsvCollectionFormat() {
+    assertEncodedBody(
+        "from=%2B987654321&to=%2B123456789&tags=one%20two",
+        Arrays.asList("one", "two"), Client::mapSsv);
+  }
+
+  @Test
+  void arrayValueUsesTsvCollectionFormat() {
+    assertEncodedBody(
+        "from=%2B987654321&to=%2B123456789&tags=one%09two",
+        new String[] {"one", "two"}, Client::mapTsv);
+  }
+
+  @Test
+  void collectionValueUsesTsvCollectionFormat() {
+    assertEncodedBody(
+        "from=%2B987654321&to=%2B123456789&tags=one%09two",
+        Arrays.asList("one", "two"), Client::mapTsv);
+  }
+
+  @Test
+  void arrayValueUsesPipesCollectionFormat() {
+    assertEncodedBody(
+        "from=%2B987654321&to=%2B123456789&tags=one%7Ctwo",
+        new String[] {"one", "two"}, Client::mapPipes);
+  }
+
+  @Test
+  void collectionValueUsesPipesCollectionFormat() {
+    assertEncodedBody(
+        "from=%2B987654321&to=%2B123456789&tags=one%7Ctwo",
+        Arrays.asList("one", "two"), Client::mapPipes);
+  }
+
+  private void assertEncodedBody(
+      String expectedBody,
+      Object tags,
+      BiFunction<Client, Map<String, Object>, String> requestCall) {
+    try (var server =
+        UndertowServer.builder()
+            .callback((exchange, message) -> assertRequest(exchange, message, expectedBody))
+            .start()) {
+      var client =
+          Feign.builder()
+              .encoder(new FormEncoder(new JacksonEncoder()))
+              .target(Client.class, server.getConnectUrl());
+
+      var data = createRequestData(tags);
+      assertThat(requestCall.apply(client, data)).isEqualTo("ok");
+    }
+  }
+
+  private Map<String, Object> createRequestData(Object tags) {
+    var data = new LinkedHashMap<String, Object>();
+    data.put("from", "+987654321");
+    data.put("to", "+123456789");
+    data.put("tags", tags);
+    return data;
+  }
+
+  private void assertRequest(HttpServerExchange exchange, byte[] message, String expectedBody) {
+    assertThat(exchange.getRequestHeaders().getFirst(io.undertow.util.Headers.CONTENT_TYPE))
+        .isEqualTo("application/x-www-form-urlencoded; charset=UTF-8");
+    assertThat(message).asString().isEqualTo(expectedBody);
+
+    exchange.getResponseHeaders().put(io.undertow.util.Headers.CONTENT_TYPE, "text/plain");
+    exchange.getResponseSender().send("ok");
+  }
+
+  interface Client {
+
+    @RequestLine("POST")
+    @Headers("Content-Type: application/x-www-form-urlencoded; charset=utf-8")
+    String map(Map<String, Object> data);
+
+    @RequestLine(value = "POST", collectionFormat = CollectionFormat.CSV)
+    @Headers("Content-Type: application/x-www-form-urlencoded; charset=utf-8")
+    String mapCsv(Map<String, Object> data);
+
+    @RequestLine(value = "POST", collectionFormat = CollectionFormat.SSV)
+    @Headers("Content-Type: application/x-www-form-urlencoded; charset=utf-8")
+    String mapSsv(Map<String, Object> data);
+
+    @RequestLine(value = "POST", collectionFormat = CollectionFormat.TSV)
+    @Headers("Content-Type: application/x-www-form-urlencoded; charset=utf-8")
+    String mapTsv(Map<String, Object> data);
+
+    @RequestLine(value = "POST", collectionFormat = CollectionFormat.PIPES)
+    @Headers("Content-Type: application/x-www-form-urlencoded; charset=utf-8")
+    String mapPipes(Map<String, Object> data);
+  }
+}