-
Notifications
You must be signed in to change notification settings - Fork 810
Feature/payment queue controller #131
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…RestorePurchasesController + tests
…eference and documentation about the order in which transactions are processed.
…t identifier. Added tests to check correct behaviour.
…transaction in an array, and call the callback in restoreCompletedTransactionsFailed or restoreCompletedTransactionsFinished
…/ variables. Added internal initialiser and methods to enable testability
|
|
||
| private func allProductsMatching(_ productIds: Set<String>) -> Set<SKProduct>? { | ||
|
|
||
| var requestedProducts = Set<SKProduct>() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the function below can be replaced by:
return Set(productIds.flatMap { self.products[$0] })
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not precisely. The previous method returns nil if at least one product ID has not been fetched yet. Doing this with flatMap discards all nil values but still returns a non empty array for values that have been already fetched.
This could result in the a purchase request getting stuck because the required SKProduct is not queried.
I have found a way to make this work with flatMap, as follows:
private func allProductsMatching(_ productIds: Set<String>) -> Set<SKProduct>? {
return Set(productIds.flatMap { self.products[$0] })
}
func retrieveProductsInfo(_ productIds: Set<String>, completion: @escaping (RetrieveResults) -> ()) {
guard let products = allProductsMatching(productIds), products.count == productIds.count else {
requestProducts(productIds, completion: completion)
return
}
completion(RetrieveResults(retrievedProducts: products, invalidProductIDs: [], error: nil))
}
This way, if allProductsMatching returns a set of size less than the set of product IDs, the products are requested again.
This is a major refactor of the purchase flows in SwiftyStoreKit. Public facing API is unchanged.
Before: Payments / restore purchases / complete transactions operations were handled by specific requests that would register as observers of
SKPaymentQueue, and deallocated as soon as the response was returned. This caused various problems in correctly dispatching the updated transactions to the correct requests.After: Payments / restore purchases / complete transactions are all composed together inside a
PaymentQueueControllerclass which is the only observer ofSKPaymentQueue.The new functionality is extensively unit tested against the following set of criteria:
The order in which transaction updates are processed is:
.purchasedand.failedfor matching product identifiers).restored, orrestoreCompletedTransactionsFailedWithError, orpaymentQueueRestoreCompletedTransactionsFinished).purchased,.failed,.restored,.deferred)Any transactions where state ==
.purchasingare ignored.Checklist
PaymentQueueControllerPaymentQueueControllerunit testsPaymentQueueControllerintegration testsPaymentControllerPaymentControllerunit testsRestorePurchasesControllerRestorePurchasesControllerunit testsCompeteTransactionsControllerCompeteTransactionsControllerunit testsPaymentQueueControllerintoSwiftyStoreKit.swiftProductsInfoControllerProductsInfoControllerunit testsSwiftyStoreKitTeststo Travis CIinternalrather thanpublicwhere appropriate: this enables unit tests but restricts usage to existing API)SwiftyStoreKitSwiftyStoreKitunit tests (this could be achieved by creating aninternal init()method with default parameters forSKPaymentQueueand other required dependencies)Planned future work (will be done in separate PR)