Merge pull request #697 from snowplow/release/6.0.5

Release/6.0.5

CHANGELOG

@@ -1,3 +1,8 @@
+Version 6.0.5 (2024-06-22)
+--------------------------
+Handle exceptions raised by the Executor during initialization of the thread pool for emitter (#694)
+Fix concurrent modification when creating TrackerEvent by copying the payload (#692)
+
 Version 6.0.4 (2024-06-05)
 --------------------------
 Send correct install referrer timestamps (#687)

VERSION

@@ -1 +1 @@
-6.0.4
+6.0.5

build.gradle

@@ -22,7 +22,7 @@ plugins {
 
 subprojects {
     group = 'com.snowplowanalytics'
-    version = '6.0.4'
+    version = '6.0.5'
     repositories {
         google()
         maven {

gradle.properties

@@ -31,7 +31,7 @@ systemProp.org.gradle.internal.http.socketTimeout=120000
 SONATYPE_STAGING_PROFILE=comsnowplowanalytics
 GROUP=com.snowplowanalytics
 POM_ARTIFACT_ID=snowplow-android-tracker
-VERSION_NAME=6.0.4
+VERSION_NAME=6.0.5
 
 POM_NAME=snowplow-android-tracker
 POM_PACKAGING=aar

snowplow-tracker/src/androidTest/java/com/snowplowanalytics/snowplow/internal/emitter/storage/EventStoreTest.kt

@@ -122,7 +122,7 @@ class EventStoreTest {
     @Throws(InterruptedException::class)
     fun testInsertPayload() {
         val eventStore = eventStore()
-        val id = eventStore.insertEvent(payload())
+        val id = eventStore.insertEvent(payload())!!
         val lastRowId = eventStore.lastInsertedRowId
         val event = eventStore.getEvent(id)
         Assert.assertEquals(id, lastRowId)
@@ -153,7 +153,7 @@ class EventStoreTest {
     @Throws(InterruptedException::class)
     fun testRemoveIndividualEvent() {
         val eventStore = eventStore()
-        val id = eventStore.insertEvent(payload())
+        val id = eventStore.insertEvent(payload())!!
         var res = eventStore.removeEvent(id)
         Assert.assertEquals(0, eventStore.size())
         Assert.assertTrue(res)
@@ -169,9 +169,9 @@ class EventStoreTest {
     fun testRemoveRangeOfEvents() {
         val eventStore = eventStore()
         val idList: MutableList<Long> = ArrayList()
-        idList.add(eventStore.insertEvent(payload()))
-        idList.add(eventStore.insertEvent(payload()))
-        idList.add(eventStore.insertEvent(payload()))
+        idList.add(eventStore.insertEvent(payload())!!)
+        idList.add(eventStore.insertEvent(payload())!!)
+        idList.add(eventStore.insertEvent(payload())!!)
         Assert.assertEquals(3, idList.size.toLong())
         Assert.assertEquals(3, eventStore.size())
         var res = eventStore.removeEvents(idList)

snowplow-tracker/src/main/java/com/snowplowanalytics/core/emitter/Executor.kt

@@ -14,10 +14,7 @@ package com.snowplowanalytics.core.emitter
 
 import androidx.annotation.RestrictTo
 import com.snowplowanalytics.core.tracker.Logger
-import java.util.concurrent.Callable
-import java.util.concurrent.ExecutorService
-import java.util.concurrent.Executors
-import java.util.concurrent.Future
+import java.util.concurrent.*
 
 /**
  * Static Class which holds the logic for controlling
@@ -51,11 +48,15 @@ object Executor {
      */
     @Synchronized
     @JvmStatic
-    private fun getExecutor(): ExecutorService {
+    private fun getExecutor(): ExecutorService? {
         if (executor == null) {
-            executor = Executors.newScheduledThreadPool(threadCount)
+            try {
+                executor = Executors.newScheduledThreadPool(threadCount)
+            } catch (e: Exception) {
+                Logger.e("Executor", e.message ?: "Failed to create thread pool")
+            }
         }
-        return executor!!
+        return executor
     }
 
     /**
@@ -105,7 +106,7 @@ object Executor {
     fun execute(runnable: Runnable?, exceptionHandler: ExceptionHandler?) {
         val executor = getExecutor()
         try {
-            executor.execute {
+            executor?.execute {
                 try {
                     runnable?.run()
                 } catch (t: Throwable) {
@@ -125,8 +126,13 @@ object Executor {
      * @return the future object to be queried
      */
     @JvmStatic
-    fun futureCallable(callable: Callable<*>): Future<*> {
-        return getExecutor().submit(callable)
+    fun futureCallable(callable: Callable<*>): Future<*>? {
+        return try {
+            getExecutor()?.submit(callable)
+        } catch (e: Exception) {
+            Logger.e("Executor", e.message ?: "Failed to submit task")
+            null
+        }
     }
 
     /**
@@ -136,7 +142,11 @@ object Executor {
     @JvmStatic
     fun shutdown(): ExecutorService? {
         if (executor != null) {
-            executor!!.shutdown()
+            try {
+                executor?.shutdown()
+            } catch (e: Exception) {
+                Logger.e("Executor", e.message ?: "Failed to shutdown")
+            }
             val es = executor
             executor = null
             return es

snowplow-tracker/src/main/java/com/snowplowanalytics/core/emitter/storage/SQLiteEventStore.kt

@@ -41,7 +41,7 @@ import kotlin.time.Duration
 class SQLiteEventStore(context: Context, private val namespace: String) : EventStore {
     private val payloadWaitingList: MutableList<Payload> = ArrayList()
     private var database: SQLiteDatabase? = null
-    private lateinit var dbHelper: EventStoreHelper
+    private var dbHelper: EventStoreHelper? = null
     private val allColumns = arrayOf(
         EventStoreHelper.COLUMN_ID,
         EventStoreHelper.COLUMN_EVENT_DATA,
@@ -62,7 +62,7 @@ class SQLiteEventStore(context: Context, private val namespace: String) : EventS
      * @return a boolean for database status
      */
     val databaseOpen: Boolean
-        get() = database != null && database!!.isOpen
+        get() = database != null && database?.isOpen ?: false
 
     /**
      * Creates a new Event Store
@@ -91,7 +91,7 @@ class SQLiteEventStore(context: Context, private val namespace: String) : EventS
      */
     fun open() {
         if (!databaseOpen) {
-            database = dbHelper.writableDatabase
+            database = dbHelper?.writableDatabase
             database?.enableWriteAheadLogging()
         }
     }
@@ -100,7 +100,7 @@ class SQLiteEventStore(context: Context, private val namespace: String) : EventS
      * Closes the database
      */
     fun close() {
-        dbHelper.close()
+        dbHelper?.close()
         EventStoreHelper.removeInstance(namespace)
     }
 
@@ -112,25 +112,27 @@ class SQLiteEventStore(context: Context, private val namespace: String) : EventS
      * @return a boolean stating if the insert
      * was a success or not
      */
-    fun insertEvent(payload: Payload): Long {
+    fun insertEvent(payload: Payload): Long? {
         if (databaseOpen) {
+            val database = database ?: return null
             val bytes = Util.serialize(Util.objectMapToString(payload.map))
             val values = ContentValues(2)
             values.put(EventStoreHelper.COLUMN_EVENT_DATA, bytes)
             lastInsertedRowId =
-                database!!.insert(EventStoreHelper.TABLE_EVENTS, null, values)
+                database.insert(EventStoreHelper.TABLE_EVENTS, null, values)
+            Logger.d(TAG, "Added event to database: %s", lastInsertedRowId)
+            return lastInsertedRowId
         }
-        Logger.d(TAG, "Added event to database: %s", lastInsertedRowId)
-        return lastInsertedRowId
+        return null
     }
 
     override fun removeEvent(id: Long): Boolean {
         var retval = -1
         if (databaseOpen) {
-            retval = database!!.delete(
+            retval = database?.delete(
                 EventStoreHelper.TABLE_EVENTS,
                 EventStoreHelper.COLUMN_ID + "=" + id, null
-            )
+            ) ?: retval
         }
         Logger.d(TAG, "Removed event from database: %s", "" + id)
         return retval == 1
@@ -142,10 +144,10 @@ class SQLiteEventStore(context: Context, private val namespace: String) : EventS
         }
         var retval = -1
         if (databaseOpen) {
-            retval = database!!.delete(
+            retval = database?.delete(
                 EventStoreHelper.TABLE_EVENTS,
                 EventStoreHelper.COLUMN_ID + " in (" + Util.joinLongList(ids) + ")", null
-            )
+            ) ?: retval
         }
         Logger.d(TAG, "Removed events from database: %s", retval)
         return retval == ids.size
@@ -155,7 +157,7 @@ class SQLiteEventStore(context: Context, private val namespace: String) : EventS
         var retval = 0
         Logger.d(TAG, "Removing all events from database.")
         if (databaseOpen) {
-            retval = database!!.delete(EventStoreHelper.TABLE_EVENTS, null, null)
+            retval = database?.delete(EventStoreHelper.TABLE_EVENTS, null, null) ?: retval
         } else {
             Logger.e(TAG, "Database is not open.")
         }
@@ -196,9 +198,10 @@ class SQLiteEventStore(context: Context, private val namespace: String) : EventS
     private fun queryDatabase(query: String?, orderBy: String?): List<Map<String, Any?>> {
         val res: MutableList<Map<String, Any?>> = ArrayList()
         if (databaseOpen) {
+            val database = database ?: return res
             var cursor: Cursor? = null
             try {
-                cursor = database!!.query(
+                cursor = database.query(
                     EventStoreHelper.TABLE_EVENTS,
                     allColumns,
                     query,

snowplow-tracker/src/main/java/com/snowplowanalytics/core/tracker/TrackerEvent.kt

@@ -43,8 +43,12 @@ class TrackerEvent @JvmOverloads constructor(event: Event, state: TrackerStateSn
     init {
         entities = event.entities.toMutableList()
         trueTimestamp = event.trueTimestamp
-        payload = HashMap(event.dataPayload)
-
+        // NOTE: this code is a workaround since the types of the `Event.dataPayload` and `TrackerEvent.payload` don't match
+        // `Event` allows the values to be optional, while `TrackerEvent` does not.
+        // We should fix this in the next major version to unify the types.
+        // The `toMap()` is called in order to make a copy before calling HashMap as that can lead to concurrency issues, see #692
+        payload = HashMap(event.dataPayload.toMap())
+
         if (state != null) {
             this.state = state
         } else {

snowplow-tracker/src/main/java/com/snowplowanalytics/snowplow/network/OkHttpNetworkConnection.kt

@@ -248,7 +248,7 @@ class OkHttpNetworkConnection private constructor(builder: OkHttpNetworkConnecti
                 request,
                 userAgent
             ) else buildPostRequest(request, userAgent)
-            futures.add(Executor.futureCallable(getRequestCallable(okHttpRequest)))
+            Executor.futureCallable(getRequestCallable(okHttpRequest))?.let { futures.add(it) }
         }
         Logger.d(TAG, "Request Futures: %s", futures.size)