Changes to make eth service methods more testable am: a5f724dfdd am: 4f22cf4a85

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

Change-Id: I747b401718c4860f6f38d925cf6b050fb7957470

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);
     }
 }