Merge "Catch exception in telephonyRepository.isDataEnabledFlow" into main

src/com/android/settings/datausage/BillingCyclePreference.kt

@@ -26,11 +26,9 @@ import androidx.lifecycle.compose.collectAsStateWithLifecycle
 import com.android.settings.R
 import com.android.settings.core.SubSettingLauncher
 import com.android.settings.datausage.lib.BillingCycleRepository
-import com.android.settings.network.mobileDataEnabledFlow
 import com.android.settings.spa.preference.ComposePreference
 import com.android.settingslib.spa.widget.preference.Preference
 import com.android.settingslib.spa.widget.preference.PreferenceModel
-import kotlinx.coroutines.flow.map
 
 /**
  * Preference which displays billing cycle of subscription
@@ -46,8 +44,8 @@ class BillingCyclePreference @JvmOverloads constructor(
 
     override fun setTemplate(template: NetworkTemplate, subId: Int) {
         setContent {
-            val isModifiable by remember {
+            val isModifiable by remember(subId) {
-                context.mobileDataEnabledFlow(subId).map { repository.isModifiable(subId) }
+                repository.isModifiableFlow(subId)
             }.collectAsStateWithLifecycle(initialValue = false)
 
             Preference(object : PreferenceModel {

src/com/android/settings/datausage/DataUsageList.kt

@@ -35,7 +35,7 @@ import com.android.settings.datausage.lib.BillingCycleRepository
 import com.android.settings.datausage.lib.NetworkUsageData
 import com.android.settings.network.MobileNetworkRepository
 import com.android.settings.network.SubscriptionUtil
-import com.android.settings.network.mobileDataEnabledFlow
+import com.android.settings.network.telephony.requireSubscriptionManager
 import com.android.settingslib.mobile.dataservice.SubscriptionInfoEntity
 import com.android.settingslib.spa.framework.util.collectLatestWithLifecycle
 import com.android.settingslib.spaprivileged.framework.common.userManager
@@ -113,8 +113,8 @@ open class DataUsageList : DashboardFragment() {
     override fun onViewCreated(v: View, savedInstanceState: Bundle?) {
         super.onViewCreated(v, savedInstanceState)
 
-        requireContext().mobileDataEnabledFlow(subId)
+        billingCycleRepository.isModifiableFlow(subId)
-            .collectLatestWithLifecycle(viewLifecycleOwner) { updatePolicy() }
+            .collectLatestWithLifecycle(viewLifecycleOwner, action = ::updatePolicy)
 
         val template = template ?: return
         viewModel.templateFlow.value = template
@@ -163,16 +163,14 @@ open class DataUsageList : DashboardFragment() {
     }
 
     /** Update chart sweeps and cycle list to reflect [NetworkPolicy] for current [template]. */
-    private fun updatePolicy() {
+    private fun updatePolicy(isModifiable: Boolean) {
-        val isBillingCycleModifiable = isBillingCycleModifiable()
+        val isBillingCycleModifiable = isModifiable && isActiveSubscription()
         dataUsageListHeaderController?.setConfigButtonVisible(isBillingCycleModifiable)
         chartDataUsagePreferenceController?.setBillingCycleModifiable(isBillingCycleModifiable)
     }
 
-    private fun isBillingCycleModifiable(): Boolean =
+    private fun isActiveSubscription(): Boolean =
-        billingCycleRepository.isModifiable(subId) &&
+            requireContext().requireSubscriptionManager().getActiveSubscriptionInfo(subId) != null
-            requireContext().getSystemService(SubscriptionManager::class.java)!!
-                .getActiveSubscriptionInfo(subId) != null
 
     /**
      * Updates the chart and detail data when initial loaded or selected cycle changed.

src/com/android/settings/datausage/lib/BillingCycleRepository.kt

@@ -19,10 +19,15 @@ package com.android.settings.datausage.lib
 import android.content.Context
 import android.os.INetworkManagementService
 import android.os.ServiceManager
-import android.telephony.TelephonyManager
 import android.util.Log
 import androidx.annotation.OpenForTesting
+import com.android.settings.network.telephony.TelephonyRepository
 import com.android.settingslib.spaprivileged.framework.common.userManager
+import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.flow.Flow
+import kotlinx.coroutines.flow.conflate
+import kotlinx.coroutines.flow.flowOn
+import kotlinx.coroutines.flow.map
 
 @OpenForTesting
 open class BillingCycleRepository @JvmOverloads constructor(
@@ -31,12 +36,14 @@ open class BillingCycleRepository @JvmOverloads constructor(
         INetworkManagementService.Stub.asInterface(
             ServiceManager.getService(Context.NETWORKMANAGEMENT_SERVICE)
         ),
+    private val telephonyRepository: TelephonyRepository = TelephonyRepository(context),
 ) {
     private val userManager = context.userManager
-    private val telephonyManager = context.getSystemService(TelephonyManager::class.java)!!
 
-    fun isModifiable(subId: Int): Boolean =
+    fun isModifiableFlow(subId: Int): Flow<Boolean> =
-        isBandwidthControlEnabled() && userManager.isAdminUser && isDataEnabled(subId)
+        telephonyRepository.isDataEnabledFlow(subId).map { isDataEnabled ->
+            isDataEnabled && isBandwidthControlEnabled() && userManager.isAdminUser
+        }.conflate().flowOn(Dispatchers.Default)
 
     open fun isBandwidthControlEnabled(): Boolean = try {
         networkService.isBandwidthControlEnabled
@@ -45,10 +52,6 @@ open class BillingCycleRepository @JvmOverloads constructor(
         false
     }
 
-    private fun isDataEnabled(subId: Int): Boolean =
-        telephonyManager.createForSubscriptionId(subId)
-            .isDataEnabledForReason(TelephonyManager.DATA_ENABLED_REASON_USER)
-
     companion object {
         private const val TAG = "BillingCycleRepository"
     }

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

@@ -29,10 +29,12 @@ import kotlinx.coroutines.channels.ProducerScope
 import kotlinx.coroutines.channels.awaitClose
 import kotlinx.coroutines.flow.Flow
 import kotlinx.coroutines.flow.callbackFlow
+import kotlinx.coroutines.flow.catch
 import kotlinx.coroutines.flow.conflate
 import kotlinx.coroutines.flow.flowOf
 import kotlinx.coroutines.flow.flowOn
 import kotlinx.coroutines.flow.map
+import kotlinx.coroutines.flow.onEach
 
 class TelephonyRepository(
     private val context: Context,
@@ -64,19 +66,21 @@ class TelephonyRepository(
         telephonyManager.setMobileDataPolicyEnabled(policy, enabled)
     }
 
-    fun isDataEnabled(
+    fun isDataEnabledFlow(subId: Int): Flow<Boolean> {
-        subId: Int,
-    ): Flow<Boolean> {
         if (!SubscriptionManager.isValidSubscriptionId(subId)) return flowOf(false)
 
-        Log.d(TAG, "register mobileDataEnabledFlow: [$subId]")
         return context.mobileDataEnabledFlow(subId)
             .map {
-                Log.d(TAG, "mobileDataEnabledFlow: receive mobile data [$subId] start")
                 val telephonyManager = context.telephonyManager(subId)
                 telephonyManager.isDataEnabledForReason(TelephonyManager.DATA_ENABLED_REASON_USER)
-                    .also { Log.d(TAG, "mobileDataEnabledFlow: [$subId] isDataEnabled(): $it") }
             }
+            .catch {
+                Log.w(TAG, "[$subId] isDataEnabledFlow: exception", it)
+                emit(false)
+            }
+            .onEach { Log.d(TAG, "[$subId] isDataEnabledFlow: isDataEnabled() = $it") }
+            .conflate()
+            .flowOn(Dispatchers.Default)
     }
 
     fun setMobileData(
@@ -100,6 +104,7 @@ class TelephonyRepository(
             wifiPickerTrackerHelper.setCarrierNetworkEnabled(enabled)
         }
     }
+
     private companion object {
         private const val TAG = "TelephonyRepository"
     }

src/com/android/settings/spa/network/NetworkCellularGroupProvider.kt

@@ -36,11 +36,11 @@ import androidx.compose.runtime.rememberCoroutineScope
 import androidx.compose.runtime.saveable.rememberSaveable
 import androidx.compose.ui.graphics.vector.ImageVector
 import androidx.compose.ui.platform.LocalContext
-import androidx.compose.ui.platform.LocalLifecycleOwner
 import androidx.compose.ui.res.stringResource
 import androidx.compose.ui.res.vectorResource
 import androidx.lifecycle.LifecycleOwner
 import androidx.lifecycle.LifecycleRegistry
+import androidx.lifecycle.compose.LocalLifecycleOwner
 import androidx.lifecycle.compose.collectAsStateWithLifecycle
 import androidx.lifecycle.viewmodel.compose.viewModel
 import com.android.settings.R
@@ -62,7 +62,6 @@ import com.android.settingslib.spaprivileged.settingsprovider.settingsGlobalBool
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.flow.Flow
-import kotlinx.coroutines.flow.StateFlow
 import kotlinx.coroutines.flow.combine
 import kotlinx.coroutines.flow.conflate
 import kotlinx.coroutines.flow.flowOf
@@ -207,7 +206,7 @@ fun MobileDataSectionImpl(
         }.collectAsStateWithLifecycle(initialValue = null)
 
         val mobileDataStateChanged by remember(mobileDataSelectedId.intValue) {
-            TelephonyRepository(context).isDataEnabled(mobileDataSelectedId.intValue)
+            TelephonyRepository(context).isDataEnabledFlow(mobileDataSelectedId.intValue)
         }.collectAsStateWithLifecycle(initialValue = false)
         val coroutineScope = rememberCoroutineScope()
 

tests/spa_unit/src/com/android/settings/datausage/BillingCyclePreferenceTest.kt

@@ -27,6 +27,8 @@ import androidx.test.core.app.ApplicationProvider
 import androidx.test.ext.junit.runners.AndroidJUnit4
 import com.android.settings.R
 import com.android.settings.datausage.lib.BillingCycleRepository
+import kotlinx.coroutines.flow.emptyFlow
+import kotlinx.coroutines.flow.flowOf
 import org.junit.Rule
 import org.junit.Test
 import org.junit.runner.RunWith
@@ -39,7 +41,9 @@ class BillingCyclePreferenceTest {
     @get:Rule
     val composeTestRule = createComposeRule()
 
-    private val mockBillingCycleRepository = mock<BillingCycleRepository>()
+    private val mockBillingCycleRepository = mock<BillingCycleRepository> {
+        on { isModifiableFlow(SUB_ID) } doReturn emptyFlow()
+    }
 
     private val context: Context = ApplicationProvider.getApplicationContext()
 
@@ -56,7 +60,7 @@ class BillingCyclePreferenceTest {
     @Test
     fun setTemplate_modifiable_enabled() {
         mockBillingCycleRepository.stub {
-            on { isModifiable(SUB_ID) } doReturn true
+            on { isModifiableFlow(SUB_ID) } doReturn flowOf(true)
         }
 
         setTemplate()
@@ -67,7 +71,7 @@ class BillingCyclePreferenceTest {
     @Test
     fun setTemplate_notModifiable_notEnabled() {
         mockBillingCycleRepository.stub {
-            on { isModifiable(SUB_ID) } doReturn false
+            on { isModifiableFlow(SUB_ID) } doReturn flowOf(false)
         }
 
         setTemplate()

tests/spa_unit/src/com/android/settings/datausage/lib/BillingCycleRepositoryTest.kt

@@ -22,8 +22,10 @@ import android.os.UserManager
 import android.telephony.TelephonyManager
 import androidx.test.core.app.ApplicationProvider
 import androidx.test.ext.junit.runners.AndroidJUnit4
+import com.android.settingslib.spa.testutils.firstWithTimeoutOrNull
 import com.android.settingslib.spaprivileged.framework.common.userManager
 import com.google.common.truth.Truth.assertThat
+import kotlinx.coroutines.runBlocking
 import org.junit.Test
 import org.junit.runner.RunWith
 import org.mockito.kotlin.doReturn
@@ -55,43 +57,43 @@ class BillingCycleRepositoryTest {
     private val repository = BillingCycleRepository(context, mockNetworkManagementService)
 
     @Test
-    fun isModifiable_bandwidthControlDisabled_returnFalse() {
+    fun isModifiable_bandwidthControlDisabled_returnFalse() = runBlocking {
         whenever(mockNetworkManagementService.isBandwidthControlEnabled).thenReturn(false)
 
-        val modifiable = repository.isModifiable(SUB_ID)
+        val modifiable = repository.isModifiableFlow(SUB_ID).firstWithTimeoutOrNull()
 
         assertThat(modifiable).isFalse()
     }
 
     @Test
-    fun isModifiable_notAdminUser_returnFalse() {
+    fun isModifiable_notAdminUser_returnFalse() = runBlocking {
         whenever(mockUserManager.isAdminUser).thenReturn(false)
 
-        val modifiable = repository.isModifiable(SUB_ID)
+        val modifiable = repository.isModifiableFlow(SUB_ID).firstWithTimeoutOrNull()
 
         assertThat(modifiable).isFalse()
     }
 
     @Test
-    fun isModifiable_dataDisabled_returnFalse() {
+    fun isModifiable_dataDisabled_returnFalse() = runBlocking {
         whenever(
             mockTelephonyManager.isDataEnabledForReason(TelephonyManager.DATA_ENABLED_REASON_USER)
         ).thenReturn(false)
 
-        val modifiable = repository.isModifiable(SUB_ID)
+        val modifiable = repository.isModifiableFlow(SUB_ID).firstWithTimeoutOrNull()
 
         assertThat(modifiable).isFalse()
     }
 
     @Test
-    fun isModifiable_meetAllRequirements_returnTrue() {
+    fun isModifiable_meetAllRequirements_returnTrue() = runBlocking {
         whenever(mockNetworkManagementService.isBandwidthControlEnabled).thenReturn(true)
         whenever(mockUserManager.isAdminUser).thenReturn(true)
         whenever(
             mockTelephonyManager.isDataEnabledForReason(TelephonyManager.DATA_ENABLED_REASON_USER)
         ).thenReturn(true)
 
-        val modifiable = repository.isModifiable(SUB_ID)
+        val modifiable = repository.isModifiableFlow(SUB_ID).firstWithTimeoutOrNull()
 
         assertThat(modifiable).isTrue()
     }

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

@@ -93,7 +93,7 @@ class TelephonyRepositoryTest {
 
     @Test
     fun isDataEnabled_invalidSub_returnFalse() = runBlocking {
-        val state = repository.isDataEnabled(
+        val state = repository.isDataEnabledFlow(
             subId = SubscriptionManager.INVALID_SUBSCRIPTION_ID,
         )
 
@@ -108,9 +108,7 @@ class TelephonyRepositoryTest {
             } doReturn true
         }
 
-        val state = repository.isDataEnabled(
+        val state = repository.isDataEnabledFlow(subId = SUB_ID)
-            subId = SUB_ID,
-        )
 
         assertThat(state.firstWithTimeoutOrNull()).isTrue()
     }