Skip to content

Commit 84eeea5

Browse files
author
Bilal Al
committed
fixed okhttp decorator errors and updated tests
1 parent 568b511 commit 84eeea5

File tree

9 files changed

+79
-52
lines changed

9 files changed

+79
-52
lines changed

‎okhttp-modules/pom.xml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
<version>4.13.0-rc3</version>
99
</parent>
1010
<modelVersion>4.0.0</modelVersion>
11-
11+
<version>4.13.0-rc3</version>
1212
<artifactId>okhttp-modules</artifactId>
1313
<packaging>jar</packaging>
1414
<name>http-modules</name>

‎okhttp-modules/src/main/java/io/split/httpmodules/okhttp/OkHttpClientImpl.java‎

Lines changed: 32 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,19 @@ public SplitHttpResponse get(URI uri, FetchOptions options, Map<String, List<Str
9191
okhttp3.Request.Builder requestBuilder = getRequestBuilder();
9292
requestBuilder.url(uri.toString());
9393
Map<String, List<String>> headers = mergeHeaders(buildBasicHeaders(), additionalHeaders);
94-
requestBuilder = OkHttpRequestDecorator.decorate(headers, requestBuilder, _decorator);
94+
Map<String, List<String>> decorateHeaders = OkHttpRequestDecorator.decorate(headers, _decorator);
95+
Map<String, List<String>> finalHeaders;
96+
if (decorateHeaders.isEmpty()) {
97+
finalHeaders = headers;
98+
} else {
99+
finalHeaders = decorateHeaders;
100+
}
101+
for (Map.Entry<String, List<String>> e : finalHeaders.entrySet()) {
102+
for (String headerValue : e.getValue()) {
103+
requestBuilder.addHeader(e.getKey(), headerValue);
104+
}
105+
}
106+
95107
if (options.cacheControlHeadersEnabled()) {
96108
requestBuilder.addHeader(HEADER_CACHE_CONTROL_NAME, HEADER_CACHE_CONTROL_VALUE);
97109
}
@@ -133,10 +145,21 @@ public SplitHttpResponse post(URI url, String entity,
133145
okhttp3.Request.Builder requestBuilder = getRequestBuilder();
134146
requestBuilder.url(url.toString());
135147
Map<String, List<String>> headers = mergeHeaders(buildBasicHeaders(), additionalHeaders);
136-
requestBuilder = OkHttpRequestDecorator.decorate(headers, requestBuilder, _decorator);
148+
Map<String, List<String>> decorateHeaders = OkHttpRequestDecorator.decorate(headers, _decorator);
149+
Map<String, List<String>> finalHeaders;
150+
if (decorateHeaders.isEmpty()) {
151+
finalHeaders = headers;
152+
} else {
153+
finalHeaders = decorateHeaders;
154+
}
155+
for (Map.Entry<String, List<String>> e : finalHeaders.entrySet()) {
156+
for (String headerValue : e.getValue()) {
157+
requestBuilder.addHeader(e.getKey(), headerValue);
158+
}
159+
}
137160
requestBuilder.addHeader("Accept-Encoding", "gzip");
138161
requestBuilder.addHeader("Content-Type", "application/json");
139-
RequestBody postBody = RequestBody.create(MediaType.parse("application/json; charset=utf-16"), entity);
162+
RequestBody postBody = RequestBody.create(MediaType.parse("application/json; charset=utf-8"), entity);
140163
requestBuilder.post(postBody);
141164

142165
Request request = requestBuilder.build();
@@ -172,7 +195,7 @@ protected Request getRequest(okhttp3.Request.Builder requestBuilder) {
172195
return requestBuilder.build();
173196
}
174197

175-
private Map<String, List<String>> buildBasicHeaders() {
198+
protected Map<String, List<String>> buildBasicHeaders() {
176199
Map<String, List<String>> h = new HashMap<>();
177200
h.put(HEADER_API_KEY, Collections.singletonList("Bearer " + _apikey));
178201
h.put(HEADER_CLIENT_VERSION, Collections.singletonList(_metadata.getSdkVersion()));
@@ -184,16 +207,17 @@ private Map<String, List<String>> buildBasicHeaders() {
184207
return h;
185208
}
186209

187-
private static Map<String, List<String>> mergeHeaders(Map<String, List<String>> headers,
210+
protected Map<String, List<String>> mergeHeaders(Map<String, List<String>> headers,
188211
Map<String, List<String>> toAdd) {
189212
if (toAdd == null || toAdd.size() == 0) {
190213
return headers;
191214
}
192215

193216
for (Map.Entry<String, List<String>> entry : toAdd.entrySet()) {
194-
headers.computeIfPresent(entry.getKey(),
195-
(k, oldValue) -> Stream.concat(oldValue.stream(), entry.getValue().stream())
196-
.collect(Collectors.toList()));
217+
headers.put(entry.getKey(), entry.getValue());
218+
// headers.computeIfPresent(entry.getKey(),
219+
// (k, oldValue) -> Stream.concat(oldValue.stream(), entry.getValue().stream())
220+
// .collect(Collectors.toList()));
197221
}
198222

199223
return headers;

‎okhttp-modules/src/main/java/io/split/httpmodules/okhttp/OkHttpRequestDecorator.java‎

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,8 @@
88

99
class OkHttpRequestDecorator {
1010

11-
public static okhttp3.Request.Builder decorate(Map<String, List<String>> headers, okhttp3.Request.Builder b,
11+
public static Map<String, List<String>> decorate(Map<String, List<String>> headers,
1212
RequestDecorator decorator) {
13-
headers = decorator.decorateHeaders(new RequestContext(headers)).headers();
14-
for (Map.Entry<String, List<String>> e : headers.entrySet()) {
15-
for (String headerValue : e.getValue()) {
16-
b.addHeader(e.getKey(), headerValue);
17-
}
18-
}
19-
return b;
13+
return decorator.decorateHeaders(new RequestContext(headers)).headers();
2014
}
2115
}

‎okhttp-modules/src/test/java/io/split/httpmodules/okhttp/HTTPKerberosAuthIntercepterTest.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ public void testBasicFlow() throws Exception {
6262
okhttp3.Request request = kerberosAuthInterceptor.authenticate(null, response);
6363
assertThat(request.headers("Proxy-authorization"), is(equalTo(Arrays.asList("Negotiate secured-token"))));
6464
}
65-
/*
65+
6666
@Test
6767
public void testKerberosLoginConfiguration() {
6868
Map<String, String> kerberosOptions = new HashMap<String, String>();
@@ -82,7 +82,7 @@ public void testKerberosLoginConfigurationException() {
8282
HTTPKerberosAuthInterceptor.KerberosLoginConfiguration kerberosConfig = new HTTPKerberosAuthInterceptor.KerberosLoginConfiguration();
8383
AppConfigurationEntry[] appConfig = kerberosConfig.getAppConfigurationEntry("");
8484
}
85-
*/
85+
8686
@Test
8787
public void testBuildAuthorizationHeader() throws LoginException, PrivilegedActionException {
8888
System.setProperty("java.security.krb5.conf", "src/test/resources/krb5.conf");

‎okhttp-modules/src/test/java/io/split/httpmodules/okhttp/OkHttpClientImplTest.java‎

Lines changed: 38 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
package io.split.httpmodules.okhttp;
22

3-
import com.sun.tools.javac.util.StringUtils;
43
import org.powermock.api.mockito.PowerMockito;
54
import org.powermock.reflect.Whitebox;
65

@@ -10,7 +9,6 @@
109
import io.split.client.impressions.Impression;
1110
import io.split.client.utils.Json;
1211
import io.split.client.utils.SDKMetadata;
13-
import io.split.client.utils.Utils;
1412
import io.split.client.dtos.SplitHttpResponse.Header;
1513
import io.split.engine.common.FetchOptions;
1614

@@ -58,7 +56,7 @@ public void testGetWithSpecialCharacters() throws IOException, InterruptedExcept
5856
} finally {
5957
br.close();
6058
}
61-
/*
59+
6260
server.enqueue(new MockResponse().setBody(body).addHeader("via", "HTTP/1.1 s_proxy_rio1"));
6361
server.start();
6462
HttpUrl baseUrl = server.url("/v1/");
@@ -76,16 +74,16 @@ public void testGetWithSpecialCharacters() throws IOException, InterruptedExcept
7674
Map<String, List<String>> additionalHeaders = Collections.singletonMap("AdditionalHeader",
7775
Collections.singletonList("add"));
7876
FetchOptions fetchOptions = new FetchOptions.Builder().cacheControlHeaders(true).build();
77+
RequestDecorator requestDecorator = new RequestDecorator(null);
7978
PowerMockito.doCallRealMethod().when(okHttpClientImpl).get(rootTarget, fetchOptions, additionalHeaders);
8079
okhttp3.Request.Builder requestBuilder = new okhttp3.Request.Builder();
8180
requestBuilder.url(rootTarget.toString());
8281
PowerMockito.doReturn(requestBuilder).when(okHttpClientImpl).getRequestBuilder();
83-
PowerMockito.doCallRealMethod().when(okHttpClientImpl).setBasicHeaders(requestBuilder);
82+
PowerMockito.doCallRealMethod().when(okHttpClientImpl).buildBasicHeaders();
8483
Whitebox.setInternalState(okHttpClientImpl, "_metadata", metadata());
8584
Whitebox.setInternalState(okHttpClientImpl, "_apikey", "qwerty");
86-
PowerMockito.doCallRealMethod().when(okHttpClientImpl).setAdditionalAndDecoratedHeaders(requestBuilder, additionalHeaders);
87-
RequestDecorator requestDecorator = new RequestDecorator(null);
88-
// Whitebox.setInternalState(okHttpClientImpl, "_requestDecorator", requestDecorator);
85+
Whitebox.setInternalState(okHttpClientImpl, "_decorator", requestDecorator);
86+
PowerMockito.doCallRealMethod().when(okHttpClientImpl).mergeHeaders(buildBasicHeaders(), additionalHeaders);
8987
PowerMockito.doCallRealMethod().when(okHttpClientImpl).getResponseHeaders(any());
9088
PowerMockito.doReturn(requestBuilder.build()).when(okHttpClientImpl).getRequest(requestBuilder);
9189
PowerMockito.doCallRealMethod().when(okHttpClientImpl).getRequest(requestBuilder);
@@ -148,12 +146,12 @@ public void testGetErrors() throws IOException, InterruptedException {
148146
okhttp3.Request.Builder requestBuilder = new okhttp3.Request.Builder();
149147
requestBuilder.url(rootTarget.toString());
150148
PowerMockito.doReturn(requestBuilder).when(okHttpClientImpl).getRequestBuilder();
151-
PowerMockito.doCallRealMethod().when(okHttpClientImpl).setBasicHeaders(requestBuilder);
149+
PowerMockito.doCallRealMethod().when(okHttpClientImpl).buildBasicHeaders();
152150
Whitebox.setInternalState(okHttpClientImpl, "_metadata", metadata());
153151
Whitebox.setInternalState(okHttpClientImpl, "_apikey", "qwerty");
154-
PowerMockito.doCallRealMethod().when(okHttpClientImpl).setAdditionalAndDecoratedHeaders(requestBuilder, additionalHeaders);
152+
PowerMockito.doCallRealMethod().when(okHttpClientImpl).mergeHeaders(buildBasicHeaders(), additionalHeaders);
155153
RequestDecorator requestDecorator = new RequestDecorator(null);
156-
// Whitebox.setInternalState(okHttpClientImpl, "_requestDecorator", requestDecorator);
154+
Whitebox.setInternalState(okHttpClientImpl, "_decorator", requestDecorator);
157155
PowerMockito.doCallRealMethod().when(okHttpClientImpl).getResponseHeaders(any());
158156

159157
SplitHttpResponse splitHttpResponse = okHttpClientImpl.get(rootTarget,
@@ -215,11 +213,11 @@ public Map<String, List<String>> getHeaderOverrides(RequestContext context) {
215213
okhttp3.Request.Builder requestBuilder = new okhttp3.Request.Builder();
216214
requestBuilder.url(rootTarget.toString());
217215
PowerMockito.doReturn(requestBuilder).when(okHttpClientImpl).getRequestBuilder();
218-
PowerMockito.doCallRealMethod().when(okHttpClientImpl).setBasicHeaders(requestBuilder);
216+
PowerMockito.doCallRealMethod().when(okHttpClientImpl).buildBasicHeaders();
219217
Whitebox.setInternalState(okHttpClientImpl, "_metadata", metadata());
220218
Whitebox.setInternalState(okHttpClientImpl, "_apikey", "qwerty");
221-
// Whitebox.setInternalState(okHttpClientImpl, "_requestDecorator", requestDecorator);
222-
PowerMockito.doCallRealMethod().when(okHttpClientImpl).setAdditionalAndDecoratedHeaders(requestBuilder, null);
219+
Whitebox.setInternalState(okHttpClientImpl, "_decorator", requestDecorator);
220+
PowerMockito.doCallRealMethod().when(okHttpClientImpl).mergeHeaders(buildBasicHeaders(), null);
223221
PowerMockito.doCallRealMethod().when(okHttpClientImpl).getResponseHeaders(any());
224222
FetchOptions options = new FetchOptions.Builder().cacheControlHeaders(true).build();
225223

@@ -230,9 +228,9 @@ public Map<String, List<String>> getHeaderOverrides(RequestContext context) {
230228
Headers requestHeaders = request.getHeaders();
231229

232230
assertThat(requestHeaders.get("Cache-Control"), is(equalTo("no-cache")));
233-
// assertThat(requestHeaders.get("first"), is(equalTo("1")));
234-
// assertThat(requestHeaders.values("second"), is(equalTo(Arrays.asList("2.1","2.2"))));
235-
// assertThat(requestHeaders.get("third"), is(equalTo("3")));
231+
assertThat(requestHeaders.get("first"), is(equalTo("1")));
232+
assertThat(requestHeaders.values("second"), is(equalTo(Arrays.asList("2.1","2.2"))));
233+
assertThat(requestHeaders.get("third"), is(equalTo("3")));
236234
Assert.assertEquals("/splitChanges?since=1234567", request.getPath());
237235
assertThat(request.getMethod(), is(equalTo("GET")));
238236
}
@@ -256,11 +254,11 @@ public void testException() throws URISyntaxException, IOException {
256254
okhttp3.Request.Builder requestBuilder = new okhttp3.Request.Builder();
257255
requestBuilder.url(rootTarget.toString());
258256
PowerMockito.doReturn(requestBuilder).when(okHttpClientImpl).getRequestBuilder();
259-
PowerMockito.doCallRealMethod().when(okHttpClientImpl).setBasicHeaders(requestBuilder);
257+
PowerMockito.doCallRealMethod().when(okHttpClientImpl).buildBasicHeaders();
260258
Whitebox.setInternalState(okHttpClientImpl, "_metadata", metadata());
261259
Whitebox.setInternalState(okHttpClientImpl, "_apikey", "qwerty");
262-
Whitebox.setInternalState(okHttpClientImpl, "_requestDecorator", requestDecorator);
263-
PowerMockito.doCallRealMethod().when(okHttpClientImpl).setAdditionalAndDecoratedHeaders(requestBuilder, null);
260+
Whitebox.setInternalState(okHttpClientImpl, "_decorator", requestDecorator);
261+
PowerMockito.doCallRealMethod().when(okHttpClientImpl).mergeHeaders(buildBasicHeaders(), null);
264262
PowerMockito.doCallRealMethod().when(okHttpClientImpl).getResponseHeaders(any());
265263
FetchOptions options = new FetchOptions.Builder().cacheControlHeaders(true).build();
266264

@@ -294,11 +292,11 @@ public void testPost() throws IOException, InterruptedException {
294292
okhttp3.Request.Builder requestBuilder = new okhttp3.Request.Builder();
295293
requestBuilder.url(rootTarget.toString());
296294
PowerMockito.doReturn(requestBuilder).when(okHttpClientImpl).getRequestBuilder();
297-
PowerMockito.doCallRealMethod().when(okHttpClientImpl).setBasicHeaders(requestBuilder);
295+
PowerMockito.doCallRealMethod().when(okHttpClientImpl).buildBasicHeaders();
298296
Whitebox.setInternalState(okHttpClientImpl, "_metadata", metadata());
299297
Whitebox.setInternalState(okHttpClientImpl, "_apikey", "qwerty");
300-
PowerMockito.doCallRealMethod().when(okHttpClientImpl).setAdditionalAndDecoratedHeaders(requestBuilder, additionalHeaders);
301-
// Whitebox.setInternalState(okHttpClientImpl, "_requestDecorator", requestDecorator);
298+
PowerMockito.doCallRealMethod().when(okHttpClientImpl).mergeHeaders(buildBasicHeaders(), additionalHeaders);
299+
Whitebox.setInternalState(okHttpClientImpl, "_decorator", requestDecorator);
302300
PowerMockito.doCallRealMethod().when(okHttpClientImpl).getResponseHeaders(any());
303301
// Send impressions
304302
List<TestImpressions> toSend = Arrays.asList(new TestImpressions("t1", Arrays.asList(
@@ -358,11 +356,11 @@ public void testPostErrors() throws IOException, InterruptedException {
358356
okhttp3.Request.Builder requestBuilder = new okhttp3.Request.Builder();
359357
requestBuilder.url(rootTarget.toString());
360358
PowerMockito.doReturn(requestBuilder).when(okHttpClientImpl).getRequestBuilder();
361-
PowerMockito.doCallRealMethod().when(okHttpClientImpl).setBasicHeaders(requestBuilder);
359+
PowerMockito.doCallRealMethod().when(okHttpClientImpl).buildBasicHeaders();
362360
Whitebox.setInternalState(okHttpClientImpl, "_metadata", metadata());
363361
Whitebox.setInternalState(okHttpClientImpl, "_apikey", "qwerty");
364-
PowerMockito.doCallRealMethod().when(okHttpClientImpl).setAdditionalAndDecoratedHeaders(requestBuilder, additionalHeaders);
365-
// Whitebox.setInternalState(okHttpClientImpl, "_requestDecorator", requestDecorator);
362+
PowerMockito.doCallRealMethod().when(okHttpClientImpl).mergeHeaders(buildBasicHeaders(), additionalHeaders);
363+
Whitebox.setInternalState(okHttpClientImpl, "_decorator", requestDecorator);
366364
PowerMockito.doCallRealMethod().when(okHttpClientImpl).getResponseHeaders(any());
367365

368366
String data = Json.toJson("<>");
@@ -394,13 +392,14 @@ public void testPosttException() throws URISyntaxException, IOException {
394392
Collections.singletonList("OPTIMIZED"));
395393

396394
okhttp3.Request.Builder requestBuilder = new okhttp3.Request.Builder();
395+
RequestDecorator requestDecorator = new RequestDecorator(null);
397396
requestBuilder.url(rootTarget.toString());
398397
PowerMockito.doReturn(requestBuilder).when(okHttpClientImpl).getRequestBuilder();
399-
PowerMockito.doCallRealMethod().when(okHttpClientImpl).setBasicHeaders(requestBuilder);
398+
PowerMockito.doCallRealMethod().when(okHttpClientImpl).buildBasicHeaders();
400399
Whitebox.setInternalState(okHttpClientImpl, "_metadata", metadata());
401400
Whitebox.setInternalState(okHttpClientImpl, "_apikey", "qwerty");
402-
PowerMockito.doCallRealMethod().when(okHttpClientImpl).setAdditionalAndDecoratedHeaders(requestBuilder, additionalHeaders);
403-
Whitebox.setInternalState(okHttpClientImpl, "_requestDecorator", requestDecorator);
401+
PowerMockito.doCallRealMethod().when(okHttpClientImpl).mergeHeaders(buildBasicHeaders(), additionalHeaders);
402+
Whitebox.setInternalState(okHttpClientImpl, "_decorator", requestDecorator);
404403
PowerMockito.doCallRealMethod().when(okHttpClientImpl).getResponseHeaders(any());
405404

406405
String data = Json.toJson("<>");
@@ -409,12 +408,22 @@ public void testPosttException() throws URISyntaxException, IOException {
409408

410409
SplitHttpResponse splitHttpResponse = okHttpClientImpl.post(rootTarget, data,
411410
additionalHeaders);
412-
*/
413411
}
414412

415413
private SDKMetadata metadata() {
416414
return new SDKMetadata("java-1.2.3", "1.2.3.4", "someIP");
417415
}
418416

417+
private Map<String, List<String>> buildBasicHeaders() {
418+
Map<String, List<String>> h = new HashMap<>();
419+
h.put("Authorization", Collections.singletonList("Bearer qwerty"));
420+
h.put("SplitSDKVersion", Collections.singletonList(metadata().getSdkVersion()));
421+
h.put("SplitSDKMachineIP", Collections.singletonList(metadata().getMachineIp()));
422+
h.put("SplitSDKMachineName", Collections.singletonList(metadata().getMachineName()));
423+
h.put("SplitSDKClientKey", Collections.singletonList("qwerty".length() > 4
424+
? "qwerty".substring("qwerty".length() - 4)
425+
: "qwerty"));
426+
return h;
427+
}
419428

420429
}

‎okhttp-modules/src/test/java/io/split/httpmodules/okhttp/OkHttpModuleTests.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,12 +85,12 @@ public void testCreateClient() throws Exception {
8585
.then((Answer<OkHttpClientImpl>) invocationOnMock -> {
8686
assertThat("qwerty", is(equalTo((String) invocationOnMock.getArguments()[0])));
8787
assertThat(sdkMetadata, is(equalTo((SDKMetadata) invocationOnMock.getArguments()[1])));
88-
// assertThat(requestDecorator, is(equalTo((RequestDecorator) invocationOnMock.getArguments()[2])));
8988
assertThat(proxy, is(equalTo((Proxy) invocationOnMock.getArguments()[2])));
9089
assertThat("bilal@bilal", is(equalTo((String) invocationOnMock.getArguments()[3])));
9190
assertThat(false, is(equalTo((Boolean) invocationOnMock.getArguments()[4])));
9291
assertThat(11000, is(equalTo((Integer) invocationOnMock.getArguments()[5])));
9392
assertThat(12000, is(equalTo((Integer) invocationOnMock.getArguments()[6])));
93+
assertThat(requestDecorator, is(equalTo((RequestDecorator) invocationOnMock.getArguments()[7])));
9494
argsCaptured.set(true);
9595
return mockclient;
9696
}

0 commit comments

Comments
 (0)