Skip to content

Commit 543c1c2

Browse files
aliabbasrizvialiabbasrizvi
authored
Update HttpProjectConfigManager to use Last-Modified time (#292)
1 parent 73a9d88 commit 543c1c2

File tree

3 files changed

+112
-50
lines changed

3 files changed

+112
-50
lines changed

‎core-httpclient-impl/src/main/java/com/optimizely/ab/OptimizelyHttpClient.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import com.optimizely.ab.annotations.VisibleForTesting;
2020
import org.apache.http.client.HttpClient;
2121
import org.apache.http.client.ResponseHandler;
22+
import org.apache.http.client.methods.CloseableHttpResponse;
2223
import org.apache.http.client.methods.HttpUriRequest;
2324
import org.apache.http.impl.client.CloseableHttpClient;
2425
import org.apache.http.impl.client.HttpClients;
@@ -59,6 +60,10 @@ public <T> T execute(final HttpUriRequest request, final ResponseHandler<? exten
5960
return httpClient.execute(request, responseHandler);
6061
}
6162

63+
public CloseableHttpResponse execute(final HttpUriRequest request) throws IOException {
64+
return httpClient.execute(request);
65+
}
66+
6267
public static class Builder {
6368
// The following static values are public so that they can be tweaked if necessary.
6469
// These are the recommended settings for http protocol. https://hc.apache.org/httpcomponents-client-ga/tutorial/html/connmgmt.html

‎core-httpclient-impl/src/main/java/com/optimizely/ab/config/HttpProjectConfigManager.java

Lines changed: 44 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -18,19 +18,14 @@
1818

1919
import com.optimizely.ab.HttpClientUtils;
2020
import com.optimizely.ab.OptimizelyHttpClient;
21-
import com.optimizely.ab.OptimizelyRuntimeException;
22-
import com.optimizely.ab.annotations.VisibleForTesting;
2321
import com.optimizely.ab.config.parser.ConfigParseException;
24-
import org.apache.http.HttpEntity;
25-
import org.apache.http.HttpResponse;
22+
import org.apache.http.*;
2623
import org.apache.http.client.ClientProtocolException;
27-
import org.apache.http.client.ResponseHandler;
2824
import org.apache.http.client.methods.HttpGet;
2925
import org.apache.http.util.EntityUtils;
3026
import org.slf4j.Logger;
3127
import org.slf4j.LoggerFactory;
3228

33-
import javax.annotation.CheckForNull;
3429
import java.io.IOException;
3530
import java.net.URI;
3631
import java.util.concurrent.TimeUnit;
@@ -46,7 +41,7 @@ public class HttpProjectConfigManager extends PollingProjectConfigManager {
4641

4742
private final OptimizelyHttpClient httpClient;
4843
private final URI uri;
49-
private final ResponseHandler<String> responseHandler = new ProjectConfigResponseHandler();
44+
private String datafileLastModified;
5045

5146
private HttpProjectConfigManager(long period, TimeUnit timeUnit, OptimizelyHttpClient httpClient, String url, long blockingTimeoutPeriod, TimeUnit blockingTimeoutUnit) {
5247
super(period, timeUnit, blockingTimeoutPeriod, blockingTimeoutUnit);
@@ -58,16 +53,57 @@ public URI getUri() {
5853
return uri;
5954
}
6055

56+
public String getLastModified() {
57+
return datafileLastModified;
58+
}
59+
60+
public String getDatafileFromResponse(HttpResponse response) throws NullPointerException, IOException {
61+
StatusLine statusLine = response.getStatusLine();
62+
63+
if (statusLine == null) {
64+
throw new ClientProtocolException("unexpected response from event endpoint, status is null");
65+
}
66+
67+
int status = statusLine.getStatusCode();
68+
69+
// Datafile has not updated
70+
if (status == HttpStatus.SC_NOT_MODIFIED) {
71+
logger.debug("Not updating ProjectConfig as datafile has not updated since " + datafileLastModified);
72+
return null;
73+
}
74+
75+
if (status >= 200 && status < 300) {
76+
// read the response, so we can close the connection
77+
HttpEntity entity = response.getEntity();
78+
Header lastModifiedHeader = response.getFirstHeader(HttpHeaders.LAST_MODIFIED);
79+
if (lastModifiedHeader != null) {
80+
datafileLastModified = lastModifiedHeader.getValue();
81+
}
82+
return EntityUtils.toString(entity, "UTF-8");
83+
} else {
84+
throw new ClientProtocolException("unexpected response from event endpoint, status: " + status);
85+
}
86+
}
87+
6188
static ProjectConfig parseProjectConfig(String datafile) throws ConfigParseException {
6289
return new DatafileProjectConfig.Builder().withDatafile(datafile).build();
6390
}
6491

6592
@Override
6693
protected ProjectConfig poll() {
6794
HttpGet httpGet = new HttpGet(uri);
95+
96+
if (datafileLastModified != null) {
97+
httpGet.setHeader(HttpHeaders.IF_MODIFIED_SINCE, datafileLastModified);
98+
}
99+
68100
logger.info("Fetching datafile from: {}", httpGet.getURI());
69101
try {
70-
String datafile = httpClient.execute(httpGet, responseHandler);
102+
HttpResponse response = httpClient.execute(httpGet);
103+
String datafile = getDatafileFromResponse(response);
104+
if (datafile == null) {
105+
return null;
106+
}
71107
return parseProjectConfig(datafile);
72108
} catch (ConfigParseException | IOException e) {
73109
logger.error("Error fetching datafile", e);
@@ -194,24 +230,4 @@ public HttpProjectConfigManager build(boolean defer) {
194230
return httpProjectManager;
195231
}
196232
}
197-
198-
/**
199-
* Handler for the event request that returns nothing (i.e., Void)
200-
*/
201-
static final class ProjectConfigResponseHandler implements ResponseHandler<String> {
202-
203-
@Override
204-
@CheckForNull
205-
public String handleResponse(HttpResponse response) throws IOException {
206-
int status = response.getStatusLine().getStatusCode();
207-
if (status >= 200 && status < 300) {
208-
// read the response, so we can close the connection
209-
HttpEntity entity = response.getEntity();
210-
return EntityUtils.toString(entity, "UTF-8");
211-
} else {
212-
// TODO handle unmodifed response.
213-
throw new ClientProtocolException("unexpected response from event endpoint, status: " + status);
214-
}
215-
}
216-
}
217233
}

‎core-httpclient-impl/src/test/java/com/optimizely/ab/config/HttpProjectConfigManagerTest.java

Lines changed: 63 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,12 @@
1919
import com.google.common.base.Charsets;
2020
import com.google.common.io.Resources;
2121
import com.optimizely.ab.OptimizelyHttpClient;
22+
import org.apache.http.HttpHeaders;
2223
import org.apache.http.HttpResponse;
2324
import org.apache.http.ProtocolVersion;
25+
import org.apache.http.StatusLine;
2426
import org.apache.http.client.ClientProtocolException;
25-
import org.apache.http.client.ResponseHandler;
27+
import org.apache.http.client.methods.CloseableHttpResponse;
2628
import org.apache.http.client.methods.HttpGet;
2729
import org.apache.http.entity.StringEntity;
2830
import org.apache.http.message.BasicHttpResponse;
@@ -56,8 +58,20 @@ public class HttpProjectConfigManagerTest {
5658
@Before
5759
public void setUp() throws Exception {
5860
datafileString = Resources.toString(Resources.getResource("valid-project-config-v4.json"), Charsets.UTF_8);
59-
when(mockHttpClient.execute(any(HttpGet.class), any(ResponseHandler.class)))
60-
.thenReturn(datafileString);
61+
CloseableHttpResponse httpResponse = mock(CloseableHttpResponse.class);
62+
StatusLine statusLine = mock(StatusLine.class);
63+
64+
when(statusLine.getStatusCode()).thenReturn(200);
65+
when(httpResponse.getStatusLine()).thenReturn(statusLine);
66+
when(httpResponse.getEntity()).thenReturn(new StringEntity(datafileString));
67+
68+
when(mockHttpClient.execute(any(HttpGet.class)))
69+
.thenReturn(httpResponse);
70+
71+
projectConfigManager = builder()
72+
.withOptimizelyHttpClient(mockHttpClient)
73+
.withSdkKey("sdk-key")
74+
.build();
6175
}
6276

6377
@After
@@ -128,53 +142,64 @@ public void testBuildDefer() throws Exception {
128142

129143
@Test
130144
@Ignore
131-
public void testProjectConfigResponseHandler2XX() throws Exception {
132-
ResponseHandler<String> handler = new ProjectConfigResponseHandler();
133-
145+
public void testGetDatafileHttpResponse2XX() throws Exception {
146+
String modifiedStamp = "Wed, 24 Apr 2019 07:07:07 GMT";
134147
HttpResponse getResponse = new BasicHttpResponse(new ProtocolVersion("TEST", 0, 0), 200, "TEST");
135148
getResponse.setEntity(new StringEntity(datafileString));
149+
getResponse.setHeader(HttpHeaders.LAST_MODIFIED, modifiedStamp);
136150

137-
String datafile = handler.handleResponse(getResponse);
151+
String datafile = projectConfigManager.getDatafileFromResponse(getResponse);
138152
assertNotNull(datafile);
139153

140154
assertEquals("4", parseProjectConfig(datafile).getVersion());
155+
// Confirm last modified time is set
156+
assertEquals(modifiedStamp, projectConfigManager.getLastModified());
141157
}
142158

143159
@Test(expected = ClientProtocolException.class)
144-
public void testProjectConfigResponseHandler3XX() throws Exception {
145-
ResponseHandler<String> handler = new ProjectConfigResponseHandler();
146-
160+
public void testGetDatafileHttpResponse3XX() throws Exception {
147161
HttpResponse getResponse = new BasicHttpResponse(new ProtocolVersion("TEST", 0, 0), 300, "TEST");
148162
getResponse.setEntity(new StringEntity(datafileString));
149163

150-
handler.handleResponse(getResponse);
164+
projectConfigManager.getDatafileFromResponse(getResponse);
151165
}
152166

153-
@Test(expected = ClientProtocolException.class)
154-
public void testProjectConfigResponseHandler4XX() throws Exception {
155-
ResponseHandler<String> handler = new ProjectConfigResponseHandler();
167+
@Test
168+
public void testGetDatafileHttpResponse304() throws Exception {
169+
HttpResponse getResponse = new BasicHttpResponse(new ProtocolVersion("TEST", 0, 0), 304, "TEST");
170+
getResponse.setEntity(new StringEntity(datafileString));
156171

172+
String datafile = projectConfigManager.getDatafileFromResponse(getResponse);
173+
assertNull(datafile);
174+
}
175+
176+
@Test(expected = ClientProtocolException.class)
177+
public void testGetDatafileHttpResponse4XX() throws Exception {
157178
HttpResponse getResponse = new BasicHttpResponse(new ProtocolVersion("TEST", 0, 0), 400, "TEST");
158179
getResponse.setEntity(new StringEntity(datafileString));
159180

160-
handler.handleResponse(getResponse);
181+
projectConfigManager.getDatafileFromResponse(getResponse);
161182
}
162183

163184
@Test(expected = ClientProtocolException.class)
164-
public void testProjectConfigResponseHandler5XX() throws Exception {
165-
ResponseHandler<String> handler = new ProjectConfigResponseHandler();
166-
185+
public void testGetDatafileHttpResponse5XX() throws Exception {
167186
HttpResponse getResponse = new BasicHttpResponse(new ProtocolVersion("TEST", 0, 0), 500, "TEST");
168187
getResponse.setEntity(new StringEntity(datafileString));
169188

170-
handler.handleResponse(getResponse);
189+
projectConfigManager.getDatafileFromResponse(getResponse);
171190
}
172191

173-
@Test
174192
public void testInvalidPayload() throws Exception {
175193
reset(mockHttpClient);
176-
when(mockHttpClient.execute(any(HttpGet.class), any(ResponseHandler.class)))
177-
.thenReturn("I am an invalid response!");
194+
CloseableHttpResponse invalidPayloadResponse = mock(CloseableHttpResponse.class);
195+
StatusLine statusLine = mock(StatusLine.class);
196+
197+
when(statusLine.getStatusCode()).thenReturn(200);
198+
when(invalidPayloadResponse.getStatusLine()).thenReturn(statusLine);
199+
when(invalidPayloadResponse.getEntity()).thenReturn(new StringEntity("I am an invalid response!"));
200+
201+
when(mockHttpClient.execute(any(HttpGet.class)))
202+
.thenReturn(invalidPayloadResponse);
178203

179204
projectConfigManager = builder()
180205
.withOptimizelyHttpClient(mockHttpClient)
@@ -196,4 +221,20 @@ public void testBasicFetch() throws Exception {
196221
assertNotNull(actual);
197222
assertNotNull(actual.getVersion());
198223
}
224+
225+
@Test
226+
@Ignore
227+
public void testBasicFetchTwice() throws Exception {
228+
projectConfigManager = builder()
229+
.withSdkKey("7vPf3v7zye3fY4PcbejeCz")
230+
.build();
231+
232+
ProjectConfig actual = projectConfigManager.getConfig();
233+
assertNotNull(actual);
234+
assertNotNull(actual.getVersion());
235+
236+
// Assert ProjectConfig when refetched as datafile has not changed
237+
ProjectConfig latestConfig = projectConfigManager.getConfig();
238+
assertEquals(actual, latestConfig);
239+
}
199240
}

0 commit comments

Comments
 (0)