PurchasesOrchestrator: fixed callback not invoked regression during…

… downgrades (#3028)

Fixes #3020.
This essentially reverts #2890.

As shown in #3020, downgrades are reported with the same transaction
identifier as the original transaction. Therefore our approach in #2890
is invalid.
We considered checking whether the `SKPayment` in the
`SKPaymentTransaction` matches the one added to the queue as a way to
disambiguate these transactions, but it doesn't. So we have no way of
telling the difference between queue transactions and purchases.

I've left the original tests as a way to at least document this
behavior.

Sources/Logging/Strings/PurchaseStrings.swift

@@ -42,9 +42,6 @@ enum PurchaseStrings {
                                                              SKPaymentTransaction)
     case paymentqueue_updated_transaction(SKPaymentTransactionObserver,
                                           SKPaymentTransaction)
-    case paymentqueue_ignoring_callback_for_older_transaction(PurchasesOrchestrator,
-                                                              StoreTransactionType,
-                                                              Date)
     case presenting_code_redemption_sheet
     case unable_to_present_redemption_sheet
     case purchases_synced
@@ -180,11 +177,6 @@ extension PurchaseStrings: LogMessage {
                 .compactMap { $0 }
                 .joined(separator: " ")
 
-        case let .paymentqueue_ignoring_callback_for_older_transaction(observer, transaction, date):
-            return "\(Strings.objectDescription(observer)): will not notify callback for transaction " +
-            "'\(transaction.transactionIdentifier)'. " +
-            "Transaction date '\(transaction.purchaseDate)' - callback date '\(date)'"
-
         case .presenting_code_redemption_sheet:
             return "Presenting code redemption sheet."
 

Sources/Purchasing/Purchases/PurchasesOrchestrator.swift

@@ -43,7 +43,7 @@ final class PurchasesOrchestrator {
 
     private let _allowSharingAppStoreAccount: Atomic<Bool?> = nil
     private let presentedOfferingIDsByProductID: Atomic<[String: String]> = .init([:])
-    private let purchaseCompleteCallbacksByProductID: Atomic<[String: CallbackData]> = .init([:])
+    private let purchaseCompleteCallbacksByProductID: Atomic<[String: PurchaseCompletedBlock]> = .init([:])
 
     private var appUserID: String { self.currentUserProvider.currentAppUserID }
     private var unsyncedAttributes: SubscriberAttribute.Dictionary {
@@ -825,28 +825,19 @@ private extension PurchasesOrchestrator {
     /// - Parameter restored: whether the transaction state was `.restored` instead of `.purchased`.
     private func purchaseSource(
         for productIdentifier: String,
-        transaction: StoreTransaction,
         restored: Bool
     ) -> PurchaseSource {
         let initiationSource: ProductRequestData.InitiationSource = {
             // Having a purchase completed callback implies that the transation comes from an explicit call
             // to `purchase()` instead of a StoreKit transaction notification.
-            // As long as the transaction date is _after_ the completion block was added.
-            let completionBlockDate = self.purchaseCompleteCallbacksByProductID.value[productIdentifier]?
-                .creationDate
-
-            switch (completionBlockDate, restored) {
-            case let (.some(completionBlockDate), false):
-                if transaction.purchaseDate > completionBlockDate {
-                    return .purchase
-                } else {
-                    return .queue
-                }
+            let hasPurchaseCallback = self.purchaseCompleteCallbacksByProductID.value.keys.contains(productIdentifier)
 
+            switch (hasPurchaseCallback, restored) {
+            case (true, false): return .purchase
                 // Note that restores initiated through the SDK with `restorePurchases`
                 // won't use this method since those set the initiation source explicitly.
-            case (.some, true): return .restore
-            case (.none, _): return .queue
+            case (true, true): return .restore
+            case (false, _): return .queue
             }
         }()
 
@@ -916,18 +907,6 @@ extension PurchasesOrchestrator: StoreKit2StorefrontListenerDelegate {
 
 private extension PurchasesOrchestrator {
 
-    struct CallbackData {
-
-        var completion: PurchaseCompletedBlock
-        var creationDate: Date
-
-        init(_ completion: @escaping PurchaseCompletedBlock, _ creationDate: Date = Date()) {
-            self.completion = completion
-            self.creationDate = creationDate
-        }
-
-    }
-
     /// - Returns: whether the callback was added
     @discardableResult
     func addPurchaseCompletedCallback(
@@ -956,30 +935,16 @@ private extension PurchasesOrchestrator {
                 return false
             }
 
-            callbacks[productIdentifier] = .init(completion)
+            callbacks[productIdentifier] = completion
             return true
         }
     }
 
     func getAndRemovePurchaseCompletedCallback(
         forTransaction transaction: StoreTransaction
     ) -> PurchaseCompletedBlock? {
-        return self.purchaseCompleteCallbacksByProductID.modify { callbacks -> PurchaseCompletedBlock? in
-            guard let value = callbacks[transaction.productIdentifier] else { return nil }
-
-            if !transaction.hasKnownPurchaseDate || value.creationDate <= transaction.purchaseDate {
-                callbacks.removeValue(forKey: transaction.productIdentifier)
-                return value.completion
-            } else {
-                // This callback was added to handle a purchase _after_ the transaction was created,
-                // therefore it should not be notified.
-                Logger.verbose(Strings.purchase.paymentqueue_ignoring_callback_for_older_transaction(
-                    self,
-                    transaction,
-                    value.creationDate
-                ))
-                return nil
-            }
+        return self.purchaseCompleteCallbacksByProductID.modify {
+            return $0.removeValue(forKey: transaction.productIdentifier)
         }
     }
 
@@ -1115,7 +1080,6 @@ private extension PurchasesOrchestrator {
                 aadAttributionToken: adServicesToken,
                 storefront: storefront,
                 source: self.purchaseSource(for: purchasedTransaction.productIdentifier,
-                                            transaction: purchasedTransaction,
                                             restored: restored)
             )
         ) { result in

Tests/UnitTests/Purchasing/Purchases/PurchasesPurchasingTests.swift

@@ -80,7 +80,11 @@ class PurchasesPurchasingTests: BasePurchasesTests {
         expect(self.backend.postedIsRestore) == false
     }
 
-    func testPurchaseCallbackIsNotInvokedWhenProcessingQueueTransactionForSameProduct() {
+    func testPurchaseCallbackIsInvokedWhenProcessingQueueTransactionForSameProduct() {
+        // This documents a race condition that we can't detect in the implementation
+        // where `PurchasesOrchestrator` can't tell the difference between `StoreKit 1` sending
+        // us a transaction from the queue, and sending us a transaction as a result of adding an `SKPayment`.
+
         let product = StoreProduct(sk1Product: MockSK1Product(mockProductIdentifier: "com.product.id1"))
 
         let mockPayment = MockPayment()
@@ -104,16 +108,20 @@ class PurchasesPurchasingTests: BasePurchasesTests {
         self.storeKit1Wrapper.delegate?.storeKit1Wrapper(self.storeKit1Wrapper, updatedTransaction: transaction)
 
         expect(self.backend.postReceiptDataCalled) == true
-        expect(self.backend.postedInitiationSource) == .queue
+        expect(self.backend.postedInitiationSource) == .purchase
 
         expect(self.storeKit1Wrapper.finishCalled).toEventually(beTrue())
 
         // Avoid false positives because the callback hasn't been invoked yet
         expect(self.mockOperationDispatcher.pendingMainActorDispatches.value).toEventually(equal(0))
-        expect(callbackInvoked) == false
+        expect(callbackInvoked) == true
     }
 
-    func testHandlesTransactionFromPurchaseIndependentlyFromQueueUpdateForSameProductIdentifier() throws {
+    func testHandlesTransactionFromPurchaseAfterReviewingQueueUpdateForSameProductIdentifier() throws {
+        // This documents a race condition that we can't detect in the implementation
+        // where `PurchasesOrchestrator` can't tell the difference between `StoreKit 1` sending
+        // us a transaction from the queue, and sending us a transaction as a result of adding an `SKPayment`.
+
         let product = StoreProduct(sk1Product: MockSK1Product(mockProductIdentifier: "com.product.id1"))
 
         let mockPayment = MockPayment()
@@ -135,7 +143,7 @@ class PurchasesPurchasingTests: BasePurchasesTests {
         self.storeKit1Wrapper.delegate?.storeKit1Wrapper(self.storeKit1Wrapper, updatedTransaction: queueTransaction)
 
         expect(self.backend.postReceiptDataCalled) == true
-        expect(self.backend.postedInitiationSource) == .queue
+        expect(self.backend.postedInitiationSource) == .purchase
         expect(callbackInvoked) == false
 
         let purchaseTransaction = MockTransaction()