From 04b9d3cc2cba0d23e7e3760b7e7b7abfd7e040c0 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Thu, 7 Sep 2023 10:29:10 +0200 Subject: [PATCH] Rework the feature flag module. Fix typo, rename class and interface, add doc, do small refacto, to improve code clarity. --- .../featureflag/api/FeatureFlagService.kt | 3 ++- .../impl/DefaultFeatureFlagService.kt | 2 +- ...ovider.kt => MutableFeatureFlagProvider.kt} | 2 +- .../impl/PreferencesFeatureFlagProvider.kt | 9 +++++---- ...rovider.kt => StaticFeatureFlagProvider.kt} | 9 ++++++--- .../featureflag/impl/di/FeatureFlagModule.kt | 18 +++++++++++------- .../impl/DefaultFeatureFlagServiceTest.kt | 14 +++++++------- ...er.kt => FakeMutableFeatureFlagProvider.kt} | 2 +- 8 files changed, 34 insertions(+), 25 deletions(-) rename libraries/featureflag/impl/src/main/kotlin/io/element/android/libraries/featureflag/impl/{RuntimeFeatureFlagProvider.kt => MutableFeatureFlagProvider.kt} (92%) rename libraries/featureflag/impl/src/main/kotlin/io/element/android/libraries/featureflag/impl/{BuildtimeFeatureFlagProvider.kt => StaticFeatureFlagProvider.kt} (84%) rename libraries/featureflag/impl/src/test/kotlin/io/element/android/libraries/featureflag/impl/{FakeRuntimeFeatureFlagProvider.kt => FakeMutableFeatureFlagProvider.kt} (92%) 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 59e224a1ae..8089c837ff 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 @@ -28,7 +28,8 @@ interface FeatureFlagService { * @param feature the feature to enable or disable * @param enabled true to enable the feature * - * @return true if the method succeeds, ie if a RuntimeFeatureFlagProvider is registered + * @return true if the method succeeds, ie if a [io.element.android.libraries.featureflag.impl.MutableFeatureFlagProvider] + * is registered */ suspend fun setFeatureEnabled(feature: Feature, enabled: Boolean): Boolean } 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 7298929aea..b445599693 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 @@ -38,7 +38,7 @@ class DefaultFeatureFlagService @Inject constructor( } override suspend fun setFeatureEnabled(feature: Feature, enabled: Boolean): Boolean { - return providers.filterIsInstance(RuntimeFeatureFlagProvider::class.java) + return providers.filterIsInstance(MutableFeatureFlagProvider::class.java) .sortedBy(FeatureFlagProvider::priority) .firstOrNull() ?.setFeatureEnabled(feature, enabled) diff --git a/libraries/featureflag/impl/src/main/kotlin/io/element/android/libraries/featureflag/impl/RuntimeFeatureFlagProvider.kt b/libraries/featureflag/impl/src/main/kotlin/io/element/android/libraries/featureflag/impl/MutableFeatureFlagProvider.kt similarity index 92% rename from libraries/featureflag/impl/src/main/kotlin/io/element/android/libraries/featureflag/impl/RuntimeFeatureFlagProvider.kt rename to libraries/featureflag/impl/src/main/kotlin/io/element/android/libraries/featureflag/impl/MutableFeatureFlagProvider.kt index 1238ad354c..7e2da181b6 100644 --- a/libraries/featureflag/impl/src/main/kotlin/io/element/android/libraries/featureflag/impl/RuntimeFeatureFlagProvider.kt +++ b/libraries/featureflag/impl/src/main/kotlin/io/element/android/libraries/featureflag/impl/MutableFeatureFlagProvider.kt @@ -18,6 +18,6 @@ package io.element.android.libraries.featureflag.impl import io.element.android.libraries.featureflag.api.Feature -interface RuntimeFeatureFlagProvider : FeatureFlagProvider { +interface MutableFeatureFlagProvider : FeatureFlagProvider { suspend fun setFeatureEnabled(feature: Feature, enabled: 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 15ab08b338..ddffdebd34 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 @@ -30,12 +30,13 @@ import javax.inject.Inject private val Context.dataStore: DataStore by preferencesDataStore(name = "elementx_featureflag") -class PreferencesFeatureFlagProvider @Inject constructor(@ApplicationContext context: Context) : RuntimeFeatureFlagProvider { - +/** + * Note: this will be used only in the nightly and in the debug build. + */ +class PreferencesFeatureFlagProvider @Inject constructor(@ApplicationContext context: Context) : MutableFeatureFlagProvider { private val store = context.dataStore - override val priority: Int - get() = MEDIUM_PRIORITY + override val priority = MEDIUM_PRIORITY override suspend fun setFeatureEnabled(feature: Feature, enabled: Boolean) { store.edit { prefs -> diff --git a/libraries/featureflag/impl/src/main/kotlin/io/element/android/libraries/featureflag/impl/BuildtimeFeatureFlagProvider.kt b/libraries/featureflag/impl/src/main/kotlin/io/element/android/libraries/featureflag/impl/StaticFeatureFlagProvider.kt similarity index 84% rename from libraries/featureflag/impl/src/main/kotlin/io/element/android/libraries/featureflag/impl/BuildtimeFeatureFlagProvider.kt rename to libraries/featureflag/impl/src/main/kotlin/io/element/android/libraries/featureflag/impl/StaticFeatureFlagProvider.kt index 0117a4aa22..2d66d3be21 100644 --- a/libraries/featureflag/impl/src/main/kotlin/io/element/android/libraries/featureflag/impl/BuildtimeFeatureFlagProvider.kt +++ b/libraries/featureflag/impl/src/main/kotlin/io/element/android/libraries/featureflag/impl/StaticFeatureFlagProvider.kt @@ -20,11 +20,14 @@ import io.element.android.libraries.featureflag.api.Feature import io.element.android.libraries.featureflag.api.FeatureFlags import javax.inject.Inject -class BuildtimeFeatureFlagProvider @Inject constructor() : +/** + * This provider is used for release build. + * Change the value return by [isFeatureEnabled] to enable/disable features. + */ +class StaticFeatureFlagProvider @Inject constructor() : FeatureFlagProvider { - override val priority: Int - get() = LOW_PRIORITY + override val priority = LOW_PRIORITY override suspend fun isFeatureEnabled(feature: Feature): Boolean { return if (feature is FeatureFlags) { diff --git a/libraries/featureflag/impl/src/main/kotlin/io/element/android/libraries/featureflag/impl/di/FeatureFlagModule.kt b/libraries/featureflag/impl/src/main/kotlin/io/element/android/libraries/featureflag/impl/di/FeatureFlagModule.kt index 07ee53ceee..b2f0a4106d 100644 --- a/libraries/featureflag/impl/src/main/kotlin/io/element/android/libraries/featureflag/impl/di/FeatureFlagModule.kt +++ b/libraries/featureflag/impl/src/main/kotlin/io/element/android/libraries/featureflag/impl/di/FeatureFlagModule.kt @@ -22,7 +22,7 @@ import dagger.Provides import dagger.multibindings.ElementsIntoSet import io.element.android.libraries.core.meta.BuildType import io.element.android.libraries.di.AppScope -import io.element.android.libraries.featureflag.impl.BuildtimeFeatureFlagProvider +import io.element.android.libraries.featureflag.impl.StaticFeatureFlagProvider import io.element.android.libraries.featureflag.impl.FeatureFlagProvider import io.element.android.libraries.featureflag.impl.PreferencesFeatureFlagProvider @@ -35,14 +35,18 @@ object FeatureFlagModule { @ElementsIntoSet fun providesFeatureFlagProvider( buildType: BuildType, - runtimeFeatureFlagProvider: PreferencesFeatureFlagProvider, - buildtimeFeatureFlagProvider: BuildtimeFeatureFlagProvider, + mutableFeatureFlagProvider: PreferencesFeatureFlagProvider, + staticFeatureFlagProvider: StaticFeatureFlagProvider, ): Set { val providers = HashSet() - if (buildType == BuildType.RELEASE) { - providers.add(buildtimeFeatureFlagProvider) - } else { - providers.add(runtimeFeatureFlagProvider) + when (buildType) { + BuildType.RELEASE -> { + providers.add(staticFeatureFlagProvider) + } + BuildType.NIGHTLY, + BuildType.DEBUG -> { + providers.add(mutableFeatureFlagProvider) + } } return providers } 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 ea9b03acdf..890959639f 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 @@ -39,7 +39,7 @@ class DefaultFeatureFlagServiceTest { @Test fun `given service with a runtime provider when set enabled feature is called then it returns true`() = runTest { - val featureFlagProvider = FakeRuntimeFeatureFlagProvider(0) + val featureFlagProvider = FakeMutableFeatureFlagProvider(0) val featureFlagService = DefaultFeatureFlagService(setOf(featureFlagProvider)) val result = featureFlagService.setFeatureEnabled(FeatureFlags.LocationSharing, true) assertThat(result).isEqualTo(true) @@ -47,7 +47,7 @@ class DefaultFeatureFlagServiceTest { @Test fun `given service with a runtime provider and feature enabled when feature is checked then it returns the correct value`() = runTest { - val featureFlagProvider = FakeRuntimeFeatureFlagProvider(0) + val featureFlagProvider = FakeMutableFeatureFlagProvider(0) val featureFlagService = DefaultFeatureFlagService(setOf(featureFlagProvider)) featureFlagService.setFeatureEnabled(FeatureFlags.LocationSharing, true) assertThat(featureFlagService.isFeatureEnabled(FeatureFlags.LocationSharing)).isEqualTo(true) @@ -57,11 +57,11 @@ class DefaultFeatureFlagServiceTest { @Test fun `given service with 2 runtime providers when feature is checked then it uses the priority correctly`() = runTest { - val lowPriorityfeatureFlagProvider = FakeRuntimeFeatureFlagProvider(LOW_PRIORITY) - val highPriorityfeatureFlagProvider = FakeRuntimeFeatureFlagProvider(HIGH_PRIORITY) - val featureFlagService = DefaultFeatureFlagService(setOf(lowPriorityfeatureFlagProvider, highPriorityfeatureFlagProvider)) - lowPriorityfeatureFlagProvider.setFeatureEnabled(FeatureFlags.LocationSharing, false) - highPriorityfeatureFlagProvider.setFeatureEnabled(FeatureFlags.LocationSharing, true) + val lowPriorityFeatureFlagProvider = FakeMutableFeatureFlagProvider(LOW_PRIORITY) + val highPriorityFeatureFlagProvider = FakeMutableFeatureFlagProvider(HIGH_PRIORITY) + val featureFlagService = DefaultFeatureFlagService(setOf(lowPriorityFeatureFlagProvider, highPriorityFeatureFlagProvider)) + lowPriorityFeatureFlagProvider.setFeatureEnabled(FeatureFlags.LocationSharing, false) + highPriorityFeatureFlagProvider.setFeatureEnabled(FeatureFlags.LocationSharing, true) assertThat(featureFlagService.isFeatureEnabled(FeatureFlags.LocationSharing)).isEqualTo(true) } } diff --git a/libraries/featureflag/impl/src/test/kotlin/io/element/android/libraries/featureflag/impl/FakeRuntimeFeatureFlagProvider.kt b/libraries/featureflag/impl/src/test/kotlin/io/element/android/libraries/featureflag/impl/FakeMutableFeatureFlagProvider.kt similarity index 92% rename from libraries/featureflag/impl/src/test/kotlin/io/element/android/libraries/featureflag/impl/FakeRuntimeFeatureFlagProvider.kt rename to libraries/featureflag/impl/src/test/kotlin/io/element/android/libraries/featureflag/impl/FakeMutableFeatureFlagProvider.kt index 5ff5cf932f..f1d075ace2 100644 --- a/libraries/featureflag/impl/src/test/kotlin/io/element/android/libraries/featureflag/impl/FakeRuntimeFeatureFlagProvider.kt +++ b/libraries/featureflag/impl/src/test/kotlin/io/element/android/libraries/featureflag/impl/FakeMutableFeatureFlagProvider.kt @@ -18,7 +18,7 @@ package io.element.android.libraries.featureflag.impl import io.element.android.libraries.featureflag.api.Feature -class FakeRuntimeFeatureFlagProvider(override val priority: Int) : RuntimeFeatureFlagProvider { +class FakeMutableFeatureFlagProvider(override val priority: Int) : MutableFeatureFlagProvider { private val enabledFeatures = HashMap()