From a9c3eab558239d21a20db2d510f08a464e8acf15 Mon Sep 17 00:00:00 2001 From: Bilal Al Date: Wed, 13 Mar 2024 10:40:36 -0700 Subject: [PATCH 1/6] updated split change fetcher --- .../split/client/HttpSplitChangeFetcher.java | 39 +++++-------------- .../io/split/client/SplitHttpClientImpl.java | 6 +-- .../client/HttpSplitChangeFetcherTest.java | 23 +++++++---- .../io/split/client/HttpSplitClientTest.java | 14 ++++++- 4 files changed, 41 insertions(+), 41 deletions(-) diff --git a/client/src/main/java/io/split/client/HttpSplitChangeFetcher.java b/client/src/main/java/io/split/client/HttpSplitChangeFetcher.java index 65d37d144..8419c76f4 100644 --- a/client/src/main/java/io/split/client/HttpSplitChangeFetcher.java +++ b/client/src/main/java/io/split/client/HttpSplitChangeFetcher.java @@ -2,6 +2,7 @@ import com.google.common.annotations.VisibleForTesting; import io.split.client.dtos.SplitChange; +import io.split.client.dtos.SplitHttpResponse; import io.split.client.exceptions.UriTooLongException; import io.split.client.utils.Json; import io.split.client.utils.Utils; @@ -10,18 +11,13 @@ import io.split.telemetry.domain.enums.HTTPLatenciesEnum; import io.split.telemetry.domain.enums.ResourceEnum; import io.split.telemetry.storage.TelemetryRuntimeProducer; -import org.apache.hc.client5.http.classic.methods.HttpGet; -import org.apache.hc.client5.http.impl.classic.CloseableHttpClient; -import org.apache.hc.client5.http.impl.classic.CloseableHttpResponse; import org.apache.hc.core5.http.HttpStatus; -import org.apache.hc.core5.http.io.entity.EntityUtils; import org.apache.hc.core5.net.URIBuilder; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import java.net.URI; import java.net.URISyntaxException; -import java.nio.charset.StandardCharsets; import static com.google.common.base.Preconditions.checkNotNull; @@ -38,16 +34,16 @@ public final class HttpSplitChangeFetcher implements SplitChangeFetcher { private static final String HEADER_CACHE_CONTROL_NAME = "Cache-Control"; private static final String HEADER_CACHE_CONTROL_VALUE = "no-cache"; - private final CloseableHttpClient _client; + private final SplitHttpClient _client; private final URI _target; private final TelemetryRuntimeProducer _telemetryRuntimeProducer; - public static HttpSplitChangeFetcher create(CloseableHttpClient client, URI root, TelemetryRuntimeProducer telemetryRuntimeProducer) + public static HttpSplitChangeFetcher create(SplitHttpClient client, URI root, TelemetryRuntimeProducer telemetryRuntimeProducer) throws URISyntaxException { return new HttpSplitChangeFetcher(client, Utils.appendPath(root, "api/splitChanges"), telemetryRuntimeProducer); } - private HttpSplitChangeFetcher(CloseableHttpClient client, URI uri, TelemetryRuntimeProducer telemetryRuntimeProducer) { + private HttpSplitChangeFetcher(SplitHttpClient client, URI uri, TelemetryRuntimeProducer telemetryRuntimeProducer) { _client = client; _target = uri; checkNotNull(_target); @@ -64,7 +60,7 @@ public SplitChange fetch(long since, FetchOptions options) { long start = System.currentTimeMillis(); - CloseableHttpResponse response = null; + SplitHttpResponse response = null; try { URIBuilder uriBuilder = new URIBuilder(_target).addParameter(SINCE, "" + since); @@ -75,38 +71,21 @@ public SplitChange fetch(long since, FetchOptions options) { uriBuilder.addParameter(SETS, "" + options.flagSetsFilter()); } URI uri = uriBuilder.build(); + response = _client.get(uri, options); - HttpGet request = new HttpGet(uri); - if(options.cacheControlHeadersEnabled()) { - request.setHeader(HEADER_CACHE_CONTROL_NAME, HEADER_CACHE_CONTROL_VALUE); - } - - response = _client.execute(request); - - int statusCode = response.getCode(); - - if (_log.isDebugEnabled()) { - _log.debug(String.format("[%s] %s. Status code: %s", request.getMethod(), uri.toURL(), statusCode)); - } + int statusCode = response.statusCode; if (statusCode < HttpStatus.SC_OK || statusCode >= HttpStatus.SC_MULTIPLE_CHOICES) { + _log.warn(String.format("Response status was: %s. Reason: %s", statusCode , response.statusMessage)); _telemetryRuntimeProducer.recordSyncError(ResourceEnum.SPLIT_SYNC, statusCode); - if (statusCode == HttpStatus.SC_REQUEST_URI_TOO_LONG) { - _log.error("The amount of flag sets provided are big causing uri length error."); - throw new UriTooLongException(String.format("Status code: %s. Message: %s", statusCode, response.getReasonPhrase())); - } - _log.warn(String.format("Response status was: %s. Reason: %s", statusCode , response.getReasonPhrase())); throw new IllegalStateException(String.format("Could not retrieve splitChanges since %s; http return code %s", since, statusCode)); } - String json = EntityUtils.toString(response.getEntity(), StandardCharsets.UTF_8); - - return Json.fromJson(json, SplitChange.class); + return Json.fromJson(response.body, SplitChange.class); } catch (Exception e) { throw new IllegalStateException(String.format("Problem fetching splitChanges since %s: %s", since, e), e); } finally { _telemetryRuntimeProducer.recordSyncLatency(HTTPLatenciesEnum.SPLITS, System.currentTimeMillis()-start); - Utils.forceClose(response); } } diff --git a/client/src/main/java/io/split/client/SplitHttpClientImpl.java b/client/src/main/java/io/split/client/SplitHttpClientImpl.java index b38562971..a578b1913 100644 --- a/client/src/main/java/io/split/client/SplitHttpClientImpl.java +++ b/client/src/main/java/io/split/client/SplitHttpClientImpl.java @@ -60,18 +60,18 @@ public SplitHttpResponse get(URI uri, FetchOptions options) { _log.debug(String.format("[%s] %s. Status code: %s", request.getMethod(), uri.toURL(), statusCode)); } + SplitHttpResponse httpResponse = new SplitHttpResponse(); + httpResponse.statusMessage = ""; if (statusCode < HttpStatus.SC_OK || statusCode >= HttpStatus.SC_MULTIPLE_CHOICES) { if (statusCode == HttpStatus.SC_REQUEST_URI_TOO_LONG) { _log.error("The amount of flag sets provided are big causing uri length error."); throw new UriTooLongException(String.format("Status code: %s. Message: %s", statusCode, response.getReasonPhrase())); } _log.warn(String.format("Response status was: %s. Reason: %s", statusCode , response.getReasonPhrase())); - throw new IllegalStateException(String.format("Http get received non-successful return code %s", statusCode)); + httpResponse.statusMessage = response.getReasonPhrase(); } - SplitHttpResponse httpResponse = new SplitHttpResponse(); httpResponse.statusCode = statusCode; httpResponse.body = EntityUtils.toString(response.getEntity(), StandardCharsets.UTF_8); - httpResponse.statusMessage = ""; return httpResponse; } catch (Exception e) { throw new IllegalStateException(String.format("Problem in http get operation: %s", e), e); diff --git a/client/src/test/java/io/split/client/HttpSplitChangeFetcherTest.java b/client/src/test/java/io/split/client/HttpSplitChangeFetcherTest.java index 1784c44ed..e5a70198c 100644 --- a/client/src/test/java/io/split/client/HttpSplitChangeFetcherTest.java +++ b/client/src/test/java/io/split/client/HttpSplitChangeFetcherTest.java @@ -40,7 +40,9 @@ public void testDefaultURL() throws URISyntaxException { URI rootTarget = URI.create("https://api.split.io"); CloseableHttpClient httpClient = HttpClients.custom().build(); Metrics.NoopMetrics metrics = new Metrics.NoopMetrics(); - HttpSplitChangeFetcher fetcher = HttpSplitChangeFetcher.create(httpClient, rootTarget, TELEMETRY_STORAGE); + SplitHttpClient splitHtpClient = SplitHttpClientImpl.create(httpClient, new RequestDecorator(null)); + + HttpSplitChangeFetcher fetcher = HttpSplitChangeFetcher.create(splitHtpClient, rootTarget, TELEMETRY_STORAGE); Assert.assertEquals("https://api.split.io/api/splitChanges", fetcher.getTarget().toString()); } @@ -48,7 +50,9 @@ public void testDefaultURL() throws URISyntaxException { public void testCustomURLNoPathNoBackslash() throws URISyntaxException { URI rootTarget = URI.create("https://kubernetesturl.com/split"); CloseableHttpClient httpClient = HttpClients.custom().build(); - HttpSplitChangeFetcher fetcher = HttpSplitChangeFetcher.create(httpClient, rootTarget, TELEMETRY_STORAGE); + SplitHttpClient splitHtpClient = SplitHttpClientImpl.create(httpClient, new RequestDecorator(null)); + + HttpSplitChangeFetcher fetcher = HttpSplitChangeFetcher.create(splitHtpClient, rootTarget, TELEMETRY_STORAGE); Assert.assertEquals("https://kubernetesturl.com/split/api/splitChanges", fetcher.getTarget().toString()); } @@ -56,7 +60,8 @@ public void testCustomURLNoPathNoBackslash() throws URISyntaxException { public void testCustomURLAppendingPath() throws URISyntaxException { URI rootTarget = URI.create("https://kubernetesturl.com/split/"); CloseableHttpClient httpClient = HttpClients.custom().build(); - HttpSplitChangeFetcher fetcher = HttpSplitChangeFetcher.create(httpClient, rootTarget, TELEMETRY_STORAGE); + SplitHttpClient splitHtpClient = SplitHttpClientImpl.create(httpClient, new RequestDecorator(null)); + HttpSplitChangeFetcher fetcher = HttpSplitChangeFetcher.create(splitHtpClient, rootTarget, TELEMETRY_STORAGE); Assert.assertEquals("https://kubernetesturl.com/split/api/splitChanges", fetcher.getTarget().toString()); } @@ -64,7 +69,8 @@ public void testCustomURLAppendingPath() throws URISyntaxException { public void testCustomURLAppendingPathNoBackslash() throws URISyntaxException { URI rootTarget = URI.create("https://kubernetesturl.com/split"); CloseableHttpClient httpClient = HttpClients.custom().build(); - HttpSplitChangeFetcher fetcher = HttpSplitChangeFetcher.create(httpClient, rootTarget, TELEMETRY_STORAGE); + SplitHttpClient splitHtpClient = SplitHttpClientImpl.create(httpClient, new RequestDecorator(null)); + HttpSplitChangeFetcher fetcher = HttpSplitChangeFetcher.create(splitHtpClient, rootTarget, TELEMETRY_STORAGE); Assert.assertEquals("https://kubernetesturl.com/split/api/splitChanges", fetcher.getTarget().toString()); } @@ -73,8 +79,9 @@ public void testFetcherWithSpecialCharacters() throws URISyntaxException, Invoca URI rootTarget = URI.create("https://api.split.io"); CloseableHttpClient httpClientMock = TestHelper.mockHttpClient("split-change-special-characters.json", HttpStatus.SC_OK); + SplitHttpClient splitHtpClient = SplitHttpClientImpl.create(httpClientMock, new RequestDecorator(null)); - HttpSplitChangeFetcher fetcher = HttpSplitChangeFetcher.create(httpClientMock, rootTarget, TELEMETRY_STORAGE); + HttpSplitChangeFetcher fetcher = HttpSplitChangeFetcher.create(splitHtpClient, rootTarget, TELEMETRY_STORAGE); SplitChange change = fetcher.fetch(1234567, new FetchOptions.Builder().cacheControlHeaders(true).build()); @@ -104,8 +111,9 @@ public void testFetcherWithCDNBypassOption() throws IOException, URISyntaxExcept ArgumentCaptor requestCaptor = ArgumentCaptor.forClass(ClassicHttpRequest.class); CloseableHttpClient httpClientMock = Mockito.mock(CloseableHttpClient.class); when(httpClientMock.execute(requestCaptor.capture())).thenReturn(TestHelper.classicResponseToCloseableMock(response)); + SplitHttpClient splitHtpClient = SplitHttpClientImpl.create(httpClientMock, new RequestDecorator(null)); - HttpSplitChangeFetcher fetcher = HttpSplitChangeFetcher.create(httpClientMock, rootTarget, Mockito.mock(TelemetryRuntimeProducer.class)); + HttpSplitChangeFetcher fetcher = HttpSplitChangeFetcher.create(splitHtpClient, rootTarget, Mockito.mock(TelemetryRuntimeProducer.class)); fetcher.fetch(-1, new FetchOptions.Builder().targetChangeNumber(123).build()); fetcher.fetch(-1, new FetchOptions.Builder().build()); @@ -119,7 +127,8 @@ public void testFetcherWithCDNBypassOption() throws IOException, URISyntaxExcept public void testRandomNumberGeneration() throws URISyntaxException { URI rootTarget = URI.create("https://api.split.io"); CloseableHttpClient httpClientMock = Mockito.mock(CloseableHttpClient.class); - HttpSplitChangeFetcher fetcher = HttpSplitChangeFetcher.create(httpClientMock, rootTarget, Mockito.mock(TelemetryRuntimeProducer.class)); + SplitHttpClient splitHtpClient = SplitHttpClientImpl.create(httpClientMock, new RequestDecorator(null)); + HttpSplitChangeFetcher fetcher = HttpSplitChangeFetcher.create(splitHtpClient, rootTarget, Mockito.mock(TelemetryRuntimeProducer.class)); Set seen = new HashSet<>(); long min = (long)Math.pow(2, 63) * (-1); diff --git a/client/src/test/java/io/split/client/HttpSplitClientTest.java b/client/src/test/java/io/split/client/HttpSplitClientTest.java index 91c946cac..6a2fd010a 100644 --- a/client/src/test/java/io/split/client/HttpSplitClientTest.java +++ b/client/src/test/java/io/split/client/HttpSplitClientTest.java @@ -59,12 +59,24 @@ public void testGetWithSpecialCharacters() throws URISyntaxException, Invocation Assert.assertEquals(2, split.sets.size()); } - @Test(expected = IllegalStateException.class) + @Test public void testGetError() throws URISyntaxException, InvocationTargetException, NoSuchMethodException, IllegalAccessException, IOException { URI rootTarget = URI.create("https://api.split.io/splitChanges?since=1234567"); CloseableHttpClient httpClientMock = TestHelper.mockHttpClient("split-change-special-characters.json", HttpStatus.SC_INTERNAL_SERVER_ERROR); RequestDecorator decorator = new RequestDecorator(null); + SplitHttpClient splitHtpClient = SplitHttpClientImpl.create(httpClientMock, decorator); + SplitHttpResponse splitHttpResponse = splitHtpClient.get(rootTarget, + new FetchOptions.Builder().cacheControlHeaders(true).build()); + Assert.assertEquals(HttpStatus.SC_INTERNAL_SERVER_ERROR, (long) splitHttpResponse.statusCode); + } + + @Test(expected = IllegalStateException.class) + public void testException() throws URISyntaxException, InvocationTargetException, NoSuchMethodException, IllegalAccessException, IOException { + URI rootTarget = URI.create("https://api.split.io/splitChanges?since=1234567"); + CloseableHttpClient httpClientMock = TestHelper.mockHttpClient("split-change-special-characters.json", HttpStatus.SC_INTERNAL_SERVER_ERROR); + RequestDecorator decorator = null; + SplitHttpClient splitHtpClient = SplitHttpClientImpl.create(httpClientMock, decorator); splitHtpClient.get(rootTarget, new FetchOptions.Builder().cacheControlHeaders(true).build()); From 8041204c760f3bd613b20c87247cdf20a0fc773c Mon Sep 17 00:00:00 2001 From: Bilal Al Date: Wed, 13 Mar 2024 11:42:11 -0700 Subject: [PATCH 2/6] moved flagset warning out of splithttp client and moved the client class to service --- .../java/io/split/client/HttpSplitChangeFetcher.java | 6 +++++- .../src/main/java/io/split/client/RequestDecorator.java | 2 +- .../io/split/{client => service}/SplitHttpClient.java | 2 +- .../split/{client => service}/SplitHttpClientImpl.java | 9 ++------- .../java/io/split/client/HttpSplitChangeFetcherTest.java | 2 ++ .../test/java/io/split/client/HttpSplitClientTest.java | 6 ++---- 6 files changed, 13 insertions(+), 14 deletions(-) rename client/src/main/java/io/split/{client => service}/SplitHttpClient.java (95%) rename client/src/main/java/io/split/{client => service}/SplitHttpClientImpl.java (91%) diff --git a/client/src/main/java/io/split/client/HttpSplitChangeFetcher.java b/client/src/main/java/io/split/client/HttpSplitChangeFetcher.java index 8419c76f4..609070b7d 100644 --- a/client/src/main/java/io/split/client/HttpSplitChangeFetcher.java +++ b/client/src/main/java/io/split/client/HttpSplitChangeFetcher.java @@ -8,6 +8,7 @@ import io.split.client.utils.Utils; import io.split.engine.common.FetchOptions; import io.split.engine.experiments.SplitChangeFetcher; +import io.split.service.SplitHttpClient; import io.split.telemetry.domain.enums.HTTPLatenciesEnum; import io.split.telemetry.domain.enums.ResourceEnum; import io.split.telemetry.storage.TelemetryRuntimeProducer; @@ -76,7 +77,10 @@ public SplitChange fetch(long since, FetchOptions options) { int statusCode = response.statusCode; if (statusCode < HttpStatus.SC_OK || statusCode >= HttpStatus.SC_MULTIPLE_CHOICES) { - _log.warn(String.format("Response status was: %s. Reason: %s", statusCode , response.statusMessage)); + if (statusCode == HttpStatus.SC_REQUEST_URI_TOO_LONG) { + _log.error("The amount of flag sets provided are big causing uri length error."); + throw new UriTooLongException(String.format("Status code: %s. Message: %s", statusCode, response.statusMessage)); + } _telemetryRuntimeProducer.recordSyncError(ResourceEnum.SPLIT_SYNC, statusCode); throw new IllegalStateException(String.format("Could not retrieve splitChanges since %s; http return code %s", since, statusCode)); } diff --git a/client/src/main/java/io/split/client/RequestDecorator.java b/client/src/main/java/io/split/client/RequestDecorator.java index de4695544..49d491868 100644 --- a/client/src/main/java/io/split/client/RequestDecorator.java +++ b/client/src/main/java/io/split/client/RequestDecorator.java @@ -16,7 +16,7 @@ public Map getHeaderOverrides() { } } -class RequestDecorator { +public final class RequestDecorator { UserCustomHeaderDecorator _headerDecorator; private static final Set forbiddenHeaders = new HashSet<>(Arrays.asList( diff --git a/client/src/main/java/io/split/client/SplitHttpClient.java b/client/src/main/java/io/split/service/SplitHttpClient.java similarity index 95% rename from client/src/main/java/io/split/client/SplitHttpClient.java rename to client/src/main/java/io/split/service/SplitHttpClient.java index 6d54d768f..637510e78 100644 --- a/client/src/main/java/io/split/client/SplitHttpClient.java +++ b/client/src/main/java/io/split/service/SplitHttpClient.java @@ -1,4 +1,4 @@ -package io.split.client; +package io.split.service; import io.split.engine.common.FetchOptions; import io.split.telemetry.domain.enums.HttpParamsWrapper; diff --git a/client/src/main/java/io/split/client/SplitHttpClientImpl.java b/client/src/main/java/io/split/service/SplitHttpClientImpl.java similarity index 91% rename from client/src/main/java/io/split/client/SplitHttpClientImpl.java rename to client/src/main/java/io/split/service/SplitHttpClientImpl.java index a578b1913..92cab6659 100644 --- a/client/src/main/java/io/split/client/SplitHttpClientImpl.java +++ b/client/src/main/java/io/split/service/SplitHttpClientImpl.java @@ -1,7 +1,6 @@ -package io.split.client; +package io.split.service; -import io.split.client.exceptions.UriTooLongException; -import io.split.client.utils.Json; +import io.split.client.RequestDecorator; import io.split.client.utils.Utils; import io.split.engine.common.FetchOptions; import io.split.client.dtos.SplitHttpResponse; @@ -63,10 +62,6 @@ public SplitHttpResponse get(URI uri, FetchOptions options) { SplitHttpResponse httpResponse = new SplitHttpResponse(); httpResponse.statusMessage = ""; if (statusCode < HttpStatus.SC_OK || statusCode >= HttpStatus.SC_MULTIPLE_CHOICES) { - if (statusCode == HttpStatus.SC_REQUEST_URI_TOO_LONG) { - _log.error("The amount of flag sets provided are big causing uri length error."); - throw new UriTooLongException(String.format("Status code: %s. Message: %s", statusCode, response.getReasonPhrase())); - } _log.warn(String.format("Response status was: %s. Reason: %s", statusCode , response.getReasonPhrase())); httpResponse.statusMessage = response.getReasonPhrase(); } diff --git a/client/src/test/java/io/split/client/HttpSplitChangeFetcherTest.java b/client/src/test/java/io/split/client/HttpSplitChangeFetcherTest.java index e5a70198c..794a7f057 100644 --- a/client/src/test/java/io/split/client/HttpSplitChangeFetcherTest.java +++ b/client/src/test/java/io/split/client/HttpSplitChangeFetcherTest.java @@ -5,6 +5,8 @@ import io.split.client.dtos.SplitChange; import io.split.engine.common.FetchOptions; import io.split.engine.metrics.Metrics; +import io.split.service.SplitHttpClient; +import io.split.service.SplitHttpClientImpl; import io.split.telemetry.storage.InMemoryTelemetryStorage; import io.split.telemetry.storage.TelemetryRuntimeProducer; import io.split.telemetry.storage.TelemetryStorage; diff --git a/client/src/test/java/io/split/client/HttpSplitClientTest.java b/client/src/test/java/io/split/client/HttpSplitClientTest.java index 6a2fd010a..b8effa2ae 100644 --- a/client/src/test/java/io/split/client/HttpSplitClientTest.java +++ b/client/src/test/java/io/split/client/HttpSplitClientTest.java @@ -8,17 +8,15 @@ import io.split.client.utils.Json; import io.split.client.utils.Utils; import io.split.engine.common.FetchOptions; -import io.split.telemetry.storage.InMemoryTelemetryStorage; -import io.split.telemetry.storage.TelemetryStorage; +import io.split.service.SplitHttpClient; +import io.split.service.SplitHttpClientImpl; import org.apache.hc.client5.http.classic.methods.HttpPost; import org.apache.hc.client5.http.classic.methods.HttpUriRequest; import org.apache.hc.client5.http.impl.classic.CloseableHttpClient; -import io.split.telemetry.domain.enums.HttpParamsWrapper; import org.apache.hc.core5.http.*; import org.junit.Assert; import org.junit.Test; import org.mockito.ArgumentCaptor; -import org.mockito.Mockito; import java.io.IOException; import java.io.InputStreamReader; From ab42b943fba36c915b8ca36c12e1b4c52327dde3 Mon Sep 17 00:00:00 2001 From: Bilal Al Date: Thu, 14 Mar 2024 10:54:36 -0700 Subject: [PATCH 3/6] updated segment fetcher class --- .../client/HttpSegmentChangeFetcher.java | 36 +++++-------------- .../client/HttpSegmentChangeFetcherTest.java | 21 +++++++---- .../client/HttpSplitChangeFetcherTest.java | 2 +- 3 files changed, 25 insertions(+), 34 deletions(-) diff --git a/client/src/main/java/io/split/client/HttpSegmentChangeFetcher.java b/client/src/main/java/io/split/client/HttpSegmentChangeFetcher.java index 84bf09509..7014646d6 100644 --- a/client/src/main/java/io/split/client/HttpSegmentChangeFetcher.java +++ b/client/src/main/java/io/split/client/HttpSegmentChangeFetcher.java @@ -2,26 +2,23 @@ import com.google.common.annotations.VisibleForTesting; import io.split.client.dtos.SegmentChange; +import io.split.client.dtos.SplitHttpResponse; import io.split.client.utils.Json; import io.split.client.utils.Utils; import io.split.engine.common.FetchOptions; import io.split.engine.segments.SegmentChangeFetcher; +import io.split.service.SplitHttpClient; import io.split.telemetry.domain.enums.HTTPLatenciesEnum; import io.split.telemetry.domain.enums.LastSynchronizationRecordsEnum; import io.split.telemetry.domain.enums.ResourceEnum; import io.split.telemetry.storage.TelemetryRuntimeProducer; -import org.apache.hc.client5.http.classic.methods.HttpGet; -import org.apache.hc.client5.http.impl.classic.CloseableHttpClient; -import org.apache.hc.client5.http.impl.classic.CloseableHttpResponse; import org.apache.hc.core5.http.HttpStatus; -import org.apache.hc.core5.http.io.entity.EntityUtils; import org.apache.hc.core5.net.URIBuilder; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import java.net.URI; import java.net.URISyntaxException; -import java.nio.charset.StandardCharsets; import static com.google.common.base.Preconditions.checkNotNull; @@ -37,16 +34,16 @@ public final class HttpSegmentChangeFetcher implements SegmentChangeFetcher { private static final String CACHE_CONTROL_HEADER_NAME = "Cache-Control"; private static final String CACHE_CONTROL_HEADER_VALUE = "no-cache"; - private final CloseableHttpClient _client; + private final SplitHttpClient _client; private final URI _target; private final TelemetryRuntimeProducer _telemetryRuntimeProducer; - public static HttpSegmentChangeFetcher create(CloseableHttpClient client, URI root, TelemetryRuntimeProducer telemetryRuntimeProducer) + public static HttpSegmentChangeFetcher create(SplitHttpClient client, URI root, TelemetryRuntimeProducer telemetryRuntimeProducer) throws URISyntaxException { return new HttpSegmentChangeFetcher(client, Utils.appendPath(root, "api/segmentChanges"), telemetryRuntimeProducer); } - private HttpSegmentChangeFetcher(CloseableHttpClient client, URI uri, TelemetryRuntimeProducer telemetryRuntimeProducer) { + private HttpSegmentChangeFetcher(SplitHttpClient client, URI uri, TelemetryRuntimeProducer telemetryRuntimeProducer) { _client = client; _target = uri; checkNotNull(_target); @@ -57,7 +54,7 @@ private HttpSegmentChangeFetcher(CloseableHttpClient client, URI uri, TelemetryR public SegmentChange fetch(String segmentName, long since, FetchOptions options) { long start = System.currentTimeMillis(); - CloseableHttpResponse response = null; + SplitHttpResponse response = null; try { String path = _target.getPath() + "/" + segmentName; @@ -69,23 +66,12 @@ public SegmentChange fetch(String segmentName, long since, FetchOptions options) } URI uri = uriBuilder.build(); - HttpGet request = new HttpGet(uri); - if(options.cacheControlHeadersEnabled()) { - request.setHeader(CACHE_CONTROL_HEADER_NAME, CACHE_CONTROL_HEADER_VALUE); - } - - response = _client.execute(request); - - int statusCode = response.getCode(); - - if (_log.isDebugEnabled()) { - _log.debug(String.format("[%s] %s. Status code: %s", request.getMethod(), uri.toURL(), statusCode)); - } + response = _client.get(uri, options); + int statusCode = response.statusCode; if (statusCode < HttpStatus.SC_OK || statusCode >= HttpStatus.SC_MULTIPLE_CHOICES) { _telemetryRuntimeProducer.recordSyncError(ResourceEnum.SEGMENT_SYNC, statusCode); - _log.error(String.format("Response status was: %s. Reason: %s", statusCode , response.getReasonPhrase())); if (statusCode == HttpStatus.SC_FORBIDDEN) { _log.error("factory instantiation: you passed a client side type sdkKey, " + "please grab an sdk key from the Split user interface that is of type server side"); @@ -93,18 +79,14 @@ public SegmentChange fetch(String segmentName, long since, FetchOptions options) throw new IllegalStateException(String.format("Could not retrieve segment changes for %s, since %s; http return code %s", segmentName, since, statusCode)); } - _telemetryRuntimeProducer.recordSuccessfulSync(LastSynchronizationRecordsEnum.SEGMENTS, System.currentTimeMillis()); - String json = EntityUtils.toString(response.getEntity(), StandardCharsets.UTF_8); - - return Json.fromJson(json, SegmentChange.class); + return Json.fromJson(response.body, SegmentChange.class); } catch (Exception e) { throw new IllegalStateException(String.format("Error occurred when trying to sync segment: %s, since: %s. Details: %s", segmentName, since, e), e); } finally { _telemetryRuntimeProducer.recordSyncLatency(HTTPLatenciesEnum.SEGMENTS, System.currentTimeMillis()-start); - Utils.forceClose(response); } diff --git a/client/src/test/java/io/split/client/HttpSegmentChangeFetcherTest.java b/client/src/test/java/io/split/client/HttpSegmentChangeFetcherTest.java index be1fb4b3c..e40c61dd9 100644 --- a/client/src/test/java/io/split/client/HttpSegmentChangeFetcherTest.java +++ b/client/src/test/java/io/split/client/HttpSegmentChangeFetcherTest.java @@ -4,6 +4,8 @@ import io.split.client.dtos.SegmentChange; import io.split.engine.common.FetchOptions; import io.split.engine.metrics.Metrics; +import io.split.service.SplitHttpClient; +import io.split.service.SplitHttpClientImpl; import io.split.telemetry.storage.InMemoryTelemetryStorage; import io.split.telemetry.storage.TelemetryStorage; import org.apache.hc.client5.http.impl.classic.CloseableHttpClient; @@ -31,8 +33,9 @@ public class HttpSegmentChangeFetcherTest { public void testDefaultURL() throws URISyntaxException { URI rootTarget = URI.create("https://api.split.io"); CloseableHttpClient httpClient = HttpClients.custom().build(); + SplitHttpClient splitHtpClient = SplitHttpClientImpl.create(httpClient, new RequestDecorator(null)); Metrics.NoopMetrics metrics = new Metrics.NoopMetrics(); - HttpSegmentChangeFetcher fetcher = HttpSegmentChangeFetcher.create(httpClient, rootTarget, TELEMETRY_STORAGE); + HttpSegmentChangeFetcher fetcher = HttpSegmentChangeFetcher.create(splitHtpClient, rootTarget, TELEMETRY_STORAGE); Assert.assertThat(fetcher.getTarget().toString(), Matchers.is(Matchers.equalTo("https://api.split.io/api/segmentChanges"))); } @@ -40,8 +43,9 @@ public void testDefaultURL() throws URISyntaxException { public void testCustomURLNoPathNoBackslash() throws URISyntaxException { URI rootTarget = URI.create("https://kubernetesturl.com/split"); CloseableHttpClient httpClient = HttpClients.custom().build(); + SplitHttpClient splitHtpClient = SplitHttpClientImpl.create(httpClient, new RequestDecorator(null)); Metrics.NoopMetrics metrics = new Metrics.NoopMetrics(); - HttpSegmentChangeFetcher fetcher = HttpSegmentChangeFetcher.create(httpClient, rootTarget, TELEMETRY_STORAGE); + HttpSegmentChangeFetcher fetcher = HttpSegmentChangeFetcher.create(splitHtpClient, rootTarget, TELEMETRY_STORAGE); Assert.assertThat(fetcher.getTarget().toString(), Matchers.is(Matchers.equalTo("https://kubernetesturl.com/split/api/segmentChanges"))); } @@ -49,8 +53,9 @@ public void testCustomURLNoPathNoBackslash() throws URISyntaxException { public void testCustomURLAppendingPath() throws URISyntaxException { URI rootTarget = URI.create("https://kubernetesturl.com/split/"); CloseableHttpClient httpClient = HttpClients.custom().build(); + SplitHttpClient splitHtpClient = SplitHttpClientImpl.create(httpClient, new RequestDecorator(null)); Metrics.NoopMetrics metrics = new Metrics.NoopMetrics(); - HttpSegmentChangeFetcher fetcher = HttpSegmentChangeFetcher.create(httpClient, rootTarget, TELEMETRY_STORAGE); + HttpSegmentChangeFetcher fetcher = HttpSegmentChangeFetcher.create(splitHtpClient, rootTarget, TELEMETRY_STORAGE); Assert.assertThat(fetcher.getTarget().toString(), Matchers.is(Matchers.equalTo("https://kubernetesturl.com/split/api/segmentChanges"))); } @@ -58,8 +63,9 @@ public void testCustomURLAppendingPath() throws URISyntaxException { public void testCustomURLAppendingPathNoBackslash() throws URISyntaxException { URI rootTarget = URI.create("https://kubernetesturl.com/split"); CloseableHttpClient httpClient = HttpClients.custom().build(); + SplitHttpClient splitHtpClient = SplitHttpClientImpl.create(httpClient, new RequestDecorator(null)); Metrics.NoopMetrics metrics = new Metrics.NoopMetrics(); - HttpSegmentChangeFetcher fetcher = HttpSegmentChangeFetcher.create(httpClient, rootTarget, TELEMETRY_STORAGE); + HttpSegmentChangeFetcher fetcher = HttpSegmentChangeFetcher.create(splitHtpClient, rootTarget, TELEMETRY_STORAGE); Assert.assertThat(fetcher.getTarget().toString(), Matchers.is(Matchers.equalTo("https://kubernetesturl.com/split/api/segmentChanges"))); } @@ -68,9 +74,10 @@ public void testFetcherWithSpecialCharacters() throws URISyntaxException, IOExce URI rootTarget = URI.create("https://api.split.io/api/segmentChanges"); CloseableHttpClient httpClientMock = TestHelper.mockHttpClient("segment-change-special-chatacters.json", HttpStatus.SC_OK); + SplitHttpClient splitHtpClient = SplitHttpClientImpl.create(httpClientMock, new RequestDecorator(null)); Metrics.NoopMetrics metrics = new Metrics.NoopMetrics(); - HttpSegmentChangeFetcher fetcher = HttpSegmentChangeFetcher.create(httpClientMock, rootTarget, TELEMETRY_STORAGE); + HttpSegmentChangeFetcher fetcher = HttpSegmentChangeFetcher.create(splitHtpClient, rootTarget, TELEMETRY_STORAGE); SegmentChange change = fetcher.fetch("some_segment", 1234567, new FetchOptions.Builder().build()); @@ -94,10 +101,12 @@ public void testFetcherWithCDNBypassOption() throws IOException, URISyntaxExcept ArgumentCaptor requestCaptor = ArgumentCaptor.forClass(ClassicHttpRequest.class); CloseableHttpClient httpClientMock = Mockito.mock(CloseableHttpClient.class); + SplitHttpClient splitHtpClient = SplitHttpClientImpl.create(httpClientMock, new RequestDecorator(null)); + when(httpClientMock.execute(requestCaptor.capture())).thenReturn(TestHelper.classicResponseToCloseableMock(response)); Metrics.NoopMetrics metrics = new Metrics.NoopMetrics(); - HttpSegmentChangeFetcher fetcher = HttpSegmentChangeFetcher.create(httpClientMock, rootTarget, Mockito.mock(TelemetryStorage.class)); + HttpSegmentChangeFetcher fetcher = HttpSegmentChangeFetcher.create(splitHtpClient, rootTarget, Mockito.mock(TelemetryStorage.class)); fetcher.fetch("someSegment", -1, new FetchOptions.Builder().targetChangeNumber(123).build()); fetcher.fetch("someSegment2",-1, new FetchOptions.Builder().build()); diff --git a/client/src/test/java/io/split/client/HttpSplitChangeFetcherTest.java b/client/src/test/java/io/split/client/HttpSplitChangeFetcherTest.java index 794a7f057..4140f726d 100644 --- a/client/src/test/java/io/split/client/HttpSplitChangeFetcherTest.java +++ b/client/src/test/java/io/split/client/HttpSplitChangeFetcherTest.java @@ -41,8 +41,8 @@ public class HttpSplitChangeFetcherTest { public void testDefaultURL() throws URISyntaxException { URI rootTarget = URI.create("https://api.split.io"); CloseableHttpClient httpClient = HttpClients.custom().build(); - Metrics.NoopMetrics metrics = new Metrics.NoopMetrics(); SplitHttpClient splitHtpClient = SplitHttpClientImpl.create(httpClient, new RequestDecorator(null)); + Metrics.NoopMetrics metrics = new Metrics.NoopMetrics(); HttpSplitChangeFetcher fetcher = HttpSplitChangeFetcher.create(splitHtpClient, rootTarget, TELEMETRY_STORAGE); Assert.assertEquals("https://api.split.io/api/splitChanges", fetcher.getTarget().toString()); From 7dd45c2a1e8d2e478c09ebb089190ab80758dd01 Mon Sep 17 00:00:00 2001 From: Bilal Al Date: Thu, 14 Mar 2024 11:00:05 -0700 Subject: [PATCH 4/6] polish --- .../test/java/io/split/client/HttpSplitChangeFetcherTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/src/test/java/io/split/client/HttpSplitChangeFetcherTest.java b/client/src/test/java/io/split/client/HttpSplitChangeFetcherTest.java index 4140f726d..794a7f057 100644 --- a/client/src/test/java/io/split/client/HttpSplitChangeFetcherTest.java +++ b/client/src/test/java/io/split/client/HttpSplitChangeFetcherTest.java @@ -41,8 +41,8 @@ public class HttpSplitChangeFetcherTest { public void testDefaultURL() throws URISyntaxException { URI rootTarget = URI.create("https://api.split.io"); CloseableHttpClient httpClient = HttpClients.custom().build(); - SplitHttpClient splitHtpClient = SplitHttpClientImpl.create(httpClient, new RequestDecorator(null)); Metrics.NoopMetrics metrics = new Metrics.NoopMetrics(); + SplitHttpClient splitHtpClient = SplitHttpClientImpl.create(httpClient, new RequestDecorator(null)); HttpSplitChangeFetcher fetcher = HttpSplitChangeFetcher.create(splitHtpClient, rootTarget, TELEMETRY_STORAGE); Assert.assertEquals("https://api.split.io/api/splitChanges", fetcher.getTarget().toString()); From 0675dc42bcf38f0815a82b4c054f1e1d49dfd8fb Mon Sep 17 00:00:00 2001 From: Bilal Al Date: Thu, 14 Mar 2024 12:35:14 -0700 Subject: [PATCH 5/6] polish --- .../main/java/io/split/client/HttpSegmentChangeFetcher.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/client/src/main/java/io/split/client/HttpSegmentChangeFetcher.java b/client/src/main/java/io/split/client/HttpSegmentChangeFetcher.java index 7014646d6..e55952304 100644 --- a/client/src/main/java/io/split/client/HttpSegmentChangeFetcher.java +++ b/client/src/main/java/io/split/client/HttpSegmentChangeFetcher.java @@ -30,9 +30,6 @@ public final class HttpSegmentChangeFetcher implements SegmentChangeFetcher { private static final String SINCE = "since"; private static final String TILL = "till"; - private static final String PREFIX = "segmentChangeFetcher"; - private static final String CACHE_CONTROL_HEADER_NAME = "Cache-Control"; - private static final String CACHE_CONTROL_HEADER_VALUE = "no-cache"; private final SplitHttpClient _client; private final URI _target; From d10e3dc327dc04e8a3dc1750b9e8c4afde6875af Mon Sep 17 00:00:00 2001 From: Bilal Al Date: Thu, 14 Mar 2024 13:42:55 -0700 Subject: [PATCH 6/6] polish --- client/src/main/java/io/split/service/SplitHttpClientImpl.java | 1 - 1 file changed, 1 deletion(-) diff --git a/client/src/main/java/io/split/service/SplitHttpClientImpl.java b/client/src/main/java/io/split/service/SplitHttpClientImpl.java index 92cab6659..5497a69f2 100644 --- a/client/src/main/java/io/split/service/SplitHttpClientImpl.java +++ b/client/src/main/java/io/split/service/SplitHttpClientImpl.java @@ -13,7 +13,6 @@ import org.apache.hc.core5.http.io.entity.EntityUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; - import java.io.IOException; import java.net.URI; import java.net.URISyntaxException;