Allows multiple headers with the same name; Backfills default client tests.

CHANGELOG.md

@@ -1,5 +1,6 @@
 ### Version 7.1
 * Introduces feign.@Param to annotate template parameters. Users must migrate from `javax.inject.@Named` to `feign.@Param` before updating to Feign 8.0.
+* Allows multiple headers with the same name.
 
 ### Version 7.0
 * Expose reflective dispatch hook: InvocationHandlerFactory

core/src/main/java/feign/Contract.java

@@ -15,6 +15,7 @@
  */
 package feign;
 
+import java.util.LinkedHashMap;
 import javax.inject.Named;
 import java.lang.annotation.Annotation;
 import java.lang.reflect.Method;
@@ -149,10 +150,16 @@ public interface Contract {
       } else if (annotationType == Headers.class) {
         String[] headersToParse = Headers.class.cast(methodAnnotation).value();
         checkState(headersToParse.length > 0, "Headers annotation was empty on method %s.", method.getName());
+        Map<String, Collection<String>> headers = new LinkedHashMap<String, Collection<String>>(headersToParse.length);
         for (String header : headersToParse) {
           int colon = header.indexOf(':');
-          data.template().header(header.substring(0, colon), header.substring(colon + 2));
+          String name = header.substring(0, colon);
+          if (!headers.containsKey(name)) {
+            headers.put(name, new ArrayList<String>(1));
+          }
+          headers.get(name).add(header.substring(colon + 2));
         }
+        data.template().headers(headers);
       }
     }
 

core/src/main/java/feign/RequestTemplate.java

@@ -332,28 +332,28 @@ public final class RequestTemplate implements Serializable {
    * template.query("X-Application-Version", "{version}");
    * </pre>
    *
-   * @param configKey the configKey of the header
+   * @param name the name of the header
    * @param values    can be a single null to imply removing all values. Else no
    *                  values are expected to be null.
    * @see #headers()
    */
-  public RequestTemplate header(String configKey, String... values) {
-    checkNotNull(configKey, "header configKey");
+  public RequestTemplate header(String name, String... values) {
+    checkNotNull(name, "header name");
     if (values == null || (values.length == 1 && values[0] == null)) {
-      headers.remove(configKey);
+      headers.remove(name);
     } else {
       List<String> headers = new ArrayList<String>();
       headers.addAll(Arrays.asList(values));
-      this.headers.put(configKey, headers);
+      this.headers.put(name, headers);
     }
     return this;
   }
 
   /* @see #header(String, String...) */
-  public RequestTemplate header(String configKey, Iterable<String> values) {
+  public RequestTemplate header(String name, Iterable<String> values) {
     if (values != null)
-      return header(configKey, toArray(values, String.class));
-    return header(configKey, (String[]) null);
+      return header(name, toArray(values, String.class));
+    return header(name, (String[]) null);
   }
 
   /**

core/src/test/java/feign/DefaultContractTest.java

@@ -233,13 +233,15 @@ public class DefaultContractTest {
 
   interface HeaderParams {
     @RequestLine("POST /")
-    @Headers("Auth-Token: {Auth-Token}") void logout(@Param("Auth-Token") String token);
+    @Headers({"Auth-Token: {Auth-Token}", "Auth-Token: Foo"})
+    void logout(@Param("Auth-Token") String token);
   }
 
   @Test public void headerParamsParseIntoIndexToName() throws Exception {
     MethodMetadata md = contract.parseAndValidatateMetadata(HeaderParams.class.getDeclaredMethod("logout", String.class));
 
-    assertThat(md.template()).hasHeaders(entry("Auth-Token", asList("{Auth-Token}")));
+    assertThat(md.template())
+        .hasHeaders(entry("Auth-Token", asList("{Auth-Token}", "Foo")));
 
     assertThat(md.indexToName())
         .containsExactly(entry(0, asList("Auth-Token")));

core/src/test/java/feign/FeignTest.java

@@ -17,7 +17,6 @@ package feign;
 
 import com.google.gson.Gson;
 import com.squareup.okhttp.mockwebserver.MockResponse;
-import com.squareup.okhttp.mockwebserver.MockWebServer;
 import com.squareup.okhttp.mockwebserver.SocketPolicy;
 import com.squareup.okhttp.mockwebserver.rule.MockWebServerRule;
 import dagger.Module;
@@ -34,9 +33,6 @@ import java.util.Arrays;
 import java.util.List;
 import java.util.Map;
 import javax.inject.Singleton;
-import javax.net.ssl.HostnameVerifier;
-import javax.net.ssl.SSLSession;
-import javax.net.ssl.SSLSocketFactory;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
@@ -156,7 +152,8 @@ public class FeignTest {
   public void postBodyParam() throws IOException, InterruptedException {
     server.enqueue(new MockResponse().setBody("foo"));
 
-    TestInterface api = Feign.create(TestInterface.class, "http://localhost:" + server.getPort(), new TestInterface.Module());
+    TestInterface api = Feign.create(TestInterface.class, "http://localhost:" + server.getPort(),
+        new TestInterface.Module());
 
     api.body(Arrays.asList("netflix", "denominator", "password"));
 
@@ -341,8 +338,7 @@ public class FeignTest {
     thrown.expect(FeignException.class);
     thrown.expectMessage("error reading response POST http://");
 
-    TestInterface api = Feign.create(TestInterface.class, "http://localhost:" + server.getPort(),
-        new IOEOnDecode());
+    TestInterface api = Feign.create(TestInterface.class, "http://localhost:" + server.getPort(), new IOEOnDecode());
 
     try {
       api.post();
@@ -351,72 +347,6 @@ public class FeignTest {
     }
   }
 
-  @Module(overrides = true, includes = TestInterface.Module.class)
-  static class TrustSSLSockets {
-    @Provides SSLSocketFactory trustingSSLSocketFactory() {
-      return TrustingSSLSocketFactory.get();
-    }
-  }
-
-  @Test public void canOverrideSSLSocketFactory() throws IOException, InterruptedException {
-    MockWebServer server = new MockWebServer();
-    server.useHttps(TrustingSSLSocketFactory.get("localhost"), false);
-    server.enqueue(new MockResponse().setBody("success!"));
-    server.play();
-
-    try {
-      TestInterface api = Feign.create(TestInterface.class, "https://localhost:" + server.getPort(),
-          new TrustSSLSockets());
-      api.post();
-    } finally {
-      server.shutdown();
-    }
-  }
-
-  @Module(overrides = true, includes = TrustSSLSockets.class)
-  static class DisableHostnameVerification {
-    @Provides HostnameVerifier acceptAllHostnameVerifier() {
-      return new HostnameVerifier() {
-        @Override
-        public boolean verify(String s, SSLSession sslSession) {
-          return true;
-        }
-      };
-    }
-  }
-
-  @Test public void canOverrideHostnameVerifier() throws IOException, InterruptedException {
-    MockWebServer server = new MockWebServer();
-    server.useHttps(TrustingSSLSocketFactory.get("bad.example.com"), false);
-    server.enqueue(new MockResponse().setBody("success!"));
-    server.play();
-
-    try {
-      TestInterface api = Feign.create(TestInterface.class, "https://localhost:" + server.getPort(),
-          new DisableHostnameVerification());
-      api.post();
-    } finally {
-      server.shutdown();
-    }
-  }
-
-  @Test public void retriesFailedHandshake() throws IOException, InterruptedException {
-    MockWebServer server = new MockWebServer();
-    server.useHttps(TrustingSSLSocketFactory.get("localhost"), false);
-    server.enqueue(new MockResponse().setSocketPolicy(SocketPolicy.FAIL_HANDSHAKE));
-    server.enqueue(new MockResponse().setBody("success!"));
-    server.play();
-
-    try {
-      TestInterface api = Feign.create(TestInterface.class, "https://localhost:" + server.getPort(),
-          new TestInterface.Module(), new TrustSSLSockets());
-      api.post();
-      assertEquals(2, server.getRequestCount());
-    } finally {
-      server.shutdown();
-    }
-  }
-
   @Test public void equalsHashCodeAndToStringWork() {
     Target<TestInterface> t1 = new HardCodedTarget<TestInterface>(TestInterface.class, "http://localhost:8080");
     Target<TestInterface> t2 = new HardCodedTarget<TestInterface>(TestInterface.class, "http://localhost:8888");

core/src/test/java/feign/client/DefaultClientTest.java

@@ -0,0 +1,160 @@
+/*
+ * 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.client;
+
+import com.squareup.okhttp.mockwebserver.MockResponse;
+import com.squareup.okhttp.mockwebserver.SocketPolicy;
+import com.squareup.okhttp.mockwebserver.rule.MockWebServerRule;
+import dagger.Lazy;
+import feign.Client;
+import feign.Feign;
+import feign.FeignException;
+import feign.Headers;
+import feign.RequestLine;
+import feign.Response;
+import java.io.ByteArrayInputStream;
+import java.io.IOException;
+import java.net.ProtocolException;
+import javax.net.ssl.HostnameVerifier;
+import javax.net.ssl.HttpsURLConnection;
+import javax.net.ssl.SSLSession;
+import javax.net.ssl.SSLSocketFactory;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.ExpectedException;
+
+import static feign.Util.UTF_8;
+import static feign.assertj.MockWebServerAssertions.assertThat;
+import static java.util.Arrays.asList;
+import static org.hamcrest.core.Is.isA;
+import static org.junit.Assert.assertEquals;
+
+public class DefaultClientTest {
+  @Rule public final ExpectedException thrown = ExpectedException.none();
+  @Rule public final MockWebServerRule server = new MockWebServerRule();
+
+  interface TestInterface {
+    @RequestLine("POST /?foo=bar&foo=baz&qux=")
+    @Headers({"Foo: Bar", "Foo: Baz", "Qux: ", "Content-Type: text/plain"}) Response post(String body);
+
+    @RequestLine("PATCH /") String patch();
+  }
+
+  @Test public void parsesRequestAndResponse() throws IOException, InterruptedException {
+    server.enqueue(new MockResponse().setBody("foo").addHeader("Foo: Bar"));
+
+    TestInterface api = Feign.builder().target(TestInterface.class, "http://localhost:" + server.getPort());
+
+    Response response = api.post("foo");
+
+    assertThat(response.status()).isEqualTo(200);
+    assertThat(response.reason()).isEqualTo("OK");
+    assertThat(response.headers())
+        .containsEntry("Content-Length", asList("3"))
+        .containsEntry("Foo", asList("Bar"));
+    assertThat(response.body().asInputStream()).hasContentEqualTo(new ByteArrayInputStream("foo".getBytes(UTF_8)));
+
+    assertThat(server.takeRequest()).hasMethod("POST")
+        .hasPath("/?foo=bar&foo=baz&qux=")
+        .hasHeaders("Foo: Bar", "Foo: Baz", "Qux: ", "Content-Length: 3")
+        .hasBody("foo");
+  }
+
+  @Test public void parsesErrorResponse() throws IOException, InterruptedException {
+    thrown.expect(FeignException.class);
+    thrown.expectMessage("status 500 reading TestInterface#post(String); content:\n" + "ARGHH");
+
+    server.enqueue(new MockResponse().setResponseCode(500).setBody("ARGHH"));
+
+    TestInterface api = Feign.builder().target(TestInterface.class, "http://localhost:" + server.getPort());
+
+    api.post("foo");
+  }
+
+  /**
+   * We currently don't include the <a href="http://java.net/jira/browse/JERSEY-639">60-line workaround</a>
+   * jersey uses to overcome the lack of support for PATCH. For now, prefer okhttp.
+   *
+   * @see java.net.HttpURLConnection#setRequestMethod
+   */
+  @Test public void patchUnsupported() throws IOException, InterruptedException {
+    thrown.expectCause(isA(ProtocolException.class));
+
+    TestInterface api = Feign.builder().target(TestInterface.class, "http://localhost:" + server.getPort());
+
+    api.patch();
+  }
+
+  Client trustSSLSockets = new Client.Default(new Lazy<SSLSocketFactory>() {
+    @Override public SSLSocketFactory get() {
+      return TrustingSSLSocketFactory.get();
+    }
+  }, new Lazy<HostnameVerifier>() {
+    @Override public HostnameVerifier get() {
+      return HttpsURLConnection.getDefaultHostnameVerifier();
+    }
+  });
+  
+  @Test public void canOverrideSSLSocketFactory() throws IOException, InterruptedException {
+    server.get().useHttps(TrustingSSLSocketFactory.get("localhost"), false);
+    server.enqueue(new MockResponse());
+
+    TestInterface api = Feign.builder()
+        .client(trustSSLSockets)
+        .target(TestInterface.class, "https://localhost:" + server.getPort());
+
+    api.post("foo");
+  }
+
+  Client disableHostnameVerification = new Client.Default(new Lazy<SSLSocketFactory>() {
+    @Override public SSLSocketFactory get() {
+      return TrustingSSLSocketFactory.get();
+    }
+  }, new Lazy<HostnameVerifier>() {
+    @Override public HostnameVerifier get() {
+      return new HostnameVerifier() {
+        @Override
+        public boolean verify(String s, SSLSession sslSession) {
+          return true;
+        }
+      };
+    }
+  });
+
+  @Test public void canOverrideHostnameVerifier() throws IOException, InterruptedException {
+    server.get().useHttps(TrustingSSLSocketFactory.get("bad.example.com"), false);
+    server.enqueue(new MockResponse());
+
+    TestInterface api = Feign.builder()
+        .client(disableHostnameVerification)
+        .target(TestInterface.class, "https://localhost:" + server.getPort());
+
+    api.post("foo");
+  }
+
+  @Test public void retriesFailedHandshake() throws IOException, InterruptedException {
+    server.get().useHttps(TrustingSSLSocketFactory.get("localhost"), false);
+    server.enqueue(new MockResponse().setSocketPolicy(SocketPolicy.FAIL_HANDSHAKE));
+    server.enqueue(new MockResponse());
+
+    TestInterface api = Feign.builder()
+        .client(trustSSLSockets)
+        .target(TestInterface.class, "https://localhost:" + server.getPort());
+
+    api.post("foo");
+    assertEquals(2, server.getRequestCount());
+  }
+}

core/src/test/java/feign/client/TrustingSSLSocketFactory.java

@@ -13,7 +13,7 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-package feign;
+package feign.client;
 
 import java.io.IOException;
 import java.io.InputStream;