Remove flag plugin with flag manager with DI

Test: FeatureFlagsTest
Bug: 196602427
Change-Id: I43961a091f0f10ebd1a168f40703f0067d9f8a18
Merged-In: I43961a091f0f10ebd1a168f40703f0067d9f8a18
This commit is contained in:
Jay Aliomer
2021-09-28 13:23:51 -04:00
committed by Dave Mankoff
parent 2a482869f7
commit 5eac15cf85
9 changed files with 102 additions and 143 deletions

View File

@@ -15,15 +15,19 @@
*/
package com.android.systemui.flags;
import android.util.ArraySet;
import com.android.systemui.dagger.SysUISingleton;
import javax.inject.Inject;
/**
* Concrete implementation of the a Flag manager that returns default values for debug builds
*/
@SysUISingleton
public class FeatureFlagManager {
public class FeatureFlagManager implements FlagReader, FlagWriter {
@Inject
public FeatureFlagManager() {}
public boolean isEnabled(int key, boolean defaultValue) {
return isEnabled(Integer.toString(key), defaultValue);
}
@@ -40,4 +44,9 @@ public class FeatureFlagManager {
public void setEnabled(String key, boolean value) {
// TODO
}
}
public void addListener(Listener run) {}
public void removeListener(Listener run) {}
}

View File

@@ -38,7 +38,10 @@ import com.android.systemui.dagger.qualifiers.Main;
import com.android.systemui.demomode.dagger.DemoModeModule;
import com.android.systemui.doze.dagger.DozeComponent;
import com.android.systemui.dump.DumpManager;
import com.android.systemui.flags.FeatureFlagManager;
import com.android.systemui.flags.FeatureFlags;
import com.android.systemui.flags.FlagReader;
import com.android.systemui.flags.FlagWriter;
import com.android.systemui.fragments.FragmentService;
import com.android.systemui.log.dagger.LogModule;
import com.android.systemui.model.SysUiState;
@@ -149,6 +152,12 @@ public abstract class SystemUIModule {
return state;
}
@Binds
abstract FlagReader provideFlagReader(FeatureFlagManager impl);
@Binds
abstract FlagWriter provideFlagWriter(FeatureFlagManager impl);
@BindsOptionalOf
abstract CommandQueue optionalCommandQueue();

View File

@@ -18,19 +18,21 @@ package com.android.systemui.flags;
import com.android.systemui.dagger.SysUISingleton;
import javax.inject.Inject;
/**
* Default implementation of the a Flag manager that returns default values for release builds
*/
@SysUISingleton
public class FeatureFlagManager {
public boolean getBoolean(int key, boolean defaultValue) {
public class FeatureFlagManager implements FlagReader, FlagWriter {
@Inject
public FeatureFlagManager() {}
public boolean isEnabled(String key, boolean defaultValue) {
return defaultValue;
}
public void setBoolean(int key, boolean value) {}
public boolean getBoolean(String key, boolean defaultValue) {
public boolean isEnabled(int key, boolean defaultValue) {
return defaultValue;
}
public void setBoolean(String key, boolean value) {}
public void addFlagChangedListener(Runnable run) {}
public void removeFlagUpdatedListener(Runnable run) {}
public void setEnabled(String key, boolean value) {}
public void setEnabled(int key, boolean value) {}
}

View File

@@ -16,26 +16,29 @@
package com.android.systemui.flags;
import android.content.Context;
import android.content.res.Resources;
import android.util.SparseArray;
import androidx.annotation.BoolRes;
import androidx.annotation.NonNull;
import androidx.annotation.Nullable;
import com.android.systemui.Dumpable;
import com.android.systemui.R;
import com.android.systemui.dagger.SysUISingleton;
import com.android.systemui.dagger.qualifiers.Main;
import com.android.systemui.plugins.FlagReaderPlugin;
import com.android.systemui.plugins.PluginListener;
import com.android.systemui.shared.plugins.PluginManager;
import com.android.systemui.dump.DumpManager;
import com.android.systemui.util.wrapper.BuildInfo;
import java.io.FileDescriptor;
import java.io.PrintWriter;
import java.util.ArrayList;
import javax.inject.Inject;
/**
* Reads and caches feature flags for quick access
*
* Feature flags must be defined as boolean resources. For example:
* Feature flags must be defined as boolean resources. For example:t
*
* {@code
* <bool name="flag_foo_bar_baz">false</bool>
@@ -55,71 +58,39 @@ import javax.inject.Inject;
* Calls to this class should probably be wrapped by a method in {@link FeatureFlags}.
*/
@SysUISingleton
public class FeatureFlagReader {
public class FeatureFlagReader implements Dumpable {
private final Resources mResources;
private final boolean mAreFlagsOverrideable;
private final PluginManager mPluginManager;
private final SystemPropertiesHelper mSystemPropertiesHelper;
private final SparseArray<CachedFlag> mCachedFlags = new SparseArray<>();
private FlagReaderPlugin mPlugin = new FlagReaderPlugin(){};
private final FlagReader mFlagReader;
@Inject
public FeatureFlagReader(
@Main Resources resources,
BuildInfo build,
PluginManager pluginManager,
SystemPropertiesHelper systemPropertiesHelper) {
DumpManager dumpManager,
SystemPropertiesHelper systemPropertiesHelper,
FlagReader reader) {
mResources = resources;
mPluginManager = pluginManager;
mFlagReader = reader;
mSystemPropertiesHelper = systemPropertiesHelper;
mAreFlagsOverrideable =
build.isDebuggable() && mResources.getBoolean(R.bool.are_flags_overrideable);
mPluginManager.addPluginListener(mPluginListener, FlagReaderPlugin.class);
dumpManager.registerDumpable("FeatureFlags", this);
}
private final PluginListener<FlagReaderPlugin> mPluginListener =
new PluginListener<FlagReaderPlugin>() {
public void onPluginConnected(FlagReaderPlugin plugin, Context context) {
mPlugin = plugin;
}
public void onPluginDisconnected(FlagReaderPlugin plugin) {
mPlugin = new FlagReaderPlugin() {};
}
};
boolean isEnabled(BooleanFlag flag) {
return mPlugin.isEnabled(flag.getId(), flag.getDefault());
return mFlagReader.isEnabled(flag.getId(), flag.getDefault());
}
String getValue(StringFlag flag) {
return mPlugin.getValue(flag.getId(), flag.getDefault());
void addListener(FlagReader.Listener listener) {
mFlagReader.addListener(listener);
}
int getValue(IntFlag flag) {
return mPlugin.getValue(flag.getId(), flag.getDefault());
}
long getValue(LongFlag flag) {
return mPlugin.getValue(flag.getId(), flag.getDefault());
}
float getValue(FloatFlag flag) {
return mPlugin.getValue(flag.getId(), flag.getDefault());
}
double getValue(DoubleFlag flag) {
return mPlugin.getValue(flag.getId(), flag.getDefault());
}
void addListener(FlagReaderPlugin.Listener listener) {
mPlugin.addListener(listener);
}
void removeListener(FlagReaderPlugin.Listener listener) {
mPlugin.removeListener(listener);
void removeListener(FlagReader.Listener listener) {
mFlagReader.removeListener(listener);
}
/**
@@ -172,6 +143,23 @@ public class FeatureFlagReader {
}
}
@Override
public void dump(@NonNull FileDescriptor fd, @NonNull PrintWriter pw, @NonNull String[] args) {
ArrayList<String> flagStrings = new ArrayList<>(mCachedFlags.size());
for (int i = 0; i < mCachedFlags.size(); i++) {
int key = mCachedFlags.keyAt(i);
// get the object by the key.
CachedFlag flag = mCachedFlags.get(key);
flagStrings.add(" " + RESNAME_PREFIX + flag.name + ": " + flag.value + "\n");
}
flagStrings.sort(String.CASE_INSENSITIVE_ORDER);
pw.println("AreFlagsOverrideable: " + mAreFlagsOverrideable);
pw.println("Cached FeatureFlags:");
for (String flagString : flagStrings) {
pw.print(flagString);
}
}
private static class CachedFlag {
public final String name;
public final boolean value;

View File

@@ -22,7 +22,6 @@ import android.util.FeatureFlagUtils;
import com.android.internal.annotations.VisibleForTesting;
import com.android.systemui.R;
import com.android.systemui.dagger.SysUISingleton;
import com.android.systemui.plugins.FlagReaderPlugin;
import java.util.ArrayList;
import java.util.HashMap;
@@ -51,7 +50,7 @@ public class FeatureFlags {
flagReader.addListener(mListener);
}
private final FlagReaderPlugin.Listener mListener = id -> {
private final FlagReader.Listener mListener = id -> {
if (mListeners.containsKey(id) && mFlagMap.containsKey(id)) {
mListeners.get(id).forEach(listener -> listener.onFlagChanged(mFlagMap.get(id)));
}
@@ -70,45 +69,8 @@ public class FeatureFlags {
return mFlagReader.isEnabled(flag);
}
/**
* @param flag The {@link StringFlag} of interest.
* @return The value of the flag.
*/
public String getValue(StringFlag flag) {
return mFlagReader.getValue(flag);
}
/**
* @param flag The {@link IntFlag} of interest.
* @return The value of the flag.
*/
public int getValue(IntFlag flag) {
return mFlagReader.getValue(flag);
}
/**
* @param flag The {@link LongFlag} of interest.
* @return The value of the flag.
*/
public long getValue(LongFlag flag) {
return mFlagReader.getValue(flag);
}
/**
* @param flag The {@link FloatFlag} of interest.
* @return The value of the flag.
*/
public float getValue(FloatFlag flag) {
return mFlagReader.getValue(flag);
}
/**
* @param flag The {@link DoubleFlag} of interest.
* @return The value of the flag.
*/
public double getValue(DoubleFlag flag) {
return mFlagReader.getValue(flag);
}
/** Add a listener for a specific flag. */
public void addFlagListener(Flag<?> flag, Listener listener) {

View File

@@ -14,49 +14,18 @@
* limitations under the License.
*/
package com.android.systemui.plugins;
import com.android.systemui.plugins.annotations.ProvidesInterface;
package com.android.systemui.flags;
/**
* Plugin for loading flag values from an alternate source of truth.
* Plugin for loading flag values
*/
@ProvidesInterface(action = FlagReaderPlugin.ACTION, version = FlagReaderPlugin.VERSION)
public interface FlagReaderPlugin extends Plugin {
int VERSION = 1;
String ACTION = "com.android.systemui.flags.FLAG_READER_PLUGIN";
public interface FlagReader {
/** Returns a boolean value for the given flag. */
default boolean isEnabled(int id, boolean def) {
return def;
}
/** Returns a string value for the given flag id. */
default String getValue(int id, String def) {
return def;
}
/** Returns a int value for the given flag. */
default int getValue(int id, int def) {
return def;
}
/** Returns a long value for the given flag. */
default long getValue(int id, long def) {
return def;
}
/** Returns a float value for the given flag. */
default float getValue(int id, float def) {
return def;
}
/** Returns a double value for the given flag. */
default double getValue(int id, double def) {
return def;
}
/** Add a listener to be alerted when any flag changes. */
default void addListener(Listener listener) {}

View File

@@ -0,0 +1,21 @@
/*
* Copyright (C) 2021 The Android Open Source Project
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.android.systemui.flags
interface FlagWriter {
fun setEnabled(key: Int, value: Boolean) {}
}

View File

@@ -32,6 +32,7 @@ import androidx.test.filters.SmallTest;
import com.android.systemui.R;
import com.android.systemui.SysuiTestCase;
import com.android.systemui.dump.DumpManager;
import com.android.systemui.shared.plugins.PluginManager;
import com.android.systemui.util.wrapper.BuildInfo;
@@ -44,8 +45,9 @@ import org.mockito.MockitoAnnotations;
public class FeatureFlagReaderTest extends SysuiTestCase {
@Mock private Resources mResources;
@Mock private BuildInfo mBuildInfo;
@Mock private PluginManager mPluginManager;
@Mock private DumpManager mDumpManager;
@Mock private SystemPropertiesHelper mSystemPropertiesHelper;
@Mock private FlagReader mFlagReader;
private FeatureFlagReader mReader;
@@ -66,7 +68,7 @@ public class FeatureFlagReaderTest extends SysuiTestCase {
when(mBuildInfo.isDebuggable()).thenReturn(isDebuggable);
when(mResources.getBoolean(R.bool.are_flags_overrideable)).thenReturn(isOverrideable);
mReader = new FeatureFlagReader(
mResources, mBuildInfo, mPluginManager, mSystemPropertiesHelper);
mResources, mBuildInfo, mDumpManager, mSystemPropertiesHelper, mFlagReader);
}
@Test

View File

@@ -23,7 +23,6 @@ import static org.mockito.Mockito.verify;
import androidx.test.filters.SmallTest;
import com.android.systemui.SysuiTestCase;
import com.android.systemui.plugins.FlagReaderPlugin;
import org.junit.Before;
import org.junit.Test;
@@ -51,11 +50,10 @@ public class FeatureFlagsTest extends SysuiTestCase {
mFeatureFlags.addFlag(flag);
// Assert and capture that a plugin listener was added.
ArgumentCaptor<FlagReaderPlugin.Listener> pluginListenerCaptor =
ArgumentCaptor.forClass(FlagReaderPlugin.Listener.class);
ArgumentCaptor<FlagReader.Listener> pluginListenerCaptor =
ArgumentCaptor.forClass(FlagReader.Listener.class);
verify(mFeatureFlagReader).addListener(pluginListenerCaptor.capture());
FlagReaderPlugin.Listener pluginListener = pluginListenerCaptor.getValue();
FlagReader.Listener pluginListener = pluginListenerCaptor.getValue();
// Signal a change. No listeners, so no real effect.
pluginListener.onFlagChanged(flag.getId());
@@ -81,11 +79,10 @@ public class FeatureFlagsTest extends SysuiTestCase {
mFeatureFlags.addFlag(flag);
// Assert and capture that a plugin listener was added.
ArgumentCaptor<FlagReaderPlugin.Listener> pluginListenerCaptor =
ArgumentCaptor.forClass(FlagReaderPlugin.Listener.class);
ArgumentCaptor<FlagReader.Listener> pluginListenerCaptor =
ArgumentCaptor.forClass(FlagReader.Listener.class);
verify(mFeatureFlagReader).addListener(pluginListenerCaptor.capture());
FlagReaderPlugin.Listener pluginListener = pluginListenerCaptor.getValue();
FlagReader.Listener pluginListener = pluginListenerCaptor.getValue();
// Add a listener for the flag
final Flag<?>[] changedFlag = {null};