Merge pull request #1793 from OneSignal/user_model/fix_outcome_requests

[5.0.0] Fix outcome requests

Author: emawby

OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/database/impl/OSDatabase.kt

@@ -16,7 +16,7 @@ import com.onesignal.core.internal.database.IDatabase
 import com.onesignal.debug.internal.logging.Logging
 import com.onesignal.session.internal.outcomes.impl.OutcomeTableProvider
 import com.onesignal.session.internal.outcomes.impl.OutcomesDbContract.SQL_CREATE_OUTCOME_ENTRIES_V1
-import com.onesignal.session.internal.outcomes.impl.OutcomesDbContract.SQL_CREATE_OUTCOME_ENTRIES_V3
+import com.onesignal.session.internal.outcomes.impl.OutcomesDbContract.SQL_CREATE_OUTCOME_ENTRIES_V4
 import com.onesignal.session.internal.outcomes.impl.OutcomesDbContract.SQL_CREATE_UNIQUE_OUTCOME_ENTRIES_V1
 import com.onesignal.session.internal.outcomes.impl.OutcomesDbContract.SQL_CREATE_UNIQUE_OUTCOME_ENTRIES_V2
 
@@ -246,7 +246,7 @@ internal open class OSDatabase(
 
     override fun onCreate(db: SQLiteDatabase) {
         db.execSQL(SQL_CREATE_ENTRIES)
-        db.execSQL(SQL_CREATE_OUTCOME_ENTRIES_V3)
+        db.execSQL(SQL_CREATE_OUTCOME_ENTRIES_V4)
         db.execSQL(SQL_CREATE_UNIQUE_OUTCOME_ENTRIES_V2)
         db.execSQL(SQL_CREATE_IN_APP_MESSAGE_ENTRIES)
         for (ind in SQL_INDEX_ENTRIES) {
@@ -277,6 +277,7 @@ internal open class OSDatabase(
         if (oldVersion == 5 && newVersion >= 6) upgradeFromV5ToV6(db)
         if (oldVersion < 7 && newVersion >= 7) upgradeToV7(db)
         if (oldVersion < 8 && newVersion >= 8) upgradeToV8(db)
+        if (oldVersion < 9 && newVersion >= 9) upgradeToV9(db)
     }
 
     // Add collapse_id field and index
@@ -342,6 +343,10 @@ internal open class OSDatabase(
         _outcomeTableProvider.upgradeCacheOutcomeTableRevision1To2(db)
     }
 
+    private fun upgradeToV9(db: SQLiteDatabase) {
+        _outcomeTableProvider.upgradeOutcomeTableRevision3To4(db)
+    }
+
     override fun onDowngrade(db: SQLiteDatabase, oldVersion: Int, newVersion: Int) {
         Logging.warn("SDK version rolled back! Clearing $DATABASE_NAME as it could be in an unexpected state.")
 
@@ -357,7 +362,7 @@ internal open class OSDatabase(
     }
 
     companion object {
-        private const val dbVersion = 8
+        private const val dbVersion = 9
         private val LOCK = Any()
         private const val DATABASE_NAME = "OneSignal.db"
         private const val INTEGER_PRIMARY_KEY_TYPE = " INTEGER PRIMARY KEY"

OneSignalSDK/onesignal/core/src/main/java/com/onesignal/session/internal/outcomes/IOutcomeEvent.kt

@@ -8,5 +8,6 @@ interface IOutcomeEvent {
     val notificationIds: JSONArray?
     val name: String
     val timestamp: Long
+    val sessionTime: Long // in seconds
     val weight: Float
 }

OneSignalSDK/onesignal/core/src/main/java/com/onesignal/session/internal/outcomes/IOutcomeEventsController.kt

@@ -4,6 +4,11 @@ package com.onesignal.session.internal.outcomes
  * The gateway to outcomes logic.
  */
 interface IOutcomeEventsController {
+    /**
+     * Send a session ending outcome event to the backend.
+     */
+    suspend fun sendSessionEndOutcomeEvent(duration: Long): IOutcomeEvent?
+
     /**
      * Send a unique outcome event to the backend.
      */

OneSignalSDK/onesignal/core/src/main/java/com/onesignal/session/internal/outcomes/impl/IOutcomeEventsBackendService.kt

@@ -15,8 +15,9 @@ internal interface IOutcomeEventsBackendService {
      * @param appId The ID of the application this outcome event occurred under.
      * @param userId The OneSignal user ID that is active during the outcome event.
      * @param subscriptionId The subscription ID that is active during the outcome event.
+     * @param deviceType The type of device that the outcome event occurred on.
      * @param direct Whether this outcome event is direct. `true` if it is, `false` if it isn't, `null` if should not be specified.
      * @param event The outcome event to send up.
      */
-    suspend fun sendOutcomeEvent(appId: String, userId: String, subscriptionId: String, direct: Boolean?, event: OutcomeEvent)
+    suspend fun sendOutcomeEvent(appId: String, userId: String, subscriptionId: String, deviceType: String, direct: Boolean?, event: OutcomeEvent)
 }

OneSignalSDK/onesignal/core/src/main/java/com/onesignal/session/internal/outcomes/impl/OutcomeConstants.kt

@@ -6,6 +6,7 @@ internal object OutcomeConstants {
     const val OUTCOME_SOURCES = "sources"
     const val WEIGHT = "weight"
     const val TIMESTAMP = "timestamp"
+    const val SESSION_TIME = "session_time"
 
     // OSOutcomeSource Constants
     const val DIRECT = "direct"

OneSignalSDK/onesignal/core/src/main/java/com/onesignal/session/internal/outcomes/impl/OutcomeEvent.kt

@@ -11,6 +11,7 @@ internal class OutcomeEvent(
     override val notificationIds: JSONArray?,
     override val name: String,
     override val timestamp: Long,
+    override val sessionTime: Long,
     override val weight: Float,
 ) : IOutcomeEvent {
     @Throws(JSONException::class)
@@ -20,6 +21,7 @@ internal class OutcomeEvent(
         json.put(NOTIFICATION_IDS, notificationIds)
         json.put(OUTCOME_ID, name)
         json.put(TIMESTAMP, timestamp)
+        json.put(SESSION_TIME, sessionTime)
         json.put(WEIGHT, weight)
         return json
     }
@@ -28,11 +30,11 @@ internal class OutcomeEvent(
         if (this === o) return true
         if (o == null || this.javaClass != o.javaClass) return false
         val event = o as OutcomeEvent
-        return session == event.session && notificationIds == event.notificationIds && name == event.name && timestamp == event.timestamp && weight == event.weight
+        return session == event.session && notificationIds == event.notificationIds && name == event.name && timestamp == event.timestamp && sessionTime == event.sessionTime && weight == event.weight
     }
 
     override fun hashCode(): Int {
-        val a = arrayOf(session, notificationIds, name, timestamp, weight)
+        val a = arrayOf(session, notificationIds, name, timestamp, sessionTime, weight)
         var result = 1
         for (element in a) result = 31 * result + (element?.hashCode() ?: 0)
         return result
@@ -44,6 +46,7 @@ internal class OutcomeEvent(
             ", notificationIds=" + notificationIds +
             ", name='" + name + '\'' +
             ", timestamp=" + timestamp +
+            ", sessionTime=" + sessionTime +
             ", weight=" + weight +
             '}'
     }
@@ -53,6 +56,7 @@ internal class OutcomeEvent(
         private const val NOTIFICATION_IDS = "notification_ids"
         private const val OUTCOME_ID = "id"
         private const val TIMESTAMP = "timestamp"
+        private const val SESSION_TIME = "session_time"
         private const val WEIGHT = "weight"
 
         /**
@@ -76,6 +80,7 @@ internal class OutcomeEvent(
                 notificationId,
                 outcomeEventParams.outcomeId,
                 outcomeEventParams.timestamp,
+                outcomeEventParams.sessionTime,
                 outcomeEventParams.weight,
             )
         }

OneSignalSDK/onesignal/core/src/main/java/com/onesignal/session/internal/outcomes/impl/OutcomeEventParams.kt

@@ -7,7 +7,8 @@ internal class OutcomeEventParams constructor(
     val outcomeId: String,
     val outcomeSource: OutcomeSource?, // This field is optional
     var weight: Float, // This field is optional.
-    var timestamp: Long = 0,
+    var sessionTime: Long, // This field is optional
+    var timestamp: Long, // This should start out as zero
 ) {
     @Throws(JSONException::class)
     fun toJSONObject(): JSONObject {
@@ -18,6 +19,7 @@ internal class OutcomeEventParams constructor(
         }
         if (weight > 0) json.put(OutcomeConstants.WEIGHT, weight)
         if (timestamp > 0) json.put(OutcomeConstants.TIMESTAMP, timestamp)
+        if (sessionTime > 0) json.put(OutcomeConstants.SESSION_TIME, sessionTime)
         return json
     }
 
@@ -29,6 +31,7 @@ internal class OutcomeEventParams constructor(
             ", outcomeSource=" + outcomeSource +
             ", weight=" + weight +
             ", timestamp=" + timestamp +
+            ", sessionTime=" + sessionTime +
             '}'
     }
 }

OneSignalSDK/onesignal/core/src/main/java/com/onesignal/session/internal/outcomes/impl/OutcomeEventsBackendService.kt

@@ -7,14 +7,15 @@ import org.json.JSONObject
 internal class OutcomeEventsBackendService(private val _http: IHttpClient) :
     IOutcomeEventsBackendService {
 
-    override suspend fun sendOutcomeEvent(appId: String, userId: String, subscriptionId: String, direct: Boolean?, event: OutcomeEvent) {
+    override suspend fun sendOutcomeEvent(appId: String, userId: String, subscriptionId: String, deviceType: String, direct: Boolean?, event: OutcomeEvent) {
         val jsonObject = JSONObject()
             .put("app_id", appId)
             .put("onesignal_id", userId)
             .put(
                 "subscription",
                 JSONObject()
-                    .put("id", subscriptionId),
+                    .put("id", subscriptionId)
+                    .put("type", deviceType)
             )
 
         if (direct != null) {
@@ -34,6 +35,10 @@ internal class OutcomeEventsBackendService(private val _http: IHttpClient) :
             jsonObject.put("timestamp", event.timestamp)
         }
 
+        if (event.sessionTime > 0) {
+            jsonObject.put("session_time", event.sessionTime)
+        }
+
         val response = _http.post("outcomes/measure", jsonObject)
 
         if (!response.isSuccess) {

OneSignalSDK/onesignal/core/src/main/java/com/onesignal/session/internal/outcomes/impl/OutcomeEventsController.kt

@@ -4,6 +4,7 @@ import android.os.Process
 import com.onesignal.common.exceptions.BackendException
 import com.onesignal.common.threading.suspendifyOnThread
 import com.onesignal.core.internal.config.ConfigModelStore
+import com.onesignal.core.internal.device.IDeviceService
 import com.onesignal.core.internal.startup.IStartableService
 import com.onesignal.core.internal.time.ITime
 import com.onesignal.debug.internal.logging.Logging
@@ -14,6 +15,7 @@ import com.onesignal.session.internal.influence.InfluenceType
 import com.onesignal.session.internal.outcomes.IOutcomeEventsController
 import com.onesignal.session.internal.session.ISessionLifecycleHandler
 import com.onesignal.session.internal.session.ISessionService
+import com.onesignal.user.internal.backend.SubscriptionObjectType
 import com.onesignal.user.internal.identity.IdentityModelStore
 import com.onesignal.user.internal.subscriptions.ISubscriptionManager
 
@@ -26,6 +28,7 @@ internal class OutcomeEventsController(
     private val _configModelStore: ConfigModelStore,
     private val _identityModelStore: IdentityModelStore,
     private val _subscriptionManager: ISubscriptionManager,
+    private val _deviceService: IDeviceService,
     private val _time: ITime,
 ) : IOutcomeEventsController, IStartableService, ISessionLifecycleHandler {
     // Keeps track of unique outcome events sent for UNATTRIBUTED sessions on a per session level
@@ -78,19 +81,31 @@ Outcome event was cached and will be reattempted on app cold start""",
         }
     }
 
+    override suspend fun sendSessionEndOutcomeEvent(duration: Long): OutcomeEvent? {
+        val influences: List<Influence> = _influenceManager.influences
+
+        // only send the outcome if there are any influences associated with the session
+        for (influence in influences) {
+            if (influence.ids != null) {
+                return sendAndCreateOutcomeEvent("os__session_duration", 0f, duration, influences)
+            }
+        }
+        return null
+    }
+
     override suspend fun sendUniqueOutcomeEvent(name: String): OutcomeEvent? {
         val sessionResult: List<Influence> = _influenceManager.influences
         return sendUniqueOutcomeEvent(name, sessionResult)
     }
 
     override suspend fun sendOutcomeEvent(name: String): OutcomeEvent? {
         val influences: List<Influence> = _influenceManager.influences
-        return sendAndCreateOutcomeEvent(name, 0f, influences)
+        return sendAndCreateOutcomeEvent(name, 0f, 0, influences)
     }
 
     override suspend fun sendOutcomeEventWithValue(name: String, weight: Float): OutcomeEvent? {
         val influences: List<Influence> = _influenceManager.influences
-        return sendAndCreateOutcomeEvent(name, weight, influences)
+        return sendAndCreateOutcomeEvent(name, weight, 0, influences)
     }
 
     /**
@@ -131,7 +146,7 @@ Outcome event was cached and will be reattempted on app cold start""",
                 // Return null to determine not a failure, but not a success in terms of the request made
                 return null
             }
-            return sendAndCreateOutcomeEvent(name, 0f, uniqueInfluences)
+            return sendAndCreateOutcomeEvent(name, 0f, 0, uniqueInfluences)
         } else {
             // Make sure unique outcome has not been sent for current unattributed session
             if (unattributedUniqueOutcomeEventsSentOnSession.contains(name)) {
@@ -147,13 +162,14 @@ Outcome event was cached and will be reattempted on app cold start""",
                 return null
             }
             unattributedUniqueOutcomeEventsSentOnSession.add(name)
-            return sendAndCreateOutcomeEvent(name, 0f, influences)
+            return sendAndCreateOutcomeEvent(name, 0f, 0, influences)
         }
     }
 
     private suspend fun sendAndCreateOutcomeEvent(
         name: String,
         weight: Float,
+        sessionTime: Long, // Note: this is optional
         influences: List<Influence>,
     ): OutcomeEvent? {
         val timestampSeconds: Long = _time.currentTimeMillis / 1000
@@ -183,7 +199,7 @@ Outcome event was cached and will be reattempted on app cold start""",
         }
 
         val source = OutcomeSource(directSourceBody, indirectSourceBody)
-        val eventParams = OutcomeEventParams(name, source, weight, 0)
+        val eventParams = OutcomeEventParams(name, source, weight, sessionTime, 0)
 
         try {
             requestMeasureOutcomeEvent(eventParams)
@@ -267,10 +283,11 @@ Outcome event was cached and will be reattempted on app cold start""",
     private suspend fun requestMeasureOutcomeEvent(eventParams: OutcomeEventParams) {
         val appId: String = _configModelStore.model.appId
         val subscriptionId = _subscriptionManager.subscriptions.push.id
+        val deviceType = SubscriptionObjectType.fromDeviceType(_deviceService.deviceType).value
 
         // if we don't have a subscription ID yet, throw an exception. The outcome will be saved and processed
         // later, when we do have a subscription ID.
-        if (subscriptionId.isEmpty()) {
+        if (subscriptionId.isEmpty() || deviceType.isEmpty()) {
             throw BackendException(0)
         }
 
@@ -282,6 +299,6 @@ Outcome event was cached and will be reattempted on app cold start""",
             else -> null
         }
 
-        _outcomeEventsBackend.sendOutcomeEvent(appId, _identityModelStore.model.onesignalId, subscriptionId, direct, event)
+        _outcomeEventsBackend.sendOutcomeEvent(appId, _identityModelStore.model.onesignalId, subscriptionId, deviceType, direct, event)
     }
 }

OneSignalSDK/onesignal/core/src/main/java/com/onesignal/session/internal/outcomes/impl/OutcomeEventsRepository.kt

@@ -88,6 +88,7 @@ internal class OutcomeEventsRepository(
                 put(OutcomeEventsTable.COLUMN_NAME_NAME, eventParams.outcomeId)
                 put(OutcomeEventsTable.COLUMN_NAME_WEIGHT, eventParams.weight)
                 put(OutcomeEventsTable.COLUMN_NAME_TIMESTAMP, eventParams.timestamp)
+                put(OutcomeEventsTable.COLUMN_NAME_SESSION_TIME, eventParams.sessionTime)
             }.also { values ->
                 _databaseProvider.os.insert(OutcomeEventsTable.TABLE_NAME, null, values)
             }
@@ -128,6 +129,8 @@ internal class OutcomeEventsRepository(
                             cursor.getFloat(OutcomeEventsTable.COLUMN_NAME_WEIGHT)
                         val timestamp =
                             cursor.getLong(OutcomeEventsTable.COLUMN_NAME_TIMESTAMP)
+                        val sessionTime =
+                            cursor.getLong(OutcomeEventsTable.COLUMN_NAME_SESSION_TIME)
 
                         try {
                             val directSourceBody = OutcomeSourceBody()
@@ -147,7 +150,7 @@ internal class OutcomeEventsRepository(
                                         it,
                                     )
                                 } ?: OutcomeSource(null, null)
-                            OutcomeEventParams(name, source, weight, timestamp).also {
+                            OutcomeEventParams(name, source, weight, sessionTime, timestamp).also {
                                 events.add(it)
                             }
                         } catch (e: JSONException) {

OneSignalSDK/onesignal/core/src/main/java/com/onesignal/session/internal/outcomes/impl/OutcomeTableProvider.kt

@@ -78,6 +78,24 @@ internal class OutcomeTableProvider {
         }
     }
 
+    /**
+     * On the outcome table this adds the new session_time column.
+     *
+     * @param db
+     */
+    fun upgradeOutcomeTableRevision3To4(db: SQLiteDatabase) {
+        try {
+            db.execSQL("BEGIN TRANSACTION;")
+            db.execSQL("ALTER TABLE " + OutcomeEventsTable.TABLE_NAME + " ADD COLUMN " + OutcomeEventsTable.COLUMN_NAME_SESSION_TIME + " INTEGER DEFAULT 1;")
+            // We intentionally choose to default session_time to 1 to address a bug on cached outcomes from v5.0.0-beta's
+            // os__session_duration requests expect a session_time and these will keep failing and caching, so let's just send them with a time of 1 for migrations
+        } catch (e: SQLiteException) {
+            e.printStackTrace()
+        } finally {
+            db.execSQL("COMMIT;")
+        }
+    }
+
     /**
      * On the cache unique outcome table rename table, rename column notification id to influence id
      * Add column channel type

OneSignalSDK/onesignal/core/src/main/java/com/onesignal/session/internal/outcomes/impl/OutcomesDbContract.kt

@@ -20,6 +20,9 @@ internal object OutcomeEventsTable {
     const val COLUMN_NAME_WEIGHT = "weight" // Added on DB v5 SDK v3.12.1, migration added on DB v6 SDK v3.12.2
     const val COLUMN_NAME_TIMESTAMP = "timestamp" // Added on DB v4 SDK v3.12.0
     const val COLUMN_NAME_PARAMS = "params" // Added on DB v4 SDK v3.12.0 replaced with weight on DB v5 SDK v3.12.1, migration added on DB v6 SDK v3.12.2
+
+    // Session time
+    const val COLUMN_NAME_SESSION_TIME = "session_time" // Added on DB v9 SDK v5.0.0 (note that 5.0.0-beta's were still on v8)
 }
 
 internal object CachedUniqueOutcomeTable {
@@ -72,6 +75,22 @@ internal object OutcomesDbContract {
         OutcomeEventsTable.COLUMN_NAME_TIMESTAMP + TIMESTAMP_TYPE + "," + // "params TEXT" Added in v4, removed in v5.
         OutcomeEventsTable.COLUMN_NAME_WEIGHT + FLOAT_TYPE + // New in v5, missing migration added in v6
         ");"
+
+    /**
+     * Adds a new column called session_time
+     */
+    const val SQL_CREATE_OUTCOME_ENTRIES_V4 = "CREATE TABLE " + OutcomeEventsTable.TABLE_NAME + " (" +
+        OutcomeEventsTable.ID + INTEGER_PRIMARY_KEY_TYPE + "," +
+        OutcomeEventsTable.COLUMN_NAME_NOTIFICATION_INFLUENCE_TYPE + TEXT_TYPE + "," +
+        OutcomeEventsTable.COLUMN_NAME_IAM_INFLUENCE_TYPE + TEXT_TYPE + "," +
+        OutcomeEventsTable.COLUMN_NAME_NOTIFICATION_IDS + TEXT_TYPE + "," +
+        OutcomeEventsTable.COLUMN_NAME_IAM_IDS + TEXT_TYPE + "," +
+        OutcomeEventsTable.COLUMN_NAME_NAME + TEXT_TYPE + "," +
+        OutcomeEventsTable.COLUMN_NAME_TIMESTAMP + TIMESTAMP_TYPE + "," +
+        OutcomeEventsTable.COLUMN_NAME_WEIGHT + FLOAT_TYPE + "," +
+        OutcomeEventsTable.COLUMN_NAME_SESSION_TIME + INT_TYPE +
+        ");"
+
     const val SQL_CREATE_UNIQUE_OUTCOME_ENTRIES_V1 = "CREATE TABLE " + CachedUniqueOutcomeTable.TABLE_NAME_V1 + " (" +
         CachedUniqueOutcomeTable.ID + INTEGER_PRIMARY_KEY_TYPE + "," +
         CachedUniqueOutcomeTable.COLUMN_NAME_NOTIFICATION_ID + TEXT_TYPE + "," +