Merge pull request #1703 from vector-im/feature/bma/featureFlagFlow

Change FeatureFlagService.isFeatureEnabled return value from Boolean to Flow<Boolean>
This commit is contained in:
Benoit Marty
2023-10-31 17:14:59 +01:00
committed by GitHub
11 changed files with 70 additions and 38 deletions

View File

@@ -24,6 +24,7 @@ import io.element.android.libraries.featureflag.api.FeatureFlags
import io.element.android.libraries.featureflag.test.FakeFeatureFlagService
import io.element.android.libraries.featureflag.test.InMemoryPreferencesStore
import io.element.android.tests.testutils.WarmUpRule
import io.element.android.tests.testutils.awaitLastSequentialItem
import kotlinx.coroutines.test.runTest
import org.junit.Rule
import org.junit.Test
@@ -41,10 +42,10 @@ class AdvancedSettingsPresenterTest {
moleculeFlow(RecompositionMode.Immediate) {
presenter.present()
}.test {
val initialState = awaitItem()
val initialState = awaitLastSequentialItem()
assertThat(initialState.isDeveloperModeEnabled).isFalse()
assertThat(initialState.isRichTextEditorEnabled).isFalse()
assertThat(initialState.customElementCallBaseUrlState).isNull()
assertThat(initialState.customElementCallBaseUrlState?.baseUrl).isNull()
}
}
@@ -56,7 +57,7 @@ class AdvancedSettingsPresenterTest {
moleculeFlow(RecompositionMode.Immediate) {
presenter.present()
}.test {
val initialState = awaitItem()
val initialState = awaitLastSequentialItem()
assertThat(initialState.isDeveloperModeEnabled).isFalse()
initialState.eventSink.invoke(AdvancedSettingsEvents.SetDeveloperModeEnabled(true))
assertThat(awaitItem().isDeveloperModeEnabled).isTrue()
@@ -73,7 +74,7 @@ class AdvancedSettingsPresenterTest {
moleculeFlow(RecompositionMode.Immediate) {
presenter.present()
}.test {
val initialState = awaitItem()
val initialState = awaitLastSequentialItem()
assertThat(initialState.isRichTextEditorEnabled).isFalse()
initialState.eventSink.invoke(AdvancedSettingsEvents.SetRichTextEditorEnabled(true))
assertThat(awaitItem().isRichTextEditorEnabled).isTrue()
@@ -92,7 +93,7 @@ class AdvancedSettingsPresenterTest {
moleculeFlow(RecompositionMode.Immediate) {
presenter.present()
}.test {
val initialState = awaitItem()
val initialState = awaitLastSequentialItem()
assertThat(initialState.customElementCallBaseUrlState).isNull()
}
}
@@ -105,10 +106,7 @@ class AdvancedSettingsPresenterTest {
moleculeFlow(RecompositionMode.Immediate) {
presenter.present()
}.test {
// Initial state has a default `false` feature flag value, so the state will still be null
skipItems(1)
val initialState = awaitItem()
val initialState = awaitLastSequentialItem()
assertThat(initialState.customElementCallBaseUrlState).isNotNull()
assertThat(initialState.customElementCallBaseUrlState?.baseUrl).isNull()
@@ -128,10 +126,7 @@ class AdvancedSettingsPresenterTest {
moleculeFlow(RecompositionMode.Immediate) {
presenter.present()
}.test {
// Initial state has a default `false` feature flag value, so the state will still be null
skipItems(1)
val urlValidator = awaitItem().customElementCallBaseUrlState!!.validator
val urlValidator = awaitLastSequentialItem().customElementCallBaseUrlState!!.validator
assertThat(urlValidator("")).isTrue() // We allow empty string to clear the value and use the default one
assertThat(urlValidator("test")).isFalse()
assertThat(urlValidator("http://")).isFalse()

View File

@@ -16,13 +16,23 @@
package io.element.android.libraries.featureflag.api
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.first
interface FeatureFlagService {
/**
* @param feature the feature to check for
*
* @return true if the feature is enabled
*/
suspend fun isFeatureEnabled(feature: Feature): Boolean
suspend fun isFeatureEnabled(feature: Feature): Boolean = isFeatureEnabledFlow(feature).first()
/**
* @param feature the feature to check for
*
* @return a flow of booleans, true if the feature is enabled, false if it is disabled.
*/
fun isFeatureEnabledFlow(feature: Feature): Flow<Boolean>
/**
* @param feature the feature to enable or disable

View File

@@ -35,6 +35,7 @@ dependencies {
implementation(libs.androidx.datastore.preferences)
implementation(projects.libraries.di)
implementation(projects.libraries.core)
implementation(libs.coroutines.core)
testImplementation(libs.test.junit)
testImplementation(libs.coroutines.test)
testImplementation(libs.test.truth)

View File

@@ -21,6 +21,8 @@ import io.element.android.libraries.di.AppScope
import io.element.android.libraries.di.SingleIn
import io.element.android.libraries.featureflag.api.Feature
import io.element.android.libraries.featureflag.api.FeatureFlagService
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.flowOf
import javax.inject.Inject
@ContributesBinding(AppScope::class)
@@ -29,12 +31,12 @@ class DefaultFeatureFlagService @Inject constructor(
private val providers: Set<@JvmSuppressWildcards FeatureFlagProvider>
) : FeatureFlagService {
override suspend fun isFeatureEnabled(feature: Feature): Boolean {
override fun isFeatureEnabledFlow(feature: Feature): Flow<Boolean> {
return providers.filter { it.hasFeature(feature) }
.sortedByDescending(FeatureFlagProvider::priority)
.firstOrNull()
?.isFeatureEnabled(feature)
?: feature.defaultValue
?.isFeatureEnabledFlow(feature)
?: flowOf(feature.defaultValue)
}
override suspend fun setFeatureEnabled(feature: Feature, enabled: Boolean): Boolean {

View File

@@ -17,10 +17,11 @@
package io.element.android.libraries.featureflag.impl
import io.element.android.libraries.featureflag.api.Feature
import kotlinx.coroutines.flow.Flow
interface FeatureFlagProvider {
val priority: Int
suspend fun isFeatureEnabled(feature: Feature): Boolean
fun isFeatureEnabledFlow(feature: Feature): Flow<Boolean>
fun hasFeature(feature: Feature): Boolean
}

View File

@@ -24,7 +24,8 @@ import androidx.datastore.preferences.core.edit
import androidx.datastore.preferences.preferencesDataStore
import io.element.android.libraries.di.ApplicationContext
import io.element.android.libraries.featureflag.api.Feature
import kotlinx.coroutines.flow.first
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.distinctUntilChanged
import kotlinx.coroutines.flow.map
import javax.inject.Inject
@@ -44,10 +45,10 @@ class PreferencesFeatureFlagProvider @Inject constructor(@ApplicationContext con
}
}
override suspend fun isFeatureEnabled(feature: Feature): Boolean {
override fun isFeatureEnabledFlow(feature: Feature): Flow<Boolean> {
return store.data.map { prefs ->
prefs[booleanPreferencesKey(feature.key)] ?: feature.defaultValue
}.first()
}.distinctUntilChanged()
}
override fun hasFeature(feature: Feature): Boolean {

View File

@@ -18,6 +18,8 @@ package io.element.android.libraries.featureflag.impl
import io.element.android.libraries.featureflag.api.Feature
import io.element.android.libraries.featureflag.api.FeatureFlags
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.flowOf
import javax.inject.Inject
/**
@@ -29,9 +31,9 @@ class StaticFeatureFlagProvider @Inject constructor() :
override val priority = LOW_PRIORITY
override suspend fun isFeatureEnabled(feature: Feature): Boolean {
return if (feature is FeatureFlags) {
when(feature) {
override fun isFeatureEnabledFlow(feature: Feature): Flow<Boolean> {
val isFeatureEnabled = if (feature is FeatureFlags) {
when (feature) {
FeatureFlags.LocationSharing -> true
FeatureFlags.Polls -> true
FeatureFlags.NotificationSettings -> true
@@ -43,6 +45,7 @@ class StaticFeatureFlagProvider @Inject constructor() :
} else {
false
}
return flowOf(isFeatureEnabled)
}
override fun hasFeature(feature: Feature) = true

View File

@@ -16,6 +16,7 @@
package io.element.android.libraries.featureflag.impl
import app.cash.turbine.test
import com.google.common.truth.Truth.assertThat
import io.element.android.libraries.featureflag.api.FeatureFlags
import kotlinx.coroutines.test.runTest
@@ -26,8 +27,10 @@ class DefaultFeatureFlagServiceTest {
@Test
fun `given service without provider when feature is checked then it returns the default value`() = runTest {
val featureFlagService = DefaultFeatureFlagService(emptySet())
val isFeatureEnabled = featureFlagService.isFeatureEnabled(FeatureFlags.LocationSharing)
assertThat(isFeatureEnabled).isEqualTo(FeatureFlags.LocationSharing.defaultValue)
featureFlagService.isFeatureEnabledFlow(FeatureFlags.LocationSharing).test {
assertThat(awaitItem()).isEqualTo(FeatureFlags.LocationSharing.defaultValue)
cancelAndIgnoreRemainingEvents()
}
}
@Test
@@ -50,9 +53,11 @@ class DefaultFeatureFlagServiceTest {
val featureFlagProvider = FakeMutableFeatureFlagProvider(0)
val featureFlagService = DefaultFeatureFlagService(setOf(featureFlagProvider))
featureFlagService.setFeatureEnabled(FeatureFlags.LocationSharing, true)
assertThat(featureFlagService.isFeatureEnabled(FeatureFlags.LocationSharing)).isEqualTo(true)
featureFlagService.setFeatureEnabled(FeatureFlags.LocationSharing, false)
assertThat(featureFlagService.isFeatureEnabled(FeatureFlags.LocationSharing)).isEqualTo(false)
featureFlagService.isFeatureEnabledFlow(FeatureFlags.LocationSharing).test {
assertThat(awaitItem()).isEqualTo(true)
featureFlagService.setFeatureEnabled(FeatureFlags.LocationSharing, false)
assertThat(awaitItem()).isEqualTo(false)
}
}
@Test
@@ -62,6 +67,8 @@ class DefaultFeatureFlagServiceTest {
val featureFlagService = DefaultFeatureFlagService(setOf(lowPriorityFeatureFlagProvider, highPriorityFeatureFlagProvider))
lowPriorityFeatureFlagProvider.setFeatureEnabled(FeatureFlags.LocationSharing, false)
highPriorityFeatureFlagProvider.setFeatureEnabled(FeatureFlags.LocationSharing, true)
assertThat(featureFlagService.isFeatureEnabled(FeatureFlags.LocationSharing)).isEqualTo(true)
featureFlagService.isFeatureEnabledFlow(FeatureFlags.LocationSharing).test {
assertThat(awaitItem()).isEqualTo(true)
}
}
}

View File

@@ -17,17 +17,20 @@
package io.element.android.libraries.featureflag.impl
import io.element.android.libraries.featureflag.api.Feature
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.MutableStateFlow
class FakeMutableFeatureFlagProvider(override val priority: Int) : MutableFeatureFlagProvider {
private val enabledFeatures = HashMap<String, Boolean>()
private val enabledFeatures = mutableMapOf<String, MutableStateFlow<Boolean>>()
override suspend fun setFeatureEnabled(feature: Feature, enabled: Boolean) {
enabledFeatures[feature.key] = enabled
val flow = enabledFeatures.getOrPut(feature.key) { MutableStateFlow(enabled) }
flow.emit(enabled)
}
override suspend fun isFeatureEnabled(feature: Feature): Boolean {
return enabledFeatures[feature.key] ?: feature.defaultValue
override fun isFeatureEnabledFlow(feature: Feature): Flow<Boolean> {
return enabledFeatures.getOrPut(feature.key) { MutableStateFlow(feature.defaultValue) }
}
override fun hasFeature(feature: Feature): Boolean = true

View File

@@ -23,5 +23,6 @@ android {
dependencies {
api(projects.libraries.featureflag.api)
implementation(libs.coroutines.core)
}
}

View File

@@ -18,19 +18,27 @@ package io.element.android.libraries.featureflag.test
import io.element.android.libraries.featureflag.api.Feature
import io.element.android.libraries.featureflag.api.FeatureFlagService
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.MutableStateFlow
class FakeFeatureFlagService(
initialState: Map<String, Boolean> = emptyMap()
) : FeatureFlagService {
private val enabledFeatures = HashMap(initialState)
private val enabledFeatures = initialState
.map {
it.key to MutableStateFlow(it.value)
}
.toMap()
.toMutableMap()
override suspend fun setFeatureEnabled(feature: Feature, enabled: Boolean): Boolean {
enabledFeatures[feature.key] = enabled
val flow = enabledFeatures.getOrPut(feature.key) { MutableStateFlow(enabled) }
flow.emit(enabled)
return true
}
override suspend fun isFeatureEnabled(feature: Feature): Boolean {
return enabledFeatures[feature.key] ?: false
override fun isFeatureEnabledFlow(feature: Feature): Flow<Boolean> {
return enabledFeatures.getOrPut(feature.key) { MutableStateFlow(feature.defaultValue) }
}
}