Skip to content

Commit 274f1e8

Browse files
aliabbasrizvialiabbasrizvi
committed
Update HttpProjectConfigManager to use Last-Modified time
1 parent e19dabd commit 274f1e8

File tree

3 files changed

+117
-91
lines changed

3 files changed

+117
-91
lines changed

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

Lines changed: 6 additions & 1 deletion
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
@@ -104,4 +109,4 @@ public OptimizelyHttpClient build() {
104109
}
105110
}
106111

107-
}
112+
}

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

Lines changed: 61 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -18,38 +18,38 @@
1818

1919
import com.optimizely.ab.HttpClientUtils;
2020
import com.optimizely.ab.OptimizelyHttpClient;
21-
import com.optimizely.ab.OptimizelyRuntimeException;
2221
import com.optimizely.ab.annotations.VisibleForTesting;
2322
import com.optimizely.ab.config.parser.ConfigParseException;
24-
import org.apache.http.HttpEntity;
25-
import org.apache.http.HttpResponse;
23+
import org.apache.http.*;
2624
import org.apache.http.client.ClientProtocolException;
27-
import org.apache.http.client.ResponseHandler;
2825
import org.apache.http.client.methods.HttpGet;
2926
import org.apache.http.util.EntityUtils;
3027
import org.slf4j.Logger;
3128
import org.slf4j.LoggerFactory;
3229

33-
import javax.annotation.CheckForNull;
3430
import java.io.IOException;
3531
import java.net.URI;
3632
import java.util.concurrent.TimeUnit;
3733

3834
/**
39-
* HttpProjectConfigManager is an implementation of a {@link PollingProjectConfigManager}
35+
* HttpProjectConfigManager is an implementation of a ProjectConfigManager
4036
* backed by a datafile. Currently this is loosely tied to Apache HttpClient
4137
* implementation which is the client of choice in this package.
38+
*
39+
* Note that this implementation is blocking and stateless. This is best used in
40+
* conjunction with the {@link PollingProjectConfigManager} to provide caching
41+
* and asynchronous fetching.
4242
*/
4343
public class HttpProjectConfigManager extends PollingProjectConfigManager {
4444

4545
private static final Logger logger = LoggerFactory.getLogger(HttpProjectConfigManager.class);
4646

4747
private final OptimizelyHttpClient httpClient;
4848
private final URI uri;
49-
private final ResponseHandler<String> responseHandler = new ProjectConfigResponseHandler();
49+
private String datafileLastModified;
5050

51-
private HttpProjectConfigManager(long period, TimeUnit timeUnit, OptimizelyHttpClient httpClient, String url, long blockingTimeoutPeriod, TimeUnit blockingTimeoutUnit) {
52-
super(period, timeUnit, blockingTimeoutPeriod, blockingTimeoutUnit);
51+
private HttpProjectConfigManager(long period, TimeUnit timeUnit, OptimizelyHttpClient httpClient, String url) {
52+
super(period, timeUnit);
5353
this.httpClient = httpClient;
5454
this.uri = URI.create(url);
5555
}
@@ -58,16 +58,58 @@ public URI getUri() {
5858
return uri;
5959
}
6060

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

6598
@Override
6699
protected ProjectConfig poll() {
67100
HttpGet httpGet = new HttpGet(uri);
101+
102+
if (datafileLastModified != null) {
103+
httpGet.setHeader(HttpHeaders.IF_MODIFIED_SINCE, datafileLastModified);
104+
}
105+
68106
logger.info("Fetching datafile from: {}", httpGet.getURI());
69107
try {
70-
String datafile = httpClient.execute(httpGet, responseHandler);
108+
HttpResponse response = httpClient.execute(httpGet);
109+
String datafile = getDatafileFromResponse(response);
110+
if (datafile == null) {
111+
return null;
112+
}
71113
return parseProjectConfig(datafile);
72114
} catch (ConfigParseException | IOException e) {
73115
logger.error("Error fetching datafile", e);
@@ -86,13 +128,9 @@ public static class Builder {
86128
private String url;
87129
private String format = "https://cdn.optimizely.com/datafiles/%s.json";
88130
private OptimizelyHttpClient httpClient;
89-
90131
private long period = 5;
91132
private TimeUnit timeUnit = TimeUnit.MINUTES;
92133

93-
private long blockingTimeoutPeriod = 10;
94-
private TimeUnit blockingTimeoutUnit = TimeUnit.SECONDS;
95-
96134
public Builder withDatafile(String datafile) {
97135
this.datafile = datafile;
98136
return this;
@@ -118,23 +156,6 @@ public Builder withOptimizelyHttpClient(OptimizelyHttpClient httpClient) {
118156
return this;
119157
}
120158

121-
/**
122-
* Configure time to block before Completing the future. This timeout is used on the first call
123-
* to {@link PollingProjectConfigManager#getConfig()}. If the timeout is exceeded then the
124-
* PollingProjectConfigManager will begin returning null immediately until the call to Poll
125-
* succeeds.
126-
*/
127-
public Builder withBlockingTimeout(long period, TimeUnit timeUnit) {
128-
if (timeUnit == null) {
129-
throw new NullPointerException("Must provide valid timeUnit");
130-
}
131-
132-
this.blockingTimeoutPeriod = period;
133-
this.blockingTimeoutUnit = timeUnit;
134-
135-
return this;
136-
}
137-
138159
public Builder withPollingInterval(long period, TimeUnit timeUnit) {
139160
if (timeUnit == null) {
140161
throw new NullPointerException("Must provide valid timeUnit");
@@ -165,15 +186,17 @@ public HttpProjectConfigManager build(boolean defer) {
165186
httpClient = HttpClientUtils.getDefaultHttpClient();
166187
}
167188

168-
if (url == null) {
169-
if (sdkKey == null) {
170-
throw new NullPointerException("sdkKey cannot be null");
171-
}
189+
if (url != null) {
190+
return new HttpProjectConfigManager(period, timeUnit, httpClient, url);
191+
}
172192

173-
url = String.format(format, sdkKey);
193+
if (sdkKey == null) {
194+
throw new NullPointerException("sdkKey cannot be null");
174195
}
175196

176-
HttpProjectConfigManager httpProjectManager = new HttpProjectConfigManager(period, timeUnit, httpClient, url, blockingTimeoutPeriod, blockingTimeoutUnit);
197+
url = String.format(format, sdkKey);
198+
199+
HttpProjectConfigManager httpProjectManager = new HttpProjectConfigManager(period, timeUnit, httpClient, url);
177200

178201
if (datafile != null) {
179202
try {
@@ -194,24 +217,4 @@ public HttpProjectConfigManager build(boolean defer) {
194217
return httpProjectManager;
195218
}
196219
}
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-
}
217-
}
220+
}

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

Lines changed: 50 additions & 32 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;
@@ -35,13 +37,11 @@
3537
import org.mockito.runners.MockitoJUnitRunner;
3638

3739
import java.net.URI;
38-
import java.util.concurrent.TimeUnit;
3940

4041
import static com.optimizely.ab.config.HttpProjectConfigManager.*;
4142
import static org.junit.Assert.*;
4243
import static org.mockito.Matchers.any;
4344
import static org.mockito.Mockito.mock;
44-
import static org.mockito.Mockito.reset;
4545
import static org.mockito.Mockito.when;
4646

4747
@RunWith(MockitoJUnitRunner.class)
@@ -56,8 +56,20 @@ public class HttpProjectConfigManagerTest {
5656
@Before
5757
public void setUp() throws Exception {
5858
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);
59+
CloseableHttpResponse httpResponse = mock(CloseableHttpResponse.class);
60+
StatusLine statusLine = mock(StatusLine.class);
61+
62+
when(statusLine.getStatusCode()).thenReturn(200);
63+
when(httpResponse.getStatusLine()).thenReturn(statusLine);
64+
when(httpResponse.getEntity()).thenReturn(new StringEntity(datafileString));
65+
66+
when(mockHttpClient.execute(any(HttpGet.class)))
67+
.thenReturn(httpResponse);
68+
69+
projectConfigManager = builder()
70+
.withOptimizelyHttpClient(mockHttpClient)
71+
.withSdkKey("sdk-key")
72+
.build();
6173
}
6274

6375
@After
@@ -128,72 +140,78 @@ public void testBuildDefer() throws Exception {
128140

129141
@Test
130142
@Ignore
131-
public void testProjectConfigResponseHandler2XX() throws Exception {
132-
ResponseHandler<String> handler = new ProjectConfigResponseHandler();
133-
143+
public void testGetDatafileHttpResponse2XX() throws Exception {
144+
String modifiedStamp = "Wed, 24 Apr 2019 07:07:07 GMT";
134145
HttpResponse getResponse = new BasicHttpResponse(new ProtocolVersion("TEST", 0, 0), 200, "TEST");
135146
getResponse.setEntity(new StringEntity(datafileString));
147+
getResponse.setHeader(HttpHeaders.LAST_MODIFIED, modifiedStamp);
136148

137-
String datafile = handler.handleResponse(getResponse);
149+
String datafile = projectConfigManager.getDatafileFromResponse(getResponse);
138150
assertNotNull(datafile);
139151

140152
assertEquals("4", parseProjectConfig(datafile).getVersion());
153+
// Confirm last modified time is set
154+
assertEquals(modifiedStamp, projectConfigManager.getLastModified());
141155
}
142156

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

150-
handler.handleResponse(getResponse);
162+
projectConfigManager.getDatafileFromResponse(getResponse);
151163
}
152164

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

174+
@Test(expected = ClientProtocolException.class)
175+
public void testGetDatafileHttpResponse4XX() throws Exception {
157176
HttpResponse getResponse = new BasicHttpResponse(new ProtocolVersion("TEST", 0, 0), 400, "TEST");
158177
getResponse.setEntity(new StringEntity(datafileString));
159178

160-
handler.handleResponse(getResponse);
179+
projectConfigManager.getDatafileFromResponse(getResponse);
161180
}
162181

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

170-
handler.handleResponse(getResponse);
187+
projectConfigManager.getDatafileFromResponse(getResponse);
171188
}
172189

173190
@Test
174-
public void testInvalidPayload() throws Exception {
175-
reset(mockHttpClient);
176-
when(mockHttpClient.execute(any(HttpGet.class), any(ResponseHandler.class)))
177-
.thenReturn("I am an invalid response!");
178-
191+
@Ignore
192+
public void testBasicFetch() throws Exception {
179193
projectConfigManager = builder()
180-
.withOptimizelyHttpClient(mockHttpClient)
181-
.withSdkKey("sdk-key")
182-
.withBlockingTimeout(10, TimeUnit.MILLISECONDS)
194+
.withSdkKey("7vPf3v7zye3fY4PcbejeCz")
183195
.build();
184196

185-
assertNull(projectConfigManager.getConfig());
197+
ProjectConfig actual = projectConfigManager.getConfig();
198+
assertNotNull(actual);
199+
assertNotNull(actual.getVersion());
186200
}
187201

188202
@Test
189203
@Ignore
190-
public void testBasicFetch() throws Exception {
204+
public void testBasicFetchTwice() throws Exception {
191205
projectConfigManager = builder()
192206
.withSdkKey("7vPf3v7zye3fY4PcbejeCz")
193207
.build();
194208

195209
ProjectConfig actual = projectConfigManager.getConfig();
196210
assertNotNull(actual);
197211
assertNotNull(actual.getVersion());
212+
213+
// Assert ProjectConfig when refetched as datafile has not changed
214+
ProjectConfig latestConfig = projectConfigManager.getConfig();
215+
assertEquals(actual, latestConfig);
198216
}
199-
}
217+
}

0 commit comments

Comments
 (0)