From f554fb8dcc42482707db3a4af3eea7f2732f4875 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Tue, 31 Oct 2023 16:48:24 +0100 Subject: [PATCH] Change FeatureFlagService.isFeatureEnabled return value from Boolean to Flow --- .../featureflag/api/FeatureFlagService.kt | 12 +++++++++++- libraries/featureflag/impl/build.gradle.kts | 1 + .../impl/DefaultFeatureFlagService.kt | 8 +++++--- .../featureflag/impl/FeatureFlagProvider.kt | 3 ++- .../impl/PreferencesFeatureFlagProvider.kt | 7 ++++--- .../impl/StaticFeatureFlagProvider.kt | 9 ++++++--- .../impl/DefaultFeatureFlagServiceTest.kt | 19 +++++++++++++------ .../impl/FakeMutableFeatureFlagProvider.kt | 11 +++++++---- libraries/featureflag/test/build.gradle.kts | 1 + .../test/FakeFeatureFlagService.kt | 16 ++++++++++++---- 10 files changed, 62 insertions(+), 25 deletions(-) diff --git a/libraries/featureflag/api/src/main/kotlin/io/element/android/libraries/featureflag/api/FeatureFlagService.kt b/libraries/featureflag/api/src/main/kotlin/io/element/android/libraries/featureflag/api/FeatureFlagService.kt index 8089c837ff..cd0c8937d7 100644 --- a/libraries/featureflag/api/src/main/kotlin/io/element/android/libraries/featureflag/api/FeatureFlagService.kt +++ b/libraries/featureflag/api/src/main/kotlin/io/element/android/libraries/featureflag/api/FeatureFlagService.kt @@ -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 /** * @param feature the feature to enable or disable diff --git a/libraries/featureflag/impl/build.gradle.kts b/libraries/featureflag/impl/build.gradle.kts index ba0747b28e..64fcd1eece 100644 --- a/libraries/featureflag/impl/build.gradle.kts +++ b/libraries/featureflag/impl/build.gradle.kts @@ -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) diff --git a/libraries/featureflag/impl/src/main/kotlin/io/element/android/libraries/featureflag/impl/DefaultFeatureFlagService.kt b/libraries/featureflag/impl/src/main/kotlin/io/element/android/libraries/featureflag/impl/DefaultFeatureFlagService.kt index b445599693..fd6f1b1f47 100644 --- a/libraries/featureflag/impl/src/main/kotlin/io/element/android/libraries/featureflag/impl/DefaultFeatureFlagService.kt +++ b/libraries/featureflag/impl/src/main/kotlin/io/element/android/libraries/featureflag/impl/DefaultFeatureFlagService.kt @@ -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 { 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 { diff --git a/libraries/featureflag/impl/src/main/kotlin/io/element/android/libraries/featureflag/impl/FeatureFlagProvider.kt b/libraries/featureflag/impl/src/main/kotlin/io/element/android/libraries/featureflag/impl/FeatureFlagProvider.kt index 833d9f9003..e40d59ec5b 100644 --- a/libraries/featureflag/impl/src/main/kotlin/io/element/android/libraries/featureflag/impl/FeatureFlagProvider.kt +++ b/libraries/featureflag/impl/src/main/kotlin/io/element/android/libraries/featureflag/impl/FeatureFlagProvider.kt @@ -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 fun hasFeature(feature: Feature): Boolean } diff --git a/libraries/featureflag/impl/src/main/kotlin/io/element/android/libraries/featureflag/impl/PreferencesFeatureFlagProvider.kt b/libraries/featureflag/impl/src/main/kotlin/io/element/android/libraries/featureflag/impl/PreferencesFeatureFlagProvider.kt index ddffdebd34..76344e078c 100644 --- a/libraries/featureflag/impl/src/main/kotlin/io/element/android/libraries/featureflag/impl/PreferencesFeatureFlagProvider.kt +++ b/libraries/featureflag/impl/src/main/kotlin/io/element/android/libraries/featureflag/impl/PreferencesFeatureFlagProvider.kt @@ -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 { return store.data.map { prefs -> prefs[booleanPreferencesKey(feature.key)] ?: feature.defaultValue - }.first() + }.distinctUntilChanged() } override fun hasFeature(feature: Feature): Boolean { diff --git a/libraries/featureflag/impl/src/main/kotlin/io/element/android/libraries/featureflag/impl/StaticFeatureFlagProvider.kt b/libraries/featureflag/impl/src/main/kotlin/io/element/android/libraries/featureflag/impl/StaticFeatureFlagProvider.kt index 82471c5983..e9dcc88a21 100644 --- a/libraries/featureflag/impl/src/main/kotlin/io/element/android/libraries/featureflag/impl/StaticFeatureFlagProvider.kt +++ b/libraries/featureflag/impl/src/main/kotlin/io/element/android/libraries/featureflag/impl/StaticFeatureFlagProvider.kt @@ -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 { + 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 diff --git a/libraries/featureflag/impl/src/test/kotlin/io/element/android/libraries/featureflag/impl/DefaultFeatureFlagServiceTest.kt b/libraries/featureflag/impl/src/test/kotlin/io/element/android/libraries/featureflag/impl/DefaultFeatureFlagServiceTest.kt index 890959639f..117005fd6f 100644 --- a/libraries/featureflag/impl/src/test/kotlin/io/element/android/libraries/featureflag/impl/DefaultFeatureFlagServiceTest.kt +++ b/libraries/featureflag/impl/src/test/kotlin/io/element/android/libraries/featureflag/impl/DefaultFeatureFlagServiceTest.kt @@ -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) + } } } diff --git a/libraries/featureflag/impl/src/test/kotlin/io/element/android/libraries/featureflag/impl/FakeMutableFeatureFlagProvider.kt b/libraries/featureflag/impl/src/test/kotlin/io/element/android/libraries/featureflag/impl/FakeMutableFeatureFlagProvider.kt index f1d075ace2..20242cca69 100644 --- a/libraries/featureflag/impl/src/test/kotlin/io/element/android/libraries/featureflag/impl/FakeMutableFeatureFlagProvider.kt +++ b/libraries/featureflag/impl/src/test/kotlin/io/element/android/libraries/featureflag/impl/FakeMutableFeatureFlagProvider.kt @@ -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() + private val enabledFeatures = mutableMapOf>() 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 { + return enabledFeatures.getOrPut(feature.key) { MutableStateFlow(feature.defaultValue) } } override fun hasFeature(feature: Feature): Boolean = true diff --git a/libraries/featureflag/test/build.gradle.kts b/libraries/featureflag/test/build.gradle.kts index 952b9323f6..3c3b199ce6 100644 --- a/libraries/featureflag/test/build.gradle.kts +++ b/libraries/featureflag/test/build.gradle.kts @@ -23,5 +23,6 @@ android { dependencies { api(projects.libraries.featureflag.api) + implementation(libs.coroutines.core) } } diff --git a/libraries/featureflag/test/src/main/java/io/element/android/libraries/featureflag/test/FakeFeatureFlagService.kt b/libraries/featureflag/test/src/main/java/io/element/android/libraries/featureflag/test/FakeFeatureFlagService.kt index 9c86bad752..18c9920d74 100644 --- a/libraries/featureflag/test/src/main/java/io/element/android/libraries/featureflag/test/FakeFeatureFlagService.kt +++ b/libraries/featureflag/test/src/main/java/io/element/android/libraries/featureflag/test/FakeFeatureFlagService.kt @@ -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 = 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 { + return enabledFeatures.getOrPut(feature.key) { MutableStateFlow(feature.defaultValue) } } }