[Networking] Filter out PROFILE_CLASS_PROVISIONING subscriptions
Use the newly-tracked `profileClass` bit to filter out subscriptions
which are being provisioned. They are unusable and should not be
considered valid for UI consumption yet.
Test: MobileIconsInteractorTest
Fixes: 300018361
Flag: NONE
Change-Id: I02f84f736534ff48f307642250cfc2495be60561
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/domain/interactor/MobileIconsInteractor.kt b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/domain/interactor/MobileIconsInteractor.kt
index 62150e9..dad4093 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/domain/interactor/MobileIconsInteractor.kt
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/domain/interactor/MobileIconsInteractor.kt
@@ -19,10 +19,13 @@
import android.content.Context
import android.telephony.CarrierConfigManager
import android.telephony.SubscriptionManager
+import android.telephony.SubscriptionManager.PROFILE_CLASS_PROVISIONING
import com.android.settingslib.SignalIcon.MobileIconGroup
import com.android.settingslib.mobile.TelephonyIcons
import com.android.systemui.dagger.SysUISingleton
import com.android.systemui.dagger.qualifiers.Application
+import com.android.systemui.flags.FeatureFlagsClassic
+import com.android.systemui.flags.Flags.FILTER_PROVISIONING_NETWORK_SUBSCRIPTIONS
import com.android.systemui.log.table.TableLogBuffer
import com.android.systemui.log.table.logDiffsForTable
import com.android.systemui.statusbar.pipeline.dagger.MobileSummaryLog
@@ -121,6 +124,7 @@
userSetupRepo: UserSetupRepository,
@Application private val scope: CoroutineScope,
private val context: Context,
+ private val featureFlagsClassic: FeatureFlagsClassic,
) : MobileIconsInteractor {
// Weak reference lookup for created interactors
@@ -163,6 +167,20 @@
mobileConnectionsRepo.subscriptions
/**
+ * Any filtering that we can do based purely on the info of each subscription. Currently this
+ * only applies the ProfileClass-based filter, but if we need other they can go here
+ */
+ private val subscriptionsBasedFilteredSubs =
+ unfilteredSubscriptions.map { subs -> applyProvisioningFilter(subs) }.distinctUntilChanged()
+
+ private fun applyProvisioningFilter(subs: List<SubscriptionModel>): List<SubscriptionModel> =
+ if (!featureFlagsClassic.isEnabled(FILTER_PROVISIONING_NETWORK_SUBSCRIPTIONS)) {
+ subs
+ } else {
+ subs.filter { it.profileClass != PROFILE_CLASS_PROVISIONING }
+ }
+
+ /**
* Generally, SystemUI wants to show iconography for each subscription that is listed by
* [SubscriptionManager]. However, in the case of opportunistic subscriptions, we want to only
* show a single representation of the pair of subscriptions. The docs define opportunistic as:
@@ -177,48 +195,15 @@
*/
override val filteredSubscriptions: Flow<List<SubscriptionModel>> =
combine(
- unfilteredSubscriptions,
+ subscriptionsBasedFilteredSubs,
mobileConnectionsRepo.activeMobileDataSubscriptionId,
connectivityRepository.vcnSubId,
) { unfilteredSubs, activeId, vcnSubId ->
- // Based on the old logic,
- if (unfilteredSubs.size != 2) {
- return@combine unfilteredSubs
- }
-
- val info1 = unfilteredSubs[0]
- val info2 = unfilteredSubs[1]
-
- // Filtering only applies to subscriptions in the same group
- if (info1.groupUuid == null || info1.groupUuid != info2.groupUuid) {
- return@combine unfilteredSubs
- }
-
- // If both subscriptions are primary, show both
- if (!info1.isOpportunistic && !info2.isOpportunistic) {
- return@combine unfilteredSubs
- }
-
- // NOTE: at this point, we are now returning a single SubscriptionInfo
-
- // If carrier required, always show the icon of the primary subscription.
- // Otherwise, show whichever subscription is currently active for internet.
- if (carrierConfigTracker.alwaysShowPrimarySignalBarInOpportunisticNetworkDefault) {
- // return the non-opportunistic info
- return@combine if (info1.isOpportunistic) listOf(info2) else listOf(info1)
- } else {
- // It's possible for the subId of the VCN to disagree with the active subId in
- // cases where the system has tried to switch but found no connection. In these
- // scenarios, VCN will always have the subId that we want to use, so use that
- // value instead of the activeId reported by telephony
- val subIdToKeep = vcnSubId ?: activeId
-
- return@combine if (info1.subscriptionId == subIdToKeep) {
- listOf(info1)
- } else {
- listOf(info2)
- }
- }
+ filterSubsBasedOnOpportunistic(
+ unfilteredSubs,
+ activeId,
+ vcnSubId,
+ )
}
.distinctUntilChanged()
.logDiffsForTable(
@@ -229,6 +214,51 @@
)
.stateIn(scope, SharingStarted.WhileSubscribed(), listOf())
+ private fun filterSubsBasedOnOpportunistic(
+ subList: List<SubscriptionModel>,
+ activeId: Int?,
+ vcnSubId: Int?,
+ ): List<SubscriptionModel> {
+ // Based on the old logic,
+ if (subList.size != 2) {
+ return subList
+ }
+
+ val info1 = subList[0]
+ val info2 = subList[1]
+
+ // Filtering only applies to subscriptions in the same group
+ if (info1.groupUuid == null || info1.groupUuid != info2.groupUuid) {
+ return subList
+ }
+
+ // If both subscriptions are primary, show both
+ if (!info1.isOpportunistic && !info2.isOpportunistic) {
+ return subList
+ }
+
+ // NOTE: at this point, we are now returning a single SubscriptionInfo
+
+ // If carrier required, always show the icon of the primary subscription.
+ // Otherwise, show whichever subscription is currently active for internet.
+ if (carrierConfigTracker.alwaysShowPrimarySignalBarInOpportunisticNetworkDefault) {
+ // return the non-opportunistic info
+ return if (info1.isOpportunistic) listOf(info2) else listOf(info1)
+ } else {
+ // It's possible for the subId of the VCN to disagree with the active subId in
+ // cases where the system has tried to switch but found no connection. In these
+ // scenarios, VCN will always have the subId that we want to use, so use that
+ // value instead of the activeId reported by telephony
+ val subIdToKeep = vcnSubId ?: activeId
+
+ return if (info1.subscriptionId == subIdToKeep) {
+ listOf(info1)
+ } else {
+ listOf(info2)
+ }
+ }
+ }
+
/**
* Copied from the old pipeline. We maintain a 2s period of time where we will keep the
* validated bit from the old active network (A) while data is changing to the new one (B).
diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/domain/interactor/MobileIconsInteractorTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/domain/interactor/MobileIconsInteractorTest.kt
index 12d5702..2060288 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/domain/interactor/MobileIconsInteractorTest.kt
+++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/domain/interactor/MobileIconsInteractorTest.kt
@@ -17,11 +17,16 @@
package com.android.systemui.statusbar.pipeline.mobile.domain.interactor
import android.os.ParcelUuid
+import android.telephony.SubscriptionManager
import android.telephony.SubscriptionManager.INVALID_SUBSCRIPTION_ID
+import android.telephony.SubscriptionManager.PROFILE_CLASS_PROVISIONING
import android.telephony.SubscriptionManager.PROFILE_CLASS_UNSET
import androidx.test.filters.SmallTest
import com.android.settingslib.mobile.MobileMappings
import com.android.systemui.SysuiTestCase
+import com.android.systemui.coroutines.collectLastValue
+import com.android.systemui.flags.FakeFeatureFlagsClassic
+import com.android.systemui.flags.Flags
import com.android.systemui.log.table.TableLogBuffer
import com.android.systemui.statusbar.pipeline.mobile.data.model.SubscriptionModel
import com.android.systemui.statusbar.pipeline.mobile.data.repository.FakeMobileConnectionRepository
@@ -58,6 +63,10 @@
private lateinit var connectionsRepository: FakeMobileConnectionsRepository
private val userSetupRepository = FakeUserSetupRepository()
private val mobileMappingsProxy = FakeMobileMappingsProxy()
+ private val flags =
+ FakeFeatureFlagsClassic().apply {
+ set(Flags.FILTER_PROVISIONING_NETWORK_SUBSCRIPTIONS, true)
+ }
private val testDispatcher = UnconfinedTestDispatcher()
private val testScope = TestScope(testDispatcher)
@@ -100,6 +109,7 @@
userSetupRepository,
testScope.backgroundScope,
context,
+ flags,
)
}
@@ -319,6 +329,123 @@
}
@Test
+ fun filteredSubscriptions_doesNotFilterProvisioningWhenFlagIsFalse() =
+ testScope.runTest {
+ // GIVEN the flag is false
+ flags.set(Flags.FILTER_PROVISIONING_NETWORK_SUBSCRIPTIONS, false)
+
+ // GIVEN 1 sub that is in PROFILE_CLASS_PROVISIONING
+ val sub1 =
+ SubscriptionModel(
+ subscriptionId = SUB_1_ID,
+ isOpportunistic = false,
+ carrierName = "Carrier 1",
+ profileClass = PROFILE_CLASS_PROVISIONING,
+ )
+
+ connectionsRepository.setSubscriptions(listOf(sub1))
+
+ // WHEN filtering is applied
+ val latest by collectLastValue(underTest.filteredSubscriptions)
+
+ // THEN the provisioning sub is still present (unfiltered)
+ assertThat(latest).isEqualTo(listOf(sub1))
+ }
+
+ @Test
+ fun filteredSubscriptions_filtersOutProvisioningSubs() =
+ testScope.runTest {
+ val sub1 =
+ SubscriptionModel(
+ subscriptionId = SUB_1_ID,
+ isOpportunistic = false,
+ carrierName = "Carrier 1",
+ profileClass = PROFILE_CLASS_UNSET,
+ )
+ val sub2 =
+ SubscriptionModel(
+ subscriptionId = SUB_2_ID,
+ isOpportunistic = false,
+ carrierName = "Carrier 2",
+ profileClass = SubscriptionManager.PROFILE_CLASS_PROVISIONING,
+ )
+
+ connectionsRepository.setSubscriptions(listOf(sub1, sub2))
+
+ val latest by collectLastValue(underTest.filteredSubscriptions)
+
+ assertThat(latest).isEqualTo(listOf(sub1))
+ }
+
+ /** Note: I'm not sure if this will ever be the case, but we can test it at least */
+ @Test
+ fun filteredSubscriptions_filtersOutProvisioningSubsBeforeOpportunistic() =
+ testScope.runTest {
+ // This is a contrived test case, where the active subId is the one that would
+ // also be filtered by opportunistic filtering.
+
+ // GIVEN grouped, opportunistic subscriptions
+ val groupUuid = ParcelUuid(UUID.randomUUID())
+ val sub1 =
+ SubscriptionModel(
+ subscriptionId = 1,
+ isOpportunistic = true,
+ groupUuid = groupUuid,
+ carrierName = "Carrier 1",
+ profileClass = PROFILE_CLASS_PROVISIONING,
+ )
+
+ val sub2 =
+ SubscriptionModel(
+ subscriptionId = 2,
+ isOpportunistic = true,
+ groupUuid = groupUuid,
+ carrierName = "Carrier 2",
+ profileClass = PROFILE_CLASS_UNSET,
+ )
+
+ // GIVEN active subId is 1
+ connectionsRepository.setSubscriptions(listOf(sub1, sub2))
+ connectionsRepository.setActiveMobileDataSubscriptionId(1)
+
+ // THEN filtering of provisioning subs takes place first, and we result in sub2
+
+ val latest by collectLastValue(underTest.filteredSubscriptions)
+
+ assertThat(latest).isEqualTo(listOf(sub2))
+ }
+
+ @Test
+ fun filteredSubscriptions_groupedPairAndNonProvisioned_groupedFilteringStillHappens() =
+ testScope.runTest {
+ // Grouped filtering only happens when the list of subs is length 2. In this case
+ // we'll show that filtering of provisioning subs happens before, and thus grouped
+ // filtering happens even though the unfiltered list is length 3
+ val (sub1, sub3) =
+ createSubscriptionPair(
+ subscriptionIds = Pair(SUB_1_ID, SUB_3_ID),
+ opportunistic = Pair(true, true),
+ grouped = true,
+ )
+
+ val sub2 =
+ SubscriptionModel(
+ subscriptionId = 2,
+ isOpportunistic = true,
+ groupUuid = null,
+ carrierName = "Carrier 2",
+ profileClass = PROFILE_CLASS_PROVISIONING,
+ )
+
+ connectionsRepository.setSubscriptions(listOf(sub1, sub2, sub3))
+ connectionsRepository.setActiveMobileDataSubscriptionId(1)
+
+ val latest by collectLastValue(underTest.filteredSubscriptions)
+
+ assertThat(latest).isEqualTo(listOf(sub1))
+ }
+
+ @Test
fun activeDataConnection_turnedOn() =
testScope.runTest {
CONNECTION_1.setDataEnabled(true)
diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/ui/viewmodel/LocationBasedMobileIconViewModelTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/ui/viewmodel/LocationBasedMobileIconViewModelTest.kt
index 1d5487f..6a69d1f 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/ui/viewmodel/LocationBasedMobileIconViewModelTest.kt
+++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/ui/viewmodel/LocationBasedMobileIconViewModelTest.kt
@@ -70,7 +70,11 @@
private lateinit var airplaneModeInteractor: AirplaneModeInteractor
private val connectivityRepository = FakeConnectivityRepository()
- private val flags = FakeFeatureFlagsClassic().also { it.set(Flags.NEW_NETWORK_SLICE_UI, false) }
+ private val flags =
+ FakeFeatureFlagsClassic().also {
+ it.set(Flags.NEW_NETWORK_SLICE_UI, false)
+ it.set(Flags.FILTER_PROVISIONING_NETWORK_SUBSCRIPTIONS, true)
+ }
@Mock private lateinit var statusBarPipelineFlags: StatusBarPipelineFlags
@Mock private lateinit var constants: ConnectivityConstants
@@ -114,6 +118,7 @@
FakeUserSetupRepository(),
testScope.backgroundScope,
context,
+ flags,
)
interactor =
diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/ui/viewmodel/MobileIconViewModelTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/ui/viewmodel/MobileIconViewModelTest.kt
index c831e62..b39fc5b 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/ui/viewmodel/MobileIconViewModelTest.kt
+++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/ui/viewmodel/MobileIconViewModelTest.kt
@@ -82,7 +82,11 @@
@Mock private lateinit var tableLogBuffer: TableLogBuffer
@Mock private lateinit var carrierConfigTracker: CarrierConfigTracker
- private val flags = FakeFeatureFlagsClassic().also { it.set(Flags.NEW_NETWORK_SLICE_UI, false) }
+ private val flags =
+ FakeFeatureFlagsClassic().also {
+ it.set(Flags.NEW_NETWORK_SLICE_UI, false)
+ it.set(Flags.FILTER_PROVISIONING_NETWORK_SUBSCRIPTIONS, true)
+ }
private val testDispatcher = UnconfinedTestDispatcher()
private val testScope = TestScope(testDispatcher)
@@ -120,6 +124,7 @@
FakeUserSetupRepository(),
testScope.backgroundScope,
context,
+ flags,
)
interactor =
diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/shared/ui/viewmodel/InternetTileViewModelTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/shared/ui/viewmodel/InternetTileViewModelTest.kt
index c935dbb..8405fb4 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/shared/ui/viewmodel/InternetTileViewModelTest.kt
+++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/shared/ui/viewmodel/InternetTileViewModelTest.kt
@@ -22,6 +22,8 @@
import com.android.systemui.common.shared.model.ContentDescription.Companion.loadContentDescription
import com.android.systemui.common.shared.model.Text
import com.android.systemui.coroutines.collectLastValue
+import com.android.systemui.flags.FakeFeatureFlagsClassic
+import com.android.systemui.flags.Flags
import com.android.systemui.log.table.TableLogBuffer
import com.android.systemui.qs.tileimpl.QSTileImpl.ResourceIcon
import com.android.systemui.res.R
@@ -75,6 +77,11 @@
private val mobileConnectionRepository =
FakeMobileConnectionRepository(SUB_1_ID, tableLogBuffer)
+ private val flags =
+ FakeFeatureFlagsClassic().also {
+ it.set(Flags.FILTER_PROVISIONING_NETWORK_SUBSCRIPTIONS, true)
+ }
+
private val internet = context.getString(R.string.quick_settings_internet_label)
@Before
@@ -101,6 +108,7 @@
userSetupRepo,
testScope.backgroundScope,
context,
+ flags,
)
underTest =