Skip to content

Validate foreign keys point to synchronized tables. #152

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
stephencelis merged 1 commit into from
Sep 2, 2025
Merged

Validate foreign keys point to synchronized tables. #152

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
154 changes: 81 additions & 73 deletions Sources/SharingGRDBCore/CloudKit/SyncEngine.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,11 +177,6 @@
}
}
)
try validateSchema(
tables: tables,
foreignKeysByTableName: foreignKeysByTableName,
userDatabase: userDatabase
)
self.container = container
self.defaultZone = defaultZone
self.defaultSyncEngines = defaultSyncEngines
Expand All @@ -206,6 +201,7 @@
tables: tables,
tablesByName: tablesByName
)
try validateSchema()
Copy link
Copy Markdown
Member Author

@mbrandonw mbrandonw Sep 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I moved this to be an instance method instead of a private free function so that we get access to all the data in the sync engine.

}

@TaskLocal package static var _isSynchronizingChanges = false
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -1273,7 +1269,9 @@
else { continue }
func open<T: PrimaryKeyedTable>(_: 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
Expand Down Expand Up @@ -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)
Expand All @@ -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).
"""
)
}
Comment on lines +1848 to +1859
Copy link
Copy Markdown
Member Author

@mbrandonw mbrandonw Sep 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the new validation code. The rest of the diff is me moving validateSchema to be an instance method.


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
Expand All @@ -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) {
Expand Down Expand Up @@ -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: ", ")
)
Expand Down
66 changes: 66 additions & 0 deletions Tests/SharingGRDBTests/CloudKitTests/SyncEngineValidationTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
Loading