From 2b679cec72477b8ef8199690729b71bdeb466f92 Mon Sep 17 00:00:00 2001 From: Evan Laird Date: Wed, 18 Jan 2023 13:08:26 -0500 Subject: [PATCH] [Sb refactor] Fix active subId in group event flow This flow was malformed and would never actually emit based on the original defintion. Added some tests too to make sure it works Test: MobileConnectionsRepositoryTest Bug: 238425913 Change-Id: I7522bf7d34d86632f0b863b73c8f11eaeb620430 --- .../prod/MobileConnectionsRepositoryImpl.kt | 25 ++---- .../prod/MobileConnectionsRepositoryTest.kt | 80 ++++++++++++++++++- 2 files changed, 86 insertions(+), 19 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionsRepositoryImpl.kt b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionsRepositoryImpl.kt index 510482dcb21e0..39ad31f7eee82 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionsRepositoryImpl.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionsRepositoryImpl.kt @@ -53,7 +53,7 @@ import com.android.systemui.statusbar.pipeline.shared.ConnectivityPipelineLogger import com.android.systemui.statusbar.pipeline.shared.ConnectivityPipelineLogger.Companion.logInputChange import com.android.systemui.statusbar.pipeline.wifi.data.model.WifiNetworkModel import com.android.systemui.statusbar.pipeline.wifi.data.repository.WifiRepository -import com.android.systemui.util.kotlin.pairwiseBy +import com.android.systemui.util.kotlin.pairwise import com.android.systemui.util.settings.GlobalSettings import javax.inject.Inject import kotlinx.coroutines.CoroutineDispatcher @@ -66,10 +66,10 @@ import kotlinx.coroutines.flow.MutableSharedFlow import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.distinctUntilChanged -import kotlinx.coroutines.flow.flow import kotlinx.coroutines.flow.flowOn import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.mapLatest +import kotlinx.coroutines.flow.mapNotNull import kotlinx.coroutines.flow.merge import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.flow.stateIn @@ -287,22 +287,13 @@ constructor( */ @SuppressLint("MissingPermission") override val activeSubChangedInGroupEvent = - flow { - activeMobileDataSubscriptionId.pairwiseBy { prevVal: Int, newVal: Int -> - if (!defaultMobileNetworkConnectivity.value.isValidated) { - return@pairwiseBy - } - val prevSub = subscriptionManager.getActiveSubscriptionInfo(prevVal) - val nextSub = subscriptionManager.getActiveSubscriptionInfo(newVal) + activeMobileDataSubscriptionId + .pairwise() + .mapNotNull { (prevVal: Int, newVal: Int) -> + val prevSub = subscriptionManager.getActiveSubscriptionInfo(prevVal)?.groupUuid + val nextSub = subscriptionManager.getActiveSubscriptionInfo(newVal)?.groupUuid - if (prevSub == null || nextSub == null) { - return@pairwiseBy - } - - if (prevSub.groupUuid != null && prevSub.groupUuid == nextSub.groupUuid) { - emit(Unit) - } - } + if (prevSub != null && prevSub == nextSub) Unit else null } .flowOn(bgDispatcher) diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionsRepositoryTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionsRepositoryTest.kt index ae390a0e29591..db8172a5cacfa 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionsRepositoryTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionsRepositoryTest.kt @@ -22,6 +22,7 @@ import android.net.Network import android.net.NetworkCapabilities import android.net.NetworkCapabilities.NET_CAPABILITY_VALIDATED import android.net.NetworkCapabilities.TRANSPORT_CELLULAR +import android.os.ParcelUuid import android.provider.Settings import android.telephony.CarrierConfigManager import android.telephony.SubscriptionInfo @@ -50,6 +51,7 @@ import com.android.systemui.util.mockito.mock import com.android.systemui.util.mockito.whenever import com.android.systemui.util.settings.FakeSettings import com.google.common.truth.Truth.assertThat +import java.util.UUID import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.ExperimentalCoroutinesApi @@ -104,6 +106,17 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { mock() } + // For convenience, set up the subscription info callbacks + whenever(subscriptionManager.getActiveSubscriptionInfo(anyInt())).thenAnswer { invocation -> + when (invocation.getArgument(0) as Int) { + 1 -> SUB_1 + 2 -> SUB_2 + 3 -> SUB_3 + 4 -> SUB_4 + else -> null + } + } + wifiRepository = FakeWifiRepository() connectionFactory = @@ -686,6 +699,38 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { job.cancel() } + @Test + fun `active data change - in same group - emits unit`() = + runBlocking(IMMEDIATE) { + var latest: Unit? = null + val job = underTest.activeSubChangedInGroupEvent.onEach { latest = it }.launchIn(this) + + getTelephonyCallbackForType() + .onActiveDataSubscriptionIdChanged(SUB_3_ID_GROUPED) + getTelephonyCallbackForType() + .onActiveDataSubscriptionIdChanged(SUB_4_ID_GROUPED) + + assertThat(latest).isEqualTo(Unit) + + job.cancel() + } + + @Test + fun `active data change - not in same group - does not emit`() = + runBlocking(IMMEDIATE) { + var latest: Unit? = null + val job = underTest.activeSubChangedInGroupEvent.onEach { latest = it }.launchIn(this) + + getTelephonyCallbackForType() + .onActiveDataSubscriptionIdChanged(SUB_3_ID_GROUPED) + getTelephonyCallbackForType() + .onActiveDataSubscriptionIdChanged(SUB_1_ID) + + assertThat(latest).isEqualTo(null) + + job.cancel() + } + private fun createCapabilities(connected: Boolean, validated: Boolean): NetworkCapabilities = mock().also { whenever(it.hasTransport(TRANSPORT_CELLULAR)).thenReturn(connected) @@ -719,19 +764,50 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { companion object { private val IMMEDIATE = Dispatchers.Main.immediate + + // Subscription 1 private const val SUB_1_ID = 1 private val SUB_1 = - mock().also { whenever(it.subscriptionId).thenReturn(SUB_1_ID) } + mock().also { + whenever(it.subscriptionId).thenReturn(SUB_1_ID) + whenever(it.groupUuid).thenReturn(ParcelUuid(UUID.randomUUID())) + } private val MODEL_1 = SubscriptionModel(subscriptionId = SUB_1_ID) + // Subscription 2 private const val SUB_2_ID = 2 private val SUB_2 = - mock().also { whenever(it.subscriptionId).thenReturn(SUB_2_ID) } + mock().also { + whenever(it.subscriptionId).thenReturn(SUB_2_ID) + whenever(it.groupUuid).thenReturn(ParcelUuid(UUID.randomUUID())) + } private val MODEL_2 = SubscriptionModel(subscriptionId = SUB_2_ID) + // Subs 3 and 4 are considered to be in the same group ------------------------------------ + private val GROUP_ID_3_4 = ParcelUuid(UUID.randomUUID()) + + // Subscription 3 + private const val SUB_3_ID_GROUPED = 3 + private val SUB_3 = + mock().also { + whenever(it.subscriptionId).thenReturn(SUB_3_ID_GROUPED) + whenever(it.groupUuid).thenReturn(GROUP_ID_3_4) + } + + // Subscription 4 + private const val SUB_4_ID_GROUPED = 4 + private val SUB_4 = + mock().also { + whenever(it.subscriptionId).thenReturn(SUB_4_ID_GROUPED) + whenever(it.groupUuid).thenReturn(GROUP_ID_3_4) + } + + // Subs 3 and 4 are considered to be in the same group ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + private const val NET_ID = 123 private val NETWORK = mock().apply { whenever(getNetId()).thenReturn(NET_ID) } + // Carrier merged subscription private const val SUB_CM_ID = 5 private val SUB_CM = mock().also { whenever(it.subscriptionId).thenReturn(SUB_CM_ID) }