diff --git a/app/src/main/java/org/mozilla/fenix/FenixApplication.kt b/app/src/main/java/org/mozilla/fenix/FenixApplication.kt index 1def45caf..99d81ff22 100644 --- a/app/src/main/java/org/mozilla/fenix/FenixApplication.kt +++ b/app/src/main/java/org/mozilla/fenix/FenixApplication.kt @@ -156,6 +156,9 @@ open class FenixApplication : LocaleAwareApplication(), Provider { GlobalScope.launch(Dispatchers.IO) { setStartupMetrics(store, settings()) } + if (FeatureFlags.messagingFeature && settings().isExperimentationEnabled) { + components.appStore.dispatch(AppAction.MessagingAction.Restore) + } } @CallSuper @@ -752,11 +755,6 @@ 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 031b5c234..9d83bddf7 100644 --- a/app/src/main/java/org/mozilla/fenix/components/Analytics.kt +++ b/app/src/main/java/org/mozilla/fenix/components/Analytics.kt @@ -26,8 +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.CustomAttributeProvider -import org.mozilla.fenix.gleanplumb.OnDiskMessageMetadataStorage +import org.mozilla.fenix.gleanplumb.KeyPairMessageMetadataStorage import org.mozilla.fenix.gleanplumb.NimbusMessagingStorage import org.mozilla.fenix.nimbus.FxNimbus import org.mozilla.fenix.perf.lazyMonitored @@ -132,13 +131,12 @@ class Analytics( val messagingStorage by lazyMonitored { NimbusMessagingStorage( context = context, - metadataStorage = OnDiskMessageMetadataStorage(context), + metadataStorage = KeyPairMessageMetadataStorage(), gleanPlumb = experiments, reportMalformedMessage = { metrics.track(Event.Messaging.MessageMalformed(it)) }, messagingFeature = FxNimbus.features.messaging, - attributeProvider = CustomAttributeProvider, ) } } diff --git a/app/src/main/java/org/mozilla/fenix/gleanplumb/KeyPairMessageMetadataStorage.kt b/app/src/main/java/org/mozilla/fenix/gleanplumb/KeyPairMessageMetadataStorage.kt new file mode 100644 index 000000000..3c6942117 --- /dev/null +++ b/app/src/main/java/org/mozilla/fenix/gleanplumb/KeyPairMessageMetadataStorage.kt @@ -0,0 +1,29 @@ +/* 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 b0e9df527..9bdc745fd 100644 --- a/app/src/main/java/org/mozilla/fenix/gleanplumb/Message.kt +++ b/app/src/main/java/org/mozilla/fenix/gleanplumb/Message.kt @@ -40,13 +40,11 @@ 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 lastTimeShown: Long = 0L + val dismissed: Boolean = false ) } 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 c30d8535d..a9ca921d0 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. */ - suspend fun getMetadata(): Map + fun getMetadata(): List /** * Given a [metadata] add the message metadata on the storage. * @return the added message on the [MessageMetadataStorage] */ - suspend fun addMetadata(metadata: Message.Metadata): Message.Metadata + fun addMetadata(metadata: Message.Metadata): Message.Metadata /** * Given a [metadata] update the message metadata on the storage. */ - suspend fun updateMetadata(metadata: Message.Metadata) + 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 e83207a3a..d1d47e04f 100644 --- a/app/src/main/java/org/mozilla/fenix/gleanplumb/NimbusMessagingStorage.kt +++ b/app/src/main/java/org/mozilla/fenix/gleanplumb/NimbusMessagingStorage.kt @@ -12,7 +12,6 @@ import org.mozilla.experiments.nimbus.GleanPlumbInterface import org.mozilla.experiments.nimbus.GleanPlumbMessageHelper import org.mozilla.experiments.nimbus.internal.FeatureHolder import org.mozilla.experiments.nimbus.internal.NimbusException -import org.mozilla.fenix.nimbus.ControlMessageBehavior import org.mozilla.fenix.nimbus.Messaging import org.mozilla.fenix.nimbus.StyleData @@ -24,25 +23,26 @@ class NimbusMessagingStorage( private val metadataStorage: MessageMetadataStorage, private val reportMalformedMessage: (String) -> Unit, private val gleanPlumb: GleanPlumbInterface, - private val messagingFeature: FeatureHolder, - private val attributeProvider: CustomAttributeProvider? = null + private val messagingFeature: FeatureHolder ) { private val logger = Logger("MessagingStorage") private val nimbusFeature = messagingFeature.value() private val customAttributes: JSONObject - get() = attributeProvider?.getCustomAttributes(context) ?: JSONObject() + get() = JSONObject() /** * Returns a list of available messages descending sorted by their priority. */ - suspend fun getMessages(): List { + 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() + val storageMetadata = metadataStorage.getMetadata().associateBy { + it.id + } return nimbusMessages.mapNotNull { (key, value) -> val action = sanitizeAction(key, value.action, nimbusActions) ?: return@mapNotNull null @@ -56,7 +56,7 @@ class NimbusMessagingStorage( ?: return@mapNotNull null ) }.filter { - it.maxDisplayCount >= it.metadata.displayCount && + it.data.maxDisplayCount >= it.metadata.displayCount && !it.metadata.dismissed && !it.metadata.pressed }.sortedByDescending { @@ -68,33 +68,21 @@ class NimbusMessagingStorage( * Returns the next higher priority message which all their triggers are true. */ fun getNextMessage(availableMessages: List): Message? { - val jexlCache = HashMap() val helper = gleanPlumb.createMessageHelper(customAttributes) - val message = availableMessages.firstOrNull { - isMessageEligible(it, helper, jexlCache) + var message = availableMessages.firstOrNull { + isMessageEligible(it, helper) } ?: return null - // Check this isn't an experimental message. If not, we can go ahead and return it. - if (!isMessageUnderExperiment(message, nimbusFeature.messageUnderExperiment)) { - return message - } - // If the message is under experiment, then we need to record the exposure - messagingFeature.recordExposure() + if (isMessageUnderExperiment(message, nimbusFeature.messageUnderExperiment)) { + messagingFeature.recordExposure() - // If this is an experimental message, but not a placebo, then just return the message. - return if (!message.data.isControl) { - message - } else { - // This is a control, so we need to either return the next message (there may not be one) - // or not display anything. - when (getOnControlBehavior()) { - ControlMessageBehavior.SHOW_NEXT_MESSAGE -> availableMessages.firstOrNull { - // There should only be one control message, and we've just detected it. - !it.data.isControl && isMessageEligible(it, helper, jexlCache) - } - ControlMessageBehavior.SHOW_NONE -> null + if (message.data.isControl) { + message = availableMessages.firstOrNull { + !it.data.isControl && isMessageEligible(it, helper) + } ?: return null } } + return message } /** @@ -110,7 +98,7 @@ class NimbusMessagingStorage( /** * Updated the provided [metadata] in the storage. */ - suspend fun updateMetadata(metadata: Message.Metadata) { + fun updateMetadata(metadata: Message.Metadata) { metadataStorage.updateMetadata(metadata) } @@ -150,7 +138,7 @@ class NimbusMessagingStorage( @VisibleForTesting internal fun isMessageUnderExperiment(message: Message, expression: String?): Boolean { - return message.data.isControl || when { + return when { expression.isNullOrBlank() -> { false } @@ -166,27 +154,21 @@ class NimbusMessagingStorage( @VisibleForTesting internal fun isMessageEligible( message: Message, - helper: GleanPlumbMessageHelper, - jexlCache: MutableMap = mutableMapOf() + helper: GleanPlumbMessageHelper ): Boolean { return message.triggers.all { condition -> - jexlCache[condition] - ?: try { - helper.evalJexl(condition).also { result -> - jexlCache[condition] = result - } - } catch (e: NimbusException.EvaluationException) { - reportMalformedMessage(message.id) - logger.info("Unable to evaluate $condition") - false - } + try { + helper.evalJexl(condition) + } catch (e: NimbusException.EvaluationException) { + reportMalformedMessage(message.id) + logger.info("Unable to evaluate $condition") + false + } } } - @VisibleForTesting - internal fun getOnControlBehavior(): ControlMessageBehavior = nimbusFeature.onControl - - private suspend fun addMetadata(id: String): Message.Metadata { + private fun addMetadata(id: String): Message.Metadata { + // This will be improve on https://github.com/mozilla-mobile/fenix/issues/24222 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 deleted file mode 100644 index b87f3023f..000000000 --- a/app/src/main/java/org/mozilla/fenix/gleanplumb/OnDiskMessageMetadataStorage.kt +++ /dev/null @@ -1,95 +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 - -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 625e8001d..3869c59fa 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,9 +5,6 @@ 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 @@ -26,8 +23,7 @@ import org.mozilla.fenix.gleanplumb.NimbusMessagingStorage typealias AppStoreMiddlewareContext = MiddlewareContext class MessagingMiddleware( - private val messagingStorage: NimbusMessagingStorage, - private val coroutineScope: CoroutineScope = CoroutineScope(Dispatchers.IO), + private val messagingStorage: NimbusMessagingStorage ) : Middleware { override fun invoke( @@ -37,10 +33,9 @@ class MessagingMiddleware( ) { when (action) { is Restore -> { - coroutineScope.launch { - val messages = messagingStorage.getMessages() - context.store.dispatch(UpdateMessages(messages)) - } + val messages = messagingStorage.getMessages() + + context.dispatch(UpdateMessages(messages)) } is Evaluate -> { @@ -67,8 +62,7 @@ class MessagingMiddleware( context: AppStoreMiddlewareContext ) { val newMetadata = oldMessage.metadata.copy( - displayCount = oldMessage.metadata.displayCount + 1, - lastTimeShown = now() + displayCount = oldMessage.metadata.displayCount + 1 ) val newMessage = oldMessage.copy( metadata = newMetadata @@ -80,9 +74,7 @@ class MessagingMiddleware( removeMessage(context, oldMessage) } context.dispatch(UpdateMessages(newMessages)) - coroutineScope.launch { - messagingStorage.updateMetadata(newMetadata) - } + messagingStorage.updateMetadata(newMetadata) } @VisibleForTesting @@ -91,12 +83,11 @@ 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 @@ -105,10 +96,9 @@ class MessagingMiddleware( context: AppStoreMiddlewareContext ) { // Update Nimbus storage. - coroutineScope.launch { - val updatedMetadata = message.metadata.copy(pressed = true) - messagingStorage.updateMetadata(updatedMetadata) - } + val updatedMetadata = message.metadata.copy(pressed = true) + messagingStorage.updateMetadata(updatedMetadata) + // Update app state. val newMessages = removeMessage(context, message) context.dispatch(UpdateMessages(newMessages)) @@ -146,7 +136,4 @@ 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 8bc94634b..20dc08b52 100644 --- a/app/src/test/java/org/mozilla/fenix/gleanplumb/NimbusMessagingStorageTest.kt +++ b/app/src/test/java/org/mozilla/fenix/gleanplumb/NimbusMessagingStorageTest.kt @@ -4,12 +4,11 @@ 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.runBlockingTest +import mozilla.components.support.test.mock import mozilla.components.support.test.robolectric.testContext import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse @@ -24,7 +23,6 @@ import org.mozilla.experiments.nimbus.internal.FeatureHolder import org.mozilla.experiments.nimbus.internal.NimbusException import org.mozilla.fenix.HomeActivity import org.mozilla.fenix.helpers.FenixRobolectricTestRunner -import org.mozilla.fenix.nimbus.ControlMessageBehavior.SHOW_NEXT_MESSAGE import org.mozilla.fenix.nimbus.MessageData import org.mozilla.fenix.nimbus.Messaging import org.mozilla.fenix.nimbus.StyleData @@ -43,6 +41,7 @@ class NimbusMessagingStorageTest { private val reportMalformedMessage: (String) -> Unit = { malformedWasReported = true } + @Before fun setup() { gleanPlumb = mockk(relaxed = true) @@ -50,7 +49,7 @@ class NimbusMessagingStorageTest { malformedWasReported = false messagingFeature = createMessagingFeature() - coEvery { metadataStorage.getMetadata() } returns mapOf("message-1" to Message.Metadata(id = "message-1")) + every { metadataStorage.getMetadata() } returns listOf(Message.Metadata(id = "message-1")) storage = NimbusMessagingStorage( testContext, @@ -62,7 +61,7 @@ class NimbusMessagingStorageTest { } @Test - fun `WHEN calling getMessages THEN provide a list of available messages`() = runBlockingTest { + fun `WHEN calling getMessages THEN provide a list of available messages`() { val message = storage.getMessages().first() assertEquals("message-1", message.id) @@ -70,160 +69,150 @@ class NimbusMessagingStorageTest { } @Test - 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) - } + 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) + } @Test - 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) - } + 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) + } @Test - 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) - } + 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) + } @Test - 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" - ), - "normal-message" to createMessageData(style = "high-priority"), - ) - val styles = mapOf( - "high-priority" to createStyle(priority = 100, maxDisplayCount = 2), - ) - 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) - } + 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) + } @Test - fun `GIVEN a malformed message WHEN calling getMessages THEN provide a list of messages ignoring the malformed one`() = runBlockingTest { + fun `GIVEN a malformed message WHEN calling getMessages THEN provide a list of messages ignoring the malformed one`() { val messages = storage.getMessages() val firstMessage = messages.first() @@ -248,11 +237,11 @@ class NimbusMessagingStorageTest { } @Test - fun `WHEN calling updateMetadata THEN delegate to metadataStorage`() = runBlockingTest { + fun `WHEN calling updateMetadata THEN delegate to metadataStorage`() { - storage.updateMetadata(mockk(relaxed = true)) + storage.updateMetadata(mockk()) - coEvery { metadataStorage.updateMetadata(any()) } + verify { metadataStorage.updateMetadata(any()) } } @Test @@ -291,10 +280,9 @@ class NimbusMessagingStorageTest { @Test fun `GIVEN a null or black expression WHEN calling isMessageUnderExperiment THEN return false`() { val message = Message( - "id", - mockk(relaxed = true), + "id", mockk(), action = "action", - mockk(relaxed = true), + mock(), emptyList(), Message.Metadata("id") ) @@ -307,10 +295,9 @@ class NimbusMessagingStorageTest { @Test fun `GIVEN messages id that ends with - WHEN calling isMessageUnderExperiment THEN return true`() { val message = Message( - "end-", - mockk(relaxed = true), + "end-", mockk(), action = "action", - mockk(relaxed = true), + mock(), emptyList(), Message.Metadata("end-") ) @@ -323,10 +310,9 @@ class NimbusMessagingStorageTest { @Test fun `GIVEN message under experiment WHEN calling isMessageUnderExperiment THEN return true`() { val message = Message( - "same-id", - mockk(relaxed = true), + "same-id", mockk(), action = "action", - mockk(relaxed = true), + mock(), emptyList(), Message.Metadata("same-id") ) @@ -340,10 +326,9 @@ class NimbusMessagingStorageTest { fun `GIVEN an eligible message WHEN calling isMessageEligible THEN return true`() { val helper: GleanPlumbMessageHelper = mockk(relaxed = true) val message = Message( - "same-id", - mockk(relaxed = true), + "same-id", mockk(), action = "action", - mockk(relaxed = true), + mock(), listOf("trigger"), Message.Metadata("same-id") ) @@ -359,10 +344,9 @@ class NimbusMessagingStorageTest { fun `GIVEN a malformed trigger WHEN calling isMessageEligible THEN return false`() { val helper: GleanPlumbMessageHelper = mockk(relaxed = true) val message = Message( - "same-id", - mockk(relaxed = true), + "same-id", mockk(), action = "action", - mockk(relaxed = true), + mock(), listOf("trigger"), Message.Metadata("same-id") ) @@ -378,10 +362,9 @@ class NimbusMessagingStorageTest { fun `GIVEN none available messages are eligible WHEN calling getNextMessage THEN return null`() { val spiedStorage = spyk(storage) val message = Message( - "same-id", - mockk(relaxed = true), + "same-id", mockk(), action = "action", - mockk(relaxed = true), + mock(), listOf("trigger"), Message.Metadata("same-id") ) @@ -397,10 +380,9 @@ class NimbusMessagingStorageTest { fun `GIVEN an eligible message WHEN calling getNextMessage THEN return the message`() { val spiedStorage = spyk(storage) val message = Message( - "same-id", - mockk(relaxed = true), + "same-id", mockk(), action = "action", - mockk(relaxed = true), + mock(), listOf("trigger"), Message.Metadata("same-id") ) @@ -445,7 +427,6 @@ class NimbusMessagingStorageTest { val controlMessageData: MessageData = mockk(relaxed = true) every { messageData.isControl } returns false - every { spiedStorage.getOnControlBehavior() } returns SHOW_NEXT_MESSAGE every { controlMessageData.isControl } returns true val message = Message( @@ -478,12 +459,14 @@ class NimbusMessagingStorageTest { private fun createMessageData( action: String = "action-1", style: String = "style-1", - triggers: List = listOf("trigger-1") + triggers: List = listOf("trigger-1"), + maxDisplayCount: Int = 5 ): MessageData { val messageData1: MessageData = mockk(relaxed = true) every { messageData1.action } returns action every { messageData1.style } returns style every { messageData1.trigger } returns triggers + every { messageData1.maxDisplayCount } returns maxDisplayCount return messageData1 } @@ -509,10 +492,9 @@ class NimbusMessagingStorageTest { return messagingFeature } - private fun createStyle(priority: Int = 1, maxDisplayCount: Int = 5): StyleData { + private fun createStyle(priority: Int = 1): StyleData { val style1: StyleData = mockk(relaxed = true) every { style1.priority } returns priority - every { style1.maxDisplayCount } returns maxDisplayCount return style1 } } diff --git a/app/src/test/java/org/mozilla/fenix/gleanplumb/OnDiskMessageMetadataStorageTest.kt b/app/src/test/java/org/mozilla/fenix/gleanplumb/OnDiskMessageMetadataStorageTest.kt deleted file mode 100644 index 17ef24504..000000000 --- a/app/src/test/java/org/mozilla/fenix/gleanplumb/OnDiskMessageMetadataStorageTest.kt +++ /dev/null @@ -1,144 +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 - -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 3408f6ccc..8377d5921 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,14 +5,11 @@ 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 @@ -37,12 +34,10 @@ import org.mozilla.fenix.gleanplumb.MessagingState import org.mozilla.fenix.gleanplumb.NimbusMessagingStorage import org.mozilla.fenix.helpers.FenixRobolectricTestRunner import org.mozilla.fenix.nimbus.MessageData -import org.mozilla.fenix.nimbus.StyleData @RunWith(FenixRobolectricTestRunner::class) class MessagingMiddlewareTest { - private val coroutineScope = TestCoroutineScope() private lateinit var store: AppStore private lateinit var middleware: MessagingMiddleware private lateinit var messagingStorage: NimbusMessagingStorage @@ -53,14 +48,10 @@ 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, - coroutineScope + messagingStorage ) } @@ -68,11 +59,11 @@ class MessagingMiddlewareTest { fun `WHEN Restore THEN get messages from the storage and UpdateMessages`() { val messages: List = emptyList() - coEvery { messagingStorage.getMessages() } returns messages + every { messagingStorage.getMessages() } returns messages middleware.invoke(middlewareContext, {}, Restore) - verify { store.dispatch(UpdateMessages(messages)) } + verify { middlewareContext.dispatch(UpdateMessages(messages)) } } @Test @@ -110,7 +101,7 @@ class MessagingMiddlewareTest { middleware.invoke(middlewareContext, {}, MessageClicked(message)) - coVerify { messagingStorage.updateMetadata(message.metadata.copy(pressed = true)) } + verify { messagingStorage.updateMetadata(message.metadata.copy(pressed = true)) } verify { middlewareContext.dispatch(UpdateMessages(emptyList())) } } @@ -136,7 +127,7 @@ class MessagingMiddlewareTest { MessageDismissed(message) ) - coVerify { messagingStorage.updateMetadata(message.metadata.copy(dismissed = true)) } + verify { messagingStorage.updateMetadata(message.metadata.copy(dismissed = true)) } verify { middlewareContext.dispatch(UpdateMessages(emptyList())) } } @@ -152,19 +143,17 @@ 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 - spiedMiddleware.invoke( + middleware.invoke( middlewareContext, {}, MessageDisplayed(message) ) - coVerify { messagingStorage.updateMetadata(message.metadata.copy(displayCount = 1)) } + verify { messagingStorage.updateMetadata(message.metadata.copy(displayCount = 1)) } verify { middlewareContext.dispatch(UpdateMessages(emptyList())) } } @@ -186,7 +175,7 @@ class MessagingMiddlewareTest { spiedMiddleware.onMessageDismissed(middlewareContext, message) - coVerify { messagingStorage.updateMetadata(message.metadata.copy(dismissed = true)) } + verify { messagingStorage.updateMetadata(message.metadata.copy(dismissed = true)) } verify { middlewareContext.dispatch(UpdateMessages(emptyList())) } verify { spiedMiddleware.removeMessage(middlewareContext, message) } } @@ -277,21 +266,19 @@ class MessagingMiddlewareTest { @Test fun `GIVEN a message with that not surpassed the maxDisplayCount WHEN onMessagedDisplayed THEN update the available messages and the updateMetadata`() { - val style: StyleData = mockk(relaxed = true) val oldMessageData: MessageData = mockk(relaxed = true) val oldMessage = Message( "oldMessage", oldMessageData, action = "action", - style, + mockk(relaxed = true), listOf("trigger"), Message.Metadata("same-id", displayCount = 0) ) val updatedMessage = oldMessage.copy(metadata = oldMessage.metadata.copy(displayCount = 1)) val spiedMiddleware = spyk(middleware) - every { spiedMiddleware.now() } returns 0 - every { style.maxDisplayCount } returns 2 + every { oldMessageData.maxDisplayCount } returns 2 every { spiedMiddleware.updateMessage( middlewareContext, @@ -304,26 +291,24 @@ class MessagingMiddlewareTest { verify { spiedMiddleware.updateMessage(middlewareContext, oldMessage, updatedMessage) } verify { middlewareContext.dispatch(UpdateMessages(emptyList())) } - coVerify { messagingStorage.updateMetadata(updatedMessage.metadata) } + verify { messagingStorage.updateMetadata(updatedMessage.metadata) } } @Test fun `GIVEN a message with that surpassed the maxDisplayCount WHEN onMessagedDisplayed THEN remove the message and consume it`() { - val style: StyleData = mockk(relaxed = true) val oldMessageData: MessageData = mockk(relaxed = true) val oldMessage = Message( "oldMessage", oldMessageData, action = "action", - style, + mockk(relaxed = true), listOf("trigger"), Message.Metadata("same-id", displayCount = 0) ) val updatedMessage = oldMessage.copy(metadata = oldMessage.metadata.copy(displayCount = 1)) val spiedMiddleware = spyk(middleware) - every { spiedMiddleware.now() } returns 0 - every { style.maxDisplayCount } returns 1 + every { oldMessageData.maxDisplayCount } returns 1 every { spiedMiddleware.consumeMessageToShowIfNeeded( middlewareContext, @@ -337,6 +322,6 @@ class MessagingMiddlewareTest { verify { spiedMiddleware.consumeMessageToShowIfNeeded(middlewareContext, oldMessage) } verify { spiedMiddleware.removeMessage(middlewareContext, oldMessage) } verify { middlewareContext.dispatch(UpdateMessages(emptyList())) } - coVerify { messagingStorage.updateMetadata(updatedMessage.metadata) } + verify { messagingStorage.updateMetadata(updatedMessage.metadata) } } }