Always initialize `StoreKit2TransactionListener` even on SK1 mode #2612

NachoSoto · 2023-06-09T19:31:53Z

This is necessary for the new paywall screens to work.
Tests for this added in #2590.

Looks like `Storefront.current` is usually `nil` during tests. After #2612, it's important than renewals posted from SK1 and SK2 end up with equivalent cache keys. This will be ensured by a test in #2590, which breaks unless we do this. Example cache keys between SK1 and SK2: ``` PostReceiptDataOperation $RCAnonymousID:9d38b05059d442b082f915011041bbcb-true-77c15e6fad291c0ff9bf8a4efdfa21eca7cdd74e7a0a4a1bdcf320e8ddcfc2d5\n-com.revenuecat.monthly_4.99.no_intro-0.99-USD-USA--1-0-7096FF06-P1M---1-com.revenuecat.monthly_4.99.1_free_week\n--true\n-[\"$attConsentStatus\": [SubscriberAttribute] key: $attConsentStatus value: notDetermined setTime: 2023-06-12 22:31:56 +0000] PostReceiptDataOperation $RCAnonymousID:9d38b05059d442b082f915011041bbcb-true-77c15e6fad291c0ff9bf8a4efdfa21eca7cdd74e7a0a4a1bdcf320e8ddcfc2d5\n-com.revenuecat.monthly_4.99.no_intro-0.99-USD---1-0-7096FF06-P1M---1-com.revenuecat.monthly_4.99.1_free_week\n--true\n-["$attConsentStatus": [SubscriberAttribute] key: $attConsentStatus value: notDetermined setTime: 2023-06-12 22:31:56 +0000] ``` Notice `USA` is missing on the SK2 posted product.

tonidero

Code looks good, but this seems like a change we should be careful with so will hold on approving until @aboedo has a chance to look at it.

tonidero · 2023-06-13T08:01:33Z

Sources/Purchasing/Purchases/PurchasesOrchestrator.swift

@@ -130,8 +130,9 @@ final class PurchasesOrchestrator {
        storeKit2TransactionListener.delegate = self
        storeKit2StorefrontListener.delegate = self

+        storeKit2TransactionListener.listenForTransactions()


Is there any danger of this sending the same transaction multiple times and/or with different data to our backend?

The only danger is increased load. @aboedo and I talked about this yesterday.
#2590 adds a test that verifies this particular scenario and checks that we only post once.

Yeah, definitely a compromise. Our system is pretty good at de-duping post receipts that are in flight, but there's always a chance that we get slightly delayed signals from SK1 and SK2 and the response from one gets back before we post the other one and we do a double post.

But on the other hand, that seems like a better scenario than if we miss a purchase in observer mode because we were observing the wrong object.

codecov · 2023-06-13T14:59:50Z

Codecov Report

Merging #2612 (77d1f77) into main (89ef553) will decrease coverage by 0.02%.
The diff coverage is 92.85%.

@@            Coverage Diff             @@
##             main    #2612      +/-   ##
==========================================
- Coverage   85.97%   85.95%   -0.02%     
==========================================
  Files         205      205              
  Lines       14442    14448       +6     
==========================================
+ Hits        12416    12419       +3     
- Misses       2026     2029       +3

Impacted Files	Coverage Δ
...ces/Purchasing/StoreKit1/PaymentQueueWrapper.swift	`41.66% <0.00%> (ø)`
Sources/Logging/Strings/PurchaseStrings.swift	`88.42% <100.00%> (ø)`
Sources/Logging/Strings/StoreKitStrings.swift	`89.28% <100.00%> (+0.39%)`	⬆️
...s/Purchasing/Purchases/PurchasesOrchestrator.swift	`85.59% <100.00%> (+0.01%)`	⬆️
...asing/StoreKit2/StoreKit2TransactionListener.swift	`92.53% <100.00%> (+0.22%)`	⬆️

... and 1 file with indirect coverage changes

aboedo · 2023-06-14T19:12:54Z

Sources/Purchasing/Purchases/PurchasesOrchestrator.swift

@@ -130,8 +130,9 @@ final class PurchasesOrchestrator {
        storeKit2TransactionListener.delegate = self
        storeKit2StorefrontListener.delegate = self

+        storeKit2TransactionListener.listenForTransactions()


Yeah, definitely a compromise. Our system is pretty good at de-duping post receipts that are in flight, but there's always a chance that we get slightly delayed signals from SK1 and SK2 and the response from one gets back before we post the other one and we do a double post.

But on the other hand, that seems like a better scenario than if we miss a purchase in observer mode because we were observing the wrong object.

aboedo · 2023-06-14T19:13:21Z

Sources/Purchasing/StoreKit1/PaymentQueueWrapper.swift

-        // Ignored. Either `StoreKit1Wrapper` will handle this, or `StoreKit2TransactionListener` if `SK2` is enabled.
+        // Ignored. Either `StoreKit1Wrapper` or `StoreKit2TransactionListener` will handle this.


feels nuts that codecov looks for comments too?

Sources/Purchasing/StoreKit2/StoreKit2TransactionListener.swift

Co-authored-by: Andy Boedo <andresboedo@gmail.com>

…ls (#2590) Follow up to #2589. Depends on #2612.

…cessing transactions This is the last piece needed after #2612. Transactions handled from `StoreKit.Transaction.updates` were ignoring the result `CustomerInfo`. On example where this was noticed is when redeeming promo codes: we posted the transaction, but `PurchasesDelegate` was not notified of the change.

…cessing transactions (#2676) This is the last piece needed after #2612. Transactions handled from `StoreKit.Transaction.updates` were ignoring the result `CustomerInfo`. On example where this was noticed is when redeeming promo codes: we posted the transaction, but `PurchasesDelegate` was not notified of the change.

@NachoSoto

**This is an automatic release.** ### Bugfixes * `PurchasesOrchestrator`: update `CustomerInfoManager` cache after processing transactions (#2676) via NachoSoto (@NachoSoto) * `ErrorResponse`: drastically improved error messages, no more "unknown error"s (#2660) via NachoSoto (@NachoSoto) * `PaywallExtensions`: post purchases with `Offering` identifier (#2645) via NachoSoto (@NachoSoto) * Support `product_plan_identifier` for purchased subscriptions from `Google Play` (#2654) via Josh Holtz (@joshdholtz) ### Performance Improvements * `copy(with: VerificationResult)`: optimization to avoid copies (#2639) via NachoSoto (@NachoSoto) ### Other Changes * `ETagManager`: refactored e-tag creation and tests (#2671) via NachoSoto (@NachoSoto) * `getPromotionalOffer`: return `ErrorCode.ineligibleError` if receipt is not found (#2678) via NachoSoto (@NachoSoto) * `TimingUtil`: removed slow purchase logs (#2677) via NachoSoto (@NachoSoto) * `CI`: changed `Codecov` to `informational` (#2670) via NachoSoto (@NachoSoto) * `LoadShedderIntegrationTests`: verify requests are actually handled by load shedder (#2663) via NachoSoto (@NachoSoto) * `ETagManager.httpResultFromCacheOrBackend`: return response headers (#2666) via NachoSoto (@NachoSoto) * `Integration Tests`: added tests to verify 304 behavior (#2659) via NachoSoto (@NachoSoto) * `HTTPClient`: disable `URLSession` cache (#2668) via NachoSoto (@NachoSoto) * Documented `HTTPStatusCode.isSuccessfullySynced` (#2661) via NachoSoto (@NachoSoto) * `NetworkError.signatureVerificationFailed`: added status code to error `userInfo` (#2657) via NachoSoto (@NachoSoto) * `HTTPClient`: improved log for failed requests (#2669) via NachoSoto (@NachoSoto) * `ETagManager`: added new verbose logs (#2656) via NachoSoto (@NachoSoto) * `Signature Verification`: added test-only log for debugging invalid signatures (#2658) via NachoSoto (@NachoSoto) * Fixed `HTTPResponse.description` (#2664) via NachoSoto (@NachoSoto) * Changed `Logger` to use `os_log` (#2608) via NachoSoto (@NachoSoto) * `MainThreadMonitor`: increased threshold (#2662) via NachoSoto (@NachoSoto) * `debugRevenueCatOverlay`: display `receiptURL` (#2652) via NachoSoto (@NachoSoto) * `PurchaseTester`: added ability to display `debugRevenueCatOverlay` (#2653) via NachoSoto (@NachoSoto) * `debugRevenueCatOverlay`: ability to close on `macOS`/`Catalyst` (#2649) via NachoSoto (@NachoSoto) * `debugRevenueCatOverlay`: added support for `macOS` (#2648) via NachoSoto (@NachoSoto) * `LoadShedderIntegrationTests`: enable signature verification (#2655) via NachoSoto (@NachoSoto) * `ImageSnapshot`: fixed Xcode 15 compilation (#2651) via NachoSoto (@NachoSoto) * `OfferingsManager`: don't clear offerings cache timestamp when request fails (#2359) via NachoSoto (@NachoSoto) * `StoreKitObserverModeIntegrationTests`: added test for posting renewals (#2590) via NachoSoto (@NachoSoto) * Always initialize `StoreKit2TransactionListener` even on SK1 mode (#2612) via NachoSoto (@NachoSoto) * `ErrorUtils.missingReceiptFileError`: added receipt URL `userInfo` context (#2650) via NachoSoto (@NachoSoto) * Added `.xcprivacy` for Xcode 15 (#2619) via NachoSoto (@NachoSoto) * `Trusted Entitlements`: added debug log with `ResponseVerificationMode` (#2647) via NachoSoto (@NachoSoto) * `debugRevenueCatOverlay`: simplified title (#2641) via NachoSoto (@NachoSoto) * Simplified `Purchases.updateAllCachesIfNeeded` (#2626) via NachoSoto (@NachoSoto) * `HTTPResponseTests`: fixed disabled test (#2643) via NachoSoto (@NachoSoto) * Add `InternalDangerousSettings.forceSignatureFailures` (#2635) via NachoSoto (@NachoSoto) * `IntegrationTests`: explicit `StoreKit 1` mode (#2636) via NachoSoto (@NachoSoto) * `Signing`: removed API for loading key from a file (#2638) via NachoSoto (@NachoSoto)

This is a performance optimization that will help avoid duplicate posts after #2612.

…is enabled This essentially reverts #2612. This has proven to cause more issues than anything, including duplicate POST receipt requests. The main reason for #2612 was to support `SwiftUI` paywalls. This instead adds a warning when attempting to use those without SK2.

NachoSoto added feat:StoreKit 2 refactor labels Jun 9, 2023

NachoSoto requested a review from a team June 9, 2023 19:31

NachoSoto marked this pull request as draft June 9, 2023 20:08

NachoSoto force-pushed the nacho/sdk-3170-always-initialize-storekit2transactionlistener-even-on-sk1 branch 4 times, most recently from 9c64309 to 2ef65c1 Compare June 12, 2023 21:47

NachoSoto changed the title ~~[WIP] Always initialize StoreKit2TransactionListener even on SK1 mode~~ Always initialize StoreKit2TransactionListener even on SK1 mode Jun 12, 2023

NachoSoto marked this pull request as ready for review June 12, 2023 21:47

NachoSoto mentioned this pull request Jun 12, 2023

StoreKitObserverModeIntegrationTests: added test for posting renewals #2590

Merged

NachoSoto mentioned this pull request Jun 12, 2023

PurchasesOrchestrator: also get Storefront from SK1 #2629

Merged

tonidero reviewed Jun 13, 2023

View reviewed changes

NachoSoto requested review from aboedo and a team June 13, 2023 14:43

Always listen to updates

77d1f77

NachoSoto force-pushed the nacho/sdk-3170-always-initialize-storekit2transactionlistener-even-on-sk1 branch from 2ef65c1 to 77d1f77 Compare June 13, 2023 14:45

aboedo approved these changes Jun 14, 2023

View reviewed changes

Update Sources/Purchasing/StoreKit2/StoreKit2TransactionListener.swift

fe0a1cc

Co-authored-by: Andy Boedo <andresboedo@gmail.com>

NachoSoto enabled auto-merge (squash) June 14, 2023 19:27

NachoSoto merged commit 6c3b4d4 into main Jun 14, 2023

NachoSoto deleted the nacho/sdk-3170-always-initialize-storekit2transactionlistener-even-on-sk1 branch June 14, 2023 19:47

NachoSoto added a commit that referenced this pull request Jun 14, 2023

StoreKitObserverModeIntegrationTests: added test for posting renewa…

7cfbfe0

…ls (#2590) Follow up to #2589. Depends on #2612.

NachoSoto mentioned this pull request Jun 20, 2023

PurchasesOrchestrator: update CustomerInfoManager cache after processing transactions #2676

Merged

RCGitBot mentioned this pull request Jun 21, 2023

[AUTOMATIC] Release/4.22.1 #2684

Merged

NachoSoto mentioned this pull request Aug 3, 2023

TransactionPoster: avoid posting transactions multiple times #2914

Closed

4 tasks

NachoSoto added a commit that referenced this pull request Aug 16, 2023

TransactionPoster: avoid posting transactions multiple times

57d4017

This is a performance optimization that will help avoid duplicate posts after #2612.

NachoSoto mentioned this pull request Aug 16, 2023

StoreKit 2: only listen to StoreKit.Transaction.updates when SK2 is enabled #3032

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Always initialize `StoreKit2TransactionListener` even on SK1 mode #2612

Always initialize `StoreKit2TransactionListener` even on SK1 mode #2612

NachoSoto commented Jun 9, 2023 •

edited

Loading

tonidero left a comment

tonidero Jun 13, 2023

NachoSoto Jun 13, 2023

aboedo Jun 14, 2023

codecov bot commented Jun 13, 2023 •

edited

Loading

aboedo Jun 14, 2023

aboedo Jun 14, 2023

		// Ignored. Either `StoreKit1Wrapper` will handle this, or `StoreKit2TransactionListener` if `SK2` is enabled.
		// Ignored. Either `StoreKit1Wrapper` or `StoreKit2TransactionListener` will handle this.

Always initialize StoreKit2TransactionListener even on SK1 mode #2612

Always initialize StoreKit2TransactionListener even on SK1 mode #2612

Conversation

NachoSoto commented Jun 9, 2023 • edited Loading

tonidero left a comment

Choose a reason for hiding this comment

tonidero Jun 13, 2023

Choose a reason for hiding this comment

NachoSoto Jun 13, 2023

Choose a reason for hiding this comment

aboedo Jun 14, 2023

Choose a reason for hiding this comment

codecov bot commented Jun 13, 2023 • edited Loading

Codecov Report

aboedo Jun 14, 2023

Choose a reason for hiding this comment

aboedo Jun 14, 2023

Choose a reason for hiding this comment

Always initialize `StoreKit2TransactionListener` even on SK1 mode #2612

Always initialize `StoreKit2TransactionListener` even on SK1 mode #2612

NachoSoto commented Jun 9, 2023 •

edited

Loading

codecov bot commented Jun 13, 2023 •

edited

Loading