Skip to content

Update HttpProjectConfigManager to use Last-Modified time #292

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
aliabbasrizvi merged 4 commits into from
May 4, 2019
Merged

Update HttpProjectConfigManager to use Last-Modified time #292

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
61ed773
Update HttpProjectConfigManager to use Last-Modified time
aliabbasrizvi May 3, 2019
09e1a1c
Adjusting against current version.
aliabbasrizvi May 3, 2019
8333f74
Fixing end of file
aliabbasrizvi May 3, 2019
01ab857
Addressing feedback
aliabbasrizvi May 4, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions core-httpclient-impl/src/main/java/com/optimizely/ab/OptimizelyHttpClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -59,6 +60,10 @@ public <T> T execute(final HttpUriRequest request, final ResponseHandler<? exten
return httpClient.execute(request, responseHandler);
}

public CloseableHttpResponse execute(final HttpUriRequest request) throws IOException {
return httpClient.execute(request);
}

public static class Builder {
// The following static values are public so that they can be tweaked if necessary.
// These are the recommended settings for http protocol. https://hc.apache.org/httpcomponents-client-ga/tutorial/html/connmgmt.html
Expand Down
72 changes: 44 additions & 28 deletions core-httpclient-impl/src/main/java/com/optimizely/ab/config/HttpProjectConfigManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,19 +18,14 @@

import com.optimizely.ab.HttpClientUtils;
import com.optimizely.ab.OptimizelyHttpClient;
import com.optimizely.ab.OptimizelyRuntimeException;
import com.optimizely.ab.annotations.VisibleForTesting;
import com.optimizely.ab.config.parser.ConfigParseException;
import org.apache.http.HttpEntity;
import org.apache.http.HttpResponse;
import org.apache.http.*;
import org.apache.http.client.ClientProtocolException;
import org.apache.http.client.ResponseHandler;
import org.apache.http.client.methods.HttpGet;
import org.apache.http.util.EntityUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import javax.annotation.CheckForNull;
import java.io.IOException;
import java.net.URI;
import java.util.concurrent.TimeUnit;
Expand All @@ -46,7 +41,7 @@ public class HttpProjectConfigManager extends PollingProjectConfigManager {

private final OptimizelyHttpClient httpClient;
private final URI uri;
private final ResponseHandler<String> 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);
Expand All @@ -58,16 +53,57 @@ 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();
}

@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);
Expand Down Expand Up @@ -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<String> {

@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);
}
}
}
}
85 changes: 63 additions & 22 deletions ...-httpclient-impl/src/test/java/com/optimizely/ab/config/HttpProjectConfigManagerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -128,53 +142,64 @@ public void testBuildDefer() throws Exception {

@Test
@Ignore
public void testProjectConfigResponseHandler2XX() throws Exception {
ResponseHandler<String> 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<String> 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<String> 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<String> 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)
Expand All @@ -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);
}
}