Changes to make eth service methods more testable

Updates to make methods in EthernetServiceImpl that rely on
EthernetTracker unit testable. This CL also includes added tests for
such methods in EthernetServiceImplTest.

Bug: 210485380
Test: atest EthernetServiceTests
Change-Id: I63969b60cc4cf9d391e2cd21d02e1bdc8988aba8

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

@@ -17,17 +17,24 @@
 package com.android.server.ethernet;
 
 import android.content.Context;
+import android.os.Handler;
+import android.os.HandlerThread;
 import android.util.Log;
 import com.android.server.SystemService;
 
 public final class EthernetService extends SystemService {
 
     private static final String TAG = "EthernetService";
-    final EthernetServiceImpl mImpl;
+    private static final String THREAD_NAME = "EthernetServiceThread";
+    private final EthernetServiceImpl mImpl;
 
     public EthernetService(Context context) {
         super(context);
-        mImpl = new EthernetServiceImpl(context);
+        final HandlerThread handlerThread = new HandlerThread(THREAD_NAME);
+        handlerThread.start();
+        mImpl = new EthernetServiceImpl(
+                    context, handlerThread.getThreadHandler(),
+                    new EthernetTracker(context, handlerThread.getThreadHandler()));
     }
 
     @Override

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

@@ -28,7 +28,6 @@ import android.net.InternalNetworkUpdateRequest;
 import android.net.IpConfiguration;
 import android.os.Binder;
 import android.os.Handler;
-import android.os.HandlerThread;
 import android.os.RemoteException;
 import android.util.Log;
 import android.util.PrintWriterPrinter;
@@ -49,15 +48,17 @@ import java.util.concurrent.atomic.AtomicBoolean;
 public class EthernetServiceImpl extends IEthernetManager.Stub {
     private static final String TAG = "EthernetServiceImpl";
 
-    private final Context mContext;
     @VisibleForTesting
     final AtomicBoolean mStarted = new AtomicBoolean(false);
+    private final Context mContext;
+    private final Handler mHandler;
+    private final EthernetTracker mTracker;
 
-    private Handler mHandler;
+    EthernetServiceImpl(@NonNull final Context context, @NonNull final Handler handler,
-    private EthernetTracker mTracker;
+            @NonNull final EthernetTracker tracker) {
-
-    public EthernetServiceImpl(Context context) {
         mContext = context;
+        mHandler = handler;
+        mTracker = tracker;
     }
 
     private void enforceAccessPermission() {
@@ -91,14 +92,7 @@ public class EthernetServiceImpl extends IEthernetManager.Stub {
 
     public void start() {
         Log.i(TAG, "Starting Ethernet service");
-
-        HandlerThread handlerThread = new HandlerThread("EthernetServiceThread");
-        handlerThread.start();
-        mHandler = new Handler(handlerThread.getLooper());
-
-        mTracker = new EthernetTracker(mContext, mHandler);
         mTracker.start();
-
         mStarted.set(true);
     }
 

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

@@ -18,6 +18,8 @@ package com.android.server.ethernet;
 
 import static android.net.TestNetworkManager.TEST_TAP_PREFIX;
 
+import static com.android.internal.annotations.VisibleForTesting.Visibility.PACKAGE;
+
 import android.annotation.NonNull;
 import android.annotation.Nullable;
 import android.content.Context;
@@ -67,7 +69,8 @@ import java.util.concurrent.ConcurrentHashMap;
  *
  * <p>All public or package private methods must be thread-safe unless stated otherwise.
  */
-final class EthernetTracker {
+@VisibleForTesting(visibility = PACKAGE)
+public class EthernetTracker {
     private static final int INTERFACE_MODE_CLIENT = 1;
     private static final int INTERFACE_MODE_SERVER = 2;
 
@@ -138,10 +141,10 @@ final class EthernetTracker {
 
         NetworkCapabilities nc = createNetworkCapabilities(true /* clear default capabilities */);
         mFactory = new EthernetNetworkFactory(handler, context, nc);
-        mFactory.register();
     }
 
     void start() {
+        mFactory.register();
         mConfigStore.read();
 
         // Default interface is just the first one we want to track.
@@ -176,7 +179,8 @@ final class EthernetTracker {
         return mIpConfigurations.get(iface);
     }
 
-    boolean isTrackingInterface(String iface) {
+    @VisibleForTesting(visibility = PACKAGE)
+    protected boolean isTrackingInterface(String iface) {
         return mFactory.hasInterface(iface);
     }
 

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

@@ -20,11 +20,13 @@ import static org.junit.Assert.assertThrows;
 
 import static org.mockito.Mockito.doReturn;
 
+import android.annotation.NonNull;
 import android.content.Context;
 import android.content.pm.PackageManager;
 import android.net.InternalNetworkUpdateRequest;
 import android.net.IpConfiguration;
 import android.net.StaticIpConfiguration;
+import android.os.Handler;
 
 import androidx.test.filters.SmallTest;
 import androidx.test.runner.AndroidJUnit4;
@@ -38,16 +40,21 @@ import org.mockito.MockitoAnnotations;
 @RunWith(AndroidJUnit4.class)
 @SmallTest
 public class EthernetServiceImplTest {
+    private static final String TEST_IFACE = "test123";
     private EthernetServiceImpl mEthernetServiceImpl;
     @Mock private Context mContext;
+    @Mock private Handler mHandler;
+    @Mock private EthernetTracker mEthernetTracker;
     @Mock private PackageManager mPackageManager;
 
     @Before
     public void setup() {
         MockitoAnnotations.initMocks(this);
         doReturn(mPackageManager).when(mContext).getPackageManager();
-        mEthernetServiceImpl = new EthernetServiceImpl(mContext);
+        mEthernetServiceImpl = new EthernetServiceImpl(mContext, mHandler, mEthernetTracker);
         mEthernetServiceImpl.mStarted.set(true);
+        toggleAutomotiveFeature(true);
+        shouldTrackIface(TEST_IFACE, true);
     }
 
     @Test
@@ -111,7 +118,7 @@ public class EthernetServiceImplTest {
 
     @Test
     public void testUpdateConfigurationRejectsWithoutAutomotiveFeature() {
-        disableAutomotiveFeature();
+        toggleAutomotiveFeature(false);
         assertThrows(UnsupportedOperationException.class, () -> {
             final InternalNetworkUpdateRequest r =
                     new InternalNetworkUpdateRequest(new StaticIpConfiguration(), null);
@@ -122,7 +129,7 @@ public class EthernetServiceImplTest {
 
     @Test
     public void testConnectNetworkRejectsWithoutAutomotiveFeature() {
-        disableAutomotiveFeature();
+        toggleAutomotiveFeature(false);
         assertThrows(UnsupportedOperationException.class, () -> {
             mEthernetServiceImpl.connectNetwork("" /* iface */, null /* listener */);
         });
@@ -130,13 +137,45 @@ public class EthernetServiceImplTest {
 
     @Test
     public void testDisconnectNetworkRejectsWithoutAutomotiveFeature() {
-        disableAutomotiveFeature();
+        toggleAutomotiveFeature(false);
         assertThrows(UnsupportedOperationException.class, () -> {
             mEthernetServiceImpl.disconnectNetwork("" /* iface */, null /* listener */);
         });
     }
 
-    private void disableAutomotiveFeature() {
+    private void toggleAutomotiveFeature(final boolean isEnabled) {
-        doReturn(false).when(mPackageManager).hasSystemFeature(PackageManager.FEATURE_AUTOMOTIVE);
+        doReturn(isEnabled)
+                .when(mPackageManager).hasSystemFeature(PackageManager.FEATURE_AUTOMOTIVE);
+    }
+
+    @Test
+    public void testUpdateConfigurationRejectsWithUntrackedIface() {
+        shouldTrackIface(TEST_IFACE, false);
+        assertThrows(UnsupportedOperationException.class, () -> {
+            final InternalNetworkUpdateRequest r =
+                    new InternalNetworkUpdateRequest(new StaticIpConfiguration(), null);
+
+            mEthernetServiceImpl.updateConfiguration(TEST_IFACE, r, null /* listener */);
+        });
+    }
+
+    @Test
+    public void testConnectNetworkRejectsWithUntrackedIface() {
+        shouldTrackIface(TEST_IFACE, false);
+        assertThrows(UnsupportedOperationException.class, () -> {
+            mEthernetServiceImpl.connectNetwork(TEST_IFACE, null /* listener */);
+        });
+    }
+
+    @Test
+    public void testDisconnectNetworkRejectsWithUntrackedIface() {
+        shouldTrackIface(TEST_IFACE, false);
+        assertThrows(UnsupportedOperationException.class, () -> {
+            mEthernetServiceImpl.disconnectNetwork(TEST_IFACE, null /* listener */);
+        });
+    }
+
+    private void shouldTrackIface(@NonNull final String iface, final boolean shouldTrack) {
+        doReturn(shouldTrack).when(mEthernetTracker).isTrackingInterface(iface);
     }
 }