Skip to content

Commit 6359fcc

Browse files
TimoPtrTimoPtr
authored
Only register one network callback at the time (#6435)
* Only register one network callback at the time * Add missing test
1 parent 39c3907 commit 6359fcc

7 files changed

Lines changed: 262 additions & 110 deletions

File tree

‎common/src/main/kotlin/io/homeassistant/companion/android/common/data/network/NetworkChangeObserver.kt‎

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
package io.homeassistant.companion.android.common.data.network
2+
3+
import android.net.ConnectivityManager
4+
import android.net.Network
5+
import android.net.NetworkCapabilities
6+
import android.net.NetworkRequest
7+
import androidx.annotation.VisibleForTesting
8+
import javax.inject.Inject
9+
import javax.inject.Singleton
10+
import kotlinx.coroutines.CoroutineScope
11+
import kotlinx.coroutines.Dispatchers
12+
import kotlinx.coroutines.SupervisorJob
13+
import kotlinx.coroutines.channels.awaitClose
14+
import kotlinx.coroutines.flow.Flow
15+
import kotlinx.coroutines.flow.SharedFlow
16+
import kotlinx.coroutines.flow.SharingStarted
17+
import kotlinx.coroutines.flow.callbackFlow
18+
import kotlinx.coroutines.flow.shareIn
19+
import timber.log.Timber
20+
21+
/**
22+
* Observes network connectivity changes using [ConnectivityManager] and exposes them
23+
* as a [SharedFlow]. The underlying callback is shared across all subscribers.
24+
*
25+
* All components that need to react to network changes must use this shared observer
26+
* instead of registering their own [ConnectivityManager.NetworkCallback].
27+
* Android enforces a strict per-app limit on registered network callbacks and throws
28+
* `android.net.ConnectivityManager.TooManyRequestsException` when that limit is exceeded.
29+
* Sharing a single callback registration avoids hitting this limit.
30+
*/
31+
interface NetworkChangeObserver {
32+
/**
33+
* Emits [Unit] whenever the network state changes (available, lost, capabilities changed).
34+
* Replays the last emission to new subscribers.
35+
* The [ConnectivityManager] callback is registered while there are active subscribers.
36+
*/
37+
val observerNetworkChange: Flow<Unit>
38+
}
39+
40+
@Singleton
41+
internal class NetworkChangeObserverImpl @VisibleForTesting constructor(
42+
private val connectivityManager: ConnectivityManager,
43+
private val scope: CoroutineScope
44+
) : NetworkChangeObserver {
45+
46+
@Inject
47+
constructor(
48+
connectivityManager: ConnectivityManager,
49+
) : this(connectivityManager, CoroutineScope(SupervisorJob() + Dispatchers.Default))
50+
51+
override val observerNetworkChange: Flow<Unit> = callbackFlow {
52+
val networkRequest = NetworkRequest.Builder().build()
53+
54+
val callback = object : ConnectivityManager.NetworkCallback() {
55+
override fun onAvailable(network: Network) {
56+
trySend(Unit)
57+
}
58+
59+
override fun onLost(network: Network) {
60+
trySend(Unit)
61+
}
62+
63+
override fun onCapabilitiesChanged(network: Network, capabilities: NetworkCapabilities) {
64+
trySend(Unit)
65+
}
66+
}
67+
68+
Timber.d("Register network callback")
69+
connectivityManager.registerNetworkCallback(networkRequest, callback)
70+
71+
trySend(Unit)
72+
73+
awaitClose {
74+
Timber.d("Unregister network callback")
75+
connectivityManager.unregisterNetworkCallback(callback)
76+
}
77+
}.shareIn(
78+
scope = scope,
79+
started = SharingStarted.WhileSubscribed(),
80+
replay = 1,
81+
)
82+
}

‎common/src/main/kotlin/io/homeassistant/companion/android/common/data/network/NetworkStatusMonitor.kt‎

Lines changed: 2 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,12 @@
11
package io.homeassistant.companion.android.common.data.network
22

3-
import android.net.ConnectivityManager
4-
import android.net.Network
5-
import android.net.NetworkCapabilities
6-
import android.net.NetworkRequest
73
import io.homeassistant.companion.android.common.data.servers.ServerConnectionStateProvider
84
import io.homeassistant.companion.android.util.isPubliclyAccessible
95
import javax.inject.Inject
106
import javax.inject.Singleton
117
import kotlin.coroutines.cancellation.CancellationException
128
import kotlinx.coroutines.ExperimentalCoroutinesApi
13-
import kotlinx.coroutines.channels.awaitClose
149
import kotlinx.coroutines.flow.Flow
15-
import kotlinx.coroutines.flow.callbackFlow
1610
import kotlinx.coroutines.flow.distinctUntilChanged
1711
import kotlinx.coroutines.flow.mapLatest
1812
import timber.log.Timber
@@ -72,36 +66,13 @@ enum class NetworkState {
7266
*/
7367
@Singleton
7468
internal class NetworkStatusMonitorImpl @Inject constructor(
75-
private val connectivityManager: ConnectivityManager,
69+
private val networkChangeObserver: NetworkChangeObserver,
7670
private val networkHelper: NetworkHelper,
7771
) : NetworkStatusMonitor {
7872

7973
@OptIn(ExperimentalCoroutinesApi::class)
8074
override fun observeNetworkStatus(connectionStateProvider: ServerConnectionStateProvider): Flow<NetworkState> =
81-
callbackFlow {
82-
val networkRequest = NetworkRequest.Builder().build()
83-
84-
val callback = object : ConnectivityManager.NetworkCallback() {
85-
override fun onAvailable(network: Network) {
86-
trySend(Unit)
87-
}
88-
89-
override fun onLost(network: Network) {
90-
trySend(Unit)
91-
}
92-
93-
override fun onCapabilitiesChanged(network: Network, capabilities: NetworkCapabilities) {
94-
trySend(Unit)
95-
}
96-
}
97-
98-
connectivityManager.registerNetworkCallback(networkRequest, callback)
99-
trySend(Unit) // Emit status at start
100-
101-
awaitClose {
102-
connectivityManager.unregisterNetworkCallback(callback)
103-
}
104-
}.mapLatest {
75+
networkChangeObserver.observerNetworkChange.mapLatest {
10576
getCurrentNetworkState(connectionStateProvider)
10677
}.distinctUntilChanged()
10778

‎common/src/main/kotlin/io/homeassistant/companion/android/common/data/servers/ServerConnectionStateProviderImpl.kt‎

Lines changed: 3 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,11 @@ import android.content.Intent
77
import android.content.IntentFilter
88
import android.content.pm.PackageManager
99
import android.location.LocationManager
10-
import android.net.ConnectivityManager
11-
import android.net.Network
12-
import android.net.NetworkCapabilities
13-
import android.net.NetworkRequest
1410
import androidx.core.content.ContextCompat
1511
import dagger.assisted.Assisted
1612
import dagger.assisted.AssistedInject
1713
import dagger.hilt.android.qualifiers.ApplicationContext
14+
import io.homeassistant.companion.android.common.data.network.NetworkChangeObserver
1815
import io.homeassistant.companion.android.common.data.network.NetworkHelper
1916
import io.homeassistant.companion.android.common.data.network.WifiHelper
2017
import io.homeassistant.companion.android.common.util.DisabledLocationHandler
@@ -38,7 +35,7 @@ class ServerConnectionStateProviderImpl @AssistedInject constructor(
3835
private val serverDao: ServerDao,
3936
private val wifiHelper: WifiHelper,
4037
private val networkHelper: NetworkHelper,
41-
private val connectivityManager: ConnectivityManager,
38+
private val networkChangeObserver: NetworkChangeObserver,
4239
@Assisted private val serverId: Int,
4340
) : ServerConnectionStateProvider {
4441

@@ -155,7 +152,7 @@ class ServerConnectionStateProviderImpl @AssistedInject constructor(
155152
return merge(
156153
flowOf(Unit), // Used to trigger a getUrl
157154
observeLocationState(),
158-
observeHomeNetworkState(),
155+
networkChangeObserver.observerNetworkChange,
159156
observeConnectionInfoChanges(),
160157
).map {
161158
val connection = connection()
@@ -212,32 +209,6 @@ class ServerConnectionStateProviderImpl @AssistedInject constructor(
212209
}
213210
}
214211

215-
private fun observeHomeNetworkState(): Flow<Unit> = callbackFlow {
216-
val networkRequest = NetworkRequest.Builder().build()
217-
218-
val callback = object : ConnectivityManager.NetworkCallback() {
219-
override fun onAvailable(network: Network) {
220-
trySend(Unit)
221-
}
222-
223-
override fun onLost(network: Network) {
224-
trySend(Unit)
225-
}
226-
227-
override fun onCapabilitiesChanged(network: Network, capabilities: NetworkCapabilities) {
228-
trySend(Unit)
229-
}
230-
}
231-
232-
connectivityManager.registerNetworkCallback(networkRequest, callback)
233-
234-
// Emit initial state
235-
trySend(Unit)
236-
237-
awaitClose {
238-
connectivityManager.unregisterNetworkCallback(callback)
239-
}
240-
}
241212
}
242213

243214
private fun HttpUrl.buildWebhookUrl(webhookId: String): HttpUrl {

‎common/src/main/kotlin/io/homeassistant/companion/android/di/NetworkModule.kt‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ import dagger.Provides
1010
import dagger.hilt.InstallIn
1111
import dagger.hilt.android.qualifiers.ApplicationContext
1212
import dagger.hilt.components.SingletonComponent
13+
import io.homeassistant.companion.android.common.data.network.NetworkChangeObserver
14+
import io.homeassistant.companion.android.common.data.network.NetworkChangeObserverImpl
1315
import io.homeassistant.companion.android.common.data.network.NetworkHelper
1416
import io.homeassistant.companion.android.common.data.network.NetworkHelperImpl
1517
import io.homeassistant.companion.android.common.data.network.NetworkStatusMonitor
@@ -44,6 +46,10 @@ internal abstract class NetworkModule {
4446
@Singleton
4547
abstract fun bindNetworkHelper(networkHelper: NetworkHelperImpl): NetworkHelper
4648

49+
@Binds
50+
@Singleton
51+
abstract fun bindNetworkChangeObserver(networkChangeObserver: NetworkChangeObserverImpl): NetworkChangeObserver
52+
4753
@Binds
4854
@Singleton
4955
abstract fun bindNetworkStatusMonitor(networkStatusMonitor: NetworkStatusMonitorImpl): NetworkStatusMonitor

‎common/src/test/kotlin/io/homeassistant/companion/android/common/data/network/NetworkChangeObserverImplTest.kt‎

Lines changed: 146 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,146 @@
1+
package io.homeassistant.companion.android.common.data.network
2+
3+
import android.net.ConnectivityManager
4+
import android.net.Network
5+
import android.net.NetworkCapabilities
6+
import android.net.NetworkRequest
7+
import app.cash.turbine.test
8+
import app.cash.turbine.turbineScope
9+
import io.homeassistant.companion.android.testing.unit.ConsoleLogExtension
10+
import io.mockk.CapturingSlot
11+
import io.mockk.Runs
12+
import io.mockk.every
13+
import io.mockk.just
14+
import io.mockk.mockk
15+
import io.mockk.slot
16+
import io.mockk.verify
17+
import kotlinx.coroutines.CoroutineScope
18+
import kotlinx.coroutines.ExperimentalCoroutinesApi
19+
import kotlinx.coroutines.test.TestScope
20+
import kotlinx.coroutines.test.UnconfinedTestDispatcher
21+
import kotlinx.coroutines.test.runTest
22+
import org.junit.jupiter.api.BeforeEach
23+
import org.junit.jupiter.api.Test
24+
import org.junit.jupiter.api.extension.ExtendWith
25+
26+
@OptIn(ExperimentalCoroutinesApi::class)
27+
@ExtendWith(ConsoleLogExtension::class)
28+
class NetworkChangeObserverImplTest {
29+
30+
private val connectivityManager: ConnectivityManager = mockk()
31+
private val network: Network = mockk()
32+
private val networkCapabilities: NetworkCapabilities = mockk()
33+
34+
private lateinit var observer: NetworkChangeObserverImpl
35+
private lateinit var callbackSlot: CapturingSlot<ConnectivityManager.NetworkCallback>
36+
37+
@BeforeEach
38+
fun setup() {
39+
callbackSlot = slot()
40+
41+
every {
42+
connectivityManager.registerNetworkCallback(
43+
any<NetworkRequest>(),
44+
capture(callbackSlot),
45+
)
46+
} just Runs
47+
48+
every {
49+
connectivityManager.unregisterNetworkCallback(any<ConnectivityManager.NetworkCallback>())
50+
} just Runs
51+
}
52+
53+
private fun createObserver(testScope: TestScope) {
54+
observer = NetworkChangeObserverImpl(
55+
connectivityManager,
56+
CoroutineScope(UnconfinedTestDispatcher(testScope.testScheduler)),
57+
)
58+
}
59+
60+
@Test
61+
fun `Given observer when first subscriber collects then registers callback and emits initial value`() = runTest {
62+
createObserver(this)
63+
64+
observer.observerNetworkChange.test {
65+
awaitItem()
66+
verify { connectivityManager.registerNetworkCallback(any<NetworkRequest>(), any<ConnectivityManager.NetworkCallback>()) }
67+
cancelAndIgnoreRemainingEvents()
68+
}
69+
}
70+
71+
@Test
72+
fun `Given observer when all subscribers cancel then unregisters callback`() = runTest {
73+
createObserver(this)
74+
75+
observer.observerNetworkChange.test {
76+
awaitItem()
77+
cancelAndIgnoreRemainingEvents()
78+
}
79+
80+
verify { connectivityManager.unregisterNetworkCallback(callbackSlot.captured) }
81+
}
82+
83+
@Test
84+
fun `Given two subscribers then registers and unregister callbacks are invoked only once`() = runTest {
85+
createObserver(this)
86+
87+
turbineScope {
88+
val subscriber1 = observer.observerNetworkChange.testIn(backgroundScope)
89+
subscriber1.awaitItem()
90+
91+
val subscriber2 = observer.observerNetworkChange.testIn(backgroundScope)
92+
subscriber2.awaitItem()
93+
94+
verify(exactly = 1) {
95+
connectivityManager.registerNetworkCallback(any<NetworkRequest>(), any<ConnectivityManager.NetworkCallback>())
96+
}
97+
98+
subscriber1.cancelAndIgnoreRemainingEvents()
99+
100+
verify(exactly = 0) {
101+
connectivityManager.unregisterNetworkCallback(any<ConnectivityManager.NetworkCallback>())
102+
}
103+
104+
subscriber2.cancelAndIgnoreRemainingEvents()
105+
verify(exactly = 1) {
106+
connectivityManager.unregisterNetworkCallback(any<ConnectivityManager.NetworkCallback>())
107+
}
108+
}
109+
}
110+
111+
@Test
112+
fun `Given active subscription when onAvailable called then emits`() = runTest {
113+
createObserver(this)
114+
115+
observer.observerNetworkChange.test {
116+
awaitItem() // Initial emission
117+
callbackSlot.captured.onAvailable(network)
118+
awaitItem()
119+
cancelAndIgnoreRemainingEvents()
120+
}
121+
}
122+
123+
@Test
124+
fun `Given active subscription when onLost called then emits`() = runTest {
125+
createObserver(this)
126+
127+
observer.observerNetworkChange.test {
128+
awaitItem() // Initial emission
129+
callbackSlot.captured.onLost(network)
130+
awaitItem()
131+
cancelAndIgnoreRemainingEvents()
132+
}
133+
}
134+
135+
@Test
136+
fun `Given active subscription when onCapabilitiesChanged called then emits`() = runTest {
137+
createObserver(this)
138+
139+
observer.observerNetworkChange.test {
140+
awaitItem() // Initial emission
141+
callbackSlot.captured.onCapabilitiesChanged(network, networkCapabilities)
142+
awaitItem()
143+
cancelAndIgnoreRemainingEvents()
144+
}
145+
}
146+
}

0 commit comments

Comments
 (0)