Implement glean test API

- Implement test API functionality
- Make library tests more concise by using test API features
- Add tests for test API throwing NPE

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

@@ -4,6 +4,7 @@
 
 package mozilla.components.service.glean
 
+import android.support.annotation.VisibleForTesting
 import kotlinx.coroutines.launch
 import mozilla.components.service.glean.storages.BooleansStorageEngine
 import mozilla.components.support.base.log.logger.Logger
@@ -48,4 +49,31 @@ data class BooleanMetricType(
             )
         }
     }
+
+    /**
+     * Tests whether a value is stored for the metric for testing purposes only
+     *
+     * @param pingName represents the name of the ping to retrieve the metric for.  Defaults
+     *                 to the either the first value in [defaultStorageDestinations] or the first
+     *                 value in [sendInPings]
+     * @return true if metric value exists, otherwise false
+     */
+    @VisibleForTesting(otherwise = VisibleForTesting.NONE)
+    fun testHasValue(pingName: String = getStorageNames().first()): Boolean {
+        return BooleansStorageEngine.getSnapshot(pingName, false)?.get(identifier) != null
+    }
+
+    /**
+     * Returns the stored value for testing purposes only
+     *
+     * @param pingName represents the name of the ping to retrieve the metric for.  Defaults
+     *                 to the either the first value in [defaultStorageDestinations] or the first
+     *                 value in [sendInPings]
+     * @return value of the stored metric
+     * @throws [NullPointerException] if no value is stored
+     */
+    @VisibleForTesting(otherwise = VisibleForTesting.NONE)
+    fun testGetValue(pingName: String = getStorageNames().first()): Boolean {
+        return BooleansStorageEngine.getSnapshot(pingName, false)!![identifier]!!
+    }
 }

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

@@ -35,6 +35,8 @@ interface CommonMetricData {
     val name: String
     val sendInPings: List<String>
 
+    val identifier: String get() = if (category.isEmpty()) { name } else { "$category.$name" }
+
     companion object {
         internal const val DEFAULT_STORAGE_NAME = "default"
     }

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

@@ -4,6 +4,7 @@
 
 package mozilla.components.service.glean
 
+import android.support.annotation.VisibleForTesting
 import kotlinx.coroutines.launch
 import mozilla.components.service.glean.storages.CountersStorageEngine
 import mozilla.components.support.base.log.logger.Logger
@@ -55,4 +56,31 @@ data class CounterMetricType(
             )
         }
     }
+
+    /**
+     * Tests whether a value is stored for the metric for testing purposes only
+     *
+     * @param pingName represents the name of the ping to retrieve the metric for.  Defaults
+     *                 to the either the first value in [defaultStorageDestinations] or the first
+     *                 value in [sendInPings]
+     * @return true if metric value exists, otherwise false
+     */
+    @VisibleForTesting(otherwise = VisibleForTesting.NONE)
+    fun testHasValue(pingName: String = getStorageNames().first()): Boolean {
+        return CountersStorageEngine.getSnapshot(pingName, false)?.get(identifier) != null
+    }
+
+    /**
+     * Returns the stored value for testing purposes only
+     *
+     * @param pingName represents the name of the ping to retrieve the metric for.  Defaults
+     *                 to the either the first value in [defaultStorageDestinations] or the first
+     *                 value in [sendInPings]
+     * @return value of the stored metric
+     * @throws [NullPointerException] if no value is stored
+     */
+    @VisibleForTesting(otherwise = VisibleForTesting.NONE)
+    fun testGetValue(pingName: String = getStorageNames().first()): Int {
+        return CountersStorageEngine.getSnapshot(pingName, false)!![identifier]!!
+    }
 }

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

@@ -4,8 +4,10 @@
 
 package mozilla.components.service.glean
 
+import android.support.annotation.VisibleForTesting
 import kotlinx.coroutines.launch
 import mozilla.components.service.glean.storages.EventsStorageEngine
+import mozilla.components.service.glean.storages.RecordedEventData
 import mozilla.components.support.base.log.logger.Logger
 
 /**
@@ -109,4 +111,36 @@ data class EventMetricType(
             )
         }
     }
+
+    /**
+     * Tests whether a value is stored for the metric for testing purposes only
+     *
+     * @param pingName represents the name of the ping to retrieve the metric for.  Defaults
+     *                 to the either the first value in [defaultStorageDestinations] or the first
+     *                 value in [sendInPings]
+     * @return true if metric value exists, otherwise false
+     */
+    @VisibleForTesting(otherwise = VisibleForTesting.NONE)
+    fun testHasValue(pingName: String = getStorageNames().first()): Boolean {
+        val snapshot = EventsStorageEngine.getSnapshot(pingName, false) ?: return false
+        return snapshot.any { event ->
+            event.identifier == identifier
+        }
+    }
+
+    /**
+     * Returns the stored value for testing purposes only
+     *
+     * @param pingName represents the name of the ping to retrieve the metric for.  Defaults
+     *                 to the either the first value in [defaultStorageDestinations] or the first
+     *                 value in [sendInPings]
+     * @return value of the stored metric
+     * @throws [NullPointerException] if no value is stored
+     */
+    @VisibleForTesting(otherwise = VisibleForTesting.NONE)
+    fun testGetValue(pingName: String = getStorageNames().first()): List<RecordedEventData> {
+        return EventsStorageEngine.getSnapshot(pingName, false)!!.filter { event ->
+            event.identifier == identifier
+        }
+    }
 }

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

@@ -269,6 +269,15 @@ open class GleanInternalAPI internal constructor () {
             }
         }
     }
+
+    /**
+     * Test only function to clear all storages and metrics.  Note that this also includes 'user'
+     * lifetime metrics so be aware that things like clientId will be wiped as well.
+     */
+    @VisibleForTesting(otherwise = VisibleForTesting.NONE)
+    fun testClearAllData() {
+        storageEngineManager.clearAllStores()
+    }
 }
 
 object Glean : GleanInternalAPI() {

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

@@ -4,6 +4,7 @@
 
 package mozilla.components.service.glean
 
+import android.support.annotation.VisibleForTesting
 import kotlinx.coroutines.launch
 import mozilla.components.service.glean.storages.StringListsStorageEngine
 import mozilla.components.support.base.log.logger.Logger
@@ -100,4 +101,31 @@ data class StringListMetricType(
             )
         }
     }
+
+    /**
+     * Tests whether a value is stored for the metric for testing purposes only
+     *
+     * @param pingName represents the name of the ping to retrieve the metric for.  Defaults
+     *                 to the either the first value in [defaultStorageDestinations] or the first
+     *                 value in [sendInPings]
+     * @return true if metric value exists, otherwise false
+     */
+    @VisibleForTesting(otherwise = VisibleForTesting.NONE)
+    fun testHasValue(pingName: String = getStorageNames().first()): Boolean {
+        return StringListsStorageEngine.getSnapshot(pingName, false)?.get(identifier) != null
+    }
+
+    /**
+     * Returns the stored value for testing purposes only
+     *
+     * @param pingName represents the name of the ping to retrieve the metric for.  Defaults
+     *                 to the either the first value in [defaultStorageDestinations] or the first
+     *                 value in [sendInPings]
+     * @return value of the stored metric
+     * @throws [NullPointerException] if no value is stored
+     */
+    @VisibleForTesting(otherwise = VisibleForTesting.NONE)
+    fun testGetValue(pingName: String = getStorageNames().first()): List<String> {
+        return StringListsStorageEngine.getSnapshot(pingName, false)!![identifier]!!
+    }
 }

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

@@ -4,6 +4,7 @@
 
 package mozilla.components.service.glean
 
+import android.support.annotation.VisibleForTesting
 import kotlinx.coroutines.launch
 import mozilla.components.service.glean.storages.StringsStorageEngine
 import mozilla.components.support.base.log.logger.Logger
@@ -63,4 +64,31 @@ data class StringMetricType(
             )
         }
     }
+
+    /**
+     * Tests whether a value is stored for the metric for testing purposes only
+     *
+     * @param pingName represents the name of the ping to retrieve the metric for.  Defaults
+     *                 to the either the first value in [defaultStorageDestinations] or the first
+     *                 value in [sendInPings]
+     * @return true if metric value exists, otherwise false
+     */
+    @VisibleForTesting(otherwise = VisibleForTesting.NONE)
+    fun testHasValue(pingName: String = getStorageNames().first()): Boolean {
+        return StringsStorageEngine.getSnapshot(pingName, false)?.get(identifier) != null
+    }
+
+    /**
+     * Returns the stored value for testing purposes only
+     *
+     * @param pingName represents the name of the ping to retrieve the metric for.  Defaults
+     *                 to the either the first value in [defaultStorageDestinations] or the first
+     *                 value in [sendInPings]
+     * @return value of the stored metric
+     * @throws [NullPointerException] if no value is stored
+     */
+    @VisibleForTesting(otherwise = VisibleForTesting.NONE)
+    fun testGetValue(pingName: String = getStorageNames().first()): String {
+        return StringsStorageEngine.getSnapshot(pingName, false)!![identifier]!!
+    }
 }

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

@@ -4,6 +4,7 @@
 
 package mozilla.components.service.glean
 
+import android.support.annotation.VisibleForTesting
 import mozilla.components.service.glean.storages.TimespansStorageEngine
 import mozilla.components.support.base.log.logger.Logger
 
@@ -63,4 +64,31 @@ data class TimespanMetricType(
 
         TimespansStorageEngine.cancel(this)
     }
+
+    /**
+     * Tests whether a value is stored for the metric for testing purposes only
+     *
+     * @param pingName represents the name of the ping to retrieve the metric for.  Defaults
+     *                 to the either the first value in [defaultStorageDestinations] or the first
+     *                 value in [sendInPings]
+     * @return true if metric value exists, otherwise false
+     */
+    @VisibleForTesting(otherwise = VisibleForTesting.NONE)
+    fun testHasValue(pingName: String = getStorageNames().first()): Boolean {
+        return TimespansStorageEngine.getSnapshot(pingName, false)?.get(identifier) != null
+    }
+
+    /**
+     * Returns the stored value for testing purposes only
+     *
+     * @param pingName represents the name of the ping to retrieve the metric for.  Defaults
+     *                 to the either the first value in [defaultStorageDestinations] or the first
+     *                 value in [sendInPings]
+     * @return value of the stored metric
+     * @throws [NullPointerException] if no value is stored
+     */
+    @VisibleForTesting(otherwise = VisibleForTesting.NONE)
+    fun testGetValue(pingName: String = getStorageNames().first()): Long {
+        return TimespansStorageEngine.getSnapshot(pingName, false)!![identifier]!!
+    }
 }

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

@@ -4,6 +4,7 @@
 
 package mozilla.components.service.glean
 
+import android.support.annotation.VisibleForTesting
 import kotlinx.coroutines.launch
 import java.util.UUID
 
@@ -68,4 +69,32 @@ data class UuidMetricType(
             )
         }
     }
+
+    /**
+     * Tests whether a value is stored for the metric for testing purposes only
+     *
+     * @param pingName represents the name of the ping to retrieve the metric for.  Defaults
+     *                 to the either the first value in [defaultStorageDestinations] or the first
+     *                 value in [sendInPings]
+     * @return true if metric value exists, otherwise false
+     */
+    @VisibleForTesting(otherwise = VisibleForTesting.NONE)
+    fun testHasValue(pingName: String = getStorageNames().first()): Boolean {
+        return UuidsStorageEngine
+            .getSnapshot(pingName, false)?.get(identifier) != null
+    }
+
+    /**
+     * Returns the stored value for testing purposes only
+     *
+     * @param pingName represents the name of the ping to retrieve the metric for.  Defaults
+     *                 to the either the first value in [defaultStorageDestinations] or the first
+     *                 value in [sendInPings]
+     * @return value of the stored metric
+     * @throws [NullPointerException] if no value is stored
+     */
+    @VisibleForTesting(otherwise = VisibleForTesting.NONE)
+    fun testGetValue(pingName: String = getStorageNames().first()): UUID {
+        return UuidsStorageEngine.getSnapshot(pingName, false)!![identifier]!!
+    }
 }

components/service/glean/src/main/java/mozilla/components/service/glean/storages/EventsStorageEngine.kt

@@ -34,15 +34,6 @@ internal object EventsStorageEngine : StorageEngine {
     // to the docs, the used clock is guaranteed to be monotonic.
     private val startTime: Long = SystemClock.elapsedRealtime()
 
-    data class RecordedEventData(
-        val category: String,
-        val name: String,
-        val objectId: String,
-        val msSinceStart: Long,
-        val value: String? = null,
-        val extra: Map<String, String>? = null
-    )
-
     /**
      * Record an event in the desired stores.
      *
@@ -55,7 +46,7 @@ internal object EventsStorageEngine : StorageEngine {
      *              context if needed
      */
     @Suppress("LongParameterList")
-    public fun record(
+    fun record(
         stores: List<String>,
         category: String,
         name: String,
@@ -88,7 +79,7 @@ internal object EventsStorageEngine : StorageEngine {
      * @return the list of events recorded in the requested store
      */
     @Synchronized
-    public fun getSnapshot(storeName: String, clearStore: Boolean): List<RecordedEventData>? {
+    fun getSnapshot(storeName: String, clearStore: Boolean): List<RecordedEventData>? {
         if (clearStore) {
             return eventStores.remove(storeName)
         }
@@ -124,8 +115,20 @@ internal object EventsStorageEngine : StorageEngine {
     override val sendAsTopLevelField: Boolean
         get() = true
 
-    @VisibleForTesting
-    internal fun clearAllStores() {
+    @VisibleForTesting(otherwise = VisibleForTesting.NONE)
+    override fun clearAllStores() {
         eventStores.clear()
     }
 }
+
+@VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
+data class RecordedEventData(
+    val category: String,
+    val name: String,
+    val objectId: String,
+    val msSinceStart: Long,
+    val value: String? = null,
+    val extra: Map<String, String>? = null,
+
+    internal val identifier: String = if (category.isEmpty()) { name } else { "$category.$name" }
+)

components/service/glean/src/main/java/mozilla/components/service/glean/storages/ExperimentsStorageEngine.kt

@@ -136,8 +136,8 @@ internal object ExperimentsStorageEngine : StorageEngine {
     override val sendAsTopLevelField: Boolean
         get() = true
 
-    @VisibleForTesting
-    internal fun clearAllStores() {
+    @VisibleForTesting(otherwise = VisibleForTesting.NONE)
+    override fun clearAllStores() {
         experiments.clear()
     }
 }

components/service/glean/src/main/java/mozilla/components/service/glean/storages/GenericScalarStorageEngine.kt

@@ -7,6 +7,7 @@ package mozilla.components.service.glean.storages
 import android.annotation.SuppressLint
 import android.content.Context
 import android.content.SharedPreferences
+import android.support.annotation.VisibleForTesting
 import mozilla.components.service.glean.Lifetime
 import mozilla.components.service.glean.CommonMetricData
 import mozilla.components.support.base.log.logger.Logger
@@ -77,20 +78,6 @@ internal abstract class GenericScalarStorageEngine<ScalarType> : StorageEngine {
         extraSerializationData: Any?
     )
 
-    /**
-     * Helper function to return the name of the stored metric.
-     * This is used to support empty categories.
-     */
-    protected fun getStoredName(metricData: CommonMetricData): String {
-        with(metricData) {
-            return if (category.isEmpty()) {
-                name
-            } else {
-                "$category.$name"
-            }
-        }
-    }
-
     /**
      * Deserialize the metrics with a lifetime = User that are on disk.
      * This will be called the first time a metric is used or before a snapshot is
@@ -229,7 +216,7 @@ internal abstract class GenericScalarStorageEngine<ScalarType> : StorageEngine {
             val storeData = dataStores[metricData.lifetime.ordinal].getOrPut(it) { mutableMapOf() }
             // We support empty categories for enabling the internal use of metrics
             // when assembling pings in [PingMaker].
-            val entryName = getStoredName(metricData)
+            val entryName = metricData.identifier
             val combinedValue = combine(storeData[entryName], value)
             storeData[entryName] = combinedValue
             // Persist data with "user" lifetime
@@ -245,5 +232,9 @@ internal abstract class GenericScalarStorageEngine<ScalarType> : StorageEngine {
         userPrefs?.apply()
     }
 
-    internal open fun clearAllStores() = dataStores.forEach { it.clear() }
+    @VisibleForTesting(otherwise = VisibleForTesting.NONE)
+    override fun clearAllStores() {
+        userLifetimeStorage.edit().clear().apply()
+        dataStores.forEach { it.clear() }
+    }
 }

components/service/glean/src/main/java/mozilla/components/service/glean/storages/StorageEngine.kt

@@ -5,6 +5,7 @@
 package mozilla.components.service.glean.storages
 
 import android.content.Context
+import android.support.annotation.VisibleForTesting
 
 /**
  * Base interface intended to be implemented by the different
@@ -25,6 +26,12 @@ internal interface StorageEngine {
      */
     fun getSnapshotAsJSON(storeName: String, clearStore: Boolean): Any?
 
+    /**
+     * Clear all stored data in the storage engine
+     */
+    @VisibleForTesting(otherwise = VisibleForTesting.NONE)
+    fun clearAllStores()
+
     /**
      * Indicate whether this storage engine is sent at the top level of the ping
      * (rather than in the metrics section).

components/service/glean/src/main/java/mozilla/components/service/glean/storages/StorageEngineManager.kt

@@ -5,6 +5,7 @@
 package mozilla.components.service.glean.storages
 
 import android.content.Context
+import android.support.annotation.VisibleForTesting
 import org.json.JSONObject
 
 /**
@@ -55,4 +56,11 @@ internal class StorageEngineManager(
 
         return jsonPing
     }
+
+    @VisibleForTesting(otherwise = VisibleForTesting.NONE)
+    fun clearAllStores() {
+        for (storageEngine in storageEngines) {
+            storageEngine.value.clearAllStores()
+        }
+    }
 }

components/service/glean/src/main/java/mozilla/components/service/glean/storages/TimespansStorageEngine.kt

@@ -7,6 +7,7 @@ package mozilla.components.service.glean.storages
 import android.annotation.SuppressLint
 import android.content.SharedPreferences
 import android.os.SystemClock
+import android.support.annotation.VisibleForTesting
 import mozilla.components.service.glean.CommonMetricData
 import mozilla.components.service.glean.TimeUnit
 
@@ -138,7 +139,7 @@ internal open class TimespansStorageEngineImplementation(
      * @param metricData the metric information for the timespan
      */
     fun start(metricData: CommonMetricData) {
-        val timespanName = getStoredName(metricData)
+        val timespanName = metricData.identifier
 
         if (timespanName in uncommittedStartTimes) {
             // TODO report errors if already tracking time through internal metrics. See bug 1499761.
@@ -165,7 +166,7 @@ internal open class TimespansStorageEngineImplementation(
     ) {
         // TODO report errors if not tracking time through internal metrics. See bug 1499761.
         // Look for the start time: if it's there, commit the timespan.
-        val timespanName = getStoredName(metricData)
+        val timespanName = metricData.identifier
         uncommittedStartTimes.remove(timespanName)?.let { startTime ->
             val elapsedNanos = getElapsedNanos() - startTime
 
@@ -190,7 +191,7 @@ internal open class TimespansStorageEngineImplementation(
      */
     @Synchronized
     fun cancel(metricData: CommonMetricData) {
-        uncommittedStartTimes.remove(getStoredName(metricData))
+        uncommittedStartTimes.remove(metricData.identifier)
     }
 
     /**
@@ -253,6 +254,7 @@ internal open class TimespansStorageEngineImplementation(
     /**
      * Test-only method used to clear the timespans stores.
      */
+    @VisibleForTesting(otherwise = VisibleForTesting.NONE)
     override fun clearAllStores() {
         super.clearAllStores()
         timeUnitsMap.clear()

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

@@ -9,13 +9,15 @@ import androidx.test.core.app.ApplicationProvider
 import kotlinx.coroutines.ExperimentalCoroutinesApi
 import kotlinx.coroutines.ObsoleteCoroutinesApi
 import mozilla.components.service.glean.storages.BooleansStorageEngine
-import org.junit.Assert.assertNull
 import org.junit.Assert.assertEquals
+import org.junit.Assert.assertTrue
+import org.junit.Assert.assertFalse
 import org.junit.Before
 import org.junit.Rule
 import org.junit.Test
 import org.junit.runner.RunWith
 import org.robolectric.RobolectricTestRunner
+import java.lang.NullPointerException
 
 @ObsoleteCoroutinesApi
 @ExperimentalCoroutinesApi
@@ -64,19 +66,14 @@ class BooleanMetricTypeTest {
 
         // Record two booleans of the same type, with a little delay.
         booleanMetric.set(true)
-
         // Check that data was properly recorded.
-        val snapshot = BooleansStorageEngine.getSnapshot(storeName = "store1", clearStore = false)
-        assertEquals(1, snapshot!!.size)
-        assertEquals(true, snapshot.containsKey("telemetry.boolean_metric"))
-        assertEquals(true, snapshot.get("telemetry.boolean_metric"))
+        assertTrue(booleanMetric.testHasValue())
+        assertTrue(booleanMetric.testGetValue())
 
         booleanMetric.set(false)
         // Check that data was properly recorded.
-        val snapshot2 = BooleansStorageEngine.getSnapshot(storeName = "store1", clearStore = false)
-        assertEquals(1, snapshot2!!.size)
-        assertEquals(true, snapshot2.containsKey("telemetry.boolean_metric"))
-        assertEquals(false, snapshot2.get("telemetry.boolean_metric"))
+        assertTrue(booleanMetric.testHasValue())
+        assertFalse(booleanMetric.testGetValue())
     }
 
     @Test
@@ -94,7 +91,43 @@ class BooleanMetricTypeTest {
         // Attempt to store the boolean.
         booleanMetric.set(true)
         // Check that nothing was recorded.
-        val snapshot = BooleansStorageEngine.getSnapshot(storeName = "store1", clearStore = false)
-        assertNull("Booleans must not be recorded if they are disabled", snapshot)
+        assertFalse(booleanMetric.testHasValue())
+    }
+
+    @Test(expected = NullPointerException::class)
+    fun `testGetValue() throws NullPointerException if nothing is stored`() {
+        // Define a 'booleanMetric' boolean metric to have an instance to call