Skip to content

Commit f070505

Browse files
author
Josh Deffibaugh
committed
Sets up exponential backoff for event dispatch and datafile download
1 parent c4f2638 commit f070505

File tree

9 files changed

+278
-66
lines changed

9 files changed

+278
-66
lines changed

‎android-sdk/src/androidTest/java/com/optimizely/ab/android/sdk/DataFileClientTest.java‎

Lines changed: 72 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -22,16 +22,20 @@
2222
import org.junit.Test;
2323
import org.junit.runner.RunWith;
2424
import org.junit.runners.JUnit4;
25+
import org.mockito.ArgumentCaptor;
2526
import org.slf4j.Logger;
2627

2728
import java.io.IOException;
2829
import java.net.HttpURLConnection;
30+
import java.net.MalformedURLException;
2931
import java.net.URL;
3032

33+
import static junit.framework.Assert.assertTrue;
3134
import static org.junit.Assert.assertEquals;
3235
import static org.junit.Assert.assertNull;
3336
import static org.mockito.Matchers.any;
3437
import static org.mockito.Matchers.contains;
38+
import static org.mockito.Matchers.eq;
3539
import static org.mockito.Mockito.doThrow;
3640
import static org.mockito.Mockito.mock;
3741
import static org.mockito.Mockito.verify;
@@ -63,12 +67,22 @@ public void request200() throws IOException {
6367
when(urlConnection.getResponseCode()).thenReturn(200);
6468
when(client.readStream(urlConnection)).thenReturn("{}");
6569

66-
String response = dataFileClient.request(url.toString());
67-
assertEquals(response, "{}");
70+
dataFileClient.request(url.toString());
71+
72+
ArgumentCaptor<Client.Request> captor1 = ArgumentCaptor.forClass(Client.Request.class);
73+
ArgumentCaptor<Integer> captor2 = ArgumentCaptor.forClass(Integer.class);
74+
ArgumentCaptor<Integer> captor3 = ArgumentCaptor.forClass(Integer.class);
75+
verify(client).execute(captor1.capture(), captor2.capture(), captor3.capture());
76+
assertEquals(Integer.valueOf(2), captor2.getValue());
77+
assertEquals(Integer.valueOf(3), captor3.getValue());
78+
Object response = captor1.getValue().execute();
79+
assertTrue(String.class.isInstance(response));
80+
assertEquals("{}", response);
6881

6982
verify(logger).info("Requesting data file from {}", url);
7083
verify(client).saveLastModified(urlConnection);
7184
verify(client).readStream(urlConnection);
85+
verify(urlConnection).disconnect();
7286
}
7387

7488
@Test
@@ -78,12 +92,22 @@ public void request201() throws IOException {
7892
when(urlConnection.getResponseCode()).thenReturn(201);
7993
when(client.readStream(urlConnection)).thenReturn("{}");
8094

81-
String response = dataFileClient.request(url.toString());
82-
assertEquals(response, "{}");
95+
dataFileClient.request(url.toString());
96+
97+
ArgumentCaptor<Client.Request> captor1 = ArgumentCaptor.forClass(Client.Request.class);
98+
ArgumentCaptor<Integer> captor2 = ArgumentCaptor.forClass(Integer.class);
99+
ArgumentCaptor<Integer> captor3 = ArgumentCaptor.forClass(Integer.class);
100+
verify(client).execute(captor1.capture(), captor2.capture(), captor3.capture());
101+
assertEquals(Integer.valueOf(2), captor2.getValue());
102+
assertEquals(Integer.valueOf(3), captor3.getValue());
103+
Object response = captor1.getValue().execute();
104+
assertTrue(String.class.isInstance(response));
105+
assertEquals("{}", response);
83106

84107
verify(logger).info("Requesting data file from {}", url);
85108
verify(client).saveLastModified(urlConnection);
86109
verify(client).readStream(urlConnection);
110+
verify(urlConnection).disconnect();
87111
}
88112

89113
@Test
@@ -93,12 +117,22 @@ public void request299() throws IOException {
93117
when(urlConnection.getResponseCode()).thenReturn(299);
94118
when(client.readStream(urlConnection)).thenReturn("{}");
95119

96-
String response = dataFileClient.request(url.toString());
97-
assertEquals(response, "{}");
120+
dataFileClient.request(url.toString());
121+
122+
ArgumentCaptor<Client.Request> captor1 = ArgumentCaptor.forClass(Client.Request.class);
123+
ArgumentCaptor<Integer> captor2 = ArgumentCaptor.forClass(Integer.class);
124+
ArgumentCaptor<Integer> captor3 = ArgumentCaptor.forClass(Integer.class);
125+
verify(client).execute(captor1.capture(), captor2.capture(), captor3.capture());
126+
assertEquals(Integer.valueOf(2), captor2.getValue());
127+
assertEquals(Integer.valueOf(3), captor3.getValue());
128+
Object response = captor1.getValue().execute();
129+
assertTrue(String.class.isInstance(response));
130+
assertEquals("{}", response);
98131

99132
verify(logger).info("Requesting data file from {}", url);
100133
verify(client).saveLastModified(urlConnection);
101134
verify(client).readStream(urlConnection);
135+
verify(urlConnection).disconnect();
102136
}
103137

104138
@Test
@@ -107,10 +141,18 @@ public void request300() throws IOException {
107141
when(client.openConnection(url)).thenReturn(urlConnection);
108142
when(urlConnection.getResponseCode()).thenReturn(300);
109143

110-
String response = dataFileClient.request(url.toString());
144+
dataFileClient.request(url.toString());
145+
ArgumentCaptor<Client.Request> captor1 = ArgumentCaptor.forClass(Client.Request.class);
146+
ArgumentCaptor<Integer> captor2 = ArgumentCaptor.forClass(Integer.class);
147+
ArgumentCaptor<Integer> captor3 = ArgumentCaptor.forClass(Integer.class);
148+
verify(client).execute(captor1.capture(), captor2.capture(), captor3.capture());
149+
assertEquals(Integer.valueOf(2), captor2.getValue());
150+
assertEquals(Integer.valueOf(3), captor3.getValue());
151+
Object response = captor1.getValue().execute();
111152
assertNull(response);
112153

113154
verify(logger).error("Unexpected response from data file cdn, status: {}", 300);
155+
verify(urlConnection).disconnect();
114156
}
115157

116158
@Test
@@ -120,10 +162,32 @@ public void handlesIOException() throws IOException {
120162
when(urlConnection.getResponseCode()).thenReturn(200);
121163
doThrow(new IOException()).when(client).readStream(urlConnection);
122164

123-
String response = dataFileClient.request(url.toString());
165+
dataFileClient.request(url.toString());
166+
ArgumentCaptor<Client.Request> captor1 = ArgumentCaptor.forClass(Client.Request.class);
167+
ArgumentCaptor<Integer> captor2 = ArgumentCaptor.forClass(Integer.class);
168+
ArgumentCaptor<Integer> captor3 = ArgumentCaptor.forClass(Integer.class);
169+
verify(client).execute(captor1.capture(), captor2.capture(), captor3.capture());
170+
assertEquals(Integer.valueOf(2), captor2.getValue());
171+
assertEquals(Integer.valueOf(3), captor3.getValue());
172+
Object response = captor1.getValue().execute();
124173
assertNull(response);
125174

126175
verify(logger).error(contains("Error making request"), any(IOException.class));
127176
verify(urlConnection).disconnect();
177+
verify(urlConnection).disconnect();
178+
}
179+
180+
@Test
181+
public void handlesNullResponse() throws MalformedURLException {
182+
URL url = new URL(String.format(DataFileLoader.RequestDataFileFromClientTask.FORMAT_CDN_URL, "1"));
183+
when(client.execute(any(Client.Request.class), eq(2), eq(3))).thenReturn(null);
184+
assertNull(dataFileClient.request(url.toString()));
185+
}
186+
187+
@Test
188+
public void handlesEmptyStringResponse() throws MalformedURLException {
189+
URL url = new URL(String.format(DataFileLoader.RequestDataFileFromClientTask.FORMAT_CDN_URL, "1"));
190+
when(client.execute(any(Client.Request.class), eq(2), eq(3))).thenReturn("");
191+
assertNull(dataFileClient.request(url.toString()));
128192
}
129193
}

‎android-sdk/src/main/java/com/optimizely/ab/android/sdk/DataFileClient.java‎

Lines changed: 40 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -39,38 +39,50 @@ class DataFileClient {
3939
this.logger = logger;
4040
}
4141

42-
String request(String urlString) {
43-
HttpURLConnection urlConnection = null;
44-
try {
45-
URL url = new URL(urlString);
46-
logger.info("Requesting data file from {}", url);
47-
urlConnection = client.openConnection(url);
42+
String request(final String urlString) {
43+
Client.Request<String> request = new Client.Request<String>() {
44+
@Override
45+
public String execute() {
46+
HttpURLConnection urlConnection = null;
47+
try {
48+
URL url = new URL(urlString);
49+
logger.info("Requesting data file from {}", url);
50+
urlConnection = client.openConnection(url);
4851

49-
client.setIfModifiedSince(urlConnection);
52+
client.setIfModifiedSince(urlConnection);
5053

51-
urlConnection.setConnectTimeout(5 * 1000);
52-
urlConnection.connect();
54+
urlConnection.setConnectTimeout(5 * 1000);
55+
urlConnection.connect();
5356

54-
int status = urlConnection.getResponseCode();
55-
if (status >= 200 && status < 300) {
56-
client.saveLastModified(urlConnection);
57-
return client.readStream(urlConnection);
58-
} else if (status == 304) {
59-
logger.info("Data file has not been modified on the cdn");
60-
return null;
61-
} else {
62-
logger.error("Unexpected response from data file cdn, status: {}", status);
63-
return null;
57+
int status = urlConnection.getResponseCode();
58+
if (status >= 200 && status < 300) {
59+
client.saveLastModified(urlConnection);
60+
return client.readStream(urlConnection);
61+
} else if (status == 304) {
62+
logger.info("Data file has not been modified on the cdn");
63+
return "";
64+
} else {
65+
logger.error("Unexpected response from data file cdn, status: {}", status);
66+
return null;
67+
}
68+
} catch (IOException e) {
69+
logger.error("Error making request", e);
70+
return null;
71+
} finally {
72+
if (urlConnection != null) {
73+
urlConnection.disconnect();
74+
}
75+
}
6476
}
65-
} catch (IOException e) {
66-
logger.error("Error making request", e);
67-
return null;
68-
} finally {
69-
if (urlConnection != null) {
70-
urlConnection.disconnect();
71-
}
72-
}
73-
}
77+
};
7478

79+
// If the response was a 304 Not Modified an empty string is returned
80+
// Consumers of this method expect either a valid datafile or null
81+
String response = client.execute(request, 2, 3);
82+
if (response != null && response.isEmpty()) {
83+
response = null;
84+
}
7585

86+
return response;
87+
}
7688
}

‎build.gradle‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,4 +83,4 @@ task testAllModules << {
8383

8484
testAllModules.dependsOn(':android-sdk:connectedAndroidTest', ':android-sdk:test',
8585
':event-handler:connectedAndroidTest', ':event-handler:test',
86-
':user-experiment-record:connectedAndroidTest', ':shared:connectedAndroidTest')
86+
':user-experiment-record:connectedAndroidTest', ':shared:connectedAndroidTest', ':test-app:connectedAndroidTest')

‎event-handler/src/androidTest/java/com/optimizely/ab/android/event_handler/EventDispatcherTest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@
4242

4343
import static org.mockito.Matchers.any;
4444
import static org.mockito.Matchers.contains;
45+
import static org.mockito.Matchers.eq;
4546
import static org.mockito.Mockito.mock;
4647
import static org.mockito.Mockito.verify;
4748
import static org.mockito.Mockito.when;
@@ -103,7 +104,6 @@ public void handleIntentSchedulesWhenEventsLeftInStorage() throws IOException {
103104
verify(serviceScheduler).schedule(mockIntent, AlarmManager.INTERVAL_HOUR);
104105
verify(optlyStorage).saveLong(EventIntentService.EXTRA_INTERVAL, AlarmManager.INTERVAL_HOUR);
105106

106-
verify(logger).error("Unexpected response from event endpoint, status: " + 400);
107107
verify(logger).info("Scheduled events to be dispatched");
108108
}
109109

‎event-handler/src/main/java/com/optimizely/ab/android/event_handler/EventClient.java‎

Lines changed: 33 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -41,29 +41,40 @@ class EventClient {
4141
this.logger = logger;
4242
}
4343

44-
boolean sendEvent(Event event) {
45-
try {
46-
logger.info("Dispatching event: {}", event);
47-
HttpURLConnection urlConnection = client.openConnection(event.getURL());
48-
urlConnection.setRequestMethod("POST");
49-
urlConnection.setRequestProperty("Content-Type", "application/json");
50-
urlConnection.setDoOutput(true);
51-
OutputStream outputStream = urlConnection.getOutputStream();
52-
outputStream.write(event.getRequestBody().getBytes());
53-
outputStream.flush();
54-
outputStream.close();
55-
int status = urlConnection.getResponseCode();
56-
if (status >= 200 && status < 300) {
57-
InputStream in = new BufferedInputStream(urlConnection.getInputStream());
58-
in.close();
59-
return true;
60-
} else {
61-
logger.error("Unexpected response from event endpoint, status: " + status);
62-
return false;
44+
boolean sendEvent(final Event event) {
45+
Client.Request<Boolean> request = new Client.Request<Boolean>() {
46+
@Override
47+
public Boolean execute() {
48+
try {
49+
logger.info("Dispatching event: {}", event);
50+
HttpURLConnection urlConnection = client.openConnection(event.getURL());
51+
urlConnection.setRequestMethod("POST");
52+
urlConnection.setRequestProperty("Content-Type", "application/json");
53+
urlConnection.setDoOutput(true);
54+
OutputStream outputStream = urlConnection.getOutputStream();
55+
outputStream.write(event.getRequestBody().getBytes());
56+
outputStream.flush();
57+
outputStream.close();
58+
int status = urlConnection.getResponseCode();
59+
if (status >= 200 && status < 300) {
60+
InputStream in = new BufferedInputStream(urlConnection.getInputStream());
61+
in.close();
62+
return Boolean.TRUE;
63+
} else {
64+
logger.error("Unexpected response from event endpoint, status: " + status);
65+
return Boolean.FALSE;
66+
}
67+
} catch (IOException e) {
68+
logger.error("Unable to send event: {}", event, e);
69+
return Boolean.FALSE;
70+
}
6371
}
64-
} catch (IOException e) {
65-
logger.error("Unable to send event: {}", event, e);
66-
return false;
72+
};
73+
74+
Boolean success = client.execute(request, 2, 5);
75+
if (success == null) {
76+
success = false;
6777
}
78+
return success;
6879
}
6980
}

0 commit comments

Comments
 (0)