From b4859198f5c7be7766c13b8d309fda344d661c5a Mon Sep 17 00:00:00 2001 From: Remi NGUYEN VAN Date: Fri, 19 Feb 2021 07:05:39 +0000 Subject: [PATCH] Add flag to force choosing upstreams automatically The flag allows overriding the value of config_tether_upstream_automatic on released R devices, as issues have been found on devices where an overlay was used to set it to false. The flag is only usable on R devices, as S devices can either not set the setting to false, or fix the underlying issues. Bug: 173068192 Test: atest TetheringCoverageTests Change-Id: Id99638916e08e596fab21cedd7bfe39906ce2fe5 --- .../tethering/TetheringConfiguration.java | 25 +++++- .../tethering/EntitlementManagerTest.java | 9 ++- .../tethering/TetheringConfigurationTest.java | 79 +++++++++++++++++++ .../networkstack/tethering/TetheringTest.java | 5 ++ 4 files changed, 116 insertions(+), 2 deletions(-) diff --git a/Tethering/src/com/android/networkstack/tethering/TetheringConfiguration.java b/Tethering/src/com/android/networkstack/tethering/TetheringConfiguration.java index 799637c9b1..413b0cb6bc 100644 --- a/Tethering/src/com/android/networkstack/tethering/TetheringConfiguration.java +++ b/Tethering/src/com/android/networkstack/tethering/TetheringConfiguration.java @@ -33,6 +33,8 @@ import android.telephony.TelephonyManager; import android.text.TextUtils; import com.android.internal.annotations.VisibleForTesting; +import com.android.modules.utils.build.SdkLevel; +import com.android.net.module.util.DeviceConfigUtils; import java.io.PrintWriter; import java.util.ArrayList; @@ -93,6 +95,20 @@ public class TetheringConfiguration { public static final String TETHER_ENABLE_SELECT_ALL_PREFIX_RANGES = "tether_enable_select_all_prefix_ranges"; + /** + * Experiment flag to force choosing upstreams automatically. + * + * This setting is intended to help force-enable the feature on OEM devices that disabled it + * via resource overlays, and later noticed issues. To that end, it overrides + * config_tether_upstream_automatic when set to true. + * + * This flag is enabled if !=0 and less than the module APK version: see + * {@link DeviceConfigUtils#isFeatureEnabled}. It is also ignored after R, as later devices + * should just set config_tether_upstream_automatic to true instead. + */ + public static final String TETHER_FORCE_UPSTREAM_AUTOMATIC_VERSION = + "tether_force_upstream_automatic_version"; + /** * Default value that used to periodic polls tether offload stats from tethering offload HAL * to make the data warnings work. @@ -146,7 +162,9 @@ public class TetheringConfiguration { isDunRequired = checkDunRequired(ctx); - chooseUpstreamAutomatically = getResourceBoolean( + final boolean forceAutomaticUpstream = !SdkLevel.isAtLeastS() + && isFeatureEnabled(ctx, TETHER_FORCE_UPSTREAM_AUTOMATIC_VERSION); + chooseUpstreamAutomatically = forceAutomaticUpstream || getResourceBoolean( res, R.bool.config_tether_upstream_automatic, false /** defaultValue */); preferredUpstreamIfaceTypes = getUpstreamIfaceTypes(res, isDunRequired); @@ -453,6 +471,11 @@ public class TetheringConfiguration { return DeviceConfig.getProperty(NAMESPACE_CONNECTIVITY, name); } + @VisibleForTesting + protected boolean isFeatureEnabled(Context ctx, String featureVersionFlag) { + return DeviceConfigUtils.isFeatureEnabled(ctx, NAMESPACE_CONNECTIVITY, featureVersionFlag); + } + private Resources getResources(Context ctx, int subId) { if (subId != SubscriptionManager.INVALID_SUBSCRIPTION_ID) { return getResourcesForSubIdWrapper(ctx, subId); diff --git a/Tethering/tests/unit/src/com/android/networkstack/tethering/EntitlementManagerTest.java b/Tethering/tests/unit/src/com/android/networkstack/tethering/EntitlementManagerTest.java index 354e75356e..8cfa7d01f2 100644 --- a/Tethering/tests/unit/src/com/android/networkstack/tethering/EntitlementManagerTest.java +++ b/Tethering/tests/unit/src/com/android/networkstack/tethering/EntitlementManagerTest.java @@ -53,6 +53,8 @@ import static org.mockito.Mockito.when; import android.content.Context; import android.content.Intent; +import android.content.pm.PackageInfo; +import android.content.pm.PackageManager; import android.content.res.Resources; import android.net.util.SharedLog; import android.os.Bundle; @@ -87,11 +89,13 @@ public final class EntitlementManagerTest { private static final String[] PROVISIONING_APP_NAME = {"some", "app"}; private static final String PROVISIONING_NO_UI_APP_NAME = "no_ui_app"; private static final String PROVISIONING_APP_RESPONSE = "app_response"; + private static final String TEST_PACKAGE_NAME = "com.android.tethering.test"; @Mock private CarrierConfigManager mCarrierConfigManager; @Mock private Context mContext; @Mock private Resources mResources; @Mock private SharedLog mLog; + @Mock private PackageManager mPm; @Mock private EntitlementManager.OnUiEntitlementFailedListener mEntitlementFailedListener; // Like so many Android system APIs, these cannot be mocked because it is marked final. @@ -182,7 +186,7 @@ public final class EntitlementManagerTest { } @Before - public void setUp() { + public void setUp() throws Exception { MockitoAnnotations.initMocks(this); mMockingSession = mockitoSession() .initMocks(this) @@ -196,6 +200,9 @@ public final class EntitlementManagerTest { eq(EntitlementManager.DISABLE_PROVISIONING_SYSPROP_KEY), anyBoolean())); doReturn(null).when( () -> DeviceConfig.getProperty(eq(NAMESPACE_CONNECTIVITY), anyString())); + doReturn(mPm).when(mContext).getPackageManager(); + doReturn(TEST_PACKAGE_NAME).when(mContext).getPackageName(); + doReturn(new PackageInfo()).when(mPm).getPackageInfo(anyString(), anyInt()); when(mResources.getStringArray(R.array.config_tether_dhcp_range)) .thenReturn(new String[0]); diff --git a/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringConfigurationTest.java b/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringConfigurationTest.java index 237e2c27bf..1f4e371c94 100644 --- a/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringConfigurationTest.java +++ b/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringConfigurationTest.java @@ -30,12 +30,16 @@ import static com.android.dx.mockito.inline.extended.ExtendedMockito.mockitoSess import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.Matchers.eq; import static org.mockito.Mockito.when; import android.content.Context; +import android.content.pm.PackageInfo; +import android.content.pm.PackageManager; import android.content.res.Resources; import android.net.util.SharedLog; +import android.os.Build; import android.provider.DeviceConfig; import android.telephony.TelephonyManager; @@ -43,9 +47,14 @@ import androidx.test.filters.SmallTest; import androidx.test.runner.AndroidJUnit4; import com.android.internal.util.test.BroadcastInterceptingContext; +import com.android.net.module.util.DeviceConfigUtils; +import com.android.testutils.DevSdkIgnoreRule; +import com.android.testutils.DevSdkIgnoreRule.IgnoreAfter; +import com.android.testutils.DevSdkIgnoreRule.IgnoreUpTo; import org.junit.After; import org.junit.Before; +import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mock; @@ -60,13 +69,18 @@ import java.util.Iterator; public class TetheringConfigurationTest { private final SharedLog mLog = new SharedLog("TetheringConfigurationTest"); + @Rule public final DevSdkIgnoreRule mIgnoreRule = new DevSdkIgnoreRule(); + private static final String[] PROVISIONING_APP_NAME = {"some", "app"}; private static final String PROVISIONING_NO_UI_APP_NAME = "no_ui_app"; private static final String PROVISIONING_APP_RESPONSE = "app_response"; + private static final String TEST_PACKAGE_NAME = "com.android.tethering.test"; + private static final long TEST_PACKAGE_VERSION = 1234L; @Mock private Context mContext; @Mock private TelephonyManager mTelephonyManager; @Mock private Resources mResources; @Mock private Resources mResourcesForSubId; + @Mock private PackageManager mPackageManager; private Context mMockContext; private boolean mHasTelephonyManager; private boolean mEnableLegacyDhcpServer; @@ -100,6 +114,16 @@ public class TetheringConfigurationTest { } return super.getSystemService(name); } + + @Override + public PackageManager getPackageManager() { + return mPackageManager; + } + + @Override + public String getPackageName() { + return TEST_PACKAGE_NAME; + } } @Before @@ -110,9 +134,15 @@ public class TetheringConfigurationTest { .mockStatic(DeviceConfig.class) .strictness(Strictness.WARN) .startMocking(); + DeviceConfigUtils.resetPackageVersionCacheForTest(); doReturn(null).when( () -> DeviceConfig.getProperty(eq(NAMESPACE_CONNECTIVITY), eq(TetheringConfiguration.TETHER_ENABLE_LEGACY_DHCP_SERVER))); + setTetherForceUpstreamAutomaticFlagVersion(null); + + final PackageInfo pi = new PackageInfo(); + pi.setLongVersionCode(TEST_PACKAGE_VERSION); + doReturn(pi).when(mPackageManager).getPackageInfo(eq(TEST_PACKAGE_NAME), anyInt()); when(mResources.getStringArray(R.array.config_tether_dhcp_range)).thenReturn( new String[0]); @@ -141,6 +171,7 @@ public class TetheringConfigurationTest { @After public void tearDown() throws Exception { mMockingSession.finishMocking(); + DeviceConfigUtils.resetPackageVersionCacheForTest(); } private TetheringConfiguration getTetheringConfiguration(int... legacyTetherUpstreamTypes) { @@ -455,4 +486,52 @@ public class TetheringConfigurationTest { mMockContext, mLog, INVALID_SUBSCRIPTION_ID); assertTrue(testEnable.isSelectAllPrefixRangeEnabled()); } + + @Test + public void testChooseUpstreamAutomatically() throws Exception { + when(mResources.getBoolean(R.bool.config_tether_upstream_automatic)) + .thenReturn(true); + assertChooseUpstreamAutomaticallyIs(true); + + when(mResources.getBoolean(R.bool.config_tether_upstream_automatic)) + .thenReturn(false); + assertChooseUpstreamAutomaticallyIs(false); + } + + // The flag override only works on R- + @Test @IgnoreAfter(Build.VERSION_CODES.R) + public void testChooseUpstreamAutomatically_FlagOverride() throws Exception { + when(mResources.getBoolean(R.bool.config_tether_upstream_automatic)) + .thenReturn(false); + setTetherForceUpstreamAutomaticFlagVersion(TEST_PACKAGE_VERSION - 1); + assertTrue(DeviceConfigUtils.isFeatureEnabled(mMockContext, NAMESPACE_CONNECTIVITY, + TetheringConfiguration.TETHER_FORCE_UPSTREAM_AUTOMATIC_VERSION)); + + assertChooseUpstreamAutomaticallyIs(true); + + setTetherForceUpstreamAutomaticFlagVersion(0L); + assertChooseUpstreamAutomaticallyIs(false); + + setTetherForceUpstreamAutomaticFlagVersion(Long.MAX_VALUE); + assertChooseUpstreamAutomaticallyIs(false); + } + + @Test @IgnoreUpTo(Build.VERSION_CODES.R) + public void testChooseUpstreamAutomatically_FlagOverrideAfterR() throws Exception { + when(mResources.getBoolean(R.bool.config_tether_upstream_automatic)) + .thenReturn(false); + setTetherForceUpstreamAutomaticFlagVersion(TEST_PACKAGE_VERSION - 1); + assertChooseUpstreamAutomaticallyIs(false); + } + + private void setTetherForceUpstreamAutomaticFlagVersion(Long version) { + doReturn(version == null ? null : Long.toString(version)).when( + () -> DeviceConfig.getProperty(eq(NAMESPACE_CONNECTIVITY), + eq(TetheringConfiguration.TETHER_FORCE_UPSTREAM_AUTOMATIC_VERSION))); + } + + private void assertChooseUpstreamAutomaticallyIs(boolean value) { + assertEquals(value, new TetheringConfiguration(mMockContext, mLog, INVALID_SUBSCRIPTION_ID) + .chooseUpstreamAutomatically); + } } diff --git a/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringTest.java b/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringTest.java index f4b3749132..4b6945679c 100644 --- a/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringTest.java +++ b/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringTest.java @@ -357,6 +357,11 @@ public class TetheringTest { return null; } + @Override + protected boolean isFeatureEnabled(Context ctx, String featureVersionFlag) { + return false; + } + @Override protected Resources getResourcesForSubIdWrapper(Context ctx, int subId) { return mResources;