Skip to content

Commit 4f82b6b

Browse files
RawaRawa
committed
Improve connectivity listener online detection
Fixes incorrect filter for networkEvents. Also adds debouncing to offline state when roaming to cellular from WiFi, this would otherwise cause a full reconnection.
1 parent 01e094c commit 4f82b6b

File tree

5 files changed

+296
-89
lines changed

5 files changed

+296
-89
lines changed

‎android/app/build.gradle.kts

+1
Original file line numberDiff line numberDiff line change
@@ -431,6 +431,7 @@ dependencies {
431431
androidTestImplementation(libs.koin.test)
432432
androidTestImplementation(libs.kotlin.test)
433433
androidTestImplementation(libs.mockk.android)
434+
androidTestImplementation(libs.turbine)
434435
androidTestImplementation(Dependencies.junitJupiterApi)
435436
androidTestImplementation(Dependencies.junit5AndroidTestCompose)
436437
}

‎android/app/src/androidTest/kotlin/net/mullvad/mullvadvpn/talpid/util/ConnectivityManagerUtilKtTest.kt

+218
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,218 @@
1+
package net.mullvad.mullvadvpn.talpid.util
2+
3+
import android.net.ConnectivityManager
4+
import android.net.Network
5+
import app.cash.turbine.test
6+
import io.mockk.every
7+
import io.mockk.mockk
8+
import io.mockk.mockkStatic
9+
import kotlin.test.assertEquals
10+
import kotlin.time.Duration.Companion.milliseconds
11+
import kotlin.time.Duration.Companion.seconds
12+
import kotlinx.coroutines.channels.awaitClose
13+
import kotlinx.coroutines.delay
14+
import kotlinx.coroutines.flow.callbackFlow
15+
import kotlinx.coroutines.test.runTest
16+
import net.mullvad.talpid.util.NetworkEvent
17+
import net.mullvad.talpid.util.hasInternetConnectivity
18+
import net.mullvad.talpid.util.networkEvents
19+
import net.mullvad.talpid.util.networksWithInternetConnectivity
20+
import org.junit.jupiter.api.BeforeEach
21+
import org.junit.jupiter.api.Test
22+
23+
class ConnectivityManagerUtilKtTest {
24+
private val connectivityManager = mockk<ConnectivityManager>()
25+
26+
@BeforeEach
27+
fun setup() {
28+
mockkStatic(CONNECTIVITY_MANAGER_UTIL_CLASS)
29+
}
30+
31+
/** User being online, the listener should emit once with `true` */
32+
@Test
33+
fun userIsOnline() = runTest {
34+
val network = mockk<Network>()
35+
every { connectivityManager.networksWithInternetConnectivity() } returns setOf(network)
36+
every { connectivityManager.networkEvents(any()) } returns
37+
callbackFlow {
38+
delay(100.milliseconds) // Simulate connectivity listener being a bit slow
39+
send(NetworkEvent.Available(network))
40+
awaitClose {}
41+
}
42+
43+
connectivityManager.hasInternetConnectivity().test {
44+
// Since initial state and listener both return `true` within debounce we only see one
45+
// event
46+
assertEquals(true, awaitItem())
47+
expectNoEvents()
48+
}
49+
}
50+
51+
/** User being offline, the listener should emit once with `false` */
52+
@Test
53+
fun userIsOffline() = runTest {
54+
every { connectivityManager.networksWithInternetConnectivity() } returns setOf()
55+
every { connectivityManager.networkEvents(any()) } returns callbackFlow { awaitClose {} }
56+
57+
connectivityManager.hasInternetConnectivity().test {
58+
// Initially offline and no network events, so we should get a single `false` event
59+
assertEquals(false, awaitItem())
60+
expectNoEvents()
61+
}
62+
}
63+
64+
/** User starting offline and then turning on a online after a while */
65+
@Test
66+
fun initiallyOfflineThenBecomingOnline() = runTest {
67+
every { connectivityManager.networksWithInternetConnectivity() } returns emptySet()
68+
every { connectivityManager.networkEvents(any()) } returns
69+
callbackFlow {
70+
// Simulate offline for a little while
71+
delay(5.seconds)
72+
// Then become online
73+
send(NetworkEvent.Available(mockk()))
74+
awaitClose {}
75+
}
76+
77+
connectivityManager.hasInternetConnectivity().test {
78+
assertEquals(false, awaitItem())
79+
assertEquals(true, awaitItem())
80+
expectNoEvents()
81+
}
82+
}
83+
84+
/** User starting online and then becoming offline after 5 seconds */
85+
@Test
86+
fun initiallyOnlineAndThenTurningBecomingOffline() = runTest {
87+
val network = mockk<Network>()
88+
every { connectivityManager.networksWithInternetConnectivity() } returns setOf(network)
89+
every { connectivityManager.networkEvents(any()) } returns
90+
callbackFlow {
91+
// Starting as online
92+
send(NetworkEvent.Available(network))
93+
delay(5.seconds)
94+
// Then becoming offline
95+
send(NetworkEvent.Lost(network))
96+
awaitClose {}
97+
}
98+
99+
connectivityManager.hasInternetConnectivity().test {
100+
assertEquals(true, awaitItem())
101+
assertEquals(false, awaitItem())
102+
expectNoEvents()
103+
}
104+
}
105+
106+
/**
107+
* User turning on Airplane mode as our connectivity listener starts so we never get any
108+
* onAvailable event from our listener. Initial value will be `true`, followed by no
109+
* `networkEvent` and then turning on network again after 5 seconds
110+
*/
111+
@Test
112+
fun incorrectInitialValueThenBecomingOnline() = runTest {
113+
every { connectivityManager.networksWithInternetConnectivity() } returns setOf(mockk())
114+
every { connectivityManager.networkEvents(any()) } returns
115+
callbackFlow {
116+
delay(5.seconds)
117+
send(NetworkEvent.Available(mockk()))
118+
awaitClose {}
119+
}
120+
121+
connectivityManager.hasInternetConnectivity().test {
122+
// Initial value is connected
123+
assertEquals(true, awaitItem())
124+
// Debounce time has passed, and we never received any network events, so we are offline
125+
assertEquals(false, awaitItem())
126+
// Network is back online
127+
assertEquals(true, awaitItem())
128+
expectNoEvents()
129+
}
130+
}
131+
132+
/** User roaming from cellular to WiFi. This behavior has been recorded on a Pixel 8 */
133+
@Test
134+
fun roamingFromCellularToWifi() = runTest {
135+
val wifiNetwork = mockk<Network>()
136+
val cellularNetwork = mockk<Network>()
137+
138+
every { connectivityManager.networksWithInternetConnectivity() } returns
139+
setOf(cellularNetwork)
140+
every { connectivityManager.networkEvents(any()) } returns
141+
callbackFlow {
142+
send(NetworkEvent.Available(cellularNetwork))
143+
delay(5.seconds)
144+
// Turning on WiFi, we'll have duplicate networks until phone decides to turn of
145+
// cellular
146+
send(NetworkEvent.Available(wifiNetwork))
147+
delay(30.seconds)
148+
// Phone turning off cellular network
149+
send(NetworkEvent.Lost(cellularNetwork))
150+
awaitClose {}
151+
}
152+
153+
connectivityManager.hasInternetConnectivity().test {
154+
// We should always only see us being online
155+
assertEquals(true, awaitItem())
156+
expectNoEvents()
157+
}
158+
}
159+
160+
/** User roaming from WiFi to Cellular. This behavior has been recorded on a Pixel 8 */
161+
@Test
162+
fun roamingFromWifiToCellular() = runTest {
163+
val wifiNetwork = mockk<Network>()
164+
val cellularNetwork = mockk<Network>()
165+
166+
every { connectivityManager.networksWithInternetConnectivity() } returns setOf(wifiNetwork)
167+
every { connectivityManager.networkEvents(any()) } returns
168+
callbackFlow {
169+
send(NetworkEvent.Available(wifiNetwork))
170+
delay(5.seconds)
171+
send(NetworkEvent.Lost(wifiNetwork))
172+
// We will have no network for a little time until cellular chip is on.
173+
delay(150.milliseconds)
174+
send(NetworkEvent.Available(cellularNetwork))
175+
awaitClose {}
176+
}
177+
178+
connectivityManager.hasInternetConnectivity().test {
179+
// We should always only see us being online, small offline state is caught by debounce
180+
assertEquals(true, awaitItem())
181+
expectNoEvents()
182+
}
183+
}
184+
185+
/** User slow roaming from WiFi to Cellular. */
186+
@Test
187+
fun slowRoamingFromWifiToCellular() = runTest {
188+
val wifiNetwork = mockk<Network>()
189+
val cellularNetwork = mockk<Network>()
190+
191+
every { connectivityManager.networksWithInternetConnectivity() } returns setOf(wifiNetwork)
192+
every { connectivityManager.networkEvents(any()) } returns
193+
callbackFlow {
194+
send(NetworkEvent.Available(wifiNetwork))
195+
delay(5.seconds)
196+
send(NetworkEvent.Lost(wifiNetwork))
197+
// We will have no network for a little time until cellular chip is on.
198+
delay(500.milliseconds)
199+
send(NetworkEvent.Available(cellularNetwork))
200+
awaitClose {}
201+
}
202+
203+
connectivityManager.hasInternetConnectivity().test {
204+
// Wifi is online
205+
assertEquals(true, awaitItem())
206+
// We didn't get any network within debounce time, so we are offline
207+
assertEquals(false, awaitItem())
208+
// Cellular network is online
209+
assertEquals(true, awaitItem())
210+
expectNoEvents()
211+
}
212+
}
213+
214+
companion object {
215+
private const val CONNECTIVITY_MANAGER_UTIL_CLASS =
216+
"net.mullvad.talpid.util.ConnectivityManagerUtilKt"
217+
}
218+
}

‎android/lib/common/src/main/kotlin/net/mullvad/mullvadvpn/lib/common/util/CommonFlowUtils.kt

-17
Original file line numberDiff line numberDiff line change
@@ -1,26 +1,9 @@
11
package net.mullvad.mullvadvpn.lib.common.util
22

3-
import kotlin.time.Duration
4-
import kotlinx.coroutines.FlowPreview
53
import kotlinx.coroutines.flow.Flow
6-
import kotlinx.coroutines.flow.debounce
74
import kotlinx.coroutines.flow.firstOrNull
8-
import kotlinx.coroutines.flow.map
9-
import kotlinx.coroutines.flow.withIndex
105
import kotlinx.coroutines.withTimeoutOrNull
116

127
suspend fun <T> Flow<T>.firstOrNullWithTimeout(timeMillis: Long): T? {
138
return withTimeoutOrNull(timeMillis) { firstOrNull() }
149
}
15-
16-
@OptIn(FlowPreview::class)
17-
fun <T> Flow<T>.debounceFirst(timeout: Duration): Flow<T> =
18-
withIndex()
19-
.debounce {
20-
if (it.index == 0) {
21-
timeout
22-
} else {
23-
Duration.ZERO
24-
}
25-
}
26-
.map { it.value }

‎android/lib/talpid/src/main/kotlin/net/mullvad/talpid/ConnectivityListener.kt

+3-72
Original file line numberDiff line numberDiff line change
@@ -2,35 +2,23 @@ package net.mullvad.talpid
22

33
import android.net.ConnectivityManager
44
import android.net.LinkProperties
5-
import android.net.Network
6-
import android.net.NetworkCapabilities
7-
import android.net.NetworkRequest
8-
import co.touchlab.kermit.Logger
95
import java.net.InetAddress
106
import kotlin.collections.ArrayList
11-
import kotlin.time.Duration.Companion.seconds
127
import kotlinx.coroutines.CoroutineScope
138
import kotlinx.coroutines.channels.Channel
14-
import kotlinx.coroutines.flow.Flow
159
import kotlinx.coroutines.flow.MutableStateFlow
1610
import kotlinx.coroutines.flow.SharingStarted
1711
import kotlinx.coroutines.flow.StateFlow
18-
import kotlinx.coroutines.flow.distinctUntilChanged
19-
import kotlinx.coroutines.flow.filter
2012
import kotlinx.coroutines.flow.map
2113
import kotlinx.coroutines.flow.merge
2214
import kotlinx.coroutines.flow.onEach
23-
import kotlinx.coroutines.flow.onStart
2415
import kotlinx.coroutines.flow.receiveAsFlow
25-
import kotlinx.coroutines.flow.scan
2616
import kotlinx.coroutines.flow.stateIn
2717
import kotlinx.coroutines.launch
28-
import net.mullvad.mullvadvpn.lib.common.util.debounceFirst
2918
import net.mullvad.talpid.model.NetworkState
30-
import net.mullvad.talpid.util.NetworkEvent
3119
import net.mullvad.talpid.util.RawNetworkState
3220
import net.mullvad.talpid.util.defaultRawNetworkStateFlow
33-
import net.mullvad.talpid.util.networkEvents
21+
import net.mullvad.talpid.util.hasInternetConnectivity
3422

3523
class ConnectivityListener(private val connectivityManager: ConnectivityManager) {
3624
private lateinit var _isConnected: StateFlow<Boolean>
@@ -64,7 +52,8 @@ class ConnectivityListener(private val connectivityManager: ConnectivityManager)
6452
}
6553

6654
_isConnected =
67-
hasInternetConnectivity()
55+
connectivityManager
56+
.hasInternetConnectivity()
6857
.onEach { notifyConnectivityChange(it) }
6958
.stateIn(
7059
scope,
@@ -84,64 +73,6 @@ class ConnectivityListener(private val connectivityManager: ConnectivityManager)
8473
private fun LinkProperties.dnsServersWithoutFallback(): List<InetAddress> =
8574
dnsServers.filter { it.hostAddress != TalpidVpnService.FALLBACK_DUMMY_DNS_SERVER }
8675

87-
private val nonVPNInternetNetworksRequest =
88-
NetworkRequest.Builder()
89-
.addCapability(NetworkCapabilities.NET_CAPABILITY_NOT_VPN)
90-
.addCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET)
91-
.build()
92-
93-
/**
94-
* Return a flow notifying us if we have internet connectivity. Initial state will be taken from
95-
* `allNetworks` and then updated when network events occur. Important to note that
96-
* `allNetworks` may return a network that we never get updates from if turned off at the moment
97-
* of the initial query.
98-
*/
99-
private fun hasInternetConnectivity(): Flow<Boolean> {
100-
return connectivityManager
101-
.networkEvents(nonVPNInternetNetworksRequest)
102-
.filter { it is NetworkEvent.Lost || it is NetworkEvent.CapabilitiesChanged }
103-
.scan(emptySet<Network>()) { networks, event ->
104-
when (event) {
105-
is NetworkEvent.Lost -> networks - event.network
106-
is NetworkEvent.Available -> networks + event.network
107-
else -> networks // Should never happen
108-
}.also { Logger.d("Networks: $it") }
109-
}
110-
// NetworkEvents are slow, can several 100 millis to arrive. If we are online, we don't
111-
// want to emit a false offline with the initial accumulator, so we wait a bit before
112-
// emitting, and rely on `networksWithInternetConnectivity`.
113-
//
114-
// Also if our initial state was "online", but it just got turned off we might not see
115-
// any updates for this network even though we already were registered for updated, and
116-
// thus we can't drop initial value accumulator value.
117-
.debounceFirst(1.seconds)
118-
.onStart {
119-
// We should not use this as initial state in scan, because it may contain networks
120-
// that won't be included in `networkEvents` updates.
121-
emit(
122-
connectivityManager.networksWithInternetConnectivity().also {
123-
Logger.d("Networks (Initial): $it")
124-
}
125-
)
126-
}
127-
.map { it.isNotEmpty() }
128-
.distinctUntilChanged()
129-
}
130-
131-
@Suppress("DEPRECATION")
132-
private fun ConnectivityManager.networksWithInternetConnectivity(): Set<Network> =
133-
// Currently the use of `allNetworks` (which is deprecated in favor of listening to network
134-
// events) is our only option because network events does not give us the initial state fast
135-
// enough.
136-
allNetworks
137-
.filter {
138-
val capabilities = getNetworkCapabilities(it) ?: return@filter false
139-
140-
capabilities.hasCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET) &&
141-
capabilities.hasCapability(NetworkCapabilities.NET_CAPABILITY_NOT_VPN)
142-
}
143-
.toSet()
144-
14576
private fun RawNetworkState.toNetworkState(): NetworkState =
14677
NetworkState(
14778
network.networkHandle,

0 commit comments

Comments
 (0)