From 06037712dd9bc6d5d2111b94991622f9c6bf42b4 Mon Sep 17 00:00:00 2001 From: Dave Mankoff Date: Tue, 27 Jul 2021 13:10:42 -0400 Subject: [PATCH] Add new Flag class. New FlagReaderPlugin. This CL is a stepping stone towards a more complete flag system. This cl allows for the definition of flags within systemui, each of which have a unique id and a default value. Methods for retrieving the value of a flag are defined. By default, reading a flag always returns its default value. A FlagReaderPlugin is also defined that, when loaded, allows for returning alternate values for a given flag. The details of how an alternate value are determined are left for the plugin to decide. The plugin interface should be considered incomplete with this cl. Methods for reading flags are defined, but hooks for knowing when flags change (and hooks for knowing when a flag plugin has been loaded), are not defined. Moreover, the flag plugin is not loaded early in the process with this cl, so anyone who reads flags before the plugin is may misss out on the values they care about. More changes to come. Bug: 194781951 Test: atest SystemUITests Change-Id: I888d5eba82359a37878a3a4d4747990b79a77804 --- .../src/com/android/systemui/flags/Flag.kt | 52 ++++++++ .../src/com/android/systemui/flags/Flags.java | 23 ++++ .../systemui/plugins/FlagReaderPlugin.java | 59 +++++++++ .../systemui/flags/FeatureFlagReader.java | 46 +++++++ .../android/systemui/flags/FeatureFlags.java | 79 ++++++++++++ .../systemui/flags/FeatureFlagReaderTest.java | 5 +- .../com/android/systemui/flags/FlagsTest.java | 119 ++++++++++++++++++ 7 files changed, 382 insertions(+), 1 deletion(-) create mode 100644 packages/SystemUI/plugin/src/com/android/systemui/flags/Flag.kt create mode 100644 packages/SystemUI/plugin/src/com/android/systemui/flags/Flags.java create mode 100644 packages/SystemUI/plugin/src/com/android/systemui/plugins/FlagReaderPlugin.java create mode 100644 packages/SystemUI/tests/src/com/android/systemui/flags/FlagsTest.java diff --git a/packages/SystemUI/plugin/src/com/android/systemui/flags/Flag.kt b/packages/SystemUI/plugin/src/com/android/systemui/flags/Flag.kt new file mode 100644 index 0000000000000..68834bc2aa23c --- /dev/null +++ b/packages/SystemUI/plugin/src/com/android/systemui/flags/Flag.kt @@ -0,0 +1,52 @@ +/* + * 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 Flag { + val id: Int + val default: T +} + +data class BooleanFlag @JvmOverloads constructor( + override val id: Int, + override val default: Boolean = false +) : Flag + +data class StringFlag @JvmOverloads constructor( + override val id: Int, + override val default: String = "" +) : Flag + +data class IntFlag @JvmOverloads constructor( + override val id: Int, + override val default: Int = 0 +) : Flag + +data class LongFlag @JvmOverloads constructor( + override val id: Int, + override val default: Long = 0 +) : Flag + +data class FloatFlag @JvmOverloads constructor( + override val id: Int, + override val default: Float = 0f +) : Flag + +data class DoubleFlag @JvmOverloads constructor( + override val id: Int, + override val default: Double = 0.0 +) : Flag \ No newline at end of file diff --git a/packages/SystemUI/plugin/src/com/android/systemui/flags/Flags.java b/packages/SystemUI/plugin/src/com/android/systemui/flags/Flags.java new file mode 100644 index 0000000000000..d5b9243ebe861 --- /dev/null +++ b/packages/SystemUI/plugin/src/com/android/systemui/flags/Flags.java @@ -0,0 +1,23 @@ +/* + * 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; + +/** + * List of {@link Flag} objects for use in SystemUI. + */ +public class Flags { +} diff --git a/packages/SystemUI/plugin/src/com/android/systemui/plugins/FlagReaderPlugin.java b/packages/SystemUI/plugin/src/com/android/systemui/plugins/FlagReaderPlugin.java new file mode 100644 index 0000000000000..d153bd8eac098 --- /dev/null +++ b/packages/SystemUI/plugin/src/com/android/systemui/plugins/FlagReaderPlugin.java @@ -0,0 +1,59 @@ +/* + * 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.plugins; + +import com.android.systemui.plugins.annotations.ProvidesInterface; + + +/** + * Plugin for loading flag values from an alternate source of truth. + */ +@ProvidesInterface(action = FlagReaderPlugin.ACTION, version = FlagReaderPlugin.VERSION) +public interface FlagReaderPlugin extends Plugin { + int VERSION = 1; + String ACTION = "com.android.systemui.flags.FLAG_READER_PLUGIN"; + + /** 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; + } +} diff --git a/packages/SystemUI/src/com/android/systemui/flags/FeatureFlagReader.java b/packages/SystemUI/src/com/android/systemui/flags/FeatureFlagReader.java index 1fec7a669cc8c..08247a824ffb9 100644 --- a/packages/SystemUI/src/com/android/systemui/flags/FeatureFlagReader.java +++ b/packages/SystemUI/src/com/android/systemui/flags/FeatureFlagReader.java @@ -16,6 +16,7 @@ package com.android.systemui.flags; +import android.content.Context; import android.content.res.Resources; import android.util.SparseArray; @@ -25,6 +26,9 @@ import androidx.annotation.Nullable; 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.util.wrapper.BuildInfo; import javax.inject.Inject; @@ -54,18 +58,60 @@ import javax.inject.Inject; public class FeatureFlagReader { private final Resources mResources; private final boolean mAreFlagsOverrideable; + private final PluginManager mPluginManager; private final SystemPropertiesHelper mSystemPropertiesHelper; private final SparseArray mCachedFlags = new SparseArray<>(); + private FlagReaderPlugin mPlugin = new FlagReaderPlugin(){}; + @Inject public FeatureFlagReader( @Main Resources resources, BuildInfo build, + PluginManager pluginManager, SystemPropertiesHelper systemPropertiesHelper) { mResources = resources; + mPluginManager = pluginManager; mSystemPropertiesHelper = systemPropertiesHelper; mAreFlagsOverrideable = build.isDebuggable() && mResources.getBoolean(R.bool.are_flags_overrideable); + + mPluginManager.addPluginListener(mPluginListener, FlagReaderPlugin.class); + } + + private final PluginListener mPluginListener = + new PluginListener() { + 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()); + } + + String getValue(StringFlag flag) { + return mPlugin.getValue(flag.getId(), flag.getDefault()); + } + + 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()); } /** diff --git a/packages/SystemUI/src/com/android/systemui/flags/FeatureFlags.java b/packages/SystemUI/src/com/android/systemui/flags/FeatureFlags.java index 5882179226c9c..0c9e6de4e4d03 100644 --- a/packages/SystemUI/src/com/android/systemui/flags/FeatureFlags.java +++ b/packages/SystemUI/src/com/android/systemui/flags/FeatureFlags.java @@ -22,6 +22,10 @@ import android.util.FeatureFlagUtils; import com.android.systemui.R; import com.android.systemui.dagger.SysUISingleton; +import java.lang.reflect.Field; +import java.util.HashMap; +import java.util.Map; + import javax.inject.Inject; /** @@ -40,6 +44,54 @@ public class FeatureFlags { mContext = context; } + /** + * @param flag The {@link BooleanFlag} of interest. + * @return The value of the flag. + */ + public boolean isEnabled(BooleanFlag flag) { + 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); + } + public boolean isNewNotifPipelineEnabled() { return mFlagReader.isEnabled(R.bool.flag_notification_pipeline2); } @@ -107,4 +159,31 @@ public class FeatureFlags { public static boolean isProviderModelSettingEnabled(Context context) { return FeatureFlagUtils.isEnabled(context, FeatureFlagUtils.SETTINGS_PROVIDER_MODEL); } + + private Map> collectFlags() { + Map> flags = new HashMap<>(); + + Field[] fields = this.getClass().getFields(); + + for (Field field : fields) { + Class t = field.getType(); + if (Flag.class.isAssignableFrom(t)) { + try { + //flags.add((Flag) field.get(null)); + Flag flag = (Flag) field.get(null); + flags.put(flag.getId(), flag); + } catch (IllegalAccessException e) { + // no-op + } + } + } + + return flags; + } + + /** Simple interface for beinga alerted when a specific flag changes value. */ + public interface Listener { + /** */ + void onFlagChanged(Flag flag); + } } diff --git a/packages/SystemUI/tests/src/com/android/systemui/flags/FeatureFlagReaderTest.java b/packages/SystemUI/tests/src/com/android/systemui/flags/FeatureFlagReaderTest.java index 223714cfda309..7bc5f86510a11 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/flags/FeatureFlagReaderTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/flags/FeatureFlagReaderTest.java @@ -32,6 +32,7 @@ import androidx.test.filters.SmallTest; import com.android.systemui.R; import com.android.systemui.SysuiTestCase; +import com.android.systemui.shared.plugins.PluginManager; import com.android.systemui.util.wrapper.BuildInfo; import org.junit.Before; @@ -43,6 +44,7 @@ import org.mockito.MockitoAnnotations; public class FeatureFlagReaderTest extends SysuiTestCase { @Mock private Resources mResources; @Mock private BuildInfo mBuildInfo; + @Mock private PluginManager mPluginManager; @Mock private SystemPropertiesHelper mSystemPropertiesHelper; private FeatureFlagReader mReader; @@ -63,7 +65,8 @@ public class FeatureFlagReaderTest extends SysuiTestCase { private void initialize(boolean isDebuggable, boolean isOverrideable) { when(mBuildInfo.isDebuggable()).thenReturn(isDebuggable); when(mResources.getBoolean(R.bool.are_flags_overrideable)).thenReturn(isOverrideable); - mReader = new FeatureFlagReader(mResources, mBuildInfo, mSystemPropertiesHelper); + mReader = new FeatureFlagReader( + mResources, mBuildInfo, mPluginManager, mSystemPropertiesHelper); } @Test diff --git a/packages/SystemUI/tests/src/com/android/systemui/flags/FlagsTest.java b/packages/SystemUI/tests/src/com/android/systemui/flags/FlagsTest.java new file mode 100644 index 0000000000000..25c302885e078 --- /dev/null +++ b/packages/SystemUI/tests/src/com/android/systemui/flags/FlagsTest.java @@ -0,0 +1,119 @@ +/* + * 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; + +import static com.google.common.truth.Truth.assertWithMessage; + +import android.util.Pair; + +import androidx.test.filters.SmallTest; + +import com.android.systemui.SysuiTestCase; + +import org.junit.Test; + +import java.lang.reflect.Field; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +@SmallTest +public class FlagsTest extends SysuiTestCase { + + @Test + public void testDuplicateFlagIdCheckWorks() { + List>> flags = collectFlags(DuplicateFlagContainer.class); + Map> duplicates = groupDuplicateFlags(flags); + + assertWithMessage(generateAssertionMessage(duplicates)) + .that(duplicates.size()).isEqualTo(2); + } + + @Test + public void testNoDuplicateFlagIds() { + List>> flags = collectFlags(Flags.class); + Map> duplicates = groupDuplicateFlags(flags); + + assertWithMessage(generateAssertionMessage(duplicates)) + .that(duplicates.size()).isEqualTo(0); + } + + private String generateAssertionMessage(Map> duplicates) { + StringBuilder stringBuilder = new StringBuilder(); + stringBuilder.append("Duplicate flag keys found: {"); + for (int id : duplicates.keySet()) { + stringBuilder + .append(" ") + .append(id) + .append(": [") + .append(String.join(", ", duplicates.get(id))) + .append("]"); + } + stringBuilder.append(" }"); + + return stringBuilder.toString(); + } + + private List>> collectFlags(Class clz) { + List>> flags = new ArrayList<>(); + + Field[] fields = clz.getFields(); + + for (Field field : fields) { + Class t = field.getType(); + if (Flag.class.isAssignableFrom(t)) { + try { + flags.add(Pair.create(field.getName(), (Flag) field.get(null))); + } catch (IllegalAccessException e) { + // no-op + } + } + } + + return flags; + } + + private Map> groupDuplicateFlags(List>> flags) { + Map> grouping = new HashMap<>(); + + for (Pair> flag : flags) { + grouping.putIfAbsent(flag.second.getId(), new ArrayList<>()); + grouping.get(flag.second.getId()).add(flag.first); + } + + Map> result = new HashMap<>(); + for (Integer id : grouping.keySet()) { + if (grouping.get(id).size() > 1) { + result.put(id, grouping.get(id)); + } + } + + return result; + } + + private static class DuplicateFlagContainer { + public static final BooleanFlag A_FLAG = new BooleanFlag(0); + public static final BooleanFlag B_FLAG = new BooleanFlag(0); + public static final StringFlag C_FLAG = new StringFlag(0); + + public static final BooleanFlag D_FLAG = new BooleanFlag(1); + + public static final DoubleFlag E_FLAG = new DoubleFlag(3); + public static final DoubleFlag F_FLAG = new DoubleFlag(3); + } +}