Revert ConnectivityServiceDependencies access modifier

Address review comment on aosp/2490881

ConnectivityServiceDependencies was made public in aosp/2490881 to
verify destroyLiveTcpSockets call.
This CL reverts that change and updates tests to use mocked
DestroySocketsWrapper to verify destroy sockets call.

Bug: 270298713
Test: atest FrameworksNetTests
Change-Id: I101016399ea29bc5176b7559edf0447f0f7901ce
This commit is contained in:
Motomu Utsumi
2023-05-17 13:33:22 +09:00
parent 3bf75ef193
commit d54120344d

View File

@@ -387,6 +387,7 @@ import com.android.networkstack.apishim.common.BroadcastOptionsShim;
import com.android.networkstack.apishim.common.UnsupportedApiLevelException; import com.android.networkstack.apishim.common.UnsupportedApiLevelException;
import com.android.server.ConnectivityService.ConnectivityDiagnosticsCallbackInfo; import com.android.server.ConnectivityService.ConnectivityDiagnosticsCallbackInfo;
import com.android.server.ConnectivityService.NetworkRequestInfo; import com.android.server.ConnectivityService.NetworkRequestInfo;
import com.android.server.ConnectivityServiceTest.ConnectivityServiceDependencies.DestroySocketsWrapper;
import com.android.server.ConnectivityServiceTest.ConnectivityServiceDependencies.ReportedInterfaces; import com.android.server.ConnectivityServiceTest.ConnectivityServiceDependencies.ReportedInterfaces;
import com.android.server.connectivity.ApplicationSelfCertifiedNetworkCapabilities; import com.android.server.connectivity.ApplicationSelfCertifiedNetworkCapabilities;
import com.android.server.connectivity.AutomaticOnOffKeepaliveTracker; import com.android.server.connectivity.AutomaticOnOffKeepaliveTracker;
@@ -615,6 +616,7 @@ public class ConnectivityServiceTest {
@Mock TetheringManager mTetheringManager; @Mock TetheringManager mTetheringManager;
@Mock BroadcastOptionsShim mBroadcastOptionsShim; @Mock BroadcastOptionsShim mBroadcastOptionsShim;
@Mock ActivityManager mActivityManager; @Mock ActivityManager mActivityManager;
@Mock DestroySocketsWrapper mDestroySocketsWrapper;
// BatteryStatsManager is final and cannot be mocked with regular mockito, so just mock the // BatteryStatsManager is final and cannot be mocked with regular mockito, so just mock the
// underlying binder calls. // underlying binder calls.
@@ -1865,7 +1867,7 @@ public class ConnectivityServiceTest {
final Context mockResContext = mock(Context.class); final Context mockResContext = mock(Context.class);
doReturn(mResources).when(mockResContext).getResources(); doReturn(mResources).when(mockResContext).getResources();
ConnectivityResources.setResourcesContextForTest(mockResContext); ConnectivityResources.setResourcesContextForTest(mockResContext);
mDeps = spy(new ConnectivityServiceDependencies(mockResContext)); mDeps = new ConnectivityServiceDependencies(mockResContext);
mAutoOnOffKeepaliveDependencies = mAutoOnOffKeepaliveDependencies =
new AutomaticOnOffKeepaliveTrackerDependencies(mServiceContext); new AutomaticOnOffKeepaliveTrackerDependencies(mServiceContext);
mService = new ConnectivityService(mServiceContext, mService = new ConnectivityService(mServiceContext,
@@ -1928,8 +1930,7 @@ public class ConnectivityServiceTest {
R.integer.config_networkWakeupPacketMark); R.integer.config_networkWakeupPacketMark);
} }
// ConnectivityServiceDependencies is public to use Mockito.spy class ConnectivityServiceDependencies extends ConnectivityService.Dependencies {
public class ConnectivityServiceDependencies extends ConnectivityService.Dependencies {
final ConnectivityResources mConnRes; final ConnectivityResources mConnRes;
ConnectivityServiceDependencies(final Context mockResContext) { ConnectivityServiceDependencies(final Context mockResContext) {
@@ -2168,15 +2169,24 @@ public class ConnectivityServiceTest {
} }
} }
@Override // Class to be mocked and used to verify destroy sockets methods call
public void destroyLiveTcpSockets(final Set<Range<Integer>> ranges, public class DestroySocketsWrapper {
final Set<Integer> exemptUids) { public void destroyLiveTcpSockets(final Set<Range<Integer>> ranges,
// This function is empty since the invocation of this method is verified by mocks final Set<Integer> exemptUids){}
public void destroyLiveTcpSocketsByOwnerUids(final Set<Integer> ownerUids){}
} }
@Override @Override @SuppressWarnings("DirectInvocationOnMock")
public void destroyLiveTcpSockets(final Set<Range<Integer>> ranges,
final Set<Integer> exemptUids) {
// Call mocked destroyLiveTcpSockets so that test can verify this method call
mDestroySocketsWrapper.destroyLiveTcpSockets(ranges, exemptUids);
}
@Override @SuppressWarnings("DirectInvocationOnMock")
public void destroyLiveTcpSocketsByOwnerUids(final Set<Integer> ownerUids) { public void destroyLiveTcpSocketsByOwnerUids(final Set<Integer> ownerUids) {
// This function is empty since the invocation of this method is verified by mocks // Call mocked destroyLiveTcpSocketsByOwnerUids so that test can verify this method call
mDestroySocketsWrapper.destroyLiveTcpSocketsByOwnerUids(ownerUids);
} }
} }
@@ -10276,7 +10286,7 @@ public class ConnectivityServiceTest {
private void doTestSetFirewallChainEnabledCloseSocket(final int chain, private void doTestSetFirewallChainEnabledCloseSocket(final int chain,
final boolean isAllowList) throws Exception { final boolean isAllowList) throws Exception {
reset(mDeps); reset(mDestroySocketsWrapper);
mCm.setFirewallChainEnabled(chain, true /* enabled */); mCm.setFirewallChainEnabled(chain, true /* enabled */);
final Set<Integer> uids = final Set<Integer> uids =
@@ -10284,13 +10294,13 @@ public class ConnectivityServiceTest {
if (isAllowList) { if (isAllowList) {
final Set<Range<Integer>> range = new ArraySet<>( final Set<Range<Integer>> range = new ArraySet<>(
List.of(new Range<>(Process.FIRST_APPLICATION_UID, Integer.MAX_VALUE))); List.of(new Range<>(Process.FIRST_APPLICATION_UID, Integer.MAX_VALUE)));
verify(mDeps).destroyLiveTcpSockets(range, uids); verify(mDestroySocketsWrapper).destroyLiveTcpSockets(range, uids);
} else { } else {
verify(mDeps).destroyLiveTcpSocketsByOwnerUids(uids); verify(mDestroySocketsWrapper).destroyLiveTcpSocketsByOwnerUids(uids);
} }
mCm.setFirewallChainEnabled(chain, false /* enabled */); mCm.setFirewallChainEnabled(chain, false /* enabled */);
verifyNoMoreInteractions(mDeps); verifyNoMoreInteractions(mDestroySocketsWrapper);
} }
@Test @IgnoreUpTo(Build.VERSION_CODES.TIRAMISU) @Test @IgnoreUpTo(Build.VERSION_CODES.TIRAMISU)
@@ -12627,11 +12637,11 @@ public class ConnectivityServiceTest {
private void assertVpnUidRangesUpdated(boolean add, Set<UidRange> vpnRanges, int exemptUid) private void assertVpnUidRangesUpdated(boolean add, Set<UidRange> vpnRanges, int exemptUid)
throws Exception { throws Exception {
InOrder inOrder = inOrder(mMockNetd, mDeps); InOrder inOrder = inOrder(mMockNetd, mDestroySocketsWrapper);
final Set<Integer> exemptUidSet = new ArraySet<>(List.of(exemptUid, Process.VPN_UID)); final Set<Integer> exemptUidSet = new ArraySet<>(List.of(exemptUid, Process.VPN_UID));
inOrder.verify(mDeps).destroyLiveTcpSockets(UidRange.toIntRanges(vpnRanges), inOrder.verify(mDestroySocketsWrapper).destroyLiveTcpSockets(
exemptUidSet); UidRange.toIntRanges(vpnRanges), exemptUidSet);
if (add) { if (add) {
inOrder.verify(mMockNetd, times(1)).networkAddUidRangesParcel( inOrder.verify(mMockNetd, times(1)).networkAddUidRangesParcel(
@@ -12643,8 +12653,8 @@ public class ConnectivityServiceTest {
toUidRangeStableParcels(vpnRanges), PREFERENCE_ORDER_VPN)); toUidRangeStableParcels(vpnRanges), PREFERENCE_ORDER_VPN));
} }
inOrder.verify(mDeps).destroyLiveTcpSockets(UidRange.toIntRanges(vpnRanges), inOrder.verify(mDestroySocketsWrapper).destroyLiveTcpSockets(
exemptUidSet); UidRange.toIntRanges(vpnRanges), exemptUidSet);
} }
@Test @Test
@@ -17984,7 +17994,7 @@ public class ConnectivityServiceTest {
final UidRange frozenUidRange = new UidRange(TEST_FROZEN_UID, TEST_FROZEN_UID); final UidRange frozenUidRange = new UidRange(TEST_FROZEN_UID, TEST_FROZEN_UID);
final Set<UidRange> ranges = Collections.singleton(frozenUidRange); final Set<UidRange> ranges = Collections.singleton(frozenUidRange);
verify(mDeps).destroyLiveTcpSockets(eq(UidRange.toIntRanges(ranges)), verify(mDestroySocketsWrapper).destroyLiveTcpSockets(eq(UidRange.toIntRanges(ranges)),
eq(exemptUids)); eq(exemptUids));
} }
} }