Skip to content

Commit d2dacad

Browse files
mbrandonwmbrandonw
authored
Fixes with sharing records (#259)
* A more correct way to create shares. * better fix * wip * wip * wip * fix * wip * test for share twice * added test to show problem
1 parent 0c3d0ce commit d2dacad

3 files changed

Lines changed: 199 additions & 82 deletions

File tree

‎Sources/SQLiteData/CloudKit/CloudKitSharing.swift‎

Lines changed: 42 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
extension SyncEngine {
3232
private struct SharingError: LocalizedError {
3333
enum Reason {
34+
case shareCouldNotBeCreated
3435
case recordMetadataNotFound
3536
case recordNotRoot([ForeignKey])
3637
case recordTableNotSynchronized
@@ -145,8 +146,16 @@
145146

146147
var existingShare: CKShare? {
147148
get async throws {
148-
guard let shareRecordID = rootRecord.share?.recordID
149-
else { return nil }
149+
let share = try await metadatabase.read { db in
150+
try SyncMetadata
151+
.find(rootRecord.recordID)
152+
.select(\.share)
153+
.fetchOne(db) ?? nil
154+
}
155+
guard let shareRecordID = share?.recordID
156+
else {
157+
return nil
158+
}
150159
do {
151160
return try await container.database(for: rootRecord.recordID)
152161
.record(for: shareRecordID) as? CKShare
@@ -167,18 +176,43 @@
167176
)
168177

169178
configure(sharedRecord)
170-
_ = try await container.privateCloudDatabase.modifyRecords(
179+
let (saveResults, _) = try await container.privateCloudDatabase.modifyRecords(
171180
saving: [sharedRecord, rootRecord],
172181
deleting: []
173182
)
174-
try await userDatabase.write { db in
183+
184+
let savedShare = try saveResults.values.compactMap { result in
185+
let record = try result.get()
186+
return record.recordID == sharedRecord.recordID ? record as? CKShare : nil
187+
}
188+
.first
189+
let savedRootRecord = try saveResults.values.compactMap { result in
190+
let record = try result.get()
191+
return record.recordID == rootRecord.recordID ? record : nil
192+
}
193+
.first
194+
guard let savedShare, let savedRootRecord
195+
else {
196+
throw SharingError(
197+
recordTableName: T.tableName,
198+
recordPrimaryKey: record.primaryKey.rawIdentifier,
199+
reason: .shareCouldNotBeCreated,
200+
debugDescription: """
201+
A 'CKShare' could not be created in iCloud.
202+
"""
203+
)
204+
}
205+
try await metadatabase.write { db in
175206
try SyncMetadata
176207
.where { $0.recordName.eq(recordName) }
177-
.update { $0.share = sharedRecord }
208+
.update {
209+
$0.setLastKnownServerRecord(savedRootRecord)
210+
$0.share = savedShare
211+
}
178212
.execute(db)
179213
}
180214

181-
return SharedRecord(container: container, share: sharedRecord)
215+
return SharedRecord(container: container, share: savedShare)
182216
}
183217

184218
public func unshare<T: PrimaryKeyedTable>(record: T) async throws
@@ -195,7 +229,8 @@
195229
reportIssue(
196230
"""
197231
No share found associated with record.
198-
""")
232+
"""
233+
)
199234
return
200235
}
201236

‎Sources/SQLiteData/CloudKit/SyncEngine.swift‎

Lines changed: 98 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -358,7 +358,7 @@
358358
public func start() async throws {
359359
try await start().value
360360
}
361-
361+
362362
/// Determines if the sync engine is currently sending local changes to the CloudKit server.
363363
///
364364
/// It is an observable value, which means if it is accessed in a SwiftUI view, or some other
@@ -872,19 +872,31 @@
872872
)
873873

874874
case .willFetchRecordZoneChanges:
875-
fetchingChangesCount += 1
875+
await MainActor.run {
876+
fetchingChangesCount += 1
877+
}
876878
case .didFetchRecordZoneChanges:
877-
fetchingChangesCount -= 1
879+
await MainActor.run {
880+
fetchingChangesCount -= 1
881+
}
878882

879883
case .willFetchChanges:
880-
fetchingChangesCount += 1
884+
await MainActor.run {
885+
fetchingChangesCount += 1
886+
}
881887
case .didFetchChanges:
882-
fetchingChangesCount -= 1
888+
await MainActor.run {
889+
fetchingChangesCount -= 1
890+
}
883891

884892
case .willSendChanges:
885-
sendingChangesCount += 1
893+
await MainActor.run {
894+
sendingChangesCount += 1
895+
}
886896
case .didSendChanges:
887-
sendingChangesCount -= 1
897+
await MainActor.run {
898+
sendingChangesCount -= 1
899+
}
888900

889901
@unknown default:
890902
break
@@ -1076,74 +1088,76 @@
10761088
}
10771089
}
10781090

1079-
let (sharesToDelete, recordsWithRoot):
1080-
([CKShare?], [(lastKnownServerRecord: CKRecord?, rootLastKnownServerRecord: CKRecord?)]) =
1081-
await withErrorReporting(.sqliteDataCloudKitFailure) {
1082-
guard !deletedRecordIDs.isEmpty
1083-
else { return ([], []) }
1084-
1085-
return try await metadatabase.read { db in
1086-
let sharesToDelete =
1087-
try SyncMetadata
1088-
.findAll(deletedRecordIDs)
1089-
.where(\.isShared)
1090-
.select(\.share)
1091-
.fetchAll(db)
1091+
if syncEngine.database.databaseScope == .shared {
1092+
let (sharesToDelete, recordsWithRoot):
1093+
([CKShare?], [(lastKnownServerRecord: CKRecord?, rootLastKnownServerRecord: CKRecord?)]) =
1094+
await withErrorReporting(.sqliteDataCloudKitFailure) {
1095+
guard !deletedRecordIDs.isEmpty
1096+
else { return ([], []) }
1097+
1098+
return try await metadatabase.read { db in
1099+
let sharesToDelete =
1100+
try SyncMetadata
1101+
.findAll(deletedRecordIDs)
1102+
.where(\.isShared)
1103+
.select(\.share)
1104+
.fetchAll(db)
10921105

1093-
let recordsWithRoot =
1094-
try With {
1095-
SyncMetadata
1096-
.findAll(deletedRecordIDs)
1097-
.where { $0.parentRecordName.is(nil) }
1098-
.select {
1099-
RecordWithRoot.Columns(
1100-
parentRecordName: $0.parentRecordName,
1101-
recordName: $0.recordName,
1102-
lastKnownServerRecord: $0.lastKnownServerRecord,
1103-
rootRecordName: $0.recordName,
1104-
rootLastKnownServerRecord: $0.lastKnownServerRecord
1106+
let recordsWithRoot =
1107+
try With {
1108+
SyncMetadata
1109+
.findAll(deletedRecordIDs)
1110+
.where { $0.parentRecordName.is(nil) }
1111+
.select {
1112+
RecordWithRoot.Columns(
1113+
parentRecordName: $0.parentRecordName,
1114+
recordName: $0.recordName,
1115+
lastKnownServerRecord: $0.lastKnownServerRecord,
1116+
rootRecordName: $0.recordName,
1117+
rootLastKnownServerRecord: $0.lastKnownServerRecord
1118+
)
1119+
}
1120+
.union(
1121+
all: true,
1122+
SyncMetadata
1123+
.join(RecordWithRoot.all) { $1.recordName.is($0.parentRecordName) }
1124+
.select { metadata, tree in
1125+
RecordWithRoot.Columns(
1126+
parentRecordName: metadata.parentRecordName,
1127+
recordName: metadata.recordName,
1128+
lastKnownServerRecord: metadata.lastKnownServerRecord,
1129+
rootRecordName: tree.rootRecordName,
1130+
rootLastKnownServerRecord: tree.lastKnownServerRecord
1131+
)
1132+
}
11051133
)
1106-
}
1107-
.union(
1108-
all: true,
1109-
SyncMetadata
1110-
.join(RecordWithRoot.all) { $1.recordName.is($0.parentRecordName) }
1111-
.select { metadata, tree in
1112-
RecordWithRoot.Columns(
1113-
parentRecordName: metadata.parentRecordName,
1114-
recordName: metadata.recordName,
1115-
lastKnownServerRecord: metadata.lastKnownServerRecord,
1116-
rootRecordName: tree.rootRecordName,
1117-
rootLastKnownServerRecord: tree.lastKnownServerRecord
1118-
)
1119-
}
1120-
)
1121-
} query: {
1122-
RecordWithRoot
1123-
.select { ($0.lastKnownServerRecord, $0.rootLastKnownServerRecord) }
1124-
}
1125-
.fetchAll(db)
1134+
} query: {
1135+
RecordWithRoot
1136+
.select { ($0.lastKnownServerRecord, $0.rootLastKnownServerRecord) }
1137+
}
1138+
.fetchAll(db)
11261139

1127-
return (sharesToDelete, recordsWithRoot)
1140+
return (sharesToDelete, recordsWithRoot)
1141+
}
11281142
}
1129-
}
1130-
?? ([], [])
1143+
?? ([], [])
11311144

1132-
let shareRecordIDsToDelete = sharesToDelete.compactMap(\.?.recordID)
1145+
let shareRecordIDsToDelete = sharesToDelete.compactMap(\.?.recordID)
11331146

1134-
for recordWithRoot in recordsWithRoot {
1135-
guard
1136-
let lastKnownServerRecord = recordWithRoot.lastKnownServerRecord,
1137-
let rootLastKnownServerRecord = recordWithRoot.rootLastKnownServerRecord
1138-
else { continue }
1139-
guard let rootShareRecordID = rootLastKnownServerRecord.share?.recordID
1140-
else { continue }
1141-
guard shareRecordIDsToDelete.contains(rootShareRecordID)
1142-
else { continue }
1143-
changes.removeAll(where: { $0 == .deleteRecord(lastKnownServerRecord.recordID) })
1144-
syncEngine.state.remove(
1145-
pendingRecordZoneChanges: [.deleteRecord(lastKnownServerRecord.recordID)]
1146-
)
1147+
for recordWithRoot in recordsWithRoot {
1148+
guard
1149+
let lastKnownServerRecord = recordWithRoot.lastKnownServerRecord,
1150+
let rootLastKnownServerRecord = recordWithRoot.rootLastKnownServerRecord
1151+
else { continue }
1152+
guard let rootShareRecordID = rootLastKnownServerRecord.share?.recordID
1153+
else { continue }
1154+
guard shareRecordIDsToDelete.contains(rootShareRecordID)
1155+
else { continue }
1156+
changes.removeAll(where: { $0 == .deleteRecord(lastKnownServerRecord.recordID) })
1157+
syncEngine.state.remove(
1158+
pendingRecordZoneChanges: [.deleteRecord(lastKnownServerRecord.recordID)]
1159+
)
1160+
}
11471161
}
11481162

11491163
await withErrorReporting(.sqliteDataCloudKitFailure) {
@@ -1618,23 +1632,32 @@
16181632
}
16191633

16201634
func deleteShare(shareRecordID: CKRecord.ID) async throws {
1621-
try await userDatabase.write { db in
1622-
let shareAndRecordNameAndZone =
1623-
try SyncMetadata
1635+
let shareAndRecordNameAndZone = try await userDatabase.read { db in
1636+
try SyncMetadata
16241637
.where(\.isShared)
16251638
.select { ($0.share, $0.recordName, $0.zoneName, $0.ownerName) }
16261639
.fetchAll(db)
16271640
.first(where: { share, _, _, _ in share?.recordID == shareRecordID }) ?? nil
1628-
guard let (_, recordName, zoneName, ownerName) = shareAndRecordNameAndZone
1629-
else { return }
1641+
}
1642+
guard let (_, recordName, zoneName, ownerName) = shareAndRecordNameAndZone
1643+
else { return }
1644+
let rootRecordID = CKRecord.ID(
1645+
recordName: recordName,
1646+
zoneID: CKRecordZone.ID(zoneName: zoneName, ownerName: ownerName)
1647+
)
1648+
let rootRecord = try await container.privateCloudDatabase.record(for: rootRecordID)
1649+
try await userDatabase.write { db in
16301650
try SyncMetadata
16311651
.find(
16321652
CKRecord.ID(
16331653
recordName: recordName,
16341654
zoneID: CKRecordZone.ID(zoneName: zoneName, ownerName: ownerName)
16351655
)
16361656
)
1637-
.update { $0.share = nil }
1657+
.update {
1658+
$0.setLastKnownServerRecord(rootRecord)
1659+
$0.share = nil
1660+
}
16381661
.execute(db)
16391662
}
16401663
}

‎Tests/SQLiteDataTests/CloudKitTests/SharingTests.swift‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -644,6 +644,65 @@
644644
}
645645
}
646646

647+
@available(iOS 17, macOS 14, tvOS 17, watchOS 10, *)
648+
@Test func shareTwice() async throws {
649+
let remindersList = RemindersList(id: 1, title: "Personal")
650+
try await userDatabase.userWrite { db in
651+
try db.seed {
652+
remindersList
653+
}
654+
}
655+
try await syncEngine.processPendingRecordZoneChanges(scope: .private)
656+
657+
let _ = try await syncEngine.share(record: remindersList, configure: {
658+
$0[CKShare.SystemFieldKey.title] = "Join my list!"
659+
})
660+
let _ = try await syncEngine.share(record: remindersList, configure: {
661+
$0[CKShare.SystemFieldKey.title] = "Please join my list!"
662+
})
663+
664+
assertQuery(SyncMetadata.select(\.share), database: syncEngine.metadatabase) {
665+
"""
666+
┌────────────────────────────────────────────────────────────────────────┐
667+
│ CKRecord( │
668+
│ recordID: CKRecord.ID(share-1:remindersLists/zone/__defaultOwner__), │
669+
│ recordType: "cloudkit.share", │
670+
│ parent: nil, │
671+
│ share: nil │
672+
│ ) │
673+
└────────────────────────────────────────────────────────────────────────┘
674+
"""
675+
}
676+
assertInlineSnapshot(of: container, as: .customDump) {
677+
"""
678+
MockCloudContainer(
679+
privateCloudDatabase: MockCloudDatabase(
680+
databaseScope: .private,
681+
storage: [
682+
[0]: CKRecord(
683+
recordID: CKRecord.ID(share-1:remindersLists/zone/__defaultOwner__),
684+
recordType: "cloudkit.share",
685+
parent: nil,
686+
share: nil,
687+
cloudkit.title: "Please join my list!"
688+
),
689+
[1]: CKRecord(
690+
recordID: CKRecord.ID(1:remindersLists/zone/__defaultOwner__),
691+
recordType: "remindersLists",
692+
parent: nil,
693+
share: CKReference(recordID: CKRecord.ID(share-1:remindersLists/zone/__defaultOwner__))
694+
)
695+
]
696+
),
697+
sharedCloudDatabase: MockCloudDatabase(
698+
databaseScope: .shared,
699+
storage: []
700+
)
701+
)
702+
"""
703+
}
704+
}
705+
647706
// NB: Swift 6.2 cannot currently compile this:
648707
// Pattern that the region based isolation checker does not understand how to check.
649708
// Please file a bug.

0 commit comments

Comments
 (0)