From fff0f8ba339a4079e223d2c5f16bbd21fd25ff02 Mon Sep 17 00:00:00 2001 From: Benedict Wong Date: Mon, 12 Apr 2021 23:18:27 -0700 Subject: [PATCH] Remove use of network scores, and provider IDs in VcnNetworkProvider This change updates the VcnNetworkProvider and associated requestListeners to no longer use the network score and provider IDs, in order to match the new NetworkProvider APIs. Bug: 185204197 Test: atest FrameworksVcnTests Change-Id: I0cabce702cfd8457fd1a152af07a847b87d67028 --- .../core/java/com/android/server/vcn/Vcn.java | 24 ++----- .../server/vcn/VcnNetworkProvider.java | 66 ++++--------------- .../server/vcn/VcnNetworkProviderTest.java | 41 ++++++++---- .../java/com/android/server/vcn/VcnTest.java | 4 +- 4 files changed, 47 insertions(+), 88 deletions(-) diff --git a/services/core/java/com/android/server/vcn/Vcn.java b/services/core/java/com/android/server/vcn/Vcn.java index 70a8a7ff7bb31..c7c538521de8e 100644 --- a/services/core/java/com/android/server/vcn/Vcn.java +++ b/services/core/java/com/android/server/vcn/Vcn.java @@ -265,10 +265,10 @@ public class Vcn extends Handler { private class VcnNetworkRequestListener implements VcnNetworkProvider.NetworkRequestListener { @Override - public void onNetworkRequested(@NonNull NetworkRequest request, int score, int providerId) { + public void onNetworkRequested(@NonNull NetworkRequest request) { Objects.requireNonNull(request, "Missing request"); - sendMessage(obtainMessage(MSG_EVENT_NETWORK_REQUESTED, score, providerId, request)); + sendMessage(obtainMessage(MSG_EVENT_NETWORK_REQUESTED, request)); } } @@ -284,7 +284,7 @@ public class Vcn extends Handler { handleConfigUpdated((VcnConfig) msg.obj); break; case MSG_EVENT_NETWORK_REQUESTED: - handleNetworkRequested((NetworkRequest) msg.obj, msg.arg1, msg.arg2); + handleNetworkRequested((NetworkRequest) msg.obj); break; case MSG_EVENT_SUBSCRIPTIONS_CHANGED: handleSubscriptionsChanged((TelephonySubscriptionSnapshot) msg.obj); @@ -365,25 +365,9 @@ public class Vcn extends Handler { } } - private void handleNetworkRequested( - @NonNull NetworkRequest request, int score, int providerId) { + private void handleNetworkRequested(@NonNull NetworkRequest request) { Slog.v(getLogTag(), "Received request " + request); - if (score > getNetworkScore()) { - if (VDBG) { - Slog.v( - getLogTag(), - "Request already satisfied by higher-scoring (" - + score - + ") network from " - + "provider " - + providerId - + ": " - + request); - } - return; - } - // If preexisting VcnGatewayConnection(s) satisfy request, return for (VcnGatewayConnectionConfig gatewayConnectionConfig : mVcnGatewayConnections.keySet()) { if (isRequestSatisfiedByGatewayConnectionConfig(request, gatewayConnectionConfig)) { diff --git a/services/core/java/com/android/server/vcn/VcnNetworkProvider.java b/services/core/java/com/android/server/vcn/VcnNetworkProvider.java index 4de0fdaba676b..8adcf95e780f5 100644 --- a/services/core/java/com/android/server/vcn/VcnNetworkProvider.java +++ b/services/core/java/com/android/server/vcn/VcnNetworkProvider.java @@ -23,7 +23,6 @@ import android.content.Context; import android.net.NetworkProvider; import android.net.NetworkRequest; import android.os.Looper; -import android.util.ArrayMap; import android.util.ArraySet; import android.util.Slog; @@ -31,7 +30,6 @@ import com.android.internal.annotations.VisibleForTesting; import com.android.internal.annotations.VisibleForTesting.Visibility; import com.android.internal.util.IndentingPrintWriter; -import java.util.Objects; import java.util.Set; /** @@ -48,11 +46,11 @@ public class VcnNetworkProvider extends NetworkProvider { private final Set mListeners = new ArraySet<>(); /** - * Cache of NetworkRequest(s), scores and network providers, keyed by NetworkRequest + * Cache of NetworkRequest(s). * *

NetworkRequests are immutable once created, and therefore can be used as stable keys. */ - private final ArrayMap mRequests = new ArrayMap<>(); + private final Set mRequests = new ArraySet<>(); public VcnNetworkProvider(Context context, Looper looper) { super(context, looper, VcnNetworkProvider.class.getSimpleName()); @@ -80,38 +78,28 @@ public class VcnNetworkProvider extends NetworkProvider { /** Sends all cached NetworkRequest(s) to the specified listener. */ @VisibleForTesting(visibility = Visibility.PACKAGE) public void resendAllRequests(@NonNull NetworkRequestListener listener) { - for (NetworkRequestEntry entry : mRequests.values()) { - notifyListenerForEvent(listener, entry); + for (NetworkRequest request : mRequests) { + notifyListenerForEvent(listener, request); } } private void notifyListenerForEvent( - @NonNull NetworkRequestListener listener, @NonNull NetworkRequestEntry entry) { - listener.onNetworkRequested(entry.mRequest, entry.mScore, entry.mProviderId); + @NonNull NetworkRequestListener listener, @NonNull NetworkRequest request) { + listener.onNetworkRequested(request); } @Override public void onNetworkRequested(@NonNull NetworkRequest request, int score, int providerId) { if (VDBG) { - Slog.v( - TAG, - "Network requested: Request = " - + request - + ", score = " - + score - + ", providerId = " - + providerId); + Slog.v(TAG, "Network requested: Request = " + request); } - final NetworkRequestEntry entry = new NetworkRequestEntry(request, score, providerId); - - // NetworkRequests are immutable once created, and therefore can be used as stable keys. - mRequests.put(request, entry); + mRequests.add(request); // TODO(b/176939047): Intelligently route requests to prioritized VcnInstances (based on // Default Data Sub, or similar) for (NetworkRequestListener listener : mListeners) { - notifyListenerForEvent(listener, entry); + notifyListenerForEvent(listener, request); } } @@ -120,37 +108,9 @@ public class VcnNetworkProvider extends NetworkProvider { mRequests.remove(request); } - private static class NetworkRequestEntry { - public final NetworkRequest mRequest; - public final int mScore; - public final int mProviderId; - - private NetworkRequestEntry(@NonNull NetworkRequest request, int score, int providerId) { - mRequest = Objects.requireNonNull(request, "Missing request"); - mScore = score; - mProviderId = providerId; - } - - /** - * Dumps the state of this NetworkRequestEntry for logging and debugging purposes. - * - *

PII and credentials MUST NEVER be dumped here. - */ - public void dump(IndentingPrintWriter pw) { - pw.println("NetworkRequestEntry:"); - pw.increaseIndent(); - - pw.println("mRequest: " + mRequest); - pw.println("mScore: " + mScore); - pw.println("mProviderId: " + mProviderId); - - pw.decreaseIndent(); - } - } - // package-private interface NetworkRequestListener { - void onNetworkRequested(@NonNull NetworkRequest request, int score, int providerId); + void onNetworkRequested(@NonNull NetworkRequest request); } /** @@ -170,10 +130,10 @@ public class VcnNetworkProvider extends NetworkProvider { pw.decreaseIndent(); pw.println(); - pw.println("mRequests.values:"); + pw.println("mRequests:"); pw.increaseIndent(); - for (NetworkRequestEntry entry : mRequests.values()) { - entry.dump(pw); + for (NetworkRequest request : mRequests) { + pw.println(request); } pw.decreaseIndent(); pw.println(); diff --git a/tests/vcn/java/com/android/server/vcn/VcnNetworkProviderTest.java b/tests/vcn/java/com/android/server/vcn/VcnNetworkProviderTest.java index f943f34c9d523..79cf746f105fa 100644 --- a/tests/vcn/java/com/android/server/vcn/VcnNetworkProviderTest.java +++ b/tests/vcn/java/com/android/server/vcn/VcnNetworkProviderTest.java @@ -34,12 +34,14 @@ import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import java.util.ArrayList; +import java.util.List; + /** Tests for TelephonySubscriptionTracker */ @RunWith(AndroidJUnit4.class) @SmallTest public class VcnNetworkProviderTest { private static final int TEST_SCORE_UNSATISFIED = 0; - private static final int TEST_SCORE_HIGH = 100; private static final int TEST_PROVIDER_ID = 1; @NonNull private final Context mContext; @@ -65,17 +67,7 @@ public class VcnNetworkProviderTest { final NetworkRequest request = mock(NetworkRequest.class); mVcnNetworkProvider.onNetworkRequested(request, TEST_SCORE_UNSATISFIED, TEST_PROVIDER_ID); - verify(mListener).onNetworkRequested(request, TEST_SCORE_UNSATISFIED, TEST_PROVIDER_ID); - } - - @Test - public void testRequestsPassedToRegisteredListeners_satisfiedByHighScoringProvider() - throws Exception { - mVcnNetworkProvider.registerListener(mListener); - - final NetworkRequest request = mock(NetworkRequest.class); - mVcnNetworkProvider.onNetworkRequested(request, TEST_SCORE_HIGH, TEST_PROVIDER_ID); - verify(mListener).onNetworkRequested(request, TEST_SCORE_HIGH, TEST_PROVIDER_ID); + verify(mListener).onNetworkRequested(request); } @Test @@ -87,4 +79,29 @@ public class VcnNetworkProviderTest { mVcnNetworkProvider.onNetworkRequested(request, TEST_SCORE_UNSATISFIED, TEST_PROVIDER_ID); verifyNoMoreInteractions(mListener); } + + @Test + public void testCachedRequestsPassedOnRegister() throws Exception { + final List requests = new ArrayList<>(); + + for (int i = 0; i < 10; i++) { + // Build unique network requests; in this case, iterate down the capabilities as a way + // to unique-ify requests. + final NetworkRequest request = + new NetworkRequest.Builder().clearCapabilities().addCapability(i).build(); + + requests.add(request); + mVcnNetworkProvider.onNetworkRequested(request, i, i + 1); + } + + // Remove one, and verify that it is never sent to the listeners. + final NetworkRequest removed = requests.remove(0); + mVcnNetworkProvider.onNetworkRequestWithdrawn(removed); + + mVcnNetworkProvider.registerListener(mListener); + for (NetworkRequest request : requests) { + verify(mListener).onNetworkRequested(request); + } + verifyNoMoreInteractions(mListener); + } } diff --git a/tests/vcn/java/com/android/server/vcn/VcnTest.java b/tests/vcn/java/com/android/server/vcn/VcnTest.java index 7c2a5a1cfc5d6..736fabdb1ac54 100644 --- a/tests/vcn/java/com/android/server/vcn/VcnTest.java +++ b/tests/vcn/java/com/android/server/vcn/VcnTest.java @@ -75,8 +75,6 @@ import java.util.UUID; public class VcnTest { private static final String PKG_NAME = VcnTest.class.getPackage().getName(); private static final ParcelUuid TEST_SUB_GROUP = new ParcelUuid(new UUID(0, 0)); - private static final int NETWORK_SCORE = 0; - private static final int PROVIDER_ID = 5; private static final boolean MOBILE_DATA_ENABLED = true; private static final Set TEST_SUB_IDS_IN_GROUP = new ArraySet<>(Arrays.asList(1, 2, 3)); @@ -177,7 +175,7 @@ public class VcnTest { requestBuilder.addCapability(netCapability); } - requestListener.onNetworkRequested(requestBuilder.build(), NETWORK_SCORE, PROVIDER_ID); + requestListener.onNetworkRequested(requestBuilder.build()); mTestLooper.dispatchAll(); }