diff options
8 files changed, 273 insertions, 31 deletions
diff --git a/packages/SystemUI/shared/src/com/android/systemui/flags/FlagSerializer.kt b/packages/SystemUI/shared/src/com/android/systemui/flags/FlagSerializer.kt index e9ea19dad424..eeb6031df6d9 100644 --- a/packages/SystemUI/shared/src/com/android/systemui/flags/FlagSerializer.kt +++ b/packages/SystemUI/shared/src/com/android/systemui/flags/FlagSerializer.kt @@ -24,6 +24,7 @@ private const val FIELD_VALUE = "value" private const val FIELD_TYPE = "type" private const val TYPE_BOOLEAN = "boolean" private const val TYPE_STRING = "string" +private const val TYPE_INT = "int" private const val TAG = "FlagSerializer" @@ -77,4 +78,10 @@ object StringFlagSerializer : FlagSerializer<String>( JSONObject::getString ) +object IntFlagSerializer : FlagSerializer<Int>( + TYPE_INT, + JSONObject::put, + JSONObject::getInt +) + class InvalidFlagStorageException : Exception("Data found but is invalid") diff --git a/packages/SystemUI/src/com/android/systemui/flags/FeatureFlags.kt b/packages/SystemUI/src/com/android/systemui/flags/FeatureFlags.kt index fb4fc928c8dc..a49aaccf92ba 100644 --- a/packages/SystemUI/src/com/android/systemui/flags/FeatureFlags.kt +++ b/packages/SystemUI/src/com/android/systemui/flags/FeatureFlags.kt @@ -44,4 +44,10 @@ interface FeatureFlags : FlagListenable, Dumpable { /** Returns a string value for the given flag. */ fun getString(flag: ResourceStringFlag): String + + /** Returns an int value for a given flag/ */ + fun getInt(flag: IntFlag): Int + + /** Returns an int value for a given flag/ */ + fun getInt(flag: ResourceIntFlag): Int } diff --git a/packages/SystemUI/src/com/android/systemui/flags/FeatureFlagsDebug.java b/packages/SystemUI/src/com/android/systemui/flags/FeatureFlagsDebug.java index 20e55a0f0507..98d10a77e2d1 100644 --- a/packages/SystemUI/src/com/android/systemui/flags/FeatureFlagsDebug.java +++ b/packages/SystemUI/src/com/android/systemui/flags/FeatureFlagsDebug.java @@ -81,6 +81,7 @@ public class FeatureFlagsDebug implements FeatureFlags { private final Map<Integer, Flag<?>> mAllFlags; private final Map<Integer, Boolean> mBooleanFlagCache = new TreeMap<>(); private final Map<Integer, String> mStringFlagCache = new TreeMap<>(); + private final Map<Integer, Integer> mIntFlagCache = new TreeMap<>(); private final Restarter mRestarter; private final ServerFlagReader.ChangeListener mOnPropertiesChanged = @@ -209,6 +210,31 @@ public class FeatureFlagsDebug implements FeatureFlags { return mStringFlagCache.get(id); } + @NonNull + @Override + public int getInt(@NonNull IntFlag flag) { + int id = flag.getId(); + if (!mIntFlagCache.containsKey(id)) { + mIntFlagCache.put(id, + readFlagValue(id, flag.getDefault(), IntFlagSerializer.INSTANCE)); + } + + return mIntFlagCache.get(id); + } + + @NonNull + @Override + public int getInt(@NonNull ResourceIntFlag flag) { + int id = flag.getId(); + if (!mIntFlagCache.containsKey(id)) { + mIntFlagCache.put(id, + readFlagValue(id, mResources.getInteger(flag.getResourceId()), + IntFlagSerializer.INSTANCE)); + } + + return mIntFlagCache.get(id); + } + /** Specific override for Boolean flags that checks against the teamfood list. */ private boolean readFlagValue(int id, boolean defaultValue) { Boolean result = readBooleanFlagOverride(id); @@ -351,6 +377,16 @@ public class FeatureFlagsDebug implements FeatureFlags { } } + void setIntFlagInternal(Flag<?> flag, int value) { + if (flag instanceof IntFlag) { + setFlagValue(flag.getId(), value, IntFlagSerializer.INSTANCE); + } else if (flag instanceof ResourceIntFlag) { + setFlagValue(flag.getId(), value, IntFlagSerializer.INSTANCE); + } else { + throw new IllegalArgumentException("Unknown flag type"); + } + } + private final BroadcastReceiver mReceiver = new BroadcastReceiver() { @Override public void onReceive(Context context, Intent intent) { diff --git a/packages/SystemUI/src/com/android/systemui/flags/FeatureFlagsRelease.java b/packages/SystemUI/src/com/android/systemui/flags/FeatureFlagsRelease.java index 30cad5faa2de..8996d5258920 100644 --- a/packages/SystemUI/src/com/android/systemui/flags/FeatureFlagsRelease.java +++ b/packages/SystemUI/src/com/android/systemui/flags/FeatureFlagsRelease.java @@ -165,6 +165,18 @@ public class FeatureFlagsRelease implements FeatureFlags { return defaultValue; } + @NonNull + @Override + public int getInt(@NonNull IntFlag flag) { + return flag.getDefault(); + } + + @NonNull + @Override + public int getInt(@NonNull ResourceIntFlag flag) { + return mResources.getInteger(flag.getResourceId()); + } + @Override public void dump(@NonNull PrintWriter pw, @NonNull String[] args) { pw.println("can override: false"); diff --git a/packages/SystemUI/src/com/android/systemui/flags/FlagCommand.java b/packages/SystemUI/src/com/android/systemui/flags/FlagCommand.java index 1e93c0b7002d..2a1e3cd937bd 100644 --- a/packages/SystemUI/src/com/android/systemui/flags/FlagCommand.java +++ b/packages/SystemUI/src/com/android/systemui/flags/FlagCommand.java @@ -38,6 +38,7 @@ public class FlagCommand implements Command { private final List<String> mOnCommands = List.of("true", "on", "1", "enabled"); private final List<String> mOffCommands = List.of("false", "off", "0", "disable"); + private final List<String> mSetCommands = List.of("set", "put"); private final FeatureFlagsDebug mFeatureFlags; private final Map<Integer, Flag<?>> mAllFlags; @@ -60,12 +61,6 @@ public class FlagCommand implements Command { return; } - if (args.size() > 2) { - pw.println("Invalid number of arguments."); - help(pw); - return; - } - int id = 0; try { id = Integer.parseInt(args.get(0)); @@ -85,48 +80,113 @@ public class FlagCommand implements Command { Flag<?> flag = mAllFlags.get(id); String cmd = ""; - if (args.size() == 2) { + if (args.size() > 1) { cmd = args.get(1).toLowerCase(); } if ("erase".equals(cmd) || "reset".equals(cmd)) { + if (args.size() > 2) { + pw.println("Invalid number of arguments to reset a flag."); + help(pw); + return; + } + mFeatureFlags.eraseFlag(flag); return; } - boolean newValue = true; - if (args.size() == 1 || "toggle".equals(cmd)) { - boolean enabled = isBooleanFlagEnabled(flag); - - if (args.size() == 1) { - pw.println("Flag " + id + " is " + enabled); + boolean shouldSet = true; + if (args.size() == 1) { + shouldSet = false; + } + if (isBooleanFlag(flag)) { + if (args.size() > 2) { + pw.println("Invalid number of arguments for a boolean flag."); + help(pw); return; } - - newValue = !enabled; - } else { - newValue = mOnCommands.contains(cmd); - if (!newValue && !mOffCommands.contains(cmd)) { + boolean newValue = isBooleanFlagEnabled(flag); + if ("toggle".equals(cmd)) { + newValue = !newValue; + } else if (mOnCommands.contains(cmd)) { + newValue = true; + } else if (mOffCommands.contains(cmd)) { + newValue = false; + } else if (shouldSet) { pw.println("Invalid on/off argument supplied"); help(pw); return; } - } - pw.flush(); // Next command will restart sysui, so flush before we do so. - mFeatureFlags.setBooleanFlagInternal(flag, newValue); + pw.println("Flag " + id + " is " + newValue); + pw.flush(); // Next command will restart sysui, so flush before we do so. + if (shouldSet) { + mFeatureFlags.setBooleanFlagInternal(flag, newValue); + } + return; + + } else if (isStringFlag(flag)) { + if (shouldSet) { + if (args.size() != 3) { + pw.println("Invalid number of arguments a StringFlag."); + help(pw); + return; + } else if (!mSetCommands.contains(cmd)) { + pw.println("Unknown command: " + cmd); + help(pw); + return; + } + String value = args.get(2); + pw.println("Setting Flag " + id + " 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)); + } + return; + } else if (isIntFlag(flag)) { + if (shouldSet) { + if (args.size() != 3) { + pw.println("Invalid number of arguments for an IntFlag."); + help(pw); + return; + } else if (!mSetCommands.contains(cmd)) { + pw.println("Unknown command: " + cmd); + help(pw); + return; + } + int value = Integer.parseInt(args.get(2)); + pw.println("Setting Flag " + id + " 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)); + } + return; + } } @Override public void help(PrintWriter pw) { - pw.println( - "Usage: adb shell cmd statusbar flag <id> " + pw.println("Usage: adb shell cmd statusbar flag <id> [options]"); + pw.println(); + pw.println(" Boolean Flag Options: " + "[true|false|1|0|on|off|enable|disable|toggle|erase|reset]"); + pw.println(" String Flag Options: [set|put \"<value>\"]"); + pw.println(" Int Flag Options: [set|put <value>]"); + pw.println(); pw.println("The id can either be a numeric integer or the corresponding field name"); pw.println( "If no argument is supplied after the id, the flags runtime value is output"); } + private boolean isBooleanFlag(Flag<?> flag) { + return (flag instanceof BooleanFlag) + || (flag instanceof ResourceBooleanFlag) + || (flag instanceof SysPropFlag) + || (flag instanceof DeviceConfigBooleanFlag); + } + private boolean isBooleanFlagEnabled(Flag<?> flag) { if (flag instanceof ReleasedFlag) { return mFeatureFlags.isEnabled((ReleasedFlag) flag); @@ -141,6 +201,34 @@ public class FlagCommand implements Command { return false; } + private boolean isStringFlag(Flag<?> flag) { + return (flag instanceof StringFlag) || (flag instanceof ResourceStringFlag); + } + + private String getStringFlag(Flag<?> flag) { + if (flag instanceof StringFlag) { + return mFeatureFlags.getString((StringFlag) flag); + } else if (flag instanceof ResourceStringFlag) { + return mFeatureFlags.getString((ResourceStringFlag) flag); + } + + return ""; + } + + private boolean isIntFlag(Flag<?> flag) { + return (flag instanceof IntFlag) || (flag instanceof ResourceIntFlag); + } + + private int getIntFlag(Flag<?> flag) { + if (flag instanceof IntFlag) { + return mFeatureFlags.getInt((IntFlag) flag); + } else if (flag instanceof ResourceIntFlag) { + return mFeatureFlags.getInt((ResourceIntFlag) flag); + } + + return 0; + } + private int flagNameToId(String flagName) { List<Field> fields = Flags.getFlagFields(); for (Field field : fields) { @@ -176,13 +264,15 @@ public class FlagCommand implements Command { for (int i = 0; i < longestFieldName - "Flag Name".length() + 1; i++) { pw.print(" "); } - pw.println("ID Enabled?"); + pw.println("ID Value"); for (int i = 0; i < longestFieldName; i++) { pw.print("="); } - pw.println(" ==== ========"); + pw.println(" ==== ====="); for (Field field : fields) { int id = fieldToId(field); + Flag<?> flag = mAllFlags.get(id); + if (id == 0 || !mAllFlags.containsKey(id)) { continue; } @@ -192,7 +282,15 @@ public class FlagCommand implements Command { pw.print(" "); } pw.printf("%-4d ", id); - pw.println(isBooleanFlagEnabled(mAllFlags.get(id))); + if (isBooleanFlag(flag)) { + pw.println(isBooleanFlagEnabled(mAllFlags.get(id))); + } else if (isStringFlag(flag)) { + pw.println(getStringFlag(flag)); + } else if (isIntFlag(flag)) { + pw.println(getIntFlag(flag)); + } else { + pw.println("<unknown flag type>"); + } } } } 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 4b3b70e3ae77..9c22cd2a077d 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/flags/FeatureFlagsDebugTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/flags/FeatureFlagsDebugTest.kt @@ -20,6 +20,7 @@ import android.content.Context import android.content.Intent import android.content.pm.PackageManager.NameNotFoundException 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 @@ -246,6 +247,43 @@ class FeatureFlagsDebugTest : SysuiTestCase() { } @Test + fun readIntFlag() { + whenever(flagManager.readFlagValue<Int>(eq(3), any())).thenReturn(22) + whenever(flagManager.readFlagValue<Int>(eq(4), any())).thenReturn(48) + assertThat(mFeatureFlagsDebug.getInt(IntFlag(1, 12))).isEqualTo(12) + assertThat(mFeatureFlagsDebug.getInt(IntFlag(2, 93))).isEqualTo(93) + assertThat(mFeatureFlagsDebug.getInt(IntFlag(3, 8))).isEqualTo(22) + assertThat(mFeatureFlagsDebug.getInt(IntFlag(4, 234))).isEqualTo(48) + } + + @Test + fun readResourceIntFlag() { + whenever(resources.getInteger(1001)).thenReturn(88) + whenever(resources.getInteger(1002)).thenReturn(61) + whenever(resources.getInteger(1003)).thenReturn(9342) + whenever(resources.getInteger(1004)).thenThrow(NotFoundException("unknown resource")) + whenever(resources.getInteger(1005)).thenThrow(NotFoundException("unknown resource")) + whenever(resources.getInteger(1006)).thenThrow(NotFoundException("unknown resource")) + + whenever(flagManager.readFlagValue<Int>(eq(3), any())).thenReturn(20) + whenever(flagManager.readFlagValue<Int>(eq(4), any())).thenReturn(500) + whenever(flagManager.readFlagValue<Int>(eq(5), any())).thenReturn(9519) + + assertThat(mFeatureFlagsDebug.getInt(ResourceIntFlag(1, 1001))).isEqualTo(88) + assertThat(mFeatureFlagsDebug.getInt(ResourceIntFlag(2, 1002))).isEqualTo(61) + assertThat(mFeatureFlagsDebug.getInt(ResourceIntFlag(3, 1003))).isEqualTo(20) + + Assert.assertThrows(NotFoundException::class.java) { + mFeatureFlagsDebug.getInt(ResourceIntFlag(4, 1004)) + } + // Test that resource is loaded (and validated) even when the setting is set. + // This prevents developers from not noticing when they reference an invalid resource. + Assert.assertThrows(NotFoundException::class.java) { + mFeatureFlagsDebug.getInt(ResourceIntFlag(5, 1005)) + } + } + + @Test fun broadcastReceiver_IgnoresInvalidData() { addFlag(UnreleasedFlag(1)) addFlag(ResourceBooleanFlag(2, 1002)) 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 9628ee93ceff..7355319a2fbc 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/flags/FlagCommandTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/flags/FlagCommandTest.kt @@ -35,6 +35,8 @@ class FlagCommandTest : SysuiTestCase() { private val flagMap = mutableMapOf<Int, Flag<*>>() private val flagA = UnreleasedFlag(500) private val flagB = ReleasedFlag(501) + private val stringFlag = StringFlag(502, "abracadabra") + private val intFlag = IntFlag(503, 12) private lateinit var cmd: FlagCommand @@ -44,26 +46,59 @@ class FlagCommandTest : SysuiTestCase() { whenever(featureFlags.isEnabled(any(UnreleasedFlag::class.java))).thenReturn(false) whenever(featureFlags.isEnabled(any(ReleasedFlag::class.java))).thenReturn(true) + whenever(featureFlags.getString(any(StringFlag::class.java))).thenAnswer { invocation -> + (invocation.getArgument(0) as StringFlag).default + } + whenever(featureFlags.getInt(any(IntFlag::class.java))).thenAnswer { invocation -> + (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) cmd = FlagCommand(featureFlags, flagMap) } @Test - fun readFlagCommand() { + fun readBooleanFlagCommand() { cmd.execute(pw, listOf(flagA.id.toString())) Mockito.verify(featureFlags).isEnabled(flagA) } @Test - fun setFlagCommand() { + fun readStringFlagCommand() { + cmd.execute(pw, listOf(stringFlag.id.toString())) + Mockito.verify(featureFlags).getString(stringFlag) + } + + @Test + fun readIntFlag() { + cmd.execute(pw, listOf(intFlag.id.toString())) + Mockito.verify(featureFlags).getInt(intFlag) + } + + @Test + fun setBooleanFlagCommand() { cmd.execute(pw, listOf(flagB.id.toString(), "on")) Mockito.verify(featureFlags).setBooleanFlagInternal(flagB, true) } @Test - fun toggleFlagCommand() { + fun setStringFlagCommand() { + cmd.execute(pw, listOf(stringFlag.id.toString(), "set", "foobar")) + Mockito.verify(featureFlags).setStringFlagInternal(stringFlag, "foobar") + } + + @Test + fun setIntFlag() { + cmd.execute(pw, listOf(intFlag.id.toString(), "put", "123")) + Mockito.verify(featureFlags).setIntFlagInternal(intFlag, 123) + } + + @Test + fun toggleBooleanFlagCommand() { cmd.execute(pw, listOf(flagB.id.toString(), "toggle")) Mockito.verify(featureFlags).setBooleanFlagInternal(flagB, false) } 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 a60b7735fbd4..c132537eeb71 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 @@ -21,6 +21,7 @@ import java.io.PrintWriter class FakeFeatureFlags : FeatureFlags { private val booleanFlags = mutableMapOf<Int, Boolean>() private val stringFlags = mutableMapOf<Int, String>() + private val intFlags = mutableMapOf<Int, Int>() private val knownFlagNames = mutableMapOf<Int, String>() private val flagListeners = mutableMapOf<Int, MutableSet<FlagListenable.Listener>>() private val listenerFlagIds = mutableMapOf<FlagListenable.Listener, MutableSet<Int>>() @@ -95,6 +96,10 @@ class FakeFeatureFlags : FeatureFlags { override fun getString(flag: ResourceStringFlag): String = requireStringValue(flag.id) + override fun getInt(flag: IntFlag): Int = requireIntValue(flag.id) + + override fun getInt(flag: ResourceIntFlag): Int = requireIntValue(flag.id) + override fun addListener(flag: Flag<*>, listener: FlagListenable.Listener) { flagListeners.getOrPut(flag.id) { mutableSetOf() }.add(listener) listenerFlagIds.getOrPut(listener) { mutableSetOf() }.add(flag.id) @@ -118,11 +123,16 @@ class FakeFeatureFlags : FeatureFlags { private fun requireBooleanValue(flagId: Int): Boolean { return booleanFlags[flagId] - ?: error("Flag ${flagName(flagId)} was accessed but not specified.") + ?: error("Flag ${flagName(flagId)} was accessed as boolean but not specified.") } private fun requireStringValue(flagId: Int): String { return stringFlags[flagId] - ?: error("Flag ${flagName(flagId)} was accessed but not specified.") + ?: error("Flag ${flagName(flagId)} was accessed as string but not specified.") + } + + private fun requireIntValue(flagId: Int): Int { + return intFlags[flagId] + ?: error("Flag ${flagName(flagId)} was accessed as int but not specified.") } } |