diff options
| author | 2022-11-02 22:11:23 +0000 | |
|---|---|---|
| committer | 2022-11-03 15:42:17 +0000 | |
| commit | f3df059ebeac734ba7c9efd5dbecde04cd4705b5 (patch) | |
| tree | 157aa89f75b60118d24de7100841af48b2ad9d23 | |
| parent | 847d0f53e7e84c181b75f7fe1fc0c057c574758d (diff) | |
Enable Int and String flags.
Puts mechanics in to support IntFlag and ResourceIntFlag.
Also adds support for both IntFlag and StringFlag via the
command line.
Support in the app will come later.
Bug: 257066497
Test: manual via the command line.
Change-Id: I378f0f1e0db028cee33f729c47a63262124389d7
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.") } } |