From 4389da78119acca75fbbd6345e6fdaa76aa90a4c Mon Sep 17 00:00:00 2001 From: Arturo Mejia Date: Sat, 26 Mar 2022 19:33:40 -0400 Subject: [PATCH] For #24222: Persist user interactions with nimbus messages --- .../org/mozilla/fenix/FenixApplication.kt | 8 +- .../org/mozilla/fenix/components/Analytics.kt | 4 +- .../KeyPairMessageMetadataStorage.kt | 29 -- .../org/mozilla/fenix/gleanplumb/Message.kt | 4 +- .../gleanplumb/MessageMetadataStorage.kt | 6 +- .../gleanplumb/NimbusMessagingStorage.kt | 11 +- .../OnDiskMessageMetadataStorage.kt | 95 ++++++ .../gleanplumb/state/MessagingMiddleware.kt | 37 ++- .../gleanplumb/NimbusMessagingStorageTest.kt | 296 +++++++++--------- .../OnDiskMessageMetadataStorageTest.kt | 144 +++++++++ .../state/MessagingMiddlewareTest.kt | 32 +- 11 files changed, 458 insertions(+), 208 deletions(-) delete mode 100644 app/src/main/java/org/mozilla/fenix/gleanplumb/KeyPairMessageMetadataStorage.kt create mode 100644 app/src/main/java/org/mozilla/fenix/gleanplumb/OnDiskMessageMetadataStorage.kt create mode 100644 app/src/test/java/org/mozilla/fenix/gleanplumb/OnDiskMessageMetadataStorageTest.kt diff --git a/app/src/main/java/org/mozilla/fenix/FenixApplication.kt b/app/src/main/java/org/mozilla/fenix/FenixApplication.kt index 8fbfc87e2..2313d0332 100644 --- a/app/src/main/java/org/mozilla/fenix/FenixApplication.kt +++ b/app/src/main/java/org/mozilla/fenix/FenixApplication.kt @@ -160,9 +160,6 @@ open class FenixApplication : LocaleAwareApplication(), Provider { GlobalScope.launch(Dispatchers.IO) { setStartupMetrics(store, settings()) } - if (FeatureFlags.messagingFeature && settings().isExperimentationEnabled) { - components.appStore.dispatch(AppAction.MessagingAction.Restore) - } } @CallSuper @@ -755,6 +752,11 @@ open class FenixApplication : LocaleAwareApplication(), Provider { } ) components.analytics.experiments.register(object : NimbusInterface.Observer { + override fun onExperimentsFetched() { + if (FeatureFlags.messagingFeature && settings().isExperimentationEnabled) { + components.appStore.dispatch(AppAction.MessagingAction.Restore) + } + } override fun onUpdatesApplied(updated: List) { CustomizeHome.jumpBackIn.set(settings.showRecentTabsFeature) CustomizeHome.recentlySaved.set(settings.showRecentBookmarksFeature) diff --git a/app/src/main/java/org/mozilla/fenix/components/Analytics.kt b/app/src/main/java/org/mozilla/fenix/components/Analytics.kt index 6621a286d..b58c3ec5c 100644 --- a/app/src/main/java/org/mozilla/fenix/components/Analytics.kt +++ b/app/src/main/java/org/mozilla/fenix/components/Analytics.kt @@ -26,7 +26,7 @@ import org.mozilla.fenix.components.metrics.GleanMetricsService import org.mozilla.fenix.components.metrics.MetricController import org.mozilla.fenix.experiments.createNimbus import org.mozilla.fenix.ext.settings -import org.mozilla.fenix.gleanplumb.KeyPairMessageMetadataStorage +import org.mozilla.fenix.gleanplumb.OnDiskMessageMetadataStorage import org.mozilla.fenix.gleanplumb.NimbusMessagingStorage import org.mozilla.fenix.nimbus.FxNimbus import org.mozilla.fenix.perf.lazyMonitored @@ -123,7 +123,7 @@ class Analytics( val messagingStorage by lazyMonitored { NimbusMessagingStorage( context = context, - metadataStorage = KeyPairMessageMetadataStorage(), + metadataStorage = OnDiskMessageMetadataStorage(context), gleanPlumb = experiments, reportMalformedMessage = { metrics.track(Event.Messaging.MessageMalformed(it)) diff --git a/app/src/main/java/org/mozilla/fenix/gleanplumb/KeyPairMessageMetadataStorage.kt b/app/src/main/java/org/mozilla/fenix/gleanplumb/KeyPairMessageMetadataStorage.kt deleted file mode 100644 index 3c6942117..000000000 --- a/app/src/main/java/org/mozilla/fenix/gleanplumb/KeyPairMessageMetadataStorage.kt +++ /dev/null @@ -1,29 +0,0 @@ -/* 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 org.mozilla.fenix.gleanplumb - -/* Dummy implementation until we provide full implementation. -* This will covered on https://github.com/mozilla-mobile/fenix/issues/24222 -* */ -class KeyPairMessageMetadataStorage : MessageMetadataStorage { - override fun getMetadata(): List { - return listOf( - Message.Metadata( - id = "eu-tracking-protection-for-ireland", - displayCount = 0, - pressed = false, - dismissed = false - ) - ) - } - - override fun addMetadata(metadata: Message.Metadata): Message.Metadata { - return metadata - } - - @SuppressWarnings("EmptyFunctionBlock") - override fun updateMetadata(metadata: Message.Metadata) { - } -} diff --git a/app/src/main/java/org/mozilla/fenix/gleanplumb/Message.kt b/app/src/main/java/org/mozilla/fenix/gleanplumb/Message.kt index 6d17f9054..26d75afc9 100644 --- a/app/src/main/java/org/mozilla/fenix/gleanplumb/Message.kt +++ b/app/src/main/java/org/mozilla/fenix/gleanplumb/Message.kt @@ -34,11 +34,13 @@ data class Message( * @param displayCount Indicates how many times a message is displayed. * @param pressed Indicates if a message has been clicked. * @param dismissed Indicates if a message has been closed. + * @param lastTimeShown A timestamp indicating when was the last time, the message was shown. */ data class Metadata( val id: String, val displayCount: Int = 0, val pressed: Boolean = false, - val dismissed: Boolean = false + val dismissed: Boolean = false, + val lastTimeShown: Long = 0L ) } diff --git a/app/src/main/java/org/mozilla/fenix/gleanplumb/MessageMetadataStorage.kt b/app/src/main/java/org/mozilla/fenix/gleanplumb/MessageMetadataStorage.kt index a9ca921d0..c30d8535d 100644 --- a/app/src/main/java/org/mozilla/fenix/gleanplumb/MessageMetadataStorage.kt +++ b/app/src/main/java/org/mozilla/fenix/gleanplumb/MessageMetadataStorage.kt @@ -8,16 +8,16 @@ interface MessageMetadataStorage { /** * Provide all the message metadata saved in the storage. */ - fun getMetadata(): List + suspend fun getMetadata(): Map /** * Given a [metadata] add the message metadata on the storage. * @return the added message on the [MessageMetadataStorage] */ - fun addMetadata(metadata: Message.Metadata): Message.Metadata + suspend fun addMetadata(metadata: Message.Metadata): Message.Metadata /** * Given a [metadata] update the message metadata on the storage. */ - fun updateMetadata(metadata: Message.Metadata) + suspend fun updateMetadata(metadata: Message.Metadata) } diff --git a/app/src/main/java/org/mozilla/fenix/gleanplumb/NimbusMessagingStorage.kt b/app/src/main/java/org/mozilla/fenix/gleanplumb/NimbusMessagingStorage.kt index d1d47e04f..ede523f6a 100644 --- a/app/src/main/java/org/mozilla/fenix/gleanplumb/NimbusMessagingStorage.kt +++ b/app/src/main/java/org/mozilla/fenix/gleanplumb/NimbusMessagingStorage.kt @@ -33,16 +33,14 @@ class NimbusMessagingStorage( /** * Returns a list of available messages descending sorted by their priority. */ - fun getMessages(): List { + suspend fun getMessages(): List { val nimbusTriggers = nimbusFeature.triggers val nimbusStyles = nimbusFeature.styles val nimbusActions = nimbusFeature.actions val nimbusMessages = nimbusFeature.messages val defaultStyle = StyleData(context) - val storageMetadata = metadataStorage.getMetadata().associateBy { - it.id - } + val storageMetadata = metadataStorage.getMetadata() return nimbusMessages.mapNotNull { (key, value) -> val action = sanitizeAction(key, value.action, nimbusActions) ?: return@mapNotNull null @@ -98,7 +96,7 @@ class NimbusMessagingStorage( /** * Updated the provided [metadata] in the storage. */ - fun updateMetadata(metadata: Message.Metadata) { + suspend fun updateMetadata(metadata: Message.Metadata) { metadataStorage.updateMetadata(metadata) } @@ -167,8 +165,7 @@ class NimbusMessagingStorage( } } - private fun addMetadata(id: String): Message.Metadata { - // This will be improve on https://github.com/mozilla-mobile/fenix/issues/24222 + private suspend fun addMetadata(id: String): Message.Metadata { return metadataStorage.addMetadata( Message.Metadata( id = id, diff --git a/app/src/main/java/org/mozilla/fenix/gleanplumb/OnDiskMessageMetadataStorage.kt b/app/src/main/java/org/mozilla/fenix/gleanplumb/OnDiskMessageMetadataStorage.kt new file mode 100644 index 000000000..b87f3023f --- /dev/null +++ b/app/src/main/java/org/mozilla/fenix/gleanplumb/OnDiskMessageMetadataStorage.kt @@ -0,0 +1,95 @@ +/* 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 org.mozilla.fenix.gleanplumb + +import android.content.Context +import android.util.AtomicFile +import androidx.annotation.VisibleForTesting +import mozilla.components.support.ktx.util.readAndDeserialize +import mozilla.components.support.ktx.util.writeString +import org.json.JSONArray +import org.json.JSONObject +import java.io.File + +internal const val FILE_NAME = "nimbus_messages_metadata.json" + +/** + * A storage that persists [Message.Metadata] into disk. + */ +class OnDiskMessageMetadataStorage( + private val context: Context +) : MessageMetadataStorage { + private val diskCacheLock = Any() + + @VisibleForTesting + internal var metadataMap: MutableMap = hashMapOf() + + override suspend fun getMetadata(): Map { + if (metadataMap.isEmpty()) { + metadataMap = readFromDisk().toMutableMap() + } + return metadataMap + } + + override suspend fun addMetadata(metadata: Message.Metadata): Message.Metadata { + metadataMap[metadata.id] = metadata + writeToDisk() + return metadata + } + + override suspend fun updateMetadata(metadata: Message.Metadata) { + addMetadata(metadata) + } + + @VisibleForTesting + internal fun readFromDisk(): Map { + synchronized(diskCacheLock) { + return getFile().readAndDeserialize { + JSONArray(it).toMetadataMap() + } ?: emptyMap() + } + } + + @VisibleForTesting + internal fun writeToDisk() { + synchronized(diskCacheLock) { + val json = metadataMap.values.toList().fold("") { acc, next -> + if (acc.isEmpty()) { + next.toJson() + } else { + "$acc,${next.toJson()}" + } + } + getFile().writeString { "[$json]" } + } + } + + private fun getFile(): AtomicFile { + return AtomicFile(File(context.filesDir, FILE_NAME)) + } +} + +internal fun JSONArray.toMetadataMap(): Map { + return (0 until length()).map { index -> + getJSONObject(index).toMetadata() + }.associateBy { + it.id + } +} + +@Suppress("MaxLineLength") // To avoid adding any extra space to the string. +internal fun Message.Metadata.toJson(): String { + return """{"id":"$id","displayCount":$displayCount,"pressed":$pressed,"dismissed":$dismissed,"lastTimeShown":$lastTimeShown}""" +} + +internal fun JSONObject.toMetadata(): Message.Metadata { + return Message.Metadata( + id = optString("id"), + displayCount = optInt("displayCount"), + pressed = optBoolean("pressed"), + dismissed = optBoolean("dismissed"), + lastTimeShown = optLong("lastTimeShown") + ) +} diff --git a/app/src/main/java/org/mozilla/fenix/gleanplumb/state/MessagingMiddleware.kt b/app/src/main/java/org/mozilla/fenix/gleanplumb/state/MessagingMiddleware.kt index 5ccfccb81..45e1a45fa 100644 --- a/app/src/main/java/org/mozilla/fenix/gleanplumb/state/MessagingMiddleware.kt +++ b/app/src/main/java/org/mozilla/fenix/gleanplumb/state/MessagingMiddleware.kt @@ -5,6 +5,9 @@ package org.mozilla.fenix.gleanplumb.state import androidx.annotation.VisibleForTesting +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.launch import mozilla.components.lib.state.Middleware import mozilla.components.lib.state.MiddlewareContext import org.mozilla.fenix.components.appstate.AppAction @@ -23,7 +26,8 @@ import org.mozilla.fenix.gleanplumb.NimbusMessagingStorage typealias AppStoreMiddlewareContext = MiddlewareContext class MessagingMiddleware( - private val messagingStorage: NimbusMessagingStorage + private val messagingStorage: NimbusMessagingStorage, + private val coroutineScope: CoroutineScope = CoroutineScope(Dispatchers.IO), ) : Middleware { override fun invoke( @@ -33,9 +37,10 @@ class MessagingMiddleware( ) { when (action) { is Restore -> { - val messages = messagingStorage.getMessages() - - context.dispatch(UpdateMessages(messages)) + coroutineScope.launch { + val messages = messagingStorage.getMessages() + context.store.dispatch(UpdateMessages(messages)) + } } is Evaluate -> { @@ -62,7 +67,8 @@ class MessagingMiddleware( context: AppStoreMiddlewareContext ) { val newMetadata = oldMessage.metadata.copy( - displayCount = oldMessage.metadata.displayCount + 1 + displayCount = oldMessage.metadata.displayCount + 1, + lastTimeShown = now() ) val newMessage = oldMessage.copy( metadata = newMetadata @@ -74,7 +80,9 @@ class MessagingMiddleware( removeMessage(context, oldMessage) } context.dispatch(UpdateMessages(newMessages)) - messagingStorage.updateMetadata(newMetadata) + coroutineScope.launch { + messagingStorage.updateMetadata(newMetadata) + } } @VisibleForTesting @@ -83,11 +91,12 @@ class MessagingMiddleware( message: Message ) { val newMessages = removeMessage(context, message) - val updatedMetadata = message.metadata.copy(dismissed = true) - - messagingStorage.updateMetadata(updatedMetadata) context.dispatch(UpdateMessages(newMessages)) consumeMessageToShowIfNeeded(context, message) + coroutineScope.launch { + val updatedMetadata = message.metadata.copy(dismissed = true) + messagingStorage.updateMetadata(updatedMetadata) + } } @VisibleForTesting @@ -96,9 +105,10 @@ class MessagingMiddleware( context: AppStoreMiddlewareContext ) { // Update Nimbus storage. - val updatedMetadata = message.metadata.copy(pressed = true) - messagingStorage.updateMetadata(updatedMetadata) - + coroutineScope.launch { + val updatedMetadata = message.metadata.copy(pressed = true) + messagingStorage.updateMetadata(updatedMetadata) + } // Update app state. val newMessages = removeMessage(context, message) context.dispatch(UpdateMessages(newMessages)) @@ -136,4 +146,7 @@ class MessagingMiddleware( } return removeMessage(context, oldMessage) + updatedMessage } + + @VisibleForTesting + internal fun now(): Long = System.currentTimeMillis() } diff --git a/app/src/test/java/org/mozilla/fenix/gleanplumb/NimbusMessagingStorageTest.kt b/app/src/test/java/org/mozilla/fenix/gleanplumb/NimbusMessagingStorageTest.kt index 20dc08b52..316cf18ca 100644 --- a/app/src/test/java/org/mozilla/fenix/gleanplumb/NimbusMessagingStorageTest.kt +++ b/app/src/test/java/org/mozilla/fenix/gleanplumb/NimbusMessagingStorageTest.kt @@ -4,10 +4,13 @@ package org.mozilla.fenix.gleanplumb +import io.mockk.coEvery import io.mockk.every import io.mockk.mockk import io.mockk.spyk import io.mockk.verify +import kotlinx.coroutines.test.TestCoroutineScope +import kotlinx.coroutines.test.runBlockingTest import mozilla.components.support.test.mock import mozilla.components.support.test.robolectric.testContext import org.junit.Assert.assertEquals @@ -37,11 +40,11 @@ class NimbusMessagingStorageTest { private lateinit var gleanPlumb: GleanPlumbInterface private lateinit var messagingFeature: FeatureHolder private lateinit var messaging: Messaging + private val coroutineScope = TestCoroutineScope() private var malformedWasReported = false private val reportMalformedMessage: (String) -> Unit = { malformedWasReported = true } - @Before fun setup() { gleanPlumb = mockk(relaxed = true) @@ -49,7 +52,7 @@ class NimbusMessagingStorageTest { malformedWasReported = false messagingFeature = createMessagingFeature() - every { metadataStorage.getMetadata() } returns listOf(Message.Metadata(id = "message-1")) + coEvery { metadataStorage.getMetadata() } returns mapOf("message-1" to Message.Metadata(id = "message-1")) storage = NimbusMessagingStorage( testContext, @@ -61,7 +64,7 @@ class NimbusMessagingStorageTest { } @Test - fun `WHEN calling getMessages THEN provide a list of available messages`() { + fun `WHEN calling getMessages THEN provide a list of available messages`() = runBlockingTest { val message = storage.getMessages().first() assertEquals("message-1", message.id) @@ -69,150 +72,161 @@ class NimbusMessagingStorageTest { } @Test - fun `WHEN calling getMessages THEN provide a list of sorted messages by priority`() { - val messages = mapOf( - "low-message" to createMessageData(style = "low-priority"), - "high-message" to createMessageData(style = "high-priority"), - "medium-message" to createMessageData(style = "medium-priority"), - ) - val styles = mapOf( - "high-priority" to createStyle(priority = 100), - "medium-priority" to createStyle(priority = 50), - "low-priority" to createStyle(priority = 1) - ) - val metadataStorage: MessageMetadataStorage = mockk(relaxed = true) - val messagingFeature = createMessagingFeature( - styles = styles, - messages = messages - ) - - every { metadataStorage.getMetadata() } returns listOf(Message.Metadata(id = "message-1")) - - val storage = NimbusMessagingStorage( - testContext, - metadataStorage, - reportMalformedMessage, - gleanPlumb, - messagingFeature - ) - - val results = storage.getMessages() - - assertEquals("high-message", results[0].id) - assertEquals("medium-message", results[1].id) - assertEquals("low-message", results[2].id) - } + fun `WHEN calling getMessages THEN provide a list of sorted messages by priority`() = + runBlockingTest { + val messages = mapOf( + "low-message" to createMessageData(style = "low-priority"), + "high-message" to createMessageData(style = "high-priority"), + "medium-message" to createMessageData(style = "medium-priority"), + ) + val styles = mapOf( + "high-priority" to createStyle(priority = 100), + "medium-priority" to createStyle(priority = 50), + "low-priority" to createStyle(priority = 1) + ) + val metadataStorage: MessageMetadataStorage = mockk(relaxed = true) + val messagingFeature = createMessagingFeature( + styles = styles, + messages = messages + ) + + coEvery { metadataStorage.getMetadata() } returns mapOf( + "message-1" to Message.Metadata( + id = "message-1" + ) + ) + + val storage = NimbusMessagingStorage( + testContext, + metadataStorage, + reportMalformedMessage, + gleanPlumb, + messagingFeature + ) + + val results = storage.getMessages() + + assertEquals("high-message", results[0].id) + assertEquals("medium-message", results[1].id) + assertEquals("low-message", results[2].id) + } @Test - fun `GIVEN pressed message WHEN calling getMessages THEN filter out the pressed message`() { - val metadataList = listOf( - Message.Metadata(id = "pressed-message", pressed = true), - Message.Metadata(id = "normal-message", pressed = false) - ) - val messages = mapOf( - "pressed-message" to createMessageData(style = "high-priority"), - "normal-message" to createMessageData(style = "high-priority"), - ) - val styles = mapOf( - "high-priority" to createStyle(priority = 100), - ) - val metadataStorage: MessageMetadataStorage = mockk(relaxed = true) - val messagingFeature = createMessagingFeature( - styles = styles, - messages = messages - ) - - every { metadataStorage.getMetadata() } returns metadataList - - val storage = NimbusMessagingStorage( - testContext, - metadataStorage, - reportMalformedMessage, - gleanPlumb, - messagingFeature - ) - - val results = storage.getMessages() - - assertEquals(1, results.size) - assertEquals("normal-message", results[0].id) - } + fun `GIVEN pressed message WHEN calling getMessages THEN filter out the pressed message`() = + runBlockingTest { + val metadataList = mapOf( + "pressed-message" to Message.Metadata(id = "pressed-message", pressed = true), + "normal-message" to Message.Metadata(id = "normal-message", pressed = false) + ) + val messages = mapOf( + "pressed-message" to createMessageData(style = "high-priority"), + "normal-message" to createMessageData(style = "high-priority"), + ) + val styles = mapOf( + "high-priority" to createStyle(priority = 100), + ) + val metadataStorage: MessageMetadataStorage = mockk(relaxed = true) + val messagingFeature = createMessagingFeature( + styles = styles, + messages = messages + ) + + coEvery { metadataStorage.getMetadata() } returns metadataList + + val storage = NimbusMessagingStorage( + testContext, + metadataStorage, + reportMalformedMessage, + gleanPlumb, + messagingFeature + ) + + val results = storage.getMessages() + + assertEquals(1, results.size) + assertEquals("normal-message", results[0].id) + } @Test - fun `GIVEN dismissed message WHEN calling getMessages THEN filter out the dismissed message`() { - val metadataList = listOf( - Message.Metadata(id = "dismissed-message", dismissed = true), - Message.Metadata(id = "normal-message", dismissed = false) - ) - val messages = mapOf( - "dismissed-message" to createMessageData(style = "high-priority"), - "normal-message" to createMessageData(style = "high-priority"), - ) - val styles = mapOf( - "high-priority" to createStyle(priority = 100), - ) - val metadataStorage: MessageMetadataStorage = mockk(relaxed = true) - val messagingFeature = createMessagingFeature( - styles = styles, - messages = messages - ) - - every { metadataStorage.getMetadata() } returns metadataList - - val storage = NimbusMessagingStorage( - testContext, - metadataStorage, - reportMalformedMessage, - gleanPlumb, - messagingFeature - ) - - val results = storage.getMessages() - - assertEquals(1, results.size) - assertEquals("normal-message", results[0].id) - } + fun `GIVEN dismissed message WHEN calling getMessages THEN filter out the dismissed message`() = + runBlockingTest { + val metadataList = mapOf( + "dismissed-message" to Message.Metadata(id = "dismissed-message", dismissed = true), + "normal-message" to Message.Metadata(id = "normal-message", dismissed = false) + ) + val messages = mapOf( + "dismissed-message" to createMessageData(style = "high-priority"), + "normal-message" to createMessageData(style = "high-priority"), + ) + val styles = mapOf( + "high-priority" to createStyle(priority = 100), + ) + val metadataStorage: MessageMetadataStorage = mockk(relaxed = true) + val messagingFeature = createMessagingFeature( + styles = styles, + messages = messages + ) + + coEvery { metadataStorage.getMetadata() } returns metadataList + + val storage = NimbusMessagingStorage( + testContext, + metadataStorage, + reportMalformedMessage, + gleanPlumb, + messagingFeature + ) + + val results = storage.getMessages() + + assertEquals(1, results.size) + assertEquals("normal-message", results[0].id) + } @Test - fun `GIVEN a message that the maxDisplayCount WHEN calling getMessages THEN filter out the message`() { - val metadataList = listOf( - Message.Metadata(id = "shown-many-times-message", displayCount = 10), - Message.Metadata(id = "normal-message", displayCount = 0) - ) - val messages = mapOf( - "shown-many-times-message" to createMessageData( - style = "high-priority", - maxDisplayCount = 2 - ), - "normal-message" to createMessageData(style = "high-priority"), - ) - val styles = mapOf( - "high-priority" to createStyle(priority = 100), - ) - val metadataStorage: MessageMetadataStorage = mockk(relaxed = true) - val messagingFeature = createMessagingFeature( - styles = styles, - messages = messages - ) - - every { metadataStorage.getMetadata() } returns metadataList - - val storage = NimbusMessagingStorage( - testContext, - metadataStorage, - reportMalformedMessage, - gleanPlumb, - messagingFeature - ) - - val results = storage.getMessages() - - assertEquals(1, results.size) - assertEquals("normal-message", results[0].id) - } + fun `GIVEN a message that the maxDisplayCount WHEN calling getMessages THEN filter out the message`() = + runBlockingTest { + val metadataList = mapOf( + "shown-many-times-message" to Message.Metadata( + id = "shown-many-times-message", + displayCount = 10 + ), + "normal-message" to Message.Metadata(id = "normal-message", displayCount = 0) + ) + val messages = mapOf( + "shown-many-times-message" to createMessageData( + style = "high-priority", + maxDisplayCount = 2 + ), + "normal-message" to createMessageData(style = "high-priority"), + ) + val styles = mapOf( + "high-priority" to createStyle(priority = 100), + ) + val metadataStorage: MessageMetadataStorage = mockk(relaxed = true) + val messagingFeature = createMessagingFeature( + styles = styles, + messages = messages + ) + + coEvery { metadataStorage.getMetadata() } returns metadataList + + val storage = NimbusMessagingStorage( + testContext, + metadataStorage, + reportMalformedMessage, + gleanPlumb, + messagingFeature + ) + + val results = storage.getMessages() + + assertEquals(1, results.size) + assertEquals("normal-message", results[0].id) + } @Test - fun `GIVEN a malformed message WHEN calling getMessages THEN provide a list of messages ignoring the malformed one`() { + fun `GIVEN a malformed message WHEN calling getMessages THEN provide a list of messages ignoring the malformed one`() = runBlockingTest { val messages = storage.getMessages() val firstMessage = messages.first() @@ -237,11 +251,11 @@ class NimbusMessagingStorageTest { } @Test - fun `WHEN calling updateMetadata THEN delegate to metadataStorage`() { + fun `WHEN calling updateMetadata THEN delegate to metadataStorage`() = runBlockingTest { storage.updateMetadata(mockk()) - verify { metadataStorage.updateMetadata(any()) } + coEvery { metadataStorage.updateMetadata(any()) } } @Test diff --git a/app/src/test/java/org/mozilla/fenix/gleanplumb/OnDiskMessageMetadataStorageTest.kt b/app/src/test/java/org/mozilla/fenix/gleanplumb/OnDiskMessageMetadataStorageTest.kt new file mode 100644 index 000000000..17ef24504 --- /dev/null +++ b/app/src/test/java/org/mozilla/fenix/gleanplumb/OnDiskMessageMetadataStorageTest.kt @@ -0,0 +1,144 @@ +/* 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 org.mozilla.fenix.gleanplumb + +import io.mockk.Runs +import io.mockk.coEvery +import io.mockk.coVerify +import io.mockk.just +import io.mockk.spyk +import io.mockk.verify +import kotlinx.coroutines.test.runBlockingTest +import mozilla.components.support.test.robolectric.testContext +import org.json.JSONArray +import org.json.JSONObject +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.mozilla.experiments.nimbus.GleanPlumbInterface +import org.mozilla.experiments.nimbus.internal.FeatureHolder +import org.mozilla.fenix.helpers.FenixRobolectricTestRunner +import org.mozilla.fenix.nimbus.Messaging + +@RunWith(FenixRobolectricTestRunner::class) +class OnDiskMessageMetadataStorageTest { + + private lateinit var storage: OnDiskMessageMetadataStorage + private lateinit var metadataStorage: MessageMetadataStorage + private lateinit var gleanPlumb: GleanPlumbInterface + private lateinit var messagingFeature: FeatureHolder + private lateinit var messaging: Messaging + + @Before + fun setup() { + storage = OnDiskMessageMetadataStorage( + testContext + ) + } + + @Test + fun `GIVEN metadata is not loaded from disk WHEN calling getMetadata THEN load it`() = + runBlockingTest { + val spiedStorage = spyk(storage) + + coEvery { spiedStorage.readFromDisk() } returns emptyMap() + + spiedStorage.getMetadata() + + verify { spiedStorage.readFromDisk() } + } + + @Test + fun `GIVEN metadata is loaded from disk WHEN calling getMetadata THEN do not load it from disk`() = + runBlockingTest { + val spiedStorage = spyk(storage) + + spiedStorage.metadataMap = hashMapOf("" to Message.Metadata("id")) + + spiedStorage.getMetadata() + + verify(exactly = 0) { spiedStorage.readFromDisk() } + } + + @Test + fun `WHEN calling addMetadata THEN add in memory and disk`() = runBlockingTest { + val spiedStorage = spyk(storage) + + assertTrue(spiedStorage.metadataMap.isEmpty()) + + coEvery { spiedStorage.writeToDisk() } just Runs + + spiedStorage.addMetadata(Message.Metadata("id")) + + assertFalse(spiedStorage.metadataMap.isEmpty()) + coVerify { spiedStorage.writeToDisk() } + } + + @Test + fun `WHEN calling updateMetadata THEN delegate to addMetadata`() = runBlockingTest { + val spiedStorage = spyk(storage) + val metadata = Message.Metadata("id") + coEvery { spiedStorage.writeToDisk() } just Runs + + spiedStorage.updateMetadata(metadata) + + coVerify { spiedStorage.addMetadata(metadata) } + } + + @Test + fun `WHEN calling toJson THEN return an string json representation`() { + val metadata = Message.Metadata( + id = "id", + displayCount = 1, + pressed = false, + dismissed = false, + lastTimeShown = 0L, + ) + + val expected = + """{"id":"id","displayCount":1,"pressed":false,"dismissed":false,"lastTimeShown":0}""" + + assertEquals(expected, metadata.toJson()) + } + + @Test + fun `WHEN calling toMetadata THEN return Metadata representation`() { + val json = + """{"id":"id","displayCount":1,"pressed":false,"dismissed":false,"lastTimeShown":0}""" + + val jsonObject = JSONObject(json) + + val metadata = Message.Metadata( + id = "id", + displayCount = 1, + pressed = false, + dismissed = false, + lastTimeShown = 0L, + ) + + assertEquals(metadata, jsonObject.toMetadata()) + } + + @Test + fun `WHEN calling toMetadataMap THEN return map representation`() { + val json = + """[{"id":"id","displayCount":1,"pressed":false,"dismissed":false,"lastTimeShown":0}]""" + + val jsonArray = JSONArray(json) + + val metadata = Message.Metadata( + id = "id", + displayCount = 1, + pressed = false, + dismissed = false, + lastTimeShown = 0L, + ) + + assertEquals(metadata, jsonArray.toMetadataMap()[metadata.id]) + } +} diff --git a/app/src/test/java/org/mozilla/fenix/gleanplumb/state/MessagingMiddlewareTest.kt b/app/src/test/java/org/mozilla/fenix/gleanplumb/state/MessagingMiddlewareTest.kt index 8377d5921..6c419f122 100644 --- a/app/src/test/java/org/mozilla/fenix/gleanplumb/state/MessagingMiddlewareTest.kt +++ b/app/src/test/java/org/mozilla/fenix/gleanplumb/state/MessagingMiddlewareTest.kt @@ -5,11 +5,14 @@ package org.mozilla.fenix.gleanplumb.state import io.mockk.Runs +import io.mockk.coEvery +import io.mockk.coVerify import io.mockk.every import io.mockk.just import io.mockk.mockk import io.mockk.spyk import io.mockk.verify +import kotlinx.coroutines.test.TestCoroutineScope import mozilla.components.lib.state.MiddlewareContext import mozilla.components.service.glean.testing.GleanTestRule import mozilla.components.support.test.robolectric.testContext @@ -38,6 +41,7 @@ import org.mozilla.fenix.nimbus.MessageData @RunWith(FenixRobolectricTestRunner::class) class MessagingMiddlewareTest { + private val coroutineScope = TestCoroutineScope() private lateinit var store: AppStore private lateinit var middleware: MessagingMiddleware private lateinit var messagingStorage: NimbusMessagingStorage @@ -48,10 +52,14 @@ class MessagingMiddlewareTest { @Before fun setUp() { + store = mockk(relaxed = true) messagingStorage = mockk(relaxed = true) middlewareContext = mockk(relaxed = true) + every { middlewareContext.store } returns store + middleware = MessagingMiddleware( - messagingStorage + messagingStorage, + coroutineScope ) } @@ -59,11 +67,11 @@ class MessagingMiddlewareTest { fun `WHEN Restore THEN get messages from the storage and UpdateMessages`() { val messages: List = emptyList() - every { messagingStorage.getMessages() } returns messages + coEvery { messagingStorage.getMessages() } returns messages middleware.invoke(middlewareContext, {}, Restore) - verify { middlewareContext.dispatch(UpdateMessages(messages)) } + verify { store.dispatch(UpdateMessages(messages)) } } @Test @@ -101,7 +109,7 @@ class MessagingMiddlewareTest { middleware.invoke(middlewareContext, {}, MessageClicked(message)) - verify { messagingStorage.updateMetadata(message.metadata.copy(pressed = true)) } + coVerify { messagingStorage.updateMetadata(message.metadata.copy(pressed = true)) } verify { middlewareContext.dispatch(UpdateMessages(emptyList())) } } @@ -127,7 +135,7 @@ class MessagingMiddlewareTest { MessageDismissed(message) ) - verify { messagingStorage.updateMetadata(message.metadata.copy(dismissed = true)) } + coVerify { messagingStorage.updateMetadata(message.metadata.copy(dismissed = true)) } verify { middlewareContext.dispatch(UpdateMessages(emptyList())) } } @@ -143,17 +151,19 @@ class MessagingMiddlewareTest { ) val appState: AppState = mockk(relaxed = true) val messagingState: MessagingState = mockk(relaxed = true) + val spiedMiddleware = spyk(middleware) + every { spiedMiddleware.now() } returns 0L every { messagingState.messages } returns emptyList() every { appState.messaging } returns messagingState every { middlewareContext.state } returns appState - middleware.invoke( + spiedMiddleware.invoke( middlewareContext, {}, MessageDisplayed(message) ) - verify { messagingStorage.updateMetadata(message.metadata.copy(displayCount = 1)) } + coVerify { messagingStorage.updateMetadata(message.metadata.copy(displayCount = 1)) } verify { middlewareContext.dispatch(UpdateMessages(emptyList())) } } @@ -175,7 +185,7 @@ class MessagingMiddlewareTest { spiedMiddleware.onMessageDismissed(middlewareContext, message) - verify { messagingStorage.updateMetadata(message.metadata.copy(dismissed = true)) } + coVerify { messagingStorage.updateMetadata(message.metadata.copy(dismissed = true)) } verify { middlewareContext.dispatch(UpdateMessages(emptyList())) } verify { spiedMiddleware.removeMessage(middlewareContext, message) } } @@ -278,6 +288,7 @@ class MessagingMiddlewareTest { val updatedMessage = oldMessage.copy(metadata = oldMessage.metadata.copy(displayCount = 1)) val spiedMiddleware = spyk(middleware) + every { spiedMiddleware.now() } returns 0 every { oldMessageData.maxDisplayCount } returns 2 every { spiedMiddleware.updateMessage( @@ -291,7 +302,7 @@ class MessagingMiddlewareTest { verify { spiedMiddleware.updateMessage(middlewareContext, oldMessage, updatedMessage) } verify { middlewareContext.dispatch(UpdateMessages(emptyList())) } - verify { messagingStorage.updateMetadata(updatedMessage.metadata) } + coVerify { messagingStorage.updateMetadata(updatedMessage.metadata) } } @Test @@ -308,6 +319,7 @@ class MessagingMiddlewareTest { val updatedMessage = oldMessage.copy(metadata = oldMessage.metadata.copy(displayCount = 1)) val spiedMiddleware = spyk(middleware) + every { spiedMiddleware.now() } returns 0 every { oldMessageData.maxDisplayCount } returns 1 every { spiedMiddleware.consumeMessageToShowIfNeeded( @@ -322,6 +334,6 @@ class MessagingMiddlewareTest { verify { spiedMiddleware.consumeMessageToShowIfNeeded(middlewareContext, oldMessage) } verify { spiedMiddleware.removeMessage(middlewareContext, oldMessage) } verify { middlewareContext.dispatch(UpdateMessages(emptyList())) } - verify { messagingStorage.updateMetadata(updatedMessage.metadata) } + coVerify { messagingStorage.updateMetadata(updatedMessage.metadata) } } }