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
This commit is contained in:
Dave Mankoff
2023-03-10 22:31:31 +00:00
parent 767ab1e8c4
commit 35d8b91667
6 changed files with 247 additions and 104 deletions

View File

@@ -85,9 +85,34 @@ public class FeatureFlagsDebug implements FeatureFlags {
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());
}
}
};

View File

@@ -53,13 +53,38 @@ public class FeatureFlagsRelease implements FeatureFlags {
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 @@ public class FeatureFlagsRelease implements FeatureFlags {
@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");

View File

@@ -37,7 +37,7 @@ interface ServerFlagReader {
fun listenForChanges(values: Collection<Flag<*>>, listener: ChangeListener)
interface ChangeListener {
fun onChange(flag: Flag<*>)
fun onChange(flag: Flag<*>, value: String?)
}
}
@@ -67,7 +67,7 @@ class ServerFlagReaderImpl @Inject constructor(
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 @@ class ServerFlagReaderFake : ServerFlagReader {
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 @@ class ServerFlagReaderFake : ServerFlagReader {
flags: Collection<Flag<*>>,
listener: ServerFlagReader.ChangeListener
) {
listeners.add(Pair(listener, flags))
}
}

View File

@@ -37,6 +37,7 @@ import org.mockito.Mock
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 @@ import org.mockito.Mockito.`when` as whenever
*/
@SmallTest
class FeatureFlagsDebugTest : SysuiTestCase() {
private lateinit var mFeatureFlagsDebug: FeatureFlagsDebug
private lateinit var featureFlagsDebug: FeatureFlagsDebug
@Mock
private lateinit var flagManager: FlagManager
@@ -85,7 +86,7 @@ class FeatureFlagsDebugTest : SysuiTestCase() {
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 @@ class FeatureFlagsDebugTest : SysuiTestCase() {
flagMap,
restarter
)
mFeatureFlagsDebug.init()
featureFlagsDebug.init()
verify(flagManager).onSettingsChangedAction = any()
broadcastReceiver = withArgCaptor {
verify(mockContext).registerReceiver(
@@ -116,7 +117,7 @@ class FeatureFlagsDebugTest : SysuiTestCase() {
whenever(flagManager.readFlagValue<Boolean>(eq("4"), any())).thenReturn(false)
assertThat(
mFeatureFlagsDebug.isEnabled(
featureFlagsDebug.isEnabled(
ReleasedFlag(
2,
name = "2",
@@ -125,7 +126,7 @@ class FeatureFlagsDebugTest : SysuiTestCase() {
)
).isTrue()
assertThat(
mFeatureFlagsDebug.isEnabled(
featureFlagsDebug.isEnabled(
UnreleasedFlag(
3,
name = "3",
@@ -134,7 +135,7 @@ class FeatureFlagsDebugTest : SysuiTestCase() {
)
).isTrue()
assertThat(
mFeatureFlagsDebug.isEnabled(
featureFlagsDebug.isEnabled(
ReleasedFlag(
4,
name = "4",
@@ -143,7 +144,7 @@ class FeatureFlagsDebugTest : SysuiTestCase() {
)
).isFalse()
assertThat(
mFeatureFlagsDebug.isEnabled(
featureFlagsDebug.isEnabled(
UnreleasedFlag(
5,
name = "5",
@@ -157,8 +158,8 @@ class FeatureFlagsDebugTest : SysuiTestCase() {
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 @@ class FeatureFlagsDebugTest : SysuiTestCase() {
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 @@ class FeatureFlagsDebugTest : SysuiTestCase() {
.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 @@ class FeatureFlagsDebugTest : SysuiTestCase() {
whenever(flagManager.readFlagValue<Boolean>(eq("5"), any())).thenReturn(false)
assertThat(
mFeatureFlagsDebug.isEnabled(
featureFlagsDebug.isEnabled(
ResourceBooleanFlag(
1,
"1",
@@ -214,16 +215,16 @@ class FeatureFlagsDebugTest : SysuiTestCase() {
)
)
).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 @@ class FeatureFlagsDebugTest : SysuiTestCase() {
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 @@ class FeatureFlagsDebugTest : SysuiTestCase() {
)
)
).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 @@ class FeatureFlagsDebugTest : SysuiTestCase() {
whenever(flagManager.readFlagValue<String>(eq("6"), any())).thenReturn("override6")
assertThat(
mFeatureFlagsDebug.getString(
featureFlagsDebug.getString(
ResourceStringFlag(
1,
"1",
@@ -286,7 +287,7 @@ class FeatureFlagsDebugTest : SysuiTestCase() {
)
).isEqualTo("")
assertThat(
mFeatureFlagsDebug.getString(
featureFlagsDebug.getString(
ResourceStringFlag(
2,
"2",
@@ -296,7 +297,7 @@ class FeatureFlagsDebugTest : SysuiTestCase() {
)
).isEqualTo("resource2")
assertThat(
mFeatureFlagsDebug.getString(
featureFlagsDebug.getString(
ResourceStringFlag(
3,
"3",
@@ -307,15 +308,15 @@ class FeatureFlagsDebugTest : SysuiTestCase() {
).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 @@ class FeatureFlagsDebugTest : SysuiTestCase() {
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 @@ class FeatureFlagsDebugTest : SysuiTestCase() {
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 @@ class FeatureFlagsDebugTest : SysuiTestCase() {
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 @@ class FeatureFlagsDebugTest : SysuiTestCase() {
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 @@ class FeatureFlagsDebugTest : SysuiTestCase() {
serverFlagReader.setFlagValue(flag.namespace, flag.name, false)
assertThat(mFeatureFlagsDebug.isEnabled(flag)).isFalse()
assertThat(featureFlagsDebug.isEnabled(flag)).isFalse()
}
@Test
@@ -462,7 +463,33 @@ class FeatureFlagsDebugTest : SysuiTestCase() {
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 @@ class FeatureFlagsDebugTest : SysuiTestCase() {
.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 @@ class FeatureFlagsDebugTest : SysuiTestCase() {
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()
}

View File

@@ -24,6 +24,8 @@ import org.junit.Assert.assertThrows
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 @@ import org.mockito.Mockito.`when` as whenever
*/
@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 @@ class FeatureFlagsReleaseTest : SysuiTestCase() {
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 @@ class FeatureFlagsReleaseTest : SysuiTestCase() {
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 @@ class FeatureFlagsReleaseTest : SysuiTestCase() {
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 @@ class FeatureFlagsReleaseTest : SysuiTestCase() {
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 @@ class FeatureFlagsReleaseTest : SysuiTestCase() {
serverFlagReader.setFlagValue(flag.namespace, flag.name, false)
assertThat(mFeatureFlagsRelease.isEnabled(flag)).isFalse()
assertThat(featureFlagsRelease.isEnabled(flag)).isFalse()
}
@Test
@@ -110,6 +118,32 @@ class FeatureFlagsReleaseTest : SysuiTestCase() {
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())
}
}

View File

@@ -21,11 +21,13 @@ import android.testing.AndroidTestingRunner
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 @@ class ServerFlagReaderImplTest : SysuiTestCase() {
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())
}
}