diff options
| -rw-r--r-- | core/java/android/flags/BooleanFlag.java | 25 | ||||
| -rw-r--r-- | core/java/android/flags/BooleanFlagBase.java | 54 | ||||
| -rw-r--r-- | core/java/android/flags/DynamicBooleanFlag.java | 22 | ||||
| -rw-r--r-- | core/java/android/flags/FeatureFlags.java | 279 | ||||
| -rw-r--r-- | core/java/android/flags/FeatureFlagsFake.java | 78 | ||||
| -rw-r--r-- | core/java/android/flags/FusedOffFlag.java | 31 | ||||
| -rw-r--r-- | core/java/android/flags/FusedOnFlag.java | 31 | ||||
| -rw-r--r-- | core/java/android/flags/SyncableFlag.java | 9 | ||||
| -rw-r--r-- | core/tests/coretests/src/android/flags/FeatureFlagsTest.java | 127 | ||||
| -rw-r--r-- | core/tests/coretests/src/android/flags/IFeatureFlagsFake.java | 63 | ||||
| -rw-r--r-- | services/flags/java/com/android/server/flags/DynamicFlagBinderDelegate.java | 6 | ||||
| -rw-r--r-- | services/flags/java/com/android/server/flags/FeatureFlagsBinder.java | 10 |
12 files changed, 592 insertions, 143 deletions
diff --git a/core/java/android/flags/BooleanFlag.java b/core/java/android/flags/BooleanFlag.java index 5519961a917f..ae9ccf8c52c3 100644 --- a/core/java/android/flags/BooleanFlag.java +++ b/core/java/android/flags/BooleanFlag.java @@ -25,9 +25,7 @@ import android.annotation.NonNull; * * @hide */ -public class BooleanFlag implements Flag<Boolean> { - private final String mNamespace; - private final String mName; +public class BooleanFlag extends BooleanFlagBase { private final boolean mDefault; /** @@ -36,8 +34,7 @@ public class BooleanFlag implements Flag<Boolean> { * @param defaultValue The value of this flag if no other override is present. */ BooleanFlag(String namespace, String name, boolean defaultValue) { - mNamespace = namespace; - mName = name; + super(namespace, name); mDefault = defaultValue; } @@ -46,22 +43,4 @@ public class BooleanFlag implements Flag<Boolean> { public Boolean getDefault() { return mDefault; } - - @Override - @NonNull - public String getNamespace() { - return mNamespace; - } - - @Override - @NonNull - public String getName() { - return mName; - } - - @Override - @NonNull - public String toString() { - return getNamespace() + "." + getName() + "[" + getDefault() + "]"; - } } diff --git a/core/java/android/flags/BooleanFlagBase.java b/core/java/android/flags/BooleanFlagBase.java new file mode 100644 index 000000000000..f4141ecab01b --- /dev/null +++ b/core/java/android/flags/BooleanFlagBase.java @@ -0,0 +1,54 @@ +/* + * Copyright (C) 2023 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 android.flags; + +import android.annotation.NonNull; + +abstract class BooleanFlagBase implements Flag<Boolean> { + + private final String mNamespace; + private final String mName; + + /** + * @param namespace A namespace for this flag. See {@link android.provider.DeviceConfig}. + * @param name A name for this flag. + */ + BooleanFlagBase(String namespace, String name) { + mNamespace = namespace; + mName = name; + } + + public abstract Boolean getDefault(); + + @Override + @NonNull + public String getNamespace() { + return mNamespace; + } + + @Override + @NonNull + public String getName() { + return mName; + } + + @Override + @NonNull + public String toString() { + return getNamespace() + "." + getName() + "[" + getDefault() + "]"; + } +} diff --git a/core/java/android/flags/DynamicBooleanFlag.java b/core/java/android/flags/DynamicBooleanFlag.java index d76fd9ec9a3c..92009c60d91a 100644 --- a/core/java/android/flags/DynamicBooleanFlag.java +++ b/core/java/android/flags/DynamicBooleanFlag.java @@ -23,10 +23,8 @@ package android.flags; * * @hide */ -public class DynamicBooleanFlag implements DynamicFlag<Boolean> { +public class DynamicBooleanFlag extends BooleanFlagBase implements DynamicFlag<Boolean> { - private final String mNamespace; - private final String mName; private final boolean mDefault; /** @@ -35,28 +33,12 @@ public class DynamicBooleanFlag implements DynamicFlag<Boolean> { * @param defaultValue The value of this flag if no other override is present. */ DynamicBooleanFlag(String namespace, String name, boolean defaultValue) { - mNamespace = namespace; - mName = name; + super(namespace, name); mDefault = defaultValue; } @Override - public String getNamespace() { - return mNamespace; - } - - @Override - public String getName() { - return mName; - } - - @Override public Boolean getDefault() { return mDefault; } - - @Override - public String toString() { - return getNamespace() + "." + getName() + "[" + getDefault() + "]"; - } } diff --git a/core/java/android/flags/FeatureFlags.java b/core/java/android/flags/FeatureFlags.java index c7d9d57e68b5..2c722a486e81 100644 --- a/core/java/android/flags/FeatureFlags.java +++ b/core/java/android/flags/FeatureFlags.java @@ -17,8 +17,19 @@ package android.flags; import android.annotation.NonNull; +import android.content.Context; +import android.os.RemoteException; +import android.os.ServiceManager; +import android.util.ArraySet; +import android.util.Log; +import com.android.internal.annotations.VisibleForTesting; + +import java.util.ArrayList; +import java.util.HashMap; import java.util.HashSet; +import java.util.List; +import java.util.Map; import java.util.Set; /** @@ -32,14 +43,20 @@ import java.util.Set; * @hide */ public class FeatureFlags { + private static final String TAG = "FeatureFlags"; private static FeatureFlags sInstance; private static final Object sInstanceLock = new Object(); + private final Set<Flag<?>> mKnownFlags = new ArraySet<>(); + private final Set<Flag<?>> mDirtyFlags = new ArraySet<>(); + + private IFeatureFlags mIFeatureFlags; + private final Map<String, Map<String, Boolean>> mBooleanOverrides = new HashMap<>(); private final Set<ChangeListener> mListeners = new HashSet<>(); /** * Obtain a per-process instance of FeatureFlags. - * @return + * @return A singleton instance of {@link FeatureFlags}. */ @NonNull public static FeatureFlags getInstance() { @@ -52,7 +69,124 @@ public class FeatureFlags { return sInstance; } - FeatureFlags() { + /** See {@link FeatureFlagsFake}. */ + @VisibleForTesting(visibility = VisibleForTesting.Visibility.PRIVATE) + public static void setInstance(FeatureFlags instance) { + synchronized (sInstanceLock) { + sInstance = instance; + } + } + + private final IFeatureFlagsCallback mIFeatureFlagsCallback = new IFeatureFlagsCallback.Stub() { + @Override + public void onFlagChange(SyncableFlag flag) { + for (Flag<?> f : mKnownFlags) { + if (flagEqualsSyncableFlag(f, flag)) { + if (f instanceof DynamicFlag<?>) { + if (f instanceof DynamicBooleanFlag) { + String value = flag.getValue(); + if (value == null) { // Null means any existing overrides were erased. + value = ((DynamicBooleanFlag) f).getDefault().toString(); + } + addBooleanOverride(flag.getNamespace(), flag.getName(), value); + } + FeatureFlags.this.onFlagChange((DynamicFlag<?>) f); + } + break; + } + } + } + }; + + private FeatureFlags() { + this(null); + } + + @VisibleForTesting(visibility = VisibleForTesting.Visibility.PRIVATE) + public FeatureFlags(IFeatureFlags iFeatureFlags) { + mIFeatureFlags = iFeatureFlags; + + if (mIFeatureFlags != null) { + try { + mIFeatureFlags.registerCallback(mIFeatureFlagsCallback); + } catch (RemoteException e) { + // Won't happen in tests. + } + } + } + + /** + * Construct a new {@link BooleanFlag}. + * + * Use this instead of constructing a {@link BooleanFlag} directly, as it registers the flag + * with the internals of the flagging system. + */ + @NonNull + public static BooleanFlag booleanFlag( + @NonNull String namespace, @NonNull String name, boolean def) { + return getInstance().addFlag(new BooleanFlag(namespace, name, def)); + } + + /** + * Construct a new {@link FusedOffFlag}. + * + * Use this instead of constructing a {@link FusedOffFlag} directly, as it registers the + * flag with the internals of the flagging system. + */ + @NonNull + public static FusedOffFlag fusedOffFlag(@NonNull String namespace, @NonNull String name) { + return getInstance().addFlag(new FusedOffFlag(namespace, name)); + } + + /** + * Construct a new {@link FusedOnFlag}. + * + * Use this instead of constructing a {@link FusedOnFlag} directly, as it registers the flag + * with the internals of the flagging system. + */ + @NonNull + public static FusedOnFlag fusedOnFlag(@NonNull String namespace, @NonNull String name) { + return getInstance().addFlag(new FusedOnFlag(namespace, name)); + } + + /** + * Construct a new {@link DynamicBooleanFlag}. + * + * Use this instead of constructing a {@link DynamicBooleanFlag} directly, as it registers + * the flag with the internals of the flagging system. + */ + @NonNull + public static DynamicBooleanFlag dynamicBooleanFlag( + @NonNull String namespace, @NonNull String name, boolean def) { + return getInstance().addFlag(new DynamicBooleanFlag(namespace, name, def)); + } + + /** + * Add a listener to be alerted when a {@link DynamicFlag} changes. + * + * See also {@link #removeChangeListener(ChangeListener)}. + * + * @param listener The listener to add. + */ + public void addChangeListener(@NonNull ChangeListener listener) { + mListeners.add(listener); + } + + /** + * Remove a listener that was added earlier. + * + * See also {@link #addChangeListener(ChangeListener)}. + * + * @param listener The listener to remove. + */ + public void removeChangeListener(@NonNull ChangeListener listener) { + mListeners.remove(listener); + } + + protected void onFlagChange(@NonNull DynamicFlag<?> flag) { + for (ChangeListener l : mListeners) { + l.onFlagChanged(flag); + } } /** @@ -63,7 +197,7 @@ public class FeatureFlags { * The first time a flag is read, its value is cached for the lifetime of the process. */ public boolean isEnabled(@NonNull BooleanFlag flag) { - return flag.getDefault(); + return getBooleanInternal(flag); } /** @@ -90,31 +224,138 @@ public class FeatureFlags { * Can return a different value for the flag each time it is called if an override comes in. */ public boolean isCurrentlyEnabled(@NonNull DynamicBooleanFlag flag) { - return flag.getDefault(); + return getBooleanInternal(flag); } - /** - * Add a listener to be alerted when a {@link DynamicFlag} changes. - * - * See also {@link #removeChangeListener(ChangeListener)}. - * - * @param listener The listener to add. - */ - public void addChangeListener(@NonNull ChangeListener listener) { - mListeners.add(listener); + private boolean getBooleanInternal(Flag<Boolean> flag) { + sync(); + Map<String, Boolean> ns = mBooleanOverrides.get(flag.getNamespace()); + Boolean value = null; + if (ns != null) { + value = ns.get(flag.getName()); + } + if (value == null) { + throw new IllegalStateException("Boolean flag being read but was not synced: " + flag); + } + + return value; + } + + private <T extends Flag<?>> T addFlag(T flag) { + synchronized (FeatureFlags.class) { + mDirtyFlags.add(flag); + mKnownFlags.add(flag); + } + return flag; + } + + protected void sync() { + synchronized (FeatureFlags.class) { + if (mDirtyFlags.isEmpty()) { + return; + } + syncInternal(mDirtyFlags); + mDirtyFlags.clear(); + } } /** - * Remove a listener that was added earlier. - * - * See also {@link #addChangeListener(ChangeListener)}. + * Called when new flags have been declared. Gives the implementation a chance to act on them. * - * @param listener The listener to remove. + * Guaranteed to be called from a synchronized, thread-safe context. */ - public void removeChangeListener(@NonNull ChangeListener listener) { - mListeners.remove(listener); + protected void syncInternal(Set<Flag<?>> dirtyFlags) { + IFeatureFlags iFeatureFlags = bind(); + List<SyncableFlag> syncableFlags = new ArrayList<>(); + for (Flag<?> f : dirtyFlags) { + syncableFlags.add(flagToSyncableFlag(f)); + } + + List<SyncableFlag> serverFlags = List.of(); // Need to initialize the list with something. + try { + // New values come back from the service. + serverFlags = iFeatureFlags.syncFlags(syncableFlags); + } catch (RemoteException e) { + e.rethrowFromSystemServer(); + } + + for (Flag<?> f : dirtyFlags) { + boolean found = false; + for (SyncableFlag sf : serverFlags) { + if (flagEqualsSyncableFlag(f, sf)) { + if (f instanceof BooleanFlag || f instanceof DynamicBooleanFlag) { + addBooleanOverride(sf.getNamespace(), sf.getName(), sf.getValue()); + } + found = true; + break; + } + } + if (!found) { + if (f instanceof BooleanFlag) { + addBooleanOverride( + f.getNamespace(), + f.getName(), + ((BooleanFlag) f).getDefault() ? "true" : "false"); + } + } + } + } + + private void addBooleanOverride(String namespace, String name, String override) { + Map<String, Boolean> nsOverrides = mBooleanOverrides.get(namespace); + if (nsOverrides == null) { + nsOverrides = new HashMap<>(); + mBooleanOverrides.put(namespace, nsOverrides); + } + nsOverrides.put(name, parseBoolean(override)); } + private SyncableFlag flagToSyncableFlag(Flag<?> f) { + return new SyncableFlag( + f.getNamespace(), + f.getName(), + f.getDefault().toString(), + f instanceof DynamicFlag<?>); + } + + private IFeatureFlags bind() { + if (mIFeatureFlags == null) { + mIFeatureFlags = IFeatureFlags.Stub.asInterface( + ServiceManager.getService(Context.FEATURE_FLAGS_SERVICE)); + try { + mIFeatureFlags.registerCallback(mIFeatureFlagsCallback); + } catch (RemoteException e) { + Log.e(TAG, "Failed to listen for flag changes!"); + } + } + + return mIFeatureFlags; + } + + static boolean parseBoolean(String value) { + // Check for a truish string. + boolean result = value.equalsIgnoreCase("true") + || value.equals("1") + || value.equalsIgnoreCase("t") + || value.equalsIgnoreCase("on"); + if (!result) { // Expect a falsish string, else log an error. + if (!(value.equalsIgnoreCase("false") + || value.equals("0") + || value.equalsIgnoreCase("f") + || value.equalsIgnoreCase("off"))) { + Log.e(TAG, + "Tried parsing " + value + " as boolean but it doesn't look like one. " + + "Value expected to be one of true|false, 1|0, t|f, on|off."); + } + } + return result; + } + + private static boolean flagEqualsSyncableFlag(Flag<?> f, SyncableFlag sf) { + return f.getName().equals(sf.getName()) && f.getNamespace().equals(sf.getNamespace()); + } + + /** * A simpler listener that is alerted when a {@link DynamicFlag} changes. * diff --git a/core/java/android/flags/FeatureFlagsFake.java b/core/java/android/flags/FeatureFlagsFake.java index 03bf2b6d33d2..daedcdae0b5f 100644 --- a/core/java/android/flags/FeatureFlagsFake.java +++ b/core/java/android/flags/FeatureFlagsFake.java @@ -18,32 +18,98 @@ package android.flags; import android.annotation.NonNull; +import java.util.HashMap; +import java.util.HashSet; +import java.util.Map; +import java.util.Set; + /** * An implementation of {@link FeatureFlags} for testing. + * + * Before you read a flag from using this Fake, you must set that flag using + * {@link #setFlagValue(BooleanFlagBase, boolean)}. This ensures that your tests are deterministic. + * + * If you are relying on {@link FeatureFlags#getInstance()} to access FeatureFlags in your code + * under test, (instead of dependency injection), you can pass an instance of this fake to + * {@link FeatureFlags#setInstance(FeatureFlags)}. Be sure to call that method again, passing null, + * to ensure hermetic testing - you don't want static state persisting between your test methods. + * * @hide */ public class FeatureFlagsFake extends FeatureFlags { - public FeatureFlagsFake() { - super(); + private final Map<BooleanFlagBase, Boolean> mFlagValues = new HashMap<>(); + private final Set<BooleanFlagBase> mReadFlags = new HashSet<>(); + + public FeatureFlagsFake(IFeatureFlags iFeatureFlags) { + super(iFeatureFlags); } @Override public boolean isEnabled(@NonNull BooleanFlag flag) { - return flag.getDefault(); + return requireFlag(flag); } @Override public boolean isEnabled(@NonNull FusedOffFlag flag) { - return false; + return requireFlag(flag); } @Override public boolean isEnabled(@NonNull FusedOnFlag flag) { - return true; + return requireFlag(flag); } @Override public boolean isCurrentlyEnabled(@NonNull DynamicBooleanFlag flag) { - return flag.getDefault(); + return requireFlag(flag); + } + + @Override + protected void syncInternal(Set<Flag<?>> dirtyFlags) { + } + + /** + * Explicitly set a flag's value for reading in tests. + * + * You _must_ call this for every flag your code-under-test will read. Otherwise, an + * {@link IllegalStateException} will be thrown. + * + * You are able to set values for {@link FusedOffFlag} and {@link FusedOnFlag}, despite those + * flags having a fixed value at compile time, since unit tests should still test the state of + * those flags as both true and false. I.e. a flag that is off might be turned on in a future + * build or vice versa. + * + * You can not call this method _after_ a non-dynamic flag has been read. Non-dynamic flags + * are held stable in the system, so changing a value after reading would not match + * real-implementation behavior. + * + * Calling this method will trigger any {@link android.flags.FeatureFlags.ChangeListener}s that + * are registered for the supplied flag if the flag is a {@link DynamicFlag}. + * + * @param flag The BooleanFlag that you want to set a value for. + * @param value The value that the flag should return when accessed. + */ + public void setFlagValue(@NonNull BooleanFlagBase flag, boolean value) { + if (!(flag instanceof DynamicBooleanFlag) && mReadFlags.contains(flag)) { + throw new RuntimeException( + "You can not set the value of a flag after it has been read. Tried to set " + + flag + " to " + value + " but it already " + mFlagValues.get(flag)); + } + mFlagValues.put(flag, value); + if (flag instanceof DynamicBooleanFlag) { + onFlagChange((DynamicFlag<?>) flag); + } } + + private boolean requireFlag(BooleanFlagBase flag) { + if (!mFlagValues.containsKey(flag)) { + throw new IllegalStateException( + "Tried to access " + flag + " in test but no overrided specified. You must " + + "call #setFlagValue for each flag read in a test."); + } + mReadFlags.add(flag); + + return mFlagValues.get(flag); + } + } diff --git a/core/java/android/flags/FusedOffFlag.java b/core/java/android/flags/FusedOffFlag.java index 999d0f981780..5f435beb2642 100644 --- a/core/java/android/flags/FusedOffFlag.java +++ b/core/java/android/flags/FusedOffFlag.java @@ -17,6 +17,7 @@ package android.flags; import android.annotation.NonNull; +import android.provider.DeviceConfig; /** * A flag representing a false value. @@ -25,13 +26,13 @@ import android.annotation.NonNull; * * @hide */ -public final class FusedOffFlag implements Flag<Boolean> { - private final String mNamespace; - private final String mName; - +public final class FusedOffFlag extends BooleanFlagBase { + /** + * @param namespace A namespace for this flag. See {@link DeviceConfig}. + * @param name A name for this flag. + */ FusedOffFlag(String namespace, String name) { - mNamespace = namespace; - mName = name; + super(namespace, name); } @Override @@ -39,22 +40,4 @@ public final class FusedOffFlag implements Flag<Boolean> { public Boolean getDefault() { return false; } - - @Override - @NonNull - public String getNamespace() { - return mNamespace; - } - - @Override - @NonNull - public String getName() { - return mName; - } - - @Override - @NonNull - public String toString() { - return getNamespace() + "." + getName() + "[false]"; - } } diff --git a/core/java/android/flags/FusedOnFlag.java b/core/java/android/flags/FusedOnFlag.java index 4f33bee8abe1..ea8d9b29b3ce 100644 --- a/core/java/android/flags/FusedOnFlag.java +++ b/core/java/android/flags/FusedOnFlag.java @@ -17,6 +17,7 @@ package android.flags; import android.annotation.NonNull; +import android.provider.DeviceConfig; /** * A flag representing a true value. @@ -25,13 +26,13 @@ import android.annotation.NonNull; * * @hide */ -public final class FusedOnFlag implements Flag<Boolean> { - private final String mNamespace; - private final String mName; - +public final class FusedOnFlag extends BooleanFlagBase { + /** + * @param namespace A namespace for this flag. See {@link DeviceConfig}. + * @param name A name for this flag. + */ FusedOnFlag(String namespace, String name) { - mNamespace = namespace; - mName = name; + super(namespace, name); } @Override @@ -39,22 +40,4 @@ public final class FusedOnFlag implements Flag<Boolean> { public Boolean getDefault() { return true; } - - @Override - @NonNull - public String getNamespace() { - return mNamespace; - } - - @Override - @NonNull - public String getName() { - return mName; - } - - @Override - @NonNull - public String toString() { - return getNamespace() + "." + getName() + "[true]"; - } } diff --git a/core/java/android/flags/SyncableFlag.java b/core/java/android/flags/SyncableFlag.java index 54c4c6d5fdf9..0b2a4d9e4f98 100644 --- a/core/java/android/flags/SyncableFlag.java +++ b/core/java/android/flags/SyncableFlag.java @@ -20,18 +20,15 @@ import android.annotation.NonNull; import android.os.Parcel; import android.os.Parcelable; -import com.android.internal.annotations.VisibleForTesting; - /** * @hide */ public final class SyncableFlag implements Parcelable { private final String mNamespace; private final String mName; - private String mValue; + private final String mValue; private final boolean mDynamic; - @VisibleForTesting(visibility = VisibleForTesting.Visibility.PACKAGE) public SyncableFlag( @NonNull String namespace, @NonNull String name, @@ -43,10 +40,6 @@ public final class SyncableFlag implements Parcelable { mDynamic = dynamic; } - public void setValue(@NonNull String value) { - mValue = value; - } - @NonNull public String getNamespace() { return mNamespace; diff --git a/core/tests/coretests/src/android/flags/FeatureFlagsTest.java b/core/tests/coretests/src/android/flags/FeatureFlagsTest.java index dc6006a8e5fa..3fc94394d12c 100644 --- a/core/tests/coretests/src/android/flags/FeatureFlagsTest.java +++ b/core/tests/coretests/src/android/flags/FeatureFlagsTest.java @@ -13,7 +13,6 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - package android.flags; import static com.google.common.truth.Truth.assertThat; @@ -22,33 +21,135 @@ import android.platform.test.annotations.Presubmit; import androidx.test.filters.SmallTest; -import org.junit.After; import org.junit.Before; -import org.junit.Ignore; import org.junit.Test; -import org.mockito.MockitoAnnotations; + +import java.util.List; +import java.util.concurrent.atomic.AtomicBoolean; + @SmallTest @Presubmit -public final class FeatureFlagsTest { +public class FeatureFlagsTest { + + IFeatureFlagsFake mIFeatureFlagsFake = new IFeatureFlagsFake(); + FeatureFlags mFeatureFlags = new FeatureFlags(mIFeatureFlagsFake); @Before public void setup() { - MockitoAnnotations.initMocks(this); + FeatureFlags.setInstance(mFeatureFlags); + } + + @Test + public void testFusedOff_Disabled() { + FusedOffFlag flag = FeatureFlags.fusedOffFlag("test", "a"); + assertThat(mFeatureFlags.isEnabled(flag)).isFalse(); + } + + @Test + public void testFusedOn_Enabled() { + FusedOnFlag flag = FeatureFlags.fusedOnFlag("test", "a"); + assertThat(mFeatureFlags.isEnabled(flag)).isTrue(); + } + + @Test + public void testBooleanFlag_DefaultDisabled() { + BooleanFlag flag = FeatureFlags.booleanFlag("test", "a", false); + assertThat(mFeatureFlags.isEnabled(flag)).isFalse(); + } + + @Test + public void testBooleanFlag_DefaultEnabled() { + BooleanFlag flag = FeatureFlags.booleanFlag("test", "a", true); + assertThat(mFeatureFlags.isEnabled(flag)).isTrue(); + } + + @Test + public void testDynamicBooleanFlag_DefaultDisabled() { + DynamicBooleanFlag flag = FeatureFlags.dynamicBooleanFlag("test", "a", false); + assertThat(mFeatureFlags.isCurrentlyEnabled(flag)).isFalse(); } - @After - public void tearDown() { + @Test + public void testDynamicBooleanFlag_DefaultEnabled() { + DynamicBooleanFlag flag = FeatureFlags.dynamicBooleanFlag("test", "a", true); + assertThat(mFeatureFlags.isCurrentlyEnabled(flag)).isTrue(); } @Test - public void testPass() { - assertThat(true).isTrue(); + public void testBooleanFlag_OverrideBeforeRead() { + BooleanFlag flag = FeatureFlags.booleanFlag("test", "a", false); + SyncableFlag syncableFlag = new SyncableFlag( + flag.getNamespace(), flag.getName(), "true", false); + + mIFeatureFlagsFake.setFlagOverrides(List.of(syncableFlag)); + + assertThat(mFeatureFlags.isEnabled(flag)).isTrue(); } - @Ignore @Test - public void testFail() { - assertThat(false).isTrue(); + public void testFusedOffFlag_OverrideHasNoEffect() { + FusedOffFlag flag = FeatureFlags.fusedOffFlag("test", "a"); + SyncableFlag syncableFlag = new SyncableFlag( + flag.getNamespace(), flag.getName(), "true", false); + + mIFeatureFlagsFake.setFlagOverrides(List.of(syncableFlag)); + + assertThat(mFeatureFlags.isEnabled(flag)).isFalse(); + } + + @Test + public void testFusedOnFlag_OverrideHasNoEffect() { + FusedOnFlag flag = FeatureFlags.fusedOnFlag("test", "a"); + SyncableFlag syncableFlag = new SyncableFlag( + flag.getNamespace(), flag.getName(), "false", false); + + mIFeatureFlagsFake.setFlagOverrides(List.of(syncableFlag)); + + assertThat(mFeatureFlags.isEnabled(flag)).isTrue(); + } + + @Test + public void testDynamicFlag_OverrideBeforeRead() { + DynamicBooleanFlag flag = FeatureFlags.dynamicBooleanFlag("test", "a", false); + SyncableFlag syncableFlag = new SyncableFlag( + flag.getNamespace(), flag.getName(), "true", true); + + mIFeatureFlagsFake.setFlagOverrides(List.of(syncableFlag)); + + // Changes to true + assertThat(mFeatureFlags.isCurrentlyEnabled(flag)).isTrue(); + } + + @Test + public void testDynamicFlag_OverrideAfterRead() { + DynamicBooleanFlag flag = FeatureFlags.dynamicBooleanFlag("test", "a", false); + SyncableFlag syncableFlag = new SyncableFlag( + flag.getNamespace(), flag.getName(), "true", true); + + // Starts false + assertThat(mFeatureFlags.isCurrentlyEnabled(flag)).isFalse(); + + mIFeatureFlagsFake.setFlagOverrides(List.of(syncableFlag)); + + // Changes to true + assertThat(mFeatureFlags.isCurrentlyEnabled(flag)).isTrue(); + } + + @Test + public void testDynamicFlag_FiresListener() { + DynamicBooleanFlag flag = FeatureFlags.dynamicBooleanFlag("test", "a", false); + AtomicBoolean called = new AtomicBoolean(false); + FeatureFlags.ChangeListener listener = flag1 -> called.set(true); + + mFeatureFlags.addChangeListener(listener); + + SyncableFlag syncableFlag = new SyncableFlag( + flag.getNamespace(), flag.getName(), flag.getDefault().toString(), true); + + mIFeatureFlagsFake.setFlagOverrides(List.of(syncableFlag)); + + // Fires listener. + assertThat(called.get()).isTrue(); } } diff --git a/core/tests/coretests/src/android/flags/IFeatureFlagsFake.java b/core/tests/coretests/src/android/flags/IFeatureFlagsFake.java new file mode 100644 index 000000000000..70b65ce1eb57 --- /dev/null +++ b/core/tests/coretests/src/android/flags/IFeatureFlagsFake.java @@ -0,0 +1,63 @@ +/* + * Copyright (C) 2023 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 android.flags; + +import android.os.IBinder; +import android.os.RemoteException; + +import java.util.HashSet; +import java.util.List; +import java.util.Set; + +class IFeatureFlagsFake implements IFeatureFlags { + + private final Set<IFeatureFlagsCallback> mCallbacks = new HashSet<>(); + + List<SyncableFlag> mOverrides; + + @Override + public IBinder asBinder() { + return null; + } + + @Override + public List<SyncableFlag> syncFlags(List<SyncableFlag> flagList) { + return mOverrides == null ? flagList : mOverrides; + } + + @Override + public void registerCallback(IFeatureFlagsCallback callback) { + mCallbacks.add(callback); + } + + @Override + public void unregisterCallback(IFeatureFlagsCallback callback) { + mCallbacks.remove(callback); + } + + public void setFlagOverrides(List<SyncableFlag> flagList) { + mOverrides = flagList; + for (SyncableFlag sf : flagList) { + for (IFeatureFlagsCallback cb : mCallbacks) { + try { + cb.onFlagChange(sf); + } catch (RemoteException e) { + // does not happen in fakes. + } + } + } + } +} diff --git a/services/flags/java/com/android/server/flags/DynamicFlagBinderDelegate.java b/services/flags/java/com/android/server/flags/DynamicFlagBinderDelegate.java index 322a52bf01cc..8da5aae6abdd 100644 --- a/services/flags/java/com/android/server/flags/DynamicFlagBinderDelegate.java +++ b/services/flags/java/com/android/server/flags/DynamicFlagBinderDelegate.java @@ -118,9 +118,9 @@ class DynamicFlagBinderDelegate { mFlagStore.setChangeCallback(mFlagChangeCallback); } - void syncDynamicFlag(int pid, SyncableFlag sf) { + SyncableFlag syncDynamicFlag(int pid, SyncableFlag sf) { if (!sf.isDynamic()) { - return; + return sf; } String ns = sf.getNamespace(); @@ -162,7 +162,7 @@ class DynamicFlagBinderDelegate { // to something. data.setDefaultValue(sf.getValue()); - sf.setValue(value); + return new SyncableFlag(sf.getNamespace(), sf.getName(), value, true); } diff --git a/services/flags/java/com/android/server/flags/FeatureFlagsBinder.java b/services/flags/java/com/android/server/flags/FeatureFlagsBinder.java index dc97fde758dc..86b464dbdf5d 100644 --- a/services/flags/java/com/android/server/flags/FeatureFlagsBinder.java +++ b/services/flags/java/com/android/server/flags/FeatureFlagsBinder.java @@ -25,6 +25,7 @@ import android.os.Build; import android.os.ParcelFileDescriptor; import java.io.FileOutputStream; +import java.util.ArrayList; import java.util.List; class FeatureFlagsBinder extends IFeatureFlags.Stub { @@ -52,11 +53,13 @@ class FeatureFlagsBinder extends IFeatureFlags.Stub { @Override public List<SyncableFlag> syncFlags(List<SyncableFlag> incomingFlags) { int pid = getCallingPid(); + List<SyncableFlag> outputFlags = new ArrayList<>(); for (SyncableFlag sf : incomingFlags) { String ns = sf.getNamespace(); String name = sf.getName(); + SyncableFlag outFlag; if (sf.isDynamic()) { - mDynamicFlagDelegate.syncDynamicFlag(pid, sf); + outFlag = mDynamicFlagDelegate.syncDynamicFlag(pid, sf); } else { synchronized (mFlagCache) { String value = mFlagCache.getOrNull(ns, name); @@ -65,11 +68,12 @@ class FeatureFlagsBinder extends IFeatureFlags.Stub { value = overrideValue != null ? overrideValue : sf.getValue(); mFlagCache.setIfChanged(ns, name, value); } - sf.setValue(value); + outFlag = new SyncableFlag(sf.getNamespace(), sf.getName(), value, false); } } + outputFlags.add(outFlag); } - return incomingFlags; + return outputFlags; } @SystemApi |