From a4e1fd83008a38fb158e5ac0ffa2040d9a4cbdf1 Mon Sep 17 00:00:00 2001 From: Brandon Williams Date: Wed, 10 Sep 2025 16:25:31 -0500 Subject: [PATCH 1/5] Throw error when moving records between zones. --- Package.resolved | 6 +- .../CloudKit/Internal/CloudKitFunctions.swift | 3 +- .../CloudKit/Internal/Triggers.swift | 79 ++++++++- Sources/SQLiteData/CloudKit/SyncEngine.swift | 72 +++++++- .../SQLiteData/CloudKit/SyncMetadata.swift | 10 +- .../NextRecordZoneChangeBatchTests.swift | 44 ++--- .../CloudKitTests/SharingTests.swift | 163 ++++++++++++++++++ 7 files changed, 342 insertions(+), 35 deletions(-) diff --git a/Package.resolved b/Package.resolved index bc05ab9a..a5072755 100644 --- a/Package.resolved +++ b/Package.resolved @@ -1,5 +1,5 @@ { - "originHash" : "5f7df83a450ddad2662622f41356bb1fb40d7774f6d1e85438ffac479b93045a", + "originHash" : "32f70dab99ea92a018a46453e9ed1e8bbf1e5d17b2993ba8abc63cae49896bbc", "pins" : [ { "identity" : "combine-schedulers", @@ -123,8 +123,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/pointfreeco/swift-structured-queries", "state" : { - "revision" : "1b653aba57486afef66d8aafdbe83246249118fd", - "version" : "0.19.0" + "revision" : "f576582c138311e442c564bd7a893e950765e46a", + "version" : "0.19.1" } }, { diff --git a/Sources/SQLiteData/CloudKit/Internal/CloudKitFunctions.swift b/Sources/SQLiteData/CloudKit/Internal/CloudKitFunctions.swift index fb1fa47f..d7d6670a 100644 --- a/Sources/SQLiteData/CloudKit/Internal/CloudKitFunctions.swift +++ b/Sources/SQLiteData/CloudKit/Internal/CloudKitFunctions.swift @@ -10,7 +10,8 @@ @DatabaseFunction( "sqlitedata_icloud_hasPermission", - as: ((CKShare?.SystemFieldsRepresentation) -> Bool).self + as: ((CKShare?.SystemFieldsRepresentation) -> Bool).self, + isDeterministic: true ) func hasPermission(_ share: CKShare?) -> Bool { guard let share else { return true } diff --git a/Sources/SQLiteData/CloudKit/Internal/Triggers.swift b/Sources/SQLiteData/CloudKit/Internal/Triggers.swift index c2fbe727..c451ed2c 100644 --- a/Sources/SQLiteData/CloudKit/Internal/Triggers.swift +++ b/Sources/SQLiteData/CloudKit/Internal/Triggers.swift @@ -145,8 +145,10 @@ Values( syncEngine.$didUpdate( recordName: new.recordName, - record: new.lastKnownServerRecord - ?? rootServerRecord(recordName: new.recordName) + lastKnownServerRecord: new.lastKnownServerRecord + ?? rootServerRecord(recordName: new.recordName), + newParentLastKnownServerRecord: #bind(nil), + childrenLastKnownServerRecordNames: #bind([]) ) ) } when: { _ in @@ -165,8 +167,15 @@ Values( syncEngine.$didUpdate( recordName: new.recordName, - record: new.lastKnownServerRecord - ?? rootServerRecord(recordName: new.recordName) + lastKnownServerRecord: new.lastKnownServerRecord + ?? rootServerRecord(recordName: new.recordName), + newParentLastKnownServerRecord: parentLastKnownServerRecordIfShared( + recordName: new.recordName, + parentRecordName: new.parentRecordName + ), + childrenLastKnownServerRecordNames: childrenLastKnownServerRecordNamesIfShared( + recordName: new.recordName + ) ) ) } when: { old, new in @@ -281,10 +290,72 @@ } } + @available(iOS 17, macOS 14, tvOS 17, watchOS 10, *) + private func hasSharedAncestor( + recordName: some QueryExpression + ) -> some QueryExpression { + With { + SyncMetadata + .where { $0.recordName.eq(recordName) } + .select { AncestorMetadata.Columns($0) } + .union( + all: true, + SyncMetadata + .select { AncestorMetadata.Columns($0) } + .join(AncestorMetadata.all) { $0.recordName.is($1.parentRecordName) } + ) + } query: { + AncestorMetadata + .select(\.isShared) + .where { $0.parentRecordName.is(nil) && $0.isShared } + } + } + + @available(iOS 17, macOS 14, tvOS 17, watchOS 10, *) + private func parentLastKnownServerRecordIfShared( + recordName: some QueryExpression, + parentRecordName: some QueryExpression + ) -> some QueryExpression { + SyncMetadata + .select(\.lastKnownServerRecord) + .where { + $0.recordName.is(parentRecordName) + && hasSharedAncestor(recordName: recordName) + } + } + +@available(iOS 17, macOS 14, tvOS 17, watchOS 10, *) + private func childrenLastKnownServerRecordNamesIfShared( + recordName: some QueryExpression + ) -> some QueryExpression<[String].JSONRepresentation> { + With { + SyncMetadata + .where { $0.recordName.eq(recordName) } + .select { ChildMetadata.Columns(recordName: $0.recordName, parentRecordName: #bind(nil)) } + .union(all: true, + SyncMetadata + .select { + ChildMetadata.Columns( + recordName: $0.recordName, + parentRecordName: $0.parentRecordName + ) + } + .join(ChildMetadata.all) { $0.parentRecordName.eq($1.recordName) } + ) + } query: { + ChildMetadata + .where { $0.recordName.neq(recordName) } + .select { + $0.recordName.jsonGroupArray() + } + } + } + @available(iOS 17, macOS 14, tvOS 17, watchOS 10, *) extension AncestorMetadata.Columns { init(_ metadata: SyncMetadata.TableColumns) { self.init( + isShared: metadata.isShared, recordName: metadata.recordName, parentRecordName: metadata.parentRecordName, lastKnownServerRecord: metadata.lastKnownServerRecord diff --git a/Sources/SQLiteData/CloudKit/SyncEngine.swift b/Sources/SQLiteData/CloudKit/SyncEngine.swift index 990e66f0..bf23c93e 100644 --- a/Sources/SQLiteData/CloudKit/SyncEngine.swift +++ b/Sources/SQLiteData/CloudKit/SyncEngine.swift @@ -596,10 +596,60 @@ @DatabaseFunction( "sqlitedata_icloud_didUpdate", - as: ((String, CKRecord?.SystemFieldsRepresentation) -> Void).self + as: ( + ( + String, + CKRecord?.SystemFieldsRepresentation, + CKRecord?.SystemFieldsRepresentation, + [String].JSONRepresentation + ) -> Void + ).self ) - func didUpdate(recordName: String, record: CKRecord?) { - let zoneID = record?.recordID.zoneID ?? defaultZone.zoneID + func didUpdate( + recordName: String, + lastKnownServerRecord: CKRecord?, + newParentLastKnownServerRecord: CKRecord? + ) { + // , + // newParentRecord: CKRecord?, + // childrenRecordNames: [String] +// if record.zoneID != newParentRecord.zoneID { +// .deleteRecord(record.recordID) +// .deleteRecord(/*all child records*/) +// .saveRecord(/*construct a record ID with recordName and new zone ?? defaultZone */) +// .saveRecord(/*for all child records*/) +// } + +// let oldZoneID = lastKnownServerRecord?.recordID.zoneID ?? defaultZone.zoneID +// let newZoneID = newParentLastKnownServerRecord?.recordID.zoneID ?? defaultZone.zoneID +// let oldSyncEngine = self.syncEngines.withValue { +// oldZoneID.ownerName == CKCurrentUserDefaultName ? $0.private : $0.shared +// } +// let newSyncEngine = self.syncEngines.withValue { +// newZoneID.ownerName == CKCurrentUserDefaultName ? $0.private : $0.shared +// } +// if oldZoneID != newZoneID { +// oldSyncEngine?.state +// .add(pendingRecordZoneChanges: [.deleteRecord(lastKnownServerRecord!.recordID)]) +// oldSyncEngine?.state.add(pendingRecordZoneChanges: childrenLastKnownServerRecordNames.map { +// .deleteRecord(CKRecord.ID(recordName: $0, zoneID: oldZoneID)) +// }) +// +// newSyncEngine?.state +// .add( +// pendingRecordZoneChanges: [.saveRecord(CKRecord.ID.init(recordName: lastKnownServerRecord!.recordID.recordName, zoneID: newZoneID))] +// ) +// newSyncEngine?.state +// .add( +// pendingRecordZoneChanges: childrenLastKnownServerRecordNames.map { +// .saveRecord(CKRecord.ID.init(recordName: $0, zoneID: newZoneID)) +// } +// ) +// return +// } + + + let zoneID = lastKnownServerRecord?.recordID.zoneID ?? defaultZone.zoneID let change = CKSyncEngine.PendingRecordZoneChange.saveRecord( CKRecord.ID( recordName: recordName, @@ -870,6 +920,18 @@ .sqliteDataCloudKitFailure, catching: { try await metadatabase.read { db in + + try SyncMetadata + .upsert { + SyncMetadata.init( + recordPrimaryKey: <#T##String#>, + recordType: <#T##String#>, + parentRecordPrimaryKey: <#T##String?#>, + parentRecordType: <#T##String?#> + ) + } + .returning(\.self) + try SyncMetadata .where { $0.recordName.eq(recordID.recordName) } .select { ($0, $0._lastKnownServerRecordAllFields) } @@ -1345,7 +1407,9 @@ let table = tablesByName[failedRecord.recordType], foreignKeysByTableName[table.tableName]?.count == 1, let foreignKey = foreignKeysByTableName[table.tableName]?.first - else { continue } + else { + continue + } func open(_: T.Type) async throws { try await userDatabase.write { db in try $_isSynchronizingChanges.withValue(false) { diff --git a/Sources/SQLiteData/CloudKit/SyncMetadata.swift b/Sources/SQLiteData/CloudKit/SyncMetadata.swift index 8fca929d..2ca534a1 100644 --- a/Sources/SQLiteData/CloudKit/SyncMetadata.swift +++ b/Sources/SQLiteData/CloudKit/SyncMetadata.swift @@ -66,7 +66,7 @@ @Column(generated: .virtual) public let hasLastKnownServerRecord: Bool - + /// Determines if the record associated with this metadata is currently shared in CloudKit. /// /// This can only return `true` for root records. For example, the metadata associated with a @@ -82,12 +82,20 @@ @available(iOS 17, macOS 14, tvOS 17, watchOS 10, *) @Table @Selection struct AncestorMetadata { + let isShared: Bool let recordName: String let parentRecordName: String? @Column(as: CKRecord?.SystemFieldsRepresentation.self) let lastKnownServerRecord: CKRecord? } + @available(iOS 17, macOS 14, tvOS 17, watchOS 10, *) + @Table @Selection + struct ChildMetadata { + let recordName: String + let parentRecordName: String? + } + @available(iOS 17, macOS 14, tvOS 17, watchOS 10, *) @Table @Selection struct RecordWithRoot { diff --git a/Tests/SQLiteDataTests/CloudKitTests/NextRecordZoneChangeBatchTests.swift b/Tests/SQLiteDataTests/CloudKitTests/NextRecordZoneChangeBatchTests.swift index 93650a97..c2b3a3fd 100644 --- a/Tests/SQLiteDataTests/CloudKitTests/NextRecordZoneChangeBatchTests.swift +++ b/Tests/SQLiteDataTests/CloudKitTests/NextRecordZoneChangeBatchTests.swift @@ -10,28 +10,28 @@ extension BaseCloudKitTests { @MainActor final class NextRecordZoneChangeBatchTests: BaseCloudKitTests, @unchecked Sendable { - @available(iOS 17, macOS 14, tvOS 17, watchOS 10, *) - @Test func noMetadataForRecord() async throws { - syncEngine.private.state.add( - pendingRecordZoneChanges: [.saveRecord(Reminder.recordID(for: 1))] - ) - - try await syncEngine.processPendingRecordZoneChanges(scope: .private) - assertInlineSnapshot(of: container, as: .customDump) { - """ - MockCloudContainer( - privateCloudDatabase: MockCloudDatabase( - databaseScope: .private, - storage: [] - ), - sharedCloudDatabase: MockCloudDatabase( - databaseScope: .shared, - storage: [] - ) - ) - """ - } - } +// @available(iOS 17, macOS 14, tvOS 17, watchOS 10, *) +// @Test func noMetadataForRecord() async throws { +// syncEngine.private.state.add( +// pendingRecordZoneChanges: [.saveRecord(Reminder.recordID(for: 1))] +// ) +// +// try await syncEngine.processPendingRecordZoneChanges(scope: .private) +// assertInlineSnapshot(of: container, as: .customDump) { +// """ +// MockCloudContainer( +// privateCloudDatabase: MockCloudDatabase( +// databaseScope: .private, +// storage: [] +// ), +// sharedCloudDatabase: MockCloudDatabase( +// databaseScope: .shared, +// storage: [] +// ) +// ) +// """ +// } +// } @available(iOS 17, macOS 14, tvOS 17, watchOS 10, *) @Test func nonExistentTable() async throws { diff --git a/Tests/SQLiteDataTests/CloudKitTests/SharingTests.swift b/Tests/SQLiteDataTests/CloudKitTests/SharingTests.swift index 12ce91a5..f5271ac0 100644 --- a/Tests/SQLiteDataTests/CloudKitTests/SharingTests.swift +++ b/Tests/SQLiteDataTests/CloudKitTests/SharingTests.swift @@ -909,6 +909,169 @@ """ } } + +// @available(iOS 17, macOS 14, tvOS 17, watchOS 10, *) +// @Test func movesChildRecordFromPrivateParentToSharedParent() async throws { +// try await userDatabase.userWrite { db in +// try db.seed { +// ModelA.Draft(id: 1, count: 42) +// ModelB.Draft(id: 1, isOn: true, modelAID: 1) +// ModelC.Draft(id: 1, title: "Blob", modelBID: 1) +// ModelC.Draft(id: 2, title: "Blob Jr", modelBID: 1) +// ModelC.Draft(id: 3, title: "Blob Sr", modelBID: 1) +// } +// } +// try await syncEngine.processPendingRecordZoneChanges(scope: .private) +// +// let externalZone = CKRecordZone( +// zoneID: CKRecordZone.ID( +// zoneName: "external.zone", +// ownerName: "external.owner" +// ) +// ) +// try await syncEngine.modifyRecordZones(scope: .shared, saving: [externalZone]).notify() +// +// let modelARecord = CKRecord( +// recordType: ModelA.tableName, +// recordID: ModelA.recordID(for: 2, zoneID: externalZone.zoneID) +// ) +// modelARecord.setValue(2, forKey: "id", at: now) +// modelARecord.setValue(1729, forKey: "count", at: now) +// let share = CKShare( +// rootRecord: modelARecord, +// shareID: CKRecord.ID( +// recordName: "share-\(modelARecord.recordID.recordName)", +// zoneID: modelARecord.recordID.zoneID +// ) +// ) +// _ = try syncEngine.modifyRecords(scope: .shared, saving: [share, modelARecord]) +// let freshShare = try syncEngine.shared.database.record(for: share.recordID) as! CKShare +// let freshModelARecord = try syncEngine.shared.database.record(for: modelARecord.recordID) +// +// try await syncEngine +// .acceptShare( +// metadata: ShareMetadata( +// containerIdentifier: container.containerIdentifier!, +// hierarchicalRootRecordID: freshModelARecord.recordID, +// rootRecord: freshModelARecord, +// share: freshShare +// ) +// ) +// +// try await userDatabase.userWrite { db in +// try ModelB.find(1).update { $0.modelAID = 2 }.execute(db) +// } +// try await syncEngine.processPendingRecordZoneChanges(scope: .private) +// try await syncEngine.processPendingRecordZoneChanges(scope: .shared) +// +// assertQuery(ModelA.all, database: userDatabase.database) +// assertQuery(ModelB.all, database: userDatabase.database) +// assertQuery(ModelC.all, database: userDatabase.database) +// assertQuery(SyncMetadata.all, database: syncEngine.metadatabase) { +// """ +// ┌──────────────────────────────────────────────────────────────────────────────────────────────┐ +// │ SyncMetadata( │ +// │ recordPrimaryKey: "1", │ +// │ recordType: "modelAs", │ +// │ recordName: "1:modelAs", │ +// │ parentRecordPrimaryKey: nil, │ +// │ parentRecordType: nil, │ +// │ parentRecordName: nil, │ +// │ lastKnownServerRecord: CKRecord( │ +// │ recordID: CKRecord.ID(1:modelAs/zone/__defaultOwner__), │ +// │ recordType: "modelAs", │ +// │ parent: nil, │ +// │ share: nil │ +// │ ), │ +// │ _lastKnownServerRecordAllFields: CKRecord( │ +// │ recordID: CKRecord.ID(1:modelAs/zone/__defaultOwner__), │ +// │ recordType: "modelAs", │ +// │ parent: nil, │ +// │ share: nil, │ +// │ count: 42, │ +// │ id: 1 │ +// │ ), │ +// │ share: nil, │ +// │ _isDeleted: false, │ +// │ hasLastKnownServerRecord: true, │ +// │ isShared: false, │ +// │ userModificationDate: Date(1970-01-01T00:00:00.000Z) │ +// │ ) │ +// ├──────────────────────────────────────────────────────────────────────────────────────────────┤ +// │ SyncMetadata( │ +// │ recordPrimaryKey: "2", │ +// │ recordType: "modelAs", │ +// │ recordName: "2:modelAs", │ +// │ parentRecordPrimaryKey: nil, │ +// │ parentRecordType: nil, │ +// │ parentRecordName: nil, │ +// │ lastKnownServerRecord: CKRecord( │ +// │ recordID: CKRecord.ID(2:modelAs/external.zone/external.owner), │ +// │ recordType: "modelAs", │ +// │ parent: nil, │ +// │ share: CKReference(recordID: CKRecord.ID(share-2:modelAs/external.zone/external.owner)) │ +// │ ), │ +// │ _lastKnownServerRecordAllFields: CKRecord( │ +// │ recordID: CKRecord.ID(2:modelAs/external.zone/external.owner), │ +// │ recordType: "modelAs", │ +// │ parent: nil, │ +// │ share: CKReference(recordID: CKRecord.ID(share-2:modelAs/external.zone/external.owner)), │ +// │ count: 1729, │ +// │ id: 2 │ +// │ ), │ +// │ share: CKRecord( │ +// │ recordID: CKRecord.ID(share-2:modelAs/external.zone/external.owner), │ +// │ recordType: "cloudkit.share", │ +// │ parent: nil, │ +// │ share: nil │ +// │ ), │ +// │ _isDeleted: false, │ +// │ hasLastKnownServerRecord: true, │ +// │ isShared: true, │ +// │ userModificationDate: Date(1970-01-01T00:00:00.000Z) │ +// │ ) │ +// └──────────────────────────────────────────────────────────────────────────────────────────────┘ +// """ +// } +// assertInlineSnapshot(of: container, as: .customDump) { +// """ +// MockCloudContainer( +// privateCloudDatabase: MockCloudDatabase( +// databaseScope: .private, +// storage: [ +// [0]: CKRecord( +// recordID: CKRecord.ID(1:modelAs/zone/__defaultOwner__), +// recordType: "modelAs", +// parent: nil, +// share: nil, +// count: 42, +// id: 1 +// ) +// ] +// ), +// sharedCloudDatabase: MockCloudDatabase( +// databaseScope: .shared, +// storage: [ +// [0]: CKRecord( +// recordID: CKRecord.ID(share-2:modelAs/external.zone/external.owner), +// recordType: "cloudkit.share", +// parent: nil, +// share: nil +// ), +// [1]: CKRecord( +// recordID: CKRecord.ID(2:modelAs/external.zone/external.owner), +// recordType: "modelAs", +// parent: nil, +// share: CKReference(recordID: CKRecord.ID(share-2:modelAs/external.zone/external.owner)), +// count: 1729, +// id: 2 +// ) +// ] +// ) +// ) +// """ +// } +// } } } #endif From 6f4523ecd788a3bffb2e401a510db37646e65e7a Mon Sep 17 00:00:00 2001 From: Brandon Williams Date: Wed, 10 Sep 2025 17:00:07 -0500 Subject: [PATCH 2/5] wip --- .../CloudKit/Internal/Triggers.swift | 33 +-- Sources/SQLiteData/CloudKit/SyncEngine.swift | 81 ++---- .../CloudKitTests/SharingTests.swift | 231 ++++++------------ 3 files changed, 93 insertions(+), 252 deletions(-) diff --git a/Sources/SQLiteData/CloudKit/Internal/Triggers.swift b/Sources/SQLiteData/CloudKit/Internal/Triggers.swift index c451ed2c..56877085 100644 --- a/Sources/SQLiteData/CloudKit/Internal/Triggers.swift +++ b/Sources/SQLiteData/CloudKit/Internal/Triggers.swift @@ -147,8 +147,7 @@ recordName: new.recordName, lastKnownServerRecord: new.lastKnownServerRecord ?? rootServerRecord(recordName: new.recordName), - newParentLastKnownServerRecord: #bind(nil), - childrenLastKnownServerRecordNames: #bind([]) + newParentLastKnownServerRecord: #bind(nil) ) ) } when: { _ in @@ -172,9 +171,6 @@ newParentLastKnownServerRecord: parentLastKnownServerRecordIfShared( recordName: new.recordName, parentRecordName: new.parentRecordName - ), - childrenLastKnownServerRecordNames: childrenLastKnownServerRecordNamesIfShared( - recordName: new.recordName ) ) ) @@ -324,33 +320,6 @@ } } -@available(iOS 17, macOS 14, tvOS 17, watchOS 10, *) - private func childrenLastKnownServerRecordNamesIfShared( - recordName: some QueryExpression - ) -> some QueryExpression<[String].JSONRepresentation> { - With { - SyncMetadata - .where { $0.recordName.eq(recordName) } - .select { ChildMetadata.Columns(recordName: $0.recordName, parentRecordName: #bind(nil)) } - .union(all: true, - SyncMetadata - .select { - ChildMetadata.Columns( - recordName: $0.recordName, - parentRecordName: $0.parentRecordName - ) - } - .join(ChildMetadata.all) { $0.parentRecordName.eq($1.recordName) } - ) - } query: { - ChildMetadata - .where { $0.recordName.neq(recordName) } - .select { - $0.recordName.jsonGroupArray() - } - } - } - @available(iOS 17, macOS 14, tvOS 17, watchOS 10, *) extension AncestorMetadata.Columns { init(_ metadata: SyncMetadata.TableColumns) { diff --git a/Sources/SQLiteData/CloudKit/SyncEngine.swift b/Sources/SQLiteData/CloudKit/SyncEngine.swift index bf23c93e..421a9de9 100644 --- a/Sources/SQLiteData/CloudKit/SyncEngine.swift +++ b/Sources/SQLiteData/CloudKit/SyncEngine.swift @@ -597,57 +597,36 @@ @DatabaseFunction( "sqlitedata_icloud_didUpdate", as: ( - ( - String, - CKRecord?.SystemFieldsRepresentation, - CKRecord?.SystemFieldsRepresentation, - [String].JSONRepresentation - ) -> Void + (String, CKRecord?.SystemFieldsRepresentation, CKRecord?.SystemFieldsRepresentation) -> Void ).self ) func didUpdate( recordName: String, lastKnownServerRecord: CKRecord?, newParentLastKnownServerRecord: CKRecord? - ) { - // , - // newParentRecord: CKRecord?, - // childrenRecordNames: [String] -// if record.zoneID != newParentRecord.zoneID { -// .deleteRecord(record.recordID) -// .deleteRecord(/*all child records*/) -// .saveRecord(/*construct a record ID with recordName and new zone ?? defaultZone */) -// .saveRecord(/*for all child records*/) -// } - -// let oldZoneID = lastKnownServerRecord?.recordID.zoneID ?? defaultZone.zoneID -// let newZoneID = newParentLastKnownServerRecord?.recordID.zoneID ?? defaultZone.zoneID -// let oldSyncEngine = self.syncEngines.withValue { -// oldZoneID.ownerName == CKCurrentUserDefaultName ? $0.private : $0.shared -// } -// let newSyncEngine = self.syncEngines.withValue { -// newZoneID.ownerName == CKCurrentUserDefaultName ? $0.private : $0.shared -// } -// if oldZoneID != newZoneID { -// oldSyncEngine?.state -// .add(pendingRecordZoneChanges: [.deleteRecord(lastKnownServerRecord!.recordID)]) -// oldSyncEngine?.state.add(pendingRecordZoneChanges: childrenLastKnownServerRecordNames.map { -// .deleteRecord(CKRecord.ID(recordName: $0, zoneID: oldZoneID)) -// }) -// -// newSyncEngine?.state -// .add( -// pendingRecordZoneChanges: [.saveRecord(CKRecord.ID.init(recordName: lastKnownServerRecord!.recordID.recordName, zoneID: newZoneID))] -// ) -// newSyncEngine?.state -// .add( -// pendingRecordZoneChanges: childrenLastKnownServerRecordNames.map { -// .saveRecord(CKRecord.ID.init(recordName: $0, zoneID: newZoneID)) -// } -// ) -// return -// } - + ) throws { + let oldZoneID = lastKnownServerRecord?.recordID.zoneID ?? defaultZone.zoneID + let newZoneID = newParentLastKnownServerRecord?.recordID.zoneID ?? defaultZone.zoneID + guard oldZoneID == newZoneID else { + struct RecordChangedZonesError: LocalizedError { + let recordName: String + let oldZoneID: CKRecordZone.ID + let newZoneID: CKRecordZone.ID + var errorDescription: String? { + """ + The record '\(recordName)' was moved from zone \ + '\(oldZoneID.zoneName)/\(oldZoneID.ownerName)' to \ + '\(newZoneID.zoneName)/\(newZoneID.ownerName)'. This is currently not supported in \ + SQLiteData. To work around, delete the old record and create a new record. + """ + } + } + throw RecordChangedZonesError( + recordName: recordName, + oldZoneID: oldZoneID, + newZoneID: newZoneID + ) + } let zoneID = lastKnownServerRecord?.recordID.zoneID ?? defaultZone.zoneID let change = CKSyncEngine.PendingRecordZoneChange.saveRecord( @@ -920,18 +899,6 @@ .sqliteDataCloudKitFailure, catching: { try await metadatabase.read { db in - - try SyncMetadata - .upsert { - SyncMetadata.init( - recordPrimaryKey: <#T##String#>, - recordType: <#T##String#>, - parentRecordPrimaryKey: <#T##String?#>, - parentRecordType: <#T##String?#> - ) - } - .returning(\.self) - try SyncMetadata .where { $0.recordName.eq(recordID.recordName) } .select { ($0, $0._lastKnownServerRecordAllFields) } diff --git a/Tests/SQLiteDataTests/CloudKitTests/SharingTests.swift b/Tests/SQLiteDataTests/CloudKitTests/SharingTests.swift index f5271ac0..86f84043 100644 --- a/Tests/SQLiteDataTests/CloudKitTests/SharingTests.swift +++ b/Tests/SQLiteDataTests/CloudKitTests/SharingTests.swift @@ -629,7 +629,9 @@ ) _ = 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) + let freshRemindersListRecord = try syncEngine.shared.database.record( + for: remindersListRecord.recordID + ) try await syncEngine .acceptShare( @@ -910,168 +912,71 @@ } } -// @available(iOS 17, macOS 14, tvOS 17, watchOS 10, *) -// @Test func movesChildRecordFromPrivateParentToSharedParent() async throws { -// try await userDatabase.userWrite { db in -// try db.seed { -// ModelA.Draft(id: 1, count: 42) -// ModelB.Draft(id: 1, isOn: true, modelAID: 1) -// ModelC.Draft(id: 1, title: "Blob", modelBID: 1) -// ModelC.Draft(id: 2, title: "Blob Jr", modelBID: 1) -// ModelC.Draft(id: 3, title: "Blob Sr", modelBID: 1) -// } -// } -// try await syncEngine.processPendingRecordZoneChanges(scope: .private) -// -// let externalZone = CKRecordZone( -// zoneID: CKRecordZone.ID( -// zoneName: "external.zone", -// ownerName: "external.owner" -// ) -// ) -// try await syncEngine.modifyRecordZones(scope: .shared, saving: [externalZone]).notify() -// -// let modelARecord = CKRecord( -// recordType: ModelA.tableName, -// recordID: ModelA.recordID(for: 2, zoneID: externalZone.zoneID) -// ) -// modelARecord.setValue(2, forKey: "id", at: now) -// modelARecord.setValue(1729, forKey: "count", at: now) -// let share = CKShare( -// rootRecord: modelARecord, -// shareID: CKRecord.ID( -// recordName: "share-\(modelARecord.recordID.recordName)", -// zoneID: modelARecord.recordID.zoneID -// ) -// ) -// _ = try syncEngine.modifyRecords(scope: .shared, saving: [share, modelARecord]) -// let freshShare = try syncEngine.shared.database.record(for: share.recordID) as! CKShare -// let freshModelARecord = try syncEngine.shared.database.record(for: modelARecord.recordID) -// -// try await syncEngine -// .acceptShare( -// metadata: ShareMetadata( -// containerIdentifier: container.containerIdentifier!, -// hierarchicalRootRecordID: freshModelARecord.recordID, -// rootRecord: freshModelARecord, -// share: freshShare -// ) -// ) -// -// try await userDatabase.userWrite { db in -// try ModelB.find(1).update { $0.modelAID = 2 }.execute(db) -// } -// try await syncEngine.processPendingRecordZoneChanges(scope: .private) -// try await syncEngine.processPendingRecordZoneChanges(scope: .shared) -// -// assertQuery(ModelA.all, database: userDatabase.database) -// assertQuery(ModelB.all, database: userDatabase.database) -// assertQuery(ModelC.all, database: userDatabase.database) -// assertQuery(SyncMetadata.all, database: syncEngine.metadatabase) { -// """ -// ┌──────────────────────────────────────────────────────────────────────────────────────────────┐ -// │ SyncMetadata( │ -// │ recordPrimaryKey: "1", │ -// │ recordType: "modelAs", │ -// │ recordName: "1:modelAs", │ -// │ parentRecordPrimaryKey: nil, │ -// │ parentRecordType: nil, │ -// │ parentRecordName: nil, │ -// │ lastKnownServerRecord: CKRecord( │ -// │ recordID: CKRecord.ID(1:modelAs/zone/__defaultOwner__), │ -// │ recordType: "modelAs", │ -// │ parent: nil, │ -// │ share: nil │ -// │ ), │ -// │ _lastKnownServerRecordAllFields: CKRecord( │ -// │ recordID: CKRecord.ID(1:modelAs/zone/__defaultOwner__), │ -// │ recordType: "modelAs", │ -// │ parent: nil, │ -// │ share: nil, │ -// │ count: 42, │ -// │ id: 1 │ -// │ ), │ -// │ share: nil, │ -// │ _isDeleted: false, │ -// │ hasLastKnownServerRecord: true, │ -// │ isShared: false, │ -// │ userModificationDate: Date(1970-01-01T00:00:00.000Z) │ -// │ ) │ -// ├──────────────────────────────────────────────────────────────────────────────────────────────┤ -// │ SyncMetadata( │ -// │ recordPrimaryKey: "2", │ -// │ recordType: "modelAs", │ -// │ recordName: "2:modelAs", │ -// │ parentRecordPrimaryKey: nil, │ -// │ parentRecordType: nil, │ -// │ parentRecordName: nil, │ -// │ lastKnownServerRecord: CKRecord( │ -// │ recordID: CKRecord.ID(2:modelAs/external.zone/external.owner), │ -// │ recordType: "modelAs", │ -// │ parent: nil, │ -// │ share: CKReference(recordID: CKRecord.ID(share-2:modelAs/external.zone/external.owner)) │ -// │ ), │ -// │ _lastKnownServerRecordAllFields: CKRecord( │ -// │ recordID: CKRecord.ID(2:modelAs/external.zone/external.owner), │ -// │ recordType: "modelAs", │ -// │ parent: nil, │ -// │ share: CKReference(recordID: CKRecord.ID(share-2:modelAs/external.zone/external.owner)), │ -// │ count: 1729, │ -// │ id: 2 │ -// │ ), │ -// │ share: CKRecord( │ -// │ recordID: CKRecord.ID(share-2:modelAs/external.zone/external.owner), │ -// │ recordType: "cloudkit.share", │ -// │ parent: nil, │ -// │ share: nil │ -// │ ), │ -// │ _isDeleted: false, │ -// │ hasLastKnownServerRecord: true, │ -// │ isShared: true, │ -// │ userModificationDate: Date(1970-01-01T00:00:00.000Z) │ -// │ ) │ -// └──────────────────────────────────────────────────────────────────────────────────────────────┘ -// """ -// } -// assertInlineSnapshot(of: container, as: .customDump) { -// """ -// MockCloudContainer( -// privateCloudDatabase: MockCloudDatabase( -// databaseScope: .private, -// storage: [ -// [0]: CKRecord( -// recordID: CKRecord.ID(1:modelAs/zone/__defaultOwner__), -// recordType: "modelAs", -// parent: nil, -// share: nil, -// count: 42, -// id: 1 -// ) -// ] -// ), -// sharedCloudDatabase: MockCloudDatabase( -// databaseScope: .shared, -// storage: [ -// [0]: CKRecord( -// recordID: CKRecord.ID(share-2:modelAs/external.zone/external.owner), -// recordType: "cloudkit.share", -// parent: nil, -// share: nil -// ), -// [1]: CKRecord( -// recordID: CKRecord.ID(2:modelAs/external.zone/external.owner), -// recordType: "modelAs", -// parent: nil, -// share: CKReference(recordID: CKRecord.ID(share-2:modelAs/external.zone/external.owner)), -// count: 1729, -// id: 2 -// ) -// ] -// ) -// ) -// """ -// } -// } + @available(iOS 17, macOS 14, tvOS 17, watchOS 10, *) + @Test func movesChildRecordFromPrivateParentToSharedParent() async throws { + try await userDatabase.userWrite { db in + try db.seed { + ModelA.Draft(id: 1, count: 42) + ModelB.Draft(id: 1, isOn: true, modelAID: 1) + ModelC.Draft(id: 1, title: "Blob", modelBID: 1) + ModelC.Draft(id: 2, title: "Blob Jr", modelBID: 1) + ModelC.Draft(id: 3, title: "Blob Sr", modelBID: 1) + } + } + try await syncEngine.processPendingRecordZoneChanges(scope: .private) + + let externalZone = CKRecordZone( + zoneID: CKRecordZone.ID( + zoneName: "external.zone", + ownerName: "external.owner" + ) + ) + try await syncEngine.modifyRecordZones(scope: .shared, saving: [externalZone]).notify() + + let modelARecord = CKRecord( + recordType: ModelA.tableName, + recordID: ModelA.recordID(for: 2, zoneID: externalZone.zoneID) + ) + modelARecord.setValue(2, forKey: "id", at: now) + modelARecord.setValue(1729, forKey: "count", at: now) + let share = CKShare( + rootRecord: modelARecord, + shareID: CKRecord.ID( + recordName: "share-\(modelARecord.recordID.recordName)", + zoneID: modelARecord.recordID.zoneID + ) + ) + _ = try syncEngine.modifyRecords(scope: .shared, saving: [share, modelARecord]) + let freshShare = try syncEngine.shared.database.record(for: share.recordID) as! CKShare + let freshModelARecord = try syncEngine.shared.database.record(for: modelARecord.recordID) + + try await syncEngine + .acceptShare( + metadata: ShareMetadata( + containerIdentifier: container.containerIdentifier!, + hierarchicalRootRecordID: freshModelARecord.recordID, + rootRecord: freshModelARecord, + share: freshShare + ) + ) + + let error = try #require( + await #expect(throws: DatabaseError.self) { + try await userDatabase.userWrite { db in + try ModelB.find(1).update { $0.modelAID = 2 }.execute(db) + } + } + ) + #expect( + error.localizedDescription.contains( + """ + The record '1:modelBs' was moved from zone 'zone/__defaultOwner__' to \ + 'external.zone/external.owner'. This is currently not supported in SQLiteData. To work \ + around, delete the old record and create a new record. + """ + ) + ) + } } } #endif From 6ac1f22f31acf4ffd4778cb07c7f14f8f71180a0 Mon Sep 17 00:00:00 2001 From: Brandon Williams Date: Thu, 11 Sep 2025 09:01:14 -0500 Subject: [PATCH 3/5] wip --- Package.swift | 2 +- Package@swift-6.0.swift | 2 +- .../CloudKit/Internal/Triggers.swift | 43 ++++++--------- Sources/SQLiteData/CloudKit/SyncEngine.swift | 6 ++- .../NextRecordZoneChangeBatchTests.swift | 44 ++++++++-------- .../CloudKitTests/SharingTests.swift | 3 -- .../SyncEngineLifecycleTests.swift | 52 +++++++++++++++++++ 7 files changed, 96 insertions(+), 56 deletions(-) diff --git a/Package.swift b/Package.swift index 6c1e8b2a..c1d9d21d 100644 --- a/Package.swift +++ b/Package.swift @@ -35,7 +35,7 @@ let package = Package( .package(url: "https://github.com/pointfreeco/swift-snapshot-testing", from: "1.18.4"), .package( url: "https://github.com/pointfreeco/swift-structured-queries", - from: "0.19.0", + from: "0.19.1", traits: [ .trait(name: "StructuredQueriesTagged", condition: .when(traits: ["SQLiteDataTagged"])) ] diff --git a/Package@swift-6.0.swift b/Package@swift-6.0.swift index 5d4e3302..7f09a24d 100644 --- a/Package@swift-6.0.swift +++ b/Package@swift-6.0.swift @@ -27,7 +27,7 @@ let package = Package( .package(url: "https://github.com/pointfreeco/swift-dependencies", from: "1.9.0"), .package(url: "https://github.com/pointfreeco/swift-sharing", from: "2.3.0"), .package(url: "https://github.com/pointfreeco/swift-snapshot-testing", from: "1.18.4"), - .package(url: "https://github.com/pointfreeco/swift-structured-queries", from: "0.19.0"), + .package(url: "https://github.com/pointfreeco/swift-structured-queries", from: "0.19.1"), .package(url: "https://github.com/pointfreeco/xctest-dynamic-overlay", from: "1.5.0"), ], targets: [ diff --git a/Sources/SQLiteData/CloudKit/Internal/Triggers.swift b/Sources/SQLiteData/CloudKit/Internal/Triggers.swift index 56877085..3e1b9d9a 100644 --- a/Sources/SQLiteData/CloudKit/Internal/Triggers.swift +++ b/Sources/SQLiteData/CloudKit/Internal/Triggers.swift @@ -147,7 +147,13 @@ recordName: new.recordName, lastKnownServerRecord: new.lastKnownServerRecord ?? rootServerRecord(recordName: new.recordName), - newParentLastKnownServerRecord: #bind(nil) + newParentLastKnownServerRecord: parentLastKnownServerRecordIfShared( + recordName: new.recordName, + parentRecordPrimaryKey: new.parentRecordPrimaryKey, + parentRecordType: new.parentRecordType + ), + parentRecordPrimaryKey: new.parentRecordPrimaryKey, + parentRecordType: new.parentRecordType ) ) } when: { _ in @@ -170,8 +176,11 @@ ?? rootServerRecord(recordName: new.recordName), newParentLastKnownServerRecord: parentLastKnownServerRecordIfShared( recordName: new.recordName, - parentRecordName: new.parentRecordName - ) + parentRecordPrimaryKey: new.parentRecordPrimaryKey, + parentRecordType: new.parentRecordType + ), + parentRecordPrimaryKey: new.parentRecordPrimaryKey, + parentRecordType: new.parentRecordType ) ) } when: { old, new in @@ -286,37 +295,17 @@ } } - @available(iOS 17, macOS 14, tvOS 17, watchOS 10, *) - private func hasSharedAncestor( - recordName: some QueryExpression - ) -> some QueryExpression { - With { - SyncMetadata - .where { $0.recordName.eq(recordName) } - .select { AncestorMetadata.Columns($0) } - .union( - all: true, - SyncMetadata - .select { AncestorMetadata.Columns($0) } - .join(AncestorMetadata.all) { $0.recordName.is($1.parentRecordName) } - ) - } query: { - AncestorMetadata - .select(\.isShared) - .where { $0.parentRecordName.is(nil) && $0.isShared } - } - } - @available(iOS 17, macOS 14, tvOS 17, watchOS 10, *) private func parentLastKnownServerRecordIfShared( recordName: some QueryExpression, - parentRecordName: some QueryExpression + parentRecordPrimaryKey: some QueryExpression, + parentRecordType: some QueryExpression ) -> some QueryExpression { SyncMetadata .select(\.lastKnownServerRecord) .where { - $0.recordName.is(parentRecordName) - && hasSharedAncestor(recordName: recordName) + $0.recordPrimaryKey.is(parentRecordPrimaryKey) + && $0.recordType.is(parentRecordType) } } diff --git a/Sources/SQLiteData/CloudKit/SyncEngine.swift b/Sources/SQLiteData/CloudKit/SyncEngine.swift index 421a9de9..0cf36ad8 100644 --- a/Sources/SQLiteData/CloudKit/SyncEngine.swift +++ b/Sources/SQLiteData/CloudKit/SyncEngine.swift @@ -597,13 +597,15 @@ @DatabaseFunction( "sqlitedata_icloud_didUpdate", as: ( - (String, CKRecord?.SystemFieldsRepresentation, CKRecord?.SystemFieldsRepresentation) -> Void + (String, CKRecord?.SystemFieldsRepresentation, CKRecord?.SystemFieldsRepresentation, String?, String?) -> Void ).self ) func didUpdate( recordName: String, lastKnownServerRecord: CKRecord?, - newParentLastKnownServerRecord: CKRecord? + newParentLastKnownServerRecord: CKRecord?, + parentRecordPrimaryKey: String? = nil, + parentRecordType: String? = nil ) throws { let oldZoneID = lastKnownServerRecord?.recordID.zoneID ?? defaultZone.zoneID let newZoneID = newParentLastKnownServerRecord?.recordID.zoneID ?? defaultZone.zoneID diff --git a/Tests/SQLiteDataTests/CloudKitTests/NextRecordZoneChangeBatchTests.swift b/Tests/SQLiteDataTests/CloudKitTests/NextRecordZoneChangeBatchTests.swift index c2b3a3fd..93650a97 100644 --- a/Tests/SQLiteDataTests/CloudKitTests/NextRecordZoneChangeBatchTests.swift +++ b/Tests/SQLiteDataTests/CloudKitTests/NextRecordZoneChangeBatchTests.swift @@ -10,28 +10,28 @@ extension BaseCloudKitTests { @MainActor final class NextRecordZoneChangeBatchTests: BaseCloudKitTests, @unchecked Sendable { -// @available(iOS 17, macOS 14, tvOS 17, watchOS 10, *) -// @Test func noMetadataForRecord() async throws { -// syncEngine.private.state.add( -// pendingRecordZoneChanges: [.saveRecord(Reminder.recordID(for: 1))] -// ) -// -// try await syncEngine.processPendingRecordZoneChanges(scope: .private) -// assertInlineSnapshot(of: container, as: .customDump) { -// """ -// MockCloudContainer( -// privateCloudDatabase: MockCloudDatabase( -// databaseScope: .private, -// storage: [] -// ), -// sharedCloudDatabase: MockCloudDatabase( -// databaseScope: .shared, -// storage: [] -// ) -// ) -// """ -// } -// } + @available(iOS 17, macOS 14, tvOS 17, watchOS 10, *) + @Test func noMetadataForRecord() async throws { + syncEngine.private.state.add( + pendingRecordZoneChanges: [.saveRecord(Reminder.recordID(for: 1))] + ) + + try await syncEngine.processPendingRecordZoneChanges(scope: .private) + assertInlineSnapshot(of: container, as: .customDump) { + """ + MockCloudContainer( + privateCloudDatabase: MockCloudDatabase( + databaseScope: .private, + storage: [] + ), + sharedCloudDatabase: MockCloudDatabase( + databaseScope: .shared, + storage: [] + ) + ) + """ + } + } @available(iOS 17, macOS 14, tvOS 17, watchOS 10, *) @Test func nonExistentTable() async throws { diff --git a/Tests/SQLiteDataTests/CloudKitTests/SharingTests.swift b/Tests/SQLiteDataTests/CloudKitTests/SharingTests.swift index 86f84043..1d8b1420 100644 --- a/Tests/SQLiteDataTests/CloudKitTests/SharingTests.swift +++ b/Tests/SQLiteDataTests/CloudKitTests/SharingTests.swift @@ -918,9 +918,6 @@ try db.seed { ModelA.Draft(id: 1, count: 42) ModelB.Draft(id: 1, isOn: true, modelAID: 1) - ModelC.Draft(id: 1, title: "Blob", modelBID: 1) - ModelC.Draft(id: 2, title: "Blob Jr", modelBID: 1) - ModelC.Draft(id: 3, title: "Blob Sr", modelBID: 1) } } try await syncEngine.processPendingRecordZoneChanges(scope: .private) diff --git a/Tests/SQLiteDataTests/CloudKitTests/SyncEngineLifecycleTests.swift b/Tests/SQLiteDataTests/CloudKitTests/SyncEngineLifecycleTests.swift index 8ed166d6..60e3e0aa 100644 --- a/Tests/SQLiteDataTests/CloudKitTests/SyncEngineLifecycleTests.swift +++ b/Tests/SQLiteDataTests/CloudKitTests/SyncEngineLifecycleTests.swift @@ -266,6 +266,58 @@ } } + try await Task.sleep(for: .seconds(1)) + assertQuery(SyncMetadata.all, database: syncEngine.metadatabase) { + """ + ┌───────────────────────────────────────────────────────────────────────────┐ + │ SyncMetadata( │ + │ recordPrimaryKey: "1", │ + │ recordType: "remindersLists", │ + │ recordName: "1:remindersLists", │ + │ parentRecordPrimaryKey: nil, │ + │ parentRecordType: nil, │ + │ parentRecordName: nil, │ + │ lastKnownServerRecord: CKRecord( │ + │ recordID: CKRecord.ID(1:remindersLists/external.zone/external.owner), │ + │ recordType: "remindersLists", │ + │ parent: nil, │ + │ share: nil │ + │ ), │ + │ _lastKnownServerRecordAllFields: CKRecord( │ + │ recordID: CKRecord.ID(1:remindersLists/external.zone/external.owner), │ + │ recordType: "remindersLists", │ + │ parent: nil, │ + │ share: nil, │ + │ id: 1, │ + │ isCompleted: 0, │ + │ title: "Personal" │ + │ ), │ + │ share: nil, │ + │ _isDeleted: false, │ + │ hasLastKnownServerRecord: true, │ + │ isShared: false, │ + │ userModificationDate: Date(1970-01-01T00:00:00.000Z) │ + │ ) │ + ├───────────────────────────────────────────────────────────────────────────┤ + │ SyncMetadata( │ + │ recordPrimaryKey: "1", │ + │ recordType: "reminders", │ + │ recordName: "1:reminders", │ + │ parentRecordPrimaryKey: "1", │ + │ parentRecordType: "remindersLists", │ + │ parentRecordName: "1:remindersLists", │ + │ lastKnownServerRecord: nil, │ + │ _lastKnownServerRecordAllFields: nil, │ + │ share: nil, │ + │ _isDeleted: false, │ + │ hasLastKnownServerRecord: false, │ + │ isShared: false, │ + │ userModificationDate: Date(1970-01-01T00:01:00.000Z) │ + │ ) │ + └───────────────────────────────────────────────────────────────────────────┘ + """ + } + try await Task.sleep(for: .seconds(0.5)) try await syncEngine.start() try await syncEngine.processPendingRecordZoneChanges(scope: .shared) From ec97dd4ad8143fe98a04d3502ebf5aeeebfe9bf8 Mon Sep 17 00:00:00 2001 From: Brandon Williams Date: Thu, 11 Sep 2025 09:32:27 -0500 Subject: [PATCH 4/5] wip --- .../CloudKit/Internal/Triggers.swift | 4 - Sources/SQLiteData/CloudKit/SyncEngine.swift | 32 +-- .../SQLiteData/CloudKit/SyncMetadata.swift | 8 - .../CloudKitTests/SharingTests.swift | 223 +++++++++++++++++- .../CloudKitTests/TriggerTests.swift | 12 +- 5 files changed, 230 insertions(+), 49 deletions(-) diff --git a/Sources/SQLiteData/CloudKit/Internal/Triggers.swift b/Sources/SQLiteData/CloudKit/Internal/Triggers.swift index 3e1b9d9a..c934286b 100644 --- a/Sources/SQLiteData/CloudKit/Internal/Triggers.swift +++ b/Sources/SQLiteData/CloudKit/Internal/Triggers.swift @@ -148,7 +148,6 @@ lastKnownServerRecord: new.lastKnownServerRecord ?? rootServerRecord(recordName: new.recordName), newParentLastKnownServerRecord: parentLastKnownServerRecordIfShared( - recordName: new.recordName, parentRecordPrimaryKey: new.parentRecordPrimaryKey, parentRecordType: new.parentRecordType ), @@ -175,7 +174,6 @@ lastKnownServerRecord: new.lastKnownServerRecord ?? rootServerRecord(recordName: new.recordName), newParentLastKnownServerRecord: parentLastKnownServerRecordIfShared( - recordName: new.recordName, parentRecordPrimaryKey: new.parentRecordPrimaryKey, parentRecordType: new.parentRecordType ), @@ -297,7 +295,6 @@ @available(iOS 17, macOS 14, tvOS 17, watchOS 10, *) private func parentLastKnownServerRecordIfShared( - recordName: some QueryExpression, parentRecordPrimaryKey: some QueryExpression, parentRecordType: some QueryExpression ) -> some QueryExpression { @@ -313,7 +310,6 @@ extension AncestorMetadata.Columns { init(_ metadata: SyncMetadata.TableColumns) { self.init( - isShared: metadata.isShared, recordName: metadata.recordName, parentRecordName: metadata.parentRecordName, lastKnownServerRecord: metadata.lastKnownServerRecord diff --git a/Sources/SQLiteData/CloudKit/SyncEngine.swift b/Sources/SQLiteData/CloudKit/SyncEngine.swift index 0cf36ad8..3d27096e 100644 --- a/Sources/SQLiteData/CloudKit/SyncEngine.swift +++ b/Sources/SQLiteData/CloudKit/SyncEngine.swift @@ -607,30 +607,18 @@ parentRecordPrimaryKey: String? = nil, parentRecordType: String? = nil ) throws { - let oldZoneID = lastKnownServerRecord?.recordID.zoneID ?? defaultZone.zoneID - let newZoneID = newParentLastKnownServerRecord?.recordID.zoneID ?? defaultZone.zoneID - guard oldZoneID == newZoneID else { - struct RecordChangedZonesError: LocalizedError { - let recordName: String - let oldZoneID: CKRecordZone.ID - let newZoneID: CKRecordZone.ID - var errorDescription: String? { - """ - The record '\(recordName)' was moved from zone \ - '\(oldZoneID.zoneName)/\(oldZoneID.ownerName)' to \ - '\(newZoneID.zoneName)/\(newZoneID.ownerName)'. This is currently not supported in \ - SQLiteData. To work around, delete the old record and create a new record. - """ - } - } - throw RecordChangedZonesError( - recordName: recordName, - oldZoneID: oldZoneID, - newZoneID: newZoneID - ) + let zoneID = lastKnownServerRecord?.recordID.zoneID ?? defaultZone.zoneID + let newZoneID = newParentLastKnownServerRecord?.recordID.zoneID + if let newZoneID, zoneID != newZoneID { + reportIssue(""" + The record '\(recordName)' was moved from zone \ + '\(zoneID.zoneName)/\(zoneID.ownerName)' to \ + '\(newZoneID.zoneName)/\(newZoneID.ownerName)'. This is currently not supported in \ + SQLiteData. To work around, delete the record and then create a new record with its \ + new parent association. + """) } - let zoneID = lastKnownServerRecord?.recordID.zoneID ?? defaultZone.zoneID let change = CKSyncEngine.PendingRecordZoneChange.saveRecord( CKRecord.ID( recordName: recordName, diff --git a/Sources/SQLiteData/CloudKit/SyncMetadata.swift b/Sources/SQLiteData/CloudKit/SyncMetadata.swift index 2ca534a1..29a05cb7 100644 --- a/Sources/SQLiteData/CloudKit/SyncMetadata.swift +++ b/Sources/SQLiteData/CloudKit/SyncMetadata.swift @@ -82,20 +82,12 @@ @available(iOS 17, macOS 14, tvOS 17, watchOS 10, *) @Table @Selection struct AncestorMetadata { - let isShared: Bool let recordName: String let parentRecordName: String? @Column(as: CKRecord?.SystemFieldsRepresentation.self) let lastKnownServerRecord: CKRecord? } - @available(iOS 17, macOS 14, tvOS 17, watchOS 10, *) - @Table @Selection - struct ChildMetadata { - let recordName: String - let parentRecordName: String? - } - @available(iOS 17, macOS 14, tvOS 17, watchOS 10, *) @Table @Selection struct RecordWithRoot { diff --git a/Tests/SQLiteDataTests/CloudKitTests/SharingTests.swift b/Tests/SQLiteDataTests/CloudKitTests/SharingTests.swift index 1d8b1420..30bf82b4 100644 --- a/Tests/SQLiteDataTests/CloudKitTests/SharingTests.swift +++ b/Tests/SQLiteDataTests/CloudKitTests/SharingTests.swift @@ -355,6 +355,97 @@ } try await syncEngine.processPendingRecordZoneChanges(scope: .shared) + assertQuery(SyncMetadata.all, database: syncEngine.metadatabase) { + """ + ┌─────────────────────────────────────────────────────────────────────────────────────────┐ + │ SyncMetadata( │ + │ recordPrimaryKey: "1", │ + │ recordType: "modelAs", │ + │ recordName: "1:modelAs", │ + │ parentRecordPrimaryKey: nil, │ + │ parentRecordType: nil, │ + │ parentRecordName: nil, │ + │ lastKnownServerRecord: CKRecord( │ + │ recordID: CKRecord.ID(1:modelAs/external.zone/external.owner), │ + │ recordType: "modelAs", │ + │ parent: nil, │ + │ share: nil │ + │ ), │ + │ _lastKnownServerRecordAllFields: CKRecord( │ + │ recordID: CKRecord.ID(1:modelAs/external.zone/external.owner), │ + │ recordType: "modelAs", │ + │ parent: nil, │ + │ share: nil, │ + │ count: 0, │ + │ id: 1 │ + │ ), │ + │ share: nil, │ + │ _isDeleted: false, │ + │ hasLastKnownServerRecord: true, │ + │ isShared: false, │ + │ userModificationDate: Date(1970-01-01T00:00:00.000Z) │ + │ ) │ + ├─────────────────────────────────────────────────────────────────────────────────────────┤ + │ SyncMetadata( │ + │ recordPrimaryKey: "1", │ + │ recordType: "modelBs", │ + │ recordName: "1:modelBs", │ + │ parentRecordPrimaryKey: "1", │ + │ parentRecordType: "modelAs", │ + │ parentRecordName: "1:modelAs", │ + │ lastKnownServerRecord: CKRecord( │ + │ recordID: CKRecord.ID(1:modelBs/external.zone/external.owner), │ + │ recordType: "modelBs", │ + │ parent: CKReference(recordID: CKRecord.ID(1:modelAs/external.zone/external.owner)), │ + │ share: nil │ + │ ), │ + │ _lastKnownServerRecordAllFields: CKRecord( │ + │ recordID: CKRecord.ID(1:modelBs/external.zone/external.owner), │ + │ recordType: "modelBs", │ + │ parent: CKReference(recordID: CKRecord.ID(1:modelAs/external.zone/external.owner)), │ + │ share: nil, │ + │ id: 1, │ + │ isOn: 0, │ + │ modelAID: 1 │ + │ ), │ + │ share: nil, │ + │ _isDeleted: false, │ + │ hasLastKnownServerRecord: true, │ + │ isShared: false, │ + │ userModificationDate: Date(1970-01-01T00:01:00.000Z) │ + │ ) │ + ├─────────────────────────────────────────────────────────────────────────────────────────┤ + │ SyncMetadata( │ + │ recordPrimaryKey: "1", │ + │ recordType: "modelCs", │ + │ recordName: "1:modelCs", │ + │ parentRecordPrimaryKey: "1", │ + │ parentRecordType: "modelBs", │ + │ parentRecordName: "1:modelBs", │ + │ lastKnownServerRecord: CKRecord( │ + │ recordID: CKRecord.ID(1:modelCs/external.zone/external.owner), │ + │ recordType: "modelCs", │ + │ parent: CKReference(recordID: CKRecord.ID(1:modelBs/external.zone/external.owner)), │ + │ share: nil │ + │ ), │ + │ _lastKnownServerRecordAllFields: CKRecord( │ + │ recordID: CKRecord.ID(1:modelCs/external.zone/external.owner), │ + │ recordType: "modelCs", │ + │ parent: CKReference(recordID: CKRecord.ID(1:modelBs/external.zone/external.owner)), │ + │ share: nil, │ + │ id: 1, │ + │ modelBID: 1, │ + │ title: "" │ + │ ), │ + │ share: nil, │ + │ _isDeleted: false, │ + │ hasLastKnownServerRecord: true, │ + │ isShared: false, │ + │ userModificationDate: Date(1970-01-01T00:01:00.000Z) │ + │ ) │ + └─────────────────────────────────────────────────────────────────────────────────────────┘ + """ + } assertInlineSnapshot(of: container, as: .customDump) { """ MockCloudContainer( @@ -957,22 +1048,128 @@ ) ) - let error = try #require( - await #expect(throws: DatabaseError.self) { - try await userDatabase.userWrite { db in - try ModelB.find(1).update { $0.modelAID = 2 }.execute(db) - } + try await withKnownIssue { + try await userDatabase.userWrite { db in + try ModelB.find(1).update { $0.modelAID = 2 }.execute(db) } - ) - #expect( - error.localizedDescription.contains( - """ + } matching: { issue in + issue.description.hasSuffix(""" The record '1:modelBs' was moved from zone 'zone/__defaultOwner__' to \ - 'external.zone/external.owner'. This is currently not supported in SQLiteData. To work \ - around, delete the old record and create a new record. - """ + 'external.zone/external.owner'. This is currently not supported in \ + SQLiteData. To work around, delete the record and then create a new record with its \ + new parent association. + """) + } + try await syncEngine.processPendingRecordZoneChanges(scope: .private) + try await syncEngine.processPendingRecordZoneChanges(scope: .private) + assertQuery(ModelB.all, database: userDatabase.database) { + """ + """ + } + assertQuery(SyncMetadata.all, database: syncEngine.metadatabase) { + """ + ┌──────────────────────────────────────────────────────────────────────────────────────────────┐ + │ SyncMetadata( │ + │ recordPrimaryKey: "1", │ + │ recordType: "modelAs", │ + │ recordName: "1:modelAs", │ + │ parentRecordPrimaryKey: nil, │ + │ parentRecordType: nil, │ + │ parentRecordName: nil, │ + │ lastKnownServerRecord: CKRecord( │ + │ recordID: CKRecord.ID(1:modelAs/zone/__defaultOwner__), │ + │ recordType: "modelAs", │ + │ parent: nil, │ + │ share: nil │ + │ ), │ + │ _lastKnownServerRecordAllFields: CKRecord( │ + │ recordID: CKRecord.ID(1:modelAs/zone/__defaultOwner__), │ + │ recordType: "modelAs", │ + │ parent: nil, │ + │ share: nil, │ + │ count: 42, │ + │ id: 1 │ + │ ), │ + │ share: nil, │ + │ _isDeleted: false, │ + │ hasLastKnownServerRecord: true, │ + │ isShared: false, │ + │ userModificationDate: Date(1970-01-01T00:00:00.000Z) │ + │ ) │ + ├──────────────────────────────────────────────────────────────────────────────────────────────┤ + │ SyncMetadata( │ + │ recordPrimaryKey: "2", │ + │ recordType: "modelAs", │ + │ recordName: "2:modelAs", │ + │ parentRecordPrimaryKey: nil, │ + │ parentRecordType: nil, │ + │ parentRecordName: nil, │ + │ lastKnownServerRecord: CKRecord( │ + │ recordID: CKRecord.ID(2:modelAs/external.zone/external.owner), │ + │ recordType: "modelAs", │ + │ parent: nil, │ + │ share: CKReference(recordID: CKRecord.ID(share-2:modelAs/external.zone/external.owner)) │ + │ ), │ + │ _lastKnownServerRecordAllFields: CKRecord( │ + │ recordID: CKRecord.ID(2:modelAs/external.zone/external.owner), │ + │ recordType: "modelAs", │ + │ parent: nil, │ + │ share: CKReference(recordID: CKRecord.ID(share-2:modelAs/external.zone/external.owner)), │ + │ count: 1729, │ + │ id: 2 │ + │ ), │ + │ share: CKRecord( │ + │ recordID: CKRecord.ID(share-2:modelAs/external.zone/external.owner), │ + │ recordType: "cloudkit.share", │ + │ parent: nil, │ + │ share: nil │ + │ ), │ + │ _isDeleted: false, │ + │ hasLastKnownServerRecord: true, │ + │ isShared: true, │ + │ userModificationDate: Date(1970-01-01T00:00:00.000Z) │ + │ ) │ + └──────────────────────────────────────────────────────────────────────────────────────────────┘ + """ + } + assertInlineSnapshot(of: container, as: .customDump) { + """ + MockCloudContainer( + privateCloudDatabase: MockCloudDatabase( + databaseScope: .private, + storage: [ + [0]: CKRecord( + recordID: CKRecord.ID(1:modelAs/zone/__defaultOwner__), + recordType: "modelAs", + parent: nil, + share: nil, + count: 42, + id: 1 + ) + ] + ), + sharedCloudDatabase: MockCloudDatabase( + databaseScope: .shared, + storage: [ + [0]: CKRecord( + recordID: CKRecord.ID(share-2:modelAs/external.zone/external.owner), + recordType: "cloudkit.share", + parent: nil, + share: nil + ), + [1]: CKRecord( + recordID: CKRecord.ID(2:modelAs/external.zone/external.owner), + recordType: "modelAs", + parent: nil, + share: CKReference(recordID: CKRecord.ID(share-2:modelAs/external.zone/external.owner)), + count: 1729, + id: 2 + ) + ] + ) ) - ) + """ + } } } } diff --git a/Tests/SQLiteDataTests/CloudKitTests/TriggerTests.swift b/Tests/SQLiteDataTests/CloudKitTests/TriggerTests.swift index 5c407e2a..9afac03a 100644 --- a/Tests/SQLiteDataTests/CloudKitTests/TriggerTests.swift +++ b/Tests/SQLiteDataTests/CloudKitTests/TriggerTests.swift @@ -57,7 +57,11 @@ SELECT "ancestorMetadatas"."lastKnownServerRecord" FROM "ancestorMetadatas" WHERE ("ancestorMetadatas"."parentRecordName" IS NULL) - ))); + )), ( + SELECT "sqlitedata_icloud_metadata"."lastKnownServerRecord" + FROM "sqlitedata_icloud_metadata" + WHERE (("sqlitedata_icloud_metadata"."recordPrimaryKey" IS "new"."parentRecordPrimaryKey") AND ("sqlitedata_icloud_metadata"."recordType" IS "new"."parentRecordType")) + ), "new"."parentRecordPrimaryKey", "new"."parentRecordType"); END """, [2]: """ @@ -79,7 +83,11 @@ SELECT "ancestorMetadatas"."lastKnownServerRecord" FROM "ancestorMetadatas" WHERE ("ancestorMetadatas"."parentRecordName" IS NULL) - ))); + )), ( + SELECT "sqlitedata_icloud_metadata"."lastKnownServerRecord" + FROM "sqlitedata_icloud_metadata" + WHERE (("sqlitedata_icloud_metadata"."recordPrimaryKey" IS "new"."parentRecordPrimaryKey") AND ("sqlitedata_icloud_metadata"."recordType" IS "new"."parentRecordType")) + ), "new"."parentRecordPrimaryKey", "new"."parentRecordType"); END """, [3]: """ From 24c953d181a89de2ba0b2052840c8072bd2653cf Mon Sep 17 00:00:00 2001 From: Brandon Williams Date: Mon, 15 Sep 2025 11:12:06 -0500 Subject: [PATCH 5/5] wip --- Sources/SQLiteData/CloudKit/SyncEngine.swift | 22 +++++-- .../CloudKitTests/SharingTests.swift | 64 +++++++++++++++---- 2 files changed, 68 insertions(+), 18 deletions(-) diff --git a/Sources/SQLiteData/CloudKit/SyncEngine.swift b/Sources/SQLiteData/CloudKit/SyncEngine.swift index 3d27096e..3ad8abf5 100644 --- a/Sources/SQLiteData/CloudKit/SyncEngine.swift +++ b/Sources/SQLiteData/CloudKit/SyncEngine.swift @@ -610,13 +610,21 @@ let zoneID = lastKnownServerRecord?.recordID.zoneID ?? defaultZone.zoneID let newZoneID = newParentLastKnownServerRecord?.recordID.zoneID if let newZoneID, zoneID != newZoneID { - reportIssue(""" - The record '\(recordName)' was moved from zone \ - '\(zoneID.zoneName)/\(zoneID.ownerName)' to \ - '\(newZoneID.zoneName)/\(newZoneID.ownerName)'. This is currently not supported in \ - SQLiteData. To work around, delete the record and then create a new record with its \ - new parent association. - """) + struct ZoneChangingError: Error, LocalizedError { + let recordName: String + let zoneID: CKRecordZone.ID + let newZoneID: CKRecordZone.ID + var errorDescription: String? { + """ + The record '\(recordName)' was moved from zone \ + '\(zoneID.zoneName)/\(zoneID.ownerName)' to \ + '\(newZoneID.zoneName)/\(newZoneID.ownerName)'. This is currently not supported in \ + SQLiteData. To work around, delete the record and then create a new record with its \ + new parent association. + """ + } + } + throw ZoneChangingError(recordName: recordName, zoneID: zoneID, newZoneID: newZoneID) } let change = CKSyncEngine.PendingRecordZoneChange.saveRecord( diff --git a/Tests/SQLiteDataTests/CloudKitTests/SharingTests.swift b/Tests/SQLiteDataTests/CloudKitTests/SharingTests.swift index 30bf82b4..00629259 100644 --- a/Tests/SQLiteDataTests/CloudKitTests/SharingTests.swift +++ b/Tests/SQLiteDataTests/CloudKitTests/SharingTests.swift @@ -1048,22 +1048,26 @@ ) ) - try await withKnownIssue { - try await userDatabase.userWrite { db in + let error = await #expect(throws: DatabaseError.self) { + try await self.userDatabase.userWrite { db in try ModelB.find(1).update { $0.modelAID = 2 }.execute(db) } - } matching: { issue in - issue.description.hasSuffix(""" - The record '1:modelBs' was moved from zone 'zone/__defaultOwner__' to \ - 'external.zone/external.owner'. This is currently not supported in \ - SQLiteData. To work around, delete the record and then create a new record with its \ - new parent association. - """) } - try await syncEngine.processPendingRecordZoneChanges(scope: .private) - try await syncEngine.processPendingRecordZoneChanges(scope: .private) + #expect(error?.message == """ + The record '1:modelBs' was moved from zone 'zone/__defaultOwner__' to \ + 'external.zone/external.owner'. This is currently not supported in SQLiteData. To work \ + around, delete the record and then create a new record with its new parent association. + """) + assertQuery(ModelB.all, database: userDatabase.database) { """ + ┌───────────────┐ + │ ModelB( │ + │ id: 1, │ + │ isOn: true, │ + │ modelAID: 1 │ + │ ) │ + └───────────────┘ """ } assertQuery(SyncMetadata.all, database: syncEngine.metadatabase) { @@ -1098,6 +1102,35 @@ │ ) │ ├──────────────────────────────────────────────────────────────────────────────────────────────┤ │ SyncMetadata( │ + │ recordPrimaryKey: "1", │ + │ recordType: "modelBs", │ + │ recordName: "1:modelBs", │ + │ parentRecordPrimaryKey: "1", │ + │ parentRecordType: "modelAs", │ + │ parentRecordName: "1:modelAs", │ + │ lastKnownServerRecord: CKRecord( │ + │ recordID: CKRecord.ID(1:modelBs/zone/__defaultOwner__), │ + │ recordType: "modelBs", │ + │ parent: CKReference(recordID: CKRecord.ID(1:modelAs/zone/__defaultOwner__)), │ + │ share: nil │ + │ ), │ + │ _lastKnownServerRecordAllFields: CKRecord( │ + │ recordID: CKRecord.ID(1:modelBs/zone/__defaultOwner__), │ + │ recordType: "modelBs", │ + │ parent: CKReference(recordID: CKRecord.ID(1:modelAs/zone/__defaultOwner__)), │ + │ share: nil, │ + │ id: 1, │ + │ isOn: 1, │ + │ modelAID: 1 │ + │ ), │ + │ share: nil, │ + │ _isDeleted: false, │ + │ hasLastKnownServerRecord: true, │ + │ isShared: false, │ + │ userModificationDate: Date(1970-01-01T00:00:00.000Z) │ + │ ) │ + ├──────────────────────────────────────────────────────────────────────────────────────────────┤ + │ SyncMetadata( │ │ recordPrimaryKey: "2", │ │ recordType: "modelAs", │ │ recordName: "2:modelAs", │ @@ -1145,6 +1178,15 @@ share: nil, count: 42, id: 1 + ), + [1]: CKRecord( + recordID: CKRecord.ID(1:modelBs/zone/__defaultOwner__), + recordType: "modelBs", + parent: CKReference(recordID: CKRecord.ID(1:modelAs/zone/__defaultOwner__)), + share: nil, + id: 1, + isOn: 1, + modelAID: 1 ) ] ),