Merge "[Sb refactor] Check groupUuid in subscription filtering" into tm-qpr-dev

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

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

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

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

@@ -896,21 +896,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

@@ -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)
     }
 }