Merge "Remove unnecessary arguments from isTetheringFeatureEnabled" into main

2 files changed, 44 insertions, 52 deletions

diff --git a/common/device/com/android/net/module/util/DeviceConfigUtils.java b/common/device/com/android/net/module/util/DeviceConfigUtils.java
index fb130f67..771d44fa 100644
--- a/common/device/com/android/net/module/util/DeviceConfigUtils.java
+++ b/common/device/com/android/net/module/util/DeviceConfigUtils.java
@@ -200,24 +200,20 @@ public final class DeviceConfigUtils {
      *
      * This is useful to ensure that if a module install is rolled back, flags are not left fully
      * rolled out on a version where they have not been well tested.
+     *
+     * If the feature is disabled by default and enabled by flag push, this method should be used.
+     * If the feature is enabled by default and disabled by flag push (kill switch),
+     * {@link #isTetheringFeatureNotChickenedOut(String)} should be used.
+     *
      * @param context The global context information about an app environment.
-     * @param namespace The namespace containing the property to look up.
      * @param name The name of the property to look up.
-     * @param moduleName The mainline module name which is released as apex.
-     * @param defaultEnabled The value to return if the property does not exist or its value is
-     *                       null.
      * @return true if this feature is enabled, or false if disabled.
      */
     public static boolean isTetheringFeatureEnabled(@NonNull Context context,
-            @NonNull String namespace, @NonNull String name, @NonNull String moduleName,
-            boolean defaultEnabled) {
-        // TODO: migrate callers to a non-generic isTetheringFeatureEnabled method.
-        if (!TETHERING_MODULE_NAME.equals(moduleName)) {
-            throw new IllegalArgumentException(
-                    "This method is only usable by the tethering module");
-        }
+            @NonNull String name) {
         final long packageVersion = getTetheringModuleVersion(context);
-        return isFeatureEnabled(context, packageVersion, namespace, name, defaultEnabled);
+        return isFeatureEnabled(context, packageVersion, NAMESPACE_TETHERING, name,
+                false /* defaultEnabled */);
     }
 
     private static boolean isFeatureEnabled(@NonNull Context context, long packageVersion,
diff --git a/common/tests/unit/src/com/android/net/module/util/DeviceConfigUtilsTest.java b/common/tests/unit/src/com/android/net/module/util/DeviceConfigUtilsTest.java
index f259e687..7a71a4ae 100644
--- a/common/tests/unit/src/com/android/net/module/util/DeviceConfigUtilsTest.java
+++ b/common/tests/unit/src/com/android/net/module/util/DeviceConfigUtilsTest.java
@@ -84,7 +84,6 @@ public class DeviceConfigUtilsTest {
     // that manifest, and is reflected in android.content.pm.ApplicationInfo. Contrary to the APEX
     // (module) name, different package names are typically used to identify the organization that
     // built and signed the APEX modules.
-    private static final String TEST_APEX_NAME = "com.android.tethering";
     private static final String TEST_APEX_PACKAGE_NAME = "com.prefix.android.tethering";
     private static final String TEST_GO_APEX_PACKAGE_NAME = "com.prefix.android.go.tethering";
     private static final String TEST_CONNRES_PACKAGE_NAME =
@@ -230,24 +229,24 @@ public class DeviceConfigUtilsTest {
 
     @Test
     public void testFeatureIsEnabled() {
-        doReturn(TEST_FLAG_VALUE_STRING).when(() -> DeviceConfig.getProperty(eq(TEST_NAME_SPACE),
-                eq(TEST_EXPERIMENT_FLAG)));
+        doReturn(TEST_FLAG_VALUE_STRING).when(() -> DeviceConfig.getProperty(TEST_NAME_SPACE,
+                TEST_EXPERIMENT_FLAG));
+        doReturn(TEST_FLAG_VALUE_STRING).when(() -> DeviceConfig.getProperty(NAMESPACE_TETHERING,
+                TEST_EXPERIMENT_FLAG));
         assertTrue(DeviceConfigUtils.isFeatureEnabled(mContext, TEST_NAME_SPACE,
                 TEST_EXPERIMENT_FLAG));
-        assertTrue(DeviceConfigUtils.isTetheringFeatureEnabled(mContext, TEST_NAME_SPACE,
-                TEST_EXPERIMENT_FLAG, TEST_APEX_NAME, false /* defaultEnabled */));
+        assertTrue(DeviceConfigUtils.isTetheringFeatureEnabled(mContext, TEST_EXPERIMENT_FLAG));
     }
 
     @Test
     public void testFeatureDefaultEnabled() {
-        doReturn(null).when(() -> DeviceConfig.getProperty(eq(TEST_NAME_SPACE),
-                eq(TEST_EXPERIMENT_FLAG)));
+        doReturn(null).when(() -> DeviceConfig.getProperty(TEST_NAME_SPACE,
+                TEST_EXPERIMENT_FLAG));
+        doReturn(null).when(() -> DeviceConfig.getProperty(NAMESPACE_TETHERING,
+                TEST_EXPERIMENT_FLAG));
         assertFalse(DeviceConfigUtils.isFeatureEnabled(mContext, TEST_NAME_SPACE,
                 TEST_EXPERIMENT_FLAG));
-        assertFalse(DeviceConfigUtils.isTetheringFeatureEnabled(mContext, TEST_NAME_SPACE,
-                TEST_EXPERIMENT_FLAG, TEST_APEX_NAME, false /* defaultEnabled */));
-        assertTrue(DeviceConfigUtils.isTetheringFeatureEnabled(mContext, TEST_NAME_SPACE,
-                TEST_EXPERIMENT_FLAG, TEST_APEX_NAME, true /* defaultEnabled */));
+        assertFalse(DeviceConfigUtils.isTetheringFeatureEnabled(mContext, TEST_EXPERIMENT_FLAG));
     }
 
     @Test
@@ -255,32 +254,33 @@ public class DeviceConfigUtilsTest {
         doThrow(NameNotFoundException.class).when(mPm).getPackageInfo(anyString(), anyInt());
 
         // Feature should be enabled by flag value "1".
-        doReturn("1").when(() -> DeviceConfig.getProperty(eq(TEST_NAME_SPACE),
-                eq(TEST_EXPERIMENT_FLAG)));
+        doReturn("1").when(() -> DeviceConfig.getProperty(TEST_NAME_SPACE,
+                TEST_EXPERIMENT_FLAG));
+        doReturn("1").when(() -> DeviceConfig.getProperty(NAMESPACE_TETHERING,
+                TEST_EXPERIMENT_FLAG));
         assertTrue(DeviceConfigUtils.isFeatureEnabled(mContext, TEST_NAME_SPACE,
                 TEST_EXPERIMENT_FLAG));
-        assertTrue(DeviceConfigUtils.isTetheringFeatureEnabled(mContext, TEST_NAME_SPACE,
-                TEST_EXPERIMENT_FLAG, TEST_APEX_NAME, false /* defaultEnabled */));
+        assertTrue(DeviceConfigUtils.isTetheringFeatureEnabled(mContext, TEST_EXPERIMENT_FLAG));
 
         // Feature should be disabled by flag value "999999999".
-        doReturn("999999999").when(() -> DeviceConfig.getProperty(eq(TEST_NAME_SPACE),
-                eq(TEST_EXPERIMENT_FLAG)));
+        doReturn("999999999").when(() -> DeviceConfig.getProperty(TEST_NAME_SPACE,
+                TEST_EXPERIMENT_FLAG));
+        doReturn("999999999").when(() -> DeviceConfig.getProperty(NAMESPACE_TETHERING,
+                TEST_EXPERIMENT_FLAG));
         assertFalse(DeviceConfigUtils.isFeatureEnabled(mContext, TEST_NAME_SPACE,
                 TEST_EXPERIMENT_FLAG));
-        assertFalse(DeviceConfigUtils.isTetheringFeatureEnabled(mContext, TEST_NAME_SPACE,
-                TEST_EXPERIMENT_FLAG, TEST_APEX_NAME, false /* defaultEnabled */));
+        assertFalse(DeviceConfigUtils.isTetheringFeatureEnabled(mContext, TEST_EXPERIMENT_FLAG));
 
         // Follow defaultEnabled if the flag is not set
-        doReturn(null).when(() -> DeviceConfig.getProperty(eq(TEST_NAME_SPACE),
-                eq(TEST_EXPERIMENT_FLAG)));
+        doReturn(null).when(() -> DeviceConfig.getProperty(TEST_NAME_SPACE,
+                TEST_EXPERIMENT_FLAG));
+        doReturn(null).when(() -> DeviceConfig.getProperty(NAMESPACE_TETHERING,
+                TEST_EXPERIMENT_FLAG));
         assertFalse(DeviceConfigUtils.isFeatureEnabled(mContext, TEST_NAME_SPACE,
                 TEST_EXPERIMENT_FLAG, false /* defaultEnabled */));
         assertTrue(DeviceConfigUtils.isFeatureEnabled(mContext, TEST_NAME_SPACE,
                 TEST_EXPERIMENT_FLAG, true /* defaultEnabled */));
-        assertFalse(DeviceConfigUtils.isTetheringFeatureEnabled(mContext, TEST_NAME_SPACE,
-                TEST_EXPERIMENT_FLAG, TEST_APEX_NAME, false /* defaultEnabled */));
-        assertTrue(DeviceConfigUtils.isTetheringFeatureEnabled(mContext, TEST_NAME_SPACE,
-                TEST_EXPERIMENT_FLAG, TEST_APEX_NAME, true /* defaultEnabled */));
+        assertFalse(DeviceConfigUtils.isTetheringFeatureEnabled(mContext, TEST_EXPERIMENT_FLAG));
     }
 
     @Test
@@ -290,19 +290,17 @@ public class DeviceConfigUtilsTest {
         doReturn(mApexPackageInfo).when(mPm).getPackageInfo(
                 eq(TEST_GO_APEX_PACKAGE_NAME), anyInt());
         doReturn("0").when(() -> DeviceConfig.getProperty(
-                eq(TEST_NAME_SPACE), eq(TEST_EXPERIMENT_FLAG)));
+                TEST_NAME_SPACE, TEST_EXPERIMENT_FLAG));
+        doReturn("0").when(() -> DeviceConfig.getProperty(
+                NAMESPACE_TETHERING, TEST_EXPERIMENT_FLAG));
 
         assertFalse(DeviceConfigUtils.isFeatureEnabled(mContext, TEST_NAME_SPACE,
                 TEST_EXPERIMENT_FLAG));
-        assertFalse(DeviceConfigUtils.isTetheringFeatureEnabled(mContext, TEST_NAME_SPACE,
-                TEST_EXPERIMENT_FLAG, TEST_APEX_NAME, false /* defaultEnabled */));
-        assertTrue(DeviceConfigUtils.isTetheringFeatureEnabled(mContext, TEST_NAME_SPACE,
-                TEST_EXPERIMENT_FLAG, TEST_APEX_NAME, true /* defaultEnabled */));
+        assertFalse(DeviceConfigUtils.isTetheringFeatureEnabled(mContext, TEST_EXPERIMENT_FLAG));
 
-        doReturn(TEST_FLAG_VALUE_STRING).when(() -> DeviceConfig.getProperty(eq(TEST_NAME_SPACE),
-                eq(TEST_EXPERIMENT_FLAG)));
-        assertTrue(DeviceConfigUtils.isTetheringFeatureEnabled(mContext, TEST_NAME_SPACE,
-                TEST_EXPERIMENT_FLAG, TEST_APEX_NAME, false /* defaultEnabled */));
+        doReturn(TEST_FLAG_VALUE_STRING).when(() -> DeviceConfig.getProperty(NAMESPACE_TETHERING,
+                TEST_EXPERIMENT_FLAG));
+        assertTrue(DeviceConfigUtils.isTetheringFeatureEnabled(mContext, TEST_EXPERIMENT_FLAG));
     }
 
     @Test
@@ -322,12 +320,10 @@ public class DeviceConfigUtilsTest {
 
     @Test
     public void testFeatureIsEnabledCaching_APEX() throws Exception {
-        doReturn(TEST_FLAG_VALUE_STRING).when(() -> DeviceConfig.getProperty(eq(TEST_NAME_SPACE),
-                eq(TEST_EXPERIMENT_FLAG)));
-        assertTrue(DeviceConfigUtils.isTetheringFeatureEnabled(mContext, TEST_NAME_SPACE,
-                TEST_EXPERIMENT_FLAG, TEST_APEX_NAME, false /* defaultEnabled */));
-        assertTrue(DeviceConfigUtils.isTetheringFeatureEnabled(mContext, TEST_NAME_SPACE,
-                TEST_EXPERIMENT_FLAG, TEST_APEX_NAME, false /* defaultEnabled */));
+        doReturn(TEST_FLAG_VALUE_STRING).when(() -> DeviceConfig.getProperty(NAMESPACE_TETHERING,
+                TEST_EXPERIMENT_FLAG));
+        assertTrue(DeviceConfigUtils.isTetheringFeatureEnabled(mContext, TEST_EXPERIMENT_FLAG));
+        assertTrue(DeviceConfigUtils.isTetheringFeatureEnabled(mContext, TEST_EXPERIMENT_FLAG));
 
         // Package info is only queried once
         verify(mPm, times(1)).getPackageInfo(anyString(), anyInt());