-
Notifications
You must be signed in to change notification settings - Fork 3.6k
[in_app_purchase_storekit] Fix consumable repurchase issue in StoreKit2 #10521
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
base: main
Are you sure you want to change the base?
Changes from 3 commits
814e600
97dcd3b
cc0994d
41a7230
d031e46
9f90cc6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -263,10 +263,24 @@ extension InAppPurchasePlugin: InAppPurchase2API { | |
| /// Wrapper method around StoreKit2's finish() method https://developer.apple.com/documentation/storekit/transaction/3749694-finish | ||
| func finish(id: Int64, completion: @escaping (Result<Void, Error>) -> Void) { | ||
| Task { | ||
| let transaction = try await fetchTransaction(by: UInt64(id)) | ||
| if let transaction = transaction { | ||
| await transaction.finish() | ||
| completion(.success(Void())) | ||
| do { | ||
| let transaction = try await fetchTransaction(by: UInt64(id)) | ||
| if let transaction = transaction { | ||
| await transaction.finish() | ||
| completion(.success(Void())) | ||
| } else { | ||
| // Transaction not found - this can happen for consumables that have | ||
| // already been finished or are no longer in the transaction history. | ||
| // We still return success as the transaction is effectively complete. | ||
| completion(.success(Void())) | ||
| } | ||
| } catch { | ||
| completion( | ||
| .failure( | ||
| PigeonError( | ||
| code: "storekit2_finish_transaction_failed", | ||
| message: "Failed to finish transaction: \(error.localizedDescription)", | ||
| details: "Transaction ID: \(id)"))) | ||
| } | ||
| } | ||
| } | ||
|
|
@@ -362,8 +376,11 @@ extension InAppPurchasePlugin: InAppPurchase2API { | |
| return transactions | ||
| } | ||
|
|
||
| /// Helper function to fetch specific transaction | ||
| /// Helper function to fetch specific transaction by ID. | ||
| /// First checks Transaction.all, then falls back to unfinished transactions | ||
| /// to ensure consumable transactions can be found and finished. | ||
| func fetchTransaction(by id: UInt64) async throws -> Transaction? { | ||
| // First, try to find in Transaction.all | ||
| for await result in Transaction.all { | ||
| switch result { | ||
| case .verified(let transaction): | ||
|
|
@@ -374,6 +391,21 @@ extension InAppPurchasePlugin: InAppPurchase2API { | |
| continue | ||
| } | ||
| } | ||
|
|
||
| // If not found in Transaction.all, check unfinished transactions | ||
|
||
| // This is important for consumables that may have been purchased | ||
| // but not yet iterated through Transaction.all | ||
| for await result in Transaction.unfinished { | ||
| switch result { | ||
| case .verified(let transaction): | ||
| if transaction.id == id { | ||
| return transaction | ||
| } | ||
| case .unverified: | ||
| continue | ||
| } | ||
| } | ||
|
|
||
| return nil | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -415,6 +415,68 @@ final class InAppPurchase2PluginTests: XCTestCase { | |
| await fulfillment(of: [finishExpectation], timeout: 5) | ||
| } | ||
|
|
||
| func testFinishNonExistentTransactionSucceeds() async throws { | ||
| // Test that finishing a non-existent transaction returns success | ||
| // This is important for consumables that may have already been finished | ||
| // or are no longer in the transaction history | ||
| let finishExpectation = self.expectation( | ||
| description: "Finishing non-existent transaction should succeed") | ||
|
|
||
| plugin.finish(id: 999999) { result in | ||
| switch result { | ||
| case .success(): | ||
| finishExpectation.fulfill() | ||
| case .failure(let error): | ||
| XCTFail("Finish should NOT fail for non-existent transaction. Failed with \(error)") | ||
| } | ||
| } | ||
|
|
||
| await fulfillment(of: [finishExpectation], timeout: 5) | ||
| } | ||
|
|
||
| func testConsumableCanBeRepurchasedAfterFinish() async throws { | ||
| // Test that a consumable can be purchased again after finishing | ||
| let firstPurchaseExpectation = self.expectation(description: "First purchase should succeed") | ||
| let finishExpectation = self.expectation(description: "Finishing purchase should succeed") | ||
| let secondPurchaseExpectation = self.expectation(description: "Second purchase should succeed") | ||
|
|
||
| // First purchase | ||
| plugin.purchase(id: "consumable", options: nil) { result in | ||
| switch result { | ||
| case .success: | ||
| firstPurchaseExpectation.fulfill() | ||
| case .failure(let error): | ||
| XCTFail("First purchase should NOT fail. Failed with \(error)") | ||
| } | ||
| } | ||
|
|
||
| await fulfillment(of: [firstPurchaseExpectation], timeout: 5) | ||
|
|
||
| // Finish the transaction | ||
| plugin.finish(id: 0) { result in | ||
| switch result { | ||
| case .success(): | ||
| finishExpectation.fulfill() | ||
| case .failure(let error): | ||
| XCTFail("Finish should NOT fail. Failed with \(error)") | ||
| } | ||
| } | ||
|
|
||
| await fulfillment(of: [finishExpectation], timeout: 5) | ||
|
Comment on lines
+454
to
+464
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Using a hardcoded transaction ID let transactionsExpectation = self.expectation(description: "Get transactions")
var transactionId: Int64?
plugin.transactions { result in
switch result {
case .success(let transactions):
transactionId = transactions.first?.id
transactionsExpectation.fulfill()
case .failure(let error):
XCTFail("Getting transactions should NOT fail. Failed with \(error)")
transactionsExpectation.fulfill()
}
}
await fulfillment(of: [transactionsExpectation], timeout: 5)
guard let idToFinish = transactionId else {
XCTFail("Could not get transaction ID to finish")
return
}
// Finish the transaction
plugin.finish(id: idToFinish) { result in
switch result {
case .success():
finishExpectation.fulfill()
case .failure(let error):
XCTFail("Finish should NOT fail. Failed with \(error)")
}
}
await fulfillment(of: [finishExpectation], timeout: 5) |
||
|
|
||
| // Second purchase - this should also succeed | ||
| plugin.purchase(id: "consumable", options: nil) { result in | ||
| switch result { | ||
| case .success: | ||
| secondPurchaseExpectation.fulfill() | ||
| case .failure(let error): | ||
| XCTFail("Second purchase should NOT fail. Failed with \(error)") | ||
| } | ||
| } | ||
|
|
||
| await fulfillment(of: [secondPurchaseExpectation], timeout: 5) | ||
| } | ||
|
|
||
| @available(iOS 18.0, macOS 15.0, *) | ||
| func testIsWinBackOfferEligibleEligible() async throws { | ||
| let purchaseExpectation = self.expectation(description: "Purchase should succeed") | ||
|
|
||
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.
This feels like a case that should be treated as an error since the programmer is trying to finish a non-existent transaction and we should not fail silently? Unless there're legitimate cases where developers can't tell if a transaction is finished or not.
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.
Thanks for the review! I've addressed both points:
finish() now uses a new fetchUnfinishedTransaction() helper that only checks Transaction.unfinished
finish() now returns an error (storekit2_transaction_not_found) when the transaction is not found, instead of silently succeeding