Skip to content

Commit 282a3df

Browse files
mikechochmikechoch
authored
Adding 2 layers of control to prevent locks in SQL DB and minor clean up (#1050)
* Adding 2 layers of SQL DB security and minor clean up * Removed the commented `enableWriteAheadLogging` * No need for this as this allows concurrent read and writes to occur and we would rather pause a read while a write occurs * Pausing will ensure that we are querying the most updated data * 2 layers of security for SQL DB regarding `OSInAppMessageRepository.java` * First one will be transactions using `beginTransaction`, `setTransactionSuccessful`, and `endTransaction` methods wherever we `insert`, `delete`, or `update` * Second one is keeping TABLE manipulation or reading within the same file and setting these methods as synchronized so that any other TABLE touching that occurs won't happen till any current methods are complete * The combination of the two steps should help prevent any conflicts between SQL DB and solve the locking issues * WIP - Making sure any DB reading and writing is from the same file so `synchronized` along with transactions provide this 2 layer security around our single SQL DB instance * Forgot to change `Throwable` to `SQLException` instead * Fixed within `OSInAppMessageRepository.java` * Using singleton `OSInAppMessageController.java` instead of new * Created singleton based `getInAppMessageRepository` method inside of `OSInAppMessageController.java` class * This allows the synchronized to apply to multiple methods being called from the class instance * Deleted unused thread name since new Thread was moved toi cache cleaning file with different thread name now * Needed to change defaults again for `getStringSet` * `OneSignalCacheCleaner.java` was rearranged a bit and it caused conflicts * `OSInAppMessageRepository.java` now has those correct changes an the default was flipped `OSUtils.<String>newConcurrentSet()` to `null`
2 parents 87f75f4 + 2feac9d commit 282a3df

File tree

5 files changed

+241
-184
lines changed

5 files changed

+241
-184
lines changed

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

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@ class OSInAppMessageController implements OSDynamicTriggerControllerObserver, OS
3232

3333
public static final String IN_APP_MESSAGES_JSON_KEY = "in_app_messages";
3434
private static final String OS_SAVE_IN_APP_MESSAGE = "OS_SAVE_IN_APP_MESSAGE";
35-
private static final String OS_DELETE_IN_APP_MESSAGE = "OS_DELETE_IN_APP_MESSAGE";
3635

3736
OSTriggerController triggerController;
3837
private OSSystemConditionController systemConditionController;
@@ -122,8 +121,15 @@ protected OSInAppMessageController(OneSignalDbHelper dbHelper) {
122121
initRedisplayData(dbHelper);
123122
}
124123

124+
OSInAppMessageRepository getInAppMessageRepository(OneSignalDbHelper dbHelper) {
125+
if (inAppMessageRepository == null)
126+
inAppMessageRepository = new OSInAppMessageRepository(dbHelper);
127+
128+
return inAppMessageRepository;
129+
}
130+
125131
protected void initRedisplayData(OneSignalDbHelper dbHelper) {
126-
inAppMessageRepository = new OSInAppMessageRepository(dbHelper);
132+
inAppMessageRepository = getInAppMessageRepository(dbHelper);
127133
redisplayedInAppMessages = inAppMessageRepository.getCachedInAppMessages();
128134

129135
OneSignal.Log(OneSignal.LOG_LEVEL.DEBUG, "redisplayedInAppMessages: " + redisplayedInAppMessages.toString());

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

Lines changed: 168 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import android.content.ContentValues;
44
import android.database.Cursor;
5+
import android.database.SQLException;
56
import android.database.sqlite.SQLiteDatabase;
67
import android.support.annotation.WorkerThread;
78

@@ -14,6 +15,8 @@
1415

1516
class OSInAppMessageRepository {
1617

18+
final static long IAM_CACHE_DATA_LIFETIME = 15_552_000L; // 6 months in seconds
19+
1720
private final OneSignalDbHelper dbHelper;
1821

1922
OSInAppMessageRepository(OneSignalDbHelper dbHelper) {
@@ -23,18 +26,28 @@ class OSInAppMessageRepository {
2326
@WorkerThread
2427
synchronized void saveInAppMessage(OSInAppMessage inAppMessage) {
2528
SQLiteDatabase writableDb = dbHelper.getSQLiteDatabaseWithRetries();
26-
27-
ContentValues values = new ContentValues();
28-
values.put(OneSignalDbContract.InAppMessageTable.COLUMN_NAME_MESSAGE_ID, inAppMessage.messageId);
29-
values.put(OneSignalDbContract.InAppMessageTable.COLUMN_NAME_DISPLAY_QUANTITY, inAppMessage.getRedisplayStats().getDisplayQuantity());
30-
values.put(OneSignalDbContract.InAppMessageTable.COLUMN_NAME_LAST_DISPLAY, inAppMessage.getRedisplayStats().getLastDisplayTime());
31-
values.put(OneSignalDbContract.InAppMessageTable.COLUMN_CLICK_IDS, inAppMessage.getClickedClickIds().toString());
32-
values.put(OneSignalDbContract.InAppMessageTable.COLUMN_DISPLAYED_IN_SESSION, inAppMessage.isDisplayedInSession());
33-
34-
int rowsUpdated = writableDb.update(OneSignalDbContract.InAppMessageTable.TABLE_NAME, values,
35-
OneSignalDbContract.InAppMessageTable.COLUMN_NAME_MESSAGE_ID + " = ?", new String[]{inAppMessage.messageId});
36-
if (rowsUpdated == 0)
37-
writableDb.insert(OneSignalDbContract.InAppMessageTable.TABLE_NAME, null, values);
29+
writableDb.beginTransaction();
30+
try {
31+
ContentValues values = new ContentValues();
32+
values.put(OneSignalDbContract.InAppMessageTable.COLUMN_NAME_MESSAGE_ID, inAppMessage.messageId);
33+
values.put(OneSignalDbContract.InAppMessageTable.COLUMN_NAME_DISPLAY_QUANTITY, inAppMessage.getRedisplayStats().getDisplayQuantity());
34+
values.put(OneSignalDbContract.InAppMessageTable.COLUMN_NAME_LAST_DISPLAY, inAppMessage.getRedisplayStats().getLastDisplayTime());
35+
values.put(OneSignalDbContract.InAppMessageTable.COLUMN_CLICK_IDS, inAppMessage.getClickedClickIds().toString());
36+
values.put(OneSignalDbContract.InAppMessageTable.COLUMN_DISPLAYED_IN_SESSION, inAppMessage.isDisplayedInSession());
37+
38+
int rowsUpdated = writableDb.update(OneSignalDbContract.InAppMessageTable.TABLE_NAME, values,
39+
OneSignalDbContract.InAppMessageTable.COLUMN_NAME_MESSAGE_ID + " = ?", new String[]{inAppMessage.messageId});
40+
if (rowsUpdated == 0)
41+
writableDb.insert(OneSignalDbContract.InAppMessageTable.TABLE_NAME, null, values);
42+
43+
writableDb.setTransactionSuccessful();
44+
} finally {
45+
try {
46+
writableDb.endTransaction(); // May throw if transaction was never opened or DB is full.
47+
} catch (SQLException e) {
48+
OneSignal.Log(OneSignal.LOG_LEVEL.ERROR, "Error closing transaction! ", e);
49+
}
50+
}
3851
}
3952

4053
@WorkerThread
@@ -78,4 +91,147 @@ synchronized List<OSInAppMessage> getCachedInAppMessages() {
7891
return iams;
7992
}
8093

94+
@WorkerThread
95+
synchronized void cleanCachedInAppMessages() {
96+
SQLiteDatabase writableDb = dbHelper.getSQLiteDatabaseWithRetries();
97+
98+
// 1. Query for all old message ids and old clicked click ids
99+
String[] retColumns = new String[]{
100+
OneSignalDbContract.InAppMessageTable.COLUMN_NAME_MESSAGE_ID,
101+
OneSignalDbContract.InAppMessageTable.COLUMN_CLICK_IDS
102+
};
103+
104+
String whereStr = OneSignalDbContract.InAppMessageTable.COLUMN_NAME_LAST_DISPLAY + " < ?";
105+
106+
String sixMonthsAgoInSeconds = String.valueOf((System.currentTimeMillis() / 1_000L) - IAM_CACHE_DATA_LIFETIME);
107+
String[] whereArgs = new String[]{sixMonthsAgoInSeconds};
108+
109+
Set<String> oldMessageIds = OSUtils.newConcurrentSet();
110+
Set<String> oldClickedClickIds = OSUtils.newConcurrentSet();
111+
112+
Cursor cursor = null;
113+
try {
114+
cursor = writableDb.query(OneSignalDbContract.InAppMessageTable.TABLE_NAME,
115+
retColumns,
116+
whereStr,
117+
whereArgs,
118+
null,
119+
null,
120+
null);
121+
122+
if (cursor == null || cursor.getCount() == 0) {
123+
OneSignal.onesignalLog(OneSignal.LOG_LEVEL.DEBUG, "Attempted to clean 6 month old IAM data, but none exists!");
124+
return;
125+
}
126+
127+
// From cursor get all of the old message ids and old clicked click ids
128+
if (cursor.moveToFirst()) {
129+
do {
130+
String oldMessageId = cursor.getString(
131+
cursor.getColumnIndex(
132+
OneSignalDbContract.InAppMessageTable.COLUMN_NAME_MESSAGE_ID));
133+
String oldClickIds = cursor.getString(
134+
cursor.getColumnIndex(
135+
OneSignalDbContract.InAppMessageTable.COLUMN_CLICK_IDS));
136+
137+
oldMessageIds.add(oldMessageId);
138+
oldClickedClickIds.addAll(OSUtils.newStringSetFromJSONArray(new JSONArray(oldClickIds)));
139+
} while (cursor.moveToNext());
140+
}
141+
} catch (JSONException e) {
142+
e.printStackTrace();
143+
} finally {
144+
if (cursor != null && !cursor.isClosed())
145+
cursor.close();
146+
}
147+
148+
writableDb.beginTransaction();
149+
try {
150+
// 2. Delete old IAMs from SQL
151+
writableDb.delete(
152+
OneSignalDbContract.InAppMessageTable.TABLE_NAME,
153+
whereStr,
154+
whereArgs);
155+
writableDb.setTransactionSuccessful();
156+
} finally {
157+
try {
158+
writableDb.endTransaction(); // May throw if transaction was never opened or DB is full.
159+
} catch (SQLException e) {
160+
OneSignal.Log(OneSignal.LOG_LEVEL.ERROR, "Error closing transaction! ", e);
161+
}
162+
}
163+
164+
// 3. Use queried data to clean SharedPreferences
165+
cleanInAppMessageIds(oldMessageIds);
166+
cleanInAppMessageClickedClickIds(oldClickedClickIds);
167+
}
168+
169+
/**
170+
* Clean up 6 month old IAM ids in {@link android.content.SharedPreferences}:
171+
* 1. Dismissed message ids
172+
* 2. Impressioned message ids
173+
* <br/><br/>
174+
* Note: This should only ever be called by {@link OSInAppMessageRepository#cleanCachedInAppMessages()}
175+
* <br/><br/>
176+
* @see OneSignalCacheCleaner#cleanCachedInAppMessages(OneSignalDbHelper)
177+
* @see OSInAppMessageRepository#cleanCachedInAppMessages()
178+
*/
179+
private void cleanInAppMessageIds(Set<String> oldMessageIds) {
180+
if (oldMessageIds != null && oldMessageIds.size() > 0) {
181+
Set<String> dismissedMessages = OneSignalPrefs.getStringSet(
182+
OneSignalPrefs.PREFS_ONESIGNAL,
183+
OneSignalPrefs.PREFS_OS_DISMISSED_IAMS,
184+
null);
185+
186+
Set<String> impressionedMessages = OneSignalPrefs.getStringSet(
187+
OneSignalPrefs.PREFS_ONESIGNAL,
188+
OneSignalPrefs.PREFS_OS_IMPRESSIONED_IAMS,
189+
null);
190+
191+
if (dismissedMessages != null && dismissedMessages.size() > 0) {
192+
dismissedMessages.removeAll(oldMessageIds);
193+
OneSignalPrefs.saveStringSet(
194+
OneSignalPrefs.PREFS_ONESIGNAL,
195+
OneSignalPrefs.PREFS_OS_DISMISSED_IAMS,
196+
dismissedMessages);
197+
}
198+
199+
if (impressionedMessages != null && impressionedMessages.size() > 0) {
200+
impressionedMessages.removeAll(oldMessageIds);
201+
OneSignalPrefs.saveStringSet(
202+
OneSignalPrefs.PREFS_ONESIGNAL,
203+
OneSignalPrefs.PREFS_OS_IMPRESSIONED_IAMS,
204+
impressionedMessages);
205+
}
206+
}
207+
}
208+
209+
/**
210+
* Clean up 6 month old IAM clicked click ids in {@link android.content.SharedPreferences}:
211+
* 1. Clicked click ids from elements within IAM
212+
* <br/><br/>
213+
* Note: This should only ever be called by {@link OSInAppMessageRepository#cleanCachedInAppMessages()}
214+
* <br/><br/>
215+
* @see OneSignalCacheCleaner#cleanCachedInAppMessages(OneSignalDbHelper)
216+
* @see OSInAppMessageRepository#cleanCachedInAppMessages()
217+
*/
218+
private void cleanInAppMessageClickedClickIds(Set<String> oldClickedClickIds) {
219+
if (oldClickedClickIds != null && oldClickedClickIds.size() > 0) {
220+
Set<String> clickedClickIds = OneSignalPrefs.getStringSet(
221+
OneSignalPrefs.PREFS_ONESIGNAL,
222+
OneSignalPrefs.PREFS_OS_CLICKED_CLICK_IDS_IAMS,
223+
null);
224+
225+
if (clickedClickIds != null && clickedClickIds.size() > 0) {
226+
clickedClickIds.removeAll(oldClickedClickIds);
227+
OneSignalPrefs.saveStringSet(
228+
OneSignalPrefs.PREFS_ONESIGNAL,
229+
OneSignalPrefs.PREFS_OS_CLICKED_CLICK_IDS_IAMS,
230+
clickedClickIds);
231+
}
232+
}
233+
}
234+
235+
236+
81237
}

0 commit comments

Comments
 (0)