fix: backup loops because isFilenameOnly gate disables filename fallback for mixed manifests
Root cause: ManifestIndex.isFilenameOnly is false as soon as any single entry has a localIdentifier. All filename-only entries (orphans uploaded in prior sessions without a saved localIdentifier) are never matched, so their assets appear pending on every reconcile and the backup re-starts from scratch on every app open. Fix: add unclaimedFilenames set — filenames of entries with localIdentifier: "". Use this as a targeted fallback in countSafe / pendingIDs / BackupEngine filter: an asset not matched by localIdentifier is still considered safe if its filename matches an unclaimed entry. This is safe because unclaimed entries are specifically files we know are on NAS but whose identity we can only track by name. Co-Authored-By: Kutesir <kutesir@provoc.ug> Co-Authored-By: Sentry <sentry@provoc.ug>
This commit is contained in:
@@ -7,31 +7,50 @@ import Photos
|
|||||||
// Sendable: all stored properties are value types — safe to pass across task boundaries.
|
// Sendable: all stored properties are value types — safe to pass across task boundaries.
|
||||||
struct ManifestIndex: Sendable {
|
struct ManifestIndex: Sendable {
|
||||||
let localIdentifiers: Set<String> // PHAsset.localIdentifier (primary key)
|
let localIdentifiers: Set<String> // PHAsset.localIdentifier (primary key)
|
||||||
let lowercasedFilenames: Set<String> // filename fallback for legacy/bootstrap entries
|
let lowercasedFilenames: Set<String> // all filenames — used for pure bootstrap mode
|
||||||
|
let unclaimedFilenames: Set<String> // filenames of entries with no localIdentifier
|
||||||
|
// (orphans / pre-ID backups) — filename fallback
|
||||||
let totalCount: Int // non-hidden file count for NAS Archive stat
|
let totalCount: Int // non-hidden file count for NAS Archive stat
|
||||||
|
|
||||||
|
/// True only when every entry lacks a localIdentifier (pure bootstrap from NAS listing).
|
||||||
var isFilenameOnly: Bool { localIdentifiers.isEmpty }
|
var isFilenameOnly: Bool { localIdentifiers.isEmpty }
|
||||||
|
|
||||||
func matches(localIdentifier id: String) -> Bool {
|
func matches(localIdentifier id: String) -> Bool {
|
||||||
!id.isEmpty && localIdentifiers.contains(id)
|
!id.isEmpty && localIdentifiers.contains(id)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Full filename match — only used in pure bootstrap mode.
|
||||||
func matches(filename: String) -> Bool {
|
func matches(filename: String) -> Bool {
|
||||||
lowercasedFilenames.contains(filename.lowercased())
|
lowercasedFilenames.contains(filename.lowercased())
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Filename match against orphan entries only.
|
||||||
|
/// Use as fallback when localIdentifier matching fails: if a NAS file was uploaded
|
||||||
|
/// in a session where the localIdentifier wasn't saved (interrupted / pre-ID backup),
|
||||||
|
/// matching by filename prevents re-uploading it.
|
||||||
|
func matchesUnclaimed(filename: String) -> Bool {
|
||||||
|
unclaimedFilenames.contains(filename.lowercased())
|
||||||
|
}
|
||||||
|
|
||||||
// Build from decoded manifest — manifest released by caller after this returns
|
// Build from decoded manifest — manifest released by caller after this returns
|
||||||
init(manifest: BackupManifest) {
|
init(manifest: BackupManifest) {
|
||||||
var ids = Set<String>(minimumCapacity: manifest.entries.count)
|
var ids = Set<String>(minimumCapacity: manifest.entries.count)
|
||||||
var names = Set<String>(minimumCapacity: manifest.entries.count)
|
var names = Set<String>(minimumCapacity: manifest.entries.count)
|
||||||
|
var unclaimed = Set<String>()
|
||||||
var count = 0
|
var count = 0
|
||||||
for entry in manifest.entries {
|
for entry in manifest.entries {
|
||||||
if !entry.filename.hasPrefix(".") { count += 1 }
|
if !entry.filename.hasPrefix(".") { count += 1 }
|
||||||
if !entry.localIdentifier.isEmpty { ids.insert(entry.localIdentifier) }
|
let lower = entry.filename.lowercased()
|
||||||
names.insert(entry.filename.lowercased())
|
names.insert(lower)
|
||||||
|
if !entry.localIdentifier.isEmpty {
|
||||||
|
ids.insert(entry.localIdentifier)
|
||||||
|
} else {
|
||||||
|
unclaimed.insert(lower)
|
||||||
|
}
|
||||||
}
|
}
|
||||||
self.localIdentifiers = ids
|
self.localIdentifiers = ids
|
||||||
self.lowercasedFilenames = names
|
self.lowercasedFilenames = names
|
||||||
|
self.unclaimedFilenames = unclaimed
|
||||||
self.totalCount = count
|
self.totalCount = count
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -45,6 +64,7 @@ struct ManifestIndex: Sendable {
|
|||||||
}
|
}
|
||||||
self.localIdentifiers = []
|
self.localIdentifiers = []
|
||||||
self.lowercasedFilenames = names
|
self.lowercasedFilenames = names
|
||||||
|
self.unclaimedFilenames = names // all filename-only in bootstrap mode
|
||||||
self.totalCount = count
|
self.totalCount = count
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -160,7 +160,8 @@ final class BackupEngine: ObservableObject {
|
|||||||
let pendingAssets: [PhotoAsset] = await Task.detached(priority: .userInitiated) {
|
let pendingAssets: [PhotoAsset] = await Task.detached(priority: .userInitiated) {
|
||||||
assets.filter { asset in
|
assets.filter { asset in
|
||||||
!currentIndex.matches(localIdentifier: asset.localIdentifier) &&
|
!currentIndex.matches(localIdentifier: asset.localIdentifier) &&
|
||||||
!(currentIndex.isFilenameOnly && currentIndex.matches(filename: asset.filename))
|
!(currentIndex.isFilenameOnly && currentIndex.matches(filename: asset.filename)) &&
|
||||||
|
!currentIndex.matchesUnclaimed(filename: asset.filename)
|
||||||
}
|
}
|
||||||
}.value
|
}.value
|
||||||
signposter.endInterval("BuildPendingQueue", spFilter, "\(pendingAssets.count) pending")
|
signposter.endInterval("BuildPendingQueue", spFilter, "\(pendingAssets.count) pending")
|
||||||
|
|||||||
@@ -282,11 +282,14 @@ final class BackupStatusService: NSObject, ObservableObject {
|
|||||||
let asset = result.object(at: i)
|
let asset = result.object(at: i)
|
||||||
if index.matches(localIdentifier: asset.localIdentifier) {
|
if index.matches(localIdentifier: asset.localIdentifier) {
|
||||||
count += 1
|
count += 1
|
||||||
} else if index.isFilenameOnly {
|
} else {
|
||||||
let resources = PHAssetResource.assetResources(for: asset)
|
let resources = PHAssetResource.assetResources(for: asset)
|
||||||
if let name = resources.first?.originalFilename,
|
if let name = resources.first?.originalFilename {
|
||||||
index.matches(filename: name) {
|
if index.isFilenameOnly && index.matches(filename: name) {
|
||||||
count += 1
|
count += 1
|
||||||
|
} else if index.matchesUnclaimed(filename: name) {
|
||||||
|
count += 1
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -59,6 +59,11 @@ actor LocalPhotoIndex {
|
|||||||
if index.matches(localIdentifier: record.localIdentifier) {
|
if index.matches(localIdentifier: record.localIdentifier) {
|
||||||
safe += 1
|
safe += 1
|
||||||
} else if index.isFilenameOnly && index.matches(filename: record.filename) {
|
} else if index.isFilenameOnly && index.matches(filename: record.filename) {
|
||||||
|
// Pure bootstrap mode: every entry is filename-only, match by name.
|
||||||
|
safe += 1
|
||||||
|
} else if index.matchesUnclaimed(filename: record.filename) {
|
||||||
|
// Mixed mode: this specific entry was uploaded without a localIdentifier
|
||||||
|
// (interrupted session / pre-ID backup). Filename match prevents re-upload.
|
||||||
safe += 1
|
safe += 1
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -71,7 +76,8 @@ actor LocalPhotoIndex {
|
|||||||
guard passes(record, filter: filter) else { return nil }
|
guard passes(record, filter: filter) else { return nil }
|
||||||
let alreadyBacked =
|
let alreadyBacked =
|
||||||
index.matches(localIdentifier: record.localIdentifier) ||
|
index.matches(localIdentifier: record.localIdentifier) ||
|
||||||
(index.isFilenameOnly && index.matches(filename: record.filename))
|
(index.isFilenameOnly && index.matches(filename: record.filename)) ||
|
||||||
|
index.matchesUnclaimed(filename: record.filename)
|
||||||
return alreadyBacked ? nil : record.localIdentifier
|
return alreadyBacked ? nil : record.localIdentifier
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user