-
Notifications
You must be signed in to change notification settings - Fork 0
feat: urlsessionconfiguration change requestTimeout #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
base: main
Are you sure you want to change the base?
Changes from all commits
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,76 @@ | ||
| import Foundation | ||
| import FlagsmithClient | ||
|
|
||
| class NetworkConfigExample { | ||
|
|
||
| func demonstrateNetworkConfiguration() { | ||
| // Get the shared Flagsmith instance | ||
| let flagsmith = Flagsmith.shared | ||
|
|
||
| // Set your API key | ||
| flagsmith.apiKey = "your-api-key-here" | ||
|
|
||
| // Configure network settings | ||
| configureNetworkSettings(flagsmith) | ||
|
|
||
| // Now all network requests will use these settings | ||
| fetchFeatureFlags(flagsmith) | ||
| } | ||
|
|
||
| private func configureNetworkSettings(_ flagsmith: Flagsmith) { | ||
| // Set a custom request timeout (customer requested 1 second instead of 60) | ||
| flagsmith.networkConfig.requestTimeout = 1.0 | ||
| } | ||
|
|
||
| private func fetchFeatureFlags(_ flagsmith: Flagsmith) { | ||
| flagsmith.getFeatureFlags { _ in | ||
| // Handle result... | ||
| } | ||
| } | ||
|
|
||
| func demonstrateTimeoutBehavior() { | ||
| let flagsmith = Flagsmith.shared | ||
| flagsmith.apiKey = "your-api-key-here" | ||
|
|
||
| // Set a very short timeout to demonstrate timeout behavior | ||
| flagsmith.networkConfig.requestTimeout = 0.1 // 100ms | ||
|
|
||
| print("Testing with 100ms timeout...") | ||
|
|
||
| flagsmith.getFeatureFlags { _ in | ||
| // Handle result... | ||
| } | ||
| } | ||
|
|
||
| func demonstrateCustomTimeout() { | ||
| let flagsmith = Flagsmith.shared | ||
| flagsmith.apiKey = "your-api-key-here" | ||
|
|
||
| // Set a custom timeout for your application's needs | ||
| flagsmith.networkConfig.requestTimeout = 5.0 // 5 seconds | ||
|
|
||
| print("Configured 5-second timeout for all requests") | ||
|
|
||
| // All API requests will now use the 5-second timeout | ||
| flagsmith.getFeatureFlags { _ in | ||
| // Handle result... | ||
| } | ||
| } | ||
|
|
||
| func demonstrateTimeoutChanges() { | ||
| let flagsmith = Flagsmith.shared | ||
| flagsmith.apiKey = "your-api-key-here" | ||
|
|
||
| // Start with default timeout | ||
| print("Using default timeout: \(flagsmith.networkConfig.requestTimeout) seconds") | ||
|
|
||
| // Change timeout during runtime | ||
| flagsmith.networkConfig.requestTimeout = 2.0 | ||
| print("Changed timeout to: \(flagsmith.networkConfig.requestTimeout) seconds") | ||
|
|
||
| // All subsequent requests will use the new timeout | ||
| flagsmith.getFeatureFlags { _ in | ||
| // Handle result... | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -111,6 +111,19 @@ public final class Flagsmith: @unchecked Sendable { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// Configuration class for network settings | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private var _networkConfig: NetworkConfig = .init() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| public var networkConfig: NetworkConfig { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| get { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| apiManager.propertiesSerialAccessQueue.sync { _networkConfig } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| set { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| apiManager.propertiesSerialAccessQueue.sync { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| _networkConfig = newValue | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private init() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| apiManager = APIManager() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| sseManager = SSEManager() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -408,3 +421,9 @@ public final class CacheConfig { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// Skip API if there is a cache available | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| public var skipAPI: Bool = false | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// Configuration class for network settings | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| public final class NetworkConfig { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// The timeout interval for URL requests, in seconds. Default is 60.0 seconds. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| public var requestTimeout: TimeInterval = 60.0 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+425
to
+429
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. Implement full NetworkConfig (thread-safe) to match docs/example Add missing properties and make them thread-safe. Also mark as Sendable if accessed across concurrency domains. -public final class NetworkConfig {
- /// The timeout interval for URL requests, in seconds. Default is 60.0 seconds.
- public var requestTimeout: TimeInterval = 60.0
-}
+public final class NetworkConfig: @unchecked Sendable {
+ private let queue = DispatchQueue(label: "flagsmith.networkConfig.queue", attributes: .concurrent)
+
+ // Backing storage
+ private var _requestTimeout: TimeInterval = 60.0
+ private var _resourceTimeout: TimeInterval = 604_800.0 // 7 days
+ private var _waitsForConnectivity: Bool = true
+ private var _allowsCellularAccess: Bool = true
+ private var _httpMaximumConnectionsPerHost: Int = 6
+ private var _httpAdditionalHeaders: [String: String] = [:]
+ private var _httpShouldUsePipelining: Bool = true
+ private var _httpShouldSetCookies: Bool = true
+
+ public init() {}
+
+ /// Timeout for individual requests (seconds). Default 60.0
+ public var requestTimeout: TimeInterval {
+ get { queue.sync { _requestTimeout } }
+ set { queue.async(flags: .barrier) { self._requestTimeout = newValue } }
+ }
+ /// Total timeout for a resource request (seconds). Default 7 days
+ public var resourceTimeout: TimeInterval {
+ get { queue.sync { _resourceTimeout } }
+ set { queue.async(flags: .barrier) { self._resourceTimeout = newValue } }
+ }
+ public var waitsForConnectivity: Bool {
+ get { queue.sync { _waitsForConnectivity } }
+ set { queue.async(flags: .barrier) { self._waitsForConnectivity = newValue } }
+ }
+ public var allowsCellularAccess: Bool {
+ get { queue.sync { _allowsCellularAccess } }
+ set { queue.async(flags: .barrier) { self._allowsCellularAccess = newValue } }
+ }
+ public var httpMaximumConnectionsPerHost: Int {
+ get { queue.sync { _httpMaximumConnectionsPerHost } }
+ set { queue.async(flags: .barrier) { self._httpMaximumConnectionsPerHost = newValue } }
+ }
+ public var httpAdditionalHeaders: [String: String] {
+ get { queue.sync { _httpAdditionalHeaders } }
+ set { queue.async(flags: .barrier) { self._httpAdditionalHeaders = newValue } }
+ }
+ public var httpShouldUsePipelining: Bool {
+ get { queue.sync { _httpShouldUsePipelining } }
+ set { queue.async(flags: .barrier) { self._httpShouldUsePipelining = newValue } }
+ }
+ public var httpShouldSetCookies: Bool {
+ get { queue.sync { _httpShouldSetCookies } }
+ set { queue.async(flags: .barrier) { self._httpShouldSetCookies = newValue } }
+ }
+}Additionally, ensure APIManager/SSEManager apply all of these properties when creating configurations (see file-specific suggestions). 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -13,7 +13,7 @@ import Foundation | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// Handles interaction with a **Flagsmith** api. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| final class APIManager: NSObject, URLSessionDataDelegate, @unchecked Sendable { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private var _session: URLSession! | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private var session: URLSession { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| internal var session: URLSession { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| get { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| propertiesSerialAccessQueue.sync { _session } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -70,11 +70,43 @@ final class APIManager: NSObject, URLSessionDataDelegate, @unchecked Sendable { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| override init() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| super.init() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let configuration = createURLSessionConfiguration() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| session = URLSession(configuration: configuration, delegate: self, delegateQueue: OperationQueue.main) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// Creates a URLSessionConfiguration with current network and cache settings | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private func createURLSessionConfiguration() -> URLSessionConfiguration { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let configuration = URLSessionConfiguration.default | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Set initial cache configuration - this will be updated when cache settings change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Apply network configuration - use default values during initialization | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| configuration.timeoutIntervalForRequest = 60.0 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| configuration.timeoutIntervalForResource = 604800.0 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| configuration.waitsForConnectivity = true | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| configuration.allowsCellularAccess = true | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| configuration.httpMaximumConnectionsPerHost = 6 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| configuration.httpAdditionalHeaders = [:] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| configuration.httpShouldUsePipelining = true | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| configuration.httpShouldSetCookies = true | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Apply cache configuration | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| configuration.urlCache = URLCache.shared | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| session = URLSession(configuration: configuration, delegate: self, delegateQueue: OperationQueue.main) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return configuration | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// Creates a URLSessionConfiguration with specific network and cache settings | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private func createURLSessionConfiguration(networkConfig: NetworkConfig, cacheConfig: CacheConfig) -> URLSessionConfiguration { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let configuration = URLSessionConfiguration.default | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Apply network configuration | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| configuration.timeoutIntervalForRequest = networkConfig.requestTimeout | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Apply cache configuration | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| configuration.urlCache = cacheConfig.cache | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return configuration | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+97
to
+108
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. 🛠️ Refactor suggestion | 🟠 Major Apply full NetworkConfig and CacheConfig when creating session config Only requestTimeout/urlCache are applied. Propagate all documented settings for consistency. - private func createURLSessionConfiguration(networkConfig: NetworkConfig, cacheConfig: CacheConfig) -> URLSessionConfiguration {
+ private func createURLSessionConfiguration(networkConfig: NetworkConfig, cacheConfig: CacheConfig) -> URLSessionConfiguration {
let configuration = URLSessionConfiguration.default
// Apply network configuration
- configuration.timeoutIntervalForRequest = networkConfig.requestTimeout
+ configuration.timeoutIntervalForRequest = networkConfig.requestTimeout
+ configuration.timeoutIntervalForResource = networkConfig.resourceTimeout
+ configuration.waitsForConnectivity = networkConfig.waitsForConnectivity
+ configuration.allowsCellularAccess = networkConfig.allowsCellularAccess
+ configuration.httpMaximumConnectionsPerHost = networkConfig.httpMaximumConnectionsPerHost
+ configuration.httpAdditionalHeaders = networkConfig.httpAdditionalHeaders as [AnyHashable: Any]
+ configuration.httpShouldUsePipelining = networkConfig.httpShouldUsePipelining
+ configuration.httpShouldSetCookies = networkConfig.httpShouldSetCookies
// Apply cache configuration
configuration.urlCache = cacheConfig.cache
return configuration
}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| func urlSession(_: URLSession, task: URLSessionTask, didCompleteWithError error: (any Error)?) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| serialAccessQueue.sync { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -153,14 +185,20 @@ final class APIManager: NSObject, URLSessionDataDelegate, @unchecked Sendable { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // we must use the delegate form here, not the completion handler, to be able to modify the cache | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| serialAccessQueue.sync { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Update session cache configuration if it has changed (must be done inside the serial queue) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if session.configuration.urlCache !== Flagsmith.shared.cacheConfig.cache { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let configuration = URLSessionConfiguration.default | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| configuration.urlCache = Flagsmith.shared.cacheConfig.cache | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| session = URLSession(configuration: configuration, delegate: self, delegateQueue: OperationQueue.main) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Always recreate session with current network and cache configuration | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // This ensures that any changes to network config are applied immediately | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let networkConfig = Flagsmith.shared.networkConfig | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let cacheConfig = Flagsmith.shared.cacheConfig | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let newConfig = createURLSessionConfiguration(networkConfig: networkConfig, cacheConfig: cacheConfig) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let newSession = URLSession(configuration: newConfig, delegate: self, delegateQueue: OperationQueue.main) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Invalidate previous session before swapping to avoid resource buildup | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let oldSession = self.session | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| self.session = newSession | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| oldSession.invalidateAndCancel() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let task = session.dataTask(with: request) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let task = newSession.dataTask(with: request) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| tasksToCompletionHandlers[task.taskIdentifier] = completion | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| task.resume() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
186
to
203
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. Do not cancel in-flight requests when rebuilding the session. Calling - let oldSession = self.session
- self.session = newSession
- oldSession.invalidateAndCancel()
+ let oldSession = self.session
+ self.session = newSession
+ oldSession.finishTasksAndInvalidate()📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -15,7 +15,7 @@ import Foundation | |||||||||||||||||||||||||||||||||||||||||||||||||||
| /// and handles reconnection logic with a backoff strategy. | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| final class SSEManager: NSObject, URLSessionDataDelegate, @unchecked Sendable { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| private var _session: URLSession! | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| private var session: URLSession { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| internal var session: URLSession { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| get { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| propertiesSerialAccessQueue.sync { _session } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -75,9 +75,37 @@ final class SSEManager: NSObject, URLSessionDataDelegate, @unchecked Sendable { | |||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| override init() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| super.init() | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| let configuration = URLSessionConfiguration.default | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| let configuration = createURLSessionConfiguration() | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| session = URLSession(configuration: configuration, delegate: self, delegateQueue: OperationQueue.main) | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// Creates a URLSessionConfiguration with current network settings | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| private func createURLSessionConfiguration() -> URLSessionConfiguration { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| let configuration = URLSessionConfiguration.default | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Apply network configuration - use default values during initialization | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| configuration.timeoutIntervalForRequest = 60.0 | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| configuration.timeoutIntervalForResource = 604800.0 | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| configuration.waitsForConnectivity = true | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| configuration.allowsCellularAccess = true | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| configuration.httpMaximumConnectionsPerHost = 6 | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| configuration.httpAdditionalHeaders = [:] | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| configuration.httpShouldUsePipelining = true | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| configuration.httpShouldSetCookies = true | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| return configuration | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// Creates a URLSessionConfiguration with specific network settings | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| private func createURLSessionConfiguration(networkConfig: NetworkConfig) -> URLSessionConfiguration { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| let configuration = URLSessionConfiguration.default | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Apply network configuration | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| configuration.timeoutIntervalForRequest = networkConfig.requestTimeout | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| return configuration | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+99
to
+107
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. 🛠️ Refactor suggestion | 🟠 Major Apply full NetworkConfig when creating session config Currently only requestTimeout is propagated. Apply the rest for consistency with docs/example. - private func createURLSessionConfiguration(networkConfig: NetworkConfig) -> URLSessionConfiguration {
+ private func createURLSessionConfiguration(networkConfig: NetworkConfig) -> URLSessionConfiguration {
let configuration = URLSessionConfiguration.default
// Apply network configuration
- configuration.timeoutIntervalForRequest = networkConfig.requestTimeout
+ configuration.timeoutIntervalForRequest = networkConfig.requestTimeout
+ configuration.timeoutIntervalForResource = networkConfig.resourceTimeout
+ configuration.waitsForConnectivity = networkConfig.waitsForConnectivity
+ configuration.allowsCellularAccess = networkConfig.allowsCellularAccess
+ configuration.httpMaximumConnectionsPerHost = networkConfig.httpMaximumConnectionsPerHost
+ configuration.httpAdditionalHeaders = networkConfig.httpAdditionalHeaders as [AnyHashable: Any]
+ configuration.httpShouldUsePipelining = networkConfig.httpShouldUsePipelining
+ configuration.httpShouldSetCookies = networkConfig.httpShouldSetCookies
return configuration
}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Helper function to process SSE data | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| internal func processSSEData(_ data: String) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -158,13 +186,24 @@ final class SSEManager: NSObject, URLSessionDataDelegate, @unchecked Sendable { | |||||||||||||||||||||||||||||||||||||||||||||||||||
| request.setValue("no-cache", forHTTPHeaderField: "Cache-Control") | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| request.setValue("keep-alive", forHTTPHeaderField: "Connection") | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Always recreate session with current network configuration | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| // This ensures that any changes to network config are applied immediately | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| let newConfig = createURLSessionConfiguration(networkConfig: Flagsmith.shared.networkConfig) | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| let newSession = URLSession(configuration: newConfig, delegate: self, delegateQueue: OperationQueue.main) | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Invalidate previous session before swapping to avoid resource buildup | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| let oldSession = self.session | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| self.session = newSession | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| oldSession.invalidateAndCancel() | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| completionHandler = completion | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| dataTask = session.dataTask(with: request) | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| dataTask = newSession.dataTask(with: request) | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| dataTask?.resume() | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| func stop() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| dataTask?.cancel() | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| dataTask = nil | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| completionHandler = nil | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,121 @@ | ||
| @testable import FlagsmithClient | ||
| import XCTest | ||
|
|
||
| final class NetworkConfigIntegrationTests: FlagsmithClientTestCase { | ||
|
|
||
| func testNetworkConfigValues() { | ||
| let flagsmith = Flagsmith.shared | ||
|
|
||
| // Set custom network configuration | ||
| flagsmith.networkConfig.requestTimeout = 1.0 | ||
|
|
||
| // Verify the configuration is applied correctly | ||
| XCTAssertEqual(flagsmith.networkConfig.requestTimeout, 1.0, "Request timeout should be applied") | ||
| } | ||
|
|
||
| func testAPIManagerUsesNetworkConfig() { | ||
| let flagsmith = Flagsmith.shared | ||
|
|
||
| // Set custom network configuration BEFORE creating APIManager | ||
| flagsmith.networkConfig.requestTimeout = 1.0 | ||
|
|
||
| // Create a new APIManager to test the configuration | ||
| let apiManager = APIManager() | ||
|
|
||
| // Set the API key on the APIManager instance | ||
| apiManager.apiKey = TestConfig.apiKey | ||
|
|
||
| // Trigger a request to apply the network configuration | ||
| let expectation = XCTestExpectation(description: "Request to apply config") | ||
| apiManager.request(.getFlags) { _ in | ||
| // Check the session configuration inside the completion handler | ||
| let sessionConfig = apiManager.session.configuration | ||
| XCTAssertEqual(sessionConfig.timeoutIntervalForRequest, 1.0, "Request timeout should be applied") | ||
| expectation.fulfill() | ||
| } | ||
| wait(for: [expectation], timeout: 2.0) | ||
| } | ||
|
|
||
| func testSSEManagerUsesNetworkConfig() { | ||
| let flagsmith = Flagsmith.shared | ||
|
|
||
| // Set custom network configuration | ||
| flagsmith.networkConfig.requestTimeout = 2.0 | ||
|
|
||
| // Create a new SSEManager to test the configuration | ||
| let sseManager = SSEManager() | ||
|
|
||
| // Set the API key on the SSEManager instance | ||
| sseManager.apiKey = TestConfig.apiKey | ||
|
|
||
| // Trigger the start method to apply the network configuration | ||
| // The start method calls the completion handler immediately, so we don't need to wait | ||
| sseManager.start { _ in | ||
| // This will be called immediately when start() is called | ||
| } | ||
|
|
||
| // Verify that the session configuration reflects our network config | ||
| let sessionConfig = sseManager.session.configuration | ||
| XCTAssertEqual(sessionConfig.timeoutIntervalForRequest, 2.0, "Request timeout should be applied") | ||
|
|
||
| // Clean up | ||
| sseManager.stop() | ||
| } | ||
|
|
||
| func testNetworkConfigChangesTriggerSessionRecreation() { | ||
| let flagsmith = Flagsmith.shared | ||
| let apiManager = APIManager() | ||
|
|
||
| // Set initial configuration and trigger a request to create the initial session | ||
| apiManager.apiKey = TestConfig.apiKey | ||
| flagsmith.networkConfig.requestTimeout = 60.0 | ||
|
|
||
| let initialExpectation = XCTestExpectation(description: "Initial request") | ||
| apiManager.request(.getFlags) { _ in | ||
| initialExpectation.fulfill() | ||
| } | ||
| wait(for: [initialExpectation], timeout: 1.0) | ||
|
|
||
| // Verify initial configuration | ||
| let initialSessionConfig = apiManager.session.configuration | ||
| XCTAssertEqual(initialSessionConfig.timeoutIntervalForRequest, 60.0, "Initial timeout should be applied") | ||
|
|
||
| // Change network configuration | ||
| flagsmith.networkConfig.requestTimeout = 30.0 | ||
|
|
||
| // Trigger a request to apply the new configuration | ||
| let expectation = XCTestExpectation(description: "Request completion") | ||
| apiManager.request(.getFlags) { _ in | ||
| expectation.fulfill() | ||
| } | ||
|
|
||
| wait(for: [expectation], timeout: 5.0) | ||
|
|
||
| // Verify that the new configuration is applied | ||
| let newSessionConfig = apiManager.session.configuration | ||
| XCTAssertEqual(newSessionConfig.timeoutIntervalForRequest, 30.0, "New timeout should be applied") | ||
| } | ||
|
|
||
| func testURLSessionConfigurationCreation() { | ||
| let flagsmith = Flagsmith.shared | ||
| flagsmith.networkConfig.requestTimeout = 1.0 | ||
|
|
||
| // Create a new APIManager to test the configuration | ||
| let apiManager = APIManager() | ||
|
|
||
| // Verify the configuration is applied correctly | ||
| let config = apiManager.session.configuration | ||
| XCTAssertEqual(config.timeoutIntervalForRequest, 1.0, "Request timeout should be applied") | ||
| } | ||
|
|
||
| func testNetworkConfigPerformance() { | ||
| let flagsmith = Flagsmith.shared | ||
|
|
||
| // Measure time to create and configure multiple network configs | ||
| measure { | ||
| for iteration in 0..<100 { | ||
| flagsmith.networkConfig.requestTimeout = Double(iteration) | ||
| } | ||
| } | ||
| } | ||
| } | ||
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.
Exposing a mutable reference is not thread-safe
networkConfig returns a shared reference to a class with unsynchronized vars. Concurrent mutations (as tested) create data races. Make NetworkConfig internally synchronized or expose a value-type snapshot API.
Follow the NetworkConfig refactor below to make properties thread-safe. This keeps the external API intact while preventing races.
🤖 Prompt for AI Agents