ethernet: remove callback from factory updateInterfaceLinkState
updateInterfaceLinkState already returns true / false depending on the
success, so the EthernetCallback can be removed and sent from
EthernetTracker instead.
Test: atest EthernetManagerTest
Bug: 225317892
Change-Id: Id6ae7929097840ac5c9a4d244fdf669a06b94ed5
diff --git a/service-t/src/com/android/server/ethernet/EthernetNetworkFactory.java b/service-t/src/com/android/server/ethernet/EthernetNetworkFactory.java
index 51c750c..56c21eb 100644
--- a/service-t/src/com/android/server/ethernet/EthernetNetworkFactory.java
+++ b/service-t/src/com/android/server/ethernet/EthernetNetworkFactory.java
@@ -232,10 +232,8 @@
/** Returns true if state has been modified */
@VisibleForTesting(visibility = VisibleForTesting.Visibility.PACKAGE)
- protected boolean updateInterfaceLinkState(@NonNull final String ifaceName, final boolean up,
- final EthernetCallback cb) {
+ protected boolean updateInterfaceLinkState(@NonNull final String ifaceName, final boolean up) {
if (!hasInterface(ifaceName)) {
- cb.onError(ifaceName + " is not tracked by the ethernet service");
return false;
}
@@ -244,7 +242,7 @@
}
NetworkInterfaceState iface = mTrackingInterfaces.get(ifaceName);
- return iface.updateLinkState(up, cb);
+ return iface.updateLinkState(up);
}
@VisibleForTesting
@@ -570,9 +568,8 @@
}
/** Returns true if state has been modified */
- boolean updateLinkState(final boolean up, final EthernetCallback cb) {
+ boolean updateLinkState(final boolean up) {
if (mLinkUp == up) {
- cb.onError("No changes with requested link state " + up + " for " + name);
return false;
}
mLinkUp = up;
@@ -585,7 +582,6 @@
registerNetworkOffer();
}
- cb.onResult(name);
return true;
}
diff --git a/service-t/src/com/android/server/ethernet/EthernetTracker.java b/service-t/src/com/android/server/ethernet/EthernetTracker.java
index 8b956fa..77addcf 100644
--- a/service-t/src/com/android/server/ethernet/EthernetTracker.java
+++ b/service-t/src/com/android/server/ethernet/EthernetTracker.java
@@ -629,10 +629,17 @@
@Nullable final EthernetCallback cb) {
final int mode = getInterfaceMode(iface);
final boolean factoryLinkStateUpdated = (mode == INTERFACE_MODE_CLIENT)
- && mFactory.updateInterfaceLinkState(iface, up, cb);
+ && 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 dcacf23..aad80d5 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.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,7 +37,6 @@
import android.content.Context;
import android.content.res.Resources;
import android.net.ConnectivityManager;
-import android.net.EthernetNetworkManagementException;
import android.net.EthernetNetworkSpecifier;
import android.net.IpConfiguration;
import android.net.LinkAddress;
@@ -72,9 +70,6 @@
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)
@@ -82,7 +77,6 @@
public class EthernetNetworkFactoryTest {
private static final int TIMEOUT_MS = 2_000;
private static final String TEST_IFACE = "test123";
- private static final EthernetCallback NULL_CB = new EthernetCallback(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";
@@ -239,7 +233,7 @@
final IpConfiguration ipConfig = createDefaultIpConfig();
mNetFactory.addInterface(iface, HW_ADDR, ipConfig,
createInterfaceCapsBuilder(transportType).build());
- assertTrue(mNetFactory.updateInterfaceLinkState(iface, true, NULL_CB));
+ assertTrue(mNetFactory.updateInterfaceLinkState(iface, true));
ArgumentCaptor<NetworkOfferCallback> captor = ArgumentCaptor.forClass(
NetworkOfferCallback.class);
@@ -293,7 +287,7 @@
// then calling onNetworkUnwanted.
mNetFactory.addInterface(iface, HW_ADDR, createDefaultIpConfig(),
createInterfaceCapsBuilder(NetworkCapabilities.TRANSPORT_ETHERNET).build());
- assertTrue(mNetFactory.updateInterfaceLinkState(iface, true, NULL_CB));
+ assertTrue(mNetFactory.updateInterfaceLinkState(iface, true));
clearInvocations(mIpClient);
clearInvocations(mNetworkAgent);
@@ -303,81 +297,63 @@
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
@@ -569,39 +545,6 @@
verify(mNetworkAgent).markConnected();
}
- private static final class TestNetworkManagementListener
- extends EthernetCallback {
- private final CompletableFuture<String> mResult = new CompletableFuture<>();
-
- TestNetworkManagementListener() {
- super(null);
- }
-
- @Override
- public void onResult(@NonNull String iface) {
- mResult.complete(iface);
- }
-
- @Override
- public void onError(String msg) {
- mResult.completeExceptionally(new EthernetNetworkManagementException(msg));
- }
-
- 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();
- }
- });
- }
- }
-
@Test
public void testUpdateInterfaceRestartsAgentCorrectly() throws Exception {
initEthernetNetworkFactory();
@@ -656,7 +599,7 @@
public void testOnNetworkNeededOnStaleNetworkOffer() throws Exception {
initEthernetNetworkFactory();
createAndVerifyProvisionedInterface(TEST_IFACE);
- mNetFactory.updateInterfaceLinkState(TEST_IFACE, false, new EthernetCallback(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/EthernetTrackerTest.java b/tests/unit/java/com/android/server/ethernet/EthernetTrackerTest.java
index 1df56c7..082a016 100644
--- a/tests/unit/java/com/android/server/ethernet/EthernetTrackerTest.java
+++ b/tests/unit/java/com/android/server/ethernet/EthernetTrackerTest.java
@@ -87,7 +87,7 @@
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();
@@ -357,8 +357,7 @@
tracker.enableInterface(TEST_IFACE, NULL_CB);
waitForIdle();
- verify(mFactory).updateInterfaceLinkState(eq(TEST_IFACE), eq(true /* up */),
- eq(NULL_CB));
+ verify(mFactory).updateInterfaceLinkState(eq(TEST_IFACE), eq(true /* up */));
}
@Test
@@ -366,8 +365,7 @@
tracker.disableInterface(TEST_IFACE, NULL_CB);
waitForIdle();
- verify(mFactory).updateInterfaceLinkState(eq(TEST_IFACE), eq(false /* up */),
- eq(NULL_CB));
+ verify(mFactory).updateInterfaceLinkState(eq(TEST_IFACE), eq(false /* up */));
}
@Test