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

Commit 84a5d4b0 authored by Evan Laird's avatar Evan Laird
Browse files

[Sb refactor] Check groupUuid in subscription filtering 

This was a bit of logic missing from the migration from the old
pipeline. When filtering subscriptions based on their opportunistic
fields, we first need to check that they are both in the same group (per
SubscriptionInfo.getGroupUuid()).

Test: MobileIconsInteractorTest
Fixes: 268533422
Change-Id: I4f035c83d074cddb45f03c996fce0a3e10797b11
parent b83e1afc

packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/model/SubscriptionModel.kt

+5 −0
Original line number Diff line number Diff line
@@ -16,6 +16,8 @@ 


package com.android.systemui.statusbar.pipeline.mobile.data.model



import android.os.ParcelUuid



/**

 * SystemUI representation of [SubscriptionInfo]. Currently we only use two fields on the

 * subscriptions themselves: subscriptionId and isOpportunistic. Any new fields that we need can be
@@ -29,4 +31,7 @@ data class SubscriptionModel( 
     * filtering in certain cases. See [MobileIconsInteractor] for the filtering logic

     */

    val isOpportunistic: Boolean = false,



    /** Subscriptions in the same group may be filtered or treated as a single subscription */

    val groupUuid: ParcelUuid? = null,

)

packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionsRepositoryImpl.kt

+1 −0
Original line number Diff line number Diff line
@@ -376,6 +376,7 @@ constructor( 
        SubscriptionModel(

            subscriptionId = subscriptionId,

            isOpportunistic = isOpportunistic,

            groupUuid = groupUuid,

        )



    companion object {

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

+7 −1
Original line number Diff line number Diff line
@@ -150,6 +150,12 @@ constructor( 


                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
@@ -186,7 +192,7 @@ constructor( 
     * validated bit from the old active network (A) while data is changing to the new one (B).

     *

     * This condition only applies if

     * 1. A and B are in the same subscription group (e.c. for CBRS data switching) and

     * 1. A and B are in the same subscription group (e.g. for CBRS data switching) and

     * 2. A was validated before the switch

     *

     * The goal of this is to minimize the flickering in the UI of the cellular indicator

packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionsRepositoryTest.kt

+14 −4
Original line number Diff line number Diff line
@@ -894,21 +894,31 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { 


        // Subscription 1

        private const val SUB_1_ID = 1

        private val GROUP_1 = ParcelUuid(UUID.randomUUID())

        private val SUB_1 =

            mock<SubscriptionInfo>().also {

                whenever(it.subscriptionId).thenReturn(SUB_1_ID)

                whenever(it.groupUuid).thenReturn(ParcelUuid(UUID.randomUUID()))

                whenever(it.groupUuid).thenReturn(GROUP_1)

            }

        private val MODEL_1 = SubscriptionModel(subscriptionId = SUB_1_ID)

        private val MODEL_1 =

            SubscriptionModel(

                subscriptionId = SUB_1_ID,

                groupUuid = GROUP_1,

            )



        // Subscription 2

        private const val SUB_2_ID = 2

        private val GROUP_2 = ParcelUuid(UUID.randomUUID())

        private val SUB_2 =

            mock<SubscriptionInfo>().also {

                whenever(it.subscriptionId).thenReturn(SUB_2_ID)

                whenever(it.groupUuid).thenReturn(ParcelUuid(UUID.randomUUID()))

                whenever(it.groupUuid).thenReturn(GROUP_2)

            }

        private val MODEL_2 = SubscriptionModel(subscriptionId = SUB_2_ID)

        private val MODEL_2 =

            SubscriptionModel(

                subscriptionId = SUB_2_ID,

                groupUuid = GROUP_2,

            )



        // Subs 3 and 4 are considered to be in the same group ------------------------------------

        private val GROUP_ID_3_4 = ParcelUuid(UUID.randomUUID())

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

+125 −13
Original line number Diff line number Diff line
@@ -16,6 +16,7 @@ 


package com.android.systemui.statusbar.pipeline.mobile.domain.interactor



import android.os.ParcelUuid

import android.telephony.SubscriptionManager.INVALID_SUBSCRIPTION_ID

import androidx.test.filters.SmallTest

import com.android.settingslib.mobile.MobileMappings
@@ -34,6 +35,7 @@ import com.android.systemui.util.mockito.mock 
import com.android.systemui.util.mockito.whenever

import com.android.systemui.util.time.FakeSystemClock

import com.google.common.truth.Truth.assertThat

import java.util.UUID

import kotlinx.coroutines.ExperimentalCoroutinesApi

import kotlinx.coroutines.flow.launchIn

import kotlinx.coroutines.flow.onEach
@@ -104,6 +106,21 @@ class MobileIconsInteractorTest : SysuiTestCase() { 
            job.cancel()

        }



    // Based on the logic from the old pipeline, we'll never filter subs when there are more than 2

    @Test

    fun filteredSubscriptions_moreThanTwo_doesNotFilter() =

        testScope.runTest {

            connectionsRepository.setSubscriptions(listOf(SUB_1, SUB_3_OPP, SUB_4_OPP))

            connectionsRepository.setActiveMobileDataSubscriptionId(SUB_4_ID)



            var latest: List<SubscriptionModel>? = null

            val job = underTest.filteredSubscriptions.onEach { latest = it }.launchIn(this)



            assertThat(latest).isEqualTo(listOf(SUB_1, SUB_3_OPP, SUB_4_OPP))



            job.cancel()

        }



    @Test

    fun filteredSubscriptions_nonOpportunistic_updatesWithMultipleSubs() =

        testScope.runTest {
@@ -118,10 +135,50 @@ class MobileIconsInteractorTest : SysuiTestCase() { 
        }



    @Test

    fun filteredSubscriptions_bothOpportunistic_configFalse_showsActive_3() =

    fun filteredSubscriptions_opportunistic_differentGroups_doesNotFilter() =

        testScope.runTest {

            connectionsRepository.setSubscriptions(listOf(SUB_3_OPP, SUB_4_OPP))

            connectionsRepository.setActiveMobileDataSubscriptionId(SUB_3_ID)



            var latest: List<SubscriptionModel>? = null

            val job = underTest.filteredSubscriptions.onEach { latest = it }.launchIn(this)



            assertThat(latest).isEqualTo(listOf(SUB_3_OPP, SUB_4_OPP))



            job.cancel()

        }



    @Test

    fun filteredSubscriptions_opportunistic_nonGrouped_doesNotFilter() =

        testScope.runTest {

            val (sub1, sub2) =

                createSubscriptionPair(

                    subscriptionIds = Pair(SUB_1_ID, SUB_2_ID),

                    opportunistic = Pair(true, true),

                    grouped = false,

                )

            connectionsRepository.setSubscriptions(listOf(sub1, sub2))

            connectionsRepository.setActiveMobileDataSubscriptionId(SUB_1_ID)



            var latest: List<SubscriptionModel>? = null

            val job = underTest.filteredSubscriptions.onEach { latest = it }.launchIn(this)



            assertThat(latest).isEqualTo(listOf(sub1, sub2))



            job.cancel()

        }



    @Test

    fun filteredSubscriptions_opportunistic_grouped_configFalse_showsActive_3() =

        testScope.runTest {

            val (sub3, sub4) =

                createSubscriptionPair(

                    subscriptionIds = Pair(SUB_3_ID, SUB_4_ID),

                    opportunistic = Pair(true, true),

                    grouped = true,

                )

            connectionsRepository.setSubscriptions(listOf(sub3, sub4))

            connectionsRepository.setActiveMobileDataSubscriptionId(SUB_3_ID)

            whenever(carrierConfigTracker.alwaysShowPrimarySignalBarInOpportunisticNetworkDefault)

                .thenReturn(false)
@@ -129,15 +186,21 @@ class MobileIconsInteractorTest : SysuiTestCase() { 
            val job = underTest.filteredSubscriptions.onEach { latest = it }.launchIn(this)



            // Filtered subscriptions should show the active one when the config is false

            assertThat(latest).isEqualTo(listOf(SUB_3_OPP))

            assertThat(latest).isEqualTo(listOf(sub3))



            job.cancel()

        }



    @Test

    fun filteredSubscriptions_bothOpportunistic_configFalse_showsActive_4() =

    fun filteredSubscriptions_opportunistic_grouped_configFalse_showsActive_4() =

        testScope.runTest {

            connectionsRepository.setSubscriptions(listOf(SUB_3_OPP, SUB_4_OPP))

            val (sub3, sub4) =

                createSubscriptionPair(

                    subscriptionIds = Pair(SUB_3_ID, SUB_4_ID),

                    opportunistic = Pair(true, true),

                    grouped = true,

                )

            connectionsRepository.setSubscriptions(listOf(sub3, sub4))

            connectionsRepository.setActiveMobileDataSubscriptionId(SUB_4_ID)

            whenever(carrierConfigTracker.alwaysShowPrimarySignalBarInOpportunisticNetworkDefault)

                .thenReturn(false)
@@ -146,15 +209,21 @@ class MobileIconsInteractorTest : SysuiTestCase() { 
            val job = underTest.filteredSubscriptions.onEach { latest = it }.launchIn(this)



            // Filtered subscriptions should show the active one when the config is false

            assertThat(latest).isEqualTo(listOf(SUB_4_OPP))

            assertThat(latest).isEqualTo(listOf(sub4))



            job.cancel()

        }



    @Test

    fun filteredSubscriptions_oneOpportunistic_configTrue_showsPrimary_active_1() =

    fun filteredSubscriptions_oneOpportunistic_grouped_configTrue_showsPrimary_active_1() =

        testScope.runTest {

            connectionsRepository.setSubscriptions(listOf(SUB_1, SUB_3_OPP))

            val (sub1, sub3) =

                createSubscriptionPair(

                    subscriptionIds = Pair(SUB_1_ID, SUB_3_ID),

                    opportunistic = Pair(false, true),

                    grouped = true,

                )

            connectionsRepository.setSubscriptions(listOf(sub1, sub3))

            connectionsRepository.setActiveMobileDataSubscriptionId(SUB_1_ID)

            whenever(carrierConfigTracker.alwaysShowPrimarySignalBarInOpportunisticNetworkDefault)

                .thenReturn(true)
@@ -164,15 +233,21 @@ class MobileIconsInteractorTest : SysuiTestCase() { 


            // Filtered subscriptions should show the primary (non-opportunistic) if the config is

            // true

            assertThat(latest).isEqualTo(listOf(SUB_1))

            assertThat(latest).isEqualTo(listOf(sub1))



            job.cancel()

        }



    @Test

    fun filteredSubscriptions_oneOpportunistic_configTrue_showsPrimary_nonActive_1() =

    fun filteredSubscriptions_oneOpportunistic_grouped_configTrue_showsPrimary_nonActive_1() =

        testScope.runTest {

            connectionsRepository.setSubscriptions(listOf(SUB_1, SUB_3_OPP))

            val (sub1, sub3) =

                createSubscriptionPair(

                    subscriptionIds = Pair(SUB_1_ID, SUB_3_ID),

                    opportunistic = Pair(false, true),

                    grouped = true,

                )

            connectionsRepository.setSubscriptions(listOf(sub1, sub3))

            connectionsRepository.setActiveMobileDataSubscriptionId(SUB_3_ID)

            whenever(carrierConfigTracker.alwaysShowPrimarySignalBarInOpportunisticNetworkDefault)

                .thenReturn(true)
@@ -182,7 +257,7 @@ class MobileIconsInteractorTest : SysuiTestCase() { 


            // Filtered subscriptions should show the primary (non-opportunistic) if the config is

            // true

            assertThat(latest).isEqualTo(listOf(SUB_1))

            assertThat(latest).isEqualTo(listOf(sub1))



            job.cancel()

        }
@@ -642,6 +717,33 @@ class MobileIconsInteractorTest : SysuiTestCase() { 
            job.cancel()

        }



    /**

     * Convenience method for creating a pair of subscriptions to test the filteredSubscriptions

     * flow.

     */

    private fun createSubscriptionPair(

        subscriptionIds: Pair<Int, Int>,

        opportunistic: Pair<Boolean, Boolean> = Pair(false, false),

        grouped: Boolean = false,

    ): Pair<SubscriptionModel, SubscriptionModel> {

        val groupUuid = if (grouped) ParcelUuid(UUID.randomUUID()) else null

        val sub1 =

            SubscriptionModel(

                subscriptionId = subscriptionIds.first,

                isOpportunistic = opportunistic.first,

                groupUuid = groupUuid,

            )



        val sub2 =

            SubscriptionModel(

                subscriptionId = subscriptionIds.second,

                isOpportunistic = opportunistic.second,

                groupUuid = groupUuid,

            )



        return Pair(sub1, sub2)

    }



    companion object {

        private val tableLogBuffer =

            TableLogBuffer(8, "MobileIconsInteractorTest", FakeSystemClock())
@@ -655,11 +757,21 @@ class MobileIconsInteractorTest : SysuiTestCase() { 
        private val CONNECTION_2 = FakeMobileConnectionRepository(SUB_2_ID, tableLogBuffer)



        private const val SUB_3_ID = 3

        private val SUB_3_OPP = SubscriptionModel(subscriptionId = SUB_3_ID, isOpportunistic = true)

        private val SUB_3_OPP =

            SubscriptionModel(

                subscriptionId = SUB_3_ID,

                isOpportunistic = true,

                groupUuid = ParcelUuid(UUID.randomUUID()),

            )

        private val CONNECTION_3 = FakeMobileConnectionRepository(SUB_3_ID, tableLogBuffer)



        private const val SUB_4_ID = 4

        private val SUB_4_OPP = SubscriptionModel(subscriptionId = SUB_4_ID, isOpportunistic = true)

        private val SUB_4_OPP =

            SubscriptionModel(

                subscriptionId = SUB_4_ID,

                isOpportunistic = true,

                groupUuid = ParcelUuid(UUID.randomUUID()),

            )

        private val CONNECTION_4 = FakeMobileConnectionRepository(SUB_4_ID, tableLogBuffer)

    }

}