diff --git a/app/src/main/java/com/x8bit/bitwarden/data/platform/manager/FeatureFlagManagerImpl.kt b/app/src/main/java/com/x8bit/bitwarden/data/platform/manager/FeatureFlagManagerImpl.kt index 1e8c693d9..1bf58091b 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/platform/manager/FeatureFlagManagerImpl.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/platform/manager/FeatureFlagManagerImpl.kt @@ -42,7 +42,9 @@ class FeatureFlagManagerImpl( private fun ServerConfig?.getFlagValueOrDefault(key: FlagKey): T { val defaultValue = key.defaultValue - return this?.serverData + if (!key.isRemotelyConfigured) return key.defaultValue + return this + ?.serverData ?.featureStates ?.get(key.keyName) ?.let { diff --git a/app/src/main/java/com/x8bit/bitwarden/data/platform/manager/model/FlagKey.kt b/app/src/main/java/com/x8bit/bitwarden/data/platform/manager/model/FlagKey.kt index a466ab209..92f8bb69a 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/platform/manager/model/FlagKey.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/platform/manager/model/FlagKey.kt @@ -2,50 +2,75 @@ package com.x8bit.bitwarden.data.platform.manager.model /** * Class to hold feature flag keys. - * @property [keyName] corresponds to the string value of a given key - * @property [defaultValue] corresponds to default value of the flag of type [T] */ sealed class FlagKey { + /** + * The string value of the given key. This must match the network value. + */ abstract val keyName: String + + /** + * The value to be used if the flags value cannot be determined or is not remotely configured. + */ abstract val defaultValue: T /** - * Data object holding the key for Email Verification feature + * Indicates if the flag should respect the network value or not. + */ + abstract val isRemotelyConfigured: Boolean + + /** + * Data object holding the key for Email Verification feature. */ data object EmailVerification : FlagKey() { override val keyName: String = "email-verification" override val defaultValue: Boolean = false + override val isRemotelyConfigured: Boolean = false } /** - * Data object holding the feature flag key for the Onboarding Carousel feature + * Data object holding the feature flag key for the Onboarding Carousel feature. */ data object OnboardingCarousel : FlagKey() { override val keyName: String = "native-carousel-flow" override val defaultValue: Boolean = false + override val isRemotelyConfigured: Boolean = false } /** - * Data object holding the feature flag key for the new onboarding feature + * Data object holding the feature flag key for the new onboarding feature. */ data object OnboardingFlow : FlagKey() { override val keyName: String = "native-create-account-flow" override val defaultValue: Boolean = false + override val isRemotelyConfigured: Boolean = false } /** - * Data object holding the key for an Int flag to be used in tests + * Data object holding the key for a [Boolean] flag to be used in tests. */ - data object DummyInt : FlagKey() { + data object DummyBoolean : FlagKey() { + override val keyName: String = "dummy-boolean" + override val defaultValue: Boolean = false + override val isRemotelyConfigured: Boolean = true + } + + /** + * Data object holding the key for an [Int] flag to be used in tests. + */ + data class DummyInt( + override val isRemotelyConfigured: Boolean = true, + ) : FlagKey() { override val keyName: String = "dummy-int" override val defaultValue: Int = Int.MIN_VALUE } /** - * Data object holding the key for an String flag to be used in tests + * Data object holding the key for a [String] flag to be used in tests. */ data object DummyString : FlagKey() { override val keyName: String = "dummy-string" override val defaultValue: String = "defaultValue" + override val isRemotelyConfigured: Boolean = true } } diff --git a/app/src/test/java/com/x8bit/bitwarden/data/platform/manager/FeatureFlagManagerTest.kt b/app/src/test/java/com/x8bit/bitwarden/data/platform/manager/FeatureFlagManagerTest.kt index 472f6f489..8f5466c8b 100644 --- a/app/src/test/java/com/x8bit/bitwarden/data/platform/manager/FeatureFlagManagerTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/data/platform/manager/FeatureFlagManagerTest.kt @@ -43,7 +43,7 @@ class FeatureFlagManagerTest { // This should trigger a new server config to be fetched fakeServerConfigRepository.serverConfigValue = SERVER_CONFIG - manager.getFeatureFlagFlow(FlagKey.EmailVerification).test { + manager.getFeatureFlagFlow(FlagKey.DummyBoolean).test { assertNotNull( awaitItem(), ) @@ -55,7 +55,7 @@ class FeatureFlagManagerTest { runTest { fakeServerConfigRepository.serverConfigValue = null - manager.getFeatureFlagFlow(FlagKey.EmailVerification).test { + manager.getFeatureFlagFlow(FlagKey.DummyBoolean).test { assertFalse( awaitItem(), ) @@ -65,7 +65,7 @@ class FeatureFlagManagerTest { @Test fun `getFeatureFlag Boolean should return value if exists`() = runTest { val flagValue = manager.getFeatureFlag( - key = FlagKey.EmailVerification, + key = FlagKey.DummyBoolean, forceRefresh = true, ) assertTrue(flagValue) @@ -99,7 +99,7 @@ class FeatureFlagManagerTest { ) val flagValue = manager.getFeatureFlag( - key = FlagKey.DummyInt, + key = FlagKey.DummyInt(), forceRefresh = false, ) @@ -120,7 +120,7 @@ class FeatureFlagManagerTest { ) val flagValue = manager.getFeatureFlag( - key = FlagKey.DummyInt, + key = FlagKey.DummyInt(), forceRefresh = false, ) @@ -192,7 +192,22 @@ class FeatureFlagManagerTest { fakeServerConfigRepository.serverConfigValue = null val flagValue = manager.getFeatureFlag( - key = FlagKey.DummyInt, + key = FlagKey.DummyInt(), + forceRefresh = false, + ) + + assertEquals( + Int.MIN_VALUE, + flagValue, + ) + } + + @Test + fun `getFeatureFlag Int should return default value when not remotely controlled`() = runTest { + fakeServerConfigRepository.serverConfigValue = null + + val flagValue = manager.getFeatureFlag( + key = FlagKey.DummyInt(isRemotelyConfigured = false), forceRefresh = false, ) @@ -225,7 +240,7 @@ class FeatureFlagManagerTest { ), ) - val flagValue = manager.getFeatureFlag(key = FlagKey.DummyInt) + val flagValue = manager.getFeatureFlag(key = FlagKey.DummyInt()) assertEquals(Int.MIN_VALUE, flagValue) } @@ -273,7 +288,7 @@ private val SERVER_CONFIG = ServerConfig( ssoUrl = "http://localhost:51822", ), featureStates = mapOf( - "email-verification" to JsonPrimitive(true), + "dummy-boolean" to JsonPrimitive(true), "flexible-collections-v-1" to JsonPrimitive(false), ), ), diff --git a/app/src/test/java/com/x8bit/bitwarden/data/platform/repository/util/FakeServerConfigRepository.kt b/app/src/test/java/com/x8bit/bitwarden/data/platform/repository/util/FakeServerConfigRepository.kt index e10787fb0..a5d3239c8 100644 --- a/app/src/test/java/com/x8bit/bitwarden/data/platform/repository/util/FakeServerConfigRepository.kt +++ b/app/src/test/java/com/x8bit/bitwarden/data/platform/repository/util/FakeServerConfigRepository.kt @@ -14,8 +14,8 @@ class FakeServerConfigRepository : ServerConfigRepository { var serverConfigValue: ServerConfig? get() = mutableServerConfigFlow.value set(value) { - mutableServerConfigFlow.value = value - } + mutableServerConfigFlow.value = value + } private val mutableServerConfigFlow = MutableStateFlow(SERVER_CONFIG) @@ -52,7 +52,7 @@ private val SERVER_CONFIG = ServerConfig( featureStates = mapOf( "duo-redirect" to JsonPrimitive(true), "flexible-collections-v-1" to JsonPrimitive(false), - "email-verification" to JsonPrimitive(true), + "dummy-boolean" to JsonPrimitive(true), ), ), )