Add network constraints for fetching notifications with WorkManager#6305
Add network constraints for fetching notifications with WorkManager#6305jmartinesp wants to merge 9 commits intodevelopfrom
WorkManager#6305Conversation
WorkManager
|
📱 Scan the QR code below to install the build (arm64 only) for this PR. |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #6305 +/- ##
========================================
Coverage 81.27% 81.27%
========================================
Files 2576 2575 -1
Lines 70105 70135 +30
Branches 9002 9005 +3
========================================
+ Hits 56975 57000 +25
- Misses 9777 9782 +5
Partials 3353 3353 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
ad00ef6 to
24fa6b3
Compare
…ent()` since it's an enterprise feature
…raints: Requesting the OS to have internet connectivity before starting a fetch notifications worker. This feature flag is enabled by default, but can be disabled in case it's problematic.
…rvice`, check in `DefaultNetworkMonitor` if the build is enterprise before updating the value.
144457b to
03595f2
Compare
|
bmarty
left a comment
There was a problem hiding this comment.
Thanks, only minor remarks and one question
| val isNetworkBlocked: StateFlow<Boolean> | ||
|
|
||
| /** | ||
| * A flow indicating whether the app is running in an air-gapped environment. |
There was a problem hiding this comment.
| * A flow indicating whether the app is running in an air-gapped environment. | |
| * A flow indicating whether the app is running in an air-gapped environment. |
| workManagerScheduler = workManagerScheduler, | ||
| buildVersionSdkIntProvider = buildVersionSdkIntProvider, | ||
| resultProcessor = resultProcessor, | ||
| syncPendingNotificationsRequestFactory = object : SyncPendingNotificationsRequestBuilder.Factory { |
There was a problem hiding this comment.
Maybe create a FakeSyncPendingNotificationsRequestFactory class?
| @@ -0,0 +1,151 @@ | |||
| /* | |||
| * Copyright (c) 2025 Element Creations Ltd. | |||
| * Copyright 2025 New Vector Ltd. | |||
There was a problem hiding this comment.
Copyright (c) 2026 Element Creations Ltd. should be enough for new files.
There was a problem hiding this comment.
Agh, this is because the file was renamed although git thinks it's new. I'll change it anyway.
|
|
||
| override fun isNetworkBlocked(): Boolean = blockedNetworkBlockedChecker.isNetworkBlocked() | ||
| override val isNetworkBlocked = MutableStateFlow(false) | ||
| override val isInAirGappedEnvironment = MutableStateFlow(false) |
There was a problem hiding this comment.
Is it fine to initialize the 2 stateflow with false? Maybe read values as it was done before in NetworkBlockedChecker?
There was a problem hiding this comment.
When the app is started, there's no chance network can be blocked by Doze, so isNetworkBlocked will always be false. And for the air-gapped env check we need to receive the callback to actually know what's its status, I think.
There was a problem hiding this comment.
When the app is started, there's no chance network can be blocked by Doze
The app can be started by a Push, so are you sure this is true?
There was a problem hiding this comment.
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 😅 .



Content
isNetworkBlockedandisInAirGappedEnvironmenttoNetworkMonitor, which are populated fromNetworkCallback. We can removeNetworkBlockedCheckernow.SyncPendingNotificationsRequestBuilderso we use assisted injection now and don't have to add its dependencies to the instantiator.EnterpriseService.isInAirGappedEnvironment.NetworkCapabilities.NET_CAPABILITY_VALIDATEDconstraint to the notification fetching workers (which checks Google's204endpoints to know if the network has internet connectivity).Motivation and context
Part of #6209.
Tests
Things that can be checked:
https://www.google.com/generate_204andhttp://connectivitycheck.gstatic.com/generate_204, your device will display your network not having internet connectivity, and the app should know it's in an air-gapped env. If using this you receive a notification, it should still be fetched.Tested devices
Checklist