Bug 1816196 - Do not fetch messaging during storage initialization

While debugging a UI test, we found that the messaging feature was being
queried during the storage initialization and acting on the cached value
alone. This could lead us to no longer receiving new messages during the
app lifetime.

For the unit tests, we switched from mocking the entire feature to
mocking just the `FeaturesInterface` as the test was providing
false-positive results. It's better to create a real feature in any case
for the other testing needs.

Co-authored-by: Benjamin Forehand Jr <bennyjr169@gmail.com>
fenix/112.0
Jonathan Almeida 2 years ago committed by mergify[bot]
parent 688d74af8f
commit c1923291d3

@ -48,7 +48,7 @@ class NimbusMessagingStorage(
@VisibleForTesting
internal val malFormedMap = mutableMapOf<String, String>()
private val logger = Logger("MessagingStorage")
private val nimbusFeature = messagingFeature.value()
private val nimbusFeature = messagingFeature
private val customAttributes: JSONObject
get() = attributeProvider?.getCustomAttributes(context) ?: JSONObject()
@ -56,11 +56,12 @@ class NimbusMessagingStorage(
* Returns a list of available messages descending sorted by their priority.
*/
suspend fun getMessages(): List<Message> {
val nimbusTriggers = nimbusFeature.triggers
val nimbusStyles = nimbusFeature.styles
val nimbusActions = nimbusFeature.actions
val featureValue = messagingFeature.value()
val nimbusTriggers = featureValue.triggers
val nimbusStyles = featureValue.styles
val nimbusActions = featureValue.actions
val nimbusMessages = nimbusFeature.messages
val nimbusMessages = featureValue.messages
val defaultStyle = StyleData()
val storageMetadata = metadataStorage.getMetadata()
@ -96,7 +97,7 @@ class NimbusMessagingStorage(
} ?: return null
// Check this isn't an experimental message. If not, we can go ahead and return it.
if (!isMessageUnderExperiment(message, nimbusFeature.messageUnderExperiment)) {
if (!isMessageUnderExperiment(message, nimbusFeature.value().messageUnderExperiment)) {
return message
}
// If the message is under experiment, then we need to record the exposure
@ -232,7 +233,7 @@ class NimbusMessagingStorage(
}
@VisibleForTesting
internal fun getOnControlBehavior(): ControlMessageBehavior = nimbusFeature.onControl
internal fun getOnControlBehavior(): ControlMessageBehavior = nimbusFeature.value().onControl
private suspend fun addMetadata(id: String): Message.Metadata {
return metadataStorage.addMetadata(

@ -4,6 +4,7 @@
package org.mozilla.fenix.gleanplumb
import io.mockk.Called
import io.mockk.coEvery
import io.mockk.every
import io.mockk.mockk
@ -37,7 +38,6 @@ class NimbusMessagingStorageTest {
private lateinit var metadataStorage: MessageMetadataStorage
private lateinit var gleanPlumb: GleanPlumbInterface
private lateinit var messagingFeature: FeatureHolder<Messaging>
private lateinit var messaging: Messaging
private var malformedWasReported = false
private val reportMalformedMessage: (String) -> Unit = {
malformedWasReported = true
@ -579,6 +579,20 @@ class NimbusMessagingStorageTest {
assertEquals(message.id, result!!.id)
}
@Test
fun `WHEN a storage instance is created THEN do not invoke the feature`() = runTest {
storage = NimbusMessagingStorage(
testContext,
metadataStorage,
reportMalformedMessage,
gleanPlumb,
messagingFeature,
)
// We should not be using the feature holder until getMessages is called.
verify { messagingFeature wasNot Called }
}
private fun createMessageData(
action: String = "action-1",
style: String = "style-1",
@ -603,16 +617,18 @@ class NimbusMessagingStorageTest {
"malformed" to createMessageData(action = "malformed-action"),
),
): FeatureHolder<Messaging> {
val messagingFeature: FeatureHolder<Messaging> = mockk(relaxed = true)
messaging = Messaging(
val messaging = Messaging(
actions = actions,
triggers = triggers,
messages = messages,
styles = styles,
)
every { messagingFeature.value() } returns messaging
val messagingFeature = FeatureHolder({ mockk(relaxed = true) }, "messaging") {
messaging
}
messagingFeature.withCachedValue(messaging)
return messagingFeature
return spyk(messagingFeature)
}
private fun createStyle(priority: Int = 1, maxDisplayCount: Int = 5): StyleData {

Loading…
Cancel
Save