Check if DUN APN exists to decide isDunRequired

Change isDunRequired variable from integer to boolean.
Before this change tethering would reference
config_tether_upstream_types reource configuraton to decide
isDunRequired for DUN_UNSPECIFIED case.
After the change, if isDunRequired is true, tethering would
request DUN. If it is false, tethering skip DUN type. In order
to backwards compatibility, tethering would not add mobile/hipri
type to upstream configuration if one of them already exist in
upstream configuration.

Test: - build, flash, booted
      - runtest frameworks-net
      - manully test with TMO, VZW SIM.
bug: 63150712

Change-Id: Ia998ea502674ff186d52af37e3fd8f07de495245

4 files changed, 113 insertions, 111 deletions

diff --git a/core/res/res/values/config.xml b/core/res/res/values/config.xml
index 0ffe2aad5e89..0f1054707e59 100644
--- a/core/res/res/values/config.xml
+++ b/core/res/res/values/config.xml
@@ -477,22 +477,16 @@
              - the return value of TelephonyManager.getTetherApnRequired()
                determines how the array is further modified:
 
-                   * DUN_REQUIRED
-                     TYPE_MOBILE is removed (if present)
-                     TYPE_MOBILE_HIPRI is removed (if present)
-                     TYPE_MOBILE_DUN is appended (if not already present)
-
-                   * DUN_NOT_REQUIRED
-                     TYPE_MOBILE_DUN is removed (if present)
-                     TYPE_MOBILE is appended (if not already present)
-                     TYPE_MOBILE_HIPRI is appended (if not already present)
-
-                   * DUN_UNSPECIFIED
-                     if any of TYPE_MOBILE{,_DUN,_HIPRI} are present:
-                         change nothing
-                     else:
-                         TYPE_MOBILE is appended
-                         TYPE_MOBILE_HIPRI is appended
+                   * TRUE (DUN REQUIRED).
+                     TYPE_MOBILE is removed (if present).
+                     TYPE_MOBILE_HIPRI is removed (if present).
+                     TYPE_MOBILE_DUN is appended (if not already present).
+
+                   * FALSE (DUN NOT REQUIRED).
+                     TYPE_MOBILE_DUN is removed (if present).
+                     If both of TYPE_MOBILE{,_HIPRI} are not present:
+                        TYPE_MOBILE is appended.
+                        TYPE_MOBILE_HIPRI is appended.
 
          For other changes applied to this list, now and in the future, see
          com.android.server.connectivity.tethering.TetheringConfiguration.
diff --git a/services/core/java/com/android/server/connectivity/Tethering.java b/services/core/java/com/android/server/connectivity/Tethering.java
index c91e1a12078e..13ff30d6ad3e 100644
--- a/services/core/java/com/android/server/connectivity/Tethering.java
+++ b/services/core/java/com/android/server/connectivity/Tethering.java
@@ -289,8 +289,8 @@ public class Tethering extends BaseNetworkObserver {
     }
 
     private void maybeUpdateConfiguration() {
-        final int dunCheck = TetheringConfiguration.checkDunRequired(mContext);
-        if (dunCheck == mConfig.dunCheck) return;
+        final boolean isDunRequired = TetheringConfiguration.checkDunRequired(mContext);
+        if (isDunRequired == mConfig.isDunRequired) return;
         updateConfiguration();
     }
 
diff --git a/services/core/java/com/android/server/connectivity/tethering/TetheringConfiguration.java b/services/core/java/com/android/server/connectivity/tethering/TetheringConfiguration.java
index 8a46ff18979f..935b79546d63 100644
--- a/services/core/java/com/android/server/connectivity/tethering/TetheringConfiguration.java
+++ b/services/core/java/com/android/server/connectivity/tethering/TetheringConfiguration.java
@@ -67,11 +67,6 @@ public class TetheringConfiguration {
 
     private static final String[] EMPTY_STRING_ARRAY = new String[0];
 
-    @VisibleForTesting
-    public static final int DUN_NOT_REQUIRED = 0;
-    public static final int DUN_REQUIRED = 1;
-    public static final int DUN_UNSPECIFIED = 2;
-
     // Default ranges used for the legacy DHCP server.
     // USB is  192.168.42.1 and 255.255.255.0
     // Wifi is 192.168.43.1 and 255.255.255.0
@@ -90,7 +85,6 @@ public class TetheringConfiguration {
     public final String[] tetherableUsbRegexs;
     public final String[] tetherableWifiRegexs;
     public final String[] tetherableBluetoothRegexs;
-    public final int dunCheck;
     public final boolean isDunRequired;
     public final boolean chooseUpstreamAutomatically;
     public final Collection<Integer> preferredUpstreamIfaceTypes;
@@ -116,12 +110,10 @@ public class TetheringConfiguration {
         tetherableWifiRegexs = getResourceStringArray(res, config_tether_wifi_regexs);
         tetherableBluetoothRegexs = getResourceStringArray(res, config_tether_bluetooth_regexs);
 
-        dunCheck = checkDunRequired(ctx);
-        configLog.log("DUN check returned: " + dunCheckString(dunCheck));
+        isDunRequired = checkDunRequired(ctx);
 
         chooseUpstreamAutomatically = getResourceBoolean(res, config_tether_upstream_automatic);
-        preferredUpstreamIfaceTypes = getUpstreamIfaceTypes(res, dunCheck);
-        isDunRequired = preferredUpstreamIfaceTypes.contains(TYPE_MOBILE_DUN);
+        preferredUpstreamIfaceTypes = getUpstreamIfaceTypes(res, isDunRequired);
 
         legacyDhcpRanges = getLegacyDhcpRanges(res);
         defaultIPv4DNS = copy(DEFAULT_IPV4_DNS);
@@ -230,53 +222,43 @@ public class TetheringConfiguration {
         return upstreamNames;
     }
 
-    public static int checkDunRequired(Context ctx) {
+    /** Check whether dun is required. */
+    public static boolean checkDunRequired(Context ctx) {
         final TelephonyManager tm = (TelephonyManager) ctx.getSystemService(TELEPHONY_SERVICE);
-        return (tm != null) ? tm.getTetherApnRequired() : DUN_UNSPECIFIED;
-    }
-
-    private static String dunCheckString(int dunCheck) {
-        switch (dunCheck) {
-            case DUN_NOT_REQUIRED: return "DUN_NOT_REQUIRED";
-            case DUN_REQUIRED:     return "DUN_REQUIRED";
-            case DUN_UNSPECIFIED:  return "DUN_UNSPECIFIED";
-            default:
-                return String.format("UNKNOWN (%s)", dunCheck);
-        }
+        return (tm != null) ? tm.getTetherApnRequired() : false;
     }
 
-    private static Collection<Integer> getUpstreamIfaceTypes(Resources res, int dunCheck) {
+    private static Collection<Integer> getUpstreamIfaceTypes(Resources res, boolean dunRequired) {
         final int[] ifaceTypes = res.getIntArray(config_tether_upstream_types);
         final ArrayList<Integer> upstreamIfaceTypes = new ArrayList<>(ifaceTypes.length);
         for (int i : ifaceTypes) {
             switch (i) {
                 case TYPE_MOBILE:
                 case TYPE_MOBILE_HIPRI:
-                    if (dunCheck == DUN_REQUIRED) continue;
+                    if (dunRequired) continue;
                     break;
                 case TYPE_MOBILE_DUN:
-                    if (dunCheck == DUN_NOT_REQUIRED) continue;
+                    if (!dunRequired) continue;
                     break;
             }
             upstreamIfaceTypes.add(i);
         }
 
         // Fix up upstream interface types for DUN or mobile. NOTE: independent
-        // of the value of |dunCheck|, cell data of one form or another is
+        // of the value of |dunRequired|, cell data of one form or another is
         // *always* an upstream, regardless of the upstream interface types
         // specified by configuration resources.
-        if (dunCheck == DUN_REQUIRED) {
+        if (dunRequired) {
             appendIfNotPresent(upstreamIfaceTypes, TYPE_MOBILE_DUN);
-        } else if (dunCheck == DUN_NOT_REQUIRED) {
-            appendIfNotPresent(upstreamIfaceTypes, TYPE_MOBILE);
-            appendIfNotPresent(upstreamIfaceTypes, TYPE_MOBILE_HIPRI);
         } else {
-            // Fix upstream interface types for case DUN_UNSPECIFIED.
             // Do not modify if a cellular interface type is already present in the
             // upstream interface types. Add TYPE_MOBILE and TYPE_MOBILE_HIPRI if no
             // cellular interface types are found in the upstream interface types.
-            if (!(containsOneOf(upstreamIfaceTypes,
-                    TYPE_MOBILE_DUN, TYPE_MOBILE, TYPE_MOBILE_HIPRI))) {
+            // This preserves backwards compatibility and prevents the DUN and default
+            // mobile types incorrectly appearing together, which could happen on
+            // previous releases in the common case where checkDunRequired returned
+            // DUN_UNSPECIFIED.
+            if (!containsOneOf(upstreamIfaceTypes, TYPE_MOBILE, TYPE_MOBILE_HIPRI)) {
                 upstreamIfaceTypes.add(TYPE_MOBILE);
                 upstreamIfaceTypes.add(TYPE_MOBILE_HIPRI);
             }
diff --git a/tests/net/java/com/android/server/connectivity/tethering/TetheringConfigurationTest.java b/tests/net/java/com/android/server/connectivity/tethering/TetheringConfigurationTest.java
index 01b904d8f088..f8cfd8f3aed8 100644
--- a/tests/net/java/com/android/server/connectivity/tethering/TetheringConfigurationTest.java
+++ b/tests/net/java/com/android/server/connectivity/tethering/TetheringConfigurationTest.java
@@ -24,9 +24,7 @@ import static android.net.ConnectivityManager.TYPE_WIFI;
 import static android.provider.Settings.Global.TETHER_ENABLE_LEGACY_DHCP_SERVER;
 import static android.telephony.SubscriptionManager.INVALID_SUBSCRIPTION_ID;
 
-import static com.android.server.connectivity.tethering.TetheringConfiguration.DUN_NOT_REQUIRED;
-import static com.android.server.connectivity.tethering.TetheringConfiguration.DUN_REQUIRED;
-import static com.android.server.connectivity.tethering.TetheringConfiguration.DUN_UNSPECIFIED;
+import static com.android.internal.R.array.config_tether_upstream_types;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
@@ -52,6 +50,7 @@ import org.junit.runner.RunWith;
 import org.mockito.Mock;
 import org.mockito.MockitoAnnotations;
 
+import java.util.Arrays;
 import java.util.Iterator;
 
 @RunWith(AndroidJUnit4.class)
@@ -112,74 +111,103 @@ public class TetheringConfigurationTest {
                 .thenReturn(new String[]{ "test_wlan\\d" });
         when(mResources.getStringArray(com.android.internal.R.array.config_tether_bluetooth_regexs))
                 .thenReturn(new String[0]);
-        when(mResources.getIntArray(com.android.internal.R.array.config_tether_upstream_types))
-                .thenReturn(new int[0]);
+        when(mResources.getIntArray(config_tether_upstream_types)).thenReturn(new int[0]);
         when(mResources.getStringArray(
                 com.android.internal.R.array.config_mobile_hotspot_provision_app))
                 .thenReturn(new String[0]);
         mContentResolver = new MockContentResolver();
         mContentResolver.addProvider(Settings.AUTHORITY, new FakeSettingsProvider());
+        mHasTelephonyManager = true;
         mMockContext = new MockContext(mContext);
     }
 
-    @Test
-    public void testDunFromTelephonyManagerMeansDun() {
-        when(mResources.getIntArray(com.android.internal.R.array.config_tether_upstream_types))
-                .thenReturn(new int[]{TYPE_MOBILE, TYPE_WIFI, TYPE_MOBILE_HIPRI});
-        mHasTelephonyManager = true;
-        when(mTelephonyManager.getTetherApnRequired()).thenReturn(DUN_REQUIRED);
-
-        final TetheringConfiguration cfg = new TetheringConfiguration(
-                mMockContext, mLog, INVALID_SUBSCRIPTION_ID);
-        assertTrue(cfg.isDunRequired);
-        assertTrue(cfg.preferredUpstreamIfaceTypes.contains(TYPE_MOBILE_DUN));
-        assertFalse(cfg.preferredUpstreamIfaceTypes.contains(TYPE_MOBILE));
-        assertFalse(cfg.preferredUpstreamIfaceTypes.contains(TYPE_MOBILE_HIPRI));
-        // Just to prove we haven't clobbered Wi-Fi:
-        assertTrue(cfg.preferredUpstreamIfaceTypes.contains(TYPE_WIFI));
+    private TetheringConfiguration getTetheringConfiguration(int[] legacyTetherUpstreamTypes) {
+        when(mResources.getIntArray(config_tether_upstream_types)).thenReturn(
+                legacyTetherUpstreamTypes);
+        return new TetheringConfiguration(mMockContext, mLog, INVALID_SUBSCRIPTION_ID);
     }
 
     @Test
-    public void testDunNotRequiredFromTelephonyManagerMeansNoDun() {
-        when(mResources.getIntArray(com.android.internal.R.array.config_tether_upstream_types))
-                .thenReturn(new int[]{TYPE_MOBILE_DUN, TYPE_WIFI});
-        mHasTelephonyManager = true;
-        when(mTelephonyManager.getTetherApnRequired()).thenReturn(DUN_NOT_REQUIRED);
-
-        final TetheringConfiguration cfg = new TetheringConfiguration(
-                mMockContext, mLog, INVALID_SUBSCRIPTION_ID);
+    public void testNoTelephonyManagerMeansNoDun() {
+        mHasTelephonyManager = false;
+        final TetheringConfiguration cfg = getTetheringConfiguration(
+                new int[]{TYPE_MOBILE_DUN, TYPE_WIFI});
         assertFalse(cfg.isDunRequired);
         assertFalse(cfg.preferredUpstreamIfaceTypes.contains(TYPE_MOBILE_DUN));
-        assertTrue(cfg.preferredUpstreamIfaceTypes.contains(TYPE_MOBILE));
-        assertTrue(cfg.preferredUpstreamIfaceTypes.contains(TYPE_MOBILE_HIPRI));
         // Just to prove we haven't clobbered Wi-Fi:
         assertTrue(cfg.preferredUpstreamIfaceTypes.contains(TYPE_WIFI));
     }
 
     @Test
-    public void testDunFromUpstreamConfigMeansDun() {
-        when(mResources.getIntArray(com.android.internal.R.array.config_tether_upstream_types))
-                .thenReturn(new int[]{TYPE_MOBILE_DUN, TYPE_WIFI});
-        mHasTelephonyManager = false;
-        when(mTelephonyManager.getTetherApnRequired()).thenReturn(DUN_UNSPECIFIED);
+    public void testDunFromTelephonyManagerMeansDun() {
+        when(mTelephonyManager.getTetherApnRequired()).thenReturn(true);
+
+        final TetheringConfiguration cfgWifi = getTetheringConfiguration(new int[]{TYPE_WIFI});
+        final TetheringConfiguration cfgMobileWifiHipri = getTetheringConfiguration(
+                new int[]{TYPE_MOBILE, TYPE_WIFI, TYPE_MOBILE_HIPRI});
+        final TetheringConfiguration cfgWifiDun = getTetheringConfiguration(
+                new int[]{TYPE_WIFI, TYPE_MOBILE_DUN});
+        final TetheringConfiguration cfgMobileWifiHipriDun = getTetheringConfiguration(
+                new int[]{TYPE_MOBILE, TYPE_WIFI, TYPE_MOBILE_HIPRI, TYPE_MOBILE_DUN});
+
+        for (TetheringConfiguration cfg : Arrays.asList(cfgWifi, cfgMobileWifiHipri,
+                cfgWifiDun, cfgMobileWifiHipriDun)) {
+            String msg = "config=" + cfg.toString();
+            assertTrue(msg, cfg.isDunRequired);
+            assertTrue(msg, cfg.preferredUpstreamIfaceTypes.contains(TYPE_MOBILE_DUN));
+            assertFalse(msg, cfg.preferredUpstreamIfaceTypes.contains(TYPE_MOBILE));
+            assertFalse(msg, cfg.preferredUpstreamIfaceTypes.contains(TYPE_MOBILE_HIPRI));
+            // Just to prove we haven't clobbered Wi-Fi:
+            assertTrue(msg, cfg.preferredUpstreamIfaceTypes.contains(TYPE_WIFI));
+        }
+    }
+
+    @Test
+    public void testDunNotRequiredFromTelephonyManagerMeansNoDun() {
+        when(mTelephonyManager.getTetherApnRequired()).thenReturn(false);
+
+        final TetheringConfiguration cfgWifi = getTetheringConfiguration(new int[]{TYPE_WIFI});
+        final TetheringConfiguration cfgMobileWifiHipri = getTetheringConfiguration(
+                new int[]{TYPE_MOBILE, TYPE_WIFI, TYPE_MOBILE_HIPRI});
+        final TetheringConfiguration cfgWifiDun = getTetheringConfiguration(
+                new int[]{TYPE_WIFI, TYPE_MOBILE_DUN});
+        final TetheringConfiguration cfgWifiMobile = getTetheringConfiguration(
+                new int[]{TYPE_WIFI, TYPE_MOBILE});
+        final TetheringConfiguration cfgWifiHipri = getTetheringConfiguration(
+                new int[]{TYPE_WIFI, TYPE_MOBILE_HIPRI});
+        final TetheringConfiguration cfgMobileWifiHipriDun = getTetheringConfiguration(
+                new int[]{TYPE_MOBILE, TYPE_WIFI, TYPE_MOBILE_HIPRI, TYPE_MOBILE_DUN});
+
+        String msg;
+        // TYPE_MOBILE_DUN should not be present in all of the combinations.
+        // TYPE_WIFI should not be affected.
+        for (TetheringConfiguration cfg : Arrays.asList(cfgWifi, cfgMobileWifiHipri, cfgWifiDun,
+                cfgWifiMobile, cfgWifiHipri, cfgMobileWifiHipriDun)) {
+            msg = "config=" + cfg.toString();
+            assertFalse(msg, cfg.isDunRequired);
+            assertFalse(msg, cfg.preferredUpstreamIfaceTypes.contains(TYPE_MOBILE_DUN));
+            assertTrue(msg, cfg.preferredUpstreamIfaceTypes.contains(TYPE_WIFI));
+        }
+
+        for (TetheringConfiguration cfg : Arrays.asList(cfgWifi, cfgMobileWifiHipri, cfgWifiDun,
+                cfgMobileWifiHipriDun)) {
+            msg = "config=" + cfg.toString();
+            assertTrue(msg, cfg.preferredUpstreamIfaceTypes.contains(TYPE_MOBILE));
+            assertTrue(msg, cfg.preferredUpstreamIfaceTypes.contains(TYPE_MOBILE_HIPRI));
+        }
+        msg = "config=" + cfgWifiMobile.toString();
+        assertTrue(msg, cfgWifiMobile.preferredUpstreamIfaceTypes.contains(TYPE_MOBILE));
+        assertFalse(msg, cfgWifiMobile.preferredUpstreamIfaceTypes.contains(TYPE_MOBILE_HIPRI));
+        msg = "config=" + cfgWifiHipri.toString();
+        assertFalse(msg, cfgWifiHipri.preferredUpstreamIfaceTypes.contains(TYPE_MOBILE));
+        assertTrue(msg, cfgWifiHipri.preferredUpstreamIfaceTypes.contains(TYPE_MOBILE_HIPRI));
 
-        final TetheringConfiguration cfg = new TetheringConfiguration(
-                mMockContext, mLog, INVALID_SUBSCRIPTION_ID);
-        assertTrue(cfg.isDunRequired);
-        assertTrue(cfg.preferredUpstreamIfaceTypes.contains(TYPE_MOBILE_DUN));
-        // Just to prove we haven't clobbered Wi-Fi:
-        assertTrue(cfg.preferredUpstreamIfaceTypes.contains(TYPE_WIFI));
-        // Check that we have not added new cellular interface types
-        assertFalse(cfg.preferredUpstreamIfaceTypes.contains(TYPE_MOBILE));
-        assertFalse(cfg.preferredUpstreamIfaceTypes.contains(TYPE_MOBILE_HIPRI));
     }
 
     @Test
     public void testNoDefinedUpstreamTypesAddsEthernet() {
-        when(mResources.getIntArray(com.android.internal.R.array.config_tether_upstream_types))
-                .thenReturn(new int[]{});
-        mHasTelephonyManager = false;
-        when(mTelephonyManager.getTetherApnRequired()).thenReturn(DUN_UNSPECIFIED);
+        when(mResources.getIntArray(config_tether_upstream_types)).thenReturn(new int[]{});
+        when(mTelephonyManager.getTetherApnRequired()).thenReturn(false);
 
         final TetheringConfiguration cfg = new TetheringConfiguration(
                 mMockContext, mLog, INVALID_SUBSCRIPTION_ID);
@@ -187,8 +215,10 @@ public class TetheringConfigurationTest {
         assertTrue(upstreamIterator.hasNext());
         assertEquals(TYPE_ETHERNET, upstreamIterator.next().intValue());
         // The following is because the code always adds some kind of mobile
-        // upstream, be it DUN or, in this case where we use DUN_UNSPECIFIED,
-        // both vanilla and hipri mobile types.
+        // upstream, be it DUN or, in this case where DUN is NOT required,
+        // make sure there is at least one of MOBILE or HIPRI. With the empty
+        // list of the configuration in this test, it will always add both
+        // MOBILE and HIPRI, in that order.
         assertTrue(upstreamIterator.hasNext());
         assertEquals(TYPE_MOBILE, upstreamIterator.next().intValue());
         assertTrue(upstreamIterator.hasNext());
@@ -198,10 +228,9 @@ public class TetheringConfigurationTest {
 
     @Test
     public void testDefinedUpstreamTypesSansEthernetAddsEthernet() {
-        when(mResources.getIntArray(com.android.internal.R.array.config_tether_upstream_types))
-                .thenReturn(new int[]{TYPE_WIFI, TYPE_MOBILE_HIPRI});
-        mHasTelephonyManager = false;
-        when(mTelephonyManager.getTetherApnRequired()).thenReturn(DUN_UNSPECIFIED);
+        when(mResources.getIntArray(config_tether_upstream_types)).thenReturn(
+                new int[]{TYPE_WIFI, TYPE_MOBILE_HIPRI});
+        when(mTelephonyManager.getTetherApnRequired()).thenReturn(false);
 
         final TetheringConfiguration cfg = new TetheringConfiguration(
                 mMockContext, mLog, INVALID_SUBSCRIPTION_ID);
@@ -217,10 +246,9 @@ public class TetheringConfigurationTest {
 
     @Test
     public void testDefinedUpstreamTypesWithEthernetDoesNotAddEthernet() {
-        when(mResources.getIntArray(com.android.internal.R.array.config_tether_upstream_types))
+        when(mResources.getIntArray(config_tether_upstream_types))
                 .thenReturn(new int[]{TYPE_WIFI, TYPE_ETHERNET, TYPE_MOBILE_HIPRI});
-        mHasTelephonyManager = false;
-        when(mTelephonyManager.getTetherApnRequired()).thenReturn(DUN_UNSPECIFIED);
+        when(mTelephonyManager.getTetherApnRequired()).thenReturn(false);
 
         final TetheringConfiguration cfg = new TetheringConfiguration(
                 mMockContext, mLog, INVALID_SUBSCRIPTION_ID);
@@ -276,9 +304,7 @@ public class TetheringConfigurationTest {
         when(mResourcesForSubId.getStringArray(
                 com.android.internal.R.array.config_tether_bluetooth_regexs))
                 .thenReturn(new String[0]);
-        when(mResourcesForSubId.getIntArray(
-                com.android.internal.R.array.config_tether_upstream_types))
-                .thenReturn(new int[0]);
+        when(mResourcesForSubId.getIntArray(config_tether_upstream_types)).thenReturn(new int[0]);
         when(mResourcesForSubId.getStringArray(
                 com.android.internal.R.array.config_mobile_hotspot_provision_app))
                 .thenReturn(PROVISIONING_APP_NAME);