-
Notifications
You must be signed in to change notification settings - Fork 283
[Chore] Split background tasks on develop branch #728
[Chore] Split background tasks on develop branch #728
Conversation
- Added separate task for fetch-test-results back into app - Use generic $(PRODUCT_BUNDLE_IDENTIFIER) taskID in Info.plist - Use Bundle.main.bundleIdentifier as prefix in ENATaskScheduler - Ensure execute functions are handled separately
…it-background-tasks-on-develop-branch * fix/split-background-tasks-on-development: Use generic $(PRODUCT_BUNDLE_IDENTIFIER) taskID in Info.plist Split out execution of background tasks into separate tasks: - Added separate task for fetch-test-results back into app - Use generic $(PRODUCT_BUNDLE_IDENTIFIER) taskID in Info.plist - Use Bundle.main.bundleIdentifier as prefix in ENATaskScheduler - Ensure execute functions are handled separately # Conflicts: # src/xcode/ENA/ENA/Source/Models/Task Scheduling/ENATaskScheduler.swift
* develop: fix wrong url Bring back non-breakable whitespaces Removed duplicate table entry further clarification Prominently add feature requests target Add visible not about closing PRs w/o issue assigned Add explicit link to cwa-wishlist repo Clarify contribution guidelines [INTERNAL] Translation delivery: commit by LX Lab [INTERNAL] Translation delivery: commit by LX Lab fixed risk view [INTERNAL] Translation delivery: commit by LX Lab
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.
Splitting the background task into two separate ones makes the code a lot easier to understand. Thanks!
src/xcode/ENA/ENA/Source/Models/Task Scheduling/ENATaskScheduler.swift
Outdated
Show resolved
Hide resolved
src/xcode/ENA/ENA/Source/Models/Task Scheduling/ENATaskScheduler.swift
Outdated
Show resolved
Hide resolved
src/xcode/ENA/ENA/Source/Models/Task Scheduling/ENATaskScheduler.swift
Outdated
Show resolved
Hide resolved
src/xcode/ENA/ENA/Source/Models/Task Scheduling/ENATaskScheduler.swift
Outdated
Show resolved
Hide resolved
@@ -62,12 +66,18 @@ final class ENATaskScheduler { | |||
|
|||
func scheduleTasks() { | |||
scheduleTask(for: .primaryBackgroundTask, cancelExisting: true) | |||
scheduleTask(for: .secondaryBackgroundTask, cancelExisting: true) | |||
} | |||
|
|||
func cancelTasks() { | |||
BGTaskScheduler.shared.cancelAllTaskRequests() |
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.
If you cancel all task requests, doesn't it mean that you would be canceling the secondaryBackgroundTask
from the primaryBackgroundTask
and vice versa? Do you really want this?
Nevermind, I misread.
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.
👍
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.
Just a few minor questions, the rest looks fine to me :)
Renamed secondaryBackgroundTask to fetchTestResults Fixed typo for taskHander to taskHandler
Looks awesome! Thanks a lot! 🎈 🥳 |
<string>$(PRODUCT_BUNDLE_IDENTIFIER).exposure-notification</string> | ||
<string>$(PRODUCT_BUNDLE_IDENTIFIER).fetch-test-results</string> |
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.
why do you replace it with the environment var?
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.
i guess its done to make sure there are no collisions with different app versions, e.g. one that was installed via test flight vs. a debug build? (e.g. the debug target uses de.rki.coronawarnapp-dev, production targets use de.rki.coronawarnapp)
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.
It allows us to use the app bundle ID as the prefix for the identifier. In the code, we use Bundle.main.bundleIdentifier as the prefix. This means that we can deploy either app targets during dev (eg from 'de.rki.coronawarnapp' to 'de.rki.coronawarnapp-dev'), and ensure that background tasks from one instance will not collide with the other app's background task identifiers. It's also the method used by Apple's sample code for exposure notifications.
* develop: (39 commits) Fixed the height of Update button on Exposure Detection screen. Apply ENAFooterView to invite friends controller. Added full ENAButton dynamic type support. Removed ENACloneButton. Fixed lost navigation item observer when dismissing modal presentation. Minor fix. Disabled next button when sharing keys. Fixed ENATanInput bottom line visibility when entering text. Fixed fragile TAN input first responder handling for error state. Fixed spinner animation when injecting. Updated tests. Fixed IB warning. Improved share text. Add missing label Update pr guidelines Switch to grenrc.js Update gren config Bump version Integrate gren for release notes Removed deprecated file. ...
This PR changes the operation of the background tasks.
Previously, the background task only triggered once every 4 hours minimum. On triggering the task, the fetch-test-results API call is executed, and on completion, the exposure-detection API call is executed, effectively executing the two API asynchronous calls in series.
This change splits the execution of the API calls into two separate tasks. This allows the exposure detection call to maintain the higher priority, and adhere to the minimum timing specified by the ExposureNotification framework.
No changes have been made to the logic to determine whether to bypass the API calls, which are still driven by the RiskProvider class (for the exposure-detection API call) and the ENAExposureSubmissionService class (for the fetch-test-results API call)