diff --git a/core-httpclient-impl/src/main/java/com/optimizely/ab/OptimizelyHttpClient.java b/core-httpclient-impl/src/main/java/com/optimizely/ab/OptimizelyHttpClient.java index 4c3839519..d5e0818df 100644 --- a/core-httpclient-impl/src/main/java/com/optimizely/ab/OptimizelyHttpClient.java +++ b/core-httpclient-impl/src/main/java/com/optimizely/ab/OptimizelyHttpClient.java @@ -19,6 +19,7 @@ import com.optimizely.ab.annotations.VisibleForTesting; import org.apache.http.client.HttpClient; import org.apache.http.client.ResponseHandler; +import org.apache.http.client.methods.CloseableHttpResponse; import org.apache.http.client.methods.HttpUriRequest; import org.apache.http.impl.client.CloseableHttpClient; import org.apache.http.impl.client.HttpClients; @@ -59,6 +60,10 @@ public T execute(final HttpUriRequest request, final ResponseHandler responseHandler = new ProjectConfigResponseHandler(); + private String datafileLastModified; private HttpProjectConfigManager(long period, TimeUnit timeUnit, OptimizelyHttpClient httpClient, String url, long blockingTimeoutPeriod, TimeUnit blockingTimeoutUnit) { super(period, timeUnit, blockingTimeoutPeriod, blockingTimeoutUnit); @@ -58,6 +53,38 @@ public URI getUri() { return uri; } + public String getLastModified() { + return datafileLastModified; + } + + public String getDatafileFromResponse(HttpResponse response) throws NullPointerException, IOException { + StatusLine statusLine = response.getStatusLine(); + + if (statusLine == null) { + throw new ClientProtocolException("unexpected response from event endpoint, status is null"); + } + + int status = statusLine.getStatusCode(); + + // Datafile has not updated + if (status == HttpStatus.SC_NOT_MODIFIED) { + logger.debug("Not updating ProjectConfig as datafile has not updated since " + datafileLastModified); + return null; + } + + if (status >= 200 && status < 300) { + // read the response, so we can close the connection + HttpEntity entity = response.getEntity(); + Header lastModifiedHeader = response.getFirstHeader(HttpHeaders.LAST_MODIFIED); + if (lastModifiedHeader != null) { + datafileLastModified = lastModifiedHeader.getValue(); + } + return EntityUtils.toString(entity, "UTF-8"); + } else { + throw new ClientProtocolException("unexpected response from event endpoint, status: " + status); + } + } + static ProjectConfig parseProjectConfig(String datafile) throws ConfigParseException { return new DatafileProjectConfig.Builder().withDatafile(datafile).build(); } @@ -65,9 +92,18 @@ static ProjectConfig parseProjectConfig(String datafile) throws ConfigParseExcep @Override protected ProjectConfig poll() { HttpGet httpGet = new HttpGet(uri); + + if (datafileLastModified != null) { + httpGet.setHeader(HttpHeaders.IF_MODIFIED_SINCE, datafileLastModified); + } + logger.info("Fetching datafile from: {}", httpGet.getURI()); try { - String datafile = httpClient.execute(httpGet, responseHandler); + HttpResponse response = httpClient.execute(httpGet); + String datafile = getDatafileFromResponse(response); + if (datafile == null) { + return null; + } return parseProjectConfig(datafile); } catch (ConfigParseException | IOException e) { logger.error("Error fetching datafile", e); @@ -194,24 +230,4 @@ public HttpProjectConfigManager build(boolean defer) { return httpProjectManager; } } - - /** - * Handler for the event request that returns nothing (i.e., Void) - */ - static final class ProjectConfigResponseHandler implements ResponseHandler { - - @Override - @CheckForNull - public String handleResponse(HttpResponse response) throws IOException { - int status = response.getStatusLine().getStatusCode(); - if (status >= 200 && status < 300) { - // read the response, so we can close the connection - HttpEntity entity = response.getEntity(); - return EntityUtils.toString(entity, "UTF-8"); - } else { - // TODO handle unmodifed response. - throw new ClientProtocolException("unexpected response from event endpoint, status: " + status); - } - } - } } diff --git a/core-httpclient-impl/src/test/java/com/optimizely/ab/config/HttpProjectConfigManagerTest.java b/core-httpclient-impl/src/test/java/com/optimizely/ab/config/HttpProjectConfigManagerTest.java index d9af84914..b105a97c5 100644 --- a/core-httpclient-impl/src/test/java/com/optimizely/ab/config/HttpProjectConfigManagerTest.java +++ b/core-httpclient-impl/src/test/java/com/optimizely/ab/config/HttpProjectConfigManagerTest.java @@ -19,10 +19,12 @@ import com.google.common.base.Charsets; import com.google.common.io.Resources; import com.optimizely.ab.OptimizelyHttpClient; +import org.apache.http.HttpHeaders; import org.apache.http.HttpResponse; import org.apache.http.ProtocolVersion; +import org.apache.http.StatusLine; import org.apache.http.client.ClientProtocolException; -import org.apache.http.client.ResponseHandler; +import org.apache.http.client.methods.CloseableHttpResponse; import org.apache.http.client.methods.HttpGet; import org.apache.http.entity.StringEntity; import org.apache.http.message.BasicHttpResponse; @@ -56,8 +58,20 @@ public class HttpProjectConfigManagerTest { @Before public void setUp() throws Exception { datafileString = Resources.toString(Resources.getResource("valid-project-config-v4.json"), Charsets.UTF_8); - when(mockHttpClient.execute(any(HttpGet.class), any(ResponseHandler.class))) - .thenReturn(datafileString); + CloseableHttpResponse httpResponse = mock(CloseableHttpResponse.class); + StatusLine statusLine = mock(StatusLine.class); + + when(statusLine.getStatusCode()).thenReturn(200); + when(httpResponse.getStatusLine()).thenReturn(statusLine); + when(httpResponse.getEntity()).thenReturn(new StringEntity(datafileString)); + + when(mockHttpClient.execute(any(HttpGet.class))) + .thenReturn(httpResponse); + + projectConfigManager = builder() + .withOptimizelyHttpClient(mockHttpClient) + .withSdkKey("sdk-key") + .build(); } @After @@ -128,53 +142,64 @@ public void testBuildDefer() throws Exception { @Test @Ignore - public void testProjectConfigResponseHandler2XX() throws Exception { - ResponseHandler handler = new ProjectConfigResponseHandler(); - + public void testGetDatafileHttpResponse2XX() throws Exception { + String modifiedStamp = "Wed, 24 Apr 2019 07:07:07 GMT"; HttpResponse getResponse = new BasicHttpResponse(new ProtocolVersion("TEST", 0, 0), 200, "TEST"); getResponse.setEntity(new StringEntity(datafileString)); + getResponse.setHeader(HttpHeaders.LAST_MODIFIED, modifiedStamp); - String datafile = handler.handleResponse(getResponse); + String datafile = projectConfigManager.getDatafileFromResponse(getResponse); assertNotNull(datafile); assertEquals("4", parseProjectConfig(datafile).getVersion()); + // Confirm last modified time is set + assertEquals(modifiedStamp, projectConfigManager.getLastModified()); } @Test(expected = ClientProtocolException.class) - public void testProjectConfigResponseHandler3XX() throws Exception { - ResponseHandler handler = new ProjectConfigResponseHandler(); - + public void testGetDatafileHttpResponse3XX() throws Exception { HttpResponse getResponse = new BasicHttpResponse(new ProtocolVersion("TEST", 0, 0), 300, "TEST"); getResponse.setEntity(new StringEntity(datafileString)); - handler.handleResponse(getResponse); + projectConfigManager.getDatafileFromResponse(getResponse); } - @Test(expected = ClientProtocolException.class) - public void testProjectConfigResponseHandler4XX() throws Exception { - ResponseHandler handler = new ProjectConfigResponseHandler(); + @Test + public void testGetDatafileHttpResponse304() throws Exception { + HttpResponse getResponse = new BasicHttpResponse(new ProtocolVersion("TEST", 0, 0), 304, "TEST"); + getResponse.setEntity(new StringEntity(datafileString)); + String datafile = projectConfigManager.getDatafileFromResponse(getResponse); + assertNull(datafile); + } + + @Test(expected = ClientProtocolException.class) + public void testGetDatafileHttpResponse4XX() throws Exception { HttpResponse getResponse = new BasicHttpResponse(new ProtocolVersion("TEST", 0, 0), 400, "TEST"); getResponse.setEntity(new StringEntity(datafileString)); - handler.handleResponse(getResponse); + projectConfigManager.getDatafileFromResponse(getResponse); } @Test(expected = ClientProtocolException.class) - public void testProjectConfigResponseHandler5XX() throws Exception { - ResponseHandler handler = new ProjectConfigResponseHandler(); - + public void testGetDatafileHttpResponse5XX() throws Exception { HttpResponse getResponse = new BasicHttpResponse(new ProtocolVersion("TEST", 0, 0), 500, "TEST"); getResponse.setEntity(new StringEntity(datafileString)); - handler.handleResponse(getResponse); + projectConfigManager.getDatafileFromResponse(getResponse); } - @Test public void testInvalidPayload() throws Exception { reset(mockHttpClient); - when(mockHttpClient.execute(any(HttpGet.class), any(ResponseHandler.class))) - .thenReturn("I am an invalid response!"); + CloseableHttpResponse invalidPayloadResponse = mock(CloseableHttpResponse.class); + StatusLine statusLine = mock(StatusLine.class); + + when(statusLine.getStatusCode()).thenReturn(200); + when(invalidPayloadResponse.getStatusLine()).thenReturn(statusLine); + when(invalidPayloadResponse.getEntity()).thenReturn(new StringEntity("I am an invalid response!")); + + when(mockHttpClient.execute(any(HttpGet.class))) + .thenReturn(invalidPayloadResponse); projectConfigManager = builder() .withOptimizelyHttpClient(mockHttpClient) @@ -196,4 +221,20 @@ public void testBasicFetch() throws Exception { assertNotNull(actual); assertNotNull(actual.getVersion()); } + + @Test + @Ignore + public void testBasicFetchTwice() throws Exception { + projectConfigManager = builder() + .withSdkKey("7vPf3v7zye3fY4PcbejeCz") + .build(); + + ProjectConfig actual = projectConfigManager.getConfig(); + assertNotNull(actual); + assertNotNull(actual.getVersion()); + + // Assert ProjectConfig when refetched as datafile has not changed + ProjectConfig latestConfig = projectConfigManager.getConfig(); + assertEquals(actual, latestConfig); + } }