From 828ee38c09ed68337ea88a931a5de0265e5908a5 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Fri, 17 Oct 2025 10:44:03 +0200 Subject: [PATCH 01/16] Let the order of FeatureFlag follow the order they are declared in the code. Using map.keys does not guarantee that the order is kept, so using List instead. --- .../developer/DeveloperSettingsPresenter.kt | 48 +++++++++---------- 1 file changed, 22 insertions(+), 26 deletions(-) diff --git a/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/developer/DeveloperSettingsPresenter.kt b/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/developer/DeveloperSettingsPresenter.kt index 7106b53503..73df8cbbfb 100644 --- a/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/developer/DeveloperSettingsPresenter.kt +++ b/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/developer/DeveloperSettingsPresenter.kt @@ -13,13 +13,13 @@ import androidx.compose.runtime.MutableState import androidx.compose.runtime.collectAsState import androidx.compose.runtime.getValue import androidx.compose.runtime.key -import androidx.compose.runtime.mutableStateMapOf +import androidx.compose.runtime.mutableStateListOf import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.produceState import androidx.compose.runtime.remember import androidx.compose.runtime.rememberCoroutineScope import androidx.compose.runtime.setValue -import androidx.compose.runtime.snapshots.SnapshotStateMap +import androidx.compose.runtime.snapshots.SnapshotStateList import dev.zacsweers.metro.Inject import io.element.android.features.enterprise.api.EnterpriseService import io.element.android.features.preferences.impl.developer.tracing.toLogLevel @@ -31,7 +31,6 @@ import io.element.android.libraries.architecture.AsyncAction import io.element.android.libraries.architecture.AsyncData import io.element.android.libraries.architecture.Presenter import io.element.android.libraries.architecture.runCatchingUpdatingState -import io.element.android.libraries.core.bool.orFalse import io.element.android.libraries.core.extensions.runCatchingExceptions import io.element.android.libraries.core.meta.BuildMeta import io.element.android.libraries.core.meta.BuildType @@ -61,12 +60,8 @@ class DeveloperSettingsPresenter( @Composable override fun present(): DeveloperSettingsState { val rageshakeState = rageshakePresenter.present() - - val features = remember { - mutableStateMapOf() - } val enabledFeatures = remember { - mutableStateMapOf() + mutableStateListOf() } val cacheSize = remember { mutableStateOf>(AsyncData.Uninitialized) @@ -105,11 +100,10 @@ class DeveloperSettingsPresenter( } } .forEach { feature -> - features[feature.key] = feature - enabledFeatures[feature.key] = featureFlagService.isFeatureEnabled(feature) + enabledFeatures.add(EnabledFeature(feature, featureFlagService.isFeatureEnabled(feature))) } } - val featureUiModels = createUiModels(features, enabledFeatures) + val featureUiModels = createUiModels(enabledFeatures) val coroutineScope = rememberCoroutineScope() // Compute cache size each time the clear cache action value is changed LaunchedEffect(clearCacheAction.value.isSuccess()) { @@ -119,7 +113,6 @@ class DeveloperSettingsPresenter( fun handleEvents(event: DeveloperSettingsEvents) { when (event) { is DeveloperSettingsEvents.UpdateEnabledFeature -> coroutineScope.updateEnabledFeature( - features, enabledFeatures, event.feature, event.isEnabled, @@ -172,19 +165,17 @@ class DeveloperSettingsPresenter( @Composable private fun createUiModels( - features: SnapshotStateMap, - enabledFeatures: SnapshotStateMap + enabledFeatures: SnapshotStateList, ): List { - return features.values.map { feature -> - key(feature.key) { - val isEnabled = enabledFeatures[feature.key].orFalse() - remember(feature, isEnabled) { + return enabledFeatures.map { enabledFeature -> + key(enabledFeature.feature.key) { + remember(enabledFeature) { FeatureUiModel( - key = feature.key, - title = feature.title, - description = feature.description, + key = enabledFeature.feature.key, + title = enabledFeature.feature.title, + description = enabledFeature.feature.description, icon = null, - isEnabled = isEnabled + isEnabled = enabledFeature.isEnabled ) } } @@ -192,15 +183,15 @@ class DeveloperSettingsPresenter( } private fun CoroutineScope.updateEnabledFeature( - features: SnapshotStateMap, - enabledFeatures: SnapshotStateMap, + enabledFeatures: SnapshotStateList, featureUiModel: FeatureUiModel, enabled: Boolean, @Suppress("UNUSED_PARAMETER") triggerClearCache: () -> Unit, ) = launch { - val feature = features[featureUiModel.key] ?: return@launch + val featureIndex = enabledFeatures.indexOfFirst { it.feature.key == featureUiModel.key }.takeIf { it != -1 } ?: return@launch + val feature = enabledFeatures[featureIndex].feature if (featureFlagService.setFeatureEnabled(feature, enabled)) { - enabledFeatures[featureUiModel.key] = enabled + enabledFeatures[featureIndex] = enabledFeatures[featureIndex].copy(isEnabled = enabled) } } @@ -225,3 +216,8 @@ private fun customElementCallUrlValidator(url: String?): Boolean { if (parsedUrl.host.isNullOrBlank()) error("Missing host") }.isSuccess } + +private data class EnabledFeature( + val feature: Feature, + val isEnabled: Boolean, +) From d913c6db0c344e01463ab4eddea6f19b9653a475 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Fri, 17 Oct 2025 10:44:50 +0200 Subject: [PATCH 02/16] Improve fun api. --- .../impl/developer/DeveloperSettingsPresenter.kt | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/developer/DeveloperSettingsPresenter.kt b/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/developer/DeveloperSettingsPresenter.kt index 73df8cbbfb..3939e3a944 100644 --- a/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/developer/DeveloperSettingsPresenter.kt +++ b/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/developer/DeveloperSettingsPresenter.kt @@ -113,9 +113,9 @@ class DeveloperSettingsPresenter( fun handleEvents(event: DeveloperSettingsEvents) { when (event) { is DeveloperSettingsEvents.UpdateEnabledFeature -> coroutineScope.updateEnabledFeature( - enabledFeatures, - event.feature, - event.isEnabled, + enabledFeatures = enabledFeatures, + featureKey = event.feature.key, + enabled = event.isEnabled, triggerClearCache = { handleEvents(DeveloperSettingsEvents.ClearCache) } ) is DeveloperSettingsEvents.SetCustomElementCallBaseUrl -> coroutineScope.launch { @@ -184,11 +184,11 @@ class DeveloperSettingsPresenter( private fun CoroutineScope.updateEnabledFeature( enabledFeatures: SnapshotStateList, - featureUiModel: FeatureUiModel, + featureKey: String, enabled: Boolean, @Suppress("UNUSED_PARAMETER") triggerClearCache: () -> Unit, ) = launch { - val featureIndex = enabledFeatures.indexOfFirst { it.feature.key == featureUiModel.key }.takeIf { it != -1 } ?: return@launch + val featureIndex = enabledFeatures.indexOfFirst { it.feature.key == featureKey }.takeIf { it != -1 } ?: return@launch val feature = enabledFeatures[featureIndex].feature if (featureFlagService.setFeatureEnabled(feature, enabled)) { enabledFeatures[featureIndex] = enabledFeatures[featureIndex].copy(isEnabled = enabled) From 9094f2e82324a42a2a82f92ffaf3d5144f46052b Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Fri, 17 Oct 2025 10:46:47 +0200 Subject: [PATCH 03/16] Fix warning --- .../preferences/impl/developer/DeveloperSettingsPresenter.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/developer/DeveloperSettingsPresenter.kt b/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/developer/DeveloperSettingsPresenter.kt index 3939e3a944..22c8d9c735 100644 --- a/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/developer/DeveloperSettingsPresenter.kt +++ b/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/developer/DeveloperSettingsPresenter.kt @@ -84,7 +84,7 @@ class DeveloperSettingsPresenter( val tracingLogPacks by produceState(persistentListOf()) { appPreferencesStore.getTracingLogPacksFlow() // Sort the entries alphabetically by its title - .map { it.sortedBy { it.title } } + .map { it.sortedBy { pack -> pack.title } } .collectLatest { value = it.toImmutableList() } } From 6854f014b0445ee085f8405fec2a618262412be3 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Fri, 17 Oct 2025 10:52:41 +0200 Subject: [PATCH 04/16] Fix warning and also fix bug! --- .../libraries/featureflag/impl/DefaultFeatureFlagService.kt | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) 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 01c1e8a258..a7ff8c09dd 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 @@ -27,16 +27,14 @@ class DefaultFeatureFlagService( ) : FeatureFlagService { override fun isFeatureEnabledFlow(feature: Feature): Flow { 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() - .sortedBy(FeatureFlagProvider::priority) - .firstOrNull() + .maxByOrNull(FeatureFlagProvider::priority) ?.setFeatureEnabled(feature, enabled) ?.let { true } ?: false From b7ac322d18dc683565fb61b6bd29614eb64fc5cd Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Fri, 17 Oct 2025 10:59:17 +0200 Subject: [PATCH 05/16] Improve FeatureFlagService API. --- .../impl/developer/DeveloperSettingsPresenter.kt | 1 - .../features/preferences/impl/labs/LabsPresenter.kt | 3 +-- .../preferences/impl/root/PreferencesRootPresenter.kt | 2 +- .../libraries/featureflag/api/FeatureFlagService.kt | 9 +++++++-- .../featureflag/impl/DefaultFeatureFlagService.kt | 10 ++++++++-- .../featureflag/test/FakeFeatureFlagService.kt | 5 ++++- 6 files changed, 21 insertions(+), 9 deletions(-) diff --git a/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/developer/DeveloperSettingsPresenter.kt b/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/developer/DeveloperSettingsPresenter.kt index 22c8d9c735..d0cfd76fae 100644 --- a/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/developer/DeveloperSettingsPresenter.kt +++ b/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/developer/DeveloperSettingsPresenter.kt @@ -90,7 +90,6 @@ class DeveloperSettingsPresenter( LaunchedEffect(Unit) { featureFlagService.getAvailableFeatures() - .filter { it.isInLabs.not() && it.isFinished.not() } .run { // Never display room directory search in release builds for Play Store if (buildMeta.flavorDescription == "GooglePlay" && buildMeta.buildType == BuildType.RELEASE) { diff --git a/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/labs/LabsPresenter.kt b/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/labs/LabsPresenter.kt index 5e74454d75..b1e3fc7b77 100644 --- a/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/labs/LabsPresenter.kt +++ b/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/labs/LabsPresenter.kt @@ -43,8 +43,7 @@ class LabsPresenter( override fun present(): LabsState { val coroutineScope = rememberCoroutineScope() val features = remember { - val entries = featureFlagService.getAvailableFeatures() - .filter { it.isInLabs && !it.isFinished } + val entries = featureFlagService.getAvailableFeatures(isInLabs = true) .map { it.key to it } mutableStateMapOf(*entries.toTypedArray()) } diff --git a/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/root/PreferencesRootPresenter.kt b/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/root/PreferencesRootPresenter.kt index 3da72f982e..2a8eeaea1f 100644 --- a/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/root/PreferencesRootPresenter.kt +++ b/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/root/PreferencesRootPresenter.kt @@ -112,7 +112,7 @@ class PreferencesRootPresenter( .launchIn(this) } - val showLabsItem = remember { featureFlagService.getAvailableFeatures().any { it.isInLabs && !it.isFinished } } + val showLabsItem = remember { featureFlagService.getAvailableFeatures(isInLabs = true).isNotEmpty() } val directLogoutState = directLogoutPresenter.present() 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 1358909771..c6319cc03b 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 @@ -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 includeFinishFeatures 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 + fun getAvailableFeatures( + includeFinishFeatures: Boolean = false, + isInLabs: Boolean = false, + ): List } 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 a7ff8c09dd..dca114f5d8 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 @@ -40,7 +40,13 @@ class DefaultFeatureFlagService( ?: false } - override fun getAvailableFeatures(): List { - return FeatureFlags.entries.filter { !it.isFinished } + override fun getAvailableFeatures( + includeFinishFeatures: Boolean, + isInLabs: Boolean, + ): List { + return FeatureFlags.entries.filter { flag -> + (includeFinishFeatures || !flag.isFinished) && + flag.isInLabs == isInLabs + } } } 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 49fc095ed4..88a4941da1 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 @@ -33,7 +33,10 @@ class FakeFeatureFlagService( return enabledFeatures.getOrPut(feature.key) { MutableStateFlow(feature.defaultValue(buildMeta)) } } - override fun getAvailableFeatures(): List { + override fun getAvailableFeatures( + includeFinishFeatures: Boolean, + isInLabs: Boolean, + ): List { return providedAvailableFeatures } } From e747426bf96ee1404efbd3501f69fb70aa7a9ea8 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Fri, 17 Oct 2025 11:35:13 +0200 Subject: [PATCH 06/16] Show a ProgressDialog during Clear cache action. --- .../impl/developer/DeveloperSettingsState.kt | 4 +++- .../impl/developer/DeveloperSettingsView.kt | 15 ++++++++++++++- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/developer/DeveloperSettingsState.kt b/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/developer/DeveloperSettingsState.kt index 389297a46b..e69f315a36 100644 --- a/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/developer/DeveloperSettingsState.kt +++ b/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/developer/DeveloperSettingsState.kt @@ -26,7 +26,9 @@ data class DeveloperSettingsState( val isEnterpriseBuild: Boolean, val showColorPicker: Boolean, val eventSink: (DeveloperSettingsEvents) -> Unit -) +) { + val showLoader = clearCacheAction is AsyncAction.Loading +} data class CustomElementCallBaseUrlState( val baseUrl: String?, diff --git a/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/developer/DeveloperSettingsView.kt b/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/developer/DeveloperSettingsView.kt index e00716e2ce..d60b210381 100644 --- a/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/developer/DeveloperSettingsView.kt +++ b/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/developer/DeveloperSettingsView.kt @@ -7,6 +7,7 @@ package io.element.android.features.preferences.impl.developer +import androidx.activity.compose.BackHandler import androidx.compose.foundation.layout.padding import androidx.compose.foundation.layout.size import androidx.compose.foundation.progressSemantics @@ -22,6 +23,7 @@ import io.element.android.compound.theme.ElementTheme import io.element.android.features.preferences.impl.R import io.element.android.features.preferences.impl.developer.tracing.LogLevelItem import io.element.android.features.rageshake.api.preferences.RageshakePreferencesView +import io.element.android.libraries.designsystem.components.ProgressDialog import io.element.android.libraries.designsystem.components.list.ListItemContent import io.element.android.libraries.designsystem.components.preferences.PreferenceCategory import io.element.android.libraries.designsystem.components.preferences.PreferenceDropdown @@ -50,9 +52,20 @@ fun DeveloperSettingsView( onBackClick: () -> Unit, modifier: Modifier = Modifier, ) { + if (state.showLoader) { + ProgressDialog() + } + BackHandler( + enabled = !state.showLoader, + onBack = onBackClick, + ) PreferencePage( modifier = modifier, - onBackClick = onBackClick, + onBackClick = { + if (!state.showLoader) { + onBackClick() + } + }, title = stringResource(id = CommonStrings.common_developer_options) ) { // Note: this is OK to hardcode strings in this debug screen. From 098a74435d8549ea7be4df55129265b024d9984b Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Fri, 17 Oct 2025 11:37:11 +0200 Subject: [PATCH 07/16] Extract EnabledFeature. --- .../impl/developer/DeveloperSettingsPresenter.kt | 7 +------ .../preferences/impl/model/EnabledFeature.kt | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 6 deletions(-) create mode 100644 features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/model/EnabledFeature.kt diff --git a/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/developer/DeveloperSettingsPresenter.kt b/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/developer/DeveloperSettingsPresenter.kt index d0cfd76fae..56111fcdca 100644 --- a/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/developer/DeveloperSettingsPresenter.kt +++ b/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/developer/DeveloperSettingsPresenter.kt @@ -24,6 +24,7 @@ import dev.zacsweers.metro.Inject import io.element.android.features.enterprise.api.EnterpriseService import io.element.android.features.preferences.impl.developer.tracing.toLogLevel import io.element.android.features.preferences.impl.developer.tracing.toLogLevelItem +import io.element.android.features.preferences.impl.model.EnabledFeature import io.element.android.features.preferences.impl.tasks.ClearCacheUseCase import io.element.android.features.preferences.impl.tasks.ComputeCacheSizeUseCase import io.element.android.features.rageshake.api.preferences.RageshakePreferencesState @@ -34,7 +35,6 @@ import io.element.android.libraries.architecture.runCatchingUpdatingState import io.element.android.libraries.core.extensions.runCatchingExceptions import io.element.android.libraries.core.meta.BuildMeta import io.element.android.libraries.core.meta.BuildType -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 io.element.android.libraries.featureflag.ui.model.FeatureUiModel @@ -215,8 +215,3 @@ private fun customElementCallUrlValidator(url: String?): Boolean { if (parsedUrl.host.isNullOrBlank()) error("Missing host") }.isSuccess } - -private data class EnabledFeature( - val feature: Feature, - val isEnabled: Boolean, -) diff --git a/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/model/EnabledFeature.kt b/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/model/EnabledFeature.kt new file mode 100644 index 0000000000..715184c56a --- /dev/null +++ b/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/model/EnabledFeature.kt @@ -0,0 +1,15 @@ +/* + * 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.features.preferences.impl.model + +import io.element.android.libraries.featureflag.api.Feature + +data class EnabledFeature( + val feature: Feature, + val isEnabled: Boolean, +) From ff70722f8dc651e6d7a32e84d51b69f8f1b12b29 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Fri, 17 Oct 2025 11:42:21 +0200 Subject: [PATCH 08/16] Perform toImmutableList() less often. --- .../impl/developer/DeveloperSettingsPresenter.kt | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/developer/DeveloperSettingsPresenter.kt b/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/developer/DeveloperSettingsPresenter.kt index 56111fcdca..85aadbb06b 100644 --- a/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/developer/DeveloperSettingsPresenter.kt +++ b/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/developer/DeveloperSettingsPresenter.kt @@ -39,6 +39,7 @@ import io.element.android.libraries.featureflag.api.FeatureFlagService import io.element.android.libraries.featureflag.api.FeatureFlags import io.element.android.libraries.featureflag.ui.model.FeatureUiModel import io.element.android.libraries.preferences.api.store.AppPreferencesStore +import kotlinx.collections.immutable.ImmutableList import kotlinx.collections.immutable.persistentListOf import kotlinx.collections.immutable.toImmutableList import kotlinx.coroutines.CoroutineScope @@ -146,7 +147,7 @@ class DeveloperSettingsPresenter( } return DeveloperSettingsState( - features = featureUiModels.toImmutableList(), + features = featureUiModels, cacheSize = cacheSize.value, clearCacheAction = clearCacheAction.value, rageshakeState = rageshakeState, @@ -165,7 +166,7 @@ class DeveloperSettingsPresenter( @Composable private fun createUiModels( enabledFeatures: SnapshotStateList, - ): List { + ): ImmutableList { return enabledFeatures.map { enabledFeature -> key(enabledFeature.feature.key) { remember(enabledFeature) { @@ -178,7 +179,7 @@ class DeveloperSettingsPresenter( ) } } - } + }.toImmutableList() } private fun CoroutineScope.updateEnabledFeature( From b6ec06ebc6496a8f8e50562e3e8993eeae1f4c59 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Fri, 17 Oct 2025 11:52:38 +0200 Subject: [PATCH 09/16] Ensure labs feature are ordered as they are declared. --- .../preferences/impl/labs/LabsPresenter.kt | 74 ++++++++----------- .../impl/labs/LabsPresenterTest.kt | 28 ++----- 2 files changed, 39 insertions(+), 63 deletions(-) diff --git a/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/labs/LabsPresenter.kt b/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/labs/LabsPresenter.kt index b1e3fc7b77..9ebd9e56e3 100644 --- a/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/labs/LabsPresenter.kt +++ b/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/labs/LabsPresenter.kt @@ -11,20 +11,19 @@ import androidx.compose.runtime.Composable import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.getValue import androidx.compose.runtime.key -import androidx.compose.runtime.mutableStateMapOf +import androidx.compose.runtime.mutableStateListOf import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember import androidx.compose.runtime.rememberCoroutineScope import androidx.compose.runtime.setValue -import androidx.compose.runtime.snapshots.SnapshotStateMap +import androidx.compose.runtime.snapshots.SnapshotStateList import dev.zacsweers.metro.Inject import io.element.android.compound.tokens.generated.CompoundIcons import io.element.android.features.preferences.impl.R +import io.element.android.features.preferences.impl.model.EnabledFeature import io.element.android.features.preferences.impl.tasks.ClearCacheUseCase import io.element.android.libraries.architecture.Presenter -import io.element.android.libraries.core.bool.orFalse import io.element.android.libraries.designsystem.theme.components.IconSource -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 io.element.android.libraries.featureflag.ui.model.FeatureUiModel @@ -42,45 +41,38 @@ class LabsPresenter( @Composable override fun present(): LabsState { val coroutineScope = rememberCoroutineScope() - val features = remember { - val entries = featureFlagService.getAvailableFeatures(isInLabs = true) - .map { it.key to it } - mutableStateMapOf(*entries.toTypedArray()) - } val enabledFeatures = remember { - mutableStateMapOf() + mutableStateListOf() } - LaunchedEffect(Unit) { - for (feature in features.values) { - val isEnabled = featureFlagService.isFeatureEnabled(feature) - enabledFeatures[feature.key] = isEnabled - } + featureFlagService.getAvailableFeatures(isInLabs = true) + .forEach { feature -> + enabledFeatures.add(EnabledFeature(feature, featureFlagService.isFeatureEnabled(feature))) + } } - var isApplyingChanges by remember { mutableStateOf(false) } - - val featureUiModels = createUiModels(features, enabledFeatures) + val featureUiModels = createUiModels(enabledFeatures) fun handleEvent(event: LabsEvents) { when (event) { is LabsEvents.ToggleFeature -> coroutineScope.launch { - val feature = features[event.feature.key] ?: return@launch - val isEnabled = featureFlagService.isFeatureEnabled(feature) - featureFlagService.setFeatureEnabled(feature = feature, enabled = !isEnabled) - enabledFeatures[feature.key] = !isEnabled - - when (feature.key) { - FeatureFlags.Threads.key -> { - // Threads require a cache clear to recreate the event cache - clearCacheUseCase() - isApplyingChanges = true + val featureIndex = enabledFeatures.indexOfFirst { it.feature.key == event.feature.key }.takeIf { it != -1 } ?: return@launch + val enabledFeature = enabledFeatures[featureIndex] + val feature = enabledFeature.feature + val newValue = enabledFeature.isEnabled.not() + if (featureFlagService.setFeatureEnabled(feature, newValue)) { + enabledFeatures[featureIndex] = enabledFeatures[featureIndex].copy(isEnabled = newValue) + when (feature.key) { + FeatureFlags.Threads.key -> { + // Threads require a cache clear to recreate the event cache + clearCacheUseCase() + isApplyingChanges = true + } } } } } } - return LabsState( features = featureUiModels, isApplyingChanges = isApplyingChanges, @@ -90,31 +82,29 @@ class LabsPresenter( @Composable private fun createUiModels( - features: SnapshotStateMap, - enabledFeatures: SnapshotStateMap + enabledFeatures: SnapshotStateList, ): ImmutableList { - return features.values.map { feature -> - key(feature.key) { - val isEnabled = enabledFeatures[feature.key].orFalse() - val title = when (feature) { + return enabledFeatures.map { enabledFeature -> + key(enabledFeature.feature.key) { + val title = when (enabledFeature.feature) { FeatureFlags.Threads -> stringProvider.getString(R.string.screen_labs_enable_threads) - else -> feature.title + else -> enabledFeature.feature.title } - val description = when (feature) { + val description = when (enabledFeature.feature) { FeatureFlags.Threads -> stringProvider.getString(R.string.screen_labs_enable_threads_description) - else -> feature.description + else -> enabledFeature.feature.description } - val icon = when (feature) { + val icon = when (enabledFeature.feature) { FeatureFlags.Threads -> CompoundIcons.Threads() else -> null } - remember(feature, isEnabled) { + remember(enabledFeature) { FeatureUiModel( - key = feature.key, + key = enabledFeature.feature.key, title = title, description = description, icon = icon?.let(IconSource::Vector), - isEnabled = isEnabled + isEnabled = enabledFeature.isEnabled ) } } diff --git a/features/preferences/impl/src/test/kotlin/io/element/android/features/preferences/impl/labs/LabsPresenterTest.kt b/features/preferences/impl/src/test/kotlin/io/element/android/features/preferences/impl/labs/LabsPresenterTest.kt index 5117a9fe94..c9af521960 100644 --- a/features/preferences/impl/src/test/kotlin/io/element/android/features/preferences/impl/labs/LabsPresenterTest.kt +++ b/features/preferences/impl/src/test/kotlin/io/element/android/features/preferences/impl/labs/LabsPresenterTest.kt @@ -21,7 +21,7 @@ import org.junit.Test class LabsPresenterTest { @Test - fun `present - ensures only unfinished features in labs are displayed`() = runTest { + fun `present - ensures features are displayed in the correct order`() = runTest { val availableFeatures = listOf( FakeFeature( key = "feature_1", @@ -30,24 +30,18 @@ class LabsPresenterTest { ), FakeFeature( key = "feature_2", - title = "Feature 2", - isInLabs = false, - ), - FakeFeature( - key = "feature_3", title = "Feature 3", isInLabs = true, - isFinished = true, ) ) createLabsPresenter( availableFeatures = availableFeatures, ).test { + skipItems(1) val receivedFeatures = awaitItem().features - assertThat(receivedFeatures).hasSize(1) - assertThat(receivedFeatures.first().key).isEqualTo(availableFeatures.first().key) - - cancelAndIgnoreRemainingEvents() + assertThat(receivedFeatures).hasSize(2) + assertThat(receivedFeatures[0].key).isEqualTo(availableFeatures[0].key) + assertThat(receivedFeatures[1].key).isEqualTo(availableFeatures[1].key) } } @@ -63,17 +57,13 @@ class LabsPresenterTest { createLabsPresenter( availableFeatures = availableFeatures, ).test { + skipItems(1) val initialItem = awaitItem() val feature = initialItem.features.first() assertThat(feature.isEnabled).isFalse() - - // Wait until the data finished loading - skipItems(1) - // Toggle the feature, should be true now initialItem.eventSink(LabsEvents.ToggleFeature(feature)) assertThat(awaitItem().features.first().isEnabled).isTrue() - // Toggle the feature, should be false now initialItem.eventSink(LabsEvents.ToggleFeature(feature)) assertThat(awaitItem().features.first().isEnabled).isFalse() @@ -95,18 +85,14 @@ class LabsPresenterTest { availableFeatures = availableFeatures, clearCacheUseCase = clearCacheUseCase, ).test { + skipItems(1) val initialItem = awaitItem() val feature = initialItem.features.first() assertThat(feature.isEnabled).isFalse() assertThat(initialItem.isApplyingChanges).isFalse() - - // Wait until the data finished loading - skipItems(1) - // Toggle the feature initialItem.eventSink(LabsEvents.ToggleFeature(feature)) assertThat(awaitItem().features.first().isEnabled).isTrue() - // The clear cache use case should have been called assertThat(awaitItem().isApplyingChanges).isTrue() assertThat(clearCacheUseCase.executeHasBeenCalled).isTrue() From 09a6ffc081d39281179907a28ce7f66b662aa3ee Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Fri, 17 Oct 2025 12:04:18 +0200 Subject: [PATCH 10/16] Improve and fix tests. --- .../DeveloperSettingsPresenterTest.kt | 68 ++++++------------- .../impl/labs/LabsPresenterTest.kt | 17 +++-- .../impl/root/PreferencesRootPresenterTest.kt | 32 +++++---- .../test/FakeFeatureFlagService.kt | 4 +- 4 files changed, 52 insertions(+), 69 deletions(-) diff --git a/features/preferences/impl/src/test/kotlin/io/element/android/features/preferences/impl/developer/DeveloperSettingsPresenterTest.kt b/features/preferences/impl/src/test/kotlin/io/element/android/features/preferences/impl/developer/DeveloperSettingsPresenterTest.kt index 6761a37fc6..f13d5b3cd8 100644 --- a/features/preferences/impl/src/test/kotlin/io/element/android/features/preferences/impl/developer/DeveloperSettingsPresenterTest.kt +++ b/features/preferences/impl/src/test/kotlin/io/element/android/features/preferences/impl/developer/DeveloperSettingsPresenterTest.kt @@ -21,6 +21,7 @@ import io.element.android.libraries.architecture.AsyncAction import io.element.android.libraries.architecture.AsyncData import io.element.android.libraries.core.meta.BuildMeta import io.element.android.libraries.core.meta.BuildType +import io.element.android.libraries.featureflag.api.Feature import io.element.android.libraries.featureflag.api.FeatureFlags import io.element.android.libraries.featureflag.test.FakeFeature import io.element.android.libraries.featureflag.test.FakeFeatureFlagService @@ -41,15 +42,17 @@ class DeveloperSettingsPresenterTest { @Test fun `present - ensures initial states are correct`() = runTest { - val availableFeatures = listOf( - FakeFeature( - key = "feature_1", - title = "Feature 1", - isInLabs = false, + val getAvailableFeaturesResult = lambdaRecorder> { _, _ -> + listOf( + FakeFeature( + key = "feature_1", + title = "Feature 1", + isInLabs = false, + ) ) - ) + } val presenter = createDeveloperSettingsPresenter( - featureFlagService = FakeFeatureFlagService(providedAvailableFeatures = availableFeatures) + featureFlagService = FakeFeatureFlagService(getAvailableFeaturesResult = getAvailableFeaturesResult) ) presenter.test { awaitItem().also { state -> @@ -73,6 +76,8 @@ class DeveloperSettingsPresenterTest { awaitItem().also { state -> assertThat(state.cacheSize).isInstanceOf(AsyncData.Success::class.java) } + getAvailableFeaturesResult.assertions().isCalledOnce() + .with(value(false), value(false)) } } @@ -203,50 +208,17 @@ class DeveloperSettingsPresenterTest { } } - @Test - fun `present - won't display features in labs or finished`() = runTest { - val availableFeatures = listOf( - // Only this feature should be displayed - FakeFeature( - key = "feature_1", - title = "Feature 1", - isInLabs = false, - ), - FakeFeature( - key = "feature_2", - title = "Feature 2", - isInLabs = true, - ), - FakeFeature( - key = "feature_3", - title = "Feature 3", - isInLabs = false, - isFinished = true, - ) - ) - - val presenter = createDeveloperSettingsPresenter( - featureFlagService = FakeFeatureFlagService( - providedAvailableFeatures = availableFeatures, - ) - ) - presenter.test { - skipItems(2) - awaitItem().also { state -> - assertThat(state.features).hasSize(1) - } - } - } - private fun createDeveloperSettingsPresenter( featureFlagService: FakeFeatureFlagService = FakeFeatureFlagService( - providedAvailableFeatures = listOf( - FakeFeature( - key = "feature_1", - title = "Feature 1", - isInLabs = false, + getAvailableFeaturesResult = { _, _ -> + listOf( + FakeFeature( + key = "feature_1", + title = "Feature 1", + isInLabs = false, + ) ) - ) + } ), cacheSizeUseCase: FakeComputeCacheSizeUseCase = FakeComputeCacheSizeUseCase(), clearCacheUseCase: FakeClearCacheUseCase = FakeClearCacheUseCase(), diff --git a/features/preferences/impl/src/test/kotlin/io/element/android/features/preferences/impl/labs/LabsPresenterTest.kt b/features/preferences/impl/src/test/kotlin/io/element/android/features/preferences/impl/labs/LabsPresenterTest.kt index c9af521960..699e8102d5 100644 --- a/features/preferences/impl/src/test/kotlin/io/element/android/features/preferences/impl/labs/LabsPresenterTest.kt +++ b/features/preferences/impl/src/test/kotlin/io/element/android/features/preferences/impl/labs/LabsPresenterTest.kt @@ -15,6 +15,8 @@ import io.element.android.libraries.featureflag.api.FeatureFlags import io.element.android.libraries.featureflag.test.FakeFeature import io.element.android.libraries.featureflag.test.FakeFeatureFlagService import io.element.android.services.toolbox.test.strings.FakeStringProvider +import io.element.android.tests.testutils.lambda.lambdaRecorder +import io.element.android.tests.testutils.lambda.value import io.element.android.tests.testutils.test import kotlinx.coroutines.test.runTest import org.junit.Test @@ -34,14 +36,19 @@ class LabsPresenterTest { isInLabs = true, ) ) + val getAvailableFeaturesResult = lambdaRecorder> { _, _ -> + availableFeatures + } createLabsPresenter( - availableFeatures = availableFeatures, + getAvailableFeaturesResult = getAvailableFeaturesResult, ).test { skipItems(1) val receivedFeatures = awaitItem().features assertThat(receivedFeatures).hasSize(2) assertThat(receivedFeatures[0].key).isEqualTo(availableFeatures[0].key) assertThat(receivedFeatures[1].key).isEqualTo(availableFeatures[1].key) + getAvailableFeaturesResult.assertions().isCalledOnce() + .with(value(false), value(true)) } } @@ -55,7 +62,7 @@ class LabsPresenterTest { ), ) createLabsPresenter( - availableFeatures = availableFeatures, + getAvailableFeaturesResult = { _, _ -> availableFeatures }, ).test { skipItems(1) val initialItem = awaitItem() @@ -82,7 +89,7 @@ class LabsPresenterTest { val clearCacheUseCase = FakeClearCacheUseCase() createLabsPresenter( - availableFeatures = availableFeatures, + getAvailableFeaturesResult = { _, _ -> availableFeatures }, clearCacheUseCase = clearCacheUseCase, ).test { skipItems(1) @@ -100,12 +107,12 @@ class LabsPresenterTest { } private fun createLabsPresenter( - availableFeatures: List = emptyList(), + getAvailableFeaturesResult: (Boolean, Boolean) -> List = { _, _ -> emptyList() }, clearCacheUseCase: ClearCacheUseCase = FakeClearCacheUseCase(), ): LabsPresenter { return LabsPresenter( stringProvider = FakeStringProvider(), - featureFlagService = FakeFeatureFlagService(providedAvailableFeatures = availableFeatures), + featureFlagService = FakeFeatureFlagService(getAvailableFeaturesResult = getAvailableFeaturesResult), clearCacheUseCase = clearCacheUseCase, ) } diff --git a/features/preferences/impl/src/test/kotlin/io/element/android/features/preferences/impl/root/PreferencesRootPresenterTest.kt b/features/preferences/impl/src/test/kotlin/io/element/android/features/preferences/impl/root/PreferencesRootPresenterTest.kt index f65589beb6..ad561127da 100644 --- a/features/preferences/impl/src/test/kotlin/io/element/android/features/preferences/impl/root/PreferencesRootPresenterTest.kt +++ b/features/preferences/impl/src/test/kotlin/io/element/android/features/preferences/impl/root/PreferencesRootPresenterTest.kt @@ -189,14 +189,16 @@ class PreferencesRootPresenterTest { fun `present - labs can be shown if any feature flag is in labs and not finished`() = runTest { createPresenter( featureFlagService = FakeFeatureFlagService( - providedAvailableFeatures = listOf( - FakeFeature( - key = "feature_1", - title = "Feature 1", - isInLabs = true, - isFinished = false, + getAvailableFeaturesResult = { _, _ -> + listOf( + FakeFeature( + key = "feature_1", + title = "Feature 1", + isInLabs = true, + isFinished = false, + ) ) - ) + } ), matrixClient = FakeMatrixClient( canDeactivateAccountResult = { true }, @@ -212,14 +214,16 @@ class PreferencesRootPresenterTest { fun `present - labs can't be shown if all feature flags in labs are finished`() = runTest { createPresenter( featureFlagService = FakeFeatureFlagService( - providedAvailableFeatures = listOf( - FakeFeature( - key = "feature_1", - title = "Feature 1", - isInLabs = true, - isFinished = true, + getAvailableFeaturesResult = { _, _ -> + listOf( + FakeFeature( + key = "feature_1", + title = "Feature 1", + isInLabs = true, + isFinished = true, + ) ) - ) + } ), matrixClient = FakeMatrixClient( canDeactivateAccountResult = { true }, 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 88a4941da1..60bd401cef 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 @@ -17,7 +17,7 @@ import kotlinx.coroutines.flow.MutableStateFlow class FakeFeatureFlagService( initialState: Map = emptyMap(), private val buildMeta: BuildMeta = aBuildMeta(), - var providedAvailableFeatures: List = emptyList(), + private val getAvailableFeaturesResult: (Boolean, Boolean) -> List = { _, _ -> emptyList() }, ) : FeatureFlagService { private val enabledFeatures = initialState .mapValues { MutableStateFlow(it.value) } @@ -37,6 +37,6 @@ class FakeFeatureFlagService( includeFinishFeatures: Boolean, isInLabs: Boolean, ): List { - return providedAvailableFeatures + return getAvailableFeaturesResult(includeFinishFeatures, isInLabs) } } From a76ab22ed559a630c61664062070c4b849cd691a Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Fri, 17 Oct 2025 12:27:23 +0200 Subject: [PATCH 11/16] Add test on api getAvailableFeatures. --- libraries/featureflag/impl/build.gradle.kts | 1 + .../impl/DefaultFeatureFlagService.kt | 4 +- .../featureflag/impl/FeaturesProvider.kt | 24 +++++ .../impl/DefaultFeatureFlagServiceTest.kt | 92 ++++++++++++++++++- 4 files changed, 114 insertions(+), 7 deletions(-) create mode 100644 libraries/featureflag/impl/src/main/kotlin/io/element/android/libraries/featureflag/impl/FeaturesProvider.kt diff --git a/libraries/featureflag/impl/build.gradle.kts b/libraries/featureflag/impl/build.gradle.kts index c54f95a293..f44032661c 100644 --- a/libraries/featureflag/impl/build.gradle.kts +++ b/libraries/featureflag/impl/build.gradle.kts @@ -31,4 +31,5 @@ dependencies { testCommonDependencies(libs) testImplementation(projects.libraries.matrix.test) + testImplementation(projects.libraries.featureflag.test) } 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 dca114f5d8..0e7a389250 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 @@ -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,6 +23,7 @@ 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 { return providers.filter { it.hasFeature(feature) } @@ -44,7 +44,7 @@ class DefaultFeatureFlagService( includeFinishFeatures: Boolean, isInLabs: Boolean, ): List { - return FeatureFlags.entries.filter { flag -> + return featuresProvider.provide().filter { flag -> (includeFinishFeatures || !flag.isFinished) && flag.isInLabs == isInLabs } diff --git a/libraries/featureflag/impl/src/main/kotlin/io/element/android/libraries/featureflag/impl/FeaturesProvider.kt b/libraries/featureflag/impl/src/main/kotlin/io/element/android/libraries/featureflag/impl/FeaturesProvider.kt new file mode 100644 index 0000000000..8d580006e7 --- /dev/null +++ b/libraries/featureflag/impl/src/main/kotlin/io/element/android/libraries/featureflag/impl/FeaturesProvider.kt @@ -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 +} + +@ContributesBinding(AppScope::class) +@Inject +class DefaultFeaturesProvider : FeaturesProvider { + override fun provide(): List = FeatureFlags.entries +} 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 097e318609..becb82da49 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 @@ -9,7 +9,10 @@ package io.element.android.libraries.featureflag.impl import app.cash.turbine.test import com.google.common.truth.Truth.assertThat +import io.element.android.libraries.core.meta.BuildMeta +import io.element.android.libraries.featureflag.api.Feature import io.element.android.libraries.featureflag.api.FeatureFlags +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 @@ -18,7 +21,7 @@ class DefaultFeatureFlagServiceTest { @Test fun `given service without provider when feature is checked then it returns the default value`() = runTest { val buildMeta = aBuildMeta() - val featureFlagService = DefaultFeatureFlagService(emptySet(), buildMeta) + val featureFlagService = createDefaultFeatureFlagService(buildMeta = buildMeta) featureFlagService.isFeatureEnabledFlow(FeatureFlags.Space).test { assertThat(awaitItem()).isEqualTo(FeatureFlags.Space.defaultValue(buildMeta)) cancelAndIgnoreRemainingEvents() @@ -27,7 +30,7 @@ class DefaultFeatureFlagServiceTest { @Test fun `given service without provider when set enabled feature is called then it returns false`() = runTest { - val featureFlagService = DefaultFeatureFlagService(emptySet(), aBuildMeta()) + val featureFlagService = createDefaultFeatureFlagService() val result = featureFlagService.setFeatureEnabled(FeatureFlags.Space, true) assertThat(result).isFalse() } @@ -36,7 +39,10 @@ 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 featureFlagService = createDefaultFeatureFlagService( + providers = setOf(featureFlagProvider), + buildMeta = buildMeta, + ) val result = featureFlagService.setFeatureEnabled(FeatureFlags.Space, true) assertThat(result).isTrue() } @@ -45,7 +51,10 @@ 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) + val featureFlagService = createDefaultFeatureFlagService( + providers = setOf(featureFlagProvider), + buildMeta = buildMeta + ) featureFlagService.setFeatureEnabled(FeatureFlags.Space, true) featureFlagService.isFeatureEnabledFlow(FeatureFlags.Space).test { assertThat(awaitItem()).isTrue() @@ -59,11 +68,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) + val featureFlagService = createDefaultFeatureFlagService( + providers = setOf(lowPriorityFeatureFlagProvider, highPriorityFeatureFlagProvider), + buildMeta = buildMeta + ) lowPriorityFeatureFlagProvider.setFeatureEnabled(FeatureFlags.Space, false) highPriorityFeatureFlagProvider.setFeatureEnabled(FeatureFlags.Space, true) featureFlagService.isFeatureEnabledFlow(FeatureFlags.Space).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( + includeFinishFeatures = false, + isInLabs = true, + ) + ).containsExactly(anUnfinishedLabFeature) + assertThat( + featureFlagService.getAvailableFeatures( + includeFinishFeatures = true, + isInLabs = true, + ) + ).containsExactly(aFinishedLabFeature, anUnfinishedLabFeature) + assertThat( + featureFlagService.getAvailableFeatures( + includeFinishFeatures = false, + isInLabs = false, + ) + ).containsExactly(anUnfinishedDevFeature) + assertThat( + featureFlagService.getAvailableFeatures( + includeFinishFeatures = true, + isInLabs = false, + ) + ).containsExactly(aFinishedDevFeature, anUnfinishedDevFeature) + } } + +private fun createDefaultFeatureFlagService( + providers: Set = emptySet(), + buildMeta: BuildMeta = aBuildMeta(), + features: List = FeatureFlags.entries, +) = DefaultFeatureFlagService( + providers = providers, + buildMeta = buildMeta, + featuresProvider = { features } +) From ba49cfb07c8ed3a5d45b731cc04c5d5ab1809a31 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Fri, 17 Oct 2025 12:33:07 +0200 Subject: [PATCH 12/16] Avoid using volatile FeatureFlags in DefaultFeatureFlagServiceTest --- .../impl/DefaultFeatureFlagServiceTest.kt | 42 +++++++++++++------ 1 file changed, 30 insertions(+), 12 deletions(-) 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 becb82da49..c1f33333c4 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 @@ -11,19 +11,37 @@ import app.cash.turbine.test import com.google.common.truth.Truth.assertThat import io.element.android.libraries.core.meta.BuildMeta import io.element.android.libraries.featureflag.api.Feature -import io.element.android.libraries.featureflag.api.FeatureFlags 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 = createDefaultFeatureFlagService(buildMeta = buildMeta) - featureFlagService.isFeatureEnabledFlow(FeatureFlags.Space).test { - assertThat(awaitItem()).isEqualTo(FeatureFlags.Space.defaultValue(buildMeta)) + featureFlagService.isFeatureEnabledFlow(featureWithDefaultToFalse).test { + assertThat(awaitItem()).isFalse() + cancelAndIgnoreRemainingEvents() + } + featureFlagService.isFeatureEnabledFlow(featureWithDefaultToTrue).test { + assertThat(awaitItem()).isTrue() cancelAndIgnoreRemainingEvents() } } @@ -31,7 +49,7 @@ class DefaultFeatureFlagServiceTest { @Test fun `given service without provider when set enabled feature is called then it returns false`() = runTest { val featureFlagService = createDefaultFeatureFlagService() - val result = featureFlagService.setFeatureEnabled(FeatureFlags.Space, true) + val result = featureFlagService.setFeatureEnabled(aFeature, true) assertThat(result).isFalse() } @@ -43,7 +61,7 @@ class DefaultFeatureFlagServiceTest { providers = setOf(featureFlagProvider), buildMeta = buildMeta, ) - val result = featureFlagService.setFeatureEnabled(FeatureFlags.Space, true) + val result = featureFlagService.setFeatureEnabled(aFeature, true) assertThat(result).isTrue() } @@ -55,10 +73,10 @@ class DefaultFeatureFlagServiceTest { providers = setOf(featureFlagProvider), buildMeta = buildMeta ) - featureFlagService.setFeatureEnabled(FeatureFlags.Space, true) - featureFlagService.isFeatureEnabledFlow(FeatureFlags.Space).test { + featureFlagService.setFeatureEnabled(aFeature, true) + featureFlagService.isFeatureEnabledFlow(aFeature).test { assertThat(awaitItem()).isTrue() - featureFlagService.setFeatureEnabled(FeatureFlags.Space, false) + featureFlagService.setFeatureEnabled(aFeature, false) assertThat(awaitItem()).isFalse() } } @@ -72,9 +90,9 @@ class DefaultFeatureFlagServiceTest { providers = setOf(lowPriorityFeatureFlagProvider, highPriorityFeatureFlagProvider), buildMeta = buildMeta ) - lowPriorityFeatureFlagProvider.setFeatureEnabled(FeatureFlags.Space, false) - highPriorityFeatureFlagProvider.setFeatureEnabled(FeatureFlags.Space, true) - featureFlagService.isFeatureEnabledFlow(FeatureFlags.Space).test { + lowPriorityFeatureFlagProvider.setFeatureEnabled(aFeature, false) + highPriorityFeatureFlagProvider.setFeatureEnabled(aFeature, true) + featureFlagService.isFeatureEnabledFlow(aFeature).test { assertThat(awaitItem()).isTrue() } } @@ -143,7 +161,7 @@ class DefaultFeatureFlagServiceTest { private fun createDefaultFeatureFlagService( providers: Set = emptySet(), buildMeta: BuildMeta = aBuildMeta(), - features: List = FeatureFlags.entries, + features: List = emptyList(), ) = DefaultFeatureFlagService( providers = providers, buildMeta = buildMeta, From 52789cec8c5069ecdd1eb80a075e2cdc0f220b45 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Fri, 17 Oct 2025 12:35:19 +0200 Subject: [PATCH 13/16] Fix test. --- .../impl/root/PreferencesRootPresenterTest.kt | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/features/preferences/impl/src/test/kotlin/io/element/android/features/preferences/impl/root/PreferencesRootPresenterTest.kt b/features/preferences/impl/src/test/kotlin/io/element/android/features/preferences/impl/root/PreferencesRootPresenterTest.kt index ad561127da..80d4706c33 100644 --- a/features/preferences/impl/src/test/kotlin/io/element/android/features/preferences/impl/root/PreferencesRootPresenterTest.kt +++ b/features/preferences/impl/src/test/kotlin/io/element/android/features/preferences/impl/root/PreferencesRootPresenterTest.kt @@ -215,14 +215,7 @@ class PreferencesRootPresenterTest { createPresenter( featureFlagService = FakeFeatureFlagService( getAvailableFeaturesResult = { _, _ -> - listOf( - FakeFeature( - key = "feature_1", - title = "Feature 1", - isInLabs = true, - isFinished = true, - ) - ) + emptyList() } ), matrixClient = FakeMatrixClient( @@ -230,6 +223,7 @@ class PreferencesRootPresenterTest { accountManagementUrlResult = { Result.success(null) }, ), ).test { + skipItems(1) assertThat(awaitItem().showLabsItem).isFalse() cancelAndIgnoreRemainingEvents() } From e5a9e2dc74bd0462e5bd8b7fdd66e106be804e40 Mon Sep 17 00:00:00 2001 From: ElementBot Date: Fri, 17 Oct 2025 11:00:29 +0000 Subject: [PATCH 14/16] Update screenshots --- ...ferences.impl.developer_DeveloperSettingsView_Day_1_en.png | 4 ++-- ...rences.impl.developer_DeveloperSettingsView_Night_1_en.png | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/uitests/src/test/snapshots/images/features.preferences.impl.developer_DeveloperSettingsView_Day_1_en.png b/tests/uitests/src/test/snapshots/images/features.preferences.impl.developer_DeveloperSettingsView_Day_1_en.png index 176ba2c9ea..6678623bc5 100644 --- a/tests/uitests/src/test/snapshots/images/features.preferences.impl.developer_DeveloperSettingsView_Day_1_en.png +++ b/tests/uitests/src/test/snapshots/images/features.preferences.impl.developer_DeveloperSettingsView_Day_1_en.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:d4d03e543f38658338d7fedcc7f290ad973a68e1ec4a828d4651c188769e6769 -size 46130 +oid sha256:84e4c7fd597e4cdcb043514b370b0fd1e3b65f42b8c7811a206433ad422841fe +size 42445 diff --git a/tests/uitests/src/test/snapshots/images/features.preferences.impl.developer_DeveloperSettingsView_Night_1_en.png b/tests/uitests/src/test/snapshots/images/features.preferences.impl.developer_DeveloperSettingsView_Night_1_en.png index 2f74fe63fc..536d5a22c8 100644 --- a/tests/uitests/src/test/snapshots/images/features.preferences.impl.developer_DeveloperSettingsView_Night_1_en.png +++ b/tests/uitests/src/test/snapshots/images/features.preferences.impl.developer_DeveloperSettingsView_Night_1_en.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:c8b5df08672db5d6c6df1daba79166bb3251edc6e612b29eaad881a2b2dc44d3 -size 44788 +oid sha256:8880ddfe80d8062bcaceac1e9d1407e24d365e9c6b69fc8ce65e3ade973e8acf +size 41037 From 6e722a299820ed12e8e939a5dc70e218f765d3d3 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Fri, 17 Oct 2025 14:52:06 +0200 Subject: [PATCH 15/16] Fix typo. --- .../libraries/featureflag/api/FeatureFlagService.kt | 4 ++-- .../featureflag/impl/DefaultFeatureFlagService.kt | 4 ++-- .../featureflag/impl/DefaultFeatureFlagServiceTest.kt | 8 ++++---- .../libraries/featureflag/test/FakeFeatureFlagService.kt | 4 ++-- 4 files changed, 10 insertions(+), 10 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 c6319cc03b..2623faa3ab 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 @@ -36,11 +36,11 @@ interface FeatureFlagService { /** * @return the list of available features that can be toggled. - * @param includeFinishFeatures whether to include finished features, default is false + * @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( - includeFinishFeatures: Boolean = false, + includeFinishedFeatures: Boolean = false, isInLabs: Boolean = false, ): List } 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 0e7a389250..cecd0ca0a8 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 @@ -41,11 +41,11 @@ class DefaultFeatureFlagService( } override fun getAvailableFeatures( - includeFinishFeatures: Boolean, + includeFinishedFeatures: Boolean, isInLabs: Boolean, ): List { return featuresProvider.provide().filter { flag -> - (includeFinishFeatures || !flag.isFinished) && + (includeFinishedFeatures || !flag.isFinished) && flag.isInLabs == isInLabs } } 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 c1f33333c4..438dbe1ecc 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 @@ -133,25 +133,25 @@ class DefaultFeatureFlagServiceTest { ) assertThat( featureFlagService.getAvailableFeatures( - includeFinishFeatures = false, + includeFinishedFeatures = false, isInLabs = true, ) ).containsExactly(anUnfinishedLabFeature) assertThat( featureFlagService.getAvailableFeatures( - includeFinishFeatures = true, + includeFinishedFeatures = true, isInLabs = true, ) ).containsExactly(aFinishedLabFeature, anUnfinishedLabFeature) assertThat( featureFlagService.getAvailableFeatures( - includeFinishFeatures = false, + includeFinishedFeatures = false, isInLabs = false, ) ).containsExactly(anUnfinishedDevFeature) assertThat( featureFlagService.getAvailableFeatures( - includeFinishFeatures = true, + includeFinishedFeatures = true, isInLabs = false, ) ).containsExactly(aFinishedDevFeature, anUnfinishedDevFeature) 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 60bd401cef..456936e501 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 @@ -34,9 +34,9 @@ class FakeFeatureFlagService( } override fun getAvailableFeatures( - includeFinishFeatures: Boolean, + includeFinishedFeatures: Boolean, isInLabs: Boolean, ): List { - return getAvailableFeaturesResult(includeFinishFeatures, isInLabs) + return getAvailableFeaturesResult(includeFinishedFeatures, isInLabs) } } From 69aafe98410e7ddaeead59ec69415bc752071a3b Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Fri, 17 Oct 2025 14:55:41 +0200 Subject: [PATCH 16/16] Add unit test on DefaultFeaturesProvider. --- .../impl/DefaultFeaturesProviderTest.kt | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) create mode 100644 libraries/featureflag/impl/src/test/kotlin/io/element/android/libraries/featureflag/impl/DefaultFeaturesProviderTest.kt diff --git a/libraries/featureflag/impl/src/test/kotlin/io/element/android/libraries/featureflag/impl/DefaultFeaturesProviderTest.kt b/libraries/featureflag/impl/src/test/kotlin/io/element/android/libraries/featureflag/impl/DefaultFeaturesProviderTest.kt new file mode 100644 index 0000000000..8b746c2e89 --- /dev/null +++ b/libraries/featureflag/impl/src/test/kotlin/io/element/android/libraries/featureflag/impl/DefaultFeaturesProviderTest.kt @@ -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() + } + } +}