From 6f2619208cfb8c84d8c8aff7ac25468a9761a34e Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Mon, 9 Mar 2026 11:57:18 +0700 Subject: [PATCH 1/2] fix: resolve plugin system tech debt across 6 areas --- CHANGELOG.md | 4 ++ .../PluginDatabaseDriver.swift | 32 +++++++-- .../TableProPluginKit/TableProPlugin.swift | 5 ++ TablePro/Core/Plugins/PluginError.swift | 3 + TablePro/Core/Plugins/PluginManager.swift | 66 ++++++++++++++++--- .../Infrastructure/AppNotifications.swift | 4 ++ .../Plugins/InstalledPluginsView.swift | 12 ++++ 7 files changed, 111 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fb5fb8b6e..8d854de1e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- Plugin capability enforcement — registration now gated on declared capabilities, with validation warnings for mismatches +- Plugin dependency declarations — plugins can declare required dependencies via `TableProPlugin.dependencies`, validated at load time +- Plugin state change notification (`pluginStateDidChange`) posted when plugins are enabled/disabled +- Restart recommendation banner in Settings > Plugins after uninstalling a plugin - Startup commands — run custom SQL after connecting (e.g., SET time_zone) in Connection > Advanced tab - Plugin system architecture — all 8 database drivers (MySQL, PostgreSQL, SQLite, ClickHouse, MSSQL, MongoDB, Redis, Oracle) extracted into `.tableplugin` bundles loaded at runtime - Export format plugins — all 5 export formats (CSV, JSON, SQL, XLSX, MQL) extracted into `.tableplugin` bundles with plugin-provided option views and per-table option columns diff --git a/Plugins/TableProPluginKit/PluginDatabaseDriver.swift b/Plugins/TableProPluginKit/PluginDatabaseDriver.swift index 45358dac1..d176871bd 100644 --- a/Plugins/TableProPluginKit/PluginDatabaseDriver.swift +++ b/Plugins/TableProPluginKit/PluginDatabaseDriver.swift @@ -136,13 +136,35 @@ public extension PluginDatabaseDriver { } func executeParameterized(query: String, parameters: [String?]) async throws -> PluginQueryResult { - var sql = query - for param in parameters.reversed() { - if let range = sql.range(of: "?", options: .backwards) { - let replacement = param.map { "'\($0.replacingOccurrences(of: "'", with: "''"))'" } ?? "NULL" - sql.replaceSubrange(range, with: replacement) + var sql = "" + var paramIndex = 0 + var inSingleQuote = false + var inDoubleQuote = false + var prevChar: Character = "\0" + + for char in query { + if char == "'" && !inDoubleQuote && prevChar != "\\" { + inSingleQuote.toggle() + } else if char == "\"" && !inSingleQuote && prevChar != "\\" { + inDoubleQuote.toggle() } + + if char == "?" && !inSingleQuote && !inDoubleQuote && paramIndex < parameters.count { + if let value = parameters[paramIndex] { + let escaped = value + .replacingOccurrences(of: "\\", with: "\\\\") + .replacingOccurrences(of: "'", with: "''") + sql.append("'\(escaped)'") + } else { + sql.append("NULL") + } + paramIndex += 1 + } else { + sql.append(char) + } + prevChar = char } + return try await execute(query: sql) } diff --git a/Plugins/TableProPluginKit/TableProPlugin.swift b/Plugins/TableProPluginKit/TableProPlugin.swift index 9bae6eab9..84001e6e1 100644 --- a/Plugins/TableProPluginKit/TableProPlugin.swift +++ b/Plugins/TableProPluginKit/TableProPlugin.swift @@ -5,6 +5,11 @@ public protocol TableProPlugin: AnyObject { static var pluginVersion: String { get } static var pluginDescription: String { get } static var capabilities: [PluginCapability] { get } + static var dependencies: [String] { get } init() } + +public extension TableProPlugin { + static var dependencies: [String] { [] } +} diff --git a/TablePro/Core/Plugins/PluginError.swift b/TablePro/Core/Plugins/PluginError.swift index 529639310..5333996a0 100644 --- a/TablePro/Core/Plugins/PluginError.swift +++ b/TablePro/Core/Plugins/PluginError.swift @@ -18,6 +18,7 @@ enum PluginError: LocalizedError { case appVersionTooOld(minimumRequired: String, currentApp: String) case downloadFailed(String) case incompatibleWithCurrentApp(minimumRequired: String) + case dependencyMissing(pluginName: String, missingDependency: String) var errorDescription: String? { switch self { @@ -45,6 +46,8 @@ enum PluginError: LocalizedError { return String(localized: "Plugin download failed: \(reason)") case .incompatibleWithCurrentApp(let minimumRequired): return String(localized: "This plugin requires TablePro \(minimumRequired) or later") + case .dependencyMissing(let pluginName, let missingDependency): + return String(localized: "Plugin \"\(pluginName)\" requires \"\(missingDependency)\" which is not installed or disabled") } } } diff --git a/TablePro/Core/Plugins/PluginManager.swift b/TablePro/Core/Plugins/PluginManager.swift index 34ab2a1e2..38b46d56a 100644 --- a/TablePro/Core/Plugins/PluginManager.swift +++ b/TablePro/Core/Plugins/PluginManager.swift @@ -15,6 +15,8 @@ final class PluginManager { private(set) var plugins: [PluginEntry] = [] + private(set) var needsRestart = false + private(set) var driverPlugins: [String: any DriverPlugin] = [:] private(set) var exportPlugins: [String: any ExportFormatPlugin] = [:] @@ -53,6 +55,8 @@ final class PluginManager { loadPlugins(from: userPluginsDir, source: .userInstalled) + validateDependencies() + Self.logger.info("Loaded \(self.plugins.count) plugin(s): \(self.driverPlugins.count) driver(s), \(self.exportPlugins.count) export format(s)") } @@ -126,6 +130,7 @@ final class PluginManager { ) plugins.append(entry) + validateCapabilityDeclarations(principalClass, pluginId: bundleId) if entry.isEnabled { let instance = principalClass.init() @@ -140,19 +145,42 @@ final class PluginManager { // MARK: - Capability Registration private func registerCapabilities(_ instance: any TableProPlugin, pluginId: String) { + let declared = Set(type(of: instance).capabilities) + if let driver = instance as? any DriverPlugin { - let typeId = type(of: driver).databaseTypeId - driverPlugins[typeId] = driver - for additionalId in type(of: driver).additionalDatabaseTypeIds { - driverPlugins[additionalId] = driver + if declared.contains(.databaseDriver) { + let typeId = type(of: driver).databaseTypeId + driverPlugins[typeId] = driver + for additionalId in type(of: driver).additionalDatabaseTypeIds { + driverPlugins[additionalId] = driver + } + Self.logger.debug("Registered driver plugin '\(pluginId)' for database type '\(typeId)'") + } else { + Self.logger.warning("Plugin '\(pluginId)' conforms to DriverPlugin but does not declare .databaseDriver capability — skipping registration") } - Self.logger.debug("Registered driver plugin '\(pluginId)' for database type '\(typeId)'") } if let exportPlugin = instance as? any ExportFormatPlugin { - let formatId = type(of: exportPlugin).formatId - exportPlugins[formatId] = exportPlugin - Self.logger.debug("Registered export plugin '\(pluginId)' for format '\(formatId)'") + if declared.contains(.exportFormat) { + let formatId = type(of: exportPlugin).formatId + exportPlugins[formatId] = exportPlugin + Self.logger.debug("Registered export plugin '\(pluginId)' for format '\(formatId)'") + } else { + Self.logger.warning("Plugin '\(pluginId)' conforms to ExportFormatPlugin but does not declare .exportFormat capability — skipping registration") + } + } + } + + private func validateCapabilityDeclarations(_ pluginType: any TableProPlugin.Type, pluginId: String) { + let declared = Set(pluginType.capabilities) + let isDriver = pluginType is any DriverPlugin.Type + let isExporter = pluginType is any ExportFormatPlugin.Type + + if declared.contains(.databaseDriver) && !isDriver { + Self.logger.warning("Plugin '\(pluginId)' declares .databaseDriver but does not conform to DriverPlugin") + } + if declared.contains(.exportFormat) && !isExporter { + Self.logger.warning("Plugin '\(pluginId)' declares .exportFormat but does not conform to ExportFormatPlugin") } } @@ -200,6 +228,7 @@ final class PluginManager { } Self.logger.info("Plugin '\(pluginId)' \(enabled ? "enabled" : "disabled")") + NotificationCenter.default.post(name: .pluginStateDidChange, object: pluginId) } // MARK: - Install / Uninstall @@ -292,12 +321,29 @@ final class PluginManager { disabledPluginIds = disabled Self.logger.info("Uninstalled plugin '\(id)'") + needsRestart = true + } + + // MARK: - Dependency Validation + + private func validateDependencies() { + let loadedIds = Set(plugins.map(\.id)) + for plugin in plugins where plugin.isEnabled { + guard let principalClass = plugin.bundle.principalClass as? any TableProPlugin.Type else { continue } + let deps = principalClass.dependencies + for dep in deps { + if !loadedIds.contains(dep) { + Self.logger.warning("Plugin '\(plugin.id)' requires '\(dep)' which is not installed") + } else if let depEntry = plugins.first(where: { $0.id == dep }), !depEntry.isEnabled { + Self.logger.warning("Plugin '\(plugin.id)' requires '\(dep)' which is disabled") + } + } + } } // MARK: - Code Signature Verification - // TODO: Replace with actual team identifier - private static let signingTeamId = "YOURTEAMID" + private static let signingTeamId = "D7HJ5TFYCU" private func createSigningRequirement() -> SecRequirement? { var requirement: SecRequirement? diff --git a/TablePro/Core/Services/Infrastructure/AppNotifications.swift b/TablePro/Core/Services/Infrastructure/AppNotifications.swift index 1bd5c1535..2cdc88ffc 100644 --- a/TablePro/Core/Services/Infrastructure/AppNotifications.swift +++ b/TablePro/Core/Services/Infrastructure/AppNotifications.swift @@ -34,4 +34,8 @@ extension Notification.Name { static let sshTunnelDied = Notification.Name("sshTunnelDied") static let lastWindowDidClose = Notification.Name("lastWindowDidClose") + + // MARK: - Plugins + + static let pluginStateDidChange = Notification.Name("pluginStateDidChange") } diff --git a/TablePro/Views/Settings/Plugins/InstalledPluginsView.swift b/TablePro/Views/Settings/Plugins/InstalledPluginsView.swift index ce1982771..cc02a11dc 100644 --- a/TablePro/Views/Settings/Plugins/InstalledPluginsView.swift +++ b/TablePro/Views/Settings/Plugins/InstalledPluginsView.swift @@ -19,6 +19,18 @@ struct InstalledPluginsView: View { var body: some View { Form { + if pluginManager.needsRestart { + Section { + HStack(spacing: 8) { + Image(systemName: "arrow.clockwise.circle.fill") + .foregroundStyle(.orange) + Text("Restart TablePro to fully unload removed plugins.") + .font(.callout) + .foregroundStyle(.secondary) + } + } + } + Section("Installed Plugins") { ForEach(pluginManager.plugins) { plugin in pluginRow(plugin) From 3bc7a99079925755124f47f67af8596659168b52 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Mon, 9 Mar 2026 12:04:15 +0700 Subject: [PATCH 2/2] fix: address review feedback on plugin tech debt PR --- .../PluginDatabaseDriver.swift | 19 +++++++++--- TablePro/Core/Plugins/PluginError.swift | 3 -- TablePro/Core/Plugins/PluginManager.swift | 30 +++++++++---------- .../Plugins/InstalledPluginsView.swift | 11 ++++++- 4 files changed, 39 insertions(+), 24 deletions(-) diff --git a/Plugins/TableProPluginKit/PluginDatabaseDriver.swift b/Plugins/TableProPluginKit/PluginDatabaseDriver.swift index d176871bd..d9de50a83 100644 --- a/Plugins/TableProPluginKit/PluginDatabaseDriver.swift +++ b/Plugins/TableProPluginKit/PluginDatabaseDriver.swift @@ -140,12 +140,24 @@ public extension PluginDatabaseDriver { var paramIndex = 0 var inSingleQuote = false var inDoubleQuote = false - var prevChar: Character = "\0" + var isEscaped = false for char in query { - if char == "'" && !inDoubleQuote && prevChar != "\\" { + if isEscaped { + isEscaped = false + sql.append(char) + continue + } + + if char == "\\" && (inSingleQuote || inDoubleQuote) { + isEscaped = true + sql.append(char) + continue + } + + if char == "'" && !inDoubleQuote { inSingleQuote.toggle() - } else if char == "\"" && !inSingleQuote && prevChar != "\\" { + } else if char == "\"" && !inSingleQuote { inDoubleQuote.toggle() } @@ -162,7 +174,6 @@ public extension PluginDatabaseDriver { } else { sql.append(char) } - prevChar = char } return try await execute(query: sql) diff --git a/TablePro/Core/Plugins/PluginError.swift b/TablePro/Core/Plugins/PluginError.swift index 5333996a0..529639310 100644 --- a/TablePro/Core/Plugins/PluginError.swift +++ b/TablePro/Core/Plugins/PluginError.swift @@ -18,7 +18,6 @@ enum PluginError: LocalizedError { case appVersionTooOld(minimumRequired: String, currentApp: String) case downloadFailed(String) case incompatibleWithCurrentApp(minimumRequired: String) - case dependencyMissing(pluginName: String, missingDependency: String) var errorDescription: String? { switch self { @@ -46,8 +45,6 @@ enum PluginError: LocalizedError { return String(localized: "Plugin download failed: \(reason)") case .incompatibleWithCurrentApp(let minimumRequired): return String(localized: "This plugin requires TablePro \(minimumRequired) or later") - case .dependencyMissing(let pluginName, let missingDependency): - return String(localized: "Plugin \"\(pluginName)\" requires \"\(missingDependency)\" which is not installed or disabled") } } } diff --git a/TablePro/Core/Plugins/PluginManager.swift b/TablePro/Core/Plugins/PluginManager.swift index 38b46d56a..3d3c9ce8a 100644 --- a/TablePro/Core/Plugins/PluginManager.swift +++ b/TablePro/Core/Plugins/PluginManager.swift @@ -148,26 +148,24 @@ final class PluginManager { let declared = Set(type(of: instance).capabilities) if let driver = instance as? any DriverPlugin { - if declared.contains(.databaseDriver) { - let typeId = type(of: driver).databaseTypeId - driverPlugins[typeId] = driver - for additionalId in type(of: driver).additionalDatabaseTypeIds { - driverPlugins[additionalId] = driver - } - Self.logger.debug("Registered driver plugin '\(pluginId)' for database type '\(typeId)'") - } else { - Self.logger.warning("Plugin '\(pluginId)' conforms to DriverPlugin but does not declare .databaseDriver capability — skipping registration") + if !declared.contains(.databaseDriver) { + Self.logger.warning("Plugin '\(pluginId)' conforms to DriverPlugin but does not declare .databaseDriver capability — registering anyway") + } + let typeId = type(of: driver).databaseTypeId + driverPlugins[typeId] = driver + for additionalId in type(of: driver).additionalDatabaseTypeIds { + driverPlugins[additionalId] = driver } + Self.logger.debug("Registered driver plugin '\(pluginId)' for database type '\(typeId)'") } if let exportPlugin = instance as? any ExportFormatPlugin { - if declared.contains(.exportFormat) { - let formatId = type(of: exportPlugin).formatId - exportPlugins[formatId] = exportPlugin - Self.logger.debug("Registered export plugin '\(pluginId)' for format '\(formatId)'") - } else { - Self.logger.warning("Plugin '\(pluginId)' conforms to ExportFormatPlugin but does not declare .exportFormat capability — skipping registration") + if !declared.contains(.exportFormat) { + Self.logger.warning("Plugin '\(pluginId)' conforms to ExportFormatPlugin but does not declare .exportFormat capability — registering anyway") } + let formatId = type(of: exportPlugin).formatId + exportPlugins[formatId] = exportPlugin + Self.logger.debug("Registered export plugin '\(pluginId)' for format '\(formatId)'") } } @@ -228,7 +226,7 @@ final class PluginManager { } Self.logger.info("Plugin '\(pluginId)' \(enabled ? "enabled" : "disabled")") - NotificationCenter.default.post(name: .pluginStateDidChange, object: pluginId) + NotificationCenter.default.post(name: .pluginStateDidChange, object: nil, userInfo: ["pluginId": pluginId]) } // MARK: - Install / Uninstall diff --git a/TablePro/Views/Settings/Plugins/InstalledPluginsView.swift b/TablePro/Views/Settings/Plugins/InstalledPluginsView.swift index cc02a11dc..2b039f987 100644 --- a/TablePro/Views/Settings/Plugins/InstalledPluginsView.swift +++ b/TablePro/Views/Settings/Plugins/InstalledPluginsView.swift @@ -16,10 +16,11 @@ struct InstalledPluginsView: View { @State private var showErrorAlert = false @State private var errorAlertTitle = "" @State private var errorAlertMessage = "" + @State private var dismissedRestartBanner = false var body: some View { Form { - if pluginManager.needsRestart { + if pluginManager.needsRestart && !dismissedRestartBanner { Section { HStack(spacing: 8) { Image(systemName: "arrow.clockwise.circle.fill") @@ -27,6 +28,14 @@ struct InstalledPluginsView: View { Text("Restart TablePro to fully unload removed plugins.") .font(.callout) .foregroundStyle(.secondary) + Spacer() + Button { + dismissedRestartBanner = true + } label: { + Image(systemName: "xmark") + .foregroundStyle(.secondary) + } + .buttonStyle(.plain) } } }