From 2faad6df6ee4a5bcd067c076326af19017f59def Mon Sep 17 00:00:00 2001 From: Chaohui Wang Date: Fri, 24 May 2024 15:56:06 +0800 Subject: [PATCH] Check APN type is selected before save Not allow save APN without type. Fix: 340969975 Test: manual - APN Settings Test: unit test Change-Id: I91f8e8137fc234c9c860fd446b7a1e2acfa11fc7 --- res/values/strings.xml | 2 + .../android/settings/network/apn/ApnStatus.kt | 19 +++---- .../settings/network/apn/ApnTypeCheckBox.kt | 3 ++ .../android/settings/network/apn/ApnTypes.kt | 2 + .../settings/network/apn/ApnTypesTest.kt | 52 +++++++++++++++++++ 5 files changed, 65 insertions(+), 13 deletions(-) diff --git a/res/values/strings.xml b/res/values/strings.xml index 56b0a41d370..5f739f9ac20 100644 --- a/res/values/strings.xml +++ b/res/values/strings.xml @@ -3408,6 +3408,8 @@ The Name field can\u2019t be empty. The APN can\u2019t be empty. + + The APN type can\u2019t be empty. MCC field must be 3 digits. diff --git a/src/com/android/settings/network/apn/ApnStatus.kt b/src/com/android/settings/network/apn/ApnStatus.kt index aa816fc8a62..2f41f238d38 100644 --- a/src/com/android/settings/network/apn/ApnStatus.kt +++ b/src/com/android/settings/network/apn/ApnStatus.kt @@ -174,20 +174,13 @@ fun validateAndSaveApnData( * @return An error message if the apn data is invalid, otherwise return null. */ fun validateApnData(apnData: ApnData, context: Context): String? { - var errorMsg: String? - val name = apnData.name - val apn = apnData.apn - errorMsg = if (name == "") { - context.resources.getString(R.string.error_name_empty) - } else if (apn == "") { - context.resources.getString(R.string.error_apn_empty) - } else { - validateMMSC(true, apnData.mmsc, context) + val errorMsg: String? = when { + apnData.name.isEmpty() -> context.resources.getString(R.string.error_name_empty) + apnData.apn.isEmpty() -> context.resources.getString(R.string.error_apn_empty) + apnData.apnType.isEmpty() -> context.resources.getString(R.string.error_apn_type_empty) + else -> validateMMSC(true, apnData.mmsc, context) ?: isItemExist(apnData, context) } - if (errorMsg == null) { - errorMsg = isItemExist(apnData, context) - } - return errorMsg?.apply { Log.d(TAG, "APN data not valid, reason: $this") } + return errorMsg?.also { Log.d(TAG, "APN data not valid, reason: $it") } } /** diff --git a/src/com/android/settings/network/apn/ApnTypeCheckBox.kt b/src/com/android/settings/network/apn/ApnTypeCheckBox.kt index aa757cc9513..f4e73fbe2f4 100644 --- a/src/com/android/settings/network/apn/ApnTypeCheckBox.kt +++ b/src/com/android/settings/network/apn/ApnTypeCheckBox.kt @@ -24,6 +24,7 @@ import androidx.compose.runtime.remember import androidx.compose.ui.platform.LocalContext import androidx.compose.ui.res.stringResource import com.android.settings.R +import com.android.settings.network.apn.ApnTypes.isValid import com.android.settings.network.apn.ApnTypes.toApnType import com.android.settingslib.spa.widget.editor.SettingsDropdownCheckBox @@ -47,6 +48,8 @@ fun ApnTypeCheckBox( label = stringResource(R.string.apn_type), options = apnTypeOptions, enabled = apnData.isFieldEnabled(Telephony.Carriers.TYPE), + errorMessage = stringResource(R.string.error_apn_type_empty) + .takeUnless { apnTypeOptions.isValid() }, ) { onTypeChanged(apnTypeOptions.toApnType()) updateMmsSelected() diff --git a/src/com/android/settings/network/apn/ApnTypes.kt b/src/com/android/settings/network/apn/ApnTypes.kt index d384625a149..b9bc480cc35 100644 --- a/src/com/android/settings/network/apn/ApnTypes.kt +++ b/src/com/android/settings/network/apn/ApnTypes.kt @@ -101,6 +101,8 @@ object ApnTypes { } } + fun List.isValid(): Boolean = any { it.selected.value } + fun List.toApnType(): String { val (selectAllOptions, regularOptions) = partition { it.isSelectAll } for (selectAllOption in selectAllOptions) { diff --git a/tests/spa_unit/src/com/android/settings/network/apn/ApnTypesTest.kt b/tests/spa_unit/src/com/android/settings/network/apn/ApnTypesTest.kt index 95471b0d4cc..ce0d0f55ffd 100644 --- a/tests/spa_unit/src/com/android/settings/network/apn/ApnTypesTest.kt +++ b/tests/spa_unit/src/com/android/settings/network/apn/ApnTypesTest.kt @@ -17,7 +17,11 @@ package com.android.settings.network.apn import android.telephony.data.ApnSetting +import androidx.compose.runtime.mutableStateOf import androidx.test.ext.junit.runners.AndroidJUnit4 +import com.android.settings.network.apn.ApnTypes.isValid +import com.android.settings.network.apn.ApnTypes.toApnType +import com.android.settingslib.spa.widget.editor.SettingsDropdownCheckOption import com.google.common.truth.Truth.assertThat import org.junit.Test import org.junit.runner.RunWith @@ -25,6 +29,50 @@ import org.junit.runner.RunWith @RunWith(AndroidJUnit4::class) class ApnTypesTest { + @Test + fun isValid_hasSelected() { + val options = listOf( + SettingsDropdownCheckOption(text = APN_TYPE, selected = mutableStateOf(true)), + ) + + val isValid = options.isValid() + + assertThat(isValid).isTrue() + } + + @Test + fun isValid_hasNotSelected() { + val options = listOf( + SettingsDropdownCheckOption(text = APN_TYPE, selected = mutableStateOf(false)), + ) + + val isValid = options.isValid() + + assertThat(isValid).isFalse() + } + + @Test + fun toApnType_hasSelected() { + val options = listOf( + SettingsDropdownCheckOption(text = APN_TYPE, selected = mutableStateOf(true)), + ) + + val apnType = options.toApnType() + + assertThat(apnType).isEqualTo(APN_TYPE) + } + + @Test + fun toApnType_hasNotSelected() { + val options = listOf( + SettingsDropdownCheckOption(text = APN_TYPE, selected = mutableStateOf(false)), + ) + + val apnType = options.toApnType() + + assertThat(apnType).isEmpty() + } + @Test fun getPreSelectedApnType_regular() { val apnType = ApnTypes.getPreSelectedApnType(CustomizedConfig()) @@ -42,4 +90,8 @@ class ApnTypesTest { assertThat(apnType).isEqualTo("default,mms,supl,hipri,fota,cbs,xcap") } + + private companion object { + const val APN_TYPE = "type" + } }