CustomerInfoManager: post transactions in parallel to POST receipts only once#2954
Merged
Conversation
NachoSoto
commented
Aug 2, 2023
Contributor
Author
There was a problem hiding this comment.
Made this a trailing closure which is simpler to use.
NachoSoto
commented
Aug 2, 2023
NachoSoto
commented
Aug 2, 2023
Contributor
Author
There was a problem hiding this comment.
Equivalent log to the one for addedToExistingInFlightList, useful for debugging.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2954 +/- ##
=======================================
Coverage 86.69% 86.70%
=======================================
Files 219 219
Lines 15619 15636 +17
=======================================
+ Hits 13541 13557 +16
- Misses 2078 2079 +1 ☔ View full report in Codecov by Sentry. |
39b2f8f to
8d1d751
Compare
joshdholtz
reviewed
Aug 7, 2023
8d1d751 to
e214dfe
Compare
NachoSoto
commented
Aug 11, 2023
e214dfe to
75f5734
Compare
aboedo
approved these changes
Aug 11, 2023
Comment on lines
+383
to
+386
| await withTaskGroup(of: Void.self) { group in | ||
| for transaction in transactions { | ||
| group.addTask { | ||
| _ = await self.transactionPoster.handlePurchasedTransaction( |
Contributor
Author
There was a problem hiding this comment.
TransactionPoster already has a log in handlePurchasedTransaction.
NachoSoto
added a commit
that referenced
this pull request
Aug 14, 2023
…p launch Possibly as a consequence of #2623 we were calling `updateAllCachesIfNeeded` on app launch together with `updateCachesIfInForeground`, meaning that we were updating all caches twice. Thanks to our de-duping logic this is normally not a big issue, but this issue combined with what #2954 fixes, means that if there are pending transactions, opening the app would probably end up posting receipts duplicated times. This adds a new log as well to detect this: > [purchases] DEBUG: ℹ️ Throttling cache update, only 0.8 seconds elapsed And a new integration test that posts `UIApplication.willEnterForegroundNotification` to test this.
4 tasks
This was referenced Aug 16, 2023
NachoSoto
pushed a commit
that referenced
this pull request
Aug 17, 2023
**This is an automatic release.** ### Bugfixes * `PurchasesOrchestrator`: fixed callback not invoked regression during downgrades (#3028) via NachoSoto (@NachoSoto) * `TransactionPoster`: don't finish transactions for non-subscriptions if they're not processed (#2841) via NachoSoto (@NachoSoto) ### Performance Improvements * `StoreKit 2`: only listen to `StoreKit.Transaction.updates` when SK2 is enabled (#3032) via NachoSoto (@NachoSoto) * `CustomerInfoManager`: post transactions in parallel to POST receipts only once (#2954) via NachoSoto (@NachoSoto) ### Other Changes * `PostedTransactionCache`: remove implementation (#3030) via NachoSoto (@NachoSoto) * `Integration Tests`: improved `testCanPurchaseMultipleSubscriptions` (#3025) via NachoSoto (@NachoSoto) * `GitHub`: improved `ISSUE_TEMPLATE` (#3022) via NachoSoto (@NachoSoto) * `TransactionPoster`: added transaction ID and Date to log (#3026) via NachoSoto (@NachoSoto) * `TransactionPoster`: fix iOS 12 test (#3018) via NachoSoto (@NachoSoto) * `SystemInfo`: added `ClockType` (#3014) via NachoSoto (@NachoSoto) * `Integration Tests`: begin tests with `UIApplication.willEnterForegroundNotification` to simulate a real app (#3015) via NachoSoto (@NachoSoto) * `Integration Tests`: add tests to verify `CustomerInfo`+`Offerings` request de-dupping (#3013) via NachoSoto (@NachoSoto) * `SwiftLint`: disable `unneeded_synthesized_initializer` (#3010) via NachoSoto (@NachoSoto) * Added `internal` `NonSubscriptionTransaction.storeTransactionIdentifier` (#3009) via NachoSoto (@NachoSoto) * `Integration Tests`: added tests for non-renewing and non-consumable packages (#3008) via NachoSoto (@NachoSoto) * Expanded `EnsureNonEmptyArrayDecodable` to `EnsureNonEmptyCollectionDecodable` (#3002) via NachoSoto (@NachoSoto)
MarkVillacampa
pushed a commit
that referenced
this pull request
Sep 6, 2023
… only once (#2954) The existing implementation was posting them in sequence, which meant that it was always going to make N requests. With this new implementation we can ensure that only one request is made for all transactions.
MarkVillacampa
pushed a commit
that referenced
this pull request
Sep 6, 2023
**This is an automatic release.** ### Bugfixes * `PurchasesOrchestrator`: fixed callback not invoked regression during downgrades (#3028) via NachoSoto (@NachoSoto) * `TransactionPoster`: don't finish transactions for non-subscriptions if they're not processed (#2841) via NachoSoto (@NachoSoto) ### Performance Improvements * `StoreKit 2`: only listen to `StoreKit.Transaction.updates` when SK2 is enabled (#3032) via NachoSoto (@NachoSoto) * `CustomerInfoManager`: post transactions in parallel to POST receipts only once (#2954) via NachoSoto (@NachoSoto) ### Other Changes * `PostedTransactionCache`: remove implementation (#3030) via NachoSoto (@NachoSoto) * `Integration Tests`: improved `testCanPurchaseMultipleSubscriptions` (#3025) via NachoSoto (@NachoSoto) * `GitHub`: improved `ISSUE_TEMPLATE` (#3022) via NachoSoto (@NachoSoto) * `TransactionPoster`: added transaction ID and Date to log (#3026) via NachoSoto (@NachoSoto) * `TransactionPoster`: fix iOS 12 test (#3018) via NachoSoto (@NachoSoto) * `SystemInfo`: added `ClockType` (#3014) via NachoSoto (@NachoSoto) * `Integration Tests`: begin tests with `UIApplication.willEnterForegroundNotification` to simulate a real app (#3015) via NachoSoto (@NachoSoto) * `Integration Tests`: add tests to verify `CustomerInfo`+`Offerings` request de-dupping (#3013) via NachoSoto (@NachoSoto) * `SwiftLint`: disable `unneeded_synthesized_initializer` (#3010) via NachoSoto (@NachoSoto) * Added `internal` `NonSubscriptionTransaction.storeTransactionIdentifier` (#3009) via NachoSoto (@NachoSoto) * `Integration Tests`: added tests for non-renewing and non-consumable packages (#3008) via NachoSoto (@NachoSoto) * Expanded `EnsureNonEmptyArrayDecodable` to `EnsureNonEmptyCollectionDecodable` (#3002) via NachoSoto (@NachoSoto)
1 task
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The existing implementation was posting them in sequence, which meant that it was always going to make N requests.
With this new implementation we can ensure that only one request is made for all transactions.