mirror of
https://github.com/ProtonMail/android-mail.git
synced 2026-05-15 09:50:40 +00:00
Improve Feature Flags retrieval + logging
ET-6180
This commit is contained in:
committed by
MargeBot
parent
72dec3bffc
commit
4fcbbe2cb6
-9
@@ -23,7 +23,6 @@ import ch.protonmail.android.mailfeatureflags.data.local.DefaultFeatureFlagValue
|
||||
import ch.protonmail.android.mailfeatureflags.data.local.UnleashFeatureFlagValueProvider
|
||||
import ch.protonmail.android.mailfeatureflags.data.local.factory.BooleanFeatureFlagFactory
|
||||
import ch.protonmail.android.mailfeatureflags.domain.FeatureFlagValueProvider
|
||||
import ch.protonmail.android.mailfeatureflags.domain.annotation.FeatureFlagsCoroutineScope
|
||||
import ch.protonmail.android.mailfeatureflags.domain.annotation.IsBgProcessingRelaxedBatteryConstraintEnabled
|
||||
import ch.protonmail.android.mailfeatureflags.domain.annotation.IsBlackFridayWave1Enabled
|
||||
import ch.protonmail.android.mailfeatureflags.domain.annotation.IsBlackFridayWave2Enabled
|
||||
@@ -68,9 +67,6 @@ import dagger.Provides
|
||||
import dagger.hilt.InstallIn
|
||||
import dagger.hilt.components.SingletonComponent
|
||||
import dagger.multibindings.IntoSet
|
||||
import kotlinx.coroutines.CoroutineScope
|
||||
import kotlinx.coroutines.Dispatchers
|
||||
import kotlinx.coroutines.SupervisorJob
|
||||
import javax.inject.Singleton
|
||||
|
||||
@Module
|
||||
@@ -112,11 +108,6 @@ object FeatureFlagsModule {
|
||||
fun provideDetailCssOverrideEnabledOverride(factory: BooleanFeatureFlagFactory) =
|
||||
factory.create(InjectDetailCssOverrideEnabled.key, false)
|
||||
|
||||
@Provides
|
||||
@Singleton
|
||||
@FeatureFlagsCoroutineScope
|
||||
fun provideFeatureFlagsCoroutineScope(): CoroutineScope = CoroutineScope(Dispatchers.IO + SupervisorJob())
|
||||
|
||||
@Provides
|
||||
@Singleton
|
||||
@IsUpsellEnabled
|
||||
|
||||
+6
-6
@@ -18,11 +18,11 @@
|
||||
|
||||
package ch.protonmail.android.mailfeatureflags.data.local
|
||||
|
||||
import ch.protonmail.android.mailcommon.domain.coroutines.IODispatcher
|
||||
import ch.protonmail.android.mailfeatureflags.domain.FeatureFlagProviderPriority
|
||||
import ch.protonmail.android.mailfeatureflags.domain.FeatureFlagValueProvider
|
||||
import ch.protonmail.android.mailfeatureflags.domain.annotation.FeatureFlagsCoroutineScope
|
||||
import dagger.Lazy
|
||||
import kotlinx.coroutines.CoroutineScope
|
||||
import kotlinx.coroutines.CoroutineDispatcher
|
||||
import kotlinx.coroutines.withContext
|
||||
import me.proton.core.domain.entity.UserId
|
||||
import timber.log.Timber
|
||||
@@ -34,17 +34,17 @@ import javax.inject.Singleton
|
||||
@Singleton
|
||||
class UnleashFeatureFlagValueProvider @Inject constructor(
|
||||
private val sessionFacade: Lazy<SessionFacade>,
|
||||
@FeatureFlagsCoroutineScope private val coroutineScope: CoroutineScope
|
||||
@IODispatcher private val ioDispatcher: CoroutineDispatcher
|
||||
) : FeatureFlagValueProvider {
|
||||
|
||||
override val priority: Int = FeatureFlagProviderPriority.UnleashProvider
|
||||
|
||||
override val name: String = "Unleash FF provider"
|
||||
|
||||
override suspend fun getFeatureFlagValue(key: String): Boolean? = withContext(coroutineScope.coroutineContext) {
|
||||
// needs to be lazy because of initialisation steps
|
||||
override suspend fun getFeatureFlagValue(key: String): Boolean? = withContext(ioDispatcher) {
|
||||
// needs to be lazy because of initialization steps
|
||||
val session = sessionFacade.get()
|
||||
// For feature flags that are used in the app initialisation
|
||||
// For feature flags that are used in the app initialization
|
||||
if (session.isMailSessionInitialised().not()) {
|
||||
Timber.w(
|
||||
"Getting FeatureFlag:: MailSession is not initialized yet. " +
|
||||
|
||||
+2
-3
@@ -25,7 +25,6 @@ import io.mockk.coEvery
|
||||
import io.mockk.coVerify
|
||||
import io.mockk.every
|
||||
import io.mockk.mockk
|
||||
import kotlinx.coroutines.CoroutineScope
|
||||
import kotlinx.coroutines.ExperimentalCoroutinesApi
|
||||
import kotlinx.coroutines.test.runTest
|
||||
import me.proton.core.domain.entity.UserId
|
||||
@@ -45,7 +44,7 @@ class UnleashFeatureFlagValueProviderTest {
|
||||
|
||||
@get:Rule
|
||||
val mainDispatcherRule = MainDispatcherRule()
|
||||
private val scope = CoroutineScope(mainDispatcherRule.testDispatcher)
|
||||
private val testDispatcher = mainDispatcherRule.testDispatcher
|
||||
private val sessionFacade: SessionFacade = mockk()
|
||||
private val sessionLazy: Lazy<SessionFacade> = mockk()
|
||||
|
||||
@@ -64,7 +63,7 @@ class UnleashFeatureFlagValueProviderTest {
|
||||
|
||||
provider = UnleashFeatureFlagValueProvider(
|
||||
sessionLazy,
|
||||
scope
|
||||
testDispatcher
|
||||
)
|
||||
|
||||
coEvery { sessionFacade.getIsMailSessionFeatureEnabled(testFeatureFlagKey) } returns
|
||||
|
||||
+26
-17
@@ -18,36 +18,45 @@
|
||||
|
||||
package ch.protonmail.android.mailfeatureflags.domain
|
||||
|
||||
import ch.protonmail.android.mailfeatureflags.domain.annotation.FeatureFlagsCoroutineScope
|
||||
import kotlinx.coroutines.CoroutineScope
|
||||
import kotlinx.coroutines.withContext
|
||||
import java.util.concurrent.ConcurrentHashMap
|
||||
import kotlinx.coroutines.CancellationException
|
||||
import timber.log.Timber
|
||||
import javax.inject.Inject
|
||||
import javax.inject.Singleton
|
||||
|
||||
@Singleton
|
||||
class FeatureFlagResolver @Inject constructor(
|
||||
private val providers: Set<@JvmSuppressWildcards FeatureFlagValueProvider>,
|
||||
@FeatureFlagsCoroutineScope private val coroutineScope: CoroutineScope
|
||||
private val providers: Set<@JvmSuppressWildcards FeatureFlagValueProvider>
|
||||
) {
|
||||
|
||||
private val lastLogged = ConcurrentHashMap<String, Pair<Boolean, String>>()
|
||||
|
||||
/**
|
||||
* Gets the value of a feature flag by its key, taking provider priorities into account.
|
||||
*/
|
||||
@Suppress("TooGenericExceptionCaught")
|
||||
suspend fun getFeatureFlag(key: String, defaultValue: Boolean): Boolean {
|
||||
return withContext(coroutineScope.coroutineContext) {
|
||||
providers
|
||||
.filter { it.isEnabled() }
|
||||
.sortedByDescending { it.priority }
|
||||
.firstNotNullOfOrNull { provider ->
|
||||
val value = runCatching {
|
||||
provider.getFeatureFlagValue(key)
|
||||
}.getOrNull()
|
||||
|
||||
Timber.d("'${provider.name}' - Resolved FF '$key': $value")
|
||||
value
|
||||
val match = providers
|
||||
.filter { it.isEnabled() }
|
||||
.sortedByDescending { it.priority }
|
||||
.firstNotNullOfOrNull { provider ->
|
||||
val value = try {
|
||||
provider.getFeatureFlagValue(key)
|
||||
} catch (c: CancellationException) {
|
||||
throw c
|
||||
} catch (t: Throwable) {
|
||||
Timber.w(t, "FF provider '${provider.name}' failed for '$key'")
|
||||
null
|
||||
}
|
||||
?: defaultValue
|
||||
value?.let { provider.name to it }
|
||||
}
|
||||
|
||||
val resolved = match?.second ?: defaultValue
|
||||
val source = match?.first ?: "default"
|
||||
val current = resolved to source
|
||||
if (lastLogged.put(key, current) != current) {
|
||||
Timber.d("Resolved FF '$key' = $resolved (via $source)")
|
||||
}
|
||||
return resolved
|
||||
}
|
||||
}
|
||||
|
||||
-25
@@ -1,25 +0,0 @@
|
||||
/*
|
||||
* Copyright (c) 2025 Proton Technologies AG
|
||||
* This file is part of Proton Technologies AG and Proton Mail.
|
||||
*
|
||||
* Proton Mail is free software: you can redistribute it and/or modify
|
||||
* it under the terms of the GNU General Public License as published by
|
||||
* the Free Software Foundation, either version 3 of the License, or
|
||||
* (at your option) any later version.
|
||||
*
|
||||
* Proton Mail is distributed in the hope that it will be useful,
|
||||
* but WITHOUT ANY WARRANTY; without even the implied warranty of
|
||||
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
|
||||
* GNU General Public License for more details.
|
||||
*
|
||||
* You should have received a copy of the GNU General Public License
|
||||
* along with Proton Mail. If not, see <https://www.gnu.org/licenses/>.
|
||||
*/
|
||||
|
||||
package ch.protonmail.android.mailfeatureflags.domain.annotation
|
||||
|
||||
import javax.inject.Qualifier
|
||||
|
||||
@Qualifier
|
||||
@Retention(AnnotationRetention.BINARY)
|
||||
annotation class FeatureFlagsCoroutineScope
|
||||
+5
-14
@@ -18,30 +18,21 @@
|
||||
|
||||
package ch.protonmail.android.mailfeatureflags.domain
|
||||
|
||||
import ch.protonmail.android.test.utils.rule.MainDispatcherRule
|
||||
import io.mockk.coEvery
|
||||
import io.mockk.coVerify
|
||||
import io.mockk.every
|
||||
import io.mockk.mockk
|
||||
import kotlinx.coroutines.CoroutineScope
|
||||
import kotlinx.coroutines.test.runTest
|
||||
import org.junit.Rule
|
||||
import kotlin.test.Test
|
||||
import kotlin.test.assertFalse
|
||||
import kotlin.test.assertTrue
|
||||
|
||||
internal class FeatureFlagResolverTest {
|
||||
|
||||
@get:Rule
|
||||
val mainDispatcherRule = MainDispatcherRule()
|
||||
|
||||
private val coroutineScope = CoroutineScope(mainDispatcherRule.testDispatcher)
|
||||
|
||||
|
||||
@Test
|
||||
fun `should default to the default value when no providers are present`() = runTest {
|
||||
// Given
|
||||
val resolver = FeatureFlagResolver(emptySet(), coroutineScope)
|
||||
val resolver = FeatureFlagResolver(emptySet())
|
||||
|
||||
// When + Then
|
||||
val actual = resolver.getFeatureFlag(FeatureFlagKey, false)
|
||||
@@ -64,7 +55,7 @@ internal class FeatureFlagResolverTest {
|
||||
every { priority } returns 10
|
||||
every { isEnabled() } returns false
|
||||
}
|
||||
val resolver = FeatureFlagResolver(setOf(notEnabledProvider1, notEnabledProvider2), coroutineScope)
|
||||
val resolver = FeatureFlagResolver(setOf(notEnabledProvider1, notEnabledProvider2))
|
||||
|
||||
// When
|
||||
val actual = resolver.getFeatureFlag(FeatureFlagKey, false)
|
||||
@@ -91,7 +82,7 @@ internal class FeatureFlagResolverTest {
|
||||
every { priority } returns 10
|
||||
every { isEnabled() } returns false
|
||||
}
|
||||
val resolver = FeatureFlagResolver(setOf(enabledProvider, notEnabledProvider), coroutineScope)
|
||||
val resolver = FeatureFlagResolver(setOf(enabledProvider, notEnabledProvider))
|
||||
|
||||
// When + Then
|
||||
val actual = resolver.getFeatureFlag(FeatureFlagKey, false)
|
||||
@@ -110,7 +101,7 @@ internal class FeatureFlagResolverTest {
|
||||
every { priority } returns 0
|
||||
every { isEnabled() } returns true
|
||||
}
|
||||
val resolver = FeatureFlagResolver(setOf(provider), coroutineScope)
|
||||
val resolver = FeatureFlagResolver(setOf(provider))
|
||||
|
||||
// When
|
||||
val actual = resolver.getFeatureFlag(FeatureFlagKey, false)
|
||||
@@ -137,7 +128,7 @@ internal class FeatureFlagResolverTest {
|
||||
every { isEnabled() } returns true
|
||||
}
|
||||
|
||||
val resolver = FeatureFlagResolver(setOf(lowPriorityProvider, topPriorityProvider), coroutineScope)
|
||||
val resolver = FeatureFlagResolver(setOf(lowPriorityProvider, topPriorityProvider))
|
||||
|
||||
// When
|
||||
val actual = resolver.getFeatureFlag(FeatureFlagKey, false)
|
||||
|
||||
Reference in New Issue
Block a user