Fix Already Safe = 0: hybrid ManifestIndex preserves localIdentifiers + NAS filenames
The root cause: buildManifestIndex was discarding manifest localIdentifiers and switching to pure filename-only matching whenever dirCount > manifest.totalCount (which is always true when the NAS has files from other tools). If filenames didn't match exactly, Already Safe dropped to 0 on every stale-cache reconcile. Fix: introduce ManifestIndex(manifest:nasFilenames:totalCount:) — a hybrid that keeps Kisani's localIdentifiers as primary keys AND adds all NAS directory filenames for filename-fallback. Used in: - buildManifestIndex (full reconcile) - fast path (when dirCount > cached.entries.count, previously gated at <= 1) - BackupEngine step 3b (so "Back up again" only uploads genuinely missing files) Also simplify countSafe / pendingIDs / step-4 filter to always try filename fallback (removes the isFilenameOnly gate that blocked matches when the index had IDs). Co-Authored-By: Kutesir <kutesir@provoc.ug> Co-Authored-By: Sentry <sentry@provoc.ug>
This commit is contained in:
@@ -85,6 +85,29 @@ struct ManifestIndex: Sendable {
|
|||||||
self.totalCount = names.count
|
self.totalCount = names.count
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Hybrid: manifest localIdentifiers (Kisani primary key) + all NAS filenames
|
||||||
|
// for filename-fallback matching when the NAS directory has more files than the
|
||||||
|
// manifest tracks (uploaded by other tools). totalCount = actual NAS file count.
|
||||||
|
init(manifest: BackupManifest, nasFilenames: [String], totalCount: Int) {
|
||||||
|
var ids = Set<String>(minimumCapacity: manifest.entries.count)
|
||||||
|
var names = Set<String>(minimumCapacity: manifest.entries.count + nasFilenames.count)
|
||||||
|
var unclaimed = Set<String>()
|
||||||
|
for entry in manifest.entries {
|
||||||
|
let lower = entry.filename.lowercased()
|
||||||
|
names.insert(lower)
|
||||||
|
if !entry.localIdentifier.isEmpty {
|
||||||
|
ids.insert(entry.localIdentifier)
|
||||||
|
} else {
|
||||||
|
unclaimed.insert(lower)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
for name in nasFilenames { names.insert(name.lowercased()) }
|
||||||
|
self.localIdentifiers = ids
|
||||||
|
self.lowercasedFilenames = names
|
||||||
|
self.unclaimedFilenames = unclaimed
|
||||||
|
self.totalCount = totalCount
|
||||||
|
}
|
||||||
|
|
||||||
// Build from NAS directory listing (bootstrap — no localIdentifiers known)
|
// Build from NAS directory listing (bootstrap — no localIdentifiers known)
|
||||||
init(nasListing items: [NASItem]) {
|
init(nasListing items: [NASItem]) {
|
||||||
var names = Set<String>()
|
var names = Set<String>()
|
||||||
|
|||||||
@@ -135,17 +135,19 @@ final class BackupEngine: ObservableObject {
|
|||||||
// uploaded by other tools), a manifest-only index will misidentify those files
|
// uploaded by other tools), a manifest-only index will misidentify those files
|
||||||
// as "not backed up" and re-upload them on every "Back up again" tap.
|
// as "not backed up" and re-upload them on every "Back up again" tap.
|
||||||
// Use the broader NAS directory listing as the filter when the manifest is sparse.
|
// Use the broader NAS directory listing as the filter when the manifest is sparse.
|
||||||
|
// Build a hybrid filter index: manifest localIdentifiers (primary) + all NAS
|
||||||
|
// filenames (fallback). This ensures files uploaded by other tools are recognised
|
||||||
|
// as "already on NAS" by filename and are not re-uploaded on every backup run.
|
||||||
let filterIndex: ManifestIndex
|
let filterIndex: ManifestIndex
|
||||||
let cachedDirCount = await NASManifestCache.shared.directoryCount ?? 0
|
let cachedDirCount = await NASManifestCache.shared.directoryCount ?? 0
|
||||||
if baseIndex.totalCount < cachedDirCount {
|
if baseIndex.totalCount < cachedDirCount {
|
||||||
if let cachedFilenames = await NASManifestCache.shared.directoryFilenames,
|
if let cachedFilenames = await NASManifestCache.shared.directoryFilenames,
|
||||||
cachedFilenames.count > baseIndex.totalCount {
|
cachedFilenames.count > baseIndex.totalCount {
|
||||||
filterIndex = await Task.detached(priority: .utility) {
|
filterIndex = await Task.detached(priority: .utility) {
|
||||||
ManifestIndex(nasFilenames: cachedFilenames)
|
ManifestIndex(manifest: baseManifest, nasFilenames: cachedFilenames, totalCount: cachedDirCount)
|
||||||
}.value
|
}.value
|
||||||
logger.info("Filter index: \(cachedFilenames.count) cached NAS filenames (manifest=\(baseIndex.totalCount))")
|
logger.info("Filter index: hybrid — \(baseIndex.totalCount) manifest + \(cachedFilenames.count) cached NAS filenames")
|
||||||
} else {
|
} else {
|
||||||
// No cached filenames — do a fresh listing (connection is already open)
|
|
||||||
let nasItems = (try? await transfer.listDirectory(at: connection.remotePath)) ?? []
|
let nasItems = (try? await transfer.listDirectory(at: connection.remotePath)) ?? []
|
||||||
let names = nasItems.compactMap { item -> String? in
|
let names = nasItems.compactMap { item -> String? in
|
||||||
guard !item.isDirectory, !item.name.hasPrefix("."),
|
guard !item.isDirectory, !item.name.hasPrefix("."),
|
||||||
@@ -154,9 +156,9 @@ final class BackupEngine: ObservableObject {
|
|||||||
}
|
}
|
||||||
await NASManifestCache.shared.setDirectoryFilenames(names)
|
await NASManifestCache.shared.setDirectoryFilenames(names)
|
||||||
filterIndex = await Task.detached(priority: .utility) {
|
filterIndex = await Task.detached(priority: .utility) {
|
||||||
ManifestIndex(nasFilenames: names)
|
ManifestIndex(manifest: baseManifest, nasFilenames: names, totalCount: names.count)
|
||||||
}.value
|
}.value
|
||||||
logger.info("Filter index: \(names.count) fresh NAS filenames (manifest=\(baseIndex.totalCount))")
|
logger.info("Filter index: hybrid — \(baseIndex.totalCount) manifest + \(names.count) fresh NAS filenames")
|
||||||
}
|
}
|
||||||
} else {
|
} else {
|
||||||
filterIndex = baseIndex
|
filterIndex = baseIndex
|
||||||
@@ -169,8 +171,7 @@ 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
|
||||||
!filterIndex.matches(localIdentifier: asset.localIdentifier) &&
|
!filterIndex.matches(localIdentifier: asset.localIdentifier) &&
|
||||||
!(filterIndex.isFilenameOnly && filterIndex.matches(filename: asset.filename)) &&
|
!filterIndex.matches(filename: asset.filename)
|
||||||
!filterIndex.matchesUnclaimed(filename: asset.filename)
|
|
||||||
}
|
}
|
||||||
}.value
|
}.value
|
||||||
signposter.endInterval("BuildPendingQueue", spFilter, "\(pendingAssets.count) pending")
|
signposter.endInterval("BuildPendingQueue", spFilter, "\(pendingAssets.count) pending")
|
||||||
|
|||||||
@@ -162,16 +162,17 @@ final class BackupStatusService: NSObject, ObservableObject {
|
|||||||
fastPath: if !manifestCacheIsStale, let cached = await NASManifestCache.shared.manifest, localIndexCount > 0 {
|
fastPath: if !manifestCacheIsStale, let cached = await NASManifestCache.shared.manifest, localIndexCount > 0 {
|
||||||
let dirCount = await NASManifestCache.shared.directoryCount ?? 0
|
let dirCount = await NASManifestCache.shared.directoryCount ?? 0
|
||||||
let index: ManifestIndex
|
let index: ManifestIndex
|
||||||
if cached.entries.count <= 1 && dirCount > 1 {
|
if dirCount > cached.entries.count {
|
||||||
// Manifest is sparse vs. actual NAS directory. Use cached filenames for
|
// NAS has more files than the manifest tracks. Build a hybrid index that keeps
|
||||||
// filename-based matching so countSafe reflects real NAS coverage.
|
// manifest localIdentifiers (for Kisani uploads) and adds all cached NAS
|
||||||
// If filenames aren't cached yet, fall through to a full NAS reconcile.
|
// filenames as a filename-fallback. If filenames aren't cached yet, fall
|
||||||
|
// through to a full NAS reconcile so they get populated.
|
||||||
guard let filenames = await NASManifestCache.shared.directoryFilenames else {
|
guard let filenames = await NASManifestCache.shared.directoryFilenames else {
|
||||||
log.info("Reconcile (cached): sparse manifest, no cached filenames — falling to full reconcile")
|
log.info("Reconcile (cached): NAS dir(\(dirCount)) > manifest(\(cached.entries.count)) — no cached filenames, falling to full reconcile")
|
||||||
break fastPath
|
break fastPath
|
||||||
}
|
}
|
||||||
index = await Task.detached(priority: .utility) {
|
index = await Task.detached(priority: .utility) {
|
||||||
ManifestIndex(nasFilenames: filenames)
|
ManifestIndex(manifest: cached, nasFilenames: filenames, totalCount: dirCount)
|
||||||
}.value
|
}.value
|
||||||
} else {
|
} else {
|
||||||
index = await Task.detached(priority: .utility) {
|
index = await Task.detached(priority: .utility) {
|
||||||
@@ -320,9 +321,13 @@ final class BackupStatusService: NSObject, ObservableObject {
|
|||||||
let dirCount = filteredItems.count
|
let dirCount = filteredItems.count
|
||||||
log.info("buildManifestIndex: dir=\(dirCount) manifest=\(index.totalCount)")
|
log.info("buildManifestIndex: dir=\(dirCount) manifest=\(index.totalCount)")
|
||||||
if dirCount > index.totalCount {
|
if dirCount > index.totalCount {
|
||||||
await NASManifestCache.shared.setDirectoryFilenames(filteredItems.map { $0.name })
|
// NAS has more files than manifest: keep manifest localIdentifiers
|
||||||
|
// for Kisani-tracked uploads and add all NAS filenames as fallback.
|
||||||
|
// This prevents discarding accumulated localIdentifier history.
|
||||||
|
let nasNames = filteredItems.map { $0.name }
|
||||||
|
await NASManifestCache.shared.setDirectoryFilenames(nasNames)
|
||||||
return await Task.detached(priority: .utility) {
|
return await Task.detached(priority: .utility) {
|
||||||
ManifestIndex(nasListing: items)
|
ManifestIndex(manifest: manifest, nasFilenames: nasNames, totalCount: dirCount)
|
||||||
}.value
|
}.value
|
||||||
} else {
|
} else {
|
||||||
await NASManifestCache.shared.setDirectoryCount(dirCount)
|
await NASManifestCache.shared.setDirectoryCount(dirCount)
|
||||||
@@ -371,12 +376,9 @@ final class BackupStatusService: NSObject, ObservableObject {
|
|||||||
count += 1
|
count += 1
|
||||||
} else {
|
} 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,
|
||||||
if index.isFilenameOnly && index.matches(filename: name) {
|
index.matches(filename: name) {
|
||||||
count += 1
|
count += 1
|
||||||
} else if index.matchesUnclaimed(filename: name) {
|
|
||||||
count += 1
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -61,14 +61,7 @@ actor LocalPhotoIndex {
|
|||||||
var safe = 0
|
var safe = 0
|
||||||
for record in records.values {
|
for record in records.values {
|
||||||
guard passes(record, filter: filter) else { continue }
|
guard passes(record, filter: filter) else { continue }
|
||||||
if index.matches(localIdentifier: record.localIdentifier) {
|
if index.matches(localIdentifier: record.localIdentifier) || index.matches(filename: record.filename) {
|
||||||
safe += 1
|
|
||||||
} 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
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -79,10 +72,7 @@ actor LocalPhotoIndex {
|
|||||||
func pendingIDs(against index: ManifestIndex, filter: BackupFilter) -> [String] {
|
func pendingIDs(against index: ManifestIndex, filter: BackupFilter) -> [String] {
|
||||||
records.values.compactMap { record in
|
records.values.compactMap { record in
|
||||||
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(filename: record.filename)
|
||||||
index.matches(localIdentifier: record.localIdentifier) ||
|
|
||||||
(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