Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit b145ad5e authored by Lucas Dupin's avatar Lucas Dupin
Browse files

Move FeatureFlagReader to sysprops 

DeviceConfigs ended up being flaky and buggy. They were also eventually
reset.
SystemProperties are stable and widely used across the system.

Test: adb shell setprop persist.systemui.flag_monet 1
Test: atest FeatureFlagReaderTest
Fixes: 180559762
Change-Id: I20b1ae82ac66a2257159048ac6f9b5cd00f677c5
parent add92198

packages/SystemUI/src/com/android/systemui/flags/FeatureFlagReader.java

+5 −36
Original line number Diff line number Diff line
@@ -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_";

}

packages/SystemUI/src/com/android/systemui/flags/SystemPropertiesHelper.kt

0 → 100644
+32 −0
Original line number Diff line number Diff line 
/*

 * 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)

    }

}
 
 No newline at end of file

packages/SystemUI/tests/src/com/android/systemui/flags/FeatureFlagReaderTest.java

+7 −39
Original line number Diff line number Diff line
@@ -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;