From a07a30d977029ea863d9e4528f8aabf567880387 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Sun, 18 Oct 2026 18:54:01 +0700 Subject: [PATCH] fix(plugins): install rejected plugins one at a time --- .../Plugins/PluginManager+AutoUpdate.swift | 41 +++++---- TablePro/Core/Plugins/PluginManager.swift | 7 +- .../PluginRegistryUpdatePassTests.swift | 84 ++++++++++++++++--- 3 files changed, 103 insertions(+), 29 deletions(-) diff --git a/TablePro/Core/Plugins/PluginManager+AutoUpdate.swift b/TablePro/Core/Plugins/PluginManager+AutoUpdate.swift index a3981f03be..31ce3b0ba0 100644 --- a/TablePro/Core/Plugins/PluginManager+AutoUpdate.swift +++ b/TablePro/Core/Plugins/PluginManager+AutoUpdate.swift @@ -39,7 +39,7 @@ extension PluginManager { } func scheduleRegistryUpdatePass(after delay: Duration = .zero) { - guard SoftwareUpdater.isUpdatingEnabled else { return } + guard isUpdatingEnabled else { return } registryUpdateTask?.cancel() registryUpdateTask = Task { [weak self] in if delay > .zero { @@ -78,14 +78,27 @@ extension PluginManager { Self.logger.warning("Plugin update pass deferred: the plugin manifest is unavailable") return .retry } - isRepairingRejectedPlugins = true - defer { isRepairingRejectedPlugins = false } - return await applyRegistryUpdates(to: targets, manifest: manifest) + let repaired = await repairRejectedPlugins(targets.filter { $0.rejectedURL != nil }, manifest: manifest) + let updated = await applyRegistryUpdates(to: targets.filter { $0.rejectedURL == nil }, manifest: manifest) + return repaired == .retry ? .retry : updated } func waitForRejectedPluginRepairs() async { - guard isRepairingRejectedPlugins else { return } - await withCheckedContinuation { rejectedRepairWaiters.append($0) } + while isRepairingRejectedPlugins { + await withCheckedContinuation { rejectedRepairWaiters.append($0) } + } + } + + // One holder at a time: the pass and a connection would otherwise both install the same rejected plugin. + private func repairRejectedPlugins( + _ targets: [RegistryUpdateTarget], + manifest: RegistryManifest + ) async -> RegistryUpdatePassResult { + guard !targets.isEmpty else { return .finished } + await waitForRejectedPluginRepairs() + isRepairingRejectedPlugins = true + defer { isRepairingRejectedPlugins = false } + return await applyRegistryUpdates(to: targets, manifest: manifest) } private func applyRegistryUpdates( @@ -103,9 +116,6 @@ extension PluginManager { for update in pending { guard !Task.isCancelled else { return .finished } guard isRegistryUpdateTarget(update.target) else { continue } - if update.target.rejectedURL == nil { - isRepairingRejectedPlugins = false - } do { try await applyRegistryUpdate(update) } catch { @@ -341,19 +351,18 @@ extension PluginManager { } } - func ensurePluginReady(forTypeId typeId: String, registryClient: RegistryClient = .shared) async { - guard SoftwareUpdater.isUpdatingEnabled else { return } + func ensurePluginReady(forTypeId typeId: String, registryClient: RegistryClient) async { + guard 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 { + guard let manifest = await registryClient.fetch(policy: .mustBeCurrent) else { Self.logger.warning("Plugin '\(typeId)' not updated before connect: the plugin manifest is unavailable") + // The pass may have got its manifest meanwhile and be installing this plugin. + await waitForRejectedPluginRepairs() return } let targets = registryUpdateTargets(forTypeId: typeId, appBuild: Bundle.main.buildNumber) - _ = await applyRegistryUpdates(to: targets, manifest: manifest) + _ = await repairRejectedPlugins(targets, manifest: manifest) AppEvents.shared.pluginsRejected.send(rejectedPlugins) } diff --git a/TablePro/Core/Plugins/PluginManager.swift b/TablePro/Core/Plugins/PluginManager.swift index a2f90e47db..45502305e7 100644 --- a/TablePro/Core/Plugins/PluginManager.swift +++ b/TablePro/Core/Plugins/PluginManager.swift @@ -25,6 +25,7 @@ final class PluginManager: ObservableObject { private let builtInPluginsURL: URL? internal let userPluginsDir: URL internal let registryInstaller: any RegistryPluginInstalling + internal let isUpdatingEnabled: Bool @Published internal(set) var plugins: [PluginEntry] = [] @@ -112,7 +113,7 @@ final class PluginManager: ObservableObject { internal var registryUpdateAttempts = 0 internal var installsInFlight: Set = [] - // Set while a pass installs rejected plugins, so a connection waits for those but not for the network or a staging download. + // Held while the pass or a connection installs rejected plugins, never during a manifest fetch or a staging download. internal var isRepairingRejectedPlugins = false { didSet { guard !isRepairingRejectedPlugins else { return } @@ -132,12 +133,14 @@ final class PluginManager: ObservableObject { builtInPluginsURL: URL? = Bundle.main.builtInPlugInsURL, userPluginsDir: URL = PluginManager.defaultUserPluginsDir(), registryInstaller: any RegistryPluginInstalling = PluginInstaller.shared, - initialLoadClock: any Clock = ContinuousClock() + initialLoadClock: any Clock = ContinuousClock(), + isUpdatingEnabled: Bool = SoftwareUpdater.isUpdatingEnabled ) { self.builtInPluginsURL = builtInPluginsURL self.userPluginsDir = userPluginsDir self.registryInstaller = registryInstaller self.initialLoadClock = initialLoadClock + self.isUpdatingEnabled = isUpdatingEnabled } nonisolated static func defaultUserPluginsDir() -> URL { diff --git a/TableProTests/Core/Plugins/PluginRegistryUpdatePassTests.swift b/TableProTests/Core/Plugins/PluginRegistryUpdatePassTests.swift index 5125e8a678..e67d3d6cad 100644 --- a/TableProTests/Core/Plugins/PluginRegistryUpdatePassTests.swift +++ b/TableProTests/Core/Plugins/PluginRegistryUpdatePassTests.swift @@ -141,13 +141,19 @@ struct PluginRegistryUpdatePassTests { } private func makeFixture( - installer: any RegistryPluginInstalling = PluginInstaller.shared + installer: any RegistryPluginInstalling = PluginInstaller.shared, + isUpdatingEnabled: Bool = false ) throws -> Fixture { let root = FileManager.default.temporaryDirectory .appendingPathComponent("PluginRegistryUpdatePassTests-\(UUID().uuidString)", isDirectory: true) let pluginsDir = root.appendingPathComponent("Plugins", isDirectory: true) try FileManager.default.createDirectory(at: pluginsDir, withIntermediateDirectories: true) - let manager = PluginManager(builtInPluginsURL: nil, userPluginsDir: pluginsDir, registryInstaller: installer) + let manager = PluginManager( + builtInPluginsURL: nil, + userPluginsDir: pluginsDir, + registryInstaller: installer, + isUpdatingEnabled: isUpdatingEnabled + ) return Fixture(manager: manager, pluginsDir: pluginsDir, root: root) } @@ -461,29 +467,85 @@ struct PluginRegistryUpdatePassTests { #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() + @Test("While the pass waits for the manifest, connecting repairs the plugin with a manifest request of its own") + func connectionRepairsWhileThePassWaitsForTheNetwork() async throws { + let installer = FixtureRegistryInstaller() + let fixture = try makeFixture(installer: installer, isUpdatingEnabled: true) defer { try? FileManager.default.removeItem(at: fixture.root) } let manager = fixture.manager - manager.rejectedPlugins = [makeRejected(bundleId: "com.example.driver")] + manager.rejectedPlugins = [ + makeRejected(bundleId: "com.example.driver", providedDatabaseTypeIds: [DatabaseType.mssql.pluginTypeId]) + ] 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) + try await manager.prepareForConnecting(to: .mssql, registryClient: client) + #expect(ManifestStubProtocol.requestCount(for: manifestURL) == 2) + let installedIds = await installer.installedIds + #expect(installedIds == ["com.example.driver"]) pass.cancel() let result = await pass.value #expect(result == .retry) } + @Test("A pass whose manifest arrives during a connection's repair waits for it, then skips the plugin it repaired") + func passWaitsForAConnectionsRepair() async throws { + let installGate = AsyncGate() + let installer = FixtureRegistryInstaller(installGate: installGate) + let fixture = try makeFixture(installer: installer, isUpdatingEnabled: true) + defer { try? FileManager.default.removeItem(at: fixture.root) } + let manager = fixture.manager + let typeId = DatabaseType.mssql.pluginTypeId + let rejected = makeRejected(bundleId: "com.example.driver", providedDatabaseTypeIds: [typeId]) + manager.rejectedPlugins = [rejected] + let (client, _) = try makeRegistryClient(servingBuild: 20) + + let connection = Task { await manager.ensurePluginReady(forTypeId: typeId, registryClient: client) } + await installGate.arrived(1) + let pass = Task { await manager.runRegistryUpdatePass(registryClient: client, appBuild: 20) } + try await waitUntil { manager.rejectedRepairWaiters.count == 1 } + // Stands in for the load, which the unsigned fixture cannot pass. + manager.removeFromRejected(url: rejected.url) + await installGate.open() + await connection.value + + let result = await pass.value + #expect(result == .finished) + let installedIds = await installer.installedIds + #expect(installedIds == ["com.example.driver"]) + } + + @Test("A connection that arrives during the pass's repair waits for it, then skips the plugin it repaired") + func connectionWaitsForThePassRepair() async throws { + let installGate = AsyncGate() + let installer = FixtureRegistryInstaller(installGate: installGate) + let fixture = try makeFixture(installer: installer, isUpdatingEnabled: true) + defer { try? FileManager.default.removeItem(at: fixture.root) } + let manager = fixture.manager + let typeId = DatabaseType.mssql.pluginTypeId + let rejected = makeRejected(bundleId: "com.example.driver", providedDatabaseTypeIds: [typeId]) + manager.rejectedPlugins = [rejected] + let (client, manifestURL) = try makeRegistryClient(servingBuild: 20) + + let pass = Task { await manager.runRegistryUpdatePass(registryClient: client, appBuild: 20) } + await installGate.arrived(1) + let connection = Task { await manager.ensurePluginReady(forTypeId: typeId, registryClient: client) } + try await waitUntil { manager.rejectedRepairWaiters.count == 1 } + // Stands in for the load, which the unsigned fixture cannot pass. + manager.removeFromRejected(url: rejected.url) + await installGate.open() + await connection.value + + #expect(ManifestStubProtocol.requestCount(for: manifestURL) == 1) + let installedIds = await installer.installedIds + #expect(installedIds == ["com.example.driver"]) + _ = await pass.value + } + @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) }