diff --git a/TablePro/Core/Plugins/PluginManager+AutoUpdate.swift b/TablePro/Core/Plugins/PluginManager+AutoUpdate.swift index fd97147482..a3981f03be 100644 --- a/TablePro/Core/Plugins/PluginManager+AutoUpdate.swift +++ b/TablePro/Core/Plugins/PluginManager+AutoUpdate.swift @@ -41,8 +41,6 @@ extension PluginManager { func scheduleRegistryUpdatePass(after delay: Duration = .zero) { guard SoftwareUpdater.isUpdatingEnabled else { return } registryUpdateTask?.cancel() - // A connection the initial load releases runs before this task starts, and must wait for its repairs. - isRepairingRejectedPlugins = delay == .zero registryUpdateTask = Task { [weak self] in if delay > .zero { try? await Task.sleep(for: delay) @@ -74,9 +72,15 @@ extension PluginManager { registryClient: RegistryClient = .shared, appBuild: Int = Bundle.main.buildNumber ) async -> RegistryUpdatePassResult { + let targets = registryUpdateTargets(appBuild: appBuild) + guard !targets.isEmpty else { return .finished } + guard let manifest = await registryClient.fetchWaitingForConnectivity() else { + Self.logger.warning("Plugin update pass deferred: the plugin manifest is unavailable") + return .retry + } isRepairingRejectedPlugins = true defer { isRepairingRejectedPlugins = false } - return await applyRegistryUpdates(to: registryUpdateTargets(appBuild: appBuild), registryClient: registryClient) + return await applyRegistryUpdates(to: targets, manifest: manifest) } func waitForRejectedPluginRepairs() async { @@ -86,16 +90,8 @@ extension PluginManager { private func applyRegistryUpdates( to targets: [RegistryUpdateTarget], - registryClient: RegistryClient + manifest: RegistryManifest ) async -> RegistryUpdatePassResult { - guard !targets.isEmpty else { return .finished } - - await registryClient.fetch(policy: .mustBeCurrent) - guard registryClient.isManifestCurrent, let manifest = registryClient.manifest else { - Self.logger.warning("Plugin update pass deferred: the plugin manifest is unavailable") - return .retry - } - let pending = Self.pendingRegistryUpdates( installed: targets, manifest: manifest, @@ -320,7 +316,7 @@ extension PluginManager { } if driverPlugin(for: type) == nil, hasOutdatedRejectedPlugin(forTypeId: typeId) { Self.logger.info("Plugin '\(typeId)' is installed but outdated, updating it before connect") - await ensurePluginReady(forTypeId: typeId) + await ensurePluginReady(forTypeId: typeId, registryClient: registryClient) } guard driverPlugin(for: type) == nil else { return } if PluginMetadataRegistry.shared.snapshot(for: type) == nil { @@ -345,12 +341,19 @@ extension PluginManager { } } - func ensurePluginReady(forTypeId typeId: String) async { - guard SoftwareUpdater.isUpdatingEnabled, hasOutdatedRejectedPlugin(forTypeId: typeId) else { return } + func ensurePluginReady(forTypeId typeId: String, registryClient: RegistryClient = .shared) async { + guard SoftwareUpdater.isUpdatingEnabled else { return } await waitForRejectedPluginRepairs() + guard !registryUpdateTargets(forTypeId: typeId, appBuild: Bundle.main.buildNumber).isEmpty else { return } + let fetched = await registryClient.fetch(policy: .mustBeCurrent) + // The update pass may have got its manifest meanwhile and be installing this plugin. + await waitForRejectedPluginRepairs() + guard let manifest = fetched else { + Self.logger.warning("Plugin '\(typeId)' not updated before connect: the plugin manifest is unavailable") + return + } let targets = registryUpdateTargets(forTypeId: typeId, appBuild: Bundle.main.buildNumber) - guard !targets.isEmpty else { return } - _ = await applyRegistryUpdates(to: targets, registryClient: .shared) + _ = await applyRegistryUpdates(to: targets, manifest: manifest) AppEvents.shared.pluginsRejected.send(rejectedPlugins) } diff --git a/TablePro/Core/Plugins/PluginManager.swift b/TablePro/Core/Plugins/PluginManager.swift index 5a96b81733..9ee4c8efbd 100644 --- a/TablePro/Core/Plugins/PluginManager.swift +++ b/TablePro/Core/Plugins/PluginManager.swift @@ -112,7 +112,7 @@ final class PluginManager: ObservableObject { internal var registryUpdateAttempts = 0 internal var installsInFlight: Set = [] - // Set from a pass's start until its rejected plugins are done, so a connection never waits on a staging download. + // Set while a pass installs rejected plugins, so a connection waits for those but not for the network or a staging download. internal var isRepairingRejectedPlugins = false { didSet { guard !isRepairingRejectedPlugins else { return } diff --git a/TablePro/Core/Plugins/Registry/RegistryClient.swift b/TablePro/Core/Plugins/Registry/RegistryClient.swift index 16b68416bf..082bc1366f 100644 --- a/TablePro/Core/Plugins/Registry/RegistryClient.swift +++ b/TablePro/Core/Plugins/Registry/RegistryClient.swift @@ -19,13 +19,14 @@ internal final class RegistryClient: ObservableObject { internal private(set) var isManifestCurrent = false internal let session: URLSession + internal let updatePassSession: URLSession internal let manifestURL: URL private var inFlight: InFlightFetch? private struct InFlightFetch { let policy: RegistryFetchPolicy - let task: Task + let task: Task } internal init( @@ -34,6 +35,7 @@ internal final class RegistryClient: ObservableObject { ) { self.manifestURL = manifestURL self.session = session + updatePassSession = Self.makeUpdatePassSession(from: session) manifest = Self.cachedManifest(for: manifestURL, in: session.configuration.urlCache) } @@ -52,6 +54,14 @@ internal final class RegistryClient: ObservableObject { return URLSession(configuration: config) } + // Shares the interactive session's cache; offline it waits for the network until the resource timeout. + nonisolated internal static func makeUpdatePassSession(from session: URLSession) -> URLSession { + let config = session.configuration + config.waitsForConnectivity = true + config.timeoutIntervalForResource = 7 * 24 * 60 * 60 + return URLSession(configuration: config) + } + nonisolated private static func cachedManifest(for url: URL, in cache: URLCache?) -> RegistryManifest? { guard let cached = cache?.cachedResponse(for: URLRequest(url: url)) else { return nil } do { @@ -62,14 +72,24 @@ internal final class RegistryClient: ObservableObject { } } - internal func fetch(policy: RegistryFetchPolicy) async { + // The result is this fetch's own; isManifestCurrent also reflects the update pass's fetch. + @discardableResult + internal func fetch(policy: RegistryFetchPolicy) async -> RegistryManifest? { while let current = inFlight { - await current.task.value - if policy == .ifStale || current.policy == .mustBeCurrent { return } + let result = await current.task.value + if policy == .ifStale || current.policy == .mustBeCurrent { return result } + } + let task = Task { + defer { self.inFlight = nil } + return await self.load(self.makeRequest(policy: policy), from: self.session) } - let task = Task { await self.load(policy) } inFlight = InFlightFetch(policy: policy, task: task) - await task.value + return await task.value + } + + // Never coalesced with fetch(policy:), so an interactive caller never waits for the network with the pass. + internal func fetchWaitingForConnectivity() async -> RegistryManifest? { + await load(makeRequest(policy: .mustBeCurrent), from: updatePassSession) } internal func makeRequest(policy: RegistryFetchPolicy) -> URLRequest { @@ -83,23 +103,24 @@ internal final class RegistryClient: ObservableObject { return request } - private func load(_ policy: RegistryFetchPolicy) async { - defer { inFlight = nil } + private func load(_ request: URLRequest, from session: URLSession) async -> RegistryManifest? { do { - let (data, response) = try await session.data(for: makeRequest(policy: policy)) + let (data, response) = try await session.data(for: request) let status = (response as? HTTPURLResponse)?.statusCode ?? 0 guard (200...299).contains(status) else { Self.logger.error("Plugin manifest fetch returned HTTP \(status)") isManifestCurrent = false - return + return nil } let decoded = try JSONDecoder().decode(RegistryManifest.self, from: data) manifest = decoded isManifestCurrent = true Self.logger.info("Fetched the plugin manifest with \(decoded.plugins.count) plugin(s)") + return decoded } catch { isManifestCurrent = false Self.logger.error("Plugin manifest fetch failed: \(error.publicLogShape, privacy: .public)") + return nil } } } diff --git a/TableProTests/Core/Plugins/PluginRegistryUpdatePassTests.swift b/TableProTests/Core/Plugins/PluginRegistryUpdatePassTests.swift index f059ab4ce8..d97a591708 100644 --- a/TableProTests/Core/Plugins/PluginRegistryUpdatePassTests.swift +++ b/TableProTests/Core/Plugins/PluginRegistryUpdatePassTests.swift @@ -8,11 +8,17 @@ private final class ManifestStubProtocol: URLProtocol, @unchecked Sendable { private static let lock = NSLock() nonisolated(unsafe) private static var bodies: [URL: Data] = [:] nonisolated(unsafe) private static var requests: [URL: Int] = [:] + nonisolated(unsafe) private static var unansweredRequests: [URL: Int] = [:] static func serve(_ body: Data, at url: URL) { lock.withLock { bodies[url] = body } } + // An unanswered request stays open until its task is cancelled, like one waiting for the network. + static func leaveUnanswered(next count: Int, at url: URL) { + lock.withLock { unansweredRequests[url] = count } + } + static func requestCount(for url: URL) -> Int { lock.withLock { requests[url, default: 0] } } @@ -31,10 +37,15 @@ private final class ManifestStubProtocol: URLProtocol, @unchecked Sendable { client?.urlProtocol(self, didFailWithError: URLError(.badURL)) return } - let body = Self.lock.withLock { + let body: Data? = Self.lock.withLock { Self.requests[url, default: 0] += 1 + guard Self.unansweredRequests[url, default: 0] == 0 else { + Self.unansweredRequests[url, default: 0] -= 1 + return nil + } return Self.bodies[url] ?? Data() } + guard let body else { return } client?.urlProtocol(self, didReceive: response, cacheStoragePolicy: .notAllowed) client?.urlProtocol(self, didLoad: body) client?.urlProtocolDidFinishLoading(self) @@ -438,6 +449,42 @@ struct PluginRegistryUpdatePassTests { #expect(manager.stagedPluginIds.isEmpty) } + @Test("The pass fetches on a session that waits for the network; interactive fetches keep the 30 s one") + func passSessionWaitsForConnectivity() throws { + let manifestURL = try #require(URL(string: "https://registry.invalid/\(UUID().uuidString)/plugins/v3.json")) + let client = RegistryClient(manifestURL: manifestURL, session: RegistryClient.makeDefaultSession()) + let interactive = client.session.configuration + let pass = client.updatePassSession.configuration + + #expect(pass.waitsForConnectivity) + #expect(pass.timeoutIntervalForResource == 7 * 24 * 60 * 60) + #expect(interactive.timeoutIntervalForResource == 30) + #expect(pass.urlCache === interactive.urlCache) + } + + @Test("While the pass waits for the manifest, a connection neither waits for the pass nor joins its fetch") + func connectionSkipsThePassWaitingForTheNetwork() async throws { + let fixture = try makeFixture() + defer { try? FileManager.default.removeItem(at: fixture.root) } + let manager = fixture.manager + manager.rejectedPlugins = [makeRejected(bundleId: "com.example.driver")] + let (client, manifestURL) = try makeRegistryClient(servingBuild: 20) + ManifestStubProtocol.leaveUnanswered(next: 1, at: manifestURL) + + let pass = Task { await manager.runRegistryUpdatePass(registryClient: client, appBuild: 20) } + try await waitUntil { ManifestStubProtocol.requestCount(for: manifestURL) == 1 } + + #expect(!manager.isRepairingRejectedPlugins) + await manager.waitForRejectedPluginRepairs() + await client.fetch(policy: .mustBeCurrent) + #expect(client.isManifestCurrent) + #expect(ManifestStubProtocol.requestCount(for: manifestURL) == 2) + + pass.cancel() + let result = await pass.value + #expect(result == .retry) + } + @Test("Retries wait 30 s, 300 s, then 600 s, and stop after the fifth attempt") func retryBackoff() { let delays = (1...5).map { PluginManager.registryUpdateRetryDelay(afterAttempt: $0) } diff --git a/TableProTests/Core/Plugins/RegistryClientFreshnessTests.swift b/TableProTests/Core/Plugins/RegistryClientFreshnessTests.swift index 79bf4969a1..2fb699af01 100644 --- a/TableProTests/Core/Plugins/RegistryClientFreshnessTests.swift +++ b/TableProTests/Core/Plugins/RegistryClientFreshnessTests.swift @@ -144,12 +144,13 @@ struct RegistryClientFreshnessTests { ]) let (client, _) = try makeEnvironment() - async let first: Void = client.fetch(policy: .ifStale) - async let second: Void = client.fetch(policy: .ifStale) - _ = await (first, second) + async let first = client.fetch(policy: .ifStale) + async let second = client.fetch(policy: .ifStale) + let (firstResult, secondResult) = await (first, second) #expect(MockRegistryProtocol.requestCount == 1) #expect(client.isManifestCurrent) + #expect(firstResult != nil && secondResult != nil) } @Test("mustBeCurrent fetches again after an ifStale fetch and replaces the manifest") @@ -173,8 +174,9 @@ struct RegistryClientFreshnessTests { MockRegistryProtocol.reset(responses: [(500, Data())]) let (client, _) = try makeEnvironment(cachedManifest: manifestData(includingTestPlugin: true)) - await client.fetch(policy: .mustBeCurrent) + let result = await client.fetch(policy: .mustBeCurrent) + #expect(result == nil) #expect(client.manifest?.plugins.count == 1) #expect(!client.isManifestCurrent) }