Only restart SystemUI when Server Flags actually change.
DeviceConfig is alerting us to all server pushes (pulls?). Even if
the flags is checking on aren't actually changing.
This CL ensures we only restart if the flag value has actually
changed.
Fixes: 272780732
Test: atest SystemUITests
Change-Id: I3571d210885472dc69f50c76922a89733f1a5d83
diff --git a/packages/SystemUI/src/com/android/systemui/flags/FeatureFlagsDebug.java b/packages/SystemUI/src/com/android/systemui/flags/FeatureFlagsDebug.java
index 58fe094..367a9b9 100644
--- a/packages/SystemUI/src/com/android/systemui/flags/FeatureFlagsDebug.java
+++ b/packages/SystemUI/src/com/android/systemui/flags/FeatureFlagsDebug.java
@@ -85,9 +85,34 @@
private final ServerFlagReader.ChangeListener mOnPropertiesChanged =
new ServerFlagReader.ChangeListener() {
@Override
- public void onChange(Flag<?> flag) {
- mRestarter.restartSystemUI(
- "Server flag change: " + flag.getNamespace() + "." + flag.getName());
+ public void onChange(Flag<?> flag, String value) {
+ boolean shouldRestart = false;
+ if (mBooleanFlagCache.containsKey(flag.getName())) {
+ boolean newValue = value == null ? false : Boolean.parseBoolean(value);
+ if (mBooleanFlagCache.get(flag.getName()) != newValue) {
+ shouldRestart = true;
+ }
+ } else if (mStringFlagCache.containsKey(flag.getName())) {
+ String newValue = value == null ? "" : value;
+ if (mStringFlagCache.get(flag.getName()) != value) {
+ shouldRestart = true;
+ }
+ } else if (mIntFlagCache.containsKey(flag.getName())) {
+ int newValue = 0;
+ try {
+ newValue = value == null ? 0 : Integer.parseInt(value);
+ } catch (NumberFormatException e) {
+ }
+ if (mIntFlagCache.get(flag.getName()) != newValue) {
+ shouldRestart = true;
+ }
+ }
+ if (shouldRestart) {
+ mRestarter.restartSystemUI(
+ "Server flag change: " + flag.getNamespace() + "."
+ + flag.getName());
+
+ }
}
};
diff --git a/packages/SystemUI/src/com/android/systemui/flags/FeatureFlagsRelease.java b/packages/SystemUI/src/com/android/systemui/flags/FeatureFlagsRelease.java
index 9859ff6..9d19a7d 100644
--- a/packages/SystemUI/src/com/android/systemui/flags/FeatureFlagsRelease.java
+++ b/packages/SystemUI/src/com/android/systemui/flags/FeatureFlagsRelease.java
@@ -53,13 +53,38 @@
private final Map<String, Flag<?>> mAllFlags;
private final Map<String, Boolean> mBooleanCache = new HashMap<>();
private final Map<String, String> mStringCache = new HashMap<>();
+ private final Map<String, Integer> mIntCache = new HashMap<>();
private final ServerFlagReader.ChangeListener mOnPropertiesChanged =
new ServerFlagReader.ChangeListener() {
@Override
- public void onChange(Flag<?> flag) {
- mRestarter.restartSystemUI(
- "Server flag change: " + flag.getNamespace() + "." + flag.getName());
+ public void onChange(Flag<?> flag, String value) {
+ boolean shouldRestart = false;
+ if (mBooleanCache.containsKey(flag.getName())) {
+ boolean newValue = value == null ? false : Boolean.parseBoolean(value);
+ if (mBooleanCache.get(flag.getName()) != newValue) {
+ shouldRestart = true;
+ }
+ } else if (mStringCache.containsKey(flag.getName())) {
+ String newValue = value == null ? "" : value;
+ if (mStringCache.get(flag.getName()) != newValue) {
+ shouldRestart = true;
+ }
+ } else if (mIntCache.containsKey(flag.getName())) {
+ int newValue = 0;
+ try {
+ newValue = value == null ? 0 : Integer.parseInt(value);
+ } catch (NumberFormatException e) {
+ }
+ if (mIntCache.get(flag.getName()) != newValue) {
+ shouldRestart = true;
+ }
+ }
+ if (shouldRestart) {
+ mRestarter.restartSystemUI(
+ "Server flag change: " + flag.getNamespace() + "."
+ + flag.getName());
+ }
}
};
@@ -97,68 +122,97 @@
@Override
public boolean isEnabled(@NotNull ReleasedFlag flag) {
- return mServerFlagReader.readServerOverride(flag.getNamespace(), flag.getName(), true);
+ // Fill the cache.
+ return isEnabledInternal(flag.getName(),
+ mServerFlagReader.readServerOverride(flag.getNamespace(), flag.getName(), true));
}
@Override
public boolean isEnabled(ResourceBooleanFlag flag) {
- if (!mBooleanCache.containsKey(flag.getName())) {
- return isEnabled(flag.getName(), mResources.getBoolean(flag.getResourceId()));
- }
-
- return mBooleanCache.get(flag.getName());
+ // Fill the cache.
+ return isEnabledInternal(flag.getName(), mResources.getBoolean(flag.getResourceId()));
}
@Override
public boolean isEnabled(SysPropBooleanFlag flag) {
- if (!mBooleanCache.containsKey(flag.getName())) {
- return isEnabled(
- flag.getName(),
- mSystemProperties.getBoolean(flag.getName(), flag.getDefault()));
- }
-
- return mBooleanCache.get(flag.getName());
+ // Fill the cache.
+ return isEnabledInternal(
+ flag.getName(),
+ mSystemProperties.getBoolean(flag.getName(), flag.getDefault()));
}
- private boolean isEnabled(String name, boolean defaultValue) {
- mBooleanCache.put(name, defaultValue);
- return defaultValue;
+ /**
+ * Checks and fills the boolean cache. This is important, Always call through to this method!
+ *
+ * We use the cache as a way to decide if we need to restart the process when server-side
+ * changes occur.
+ */
+ private boolean isEnabledInternal(String name, boolean defaultValue) {
+ // Fill the cache.
+ if (!mBooleanCache.containsKey(name)) {
+ mBooleanCache.put(name, defaultValue);
+ }
+
+ return mBooleanCache.get(name);
}
@NonNull
@Override
public String getString(@NonNull StringFlag flag) {
- return getString(flag.getName(), flag.getDefault());
+ // Fill the cache.
+ return getStringInternal(flag.getName(), flag.getDefault());
}
@NonNull
@Override
public String getString(@NonNull ResourceStringFlag flag) {
- if (!mStringCache.containsKey(flag.getName())) {
- return getString(flag.getName(),
- requireNonNull(mResources.getString(flag.getResourceId())));
- }
-
- return mStringCache.get(flag.getName());
+ // Fill the cache.
+ return getStringInternal(flag.getName(),
+ requireNonNull(mResources.getString(flag.getResourceId())));
}
- private String getString(String name, String defaultValue) {
- mStringCache.put(name, defaultValue);
- return defaultValue;
+ /**
+ * Checks and fills the String cache. This is important, Always call through to this method!
+ *
+ * We use the cache as a way to decide if we need to restart the process when server-side
+ * changes occur.
+ */
+ private String getStringInternal(String name, String defaultValue) {
+ if (!mStringCache.containsKey(name)) {
+ mStringCache.put(name, defaultValue);
+ }
+
+ return mStringCache.get(name);
}
@NonNull
@Override
public int getInt(@NonNull IntFlag flag) {
- return flag.getDefault();
+ // Fill the cache.
+ return getIntInternal(flag.getName(), flag.getDefault());
}
@NonNull
@Override
public int getInt(@NonNull ResourceIntFlag flag) {
+ // Fill the cache.
return mResources.getInteger(flag.getResourceId());
}
+ /**
+ * Checks and fills the integer cache. This is important, Always call through to this method!
+ *
+ * We use the cache as a way to decide if we need to restart the process when server-side
+ * changes occur.
+ */
+ private int getIntInternal(String name, int defaultValue) {
+ if (!mIntCache.containsKey(name)) {
+ mIntCache.put(name, defaultValue);
+ }
+
+ return mIntCache.get(name);
+ }
+
@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/ServerFlagReader.kt b/packages/SystemUI/src/com/android/systemui/flags/ServerFlagReader.kt
index 9b748d0..eaf5eac 100644
--- a/packages/SystemUI/src/com/android/systemui/flags/ServerFlagReader.kt
+++ b/packages/SystemUI/src/com/android/systemui/flags/ServerFlagReader.kt
@@ -37,7 +37,7 @@
fun listenForChanges(values: Collection<Flag<*>>, listener: ChangeListener)
interface ChangeListener {
- fun onChange(flag: Flag<*>)
+ fun onChange(flag: Flag<*>, value: String?)
}
}
@@ -67,7 +67,7 @@
propLoop@ for (propName in properties.keyset) {
for (flag in flags) {
if (propName == flag.name) {
- listener.onChange(flag)
+ listener.onChange(flag, properties.getString(propName, null))
break@propLoop
}
}
@@ -144,7 +144,7 @@
for ((listener, flags) in listeners) {
flagLoop@ for (flag in flags) {
if (name == flag.name) {
- listener.onChange(flag)
+ listener.onChange(flag, if (value) "true" else "false")
break@flagLoop
}
}
@@ -159,5 +159,6 @@
flags: Collection<Flag<*>>,
listener: ServerFlagReader.ChangeListener
) {
+ listeners.add(Pair(listener, flags))
}
}
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 2bcd75b..18f7db1 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/flags/FeatureFlagsDebugTest.kt
+++ b/packages/SystemUI/tests/src/com/android/systemui/flags/FeatureFlagsDebugTest.kt
@@ -37,6 +37,7 @@
import org.mockito.Mockito.anyBoolean
import org.mockito.Mockito.anyString
import org.mockito.Mockito.inOrder
+import org.mockito.Mockito.never
import org.mockito.Mockito.times
import org.mockito.Mockito.verify
import org.mockito.Mockito.verifyNoMoreInteractions
@@ -53,7 +54,7 @@
*/
@SmallTest
class FeatureFlagsDebugTest : SysuiTestCase() {
- private lateinit var mFeatureFlagsDebug: FeatureFlagsDebug
+ private lateinit var featureFlagsDebug: FeatureFlagsDebug
@Mock
private lateinit var flagManager: FlagManager
@@ -85,7 +86,7 @@
flagMap.put(Flags.TEAMFOOD.name, Flags.TEAMFOOD)
flagMap.put(teamfoodableFlagA.name, teamfoodableFlagA)
flagMap.put(teamfoodableFlagB.name, teamfoodableFlagB)
- mFeatureFlagsDebug = FeatureFlagsDebug(
+ featureFlagsDebug = FeatureFlagsDebug(
flagManager,
mockContext,
globalSettings,
@@ -95,7 +96,7 @@
flagMap,
restarter
)
- mFeatureFlagsDebug.init()
+ featureFlagsDebug.init()
verify(flagManager).onSettingsChangedAction = any()
broadcastReceiver = withArgCaptor {
verify(mockContext).registerReceiver(
@@ -116,7 +117,7 @@
whenever(flagManager.readFlagValue<Boolean>(eq("4"), any())).thenReturn(false)
assertThat(
- mFeatureFlagsDebug.isEnabled(
+ featureFlagsDebug.isEnabled(
ReleasedFlag(
2,
name = "2",
@@ -125,7 +126,7 @@
)
).isTrue()
assertThat(
- mFeatureFlagsDebug.isEnabled(
+ featureFlagsDebug.isEnabled(
UnreleasedFlag(
3,
name = "3",
@@ -134,7 +135,7 @@
)
).isTrue()
assertThat(
- mFeatureFlagsDebug.isEnabled(
+ featureFlagsDebug.isEnabled(
ReleasedFlag(
4,
name = "4",
@@ -143,7 +144,7 @@
)
).isFalse()
assertThat(
- mFeatureFlagsDebug.isEnabled(
+ featureFlagsDebug.isEnabled(
UnreleasedFlag(
5,
name = "5",
@@ -157,8 +158,8 @@
fun teamFoodFlag_False() {
whenever(flagManager.readFlagValue<Boolean>(
eq(Flags.TEAMFOOD.name), any())).thenReturn(false)
- assertThat(mFeatureFlagsDebug.isEnabled(teamfoodableFlagA)).isFalse()
- assertThat(mFeatureFlagsDebug.isEnabled(teamfoodableFlagB)).isTrue()
+ assertThat(featureFlagsDebug.isEnabled(teamfoodableFlagA)).isFalse()
+ assertThat(featureFlagsDebug.isEnabled(teamfoodableFlagB)).isTrue()
// Regular boolean flags should still test the same.
// Only our teamfoodableFlag should change.
@@ -169,8 +170,8 @@
fun teamFoodFlag_True() {
whenever(flagManager.readFlagValue<Boolean>(
eq(Flags.TEAMFOOD.name), any())).thenReturn(true)
- assertThat(mFeatureFlagsDebug.isEnabled(teamfoodableFlagA)).isTrue()
- assertThat(mFeatureFlagsDebug.isEnabled(teamfoodableFlagB)).isTrue()
+ assertThat(featureFlagsDebug.isEnabled(teamfoodableFlagA)).isTrue()
+ assertThat(featureFlagsDebug.isEnabled(teamfoodableFlagB)).isTrue()
// Regular boolean flags should still test the same.
// Only our teamfoodableFlag should change.
@@ -185,8 +186,8 @@
.thenReturn(false)
whenever(flagManager.readFlagValue<Boolean>(
eq(Flags.TEAMFOOD.name), any())).thenReturn(true)
- assertThat(mFeatureFlagsDebug.isEnabled(teamfoodableFlagA)).isTrue()
- assertThat(mFeatureFlagsDebug.isEnabled(teamfoodableFlagB)).isFalse()
+ assertThat(featureFlagsDebug.isEnabled(teamfoodableFlagA)).isTrue()
+ assertThat(featureFlagsDebug.isEnabled(teamfoodableFlagB)).isFalse()
// Regular boolean flags should still test the same.
// Only our teamfoodableFlag should change.
@@ -205,7 +206,7 @@
whenever(flagManager.readFlagValue<Boolean>(eq("5"), any())).thenReturn(false)
assertThat(
- mFeatureFlagsDebug.isEnabled(
+ featureFlagsDebug.isEnabled(
ResourceBooleanFlag(
1,
"1",
@@ -214,16 +215,16 @@
)
)
).isFalse()
- assertThat(mFeatureFlagsDebug.isEnabled(ResourceBooleanFlag(2, "2", "test", 1002))).isTrue()
- assertThat(mFeatureFlagsDebug.isEnabled(ResourceBooleanFlag(3, "3", "test", 1003))).isTrue()
+ assertThat(featureFlagsDebug.isEnabled(ResourceBooleanFlag(2, "2", "test", 1002))).isTrue()
+ assertThat(featureFlagsDebug.isEnabled(ResourceBooleanFlag(3, "3", "test", 1003))).isTrue()
Assert.assertThrows(NameNotFoundException::class.java) {
- mFeatureFlagsDebug.isEnabled(ResourceBooleanFlag(4, "4", "test", 1004))
+ featureFlagsDebug.isEnabled(ResourceBooleanFlag(4, "4", "test", 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(NameNotFoundException::class.java) {
- mFeatureFlagsDebug.isEnabled(ResourceBooleanFlag(5, "5", "test", 1005))
+ featureFlagsDebug.isEnabled(ResourceBooleanFlag(5, "5", "test", 1005))
}
}
@@ -236,11 +237,11 @@
return@thenAnswer it.getArgument(1)
}
- assertThat(mFeatureFlagsDebug.isEnabled(SysPropBooleanFlag(1, "a", "test"))).isFalse()
- assertThat(mFeatureFlagsDebug.isEnabled(SysPropBooleanFlag(2, "b", "test"))).isTrue()
- assertThat(mFeatureFlagsDebug.isEnabled(SysPropBooleanFlag(3, "c", "test", true))).isTrue()
+ assertThat(featureFlagsDebug.isEnabled(SysPropBooleanFlag(1, "a", "test"))).isFalse()
+ assertThat(featureFlagsDebug.isEnabled(SysPropBooleanFlag(2, "b", "test"))).isTrue()
+ assertThat(featureFlagsDebug.isEnabled(SysPropBooleanFlag(3, "c", "test", true))).isTrue()
assertThat(
- mFeatureFlagsDebug.isEnabled(
+ featureFlagsDebug.isEnabled(
SysPropBooleanFlag(
4,
"d",
@@ -249,17 +250,17 @@
)
)
).isFalse()
- assertThat(mFeatureFlagsDebug.isEnabled(SysPropBooleanFlag(5, "e", "test"))).isFalse()
+ assertThat(featureFlagsDebug.isEnabled(SysPropBooleanFlag(5, "e", "test"))).isFalse()
}
@Test
fun readStringFlag() {
whenever(flagManager.readFlagValue<String>(eq("3"), any())).thenReturn("foo")
whenever(flagManager.readFlagValue<String>(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")
- assertThat(mFeatureFlagsDebug.getString(StringFlag(4, "4", "test", "buz"))).isEqualTo("bar")
+ assertThat(featureFlagsDebug.getString(StringFlag(1, "1", "test", "biz"))).isEqualTo("biz")
+ assertThat(featureFlagsDebug.getString(StringFlag(2, "2", "test", "baz"))).isEqualTo("baz")
+ assertThat(featureFlagsDebug.getString(StringFlag(3, "3", "test", "buz"))).isEqualTo("foo")
+ assertThat(featureFlagsDebug.getString(StringFlag(4, "4", "test", "buz"))).isEqualTo("bar")
}
@Test
@@ -276,7 +277,7 @@
whenever(flagManager.readFlagValue<String>(eq("6"), any())).thenReturn("override6")
assertThat(
- mFeatureFlagsDebug.getString(
+ featureFlagsDebug.getString(
ResourceStringFlag(
1,
"1",
@@ -286,7 +287,7 @@
)
).isEqualTo("")
assertThat(
- mFeatureFlagsDebug.getString(
+ featureFlagsDebug.getString(
ResourceStringFlag(
2,
"2",
@@ -296,7 +297,7 @@
)
).isEqualTo("resource2")
assertThat(
- mFeatureFlagsDebug.getString(
+ featureFlagsDebug.getString(
ResourceStringFlag(
3,
"3",
@@ -307,15 +308,15 @@
).isEqualTo("override3")
Assert.assertThrows(NullPointerException::class.java) {
- mFeatureFlagsDebug.getString(ResourceStringFlag(4, "4", "test", 1004))
+ featureFlagsDebug.getString(ResourceStringFlag(4, "4", "test", 1004))
}
Assert.assertThrows(NameNotFoundException::class.java) {
- mFeatureFlagsDebug.getString(ResourceStringFlag(5, "5", "test", 1005))
+ featureFlagsDebug.getString(ResourceStringFlag(5, "5", "test", 1005))
}
// 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(NameNotFoundException::class.java) {
- mFeatureFlagsDebug.getString(ResourceStringFlag(6, "6", "test", 1005))
+ featureFlagsDebug.getString(ResourceStringFlag(6, "6", "test", 1005))
}
}
@@ -323,10 +324,10 @@
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, "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)
- assertThat(mFeatureFlagsDebug.getInt(IntFlag(4, "4", "test", 234))).isEqualTo(48)
+ assertThat(featureFlagsDebug.getInt(IntFlag(1, "1", "test", 12))).isEqualTo(12)
+ assertThat(featureFlagsDebug.getInt(IntFlag(2, "2", "test", 93))).isEqualTo(93)
+ assertThat(featureFlagsDebug.getInt(IntFlag(3, "3", "test", 8))).isEqualTo(22)
+ assertThat(featureFlagsDebug.getInt(IntFlag(4, "4", "test", 234))).isEqualTo(48)
}
@Test
@@ -342,17 +343,17 @@
whenever(flagManager.readFlagValue<Int>(eq(4), any())).thenReturn(500)
whenever(flagManager.readFlagValue<Int>(eq(5), any())).thenReturn(9519)
- assertThat(mFeatureFlagsDebug.getInt(ResourceIntFlag(1, "1", "test", 1001))).isEqualTo(88)
- assertThat(mFeatureFlagsDebug.getInt(ResourceIntFlag(2, "2", "test", 1002))).isEqualTo(61)
- assertThat(mFeatureFlagsDebug.getInt(ResourceIntFlag(3, "3", "test", 1003))).isEqualTo(20)
+ assertThat(featureFlagsDebug.getInt(ResourceIntFlag(1, "1", "test", 1001))).isEqualTo(88)
+ assertThat(featureFlagsDebug.getInt(ResourceIntFlag(2, "2", "test", 1002))).isEqualTo(61)
+ assertThat(featureFlagsDebug.getInt(ResourceIntFlag(3, "3", "test", 1003))).isEqualTo(20)
Assert.assertThrows(NotFoundException::class.java) {
- mFeatureFlagsDebug.getInt(ResourceIntFlag(4, "4", "test", 1004))
+ featureFlagsDebug.getInt(ResourceIntFlag(4, "4", "test", 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, "5", "test", 1005))
+ featureFlagsDebug.getInt(ResourceIntFlag(5, "5", "test", 1005))
}
}
@@ -432,11 +433,11 @@
whenever(flagManager.readFlagValue<String>(eq("1"), any())).thenReturn("original")
// gets the flag & cache it
- assertThat(mFeatureFlagsDebug.getString(flag1)).isEqualTo("original")
+ assertThat(featureFlagsDebug.getString(flag1)).isEqualTo("original")
verify(flagManager, times(1)).readFlagValue(eq("1"), eq(StringFlagSerializer))
// hit the cache
- assertThat(mFeatureFlagsDebug.getString(flag1)).isEqualTo("original")
+ assertThat(featureFlagsDebug.getString(flag1)).isEqualTo("original")
verifyNoMoreInteractions(flagManager)
// set the flag
@@ -444,7 +445,7 @@
verifyPutData("1", "{\"type\":\"string\",\"value\":\"new\"}", numReads = 2)
whenever(flagManager.readFlagValue<String>(eq("1"), any())).thenReturn("new")
- assertThat(mFeatureFlagsDebug.getString(flag1)).isEqualTo("new")
+ assertThat(featureFlagsDebug.getString(flag1)).isEqualTo("new")
verify(flagManager, times(3)).readFlagValue(eq("1"), eq(StringFlagSerializer))
}
@@ -454,7 +455,7 @@
serverFlagReader.setFlagValue(flag.namespace, flag.name, false)
- assertThat(mFeatureFlagsDebug.isEnabled(flag)).isFalse()
+ assertThat(featureFlagsDebug.isEnabled(flag)).isFalse()
}
@Test
@@ -462,7 +463,33 @@
val flag = UnreleasedFlag(100, name = "100", namespace = "test")
serverFlagReader.setFlagValue(flag.namespace, flag.name, true)
- assertThat(mFeatureFlagsDebug.isEnabled(flag)).isTrue()
+ assertThat(featureFlagsDebug.isEnabled(flag)).isTrue()
+ }
+
+ @Test
+ fun serverSide_OverrideUncached_NoRestart() {
+ // No one has read the flag, so it's not in the cache.
+ serverFlagReader.setFlagValue(
+ teamfoodableFlagA.namespace, teamfoodableFlagA.name, !teamfoodableFlagA.default)
+ verify(restarter, never()).restartSystemUI(anyString())
+ }
+
+ @Test
+ fun serverSide_Override_Restarts() {
+ // Read it to put it in the cache.
+ featureFlagsDebug.isEnabled(teamfoodableFlagA)
+ serverFlagReader.setFlagValue(
+ teamfoodableFlagA.namespace, teamfoodableFlagA.name, !teamfoodableFlagA.default)
+ verify(restarter).restartSystemUI(anyString())
+ }
+
+ @Test
+ fun serverSide_RedundantOverride_NoRestart() {
+ // Read it to put it in the cache.
+ featureFlagsDebug.isEnabled(teamfoodableFlagA)
+ serverFlagReader.setFlagValue(
+ teamfoodableFlagA.namespace, teamfoodableFlagA.name, teamfoodableFlagA.default)
+ verify(restarter, never()).restartSystemUI(anyString())
}
@Test
@@ -482,13 +509,13 @@
.thenReturn("override7")
// WHEN the flags have been accessed
- assertThat(mFeatureFlagsDebug.isEnabled(flag1)).isTrue()
- assertThat(mFeatureFlagsDebug.isEnabled(flag2)).isTrue()
- assertThat(mFeatureFlagsDebug.isEnabled(flag3)).isFalse()
- assertThat(mFeatureFlagsDebug.getString(flag4)).isEmpty()
- assertThat(mFeatureFlagsDebug.getString(flag5)).isEqualTo("flag5default")
- assertThat(mFeatureFlagsDebug.getString(flag6)).isEqualTo("resource1006")
- assertThat(mFeatureFlagsDebug.getString(flag7)).isEqualTo("override7")
+ assertThat(featureFlagsDebug.isEnabled(flag1)).isTrue()
+ assertThat(featureFlagsDebug.isEnabled(flag2)).isTrue()
+ assertThat(featureFlagsDebug.isEnabled(flag3)).isFalse()
+ assertThat(featureFlagsDebug.getString(flag4)).isEmpty()
+ assertThat(featureFlagsDebug.getString(flag5)).isEqualTo("flag5default")
+ assertThat(featureFlagsDebug.getString(flag6)).isEqualTo("resource1006")
+ assertThat(featureFlagsDebug.getString(flag7)).isEqualTo("override7")
// THEN the dump contains the flags and the default values
val dump = dumpToString()
@@ -527,7 +554,7 @@
private fun dumpToString(): String {
val sw = StringWriter()
val pw = PrintWriter(sw)
- mFeatureFlagsDebug.dump(pw, emptyArray<String>())
+ featureFlagsDebug.dump(pw, emptyArray<String>())
pw.flush()
return sw.toString()
}
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 4c6028c..917147b 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/flags/FeatureFlagsReleaseTest.kt
+++ b/packages/SystemUI/tests/src/com/android/systemui/flags/FeatureFlagsReleaseTest.kt
@@ -24,6 +24,8 @@
import org.junit.Before
import org.junit.Test
import org.mockito.Mock
+import org.mockito.Mockito
+import org.mockito.Mockito.never
import org.mockito.MockitoAnnotations
import org.mockito.Mockito.`when` as whenever
@@ -33,7 +35,7 @@
*/
@SmallTest
class FeatureFlagsReleaseTest : SysuiTestCase() {
- private lateinit var mFeatureFlagsRelease: FeatureFlagsRelease
+ private lateinit var featureFlagsRelease: FeatureFlagsRelease
@Mock private lateinit var mResources: Resources
@Mock private lateinit var mSystemProperties: SystemPropertiesHelper
@@ -41,15 +43,21 @@
private val flagMap = mutableMapOf<String, Flag<*>>()
private val serverFlagReader = ServerFlagReaderFake()
+
+ private val flagA = ReleasedFlag(501, name = "a", namespace = "test")
+
@Before
fun setup() {
MockitoAnnotations.initMocks(this)
- mFeatureFlagsRelease = FeatureFlagsRelease(
+ flagMap.put(flagA.name, flagA)
+ featureFlagsRelease = FeatureFlagsRelease(
mResources,
mSystemProperties,
serverFlagReader,
flagMap,
restarter)
+
+ featureFlagsRelease.init()
}
@Test
@@ -60,7 +68,7 @@
val flagNamespace = "test"
val flag = ResourceBooleanFlag(flagId, flagName, flagNamespace, flagResourceId)
whenever(mResources.getBoolean(flagResourceId)).thenReturn(true)
- assertThat(mFeatureFlagsRelease.isEnabled(flag)).isTrue()
+ assertThat(featureFlagsRelease.isEnabled(flag)).isTrue()
}
@Test
@@ -70,16 +78,16 @@
whenever(mResources.getString(1003)).thenReturn(null)
whenever(mResources.getString(1004)).thenAnswer { throw NameNotFoundException() }
- assertThat(mFeatureFlagsRelease.getString(
+ assertThat(featureFlagsRelease.getString(
ResourceStringFlag(1, "1", "test", 1001))).isEqualTo("")
- assertThat(mFeatureFlagsRelease.getString(
+ assertThat(featureFlagsRelease.getString(
ResourceStringFlag(2, "2", "test", 1002))).isEqualTo("res2")
assertThrows(NullPointerException::class.java) {
- mFeatureFlagsRelease.getString(ResourceStringFlag(3, "3", "test", 1003))
+ featureFlagsRelease.getString(ResourceStringFlag(3, "3", "test", 1003))
}
assertThrows(NameNotFoundException::class.java) {
- mFeatureFlagsRelease.getString(ResourceStringFlag(4, "4", "test", 1004))
+ featureFlagsRelease.getString(ResourceStringFlag(4, "4", "test", 1004))
}
}
@@ -92,7 +100,7 @@
val flag = SysPropBooleanFlag(flagId, flagName, flagNamespace, flagDefault)
whenever(mSystemProperties.getBoolean(flagName, flagDefault)).thenReturn(flagDefault)
- assertThat(mFeatureFlagsRelease.isEnabled(flag)).isEqualTo(flagDefault)
+ assertThat(featureFlagsRelease.isEnabled(flag)).isEqualTo(flagDefault)
}
@Test
@@ -101,7 +109,7 @@
serverFlagReader.setFlagValue(flag.namespace, flag.name, false)
- assertThat(mFeatureFlagsRelease.isEnabled(flag)).isFalse()
+ assertThat(featureFlagsRelease.isEnabled(flag)).isFalse()
}
@Test
@@ -110,6 +118,32 @@
serverFlagReader.setFlagValue(flag.namespace, flag.name, true)
- assertThat(mFeatureFlagsRelease.isEnabled(flag)).isFalse()
+ assertThat(featureFlagsRelease.isEnabled(flag)).isFalse()
+ }
+
+ @Test
+ fun serverSide_OverrideUncached_NoRestart() {
+ // No one has read the flag, so it's not in the cache.
+ serverFlagReader.setFlagValue(
+ flagA.namespace, flagA.name, !flagA.default)
+ Mockito.verify(restarter, never()).restartSystemUI(Mockito.anyString())
+ }
+
+ @Test
+ fun serverSide_Override_Restarts() {
+ // Read it to put it in the cache.
+ featureFlagsRelease.isEnabled(flagA)
+ serverFlagReader.setFlagValue(
+ flagA.namespace, flagA.name, !flagA.default)
+ Mockito.verify(restarter).restartSystemUI(Mockito.anyString())
+ }
+
+ @Test
+ fun serverSide_RedundantOverride_NoRestart() {
+ // Read it to put it in the cache.
+ featureFlagsRelease.isEnabled(flagA)
+ serverFlagReader.setFlagValue(
+ flagA.namespace, flagA.name, flagA.default)
+ Mockito.verify(restarter, never()).restartSystemUI(Mockito.anyString())
}
}
diff --git a/packages/SystemUI/tests/src/com/android/systemui/flags/ServerFlagReaderImplTest.kt b/packages/SystemUI/tests/src/com/android/systemui/flags/ServerFlagReaderImplTest.kt
index 2e98006..953b7fb 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/flags/ServerFlagReaderImplTest.kt
+++ b/packages/SystemUI/tests/src/com/android/systemui/flags/ServerFlagReaderImplTest.kt
@@ -21,11 +21,13 @@
import com.android.systemui.SysuiTestCase
import com.android.systemui.util.DeviceConfigProxyFake
import com.android.systemui.util.concurrency.FakeExecutor
+import com.android.systemui.util.mockito.any
import com.android.systemui.util.time.FakeSystemClock
import org.junit.Before
import org.junit.Test
import org.junit.runner.RunWith
import org.mockito.Mock
+import org.mockito.Mockito.anyString
import org.mockito.Mockito.never
import org.mockito.Mockito.verify
import org.mockito.MockitoAnnotations
@@ -57,18 +59,18 @@
deviceConfig.setProperty(NAMESPACE, "flag_1", "1", false)
executor.runAllReady()
- verify(changeListener).onChange(flag)
+ verify(changeListener).onChange(flag, "1")
}
@Test
fun testChange_ignoresListenersDuringTest() {
val serverFlagReader = ServerFlagReaderImpl(NAMESPACE, deviceConfig, executor, true)
- val flag = ReleasedFlag(1, "1", "test")
+ val flag = ReleasedFlag(1, "1", " test")
serverFlagReader.listenForChanges(listOf(flag), changeListener)
deviceConfig.setProperty(NAMESPACE, "flag_override_1", "1", false)
executor.runAllReady()
- verify(changeListener, never()).onChange(flag)
+ verify(changeListener, never()).onChange(any(), anyString())
}
}
