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

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

@@ -33,6 +33,7 @@ import android.net.NetworkRequest;
 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 @@ public class EthernetNetworkFactory extends NetworkFactory {
             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 @@ public class EthernetNetworkFactory extends NetworkFactory {
         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 @@ public class EthernetNetworkFactory extends NetworkFactory {
 
             @Override
             public void onIpClientCreated(IIpClient ipClient) {
-                mIpClient = ipClient;
+                mIpClient = mDeps.makeIpClientManager(ipClient);
                 mIpClientStartCv.open();
             }
 
@@ -356,14 +361,6 @@ public class EthernetNetworkFactory extends NetworkFactory {
             }
         }
 
-        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 @@ public class EthernetNetworkFactory extends NetworkFactory {
         void stop() {
             // Invalidate all previous start requests
             if (mIpClient != null) {
-                shutdownIpClient(mIpClient);
+                mIpClient.shutdown();
                 mIpClientCallback.awaitIpClientShutdown();
                 mIpClient = null;
             }
@@ -522,41 +519,30 @@ public class EthernetNetworkFactory extends NetworkFactory {
             mLinkProperties.clear();
         }
 
-        private static void provisionIpClient(IIpClient ipClient, IpConfiguration config,
+        private static void provisionIpClient(@NonNull final IpClientManager ipClient,
-                String tcpBufferSizes) {
+                @NonNull final IpConfiguration config, @NonNull final String tcpBufferSizes) {
             if (config.getProxySettings() == ProxySettings.STATIC ||
                     config.getProxySettings() == ProxySettings.PAC) {
-                try {
+                ipClient.setHttpProxy(config.getHttpProxy());
-                    ipClient.setHttpProxy(config.getHttpProxy());
-                } catch (RemoteException e) {
-                    e.rethrowFromSystemServer();
-                }
             }
 
             if (!TextUtils.isEmpty(tcpBufferSizes)) {
-                try {
+                ipClient.setTcpBufferSizes(tcpBufferSizes);
-                    ipClient.setTcpBufferSizes(tcpBufferSizes);
-                } catch (RemoteException e) {
-                    e.rethrowFromSystemServer();
-                }
             }
 
-            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 @@ public class EthernetNetworkFactory extends NetworkFactory {
             NetworkInterfaceState ifaceState = mTrackingInterfaces.get(iface);
             pw.println(iface + ":" + ifaceState);
             pw.increaseIndent();
-            final IIpClient ipClient = ifaceState.mIpClient;
+            if (null == ifaceState.mIpClient) {
-            if (ipClient != null) {
-                IpClientUtil.dumpIpClient(ipClient, fd, pw, args);
-            } else {
                 pw.println("IpClient is null");
             }
             pw.decreaseIndent();

tests/ethernet/java/com/android/server/ethernet/EthernetNetworkFactoryTest.java

@@ -42,8 +42,8 @@ import android.net.NetworkAgentConfig;
 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 @@ public class EthernetNetworkFactoryTest {
     @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 @@ public class EthernetNetworkFactoryTest {
             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 @@ public class EthernetNetworkFactoryTest {
             mIpClientCallbacks = null;
             return null;
         }).when(mIpClient).shutdown();
+
+        when(mDeps.makeIpClientManager(any())).thenReturn(mIpClient);
     }
 
     private void triggerOnProvisioningSuccess() {
@@ -185,13 +187,13 @@ public class EthernetNetworkFactoryTest {
 
     // 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 @@ public class EthernetNetworkFactoryTest {
         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();
     }