Implementation of Eth Connect & Disconnect Network am: 9b9952ebe1

Original change: https://android-review.googlesource.com/c/platform/frameworks/opt/net/ethernet/+/1955448

Change-Id: I58b045be9e06378cca7c35d996f2cc700e5500db

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

@@ -252,8 +252,13 @@ public class EthernetNetworkFactory extends NetworkFactory {
     }
 
     /** Returns true if state has been modified */
-    boolean updateInterfaceLinkState(String ifaceName, boolean up) {
+    @VisibleForTesting(visibility = VisibleForTesting.Visibility.PACKAGE)
+    protected boolean updateInterfaceLinkState(@NonNull final String ifaceName, final boolean up,
+            @Nullable final IInternalNetworkManagementListener listener) {
         if (!mTrackingInterfaces.containsKey(ifaceName)) {
+            maybeSendNetworkManagementCallback(listener, null,
+                    new InternalNetworkManagementException(
+                            ifaceName + " can't be updated as it is not available."));
             return false;
         }
 
@@ -262,7 +267,7 @@ public class EthernetNetworkFactory extends NetworkFactory {
         }
 
         NetworkInterfaceState iface = mTrackingInterfaces.get(ifaceName);
-        return iface.updateLinkState(up);
+        return iface.updateLinkState(up, listener);
     }
 
     boolean hasInterface(String ifaceName) {
@@ -615,16 +620,27 @@ public class EthernetNetworkFactory extends NetworkFactory {
         }
 
         /** Returns true if state has been modified */
-        boolean updateLinkState(boolean up) {
+        boolean updateLinkState(final boolean up,
-            if (mLinkUp == up) return false;
+                @Nullable final IInternalNetworkManagementListener listener) {
+            if (mLinkUp == up)  {
+                EthernetNetworkFactory.maybeSendNetworkManagementCallback(listener, null,
+                        new InternalNetworkManagementException(
+                                "No changes with requested link state " + up + " for " + name));
+                return false;
+            }
             mLinkUp = up;
 
             if (!up) { // was up, goes down
+                // Save an instance of the current network to use with the callback before stop().
+                final Network network = mNetworkAgent != null ? mNetworkAgent.getNetwork() : null;
+                // Send an abort on a provisioning request callback if necessary before stopping.
                 maybeSendNetworkManagementCallbackForAbort();
                 stop();
+                // If only setting the interface down, send a callback to signal completion.
+                EthernetNetworkFactory.maybeSendNetworkManagementCallback(listener, network, null);
             } else { // was down, goes up
                 stop();
-                start();
+                start(listener);
             }
 
             return true;

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

@@ -243,6 +243,7 @@ public class EthernetServiceImpl extends IEthernetManager.Stub {
             @Nullable final IInternalNetworkManagementListener listener) {
         Log.i(TAG, "connectNetwork called with: iface=" + iface + ", listener=" + listener);
         validateNetworkManagementState(iface, "connectNetwork()");
+        mTracker.connectNetwork(iface, listener);
     }
 
     @Override
@@ -250,5 +251,6 @@ public class EthernetServiceImpl extends IEthernetManager.Stub {
             @Nullable final IInternalNetworkManagementListener listener) {
         Log.i(TAG, "disconnectNetwork called with: iface=" + iface + ", listener=" + listener);
         validateNetworkManagementState(iface, "disconnectNetwork()");
+        mTracker.disconnectNetwork(iface, listener);
     }
 }

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

@@ -190,6 +190,18 @@ public class EthernetTracker {
                 mFactory.updateInterface(iface, ipConfig, capabilities, listener));
     }
 
+    @VisibleForTesting(visibility = PACKAGE)
+    protected void connectNetwork(@NonNull final String iface,
+            @Nullable final IInternalNetworkManagementListener listener) {
+        mHandler.post(() -> updateInterfaceState(iface, true, listener));
+    }
+
+    @VisibleForTesting(visibility = PACKAGE)
+    protected void disconnectNetwork(@NonNull final String iface,
+            @Nullable final IInternalNetworkManagementListener listener) {
+        mHandler.post(() -> updateInterfaceState(iface, false, listener));
+    }
+
     IpConfiguration getIpConfiguration(String iface) {
         return mIpConfigurations.get(iface);
     }
@@ -354,9 +366,14 @@ public class EthernetTracker {
     }
 
     private void updateInterfaceState(String iface, boolean up) {
+        updateInterfaceState(iface, up, null /* listener */);
+    }
+
+    private void updateInterfaceState(@NonNull final String iface, final boolean up,
+            @Nullable final IInternalNetworkManagementListener listener) {
         final int mode = getInterfaceMode(iface);
         final boolean factoryLinkStateUpdated = (mode == INTERFACE_MODE_CLIENT)
-                && mFactory.updateInterfaceLinkState(iface, up);
+                && mFactory.updateInterfaceLinkState(iface, up, listener);
 
         if (factoryLinkStateUpdated) {
             boolean restricted = isRestrictedInterface(iface);

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

@@ -23,7 +23,6 @@ import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyString;
 import static org.mockito.ArgumentMatchers.argThat;
@@ -84,6 +83,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 IInternalNetworkManagementListener NULL_LISTENER = 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";
@@ -238,7 +238,7 @@ public class EthernetNetworkFactoryTest {
         final IpConfiguration ipConfig = createDefaultIpConfig();
         mNetFactory.addInterface(iface, HW_ADDR, ipConfig,
                 createInterfaceCapsBuilder(transportType).build());
-        assertTrue(mNetFactory.updateInterfaceLinkState(iface, true));
+        assertTrue(mNetFactory.updateInterfaceLinkState(iface, true, NULL_LISTENER));
         verifyStart(ipConfig);
         clearInvocations(mDeps);
         clearInvocations(mIpClient);
@@ -309,36 +309,74 @@ public class EthernetNetworkFactoryTest {
     public void testUpdateInterfaceLinkStateForActiveProvisioningInterface() throws Exception {
         initEthernetNetworkFactory();
         createInterfaceUndergoingProvisioning(TEST_IFACE);
+        final TestNetworkManagementListener listener = new TestNetworkManagementListener();
+
         // verify that the IpClient gets shut down when interface state changes to down.
-        assertTrue(mNetFactory.updateInterfaceLinkState(TEST_IFACE, false));
+        final boolean ret =
+                mNetFactory.updateInterfaceLinkState(TEST_IFACE, false /* up */, listener);
+
+        assertTrue(ret);
         verify(mIpClient).shutdown();
+        assertSuccessfulListener(listener, null);
     }
 
     @Test
     public void testUpdateInterfaceLinkStateForProvisionedInterface() throws Exception {
         initEthernetNetworkFactory();
         createAndVerifyProvisionedInterface(TEST_IFACE);
-        assertTrue(mNetFactory.updateInterfaceLinkState(TEST_IFACE, false));
+        final TestNetworkManagementListener listener = new TestNetworkManagementListener();
+
+        final boolean ret =
+                mNetFactory.updateInterfaceLinkState(TEST_IFACE, false /* up */, listener);
+
+        assertTrue(ret);
         verifyStop();
+        assertSuccessfulListener(listener, mMockNetwork);
     }
 
     @Test
     public void testUpdateInterfaceLinkStateForUnprovisionedInterface() throws Exception {
         initEthernetNetworkFactory();
         createUnprovisionedInterface(TEST_IFACE);
-        assertTrue(mNetFactory.updateInterfaceLinkState(TEST_IFACE, false));
+        final TestNetworkManagementListener listener = new TestNetworkManagementListener();
+
+        final boolean ret =
+                mNetFactory.updateInterfaceLinkState(TEST_IFACE, false /* up */, listener);
+
+        assertTrue(ret);
         // There should not be an active IPClient or NetworkAgent.
         verify(mDeps, never()).makeIpClient(any(), any(), any());
         verify(mDeps, never())
                 .makeEthernetNetworkAgent(any(), any(), any(), any(), any(), any(), any());
+        assertSuccessfulListener(listener, null);
     }
 
     @Test
     public void testUpdateInterfaceLinkStateForNonExistingInterface() throws Exception {
         initEthernetNetworkFactory();
+        final TestNetworkManagementListener listener = new TestNetworkManagementListener();
+
         // if interface was never added, link state cannot be updated.
-        assertFalse(mNetFactory.updateInterfaceLinkState("eth1", true));
+        final boolean ret =
-        verify(mDeps, never()).makeIpClient(any(), any(), any());
+                mNetFactory.updateInterfaceLinkState(TEST_IFACE, true /* up */, listener);
+
+        assertFalse(ret);
+        verifyNoStopOrStart();
+        assertFailedListener(listener, "can't be updated as it is not configured");
+    }
+
+    @Test
+    public void testUpdateInterfaceLinkStateWithNoChanges() throws Exception {
+        initEthernetNetworkFactory();
+        createAndVerifyProvisionedInterface(TEST_IFACE);
+        final TestNetworkManagementListener listener = new TestNetworkManagementListener();
+
+        final boolean ret =
+                mNetFactory.updateInterfaceLinkState(TEST_IFACE, true /* up */, listener);
+
+        assertFalse(ret);
+        verifyNoStopOrStart();
+        assertFailedListener(listener, "No changes");
     }
 
     @Test
@@ -394,6 +432,10 @@ public class EthernetNetworkFactoryTest {
 
         // the interface disappeared and getNetworkInterfaceByName returns null, we should not retry
         verify(mIpClient, never()).startProvisioning(any());
+        verifyNoStopOrStart();
+    }
+
+    private void verifyNoStopOrStart() {
         verify(mNetworkAgent, never()).register();
         verify(mIpClient, never()).shutdown();
         verify(mNetworkAgent, never()).unregister();
@@ -404,7 +446,7 @@ public class EthernetNetworkFactoryTest {
     public void testIpClientIsNotStartedWhenLinkIsDown() throws Exception {
         initEthernetNetworkFactory();
         createUnprovisionedInterface(TEST_IFACE);
-        mNetFactory.updateInterfaceLinkState(TEST_IFACE, false);
+        mNetFactory.updateInterfaceLinkState(TEST_IFACE, false, NULL_LISTENER);
 
         mNetFactory.needNetworkFor(createDefaultRequest());
 
@@ -534,7 +576,7 @@ public class EthernetNetworkFactoryTest {
         // verifies onLinkPropertiesChange doesn't complete execution for a downed interface.
         // Posting is necessary as updateInterfaceLinkState with false will set mIpClientCallbacks
         // to null which will throw an NPE in the test if executed synchronously.
-        mHandler.post(() -> mNetFactory.updateInterfaceLinkState(TEST_IFACE, false));
+        mHandler.post(() -> mNetFactory.updateInterfaceLinkState(TEST_IFACE, false, NULL_LISTENER));
         mIpClientCallbacks.onLinkPropertiesChange(lp);
         mLooper.dispatchAll();
 
@@ -552,7 +594,7 @@ public class EthernetNetworkFactoryTest {
         // verifies onReachabilityLost doesn't complete execution for a downed interface.
         // Posting is necessary as updateInterfaceLinkState with false will set mIpClientCallbacks
         // to null which will throw an NPE in the test if executed synchronously.
-        mHandler.post(() -> mNetFactory.updateInterfaceLinkState(TEST_IFACE, false));
+        mHandler.post(() -> mNetFactory.updateInterfaceLinkState(TEST_IFACE, false, NULL_LISTENER));
         mIpClientCallbacks.onReachabilityLost("Neighbor Lost");
         mLooper.dispatchAll();
 
@@ -616,9 +658,7 @@ public class EthernetNetworkFactoryTest {
         mNetFactory.updateInterface(TEST_IFACE, ipConfiguration, capabilities, listener);
         triggerOnProvisioningSuccess();
 
-        final Pair<Network, InternalNetworkManagementException> ret = listener.expectOnComplete();
+        assertSuccessfulListener(listener, mMockNetwork);
-        assertEquals(mMockNetwork, ret.first);
-        assertNull(ret.second);
     }
 
     @DevSdkIgnoreRule.IgnoreUpTo(SC_V2) // TODO: Use to Build.VERSION_CODES.SC_V2 when available
@@ -636,7 +676,7 @@ public class EthernetNetworkFactoryTest {
         initEthernetNetworkFactory();
         verifyNetworkManagementCallIsAbortedWhenInterrupted(
                 TEST_IFACE,
-                () -> mNetFactory.updateInterfaceLinkState(TEST_IFACE, false));
+                () -> mNetFactory.updateInterfaceLinkState(TEST_IFACE, false, NULL_LISTENER));
     }
 
     @DevSdkIgnoreRule.IgnoreUpTo(SC_V2) // TODO: Use to Build.VERSION_CODES.SC_V2 when available
@@ -658,12 +698,28 @@ public class EthernetNetworkFactoryTest {
                     triggerOnProvisioningSuccess();
                 });
 
+        assertSuccessfulListener(successfulListener, mMockNetwork);
+    }
+
+    private void assertSuccessfulListener(
+            @NonNull final TestNetworkManagementListener successfulListener,
+            @NonNull final Network expectedNetwork) throws Exception {
         final Pair<Network, InternalNetworkManagementException> successfulResult =
                 successfulListener.expectOnComplete();
-        assertEquals(mMockNetwork, successfulResult.first);
+        assertEquals(expectedNetwork, successfulResult.first);
         assertNull(successfulResult.second);
     }
 
+    private void assertFailedListener(@NonNull final TestNetworkManagementListener failedListener,
+            @NonNull final String errMsg)
+            throws Exception {
+        final Pair<Network, InternalNetworkManagementException> failedResult =
+                failedListener.expectOnComplete();
+        assertNull(failedResult.first);
+        assertNotNull(failedResult.second);
+        assertTrue(failedResult.second.getMessage().contains(errMsg));
+    }
+
     private void verifyNetworkManagementCallIsAbortedWhenInterrupted(
             @NonNull final String iface,
             @NonNull final Runnable interruptingRunnable) throws Exception {
@@ -676,11 +732,7 @@ public class EthernetNetworkFactoryTest {
         mNetFactory.updateInterface(iface, ipConfiguration, capabilities, failedListener);
         interruptingRunnable.run();
 
-        final Pair<Network, InternalNetworkManagementException> failedResult =
+        assertFailedListener(failedListener, "aborted");
-                failedListener.expectOnComplete();
-        assertNull(failedResult.first);
-        assertNotNull(failedResult.second);
-        assertTrue(failedResult.second.getMessage().contains("aborted"));
     }
 
     @Test

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

@@ -65,6 +65,15 @@ public class EthernetServiceImplTest {
         shouldTrackIface(TEST_IFACE, true);
     }
 
+    private void toggleAutomotiveFeature(final boolean isEnabled) {
+        doReturn(isEnabled)
+                .when(mPackageManager).hasSystemFeature(PackageManager.FEATURE_AUTOMOTIVE);
+    }
+
+    private void shouldTrackIface(@NonNull final String iface, final boolean shouldTrack) {
+        doReturn(shouldTrack).when(mEthernetTracker).isTrackingInterface(iface);
+    }
+
     @Test
     public void testSetConfigurationRejectsWhenEthNotStarted() {
         mEthernetServiceImpl.mStarted.set(false);
@@ -143,11 +152,6 @@ public class EthernetServiceImplTest {
         });
     }
 
-    private void toggleAutomotiveFeature(final boolean isEnabled) {
-        doReturn(isEnabled)
-                .when(mPackageManager).hasSystemFeature(PackageManager.FEATURE_AUTOMOTIVE);
-    }
-
     @Test
     public void testUpdateConfigurationRejectsWithUntrackedIface() {
         shouldTrackIface(TEST_IFACE, false);
@@ -172,10 +176,6 @@ public class EthernetServiceImplTest {
         });
     }
 
-    private void shouldTrackIface(@NonNull final String iface, final boolean shouldTrack) {
-        doReturn(shouldTrack).when(mEthernetTracker).isTrackingInterface(iface);
-    }
-
     @Test
     public void testUpdateConfiguration() {
         mEthernetServiceImpl.updateConfiguration(TEST_IFACE, UPDATE_REQUEST, NULL_LISTENER);
@@ -184,4 +184,16 @@ public class EthernetServiceImplTest {
                 eq(UPDATE_REQUEST.getIpConfig()),
                 eq(UPDATE_REQUEST.getNetworkCapabilities()), eq(NULL_LISTENER));
     }
+
+    @Test
+    public void testConnectNetwork() {
+        mEthernetServiceImpl.connectNetwork(TEST_IFACE, NULL_LISTENER);
+        verify(mEthernetTracker).connectNetwork(eq(TEST_IFACE), eq(NULL_LISTENER));
+    }
+
+    @Test
+    public void testDisconnectNetwork() {
+        mEthernetServiceImpl.disconnectNetwork(TEST_IFACE, NULL_LISTENER);
+        verify(mEthernetTracker).disconnectNetwork(eq(TEST_IFACE), eq(NULL_LISTENER));
+    }
 }

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

@@ -19,9 +19,13 @@ package com.android.server.ethernet;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertThrows;
 import static org.junit.Assert.fail;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyBoolean;
+import static org.mockito.ArgumentMatchers.anyString;
 import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
 
 import android.content.Context;
 import android.content.res.Resources;
@@ -55,8 +59,10 @@ import java.util.ArrayList;
 @SmallTest
 @RunWith(AndroidJUnit4.class)
 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 IInternalNetworkManagementListener NULL_LISTENER = null;
     private EthernetTracker tracker;
     private HandlerThread mHandlerThread;
     @Mock private Context mContext;
@@ -68,6 +74,7 @@ public class EthernetTrackerTest {
     public void setUp() {
         MockitoAnnotations.initMocks(this);
         initMockResources();
+        when(mFactory.updateInterfaceLinkState(anyString(), anyBoolean(), any())).thenReturn(false);
         mHandlerThread = new HandlerThread(THREAD_NAME);
         mHandlerThread.start();
         tracker = new EthernetTracker(mContext, mHandlerThread.getThreadHandler(), mFactory, mNetd);
@@ -294,16 +301,15 @@ public class EthernetTrackerTest {
 
     @Test
     public void testCreateEthernetTrackerConfigReturnsCorrectValue() {
-        final String iface = "1";
         final String capabilities = "2";
         final String ipConfig = "3";
         final String transport = "4";
-        final String configString = String.join(";", iface, capabilities, ipConfig, transport);
+        final String configString = String.join(";", TEST_IFACE, capabilities, ipConfig, transport);
 
         final EthernetTracker.EthernetTrackerConfig config =
                 EthernetTracker.createEthernetTrackerConfig(configString);
 
-        assertEquals(iface, config.mIface);
+        assertEquals(TEST_IFACE, config.mIface);
         assertEquals(capabilities, config.mCapabilities);
         assertEquals(ipConfig, config.mIpConfig);
         assertEquals(transport, config.mTransport);
@@ -317,16 +323,33 @@ public class EthernetTrackerTest {
 
     @Test
     public void testUpdateConfiguration() {
-        final String iface = "testiface";
         final NetworkCapabilities capabilities = new NetworkCapabilities.Builder().build();
         final StaticIpConfiguration staticIpConfig = new StaticIpConfiguration();
         final IInternalNetworkManagementListener listener = null;
 
-        tracker.updateConfiguration(iface, staticIpConfig, capabilities, listener);
+        tracker.updateConfiguration(TEST_IFACE, staticIpConfig, capabilities, listener);
         waitForIdle();
 
         verify(mFactory).updateInterface(
-                eq(iface), eq(EthernetTracker.createIpConfiguration(staticIpConfig)),
+                eq(TEST_IFACE), eq(EthernetTracker.createIpConfiguration(staticIpConfig)),
                 eq(capabilities), eq(listener));
     }
+
+    @Test
+    public void testConnectNetworkCorrectlyCallsFactory() {
+        tracker.connectNetwork(TEST_IFACE, NULL_LISTENER);
+        waitForIdle();
+
+        verify(mFactory).updateInterfaceLinkState(eq(TEST_IFACE), eq(true /* up */),
+                eq(NULL_LISTENER));
+    }
+
+    @Test
+    public void testDisconnectNetworkCorrectlyCallsFactory() {
+        tracker.disconnectNetwork(TEST_IFACE, NULL_LISTENER);
+        waitForIdle();
+
+        verify(mFactory).updateInterfaceLinkState(eq(TEST_IFACE), eq(false /* up */),
+                eq(NULL_LISTENER));
+    }
 }