summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
author Dave Mankoff <mankoff@google.com> 2023-07-12 18:18:39 +0000
committer Dave Mankoff <mankoff@google.com> 2023-07-12 18:18:39 +0000
commit6b375c276f423c6d10b0080b64c3625bd93fdbce (patch)
tree4e34cf1556da9ea9efd87a462bba74d322fc6721
parentd7d04dccb9b91a426a7a291bc1e4ef0728e3b361 (diff)
Revert "Add permissions to FeatureFlagService and override api."
Revert submission 23727772-b279054964-flag-api-udc-qpr-dev Reason for revert: b/290929382 Reverted changes: /q/submissionid:23727772-b279054964-flag-api-udc-qpr-dev Change-Id: I2148317cb3aa35403a7d4df4e2b0db18aca1d91b
-rw-r--r--core/java/android/flags/DynamicFlag.java4
-rw-r--r--core/java/android/flags/FeatureFlags.java8
-rw-r--r--core/java/android/flags/Flag.java5
-rw-r--r--core/java/android/flags/IFeatureFlags.aidl24
-rw-r--r--core/java/android/flags/SyncableFlag.java24
-rw-r--r--core/java/com/android/internal/flags/CoreFlags.java93
-rw-r--r--core/res/AndroidManifest.xml18
-rw-r--r--core/tests/coretests/src/android/flags/IFeatureFlagsFake.java50
-rw-r--r--services/flags/java/com/android/server/flags/DynamicFlagBinderDelegate.java43
-rw-r--r--services/flags/java/com/android/server/flags/FeatureFlagsBinder.java80
-rw-r--r--services/flags/java/com/android/server/flags/FeatureFlagsService.java53
-rw-r--r--services/flags/java/com/android/server/flags/FlagOverrideStore.java6
-rw-r--r--services/tests/servicestests/src/com/android/server/flags/FeatureFlagsServiceTest.java118
13 files changed, 27 insertions, 499 deletions
diff --git a/core/java/android/flags/DynamicFlag.java b/core/java/android/flags/DynamicFlag.java
index 68819c58c064..8583ea6034d8 100644
--- a/core/java/android/flags/DynamicFlag.java
+++ b/core/java/android/flags/DynamicFlag.java
@@ -24,8 +24,4 @@ package android.flags;
* @hide
*/
public interface DynamicFlag<T> extends Flag<T> {
- @Override
- default boolean isDynamic() {
- return true;
- }
}
diff --git a/core/java/android/flags/FeatureFlags.java b/core/java/android/flags/FeatureFlags.java
index 8d3112c35d51..93e56b172434 100644
--- a/core/java/android/flags/FeatureFlags.java
+++ b/core/java/android/flags/FeatureFlags.java
@@ -250,13 +250,7 @@ public class FeatureFlags {
return flag;
}
- /**
- * Sync any known flags that have not yet been synced.
- *
- * This is called implicitly when any flag is read, and is not generally needed except in
- * exceptional circumstances.
- */
- public void sync() {
+ protected void sync() {
synchronized (FeatureFlags.class) {
if (mDirtyFlags.isEmpty()) {
return;
diff --git a/core/java/android/flags/Flag.java b/core/java/android/flags/Flag.java
index 0ab3c8a5d617..ae0a6ba33f44 100644
--- a/core/java/android/flags/Flag.java
+++ b/core/java/android/flags/Flag.java
@@ -37,9 +37,4 @@ public interface Flag<T> {
/** The value of this flag if no override has been set. Null values are not supported. */
@NonNull
T getDefault();
-
- /** Returns true if the value of this flag can change at runtime. */
- default boolean isDynamic() {
- return false;
- }
}
diff --git a/core/java/android/flags/IFeatureFlags.aidl b/core/java/android/flags/IFeatureFlags.aidl
index 3efcec97fe6d..1eef47feaa8b 100644
--- a/core/java/android/flags/IFeatureFlags.aidl
+++ b/core/java/android/flags/IFeatureFlags.aidl
@@ -62,28 +62,4 @@ interface IFeatureFlags {
* {@link #registerCallback}.
*/
void unregisterCallback(IFeatureFlagsCallback callback);
-
- /**
- * Query the {@link com.android.server.flags.FeatureFlagsService} for flags, but don't
- * cache them. See {@link #syncFlags}.
- *
- * You almost certainly don't want this method. This is intended for the Flag Flipper
- * application that needs to query the state of system but doesn't want to affect it by
- * doing so. All other clients should use {@link syncFlags}.
- */
- List<SyncableFlag> queryFlags(in List<SyncableFlag> flagList);
-
- /**
- * Change a flags value in the system.
- *
- * This is intended for use by the Flag Flipper application.
- */
- void overrideFlag(in SyncableFlag flag);
-
- /**
- * Restore a flag to its default value.
- *
- * This is intended for use by the Flag Flipper application.
- */
- void resetFlag(in SyncableFlag flag);
} \ No newline at end of file
diff --git a/core/java/android/flags/SyncableFlag.java b/core/java/android/flags/SyncableFlag.java
index 449bcc3c49f5..0b2a4d9e4f98 100644
--- a/core/java/android/flags/SyncableFlag.java
+++ b/core/java/android/flags/SyncableFlag.java
@@ -28,28 +28,16 @@ public final class SyncableFlag implements Parcelable {
private final String mName;
private final String mValue;
private final boolean mDynamic;
- private final boolean mOverridden;
public SyncableFlag(
@NonNull String namespace,
@NonNull String name,
@NonNull String value,
boolean dynamic) {
- this(namespace, name, value, dynamic, false);
- }
-
- public SyncableFlag(
- @NonNull String namespace,
- @NonNull String name,
- @NonNull String value,
- boolean dynamic,
- boolean overridden
- ) {
mNamespace = namespace;
mName = name;
mValue = value;
mDynamic = dynamic;
- mOverridden = overridden;
}
@NonNull
@@ -67,23 +55,16 @@ public final class SyncableFlag implements Parcelable {
return mValue;
}
+ @NonNull
public boolean isDynamic() {
return mDynamic;
}
- public boolean isOverridden() {
- return mOverridden;
- }
-
@NonNull
public static final Parcelable.Creator<SyncableFlag> CREATOR = new Parcelable.Creator<>() {
public SyncableFlag createFromParcel(Parcel in) {
return new SyncableFlag(
- in.readString(),
- in.readString(),
- in.readString(),
- in.readBoolean(),
- in.readBoolean());
+ in.readString(), in.readString(), in.readString(), in.readBoolean());
}
public SyncableFlag[] newArray(int size) {
@@ -102,7 +83,6 @@ public final class SyncableFlag implements Parcelable {
dest.writeString(mName);
dest.writeString(mValue);
dest.writeBoolean(mDynamic);
- dest.writeBoolean(mOverridden);
}
@Override
diff --git a/core/java/com/android/internal/flags/CoreFlags.java b/core/java/com/android/internal/flags/CoreFlags.java
deleted file mode 100644
index f177ef88c38f..000000000000
--- a/core/java/com/android/internal/flags/CoreFlags.java
+++ /dev/null
@@ -1,93 +0,0 @@
-/*
- * 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 com.android.internal.flags;
-
-import android.flags.BooleanFlag;
-import android.flags.DynamicBooleanFlag;
-import android.flags.FeatureFlags;
-import android.flags.FusedOffFlag;
-import android.flags.FusedOnFlag;
-import android.flags.SyncableFlag;
-
-import java.util.ArrayList;
-import java.util.List;
-
-/**
- * Flags defined here are can be read by code in core.
- *
- * Flags not defined here will throw a security exception if third-party processes attempts to read
- * them.
- *
- * DO NOT define a flag here unless you explicitly intend for that flag to be readable by code that
- * runs inside a third party process.
- */
-public abstract class CoreFlags {
- private static final List<SyncableFlag> sKnownFlags = new ArrayList<>();
-
- public static BooleanFlag BOOL_FLAG = booleanFlag("core", "bool_flag", false);
- public static FusedOffFlag OFF_FLAG = fusedOffFlag("core", "off_flag");
- public static FusedOnFlag ON_FLAG = fusedOnFlag("core", "on_flag");
- public static DynamicBooleanFlag DYN_FLAG = dynamicBooleanFlag("core", "dyn_flag", true);
-
- /** Returns true if the passed in flag matches a flag in this class. */
- public static boolean isCoreFlag(SyncableFlag flag) {
- for (SyncableFlag knownFlag : sKnownFlags) {
- if (knownFlag.getName().equals(flag.getName())
- && knownFlag.getNamespace().equals(flag.getNamespace())) {
- return true;
- }
- }
- return false;
- }
-
- public static List<SyncableFlag> getCoreFlags() {
- return sKnownFlags;
- }
-
- private static BooleanFlag booleanFlag(String namespace, String name, boolean defaultValue) {
- BooleanFlag f = FeatureFlags.booleanFlag(namespace, name, defaultValue);
-
- sKnownFlags.add(new SyncableFlag(namespace, name, Boolean.toString(defaultValue), false));
-
- return f;
- }
-
- private static FusedOffFlag fusedOffFlag(String namespace, String name) {
- FusedOffFlag f = FeatureFlags.fusedOffFlag(namespace, name);
-
- sKnownFlags.add(new SyncableFlag(namespace, name, "false", false));
-
- return f;
- }
-
- private static FusedOnFlag fusedOnFlag(String namespace, String name) {
- FusedOnFlag f = FeatureFlags.fusedOnFlag(namespace, name);
-
- sKnownFlags.add(new SyncableFlag(namespace, name, "true", false));
-
- return f;
- }
-
- private static DynamicBooleanFlag dynamicBooleanFlag(
- String namespace, String name, boolean defaultValue) {
- DynamicBooleanFlag f = FeatureFlags.dynamicBooleanFlag(namespace, name, defaultValue);
-
- sKnownFlags.add(new SyncableFlag(namespace, name, Boolean.toString(defaultValue), true));
-
- return f;
- }
-}
diff --git a/core/res/AndroidManifest.xml b/core/res/AndroidManifest.xml
index 10cf353bf5e9..a01c7b6355c1 100644
--- a/core/res/AndroidManifest.xml
+++ b/core/res/AndroidManifest.xml
@@ -7655,24 +7655,6 @@
<permission android:name="android.permission.GET_ANY_PROVIDER_TYPE"
android:protectionLevel="signature" />
-
- <!-- @hide Allows internal applications to read and synchronize non-core flags.
- Apps without this permission can only read a subset of flags specifically intended
- for use in "core", (i.e. third party apps). Apps with this permission can define their
- own flags, and federate those values with other system-level apps.
- <p>Not for use by third-party applications.
- <p>Protection level: signature
- -->
- <permission android:name="android.permission.SYNC_FLAGS"
- android:protectionLevel="signature" />
-
- <!-- @hide Allows internal applications to override flags in the FeatureFlags service.
- <p>Not for use by third-party applications.
- <p>Protection level: signature
- -->
- <permission android:name="android.permission.WRITE_FLAGS"
- android:protectionLevel="signature" />
-
<!-- Attribution for Geofencing service. -->
<attribution android:tag="GeofencingService" android:label="@string/geofencing_service"/>
<!-- Attribution for Country Detector. -->
diff --git a/core/tests/coretests/src/android/flags/IFeatureFlagsFake.java b/core/tests/coretests/src/android/flags/IFeatureFlagsFake.java
index bc5d8aa3ac73..70b65ce1eb57 100644
--- a/core/tests/coretests/src/android/flags/IFeatureFlagsFake.java
+++ b/core/tests/coretests/src/android/flags/IFeatureFlagsFake.java
@@ -39,56 +39,6 @@ class IFeatureFlagsFake implements IFeatureFlags {
}
@Override
- public List<SyncableFlag> queryFlags(List<SyncableFlag> flagList) {
- return mOverrides == null ? flagList : mOverrides; }
-
- @Override
- public void overrideFlag(SyncableFlag syncableFlag) {
- SyncableFlag match = findFlag(syncableFlag);
- if (match != null) {
- mOverrides.remove(match);
- }
-
- mOverrides.add(syncableFlag);
-
- for (IFeatureFlagsCallback cb : mCallbacks) {
- try {
- cb.onFlagChange(syncableFlag);
- } catch (RemoteException e) {
- // does not happen in fakes.
- }
- }
- }
-
- @Override
- public void resetFlag(SyncableFlag syncableFlag) {
- SyncableFlag match = findFlag(syncableFlag);
- if (match != null) {
- mOverrides.remove(match);
- }
-
- for (IFeatureFlagsCallback cb : mCallbacks) {
- try {
- cb.onFlagChange(syncableFlag);
- } catch (RemoteException e) {
- // does not happen in fakes.
- }
- }
- }
-
- private SyncableFlag findFlag(SyncableFlag syncableFlag) {
- SyncableFlag match = null;
- for (SyncableFlag sf : mOverrides) {
- if (sf.getName().equals(syncableFlag.getName())
- && sf.getNamespace().equals(syncableFlag.getNamespace())) {
- match = sf;
- break;
- }
- }
-
- return match;
- }
- @Override
public void registerCallback(IFeatureFlagsCallback callback) {
mCallbacks.add(callback);
}
diff --git a/services/flags/java/com/android/server/flags/DynamicFlagBinderDelegate.java b/services/flags/java/com/android/server/flags/DynamicFlagBinderDelegate.java
index 0db328792cf3..8da5aae6abdd 100644
--- a/services/flags/java/com/android/server/flags/DynamicFlagBinderDelegate.java
+++ b/services/flags/java/com/android/server/flags/DynamicFlagBinderDelegate.java
@@ -132,8 +132,25 @@ class DynamicFlagBinderDelegate {
// about changes coming in from adb, DeviceConfig, or other sources.
// And also so that we can keep flags relatively consistent across processes.
+ // If we already have a value cached, just use that.
+ String value = null;
DynamicFlagData data = mDynamicFlags.getOrNull(ns, name);
- String value = getFlagValue(ns, name, sf.getValue());
+ if (data != null) {
+ value = data.getValue();
+ } else {
+ // Put the value in the cache for future reference.
+ data = new DynamicFlagData(ns, name);
+ mDynamicFlags.setIfChanged(ns, name, data);
+ }
+ // If we're not in a release build, flags can be overridden locally on device.
+ if (!Build.IS_USER && value == null) {
+ value = mFlagStore.get(ns, name);
+ }
+ // If we still don't have a value, maybe DeviceConfig does?
+ // Fallback to sf.getValue() here as well.
+ if (value == null) {
+ value = DeviceConfig.getString(ns, name, sf.getValue());
+ }
// DeviceConfig listeners are per-namespace.
if (!mDynamicFlags.containsNamespace(ns)) {
DeviceConfig.addOnPropertiesChangedListener(
@@ -183,30 +200,6 @@ class DynamicFlagBinderDelegate {
}
}
- String getFlagValue(String namespace, String name, String defaultValue) {
- // If we already have a value cached, just use that.
- String value = null;
- DynamicFlagData data = mDynamicFlags.getOrNull(namespace, name);
- if (data != null) {
- value = data.getValue();
- } else {
- // Put the value in the cache for future reference.
- data = new DynamicFlagData(namespace, name);
- mDynamicFlags.setIfChanged(namespace, name, data);
- }
- // If we're not in a release build, flags can be overridden locally on device.
- if (!Build.IS_USER && value == null) {
- value = mFlagStore.get(namespace, name);
- }
- // If we still don't have a value, maybe DeviceConfig does?
- // Fallback to sf.getValue() here as well.
- if (value == null) {
- value = DeviceConfig.getString(namespace, name, defaultValue);
- }
-
- return value;
- }
-
private static class DynamicFlagData {
private final String mNamespace;
private final String mName;
diff --git a/services/flags/java/com/android/server/flags/FeatureFlagsBinder.java b/services/flags/java/com/android/server/flags/FeatureFlagsBinder.java
index 1fa85325aea6..86b464dbdf5d 100644
--- a/services/flags/java/com/android/server/flags/FeatureFlagsBinder.java
+++ b/services/flags/java/com/android/server/flags/FeatureFlagsBinder.java
@@ -24,9 +24,6 @@ import android.flags.SyncableFlag;
import android.os.Build;
import android.os.ParcelFileDescriptor;
-import com.android.internal.flags.CoreFlags;
-import com.android.server.flags.FeatureFlagsService.PermissionsChecker;
-
import java.io.FileOutputStream;
import java.util.ArrayList;
import java.util.List;
@@ -36,16 +33,11 @@ class FeatureFlagsBinder extends IFeatureFlags.Stub {
private final FlagsShellCommand mShellCommand;
private final FlagCache<String> mFlagCache = new FlagCache<>();
private final DynamicFlagBinderDelegate mDynamicFlagDelegate;
- private final PermissionsChecker mPermissionsChecker;
- FeatureFlagsBinder(
- FlagOverrideStore flagStore,
- FlagsShellCommand shellCommand,
- PermissionsChecker permissionsChecker) {
+ FeatureFlagsBinder(FlagOverrideStore flagStore, FlagsShellCommand shellCommand) {
mFlagStore = flagStore;
mShellCommand = shellCommand;
mDynamicFlagDelegate = new DynamicFlagBinderDelegate(flagStore);
- mPermissionsChecker = permissionsChecker;
}
@Override
@@ -58,29 +50,11 @@ class FeatureFlagsBinder extends IFeatureFlags.Stub {
mDynamicFlagDelegate.unregisterCallback(getCallingPid(), callback);
}
- // Note: The internals of this method should be kept in sync with queryFlags
- // as they both should return identical results. The difference is that this method
- // caches any values it receives and/or reads, whereas queryFlags does not.
-
@Override
public List<SyncableFlag> syncFlags(List<SyncableFlag> incomingFlags) {
int pid = getCallingPid();
List<SyncableFlag> outputFlags = new ArrayList<>();
-
- boolean hasFullSyncPrivileges = false;
- SecurityException permissionFailureException = null;
- try {
- assertSyncPermission();
- hasFullSyncPrivileges = true;
- } catch (SecurityException e) {
- permissionFailureException = e;
- }
-
for (SyncableFlag sf : incomingFlags) {
- if (!hasFullSyncPrivileges && !CoreFlags.isCoreFlag(sf)) {
- throw permissionFailureException;
- }
-
String ns = sf.getNamespace();
String name = sf.getName();
SyncableFlag outFlag;
@@ -102,58 +76,6 @@ class FeatureFlagsBinder extends IFeatureFlags.Stub {
return outputFlags;
}
- @Override
- public void overrideFlag(SyncableFlag flag) {
- assertWritePermission();
- mFlagStore.set(flag.getNamespace(), flag.getName(), flag.getValue());
- }
-
- @Override
- public void resetFlag(SyncableFlag flag) {
- assertWritePermission();
- mFlagStore.erase(flag.getNamespace(), flag.getName());
- }
-
- @Override
- public List<SyncableFlag> queryFlags(List<SyncableFlag> incomingFlags) {
- assertSyncPermission();
- List<SyncableFlag> outputFlags = new ArrayList<>();
- for (SyncableFlag sf : incomingFlags) {
- String ns = sf.getNamespace();
- String name = sf.getName();
- String value;
- String storeValue = mFlagStore.get(ns, name);
- boolean overridden = storeValue != null;
-
- if (sf.isDynamic()) {
- value = mDynamicFlagDelegate.getFlagValue(ns, name, sf.getValue());
- } else {
- value = mFlagCache.getOrNull(ns, name);
- if (value == null) {
- value = Build.IS_USER ? null : storeValue;
- if (value == null) {
- value = sf.getValue();
- }
- }
- }
- outputFlags.add(new SyncableFlag(
- sf.getNamespace(), sf.getName(), value, sf.isDynamic(), overridden));
- }
-
- return outputFlags;
- }
-
- private void assertSyncPermission() {
- mPermissionsChecker.assertSyncPermission();
- clearCallingIdentity();
- }
-
- private void assertWritePermission() {
- mPermissionsChecker.assertWritePermission();
- clearCallingIdentity();
- }
-
-
@SystemApi
public int handleShellCommand(
@NonNull ParcelFileDescriptor in,
diff --git a/services/flags/java/com/android/server/flags/FeatureFlagsService.java b/services/flags/java/com/android/server/flags/FeatureFlagsService.java
index 93b9e9e0dc8c..a9de17348639 100644
--- a/services/flags/java/com/android/server/flags/FeatureFlagsService.java
+++ b/services/flags/java/com/android/server/flags/FeatureFlagsService.java
@@ -15,15 +15,10 @@
*/
package com.android.server.flags;
-import static android.Manifest.permission.SYNC_FLAGS;
-import static android.Manifest.permission.WRITE_FLAGS;
-
import android.content.Context;
-import android.content.pm.PackageManager;
import android.flags.FeatureFlags;
import android.util.Slog;
-import com.android.internal.annotations.VisibleForTesting;
import com.android.server.SystemService;
/**
@@ -58,56 +53,10 @@ public class FeatureFlagsService extends SystemService {
@Override
public void onStart() {
Slog.d(TAG, "Started Feature Flag Service");
- FeatureFlagsBinder service = new FeatureFlagsBinder(
- mFlagStore, mShellCommand, new PermissionsChecker(getContext()));
+ FeatureFlagsBinder service = new FeatureFlagsBinder(mFlagStore, mShellCommand);
publishBinderService(
Context.FEATURE_FLAGS_SERVICE, service);
publishLocalService(FeatureFlags.class, new FeatureFlags(service));
}
- @Override
- public void onBootPhase(int phase) {
- super.onBootPhase(phase);
-
- if (phase == PHASE_SYSTEM_SERVICES_READY) {
- // Immediately sync our core flags so that they get locked in. We don't want third-party
- // apps to override them, and syncing immediately is the easiest way to prevent that.
- FeatureFlags.getInstance().sync();
- }
- }
-
- /**
- * Delegate for checking flag permissions.
- */
- @VisibleForTesting
- public static class PermissionsChecker {
- private final Context mContext;
-
- @VisibleForTesting(visibility = VisibleForTesting.Visibility.PACKAGE)
- public PermissionsChecker(Context context) {
- mContext = context;
- }
-
- /**
- * Ensures that the caller has {@link SYNC_FLAGS} permission.
- */
- @VisibleForTesting(visibility = VisibleForTesting.Visibility.PACKAGE)
- public void assertSyncPermission() {
- if (mContext.checkCallingOrSelfPermission(SYNC_FLAGS)
- != PackageManager.PERMISSION_GRANTED) {
- throw new SecurityException(
- "Non-core flag queried. Requires SYNC_FLAGS permission!");
- }
- }
-
- /**
- * Ensures that the caller has {@link WRITE_FLAGS} permission.
- */
- @VisibleForTesting(visibility = VisibleForTesting.Visibility.PACKAGE)
- public void assertWritePermission() {
- if (mContext.checkCallingPermission(WRITE_FLAGS) != PackageManager.PERMISSION_GRANTED) {
- throw new SecurityException("Requires WRITE_FLAGS permission!");
- }
- }
- }
}
diff --git a/services/flags/java/com/android/server/flags/FlagOverrideStore.java b/services/flags/java/com/android/server/flags/FlagOverrideStore.java
index b1ddc7e67f68..9866b1c7bb1c 100644
--- a/services/flags/java/com/android/server/flags/FlagOverrideStore.java
+++ b/services/flags/java/com/android/server/flags/FlagOverrideStore.java
@@ -53,8 +53,7 @@ public class FlagOverrideStore {
}
/** Put a value in the store. */
- @VisibleForTesting
- public void set(String namespace, String name, String value) {
+ void set(String namespace, String name, String value) {
mSettingsProxy.putString(getPropName(namespace, name), value);
mCallback.onFlagChanged(namespace, name, value);
}
@@ -66,8 +65,7 @@ public class FlagOverrideStore {
}
/** Erase a value from the store. */
- @VisibleForTesting
- public void erase(String namespace, String name) {
+ void erase(String namespace, String name) {
set(namespace, name, null);
}
diff --git a/services/tests/servicestests/src/com/android/server/flags/FeatureFlagsServiceTest.java b/services/tests/servicestests/src/com/android/server/flags/FeatureFlagsServiceTest.java
index df4731fb0bb7..8455b88ae9af 100644
--- a/services/tests/servicestests/src/com/android/server/flags/FeatureFlagsServiceTest.java
+++ b/services/tests/servicestests/src/com/android/server/flags/FeatureFlagsServiceTest.java
@@ -21,7 +21,6 @@ import static com.google.common.truth.Truth.assertWithMessage;
import static org.junit.Assert.fail;
import static org.mockito.Mockito.any;
-import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.eq;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
@@ -61,16 +60,13 @@ public class FeatureFlagsServiceTest {
private IFeatureFlagsCallback mIFeatureFlagsCallback;
@Mock
private IBinder mIFeatureFlagsCallbackAsBinder;
- @Mock
- private FeatureFlagsService.PermissionsChecker mPermissionsChecker;
private FeatureFlagsBinder mFeatureFlagsService;
@Before
public void setup() {
when(mIFeatureFlagsCallback.asBinder()).thenReturn(mIFeatureFlagsCallbackAsBinder);
- mFeatureFlagsService = new FeatureFlagsBinder(
- mFlagStore, mFlagCommand, mPermissionsChecker);
+ mFeatureFlagsService = new FeatureFlagsBinder(mFlagStore, mFlagCommand);
}
@Test
@@ -84,40 +80,6 @@ public class FeatureFlagsServiceTest {
}
@Test
- public void testOverrideFlag_requiresWritePermission() {
- SecurityException exc = new SecurityException("not allowed");
- doThrow(exc).when(mPermissionsChecker).assertWritePermission();
-
- SyncableFlag f = new SyncableFlag(NS, "a", "false", false);
-
- try {
- mFeatureFlagsService.overrideFlag(f);
- fail("Should have thrown exception");
- } catch (SecurityException e) {
- assertThat(exc).isEqualTo(e);
- } catch (Exception e) {
- fail("should have thrown a security exception");
- }
- }
-
- @Test
- public void testResetFlag_requiresWritePermission() {
- SecurityException exc = new SecurityException("not allowed");
- doThrow(exc).when(mPermissionsChecker).assertWritePermission();
-
- SyncableFlag f = new SyncableFlag(NS, "a", "false", false);
-
- try {
- mFeatureFlagsService.resetFlag(f);
- fail("Should have thrown exception");
- } catch (SecurityException e) {
- assertThat(exc).isEqualTo(e);
- } catch (Exception e) {
- fail("should have thrown a security exception");
- }
- }
-
- @Test
public void testSyncFlags_noOverrides() {
List<SyncableFlag> inputFlags = List.of(
new SyncableFlag(NS, "a", "false", false),
@@ -175,6 +137,7 @@ public class FeatureFlagsServiceTest {
}
}
+
@Test
public void testSyncFlags_twoCallsWithDifferentDefaults() {
List<SyncableFlag> inputFlagsFirst = List.of(
@@ -208,83 +171,6 @@ public class FeatureFlagsServiceTest {
.that(found).isTrue();
}
- @Test
- public void testQueryFlags_onlyOnce() {
- List<SyncableFlag> inputFlags = List.of(
- new SyncableFlag(NS, "a", "false", false),
- new SyncableFlag(NS, "b", "true", false),
- new SyncableFlag(NS, "c", "false", false)
- );
-
- List<SyncableFlag> outputFlags = mFeatureFlagsService.queryFlags(inputFlags);
-
- assertThat(inputFlags.size()).isEqualTo(outputFlags.size());
-
- for (SyncableFlag inpF: inputFlags) {
- boolean found = false;
- for (SyncableFlag outF : outputFlags) {
- if (compareSyncableFlagsNames(inpF, outF)) {
- found = true;
- break;
- }
- }
- assertWithMessage("Failed to find input flag " + inpF + " in the output")
- .that(found).isTrue();
- }
- }
-
- @Test
- public void testQueryFlags_twoCallsWithDifferentDefaults() {
- List<SyncableFlag> inputFlagsFirst = List.of(
- new SyncableFlag(NS, "a", "false", false)
- );
- List<SyncableFlag> inputFlagsSecond = List.of(
- new SyncableFlag(NS, "a", "true", false),
- new SyncableFlag(NS, "b", "false", false)
- );
-
- List<SyncableFlag> outputFlagsFirst = mFeatureFlagsService.queryFlags(inputFlagsFirst);
- List<SyncableFlag> outputFlagsSecond = mFeatureFlagsService.queryFlags(inputFlagsSecond);
-
- assertThat(inputFlagsFirst.size()).isEqualTo(outputFlagsFirst.size());
- assertThat(inputFlagsSecond.size()).isEqualTo(outputFlagsSecond.size());
-
- // This test only cares that the "a" flag passed in the second time came out with the
- // same value that was passed in (i.e. it wasn't cached).
-
- boolean found = false;
- for (SyncableFlag second : outputFlagsSecond) {
- if (compareSyncableFlagsNames(second, inputFlagsSecond.get(0))) {
- found = true;
- assertThat(second.getValue()).isEqualTo(inputFlagsSecond.get(0).getValue());
- break;
- }
- }
-
- assertWithMessage(
- "Failed to find flag " + inputFlagsSecond.get(0) + " in the second calls output")
- .that(found).isTrue();
- }
-
- @Test
- public void testOverrideFlag() {
- SyncableFlag f = new SyncableFlag(NS, "a", "false", false);
-
- mFeatureFlagsService.overrideFlag(f);
-
- verify(mFlagStore).set(f.getNamespace(), f.getName(), f.getValue());
- }
-
- @Test
- public void testResetFlag() {
- SyncableFlag f = new SyncableFlag(NS, "a", "false", false);
-
- mFeatureFlagsService.resetFlag(f);
-
- verify(mFlagStore).erase(f.getNamespace(), f.getName());
- }
-
-
private static boolean compareSyncableFlagsNames(SyncableFlag a, SyncableFlag b) {
return a.getNamespace().equals(b.getNamespace())
&& a.getName().equals(b.getName())