-
Notifications
You must be signed in to change notification settings - Fork 6
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
feat: Cache configuration for a given ClientSession #959
Changes from 14 commits
9d11732
29d48dc
b29480d
daae31c
0e7114c
da839d8
f9d54db
21a438e
5d3c7e4
f4e130e
64f86ce
661f8de
25220ff
228c930
3558a8c
9787b5f
4b75f7b
cf0b551
f9915e2
06048cb
6ff4a56
e0ad8b4
3ff3058
ca93d3e
5de40a6
6c03ba0
b457de7
5914d17
5e887f9
9e43a6f
4352203
0d4e825
dcc5998
56c8a10
0e20c49
1df039a
68e7282
9663e6d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,50 @@ | ||
// | ||
// Cache.swift | ||
// PrimerSDK | ||
// | ||
// Created by Niall Quinn on 31/07/24. | ||
// | ||
|
||
import Foundation | ||
|
||
class Cache<Key: Hashable, Value> { | ||
private let cache = NSCache<WrappedKey, Entry>() | ||
|
||
func insert(_ value: Value, forKey key: Key) { | ||
cache.setObject(Entry(value: value), forKey: WrappedKey(key)) | ||
} | ||
|
||
func value(forKey key: Key) -> Value? { | ||
guard let entry = cache.object(forKey: WrappedKey(key)) else { | ||
return nil | ||
} | ||
return entry.value | ||
} | ||
|
||
func removeValue(forKey key: Key) { | ||
cache.removeObject(forKey: WrappedKey(key)) | ||
} | ||
} | ||
|
||
private extension Cache { | ||
final class WrappedKey: NSObject { | ||
let key: Key | ||
init(_ key: Key) { self.key = key } | ||
|
||
override var hash: Int { return key.hashValue } | ||
|
||
override func isEqual(_ object: Any?) -> Bool { | ||
guard let value = object as? WrappedKey else { | ||
return false | ||
} | ||
return value.key == key | ||
} | ||
} | ||
} | ||
|
||
private extension Cache { | ||
final class Entry { | ||
let value: Value | ||
init(value: Value) { self.value = value } | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,78 @@ | ||
// | ||
// ConfigurationCache.swift | ||
// PrimerSDK | ||
// | ||
// Created by Niall Quinn on 31/07/24. | ||
// | ||
|
||
import Foundation | ||
|
||
protocol ConfigurationCaching { | ||
func clearCache() | ||
func setData(_ data: ConfigurationCachedData, forKey key: String) | ||
func data(forKey key: String) -> ConfigurationCachedData? | ||
} | ||
|
||
class ConfigurationCache: ConfigurationCaching { | ||
static let shared = ConfigurationCache() | ||
private var cache = Cache<String, ConfigurationCachedData>() | ||
|
||
func clearCache() { | ||
cache = Cache<String, ConfigurationCachedData>() | ||
} | ||
|
||
func data(forKey key: String) -> ConfigurationCachedData? { | ||
if let cachedData = cache.value(forKey: key) { | ||
if validateCachedConfig(key: key, cachedData: cachedData) == false { | ||
cache.removeValue(forKey: key) | ||
return nil | ||
} | ||
return cachedData | ||
} | ||
return nil | ||
} | ||
|
||
func setData(_ data: ConfigurationCachedData, forKey key: String) { | ||
// Cache includes at most one cached configuration | ||
clearCache() | ||
cache.insert(data, forKey: key) | ||
} | ||
|
||
private func validateCachedConfig(key: String, cachedData: ConfigurationCachedData) -> Bool { | ||
let timestamp = cachedData.timestamp | ||
let now = Date().timeIntervalSince1970 | ||
let timeInterval = now - timestamp | ||
|
||
if timeInterval > cachedData.ttl { | ||
return false | ||
} | ||
|
||
return true | ||
} | ||
} | ||
|
||
class ConfigurationCachedData { | ||
|
||
let config: PrimerAPIConfiguration | ||
let timestamp: TimeInterval | ||
let ttl: TimeInterval | ||
|
||
init(config: PrimerAPIConfiguration, headers: [String: String]? = nil) { | ||
//Extract ttl from headers | ||
self.config = config | ||
self.timestamp = Date().timeIntervalSince1970 | ||
self.ttl = Self.extractTtlFromHeaders(headers) | ||
} | ||
|
||
static let FallbackCacheExpiration: TimeInterval = 0 | ||
static let CacheHeaderKey = "x-primer-session-cache-ttl" | ||
|
||
private static func extractTtlFromHeaders(_ headers: [String: String]?) -> TimeInterval { | ||
guard let headers, | ||
let ttlHeaderValue = headers[Self.CacheHeaderKey], | ||
let ttlInt = Int(ttlHeaderValue) else { | ||
return Self.FallbackCacheExpiration | ||
} | ||
return TimeInterval(ttlInt) | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,6 +8,7 @@ internal protocol PrimerAPIConfigurationModuleProtocol { | |
static var clientToken: JWTToken? { get } | ||
static var decodedJWTToken: DecodedJWTToken? { get } | ||
static var apiConfiguration: PrimerAPIConfiguration? { get } | ||
|
||
static func resetSession() | ||
|
||
func setupSession( | ||
|
@@ -25,6 +26,11 @@ internal class PrimerAPIConfigurationModule: PrimerAPIConfigurationModuleProtoco | |
|
||
static var apiClient: PrimerAPIClientProtocol? | ||
|
||
private static let queue = DispatchQueue(label: "com.primer.configurationQueue") | ||
private static var pendingPromises: [String: Promise<PrimerAPIConfiguration>] = [:] | ||
|
||
private let logger = PrimerLogging.shared.logger | ||
NQuinn27 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
static var clientToken: JWTToken? { | ||
get { | ||
if PrimerAPIConfigurationModule.decodedJWTToken == nil { | ||
|
@@ -66,6 +72,13 @@ internal class PrimerAPIConfigurationModule: PrimerAPIConfigurationModuleProtoco | |
return decodedJWTToken | ||
} | ||
|
||
static var cacheKey: String? { | ||
guard let cacheKey = Self.clientToken else { | ||
return nil | ||
} | ||
return cacheKey | ||
} | ||
|
||
static func resetSession() { | ||
AppState.current.clientToken = nil | ||
AppState.current.apiConfiguration = nil | ||
|
@@ -102,7 +115,8 @@ internal class PrimerAPIConfigurationModule: PrimerAPIConfigurationModuleProtoco | |
|
||
func updateSession(withActions actionsRequest: ClientSessionUpdateRequest) -> Promise<Void> { | ||
return Promise { seal in | ||
guard let decodedJWTToken = PrimerAPIConfigurationModule.decodedJWTToken else { | ||
guard let decodedJWTToken = PrimerAPIConfigurationModule.decodedJWTToken, | ||
let cacheKey = Self.cacheKey else { | ||
let err = PrimerError.invalidClientToken(userInfo: .errorUserInfoDictionary(), | ||
diagnosticsId: UUID().uuidString) | ||
ErrorHandler.handle(error: err) | ||
|
@@ -112,11 +126,15 @@ internal class PrimerAPIConfigurationModule: PrimerAPIConfigurationModuleProtoco | |
|
||
let apiClient: PrimerAPIClientProtocol = PrimerAPIConfigurationModule.apiClient ?? PrimerAPIClient() | ||
apiClient.requestPrimerConfigurationWithActions(clientToken: decodedJWTToken, | ||
request: actionsRequest) { result in | ||
request: actionsRequest) { result, responseHeaders in | ||
switch result { | ||
case .success(let configuration): | ||
PrimerAPIConfigurationModule.apiConfiguration?.clientSession = configuration.clientSession | ||
seal.fulfill() | ||
_ = ImageFileProcessor().process(configuration: configuration).ensure { | ||
PrimerAPIConfigurationModule.apiConfiguration?.clientSession = configuration.clientSession | ||
let cachedData = ConfigurationCachedData(config: configuration, headers: responseHeaders) | ||
ConfigurationCache.shared.setData(cachedData, forKey: cacheKey) | ||
seal.fulfill() | ||
} | ||
case .failure(let err): | ||
seal.reject(err) | ||
} | ||
|
@@ -244,28 +262,72 @@ internal class PrimerAPIConfigurationModule: PrimerAPIConfigurationModuleProtoco | |
} | ||
} | ||
|
||
// swiftlint:disable:next function_body_length | ||
private func fetchConfiguration(requestDisplayMetadata: Bool) -> Promise<PrimerAPIConfiguration> { | ||
return Promise { seal in | ||
guard let clientToken = PrimerAPIConfigurationModule.decodedJWTToken else { | ||
guard let clientToken = PrimerAPIConfigurationModule.decodedJWTToken, | ||
let cacheKey = Self.cacheKey else { | ||
let err = PrimerError.invalidClientToken(userInfo: .errorUserInfoDictionary(), | ||
diagnosticsId: UUID().uuidString) | ||
ErrorHandler.handle(error: err) | ||
seal.reject(err) | ||
return | ||
} | ||
|
||
let requestParameters = Request.URLParameters.Configuration( | ||
skipPaymentMethodTypes: [], | ||
requestDisplayMetadata: requestDisplayMetadata) | ||
PrimerAPIConfigurationModule.queue.sync { | ||
if let cachedConfig = ConfigurationCache.shared.data(forKey: cacheKey) { | ||
let event = Analytics.Event.message( | ||
message: "Configuration cache hit with key: \(cacheKey)", | ||
messageType: .info, | ||
severity: .info | ||
) | ||
Analytics.Service.record(event: event) | ||
logger.debug(message: "Cached config used") | ||
seal.fulfill(cachedConfig.config) | ||
return | ||
} | ||
|
||
let apiClient: PrimerAPIClientProtocol = PrimerAPIConfigurationModule.apiClient ?? PrimerAPIClient() | ||
apiClient.fetchConfiguration(clientToken: clientToken, requestParameters: requestParameters) { (result) in | ||
switch result { | ||
case .failure(let err): | ||
seal.reject(err) | ||
case .success(let config): | ||
_ = ImageFileProcessor().process(configuration: config).ensure { | ||
if let pendingPromise = PrimerAPIConfigurationModule.pendingPromises[cacheKey as String] { | ||
pendingPromise.done { config in | ||
seal.fulfill(config) | ||
}.catch { error in | ||
seal.reject(error) | ||
} | ||
return | ||
} | ||
|
||
let promise = Promise<PrimerAPIConfiguration> { innerSeal in | ||
let requestParameters = Request.URLParameters.Configuration( | ||
skipPaymentMethodTypes: [], | ||
requestDisplayMetadata: requestDisplayMetadata) | ||
|
||
let apiClient: PrimerAPIClientProtocol = PrimerAPIConfigurationModule.apiClient ?? PrimerAPIClient() | ||
apiClient.fetchConfiguration(clientToken: clientToken, requestParameters: requestParameters) { (result, responseHeaders) in | ||
switch result { | ||
case .failure(let err): | ||
innerSeal.reject(err) | ||
case .success(let config): | ||
_ = ImageFileProcessor().process(configuration: config).ensure { | ||
// Cache the result | ||
let cachedData = ConfigurationCachedData(config: config, headers: responseHeaders) | ||
ConfigurationCache.shared.setData(cachedData, forKey: cacheKey) | ||
innerSeal.fulfill(config) | ||
} | ||
} | ||
} | ||
} | ||
|
||
PrimerAPIConfigurationModule.pendingPromises[cacheKey as String] = promise | ||
|
||
promise.done { config in | ||
seal.fulfill(config) | ||
}.catch { error in | ||
seal.reject(error) | ||
} | ||
|
||
promise.ensure { | ||
PrimerAPIConfigurationModule.queue.async { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I suspect there is a small chance here that you could end up adding a callback here that is never returned because it was added after ensure was called but prior to the queued item completing. I don't think this will play out in practice but worth thinking of whether there's a way to further streamline the promise-nesting to avoid this in the future. (Which is admittedly easier said than done!) |
||
PrimerAPIConfigurationModule.pendingPromises.removeValue(forKey: cacheKey as String) | ||
} | ||
} | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a potential case for having a
SingleItemCache
wrapper aroundNSCache
instead? Then the key could be passed in the init and removed from all the upstream methods hereMy concern would be someone seeing this and thinking that it supports multiple configurations because of the provision of a
key