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

PostedTransactionCache: add a TTL to every transaction #3023

Closed
wants to merge 1 commit into from
Closed
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
4 changes: 4 additions & 0 deletions RevenueCat.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -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 */; };
Expand Down Expand Up @@ -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>"; };
Expand Down Expand Up @@ -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 */,
Expand Down Expand Up @@ -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 */,
Expand Down
62 changes: 56 additions & 6 deletions Sources/Caching/PostedTransactionCache.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
Comment on lines +47 to +48
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 entire modification is done in place so we don't end up with 2 dictionary copies in memory.

}
}

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) }
}

}
Expand Down
36 changes: 36 additions & 0 deletions Sources/Error Handling/Assertions.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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,
Expand All @@ -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
}
26 changes: 26 additions & 0 deletions Sources/FoundationExtensions/Dictionary+Extensions.swift
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ extension Dictionary {

}

// MARK: - Merging

extension Dictionary {

/// Merge strategy to use for any duplicate keys.
Expand Down Expand Up @@ -100,6 +102,8 @@ extension Dictionary {

}

// MARK: - Mapping

extension Dictionary {

func mapKeys<NewKey: Hashable>(_ transformer: (Key) -> NewKey) -> [NewKey: Value] {
Expand All @@ -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`.
Expand All @@ -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
}

}
9 changes: 9 additions & 0 deletions Sources/Logging/Strings/PurchaseStrings.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)"
Expand Down
8 changes: 8 additions & 0 deletions Sources/Misc/DateAndTime/Clock.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

}
105 changes: 104 additions & 1 deletion Tests/UnitTests/Caching/PostedTransactionCacheTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -54,4 +56,105 @@ class PostedTransactionCacheTests: TestCase {
expect(self.cache.hasPostedTransaction(transaction2)) == true
}

func testHasPostedTransactionDoesNotTakeDateIntoAccount() {
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 makes hasPostedTransaction faster, we only prune when writing.

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)
}

}
Loading