Merge pull request #5557 from element-hq/feature/bma/sortFF

Sort feature flags
This commit is contained in:
Benoit Marty 2025-10-17 15:26:39 +02:00 committed by GitHub
commit fa8ddba1f5
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
18 changed files with 337 additions and 203 deletions

View file

@ -35,7 +35,12 @@ interface FeatureFlagService {
suspend fun setFeatureEnabled(feature: Feature, enabled: Boolean): Boolean
/**
* @return the list of available (not finished) features that can be toggled.
* @return the list of available features that can be toggled.
* @param includeFinishedFeatures whether to include finished features, default is false
* @param isInLabs whether the user is in labs (to include lab features), default is false
*/
fun getAvailableFeatures(): List<Feature>
fun getAvailableFeatures(
includeFinishedFeatures: Boolean = false,
isInLabs: Boolean = false,
): List<Feature>
}

View file

@ -31,4 +31,5 @@ dependencies {
testCommonDependencies(libs)
testImplementation(projects.libraries.matrix.test)
testImplementation(projects.libraries.featureflag.test)
}

View file

@ -14,7 +14,6 @@ import dev.zacsweers.metro.SingleIn
import io.element.android.libraries.core.meta.BuildMeta
import io.element.android.libraries.featureflag.api.Feature
import io.element.android.libraries.featureflag.api.FeatureFlagService
import io.element.android.libraries.featureflag.api.FeatureFlags
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.flowOf
@ -24,25 +23,30 @@ import kotlinx.coroutines.flow.flowOf
class DefaultFeatureFlagService(
private val providers: Set<@JvmSuppressWildcards FeatureFlagProvider>,
private val buildMeta: BuildMeta,
private val featuresProvider: FeaturesProvider,
) : FeatureFlagService {
override fun isFeatureEnabledFlow(feature: Feature): Flow<Boolean> {
return providers.filter { it.hasFeature(feature) }
.sortedByDescending(FeatureFlagProvider::priority)
.firstOrNull()
.maxByOrNull(FeatureFlagProvider::priority)
?.isFeatureEnabledFlow(feature)
?: flowOf(feature.defaultValue(buildMeta))
}
override suspend fun setFeatureEnabled(feature: Feature, enabled: Boolean): Boolean {
return providers.filterIsInstance<MutableFeatureFlagProvider>()
.sortedBy(FeatureFlagProvider::priority)
.firstOrNull()
.maxByOrNull(FeatureFlagProvider::priority)
?.setFeatureEnabled(feature, enabled)
?.let { true }
?: false
}
override fun getAvailableFeatures(): List<Feature> {
return FeatureFlags.entries.filter { !it.isFinished }
override fun getAvailableFeatures(
includeFinishedFeatures: Boolean,
isInLabs: Boolean,
): List<Feature> {
return featuresProvider.provide().filter { flag ->
(includeFinishedFeatures || !flag.isFinished) &&
flag.isInLabs == isInLabs
}
}
}

View file

@ -0,0 +1,24 @@
/*
* Copyright 2025 New Vector Ltd.
*
* SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial
* Please see LICENSE files in the repository root for full details.
*/
package io.element.android.libraries.featureflag.impl
import dev.zacsweers.metro.AppScope
import dev.zacsweers.metro.ContributesBinding
import dev.zacsweers.metro.Inject
import io.element.android.libraries.featureflag.api.Feature
import io.element.android.libraries.featureflag.api.FeatureFlags
fun interface FeaturesProvider {
fun provide(): List<Feature>
}
@ContributesBinding(AppScope::class)
@Inject
class DefaultFeaturesProvider : FeaturesProvider {
override fun provide(): List<Feature> = FeatureFlags.entries
}

View file

@ -9,26 +9,47 @@ 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 io.element.android.libraries.core.meta.BuildMeta
import io.element.android.libraries.featureflag.api.Feature
import io.element.android.libraries.featureflag.test.FakeFeature
import io.element.android.libraries.matrix.test.core.aBuildMeta
import kotlinx.coroutines.test.runTest
import org.junit.Test
class DefaultFeatureFlagServiceTest {
private val aFeature = FakeFeature(
key = "test_feature",
title = "Test Feature",
)
@Test
fun `given service without provider when feature is checked then it returns the default value`() = runTest {
val featureWithDefaultToFalse = FakeFeature(
key = "test_feature",
title = "Test Feature",
defaultValue = { false }
)
val featureWithDefaultToTrue = FakeFeature(
key = "test_feature_2",
title = "Test Feature 2",
defaultValue = { true }
)
val buildMeta = aBuildMeta()
val featureFlagService = DefaultFeatureFlagService(emptySet(), buildMeta)
featureFlagService.isFeatureEnabledFlow(FeatureFlags.Space).test {
assertThat(awaitItem()).isEqualTo(FeatureFlags.Space.defaultValue(buildMeta))
val featureFlagService = createDefaultFeatureFlagService(buildMeta = buildMeta)
featureFlagService.isFeatureEnabledFlow(featureWithDefaultToFalse).test {
assertThat(awaitItem()).isFalse()
cancelAndIgnoreRemainingEvents()
}
featureFlagService.isFeatureEnabledFlow(featureWithDefaultToTrue).test {
assertThat(awaitItem()).isTrue()
cancelAndIgnoreRemainingEvents()
}
}
@Test
fun `given service without provider when set enabled feature is called then it returns false`() = runTest {
val featureFlagService = DefaultFeatureFlagService(emptySet(), aBuildMeta())
val result = featureFlagService.setFeatureEnabled(FeatureFlags.Space, true)
val featureFlagService = createDefaultFeatureFlagService()
val result = featureFlagService.setFeatureEnabled(aFeature, true)
assertThat(result).isFalse()
}
@ -36,8 +57,11 @@ class DefaultFeatureFlagServiceTest {
fun `given service with a runtime provider when set enabled feature is called then it returns true`() = runTest {
val buildMeta = aBuildMeta()
val featureFlagProvider = FakeMutableFeatureFlagProvider(0, buildMeta)
val featureFlagService = DefaultFeatureFlagService(setOf(featureFlagProvider), buildMeta)
val result = featureFlagService.setFeatureEnabled(FeatureFlags.Space, true)
val featureFlagService = createDefaultFeatureFlagService(
providers = setOf(featureFlagProvider),
buildMeta = buildMeta,
)
val result = featureFlagService.setFeatureEnabled(aFeature, true)
assertThat(result).isTrue()
}
@ -45,11 +69,14 @@ class DefaultFeatureFlagServiceTest {
fun `given service with a runtime provider and feature enabled when feature is checked then it returns the correct value`() = runTest {
val buildMeta = aBuildMeta()
val featureFlagProvider = FakeMutableFeatureFlagProvider(0, buildMeta)
val featureFlagService = DefaultFeatureFlagService(setOf(featureFlagProvider), buildMeta)
featureFlagService.setFeatureEnabled(FeatureFlags.Space, true)
featureFlagService.isFeatureEnabledFlow(FeatureFlags.Space).test {
val featureFlagService = createDefaultFeatureFlagService(
providers = setOf(featureFlagProvider),
buildMeta = buildMeta
)
featureFlagService.setFeatureEnabled(aFeature, true)
featureFlagService.isFeatureEnabledFlow(aFeature).test {
assertThat(awaitItem()).isTrue()
featureFlagService.setFeatureEnabled(FeatureFlags.Space, false)
featureFlagService.setFeatureEnabled(aFeature, false)
assertThat(awaitItem()).isFalse()
}
}
@ -59,11 +86,84 @@ class DefaultFeatureFlagServiceTest {
val buildMeta = aBuildMeta()
val lowPriorityFeatureFlagProvider = FakeMutableFeatureFlagProvider(LOW_PRIORITY, buildMeta)
val highPriorityFeatureFlagProvider = FakeMutableFeatureFlagProvider(HIGH_PRIORITY, buildMeta)
val featureFlagService = DefaultFeatureFlagService(setOf(lowPriorityFeatureFlagProvider, highPriorityFeatureFlagProvider), buildMeta)
lowPriorityFeatureFlagProvider.setFeatureEnabled(FeatureFlags.Space, false)
highPriorityFeatureFlagProvider.setFeatureEnabled(FeatureFlags.Space, true)
featureFlagService.isFeatureEnabledFlow(FeatureFlags.Space).test {
val featureFlagService = createDefaultFeatureFlagService(
providers = setOf(lowPriorityFeatureFlagProvider, highPriorityFeatureFlagProvider),
buildMeta = buildMeta
)
lowPriorityFeatureFlagProvider.setFeatureEnabled(aFeature, false)
highPriorityFeatureFlagProvider.setFeatureEnabled(aFeature, true)
featureFlagService.isFeatureEnabledFlow(aFeature).test {
assertThat(awaitItem()).isTrue()
}
}
@Test
fun `getAvailableFeatures should return expected features`() {
val aFinishedLabFeature = FakeFeature(
key = "finished_lab_feature",
title = "Finished Lab Feature",
isFinished = true,
isInLabs = true,
)
val aFinishedDevFeature = FakeFeature(
key = "finished_dev_feature",
title = "Finished Dev Feature",
isFinished = true,
isInLabs = false,
)
val anUnfinishedLabFeature = FakeFeature(
key = "unfinished_lab_feature",
title = "Unfinished Lab Feature",
isFinished = false,
isInLabs = true,
)
val anUnfinishedDevFeature = FakeFeature(
key = "unfinished_dev_feature",
title = "Unfinished Dev Feature",
isFinished = false,
isInLabs = false,
)
val featureFlagService = createDefaultFeatureFlagService(
features = listOf(
aFinishedLabFeature,
aFinishedDevFeature,
anUnfinishedLabFeature,
anUnfinishedDevFeature,
),
)
assertThat(
featureFlagService.getAvailableFeatures(
includeFinishedFeatures = false,
isInLabs = true,
)
).containsExactly(anUnfinishedLabFeature)
assertThat(
featureFlagService.getAvailableFeatures(
includeFinishedFeatures = true,
isInLabs = true,
)
).containsExactly(aFinishedLabFeature, anUnfinishedLabFeature)
assertThat(
featureFlagService.getAvailableFeatures(
includeFinishedFeatures = false,
isInLabs = false,
)
).containsExactly(anUnfinishedDevFeature)
assertThat(
featureFlagService.getAvailableFeatures(
includeFinishedFeatures = true,
isInLabs = false,
)
).containsExactly(aFinishedDevFeature, anUnfinishedDevFeature)
}
}
private fun createDefaultFeatureFlagService(
providers: Set<FeatureFlagProvider> = emptySet(),
buildMeta: BuildMeta = aBuildMeta(),
features: List<Feature> = emptyList(),
) = DefaultFeatureFlagService(
providers = providers,
buildMeta = buildMeta,
featuresProvider = { features }
)

View file

@ -0,0 +1,24 @@
/*
* Copyright 2025 New Vector Ltd.
*
* SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial
* Please see LICENSE files in the repository root for full details.
*/
package io.element.android.libraries.featureflag.impl
import com.google.common.truth.Truth.assertThat
import io.element.android.libraries.featureflag.api.FeatureFlags
import org.junit.Test
class DefaultFeaturesProviderTest {
@Test
fun `provide should return all features`() {
val provider = DefaultFeaturesProvider()
val features = provider.provide()
assertThat(features.size).isEqualTo(FeatureFlags.entries.size)
FeatureFlags.entries.forEach {
assertThat(features.contains(it)).isTrue()
}
}
}

View file

@ -17,7 +17,7 @@ import kotlinx.coroutines.flow.MutableStateFlow
class FakeFeatureFlagService(
initialState: Map<String, Boolean> = emptyMap(),
private val buildMeta: BuildMeta = aBuildMeta(),
var providedAvailableFeatures: List<Feature> = emptyList(),
private val getAvailableFeaturesResult: (Boolean, Boolean) -> List<Feature> = { _, _ -> emptyList() },
) : FeatureFlagService {
private val enabledFeatures = initialState
.mapValues { MutableStateFlow(it.value) }
@ -33,7 +33,10 @@ class FakeFeatureFlagService(
return enabledFeatures.getOrPut(feature.key) { MutableStateFlow(feature.defaultValue(buildMeta)) }
}
override fun getAvailableFeatures(): List<Feature> {
return providedAvailableFeatures
override fun getAvailableFeatures(
includeFinishedFeatures: Boolean,
isInLabs: Boolean,
): List<Feature> {
return getAvailableFeaturesResult(includeFinishedFeatures, isInLabs)
}
}