Remove IAM from db

  * Remove IAMs with redisplay from DB when lastDisplay is
    older than six month

Author: Jeasmine

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

@@ -30,6 +30,7 @@ class OSInAppMessageController implements OSDynamicTriggerControllerObserver, OS
 
     public static final String IN_APP_MESSAGES_JSON_KEY = "in_app_messages";
     private static final String OS_SAVE_IN_APP_MESSAGE = "OS_SAVE_IN_APP_MESSAGE";
+    private static final String OS_DELETE_IN_APP_MESSAGE = "OS_DELETE_IN_APP_MESSAGE";
 
     OSTriggerController triggerController;
     private OSSystemConditionController systemConditionController;
@@ -49,7 +50,8 @@ class OSInAppMessageController implements OSDynamicTriggerControllerObserver, OS
     // Ordered IAMs queued to display, includes the message currently displaying, if any.
     @NonNull final ArrayList<OSInAppMessage> messageDisplayQueue;
     // IAMs displayed with last displayed time and quantity of displays data
-    @NonNull private List<OSInAppMessage> displayedMessagesData;
+    // This is retrieved from a DB Table that take care of each object to be unique
+    @NonNull private List<OSInAppMessage> redisplayedInAppMessages;
 
     boolean inAppMessagingEnabled = true;
     boolean inAppMessageShowing = false;
@@ -110,9 +112,9 @@ protected OSInAppMessageController(OneSignalDbHelper dbInstance) {
 
     void initRedisplayData(OneSignalDbHelper dbInstance) {
         inAppMessageRepository = new OSInAppMessageRepository(dbInstance);
-        displayedMessagesData = inAppMessageRepository.getAllInAppMessages();
+        redisplayedInAppMessages = inAppMessageRepository.getRedisplayedInAppMessages();
 
-        OneSignal.Log(OneSignal.LOG_LEVEL.DEBUG, "displayedMessagesData: " + displayedMessagesData.toString());
+        OneSignal.Log(OneSignal.LOG_LEVEL.DEBUG, "redisplayedInAppMessages: " + redisplayedInAppMessages.toString());
     }
 
     // Normally we wait until on_session call to download the latest IAMs
@@ -151,6 +153,15 @@ void receivedInAppMessageJson(@NonNull JSONArray json) throws JSONException {
                 json.toString());
 
         processInAppMessageJson(json);
+
+        // Remove cache redisplayed messages
+        new Thread(new Runnable() {
+            @Override
+            public void run() {
+                Thread.currentThread().setPriority(Process.THREAD_PRIORITY_BACKGROUND);
+                inAppMessageRepository.deleteOldRedisplayedInAppMessages();
+            }
+        }, OS_DELETE_IN_APP_MESSAGE).start();
     }
 
     private void processInAppMessageJson(@NonNull JSONArray json) throws JSONException {
@@ -291,13 +302,15 @@ private void fireRESTCallForClick(@NonNull final OSInAppMessage message, @NonNul
             return;
 
         final String clickId = action.clickId;
+        // If IAM has redisplay the clickId may be available
         boolean clickAvailableByRedisplay = message.getDisplayStats().isRedisplayEnabled() && message.isClickAvailable(clickId);
 
         // Never count multiple clicks for the same click UUID unless that click is from an IAM with redisplay
         if (!clickAvailableByRedisplay && clickedClickIds.contains(clickId))
             return;
 
         clickedClickIds.add(clickId);
+        // Track clickId per IAM
         message.addClickId(clickId);
 
         try {
@@ -338,26 +351,26 @@ void onFailure(int statusCode, String response, Throwable throwable) {
     /**
      * Part of redisplay logic
      *
-     * In order to an IAM to be re display, the following conditions need to be satisfied
-     *  - IAM has redisplay property
-     *  - Gap between displays is satisfied
-     *  - Quantity of displays is available
-     *  - Some IAM Trigger was fired
+     * In order to redisplay an IAM, the following conditions must be satisfied:
+     *     1. IAM has redisplay property
+     *     2. Time delay between redisplay satisfied
+     *     3. Has more redisplays
+     *     4. An IAM trigger was satisfied
      *
-     * For re display, the message need to be removed from the arrays that track the display/impression
+     * For redisplay, the message need to be removed from the arrays that track the display/impression
      * For click counting, every message has it click id array
      * */
     private void setDataForRedisplay(OSInAppMessage message) {
         if (!message.getDisplayStats().isRedisplayEnabled())
             return;
 
         boolean messageDismissed = dismissedMessages.contains(message.messageId);
-        int index = displayedMessagesData.indexOf(message);
+        int index = redisplayedInAppMessages.indexOf(message);
 
         if (messageDismissed && index != -1) {
             OneSignal.onesignalLog(OneSignal.LOG_LEVEL.DEBUG, "setDataForRedisplay: " + message.messageId);
 
-            OSInAppMessage savedIAM = displayedMessagesData.get(index);
+            OSInAppMessage savedIAM = redisplayedInAppMessages.get(index);
             message.getDisplayStats().setDisplayStats(savedIAM.getDisplayStats());
 
             // Check if conditions are correct for redisplay
@@ -431,7 +444,7 @@ void messageWasDismissed(@NonNull OSInAppMessage message) {
 
             // Don't keep track of last displayed time for a preview
             lastTimeInAppDismissed = new Date();
-            persisIAMessageForRedisplay(message);
+            persistInAppMessageForRedisplay(message);
         }
 
         dismissCurrentMessage();
@@ -456,7 +469,7 @@ private void dismissCurrentMessage() {
         }
     }
 
-    private void persisIAMessageForRedisplay(final OSInAppMessage message) {
+    private void persistInAppMessageForRedisplay(final OSInAppMessage message) {
         //If the IAM doesn't have the re display configuration then no need to save it
         if (!message.getDisplayStats().isRedisplayEnabled())
             return;
@@ -474,16 +487,16 @@ public void run() {
             }
         }, OS_SAVE_IN_APP_MESSAGE).start();
 
-        //Update the data to enable future re displays
-        //Avoid calling the repository data again
-        int index = displayedMessagesData.indexOf(message);
+        // Update the data to enable future re displays
+        // Avoid calling the repository data again
+        int index = redisplayedInAppMessages.indexOf(message);
         if (index != -1) {
-            displayedMessagesData.set(index, message);
+            redisplayedInAppMessages.set(index, message);
         } else {
-            displayedMessagesData.add(message);
+            redisplayedInAppMessages.add(message);
         }
 
-        OneSignal.onesignalLog(OneSignal.LOG_LEVEL.DEBUG, "persisIAMessageForRedisplay: " + message.toString() + " with msg array data: " + displayedMessagesData.toString());
+        OneSignal.onesignalLog(OneSignal.LOG_LEVEL.DEBUG, "persistInAppMessageForRedisplay: " + message.toString() + " with msg array data: " + redisplayedInAppMessages.toString());
     }
 
     private static @Nullable String htmlPathForMessage(OSInAppMessage message) {
@@ -589,36 +602,36 @@ public void messageTriggerConditionChanged() {
     }
 
     /**
-     * Part of redisplay logic
+     *  Part of redisplay logic
      *
-     * Make all messages with redisplay enable available for redisplay if:
+     *  Make all messages with redisplay available if:
      *   - Already displayed
-     *   - Trigger changed
-     * */
-    private void checkTriggerChanged(Collection<String> newTriggersKeys) {
+     *   - At least one Trigger has changed
+     */
+    private void makeRedisplayMessagesAvailableWithTriggers(Collection<String> newTriggersKeys) {
         for (OSInAppMessage message : messages) {
-            if (displayedMessagesData.contains(message) &&
+            if (redisplayedInAppMessages.contains(message) &&
                     triggerController.isTriggerOnMessage(message, newTriggersKeys)) {
                 message.setTriggerChanged(true);
             }
         }
     }
 
     /**
-     * Trigger logic
-     * These methods mostly pass data to the Trigger Controller, but also cause the SDK to
+     *  Trigger logic
+     *  These methods mostly pass data to the Trigger Controller, but also cause the SDK to
      *  re-evaluate messages to see if we should display a message now that the trigger
      *  conditions have changed.
      */
     void addTriggers(Map<String, Object> newTriggers) {
         triggerController.addTriggers(newTriggers);
-        checkTriggerChanged(newTriggers.keySet());
+        makeRedisplayMessagesAvailableWithTriggers(newTriggers.keySet());
         evaluateInAppMessages();
     }
 
     void removeTriggersForKeys(Collection<String> keys) {
         triggerController.removeTriggersForKeys(keys);
-        checkTriggerChanged(keys);
+        makeRedisplayMessagesAvailableWithTriggers(keys);
         evaluateInAppMessages();
     }
 

OneSignalSDK/onesignal/src/main/java/com/onesignal/OSInAppMessageDisplayStats.java

@@ -87,14 +87,11 @@ boolean shouldDisplayAgain() {
     }
 
     boolean isDelayTimeSatisfied() {
-        long currentTimeInSeconds = System.currentTimeMillis() / 1000;
-
         if (lastDisplayTime < 0) {
-            lastDisplayTime = currentTimeInSeconds;
             return true;
         }
-
-        //Calculate gap between display times
+        long currentTimeInSeconds = System.currentTimeMillis() / 1000;
+        // Calculate gap between display times
         long diffInSeconds = currentTimeInSeconds - lastDisplayTime;
         OneSignal.Log(OneSignal.LOG_LEVEL.DEBUG, "OSInAppMessage lastDisplayTime: " + lastDisplayTime +
                 " currentTimeInSeconds: " + currentTimeInSeconds + " diffInSeconds: " + diffInSeconds + " displayDelay: " + displayDelay);

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

@@ -15,32 +15,26 @@
 
 class OSInAppMessageRepository {
 
+    // The max time we keep the IAM on the DB
+    // Currently value: Six months in seconds
+    private static final long OS_IAM_MAX_CACHE_TIME = 6 * 30 * 24 * 60 * 60;
     private final OneSignalDbHelper dbHelper;
 
     OSInAppMessageRepository(OneSignalDbHelper dbHelper) {
         this.dbHelper = dbHelper;
     }
 
+    /**
+     * Remove IAMs that the last display time was six month ago
+     */
     @WorkerThread
-    synchronized void deleteInAppMessage(String messageId) {
+    synchronized void deleteOldRedisplayedInAppMessages() {
+        long sixMonthsAgo = System.currentTimeMillis() / 1000 - OS_IAM_MAX_CACHE_TIME;
         SQLiteDatabase writableDb = dbHelper.getWritableDbWithRetries();
-
-        try {
-            writableDb.beginTransaction();
-            writableDb.delete(OneSignalDbContract.InAppMessageTable.TABLE_NAME,
-                    OneSignalDbContract.InAppMessageTable.COLUMN_NAME_MESSAGE_ID + " = ?", new String[]{messageId});
-            writableDb.setTransactionSuccessful();
-        } catch (Throwable t) {
-            OneSignal.Log(OneSignal.LOG_LEVEL.ERROR, "Error deleting in app message! ", t);
-        } finally {
-            if (writableDb != null) {
-                try {
-                    writableDb.endTransaction(); // May throw if transaction was never opened or DB is full.
-                } catch (Throwable t) {
-                    OneSignal.Log(OneSignal.LOG_LEVEL.ERROR, "Error closing transaction! ", t);
-                }
-            }
-        }
+        writableDb.delete(OneSignalDbContract.InAppMessageTable.TABLE_NAME,
+                OneSignalDbContract.InAppMessageTable.COLUMN_NAME_LAST_DISPLAY + "< ?",
+                new String[]{String.valueOf(sixMonthsAgo)});
+        writableDb.close();
     }
 
     @WorkerThread
@@ -61,7 +55,7 @@ synchronized void saveInAppMessage(OSInAppMessage inAppMessage) {
     }
 
     @WorkerThread
-    synchronized List<OSInAppMessage> getAllInAppMessages() {
+    synchronized List<OSInAppMessage> getRedisplayedInAppMessages() {
         List<OSInAppMessage> iams = new ArrayList<>();
         Cursor cursor = null;
 

OneSignalSDK/unittest/src/test/java/com/onesignal/OneSignalPackagePrivateHelper.java

@@ -457,22 +457,22 @@ public static ArrayList<com.onesignal.OSInAppMessage> getInAppMessageDisplayQueu
       return com.onesignal.OSInAppMessageController.getController().messageDisplayQueue;
    }
 
-   public static class OSInAppMessageController extends com.onesignal.OSInAppMessageController {
+   public static class OSInAppMessageController {
+      public static final String IN_APP_MESSAGES_JSON_KEY = com.onesignal.OSInAppMessageController.IN_APP_MESSAGES_JSON_KEY;
       private static OSInAppMessageController sharedInstance;
 
-      OSInAppMessageController(OneSignalDbHelper dbInstance) {
-         super((dbInstance));
+      OSInAppMessageController() {
       }
 
       public static OSInAppMessageController getController() {
          if (sharedInstance == null)
-            sharedInstance = new OSInAppMessageController(OneSignal.getDBHelperInstance());
+            sharedInstance = new OSInAppMessageController();
 
          return sharedInstance;
       }
 
       public void onMessageActionOccurredOnMessage(@NonNull final com.onesignal.OSInAppMessage message, @NonNull final JSONObject actionJson) {
-         super.onMessageActionOccurredOnMessage(message, actionJson);
+         com.onesignal.OSInAppMessageController.getController().onMessageActionOccurredOnMessage(message, actionJson);
       }
 
       public void onMessageWasShown(@NonNull com.onesignal.OSInAppMessage message) {

OneSignalSDK/unittest/src/test/java/com/test/onesignal/InAppMessageIntegrationTests.java

@@ -44,6 +44,7 @@
 import java.lang.reflect.Field;
 import java.util.ArrayList;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Set;
 import java.util.concurrent.Callable;
 import java.util.concurrent.TimeUnit;
@@ -76,8 +77,10 @@
 
 @RunWith(RobolectricTestRunner.class)
 public class InAppMessageIntegrationTests {
+
     private static final String IAM_CLICK_ID = "button_id_123";
     private static final String ONESIGNAL_APP_ID = "b2f7f966-d8cc-11e4-bed1-df8f05be55ba";
+    private static final long SIX_MONTHS_TIME_SECONDS = 6 * 30 * 24 * 60 * 60;
     private static final int LIMIT = 5;
     private static final int DELAY = 60;
 
@@ -849,6 +852,46 @@ public void testInAppMessageMultipleRedisplayReceivesClickId() throws Exception
         assertEquals(4, ShadowOneSignalRestClient.requests.size());
     }
 
+    @Test
+    public void testInAppMessageRedisplayCacheUpdate() throws Exception {
+        final long currentTimeInSeconds = System.currentTimeMillis() / 1000;
+
+        final OSTestInAppMessage inAppMessage = InAppMessagingHelpers.buildTestMessageWithSingleTriggerAndRedisplay(
+                OSTriggerKind.CUSTOM,"test_saved", OneSignalPackagePrivateHelper.OSTestTrigger.OSTriggerOperator.EQUAL_TO.toString(), 2, LIMIT, DELAY);
+
+        String firstID = inAppMessage.messageId + "_test";
+        inAppMessage.messageId = firstID;
+        inAppMessage.getDisplayStats().setLastDisplayTime(currentTimeInSeconds - SIX_MONTHS_TIME_SECONDS + 1);
+        TestHelpers.saveIAM(inAppMessage);
+
+        inAppMessage.getDisplayStats().setLastDisplayTime(currentTimeInSeconds - SIX_MONTHS_TIME_SECONDS - 1);
+        inAppMessage.messageId += "1";
+        TestHelpers.saveIAM(inAppMessage);
+
+        List<OSTestInAppMessage> savedInAppMessages = TestHelpers.getAllInAppMessages();
+
+        assertEquals(2, savedInAppMessages.size());
+
+        final OSTestInAppMessage message1 = InAppMessagingHelpers.buildTestMessageWithSingleTriggerAndRedisplay(
+                OSTriggerKind.CUSTOM,"test_1", OSTestTrigger.OSTriggerOperator.EQUAL_TO.toString(), 2, LIMIT, DELAY);
+        final OSTestInAppMessage message2 = InAppMessagingHelpers.buildTestMessageWithSingleTriggerAndRedisplay(
+                OSTriggerKind.CUSTOM,"test_2", OSTestTrigger.OSTriggerOperator.EQUAL_TO.toString(), 2, LIMIT, DELAY);
+
+        setMockRegistrationResponseWithMessages(new ArrayList<OSTestInAppMessage>() {{
+            add(message1);
+            add(message2);
+        }});
+
+        // Init OneSignal with IAM with redisplay
+        OneSignalInit();
+        threadAndTaskWait();
+
+        List<OSTestInAppMessage> savedInAppMessagesAfterInit = TestHelpers.getAllInAppMessages();
+        // Message with old display time should be removed
+        assertEquals(1, savedInAppMessagesAfterInit.size());
+        assertEquals(firstID, savedInAppMessagesAfterInit.get(0).messageId);
+    }
+
     @Test
     @Config(sdk = 18)
     public void testMessageNotShownForAndroidApi18Lower() throws Exception {

OneSignalSDK/unittest/src/test/java/com/test/onesignal/InAppMessagingUnitTests.java

@@ -198,14 +198,13 @@ public void testBuiltMessageRedisplayLimit() throws JSONException {
 
     @Test
     public void testBuiltMessageRedisplayDelay() throws JSONException {
-        final long currentTimeInSeconds = new Date().getTime() / 1000;
-
         OSTestInAppMessage message = InAppMessagingHelpers.buildTestMessageWitRedisplay(
                 LIMIT,
                 DELAY
         );
 
         assertTrue(message.getDisplayStats().isDelayTimeSatisfied());
+        final long currentTimeInSeconds = System.currentTimeMillis() / 1000;
 
         message.getDisplayStats().setLastDisplayTime(currentTimeInSeconds - DELAY);
         assertTrue(message.getDisplayStats().isDelayTimeSatisfied());

OneSignalSDK/unittest/src/test/java/com/test/onesignal/MainOneSignalClassRunner.java

@@ -247,7 +247,6 @@ public static void afterEverything() throws Exception {
       cleanUp();
    }
 
-
    @Test
    public void testInitFromApplicationContext() throws Exception {
       // Application.onCreate