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() {} } )