LineageOS/android_packages_modules_Connectivity
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

ethernet: remove callback from factory updateInterfaceLinkState

Browse Source 
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
This commit is contained in:
Patrick Rohr
2022-08-22 09:39:25 -07:00
parent ea5c6dd136
commit 750036b286
4 changed files with 23 additions and 79 deletions
Download Patch File Download Diff File Expand all files Collapse all files

10
service-t/src/com/android/server/ethernet/EthernetNetworkFactory.java
View File

@@ -232,10 +232,8 @@ public class EthernetNetworkFactory {
/** Returns true if state has been modified */ /** Returns true if state has been modified */
@VisibleForTesting(visibility = VisibleForTesting.Visibility.PACKAGE) @VisibleForTesting(visibility = VisibleForTesting.Visibility.PACKAGE)
protected boolean updateInterfaceLinkState(@NonNull final String ifaceName, final boolean up, protected boolean updateInterfaceLinkState(@NonNull final String ifaceName, final boolean up) {
final EthernetCallback cb) {
if (!hasInterface(ifaceName)) { if (!hasInterface(ifaceName)) {
cb.onError(ifaceName + " is not tracked by the ethernet service");
return false; return false;
} }
@@ -244,7 +242,7 @@ public class EthernetNetworkFactory {
} }
NetworkInterfaceState iface = mTrackingInterfaces.get(ifaceName); NetworkInterfaceState iface = mTrackingInterfaces.get(ifaceName);
return iface.updateLinkState(up, cb); return iface.updateLinkState(up);
} }
@VisibleForTesting @VisibleForTesting
@@ -570,9 +568,8 @@ public class EthernetNetworkFactory {
} }
/** Returns true if state has been modified */ /** Returns true if state has been modified */
boolean updateLinkState(final boolean up, final EthernetCallback cb) { boolean updateLinkState(final boolean up) {
if (mLinkUp == up) { if (mLinkUp == up) {
cb.onError("No changes with requested link state " + up + " for " + name);
return false; return false;
} }
mLinkUp = up; mLinkUp = up;
@@ -585,7 +582,6 @@ public class EthernetNetworkFactory {
registerNetworkOffer(); registerNetworkOffer();
} }
cb.onResult(name);
return true; return true;
} }

9
service-t/src/com/android/server/ethernet/EthernetTracker.java
View File

@@ -629,10 +629,17 @@ public class EthernetTracker {
@Nullable final EthernetCallback cb) { @Nullable final EthernetCallback cb) {
final int mode = getInterfaceMode(iface); final int mode = getInterfaceMode(iface);
final boolean factoryLinkStateUpdated = (mode == INTERFACE_MODE_CLIENT) final boolean factoryLinkStateUpdated = (mode == INTERFACE_MODE_CLIENT)
&& mFactory.updateInterfaceLinkState(iface, up, cb); && mFactory.updateInterfaceLinkState(iface, up);
if (factoryLinkStateUpdated) { if (factoryLinkStateUpdated) {
broadcastInterfaceStateChange(iface); 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);
} }
} }

75
tests/unit/java/com/android/server/ethernet/EthernetNetworkFactoryTest.java
View File

@@ -20,7 +20,6 @@ import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotSame; import static org.junit.Assert.assertNotSame;
import static org.junit.Assert.assertNull; import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertThrows;
import static org.junit.Assert.assertTrue; import static org.junit.Assert.assertTrue;
import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.anyString;
@@ -38,7 +37,6 @@ import android.app.test.MockAnswerUtil.AnswerWithArguments;
import android.content.Context; import android.content.Context;
import android.content.res.Resources; import android.content.res.Resources;
import android.net.ConnectivityManager; import android.net.ConnectivityManager;
import android.net.EthernetNetworkManagementException;
import android.net.EthernetNetworkSpecifier; import android.net.EthernetNetworkSpecifier;
import android.net.IpConfiguration; import android.net.IpConfiguration;
import android.net.LinkAddress; import android.net.LinkAddress;
@@ -72,9 +70,6 @@ import org.mockito.Mock;
import org.mockito.MockitoAnnotations; import org.mockito.MockitoAnnotations;
import java.util.Objects; import java.util.Objects;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeUnit;
@SmallTest @SmallTest
@RunWith(DevSdkIgnoreRunner.class) @RunWith(DevSdkIgnoreRunner.class)
@@ -82,7 +77,6 @@ import java.util.concurrent.TimeUnit;
public class EthernetNetworkFactoryTest { public class EthernetNetworkFactoryTest {
private static final int TIMEOUT_MS = 2_000; private static final int TIMEOUT_MS = 2_000;
private static final String TEST_IFACE = "test123"; 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 String IP_ADDR = "192.0.2.2/25";
private static final LinkAddress LINK_ADDR = new LinkAddress(IP_ADDR); private static final LinkAddress LINK_ADDR = new LinkAddress(IP_ADDR);
private static final String HW_ADDR = "01:02:03:04:05:06"; private static final String HW_ADDR = "01:02:03:04:05:06";
@@ -239,7 +233,7 @@ public class EthernetNetworkFactoryTest {
final IpConfiguration ipConfig = createDefaultIpConfig(); final IpConfiguration ipConfig = createDefaultIpConfig();
mNetFactory.addInterface(iface, HW_ADDR, ipConfig, mNetFactory.addInterface(iface, HW_ADDR, ipConfig,
createInterfaceCapsBuilder(transportType).build()); createInterfaceCapsBuilder(transportType).build());
assertTrue(mNetFactory.updateInterfaceLinkState(iface, true, NULL_CB)); assertTrue(mNetFactory.updateInterfaceLinkState(iface, true));
ArgumentCaptor<NetworkOfferCallback> captor = ArgumentCaptor.forClass( ArgumentCaptor<NetworkOfferCallback> captor = ArgumentCaptor.forClass(
NetworkOfferCallback.class); NetworkOfferCallback.class);
@@ -293,7 +287,7 @@ public class EthernetNetworkFactoryTest {
// then calling onNetworkUnwanted. // then calling onNetworkUnwanted.
mNetFactory.addInterface(iface, HW_ADDR, createDefaultIpConfig(), mNetFactory.addInterface(iface, HW_ADDR, createDefaultIpConfig(),
createInterfaceCapsBuilder(NetworkCapabilities.TRANSPORT_ETHERNET).build()); createInterfaceCapsBuilder(NetworkCapabilities.TRANSPORT_ETHERNET).build());
assertTrue(mNetFactory.updateInterfaceLinkState(iface, true, NULL_CB)); assertTrue(mNetFactory.updateInterfaceLinkState(iface, true));
clearInvocations(mIpClient); clearInvocations(mIpClient);
clearInvocations(mNetworkAgent); clearInvocations(mNetworkAgent);
@@ -303,81 +297,63 @@ public class EthernetNetworkFactoryTest {
public void testUpdateInterfaceLinkStateForActiveProvisioningInterface() throws Exception { public void testUpdateInterfaceLinkStateForActiveProvisioningInterface() throws Exception {
initEthernetNetworkFactory(); initEthernetNetworkFactory();
createInterfaceUndergoingProvisioning(TEST_IFACE); createInterfaceUndergoingProvisioning(TEST_IFACE);
final TestNetworkManagementListener listener = new TestNetworkManagementListener();
// verify that the IpClient gets shut down when interface state changes to down. // verify that the IpClient gets shut down when interface state changes to down.
final boolean ret = final boolean ret = mNetFactory.updateInterfaceLinkState(TEST_IFACE, false /* up */);
mNetFactory.updateInterfaceLinkState(TEST_IFACE, false /* up */, listener);
assertTrue(ret); assertTrue(ret);
verify(mIpClient).shutdown(); verify(mIpClient).shutdown();
assertEquals(TEST_IFACE, listener.expectOnResult());
} }
@Test @Test
public void testUpdateInterfaceLinkStateForProvisionedInterface() throws Exception { public void testUpdateInterfaceLinkStateForProvisionedInterface() throws Exception {
initEthernetNetworkFactory(); initEthernetNetworkFactory();
createAndVerifyProvisionedInterface(TEST_IFACE); createAndVerifyProvisionedInterface(TEST_IFACE);
final TestNetworkManagementListener listenerDown = new TestNetworkManagementListener();
final TestNetworkManagementListener listenerUp = new TestNetworkManagementListener();
final boolean retDown = final boolean retDown = mNetFactory.updateInterfaceLinkState(TEST_IFACE, false /* up */);
mNetFactory.updateInterfaceLinkState(TEST_IFACE, false /* up */, listenerDown);
assertTrue(retDown); assertTrue(retDown);
verifyStop(); verifyStop();
assertEquals(TEST_IFACE, listenerDown.expectOnResult());
final boolean retUp = final boolean retUp = mNetFactory.updateInterfaceLinkState(TEST_IFACE, true /* up */);
mNetFactory.updateInterfaceLinkState(TEST_IFACE, true /* up */, listenerUp);
assertTrue(retUp); assertTrue(retUp);
assertEquals(TEST_IFACE, listenerUp.expectOnResult());
} }
@Test @Test
public void testUpdateInterfaceLinkStateForUnprovisionedInterface() throws Exception { public void testUpdateInterfaceLinkStateForUnprovisionedInterface() throws Exception {
initEthernetNetworkFactory(); initEthernetNetworkFactory();
createUnprovisionedInterface(TEST_IFACE); createUnprovisionedInterface(TEST_IFACE);
final TestNetworkManagementListener listener = new TestNetworkManagementListener();
final boolean ret = final boolean ret = mNetFactory.updateInterfaceLinkState(TEST_IFACE, false /* up */);
mNetFactory.updateInterfaceLinkState(TEST_IFACE, false /* up */, listener);
assertTrue(ret); assertTrue(ret);
// There should not be an active IPClient or NetworkAgent. // There should not be an active IPClient or NetworkAgent.
verify(mDeps, never()).makeIpClient(any(), any(), any()); verify(mDeps, never()).makeIpClient(any(), any(), any());
verify(mDeps, never()) verify(mDeps, never())
.makeEthernetNetworkAgent(any(), any(), any(), any(), any(), any(), any()); .makeEthernetNetworkAgent(any(), any(), any(), any(), any(), any(), any());
assertEquals(TEST_IFACE, listener.expectOnResult());
} }
@Test @Test
public void testUpdateInterfaceLinkStateForNonExistingInterface() throws Exception { public void testUpdateInterfaceLinkStateForNonExistingInterface() throws Exception {
initEthernetNetworkFactory(); initEthernetNetworkFactory();
final TestNetworkManagementListener listener = new TestNetworkManagementListener();
// if interface was never added, link state cannot be updated. // if interface was never added, link state cannot be updated.
final boolean ret = final boolean ret = mNetFactory.updateInterfaceLinkState(TEST_IFACE, true /* up */);
mNetFactory.updateInterfaceLinkState(TEST_IFACE, true /* up */, listener);
assertFalse(ret); assertFalse(ret);
verifyNoStopOrStart(); verifyNoStopOrStart();
listener.expectOnError();
} }
@Test @Test
public void testUpdateInterfaceLinkStateWithNoChanges() throws Exception { public void testUpdateInterfaceLinkStateWithNoChanges() throws Exception {
initEthernetNetworkFactory(); initEthernetNetworkFactory();
createAndVerifyProvisionedInterface(TEST_IFACE); createAndVerifyProvisionedInterface(TEST_IFACE);
final TestNetworkManagementListener listener = new TestNetworkManagementListener();
final boolean ret = final boolean ret = mNetFactory.updateInterfaceLinkState(TEST_IFACE, true /* up */);
mNetFactory.updateInterfaceLinkState(TEST_IFACE, true /* up */, listener);
assertFalse(ret); assertFalse(ret);
verifyNoStopOrStart(); verifyNoStopOrStart();
listener.expectOnError();
} }
@Test @Test
@@ -569,39 +545,6 @@ public class EthernetNetworkFactoryTest {
verify(mNetworkAgent).markConnected(); 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 @Test
public void testUpdateInterfaceRestartsAgentCorrectly() throws Exception { public void testUpdateInterfaceRestartsAgentCorrectly() throws Exception {
initEthernetNetworkFactory(); initEthernetNetworkFactory();
@@ -656,7 +599,7 @@ public class EthernetNetworkFactoryTest {
public void testOnNetworkNeededOnStaleNetworkOffer() throws Exception { public void testOnNetworkNeededOnStaleNetworkOffer() throws Exception {
initEthernetNetworkFactory(); initEthernetNetworkFactory();
createAndVerifyProvisionedInterface(TEST_IFACE); createAndVerifyProvisionedInterface(TEST_IFACE);
mNetFactory.updateInterfaceLinkState(TEST_IFACE, false, new EthernetCallback(null)); mNetFactory.updateInterfaceLinkState(TEST_IFACE, false);
verify(mNetworkProvider).unregisterNetworkOffer(mNetworkOfferCallback); verify(mNetworkProvider).unregisterNetworkOffer(mNetworkOfferCallback);
// It is possible that even after a network offer is unregistered, CS still sends it // It is possible that even after a network offer is unregistered, CS still sends it
// onNetworkNeeded() callbacks. // onNetworkNeeded() callbacks.

8
tests/unit/java/com/android/server/ethernet/EthernetTrackerTest.java
View File

@@ -87,7 +87,7 @@ public class EthernetTrackerTest {
public void setUp() throws RemoteException { public void setUp() throws RemoteException {
MockitoAnnotations.initMocks(this); MockitoAnnotations.initMocks(this);
initMockResources(); initMockResources();
when(mFactory.updateInterfaceLinkState(anyString(), anyBoolean(), any())).thenReturn(false); when(mFactory.updateInterfaceLinkState(anyString(), anyBoolean())).thenReturn(false);
when(mNetd.interfaceGetList()).thenReturn(new String[0]); when(mNetd.interfaceGetList()).thenReturn(new String[0]);
mHandlerThread = new HandlerThread(THREAD_NAME); mHandlerThread = new HandlerThread(THREAD_NAME);
mHandlerThread.start(); mHandlerThread.start();
@@ -357,8 +357,7 @@ public class EthernetTrackerTest {
tracker.enableInterface(TEST_IFACE, NULL_CB); tracker.enableInterface(TEST_IFACE, NULL_CB);
waitForIdle(); waitForIdle();
verify(mFactory).updateInterfaceLinkState(eq(TEST_IFACE), eq(true /* up */), verify(mFactory).updateInterfaceLinkState(eq(TEST_IFACE), eq(true /* up */));
eq(NULL_CB));
} }
@Test @Test
@@ -366,8 +365,7 @@ public class EthernetTrackerTest {
tracker.disableInterface(TEST_IFACE, NULL_CB); tracker.disableInterface(TEST_IFACE, NULL_CB);
waitForIdle(); waitForIdle();
verify(mFactory).updateInterfaceLinkState(eq(TEST_IFACE), eq(false /* up */), verify(mFactory).updateInterfaceLinkState(eq(TEST_IFACE), eq(false /* up */));
eq(NULL_CB));
} }
@Test @Test