Merge pull request #220 from datlechin/refactor/transaction-delegation

datlechin · web-flow · commit eec73feb2a6d · 2026-03-09T13:12:31.000+07:00
refactor: delegate transaction handling to plugin drivers
diff --git a/Plugins/ClickHouseDriverPlugin/ClickHousePlugin.swift b/Plugins/ClickHouseDriverPlugin/ClickHousePlugin.swift
@@ -64,6 +64,9 @@ final class ClickHousePluginDriver: PluginDatabaseDriver, @unchecked Sendable {
     var serverVersion: String? { _serverVersion }
     var supportsSchemas: Bool { false }
     var supportsTransactions: Bool { false }
+    func beginTransaction() async throws {}
+    func commitTransaction() async throws {}
+    func rollbackTransaction() async throws {}
     var currentSchema: String? { nil }
 
     init(config: DriverConnectionConfig) {
diff --git a/Plugins/MSSQLDriverPlugin/MSSQLPlugin.swift b/Plugins/MSSQLDriverPlugin/MSSQLPlugin.swift
@@ -374,6 +374,12 @@ final class MSSQLPluginDriver: PluginDatabaseDriver, @unchecked Sendable {
         _ = try await execute(query: "SELECT 1")
     }
 
+    // MARK: - Transaction Management
+
+    func beginTransaction() async throws {
+        _ = try await execute(query: "BEGIN TRANSACTION")
+    }
+
     // MARK: - Query Execution
 
     func execute(query: String) async throws -> PluginQueryResult {
diff --git a/Plugins/MongoDBDriverPlugin/MongoDBPluginDriver.swift b/Plugins/MongoDBDriverPlugin/MongoDBPluginDriver.swift
@@ -18,6 +18,10 @@ final class MongoDBPluginDriver: PluginDatabaseDriver {
 
     var serverVersion: String? { mongoConnection?.serverVersion() }
     var currentSchema: String? { nil }
+    var supportsTransactions: Bool { false }
+    func beginTransaction() async throws {}
+    func commitTransaction() async throws {}
+    func rollbackTransaction() async throws {}
 
     init(config: DriverConnectionConfig) {
         self.config = config
diff --git a/Plugins/MySQLDriverPlugin/MySQLPluginDriver.swift b/Plugins/MySQLDriverPlugin/MySQLPluginDriver.swift
@@ -66,6 +66,12 @@ final class MySQLPluginDriver: PluginDatabaseDriver, @unchecked Sendable {
         _ = try await execute(query: "SELECT 1")
     }
 
+    // MARK: - Transaction Management
+
+    func beginTransaction() async throws {
+        _ = try await execute(query: "START TRANSACTION")
+    }
+
     // MARK: - Query Execution
 
     func execute(query: String) async throws -> PluginQueryResult {
diff --git a/Plugins/OracleDriverPlugin/OraclePlugin.swift b/Plugins/OracleDriverPlugin/OraclePlugin.swift
@@ -80,6 +80,12 @@ final class OraclePluginDriver: PluginDatabaseDriver, @unchecked Sendable {
         _ = try await execute(query: "SELECT 1 FROM DUAL")
     }
 
+    // MARK: - Transaction Management
+
+    func beginTransaction() async throws {
+        // Oracle uses implicit transactions — no explicit BEGIN needed
+    }
+
     // MARK: - Query Execution
 
     func execute(query: String) async throws -> PluginQueryResult {
diff --git a/TablePro/Core/Database/DatabaseDriver.swift b/TablePro/Core/Database/DatabaseDriver.swift
@@ -290,23 +290,6 @@ extension DatabaseDriver {
                 .warning("Failed to set query timeout: \(error.localizedDescription)")
         }
     }
-
-    // MARK: - Default Transaction Implementation
-
-    /// Default transaction implementation using database-specific SQL
-    func beginTransaction() async throws {
-        let sql = connection.type.beginTransactionSQL
-        guard !sql.isEmpty else { return }
-        _ = try await execute(query: sql)
-    }
-
-    func commitTransaction() async throws {
-        _ = try await execute(query: "COMMIT")
-    }
-
-    func rollbackTransaction() async throws {
-        _ = try await execute(query: "ROLLBACK")
-    }
 }
 
 /// Factory for creating database drivers via plugin lookup
diff --git a/TablePro/Core/Plugins/PluginDriverAdapter.swift b/TablePro/Core/Plugins/PluginDriverAdapter.swift
@@ -239,15 +239,15 @@ final class PluginDriverAdapter: DatabaseDriver, SchemaSwitchable {
     // MARK: - Transaction Management
 
     func beginTransaction() async throws {
-        _ = try await pluginDriver.execute(query: "BEGIN")
+        try await pluginDriver.beginTransaction()
     }
 
     func commitTransaction() async throws {
-        _ = try await pluginDriver.execute(query: "COMMIT")
+        try await pluginDriver.commitTransaction()
     }
 
     func rollbackTransaction() async throws {
-        _ = try await pluginDriver.execute(query: "ROLLBACK")
+        try await pluginDriver.rollbackTransaction()
     }
 
     // MARK: - Schema Switching
diff --git a/TablePro/Core/Services/Export/ImportService.swift b/TablePro/Core/Services/Export/ImportService.swift
@@ -133,10 +133,7 @@ final class ImportService {
 
             // 5. Begin transaction (if enabled)
             if config.wrapInTransaction {
-                let beginStmt = connection.type.beginTransactionSQL
-                if !beginStmt.isEmpty {
-                    _ = try await driver.execute(query: beginStmt)
-                }
+                try await driver.beginTransaction()
             }
 
             // 6. Parse and execute statements (single pass — no prior counting pass)
@@ -174,10 +171,7 @@ final class ImportService {
 
             // 7. Commit transaction (if enabled)
             if config.wrapInTransaction {
-                let commitStmt = commitStatement(for: connection.type)
-                if !commitStmt.isEmpty {
-                    _ = try await driver.execute(query: commitStmt)
-                }
+                try await driver.commitTransaction()
             }
 
             // 8. Re-enable FK checks (if enabled) - AFTER transaction
@@ -191,10 +185,7 @@ final class ImportService {
             // Rollback on error - this is CRITICAL and must not fail silently
             if config.wrapInTransaction {
                 do {
-                    let rollbackStmt = rollbackStatement(for: connection.type)
-                    if !rollbackStmt.isEmpty {
-                        _ = try await driver.execute(query: rollbackStmt)
-                    }
+                    try await driver.rollbackTransaction()
                 } catch let rollbackError {
                     throw ImportError.rollbackFailed(rollbackError.localizedDescription)
                 }
@@ -311,23 +302,4 @@ final class ImportService {
             return []
         }
     }
-
-
-    private func commitStatement(for dbType: DatabaseType) -> String {
-        switch dbType {
-        case .mongodb, .redis, .clickhouse:
-            return ""
-        default:
-            return "COMMIT"
-        }
-    }
-
-    private func rollbackStatement(for dbType: DatabaseType) -> String {
-        switch dbType {
-        case .mongodb, .redis, .clickhouse:
-            return ""
-        default:
-            return "ROLLBACK"
-        }
-    }
 }
diff --git a/TablePro/Models/Connection/DatabaseConnection.swift b/TablePro/Models/Connection/DatabaseConnection.swift
@@ -287,16 +287,6 @@ enum DatabaseType: String, CaseIterable, Identifiable, Codable {
         }
     }
 
-    var beginTransactionSQL: String {
-        switch self {
-        case .mysql, .mariadb: return "START TRANSACTION"
-        case .postgresql, .redshift, .sqlite: return "BEGIN"
-        case .mssql: return "BEGIN TRANSACTION"
-        case .oracle: return ""
-        case .mongodb, .redis, .clickhouse: return ""
-        }
-    }
-
     /// Whether this database type supports SQL-based schema editing (ALTER TABLE etc.)
     var supportsSchemaEditing: Bool {
         switch self {
diff --git a/TablePro/Views/Main/Extensions/MainContentCoordinator+Discard.swift b/TablePro/Views/Main/Extensions/MainContentCoordinator+Discard.swift
@@ -16,32 +16,19 @@ extension MainContentCoordinator {
             throw DatabaseError.notConnected
         }
 
-        let dbType = connection.type
-        let supportsTransactions = dbType != .redis && dbType != .mongodb && dbType != .clickhouse
-        var allStatements: [ParameterizedStatement] = []
-
-        if supportsTransactions {
-            allStatements.append(ParameterizedStatement(sql: dbType.beginTransactionSQL, parameters: []))
-        }
-
-        allStatements.append(contentsOf: statements)
-
-        if supportsTransactions {
-            allStatements.append(ParameterizedStatement(sql: "COMMIT", parameters: []))
-        }
+        try await driver.beginTransaction()
 
         do {
-            for stmt in allStatements {
+            for stmt in statements {
                 if stmt.parameters.isEmpty {
                     _ = try await driver.execute(query: stmt.sql)
                 } else {
                     _ = try await driver.executeParameterized(query: stmt.sql, parameters: stmt.parameters)
                 }
             }
+            try await driver.commitTransaction()
         } catch {
-            if supportsTransactions {
-                _ = try? await driver.execute(query: "ROLLBACK")
-            }
+            try? await driver.rollbackTransaction()
             throw error
         }
     }
diff --git a/TablePro/Views/Main/Extensions/MainContentCoordinator+MultiStatement.swift b/TablePro/Views/Main/Extensions/MainContentCoordinator+MultiStatement.swift
@@ -33,7 +33,6 @@ extension MainContentCoordinator {
         let conn = connection
         let tabId = tabManager.tabs[index].id
         let totalCount = statements.count
-        let dbType = connection.type
 
         currentQueryTask = Task {
             var cumulativeTime: TimeInterval = 0
@@ -49,12 +48,12 @@ extension MainContentCoordinator {
                 }
 
                 // Wrap in a transaction for atomicity
-                _ = try await driver.execute(query: dbType.beginTransactionSQL)
+                try await driver.beginTransaction()
 
                 for (stmtIndex, sql) in statements.enumerated() {
                     guard !Task.isCancelled else { break }
                     guard capturedGeneration == queryGeneration else {
-                        _ = try? await driver.execute(query: "ROLLBACK")
+                        try? await driver.rollbackTransaction()
                         return
                     }
 
@@ -86,7 +85,7 @@ extension MainContentCoordinator {
                 }
 
                 // Commit the transaction
-                _ = try await driver.execute(query: "COMMIT")
+                try await driver.commitTransaction()
 
                 // All statements succeeded — update tab with results
                 await MainActor.run {
@@ -140,7 +139,7 @@ extension MainContentCoordinator {
             } catch {
                 // Rollback on failure
                 if let driver = DatabaseManager.shared.driver(for: conn.id) {
-                    _ = try? await driver.execute(query: "ROLLBACK")
+                    try? await driver.rollbackTransaction()
                 }
 
                 guard capturedGeneration == queryGeneration else { return }
diff --git a/TablePro/Views/Main/Extensions/MainContentCoordinator+SQLPreview.swift b/TablePro/Views/Main/Extensions/MainContentCoordinator+SQLPreview.swift
@@ -51,6 +51,7 @@ extension MainContentCoordinator {
 
     /// Assembles all pending SQL statements (cell edits + table operations) in execution order.
     /// Used by both `saveChanges()` and `generatePreviewSQL()` to ensure consistency.
+    /// Transaction wrapping is handled by the caller using driver protocol methods.
     func assemblePendingStatements(
         pendingTruncates: Set<String>,
         pendingDeletes: Set<String>,
@@ -59,7 +60,6 @@ extension MainContentCoordinator {
         var allStatements: [ParameterizedStatement] = []
         let dbType = connection.type
 
-        let hasEditedCells = changeManager.hasChanges
         let hasPendingTableOps = !pendingTruncates.isEmpty || !pendingDeletes.isEmpty
 
         // Check if any table operation needs FK disabled (must be outside transaction)
@@ -74,36 +74,23 @@ extension MainContentCoordinator {
             })
         }
 
-        // Wrap all operations in a single transaction when we have multiple operations
-        let needsTransaction = hasEditedCells && hasPendingTableOps
-        if needsTransaction {
-            let beginSQL = dbType.beginTransactionSQL
-            allStatements.append(ParameterizedStatement(sql: beginSQL, parameters: []))
-        }
-
-        if hasEditedCells {
+        if changeManager.hasChanges {
             let editStatements = try changeManager.generateSQL()
             allStatements.append(contentsOf: editStatements)
         }
 
         if hasPendingTableOps {
-            // Generate table operation SQL WITHOUT FK handling (already done above)
             let tableOpStatements = generateTableOperationSQL(
                 truncates: pendingTruncates,
                 deletes: pendingDeletes,
                 options: tableOperationOptions,
-                wrapInTransaction: !needsTransaction,
                 includeFKHandling: false
             )
             allStatements.append(contentsOf: tableOpStatements.map {
                 ParameterizedStatement(sql: $0, parameters: [])
             })
         }
 
-        if needsTransaction {
-            allStatements.append(ParameterizedStatement(sql: "COMMIT", parameters: []))
-        }
-
         // FK re-enable must be LAST, after transaction commits
         if needsDisableFK {
             allStatements.append(contentsOf: fkEnableStatements(for: dbType).map {
diff --git a/TablePro/Views/Main/Extensions/MainContentCoordinator+TableOperations.swift b/TablePro/Views/Main/Extensions/MainContentCoordinator+TableOperations.swift
@@ -15,14 +15,12 @@ extension MainContentCoordinator {
     ///   - truncates: Set of table names to truncate
     ///   - deletes: Set of table names to drop
     ///   - options: Per-table options for FK and cascade handling
-    ///   - wrapInTransaction: Whether to wrap statements in BEGIN/COMMIT
     ///   - includeFKHandling: Whether to include FK disable/enable statements (set false when caller handles FK)
     /// - Returns: Array of SQL statements to execute
     func generateTableOperationSQL(
         truncates: Set<String>,
         deletes: Set<String>,
         options: [String: TableOperationOptions],
-        wrapInTransaction: Bool = true,
         includeFKHandling: Bool = true
     ) -> [String] {
         var statements: [String] = []
@@ -42,12 +40,6 @@ extension MainContentCoordinator {
             statements.append(contentsOf: fkDisableStatements(for: dbType))
         }
 
-        // Wrap in transaction for atomicity
-        let needsTransaction = wrapInTransaction && (sortedTruncates.count + sortedDeletes.count) > 1
-        if needsTransaction {
-            statements.append(dbType.beginTransactionSQL)
-        }
-
         for tableName in sortedTruncates {
             let quotedName = dbType.quoteIdentifier(tableName)
             let tableOptions = options[tableName] ?? TableOperationOptions()
@@ -65,10 +57,6 @@ extension MainContentCoordinator {
             statements.append(dropTableStatement(tableName: tableName, quotedName: quotedName, isView: viewNames.contains(tableName), options: tableOptions, dbType: dbType))
         }
 
-        if needsTransaction {
-            statements.append("COMMIT")
-        }
-
         // FK re-enable must be OUTSIDE transaction to ensure it runs even on rollback
         if needsDisableFK {
             statements.append(contentsOf: fkEnableStatements(for: dbType))
diff --git a/TablePro/Views/Main/MainContentCoordinator.swift b/TablePro/Views/Main/MainContentCoordinator.swift

Original file line number	Diff line number	Diff line change
`@@ -239,15 +239,15 @@ final class PluginDriverAdapter: DatabaseDriver, SchemaSwitchable {`
`239`	`239`	`// MARK: - Transaction Management`
`240`	`240`
`241`	`241`	`func beginTransaction() async throws {`
`242`		`- _ = try await pluginDriver.execute(query: "BEGIN")`
	`242`	`+ try await pluginDriver.beginTransaction()`
`243`	`243`	`}`
`244`	`244`
`245`	`245`	`func commitTransaction() async throws {`
`246`		`- _ = try await pluginDriver.execute(query: "COMMIT")`
	`246`	`+ try await pluginDriver.commitTransaction()`
`247`	`247`	`}`
`248`	`248`
`249`	`249`	`func rollbackTransaction() async throws {`
`250`		`- _ = try await pluginDriver.execute(query: "ROLLBACK")`
	`250`	`+ try await pluginDriver.rollbackTransaction()`
`251`	`251`	`}`
`252`	`252`
`253`	`253`	`// MARK: - Schema Switching`