Sort the selectable subscription by id

To make sure users always see a fixed order.

Bug: 318310357
Test: manual - on SIMs and turn on / off SIM
Change-Id: I2910d83f5b1d04287afca0e3ba91dedbeb588c14

src/com/android/settings/network/SubscriptionInfoListViewModel.kt

@@ -20,6 +20,7 @@ import android.app.Application
 import android.telephony.SubscriptionManager
 import androidx.lifecycle.AndroidViewModel
 import androidx.lifecycle.viewModelScope
+import com.android.settings.network.telephony.getSelectableSubscriptionInfoList
 import com.android.settings.network.telephony.subscriptionsChangedFlow
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.flow.SharingStarted
@@ -41,10 +42,10 @@ class SubscriptionInfoListViewModel(application: Application) : AndroidViewModel
     }.stateIn(scope, SharingStarted.Eagerly, initialValue = emptyList())
 
     /**
-     * Getting the Selectable SubscriptionInfo List from the SubscriptionManager's
+     * Getting the Selectable SubscriptionInfo List from the SubscriptionRepository's
      * getAvailableSubscriptionInfoList
      */
     val selectableSubscriptionInfoListFlow = application.subscriptionsChangedFlow().map {
-        SubscriptionUtil.getSelectableSubscriptionInfoList(application)
+        application.getSelectableSubscriptionInfoList()
     }.stateIn(scope, SharingStarted.Eagerly, initialValue = emptyList())
 }

src/com/android/settings/network/SubscriptionUtil.java

@@ -50,12 +50,12 @@ import com.android.settings.network.helper.SelectableSubscriptions;
 import com.android.settings.network.helper.SubscriptionAnnotation;
 import com.android.settings.network.telephony.DeleteEuiccSubscriptionDialogActivity;
 import com.android.settings.network.telephony.EuiccRacConnectivityDialogActivity;
+import com.android.settings.network.telephony.SubscriptionRepositoryKt;
 import com.android.settings.network.telephony.ToggleSubscriptionDialogActivity;
 
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
-import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Locale;
@@ -499,40 +499,7 @@ public class SubscriptionUtil {
      * @return list of user selectable subscriptions.
      */
     public static List<SubscriptionInfo> getSelectableSubscriptionInfoList(Context context) {
-        SubscriptionManager subManager = context.getSystemService(SubscriptionManager.class);
+        return SubscriptionRepositoryKt.getSelectableSubscriptionInfoList(context);
-        List<SubscriptionInfo> availableList = subManager.getAvailableSubscriptionInfoList();
-        if (availableList == null) {
-            return null;
-        } else {
-            // Multiple subscriptions in a group should only have one representative.
-            // It should be the current active primary subscription if any, or any
-            // primary subscription.
-            List<SubscriptionInfo> selectableList = new ArrayList<>();
-            Map<ParcelUuid, SubscriptionInfo> groupMap = new HashMap<>();
-
-            for (SubscriptionInfo info : availableList) {
-                // Opportunistic subscriptions are considered invisible
-                // to users so they should never be returned.
-                if (!isSubscriptionVisible(subManager, context, info)) continue;
-
-                ParcelUuid groupUuid = info.getGroupUuid();
-                if (groupUuid == null) {
-                    // Doesn't belong to any group. Add in the list.
-                    selectableList.add(info);
-                } else if (!groupMap.containsKey(groupUuid)
-                        || (groupMap.get(groupUuid).getSimSlotIndex() == INVALID_SIM_SLOT_INDEX
-                        && info.getSimSlotIndex() != INVALID_SIM_SLOT_INDEX)) {
-                    // If it belongs to a group that has never been recorded or it's the current
-                    // active subscription, add it in the list.
-                    selectableList.remove(groupMap.get(groupUuid));
-                    selectableList.add(info);
-                    groupMap.put(groupUuid, info);
-                }
-
-            }
-            Log.d(TAG, "getSelectableSubscriptionInfoList: " + selectableList);
-            return selectableList;
-        }
     }
 
     /**

src/com/android/settings/network/telephony/SubscriptionRepository.kt

@@ -32,9 +32,12 @@ import kotlinx.coroutines.flow.onEach
 
 private const val TAG = "SubscriptionRepository"
 
-fun Context.isSubscriptionEnabledFlow(subId: Int) = subscriptionsChangedFlow().map {
+val Context.subscriptionManager: SubscriptionManager?
-    val subscriptionManager = getSystemService(SubscriptionManager::class.java)
+    get() = getSystemService(SubscriptionManager::class.java)
 
+fun Context.requireSubscriptionManager(): SubscriptionManager = subscriptionManager!!
+
+fun Context.isSubscriptionEnabledFlow(subId: Int) = subscriptionsChangedFlow().map {
     subscriptionManager?.isSubscriptionEnabled(subId) ?: false
 }.flowOn(Dispatchers.Default)
 
@@ -43,7 +46,7 @@ fun Context.phoneNumberFlow(subscriptionInfo: SubscriptionInfo) = subscriptionsC
 }.flowOn(Dispatchers.Default)
 
 fun Context.subscriptionsChangedFlow() = callbackFlow {
-    val subscriptionManager = getSystemService(SubscriptionManager::class.java)!!
+    val subscriptionManager = requireSubscriptionManager()
 
     val listener = object : SubscriptionManager.OnSubscriptionsChangedListener() {
         override fun onSubscriptionsChanged() {
@@ -58,3 +61,35 @@ fun Context.subscriptionsChangedFlow() = callbackFlow {
 
     awaitClose { subscriptionManager.removeOnSubscriptionsChangedListener(listener) }
 }.conflate().onEach { Log.d(TAG, "subscriptions changed") }.flowOn(Dispatchers.Default)
+
+/**
+ * Return a list of subscriptions that are available and visible to the user.
+ *
+ * @return list of user selectable subscriptions.
+ */
+fun Context.getSelectableSubscriptionInfoList(): List<SubscriptionInfo> {
+    val subscriptionManager = requireSubscriptionManager()
+    val availableList = subscriptionManager.getAvailableSubscriptionInfoList() ?: return emptyList()
+    val visibleList = availableList.filter { subInfo ->
+        // Opportunistic subscriptions are considered invisible
+        // to users so they should never be returned.
+        SubscriptionUtil.isSubscriptionVisible(subscriptionManager, this, subInfo)
+    }
+    // Multiple subscriptions in a group should only have one representative.
+    // It should be the current active primary subscription if any, or any primary subscription.
+    val groupUuidToSelectedIdMap = visibleList
+        .groupBy { it.groupUuid }
+        .mapValues { (_, subInfos) ->
+            subInfos.filter { it.simSlotIndex != SubscriptionManager.INVALID_SIM_SLOT_INDEX }
+                .ifEmpty { subInfos }
+                .minOf { it.subscriptionId }
+        }
+
+    return visibleList
+        .filter { subInfo ->
+            val groupUuid = subInfo.groupUuid ?: return@filter true
+            groupUuidToSelectedIdMap[groupUuid] == subInfo.subscriptionId
+        }
+        .sortedBy { it.subscriptionId }
+        .also { Log.d(TAG, "getSelectableSubscriptionInfoList: $it") }
+}

tests/spa_unit/src/com/android/settings/network/telephony/SubscriptionRepositoryTest.kt

@@ -17,12 +17,14 @@
 package com.android.settings.network.telephony
 
 import android.content.Context
+import android.telephony.SubscriptionInfo
 import android.telephony.SubscriptionManager
 import androidx.test.core.app.ApplicationProvider
 import androidx.test.ext.junit.runners.AndroidJUnit4
 import com.android.settingslib.spa.testutils.firstWithTimeoutOrNull
 import com.android.settingslib.spa.testutils.toListWithTimeout
 import com.google.common.truth.Truth.assertThat
+import java.util.UUID
 import kotlinx.coroutines.async
 import kotlinx.coroutines.delay
 import kotlinx.coroutines.runBlocking
@@ -47,16 +49,16 @@ class SubscriptionRepositoryTest {
     }
 
     private val context: Context = spy(ApplicationProvider.getApplicationContext()) {
-        on { getSystemService(SubscriptionManager::class.java) } doReturn mockSubscriptionManager
+        on { subscriptionManager } doReturn mockSubscriptionManager
     }
 
     @Test
     fun isSubscriptionEnabledFlow() = runBlocking {
         mockSubscriptionManager.stub {
-            on { isSubscriptionEnabled(SUB_ID) } doReturn true
+            on { isSubscriptionEnabled(SUB_ID_1) } doReturn true
         }
 
-        val isEnabled = context.isSubscriptionEnabledFlow(SUB_ID).firstWithTimeoutOrNull()
+        val isEnabled = context.isSubscriptionEnabledFlow(SUB_ID_1).firstWithTimeoutOrNull()
 
         assertThat(isEnabled).isTrue()
     }
@@ -80,7 +82,69 @@ class SubscriptionRepositoryTest {
         assertThat(listDeferred.await()).hasSize(2)
     }
 
+    @Test
+    fun getSelectableSubscriptionInfoList_sortedBySubId() {
+        mockSubscriptionManager.stub {
+            on { getAvailableSubscriptionInfoList() } doReturn listOf(
+                SubscriptionInfo.Builder().apply {
+                    setId(SUB_ID_2)
+                }.build(),
+                SubscriptionInfo.Builder().apply {
+                    setId(SUB_ID_1)
+                }.build(),
+            )
+        }
+
+        val subInfos = context.getSelectableSubscriptionInfoList()
+
+        assertThat(subInfos.map { it.subscriptionId }).containsExactly(SUB_ID_1, SUB_ID_2).inOrder()
+    }
+
+    @Test
+    fun getSelectableSubscriptionInfoList_sameGroupAndOneHasSlot_returnTheOneWithSimSlotIndex() {
+        mockSubscriptionManager.stub {
+            on { getAvailableSubscriptionInfoList() } doReturn listOf(
+                SubscriptionInfo.Builder().apply {
+                    setId(SUB_ID_1)
+                    setGroupUuid(GROUP_UUID)
+                }.build(),
+                SubscriptionInfo.Builder().apply {
+                    setId(SUB_ID_2)
+                    setGroupUuid(GROUP_UUID)
+                    setSimSlotIndex(SIM_SLOT_INDEX)
+                }.build(),
+            )
+        }
+
+        val subInfos = context.getSelectableSubscriptionInfoList()
+
+        assertThat(subInfos.map { it.subscriptionId }).containsExactly(SUB_ID_2)
+    }
+
+    @Test
+    fun getSelectableSubscriptionInfoList_sameGroupAndNonHasSlot_returnTheOneWithMinimumSubId() {
+        mockSubscriptionManager.stub {
+            on { getAvailableSubscriptionInfoList() } doReturn listOf(
+                SubscriptionInfo.Builder().apply {
+                    setId(SUB_ID_2)
+                    setGroupUuid(GROUP_UUID)
+                }.build(),
+                SubscriptionInfo.Builder().apply {
+                    setId(SUB_ID_1)
+                    setGroupUuid(GROUP_UUID)
+                }.build(),
+            )
+        }
+
+        val subInfos = context.getSelectableSubscriptionInfoList()
+
+        assertThat(subInfos.map { it.subscriptionId }).containsExactly(SUB_ID_1)
+    }
+
     private companion object {
-        const val SUB_ID = 1
+        const val SUB_ID_1 = 1
+        const val SUB_ID_2 = 2
+        val GROUP_UUID = UUID.randomUUID().toString()
+        const val SIM_SLOT_INDEX = 1
     }
 }