FFs can now be toggled in release builds too (#3101)

- Removed `StaticFeatureFlagProvider`.
- Always provide `PreferencesFeatureFlagProvider`.
- For the default values of feature flags, use a lambda with a `BuildMeta` parameter so we can customize the return value based on its data.
This commit is contained in:
Jorge Martin Espinosa
2024-07-02 18:06:42 +02:00
committed by GitHub
parent d846113fdf
commit e9e7d4d0c4
19 changed files with 118 additions and 118 deletions

View File

@@ -18,7 +18,7 @@ package io.element.android.appconfig
object OnBoardingConfig {
/** Whether the user can use QR code login. */
const val CAN_LOGIN_WITH_QR_CODE = false
const val CAN_LOGIN_WITH_QR_CODE = true
/** Whether the user can create an account using the app. */
const val CAN_CREATE_ACCOUNT = false

1
changelog.d/3073.bugfix Normal file
View File

@@ -0,0 +1 @@
Fix feature flags not being able to be toggle in developer settings in release builds.

View File

@@ -745,7 +745,7 @@ class MessagesPresenterTest {
private suspend fun <T> ReceiveTurbine<T>.awaitFirstItem(): T {
// Skip 2 item if Mentions feature is enabled, else 1
skipItems(if (FeatureFlags.Mentions.defaultValue) 2 else 1)
skipItems(if (FeatureFlags.Mentions.defaultValue(aBuildMeta())) 2 else 1)
return awaitItem()
}

View File

@@ -63,6 +63,7 @@ import io.element.android.libraries.matrix.test.A_USER_ID
import io.element.android.libraries.matrix.test.A_USER_ID_2
import io.element.android.libraries.matrix.test.A_USER_ID_3
import io.element.android.libraries.matrix.test.A_USER_ID_4
import io.element.android.libraries.matrix.test.core.aBuildMeta
import io.element.android.libraries.matrix.test.permalink.FakePermalinkBuilder
import io.element.android.libraries.matrix.test.permalink.FakePermalinkParser
import io.element.android.libraries.matrix.test.room.FakeMatrixRoom
@@ -828,7 +829,6 @@ class MessageComposerPresenterTest {
// If room is a DM, `RoomMemberSuggestion.Room` is not returned
room.givenCanTriggerRoomNotification(Result.success(true))
room.isDirect
}
}
@@ -1300,7 +1300,7 @@ class MessageComposerPresenterTest {
private suspend fun <T> ReceiveTurbine<T>.awaitFirstItem(): T {
// Skip 2 item if Mentions feature is enabled, else 1
skipItems(if (FeatureFlags.Mentions.defaultValue) 2 else 1)
skipItems(if (FeatureFlags.Mentions.defaultValue(aBuildMeta())) 2 else 1)
return awaitItem()
}
}

View File

@@ -35,13 +35,18 @@ class OnBoardingPresenterTest {
@Test
fun `present - initial state`() = runTest {
val presenter = OnBoardingPresenter(
buildMeta = aBuildMeta(
val buildMeta = aBuildMeta(
applicationName = "A",
productionApplicationName = "B",
desktopApplicationName = "C",
),
featureFlagService = FakeFeatureFlagService(initialState = mapOf(FeatureFlags.QrCodeLogin.name to true)),
)
val featureFlagService = FakeFeatureFlagService(
initialState = mapOf(FeatureFlags.QrCodeLogin.key to true),
buildMeta = buildMeta,
)
val presenter = OnBoardingPresenter(
buildMeta = buildMeta,
featureFlagService = featureFlagService,
)
moleculeFlow(RecompositionMode.Immediate) {
presenter.present()

View File

@@ -23,5 +23,7 @@ android {
}
dependencies {
implementation(projects.appconfig)
implementation(projects.libraries.core)
implementation(libs.coroutines.core)
}

View File

@@ -16,6 +16,8 @@
package io.element.android.libraries.featureflag.api
import io.element.android.libraries.core.meta.BuildMeta
interface Feature {
/**
* Unique key to identify the feature.
@@ -33,9 +35,9 @@ interface Feature {
val description: String?
/**
* The default value of the feature (enabled or disabled).
* Calculate the default value of the feature (enabled or disabled) given a [BuildMeta].
*/
val defaultValue: Boolean
val defaultValue: (BuildMeta) -> Boolean
/**
* Whether the feature is finished or not.

View File

@@ -16,6 +16,10 @@
package io.element.android.libraries.featureflag.api
import io.element.android.appconfig.OnBoardingConfig
import io.element.android.libraries.core.meta.BuildMeta
import io.element.android.libraries.core.meta.BuildType
/**
* To enable or disable a FeatureFlags, change the `defaultValue` value.
* Warning: to enable a flag for the release app, you MUST update the file
@@ -25,82 +29,88 @@ enum class FeatureFlags(
override val key: String,
override val title: String,
override val description: String? = null,
override val defaultValue: Boolean,
override val defaultValue: (BuildMeta) -> Boolean,
override val isFinished: Boolean,
) : Feature {
LocationSharing(
key = "feature.locationsharing",
title = "Allow user to share location",
defaultValue = true,
defaultValue = { true },
isFinished = true,
),
Polls(
key = "feature.polls",
title = "Polls",
description = "Create poll and render poll events in the timeline",
defaultValue = true,
defaultValue = { true },
isFinished = true,
),
NotificationSettings(
key = "feature.notificationsettings",
title = "Show notification settings",
defaultValue = true,
defaultValue = { true },
isFinished = true,
),
VoiceMessages(
key = "feature.voicemessages",
title = "Voice messages",
description = "Send and receive voice messages",
defaultValue = true,
defaultValue = { true },
isFinished = true,
),
PinUnlock(
key = "feature.pinunlock",
title = "Pin unlock",
description = "Allow user to lock/unlock the app with a pin code or biometrics",
defaultValue = true,
defaultValue = { true },
isFinished = true,
),
Mentions(
key = "feature.mentions",
title = "Mentions",
description = "Type `@` to get mention suggestions and insert them",
defaultValue = true,
defaultValue = { true },
isFinished = false,
),
MarkAsUnread(
key = "feature.markAsUnread",
title = "Mark as unread",
description = "Allow user to mark a room as unread",
defaultValue = true,
defaultValue = { true },
isFinished = false,
),
RoomDirectorySearch(
key = "feature.roomdirectorysearch",
title = "Room directory search",
description = "Allow user to search for public rooms in their homeserver",
defaultValue = false,
defaultValue = { false },
isFinished = false,
),
ShowBlockedUsersDetails(
key = "feature.showBlockedUsersDetails",
title = "Show blocked users details",
description = "Show the name and avatar of blocked users in the blocked users list",
defaultValue = false,
defaultValue = { false },
isFinished = false,
),
QrCodeLogin(
key = "feature.qrCodeLogin",
title = "Enable login using QR code",
description = "Allow the user to login using the QR code flow",
defaultValue = true,
defaultValue = { buildMeta ->
when (buildMeta.buildType) {
// TODO remove once the feature is ready to publish
BuildType.RELEASE -> false
else -> OnBoardingConfig.CAN_LOGIN_WITH_QR_CODE
}
},
isFinished = false,
),
IncomingShare(
key = "feature.incomingShare",
title = "Incoming Share support",
description = "Allow the application to receive data from other applications",
defaultValue = true,
defaultValue = { true },
isFinished = false,
),
}

View File

@@ -41,4 +41,5 @@ dependencies {
testImplementation(libs.coroutines.test)
testImplementation(libs.test.truth)
testImplementation(libs.test.turbine)
testImplementation(projects.libraries.matrix.test)
}

View File

@@ -17,6 +17,7 @@
package io.element.android.libraries.featureflag.impl
import com.squareup.anvil.annotations.ContributesBinding
import io.element.android.libraries.core.meta.BuildMeta
import io.element.android.libraries.di.AppScope
import io.element.android.libraries.di.SingleIn
import io.element.android.libraries.featureflag.api.Feature
@@ -28,14 +29,15 @@ import javax.inject.Inject
@ContributesBinding(AppScope::class)
@SingleIn(AppScope::class)
class DefaultFeatureFlagService @Inject constructor(
private val providers: Set<@JvmSuppressWildcards FeatureFlagProvider>
private val providers: Set<@JvmSuppressWildcards FeatureFlagProvider>,
private val buildMeta: BuildMeta,
) : FeatureFlagService {
override fun isFeatureEnabledFlow(feature: Feature): Flow<Boolean> {
return providers.filter { it.hasFeature(feature) }
.sortedByDescending(FeatureFlagProvider::priority)
.firstOrNull()
?.isFeatureEnabledFlow(feature)
?: flowOf(feature.defaultValue)
?: flowOf(feature.defaultValue(buildMeta))
}
override suspend fun setFeatureEnabled(feature: Feature, enabled: Boolean): Boolean {

View File

@@ -22,6 +22,7 @@ import androidx.datastore.preferences.core.Preferences
import androidx.datastore.preferences.core.booleanPreferencesKey
import androidx.datastore.preferences.core.edit
import androidx.datastore.preferences.preferencesDataStore
import io.element.android.libraries.core.meta.BuildMeta
import io.element.android.libraries.di.ApplicationContext
import io.element.android.libraries.featureflag.api.Feature
import kotlinx.coroutines.flow.Flow
@@ -34,7 +35,10 @@ private val Context.dataStore: DataStore<Preferences> by preferencesDataStore(na
/**
* Note: this will be used only in the nightly and in the debug build.
*/
class PreferencesFeatureFlagProvider @Inject constructor(@ApplicationContext context: Context) : MutableFeatureFlagProvider {
class PreferencesFeatureFlagProvider @Inject constructor(
@ApplicationContext context: Context,
private val buildMeta: BuildMeta,
) : MutableFeatureFlagProvider {
private val store = context.dataStore
override val priority = MEDIUM_PRIORITY
@@ -47,7 +51,7 @@ class PreferencesFeatureFlagProvider @Inject constructor(@ApplicationContext con
override fun isFeatureEnabledFlow(feature: Feature): Flow<Boolean> {
return store.data.map { prefs ->
prefs[booleanPreferencesKey(feature.key)] ?: feature.defaultValue
prefs[booleanPreferencesKey(feature.key)] ?: feature.defaultValue(buildMeta)
}.distinctUntilChanged()
}

View File

@@ -1,56 +0,0 @@
/*
* Copyright (c) 2023 New Vector Ltd
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package io.element.android.libraries.featureflag.impl
import io.element.android.appconfig.OnBoardingConfig
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
/**
* This provider is used for release build.
* This is the place to enable or disable feature for the release build.
*/
class StaticFeatureFlagProvider @Inject constructor() :
FeatureFlagProvider {
override val priority = LOW_PRIORITY
override fun isFeatureEnabledFlow(feature: Feature): Flow<Boolean> {
val isFeatureEnabled = if (feature is FeatureFlags) {
when (feature) {
FeatureFlags.LocationSharing -> true
FeatureFlags.Polls -> true
FeatureFlags.NotificationSettings -> true
FeatureFlags.VoiceMessages -> true
FeatureFlags.PinUnlock -> true
FeatureFlags.Mentions -> true
FeatureFlags.MarkAsUnread -> true
FeatureFlags.RoomDirectorySearch -> false
FeatureFlags.ShowBlockedUsersDetails -> false
FeatureFlags.QrCodeLogin -> OnBoardingConfig.CAN_LOGIN_WITH_QR_CODE
FeatureFlags.IncomingShare -> true
}
} else {
false
}
return flowOf(isFeatureEnabled)
}
override fun hasFeature(feature: Feature) = true
}

View File

@@ -20,11 +20,9 @@ import com.squareup.anvil.annotations.ContributesTo
import dagger.Module
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.FeatureFlagProvider
import io.element.android.libraries.featureflag.impl.PreferencesFeatureFlagProvider
import io.element.android.libraries.featureflag.impl.StaticFeatureFlagProvider
@Module
@ContributesTo(AppScope::class)
@@ -33,20 +31,10 @@ object FeatureFlagModule {
@Provides
@ElementsIntoSet
fun providesFeatureFlagProvider(
buildType: BuildType,
mutableFeatureFlagProvider: PreferencesFeatureFlagProvider,
staticFeatureFlagProvider: StaticFeatureFlagProvider,
): Set<FeatureFlagProvider> {
val providers = HashSet<FeatureFlagProvider>()
when (buildType) {
BuildType.RELEASE -> {
providers.add(staticFeatureFlagProvider)
}
BuildType.NIGHTLY,
BuildType.DEBUG -> {
providers.add(mutableFeatureFlagProvider)
return buildSet {
add(mutableFeatureFlagProvider)
}
}
return providers
}
}

View File

@@ -19,38 +19,42 @@ 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.matrix.test.core.aBuildMeta
import kotlinx.coroutines.test.runTest
import org.junit.Test
class DefaultFeatureFlagServiceTest {
@Test
fun `given service without provider when feature is checked then it returns the default value`() = runTest {
val featureFlagService = DefaultFeatureFlagService(emptySet())
val buildMeta = aBuildMeta()
val featureFlagService = DefaultFeatureFlagService(emptySet(), buildMeta)
featureFlagService.isFeatureEnabledFlow(FeatureFlags.LocationSharing).test {
assertThat(awaitItem()).isEqualTo(FeatureFlags.LocationSharing.defaultValue)
assertThat(awaitItem()).isEqualTo(FeatureFlags.LocationSharing.defaultValue(buildMeta))
cancelAndIgnoreRemainingEvents()
}
}
@Test
fun `given service without provider when set enabled feature is called then it returns false`() = runTest {
val featureFlagService = DefaultFeatureFlagService(emptySet())
val featureFlagService = DefaultFeatureFlagService(emptySet(), aBuildMeta())
val result = featureFlagService.setFeatureEnabled(FeatureFlags.LocationSharing, true)
assertThat(result).isFalse()
}
@Test
fun `given service with a runtime provider when set enabled feature is called then it returns true`() = runTest {
val featureFlagProvider = FakeMutableFeatureFlagProvider(0)
val featureFlagService = DefaultFeatureFlagService(setOf(featureFlagProvider))
val buildMeta = aBuildMeta()
val featureFlagProvider = FakeMutableFeatureFlagProvider(0, buildMeta)
val featureFlagService = DefaultFeatureFlagService(setOf(featureFlagProvider), buildMeta)
val result = featureFlagService.setFeatureEnabled(FeatureFlags.LocationSharing, true)
assertThat(result).isTrue()
}
@Test
fun `given service with a runtime provider and feature enabled when feature is checked then it returns the correct value`() = runTest {
val featureFlagProvider = FakeMutableFeatureFlagProvider(0)
val featureFlagService = DefaultFeatureFlagService(setOf(featureFlagProvider))
val buildMeta = aBuildMeta()
val featureFlagProvider = FakeMutableFeatureFlagProvider(0, buildMeta)
val featureFlagService = DefaultFeatureFlagService(setOf(featureFlagProvider), buildMeta)
featureFlagService.setFeatureEnabled(FeatureFlags.LocationSharing, true)
featureFlagService.isFeatureEnabledFlow(FeatureFlags.LocationSharing).test {
assertThat(awaitItem()).isTrue()
@@ -61,9 +65,10 @@ class DefaultFeatureFlagServiceTest {
@Test
fun `given service with 2 runtime providers when feature is checked then it uses the priority correctly`() = runTest {
val lowPriorityFeatureFlagProvider = FakeMutableFeatureFlagProvider(LOW_PRIORITY)
val highPriorityFeatureFlagProvider = FakeMutableFeatureFlagProvider(HIGH_PRIORITY)
val featureFlagService = DefaultFeatureFlagService(setOf(lowPriorityFeatureFlagProvider, highPriorityFeatureFlagProvider))
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.LocationSharing, false)
highPriorityFeatureFlagProvider.setFeatureEnabled(FeatureFlags.LocationSharing, true)
featureFlagService.isFeatureEnabledFlow(FeatureFlags.LocationSharing).test {

View File

@@ -16,11 +16,15 @@
package io.element.android.libraries.featureflag.impl
import io.element.android.libraries.core.meta.BuildMeta
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 {
class FakeMutableFeatureFlagProvider(
override val priority: Int,
private val buildMeta: BuildMeta,
) : MutableFeatureFlagProvider {
private val enabledFeatures = mutableMapOf<String, MutableStateFlow<Boolean>>()
override suspend fun setFeatureEnabled(feature: Feature, enabled: Boolean) {
@@ -29,7 +33,7 @@ class FakeMutableFeatureFlagProvider(override val priority: Int) : MutableFeatur
}
override fun isFeatureEnabledFlow(feature: Feature): Flow<Boolean> {
return enabledFeatures.getOrPut(feature.key) { MutableStateFlow(feature.defaultValue) }
return enabledFeatures.getOrPut(feature.key) { MutableStateFlow(feature.defaultValue(buildMeta)) }
}
override fun hasFeature(feature: Feature): Boolean = true

View File

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

View File

@@ -16,19 +16,19 @@
package io.element.android.libraries.featureflag.test
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.matrix.test.core.aBuildMeta
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.MutableStateFlow
class FakeFeatureFlagService(
initialState: Map<String, Boolean> = emptyMap()
initialState: Map<String, Boolean> = emptyMap(),
private val buildMeta: BuildMeta = aBuildMeta(),
) : FeatureFlagService {
private val enabledFeatures = initialState
.map {
it.key to MutableStateFlow(it.value)
}
.toMap()
.mapValues { MutableStateFlow(it.value) }
.toMutableMap()
override suspend fun setFeatureEnabled(feature: Feature, enabled: Boolean): Boolean {
@@ -38,6 +38,6 @@ class FakeFeatureFlagService(
}
override fun isFeatureEnabledFlow(feature: Feature): Flow<Boolean> {
return enabledFeatures.getOrPut(feature.key) { MutableStateFlow(feature.defaultValue) }
return enabledFeatures.getOrPut(feature.key) { MutableStateFlow(feature.defaultValue(buildMeta)) }
}
}

View File

@@ -30,6 +30,10 @@ android {
testInstrumentationRunner = "androidx.test.runner.AndroidJUnitRunner"
}
buildFeatures {
buildConfig = true
}
buildTypes {
release {
isMinifyEnabled = false

View File

@@ -35,6 +35,8 @@ import io.element.android.features.roomlist.impl.migration.SharedPreferencesMigr
import io.element.android.features.roomlist.impl.search.RoomListSearchDataSource
import io.element.android.features.roomlist.impl.search.RoomListSearchPresenter
import io.element.android.libraries.core.coroutine.CoroutineDispatchers
import io.element.android.libraries.core.meta.BuildMeta
import io.element.android.libraries.core.meta.BuildType
import io.element.android.libraries.dateformatter.impl.DateFormatters
import io.element.android.libraries.dateformatter.impl.DefaultLastMessageTimestampFormatter
import io.element.android.libraries.dateformatter.impl.LocalDateTimeProvider
@@ -44,7 +46,7 @@ import io.element.android.libraries.eventformatter.impl.ProfileChangeContentForm
import io.element.android.libraries.eventformatter.impl.RoomMembershipContentFormatter
import io.element.android.libraries.eventformatter.impl.StateContentFormatter
import io.element.android.libraries.featureflag.impl.DefaultFeatureFlagService
import io.element.android.libraries.featureflag.impl.StaticFeatureFlagProvider
import io.element.android.libraries.featureflag.impl.PreferencesFeatureFlagProvider
import io.element.android.libraries.fullscreenintent.api.FullScreenIntentPermissionsPresenter
import io.element.android.libraries.fullscreenintent.api.FullScreenIntentPermissionsState
import io.element.android.libraries.indicator.impl.DefaultIndicatorService
@@ -78,8 +80,12 @@ class RoomListScreen(
private val sessionVerificationService = matrixClient.sessionVerificationService()
private val encryptionService = matrixClient.encryptionService()
private val stringProvider = AndroidStringProvider(context.resources)
private val buildMeta = getBuildMeta(context)
private val featureFlagService = DefaultFeatureFlagService(
providers = setOf(StaticFeatureFlagProvider())
providers = setOf(
PreferencesFeatureFlagProvider(context = context, buildMeta = buildMeta)
),
buildMeta = buildMeta,
)
private val roomListRoomSummaryFactory = RoomListRoomSummaryFactory(
lastMessageTimestampFormatter = DefaultLastMessageTimestampFormatter(
@@ -195,4 +201,24 @@ class RoomListScreen(
}
}
}
private fun getBuildMeta(context: Context): BuildMeta {
val buildType = BuildType.valueOf(BuildConfig.BUILD_TYPE.uppercase())
val name = context.getString(R.string.app_name)
return BuildMeta(
isDebuggable = BuildConfig.DEBUG,
buildType = buildType,
applicationName = name,
productionApplicationName = name,
desktopApplicationName = name,
applicationId = BuildConfig.APPLICATION_ID,
lowPrivacyLoggingEnabled = false,
versionName = BuildConfig.VERSION_NAME,
versionCode = BuildConfig.VERSION_CODE.toLong(),
gitRevision = "",
gitBranchName = "",
flavorDescription = "",
flavorShortDescription = "",
)
}
}