Fallback to NUMERIC when CARRIER_ID is unknown

For some subscription, the carrier id is unknown (-1), which cause the inserted APN not be displayed. Fallback to NUMERIC when CARRIER_ID is unknown to fix this issue. Fix: 324394199 Test: manual with esim test profile - carrier id -1 Test: unit tests Change-Id: Ic63a3e0c9ab37c8bdf86a2c7155b08778f05deff
2024-02-18 16:26:15 +08:00
parent faba808e5e
commit e9917e2f48
3 changed files with 113 additions and 60 deletions
--- a/src/com/android/settings/network/apn/ApnRepository.kt
+++ b/src/com/android/settings/network/apn/ApnRepository.kt
@@ -20,6 +20,8 @@ import android.content.ContentValues
 import android.content.Context
 import android.net.Uri
 import android.provider.Telephony
+import android.telephony.SubscriptionManager
+import android.telephony.TelephonyManager
 import android.util.Log
 import com.android.settings.R
 import com.android.settingslib.utils.ThreadUtils
@@ -43,7 +45,6 @@ const val NETWORK_TYPE_INDEX = 15
 const val ROAMING_PROTOCOL_INDEX = 16
 const val EDITED_INDEX = 17
 const val USER_EDITABLE_INDEX = 18
-const val CARRIER_ID_INDEX = 19

 val sProjection = arrayOf(
    Telephony.Carriers._ID,  // 0
@@ -65,7 +66,6 @@ val sProjection = arrayOf(
    Telephony.Carriers.ROAMING_PROTOCOL,  // 16
    Telephony.Carriers.EDITED_STATUS,  // 17
    Telephony.Carriers.USER_EDITABLE,  // 18
-    Telephony.Carriers.CARRIER_ID // 19
 )

 const val TAG = "ApnRepository"
@@ -109,7 +109,6 @@ fun getApnDataFromUri(uri: Uri, context: Context): ApnData {

            val edited = cursor.getInt(EDITED_INDEX)
            val userEditable = cursor.getInt(USER_EDITABLE_INDEX)
-            val carrierId = cursor.getInt(CARRIER_ID_INDEX)

            apnData = apnData.copy(
                name = name,
@@ -130,7 +129,6 @@ fun getApnDataFromUri(uri: Uri, context: Context): ApnData {
                networkType = networkType,
                edited = edited,
                userEditable = userEditable,
-                carrierId = carrierId
            )
        }
    }
@@ -199,29 +197,44 @@ fun updateApnDataToDatabase(
    }
 }

+/** Not allowing add duplicated items, if the values of the following keys are all identical. */
+private val NonDuplicatedKeys = setOf(
+    Telephony.Carriers.APN,
+    Telephony.Carriers.PROXY,
+    Telephony.Carriers.PORT,
+    Telephony.Carriers.MMSC,
+    Telephony.Carriers.MMSPROXY,
+    Telephony.Carriers.MMSPORT,
+    Telephony.Carriers.PROTOCOL,
+    Telephony.Carriers.ROAMING_PROTOCOL,
+)
+
 fun isItemExist(apnData: ApnData, context: Context): String? {
-    var contentValueMap = apnData.getContentValueMap(context)
-    val removedList = arrayListOf(
-        Telephony.Carriers.NAME, Telephony.Carriers.USER,
-        Telephony.Carriers.SERVER, Telephony.Carriers.PASSWORD, Telephony.Carriers.AUTH_TYPE,
-        Telephony.Carriers.TYPE, Telephony.Carriers.NETWORK_TYPE_BITMASK,
-        Telephony.Carriers.CARRIER_ENABLED
-    )
-    contentValueMap =
-        contentValueMap.filterNot { removedList.contains(it.key) } as MutableMap<String, Any>
+    val contentValueMap = apnData.getContentValueMap(context).filterKeys { it in NonDuplicatedKeys }
    val list = contentValueMap.entries.toList()
    val selection = list.joinToString(" AND ") { "${it.key} = ?" }
    val selectionArgs: Array<String> = list.map { it.value.toString() }.toTypedArray()
    context.contentResolver.query(
-        Telephony.Carriers.CONTENT_URI,
-        sProjection,
-        selection /* selection */,
-        selectionArgs /* selectionArgs */,
-        null /* sortOrder */
+        Uri.withAppendedPath(Telephony.Carriers.SIM_APN_URI, apnData.subId.toString()),
+        /* projection = */ emptyArray(),
+        selection,
+        selectionArgs,
+        /* sortOrder = */ null,
    )?.use { cursor ->
        if (cursor.count > 0) {
            return context.resources.getString(R.string.error_duplicate_apn_entry)
        }
    }
    return null
-}
+}
+
+fun Context.getApnIdMap(subId: Int): Map<String, Any> {
+    val subInfo = getSystemService(SubscriptionManager::class.java)!!
+        .getActiveSubscriptionInfo(subId)
+    val carrierId = subInfo.carrierId
+    return if (carrierId != TelephonyManager.UNKNOWN_CARRIER_ID) {
+        mapOf(Telephony.Carriers.CARRIER_ID to carrierId)
+    } else {
+        mapOf(Telephony.Carriers.NUMERIC to subInfo.mccString + subInfo.mncString)
+    }.also { Log.d(TAG, "[$subId] New APN item with id: $it") }
+}
--- a/src/com/android/settings/network/apn/ApnStatus.kt
+++ b/src/com/android/settings/network/apn/ApnStatus.kt
@@ -22,13 +22,10 @@ import android.net.Uri
 import android.os.Bundle
 import android.provider.Telephony
 import android.telephony.CarrierConfigManager
-import android.telephony.TelephonyManager
 import android.text.TextUtils
 import android.util.Log
-import androidx.compose.runtime.snapshots.SnapshotStateList
 import com.android.internal.util.ArrayUtils
 import com.android.settings.R
-import com.android.settings.network.apn.ApnNetworkTypes.getNetworkType
 import com.android.settings.network.apn.ApnTypes.APN_TYPES
 import com.android.settings.network.apn.ApnTypes.APN_TYPE_ALL
 import com.android.settings.network.apn.ApnTypes.APN_TYPE_EMERGENCY
@@ -56,7 +53,6 @@ data class ApnData(
    val networkType: Long = 0,
    val edited: Int = Telephony.Carriers.USER_EDITED,
    val userEditable: Int = 1,
-    val carrierId: Int = TelephonyManager.UNKNOWN_CARRIER_ID,
    val nameEnabled: Boolean = true,
    val apnEnabled: Boolean = true,
    val proxyEnabled: Boolean = true,
@@ -78,34 +74,29 @@ data class ApnData(
    val validEnabled: Boolean = false,
    val customizedConfig: CustomizedConfig = CustomizedConfig()
 ) {
-    fun getContentValueMap(context: Context): MutableMap<String, Any> {
-        val simCarrierId =
-            context.getSystemService(TelephonyManager::class.java)!!
-                .createForSubscriptionId(subId)
-                .getSimCarrierId()
-        return mutableMapOf(
-            Telephony.Carriers.NAME to name, Telephony.Carriers.APN to apn,
-            Telephony.Carriers.PROXY to proxy, Telephony.Carriers.PORT to port,
-            Telephony.Carriers.MMSPROXY to mmsProxy, Telephony.Carriers.MMSPORT to mmsPort,
-            Telephony.Carriers.USER to userName, Telephony.Carriers.SERVER to server,
-            Telephony.Carriers.PASSWORD to passWord, Telephony.Carriers.MMSC to mmsc,
-            Telephony.Carriers.AUTH_TYPE to authType,
-            Telephony.Carriers.PROTOCOL to convertOptions2Protocol(apnProtocol, context),
-            Telephony.Carriers.ROAMING_PROTOCOL to convertOptions2Protocol(apnRoaming, context),
-            Telephony.Carriers.TYPE to apnType,
-            Telephony.Carriers.NETWORK_TYPE_BITMASK to networkType,
-            Telephony.Carriers.CARRIER_ENABLED to apnEnable,
-            Telephony.Carriers.EDITED_STATUS to Telephony.Carriers.USER_EDITED,
-            Telephony.Carriers.CARRIER_ID to simCarrierId
-        )
-    }
+    fun getContentValueMap(context: Context): Map<String, Any> = mapOf(
+        Telephony.Carriers.NAME to name,
+        Telephony.Carriers.APN to apn,
+        Telephony.Carriers.PROXY to proxy,
+        Telephony.Carriers.PORT to port,
+        Telephony.Carriers.USER to userName,
+        Telephony.Carriers.SERVER to server,
+        Telephony.Carriers.PASSWORD to passWord,
+        Telephony.Carriers.MMSC to mmsc,
+        Telephony.Carriers.MMSPROXY to mmsProxy,
+        Telephony.Carriers.MMSPORT to mmsPort,
+        Telephony.Carriers.AUTH_TYPE to authType,
+        Telephony.Carriers.PROTOCOL to convertOptions2Protocol(apnProtocol, context),
+        Telephony.Carriers.ROAMING_PROTOCOL to convertOptions2Protocol(apnRoaming, context),
+        Telephony.Carriers.TYPE to apnType,
+        Telephony.Carriers.NETWORK_TYPE_BITMASK to networkType,
+        Telephony.Carriers.CARRIER_ENABLED to apnEnable,
+        Telephony.Carriers.EDITED_STATUS to Telephony.Carriers.USER_EDITED,
+    )

-    fun getContentValues(context: Context): ContentValues {
-        val values = ContentValues()
-        val contentValueMap = getContentValueMap(context)
-        if (!newApn) contentValueMap.remove(Telephony.Carriers.CARRIER_ID)
-        contentValueMap.forEach { (key, value) -> values.putObject(key, value) }
-        return values
+    fun getContentValues(context: Context) = ContentValues().apply {
+        if (newApn) context.getApnIdMap(subId).forEach(::putObject)
+        getContentValueMap(context).forEach(::putObject)
    }
 }

--- a/tests/spa_unit/src/com/android/settings/network/apn/ApnRepositoryTest.kt
+++ b/tests/spa_unit/src/com/android/settings/network/apn/ApnRepositoryTest.kt
@@ -20,19 +20,39 @@ import android.content.ContentResolver
 import android.content.Context
 import android.database.MatrixCursor
 import android.net.Uri
+import android.provider.Telephony
+import android.telephony.SubscriptionInfo
+import android.telephony.SubscriptionManager
+import android.telephony.TelephonyManager
 import androidx.test.core.app.ApplicationProvider
 import androidx.test.ext.junit.runners.AndroidJUnit4
+import com.google.common.truth.Truth.assertThat
 import org.junit.Test
 import org.junit.runner.RunWith
-import org.mockito.Mockito
+import org.mockito.kotlin.doReturn
 import org.mockito.kotlin.mock
+import org.mockito.kotlin.spy
+import org.mockito.kotlin.stub
 import org.mockito.kotlin.whenever

@RunWith(AndroidJUnit4::class)
 class ApnRepositoryTest {

-    private val context: Context = ApplicationProvider.getApplicationContext()
-    private val mContentResolver = mock<ContentResolver> {}
+    private val contentResolver = mock<ContentResolver>()
+
+    private val mockSubscriptionInfo = mock<SubscriptionInfo> {
+        on { mccString } doReturn MCC
+        on { mncString } doReturn MNC
+    }
+
+    private val mockSubscriptionManager = mock<SubscriptionManager> {
+        on { getActiveSubscriptionInfo(SUB_ID) } doReturn mockSubscriptionInfo
+    }
+
+    private val context: Context = spy(ApplicationProvider.getApplicationContext()) {
+        on { contentResolver } doReturn contentResolver
+        on { getSystemService(SubscriptionManager::class.java) } doReturn mockSubscriptionManager
+    }
    private val uri = mock<Uri> {}

    @Test
@@ -40,7 +60,7 @@ class ApnRepositoryTest {
        // mock out resources and the feature provider
        val cursor = MatrixCursor(sProjection)
        cursor.addRow(
-            arrayOf<Any?>(
+            arrayOf<Any>(
                0,
                "name",
                "apn",
@@ -60,12 +80,41 @@ class ApnRepositoryTest {
                "apnRoaming",
                0,
                1,
-                0
            )
        )
-        val context = Mockito.spy(context)
-        whenever(context.contentResolver).thenReturn(mContentResolver)
-        whenever(mContentResolver.query(uri, sProjection, null, null, null)).thenReturn(cursor)
-        assert(getApnDataFromUri(uri, context).name == "name")
+        whenever(contentResolver.query(uri, sProjection, null, null, null)).thenReturn(cursor)
+
+        val apnData = getApnDataFromUri(uri, context)
+
+        assertThat(apnData.name).isEqualTo("name")
    }
-}
+
+    @Test
+    fun getApnIdMap_knownCarrierId() {
+        mockSubscriptionInfo.stub {
+            on { carrierId } doReturn CARRIER_ID
+        }
+
+        val idMap = context.getApnIdMap(SUB_ID)
+
+        assertThat(idMap).containsExactly(Telephony.Carriers.CARRIER_ID, CARRIER_ID)
+    }
+
+    @Test
+    fun getApnIdMap_unknownCarrierId() {
+        mockSubscriptionInfo.stub {
+            on { carrierId } doReturn TelephonyManager.UNKNOWN_CARRIER_ID
+        }
+
+        val idMap = context.getApnIdMap(SUB_ID)
+
+        assertThat(idMap).containsExactly(Telephony.Carriers.NUMERIC, MCC + MNC)
+    }
+
+    private companion object {
+        const val SUB_ID = 2
+        const val CARRIER_ID = 10
+        const val MCC = "310"
+        const val MNC = "101"
+    }
+}