Remove accessors for legacy int score from NAI

These are no longer useful. Also, NAI no longer needs to
be comparable, and it should no longer be – comparing 2
NAIs only make sense in the context of a specific request.

Bug: 238139913
Test: ConnectivityServiceTest IpConnectivityMetricsTest
Change-Id: I6967ad6f08380213c29f3d47d6422f8bbea1946c
This commit is contained in:
Chalard Jean
2022-02-26 21:34:48 +09:00
committed by Paul Hu
parent 671a439f5a
commit aa5bc624f6
5 changed files with 63 additions and 47 deletions

View File

@@ -3609,7 +3609,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
final NetworkCapabilities sanitized = final NetworkCapabilities sanitized =
nai.getDeclaredCapabilitiesSanitized(mCarrierPrivilegeAuthenticator); nai.getDeclaredCapabilitiesSanitized(mCarrierPrivilegeAuthenticator);
maybeUpdateWifiRoamTimestamp(nai, sanitized); maybeUpdateWifiRoamTimestamp(nai, sanitized);
updateCapabilities(nai.getCurrentScore(), nai, sanitized); updateCapabilities(nai.getScore(), nai, sanitized);
break; break;
} }
case NetworkAgent.EVENT_NETWORK_PROPERTIES_CHANGED: { case NetworkAgent.EVENT_NETWORK_PROPERTIES_CHANGED: {
@@ -3877,7 +3877,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
log(nai.toShortString() + " validation " + (valid ? "passed" : "failed") + logMsg); log(nai.toShortString() + " validation " + (valid ? "passed" : "failed") + logMsg);
} }
if (valid != nai.lastValidated) { if (valid != nai.lastValidated) {
final int oldScore = nai.getCurrentScore(); final FullScore oldScore = nai.getScore();
nai.lastValidated = valid; nai.lastValidated = valid;
nai.everValidated |= valid; nai.everValidated |= valid;
updateCapabilities(oldScore, nai, nai.networkCapabilities); updateCapabilities(oldScore, nai, nai.networkCapabilities);
@@ -7262,8 +7262,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
* later : see {@link #updateLinkProperties}. * later : see {@link #updateLinkProperties}.
* @param networkCapabilities the initial capabilites of this network. They can be updated * @param networkCapabilities the initial capabilites of this network. They can be updated
* later : see {@link #updateCapabilities}. * later : see {@link #updateCapabilities}.
* @param initialScore the initial score of the network. See * @param initialScore the initial score of the network. See {@link NetworkAgentInfo#getScore}.
* {@link NetworkAgentInfo#getCurrentScore}.
* @param networkAgentConfig metadata about the network. This is never updated. * @param networkAgentConfig metadata about the network. This is never updated.
* @param providerId the ID of the provider owning this NetworkAgent. * @param providerId the ID of the provider owning this NetworkAgent.
* @return the network created for this agent. * @return the network created for this agent.
@@ -7962,7 +7961,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
* @param nai the network having its capabilities updated. * @param nai the network having its capabilities updated.
* @param nc the new network capabilities. * @param nc the new network capabilities.
*/ */
private void updateCapabilities(final int oldScore, @NonNull final NetworkAgentInfo nai, private void updateCapabilities(final FullScore oldScore, @NonNull final NetworkAgentInfo nai,
@NonNull final NetworkCapabilities nc) { @NonNull final NetworkCapabilities nc) {
NetworkCapabilities newNc = mixInCapabilities(nai, nc); NetworkCapabilities newNc = mixInCapabilities(nai, nc);
if (Objects.equals(nai.networkCapabilities, newNc)) return; if (Objects.equals(nai.networkCapabilities, newNc)) return;
@@ -7973,7 +7972,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
updateAllowedUids(nai, prevNc, newNc); updateAllowedUids(nai, prevNc, newNc);
nai.updateScoreForNetworkAgentUpdate(); nai.updateScoreForNetworkAgentUpdate();
if (nai.getCurrentScore() == oldScore && newNc.equalRequestableCapabilities(prevNc)) { if (nai.getScore().equals(oldScore) && newNc.equalRequestableCapabilities(prevNc)) {
// If the requestable capabilities haven't changed, and the score hasn't changed, then // If the requestable capabilities haven't changed, and the score hasn't changed, then
// the change we're processing can't affect any requests, it can only affect the listens // the change we're processing can't affect any requests, it can only affect the listens
// on this network. We might have been called by rematchNetworkAndRequests when a // on this network. We might have been called by rematchNetworkAndRequests when a
@@ -8017,7 +8016,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
/** Convenience method to update the capabilities for a given network. */ /** Convenience method to update the capabilities for a given network. */
private void updateCapabilitiesForNetwork(NetworkAgentInfo nai) { private void updateCapabilitiesForNetwork(NetworkAgentInfo nai) {
updateCapabilities(nai.getCurrentScore(), nai, nai.networkCapabilities); updateCapabilities(nai.getScore(), nai, nai.networkCapabilities);
} }
/** /**

View File

@@ -350,6 +350,26 @@ public class FullScore {
return mKeepConnectedReason; return mKeepConnectedReason;
} }
@Override
public boolean equals(final Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
final FullScore fullScore = (FullScore) o;
if (mLegacyInt != fullScore.mLegacyInt) return false;
if (mPolicies != fullScore.mPolicies) return false;
return mKeepConnectedReason == fullScore.mKeepConnectedReason;
}
@Override
public int hashCode() {
return 2 * ((int) mPolicies)
+ 3 * (int) (mPolicies >>> 32)
+ 5 * mKeepConnectedReason
+ 7 * mLegacyInt;
}
// Example output : // Example output :
// Score(50 ; Policies : EVER_USER_SELECTED&IS_VALIDATED) // Score(50 ; Policies : EVER_USER_SELECTED&IS_VALIDATED)
@Override @Override

View File

@@ -159,7 +159,7 @@ import java.util.TreeSet;
// the network is no longer considered "lingering". After the linger timer expires, if the network // the network is no longer considered "lingering". After the linger timer expires, if the network
// is satisfying one or more background NetworkRequests it is kept up in the background. If it is // is satisfying one or more background NetworkRequests it is kept up in the background. If it is
// not, ConnectivityService disconnects the NetworkAgent's AsyncChannel. // not, ConnectivityService disconnects the NetworkAgent's AsyncChannel.
public class NetworkAgentInfo implements Comparable<NetworkAgentInfo>, NetworkRanker.Scoreable { public class NetworkAgentInfo implements NetworkRanker.Scoreable {
@NonNull public NetworkInfo networkInfo; @NonNull public NetworkInfo networkInfo;
// This Network object should always be used if possible, so as to encourage reuse of the // This Network object should always be used if possible, so as to encourage reuse of the
@@ -1018,18 +1018,6 @@ public class NetworkAgentInfo implements Comparable<NetworkAgentInfo>, NetworkRa
return mScore; return mScore;
} }
// Get the current score for this Network. This may be modified from what the
// NetworkAgent sent, as it has modifiers applied to it.
public int getCurrentScore() {
return mScore.getLegacyInt();
}
// Get the current score for this Network as if it was validated. This may be modified from
// what the NetworkAgent sent, as it has modifiers applied to it.
public int getCurrentScoreAsValidated() {
return mScore.getLegacyIntAsValidated();
}
/** /**
* Mix-in the ConnectivityService-managed bits in the score. * Mix-in the ConnectivityService-managed bits in the score.
*/ */
@@ -1368,12 +1356,6 @@ public class NetworkAgentInfo implements Comparable<NetworkAgentInfo>, NetworkRa
+ transportNamesOf(networkCapabilities.getTransportTypes()) + "]"; + transportNamesOf(networkCapabilities.getTransportTypes()) + "]";
} }
// Enables sorting in descending order of score.
@Override
public int compareTo(NetworkAgentInfo other) {
return other.getCurrentScore() - getCurrentScore();
}
/** /**
* Null-guarding version of NetworkAgentInfo#toShortString() * Null-guarding version of NetworkAgentInfo#toShortString()
*/ */

View File

@@ -18,6 +18,8 @@ package com.android.server.connectivity
import android.net.NetworkAgentConfig import android.net.NetworkAgentConfig
import android.net.NetworkCapabilities import android.net.NetworkCapabilities
import android.net.NetworkScore
import android.net.NetworkScore.KEEP_CONNECTED_FOR_HANDOVER
import android.net.NetworkScore.KEEP_CONNECTED_NONE import android.net.NetworkScore.KEEP_CONNECTED_NONE
import android.os.Build import android.os.Build
import android.text.TextUtils import android.text.TextUtils
@@ -40,6 +42,7 @@ import org.junit.runner.RunWith
import kotlin.reflect.full.staticProperties import kotlin.reflect.full.staticProperties
import kotlin.test.assertEquals import kotlin.test.assertEquals
import kotlin.test.assertFalse import kotlin.test.assertFalse
import kotlin.test.assertNotEquals
import kotlin.test.assertTrue import kotlin.test.assertTrue
@RunWith(DevSdkIgnoreRunner::class) @RunWith(DevSdkIgnoreRunner::class)
@@ -156,4 +159,19 @@ class FullScoreTest {
assertEquals(FullScore.MAX_CS_MANAGED_POLICY, assertEquals(FullScore.MAX_CS_MANAGED_POLICY,
policies.maxOfOrNull { it.get() as Int }) policies.maxOfOrNull { it.get() as Int })
} }
@Test
fun testEquals() {
val ns1 = FullScore(50 /* legacyInt */, 0L /* policy */, KEEP_CONNECTED_NONE)
val ns2 = FullScore(50 /* legacyInt */, 0L /* policy */, KEEP_CONNECTED_NONE)
val ns3 = FullScore(60 /* legacyInt */, 0L /* policy */, KEEP_CONNECTED_NONE)
val ns4 = FullScore(50 /* legacyInt */, 0L /* policy */, KEEP_CONNECTED_FOR_HANDOVER)
val ns5 = NetworkScore.Builder().setLegacyInt(50).build()
assertEquals(ns1, ns1)
assertEquals(ns2, ns1)
assertNotEquals(ns1.withPolicies(validated = true), ns1)
assertNotEquals(ns3, ns1)
assertNotEquals(ns4, ns1)
assertFalse(ns1.equals(ns5))
}
} }

View File

@@ -138,18 +138,16 @@ public class IpConnectivityMetricsTest {
private void logDefaultNetworkEvent(long timeMs, NetworkAgentInfo nai, private void logDefaultNetworkEvent(long timeMs, NetworkAgentInfo nai,
NetworkAgentInfo oldNai) { NetworkAgentInfo oldNai) {
final Network network = (nai != null) ? nai.network() : null; final Network network = (nai != null) ? nai.network() : null;
final int score = (nai != null) ? nai.getCurrentScore() : 0;
final boolean validated = (nai != null) ? nai.lastValidated : false; final boolean validated = (nai != null) ? nai.lastValidated : false;
final LinkProperties lp = (nai != null) ? nai.linkProperties : null; final LinkProperties lp = (nai != null) ? nai.linkProperties : null;
final NetworkCapabilities nc = (nai != null) ? nai.networkCapabilities : null; final NetworkCapabilities nc = (nai != null) ? nai.networkCapabilities : null;
final Network prevNetwork = (oldNai != null) ? oldNai.network() : null; final Network prevNetwork = (oldNai != null) ? oldNai.network() : null;
final int prevScore = (oldNai != null) ? oldNai.getCurrentScore() : 0;
final LinkProperties prevLp = (oldNai != null) ? oldNai.linkProperties : null; final LinkProperties prevLp = (oldNai != null) ? oldNai.linkProperties : null;
final NetworkCapabilities prevNc = (oldNai != null) ? oldNai.networkCapabilities : null; final NetworkCapabilities prevNc = (oldNai != null) ? oldNai.networkCapabilities : null;
mService.mDefaultNetworkMetrics.logDefaultNetworkEvent(timeMs, network, score, validated, mService.mDefaultNetworkMetrics.logDefaultNetworkEvent(timeMs, network, 0 /* legacyScore */,
lp, nc, prevNetwork, prevScore, prevLp, prevNc); validated, lp, nc, prevNetwork, 0 /* prevLegacyScore */, prevLp, prevNc);
} }
@Test @Test
public void testDefaultNetworkEvents() throws Exception { public void testDefaultNetworkEvents() throws Exception {
@@ -158,15 +156,15 @@ public class IpConnectivityMetricsTest {
NetworkAgentInfo[][] defaultNetworks = { NetworkAgentInfo[][] defaultNetworks = {
// nothing -> cell // nothing -> cell
{null, makeNai(100, 10, false, true, cell)}, {null, makeNai(100, false, true, cell)},
// cell -> wifi // cell -> wifi
{makeNai(100, 50, true, true, cell), makeNai(101, 20, true, false, wifi)}, {makeNai(100, true, true, cell), makeNai(101, true, false, wifi)},
// wifi -> nothing // wifi -> nothing
{makeNai(101, 60, true, false, wifi), null}, {makeNai(101, true, false, wifi), null},
// nothing -> cell // nothing -> cell
{null, makeNai(102, 10, true, true, cell)}, {null, makeNai(102, true, true, cell)},
// cell -> wifi // cell -> wifi
{makeNai(102, 50, true, true, cell), makeNai(103, 20, true, false, wifi)}, {makeNai(102, true, true, cell), makeNai(103, true, false, wifi)},
}; };
long timeMs = mService.mDefaultNetworkMetrics.creationTimeMs; long timeMs = mService.mDefaultNetworkMetrics.creationTimeMs;
@@ -204,8 +202,8 @@ public class IpConnectivityMetricsTest {
" transports: 1", " transports: 1",
" default_network_event <", " default_network_event <",
" default_network_duration_ms: 2002", " default_network_duration_ms: 2002",
" final_score: 50", " final_score: 0",
" initial_score: 10", " initial_score: 0",
" ip_support: 3", " ip_support: 3",
" no_default_network_duration_ms: 0", " no_default_network_duration_ms: 0",
" previous_default_network_link_layer: 0", " previous_default_network_link_layer: 0",
@@ -221,8 +219,8 @@ public class IpConnectivityMetricsTest {
" transports: 2", " transports: 2",
" default_network_event <", " default_network_event <",
" default_network_duration_ms: 4004", " default_network_duration_ms: 4004",
" final_score: 60", " final_score: 0",
" initial_score: 20", " initial_score: 0",
" ip_support: 1", " ip_support: 1",
" no_default_network_duration_ms: 0", " no_default_network_duration_ms: 0",
" previous_default_network_link_layer: 2", " previous_default_network_link_layer: 2",
@@ -255,8 +253,8 @@ public class IpConnectivityMetricsTest {
" transports: 1", " transports: 1",
" default_network_event <", " default_network_event <",
" default_network_duration_ms: 16016", " default_network_duration_ms: 16016",
" final_score: 50", " final_score: 0",
" initial_score: 10", " initial_score: 0",
" ip_support: 3", " ip_support: 3",
" no_default_network_duration_ms: 0", " no_default_network_duration_ms: 0",
" previous_default_network_link_layer: 4", " previous_default_network_link_layer: 4",
@@ -348,8 +346,8 @@ public class IpConnectivityMetricsTest {
long timeMs = mService.mDefaultNetworkMetrics.creationTimeMs; long timeMs = mService.mDefaultNetworkMetrics.creationTimeMs;
final long cell = BitUtils.packBits(new int[]{NetworkCapabilities.TRANSPORT_CELLULAR}); final long cell = BitUtils.packBits(new int[]{NetworkCapabilities.TRANSPORT_CELLULAR});
final long wifi = BitUtils.packBits(new int[]{NetworkCapabilities.TRANSPORT_WIFI}); final long wifi = BitUtils.packBits(new int[]{NetworkCapabilities.TRANSPORT_WIFI});
NetworkAgentInfo cellNai = makeNai(100, 50, false, true, cell); final NetworkAgentInfo cellNai = makeNai(100, false, true, cell);
NetworkAgentInfo wifiNai = makeNai(101, 60, true, false, wifi); final NetworkAgentInfo wifiNai = makeNai(101, true, false, wifi);
logDefaultNetworkEvent(timeMs + 200L, cellNai, null); logDefaultNetworkEvent(timeMs + 200L, cellNai, null);
logDefaultNetworkEvent(timeMs + 300L, wifiNai, cellNai); logDefaultNetworkEvent(timeMs + 300L, wifiNai, cellNai);
@@ -463,8 +461,8 @@ public class IpConnectivityMetricsTest {
" transports: 1", " transports: 1",
" default_network_event <", " default_network_event <",
" default_network_duration_ms: 100", " default_network_duration_ms: 100",
" final_score: 50", " final_score: 0",
" initial_score: 50", " initial_score: 0",
" ip_support: 2", " ip_support: 2",
" no_default_network_duration_ms: 0", " no_default_network_duration_ms: 0",
" previous_default_network_link_layer: 0", " previous_default_network_link_layer: 0",
@@ -611,10 +609,9 @@ public class IpConnectivityMetricsTest {
mNetdListener.onWakeupEvent(prefix, uid, ether, ip, mac, srcIp, dstIp, sport, dport, now); mNetdListener.onWakeupEvent(prefix, uid, ether, ip, mac, srcIp, dstIp, sport, dport, now);
} }
NetworkAgentInfo makeNai(int netId, int score, boolean ipv4, boolean ipv6, long transports) { NetworkAgentInfo makeNai(int netId, boolean ipv4, boolean ipv6, long transports) {
NetworkAgentInfo nai = mock(NetworkAgentInfo.class); NetworkAgentInfo nai = mock(NetworkAgentInfo.class);
when(nai.network()).thenReturn(new Network(netId)); when(nai.network()).thenReturn(new Network(netId));
when(nai.getCurrentScore()).thenReturn(score);
nai.linkProperties = new LinkProperties(); nai.linkProperties = new LinkProperties();
nai.networkCapabilities = new NetworkCapabilities(); nai.networkCapabilities = new NetworkCapabilities();
nai.lastValidated = true; nai.lastValidated = true;