Skip to content

Commit 03bf40a

Browse files
author
Adrian Cole
committed
Allows multiple headers with the same name; Backfills default client tests.
1 parent a4e7289 commit 03bf40a

7 files changed

Lines changed: 185 additions & 85 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
### Version 7.1
22
* Introduces feign.@Param to annotate template parameters. Users must migrate from `javax.inject.@Named` to `feign.@Param` before updating to Feign 8.0.
3+
* Allows multiple headers with the same name.
34

45
### Version 7.0
56
* Expose reflective dispatch hook: InvocationHandlerFactory

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

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
*/
1616
package feign;
1717

18+
import java.util.LinkedHashMap;
1819
import javax.inject.Named;
1920
import java.lang.annotation.Annotation;
2021
import java.lang.reflect.Method;
@@ -149,10 +150,16 @@ protected void processAnnotationOnMethod(MethodMetadata data, Annotation methodA
149150
} else if (annotationType == Headers.class) {
150151
String[] headersToParse = Headers.class.cast(methodAnnotation).value();
151152
checkState(headersToParse.length > 0, "Headers annotation was empty on method %s.", method.getName());
153+
Map<String, Collection<String>> headers = new LinkedHashMap<String, Collection<String>>(headersToParse.length);
152154
for (String header : headersToParse) {
153155
int colon = header.indexOf(':');
154-
data.template().header(header.substring(0, colon), header.substring(colon + 2));
156+
String name = header.substring(0, colon);
157+
if (!headers.containsKey(name)) {
158+
headers.put(name, new ArrayList<String>(1));
159+
}
160+
headers.get(name).add(header.substring(colon + 2));
155161
}
162+
data.template().headers(headers);
156163
}
157164
}
158165

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

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -332,28 +332,28 @@ public Map<String, Collection<String>> queries() {
332332
* template.query(&quot;X-Application-Version&quot;, &quot;{version}&quot;);
333333
* </pre>
334334
*
335-
* @param configKey the configKey of the header
335+
* @param name the name of the header
336336
* @param values can be a single null to imply removing all values. Else no
337337
* values are expected to be null.
338338
* @see #headers()
339339
*/
340-
public RequestTemplate header(String configKey, String... values) {
341-
checkNotNull(configKey, "header configKey");
340+
public RequestTemplate header(String name, String... values) {
341+
checkNotNull(name, "header name");
342342
if (values == null || (values.length == 1 && values[0] == null)) {
343-
headers.remove(configKey);
343+
headers.remove(name);
344344
} else {
345345
List<String> headers = new ArrayList<String>();
346346
headers.addAll(Arrays.asList(values));
347-
this.headers.put(configKey, headers);
347+
this.headers.put(name, headers);
348348
}
349349
return this;
350350
}
351351

352352
/* @see #header(String, String...) */
353-
public RequestTemplate header(String configKey, Iterable<String> values) {
353+
public RequestTemplate header(String name, Iterable<String> values) {
354354
if (values != null)
355-
return header(configKey, toArray(values, String.class));
356-
return header(configKey, (String[]) null);
355+
return header(name, toArray(values, String.class));
356+
return header(name, (String[]) null);
357357
}
358358

359359
/**

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -233,13 +233,15 @@ void login(
233233

234234
interface HeaderParams {
235235
@RequestLine("POST /")
236-
@Headers("Auth-Token: {Auth-Token}") void logout(@Param("Auth-Token") String token);
236+
@Headers({"Auth-Token: {Auth-Token}", "Auth-Token: Foo"})
237+
void logout(@Param("Auth-Token") String token);
237238
}
238239

239240
@Test public void headerParamsParseIntoIndexToName() throws Exception {
240241
MethodMetadata md = contract.parseAndValidatateMetadata(HeaderParams.class.getDeclaredMethod("logout", String.class));
241242

242-
assertThat(md.template()).hasHeaders(entry("Auth-Token", asList("{Auth-Token}")));
243+
assertThat(md.template())
244+
.hasHeaders(entry("Auth-Token", asList("{Auth-Token}", "Foo")));
243245

244246
assertThat(md.indexToName())
245247
.containsExactly(entry(0, asList("Auth-Token")));

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

Lines changed: 3 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717

1818
import com.google.gson.Gson;
1919
import com.squareup.okhttp.mockwebserver.MockResponse;
20-
import com.squareup.okhttp.mockwebserver.MockWebServer;
2120
import com.squareup.okhttp.mockwebserver.SocketPolicy;
2221
import com.squareup.okhttp.mockwebserver.rule.MockWebServerRule;
2322
import dagger.Module;
@@ -34,9 +33,6 @@
3433
import java.util.List;
3534
import java.util.Map;
3635
import javax.inject.Singleton;
37-
import javax.net.ssl.HostnameVerifier;
38-
import javax.net.ssl.SSLSession;
39-
import javax.net.ssl.SSLSocketFactory;
4036
import org.junit.Rule;
4137
import org.junit.Test;
4238
import org.junit.rules.ExpectedException;
@@ -156,7 +152,8 @@ public void postFormParams() throws IOException, InterruptedException {
156152
public void postBodyParam() throws IOException, InterruptedException {
157153
server.enqueue(new MockResponse().setBody("foo"));
158154

159-
TestInterface api = Feign.create(TestInterface.class, "http://localhost:" + server.getPort(), new TestInterface.Module());
155+
TestInterface api = Feign.create(TestInterface.class, "http://localhost:" + server.getPort(),
156+
new TestInterface.Module());
160157

161158
api.body(Arrays.asList("netflix", "denominator", "password"));
162159

@@ -341,8 +338,7 @@ public void doesntRetryAfterResponseIsSent() throws IOException, InterruptedExce
341338
thrown.expect(FeignException.class);
342339
thrown.expectMessage("error reading response POST http://");
343340

344-
TestInterface api = Feign.create(TestInterface.class, "http://localhost:" + server.getPort(),
345-
new IOEOnDecode());
341+
TestInterface api = Feign.create(TestInterface.class, "http://localhost:" + server.getPort(), new IOEOnDecode());
346342

347343
try {
348344
api.post();
@@ -351,72 +347,6 @@ public void doesntRetryAfterResponseIsSent() throws IOException, InterruptedExce
351347
}
352348
}
353349

354-
@Module(overrides = true, includes = TestInterface.Module.class)
355-
static class TrustSSLSockets {
356-
@Provides SSLSocketFactory trustingSSLSocketFactory() {
357-
return TrustingSSLSocketFactory.get();
358-
}
359-
}
360-
361-
@Test public void canOverrideSSLSocketFactory() throws IOException, InterruptedException {
362-
MockWebServer server = new MockWebServer();
363-
server.useHttps(TrustingSSLSocketFactory.get("localhost"), false);
364-
server.enqueue(new MockResponse().setBody("success!"));
365-
server.play();
366-
367-
try {
368-
TestInterface api = Feign.create(TestInterface.class, "https://localhost:" + server.getPort(),
369-
new TrustSSLSockets());
370-
api.post();
371-
} finally {
372-
server.shutdown();
373-
}
374-
}
375-
376-
@Module(overrides = true, includes = TrustSSLSockets.class)
377-
static class DisableHostnameVerification {
378-
@Provides HostnameVerifier acceptAllHostnameVerifier() {
379-
return new HostnameVerifier() {
380-
@Override
381-
public boolean verify(String s, SSLSession sslSession) {
382-
return true;
383-
}
384-
};
385-
}
386-
}
387-
388-
@Test public void canOverrideHostnameVerifier() throws IOException, InterruptedException {
389-
MockWebServer server = new MockWebServer();
390-
server.useHttps(TrustingSSLSocketFactory.get("bad.example.com"), false);
391-
server.enqueue(new MockResponse().setBody("success!"));
392-
server.play();
393-
394-
try {
395-
TestInterface api = Feign.create(TestInterface.class, "https://localhost:" + server.getPort(),
396-
new DisableHostnameVerification());
397-
api.post();
398-
} finally {
399-
server.shutdown();
400-
}
401-
}
402-
403-
@Test public void retriesFailedHandshake() throws IOException, InterruptedException {
404-
MockWebServer server = new MockWebServer();
405-
server.useHttps(TrustingSSLSocketFactory.get("localhost"), false);
406-
server.enqueue(new MockResponse().setSocketPolicy(SocketPolicy.FAIL_HANDSHAKE));
407-
server.enqueue(new MockResponse().setBody("success!"));
408-
server.play();
409-
410-
try {
411-
TestInterface api = Feign.create(TestInterface.class, "https://localhost:" + server.getPort(),
412-
new TestInterface.Module(), new TrustSSLSockets());
413-
api.post();
414-
assertEquals(2, server.getRequestCount());
415-
} finally {
416-
server.shutdown();
417-
}
418-
}
419-
420350
@Test public void equalsHashCodeAndToStringWork() {
421351
Target<TestInterface> t1 = new HardCodedTarget<TestInterface>(TestInterface.class, "http://localhost:8080");
422352
Target<TestInterface> t2 = new HardCodedTarget<TestInterface>(TestInterface.class, "http://localhost:8888");
Lines changed: 160 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,160 @@
1+
/*
2+
* Copyright 2015 Netflix, Inc.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package feign.client;
17+
18+
import com.squareup.okhttp.mockwebserver.MockResponse;
19+
import com.squareup.okhttp.mockwebserver.SocketPolicy;
20+
import com.squareup.okhttp.mockwebserver.rule.MockWebServerRule;
21+
import dagger.Lazy;
22+
import feign.Client;
23+
import feign.Feign;
24+
import feign.FeignException;
25+
import feign.Headers;
26+
import feign.RequestLine;
27+
import feign.Response;
28+
import java.io.ByteArrayInputStream;
29+
import java.io.IOException;
30+
import java.net.ProtocolException;
31+
import javax.net.ssl.HostnameVerifier;
32+
import javax.net.ssl.HttpsURLConnection;
33+
import javax.net.ssl.SSLSession;
34+
import javax.net.ssl.SSLSocketFactory;
35+
import org.junit.Rule;
36+
import org.junit.Test;
37+
import org.junit.rules.ExpectedException;
38+
39+
import static feign.Util.UTF_8;
40+
import static feign.assertj.MockWebServerAssertions.assertThat;
41+
import static java.util.Arrays.asList;
42+
import static org.hamcrest.core.Is.isA;
43+
import static org.junit.Assert.assertEquals;
44+
45+
public class DefaultClientTest {
46+
@Rule public final ExpectedException thrown = ExpectedException.none();
47+
@Rule public final MockWebServerRule server = new MockWebServerRule();
48+
49+
interface TestInterface {
50+
@RequestLine("POST /?foo=bar&foo=baz&qux=")
51+
@Headers({"Foo: Bar", "Foo: Baz", "Qux: ", "Content-Type: text/plain"}) Response post(String body);
52+
53+
@RequestLine("PATCH /") String patch();
54+
}
55+
56+
@Test public void parsesRequestAndResponse() throws IOException, InterruptedException {
57+
server.enqueue(new MockResponse().setBody("foo").addHeader("Foo: Bar"));
58+
59+
TestInterface api = Feign.builder().target(TestInterface.class, "http://localhost:" + server.getPort());
60+
61+
Response response = api.post("foo");
62+
63+
assertThat(response.status()).isEqualTo(200);
64+
assertThat(response.reason()).isEqualTo("OK");
65+
assertThat(response.headers())
66+
.containsEntry("Content-Length", asList("3"))
67+
.containsEntry("Foo", asList("Bar"));
68+
assertThat(response.body().asInputStream()).hasContentEqualTo(new ByteArrayInputStream("foo".getBytes(UTF_8)));
69+
70+
assertThat(server.takeRequest()).hasMethod("POST")
71+
.hasPath("/?foo=bar&foo=baz&qux=")
72+
.hasHeaders("Foo: Bar", "Foo: Baz", "Qux: ", "Content-Length: 3")
73+
.hasBody("foo");
74+
}
75+
76+
@Test public void parsesErrorResponse() throws IOException, InterruptedException {
77+
thrown.expect(FeignException.class);
78+
thrown.expectMessage("status 500 reading TestInterface#post(String); content:\n" + "ARGHH");
79+
80+
server.enqueue(new MockResponse().setResponseCode(500).setBody("ARGHH"));
81+
82+
TestInterface api = Feign.builder().target(TestInterface.class, "http://localhost:" + server.getPort());
83+
84+
api.post("foo");
85+
}
86+
87+
/**
88+
* We currently don't include the <a href="http://java.net/jira/browse/JERSEY-639">60-line workaround</a>
89+
* jersey uses to overcome the lack of support for PATCH. For now, prefer okhttp.
90+
*
91+
* @see java.net.HttpURLConnection#setRequestMethod
92+
*/
93+
@Test public void patchUnsupported() throws IOException, InterruptedException {
94+
thrown.expectCause(isA(ProtocolException.class));
95+
96+
TestInterface api = Feign.builder().target(TestInterface.class, "http://localhost:" + server.getPort());
97+
98+
api.patch();
99+
}
100+
101+
Client trustSSLSockets = new Client.Default(new Lazy<SSLSocketFactory>() {
102+
@Override public SSLSocketFactory get() {
103+
return TrustingSSLSocketFactory.get();
104+
}
105+
}, new Lazy<HostnameVerifier>() {
106+
@Override public HostnameVerifier get() {
107+
return HttpsURLConnection.getDefaultHostnameVerifier();
108+
}
109+
});
110+
111+
@Test public void canOverrideSSLSocketFactory() throws IOException, InterruptedException {
112+
server.get().useHttps(TrustingSSLSocketFactory.get("localhost"), false);
113+
server.enqueue(new MockResponse());
114+
115+
TestInterface api = Feign.builder()
116+
.client(trustSSLSockets)
117+
.target(TestInterface.class, "https://localhost:" + server.getPort());
118+
119+
api.post("foo");
120+
}
121+
122+
Client disableHostnameVerification = new Client.Default(new Lazy<SSLSocketFactory>() {
123+
@Override public SSLSocketFactory get() {
124+
return TrustingSSLSocketFactory.get();
125+
}
126+
}, new Lazy<HostnameVerifier>() {
127+
@Override public HostnameVerifier get() {
128+
return new HostnameVerifier() {
129+
@Override
130+
public boolean verify(String s, SSLSession sslSession) {
131+
return true;
132+
}
133+
};
134+
}
135+
});
136+
137+
@Test public void canOverrideHostnameVerifier() throws IOException, InterruptedException {
138+
server.get().useHttps(TrustingSSLSocketFactory.get("bad.example.com"), false);
139+
server.enqueue(new MockResponse());
140+
141+
TestInterface api = Feign.builder()
142+
.client(disableHostnameVerification)
143+
.target(TestInterface.class, "https://localhost:" + server.getPort());
144+
145+
api.post("foo");
146+
}
147+
148+
@Test public void retriesFailedHandshake() throws IOException, InterruptedException {
149+
server.get().useHttps(TrustingSSLSocketFactory.get("localhost"), false);
150+
server.enqueue(new MockResponse().setSocketPolicy(SocketPolicy.FAIL_HANDSHAKE));
151+
server.enqueue(new MockResponse());
152+
153+
TestInterface api = Feign.builder()
154+
.client(trustSSLSockets)
155+
.target(TestInterface.class, "https://localhost:" + server.getPort());
156+
157+
api.post("foo");
158+
assertEquals(2, server.getRequestCount());
159+
}
160+
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
* See the License for the specific language governing permissions and
1414
* limitations under the License.
1515
*/
16-
package feign;
16+
package feign.client;
1717

1818
import java.io.IOException;
1919
import java.io.InputStream;

0 commit comments

Comments
 (0)