diff --git a/packages/SystemUI/shared/src/com/android/systemui/flags/Flag.kt b/packages/SystemUI/shared/src/com/android/systemui/flags/Flag.kt index a8a526a332291..f7049cf8f4f2e 100644 --- a/packages/SystemUI/shared/src/com/android/systemui/flags/Flag.kt +++ b/packages/SystemUI/shared/src/com/android/systemui/flags/Flag.kt @@ -48,9 +48,13 @@ interface SysPropFlag : Flag { val default: T } +/** + * Base class for most common boolean flags. + * + * See [UnreleasedFlag] and [ReleasedFlag] for useful implementations. + */ // Consider using the "parcelize" kotlin library. - -data class BooleanFlag @JvmOverloads constructor( +abstract class BooleanFlag constructor( override val id: Int, override val default: Boolean = false, override val teamfood: Boolean = false, @@ -60,7 +64,7 @@ data class BooleanFlag @JvmOverloads constructor( companion object { @JvmField val CREATOR = object : Parcelable.Creator { - override fun createFromParcel(parcel: Parcel) = BooleanFlag(parcel) + override fun createFromParcel(parcel: Parcel) = object : BooleanFlag(parcel) {} override fun newArray(size: Int) = arrayOfNulls(size) } } @@ -80,12 +84,46 @@ data class BooleanFlag @JvmOverloads constructor( } } +/** + * A Flag that is is false by default. + * + * It can be changed or overridden in debug builds but not in release builds. + */ +data class UnreleasedFlag @JvmOverloads constructor( + override val id: Int, + override val teamfood: Boolean = false, + override val overridden: Boolean = false +) : BooleanFlag(id, false, teamfood, overridden) + +/** + * A Flag that is is true by default. + * + * It can be changed or overridden in any build, meaning it can be turned off if needed. + */ +data class ReleasedFlag @JvmOverloads constructor( + override val id: Int, + override val teamfood: Boolean = false, + override val overridden: Boolean = false +) : BooleanFlag(id, true, teamfood, overridden) + +/** + * A Flag that reads its default values from a resource overlay instead of code. + * + * Prefer [UnreleasedFlag] and [ReleasedFlag]. + */ data class ResourceBooleanFlag @JvmOverloads constructor( override val id: Int, @BoolRes override val resourceId: Int, override val teamfood: Boolean = false ) : ResourceFlag +/** + * A Flag that can reads its overrides from DeviceConfig. + * + * This is generally useful for flags that come from or are used _outside_ of SystemUI. + * + * Prefer [UnreleasedFlag] and [ReleasedFlag]. + */ data class DeviceConfigBooleanFlag @JvmOverloads constructor( override val id: Int, override val name: String, @@ -94,6 +132,13 @@ data class DeviceConfigBooleanFlag @JvmOverloads constructor( override val teamfood: Boolean = false ) : DeviceConfigFlag +/** + * A Flag that can reads its overrides from System Properties. + * + * This is generally useful for flags that come from or are used _outside_ of SystemUI. + * + * Prefer [UnreleasedFlag] and [ReleasedFlag]. + */ data class SysPropBooleanFlag @JvmOverloads constructor( override val id: Int, override val name: String, diff --git a/packages/SystemUI/src-debug/com/android/systemui/flags/FlagsModule.kt b/packages/SystemUI/src-debug/com/android/systemui/flags/FlagsModule.kt index 256028417a1d7..74bd9c6c287df 100644 --- a/packages/SystemUI/src-debug/com/android/systemui/flags/FlagsModule.kt +++ b/packages/SystemUI/src-debug/com/android/systemui/flags/FlagsModule.kt @@ -27,7 +27,8 @@ import dagger.Provides import javax.inject.Named @Module(includes = [ - SettingsUtilModule::class + ServerFlagReaderModule::class, + SettingsUtilModule::class, ]) abstract class FlagsModule { @Binds @@ -46,4 +47,4 @@ abstract class FlagsModule { @Named(ALL_FLAGS) fun providesAllFlags(): Map> = Flags.collectFlags() } -} \ No newline at end of file +} diff --git a/packages/SystemUI/src-release/com/android/systemui/flags/FlagsModule.kt b/packages/SystemUI/src-release/com/android/systemui/flags/FlagsModule.kt index ab9e01eebaf78..38b5c9a9fa794 100644 --- a/packages/SystemUI/src-release/com/android/systemui/flags/FlagsModule.kt +++ b/packages/SystemUI/src-release/com/android/systemui/flags/FlagsModule.kt @@ -19,8 +19,8 @@ package com.android.systemui.flags import dagger.Binds import dagger.Module -@Module +@Module(includes = [ServerFlagReaderModule::class]) abstract class FlagsModule { @Binds abstract fun bindsFeatureFlagRelease(impl: FeatureFlagsRelease): FeatureFlags -} \ No newline at end of file +} diff --git a/packages/SystemUI/src/com/android/systemui/flags/FeatureFlags.kt b/packages/SystemUI/src/com/android/systemui/flags/FeatureFlags.kt index 2cee2520ce556..dfa3bcda7d725 100644 --- a/packages/SystemUI/src/com/android/systemui/flags/FeatureFlags.kt +++ b/packages/SystemUI/src/com/android/systemui/flags/FeatureFlags.kt @@ -23,7 +23,10 @@ package com.android.systemui.flags */ interface FeatureFlags : FlagListenable { /** Returns a boolean value for the given flag. */ - fun isEnabled(flag: BooleanFlag): Boolean + fun isEnabled(flag: UnreleasedFlag): Boolean + + /** Returns a boolean value for the given flag. */ + fun isEnabled(flag: ReleasedFlag): Boolean /** Returns a boolean value for the given flag. */ fun isEnabled(flag: ResourceBooleanFlag): Boolean @@ -39,4 +42,4 @@ interface FeatureFlags : FlagListenable { /** Returns a string value for the given flag. */ fun getString(flag: ResourceStringFlag): String -} \ No newline at end of file +} diff --git a/packages/SystemUI/src/com/android/systemui/flags/FeatureFlagsDebug.java b/packages/SystemUI/src/com/android/systemui/flags/FeatureFlagsDebug.java index c5221cd9641b9..00c1a99983df7 100644 --- a/packages/SystemUI/src/com/android/systemui/flags/FeatureFlagsDebug.java +++ b/packages/SystemUI/src/com/android/systemui/flags/FeatureFlagsDebug.java @@ -47,6 +47,8 @@ import com.android.systemui.statusbar.commandline.CommandRegistry; import com.android.systemui.util.DeviceConfigProxy; import com.android.systemui.util.settings.SecureSettings; +import org.jetbrains.annotations.NotNull; + import java.io.PrintWriter; import java.lang.reflect.Field; import java.util.ArrayList; @@ -83,6 +85,7 @@ public class FeatureFlagsDebug implements FeatureFlags, Dumpable { private final Resources mResources; private final SystemPropertiesHelper mSystemProperties; private final DeviceConfigProxy mDeviceConfigProxy; + private final ServerFlagReader mServerFlagReader; private final Map> mAllFlags; private final Map mBooleanFlagCache = new TreeMap<>(); private final Map mStringFlagCache = new TreeMap<>(); @@ -97,6 +100,7 @@ public class FeatureFlagsDebug implements FeatureFlags, Dumpable { @Main Resources resources, DumpManager dumpManager, DeviceConfigProxy deviceConfigProxy, + ServerFlagReader serverFlagReader, @Named(ALL_FLAGS) Map> allFlags, CommandRegistry commandRegistry, IStatusBarService barService) { @@ -105,6 +109,7 @@ public class FeatureFlagsDebug implements FeatureFlags, Dumpable { mResources = resources; mSystemProperties = systemProperties; mDeviceConfigProxy = deviceConfigProxy; + mServerFlagReader = serverFlagReader; mAllFlags = allFlags; mBarService = barService; @@ -120,7 +125,16 @@ public class FeatureFlagsDebug implements FeatureFlags, Dumpable { } @Override - public boolean isEnabled(@NonNull BooleanFlag flag) { + public boolean isEnabled(@NotNull UnreleasedFlag flag) { + return isEnabledInternal(flag); + } + + @Override + public boolean isEnabled(@NotNull ReleasedFlag flag) { + return isEnabledInternal(flag); + } + + private boolean isEnabledInternal(@NotNull BooleanFlag flag) { int id = flag.getId(); if (!mBooleanFlagCache.containsKey(id)) { mBooleanFlagCache.put(id, @@ -197,14 +211,17 @@ public class FeatureFlagsDebug implements FeatureFlags, Dumpable { /** Specific override for Boolean flags that checks against the teamfood list.*/ private boolean readFlagValue(int id, boolean defaultValue) { Boolean result = readBooleanFlagOverride(id); - // Only check for teamfood if the default is false. - if (!defaultValue && result == null && id != Flags.TEAMFOOD.getId()) { + boolean hasServerOverride = mServerFlagReader.hasOverride(id); + + // Only check for teamfood if the default is false + // and there is no server override. + if (!hasServerOverride && !defaultValue && result == null && id != Flags.TEAMFOOD.getId()) { if (mAllFlags.containsKey(id) && mAllFlags.get(id).getTeamfood()) { return isEnabled(Flags.TEAMFOOD); } } - return result == null ? defaultValue : result; + return result == null ? mServerFlagReader.readServerOverride(id, defaultValue) : result; } private Boolean readBooleanFlagOverride(int id) { @@ -409,36 +426,38 @@ public class FeatureFlagsDebug implements FeatureFlags, Dumpable { */ @Nullable private ParcelableFlag toParcelableFlag(Flag f) { - if (f instanceof BooleanFlag) { - return new BooleanFlag( - f.getId(), - isEnabled((BooleanFlag) f), - f.getTeamfood(), - readBooleanFlagOverride(f.getId()) != null); - } - if (f instanceof ResourceBooleanFlag) { - return new BooleanFlag( - f.getId(), - isEnabled((ResourceBooleanFlag) f), - f.getTeamfood(), - readBooleanFlagOverride(f.getId()) != null); - } - if (f instanceof DeviceConfigBooleanFlag) { - return new BooleanFlag( - f.getId(), isEnabled((DeviceConfigBooleanFlag) f), f.getTeamfood()); - } - if (f instanceof SysPropBooleanFlag) { + boolean enabled; + boolean teamfood = f.getTeamfood(); + boolean overridden; + + if (f instanceof ReleasedFlag) { + enabled = isEnabled((ReleasedFlag) f); + overridden = readBooleanFlagOverride(f.getId()) != null; + } else if (f instanceof UnreleasedFlag) { + enabled = isEnabled((UnreleasedFlag) f); + overridden = readBooleanFlagOverride(f.getId()) != null; + } else if (f instanceof ResourceBooleanFlag) { + enabled = isEnabled((ResourceBooleanFlag) f); + overridden = readBooleanFlagOverride(f.getId()) != null; + } else if (f instanceof DeviceConfigBooleanFlag) { + enabled = isEnabled((DeviceConfigBooleanFlag) f); + overridden = false; + } else if (f instanceof SysPropBooleanFlag) { // TODO(b/223379190): Teamfood not supported for sysprop flags yet. - return new BooleanFlag( - f.getId(), - ((SysPropBooleanFlag) f).getDefault(), - false, - !mSystemProperties.get(((SysPropBooleanFlag) f).getName()).isEmpty()); + enabled = isEnabled((SysPropBooleanFlag) f); + teamfood = false; + overridden = !mSystemProperties.get(((SysPropBooleanFlag) f).getName()).isEmpty(); + } else { + // TODO: add support for other flag types. + Log.w(TAG, "Unsupported Flag Type. Please file a bug."); + return null; } - // TODO: add support for other flag types. - Log.w(TAG, "Unsupported Flag Type. Please file a bug."); - return null; + if (enabled) { + return new ReleasedFlag(f.getId(), teamfood, overridden); + } else { + return new UnreleasedFlag(f.getId(), teamfood, overridden); + } } }; @@ -539,8 +558,10 @@ public class FeatureFlagsDebug implements FeatureFlags, Dumpable { } private boolean isBooleanFlagEnabled(Flag flag) { - if (flag instanceof BooleanFlag) { - return isEnabled((BooleanFlag) flag); + if (flag instanceof ReleasedFlag) { + return isEnabled((ReleasedFlag) flag); + } else if (flag instanceof UnreleasedFlag) { + return isEnabled((UnreleasedFlag) flag); } else if (flag instanceof ResourceBooleanFlag) { return isEnabled((ResourceBooleanFlag) flag); } else if (flag instanceof SysPropFlag) { diff --git a/packages/SystemUI/src/com/android/systemui/flags/FeatureFlagsRelease.java b/packages/SystemUI/src/com/android/systemui/flags/FeatureFlagsRelease.java index 1492a2bc0cebf..049b17d383a27 100644 --- a/packages/SystemUI/src/com/android/systemui/flags/FeatureFlagsRelease.java +++ b/packages/SystemUI/src/com/android/systemui/flags/FeatureFlagsRelease.java @@ -19,7 +19,6 @@ package com.android.systemui.flags; import static java.util.Objects.requireNonNull; import android.content.res.Resources; -import android.provider.DeviceConfig; import android.util.SparseArray; import android.util.SparseBooleanArray; @@ -31,6 +30,8 @@ import com.android.systemui.dagger.qualifiers.Main; import com.android.systemui.dump.DumpManager; import com.android.systemui.util.DeviceConfigProxy; +import org.jetbrains.annotations.NotNull; + import java.io.PrintWriter; import java.util.Map; @@ -47,18 +48,22 @@ public class FeatureFlagsRelease implements FeatureFlags, Dumpable { private final Resources mResources; private final SystemPropertiesHelper mSystemProperties; private final DeviceConfigProxy mDeviceConfigProxy; + private final ServerFlagReader mServerFlagReader; SparseBooleanArray mBooleanCache = new SparseBooleanArray(); SparseArray mStringCache = new SparseArray<>(); + private boolean mInited; @Inject public FeatureFlagsRelease( @Main Resources resources, SystemPropertiesHelper systemProperties, DeviceConfigProxy deviceConfigProxy, + ServerFlagReader serverFlagReader, DumpManager dumpManager) { mResources = resources; mSystemProperties = systemProperties; mDeviceConfigProxy = deviceConfigProxy; + mServerFlagReader = serverFlagReader; dumpManager.registerDumpable("SysUIFlags", this); } @@ -69,8 +74,13 @@ public class FeatureFlagsRelease implements FeatureFlags, Dumpable { public void removeListener(@NonNull Listener listener) {} @Override - public boolean isEnabled(BooleanFlag flag) { - return flag.getDefault(); + public boolean isEnabled(@NotNull UnreleasedFlag flag) { + return false; + } + + @Override + public boolean isEnabled(@NotNull ReleasedFlag flag) { + return mServerFlagReader.readServerOverride(flag.getId(), true); } @Override diff --git a/packages/SystemUI/src/com/android/systemui/flags/Flags.java b/packages/SystemUI/src/com/android/systemui/flags/Flags.java index f13729e57fa05..0dc07ac35e3d6 100644 --- a/packages/SystemUI/src/com/android/systemui/flags/Flags.java +++ b/packages/SystemUI/src/com/android/systemui/flags/Flags.java @@ -32,7 +32,7 @@ import java.util.Map; * * Flag Ids are integers. * Ids must be unique. This is enforced in a unit test. - * Ids need not be sequential. Flags can "claim" a chunk of ids for flags in related featurs with + * Ids need not be sequential. Flags can "claim" a chunk of ids for flags in related features with * a comment. This is purely for organizational purposes. * * On public release builds, flags will always return their default value. There is no way to @@ -41,30 +41,30 @@ import java.util.Map; * See {@link FeatureFlagsDebug} for instructions on flipping the flags via adb. */ public class Flags { - public static final BooleanFlag TEAMFOOD = new BooleanFlag(1, false); + public static final UnreleasedFlag TEAMFOOD = new UnreleasedFlag(1); /***************************************/ // 100 - notification - public static final BooleanFlag NOTIFICATION_PIPELINE_DEVELOPER_LOGGING = - new BooleanFlag(103, false); + public static final UnreleasedFlag NOTIFICATION_PIPELINE_DEVELOPER_LOGGING = + new UnreleasedFlag(103); - public static final BooleanFlag NSSL_DEBUG_LINES = - new BooleanFlag(105, false); + public static final UnreleasedFlag NSSL_DEBUG_LINES = + new UnreleasedFlag(105); - public static final BooleanFlag NSSL_DEBUG_REMOVE_ANIMATION = - new BooleanFlag(106, false); + public static final UnreleasedFlag NSSL_DEBUG_REMOVE_ANIMATION = + new UnreleasedFlag(106); - public static final BooleanFlag NEW_PIPELINE_CRASH_ON_CALL_TO_OLD_PIPELINE = - new BooleanFlag(107, false); + public static final UnreleasedFlag NEW_PIPELINE_CRASH_ON_CALL_TO_OLD_PIPELINE = + new UnreleasedFlag(107); public static final ResourceBooleanFlag NOTIFICATION_DRAG_TO_CONTENTS = new ResourceBooleanFlag(108, R.bool.config_notificationToContents); - public static final BooleanFlag REMOVE_UNRANKED_NOTIFICATIONS = - new BooleanFlag(109, false, true); + public static final UnreleasedFlag REMOVE_UNRANKED_NOTIFICATIONS = + new UnreleasedFlag(109, true); - public static final BooleanFlag FSI_REQUIRES_KEYGUARD = - new BooleanFlag(110, false, true); + public static final UnreleasedFlag FSI_REQUIRES_KEYGUARD = + new UnreleasedFlag(110, true); /***************************************/ // 200 - keyguard/lockscreen @@ -73,11 +73,11 @@ public class Flags { // public static final BooleanFlag KEYGUARD_LAYOUT = // new BooleanFlag(200, true); - public static final BooleanFlag LOCKSCREEN_ANIMATIONS = - new BooleanFlag(201, true); + public static final ReleasedFlag LOCKSCREEN_ANIMATIONS = + new ReleasedFlag(201); - public static final BooleanFlag NEW_UNLOCK_SWIPE_ANIMATION = - new BooleanFlag(202, true); + public static final ReleasedFlag NEW_UNLOCK_SWIPE_ANIMATION = + new ReleasedFlag(202); public static final ResourceBooleanFlag CHARGING_RIPPLE = new ResourceBooleanFlag(203, R.bool.flag_charging_ripple); @@ -92,31 +92,29 @@ public class Flags { * Whether the KeyguardBottomArea(View|Controller) should use the modern architecture or the old * one. */ - public static final BooleanFlag MODERN_BOTTOM_AREA = new BooleanFlag( - 206, - /* default= */ false, - /* teamfood= */ true); + public static final UnreleasedFlag MODERN_BOTTOM_AREA = new UnreleasedFlag(206, true); - public static final BooleanFlag LOCKSCREEN_CUSTOM_CLOCKS = new BooleanFlag(207, false); + + public static final UnreleasedFlag LOCKSCREEN_CUSTOM_CLOCKS = new UnreleasedFlag(207); /** * Flag to enable the usage of the new bouncer data source. This is a refactor of and * eventual replacement of KeyguardBouncer.java. */ - public static final BooleanFlag MODERN_BOUNCER = new BooleanFlag(208, true); + public static final ReleasedFlag MODERN_BOUNCER = new ReleasedFlag(208); /***************************************/ // 300 - power menu - public static final BooleanFlag POWER_MENU_LITE = - new BooleanFlag(300, true); + public static final ReleasedFlag POWER_MENU_LITE = + new ReleasedFlag(300); /***************************************/ // 400 - smartspace - public static final BooleanFlag SMARTSPACE_DEDUPING = - new BooleanFlag(400, true); + public static final ReleasedFlag SMARTSPACE_DEDUPING = + new ReleasedFlag(400); - public static final BooleanFlag SMARTSPACE_SHARED_ELEMENT_TRANSITION_ENABLED = - new BooleanFlag(401, true); + public static final ReleasedFlag SMARTSPACE_SHARED_ELEMENT_TRANSITION_ENABLED = + new ReleasedFlag(401); public static final ResourceBooleanFlag SMARTSPACE = new ResourceBooleanFlag(402, R.bool.flag_smartspace); @@ -127,11 +125,11 @@ public class Flags { * @deprecated Not needed anymore */ @Deprecated - public static final BooleanFlag NEW_USER_SWITCHER = - new BooleanFlag(500, true); + public static final ReleasedFlag NEW_USER_SWITCHER = + new ReleasedFlag(500); - public static final BooleanFlag COMBINED_QS_HEADERS = - new BooleanFlag(501, false, true); + public static final UnreleasedFlag COMBINED_QS_HEADERS = + new UnreleasedFlag(501, true); public static final ResourceBooleanFlag PEOPLE_TILE = new ResourceBooleanFlag(502, R.bool.flag_conversations); @@ -143,9 +141,9 @@ public class Flags { * @deprecated Not needed anymore */ @Deprecated - public static final BooleanFlag NEW_FOOTER = new BooleanFlag(504, true); + public static final ReleasedFlag NEW_FOOTER = new ReleasedFlag(504); - public static final BooleanFlag NEW_HEADER = new BooleanFlag(505, false, true); + public static final UnreleasedFlag NEW_HEADER = new UnreleasedFlag(505, true); public static final ResourceBooleanFlag FULL_SCREEN_USER_SWITCHER = new ResourceBooleanFlag(506, R.bool.config_enableFullscreenUserSwitcher); @@ -154,21 +152,21 @@ public class Flags { public static final ResourceBooleanFlag STATUS_BAR_USER_SWITCHER = new ResourceBooleanFlag(602, R.bool.flag_user_switcher_chip); - public static final BooleanFlag STATUS_BAR_LETTERBOX_APPEARANCE = - new BooleanFlag(603, false); + public static final UnreleasedFlag STATUS_BAR_LETTERBOX_APPEARANCE = + new UnreleasedFlag(603, false); - public static final BooleanFlag NEW_STATUS_BAR_PIPELINE = new BooleanFlag(604, false); + public static final UnreleasedFlag NEW_STATUS_BAR_PIPELINE = new UnreleasedFlag(604, true); /***************************************/ // 700 - dialer/calls - public static final BooleanFlag ONGOING_CALL_STATUS_BAR_CHIP = - new BooleanFlag(700, true); + public static final ReleasedFlag ONGOING_CALL_STATUS_BAR_CHIP = + new ReleasedFlag(700); - public static final BooleanFlag ONGOING_CALL_IN_IMMERSIVE = - new BooleanFlag(701, true); + public static final ReleasedFlag ONGOING_CALL_IN_IMMERSIVE = + new ReleasedFlag(701); - public static final BooleanFlag ONGOING_CALL_IN_IMMERSIVE_CHIP_TAP = - new BooleanFlag(702, true); + public static final ReleasedFlag ONGOING_CALL_IN_IMMERSIVE_CHIP_TAP = + new ReleasedFlag(702); /***************************************/ // 800 - general visual/theme @@ -177,15 +175,15 @@ public class Flags { /***************************************/ // 900 - media - public static final BooleanFlag MEDIA_TAP_TO_TRANSFER = new BooleanFlag(900, true); - public static final BooleanFlag MEDIA_SESSION_ACTIONS = new BooleanFlag(901, false); - public static final BooleanFlag MEDIA_NEARBY_DEVICES = new BooleanFlag(903, true); - public static final BooleanFlag MEDIA_MUTE_AWAIT = new BooleanFlag(904, true); + public static final ReleasedFlag MEDIA_TAP_TO_TRANSFER = new ReleasedFlag(900); + public static final UnreleasedFlag MEDIA_SESSION_ACTIONS = new UnreleasedFlag(901); + public static final ReleasedFlag MEDIA_NEARBY_DEVICES = new ReleasedFlag(903); + public static final ReleasedFlag MEDIA_MUTE_AWAIT = new ReleasedFlag(904); // 1000 - dock - public static final BooleanFlag SIMULATE_DOCK_THROUGH_CHARGING = - new BooleanFlag(1000, true); - public static final BooleanFlag DOCK_SETUP_ENABLED = new BooleanFlag(1001, true); + public static final ReleasedFlag SIMULATE_DOCK_THROUGH_CHARGING = + new ReleasedFlag(1000); + public static final ReleasedFlag DOCK_SETUP_ENABLED = new ReleasedFlag(1001); // 1100 - windowing @@ -220,12 +218,12 @@ public class Flags { public static final SysPropBooleanFlag WM_ALWAYS_ENFORCE_PREDICTIVE_BACK = new SysPropBooleanFlag(1202, "persist.wm.debug.predictive_back_always_enforce", false); - public static final BooleanFlag NEW_BACK_AFFORDANCE = - new BooleanFlag(1203, false /* default */, false /* teamfood */); + public static final UnreleasedFlag NEW_BACK_AFFORDANCE = + new UnreleasedFlag(1203, false /* teamfood */); // 1300 - screenshots - public static final BooleanFlag SCREENSHOT_REQUEST_PROCESSOR = new BooleanFlag(1300, false); + public static final UnreleasedFlag SCREENSHOT_REQUEST_PROCESSOR = new UnreleasedFlag(1300); // Pay no attention to the reflection behind the curtain. // ========================== Curtain ========================== diff --git a/packages/SystemUI/src/com/android/systemui/flags/ServerFlagReader.kt b/packages/SystemUI/src/com/android/systemui/flags/ServerFlagReader.kt new file mode 100644 index 0000000000000..fc5b9f4eea056 --- /dev/null +++ b/packages/SystemUI/src/com/android/systemui/flags/ServerFlagReader.kt @@ -0,0 +1,80 @@ +/* + * Copyright (C) 2022 The Android Open Source Project + * + * 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 com.android.systemui.flags + +import com.android.systemui.util.DeviceConfigProxy +import dagger.Binds +import dagger.Module +import javax.inject.Inject + +interface ServerFlagReader { + /** Returns true if there is a server-side setting stored. */ + fun hasOverride(flagId: Int): Boolean + + /** Returns any stored server-side setting or the default if not set. */ + fun readServerOverride(flagId: Int, default: Boolean): Boolean +} + +class ServerFlagReaderImpl @Inject constructor( + private val deviceConfig: DeviceConfigProxy +) : ServerFlagReader { + override fun hasOverride(flagId: Int): Boolean = + deviceConfig.getProperty( + SYSUI_NAMESPACE, + getServerOverrideName(flagId) + ) != null + + override fun readServerOverride(flagId: Int, default: Boolean): Boolean { + return deviceConfig.getBoolean( + SYSUI_NAMESPACE, + getServerOverrideName(flagId), + default + ) + } + + private fun getServerOverrideName(flagId: Int): String { + return "flag_override_$flagId" + } +} + +private val SYSUI_NAMESPACE = "systemui" + +@Module +interface ServerFlagReaderModule { + @Binds + fun bindsReader(impl: ServerFlagReaderImpl): ServerFlagReader +} + +class ServerFlagReaderFake : ServerFlagReader { + private val flagMap: MutableMap = mutableMapOf() + + override fun hasOverride(flagId: Int): Boolean { + return flagMap.containsKey(flagId) + } + + override fun readServerOverride(flagId: Int, default: Boolean): Boolean { + return flagMap.getOrDefault(flagId, default) + } + + fun setFlagValue(flagId: Int, value: Boolean) { + flagMap.put(flagId, value) + } + + fun eraseFlag(flagId: Int) { + flagMap.remove(flagId) + } +} diff --git a/packages/SystemUI/tests/src/com/android/systemui/flags/FakeFeatureFlags.kt b/packages/SystemUI/tests/src/com/android/systemui/flags/FakeFeatureFlags.kt index b2a4e33978a14..7b1455cb2e465 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/flags/FakeFeatureFlags.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/flags/FakeFeatureFlags.kt @@ -19,7 +19,6 @@ package com.android.systemui.flags import android.util.SparseArray import android.util.SparseBooleanArray import androidx.core.util.containsKey -import java.lang.IllegalStateException class FakeFeatureFlags : FeatureFlags { private val booleanFlags = SparseBooleanArray() @@ -57,7 +56,10 @@ class FakeFeatureFlags : FeatureFlags { stringFlags.put(flag.id, value) } - override fun isEnabled(flag: BooleanFlag): Boolean = requireBooleanValue(flag.id) + + override fun isEnabled(flag: UnreleasedFlag): Boolean = requireBooleanValue(flag.id) + + override fun isEnabled(flag: ReleasedFlag): Boolean = requireBooleanValue(flag.id) override fun isEnabled(flag: ResourceBooleanFlag): Boolean = requireBooleanValue(flag.id) diff --git a/packages/SystemUI/tests/src/com/android/systemui/flags/FakeFeatureFlagsTest.kt b/packages/SystemUI/tests/src/com/android/systemui/flags/FakeFeatureFlagsTest.kt index 7d4b4f53e3808..ff579a1cc4aad 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/flags/FakeFeatureFlagsTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/flags/FakeFeatureFlagsTest.kt @@ -29,11 +29,12 @@ import org.junit.runner.RunWith @RunWith(AndroidTestingRunner::class) class FakeFeatureFlagsTest : SysuiTestCase() { - private val booleanFlag = BooleanFlag(-1000) - private val stringFlag = StringFlag(-1001) - private val resourceBooleanFlag = ResourceBooleanFlag(-1002, resourceId = -1) - private val resourceStringFlag = ResourceStringFlag(-1003, resourceId = -1) - private val sysPropBooleanFlag = SysPropBooleanFlag(-1004, name = "test") + private val unreleasedFlag = UnreleasedFlag(-1000) + private val releasedFlag = ReleasedFlag(-1001) + private val stringFlag = StringFlag(-1002) + private val resourceBooleanFlag = ResourceBooleanFlag(-1003, resourceId = -1) + private val resourceStringFlag = ResourceStringFlag(-1004, resourceId = -1) + private val sysPropBooleanFlag = SysPropBooleanFlag(-1005, name = "test") /** * FakeFeatureFlags does not honor any default values. All flags which are accessed must be @@ -49,56 +50,66 @@ class FakeFeatureFlagsTest : SysuiTestCase() { assertThat(ex.message).contains("TEAMFOOD") } try { - assertThat(flags.isEnabled(booleanFlag)).isFalse() + assertThat(flags.isEnabled(unreleasedFlag)).isFalse() fail("Expected an exception when accessing an unspecified flag.") } catch (ex: IllegalStateException) { assertThat(ex.message).contains("UNKNOWN(id=-1000)") } try { - assertThat(flags.isEnabled(resourceBooleanFlag)).isFalse() - fail("Expected an exception when accessing an unspecified flag.") - } catch (ex: IllegalStateException) { - assertThat(ex.message).contains("UNKNOWN(id=-1002)") - } - try { - assertThat(flags.isEnabled(sysPropBooleanFlag)).isFalse() - fail("Expected an exception when accessing an unspecified flag.") - } catch (ex: IllegalStateException) { - assertThat(ex.message).contains("UNKNOWN(id=-1004)") - } - try { - assertThat(flags.getString(stringFlag)).isEmpty() + assertThat(flags.isEnabled(releasedFlag)).isFalse() fail("Expected an exception when accessing an unspecified flag.") } catch (ex: IllegalStateException) { assertThat(ex.message).contains("UNKNOWN(id=-1001)") } try { - assertThat(flags.getString(resourceStringFlag)).isEmpty() + assertThat(flags.isEnabled(resourceBooleanFlag)).isFalse() fail("Expected an exception when accessing an unspecified flag.") } catch (ex: IllegalStateException) { assertThat(ex.message).contains("UNKNOWN(id=-1003)") } + try { + assertThat(flags.isEnabled(sysPropBooleanFlag)).isFalse() + fail("Expected an exception when accessing an unspecified flag.") + } catch (ex: IllegalStateException) { + assertThat(ex.message).contains("UNKNOWN(id=-1005)") + } + try { + assertThat(flags.getString(stringFlag)).isEmpty() + fail("Expected an exception when accessing an unspecified flag.") + } catch (ex: IllegalStateException) { + assertThat(ex.message).contains("UNKNOWN(id=-1002)") + } + try { + assertThat(flags.getString(resourceStringFlag)).isEmpty() + fail("Expected an exception when accessing an unspecified flag.") + } catch (ex: IllegalStateException) { + assertThat(ex.message).contains("UNKNOWN(id=-1004)") + } } @Test fun specifiedFlagsReturnCorrectValues() { val flags = FakeFeatureFlags() - flags.set(booleanFlag, false) + flags.set(unreleasedFlag, false) + flags.set(releasedFlag, false) flags.set(resourceBooleanFlag, false) flags.set(sysPropBooleanFlag, false) flags.set(resourceStringFlag, "") - assertThat(flags.isEnabled(booleanFlag)).isFalse() + assertThat(flags.isEnabled(unreleasedFlag)).isFalse() + assertThat(flags.isEnabled(releasedFlag)).isFalse() assertThat(flags.isEnabled(resourceBooleanFlag)).isFalse() assertThat(flags.isEnabled(sysPropBooleanFlag)).isFalse() assertThat(flags.getString(resourceStringFlag)).isEmpty() - flags.set(booleanFlag, true) + flags.set(unreleasedFlag, true) + flags.set(releasedFlag, true) flags.set(resourceBooleanFlag, true) flags.set(sysPropBooleanFlag, true) flags.set(resourceStringFlag, "Android") - assertThat(flags.isEnabled(booleanFlag)).isTrue() + assertThat(flags.isEnabled(unreleasedFlag)).isTrue() + assertThat(flags.isEnabled(releasedFlag)).isTrue() assertThat(flags.isEnabled(resourceBooleanFlag)).isTrue() assertThat(flags.isEnabled(sysPropBooleanFlag)).isTrue() assertThat(flags.getString(resourceStringFlag)).isEqualTo("Android") diff --git a/packages/SystemUI/tests/src/com/android/systemui/flags/FeatureFlagsDebugTest.kt b/packages/SystemUI/tests/src/com/android/systemui/flags/FeatureFlagsDebugTest.kt index 51f3404a9bca6..4511193d41d3a 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/flags/FeatureFlagsDebugTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/flags/FeatureFlagsDebugTest.kt @@ -35,6 +35,10 @@ import com.android.systemui.util.mockito.nullable import com.android.systemui.util.mockito.withArgCaptor import com.android.systemui.util.settings.SecureSettings import com.google.common.truth.Truth.assertThat +import java.io.PrintWriter +import java.io.Serializable +import java.io.StringWriter +import java.util.function.Consumer import org.junit.Assert import org.junit.Before import org.junit.Test @@ -48,12 +52,8 @@ import org.mockito.Mockito.times import org.mockito.Mockito.verify import org.mockito.Mockito.verifyNoMoreInteractions import org.mockito.Mockito.verifyZeroInteractions -import org.mockito.MockitoAnnotations -import java.io.PrintWriter -import java.io.Serializable -import java.io.StringWriter -import java.util.function.Consumer import org.mockito.Mockito.`when` as whenever +import org.mockito.MockitoAnnotations /** * NOTE: This test is for the version of FeatureFlagManager in src-release, which should not allow @@ -75,10 +75,11 @@ class FeatureFlagsDebugTest : SysuiTestCase() { private val flagMap = mutableMapOf>() private lateinit var broadcastReceiver: BroadcastReceiver private lateinit var clearCacheAction: Consumer + private val serverFlagReader = ServerFlagReaderFake() private val deviceConfig = DeviceConfigProxyFake() - private val teamfoodableFlagA = BooleanFlag(500, false, true) - private val teamfoodableFlagB = BooleanFlag(501, true, true) + private val teamfoodableFlagA = UnreleasedFlag(500, true) + private val teamfoodableFlagB = ReleasedFlag(501, true) @Before fun setup() { @@ -93,6 +94,7 @@ class FeatureFlagsDebugTest : SysuiTestCase() { resources, dumpManager, deviceConfig, + serverFlagReader, flagMap, commandRegistry, barService @@ -109,40 +111,41 @@ class FeatureFlagsDebugTest : SysuiTestCase() { } @Test - fun testReadBooleanFlag() { + fun readBooleanFlag() { // Remember that the TEAMFOOD flag is id#1 and has special behavior. whenever(flagManager.readFlagValue(eq(3), any())).thenReturn(true) whenever(flagManager.readFlagValue(eq(4), any())).thenReturn(false) - assertThat(mFeatureFlagsDebug.isEnabled(BooleanFlag(2, true))).isTrue() - assertThat(mFeatureFlagsDebug.isEnabled(BooleanFlag(3, false))).isTrue() - assertThat(mFeatureFlagsDebug.isEnabled(BooleanFlag(4, true))).isFalse() - assertThat(mFeatureFlagsDebug.isEnabled(BooleanFlag(5, false))).isFalse() + + assertThat(mFeatureFlagsDebug.isEnabled(ReleasedFlag(2))).isTrue() + assertThat(mFeatureFlagsDebug.isEnabled(UnreleasedFlag(3))).isTrue() + assertThat(mFeatureFlagsDebug.isEnabled(ReleasedFlag(4))).isFalse() + assertThat(mFeatureFlagsDebug.isEnabled(UnreleasedFlag(5))).isFalse() } @Test - fun testTeamFoodFlag_False() { + fun teamFoodFlag_False() { whenever(flagManager.readFlagValue(eq(1), any())).thenReturn(false) assertThat(mFeatureFlagsDebug.isEnabled(teamfoodableFlagA)).isFalse() assertThat(mFeatureFlagsDebug.isEnabled(teamfoodableFlagB)).isTrue() // Regular boolean flags should still test the same. // Only our teamfoodableFlag should change. - testReadBooleanFlag() + readBooleanFlag() } @Test - fun testTeamFoodFlag_True() { + fun teamFoodFlag_True() { whenever(flagManager.readFlagValue(eq(1), any())).thenReturn(true) assertThat(mFeatureFlagsDebug.isEnabled(teamfoodableFlagA)).isTrue() assertThat(mFeatureFlagsDebug.isEnabled(teamfoodableFlagB)).isTrue() // Regular boolean flags should still test the same. // Only our teamfoodableFlag should change. - testReadBooleanFlag() + readBooleanFlag() } @Test - fun testTeamFoodFlag_Overridden() { + fun teamFoodFlag_Overridden() { whenever(flagManager.readFlagValue(eq(teamfoodableFlagA.id), any())) .thenReturn(true) whenever(flagManager.readFlagValue(eq(teamfoodableFlagB.id), any())) @@ -153,11 +156,11 @@ class FeatureFlagsDebugTest : SysuiTestCase() { // Regular boolean flags should still test the same. // Only our teamfoodableFlag should change. - testReadBooleanFlag() + readBooleanFlag() } @Test - fun testReadResourceBooleanFlag() { + fun readResourceBooleanFlag() { whenever(resources.getBoolean(1001)).thenReturn(false) whenever(resources.getBoolean(1002)).thenReturn(true) whenever(resources.getBoolean(1003)).thenReturn(false) @@ -182,7 +185,7 @@ class FeatureFlagsDebugTest : SysuiTestCase() { } @Test - fun testReadSysPropBooleanFlag() { + fun readSysPropBooleanFlag() { whenever(systemProperties.getBoolean(anyString(), anyBoolean())).thenAnswer { if ("b".equals(it.getArgument(0))) { return@thenAnswer true @@ -198,7 +201,7 @@ class FeatureFlagsDebugTest : SysuiTestCase() { } @Test - fun testReadDeviceConfigBooleanFlag() { + fun readDeviceConfigBooleanFlag() { val namespace = "test_namespace" deviceConfig.setProperty(namespace, "a", "true", false) deviceConfig.setProperty(namespace, "b", "false", false) @@ -213,7 +216,7 @@ class FeatureFlagsDebugTest : SysuiTestCase() { } @Test - fun testReadStringFlag() { + fun readStringFlag() { whenever(flagManager.readFlagValue(eq(3), any())).thenReturn("foo") whenever(flagManager.readFlagValue(eq(4), any())).thenReturn("bar") assertThat(mFeatureFlagsDebug.getString(StringFlag(1, "biz"))).isEqualTo("biz") @@ -223,7 +226,7 @@ class FeatureFlagsDebugTest : SysuiTestCase() { } @Test - fun testReadResourceStringFlag() { + fun readResourceStringFlag() { whenever(resources.getString(1001)).thenReturn("") whenever(resources.getString(1002)).thenReturn("resource2") whenever(resources.getString(1003)).thenReturn("resource3") @@ -253,8 +256,8 @@ class FeatureFlagsDebugTest : SysuiTestCase() { } @Test - fun testBroadcastReceiverIgnoresInvalidData() { - addFlag(BooleanFlag(1, false)) + fun broadcastReceiver_IgnoresInvalidData() { + addFlag(UnreleasedFlag(1)) addFlag(ResourceBooleanFlag(2, 1002)) addFlag(StringFlag(3, "flag3")) addFlag(ResourceStringFlag(4, 1004)) @@ -272,10 +275,10 @@ class FeatureFlagsDebugTest : SysuiTestCase() { } @Test - fun testIntentWithIdButNoValueKeyClears() { - addFlag(BooleanFlag(1, false)) + fun intentWithId_NoValueKeyClears() { + addFlag(UnreleasedFlag(1)) - // trying to erase an id not in the map does noting + // trying to erase an id not in the map does nothing broadcastReceiver.onReceive( mockContext, Intent(FlagManager.ACTION_SET_FLAG).putExtra(FlagManager.EXTRA_ID, 0) @@ -291,9 +294,9 @@ class FeatureFlagsDebugTest : SysuiTestCase() { } @Test - fun testSetBooleanFlag() { - addFlag(BooleanFlag(1, false)) - addFlag(BooleanFlag(2, false)) + fun setBooleanFlag() { + addFlag(UnreleasedFlag(1)) + addFlag(UnreleasedFlag(2)) addFlag(ResourceBooleanFlag(3, 1003)) addFlag(ResourceBooleanFlag(4, 1004)) @@ -311,7 +314,7 @@ class FeatureFlagsDebugTest : SysuiTestCase() { } @Test - fun testSetStringFlag() { + fun setStringFlag() { addFlag(StringFlag(1, "flag1")) addFlag(ResourceStringFlag(2, 1002)) @@ -323,7 +326,7 @@ class FeatureFlagsDebugTest : SysuiTestCase() { } @Test - fun testSetFlagClearsCache() { + fun setFlag_ClearsCache() { val flag1 = addFlag(StringFlag(1, "flag1")) whenever(flagManager.readFlagValue(eq(1), any())).thenReturn("original") @@ -345,12 +348,30 @@ class FeatureFlagsDebugTest : SysuiTestCase() { } @Test - fun testRegisterCommand() { + fun serverSide_Overrides_MakesFalse() { + val flag = ReleasedFlag(100) + + serverFlagReader.setFlagValue(flag.id, false) + + assertThat(mFeatureFlagsDebug.isEnabled(flag)).isFalse() + } + + @Test + fun serverSide_Overrides_MakesTrue() { + val flag = UnreleasedFlag(100) + + serverFlagReader.setFlagValue(flag.id, true) + + assertThat(mFeatureFlagsDebug.isEnabled(flag)).isTrue() + } + + @Test + fun statusBarCommand_IsRegistered() { verify(commandRegistry).registerCommand(anyString(), any()) } @Test - fun testNoOpCommand() { + fun noOpCommand() { val cmd = captureCommand() cmd.execute(pw, ArrayList()) @@ -360,37 +381,73 @@ class FeatureFlagsDebugTest : SysuiTestCase() { } @Test - fun testReadFlagCommand() { - addFlag(BooleanFlag(1, false)) + fun readFlagCommand() { + addFlag(UnreleasedFlag(1)) val cmd = captureCommand() cmd.execute(pw, listOf("1")) verify(flagManager).readFlagValue(eq(1), any()) } @Test - fun testSetFlagCommand() { - addFlag(BooleanFlag(1, false)) + fun setFlagCommand() { + addFlag(UnreleasedFlag(1)) val cmd = captureCommand() cmd.execute(pw, listOf("1", "on")) verifyPutData(1, "{\"type\":\"boolean\",\"value\":true}") } @Test - fun testToggleFlagCommand() { - addFlag(BooleanFlag(1, true)) + fun toggleFlagCommand() { + addFlag(ReleasedFlag(1)) val cmd = captureCommand() cmd.execute(pw, listOf("1", "toggle")) verifyPutData(1, "{\"type\":\"boolean\",\"value\":false}", 2) } @Test - fun testEraseFlagCommand() { - addFlag(BooleanFlag(1, true)) + fun eraseFlagCommand() { + addFlag(ReleasedFlag(1)) val cmd = captureCommand() cmd.execute(pw, listOf("1", "erase")) verify(secureSettings).putStringForUser(eq("key-1"), eq(""), anyInt()) } + @Test + fun dumpFormat() { + val flag1 = ReleasedFlag(1) + val flag2 = ResourceBooleanFlag(2, 1002) + val flag3 = UnreleasedFlag(3) + val flag4 = StringFlag(4, "") + val flag5 = StringFlag(5, "flag5default") + val flag6 = ResourceStringFlag(6, 1006) + val flag7 = ResourceStringFlag(7, 1007) + + whenever(resources.getBoolean(1002)).thenReturn(true) + whenever(resources.getString(1006)).thenReturn("resource1006") + whenever(resources.getString(1007)).thenReturn("resource1007") + whenever(flagManager.readFlagValue(eq(7), eq(StringFlagSerializer))) + .thenReturn("override7") + + // WHEN the flags have been accessed + assertThat(mFeatureFlagsDebug.isEnabled(flag1)).isTrue() + assertThat(mFeatureFlagsDebug.isEnabled(flag2)).isTrue() + assertThat(mFeatureFlagsDebug.isEnabled(flag3)).isFalse() + assertThat(mFeatureFlagsDebug.getString(flag4)).isEmpty() + assertThat(mFeatureFlagsDebug.getString(flag5)).isEqualTo("flag5default") + assertThat(mFeatureFlagsDebug.getString(flag6)).isEqualTo("resource1006") + assertThat(mFeatureFlagsDebug.getString(flag7)).isEqualTo("override7") + + // THEN the dump contains the flags and the default values + val dump = dumpToString() + assertThat(dump).contains(" sysui_flag_1: true\n") + assertThat(dump).contains(" sysui_flag_2: true\n") + assertThat(dump).contains(" sysui_flag_3: false\n") + assertThat(dump).contains(" sysui_flag_4: [length=0] \"\"\n") + assertThat(dump).contains(" sysui_flag_5: [length=12] \"flag5default\"\n") + assertThat(dump).contains(" sysui_flag_6: [length=12] \"resource1006\"\n") + assertThat(dump).contains(" sysui_flag_7: [length=9] \"override7\"\n") + } + private fun verifyPutData(id: Int, data: String, numReads: Int = 1) { inOrder(flagManager, secureSettings).apply { verify(flagManager, times(numReads)).readFlagValue(eq(id), any>()) @@ -421,42 +478,6 @@ class FeatureFlagsDebugTest : SysuiTestCase() { return captor.value.invoke() } - @Test - fun testDump() { - val flag1 = BooleanFlag(1, true) - val flag2 = ResourceBooleanFlag(2, 1002) - val flag3 = BooleanFlag(3, false) - val flag4 = StringFlag(4, "") - val flag5 = StringFlag(5, "flag5default") - val flag6 = ResourceStringFlag(6, 1006) - val flag7 = ResourceStringFlag(7, 1007) - - whenever(resources.getBoolean(1002)).thenReturn(true) - whenever(resources.getString(1006)).thenReturn("resource1006") - whenever(resources.getString(1007)).thenReturn("resource1007") - whenever(flagManager.readFlagValue(eq(7), eq(StringFlagSerializer))) - .thenReturn("override7") - - // WHEN the flags have been accessed - assertThat(mFeatureFlagsDebug.isEnabled(flag1)).isTrue() - assertThat(mFeatureFlagsDebug.isEnabled(flag2)).isTrue() - assertThat(mFeatureFlagsDebug.isEnabled(flag3)).isFalse() - assertThat(mFeatureFlagsDebug.getString(flag4)).isEmpty() - assertThat(mFeatureFlagsDebug.getString(flag5)).isEqualTo("flag5default") - assertThat(mFeatureFlagsDebug.getString(flag6)).isEqualTo("resource1006") - assertThat(mFeatureFlagsDebug.getString(flag7)).isEqualTo("override7") - - // THEN the dump contains the flags and the default values - val dump = dumpToString() - assertThat(dump).contains(" sysui_flag_1: true\n") - assertThat(dump).contains(" sysui_flag_2: true\n") - assertThat(dump).contains(" sysui_flag_3: false\n") - assertThat(dump).contains(" sysui_flag_4: [length=0] \"\"\n") - assertThat(dump).contains(" sysui_flag_5: [length=12] \"flag5default\"\n") - assertThat(dump).contains(" sysui_flag_6: [length=12] \"resource1006\"\n") - assertThat(dump).contains(" sysui_flag_7: [length=9] \"override7\"\n") - } - private fun dumpToString(): String { val sw = StringWriter() val pw = PrintWriter(sw) @@ -464,4 +485,4 @@ class FeatureFlagsDebugTest : SysuiTestCase() { pw.flush() return sw.toString() } -} \ No newline at end of file +} diff --git a/packages/SystemUI/tests/src/com/android/systemui/flags/FeatureFlagsReleaseTest.kt b/packages/SystemUI/tests/src/com/android/systemui/flags/FeatureFlagsReleaseTest.kt index 6b683f456ea7c..e94b5202956dd 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/flags/FeatureFlagsReleaseTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/flags/FeatureFlagsReleaseTest.kt @@ -30,8 +30,8 @@ import org.junit.Test import org.mockito.Mock import org.mockito.Mockito.verify import org.mockito.Mockito.verifyNoMoreInteractions -import org.mockito.MockitoAnnotations import org.mockito.Mockito.`when` as whenever +import org.mockito.MockitoAnnotations /** * NOTE: This test is for the version of FeatureFlagManager in src-release, which should not allow @@ -44,13 +44,18 @@ class FeatureFlagsReleaseTest : SysuiTestCase() { @Mock private lateinit var mResources: Resources @Mock private lateinit var mSystemProperties: SystemPropertiesHelper @Mock private lateinit var mDumpManager: DumpManager + private val serverFlagReader = ServerFlagReaderFake() private val deviceConfig = DeviceConfigProxyFake() @Before fun setup() { MockitoAnnotations.initMocks(this) - mFeatureFlagsRelease = FeatureFlagsRelease(mResources, mSystemProperties, deviceConfig, + mFeatureFlagsRelease = FeatureFlagsRelease( + mResources, + mSystemProperties, + deviceConfig, + serverFlagReader, mDumpManager) } @@ -113,4 +118,22 @@ class FeatureFlagsReleaseTest : SysuiTestCase() { whenever(mSystemProperties.getBoolean(flagName, flagDefault)).thenReturn(flagDefault) assertThat(mFeatureFlagsRelease.isEnabled(flag)).isEqualTo(flagDefault) } -} \ No newline at end of file + + @Test + fun serverSide_OverridesReleased_MakesFalse() { + val flag = ReleasedFlag(100) + + serverFlagReader.setFlagValue(flag.id, false) + + assertThat(mFeatureFlagsRelease.isEnabled(flag)).isFalse() + } + + @Test + fun serverSide_OverridesUnreleased_Ignored() { + val flag = UnreleasedFlag(100) + + serverFlagReader.setFlagValue(flag.id, true) + + assertThat(mFeatureFlagsRelease.isEnabled(flag)).isFalse() + } +} diff --git a/packages/SystemUI/tests/src/com/android/systemui/flags/FlagManagerTest.kt b/packages/SystemUI/tests/src/com/android/systemui/flags/FlagManagerTest.kt index a2eca81b04ed1..17324a01502b1 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/flags/FlagManagerTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/flags/FlagManagerTest.kt @@ -26,6 +26,7 @@ import com.android.systemui.util.mockito.eq import com.android.systemui.util.mockito.mock import com.android.systemui.util.mockito.withArgCaptor import com.google.common.truth.Truth.assertThat +import java.util.function.Consumer import org.junit.Assert.assertThrows import org.junit.Before import org.junit.Test @@ -33,9 +34,8 @@ import org.mockito.Mock import org.mockito.Mockito.times import org.mockito.Mockito.verify import org.mockito.Mockito.verifyNoMoreInteractions -import org.mockito.MockitoAnnotations -import java.util.function.Consumer import org.mockito.Mockito.`when` as whenever +import org.mockito.MockitoAnnotations /** * NOTE: This test is for the version of FeatureFlagManager in src-release, which should not allow @@ -64,14 +64,14 @@ class FlagManagerTest : SysuiTestCase() { verifyNoMoreInteractions(mFlagSettingsHelper) // adding the first listener registers the observer - mFlagManager.addListener(BooleanFlag(1, true), listener1) + mFlagManager.addListener(ReleasedFlag(1), listener1) val observer = withArgCaptor { verify(mFlagSettingsHelper).registerContentObserver(any(), any(), capture()) } verifyNoMoreInteractions(mFlagSettingsHelper) // adding another listener does nothing - mFlagManager.addListener(BooleanFlag(2, true), listener2) + mFlagManager.addListener(ReleasedFlag(2), listener2) verifyNoMoreInteractions(mFlagSettingsHelper) // removing the original listener does nothing with second one still present @@ -89,7 +89,7 @@ class FlagManagerTest : SysuiTestCase() { val listener = mock() val clearCacheAction = mock>() mFlagManager.clearCacheAction = clearCacheAction - mFlagManager.addListener(BooleanFlag(1, true), listener) + mFlagManager.addListener(ReleasedFlag(1), listener) val observer = withArgCaptor { verify(mFlagSettingsHelper).registerContentObserver(any(), any(), capture()) } @@ -101,8 +101,8 @@ class FlagManagerTest : SysuiTestCase() { fun testObserverInvokesListeners() { val listener1 = mock() val listener10 = mock() - mFlagManager.addListener(BooleanFlag(1, true), listener1) - mFlagManager.addListener(BooleanFlag(10, true), listener10) + mFlagManager.addListener(ReleasedFlag(1), listener1) + mFlagManager.addListener(ReleasedFlag(10), listener10) val observer = withArgCaptor { verify(mFlagSettingsHelper).registerContentObserver(any(), any(), capture()) } @@ -127,8 +127,8 @@ class FlagManagerTest : SysuiTestCase() { fun testOnlySpecificFlagListenerIsInvoked() { val listener1 = mock() val listener10 = mock() - mFlagManager.addListener(BooleanFlag(1, true), listener1) - mFlagManager.addListener(BooleanFlag(10, true), listener10) + mFlagManager.addListener(ReleasedFlag(1), listener1) + mFlagManager.addListener(ReleasedFlag(10), listener10) mFlagManager.dispatchListenersAndMaybeRestart(1, null) val flagEvent1 = withArgCaptor { @@ -148,8 +148,8 @@ class FlagManagerTest : SysuiTestCase() { @Test fun testSameListenerCanBeUsedForMultipleFlags() { val listener = mock() - mFlagManager.addListener(BooleanFlag(1, true), listener) - mFlagManager.addListener(BooleanFlag(10, true), listener) + mFlagManager.addListener(ReleasedFlag(1), listener) + mFlagManager.addListener(ReleasedFlag(10), listener) mFlagManager.dispatchListenersAndMaybeRestart(1, null) val flagEvent1 = withArgCaptor { @@ -177,7 +177,7 @@ class FlagManagerTest : SysuiTestCase() { @Test fun testListenerCanSuppressRestart() { val restartAction = mock>() - mFlagManager.addListener(BooleanFlag(1, true)) { event -> + mFlagManager.addListener(ReleasedFlag(1)) { event -> event.requestNoRestart() } mFlagManager.dispatchListenersAndMaybeRestart(1, restartAction) @@ -188,7 +188,7 @@ class FlagManagerTest : SysuiTestCase() { @Test fun testListenerOnlySuppressesRestartForOwnFlag() { val restartAction = mock>() - mFlagManager.addListener(BooleanFlag(10, true)) { event -> + mFlagManager.addListener(ReleasedFlag(10)) { event -> event.requestNoRestart() } mFlagManager.dispatchListenersAndMaybeRestart(1, restartAction) @@ -199,10 +199,10 @@ class FlagManagerTest : SysuiTestCase() { @Test fun testRestartWhenNotAllListenersRequestSuppress() { val restartAction = mock>() - mFlagManager.addListener(BooleanFlag(10, true)) { event -> + mFlagManager.addListener(ReleasedFlag(10)) { event -> event.requestNoRestart() } - mFlagManager.addListener(BooleanFlag(10, true)) { + mFlagManager.addListener(ReleasedFlag(10)) { // do not request } mFlagManager.dispatchListenersAndMaybeRestart(1, restartAction) @@ -295,4 +295,4 @@ class FlagManagerTest : SysuiTestCase() { assertThat(StringFlagSerializer.toSettingsData("foo")) .isEqualTo("{\"type\":\"string\",\"value\":\"foo\"}") } -} \ No newline at end of file +} diff --git a/packages/SystemUI/tests/src/com/android/systemui/flags/FlagsTest.java b/packages/SystemUI/tests/src/com/android/systemui/flags/FlagsTest.java index 25c302885e078..250cc48fece3e 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/flags/FlagsTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/flags/FlagsTest.java @@ -107,11 +107,11 @@ public class FlagsTest extends SysuiTestCase { } private static class DuplicateFlagContainer { - public static final BooleanFlag A_FLAG = new BooleanFlag(0); - public static final BooleanFlag B_FLAG = new BooleanFlag(0); + public static final BooleanFlag A_FLAG = new UnreleasedFlag(0); + public static final BooleanFlag B_FLAG = new UnreleasedFlag(0); public static final StringFlag C_FLAG = new StringFlag(0); - public static final BooleanFlag D_FLAG = new BooleanFlag(1); + public static final BooleanFlag D_FLAG = new UnreleasedFlag(1); public static final DoubleFlag E_FLAG = new DoubleFlag(3); public static final DoubleFlag F_FLAG = new DoubleFlag(3);