ethernet: add EthernetCallback class to wrap OutcomeReceiver

The OutcomeReceiver that gets passed into the EthernetServiceImpl is
hard to use (it is nullable and both onError and onResult can throw
RemoteExceptions).

Note: This is an intermediary state.
The next step will be to completely tear EthernetCallback out of
EthernetNetworkFactory, which will remove a lot of boilerplate code.

Bug: 225317892
Test: atest EthernetManagerTest
Change-Id: Ifc0e1ba29ded933c418e4b335cb731c3496d7e44
This commit is contained in:
Patrick Rohr
2022-08-19 14:35:52 -07:00
parent 0d3c8b692d
commit 251f2fcf13
7 changed files with 157 additions and 111 deletions

View File

@@ -40,7 +40,6 @@ 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 +54,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;
@@ -84,7 +82,7 @@ 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 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";
@@ -241,7 +239,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, NULL_CB));
ArgumentCaptor<NetworkOfferCallback> captor = ArgumentCaptor.forClass(
NetworkOfferCallback.class);
@@ -295,7 +293,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, NULL_CB));
clearInvocations(mIpClient);
clearInvocations(mNetworkAgent);
@@ -572,17 +570,21 @@ public class EthernetNetworkFactoryTest {
}
private static final class TestNetworkManagementListener
implements INetworkInterfaceOutcomeReceiver {
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(@NonNull EthernetNetworkManagementException exception) {
mResult.completeExceptionally(exception);
public void onError(String msg) {
mResult.completeExceptionally(new EthernetNetworkManagementException(msg));
}
String expectOnResult() throws Exception {
@@ -598,11 +600,6 @@ public class EthernetNetworkFactoryTest {
}
});
}
@Override
public IBinder asBinder() {
return null;
}
}
@Test
@@ -632,7 +629,7 @@ public class EthernetNetworkFactoryTest {
initEthernetNetworkFactory();
verifyNetworkManagementCallIsAbortedWhenInterrupted(
TEST_IFACE,
() -> mNetFactory.updateInterfaceLinkState(TEST_IFACE, false, NULL_LISTENER));
() -> mNetFactory.updateInterfaceLinkState(TEST_IFACE, false, NULL_CB));
}
@Test
@@ -718,7 +715,7 @@ public class EthernetNetworkFactoryTest {
final IpConfiguration initialIpConfig = createStaticIpConfig();
mNetFactory.updateInterface(TEST_IFACE, initialIpConfig, null /*capabilities*/,
null /*listener*/);
new EthernetCallback(null /* cb */));
triggerOnProvisioningSuccess();
verifyRestart(initialIpConfig);
@@ -730,7 +727,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*/);
new EthernetCallback(null /* cb */));
triggerOnProvisioningSuccess();
verifyRestart(initialIpConfig);
}
@@ -739,7 +736,7 @@ public class EthernetNetworkFactoryTest {
public void testOnNetworkNeededOnStaleNetworkOffer() throws Exception {
initEthernetNetworkFactory();
createAndVerifyProvisionedInterface(TEST_IFACE);
mNetFactory.updateInterfaceLinkState(TEST_IFACE, false, null);
mNetFactory.updateInterfaceLinkState(TEST_IFACE, false, new EthernetCallback(null));
verify(mNetworkProvider).unregisterNetworkOffer(mNetworkOfferCallback);
// It is possible that even after a network offer is unregistered, CS still sends it
// onNetworkNeeded() callbacks.

View File

@@ -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) {

View File

@@ -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;
@@ -344,7 +343,7 @@ 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();
@@ -355,20 +354,20 @@ public class EthernetTrackerTest {
@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));
eq(NULL_CB));
}
@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));
eq(NULL_CB));
}
@Test