allow for basic configuration of HttpClient

Allow the underlying OkHttpClient instance to be configured with a few
basic properties from the Config, if specified. The configurable
properties are:

- connect timeout - `http.client.connectTimeout`
- read timeout - `http.client.readTimeout`
- write timeout - `http.client.writeTimeout`
- connection pool: max idle connections - `http.client.maxIdleConnections`
- connection pool: keep alive duration - `http.client.keepAliveDuration`
- max async requests - `http.client.async.maxRequests`
- max async requests per host - `http.client.async.maxRequestsPerHost`

The default value from OkHttpClient is used for any unspecified
properties (which is the behavior previous to this commit).

Also rename the factory method `HttpClient.create()` to
`HttpClient.createDefault()` to make it clear that the instance being
returned is using all of the default configuration properties.

modules/okhttp-client/src/main/java/com/spotify/apollo/http/client/HttpClient.java

@@ -20,6 +20,8 @@
 package com.spotify.apollo.http.client;
 
 import com.spotify.apollo.environment.IncomingRequestAwareClient;
+
+import com.google.inject.Inject;
 import com.squareup.okhttp.MediaType;
 import com.squareup.okhttp.OkHttpClient;
 import com.squareup.okhttp.Request;
@@ -38,11 +40,18 @@ class HttpClient implements IncomingRequestAwareClient {
 
   private final OkHttpClient client;
 
+  @Inject
   HttpClient(OkHttpClient client) {
     this.client = client;
   }
 
+  @Deprecated
+  /** @deprecated use {@link #createDefault()} instead. */
   public static HttpClient create() {
+    return createDefault();
+  }
+
+  public static HttpClient createDefault() {
     return new HttpClient(new OkHttpClient());
   }
 

modules/okhttp-client/src/main/java/com/spotify/apollo/http/client/HttpClientDecorator.java

@@ -24,17 +24,14 @@
 
 class HttpClientDecorator implements ClientDecorator {
 
-  private static final HttpClientDecorator INSTANCE = new HttpClientDecorator();
+  private final HttpClient httpClient;
 
-  HttpClientDecorator() {
-  }
-
-  public static HttpClientDecorator create() {
-    return INSTANCE;
+  HttpClientDecorator(HttpClient httpClient) {
+    this.httpClient = httpClient;
   }
 
   @Override
   public IncomingRequestAwareClient apply(IncomingRequestAwareClient baseClient) {
-    return ForwardingHttpClient.create(baseClient, HttpClient.create());
+    return ForwardingHttpClient.create(baseClient, this.httpClient);
   }
 }

modules/okhttp-client/src/main/java/com/spotify/apollo/http/client/HttpClientDecoratorProvider.java

@@ -21,12 +21,21 @@
 
 import com.spotify.apollo.environment.ClientDecorator;
 
+import com.google.inject.Inject;
+
 import javax.inject.Provider;
 
 class HttpClientDecoratorProvider implements Provider<ClientDecorator> {
 
+  private final HttpClient httpClient;
+
+  @Inject
+  HttpClientDecoratorProvider(final HttpClient httpClient) {
+    this.httpClient = httpClient;
+  }
+
   @Override
   public ClientDecorator get() {
-    return HttpClientDecorator.create();
+    return new HttpClientDecorator(httpClient);
   }
 }

modules/okhttp-client/src/main/java/com/spotify/apollo/http/client/HttpClientModule.java

@@ -19,12 +19,13 @@
  */
 package com.spotify.apollo.http.client;
 
-import com.google.inject.multibindings.Multibinder;
-
 import com.spotify.apollo.environment.ClientDecorator;
 import com.spotify.apollo.module.AbstractApolloModule;
 import com.spotify.apollo.module.ApolloModule;
 
+import com.google.inject.multibindings.Multibinder;
+import com.squareup.okhttp.OkHttpClient;
+
 public class HttpClientModule extends AbstractApolloModule {
 
   HttpClientModule() {
@@ -38,6 +39,9 @@ public static ApolloModule create() {
   protected void configure() {
     Multibinder.newSetBinder(binder(), ClientDecorator.class)
         .addBinding().toProvider(HttpClientDecoratorProvider.class);
+
+    bind(HttpClient.class).toProvider(HttpClientProvider.class);
+    bind(OkHttpClient.class).toProvider(OkHttpClientProvider.class);
   }
 
   @Override

modules/okhttp-client/src/main/java/com/spotify/apollo/http/client/HttpClientProvider.java

@@ -0,0 +1,20 @@
+package com.spotify.apollo.http.client;
+
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+import com.squareup.okhttp.OkHttpClient;
+
+class HttpClientProvider implements Provider<HttpClient> {
+
+  private final OkHttpClient okHttpClient;
+
+  @Inject
+  HttpClientProvider(OkHttpClient okHttpClient) {
+    this.okHttpClient = okHttpClient;
+  }
+
+  @Override
+  public HttpClient get() {
+    return new HttpClient(okHttpClient);
+  }
+}

modules/okhttp-client/src/main/java/com/spotify/apollo/http/client/OkHttpClientProvider.java

@@ -0,0 +1,108 @@
+/*
+ * -\-\-
+ * Spotify Apollo okhttp Client Module
+ * --
+ * Copyright (C) 2013 - 2015 Spotify AB
+ * --
+ * 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 com.spotify.apollo.http.client;
+
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+import com.squareup.okhttp.ConnectionPool;
+import com.squareup.okhttp.OkHttpClient;
+import com.typesafe.config.Config;
+
+import java.util.Optional;
+import java.util.concurrent.TimeUnit;
+
+import static com.spotify.apollo.environment.ConfigUtil.optionalInt;
+
+class OkHttpClientProvider implements Provider<OkHttpClient> {
+
+  private final OkHttpClientConfig config;
+
+  @Inject
+  OkHttpClientProvider(Config config) {
+    this.config = new OkHttpClientConfig(config);
+  }
+
+  @Override
+  public OkHttpClient get() {
+    final OkHttpClient client = new OkHttpClient();
+
+    //timeouts settings
+    config.connectTimeoutMillis().ifPresent(
+        millis -> client.setConnectTimeout(millis, TimeUnit.MILLISECONDS));
+
+    config.readTimeoutMillis().ifPresent(
+        millis -> client.setReadTimeout(millis, TimeUnit.MILLISECONDS));
+
+    config.writeTimeoutMillis().ifPresent(
+        millis -> client.setWriteTimeout(millis, TimeUnit.MILLISECONDS));
+
+    // connection pool settings
+    client.setConnectionPool(new ConnectionPool(
+        // defaults that come from com.squareup.okhttp.ConnectionPool
+        config.maxIdleConnections().orElse(5),
+        config.connectionKeepAliveDurationMillis().orElse(5 * 60 * 1000)
+    ));
+
+    // async dispatcher settings
+    config.maxAsyncRequests().ifPresent(max -> client.getDispatcher().setMaxRequests(max));
+
+    config.maxAsyncRequestsPerHost().ifPresent(
+        max -> client.getDispatcher().setMaxRequestsPerHost(max));
+
+    return client;
+  }
+
+  private static class OkHttpClientConfig {
+
+    private final Config config;
+
+    OkHttpClientConfig(final Config config) {
+      this.config = config;
+    }
+
+    Optional<Integer> connectTimeoutMillis() {
+      return optionalInt(config, "http.client.connectTimeout");
+    }
+
+    Optional<Integer> readTimeoutMillis() {
+      return optionalInt(config, "http.client.readTimeout");
+    }
+
+    Optional<Integer> writeTimeoutMillis() {
+      return optionalInt(config, "http.client.writeTimeout");
+    }
+
+    Optional<Integer> maxIdleConnections() {
+      return optionalInt(config, "http.client.maxIdleConnections");
+    }
+
+    Optional<Integer> connectionKeepAliveDurationMillis() {
+      return optionalInt(config, "http.client.keepAliveDuration");
+    }
+
+    Optional<Integer> maxAsyncRequests() {
+      return optionalInt(config, "http.client.async.maxRequests");
+    }
+
+    Optional<Integer> maxAsyncRequestsPerHost() {
+      return optionalInt(config, "http.client.async.maxRequestsPerHost");
+    }
+  }
+}

modules/okhttp-client/src/test/java/com/spotify/apollo/http/client/HttpClientTest.java

@@ -76,7 +76,7 @@ public void testSend() throws Exception {
 
     String uri = format("http://localhost:%d/foo.php?bar=baz&qur=quz", mockServerRule.getHttpPort());
     Request request = Request.forUri(uri, "GET");
-    Response<ByteString> response = HttpClient.create()
+    Response<ByteString> response = HttpClient.createDefault()
         .send(request, empty())
         .toCompletableFuture().get();
 
@@ -108,7 +108,7 @@ public void testSendWithBody() throws Exception {
         .withHeader("Content-Type", "application/x-spotify-greeting")
         .withPayload(ByteString.encodeUtf8("hello"));
 
-    Response<ByteString> response = HttpClient.create()
+    Response<ByteString> response = HttpClient.createDefault()
         .send(request, empty())
         .toCompletableFuture().get();
 
@@ -134,7 +134,7 @@ public void testTimeout() throws Exception {
     String uri = format("http://localhost:%d/foo.php", mockServerRule.getHttpPort());
     Request request = Request.forUri(uri, "GET");
 
-    Response<ByteString> response = HttpClient.create()
+    Response<ByteString> response = HttpClient.createDefault()
         .send(request, empty())
         .toCompletableFuture().get();
 
@@ -156,7 +156,7 @@ public void testTimeoutFail() throws Exception {
     Request request = Request.forUri(uri, "GET").withTtl(Duration.ofMillis(200));
 
     thrown.expect(hasCause(instanceOf(SocketTimeoutException.class)));
-      HttpClient.create()
+      HttpClient.createDefault()
           .send(request, empty())
           .toCompletableFuture().get();
   }
@@ -174,7 +174,7 @@ public void testSendWeirdStatus() throws Exception {
 
     String uri = format("http://localhost:%d/foo.php", mockServerRule.getHttpPort());
     Request request = Request.forUri(uri, "GET");
-    final Response<ByteString> response = HttpClient.create()
+    final Response<ByteString> response = HttpClient.createDefault()
         .send(request, empty())
         .toCompletableFuture().get();
 

modules/okhttp-client/src/test/java/com/spotify/apollo/http/client/OkHttpClientProviderTest.java

@@ -0,0 +1,64 @@
+/*
+ * -\-\-
+ * Spotify Apollo okhttp Client Module
+ * --
+ * Copyright (C) 2013 - 2015 Spotify AB
+ * --
+ * 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 com.spotify.apollo.http.client;
+
+import com.squareup.okhttp.OkHttpClient;
+import com.typesafe.config.Config;
+import com.typesafe.config.ConfigFactory;
+
+import org.junit.Test;
+
+import static org.junit.Assert.assertEquals;
+
+public class OkHttpClientProviderTest {
+
+  private static OkHttpClient buildClient(final String str) {
+    final Config config = ConfigFactory.parseString(str);
+    final OkHttpClientProvider provider = new OkHttpClientProvider(config);
+    return provider.get();
+  }
+
+  @Test
+  public void testConnectTimeout() {
+    assertEquals(1234, buildClient("http.client.connectTimeout: 1234").getConnectTimeout());
+  }
+
+  @Test
+  public void testReadTimeout() {
+    assertEquals(444, buildClient("http.client.readTimeout: 444").getReadTimeout());
+  }
+
+  @Test
+  public void testWriteTimeout() {
+    assertEquals(5555, buildClient("http.client.writeTimeout: 5555").getWriteTimeout());
+  }
+
+  @Test
+  public void testMaxRequests() {
+    final OkHttpClient client = buildClient("http.client.async.maxRequests: 72");
+    assertEquals(72, client.getDispatcher().getMaxRequests());
+  }
+
+  @Test
+  public void testMaxRequestsPerHost() {
+    final OkHttpClient client = buildClient("http.client.async.maxRequestsPerHost: 79");
+    assertEquals(79, client.getDispatcher().getMaxRequestsPerHost());
+  }
+}