From fa2b366f5462eaff94478e6c1163f9034be88aae Mon Sep 17 00:00:00 2001 From: Dan Richelson Date: Fri, 7 Apr 2017 18:20:09 -0700 Subject: [PATCH 1/2] Remove apache http in favor of okhttp --- .gitignore | 2 + build.gradle | 6 +- .../launchdarkly/client/EventProcessor.java | 55 ++--- .../com/launchdarkly/client/FeatureFlag.java | 6 +- .../launchdarkly/client/FeatureRequestor.java | 128 +++--------- .../com/launchdarkly/client/LDClient.java | 4 +- .../com/launchdarkly/client/LDConfig.java | 189 ++++++++---------- .../launchdarkly/client/PollingProcessor.java | 5 +- .../client/RedisFeatureStoreBuilder.java | 3 +- .../launchdarkly/client/StreamProcessor.java | 24 +-- .../java/com/launchdarkly/client/Util.java | 27 +-- .../launchdarkly/client/VeryBasicFuture.java | 30 --- .../com/launchdarkly/client/LDConfigTest.java | 75 ++----- 13 files changed, 172 insertions(+), 382 deletions(-) delete mode 100644 src/main/java/com/launchdarkly/client/VeryBasicFuture.java diff --git a/.gitignore b/.gitignore index 5bed4f055..a123472de 100644 --- a/.gitignore +++ b/.gitignore @@ -14,3 +14,5 @@ build/ bin/ gradle.properties + +classes/ \ No newline at end of file diff --git a/build.gradle b/build.gradle index 96bcb7b74..d0916ae18 100644 --- a/build.gradle +++ b/build.gradle @@ -19,20 +19,18 @@ repositories { allprojects { group = 'com.launchdarkly' - version = "2.1.0" + version = "2.2.0-SNAPSHOT" sourceCompatibility = 1.7 targetCompatibility = 1.7 } dependencies { - compile "org.apache.httpcomponents:httpclient:4.5.2" - compile "org.apache.httpcomponents:httpclient-cache:4.5.2" compile "commons-codec:commons-codec:1.10" compile "com.google.code.gson:gson:2.7" compile "com.google.guava:guava:19.0" compile "joda-time:joda-time:2.9.3" compile "org.slf4j:slf4j-api:1.7.21" - compile group: "com.launchdarkly", name: "okhttp-eventsource", version: "1.1.1", changing: true + compile group: "com.launchdarkly", name: "okhttp-eventsource", version: "1.2.0-SNAPSHOT", changing: true compile "redis.clients:jedis:2.9.0" testCompile "org.easymock:easymock:3.4" testCompile 'junit:junit:4.12' diff --git a/src/main/java/com/launchdarkly/client/EventProcessor.java b/src/main/java/com/launchdarkly/client/EventProcessor.java index a578e771c..e88f482f7 100644 --- a/src/main/java/com/launchdarkly/client/EventProcessor.java +++ b/src/main/java/com/launchdarkly/client/EventProcessor.java @@ -1,13 +1,10 @@ package com.launchdarkly.client; import com.google.common.util.concurrent.ThreadFactoryBuilder; -import com.google.gson.Gson; -import org.apache.http.client.config.RequestConfig; -import org.apache.http.client.methods.CloseableHttpResponse; -import org.apache.http.client.methods.HttpPost; -import org.apache.http.entity.StringEntity; -import org.apache.http.impl.client.CloseableHttpClient; -import org.apache.http.impl.client.HttpClients; +import okhttp3.MediaType; +import okhttp3.Request; +import okhttp3.RequestBody; +import okhttp3.Response; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -58,19 +55,10 @@ public void flush() { class Consumer implements Runnable { private final Logger logger = LoggerFactory.getLogger(Consumer.class); - private final CloseableHttpClient client; private final LDConfig config; Consumer(LDConfig config) { this.config = config; - RequestConfig requestConfig = RequestConfig.custom() - .setConnectTimeout(config.connectTimeout) - .setSocketTimeout(config.socketTimeout) - .setProxy(config.proxyHost) - .build(); - client = HttpClients.custom() - .setDefaultRequestConfig(requestConfig) - .build(); } @Override @@ -88,29 +76,28 @@ public void flush() { } private void postEvents(List events) { - CloseableHttpResponse response = null; - Gson gson = new Gson(); - String json = gson.toJson(events); + + String json = LDConfig.Gson.toJson(events); logger.debug("Posting " + events.size() + " event(s) to " + config.eventsURI + " with payload: " + json); - HttpPost request = config.postEventsRequest(sdkKey, "/bulk"); - StringEntity entity = new StringEntity(json, "UTF-8"); - entity.setContentType("application/json"); - request.setEntity(entity); + String content = LDConfig.Gson.toJson(events); - try { - response = client.execute(request); - if (Util.handleResponse(logger, request, response)) { - logger.debug("Successfully posted " + events.size() + " event(s)."); + Request request = config.getRequestBuilder(sdkKey) + .url(config.eventsURI.toString() + "/bulk") + .post(RequestBody.create(MediaType.parse("application/json; charset=utf-8"), content)) + .addHeader("Content-Type", "application/json") + .build(); + + logger.debug("Posting " + events.size() + " event(s) using request: " + request); + + try (Response response = config.httpClient.newCall(request).execute()) { + if (!response.isSuccessful()) { + logger.info("Got unexpected response when posting events: " + response); + } else { + logger.debug("Events Response: " + response.code()); } } catch (IOException e) { - logger.error("Unhandled exception in LaunchDarkly client attempting to connect to URI: " + config.eventsURI, e); - } finally { - try { - if (response != null) response.close(); - } catch (IOException e) { - logger.error("Unhandled exception in LaunchDarkly client", e); - } + logger.info("Unhandled exception in LaunchDarkly client when posting events to URL: " + request.url(), e); } } } diff --git a/src/main/java/com/launchdarkly/client/FeatureFlag.java b/src/main/java/com/launchdarkly/client/FeatureFlag.java index 7dbaa0810..5212d0fc2 100644 --- a/src/main/java/com/launchdarkly/client/FeatureFlag.java +++ b/src/main/java/com/launchdarkly/client/FeatureFlag.java @@ -1,6 +1,5 @@ package com.launchdarkly.client; -import com.google.gson.Gson; import com.google.gson.JsonElement; import com.google.gson.reflect.TypeToken; import org.slf4j.Logger; @@ -14,7 +13,6 @@ class FeatureFlag { private final static Logger logger = LoggerFactory.getLogger(FeatureFlag.class); - private static final Gson gson = new Gson(); private static final Type mapType = new TypeToken>() { }.getType(); @@ -31,11 +29,11 @@ class FeatureFlag { private final boolean deleted; static FeatureFlag fromJson(String json) { - return gson.fromJson(json, FeatureFlag.class); + return LDConfig.Gson.fromJson(json, FeatureFlag.class); } static Map fromJsonMap(String json) { - return gson.fromJson(json, mapType); + return LDConfig.Gson.fromJson(json, mapType); } FeatureFlag(String key, int version, boolean on, List prerequisites, String salt, List targets, List rules, VariationOrRollout fallthrough, Integer offVariation, List variations, boolean deleted) { diff --git a/src/main/java/com/launchdarkly/client/FeatureRequestor.java b/src/main/java/com/launchdarkly/client/FeatureRequestor.java index 543c93b91..3ce564d97 100644 --- a/src/main/java/com/launchdarkly/client/FeatureRequestor.java +++ b/src/main/java/com/launchdarkly/client/FeatureRequestor.java @@ -1,15 +1,7 @@ package com.launchdarkly.client; -import org.apache.http.client.cache.CacheResponseStatus; -import org.apache.http.client.cache.HttpCacheContext; -import org.apache.http.client.config.RequestConfig; -import org.apache.http.client.methods.CloseableHttpResponse; -import org.apache.http.client.methods.HttpGet; -import org.apache.http.impl.client.CloseableHttpClient; -import org.apache.http.impl.client.cache.CacheConfig; -import org.apache.http.impl.client.cache.CachingHttpClients; -import org.apache.http.impl.conn.PoolingHttpClientConnectionManager; -import org.apache.http.util.EntityUtils; +import okhttp3.Request; +import okhttp3.Response; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -17,110 +9,50 @@ import java.util.Map; class FeatureRequestor { - - public static final String GET_LATEST_FLAGS_PATH = "/sdk/latest-flags"; + private static final Logger logger = LoggerFactory.getLogger(FeatureRequestor.class); + private static final String GET_LATEST_FLAGS_PATH = "/sdk/latest-flags"; private final String sdkKey; private final LDConfig config; - private final CloseableHttpClient client; - private static final Logger logger = LoggerFactory.getLogger(FeatureRequestor.class); FeatureRequestor(String sdkKey, LDConfig config) { this.sdkKey = sdkKey; this.config = config; - this.client = createClient(); - } - - protected CloseableHttpClient createClient() { - CloseableHttpClient client; - PoolingHttpClientConnectionManager manager = new PoolingHttpClientConnectionManager(); - manager.setMaxTotal(100); - manager.setDefaultMaxPerRoute(20); - - CacheConfig cacheConfig = CacheConfig.custom() - .setMaxCacheEntries(1000) - .setMaxObjectSize(131072) - .setSharedCache(false) - .build(); - - RequestConfig requestConfig = RequestConfig.custom() - .setConnectTimeout(config.connectTimeout) - .setSocketTimeout(config.socketTimeout) - .setProxy(config.proxyHost) - .build(); - client = CachingHttpClients.custom() - .setCacheConfig(cacheConfig) - .setConnectionManager(manager) - .setDefaultRequestConfig(requestConfig) - .build(); - return client; } Map getAllFlags() throws IOException { - HttpCacheContext context = HttpCacheContext.create(); - - HttpGet request = config.getRequest(sdkKey, GET_LATEST_FLAGS_PATH); - - CloseableHttpResponse response = null; - try { - logger.debug("Making request: " + request); - response = client.execute(request, context); - - logCacheResponse(context.getCacheResponseStatus()); - if (!Util.handleResponse(logger, request, response)) { - throw new IOException("Failed to fetch flags"); - } - - String json = EntityUtils.toString(response.getEntity()); - logger.debug("Got response: " + response.toString()); - return FeatureFlag.fromJsonMap(json); - } - finally { - try { - if (response != null) response.close(); - } catch (IOException ignored) { - } - } + String body = get(GET_LATEST_FLAGS_PATH); + return FeatureFlag.fromJsonMap(body); } - void logCacheResponse(CacheResponseStatus status) { - switch (status) { - case CACHE_HIT: - logger.debug("A response was generated from the cache with " + - "no requests sent upstream"); - break; - case CACHE_MODULE_RESPONSE: - logger.debug("The response was generated directly by the " + - "caching module"); - break; - case CACHE_MISS: - logger.debug("The response came from an upstream server"); - break; - case VALIDATED: - logger.debug("The response was generated from the cache " + - "after validating the entry with the origin server"); - break; - } + FeatureFlag getFlag(String featureKey) throws IOException { + String body = get(GET_LATEST_FLAGS_PATH + "/" + featureKey); + return FeatureFlag.fromJson(body); } - FeatureFlag getFlag(String featureKey) throws IOException { - HttpCacheContext context = HttpCacheContext.create(); - HttpGet request = config.getRequest(sdkKey, GET_LATEST_FLAGS_PATH + "/" + featureKey); - CloseableHttpResponse response = null; - try { - response = client.execute(request, context); + private String get(String path) throws IOException { + Request request = config.getRequestBuilder(sdkKey) + .url(config.baseURI.toString() + path) + .get() + .build(); - logCacheResponse(context.getCacheResponseStatus()); + logger.debug("Making request: " + request); - if (!Util.handleResponse(logger, request, response)) { - throw new IOException("Failed to fetch flag"); - } - return FeatureFlag.fromJson(EntityUtils.toString(response.getEntity())); - } - finally { - try { - if (response != null) response.close(); - } catch (IOException ignored) { + try (Response response = config.httpClient.newCall(request).execute()) { + String body = response.body().string(); + + if (!response.isSuccessful()) { + if (response.code() == 401) { + logger.error("[401] Invalid SDK key when accessing URI: " + request.url()); + } + throw new IOException("Unexpected response when retrieving Feature Flag(s): " + response + " using url: " + + request.url() + " with body: " + body); } + logger.debug("Get flag(s) response: " + response.toString() + " with body: " + body); + logger.debug("Cache hit count: " + config.httpClient.cache().hitCount() + " Cache network Count: " + config.httpClient.cache().networkCount()); + logger.debug("Cache response: " + response.cacheResponse()); + logger.debug("Network response: " + response.networkResponse()); + + return body; } } } diff --git a/src/main/java/com/launchdarkly/client/LDClient.java b/src/main/java/com/launchdarkly/client/LDClient.java index 50fc98628..33c32246e 100644 --- a/src/main/java/com/launchdarkly/client/LDClient.java +++ b/src/main/java/com/launchdarkly/client/LDClient.java @@ -5,7 +5,6 @@ import com.google.gson.JsonElement; import com.google.gson.JsonPrimitive; import org.apache.commons.codec.binary.Hex; -import org.apache.http.annotation.ThreadSafe; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -28,11 +27,10 @@ * A client for the LaunchDarkly API. Client instances are thread-safe. Applications should instantiate * a single {@code LDClient} for the lifetime of their application. */ -@ThreadSafe public class LDClient implements LDClientInterface { private static final Logger logger = LoggerFactory.getLogger(LDClient.class); private static final String HMAC_ALGORITHM = "HmacSHA256"; - protected static final String CLIENT_VERSION = getClientVersion(); + static final String CLIENT_VERSION = getClientVersion(); private final LDConfig config; private final String sdkKey; diff --git a/src/main/java/com/launchdarkly/client/LDConfig.java b/src/main/java/com/launchdarkly/client/LDConfig.java index ad7ab7837..654943d06 100644 --- a/src/main/java/com/launchdarkly/client/LDConfig.java +++ b/src/main/java/com/launchdarkly/client/LDConfig.java @@ -1,31 +1,40 @@ package com.launchdarkly.client; -import org.apache.http.HttpHost; -import org.apache.http.client.methods.HttpGet; -import org.apache.http.client.methods.HttpPost; -import org.apache.http.client.utils.URIBuilder; +import com.google.common.io.Files; +import com.google.gson.Gson; +import okhttp3.Cache; +import okhttp3.ConnectionPool; +import okhttp3.OkHttpClient; +import okhttp3.Request; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import java.io.File; +import java.net.InetSocketAddress; +import java.net.Proxy; import java.net.URI; +import java.util.concurrent.TimeUnit; /** * This class exposes advanced configuration options for the {@link LDClient}. Instances of this class must be constructed with a {@link com.launchdarkly.client.LDConfig.Builder}. - * */ public final class LDConfig { + private static final Logger logger = LoggerFactory.getLogger(LDConfig.class); + static final Gson Gson = new Gson(); + private static final URI DEFAULT_BASE_URI = URI.create("https://app.launchdarkly.com"); private static final URI DEFAULT_EVENTS_URI = URI.create("https://events.launchdarkly.com"); private static final URI DEFAULT_STREAM_URI = URI.create("https://stream.launchdarkly.com"); private static final int DEFAULT_CAPACITY = 10000; - private static final int DEFAULT_CONNECT_TIMEOUT = 2000; - private static final int DEFAULT_SOCKET_TIMEOUT = 10000; - private static final int DEFAULT_FLUSH_INTERVAL = 5; + private static final int DEFAULT_CONNECT_TIMEOUT_MILLIS = 2000; + private static final int DEFAULT_SOCKET_TIMEOUT_MILLIS = 10000; + private static final int DEFAULT_FLUSH_INTERVAL_SECONDS = 5; private static final long DEFAULT_POLLING_INTERVAL_MILLIS = 1000L; private static final long DEFAULT_START_WAIT_MILLIS = 5000L; private static final int DEFAULT_SAMPLING_INTERVAL = 0; + private static final long DEFAULT_RECONNECT_TIME_MILLIS = 1000; - private static final Logger logger = LoggerFactory.getLogger(LDConfig.class); + private static final long MAX_HTTP_CACHE_SIZE_BYTES = 10 * 1024 * 1024; // 10 MB protected static final LDConfig DEFAULT = new Builder().build(); @@ -33,10 +42,11 @@ public final class LDConfig { final URI eventsURI; final URI streamURI; final int capacity; - final int connectTimeout; - final int socketTimeout; + final int connectTimeoutMillis; + final int socketTimeoutMillis; final int flushInterval; - final HttpHost proxyHost; + final Proxy proxy; + final OkHttpClient httpClient; final boolean stream; final FeatureStore featureStore; final boolean useLdd; @@ -50,10 +60,10 @@ protected LDConfig(Builder builder) { this.baseURI = builder.baseURI; this.eventsURI = builder.eventsURI; this.capacity = builder.capacity; - this.connectTimeout = builder.connectTimeout; - this.socketTimeout = builder.socketTimeout; - this.flushInterval = builder.flushInterval; - this.proxyHost = builder.proxyHost(); + this.connectTimeoutMillis = builder.connectTimeoutMillis; + this.socketTimeoutMillis = builder.socketTimeoutMillis; + this.flushInterval = builder.flushIntervalSeconds; + this.proxy = builder.proxy(); this.streamURI = builder.streamURI; this.stream = builder.stream; this.featureStore = builder.featureStore; @@ -66,17 +76,35 @@ protected LDConfig(Builder builder) { } this.startWaitMillis = builder.startWaitMillis; this.samplingInterval = builder.samplingInterval; - this.reconnectTimeMs = builder.reconnectTimeMs; + this.reconnectTimeMs = builder.reconnectTimeMillis; + + File cacheDir = Files.createTempDir(); + Cache cache = new Cache(cacheDir, MAX_HTTP_CACHE_SIZE_BYTES); + + OkHttpClient.Builder httpClientBuilder = new OkHttpClient.Builder() + .cache(cache) + .connectionPool(new ConnectionPool(5, 5, TimeUnit.MINUTES)) + .connectTimeout(connectTimeoutMillis, TimeUnit.MILLISECONDS) + .readTimeout(socketTimeoutMillis, TimeUnit.MILLISECONDS) + .writeTimeout(socketTimeoutMillis, TimeUnit.MILLISECONDS) + .retryOnConnectionFailure(true); + + if (proxy != null) { + httpClientBuilder.proxy(proxy); + } + + httpClient = httpClientBuilder + .build(); } /** * A builder that helps construct {@link com.launchdarkly.client.LDConfig} objects. Builder * calls can be chained, enabling the following pattern: - * + *

* 

    * LDConfig config = new LDConfig.Builder()
-   *      .connectTimeout(3)
-   *      .socketTimeout(3)
+   *      .connectTimeoutMillis(3)
+   *      .socketTimeoutMillis(3)
    *      .build()
    *
*/ @@ -84,13 +112,12 @@ public static class Builder { private URI baseURI = DEFAULT_BASE_URI; private URI eventsURI = DEFAULT_EVENTS_URI; private URI streamURI = DEFAULT_STREAM_URI; - private int connectTimeout = DEFAULT_CONNECT_TIMEOUT; - private int socketTimeout = DEFAULT_SOCKET_TIMEOUT; + private int connectTimeoutMillis = DEFAULT_CONNECT_TIMEOUT_MILLIS; + private int socketTimeoutMillis = DEFAULT_SOCKET_TIMEOUT_MILLIS; private int capacity = DEFAULT_CAPACITY; - private int flushInterval = DEFAULT_FLUSH_INTERVAL; - private String proxyHost; + private int flushIntervalSeconds = DEFAULT_FLUSH_INTERVAL_SECONDS; + private String proxyHost = "localhost"; private int proxyPort = -1; - private String proxyScheme; private boolean stream = true; private boolean useLdd = false; private boolean offline = false; @@ -98,7 +125,7 @@ public static class Builder { private FeatureStore featureStore = new InMemoryFeatureStore(); private long startWaitMillis = DEFAULT_START_WAIT_MILLIS; private int samplingInterval = DEFAULT_SAMPLING_INTERVAL; - private long reconnectTimeMs = DEFAULT_RECONNECT_TIME_MILLIS; + private long reconnectTimeMillis = DEFAULT_RECONNECT_TIME_MILLIS; /** * Creates a builder with all configuration parameters set to the default @@ -108,6 +135,7 @@ public Builder() { /** * Set the base URL of the LaunchDarkly server for this configuration + * * @param baseURI the base URL of the LaunchDarkly server for this configuration * @return the builder */ @@ -118,6 +146,7 @@ public Builder baseURI(URI baseURI) { /** * Set the events URL of the LaunchDarkly server for this configuration + * * @param eventsURI the events URL of the LaunchDarkly server for this configuration * @return the builder */ @@ -128,6 +157,7 @@ public Builder eventsURI(URI eventsURI) { /** * Set the base URL of the LaunchDarkly streaming server for this configuration + * * @param streamURI the base URL of the LaunchDarkly streaming server * @return the builder */ @@ -143,6 +173,7 @@ public Builder featureStore(FeatureStore store) { /** * Set whether streaming mode should be enabled. By default, streaming is enabled. + * * @param stream whether streaming mode should be enabled * @return the builder */ @@ -154,56 +185,56 @@ public Builder stream(boolean stream) { /** * Set the connection timeout in seconds for the configuration. This is the time allowed for the underlying HTTP client to connect * to the LaunchDarkly server. The default is 2 seconds. - * + *

*

Both this method and {@link #connectTimeoutMillis(int) connectTimeoutMillis} affect the same property internally.

* * @param connectTimeout the connection timeout in seconds * @return the builder */ public Builder connectTimeout(int connectTimeout) { - this.connectTimeout = connectTimeout * 1000; + this.connectTimeoutMillis = connectTimeout * 1000; return this; } /** * Set the socket timeout in seconds for the configuration. This is the number of seconds between successive packets that the * client will tolerate before flagging an error. The default is 10 seconds. - * + *

*

Both this method and {@link #socketTimeoutMillis(int) socketTimeoutMillis} affect the same property internally.

* * @param socketTimeout the socket timeout in seconds * @return the builder */ public Builder socketTimeout(int socketTimeout) { - this.socketTimeout = socketTimeout * 1000; + this.socketTimeoutMillis = socketTimeout * 1000; return this; } /** * Set the connection timeout in milliseconds for the configuration. This is the time allowed for the underlying HTTP client to connect * to the LaunchDarkly server. The default is 2000 ms. - * - *

Both this method and {@link #connectTimeout(int) connectTimeout} affect the same property internally.

+ *

+ *

Both this method and {@link #connectTimeout(int) connectTimeoutMillis} affect the same property internally.

* * @param connectTimeoutMillis the connection timeout in milliseconds * @return the builder */ public Builder connectTimeoutMillis(int connectTimeoutMillis) { - this.connectTimeout = connectTimeoutMillis; + this.connectTimeoutMillis = connectTimeoutMillis; return this; } /** * Set the socket timeout in milliseconds for the configuration. This is the number of milliseconds between successive packets that the * client will tolerate before flagging an error. The default is 10,000 milliseconds. - * - *

Both this method and {@link #socketTimeout(int) socketTimeout} affect the same property internally.

+ *

+ *

Both this method and {@link #socketTimeout(int) socketTimeoutMillis} affect the same property internally.

* * @param socketTimeoutMillis the socket timeout in milliseconds * @return the builder */ public Builder socketTimeoutMillis(int socketTimeoutMillis) { - this.socketTimeout = socketTimeoutMillis; + this.socketTimeoutMillis = socketTimeoutMillis; return this; } @@ -215,7 +246,7 @@ public Builder socketTimeoutMillis(int socketTimeoutMillis) { * @return the builder */ public Builder flushInterval(int flushInterval) { - this.flushInterval = flushInterval; + this.flushIntervalSeconds = flushInterval; return this; } @@ -233,9 +264,9 @@ public Builder capacity(int capacity) { /** * Set the host to use as an HTTP proxy for making connections to LaunchDarkly. If this is not set, but - * {@link #proxyPort(int)} or {@link #proxyScheme(String)} are specified, this will default to localhost. + * {@link #proxyPort(int)} is specified, this will default to localhost. *

- * If none of {@link #proxyHost(String)}, {@link #proxyPort(int)} or {@link #proxyScheme(String)} are specified, + * If neither {@link #proxyHost(String)} or {@link #proxyPort(int)} are specified, * a proxy will not be used, and {@link LDClient} will connect to LaunchDarkly directly. *

* @@ -248,13 +279,7 @@ public Builder proxyHost(String host) { } /** - * Set the port to use for an HTTP proxy for making connections to LaunchDarkly. If not set (but {@link #proxyHost(String)} - * or {@link #proxyScheme(String)} are specified, the default port for the scheme will be used. - *

- *

- * If none of {@link #proxyHost(String)}, {@link #proxyPort(int)} or {@link #proxyScheme(String)} are specified, - * a proxy will not be used, and {@link LDClient} will connect to LaunchDarkly directly. - *

+ * Set the port to use for an HTTP proxy for making connections to LaunchDarkly. This is required for proxied HTTP connections. * * @param port * @return the builder @@ -265,19 +290,13 @@ public Builder proxyPort(int port) { } /** - * Set the scheme to use for an HTTP proxy for making connections to LaunchDarkly. If not set (but {@link #proxyHost(String)} - * or {@link #proxyPort(int)} are specified, the default https scheme will be used. - *

- *

- * If none of {@link #proxyHost(String)}, {@link #proxyPort(int)} or {@link #proxyScheme(String)} are specified, - * a proxy will not be used, and {@link LDClient} will connect to LaunchDarkly directly. - *

+ * Deprecated. Only HTTP proxies are currently supported. * - * @param scheme + * @param unused * @return the builder */ - public Builder proxyScheme(String scheme) { - this.proxyScheme = scheme; + @Deprecated + public Builder proxyScheme(String unused) { return this; } @@ -321,7 +340,6 @@ public Builder pollingIntervalMillis(long pollingIntervalMillis) { * Setting this to 0 will not block and cause the constructor to return immediately. * Default value: 5000 * - * * @param startWaitMillis milliseconds to wait * @return the builder */ @@ -354,18 +372,17 @@ public Builder samplingInterval(int samplingInterval) { * @return the builder */ public Builder reconnectTimeMs(long reconnectTimeMs) { - this.reconnectTimeMs = reconnectTimeMs; + this.reconnectTimeMillis = reconnectTimeMs; return this; } - HttpHost proxyHost() { - if (this.proxyHost == null && this.proxyPort == -1 && this.proxyScheme == null) { + // returns null if none of the proxy bits were configured. Minimum required part: port. + Proxy proxy() { + if (this.proxyPort == -1) { return null; } else { - String hostname = this.proxyHost == null ? "localhost" : this.proxyHost; - String scheme = this.proxyScheme == null ? "https" : this.proxyScheme; - return new HttpHost(hostname, this.proxyPort, scheme); + return new Proxy(Proxy.Type.HTTP, new InetSocketAddress(proxyHost, proxyPort)); } } @@ -380,47 +397,9 @@ public LDConfig build() { } - private URIBuilder getBuilder() { - return new URIBuilder() - .setScheme(baseURI.getScheme()) - .setHost(baseURI.getHost()) - .setPort(baseURI.getPort()); - } - - private URIBuilder getEventsBuilder() { - return new URIBuilder() - .setScheme(eventsURI.getScheme()) - .setHost(eventsURI.getHost()) - .setPort(eventsURI.getPort()); - } - - HttpGet getRequest(String sdkKey, String path) { - URIBuilder builder = this.getBuilder().setPath(path); - - try { - HttpGet request = new HttpGet(builder.build()); - request.addHeader("Authorization", sdkKey); - request.addHeader("User-Agent", "JavaClient/" + LDClient.CLIENT_VERSION); - - return request; - } catch (Exception e) { - logger.error("Unhandled exception in LaunchDarkly client", e); - return null; - } - } - - HttpPost postEventsRequest(String sdkKey, String path) { - URIBuilder builder = this.getEventsBuilder().setPath(eventsURI.getPath() + path); - - try { - HttpPost request = new HttpPost(builder.build()); - request.addHeader("Authorization", sdkKey); - request.addHeader("User-Agent", "JavaClient/" + LDClient.CLIENT_VERSION); - - return request; - } catch (Exception e) { - logger.error("Unhandled exception in LaunchDarkly client", e); - return null; - } + Request.Builder getRequestBuilder(String sdkKey) { + return new Request.Builder() + .addHeader("Authorization", sdkKey) + .addHeader("User-Agent", "JavaClient/" + LDClient.CLIENT_VERSION); } } \ No newline at end of file diff --git a/src/main/java/com/launchdarkly/client/PollingProcessor.java b/src/main/java/com/launchdarkly/client/PollingProcessor.java index 134f955d7..a0cd8c707 100644 --- a/src/main/java/com/launchdarkly/client/PollingProcessor.java +++ b/src/main/java/com/launchdarkly/client/PollingProcessor.java @@ -1,5 +1,6 @@ package com.launchdarkly.client; +import com.google.common.util.concurrent.SettableFuture; import com.google.common.util.concurrent.ThreadFactoryBuilder; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -38,7 +39,7 @@ public void close() throws IOException { public Future start() { logger.info("Starting LaunchDarkly polling client with interval: " + config.pollingIntervalMillis + " milliseconds"); - final VeryBasicFuture initFuture = new VeryBasicFuture(); + final SettableFuture initFuture = SettableFuture.create(); ThreadFactory threadFactory = new ThreadFactoryBuilder() .setNameFormat("LaunchDarkly-PollingProcessor-%d") .build(); @@ -51,7 +52,7 @@ public void run() { store.init(requestor.getAllFlags()); if (!initialized.getAndSet(true)) { logger.info("Initialized LaunchDarkly client."); - initFuture.completed(null); + initFuture.set(null); } } catch (IOException e) { logger.error("Encountered exception in LaunchDarkly client when retrieving update", e); diff --git a/src/main/java/com/launchdarkly/client/RedisFeatureStoreBuilder.java b/src/main/java/com/launchdarkly/client/RedisFeatureStoreBuilder.java index 6eeb85de7..2d75b2686 100644 --- a/src/main/java/com/launchdarkly/client/RedisFeatureStoreBuilder.java +++ b/src/main/java/com/launchdarkly/client/RedisFeatureStoreBuilder.java @@ -1,6 +1,5 @@ package com.launchdarkly.client; -import org.apache.http.client.utils.URIBuilder; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import redis.clients.jedis.JedisPoolConfig; @@ -56,7 +55,7 @@ public RedisFeatureStoreBuilder(URI uri, long cacheTimeSecs) { * @throws URISyntaxException */ public RedisFeatureStoreBuilder(String scheme, String host, int port, long cacheTimeSecs) throws URISyntaxException { - this.uri = new URIBuilder().setScheme(scheme).setHost(host).setPort(port).build(); + this.uri = new URI(scheme, null, host, port, null, null, null); this.cacheTimeSecs = cacheTimeSecs; } diff --git a/src/main/java/com/launchdarkly/client/StreamProcessor.java b/src/main/java/com/launchdarkly/client/StreamProcessor.java index a27d444db..ef145ea42 100644 --- a/src/main/java/com/launchdarkly/client/StreamProcessor.java +++ b/src/main/java/com/launchdarkly/client/StreamProcessor.java @@ -1,5 +1,6 @@ package com.launchdarkly.client; +import com.google.common.util.concurrent.SettableFuture; import com.google.common.util.concurrent.ThreadFactoryBuilder; import com.google.gson.Gson; import com.launchdarkly.eventsource.EventHandler; @@ -7,8 +8,6 @@ import com.launchdarkly.eventsource.MessageEvent; import com.launchdarkly.eventsource.ReadyState; import okhttp3.Headers; - -import org.apache.http.HttpHost; import org.joda.time.DateTime; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -51,7 +50,7 @@ class StreamProcessor implements UpdateProcessor { @Override public Future start() { - final VeryBasicFuture initFuture = new VeryBasicFuture(); + final SettableFuture initFuture = SettableFuture.create(); Headers headers = new Headers.Builder() .add("Authorization", this.sdkKey) @@ -77,7 +76,7 @@ public void onMessage(String name, MessageEvent event) throws Exception { case PUT: store.init(FeatureFlag.fromJsonMap(event.getData())); if (!initialized.getAndSet(true)) { - initFuture.completed(null); + initFuture.set(null); logger.info("Initialized LaunchDarkly client."); } break; @@ -95,7 +94,7 @@ public void onMessage(String name, MessageEvent event) throws Exception { try { store.init(requestor.getAllFlags()); if (!initialized.getAndSet(true)) { - initFuture.completed(null); + initFuture.set(null); logger.info("Initialized LaunchDarkly client."); } } catch (IOException e) { @@ -133,18 +132,9 @@ public void onError(Throwable throwable) { EventSource.Builder builder = new EventSource.Builder(handler, URI.create(config.streamURI.toASCIIString() + "/flags")) .headers(headers) .reconnectTimeMs(config.reconnectTimeMs); - if (config.proxyHost != null) { - int proxyPort = config.proxyHost.getPort(); - if (proxyPort == -1) { - String scheme = config.proxyHost.getSchemeName(); - if (scheme == "http") - proxyPort = 80; - else if (scheme == "https") - proxyPort = 443; - else - logger.error("Unknown proxy scheme: " + scheme); - } - builder.proxy(config.proxyHost.getHostName(), proxyPort); + + if (config.proxy != null) { + builder.proxy(config.proxy); } es = builder.build(); diff --git a/src/main/java/com/launchdarkly/client/Util.java b/src/main/java/com/launchdarkly/client/Util.java index 2de223022..c3a2643a2 100644 --- a/src/main/java/com/launchdarkly/client/Util.java +++ b/src/main/java/com/launchdarkly/client/Util.java @@ -1,12 +1,8 @@ package com.launchdarkly.client; import com.google.gson.JsonPrimitive; -import org.apache.http.HttpStatus; -import org.apache.http.client.methods.CloseableHttpResponse; -import org.apache.http.client.methods.HttpRequestBase; import org.joda.time.DateTime; import org.joda.time.DateTimeZone; -import org.slf4j.Logger; class Util { /** @@ -14,7 +10,7 @@ class Util { * @param maybeDate wraps either a nubmer or a string that may contain a valid timestamp. * @return null if input is not a valid format. */ - protected static DateTime jsonPrimitiveToDateTime(JsonPrimitive maybeDate) { + static DateTime jsonPrimitiveToDateTime(JsonPrimitive maybeDate) { if (maybeDate.isNumber()) { long millis = maybeDate.getAsLong(); return new DateTime(millis); @@ -28,25 +24,4 @@ protected static DateTime jsonPrimitiveToDateTime(JsonPrimitive maybeDate) { return null; } } - - /** - * Logs an error if the response is not 2xx and returns false. Otherwise returns true. - * - * @param logger for logging responses. - * @param request http request - * @param response http response - * @return whether or not the response's status code is acceptable. - */ - static boolean handleResponse(Logger logger, HttpRequestBase request, CloseableHttpResponse response) { - int statusCode = response.getStatusLine().getStatusCode(); - if (statusCode / 100 == 2) { - return true; - } - if (statusCode == HttpStatus.SC_UNAUTHORIZED) { - logger.error("[401] Invalid SDK key when accessing URI: " + request.getURI().toString()); - } else { - logger.error("[" + statusCode + "] " + response.getStatusLine().getReasonPhrase() + " When accessing URI: " + request.getURI().toString()); - } - return false; - } } diff --git a/src/main/java/com/launchdarkly/client/VeryBasicFuture.java b/src/main/java/com/launchdarkly/client/VeryBasicFuture.java deleted file mode 100644 index 6bcab2811..000000000 --- a/src/main/java/com/launchdarkly/client/VeryBasicFuture.java +++ /dev/null @@ -1,30 +0,0 @@ -package com.launchdarkly.client; - -import org.apache.http.concurrent.BasicFuture; -import org.apache.http.concurrent.FutureCallback; - -import java.util.concurrent.Future; - -/** - * Very Basic {@link Future} implementation extending {@link BasicFuture} with no callback or return value. - */ -public class VeryBasicFuture extends BasicFuture { - - public VeryBasicFuture() { - super(new NoOpFutureCallback()); - } - - static class NoOpFutureCallback implements FutureCallback { - @Override - public void completed(Void result) { - } - - @Override - public void failed(Exception ex) { - } - - @Override - public void cancelled() { - } - } -} diff --git a/src/test/java/com/launchdarkly/client/LDConfigTest.java b/src/test/java/com/launchdarkly/client/LDConfigTest.java index d5078750c..9e2b8736f 100644 --- a/src/test/java/com/launchdarkly/client/LDConfigTest.java +++ b/src/test/java/com/launchdarkly/client/LDConfigTest.java @@ -1,9 +1,9 @@ package com.launchdarkly.client; -import org.apache.http.client.methods.HttpPost; import org.junit.Test; -import java.net.URI; +import java.net.InetSocketAddress; +import java.net.Proxy; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNull; @@ -13,92 +13,53 @@ public class LDConfigTest { public void testConnectTimeoutSpecifiedInSeconds() { LDConfig config = new LDConfig.Builder().connectTimeout(3).build(); - assertEquals(3000, config.connectTimeout); + assertEquals(3000, config.connectTimeoutMillis); } @Test public void testConnectTimeoutSpecifiedInMilliseconds() { LDConfig config = new LDConfig.Builder().connectTimeoutMillis(3000).build(); - assertEquals(3000, config.connectTimeout); + assertEquals(3000, config.connectTimeoutMillis); } @Test public void testSocketTimeoutSpecifiedInSeconds() { LDConfig config = new LDConfig.Builder().socketTimeout(3).build(); - assertEquals(3000, config.socketTimeout); + assertEquals(3000, config.socketTimeoutMillis); } @Test public void testSocketTimeoutSpecifiedInMilliseconds() { LDConfig config = new LDConfig.Builder().socketTimeoutMillis(3000).build(); - assertEquals(3000, config.socketTimeout); + assertEquals(3000, config.socketTimeoutMillis); } @Test public void testNoProxyConfigured() { LDConfig config = new LDConfig.Builder().build(); - - assertNull(config.proxyHost); + assertNull(config.proxy); } @Test - public void testOnlyProxyPortConfiguredHasPort() { - LDConfig config = new LDConfig.Builder().proxyPort(1234).build(); - - assertEquals(1234, config.proxyHost.getPort()); - } - - @Test - public void testOnlyProxyPortConfiguredHasDefaultHost() { - LDConfig config = new LDConfig.Builder().proxyPort(1234).build(); - - assertEquals("localhost", config.proxyHost.getHostName()); + public void testOnlyProxyHostConfiguredIsNull() { + LDConfig config = new LDConfig.Builder().proxyHost("bla").build(); + assertNull(config.proxy); } @Test - public void testOnlyProxyPortConfiguredHasDefaultScheme() { + public void testOnlyProxyPortConfiguredHasPortAndDefaultHost() { LDConfig config = new LDConfig.Builder().proxyPort(1234).build(); - - assertEquals("https", config.proxyHost.getSchemeName()); - } - - @Test - public void testOnlyProxyHostConfiguredHasDefaultPort() { - LDConfig config = new LDConfig.Builder().proxyHost("myproxy.example.com").build(); - - assertEquals(-1, config.proxyHost.getPort()); - } - - @Test - public void testProxyHostConfiguredHasHost() { - LDConfig config = new LDConfig.Builder().proxyHost("myproxy.example.com").build(); - - assertEquals("myproxy.example.com", config.proxyHost.getHostName()); - } - - @Test - public void testProxySchemeConfiguredHasScheme() { - LDConfig config = new LDConfig.Builder().proxyScheme("http").build(); - - assertEquals("http", config.proxyHost.getSchemeName()); + assertEquals(new Proxy(Proxy.Type.HTTP, new InetSocketAddress("localhost", 1234)), config.proxy); } - @Test - public void testDefaultEventsUriIsConstructedProperly(){ - LDConfig config = new LDConfig.Builder().build(); - - HttpPost post = config.postEventsRequest("dummy-api-key", "/bulk"); - assertEquals("https://events.launchdarkly.com/bulk", post.getURI().toString()); - } - - @Test - public void testCustomEventsUriIsConstructedProperly(){ - LDConfig config = new LDConfig.Builder().eventsURI(URI.create("http://localhost:3000/api/events")).build(); - - HttpPost post = config.postEventsRequest("dummy-api-key", "/bulk"); - assertEquals("http://localhost:3000/api/events/bulk", post.getURI().toString()); + public void testProxy() { + LDConfig config = new LDConfig.Builder() + .proxyHost("localhost2") + .proxyPort(4444) + .build(); + assertEquals(new Proxy(Proxy.Type.HTTP, new InetSocketAddress("localhost2", 4444)), config.proxy); } @Test From a80e94ca5a168228616518f6b0b35e281fbc8874 Mon Sep 17 00:00:00 2001 From: Dan Richelson Date: Mon, 10 Apr 2017 17:12:07 -0700 Subject: [PATCH 2/2] Address PR comments --- src/main/java/com/launchdarkly/client/EventProcessor.java | 4 ++-- src/main/java/com/launchdarkly/client/FeatureFlag.java | 4 ++-- src/main/java/com/launchdarkly/client/LDConfig.java | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/main/java/com/launchdarkly/client/EventProcessor.java b/src/main/java/com/launchdarkly/client/EventProcessor.java index e88f482f7..844f3d64f 100644 --- a/src/main/java/com/launchdarkly/client/EventProcessor.java +++ b/src/main/java/com/launchdarkly/client/EventProcessor.java @@ -77,10 +77,10 @@ public void flush() { private void postEvents(List events) { - String json = LDConfig.Gson.toJson(events); + String json = LDConfig.gson.toJson(events); logger.debug("Posting " + events.size() + " event(s) to " + config.eventsURI + " with payload: " + json); - String content = LDConfig.Gson.toJson(events); + String content = LDConfig.gson.toJson(events); Request request = config.getRequestBuilder(sdkKey) .url(config.eventsURI.toString() + "/bulk") diff --git a/src/main/java/com/launchdarkly/client/FeatureFlag.java b/src/main/java/com/launchdarkly/client/FeatureFlag.java index 5212d0fc2..5fd556a97 100644 --- a/src/main/java/com/launchdarkly/client/FeatureFlag.java +++ b/src/main/java/com/launchdarkly/client/FeatureFlag.java @@ -29,11 +29,11 @@ class FeatureFlag { private final boolean deleted; static FeatureFlag fromJson(String json) { - return LDConfig.Gson.fromJson(json, FeatureFlag.class); + return LDConfig.gson.fromJson(json, FeatureFlag.class); } static Map fromJsonMap(String json) { - return LDConfig.Gson.fromJson(json, mapType); + return LDConfig.gson.fromJson(json, mapType); } FeatureFlag(String key, int version, boolean on, List prerequisites, String salt, List targets, List rules, VariationOrRollout fallthrough, Integer offVariation, List variations, boolean deleted) { diff --git a/src/main/java/com/launchdarkly/client/LDConfig.java b/src/main/java/com/launchdarkly/client/LDConfig.java index 654943d06..6524bfbb0 100644 --- a/src/main/java/com/launchdarkly/client/LDConfig.java +++ b/src/main/java/com/launchdarkly/client/LDConfig.java @@ -20,7 +20,7 @@ */ public final class LDConfig { private static final Logger logger = LoggerFactory.getLogger(LDConfig.class); - static final Gson Gson = new Gson(); + static final Gson gson = new Gson(); private static final URI DEFAULT_BASE_URI = URI.create("https://app.launchdarkly.com"); private static final URI DEFAULT_EVENTS_URI = URI.create("https://events.launchdarkly.com"); @@ -266,7 +266,7 @@ public Builder capacity(int capacity) { * Set the host to use as an HTTP proxy for making connections to LaunchDarkly. If this is not set, but * {@link #proxyPort(int)} is specified, this will default to localhost. *

- * If neither {@link #proxyHost(String)} or {@link #proxyPort(int)} are specified, + * If neither {@link #proxyHost(String)} nor {@link #proxyPort(int)} are specified, * a proxy will not be used, and {@link LDClient} will connect to LaunchDarkly directly. *

*