Run the Glean API off the main thread by default

This introduces the internal Dispatchers object,
which allows to use a predefined scope for launching
functions off the main thread. Moreover, this also adds
the test only FakeDispatchersInTest class, which allows
to block on API calls during tests, ensuring they don't
fail due to racing.

Author: Dexterp37

components/service/glean/src/main/java/mozilla/components/service/glean/Dispatchers.kt

@@ -0,0 +1,18 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+package mozilla.components.service.glean
+
+import kotlinx.coroutines.CoroutineScope
+import kotlinx.coroutines.ObsoleteCoroutinesApi
+import kotlinx.coroutines.newSingleThreadContext
+
+@ObsoleteCoroutinesApi
+internal object Dispatchers {
+    /**
+     * A coroutine scope to make it easy to dispatch API calls off the main thread.
+     * This needs to be a `var` so that our tests can override this.
+     */
+    var API = CoroutineScope(newSingleThreadContext("GleanAPIPool"))
+}

components/service/glean/src/main/java/mozilla/components/service/glean/EventMetricType.kt

@@ -4,6 +4,7 @@
 
 package mozilla.components.service.glean
 
+import kotlinx.coroutines.launch
 import mozilla.components.service.glean.storages.EventsStorageEngine
 import mozilla.components.support.base.log.logger.Logger
 
@@ -101,14 +102,16 @@ data class EventMetricType(
             eventKeys
         }
 
-        // Delegate storing the event to the storage engine.
-        EventsStorageEngine.record(
-            stores = getStorageNames(),
-            category = category,
-            name = name,
-            objectId = objectId,
-            value = truncatedValue,
-            extra = truncatedExtraKeys
-        )
+        Dispatchers.API.launch {
+            // Delegate storing the event to the storage engine.
+            EventsStorageEngine.record(
+                stores = getStorageNames(),
+                category = category,
+                name = name,
+                objectId = objectId,
+                value = truncatedValue,
+                extra = truncatedExtraKeys
+            )
+        }
     }
 }

components/service/glean/src/main/java/mozilla/components/service/glean/StringMetricType.kt

@@ -4,6 +4,7 @@
 
 package mozilla.components.service.glean
 
+import kotlinx.coroutines.launch
 import mozilla.components.service.glean.storages.StringsStorageEngine
 import mozilla.components.support.base.log.logger.Logger
 
@@ -54,10 +55,12 @@ data class StringMetricType(
             it
         }
 
-        // Delegate storing the string to the storage engine.
-        StringsStorageEngine.record(
-            this,
-            value = truncatedValue
-        )
+        Dispatchers.API.launch {
+            // Delegate storing the string to the storage engine.
+            StringsStorageEngine.record(
+                this@StringMetricType,
+                value = truncatedValue
+            )
+        }
     }
 }

components/service/glean/src/main/java/mozilla/components/service/glean/UuidMetricType.kt

@@ -4,6 +4,7 @@
 
 package mozilla.components.service.glean
 
+import kotlinx.coroutines.launch
 import java.util.UUID
 
 import mozilla.components.service.glean.storages.UuidsStorageEngine
@@ -31,15 +32,26 @@ data class UuidMetricType(
 
     /**
      * Generate a new UUID value and set it in the metric store.
+     *
+     * @return a [UUID] or [null] if we're not allowed to record.
      */
-    fun generateAndSet(): UUID {
+    fun generateAndSet(): UUID? {
+        // Even if `set` is already checking if we're allowed to record,
+        // we need to check here as well otherwise we'd return a `UUID`
+        // that won't be stored anywhere.
+        if (!shouldRecord(logger)) {
+            return null
+        }
+
         val uuid = UUID.randomUUID()
         set(uuid)
         return uuid
     }
 
     /**
      * Explicitly set an existing UUID value
+     *
+     * @param value a valid [UUID] to set the metric to
      */
     fun set(value: UUID) {
         // TODO report errors through other special metrics handled by the SDK. See bug 1499761.
@@ -48,10 +60,12 @@ data class UuidMetricType(
             return
         }
 
-        // Delegate storing the event to the storage engine.
-        UuidsStorageEngine.record(
-                this,
+        Dispatchers.API.launch {
+            // Delegate storing the event to the storage engine.
+            UuidsStorageEngine.record(
+                this@UuidMetricType,
                 value = value
-        )
+            )
+        }
     }
 }

components/service/glean/src/test/java/mozilla/components/service/glean/DispatchersTest.kt

@@ -0,0 +1,35 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+package mozilla.components.service.glean
+
+import kotlinx.coroutines.launch
+import kotlinx.coroutines.runBlocking
+import org.junit.Test
+
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertNotSame
+import org.junit.Assert.assertSame
+
+class DispatchersTest {
+
+    @Test
+    fun `API scope runs off the main thread`() {
+        val mainThread = Thread.currentThread()
+        var threadCanary = false
+
+        runBlocking {
+            Dispatchers.API.launch {
+                assertNotSame(mainThread, Thread.currentThread())
+                // Use the canary bool to make sure this is getting called before
+                // the test completes.
+                assertEquals(false, threadCanary)
+                threadCanary = true
+            }.join()
+        }
+
+        assertEquals(true, threadCanary)
+        assertSame(mainThread, Thread.currentThread())
+    }
+}

components/service/glean/src/test/java/mozilla/components/service/glean/EventMetricTypeTest.kt

@@ -5,19 +5,27 @@
 package mozilla.components.service.glean
 
 import android.os.SystemClock
+import kotlinx.coroutines.ExperimentalCoroutinesApi
+import kotlinx.coroutines.ObsoleteCoroutinesApi
 import mozilla.components.service.glean.storages.EventsStorageEngine
 import org.junit.Assert.assertEquals
 import org.junit.Assert.assertNull
 import org.junit.Assert.assertTrue
 import org.junit.Test
 
 import org.junit.Before
+import org.junit.Rule
 import org.junit.runner.RunWith
 import org.robolectric.RobolectricTestRunner
 
+@ObsoleteCoroutinesApi
+@ExperimentalCoroutinesApi
 @RunWith(RobolectricTestRunner::class)
 class EventMetricTypeTest {
 
+    @get:Rule
+    val fakeDispatchers = FakeDispatchersInTest()
+
     @Before
     fun setUp() {
         EventsStorageEngine.clearAllStores()

components/service/glean/src/test/java/mozilla/components/service/glean/FakeDispatchersInTest.kt

@@ -0,0 +1,33 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+package mozilla.components.service.glean
+
+import kotlinx.coroutines.CoroutineScope
+import kotlinx.coroutines.ExperimentalCoroutinesApi
+import kotlinx.coroutines.ObsoleteCoroutinesApi
+import org.junit.rules.ExternalResource
+
+/**
+ * A class that stubs the coroutine scopes in [Dispatchers] to allow blocking
+ * on async API calls when running tests.
+ *
+ * Note: according to [kotlinx.coroutines.Dispatchers.Unconfined], this will work
+ * as long as the API doesn't use any suspending function.
+ */
+@ExperimentalCoroutinesApi @ObsoleteCoroutinesApi
+class FakeDispatchersInTest : ExternalResource() {
+    companion object {
+        private val testMainScope = CoroutineScope(kotlinx.coroutines.Dispatchers.Unconfined)
+        private val originalAPIScope = Dispatchers.API
+    }
+
+    override fun before() {
+        Dispatchers.API = testMainScope
+    }
+
+    override fun after() {
+        Dispatchers.API = originalAPIScope
+    }
+}

components/service/glean/src/test/java/mozilla/components/service/glean/GleanTest.kt

@@ -6,6 +6,8 @@ package mozilla.components.service.glean
 
 import android.content.Context
 import androidx.test.core.app.ApplicationProvider
+import kotlinx.coroutines.ExperimentalCoroutinesApi
+import kotlinx.coroutines.ObsoleteCoroutinesApi
 import kotlinx.coroutines.runBlocking
 import mozilla.components.service.glean.net.HttpPingUploader
 import mozilla.components.service.glean.storages.EventsStorageEngine
@@ -18,6 +20,7 @@ import org.junit.Assert.assertNotNull
 import org.junit.Assert.assertNull
 import org.junit.Assert.assertTrue
 import org.junit.Before
+import org.junit.Rule
 import org.junit.Test
 import org.junit.runner.RunWith
 import org.mockito.ArgumentMatchers.anyString
@@ -27,8 +30,14 @@ import org.robolectric.RobolectricTestRunner
 import java.io.File
 import java.util.UUID
 
+@ObsoleteCoroutinesApi
+@ExperimentalCoroutinesApi
 @RunWith(RobolectricTestRunner::class)
 class GleanTest {
+
+    @get:Rule
+    val fakeDispatchers = FakeDispatchersInTest()
+
     @Before
     fun setup() {
         Glean.initialize(

components/service/glean/src/test/java/mozilla/components/service/glean/StringMetricTypeTest.kt

@@ -6,17 +6,25 @@ package mozilla.components.service.glean
 
 import android.content.Context
 import androidx.test.core.app.ApplicationProvider
+import kotlinx.coroutines.ExperimentalCoroutinesApi
+import kotlinx.coroutines.ObsoleteCoroutinesApi
 import mozilla.components.service.glean.storages.StringsStorageEngine
 import org.junit.Assert.assertEquals
 import org.junit.Assert.assertNull
 import org.junit.Before
+import org.junit.Rule
 import org.junit.Test
 import org.junit.runner.RunWith
 import org.robolectric.RobolectricTestRunner
 
+@ObsoleteCoroutinesApi
+@ExperimentalCoroutinesApi
 @RunWith(RobolectricTestRunner::class)
 class StringMetricTypeTest {
 
+    @get:Rule
+    val fakeDispatchers = FakeDispatchersInTest()
+
     @Before
     fun setUp() {
         StringsStorageEngine.applicationContext = ApplicationProvider.getApplicationContext()

components/service/glean/src/test/java/mozilla/components/service/glean/UuidMetricTypeTest.kt

@@ -6,18 +6,26 @@ package mozilla.components.service.glean
 
 import android.content.Context
 import androidx.test.core.app.ApplicationProvider
+import kotlinx.coroutines.ExperimentalCoroutinesApi
+import kotlinx.coroutines.ObsoleteCoroutinesApi
 import mozilla.components.service.glean.storages.UuidsStorageEngine
 import org.junit.Assert.assertEquals
 import org.junit.Assert.assertNull
 import org.junit.Before
+import org.junit.Rule
 import org.junit.Test
 import org.junit.runner.RunWith
 import org.robolectric.RobolectricTestRunner
 import java.util.UUID
 
+@ObsoleteCoroutinesApi
+@ExperimentalCoroutinesApi
 @RunWith(RobolectricTestRunner::class)
 class UuidMetricTypeTest {
 
+    @get:Rule
+    val fakeDispatchers = FakeDispatchersInTest()
+
     @Before
     fun setUp() {
         UuidsStorageEngine.applicationContext = ApplicationProvider.getApplicationContext()
@@ -52,6 +60,7 @@ class UuidMetricTypeTest {
 
         val uuid2 = UUID.fromString("ce2adeb8-843a-4232-87a5-a099ed1e7bb3")
         uuidMetric.set(uuid2)
+
         // Check that data was properly recorded.
         val snapshot2 = UuidsStorageEngine.getSnapshot(storeName = "store1", clearStore = false)
         assertEquals(1, snapshot2!!.size)