[SB refactor] Upgrade most data sources to StateFlow

Most of the data model for mobile is more correctly represented as a StateFlow, since the state from callbacks needs to persist even when there is no corrent collector on the mobile icon. This change also exposes a current issue: toggling mobile data on/off multiple times can introduce an inconsistency in the UI. This is approximately the same as the old pipeline, which also experiences the same problem. Test: manual Test: atest packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/* Bug: 240492102 Change-Id: Idd5fa0205380b4ba15f7d723b32b08e114456f7f
2022-10-26 16:04:08 -04:00
parent be25e8f1f4
commit 93b945ce54
9 changed files with 112 additions and 77 deletions
--- a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/MobileConnectionRepository.kt
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/MobileConnectionRepository.kt
@@ -47,10 +47,12 @@ import kotlinx.coroutines.ExperimentalCoroutinesApi
 import kotlinx.coroutines.asExecutor
 import kotlinx.coroutines.channels.awaitClose
 import kotlinx.coroutines.flow.Flow
+import kotlinx.coroutines.flow.MutableSharedFlow
 import kotlinx.coroutines.flow.SharingStarted
 import kotlinx.coroutines.flow.StateFlow
-import kotlinx.coroutines.flow.map
+import kotlinx.coroutines.flow.mapLatest
 import kotlinx.coroutines.flow.merge
+import kotlinx.coroutines.flow.onEach
 import kotlinx.coroutines.flow.stateIn

 /**
@@ -71,12 +73,12 @@ interface MobileConnectionRepository {
     */
    val subscriptionModelFlow: Flow<MobileSubscriptionModel>
    /** Observable tracking [TelephonyManager.isDataConnectionAllowed] */
-    val dataEnabled: Flow<Boolean>
+    val dataEnabled: StateFlow<Boolean>
    /**
     * True if this connection represents the default subscription per
     * [SubscriptionManager.getDefaultDataSubscriptionId]
     */
-    val isDefaultDataSubscription: Flow<Boolean>
+    val isDefaultDataSubscription: StateFlow<Boolean>
 }

@Suppress("EXPERIMENTAL_IS_NOT_ENABLED")
@@ -86,7 +88,7 @@ class MobileConnectionRepositoryImpl(
    private val subId: Int,
    private val telephonyManager: TelephonyManager,
    private val globalSettings: GlobalSettings,
-    defaultDataSubId: Flow<Int>,
+    defaultDataSubId: StateFlow<Int>,
    globalMobileDataSettingChangedEvent: Flow<Unit>,
    bgDispatcher: CoroutineDispatcher,
    logger: ConnectivityPipelineLogger,
@@ -101,6 +103,8 @@ class MobileConnectionRepositoryImpl(
        }
    }

+    private val telephonyCallbackEvent = MutableSharedFlow<Unit>(extraBufferCapacity = 1)
+
    override val subscriptionModelFlow: StateFlow<MobileSubscriptionModel> = run {
        var state = MobileSubscriptionModel()
        conflatedCallbackFlow {
@@ -180,12 +184,11 @@ class MobileConnectionRepositoryImpl(
                telephonyManager.registerTelephonyCallback(bgDispatcher.asExecutor(), callback)
                awaitClose { telephonyManager.unregisterTelephonyCallback(callback) }
            }
+            .onEach { telephonyCallbackEvent.tryEmit(Unit) }
            .logOutputChange(logger, "MobileSubscriptionModel")
            .stateIn(scope, SharingStarted.WhileSubscribed(), state)
    }

-    private val telephonyCallbackEvent = subscriptionModelFlow.map {}
-
    /** Produces whenever the mobile data setting changes for this subId */
    private val localMobileDataSettingChangedEvent: Flow<Unit> = conflatedCallbackFlow {
        val observer =
@@ -216,11 +219,17 @@ class MobileConnectionRepositoryImpl(
            globalMobileDataSettingChangedEvent,
        )

-    override val dataEnabled: Flow<Boolean> = telephonyPollingEvent.map { dataConnectionAllowed() }
+    override val dataEnabled: StateFlow<Boolean> =
+        telephonyPollingEvent
+            .mapLatest { dataConnectionAllowed() }
+            .stateIn(scope, SharingStarted.WhileSubscribed(), dataConnectionAllowed())

    private fun dataConnectionAllowed(): Boolean = telephonyManager.isDataConnectionAllowed

-    override val isDefaultDataSubscription: Flow<Boolean> = defaultDataSubId.map { it == subId }
+    override val isDefaultDataSubscription: StateFlow<Boolean> =
+        defaultDataSubId
+            .mapLatest { it == subId }
+            .stateIn(scope, SharingStarted.WhileSubscribed(), defaultDataSubId.value == subId)

    class Factory
    @Inject
@@ -234,7 +243,7 @@ class MobileConnectionRepositoryImpl(
    ) {
        fun build(
            subId: Int,
-            defaultDataSubId: Flow<Int>,
+            defaultDataSubId: StateFlow<Int>,
            globalMobileDataSettingChangedEvent: Flow<Unit>,
        ): MobileConnectionRepository {
            return MobileConnectionRepositoryImpl(
--- a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/MobileConnectionsRepository.kt
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/MobileConnectionsRepository.kt
@@ -65,7 +65,7 @@ interface MobileConnectionsRepository {
    val subscriptionsFlow: Flow<List<SubscriptionInfo>>

    /** Observable for the subscriptionId of the current mobile data connection */
-    val activeMobileDataSubscriptionId: Flow<Int>
+    val activeMobileDataSubscriptionId: StateFlow<Int>

    /** Observable for [MobileMappings.Config] tracking the defaults */
    val defaultDataSubRatConfig: StateFlow<Config>
--- a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/UserSetupRepository.kt
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/UserSetupRepository.kt
@@ -26,7 +26,6 @@ import kotlinx.coroutines.CoroutineDispatcher
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.ExperimentalCoroutinesApi
 import kotlinx.coroutines.channels.awaitClose
-import kotlinx.coroutines.flow.Flow
 import kotlinx.coroutines.flow.SharingStarted
 import kotlinx.coroutines.flow.StateFlow
 import kotlinx.coroutines.flow.mapLatest
@@ -40,7 +39,7 @@ import kotlinx.coroutines.withContext
 */
 interface UserSetupRepository {
    /** Observable tracking [DeviceProvisionedController.isUserSetup] */
-    val isUserSetupFlow: Flow<Boolean>
+    val isUserSetupFlow: StateFlow<Boolean>
 }

@Suppress("EXPERIMENTAL_IS_NOT_ENABLED")
--- a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/domain/interactor/MobileIconInteractor.kt
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/domain/interactor/MobileIconInteractor.kt
@@ -18,81 +18,97 @@ package com.android.systemui.statusbar.pipeline.mobile.domain.interactor

 import android.telephony.CarrierConfigManager
 import com.android.settingslib.SignalIcon.MobileIconGroup
+import com.android.systemui.dagger.qualifiers.Application
 import com.android.systemui.statusbar.pipeline.mobile.data.model.DefaultNetworkType
 import com.android.systemui.statusbar.pipeline.mobile.data.model.OverrideNetworkType
 import com.android.systemui.statusbar.pipeline.mobile.data.repository.MobileConnectionRepository
 import com.android.systemui.statusbar.pipeline.mobile.util.MobileMappingsProxy
 import com.android.systemui.util.CarrierConfigTracker
+import kotlinx.coroutines.CoroutineScope
+import kotlinx.coroutines.ExperimentalCoroutinesApi
 import kotlinx.coroutines.flow.Flow
+import kotlinx.coroutines.flow.MutableStateFlow
+import kotlinx.coroutines.flow.SharingStarted
+import kotlinx.coroutines.flow.StateFlow
 import kotlinx.coroutines.flow.combine
-import kotlinx.coroutines.flow.flowOf
-import kotlinx.coroutines.flow.map
+import kotlinx.coroutines.flow.mapLatest
+import kotlinx.coroutines.flow.stateIn

 interface MobileIconInteractor {
    // TODO(b/256839546): clarify naming of default vs active
    /** True if we want to consider the data connection enabled */
-    val isDefaultDataEnabled: Flow<Boolean>
+    val isDefaultDataEnabled: StateFlow<Boolean>

    /** Observable for the data enabled state of this connection */
-    val isDataEnabled: Flow<Boolean>
+    val isDataEnabled: StateFlow<Boolean>

    /** Observable for RAT type (network type) indicator */
-    val networkTypeIconGroup: Flow<MobileIconGroup>
+    val networkTypeIconGroup: StateFlow<MobileIconGroup>

    /** True if this line of service is emergency-only */
-    val isEmergencyOnly: Flow<Boolean>
+    val isEmergencyOnly: StateFlow<Boolean>

    /** Int describing the connection strength. 0-4 OR 1-5. See [numberOfLevels] */
-    val level: Flow<Int>
+    val level: StateFlow<Int>

    /** Based on [CarrierConfigManager.KEY_INFLATE_SIGNAL_STRENGTH_BOOL], either 4 or 5 */
-    val numberOfLevels: Flow<Int>
+    val numberOfLevels: StateFlow<Int>
 }

 /** Interactor for a single mobile connection. This connection _should_ have one subscription ID */
+@Suppress("EXPERIMENTAL_IS_NOT_ENABLED")
+@OptIn(ExperimentalCoroutinesApi::class)
 class MobileIconInteractorImpl(
-    defaultSubscriptionHasDataEnabled: Flow<Boolean>,
-    defaultMobileIconMapping: Flow<Map<String, MobileIconGroup>>,
-    defaultMobileIconGroup: Flow<MobileIconGroup>,
+    @Application scope: CoroutineScope,
+    defaultSubscriptionHasDataEnabled: StateFlow<Boolean>,
+    defaultMobileIconMapping: StateFlow<Map<String, MobileIconGroup>>,
+    defaultMobileIconGroup: StateFlow<MobileIconGroup>,
    mobileMappingsProxy: MobileMappingsProxy,
    connectionRepository: MobileConnectionRepository,
 ) : MobileIconInteractor {
    private val mobileStatusInfo = connectionRepository.subscriptionModelFlow

-    override val isDataEnabled: Flow<Boolean> = connectionRepository.dataEnabled
+    override val isDataEnabled: StateFlow<Boolean> = connectionRepository.dataEnabled

    override val isDefaultDataEnabled = defaultSubscriptionHasDataEnabled

    /** Observable for the current RAT indicator icon ([MobileIconGroup]) */
-    override val networkTypeIconGroup: Flow<MobileIconGroup> =
+    override val networkTypeIconGroup: StateFlow<MobileIconGroup> =
        combine(
-            mobileStatusInfo,
-            defaultMobileIconMapping,
-            defaultMobileIconGroup,
-        ) { info, mapping, defaultGroup ->
-            val lookupKey =
-                when (val resolved = info.resolvedNetworkType) {
-                    is DefaultNetworkType -> mobileMappingsProxy.toIconKey(resolved.type)
-                    is OverrideNetworkType -> mobileMappingsProxy.toIconKeyOverride(resolved.type)
-                }
-            mapping[lookupKey] ?: defaultGroup
-        }
-
-    override val isEmergencyOnly: Flow<Boolean> = mobileStatusInfo.map { it.isEmergencyOnly }
-
-    override val level: Flow<Int> =
-        mobileStatusInfo.map { mobileModel ->
-            // TODO: incorporate [MobileMappings.Config.alwaysShowCdmaRssi]
-            if (mobileModel.isGsm) {
-                mobileModel.primaryLevel
-            } else {
-                mobileModel.cdmaLevel
+                mobileStatusInfo,
+                defaultMobileIconMapping,
+                defaultMobileIconGroup,
+            ) { info, mapping, defaultGroup ->
+                val lookupKey =
+                    when (val resolved = info.resolvedNetworkType) {
+                        is DefaultNetworkType -> mobileMappingsProxy.toIconKey(resolved.type)
+                        is OverrideNetworkType ->
+                            mobileMappingsProxy.toIconKeyOverride(resolved.type)
+                    }
+                mapping[lookupKey] ?: defaultGroup
            }
-        }
+            .stateIn(scope, SharingStarted.WhileSubscribed(), defaultMobileIconGroup.value)
+
+    override val isEmergencyOnly: StateFlow<Boolean> =
+        mobileStatusInfo
+            .mapLatest { it.isEmergencyOnly }
+            .stateIn(scope, SharingStarted.WhileSubscribed(), false)
+
+    override val level: StateFlow<Int> =
+        mobileStatusInfo
+            .mapLatest { mobileModel ->
+                // TODO: incorporate [MobileMappings.Config.alwaysShowCdmaRssi]
+                if (mobileModel.isGsm) {
+                    mobileModel.primaryLevel
+                } else {
+                    mobileModel.cdmaLevel
+                }
+            }
+            .stateIn(scope, SharingStarted.WhileSubscribed(), 0)

    /**
     * This will become variable based on [CarrierConfigManager.KEY_INFLATE_SIGNAL_STRENGTH_BOOL]
     * once it's wired up inside of [CarrierConfigTracker]
     */
-    override val numberOfLevels: Flow<Int> = flowOf(4)
+    override val numberOfLevels: StateFlow<Int> = MutableStateFlow(4)
 }
--- a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/domain/interactor/MobileIconsInteractor.kt
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/domain/interactor/MobileIconsInteractor.kt
@@ -38,7 +38,7 @@ import kotlinx.coroutines.flow.StateFlow
 import kotlinx.coroutines.flow.combine
 import kotlinx.coroutines.flow.flatMapLatest
 import kotlinx.coroutines.flow.flowOf
-import kotlinx.coroutines.flow.map
+import kotlinx.coroutines.flow.mapLatest
 import kotlinx.coroutines.flow.stateIn

 /**
@@ -55,13 +55,13 @@ interface MobileIconsInteractor {
    /** List of subscriptions, potentially filtered for CBRS */
    val filteredSubscriptions: Flow<List<SubscriptionInfo>>
    /** True if the active mobile data subscription has data enabled */
-    val activeDataConnectionHasDataEnabled: Flow<Boolean>
+    val activeDataConnectionHasDataEnabled: StateFlow<Boolean>
    /** The icon mapping from network type to [MobileIconGroup] for the default subscription */
-    val defaultMobileIconMapping: Flow<Map<String, MobileIconGroup>>
+    val defaultMobileIconMapping: StateFlow<Map<String, MobileIconGroup>>
    /** Fallback [MobileIconGroup] in the case where there is no icon in the mapping */
-    val defaultMobileIconGroup: Flow<MobileIconGroup>
+    val defaultMobileIconGroup: StateFlow<MobileIconGroup>
    /** True once the user has been set up */
-    val isUserSetup: Flow<Boolean>
+    val isUserSetup: StateFlow<Boolean>
    /**
     * Vends out a [MobileIconInteractor] tracking the [MobileConnectionRepository] for the given
     * subId. Will throw if the ID is invalid
@@ -84,17 +84,21 @@ constructor(
    private val activeMobileDataSubscriptionId =
        mobileConnectionsRepo.activeMobileDataSubscriptionId

-    private val activeMobileDataConnectionRepo: Flow<MobileConnectionRepository?> =
-        activeMobileDataSubscriptionId.map { activeId ->
-            if (activeId == INVALID_SUBSCRIPTION_ID) {
-                null
-            } else {
-                mobileConnectionsRepo.getRepoForSubId(activeId)
+    private val activeMobileDataConnectionRepo: StateFlow<MobileConnectionRepository?> =
+        activeMobileDataSubscriptionId
+            .mapLatest { activeId ->
+                if (activeId == INVALID_SUBSCRIPTION_ID) {
+                    null
+                } else {
+                    mobileConnectionsRepo.getRepoForSubId(activeId)
+                }
            }
-        }
+            .stateIn(scope, SharingStarted.WhileSubscribed(), null)

-    override val activeDataConnectionHasDataEnabled: Flow<Boolean> =
-        activeMobileDataConnectionRepo.flatMapLatest { it?.dataEnabled ?: flowOf(false) }
+    override val activeDataConnectionHasDataEnabled: StateFlow<Boolean> =
+        activeMobileDataConnectionRepo
+            .flatMapLatest { it?.dataEnabled ?: flowOf(false) }
+            .stateIn(scope, SharingStarted.WhileSubscribed(), false)

    private val unfilteredSubscriptions: Flow<List<SubscriptionInfo>> =
        mobileConnectionsRepo.subscriptionsFlow
@@ -149,20 +153,21 @@ constructor(
     */
    override val defaultMobileIconMapping: StateFlow<Map<String, MobileIconGroup>> =
        mobileConnectionsRepo.defaultDataSubRatConfig
-            .map { mobileMappingsProxy.mapIconSets(it) }
+            .mapLatest { mobileMappingsProxy.mapIconSets(it) }
            .stateIn(scope, SharingStarted.WhileSubscribed(), initialValue = mapOf())

    /** If there is no mapping in [defaultMobileIconMapping], then use this default icon group */
    override val defaultMobileIconGroup: StateFlow<MobileIconGroup> =
        mobileConnectionsRepo.defaultDataSubRatConfig
-            .map { mobileMappingsProxy.getDefaultIcons(it) }
+            .mapLatest { mobileMappingsProxy.getDefaultIcons(it) }
            .stateIn(scope, SharingStarted.WhileSubscribed(), initialValue = TelephonyIcons.G)

-    override val isUserSetup: Flow<Boolean> = userSetupRepo.isUserSetupFlow
+    override val isUserSetup: StateFlow<Boolean> = userSetupRepo.isUserSetupFlow

    /** Vends out new [MobileIconInteractor] for a particular subId */
    override fun createMobileConnectionInteractorForSubId(subId: Int): MobileIconInteractor =
        MobileIconInteractorImpl(
+            scope,
            activeDataConnectionHasDataEnabled,
            defaultMobileIconMapping,
            defaultMobileIconGroup,
--- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/FakeMobileConnectionRepository.kt
+++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/FakeMobileConnectionRepository.kt
@@ -17,12 +17,11 @@
 package com.android.systemui.statusbar.pipeline.mobile.data.repository

 import com.android.systemui.statusbar.pipeline.mobile.data.model.MobileSubscriptionModel
-import kotlinx.coroutines.flow.Flow
 import kotlinx.coroutines.flow.MutableStateFlow

 class FakeMobileConnectionRepository : MobileConnectionRepository {
    private val _subscriptionsModelFlow = MutableStateFlow(MobileSubscriptionModel())
-    override val subscriptionModelFlow: Flow<MobileSubscriptionModel> = _subscriptionsModelFlow
+    override val subscriptionModelFlow = _subscriptionsModelFlow

    private val _dataEnabled = MutableStateFlow(true)
    override val dataEnabled = _dataEnabled
--- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/FakeUserSetupRepository.kt
+++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/FakeUserSetupRepository.kt
@@ -16,13 +16,12 @@

 package com.android.systemui.statusbar.pipeline.mobile.data.repository

-import kotlinx.coroutines.flow.Flow
 import kotlinx.coroutines.flow.MutableStateFlow

 /** Defaults to `true` */
 class FakeUserSetupRepository : UserSetupRepository {
    private val _isUserSetup: MutableStateFlow<Boolean> = MutableStateFlow(true)
-    override val isUserSetupFlow: Flow<Boolean> = _isUserSetup
+    override val isUserSetupFlow = _isUserSetup

    fun setUserSetup(setup: Boolean) {
        _isUserSetup.value = setup
--- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/MobileConnectionRepositoryTest.kt
+++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/MobileConnectionRepositoryTest.kt
@@ -300,14 +300,20 @@ class MobileConnectionRepositoryTest : SysuiTestCase() {
        }

    @Test
-    fun dataEnabled_isEnabled() =
+    fun dataEnabled_initial_false() =
        runBlocking(IMMEDIATE) {
            whenever(telephonyManager.isDataConnectionAllowed).thenReturn(true)

-            var latest: Boolean? = null
-            val job = underTest.dataEnabled.onEach { latest = it }.launchIn(this)
+            assertThat(underTest.dataEnabled.value).isFalse()
+        }

-            assertThat(latest).isTrue()
+    @Test
+    fun dataEnabled_isEnabled_true() =
+        runBlocking(IMMEDIATE) {
+            whenever(telephonyManager.isDataConnectionAllowed).thenReturn(true)
+            val job = underTest.dataEnabled.launchIn(this)
+
+            assertThat(underTest.dataEnabled.value).isTrue()

            job.cancel()
        }
@@ -316,11 +322,9 @@ class MobileConnectionRepositoryTest : SysuiTestCase() {
    fun dataEnabled_isDisabled() =
        runBlocking(IMMEDIATE) {
            whenever(telephonyManager.isDataConnectionAllowed).thenReturn(false)
+            val job = underTest.dataEnabled.launchIn(this)

-            var latest: Boolean? = null
-            val job = underTest.dataEnabled.onEach { latest = it }.launchIn(this)
-
-            assertThat(latest).isFalse()
+            assertThat(underTest.dataEnabled.value).isFalse()

            job.cancel()
        }
--- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/domain/interactor/MobileIconInteractorTest.kt
+++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/domain/interactor/MobileIconInteractorTest.kt
@@ -34,6 +34,7 @@ import com.android.systemui.statusbar.pipeline.mobile.util.FakeMobileMappingsPro
 import com.android.systemui.util.mockito.mock
 import com.android.systemui.util.mockito.whenever
 import com.google.common.truth.Truth.assertThat
+import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.flow.launchIn
 import kotlinx.coroutines.flow.onEach
@@ -49,10 +50,13 @@ class MobileIconInteractorTest : SysuiTestCase() {
    private val mobileIconsInteractor = FakeMobileIconsInteractor(mobileMappingsProxy)
    private val connectionRepository = FakeMobileConnectionRepository()

+    private val scope = CoroutineScope(IMMEDIATE)
+
    @Before
    fun setUp() {
        underTest =
            MobileIconInteractorImpl(
+                scope,
                mobileIconsInteractor.activeDataConnectionHasDataEnabled,
                mobileIconsInteractor.defaultMobileIconMapping,
                mobileIconsInteractor.defaultMobileIconGroup,