Prepare the codebase to listen to the Storefront changes (1/4) by Juanpe · Pull Request #1557 · RevenueCat/purchases-ios

Juanpe · 2022-05-09T09:48:40Z

Subtask of #1382

Description

This PR is the first step to preparing the codebase to respond to Storefront changes to update some stuff that is in the cache.

Changes:

Listen to Storefront changes for SK1 via SKPaymentTransactionObserver protocol using the paymentQueueDidChangeStorefront method.
Listen to Storefront changes for SK2, creating a new listener (StoreKit2TransactionListener ).
Update PurchasesOrchestrator, adding the new delegate methods.
Fix failing tests related to these changes and add new ones.

…anges for SK2

Juanpe · 2022-05-09T09:51:28Z


    @available(iOS 15.0, macOS 12.0, tvOS 15.0, watchOS 8.0, *)
-    lazy var storeKit2Listener = StoreKit2TransactionListener(delegate: self)
+    lazy var storeKit2TransactionListener = StoreKit2TransactionListener(delegate: self)


In the future, we could create a single listener that would be composed of other "sublisteners"

Can this be private(set)?

~~yep 🙂 let me change it~~ Finally, it's not possible, we are setting this listener in the tests, so we need the set method as public 😕 Admittedly it is only needed for testing, but I would leave it there for now.
WDYT? 🤔

That's fine then!

Juanpe · 2022-05-09T09:54:23Z

+import XCTest
+
+@available(iOS 15.0, tvOS 15.0, macOS 12.0, watchOS 8.0, *)
+class StoreKit2StorefrontListenerTests: XCTestCase {


I tried to add a test case to cover when the Storefront changes, but I couldn’t. Is there is a way? 🤔

Yeah I don't think we can change the storefront mid-way through a test run unfortunately.

NachoSoto · 2022-05-09T20:09:54Z

+@available(iOS 15.0, tvOS 15.0, macOS 12.0, watchOS 8.0, *)
+class StoreKit2StorefrontListener {
+
+    private(set) var taskHandle: Task<Void, Never>?


I would implement didSet here like so:

didSet { if self.taskHandle !== oldValue { oldValue?.cancel() } }

Then you only need to set it to nil or the new value.

much better 👍🏼 Thanks :)

Just a question, why do you have to check if they are identical, checking if they are not equal wouldn't be enough? 🤔

Oh it's a struct. Yeah == is fine:

https://github.com/apple/swift/blob/3a9c8012841c64c7da9a5fc56770ca820a6499b8/stdlib/public/Concurrency/Task.swift#L183

Done! ✅ cee546e

…_changes

NachoSoto

Awesome!

NachoSoto · 2022-05-10T17:48:31Z


    @available(iOS 15.0, macOS 12.0, tvOS 15.0, watchOS 8.0, *)
-    lazy var storeKit2Listener = StoreKit2TransactionListener(delegate: self)
+    lazy var storeKit2TransactionListener = StoreKit2TransactionListener(delegate: self)


Can this be private(set)?

NachoSoto · 2022-05-10T17:48:40Z

+    lazy var storeKit2TransactionListener = StoreKit2TransactionListener(delegate: self)
+
+    @available(iOS 15.0, macOS 12.0, tvOS 15.0, watchOS 8.0, *)
+    lazy var storeKit2StorefrontListener = StoreKit2StorefrontListener(delegate: self)


Ditto private(set)

same here 🙂

NachoSoto · 2022-05-10T17:56:49Z

+        taskHandle?.cancel()
+        taskHandle = nil


This is duplicated, you can just do self.taskHandle = nil now :)
The advantage of what I suggested is that now it's impossible to have a reference to a cancelled task :)

Yeah! good catch! I forgot to update this line 🤦🏼‍♂️

Done! ✅ (171db44)

I've reverted the change because when the listener is released, the didSet observer is not called 😅 so the task is not cancelled. I've cancelled the task when the listener is released and modified the test cases to cover this scenario.

Oooh that's a great catch I forgot!!

NachoSoto · 2022-05-10T17:57:50Z

+        taskHandle = Task { [weak self] in
+            for await _ in StoreKit.Storefront.updates {
+                guard let self = self else { break }
+                self.delegate?.storefrontDidUpdate()


Maybe put this inside of MainActor.run {}?

Good point 👍🏼

Done! ✅ (ad5e799)

NachoSoto · 2022-05-10T17:58:06Z

+import XCTest
+
+@available(iOS 15.0, tvOS 15.0, macOS 12.0, watchOS 8.0, *)
+class StoreKit2StorefrontListenerTests: XCTestCase {


Yeah I don't think we can change the storefront mid-way through a test run unfortunately.

NachoSoto · 2022-05-10T17:59:12Z

+        expect(handle).toNot(beNil())
+        expect(handle?.isCancelled) == false
+
+        self.listener = nil
+        expect(handle?.isCancelled) == true


This is nice.

Co-authored-by: NachoSoto <NachoSoto@users.noreply.github.com>

Juanpe · 2022-05-10T20:35:32Z

@NachoSoto Thanks for the review and feedback! ❤️

NachoSoto · 2022-05-11T23:17:06Z

I'll wait to see if @aboedo has any feedback before merging this.

Juanpe · 2022-05-16T16:42:26Z

Hi guys! 👋🏼 Did you have a chance to take a look at this PR? 🙂 The second PR is ready to be opened but I can't because I'm working on a fork and I can't select this branch as the base of the other PR. So, I have to wait for this PR to be merged to open the other one, otherwise, you would see the same changes 😅

taquitos

🎈🐐

Follow up to #1557. Subtask of #1382 ### Description This PR aims to clear the stuff in the cache when the Storefront changes. Changes: - Add `clearCachedOfferings` method to `DeviceCache`. - Add `clearCache` method to `ProductFetcherSK1`. - Make `internal` the `clearCache` method in `ProductsFetcherSK2`. - Handle the **Storefront** changes in the `PurchasesOrchestator`. - Add tests to cover some scenarios.

Juanpe added 6 commits May 9, 2022 09:29

Listen changes via paymentQueueDidChangeStorefront method

d306755

Create StoreKit2StorefrontListener.swift to listen to Storefront ch…

a59aa39

…anges for SK2

Update PurchasesOrchestrator to listen to the Storefront changes

dbc5f8e

Fix failing tests

25aacfd

Add some unit tests related to the Storefront changes

999faa4

Include StoreKit2StorefrontListenerTests in StoreKitTests

567ffd2

Juanpe commented May 9, 2022

View reviewed changes

Juanpe mentioned this pull request May 9, 2022

Respond to Storefront updates #1382

Closed

NachoSoto reviewed May 9, 2022

View reviewed changes

Juanpe added 2 commits May 10, 2022 18:15

Merge remote-tracking branch 'origin/main' into respond_to_Storefront…

8d58de4

…_changes

Use didSet observer to cancel any previous tasks

cee546e

NachoSoto approved these changes May 10, 2022

View reviewed changes

NachoSoto reviewed May 10, 2022

View reviewed changes

Comment thread Sources/Purchasing/StoreKit2/StoreKit2StorefrontListener.swift

Juanpe and others added 6 commits May 10, 2022 21:51

Add required explicit self

683a7ef

Co-authored-by: NachoSoto <NachoSoto@users.noreply.github.com>

Remove force unwrap

ca285d5

Co-authored-by: NachoSoto <NachoSoto@users.noreply.github.com>

Add required spaces

c57ae0f

Co-authored-by: NachoSoto <NachoSoto@users.noreply.github.com>

Remove useless cancel method when the listener is released

171db44

Notify Storefront changes in the main thread

ddf8312

Add cancel task when the Listener is released

ad5e799

Merge branch 'main' into respond_to_Storefront_changes

6ed5fb9

taquitos approved these changes May 16, 2022

View reviewed changes

taquitos merged commit 3a4093d into RevenueCat:main May 16, 2022

Juanpe mentioned this pull request May 17, 2022

Clear cached offerings and products after Storefront changes (2/4) #1583

Merged

joshdholtz mentioned this pull request May 26, 2022

Release/4.4.0 #1617

Merged

Uh oh!

Conversation

Juanpe commented May 9, 2022

Description

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Juanpe May 10, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

NachoSoto left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Juanpe commented May 10, 2022

Uh oh!

NachoSoto commented May 11, 2022

Uh oh!

Juanpe commented May 16, 2022

Uh oh!

taquitos left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

Juanpe May 10, 2022 •

edited

Loading