Skip to content

Commit d3a8092

Browse files
thomaszurkan-optimizelythomaszurkan-optimizely
authored
Bug/last modified (#149)
* fix for bug where last modified was not project specific * removed last modifed prefix. just use url. * added a last modified test for two project files
1 parent 4a65625 commit d3a8092

File tree

4 files changed

+174
-17
lines changed

4 files changed

+174
-17
lines changed

‎datafile-handler/src/androidTest/java/com/optimizely/ab/android/datafile_handler/DatafileClientTest.java

Lines changed: 127 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -18,17 +18,22 @@
1818

1919
import com.optimizely.ab.android.shared.Client;
2020

21+
import junit.framework.Assert;
22+
2123
import org.junit.Before;
2224
import org.junit.Test;
2325
import org.junit.runner.RunWith;
2426
import org.junit.runners.JUnit4;
2527
import org.mockito.ArgumentCaptor;
28+
import org.mockito.invocation.InvocationOnMock;
29+
import org.mockito.stubbing.Answer;
2630
import org.slf4j.Logger;
2731

2832
import java.io.IOException;
2933
import java.net.HttpURLConnection;
3034
import java.net.MalformedURLException;
3135
import java.net.URL;
36+
import java.net.URLConnection;
3237

3338
import static junit.framework.Assert.assertTrue;
3439
import static org.junit.Assert.assertEquals;
@@ -38,6 +43,7 @@
3843
import static org.mockito.Matchers.eq;
3944
import static org.mockito.Mockito.doThrow;
4045
import static org.mockito.Mockito.mock;
46+
import static org.mockito.Mockito.times;
4147
import static org.mockito.Mockito.verify;
4248
import static org.mockito.Mockito.when;
4349

@@ -73,8 +79,8 @@ public void request200() throws IOException {
7379
ArgumentCaptor<Integer> captor2 = ArgumentCaptor.forClass(Integer.class);
7480
ArgumentCaptor<Integer> captor3 = ArgumentCaptor.forClass(Integer.class);
7581
verify(client).execute(captor1.capture(), captor2.capture(), captor3.capture());
76-
assertEquals(Integer.valueOf(2), captor2.getValue());
77-
assertEquals(Integer.valueOf(3), captor3.getValue());
82+
assertEquals(Integer.valueOf(DatafileClient.REQUEST_BACKOFF_TIMEOUT), captor2.getValue());
83+
assertEquals(Integer.valueOf(DatafileClient.REQUEST_RETRIES_POWER), captor3.getValue());
7884
Object response = captor1.getValue().execute();
7985
assertTrue(String.class.isInstance(response));
8086
assertEquals("{}", response);
@@ -85,6 +91,115 @@ public void request200() throws IOException {
8591
verify(urlConnection).disconnect();
8692
}
8793

94+
/**
95+
* testLastModified - This is a test to see if given two projects, the last modified for datafile download is project specific.
96+
* Two URLs url1 and url2 are both datafile urls, url1 is requested from the data client twice, while url2 is only asked for once.
97+
* The first time the last modified is 0 and the second time, if it is non-zero, then it is the current last modified and a 304 is returned.
98+
*
99+
* @throws IOException
100+
*/
101+
@Test
102+
public void testLastModified() throws IOException {
103+
final URL url1 = new URL(DatafileService.getDatafileUrl("1"));
104+
final URL url2 = new URL(DatafileService.getDatafileUrl("2"));
105+
HttpURLConnection urlConnection2 = mock(HttpURLConnection.class);
106+
when(urlConnection.getURL()).thenReturn(url1);
107+
when(urlConnection2.getURL()).thenReturn(url2);
108+
when(urlConnection.getLastModified()).thenReturn(0L);
109+
when(urlConnection2.getLastModified()).thenReturn(0L);
110+
when(client.openConnection(url1)).thenReturn(urlConnection);
111+
Answer<Integer> answer = new Answer<Integer>() {
112+
public Integer answer(InvocationOnMock invocation) throws Throwable {
113+
HttpURLConnection connection = (HttpURLConnection) invocation.getMock();
114+
URL url = connection.getURL();
115+
if (url == url1) {
116+
if (connection.getLastModified() == 0L) {
117+
when(connection.getLastModified()).thenReturn(300L);
118+
return 200;
119+
}
120+
else {
121+
assertEquals(connection.getLastModified(), 300L);
122+
return 304;
123+
}
124+
}
125+
else if (url == url2) {
126+
if (connection.getLastModified() == 0L) {
127+
when(connection.getLastModified()).thenReturn(200L);
128+
return 200;
129+
}
130+
else {
131+
assertEquals(connection.getLastModified(), 200L);
132+
return 304;
133+
}
134+
}
135+
//Object[] arguments = invocation.getArguments();
136+
//String string = (String) arguments[0];
137+
return 0;
138+
}
139+
};
140+
141+
when(urlConnection.getResponseCode()).thenAnswer(answer);
142+
when(urlConnection2.getResponseCode()).thenAnswer(answer);
143+
144+
when(client.openConnection(url2)).thenReturn(urlConnection2);
145+
when(client.readStream(urlConnection)).thenReturn("{}");
146+
when(client.readStream(urlConnection2)).thenReturn("{}");
147+
148+
// first call returns the project file {}
149+
datafileClient.request(url1.toString());
150+
151+
ArgumentCaptor<Client.Request> captor1 = ArgumentCaptor.forClass(Client.Request.class);
152+
ArgumentCaptor<Integer> captor2 = ArgumentCaptor.forClass(Integer.class);
153+
ArgumentCaptor<Integer> captor3 = ArgumentCaptor.forClass(Integer.class);
154+
verify(client).execute(captor1.capture(), captor2.capture(), captor3.capture());
155+
assertEquals(Integer.valueOf(DatafileClient.REQUEST_BACKOFF_TIMEOUT), captor2.getValue());
156+
assertEquals(Integer.valueOf(DatafileClient.REQUEST_RETRIES_POWER), captor3.getValue());
157+
Object response = captor1.getValue().execute();
158+
assertTrue(String.class.isInstance(response));
159+
assertEquals("{}", response);
160+
161+
verify(logger).info("Requesting data file from {}", url1);
162+
verify(client).saveLastModified(urlConnection);
163+
verify(client).readStream(urlConnection);
164+
verify(urlConnection).disconnect();
165+
166+
// second call returns 304 so the response is a empty string.
167+
datafileClient.request(url1.toString());
168+
169+
captor1 = ArgumentCaptor.forClass(Client.Request.class);
170+
captor2 = ArgumentCaptor.forClass(Integer.class);
171+
captor3 = ArgumentCaptor.forClass(Integer.class);
172+
verify(client, times(2)).execute(captor1.capture(), captor2.capture(), captor3.capture());
173+
assertEquals(Integer.valueOf(DatafileClient.REQUEST_BACKOFF_TIMEOUT), captor2.getValue());
174+
assertEquals(Integer.valueOf(DatafileClient.REQUEST_RETRIES_POWER), captor3.getValue());
175+
response = captor1.getValue().execute();
176+
assertTrue(String.class.isInstance(response));
177+
assertEquals("", response);
178+
179+
verify(logger).info("Data file has not been modified on the cdn");
180+
verify(urlConnection, times(2)).disconnect();
181+
182+
datafileClient.request(url2.toString());
183+
184+
captor1 = ArgumentCaptor.forClass(Client.Request.class);
185+
captor2 = ArgumentCaptor.forClass(Integer.class);
186+
captor3 = ArgumentCaptor.forClass(Integer.class);
187+
verify(client, times(3)).execute(captor1.capture(), captor2.capture(), captor3.capture());
188+
assertEquals(Integer.valueOf(DatafileClient.REQUEST_BACKOFF_TIMEOUT), captor2.getValue());
189+
assertEquals(Integer.valueOf(DatafileClient.REQUEST_RETRIES_POWER), captor3.getValue());
190+
response = captor1.getValue().execute();
191+
assertTrue(String.class.isInstance(response));
192+
assertEquals("{}", response);
193+
194+
verify(logger, times(2)).info("Requesting data file from {}", url1);
195+
verify(client).saveLastModified(urlConnection2);
196+
verify(client).readStream(urlConnection2);
197+
verify(urlConnection2).disconnect();
198+
199+
200+
}
201+
202+
88203
@Test
89204
public void request201() throws IOException {
90205
URL url = new URL(DatafileService.getDatafileUrl("1"));
@@ -98,8 +213,8 @@ public void request201() throws IOException {
98213
ArgumentCaptor<Integer> captor2 = ArgumentCaptor.forClass(Integer.class);
99214
ArgumentCaptor<Integer> captor3 = ArgumentCaptor.forClass(Integer.class);
100215
verify(client).execute(captor1.capture(), captor2.capture(), captor3.capture());
101-
assertEquals(Integer.valueOf(2), captor2.getValue());
102-
assertEquals(Integer.valueOf(3), captor3.getValue());
216+
assertEquals(Integer.valueOf(DatafileClient.REQUEST_BACKOFF_TIMEOUT), captor2.getValue());
217+
assertEquals(Integer.valueOf(DatafileClient.REQUEST_RETRIES_POWER), captor3.getValue());
103218
Object response = captor1.getValue().execute();
104219
assertTrue(String.class.isInstance(response));
105220
assertEquals("{}", response);
@@ -123,8 +238,8 @@ public void request299() throws IOException {
123238
ArgumentCaptor<Integer> captor2 = ArgumentCaptor.forClass(Integer.class);
124239
ArgumentCaptor<Integer> captor3 = ArgumentCaptor.forClass(Integer.class);
125240
verify(client).execute(captor1.capture(), captor2.capture(), captor3.capture());
126-
assertEquals(Integer.valueOf(2), captor2.getValue());
127-
assertEquals(Integer.valueOf(3), captor3.getValue());
241+
assertEquals(Integer.valueOf(DatafileClient.REQUEST_BACKOFF_TIMEOUT), captor2.getValue());
242+
assertEquals(Integer.valueOf(DatafileClient.REQUEST_RETRIES_POWER), captor3.getValue());
128243
Object response = captor1.getValue().execute();
129244
assertTrue(String.class.isInstance(response));
130245
assertEquals("{}", response);
@@ -146,8 +261,8 @@ public void request300() throws IOException {
146261
ArgumentCaptor<Integer> captor2 = ArgumentCaptor.forClass(Integer.class);
147262
ArgumentCaptor<Integer> captor3 = ArgumentCaptor.forClass(Integer.class);
148263
verify(client).execute(captor1.capture(), captor2.capture(), captor3.capture());
149-
assertEquals(Integer.valueOf(2), captor2.getValue());
150-
assertEquals(Integer.valueOf(3), captor3.getValue());
264+
assertEquals(Integer.valueOf(DatafileClient.REQUEST_BACKOFF_TIMEOUT), captor2.getValue());
265+
assertEquals(Integer.valueOf(DatafileClient.REQUEST_RETRIES_POWER), captor3.getValue());
151266
Object response = captor1.getValue().execute();
152267
assertNull(response);
153268

@@ -167,8 +282,8 @@ public void handlesIOException() throws IOException {
167282
ArgumentCaptor<Integer> captor2 = ArgumentCaptor.forClass(Integer.class);
168283
ArgumentCaptor<Integer> captor3 = ArgumentCaptor.forClass(Integer.class);
169284
verify(client).execute(captor1.capture(), captor2.capture(), captor3.capture());
170-
assertEquals(Integer.valueOf(2), captor2.getValue());
171-
assertEquals(Integer.valueOf(3), captor3.getValue());
285+
assertEquals(Integer.valueOf(DatafileClient.REQUEST_BACKOFF_TIMEOUT), captor2.getValue());
286+
assertEquals(Integer.valueOf(DatafileClient.REQUEST_RETRIES_POWER), captor3.getValue());
172287
Object response = captor1.getValue().execute();
173288
assertNull(response);
174289

@@ -180,14 +295,14 @@ public void handlesIOException() throws IOException {
180295
@Test
181296
public void handlesNullResponse() throws MalformedURLException {
182297
URL url = new URL(DatafileService.getDatafileUrl("1"));
183-
when(client.execute(any(Client.Request.class), eq(2), eq(3))).thenReturn(null);
298+
when(client.execute(any(Client.Request.class), eq(DatafileClient.REQUEST_BACKOFF_TIMEOUT), eq(DatafileClient.REQUEST_RETRIES_POWER))).thenReturn(null);
184299
assertNull(datafileClient.request(url.toString()));
185300
}
186301

187302
@Test
188303
public void handlesEmptyStringResponse() throws MalformedURLException {
189304
URL url = new URL(DatafileService.getDatafileUrl("1"));
190-
when(client.execute(any(Client.Request.class), eq(2), eq(3))).thenReturn("");
305+
when(client.execute(any(Client.Request.class), eq(DatafileClient.REQUEST_BACKOFF_TIMEOUT), eq(DatafileClient.REQUEST_RETRIES_POWER))).thenReturn("");
191306
assertEquals("", datafileClient.request(url.toString()));
192307
}
193308
}

‎datafile-handler/src/main/java/com/optimizely/ab/android/datafile_handler/DatafileClient.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,10 @@
3030
* Makes requests to the Optly CDN to get the datafile
3131
*/
3232
public class DatafileClient {
33+
// the numerical base for the exponential backoff
34+
public static final int REQUEST_BACKOFF_TIMEOUT = 2;
35+
// power the number of retries
36+
public static final int REQUEST_RETRIES_POWER = 3;
3337

3438
@NonNull private final Client client;
3539
@NonNull private final Logger logger;

‎shared/src/androidTest/java/com/optimizely/ab/android/shared/ClientTest.java

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@
2727
import java.io.ByteArrayInputStream;
2828
import java.io.IOException;
2929
import java.io.InputStream;
30+
import java.net.MalformedURLException;
31+
import java.net.URL;
3032
import java.net.URLConnection;
3133
import java.util.List;
3234

@@ -60,26 +62,53 @@ public void setup() {
6062

6163
@Test
6264
public void setIfModifiedSinceHasValueInStorage() {
63-
when(optlyStorage.getLong(Client.LAST_MODIFIED_HEADER_KEY, 0)).thenReturn(100L);
65+
URL url = null;
66+
67+
try {
68+
url = new URL("http://www.optimizely.com");
69+
} catch (MalformedURLException e) {
70+
e.printStackTrace();
71+
}
72+
73+
when(optlyStorage.getLong(url.toString(), 0)).thenReturn(100L);
6474
URLConnection urlConnection = mock(URLConnection.class);
75+
when(urlConnection.getURL()).thenReturn(url);
76+
6577
client.setIfModifiedSince(urlConnection);
6678
verify(urlConnection).setIfModifiedSince(100L);
6779
}
6880

6981
@Test
7082
public void saveLastModifiedNoHeader() {
83+
URL url = null;
84+
85+
try {
86+
url = new URL("http://www.optimizely.com");
87+
} catch (MalformedURLException e) {
88+
e.printStackTrace();
89+
}
7190
URLConnection urlConnection = mock(URLConnection.class);
91+
when(urlConnection.getURL()).thenReturn(url);
92+
7293
when(urlConnection.getLastModified()).thenReturn(0L);
7394
client.saveLastModified(urlConnection);
7495
verify(logger).warn("CDN response didn't have a last modified header");
7596
}
7697

7798
@Test
7899
public void saveLastModifiedWhenExists() {
100+
URL url = null;
101+
102+
try {
103+
url = new URL("http://www.optimizely.com");
104+
} catch (MalformedURLException e) {
105+
e.printStackTrace();
106+
}
79107
URLConnection urlConnection = mock(URLConnection.class);
80108
when(urlConnection.getLastModified()).thenReturn(100L);
109+
when(urlConnection.getURL()).thenReturn(url);
81110
client.saveLastModified(urlConnection);
82-
verify(optlyStorage).saveLong(Client.LAST_MODIFIED_HEADER_KEY, 100L);
111+
verify(optlyStorage).saveLong(url.toString(), 100L);
83112
}
84113

85114
@Test

‎shared/src/main/java/com/optimizely/ab/android/shared/Client.java

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@
3434
*/
3535
public class Client {
3636

37-
static final String LAST_MODIFIED_HEADER_KEY = "com.optimizely.ab.android.LAST_MODIFIED_HEADER";
3837
static final int MAX_BACKOFF_TIMEOUT = (int) Math.pow(2, 5);
3938

4039
@NonNull private final OptlyStorage optlyStorage;
@@ -73,7 +72,12 @@ public HttpURLConnection openConnection(URL url) {
7372
* @param urlConnection an open {@link URLConnection}
7473
*/
7574
public void setIfModifiedSince(@NonNull URLConnection urlConnection) {
76-
long lastModified = optlyStorage.getLong(LAST_MODIFIED_HEADER_KEY, 0);
75+
if (urlConnection == null || urlConnection.getURL() == null) {
76+
logger.error("Invalid connection");
77+
return;
78+
}
79+
80+
long lastModified = optlyStorage.getLong(urlConnection.getURL().toString(), 0);
7781
if (lastModified > 0) {
7882
urlConnection.setIfModifiedSince(lastModified);
7983
}
@@ -85,9 +89,14 @@ public void setIfModifiedSince(@NonNull URLConnection urlConnection) {
8589
* @param urlConnection a {@link URLConnection} instance
8690
*/
8791
public void saveLastModified(@NonNull URLConnection urlConnection) {
92+
if (urlConnection == null || urlConnection.getURL() == null) {
93+
logger.error("Invalid connection");
94+
return;
95+
}
96+
8897
long lastModified = urlConnection.getLastModified();
8998
if (lastModified > 0) {
90-
optlyStorage.saveLong(LAST_MODIFIED_HEADER_KEY, urlConnection.getLastModified());
99+
optlyStorage.saveLong(urlConnection.getURL().toString(), urlConnection.getLastModified());
91100
} else {
92101
logger.warn("CDN response didn't have a last modified header");
93102
}

0 commit comments

Comments
 (0)