diff --git a/CHANGELOG.md b/CHANGELOG.md index 2df79c673..69dcafa3b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- Saving on a MySQL or MariaDB server that starts sessions read-only no longer fails with "Cannot execute statement in a READ ONLY transaction". TablePro marks a transaction read-write before it writes instead of inheriting the server default. Same for PostgreSQL, CockroachDB, and Redshift. (#2009) +- Changing Safe Mode in the connection form now applies to an open connection instead of waiting for a reconnect. (#2009) +- A read-only error now says whether the database server or Safe Mode refused the write. (#2009) + ## [0.62.0] - 2026-08-02 ### Added diff --git a/Plugins/MySQLDriverPlugin/MySQLPluginDriver.swift b/Plugins/MySQLDriverPlugin/MySQLPluginDriver.swift index 4f94dd3ec..bc3d8a078 100644 --- a/Plugins/MySQLDriverPlugin/MySQLPluginDriver.swift +++ b/Plugins/MySQLDriverPlugin/MySQLPluginDriver.swift @@ -95,7 +95,11 @@ final class MySQLPluginDriver: PluginDatabaseDriver, @unchecked Sendable { // MARK: - Transaction Management func beginTransaction() async throws { - _ = try await execute(query: "START TRANSACTION") + try await beginTransaction(mode: .serverDefault) + } + + func beginTransaction(mode: PluginTransactionAccessMode) async throws { + _ = try await execute(query: mysqlBeginTransactionStatement(mode: mode)) } // MARK: - Query Execution diff --git a/Plugins/MySQLDriverPlugin/MySQLTransactionStatement.swift b/Plugins/MySQLDriverPlugin/MySQLTransactionStatement.swift new file mode 100644 index 000000000..26e567658 --- /dev/null +++ b/Plugins/MySQLDriverPlugin/MySQLTransactionStatement.swift @@ -0,0 +1,6 @@ +import Foundation +import TableProPluginKit + +internal func mysqlBeginTransactionStatement(mode: PluginTransactionAccessMode) -> String { + mode == .readWrite ? "START TRANSACTION READ WRITE" : "START TRANSACTION" +} diff --git a/Plugins/PostgreSQLDriverPlugin/LibPQDriverCore.swift b/Plugins/PostgreSQLDriverPlugin/LibPQDriverCore.swift index 1d2e47cd4..ae833912a 100644 --- a/Plugins/PostgreSQLDriverPlugin/LibPQDriverCore.swift +++ b/Plugins/PostgreSQLDriverPlugin/LibPQDriverCore.swift @@ -228,6 +228,15 @@ extension LibPQBackedDriver { var currentSchema: String? { core.currentSchema } var supportsSchemas: Bool { true } var supportsTransactions: Bool { true } + + func beginTransaction() async throws { + try await beginTransaction(mode: .serverDefault) + } + + func beginTransaction(mode: PluginTransactionAccessMode) async throws { + _ = try await execute(query: postgresBeginTransactionStatement(mode: mode)) + } + var serverVersion: String? { core.serverVersion } var parameterStyle: ParameterStyle { .dollar } diff --git a/Plugins/PostgreSQLDriverPlugin/PostgreSQLTransactionStatement.swift b/Plugins/PostgreSQLDriverPlugin/PostgreSQLTransactionStatement.swift new file mode 100644 index 000000000..b2ecc47f3 --- /dev/null +++ b/Plugins/PostgreSQLDriverPlugin/PostgreSQLTransactionStatement.swift @@ -0,0 +1,6 @@ +import Foundation +import TableProPluginKit + +internal func postgresBeginTransactionStatement(mode: PluginTransactionAccessMode) -> String { + mode == .readWrite ? "BEGIN READ WRITE" : "BEGIN" +} diff --git a/Plugins/TableProPluginKit/PluginDatabaseDriver.swift b/Plugins/TableProPluginKit/PluginDatabaseDriver.swift index 740e08bf0..0cf5fe18f 100644 --- a/Plugins/TableProPluginKit/PluginDatabaseDriver.swift +++ b/Plugins/TableProPluginKit/PluginDatabaseDriver.swift @@ -100,6 +100,7 @@ public protocol PluginDatabaseDriver: AnyObject, Sendable { var supportsTransactions: Bool { get } func beginTransaction() async throws + func beginTransaction(mode: PluginTransactionAccessMode) async throws func commitTransaction() async throws func rollbackTransaction() async throws @@ -228,6 +229,10 @@ public extension PluginDatabaseDriver { _ = try await execute(query: "BEGIN") } + func beginTransaction(mode: PluginTransactionAccessMode) async throws { + try await beginTransaction() + } + func commitTransaction() async throws { _ = try await execute(query: "COMMIT") } diff --git a/Plugins/TableProPluginKit/PluginTransactionAccessMode.swift b/Plugins/TableProPluginKit/PluginTransactionAccessMode.swift new file mode 100644 index 000000000..57eb96de2 --- /dev/null +++ b/Plugins/TableProPluginKit/PluginTransactionAccessMode.swift @@ -0,0 +1,6 @@ +import Foundation + +public enum PluginTransactionAccessMode: Sendable, Equatable { + case serverDefault + case readWrite +} diff --git a/TablePro.xcodeproj/project.pbxproj b/TablePro.xcodeproj/project.pbxproj index 50e4a4c9e..0f7b2ab8b 100644 --- a/TablePro.xcodeproj/project.pbxproj +++ b/TablePro.xcodeproj/project.pbxproj @@ -541,6 +541,7 @@ MySQLQueryTimeoutStatement.swift, MySQLSocketTimeout.swift, MySQLStatementClassification.swift, + MySQLTransactionStatement.swift, ); target = 5ABCC5A62F43856700EAF3FC /* TableProTests */; }; diff --git a/TablePro/Core/Coordinators/QueryExecutionCoordinator+Helpers.swift b/TablePro/Core/Coordinators/QueryExecutionCoordinator+Helpers.swift index e648a6f61..abcb40970 100644 --- a/TablePro/Core/Coordinators/QueryExecutionCoordinator+Helpers.swift +++ b/TablePro/Core/Coordinators/QueryExecutionCoordinator+Helpers.swift @@ -499,7 +499,7 @@ extension QueryExecutionCoordinator { ) { parent.currentQueryTask = nil parent.tabManager.mutate(tabId: tabId) { tab in - tab.execution.errorMessage = error.localizedDescription + tab.execution.errorMessage = DatabaseWriteRejectionDiagnosis.formatted(error) tab.execution.errorQuery = sql tab.execution.isExecuting = false tab.execution.lastExecutedAt = Date() diff --git a/TablePro/Core/Coordinators/QueryExecutionCoordinator+Parameters.swift b/TablePro/Core/Coordinators/QueryExecutionCoordinator+Parameters.swift index 4c9a51b84..7fd3173b7 100644 --- a/TablePro/Core/Coordinators/QueryExecutionCoordinator+Parameters.swift +++ b/TablePro/Core/Coordinators/QueryExecutionCoordinator+Parameters.swift @@ -242,9 +242,12 @@ extension QueryExecutionCoordinator { } let useTransaction = driver.supportsTransactions + let transactionKind = OperationKind.worst(of: statements, databaseType: conn.type) if useTransaction { - try await driver.beginTransaction() + try await driver.beginTransaction( + mode: transactionKind.declaresWrite ? .readWrite : .serverDefault + ) } @MainActor func rollbackAndResetState() async { diff --git a/TablePro/Core/Coordinators/RowEditingCoordinator+Discard.swift b/TablePro/Core/Coordinators/RowEditingCoordinator+Discard.swift index fb5d19958..60dcc5ebd 100644 --- a/TablePro/Core/Coordinators/RowEditingCoordinator+Discard.swift +++ b/TablePro/Core/Coordinators/RowEditingCoordinator+Discard.swift @@ -6,6 +6,7 @@ import AppKit import Foundation import os +import TableProPluginKit private let discardLogger = Logger(subsystem: "com.TablePro", category: "RowEditingCoordinator+Discard") @@ -14,12 +15,13 @@ extension RowEditingCoordinator { func executeSidebarChanges(statements: [ParameterizedStatement]) async throws { let sqlPreview = statements.map(\.sql).joined(separator: "\n") + let kind = OperationKind.from(QueryClassifier.classifyTier(sqlPreview, databaseType: parent.connection.type)) let decision = await ExecutionGateProvider.shared.authorize( OperationRequest( connectionId: parent.connectionId, databaseType: parent.connection.type, sql: sqlPreview, - kind: OperationKind.from(QueryClassifier.classifyTier(sqlPreview, databaseType: parent.connection.type)), + kind: kind, caller: .userInterface, capabilities: .interactiveUser, operationDescription: String(localized: "Save Sidebar Changes") @@ -36,7 +38,7 @@ extension RowEditingCoordinator { let useTransaction = driver.supportsTransactions if useTransaction { - try await driver.beginTransaction() + try await driver.beginTransaction(mode: kind.declaresWrite ? .readWrite : .serverDefault) } do { diff --git a/TablePro/Core/Coordinators/RowEditingCoordinator+SaveChanges.swift b/TablePro/Core/Coordinators/RowEditingCoordinator+SaveChanges.swift index 090dd8d6d..d07bf6a49 100644 --- a/TablePro/Core/Coordinators/RowEditingCoordinator+SaveChanges.swift +++ b/TablePro/Core/Coordinators/RowEditingCoordinator+SaveChanges.swift @@ -6,6 +6,7 @@ import Foundation import os import SwiftUI +import TableProPluginKit private let saveChangesLogger = Logger(subsystem: "com.TablePro", category: "RowEditingCoordinator") @@ -187,7 +188,7 @@ extension RowEditingCoordinator { let useTransaction = driver.supportsTransactions if useTransaction { - try await driver.beginTransaction() + try await driver.beginTransaction(mode: .readWrite) } do { @@ -292,15 +293,21 @@ extension RowEditingCoordinator { errorMessage: error.localizedDescription ) + let diagnosis = DatabaseWriteRejectionDiagnosis.classify(error) + if let index = parent.tabManager.selectedTabIndex { parent.tabManager.mutate(at: index) { - $0.execution.errorMessage = String(format: String(localized: "Save failed: %@"), error.localizedDescription) + $0.execution.errorMessage = String( + format: String(localized: "Save failed: %@"), + DatabaseWriteRejectionDiagnosis.formatted(error) + ) } } AlertHelper.showErrorSheet( title: String(localized: "Save Failed"), - message: error.localizedDescription, + message: diagnosis?.errorDescription ?? error.localizedDescription, + recoverySuggestion: diagnosis?.recoverySuggestion, window: parent.contentWindow ) diff --git a/TablePro/Core/Database/DatabaseDriver.swift b/TablePro/Core/Database/DatabaseDriver.swift index 3ec2e705e..43e1f8f8f 100644 --- a/TablePro/Core/Database/DatabaseDriver.swift +++ b/TablePro/Core/Database/DatabaseDriver.swift @@ -190,6 +190,8 @@ protocol DatabaseDriver: AnyObject, Sendable { /// Begin a transaction func beginTransaction() async throws + func beginTransaction(mode: PluginTransactionAccessMode) async throws + /// Commit the current transaction func commitTransaction() async throws @@ -236,6 +238,10 @@ extension DatabaseDriver { var queryBuildingPluginDriver: (any PluginDatabaseDriver)? { nil } + func beginTransaction(mode: PluginTransactionAccessMode) async throws { + try await beginTransaction() + } + func quoteIdentifier(_ name: String) -> String { SQLEscaping.quoteIdentifier(name) } diff --git a/TablePro/Core/Database/DatabaseManager+Principals.swift b/TablePro/Core/Database/DatabaseManager+Principals.swift index 882b6a1ef..3d1cbde8e 100644 --- a/TablePro/Core/Database/DatabaseManager+Principals.swift +++ b/TablePro/Core/Database/DatabaseManager+Principals.swift @@ -68,7 +68,7 @@ extension DatabaseManager { ) async throws { let useTransaction = driver.supportsTransactions && rollsBack if useTransaction { - try await driver.beginTransaction() + try await driver.beginTransaction(mode: .readWrite) } var appliedCount = 0 diff --git a/TablePro/Core/Database/DatabaseManager+Schema.swift b/TablePro/Core/Database/DatabaseManager+Schema.swift index d563fa104..1853bd28f 100644 --- a/TablePro/Core/Database/DatabaseManager+Schema.swift +++ b/TablePro/Core/Database/DatabaseManager+Schema.swift @@ -85,7 +85,7 @@ extension DatabaseManager { let useTransaction = driver.supportsTransactions if useTransaction { - try await driver.beginTransaction() + try await driver.beginTransaction(mode: schemaKind.declaresWrite ? .readWrite : .serverDefault) } do { diff --git a/TablePro/Core/Database/DatabaseManager+Sessions.swift b/TablePro/Core/Database/DatabaseManager+Sessions.swift index b0fc3ad9c..821cac485 100644 --- a/TablePro/Core/Database/DatabaseManager+Sessions.swift +++ b/TablePro/Core/Database/DatabaseManager+Sessions.swift @@ -417,6 +417,23 @@ extension DatabaseManager { setSession(session, for: sessionId) } + func observeConnectionUpdates() { + connectionUpdatedCancellable = AppEvents.shared.connectionUpdated + .receive(on: RunLoop.main) + .sink { [weak self] connectionId in + self?.reconcileSafeModeLevel(for: connectionId) + } + } + + func reconcileSafeModeLevel(for connectionId: UUID?) { + let targetIds = connectionId.map { [$0] } ?? Array(activeSessions.keys) + for id in targetIds { + guard activeSessions[id] != nil, + let stored = connectionStorage.loadConnection(id: id) else { continue } + setSafeModeLevel(stored.safeModeLevel, for: id) + } + } + func setSafeModeLevel(_ level: SafeModeLevel, for connectionId: UUID) { guard var session = activeSessions[connectionId] else { return } guard session.safeModeLevel != level || session.connection.safeModeLevel != level else { return } diff --git a/TablePro/Core/Database/DatabaseManager.swift b/TablePro/Core/Database/DatabaseManager.swift index 83263bdc8..2149a1334 100644 --- a/TablePro/Core/Database/DatabaseManager.swift +++ b/TablePro/Core/Database/DatabaseManager.swift @@ -5,6 +5,7 @@ // Created by Ngo Quoc Dat on 16/12/25. // +import Combine import Foundation import Observation import os @@ -58,6 +59,8 @@ final class DatabaseManager { /// and the wake-from-sleep handler fire for the same connection. @ObservationIgnored internal var recoveringConnectionIds = Set() + @ObservationIgnored internal var connectionUpdatedCancellable: AnyCancellable? + @ObservationIgnored internal let ensureConnectedDedup = OnceTask() /// Generation token per connection. A cancelled or superseded attempt keeps running @@ -119,5 +122,6 @@ final class DatabaseManager { self.connectionStorage = connectionStorage self.appSettingsStorage = appSettingsStorage self.pluginManager = pluginManager + observeConnectionUpdates() } } diff --git a/TablePro/Core/Database/DatabaseWriteRejectionDiagnosis.swift b/TablePro/Core/Database/DatabaseWriteRejectionDiagnosis.swift new file mode 100644 index 000000000..6d48b5b81 --- /dev/null +++ b/TablePro/Core/Database/DatabaseWriteRejectionDiagnosis.swift @@ -0,0 +1,60 @@ +// +// DatabaseWriteRejectionDiagnosis.swift +// TablePro +// + +import Foundation +import TableProPluginKit + +internal enum DatabaseWriteRejectionDiagnosis: LocalizedError, Equatable { + case serverEnforcedReadOnly(serverMessage: String) + + private static let readOnlyTransactionSqlState = "25006" + private static let mysqlReadOnlyServerCodes: Set = [1_290, 1_836, 1_874] + + static func classify(_ error: Error) -> DatabaseWriteRejectionDiagnosis? { + guard let driverError = error as? any PluginDriverError else { return nil } + + if driverError.pluginSqlState == readOnlyTransactionSqlState { + return .serverEnforcedReadOnly(serverMessage: driverError.pluginErrorMessage) + } + + if let code = driverError.pluginErrorCode, mysqlReadOnlyServerCodes.contains(code) { + return .serverEnforcedReadOnly(serverMessage: driverError.pluginErrorMessage) + } + + return nil + } + + static func formatted(_ error: Error) -> String { + guard let diagnosis = classify(error) else { return error.localizedDescription } + return [diagnosis.errorDescription, diagnosis.recoverySuggestion] + .compactMap { $0 } + .joined(separator: "\n\n") + } + + var serverMessage: String { + switch self { + case .serverEnforcedReadOnly(let message): return message + } + } + + var errorDescription: String? { + String(localized: "The database server rejected this write because the connection is read-only.") + } + + var recoverySuggestion: String? { + String( + format: String( + localized: """ + The database server enforced this, not TablePro's Safe Mode. \ + You are most likely connected to a read replica, or the server sets new transactions to read-only. \ + Connect to the primary server to write. + + Server response: %@ + """ + ), + serverMessage + ) + } +} diff --git a/TablePro/Core/Database/TriggerEditing.swift b/TablePro/Core/Database/TriggerEditing.swift index bafdce9c6..a89c7d53a 100644 --- a/TablePro/Core/Database/TriggerEditing.swift +++ b/TablePro/Core/Database/TriggerEditing.swift @@ -9,6 +9,7 @@ import Combine import Foundation import os +import TableProPluginKit enum TriggerEditingError: LocalizedError { case notConnected @@ -121,7 +122,7 @@ enum TriggerEditing { } static func runInTransaction(driver: DatabaseDriver, dropSQL: String?, sql: String) async throws { - try await driver.beginTransaction() + try await driver.beginTransaction(mode: .readWrite) do { if let dropSQL { _ = try await driver.execute(query: dropSQL) } _ = try await driver.execute(query: sql) diff --git a/TablePro/Core/Plugins/ImportDataSinkAdapter.swift b/TablePro/Core/Plugins/ImportDataSinkAdapter.swift index 5f087d70c..20f6b3850 100644 --- a/TablePro/Core/Plugins/ImportDataSinkAdapter.swift +++ b/TablePro/Core/Plugins/ImportDataSinkAdapter.swift @@ -129,7 +129,7 @@ final class ImportDataSinkAdapter: PluginImportDataSink, @unchecked Sendable { } func beginTransaction() async throws { - try await driver.beginTransaction() + try await driver.beginTransaction(mode: .readWrite) } func commitTransaction() async throws { diff --git a/TablePro/Core/Plugins/PluginDriverAdapter.swift b/TablePro/Core/Plugins/PluginDriverAdapter.swift index 8da643942..7feffeee0 100644 --- a/TablePro/Core/Plugins/PluginDriverAdapter.swift +++ b/TablePro/Core/Plugins/PluginDriverAdapter.swift @@ -516,6 +516,10 @@ final class PluginDriverAdapter: DatabaseDriver, SchemaSwitchable { try await pluginDriver.beginTransaction() } + func beginTransaction(mode: PluginTransactionAccessMode) async throws { + try await pluginDriver.beginTransaction(mode: mode) + } + func commitTransaction() async throws { try await pluginDriver.commitTransaction() } diff --git a/TablePro/Core/Services/Execution/DefaultExecutionGate.swift b/TablePro/Core/Services/Execution/DefaultExecutionGate.swift index a7a7c5c7c..bbbf2e33b 100644 --- a/TablePro/Core/Services/Execution/DefaultExecutionGate.swift +++ b/TablePro/Core/Services/Execution/DefaultExecutionGate.swift @@ -45,7 +45,9 @@ internal actor DefaultExecutionGate: ExecutionGate { } if level.blocksAllWrites, effectiveWrite { - return .denied(reason: String(localized: "Cannot execute write queries: connection is read-only")) + return .denied(reason: String( + localized: "Cannot execute write queries: TablePro's Safe Mode is set to read-only for this connection" + )) } let isMetadataRead = request.kind == .metadataRead diff --git a/TablePro/Core/Services/Execution/OperationKind.swift b/TablePro/Core/Services/Execution/OperationKind.swift index 22b2d6efb..54b6e4285 100644 --- a/TablePro/Core/Services/Execution/OperationKind.swift +++ b/TablePro/Core/Services/Execution/OperationKind.swift @@ -4,6 +4,7 @@ // import Foundation +import TableProPluginKit internal enum OperationKind: Sendable, Equatable { case readQuery @@ -29,6 +30,10 @@ internal extension OperationKind { self == .destructiveQuery } + var transactionAccessMode: PluginTransactionAccessMode { + declaresWrite ? .readWrite : .serverDefault + } + static func from(_ tier: QueryTier) -> OperationKind { switch tier { case .safe: return .readQuery diff --git a/TablePro/Core/Utilities/UI/AlertHelper.swift b/TablePro/Core/Utilities/UI/AlertHelper.swift index 6ba2fa9b9..12d3bcbc2 100644 --- a/TablePro/Core/Utilities/UI/AlertHelper.swift +++ b/TablePro/Core/Utilities/UI/AlertHelper.swift @@ -206,11 +206,14 @@ final class AlertHelper { static func showErrorSheet( title: String, message: String, + recoverySuggestion: String? = nil, window: NSWindow? ) { let alert = NSAlert() alert.messageText = title - alert.informativeText = message + alert.informativeText = [message, recoverySuggestion] + .compactMap { $0 } + .joined(separator: "\n\n") alert.alertStyle = .critical alert.addButton(withTitle: String(localized: "OK")) diff --git a/TablePro/ViewModels/AIChatViewModel+ToolApproval.swift b/TablePro/ViewModels/AIChatViewModel+ToolApproval.swift index 355cc7b02..5bd499ba2 100644 --- a/TablePro/ViewModels/AIChatViewModel+ToolApproval.swift +++ b/TablePro/ViewModels/AIChatViewModel+ToolApproval.swift @@ -95,7 +95,7 @@ extension AIChatViewModel { if toolMode == .agentOnly { if let connection, liveSafeModeLevel(for: connection).blocksAllWrites { return .denied(reason: String( - localized: "Connection is read-only. Destructive operations are not permitted." + localized: "TablePro's Safe Mode is set to read-only for this connection. Destructive operations are not permitted." )) } return .pending @@ -108,7 +108,7 @@ extension AIChatViewModel { let safeModeLevel = liveSafeModeLevel(for: connection) if safeModeLevel.blocksAllWrites { return .denied(reason: String( - localized: "Connection is read-only. Set safe mode to Confirm Writes or higher to allow this tool." + localized: "TablePro's Safe Mode is set to read-only for this connection. Set it to Confirm Writes or higher to allow this tool." )) } if !safeModeLevel.requiresConfirmation { diff --git a/TablePro/Views/Structure/TableStructureView+Schema.swift b/TablePro/Views/Structure/TableStructureView+Schema.swift index 6da5274da..e80efb18b 100644 --- a/TablePro/Views/Structure/TableStructureView+Schema.swift +++ b/TablePro/Views/Structure/TableStructureView+Schema.swift @@ -53,10 +53,13 @@ extension TableStructureView { } func executeSchemaChanges() async { - guard !connection.safeModeLevel.blocksAllWrites else { + let liveSafeModeLevel = coordinator?.safeModeLevel ?? connection.safeModeLevel + guard !liveSafeModeLevel.blocksAllWrites else { AlertHelper.showErrorSheet( - title: String(localized: "Read Only Connection"), - message: String(localized: "Cannot save schema changes: connection is read only."), + title: String(localized: "Safe Mode Is Read-Only"), + message: String( + localized: "Cannot save schema changes: TablePro's Safe Mode is set to read-only for this connection." + ), window: coordinator?.contentWindow ) return diff --git a/TableProTests/Core/Database/DatabaseWriteRejectionDiagnosisTests.swift b/TableProTests/Core/Database/DatabaseWriteRejectionDiagnosisTests.swift new file mode 100644 index 000000000..68f1a8b2b --- /dev/null +++ b/TableProTests/Core/Database/DatabaseWriteRejectionDiagnosisTests.swift @@ -0,0 +1,108 @@ +// +// DatabaseWriteRejectionDiagnosisTests.swift +// TableProTests +// + +import Foundation +@testable import TablePro +import TableProPluginKit +import Testing + +private struct FakeDriverError: PluginDriverError { + let pluginErrorMessage: String + let pluginErrorCode: Int? + let pluginSqlState: String? +} + +private struct PlainError: Error, LocalizedError { + var errorDescription: String? { "Something else went wrong" } +} + +@Suite("DatabaseWriteRejectionDiagnosis") +struct DatabaseWriteRejectionDiagnosisTests { + @Test("MySQL 1792 in a read-only transaction is recognised by its portable SQLSTATE") + func recognisesMySQLReadOnlyTransaction() { + let error = FakeDriverError( + pluginErrorMessage: "Cannot execute statement in a READ ONLY transaction.", + pluginErrorCode: 1_792, + pluginSqlState: "25006" + ) + + #expect(DatabaseWriteRejectionDiagnosis.classify(error) != nil) + } + + @Test("PostgreSQL uses the same SQLSTATE and is recognised without a native code") + func recognisesPostgresReadOnlyTransaction() { + let error = FakeDriverError( + pluginErrorMessage: "cannot execute UPDATE in a read-only transaction", + pluginErrorCode: nil, + pluginSqlState: "25006" + ) + + #expect(DatabaseWriteRejectionDiagnosis.classify(error) != nil) + } + + @Test("Server-level read-only codes report HY000 and are matched on the native code") + func recognisesServerLevelReadOnlyCodes() { + for code in [1_290, 1_836, 1_874] { + let error = FakeDriverError( + pluginErrorMessage: "The MySQL server is running with the --read-only option", + pluginErrorCode: code, + pluginSqlState: "HY000" + ) + + #expect(DatabaseWriteRejectionDiagnosis.classify(error) != nil) + } + } + + @Test("An unrelated driver error is not diagnosed as read-only") + func ignoresUnrelatedDriverError() { + let error = FakeDriverError( + pluginErrorMessage: "You have an error in your SQL syntax", + pluginErrorCode: 1_064, + pluginSqlState: "42000" + ) + + #expect(DatabaseWriteRejectionDiagnosis.classify(error) == nil) + } + + @Test("An error that is not a driver error is not diagnosed") + func ignoresNonDriverError() { + #expect(DatabaseWriteRejectionDiagnosis.classify(PlainError()) == nil) + } + + @Test("The recovery suggestion blames the server and clears TablePro's Safe Mode") + func recoverySuggestionNamesTheServer() throws { + let error = FakeDriverError( + pluginErrorMessage: "Cannot execute statement in a READ ONLY transaction.", + pluginErrorCode: 1_792, + pluginSqlState: "25006" + ) + + let diagnosis = try #require(DatabaseWriteRejectionDiagnosis.classify(error)) + let suggestion = try #require(diagnosis.recoverySuggestion) + + #expect(suggestion.contains("Safe Mode")) + #expect(suggestion.contains("Cannot execute statement in a READ ONLY transaction.")) + #expect(diagnosis.errorDescription?.isEmpty == false) + } + + @Test("Formatting a read-only rejection replaces the raw driver string") + func formattedReplacesRawDriverString() { + let error = FakeDriverError( + pluginErrorMessage: "Cannot execute statement in a READ ONLY transaction.", + pluginErrorCode: 1_792, + pluginSqlState: "25006" + ) + + let formatted = DatabaseWriteRejectionDiagnosis.formatted(error) + + #expect(formatted.contains("Safe Mode")) + #expect(!formatted.hasPrefix("[1792]")) + } + + @Test("Formatting any other error falls back to its own description") + func formattedFallsBackForOtherErrors() { + #expect(DatabaseWriteRejectionDiagnosis.formatted(PlainError()) == "Something else went wrong") + } +} diff --git a/TableProTests/Core/Plugins/PluginDriverAdapterTransactionTests.swift b/TableProTests/Core/Plugins/PluginDriverAdapterTransactionTests.swift new file mode 100644 index 000000000..60faa338e --- /dev/null +++ b/TableProTests/Core/Plugins/PluginDriverAdapterTransactionTests.swift @@ -0,0 +1,80 @@ +// +// PluginDriverAdapterTransactionTests.swift +// TableProTests +// + +import Foundation +@testable import TablePro +import TableProPluginKit +import Testing + +private class TransactionDriverBase: @unchecked Sendable { + private(set) var executedQueries: [String] = [] + + func connect() async throws {} + func disconnect() {} + + func execute(query: String) async throws -> PluginQueryResult { + executedQueries.append(query) + return PluginQueryResult(columns: [], columnTypeNames: [], rows: [], rowsAffected: 0, executionTime: 0) + } + + func fetchTables(schema: String?) async throws -> [PluginTableInfo] { [] } + func fetchColumns(table: String, schema: String?) async throws -> [PluginColumnInfo] { [] } + func fetchIndexes(table: String, schema: String?) async throws -> [PluginIndexInfo] { [] } + func fetchForeignKeys(table: String, schema: String?) async throws -> [PluginForeignKeyInfo] { [] } + func fetchTableDDL(table: String, schema: String?) async throws -> String { "" } + func fetchViewDefinition(view: String, schema: String?) async throws -> String { "" } + func fetchTableMetadata(table: String, schema: String?) async throws -> PluginTableMetadata { + PluginTableMetadata(tableName: table) + } + func fetchDatabases() async throws -> [String] { [] } + func fetchDatabaseMetadata(_ database: String) async throws -> PluginDatabaseMetadata { + PluginDatabaseMetadata(name: database) + } +} + +private final class LegacyTransactionDriver: TransactionDriverBase, PluginDatabaseDriver, @unchecked Sendable {} + +private final class ModeAwareTransactionDriver: TransactionDriverBase, PluginDatabaseDriver, @unchecked Sendable { + private(set) var receivedModes: [PluginTransactionAccessMode] = [] + + func beginTransaction(mode: PluginTransactionAccessMode) async throws { + receivedModes.append(mode) + } +} + +@Suite("PluginDriverAdapter transaction access mode") +struct PluginDriverAdapterTransactionTests { + private func connection() -> DatabaseConnection { + DatabaseConnection( + name: "Transaction Test", + host: "127.0.0.1", + port: 3_306, + database: "test", + username: "root", + type: .mysql + ) + } + + @Test("The adapter forwards the requested access mode to the plugin driver") + func forwardsAccessMode() async throws { + let driver = ModeAwareTransactionDriver() + let adapter = PluginDriverAdapter(connection: connection(), pluginDriver: driver) + + try await adapter.beginTransaction(mode: .readWrite) + + #expect(driver.receivedModes == [.readWrite]) + #expect(driver.executedQueries.isEmpty) + } + + @Test("A plugin built before the access mode existed still opens a transaction") + func fallsBackForDriversWithoutAccessModeSupport() async throws { + let driver = LegacyTransactionDriver() + let adapter = PluginDriverAdapter(connection: connection(), pluginDriver: driver) + + try await adapter.beginTransaction(mode: .readWrite) + + #expect(driver.executedQueries == ["BEGIN"]) + } +} diff --git a/TableProTests/Core/Services/Execution/TransactionAccessModePolicyTests.swift b/TableProTests/Core/Services/Execution/TransactionAccessModePolicyTests.swift new file mode 100644 index 000000000..e17771622 --- /dev/null +++ b/TableProTests/Core/Services/Execution/TransactionAccessModePolicyTests.swift @@ -0,0 +1,42 @@ +// +// TransactionAccessModePolicyTests.swift +// TableProTests +// + +@testable import TablePro +import TableProPluginKit +import Testing + +@Suite("Transaction access mode policy") +struct TransactionAccessModePolicyTests { + @Test("Read operations never declare write intent") + func readOperationsInheritServerDefault() { + #expect(OperationKind.readQuery.transactionAccessMode == .serverDefault) + #expect(OperationKind.metadataRead.transactionAccessMode == .serverDefault) + } + + @Test("Every write operation declares write intent") + func writeOperationsDeclareReadWrite() { + #expect(OperationKind.writeQuery.transactionAccessMode == .readWrite) + #expect(OperationKind.destructiveQuery.transactionAccessMode == .readWrite) + #expect(OperationKind.schemaMutation.transactionAccessMode == .readWrite) + #expect(OperationKind.importData.transactionAccessMode == .readWrite) + #expect(OperationKind.maintenance.transactionAccessMode == .readWrite) + } + + @Test("A script of only reads keeps the server default so replica browsing still works") + func readOnlyScriptKeepsServerDefault() { + let statements = ["SELECT * FROM users", "SELECT count(*) FROM orders"] + let kind = OperationKind.worst(of: statements, databaseType: .mysql) + + #expect(kind.transactionAccessMode == .serverDefault) + } + + @Test("A script containing one write declares write intent for the whole transaction") + func mixedScriptDeclaresReadWrite() { + let statements = ["SELECT * FROM users", "UPDATE users SET name = 'x' WHERE id = 1"] + let kind = OperationKind.worst(of: statements, databaseType: .mysql) + + #expect(kind.transactionAccessMode == .readWrite) + } +} diff --git a/TableProTests/Core/Storage/SafeModeMigrationTests.swift b/TableProTests/Core/Storage/SafeModeMigrationTests.swift index 15c04da5e..e7e8f2923 100644 --- a/TableProTests/Core/Storage/SafeModeMigrationTests.swift +++ b/TableProTests/Core/Storage/SafeModeMigrationTests.swift @@ -5,6 +5,7 @@ // Tests for safeModeLevel persistence and migration from old isReadOnly format. // +import Combine import Foundation @testable import TablePro import TableProPluginKit @@ -304,6 +305,110 @@ struct SafeModeMigrationTests { #expect(!tracker.dirtyRecords(for: .connection).contains(id.uuidString)) } + // MARK: - Live Session Reconciliation + + private func makeConnection(id: UUID, name: String, safeModeLevel: SafeModeLevel) -> DatabaseConnection { + DatabaseConnection( + id: id, + name: name, + host: "127.0.0.1", + port: 3_306, + database: "test", + username: "root", + type: .mysql, + safeModeLevel: safeModeLevel + ) + } + + @Test("An edit saved from the connection form reaches the open session") + func reconcileAppliesConnectionFormEditToLiveSession() { + let id = UUID() + let connection = makeConnection(id: id, name: "Form Edit", safeModeLevel: .readOnly) + storage.addConnection(connection) + + let manager = DatabaseManager(connectionStorage: storage) + manager.injectSession(ConnectionSession(connection: connection), for: id) + defer { manager.removeSession(for: id) } + + var edited = connection + edited.safeModeLevel = .safeMode + storage.updateConnection(edited) + + #expect(manager.session(for: id)?.safeModeLevel == .readOnly) + + manager.reconcileSafeModeLevel(for: id) + + #expect(manager.session(for: id)?.safeModeLevel == .safeMode) + #expect(manager.session(for: id)?.connection.safeModeLevel == .safeMode) + } + + @Test("A bulk update with no connection id reconciles every open session") + func reconcileWithoutIdCoversEveryActiveSession() { + let firstId = UUID() + let secondId = UUID() + let first = makeConnection(id: firstId, name: "First", safeModeLevel: .readOnly) + let second = makeConnection(id: secondId, name: "Second", safeModeLevel: .readOnly) + storage.addConnection(first) + storage.addConnection(second) + + let manager = DatabaseManager(connectionStorage: storage) + manager.injectSession(ConnectionSession(connection: first), for: firstId) + manager.injectSession(ConnectionSession(connection: second), for: secondId) + defer { + manager.removeSession(for: firstId) + manager.removeSession(for: secondId) + } + + var editedFirst = first + editedFirst.safeModeLevel = .silent + var editedSecond = second + editedSecond.safeModeLevel = .alert + storage.updateConnection(editedFirst) + storage.updateConnection(editedSecond) + + manager.reconcileSafeModeLevel(for: nil) + + #expect(manager.session(for: firstId)?.safeModeLevel == .silent) + #expect(manager.session(for: secondId)?.safeModeLevel == .alert) + } + + @Test("Reconciling a connection with no open session changes nothing") + func reconcileIgnoresConnectionsWithoutSession() { + let id = UUID() + let connection = makeConnection(id: id, name: "Not Connected", safeModeLevel: .readOnly) + storage.addConnection(connection) + + let manager = DatabaseManager(connectionStorage: storage) + manager.reconcileSafeModeLevel(for: id) + + #expect(manager.session(for: id) == nil) + #expect(storage.loadConnection(id: id)?.safeModeLevel == .readOnly) + } + + @Test("A connectionUpdated event reconciles the open session") + func connectionUpdatedEventReconcilesLiveSession() async { + let id = UUID() + let connection = makeConnection(id: id, name: "Event Driven", safeModeLevel: .readOnly) + storage.addConnection(connection) + + let manager = DatabaseManager(connectionStorage: storage) + manager.injectSession(ConnectionSession(connection: connection), for: id) + defer { manager.removeSession(for: id) } + + var edited = connection + edited.safeModeLevel = .silent + storage.updateConnection(edited) + + AppEvents.shared.connectionUpdated.send(id) + + let deadline = Date().addingTimeInterval(2) + while Date() < deadline, manager.session(for: id)?.safeModeLevel != .silent { + try? await Task.sleep(nanoseconds: 5_000_000) + } + + #expect(manager.session(for: id)?.safeModeLevel == .silent) + } + // MARK: - Default Level @Test("New connection defaults to silent safe mode level") diff --git a/TableProTests/PluginTestSources/PostgreSQLTransactionStatement.swift b/TableProTests/PluginTestSources/PostgreSQLTransactionStatement.swift new file mode 120000 index 000000000..8e4693e7f --- /dev/null +++ b/TableProTests/PluginTestSources/PostgreSQLTransactionStatement.swift @@ -0,0 +1 @@ +../../Plugins/PostgreSQLDriverPlugin/PostgreSQLTransactionStatement.swift \ No newline at end of file diff --git a/TableProTests/Plugins/MySQLTransactionStatementTests.swift b/TableProTests/Plugins/MySQLTransactionStatementTests.swift new file mode 100644 index 000000000..1050b7155 --- /dev/null +++ b/TableProTests/Plugins/MySQLTransactionStatementTests.swift @@ -0,0 +1,20 @@ +// +// MySQLTransactionStatementTests.swift +// TableProTests +// + +import TableProPluginKit +import Testing + +@Suite("MySQL Begin Transaction Statement") +struct MySQLTransactionStatementTests { + @Test("A read-write transaction declares the access mode so a read-only session default is overridden") + func readWriteDeclaresAccessMode() { + #expect(mysqlBeginTransactionStatement(mode: .readWrite) == "START TRANSACTION READ WRITE") + } + + @Test("A server-default transaction inherits the session access mode") + func serverDefaultInheritsSessionMode() { + #expect(mysqlBeginTransactionStatement(mode: .serverDefault) == "START TRANSACTION") + } +} diff --git a/TableProTests/Plugins/PostgreSQLTransactionStatementTests.swift b/TableProTests/Plugins/PostgreSQLTransactionStatementTests.swift new file mode 100644 index 000000000..801f491a2 --- /dev/null +++ b/TableProTests/Plugins/PostgreSQLTransactionStatementTests.swift @@ -0,0 +1,20 @@ +// +// PostgreSQLTransactionStatementTests.swift +// TableProTests +// + +import TableProPluginKit +import Testing + +@Suite("PostgreSQL Begin Transaction Statement") +struct PostgreSQLTransactionStatementTests { + @Test("A read-write transaction declares the access mode so a read-only session default is overridden") + func readWriteDeclaresAccessMode() { + #expect(postgresBeginTransactionStatement(mode: .readWrite) == "BEGIN READ WRITE") + } + + @Test("A server-default transaction inherits the session access mode") + func serverDefaultInheritsSessionMode() { + #expect(postgresBeginTransactionStatement(mode: .serverDefault) == "BEGIN") + } +} diff --git a/docs/features/safe-mode.mdx b/docs/features/safe-mode.mdx index 2870239d0..9934209aa 100644 --- a/docs/features/safe-mode.mdx +++ b/docs/features/safe-mode.mdx @@ -68,8 +68,31 @@ The current Safe Mode level appears as a badge in the toolbar (orange for Alert Changing the level from the badge applies to the whole connection and stays set as you open other tables and tabs. It writes back to the saved connection, so the Customization pane shows the new level and the change reaches your other Macs when iCloud Sync is on. There is no session-only override. +Editing the level in the connection form works the same way round: the change reaches an open connection right away, so the badge and the form always agree. + Safe Mode gates apply to query execution, saving cell edits, table operations, and sidebar changes. +## Server Read-Only Is Not Safe Mode + +Safe Mode runs inside TablePro. It never changes anything on the database server, and it cannot make the server accept a write the server itself refuses. + +If a save fails with a read-only error while Safe Mode is set to anything other than Read-Only, the database server is the one refusing the write. Common causes: + +- You are connected to a read replica or a reader endpoint rather than the primary. +- The server runs with `read_only` or `super_read_only` turned on. +- The server or the session sets new transactions to read-only. + +TablePro tells the transactions it opens for a write that they are read-write, so a server that only defaults new transactions to read-only accepts the save. A server that is genuinely read-only still refuses, and TablePro says the server refused it. + +On MySQL and MariaDB you can check which one it is: + +```sql +SHOW SESSION VARIABLES WHERE Variable_name IN + ('transaction_read_only', 'tx_read_only', 'read_only', 'super_read_only', 'innodb_read_only'); +``` + +`innodb_read_only` set to `ON` means you are on a replica. Connect to the primary to write. + ## External Clients Safe Mode runs inside the app on every query you execute. External clients (Raycast, Cursor, Claude Desktop, and other MCP clients) hit a separate gate first.