commit 9a9c11e33e1cc17191da8bc7b7520ab2633079ca
author Motomu Utsumi <motomuman@google.com> Tue Jan 10 17:41:50 2023 +0900
committer Motomu Utsumi <motomuman@google.com> Mon Jan 30 17:41:39 2023 +0900
tree 987e00fcfb955bc9b8adb311bd7ad3c5660d1600
parent ff12d3444548b206ed28077211191fdf7c315565

Remove getVpnLockdownUidRanges test api

getVpnLockdownUidRanges acquire lock and access internal state of a
Handler-based class (PermissionMonitor), which is bad practice in
general.

getVpnLockdownUidRanges returns keySet of mVpnLockdownUidRanges.
PermissionMonitor call updateUidLockdownRule and update the bpf map
based on mVpnLockdownUidRanges.
PermissionMonitorTest verifies the args of updateUidLockdownRule call.

So, It's not necessary to verify the mVpnLockdownUidRanges which
is internal states of PermissionMonitor.

Test: atest PermissionMonitorTest
Bug: 262199762
Change-Id: I85b39ab2aff44dcbe809b39560d6bb87fbb0c084

service/src/com/android/server/connectivity/PermissionMonitor.java

diff --git a/service/src/com/android/server/connectivity/PermissionMonitor.java b/service/src/com/android/server/connectivity/PermissionMonitor.java
index 771635e..c15f042 100755
--- a/service/src/com/android/server/connectivity/PermissionMonitor.java
+++ b/service/src/com/android/server/connectivity/PermissionMonitor.java
@@ -1211,12 +1211,6 @@
         }
     }
 
-    /** Should only be used by unit tests */
-    @VisibleForTesting
-    synchronized Set<UidRange> getVpnLockdownUidRanges() {
-        return mVpnLockdownUidRanges.getSet();
-    }
-
     private synchronized void onSettingChanged() {
         // Step1. Update uids allowed to use restricted networks and compute the set of uids to
         // update.

tests/unit/java/com/android/server/connectivity/PermissionMonitorTest.java

diff --git a/tests/unit/java/com/android/server/connectivity/PermissionMonitorTest.java b/tests/unit/java/com/android/server/connectivity/PermissionMonitorTest.java
index 185d6fb..cf02e3a 100644
--- a/tests/unit/java/com/android/server/connectivity/PermissionMonitorTest.java
+++ b/tests/unit/java/com/android/server/connectivity/PermissionMonitorTest.java
@@ -913,7 +913,6 @@
         verify(mBpfNetMaps, times(2)).updateUidLockdownRule(anyInt(), eq(true) /* add */);
         verify(mBpfNetMaps).updateUidLockdownRule(MOCK_UID11, true /* add */);
         verify(mBpfNetMaps).updateUidLockdownRule(VPN_UID, true /* add */);
-        assertEquals(mPermissionMonitor.getVpnLockdownUidRanges(), Set.of(lockdownRange));
 
         reset(mBpfNetMaps);
 
@@ -922,7 +921,6 @@
         verify(mBpfNetMaps, times(2)).updateUidLockdownRule(anyInt(), eq(false) /* add */);
         verify(mBpfNetMaps).updateUidLockdownRule(MOCK_UID11, false /* add */);
         verify(mBpfNetMaps).updateUidLockdownRule(VPN_UID, false /* add */);
-        assertTrue(mPermissionMonitor.getVpnLockdownUidRanges().isEmpty());
     }
 
     @Test
@@ -942,7 +940,6 @@
         mPermissionMonitor.updateVpnLockdownUidRanges(true /* add */, lockdownRange);
         verify(mBpfNetMaps).updateUidLockdownRule(anyInt(), eq(true) /* add */);
         verify(mBpfNetMaps).updateUidLockdownRule(MOCK_UID11, true /* add */);
-        assertEquals(mPermissionMonitor.getVpnLockdownUidRanges(), Set.of(lockdownRange));
 
         reset(mBpfNetMaps);
 
@@ -950,7 +947,6 @@
         // already has the rule
         mPermissionMonitor.updateVpnLockdownUidRanges(true /* add */, lockdownRange);
         verify(mBpfNetMaps, never()).updateUidLockdownRule(anyInt(),  anyBoolean());
-        assertEquals(mPermissionMonitor.getVpnLockdownUidRanges(), Set.of(lockdownRange));
 
         reset(mBpfNetMaps);
 
@@ -958,7 +954,6 @@
         // the range 2 times.
         mPermissionMonitor.updateVpnLockdownUidRanges(false /* add */, lockdownRange);
         verify(mBpfNetMaps, never()).updateUidLockdownRule(anyInt(),  anyBoolean());
-        assertEquals(mPermissionMonitor.getVpnLockdownUidRanges(), Set.of(lockdownRange));
 
         reset(mBpfNetMaps);
 
@@ -967,7 +962,6 @@
         mPermissionMonitor.updateVpnLockdownUidRanges(false /* add */, lockdownRange);
         verify(mBpfNetMaps).updateUidLockdownRule(anyInt(), eq(false) /* add */);
         verify(mBpfNetMaps).updateUidLockdownRule(MOCK_UID11, false /* add */);
-        assertTrue(mPermissionMonitor.getVpnLockdownUidRanges().isEmpty());
     }
 
     @Test
@@ -988,7 +982,6 @@
         mPermissionMonitor.updateVpnLockdownUidRanges(true /* add */, lockdownRangeDuplicates);
         verify(mBpfNetMaps).updateUidLockdownRule(anyInt(), eq(true) /* add */);
         verify(mBpfNetMaps).updateUidLockdownRule(MOCK_UID11, true /* add */);
-        assertEquals(mPermissionMonitor.getVpnLockdownUidRanges(), Set.of(lockdownRange));
 
         reset(mBpfNetMaps);
 
@@ -996,7 +989,6 @@
         // ranges we added contains duplicated uid ranges.
         mPermissionMonitor.updateVpnLockdownUidRanges(false /* add */, lockdownRange);
         verify(mBpfNetMaps, never()).updateUidLockdownRule(anyInt(), anyBoolean());
-        assertEquals(mPermissionMonitor.getVpnLockdownUidRanges(), Set.of(lockdownRange));
 
         reset(mBpfNetMaps);
 
@@ -1004,7 +996,6 @@
         mPermissionMonitor.updateVpnLockdownUidRanges(false /* add */, lockdownRange);
         verify(mBpfNetMaps).updateUidLockdownRule(anyInt(), eq(false) /* add */);
         verify(mBpfNetMaps).updateUidLockdownRule(MOCK_UID11, false /* add */);
-        assertTrue(mPermissionMonitor.getVpnLockdownUidRanges().isEmpty());
     }
 
     @Test