-
Notifications
You must be signed in to change notification settings - Fork 401
Update storefront apis #5455
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
base: main
Are you sure you want to change the base?
Update storefront apis #5455
Changes from 10 commits
9d5a13b
d0de6e3
8759a26
4a55730
b414540
2ae8b81
96d6eb0
8d07066
449f4f0
c3c7065
e33a0b0
aca314a
0e7cbb4
e838910
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 |
|---|---|---|
|
|
@@ -248,7 +248,7 @@ public typealias StartPurchaseBlock = (@escaping PurchaseCompletedBlock) -> Void | |
| } | ||
|
|
||
| @objc public var storeFrontCountryCode: String? { | ||
|
Contributor
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. Should we deprecate this property and provide an async/callback API?
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. I thought about it, but wasn't really sure. It can be done in a separate PR, with these considerations:
Contributor
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. Yeah that's a good point... I think this is something we might want to consider breaking in the next major. Should probably take note in Linear so we don't forget for the next major, whenever that is. |
||
| systemInfo.storefront?.countryCode | ||
| systemInfo.syncStorefront?.countryCode | ||
| } | ||
|
|
||
| private let attributionFetcher: AttributionFetcher | ||
|
|
||
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.
Trying to introduce an
asynccall inHTTPClientgot more complicated than I expected. Introducing aTaskto asynchronous get the Storefront for thedefaultHeadersin this callpurchases-ios/Sources/Networking/HTTPClient/HTTPClient.swift
Lines 99 to 104 in 85c8534
cannot be done without breaking the expectation of requests happening serially, as tested by
purchases-ios/Tests/UnitTests/Networking/HTTPClientTests.swift
Line 1222 in 85c8534
A deeper migration of
HTTPClientto support async/await tasks would be needed, I think.For now, I just went with this workaround to keep the storefront getter synchronous by using a Semaphore. Note that while this would block the current thread, this is OK as long as this blocking never happens in the main thread, which is true since all network operations are added to a background queue. See
purchases-ios/Sources/Networking/Backend.swift
Lines 48 to 54 in 85c8534
The (only) advantage here is that we are now using SK2's storefront when available.
The alternative would be keep using the existing (but deprecated since iOS 26) SK1's storefront getter which is synchronous but also blocking as per its documentation. The advantage is that we wouldn't need this workaround.
@RevenueCat/coresdk please, let me know what you think!