From 4854d055bb86881463abf59b9ec8febc632784ab Mon Sep 17 00:00:00 2001 From: Aaron Huang Date: Fri, 4 Nov 2022 23:14:26 +0800 Subject: [PATCH] Remove mSubscriberIdMatchRule from NetworkTemplate Currently, mSubscriberIdMatchRule is only used in NetworkTemplate and it depends on if mMatchSubscriberIds is empty or not. Thus, remove it since is not really necessary and replace it with checking matchSubscriberIds if needed. Bug: 238843364 Test: build, FrameworksNetTests CtsNetTestCases Change-Id: Ic66d2ff2826846778b004bb15a4718a62fa1f470 --- .../src/android/net/NetworkTemplate.java | 68 ++++++------------- .../net/netstats/NetworkTemplateTest.kt | 22 +++--- .../java/android/net/NetworkTemplateTest.kt | 27 +++----- 3 files changed, 42 insertions(+), 75 deletions(-) diff --git a/framework-t/src/android/net/NetworkTemplate.java b/framework-t/src/android/net/NetworkTemplate.java index c0ae8224b1..b2da3719c0 100644 --- a/framework-t/src/android/net/NetworkTemplate.java +++ b/framework-t/src/android/net/NetworkTemplate.java @@ -52,7 +52,6 @@ import android.util.ArraySet; import com.android.internal.annotations.VisibleForTesting; import com.android.net.module.util.CollectionUtils; import com.android.net.module.util.NetworkIdentityUtils; -import com.android.net.module.util.NetworkStatsUtils; import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; @@ -280,23 +279,21 @@ public final class NetworkTemplate implements Parcelable { private final int mRoaming; private final int mDefaultNetwork; private final int mRatType; - /** - * The subscriber Id match rule defines how the template should match networks with - * specific subscriberId(s). See NetworkTemplate#SUBSCRIBER_ID_MATCH_RULE_* for more detail. - */ - private final int mSubscriberIdMatchRule; // Bitfield containing OEM network properties{@code NetworkIdentity#OEM_*}. private final int mOemManaged; - private static void checkValidSubscriberIdMatchRule(int matchRule, int subscriberIdMatchRule) { + private static void checkValidMatchSubscriberIds(int matchRule, String[] matchSubscriberIds) { switch (matchRule) { case MATCH_MOBILE: case MATCH_CARRIER: // MOBILE and CARRIER templates must always specify a subscriber ID. - if (subscriberIdMatchRule == NetworkStatsUtils.SUBSCRIBER_ID_MATCH_RULE_ALL) { - throw new IllegalArgumentException("Invalid SubscriberIdMatchRule " - + "on match rule: " + getMatchRuleName(matchRule)); + if (matchSubscriberIds.length == 0) { + throw new IllegalArgumentException("checkValidMatchSubscriberIds with empty" + + " list of ids for rule" + getMatchRuleName(matchRule)); + } else if (CollectionUtils.contains(matchSubscriberIds, null)) { + throw new IllegalArgumentException("checkValidMatchSubscriberIds list of ids" + + " may not contain null for rule " + getMatchRuleName(matchRule)); } return; default: @@ -312,28 +309,21 @@ public final class NetworkTemplate implements Parcelable { @UnsupportedAppUsage(maxTargetSdk = Build.VERSION_CODES.TIRAMISU, publicAlternatives = "Use {@code Builder} instead.") public NetworkTemplate(int matchRule, String subscriberId, String wifiNetworkKey) { - this(matchRule, subscriberId, new String[] { subscriberId }, wifiNetworkKey); - } - - /** @hide */ - public NetworkTemplate(int matchRule, String subscriberId, String[] matchSubscriberIds, - String wifiNetworkKey) { // Older versions used to only match MATCH_MOBILE and MATCH_MOBILE_WILDCARD templates // to metered networks. It is now possible to match mobile with any meteredness, but // in order to preserve backward compatibility of @UnsupportedAppUsage methods, this //constructor passes METERED_YES for these types. - this(matchRule, subscriberId, matchSubscriberIds, + this(matchRule, subscriberId, new String[] { subscriberId }, wifiNetworkKey != null ? new String[] { wifiNetworkKey } : new String[0], (matchRule == MATCH_MOBILE || matchRule == MATCH_MOBILE_WILDCARD || matchRule == MATCH_CARRIER) ? METERED_YES : METERED_ALL, - ROAMING_ALL, DEFAULT_NETWORK_ALL, NETWORK_TYPE_ALL, - OEM_MANAGED_ALL, NetworkStatsUtils.SUBSCRIBER_ID_MATCH_RULE_EXACT); + ROAMING_ALL, DEFAULT_NETWORK_ALL, NETWORK_TYPE_ALL, OEM_MANAGED_ALL); } /** @hide */ public NetworkTemplate(int matchRule, String subscriberId, String[] matchSubscriberIds, String[] matchWifiNetworkKeys, int metered, int roaming, - int defaultNetwork, int ratType, int oemManaged, int subscriberIdMatchRule) { + int defaultNetwork, int ratType, int oemManaged) { Objects.requireNonNull(matchWifiNetworkKeys); Objects.requireNonNull(matchSubscriberIds); mMatchRule = matchRule; @@ -345,8 +335,7 @@ public final class NetworkTemplate implements Parcelable { mDefaultNetwork = defaultNetwork; mRatType = ratType; mOemManaged = oemManaged; - mSubscriberIdMatchRule = subscriberIdMatchRule; - checkValidSubscriberIdMatchRule(matchRule, subscriberIdMatchRule); + checkValidMatchSubscriberIds(matchRule, matchSubscriberIds); if (!isKnownMatchRule(matchRule)) { throw new IllegalArgumentException("Unknown network template rule " + matchRule + " will not match any identity."); @@ -363,7 +352,6 @@ public final class NetworkTemplate implements Parcelable { mDefaultNetwork = in.readInt(); mRatType = in.readInt(); mOemManaged = in.readInt(); - mSubscriberIdMatchRule = in.readInt(); } @Override @@ -377,7 +365,6 @@ public final class NetworkTemplate implements Parcelable { dest.writeInt(mDefaultNetwork); dest.writeInt(mRatType); dest.writeInt(mOemManaged); - dest.writeInt(mSubscriberIdMatchRule); } @Override @@ -414,15 +401,13 @@ public final class NetworkTemplate implements Parcelable { if (mOemManaged != OEM_MANAGED_ALL) { builder.append(", oemManaged=").append(getOemManagedNames(mOemManaged)); } - builder.append(", subscriberIdMatchRule=") - .append(subscriberIdMatchRuleToString(mSubscriberIdMatchRule)); return builder.toString(); } @Override public int hashCode() { return Objects.hash(mMatchRule, mSubscriberId, Arrays.hashCode(mMatchWifiNetworkKeys), - mMetered, mRoaming, mDefaultNetwork, mRatType, mOemManaged, mSubscriberIdMatchRule); + mMetered, mRoaming, mDefaultNetwork, mRatType, mOemManaged); } @Override @@ -436,23 +421,11 @@ public final class NetworkTemplate implements Parcelable { && mDefaultNetwork == other.mDefaultNetwork && mRatType == other.mRatType && mOemManaged == other.mOemManaged - && mSubscriberIdMatchRule == other.mSubscriberIdMatchRule && Arrays.equals(mMatchWifiNetworkKeys, other.mMatchWifiNetworkKeys); } return false; } - private static String subscriberIdMatchRuleToString(int rule) { - switch (rule) { - case NetworkStatsUtils.SUBSCRIBER_ID_MATCH_RULE_EXACT: - return "EXACT_MATCH"; - case NetworkStatsUtils.SUBSCRIBER_ID_MATCH_RULE_ALL: - return "ALL"; - default: - return "Unknown rule " + rule; - } - } - /** @hide */ public boolean isMatchRuleMobile() { switch (mMatchRule) { @@ -627,13 +600,13 @@ public final class NetworkTemplate implements Parcelable { /** * Check if this template matches {@code subscriberId}. Returns true if this - * template was created with {@code SUBSCRIBER_ID_MATCH_RULE_ALL}, or with a - * {@code mMatchSubscriberIds} array that contains {@code subscriberId}. + * template was created with a {@code mMatchSubscriberIds} array that contains + * {@code subscriberId} or if {@code mMatchSubscriberIds} is empty. * * @hide */ public boolean matchesSubscriberId(@Nullable String subscriberId) { - return mSubscriberIdMatchRule == NetworkStatsUtils.SUBSCRIBER_ID_MATCH_RULE_ALL + return mMatchSubscriberIds.length == 0 || CollectionUtils.contains(mMatchSubscriberIds, subscriberId); } @@ -812,7 +785,11 @@ public final class NetworkTemplate implements Parcelable { // could handle incompatible subscriberIds. See b/217805241. return new NetworkTemplate(template.mMatchRule, merged[0], merged, CollectionUtils.isEmpty(matchWifiNetworkKeys) - ? null : matchWifiNetworkKeys[0]); + ? new String[0] : new String[] { matchWifiNetworkKeys[0] }, + (template.mMatchRule == MATCH_MOBILE + || template.mMatchRule == MATCH_MOBILE_WILDCARD + || template.mMatchRule == MATCH_CARRIER) ? METERED_YES : METERED_ALL, + ROAMING_ALL, DEFAULT_NETWORK_ALL, NETWORK_TYPE_ALL, OEM_MANAGED_ALL); } return template; @@ -1024,14 +1001,11 @@ public final class NetworkTemplate implements Parcelable { @NonNull public NetworkTemplate build() { assertRequestableParameters(); - final int subscriberIdMatchRule = mMatchSubscriberIds.isEmpty() - ? NetworkStatsUtils.SUBSCRIBER_ID_MATCH_RULE_ALL - : NetworkStatsUtils.SUBSCRIBER_ID_MATCH_RULE_EXACT; return new NetworkTemplate(getWildcardDeducedMatchRule(), mMatchSubscriberIds.isEmpty() ? null : mMatchSubscriberIds.iterator().next(), mMatchSubscriberIds.toArray(new String[0]), mMatchWifiNetworkKeys.toArray(new String[0]), mMetered, mRoaming, - mDefaultNetwork, mRatType, mOemManaged, subscriberIdMatchRule); + mDefaultNetwork, mRatType, mOemManaged); } } } diff --git a/tests/common/java/android/net/netstats/NetworkTemplateTest.kt b/tests/common/java/android/net/netstats/NetworkTemplateTest.kt index cdf32a406c..3c2340cd98 100644 --- a/tests/common/java/android/net/netstats/NetworkTemplateTest.kt +++ b/tests/common/java/android/net/netstats/NetworkTemplateTest.kt @@ -19,8 +19,8 @@ package android.net.netstats import android.net.NetworkStats.DEFAULT_NETWORK_ALL import android.net.NetworkStats.METERED_ALL import android.net.NetworkStats.METERED_YES -import android.net.NetworkStats.ROAMING_YES import android.net.NetworkStats.ROAMING_ALL +import android.net.NetworkStats.ROAMING_YES import android.net.NetworkTemplate import android.net.NetworkTemplate.MATCH_BLUETOOTH import android.net.NetworkTemplate.MATCH_CARRIER @@ -33,8 +33,6 @@ import android.net.NetworkTemplate.MATCH_WIFI_WILDCARD import android.net.NetworkTemplate.NETWORK_TYPE_ALL import android.net.NetworkTemplate.OEM_MANAGED_ALL import android.telephony.TelephonyManager -import com.android.net.module.util.NetworkStatsUtils.SUBSCRIBER_ID_MATCH_RULE_ALL -import com.android.net.module.util.NetworkStatsUtils.SUBSCRIBER_ID_MATCH_RULE_EXACT import com.android.testutils.ConnectivityModuleTest import com.android.testutils.DevSdkIgnoreRule import com.android.testutils.SC_V2 @@ -80,7 +78,7 @@ class NetworkTemplateTest { val expectedTemplate = NetworkTemplate(matchRule, TEST_IMSI1, arrayOf(TEST_IMSI1), emptyArray(), METERED_YES, ROAMING_ALL, DEFAULT_NETWORK_ALL, NETWORK_TYPE_ALL, - OEM_MANAGED_ALL, SUBSCRIBER_ID_MATCH_RULE_EXACT) + OEM_MANAGED_ALL) assertEquals(expectedTemplate, it) } } @@ -93,7 +91,7 @@ class NetworkTemplateTest { val expectedTemplate = NetworkTemplate(matchRule, TEST_IMSI1, arrayOf(TEST_IMSI1), emptyArray(), METERED_YES, ROAMING_YES, DEFAULT_NETWORK_ALL, NETWORK_TYPE_ALL, - OEM_MANAGED_ALL, SUBSCRIBER_ID_MATCH_RULE_EXACT) + OEM_MANAGED_ALL) assertEquals(expectedTemplate, it) } } @@ -109,7 +107,7 @@ class NetworkTemplateTest { val expectedTemplate = NetworkTemplate(MATCH_MOBILE_WILDCARD, null /*subscriberId*/, emptyArray() /*subscriberIds*/, emptyArray(), METERED_YES, ROAMING_ALL, DEFAULT_NETWORK_ALL, NETWORK_TYPE_ALL, - OEM_MANAGED_ALL, SUBSCRIBER_ID_MATCH_RULE_ALL) + OEM_MANAGED_ALL) assertEquals(expectedTemplate, it) } @@ -121,7 +119,7 @@ class NetworkTemplateTest { val expectedTemplate = NetworkTemplate(MATCH_MOBILE, TEST_IMSI1, arrayOf(TEST_IMSI1), emptyArray(), METERED_YES, ROAMING_ALL, DEFAULT_NETWORK_ALL, TelephonyManager.NETWORK_TYPE_UMTS, - OEM_MANAGED_ALL, SUBSCRIBER_ID_MATCH_RULE_EXACT) + OEM_MANAGED_ALL) assertEquals(expectedTemplate, it) } @@ -131,7 +129,7 @@ class NetworkTemplateTest { val expectedTemplate = NetworkTemplate(MATCH_WIFI_WILDCARD, null /*subscriberId*/, emptyArray() /*subscriberIds*/, emptyArray(), METERED_ALL, ROAMING_ALL, DEFAULT_NETWORK_ALL, NETWORK_TYPE_ALL, - OEM_MANAGED_ALL, SUBSCRIBER_ID_MATCH_RULE_ALL) + OEM_MANAGED_ALL) assertEquals(expectedTemplate, it) } @@ -141,7 +139,7 @@ class NetworkTemplateTest { val expectedTemplate = NetworkTemplate(MATCH_WIFI, null /*subscriberId*/, emptyArray() /*subscriberIds*/, arrayOf(TEST_WIFI_KEY1), METERED_ALL, ROAMING_ALL, DEFAULT_NETWORK_ALL, NETWORK_TYPE_ALL, - OEM_MANAGED_ALL, SUBSCRIBER_ID_MATCH_RULE_ALL) + OEM_MANAGED_ALL) assertEquals(expectedTemplate, it) } @@ -152,7 +150,7 @@ class NetworkTemplateTest { val expectedTemplate = NetworkTemplate(MATCH_WIFI, TEST_IMSI1, arrayOf(TEST_IMSI1), arrayOf(TEST_WIFI_KEY1), METERED_ALL, ROAMING_ALL, DEFAULT_NETWORK_ALL, NETWORK_TYPE_ALL, - OEM_MANAGED_ALL, SUBSCRIBER_ID_MATCH_RULE_EXACT) + OEM_MANAGED_ALL) assertEquals(expectedTemplate, it) } @@ -163,7 +161,7 @@ class NetworkTemplateTest { val expectedTemplate = NetworkTemplate(matchRule, null /*subscriberId*/, emptyArray() /*subscriberIds*/, emptyArray(), METERED_ALL, ROAMING_ALL, DEFAULT_NETWORK_ALL, NETWORK_TYPE_ALL, - OEM_MANAGED_ALL, SUBSCRIBER_ID_MATCH_RULE_ALL) + OEM_MANAGED_ALL) assertEquals(expectedTemplate, it) } } @@ -198,7 +196,7 @@ class NetworkTemplateTest { val expectedTemplate = NetworkTemplate(MATCH_WIFI_WILDCARD, null /*subscriberId*/, emptyArray() /*subscriberIds*/, emptyArray(), METERED_ALL, ROAMING_ALL, DEFAULT_NETWORK_ALL, NETWORK_TYPE_ALL, - OEM_MANAGED_ALL, SUBSCRIBER_ID_MATCH_RULE_ALL) + OEM_MANAGED_ALL) assertEquals(expectedTemplate, it) } } diff --git a/tests/unit/java/android/net/NetworkTemplateTest.kt b/tests/unit/java/android/net/NetworkTemplateTest.kt index 3cf022812e..c3440c5c15 100644 --- a/tests/unit/java/android/net/NetworkTemplateTest.kt +++ b/tests/unit/java/android/net/NetworkTemplateTest.kt @@ -49,7 +49,6 @@ import android.net.NetworkTemplate.normalize import android.net.wifi.WifiInfo import android.os.Build import android.telephony.TelephonyManager -import com.android.net.module.util.NetworkStatsUtils.SUBSCRIBER_ID_MATCH_RULE_EXACT import com.android.testutils.DevSdkIgnoreRule import com.android.testutils.DevSdkIgnoreRunner import com.android.testutils.assertParcelSane @@ -448,19 +447,18 @@ class NetworkTemplateTest { @Test fun testParcelUnparcel() { - val templateMobile = NetworkTemplate(MATCH_MOBILE, TEST_IMSI1, emptyArray(), + val templateMobile = NetworkTemplate(MATCH_MOBILE, TEST_IMSI1, arrayOf(TEST_IMSI1), emptyArray(), METERED_ALL, ROAMING_ALL, DEFAULT_NETWORK_ALL, - TelephonyManager.NETWORK_TYPE_LTE, OEM_MANAGED_ALL, - SUBSCRIBER_ID_MATCH_RULE_EXACT) + TelephonyManager.NETWORK_TYPE_LTE, OEM_MANAGED_ALL) val templateWifi = NetworkTemplate(MATCH_WIFI, null, emptyArray(), arrayOf(TEST_WIFI_KEY1), METERED_ALL, ROAMING_ALL, DEFAULT_NETWORK_ALL, 0, - OEM_MANAGED_ALL, SUBSCRIBER_ID_MATCH_RULE_EXACT) - val templateOem = NetworkTemplate(MATCH_MOBILE, null, emptyArray(), + OEM_MANAGED_ALL) + val templateOem = NetworkTemplate(MATCH_MOBILE_WILDCARD, null, emptyArray(), emptyArray(), METERED_ALL, ROAMING_ALL, DEFAULT_NETWORK_ALL, 0, - OEM_MANAGED_YES, SUBSCRIBER_ID_MATCH_RULE_EXACT) - assertParcelSane(templateMobile, 10) - assertParcelSane(templateWifi, 10) - assertParcelSane(templateOem, 10) + OEM_MANAGED_YES) + assertParcelSane(templateMobile, 9) + assertParcelSane(templateWifi, 9) + assertParcelSane(templateOem, 9) } // Verify NETWORK_TYPE_* constants in NetworkTemplate do not conflict with @@ -514,12 +512,10 @@ class NetworkTemplateTest { val templateOemYes = NetworkTemplate(matchType, subscriberId, matchSubscriberIds, matchWifiNetworkKeys, METERED_ALL, ROAMING_ALL, - DEFAULT_NETWORK_ALL, NETWORK_TYPE_ALL, OEM_MANAGED_YES, - SUBSCRIBER_ID_MATCH_RULE_EXACT) + DEFAULT_NETWORK_ALL, NETWORK_TYPE_ALL, OEM_MANAGED_YES) val templateOemAll = NetworkTemplate(matchType, subscriberId, matchSubscriberIds, matchWifiNetworkKeys, METERED_ALL, ROAMING_ALL, - DEFAULT_NETWORK_ALL, NETWORK_TYPE_ALL, OEM_MANAGED_ALL, - SUBSCRIBER_ID_MATCH_RULE_EXACT) + DEFAULT_NETWORK_ALL, NETWORK_TYPE_ALL, OEM_MANAGED_ALL) for (identityOemManagedState in oemManagedStates) { val ident = buildNetworkIdentity(mockContext, buildNetworkState(networkType, @@ -530,8 +526,7 @@ class NetworkTemplateTest { for (templateOemManagedState in oemManagedStates) { val template = NetworkTemplate(matchType, subscriberId, matchSubscriberIds, matchWifiNetworkKeys, METERED_ALL, ROAMING_ALL, - DEFAULT_NETWORK_ALL, NETWORK_TYPE_ALL, templateOemManagedState, - SUBSCRIBER_ID_MATCH_RULE_EXACT) + DEFAULT_NETWORK_ALL, NETWORK_TYPE_ALL, templateOemManagedState) if (identityOemManagedState == templateOemManagedState) { template.assertMatches(ident) } else {