Outcome callback cleanup (#871)

* Changing outcome callbacks and removing cache settings
* Cache settings was removed as a decision to prevent users from turning off cache (will need to find a good way to clean cached notifications/outcomes in future though)
* Changed callback functionality so there is no failure now (failure could give users the ability to reattempt sending an outcome which we do already through caching and new init)

* Fixed example app text view for debugging
* Also cleaned unique notification id for-loops so only one is needed

* Previously changed/removed getParams
* Added getParams back and make sure it returns a String

* Added new outcome callback and cache cleaning
* Outcome callback should just return the entire OutcomeEvent instead of a name
* Also removed the failure and only allow success since we don't want people reattempting sending unique outcomes when they will be reattempted by us anyway (could result in duplicate outcome events failling and being sent later)
* Added SQL caching for the unique outcome events with notifications (ATTRIBUTED) and cleaning so they are wiped away when the cached notifications are wiped away
* Added SharedPrefs caching for UNATTRIBUTED unique outcome events that simply wipe and start over on enw sessions only

* Outcome event clean up (#873)

* Outcome event clean up
* Removed OutcomeParams class and instead only use a float weight
* Fixed all tests that broke after removing OutcomeParams
* Cleaned up requests in OutcomeEventsRepository so now there is only 1 of each session (DIRECT, INDIRECT, UNATTRIBUTED)
* Reordered expected outcome unit tests because of outcome event changes to JSON creation
* Added checking in two places for weight of 0 being sent with an OutcomeEvent, one is when we call the public method (logs an error) and the other is in the toJSON of the OutcomeEvent preventing a weight of 0 from being added into the JSON

* Minor clean up after PR
* Change equals check on weight to Float in OutcomeEvent equals override
* Add '||' check for valid outcome name and value instead of '&&'

* Updated example app for new outcomes callback logic

* Made CachedUniqueOutcomeNotification package-private
* Fixed all unit tests by adding CachedUniqueOutcomeNotification to OneSignalPackagePrivateHelper
* Made OutcomeEvent toJsonObject public for access in wrappers

* Removed TODO for clean up in OutcomeEventsRepository

Author: mikechoch

OneSignalSDK/app/build.gradle

@@ -8,15 +8,15 @@ apply plugin: 'com.android.application'
 //apply plugin: com.onesignal.androidsdk.GradleProjectPlugin
 
 android {
-    compileSdkVersion 27
-    buildToolsVersion '27.0.3'
+    compileSdkVersion 28
+    buildToolsVersion '28.0.3'
     defaultConfig {
         applicationId "com.onesignal.example"
         manifestPlaceholders = [onesignal_app_id: "b2f7f966-d8cc-11e4-bed1-df8f05be55ba",
                                 // Project number pulled from dashboard, local value is ignored.
                                 onesignal_google_project_number: "REMOTE"]
         minSdkVersion 15
-        targetSdkVersion 27
+        targetSdkVersion 28
         versionCode 1
         versionName "1.0"
     }
@@ -39,7 +39,7 @@ repositories {
 
 dependencies {
     implementation fileTree(include: ['*.jar'], dir: 'libs')
-    implementation 'com.android.support:appcompat-v7:27.1.0'
+    implementation 'com.android.support:appcompat-v7:28.0.0'
 
     // Use for SDK Development
     implementation(project(':onesignal')) {
@@ -64,10 +64,10 @@ dependencies {
     // Old Instructions - Use for released SDK
     // compile 'com.onesignal:OneSignal:3.+@aar'
 
-    implementation 'com.google.android.gms:play-services-location:12.0.0'
+    implementation 'com.google.android.gms:play-services-location:16.0.0'
 
     // For Chrome tabs
-    implementation 'com.android.support:customtabs:27.1.0'
+    implementation 'com.android.support:customtabs:28.0.0'
 }
 
 //apply plugin: 'com.google.gms.google-services'

OneSignalSDK/app/src/main/java/com/onesignal/MainActivity.java

@@ -330,27 +330,17 @@ public void onFailure(OneSignal.EmailUpdateError error) {
    public void onSendOutcomeClicked(View view) {
       OneSignal.sendOutcome(outcomeName.getText().toString(), new OneSignal.OutcomeCallback() {
          @Override
-         public void onOutcomeSuccess(String name) {
-            updateTextView(name + " Outcome sent successfully");
-         }
-
-         @Override
-         public void onOutcomeFail(int statusCode, String response) {
-            updateTextView("Outcome fail with status code: " + statusCode);
+         public void onSuccess(OutcomeEvent outcomeEvent) {
+            updateTextView(outcomeEvent.toString());
          }
       });
    }
 
    public void onSendUniqueOutcomeClicked(View view) {
       OneSignal.sendUniqueOutcome(outcomeUnique.getText().toString(), new OneSignal.OutcomeCallback() {
          @Override
-         public void onOutcomeSuccess(String name) {
-            updateTextView(name + " Unique Outcome sent successfully");
-         }
-
-         @Override
-         public void onOutcomeFail(int statusCode, String response) {
-            updateTextView("Unique Outcome fail with status code: " + statusCode);
+         public void onSuccess(OutcomeEvent outcomeEvent) {
+            updateTextView(outcomeEvent.toString());
          }
       });
    }
@@ -361,13 +351,8 @@ public void onSendOutcomeWithValueClicked(View view) {
 
       OneSignal.sendOutcomeWithValue(outcomeValueName.getText().toString(), Float.parseFloat(outcomeValue.getText().toString()), new OneSignal.OutcomeCallback() {
          @Override
-         public void onOutcomeSuccess(String name) {
-            updateTextView(name + " Outcome sent with value successfully");
-         }
-
-         @Override
-         public void onOutcomeFail(int statusCode, String response) {
-            updateTextView("Outcome with value fail with status code: " + statusCode);
+         public void onSuccess(OutcomeEvent outcomeEvent) {
+            updateTextView(outcomeEvent.toString());
          }
       });
    }

OneSignalSDK/app/src/main/res/layout/activity_main.xml

@@ -243,7 +243,7 @@
         <TextView
             android:id="@+id/debugTextView"
             android:layout_width="match_parent"
-            android:layout_height="80dp" />
+            android:layout_height="wrap_content" />
 
     </LinearLayout>
 

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

@@ -0,0 +1,21 @@
+package com.onesignal;
+
+class CachedUniqueOutcomeNotification {
+
+    private String notificationId;
+    private String name;
+
+    CachedUniqueOutcomeNotification(String notificationId, String name) {
+        this.notificationId = notificationId;
+        this.name = name;
+    }
+
+    String getNotificationId() {
+        return notificationId;
+    }
+
+    String getName() {
+        return name;
+    }
+
+}

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

@@ -178,8 +178,6 @@ private static void saveNotification(NotificationGenerationJob notifiJob, boolea
 
             writableDb.beginTransaction();
 
-            deleteOldNotifications(writableDb);
-            
             // Count any notifications with duplicated android notification ids as dismissed.
             // -1 is used to note never displayed
             if (notifiJob.isNotificationToDisplay()) {
@@ -272,13 +270,6 @@ static void markRestoredNotificationAsDismissed(NotificationGenerationJob notifi
       }
    }
 
-   // Clean up old records after 1 week.
-   static void deleteOldNotifications(SQLiteDatabase writableDb) {
-      writableDb.delete(NotificationTable.TABLE_NAME,
-          NotificationTable.COLUMN_NAME_CREATED_TIME + " < " + ((System.currentTimeMillis() / 1_000L) - 604_800L),
-          null);
-   }
-
    static @NonNull JSONObject bundleAsJSONObject(Bundle bundle) {
       JSONObject json = new JSONObject();
       Set<String> keys = bundle.keySet();

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

@@ -122,35 +122,13 @@ public static void restore(Context context) {
       OneSignal.Log(OneSignal.LOG_LEVEL.INFO, "Restoring notifications");
 
       OneSignalDbHelper dbHelper = OneSignalDbHelper.getInstance(context);
-      deleteOldNotificationsFromDb(dbHelper);
 
       StringBuilder dbQuerySelection = OneSignalDbHelper.recentUninteractedWithNotificationsWhere();
       skipVisibleNotifications(context, dbQuerySelection);
 
       queryAndRestoreNotificationsAndBadgeCount(context, dbHelper, dbQuerySelection);
    }
 
-   private static void deleteOldNotificationsFromDb(OneSignalDbHelper dbHelper) {
-      SQLiteDatabase writableDb = null;
-
-      try {
-         writableDb = dbHelper.getWritableDbWithRetries();
-         writableDb.beginTransaction();
-         NotificationBundleProcessor.deleteOldNotifications(writableDb);
-         writableDb.setTransactionSuccessful();
-      } catch (Throwable t) {
-         OneSignal.Log(OneSignal.LOG_LEVEL.ERROR, "Error deleting old notification records! ", 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);
-            }
-         }
-      }
-   }
-
    private static void queryAndRestoreNotificationsAndBadgeCount(
       Context context,
       OneSignalDbHelper dbHelper,

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

@@ -595,10 +595,12 @@ public static void setAppContext(@NonNull Context context) {
 
       if (wasAppContextNull) {
          sessionManager = new OSSessionManager(getNewSessionListener());
-         outcomeEventsController = new OutcomeEventsController(sessionManager, OneSignalDbHelper.getInstance(appContext), outcomeSettings);
+         outcomeEventsController = new OutcomeEventsController(sessionManager, OneSignalDbHelper.getInstance(appContext));
          // Prefs require a context to save
          // If the previous state of appContext was null, kick off write in-case it was waiting
          OneSignalPrefs.startDelayedWrite();
+         // Cleans out old cached data to prevent over using the storage on devices
+         OneSignalCacheCleaner.cleanOldCachedData(context);
       }
    }
 
@@ -3077,13 +3079,6 @@ static void fireEmailUpdateFailure() {
    /*
     * Start OneSignalOutcome module
     */
-   private static OutcomeSettings outcomeSettings = null;
-
-   static void changeOutcomeSettings(OutcomeSettings settings) {
-      outcomeSettings = settings;
-      outcomeEventsController.setOutcomeSettings(settings);
-   }
-
    static OSSessionManager getSessionManager() {
       return sessionManager;
    }
@@ -3096,6 +3091,11 @@ public static void sendOutcome(@NonNull String name, OutcomeCallback callback) {
       if (!isValidOutcomeEntry(name))
          return;
 
+      if (outcomeEventsController == null) {
+         OneSignal.Log(LOG_LEVEL.ERROR, "Make sure OneSignal.init is called first");
+         return;
+      }
+
       outcomeEventsController.sendOutcomeEvent(name, callback);
    }
 
@@ -3107,6 +3107,11 @@ public static void sendUniqueOutcome(@NonNull String name, OutcomeCallback callb
       if (!isValidOutcomeEntry(name))
          return;
 
+      if (outcomeEventsController == null) {
+         OneSignal.Log(LOG_LEVEL.ERROR, "Make sure OneSignal.init is called first");
+         return;
+      }
+
       outcomeEventsController.sendUniqueOutcomeEvent(name, callback);
    }
 
@@ -3115,18 +3120,27 @@ public static void sendOutcomeWithValue(@NonNull String name, float value) {
    }
 
    public static void sendOutcomeWithValue(@NonNull String name, float value, OutcomeCallback callback) {
-      if (!isValidOutcomeEntry(name))
+      if (!isValidOutcomeEntry(name) || !isValidOutcomeValue(value))
+         return;
+
+      if (outcomeEventsController == null) {
+         OneSignal.Log(LOG_LEVEL.ERROR, "Make sure OneSignal.init is called first");
          return;
+      }
 
       outcomeEventsController.sendOutcomeEventWithValue(name, value, callback);
    }
 
-   private static boolean isValidOutcomeEntry(String name) {
-      if (outcomeEventsController == null) {
-          OneSignal.Log(LOG_LEVEL.ERROR, "Make sure OneSignal.init is called first");
-          return false;
+   private static boolean isValidOutcomeValue(float value) {
+      if (value <= 0) {
+         OneSignal.Log(LOG_LEVEL.ERROR, "Outcome value must be greater than 0");
+         return false;
       }
 
+      return true;
+   }
+
+   private static boolean isValidOutcomeEntry(String name) {
       if (name == null || name.isEmpty()) {
          OneSignal.Log(LOG_LEVEL.ERROR, "Outcome name must not be empty");
          return false;
@@ -3136,41 +3150,7 @@ private static boolean isValidOutcomeEntry(String name) {
    }
 
    public interface OutcomeCallback {
-      void onOutcomeSuccess(String name);
-      void onOutcomeFail(int statusCode, String response);
-   }
-
-   static class OutcomeSettings {
-      private boolean cacheActive;
-
-      OutcomeSettings(Builder builder) {
-         this.cacheActive = builder.cacheActive;
-      }
-
-      boolean isCacheActive() {
-         return cacheActive;
-      }
-
-      public static class Builder {
-
-         private boolean cacheActive = true;
-
-         public static Builder newInstance() {
-            return new Builder();
-         }
-
-         private Builder() {
-         }
-
-         public Builder setCacheActive(boolean active) {
-            this.cacheActive = active;
-            return this;
-         }
-
-         public OutcomeSettings build() {
-            return new OutcomeSettings(this);
-         }
-      }
+      void onSuccess(OutcomeEvent outcomeEvent);
    }
    /*
     * End OneSignalOutcome module

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

@@ -0,0 +1,72 @@
+package com.onesignal;
+
+import com.onesignal.OneSignalDbContract.NotificationTable;
+import com.onesignal.OneSignalDbContract.CachedUniqueOutcomeNotificationTable;
+
+import android.content.Context;
+import android.database.sqlite.SQLiteDatabase;
+import android.os.Process;
+
+class OneSignalCacheCleaner {
+
+    private static String OS_DELETE_OLD_CACHED_DATA = "OS_DELETE_OLD_CACHED_DATA";
+
+    /**
+     * We clean outdated cache from several places within the OneSignal SDK here
+     * 1. In App Messaging id sets (impressions, clicks, views)
+     * 2. Notifications after 1 week
+     * 3. Unique outcome events linked to notification ids (1 week)
+     */
+    synchronized static void cleanOldCachedData(final Context context) {
+        new Thread(new Runnable() {
+            @Override
+            public void run() {
+                Thread.currentThread().setPriority(Process.THREAD_PRIORITY_BACKGROUND);
+                OneSignalDbHelper dbHelper = OneSignalDbHelper.getInstance(context);
+                SQLiteDatabase writableDb = dbHelper.getWritableDbWithRetries();
+
+                cleanInAppMessagingCache();
+                cleanNotificationCache(writableDb);
+            }
+        }, OS_DELETE_OLD_CACHED_DATA).start();
+    }
+
+    /**
+     * TODO: Needs to be implemented to clean out old IAM data used to track impressions, clicks, and viewed IAMs
+     */
+    static void cleanInAppMessagingCache() {
+        // NOTE: Currently IAMs will pile up overtime and since IAMs can be modified, active, inactive, etc.
+        //  we never truly know when it is the correct time to remove these ids form our cache
+    }
+
+    /**
+     * Cleans two notification tables
+     * 1. NotificationTable.TABLE_NAME
+     * 2. CachedUniqueOutcomeNotificationTable.TABLE_NAME
+     */
+    static void cleanNotificationCache(SQLiteDatabase writableDb) {
+        cleanOldNotificationData(writableDb);
+        cleanOldUniqueOutcomeEventNotificationsCache(writableDb);
+    }
+
+    /**
+     * Deletes any notifications with created timestamps older than 7 days
+     */
+    private static void cleanOldNotificationData(SQLiteDatabase writableDb) {
+        writableDb.delete(NotificationTable.TABLE_NAME,
+                NotificationTable.COLUMN_NAME_CREATED_TIME + " < " + ((System.currentTimeMillis() / 1_000L) - 604_800L),
+                null);
+    }
+
+    /**
+     * Deletes any notifications whose ids do not exist inside of the NotificationTable.TABLE_NAME
+     */
+    static void cleanOldUniqueOutcomeEventNotificationsCache(SQLiteDatabase writableDb) {
+        writableDb.delete(CachedUniqueOutcomeNotificationTable.TABLE_NAME,
+                "NOT EXISTS(SELECT NULL FROM " + NotificationTable.TABLE_NAME +
+                        " n WHERE" +
+                        " n." + NotificationTable.COLUMN_NAME_NOTIFICATION_ID  + " = " + CachedUniqueOutcomeNotificationTable.COLUMN_NAME_NOTIFICATION_ID + ")",
+                null);
+    }
+
+}