Skip to content
This repository was archived by the owner on Feb 22, 2023. It is now read-only.

[in_app_purchase] Implements transaction caching for StoreKit #4985

Merged
merged 7 commits into from
Mar 17, 2022
Merged

[in_app_purchase] Implements transaction caching for StoreKit #4985

merged 7 commits into from
Mar 17, 2022

Conversation

mvanbeusekom
Copy link
Contributor

In the current implementation of the in_app_purchase_storekit transactions can get lost as Apple tries to deliver them as soon as the iOS application starts and clients can only start listening when they have registered an observer on the client side and ran the method startObservingTransactions().

In this implementation the plugin registers an observer as soon as the iOS applications starts and caches transactions in memory until the client confirmed it is listening by making the call to InAppPurchaseStoreKit.startObservingTransactions(). After the call cached transactions will be delivered through the normal callbacks and future transactions will also be delivered immediately until the client indicates it doesn't won't to observe anymore by calling the InAppPurchaseStoreKit.stopObservingTransactions() method. At this point inbound transactions will be cached again until the client starts observing them again.

If you had to change anything in the flutter/tests repo, include a link to the migration guide as per the breaking change policy.

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the relevant style guides and ran the auto-formatter. (Unlike the flutter/flutter repo, the flutter/plugins repo does use dart format.)
  • I signed the CLA.
  • The title of the PR starts with the name of the plugin surrounded by square brackets, e.g. [shared_preferences]
  • I listed at least one issue that this PR fixes in the description above.
  • I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.
  • I updated CHANGELOG.md to add a description of the change, following repository CHANGELOG style.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is test-exempt.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@mvanbeusekom mvanbeusekom marked this pull request as draft March 3, 2022 16:18
@mvanbeusekom mvanbeusekom marked this pull request as ready for review March 7, 2022 22:13
@mvanbeusekom mvanbeusekom force-pushed the issue/94011 branch 2 times, most recently from 49297ef to f712114 Compare March 8, 2022 08:48
stuartmorgan-g
stuartmorgan-g suggested changes Mar 15, 2022
View reviewed changes
stuartmorgan-g
stuartmorgan-g suggested changes Mar 16, 2022
View reviewed changes
.../in_app_purchase/in_app_purchase_storekit/example/ios/RunnerTests/FIATransactionCacheTests.m
NSArray *dummyArray = @[ @1, @2, @3 ];
FIATransactionCache *cache = [[FIATransactionCache alloc] init];
[cache addObjects:dummyArray forKey:TransactionCacheKeyUpdatedTransactions];

XCTAssertEqual(dummyArray, [cache getObjectsForKey:TransactionCacheKeyUpdatedTransactions]);

[cache removeObjectsForKey:TransactionCacheKeyUpdatedTransactions];
[cache clear];
XCTAssertNil([cache getObjectsForKey:TransactionCacheKeyUpdatedTransactions]);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to be safe, it would be good to add something for each key before, and assert that they are all nil after.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right, should have done so right away.

packages/in_app_purchase/in_app_purchase_storekit/ios/Classes/FIAPaymentQueueHandler.m Outdated
// incoming transactions from the App Store are cached and delivered to the
// client as soon as it indicates it is ready to receive transactions by
// sending the "startObservingPaymentQueue" message.
self.observingTransactions = NO;
}

- (void)processCachedTransactions {
NSArray *cachedObjects =
[self.transactionCache getObjectsForKey:TransactionCacheKeyUpdatedTransactions];
if (cachedObjects) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor nit: these checks should probably be foo.count != 0 rather than a nil check, so that if for whatever reason the cache returned an empty array in the future nothing would get called.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes a lot of sense. Updated and included an addition test to guard for both (nil and empty array) situations.

packages/in_app_purchase/in_app_purchase_storekit/ios/Classes/FIAPaymentQueueHandler.m Outdated
// lifetime. Only setting the "observingTransactions" to "NO" ensures
// incoming transactions from the App Store are cached and delivered to the
// client as soon as it indicates it is ready to receive transactions by
// sending the "startObservingPaymentQueue" message.
Copy link
Contributor

@stuartmorgan-g stuartmorgan-g Mar 16, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't quite address what I was asking. My concern here is that if the Dart side stops observing, and doesn't start again, then there will be transactions in the cache that the App Store will believe have been successfully received by the application, but that will never actually get delivered anywhere. They will be thrown away, unhandled, on app exit.

Is that safe? If so, why? (E.g., will they get re-delivered the next time the app is run if they aren't acknowledged in some way?) If we don't have a guarantee from the API that this is a safe thing to do, it seems like we would need to have a persistent cache instead (e.g., using NSUserDefaults).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rephrased the comment to explain what happens when the app is killed and why it is not a problem that cached information is lost.

stuartmorgan-g
stuartmorgan-g approved these changes Mar 16, 2022
View reviewed changes
Copy link
Contributor

@stuartmorgan-g stuartmorgan-g left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for the iteration!

@mvanbeusekom mvanbeusekom added the waiting for tree to go green (Use "autosubmit") This PR is approved and tested, but waiting for the tree to be green to land. label Mar 16, 2022
@fluttergithubbot fluttergithubbot merged commit 04f64ec into flutter:main Mar 17, 2022
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Mar 17, 2022
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 17, 2022
@engine-flutter-autoroll
04f64ec89 [in_app_purchase] Implements transaction caching for StoreK…
f5d5fe1 
…it (flutter/plugins#4985)
@mvanbeusekom mvanbeusekom deleted the issue/94011 branch May 25, 2022 16:57
mauricioluz pushed a commit to mauricioluz/plugins that referenced this pull request Jan 26, 2023
@mvanbeusekom @mauricioluz
[in_app_purchase] Implements transaction caching for StoreKit (flutte…
cd6ffb7 
…r#4985)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@stuartmorgan-g stuartmorgan-g stuartmorgan-g approved these changes

Assignees
No one assigned
Labels
p: in_app_purchase platform-ios waiting for tree to go green (Use "autosubmit") This PR is approved and tested, but waiting for the tree to be green to land.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[in_app_purchase] Can not complete an unfinished pending transaction on iOS on app start
3 participants
@mvanbeusekom @stuartmorgan-g @fluttergithubbot