PostedTransactionCache: add a TTL to every transaction

This was suggested in #2914 as a way to prevent the cache from growing forever. ### Other changes: - Added `PostedTransactionCache.unpostedTransactions`: this was added in #2914, so I put it here to prevent merge conflicts. - `PostedTransactionCache` now verifies that cache interaction doesn't happen in the main thread - Added debug log when saving transactions - Added debug log when pruning cache - Added `ClockType` tests
RevenueCat · Aug 16, 2023 · b4bdf4f · b4bdf4f
1 parent 93883e7
commit b4bdf4f
Show file tree

Hide file tree

Showing 9 changed files with 354 additions and 7 deletions.
diff --git a/RevenueCat.xcodeproj/project.pbxproj b/RevenueCat.xcodeproj/project.pbxproj
@@ -276,6 +276,7 @@
 		4FC083292A4A35FB00A97089 /* Integer+Extensions.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4FC083282A4A35FB00A97089 /* Integer+Extensions.swift */; };
 		4FC0832B2A4A361700A97089 /* IntegerExtensionsTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4FC0832A2A4A361700A97089 /* IntegerExtensionsTests.swift */; };
 		4FC6F8892A73E445002139B2 /* PostedTransactionCacheTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4FC6F8882A73E445002139B2 /* PostedTransactionCacheTests.swift */; };
+		4FC8EE522A8D34D70010EDFF /* ClockTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4FC8EE512A8D34D70010EDFF /* ClockTests.swift */; };
 		4FC972172A712DCC008593DE /* CachingProductsManagerTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4FC972162A712DCC008593DE /* CachingProductsManagerTests.swift */; };
 		4FCBA84F2A15391B004134BD /* SnapshotTesting+Extensions.swift in Sources */ = {isa = PBXBuildFile; fileRef = 576C8A9127D27DDD0058FA6E /* SnapshotTesting+Extensions.swift */; };
 		4FCBA8512A153940004134BD /* SnapshotTesting in Frameworks */ = {isa = PBXBuildFile; productRef = 4FCBA8502A153940004134BD /* SnapshotTesting */; };
@@ -1001,6 +1002,7 @@
 		4FC083282A4A35FB00A97089 /* Integer+Extensions.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "Integer+Extensions.swift"; sourceTree = "<group>"; };
 		4FC0832A2A4A361700A97089 /* IntegerExtensionsTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = IntegerExtensionsTests.swift; sourceTree = "<group>"; };
 		4FC6F8882A73E445002139B2 /* PostedTransactionCacheTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = PostedTransactionCacheTests.swift; sourceTree = "<group>"; };
+		4FC8EE512A8D34D70010EDFF /* ClockTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ClockTests.swift; sourceTree = "<group>"; };
 		4FC972162A712DCC008593DE /* CachingProductsManagerTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = CachingProductsManagerTests.swift; sourceTree = "<group>"; };
 		4FCBA8522A1539D0004134BD /* __Snapshots__ */ = {isa = PBXFileReference; lastKnownFileType = folder; path = __Snapshots__; sourceTree = "<group>"; };
 		4FCEEA5D2A379B80002C2112 /* DebugViewController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DebugViewController.swift; sourceTree = "<group>"; };
@@ -1941,6 +1943,7 @@
 				37E35B9AC7A350CA2437049D /* ISOPeriodFormatterTests.swift */,
 				37E35EEE7783629CDE41B70C /* SystemInfoTests.swift */,
 				57E2230627500BB1002DB06E /* AtomicTests.swift */,
+				4FC8EE512A8D34D70010EDFF /* ClockTests.swift */,
 				5722482627C2BD3200C524A7 /* LockTests.swift */,
 				576C8A8E27CFCD110058FA6E /* AnyEncodableTests.swift */,
 				574A2EE8282C403800150D40 /* AnyDecodableTests.swift */,
@@ -3618,6 +3621,7 @@
 				351B51BE26D450E800BD2BD7 /* CustomerInfoTests.swift in Sources */,
 				35272E1B26D0029300F22C3B /* DeviceCacheSubscriberAttributesTests.swift in Sources */,
 				5796A39627D6BDAB00653165 /* BackendPostOfferForSigningTests.swift in Sources */,
+				4FC8EE522A8D34D70010EDFF /* ClockTests.swift in Sources */,
 				57CFB96C27FE0E79002A6730 /* MockCurrentUserProvider.swift in Sources */,
 				57ACB13728184CF1000DCC9F /* DecoderExtensionTests.swift in Sources */,
 				351B516126D44BEB00BD2BD7 /* IdentityManagerTests.swift in Sources */,

diff --git a/Sources/Caching/PostedTransactionCache.swift b/Sources/Caching/PostedTransactionCache.swift
@@ -19,28 +19,78 @@ protocol PostedTransactionCacheType: Sendable {
     func savePostedTransaction(_ transaction: StoreTransactionType)
     func hasPostedTransaction(_ transaction: StoreTransactionType) -> Bool
 
+    /// - Returns: the subset of `transactions` that have not been posted.
+    func unpostedTransactions<T: StoreTransactionType>(in transactions: [T]) -> [T]
+
 }
 
 final class PostedTransactionCache: PostedTransactionCacheType {
 
-    private typealias StoredTransactions = Set<String>
+    /// The format of the cache: `StoreTransactionType.transactionIdentifier` -> `Date`.
+    private typealias StoredTransactions = [String: Date]
 
     private let deviceCache: DeviceCache
+    private let clock: ClockType
 
-    init(deviceCache: DeviceCache) {
+    init(deviceCache: DeviceCache, clock: ClockType) {
         self.deviceCache = deviceCache
+        self.clock = clock
     }
 
     func savePostedTransaction(_ transaction: StoreTransactionType) {
+        RCIntegrationTestAssertNotMainThread()
+
+        Logger.debug(Strings.purchase.saving_posted_transaction(transaction))
+
         self.deviceCache.update(key: CacheKey.transactions,
-                                default: Set<String>()) { transactions in
-            transactions.insert(transaction.transactionIdentifier)
+                                default: StoredTransactions()) { transactions in
+            self.pruneOldTransactions(from: &transactions)
+            transactions[transaction.transactionIdentifier] = self.clock.now
         }
     }
 
     func hasPostedTransaction(_ transaction: StoreTransactionType) -> Bool {
-        let transactions: StoredTransactions = self.deviceCache.value(for: CacheKey.transactions) ?? []
-        return transactions.contains(transaction.transactionIdentifier)
+        RCIntegrationTestAssertNotMainThread()
+
+        return self.storedTransactions.keys.contains(transaction.transactionIdentifier)
+    }
+
+    func unpostedTransactions<T: StoreTransactionType>(in transactions: [T]) -> [T] {
+        RCIntegrationTestAssertNotMainThread()
+
+        return Self.unpostedTransactions(in: transactions, with: self.storedTransactions.keys)
+    }
+
+    // MARK: -
+
+    private var storedTransactions: StoredTransactions {
+        return self.deviceCache.value(for: CacheKey.transactions) ?? [:]
+    }
+
+    private func pruneOldTransactions(from cache: inout StoredTransactions) {
+        let removedTransactions = cache.removeAll { self.clock.durationSince($0) > Self.cacheTTL.seconds }
+
+        if removedTransactions > 0 {
+            Logger.debug(Strings.purchase.pruned_old_posted_transactions_from_cache(count: removedTransactions))
+        }
+    }
+
+}
+
+extension PostedTransactionCache {
+
+    static let cacheTTL: DispatchTimeInterval = .days(90)
+
+}
+
+extension PostedTransactionCacheType {
+
+    /// - Returns: the subset of `transactions` that aren't included in `postedTransactions`.
+    static func unpostedTransactions<T: StoreTransactionType, C: Collection>(
+        in transactions: [T],
+        with postedTransactions: C
+    ) -> [T] where C.Element == String {
+        return transactions.filter { !postedTransactions.contains($0.transactionIdentifier) }
     }
 
 }

diff --git a/Sources/Error Handling/Assertions.swift b/Sources/Error Handling/Assertions.swift
@@ -13,6 +13,8 @@
 
 import Foundation
 
+// MARK: - Assertions
+
 /// Equivalent to `assert`, but will only evaluate condition during RC tests.
 /// - Note: this is a no-op in release builds.
 @inline(__always)
@@ -29,6 +31,24 @@ func RCTestAssert(
     #endif
 }
 
+/// Equivalent to `assert`, but will only evaluate condition during RC integration tests.
+/// - Note: this is a no-op in release builds.
+@inline(__always)
+func RCIntegrationTestAssert(
+    _ condition: @autoclosure () -> Bool,
+    _ message: @autoclosure () -> String,
+    file: StaticString = #file,
+    line: UInt = #line
+) {
+    #if DEBUG
+    guard ProcessInfo.isRunningIntegrationTests else { return }
+
+    precondition(condition(), message(), file: file, line: line)
+    #endif
+}
+
+// MARK: - Main Thread checks
+
 @inline(__always)
 func RCTestAssertNotMainThread(
     function: StaticString = #function,
@@ -44,3 +64,19 @@ func RCTestAssertNotMainThread(
     )
     #endif
 }
+
+@inline(__always)
+func RCIntegrationTestAssertNotMainThread(
+    function: StaticString = #function,
+    file: StaticString = #file,
+    line: UInt = #line
+) {
+    #if DEBUG
+    RCIntegrationTestAssert(
+        !Thread.isMainThread,
+        "\(function) should not be called from the main thread",
+        file: file,
+        line: line
+    )
+    #endif
+}
diff --git a/Sources/FoundationExtensions/Dictionary+Extensions.swift b/Sources/FoundationExtensions/Dictionary+Extensions.swift
@@ -28,6 +28,8 @@ extension Dictionary {
 
 }
 
+// MARK: - Merging
+
 extension Dictionary {
 
     /// Merge strategy to use for any duplicate keys.
@@ -100,6 +102,8 @@ extension Dictionary {
 
 }
 
+// MARK: - Mapping
+
 extension Dictionary {
 
     func mapKeys<NewKey: Hashable>(_ transformer: (Key) -> NewKey) -> [NewKey: Value] {
@@ -120,6 +124,8 @@ extension Dictionary {
 
 }
 
+// MARK: - Creation
+
 extension Sequence {
 
     /// Creates a `Dictionary` with the values in the receiver sequence, and the keys provided by `key`.
@@ -137,3 +143,23 @@ extension Sequence {
     }
 
 }
+
+// MARK: - Removing
+
+extension Dictionary {
+
+    /// - Returns: the number of removed keys
+    @discardableResult
+    mutating func removeAll(where shouldRemove: (Value) -> Bool) -> Int {
+        let keysToRemove = self
+            .filter { _, value in shouldRemove(value) }
+            .map(\.key)
+
+        for key in keysToRemove {
+            self.removeValue(forKey: key)
+        }
+
+        return keysToRemove.count
+    }
+
+}
diff --git a/Sources/Logging/Strings/PurchaseStrings.swift b/Sources/Logging/Strings/PurchaseStrings.swift
@@ -31,6 +31,8 @@ enum PurchaseStrings {
     case entitlements_revoked_syncing_purchases(productIdentifiers: [String])
     case entitlement_expired_outside_grace_period(expiration: Date, reference: Date)
     case finishing_transaction(StoreTransactionType)
+    case saving_posted_transaction(StoreTransactionType)
+    case pruned_old_posted_transactions_from_cache(count: Int)
     case finish_transaction_skipped_because_its_missing_in_non_subscriptions(StoreTransactionType,
                                                                              [NonSubscriptionTransaction])
     case purchasing_with_observer_mode_and_finish_transactions_false_warning
@@ -132,6 +134,13 @@ extension PurchaseStrings: LogMessage {
             return "Finishing transaction '\(transaction.transactionIdentifier)' " +
             "for product '\(transaction.productIdentifier)'"
 
+        case let .saving_posted_transaction(transaction):
+            return "Storing transaction '\(transaction.transactionIdentifier)' " +
+                   "for product '\(transaction.productIdentifier)' in cache"
+
+        case let .pruned_old_posted_transactions_from_cache(count):
+            return "Removed \(count) transactions from cache"
+
         case let .finish_transaction_skipped_because_its_missing_in_non_subscriptions(transaction, nonSubscriptions):
             return "Transaction '\(transaction.transactionIdentifier)' will not be finished: " +
             "it's a non-subscription and it's missing in CustomerInfo list: \(nonSubscriptions)"

diff --git a/Sources/Misc/DateAndTime/Clock.swift b/Sources/Misc/DateAndTime/Clock.swift
@@ -41,4 +41,12 @@ extension ClockType {
         }
     }
 
+    func durationSince(_ date: Date) -> TimeInterval {
+        if #available(iOS 13.0, macOS 10.15, tvOS 13.0, watchOS 6.2, *) {
+            return date.distance(to: self.now)
+        } else {
+            return date.timeIntervalSince(self.now)
+        }
+    }
+
 }
diff --git a/Tests/UnitTests/Caching/PostedTransactionCacheTests.swift b/Tests/UnitTests/Caching/PostedTransactionCacheTests.swift
@@ -22,14 +22,16 @@ class PostedTransactionCacheTests: TestCase {
     private var userDefaults: UserDefaults!
     private var deviceCache: DeviceCache!
     private var cache: PostedTransactionCache!
+    private var clock: TestClock!
 
     override func setUpWithError() throws {
         try super.setUpWithError()
 
         self.userDefaults = try XCTUnwrap(.init(suiteName: UUID().uuidString))
         self.deviceCache = .init(sandboxEnvironmentDetector: MockSandboxEnvironmentDetector(isSandbox: true),
                                  userDefaults: self.userDefaults)
-        self.cache = .init(deviceCache: self.deviceCache)
+        self.clock = .init()
+        self.cache = .init(deviceCache: self.deviceCache, clock: self.clock)
     }
 
     func testNoPostedTransactions() {
@@ -54,4 +56,105 @@ class PostedTransactionCacheTests: TestCase {
         expect(self.cache.hasPostedTransaction(transaction2)) == true
     }
 
+    func testHasPostedTransactionDoesNotTakeDateIntoAccount() {
+        let transaction = MockStoreTransaction()
+        self.cache.savePostedTransaction(transaction)
+
+        self.advanceClockByTTL()
+
+        expect(self.cache.hasPostedTransaction(transaction)) == true
+    }
+
+    func testSavePostedTransactionPrunesOldTransactions() {
+        let transaction1 = MockStoreTransaction()
+        let transaction2 = MockStoreTransaction()
+        let transaction3 = MockStoreTransaction()
+
+        self.cache.savePostedTransaction(transaction1)
+        self.cache.savePostedTransaction(transaction2)
+
+        self.advanceClockByTTL()
+
+        self.cache.savePostedTransaction(transaction3)
+
+        expect(self.cache.hasPostedTransaction(transaction1)) == false
+        expect(self.cache.hasPostedTransaction(transaction2)) == false
+        expect(self.cache.hasPostedTransaction(transaction3)) == true
+
+        self.logger.verifyMessageWasLogged(Strings.purchase.pruned_old_posted_transactions_from_cache(count: 2),
+                                           level: .debug,
+                                           expectedCount: 1)
+    }
+
+    // MARK: - unpostedTransactions
+
+    func testUnpostedTransactionsWithEmptyList() {
+        expect(self.cache.unpostedTransactions(in: [MockStoreTransaction]()))
+            .to(beEmpty())
+    }
+
+    func testUnpostedTransactionsWithOneUnpostedTransaction() {
+        let transaction = MockStoreTransaction()
+        expect(self.cache.unpostedTransactionsIdentifiers(in: [transaction])) == [transaction.transactionIdentifier]
+    }
+
+    func testUnpostedTransactionsWithOtherUnpostedTransaction() {
+        let transaction1 = MockStoreTransaction()
+        let transaction2 = MockStoreTransaction()
+
+        self.cache.savePostedTransaction(transaction2)
+
+        expect(self.cache.unpostedTransactionsIdentifiers(in: [transaction1])) == [transaction1.transactionIdentifier]
+    }
+
+    func testUnpostedTransactionsWithOnePostedTransaction() {
+        let transaction = MockStoreTransaction()
+
+        self.cache.savePostedTransaction(transaction)
+        expect(self.cache.unpostedTransactions(in: [transaction])).to(beEmpty())
+    }
+
+    func testUnpostedTransactionsWithSeveralUnpostedTransactions() {
+        let transaction1 = MockStoreTransaction()
+        let transaction2 = MockStoreTransaction()
+
+        expect(self.cache.unpostedTransactionsIdentifiers(in: [transaction1, transaction2])) == [
+            transaction1.transactionIdentifier,
+            transaction2.transactionIdentifier
+        ]
+    }
+
+    func testUnpostedTransactionsWithOnlySomeUnpostedTransactions() {
+        let transaction1 = MockStoreTransaction()
+        let transaction2 = MockStoreTransaction()
+        let transaction3 = MockStoreTransaction()
+        let transaction4 = MockStoreTransaction()
+
+        self.cache.savePostedTransaction(transaction2)
+        self.cache.savePostedTransaction(transaction4)
+
+        expect(self.cache.unpostedTransactionsIdentifiers(in: [transaction1, transaction2, transaction3])) == [
+            transaction1.transactionIdentifier,
+            transaction3.transactionIdentifier
+        ]
+    }
+
+}
+
+// MARK: -
+
+private extension PostedTransactionCacheTests {
+
+    private func advanceClockByTTL() {
+        self.clock.advance(by: PostedTransactionCache.cacheTTL + .minutes(1))
+    }
+
+}
+
+private extension PostedTransactionCacheType {
+
+    func unpostedTransactionsIdentifiers<T: StoreTransactionType>(in transactions: [T]) -> [String] {
+        return self.unpostedTransactions(in: transactions).map(\.transactionIdentifier)
+    }
+
 }