Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 1a5478a7 authored by Caitlin Shkuratov's avatar Caitlin Shkuratov
Browse files

[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
parent ae8b2ea9
Loading
Loading
Loading
Loading
+5 −5
Original line number Diff line number Diff line
@@ -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,
+9 −9
Original line number Diff line number Diff line
@@ -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,
                )
+5 −5
Original line number Diff line number Diff line
@@ -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)
    }
}
+0 −3
Original line number Diff line number Diff line
@@ -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"
    }
}

+4 −4
Original line number Diff line number Diff line
@@ -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,
Loading