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.

Sign up for GitHub

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

Rework the caching logic for subscription and entitlements #741

Merged
merged 7 commits into from
Mar 22, 2024
Merged

Rework the caching logic for subscription and entitlements #741

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
caf766b
Remove default cache expiry time
miasma13 Mar 22, 2024
2fcc47e
set 20min cache expiry for subscriptions
miasma13 Mar 22, 2024
da45ce4
set 20min cache expiry for entitlements
miasma13 Mar 22, 2024
b388f92
When caching subscription if expiry or renewal is earlier then use it
miasma13 Mar 22, 2024
f9f510d
Rework refreshSubscriptionAndEntitlements
miasma13 Mar 22, 2024
9ad326a
Add convenience init
miasma13 Mar 22, 2024
6b142ea
Swiftlint fix
miasma13 Mar 22, 2024
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
18 changes: 13 additions & 5 deletions Sources/Subscription/AccountManager.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,9 @@
public convenience init(subscriptionAppGroup: String) {
let accessTokenStorage = SubscriptionTokenKeychainStorage(keychainType: .dataProtection(.named(subscriptionAppGroup)))
self.init(accessTokenStorage: accessTokenStorage,
entitlementsCache: UserDefaultsCache<[Entitlement]>(subscriptionAppGroup: subscriptionAppGroup, key: UserDefaultsCacheKey.subscriptionEntitlements))
entitlementsCache: UserDefaultsCache<[Entitlement]>(subscriptionAppGroup: subscriptionAppGroup,

Check failure on line 59 in Sources/Subscription/AccountManager.swift

View workflow job for this annotation

GitHub Actions / Run SwiftLint 

Lines should not have trailing whitespace (trailing_whitespace)
key: UserDefaultsCacheKey.subscriptionEntitlements,
settings: UserDefaultsCacheSettings(defaultExpirationInterval: .minutes(20))))
Copy link
Member

Choose a reason for hiding this comment

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

This should be a configurable param as well.

}

public init(storage: AccountStorage = AccountKeychainStorage(),
Expand Down Expand Up @@ -304,16 +306,22 @@
}
}

public func checkSubscriptionState() async {
os_log(.info, log: .subscription, "[AccountManager] checkSubscriptionState")
public func refreshSubscriptionAndEntitlements() async {
os_log(.info, log: .subscription, "[AccountManager] refreshSubscriptionAndEntitlements")

guard let token = accessToken else { return }
guard let token = accessToken else {
SubscriptionService.signOut()
entitlementsCache.reset()
return
}

if case .success(let subscription) = await SubscriptionService.getSubscription(accessToken: token) {
if case .success(let subscription) = await SubscriptionService.getSubscription(accessToken: token, cachePolicy: .reloadIgnoringLocalCacheData) {
if !subscription.isActive {
signOut()
}
}

_ = await fetchEntitlements(cachePolicy: .reloadIgnoringLocalCacheData)
Copy link
Member

@quanganhdo quanganhdo Mar 22, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

If fetchRemoteEntitlements returns a failure I think we should handle this. Otherwise just blocking the endpoint at router-level would make the refresh meaningless.

}

@discardableResult
Expand Down
7 changes: 5 additions & 2 deletions Sources/Subscription/Services/SubscriptionService.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@ public final class SubscriptionService: APIService {
}
}

private static let subscriptionCache = UserDefaultsCache<Subscription>(key: UserDefaultsCacheKey.subscription)
private static let subscriptionCache = UserDefaultsCache<Subscription>(key: UserDefaultsCacheKey.subscription,
settings: UserDefaultsCacheSettings(defaultExpirationInterval: .minutes(20)))

public enum CachePolicy {
case reloadIgnoringLocalCacheData
Expand All @@ -55,7 +56,9 @@ public final class SubscriptionService: APIService {

switch result {
case .success(let subscriptionResponse):
subscriptionCache.set(subscriptionResponse)
let defaultExpiryDate = Date().addingTimeInterval(subscriptionCache.settings.defaultExpirationInterval)
Copy link
Member

Choose a reason for hiding this comment

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

This relies on system time which can be manipulated. If device time is set to a distance future, any entitlement check that depends on the cache would just succeed. While we do use reloadIgnoringLocalCacheData when refreshing periodically, we don't currently handle the failure.

let expiryDate = min(defaultExpiryDate, subscriptionResponse.expiresOrRenewsAt)
subscriptionCache.set(subscriptionResponse, expires: expiryDate)
return .success(subscriptionResponse)
case .failure(let error):
return .failure(.apiError(error))
Expand Down
15 changes: 5 additions & 10 deletions Sources/Subscription/UserDefaultsCache.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,7 @@ import Foundation
import Common

public struct UserDefaultsCacheSettings {

// Default expiration interval set to 24 hours
public let defaultExpirationInterval: TimeInterval

public init(defaultExpirationInterval: TimeInterval = 1 * 60 * 60) {
self.defaultExpirationInterval = defaultExpirationInterval
}
}

public enum UserDefaultsCacheKey: String {
Expand All @@ -43,7 +37,7 @@ public class UserDefaultsCache<ObjectType: Codable> {
}

private var subscriptionAppGroup: String?
private var settings: UserDefaultsCacheSettings
public private(set) var settings: UserDefaultsCacheSettings
private lazy var userDefaults: UserDefaults? = {
if let appGroup = subscriptionAppGroup {
return UserDefaults(suiteName: appGroup)
Expand All @@ -55,14 +49,15 @@ public class UserDefaultsCache<ObjectType: Codable> {
private let key: UserDefaultsCacheKey

public init(subscriptionAppGroup: String? = nil, key: UserDefaultsCacheKey,
settings: UserDefaultsCacheSettings = UserDefaultsCacheSettings()) {
settings: UserDefaultsCacheSettings) {
self.subscriptionAppGroup = subscriptionAppGroup
self.key = key
self.settings = settings
}

public func set(_ object: ObjectType, expires: Date = Date().addingTimeInterval(UserDefaultsCacheSettings().defaultExpirationInterval)) {
let cacheObject = CacheObject(expires: expires, object: object)
public func set(_ object: ObjectType, expires: Date? = nil) {
let expiryDate = expires ?? Date().addingTimeInterval(self.settings.defaultExpirationInterval)
let cacheObject = CacheObject(expires: expiryDate, object: object)
let encoder = JSONEncoder()
do {
let data = try encoder.encode(cacheObject)
Expand Down
Loading