From cfa7a08bcf9acf80db19518d4bd6e5ef435a2ca1 Mon Sep 17 00:00:00 2001 From: James Mattis Date: Sun, 2 Jan 2022 19:53:39 -0800 Subject: [PATCH] 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 --- .../ethernet/EthernetNetworkFactory.java | 59 +++++++------------ .../ethernet/EthernetNetworkFactoryTest.java | 16 ++--- 2 files changed, 30 insertions(+), 45 deletions(-) diff --git a/service-t/src/com/android/server/ethernet/EthernetNetworkFactory.java b/service-t/src/com/android/server/ethernet/EthernetNetworkFactory.java index 53928f7519..d7800c027a 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.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, - 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 @@ public class EthernetNetworkFactory extends NetworkFactory { 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 08a70a6f40..990ee5fee7 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.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(); }