Skip to content

Commit 4099639

Browse files
jkasten2jkasten2
authored
Merge pull request #91 from OneSignal/db_improvments
Fixed local notification database concurrency issues
2 parents d5e3bba + b9257af commit 4099639

File tree

9 files changed

+181
-107
lines changed

9 files changed

+181
-107
lines changed

‎OneSignalSDK/app/src/test/java/com/onesignal/StaticResetHelper.java

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,13 @@ public boolean onOtherField(Field field) throws Exception {
3737
return false;
3838
}
3939
}));
40+
41+
classes.add(new StaticResetHelper().new ClassState(OneSignalDbHelper.class, new OtherFieldHandler() {
42+
@Override
43+
public boolean onOtherField(Field field) {
44+
return false;
45+
}
46+
}));
4047
}
4148

4249
private interface OtherFieldHandler {

‎OneSignalSDK/app/src/test/java/com/test/onesignal/GenerateNotificationRunner.java

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,13 +52,15 @@
5252
import com.onesignal.ShadowOneSignalRestClient;
5353
import com.onesignal.ShadowRoboNotificationManager;
5454
import com.onesignal.ShadowRoboNotificationManager.PostedNotification;
55+
import com.onesignal.StaticResetHelper;
5556
import com.onesignal.example.BlankActivity;
5657
import com.onesignal.OneSignalPackagePrivateHelper.NotificationTable;
5758
import com.onesignal.OneSignalPackagePrivateHelper.NotificationRestorer;
5859

5960
import junit.framework.Assert;
6061

6162
import org.json.JSONObject;
63+
import org.junit.AfterClass;
6264
import org.junit.Before;
6365
import org.junit.BeforeClass;
6466
import org.junit.Test;
@@ -92,6 +94,7 @@ public class GenerateNotificationRunner {
9294
@BeforeClass // Runs only once, before any tests
9395
public static void setUpClass() throws Exception {
9496
ShadowLog.stream = System.out;
97+
StaticResetHelper.saveStaticValues();
9598
}
9699

97100
@Before // Before each test
@@ -105,6 +108,12 @@ public void beforeEachTest() throws Exception {
105108
ShadowBadgeCountUpdater.lastCount = 0;
106109
NotificationManager notificationManager = (NotificationManager)blankActivity.getSystemService(Context.NOTIFICATION_SERVICE);
107110
notificationManager.cancelAll();
111+
StaticResetHelper.restSetStaticFields();
112+
}
113+
114+
@AfterClass
115+
public static void afterEverything() {
116+
StaticResetHelper.restSetStaticFields();
108117
}
109118

110119

@@ -165,7 +174,7 @@ public void shouldHandleBasicNotifications() throws Exception {
165174
Assert.assertEquals(1, ShadowBadgeCountUpdater.lastCount);
166175

167176
// Should have 1 DB record with the correct time stamp
168-
SQLiteDatabase readableDb = new OneSignalDbHelper(blankActivity).getReadableDatabase();
177+
SQLiteDatabase readableDb = OneSignalDbHelper.getInstance(blankActivity).getReadableDatabase();
169178
Cursor cursor = readableDb.query(NotificationTable.TABLE_NAME, new String[] { "created_time" }, null, null, null, null, null);
170179
Assert.assertEquals(1, cursor.getCount());
171180
// Time stamp should be set and within a small range.
@@ -185,6 +194,7 @@ public void shouldHandleBasicNotifications() throws Exception {
185194

186195
// Should not display a duplicate notification, count should still be 1
187196
NotificationBundleProcessor_ProcessFromGCMIntentService(blankActivity, bundle, null);
197+
readableDb = OneSignalDbHelper.getInstance(blankActivity).getReadableDatabase();
188198
cursor = readableDb.query(NotificationTable.TABLE_NAME, null, null, null, null, null, null);
189199
Assert.assertEquals(1, cursor.getCount());
190200
Assert.assertEquals(0, ShadowBadgeCountUpdater.lastCount);
@@ -203,6 +213,7 @@ public void shouldHandleBasicNotifications() throws Exception {
203213
// First opened should of been cleaned up, 1 week old non opened notification should stay, and one new record.
204214
bundle = getBaseNotifBundle("UUID3");
205215
NotificationBundleProcessor_ProcessFromGCMIntentService(blankActivity, bundle, null);
216+
readableDb = OneSignalDbHelper.getInstance(blankActivity).getReadableDatabase();
206217
cursor = readableDb.query(NotificationTable.TABLE_NAME, new String[] { "android_notification_id", "created_time" }, null, null, null, null, null);
207218

208219
Assert.assertEquals(1, cursor.getCount());
@@ -256,9 +267,10 @@ public void shouldGenerate2BasicGroupNotifications() throws Exception {
256267

257268

258269
// Should be 2 DB entries (summary and individual)
259-
SQLiteDatabase readableDb = new OneSignalDbHelper(blankActivity).getReadableDatabase();
270+
SQLiteDatabase readableDb = OneSignalDbHelper.getInstance(blankActivity).getReadableDatabase();
260271
Cursor cursor = readableDb.query(NotificationTable.TABLE_NAME, null, null, null, null, null, null);
261272
Assert.assertEquals(2, cursor.getCount());
273+
cursor.close();
262274

263275

264276
// Add another notification to the group.
@@ -285,6 +297,7 @@ public void shouldGenerate2BasicGroupNotifications() throws Exception {
285297

286298

287299
// Should be 3 DB entries (summary and 2 individual)
300+
readableDb = OneSignalDbHelper.getInstance(blankActivity).getReadableDatabase();
288301
cursor = readableDb.query(NotificationTable.TABLE_NAME, null, null, null, null, null, null);
289302
Assert.assertEquals(3, cursor.getCount());
290303

‎OneSignalSDK/app/src/test/java/com/test/onesignal/MainOneSignalClassRunner.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,7 @@ public void tagsAvailable(JSONObject tags) {
132132
});
133133
}
134134

135-
private static void cleanUp() {
135+
static void cleanUp() {
136136
callBackUseId = getCallBackRegId = null;
137137
StaticResetHelper.restSetStaticFields();
138138

@@ -1259,7 +1259,7 @@ public void shouldCancelAndClearNotifications() throws Exception {
12591259
Assert.assertEquals(0, ShadowRoboNotificationManager.notifications.size());
12601260

12611261
// Make sure they are marked dismissed.
1262-
SQLiteDatabase readableDb = new OneSignalDbHelper(blankActivity).getReadableDatabase();
1262+
SQLiteDatabase readableDb = OneSignalDbHelper.getInstance(blankActivity).getReadableDatabase();
12631263
Cursor cursor = readableDb.query(OneSignalPackagePrivateHelper.NotificationTable.TABLE_NAME, new String[] { "created_time" },
12641264
OneSignalPackagePrivateHelper.NotificationTable.COLUMN_NAME_DISMISSED + " = 1", null, null, null, null);
12651265
Assert.assertEquals(2, cursor.getCount());

‎OneSignalSDK/onesignal/src/main/java/com/onesignal/GenerateNotification.java

Lines changed: 56 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -331,8 +331,8 @@ static void createSummaryNotification(Context inContext, boolean updateSummary,
331331
Random random = new Random();
332332
PendingIntent summaryDeleteIntent = getNewActionPendingIntent(random.nextInt(), getNewBaseDeleteIntent(0).putExtra("summary", group));
333333

334-
OneSignalDbHelper dbHelper = new OneSignalDbHelper(currentContext);
335-
SQLiteDatabase writableDb = dbHelper.getWritableDatabase();
334+
OneSignalDbHelper dbHelper = OneSignalDbHelper.getInstance(currentContext);
335+
SQLiteDatabase readableDb = dbHelper.getReadableDatabase();
336336

337337
String[] retColumn = { NotificationTable.COLUMN_NAME_ANDROID_NOTIFICATION_ID,
338338
NotificationTable.COLUMN_NAME_FULL_DATA,
@@ -342,7 +342,7 @@ static void createSummaryNotification(Context inContext, boolean updateSummary,
342342

343343
String[] whereArgs = { group };
344344

345-
Cursor cursor = writableDb.query(
345+
Cursor cursor = readableDb.query(
346346
NotificationTable.TABLE_NAME,
347347
retColumn,
348348
NotificationTable.COLUMN_NAME_GROUP_ID + " = ? AND " + // Where String
@@ -360,42 +360,49 @@ static void createSummaryNotification(Context inContext, boolean updateSummary,
360360
String firstFullData = null;
361361
Collection<SpannableString> summeryList = null;
362362

363-
if (cursor.moveToFirst()) {
364-
SpannableString spannableString;
365-
summeryList = new ArrayList<>();
366-
367-
do {
368-
if (cursor.getInt(cursor.getColumnIndex(NotificationTable.COLUMN_NAME_IS_SUMMARY)) == 1)
369-
summaryNotificationId = cursor.getInt(cursor.getColumnIndex(NotificationTable.COLUMN_NAME_ANDROID_NOTIFICATION_ID));
370-
else {
371-
String title = cursor.getString(cursor.getColumnIndex(NotificationTable.COLUMN_NAME_TITLE));
372-
if (title == null)
373-
title = "";
374-
else
375-
title += " ";
376-
377-
// Html.fromHtml("<strong>" + line1Title + "</strong> " + gcmBundle.getString("alert"));
378-
379-
String msg = cursor.getString(cursor.getColumnIndex(NotificationTable.COLUMN_NAME_MESSAGE));
380-
381-
spannableString = new SpannableString(title + msg);
382-
if (title.length() > 0)
383-
spannableString.setSpan(new StyleSpan(android.graphics.Typeface.BOLD), 0, title.length(), 0);
384-
summeryList.add(spannableString);
385-
386-
if (firstFullData == null)
387-
firstFullData = cursor.getString(cursor.getColumnIndex(NotificationTable.COLUMN_NAME_FULL_DATA));
388-
}
389-
} while (cursor.moveToNext());
363+
try {
364+
if (cursor.moveToFirst()) {
365+
SpannableString spannableString;
366+
summeryList = new ArrayList<>();
367+
368+
do {
369+
if (cursor.getInt(cursor.getColumnIndex(NotificationTable.COLUMN_NAME_IS_SUMMARY)) == 1)
370+
summaryNotificationId = cursor.getInt(cursor.getColumnIndex(NotificationTable.COLUMN_NAME_ANDROID_NOTIFICATION_ID));
371+
else {
372+
String title = cursor.getString(cursor.getColumnIndex(NotificationTable.COLUMN_NAME_TITLE));
373+
if (title == null)
374+
title = "";
375+
else
376+
title += " ";
377+
378+
// Html.fromHtml("<strong>" + line1Title + "</strong> " + gcmBundle.getString("alert"));
379+
380+
String msg = cursor.getString(cursor.getColumnIndex(NotificationTable.COLUMN_NAME_MESSAGE));
381+
382+
spannableString = new SpannableString(title + msg);
383+
if (title.length() > 0)
384+
spannableString.setSpan(new StyleSpan(android.graphics.Typeface.BOLD), 0, title.length(), 0);
385+
summeryList.add(spannableString);
386+
387+
if (firstFullData == null)
388+
firstFullData = cursor.getString(cursor.getColumnIndex(NotificationTable.COLUMN_NAME_FULL_DATA));
389+
}
390+
} while (cursor.moveToNext());
390391

391-
if (updateSummary) {
392-
try {
393-
gcmBundle = new JSONObject(firstFullData);
394-
} catch (JSONException e) {
395-
e.printStackTrace();
392+
if (updateSummary) {
393+
try {
394+
gcmBundle = new JSONObject(firstFullData);
395+
} catch (JSONException e) {
396+
e.printStackTrace();
397+
}
396398
}
397399
}
398400
}
401+
finally {
402+
if (cursor != null && !cursor.isClosed())
403+
cursor.close();
404+
}
405+
399406

400407
if (summeryList != null && (!updateSummary || summeryList.size() > 1)) {
401408
int notificationCount = summeryList.size() + (updateSummary ? 0 : 1);
@@ -461,12 +468,21 @@ static void createSummaryNotification(Context inContext, boolean updateSummary,
461468
}
462469
else {
463470
// There currently isn't a visible notification from this group, save the group summary notification id and post it so it looks like a normal notification.
464-
ContentValues values = new ContentValues();
465-
values.put(NotificationTable.COLUMN_NAME_ANDROID_NOTIFICATION_ID, summaryNotificationId);
466-
values.put(NotificationTable.COLUMN_NAME_GROUP_ID, group);
467-
values.put(NotificationTable.COLUMN_NAME_IS_SUMMARY, 1);
471+
SQLiteDatabase writableDb = dbHelper.getWritableDatabase();
472+
writableDb.beginTransaction();
468473

469-
writableDb.insert(NotificationTable.TABLE_NAME, null, values);
474+
try {
475+
ContentValues values = new ContentValues();
476+
values.put(NotificationTable.COLUMN_NAME_ANDROID_NOTIFICATION_ID, summaryNotificationId);
477+
values.put(NotificationTable.COLUMN_NAME_GROUP_ID, group);
478+
values.put(NotificationTable.COLUMN_NAME_IS_SUMMARY, 1);
479+
writableDb.insertOrThrow(NotificationTable.TABLE_NAME, null, values);
480+
writableDb.setTransactionSuccessful();
481+
} catch (Exception e) {
482+
OneSignal.Log(OneSignal.LOG_LEVEL.ERROR, "Error adding summary notification record! ", e);
483+
} finally {
484+
writableDb.endTransaction();
485+
}
470486

471487
NotificationCompat.Builder notifBuilder = getBaseNotificationCompatBuilder(gcmBundle);
472488
if (updateSummary)
@@ -485,9 +501,6 @@ static void createSummaryNotification(Context inContext, boolean updateSummary,
485501
}
486502

487503
NotificationManagerCompat.from(currentContext).notify(summaryNotificationId, summaryNotification);
488-
489-
cursor.close();
490-
writableDb.close();
491504
}
492505

493506
// Keep 'throws Throwable' as 'onesignal_bgimage_notif_layout' may not be available if the app project isn't setup correctly.

‎OneSignalSDK/onesignal/src/main/java/com/onesignal/NotificationBundleProcessor.java

Lines changed: 24 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -110,32 +110,38 @@ static void saveNotification(Context context, JSONObject jsonPayload, boolean op
110110
try {
111111
JSONObject customJSON = new JSONObject(jsonPayload.optString("custom"));
112112

113-
OneSignalDbHelper dbHelper = new OneSignalDbHelper(context);
113+
OneSignalDbHelper dbHelper = OneSignalDbHelper.getInstance(context);
114114
SQLiteDatabase writableDb = dbHelper.getWritableDatabase();
115115

116-
deleteOldNotifications(writableDb);
117-
118-
ContentValues values = new ContentValues();
119-
values.put(NotificationTable.COLUMN_NAME_NOTIFICATION_ID, customJSON.optString("i"));
120-
if (jsonPayload.has("grp"))
121-
values.put(NotificationTable.COLUMN_NAME_GROUP_ID, jsonPayload.optString("grp"));
116+
writableDb.beginTransaction();
117+
try {
118+
deleteOldNotifications(writableDb);
122119

123-
values.put(NotificationTable.COLUMN_NAME_OPENED, opened ? 1 : 0);
124-
if (!opened)
125-
values.put(NotificationTable.COLUMN_NAME_ANDROID_NOTIFICATION_ID, notificationId);
120+
ContentValues values = new ContentValues();
121+
values.put(NotificationTable.COLUMN_NAME_NOTIFICATION_ID, customJSON.optString("i"));
122+
if (jsonPayload.has("grp"))
123+
values.put(NotificationTable.COLUMN_NAME_GROUP_ID, jsonPayload.optString("grp"));
126124

127-
if (jsonPayload.has("title"))
128-
values.put(NotificationTable.COLUMN_NAME_TITLE, jsonPayload.optString("title"));
129-
values.put(NotificationTable.COLUMN_NAME_MESSAGE, jsonPayload.optString("alert"));
125+
values.put(NotificationTable.COLUMN_NAME_OPENED, opened ? 1 : 0);
126+
if (!opened)
127+
values.put(NotificationTable.COLUMN_NAME_ANDROID_NOTIFICATION_ID, notificationId);
130128

131-
values.put(NotificationTable.COLUMN_NAME_FULL_DATA, jsonPayload.toString());
129+
if (jsonPayload.has("title"))
130+
values.put(NotificationTable.COLUMN_NAME_TITLE, jsonPayload.optString("title"));
131+
values.put(NotificationTable.COLUMN_NAME_MESSAGE, jsonPayload.optString("alert"));
132132

133-
writableDb.insert(NotificationTable.TABLE_NAME, null, values);
133+
values.put(NotificationTable.COLUMN_NAME_FULL_DATA, jsonPayload.toString());
134134

135-
if (!opened)
136-
BadgeCountUpdater.update(writableDb, context);
135+
writableDb.insertOrThrow(NotificationTable.TABLE_NAME, null, values);
137136

138-
writableDb.close();
137+
if (!opened)
138+
BadgeCountUpdater.update(writableDb, context);
139+
writableDb.setTransactionSuccessful();
140+
} catch (Exception e) {
141+
OneSignal.Log(OneSignal.LOG_LEVEL.ERROR, "Error saving notification record! ", e);
142+
} finally {
143+
writableDb.endTransaction();
144+
}
139145
} catch (JSONException e) {
140146
e.printStackTrace();
141147
}

‎OneSignalSDK/onesignal/src/main/java/com/onesignal/NotificationOpenedProcessor.java

Lines changed: 19 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -65,9 +65,6 @@ static void processIntent(Context incContext, Intent inIntent) {
6565

6666
boolean dismissed = intent.getBooleanExtra("dismissed", false);
6767

68-
OneSignalDbHelper dbHelper = new OneSignalDbHelper(context);
69-
SQLiteDatabase writableDb = dbHelper.getWritableDatabase();
70-
7168
JSONArray dataArray = null;
7269
if (!dismissed) {
7370
try {
@@ -80,17 +77,25 @@ static void processIntent(Context incContext, Intent inIntent) {
8077
}
8178
}
8279

83-
// We just opened a summary notification.
84-
if (!dismissed && summaryGroup != null)
85-
addChildNotifications(dataArray, summaryGroup, writableDb);
86-
87-
markNotificationsConsumed(writableDb);
88-
89-
// Notification is not a summary type but a single notification part of a group.
90-
if (summaryGroup == null && intent.getStringExtra("grp") != null)
91-
updateSummaryNotification(writableDb);
92-
93-
writableDb.close();
80+
OneSignalDbHelper dbHelper = OneSignalDbHelper.getInstance(context);
81+
SQLiteDatabase writableDb = dbHelper.getWritableDatabase();
82+
writableDb.beginTransaction();
83+
try {
84+
// We just opened a summary notification.
85+
if (!dismissed && summaryGroup != null)
86+
addChildNotifications(dataArray, summaryGroup, writableDb);
87+
88+
markNotificationsConsumed(writableDb);
89+
90+
// Notification is not a summary type but a single notification part of a group.
91+
if (summaryGroup == null && intent.getStringExtra("grp") != null)
92+
updateSummaryNotification(writableDb);
93+
writableDb.setTransactionSuccessful();
94+
} catch (Exception e) {
95+
OneSignal.Log(OneSignal.LOG_LEVEL.ERROR, "Error processing notification open or dismiss record! ", e);
96+
} finally {
97+
writableDb.endTransaction();
98+
}
9499

95100
if (!dismissed)
96101
OneSignal.handleNotificationOpen(context, dataArray, inIntent.getBooleanExtra("from_alert", false));

‎OneSignalSDK/onesignal/src/main/java/com/onesignal/NotificationRestorer.java

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -53,10 +53,18 @@ public static void restore(Context context) {
5353
return;
5454
restored = true;
5555

56-
OneSignalDbHelper dbHelper = new OneSignalDbHelper(context);
56+
OneSignalDbHelper dbHelper = OneSignalDbHelper.getInstance(context);
5757
SQLiteDatabase writableDb = dbHelper.getWritableDatabase();
5858

59-
NotificationBundleProcessor.deleteOldNotifications(writableDb);
59+
writableDb.beginTransaction();
60+
try {
61+
NotificationBundleProcessor.deleteOldNotifications(writableDb);
62+
writableDb.setTransactionSuccessful();
63+
} catch (Exception e) {
64+
OneSignal.Log(OneSignal.LOG_LEVEL.ERROR, "Error deleting old notification records! ", e);
65+
} finally {
66+
writableDb.endTransaction();
67+
}
6068

6169
String[] retColumn = { NotificationTable.COLUMN_NAME_ANDROID_NOTIFICATION_ID,
6270
NotificationTable.COLUMN_NAME_FULL_DATA };
@@ -97,6 +105,5 @@ public static void restore(Context context) {
97105
}
98106

99107
cursor.close();
100-
writableDb.close();
101108
}
102109
}

0 commit comments

Comments
 (0)