Disable SK1's `StoreKitWrapper` if SK2 is enabled and available by NachoSoto · Pull Request #1882 · RevenueCat/purchases-ios

NachoSoto · 2022-09-01T19:29:57Z

When SK2 is enabled, StoreKit2TransactionListener is enough to handle transactions, so it's no longer necessary to have that and StoreKitWrapper both listening to them.

Depends on #1881 and #1879.

TODO:

Improve handling of attempting to purchase SK1 products when it's implicitly disabled. This shouldn't happen (covered well in tests), but we need to handle that more gracefully.
What about presentCodeRedemptionSheet and showPriceConsentIfNeeded? We could have a dedicated "SK1Wrapper" with support for only those 2 functions, since they don't have an SK2 equivalent.
Add tests to verify this behavior.

NachoSoto · 2022-09-01T19:31:52Z

I believe all tests should pass.

aboedo · 2022-09-01T19:33:26Z

I think having a separate wrapper for the ones that are SK1-only should work. The only reason we have a wrapper for them in the first place is to make mocking easier, and that function can still be served by a separate wrapper

A couple of helpers that can be useful, for #1882 for example.

NachoSoto · 2022-09-04T21:50:04Z

This is ready now.

aboedo · 2022-09-05T21:36:29Z

starting to think we should rename this to storeKit1Wrapper

Yeah that's a good idea, let me do that.

Actually let me send a separate PR for that so we can keep the history of this particular change easier to follow in the future.

aboedo · 2022-09-05T21:37:56Z

:chefskiss:

When SK2 is enabled, `StoreKit2TransactionListener` is enough to handle transactions, so it's not longer necessary to have that and `StoreKitWrapper` both listening to them.

Follow up to #1882. Now that `StoreKit1Wrapper` is not initialized for SK2, it makes sense to clarify that this class is only for SK1. This is just a simple find-replace.

@NachoSoto

**This is an automatic release.** ### Bugfixes * `watchOS`: fixed crash when ran on single-target apps with Xcode 14 and before `watchOS 9.0` (#1895) via NachoSoto (@NachoSoto) * `CustomerInfoManager`/`OfferingsManager`: improved display of underlying errors (#1888) via NachoSoto (@NachoSoto) * `Offering`: improved confusing log for `PackageType.custom` (#1884) via NachoSoto (@NachoSoto) * `PurchasesOrchestrator`: don't log warning if `allowSharingAppStoreAccount` setting was never explicitly set (#1885) via NachoSoto (@NachoSoto) * Introduced type-safe `PurchasesError` and fixed some incorrect returned error types (#1879) via NachoSoto (@NachoSoto) * `CustomerInfoManager`: fixed thread-unsafe implementation (#1878) via NachoSoto (@NachoSoto) ### New Features * Disable SK1's `StoreKitWrapper` if SK2 is enabled and available (#1882) via NachoSoto (@NachoSoto) * `Sendable` support (#1795) via NachoSoto (@NachoSoto) ### Other Changes * Renamed `StoreKitWrapper` to `StoreKit1Wrapper` (#1886) via NachoSoto (@NachoSoto) * Enabled `DEAD_CODE_STRIPPING` (#1887) via NachoSoto (@NachoSoto) * `HTTPClient`: added `X-Client-Bundle-ID` and logged on SDK initialization (#1883) via NachoSoto (@NachoSoto) * add link to SDK reference (#1872) via Andy Boedo (@aboedo) * Added `StoreKit2Setting.shouldOnlyUseStoreKit2` (#1881) via NachoSoto (@NachoSoto) * Introduced `TestLogHandler` to simplify how we test logged messages (#1858) via NachoSoto (@NachoSoto) * `Integration Tests`: added test for purchasing `StoreProduct` instead of `Package` (#1875) via NachoSoto (@NachoSoto) * `ErrorUtils`: added test to verify that returned errors can be converted to `ErrorCode` (#1871) via NachoSoto (@NachoSoto)

…Store when SK1 is disabled Follow up to #1882. After that PR, `StoreKit1Wrapper` was no longer listening to `SKPaymentQueueDelegate.paymentQueue(:shouldAddStorePayment:for:)`, which means that purchases initiated from the App Store were being ignored by the SDK if SK2 is enabled. To fix this, `PaymentQueueWrapper` now becomes the `SKPaymentQueueDelegate` only when `StoreKit1Wrapper` is not initialized, and `PurchasesOrchestrator` initiates an SK2 purchase instead. - `PurchasesOrchestrator` purchase methods no longer need the whole `PromotionalOffer`, only the `SignedData`. This allows the new implementation to create that data from only the `SKPaymentDiscount` provided from the `SKPayment`. - [ ] Test that `StoreKit1Wrapper` is the delegate if SK1 is enabled - [ ] Test that `PaymentQueueWrapper` is the delegate otherwise - [ ] Test `PromotionalOffer.SignedData` initialization from SK1 - [ ] Test conversion of SK1 product to SK2.

…Store when SK1 is disabled (#1901) Follow up to #1882. After that PR, `StoreKit1Wrapper` was no longer listening to `SKPaymentQueueDelegate.paymentQueue(:shouldAddStorePayment:for:)`, which means that purchases initiated from the App Store were being ignored by the SDK if SK2 is enabled. To fix this, `PaymentQueueWrapper` now becomes the `SKPaymentQueueDelegate` only when `StoreKit1Wrapper` is not initialized, and `PurchasesOrchestrator` initiates an SK2 purchase instead. ## Other changes: - `PurchasesOrchestrator` purchase methods no longer need the whole `PromotionalOffer`, only the `SignedData`. This allows the new implementation to create that data from only the `SKPaymentDiscount` provided from the `SKPayment`. - Added tests for `PromotionalOffer.SignedData`

Follow up to. This is also needed when `StoreKit1Wrapper` is not setup (see #1882).

…e` wrapper This is a follow up to #1901 and #1882. Essentially before this change, `Purchases` had both `StoreKit1Wrapper?` and a `PaymentQueueWrapper`. The later wasn't actually needed when the first was not-nil, so it was essentially storing `(a?, b?)`. What we really wanted to express is that it could have one OR the other. This introduces `Either` to express this requirement in the type system, as well as `PaymentQueueWrapperType` that will be used to fix #1964.

Follow up to #1962. This is also needed when `StoreKit1Wrapper` is not setup (see #1882).

…e` wrapper This is a follow up to #1901 and #1882. Essentially before this change, `Purchases` had both `StoreKit1Wrapper?` and a `PaymentQueueWrapper`. The later wasn't actually needed when the first was not-nil, so it was essentially storing `(a?, b?)`. What we really wanted to express is that it could have one OR the other. This introduces `Either` to express this requirement in the type system, as well as `PaymentQueueWrapperType` that will be used to fix #1964.

…e` wrapper (#1968) This is a follow up to #1901 and #1882. Depends on #1967. Essentially before this change, `Purchases` had both `StoreKit1Wrapper?` and a `PaymentQueueWrapper`. The later wasn't actually needed when the first was not-nil, so it was essentially storing `(a?, b?)`. What we really wanted to express is that it could have one OR the other. This introduces `Either` to express this requirement in the type system, as well as `PaymentQueueWrapperType` that will be used to fix #1964.

NachoSoto added WIP pr:feat A new feature perf labels Sep 1, 2022

NachoSoto requested review from a team and aboedo September 1, 2022 19:29

NachoSoto mentioned this pull request Sep 1, 2022

Added StoreKit2Setting.shouldOnlyUseStoreKit2 #1881

Merged

NachoSoto changed the title ~~[WIP] Disable StoreKitWrapper if SK2 is enabled and available~~ [WIP] Disable SK1's StoreKitWrapper if SK2 is enabled and available Sep 1, 2022

NachoSoto force-pushed the disable-sk1-observer branch from 0e4f57d to 5a7a024 Compare September 4, 2022 21:30

Base automatically changed from sk-setting-enabled to main September 4, 2022 21:42

NachoSoto added a commit that referenced this pull request Sep 4, 2022

Added StoreKit2Setting.shouldOnlyUseStoreKit2 (#1881)

348d8fa

A couple of helpers that can be useful, for #1882 for example.

NachoSoto changed the title ~~[WIP] Disable SK1's StoreKitWrapper if SK2 is enabled and available~~ Disable SK1's StoreKitWrapper if SK2 is enabled and available Sep 4, 2022

NachoSoto force-pushed the disable-sk1-observer branch from 5a7a024 to cfdaabc Compare September 4, 2022 21:49

NachoSoto marked this pull request as ready for review September 4, 2022 21:49

NachoSoto changed the base branch from main to public-error September 4, 2022 21:49

Base automatically changed from public-error to main September 5, 2022 20:46

NachoSoto force-pushed the disable-sk1-observer branch from db4d86d to 282f63f Compare September 5, 2022 20:47

aboedo approved these changes Sep 5, 2022

View reviewed changes

NachoSoto added 5 commits September 5, 2022 17:25

Disable StoreKitWrapper if SK2 is enabled and available

d3fa515

When SK2 is enabled, `StoreKit2TransactionListener` is enough to handle transactions, so it's not longer necessary to have that and `StoreKitWrapper` both listening to them.

Moved methods into PaymentQueueWrapper

3a0db27

Handle TODOs

dea24e5

Added tests

19d4d9f

Fixed pre-iOS 16 compilation

cd6d097

NachoSoto force-pushed the disable-sk1-observer branch from 282f63f to cd6d097 Compare September 6, 2022 00:25

NachoSoto merged commit 32c28e1 into main Sep 6, 2022

NachoSoto deleted the disable-sk1-observer branch September 6, 2022 00:32

NachoSoto added a commit that referenced this pull request Sep 6, 2022

Renamed StoreKitWrapper to StoreKit1Wrapper

1757b47

Follow up to #1882. Now that `StoreKit1Wrapper` is not initialized for SK2, it makes sense to clarify that this class is only for SK1. This is just a simple find-replace.

NachoSoto mentioned this pull request Sep 6, 2022

Renamed StoreKitWrapper to StoreKit1Wrapper #1886

Merged

revenuecat-ops mentioned this pull request Sep 7, 2022

[AUTOMATIC] Release/4.12.0 #1890

Merged

NachoSoto mentioned this pull request Sep 10, 2022

PaymentQueueWrapper: handle promotional purchase requests from App Store when SK1 is disabled #1901

Merged

5 tasks

NachoSoto mentioned this pull request Sep 30, 2022

PaymentQueueWrapper: also implement shouldShowPriceConsent #1963

Merged

NachoSoto added a commit that referenced this pull request Sep 30, 2022

PaymentQueueWrapper: also implement shouldShowPriceConsent

f58db2d

Follow up to. This is also needed when `StoreKit1Wrapper` is not setup (see #1882).

NachoSoto added a commit that referenced this pull request Oct 7, 2022

PaymentQueueWrapper: also implement shouldShowPriceConsent

558033e

Follow up to. This is also needed when `StoreKit1Wrapper` is not setup (see #1882).

NachoSoto mentioned this pull request Oct 7, 2022

PaymentQueueWrapper: improved abstraction for active SKPaymentQueue wrapper #1968

Merged

NachoSoto added a commit that referenced this pull request Oct 7, 2022

PaymentQueueWrapper: also implement shouldShowPriceConsent (#1963)

6785b6e

Follow up to #1962. This is also needed when `StoreKit1Wrapper` is not setup (see #1882).

NachoSoto mentioned this pull request Dec 5, 2022

HTTPClient: replaced X-StoreKit2-Setting with X-StoreKit2-Enabled #2118

Merged

vegaro added pr:other and removed pr:perf labels Sep 17, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Disable SK1's `StoreKitWrapper` if SK2 is enabled and available#1882

Disable SK1's `StoreKitWrapper` if SK2 is enabled and available#1882
NachoSoto merged 5 commits into
mainfrom
disable-sk1-observer

NachoSoto commented Sep 1, 2022 •

edited

Loading

Uh oh!

NachoSoto commented Sep 1, 2022

Uh oh!

aboedo commented Sep 1, 2022

Uh oh!

NachoSoto commented Sep 4, 2022

Uh oh!

aboedo Sep 5, 2022

Uh oh!

NachoSoto Sep 5, 2022

Uh oh!

NachoSoto Sep 6, 2022

Uh oh!

aboedo Sep 5, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

NachoSoto commented Sep 1, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

TODO:

Uh oh!

NachoSoto commented Sep 1, 2022

Uh oh!

aboedo commented Sep 1, 2022

Uh oh!

NachoSoto commented Sep 4, 2022

Uh oh!

aboedo Sep 5, 2022

Choose a reason for hiding this comment

Uh oh!

NachoSoto Sep 5, 2022

Choose a reason for hiding this comment

Uh oh!

NachoSoto Sep 6, 2022

Choose a reason for hiding this comment

Uh oh!

aboedo Sep 5, 2022

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

NachoSoto commented Sep 1, 2022 •

edited

Loading