PM-21080: Remove the isRemotelyConfigured flag (#5193)

This commit is contained in:
David Perez
2025-05-19 10:56:19 -05:00
committed by GitHub
parent 9508b4ba90
commit 046bb0fa39
12 changed files with 24 additions and 220 deletions

View File

@@ -41,7 +41,6 @@ class FeatureFlagManagerImpl(
*/
fun <T : Any> ServerConfig?.getFlagValueOrDefault(key: FlagKey<T>): T {
val defaultValue = key.defaultValue
if (!key.isRemotelyConfigured) return key.defaultValue
return this
?.serverData
?.featureStates
@@ -56,9 +55,9 @@ fun <T : Any> ServerConfig?.getFlagValueOrDefault(key: FlagKey<T>): T {
Int::class -> it.content.toInt() as T
else -> defaultValue
}
} catch (ex: ClassCastException) {
} catch (_: ClassCastException) {
defaultValue
} catch (ex: NumberFormatException) {
} catch (_: NumberFormatException) {
defaultValue
}
}

View File

@@ -14,11 +14,6 @@ sealed class FlagKey<out T : Any> {
*/
abstract val defaultValue: T
/**
* Indicates if the flag should respect the network value or not.
*/
abstract val isRemotelyConfigured: Boolean
@Suppress("UndocumentedPublicClass")
companion object {
/**
@@ -38,7 +33,6 @@ sealed class FlagKey<out T : Any> {
data object BitwardenAuthenticationEnabled : FlagKey<Boolean>() {
override val keyName: String = "bitwarden-authentication-enabled"
override val defaultValue: Boolean = false
override val isRemotelyConfigured: Boolean = false
}
/**
@@ -47,7 +41,6 @@ sealed class FlagKey<out T : Any> {
data object PasswordManagerSync : FlagKey<Boolean>() {
override val keyName: String = "enable-pm-bwa-sync"
override val defaultValue: Boolean = false
override val isRemotelyConfigured: Boolean = true
}
/**
@@ -56,15 +49,12 @@ sealed class FlagKey<out T : Any> {
data object DummyBoolean : FlagKey<Boolean>() {
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<Int>() {
data object DummyInt : FlagKey<Int>() {
override val keyName: String = "dummy-int"
override val defaultValue: Int = Int.MIN_VALUE
}
@@ -75,6 +65,5 @@ sealed class FlagKey<out T : Any> {
data object DummyString : FlagKey<String>() {
override val keyName: String = "dummy-string"
override val defaultValue: String = "defaultValue"
override val isRemotelyConfigured: Boolean = true
}
}

View File

@@ -81,7 +81,7 @@ class FeatureFlagOverrideDiskSourceTest {
@Test
fun `call to save feature flag should update SharedPreferences for ints`() {
val key = FlagKey.DummyInt()
val key = FlagKey.DummyInt
assertEquals(
fakeSharedPreferences.getInt(
"$BASE_STORAGE_PREFIX${key.keyName}",
@@ -102,7 +102,7 @@ class FeatureFlagOverrideDiskSourceTest {
@Test
fun `call to get feature flag should return correct value for ints`() {
val key = FlagKey.DummyInt()
val key = FlagKey.DummyInt
assertNull(featureFlagOverrideDiskSource.getFeatureFlag(key))
val expectedValue = 1
fakeSharedPreferences.edit {

View File

@@ -90,7 +90,7 @@ class FeatureFlagManagerTest {
)
val flagValue = manager.getFeatureFlag(
key = FlagKey.DummyInt(),
key = FlagKey.DummyInt,
forceRefresh = false,
)
@@ -111,7 +111,7 @@ class FeatureFlagManagerTest {
)
val flagValue = manager.getFeatureFlag(
key = FlagKey.DummyInt(),
key = FlagKey.DummyInt,
forceRefresh = false,
)
@@ -183,22 +183,7 @@ class FeatureFlagManagerTest {
fakeServerConfigRepository.serverConfigValue = null
val flagValue = manager.getFeatureFlag(
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),
key = FlagKey.DummyInt,
forceRefresh = false,
)
@@ -231,7 +216,7 @@ class FeatureFlagManagerTest {
),
)
val flagValue = manager.getFeatureFlag(key = FlagKey.DummyInt())
val flagValue = manager.getFeatureFlag(key = FlagKey.DummyInt)
assertEquals(Int.MIN_VALUE, flagValue)
}

View File

@@ -2,7 +2,6 @@ package com.bitwarden.authenticator.data.platform.manager
import com.bitwarden.authenticator.data.platform.manager.model.FlagKey
import org.junit.jupiter.api.Assertions.assertEquals
import org.junit.jupiter.api.Assertions.assertFalse
import org.junit.jupiter.api.Assertions.assertTrue
import org.junit.jupiter.api.Test
@@ -30,10 +29,4 @@ class FlagKeyTest {
},
)
}
@Test
fun `All feature flags are correctly set to be remotely configured`() {
assertTrue(FlagKey.PasswordManagerSync.isRemotelyConfigured)
assertFalse(FlagKey.BitwardenAuthenticationEnabled.isRemotelyConfigured)
}
}

View File

@@ -5,14 +5,12 @@ import com.bitwarden.authenticator.data.platform.datasource.disk.FeatureFlagOver
import com.bitwarden.authenticator.data.platform.manager.model.FlagKey
import com.bitwarden.data.datasource.disk.model.ServerConfig
import com.bitwarden.data.repository.ServerConfigRepository
import com.bitwarden.network.model.ConfigResponseJson
import io.mockk.every
import io.mockk.just
import io.mockk.mockk
import io.mockk.verify
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.test.runTest
import kotlinx.serialization.json.JsonPrimitive
import org.junit.jupiter.api.Assertions
import org.junit.jupiter.api.Test
@@ -21,7 +19,7 @@ class DebugMenuRepositoryTest {
mockk<FeatureFlagOverrideDiskSource> {
every { getFeatureFlag(FlagKey.DummyBoolean) } returns true
every { getFeatureFlag(FlagKey.DummyString) } returns TEST_STRING_VALUE
every { getFeatureFlag(FlagKey.DummyInt()) } returns TEST_INT_VALUE
every { getFeatureFlag(FlagKey.DummyInt) } returns TEST_INT_VALUE
every { saveFeatureFlag(any(), any()) } just io.mockk.runs
}
private val mutableServerConfigStateFlow =
@@ -76,7 +74,7 @@ class DebugMenuRepositoryTest {
fun `getFeatureFlag should return the feature flag string value from disk`() {
Assertions.assertEquals(
TEST_STRING_VALUE,
debugMenuRepository.getFeatureFlag(FlagKey.DummyString)!!,
debugMenuRepository.getFeatureFlag(FlagKey.DummyString),
)
}
@@ -84,7 +82,7 @@ class DebugMenuRepositoryTest {
fun `getFeatureFlag should return the feature flag int value from disk`() {
Assertions.assertEquals(
TEST_INT_VALUE,
debugMenuRepository.getFeatureFlag(FlagKey.DummyInt())!!,
debugMenuRepository.getFeatureFlag(FlagKey.DummyInt),
)
}
@@ -115,53 +113,6 @@ class DebugMenuRepositoryTest {
expectNoEvents()
}
}
@Suppress("MaxLineLength")
@Test
fun `resetFeatureFlagOverrides should save all feature flags to values from the server config if remote configured is on`() =
runTest {
val mockServerData =
mockk<ConfigResponseJson>(
relaxed = true,
) {
every { featureStates } returns mapOf(
FlagKey.PasswordManagerSync.keyName to JsonPrimitive(
true,
),
FlagKey.BitwardenAuthenticationEnabled.keyName to JsonPrimitive(
false,
),
)
}
val mockServerConfig =
mockk<ServerConfig>(
relaxed = true,
) {
every { serverData } returns mockServerData
}
mutableServerConfigStateFlow.value = mockServerConfig
debugMenuRepository.resetFeatureFlagOverrides()
Assertions.assertTrue(FlagKey.PasswordManagerSync.isRemotelyConfigured)
Assertions.assertFalse(FlagKey.BitwardenAuthenticationEnabled.isRemotelyConfigured)
verify(exactly = 1) {
mockFeatureFlagOverrideDiskSource.saveFeatureFlag(
FlagKey.PasswordManagerSync,
true,
)
mockFeatureFlagOverrideDiskSource.saveFeatureFlag(
FlagKey.BitwardenAuthenticationEnabled,
false,
)
}
debugMenuRepository.featureFlagOverridesUpdatedFlow.test {
awaitItem() // initial value on subscription
awaitItem()
cancel()
}
}
}
private const val TEST_STRING_VALUE = "test"