From ef12f1ddb56dd2e7c7793676d049e09e76942ed3 Mon Sep 17 00:00:00 2001 From: Chaohui Wang Date: Fri, 21 Jun 2024 11:30:36 +0800 Subject: [PATCH] New CarrierConfigRepository Benefices, - Gets the configuration values of the specified keys, for better performance - Check key suffix for correctness - Support cache - If CarrierConfigManager throw exception, use default value Bug: 337417520 Flag: EXEMPT refactor Test: manual on Sim Status Test: unit Change-Id: I68f41ef66d495080f628794ade63cf807efba619 --- .../simstatus/SimStatusDialogRepository.kt | 37 ++- .../telephony/CarrierConfigManagerExt.kt | 1 + .../telephony/CarrierConfigRepository.kt | 217 ++++++++++++++++++ .../SimStatusDialogRepositoryTest.kt | 87 +++---- .../telephony/CarrierConfigRepositoryTest.kt | 138 +++++++++++ 5 files changed, 419 insertions(+), 61 deletions(-) create mode 100644 src/com/android/settings/network/telephony/CarrierConfigRepository.kt create mode 100644 tests/spa_unit/src/com/android/settings/network/telephony/CarrierConfigRepositoryTest.kt diff --git a/src/com/android/settings/deviceinfo/simstatus/SimStatusDialogRepository.kt b/src/com/android/settings/deviceinfo/simstatus/SimStatusDialogRepository.kt index 5ed6993cc98..760f8b698f5 100644 --- a/src/com/android/settings/deviceinfo/simstatus/SimStatusDialogRepository.kt +++ b/src/com/android/settings/deviceinfo/simstatus/SimStatusDialogRepository.kt @@ -23,10 +23,10 @@ import androidx.lifecycle.Lifecycle import androidx.lifecycle.LifecycleOwner import androidx.lifecycle.lifecycleScope import androidx.lifecycle.repeatOnLifecycle +import com.android.settings.network.telephony.CarrierConfigRepository import com.android.settings.network.telephony.SimSlotRepository import com.android.settings.network.telephony.ims.ImsMmTelRepository import com.android.settings.network.telephony.ims.ImsMmTelRepositoryImpl -import com.android.settings.network.telephony.safeGetConfig import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.flow.Flow @@ -39,7 +39,9 @@ import kotlinx.coroutines.flow.flowOn import kotlinx.coroutines.launch @OptIn(ExperimentalCoroutinesApi::class) -class SimStatusDialogRepository @JvmOverloads constructor( +class SimStatusDialogRepository +@JvmOverloads +constructor( private val context: Context, private val simSlotRepository: SimSlotRepository = SimSlotRepository(context), private val signalStrengthRepository: SignalStrengthRepository = @@ -48,7 +50,7 @@ class SimStatusDialogRepository @JvmOverloads constructor( ImsMmTelRepositoryImpl(context, subId) }, ) { - private val carrierConfigManager = context.getSystemService(CarrierConfigManager::class.java)!! + private val carrierConfigRepository = CarrierConfigRepository(context) data class SimStatusDialogInfo( val signalStrength: String? = null, @@ -73,7 +75,8 @@ class SimStatusDialogRepository @JvmOverloads constructor( } private fun simStatusDialogInfoBySlotFlow(simSlotIndex: Int): Flow = - simSlotRepository.subIdInSimSlotFlow(simSlotIndex) + simSlotRepository + .subIdInSimSlotFlow(simSlotIndex) .flatMapLatest { subId -> if (SubscriptionManager.isValidSubscriptionId(subId)) { simStatusDialogInfoFlow(subId) @@ -99,22 +102,16 @@ class SimStatusDialogRepository @JvmOverloads constructor( } private fun showUpFlow(subId: Int) = flow { - val config = carrierConfigManager.safeGetConfig( - keys = listOf( - CarrierConfigManager.KEY_SHOW_SIGNAL_STRENGTH_IN_SIM_STATUS_BOOL, - CarrierConfigManager.KEY_SHOW_IMS_REGISTRATION_STATUS_BOOL, - ), - subId = subId, - ) - val visibility = SimStatusDialogVisibility( - signalStrengthShowUp = config.getBoolean( - CarrierConfigManager.KEY_SHOW_SIGNAL_STRENGTH_IN_SIM_STATUS_BOOL, - true, // by default we show the signal strength in sim status - ), - imsRegisteredShowUp = config.getBoolean( - CarrierConfigManager.KEY_SHOW_IMS_REGISTRATION_STATUS_BOOL - ), - ) + val visibility = + carrierConfigRepository.transformConfig(subId) { + SimStatusDialogVisibility( + signalStrengthShowUp = + getBoolean( + CarrierConfigManager.KEY_SHOW_SIGNAL_STRENGTH_IN_SIM_STATUS_BOOL), + imsRegisteredShowUp = + getBoolean(CarrierConfigManager.KEY_SHOW_IMS_REGISTRATION_STATUS_BOOL), + ) + } emit(visibility) } } diff --git a/src/com/android/settings/network/telephony/CarrierConfigManagerExt.kt b/src/com/android/settings/network/telephony/CarrierConfigManagerExt.kt index 05b4c07c53e..5408ab04e77 100644 --- a/src/com/android/settings/network/telephony/CarrierConfigManagerExt.kt +++ b/src/com/android/settings/network/telephony/CarrierConfigManagerExt.kt @@ -24,6 +24,7 @@ import androidx.core.os.persistableBundleOf /** * Gets the configuration values of the specified config keys applied. */ +@Deprecated("Use CarrierConfigRepository instead") fun CarrierConfigManager.safeGetConfig( keys: List, subId: Int = SubscriptionManager.getDefaultSubscriptionId(), diff --git a/src/com/android/settings/network/telephony/CarrierConfigRepository.kt b/src/com/android/settings/network/telephony/CarrierConfigRepository.kt new file mode 100644 index 00000000000..3ec529dbe82 --- /dev/null +++ b/src/com/android/settings/network/telephony/CarrierConfigRepository.kt @@ -0,0 +1,217 @@ +/* + * Copyright (C) 2024 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.settings.network.telephony + +import android.content.Context +import android.os.PersistableBundle +import android.telephony.CarrierConfigManager +import android.telephony.SubscriptionManager +import android.util.Log +import androidx.annotation.VisibleForTesting +import java.util.concurrent.ConcurrentHashMap +import kotlinx.atomicfu.atomic +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.asExecutor + +class CarrierConfigRepository(private val context: Context) { + + private val carrierConfigManager: CarrierConfigManager? = + context.getSystemService(CarrierConfigManager::class.java) + + private enum class KeyType { + BOOLEAN, + INT, + STRING + } + + interface CarrierConfigAccessor { + fun getBoolean(key: String): Boolean + + fun getInt(key: String): Int + + fun getString(key: String): String? + } + + private class Accessor(private val cache: ConfigCache) : CarrierConfigAccessor { + private val keysToRetrieve = mutableMapOf() + + override fun getBoolean(key: String): Boolean { + check(key.endsWith("_bool")) { "Boolean key should ends with _bool" } + val value = cache[key] + return if (value == null) { + keysToRetrieve += key to KeyType.BOOLEAN + DefaultConfig.getBoolean(key) + } else { + check(value is BooleanConfigValue) { "Boolean value type wrong" } + value.value + } + } + + override fun getInt(key: String): Int { + check(key.endsWith("_int")) { "Int key should ends with _int" } + val value = cache[key] + return if (value == null) { + keysToRetrieve += key to KeyType.INT + DefaultConfig.getInt(key) + } else { + check(value is IntConfigValue) { "Int value type wrong" } + value.value + } + } + + override fun getString(key: String): String? { + check(key.endsWith("_string")) { "String key should ends with _string" } + val value = cache[key] + return if (value == null) { + keysToRetrieve += key to KeyType.STRING + DefaultConfig.getString(key) + } else { + check(value is StringConfigValue) { "String value type wrong" } + value.value + } + } + + fun getKeysToRetrieve(): Map = keysToRetrieve + } + + /** + * Gets the configuration values for the given [subId]. + * + * Configuration values could be accessed in [block]. Note: [block] could be called multiple + * times, so it should be pure function without side effort. + */ + fun transformConfig(subId: Int, block: CarrierConfigAccessor.() -> T): T { + val perSubCache = getPerSubCache(subId) + val accessor = Accessor(perSubCache) + val result = accessor.block() + val keysToRetrieve = accessor.getKeysToRetrieve() + // If all keys found in the first pass, no need to collect again + if (keysToRetrieve.isEmpty()) return result + + perSubCache.update(subId, keysToRetrieve) + + return accessor.block() + } + + /** Gets the configuration boolean for the given [subId] and [key]. */ + fun getBoolean(subId: Int, key: String): Boolean = transformConfig(subId) { getBoolean(key) } + + /** Gets the configuration int for the given [subId] and [key]. */ + fun getInt(subId: Int, key: String): Int = transformConfig(subId) { getInt(key) } + + /** Gets the configuration string for the given [subId] and [key]. */ + fun getString(subId: Int, key: String): String? = transformConfig(subId) { getString(key) } + + private fun ConfigCache.update(subId: Int, keysToRetrieve: Map) { + val config = safeGetConfig(subId, keysToRetrieve.keys) ?: return + for ((key, type) in keysToRetrieve) { + when (type) { + KeyType.BOOLEAN -> this[key] = BooleanConfigValue(config.getBoolean(key)) + KeyType.INT -> this[key] = IntConfigValue(config.getInt(key)) + KeyType.STRING -> this[key] = StringConfigValue(config.getString(key)) + } + } + } + + /** Gets the configuration values of the specified config keys applied. */ + private fun safeGetConfig(subId: Int, keys: Collection): PersistableBundle? { + if (carrierConfigManager == null || !SubscriptionManager.isValidSubscriptionId(subId)) { + return null + } + tryRegisterListener(context) + return try { + carrierConfigManager.getConfigForSubId(subId, *keys.toTypedArray()) + } catch (e: Exception) { + Log.e(TAG, "safeGetConfig: exception", e) + // The CarrierConfigLoader (the service implemented the CarrierConfigManager) hasn't + // been initialized yet. This may occurs during very early phase of phone booting up + // or when Phone process has been restarted. + // Settings should not assume Carrier config loader (and any other system services + // as well) are always available. If not available, use default value instead. + null + } + } + + companion object { + private const val TAG = "CarrierConfigRepository" + + private val DefaultConfig = CarrierConfigManager.getDefaultConfig() + + /** Cache of config values for each subscription. */ + private val Cache = ConcurrentHashMap() + + private fun getPerSubCache(subId: Int) = + Cache.computeIfAbsent(subId) { ConcurrentHashMap() } + + /** To make sure the registerCarrierConfigChangeListener is only called once. */ + private val ListenerRegistered = atomic(false) + + private fun tryRegisterListener(context: Context) { + if (ListenerRegistered.compareAndSet(expect = false, update = true)) { + val carrierConfigManager = + context.applicationContext.getSystemService(CarrierConfigManager::class.java) + if (carrierConfigManager != null) { + carrierConfigManager.registerCarrierConfigChangeListener() + } else { + ListenerRegistered.getAndSet(false) + } + } + } + + private fun CarrierConfigManager.registerCarrierConfigChangeListener() { + val executor = Dispatchers.Default.asExecutor() + registerCarrierConfigChangeListener(executor) { _, subId, _, _ -> + Log.d(TAG, "[$subId] onCarrierConfigChanged") + Cache.remove(subId) + } + } + + @VisibleForTesting + fun resetForTest() { + Cache.clear() + ListenerRegistered.getAndSet(false) + } + + @VisibleForTesting + fun setBooleanForTest(subId: Int, key: String, value: Boolean) { + check(key.endsWith("_bool")) { "Boolean key should ends with _bool" } + getPerSubCache(subId)[key] = BooleanConfigValue(value) + } + + @VisibleForTesting + fun setIntForTest(subId: Int, key: String, value: Int) { + check(key.endsWith("_int")) { "Int key should ends with _int" } + getPerSubCache(subId)[key] = IntConfigValue(value) + } + + @VisibleForTesting + fun setStringForTest(subId: Int, key: String, value: String) { + check(key.endsWith("_string")) { "String key should ends with _string" } + getPerSubCache(subId)[key] = StringConfigValue(value) + } + } +} + +private sealed interface ConfigValue + +private data class BooleanConfigValue(val value: Boolean) : ConfigValue + +private data class IntConfigValue(val value: Int) : ConfigValue + +private data class StringConfigValue(val value: String?) : ConfigValue + +private typealias ConfigCache = ConcurrentHashMap diff --git a/tests/spa_unit/src/com/android/settings/deviceinfo/simstatus/SimStatusDialogRepositoryTest.kt b/tests/spa_unit/src/com/android/settings/deviceinfo/simstatus/SimStatusDialogRepositoryTest.kt index 01f32bfccf0..1c1d9df79ca 100644 --- a/tests/spa_unit/src/com/android/settings/deviceinfo/simstatus/SimStatusDialogRepositoryTest.kt +++ b/tests/spa_unit/src/com/android/settings/deviceinfo/simstatus/SimStatusDialogRepositoryTest.kt @@ -17,65 +17,65 @@ package com.android.settings.deviceinfo.simstatus import android.content.Context -import android.os.PersistableBundle import android.telephony.CarrierConfigManager import androidx.lifecycle.testing.TestLifecycleOwner import androidx.test.core.app.ApplicationProvider import androidx.test.ext.junit.runners.AndroidJUnit4 import com.android.settings.deviceinfo.simstatus.SimStatusDialogRepository.SimStatusDialogInfo +import com.android.settings.network.telephony.CarrierConfigRepository import com.android.settings.network.telephony.SimSlotRepository import com.android.settings.network.telephony.ims.ImsMmTelRepository import com.google.common.truth.Truth.assertThat import kotlinx.coroutines.delay import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.runBlocking +import org.junit.Before import org.junit.Test import org.junit.runner.RunWith -import org.mockito.kotlin.anyVararg import org.mockito.kotlin.doReturn -import org.mockito.kotlin.eq import org.mockito.kotlin.mock -import org.mockito.kotlin.spy @RunWith(AndroidJUnit4::class) class SimStatusDialogRepositoryTest { - private val carrierConfig = PersistableBundle().apply { - putBoolean(CarrierConfigManager.KEY_SHOW_IMS_REGISTRATION_STATUS_BOOL, true) - } + private val context: Context = ApplicationProvider.getApplicationContext() - private val mockCarrierConfigManager = mock { - on { getConfigForSubId(eq(SUB_ID), anyVararg()) } doReturn carrierConfig - } + private val mockSimSlotRepository = + mock { + on { subIdInSimSlotFlow(SIM_SLOT_INDEX) } doReturn flowOf(SUB_ID) + } - private val context: Context = spy(ApplicationProvider.getApplicationContext()) { - on { getSystemService(CarrierConfigManager::class.java) } doReturn mockCarrierConfigManager - } + private val mockSignalStrengthRepository = + mock { + on { signalStrengthDisplayFlow(SUB_ID) } doReturn flowOf(SIGNAL_STRENGTH) + } - private val mockSimSlotRepository = mock { - on { subIdInSimSlotFlow(SIM_SLOT_INDEX) } doReturn flowOf(SUB_ID) - } + private val mockImsMmTelRepository = + mock { on { imsRegisteredFlow() } doReturn flowOf(true) } - private val mockSignalStrengthRepository = mock { - on { signalStrengthDisplayFlow(SUB_ID) } doReturn flowOf(SIGNAL_STRENGTH) - } + private val controller = + SimStatusDialogRepository( + context = context, + simSlotRepository = mockSimSlotRepository, + signalStrengthRepository = mockSignalStrengthRepository, + imsMmTelRepositoryFactory = { subId -> + assertThat(subId).isEqualTo(SUB_ID) + mockImsMmTelRepository + }, + ) - private val mockImsMmTelRepository = mock { - on { imsRegisteredFlow() } doReturn flowOf(true) + @Before + fun setUp() { + CarrierConfigRepository.resetForTest() } - private val controller = SimStatusDialogRepository( - context = context, - simSlotRepository = mockSimSlotRepository, - signalStrengthRepository = mockSignalStrengthRepository, - imsMmTelRepositoryFactory = { subId -> - assertThat(subId).isEqualTo(SUB_ID) - mockImsMmTelRepository - }, - ) - @Test fun collectSimStatusDialogInfo() = runBlocking { + CarrierConfigRepository.setBooleanForTest( + subId = SUB_ID, + key = CarrierConfigManager.KEY_SHOW_IMS_REGISTRATION_STATUS_BOOL, + value = true, + ) var simStatusDialogInfo = SimStatusDialogInfo() controller.collectSimStatusDialogInfo(TestLifecycleOwner(), SIM_SLOT_INDEX) { @@ -83,19 +83,20 @@ class SimStatusDialogRepositoryTest { } delay(100) - assertThat(simStatusDialogInfo).isEqualTo( - SimStatusDialogInfo( - signalStrength = SIGNAL_STRENGTH, - imsRegistered = true, - ) - ) + assertThat(simStatusDialogInfo) + .isEqualTo( + SimStatusDialogInfo( + signalStrength = SIGNAL_STRENGTH, + imsRegistered = true, + )) } @Test fun collectSimStatusDialogInfo_doNotShowSignalStrength() = runBlocking { - carrierConfig.putBoolean( - CarrierConfigManager.KEY_SHOW_SIGNAL_STRENGTH_IN_SIM_STATUS_BOOL, - false + CarrierConfigRepository.setBooleanForTest( + subId = SUB_ID, + key = CarrierConfigManager.KEY_SHOW_SIGNAL_STRENGTH_IN_SIM_STATUS_BOOL, + value = false, ) var simStatusDialogInfo = SimStatusDialogInfo() @@ -109,7 +110,11 @@ class SimStatusDialogRepositoryTest { @Test fun collectSimStatusDialogInfo_doNotShowImsRegistration() = runBlocking { - carrierConfig.putBoolean(CarrierConfigManager.KEY_SHOW_IMS_REGISTRATION_STATUS_BOOL, false) + CarrierConfigRepository.setBooleanForTest( + subId = SUB_ID, + key = CarrierConfigManager.KEY_SHOW_IMS_REGISTRATION_STATUS_BOOL, + value = false, + ) var simStatusDialogInfo = SimStatusDialogInfo() controller.collectSimStatusDialogInfo(TestLifecycleOwner(), SIM_SLOT_INDEX) { diff --git a/tests/spa_unit/src/com/android/settings/network/telephony/CarrierConfigRepositoryTest.kt b/tests/spa_unit/src/com/android/settings/network/telephony/CarrierConfigRepositoryTest.kt new file mode 100644 index 00000000000..8c54751da35 --- /dev/null +++ b/tests/spa_unit/src/com/android/settings/network/telephony/CarrierConfigRepositoryTest.kt @@ -0,0 +1,138 @@ +/* + * Copyright (C) 2024 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.settings.network.telephony + +import android.content.Context +import android.telephony.CarrierConfigManager +import androidx.core.os.persistableBundleOf +import androidx.test.ext.junit.runners.AndroidJUnit4 +import com.google.common.truth.Truth.assertThat +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.mockito.kotlin.any +import org.mockito.kotlin.anyVararg +import org.mockito.kotlin.doReturn +import org.mockito.kotlin.doThrow +import org.mockito.kotlin.eq +import org.mockito.kotlin.mock +import org.mockito.kotlin.stub +import org.mockito.kotlin.times +import org.mockito.kotlin.verify + +@RunWith(AndroidJUnit4::class) +class CarrierConfigRepositoryTest { + + private val mockCarrierConfigManager = mock() + + private val context = + mock { + on { applicationContext } doReturn mock + on { getSystemService(CarrierConfigManager::class.java) } doReturn + mockCarrierConfigManager + } + + private val repository = CarrierConfigRepository(context) + + @Before + fun setUp() { + CarrierConfigRepository.resetForTest() + } + + @Test + fun getBoolean_returnValue() { + val key = CarrierConfigManager.KEY_CARRIER_CONFIG_APPLIED_BOOL + mockCarrierConfigManager.stub { + on { getConfigForSubId(any(), eq(key)) } doReturn persistableBundleOf(key to true) + } + + val value = repository.getBoolean(SUB_ID, key) + + assertThat(value).isTrue() + } + + @Test + fun getInt_returnValue() { + val key = CarrierConfigManager.KEY_GBA_MODE_INT + mockCarrierConfigManager.stub { + on { getConfigForSubId(any(), eq(key)) } doReturn persistableBundleOf(key to 99) + } + + val value = repository.getInt(SUB_ID, key) + + assertThat(value).isEqualTo(99) + } + + @Test + fun getString_returnValue() { + val key = CarrierConfigManager.KEY_CARRIER_NAME_STRING + mockCarrierConfigManager.stub { + on { getConfigForSubId(any(), eq(key)) } doReturn + persistableBundleOf(key to STRING_VALUE) + } + + val value = repository.getString(SUB_ID, key) + + assertThat(value).isEqualTo(STRING_VALUE) + } + + @Test + fun transformConfig_managerThrowIllegalStateException_returnDefaultValue() { + mockCarrierConfigManager.stub { + on { getConfigForSubId(any(), anyVararg()) } doThrow IllegalStateException() + } + + val carrierName = + repository.transformConfig(SUB_ID) { + getInt(CarrierConfigManager.KEY_CARRIER_DEFAULT_WFC_IMS_MODE_INT) + } + + assertThat(carrierName) + .isEqualTo( + CarrierConfigManager.getDefaultConfig() + .getInt(CarrierConfigManager.KEY_CARRIER_DEFAULT_WFC_IMS_MODE_INT)) + } + + @Test + fun transformConfig_getValueTwice_cached() { + val key = CarrierConfigManager.KEY_CARRIER_NAME_STRING + mockCarrierConfigManager.stub { + on { getConfigForSubId(any(), eq(key)) } doReturn + persistableBundleOf(key to STRING_VALUE) + } + + repository.transformConfig(SUB_ID) { getString(key) } + repository.transformConfig(SUB_ID) { getString(key) } + + verify(mockCarrierConfigManager, times(1)).getConfigForSubId(any(), anyVararg()) + } + + @Test + fun transformConfig_registerCarrierConfigChangeListener() { + val key = CarrierConfigManager.KEY_CARRIER_NAME_STRING + + repository.transformConfig(SUB_ID) { getString(key) } + repository.transformConfig(SUB_ID) { getString(key) } + + verify(mockCarrierConfigManager, times(1)).registerCarrierConfigChangeListener(any(), any()) + } + + private companion object { + const val SUB_ID = 123 + const val STRING_VALUE = "value" + } +}