Skip to content

Commit 196d725

Browse files
mikeproeng37mikeproeng37
authored
Fix various crashes and bugs (#61)
1 parent a370ca2 commit 196d725

File tree

20 files changed

+232
-220
lines changed

20 files changed

+232
-220
lines changed

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

Lines changed: 3 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -107,33 +107,12 @@ public void testExceptionHandling() throws IOException {
107107
}
108108

109109
@Test
110-
public void testLoadFileNotFound() throws IOException {
110+
public void testLoadFileNotFound() {
111111
Cache cache = mock(Cache.class);
112112
BackgroundWatchersCache backgroundWatchersCache = new BackgroundWatchersCache(cache, logger);
113113
// Cause a JSONException to be thrown
114-
when(cache.load(BackgroundWatchersCache.BACKGROUND_WATCHERS_FILE_NAME)).thenThrow(new FileNotFoundException());
114+
when(cache.load(BackgroundWatchersCache.BACKGROUND_WATCHERS_FILE_NAME)).thenReturn(null);
115115
assertFalse(backgroundWatchersCache.setIsWatching("1", true));
116-
verify(logger).info("Creating background watchers file");
117-
}
118-
119-
@Test
120-
public void testLoadIOException() throws IOException {
121-
Cache cache = mock(Cache.class);
122-
BackgroundWatchersCache backgroundWatchersCache = new BackgroundWatchersCache(cache, logger);
123-
// Cause a JSONException to be thrown
124-
when(cache.load(BackgroundWatchersCache.BACKGROUND_WATCHERS_FILE_NAME)).thenThrow(new IOException());
125-
assertFalse(backgroundWatchersCache.setIsWatching("1", true));
126-
verify(logger).error(contains("Unable to load background watchers file"), any(IOException.class));
127-
}
128-
129-
@Test
130-
public void testSaveIOException() throws IOException {
131-
Cache cache = mock(Cache.class);
132-
BackgroundWatchersCache backgroundWatchersCache = new BackgroundWatchersCache(cache, logger);
133-
// Cause a JSONException to be thrown
134-
when(cache.load(BackgroundWatchersCache.BACKGROUND_WATCHERS_FILE_NAME)).thenReturn("{}");
135-
when(cache.save(BackgroundWatchersCache.BACKGROUND_WATCHERS_FILE_NAME, "{\"1\":true}")).thenThrow(new IOException());
136-
assertFalse(backgroundWatchersCache.setIsWatching("1", true));
137-
verify(logger).error(contains("Unable to save background watchers file"), any(IOException.class));
116+
verify(logger).info("Creating background watchers file {}.", BackgroundWatchersCache.BACKGROUND_WATCHERS_FILE_NAME);
138117
}
139118
}

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

Lines changed: 0 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -81,31 +81,4 @@ public void loadJsonException() throws IOException {
8181
assertNull(dataFileCache.load());
8282
verify(logger).error(contains("Unable to parse data file"), any(JSONException.class));
8383
}
84-
85-
@Test
86-
public void testLoadFileNotFound() throws IOException {
87-
Cache cache = mock(Cache.class);
88-
DataFileCache dataFileCache = new DataFileCache("1", cache, logger);
89-
when(cache.load(dataFileCache.getFileName())).thenThrow(new FileNotFoundException());
90-
assertNull(dataFileCache.load());
91-
verify(logger).info(contains("No data file found"));
92-
}
93-
94-
@Test
95-
public void testLoadIoException() throws IOException {
96-
Cache cache = mock(Cache.class);
97-
DataFileCache dataFileCache = new DataFileCache("1", cache, logger);
98-
when(cache.load(dataFileCache.getFileName())).thenThrow(new IOException());
99-
assertNull(dataFileCache.load());
100-
verify(logger).error(contains("Unable to load data file"), any(IOException.class));
101-
}
102-
103-
@Test
104-
public void testSaveIOException() throws IOException {
105-
Cache cache = mock(Cache.class);
106-
DataFileCache dataFileCache = new DataFileCache("1", cache, logger);
107-
when(cache.save(dataFileCache.getFileName(), "")).thenThrow(new IOException());
108-
assertFalse(dataFileCache.save(""));
109-
verify(logger).error(contains("Unable to save data file"), any(IOException.class));
110-
}
11184
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,7 @@ public void handlesIOException() throws IOException {
160160
URL url = new URL(DataFileService.getDatafileUrl("1"));
161161
when(client.openConnection(url)).thenReturn(urlConnection);
162162
when(urlConnection.getResponseCode()).thenReturn(200);
163-
doThrow(new IOException()).when(client).readStream(urlConnection);
163+
doThrow(new IOException()).when(urlConnection).connect();
164164

165165
dataFileClient.request(url.toString());
166166
ArgumentCaptor<Client.Request> captor1 = ArgumentCaptor.forClass(Client.Request.class);

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

Lines changed: 10 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -106,18 +106,10 @@ List<String> getWatchingProjectIds() {
106106

107107
@Nullable
108108
private JSONObject load() throws JSONException {
109-
String backGroundWatchersFile = null;
110-
try {
111-
backGroundWatchersFile = cache.load(BACKGROUND_WATCHERS_FILE_NAME);
112-
} catch (FileNotFoundException e) {
113-
logger.info("Creating background watchers file");
114-
backGroundWatchersFile = "{}";
115-
} catch (IOException e) {
116-
logger.error("Unable to load background watchers file", e);
117-
}
118-
109+
String backGroundWatchersFile = cache.load(BACKGROUND_WATCHERS_FILE_NAME);
119110
if (backGroundWatchersFile == null) {
120-
return null;
111+
backGroundWatchersFile = "{}";
112+
logger.info("Creating background watchers file {}.", BACKGROUND_WATCHERS_FILE_NAME);
121113
}
122114

123115
return new JSONObject(backGroundWatchersFile);
@@ -128,11 +120,13 @@ private boolean delete() {
128120
}
129121

130122
private boolean save(String backgroundWatchersJson) {
131-
try {
132-
return cache.save(BACKGROUND_WATCHERS_FILE_NAME, backgroundWatchersJson);
133-
} catch (IOException e) {
134-
logger.error("Unable to save background watchers file", e);
135-
return false;
123+
logger.info("Saving background watchers file {}.", BACKGROUND_WATCHERS_FILE_NAME);
124+
boolean saved = cache.save(BACKGROUND_WATCHERS_FILE_NAME, backgroundWatchersJson);
125+
if (saved) {
126+
logger.info("Saved background watchers file {}.", BACKGROUND_WATCHERS_FILE_NAME);
127+
} else {
128+
logger.warn("Unable to save background watchers file {}.", BACKGROUND_WATCHERS_FILE_NAME);
136129
}
130+
return saved;
137131
}
138132
}

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

Lines changed: 2 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -48,14 +48,7 @@ public class DataFileCache {
4848

4949
@Nullable
5050
JSONObject load() {
51-
String optlyDataFile = null;
52-
try {
53-
optlyDataFile = cache.load(getFileName());
54-
} catch (FileNotFoundException e) {
55-
logger.info("No data file found");
56-
} catch (IOException e) {
57-
logger.error("Unable to load data file", e);
58-
}
51+
String optlyDataFile = cache.load(getFileName());
5952

6053
if (optlyDataFile == null) {
6154
return null;
@@ -78,12 +71,7 @@ boolean exists() {
7871
}
7972

8073
boolean save(String dataFile) {
81-
try {
82-
return cache.save(getFileName(), dataFile);
83-
} catch (IOException e) {
84-
logger.error("Unable to save data file", e);
85-
return false;
86-
}
74+
return cache.save(getFileName(), dataFile);
8775
}
8876

8977

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,10 @@ public String execute() {
6464
logger.info("Requesting data file from {}", url);
6565
urlConnection = client.openConnection(url);
6666

67+
if (urlConnection == null) {
68+
return null;
69+
}
70+
6771
client.setIfModifiedSince(urlConnection);
6872

6973
urlConnection.setConnectTimeout(5 * 1000);

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -485,6 +485,10 @@ static class DataFileServiceConnection implements ServiceConnection {
485485
@Override
486486
public void onServiceConnected(ComponentName className,
487487
IBinder service) {
488+
if (!(service instanceof DataFileService.LocalBinder)) {
489+
return;
490+
}
491+
488492
// We've bound to DataFileService, cast the IBinder and get DataFileService instance
489493
DataFileService.LocalBinder binder = (DataFileService.LocalBinder) service;
490494
final DataFileService dataFileService = binder.getService();

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,11 @@ public Boolean execute() {
4848
try {
4949
logger.info("Dispatching event: {}", event);
5050
HttpURLConnection urlConnection = client.openConnection(event.getURL());
51+
52+
if (urlConnection == null) {
53+
return Boolean.FALSE;
54+
}
55+
5156
urlConnection.setRequestMethod("POST");
5257
urlConnection.setRequestProperty("Content-Type", "application/json");
5358
urlConnection.setDoOutput(true);
@@ -75,6 +80,7 @@ public Boolean execute() {
7580
if (success == null) {
7681
success = false;
7782
}
83+
logger.info("Successfully dispatched event: {}", event);
7884
return success;
7985
}
8086
}

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

Lines changed: 72 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -52,18 +52,25 @@ static EventDAO getInstance(@NonNull Context context, @NonNull String projectId,
5252
}
5353

5454
boolean storeEvent(@NonNull Event event) {
55+
logger.info("Inserting {} into db", event);
5556
ContentValues values = new ContentValues();
5657
values.put(EventTable.Column.URL, event.getURL().toString());
5758
values.put(EventTable.Column.REQUEST_BODY, event.getRequestBody());
5859

5960
// Since we are setting the "null column hack" param to null empty values will not be inserted
6061
// at all instead of inserting null.
61-
long newRowId;
62-
newRowId = dbHelper.getWritableDatabase().insert(EventTable.NAME, null, values);
62+
try {
63+
long newRowId;
64+
newRowId = dbHelper.getWritableDatabase().insert(EventTable.NAME, null, values);
6365

64-
logger.info("Inserted {} into db", event);
66+
logger.info("Inserted {} into db", event);
6567

66-
return newRowId != -1;
68+
return newRowId != -1;
69+
} catch (Exception e) {
70+
logger.error("Error inserting Optimizely event into db.", e);
71+
}
72+
73+
return false;
6774
}
6875

6976
List<Pair<Long, Event>> getEvents() {
@@ -77,40 +84,48 @@ List<Pair<Long, Event>> getEvents() {
7784
EventTable.Column.REQUEST_BODY,
7885
};
7986

80-
Cursor cursor = dbHelper.getReadableDatabase().query(
81-
EventTable.NAME, // The table to query
82-
projection, // The columns to return
83-
null, // The columns for the WHERE clause
84-
null, // The values for the WHERE clause
85-
null, // don't group the rows
86-
null, // don't filter by row groups
87-
null // The sort order
88-
);
89-
90-
if (cursor.moveToFirst()) {
91-
do {
92-
long itemId = cursor.getLong(
93-
cursor.getColumnIndexOrThrow(EventTable._ID)
94-
);
95-
String url = cursor.getString(
96-
cursor.getColumnIndexOrThrow(EventTable.Column.URL)
97-
);
98-
String requestBody = cursor.getString(
99-
cursor.getColumnIndexOrThrow(EventTable.Column.REQUEST_BODY)
100-
);
101-
try {
102-
events.add(new Pair<>(itemId, new Event(new URL(url), requestBody)));
103-
} catch (MalformedURLException e) {
104-
logger.error("Retrieved a malformed event from storage", e);
105-
106-
}
107-
} while (cursor.moveToNext());
108-
109-
cursor.close();
110-
111-
logger.info("Got events from SQLite");
87+
Cursor cursor = null;
88+
try {
89+
cursor = dbHelper.getReadableDatabase().query(
90+
EventTable.NAME, // The table to query
91+
projection, // The columns to return
92+
null, // The columns for the WHERE clause
93+
null, // The values for the WHERE clause
94+
null, // don't group the rows
95+
null, // don't filter by row groups
96+
null // The sort order
97+
);
98+
logger.info("Opened database");
99+
} catch (Exception e) {
100+
logger.error("Failed to open database.", e);
112101
}
113102

103+
try {
104+
if (cursor != null && cursor.moveToFirst()) {
105+
do {
106+
long itemId = cursor.getLong(
107+
cursor.getColumnIndexOrThrow(EventTable._ID)
108+
);
109+
String url = cursor.getString(
110+
cursor.getColumnIndexOrThrow(EventTable.Column.URL)
111+
);
112+
String requestBody = cursor.getString(
113+
cursor.getColumnIndexOrThrow(EventTable.Column.REQUEST_BODY)
114+
);
115+
try {
116+
events.add(new Pair<>(itemId, new Event(new URL(url), requestBody)));
117+
} catch (MalformedURLException e) {
118+
logger.error("Retrieved a malformed event from storage", e);
119+
}
120+
} while (cursor.moveToNext());
121+
122+
cursor.close();
123+
124+
logger.info("Got events from SQLite");
125+
}
126+
} catch (Exception e) {
127+
logger.error("Error reading events db cursor", e);
128+
}
114129

115130
return events;
116131
}
@@ -120,20 +135,31 @@ boolean removeEvent(long eventId) {
120135
String selection = EventTable._ID + " = ?";
121136
// Specify arguments in placeholder order.
122137
String[] selectionArgs = {String.valueOf(eventId)};
123-
// Issue SQL statement.
124-
int numRowsDeleted = dbHelper.getWritableDatabase().delete(EventTable.NAME, selection, selectionArgs);
125-
126-
if (numRowsDeleted > 0) {
127-
logger.info("Removed event with id {} from db", eventId);
128-
return true;
129-
} else {
130-
logger.error("Tried to remove an event id {} that does not exist", eventId);
138+
139+
try {
140+
// Issue SQL statement.
141+
int numRowsDeleted = dbHelper.getWritableDatabase().delete(EventTable.NAME, selection, selectionArgs);
142+
143+
if (numRowsDeleted > 0) {
144+
logger.info("Removed event with id {} from db", eventId);
145+
return true;
146+
} else {
147+
logger.error("Tried to remove an event id {} that does not exist", eventId);
148+
}
149+
150+
return numRowsDeleted > 0;
151+
} catch (Exception e) {
152+
logger.error("Could not open db.", e);
131153
}
132154

133-
return numRowsDeleted > 0;
155+
return false;
134156
}
135157

136158
void closeDb() {
137-
dbHelper.close();
159+
try {
160+
dbHelper.close();
161+
} catch (Exception e) {
162+
logger.warn("Error closing db.", e);
163+
}
138164
}
139165
}

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

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -71,26 +71,33 @@ void dispatch(@NonNull Intent intent) {
7171
dispatched = dispatch(event);
7272
} catch (MalformedURLException e) {
7373
logger.error("Received a malformed URL in event handler service", e);
74+
} catch (Exception e) {
75+
logger.warn("Failed to dispatch event.", e);
7476
}
7577
}
7678

77-
if (!dispatched) {
78-
long interval = getInterval(intent);
79-
serviceScheduler.schedule(intent, interval);
80-
saveInterval(interval);
81-
logger.info("Scheduled events to be dispatched");
82-
} else {
83-
// Quit trying to dispatch events because their aren't any in storage
84-
serviceScheduler.unschedule(intent);
85-
logger.info("Unscheduled event dispatch");
79+
80+
try {
81+
if (!dispatched) {
82+
long interval = getInterval(intent);
83+
serviceScheduler.schedule(intent, interval);
84+
saveInterval(interval);
85+
logger.info("Scheduled events to be dispatched");
86+
} else {
87+
// Quit trying to dispatch events because their aren't any in storage
88+
serviceScheduler.unschedule(intent);
89+
logger.info("Unscheduled event dispatch");
90+
}
91+
} catch (Exception e) {
92+
logger.warn("Failed to schedule event dispatch.", e);
8693
}
8794

8895
eventDAO.closeDb();
8996
}
9097

9198
// Either grab the interval for the first time from Intent or from storage
9299
// The extra won't exist if we are being restarted after a reboot or app update
93-
private long getInterval(Intent intent) {
100+
private long getInterval(@NonNull Intent intent) {
94101
long duration = intent.getLongExtra(EventIntentService.EXTRA_INTERVAL, -1);
95102
// We are either scheduling for the first time or rescheduling after our alarms were cancelled
96103
if (duration == -1) {

0 commit comments

Comments
 (0)