From cd1af6a4d14ee3f9fa4948ae229981a5d7bfc0aa Mon Sep 17 00:00:00 2001 From: Dave Mankoff Date: Thu, 12 Jan 2023 17:57:26 +0000 Subject: [PATCH] Step 1 of Removing Ids from Flags. With this change, we start relying more directly on the string names that are now associated with flags. The ids persist so that we can look up existing overrides and push them into the new system. After a couple of weeks, the plan will be to remove the ids entirely. Bug: 265188950 Test: manually built before and after cl to ensure overrides persist. Change-Id: I0faac671b43a0d24262e78ccdb4e23e44f73eeea --- .../src/com/android/systemui/flags/Flag.kt | 43 +--- .../android/systemui/flags/FlagListenable.kt | 2 +- .../com/android/systemui/flags/FlagManager.kt | 53 +++-- .../systemui/flags/FlagSettingsHelper.kt | 5 +- .../android/systemui/flags/FlagsFactory.kt | 15 +- .../com/android/systemui/ChooserSelector.kt | 2 +- .../systemui/flags/FeatureFlagsDebug.java | 213 +++++++++++------- .../systemui/flags/FeatureFlagsRelease.java | 90 +++++--- .../android/systemui/flags/FlagCommand.java | 54 ++--- .../systemui/flags/FlagsCommonModule.kt | 4 +- .../systemui/flags/ServerFlagReader.kt | 3 +- .../android/systemui/ChooserSelectorTest.kt | 8 +- .../systemui/flags/FakeFeatureFlagsTest.kt | 28 +-- .../systemui/flags/FeatureFlagsDebugTest.kt | 140 ++++++------ .../systemui/flags/FeatureFlagsReleaseTest.kt | 5 +- .../android/systemui/flags/FlagCommandTest.kt | 26 +-- .../android/systemui/flags/FlagManagerTest.kt | 56 ++--- .../systemui/flags/FakeFeatureFlags.kt | 8 +- 18 files changed, 395 insertions(+), 360 deletions(-) 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 196f7f05d20d7..c9a25b067b948 100644 --- a/packages/SystemUI/shared/src/com/android/systemui/flags/Flag.kt +++ b/packages/SystemUI/shared/src/com/android/systemui/flags/Flag.kt @@ -47,10 +47,6 @@ interface ResourceFlag : Flag { val resourceId: Int } -interface DeviceConfigFlag : Flag { - val default: T -} - interface SysPropFlag : Flag { val default: T } @@ -80,8 +76,8 @@ abstract class BooleanFlag constructor( private constructor(parcel: Parcel) : this( id = parcel.readInt(), - name = parcel.readString(), - namespace = parcel.readString(), + name = parcel.readString() ?: "", + namespace = parcel.readString() ?: "", default = parcel.readBoolean(), teamfood = parcel.readBoolean(), overridden = parcel.readBoolean() @@ -136,21 +132,6 @@ data class ResourceBooleanFlag constructor( 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 constructor( - override val id: Int, - override val name: String, - override val namespace: String, - override val default: Boolean = false, - override val teamfood: Boolean = false -) : DeviceConfigFlag - /** * A Flag that can reads its overrides from System Properties. * @@ -186,8 +167,8 @@ data class StringFlag constructor( private constructor(parcel: Parcel) : this( id = parcel.readInt(), - name = parcel.readString(), - namespace = parcel.readString(), + name = parcel.readString() ?: "", + namespace = parcel.readString() ?: "", default = parcel.readString() ?: "" ) @@ -226,8 +207,8 @@ data class IntFlag constructor( private constructor(parcel: Parcel) : this( id = parcel.readInt(), - name = parcel.readString(), - namespace = parcel.readString(), + name = parcel.readString() ?: "", + namespace = parcel.readString() ?: "", default = parcel.readInt() ) @@ -266,8 +247,8 @@ data class LongFlag constructor( private constructor(parcel: Parcel) : this( id = parcel.readInt(), - name = parcel.readString(), - namespace = parcel.readString(), + name = parcel.readString() ?: "", + namespace = parcel.readString() ?: "", default = parcel.readLong() ) @@ -298,8 +279,8 @@ data class FloatFlag constructor( private constructor(parcel: Parcel) : this( id = parcel.readInt(), - name = parcel.readString(), - namespace = parcel.readString(), + name = parcel.readString() ?: "", + namespace = parcel.readString() ?: "", default = parcel.readFloat() ) @@ -338,8 +319,8 @@ data class DoubleFlag constructor( private constructor(parcel: Parcel) : this( id = parcel.readInt(), - name = parcel.readString(), - namespace = parcel.readString(), + name = parcel.readString() ?: "", + namespace = parcel.readString() ?: "", default = parcel.readDouble() ) diff --git a/packages/SystemUI/shared/src/com/android/systemui/flags/FlagListenable.kt b/packages/SystemUI/shared/src/com/android/systemui/flags/FlagListenable.kt index 195ba46515cd2..72a4fabf1f0dd 100644 --- a/packages/SystemUI/shared/src/com/android/systemui/flags/FlagListenable.kt +++ b/packages/SystemUI/shared/src/com/android/systemui/flags/FlagListenable.kt @@ -34,7 +34,7 @@ interface FlagListenable { /** An event representing the change */ interface FlagEvent { /** the id of the flag which changed */ - val flagId: Int + val flagName: String /** if all listeners alerted invoke this method, the restart will be skipped */ fun requestNoRestart() } diff --git a/packages/SystemUI/shared/src/com/android/systemui/flags/FlagManager.kt b/packages/SystemUI/shared/src/com/android/systemui/flags/FlagManager.kt index d85292a90d633..da1641c326def 100644 --- a/packages/SystemUI/shared/src/com/android/systemui/flags/FlagManager.kt +++ b/packages/SystemUI/shared/src/com/android/systemui/flags/FlagManager.kt @@ -39,7 +39,7 @@ class FlagManager constructor( const val ACTION_GET_FLAGS = "com.android.systemui.action.GET_FLAGS" const val FLAGS_PERMISSION = "com.android.systemui.permission.FLAGS" const val ACTION_SYSUI_STARTED = "com.android.systemui.STARTED" - const val EXTRA_ID = "id" + const val EXTRA_NAME = "name" const val EXTRA_VALUE = "value" const val EXTRA_FLAGS = "flags" private const val SETTINGS_PREFIX = "systemui/flags" @@ -56,7 +56,7 @@ class FlagManager constructor( * that the restart be suppressed */ var onSettingsChangedAction: Consumer? = null - var clearCacheAction: Consumer? = null + var clearCacheAction: Consumer? = null private val listeners: MutableSet = mutableSetOf() private val settingsObserver: ContentObserver = SettingsObserver() @@ -96,35 +96,42 @@ class FlagManager constructor( * Returns the stored value or null if not set. * This API is used by TheFlippinApp. */ - fun isEnabled(id: Int): Boolean? = readFlagValue(id, BooleanFlagSerializer) + fun isEnabled(name: String): Boolean? = readFlagValue(name, BooleanFlagSerializer) /** * Sets the value of a boolean flag. * This API is used by TheFlippinApp. */ - fun setFlagValue(id: Int, enabled: Boolean) { - val intent = createIntent(id) + fun setFlagValue(name: String, enabled: Boolean) { + val intent = createIntent(name) intent.putExtra(EXTRA_VALUE, enabled) context.sendBroadcast(intent) } - fun eraseFlag(id: Int) { - val intent = createIntent(id) + fun eraseFlag(name: String) { + val intent = createIntent(name) context.sendBroadcast(intent) } /** Returns the stored value or null if not set. */ + // TODO(b/265188950): Remove method this once ids are fully deprecated. fun readFlagValue(id: Int, serializer: FlagSerializer): T? { - val data = settings.getString(idToSettingsKey(id)) + val data = settings.getStringFromSecure(idToSettingsKey(id)) + return serializer.fromSettingsData(data) + } + + /** Returns the stored value or null if not set. */ + fun readFlagValue(name: String, serializer: FlagSerializer): T? { + val data = settings.getString(nameToSettingsKey(name)) return serializer.fromSettingsData(data) } override fun addListener(flag: Flag<*>, listener: FlagListenable.Listener) { synchronized(listeners) { val registerNeeded = listeners.isEmpty() - listeners.add(PerFlagListener(flag.id, listener)) + listeners.add(PerFlagListener(flag.name, listener)) if (registerNeeded) { settings.registerContentObserver(SETTINGS_PREFIX, true, settingsObserver) } @@ -143,38 +150,38 @@ class FlagManager constructor( } } - private fun createIntent(id: Int): Intent { + private fun createIntent(name: String): Intent { val intent = Intent(ACTION_SET_FLAG) intent.setPackage(RECEIVING_PACKAGE) - intent.putExtra(EXTRA_ID, id) + intent.putExtra(EXTRA_NAME, name) return intent } + // TODO(b/265188950): Remove method this once ids are fully deprecated. fun idToSettingsKey(id: Int): String { return "$SETTINGS_PREFIX/$id" } + fun nameToSettingsKey(name: String): String { + return "$SETTINGS_PREFIX/$name" + } + inner class SettingsObserver : ContentObserver(handler) { override fun onChange(selfChange: Boolean, uri: Uri?) { if (uri == null) { return } val parts = uri.pathSegments - val idStr = parts[parts.size - 1] - val id = try { - idStr.toInt() - } catch (e: NumberFormatException) { - return - } - clearCacheAction?.accept(id) - dispatchListenersAndMaybeRestart(id, onSettingsChangedAction) + val name = parts[parts.size - 1] + clearCacheAction?.accept(name) + dispatchListenersAndMaybeRestart(name, onSettingsChangedAction) } } - fun dispatchListenersAndMaybeRestart(id: Int, restartAction: Consumer?) { + fun dispatchListenersAndMaybeRestart(name: String, restartAction: Consumer?) { val filteredListeners: List = synchronized(listeners) { - listeners.mapNotNull { if (it.id == id) it.listener else null } + listeners.mapNotNull { if (it.name == name) it.listener else null } } // If there are no listeners, there's nothing to dispatch to, and nothing to suppress it. if (filteredListeners.isEmpty()) { @@ -185,7 +192,7 @@ class FlagManager constructor( val suppressRestartList: List = filteredListeners.map { listener -> var didRequestNoRestart = false val event = object : FlagListenable.FlagEvent { - override val flagId = id + override val flagName = name override fun requestNoRestart() { didRequestNoRestart = true } @@ -198,7 +205,7 @@ class FlagManager constructor( restartAction?.accept(suppressRestart) } - private data class PerFlagListener(val id: Int, val listener: FlagListenable.Listener) + private data class PerFlagListener(val name: String, val listener: FlagListenable.Listener) } class NoFlagResultsException : Exception( diff --git a/packages/SystemUI/shared/src/com/android/systemui/flags/FlagSettingsHelper.kt b/packages/SystemUI/shared/src/com/android/systemui/flags/FlagSettingsHelper.kt index 742bb0b6f954d..6beb8518ab67f 100644 --- a/packages/SystemUI/shared/src/com/android/systemui/flags/FlagSettingsHelper.kt +++ b/packages/SystemUI/shared/src/com/android/systemui/flags/FlagSettingsHelper.kt @@ -22,7 +22,10 @@ import android.provider.Settings class FlagSettingsHelper(private val contentResolver: ContentResolver) { - fun getString(key: String): String? = Settings.Secure.getString(contentResolver, key) + // TODO(b/265188950): Remove method this once ids are fully deprecated. + fun getStringFromSecure(key: String): String? = Settings.Secure.getString(contentResolver, key) + + fun getString(key: String): String? = Settings.Global.getString(contentResolver, key) fun registerContentObserver( name: String, diff --git a/packages/SystemUI/src-debug/com/android/systemui/flags/FlagsFactory.kt b/packages/SystemUI/src-debug/com/android/systemui/flags/FlagsFactory.kt index 05372fec7211d..31234cf2ab533 100644 --- a/packages/SystemUI/src-debug/com/android/systemui/flags/FlagsFactory.kt +++ b/packages/SystemUI/src-debug/com/android/systemui/flags/FlagsFactory.kt @@ -35,7 +35,7 @@ object FlagsFactory { teamfood: Boolean = false ): UnreleasedFlag { val flag = UnreleasedFlag(id = id, name = name, namespace = namespace, teamfood = teamfood) - FlagsFactory.checkForDupesAndAdd(flag) + checkForDupesAndAdd(flag) return flag } @@ -46,7 +46,7 @@ object FlagsFactory { teamfood: Boolean = false ): ReleasedFlag { val flag = ReleasedFlag(id = id, name = name, namespace = namespace, teamfood = teamfood) - FlagsFactory.checkForDupesAndAdd(flag) + checkForDupesAndAdd(flag) return flag } @@ -65,7 +65,7 @@ object FlagsFactory { resourceId = resourceId, teamfood = teamfood ) - FlagsFactory.checkForDupesAndAdd(flag) + checkForDupesAndAdd(flag) return flag } @@ -77,18 +77,13 @@ object FlagsFactory { ): SysPropBooleanFlag { val flag = SysPropBooleanFlag(id = id, name = name, namespace = "systemui", default = default) - FlagsFactory.checkForDupesAndAdd(flag) + checkForDupesAndAdd(flag) return flag } private fun checkForDupesAndAdd(flag: Flag<*>) { if (flagMap.containsKey(flag.name)) { - throw IllegalArgumentException("Name {flag.name} is already registered") - } - flagMap.forEach { - if (it.value.id == flag.id) { - throw IllegalArgumentException("Name {flag.id} is already registered") - } + throw IllegalArgumentException("Name {$flag.name} is already registered") } flagMap[flag.name] = flag } diff --git a/packages/SystemUI/src/com/android/systemui/ChooserSelector.kt b/packages/SystemUI/src/com/android/systemui/ChooserSelector.kt index 9ac45b3c77cc9..227f0ace33dd7 100644 --- a/packages/SystemUI/src/com/android/systemui/ChooserSelector.kt +++ b/packages/SystemUI/src/com/android/systemui/ChooserSelector.kt @@ -34,7 +34,7 @@ class ChooserSelector @Inject constructor( override fun start() { coroutineScope.launch { val listener = FlagListenable.Listener { event -> - if (event.flagId == Flags.CHOOSER_UNBUNDLED.id) { + if (event.flagName == Flags.CHOOSER_UNBUNDLED.name) { launch { updateUnbundledChooserEnabled() } event.requestNoRestart() } diff --git a/packages/SystemUI/src/com/android/systemui/flags/FeatureFlagsDebug.java b/packages/SystemUI/src/com/android/systemui/flags/FeatureFlagsDebug.java index d1a14a1ba65ff..4bac69776319d 100644 --- a/packages/SystemUI/src/com/android/systemui/flags/FeatureFlagsDebug.java +++ b/packages/SystemUI/src/com/android/systemui/flags/FeatureFlagsDebug.java @@ -19,7 +19,7 @@ package com.android.systemui.flags; import static com.android.systemui.flags.FlagManager.ACTION_GET_FLAGS; import static com.android.systemui.flags.FlagManager.ACTION_SET_FLAG; import static com.android.systemui.flags.FlagManager.EXTRA_FLAGS; -import static com.android.systemui.flags.FlagManager.EXTRA_ID; +import static com.android.systemui.flags.FlagManager.EXTRA_NAME; import static com.android.systemui.flags.FlagManager.EXTRA_VALUE; import static com.android.systemui.flags.FlagsCommonModule.ALL_FLAGS; @@ -39,6 +39,7 @@ import androidx.annotation.Nullable; import com.android.systemui.dagger.SysUISingleton; import com.android.systemui.dagger.qualifiers.Main; +import com.android.systemui.util.settings.GlobalSettings; import com.android.systemui.util.settings.SecureSettings; import org.jetbrains.annotations.NotNull; @@ -72,14 +73,15 @@ public class FeatureFlagsDebug implements FeatureFlags { private final FlagManager mFlagManager; private final Context mContext; + private final GlobalSettings mGlobalSettings; private final SecureSettings mSecureSettings; private final Resources mResources; private final SystemPropertiesHelper mSystemProperties; private final ServerFlagReader mServerFlagReader; - private final Map> mAllFlags; - private final Map mBooleanFlagCache = new TreeMap<>(); - private final Map mStringFlagCache = new TreeMap<>(); - private final Map mIntFlagCache = new TreeMap<>(); + private final Map> mAllFlags; + private final Map mBooleanFlagCache = new TreeMap<>(); + private final Map mStringFlagCache = new TreeMap<>(); + private final Map mIntFlagCache = new TreeMap<>(); private final Restarter mRestarter; private final ServerFlagReader.ChangeListener mOnPropertiesChanged = @@ -94,14 +96,16 @@ public class FeatureFlagsDebug implements FeatureFlags { public FeatureFlagsDebug( FlagManager flagManager, Context context, + GlobalSettings globalSettings, SecureSettings secureSettings, SystemPropertiesHelper systemProperties, @Main Resources resources, ServerFlagReader serverFlagReader, - @Named(ALL_FLAGS) Map> allFlags, + @Named(ALL_FLAGS) Map> allFlags, Restarter restarter) { mFlagManager = flagManager; mContext = context; + mGlobalSettings = globalSettings; mSecureSettings = secureSettings; mResources = resources; mSystemProperties = systemProperties; @@ -133,96 +137,103 @@ public class FeatureFlagsDebug implements FeatureFlags { } private boolean isEnabledInternal(@NotNull BooleanFlag flag) { - int id = flag.getId(); - if (!mBooleanFlagCache.containsKey(id)) { - mBooleanFlagCache.put(id, + String name = flag.getName(); + if (!mBooleanFlagCache.containsKey(name)) { + mBooleanFlagCache.put(name, readBooleanFlagInternal(flag, flag.getDefault())); } - return mBooleanFlagCache.get(id); + return mBooleanFlagCache.get(name); } @Override public boolean isEnabled(@NonNull ResourceBooleanFlag flag) { - int id = flag.getId(); - if (!mBooleanFlagCache.containsKey(id)) { - mBooleanFlagCache.put(id, + String name = flag.getName(); + if (!mBooleanFlagCache.containsKey(name)) { + mBooleanFlagCache.put(name, readBooleanFlagInternal(flag, mResources.getBoolean(flag.getResourceId()))); } - return mBooleanFlagCache.get(id); + return mBooleanFlagCache.get(name); } @Override public boolean isEnabled(@NonNull SysPropBooleanFlag flag) { - int id = flag.getId(); - if (!mBooleanFlagCache.containsKey(id)) { + String name = flag.getName(); + if (!mBooleanFlagCache.containsKey(name)) { // Use #readFlagValue to get the default. That will allow it to fall through to // teamfood if need be. mBooleanFlagCache.put( - id, + name, mSystemProperties.getBoolean( flag.getName(), readBooleanFlagInternal(flag, flag.getDefault()))); } - return mBooleanFlagCache.get(id); + return mBooleanFlagCache.get(name); } @NonNull @Override public String getString(@NonNull StringFlag flag) { - int id = flag.getId(); - if (!mStringFlagCache.containsKey(id)) { - mStringFlagCache.put(id, - readFlagValueInternal(id, flag.getDefault(), StringFlagSerializer.INSTANCE)); + String name = flag.getName(); + if (!mStringFlagCache.containsKey(name)) { + mStringFlagCache.put(name, + readFlagValueInternal( + flag.getId(), name, flag.getDefault(), StringFlagSerializer.INSTANCE)); } - return mStringFlagCache.get(id); + return mStringFlagCache.get(name); } @NonNull @Override public String getString(@NonNull ResourceStringFlag flag) { - int id = flag.getId(); - if (!mStringFlagCache.containsKey(id)) { - mStringFlagCache.put(id, - readFlagValueInternal(id, mResources.getString(flag.getResourceId()), + String name = flag.getName(); + if (!mStringFlagCache.containsKey(name)) { + mStringFlagCache.put(name, + readFlagValueInternal( + flag.getId(), name, mResources.getString(flag.getResourceId()), StringFlagSerializer.INSTANCE)); } - return mStringFlagCache.get(id); + return mStringFlagCache.get(name); } @NonNull @Override public int getInt(@NonNull IntFlag flag) { - int id = flag.getId(); - if (!mIntFlagCache.containsKey(id)) { - mIntFlagCache.put(id, - readFlagValueInternal(id, flag.getDefault(), IntFlagSerializer.INSTANCE)); + String name = flag.getName(); + if (!mIntFlagCache.containsKey(name)) { + mIntFlagCache.put(name, + readFlagValueInternal( + flag.getId(), name, flag.getDefault(), IntFlagSerializer.INSTANCE)); } - return mIntFlagCache.get(id); + return mIntFlagCache.get(name); } @NonNull @Override public int getInt(@NonNull ResourceIntFlag flag) { - int id = flag.getId(); - if (!mIntFlagCache.containsKey(id)) { - mIntFlagCache.put(id, - readFlagValueInternal(id, mResources.getInteger(flag.getResourceId()), + String name = flag.getName(); + if (!mIntFlagCache.containsKey(name)) { + mIntFlagCache.put(name, + readFlagValueInternal( + flag.getId(), name, mResources.getInteger(flag.getResourceId()), IntFlagSerializer.INSTANCE)); } - return mIntFlagCache.get(id); + return mIntFlagCache.get(name); } /** Specific override for Boolean flags that checks against the teamfood list.*/ private boolean readBooleanFlagInternal(Flag flag, boolean defaultValue) { - Boolean result = readBooleanFlagOverride(flag.getId()); + Boolean result = readBooleanFlagOverride(flag.getName()); + if (result == null) { + result = readBooleanFlagOverride(flag.getId()); + } boolean hasServerOverride = mServerFlagReader.hasOverride( flag.getNamespace(), flag.getName()); @@ -231,7 +242,7 @@ public class FeatureFlagsDebug implements FeatureFlags { if (!hasServerOverride && !defaultValue && result == null - && flag.getId() != Flags.TEAMFOOD.getId() + && !flag.getName().equals(Flags.TEAMFOOD.getName()) && flag.getTeamfood()) { return isEnabled(Flags.TEAMFOOD); } @@ -244,16 +255,31 @@ public class FeatureFlagsDebug implements FeatureFlags { return readFlagValueInternal(id, BooleanFlagSerializer.INSTANCE); } + private Boolean readBooleanFlagOverride(String name) { + return readFlagValueInternal(name, BooleanFlagSerializer.INSTANCE); + } + + // TODO(b/265188950): Remove id from this method once ids are fully deprecated. @NonNull private T readFlagValueInternal( - int id, @NonNull T defaultValue, FlagSerializer serializer) { + int id, String name, @NonNull T defaultValue, FlagSerializer serializer) { requireNonNull(defaultValue, "defaultValue"); - T result = readFlagValueInternal(id, serializer); - return result == null ? defaultValue : result; + T resultForName = readFlagValueInternal(name, serializer); + if (resultForName == null) { + T resultForId = readFlagValueInternal(id, serializer); + if (resultForId == null) { + return defaultValue; + } else { + setFlagValue(name, resultForId, serializer); + return resultForId; + } + } + return resultForName; } /** Returns the stored value or null if not set. */ + // TODO(b/265188950): Remove method this once ids are fully deprecated. @Nullable private T readFlagValueInternal(int id, FlagSerializer serializer) { try { @@ -264,51 +290,71 @@ public class FeatureFlagsDebug implements FeatureFlags { return null; } - private void setFlagValue(int id, @NonNull T value, FlagSerializer serializer) { + /** Returns the stored value or null if not set. */ + @Nullable + private T readFlagValueInternal(String name, FlagSerializer serializer) { + try { + return mFlagManager.readFlagValue(name, serializer); + } catch (Exception e) { + eraseInternal(name); + } + return null; + } + + private void setFlagValue(String name, @NonNull T value, FlagSerializer serializer) { requireNonNull(value, "Cannot set a null value"); - T currentValue = readFlagValueInternal(id, serializer); + T currentValue = readFlagValueInternal(name, serializer); if (Objects.equals(currentValue, value)) { - Log.i(TAG, "Flag id " + id + " is already " + value); + Log.i(TAG, "Flag id " + name + " is already " + value); return; } final String data = serializer.toSettingsData(value); if (data == null) { - Log.w(TAG, "Failed to set id " + id + " to " + value); + Log.w(TAG, "Failed to set id " + name + " to " + value); return; } - mSecureSettings.putStringForUser(mFlagManager.idToSettingsKey(id), data, + mGlobalSettings.putStringForUser(mFlagManager.nameToSettingsKey(name), data, UserHandle.USER_CURRENT); - Log.i(TAG, "Set id " + id + " to " + value); - removeFromCache(id); - mFlagManager.dispatchListenersAndMaybeRestart(id, this::restartSystemUI); + Log.i(TAG, "Set id " + name + " to " + value); + removeFromCache(name); + mFlagManager.dispatchListenersAndMaybeRestart(name, this::restartSystemUI); } void eraseFlag(Flag flag) { if (flag instanceof SysPropFlag) { mSystemProperties.erase(((SysPropFlag) flag).getName()); - dispatchListenersAndMaybeRestart(flag.getId(), this::restartAndroid); + dispatchListenersAndMaybeRestart(flag.getName(), this::restartAndroid); } else { - eraseFlag(flag.getId()); + eraseFlag(flag.getName()); } } /** Erase a flag's overridden value if there is one. */ - private void eraseFlag(int id) { - eraseInternal(id); - removeFromCache(id); - dispatchListenersAndMaybeRestart(id, this::restartSystemUI); + private void eraseFlag(String name) { + eraseInternal(name); + removeFromCache(name); + dispatchListenersAndMaybeRestart(name, this::restartSystemUI); } - private void dispatchListenersAndMaybeRestart(int id, Consumer restartAction) { - mFlagManager.dispatchListenersAndMaybeRestart(id, restartAction); + private void dispatchListenersAndMaybeRestart(String name, Consumer restartAction) { + mFlagManager.dispatchListenersAndMaybeRestart(name, restartAction); } - /** Works just like {@link #eraseFlag(int)} except that it doesn't restart SystemUI. */ + /** Works just like {@link #eraseFlag(String)} except that it doesn't restart SystemUI. */ + // TODO(b/265188950): Remove method this once ids are fully deprecated. private void eraseInternal(int id) { - // We can't actually "erase" things from sysprops, but we can set them to empty! - mSecureSettings.putStringForUser(mFlagManager.idToSettingsKey(id), "", + // We can't actually "erase" things from settings, but we can set them to empty! + mGlobalSettings.putStringForUser(mFlagManager.idToSettingsKey(id), "", UserHandle.USER_CURRENT); - Log.i(TAG, "Erase id " + id); + Log.i(TAG, "Erase name " + id); + } + + /** Works just like {@link #eraseFlag(String)} except that it doesn't restart SystemUI. */ + private void eraseInternal(String name) { + // We can't actually "erase" things from settings, but we can set them to empty! + mGlobalSettings.putStringForUser(mFlagManager.nameToSettingsKey(name), "", + UserHandle.USER_CURRENT); + Log.i(TAG, "Erase name " + name); } @Override @@ -339,13 +385,13 @@ public class FeatureFlagsDebug implements FeatureFlags { void setBooleanFlagInternal(Flag flag, boolean value) { if (flag instanceof BooleanFlag) { - setFlagValue(flag.getId(), value, BooleanFlagSerializer.INSTANCE); + setFlagValue(flag.getName(), value, BooleanFlagSerializer.INSTANCE); } else if (flag instanceof ResourceBooleanFlag) { - setFlagValue(flag.getId(), value, BooleanFlagSerializer.INSTANCE); + setFlagValue(flag.getName(), value, BooleanFlagSerializer.INSTANCE); } else if (flag instanceof SysPropBooleanFlag) { // Store SysProp flags in SystemProperties where they can read by outside parties. mSystemProperties.setBoolean(((SysPropBooleanFlag) flag).getName(), value); - dispatchListenersAndMaybeRestart(flag.getId(), + dispatchListenersAndMaybeRestart(flag.getName(), FeatureFlagsDebug.this::restartAndroid); } else { throw new IllegalArgumentException("Unknown flag type"); @@ -354,9 +400,9 @@ public class FeatureFlagsDebug implements FeatureFlags { void setStringFlagInternal(Flag flag, String value) { if (flag instanceof StringFlag) { - setFlagValue(flag.getId(), value, StringFlagSerializer.INSTANCE); + setFlagValue(flag.getName(), value, StringFlagSerializer.INSTANCE); } else if (flag instanceof ResourceStringFlag) { - setFlagValue(flag.getId(), value, StringFlagSerializer.INSTANCE); + setFlagValue(flag.getName(), value, StringFlagSerializer.INSTANCE); } else { throw new IllegalArgumentException("Unknown flag type"); } @@ -364,9 +410,9 @@ public class FeatureFlagsDebug implements FeatureFlags { void setIntFlagInternal(Flag flag, int value) { if (flag instanceof IntFlag) { - setFlagValue(flag.getId(), value, IntFlagSerializer.INSTANCE); + setFlagValue(flag.getName(), value, IntFlagSerializer.INSTANCE); } else if (flag instanceof ResourceIntFlag) { - setFlagValue(flag.getId(), value, IntFlagSerializer.INSTANCE); + setFlagValue(flag.getName(), value, IntFlagSerializer.INSTANCE); } else { throw new IllegalArgumentException("Unknown flag type"); } @@ -405,17 +451,17 @@ public class FeatureFlagsDebug implements FeatureFlags { Log.w(TAG, "No extras"); return; } - int id = extras.getInt(EXTRA_ID); - if (id <= 0) { - Log.w(TAG, "ID not set or less than or equal to 0: " + id); + String name = extras.getString(EXTRA_NAME); + if (name == null || name.isEmpty()) { + Log.w(TAG, "NAME not set or is empty: " + name); return; } - if (!mAllFlags.containsKey(id)) { - Log.w(TAG, "Tried to set unknown id: " + id); + if (!mAllFlags.containsKey(name)) { + Log.w(TAG, "Tried to set unknown name: " + name); return; } - Flag flag = mAllFlags.get(id); + Flag flag = mAllFlags.get(name); if (!extras.containsKey(EXTRA_VALUE)) { eraseFlag(flag); @@ -452,13 +498,16 @@ public class FeatureFlagsDebug implements FeatureFlags { if (f instanceof ReleasedFlag) { enabled = isEnabled((ReleasedFlag) f); - overridden = readBooleanFlagOverride(f.getId()) != null; + overridden = readBooleanFlagOverride(f.getName()) != null + || readBooleanFlagOverride(f.getId()) != null; } else if (f instanceof UnreleasedFlag) { enabled = isEnabled((UnreleasedFlag) f); - overridden = readBooleanFlagOverride(f.getId()) != null; + overridden = readBooleanFlagOverride(f.getName()) != null + || readBooleanFlagOverride(f.getId()) != null; } else if (f instanceof ResourceBooleanFlag) { enabled = isEnabled((ResourceBooleanFlag) f); - overridden = readBooleanFlagOverride(f.getId()) != null; + overridden = readBooleanFlagOverride(f.getName()) != null + || readBooleanFlagOverride(f.getId()) != null; } else if (f instanceof SysPropBooleanFlag) { // TODO(b/223379190): Teamfood not supported for sysprop flags yet. enabled = isEnabled((SysPropBooleanFlag) f); @@ -480,9 +529,9 @@ public class FeatureFlagsDebug implements FeatureFlags { } }; - private void removeFromCache(int id) { - mBooleanFlagCache.remove(id); - mStringFlagCache.remove(id); + private void removeFromCache(String name) { + mBooleanFlagCache.remove(name); + mStringFlagCache.remove(name); } @Override diff --git a/packages/SystemUI/src/com/android/systemui/flags/FeatureFlagsRelease.java b/packages/SystemUI/src/com/android/systemui/flags/FeatureFlagsRelease.java index 8bddacc4e32ce..7e1423706fc9d 100644 --- a/packages/SystemUI/src/com/android/systemui/flags/FeatureFlagsRelease.java +++ b/packages/SystemUI/src/com/android/systemui/flags/FeatureFlagsRelease.java @@ -21,18 +21,16 @@ import static com.android.systemui.flags.FlagsCommonModule.ALL_FLAGS; import static java.util.Objects.requireNonNull; import android.content.res.Resources; -import android.util.SparseArray; -import android.util.SparseBooleanArray; import androidx.annotation.NonNull; import com.android.systemui.dagger.SysUISingleton; import com.android.systemui.dagger.qualifiers.Main; -import com.android.systemui.util.DeviceConfigProxy; import org.jetbrains.annotations.NotNull; import java.io.PrintWriter; +import java.util.HashMap; import java.util.Map; import javax.inject.Inject; @@ -50,12 +48,11 @@ public class FeatureFlagsRelease implements FeatureFlags { private final Resources mResources; private final SystemPropertiesHelper mSystemProperties; - private final DeviceConfigProxy mDeviceConfigProxy; private final ServerFlagReader mServerFlagReader; private final Restarter mRestarter; - private final Map> mAllFlags; - SparseBooleanArray mBooleanCache = new SparseBooleanArray(); - SparseArray mStringCache = new SparseArray<>(); + private final Map> mAllFlags; + private final Map mBooleanCache = new HashMap<>(); + private final Map mStringCache = new HashMap<>(); private final ServerFlagReader.ChangeListener mOnPropertiesChanged = new ServerFlagReader.ChangeListener() { @@ -69,13 +66,11 @@ public class FeatureFlagsRelease implements FeatureFlags { public FeatureFlagsRelease( @Main Resources resources, SystemPropertiesHelper systemProperties, - DeviceConfigProxy deviceConfigProxy, ServerFlagReader serverFlagReader, - @Named(ALL_FLAGS) Map> allFlags, + @Named(ALL_FLAGS) Map> allFlags, Restarter restarter) { mResources = resources; mSystemProperties = systemProperties; - mDeviceConfigProxy = deviceConfigProxy; mServerFlagReader = serverFlagReader; mAllFlags = allFlags; mRestarter = restarter; @@ -106,50 +101,48 @@ public class FeatureFlagsRelease implements FeatureFlags { @Override public boolean isEnabled(ResourceBooleanFlag flag) { - int cacheIndex = mBooleanCache.indexOfKey(flag.getId()); - if (cacheIndex < 0) { - return isEnabled(flag.getId(), mResources.getBoolean(flag.getResourceId())); + if (!mBooleanCache.containsKey(flag.getName())) { + return isEnabled(flag.getName(), mResources.getBoolean(flag.getResourceId())); } - return mBooleanCache.valueAt(cacheIndex); + return mBooleanCache.get(flag.getName()); } @Override public boolean isEnabled(SysPropBooleanFlag flag) { - int cacheIndex = mBooleanCache.indexOfKey(flag.getId()); - if (cacheIndex < 0) { + if (!mBooleanCache.containsKey(flag.getName())) { return isEnabled( - flag.getId(), mSystemProperties.getBoolean(flag.getName(), flag.getDefault())); + flag.getName(), + mSystemProperties.getBoolean(flag.getName(), flag.getDefault())); } - return mBooleanCache.valueAt(cacheIndex); + return mBooleanCache.get(flag.getName()); } - private boolean isEnabled(int key, boolean defaultValue) { - mBooleanCache.append(key, defaultValue); + private boolean isEnabled(String name, boolean defaultValue) { + mBooleanCache.put(name, defaultValue); return defaultValue; } @NonNull @Override public String getString(@NonNull StringFlag flag) { - return getString(flag.getId(), flag.getDefault()); + return getString(flag.getName(), flag.getDefault()); } @NonNull @Override public String getString(@NonNull ResourceStringFlag flag) { - int cacheIndex = mStringCache.indexOfKey(flag.getId()); - if (cacheIndex < 0) { - return getString(flag.getId(), + if (!mStringCache.containsKey(flag.getName())) { + return getString(flag.getName(), requireNonNull(mResources.getString(flag.getResourceId()))); } - return mStringCache.valueAt(cacheIndex); + return mStringCache.get(flag.getName()); } - private String getString(int key, String defaultValue) { - mStringCache.append(key, defaultValue); + private String getString(String name, String defaultValue) { + mStringCache.put(name, defaultValue); return defaultValue; } @@ -169,11 +162,17 @@ public class FeatureFlagsRelease implements FeatureFlags { public void dump(@NonNull PrintWriter pw, @NonNull String[] args) { pw.println("can override: false"); Map> knownFlags = FlagsFactory.INSTANCE.getKnownFlags(); + pw.println("Booleans: "); for (Map.Entry> nameToFlag : knownFlags.entrySet()) { Flag flag = nameToFlag.getValue(); - int id = flag.getId(); + if (!(flag instanceof BooleanFlag) + || !(flag instanceof ResourceBooleanFlag) + || !(flag instanceof SysPropBooleanFlag)) { + continue; + } + boolean def = false; - if (mBooleanCache.indexOfKey(flag.getId()) < 0) { + if (!mBooleanCache.containsKey(flag.getName())) { if (flag instanceof SysPropBooleanFlag) { SysPropBooleanFlag f = (SysPropBooleanFlag) flag; def = mSystemProperties.getBoolean(f.getName(), f.getDefault()); @@ -185,15 +184,32 @@ public class FeatureFlagsRelease implements FeatureFlags { def = f.getDefault(); } } - pw.println(" sysui_flag_" + id + ": " + (mBooleanCache.get(id, def))); + pw.println( + " " + flag.getName() + ": " + + (mBooleanCache.getOrDefault(flag.getName(), def))); } - int numStrings = mStringCache.size(); - pw.println("Strings: " + numStrings); - for (int i = 0; i < numStrings; i++) { - final int id = mStringCache.keyAt(i); - final String value = mStringCache.valueAt(i); - final int length = value.length(); - pw.println(" sysui_flag_" + id + ": [length=" + length + "] \"" + value + "\""); + + pw.println("Strings: "); + for (Map.Entry> nameToFlag : knownFlags.entrySet()) { + Flag flag = nameToFlag.getValue(); + if (!(flag instanceof StringFlag) + || !(flag instanceof ResourceStringFlag)) { + continue; + } + + String def = ""; + if (!mBooleanCache.containsKey(flag.getName())) { + if (flag instanceof ResourceStringFlag) { + ResourceStringFlag f = (ResourceStringFlag) flag; + def = mResources.getString(f.getResourceId()); + } else if (flag instanceof StringFlag) { + StringFlag f = (StringFlag) flag; + def = f.getDefault(); + } + } + String value = mStringCache.getOrDefault(flag.getName(), def); + pw.println( + " " + flag.getName() + ": [length=" + value.length() + "] \"" + value + "\""); } } } diff --git a/packages/SystemUI/src/com/android/systemui/flags/FlagCommand.java b/packages/SystemUI/src/com/android/systemui/flags/FlagCommand.java index b7fc0e41ea69d..daf9429344a79 100644 --- a/packages/SystemUI/src/com/android/systemui/flags/FlagCommand.java +++ b/packages/SystemUI/src/com/android/systemui/flags/FlagCommand.java @@ -39,12 +39,12 @@ public class FlagCommand implements Command { private final List mOffCommands = List.of("false", "off", "0", "disable"); private final List mSetCommands = List.of("set", "put"); private final FeatureFlagsDebug mFeatureFlags; - private final Map> mAllFlags; + private final Map> mAllFlags; @Inject FlagCommand( FeatureFlagsDebug featureFlags, - @Named(ALL_FLAGS) Map> allFlags + @Named(ALL_FLAGS) Map> allFlags ) { mFeatureFlags = featureFlags; mAllFlags = allFlags; @@ -53,30 +53,22 @@ public class FlagCommand implements Command { @Override public void execute(@NonNull PrintWriter pw, @NonNull List args) { if (args.size() == 0) { - pw.println("Error: no flag id supplied"); + pw.println("Error: no flag name supplied"); help(pw); pw.println(); printKnownFlags(pw); return; } - int id = 0; - try { - id = Integer.parseInt(args.get(0)); - if (!mAllFlags.containsKey(id)) { - pw.println("Unknown flag id: " + id); - pw.println(); - printKnownFlags(pw); - return; - } - } catch (NumberFormatException e) { - id = flagNameToId(args.get(0)); - if (id == 0) { - pw.println("Invalid flag. Must an integer id or flag name: " + args.get(0)); - return; - } + String name = args.get(0); + if (!mAllFlags.containsKey(name)) { + pw.println("Unknown flag name: " + name); + pw.println(); + printKnownFlags(pw); + return; } - Flag flag = mAllFlags.get(id); + + Flag flag = mAllFlags.get(name); String cmd = ""; if (args.size() > 1) { @@ -117,7 +109,7 @@ public class FlagCommand implements Command { return; } - pw.println("Flag " + id + " is " + newValue); + pw.println("Flag " + name + " is " + newValue); pw.flush(); // Next command will restart sysui, so flush before we do so. if (shouldSet) { mFeatureFlags.setBooleanFlagInternal(flag, newValue); @@ -136,11 +128,11 @@ public class FlagCommand implements Command { return; } String value = args.get(2); - pw.println("Setting Flag " + id + " to " + value); + pw.println("Setting Flag " + name + " to " + value); pw.flush(); // Next command will restart sysui, so flush before we do so. mFeatureFlags.setStringFlagInternal(flag, args.get(2)); } else { - pw.println("Flag " + id + " is " + getStringFlag(flag)); + pw.println("Flag " + name + " is " + getStringFlag(flag)); } return; } else if (isIntFlag(flag)) { @@ -155,11 +147,11 @@ public class FlagCommand implements Command { return; } int value = Integer.parseInt(args.get(2)); - pw.println("Setting Flag " + id + " to " + value); + pw.println("Setting Flag " + name + " to " + value); pw.flush(); // Next command will restart sysui, so flush before we do so. mFeatureFlags.setIntFlagInternal(flag, value); } else { - pw.println("Flag " + id + " is " + getIntFlag(flag)); + pw.println("Flag " + name + " is " + getIntFlag(flag)); } return; } @@ -182,8 +174,7 @@ public class FlagCommand implements Command { private boolean isBooleanFlag(Flag flag) { return (flag instanceof BooleanFlag) || (flag instanceof ResourceBooleanFlag) - || (flag instanceof SysPropFlag) - || (flag instanceof DeviceConfigBooleanFlag); + || (flag instanceof SysPropFlag); } private boolean isBooleanFlagEnabled(Flag flag) { @@ -252,15 +243,14 @@ public class FlagCommand implements Command { for (int i = 0; i < longestFieldName - "Flag Name".length() + 1; i++) { pw.print(" "); } - pw.println("ID Value"); + pw.println(" Value"); for (int i = 0; i < longestFieldName; i++) { pw.print("="); } - pw.println(" ==== ========"); + pw.println(" ========"); for (String fieldName : fields.keySet()) { Flag flag = fields.get(fieldName); - int id = flag.getId(); - if (id == 0 || !mAllFlags.containsKey(id)) { + if (!mAllFlags.containsKey(flag.getName())) { continue; } pw.print(fieldName); @@ -268,9 +258,9 @@ public class FlagCommand implements Command { for (int i = 0; i < longestFieldName - fieldWidth + 1; i++) { pw.print(" "); } - pw.printf("%-4d ", id); + pw.print(" "); if (isBooleanFlag(flag)) { - pw.println(isBooleanFlagEnabled(mAllFlags.get(id))); + pw.println(isBooleanFlagEnabled(flag)); } else if (isStringFlag(flag)) { pw.println(getStringFlag(flag)); } else if (isIntFlag(flag)) { diff --git a/packages/SystemUI/src/com/android/systemui/flags/FlagsCommonModule.kt b/packages/SystemUI/src/com/android/systemui/flags/FlagsCommonModule.kt index 8442230fc5b15..0054d266c283b 100644 --- a/packages/SystemUI/src/com/android/systemui/flags/FlagsCommonModule.kt +++ b/packages/SystemUI/src/com/android/systemui/flags/FlagsCommonModule.kt @@ -28,8 +28,8 @@ interface FlagsCommonModule { @JvmStatic @Provides @Named(ALL_FLAGS) - fun providesAllFlags(): Map> { - return FlagsFactory.knownFlags.map { it.value.id to it.value }.toMap() + fun providesAllFlags(): Map> { + return FlagsFactory.knownFlags } } } diff --git a/packages/SystemUI/src/com/android/systemui/flags/ServerFlagReader.kt b/packages/SystemUI/src/com/android/systemui/flags/ServerFlagReader.kt index ae05c46563499..a02b795f074eb 100644 --- a/packages/SystemUI/src/com/android/systemui/flags/ServerFlagReader.kt +++ b/packages/SystemUI/src/com/android/systemui/flags/ServerFlagReader.kt @@ -54,10 +54,11 @@ class ServerFlagReaderImpl @Inject constructor( return } + for ((listener, flags) in listeners) { propLoop@ for (propName in properties.keyset) { for (flag in flags) { - if (propName == getServerOverrideName(flag.id)) { + if (propName == getServerOverrideName(flag.id) || propName == flag.name) { listener.onChange() break@propLoop } diff --git a/packages/SystemUI/tests/src/com/android/systemui/ChooserSelectorTest.kt b/packages/SystemUI/tests/src/com/android/systemui/ChooserSelectorTest.kt index 81d0034128b17..32edf8f23aedb 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/ChooserSelectorTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/ChooserSelectorTest.kt @@ -148,7 +148,7 @@ class ChooserSelectorTest : SysuiTestCase() { // Act whenever(mockFeatureFlags.isEnabled(any())).thenReturn(true) - flagListener.value.onFlagChanged(TestFlagEvent(Flags.CHOOSER_UNBUNDLED.id)) + flagListener.value.onFlagChanged(TestFlagEvent(Flags.CHOOSER_UNBUNDLED.name)) // Assert verify(mockPackageManager, times(2)).setComponentEnabledSetting( @@ -175,7 +175,7 @@ class ChooserSelectorTest : SysuiTestCase() { // Act whenever(mockFeatureFlags.isEnabled(any())).thenReturn(false) - flagListener.value.onFlagChanged(TestFlagEvent(Flags.CHOOSER_UNBUNDLED.id)) + flagListener.value.onFlagChanged(TestFlagEvent(Flags.CHOOSER_UNBUNDLED.name)) // Assert verify(mockPackageManager, times(2)).setComponentEnabledSetting( @@ -198,13 +198,13 @@ class ChooserSelectorTest : SysuiTestCase() { // Act whenever(mockFeatureFlags.isEnabled(any())).thenReturn(false) - flagListener.value.onFlagChanged(TestFlagEvent(Flags.CHOOSER_UNBUNDLED.id + 1)) + flagListener.value.onFlagChanged(TestFlagEvent("other flag")) // Assert verifyZeroInteractions(mockPackageManager) } - private class TestFlagEvent(override val flagId: Int) : FlagListenable.FlagEvent { + private class TestFlagEvent(override val flagName: String) : FlagListenable.FlagEvent { override fun requestNoRestart() {} } } 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 170a70f2fc403..35f0f6c68798e 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/flags/FakeFeatureFlagsTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/flags/FakeFeatureFlagsTest.kt @@ -125,7 +125,7 @@ class FakeFeatureFlagsTest : SysuiTestCase() { flags.set(unreleasedFlag, false) flags.set(unreleasedFlag, false) - listener.verifyInOrder(unreleasedFlag.id, unreleasedFlag.id) + listener.verifyInOrder(unreleasedFlag.name, unreleasedFlag.name) } @Test @@ -137,7 +137,7 @@ class FakeFeatureFlagsTest : SysuiTestCase() { flags.set(stringFlag, "Test") flags.set(stringFlag, "Test") - listener.verifyInOrder(stringFlag.id) + listener.verifyInOrder(stringFlag.name) } @Test @@ -149,7 +149,7 @@ class FakeFeatureFlagsTest : SysuiTestCase() { flags.removeListener(listener) flags.set(unreleasedFlag, false) - listener.verifyInOrder(unreleasedFlag.id) + listener.verifyInOrder(unreleasedFlag.name) } @Test @@ -162,7 +162,7 @@ class FakeFeatureFlagsTest : SysuiTestCase() { flags.removeListener(listener) flags.set(stringFlag, "Other") - listener.verifyInOrder(stringFlag.id) + listener.verifyInOrder(stringFlag.name) } @Test @@ -175,7 +175,7 @@ class FakeFeatureFlagsTest : SysuiTestCase() { flags.set(releasedFlag, true) flags.set(unreleasedFlag, true) - listener.verifyInOrder(releasedFlag.id, unreleasedFlag.id) + listener.verifyInOrder(releasedFlag.name, unreleasedFlag.name) } @Test @@ -191,7 +191,7 @@ class FakeFeatureFlagsTest : SysuiTestCase() { flags.set(releasedFlag, false) flags.set(unreleasedFlag, false) - listener.verifyInOrder(releasedFlag.id, unreleasedFlag.id) + listener.verifyInOrder(releasedFlag.name, unreleasedFlag.name) } @Test @@ -204,8 +204,8 @@ class FakeFeatureFlagsTest : SysuiTestCase() { flags.set(releasedFlag, true) - listener1.verifyInOrder(releasedFlag.id) - listener2.verifyInOrder(releasedFlag.id) + listener1.verifyInOrder(releasedFlag.name) + listener2.verifyInOrder(releasedFlag.name) } @Test @@ -220,18 +220,18 @@ class FakeFeatureFlagsTest : SysuiTestCase() { flags.removeListener(listener2) flags.set(releasedFlag, false) - listener1.verifyInOrder(releasedFlag.id, releasedFlag.id) - listener2.verifyInOrder(releasedFlag.id) + listener1.verifyInOrder(releasedFlag.name, releasedFlag.name) + listener2.verifyInOrder(releasedFlag.name) } class VerifyingListener : FlagListenable.Listener { - var flagEventIds = mutableListOf() + var flagEventNames = mutableListOf() override fun onFlagChanged(event: FlagListenable.FlagEvent) { - flagEventIds.add(event.flagId) + flagEventNames.add(event.flagName) } - fun verifyInOrder(vararg eventIds: Int) { - assertThat(flagEventIds).containsExactlyElementsIn(eventIds.asList()) + fun verifyInOrder(vararg eventNames: String) { + assertThat(flagEventNames).containsExactlyElementsIn(eventNames.asList()) } } } 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 7592cc5271904..d8bbd04bfd4a6 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/flags/FeatureFlagsDebugTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/flags/FeatureFlagsDebugTest.kt @@ -23,12 +23,11 @@ import android.content.res.Resources import android.content.res.Resources.NotFoundException import android.test.suitebuilder.annotation.SmallTest import com.android.systemui.SysuiTestCase -import com.android.systemui.statusbar.commandline.CommandRegistry -import com.android.systemui.util.DeviceConfigProxyFake import com.android.systemui.util.mockito.any import com.android.systemui.util.mockito.eq import com.android.systemui.util.mockito.nullable import com.android.systemui.util.mockito.withArgCaptor +import com.android.systemui.util.settings.GlobalSettings import com.android.systemui.util.settings.SecureSettings import com.google.common.truth.Truth.assertThat import org.junit.Assert @@ -62,21 +61,20 @@ class FeatureFlagsDebugTest : SysuiTestCase() { @Mock private lateinit var mockContext: Context @Mock + private lateinit var globalSettings: GlobalSettings + @Mock private lateinit var secureSettings: SecureSettings @Mock private lateinit var systemProperties: SystemPropertiesHelper @Mock private lateinit var resources: Resources @Mock - private lateinit var commandRegistry: CommandRegistry - @Mock private lateinit var restarter: Restarter - private val flagMap = mutableMapOf>() + private val flagMap = mutableMapOf>() private lateinit var broadcastReceiver: BroadcastReceiver - private lateinit var clearCacheAction: Consumer + private lateinit var clearCacheAction: Consumer private val serverFlagReader = ServerFlagReaderFake() - private val deviceConfig = DeviceConfigProxyFake() private val teamfoodableFlagA = UnreleasedFlag( 500, name = "a", namespace = "test", teamfood = true ) @@ -87,11 +85,13 @@ class FeatureFlagsDebugTest : SysuiTestCase() { @Before fun setup() { MockitoAnnotations.initMocks(this) - flagMap.put(teamfoodableFlagA.id, teamfoodableFlagA) - flagMap.put(teamfoodableFlagB.id, teamfoodableFlagB) + flagMap.put(Flags.TEAMFOOD.name, Flags.TEAMFOOD) + flagMap.put(teamfoodableFlagA.name, teamfoodableFlagA) + flagMap.put(teamfoodableFlagB.name, teamfoodableFlagB) mFeatureFlagsDebug = FeatureFlagsDebug( flagManager, mockContext, + globalSettings, secureSettings, systemProperties, resources, @@ -110,14 +110,14 @@ class FeatureFlagsDebugTest : SysuiTestCase() { clearCacheAction = withArgCaptor { verify(flagManager).clearCacheAction = capture() } - whenever(flagManager.idToSettingsKey(any())).thenAnswer { "key-${it.arguments[0]}" } + whenever(flagManager.nameToSettingsKey(any())).thenAnswer { "key-${it.arguments[0]}" } } @Test 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) + whenever(flagManager.readFlagValue(eq("3"), any())).thenReturn(true) + whenever(flagManager.readFlagValue(eq("4"), any())).thenReturn(false) assertThat( mFeatureFlagsDebug.isEnabled( @@ -141,7 +141,7 @@ class FeatureFlagsDebugTest : SysuiTestCase() { mFeatureFlagsDebug.isEnabled( ReleasedFlag( 4, - name = "3", + name = "4", namespace = "test" ) ) @@ -150,7 +150,7 @@ class FeatureFlagsDebugTest : SysuiTestCase() { mFeatureFlagsDebug.isEnabled( UnreleasedFlag( 5, - name = "4", + name = "5", namespace = "test" ) ) @@ -159,7 +159,8 @@ class FeatureFlagsDebugTest : SysuiTestCase() { @Test fun teamFoodFlag_False() { - whenever(flagManager.readFlagValue(eq(1), any())).thenReturn(false) + whenever(flagManager.readFlagValue( + eq(Flags.TEAMFOOD.name), any())).thenReturn(false) assertThat(mFeatureFlagsDebug.isEnabled(teamfoodableFlagA)).isFalse() assertThat(mFeatureFlagsDebug.isEnabled(teamfoodableFlagB)).isTrue() @@ -170,7 +171,8 @@ class FeatureFlagsDebugTest : SysuiTestCase() { @Test fun teamFoodFlag_True() { - whenever(flagManager.readFlagValue(eq(1), any())).thenReturn(true) + whenever(flagManager.readFlagValue( + eq(Flags.TEAMFOOD.name), any())).thenReturn(true) assertThat(mFeatureFlagsDebug.isEnabled(teamfoodableFlagA)).isTrue() assertThat(mFeatureFlagsDebug.isEnabled(teamfoodableFlagB)).isTrue() @@ -181,11 +183,12 @@ class FeatureFlagsDebugTest : SysuiTestCase() { @Test fun teamFoodFlag_Overridden() { - whenever(flagManager.readFlagValue(eq(teamfoodableFlagA.id), any())) + whenever(flagManager.readFlagValue(eq(teamfoodableFlagA.name), any())) .thenReturn(true) - whenever(flagManager.readFlagValue(eq(teamfoodableFlagB.id), any())) + whenever(flagManager.readFlagValue(eq(teamfoodableFlagB.name), any())) .thenReturn(false) - whenever(flagManager.readFlagValue(eq(1), any())).thenReturn(true) + whenever(flagManager.readFlagValue( + eq(Flags.TEAMFOOD.name), any())).thenReturn(true) assertThat(mFeatureFlagsDebug.isEnabled(teamfoodableFlagA)).isTrue() assertThat(mFeatureFlagsDebug.isEnabled(teamfoodableFlagB)).isFalse() @@ -202,8 +205,8 @@ class FeatureFlagsDebugTest : SysuiTestCase() { whenever(resources.getBoolean(1004)).thenAnswer { throw NameNotFoundException() } whenever(resources.getBoolean(1005)).thenAnswer { throw NameNotFoundException() } - whenever(flagManager.readFlagValue(eq(3), any())).thenReturn(true) - whenever(flagManager.readFlagValue(eq(5), any())).thenReturn(false) + whenever(flagManager.readFlagValue(eq("3"), any())).thenReturn(true) + whenever(flagManager.readFlagValue(eq("5"), any())).thenReturn(false) assertThat( mFeatureFlagsDebug.isEnabled( @@ -255,8 +258,8 @@ class FeatureFlagsDebugTest : SysuiTestCase() { @Test fun readStringFlag() { - whenever(flagManager.readFlagValue(eq(3), any())).thenReturn("foo") - whenever(flagManager.readFlagValue(eq(4), any())).thenReturn("bar") + whenever(flagManager.readFlagValue(eq("3"), any())).thenReturn("foo") + whenever(flagManager.readFlagValue(eq("4"), any())).thenReturn("bar") assertThat(mFeatureFlagsDebug.getString(StringFlag(1, "1", "test", "biz"))).isEqualTo("biz") assertThat(mFeatureFlagsDebug.getString(StringFlag(2, "2", "test", "baz"))).isEqualTo("baz") assertThat(mFeatureFlagsDebug.getString(StringFlag(3, "3", "test", "buz"))).isEqualTo("foo") @@ -272,9 +275,9 @@ class FeatureFlagsDebugTest : SysuiTestCase() { whenever(resources.getString(1005)).thenAnswer { throw NameNotFoundException() } whenever(resources.getString(1006)).thenAnswer { throw NameNotFoundException() } - whenever(flagManager.readFlagValue(eq(3), any())).thenReturn("override3") - whenever(flagManager.readFlagValue(eq(4), any())).thenReturn("override4") - whenever(flagManager.readFlagValue(eq(6), any())).thenReturn("override6") + whenever(flagManager.readFlagValue(eq("3"), any())).thenReturn("override3") + whenever(flagManager.readFlagValue(eq("4"), any())).thenReturn("override4") + whenever(flagManager.readFlagValue(eq("6"), any())).thenReturn("override6") assertThat( mFeatureFlagsDebug.getString( @@ -322,8 +325,8 @@ class FeatureFlagsDebugTest : SysuiTestCase() { @Test fun readIntFlag() { - whenever(flagManager.readFlagValue(eq(3), any())).thenReturn(22) - whenever(flagManager.readFlagValue(eq(4), any())).thenReturn(48) + whenever(flagManager.readFlagValue(eq("3"), any())).thenReturn(22) + whenever(flagManager.readFlagValue(eq("4"), any())).thenReturn(48) assertThat(mFeatureFlagsDebug.getInt(IntFlag(1, "1", "test", 12))).isEqualTo(12) assertThat(mFeatureFlagsDebug.getInt(IntFlag(2, "2", "test", 93))).isEqualTo(93) assertThat(mFeatureFlagsDebug.getInt(IntFlag(3, "3", "test", 8))).isEqualTo(22) @@ -368,12 +371,12 @@ class FeatureFlagsDebugTest : SysuiTestCase() { broadcastReceiver.onReceive(mockContext, Intent()) broadcastReceiver.onReceive(mockContext, Intent("invalid action")) broadcastReceiver.onReceive(mockContext, Intent(FlagManager.ACTION_SET_FLAG)) - setByBroadcast(0, false) // unknown id does nothing - setByBroadcast(1, "string") // wrong type does nothing - setByBroadcast(2, 123) // wrong type does nothing - setByBroadcast(3, false) // wrong type does nothing - setByBroadcast(4, 123) // wrong type does nothing - verifyNoMoreInteractions(flagManager, secureSettings) + setByBroadcast("0", false) // unknown id does nothing + setByBroadcast("1", "string") // wrong type does nothing + setByBroadcast("2", 123) // wrong type does nothing + setByBroadcast("3", false) // wrong type does nothing + setByBroadcast("4", 123) // wrong type does nothing + verifyNoMoreInteractions(flagManager, globalSettings) } @Test @@ -383,16 +386,16 @@ class FeatureFlagsDebugTest : SysuiTestCase() { // trying to erase an id not in the map does nothing broadcastReceiver.onReceive( mockContext, - Intent(FlagManager.ACTION_SET_FLAG).putExtra(FlagManager.EXTRA_ID, 0) + Intent(FlagManager.ACTION_SET_FLAG).putExtra(FlagManager.EXTRA_NAME, "") ) - verifyNoMoreInteractions(flagManager, secureSettings) + verifyNoMoreInteractions(flagManager, globalSettings) // valid id with no value puts empty string in the setting broadcastReceiver.onReceive( mockContext, - Intent(FlagManager.ACTION_SET_FLAG).putExtra(FlagManager.EXTRA_ID, 1) + Intent(FlagManager.ACTION_SET_FLAG).putExtra(FlagManager.EXTRA_NAME, "1") ) - verifyPutData(1, "", numReads = 0) + verifyPutData("1", "", numReads = 0) } @Test @@ -402,51 +405,51 @@ class FeatureFlagsDebugTest : SysuiTestCase() { addFlag(ResourceBooleanFlag(3, "3", "test", 1003)) addFlag(ResourceBooleanFlag(4, "4", "test", 1004)) - setByBroadcast(1, false) - verifyPutData(1, "{\"type\":\"boolean\",\"value\":false}") + setByBroadcast("1", false) + verifyPutData("1", "{\"type\":\"boolean\",\"value\":false}") - setByBroadcast(2, true) - verifyPutData(2, "{\"type\":\"boolean\",\"value\":true}") + setByBroadcast("2", true) + verifyPutData("2", "{\"type\":\"boolean\",\"value\":true}") - setByBroadcast(3, false) - verifyPutData(3, "{\"type\":\"boolean\",\"value\":false}") + setByBroadcast("3", false) + verifyPutData("3", "{\"type\":\"boolean\",\"value\":false}") - setByBroadcast(4, true) - verifyPutData(4, "{\"type\":\"boolean\",\"value\":true}") + setByBroadcast("4", true) + verifyPutData("4", "{\"type\":\"boolean\",\"value\":true}") } @Test fun setStringFlag() { - addFlag(StringFlag(1, "flag1", "1", "test")) + addFlag(StringFlag(1, "1", "1", "test")) addFlag(ResourceStringFlag(2, "2", "test", 1002)) - setByBroadcast(1, "override1") - verifyPutData(1, "{\"type\":\"string\",\"value\":\"override1\"}") + setByBroadcast("1", "override1") + verifyPutData("1", "{\"type\":\"string\",\"value\":\"override1\"}") - setByBroadcast(2, "override2") - verifyPutData(2, "{\"type\":\"string\",\"value\":\"override2\"}") + setByBroadcast("2", "override2") + verifyPutData("2", "{\"type\":\"string\",\"value\":\"override2\"}") } @Test fun setFlag_ClearsCache() { val flag1 = addFlag(StringFlag(1, "1", "test", "flag1")) - whenever(flagManager.readFlagValue(eq(1), any())).thenReturn("original") + whenever(flagManager.readFlagValue(eq("1"), any())).thenReturn("original") // gets the flag & cache it assertThat(mFeatureFlagsDebug.getString(flag1)).isEqualTo("original") - verify(flagManager).readFlagValue(eq(1), eq(StringFlagSerializer)) + verify(flagManager, times(1)).readFlagValue(eq("1"), eq(StringFlagSerializer)) // hit the cache assertThat(mFeatureFlagsDebug.getString(flag1)).isEqualTo("original") verifyNoMoreInteractions(flagManager) // set the flag - setByBroadcast(1, "new") - verifyPutData(1, "{\"type\":\"string\",\"value\":\"new\"}", numReads = 2) - whenever(flagManager.readFlagValue(eq(1), any())).thenReturn("new") + setByBroadcast("1", "new") + verifyPutData("1", "{\"type\":\"string\",\"value\":\"new\"}", numReads = 2) + whenever(flagManager.readFlagValue(eq("1"), any())).thenReturn("new") assertThat(mFeatureFlagsDebug.getString(flag1)).isEqualTo("new") - verify(flagManager, times(3)).readFlagValue(eq(1), eq(StringFlagSerializer)) + verify(flagManager, times(3)).readFlagValue(eq("1"), eq(StringFlagSerializer)) } @Test @@ -463,7 +466,6 @@ class FeatureFlagsDebugTest : SysuiTestCase() { val flag = UnreleasedFlag(100, name = "100", namespace = "test") serverFlagReader.setFlagValue(flag.namespace, flag.name, true) - assertThat(mFeatureFlagsDebug.isEnabled(flag)).isTrue() } @@ -503,26 +505,26 @@ class FeatureFlagsDebugTest : SysuiTestCase() { 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>()) - verify(flagManager).idToSettingsKey(eq(id)) - verify(secureSettings).putStringForUser(eq("key-$id"), eq(data), anyInt()) - verify(flagManager).dispatchListenersAndMaybeRestart(eq(id), any()) + private fun verifyPutData(name: String, data: String, numReads: Int = 1) { + inOrder(flagManager, globalSettings).apply { + verify(flagManager, times(numReads)).readFlagValue(eq(name), any>()) + verify(flagManager).nameToSettingsKey(eq(name)) + verify(globalSettings).putStringForUser(eq("key-$name"), eq(data), anyInt()) + verify(flagManager).dispatchListenersAndMaybeRestart(eq(name), any()) }.verifyNoMoreInteractions() - verifyNoMoreInteractions(flagManager, secureSettings) + verifyNoMoreInteractions(flagManager, globalSettings) } - private fun setByBroadcast(id: Int, value: Serializable?) { + private fun setByBroadcast(name: String, value: Serializable?) { val intent = Intent(FlagManager.ACTION_SET_FLAG) - intent.putExtra(FlagManager.EXTRA_ID, id) + intent.putExtra(FlagManager.EXTRA_NAME, name) intent.putExtra(FlagManager.EXTRA_VALUE, value) broadcastReceiver.onReceive(mockContext, intent) } private fun > addFlag(flag: F): F { - val old = flagMap.put(flag.id, flag) - check(old == null) { "Flag ${flag.id} already registered" } + val old = flagMap.put(flag.name, flag) + check(old == null) { "Flag ${flag.name} already registered" } return flag } 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 d5b5a4a4101ed..4c6028c4c9b7c 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/flags/FeatureFlagsReleaseTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/flags/FeatureFlagsReleaseTest.kt @@ -19,7 +19,6 @@ import android.content.pm.PackageManager.NameNotFoundException import android.content.res.Resources import android.test.suitebuilder.annotation.SmallTest import com.android.systemui.SysuiTestCase -import com.android.systemui.util.DeviceConfigProxyFake import com.google.common.truth.Truth.assertThat import org.junit.Assert.assertThrows import org.junit.Before @@ -39,9 +38,8 @@ class FeatureFlagsReleaseTest : SysuiTestCase() { @Mock private lateinit var mResources: Resources @Mock private lateinit var mSystemProperties: SystemPropertiesHelper @Mock private lateinit var restarter: Restarter - private val flagMap = mutableMapOf>() + private val flagMap = mutableMapOf>() private val serverFlagReader = ServerFlagReaderFake() - private val deviceConfig = DeviceConfigProxyFake() @Before fun setup() { @@ -49,7 +47,6 @@ class FeatureFlagsReleaseTest : SysuiTestCase() { mFeatureFlagsRelease = FeatureFlagsRelease( mResources, mSystemProperties, - deviceConfig, serverFlagReader, flagMap, restarter) diff --git a/packages/SystemUI/tests/src/com/android/systemui/flags/FlagCommandTest.kt b/packages/SystemUI/tests/src/com/android/systemui/flags/FlagCommandTest.kt index fea91c53424d2..28131b50f04cc 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/flags/FlagCommandTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/flags/FlagCommandTest.kt @@ -32,7 +32,7 @@ class FlagCommandTest : SysuiTestCase() { @Mock private lateinit var featureFlags: FeatureFlagsDebug @Mock private lateinit var pw: PrintWriter - private val flagMap = mutableMapOf>() + private val flagMap = mutableMapOf>() private val flagA = UnreleasedFlag(500, "500", "test") private val flagB = ReleasedFlag(501, "501", "test") private val stringFlag = StringFlag(502, "502", "test", "abracadabra") @@ -53,59 +53,59 @@ class FlagCommandTest : SysuiTestCase() { (invocation.getArgument(0) as IntFlag).default } - flagMap.put(flagA.id, flagA) - flagMap.put(flagB.id, flagB) - flagMap.put(stringFlag.id, stringFlag) - flagMap.put(intFlag.id, intFlag) + flagMap.put(flagA.name, flagA) + flagMap.put(flagB.name, flagB) + flagMap.put(stringFlag.name, stringFlag) + flagMap.put(intFlag.name, intFlag) cmd = FlagCommand(featureFlags, flagMap) } @Test fun readBooleanFlagCommand() { - cmd.execute(pw, listOf(flagA.id.toString())) + cmd.execute(pw, listOf(flagA.name)) Mockito.verify(featureFlags).isEnabled(flagA) } @Test fun readStringFlagCommand() { - cmd.execute(pw, listOf(stringFlag.id.toString())) + cmd.execute(pw, listOf(stringFlag.name)) Mockito.verify(featureFlags).getString(stringFlag) } @Test fun readIntFlag() { - cmd.execute(pw, listOf(intFlag.id.toString())) + cmd.execute(pw, listOf(intFlag.name)) Mockito.verify(featureFlags).getInt(intFlag) } @Test fun setBooleanFlagCommand() { - cmd.execute(pw, listOf(flagB.id.toString(), "on")) + cmd.execute(pw, listOf(flagB.name, "on")) Mockito.verify(featureFlags).setBooleanFlagInternal(flagB, true) } @Test fun setStringFlagCommand() { - cmd.execute(pw, listOf(stringFlag.id.toString(), "set", "foobar")) + cmd.execute(pw, listOf(stringFlag.name, "set", "foobar")) Mockito.verify(featureFlags).setStringFlagInternal(stringFlag, "foobar") } @Test fun setIntFlag() { - cmd.execute(pw, listOf(intFlag.id.toString(), "put", "123")) + cmd.execute(pw, listOf(intFlag.name, "put", "123")) Mockito.verify(featureFlags).setIntFlagInternal(intFlag, 123) } @Test fun toggleBooleanFlagCommand() { - cmd.execute(pw, listOf(flagB.id.toString(), "toggle")) + cmd.execute(pw, listOf(flagB.name, "toggle")) Mockito.verify(featureFlags).setBooleanFlagInternal(flagB, false) } @Test fun eraseFlagCommand() { - cmd.execute(pw, listOf(flagA.id.toString(), "erase")) + cmd.execute(pw, listOf(flagA.name, "erase")) Mockito.verify(featureFlags).eraseFlag(flagA) } } 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 fca7e96fb1482..e679d47537b54 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/flags/FlagManagerTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/flags/FlagManagerTest.kt @@ -87,14 +87,14 @@ class FlagManagerTest : SysuiTestCase() { @Test fun testObserverClearsCache() { val listener = mock() - val clearCacheAction = mock>() + val clearCacheAction = mock>() mFlagManager.clearCacheAction = clearCacheAction mFlagManager.addListener(ReleasedFlag(1, "1", "test"), listener) val observer = withArgCaptor { verify(mFlagSettingsHelper).registerContentObserver(any(), any(), capture()) } observer.onChange(false, flagUri(1)) - verify(clearCacheAction).accept(eq(1)) + verify(clearCacheAction).accept(eq("1")) } @Test @@ -110,14 +110,14 @@ class FlagManagerTest : SysuiTestCase() { val flagEvent1 = withArgCaptor { verify(listener1).onFlagChanged(capture()) } - assertThat(flagEvent1.flagId).isEqualTo(1) + assertThat(flagEvent1.flagName).isEqualTo("1") verifyNoMoreInteractions(listener1, listener10) observer.onChange(false, flagUri(10)) val flagEvent10 = withArgCaptor { verify(listener10).onFlagChanged(capture()) } - assertThat(flagEvent10.flagId).isEqualTo(10) + assertThat(flagEvent10.flagName).isEqualTo("10") verifyNoMoreInteractions(listener1, listener10) } @@ -130,18 +130,18 @@ class FlagManagerTest : SysuiTestCase() { mFlagManager.addListener(ReleasedFlag(1, "1", "test"), listener1) mFlagManager.addListener(ReleasedFlag(10, "10", "test"), listener10) - mFlagManager.dispatchListenersAndMaybeRestart(1, null) + mFlagManager.dispatchListenersAndMaybeRestart("1", null) val flagEvent1 = withArgCaptor { verify(listener1).onFlagChanged(capture()) } - assertThat(flagEvent1.flagId).isEqualTo(1) + assertThat(flagEvent1.flagName).isEqualTo("1") verifyNoMoreInteractions(listener1, listener10) - mFlagManager.dispatchListenersAndMaybeRestart(10, null) + mFlagManager.dispatchListenersAndMaybeRestart("10", null) val flagEvent10 = withArgCaptor { verify(listener10).onFlagChanged(capture()) } - assertThat(flagEvent10.flagId).isEqualTo(10) + assertThat(flagEvent10.flagName).isEqualTo("10") verifyNoMoreInteractions(listener1, listener10) } @@ -151,25 +151,25 @@ class FlagManagerTest : SysuiTestCase() { mFlagManager.addListener(ReleasedFlag(1, "1", "test"), listener) mFlagManager.addListener(ReleasedFlag(10, "10", "test"), listener) - mFlagManager.dispatchListenersAndMaybeRestart(1, null) + mFlagManager.dispatchListenersAndMaybeRestart("1", null) val flagEvent1 = withArgCaptor { verify(listener).onFlagChanged(capture()) } - assertThat(flagEvent1.flagId).isEqualTo(1) + assertThat(flagEvent1.flagName).isEqualTo("1") verifyNoMoreInteractions(listener) - mFlagManager.dispatchListenersAndMaybeRestart(10, null) + mFlagManager.dispatchListenersAndMaybeRestart("10", null) val flagEvent10 = withArgCaptor { verify(listener, times(2)).onFlagChanged(capture()) } - assertThat(flagEvent10.flagId).isEqualTo(10) + assertThat(flagEvent10.flagName).isEqualTo("10") verifyNoMoreInteractions(listener) } @Test fun testRestartWithNoListeners() { val restartAction = mock>() - mFlagManager.dispatchListenersAndMaybeRestart(1, restartAction) + mFlagManager.dispatchListenersAndMaybeRestart("1", restartAction) verify(restartAction).accept(eq(false)) verifyNoMoreInteractions(restartAction) } @@ -180,7 +180,7 @@ class FlagManagerTest : SysuiTestCase() { mFlagManager.addListener(ReleasedFlag(1, "1", "test")) { event -> event.requestNoRestart() } - mFlagManager.dispatchListenersAndMaybeRestart(1, restartAction) + mFlagManager.dispatchListenersAndMaybeRestart("1", restartAction) verify(restartAction).accept(eq(true)) verifyNoMoreInteractions(restartAction) } @@ -191,7 +191,7 @@ class FlagManagerTest : SysuiTestCase() { mFlagManager.addListener(ReleasedFlag(10, "10", "test")) { event -> event.requestNoRestart() } - mFlagManager.dispatchListenersAndMaybeRestart(1, restartAction) + mFlagManager.dispatchListenersAndMaybeRestart("1", restartAction) verify(restartAction).accept(eq(false)) verifyNoMoreInteractions(restartAction) } @@ -205,7 +205,7 @@ class FlagManagerTest : SysuiTestCase() { mFlagManager.addListener(ReleasedFlag(10, "10", "test")) { // do not request } - mFlagManager.dispatchListenersAndMaybeRestart(1, restartAction) + mFlagManager.dispatchListenersAndMaybeRestart("1", restartAction) verify(restartAction).accept(eq(false)) verifyNoMoreInteractions(restartAction) } @@ -214,31 +214,31 @@ class FlagManagerTest : SysuiTestCase() { fun testReadBooleanFlag() { // test that null string returns null whenever(mFlagSettingsHelper.getString(any())).thenReturn(null) - assertThat(mFlagManager.readFlagValue(1, BooleanFlagSerializer)).isNull() + assertThat(mFlagManager.readFlagValue("1", BooleanFlagSerializer)).isNull() // test that empty string returns null whenever(mFlagSettingsHelper.getString(any())).thenReturn("") - assertThat(mFlagManager.readFlagValue(1, BooleanFlagSerializer)).isNull() + assertThat(mFlagManager.readFlagValue("1", BooleanFlagSerializer)).isNull() // test false whenever(mFlagSettingsHelper.getString(any())) .thenReturn("{\"type\":\"boolean\",\"value\":false}") - assertThat(mFlagManager.readFlagValue(1, BooleanFlagSerializer)).isFalse() + assertThat(mFlagManager.readFlagValue("1", BooleanFlagSerializer)).isFalse() // test true whenever(mFlagSettingsHelper.getString(any())) .thenReturn("{\"type\":\"boolean\",\"value\":true}") - assertThat(mFlagManager.readFlagValue(1, BooleanFlagSerializer)).isTrue() + assertThat(mFlagManager.readFlagValue("1", BooleanFlagSerializer)).isTrue() // Reading a value of a different type should just return null whenever(mFlagSettingsHelper.getString(any())) .thenReturn("{\"type\":\"string\",\"value\":\"foo\"}") - assertThat(mFlagManager.readFlagValue(1, BooleanFlagSerializer)).isNull() + assertThat(mFlagManager.readFlagValue("1", BooleanFlagSerializer)).isNull() // Reading a value that isn't json should throw an exception assertThrows(InvalidFlagStorageException::class.java) { whenever(mFlagSettingsHelper.getString(any())).thenReturn("1") - mFlagManager.readFlagValue(1, BooleanFlagSerializer) + mFlagManager.readFlagValue("1", BooleanFlagSerializer) } } @@ -257,31 +257,31 @@ class FlagManagerTest : SysuiTestCase() { fun testReadStringFlag() { // test that null string returns null whenever(mFlagSettingsHelper.getString(any())).thenReturn(null) - assertThat(mFlagManager.readFlagValue(1, StringFlagSerializer)).isNull() + assertThat(mFlagManager.readFlagValue("1", StringFlagSerializer)).isNull() // test that empty string returns null whenever(mFlagSettingsHelper.getString(any())).thenReturn("") - assertThat(mFlagManager.readFlagValue(1, StringFlagSerializer)).isNull() + assertThat(mFlagManager.readFlagValue("1", StringFlagSerializer)).isNull() // test json with the empty string value returns empty string whenever(mFlagSettingsHelper.getString(any())) .thenReturn("{\"type\":\"string\",\"value\":\"\"}") - assertThat(mFlagManager.readFlagValue(1, StringFlagSerializer)).isEqualTo("") + assertThat(mFlagManager.readFlagValue("1", StringFlagSerializer)).isEqualTo("") // test string with value is returned whenever(mFlagSettingsHelper.getString(any())) .thenReturn("{\"type\":\"string\",\"value\":\"foo\"}") - assertThat(mFlagManager.readFlagValue(1, StringFlagSerializer)).isEqualTo("foo") + assertThat(mFlagManager.readFlagValue("1", StringFlagSerializer)).isEqualTo("foo") // Reading a value of a different type should just return null whenever(mFlagSettingsHelper.getString(any())) .thenReturn("{\"type\":\"boolean\",\"value\":false}") - assertThat(mFlagManager.readFlagValue(1, StringFlagSerializer)).isNull() + assertThat(mFlagManager.readFlagValue("1", StringFlagSerializer)).isNull() // Reading a value that isn't json should throw an exception assertThrows(InvalidFlagStorageException::class.java) { whenever(mFlagSettingsHelper.getString(any())).thenReturn("1") - mFlagManager.readFlagValue(1, StringFlagSerializer) + mFlagManager.readFlagValue("1", StringFlagSerializer) } } diff --git a/packages/SystemUI/tests/utils/src/com/android/systemui/flags/FakeFeatureFlags.kt b/packages/SystemUI/tests/utils/src/com/android/systemui/flags/FakeFeatureFlags.kt index 6c82cef22ddbb..b94f816e1ca45 100644 --- a/packages/SystemUI/tests/utils/src/com/android/systemui/flags/FakeFeatureFlags.kt +++ b/packages/SystemUI/tests/utils/src/com/android/systemui/flags/FakeFeatureFlags.kt @@ -38,12 +38,6 @@ class FakeFeatureFlags : FeatureFlags { } } - fun set(flag: DeviceConfigBooleanFlag, value: Boolean) { - if (booleanFlags.put(flag.id, value)?.let { value != it } != false) { - notifyFlagChanged(flag) - } - } - fun set(flag: ResourceBooleanFlag, value: Boolean) { if (booleanFlags.put(flag.id, value)?.let { value != it } != false) { notifyFlagChanged(flag) @@ -73,7 +67,7 @@ class FakeFeatureFlags : FeatureFlags { listeners.forEach { listener -> listener.onFlagChanged( object : FlagListenable.FlagEvent { - override val flagId = flag.id + override val flagName = flag.name override fun requestNoRestart() {} } )