Skip to content

Private tables should always be saved in private db. #310

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
mbrandonw merged 2 commits into from
Nov 27, 2025
Merged

Private tables should always be saved in private db. #310

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3b441f8
Private tables should always be saved in private db.
mbrandonw Nov 25, 2025
05580fb
snapshots
mbrandonw Nov 25, 2025
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
97 changes: 70 additions & 27 deletions Sources/SQLiteData/CloudKit/Internal/Triggers.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,20 +6,38 @@
extension PrimaryKeyedTable {
static func metadataTriggers(
parentForeignKey: ForeignKey?,
defaultZone: CKRecordZone
defaultZone: CKRecordZone,
privateTables: [any SynchronizableTable]
) -> [TemporaryTrigger<Self>] {
[
afterInsert(parentForeignKey: parentForeignKey, defaultZone: defaultZone),
afterUpdate(parentForeignKey: parentForeignKey, defaultZone: defaultZone),
afterDeleteFromUser(parentForeignKey: parentForeignKey, defaultZone: defaultZone),
afterInsert(
parentForeignKey: parentForeignKey,
defaultZone: defaultZone,
privateTables: privateTables
),
afterUpdate(
parentForeignKey: parentForeignKey,
defaultZone: defaultZone,
privateTables: privateTables
),
afterDeleteFromUser(
parentForeignKey: parentForeignKey,
defaultZone: defaultZone,
privateTables: privateTables
),
afterDeleteFromSyncEngine,
afterPrimaryKeyChange(parentForeignKey: parentForeignKey, defaultZone: defaultZone),
afterPrimaryKeyChange(
parentForeignKey: parentForeignKey,
defaultZone: defaultZone,
privateTables: privateTables
),
]
}

fileprivate static func afterPrimaryKeyChange(
parentForeignKey: ForeignKey?,
defaultZone: CKRecordZone
defaultZone: CKRecordZone,
privateTables: [any SynchronizableTable]
) -> TemporaryTrigger<Self> {
createTemporaryTrigger(
"\(String.sqliteDataCloudKitSchemaName)_after_primary_key_change_on_\(tableName)",
Expand All @@ -28,7 +46,8 @@
checkWritePermissions(
alias: new,
parentForeignKey: parentForeignKey,
defaultZone: defaultZone
defaultZone: defaultZone,
privateTables: privateTables
)
SyncMetadata
.where {
Expand All @@ -44,7 +63,8 @@

fileprivate static func afterInsert(
parentForeignKey: ForeignKey?,
defaultZone: CKRecordZone
defaultZone: CKRecordZone,
privateTables: [any SynchronizableTable]
) -> TemporaryTrigger<Self> {
createTemporaryTrigger(
"\(String.sqliteDataCloudKitSchemaName)_after_insert_on_\(tableName)",
Expand All @@ -53,20 +73,23 @@
checkWritePermissions(
alias: new,
parentForeignKey: parentForeignKey,
defaultZone: defaultZone
defaultZone: defaultZone,
privateTables: privateTables
)
SyncMetadata.insert(
new: new,
parentForeignKey: parentForeignKey,
defaultZone: defaultZone
defaultZone: defaultZone,
privateTables: privateTables
)
}
)
}

fileprivate static func afterUpdate(
parentForeignKey: ForeignKey?,
defaultZone: CKRecordZone
defaultZone: CKRecordZone,
privateTables: [any SynchronizableTable]
) -> TemporaryTrigger<Self> {
createTemporaryTrigger(
"\(String.sqliteDataCloudKitSchemaName)_after_update_on_\(tableName)",
Expand All @@ -75,25 +98,29 @@
checkWritePermissions(
alias: new,
parentForeignKey: parentForeignKey,
defaultZone: defaultZone
defaultZone: defaultZone,
privateTables: privateTables
)
SyncMetadata.insert(
new: new,
parentForeignKey: parentForeignKey,
defaultZone: defaultZone
defaultZone: defaultZone,
privateTables: privateTables
)
SyncMetadata.update(
new: new,
parentForeignKey: parentForeignKey,
defaultZone: defaultZone
defaultZone: defaultZone,
privateTables: privateTables
)
}
)
}

fileprivate static func afterDeleteFromUser(
parentForeignKey: ForeignKey?,
defaultZone: CKRecordZone
defaultZone: CKRecordZone,
privateTables: [any SynchronizableTable]
) -> TemporaryTrigger<
Self
> {
Expand All @@ -104,7 +131,8 @@
checkWritePermissions(
alias: old,
parentForeignKey: parentForeignKey,
defaultZone: defaultZone
defaultZone: defaultZone,
privateTables: privateTables
)
SyncMetadata
.where {
Expand Down Expand Up @@ -141,12 +169,14 @@
fileprivate static func insert<T: PrimaryKeyedTable, Name>(
new: StructuredQueriesCore.TableAlias<T, Name>.TableColumns,
parentForeignKey: ForeignKey?,
defaultZone: CKRecordZone
defaultZone: CKRecordZone,
privateTables: [any SynchronizableTable]
) -> some StructuredQueriesCore.Statement {
let (parentRecordPrimaryKey, parentRecordType, zoneName, ownerName) = parentFields(
alias: new,
parentForeignKey: parentForeignKey,
defaultZone: defaultZone
defaultZone: defaultZone,
privateTables: privateTables
)
let defaultZoneName = #sql(
"\(quote: defaultZone.zoneID.zoneName, delimiter: .text)",
Expand Down Expand Up @@ -181,12 +211,14 @@
fileprivate static func update<T: PrimaryKeyedTable, Name>(
new: StructuredQueriesCore.TableAlias<T, Name>.TableColumns,
parentForeignKey: ForeignKey?,
defaultZone: CKRecordZone
defaultZone: CKRecordZone,
privateTables: [any SynchronizableTable]
) -> some StructuredQueriesCore.Statement {
let (parentRecordPrimaryKey, parentRecordType, zoneName, ownerName) = parentFields(
alias: new,
parentForeignKey: parentForeignKey,
defaultZone: defaultZone
defaultZone: defaultZone,
privateTables: privateTables
)
return Self.where {
$0.recordPrimaryKey.eq(#sql("\(new.primaryKey)"))
Expand Down Expand Up @@ -322,15 +354,24 @@
private func parentFields<Base, Name>(
alias: StructuredQueriesCore.TableAlias<Base, Name>.TableColumns,
parentForeignKey: ForeignKey?,
defaultZone: CKRecordZone
defaultZone: CKRecordZone,
privateTables: [any SynchronizableTable]
) -> (
parentRecordPrimaryKey: SQLQueryExpression<String>?,
parentRecordType: SQLQueryExpression<String>?,
zoneName: SQLQueryExpression<String?>,
ownerName: SQLQueryExpression<String?>
) {
return
parentForeignKey
let zoneNameOverride: SQLQueryExpression<String?>
let ownerNameOverride: SQLQueryExpression<String?>
if privateTables.contains(where: { $0.base.tableName == Base.tableName }) {
zoneNameOverride = #sql("\(quote: defaultZone.zoneID.zoneName, delimiter: .text)")
ownerNameOverride = #sql("\(quote: defaultZone.zoneID.ownerName, delimiter: .text)")
} else {
zoneNameOverride = #sql("NULL")
ownerNameOverride = #sql("NULL")
}
Comment on lines +365 to +373
Copy link
Copy Markdown
Member Author

@mbrandonw mbrandonw Nov 25, 2025

Choose a reason for hiding this comment

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

This is the main fix. If we are getting parent information for a private table we can make sure the zone is the default zone. We could probably clean this up a bit, I mostly just focused on getting a passing test.

return parentForeignKey
.map { foreignKey in
let parentRecordPrimaryKey = #sql(
#"\#(type(of: alias).QueryValue.self).\#(quote: foreignKey.from)"#,
Expand All @@ -344,8 +385,8 @@
return (
parentRecordPrimaryKey,
parentRecordType,
#sql("coalesce(\($currentZoneName()), (\(parentMetadata.select(\.zoneName))))"),
#sql("coalesce(\($currentOwnerName()), (\(parentMetadata.select(\.ownerName))))")
#sql("coalesce(\(zoneNameOverride), \($currentZoneName()), (\(parentMetadata.select(\.zoneName))))"),
#sql("coalesce(\(ownerNameOverride), \($currentOwnerName()), (\(parentMetadata.select(\.ownerName))))")
)
}
?? (
Expand Down Expand Up @@ -373,12 +414,14 @@
private func checkWritePermissions<Base, Name>(
alias: StructuredQueriesCore.TableAlias<Base, Name>.TableColumns,
parentForeignKey: ForeignKey?,
defaultZone: CKRecordZone
defaultZone: CKRecordZone,
privateTables: [any SynchronizableTable]
) -> some StructuredQueriesCore.Statement<Never> {
let (parentRecordPrimaryKey, parentRecordType, _, _) = parentFields(
alias: alias,
parentForeignKey: parentForeignKey,
defaultZone: defaultZone
defaultZone: defaultZone,
privateTables: privateTables
)

return With {
Expand Down
24 changes: 20 additions & 4 deletions Sources/SQLiteData/CloudKit/SyncEngine.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -359,6 +359,7 @@
foreignKeysByTableName: foreignKeysByTableName,
tablesByName: tablesByName,
defaultZone: defaultZone,
privateTables: privateTables,
db: db
)
}
Expand Down Expand Up @@ -686,7 +687,8 @@
package func tearDownSyncEngine() throws {
try userDatabase.write { db in
for table in tables.reversed() {
try table.base.dropTriggers(defaultZone: defaultZone, db: db)
try table.base
.dropTriggers(defaultZone: defaultZone, privateTables: privateTables, db: db)
}
for trigger in SyncMetadata.callbackTriggers(for: self).reversed() {
try trigger.drop().execute(db)
Expand Down Expand Up @@ -886,22 +888,36 @@
foreignKeysByTableName: [String: [ForeignKey]],
tablesByName: [String: any SynchronizableTable],
defaultZone: CKRecordZone,
privateTables: [any SynchronizableTable],
db: Database
) throws {
let parentForeignKey =
foreignKeysByTableName[tableName]?.count == 1
? foreignKeysByTableName[tableName]?.first
: nil

for trigger in metadataTriggers(parentForeignKey: parentForeignKey, defaultZone: defaultZone)
for trigger in metadataTriggers(
parentForeignKey: parentForeignKey,
defaultZone: defaultZone,
privateTables: privateTables
)
{
try trigger.execute(db)
}
}

@available(iOS 17, macOS 14, tvOS 17, watchOS 10, *)
fileprivate static func dropTriggers(defaultZone: CKRecordZone, db: Database) throws {
for trigger in metadataTriggers(parentForeignKey: nil, defaultZone: defaultZone).reversed() {
fileprivate static func dropTriggers(
defaultZone: CKRecordZone,
privateTables: [any SynchronizableTable],
db: Database
) throws {
for trigger in metadataTriggers(
parentForeignKey: nil,
defaultZone: defaultZone,
privateTables: privateTables
)
.reversed() {
try trigger.drop().execute(db)
}
}
Expand Down
87 changes: 87 additions & 0 deletions Tests/SQLiteDataTests/CloudKitTests/SharingTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,93 @@
}
}

@available(iOS 17, macOS 14, tvOS 17, watchOS 10, *)
@Test func privateTablesStayInPrivateDatabase() async throws {
Copy link
Copy Markdown
Member Author

@mbrandonw mbrandonw Nov 25, 2025

Choose a reason for hiding this comment

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

This test failed before the fix because when processing record changes in the private database a failure would be emitted since there were no changes to process.

let externalZone = CKRecordZone(
zoneID: CKRecordZone.ID(
zoneName: "external.zone",
ownerName: "external.owner"
)
)
try await syncEngine.modifyRecordZones(scope: .shared, saving: [externalZone]).notify()

let remindersListRecord = CKRecord(
recordType: RemindersList.tableName,
recordID: RemindersList.recordID(for: 1, zoneID: externalZone.zoneID)
)
remindersListRecord.setValue(1, forKey: "id", at: now)
remindersListRecord.setValue("Personal", forKey: "title", at: now)
let share = CKShare(
rootRecord: remindersListRecord,
shareID: CKRecord.ID(
recordName: "share-\(remindersListRecord.recordID.recordName)",
zoneID: remindersListRecord.recordID.zoneID
)
)
_ = try syncEngine.modifyRecords(scope: .shared, saving: [share, remindersListRecord])
let freshShare = try syncEngine.shared.database.record(for: share.recordID) as! CKShare
let freshRemindersListRecord = try syncEngine.shared.database.record(
for: remindersListRecord.recordID
)

try await syncEngine
.acceptShare(
metadata: ShareMetadata(
containerIdentifier: container.containerIdentifier!,
hierarchicalRootRecordID: freshRemindersListRecord.recordID,
rootRecord: freshRemindersListRecord,
share: freshShare
)
)

try await userDatabase.userWrite { db in
try RemindersListPrivate.insert {
RemindersListPrivate(remindersListID: 1, position: 42)
}
.execute(db)
}
try await syncEngine.processPendingRecordZoneChanges(scope: .private)

assertInlineSnapshot(of: container, as: .customDump) {
"""
MockCloudContainer(
privateCloudDatabase: MockCloudDatabase(
databaseScope: .private,
storage: [
[0]: CKRecord(
recordID: CKRecord.ID(1:remindersListPrivates/zone/__defaultOwner__),
Copy link
Copy Markdown
Member Author

@mbrandonw mbrandonw Nov 25, 2025

Choose a reason for hiding this comment

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

The fact that this record is in the private database shows the fix has worked.

recordType: "remindersListPrivates",
parent: nil,
share: nil,
position: 42,
remindersListID: 1
)
]
),
sharedCloudDatabase: MockCloudDatabase(
databaseScope: .shared,
storage: [
[0]: CKRecord(
recordID: CKRecord.ID(share-1:remindersLists/external.zone/external.owner),
recordType: "cloudkit.share",
parent: nil,
share: nil
),
[1]: CKRecord(
recordID: CKRecord.ID(1:remindersLists/external.zone/external.owner),
recordType: "remindersLists",
parent: nil,
share: CKReference(recordID: CKRecord.ID(share-1:remindersLists/external.zone/external.owner)),
id: 1,
title: "Personal"
)
]
)
)
"""
}
}

@available(iOS 17, macOS 14, tvOS 17, watchOS 10, *)
@Test func shareRecordBeforeSync() async throws {
let error = await #expect(throws: (any Error).self) {
Expand Down
Loading