From c754540d7cac1518adbdba0b6cfe30185e7a3246 Mon Sep 17 00:00:00 2001 From: Evan Laird Date: Mon, 13 Mar 2023 12:31:07 -0400 Subject: [PATCH] [Sb refactor] address code comments Address code comments from changes: Ieaf30071cf2a0082dc005dfc8e8332c0a7456df6 Id57cc4047d9290581e6b8eae813136ebfb8eb3be Test: atest SystemUITests Bug: 272784824 Change-Id: I3f3349129a39d754081749bd08ceb65ca57e4e1c --- .../prod/MobileConnectionRepositoryImpl.kt | 12 ++++-- .../prod/MobileConnectionRepositoryTest.kt | 38 ++++++++----------- .../MobileConnectionTelephonySmokeTests.kt | 15 ++++---- .../repository/prod/MobileTelephonyHelpers.kt | 7 ++++ 4 files changed, 38 insertions(+), 34 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionRepositoryImpl.kt b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionRepositoryImpl.kt index f866d65e2d2d6..d0c6215a55d8f 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionRepositoryImpl.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionRepositoryImpl.kt @@ -105,10 +105,14 @@ class MobileConnectionRepositoryImpl( * The reason we need to do this is because TelephonyManager limits the number of registered * listeners per-process, so we don't want to create a new listener for every callback. * - * A note on the design for back pressure here: We use the [coalesce] operator here to change - * the backpressure strategy to store exactly the last callback event of _each type_ here, as - * opposed to the default strategy which is to drop the oldest event (regardless of type). This - * means that we should never miss any single event as long as the flow has been started. + * A note on the design for back pressure here: We don't control _which_ telephony callback + * comes in first, since we register every relevant bit of information as a batch. E.g., if a + * downstream starts collecting on a field which is backed by + * [TelephonyCallback.ServiceStateListener], it's not possible for us to guarantee that _that_ + * callback comes in -- the first callback could very well be + * [TelephonyCallback.DataActivityListener], which would promptly be dropped if we didn't keep + * it tracked. We use the [scan] operator here to track the most recent callback of _each type_ + * here. See [TelephonyCallbackState] to see how the callbacks are stored. */ private val callbackEvents: StateFlow = run { val initial = TelephonyCallbackState() diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionRepositoryTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionRepositoryTest.kt index 542b688b162d4..934e1c64c6da3 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionRepositoryTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionRepositoryTest.kt @@ -25,7 +25,6 @@ import android.telephony.ServiceState.STATE_OUT_OF_SERVICE import android.telephony.TelephonyCallback import android.telephony.TelephonyCallback.DataActivityListener import android.telephony.TelephonyCallback.ServiceStateListener -import android.telephony.TelephonyDisplayInfo import android.telephony.TelephonyDisplayInfo.OVERRIDE_NETWORK_TYPE_LTE_CA import android.telephony.TelephonyDisplayInfo.OVERRIDE_NETWORK_TYPE_NONE import android.telephony.TelephonyManager @@ -68,6 +67,7 @@ import com.android.systemui.statusbar.pipeline.mobile.data.model.toNetworkNameMo import com.android.systemui.statusbar.pipeline.mobile.data.repository.FakeMobileConnectionsRepository import com.android.systemui.statusbar.pipeline.mobile.data.repository.MobileConnectionRepository.Companion.DEFAULT_NUM_LEVELS import com.android.systemui.statusbar.pipeline.mobile.data.repository.prod.MobileTelephonyHelpers.signalStrength +import com.android.systemui.statusbar.pipeline.mobile.data.repository.prod.MobileTelephonyHelpers.telephonyDisplayInfo import com.android.systemui.statusbar.pipeline.mobile.util.FakeMobileMappingsProxy import com.android.systemui.statusbar.pipeline.shared.data.model.DataActivityModel import com.android.systemui.statusbar.pipeline.shared.data.model.toMobileDataActivityModel @@ -392,13 +392,17 @@ class MobileConnectionRepositoryTest : SysuiTestCase() { val job = underTest.resolvedNetworkType.onEach { latest = it }.launchIn(this) val callback = getTelephonyCallbackForType() - val type = NETWORK_TYPE_UNKNOWN - val expected = UnknownNetworkType - val ti = mock().also { whenever(it.networkType).thenReturn(type) } + val ti = + telephonyDisplayInfo( + networkType = NETWORK_TYPE_UNKNOWN, + overrideNetworkType = NETWORK_TYPE_UNKNOWN, + ) + callback.onDisplayInfoChanged(ti) + val expected = UnknownNetworkType assertThat(latest).isEqualTo(expected) - assertThat(latest!!.lookupKey).isEqualTo(MobileMappings.toIconKey(type)) + assertThat(latest!!.lookupKey).isEqualTo(MobileMappings.toIconKey(NETWORK_TYPE_UNKNOWN)) job.cancel() } @@ -412,14 +416,10 @@ class MobileConnectionRepositoryTest : SysuiTestCase() { val callback = getTelephonyCallbackForType() val overrideType = OVERRIDE_NETWORK_TYPE_NONE val type = NETWORK_TYPE_LTE - val expected = DefaultNetworkType(mobileMappings.toIconKey(type)) - val ti = - mock().also { - whenever(it.overrideNetworkType).thenReturn(overrideType) - whenever(it.networkType).thenReturn(type) - } + val ti = telephonyDisplayInfo(networkType = type, overrideNetworkType = overrideType) callback.onDisplayInfoChanged(ti) + val expected = DefaultNetworkType(mobileMappings.toIconKey(type)) assertThat(latest).isEqualTo(expected) job.cancel() @@ -433,14 +433,10 @@ class MobileConnectionRepositoryTest : SysuiTestCase() { val callback = getTelephonyCallbackForType() val type = OVERRIDE_NETWORK_TYPE_LTE_CA - val expected = OverrideNetworkType(mobileMappings.toIconKeyOverride(type)) - val ti = - mock().also { - whenever(it.networkType).thenReturn(type) - whenever(it.overrideNetworkType).thenReturn(type) - } + val ti = telephonyDisplayInfo(networkType = type, overrideNetworkType = type) callback.onDisplayInfoChanged(ti) + val expected = OverrideNetworkType(mobileMappings.toIconKeyOverride(type)) assertThat(latest).isEqualTo(expected) job.cancel() @@ -455,14 +451,10 @@ class MobileConnectionRepositoryTest : SysuiTestCase() { val callback = getTelephonyCallbackForType() val unknown = NETWORK_TYPE_UNKNOWN val type = OVERRIDE_NETWORK_TYPE_LTE_CA - val expected = OverrideNetworkType(mobileMappings.toIconKeyOverride(type)) - val ti = - mock().also { - whenever(it.networkType).thenReturn(unknown) - whenever(it.overrideNetworkType).thenReturn(type) - } + val ti = telephonyDisplayInfo(unknown, type) callback.onDisplayInfoChanged(ti) + val expected = OverrideNetworkType(mobileMappings.toIconKeyOverride(type)) assertThat(latest).isEqualTo(expected) job.cancel() diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionTelephonySmokeTests.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionTelephonySmokeTests.kt index bbf04ed28fd7a..9da9ff72d3800 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionTelephonySmokeTests.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionTelephonySmokeTests.kt @@ -64,10 +64,14 @@ import org.mockito.MockitoAnnotations * * Kind of like an interaction test case build just for [TelephonyCallback] * - * The list of telephony callbacks we use is: [TelephonyCallback.CarrierNetworkListener] - * [TelephonyCallback.DataActivityListener] [TelephonyCallback.DataConnectionStateListener] - * [TelephonyCallback.DataEnabledListener] [TelephonyCallback.DisplayInfoListener] - * [TelephonyCallback.ServiceStateListener] [TelephonyCallback.SignalStrengthsListener] + * The list of telephony callbacks we use is: + * - [TelephonyCallback.CarrierNetworkListener] + * - [TelephonyCallback.DataActivityListener] + * - [TelephonyCallback.DataConnectionStateListener] + * - [TelephonyCallback.DataEnabledListener] + * - [TelephonyCallback.DisplayInfoListener] + * - [TelephonyCallback.ServiceStateListener] + * - [TelephonyCallback.SignalStrengthsListener] * * Because each of these callbacks comes in on the same callbackFlow, collecting on a field backed * by only a single callback can immediately create backpressure on the other fields related to a @@ -201,7 +205,6 @@ class MobileConnectionTelephonySmokeTests : SysuiTestCase() { 200 /* unused */ ) - // Send a bunch of events that we don't care about, to overrun the replay buffer flipActivity(100, activityCallback) val connectionJob = underTest.dataConnectionState.onEach { latest = it }.launchIn(this) @@ -225,7 +228,6 @@ class MobileConnectionTelephonySmokeTests : SysuiTestCase() { enabledCallback.onDataEnabledChanged(true, 1 /* unused */) - // Send a bunch of events that we don't care about, to overrun the replay buffer flipActivity(100, activityCallback) val job = underTest.dataEnabled.onEach { latest = it }.launchIn(this) @@ -252,7 +254,6 @@ class MobileConnectionTelephonySmokeTests : SysuiTestCase() { val ti = mock().also { whenever(it.networkType).thenReturn(type) } displayInfoCallback.onDisplayInfoChanged(ti) - // Send a bunch of events that we don't care about, to overrun the replay buffer flipActivity(100, activityCallback) val job = underTest.resolvedNetworkType.onEach { latest = it }.launchIn(this) diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileTelephonyHelpers.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileTelephonyHelpers.kt index d07b96f6609e7..cf815c27a0bf2 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileTelephonyHelpers.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileTelephonyHelpers.kt @@ -19,6 +19,7 @@ package com.android.systemui.statusbar.pipeline.mobile.data.repository.prod import android.telephony.CellSignalStrengthCdma import android.telephony.SignalStrength import android.telephony.TelephonyCallback +import android.telephony.TelephonyDisplayInfo import android.telephony.TelephonyManager import com.android.systemui.util.mockito.any import com.android.systemui.util.mockito.argumentCaptor @@ -48,6 +49,12 @@ object MobileTelephonyHelpers { return signalStrength } + fun telephonyDisplayInfo(networkType: Int, overrideNetworkType: Int) = + mock().also { + whenever(it.networkType).thenReturn(networkType) + whenever(it.overrideNetworkType).thenReturn(overrideNetworkType) + } + inline fun getTelephonyCallbackForType(mockTelephonyManager: TelephonyManager): T { val cbs = getTelephonyCallbacks(mockTelephonyManager).filterIsInstance() assertThat(cbs.size).isEqualTo(1)