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
diff --git a/service-t/src/com/android/server/ethernet/EthernetService.java b/service-t/src/com/android/server/ethernet/EthernetService.java
index 2448146..467ab67 100644
--- a/service-t/src/com/android/server/ethernet/EthernetService.java
+++ b/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
diff --git a/service-t/src/com/android/server/ethernet/EthernetServiceImpl.java b/service-t/src/com/android/server/ethernet/EthernetServiceImpl.java
index fd690b5..3479ea4 100644
--- a/service-t/src/com/android/server/ethernet/EthernetServiceImpl.java
+++ b/service-t/src/com/android/server/ethernet/EthernetServiceImpl.java
@@ -28,7 +28,6 @@
 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 @@
 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;
-    private EthernetTracker mTracker;
-
-    public EthernetServiceImpl(Context context) {
+    EthernetServiceImpl(@NonNull final Context context, @NonNull final Handler handler,
+            @NonNull final EthernetTracker tracker) {
         mContext = context;
+        mHandler = handler;
+        mTracker = tracker;
     }
 
     private void enforceAccessPermission() {
@@ -91,14 +92,7 @@
 
     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);
     }
 
diff --git a/service-t/src/com/android/server/ethernet/EthernetTracker.java b/service-t/src/com/android/server/ethernet/EthernetTracker.java
index 65c3cef..9660194 100644
--- a/service-t/src/com/android/server/ethernet/EthernetTracker.java
+++ b/service-t/src/com/android/server/ethernet/EthernetTracker.java
@@ -18,6 +18,8 @@
 
 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 @@
  *
  * <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 @@
 
         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 @@
         return mIpConfigurations.get(iface);
     }
 
-    boolean isTrackingInterface(String iface) {
+    @VisibleForTesting(visibility = PACKAGE)
+    protected boolean isTrackingInterface(String iface) {
         return mFactory.hasInterface(iface);
     }
 
diff --git a/tests/ethernet/java/com/android/server/ethernet/EthernetServiceImplTest.java b/tests/ethernet/java/com/android/server/ethernet/EthernetServiceImplTest.java
index 9869b82..516e574 100644
--- a/tests/ethernet/java/com/android/server/ethernet/EthernetServiceImplTest.java
+++ b/tests/ethernet/java/com/android/server/ethernet/EthernetServiceImplTest.java
@@ -20,11 +20,13 @@
 
 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 @@
 @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 @@
 
     @Test
     public void testUpdateConfigurationRejectsWithoutAutomotiveFeature() {
-        disableAutomotiveFeature();
+        toggleAutomotiveFeature(false);
         assertThrows(UnsupportedOperationException.class, () -> {
             final InternalNetworkUpdateRequest r =
                     new InternalNetworkUpdateRequest(new StaticIpConfiguration(), null);
@@ -122,7 +129,7 @@
 
     @Test
     public void testConnectNetworkRejectsWithoutAutomotiveFeature() {
-        disableAutomotiveFeature();
+        toggleAutomotiveFeature(false);
         assertThrows(UnsupportedOperationException.class, () -> {
             mEthernetServiceImpl.connectNetwork("" /* iface */, null /* listener */);
         });
@@ -130,13 +137,45 @@
 
     @Test
     public void testDisconnectNetworkRejectsWithoutAutomotiveFeature() {
-        disableAutomotiveFeature();
+        toggleAutomotiveFeature(false);
         assertThrows(UnsupportedOperationException.class, () -> {
             mEthernetServiceImpl.disconnectNetwork("" /* iface */, null /* listener */);
         });
     }
 
-    private void disableAutomotiveFeature() {
-        doReturn(false).when(mPackageManager).hasSystemFeature(PackageManager.FEATURE_AUTOMOTIVE);
+    private void toggleAutomotiveFeature(final boolean isEnabled) {
+        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);
     }
 }