Replacing IIpClient with Manager in ethNetFactory
Replacing IIpClient with IpClientManager to reduce code duplication,
increase readability and maintainability as well as making
EthernetNetworkFactory easier to unit test.
Bug: 210485380
Test: atest EthernetServiceTests
Change-Id: I283653171c0cc47ad94a67d6dbd65b924cdf1ada
diff --git a/service-t/src/com/android/server/ethernet/EthernetNetworkFactory.java b/service-t/src/com/android/server/ethernet/EthernetNetworkFactory.java
index 53928f7..d7800c0 100644
--- a/service-t/src/com/android/server/ethernet/EthernetNetworkFactory.java
+++ b/service-t/src/com/android/server/ethernet/EthernetNetworkFactory.java
@@ -33,6 +33,7 @@
import android.net.NetworkSpecifier;
import android.net.ip.IIpClient;
import android.net.ip.IpClientCallbacks;
+import android.net.ip.IpClientManager;
import android.net.ip.IpClientUtil;
import android.net.shared.ProvisioningConfiguration;
import android.os.ConditionVariable;
@@ -76,6 +77,10 @@
IpClientUtil.makeIpClient(context, iface, callbacks);
}
+ public IpClientManager makeIpClientManager(@NonNull final IIpClient ipClient) {
+ return new IpClientManager(ipClient, TAG);
+ }
+
public EthernetNetworkAgent makeEthernetNetworkAgent(Context context, Looper looper,
NetworkCapabilities nc, LinkProperties lp, NetworkAgentConfig config,
NetworkProvider provider, EthernetNetworkAgent.Callbacks cb) {
@@ -283,7 +288,7 @@
private boolean mLinkUp;
private LinkProperties mLinkProperties = new LinkProperties();
- private volatile @Nullable IIpClient mIpClient;
+ private volatile @Nullable IpClientManager mIpClient;
private @Nullable IpClientCallbacksImpl mIpClientCallback;
private @Nullable EthernetNetworkAgent mNetworkAgent;
private @Nullable IpConfiguration mIpConfig;
@@ -317,7 +322,7 @@
@Override
public void onIpClientCreated(IIpClient ipClient) {
- mIpClient = ipClient;
+ mIpClient = mDeps.makeIpClientManager(ipClient);
mIpClientStartCv.open();
}
@@ -356,14 +361,6 @@
}
}
- private static void shutdownIpClient(IIpClient ipClient) {
- try {
- ipClient.shutdown();
- } catch (RemoteException e) {
- Log.e(TAG, "Error stopping IpClient", e);
- }
- }
-
NetworkInterfaceState(String ifaceName, String hwAddress, Handler handler, Context context,
@NonNull NetworkCapabilities capabilities, NetworkFactory networkFactory,
Dependencies deps) {
@@ -509,7 +506,7 @@
void stop() {
// Invalidate all previous start requests
if (mIpClient != null) {
- shutdownIpClient(mIpClient);
+ mIpClient.shutdown();
mIpClientCallback.awaitIpClientShutdown();
mIpClient = null;
}
@@ -522,41 +519,30 @@
mLinkProperties.clear();
}
- private static void provisionIpClient(IIpClient ipClient, IpConfiguration config,
- String tcpBufferSizes) {
+ private static void provisionIpClient(@NonNull final IpClientManager ipClient,
+ @NonNull final IpConfiguration config, @NonNull final String tcpBufferSizes) {
if (config.getProxySettings() == ProxySettings.STATIC ||
config.getProxySettings() == ProxySettings.PAC) {
- try {
- ipClient.setHttpProxy(config.getHttpProxy());
- } catch (RemoteException e) {
- e.rethrowFromSystemServer();
- }
+ ipClient.setHttpProxy(config.getHttpProxy());
}
if (!TextUtils.isEmpty(tcpBufferSizes)) {
- try {
- ipClient.setTcpBufferSizes(tcpBufferSizes);
- } catch (RemoteException e) {
- e.rethrowFromSystemServer();
- }
+ ipClient.setTcpBufferSizes(tcpBufferSizes);
}
- final ProvisioningConfiguration provisioningConfiguration;
+ ipClient.startProvisioning(createProvisioningConfiguration(config));
+ }
+
+ private static ProvisioningConfiguration createProvisioningConfiguration(
+ @NonNull final IpConfiguration config) {
if (config.getIpAssignment() == IpAssignment.STATIC) {
- provisioningConfiguration = new ProvisioningConfiguration.Builder()
+ return new ProvisioningConfiguration.Builder()
.withStaticConfiguration(config.getStaticIpConfiguration())
.build();
- } else {
- provisioningConfiguration = new ProvisioningConfiguration.Builder()
+ }
+ return new ProvisioningConfiguration.Builder()
.withProvisioningTimeoutMs(0)
.build();
- }
-
- try {
- ipClient.startProvisioning(provisioningConfiguration.toStableParcelable());
- } catch (RemoteException e) {
- e.rethrowFromSystemServer();
- }
}
void restart(){
@@ -589,10 +575,7 @@
NetworkInterfaceState ifaceState = mTrackingInterfaces.get(iface);
pw.println(iface + ":" + ifaceState);
pw.increaseIndent();
- final IIpClient ipClient = ifaceState.mIpClient;
- if (ipClient != null) {
- IpClientUtil.dumpIpClient(ipClient, fd, pw, args);
- } else {
+ if (null == ifaceState.mIpClient) {
pw.println("IpClient is null");
}
pw.decreaseIndent();
diff --git a/tests/ethernet/java/com/android/server/ethernet/EthernetNetworkFactoryTest.java b/tests/ethernet/java/com/android/server/ethernet/EthernetNetworkFactoryTest.java
index 08a70a6..990ee5f 100644
--- a/tests/ethernet/java/com/android/server/ethernet/EthernetNetworkFactoryTest.java
+++ b/tests/ethernet/java/com/android/server/ethernet/EthernetNetworkFactoryTest.java
@@ -42,8 +42,8 @@
import android.net.NetworkCapabilities;
import android.net.NetworkProvider;
import android.net.NetworkRequest;
-import android.net.ip.IIpClient;
import android.net.ip.IpClientCallbacks;
+import android.net.ip.IpClientManager;
import android.os.Handler;
import android.os.Looper;
import android.os.test.TestLooper;
@@ -72,7 +72,7 @@
@Mock private Context mContext;
@Mock private Resources mResources;
@Mock private EthernetNetworkFactory.Dependencies mDeps;
- @Mock private IIpClient mIpClient;
+ @Mock private IpClientManager mIpClient;
@Mock private EthernetNetworkAgent mNetworkAgent;
@Mock private InterfaceParams mInterfaceParams;
@@ -113,7 +113,7 @@
assertNull("An IpClient has already been created.", mIpClientCallbacks);
mIpClientCallbacks = inv.getArgument(2);
- mIpClientCallbacks.onIpClientCreated(mIpClient);
+ mIpClientCallbacks.onIpClientCreated(null);
mLooper.dispatchAll();
return null;
}).when(mDeps).makeIpClient(any(Context.class), anyString(), any());
@@ -124,6 +124,8 @@
mIpClientCallbacks = null;
return null;
}).when(mIpClient).shutdown();
+
+ when(mDeps.makeIpClientManager(any())).thenReturn(mIpClient);
}
private void triggerOnProvisioningSuccess() {
@@ -185,13 +187,13 @@
// creates an interface with provisioning in progress (since updating the interface link state
// automatically starts the provisioning process)
- private void createInterfaceUndergoingProvisioning(String iface) throws Exception {
+ private void createInterfaceUndergoingProvisioning(String iface) {
// Default to the ethernet transport type.
createInterfaceUndergoingProvisioning(iface, NetworkCapabilities.TRANSPORT_ETHERNET);
}
private void createInterfaceUndergoingProvisioning(
- @NonNull final String iface, final int transportType) throws Exception {
+ @NonNull final String iface, final int transportType) {
mNetFactory.addInterface(iface, iface, createInterfaceCapsBuilder(transportType).build(),
createDefaultIpConfig());
assertTrue(mNetFactory.updateInterfaceLinkState(iface, true));
@@ -433,12 +435,12 @@
verifyStart();
}
- private void verifyStart() throws Exception {
+ private void verifyStart() {
verify(mDeps).makeIpClient(any(Context.class), anyString(), any());
verify(mIpClient).startProvisioning(any());
}
- private void verifyStop() throws Exception {
+ private void verifyStop() {
verify(mIpClient).shutdown();
verify(mNetworkAgent).unregister();
}