diff --git a/CHANGELOG.md b/CHANGELOG.md index eec8d803cc..64dc0307bf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -115,6 +115,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Oracle view DDL losing the view's column names, and cut off after 4000 bytes without saying so. - Trino materialized views left out of SQL exports. - Return in Switch Connection reopening the current connection after typing another connection's name. +- Clearing a date, number or boolean field on iPhone and iPad saving an empty string, stored as `0000-00-00 00:00:00` on MySQL, instead of NULL. ### Security diff --git a/Packages/TableProCore/Sources/TableProModels/ColumnInfo+EmptyValue.swift b/Packages/TableProCore/Sources/TableProModels/ColumnInfo+EmptyValue.swift new file mode 100644 index 0000000000..1972a01e21 --- /dev/null +++ b/Packages/TableProCore/Sources/TableProModels/ColumnInfo+EmptyValue.swift @@ -0,0 +1,53 @@ +import Foundation + +public extension ColumnInfo { + /// Whether `''` is a value of this column's type, read from the declared type name alone. + /// + /// A date, number, boolean, JSON, UUID or geometry has no empty value: sent `''`, MySQL without + /// strict mode stores its zero value (`0000-00-00 00:00:00`, `0`), SQL Server stores + /// `1900-01-01` or `0`, and a strict MySQL or PostgreSQL refuses the write. Every other name, + /// including one this does not know, such as PostgreSQL's `USER-DEFINED` for a `citext` or a + /// domain, keeps `''`. + var holdsEmptyString: Bool { + Self.holdsEmptyString(typeName: typeName) + } + + static func holdsEmptyString(typeName: String) -> Bool { + let upper = unwrapped(typeName.uppercased()) + if upper.hasSuffix("[]") || upper.hasPrefix("ARRAY") { return false } + let base = upper.prefix { $0 != "(" }.trimmingCharacters(in: .whitespaces) + if base == "ENUM" { return ["('',", ",'',", ",'')", "('')"].contains { upper.contains($0) } } + if base.hasPrefix("BIT VARYING") { return true } + return !typedStems.contains(stem(of: base)) + } + + /// Type names by their first word with any width digits dropped, so `INT4`, `Int64`, + /// `DATETIME2`, `DateTime64` and `FLOAT8` read as their family. + private static let typedStems: Set = [ + "DATE", "TIME", "TIMETZ", "TIMESTAMP", "TIMESTAMPTZ", "TIMESTAMP_S", "TIMESTAMP_MS", "TIMESTAMP_NS", + "TIMESTAMP_TZ", "DATETIME", "DATETIMEOFFSET", "SMALLDATETIME", "YEAR", "INTERVAL", + "INT", "UINT", "INTEGER", "UINTEGER", "TINYINT", "UTINYINT", "SMALLINT", "USMALLINT", "MEDIUMINT", + "BIGINT", "UBIGINT", "HUGEINT", "UHUGEINT", "SERIAL", "SMALLSERIAL", "BIGSERIAL", + "DECIMAL", "DEC", "NUMERIC", "NUMBER", "FLOAT", "DOUBLE", "REAL", "MONEY", "SMALLMONEY", + "BINARY_FLOAT", "BINARY_DOUBLE", "BIT", "BOOL", "BOOLEAN", "JSON", "JSONB", + "UUID", "UNIQUEIDENTIFIER", "INET", "CIDR", "MACADDR", + "GEOMETRY", "GEOGRAPHY", "POINT", "LINESTRING", "POLYGON", "MULTIPOINT", "MULTILINESTRING", + "MULTIPOLYGON", "GEOMETRYCOLLECTION" + ] + + private static func stem(of base: String) -> String { + var word = String(base.prefix { $0 != " " }) + while word.count > 1, word.last?.isNumber == true { + word.removeLast() + } + return word + } + + private static func unwrapped(_ typeName: String) -> String { + var name = typeName.trimmingCharacters(in: .whitespaces) + while let wrapper = ["NULLABLE(", "LOWCARDINALITY("].first(where: { name.hasPrefix($0) }), name.hasSuffix(")") { + name = String(name.dropFirst(wrapper.count).dropLast()) + } + return name + } +} diff --git a/Packages/TableProCore/Tests/TableProModelsTests/ColumnInfoEmptyValueTests.swift b/Packages/TableProCore/Tests/TableProModelsTests/ColumnInfoEmptyValueTests.swift new file mode 100644 index 0000000000..3389834cee --- /dev/null +++ b/Packages/TableProCore/Tests/TableProModelsTests/ColumnInfoEmptyValueTests.swift @@ -0,0 +1,39 @@ +import Foundation +import Testing + +@testable import TableProModels + +@Suite("ColumnInfo empty value") +struct ColumnInfoEmptyValueTests { + @Test( + "Dates, numbers, booleans and other typed values have no empty value", + arguments: [ + "DATETIME", "datetime(6)", "TIMESTAMP", "timestamptz", "DATE", "TIME", "YEAR", + "INT", "int4", "BIGINT UNSIGNED", "DECIMAL(10,2)", "numeric", "DOUBLE", "MONEY", + "TINYINT(1)", "BIT", "bool", "JSON", "jsonb", "uuid", "uniqueidentifier", "inet", + "geometry", "interval", "integer[]", "Nullable(UUID)", "enum('a','b')", "enum('it''s','b')", "BINARY_DOUBLE", + "timestamp without time zone", "double precision", "Int64", "DateTime64(3)", "FLOAT8" + ] + ) + func typedColumnsHaveNoEmptyValue(_ typeName: String) { + #expect(ColumnInfo.holdsEmptyString(typeName: typeName) == false) + } + + @Test( + "Character, binary and SET columns, and types this does not know, hold the empty string", + arguments: [ + "VARCHAR(64)", "character varying", "TEXT", "LONGTEXT", "NVARCHAR(MAX)", "bpchar", + "citext", "CLOB", "BLOB", "bytea", "VARBINARY(16)", "set('x','y')", "enum('','a')", "enum('a','','b')", "enum('a','')", + "Nullable(String)", "LowCardinality(String)", "", "USER-DEFINED", "mood", "bit varying" + ] + ) + func characterColumnsHoldEmptyValue(_ typeName: String) { + #expect(ColumnInfo.holdsEmptyString(typeName: typeName)) + } + + @Test("The property reads the column's own type name") + func propertyReadsTypeName() { + #expect(ColumnInfo(name: "deleted_at", typeName: "DATETIME", ordinalPosition: 0).holdsEmptyString == false) + #expect(ColumnInfo(name: "name", typeName: "VARCHAR(64)", ordinalPosition: 1).holdsEmptyString) + } +} diff --git a/TableProMobile/TableProMobile/ViewModels/RowDetailViewModel.swift b/TableProMobile/TableProMobile/ViewModels/RowDetailViewModel.swift index 01e6f107d3..fd5260ef88 100644 --- a/TableProMobile/TableProMobile/ViewModels/RowDetailViewModel.swift +++ b/TableProMobile/TableProMobile/ViewModels/RowDetailViewModel.swift @@ -197,12 +197,38 @@ final class RowDetailViewModel { return false } let oldValue = index < original.count ? original[index] : nil - return oldValue != editedValues[index] + return oldValue != writtenValue(at: index, stored: oldValue) } } private var editedChanges: [(column: String, value: String?)] { - editedColumnIndices.map { (column: columns[$0].name, value: editedValues[$0]) } + let original = currentRow + return editedColumnIndices.map { index in + (column: columns[index].name, value: writtenValue(at: index, stored: index < original.count ? original[index] : nil)) + } + } + + /// What Save writes for a field. An emptied date, number or boolean is NULL, since `''` is no + /// value of those types and MySQL without strict mode stores it as the zero date. In a NOT NULL + /// column, which holds neither, the field keeps the stored value and Save writes nothing for it. + /// A field left as it was is never read again: SQLite can hold `''` in a DATETIME column. + func writtenValue(at index: Int) -> String? { + let original = currentRow + return writtenValue(at: index, stored: index < original.count ? original[index] : nil) + } + + /// Takes the stored value from the caller's snapshot of the row, which `currentRow` rebuilds on + /// every read, so a pass over every field stays linear. + private func writtenValue(at index: Int, stored: String?) -> String? { + guard index < editedValues.count else { return nil } + let value = editedValues[index] + guard value == "", stored != "", !holdsEmptyString(at: index) else { return value } + return isNullable(at: index) ? nil : stored + } + + private func holdsEmptyString(at index: Int) -> Bool { + guard index >= 0, index < columns.count else { return true } + return (columnDetail(for: columns[index].name) ?? columns[index]).holdsEmptyString } // MARK: - Save @@ -269,8 +295,10 @@ final class RowDetailViewModel { private func execute(sql: String, session: ConnectionSession) async -> Bool { isSaving = true defer { isSaving = false } + let original = currentRow let savedCells = Dictionary(uniqueKeysWithValues: editedColumnIndices.map { index in - (index, editedValues[index].map { Cell.text($0) } ?? .null) + let stored = index < original.count ? original[index] : nil + return (index, writtenValue(at: index, stored: stored).map { Cell.text($0) } ?? .null) }) do { diff --git a/TableProMobile/TableProMobileTests/RowDetailViewModelTests.swift b/TableProMobile/TableProMobileTests/RowDetailViewModelTests.swift index 87edf16e01..f85b925570 100644 --- a/TableProMobile/TableProMobileTests/RowDetailViewModelTests.swift +++ b/TableProMobile/TableProMobileTests/RowDetailViewModelTests.swift @@ -389,6 +389,82 @@ struct RowDetailViewModelTests { #expect(vm.hasUnsavedEdits == false) } + private func makeDatedColumns() -> [ColumnInfo] { + makeColumns() + [ + ColumnInfo(name: "deleted_at", typeName: "DATETIME", ordinalPosition: 2), + ColumnInfo(name: "created_at", typeName: "TIMESTAMP", isNullable: false, ordinalPosition: 3) + ] + } + + private func makeDatedRows() -> [Row] { + [Row(cells: [.text("1"), .text("Alice"), .text("2024-01-01 10:00:00"), .text("2023-12-01 09:00:00")])] + } + + @Test("Clearing a date field saves NULL, not an empty string") + func clearedDateSavesNull() async { + let driver = MockDatabaseDriver() + driver.scriptedExecuteResults = [ + .success(QueryResult(columns: [], rows: [], rowsAffected: 1, executionTime: 0)) + ] + let vm = RowDetailViewModel( + columns: makeDatedColumns(), rows: makeDatedRows(), initialIndex: 0, + table: TableInfo(name: "users"), session: makeSession(driver: driver), + columnDetails: makeDatedColumns() + ) + vm.startEditing() + vm.setEditedValue("", at: 2) + + #expect(vm.writtenValue(at: 2) == nil) + #expect(await vm.saveChanges()) + let query = driver.executedQueries.first ?? "" + #expect(query.contains("`deleted_at` = NULL") || query.contains("\"deleted_at\" = NULL")) + #expect(!query.contains("''")) + } + + @Test("Clearing a text field still saves an empty string") + func clearedTextSavesEmpty() { + let vm = RowDetailViewModel(columns: makeDatedColumns(), rows: makeDatedRows(), initialIndex: 0) + vm.startEditing() + vm.setEditedValue("", at: 1) + + #expect(vm.writtenValue(at: 1) == "") + #expect(vm.hasUnsavedEdits) + } + + @Test("Clearing a NOT NULL timestamp is no edit, since that column holds neither value") + func clearedNotNullTimestampIsNoEdit() { + let vm = RowDetailViewModel( + columns: makeDatedColumns(), rows: makeDatedRows(), initialIndex: 0, + columnDetails: makeDatedColumns() + ) + vm.startEditing() + vm.setEditedValue("", at: 3) + + #expect(vm.writtenValue(at: 3) == "2023-12-01 09:00:00") + #expect(vm.hasUnsavedEdits == false) + } + + @Test("A stored empty string in a date column is left alone when another field is saved") + func storedEmptyDateIsUntouched() { + let rows = [Row(cells: [.text("1"), .text("Alice"), .text(""), .text("2023-12-01 09:00:00")])] + let vm = RowDetailViewModel(columns: makeDatedColumns(), rows: rows, initialIndex: 0) + vm.startEditing() + + #expect(vm.writtenValue(at: 2) == "") + #expect(vm.hasUnsavedEdits == false) + } + + @Test("Turning NULL off on a NULL date without typing writes nothing") + func nullToggledOffOnDateIsNoEdit() { + let rows = [Row(cells: [.text("1"), .text("Alice"), .null, .text("2023-12-01 09:00:00")])] + let vm = RowDetailViewModel(columns: makeDatedColumns(), rows: rows, initialIndex: 0) + vm.startEditing() + vm.toggleNull(at: 2) + + #expect(vm.editedValues[2] == "") + #expect(vm.hasUnsavedEdits == false) + } + @Test("An edited primary key is never an unsaved edit") func primaryKeyEditIsIgnored() { let vm = RowDetailViewModel(