Skip to content
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

Purchasing: fixed consumable purchases by fixing transaction-finishing #1965

Merged
merged 4 commits into from
Oct 11, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 4 additions & 5 deletions Sources/Logging/Strings/PurchaseStrings.swift
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ enum PurchaseStrings {

case cannot_purchase_product_appstore_configuration_error
case entitlements_revoked_syncing_purchases(productIdentifiers: [String])
case finishing_transaction(transaction: SKPaymentTransaction)
case finishing_transaction(StoreTransaction)
case purchasing_with_observer_mode_and_finish_transactions_false_warning
case paymentqueue_removedtransaction(transaction: SKPaymentTransaction)
case paymentqueue_revoked_entitlements_for_product_identifiers(productIdentifiers: [String])
Expand Down Expand Up @@ -80,10 +80,9 @@ extension PurchaseStrings: CustomStringConvertible {
return "Entitlements revoked for product " +
"identifiers: \(productIdentifiers). \nsyncing purchases"

case .finishing_transaction(let transaction):
return "Finishing transaction \(transaction.payment.productIdentifier) " +
"\(transaction.transactionIdentifier ?? "") " +
"(\(transaction.original?.transactionIdentifier ?? ""))"
case let .finishing_transaction(transaction):
return "Finishing transaction '\(transaction.transactionIdentifier)' " +
"for product '\(transaction.productIdentifier)'"

case .purchasing_with_observer_mode_and_finish_transactions_false_warning:
return "Observer mode is active (finishTransactions is set to false) and " +
Expand Down
4 changes: 2 additions & 2 deletions Sources/Misc/Deprecations.swift
Original file line number Diff line number Diff line change
Expand Up @@ -403,8 +403,8 @@ extension CustomerInfo {
self.quantity = 1
}

func finish(_ wrapper: PaymentQueueWrapperType) {
// Nothing to do
func finish(_ wrapper: PaymentQueueWrapperType, completion: @escaping @Sendable () -> Void) {
completion()
}

}
Expand Down
36 changes: 28 additions & 8 deletions Sources/Purchasing/Purchases/PurchasesOrchestrator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -734,7 +734,7 @@ private extension PurchasesOrchestrator {
}
}

self.finishTransactionIfNeeded(storeTransaction)
self.finishTransactionIfNeeded(storeTransaction, completion: {})
}

func handleDeferredTransaction(_ transaction: SKPaymentTransaction) {
Expand Down Expand Up @@ -899,23 +899,30 @@ private extension PurchasesOrchestrator {
error: result.error)

let completion = self.getAndRemovePurchaseCompletedCallback(forTransaction: transaction)

let error = result.error
let finishable = error?.finishable ?? false

switch result {
case let .success(customerInfo):
self.customerInfoManager.cache(customerInfo: customerInfo, appUserID: appUserID)
completion?(transaction, customerInfo, nil, false)

self.finishTransactionIfNeeded(transaction)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was out of order.

self.finishTransactionIfNeeded(transaction) {
completion?(transaction, customerInfo, nil, false)
}

case let .failure(error):
let purchasesError = error.asPublicError

completion?(transaction, nil, purchasesError, false)
@MainActor
func complete() {
completion?(transaction, nil, purchasesError, false)
}

if finishable {
self.finishTransactionIfNeeded(transaction)
self.finishTransactionIfNeeded(transaction) { complete() }
} else {
complete()
}
}
}
Expand Down Expand Up @@ -1044,10 +1051,23 @@ private extension PurchasesOrchestrator {
self.offeringsManager.invalidateAndReFetchCachedOfferingsIfAppropiate(appUserID: self.appUserID)
}

func finishTransactionIfNeeded(_ transaction: StoreTransaction) {
if self.finishTransactions {
transaction.finish(self.paymentQueueWrapper.paymentQueueWrapperType)
func finishTransactionIfNeeded(
_ transaction: StoreTransaction,
completion: @escaping @Sendable @MainActor () -> Void
) {
@Sendable
func complete() {
self.operationDispatcher.dispatchOnMainActor(completion)
}

guard self.finishTransactions else {
complete()
return
}

Logger.purchase(Strings.purchase.finishing_transaction(transaction))

transaction.finish(self.paymentQueueWrapper.paymentQueueWrapperType, completion: complete)
}

}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,9 @@ internal struct SK1StoreTransaction: StoreTransactionType {
let transactionIdentifier: String
let quantity: Int

func finish(_ wrapper: PaymentQueueWrapperType) {
func finish(_ wrapper: PaymentQueueWrapperType, completion: @escaping @Sendable () -> Void) {
wrapper.finishTransaction(self.underlyingSK1Transaction)
completion()
}

}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ internal struct SK2StoreTransaction: StoreTransactionType {
let transactionIdentifier: String
let quantity: Int

func finish(_ wrapper: PaymentQueueWrapperType) {
_ = Task<Void, Never> {
func finish(_ wrapper: PaymentQueueWrapperType, completion: @escaping @Sendable () -> Void) {
Async.call(with: completion) {
await self.underlyingSK2Transaction.finish()
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,8 @@ public typealias SK2Transaction = StoreKit.Transaction
@objc public var transactionIdentifier: String { self.transaction.transactionIdentifier }
@objc public var quantity: Int { self.transaction.quantity }

func finish(_ wrapper: PaymentQueueWrapperType) {
self.transaction.finish(wrapper)
func finish(_ wrapper: PaymentQueueWrapperType, completion: @escaping @Sendable () -> Void) {
self.transaction.finish(wrapper, completion: completion)
}

// swiftlint:enable missing_docs
Expand Down Expand Up @@ -87,7 +87,7 @@ internal protocol StoreTransactionType: Sendable {

/// Indicates to the App Store that the app delivered the purchased content
/// or enabled the service to finish the transaction.
func finish(_ wrapper: PaymentQueueWrapperType)
func finish(_ wrapper: PaymentQueueWrapperType, completion: @escaping @Sendable () -> Void)

}

Expand Down
59 changes: 59 additions & 0 deletions Tests/BackendIntegrationTests/StoreKitIntegrationTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,47 @@ class StoreKit1IntegrationTests: BaseBackendIntegrationTests {
try await self.purchaseMonthlyProduct()
}

func testCanPurchaseConsumable() async throws {
let info = try await self.purchaseConsumablePackage().customerInfo

expect(info.allPurchasedProductIdentifiers).to(contain(Self.consumable10Coins))
}

func testCanPurchaseConsumableMultipleTimes() async throws {
// See https://revenuecats.atlassian.net/browse/TRIAGE-134
try XCTSkipIf(Self.storeKit2Setting == .disabled, "This test is not currently passing on SK1")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I filed [TRIAGE-136] for this.


let count = 2

for _ in 0..<count {
try await self.purchaseConsumablePackage()
}

let info = try await Purchases.shared.customerInfo()
expect(info.nonSubscriptions).to(haveCount(count))
expect(info.nonSubscriptions.map(\.productIdentifier)) == [
Self.consumable10Coins,
Self.consumable10Coins
]
}

func testCanPurchaseConsumableWithMultipleUsers() async throws {
func verifyPurchase(_ info: CustomerInfo) {
expect(info.nonSubscriptions).to(haveCount(1))
expect(info.nonSubscriptions.onlyElement?.productIdentifier) == Self.consumable10Coins
}

_ = try await Purchases.shared.logIn("user_1.\(UUID().uuidString)")
let info1 = try await self.purchaseConsumablePackage().customerInfo
verifyPurchase(info1)

let user2 = try await Purchases.shared.logIn("user_1.\(UUID().uuidString)").customerInfo
expect(user2.nonSubscriptions).to(beEmpty())

let info2 = try await self.purchaseConsumablePackage().customerInfo
verifyPurchase(info2)
}

func testSubscriptionIsSandbox() async throws {
let info = try await self.purchaseMonthlyOffering().customerInfo

Expand Down Expand Up @@ -394,6 +435,7 @@ class StoreKit1IntegrationTests: BaseBackendIntegrationTests {
private extension StoreKit1IntegrationTests {

static let entitlementIdentifier = "premium"
static let consumable10Coins = "consumable.10_coins"

private var currentOffering: Offering {
get async throws {
Expand Down Expand Up @@ -466,6 +508,23 @@ private extension StoreKit1IntegrationTests {
return data
}

@discardableResult
func purchaseConsumablePackage(
file: FileString = #file,
line: UInt = #line
NachoSoto marked this conversation as resolved.
Show resolved Hide resolved
) async throws -> PurchaseResultData {
let offering = try await XCTAsyncUnwrap(
try await Purchases.shared.offerings().offering(identifier: "coins"),
file: file, line: line
)
let package = try XCTUnwrap(
offering.package(identifier: "10.coins"),
file: file, line: line
)

return try await Purchases.shared.purchase(package: package)
}

@discardableResult
func verifyEntitlementWentThrough(
_ customerInfo: CustomerInfo,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,21 @@
"productID" : "lifetime",
"referenceName" : "lifetime",
"type" : "NonConsumable"
},
{
"displayPrice" : "0.99",
"familyShareable" : false,
"internalID" : "67E2FE0B",
"localizations" : [
{
"description" : "10 Coins",
"displayName" : "10 Coins",
"locale" : "en_US"
}
],
"productID" : "consumable.10_coins",
"referenceName" : "10 coins",
"type" : "Consumable"
}
],
"settings" : {
Expand Down
4 changes: 3 additions & 1 deletion Tests/UnitTests/Mocks/MockStoreKit1Wrapper.swift
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,11 @@ class MockStoreKit1Wrapper: StoreKit1Wrapper {
}

var finishCalled = false
var finishProductIdentifier: String?

override func finishTransaction(_ transaction: SKPaymentTransaction) {
finishCalled = true
self.finishCalled = true
self.finishProductIdentifier = transaction.productIdentifier
}

weak var mockDelegate: StoreKit1WrapperDelegate?
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,13 +98,23 @@ class PurchasesPurchasingTests: BasePurchasesTests {
}

func testFinishesTransactionsIfSentToBackendCorrectly() throws {
let product = StoreProduct(sk1Product: MockSK1Product(mockProductIdentifier: "com.product.id1"))
self.purchases.purchase(product: product) { (_, _, _, _) in }
var finished = true

let productID = "com.product.id1"
let product = StoreProduct(sk1Product: MockSK1Product(mockProductIdentifier: productID))

self.purchases.purchase(product: product) { (_, _, _, _) in
// Transactions must be finished by the time the callback is invoked.
expect(self.storeKit1Wrapper.finishCalled) == true
expect(self.storeKit1Wrapper.finishProductIdentifier) == productID

finished = true
}

let transaction = MockTransaction()
transaction.mockPayment = try XCTUnwrap(self.storeKit1Wrapper.payment)
transaction.mockState = .purchasing

transaction.mockState = SKPaymentTransactionState.purchasing
self.storeKit1Wrapper.delegate?.storeKit1Wrapper(self.storeKit1Wrapper, updatedTransaction: transaction)

self.backend.postReceiptResult = .success(try CustomerInfo(data: Self.emptyCustomerInfoData))
Expand All @@ -113,7 +123,7 @@ class PurchasesPurchasingTests: BasePurchasesTests {
self.storeKit1Wrapper.delegate?.storeKit1Wrapper(self.storeKit1Wrapper, updatedTransaction: transaction)

expect(self.backend.postReceiptDataCalled) == true
expect(self.storeKit1Wrapper.finishCalled).toEventually(beTrue())
expect(finished).toEventually(beTrue())
}

func testDoesntFinishTransactionsIfFinishingDisabled() throws {
Expand Down Expand Up @@ -157,11 +167,22 @@ class PurchasesPurchasingTests: BasePurchasesTests {
}

func testAfterSendingFinishesFromBackendErrorIfAppropriate() throws {
let product = StoreProduct(sk1Product: MockSK1Product(mockProductIdentifier: "com.product.id1"))
self.purchases.purchase(product: product) { (_, _, _, _) in }
var finished = false

let productID = "com.product.id1"
let product = StoreProduct(sk1Product: MockSK1Product(mockProductIdentifier: productID))

self.purchases.purchase(product: product) { (_, _, _, _) in
// Transactions must be finished by the time the callback is invoked.
expect(self.storeKit1Wrapper.finishCalled) == true
expect(self.storeKit1Wrapper.finishProductIdentifier) == productID

finished = true
}

let transaction = MockTransaction()
transaction.mockPayment = try XCTUnwrap(self.storeKit1Wrapper.payment)
transaction.mockState = .purchased

self.backend.postReceiptResult = .failure(
.networkError(.errorResponse(
Expand All @@ -170,11 +191,10 @@ class PurchasesPurchasingTests: BasePurchasesTests {
))
)

transaction.mockState = SKPaymentTransactionState.purchased
self.storeKit1Wrapper.delegate?.storeKit1Wrapper(self.storeKit1Wrapper, updatedTransaction: transaction)

expect(self.backend.postReceiptDataCalled) == true
expect(self.storeKit1Wrapper.finishCalled).toEventually(beTrue())
expect(finished).toEventually(beTrue())
}

func testNotifiesIfTransactionFailsFromBackend() throws {
Expand Down Expand Up @@ -525,10 +545,11 @@ class PurchasesPurchasingTests: BasePurchasesTests {
receivedError = error as NSError?
secondCompletionCalled = true
}

self.performTransaction()
}

self.performTransaction()
self.performTransaction()
}

expect(secondCompletionCalled).toEventually(beTrue(), timeout: .seconds(10))
Expand Down