-
Notifications
You must be signed in to change notification settings - Fork 326
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
Call AppTransaction.refresh() in certain scenarios if AppTransaction.shared is invalid #4099
base: main
Are you sure you want to change the base?
Conversation
…shared is invalid: - After a purchase - When calling restorePurchaes This will always show an authentication prompt
@@ -1022,7 +1022,7 @@ private extension PurchasesOrchestrator { | |||
} | |||
} | |||
|
|||
func syncPurchases(receiptRefreshPolicy: ReceiptRefreshPolicy, | |||
func syncPurchases(receiptRefreshAllowed: Bool, |
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.
Since ReceiptRefreshPolicy
is very tied to SK1, I changed the parameter to a more SK-agnostic one and decide in the method body what policy to use for each SK version.
@@ -1139,6 +1140,9 @@ private extension PurchasesOrchestrator { | |||
return | |||
} | |||
|
|||
let appTransactionJWS = await self.transactionFetcher.appTransactionJWS( | |||
refreshPolicy: refreshPolicy) |
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.
Moved fetching the AppTransaction
to after the check to see if we have transactions, originalPurchaseDate and originalApplicationVersion.
This way, if we don't need to post the receipt, we wouldn't be fetching the AppTransaction unnecessarily, potentially showing the authentication prompt.
// Only allow refreshing the AppTransaction if the source was a user action (e.g. purchase) | ||
let refreshPolicy: AppTransactionRefreshPolicy = | ||
data.source.initiationSource == .purchase ? .onlyIfEmpty : .never | ||
self.transactionFetcher.appTransactionJWS(refreshPolicy: refreshPolicy) { appTransaction in |
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.
We dont want to potentially show the login prompt for transactions coming from the queue
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.
sounds good, but let's also add that explicitly to the comment, i.e.:
// Only allow refreshing the AppTransaction if the source was a user action (e.g. purchase)
// This is to prevent showing a login prompt unnecessarily
let refreshPolicy: AppTransactionRefreshPolicy =
self.bundleId = appTransaction.bundleID | ||
self.originalApplicationVersion = appTransaction.originalAppVersion | ||
self.originalPurchaseDate = appTransaction.originalPurchaseDate | ||
self.environment = .init(environment: appTransaction.environment) | ||
self.jwsRepresentation = jwsRepresentation |
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.
This is similar to what we already so with the SK2StoreTransaction
. The JWS token is part of the VerificationResult
, not the AppTansaction
, but we want to preserve it.
@available(iOS 15.0, tvOS 15.0, macOS 12.0, watchOS 8.0, *) | ||
private var appTransaction: SK2AppTransaction? { | ||
@available(iOS 16.0, macOS 13.0, tvOS 16.0, watchOS 9.0, *) | ||
private var refreshedAppTransaction: SK2AppTransaction? { |
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.
The unified diff of this is a bit weird.
appTransaction
was modified into the appTransaction(refreshPolicy:)
method below.
and refreshedAppTransaction
was added as a helper to fetch an AppTransaction
via StoreKit.AppTransaction.refresh()
var receivedRefreshPolicy: AppTransactionRefreshPolicy? { | ||
get { return self._receivedRefreshPolicy.value } | ||
set { self._receivedRefreshPolicy.value = newValue } | ||
} |
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.
Added a little helper in the mock so we can check the received refresh policy in different tests.
// Only allow refreshing the AppTransaction if the source was a user action (e.g. purchase) | ||
let refreshPolicy: AppTransactionRefreshPolicy = | ||
data.source.initiationSource == .purchase ? .onlyIfEmpty : .never | ||
self.transactionFetcher.appTransactionJWS(refreshPolicy: refreshPolicy) { appTransaction in |
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.
sounds good, but let's also add that explicitly to the comment, i.e.:
// Only allow refreshing the AppTransaction if the source was a user action (e.g. purchase)
// This is to prevent showing a login prompt unnecessarily
let refreshPolicy: AppTransactionRefreshPolicy =
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.
I don't have context for this change, so you'll likely want approval from someone else too.
case .onlyIfEmpty: | ||
return await refreshedAppTransaction | ||
case .never: | ||
Logger.warn(Strings.storeKit.sk2_error_fetching_app_transaction(error)) |
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.
Would this qualify for a a Logger.appleError
?
When
AppTransaction.shared
is empty or invalid, Apple recommends callingAppTransaction.refresh()
.https://developer.apple.com/documentation/storekit/apptransaction/4020517-refresh
Calling
refresh()
will always show an authentication prompt so we want to make sure we only call it in response to a user action:restorePurchases
Checklist
purchases-android
and hybridsMotivation
AppTransaction.shared
seems to be empty more often than expected. We're hoping refreshing it where applicable will improve the situation.Description