Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 08e09b9e authored by Evan Laird's avatar Evan Laird
Browse files

[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
parent 4f7709ad

packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/domain/interactor/MobileIconsInteractor.kt

+69 −39
Original line number Diff line number Diff line
@@ -19,10 +19,13 @@ package com.android.systemui.statusbar.pipeline.mobile.domain.interactor 
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 @@ constructor( 
    userSetupRepo: UserSetupRepository,

    @Application private val scope: CoroutineScope,

    private val context: Context,

    private val featureFlagsClassic: FeatureFlagsClassic,

) : MobileIconsInteractor {



    // Weak reference lookup for created interactors
@@ -162,6 +166,20 @@ constructor( 
    private val unfilteredSubscriptions: Flow<List<SubscriptionModel>> =

        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
@@ -177,26 +195,46 @@ constructor( 
     */

    override val filteredSubscriptions: Flow<List<SubscriptionModel>> =

        combine(

                unfilteredSubscriptions,

                subscriptionsBasedFilteredSubs,

                mobileConnectionsRepo.activeMobileDataSubscriptionId,

                connectivityRepository.vcnSubId,

            ) { unfilteredSubs, activeId, vcnSubId ->

                filterSubsBasedOnOpportunistic(

                    unfilteredSubs,

                    activeId,

                    vcnSubId,

                )

            }

            .distinctUntilChanged()

            .logDiffsForTable(

                tableLogger,

                LOGGING_PREFIX,

                columnName = "filteredSubscriptions",

                initialValue = listOf(),

            )

            .stateIn(scope, SharingStarted.WhileSubscribed(), listOf())



    private fun filterSubsBasedOnOpportunistic(

        subList: List<SubscriptionModel>,

        activeId: Int?,

        vcnSubId: Int?,

    ): List<SubscriptionModel> {

        // Based on the old logic,

                if (unfilteredSubs.size != 2) {

                    return@combine unfilteredSubs

        if (subList.size != 2) {

            return subList

        }



                val info1 = unfilteredSubs[0]

                val info2 = unfilteredSubs[1]

        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@combine unfilteredSubs

            return subList

        }



        // If both subscriptions are primary, show both

        if (!info1.isOpportunistic && !info2.isOpportunistic) {

                    return@combine unfilteredSubs

            return subList

        }



        // NOTE: at this point, we are now returning a single SubscriptionInfo
@@ -205,7 +243,7 @@ constructor( 
        // 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)

            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
@@ -213,21 +251,13 @@ constructor( 
            // value instead of the activeId reported by telephony

            val subIdToKeep = vcnSubId ?: activeId



                    return@combine if (info1.subscriptionId == subIdToKeep) {

            return if (info1.subscriptionId == subIdToKeep) {

                listOf(info1)

            } else {

                listOf(info2)

            }

        }

    }

            .distinctUntilChanged()

            .logDiffsForTable(

                tableLogger,

                LOGGING_PREFIX,

                columnName = "filteredSubscriptions",

                initialValue = listOf(),

            )

            .stateIn(scope, SharingStarted.WhileSubscribed(), listOf())



    /**

     * Copied from the old pipeline. We maintain a 2s period of time where we will keep the

packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/domain/interactor/MobileIconsInteractorTest.kt

+127 −0
Original line number Diff line number Diff line
@@ -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 @@ class MobileIconsInteractorTest : SysuiTestCase() { 
    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 @@ class MobileIconsInteractorTest : SysuiTestCase() { 
                userSetupRepository,

                testScope.backgroundScope,

                context,

                flags,

            )

    }
@@ -318,6 +328,123 @@ class MobileIconsInteractorTest : SysuiTestCase() { 
            job.cancel()

        }



    @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 {

packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/ui/viewmodel/LocationBasedMobileIconViewModelTest.kt

+6 −1
Original line number Diff line number Diff line
@@ -70,7 +70,11 @@ class LocationBasedMobileIconViewModelTest : SysuiTestCase() { 
    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 @@ class LocationBasedMobileIconViewModelTest : SysuiTestCase() { 
                FakeUserSetupRepository(),

                testScope.backgroundScope,

                context,

                flags,

            )



        interactor =

packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/ui/viewmodel/MobileIconViewModelTest.kt

+6 −1
Original line number Diff line number Diff line
@@ -82,7 +82,11 @@ class MobileIconViewModelTest : SysuiTestCase() { 
    @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 @@ class MobileIconViewModelTest : SysuiTestCase() { 
                FakeUserSetupRepository(),

                testScope.backgroundScope,

                context,

                flags,

            )



        interactor =

packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/shared/ui/viewmodel/InternetTileViewModelTest.kt

+8 −0
Original line number Diff line number Diff line
@@ -22,6 +22,8 @@ import com.android.systemui.SysuiTestCase 
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 @@ class InternetTileViewModelTest : SysuiTestCase() { 
    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 @@ class InternetTileViewModelTest : SysuiTestCase() { 
                userSetupRepo,

                testScope.backgroundScope,

                context,

                flags,

            )



        underTest =