-
Notifications
You must be signed in to change notification settings - Fork 423
Add network constraints for fetching notifications with WorkManager
#6305
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 9 commits
ec96dc7
bb6b493
75bc189
a3133a7
dd07d52
fd043c5
0fcf8d8
d656afd
03595f2
0b63f74
a3afd98
24ac364
35ae938
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,18 +13,21 @@ package io.element.android.features.networkmonitor.impl | |
| import android.content.Context | ||
| import android.net.ConnectivityManager | ||
| import android.net.Network | ||
| import android.net.NetworkCapabilities | ||
| import android.net.NetworkRequest | ||
| import dev.zacsweers.metro.AppScope | ||
| import dev.zacsweers.metro.ContributesBinding | ||
| import dev.zacsweers.metro.SingleIn | ||
| import io.element.android.features.networkmonitor.api.NetworkMonitor | ||
| import io.element.android.features.networkmonitor.api.NetworkStatus | ||
| import io.element.android.libraries.core.meta.BuildMeta | ||
| import io.element.android.libraries.di.annotations.AppCoroutineScope | ||
| import io.element.android.libraries.di.annotations.ApplicationContext | ||
| import kotlinx.coroutines.CoroutineScope | ||
| import kotlinx.coroutines.FlowPreview | ||
| import kotlinx.coroutines.channels.awaitClose | ||
| import kotlinx.coroutines.channels.trySendBlocking | ||
| import kotlinx.coroutines.flow.MutableStateFlow | ||
| import kotlinx.coroutines.flow.SharingStarted | ||
| import kotlinx.coroutines.flow.StateFlow | ||
| import kotlinx.coroutines.flow.callbackFlow | ||
|
|
@@ -39,13 +42,13 @@ import java.util.concurrent.atomic.AtomicInteger | |
| @SingleIn(AppScope::class) | ||
| class DefaultNetworkMonitor( | ||
| @ApplicationContext context: Context, | ||
| @AppCoroutineScope | ||
| appCoroutineScope: CoroutineScope, | ||
| @AppCoroutineScope appCoroutineScope: CoroutineScope, | ||
| private val buildMeta: BuildMeta, | ||
| ) : NetworkMonitor { | ||
| private val connectivityManager: ConnectivityManager = context.getSystemService(ConnectivityManager::class.java) | ||
| private val blockedNetworkBlockedChecker = NetworkBlockedChecker(connectivityManager) | ||
|
|
||
| override fun isNetworkBlocked(): Boolean = blockedNetworkBlockedChecker.isNetworkBlocked() | ||
| override val isNetworkBlocked = MutableStateFlow(false) | ||
| override val isInAirGappedEnvironment = MutableStateFlow(false) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it fine to initialize the 2 stateflow with
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When the app is started, there's no chance network can be blocked by Doze, so
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The app can be started by a Push, so are you sure this is true?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, good call. It should always be exempted from network restrictions in that case, according to the docs but well... we all know how reliable Google docs are 😅 .
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's "true" for Firebase, but not really for UnifiedPush...
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 24ac364 should fix it |
||
|
|
||
| override val connectivity: StateFlow<NetworkStatus> = callbackFlow { | ||
|
|
||
|
|
@@ -63,6 +66,27 @@ class DefaultNetworkMonitor( | |
| } | ||
| } | ||
|
|
||
| override fun onBlockedStatusChanged(network: Network, blocked: Boolean) { | ||
| Timber.d("Network ${network.networkHandle} blocked status changed: $blocked.") | ||
| if (network.networkHandle == connectivityManager.activeNetwork?.networkHandle) { | ||
| // If the network is blocked, it means that Doze is preventing the app from using the network, even if it's available. | ||
| isNetworkBlocked.value = blocked | ||
| } | ||
| } | ||
|
|
||
| override fun onCapabilitiesChanged(network: Network, networkCapabilities: NetworkCapabilities) { | ||
| if (!buildMeta.isEnterpriseBuild) { | ||
| // The air-gapped environment detection is only relevant for the enterprise build. | ||
| return | ||
| } | ||
|
|
||
| if (network.networkHandle == connectivityManager.activeNetwork?.networkHandle) { | ||
| // If the network doesn't have the NET_CAPABILITY_VALIDATED capability, it means that the network is not able to reach the internet | ||
| // (according to Google), which is a common case in air-gapped environments. | ||
| isInAirGappedEnvironment.value = !networkCapabilities.capabilities.contains(NetworkCapabilities.NET_CAPABILITY_VALIDATED) | ||
| } | ||
| } | ||
|
|
||
| override fun onAvailable(network: Network) { | ||
| if (activeNetworksCount.incrementAndGet() > 0) { | ||
| trySendBlocking(NetworkStatus.Connected) | ||
|
|
||
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -26,15 +26,16 @@ import io.element.android.libraries.push.impl.history.PushHistoryService | |
| import io.element.android.libraries.push.impl.notifications.FakeNotificationResultProcessor | ||
| import io.element.android.libraries.push.impl.test.DefaultTestPush | ||
| import io.element.android.libraries.push.impl.troubleshoot.DiagnosticPushHandler | ||
| import io.element.android.libraries.push.impl.workmanager.SyncPendingNotificationsRequestBuilder | ||
| import io.element.android.libraries.pushproviders.api.PushData | ||
| import io.element.android.libraries.pushstore.api.clientsecret.PushClientSecret | ||
| import io.element.android.libraries.pushstore.test.userpushstore.FakeUserPushStore | ||
| import io.element.android.libraries.pushstore.test.userpushstore.FakeUserPushStoreFactory | ||
| import io.element.android.libraries.pushstore.test.userpushstore.clientsecret.FakePushClientSecret | ||
| import io.element.android.libraries.workmanager.api.WorkManagerRequestBuilder | ||
| import io.element.android.libraries.workmanager.api.WorkManagerRequestWrapper | ||
| import io.element.android.libraries.workmanager.test.FakeWorkManagerScheduler | ||
| import io.element.android.services.analytics.test.FakeAnalyticsService | ||
| import io.element.android.services.toolbox.test.sdk.FakeBuildVersionSdkIntProvider | ||
| import io.element.android.services.toolbox.test.systemclock.FakeSystemClock | ||
| import io.element.android.tests.testutils.lambda.lambdaError | ||
| import io.element.android.tests.testutils.lambda.lambdaRecorder | ||
|
|
@@ -216,7 +217,6 @@ class DefaultPushHandlerTest { | |
| workManagerScheduler: FakeWorkManagerScheduler = FakeWorkManagerScheduler(), | ||
| analyticsService: FakeAnalyticsService = FakeAnalyticsService(), | ||
| systemClock: FakeSystemClock = FakeSystemClock(), | ||
| buildVersionSdkIntProvider: FakeBuildVersionSdkIntProvider = FakeBuildVersionSdkIntProvider(33), | ||
| resultProcessor: FakeNotificationResultProcessor = FakeNotificationResultProcessor( | ||
| emit = { Result.success(Unit) }, | ||
| start = {}, | ||
|
|
@@ -238,8 +238,16 @@ class DefaultPushHandlerTest { | |
| analyticsService = analyticsService, | ||
| systemClock = systemClock, | ||
| workManagerScheduler = workManagerScheduler, | ||
| buildVersionSdkIntProvider = buildVersionSdkIntProvider, | ||
| resultProcessor = resultProcessor, | ||
| syncPendingNotificationsRequestFactory = object : SyncPendingNotificationsRequestBuilder.Factory { | ||
|
||
| override fun create(sessionId: SessionId): SyncPendingNotificationsRequestBuilder { | ||
| return object : SyncPendingNotificationsRequestBuilder { | ||
| override suspend fun build(): Result<List<WorkManagerRequestWrapper>> { | ||
| return Result.success(emptyList()) | ||
| } | ||
| } | ||
| } | ||
| } | ||
| ) | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.