Allows customized request construction by exposing Request.create()

Adrian Cole · Adrian Cole · commit 80f047534ce4 · 2015-05-07T09:10:03.000-07:00
Users may wish to override percent encoding of query parameters. closes OpenFeign#227
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,4 +1,5 @@
 ### Version 8.2
+* Allows customized request construction by exposing `Request.create()`
 * Adds JMH benchmark module
 * Enforces source compatibility with animal-sniffer
 
diff --git a/core/src/main/java/feign/Request.java b/core/src/main/java/feign/Request.java
@@ -17,8 +17,6 @@
 
 import java.nio.charset.Charset;
 import java.util.Collection;
-import java.util.Collections;
-import java.util.LinkedHashMap;
 import java.util.Map;
 
 import static feign.Util.checkNotNull;
@@ -29,6 +27,15 @@
  */
 public final class Request {
 
+  /**
+   * No parameters can be null except {@code body} and {@code charset}. All parameters must be
+   * effectively immutable, via safe copies, not mutating or otherwise.
+   */
+  public static Request create(String method, String url, Map<String, Collection<String>> headers,
+                               byte[] body, Charset charset) {
+    return new Request(method, url, headers, body, charset);
+  }
+
   private final String method;
   private final String url;
   private final Map<String, Collection<String>> headers;
@@ -39,11 +46,7 @@ public final class Request {
           Charset charset) {
     this.method = checkNotNull(method, "method of %s", url);
     this.url = checkNotNull(url, "url");
-    LinkedHashMap<String, Collection<String>>
-        copyOf =
-        new LinkedHashMap<String, Collection<String>>();
-    copyOf.putAll(checkNotNull(headers, "headers of %s %s", method, url));
-    this.headers = Collections.unmodifiableMap(copyOf);
+    this.headers = checkNotNull(headers, "headers of %s %s", method, url);
     this.body = body; // nullable
     this.charset = charset; // nullable
   }
diff --git a/core/src/main/java/feign/RequestTemplate.java b/core/src/main/java/feign/RequestTemplate.java
@@ -46,11 +46,9 @@
 public final class RequestTemplate implements Serializable {
 
   private static final long serialVersionUID = 1L;
-  private final Map<String, Collection<String>>
-      queries =
+  private final Map<String, Collection<String>> queries =
       new LinkedHashMap<String, Collection<String>>();
-  private final Map<String, Collection<String>>
-      headers =
+  private final Map<String, Collection<String>> headers =
       new LinkedHashMap<String, Collection<String>>();
   private String method;
   /* final to encourage mutable use vs replacing the object. */
@@ -221,8 +219,14 @@ public RequestTemplate resolve(Map<String, ?> unencoded) {
 
   /* roughly analogous to {@code javax.ws.rs.client.Target.request()}. */
   public Request request() {
-    return new Request(method, new StringBuilder(url).append(queryLine()).toString(),
-                       headers, body, charset);
+    Map<String, Collection<String>> safeCopy = new LinkedHashMap<String, Collection<String>>();
+    safeCopy.putAll(headers);
+    return Request.create(
+        method,
+        new StringBuilder(url).append(queryLine()).toString(),
+        Collections.unmodifiableMap(safeCopy),
+        body, charset
+    );
   }
 
   /* @see Request#method() */
diff --git a/core/src/test/java/feign/TargetTest.java b/core/src/test/java/feign/TargetTest.java
@@ -0,0 +1,77 @@
+/*
+ * Copyright 2015 Netflix, Inc.
+ *
+ * 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;
+
+import com.squareup.okhttp.mockwebserver.MockResponse;
+import com.squareup.okhttp.mockwebserver.rule.MockWebServerRule;
+
+import org.junit.Rule;
+import org.junit.Test;
+
+import feign.Target.HardCodedTarget;
+
+import static feign.assertj.MockWebServerAssertions.assertThat;
+
+public class TargetTest {
+
+  @Rule
+  public final MockWebServerRule server = new MockWebServerRule();
+
+  interface TestQuery {
+
+    @RequestLine("GET /{path}?query={query}")
+    Response get(@Param("path") String path, @Param("query") String query);
+  }
+
+  @Test
+  public void baseCaseQueryParamsArePercentEncoded() throws InterruptedException {
+    server.enqueue(new MockResponse());
+
+    String baseUrl = server.getUrl("/default").toString();
+
+    Feign.builder().target(TestQuery.class, baseUrl).get("slash/foo", "slash/bar");
+
+    assertThat(server.takeRequest()).hasPath("/default/slash/foo?query=slash%2Fbar");
+  }
+
+  /**
+   * Per <a href="https://github.com/Netflix/feign/issues/227">#227</a>, some may want to opt out of
+   * percent encoding. Here's how.
+   */
+  @Test
+  public void targetCanCreateCustomRequest() throws InterruptedException {
+    server.enqueue(new MockResponse());
+
+    String baseUrl = server.getUrl("/default").toString();
+    Target<TestQuery> custom = new HardCodedTarget<TestQuery>(TestQuery.class, baseUrl) {
+
+      @Override
+      public Request apply(RequestTemplate input) {
+        Request urlEncoded = super.apply(input);
+        return Request.create(
+            urlEncoded.method(),
+            urlEncoded.url().replace("%2F", "/"),
+            urlEncoded.headers(),
+            urlEncoded.body(), urlEncoded.charset()
+        );
+      }
+    };
+
+    Feign.builder().target(custom).get("slash/foo", "slash/bar");
+
+    assertThat(server.takeRequest()).hasPath("/default/slash/foo?query=slash/bar");
+  }
+}

Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,5 @@`
`1`	`1`	`### Version 8.2`
	`2`	+* Allows customized request construction by exposing `Request.create()`
`2`	`3`	`* Adds JMH benchmark module`
`3`	`4`	`* Enforces source compatibility with animal-sniffer`
`4`	`5`