refactor: notification service extension [1] #87
Conversation
eligutovsky
changed the base branch from
master
to
feat/delayed-configuration
eligutovsky
changed the base branch from
feat/delayed-configuration
to
rc/5.6.0
eligutovsky
force-pushed
the
refactor/notification-service-extension
branch
from
8977e93 to
e3f2155
Compare
eligutovsky
force-pushed
the
refactor/notification-service-extension
branch
from
e3f2155 to
20a8284
Compare
eligutovsky
changed the title
eligutovsky
left a comment
eligutovsky
left a comment
There was a problem hiding this comment.
@vvoicehovics @cgwyllie I have added some comments about the code changes
|
|
||
| enum MediaHelper { | ||
| /// Use ``Region.US`` as fallback region. | ||
| static let mediaResizerBaseUrl: String = "https://i-us-east-1.app.delivery" |
There was a problem hiding this comment.
This URL was used here as a fallback URL, in case if no access to the persistent key-value storage.
Now, this code has the storage as a dependency in class init, and also will not be executed in the runtime.
There was a problem hiding this comment.
Do you think it would be helpful to replace the notification content with the text "Error: Check Optimove AppGroup capabilities" if there is no storage available? This could help us reduce the number of integration issues related to missing images.
There was a problem hiding this comment.
After some playground, I found that optimobile Core data throwing an no db file.
| @available(iOS 10.0, *) | ||
| class CategoryManager { | ||
| let categoryReadLock = DispatchSemaphore(value: 0) | ||
| let dynamicCategoryLock = DispatchSemaphore(value: 1) |
There was a problem hiding this comment.
These locks were replaced with the Swift async. You will find the usage below.
|
|
||
| // Force a reload of the categories | ||
| _ = sharedInstance.getExistingCategories() | ||
| await UNUserNotificationCenter.current().notificationCategories() |
There was a problem hiding this comment.
Awaiting for a result instead of manual mutex signaling
|
|
||
| return newArray | ||
| static func writeCategoryIds(_ ids: Set<String>) { | ||
| UserDefaults.standard.set(Array(ids), forKey: Constants.DYNAMIC_CATEGORY) |
There was a problem hiding this comment.
The standard iOS persistence will be replaced with the storage abstraction in this bulk of PRs
| self.categoryReadLock.signal() | ||
| } | ||
|
|
||
| _ = categoryReadLock.wait(timeout: DispatchTime.now() + DispatchTimeInterval.seconds(5)) |
There was a problem hiding this comment.
I'm not certain about deleting this.
The mentioned extension operates as a distinct process and is granted a maximum of 30 seconds to complete its current task. In case the time limit is exceeded, the operating system will invoke another function for this instance and we will have one last opportunity to invoke the callback from the initial method's callback function.
| name: "Optimove", | ||
| platforms: [ | ||
| .iOS(.v10), | ||
| .iOS(.v13), |
Description of Changes
This is the first PR in the series of PRs focused on increasing the reliability of the Notification Service Center by modernizing the code with the latest Swift standards.
Tests cover the new code.
Breaking Changes
Release Checklist
Prepare:
pod lib lintpassesBump versions in:
OptimoveCore.podspecOptimoveNotificationServiceExtension.podspecOptimoveSDK.podspecOptimoveCore/Sources/Classes/Constants/SDKVersion.swiftREADME.mdCHANGELOG.mdUpdate major version numbers in wiki (basic integration + push guides)
Integration tests
T&T Only
Mobile Only
Deferred Deep Links
Combined
Release:
pod trunk pushto publish to CocoaPodsPost Release: