diff options
| author | 2024-09-05 16:36:50 +0000 | |
|---|---|---|
| committer | 2024-09-09 19:39:08 +0000 | |
| commit | 1a5478a70933cb1fc04fb16dc42b76da6dbf4d38 (patch) | |
| tree | e706786f30702c14dd18b39f25aa72803537a3f2 | |
| parent | ae8b2ea9df6932dfd1ec27681e4e873fb80db047 (diff) | |
[SB][Wifi] More protection against invalid values from WifiTrackerLib.
This change creates new factory methods for WifiNetworkModel.Active and
WifiNetworkModel.CarrierMerged that will ensure the parameters are valid
before creating the object. This means that WifiRepositoryImpl doesn't
need to save the level or subscription ID to local variables.
Most of the diffs in this change is converting all WifiNetworkModel
clients to use the factory method instead of the normal constructor (the
normal constructor is now private).
Bug: 362384551
Flag: EXEMPT bugfix
Test: atest WifiRepositoryImplTest
Change-Id: Id295c2ecb15bc7a1931661b887a25ca4dd993927
19 files changed, 331 insertions, 179 deletions
diff --git a/packages/SystemUI/multivalentTests/src/com/android/systemui/qs/tiles/impl/internet/domain/interactor/InternetTileDataInteractorTest.kt b/packages/SystemUI/multivalentTests/src/com/android/systemui/qs/tiles/impl/internet/domain/interactor/InternetTileDataInteractorTest.kt index 8e61c849179d..82b26db54693 100644 --- a/packages/SystemUI/multivalentTests/src/com/android/systemui/qs/tiles/impl/internet/domain/interactor/InternetTileDataInteractorTest.kt +++ b/packages/SystemUI/multivalentTests/src/com/android/systemui/qs/tiles/impl/internet/domain/interactor/InternetTileDataInteractorTest.kt @@ -184,7 +184,7 @@ class InternetTileDataInteractorTest : SysuiTestCase() { ) val networkModel = - WifiNetworkModel.Active( + WifiNetworkModel.Active.of( level = 4, ssid = "test ssid", ) @@ -219,7 +219,7 @@ class InternetTileDataInteractorTest : SysuiTestCase() { ) val networkModel = - WifiNetworkModel.Active( + WifiNetworkModel.Active.of( level = 4, ssid = "test ssid", hotspotDeviceType = WifiNetworkModel.HotspotDeviceType.NONE, @@ -398,7 +398,7 @@ class InternetTileDataInteractorTest : SysuiTestCase() { collectLastValue( underTest.tileData(testUser, flowOf(DataUpdateTrigger.InitialRequest)) ) - val networkModel = WifiNetworkModel.Inactive + val networkModel = WifiNetworkModel.Inactive() connectivityRepository.setWifiConnected(validated = false) wifiRepository.setIsWifiDefault(true) @@ -416,7 +416,7 @@ class InternetTileDataInteractorTest : SysuiTestCase() { underTest.tileData(testUser, flowOf(DataUpdateTrigger.InitialRequest)) ) - val networkModel = WifiNetworkModel.Inactive + val networkModel = WifiNetworkModel.Inactive() connectivityRepository.setWifiConnected(validated = false) wifiRepository.setIsWifiDefault(true) @@ -543,7 +543,7 @@ class InternetTileDataInteractorTest : SysuiTestCase() { private fun setWifiNetworkWithHotspot(hotspot: WifiNetworkModel.HotspotDeviceType) { val networkModel = - WifiNetworkModel.Active( + WifiNetworkModel.Active.of( level = 4, ssid = "test ssid", hotspotDeviceType = hotspot, diff --git a/packages/SystemUI/multivalentTests/src/com/android/systemui/statusbar/pipeline/wifi/domain/interactor/WifiInteractorImplTest.kt b/packages/SystemUI/multivalentTests/src/com/android/systemui/statusbar/pipeline/wifi/domain/interactor/WifiInteractorImplTest.kt index b9ca8fc2d181..c0a15922642e 100644 --- a/packages/SystemUI/multivalentTests/src/com/android/systemui/statusbar/pipeline/wifi/domain/interactor/WifiInteractorImplTest.kt +++ b/packages/SystemUI/multivalentTests/src/com/android/systemui/statusbar/pipeline/wifi/domain/interactor/WifiInteractorImplTest.kt @@ -78,7 +78,7 @@ class WifiInteractorImplTest : SysuiTestCase() { @Test fun ssid_inactiveNetwork_outputsNull() = testScope.runTest { - wifiRepository.setWifiNetwork(WifiNetworkModel.Inactive) + wifiRepository.setWifiNetwork(WifiNetworkModel.Inactive()) var latest: String? = "default" val job = underTest.ssid.onEach { latest = it }.launchIn(this) @@ -93,7 +93,7 @@ class WifiInteractorImplTest : SysuiTestCase() { fun ssid_carrierMergedNetwork_outputsNull() = testScope.runTest { wifiRepository.setWifiNetwork( - WifiNetworkModel.CarrierMerged(subscriptionId = 2, level = 1) + WifiNetworkModel.CarrierMerged.of(subscriptionId = 2, level = 1) ) var latest: String? = "default" @@ -109,7 +109,7 @@ class WifiInteractorImplTest : SysuiTestCase() { fun ssid_unknownSsid_outputsNull() = testScope.runTest { wifiRepository.setWifiNetwork( - WifiNetworkModel.Active( + WifiNetworkModel.Active.of( level = 1, ssid = WifiManager.UNKNOWN_SSID, ) @@ -128,7 +128,7 @@ class WifiInteractorImplTest : SysuiTestCase() { fun ssid_validSsid_outputsSsid() = testScope.runTest { wifiRepository.setWifiNetwork( - WifiNetworkModel.Active( + WifiNetworkModel.Active.of( level = 1, ssid = "MyAwesomeWifiNetwork", ) @@ -189,7 +189,7 @@ class WifiInteractorImplTest : SysuiTestCase() { fun wifiNetwork_matchesRepoWifiNetwork() = testScope.runTest { val wifiNetwork = - WifiNetworkModel.Active( + WifiNetworkModel.Active.of( isValidated = true, level = 3, ssid = "AB", @@ -263,7 +263,7 @@ class WifiInteractorImplTest : SysuiTestCase() { val latest by collectLastValue(underTest.areNetworksAvailable) wifiRepository.wifiScanResults.value = emptyList() - wifiRepository.setWifiNetwork(WifiNetworkModel.Inactive) + wifiRepository.setWifiNetwork(WifiNetworkModel.Inactive()) assertThat(latest).isFalse() } @@ -280,7 +280,7 @@ class WifiInteractorImplTest : SysuiTestCase() { WifiScanEntry(ssid = "ssid 3"), ) - wifiRepository.setWifiNetwork(WifiNetworkModel.Inactive) + wifiRepository.setWifiNetwork(WifiNetworkModel.Inactive()) assertThat(latest).isTrue() } @@ -298,7 +298,7 @@ class WifiInteractorImplTest : SysuiTestCase() { ) wifiRepository.setWifiNetwork( - WifiNetworkModel.Active( + WifiNetworkModel.Active.of( ssid = "ssid 2", level = 2, ) @@ -318,7 +318,7 @@ class WifiInteractorImplTest : SysuiTestCase() { ) wifiRepository.setWifiNetwork( - WifiNetworkModel.Active( + WifiNetworkModel.Active.of( ssid = "ssid 2", level = 2, ) diff --git a/packages/SystemUI/multivalentTests/src/com/android/systemui/statusbar/pipeline/wifi/ui/viewmodel/WifiViewModelTest.kt b/packages/SystemUI/multivalentTests/src/com/android/systemui/statusbar/pipeline/wifi/ui/viewmodel/WifiViewModelTest.kt index 43b9531c6086..6c8d80932638 100644 --- a/packages/SystemUI/multivalentTests/src/com/android/systemui/statusbar/pipeline/wifi/ui/viewmodel/WifiViewModelTest.kt +++ b/packages/SystemUI/multivalentTests/src/com/android/systemui/statusbar/pipeline/wifi/ui/viewmodel/WifiViewModelTest.kt @@ -113,7 +113,7 @@ class WifiViewModelTest : SysuiTestCase() { val latestKeyguard by collectLastValue(keyguard.wifiIcon) val latestQs by collectLastValue(qs.wifiIcon) - wifiRepository.setWifiNetwork(WifiNetworkModel.Active(isValidated = true, level = 1)) + wifiRepository.setWifiNetwork(WifiNetworkModel.Active.of(isValidated = true, level = 1)) assertThat(latestHome).isInstanceOf(WifiIcon.Visible::class.java) assertThat(latestHome).isEqualTo(latestKeyguard) @@ -127,7 +127,7 @@ class WifiViewModelTest : SysuiTestCase() { // Even WHEN the network has a valid hotspot type wifiRepository.setWifiNetwork( - WifiNetworkModel.Active( + WifiNetworkModel.Active.of( isValidated = true, level = 1, hotspotDeviceType = WifiNetworkModel.HotspotDeviceType.LAPTOP, @@ -189,7 +189,7 @@ class WifiViewModelTest : SysuiTestCase() { whenever(connectivityConstants.shouldShowActivityConfig).thenReturn(true) createAndSetViewModel() - wifiRepository.setWifiNetwork(WifiNetworkModel.Active(ssid = null, level = 1)) + wifiRepository.setWifiNetwork(WifiNetworkModel.Active.of(ssid = null, level = 1)) val activityIn by collectLastValue(underTest.isActivityInViewVisible) val activityOut by collectLastValue(underTest.isActivityOutViewVisible) @@ -212,7 +212,7 @@ class WifiViewModelTest : SysuiTestCase() { whenever(connectivityConstants.shouldShowActivityConfig).thenReturn(true) createAndSetViewModel() - wifiRepository.setWifiNetwork(WifiNetworkModel.Active(ssid = null, level = 1)) + wifiRepository.setWifiNetwork(WifiNetworkModel.Active.of(ssid = null, level = 1)) val activityIn by collectLastValue(underTest.isActivityInViewVisible) val activityOut by collectLastValue(underTest.isActivityOutViewVisible) @@ -461,6 +461,6 @@ class WifiViewModelTest : SysuiTestCase() { } companion object { - private val ACTIVE_VALID_WIFI_NETWORK = WifiNetworkModel.Active(ssid = "AB", level = 1) + private val ACTIVE_VALID_WIFI_NETWORK = WifiNetworkModel.Active.of(ssid = "AB", level = 1) } } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/data/repository/WifiRepository.kt b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/data/repository/WifiRepository.kt index fc7a67233bb6..bc7d376a8740 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/data/repository/WifiRepository.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/data/repository/WifiRepository.kt @@ -64,9 +64,6 @@ interface WifiRepository { const val COL_NAME_IS_ENABLED = "isEnabled" /** Column name to use for [isWifiDefault] for table logging. */ const val COL_NAME_IS_DEFAULT = "isDefault" - - const val CARRIER_MERGED_INVALID_SUB_ID_REASON = - "Wifi network was carrier merged but had invalid sub ID" } } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/data/repository/demo/DemoWifiRepository.kt b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/data/repository/demo/DemoWifiRepository.kt index 7163e67eaa5d..f4bb1a34b05f 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/data/repository/demo/DemoWifiRepository.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/data/repository/demo/DemoWifiRepository.kt @@ -46,7 +46,7 @@ constructor( private val _isWifiDefault = MutableStateFlow(false) override val isWifiDefault: StateFlow<Boolean> = _isWifiDefault - private val _wifiNetwork = MutableStateFlow<WifiNetworkModel>(WifiNetworkModel.Inactive) + private val _wifiNetwork = MutableStateFlow<WifiNetworkModel>(WifiNetworkModel.Inactive()) override val wifiNetwork: StateFlow<WifiNetworkModel> = _wifiNetwork private val _secondaryNetworks = MutableStateFlow<List<WifiNetworkModel>>(emptyList()) @@ -82,7 +82,7 @@ constructor( _isWifiEnabled.value = false _isWifiDefault.value = false _wifiActivity.value = DataActivityModel(hasActivityIn = false, hasActivityOut = false) - _wifiNetwork.value = WifiNetworkModel.Inactive + _wifiNetwork.value = WifiNetworkModel.Inactive() } private fun processEnabledWifiState(event: FakeWifiEventModel.Wifi) { @@ -100,7 +100,7 @@ constructor( } private fun FakeWifiEventModel.Wifi.toWifiNetworkModel(): WifiNetworkModel = - WifiNetworkModel.Active( + WifiNetworkModel.Active.of( isValidated = validated ?: true, level = level ?: 0, ssid = ssid ?: DEMO_NET_SSID, @@ -108,7 +108,7 @@ constructor( ) private fun FakeWifiEventModel.CarrierMerged.toCarrierMergedModel(): WifiNetworkModel = - WifiNetworkModel.CarrierMerged( + WifiNetworkModel.CarrierMerged.of( subscriptionId = subscriptionId, level = level, numberOfLevels = numberOfLevels, diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/data/repository/prod/WifiRepositoryImpl.kt b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/data/repository/prod/WifiRepositoryImpl.kt index b23ab3a8a897..5889b3e22322 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/data/repository/prod/WifiRepositoryImpl.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/data/repository/prod/WifiRepositoryImpl.kt @@ -19,7 +19,6 @@ package com.android.systemui.statusbar.pipeline.wifi.data.repository.prod import android.annotation.SuppressLint import android.net.wifi.ScanResult import android.net.wifi.WifiManager -import android.telephony.SubscriptionManager.INVALID_SUBSCRIPTION_ID import androidx.lifecycle.Lifecycle import androidx.lifecycle.LifecycleOwner import androidx.lifecycle.LifecycleRegistry @@ -41,18 +40,14 @@ import com.android.systemui.statusbar.pipeline.dagger.WifiTableLog import com.android.systemui.statusbar.pipeline.shared.data.model.DataActivityModel import com.android.systemui.statusbar.pipeline.shared.data.model.toWifiDataActivityModel import com.android.systemui.statusbar.pipeline.wifi.data.repository.RealWifiRepository -import com.android.systemui.statusbar.pipeline.wifi.data.repository.WifiRepository.Companion.CARRIER_MERGED_INVALID_SUB_ID_REASON import com.android.systemui.statusbar.pipeline.wifi.data.repository.WifiRepository.Companion.COL_NAME_IS_DEFAULT import com.android.systemui.statusbar.pipeline.wifi.data.repository.WifiRepository.Companion.COL_NAME_IS_ENABLED import com.android.systemui.statusbar.pipeline.wifi.shared.model.WifiNetworkModel -import com.android.systemui.statusbar.pipeline.wifi.shared.model.WifiNetworkModel.Inactive.toHotspotDeviceType +import com.android.systemui.statusbar.pipeline.wifi.shared.model.WifiNetworkModel.Unavailable.toHotspotDeviceType import com.android.systemui.statusbar.pipeline.wifi.shared.model.WifiScanEntry import com.android.wifitrackerlib.HotspotNetworkEntry import com.android.wifitrackerlib.MergedCarrierEntry import com.android.wifitrackerlib.WifiEntry -import com.android.wifitrackerlib.WifiEntry.WIFI_LEVEL_MAX -import com.android.wifitrackerlib.WifiEntry.WIFI_LEVEL_MIN -import com.android.wifitrackerlib.WifiEntry.WIFI_LEVEL_UNREACHABLE import com.android.wifitrackerlib.WifiPickerTracker import java.util.concurrent.Executor import javax.inject.Inject @@ -256,36 +251,28 @@ constructor( } private fun MergedCarrierEntry.convertCarrierMergedToModel(): WifiNetworkModel { - return if (this.subscriptionId == INVALID_SUBSCRIPTION_ID) { - WifiNetworkModel.Invalid(CARRIER_MERGED_INVALID_SUB_ID_REASON) - } else { - WifiNetworkModel.CarrierMerged( - subscriptionId = this.subscriptionId, - level = this.level, - // WifiManager APIs to calculate the signal level start from 0, so - // maxSignalLevel + 1 represents the total level buckets count. - numberOfLevels = wifiManager.maxSignalLevel + 1, - ) - } + // WifiEntry instance values aren't guaranteed to be stable between method calls + // because + // WifiPickerTracker is continuously updating the same object. Save the level in a + // local + // variable so that checking the level validity here guarantees that the level will + // still be + // valid when we create the `WifiNetworkModel.Active` instance later. Otherwise, the + // level + // could be valid here but become invalid later, and `WifiNetworkModel.Active` will + // throw + // an exception. See b/362384551. + + return WifiNetworkModel.CarrierMerged.of( + subscriptionId = this.subscriptionId, + level = this.level, + // WifiManager APIs to calculate the signal level start from 0, so + // maxSignalLevel + 1 represents the total level buckets count. + numberOfLevels = wifiManager.maxSignalLevel + 1, + ) } private fun WifiEntry.convertNormalToModel(): WifiNetworkModel { - // WifiEntry instance values aren't guaranteed to be stable between method calls because - // WifiPickerTracker is continuously updating the same object. Save the level in a local - // variable so that checking the level validity here guarantees that the level will still be - // valid when we create the `WifiNetworkModel.Active` instance later. Otherwise, the level - // could be valid here but become invalid later, and `WifiNetworkModel.Active` will throw - // an exception. See b/362384551. - val currentLevel = this.level - if ( - currentLevel == WIFI_LEVEL_UNREACHABLE || - currentLevel !in WIFI_LEVEL_MIN..WIFI_LEVEL_MAX - ) { - // If our level means the network is unreachable or the level is otherwise invalid, we - // don't have an active network. - return WifiNetworkModel.Inactive - } - val hotspotDeviceType = if (isInstantTetherEnabled && this is HotspotNetworkEntry) { this.deviceType.toHotspotDeviceType() @@ -293,9 +280,9 @@ constructor( WifiNetworkModel.HotspotDeviceType.NONE } - return WifiNetworkModel.Active( + return WifiNetworkModel.Active.of( isValidated = this.hasInternetAccess(), - level = currentLevel, + level = this.level, ssid = this.title, hotspotDeviceType = hotspotDeviceType, ) @@ -433,7 +420,7 @@ constructor( companion object { // Start out with no known wifi network. - @VisibleForTesting val WIFI_NETWORK_DEFAULT = WifiNetworkModel.Inactive + @VisibleForTesting val WIFI_NETWORK_DEFAULT = WifiNetworkModel.Inactive() private const val WIFI_STATE_DEFAULT = WifiManager.WIFI_STATE_DISABLED diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/shared/model/WifiNetworkModel.kt b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/shared/model/WifiNetworkModel.kt index 39842fb39e24..32203774afd1 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/shared/model/WifiNetworkModel.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/shared/model/WifiNetworkModel.kt @@ -16,6 +16,7 @@ package com.android.systemui.statusbar.pipeline.wifi.shared.model +import android.net.wifi.WifiManager import android.net.wifi.WifiManager.UNKNOWN_SSID import android.net.wifi.sharedconnectivity.app.NetworkProviderInfo import android.telephony.SubscriptionManager @@ -23,8 +24,12 @@ import androidx.annotation.VisibleForTesting import com.android.systemui.log.table.Diffable import com.android.systemui.log.table.TableRowLogger import com.android.systemui.statusbar.pipeline.mobile.data.repository.MobileConnectionRepository +import com.android.systemui.statusbar.pipeline.wifi.shared.model.WifiNetworkModel.Active.Companion.MAX_VALID_LEVEL +import com.android.systemui.statusbar.pipeline.wifi.shared.model.WifiNetworkModel.Active.Companion.isValid +import com.android.systemui.statusbar.pipeline.wifi.shared.model.WifiNetworkModel.Active.Companion.of import com.android.wifitrackerlib.HotspotNetworkEntry.DeviceType import com.android.wifitrackerlib.WifiEntry +import com.android.wifitrackerlib.WifiEntry.WIFI_LEVEL_UNREACHABLE /** Provides information about the current wifi network. */ sealed class WifiNetworkModel : Diffable<WifiNetworkModel> { @@ -64,7 +69,7 @@ sealed class WifiNetworkModel : Diffable<WifiNetworkModel> { /** A description of why the wifi information was invalid. */ val invalidReason: String, ) : WifiNetworkModel() { - override fun toString() = "WifiNetwork.Invalid[$invalidReason]" + override fun toString() = "WifiNetwork.Invalid[reason=$invalidReason]" override fun logDiffs(prevVal: WifiNetworkModel, row: TableRowLogger) { if (prevVal !is Invalid) { @@ -73,12 +78,12 @@ sealed class WifiNetworkModel : Diffable<WifiNetworkModel> { } if (invalidReason != prevVal.invalidReason) { - row.logChange(COL_NETWORK_TYPE, "$TYPE_UNAVAILABLE $invalidReason") + row.logChange(COL_NETWORK_TYPE, "$TYPE_UNAVAILABLE[reason=$invalidReason]") } } override fun logFull(row: TableRowLogger) { - row.logChange(COL_NETWORK_TYPE, "$TYPE_UNAVAILABLE $invalidReason") + row.logChange(COL_NETWORK_TYPE, "$TYPE_UNAVAILABLE[reason=$invalidReason]") row.logChange(COL_SUB_ID, SUB_ID_DEFAULT) row.logChange(COL_VALIDATED, false) row.logChange(COL_LEVEL, LEVEL_DEFAULT) @@ -89,20 +94,25 @@ sealed class WifiNetworkModel : Diffable<WifiNetworkModel> { } /** A model representing that we have no active wifi network. */ - object Inactive : WifiNetworkModel() { - override fun toString() = "WifiNetwork.Inactive" + data class Inactive( + /** An optional description of why the wifi information was inactive. */ + val inactiveReason: String? = null, + ) : WifiNetworkModel() { + override fun toString() = "WifiNetwork.Inactive[reason=$inactiveReason]" override fun logDiffs(prevVal: WifiNetworkModel, row: TableRowLogger) { - if (prevVal is Inactive) { + if (prevVal !is Inactive) { + logFull(row) return } - // When changing to Inactive, we need to log diffs to all the fields. - logFull(row) + if (inactiveReason != prevVal.inactiveReason) { + row.logChange(COL_NETWORK_TYPE, "$TYPE_INACTIVE[reason=$inactiveReason]") + } } override fun logFull(row: TableRowLogger) { - row.logChange(COL_NETWORK_TYPE, TYPE_INACTIVE) + row.logChange(COL_NETWORK_TYPE, "$TYPE_INACTIVE[reason=$inactiveReason]") row.logChange(COL_SUB_ID, SUB_ID_DEFAULT) row.logChange(COL_VALIDATED, false) row.logChange(COL_LEVEL, LEVEL_DEFAULT) @@ -117,31 +127,71 @@ sealed class WifiNetworkModel : Diffable<WifiNetworkModel> { * treated as more of a mobile network. * * See [android.net.wifi.WifiInfo.isCarrierMerged] for more information. + * + * IMPORTANT: Do *not* call [copy] on this class. Instead, use the factory [of] methods. [of] + * will verify preconditions correctly. */ - data class CarrierMerged( + data class CarrierMerged + private constructor( /** * The subscription ID that this connection represents. * * Comes from [android.net.wifi.WifiInfo.getSubscriptionId]. * - * Per that method, this value must not be [INVALID_SUBSCRIPTION_ID] (if it was invalid, - * then this is *not* a carrier merged network). + * Per that method, this value must not be [SubscriptionManager.INVALID_SUBSCRIPTION_ID] (if + * it was invalid, then this is *not* a carrier merged network). */ val subscriptionId: Int, - /** The signal level, guaranteed to be 0 <= level <= numberOfLevels. */ + /** The signal level, required to be 0 <= level <= numberOfLevels. */ val level: Int, /** The maximum possible level. */ - val numberOfLevels: Int = MobileConnectionRepository.DEFAULT_NUM_LEVELS, + val numberOfLevels: Int, ) : WifiNetworkModel() { - init { - require(level in MIN_VALID_LEVEL..numberOfLevels) { - "CarrierMerged: $MIN_VALID_LEVEL <= wifi level <= $numberOfLevels required; " + + companion object { + /** + * Creates a [CarrierMerged] instance, or an [Invalid] instance if any of the arguments + * are invalid. + */ + fun of( + subscriptionId: Int, + level: Int, + numberOfLevels: Int = MobileConnectionRepository.DEFAULT_NUM_LEVELS + ): WifiNetworkModel { + if (!subscriptionId.isSubscriptionIdValid()) { + return Invalid(INVALID_SUB_ID_ERROR_STRING) + } + if (!level.isLevelValid(numberOfLevels)) { + return Invalid(getInvalidLevelErrorString(level, numberOfLevels)) + } + return CarrierMerged(subscriptionId, level, numberOfLevels) + } + + private fun Int.isLevelValid(maxLevel: Int): Boolean { + return this != WIFI_LEVEL_UNREACHABLE && this in MIN_VALID_LEVEL..maxLevel + } + + private fun getInvalidLevelErrorString(level: Int, maxLevel: Int): String { + return "Wifi network was carrier merged but had invalid level. " + + "$MIN_VALID_LEVEL <= wifi level <= $maxLevel required; " + "level was $level" } - require(subscriptionId != SubscriptionManager.INVALID_SUBSCRIPTION_ID) { - "subscription ID cannot be invalid" + + private fun Int.isSubscriptionIdValid(): Boolean { + return this != SubscriptionManager.INVALID_SUBSCRIPTION_ID + } + + private const val INVALID_SUB_ID_ERROR_STRING = + "Wifi network was carrier merged but had invalid sub ID" + } + + init { + require(level.isLevelValid(numberOfLevels)) { + "${getInvalidLevelErrorString(level, numberOfLevels)}. $DO_NOT_USE_COPY_ERROR" + } + require(subscriptionId.isSubscriptionIdValid()) { + "$INVALID_SUB_ID_ERROR_STRING. $DO_NOT_USE_COPY_ERROR" } } @@ -173,28 +223,64 @@ sealed class WifiNetworkModel : Diffable<WifiNetworkModel> { } } - /** Provides information about an active wifi network. */ - data class Active( + /** + * Provides information about an active wifi network. + * + * IMPORTANT: Do *not* call [copy] on this class. Instead, use the factory [of] method. [of] + * will verify preconditions correctly. + */ + data class Active + private constructor( /** See [android.net.NetworkCapabilities.NET_CAPABILITY_VALIDATED]. */ - val isValidated: Boolean = false, + val isValidated: Boolean, - /** The wifi signal level, guaranteed to be 0 <= level <= 4. */ + /** The wifi signal level, required to be 0 <= level <= 4. */ val level: Int, /** See [android.net.wifi.WifiInfo.ssid]. */ - val ssid: String? = null, + val ssid: String?, /** * The type of device providing a hotspot connection, or [HotspotDeviceType.NONE] if this * isn't a hotspot connection. */ - val hotspotDeviceType: HotspotDeviceType = WifiNetworkModel.HotspotDeviceType.NONE, + val hotspotDeviceType: HotspotDeviceType, ) : WifiNetworkModel() { - init { - require(level in MIN_VALID_LEVEL..MAX_VALID_LEVEL) { - "Active: $MIN_VALID_LEVEL <= wifi level <= $MAX_VALID_LEVEL required; " + + companion object { + /** + * Creates an [Active] instance, or an [Inactive] instance if any of the arguments are + * invalid. + */ + @JvmStatic + fun of( + isValidated: Boolean = false, + level: Int, + ssid: String? = null, + hotspotDeviceType: HotspotDeviceType = HotspotDeviceType.NONE, + ): WifiNetworkModel { + if (!level.isValid()) { + return Inactive(getInvalidLevelErrorString(level)) + } + return Active(isValidated, level, ssid, hotspotDeviceType) + } + + private fun Int.isValid(): Boolean { + return this != WIFI_LEVEL_UNREACHABLE && this in MIN_VALID_LEVEL..MAX_VALID_LEVEL + } + + private fun getInvalidLevelErrorString(level: Int): String { + return "Wifi network was active but had invalid level. " + + "$MIN_VALID_LEVEL <= wifi level <= $MAX_VALID_LEVEL required; " + "level was $level" } + + @VisibleForTesting internal const val MAX_VALID_LEVEL = WifiEntry.WIFI_LEVEL_MAX + } + + init { + require(level.isValid()) { + "${getInvalidLevelErrorString(level)}. $DO_NOT_USE_COPY_ERROR" + } } /** Returns true if this network has a valid SSID and false otherwise. */ @@ -231,10 +317,6 @@ sealed class WifiNetworkModel : Diffable<WifiNetworkModel> { row.logChange(COL_SSID, ssid) row.logChange(COL_HOTSPOT, hotspotDeviceType.name) } - - companion object { - @VisibleForTesting internal const val MAX_VALID_LEVEL = WifiEntry.WIFI_LEVEL_MAX - } } companion object { @@ -292,3 +374,7 @@ const val COL_HOTSPOT = "hotspot" val LEVEL_DEFAULT: String? = null val NUM_LEVELS_DEFAULT: String? = null val SUB_ID_DEFAULT: String? = null + +private const val DO_NOT_USE_COPY_ERROR = + "This should only be an issue if the caller incorrectly used `copy` to get a new instance. " + + "Please use the `of` method instead." diff --git a/packages/SystemUI/tests/src/com/android/keyguard/CarrierTextManagerTest.java b/packages/SystemUI/tests/src/com/android/keyguard/CarrierTextManagerTest.java index d85b77413338..bf13ceb5666a 100644 --- a/packages/SystemUI/tests/src/com/android/keyguard/CarrierTextManagerTest.java +++ b/packages/SystemUI/tests/src/com/android/keyguard/CarrierTextManagerTest.java @@ -445,7 +445,7 @@ public class CarrierTextManagerTest extends SysuiTestCase { assertFalse(mWifiRepository.isWifiConnectedWithValidSsid()); mWifiRepository.setWifiNetwork( - new WifiNetworkModel.Active( + WifiNetworkModel.Active.Companion.of( /* isValidated= */ false, /* level= */ 0, /* ssid= */ "", diff --git a/packages/SystemUI/tests/src/com/android/systemui/qs/tiles/InternetTileNewImplTest.kt b/packages/SystemUI/tests/src/com/android/systemui/qs/tiles/InternetTileNewImplTest.kt index e6ec07e97d17..9f84e346d54a 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/qs/tiles/InternetTileNewImplTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/qs/tiles/InternetTileNewImplTest.kt @@ -258,7 +258,7 @@ class InternetTileNewImplTest : SysuiTestCase() { companion object { const val WIFI_SSID = "test ssid" val ACTIVE_WIFI = - WifiNetworkModel.Active( + WifiNetworkModel.Active.of( isValidated = true, level = 4, ssid = WIFI_SSID, diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/CarrierMergedConnectionRepositoryTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/CarrierMergedConnectionRepositoryTest.kt index b6e23c1f42ee..715e3b472373 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/CarrierMergedConnectionRepositoryTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/CarrierMergedConnectionRepositoryTest.kt @@ -85,7 +85,7 @@ class CarrierMergedConnectionRepositoryTest : SysuiTestCase() { underTest.dataConnectionState.onEach { latestConnState = it }.launchIn(this) val netJob = underTest.resolvedNetworkType.onEach { latestNetType = it }.launchIn(this) - wifiRepository.setWifiNetwork(WifiNetworkModel.Inactive) + wifiRepository.setWifiNetwork(WifiNetworkModel.Inactive()) assertThat(latestConnState).isEqualTo(DataConnectionState.Disconnected) assertThat(latestNetType).isNotEqualTo(ResolvedNetworkType.CarrierMergedNetworkType) @@ -104,7 +104,7 @@ class CarrierMergedConnectionRepositoryTest : SysuiTestCase() { underTest.dataConnectionState.onEach { latestConnState = it }.launchIn(this) val netJob = underTest.resolvedNetworkType.onEach { latestNetType = it }.launchIn(this) - wifiRepository.setWifiNetwork(WifiNetworkModel.Active(level = 1)) + wifiRepository.setWifiNetwork(WifiNetworkModel.Active.of(level = 1)) assertThat(latestConnState).isEqualTo(DataConnectionState.Disconnected) assertThat(latestNetType).isNotEqualTo(ResolvedNetworkType.CarrierMergedNetworkType) @@ -123,7 +123,7 @@ class CarrierMergedConnectionRepositoryTest : SysuiTestCase() { wifiRepository.setIsWifiDefault(true) wifiRepository.setWifiNetwork( - WifiNetworkModel.CarrierMerged( + WifiNetworkModel.CarrierMerged.of( subscriptionId = SUB_ID, level = 3, ) @@ -143,7 +143,7 @@ class CarrierMergedConnectionRepositoryTest : SysuiTestCase() { wifiRepository.setIsWifiEnabled(true) wifiRepository.setIsWifiDefault(true) wifiRepository.setWifiNetwork( - WifiNetworkModel.CarrierMerged( + WifiNetworkModel.CarrierMerged.of( subscriptionId = SUB_ID, level = 3, ) @@ -180,7 +180,7 @@ class CarrierMergedConnectionRepositoryTest : SysuiTestCase() { val typeJob = underTest.resolvedNetworkType.onEach { latestType = it }.launchIn(this) wifiRepository.setWifiNetwork( - WifiNetworkModel.CarrierMerged( + WifiNetworkModel.CarrierMerged.of( subscriptionId = SUB_ID + 10, level = 3, ) @@ -201,7 +201,7 @@ class CarrierMergedConnectionRepositoryTest : SysuiTestCase() { val job = underTest.primaryLevel.onEach { latest = it }.launchIn(this) wifiRepository.setWifiNetwork( - WifiNetworkModel.CarrierMerged( + WifiNetworkModel.CarrierMerged.of( subscriptionId = SUB_ID, level = 3, ) @@ -221,7 +221,7 @@ class CarrierMergedConnectionRepositoryTest : SysuiTestCase() { val job = underTest.primaryLevel.onEach { latest = it }.launchIn(this) wifiRepository.setWifiNetwork( - WifiNetworkModel.CarrierMerged( + WifiNetworkModel.CarrierMerged.of( subscriptionId = SUB_ID, level = 3, ) @@ -240,7 +240,7 @@ class CarrierMergedConnectionRepositoryTest : SysuiTestCase() { val job = underTest.numberOfLevels.onEach { latest = it }.launchIn(this) wifiRepository.setWifiNetwork( - WifiNetworkModel.CarrierMerged( + WifiNetworkModel.CarrierMerged.of( subscriptionId = SUB_ID, level = 1, numberOfLevels = 6, @@ -303,7 +303,7 @@ class CarrierMergedConnectionRepositoryTest : SysuiTestCase() { whenever(telephonyManager.simOperatorName).thenReturn("New SIM name") wifiRepository.setWifiNetwork( - WifiNetworkModel.CarrierMerged( + WifiNetworkModel.CarrierMerged.of( subscriptionId = SUB_ID, level = 3, ) diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/FullMobileConnectionRepositoryTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/FullMobileConnectionRepositoryTest.kt index 035a05ff3db9..8aa0c759517d 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/FullMobileConnectionRepositoryTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/FullMobileConnectionRepositoryTest.kt @@ -513,7 +513,7 @@ class FullMobileConnectionRepositoryTest : SysuiTestCase() { // WHEN we set up carrier merged info wifiRepository.setWifiNetwork( - WifiNetworkModel.CarrierMerged( + WifiNetworkModel.CarrierMerged.of( SUB_ID, level = 3, ) @@ -524,7 +524,7 @@ class FullMobileConnectionRepositoryTest : SysuiTestCase() { // WHEN we update the info wifiRepository.setWifiNetwork( - WifiNetworkModel.CarrierMerged( + WifiNetworkModel.CarrierMerged.of( SUB_ID, level = 1, ) @@ -563,7 +563,7 @@ class FullMobileConnectionRepositoryTest : SysuiTestCase() { // WHEN isCarrierMerged is set to true wifiRepository.setWifiNetwork( - WifiNetworkModel.CarrierMerged( + WifiNetworkModel.CarrierMerged.of( SUB_ID, level = 3, ) @@ -575,7 +575,7 @@ class FullMobileConnectionRepositoryTest : SysuiTestCase() { // WHEN the carrier merge network is updated wifiRepository.setWifiNetwork( - WifiNetworkModel.CarrierMerged( + WifiNetworkModel.CarrierMerged.of( SUB_ID, level = 4, ) @@ -627,7 +627,7 @@ class FullMobileConnectionRepositoryTest : SysuiTestCase() { // THEN updates to the carrier merged level aren't logged wifiRepository.setWifiNetwork( - WifiNetworkModel.CarrierMerged( + WifiNetworkModel.CarrierMerged.of( SUB_ID, level = 4, ) @@ -635,7 +635,7 @@ class FullMobileConnectionRepositoryTest : SysuiTestCase() { assertThat(dumpBuffer()).doesNotContain("$COL_PRIMARY_LEVEL${BUFFER_SEPARATOR}4") wifiRepository.setWifiNetwork( - WifiNetworkModel.CarrierMerged( + WifiNetworkModel.CarrierMerged.of( SUB_ID, level = 3, ) diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/satellite/domain/interactor/DeviceBasedSatelliteInteractorTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/satellite/domain/interactor/DeviceBasedSatelliteInteractorTest.kt index a1cb29b8e95c..c0a206afe64b 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/satellite/domain/interactor/DeviceBasedSatelliteInteractorTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/satellite/domain/interactor/DeviceBasedSatelliteInteractorTest.kt @@ -574,7 +574,7 @@ class DeviceBasedSatelliteInteractorTest : SysuiTestCase() { val latest by collectLastValue(underTest.isWifiActive) // WHEN wifi is active - wifiRepository.setWifiNetwork(WifiNetworkModel.Active(level = 1)) + wifiRepository.setWifiNetwork(WifiNetworkModel.Active.of(level = 1)) // THEN the interactor returns true due to the wifi network being active assertThat(latest).isTrue() diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/satellite/ui/viewmodel/DeviceBasedSatelliteViewModelTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/satellite/ui/viewmodel/DeviceBasedSatelliteViewModelTest.kt index c1abf9826ea3..e7e496938033 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/satellite/ui/viewmodel/DeviceBasedSatelliteViewModelTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/satellite/ui/viewmodel/DeviceBasedSatelliteViewModelTest.kt @@ -375,7 +375,7 @@ class DeviceBasedSatelliteViewModelTest : SysuiTestCase() { repo.isSatelliteProvisioned.value = true // GIVEN wifi network is active - wifiRepository.setWifiNetwork(WifiNetworkModel.Active(level = 1)) + wifiRepository.setWifiNetwork(WifiNetworkModel.Active.of(level = 1)) // THEN icon is null because the device is connected to wifi assertThat(latest).isNull() @@ -573,7 +573,7 @@ class DeviceBasedSatelliteViewModelTest : SysuiTestCase() { repo.isSatelliteProvisioned.value = true // GIVEN wifi network is active - wifiRepository.setWifiNetwork(WifiNetworkModel.Active(level = 1)) + wifiRepository.setWifiNetwork(WifiNetworkModel.Active.of(level = 1)) // THEN carrier text is null because the device is connected to wifi assertThat(latest).isNull() diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/shared/ui/viewmodel/InternetTileViewModelTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/shared/ui/viewmodel/InternetTileViewModelTest.kt index ccbed7cd66ec..fc28c1e7b71d 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/shared/ui/viewmodel/InternetTileViewModelTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/shared/ui/viewmodel/InternetTileViewModelTest.kt @@ -151,7 +151,7 @@ class InternetTileViewModelTest : SysuiTestCase() { val latest by collectLastValue(underTest.tileModel) val networkModel = - WifiNetworkModel.Active( + WifiNetworkModel.Active.of( level = 4, ssid = "test ssid", ) @@ -180,7 +180,7 @@ class InternetTileViewModelTest : SysuiTestCase() { val latest by collectLastValue(underTest.tileModel) val networkModel = - WifiNetworkModel.Active( + WifiNetworkModel.Active.of( level = 4, ssid = "test ssid", hotspotDeviceType = WifiNetworkModel.HotspotDeviceType.NONE, @@ -292,7 +292,7 @@ class InternetTileViewModelTest : SysuiTestCase() { testScope.runTest { val latest by collectLastValue(underTest.tileModel) - val networkModel = WifiNetworkModel.Inactive + val networkModel = WifiNetworkModel.Inactive() connectivityRepository.setWifiConnected(validated = false) wifiRepository.setIsWifiDefault(true) @@ -307,7 +307,7 @@ class InternetTileViewModelTest : SysuiTestCase() { testScope.runTest { val latest by collectLastValue(underTest.tileModel) - val networkModel = WifiNetworkModel.Inactive + val networkModel = WifiNetworkModel.Inactive() connectivityRepository.setWifiConnected(validated = false) wifiRepository.setIsWifiDefault(true) @@ -387,7 +387,7 @@ class InternetTileViewModelTest : SysuiTestCase() { private fun setWifiNetworkWithHotspot(hotspot: WifiNetworkModel.HotspotDeviceType) { val networkModel = - WifiNetworkModel.Active( + WifiNetworkModel.Active.of( level = 4, ssid = "test ssid", hotspotDeviceType = hotspot, diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/wifi/data/repository/prod/WifiRepositoryImplTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/wifi/data/repository/prod/WifiRepositoryImplTest.kt index 9d73ad21485e..cf0801542d82 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/wifi/data/repository/prod/WifiRepositoryImplTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/wifi/data/repository/prod/WifiRepositoryImplTest.kt @@ -301,7 +301,10 @@ class WifiRepositoryImplTest : SysuiTestCase() { whenever(wifiPickerTracker.connectedWifiEntry).thenReturn(wifiEntry) getCallback().onWifiEntriesChanged() - assertThat(latest).isEqualTo(WifiNetworkModel.Inactive) + assertThat(latest).isInstanceOf(WifiNetworkModel.Inactive::class.java) + val inactiveReason = (latest as WifiNetworkModel.Inactive).inactiveReason + assertThat(inactiveReason).contains("level") + assertThat(inactiveReason).contains("$WIFI_LEVEL_UNREACHABLE") } @Test @@ -317,7 +320,10 @@ class WifiRepositoryImplTest : SysuiTestCase() { whenever(wifiPickerTracker.connectedWifiEntry).thenReturn(wifiEntry) getCallback().onWifiEntriesChanged() - assertThat(latest).isEqualTo(WifiNetworkModel.Inactive) + assertThat(latest).isInstanceOf(WifiNetworkModel.Inactive::class.java) + val inactiveReason = (latest as WifiNetworkModel.Inactive).inactiveReason + assertThat(inactiveReason).contains("level") + assertThat(inactiveReason).contains("${WIFI_LEVEL_MAX + 1}") } @Test @@ -333,7 +339,10 @@ class WifiRepositoryImplTest : SysuiTestCase() { whenever(wifiPickerTracker.connectedWifiEntry).thenReturn(wifiEntry) getCallback().onWifiEntriesChanged() - assertThat(latest).isEqualTo(WifiNetworkModel.Inactive) + assertThat(latest).isInstanceOf(WifiNetworkModel.Inactive::class.java) + val inactiveReason = (latest as WifiNetworkModel.Inactive).inactiveReason + assertThat(inactiveReason).contains("level") + assertThat(inactiveReason).contains("${WIFI_LEVEL_MIN - 1}") } @Test @@ -561,6 +570,25 @@ class WifiRepositoryImplTest : SysuiTestCase() { } @Test + fun wifiNetwork_carrierMergedButInvalidLevel_flowHasInvalid() = + testScope.runTest { + val latest by collectLastValue(underTest.wifiNetwork) + + val mergedEntry = + mock<MergedCarrierEntry>().apply { + whenever(this.isPrimaryNetwork).thenReturn(true) + whenever(this.subscriptionId).thenReturn(3) + whenever(this.isDefaultNetwork).thenReturn(true) + whenever(this.level).thenReturn(WIFI_LEVEL_UNREACHABLE) + } + whenever(wifiPickerTracker.mergedCarrierEntry).thenReturn(mergedEntry) + + getCallback().onWifiEntriesChanged() + + assertThat(latest).isInstanceOf(WifiNetworkModel.Invalid::class.java) + } + + @Test fun wifiNetwork_notValidated_networkNotValidated() = testScope.runTest { val latest by collectLastValue(underTest.wifiNetwork) @@ -602,7 +630,7 @@ class WifiRepositoryImplTest : SysuiTestCase() { whenever(wifiPickerTracker.connectedWifiEntry).thenReturn(wifiEntry) getCallback().onWifiEntriesChanged() - assertThat(latest).isEqualTo(WifiNetworkModel.Inactive) + assertThat(latest).isEqualTo(WifiNetworkModel.Inactive()) } @Test @@ -618,7 +646,7 @@ class WifiRepositoryImplTest : SysuiTestCase() { whenever(wifiPickerTracker.connectedWifiEntry).thenReturn(null) getCallback().onWifiEntriesChanged() - assertThat(latest).isEqualTo(WifiNetworkModel.Inactive) + assertThat(latest).isEqualTo(WifiNetworkModel.Inactive()) } @Test diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/wifi/shared/model/WifiNetworkModelTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/wifi/shared/model/WifiNetworkModelTest.kt index 92860efc0c35..1495519cc1a1 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/wifi/shared/model/WifiNetworkModelTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/wifi/shared/model/WifiNetworkModelTest.kt @@ -24,6 +24,7 @@ import com.android.systemui.SysuiTestCase import com.android.systemui.log.table.TableRowLogger import com.android.systemui.statusbar.pipeline.wifi.shared.model.WifiNetworkModel.Active.Companion.MAX_VALID_LEVEL import com.android.systemui.statusbar.pipeline.wifi.shared.model.WifiNetworkModel.Companion.MIN_VALID_LEVEL +import com.android.wifitrackerlib.WifiEntry.WIFI_LEVEL_UNREACHABLE import com.google.common.truth.Truth.assertThat import org.junit.Test import org.junit.runner.RunWith @@ -32,59 +33,109 @@ import org.junit.runner.RunWith @RunWith(AndroidJUnit4::class) class WifiNetworkModelTest : SysuiTestCase() { @Test - fun active_levelsInValidRange_noException() { + fun active_levelsInValidRange_createsActive() { (MIN_VALID_LEVEL..MAX_VALID_LEVEL).forEach { level -> - WifiNetworkModel.Active(level = level) - // No assert, just need no crash + val result = WifiNetworkModel.Active.of(level = level) + assertThat(result).isInstanceOf(WifiNetworkModel.Active::class.java) } } + fun active_levelTooLow_returnsInactive() { + val result = WifiNetworkModel.Active.of(level = MIN_VALID_LEVEL - 1) + assertThat(result).isInstanceOf(WifiNetworkModel.Inactive::class.java) + } + @Test(expected = IllegalArgumentException::class) - fun active_levelNegative_exceptionThrown() { - WifiNetworkModel.Active(level = MIN_VALID_LEVEL - 1) + fun active_levelTooLow_createdByCopy_exceptionThrown() { + val starting = WifiNetworkModel.Active.of(level = MIN_VALID_LEVEL) + + (starting as WifiNetworkModel.Active).copy(level = MIN_VALID_LEVEL - 1) + } + + fun active_levelTooHigh_returnsInactive() { + val result = WifiNetworkModel.Active.of(level = MAX_VALID_LEVEL + 1) + + assertThat(result).isInstanceOf(WifiNetworkModel.Inactive::class.java) } @Test(expected = IllegalArgumentException::class) - fun active_levelTooHigh_exceptionThrown() { - WifiNetworkModel.Active(level = MAX_VALID_LEVEL + 1) + fun active_levelTooHigh_createdByCopy_exceptionThrown() { + val starting = WifiNetworkModel.Active.of(level = MAX_VALID_LEVEL) + + (starting as WifiNetworkModel.Active).copy(level = MAX_VALID_LEVEL + 1) + } + + fun active_levelUnreachable_returnsInactive() { + val result = WifiNetworkModel.Active.of(level = WIFI_LEVEL_UNREACHABLE) + + assertThat(result).isInstanceOf(WifiNetworkModel.Inactive::class.java) } @Test(expected = IllegalArgumentException::class) - fun carrierMerged_invalidSubId_exceptionThrown() { - WifiNetworkModel.CarrierMerged(INVALID_SUBSCRIPTION_ID, 1) + fun active_levelUnreachable_createdByCopy_exceptionThrown() { + val starting = WifiNetworkModel.Active.of(level = MAX_VALID_LEVEL) + + (starting as WifiNetworkModel.Active).copy(level = WIFI_LEVEL_UNREACHABLE) + } + + fun carrierMerged_invalidSubId_returnsInvalid() { + val result = WifiNetworkModel.CarrierMerged.of(INVALID_SUBSCRIPTION_ID, level = 1) + + assertThat(result).isInstanceOf(WifiNetworkModel.Invalid::class.java) + } + + @Test(expected = IllegalArgumentException::class) + fun carrierMerged_invalidSubId_createdByCopy_exceptionThrown() { + val starting = WifiNetworkModel.CarrierMerged.of(subscriptionId = 1, level = 1) + + (starting as WifiNetworkModel.CarrierMerged).copy(subscriptionId = INVALID_SUBSCRIPTION_ID) + } + + fun carrierMerged_levelUnreachable_returnsInvalid() { + val result = + WifiNetworkModel.CarrierMerged.of(subscriptionId = 1, level = WIFI_LEVEL_UNREACHABLE) + + assertThat(result).isInstanceOf(WifiNetworkModel.Invalid::class.java) + } + + @Test(expected = IllegalArgumentException::class) + fun carrierMerged_levelUnreachable_createdByCopy_exceptionThrown() { + val starting = WifiNetworkModel.CarrierMerged.of(subscriptionId = 1, level = 1) + + (starting as WifiNetworkModel.CarrierMerged).copy(level = WIFI_LEVEL_UNREACHABLE) } @Test fun active_hasValidSsid_nullSsid_false() { val network = - WifiNetworkModel.Active( + WifiNetworkModel.Active.of( level = MAX_VALID_LEVEL, ssid = null, ) - assertThat(network.hasValidSsid()).isFalse() + assertThat((network as WifiNetworkModel.Active).hasValidSsid()).isFalse() } @Test fun active_hasValidSsid_unknownSsid_false() { val network = - WifiNetworkModel.Active( + WifiNetworkModel.Active.of( level = MAX_VALID_LEVEL, ssid = UNKNOWN_SSID, ) - assertThat(network.hasValidSsid()).isFalse() + assertThat((network as WifiNetworkModel.Active).hasValidSsid()).isFalse() } @Test fun active_hasValidSsid_validSsid_true() { val network = - WifiNetworkModel.Active( + WifiNetworkModel.Active.of( level = MAX_VALID_LEVEL, ssid = "FakeSsid", ) - assertThat(network.hasValidSsid()).isTrue() + assertThat((network as WifiNetworkModel.Active).hasValidSsid()).isTrue() } // Non-exhaustive logDiffs test -- just want to make sure the logging logic isn't totally broken @@ -93,14 +144,15 @@ class WifiNetworkModelTest : SysuiTestCase() { fun logDiffs_carrierMergedToInactive_resetsAllFields() { val logger = TestLogger() val prevVal = - WifiNetworkModel.CarrierMerged( + WifiNetworkModel.CarrierMerged.of( subscriptionId = 3, level = 1, ) - WifiNetworkModel.Inactive.logDiffs(prevVal, logger) + WifiNetworkModel.Inactive(inactiveReason = "TestReason").logDiffs(prevVal, logger) - assertThat(logger.changes).contains(Pair(COL_NETWORK_TYPE, TYPE_INACTIVE)) + assertThat(logger.changes) + .contains(Pair(COL_NETWORK_TYPE, "$TYPE_INACTIVE[reason=TestReason]")) assertThat(logger.changes).contains(Pair(COL_VALIDATED, "false")) assertThat(logger.changes).contains(Pair(COL_LEVEL, LEVEL_DEFAULT.toString())) assertThat(logger.changes).contains(Pair(COL_SSID, "null")) @@ -110,12 +162,12 @@ class WifiNetworkModelTest : SysuiTestCase() { fun logDiffs_inactiveToCarrierMerged_logsAllFields() { val logger = TestLogger() val carrierMerged = - WifiNetworkModel.CarrierMerged( + WifiNetworkModel.CarrierMerged.of( subscriptionId = 3, level = 2, ) - carrierMerged.logDiffs(prevVal = WifiNetworkModel.Inactive, logger) + carrierMerged.logDiffs(prevVal = WifiNetworkModel.Inactive(), logger) assertThat(logger.changes).contains(Pair(COL_NETWORK_TYPE, TYPE_CARRIER_MERGED)) assertThat(logger.changes).contains(Pair(COL_SUB_ID, "3")) @@ -128,14 +180,14 @@ class WifiNetworkModelTest : SysuiTestCase() { fun logDiffs_inactiveToActive_logsAllActiveFields() { val logger = TestLogger() val activeNetwork = - WifiNetworkModel.Active( + WifiNetworkModel.Active.of( isValidated = true, level = 3, ssid = "Test SSID", hotspotDeviceType = WifiNetworkModel.HotspotDeviceType.LAPTOP, ) - activeNetwork.logDiffs(prevVal = WifiNetworkModel.Inactive, logger) + activeNetwork.logDiffs(prevVal = WifiNetworkModel.Inactive(), logger) assertThat(logger.changes).contains(Pair(COL_NETWORK_TYPE, TYPE_ACTIVE)) assertThat(logger.changes).contains(Pair(COL_VALIDATED, "true")) @@ -148,11 +200,13 @@ class WifiNetworkModelTest : SysuiTestCase() { fun logDiffs_activeToInactive_resetsAllActiveFields() { val logger = TestLogger() val activeNetwork = - WifiNetworkModel.Active(isValidated = true, level = 3, ssid = "Test SSID") + WifiNetworkModel.Active.of(isValidated = true, level = 3, ssid = "Test SSID") - WifiNetworkModel.Inactive.logDiffs(prevVal = activeNetwork, logger) + WifiNetworkModel.Inactive(inactiveReason = "TestReason") + .logDiffs(prevVal = activeNetwork, logger) - assertThat(logger.changes).contains(Pair(COL_NETWORK_TYPE, TYPE_INACTIVE)) + assertThat(logger.changes) + .contains(Pair(COL_NETWORK_TYPE, "$TYPE_INACTIVE[reason=TestReason]")) assertThat(logger.changes).contains(Pair(COL_VALIDATED, "false")) assertThat(logger.changes).contains(Pair(COL_LEVEL, LEVEL_DEFAULT.toString())) assertThat(logger.changes).contains(Pair(COL_SSID, "null")) @@ -163,14 +217,14 @@ class WifiNetworkModelTest : SysuiTestCase() { fun logDiffs_carrierMergedToActive_logsAllActiveFields() { val logger = TestLogger() val activeNetwork = - WifiNetworkModel.Active( + WifiNetworkModel.Active.of( isValidated = true, level = 3, ssid = "Test SSID", hotspotDeviceType = WifiNetworkModel.HotspotDeviceType.AUTO, ) val prevVal = - WifiNetworkModel.CarrierMerged( + WifiNetworkModel.CarrierMerged.of( subscriptionId = 3, level = 1, ) @@ -188,9 +242,9 @@ class WifiNetworkModelTest : SysuiTestCase() { fun logDiffs_activeToCarrierMerged_logsAllFields() { val logger = TestLogger() val activeNetwork = - WifiNetworkModel.Active(isValidated = true, level = 3, ssid = "Test SSID") + WifiNetworkModel.Active.of(isValidated = true, level = 3, ssid = "Test SSID") val carrierMerged = - WifiNetworkModel.CarrierMerged( + WifiNetworkModel.CarrierMerged.of( subscriptionId = 3, level = 2, ) @@ -208,9 +262,9 @@ class WifiNetworkModelTest : SysuiTestCase() { fun logDiffs_activeChangesLevel_onlyLevelLogged() { val logger = TestLogger() val prevActiveNetwork = - WifiNetworkModel.Active(isValidated = true, level = 3, ssid = "Test SSID") + WifiNetworkModel.Active.of(isValidated = true, level = 3, ssid = "Test SSID") val newActiveNetwork = - WifiNetworkModel.Active(isValidated = true, level = 2, ssid = "Test SSID") + WifiNetworkModel.Active.of(isValidated = true, level = 2, ssid = "Test SSID") newActiveNetwork.logDiffs(prevActiveNetwork, logger) diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/wifi/ui/view/ModernStatusBarWifiViewTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/wifi/ui/view/ModernStatusBarWifiViewTest.kt index 31ffa5df4dba..bbf5bb2bc011 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/wifi/ui/view/ModernStatusBarWifiViewTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/wifi/ui/view/ModernStatusBarWifiViewTest.kt @@ -193,7 +193,7 @@ class ModernStatusBarWifiViewTest : SysuiTestCase() { @Test fun isIconVisible_notEnabled_outputsFalse() { wifiRepository.setIsWifiEnabled(false) - wifiRepository.setWifiNetwork(WifiNetworkModel.Active(isValidated = true, level = 2)) + wifiRepository.setWifiNetwork(WifiNetworkModel.Active.of(isValidated = true, level = 2)) val view = ModernStatusBarWifiView.constructAndBind(context, SLOT_NAME, viewModel) @@ -208,7 +208,7 @@ class ModernStatusBarWifiViewTest : SysuiTestCase() { @Test fun isIconVisible_enabled_outputsTrue() { wifiRepository.setIsWifiEnabled(true) - wifiRepository.setWifiNetwork(WifiNetworkModel.Active(isValidated = true, level = 2)) + wifiRepository.setWifiNetwork(WifiNetworkModel.Active.of(isValidated = true, level = 2)) val view = ModernStatusBarWifiView.constructAndBind(context, SLOT_NAME, viewModel) diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/wifi/ui/viewmodel/WifiViewModelIconParameterizedTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/wifi/ui/viewmodel/WifiViewModelIconParameterizedTest.kt index 43fee78ce220..a867847bc731 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/wifi/ui/viewmodel/WifiViewModelIconParameterizedTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/wifi/ui/viewmodel/WifiViewModelIconParameterizedTest.kt @@ -204,51 +204,51 @@ internal class WifiViewModelIconParameterizedTest(private val testCase: TestCase // Enabled = false => no networks shown TestCase( enabled = false, - network = WifiNetworkModel.CarrierMerged(subscriptionId = 1, level = 1), + network = WifiNetworkModel.CarrierMerged.of(subscriptionId = 1, level = 1), expected = null, ), TestCase( enabled = false, - network = WifiNetworkModel.Inactive, + network = WifiNetworkModel.Inactive(), expected = null, ), TestCase( enabled = false, - network = WifiNetworkModel.Active(isValidated = false, level = 1), + network = WifiNetworkModel.Active.of(isValidated = false, level = 1), expected = null, ), TestCase( enabled = false, - network = WifiNetworkModel.Active(isValidated = true, level = 3), + network = WifiNetworkModel.Active.of(isValidated = true, level = 3), expected = null, ), // forceHidden = true => no networks shown TestCase( forceHidden = true, - network = WifiNetworkModel.CarrierMerged(subscriptionId = 1, level = 1), + network = WifiNetworkModel.CarrierMerged.of(subscriptionId = 1, level = 1), expected = null, ), TestCase( forceHidden = true, - network = WifiNetworkModel.Inactive, + network = WifiNetworkModel.Inactive(), expected = null, ), TestCase( enabled = false, - network = WifiNetworkModel.Active(isValidated = false, level = 2), + network = WifiNetworkModel.Active.of(isValidated = false, level = 2), expected = null, ), TestCase( forceHidden = true, - network = WifiNetworkModel.Active(isValidated = true, level = 1), + network = WifiNetworkModel.Active.of(isValidated = true, level = 1), expected = null, ), // alwaysShowIconWhenEnabled = true => all Inactive and Active networks shown TestCase( alwaysShowIconWhenEnabled = true, - network = WifiNetworkModel.Inactive, + network = WifiNetworkModel.Inactive(), expected = Expected( iconResource = WIFI_NO_NETWORK, @@ -261,7 +261,7 @@ internal class WifiViewModelIconParameterizedTest(private val testCase: TestCase ), TestCase( alwaysShowIconWhenEnabled = true, - network = WifiNetworkModel.Active(isValidated = false, level = 4), + network = WifiNetworkModel.Active.of(isValidated = false, level = 4), expected = Expected( iconResource = WIFI_NO_INTERNET_ICONS[4], @@ -274,7 +274,7 @@ internal class WifiViewModelIconParameterizedTest(private val testCase: TestCase ), TestCase( alwaysShowIconWhenEnabled = true, - network = WifiNetworkModel.Active(isValidated = true, level = 2), + network = WifiNetworkModel.Active.of(isValidated = true, level = 2), expected = Expected( iconResource = WIFI_FULL_ICONS[2], @@ -288,7 +288,7 @@ internal class WifiViewModelIconParameterizedTest(private val testCase: TestCase // hasDataCapabilities = false => all Inactive and Active networks shown TestCase( hasDataCapabilities = false, - network = WifiNetworkModel.Inactive, + network = WifiNetworkModel.Inactive(), expected = Expected( iconResource = WIFI_NO_NETWORK, @@ -301,7 +301,7 @@ internal class WifiViewModelIconParameterizedTest(private val testCase: TestCase ), TestCase( hasDataCapabilities = false, - network = WifiNetworkModel.Active(isValidated = false, level = 2), + network = WifiNetworkModel.Active.of(isValidated = false, level = 2), expected = Expected( iconResource = WIFI_NO_INTERNET_ICONS[2], @@ -314,7 +314,7 @@ internal class WifiViewModelIconParameterizedTest(private val testCase: TestCase ), TestCase( hasDataCapabilities = false, - network = WifiNetworkModel.Active(isValidated = true, level = 0), + network = WifiNetworkModel.Active.of(isValidated = true, level = 0), expected = Expected( iconResource = WIFI_FULL_ICONS[0], @@ -328,7 +328,7 @@ internal class WifiViewModelIconParameterizedTest(private val testCase: TestCase // isDefault = true => all Inactive and Active networks shown TestCase( isDefault = true, - network = WifiNetworkModel.Inactive, + network = WifiNetworkModel.Inactive(), expected = Expected( iconResource = WIFI_NO_NETWORK, @@ -341,7 +341,7 @@ internal class WifiViewModelIconParameterizedTest(private val testCase: TestCase ), TestCase( isDefault = true, - network = WifiNetworkModel.Active(isValidated = false, level = 3), + network = WifiNetworkModel.Active.of(isValidated = false, level = 3), expected = Expected( iconResource = WIFI_NO_INTERNET_ICONS[3], @@ -354,7 +354,7 @@ internal class WifiViewModelIconParameterizedTest(private val testCase: TestCase ), TestCase( isDefault = true, - network = WifiNetworkModel.Active(isValidated = true, level = 1), + network = WifiNetworkModel.Active.of(isValidated = true, level = 1), expected = Expected( iconResource = WIFI_FULL_ICONS[1], @@ -370,14 +370,14 @@ internal class WifiViewModelIconParameterizedTest(private val testCase: TestCase enabled = true, isDefault = true, forceHidden = false, - network = WifiNetworkModel.CarrierMerged(subscriptionId = 1, level = 1), + network = WifiNetworkModel.CarrierMerged.of(subscriptionId = 1, level = 1), expected = null, ), // isDefault = false => no networks shown TestCase( isDefault = false, - network = WifiNetworkModel.Inactive, + network = WifiNetworkModel.Inactive(), expected = null, ), TestCase( @@ -387,7 +387,7 @@ internal class WifiViewModelIconParameterizedTest(private val testCase: TestCase ), TestCase( isDefault = false, - network = WifiNetworkModel.Active(isValidated = false, level = 3), + network = WifiNetworkModel.Active.of(isValidated = false, level = 3), expected = null, ), @@ -395,7 +395,7 @@ internal class WifiViewModelIconParameterizedTest(private val testCase: TestCase // because wifi isn't the default connection (b/272509965). TestCase( isDefault = false, - network = WifiNetworkModel.Active(isValidated = true, level = 4), + network = WifiNetworkModel.Active.of(isValidated = true, level = 4), expected = null, ), ) diff --git a/packages/SystemUI/tests/utils/src/com/android/systemui/statusbar/pipeline/wifi/data/repository/FakeWifiRepository.kt b/packages/SystemUI/tests/utils/src/com/android/systemui/statusbar/pipeline/wifi/data/repository/FakeWifiRepository.kt index 709be5edf4c0..7ca90ea4bd7d 100644 --- a/packages/SystemUI/tests/utils/src/com/android/systemui/statusbar/pipeline/wifi/data/repository/FakeWifiRepository.kt +++ b/packages/SystemUI/tests/utils/src/com/android/systemui/statusbar/pipeline/wifi/data/repository/FakeWifiRepository.kt @@ -32,7 +32,7 @@ class FakeWifiRepository : WifiRepository { override val isWifiDefault: StateFlow<Boolean> = _isWifiDefault private val _wifiNetwork: MutableStateFlow<WifiNetworkModel> = - MutableStateFlow(WifiNetworkModel.Inactive) + MutableStateFlow(WifiNetworkModel.Inactive()) override val wifiNetwork: StateFlow<WifiNetworkModel> = _wifiNetwork override val secondaryNetworks = MutableStateFlow<List<WifiNetworkModel>>(emptyList()) |