Merge "Move FeatureFlagReader to sysprops" into sc-dev
This commit is contained in:
@@ -16,24 +16,18 @@
|
||||
|
||||
package com.android.systemui.flags;
|
||||
|
||||
import android.annotation.NonNull;
|
||||
import android.content.res.Resources;
|
||||
import android.provider.DeviceConfig;
|
||||
import android.util.SparseArray;
|
||||
|
||||
import androidx.annotation.BoolRes;
|
||||
import androidx.annotation.Nullable;
|
||||
|
||||
import com.android.systemui.R;
|
||||
import com.android.systemui.assist.DeviceConfigHelper;
|
||||
import com.android.systemui.dagger.SysUISingleton;
|
||||
import com.android.systemui.dagger.qualifiers.Background;
|
||||
import com.android.systemui.dagger.qualifiers.Main;
|
||||
import com.android.systemui.statusbar.FeatureFlags;
|
||||
import com.android.systemui.util.wrapper.BuildInfo;
|
||||
|
||||
import java.util.concurrent.Executor;
|
||||
|
||||
import javax.inject.Inject;
|
||||
/**
|
||||
* Reads and caches feature flags for quick access
|
||||
@@ -60,23 +54,19 @@ import javax.inject.Inject;
|
||||
@SysUISingleton
|
||||
public class FeatureFlagReader {
|
||||
private final Resources mResources;
|
||||
private final DeviceConfigHelper mDeviceConfig;
|
||||
private final boolean mAreFlagsOverrideable;
|
||||
|
||||
private final SystemPropertiesHelper mSystemPropertiesHelper;
|
||||
private final SparseArray<CachedFlag> mCachedFlags = new SparseArray<>();
|
||||
|
||||
@Inject
|
||||
public FeatureFlagReader(
|
||||
@Main Resources resources,
|
||||
BuildInfo build,
|
||||
DeviceConfigHelper deviceConfig,
|
||||
@Background Executor executor) {
|
||||
SystemPropertiesHelper systemPropertiesHelper) {
|
||||
mResources = resources;
|
||||
mDeviceConfig = deviceConfig;
|
||||
mSystemPropertiesHelper = systemPropertiesHelper;
|
||||
mAreFlagsOverrideable =
|
||||
build.isDebuggable() && mResources.getBoolean(R.bool.are_flags_overrideable);
|
||||
|
||||
mDeviceConfig.addOnPropertiesChangedListener(executor, this::onPropertiesChanged);
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -93,7 +83,7 @@ public class FeatureFlagReader {
|
||||
String name = resourceIdToFlagName(resId);
|
||||
boolean value = mResources.getBoolean(resId);
|
||||
if (mAreFlagsOverrideable) {
|
||||
value = mDeviceConfig.getBoolean(flagNameToStorageKey(name), value);
|
||||
value = mSystemPropertiesHelper.getBoolean(flagNameToStorageKey(name), value);
|
||||
}
|
||||
|
||||
cachedFlag = new CachedFlag(name, value);
|
||||
@@ -104,27 +94,6 @@ public class FeatureFlagReader {
|
||||
}
|
||||
}
|
||||
|
||||
private void onPropertiesChanged(@NonNull DeviceConfig.Properties properties) {
|
||||
synchronized (mCachedFlags) {
|
||||
for (String key : properties.getKeyset()) {
|
||||
String flagName = storageKeyToFlagName(key);
|
||||
if (flagName != null) {
|
||||
clearCachedFlag(flagName);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
private void clearCachedFlag(String flagName) {
|
||||
for (int i = 0; i < mCachedFlags.size(); i++) {
|
||||
CachedFlag flag = mCachedFlags.valueAt(i);
|
||||
if (flag.name.equals(flagName)) {
|
||||
mCachedFlags.removeAt(i);
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
private String resourceIdToFlagName(@BoolRes int resId) {
|
||||
String resName = mResources.getResourceEntryName(resId);
|
||||
if (resName.startsWith(RESNAME_PREFIX)) {
|
||||
@@ -160,6 +129,6 @@ public class FeatureFlagReader {
|
||||
}
|
||||
}
|
||||
|
||||
private static final String STORAGE_KEY_PREFIX = "flag_";
|
||||
private static final String STORAGE_KEY_PREFIX = "persist.systemui.flag_";
|
||||
private static final String RESNAME_PREFIX = "flag_";
|
||||
}
|
||||
|
||||
@@ -0,0 +1,32 @@
|
||||
/*
|
||||
* 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.os.SystemProperties
|
||||
import com.android.systemui.dagger.SysUISingleton
|
||||
|
||||
import javax.inject.Inject
|
||||
|
||||
/**
|
||||
* Proxy to make {@link SystemProperties} easily testable.
|
||||
*/
|
||||
@SysUISingleton
|
||||
class SystemPropertiesHelper @Inject constructor() {
|
||||
fun getBoolean(name: String, default: Boolean): Boolean {
|
||||
return SystemProperties.getBoolean(name, default)
|
||||
}
|
||||
}
|
||||
@@ -18,7 +18,6 @@ package com.android.systemui.flags;
|
||||
|
||||
import static org.junit.Assert.assertFalse;
|
||||
import static org.junit.Assert.assertTrue;
|
||||
import static org.mockito.ArgumentMatchers.any;
|
||||
import static org.mockito.ArgumentMatchers.anyBoolean;
|
||||
import static org.mockito.ArgumentMatchers.anyString;
|
||||
import static org.mockito.ArgumentMatchers.eq;
|
||||
@@ -27,59 +26,44 @@ import static org.mockito.Mockito.verify;
|
||||
import static org.mockito.Mockito.when;
|
||||
|
||||
import android.content.res.Resources;
|
||||
import android.provider.DeviceConfig;
|
||||
|
||||
import androidx.annotation.BoolRes;
|
||||
import androidx.test.filters.SmallTest;
|
||||
|
||||
import com.android.systemui.R;
|
||||
import com.android.systemui.SysuiTestCase;
|
||||
import com.android.systemui.assist.DeviceConfigHelper;
|
||||
import com.android.systemui.util.wrapper.BuildInfo;
|
||||
|
||||
import org.junit.Before;
|
||||
import org.junit.Test;
|
||||
import org.mockito.ArgumentCaptor;
|
||||
import org.mockito.Captor;
|
||||
import org.mockito.Mock;
|
||||
import org.mockito.MockitoAnnotations;
|
||||
|
||||
import java.util.HashMap;
|
||||
import java.util.Map;
|
||||
import java.util.concurrent.Executor;
|
||||
|
||||
@SmallTest
|
||||
public class FeatureFlagReaderTest extends SysuiTestCase {
|
||||
@Mock private Resources mResources;
|
||||
@Mock private BuildInfo mBuildInfo;
|
||||
@Mock private DeviceConfigHelper mDeviceConfig;
|
||||
@Mock private Executor mBackgroundExecutor;
|
||||
@Mock private SystemPropertiesHelper mSystemPropertiesHelper;
|
||||
|
||||
private FeatureFlagReader mReader;
|
||||
|
||||
@Captor private ArgumentCaptor<DeviceConfig.OnPropertiesChangedListener> mListenerCaptor;
|
||||
private DeviceConfig.OnPropertiesChangedListener mPropChangeListener;
|
||||
|
||||
@Before
|
||||
public void setUp() {
|
||||
MockitoAnnotations.initMocks(this);
|
||||
|
||||
when(mDeviceConfig.getBoolean(anyString(), anyBoolean()))
|
||||
when(mSystemPropertiesHelper.getBoolean(anyString(), anyBoolean()))
|
||||
.thenAnswer(invocation -> invocation.getArgument(1));
|
||||
|
||||
defineFlag(FLAG_RESID_0, false);
|
||||
defineFlag(FLAG_RESID_1, true);
|
||||
|
||||
initialize(true, true);
|
||||
|
||||
verify(mDeviceConfig).addOnPropertiesChangedListener(any(), mListenerCaptor.capture());
|
||||
mPropChangeListener = mListenerCaptor.getValue();
|
||||
}
|
||||
|
||||
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, mDeviceConfig, mBackgroundExecutor);
|
||||
mReader = new FeatureFlagReader(mResources, mBuildInfo, mSystemPropertiesHelper);
|
||||
}
|
||||
|
||||
@Test
|
||||
@@ -136,24 +120,8 @@ public class FeatureFlagReaderTest extends SysuiTestCase {
|
||||
|
||||
// THEN the underlying resource and override are only queried once
|
||||
verify(mResources, times(1)).getBoolean(FLAG_RESID_0);
|
||||
verify(mDeviceConfig, times(1)).getBoolean(fakeStorageKey(FLAG_RESID_0), false);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testCachesAreClearedAfterPropsChange() {
|
||||
// GIVEN a flag whose value has already been queried
|
||||
assertFalse(mReader.isEnabled(FLAG_RESID_0));
|
||||
|
||||
// WHEN the value of the flag changes
|
||||
overrideFlag(FLAG_RESID_0, true);
|
||||
Map<String, String> changedMap = new HashMap<>();
|
||||
changedMap.put(fakeStorageKey(FLAG_RESID_0), "true");
|
||||
DeviceConfig.Properties properties =
|
||||
new DeviceConfig.Properties("systemui", changedMap);
|
||||
mPropChangeListener.onPropertiesChanged(properties);
|
||||
|
||||
// THEN the new value is provided
|
||||
assertTrue(mReader.isEnabled(FLAG_RESID_0));
|
||||
verify(mSystemPropertiesHelper, times(1))
|
||||
.getBoolean(fakeStorageKey(FLAG_RESID_0), false);
|
||||
}
|
||||
|
||||
private void defineFlag(int resId, boolean value) {
|
||||
@@ -162,12 +130,12 @@ public class FeatureFlagReaderTest extends SysuiTestCase {
|
||||
}
|
||||
|
||||
private void overrideFlag(int resId, boolean value) {
|
||||
when(mDeviceConfig.getBoolean(eq(fakeStorageKey(resId)), anyBoolean()))
|
||||
when(mSystemPropertiesHelper.getBoolean(eq(fakeStorageKey(resId)), anyBoolean()))
|
||||
.thenReturn(value);
|
||||
}
|
||||
|
||||
private String fakeStorageKey(@BoolRes int resId) {
|
||||
return "flag_testname_" + resId;
|
||||
return "persist.systemui.flag_testname_" + resId;
|
||||
}
|
||||
|
||||
private static final int FLAG_RESID_0 = 47;
|
||||
|
||||
Reference in New Issue
Block a user