diff --git a/service-t/src/com/android/server/ethernet/EthernetCallback.java b/service-t/src/com/android/server/ethernet/EthernetCallback.java new file mode 100644 index 0000000000..5461156485 --- /dev/null +++ b/service-t/src/com/android/server/ethernet/EthernetCallback.java @@ -0,0 +1,57 @@ +/* + * Copyright (C) 2022 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.server.ethernet; + +import android.net.EthernetNetworkManagementException; +import android.net.INetworkInterfaceOutcomeReceiver; +import android.os.RemoteException; +import android.util.Log; + +import com.android.internal.annotations.VisibleForTesting; + +/** Convenience wrapper for INetworkInterfaceOutcomeReceiver */ +@VisibleForTesting +public class EthernetCallback { + private static final String TAG = EthernetCallback.class.getSimpleName(); + private final INetworkInterfaceOutcomeReceiver mReceiver; + + public EthernetCallback(INetworkInterfaceOutcomeReceiver receiver) { + mReceiver = receiver; + } + + /** Calls INetworkInterfaceOutcomeReceiver#onResult */ + public void onResult(String ifname) { + try { + if (mReceiver != null) { + mReceiver.onResult(ifname); + } + } catch (RemoteException e) { + Log.e(TAG, "Failed to report error to OutcomeReceiver", e); + } + } + + /** Calls INetworkInterfaceOutcomeReceiver#onError */ + public void onError(String msg) { + try { + if (mReceiver != null) { + mReceiver.onError(new EthernetNetworkManagementException(msg)); + } + } catch (RemoteException e) { + Log.e(TAG, "Failed to report error to OutcomeReceiver", e); + } + } +} diff --git a/service-t/src/com/android/server/ethernet/EthernetNetworkFactory.java b/service-t/src/com/android/server/ethernet/EthernetNetworkFactory.java index e5bddf64fe..56c21eba22 100644 --- a/service-t/src/com/android/server/ethernet/EthernetNetworkFactory.java +++ b/service-t/src/com/android/server/ethernet/EthernetNetworkFactory.java @@ -22,9 +22,7 @@ import android.content.Context; import android.net.ConnectivityManager; import android.net.ConnectivityResources; import android.net.EthernetManager; -import android.net.EthernetNetworkManagementException; import android.net.EthernetNetworkSpecifier; -import android.net.INetworkInterfaceOutcomeReceiver; import android.net.IpConfiguration; import android.net.IpConfiguration.IpAssignment; import android.net.IpConfiguration.ProxySettings; @@ -42,7 +40,6 @@ import android.net.shared.ProvisioningConfiguration; import android.os.ConditionVariable; import android.os.Handler; import android.os.Looper; -import android.os.RemoteException; import android.text.TextUtils; import android.util.AndroidRuntimeException; import android.util.ArraySet; @@ -190,22 +187,19 @@ public class EthernetNetworkFactory { * {@code null} is passed, then the network's current * {@link NetworkCapabilities} will be used in support of existing APIs as * the public API does not allow this. - * @param listener an optional {@link INetworkInterfaceOutcomeReceiver} to notify callers of - * completion. */ @VisibleForTesting(visibility = VisibleForTesting.Visibility.PACKAGE) protected void updateInterface(@NonNull final String ifaceName, @Nullable final IpConfiguration ipConfig, - @Nullable final NetworkCapabilities capabilities, - @Nullable final INetworkInterfaceOutcomeReceiver listener) { + @Nullable final NetworkCapabilities capabilities) { if (!hasInterface(ifaceName)) { - maybeSendNetworkManagementCallbackForUntracked(ifaceName, listener); return; } final NetworkInterfaceState iface = mTrackingInterfaces.get(ifaceName); - iface.updateInterface(ipConfig, capabilities, listener); + iface.updateInterface(ipConfig, capabilities); mTrackingInterfaces.put(ifaceName, iface); + return; } private static NetworkCapabilities mixInCapabilities(NetworkCapabilities nc, @@ -238,10 +232,8 @@ public class EthernetNetworkFactory { /** Returns true if state has been modified */ @VisibleForTesting(visibility = VisibleForTesting.Visibility.PACKAGE) - protected boolean updateInterfaceLinkState(@NonNull final String ifaceName, final boolean up, - @Nullable final INetworkInterfaceOutcomeReceiver listener) { + protected boolean updateInterfaceLinkState(@NonNull final String ifaceName, final boolean up) { if (!hasInterface(ifaceName)) { - maybeSendNetworkManagementCallbackForUntracked(ifaceName, listener); return false; } @@ -250,14 +242,7 @@ public class EthernetNetworkFactory { } NetworkInterfaceState iface = mTrackingInterfaces.get(ifaceName); - return iface.updateLinkState(up, listener); - } - - private void maybeSendNetworkManagementCallbackForUntracked( - String ifaceName, INetworkInterfaceOutcomeReceiver listener) { - maybeSendNetworkManagementCallback(listener, null, - new EthernetNetworkManagementException( - ifaceName + " can't be updated as it is not available.")); + return iface.updateLinkState(up); } @VisibleForTesting @@ -265,25 +250,6 @@ public class EthernetNetworkFactory { return mTrackingInterfaces.containsKey(ifaceName); } - private static void maybeSendNetworkManagementCallback( - @Nullable final INetworkInterfaceOutcomeReceiver listener, - @Nullable final String iface, - @Nullable final EthernetNetworkManagementException e) { - if (null == listener) { - return; - } - - try { - if (iface != null) { - listener.onResult(iface); - } else { - listener.onError(e); - } - } catch (RemoteException re) { - Log.e(TAG, "Can't send onComplete for network management callback", re); - } - } - @VisibleForTesting static class NetworkInterfaceState { final String name; @@ -332,11 +298,6 @@ public class EthernetNetworkFactory { private class EthernetIpClientCallback extends IpClientCallbacks { private final ConditionVariable mIpClientStartCv = new ConditionVariable(false); private final ConditionVariable mIpClientShutdownCv = new ConditionVariable(false); - @Nullable INetworkInterfaceOutcomeReceiver mNetworkManagementListener; - - EthernetIpClientCallback(@Nullable final INetworkInterfaceOutcomeReceiver listener) { - mNetworkManagementListener = listener; - } @Override public void onIpClientCreated(IIpClient ipClient) { @@ -372,14 +333,14 @@ public class EthernetNetworkFactory { @Override public void onProvisioningSuccess(LinkProperties newLp) { - handleIpEvent(() -> onIpLayerStarted(newLp, mNetworkManagementListener)); + handleIpEvent(() -> onIpLayerStarted(newLp)); } @Override public void onProvisioningFailure(LinkProperties newLp) { // This cannot happen due to provisioning timeout, because our timeout is 0. It can // happen due to errors while provisioning or on provisioning loss. - handleIpEvent(() -> onIpLayerStopped(mNetworkManagementListener)); + handleIpEvent(() -> onIpLayerStopped()); } @Override @@ -491,13 +452,11 @@ public class EthernetNetworkFactory { } void updateInterface(@Nullable final IpConfiguration ipConfig, - @Nullable final NetworkCapabilities capabilities, - @Nullable final INetworkInterfaceOutcomeReceiver listener) { + @Nullable final NetworkCapabilities capabilities) { if (DBG) { Log.d(TAG, "updateInterface, iface: " + name + ", ipConfig: " + ipConfig + ", old ipConfig: " + mIpConfig + ", capabilities: " + capabilities + ", old capabilities: " + mCapabilities - + ", listener: " + listener ); } @@ -510,7 +469,7 @@ public class EthernetNetworkFactory { // TODO: Update this logic to only do a restart if required. Although a restart may // be required due to the capabilities or ipConfiguration values, not all // capabilities changes require a restart. - restart(listener); + restart(); } boolean isRestricted() { @@ -518,10 +477,6 @@ public class EthernetNetworkFactory { } private void start() { - start(null); - } - - private void start(@Nullable final INetworkInterfaceOutcomeReceiver listener) { if (mIpClient != null) { if (DBG) Log.d(TAG, "IpClient already started"); return; @@ -530,7 +485,7 @@ public class EthernetNetworkFactory { Log.d(TAG, String.format("Starting Ethernet IpClient(%s)", name)); } - mIpClientCallback = new EthernetIpClientCallback(listener); + mIpClientCallback = new EthernetIpClientCallback(); mDeps.makeIpClient(mContext, name, mIpClientCallback); mIpClientCallback.awaitIpClientStart(); @@ -540,8 +495,7 @@ public class EthernetNetworkFactory { provisionIpClient(mIpClient, mIpConfig, sTcpBufferSizes); } - void onIpLayerStarted(@NonNull final LinkProperties linkProperties, - @Nullable final INetworkInterfaceOutcomeReceiver listener) { + void onIpLayerStarted(@NonNull final LinkProperties linkProperties) { if (mNetworkAgent != null) { Log.e(TAG, "Already have a NetworkAgent - aborting new request"); stop(); @@ -573,40 +527,18 @@ public class EthernetNetworkFactory { }); mNetworkAgent.register(); mNetworkAgent.markConnected(); - realizeNetworkManagementCallback(name, null); } - void onIpLayerStopped(@Nullable final INetworkInterfaceOutcomeReceiver listener) { + void onIpLayerStopped() { // There is no point in continuing if the interface is gone as stop() will be triggered // by removeInterface() when processed on the handler thread and start() won't // work for a non-existent interface. if (null == mDeps.getNetworkInterfaceByName(name)) { if (DBG) Log.d(TAG, name + " is no longer available."); // Send a callback in case a provisioning request was in progress. - maybeSendNetworkManagementCallbackForAbort(); return; } - restart(listener); - } - - private void maybeSendNetworkManagementCallbackForAbort() { - realizeNetworkManagementCallback(null, - new EthernetNetworkManagementException( - "The IP provisioning request has been aborted.")); - } - - // Must be called on the handler thread - private void realizeNetworkManagementCallback(@Nullable final String iface, - @Nullable final EthernetNetworkManagementException e) { - ensureRunningOnEthernetHandlerThread(); - if (null == mIpClientCallback) { - return; - } - - EthernetNetworkFactory.maybeSendNetworkManagementCallback( - mIpClientCallback.mNetworkManagementListener, iface, e); - // Only send a single callback per listener. - mIpClientCallback.mNetworkManagementListener = null; + restart(); } private void ensureRunningOnEthernetHandlerThread() { @@ -636,12 +568,8 @@ public class EthernetNetworkFactory { } /** Returns true if state has been modified */ - boolean updateLinkState(final boolean up, - @Nullable final INetworkInterfaceOutcomeReceiver listener) { + boolean updateLinkState(final boolean up) { if (mLinkUp == up) { - EthernetNetworkFactory.maybeSendNetworkManagementCallback(listener, null, - new EthernetNetworkManagementException( - "No changes with requested link state " + up + " for " + name)); return false; } mLinkUp = up; @@ -654,7 +582,6 @@ public class EthernetNetworkFactory { registerNetworkOffer(); } - EthernetNetworkFactory.maybeSendNetworkManagementCallback(listener, name, null); return true; } @@ -665,8 +592,7 @@ public class EthernetNetworkFactory { mIpClientCallback.awaitIpClientShutdown(); mIpClient = null; } - // Send an abort callback if an updateInterface request was in progress. - maybeSendNetworkManagementCallbackForAbort(); + mIpClientCallback = null; if (mNetworkAgent != null) { @@ -723,13 +649,9 @@ public class EthernetNetworkFactory { } void restart() { - restart(null); - } - - void restart(@Nullable final INetworkInterfaceOutcomeReceiver listener) { if (DBG) Log.d(TAG, "reconnecting Ethernet"); stop(); - start(listener); + start(); } @Override diff --git a/service-t/src/com/android/server/ethernet/EthernetServiceImpl.java b/service-t/src/com/android/server/ethernet/EthernetServiceImpl.java index dae3d2a9d4..edf04b2570 100644 --- a/service-t/src/com/android/server/ethernet/EthernetServiceImpl.java +++ b/service-t/src/com/android/server/ethernet/EthernetServiceImpl.java @@ -260,7 +260,7 @@ public class EthernetServiceImpl extends IEthernetManager.Stub { @Override public void updateConfiguration(@NonNull final String iface, @NonNull final EthernetNetworkUpdateRequest request, - @Nullable final INetworkInterfaceOutcomeReceiver listener) { + @Nullable final INetworkInterfaceOutcomeReceiver cb) { Objects.requireNonNull(iface); Objects.requireNonNull(request); throwIfEthernetNotStarted(); @@ -277,31 +277,31 @@ public class EthernetServiceImpl extends IEthernetManager.Stub { } mTracker.updateConfiguration( - iface, request.getIpConfiguration(), nc, listener); + iface, request.getIpConfiguration(), nc, new EthernetCallback(cb)); } @Override public void enableInterface(@NonNull final String iface, - @Nullable final INetworkInterfaceOutcomeReceiver listener) { - Log.i(TAG, "enableInterface called with: iface=" + iface + ", listener=" + listener); + @Nullable final INetworkInterfaceOutcomeReceiver cb) { + Log.i(TAG, "enableInterface called with: iface=" + iface + ", cb=" + cb); Objects.requireNonNull(iface); throwIfEthernetNotStarted(); enforceAdminPermission(iface, false, "enableInterface()"); - mTracker.enableInterface(iface, listener); + mTracker.enableInterface(iface, new EthernetCallback(cb)); } @Override public void disableInterface(@NonNull final String iface, - @Nullable final INetworkInterfaceOutcomeReceiver listener) { - Log.i(TAG, "disableInterface called with: iface=" + iface + ", listener=" + listener); + @Nullable final INetworkInterfaceOutcomeReceiver cb) { + Log.i(TAG, "disableInterface called with: iface=" + iface + ", cb=" + cb); Objects.requireNonNull(iface); throwIfEthernetNotStarted(); enforceAdminPermission(iface, false, "disableInterface()"); - mTracker.disableInterface(iface, listener); + mTracker.disableInterface(iface, new EthernetCallback(cb)); } @Override diff --git a/service-t/src/com/android/server/ethernet/EthernetTracker.java b/service-t/src/com/android/server/ethernet/EthernetTracker.java index ba367cff64..77addcf6ff 100644 --- a/service-t/src/com/android/server/ethernet/EthernetTracker.java +++ b/service-t/src/com/android/server/ethernet/EthernetTracker.java @@ -29,7 +29,6 @@ import android.net.ConnectivityResources; import android.net.EthernetManager; import android.net.IEthernetServiceListener; import android.net.INetd; -import android.net.INetworkInterfaceOutcomeReceiver; import android.net.ITetheredInterfaceCallback; import android.net.InterfaceConfigurationParcel; import android.net.IpConfiguration; @@ -271,7 +270,7 @@ public class EthernetTracker { } writeIpConfiguration(iface, ipConfiguration); mHandler.post(() -> { - mFactory.updateInterface(iface, ipConfiguration, null, null); + mFactory.updateInterface(iface, ipConfiguration, null); broadcastInterfaceStateChange(iface); }); } @@ -335,7 +334,7 @@ public class EthernetTracker { protected void updateConfiguration(@NonNull final String iface, @Nullable final IpConfiguration ipConfig, @Nullable final NetworkCapabilities capabilities, - @Nullable final INetworkInterfaceOutcomeReceiver listener) { + @Nullable final EthernetCallback cb) { if (DBG) { Log.i(TAG, "updateConfiguration, iface: " + iface + ", capabilities: " + capabilities + ", ipConfig: " + ipConfig); @@ -353,21 +352,29 @@ public class EthernetTracker { mNetworkCapabilities.put(iface, capabilities); } mHandler.post(() -> { - mFactory.updateInterface(iface, localIpConfig, capabilities, listener); - broadcastInterfaceStateChange(iface); + mFactory.updateInterface(iface, localIpConfig, capabilities); + + // only broadcast state change when the ip configuration is updated. + if (ipConfig != null) { + broadcastInterfaceStateChange(iface); + } + // Always return success. Even if the interface does not currently exist, the + // IpConfiguration and NetworkCapabilities were saved and will be applied if an + // interface with the given name is ever added. + cb.onResult(iface); }); } @VisibleForTesting(visibility = PACKAGE) protected void enableInterface(@NonNull final String iface, - @Nullable final INetworkInterfaceOutcomeReceiver listener) { - mHandler.post(() -> updateInterfaceState(iface, true, listener)); + @Nullable final EthernetCallback cb) { + mHandler.post(() -> updateInterfaceState(iface, true, cb)); } @VisibleForTesting(visibility = PACKAGE) protected void disableInterface(@NonNull final String iface, - @Nullable final INetworkInterfaceOutcomeReceiver listener) { - mHandler.post(() -> updateInterfaceState(iface, false, listener)); + @Nullable final EthernetCallback cb) { + mHandler.post(() -> updateInterfaceState(iface, false, cb)); } IpConfiguration getIpConfiguration(String iface) { @@ -614,17 +621,25 @@ public class EthernetTracker { } private void updateInterfaceState(String iface, boolean up) { - updateInterfaceState(iface, up, null /* listener */); + // TODO: pull EthernetCallbacks out of EthernetNetworkFactory. + updateInterfaceState(iface, up, new EthernetCallback(null /* cb */)); } private void updateInterfaceState(@NonNull final String iface, final boolean up, - @Nullable final INetworkInterfaceOutcomeReceiver listener) { + @Nullable final EthernetCallback cb) { final int mode = getInterfaceMode(iface); final boolean factoryLinkStateUpdated = (mode == INTERFACE_MODE_CLIENT) - && mFactory.updateInterfaceLinkState(iface, up, listener); + && mFactory.updateInterfaceLinkState(iface, up); if (factoryLinkStateUpdated) { broadcastInterfaceStateChange(iface); + cb.onResult(iface); + } else { + // The interface may already be in the correct state. While usually this should not be + // an error, since updateInterfaceState is used in setInterfaceEnabled() / + // setInterfaceDisabled() it has to be reported as such. + // It is also possible that the interface disappeared or is in server mode. + cb.onError("Failed to set link state " + (up ? "up" : "down") + " for " + iface); } } diff --git a/tests/unit/java/com/android/server/ethernet/EthernetNetworkFactoryTest.java b/tests/unit/java/com/android/server/ethernet/EthernetNetworkFactoryTest.java index 503d920e04..aad80d51d0 100644 --- a/tests/unit/java/com/android/server/ethernet/EthernetNetworkFactoryTest.java +++ b/tests/unit/java/com/android/server/ethernet/EthernetNetworkFactoryTest.java @@ -20,7 +20,6 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotSame; import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyString; @@ -38,9 +37,7 @@ import android.app.test.MockAnswerUtil.AnswerWithArguments; import android.content.Context; import android.content.res.Resources; import android.net.ConnectivityManager; -import android.net.EthernetNetworkManagementException; import android.net.EthernetNetworkSpecifier; -import android.net.INetworkInterfaceOutcomeReceiver; import android.net.IpConfiguration; import android.net.LinkAddress; import android.net.LinkProperties; @@ -55,7 +52,6 @@ import android.net.ip.IpClientCallbacks; import android.net.ip.IpClientManager; import android.os.Build; import android.os.Handler; -import android.os.IBinder; import android.os.Looper; import android.os.test.TestLooper; @@ -74,9 +70,6 @@ import org.mockito.Mock; import org.mockito.MockitoAnnotations; import java.util.Objects; -import java.util.concurrent.CompletableFuture; -import java.util.concurrent.ExecutionException; -import java.util.concurrent.TimeUnit; @SmallTest @RunWith(DevSdkIgnoreRunner.class) @@ -84,7 +77,6 @@ import java.util.concurrent.TimeUnit; public class EthernetNetworkFactoryTest { private static final int TIMEOUT_MS = 2_000; private static final String TEST_IFACE = "test123"; - private static final INetworkInterfaceOutcomeReceiver NULL_LISTENER = null; private static final String IP_ADDR = "192.0.2.2/25"; private static final LinkAddress LINK_ADDR = new LinkAddress(IP_ADDR); private static final String HW_ADDR = "01:02:03:04:05:06"; @@ -241,7 +233,7 @@ public class EthernetNetworkFactoryTest { final IpConfiguration ipConfig = createDefaultIpConfig(); mNetFactory.addInterface(iface, HW_ADDR, ipConfig, createInterfaceCapsBuilder(transportType).build()); - assertTrue(mNetFactory.updateInterfaceLinkState(iface, true, NULL_LISTENER)); + assertTrue(mNetFactory.updateInterfaceLinkState(iface, true)); ArgumentCaptor captor = ArgumentCaptor.forClass( NetworkOfferCallback.class); @@ -295,7 +287,7 @@ public class EthernetNetworkFactoryTest { // then calling onNetworkUnwanted. mNetFactory.addInterface(iface, HW_ADDR, createDefaultIpConfig(), createInterfaceCapsBuilder(NetworkCapabilities.TRANSPORT_ETHERNET).build()); - assertTrue(mNetFactory.updateInterfaceLinkState(iface, true, NULL_LISTENER)); + assertTrue(mNetFactory.updateInterfaceLinkState(iface, true)); clearInvocations(mIpClient); clearInvocations(mNetworkAgent); @@ -305,81 +297,63 @@ public class EthernetNetworkFactoryTest { public void testUpdateInterfaceLinkStateForActiveProvisioningInterface() throws Exception { initEthernetNetworkFactory(); createInterfaceUndergoingProvisioning(TEST_IFACE); - final TestNetworkManagementListener listener = new TestNetworkManagementListener(); // verify that the IpClient gets shut down when interface state changes to down. - final boolean ret = - mNetFactory.updateInterfaceLinkState(TEST_IFACE, false /* up */, listener); + final boolean ret = mNetFactory.updateInterfaceLinkState(TEST_IFACE, false /* up */); assertTrue(ret); verify(mIpClient).shutdown(); - assertEquals(TEST_IFACE, listener.expectOnResult()); } @Test public void testUpdateInterfaceLinkStateForProvisionedInterface() throws Exception { initEthernetNetworkFactory(); createAndVerifyProvisionedInterface(TEST_IFACE); - final TestNetworkManagementListener listenerDown = new TestNetworkManagementListener(); - final TestNetworkManagementListener listenerUp = new TestNetworkManagementListener(); - final boolean retDown = - mNetFactory.updateInterfaceLinkState(TEST_IFACE, false /* up */, listenerDown); + final boolean retDown = mNetFactory.updateInterfaceLinkState(TEST_IFACE, false /* up */); assertTrue(retDown); verifyStop(); - assertEquals(TEST_IFACE, listenerDown.expectOnResult()); - final boolean retUp = - mNetFactory.updateInterfaceLinkState(TEST_IFACE, true /* up */, listenerUp); + final boolean retUp = mNetFactory.updateInterfaceLinkState(TEST_IFACE, true /* up */); assertTrue(retUp); - assertEquals(TEST_IFACE, listenerUp.expectOnResult()); } @Test public void testUpdateInterfaceLinkStateForUnprovisionedInterface() throws Exception { initEthernetNetworkFactory(); createUnprovisionedInterface(TEST_IFACE); - final TestNetworkManagementListener listener = new TestNetworkManagementListener(); - final boolean ret = - mNetFactory.updateInterfaceLinkState(TEST_IFACE, false /* up */, listener); + final boolean ret = mNetFactory.updateInterfaceLinkState(TEST_IFACE, false /* up */); assertTrue(ret); // There should not be an active IPClient or NetworkAgent. verify(mDeps, never()).makeIpClient(any(), any(), any()); verify(mDeps, never()) .makeEthernetNetworkAgent(any(), any(), any(), any(), any(), any(), any()); - assertEquals(TEST_IFACE, listener.expectOnResult()); } @Test public void testUpdateInterfaceLinkStateForNonExistingInterface() throws Exception { initEthernetNetworkFactory(); - final TestNetworkManagementListener listener = new TestNetworkManagementListener(); // if interface was never added, link state cannot be updated. - final boolean ret = - mNetFactory.updateInterfaceLinkState(TEST_IFACE, true /* up */, listener); + final boolean ret = mNetFactory.updateInterfaceLinkState(TEST_IFACE, true /* up */); assertFalse(ret); verifyNoStopOrStart(); - listener.expectOnError(); } @Test public void testUpdateInterfaceLinkStateWithNoChanges() throws Exception { initEthernetNetworkFactory(); createAndVerifyProvisionedInterface(TEST_IFACE); - final TestNetworkManagementListener listener = new TestNetworkManagementListener(); - final boolean ret = - mNetFactory.updateInterfaceLinkState(TEST_IFACE, true /* up */, listener); + final boolean ret = mNetFactory.updateInterfaceLinkState(TEST_IFACE, true /* up */); assertFalse(ret); verifyNoStopOrStart(); - listener.expectOnError(); } @Test @@ -571,127 +545,16 @@ public class EthernetNetworkFactoryTest { verify(mNetworkAgent).markConnected(); } - private static final class TestNetworkManagementListener - implements INetworkInterfaceOutcomeReceiver { - private final CompletableFuture mResult = new CompletableFuture<>(); - - @Override - public void onResult(@NonNull String iface) { - mResult.complete(iface); - } - - @Override - public void onError(@NonNull EthernetNetworkManagementException exception) { - mResult.completeExceptionally(exception); - } - - String expectOnResult() throws Exception { - return mResult.get(TIMEOUT_MS, TimeUnit.MILLISECONDS); - } - - void expectOnError() throws Exception { - assertThrows(EthernetNetworkManagementException.class, () -> { - try { - mResult.get(); - } catch (ExecutionException e) { - throw e.getCause(); - } - }); - } - - @Override - public IBinder asBinder() { - return null; - } - } - - @Test - public void testUpdateInterfaceCallsListenerCorrectlyOnSuccess() throws Exception { - initEthernetNetworkFactory(); - createAndVerifyProvisionedInterface(TEST_IFACE); - final NetworkCapabilities capabilities = createDefaultFilterCaps(); - final IpConfiguration ipConfiguration = createStaticIpConfig(); - final TestNetworkManagementListener listener = new TestNetworkManagementListener(); - - mNetFactory.updateInterface(TEST_IFACE, ipConfiguration, capabilities, listener); - triggerOnProvisioningSuccess(); - - assertEquals(TEST_IFACE, listener.expectOnResult()); - } - - @Test - public void testUpdateInterfaceAbortsOnConcurrentRemoveInterface() throws Exception { - initEthernetNetworkFactory(); - verifyNetworkManagementCallIsAbortedWhenInterrupted( - TEST_IFACE, - () -> mNetFactory.removeInterface(TEST_IFACE)); - } - - @Test - public void testUpdateInterfaceAbortsOnConcurrentUpdateInterfaceLinkState() throws Exception { - initEthernetNetworkFactory(); - verifyNetworkManagementCallIsAbortedWhenInterrupted( - TEST_IFACE, - () -> mNetFactory.updateInterfaceLinkState(TEST_IFACE, false, NULL_LISTENER)); - } - - @Test - public void testUpdateInterfaceAbortsOnNetworkUneededRemovesAllRequests() throws Exception { - initEthernetNetworkFactory(); - verifyNetworkManagementCallIsAbortedWhenInterrupted( - TEST_IFACE, - () -> mNetworkOfferCallback.onNetworkUnneeded(mRequestToKeepNetworkUp)); - } - - @Test - public void testUpdateInterfaceCallsListenerCorrectlyOnConcurrentRequests() throws Exception { - initEthernetNetworkFactory(); - final NetworkCapabilities capabilities = createDefaultFilterCaps(); - final IpConfiguration ipConfiguration = createStaticIpConfig(); - final TestNetworkManagementListener successfulListener = - new TestNetworkManagementListener(); - - // If two calls come in before the first one completes, the first listener will be aborted - // and the second one will be successful. - verifyNetworkManagementCallIsAbortedWhenInterrupted( - TEST_IFACE, - () -> { - mNetFactory.updateInterface( - TEST_IFACE, ipConfiguration, capabilities, successfulListener); - triggerOnProvisioningSuccess(); - }); - - assertEquals(successfulListener.expectOnResult(), TEST_IFACE); - assertEquals(TEST_IFACE, successfulListener.expectOnResult()); - } - - private void verifyNetworkManagementCallIsAbortedWhenInterrupted( - @NonNull final String iface, - @NonNull final Runnable interruptingRunnable) throws Exception { - createAndVerifyProvisionedInterface(iface); - final NetworkCapabilities capabilities = createDefaultFilterCaps(); - final IpConfiguration ipConfiguration = createStaticIpConfig(); - final TestNetworkManagementListener failedListener = new TestNetworkManagementListener(); - - // An active update request will be aborted on interrupt prior to provisioning completion. - mNetFactory.updateInterface(iface, ipConfiguration, capabilities, failedListener); - interruptingRunnable.run(); - - failedListener.expectOnError(); - } - @Test public void testUpdateInterfaceRestartsAgentCorrectly() throws Exception { initEthernetNetworkFactory(); createAndVerifyProvisionedInterface(TEST_IFACE); final NetworkCapabilities capabilities = createDefaultFilterCaps(); final IpConfiguration ipConfiguration = createStaticIpConfig(); - final TestNetworkManagementListener listener = new TestNetworkManagementListener(); - mNetFactory.updateInterface(TEST_IFACE, ipConfiguration, capabilities, listener); + mNetFactory.updateInterface(TEST_IFACE, ipConfiguration, capabilities); triggerOnProvisioningSuccess(); - assertEquals(TEST_IFACE, listener.expectOnResult()); verify(mDeps).makeEthernetNetworkAgent(any(), any(), eq(capabilities), any(), any(), any(), any()); verifyRestart(ipConfiguration); @@ -703,12 +566,10 @@ public class EthernetNetworkFactoryTest { // No interface exists due to not calling createAndVerifyProvisionedInterface(...). final NetworkCapabilities capabilities = createDefaultFilterCaps(); final IpConfiguration ipConfiguration = createStaticIpConfig(); - final TestNetworkManagementListener listener = new TestNetworkManagementListener(); - mNetFactory.updateInterface(TEST_IFACE, ipConfiguration, capabilities, listener); + mNetFactory.updateInterface(TEST_IFACE, ipConfiguration, capabilities); verifyNoStopOrStart(); - listener.expectOnError(); } @Test @@ -717,8 +578,8 @@ public class EthernetNetworkFactoryTest { createAndVerifyProvisionedInterface(TEST_IFACE); final IpConfiguration initialIpConfig = createStaticIpConfig(); - mNetFactory.updateInterface(TEST_IFACE, initialIpConfig, null /*capabilities*/, - null /*listener*/); + mNetFactory.updateInterface(TEST_IFACE, initialIpConfig, null /*capabilities*/); + triggerOnProvisioningSuccess(); verifyRestart(initialIpConfig); @@ -729,8 +590,7 @@ public class EthernetNetworkFactoryTest { // verify that sending a null ipConfig does not update the current ipConfig. - mNetFactory.updateInterface(TEST_IFACE, null /*ipConfig*/, null /*capabilities*/, - null /*listener*/); + mNetFactory.updateInterface(TEST_IFACE, null /*ipConfig*/, null /*capabilities*/); triggerOnProvisioningSuccess(); verifyRestart(initialIpConfig); } @@ -739,7 +599,7 @@ public class EthernetNetworkFactoryTest { public void testOnNetworkNeededOnStaleNetworkOffer() throws Exception { initEthernetNetworkFactory(); createAndVerifyProvisionedInterface(TEST_IFACE); - mNetFactory.updateInterfaceLinkState(TEST_IFACE, false, null); + mNetFactory.updateInterfaceLinkState(TEST_IFACE, false); verify(mNetworkProvider).unregisterNetworkOffer(mNetworkOfferCallback); // It is possible that even after a network offer is unregistered, CS still sends it // onNetworkNeeded() callbacks. diff --git a/tests/unit/java/com/android/server/ethernet/EthernetServiceImplTest.java b/tests/unit/java/com/android/server/ethernet/EthernetServiceImplTest.java index a1d93a025e..9bf893a58b 100644 --- a/tests/unit/java/com/android/server/ethernet/EthernetServiceImplTest.java +++ b/tests/unit/java/com/android/server/ethernet/EthernetServiceImplTest.java @@ -21,6 +21,7 @@ import static android.net.NetworkCapabilities.TRANSPORT_TEST; import static org.junit.Assert.assertThrows; import static org.junit.Assert.fail; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.ArgumentMatchers.isNull; @@ -209,7 +210,8 @@ public class EthernetServiceImplTest { NULL_LISTENER); verify(mEthernetTracker).updateConfiguration(eq(TEST_IFACE), eq(UPDATE_REQUEST_WITHOUT_CAPABILITIES.getIpConfiguration()), - eq(UPDATE_REQUEST_WITHOUT_CAPABILITIES.getNetworkCapabilities()), isNull()); + eq(UPDATE_REQUEST_WITHOUT_CAPABILITIES.getNetworkCapabilities()), + any(EthernetCallback.class)); } private void denyManageEthPermission() { @@ -285,7 +287,8 @@ public class EthernetServiceImplTest { verify(mEthernetTracker).updateConfiguration( eq(TEST_IFACE), eq(UPDATE_REQUEST.getIpConfiguration()), - eq(UPDATE_REQUEST.getNetworkCapabilities()), eq(NULL_LISTENER)); + eq(UPDATE_REQUEST.getNetworkCapabilities()), + any(EthernetCallback.class)); } @Test @@ -303,19 +306,20 @@ public class EthernetServiceImplTest { verify(mEthernetTracker).updateConfiguration( eq(TEST_IFACE), isNull(), - eq(ncWithSpecifier), eq(NULL_LISTENER)); + eq(ncWithSpecifier), any(EthernetCallback.class)); } @Test public void testEnableInterface() { mEthernetServiceImpl.enableInterface(TEST_IFACE, NULL_LISTENER); - verify(mEthernetTracker).enableInterface(eq(TEST_IFACE), eq(NULL_LISTENER)); + verify(mEthernetTracker).enableInterface(eq(TEST_IFACE), + any(EthernetCallback.class)); } @Test public void testDisableInterface() { mEthernetServiceImpl.disableInterface(TEST_IFACE, NULL_LISTENER); - verify(mEthernetTracker).disableInterface(eq(TEST_IFACE), eq(NULL_LISTENER)); + verify(mEthernetTracker).disableInterface(eq(TEST_IFACE), any(EthernetCallback.class)); } @Test @@ -328,7 +332,7 @@ public class EthernetServiceImplTest { mEthernetServiceImpl.updateConfiguration(TEST_IFACE, request, NULL_LISTENER); verify(mEthernetTracker).updateConfiguration(eq(TEST_IFACE), eq(request.getIpConfiguration()), - eq(request.getNetworkCapabilities()), isNull()); + eq(request.getNetworkCapabilities()), any(EthernetCallback.class)); } @Test @@ -337,7 +341,8 @@ public class EthernetServiceImplTest { NULL_LISTENER); verify(mEthernetTracker).updateConfiguration(eq(TEST_IFACE), eq(UPDATE_REQUEST_WITHOUT_IP_CONFIG.getIpConfiguration()), - eq(UPDATE_REQUEST_WITHOUT_IP_CONFIG.getNetworkCapabilities()), isNull()); + eq(UPDATE_REQUEST_WITHOUT_IP_CONFIG.getNetworkCapabilities()), + any(EthernetCallback.class)); } @Test @@ -369,7 +374,7 @@ public class EthernetServiceImplTest { verify(mEthernetTracker).updateConfiguration( eq(TEST_IFACE), eq(request.getIpConfiguration()), - eq(request.getNetworkCapabilities()), eq(NULL_LISTENER)); + eq(request.getNetworkCapabilities()), any(EthernetCallback.class)); } @Test @@ -379,7 +384,8 @@ public class EthernetServiceImplTest { denyManageEthPermission(); mEthernetServiceImpl.enableInterface(TEST_IFACE, NULL_LISTENER); - verify(mEthernetTracker).enableInterface(eq(TEST_IFACE), eq(NULL_LISTENER)); + verify(mEthernetTracker).enableInterface(eq(TEST_IFACE), + any(EthernetCallback.class)); } @Test @@ -389,7 +395,8 @@ public class EthernetServiceImplTest { denyManageEthPermission(); mEthernetServiceImpl.disableInterface(TEST_IFACE, NULL_LISTENER); - verify(mEthernetTracker).disableInterface(eq(TEST_IFACE), eq(NULL_LISTENER)); + verify(mEthernetTracker).disableInterface(eq(TEST_IFACE), + any(EthernetCallback.class)); } private void denyPermissions(String... permissions) { diff --git a/tests/unit/java/com/android/server/ethernet/EthernetTrackerTest.java b/tests/unit/java/com/android/server/ethernet/EthernetTrackerTest.java index 0376a2a85f..082a016241 100644 --- a/tests/unit/java/com/android/server/ethernet/EthernetTrackerTest.java +++ b/tests/unit/java/com/android/server/ethernet/EthernetTrackerTest.java @@ -39,7 +39,6 @@ import android.content.Context; import android.net.EthernetManager; import android.net.IEthernetServiceListener; import android.net.INetd; -import android.net.INetworkInterfaceOutcomeReceiver; import android.net.InetAddresses; import android.net.InterfaceConfigurationParcel; import android.net.IpConfiguration; @@ -76,7 +75,7 @@ public class EthernetTrackerTest { private static final String TEST_IFACE = "test123"; private static final int TIMEOUT_MS = 1_000; private static final String THREAD_NAME = "EthernetServiceThread"; - private static final INetworkInterfaceOutcomeReceiver NULL_LISTENER = null; + private static final EthernetCallback NULL_CB = new EthernetCallback(null); private EthernetTracker tracker; private HandlerThread mHandlerThread; @Mock private Context mContext; @@ -88,7 +87,7 @@ public class EthernetTrackerTest { public void setUp() throws RemoteException { MockitoAnnotations.initMocks(this); initMockResources(); - when(mFactory.updateInterfaceLinkState(anyString(), anyBoolean(), any())).thenReturn(false); + when(mFactory.updateInterfaceLinkState(anyString(), anyBoolean())).thenReturn(false); when(mNetd.interfaceGetList()).thenReturn(new String[0]); mHandlerThread = new HandlerThread(THREAD_NAME); mHandlerThread.start(); @@ -344,31 +343,29 @@ public class EthernetTrackerTest { new StaticIpConfiguration.Builder().setIpAddress(linkAddr).build(); final IpConfiguration ipConfig = new IpConfiguration.Builder().setStaticIpConfiguration(staticIpConfig).build(); - final INetworkInterfaceOutcomeReceiver listener = null; + final EthernetCallback listener = new EthernetCallback(null); tracker.updateConfiguration(TEST_IFACE, ipConfig, capabilities, listener); waitForIdle(); verify(mFactory).updateInterface( - eq(TEST_IFACE), eq(ipConfig), eq(capabilities), eq(listener)); + eq(TEST_IFACE), eq(ipConfig), eq(capabilities)); } @Test public void testEnableInterfaceCorrectlyCallsFactory() { - tracker.enableInterface(TEST_IFACE, NULL_LISTENER); + tracker.enableInterface(TEST_IFACE, NULL_CB); waitForIdle(); - verify(mFactory).updateInterfaceLinkState(eq(TEST_IFACE), eq(true /* up */), - eq(NULL_LISTENER)); + verify(mFactory).updateInterfaceLinkState(eq(TEST_IFACE), eq(true /* up */)); } @Test public void testDisableInterfaceCorrectlyCallsFactory() { - tracker.disableInterface(TEST_IFACE, NULL_LISTENER); + tracker.disableInterface(TEST_IFACE, NULL_CB); waitForIdle(); - verify(mFactory).updateInterfaceLinkState(eq(TEST_IFACE), eq(false /* up */), - eq(NULL_LISTENER)); + verify(mFactory).updateInterfaceLinkState(eq(TEST_IFACE), eq(false /* up */)); } @Test