mirror of
https://github.com/yattee/yattee.git
synced 2026-10-23 13:12:53 +00:00
Fix a crash when syncing watch entries whose video ID exceeds the CloudKit record name limit
Extracted media-source video IDs are full file paths, so closing a long-named WebDAV video built a watch record name longer than 255 characters and CKRecord.ID threw a fatal CKException. Scoped record names now fall back to a SHA-256 digest of the ID when the verbatim form would exceed the limit. The sync engine's local lookups for materializing pending saves and applying remote deletions recognise the digest form and match it against all entities of that type instead of the indexed ID lookup. Short IDs keep their existing names, so records already in iCloud are unaffected.
This commit is contained in:
@@ -130,6 +130,12 @@ extension DataManager {
|
||||
return (try? modelContext.fetch(descriptor)) ?? []
|
||||
}
|
||||
|
||||
/// Gets every bookmark, for lookups that cannot go through the
|
||||
/// video-ID index (CloudKit record names that carry a digest of the ID).
|
||||
func allBookmarks() -> [Bookmark] {
|
||||
(try? modelContext.fetch(FetchDescriptor<Bookmark>())) ?? []
|
||||
}
|
||||
|
||||
/// Deletes a single bookmark without queueing a CloudKit deletion.
|
||||
/// Used by CloudKitSyncEngine when applying remote deletions.
|
||||
func deleteBookmark(_ bookmark: Bookmark) {
|
||||
|
||||
@@ -167,6 +167,12 @@ extension DataManager {
|
||||
return (try? modelContext.fetch(descriptor)) ?? []
|
||||
}
|
||||
|
||||
/// Gets every watch entry, for lookups that cannot go through the
|
||||
/// video-ID index (CloudKit record names that carry a digest of the ID).
|
||||
func allWatchEntries() -> [WatchEntry] {
|
||||
(try? modelContext.fetch(FetchDescriptor<WatchEntry>())) ?? []
|
||||
}
|
||||
|
||||
/// Deletes a single watch entry without queueing a CloudKit deletion.
|
||||
/// Used by CloudKitSyncEngine when applying remote deletions.
|
||||
func deleteWatchEntry(_ entry: WatchEntry) {
|
||||
|
||||
@@ -2092,38 +2092,51 @@ extension CloudKitSyncEngine: CKSyncEngineDelegate {
|
||||
/// Bare-ID lookups can return an entity from a different source scope when
|
||||
/// IDs collide across sources; these helpers match on the mapper-derived
|
||||
/// record name so the right entity is merged, materialized, or deleted.
|
||||
/// Entities whose ID corresponds to a bare ID parsed from a record name.
|
||||
/// A verbatim bare ID is an indexed lookup; a digest (an ID too long to
|
||||
/// embed in the record name) has to be matched against every entity.
|
||||
private func candidates<Entity>(
|
||||
for bareID: String,
|
||||
exact: (String) -> [Entity],
|
||||
all: () -> [Entity],
|
||||
id: (Entity) -> String
|
||||
) -> [Entity] {
|
||||
guard SyncableRecordType.isHashedBareID(bareID) else { return exact(bareID) }
|
||||
return all().filter { SyncableRecordType.entityID(id($0), matchesBareID: bareID) }
|
||||
}
|
||||
|
||||
private func localSubscription(matching recordName: String, channelID: String, in dataManager: DataManager) -> Subscription? {
|
||||
dataManager.subscriptions(forChannelID: channelID).first {
|
||||
candidates(for: channelID, exact: dataManager.subscriptions(forChannelID:), all: dataManager.subscriptions, id: \.channelID).first {
|
||||
recordMapper.toCKRecord(subscription: $0).recordID.recordName == recordName
|
||||
}
|
||||
}
|
||||
|
||||
private func localWatchEntry(matching recordName: String, videoID: String, in dataManager: DataManager) -> WatchEntry? {
|
||||
dataManager.watchEntries(forVideoID: videoID).first {
|
||||
candidates(for: videoID, exact: dataManager.watchEntries(forVideoID:), all: dataManager.allWatchEntries, id: \.videoID).first {
|
||||
recordMapper.toCKRecord(watchEntry: $0).recordID.recordName == recordName
|
||||
}
|
||||
}
|
||||
|
||||
private func localBookmark(matching recordName: String, videoID: String, in dataManager: DataManager) -> Bookmark? {
|
||||
dataManager.bookmarks(forVideoID: videoID).first {
|
||||
candidates(for: videoID, exact: dataManager.bookmarks(forVideoID:), all: dataManager.allBookmarks, id: \.videoID).first {
|
||||
recordMapper.toCKRecord(bookmark: $0).recordID.recordName == recordName
|
||||
}
|
||||
}
|
||||
|
||||
private func localRecentChannel(matching recordName: String, channelID: String, in dataManager: DataManager) -> RecentChannel? {
|
||||
dataManager.recentChannelEntries(forChannelID: channelID).first {
|
||||
candidates(for: channelID, exact: dataManager.recentChannelEntries(forChannelID:), all: dataManager.allRecentChannels, id: \.channelID).first {
|
||||
recordMapper.toCKRecord(recentChannel: $0).recordID.recordName == recordName
|
||||
}
|
||||
}
|
||||
|
||||
private func localRecentPlaylist(matching recordName: String, playlistID: String, in dataManager: DataManager) -> RecentPlaylist? {
|
||||
dataManager.recentPlaylistEntries(forPlaylistID: playlistID).first {
|
||||
candidates(for: playlistID, exact: dataManager.recentPlaylistEntries(forPlaylistID:), all: dataManager.allRecentPlaylists, id: \.playlistID).first {
|
||||
recordMapper.toCKRecord(recentPlaylist: $0).recordID.recordName == recordName
|
||||
}
|
||||
}
|
||||
|
||||
private func localChannelNotificationSettings(matching recordName: String, channelID: String, in dataManager: DataManager) -> ChannelNotificationSettings? {
|
||||
dataManager.allChannelNotificationSettings(forChannelID: channelID).first {
|
||||
candidates(for: channelID, exact: dataManager.allChannelNotificationSettings(forChannelID:), all: dataManager.allChannelNotificationSettings, id: \.channelID).first {
|
||||
recordMapper.toCKRecord(channelNotificationSettings: $0).recordID.recordName == recordName
|
||||
}
|
||||
}
|
||||
|
||||
@@ -6,6 +6,7 @@
|
||||
//
|
||||
|
||||
import CloudKit
|
||||
import CryptoKit
|
||||
import Foundation
|
||||
|
||||
/// CloudKit record type names and constants.
|
||||
@@ -131,6 +132,43 @@ enum SyncableRecordType: Sendable {
|
||||
}
|
||||
}
|
||||
|
||||
/// CloudKit rejects record names longer than this with a fatal
|
||||
/// `CKException` at `CKRecord.ID` construction time.
|
||||
static let maxRecordNameLength = 255
|
||||
|
||||
/// Marks a bare-ID component that is a digest of the real ID rather than
|
||||
/// the ID itself. Extracted-source video IDs can be full file paths, which
|
||||
/// blow past the record name limit when embedded verbatim.
|
||||
static let hashedIDPrefix = "sha256-"
|
||||
|
||||
/// The bare-ID component used in place of `id` when the verbatim name
|
||||
/// would be too long. Deterministic, so every device derives the same
|
||||
/// record name for the same entity.
|
||||
static func hashedBareID(for id: String) -> String {
|
||||
let digest = SHA256.hash(data: Data(id.utf8))
|
||||
return hashedIDPrefix + digest.map { String(format: "%02x", $0) }.joined()
|
||||
}
|
||||
|
||||
/// Whether a bare ID parsed out of a record name is a digest, in which
|
||||
/// case the real ID must be recovered by matching local entities.
|
||||
static func isHashedBareID(_ bareID: String) -> Bool {
|
||||
bareID.hasPrefix(hashedIDPrefix) && bareID.count == hashedIDPrefix.count + 64
|
||||
}
|
||||
|
||||
/// Whether an entity ID corresponds to the bare-ID component parsed from
|
||||
/// a record name, whichever form (verbatim or digest) that component took.
|
||||
static func entityID(_ id: String, matchesBareID bareID: String) -> Bool {
|
||||
isHashedBareID(bareID) ? hashedBareID(for: id) == bareID : id == bareID
|
||||
}
|
||||
|
||||
/// Builds a scoped record name, falling back to a digest of the ID when
|
||||
/// the verbatim form would exceed CloudKit's limit.
|
||||
private static func scopedRecordName(prefix: String, id: String, scope: SourceScope) -> String {
|
||||
let verbatim = "\(prefix)\(id)\(scope.recordNameSuffix)"
|
||||
guard verbatim.utf8.count > maxRecordNameLength else { return verbatim }
|
||||
return "\(prefix)\(hashedBareID(for: id))\(scope.recordNameSuffix)"
|
||||
}
|
||||
|
||||
/// Extracts the bare ID from a record name by stripping the scope suffix.
|
||||
static func extractBareID(from value: String) -> String {
|
||||
for prefix in ["@global:", "@federated:", "@extracted:"] {
|
||||
@@ -147,11 +185,11 @@ enum SyncableRecordType: Sendable {
|
||||
|
||||
switch self {
|
||||
case .subscription(let channelID, let scope):
|
||||
recordName = "sub-\(channelID)\(scope.recordNameSuffix)"
|
||||
recordName = Self.scopedRecordName(prefix: "sub-", id: channelID, scope: scope)
|
||||
case .watchEntry(let videoID, let scope):
|
||||
recordName = "watch-\(videoID)\(scope.recordNameSuffix)"
|
||||
recordName = Self.scopedRecordName(prefix: "watch-", id: videoID, scope: scope)
|
||||
case .bookmark(let videoID, let scope):
|
||||
recordName = "bookmark-\(videoID)\(scope.recordNameSuffix)"
|
||||
recordName = Self.scopedRecordName(prefix: "bookmark-", id: videoID, scope: scope)
|
||||
case .localPlaylist(let id):
|
||||
recordName = "playlist-\(id.uuidString)"
|
||||
case .localPlaylistItem(let id):
|
||||
@@ -159,9 +197,9 @@ enum SyncableRecordType: Sendable {
|
||||
case .searchHistory(let id):
|
||||
recordName = "search-\(id.uuidString)"
|
||||
case .recentChannel(let channelID, let scope):
|
||||
recordName = "recent-channel-\(channelID)\(scope.recordNameSuffix)"
|
||||
recordName = Self.scopedRecordName(prefix: "recent-channel-", id: channelID, scope: scope)
|
||||
case .recentPlaylist(let playlistID, let scope):
|
||||
recordName = "recent-playlist-\(playlistID)\(scope.recordNameSuffix)"
|
||||
recordName = Self.scopedRecordName(prefix: "recent-playlist-", id: playlistID, scope: scope)
|
||||
case .appSettings:
|
||||
recordName = "settings-singleton"
|
||||
case .instance(let id):
|
||||
@@ -169,7 +207,7 @@ enum SyncableRecordType: Sendable {
|
||||
case .mediaSource(let id):
|
||||
recordName = "source-\(id.uuidString)"
|
||||
case .channelNotificationSettings(let channelID, let scope):
|
||||
recordName = "channel-notif-\(channelID)\(scope.recordNameSuffix)"
|
||||
recordName = Self.scopedRecordName(prefix: "channel-notif-", id: channelID, scope: scope)
|
||||
case .controlsPreset(let id):
|
||||
recordName = "controls-\(id.uuidString)"
|
||||
case .account(let id):
|
||||
|
||||
@@ -217,4 +217,70 @@ struct CloudKitSyncTests {
|
||||
#expect(resolved["notificationsEnabled"] as? Int64 == 1)
|
||||
}
|
||||
}
|
||||
|
||||
// MARK: - Record Names
|
||||
|
||||
@Suite("Record Names")
|
||||
struct RecordNameTests {
|
||||
|
||||
private static let zone = CKRecordZone(zoneName: RecordType.zoneName)
|
||||
private static let extracted = SourceScope.from(
|
||||
sourceRawValue: "extracted", globalProvider: nil, instanceURLString: nil, externalExtractor: "media_source_webdav"
|
||||
)
|
||||
|
||||
/// A WebDAV media-source ID: the full path, far past CloudKit's limit.
|
||||
private static let longVideoID = String(repeating: "Ted Lasso S04E05 Riches of Embarrassment 1080p ATVP WEB-DL/", count: 6) + "episode.mkv"
|
||||
|
||||
@Test("Short IDs keep their verbatim record name")
|
||||
func shortIDVerbatim() {
|
||||
let name = SyncableRecordType.watchEntry(videoID: "abc123", scope: Self.extracted).recordID(in: Self.zone).recordName
|
||||
#expect(name == "watch-abc123@extracted:media_source_webdav")
|
||||
#expect(SyncableRecordType.isHashedBareID(SyncableRecordType.extractBareID(from: String(name.dropFirst(6)))) == false)
|
||||
}
|
||||
|
||||
@Test("Over-long IDs fall back to a digest that fits CloudKit's limit")
|
||||
func longIDHashed() {
|
||||
let verbatim = "watch-\(Self.longVideoID)\(Self.extracted.recordNameSuffix)"
|
||||
#expect(verbatim.utf8.count > SyncableRecordType.maxRecordNameLength)
|
||||
|
||||
let recordID = SyncableRecordType.watchEntry(videoID: Self.longVideoID, scope: Self.extracted).recordID(in: Self.zone)
|
||||
let name = recordID.recordName
|
||||
#expect(name.utf8.count <= SyncableRecordType.maxRecordNameLength)
|
||||
#expect(name.hasPrefix("watch-sha256-"))
|
||||
#expect(name.hasSuffix("@extracted:media_source_webdav"))
|
||||
|
||||
let bareID = SyncableRecordType.extractBareID(from: String(name.dropFirst(6)))
|
||||
#expect(SyncableRecordType.isHashedBareID(bareID))
|
||||
#expect(SyncableRecordType.entityID(Self.longVideoID, matchesBareID: bareID))
|
||||
#expect(SyncableRecordType.entityID(Self.longVideoID + "x", matchesBareID: bareID) == false)
|
||||
}
|
||||
|
||||
@Test("Digest names are stable and distinct per ID and scope")
|
||||
func digestStable() {
|
||||
let a = SyncableRecordType.bookmark(videoID: Self.longVideoID, scope: Self.extracted).recordID(in: Self.zone).recordName
|
||||
let b = SyncableRecordType.bookmark(videoID: Self.longVideoID, scope: Self.extracted).recordID(in: Self.zone).recordName
|
||||
let other = SyncableRecordType.bookmark(videoID: Self.longVideoID + "2", scope: Self.extracted).recordID(in: Self.zone).recordName
|
||||
#expect(a == b)
|
||||
#expect(a != other)
|
||||
}
|
||||
|
||||
@Test("The mapper round-trips a long-ID watch entry without throwing")
|
||||
@MainActor
|
||||
func mapperLongID() {
|
||||
let entry = WatchEntry(
|
||||
videoID: Self.longVideoID,
|
||||
sourceRawValue: "extracted",
|
||||
title: "Video",
|
||||
authorName: "Channel",
|
||||
authorID: "ch1",
|
||||
duration: 10,
|
||||
watchedSeconds: 5,
|
||||
isLive: false
|
||||
)
|
||||
entry.externalExtractor = "media_source_webdav"
|
||||
let record = CloudKitSyncTests.makeMapper().toCKRecord(watchEntry: entry)
|
||||
#expect(record.recordID.recordName.hasPrefix("watch-sha256-"))
|
||||
#expect(record["videoID"] as? String == Self.longVideoID)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user