diff --git a/Permanent/Common/Authentication/ViewModel/AuthViewModel.swift b/Permanent/Common/Authentication/ViewModel/AuthViewModel.swift index 94be699c8..79a5de3da 100644 --- a/Permanent/Common/Authentication/ViewModel/AuthViewModel.swift +++ b/Permanent/Common/Authentication/ViewModel/AuthViewModel.swift @@ -11,7 +11,7 @@ import UIKit typealias ServerResponse = (RequestStatus) -> Void class AuthViewModel: ViewModelInterface, LoginEventProtocol { - var sessionProtocol: NetworkSessionProtocol = APINetworkSession() + var sessionProtocol: NetworkSessionProtocol = APIRequestDispatcher.defaultNetworkSession() static let updateArchiveSettingsChevron = Notification.Name("AuthViewModel.updateArchiveSettingsChevron") var archiveSetingsWasPressed: Bool = false { didSet { diff --git a/Permanent/Common/Managers/Download/DownloadManagerGCD.swift b/Permanent/Common/Managers/Download/DownloadManagerGCD.swift index 2d9c23b71..bb6446f77 100644 --- a/Permanent/Common/Managers/Download/DownloadManagerGCD.swift +++ b/Permanent/Common/Managers/Download/DownloadManagerGCD.swift @@ -184,7 +184,7 @@ class DownloadManagerGCD: Downloader { let apiOperation = APIOperation(FilesEndpoint.download(url: url, filename: scopedFileName, progressHandler: progressHandler)) self.operation = apiOperation - apiOperation.execute(in: APIRequestDispatcher(networkSession: CDNSession())) { result in + apiOperation.execute(in: APIRequestDispatcher(networkSession: APIRequestDispatcher.defaultCDNSession())) { result in switch result { case .file(let fileURL, _): guard let url = fileURL else { diff --git a/Permanent/Common/Network/Layer/APINetworkSession.swift b/Permanent/Common/Network/Layer/APINetworkSession.swift index 4e921cc73..e15a78fa7 100644 --- a/Permanent/Common/Network/Layer/APINetworkSession.swift +++ b/Permanent/Common/Network/Layer/APINetworkSession.swift @@ -25,6 +25,63 @@ class APINetworkSession: NSObject { } } +#if DEBUG +/// Network session used by the unit suite: every task fails immediately with +/// `URLError(.notConnectedToInternet)` and no `URLResponse`. +/// +/// Chosen deliberately over returning a canned HTTP response. In +/// `APIRequestDispatcher.handleJsonTaskResponse` a `URLError` is delivered before `verify()` is +/// ever reached, so no HTTP status code is produced, `APIError.unauthorized` can never be +/// synthesized, and the 401 → `sessionExpiredNotificationName` → `logout()` path is unreachable +/// from tests. Completions still fire exactly once, on main, so nothing hangs. +/// +/// A test that needs a real response injects its own seam (`childrenFetchV2Request`, +/// `relocateV1Request`, a repository double, …) or passes `networkSession:` explicitly — neither +/// goes through this type. +final class OfflineTestNetworkSession: NetworkSessionProtocol { + private static let offline = URLError(.notConnectedToInternet) + + /// A real request is never instantaneous, and several tests legitimately observe in-flight + /// state (`isLoadingArchives`, "renders while searching") between kicking a request off and + /// its completion. Completing inline would collapse that window to nothing and break them for + /// the wrong reason. This is small enough to be free — nothing waits on it that did not + /// already wait on a much slower real request — and, unlike network latency, it is constant. + /// 150ms is chosen against the two constraints actually present in the suite: tests that + /// observe in-flight state sleep up to 100ms before asserting it (e.g. + /// ShareFindArchiveByEmailViewTests.testRendersWhileSearching), while tests that wait for + /// completion either use a publisher/expectation with a 1-3s timeout or sleep 200ms. So the + /// window is (100ms, 200ms) and this sits in the middle of it. + private static let latency: DispatchTimeInterval = .milliseconds(150) + + private func complete(_ work: @escaping () -> Void) { + DispatchQueue.global(qos: .userInitiated).asyncAfter(deadline: .now() + Self.latency, execute: work) + } + + func dataTask(with request: URLRequest, completionHandler: @escaping (Data?, URLResponse?, Error?) -> Void) -> URLSessionDataTask? { + complete { completionHandler(nil, nil, Self.offline) } + return nil + } + + // Upload and download deliberately mirror the REAL `APINetworkSession` rather than reporting + // `offline`: that session already refuses both inline with this same error and never touches + // the network, so substituting a URLError here would be a behaviour change, not isolation. + // It would also be a harmful one — `UploadManager.isTransientNetworkError` treats + // `.notConnectedToInternet` as transient and exempts it from the retry cap, so an upload + // failure would become infinitely retryable under test while staying capped in production. + private static let unsupported = NSError(domain: "org.permanent", code: 1000, userInfo: nil) + + func uploadTask(with request: URLRequest, progressHandler: ProgressHandler?, completion: @escaping (Data?, URLResponse?, Error?) -> Void) -> URLSessionUploadTask? { + completion(nil, nil, Self.unsupported) + return nil + } + + func downloadTask(with request: URLRequest, fileName: String, progressHandler: ProgressHandler?, completion: @escaping (URL?, URLResponse?, Error?) -> Void) -> URLSessionDownloadTask? { + completion(nil, nil, Self.unsupported) + return nil + } +} +#endif + extension APINetworkSession: NetworkSessionProtocol { func dataTask(with request: URLRequest, completionHandler: @escaping (Data?, URLResponse?, Error?) -> Void) -> URLSessionDataTask? { // Deliver the raw response on URLSession's background delegate queue — do NOT hop to diff --git a/Permanent/Common/Network/Layer/APIRequestDispatcher.swift b/Permanent/Common/Network/Layer/APIRequestDispatcher.swift index 7f980f2f2..ed31b1502 100644 --- a/Permanent/Common/Network/Layer/APIRequestDispatcher.swift +++ b/Permanent/Common/Network/Layer/APIRequestDispatcher.swift @@ -20,13 +20,53 @@ class APIRequestDispatcher: RequestDispatcherProtocol { /// The network session configuration. private var networkSession: NetworkSessionProtocol + #if DEBUG + /// True when this process is hosting an XCTest **unit** bundle. + /// + /// The unit suite must never reach the network. It used to: a full run issued ~255 live + /// requests to staging, whose 401s posted `sessionExpiredNotificationName` and produced 714 + /// asynchronous `logout()` calls — nulling `AuthenticationManager.shared.session` at random + /// points, so any test reading session-backed state could fail on timing alone. Three CI + /// failures came from that. It also meant the suite could mutate real staging data + /// (`/api/account/update`, `/api/archive/change`) whenever it ran with a valid token. + /// + /// `XCTestConfigurationFilePath` is set by XCTest in the process the test bundle is injected + /// into. Unit tests run inside the app host, so it is present. UI tests drive a separate app + /// process that is NOT given it, so `PermanentUITests` keeps its real networking — which also + /// means this needs no test-plan wiring and cannot be missed when a plan is added. + /// Read once; the environment is fixed for the process lifetime. + private static let isHostingUnitTests = + ProcessInfo.processInfo.environment["XCTestConfigurationFilePath"] != nil + #endif + + /// The session handed to dispatchers that don't specify one. Production code constructs + /// `APIRequestDispatcher()` at hundreds of call sites, so this default is the only practical + /// chokepoint for keeping the unit suite offline. Tests that want canned responses keep + /// injecting their own seam or passing `networkSession:` explicitly, and are unaffected. + static func defaultNetworkSession() -> NetworkSessionProtocol { + #if DEBUG + if isHostingUnitTests { return OfflineTestNetworkSession() } + #endif + return APINetworkSession() + } + + /// As `defaultNetworkSession()`, for the CDN-backed download path. Separate factory because + /// `CDNSession` carries its own configuration; the point is that the offline substitution + /// happens here too, so no call site can quietly bypass it. + static func defaultCDNSession() -> NetworkSessionProtocol { + #if DEBUG + if isHostingUnitTests { return OfflineTestNetworkSession() } + #endif + return CDNSession() + } + /// Required initializer. /// - Parameters: /// - environment: Instance conforming to `EnvironmentProtocol` used to determine on which environment the requests will be executed. /// - networkSession: Instance conforming to `NetworkSessionProtocol` used for executing requests with a specific configuration. required init( environment: EnvironmentProtocol = APIEnvironment.defaultEnv, - networkSession: NetworkSessionProtocol = APINetworkSession() + networkSession: NetworkSessionProtocol = APIRequestDispatcher.defaultNetworkSession() ) { self.environment = environment self.networkSession = networkSession diff --git a/Permanent/Modules/AccountSettings/ViewModel/InfoViewModel.swift b/Permanent/Modules/AccountSettings/ViewModel/InfoViewModel.swift index e5e385439..f5f979dd7 100644 --- a/Permanent/Modules/AccountSettings/ViewModel/InfoViewModel.swift +++ b/Permanent/Modules/AccountSettings/ViewModel/InfoViewModel.swift @@ -29,7 +29,7 @@ class InfoViewModel: ViewModelInterface { self.accountId = accountId } - var sessionProtocol: NetworkSessionProtocol = APINetworkSession() + var sessionProtocol: NetworkSessionProtocol = APIRequestDispatcher.defaultNetworkSession() func getUserData(then handler: @escaping (GetUserDataStatus) -> Void) { guard let accountId = accountId else { diff --git a/Permanent/Modules/PublicProfile/ViewModel/PublicGalleryViewModel.swift b/Permanent/Modules/PublicProfile/ViewModel/PublicGalleryViewModel.swift index 924fdfb71..20336d192 100644 --- a/Permanent/Modules/PublicProfile/ViewModel/PublicGalleryViewModel.swift +++ b/Permanent/Modules/PublicProfile/ViewModel/PublicGalleryViewModel.swift @@ -26,7 +26,7 @@ class PublicGalleryViewModel: ViewModelInterface { var searchQuery: String = "" var searchOperation: APIOperation? - var sessionProtocol: NetworkSessionProtocol = APINetworkSession() + var sessionProtocol: NetworkSessionProtocol = APIRequestDispatcher.defaultNetworkSession() func currentArchive() -> ArchiveVOData? { return AuthenticationManager.shared.session?.selectedArchive diff --git a/PermanentTests/FilesViewModelTests.swift b/PermanentTests/FilesViewModelTests.swift index 47de30928..5690753dc 100644 --- a/PermanentTests/FilesViewModelTests.swift +++ b/PermanentTests/FilesViewModelTests.swift @@ -1173,13 +1173,17 @@ final class FilesViewModelTests: XCTestCase { /// Stela flag ON, restoring both afterwards. private func withStelaSessionArchive(_ body: () -> Void) { let previousSession = AuthenticationManager.shared.session + let previousFlag = FeatureFlags.useStelaNavigation let session = PermSession(token: "test_token") session.selectedArchive = ArchiveVOData.mock() // archiveID 1 AuthenticationManager.shared.session = session FeatureFlags.useStelaNavigation = true defer { AuthenticationManager.shared.session = previousSession - FeatureFlags.useStelaNavigation = false + // Restore what was there, NOT a literal: the ambient value in this build is TRUE + // (Constants.swift derives it from APIEnvironment == .staging), so restoring false + // silently changed what every later test saw. + FeatureFlags.useStelaNavigation = previousFlag } body() } @@ -1203,8 +1207,12 @@ final class FilesViewModelTests: XCTestCase { let session = PermSession(token: "test_token") session.selectedArchive = ArchiveVOData.mock() AuthenticationManager.shared.session = session + let previousFlag = FeatureFlags.useStelaNavigation FeatureFlags.useStelaNavigation = false - defer { AuthenticationManager.shared.session = previousSession } + defer { + AuthenticationManager.shared.session = previousSession + FeatureFlags.useStelaNavigation = previousFlag + } XCTAssertFalse(MyFilesViewModel().isEligibleForStelaCopy(makeV2Record()), "flag off keeps every record on V1") @@ -1852,8 +1860,9 @@ final class FilesViewModelTests: XCTestCase { private var navParams: NavigateMinParams { ("0001-test", 11, nil) } func testNavigateV2_Committed_AppendsTargetAndSucceeds() { + let previousFlag = FeatureFlags.useStelaNavigation FeatureFlags.useStelaNavigation = true - defer { FeatureFlags.useStelaNavigation = false } + defer { FeatureFlags.useStelaNavigation = previousFlag } let (vm, fetchCount) = makeNavVM(outcomes: [.committed]) let target = makeV2FolderTarget() vm.v2NavigationTarget = target @@ -1867,8 +1876,9 @@ final class FilesViewModelTests: XCTestCase { } func testNavigateV2_ForwardSupersededOnce_RetriesAndWins() { + let previousFlag = FeatureFlags.useStelaNavigation FeatureFlags.useStelaNavigation = true - defer { FeatureFlags.useStelaNavigation = false } + defer { FeatureFlags.useStelaNavigation = previousFlag } let (vm, fetchCount) = makeNavVM(outcomes: [.superseded, .committed]) let target = makeV2FolderTarget() vm.v2NavigationTarget = target @@ -1882,8 +1892,9 @@ final class FilesViewModelTests: XCTestCase { } func testNavigateV2_ForwardSupersededTwice_CompletesQuietlyWithoutNavigating() { + let previousFlag = FeatureFlags.useStelaNavigation FeatureFlags.useStelaNavigation = true - defer { FeatureFlags.useStelaNavigation = false } + defer { FeatureFlags.useStelaNavigation = previousFlag } let (vm, fetchCount) = makeNavVM(outcomes: [.superseded, .superseded]) vm.v2NavigationTarget = makeV2FolderTarget() @@ -1896,8 +1907,9 @@ final class FilesViewModelTests: XCTestCase { } func testNavigateV2_BackOrRefreshSuperseded_CompletesQuietlyWithoutRetry() { + let previousFlag = FeatureFlags.useStelaNavigation FeatureFlags.useStelaNavigation = true - defer { FeatureFlags.useStelaNavigation = false } + defer { FeatureFlags.useStelaNavigation = previousFlag } let (vm, fetchCount) = makeNavVM(outcomes: [.superseded]) let current = makeV2FolderTarget() vm.navigationStack.append(current) diff --git a/PermanentTests/ManageTagsTests.swift b/PermanentTests/ManageTagsTests.swift index 370d216cb..a6d4f63f6 100644 --- a/PermanentTests/ManageTagsTests.swift +++ b/PermanentTests/ManageTagsTests.swift @@ -12,8 +12,16 @@ import KeychainSwift class ManageTagsTests: XCTestCase { var sut: ManageTagsViewModel! + /// Captured so tearDown can put the process-wide session back. createMockSession() below + /// installs a session whose account is AccountVOData.mock() ("Mock User"); leaving it behind + /// leaked into unrelated tests — it is what made + /// OnboardingContainerViewModelTests.testInit_NilCredentials_DefaultState see "Mock User" + /// instead of "". Previously masked by the suite's live-network 401s nulling the session. + private var previousSession: PermSession? + override func setUp() { super.setUp() + previousSession = AuthenticationManager.shared.session let tagsRemoteMockDataSource = TagsRemoteMockDataSource() let tagsManagementRepository = TagsRepository(remoteDataSource: tagsRemoteMockDataSource) @@ -23,6 +31,7 @@ class ManageTagsTests: XCTestCase { override func tearDown() { sut = nil + AuthenticationManager.shared.session = previousSession super.tearDown() } diff --git a/PermanentTests/ShareExtensionTests.swift b/PermanentTests/ShareExtensionTests.swift index 86f00b414..bfcf7f53e 100644 --- a/PermanentTests/ShareExtensionTests.swift +++ b/PermanentTests/ShareExtensionTests.swift @@ -19,15 +19,21 @@ class ShareExtensionTests: XCTestCase { let token: String = "token" + /// ShareExtensionViewModel.init assigns its session to the process-wide + /// PermSession.currentSession, so it must be restored or it leaks across the suite. + private var previousCurrentSession: PermSession? + override func setUpWithError() throws { try super.setUpWithError() - + previousCurrentSession = PermSession.currentSession + positiveTestInit() } override func tearDownWithError() throws { sut = nil - + PermSession.currentSession = previousCurrentSession + try super.tearDownWithError() } diff --git a/PermanentTests/ShareManagementTests.swift b/PermanentTests/ShareManagementTests.swift index acfc75de4..c9ef8861b 100644 --- a/PermanentTests/ShareManagementTests.swift +++ b/PermanentTests/ShareManagementTests.swift @@ -10,8 +10,13 @@ class ShareManagementTests: XCTestCase { var sut: ShareLinkViewModel! let token: String = "token" + /// See ManageTagsTests: createMockSession() installs a process-wide session that must be + /// put back, or it leaks into tests that assume no session. + private var previousSession: PermSession? + override func setUp() { super.setUp() + previousSession = AuthenticationManager.shared.session let downloadManagerMock = DownloadManagerMock() let shareManagementMockDataSource = ShareManagementMockRemoteDataSource() @@ -26,6 +31,7 @@ class ShareManagementTests: XCTestCase { override func tearDown() { sut = nil + AuthenticationManager.shared.session = previousSession super.tearDown() }