Skip to content
26 changes: 7 additions & 19 deletions Modules/Sources/NetworkingCore/Network/AlamofireNetwork.swift
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,8 @@ public class AlamofireNetwork: Network {
/// authentication mode for requests
public private(set) var authenticationMode: RequestAuthenticationMode?

/// Lazy-initialized session manager. Use ensuresSessionManagerIsInitialized=true to avoid race conditions with concurrent requests.
private lazy var alamofireSession: Alamofire.Session = {
let sessionConfiguration = URLSessionConfiguration.default
let sessionManager = makeSession(configuration: sessionConfiguration)
return sessionManager
}()
/// Session manager used for Alamofire requests.
private let alamofireSession: Alamofire.Session

private let credentials: Credentials?

Expand Down Expand Up @@ -71,14 +67,11 @@ public class AlamofireNetwork: Network {
/// - selectedSite: Publisher for site selection changes.
/// This is necessary if you wish to enable network switching to direct requests while authenticated with WPCOM for better performance.
/// - sessionManager: Optional pre-configured session manager.
/// - ensuresSessionManagerIsInitialized: If true, the session is always set during initialization immediately to avoid lazy initialization race conditions.
/// Defaults to false for backward compatibility. Set to true when making concurrent requests immediately after initialization.
public required init(credentials: Credentials?,
selectedSite: AnyPublisher<JetpackSite?, Never>?,
appPasswordSupportState: AnyPublisher<Bool, Never>?,
userDefaults: UserDefaults = .standard,
sessionManager: Alamofire.Session? = nil,
ensuresSessionManagerIsInitialized: Bool = false) {
sessionManager: Alamofire.Session? = nil) {
self.credentials = credentials
self.selectedSite = selectedSite
self.userDefaults = userDefaults
Expand All @@ -96,11 +89,12 @@ public class AlamofireNetwork: Network {
}()
return RequestConverter(siteAddress: siteAddress)
}()
self.requestAuthenticator = RequestProcessor(requestAuthenticator: DefaultRequestAuthenticator(credentials: credentials))
let requestAuthenticator = RequestProcessor(requestAuthenticator: DefaultRequestAuthenticator(credentials: credentials))
self.requestAuthenticator = requestAuthenticator
if let sessionManager {
self.alamofireSession = sessionManager
} else if ensuresSessionManagerIsInitialized {
self.alamofireSession = makeSession(configuration: URLSessionConfiguration.default)
} else {
self.alamofireSession = Alamofire.Session(configuration: .default, interceptor: requestAuthenticator)
}

let authenticationMode: RequestAuthenticationMode? = {
Expand Down Expand Up @@ -294,12 +288,6 @@ private extension AlamofireNetwork {
}
}

/// Creates a session manager with request retrier and adapter
///
func makeSession(configuration sessionConfiguration: URLSessionConfiguration) -> Alamofire.Session {
Alamofire.Session(configuration: sessionConfiguration, interceptor: requestAuthenticator)
}

/// Updates `requestConverter` and `requestAuthenticator` when selected site changes
///
func observeSelectedSite(_ selectedSite: AnyPublisher<JetpackSite?, Never>) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,7 @@ public final class POSCatalogFullSyncService: POSCatalogFullSyncServiceProtocol
}
let network = AlamofireNetwork(credentials: credentials,
selectedSite: selectedSite,
appPasswordSupportState: appPasswordSupportState,
ensuresSessionManagerIsInitialized: true)
appPasswordSupportState: appPasswordSupportState)
let syncRemote = POSCatalogSyncRemote(network: network)
let persistenceService = POSCatalogPersistenceService(grdbManager: grdbManager)
self.init(syncRemote: syncRemote, batchSize: batchSize, persistenceService: persistenceService)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,7 @@ public final class POSCatalogIncrementalSyncService: POSCatalogIncrementalSyncSe
}
let network = AlamofireNetwork(credentials: credentials,
selectedSite: selectedSite,
appPasswordSupportState: appPasswordSupportState,
ensuresSessionManagerIsInitialized: true)
appPasswordSupportState: appPasswordSupportState)
let syncRemote = POSCatalogSyncRemote(network: network)
let persistenceService = POSCatalogPersistenceService(grdbManager: grdbManager)
self.init(syncRemote: syncRemote, batchSize: batchSize, persistenceService: persistenceService)
Expand Down
28 changes: 4 additions & 24 deletions Modules/Tests/NetworkingTests/Network/AlamofireNetworkTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -207,10 +207,11 @@ final class AlamofireNetworkTests: XCTestCase {

// MARK: - Session Initialization Tests

func test_concurrent_requests_do_not_fail_with_sessionDeinitialized_error_when_ensuresSessionManagerIsInitialized_is_true() async throws {
func test_concurrent_requests_do_not_fail_with_sessionDeinitialized_error() async throws {
// Given
let request = JetpackRequest(wooApiVersion: .mark1, method: .get, siteID: -1, path: "test")
let network = AlamofireNetwork(credentials: nil, selectedSite: nil, appPasswordSupportState: nil, ensuresSessionManagerIsInitialized: true)
let url = try XCTUnwrap(URL(string: "http://localhost:991929281"))
let request = URLRequest(url: url, timeoutInterval: 0.001)
let network = AlamofireNetwork(credentials: nil, selectedSite: nil, appPasswordSupportState: nil)

// When
async let request1 = network.responseDataAndHeaders(for: request)
Expand All @@ -228,27 +229,6 @@ final class AlamofireNetworkTests: XCTestCase {
}
}

func test_concurrent_requests_fail_with_sessionDeinitialized_error_when_ensuresSessionManagerIsInitialized_is_false() async throws {
// Given
let request = JetpackRequest(wooApiVersion: .mark1, method: .get, siteID: 1, path: "test")
let network = AlamofireNetwork(credentials: nil, selectedSite: nil, appPasswordSupportState: nil, ensuresSessionManagerIsInitialized: false)

// When
async let request1 = network.responseDataAndHeaders(for: request)
async let request2 = network.responseDataAndHeaders(for: request)
async let request3 = network.responseDataAndHeaders(for: request)

do {
_ = try await [request1, request2, request3]
XCTFail("Requests should fail with sessionDeinitialized error")
} catch Alamofire.AFError.sessionDeinitialized {
// Then
XCTAssertTrue(true)
} catch {
XCTFail("Requests should fail with sessionDeinitialized error, got \(error) instead")
}
}

// MARK: - Retry Logic Tests

func test_responseData_with_completion_retries_direct_request_when_converted_request_fails() throws {
Expand Down