From 1a934e9aee4571974a7b7687dcb03dbff0acfcfe Mon Sep 17 00:00:00 2001 From: Dave Mankoff Date: Thu, 21 Oct 2021 10:30:19 -0400 Subject: [PATCH] Add ability to read flag values to Flag Library. The FlagManager now partially implements the FlagReader interface. The FeatureFlagManager, part of SystemUI proper, now uses the FlagManager to read flag values, helping to ensure consistency between the two separate systems. Bug: 203548827 Test: manual Change-Id: Ia3b103f783a846cfe9437cd8e6b987962cebabde --- packages/SystemUI/Android.bp | 3 +- .../com/android/systemui/flags/FlagManager.kt | 46 ++++++++++++++++- .../com/android/systemui/flags/FlagReader.kt} | 28 +++++------ .../systemui/flags/FeatureFlagManager.java | 49 +++++-------------- .../com/android/systemui/flags/FlagsModule.kt | 30 ++++++++++++ .../systemui/flags/FeatureFlagManager.java | 12 ++++- .../com/android/systemui/flags/FlagsModule.kt | 22 +++++++++ .../systemui/dagger/SystemUIModule.java | 2 + .../flags/FeatureFlagManagerTest.java | 10 ++-- 9 files changed, 141 insertions(+), 61 deletions(-) rename packages/SystemUI/{src/com/android/systemui/flags/FlagReader.java => shared/src/com/android/systemui/flags/FlagReader.kt} (56%) create mode 100644 packages/SystemUI/src-debug/com/android/systemui/flags/FlagsModule.kt create mode 100644 packages/SystemUI/src-release/com/android/systemui/flags/FlagsModule.kt diff --git a/packages/SystemUI/Android.bp b/packages/SystemUI/Android.bp index f44893e013ab5..16a6a463512eb 100644 --- a/packages/SystemUI/Android.bp +++ b/packages/SystemUI/Android.bp @@ -179,8 +179,7 @@ android_library { "src/**/*.kt", "src/**/*.java", "src/**/I*.aidl", - "src-release/**/*.kt", - "src-release/**/*.java", + ":ReleaseJavaFiles", ], static_libs: [ "SystemUIAnimationLib", diff --git a/packages/SystemUI/shared/src/com/android/systemui/flags/FlagManager.kt b/packages/SystemUI/shared/src/com/android/systemui/flags/FlagManager.kt index 89fee7084332a..3125faf3e96e6 100644 --- a/packages/SystemUI/shared/src/com/android/systemui/flags/FlagManager.kt +++ b/packages/SystemUI/shared/src/com/android/systemui/flags/FlagManager.kt @@ -18,16 +18,22 @@ package com.android.systemui.flags import android.content.Context import android.content.Intent +import android.provider.Settings import androidx.concurrent.futures.CallbackToFutureAdapter import com.google.common.util.concurrent.ListenableFuture +import org.json.JSONException +import org.json.JSONObject -class FlagManager constructor(val context: Context) { +class FlagManager constructor(val context: Context) : FlagReader { companion object { const val RECEIVING_PACKAGE = "com.android.systemui" const val ACTION_SET_FLAG = "com.android.systemui.action.SET_FLAG" const val FLAGS_PERMISSION = "com.android.systemui.permission.FLAGS" const val FIELD_ID = "id" const val FIELD_VALUE = "value" + const val FIELD_TYPE = "type" + const val TYPE_BOOLEAN = "boolean" + private const val SETTINGS_PREFIX = "systemui/flags" } fun getFlagsFuture(): ListenableFuture>> { @@ -54,6 +60,28 @@ class FlagManager constructor(val context: Context) { context.sendBroadcast(intent) } + override fun isEnabled(id: Int, def: Boolean): Boolean { + return isEnabled(id) ?: def + } + + /** Returns the stored value or null if not set. */ + fun isEnabled(id: Int): Boolean? { + val data: String = Settings.Secure.getString( + context.contentResolver, keyToSettingsPrefix(id)) + if (data.isEmpty()) { + return null + } + val json: JSONObject + try { + json = JSONObject(data) + return if (!assertType(json, TYPE_BOOLEAN)) { + null + } else json.getBoolean(FIELD_VALUE) + } catch (e: JSONException) { + throw InvalidFlagStorageException() + } + } + private fun createIntent(id: Int): Intent { val intent = Intent(ACTION_SET_FLAG) intent.setPackage(RECEIVING_PACKAGE) @@ -61,4 +89,18 @@ class FlagManager constructor(val context: Context) { return intent } -} \ No newline at end of file + + fun keyToSettingsPrefix(key: Int): String? { + return SETTINGS_PREFIX + "/" + key + } + + private fun assertType(json: JSONObject, type: String): Boolean { + return try { + json.getString(FIELD_TYPE) == TYPE_BOOLEAN + } catch (e: JSONException) { + false + } + } +} + +class InvalidFlagStorageException : Exception("Data found but is invalid") \ No newline at end of file diff --git a/packages/SystemUI/src/com/android/systemui/flags/FlagReader.java b/packages/SystemUI/shared/src/com/android/systemui/flags/FlagReader.kt similarity index 56% rename from packages/SystemUI/src/com/android/systemui/flags/FlagReader.java rename to packages/SystemUI/shared/src/com/android/systemui/flags/FlagReader.kt index 1ae8c1f2a712a..ef8ec37d7cda5 100644 --- a/packages/SystemUI/src/com/android/systemui/flags/FlagReader.java +++ b/packages/SystemUI/shared/src/com/android/systemui/flags/FlagReader.kt @@ -13,28 +13,26 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - -package com.android.systemui.flags; - +package com.android.systemui.flags /** * Plugin for loading flag values */ -public interface FlagReader { - /** Returns a boolean value for the given flag. */ - default boolean isEnabled(int id, boolean def) { - return def; +interface FlagReader { + /** Returns a boolean value for the given flag. */ + fun isEnabled(id: Int, def: Boolean): Boolean { + return def } - /** Add a listener to be alerted when any flag changes. */ - default void addListener(Listener listener) {} + /** Add a listener to be alerted when any flag changes. */ + fun addListener(listener: Listener) {} - /** Remove a listener to be alerted when any flag changes. */ - default void removeListener(Listener listener) {} + /** Remove a listener to be alerted when any flag changes. */ + fun removeListener(listener: Listener) {} - /** A simple listener to be alerted when a flag changes. */ + /** A simple listener to be alerted when a flag changes. */ interface Listener { - /** */ - void onFlagChanged(int id); + /** */ + fun onFlagChanged(id: Int) } -} +} \ No newline at end of file diff --git a/packages/SystemUI/src-debug/com/android/systemui/flags/FeatureFlagManager.java b/packages/SystemUI/src-debug/com/android/systemui/flags/FeatureFlagManager.java index c2ed1ac6f0e87..3768123948017 100644 --- a/packages/SystemUI/src-debug/com/android/systemui/flags/FeatureFlagManager.java +++ b/packages/SystemUI/src-debug/com/android/systemui/flags/FeatureFlagManager.java @@ -33,6 +33,7 @@ import androidx.annotation.NonNull; import com.android.systemui.Dumpable; import com.android.systemui.dagger.SysUISingleton; import com.android.systemui.dump.DumpManager; +import com.android.systemui.util.settings.SecureSettings; import org.json.JSONException; import org.json.JSONObject; @@ -58,18 +59,16 @@ import javax.inject.Inject; public class FeatureFlagManager implements FlagReader, FlagWriter, Dumpable { private static final String TAG = "SysUIFlags"; - private static final String SYSPROP_PREFIX = "persist.systemui.flag_"; - private static final String FIELD_TYPE = "type"; - private static final String TYPE_BOOLEAN = "boolean"; - private final SystemPropertiesHelper mSystemPropertiesHelper; - + private final FlagManager mFlagManager; + private final SecureSettings mSecureSettings; private final Map mBooleanFlagCache = new HashMap<>(); @Inject - public FeatureFlagManager(SystemPropertiesHelper systemPropertiesHelper, Context context, + public FeatureFlagManager(FlagManager flagManager, + SecureSettings secureSettings, Context context, DumpManager dumpManager) { - mSystemPropertiesHelper = systemPropertiesHelper; - + mFlagManager = flagManager; + mSecureSettings = secureSettings; IntentFilter filter = new IntentFilter(ACTION_SET_FLAG); context.registerReceiver(mReceiver, filter, FLAGS_PERMISSION, null); dumpManager.registerDumpable(TAG, this); @@ -88,20 +87,10 @@ public class FeatureFlagManager implements FlagReader, FlagWriter, Dumpable { /** Returns the stored value or null if not set. */ private Boolean isEnabledInternal(int id) { - String data = mSystemPropertiesHelper.get(keyToSysPropKey(id)); - if (data.isEmpty()) { - return null; - } - JSONObject json; try { - json = new JSONObject(data); - if (!assertType(json, TYPE_BOOLEAN)) { - return null; - } - - return json.getBoolean(FIELD_VALUE); - } catch (JSONException e) { - eraseInternal(id); // Don't restart SystemUI in this case. + return mFlagManager.isEnabled(id); + } catch (Exception e) { + eraseInternal(id); } return null; } @@ -116,9 +105,9 @@ public class FeatureFlagManager implements FlagReader, FlagWriter, Dumpable { JSONObject json = new JSONObject(); try { - json.put(FIELD_TYPE, TYPE_BOOLEAN); + json.put(FlagManager.FIELD_TYPE, FlagManager.TYPE_BOOLEAN); json.put(FIELD_VALUE, value); - mSystemPropertiesHelper.set(keyToSysPropKey(id), json.toString()); + mSecureSettings.putString(mFlagManager.keyToSettingsPrefix(id), json.toString()); Log.i(TAG, "Set id " + id + " to " + value); restartSystemUI(); } catch (JSONException e) { @@ -135,7 +124,7 @@ public class FeatureFlagManager implements FlagReader, FlagWriter, Dumpable { /** Works just like {@link #eraseFlag(int)} except that it doesn't restart SystemUI. */ private void eraseInternal(int id) { // We can't actually "erase" things from sysprops, but we can set them to empty! - mSystemPropertiesHelper.set(keyToSysPropKey(id), ""); + mSecureSettings.putString(mFlagManager.keyToSettingsPrefix(id), ""); Log.i(TAG, "Erase id " + id); } @@ -151,18 +140,6 @@ public class FeatureFlagManager implements FlagReader, FlagWriter, Dumpable { System.exit(0); } - private static String keyToSysPropKey(int key) { - return SYSPROP_PREFIX + key; - } - - private static boolean assertType(JSONObject json, String type) { - try { - return json.getString(FIELD_TYPE).equals(TYPE_BOOLEAN); - } catch (JSONException e) { - return false; - } - } - private final BroadcastReceiver mReceiver = new BroadcastReceiver() { @Override public void onReceive(Context context, Intent intent) { diff --git a/packages/SystemUI/src-debug/com/android/systemui/flags/FlagsModule.kt b/packages/SystemUI/src-debug/com/android/systemui/flags/FlagsModule.kt new file mode 100644 index 0000000000000..0fe0fee694b93 --- /dev/null +++ b/packages/SystemUI/src-debug/com/android/systemui/flags/FlagsModule.kt @@ -0,0 +1,30 @@ +/* + * 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 android.content.Context +import dagger.Module +import dagger.Provides + +@Module +object FlagsModule { + @JvmStatic + @Provides + fun provideFlagManager(context: Context): FlagManager { + return FlagManager(context) + } +} \ No newline at end of file diff --git a/packages/SystemUI/src-release/com/android/systemui/flags/FeatureFlagManager.java b/packages/SystemUI/src-release/com/android/systemui/flags/FeatureFlagManager.java index e501a073fe189..6ff175f589a63 100644 --- a/packages/SystemUI/src-release/com/android/systemui/flags/FeatureFlagManager.java +++ b/packages/SystemUI/src-release/com/android/systemui/flags/FeatureFlagManager.java @@ -24,6 +24,7 @@ import androidx.annotation.NonNull; import com.android.systemui.Dumpable; import com.android.systemui.dagger.SysUISingleton; import com.android.systemui.dump.DumpManager; +import com.android.systemui.util.settings.SecureSettings; import java.io.FileDescriptor; import java.io.PrintWriter; @@ -40,10 +41,17 @@ import javax.inject.Inject; public class FeatureFlagManager implements FlagReader, FlagWriter, Dumpable { SparseBooleanArray mAccessedFlags = new SparseBooleanArray(); @Inject - public FeatureFlagManager(SystemPropertiesHelper systemPropertiesHelper, Context context, - DumpManager dumpManager) { + public FeatureFlagManager( + SecureSettings secureSettings, Context context, DumpManager dumpManager) { dumpManager.registerDumpable("SysUIFlags", this); } + + @Override + public void addListener(Listener run) {} + + @Override + public void removeListener(Listener run) {} + @Override public boolean isEnabled(int key, boolean defaultValue) { mAccessedFlags.append(key, defaultValue); diff --git a/packages/SystemUI/src-release/com/android/systemui/flags/FlagsModule.kt b/packages/SystemUI/src-release/com/android/systemui/flags/FlagsModule.kt new file mode 100644 index 0000000000000..7647135bd7fdc --- /dev/null +++ b/packages/SystemUI/src-release/com/android/systemui/flags/FlagsModule.kt @@ -0,0 +1,22 @@ +/* + * 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 dagger.Module + +@Module +object FlagsModule \ No newline at end of file diff --git a/packages/SystemUI/src/com/android/systemui/dagger/SystemUIModule.java b/packages/SystemUI/src/com/android/systemui/dagger/SystemUIModule.java index a4e2572836f8e..d52a8073492eb 100644 --- a/packages/SystemUI/src/com/android/systemui/dagger/SystemUIModule.java +++ b/packages/SystemUI/src/com/android/systemui/dagger/SystemUIModule.java @@ -42,6 +42,7 @@ 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.flags.FlagsModule; import com.android.systemui.fragments.FragmentService; import com.android.systemui.log.dagger.LogModule; import com.android.systemui.model.SysUiState; @@ -104,6 +105,7 @@ import dagger.Provides; ControlsModule.class, DemoModeModule.class, FalsingModule.class, + FlagsModule.class, LogModule.class, PeopleHubModule.class, PluginModule.class, diff --git a/packages/SystemUI/tests/src/com/android/systemui/flags/FeatureFlagManagerTest.java b/packages/SystemUI/tests/src/com/android/systemui/flags/FeatureFlagManagerTest.java index 8243be8448c09..2fa32ba1fe75e 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/flags/FeatureFlagManagerTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/flags/FeatureFlagManagerTest.java @@ -33,6 +33,7 @@ import androidx.test.filters.SmallTest; import com.android.systemui.SysuiTestCase; import com.android.systemui.dump.DumpManager; +import com.android.systemui.util.settings.SecureSettings; import org.junit.After; import org.junit.Before; @@ -52,7 +53,8 @@ import java.io.StringWriter; public class FeatureFlagManagerTest extends SysuiTestCase { FeatureFlagManager mFeatureFlagManager; - @Mock private SystemPropertiesHelper mProps; + @Mock private FlagManager mFlagManager; + @Mock private SecureSettings mSecureSettings; @Mock private Context mContext; @Mock private DumpManager mDumpManager; @@ -60,14 +62,14 @@ public class FeatureFlagManagerTest extends SysuiTestCase { public void setup() { MockitoAnnotations.initMocks(this); - mFeatureFlagManager = new FeatureFlagManager(mProps, mContext, mDumpManager); + mFeatureFlagManager = new FeatureFlagManager(mSecureSettings, mContext, mDumpManager); } @After public void onFinished() { - // SystemPropertiesHelper and Context are provided for constructor consistency with the + // SecureSettings and Context are provided for constructor consistency with the // debug version of the FeatureFlagManager, but should never be used. - verifyZeroInteractions(mProps, mContext); + verifyZeroInteractions(mSecureSettings, mContext); // The dump manager should be registered with even for the release version, but that's it. verify(mDumpManager).registerDumpable(anyString(), any()); verifyNoMoreInteractions(mDumpManager);