From 54eae6c348040558a8bbf8c858d1962b95d5c848 Mon Sep 17 00:00:00 2001 From: Lorenzo Colitti Date: Mon, 1 Aug 2022 18:19:17 +0900 Subject: [PATCH] Allow tests to set the NetworkAgent's callbacks when creating it. ConnectivityServiceTest#testNetworkAgentCallbacks tests the behaviour of the NetworkAgent callbacks onNetworkCreated, onNetworkUnwanted, and onNetworkDestroyed. This uses a NetworkAgentWrapper method that sets the callbacks after the test agent is constructed. This infrastructure not sufficient to test an upcoming change which will make onNetworkCreated be fired as soon as the registration onNetworkCreated is fired as soon as the agent is registered. Fix the code so that the callbacks can be specified at agent registration time. This is also a bit more realistic since in real usage, the callbacks are methods on the NetworkAgent subclass and are already set when the agent is constructed. Bug: 143158421 Test: test-only change Change-Id: I53c58e7b6c6ae4abf08e0df5051694cc4568a510 --- .../android/server/NetworkAgentWrapper.java | 52 ++++++++++- .../server/ConnectivityServiceTest.java | 88 ++++++++----------- 2 files changed, 86 insertions(+), 54 deletions(-) diff --git a/tests/integration/util/com/android/server/NetworkAgentWrapper.java b/tests/integration/util/com/android/server/NetworkAgentWrapper.java index 2763f5a789..97688d5613 100644 --- a/tests/integration/util/com/android/server/NetworkAgentWrapper.java +++ b/tests/integration/util/com/android/server/NetworkAgentWrapper.java @@ -61,6 +61,7 @@ import java.util.List; import java.util.Objects; import java.util.Set; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.function.Consumer; public class NetworkAgentWrapper implements TestableNetworkCallback.HasNetwork { private final NetworkCapabilities mNetworkCapabilities; @@ -83,14 +84,35 @@ public class NetworkAgentWrapper implements TestableNetworkCallback.HasNetwork { private final ArrayTrackRecord.ReadHead mCallbackHistory = new ArrayTrackRecord().newReadHead(); + public static class Callbacks { + public final Consumer onNetworkCreated; + public final Consumer onNetworkUnwanted; + public final Consumer onNetworkDestroyed; + + public Callbacks() { + this(null, null, null); + } + + public Callbacks(Consumer onNetworkCreated, + Consumer onNetworkUnwanted, + Consumer onNetworkDestroyed) { + this.onNetworkCreated = onNetworkCreated; + this.onNetworkUnwanted = onNetworkUnwanted; + this.onNetworkDestroyed = onNetworkDestroyed; + } + } + + private final Callbacks mCallbacks; + public NetworkAgentWrapper(int transport, LinkProperties linkProperties, NetworkCapabilities ncTemplate, Context context) throws Exception { - this(transport, linkProperties, ncTemplate, null /* provider */, context); + this(transport, linkProperties, ncTemplate, null /* provider */, + null /* callbacks */, context); } public NetworkAgentWrapper(int transport, LinkProperties linkProperties, NetworkCapabilities ncTemplate, NetworkProvider provider, - Context context) throws Exception { + Callbacks callbacks, Context context) throws Exception { final int type = transportToLegacyType(transport); final String typeName = ConnectivityManager.getNetworkTypeName(type); mNetworkCapabilities = (ncTemplate != null) ? ncTemplate : new NetworkCapabilities(); @@ -135,6 +157,7 @@ public class NetworkAgentWrapper implements TestableNetworkCallback.HasNetwork { .setLegacyTypeName(typeName) .setLegacyExtraInfo(extraInfo) .build(); + mCallbacks = (callbacks != null) ? callbacks : new Callbacks(); mNetworkAgent = makeNetworkAgent(linkProperties, mNetworkAgentConfig, provider); } @@ -214,6 +237,31 @@ public class NetworkAgentWrapper implements TestableNetworkCallback.HasNetwork { protected void removeKeepalivePacketFilter(Message msg) { Log.i(mWrapper.mLogTag, "Remove keepalive packet filter."); } + + @Override + public void onNetworkCreated() { + super.onNetworkCreated(); + if (mWrapper.mCallbacks.onNetworkCreated != null) { + mWrapper.mCallbacks.onNetworkCreated.accept(this); + } + } + + @Override + public void onNetworkUnwanted() { + super.onNetworkUnwanted(); + if (mWrapper.mCallbacks.onNetworkUnwanted != null) { + mWrapper.mCallbacks.onNetworkUnwanted.accept(this); + } + } + + @Override + public void onNetworkDestroyed() { + super.onNetworkDestroyed(); + if (mWrapper.mCallbacks.onNetworkDestroyed != null) { + mWrapper.mCallbacks.onNetworkDestroyed.accept(this); + } + } + } public void setScore(@NonNull final NetworkScore score) { diff --git a/tests/unit/java/com/android/server/ConnectivityServiceTest.java b/tests/unit/java/com/android/server/ConnectivityServiceTest.java index 28119d8e9c..1a624e3742 100644 --- a/tests/unit/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/unit/java/com/android/server/ConnectivityServiceTest.java @@ -428,6 +428,7 @@ import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicReference; +import java.util.function.Consumer; import java.util.function.Predicate; import java.util.function.Supplier; import java.util.regex.Matcher; @@ -922,9 +923,6 @@ public class ConnectivityServiceTest { private int mProbesSucceeded; private String mNmValidationRedirectUrl = null; private boolean mNmProvNotificationRequested = false; - private Runnable mCreatedCallback; - private Runnable mUnwantedCallback; - private Runnable mDisconnectedCallback; private final ConditionVariable mNetworkStatusReceived = new ConditionVariable(); // Contains the redirectUrl from networkStatus(). Before reading, wait for @@ -932,22 +930,34 @@ public class ConnectivityServiceTest { private String mRedirectUrl; TestNetworkAgentWrapper(int transport) throws Exception { - this(transport, new LinkProperties(), null /* ncTemplate */, null /* provider */); + this(transport, new LinkProperties(), null /* ncTemplate */, null /* provider */, null); } TestNetworkAgentWrapper(int transport, LinkProperties linkProperties) throws Exception { - this(transport, linkProperties, null /* ncTemplate */, null /* provider */); + this(transport, linkProperties, null /* ncTemplate */, null /* provider */, null); } private TestNetworkAgentWrapper(int transport, LinkProperties linkProperties, NetworkCapabilities ncTemplate) throws Exception { - this(transport, linkProperties, ncTemplate, null /* provider */); + this(transport, linkProperties, ncTemplate, null /* provider */, null); } private TestNetworkAgentWrapper(int transport, LinkProperties linkProperties, NetworkCapabilities ncTemplate, NetworkProvider provider) throws Exception { - super(transport, linkProperties, ncTemplate, provider, mServiceContext); + this(transport, linkProperties, ncTemplate, provider /* provider */, null); + } + + private TestNetworkAgentWrapper(int transport, NetworkAgentWrapper.Callbacks callbacks) + throws Exception { + this(transport, new LinkProperties(), null /* ncTemplate */, null /* provider */, + callbacks); + } + + private TestNetworkAgentWrapper(int transport, LinkProperties linkProperties, + NetworkCapabilities ncTemplate, NetworkProvider provider, + NetworkAgentWrapper.Callbacks callbacks) throws Exception { + super(transport, linkProperties, ncTemplate, provider, callbacks, mServiceContext); // Waits for the NetworkAgent to be registered, which includes the creation of the // NetworkMonitor. @@ -968,23 +978,6 @@ public class ConnectivityServiceTest { mNetworkStatusReceived.open(); } - @Override - public void onNetworkCreated() { - super.onNetworkCreated(); - if (mCreatedCallback != null) mCreatedCallback.run(); - } - - @Override - public void onNetworkUnwanted() { - super.onNetworkUnwanted(); - if (mUnwantedCallback != null) mUnwantedCallback.run(); - } - - @Override - public void onNetworkDestroyed() { - super.onNetworkDestroyed(); - if (mDisconnectedCallback != null) mDisconnectedCallback.run(); - } } @Override @@ -1214,18 +1207,6 @@ public class ConnectivityServiceTest { p.timestampMillis = DATA_STALL_TIMESTAMP; mNmCallbacks.notifyDataStallSuspected(p); } - - public void setCreatedCallback(Runnable r) { - mCreatedCallback = r; - } - - public void setUnwantedCallback(Runnable r) { - mUnwantedCallback = r; - } - - public void setDisconnectedCallback(Runnable r) { - mDisconnectedCallback = r; - } } /** @@ -3566,37 +3547,35 @@ public class ConnectivityServiceTest { final NetworkRequest request = new NetworkRequest.Builder() .addTransportType(TRANSPORT_WIFI).build(); final TestNetworkCallback callback = new TestNetworkCallback(); - final AtomicReference wifiNetwork = new AtomicReference<>(); - mWiFiNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_WIFI); // Expectations for state when various callbacks fire. These expectations run on the handler // thread and not on the test thread because they need to prevent the handler thread from // advancing while they examine state. // 1. When onCreated fires, netd has been told to create the network. - mWiFiNetworkAgent.setCreatedCallback(() -> { + final Consumer onNetworkCreated = (agent) -> { eventOrder.offer("onNetworkCreated"); - wifiNetwork.set(mWiFiNetworkAgent.getNetwork()); - assertNotNull(wifiNetwork.get()); try { verify(mMockNetd).networkCreate(nativeNetworkConfigPhysical( - wifiNetwork.get().getNetId(), INetd.PERMISSION_NONE)); + agent.getNetwork().getNetId(), INetd.PERMISSION_NONE)); } catch (RemoteException impossible) { fail(); } - }); + }; // 2. onNetworkUnwanted isn't precisely ordered with respect to any particular events. Just // check that it is fired at some point after disconnect. - mWiFiNetworkAgent.setUnwantedCallback(() -> eventOrder.offer("onNetworkUnwanted")); + final Consumer onNetworkUnwanted = (agent) -> { + eventOrder.offer("onNetworkUnwanted"); + }; // 3. While the teardown timer is running, connectivity APIs report the network is gone, but // netd has not yet been told to destroy it. - final Runnable duringTeardown = () -> { + final Consumer duringTeardown = (network) -> { eventOrder.offer("timePasses"); - assertNull(mCm.getLinkProperties(wifiNetwork.get())); + assertNull(mCm.getLinkProperties(network)); try { - verify(mMockNetd, never()).networkDestroy(wifiNetwork.get().getNetId()); + verify(mMockNetd, never()).networkDestroy(network.getNetId()); } catch (RemoteException impossible) { fail(); } @@ -3604,15 +3583,20 @@ public class ConnectivityServiceTest { // 4. After onNetworkDisconnected is called, connectivity APIs report the network is gone, // and netd has been told to destroy it. - mWiFiNetworkAgent.setDisconnectedCallback(() -> { + final Consumer onNetworkDisconnected = (agent) -> { eventOrder.offer("onNetworkDisconnected"); - assertNull(mCm.getLinkProperties(wifiNetwork.get())); + assertNull(mCm.getLinkProperties(agent.getNetwork())); try { - verify(mMockNetd).networkDestroy(wifiNetwork.get().getNetId()); + verify(mMockNetd).networkDestroy(agent.getNetwork().getNetId()); } catch (RemoteException impossible) { fail(); } - }); + }; + + final NetworkAgentWrapper.Callbacks callbacks = new NetworkAgentWrapper.Callbacks( + onNetworkCreated, onNetworkUnwanted, onNetworkDisconnected); + + mWiFiNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_WIFI, callbacks); // Connect a network, and file a request for it after it has come up, to ensure the nascent // timer is cleared and the test does not have to wait for it. Filing the request after the @@ -3634,7 +3618,7 @@ public class ConnectivityServiceTest { // down the network and started the teardown timer, and short enough that the lambda is // scheduled to run before the teardown timer. final Handler h = new Handler(mCsHandlerThread.getLooper()); - h.postDelayed(duringTeardown, 150); + h.postDelayed(() -> duringTeardown.accept(mWiFiNetworkAgent.getNetwork()), 150); // Disconnect the network and check that events happened in the right order. mCm.unregisterNetworkCallback(callback);