Fixed missing weight column if 3.12.0 was run

* Fixed missing weight column if version 3.12.0 was installed
* To fix added another db version with an upgrade specifically from v5 to v6

Author: jkasten2

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

@@ -42,7 +42,7 @@
 import java.util.List;
 
 public class OneSignalDbHelper extends SQLiteOpenHelper {
-   static final int DATABASE_VERSION = 5;
+   static final int DATABASE_VERSION = 6;
    private static final String DATABASE_NAME = "OneSignal.db";
 
    private static final String INTEGER_PRIMARY_KEY_TYPE = " INTEGER PRIMARY KEY";
@@ -79,7 +79,8 @@ public class OneSignalDbHelper extends SQLiteOpenHelper {
                    OutcomeEventsTable.COLUMN_NAME_NOTIFICATION_IDS + TEXT_TYPE + COMMA_SEP +
                    OutcomeEventsTable.COLUMN_NAME_NAME + TEXT_TYPE + COMMA_SEP +
                    OutcomeEventsTable.COLUMN_NAME_TIMESTAMP + TIMESTAMP_TYPE + COMMA_SEP +
-                   OutcomeEventsTable.COLUMN_NAME_WEIGHT + FLOAT_TYPE +
+                   // "params TEXT" Added in v4, removed in v5.
+                   OutcomeEventsTable.COLUMN_NAME_WEIGHT + FLOAT_TYPE + // New in v5, missing migration added in v6
                    ");";
 
    private static final String SQL_CREATE_UNIQUE_OUTCOME_NOTIFICATION_ENTRIES =
@@ -166,20 +167,24 @@ public void onUpgrade(SQLiteDatabase db, int oldVersion, int newVersion) {
 
    private static void internalOnUpgrade(SQLiteDatabase db, int oldVersion) {
       if (oldVersion < 2)
-         upgradeFromV1ToV2(db);
+         upgradeToV2(db);
 
       if (oldVersion < 3)
-         upgradeFromV2ToV3(db);
+         upgradeToV3(db);
 
       if (oldVersion < 4)
           upgradeToV4(db);
 
       if (oldVersion < 5)
          upgradeToV5(db);
+
+      // Specifically running only when going from 5 to 6+ is intentional
+      if (oldVersion == 5)
+         upgradeFromV5ToV6(db);
    }
 
    // Add collapse_id field and index
-   private static void upgradeFromV1ToV2(SQLiteDatabase db) {
+   private static void upgradeToV2(SQLiteDatabase db) {
       safeExecSQL(db,
          "ALTER TABLE " + NotificationTable.TABLE_NAME + " " +
             "ADD COLUMN " + NotificationTable.COLUMN_NAME_COLLAPSE_ID + TEXT_TYPE + ";"
@@ -189,7 +194,7 @@ private static void upgradeFromV1ToV2(SQLiteDatabase db) {
 
    // Add expire_time field and index.
    // Also backfills expire_time to create_time + 72 hours
-   private static void upgradeFromV2ToV3(SQLiteDatabase db) {
+   private static void upgradeToV3(SQLiteDatabase db) {
       safeExecSQL(db,
          "ALTER TABLE " + NotificationTable.TABLE_NAME + " " +
             "ADD COLUMN " + NotificationTable.COLUMN_NAME_EXPIRE_TIME + " TIMESTAMP" + ";"
@@ -209,7 +214,43 @@ private static void upgradeToV4(SQLiteDatabase db) {
    }
 
    private static void upgradeToV5(SQLiteDatabase db) {
+      // Added for 3.12.1
       safeExecSQL(db, SQL_CREATE_UNIQUE_OUTCOME_NOTIFICATION_ENTRIES);
+      // Added for 3.12.2
+      upgradeOutcomeTableRevision1To2(db);
+   }
+
+   // We only want to run this if going from DB v5 to v6 specifically since
+   //   it was originally missed in upgradeToV5 in 3.12.1
+   // Added for 3.12.2
+   private static void upgradeFromV5ToV6(SQLiteDatabase db) {
+      upgradeOutcomeTableRevision1To2(db);
+   }
+
+   // On the outcome table this adds the new weight column and drops params column.
+   private static void upgradeOutcomeTableRevision1To2(SQLiteDatabase db) {
+      String commonColumns = "_id,name,session,timestamp,notification_ids";
+      try {
+         // Since SQLite does not support dropping a column we need to:
+         //   1. Create a temptable
+         //   2. Copy outcome table into it
+         //   3. Drop the outcome table
+         //   4. Recreate it with the correct fields
+         //   5. Copy the temptable rows back into the new outcome table
+         //   6. Drop the temptable.
+         db.execSQL("BEGIN TRANSACTION;");
+         db.execSQL("CREATE TEMPORARY TABLE outcome_backup(" + commonColumns + ");");
+         db.execSQL("INSERT INTO outcome_backup SELECT " + commonColumns + " FROM outcome;");
+         db.execSQL("DROP TABLE outcome;");
+         db.execSQL(SQL_CREATE_OUTCOME_ENTRIES);
+         // Not converting weight from param here, just set to zero.
+         //   3.12.1 quickly replaced 3.12.0 so converting cache isn't critical.
+         db.execSQL("INSERT INTO outcome (" + commonColumns + ", weight) SELECT " + commonColumns + ", 0 FROM outcome_backup;");
+         db.execSQL("DROP TABLE outcome_backup;");
+         db.execSQL("COMMIT;");
+      } catch (SQLiteException e) {
+         e.printStackTrace();
+      }
    }
 
    private static void safeExecSQL(SQLiteDatabase db, String sql) {

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

@@ -8,9 +8,7 @@
 
 @Implements(OneSignalDbHelper.class)
 public class ShadowOneSignalDbHelper {
-
-   public static final String DATABASE_NAME = "OneSignal.db";
-   public static int DATABASE_VERSION = 5;
+   public static int DATABASE_VERSION;
    public static boolean ignoreDuplicatedFieldsOnUpgrade;
 
    private static OneSignalDbHelper sInstance;

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

@@ -15,6 +15,7 @@
 import com.onesignal.StaticResetHelper;
 
 import org.json.JSONArray;
+import org.junit.After;
 import org.junit.AfterClass;
 import org.junit.Before;
 import org.junit.BeforeClass;
@@ -33,6 +34,7 @@
 import static com.test.onesignal.TestHelpers.getAllUniqueOutcomeNotificationRecords;
 import static junit.framework.Assert.assertEquals;
 import static junit.framework.Assert.assertFalse;
+import static junit.framework.Assert.assertTrue;
 
 @Config(packageName = "com.onesignal.example",
         instrumentedPackages = { "com.onesignal" },
@@ -51,14 +53,19 @@ public static void setUpClass() throws Exception {
       StaticResetHelper.saveStaticValues();
    }
 
-   @Before // Before each test
+   @Before
    public void beforeEachTest() throws Exception {
       TestHelpers.beforeTestInitAndCleanup();
    }
 
+   @After
+   public void afterEachTest() throws Exception {
+      TestHelpers.afterTestCleanup();
+   }
+
    @AfterClass
    public static void afterEverything() throws Exception {
-      StaticResetHelper.restSetStaticFields();
+      TestHelpers.beforeTestInitAndCleanup();
    }
 
    @Test
@@ -131,13 +138,25 @@ public void shouldUpgradeDbFromV3ToV4() throws Exception {
       assertEquals(outcomeEvents.size(), 1);
    }
 
+
+   private static final String SQL_CREATE_OUTCOME_REVISION1_ENTRIES =
+      "CREATE TABLE outcome (" +
+         "_id INTEGER PRIMARY KEY, " +
+         "session TEXT," +
+         "notification_ids TEXT, " +
+         "name TEXT, " +
+         "timestamp TIMESTAMP, " +
+         "params TEXT " +
+      ")";
+
    @Test
    public void shouldUpgradeDbFromV4ToV5() {
       // 1. Init DB as version 4
       ShadowOneSignalDbHelper.DATABASE_VERSION = 4;
-      SQLiteDatabase readableDatabase = OneSignalDbHelper.getInstance(RuntimeEnvironment.application).getReadableDatabase();
+      SQLiteDatabase writableDatabase = OneSignalDbHelper.getInstance(RuntimeEnvironment.application).getWritableDatabase();
+      writableDatabase.execSQL(SQL_CREATE_OUTCOME_REVISION1_ENTRIES);
 
-      Cursor cursor = readableDatabase.rawQuery("SELECT name FROM sqlite_master WHERE type ='table' AND name='" + CachedUniqueOutcomeNotificationTable.TABLE_NAME + "'", null);
+      Cursor cursor = writableDatabase.rawQuery("SELECT name FROM sqlite_master WHERE type ='table' AND name='" + CachedUniqueOutcomeNotificationTable.TABLE_NAME + "'", null);
 
       boolean exist = false;
       if (cursor != null) {
@@ -147,7 +166,7 @@ public void shouldUpgradeDbFromV4ToV5() {
       // 2. Table must not exist
       assertFalse(exist);
 
-      readableDatabase.close();
+      writableDatabase.close();
 
       CachedUniqueOutcomeNotification notification = new CachedUniqueOutcomeNotification("notificationId", "outcome");
       ContentValues values = new ContentValues();
@@ -163,7 +182,7 @@ public void shouldUpgradeDbFromV4ToV5() {
 
       assertEquals(notifications.size(), 0);
 
-      SQLiteDatabase writableDatabase = OneSignalDbHelper.getInstance(RuntimeEnvironment.application).getWritableDatabase();
+      writableDatabase = OneSignalDbHelper.getInstance(RuntimeEnvironment.application).getWritableDatabase();
       // 5. Table now must exist
       writableDatabase.insert(CachedUniqueOutcomeNotificationTable.TABLE_NAME, null, values);
       writableDatabase.close();
@@ -172,4 +191,50 @@ public void shouldUpgradeDbFromV4ToV5() {
 
       assertEquals(uniqueOutcomeNotifications.size(), 1);
    }
+
+   @Test
+   public void shouldUpgradeDbFromV5ToV6() {
+      // 1. Init outcome table as version 5
+      SQLiteDatabase writableDatabase = OneSignalDbHelper.getInstance(RuntimeEnvironment.application).getWritableDatabase();
+
+      // Create table with the schema we had in DB v4
+      writableDatabase.execSQL(SQL_CREATE_OUTCOME_REVISION1_ENTRIES);
+
+      // Insert one outcome record so we can test migration keeps it later on
+      ContentValues values = new ContentValues();
+      values.put("name", "a");
+      writableDatabase.insertOrThrow("outcome", null, values);
+      writableDatabase.setVersion(5);
+      writableDatabase.close();
+
+      // 2. restSetStaticFields so the db reloads and upgrade is done to version 6
+      ShadowOneSignalDbHelper.restSetStaticFields();
+      writableDatabase = OneSignalDbHelper.getInstance(RuntimeEnvironment.application).getWritableDatabase();
+
+      // 3. Ensure the upgrade kept our existing record
+      Cursor cursor = writableDatabase.query(
+         "outcome",
+        null,
+         null,
+         null,
+         null,
+         null,
+         null
+      );
+      assertTrue(cursor.moveToFirst());
+      assertEquals("a", cursor.getString(cursor.getColumnIndex("name")));
+
+      // 4. Ensure new weight column exists
+      values = new ContentValues();
+      values.put("weight", 1);
+      long successful = writableDatabase.insert("outcome", null, values);
+      assertFalse(successful == -1);
+
+      // 5. Ensure params column does NOT exists
+      values = new ContentValues();
+      values.put("params", 1);
+      successful = writableDatabase.insert("outcome", null, values);
+      writableDatabase.close();
+      assertEquals(-1, successful);
+   }
 }