From 0eab098946d8250d0376481c655737a52682e132 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Sat, 26 Sep 2026 12:11:42 +0700 Subject: [PATCH] fix(datagrid): refuse a save that would leave out a change its driver cannot write --- CHANGELOG.md | 1 + .../ElasticsearchPluginDriver.swift | 7 +- .../ElasticsearchStatementGenerator.swift | 149 ++++-- .../EtcdDriverPlugin/EtcdPluginDriver.swift | 7 +- .../EtcdStatementGenerator.swift | 98 ++-- .../RedisDatabaseTarget.swift | 19 +- Plugins/RedisDriverPlugin/RedisKeySlot.swift | 12 +- .../RedisDriverPlugin/RedisPluginDriver.swift | 11 +- .../RedisStatementGenerator.swift | 207 ++++---- .../PluginDatabaseDriver.swift | 40 ++ .../TableProPluginKit/PluginRowWrite.swift | 44 ++ .../ChangeTracking/DataChangeManager.swift | 20 +- .../SQLStatementGenerator.swift | 27 +- .../RowEditingCoordinator+SaveChanges.swift | 6 +- TablePro/Core/DataWrite/DataWriteError.swift | 28 ++ .../DataWrite/RowChangeStatementFactory.swift | 175 ++++--- .../Core/DataWrite/RowWriteCoverage.swift | 95 ++++ .../Core/Plugins/PluginDriverAdapter.swift | 21 - TablePro/Resources/Localizable.xcstrings | 84 ++++ ...wChangeStatementFactoryCoverageTests.swift | 442 ++++++++++++++++++ .../Core/Redis/RedisBinaryValueTests.swift | 48 +- .../Core/Redis/RedisCommandParserTests.swift | 8 +- .../Helpers/RowWriteStubDrivers.swift | 165 +++++++ .../Plugins/ElasticsearchDriverTests.swift | 210 ++++++++- .../Plugins/EtcdStatementGeneratorTests.swift | 232 ++++++--- .../Plugins/RedisDatabaseTargetTests.swift | 7 +- TableProTests/Plugins/RedisKeySlotTests.swift | 8 +- .../RedisNamedDatabaseWriteTests.swift | 7 +- .../RedisStatementGeneratorTests.swift | 386 ++++++++++----- .../Views/Main/SaveCompletionTests.swift | 49 +- .../Views/Main/SidebarSaveCoverageTests.swift | 75 +++ docs/databases/elasticsearch.mdx | 4 +- docs/databases/etcd.mdx | 4 +- docs/databases/redis.mdx | 8 +- docs/development/plugin-development.mdx | 5 +- docs/features/change-tracking.mdx | 6 +- 36 files changed, 2159 insertions(+), 556 deletions(-) create mode 100644 Plugins/TableProPluginKit/PluginRowWrite.swift create mode 100644 TablePro/Core/DataWrite/RowWriteCoverage.swift create mode 100644 TableProTests/Core/DataWrite/RowChangeStatementFactoryCoverageTests.swift create mode 100644 TableProTests/Helpers/RowWriteStubDrivers.swift create mode 100644 TableProTests/Views/Main/SidebarSaveCoverageTests.swift diff --git a/CHANGELOG.md b/CHANGELOG.md index fd43cf0c69..1f5d8f975f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -131,6 +131,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Pre-connect script failures sometimes reported without the script's own error message. - Failed MongoDB statements, including writes the server rejected, reported as successful with an empty result. +- Save reporting success after leaving out an edit it could not write, such as a new MongoDB document left empty. (#3132) - `tablepro-mcp` crashing when its standard input was non-blocking. - `tablepro-mcp` using a full CPU core, or crashing, when its standard output or error was non-blocking. - Server connections piling up while browsing many databases or schemas, and staying open after a failed connect. (#3103) diff --git a/Plugins/ElasticsearchDriverPlugin/ElasticsearchPluginDriver.swift b/Plugins/ElasticsearchDriverPlugin/ElasticsearchPluginDriver.swift index f6daac652e..4357b95a0c 100644 --- a/Plugins/ElasticsearchDriverPlugin/ElasticsearchPluginDriver.swift +++ b/Plugins/ElasticsearchDriverPlugin/ElasticsearchPluginDriver.swift @@ -238,18 +238,19 @@ internal final class ElasticsearchPluginDriver: PluginDatabaseDriver, @unchecked // MARK: - Statement Generation - func generateStatements( + func generateRowWrites( table: String, + schema: String?, columns: [String], primaryKeyColumns: [String], changes: [PluginRowChange], insertedRowData: [Int: [PluginCellValue]], deletedRowIndices: Set, insertedRowIndices: Set - ) -> [(statement: String, parameters: [PluginCellValue])]? { + ) throws -> [PluginRowWrite]? { let typeNames = columnTypeNames(for: columns, index: table) let generator = ElasticsearchStatementGenerator(index: table, columns: columns, columnTypeNames: typeNames) - return generator.generateStatements( + return try generator.generateRowWrites( from: changes, insertedRowData: insertedRowData, deletedRowIndices: deletedRowIndices, diff --git a/Plugins/ElasticsearchDriverPlugin/ElasticsearchStatementGenerator.swift b/Plugins/ElasticsearchDriverPlugin/ElasticsearchStatementGenerator.swift index 4691964aa4..b4c46d5561 100644 --- a/Plugins/ElasticsearchDriverPlugin/ElasticsearchStatementGenerator.swift +++ b/Plugins/ElasticsearchDriverPlugin/ElasticsearchStatementGenerator.swift @@ -6,7 +6,6 @@ // import Foundation -import os import TableProPluginKit struct ElasticsearchWriteRequest: Equatable { @@ -16,7 +15,6 @@ struct ElasticsearchWriteRequest: Equatable { } struct ElasticsearchStatementGenerator { - private static let logger = Logger(subsystem: "com.TablePro", category: "ElasticsearchStatementGenerator") static let writeTag = "ELASTICSEARCH_WRITE:" private static let refreshQuery = "?refresh=true" @@ -27,64 +25,71 @@ struct ElasticsearchStatementGenerator { /// A `nested` column carries the whole array of objects, and its dotted leaves are views of /// those same bytes. Writing both makes Elasticsearch expand the dotted key into the object the /// array already fills, which it rejects as a mapping conflict, so the array is written once - /// through its parent and an edit to a leaf is refused rather than sent. - private let nestedLeafColumns: Set + /// through its parent and a value typed into a leaf is refused rather than sent. Each leaf maps + /// to the outermost array it belongs to, which is the column that writes it. + private let nestedParentByLeaf: [String: String] init(index: String, columns: [String], columnTypeNames: [String]) { self.index = index self.columns = columns self.columnTypeNames = columnTypeNames - self.nestedLeafColumns = Self.nestedLeaves(columns: columns, typeNames: columnTypeNames) + self.nestedParentByLeaf = Self.nestedParents(columns: columns, typeNames: columnTypeNames) } - private static func nestedLeaves(columns: [String], typeNames: [String]) -> Set { + private static func nestedParents(columns: [String], typeNames: [String]) -> [String: String] { let parents = zip(columns, typeNames) .filter { $0.1 == ElasticsearchMappingFlattener.nestedTypeName } .map(\.0) - guard !parents.isEmpty else { return [] } - return Set(columns.filter { column in - parents.contains { column.hasPrefix("\($0).") } - }) + guard !parents.isEmpty else { return [:] } + var parentByLeaf: [String: String] = [:] + for column in columns { + let owners = parents.filter { column.hasPrefix("\($0).") } + if let outermost = owners.min(by: { $0.count < $1.count }) { + parentByLeaf[column] = outermost + } + } + return parentByLeaf } private var metaColumns: Set { Set(ElasticsearchMappingFlattener.metaColumns) } - func generateStatements( + /// One request per change, each naming the change it writes. A change carrying a value this + /// driver cannot send is refused whole, because writing the rest of it would let the save + /// succeed and clear the value it left out. + func generateRowWrites( from changes: [PluginRowChange], insertedRowData: [Int: [PluginCellValue]], deletedRowIndices: Set, insertedRowIndices: Set - ) -> [(statement: String, parameters: [PluginCellValue])] { - var statements: [(statement: String, parameters: [PluginCellValue])] = [] + ) throws -> [PluginRowWrite] { + var writes: [PluginRowWrite] = [] for change in changes { + let request: ElasticsearchWriteRequest? switch change.type { case .insert: guard insertedRowIndices.contains(change.rowIndex) else { continue } - if let statement = generateInsert(for: change, insertedRowData: insertedRowData) { - statements.append(statement) - } + request = try insertRequest(for: change, insertedRowData: insertedRowData) case .update: - if let statement = generateUpdate(for: change) { - statements.append(statement) - } + request = try updateRequest(for: change) case .delete: guard deletedRowIndices.contains(change.rowIndex) else { continue } - if let statement = generateDelete(for: change) { - statements.append(statement) - } + request = try deleteRequest(for: change) + } + if let request { + writes.append(PluginRowWrite(statement: Self.encode(request), rowIndices: [change.rowIndex])) } } - return statements + return writes } // MARK: - INSERT - private func generateInsert( + private func insertRequest( for change: PluginRowChange, insertedRowData: [Int: [PluginCellValue]] - ) -> (statement: String, parameters: [PluginCellValue])? { + ) throws -> ElasticsearchWriteRequest { var values: [String: PluginCellValue] = [:] if let rowData = insertedRowData[change.rowIndex] { for (columnIndex, column) in columns.enumerated() where columnIndex < rowData.count { @@ -96,56 +101,106 @@ struct ElasticsearchStatementGenerator { } } + if let reason = unwritableInsertValue(in: change, values: values) { + throw PluginRowWriteRefusal(rowIndex: change.rowIndex, reason: reason) + } + var document: [String: Any] = [:] - for column in columns where !metaColumns.contains(column) && !nestedLeafColumns.contains(column) { + for column in columns where !metaColumns.contains(column) && nestedParentByLeaf[column] == nil { guard let value = values[column], let text = value.asText else { continue } document[column] = jsonValue(text, for: column) } - guard let body = serialize(document) else { return nil } + guard let body = serialize(document) else { + throw PluginRowWriteRefusal(rowIndex: change.rowIndex, reason: Self.notJSONReason) + } let explicitId = values[ElasticsearchMappingFlattener.idColumn]?.asText if let id = explicitId, !id.isEmpty { - return encode(.init(method: "PUT", path: docPath(id: id), body: body)) + return .init(method: "PUT", path: docPath(id: id), body: body) } - return encode(.init(method: "POST", path: "/\(encodedIndex)/_doc\(Self.refreshQuery)", body: body)) + return .init(method: "POST", path: "/\(encodedIndex)/_doc\(Self.refreshQuery)", body: body) + } + + /// A new row's leaf value reaches the server only inside its array, so one the user typed, or + /// one whose array is empty, would be dropped. Metadata other than `_id` is the server's to set. + private func unwritableInsertValue(in change: PluginRowChange, values: [String: PluginCellValue]) -> String? { + for cellChange in change.cellChanges where !cellChange.newValue.isNull { + let column = cellChange.columnName + if column != ElasticsearchMappingFlattener.idColumn, metaColumns.contains(column) { + return Self.metadataReason(column) + } + if let parent = nestedParentByLeaf[column] { + return Self.nestedLeafReason(leaf: column, parent: parent) + } + } + for column in columns { + guard let parent = nestedParentByLeaf[column], + values[column]?.isNull == false, + values[parent]?.isNull ?? true + else { continue } + return Self.nestedLeafReason(leaf: column, parent: parent) + } + return nil } // MARK: - UPDATE - private func generateUpdate(for change: PluginRowChange) -> (statement: String, parameters: [PluginCellValue])? { + private func updateRequest(for change: PluginRowChange) throws -> ElasticsearchWriteRequest? { + guard !change.cellChanges.isEmpty else { return nil } guard let id = documentId(from: change) else { - Self.logger.warning("Skipping UPDATE - missing _id") - return nil + throw PluginRowWriteRefusal(rowIndex: change.rowIndex, reason: Self.missingIdReason) } var doc: [String: Any] = [:] - for cellChange in change.cellChanges where !metaColumns.contains(cellChange.columnName) { - if nestedLeafColumns.contains(cellChange.columnName) { - Self.logger.warning( - "Skipping UPDATE of nested leaf \(cellChange.columnName, privacy: .public) - edit the parent column" + for cellChange in change.cellChanges { + let column = cellChange.columnName + if metaColumns.contains(column) { + throw PluginRowWriteRefusal(rowIndex: change.rowIndex, reason: Self.metadataReason(column)) + } + if let parent = nestedParentByLeaf[column] { + throw PluginRowWriteRefusal( + rowIndex: change.rowIndex, reason: Self.nestedLeafReason(leaf: column, parent: parent) ) - continue } if let text = cellChange.newValue.asText { - doc[cellChange.columnName] = jsonValue(text, for: cellChange.columnName) + doc[column] = jsonValue(text, for: column) } else { - doc[cellChange.columnName] = NSNull() + doc[column] = NSNull() } } - guard !doc.isEmpty, let body = serialize(["doc": doc]) else { return nil } - return encode(.init(method: "POST", path: "/\(encodedIndex)/_update/\(encodePathComponent(id))\(Self.refreshQuery)", body: body)) + guard let body = serialize(["doc": doc]) else { + throw PluginRowWriteRefusal(rowIndex: change.rowIndex, reason: Self.notJSONReason) + } + return .init(method: "POST", path: "/\(encodedIndex)/_update/\(encodePathComponent(id))\(Self.refreshQuery)", body: body) } // MARK: - DELETE - private func generateDelete(for change: PluginRowChange) -> (statement: String, parameters: [PluginCellValue])? { + private func deleteRequest(for change: PluginRowChange) throws -> ElasticsearchWriteRequest { guard let id = documentId(from: change) else { - Self.logger.warning("Skipping DELETE - missing _id") - return nil + throw PluginRowWriteRefusal(rowIndex: change.rowIndex, reason: Self.missingIdReason) } - return encode(.init(method: "DELETE", path: docPath(id: id), body: nil)) + return .init(method: "DELETE", path: docPath(id: id), body: nil) + } + + // MARK: - Refusals + + private static func nestedLeafReason(leaf: String, parent: String) -> String { + String(format: String(localized: "'%@' is a field of a nested array. Edit the array in '%@' instead."), leaf, parent) + } + + private static func metadataReason(_ column: String) -> String { + String(format: String(localized: "'%@' is document metadata and cannot be edited."), column) + } + + private static var missingIdReason: String { + String(localized: "The document's _id is unknown, so it cannot be addressed.") + } + + private static var notJSONReason: String { + String(localized: "The new values cannot be written as JSON.") } // MARK: - Helpers @@ -206,10 +261,6 @@ struct ElasticsearchStatementGenerator { return String(data: data, encoding: .utf8) } - private func encode(_ request: ElasticsearchWriteRequest) -> (statement: String, parameters: [PluginCellValue]) { - (statement: Self.encode(request), parameters: []) - } - static func encode(_ request: ElasticsearchWriteRequest) -> String { let b64Method = Data(request.method.utf8).base64EncodedString() let b64Path = Data(request.path.utf8).base64EncodedString() diff --git a/Plugins/EtcdDriverPlugin/EtcdPluginDriver.swift b/Plugins/EtcdDriverPlugin/EtcdPluginDriver.swift index c7293d3f90..899c3bc939 100644 --- a/Plugins/EtcdDriverPlugin/EtcdPluginDriver.swift +++ b/Plugins/EtcdDriverPlugin/EtcdPluginDriver.swift @@ -447,20 +447,21 @@ final class EtcdPluginDriver: PluginDatabaseDriver, @unchecked Sendable { // MARK: - Statement Generation - func generateStatements( + func generateRowWrites( table: String, + schema: String?, columns: [String], primaryKeyColumns: [String], changes: [PluginRowChange], insertedRowData: [Int: [PluginCellValue]], deletedRowIndices: Set, insertedRowIndices: Set - ) -> [(statement: String, parameters: [PluginCellValue])]? { + ) throws -> [PluginRowWrite]? { let generator = EtcdStatementGenerator( prefix: resolvedPrefix(for: table), columns: columns ) - return generator.generateStatements( + return try generator.generateRowWrites( from: changes, insertedRowData: insertedRowData, deletedRowIndices: deletedRowIndices, diff --git a/Plugins/EtcdDriverPlugin/EtcdStatementGenerator.swift b/Plugins/EtcdDriverPlugin/EtcdStatementGenerator.swift index a10246eca6..da9b2bf3fe 100644 --- a/Plugins/EtcdDriverPlugin/EtcdStatementGenerator.swift +++ b/Plugins/EtcdDriverPlugin/EtcdStatementGenerator.swift @@ -6,12 +6,9 @@ // import Foundation -import os import TableProPluginKit struct EtcdStatementGenerator { - private static let logger = Logger(subsystem: "com.TablePro", category: "EtcdStatementGenerator") - let prefix: String let columns: [String] @@ -19,36 +16,50 @@ struct EtcdStatementGenerator { private var valueColumnIndex: Int? { columns.firstIndex(of: "Value") } private var leaseColumnIndex: Int? { columns.firstIndex(of: "Lease") } - func generateStatements( + /// The columns a `put` can set. The rest, the version and the two revisions, are etcd's own. + /// etcd stores no NULL, so a NULL written to Value means an empty value, and to Lease, no lease. + private static let writableColumns: Set = ["Key", "Value", "Lease"] + + /// The commands that write the grid's changes, each naming the change it writes. A change + /// carrying a value these commands cannot express is refused whole, because writing the rest + /// of it would let the save succeed and clear the value it left out. + func generateRowWrites( from changes: [PluginRowChange], insertedRowData: [Int: [PluginCellValue]], deletedRowIndices: Set, insertedRowIndices: Set - ) -> [(statement: String, parameters: [PluginCellValue])] { - var statements: [(statement: String, parameters: [PluginCellValue])] = [] + ) throws -> [PluginRowWrite] { + var writes: [PluginRowWrite] = [] for change in changes { + let commands: [String] switch change.type { case .insert: guard insertedRowIndices.contains(change.rowIndex) else { continue } - statements += generateInsert(for: change, insertedRowData: insertedRowData) + commands = try insertCommands(for: change, insertedRowData: insertedRowData) case .update: - statements += generateUpdate(for: change) + commands = try updateCommands(for: change) case .delete: guard deletedRowIndices.contains(change.rowIndex) else { continue } - if let key = extractKey(from: change) { - statements.append((statement: "del \(escapeArgument(key))", parameters: [])) + guard let key = extractKey(from: change) else { + throw PluginRowWriteRefusal(rowIndex: change.rowIndex, reason: Self.unaddressableKeyReason) } + commands = ["del \(escapeArgument(key))"] } + writes += commands.map { PluginRowWrite(statement: $0, rowIndices: [change.rowIndex]) } } - return statements + return writes } - private func generateInsert( + private func insertCommands( for change: PluginRowChange, insertedRowData: [Int: [PluginCellValue]] - ) -> [(statement: String, parameters: [PluginCellValue])] { + ) throws -> [String] { + if let column = serverOwnedColumn(in: change) { + throw PluginRowWriteRefusal(rowIndex: change.rowIndex, reason: Self.serverOwnedReason(column)) + } + var key: String? var value: String? var leaseId: String? @@ -69,8 +80,7 @@ struct EtcdStatementGenerator { } guard let k = key, !k.isEmpty else { - Self.logger.warning("Skipping INSERT - no key provided") - return [] + throw PluginRowWriteRefusal(rowIndex: change.rowIndex, reason: String(localized: "A new key needs a name.")) } // Prepend the current browse prefix if the key doesn't already include it @@ -86,52 +96,72 @@ struct EtcdStatementGenerator { cmd += " --lease=\(lease)" } - return [(statement: cmd, parameters: [])] + return [cmd] } - private func generateUpdate( - for change: PluginRowChange - ) -> [(statement: String, parameters: [PluginCellValue])] { + private func updateCommands(for change: PluginRowChange) throws -> [String] { guard !change.cellChanges.isEmpty else { return [] } guard let originalKey = extractKey(from: change) else { - Self.logger.warning("Skipping UPDATE - no original key") - return [] + throw PluginRowWriteRefusal(rowIndex: change.rowIndex, reason: Self.unaddressableKeyReason) + } + if let column = serverOwnedColumn(in: change) { + throw PluginRowWriteRefusal(rowIndex: change.rowIndex, reason: Self.serverOwnedReason(column)) } - var statements: [(statement: String, parameters: [PluginCellValue])] = [] + var commands: [String] = [] let keyChange = change.cellChanges.first { $0.columnName == "Key" } - let newKey = keyChange?.newValue.asText ?? originalKey + let valueChange = change.cellChanges.first { $0.columnName == "Value" } + let leaseChange = change.cellChanges.first { $0.columnName == "Lease" } + let newKey = keyChange.map { $0.newValue.asText ?? "" } ?? originalKey guard !newKey.isEmpty else { - Self.logger.warning("Skipping UPDATE - empty key") - return [] + throw PluginRowWriteRefusal(rowIndex: change.rowIndex, reason: String(localized: "A key needs a name.")) } let shouldDeleteOriginalKey = newKey != originalKey - let valueChange = change.cellChanges.first { $0.columnName == "Value" } - let leaseChange = change.cellChanges.first { $0.columnName == "Lease" } + let lease = leaseChange.map { $0.newValue.asText ?? "" } if valueChange != nil || newKey != originalKey { - let newValue = valueChange?.newValue.asText ?? extractOriginalValue(from: change) ?? "" + let newValue = valueChange.map { $0.newValue.asText ?? "" } ?? extractOriginalValue(from: change) ?? "" var cmd = "put \(escapeArgument(newKey)) \(escapeArgument(newValue))" - if let lease = leaseChange?.newValue.asText, !lease.isEmpty, lease != "0" { + if let lease, !lease.isEmpty, lease != "0" { cmd += " --lease=\(lease)" } - statements.append((statement: cmd, parameters: [])) + commands.append(cmd) if shouldDeleteOriginalKey { - statements.append((statement: "del \(escapeArgument(originalKey))", parameters: [])) + commands.append("del \(escapeArgument(originalKey))") } - } else if let lease = leaseChange?.newValue.asText { + } else if let lease { let currentValue = extractOriginalValue(from: change) ?? "" var cmd = "put \(escapeArgument(newKey)) \(escapeArgument(currentValue))" if !lease.isEmpty && lease != "0" { cmd += " --lease=\(lease)" } - statements.append((statement: cmd, parameters: [])) + commands.append(cmd) } - return statements + return commands + } + + /// An edit to a column no `put` can set, which the save would otherwise drop. A new row's NULL + /// there leaves the value to etcd, so only a value set in one counts. + private func serverOwnedColumn(in change: PluginRowChange) -> String? { + change.cellChanges + .first { cell in + !Self.writableColumns.contains(cell.columnName) && (change.type == .update || !cell.newValue.isNull) + }? + .columnName + } + + // MARK: - Refusals + + private static var unaddressableKeyReason: String { + String(localized: "This key's name is not text, so it cannot be addressed from the grid.") + } + + private static func serverOwnedReason(_ column: String) -> String { + String(format: String(localized: "'%@' is set by etcd and cannot be edited."), column) } // MARK: - Helpers diff --git a/Plugins/RedisDriverPlugin/RedisDatabaseTarget.swift b/Plugins/RedisDriverPlugin/RedisDatabaseTarget.swift index 903017ae22..391589d56d 100644 --- a/Plugins/RedisDriverPlugin/RedisDatabaseTarget.swift +++ b/Plugins/RedisDriverPlugin/RedisDatabaseTarget.swift @@ -15,26 +15,27 @@ import TableProPluginKit private let logger = Logger(subsystem: "com.TablePro.RedisDriver", category: "RedisDatabaseTarget") enum RedisDatabaseTarget { - typealias Statement = (statement: String, parameters: [PluginCellValue]) - /// A grid's writes belong to the database its rows came from, which the session is not on /// when the user moved it elsewhere. Run inside the save's `MULTI`, the SELECTs are queued /// with the writes and applied together by `EXEC`, which leaves the session where it was. /// Without one, as on a cluster, a SELECT sent first stays in force when a write after it /// fails, so each write names its database and the session never leaves where it belongs. + /// A write keeps the changes it names; a SELECT names none. static func addressing( - _ statements: [Statement], + _ writes: [PluginRowWrite], toDatabase index: Int?, from home: Int, insideTransaction: Bool - ) -> [Statement] { - guard let index, index != home, !statements.isEmpty else { return statements } + ) -> [PluginRowWrite] { + guard let index, index != home, !writes.isEmpty else { return writes } guard insideTransaction else { - return statements.map { (statement: "DB \(index) \($0.statement)", parameters: $0.parameters) } + return writes.map { + PluginRowWrite(statement: "DB \(index) \($0.statement)", parameters: $0.parameters, rowIndices: $0.rowIndices) + } } - return [(statement: "SELECT \(index)", parameters: [])] - + statements - + [(statement: "SELECT \(home)", parameters: [])] + return [PluginRowWrite(statement: "SELECT \(index)", rowIndices: [])] + + writes + + [PluginRowWrite(statement: "SELECT \(home)", rowIndices: [])] } } diff --git a/Plugins/RedisDriverPlugin/RedisKeySlot.swift b/Plugins/RedisDriverPlugin/RedisKeySlot.swift index c2127c7b12..c4a3a53bc0 100644 --- a/Plugins/RedisDriverPlugin/RedisKeySlot.swift +++ b/Plugins/RedisDriverPlugin/RedisKeySlot.swift @@ -32,14 +32,14 @@ enum RedisKeySlot { return keys.allSatisfy { slot(for: $0) == reference } } - /// Keys that share a slot, in the order each slot first appears, with duplicates kept. - static func groupedBySlot(_ keys: [String]) -> [[String]] { + /// Elements whose keys share a slot, in the order each slot first appears, with duplicates kept. + static func groupedBySlot(_ elements: [Element], key: (Element) -> String) -> [[Element]] { var order: [Int] = [] - var groups: [Int: [String]] = [:] - for key in keys { - let keySlot = slot(for: key) + var groups: [Int: [Element]] = [:] + for element in elements { + let keySlot = slot(for: key(element)) if groups[keySlot] == nil { order.append(keySlot) } - groups[keySlot, default: []].append(key) + groups[keySlot, default: []].append(element) } return order.compactMap { groups[$0] } } diff --git a/Plugins/RedisDriverPlugin/RedisPluginDriver.swift b/Plugins/RedisDriverPlugin/RedisPluginDriver.swift index 1c5f536550..7c9b8e2997 100644 --- a/Plugins/RedisDriverPlugin/RedisPluginDriver.swift +++ b/Plugins/RedisDriverPlugin/RedisPluginDriver.swift @@ -577,27 +577,28 @@ final class RedisPluginDriver: PluginDatabaseDriver, @unchecked Sendable { ) } - func generateStatements( + func generateRowWrites( table: String, + schema: String?, columns: [String], primaryKeyColumns: [String], changes: [PluginRowChange], insertedRowData: [Int: [PluginCellValue]], deletedRowIndices: Set, insertedRowIndices: Set - ) -> [(statement: String, parameters: [PluginCellValue])]? { + ) throws -> [PluginRowWrite]? { let generator = RedisStatementGenerator( namespaceName: table, columns: columns, deleteBatching: redisConnection?.partitionsKeyspace == true ? .perHashSlot : .singleCommand ) - let statements = generator.generateStatements( + let writes = try generator.generateRowWrites( from: changes, insertedRowData: insertedRowData, deletedRowIndices: deletedRowIndices, insertedRowIndices: insertedRowIndices ) - guard let conn = redisConnection, conn.supportsDatabaseSelection else { return statements } + guard let conn = redisConnection, conn.supportsDatabaseSelection else { return writes } return RedisDatabaseTarget.addressing( - statements, + writes, toDatabase: RedisDatabaseIndex.parse(table), from: conn.homeDatabase(), insideTransaction: conn.supportsTransactions diff --git a/Plugins/RedisDriverPlugin/RedisStatementGenerator.swift b/Plugins/RedisDriverPlugin/RedisStatementGenerator.swift index af79066cc7..9625b1bd0c 100644 --- a/Plugins/RedisDriverPlugin/RedisStatementGenerator.swift +++ b/Plugins/RedisDriverPlugin/RedisStatementGenerator.swift @@ -7,7 +7,6 @@ // import Foundation -import os import TableProPluginKit /// How the grid's deleted keys become `DEL` statements. @@ -21,8 +20,6 @@ enum RedisDeleteBatching: Sendable { } struct RedisStatementGenerator { - private static let logger = Logger(subsystem: "com.TablePro", category: "RedisStatementGenerator") - let namespaceName: String let columns: [String] var deleteBatching: RedisDeleteBatching = .singleCommand @@ -47,63 +44,68 @@ struct RedisStatementGenerator { columns.firstIndex(of: "TTL") } + private static let insertableTypes: Set = ["string", "hash", "list", "set", "zset"] + // MARK: - Public API - /// Generate Redis commands from changes - func generateStatements( + /// The commands that write the grid's changes, each naming the change it writes. A change + /// carrying a value these commands cannot express is refused whole, because writing the rest + /// of it would let the save succeed and clear the value it left out. + func generateRowWrites( from changes: [PluginRowChange], insertedRowData: [Int: [PluginCellValue]], deletedRowIndices: Set, insertedRowIndices: Set - ) -> [(statement: String, parameters: [PluginCellValue])] { - var statements: [(statement: String, parameters: [PluginCellValue])] = [] - var deleteKeys: [String] = [] + ) throws -> [PluginRowWrite] { + var writes: [PluginRowWrite] = [] + var deletions: [(key: String, rowIndex: Int)] = [] for change in changes { switch change.type { case .insert: guard insertedRowIndices.contains(change.rowIndex) else { continue } - statements += generateInsert(for: change, insertedRowData: insertedRowData) + writes += try insertCommands(for: change, insertedRowData: insertedRowData) + .map { PluginRowWrite(statement: $0, rowIndices: [change.rowIndex]) } case .update: - statements += generateUpdate(for: change) + writes += try updateCommands(for: change) + .map { PluginRowWrite(statement: $0, rowIndices: [change.rowIndex]) } case .delete: guard deletedRowIndices.contains(change.rowIndex) else { continue } - if let key = extractKey(from: change) { - deleteKeys.append(key) + guard let key = extractKey(from: change) else { + throw PluginRowWriteRefusal(rowIndex: change.rowIndex, reason: Self.unaddressableKeyReason) } + deletions.append((key: key, rowIndex: change.rowIndex)) } } - return statements + deleteStatements(for: deleteKeys) + return writes + deleteWrites(for: deletions) } - private func deleteStatements(for keys: [String]) -> [(statement: String, parameters: [PluginCellValue])] { - guard !keys.isEmpty else { return [] } - let batches: [[String]] + private func deleteWrites(for deletions: [(key: String, rowIndex: Int)]) -> [PluginRowWrite] { + guard !deletions.isEmpty else { return [] } + let batches: [[(key: String, rowIndex: Int)]] switch deleteBatching { - case .singleCommand: batches = [keys] - case .perHashSlot: batches = RedisKeySlot.groupedBySlot(keys) + case .singleCommand: batches = [deletions] + case .perHashSlot: batches = RedisKeySlot.groupedBySlot(deletions) { $0.key } } return batches.map { batch in - let keyList = batch.map { RedisArgumentCodec.quote($0) }.joined(separator: " ") - return (statement: "DEL \(keyList)", parameters: []) + let keyList = batch.map { RedisArgumentCodec.quote($0.key) }.joined(separator: " ") + return PluginRowWrite(statement: "DEL \(keyList)", rowIndices: batch.map { $0.rowIndex }) } } // MARK: - INSERT - private func generateInsert( + private func insertCommands( for change: PluginRowChange, insertedRowData: [Int: [PluginCellValue]] - ) -> [(statement: String, parameters: [PluginCellValue])] { - var statements: [(statement: String, parameters: [PluginCellValue])] = [] - + ) throws -> [String] { var key: String? var value: String? var type: String? - var ttl: Int? + var ttlText: String? if let values = insertedRowData[change.rowIndex] { if let ki = keyColumnIndex, ki < values.count { @@ -115,8 +117,8 @@ struct RedisStatementGenerator { if let vi = valueColumnIndex, vi < values.count { value = Self.encodedArgument(values[vi]) } - if let ttli = ttlColumnIndex, ttli < values.count, let ttlStr = values[ttli].asText { - ttl = Int(ttlStr) + if let ttli = ttlColumnIndex, ttli < values.count { + ttlText = values[ttli].asText } } else { for cellChange in change.cellChanges { @@ -124,38 +126,50 @@ struct RedisStatementGenerator { case "Key": key = cellChange.newValue.asText case "Type": type = cellChange.newValue.asText case "Value": value = Self.encodedArgument(cellChange.newValue) - case "TTL": - if let ttlStr = cellChange.newValue.asText { ttl = Int(ttlStr) } + case "TTL": ttlText = cellChange.newValue.asText default: break } } } guard let k = key, !k.isEmpty else { - Self.logger.warning("Skipping INSERT for namespace '\(self.namespaceName)' - no key") - return [] + throw PluginRowWriteRefusal(rowIndex: change.rowIndex, reason: String(localized: "A new key needs a name.")) } - let v = value ?? RedisArgumentCodec.quote("") - let cmd = generateInsertCommand(key: k, encodedValue: v, type: type?.lowercased()) - statements.append((statement: cmd, parameters: [])) + let typeName = type?.lowercased() ?? "string" + guard typeName.isEmpty || Self.insertableTypes.contains(typeName) else { + throw PluginRowWriteRefusal( + rowIndex: change.rowIndex, + reason: String( + format: String(localized: "A %@ key cannot be added from the grid. Add it with a command in the query editor."), + typeName + ) + ) + } - if let ttlSeconds = ttl, ttlSeconds > 0 { - let expireCmd = "EXPIRE \(RedisArgumentCodec.quote(k)) \(ttlSeconds)" - statements.append((statement: expireCmd, parameters: [])) + var ttl: Int? + if let ttlText { + guard let seconds = Int(ttlText), seconds >= 0 || seconds == -1 else { + throw PluginRowWriteRefusal(rowIndex: change.rowIndex, reason: Self.invalidTTLReason) + } + ttl = seconds } - return statements + var commands = [generateInsertCommand(key: k, encodedValue: value ?? RedisArgumentCodec.quote(""), type: typeName)] + if let ttlSeconds = ttl, ttlSeconds > 0 { + commands.append("EXPIRE \(RedisArgumentCodec.quote(k)) \(ttlSeconds)") + } + return commands } /// Generate the appropriate Redis command based on the data type - private func generateInsertCommand(key: String, encodedValue: String, type: String?) -> String { + private func generateInsertCommand(key: String, encodedValue: String, type: String) -> String { let quotedKey = RedisArgumentCodec.quote(key) switch type { case "hash": if let fields = Self.hashFields(fromEncoded: encodedValue) { - return fields.reduce("HSET \(quotedKey)") { command, field in - command + " \(RedisArgumentCodec.quote(field.name)) \(RedisArgumentCodec.quote(field.value))" + return fields.reduce(into: "HSET \(quotedKey)") { command, field in + command += " \(RedisArgumentCodec.quote(field.name)) \(RedisArgumentCodec.quote(field.value))" } } return "HSET \(quotedKey) value \(encodedValue)" @@ -182,65 +196,92 @@ struct RedisStatementGenerator { // MARK: - UPDATE - private func generateUpdate(for change: PluginRowChange) -> [(statement: String, parameters: [PluginCellValue])] { + private func updateCommands(for change: PluginRowChange) throws -> [String] { guard !change.cellChanges.isEmpty else { return [] } guard let key = extractKey(from: change) else { - Self.logger.warning("Skipping UPDATE for namespace '\(self.namespaceName)' - no key value") - return [] + throw PluginRowWriteRefusal(rowIndex: change.rowIndex, reason: Self.unaddressableKeyReason) } - var statements: [(statement: String, parameters: [PluginCellValue])] = [] - - if let keyChange = change.cellChanges.first(where: { $0.columnName == "Key" }), - let newKey = keyChange.newValue.asText, newKey != key { - let renameCmd = "RENAME \(RedisArgumentCodec.quote(key)) \(RedisArgumentCodec.quote(newKey))" - statements.append((statement: renameCmd, parameters: [])) - } + var commands: [String] = [] + var effectiveKey = key - let effectiveKey: String = { - if let keyChange = change.cellChanges.first(where: { $0.columnName == "Key" }), - let newKey = keyChange.newValue.asText { - return newKey + if let keyChange = change.cellChanges.first(where: { $0.columnName == "Key" }) { + guard let newKey = keyChange.newValue.asText else { + throw PluginRowWriteRefusal( + rowIndex: change.rowIndex, reason: String(localized: "A key can only be renamed to text.") + ) } - return key - }() - - let valueType = valueWriteType(of: change) + if newKey != key { + commands.append("RENAME \(RedisArgumentCodec.quote(key)) \(RedisArgumentCodec.quote(newKey))") + } + effectiveKey = newKey + } for cellChange in change.cellChanges { switch cellChange.columnName { case "Key": - continue // Already handled above + continue case "Value": - guard let encodedValue = Self.encodedArgument(cellChange.newValue) else { continue } - guard let typeLower = valueType else { - Self.logger.warning("Skipping Value update for key '\(effectiveKey)' - its type is unknown") - continue - } - if typeLower != "string" { - // Non-string types show a preview; blindly SET would destroy the data structure - Self.logger.warning( - "Skipping Value update for \(typeLower) key '\(effectiveKey)' - use query editor" - ) - continue - } - let cmd = "SET \(RedisArgumentCodec.quote(effectiveKey)) \(encodedValue)" - statements.append((statement: cmd, parameters: [])) + commands.append(try valueCommand(setting: cellChange.newValue, of: change, key: effectiveKey)) case "TTL": - if let ttlStr = cellChange.newValue.asText, let ttlSeconds = Int(ttlStr), ttlSeconds > 0 { - let cmd = "EXPIRE \(RedisArgumentCodec.quote(effectiveKey)) \(ttlSeconds)" - statements.append((statement: cmd, parameters: [])) - } else if cellChange.newValue.isNull || cellChange.newValue.asText == "-1" { - let cmd = "PERSIST \(RedisArgumentCodec.quote(effectiveKey))" - statements.append((statement: cmd, parameters: [])) - } + commands.append(try ttlCommand(setting: cellChange.newValue, of: change, key: effectiveKey)) default: - break + throw PluginRowWriteRefusal( + rowIndex: change.rowIndex, + reason: String(format: String(localized: "'%@' cannot be changed from the grid."), cellChange.columnName) + ) } } - return statements + return commands + } + + /// Only a string's value is the whole of what the grid shows. A collection shows a preview, + /// and a `SET` over it would replace the structure with that text. + private func valueCommand(setting newValue: PluginCellValue, of change: PluginRowChange, key: String) throws -> String { + guard let encodedValue = Self.encodedArgument(newValue) else { + throw PluginRowWriteRefusal( + rowIndex: change.rowIndex, + reason: String(localized: "Redis cannot store NULL as a value. Enter an empty value instead.") + ) + } + guard let typeName = valueWriteType(of: change) else { + throw PluginRowWriteRefusal( + rowIndex: change.rowIndex, + reason: String(localized: "The key's type is unknown, so its value cannot be written safely.") + ) + } + guard typeName == "string" else { + throw PluginRowWriteRefusal( + rowIndex: change.rowIndex, + reason: String( + format: String(localized: "The value of a %@ key cannot be edited in the grid. Change it with a command in the query editor."), + typeName + ) + ) + } + return "SET \(RedisArgumentCodec.quote(key)) \(encodedValue)" + } + + private func ttlCommand(setting newValue: PluginCellValue, of change: PluginRowChange, key: String) throws -> String { + if newValue.isNull || newValue.asText == "-1" { + return "PERSIST \(RedisArgumentCodec.quote(key))" + } + guard let text = newValue.asText, let seconds = Int(text), seconds > 0 else { + throw PluginRowWriteRefusal(rowIndex: change.rowIndex, reason: Self.invalidTTLReason) + } + return "EXPIRE \(RedisArgumentCodec.quote(key)) \(seconds)" + } + + // MARK: - Refusals + + private static var unaddressableKeyReason: String { + String(localized: "This key's name is not text, so it cannot be addressed from the grid.") + } + + private static var invalidTTLReason: String { + String(localized: "TTL has to be a whole number of seconds above 0, or -1 or NULL for no expiry.") } // MARK: - Helpers diff --git a/Plugins/TableProPluginKit/PluginDatabaseDriver.swift b/Plugins/TableProPluginKit/PluginDatabaseDriver.swift index 109867ce7f..025fd287a2 100644 --- a/Plugins/TableProPluginKit/PluginDatabaseDriver.swift +++ b/Plugins/TableProPluginKit/PluginDatabaseDriver.swift @@ -298,6 +298,22 @@ public protocol PluginDatabaseDriver: AnyObject, Sendable { func generateStatements(table: String, columns: [String], primaryKeyColumns: [String], changes: [PluginRowChange], insertedRowData: [Int: [PluginCellValue]], deletedRowIndices: Set, insertedRowIndices: Set) -> [(statement: String, parameters: [PluginCellValue])]? func generateStatements(table: String, schema: String?, columns: [String], primaryKeyColumns: [String], changes: [PluginRowChange], insertedRowData: [Int: [PluginCellValue]], deletedRowIndices: Set, insertedRowIndices: Set) -> [(statement: String, parameters: [PluginCellValue])]? + /// The statements that write a save's changes, each naming the changes it writes. + /// + /// This is what the host calls; `generateStatements` stays for drivers built before it. Return + /// nil to have the host generate SQL itself. Throw `PluginRowWriteRefusal` for a change, or a + /// value in one, that this driver cannot write, and never leave it out: the host refuses a save + /// in which a pending change is named by no statement. An update with no cell changes has + /// nothing to write and is not a refusal. + /// + /// The default runs `generateStatements` on the whole set and returns those statements + /// unchanged. It then runs it once per change to learn which changes it writes, and the first + /// statement names all of them, since the default cannot tell which statement writes which. + /// It holds a driver to every change, not to every value in one: a change that produces any + /// statement counts as written. A driver that can leave out one value of a change it still + /// writes implements this requirement and refuses that change instead. + func generateRowWrites(table: String, schema: String?, columns: [String], primaryKeyColumns: [String], changes: [PluginRowChange], insertedRowData: [Int: [PluginCellValue]], deletedRowIndices: Set, insertedRowIndices: Set) throws -> [PluginRowWrite]? + /// Writes a row back exactly as it was, key included, to undo a delete. /// /// `generateStatements` writes an insert for a row the user just added, so it is free to let @@ -952,6 +968,30 @@ public extension PluginDatabaseDriver { insertedRowData: insertedRowData, deletedRowIndices: deletedRowIndices, insertedRowIndices: insertedRowIndices ) } + func generateRowWrites(table: String, schema: String?, columns: [String], primaryKeyColumns: [String], changes: [PluginRowChange], insertedRowData: [Int: [PluginCellValue]], deletedRowIndices: Set, insertedRowIndices: Set) throws -> [PluginRowWrite]? { + guard let statements = generateStatements( + table: table, schema: schema, columns: columns, primaryKeyColumns: primaryKeyColumns, changes: changes, + insertedRowData: insertedRowData, deletedRowIndices: deletedRowIndices, insertedRowIndices: insertedRowIndices + ) else { return nil } + let writtenRows = changes.compactMap { change -> Int? in + let rowIndex = change.rowIndex + let solo = generateStatements( + table: table, schema: schema, columns: columns, primaryKeyColumns: primaryKeyColumns, + changes: [change], + insertedRowData: insertedRowData[rowIndex].map { [rowIndex: $0] } ?? [:], + deletedRowIndices: deletedRowIndices.contains(rowIndex) ? [rowIndex] : [], + insertedRowIndices: insertedRowIndices.contains(rowIndex) ? [rowIndex] : [] + ) + return solo?.isEmpty == false ? rowIndex : nil + } + return statements.enumerated().map { offset, statement in + PluginRowWrite( + statement: statement.statement, + parameters: statement.parameters, + rowIndices: offset == 0 ? writtenRows : [] + ) + } + } func generateIdentityPreservingInsert(table: String, schema: String?, columns: [String], primaryKeyColumns: [String], rows: [[PluginCellValue]]) -> [(statement: String, parameters: [PluginCellValue])]? { nil } func generateAddColumnSQL(table: String, column: PluginColumnDefinition) -> String? { nil } diff --git a/Plugins/TableProPluginKit/PluginRowWrite.swift b/Plugins/TableProPluginKit/PluginRowWrite.swift new file mode 100644 index 0000000000..7ec8ea1d01 --- /dev/null +++ b/Plugins/TableProPluginKit/PluginRowWrite.swift @@ -0,0 +1,44 @@ +// +// PluginRowWrite.swift +// TableProPluginKit +// + +import Foundation + +/// One statement a driver writes for a save, and the changes it carries out. +/// +/// `rowIndices` holds the `PluginRowChange.rowIndex` of every change the statement writes. The +/// host refuses a save when a pending change is named by no statement, so a statement that writes +/// several changes, such as one delete for many rows, names all of them. +/// +/// Deliberately not `@frozen`, so it can gain a field later. Any new field arrives with its own +/// initializer overload; this one keeps its signature, because changing it would replace the +/// mangled symbol every already-built plugin references. +public struct PluginRowWrite: Sendable { + public let statement: String + public let parameters: [PluginCellValue] + public let rowIndices: [Int] + + public init(statement: String, parameters: [PluginCellValue] = [], rowIndices: [Int]) { + self.statement = statement + self.parameters = parameters + self.rowIndices = rowIndices + } +} + +/// A change the driver cannot write, with the reason in the user's language. +/// +/// Thrown from `generateRowWrites` in place of leaving the change out. The host then sends nothing, +/// keeps every change pending, and shows `reason`, so it should say what is wrong with the change +/// rather than restate that it failed. +public struct PluginRowWriteRefusal: Error, LocalizedError, Sendable, Equatable { + public let rowIndex: Int + public let reason: String + + public init(rowIndex: Int, reason: String) { + self.rowIndex = rowIndex + self.reason = reason + } + + public var errorDescription: String? { reason } +} diff --git a/TablePro/Core/ChangeTracking/DataChangeManager.swift b/TablePro/Core/ChangeTracking/DataChangeManager.swift index 6b81cc5f21..6326fa938e 100644 --- a/TablePro/Core/ChangeTracking/DataChangeManager.swift +++ b/TablePro/Core/ChangeTracking/DataChangeManager.swift @@ -566,13 +566,15 @@ final class DataChangeManager: ObservableObject, ChangeManaging { containsTableOperation: containsTableOperation ) - if let attributed = try factory.attributedStatements( + let steps: [DataWriteStep] + switch try factory.rowWriteStatements( for: pending.changes, insertedRowData: pending.insertedRowData, deletedRowIDs: pending.deletedRowIDs, insertedRowIDs: pending.insertedRowIDs ) { - let steps = attributed.map { + case .counted(let attributed): + steps = attributed.map { DataWriteStep( kind: .rowWrite, statement: $0.statement, @@ -581,16 +583,10 @@ final class DataChangeManager: ObservableObject, ChangeManaging { matchesRowsWithoutKey: primaryKeyColumns.isEmpty && $0.kind != .insert ) } - return RowWriteBuild(steps: steps, operations: operations) - } - - let steps = try factory.statements( - for: pending.changes, - insertedRowData: pending.insertedRowData, - deletedRowIDs: pending.deletedRowIDs, - insertedRowIDs: pending.insertedRowIDs - ).map { - DataWriteStep(kind: .rowWrite, statement: $0, expectedRowCount: nil, tableName: tableName) + case .driverWritten(let statements): + steps = statements.map { + DataWriteStep(kind: .rowWrite, statement: $0, expectedRowCount: nil, tableName: tableName) + } } return RowWriteBuild(steps: steps, operations: operations) } diff --git a/TablePro/Core/ChangeTracking/SQLStatementGenerator.swift b/TablePro/Core/ChangeTracking/SQLStatementGenerator.swift index 702700018b..aac1e03424 100644 --- a/TablePro/Core/ChangeTracking/SQLStatementGenerator.swift +++ b/TablePro/Core/ChangeTracking/SQLStatementGenerator.swift @@ -16,11 +16,14 @@ struct ParameterizedStatement: @unchecked Sendable { let parameters: [Any?] } -/// A statement plus the number of rows it is meant to touch, so a caller can hold the server to it. +/// A statement plus the rows it is meant to touch, so a caller can hold the server to that count +/// and tell a change that became a statement from one that did not. struct AttributedStatement: @unchecked Sendable { let statement: ParameterizedStatement let kind: RowWriteKind - let rowCount: Int + let rowIDs: [RowID] + + var rowCount: Int { rowIDs.count } } /// Generates SQL statements from data changes @@ -135,14 +138,14 @@ struct SQLStatementGenerator { case .update: flushDeleteRun() if let stmt = generateUpdateSQL(for: change) { - statements.append(AttributedStatement(statement: stmt, kind: .update, rowCount: 1)) + statements.append(AttributedStatement(statement: stmt, kind: .update, rowIDs: [change.rowID])) } case .insert: // SAFETY: Verify the row is still marked as inserted guard insertedRowIDs.contains(change.rowID) else { continue } flushDeleteRun() if let stmt = generateInsertSQL(for: change, insertedRowData: insertedRowData) { - statements.append(AttributedStatement(statement: stmt, kind: .insert, rowCount: 1)) + statements.append(AttributedStatement(statement: stmt, kind: .insert, rowIDs: [change.rowID])) } case .delete: // SAFETY: Verify the row is still marked as deleted @@ -414,29 +417,33 @@ struct SQLStatementGenerator { } private func generateDeleteStatements(for changes: [RowChange]) -> [AttributedStatement] { - let rowMatches = changes.compactMap { deleteRowMatches(for: $0) } + let rowMatches = changes.compactMap { change in + deleteRowMatches(for: change).map { (rowID: change.rowID, matches: $0) } + } guard !rowMatches.isEmpty else { return [] } var statements: [AttributedStatement] = [] - var chunk: [[DeleteColumnMatch]] = [] + var chunk: [(rowID: RowID, matches: [DeleteColumnMatch])] = [] var chunkParameterCount = 0 func flush() { statements.append( - AttributedStatement(statement: deleteStatement(for: chunk), kind: .delete, rowCount: chunk.count) + AttributedStatement( + statement: deleteStatement(for: chunk.map(\.matches)), kind: .delete, rowIDs: chunk.map(\.rowID) + ) ) } let matchesOneRowPerStatement = primaryKeyColumns.isEmpty && !rowMatchPolicy.excludedColumns.isEmpty - for matches in rowMatches { - let rowParameterCount = matches.count(where: { $0.boundValue != nil }) + for row in rowMatches { + let rowParameterCount = row.matches.count(where: { $0.boundValue != nil }) if !chunk.isEmpty, matchesOneRowPerStatement || chunkParameterCount + rowParameterCount > maxBindParameters { flush() chunk = [] chunkParameterCount = 0 } - chunk.append(matches) + chunk.append(row) chunkParameterCount += rowParameterCount } diff --git a/TablePro/Core/Coordinators/RowEditingCoordinator+SaveChanges.swift b/TablePro/Core/Coordinators/RowEditingCoordinator+SaveChanges.swift index afe307638e..ba192228ec 100644 --- a/TablePro/Core/Coordinators/RowEditingCoordinator+SaveChanges.swift +++ b/TablePro/Core/Coordinators/RowEditingCoordinator+SaveChanges.swift @@ -58,7 +58,11 @@ extension RowEditingCoordinator { tableOperationOptions: tableOperationOptions ) } catch { - failSave(message: error.localizedDescription) + failSave( + message: [error.localizedDescription, (error as? DataWriteError)?.recoverySuggestion] + .compactMap { $0 } + .joined(separator: " ") + ) return } diff --git a/TablePro/Core/DataWrite/DataWriteError.swift b/TablePro/Core/DataWrite/DataWriteError.swift index 03c5d3b1e7..4daec477fa 100644 --- a/TablePro/Core/DataWrite/DataWriteError.swift +++ b/TablePro/Core/DataWrite/DataWriteError.swift @@ -10,6 +10,8 @@ enum DataWriteError: LocalizedError, Equatable { case statementGenerationFailed(String) case objectOperationUnsupported(String) case rowsNotIdentifiable(String, RowWriteKind) + case changeRefused(table: String, kind: RowWriteKind?, reason: String) + case changesNotWritable(table: String, unwritten: UnwrittenRowCounts) case identityNotPreservable(String) case tooManyRowsAffected(table: String, expected: Int, actual: Int) case tooManyRowsAffectedUnrecoverable(table: String, expected: Int, actual: Int) @@ -51,6 +53,11 @@ enum DataWriteError: LocalizedError, Equatable { table ) } + case .changeRefused(let table, let kind, let reason): + return String(format: Self.refusalFormat(for: kind), table, reason) + case .changesNotWritable(let table, let unwritten): + let lead = String(format: String(localized: "Cannot save changes to '%@'."), table) + return ([lead] + unwritten.sentences).joined(separator: " ") case .identityNotPreservable(let engine): return String( format: String(localized: "%@ cannot restore a deleted row with its original key."), @@ -121,8 +128,29 @@ enum DataWriteError: LocalizedError, Equatable { ) case .identityNotPreservable: return String(localized: "Restore the row by inserting it again, then check anything that referenced its key.") + case .rowsNotIdentifiable(_, .insert): + return String( + localized: "This database has no statement for a new row that sets no column. Enter a value in at least one column, or insert the row with a query." + ) + case .changeRefused, .changesNotWritable: + return String( + localized: "Nothing was saved, and every change is still pending. Undo what cannot be written, or make it with a query, then save again." + ) default: return nil } } + + private static func refusalFormat(for kind: RowWriteKind?) -> String { + switch kind { + case .insert: + return String(localized: "Cannot save the new row in '%1$@'. %2$@") + case .update: + return String(localized: "Cannot save the edited row in '%1$@'. %2$@") + case .delete: + return String(localized: "Cannot delete the row in '%1$@'. %2$@") + case nil: + return String(localized: "Cannot save changes to '%1$@'. %2$@") + } + } } diff --git a/TablePro/Core/DataWrite/RowChangeStatementFactory.swift b/TablePro/Core/DataWrite/RowChangeStatementFactory.swift index 885e78bab1..58269b0b68 100644 --- a/TablePro/Core/DataWrite/RowChangeStatementFactory.swift +++ b/TablePro/Core/DataWrite/RowChangeStatementFactory.swift @@ -7,12 +7,21 @@ // This used to live inside DataChangeManager, which is @MainActor and @Observable and owns the // undo stack, so nothing but a live edit session could ask for a statement. Data Rewind needs // the same answer for a change set it read back from disk, so the generation moved here and the -// manager delegates. A plugin that overrides generateStatements therefore serves both paths. +// manager delegates. A plugin that implements generateRowWrites therefore serves both paths, and +// both are held to RowWriteCoverage: a pending change no statement writes refuses the save. // import Foundation import TableProPluginKit +/// The statements for a set of row changes, by who wrote them. +enum RowWriteStatements { + /// The host's, each carrying the rows it should touch. + case counted([AttributedStatement]) + /// A driver's, which carry no count the host can hold the server to. + case driverWritten([ParameterizedStatement]) +} + @MainActor struct RowChangeStatementFactory { let tableName: String @@ -50,49 +59,48 @@ struct RowChangeStatementFactory { deletedRowIDs: Set = [], insertedRowIDs: Set = [] ) throws -> [ParameterizedStatement] { - if let pluginStatements = pluginGeneratedStatements( + switch try rowWriteStatements( for: changes, insertedRowData: insertedRowData, deletedRowIDs: deletedRowIDs, insertedRowIDs: insertedRowIDs ) { - return pluginStatements + case .counted(let statements): + return statements.map(\.statement) + case .driverWritten(let statements): + return statements } - return try attributedHostStatements( - for: changes, - insertedRowData: insertedRowData, - deletedRowIDs: deletedRowIDs, - insertedRowIDs: insertedRowIDs - ).map(\.statement) } - /// The host generator's statements, each with the number of rows it should touch. + /// Every statement the changes need, or a throw naming the changes that would be left out. /// - /// Returns nil when the driver writes its own statements, because nothing then tells the host - /// which rows went into which statement and a guessed count is worse than no count. - func attributedStatements( + /// The host's statements carry the rows they touch, so the executor can hold the server to that + /// count. A driver's do not, because nothing tells the host how many rows its statements reach. + func rowWriteStatements( for changes: [RowChange], insertedRowData: [RowID: [PluginCellValue]] = [:], deletedRowIDs: Set = [], insertedRowIDs: Set = [] - ) throws -> [AttributedStatement]? { - if pluginGeneratedStatements( + ) throws -> RowWriteStatements { + if let driverStatements = try pluginRowWrites( for: changes, insertedRowData: insertedRowData, deletedRowIDs: deletedRowIDs, insertedRowIDs: insertedRowIDs - ) != nil { - return nil + ) { + return .driverWritten(driverStatements) } - return try attributedHostStatements( - for: changes, - insertedRowData: insertedRowData, - deletedRowIDs: deletedRowIDs, - insertedRowIDs: insertedRowIDs + return .counted( + try hostStatements( + for: changes, + insertedRowData: insertedRowData, + deletedRowIDs: deletedRowIDs, + insertedRowIDs: insertedRowIDs + ) ) } - private func attributedHostStatements( + private func hostStatements( for changes: [RowChange], insertedRowData: [RowID: [PluginCellValue]], deletedRowIDs: Set, @@ -104,11 +112,14 @@ struct RowChangeStatementFactory { deletedRowIDs: deletedRowIDs, insertedRowIDs: insertedRowIDs ) - try validate(statements.map(\.statement), against: changes, deletedRowIDs: deletedRowIDs) - let deletableCount = changes.count { $0.type == .delete && deletedRowIDs.contains($0.rowID) } - let identifiedDeletes = statements.filter { $0.kind == .delete }.reduce(0) { $0 + $1.rowCount } - if identifiedDeletes < deletableCount { - throw DataWriteError.rowsNotIdentifiable(tableName, .delete) + let unwritten = RowWriteCoverage.unwrittenChanges( + changes, + deletedRowIDs: deletedRowIDs, + insertedRowIDs: insertedRowIDs, + writtenRowIDs: Set(statements.flatMap(\.rowIDs)) + ) + if let kind = UnwrittenRowCounts(unwritten).leadingKind { + throw DataWriteError.rowsNotIdentifiable(tableName, kind) } return statements } @@ -158,21 +169,33 @@ struct RowChangeStatementFactory { /// True when the engine's statements come from the plugin rather than from /// `SQLStatementGenerator`, which is what decides whether the host may fall back. + /// + /// A driver that throws on the probe is refusing a change it owns, so it still owns the engine. var pluginOwnsStatementGeneration: Bool { - pluginGeneratedStatements( - for: [RowChange(rowID: .existing(0), type: .update, cellChanges: [], originalRow: nil)], - insertedRowData: [:], - deletedRowIDs: [], - insertedRowIDs: [] - ) != nil + guard let pluginDriver else { return false } + let probe = PluginRowChange(rowIndex: 0, type: .update, cellChanges: [], originalRow: nil) + do { + return try pluginDriver.generateRowWrites( + table: tableName, + schema: schemaName, + columns: columns, + primaryKeyColumns: primaryKeyColumns, + changes: [probe], + insertedRowData: [:], + deletedRowIndices: [], + insertedRowIndices: [] + ) != nil + } catch { + return true + } } - private func pluginGeneratedStatements( + private func pluginRowWrites( for changes: [RowChange], insertedRowData: [RowID: [PluginCellValue]], deletedRowIDs: Set, insertedRowIDs: Set - ) -> [ParameterizedStatement]? { + ) throws -> [ParameterizedStatement]? { guard let pluginDriver else { return nil } let keyed = PluginKeyedChanges( changes: changes, @@ -180,17 +203,37 @@ struct RowChangeStatementFactory { deletedRowIDs: deletedRowIDs, insertedRowIDs: insertedRowIDs ) - guard let statements = pluginDriver.generateStatements( - table: tableName, - schema: schemaName, - columns: columns, - primaryKeyColumns: primaryKeyColumns, - changes: keyed.changes, - insertedRowData: keyed.insertedRowData, - deletedRowIndices: keyed.deletedRowIndices, - insertedRowIndices: keyed.insertedRowIndices - ) else { return nil } - return statements.map { + let writes: [PluginRowWrite] + do { + guard let generated = try pluginDriver.generateRowWrites( + table: tableName, + schema: schemaName, + columns: columns, + primaryKeyColumns: primaryKeyColumns, + changes: keyed.changes, + insertedRowData: keyed.insertedRowData, + deletedRowIndices: keyed.deletedRowIndices, + insertedRowIndices: keyed.insertedRowIndices + ) else { return nil } + writes = generated + } catch let refusal as PluginRowWriteRefusal { + throw DataWriteError.changeRefused( + table: tableName, kind: keyed.writeKind(ofRowIndex: refusal.rowIndex), reason: refusal.reason + ) + } catch { + throw DataWriteError.changeRefused(table: tableName, kind: nil, reason: error.localizedDescription) + } + + let unwritten = RowWriteCoverage.unwrittenChanges( + changes, + deletedRowIDs: deletedRowIDs, + insertedRowIDs: insertedRowIDs, + writtenRowIDs: Set(writes.flatMap(\.rowIndices).compactMap(keyed.rowID(forIndex:))) + ) + guard unwritten.isEmpty else { + throw DataWriteError.changesNotWritable(table: tableName, unwritten: UnwrittenRowCounts(unwritten)) + } + return writes.map { ParameterizedStatement(sql: $0.statement, parameters: $0.parameters.map(\.asAny)) } } @@ -210,23 +253,6 @@ struct RowChangeStatementFactory { quoteIdentifier: pluginDriver?.quoteIdentifier ) } - - private func validate( - _ statements: [ParameterizedStatement], - against changes: [RowChange], - deletedRowIDs: Set - ) throws { - let expectedUpdates = changes.count(where: { $0.type == .update }) - let actualUpdates = statements.count(where: { $0.sql.hasPrefix("UPDATE") }) - if expectedUpdates > 0, actualUpdates < expectedUpdates { - throw DataWriteError.rowsNotIdentifiable(tableName, .update) - } - - let deletable = changes.filter { $0.type == .delete && deletedRowIDs.contains($0.rowID) } - if !deletable.isEmpty, deletable.allSatisfy({ $0.originalRow == nil }) { - throw DataWriteError.rowsNotIdentifiable(tableName, .delete) - } - } } struct PluginKeyedChanges { @@ -234,6 +260,8 @@ struct PluginKeyedChanges { let insertedRowData: [Int: [PluginCellValue]] let deletedRowIndices: Set let insertedRowIndices: Set + /// The row each key stands for, indexed by key. + let rowIDs: [RowID] init( changes: [RowChange], @@ -242,9 +270,12 @@ struct PluginKeyedChanges { insertedRowIDs: Set ) { var keys: [RowID: Int] = [:] + var rowIDs: [RowID] = [] for change in changes where keys[change.rowID] == nil { - keys[change.rowID] = keys.count + keys[change.rowID] = rowIDs.count + rowIDs.append(change.rowID) } + self.rowIDs = rowIDs self.changes = changes.compactMap { change in keys[change.rowID].map { PluginRowChange(change, key: $0) } } @@ -256,6 +287,20 @@ struct PluginKeyedChanges { self.deletedRowIndices = Set(deletedRowIDs.compactMap { keys[$0] }) self.insertedRowIndices = Set(insertedRowIDs.compactMap { keys[$0] }) } + + /// The row a driver's `rowIndex` names, or nil when it names none of these changes. + func rowID(forIndex index: Int) -> RowID? { + rowIDs.indices.contains(index) ? rowIDs[index] : nil + } + + func writeKind(ofRowIndex index: Int) -> RowWriteKind? { + guard let change = changes.first(where: { $0.rowIndex == index }) else { return nil } + switch change.type { + case .insert: return .insert + case .update: return .update + case .delete: return .delete + } + } } private extension PluginRowChange { diff --git a/TablePro/Core/DataWrite/RowWriteCoverage.swift b/TablePro/Core/DataWrite/RowWriteCoverage.swift new file mode 100644 index 0000000000..fef6c92260 --- /dev/null +++ b/TablePro/Core/DataWrite/RowWriteCoverage.swift @@ -0,0 +1,95 @@ +// +// RowWriteCoverage.swift +// TablePro +// +// Which pending changes no statement writes. +// +// A save clears the queue and the undo stack once it commits, so a change that never became a +// statement is lost the moment the rest of the save succeeds. Every generator, the host's and a +// driver's, is held to the same rule before anything runs: each pending change is written, or the +// save is refused and nothing is sent. +// + +import Foundation + +enum RowWriteCoverage { + /// Whether a change still has something to write: an update with a cell to set, a row still + /// marked as inserted, or a row still marked as deleted. + static func isPending(_ change: RowChange, deletedRowIDs: Set, insertedRowIDs: Set) -> Bool { + switch change.type { + case .update: return !change.cellChanges.isEmpty + case .insert: return insertedRowIDs.contains(change.rowID) + case .delete: return deletedRowIDs.contains(change.rowID) + } + } + + static func unwrittenChanges( + _ changes: [RowChange], + deletedRowIDs: Set, + insertedRowIDs: Set, + writtenRowIDs: Set + ) -> [RowChange] { + changes.filter { change in + isPending(change, deletedRowIDs: deletedRowIDs, insertedRowIDs: insertedRowIDs) + && !writtenRowIDs.contains(change.rowID) + } + } +} + +/// How many changes of each kind a save could not write, which is what the refusal names. +struct UnwrittenRowCounts: Equatable, Sendable { + let updates: Int + let inserts: Int + let deletes: Int + + init(updates: Int = 0, inserts: Int = 0, deletes: Int = 0) { + self.updates = updates + self.inserts = inserts + self.deletes = deletes + } + + init(_ changes: [RowChange]) { + self.init( + updates: changes.count { $0.type == .update }, + inserts: changes.count { $0.type == .insert }, + deletes: changes.count { $0.type == .delete } + ) + } + + var isEmpty: Bool { updates == 0 && inserts == 0 && deletes == 0 } + + /// The kind a refusal leads with, in the order the host's messages have always used. + var leadingKind: RowWriteKind? { + if updates > 0 { return .update } + if deletes > 0 { return .delete } + if inserts > 0 { return .insert } + return nil + } + + /// One sentence per kind, naming how many of that kind cannot be written. + var sentences: [String] { + var sentences: [String] = [] + if updates > 0 { + sentences.append( + updates == 1 + ? String(localized: "The driver cannot write an edited row.") + : String(format: String(localized: "The driver cannot write %lld edited rows."), updates) + ) + } + if inserts > 0 { + sentences.append( + inserts == 1 + ? String(localized: "The driver cannot write a new row.") + : String(format: String(localized: "The driver cannot write %lld new rows."), inserts) + ) + } + if deletes > 0 { + sentences.append( + deletes == 1 + ? String(localized: "The driver cannot delete a row marked for deletion.") + : String(format: String(localized: "The driver cannot delete %lld rows marked for deletion."), deletes) + ) + } + return sentences + } +} diff --git a/TablePro/Core/Plugins/PluginDriverAdapter.swift b/TablePro/Core/Plugins/PluginDriverAdapter.swift index a5e2bfe871..2cabe5e355 100644 --- a/TablePro/Core/Plugins/PluginDriverAdapter.swift +++ b/TablePro/Core/Plugins/PluginDriverAdapter.swift @@ -32,27 +32,6 @@ final class PluginDriverAdapter: DatabaseDriver, SchemaSwitchable, DatabaseRepor var sessionLexicalState: PluginSessionLexicalState? { pluginDriver.sessionLexicalState } var parameterStyle: ParameterStyle { pluginDriver.parameterStyle } - func pluginGenerateStatements( - table: String, - columns: [String], - primaryKeyColumns: [String], - changes: [PluginRowChange], - insertedRowData: [Int: [String?]], - deletedRowIndices: Set, - insertedRowIndices: Set - ) -> [(statement: String, parameters: [String?])]? { - let pluginRowData = insertedRowData.mapValues { row in - row.map(PluginCellValue.fromOptional) - } - let result = pluginDriver.generateStatements( - table: table, columns: columns, primaryKeyColumns: primaryKeyColumns, changes: changes, - insertedRowData: pluginRowData, - deletedRowIndices: deletedRowIndices, - insertedRowIndices: insertedRowIndices - ) - return result?.map { (statement: $0.statement, parameters: $0.parameters.map { $0.asText }) } - } - /// The underlying plugin driver, exposed for DDL schema generation delegation. var schemaPluginDriver: any PluginDatabaseDriver { pluginDriver } diff --git a/TablePro/Resources/Localizable.xcstrings b/TablePro/Resources/Localizable.xcstrings index 55d3752f85..0482604dcf 100644 --- a/TablePro/Resources/Localizable.xcstrings +++ b/TablePro/Resources/Localizable.xcstrings @@ -184363,6 +184363,90 @@ }, "This database does not store whole documents" : { + }, + "Cannot save changes to '%@'." : { + + }, + "The driver cannot write an edited row." : { + + }, + "The driver cannot write %lld edited rows." : { + + }, + "The driver cannot write a new row." : { + + }, + "The driver cannot write %lld new rows." : { + + }, + "The driver cannot delete a row marked for deletion." : { + + }, + "The driver cannot delete %lld rows marked for deletion." : { + + }, + "Cannot save the new row in '%1$@'. %2$@" : { + + }, + "Cannot save the edited row in '%1$@'. %2$@" : { + + }, + "Cannot delete the row in '%1$@'. %2$@" : { + + }, + "Cannot save changes to '%1$@'. %2$@" : { + + }, + "This database has no statement for a new row that sets no column. Enter a value in at least one column, or insert the row with a query." : { + + }, + "Nothing was saved, and every change is still pending. Undo what cannot be written, or make it with a query, then save again." : { + + }, + "'%@' is a field of a nested array. Edit the array in '%@' instead." : { + + }, + "'%@' is document metadata and cannot be edited." : { + + }, + "The document's _id is unknown, so it cannot be addressed." : { + + }, + "The new values cannot be written as JSON." : { + + }, + "A new key needs a name." : { + + }, + "A key needs a name." : { + + }, + "This key's name is not text, so it cannot be addressed from the grid." : { + + }, + "'%@' is set by etcd and cannot be edited." : { + + }, + "A %@ key cannot be added from the grid. Add it with a command in the query editor." : { + + }, + "A key can only be renamed to text." : { + + }, + "'%@' cannot be changed from the grid." : { + + }, + "Redis cannot store NULL as a value. Enter an empty value instead." : { + + }, + "The key's type is unknown, so its value cannot be written safely." : { + + }, + "The value of a %@ key cannot be edited in the grid. Change it with a command in the query editor." : { + + }, + "TTL has to be a whole number of seconds above 0, or -1 or NULL for no expiry." : { + } }, "version" : "1.1" diff --git a/TableProTests/Core/DataWrite/RowChangeStatementFactoryCoverageTests.swift b/TableProTests/Core/DataWrite/RowChangeStatementFactoryCoverageTests.swift new file mode 100644 index 0000000000..05c1018da5 --- /dev/null +++ b/TableProTests/Core/DataWrite/RowChangeStatementFactoryCoverageTests.swift @@ -0,0 +1,442 @@ +// +// RowChangeStatementFactoryCoverageTests.swift +// TableProTests +// + +import Foundation +@testable import TablePro +import TableProPluginKit +import Testing + +private struct UnrelatedDriverError: Error, LocalizedError { + var errorDescription: String? { "socket closed" } +} + +@MainActor +struct RowChangeStatementFactoryCoverageTests { + private let columns = ["_id", "name"] + + private func factory( + table: String = "items", + columns: [String]? = nil, + databaseType: DatabaseType = DatabaseType(rawValue: "MongoDB"), + driver: (any PluginDatabaseDriver)? + ) -> RowChangeStatementFactory { + RowChangeStatementFactory( + tableName: table, + schemaName: nil, + columns: columns ?? self.columns, + primaryKeyColumns: ["_id"], + databaseType: databaseType, + pluginDriver: driver + ) + } + + private func nameEdit(row: Int = 0) -> RowChange { + RowChange( + rowID: .existing(row), + type: .update, + cellChanges: [CellChange(columnIndex: 1, columnName: "name", oldValue: "a", newValue: "z")], + originalRow: [.text("\(row)"), "a"] + ) + } + + private let personColumns = ["_id", "identifiers", "identifiers.type", "personId"] + + /// The Elasticsearch driver as the host reaches it: its generator's own `generateRowWrites`, + /// which writes a nested array through its parent column and refuses a value typed into a leaf. + private func elasticsearchDriver() -> RowWriteStubDriver { + let generator = ElasticsearchStatementGenerator( + index: "persons", + columns: personColumns, + columnTypeNames: ["keyword", "nested", "keyword", "keyword"] + ) + return RowWriteStubDriver { changes, insertedRowData, deletedRowIndices, insertedRowIndices in + try generator.generateRowWrites( + from: changes, + insertedRowData: insertedRowData, + deletedRowIndices: deletedRowIndices, + insertedRowIndices: insertedRowIndices + ) + } + } + + private let nestedLeafReason = "'identifiers.type' is a field of a nested array. Edit the array in 'identifiers' instead." + + private func personEdit(_ cells: [CellChange]) -> RowChange { + RowChange( + rowID: .existing(0), + type: .update, + cellChanges: cells, + originalRow: ["doc1", "[{\"type\":\"CPF\"}]", "[\"CPF\"]", "p1"] + ) + } + + private var leafEdit: CellChange { + CellChange(columnIndex: 2, columnName: "identifiers.type", oldValue: "[\"CPF\"]", newValue: "[\"X\"]") + } + + private var personIdEdit: CellChange { + CellChange(columnIndex: 3, columnName: "personId", oldValue: "p1", newValue: "p2") + } + + private func writesOnlyUpdates() -> RowWriteStubDriver { + RowWriteStubDriver { changes, _, _, _ in + changes.filter { $0.type == .update }.map { + PluginRowWrite(statement: "updateOne(\($0.rowIndex))", rowIndices: [$0.rowIndex]) + } + } + } + + // MARK: - A driver that adopts generateRowWrites + + @Test("A driver that names nothing for a new row refuses the whole save") + func driverLeavingOutANewRowRefusesTheSave() { + let inserted = RowID.inserted(UUID()) + let changes = [nameEdit(), RowChange(rowID: inserted, type: .insert)] + + #expect(throws: DataWriteError.changesNotWritable(table: "items", unwritten: UnwrittenRowCounts(inserts: 1))) { + _ = try factory(driver: writesOnlyUpdates()).statements( + for: changes, insertedRowData: [inserted: [.null, .null]], insertedRowIDs: [inserted] + ) + } + } + + @Test("The refusal says which kind of change cannot be written, and how many") + func coverageRefusalNamesTheKindAndCount() { + let error = DataWriteError.changesNotWritable(table: "items", unwritten: UnwrittenRowCounts(inserts: 1)) + #expect(error.errorDescription == "Cannot save changes to 'items'. The driver cannot write a new row.") + + let several = DataWriteError.changesNotWritable( + table: "items", unwritten: UnwrittenRowCounts(updates: 2, deletes: 1) + ) + #expect( + several.errorDescription + == "Cannot save changes to 'items'. The driver cannot write 2 edited rows. The driver cannot delete a row marked for deletion." + ) + #expect(several.recoverySuggestion?.hasPrefix("Nothing was saved") == true) + } + + @Test("A driver's refusal reaches the user with its reason and the kind of change it refused") + func driverRefusalReachesTheUser() throws { + let inserted = RowID.inserted(UUID()) + let driver = RowWriteStubDriver { _, _, _, _ in + throw PluginRowWriteRefusal(rowIndex: 1, reason: "A document needs at least one field.") + } + + do { + _ = try factory(driver: driver).statements( + for: [nameEdit(), RowChange(rowID: inserted, type: .insert)], + insertedRowData: [inserted: [.null, .null]], + insertedRowIDs: [inserted] + ) + Issue.record("the save was not refused") + } catch let error as DataWriteError { + #expect(error == .changeRefused(table: "items", kind: .insert, reason: "A document needs at least one field.")) + #expect(error.errorDescription == "Cannot save the new row in 'items'. A document needs at least one field.") + } + } + + @Test("Any other error a driver throws while writing still refuses the save") + func unrelatedDriverErrorRefusesTheSave() { + let driver = RowWriteStubDriver { _, _, _, _ in throw UnrelatedDriverError() } + + #expect(throws: DataWriteError.changeRefused(table: "items", kind: nil, reason: "socket closed")) { + _ = try factory(driver: driver).statements(for: [nameEdit()]) + } + } + + @Test("A write naming a row that is not in the save covers nothing") + func aWriteNamingNoPendingChangeCoversNothing() { + let driver = RowWriteStubDriver { _, _, _, _ in + [PluginRowWrite(statement: "updateOne(7)", rowIndices: [7])] + } + + #expect(throws: DataWriteError.changesNotWritable(table: "items", unwritten: UnwrittenRowCounts(updates: 1))) { + _ = try factory(driver: driver).statements(for: [nameEdit()]) + } + } + + @Test("A driver that writes every change gets its statements back as written") + func completeDriverWriteIsKept() throws { + let statements = try factory(driver: writesOnlyUpdates()).statements(for: [nameEdit(row: 0), nameEdit(row: 1)]) + #expect(statements.map(\.sql) == ["updateOne(0)", "updateOne(1)"]) + } + + // MARK: - A driver built before generateRowWrites + + @Test("A driver built before row writes is still held to every change, which is the save from #3132") + func driverBuiltBeforeRowWritesIsHeldToEveryChange() { + let driver = LegacyStatementStubDriver(generator: DocumentStyleGenerator.statements) + let inserted = RowID.inserted(UUID()) + + #expect(throws: DataWriteError.changesNotWritable(table: "items", unwritten: UnwrittenRowCounts(inserts: 1))) { + _ = try factory(driver: driver).statements( + for: [nameEdit(), RowChange(rowID: inserted, type: .insert)], + insertedRowData: [inserted: [.null, .null]], + insertedRowIDs: [inserted] + ) + } + } + + @Test("A driver built before row writes runs exactly the statements it batched itself") + func driverBuiltBeforeRowWritesKeepsItsOwnStatements() throws { + let driver = LegacyStatementStubDriver(generator: DocumentStyleGenerator.statements) + let deletes = [ + RowChange(rowID: .existing(0), type: .delete, originalRow: ["0", "a"]), + RowChange(rowID: .existing(1), type: .delete, originalRow: ["1", "b"]), + RowChange(rowID: .existing(2), type: .delete, originalRow: ["2", "c"]) + ] + + let statements = try factory(driver: driver).statements( + for: deletes, deletedRowIDs: [.existing(0), .existing(1), .existing(2)] + ) + + #expect(statements.map(\.sql) == ["deleteMany(3)"]) + } + + @Test("The default for an older driver hands each per-change call only that change's row") + func defaultStaysLinearInTheSizeOfTheSave() throws { + let driver = LegacyStatementStubDriver(generator: DocumentStyleGenerator.statements) + let rowCount = 2_000 + let rowIDs = (0.. [(statement: String, parameters: [PluginCellValue])] { +) throws -> [PluginRowWrite] { let generator = RedisStatementGenerator(namespaceName: "", columns: browseColumns) let change = PluginRowChange( rowIndex: 0, @@ -22,7 +22,7 @@ private func updateStatements( cellChanges: [(columnIndex: 4, columnName: "Value", oldValue: .text("old"), newValue: newValue)], originalRow: [.text(key), .text(type), "-1", "3", .text("old")] ) - return generator.generateStatements( + return try generator.generateRowWrites( from: [change], insertedRowData: [:], deletedRowIndices: [], insertedRowIndices: [] ) } @@ -34,49 +34,49 @@ private func parsedValue(of statement: String) -> Data? { struct RedisWriteRoundTripTests { @Test("a plain value produces a readable command") - func plainValueStaysReadable() { - let statements = updateStatements(newValue: .text("hello")) + func plainValueStaysReadable() throws { + let statements = try updateStatements(newValue: .text("hello")) #expect(statements.count == 1) #expect(statements.first?.statement == "SET mykey hello") } @Test("a value with spaces is quoted and parses back whole") - func spacedValueRoundTrips() { - let statements = updateStatements(newValue: .text("hello world")) + func spacedValueRoundTrips() throws { + let statements = try updateStatements(newValue: .text("hello world")) #expect(statements.first?.statement == "SET mykey \"hello world\"") #expect(parsedValue(of: statements[0].statement) == Data("hello world".utf8)) } @Test("quotes, backslashes, and newlines round-trip") - func specialCharactersRoundTrip() { + func specialCharactersRoundTrip() throws { let value = "a\"b\\c\nd" - let statements = updateStatements(newValue: .text(value)) + let statements = try updateStatements(newValue: .text(value)) #expect(parsedValue(of: statements[0].statement) == Data(value.utf8)) } @Test("non-ASCII text round-trips") - func unicodeRoundTrips() { + func unicodeRoundTrips() throws { let value = "cafĆ© ā˜•" - let statements = updateStatements(newValue: .text(value)) + let statements = try updateStatements(newValue: .text(value)) #expect(parsedValue(of: statements[0].statement) == Data(value.utf8)) } @Test("a long value round-trips whole") - func longValueRoundTrips() { + func longValueRoundTrips() throws { let value = String(repeating: "x", count: 20_000) - let statements = updateStatements(newValue: .text(value)) + let statements = try updateStatements(newValue: .text(value)) #expect(parsedValue(of: statements[0].statement) == Data(value.utf8)) } @Test("a binary value round-trips byte for byte") - func binaryValueRoundTrips() { - let statements = updateStatements(newValue: .bytes(gzipPayload)) + func binaryValueRoundTrips() throws { + let statements = try updateStatements(newValue: .bytes(gzipPayload)) #expect(statements.count == 1) #expect(parsedValue(of: statements[0].statement) == gzipPayload) } @Test("binary blobs of many shapes round-trip") - func binaryBlobsRoundTrip() { + func binaryBlobsRoundTrip() throws { var seed: UInt64 = 0x2545_F491_4F6C_DD1D func nextByte() -> UInt8 { seed ^= seed << 13 @@ -86,14 +86,14 @@ struct RedisWriteRoundTripTests { } for length in 1 ... 120 { let blob = Data((0 ..< length).map { _ in nextByte() }) - let statements = updateStatements(newValue: .bytes(blob)) + let statements = try updateStatements(newValue: .bytes(blob)) #expect(parsedValue(of: statements[0].statement) == blob) } } @Test("a key containing a space round-trips") - func keyWithSpaceRoundTrips() { - let statements = updateStatements(key: "my key", newValue: .text("v")) + func keyWithSpaceRoundTrips() throws { + let statements = try updateStatements(key: "my key", newValue: .text("v")) guard case .set(let key, _, _)? = try? RedisCommandParser.parse(statements[0].statement) else { Issue.record("Expected a SET operation") return @@ -102,9 +102,9 @@ struct RedisWriteRoundTripTests { } @Test("a value cannot inject a second command") - func valueCannotInjectCommand() { + func valueCannotInjectCommand() throws { let hostile = "x\" \nDEL victim \"y" - let statements = updateStatements(newValue: .text(hostile)) + let statements = try updateStatements(newValue: .text(hostile)) #expect(statements.count == 1) #expect(RedisArgumentCodec.split(statements[0].statement)?.count == 3) #expect(parsedValue(of: statements[0].statement) == Data(hostile.utf8)) @@ -112,17 +112,19 @@ struct RedisWriteRoundTripTests { @Test("a collection value is still refused so the structure survives") func collectionValueRefused() { - #expect(updateStatements(type: "LIST", newValue: .text("[\"a\"]")).isEmpty) + #expect(throws: PluginRowWriteRefusal.self) { + _ = try updateStatements(type: "LIST", newValue: .text("[\"a\"]")) + } } @Test("a binary insert round-trips") - func binaryInsertRoundTrips() { + func binaryInsertRoundTrips() throws { let generator = RedisStatementGenerator(namespaceName: "", columns: browseColumns) let change = PluginRowChange(rowIndex: 0, type: .insert, cellChanges: [], originalRow: nil) let inserted: [Int: [PluginCellValue]] = [ 0: [.text("bin"), .text("STRING"), .null, .null, .bytes(gzipPayload)] ] - let statements = generator.generateStatements( + let statements = try generator.generateRowWrites( from: [change], insertedRowData: inserted, deletedRowIndices: [], insertedRowIndices: [0] ) #expect(statements.count == 1) diff --git a/TableProTests/Core/Redis/RedisCommandParserTests.swift b/TableProTests/Core/Redis/RedisCommandParserTests.swift index 5ff466131c..222c3fd7e9 100644 --- a/TableProTests/Core/Redis/RedisCommandParserTests.swift +++ b/TableProTests/Core/Redis/RedisCommandParserTests.swift @@ -990,11 +990,11 @@ struct RedisCommandParserAppStatementTests { return false } - private func insertStatements(key: String, type: String, value: String) -> [String] { + private func insertStatements(key: String, type: String, value: String) throws -> [String] { let generator = RedisStatementGenerator(namespaceName: "", columns: Self.browseColumns) let change = PluginRowChange(rowIndex: 0, type: .insert, cellChanges: [], originalRow: nil) let row: [PluginCellValue] = [.text(key), .text(type), "60", .null, .text(value)] - return generator.generateStatements( + return try generator.generateRowWrites( from: [change], insertedRowData: [0: row], deletedRowIndices: [], insertedRowIndices: [0] ).map(\.statement) } @@ -1005,7 +1005,7 @@ struct RedisCommandParserAppStatementTests { ) func insertsStayTyped(type: String) throws { let value = type == "hash" ? #"{"f":"v w"}"# : "a \"quoted\" value" - let statements = insertStatements(key: "user:1 x", type: type, value: value) + let statements = try insertStatements(key: "user:1 x", type: type, value: value) #expect(statements.count == 2) for statement in statements { #expect(try !isVerbatim(statement), "\(statement)") @@ -1033,7 +1033,7 @@ struct RedisCommandParserAppStatementTests { originalRow: original ) let delete = PluginRowChange(rowIndex: 2, type: .delete, cellChanges: [], originalRow: original) - let statements = generator.generateStatements( + let statements = try generator.generateRowWrites( from: [update, persist, delete], insertedRowData: [:], deletedRowIndices: [2], insertedRowIndices: [] ).map(\.statement) diff --git a/TableProTests/Helpers/RowWriteStubDrivers.swift b/TableProTests/Helpers/RowWriteStubDrivers.swift new file mode 100644 index 0000000000..2dfc96f32b --- /dev/null +++ b/TableProTests/Helpers/RowWriteStubDrivers.swift @@ -0,0 +1,165 @@ +// +// RowWriteStubDrivers.swift +// TableProTests +// + +import Foundation +@testable import TablePro +import TableProPluginKit + +/// A driver that writes its own statements through `generateRowWrites`. +internal final class RowWriteStubDriver: PluginDatabaseDriver, @unchecked Sendable { + internal typealias Writer = ( + _ changes: [PluginRowChange], + _ insertedRowData: [Int: [PluginCellValue]], + _ deletedRowIndices: Set, + _ insertedRowIndices: Set + ) throws -> [PluginRowWrite]? + + private let writer: Writer + internal private(set) var executedQueries: [String] = [] + + internal init(writer: @escaping Writer) { + self.writer = writer + } + + internal func generateRowWrites( + table: String, + schema: String?, + columns: [String], + primaryKeyColumns: [String], + changes: [PluginRowChange], + insertedRowData: [Int: [PluginCellValue]], + deletedRowIndices: Set, + insertedRowIndices: Set + ) throws -> [PluginRowWrite]? { + try writer(changes, insertedRowData, deletedRowIndices, insertedRowIndices) + } + + internal func quoteIdentifier(_ name: String) -> String { "\"\(name)\"" } + + internal func connect() async throws {} + internal func disconnect() {} + + internal func execute(query: String) async throws -> PluginQueryResult { + executedQueries.append(query) + return PluginQueryResult(columns: [], columnTypeNames: [], rows: [], rowsAffected: 0, executionTime: 0) + } + + internal func executeParameterized(query: String, parameters: [PluginCellValue]) async throws -> PluginQueryResult { + executedQueries.append(query) + return PluginQueryResult(columns: [], columnTypeNames: [], rows: [], rowsAffected: 0, executionTime: 0) + } + + internal func fetchTables(schema: String?) async throws -> [PluginTableInfo] { [] } + internal func fetchColumns(table: String, schema: String?) async throws -> [PluginColumnInfo] { [] } + internal func fetchIndexes(table: String, schema: String?) async throws -> [PluginIndexInfo] { [] } + internal func fetchForeignKeys(table: String, schema: String?) async throws -> [PluginForeignKeyInfo] { [] } + internal func fetchTableDDL(table: String, schema: String?) async throws -> String { "" } + internal func fetchViewDefinition(view: String, schema: String?) async throws -> String { "" } + + internal func fetchTableMetadata(table: String, schema: String?) async throws -> PluginTableMetadata { + PluginTableMetadata(tableName: table) + } + + internal func fetchDatabases() async throws -> [String] { [] } + + internal func fetchDatabaseMetadata(_ database: String) async throws -> PluginDatabaseMetadata { + PluginDatabaseMetadata(name: database) + } +} + +/// A driver built before `generateRowWrites` existed: it implements only `generateStatements`, so +/// the host reaches it through the PluginKit default. +internal final class LegacyStatementStubDriver: PluginDatabaseDriver, @unchecked Sendable { + internal typealias Generator = ( + _ changes: [PluginRowChange], + _ insertedRowData: [Int: [PluginCellValue]], + _ deletedRowIndices: Set, + _ insertedRowIndices: Set + ) -> [(statement: String, parameters: [PluginCellValue])]? + + private let generator: Generator + internal private(set) var generateCallCount = 0 + internal private(set) var rowsHanded = 0 + internal private(set) var cellsHanded = 0 + + internal init(generator: @escaping Generator) { + self.generator = generator + } + + internal func generateStatements( + table: String, + schema: String?, + columns: [String], + primaryKeyColumns: [String], + changes: [PluginRowChange], + insertedRowData: [Int: [PluginCellValue]], + deletedRowIndices: Set, + insertedRowIndices: Set + ) -> [(statement: String, parameters: [PluginCellValue])]? { + generateCallCount += 1 + rowsHanded += changes.count + insertedRowData.count + deletedRowIndices.count + insertedRowIndices.count + cellsHanded += changes.reduce(0) { $0 + $1.cellChanges.count } + return generator(changes, insertedRowData, deletedRowIndices, insertedRowIndices) + } + + internal func quoteIdentifier(_ name: String) -> String { "\"\(name)\"" } + + internal func connect() async throws {} + internal func disconnect() {} + + internal func execute(query: String) async throws -> PluginQueryResult { + PluginQueryResult(columns: [], columnTypeNames: [], rows: [], rowsAffected: 0, executionTime: 0) + } + + internal func fetchTables(schema: String?) async throws -> [PluginTableInfo] { [] } + internal func fetchColumns(table: String, schema: String?) async throws -> [PluginColumnInfo] { [] } + internal func fetchIndexes(table: String, schema: String?) async throws -> [PluginIndexInfo] { [] } + internal func fetchForeignKeys(table: String, schema: String?) async throws -> [PluginForeignKeyInfo] { [] } + internal func fetchTableDDL(table: String, schema: String?) async throws -> String { "" } + internal func fetchViewDefinition(view: String, schema: String?) async throws -> String { "" } + + internal func fetchTableMetadata(table: String, schema: String?) async throws -> PluginTableMetadata { + PluginTableMetadata(tableName: table) + } + + internal func fetchDatabases() async throws -> [String] { [] } + + internal func fetchDatabaseMetadata(_ database: String) async throws -> PluginDatabaseMetadata { + PluginDatabaseMetadata(name: database) + } +} + +/// The way MongoDB's generator behaves: one statement per change it can write, one `deleteMany` +/// for every deleted row, and a new row with no values left out. +internal enum DocumentStyleGenerator { + internal static func statements( + changes: [PluginRowChange], + insertedRowData: [Int: [PluginCellValue]], + deletedRowIndices: Set, + insertedRowIndices: Set + ) -> [(statement: String, parameters: [PluginCellValue])] { + var statements: [(statement: String, parameters: [PluginCellValue])] = [] + var deleted: [Int] = [] + for change in changes { + switch change.type { + case .insert: + guard insertedRowIndices.contains(change.rowIndex), + let values = insertedRowData[change.rowIndex], + values.contains(where: { !$0.isNull }) else { continue } + statements.append((statement: "insertOne(\(change.rowIndex))", parameters: [])) + case .update: + guard !change.cellChanges.isEmpty else { continue } + statements.append((statement: "updateOne(\(change.rowIndex))", parameters: [])) + case .delete: + guard deletedRowIndices.contains(change.rowIndex) else { continue } + deleted.append(change.rowIndex) + } + } + if !deleted.isEmpty { + statements.append((statement: "deleteMany(\(deleted.count))", parameters: [])) + } + return statements + } +} diff --git a/TableProTests/Plugins/ElasticsearchDriverTests.swift b/TableProTests/Plugins/ElasticsearchDriverTests.swift index b28be95147..ca73b766e7 100644 --- a/TableProTests/Plugins/ElasticsearchDriverTests.swift +++ b/TableProTests/Plugins/ElasticsearchDriverTests.swift @@ -605,7 +605,7 @@ struct ElasticsearchMappingFlattenerTests { func doublesRoundTrip() { let source: [String: Any] = [ "score": -3.9192320754595876e-07, - "total": 1847.27, + "total": 1_847.27, "counts": ["rate": 0.1, "qty": 3.0], ] let flat = ElasticsearchMappingFlattener.flattenSource(source) @@ -617,7 +617,7 @@ struct ElasticsearchMappingFlattenerTests { @Test("An array of doubles serializes without binary floating point noise") func arrayOfDoublesHasNoExcessDigits() { - let source: [String: Any] = ["samples": [0.1, 1847.27]] + let source: [String: Any] = ["samples": [0.1, 1_847.27]] let flat = ElasticsearchMappingFlattener.flattenSource(source) #expect(flat["samples"] == .text("[0.1,1847.27]")) } @@ -815,14 +815,14 @@ struct ElasticsearchStatementGeneratorTests { } @Test("Update encodes a POST _update keyed by _id") - func updateRequest() { + func updateRequest() throws { let change = PluginRowChange( rowIndex: 0, type: .update, cellChanges: [(columnIndex: 3, columnName: "name", oldValue: .text("Bob"), newValue: .text("Alice"))], originalRow: [.text("doc1"), .text("users"), .text("1"), .text("Bob"), .text("30")] ) - let statements = generator().generateStatements( + let statements = try generator().generateRowWrites( from: [change], insertedRowData: [:], deletedRowIndices: [], insertedRowIndices: [] ) #expect(statements.count == 1) @@ -833,14 +833,14 @@ struct ElasticsearchStatementGeneratorTests { } @Test("Delete encodes a DELETE _doc by _id") - func deleteRequest() { + func deleteRequest() throws { let change = PluginRowChange( rowIndex: 0, type: .delete, cellChanges: [], originalRow: [.text("doc9"), .text("users"), .text("1"), .text("Bob"), .text("30")] ) - let statements = generator().generateStatements( + let statements = try generator().generateRowWrites( from: [change], insertedRowData: [:], deletedRowIndices: [0], insertedRowIndices: [] ) let decoded = ElasticsearchStatementGenerator.decode(statements[0].statement) @@ -849,9 +849,9 @@ struct ElasticsearchStatementGeneratorTests { } @Test("Insert coerces numeric fields and omits meta columns") - func insertRequest() { + func insertRequest() throws { let change = PluginRowChange(rowIndex: 0, type: .insert, cellChanges: [], originalRow: nil) - let statements = generator().generateStatements( + let statements = try generator().generateRowWrites( from: [change], insertedRowData: [0: [.null, .null, .null, .text("Eve"), .text("25")]], deletedRowIndices: [], @@ -873,9 +873,9 @@ struct ElasticsearchStatementGeneratorTests { } @Test("Insert writes the nested array once, through its parent column") - func insertOmitsNestedLeaves() { + func insertOmitsNestedLeaves() throws { let change = PluginRowChange(rowIndex: 0, type: .insert, cellChanges: [], originalRow: nil) - let statements = nestedGenerator().generateStatements( + let statements = try nestedGenerator().generateRowWrites( from: [change], insertedRowData: [0: [ .null, @@ -891,8 +891,19 @@ struct ElasticsearchStatementGeneratorTests { #expect(body?.contains("identifiers.type") == false) } - @Test("Update skips a nested leaf edit and keeps the rest of the row") - func updateSkipsNestedLeaf() { + private func nestedLeafRefusal(rowIndex: Int = 0) -> PluginRowWriteRefusal { + PluginRowWriteRefusal( + rowIndex: rowIndex, + reason: "'identifiers.type' is a field of a nested array. Edit the array in 'identifiers' instead." + ) + } + + private func leafTyped(_ value: String) -> [(columnIndex: Int, columnName: String, oldValue: PluginCellValue, newValue: PluginCellValue)] { + [(columnIndex: 2, columnName: "identifiers.type", oldValue: .null, newValue: .text(value))] + } + + @Test("Update refuses a nested leaf edit rather than write the rest of the row without it") + func updateRefusesNestedLeaf() { let change = PluginRowChange( rowIndex: 0, type: .update, @@ -905,18 +916,167 @@ struct ElasticsearchStatementGeneratorTests { ], originalRow: [.text("doc1"), .text("[]"), .text("[\"CPF\"]"), .text("p1")] ) - let statements = nestedGenerator().generateStatements( - from: [change], insertedRowData: [:], deletedRowIndices: [], insertedRowIndices: [] + #expect(throws: nestedLeafRefusal()) { + try nestedGenerator().generateRowWrites( + from: [change], insertedRowData: [:], deletedRowIndices: [], insertedRowIndices: [] + ) + } + } + + @Test("A new row with only a nested leaf typed is refused rather than saved empty") + func insertRefusesTypedLeafWithEmptyArray() { + let change = PluginRowChange(rowIndex: 0, type: .insert, cellChanges: leafTyped("[\"X\"]"), originalRow: nil) + #expect(throws: nestedLeafRefusal()) { + try nestedGenerator().generateRowWrites( + from: [change], + insertedRowData: [0: [.null, .null, .text("[\"X\"]"), .null]], + deletedRowIndices: [], + insertedRowIndices: [0] + ) + } + } + + @Test("A leaf typed into a new row is refused even when its array has a value") + func insertRefusesTypedLeafBesideItsArray() { + let change = PluginRowChange(rowIndex: 0, type: .insert, cellChanges: leafTyped("[\"X\"]"), originalRow: nil) + #expect(throws: nestedLeafRefusal()) { + try nestedGenerator().generateRowWrites( + from: [change], + insertedRowData: [0: [.null, .text("[{\"type\":\"CPF\"}]"), .text("[\"X\"]"), .null]], + deletedRowIndices: [], + insertedRowIndices: [0] + ) + } + } + + @Test("A pasted row carrying a leaf value with no array to write it through is refused") + func insertRefusesPastedLeafWithoutItsArray() { + let change = PluginRowChange(rowIndex: 0, type: .insert, cellChanges: [], originalRow: nil) + #expect(throws: nestedLeafRefusal()) { + try nestedGenerator().generateRowWrites( + from: [change], + insertedRowData: [0: [.null, .null, .text("[\"CPF\"]"), .text("p1")]], + deletedRowIndices: [], + insertedRowIndices: [0] + ) + } + } + + @Test("An edit to document metadata is refused") + func updateRefusesMetadata() { + let change = PluginRowChange( + rowIndex: 3, + type: .update, + cellChanges: [ + (columnIndex: 3, columnName: "name", oldValue: .text("Bob"), newValue: .text("Alice")), + (columnIndex: 1, columnName: "_index", oldValue: .text("users"), newValue: .text("people")), + ], + originalRow: [.text("doc1"), .text("users"), .text("1"), .text("Bob"), .text("30")] ) - let body = ElasticsearchStatementGenerator.decode(statements[0].statement)?.body - #expect(body?.contains("identifiers.type") == false) - #expect(body?.contains("p2") == true) + let refusal = PluginRowWriteRefusal(rowIndex: 3, reason: "'_index' is document metadata and cannot be edited.") + #expect(throws: refusal) { + try generator().generateRowWrites( + from: [change], insertedRowData: [:], deletedRowIndices: [], insertedRowIndices: [] + ) + } + } + + @Test("A score typed into a new row is refused, and an _id typed into one is written") + func insertRefusesTypedScoreAndKeepsTypedId() throws { + let score = PluginRowChange( + rowIndex: 0, + type: .insert, + cellChanges: [(columnIndex: 2, columnName: "_score", oldValue: .null, newValue: .text("9"))], + originalRow: nil + ) + #expect(throws: PluginRowWriteRefusal(rowIndex: 0, reason: "'_score' is document metadata and cannot be edited.")) { + try generator().generateRowWrites( + from: [score], + insertedRowData: [0: [.null, .null, .text("9"), .text("Eve"), .null]], + deletedRowIndices: [], + insertedRowIndices: [0] + ) + } + + let id = PluginRowChange( + rowIndex: 0, + type: .insert, + cellChanges: [(columnIndex: 0, columnName: "_id", oldValue: .null, newValue: .text("e1"))], + originalRow: nil + ) + let writes = try generator().generateRowWrites( + from: [id], + insertedRowData: [0: [.text("e1"), .null, .null, .text("Eve"), .null]], + deletedRowIndices: [], + insertedRowIndices: [0] + ) + #expect(writes.map(\.rowIndices) == [[0]]) + #expect(ElasticsearchStatementGenerator.decode(writes[0].statement)?.path.contains("/users/_doc/e1") == true) + } + + @Test("A value that is not valid JSON refuses the change") + func updateRefusesAValueJSONCannotHold() { + let change = PluginRowChange( + rowIndex: 0, + type: .update, + cellChanges: [(columnIndex: 4, columnName: "age", oldValue: .text("30"), newValue: .text("nan"))], + originalRow: [.text("doc1"), .text("users"), .text("1"), .text("Bob"), .text("30")] + ) + #expect(throws: PluginRowWriteRefusal(rowIndex: 0, reason: "The new values cannot be written as JSON.")) { + try generator().generateRowWrites( + from: [change], insertedRowData: [:], deletedRowIndices: [], insertedRowIndices: [] + ) + } + } + + @Test("A change with no _id to address it by is refused") + func refusesAChangeWithoutAnId() { + let update = PluginRowChange( + rowIndex: 0, + type: .update, + cellChanges: [(columnIndex: 3, columnName: "name", oldValue: .text("Bob"), newValue: .text("Alice"))], + originalRow: [.null, .text("users"), .text("1"), .text("Bob"), .text("30")] + ) + let delete = PluginRowChange(rowIndex: 1, type: .delete, cellChanges: [], originalRow: nil) + let reason = "The document's _id is unknown, so it cannot be addressed." + #expect(throws: PluginRowWriteRefusal(rowIndex: 0, reason: reason)) { + try generator().generateRowWrites( + from: [update], insertedRowData: [:], deletedRowIndices: [], insertedRowIndices: [] + ) + } + #expect(throws: PluginRowWriteRefusal(rowIndex: 1, reason: reason)) { + try generator().generateRowWrites( + from: [delete], insertedRowData: [:], deletedRowIndices: [1], insertedRowIndices: [] + ) + } + } + + @Test("Each request names the change it writes, and an update with nothing to set writes nothing") + func writesNameTheirChange() throws { + let original: [PluginCellValue] = [.text("doc1"), .text("users"), .text("1"), .text("Bob"), .text("30")] + let writes = try generator().generateRowWrites( + from: [ + PluginRowChange(rowIndex: 0, type: .update, cellChanges: [], originalRow: nil), + PluginRowChange( + rowIndex: 1, + type: .update, + cellChanges: [(columnIndex: 3, columnName: "name", oldValue: .text("Bob"), newValue: .null)], + originalRow: original + ), + PluginRowChange(rowIndex: 2, type: .delete, cellChanges: [], originalRow: original), + ], + insertedRowData: [:], + deletedRowIndices: [2], + insertedRowIndices: [] + ) + #expect(writes.map(\.rowIndices) == [[1], [2]]) + #expect(ElasticsearchStatementGenerator.decode(writes[0].statement)?.body == "{\"doc\":{\"name\":null}}") } @Test("Insert with explicit _id uses PUT") - func insertWithId() { + func insertWithId() throws { let change = PluginRowChange(rowIndex: 0, type: .insert, cellChanges: [], originalRow: nil) - let statements = generator().generateStatements( + let statements = try generator().generateRowWrites( from: [change], insertedRowData: [0: [.text("custom"), .null, .null, .text("Eve"), .text("25")]], deletedRowIndices: [], @@ -928,14 +1088,14 @@ struct ElasticsearchStatementGeneratorTests { } @Test("Document id with a slash is percent-encoded into one path segment") - func slashInDocumentId() { + func slashInDocumentId() throws { let change = PluginRowChange( rowIndex: 0, type: .delete, cellChanges: [], originalRow: [.text("tenant/123"), .text("users"), .text("1"), .text("Bob"), .text("30")] ) - let statements = generator().generateStatements( + let statements = try generator().generateRowWrites( from: [change], insertedRowData: [:], deletedRowIndices: [0], insertedRowIndices: [] ) let decoded = ElasticsearchStatementGenerator.decode(statements[0].statement) @@ -943,9 +1103,9 @@ struct ElasticsearchStatementGeneratorTests { } @Test("Insert preserves an intentional empty string") - func insertKeepsEmptyString() { + func insertKeepsEmptyString() throws { let change = PluginRowChange(rowIndex: 0, type: .insert, cellChanges: [], originalRow: nil) - let statements = generator().generateStatements( + let statements = try generator().generateRowWrites( from: [change], insertedRowData: [0: [.null, .null, .null, .text(""), .text("25")]], deletedRowIndices: [], @@ -956,9 +1116,9 @@ struct ElasticsearchStatementGeneratorTests { } @Test("JSON object text is kept as a string on a scalar field") - func jsonObjectKeptAsStringOnScalarField() { + func jsonObjectKeptAsStringOnScalarField() throws { let change = PluginRowChange(rowIndex: 0, type: .insert, cellChanges: [], originalRow: nil) - let statements = generator().generateStatements( + let statements = try generator().generateRowWrites( from: [change], insertedRowData: [0: [.null, .null, .null, .text("{\"a\":1}"), .text("25")]], deletedRowIndices: [], diff --git a/TableProTests/Plugins/EtcdStatementGeneratorTests.swift b/TableProTests/Plugins/EtcdStatementGeneratorTests.swift index 37a8044e74..43110e8def 100644 --- a/TableProTests/Plugins/EtcdStatementGeneratorTests.swift +++ b/TableProTests/Plugins/EtcdStatementGeneratorTests.swift @@ -6,14 +6,14 @@ // import Foundation -import Testing import TableProPluginKit +import Testing // MARK: - INSERT struct EtcdStatementGeneratorInsertTests { @Test("Basic insert generates put command") - func basicInsert() { + func basicInsert() throws { let gen = EtcdStatementGenerator( prefix: "", columns: ["Key", "Value", "Version", "CreateRevision", "ModRevision", "Lease"] @@ -30,7 +30,7 @@ struct EtcdStatementGeneratorInsertTests { 0: ["mykey", "myvalue", nil, nil, nil, nil] ] - let results = gen.generateStatements( + let results = try gen.generateRowWrites( from: [change], insertedRowData: insertedData, deletedRowIndices: [], @@ -42,7 +42,7 @@ struct EtcdStatementGeneratorInsertTests { } @Test("Insert with lease generates put --lease") - func insertWithLease() { + func insertWithLease() throws { let gen = EtcdStatementGenerator( prefix: "", columns: ["Key", "Value", "Version", "CreateRevision", "ModRevision", "Lease"] @@ -59,7 +59,7 @@ struct EtcdStatementGeneratorInsertTests { 0: ["mykey", "myvalue", nil, nil, nil, "12345"] ] - let results = gen.generateStatements( + let results = try gen.generateRowWrites( from: [change], insertedRowData: insertedData, deletedRowIndices: [], @@ -71,7 +71,7 @@ struct EtcdStatementGeneratorInsertTests { } @Test("Insert with prefix prepending") - func insertWithPrefixPrepending() { + func insertWithPrefixPrepending() throws { let gen = EtcdStatementGenerator( prefix: "/app/config/", columns: ["Key", "Value", "Version", "CreateRevision", "ModRevision", "Lease"] @@ -88,7 +88,7 @@ struct EtcdStatementGeneratorInsertTests { 0: ["setting1", "value1", nil, nil, nil, nil] ] - let results = gen.generateStatements( + let results = try gen.generateRowWrites( from: [change], insertedRowData: insertedData, deletedRowIndices: [], @@ -100,7 +100,7 @@ struct EtcdStatementGeneratorInsertTests { } @Test("Insert with key already containing prefix (no double prefix)") - func insertKeyAlreadyHasPrefix() { + func insertKeyAlreadyHasPrefix() throws { let gen = EtcdStatementGenerator( prefix: "/app/", columns: ["Key", "Value", "Version", "CreateRevision", "ModRevision", "Lease"] @@ -118,7 +118,7 @@ struct EtcdStatementGeneratorInsertTests { 0: ["/app/mykey", "value", nil, nil, nil, nil] ] - let results = gen.generateStatements( + let results = try gen.generateRowWrites( from: [change], insertedRowData: insertedData, deletedRowIndices: [], @@ -130,7 +130,7 @@ struct EtcdStatementGeneratorInsertTests { } @Test("Insert with absolute key (leading slash) skips prefix prepend") - func insertAbsoluteKey() { + func insertAbsoluteKey() throws { let gen = EtcdStatementGenerator( prefix: "something/", columns: ["Key", "Value", "Version", "CreateRevision", "ModRevision", "Lease"] @@ -147,7 +147,7 @@ struct EtcdStatementGeneratorInsertTests { 0: ["/absolute/key", "value", nil, nil, nil, nil] ] - let results = gen.generateStatements( + let results = try gen.generateRowWrites( from: [change], insertedRowData: insertedData, deletedRowIndices: [], @@ -158,7 +158,7 @@ struct EtcdStatementGeneratorInsertTests { #expect(results[0].statement == "put /absolute/key value") } - @Test("Insert with empty key is skipped") + @Test("Insert with empty key is refused") func insertEmptyKey() { let gen = EtcdStatementGenerator( prefix: "", @@ -176,17 +176,17 @@ struct EtcdStatementGeneratorInsertTests { 0: ["", "value", nil, nil, nil, nil] ] - let results = gen.generateStatements( - from: [change], - insertedRowData: insertedData, - deletedRowIndices: [], - insertedRowIndices: [0] - ) - - #expect(results.isEmpty) + #expect(throws: PluginRowWriteRefusal(rowIndex: 0, reason: "A new key needs a name.")) { + try gen.generateRowWrites( + from: [change], + insertedRowData: insertedData, + deletedRowIndices: [], + insertedRowIndices: [0] + ) + } } - @Test("Insert with nil key is skipped") + @Test("Insert with nil key is refused") func insertNilKey() { let gen = EtcdStatementGenerator( prefix: "", @@ -204,18 +204,18 @@ struct EtcdStatementGeneratorInsertTests { 0: [nil, "value", nil, nil, nil, nil] ] - let results = gen.generateStatements( - from: [change], - insertedRowData: insertedData, - deletedRowIndices: [], - insertedRowIndices: [0] - ) - - #expect(results.isEmpty) + #expect(throws: PluginRowWriteRefusal(rowIndex: 0, reason: "A new key needs a name.")) { + try gen.generateRowWrites( + from: [change], + insertedRowData: insertedData, + deletedRowIndices: [], + insertedRowIndices: [0] + ) + } } @Test("Insert with nil value uses empty string") - func insertNilValue() { + func insertNilValue() throws { let gen = EtcdStatementGenerator( prefix: "", columns: ["Key", "Value", "Version", "CreateRevision", "ModRevision", "Lease"] @@ -232,7 +232,7 @@ struct EtcdStatementGeneratorInsertTests { 0: ["mykey", nil, nil, nil, nil, nil] ] - let results = gen.generateStatements( + let results = try gen.generateRowWrites( from: [change], insertedRowData: insertedData, deletedRowIndices: [], @@ -244,7 +244,7 @@ struct EtcdStatementGeneratorInsertTests { } @Test("Insert with lease=0 omits --lease flag") - func insertLeaseZero() { + func insertLeaseZero() throws { let gen = EtcdStatementGenerator( prefix: "", columns: ["Key", "Value", "Version", "CreateRevision", "ModRevision", "Lease"] @@ -261,7 +261,7 @@ struct EtcdStatementGeneratorInsertTests { 0: ["mykey", "value", nil, nil, nil, "0"] ] - let results = gen.generateStatements( + let results = try gen.generateRowWrites( from: [change], insertedRowData: insertedData, deletedRowIndices: [], @@ -273,7 +273,7 @@ struct EtcdStatementGeneratorInsertTests { } @Test("Insert from cell changes (no insertedRowData)") - func insertFromCellChanges() { + func insertFromCellChanges() throws { let gen = EtcdStatementGenerator( prefix: "", columns: ["Key", "Value", "Version", "CreateRevision", "ModRevision", "Lease"] @@ -289,7 +289,7 @@ struct EtcdStatementGeneratorInsertTests { originalRow: nil ) - let results = gen.generateStatements( + let results = try gen.generateRowWrites( from: [change], insertedRowData: [:], deletedRowIndices: [], @@ -301,7 +301,7 @@ struct EtcdStatementGeneratorInsertTests { } @Test("Insert with value containing spaces is quoted") - func insertValueWithSpaces() { + func insertValueWithSpaces() throws { let gen = EtcdStatementGenerator( prefix: "", columns: ["Key", "Value", "Version", "CreateRevision", "ModRevision", "Lease"] @@ -318,7 +318,7 @@ struct EtcdStatementGeneratorInsertTests { 0: ["mykey", "hello world", nil, nil, nil, nil] ] - let results = gen.generateStatements( + let results = try gen.generateRowWrites( from: [change], insertedRowData: insertedData, deletedRowIndices: [], @@ -334,7 +334,7 @@ struct EtcdStatementGeneratorInsertTests { struct EtcdStatementGeneratorUpdateTests { @Test("Value change generates put with original key") - func valueChange() { + func valueChange() throws { let gen = EtcdStatementGenerator( prefix: "", columns: ["Key", "Value", "Version", "CreateRevision", "ModRevision", "Lease"] @@ -349,7 +349,7 @@ struct EtcdStatementGeneratorUpdateTests { originalRow: ["mykey", "oldval", "1", "1", "1", "0"] ) - let results = gen.generateStatements( + let results = try gen.generateRowWrites( from: [change], insertedRowData: [:], deletedRowIndices: [], @@ -361,7 +361,7 @@ struct EtcdStatementGeneratorUpdateTests { } @Test("Key rename generates put then del") - func keyRename() { + func keyRename() throws { let gen = EtcdStatementGenerator( prefix: "", columns: ["Key", "Value", "Version", "CreateRevision", "ModRevision", "Lease"] @@ -376,7 +376,7 @@ struct EtcdStatementGeneratorUpdateTests { originalRow: ["oldkey", "myvalue", "1", "1", "1", "0"] ) - let results = gen.generateStatements( + let results = try gen.generateRowWrites( from: [change], insertedRowData: [:], deletedRowIndices: [], @@ -389,7 +389,7 @@ struct EtcdStatementGeneratorUpdateTests { } @Test("Value and key change combined") - func valueAndKeyChange() { + func valueAndKeyChange() throws { let gen = EtcdStatementGenerator( prefix: "", columns: ["Key", "Value", "Version", "CreateRevision", "ModRevision", "Lease"] @@ -405,7 +405,7 @@ struct EtcdStatementGeneratorUpdateTests { originalRow: ["oldkey", "oldval", "1", "1", "1", "0"] ) - let results = gen.generateStatements( + let results = try gen.generateRowWrites( from: [change], insertedRowData: [:], deletedRowIndices: [], @@ -418,7 +418,7 @@ struct EtcdStatementGeneratorUpdateTests { } @Test("Lease change only generates put with --lease") - func leaseChangeOnly() { + func leaseChangeOnly() throws { let gen = EtcdStatementGenerator( prefix: "", columns: ["Key", "Value", "Version", "CreateRevision", "ModRevision", "Lease"] @@ -433,7 +433,7 @@ struct EtcdStatementGeneratorUpdateTests { originalRow: ["mykey", "myvalue", "1", "1", "1", "0"] ) - let results = gen.generateStatements( + let results = try gen.generateRowWrites( from: [change], insertedRowData: [:], deletedRowIndices: [], @@ -445,7 +445,7 @@ struct EtcdStatementGeneratorUpdateTests { } @Test("Value and lease change combined") - func valueAndLeaseChange() { + func valueAndLeaseChange() throws { let gen = EtcdStatementGenerator( prefix: "", columns: ["Key", "Value", "Version", "CreateRevision", "ModRevision", "Lease"] @@ -461,7 +461,7 @@ struct EtcdStatementGeneratorUpdateTests { originalRow: ["mykey", "oldval", "1", "1", "1", "0"] ) - let results = gen.generateStatements( + let results = try gen.generateRowWrites( from: [change], insertedRowData: [:], deletedRowIndices: [], @@ -472,7 +472,7 @@ struct EtcdStatementGeneratorUpdateTests { #expect(results[0].statement == "put mykey newval --lease=555") } - @Test("Update with empty new key is skipped") + @Test("Update with empty new key is refused") func updateEmptyNewKey() { let gen = EtcdStatementGenerator( prefix: "", @@ -488,18 +488,18 @@ struct EtcdStatementGeneratorUpdateTests { originalRow: ["mykey", "value", "1", "1", "1", "0"] ) - let results = gen.generateStatements( - from: [change], - insertedRowData: [:], - deletedRowIndices: [], - insertedRowIndices: [] - ) - - #expect(results.isEmpty) + #expect(throws: PluginRowWriteRefusal(rowIndex: 0, reason: "A key needs a name.")) { + try gen.generateRowWrites( + from: [change], + insertedRowData: [:], + deletedRowIndices: [], + insertedRowIndices: [] + ) + } } @Test("Update with no cell changes produces nothing") - func updateNoCellChanges() { + func updateNoCellChanges() throws { let gen = EtcdStatementGenerator( prefix: "", columns: ["Key", "Value", "Version", "CreateRevision", "ModRevision", "Lease"] @@ -512,7 +512,7 @@ struct EtcdStatementGeneratorUpdateTests { originalRow: ["mykey", "value", "1", "1", "1", "0"] ) - let results = gen.generateStatements( + let results = try gen.generateRowWrites( from: [change], insertedRowData: [:], deletedRowIndices: [], @@ -523,7 +523,7 @@ struct EtcdStatementGeneratorUpdateTests { } @Test("Update with lease set to 0 omits --lease flag") - func updateLeaseToZero() { + func updateLeaseToZero() throws { let gen = EtcdStatementGenerator( prefix: "", columns: ["Key", "Value", "Version", "CreateRevision", "ModRevision", "Lease"] @@ -538,7 +538,7 @@ struct EtcdStatementGeneratorUpdateTests { originalRow: ["mykey", "myvalue", "1", "1", "1", "12345"] ) - let results = gen.generateStatements( + let results = try gen.generateRowWrites( from: [change], insertedRowData: [:], deletedRowIndices: [], @@ -554,7 +554,7 @@ struct EtcdStatementGeneratorUpdateTests { struct EtcdStatementGeneratorDeleteTests { @Test("Basic delete generates del command") - func basicDelete() { + func basicDelete() throws { let gen = EtcdStatementGenerator( prefix: "", columns: ["Key", "Value", "Version", "CreateRevision", "ModRevision", "Lease"] @@ -567,7 +567,7 @@ struct EtcdStatementGeneratorDeleteTests { originalRow: ["mykey", "myvalue", "1", "1", "1", "0"] ) - let results = gen.generateStatements( + let results = try gen.generateRowWrites( from: [change], insertedRowData: [:], deletedRowIndices: [0], @@ -579,7 +579,7 @@ struct EtcdStatementGeneratorDeleteTests { } @Test("Delete with key containing spaces is quoted") - func deleteKeyWithSpaces() { + func deleteKeyWithSpaces() throws { let gen = EtcdStatementGenerator( prefix: "", columns: ["Key", "Value", "Version", "CreateRevision", "ModRevision", "Lease"] @@ -592,7 +592,7 @@ struct EtcdStatementGeneratorDeleteTests { originalRow: ["my key", "value", "1", "1", "1", "0"] ) - let results = gen.generateStatements( + let results = try gen.generateRowWrites( from: [change], insertedRowData: [:], deletedRowIndices: [0], @@ -604,7 +604,7 @@ struct EtcdStatementGeneratorDeleteTests { } @Test("Delete not in deletedRowIndices is skipped") - func deleteNotInIndices() { + func deleteNotInIndices() throws { let gen = EtcdStatementGenerator( prefix: "", columns: ["Key", "Value", "Version", "CreateRevision", "ModRevision", "Lease"] @@ -617,7 +617,7 @@ struct EtcdStatementGeneratorDeleteTests { originalRow: ["mykey", "value", "1", "1", "1", "0"] ) - let results = gen.generateStatements( + let results = try gen.generateRowWrites( from: [change], insertedRowData: [:], deletedRowIndices: [], @@ -632,7 +632,7 @@ struct EtcdStatementGeneratorDeleteTests { struct EtcdStatementGeneratorBatchTests { @Test("Multiple changes in one batch") - func multipleBatch() { + func multipleBatch() throws { let gen = EtcdStatementGenerator( prefix: "", columns: ["Key", "Value", "Version", "CreateRevision", "ModRevision", "Lease"] @@ -665,7 +665,7 @@ struct EtcdStatementGeneratorBatchTests { 0: ["newkey", "newval", nil, nil, nil, nil] ] - let results = gen.generateStatements( + let results = try gen.generateRowWrites( from: [insertChange, updateChange, deleteChange], insertedRowData: insertedData, deletedRowIndices: [2], @@ -676,10 +676,11 @@ struct EtcdStatementGeneratorBatchTests { #expect(results[0].statement == "put newkey newval") #expect(results[1].statement == "put existingkey new") #expect(results[2].statement == "del delkey") + #expect(results.map(\.rowIndices) == [[0], [1], [2]]) } @Test("Insert not in insertedRowIndices is skipped") - func insertNotInIndices() { + func insertNotInIndices() throws { let gen = EtcdStatementGenerator( prefix: "", columns: ["Key", "Value", "Version", "CreateRevision", "ModRevision", "Lease"] @@ -696,7 +697,7 @@ struct EtcdStatementGeneratorBatchTests { 5: ["key", "val", nil, nil, nil, nil] ] - let results = gen.generateStatements( + let results = try gen.generateRowWrites( from: [change], insertedRowData: insertedData, deletedRowIndices: [], @@ -706,3 +707,94 @@ struct EtcdStatementGeneratorBatchTests { #expect(results.isEmpty) } } + +// MARK: - Values a put cannot carry + +struct EtcdStatementGeneratorRefusalTests { + private static let columns = ["Key", "Value", "Version", "CreateRevision", "ModRevision", "Lease"] + private static let original: [PluginCellValue] = ["mykey", "oldval", "3", "1", "7", ""] + + private func writes( + _ cells: [(columnIndex: Int, columnName: String, oldValue: PluginCellValue, newValue: PluginCellValue)], + original: [PluginCellValue] = EtcdStatementGeneratorRefusalTests.original + ) throws -> [PluginRowWrite] { + try EtcdStatementGenerator(prefix: "", columns: Self.columns).generateRowWrites( + from: [PluginRowChange(rowIndex: 0, type: .update, cellChanges: cells, originalRow: original)], + insertedRowData: [:], + deletedRowIndices: [], + insertedRowIndices: [] + ) + } + + /// An empty value reads back as a NULL cell, so NULL is how the grid spells one. + @Test("A Value set to NULL writes an empty value, never the old one") + func nullValueWritesEmpty() throws { + let written = try writes([(columnIndex: 1, columnName: "Value", oldValue: "oldval", newValue: .null)]) + #expect(written.map(\.statement) == ["put mykey \"\""]) + #expect(written.map(\.rowIndices) == [[0]]) + } + + @Test("A Lease set to NULL on its own removes the lease") + func nullLeaseDetaches() throws { + let written = try writes( + [(columnIndex: 5, columnName: "Lease", oldValue: "7b", newValue: .null)], + original: ["mykey", "oldval", "3", "1", "7", "7b"] + ) + #expect(written.map(\.statement) == ["put mykey oldval"]) + } + + @Test("An edit to a column etcd sets is refused, even beside a Value edit it could write") + func serverOwnedColumnRefused() { + let refusal = PluginRowWriteRefusal(rowIndex: 0, reason: "'Version' is set by etcd and cannot be edited.") + #expect(throws: refusal) { + try writes([ + (columnIndex: 1, columnName: "Value", oldValue: "oldval", newValue: "newval"), + (columnIndex: 2, columnName: "Version", oldValue: "3", newValue: "4"), + ]) + } + } + + @Test("A key renamed to NULL is refused rather than kept") + func keyRenamedToNullRefused() { + #expect(throws: PluginRowWriteRefusal(rowIndex: 0, reason: "A key needs a name.")) { + try writes([ + (columnIndex: 0, columnName: "Key", oldValue: "mykey", newValue: .null), + (columnIndex: 1, columnName: "Value", oldValue: "oldval", newValue: "newval"), + ]) + } + } + + @Test("A revision typed into a new row is refused, and a new row's NULL revision is not") + func insertRevisionRefused() throws { + let generator = EtcdStatementGenerator(prefix: "", columns: Self.columns) + let typed = PluginRowChange( + rowIndex: 0, + type: .insert, + cellChanges: [(columnIndex: 4, columnName: "ModRevision", oldValue: .null, newValue: "9")], + originalRow: nil + ) + let refusal = PluginRowWriteRefusal(rowIndex: 0, reason: "'ModRevision' is set by etcd and cannot be edited.") + #expect(throws: refusal) { + try generator.generateRowWrites( + from: [typed], + insertedRowData: [0: ["k", "v", nil, nil, "9", nil]], + deletedRowIndices: [], + insertedRowIndices: [0] + ) + } + + let cleared = PluginRowChange( + rowIndex: 0, + type: .insert, + cellChanges: [(columnIndex: 4, columnName: "ModRevision", oldValue: "9", newValue: .null)], + originalRow: nil + ) + let written = try generator.generateRowWrites( + from: [cleared], + insertedRowData: [0: ["k", "v", nil, nil, nil, nil]], + deletedRowIndices: [], + insertedRowIndices: [0] + ) + #expect(written.map(\.statement) == ["put k v"]) + } +} diff --git a/TableProTests/Plugins/RedisDatabaseTargetTests.swift b/TableProTests/Plugins/RedisDatabaseTargetTests.swift index c56b6ea36d..ce31f2c1d7 100644 --- a/TableProTests/Plugins/RedisDatabaseTargetTests.swift +++ b/TableProTests/Plugins/RedisDatabaseTargetTests.swift @@ -308,9 +308,9 @@ struct RedisAbandonedVisitTests { } struct RedisWriteAddressingTests { - private static let writes: [RedisDatabaseTarget.Statement] = [ - (statement: "SET \"k\" \"v\"", parameters: []), - (statement: "DEL \"old\"", parameters: []), + private static let writes = [ + PluginRowWrite(statement: "SET \"k\" \"v\"", rowIndices: [0]), + PluginRowWrite(statement: "DEL \"old\"", rowIndices: [1, 2]), ] @Test("Writes for the database the session belongs on are unchanged") @@ -323,6 +323,7 @@ struct RedisWriteAddressingTests { func otherDatabase() { let addressed = RedisDatabaseTarget.addressing(Self.writes, toDatabase: 3, from: 5, insideTransaction: true) #expect(addressed.map(\.statement) == ["SELECT 3", "SET \"k\" \"v\"", "DEL \"old\"", "SELECT 5"]) + #expect(addressed.map(\.rowIndices) == [[], [0], [1, 2], []]) } @Test("A table that names no database, or no writes, is left alone") diff --git a/TableProTests/Plugins/RedisKeySlotTests.swift b/TableProTests/Plugins/RedisKeySlotTests.swift index a1262922b1..e8d0c9ad3a 100644 --- a/TableProTests/Plugins/RedisKeySlotTests.swift +++ b/TableProTests/Plugins/RedisKeySlotTests.swift @@ -65,7 +65,7 @@ struct RedisKeySlotHashTagTests { @Test("An empty tag falls back to hashing the whole key") func emptyTagHashesWholeKey() { #expect(RedisKeySlot.slot(for: "somekey{}") != RedisKeySlot.slot(for: "")) - #expect(RedisKeySlot.slot(for: "foo{}{bar}") == RedisKeySlot.slot(for: "foo{}{bar}")) + #expect(RedisKeySlot.slot(for: "foo{}{bar}") != RedisKeySlot.slot(for: "bar")) } @Test("An unclosed brace is not a tag") @@ -101,17 +101,17 @@ struct RedisKeySlotCrossSlotTests { struct RedisKeySlotGroupingTests { @Test("Keys group by slot in the order each slot first appears") func firstSeenOrder() { - let groups = RedisKeySlot.groupedBySlot(["allowed:1", "{u}a", "forbidden:1", "{u}b"]) + let groups = RedisKeySlot.groupedBySlot(["allowed:1", "{u}a", "forbidden:1", "{u}b"]) { $0 } #expect(groups == [["allowed:1"], ["{u}a", "{u}b"], ["forbidden:1"]]) } @Test("A duplicate key stays in its group") func keepsDuplicates() { - #expect(RedisKeySlot.groupedBySlot(["a", "a", "b"]) == [["a", "a"], ["b"]]) + #expect(RedisKeySlot.groupedBySlot(["a", "a", "b"]) { $0 } == [["a", "a"], ["b"]]) } @Test("No keys make no groups") func empty() { - #expect(RedisKeySlot.groupedBySlot([]).isEmpty) + #expect(RedisKeySlot.groupedBySlot([String]()) { $0 }.isEmpty) } } diff --git a/TableProTests/Plugins/RedisNamedDatabaseWriteTests.swift b/TableProTests/Plugins/RedisNamedDatabaseWriteTests.swift index ab168afce6..34a8d20169 100644 --- a/TableProTests/Plugins/RedisNamedDatabaseWriteTests.swift +++ b/TableProTests/Plugins/RedisNamedDatabaseWriteTests.swift @@ -48,15 +48,16 @@ struct RedisDatabasePrefixParsingTests { } struct RedisNamedDatabaseAddressingTests { - private static let writes: [RedisDatabaseTarget.Statement] = [ - (statement: "SET \"k\" \"v\"", parameters: []), - (statement: "DEL old", parameters: []), + private static let writes = [ + PluginRowWrite(statement: "SET \"k\" \"v\"", rowIndices: [0]), + PluginRowWrite(statement: "DEL old", rowIndices: [1, 2]), ] @Test("Without a transaction every write names the database and no SELECT is sent") func namesEachWrite() { let addressed = RedisDatabaseTarget.addressing(Self.writes, toDatabase: 3, from: 0, insideTransaction: false) #expect(addressed.map(\.statement) == ["DB 3 SET \"k\" \"v\"", "DB 3 DEL old"]) + #expect(addressed.map(\.rowIndices) == [[0], [1, 2]]) } @Test("Writes for the database the session belongs on are unchanged") diff --git a/TableProTests/Plugins/RedisStatementGeneratorTests.swift b/TableProTests/Plugins/RedisStatementGeneratorTests.swift index 5d4498e806..fdd8a341b1 100644 --- a/TableProTests/Plugins/RedisStatementGeneratorTests.swift +++ b/TableProTests/Plugins/RedisStatementGeneratorTests.swift @@ -6,15 +6,14 @@ // import Foundation -import Testing import TableProPluginKit +import Testing struct RedisStatementGeneratorTests { - // MARK: - INSERT @Test("Basic insert generates SET command") - func basicInsert() { + func basicInsert() throws { let gen = RedisStatementGenerator( namespaceName: "cache:", columns: ["Key", "Value", "TTL"] @@ -31,7 +30,7 @@ struct RedisStatementGeneratorTests { 0: ["cache:mykey", "hello", nil] ] - let results = gen.generateStatements( + let results = try gen.generateRowWrites( from: [change], insertedRowData: insertedData, deletedRowIndices: [], @@ -43,7 +42,7 @@ struct RedisStatementGeneratorTests { } @Test("Insert with TTL generates SET and EXPIRE") - func insertWithTtl() { + func insertWithTtl() throws { let gen = RedisStatementGenerator( namespaceName: "", columns: ["Key", "Value", "TTL"] @@ -60,7 +59,7 @@ struct RedisStatementGeneratorTests { 0: ["session:abc", "data", "3600"] ] - let results = gen.generateStatements( + let results = try gen.generateRowWrites( from: [change], insertedRowData: insertedData, deletedRowIndices: [], @@ -73,7 +72,7 @@ struct RedisStatementGeneratorTests { } @Test("Insert with TTL=0 generates SET only") - func insertWithZeroTtl() { + func insertWithZeroTtl() throws { let gen = RedisStatementGenerator( namespaceName: "", columns: ["Key", "Value", "TTL"] @@ -90,7 +89,7 @@ struct RedisStatementGeneratorTests { 0: ["mykey", "value", "0"] ] - let results = gen.generateStatements( + let results = try gen.generateRowWrites( from: [change], insertedRowData: insertedData, deletedRowIndices: [], @@ -101,7 +100,43 @@ struct RedisStatementGeneratorTests { #expect(results[0].statement == "SET mykey value") } - @Test("Insert without key is skipped") + @Test("Insert with a negative TTL other than -1 is refused, not written without its expiry") + func insertWithNegativeTtl() { + let gen = RedisStatementGenerator( + namespaceName: "", + columns: ["Key", "Value", "TTL"] + ) + let change = PluginRowChange(rowIndex: 0, type: .insert, cellChanges: [], originalRow: nil) + + #expect(throws: PluginRowWriteRefusal.self) { + try gen.generateRowWrites( + from: [change], + insertedRowData: [0: ["mykey", "value", "-5"]], + deletedRowIndices: [], + insertedRowIndices: [0] + ) + } + } + + @Test("Insert with TTL -1 generates SET only") + func insertWithNoExpiryTtl() throws { + let gen = RedisStatementGenerator( + namespaceName: "", + columns: ["Key", "Value", "TTL"] + ) + let change = PluginRowChange(rowIndex: 0, type: .insert, cellChanges: [], originalRow: nil) + + let results = try gen.generateRowWrites( + from: [change], + insertedRowData: [0: ["mykey", "value", "-1"]], + deletedRowIndices: [], + insertedRowIndices: [0] + ) + + #expect(results.map(\.statement) == ["SET mykey value"]) + } + + @Test("Insert without key is refused") func insertWithoutKey() { let gen = RedisStatementGenerator( namespaceName: "", @@ -119,17 +154,17 @@ struct RedisStatementGeneratorTests { 0: [nil, "value", nil] ] - let results = gen.generateStatements( - from: [change], - insertedRowData: insertedData, - deletedRowIndices: [], - insertedRowIndices: [0] - ) - - #expect(results.isEmpty) + #expect(throws: PluginRowWriteRefusal(rowIndex: 0, reason: "A new key needs a name.")) { + try gen.generateRowWrites( + from: [change], + insertedRowData: insertedData, + deletedRowIndices: [], + insertedRowIndices: [0] + ) + } } - @Test("Insert with empty key is skipped") + @Test("Insert with empty key is refused") func insertEmptyKey() { let gen = RedisStatementGenerator( namespaceName: "", @@ -147,18 +182,18 @@ struct RedisStatementGeneratorTests { 0: ["", "value", nil] ] - let results = gen.generateStatements( - from: [change], - insertedRowData: insertedData, - deletedRowIndices: [], - insertedRowIndices: [0] - ) - - #expect(results.isEmpty) + #expect(throws: PluginRowWriteRefusal(rowIndex: 0, reason: "A new key needs a name.")) { + try gen.generateRowWrites( + from: [change], + insertedRowData: insertedData, + deletedRowIndices: [], + insertedRowIndices: [0] + ) + } } @Test("Insert with nil value uses empty string") - func insertNilValueUsesEmpty() { + func insertNilValueUsesEmpty() throws { let gen = RedisStatementGenerator( namespaceName: "", columns: ["Key", "Value", "TTL"] @@ -175,7 +210,7 @@ struct RedisStatementGeneratorTests { 0: ["mykey", nil, nil] ] - let results = gen.generateStatements( + let results = try gen.generateRowWrites( from: [change], insertedRowData: insertedData, deletedRowIndices: [], @@ -187,7 +222,7 @@ struct RedisStatementGeneratorTests { } @Test("Insert uses cellChanges as fallback") - func insertFallbackToCellChanges() { + func insertFallbackToCellChanges() throws { let gen = RedisStatementGenerator( namespaceName: "", columns: ["Key", "Value", "TTL"] @@ -203,7 +238,7 @@ struct RedisStatementGeneratorTests { originalRow: nil ) - let results = gen.generateStatements( + let results = try gen.generateRowWrites( from: [change], insertedRowData: [:], deletedRowIndices: [], @@ -215,7 +250,7 @@ struct RedisStatementGeneratorTests { } @Test("Insert not in insertedRowIndices is skipped") - func insertNotInIndices() { + func insertNotInIndices() throws { let gen = RedisStatementGenerator( namespaceName: "", columns: ["Key", "Value", "TTL"] @@ -228,7 +263,7 @@ struct RedisStatementGeneratorTests { originalRow: nil ) - let results = gen.generateStatements( + let results = try gen.generateRowWrites( from: [change], insertedRowData: [5: ["key", "val", nil]], deletedRowIndices: [], @@ -241,7 +276,7 @@ struct RedisStatementGeneratorTests { // MARK: - UPDATE @Test("Update value generates SET with new value") - func updateValue() { + func updateValue() throws { let gen = RedisStatementGenerator( namespaceName: "", columns: ["Key", "Value", "TTL"] @@ -256,7 +291,7 @@ struct RedisStatementGeneratorTests { originalRow: ["mykey", "old", "3600"] ) - let results = gen.generateStatements( + let results = try gen.generateRowWrites( from: [change], insertedRowData: [:], deletedRowIndices: [], @@ -268,7 +303,7 @@ struct RedisStatementGeneratorTests { } @Test("Update key generates RENAME then SET") - func updateKey() { + func updateKey() throws { let gen = RedisStatementGenerator( namespaceName: "", columns: ["Key", "Value", "TTL"] @@ -284,7 +319,7 @@ struct RedisStatementGeneratorTests { originalRow: ["oldkey", "val", "-1"] ) - let results = gen.generateStatements( + let results = try gen.generateRowWrites( from: [change], insertedRowData: [:], deletedRowIndices: [], @@ -297,7 +332,7 @@ struct RedisStatementGeneratorTests { } @Test("Update key only (no value change) generates just RENAME") - func updateKeyOnly() { + func updateKeyOnly() throws { let gen = RedisStatementGenerator( namespaceName: "", columns: ["Key", "Value", "TTL"] @@ -312,7 +347,7 @@ struct RedisStatementGeneratorTests { originalRow: ["oldkey", "val", "-1"] ) - let results = gen.generateStatements( + let results = try gen.generateRowWrites( from: [change], insertedRowData: [:], deletedRowIndices: [], @@ -324,7 +359,7 @@ struct RedisStatementGeneratorTests { } @Test("Update TTL generates EXPIRE") - func updateTtl() { + func updateTtl() throws { let gen = RedisStatementGenerator( namespaceName: "", columns: ["Key", "Value", "TTL"] @@ -339,7 +374,7 @@ struct RedisStatementGeneratorTests { originalRow: ["mykey", "value", "3600"] ) - let results = gen.generateStatements( + let results = try gen.generateRowWrites( from: [change], insertedRowData: [:], deletedRowIndices: [], @@ -351,7 +386,7 @@ struct RedisStatementGeneratorTests { } @Test("Remove TTL (set to nil) generates PERSIST") - func removeTtlNil() { + func removeTtlNil() throws { let gen = RedisStatementGenerator( namespaceName: "", columns: ["Key", "Value", "TTL"] @@ -366,7 +401,7 @@ struct RedisStatementGeneratorTests { originalRow: ["mykey", "value", "3600"] ) - let results = gen.generateStatements( + let results = try gen.generateRowWrites( from: [change], insertedRowData: [:], deletedRowIndices: [], @@ -378,7 +413,7 @@ struct RedisStatementGeneratorTests { } @Test("Remove TTL (set to -1) generates PERSIST") - func removeTtlMinusOne() { + func removeTtlMinusOne() throws { let gen = RedisStatementGenerator( namespaceName: "", columns: ["Key", "Value", "TTL"] @@ -393,7 +428,7 @@ struct RedisStatementGeneratorTests { originalRow: ["mykey", "value", "3600"] ) - let results = gen.generateStatements( + let results = try gen.generateRowWrites( from: [change], insertedRowData: [:], deletedRowIndices: [], @@ -405,7 +440,7 @@ struct RedisStatementGeneratorTests { } @Test("Update with empty cellChanges produces no statements") - func updateEmptyCellChanges() { + func updateEmptyCellChanges() throws { let gen = RedisStatementGenerator( namespaceName: "", columns: ["Key", "Value", "TTL"] @@ -418,7 +453,7 @@ struct RedisStatementGeneratorTests { originalRow: ["mykey", "value", "-1"] ) - let results = gen.generateStatements( + let results = try gen.generateRowWrites( from: [change], insertedRowData: [:], deletedRowIndices: [], @@ -428,7 +463,7 @@ struct RedisStatementGeneratorTests { #expect(results.isEmpty) } - @Test("Update without original row key is skipped") + @Test("Update without original row key is refused") func updateNoKey() { let gen = RedisStatementGenerator( namespaceName: "", @@ -444,20 +479,23 @@ struct RedisStatementGeneratorTests { originalRow: nil ) - let results = gen.generateStatements( - from: [change], - insertedRowData: [:], - deletedRowIndices: [], - insertedRowIndices: [] + let refusal = PluginRowWriteRefusal( + rowIndex: 0, reason: "This key's name is not text, so it cannot be addressed from the grid." ) - - #expect(results.isEmpty) + #expect(throws: refusal) { + try gen.generateRowWrites( + from: [change], + insertedRowData: [:], + deletedRowIndices: [], + insertedRowIndices: [] + ) + } } // MARK: - DELETE @Test("Single delete generates DEL command") - func singleDelete() { + func singleDelete() throws { let gen = RedisStatementGenerator( namespaceName: "", columns: ["Key", "Value", "TTL"] @@ -470,7 +508,7 @@ struct RedisStatementGeneratorTests { originalRow: ["mykey", "value", "-1"] ) - let results = gen.generateStatements( + let results = try gen.generateRowWrites( from: [change], insertedRowData: [:], deletedRowIndices: [0], @@ -482,7 +520,7 @@ struct RedisStatementGeneratorTests { } @Test("Bulk delete batches keys into single DEL command") - func bulkDelete() { + func bulkDelete() throws { let gen = RedisStatementGenerator( namespaceName: "", columns: ["Key", "Value", "TTL"] @@ -494,7 +532,7 @@ struct RedisStatementGeneratorTests { PluginRowChange(rowIndex: 2, type: .delete, cellChanges: [], originalRow: ["key3", "v3", "-1"]) ] - let results = gen.generateStatements( + let results = try gen.generateRowWrites( from: changes, insertedRowData: [:], deletedRowIndices: [0, 1, 2], @@ -508,7 +546,7 @@ struct RedisStatementGeneratorTests { /// A cluster splits a DEL by slot, and one slot can refuse after another ran. Deleting a slot /// per statement makes each one all or nothing, so a save can say how many went through. @Test("On a partitioned keyspace each hash slot gets its own DEL") - func deletePerHashSlot() { + func deletePerHashSlot() throws { let gen = RedisStatementGenerator( namespaceName: "", columns: ["Key", "Value", "TTL"], @@ -518,7 +556,7 @@ struct RedisStatementGeneratorTests { PluginRowChange(rowIndex: index, type: .delete, cellChanges: [], originalRow: [.text(key), "v", "-1"]) } - let results = gen.generateStatements( + let results = try gen.generateRowWrites( from: changes, insertedRowData: [:], deletedRowIndices: [0, 1, 2, 3], @@ -529,7 +567,7 @@ struct RedisStatementGeneratorTests { } @Test("Per-slot deletes quote each key the way a single DEL does") - func deletePerHashSlotQuotes() { + func deletePerHashSlotQuotes() throws { let gen = RedisStatementGenerator( namespaceName: "", columns: ["Key", "Value", "TTL"], @@ -539,7 +577,7 @@ struct RedisStatementGeneratorTests { PluginRowChange(rowIndex: index, type: .delete, cellChanges: [], originalRow: [.text(key), "v", "-1"]) } - let results = gen.generateStatements( + let results = try gen.generateRowWrites( from: changes, insertedRowData: [:], deletedRowIndices: [0, 1], @@ -550,7 +588,7 @@ struct RedisStatementGeneratorTests { } @Test("Delete not in deletedRowIndices is skipped") - func deleteNotInIndices() { + func deleteNotInIndices() throws { let gen = RedisStatementGenerator( namespaceName: "", columns: ["Key", "Value", "TTL"] @@ -563,7 +601,7 @@ struct RedisStatementGeneratorTests { originalRow: ["mykey", "val", "-1"] ) - let results = gen.generateStatements( + let results = try gen.generateRowWrites( from: [change], insertedRowData: [:], deletedRowIndices: [0], // does not contain 5 @@ -573,7 +611,7 @@ struct RedisStatementGeneratorTests { #expect(results.isEmpty) } - @Test("Delete without original row key is skipped") + @Test("Delete without original row key is refused") func deleteNoOriginalRow() { let gen = RedisStatementGenerator( namespaceName: "", @@ -587,20 +625,23 @@ struct RedisStatementGeneratorTests { originalRow: nil ) - let results = gen.generateStatements( - from: [change], - insertedRowData: [:], - deletedRowIndices: [0], - insertedRowIndices: [] + let refusal = PluginRowWriteRefusal( + rowIndex: 0, reason: "This key's name is not text, so it cannot be addressed from the grid." ) - - #expect(results.isEmpty) + #expect(throws: refusal) { + try gen.generateRowWrites( + from: [change], + insertedRowData: [:], + deletedRowIndices: [0], + insertedRowIndices: [] + ) + } } // MARK: - Values with Spaces @Test("Values with spaces are quoted") - func valuesWithSpacesQuoted() { + func valuesWithSpacesQuoted() throws { let gen = RedisStatementGenerator( namespaceName: "", columns: ["Key", "Value", "TTL"] @@ -617,7 +658,7 @@ struct RedisStatementGeneratorTests { 0: ["my key", "hello world", nil] ] - let results = gen.generateStatements( + let results = try gen.generateRowWrites( from: [change], insertedRowData: insertedData, deletedRowIndices: [], @@ -629,7 +670,7 @@ struct RedisStatementGeneratorTests { } @Test("Values with quotes are escaped") - func valuesWithQuotesEscaped() { + func valuesWithQuotesEscaped() throws { let gen = RedisStatementGenerator( namespaceName: "", columns: ["Key", "Value", "TTL"] @@ -646,7 +687,7 @@ struct RedisStatementGeneratorTests { 0: ["key", "say \"hello\"", nil] ] - let results = gen.generateStatements( + let results = try gen.generateRowWrites( from: [change], insertedRowData: insertedData, deletedRowIndices: [], @@ -660,7 +701,7 @@ struct RedisStatementGeneratorTests { // MARK: - Mixed Operations @Test("Mixed insert, update, and delete in one batch") - func mixedOperations() { + func mixedOperations() throws { let gen = RedisStatementGenerator( namespaceName: "", columns: ["Key", "Value", "TTL"] @@ -693,7 +734,7 @@ struct RedisStatementGeneratorTests { 0: ["newkey", "newval", nil] ] - let results = gen.generateStatements( + let results = try gen.generateRowWrites( from: changes, insertedRowData: insertedData, deletedRowIndices: [2], @@ -707,7 +748,7 @@ struct RedisStatementGeneratorTests { } @Test("Update value and TTL together") - func updateValueAndTtl() { + func updateValueAndTtl() throws { let gen = RedisStatementGenerator( namespaceName: "", columns: ["Key", "Value", "TTL"] @@ -723,7 +764,7 @@ struct RedisStatementGeneratorTests { originalRow: ["mykey", "old", "-1"] ) - let results = gen.generateStatements( + let results = try gen.generateRowWrites( from: [change], insertedRowData: [:], deletedRowIndices: [], @@ -736,7 +777,7 @@ struct RedisStatementGeneratorTests { } @Test("Update key, value, and TTL together") - func updateKeyValueAndTtl() { + func updateKeyValueAndTtl() throws { let gen = RedisStatementGenerator( namespaceName: "", columns: ["Key", "Value", "TTL"] @@ -753,7 +794,7 @@ struct RedisStatementGeneratorTests { originalRow: ["oldkey", "old", "-1"] ) - let results = gen.generateStatements( + let results = try gen.generateRowWrites( from: [change], insertedRowData: [:], deletedRowIndices: [], @@ -771,7 +812,7 @@ struct RedisStatementGeneratorBrowseColumnTests { private static let browseColumns = ["Key", "Type", "TTL", "Length", "Value"] @Test("A string value update still resolves with the Length column present") - func valueUpdateWithLengthColumn() { + func valueUpdateWithLengthColumn() throws { let gen = RedisStatementGenerator(namespaceName: "", columns: Self.browseColumns) let change = PluginRowChange( @@ -783,7 +824,7 @@ struct RedisStatementGeneratorBrowseColumnTests { originalRow: ["mykey", "STRING", "-1", "3", "old"] ) - let results = gen.generateStatements( + let results = try gen.generateRowWrites( from: [change], insertedRowData: [:], deletedRowIndices: [], @@ -795,7 +836,7 @@ struct RedisStatementGeneratorBrowseColumnTests { } @Test("A whole string value is written back, however long it is") - func longValueIsWrittenWhole() { + func longValueIsWrittenWhole() throws { let gen = RedisStatementGenerator(namespaceName: "", columns: Self.browseColumns) let long = String(repeating: "a", count: 5_000) @@ -808,7 +849,7 @@ struct RedisStatementGeneratorBrowseColumnTests { originalRow: ["mykey", "STRING", "-1", "3", "old"] ) - let results = gen.generateStatements( + let results = try gen.generateRowWrites( from: [change], insertedRowData: [:], deletedRowIndices: [], @@ -819,8 +860,8 @@ struct RedisStatementGeneratorBrowseColumnTests { #expect(results[0].statement == "SET mykey \(long)") } - @Test("A collection value update is skipped so the structure survives") - func collectionValueUpdateSkipped() { + @Test("A collection value update is refused so the structure survives") + func collectionValueUpdateRefused() { let gen = RedisStatementGenerator(namespaceName: "", columns: Self.browseColumns) let change = PluginRowChange( @@ -832,20 +873,24 @@ struct RedisStatementGeneratorBrowseColumnTests { originalRow: ["mylist", "LIST", "-1", "1", "[\"a\"]"] ) - let results = gen.generateStatements( - from: [change], - insertedRowData: [:], - deletedRowIndices: [], - insertedRowIndices: [] - ) - - #expect(results.isEmpty) + let refusal = PluginRowWriteRefusal( + rowIndex: 0, + reason: "The value of a list key cannot be edited in the grid. Change it with a command in the query editor." + ) + #expect(throws: refusal) { + try gen.generateRowWrites( + from: [change], + insertedRowData: [:], + deletedRowIndices: [], + insertedRowIndices: [] + ) + } } /// The Type cell is NULL when the server would not say, and `SET` over a hash the user cannot /// see replaces the hash. - @Test("A value update on a key of unknown type is skipped") - func unknownTypeValueUpdateSkipped() { + @Test("A value update on a key of unknown type is refused") + func unknownTypeValueUpdateRefused() { let gen = RedisStatementGenerator(namespaceName: "", columns: Self.browseColumns) let change = PluginRowChange( @@ -857,18 +902,21 @@ struct RedisStatementGeneratorBrowseColumnTests { originalRow: ["other:h", nil, nil, nil, nil] ) - let results = gen.generateStatements( - from: [change], - insertedRowData: [:], - deletedRowIndices: [], - insertedRowIndices: [] + let refusal = PluginRowWriteRefusal( + rowIndex: 0, reason: "The key's type is unknown, so its value cannot be written safely." ) - - #expect(results.isEmpty) + #expect(throws: refusal) { + try gen.generateRowWrites( + from: [change], + insertedRowData: [:], + deletedRowIndices: [], + insertedRowIndices: [] + ) + } } @Test("A key of unknown type still takes a TTL change") - func unknownTypeTtlUpdateApplies() { + func unknownTypeTtlUpdateApplies() throws { let gen = RedisStatementGenerator(namespaceName: "", columns: Self.browseColumns) let change = PluginRowChange( @@ -880,7 +928,7 @@ struct RedisStatementGeneratorBrowseColumnTests { originalRow: ["other:h", nil, nil, nil, nil] ) - let results = gen.generateStatements( + let results = try gen.generateRowWrites( from: [change], insertedRowData: [:], deletedRowIndices: [], @@ -891,7 +939,7 @@ struct RedisStatementGeneratorBrowseColumnTests { } @Test("An insert reads its cells by name, not by position") - func insertResolvesColumnsByName() { + func insertResolvesColumnsByName() throws { let gen = RedisStatementGenerator(namespaceName: "", columns: Self.browseColumns) let change = PluginRowChange(rowIndex: 0, type: .insert, cellChanges: [], originalRow: nil) @@ -899,7 +947,7 @@ struct RedisStatementGeneratorBrowseColumnTests { 0: ["mykey", "STRING", "600", nil, "hello"] ] - let results = gen.generateStatements( + let results = try gen.generateRowWrites( from: [change], insertedRowData: insertedData, deletedRowIndices: [], @@ -911,3 +959,123 @@ struct RedisStatementGeneratorBrowseColumnTests { #expect(results[1].statement == "EXPIRE mykey 600") } } + +struct RedisStatementGeneratorRefusalTests { + private static let browseColumns = ["Key", "Type", "TTL", "Length", "Value"] + private static let stringRow: [PluginCellValue] = ["mykey", "string", "-1", "3", "old"] + private static let invalidTTL = "TTL has to be a whole number of seconds above 0, or -1 or NULL for no expiry." + + private func generator(batching: RedisDeleteBatching = .singleCommand) -> RedisStatementGenerator { + RedisStatementGenerator(namespaceName: "", columns: Self.browseColumns, deleteBatching: batching) + } + + private func edit( + _ cells: [(columnIndex: Int, columnName: String, oldValue: PluginCellValue, newValue: PluginCellValue)], + original: [PluginCellValue] = RedisStatementGeneratorRefusalTests.stringRow + ) -> PluginRowChange { + PluginRowChange(rowIndex: 0, type: .update, cellChanges: cells, originalRow: original) + } + + private func writes(for changes: [PluginRowChange], deleting: Set = []) throws -> [PluginRowWrite] { + try generator().generateRowWrites( + from: changes, insertedRowData: [:], deletedRowIndices: deleting, insertedRowIndices: [] + ) + } + + @Test("A Value edit on a list key refuses the row even beside a TTL edit it could write") + func collectionValueBesideTtlRefusesTheRow() { + let change = edit( + [ + (columnIndex: 4, columnName: "Value", oldValue: "[\"a\"]", newValue: "[\"b\"]"), + (columnIndex: 2, columnName: "TTL", oldValue: "-1", newValue: "60"), + ], + original: ["mylist", "list", "-1", "1", "[\"a\"]"] + ) + let refusal = PluginRowWriteRefusal( + rowIndex: 0, + reason: "The value of a list key cannot be edited in the grid. Change it with a command in the query editor." + ) + #expect(throws: refusal) { try writes(for: [change]) } + } + + @Test("A Value set to NULL is refused") + func nullValueRefused() { + let change = edit([(columnIndex: 4, columnName: "Value", oldValue: "old", newValue: .null)]) + let refusal = PluginRowWriteRefusal( + rowIndex: 0, reason: "Redis cannot store NULL as a value. Enter an empty value instead." + ) + #expect(throws: refusal) { try writes(for: [change]) } + } + + @Test("A TTL that is not a number of seconds above 0 is refused", arguments: ["0", "-5", "abc", ""]) + func invalidTtlUpdateRefused(ttl: String) { + let change = edit([(columnIndex: 2, columnName: "TTL", oldValue: "-1", newValue: .text(ttl))]) + #expect(throws: PluginRowWriteRefusal(rowIndex: 0, reason: Self.invalidTTL)) { try writes(for: [change]) } + } + + @Test("A new key whose TTL is not a number is refused") + func invalidTtlInsertRefused() { + let change = PluginRowChange(rowIndex: 0, type: .insert, cellChanges: [], originalRow: nil) + #expect(throws: PluginRowWriteRefusal(rowIndex: 0, reason: Self.invalidTTL)) { + try generator().generateRowWrites( + from: [change], + insertedRowData: [0: ["k", "string", "soon", .null, "v"]], + deletedRowIndices: [], + insertedRowIndices: [0] + ) + } + } + + @Test("A new key of a type the grid cannot build is refused") + func unsupportedInsertTypeRefused() { + let change = PluginRowChange(rowIndex: 0, type: .insert, cellChanges: [], originalRow: nil) + let refusal = PluginRowWriteRefusal( + rowIndex: 0, + reason: "A stream key cannot be added from the grid. Add it with a command in the query editor." + ) + #expect(throws: refusal) { + try generator().generateRowWrites( + from: [change], + insertedRowData: [0: ["events", "STREAM", .null, .null, "x"]], + deletedRowIndices: [], + insertedRowIndices: [0] + ) + } + } + + @Test("A Type edit on an existing key is refused") + func typeEditRefused() { + let change = edit([ + (columnIndex: 4, columnName: "Value", oldValue: "old", newValue: "new"), + (columnIndex: 1, columnName: "Type", oldValue: "string", newValue: "hash"), + ]) + let refusal = PluginRowWriteRefusal(rowIndex: 0, reason: "'Type' cannot be changed from the grid.") + #expect(throws: refusal) { try writes(for: [change]) } + } + + @Test("A key renamed to NULL is refused") + func keyRenamedToNullRefused() { + let change = edit([ + (columnIndex: 0, columnName: "Key", oldValue: "mykey", newValue: .null), + (columnIndex: 4, columnName: "Value", oldValue: "old", newValue: "new"), + ]) + let refusal = PluginRowWriteRefusal(rowIndex: 0, reason: "A key can only be renamed to text.") + #expect(throws: refusal) { try writes(for: [change]) } + } + + @Test("Every command names the change it writes, and a per-slot DEL names the rows in its slot") + func writesNameTheirChanges() throws { + let row: (String) -> [PluginCellValue] = { [.text($0), "string", "-1", "1", "v"] } + let changes = [ + edit([(columnIndex: 2, columnName: "TTL", oldValue: "-1", newValue: "60")]), + PluginRowChange(rowIndex: 1, type: .delete, cellChanges: [], originalRow: row("{u}a")), + PluginRowChange(rowIndex: 2, type: .delete, cellChanges: [], originalRow: row("x")), + PluginRowChange(rowIndex: 3, type: .delete, cellChanges: [], originalRow: row("{u}b")), + ] + let written = try generator(batching: .perHashSlot).generateRowWrites( + from: changes, insertedRowData: [:], deletedRowIndices: [1, 2, 3], insertedRowIndices: [] + ) + #expect(written.map(\.statement) == ["EXPIRE mykey 60", "DEL {u}a {u}b", "DEL x"]) + #expect(written.map(\.rowIndices) == [[0], [1, 3], [2]]) + } +} diff --git a/TableProTests/Views/Main/SaveCompletionTests.swift b/TableProTests/Views/Main/SaveCompletionTests.swift index 1d5a818e97..30e580abba 100644 --- a/TableProTests/Views/Main/SaveCompletionTests.swift +++ b/TableProTests/Views/Main/SaveCompletionTests.swift @@ -8,8 +8,8 @@ // import Foundation -import TableProPluginKit @testable import TablePro +import TableProPluginKit import Testing /// Enough of a driver for `assemblePendingStatements` to produce SQL. Without one the builder has @@ -50,7 +50,8 @@ struct SaveCompletionTests { private func makeCoordinator( safeModeLevel: SafeModeLevel = .silent, - type: DatabaseType = .mysql + type: DatabaseType = .mysql, + pluginDriver: any PluginDatabaseDriver = StubSaveDriver() ) -> (MainContentCoordinator, QueryTabManager, DataChangeManager) { var conn = TestFixtures.makeConnection(type: type) conn.safeModeLevel = safeModeLevel @@ -59,7 +60,7 @@ struct SaveCompletionTests { DatabaseManager.shared.injectSession( ConnectionSession( connection: conn, - driver: PluginDriverAdapter(connection: conn, pluginDriver: StubSaveDriver()) + driver: PluginDriverAdapter(connection: conn, pluginDriver: pluginDriver) ), for: conn.id ) @@ -130,6 +131,48 @@ struct SaveCompletionTests { #expect(errorMessage != nil) } + // MARK: - Changes the Driver Cannot Write + + @Test("A save the driver cannot fully write sends nothing and keeps every change") + func saveTheDriverCannotFullyWriteKeepsEveryChange() { + let driver = RowWriteStubDriver { changes, _, _, _ in + changes.filter { $0.type == .update }.map { + PluginRowWrite(statement: "UPDATE items SET name = 'z' WHERE id = \($0.rowIndex)", rowIndices: [$0.rowIndex]) + } + } + let (coordinator, tabManager, changeManager) = makeCoordinator(pluginDriver: driver) + tabManager.addTab(databaseName: "testdb") + changeManager.configureForTable( + tableName: "items", + columns: ["_id", "name"], + primaryKeyColumns: ["_id"], + databaseType: DatabaseType(rawValue: "MongoDB"), + generatedColumns: [] + ) + changeManager.pluginDriver = driver + changeManager.recordCellChange( + rowID: .existing(0), columnIndex: 1, columnName: "name", + oldValue: "a", newValue: "z", originalRow: ["1", "a"] + ) + changeManager.recordRowInsertion(rowID: .inserted(UUID()), values: [.null, .null]) + + var truncates: Set = [] + var deletes: Set = [] + var options: [DatabaseTreeTableRef: TableOperationOptions] = [:] + coordinator.saveChanges( + pendingTruncates: &truncates, + pendingDeletes: &deletes, + tableOperationOptions: &options + ) + + let errorMessage = tabManager.tabs.first?.execution.errorMessage ?? "" + #expect(errorMessage.hasPrefix("Cannot save changes to 'items'. The driver cannot write a new row.")) + #expect(errorMessage.contains("Nothing was saved")) + #expect(changeManager.hasChanges) + #expect(changeManager.changes.count == 2) + #expect(driver.executedQueries.isEmpty) + } + // MARK: - Pending Table Operations @Test("saveChanges with no tab selected and read-only does not crash") diff --git a/TableProTests/Views/Main/SidebarSaveCoverageTests.swift b/TableProTests/Views/Main/SidebarSaveCoverageTests.swift new file mode 100644 index 0000000000..d66256307d --- /dev/null +++ b/TableProTests/Views/Main/SidebarSaveCoverageTests.swift @@ -0,0 +1,75 @@ +// +// SidebarSaveCoverageTests.swift +// TableProTests +// + +import Foundation +@testable import TablePro +import TableProPluginKit +import Testing + +@MainActor +struct SidebarSaveCoverageTests { + private func makeCoordinator(driver: any PluginDatabaseDriver) -> MainContentCoordinator { + let tabManager = QueryTabManager() + let coordinator = MainContentCoordinator( + connection: TestFixtures.makeConnection(), + tabManager: tabManager, + changeManager: DataChangeManager(), + toolbarState: ConnectionToolbarState() + ) + var tab = QueryTab(title: "items", query: "db.items.find({})", tabType: .table, tableName: "items") + tab.execution.lastExecutedAt = Date() + tabManager.tabs.append(tab) + tabManager.selectedTabId = tab.id + + coordinator.setActiveTableRows( + TableRows.from( + queryRows: [[.text("1"), .text("a")], [.text("2"), .text("b")]], + columns: ["_id", "name"], + columnTypes: [.text(rawType: nil), .text(rawType: nil)], + hasAuthoritativeSchema: true + ), + for: tab.id + ) + coordinator.changeManager.configureForTable( + tableName: "items", + columns: ["_id", "name"], + primaryKeyColumns: ["_id"], + databaseType: DatabaseType(rawValue: "MongoDB"), + generatedColumns: [] + ) + coordinator.changeManager.pluginDriver = driver + return coordinator + } + + @Test("An inspector save across rows the driver cannot all write throws instead of writing some") + func inspectorSaveRefusesAPartialWrite() { + let firstRowOnly = RowWriteStubDriver { changes, _, _, _ in + changes.prefix(1).map { PluginRowWrite(statement: "updateOne(\($0.rowIndex))", rowIndices: [$0.rowIndex]) } + } + let coordinator = makeCoordinator(driver: firstRowOnly) + coordinator.selectionState.indices = [0, 1] + + #expect(throws: DataWriteError.changesNotWritable(table: "items", unwritten: UnwrittenRowCounts(updates: 1))) { + _ = try coordinator.sidebarEditStatements( + editedFields: [(columnIndex: 1, columnName: "name", newValue: "z")] + ) + } + } + + @Test("An inspector save the driver writes in full goes through unchanged") + func inspectorSaveWrittenInFull() throws { + let everyRow = RowWriteStubDriver { changes, _, _, _ in + changes.map { PluginRowWrite(statement: "updateOne(\($0.rowIndex))", rowIndices: [$0.rowIndex]) } + } + let coordinator = makeCoordinator(driver: everyRow) + coordinator.selectionState.indices = [0, 1] + + let statements = try coordinator.sidebarEditStatements( + editedFields: [(columnIndex: 1, columnName: "name", newValue: "z")] + ) + + #expect(statements.map(\.sql) == ["updateOne(0)", "updateOne(1)"]) + } +} diff --git a/docs/databases/elasticsearch.mdx b/docs/databases/elasticsearch.mdx index cf74a11a17..45925f02c1 100644 --- a/docs/databases/elasticsearch.mdx +++ b/docs/databases/elasticsearch.mdx @@ -39,11 +39,11 @@ The form shows no Database field: a connection reaches one cluster, and its indi The sidebar lists indices as tables, hiding names that begin with `.`. Browsing an alias, or any name that resolves to several indices, shows the union of their mappings. -Columns come from the index mapping. `object` fields flatten to dotted paths such as `address.city`. A `nested` field gets one JSON column for the whole array of objects, plus a dotted column per leaf (`identifiers.type`) whose cell is a JSON array with one entry per object, `null` where an object omits the field. Array and object values render as JSON in the cell. Every document also carries `_id`, `_index` and `_score`; `_id` is the primary key and all three are read-only. +Columns come from the index mapping. `object` fields flatten to dotted paths such as `address.city`. A `nested` field gets one JSON column for the whole array of objects, plus a dotted column per leaf (`identifiers.type`) whose cell is a JSON array with one entry per object, `null` where an object omits the field. Array and object values render as JSON in the cell. Every document also carries `_id`, `_index` and `_score`, and `_id` is the primary key. Type an `_id` into a new row to name the document; any other edit to the three stops the save. Column filters translate to `term`, `range`, `wildcard`, `terms` and `exists`. Two filter rows on leaves of the same array match two different objects unless you set both rows to the same element. The **Raw SQL** filter column is the exception: its text goes over as a `query_string`, so write Lucene there, `name:Widget` or `price:>10`, matched across all fields. Sorting a `text` field targets its `.keyword` subfield when the mapping has one. A nested leaf sorts and filters; its JSON parent column does neither. -Grid edits become REST calls keyed by `_id`: `POST /index/_update/{id}`, `PUT /index/_doc/{id}` and `DELETE /index/_doc/{id}`. Edit a nested array through its JSON parent column; an edit typed into one of its leaf columns is not sent. +Grid edits become REST calls keyed by `_id`: `POST /index/_update/{id}`, `PUT /index/_doc/{id}` and `DELETE /index/_doc/{id}`. Edit a nested array through its JSON parent column. A value typed into one of its leaf columns, in an existing document or a new one, cannot be written, so Save stops, names that column, and sends nothing until you undo the edit. Deleting an index from the sidebar sends `DELETE /`, the request the confirmation shows. diff --git a/docs/databases/etcd.mdx b/docs/databases/etcd.mdx index b0bd16f732..0b18156af0 100644 --- a/docs/databases/etcd.mdx +++ b/docs/databases/etcd.mdx @@ -49,9 +49,9 @@ Both schemes import. Neither switches TLS on by itself; set **TLS Mode** in the ## Browsing keys -The sidebar groups keys by their first path segment under the **Key Prefix Root**, with segment-less keys under **(root)**. The grid gives one row per key: **Key**, **Value**, **Version**, **ModRevision**, **CreateRevision** and **Lease**. Key is the primary key and the three revision columns are read-only. +The sidebar groups keys by their first path segment under the **Key Prefix Root**, with segment-less keys under **(root)**. The grid gives one row per key: **Key**, **Value**, **Version**, **ModRevision**, **CreateRevision** and **Lease**. Key is the primary key. The three revision columns belong to etcd: an edit to one stops the save and names the column. -Saving edits generates commands. Changing **Value** or **Lease** re-`put`s the key. Changing **Key** is a `put` at the new name and a `del` of the old one, so the old key is deleted rather than moved. A new row whose key does not start with `/` gets the **Key Prefix Root** prepended. +Saving edits generates commands. Changing **Value** or **Lease** re-`put`s the key. Setting **Value** to NULL stores an empty value, and setting **Lease** to NULL removes the lease. Changing **Key** is a `put` at the new name and a `del` of the old one, so the old key is deleted rather than moved. A new row whose key does not start with `/` gets the **Key Prefix Root** prepended. ## Command editor diff --git a/docs/databases/redis.mdx b/docs/databases/redis.mdx index bb72543825..f1990e9d96 100644 --- a/docs/databases/redis.mdx +++ b/docs/databases/redis.mdx @@ -106,13 +106,15 @@ The grid columns are **Key**, **Type**, **TTL**, **Length**, and **Value**. Valu A value that is not valid UTF-8, such as a gzip or MessagePack payload, opens in the hex editor instead of as text. -A **Type**, **TTL**, **Length** or **Value** cell is NULL when the server will not say for your user. ACL key patterns do not filter `SCAN`, so a user limited to `~app:*` still sees every key name, and editing **Value** is skipped for a key whose **Type** is NULL. +A **Type**, **TTL**, **Length** or **Value** cell is NULL when the server will not say for your user. ACL key patterns do not filter `SCAN`, so a user limited to `~app:*` still sees every key name, and a **Value** edit on a key whose **Type** is NULL stops the save. ### Editing -Editing a **Key** cell runs `RENAME`. Editing a **TTL** cell runs `EXPIRE`, or `PERSIST` when you set it to `-1`; in that column `-1` means no expiry and `-2` means the key is gone. Editing a **Value** cell runs `SET`, and only on a string, since a preview of a hash or list is not the whole structure. +Editing a **Key** cell runs `RENAME`. Editing a **TTL** cell runs `EXPIRE`, or `PERSIST` when you set it to `-1` or NULL; in that column `-1` means no expiry and `-2` means the key is gone. Editing a **Value** cell runs `SET`, and only on a string, since a preview of a hash or list is not the whole structure. -Change the other types with a command: `HSET myhash field1 "value1"` rewrites one field and leaves the rest alone. Adding a row does follow the type you pick, generating `HSET`, `RPUSH`, `SADD`, `ZADD`, or `SET`. +Save sends nothing, names the reason and keeps your edits when a row holds an edit these commands cannot make: a **Value** edit on any type but a string, a NULL **Value**, a **Type** edit, or a **TTL** that is not a whole number of seconds above 0. + +Change the other types with a command: `HSET myhash field1 "value1"` rewrites one field and leaves the rest alone. Adding a row does follow the type you pick, generating `HSET`, `RPUSH`, `SADD`, `ZADD`, or `SET`; any other type stops the save. ### Filtering diff --git a/docs/development/plugin-development.mdx b/docs/development/plugin-development.mdx index a1c5faee26..b6214fcf6b 100644 --- a/docs/development/plugin-development.mdx +++ b/docs/development/plugin-development.mdx @@ -59,13 +59,14 @@ Twelve requirements have no default. Everything else on the protocol does. | Schema | `fetchTables(schema:)`, `fetchColumns(table:schema:)`, `fetchIndexes(table:schema:)`, `fetchForeignKeys(table:schema:)`, `fetchTableDDL(table:schema:)`, `fetchViewDefinition(view:schema:)`, `fetchTableMetadata(table:schema:)` | | Databases | `fetchDatabases()`, `fetchDatabaseMetadata(_:)` | -Five defaults are worth a second look before you accept them: +These defaults are worth a second look before you accept them: - `ping()` runs `SELECT 1`, and the transaction methods run `BEGIN` / `COMMIT` / `ROLLBACK` through `execute(query:)`. An engine without those keywords overrides all four. - `sessionTransactionState()` answers `.unknown`, and a driver that cannot read its session leaves it there. The app treats `.idle` as permission to open a transaction of its own, so a guess hands a multi-statement batch or a grid save the power to commit work the user has not finished. Answer `.inTransaction`, `.abortedTransaction` or `.holdsSessionLocks` only from something the server said. - `fetchAllColumns(schema:)` and `fetchAllForeignKeys(schema:)` loop one round-trip per table. Any SQL driver should replace them with a single catalog query. - `quoteIdentifier`, `escapeStringLiteral`, `executeParameterized`, and `streamRows` assume generic SQL. -- A non-SQL database implements `buildBrowseQuery`, `buildFilteredQuery`, and `generateStatements` instead, which is what makes browsing and editing work without SQL. Implement the `schema:`-aware overloads if your database has schemas; the schema-less defaults throw the schema away. +- A non-SQL database implements `buildBrowseQuery`, `buildFilteredQuery`, and `generateRowWrites` instead, which is what makes browsing and editing work without SQL. Implement the `schema:`-aware overloads if your database has schemas; the schema-less defaults throw the schema away. +- Name the `rowIndex` of every change a `PluginRowWrite` writes, and throw `PluginRowWriteRefusal` with a reason for a change, or a value in one, that the driver cannot write. The app refuses a save in which a pending change has no statement, but it cannot see a value left out of one, so only the driver can refuse that. - A database whose tables are not a list of typed columns returns a `PluginCreateTableFormSpec` from `createTableFormSpec(schema:)` and turns the filled-in form into statements in `createTableStatements(for:schema:)`. **New Table…** then shows that form in place of the column grid. The default returns `nil`, which keeps the grid. [Testing a Custom Plugin](/development/testing-plugins) covers getting the built bundle into a running app and reading the failure if it does not load. diff --git a/docs/features/change-tracking.mdx b/docs/features/change-tracking.mdx index 711718a5e1..9b95cd24e9 100644 --- a/docs/features/change-tracking.mdx +++ b/docs/features/change-tracking.mdx @@ -84,9 +84,11 @@ A table without a primary key is matched on every original column value instead, On an engine with transactions, the statements run inside one, so a failure rolls the whole save back and the table is left as it was. Without them, the statements that ran before the failure stand. Either way a failed save reports **Save Failed** with the server's message and keeps the queue intact, so correct the value and save again. A save that succeeds clears the queue, clears undo, and reloads the grid at the same place. -Each statement is held to the number of rows it was written for. On a table with no primary key, two identical rows cannot be told apart, so a statement meant for one of them matches both; the save stops there and reports what happened instead of rewriting the other row. +Every queued change has to become a statement before anything is sent. When a change cannot, such as a new Redis row with no key, or it holds a value the driver cannot write, such as an edit to one leaf of an Elasticsearch nested array, the save stops, names what it could not write, and keeps the whole queue. Undo that change, or make it with a query, then save again. -Such a statement is held to the other end too. Finding fewer rows than it was written for means the row it meant to write is no longer there, so the save rolls back and keeps the edits rather than reporting a success that wrote nothing. Engines whose drivers report no real count, ClickHouse among them, are not held to it. +Each generated SQL statement is held to the number of rows it was written for. On a table with no primary key, two identical rows cannot be told apart, so a statement meant for one of them matches both; the save stops there and reports what happened instead of rewriting the other row. + +Such a statement is held to the other end too. Finding fewer rows than it was written for means the row it meant to write is no longer there, so the save rolls back and keeps the edits rather than reporting a success that wrote nothing. Engines whose drivers report no real count, ClickHouse among them, are not held to it, and neither is a statement a driver writes itself, as the MongoDB and Redis drivers do. There is no discard button. Undo the edits, or take the **Discard Unsaved Changes?** prompt that appears when you refresh, sort, filter, change page, or close the tab with edits pending.