From 27add17d4e372c61bdacaf01c2285b6fc7aeaef4 Mon Sep 17 00:00:00 2001 From: Brandon Williams Date: Tue, 2 Sep 2025 14:16:48 -0500 Subject: [PATCH] Validate foreign keys point to synchronized tables. --- .../SharingGRDBCore/CloudKit/SyncEngine.swift | 154 +++++++++--------- .../SyncEngineValidationTests.swift | 66 ++++++++ 2 files changed, 147 insertions(+), 73 deletions(-) diff --git a/Sources/SharingGRDBCore/CloudKit/SyncEngine.swift b/Sources/SharingGRDBCore/CloudKit/SyncEngine.swift index feac3959..296ccabf 100644 --- a/Sources/SharingGRDBCore/CloudKit/SyncEngine.swift +++ b/Sources/SharingGRDBCore/CloudKit/SyncEngine.swift @@ -177,11 +177,6 @@ } } ) - try validateSchema( - tables: tables, - foreignKeysByTableName: foreignKeysByTableName, - userDatabase: userDatabase - ) self.container = container self.defaultZone = defaultZone self.defaultSyncEngines = defaultSyncEngines @@ -206,6 +201,7 @@ tables: tables, tablesByName: tablesByName ) + try validateSchema() } @TaskLocal package static var _isSynchronizingChanges = false @@ -518,7 +514,7 @@ changes.append(.deleteRecord(share.recordID)) } guard isRunning else { - Task { [changes] in + Task { [changes] in await withErrorReporting(.sqliteDataCloudKitFailure) { try await userDatabase.write { db in try PendingRecordZoneChange @@ -1273,7 +1269,9 @@ else { continue } func open(_: T.Type) async throws { do { - let serverRecord = try await container.sharedCloudDatabase.record(for: failedRecord.recordID) + let serverRecord = try await container.sharedCloudDatabase.record( + for: failedRecord.recordID + ) upsertFromServerRecord(serverRecord, force: true) } catch let error as CKError where error.code == .unknownItem { try await userDatabase.write { db in @@ -1818,6 +1816,7 @@ struct SchemaError: LocalizedError { enum Reason { case inMemoryDatabase + case invalidForeignKey(ForeignKey) case invalidForeignKeyAction(ForeignKey) case invalidTableName(String) case metadatabaseMismatch(attachedPath: String, syncEngineConfiguredPath: String) @@ -1832,6 +1831,78 @@ "Could not synchronize data with iCloud." } } + + fileprivate func validateSchema() throws { + let tableNames = Set(tables.map { $0.tableName }) + for tableName in tableNames { + if tableName.contains(":") { + throw SyncEngine.SchemaError( + reason: .invalidTableName(tableName), + debugDescription: "Table name contains invalid character ':'" + ) + } + } + try userDatabase.read { db in + for (tableName, foreignKeys) in foreignKeysByTableName { + + let invalidForeignKey = foreignKeys.first(where: { tablesByName[$0.table] == nil }) + if let invalidForeignKey { + throw SyncEngine.SchemaError( + reason: .invalidForeignKey(invalidForeignKey), + debugDescription: """ + Foreign key \(tableName.debugDescription).\(invalidForeignKey.from.debugDescription) \ + references table \(invalidForeignKey.table.debugDescription) that is not \ + synchronized. Update 'SyncEngine.init' to synchronize \ + \(invalidForeignKey.table.debugDescription). + """ + ) + } + + if foreignKeys.count == 1, + let foreignKey = foreignKeys.first, + [.restrict, .noAction].contains(foreignKey.onDelete) + { + throw SyncEngine.SchemaError( + reason: .invalidForeignKeyAction(foreignKey), + debugDescription: """ + Foreign key \(tableName.debugDescription).\(foreignKey.from.debugDescription) action \ + not supported. Must be 'CASCADE', 'SET DEFAULT' or 'SET NULL'. + """ + ) + } + } + + for table in tables { + // // TODO: write tests for this + // let columnsWithUniqueConstraints = + // try SQLQueryExpression( + // """ + // SELECT "name" FROM pragma_index_list(\(quote: table.tableName, delimiter: .text)) + // WHERE "unique" = 1 AND "origin" <> 'pk' + // """, + // as: String.self + // ) + // .fetchAll(db) + // if !columnsWithUniqueConstraints.isEmpty { + // throw UniqueConstraintDisallowed(table: table, columns: columnsWithUniqueConstraints) + // } + + // // TODO: write tests for this + // let nonNullColumnsWithNoDefault = + // try SQLQueryExpression( + // """ + // SELECT "name" FROM pragma_table_info(\(quote: table.tableName, delimiter: .text)) + // WHERE "notnull" = 1 AND "dflt_value" IS NULL + // """, + // as: String.self + // ) + // .fetchAll(db) + // if !nonNullColumnsWithNoDefault.isEmpty { + // throw NonNullColumnMustHaveDefault(table: table, columns: nonNullColumnsWithNoDefault) + // } + } + } + } } // TODO: Private, opaque error @@ -1856,69 +1927,6 @@ // } // } - @available(iOS 17, macOS 14, tvOS 17, watchOS 10, *) - private func validateSchema( - tables: [any PrimaryKeyedTable.Type], - foreignKeysByTableName: [String: [ForeignKey]], - userDatabase: UserDatabase - ) throws { - let tableNames = Set(tables.map { $0.tableName }) - for tableName in tableNames { - if tableName.contains(":") { - throw SyncEngine.SchemaError( - reason: .invalidTableName(tableName), - debugDescription: "Table name contains invalid character ':'" - ) - } - } - try userDatabase.read { db in - for (tableName, foreignKeys) in foreignKeysByTableName { - if foreignKeys.count == 1, - let foreignKey = foreignKeys.first, - [.restrict, .noAction].contains(foreignKey.onDelete) - { - throw SyncEngine.SchemaError( - reason: .invalidForeignKeyAction(foreignKey), - debugDescription: """ - Foreign key \(tableName.debugDescription).\(foreignKey.from.debugDescription) action \ - not supported. Must be 'CASCADE', 'SET DEFAULT' or 'SET NULL'. - """ - ) - } - } - - for table in tables { - // // TODO: write tests for this - // let columnsWithUniqueConstraints = - // try SQLQueryExpression( - // """ - // SELECT "name" FROM pragma_index_list(\(quote: table.tableName, delimiter: .text)) - // WHERE "unique" = 1 AND "origin" <> 'pk' - // """, - // as: String.self - // ) - // .fetchAll(db) - // if !columnsWithUniqueConstraints.isEmpty { - // throw UniqueConstraintDisallowed(table: table, columns: columnsWithUniqueConstraints) - // } - - // // TODO: write tests for this - // let nonNullColumnsWithNoDefault = - // try SQLQueryExpression( - // """ - // SELECT "name" FROM pragma_table_info(\(quote: table.tableName, delimiter: .text)) - // WHERE "notnull" = 1 AND "dflt_value" IS NULL - // """, - // as: String.self - // ) - // .fetchAll(db) - // if !nonNullColumnsWithNoDefault.isEmpty { - // throw NonNullColumnMustHaveDefault(table: table, columns: nonNullColumnsWithNoDefault) - // } - } - } - } - private struct HashablePrimaryKeyedTableType: Hashable { let type: any PrimaryKeyedTable.Type init(_ type: any PrimaryKeyedTable.Type) { @@ -2027,9 +2035,9 @@ columnNames .filter { columnName in columnName != T.columns.primaryKey.name } .map { - """ - \(quote: $0) = "excluded".\(quote: $0) - """ + """ + \(quote: $0) = "excluded".\(quote: $0) + """ } .joined(separator: ", ") ) diff --git a/Tests/SharingGRDBTests/CloudKitTests/SyncEngineValidationTests.swift b/Tests/SharingGRDBTests/CloudKitTests/SyncEngineValidationTests.swift index d0905fdb..2c741ae0 100644 --- a/Tests/SharingGRDBTests/CloudKitTests/SyncEngineValidationTests.swift +++ b/Tests/SharingGRDBTests/CloudKitTests/SyncEngineValidationTests.swift @@ -167,6 +167,72 @@ extension BaseCloudKitTests { } } + @Table struct Child: Identifiable { + let id: Int + var parentID: Parent.ID + } + @Table struct Parent: Identifiable { + let id: Int + } + @available(iOS 17, macOS 14, tvOS 17, watchOS 10, *) + @Test func foreignKeyPointsToOtherSynchronizedTable() async throws { + let error = try #require( + await #expect(throws: (any Error).self) { + let database = try DatabaseQueue() + try await database.write { db in + try #sql( + """ + CREATE TABLE "parents" ( + "id" INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL + ) STRICT + """ + ) + .execute(db) + try #sql( + """ + CREATE TABLE "childs" ( + "id" INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL, + "parentID" INTEGER REFERENCES "parents"("id") ON DELETE CASCADE + ) STRICT + """ + ) + .execute(db) + } + _ = try await SyncEngine( + container: MockCloudContainer( + containerIdentifier: "deadbeef", + privateCloudDatabase: MockCloudDatabase(databaseScope: .private), + sharedCloudDatabase: MockCloudDatabase(databaseScope: .shared) + ), + userDatabase: UserDatabase(database: database), + tables: [Child.self] + ) + } + ) + assertInlineSnapshot(of: error.localizedDescription, as: .customDump) { + """ + "Could not synchronize data with iCloud." + """ + } + assertInlineSnapshot(of: error, as: .customDump) { + """ + SyncEngine.SchemaError( + reason: .invalidForeignKey( + ForeignKey( + table: "parents", + from: "parentID", + to: "id", + onUpdate: .noAction, + onDelete: .cascade, + notnull: false + ) + ), + debugDescription: #"Foreign key "childs"."parentID" references table "parents" that is not synchronized. Update 'SyncEngine.init' to synchronize "parents". "# + ) + """ + } + } + @available(iOS 17, macOS 14, tvOS 17, watchOS 10, *) @Test func doNotValidateTriggersOnNonSyncedTables() async throws { let database = try DatabaseQueue(