Fix backup loop, NAS Archive regression, and manifest validity check
- AutoBackupCoordinator: add 55s debounce to onActive() so repeated .task and scenePhase triggers (navigation appear, rapid foreground cycles) are no-ops; start a 60s periodic recheck timer on first active call; stop timer and reset debounce on willResignActiveNotification so each new foreground session always gets an immediate fresh status check - BackupView: remove the redundant lanMonitor.nasReachable onChange that was calling statusService.refresh() directly — coordinator already handles this via its Combine $nasReachable subscriber, avoiding a double reconcile that could bypass the autoBackupOnOpen gate with the wrong lastTriggerWasLAN value - BackupStatusService: when NAS connect fails, restore nasArchiveTotal from NASManifestCache before writing the offline snapshot — prevents the count from showing 0 when the NAS is temporarily unreachable but the cache is warm - BackupStatusService: manifest validity check — if the decoded manifest has ≤ 1 entries (corrupt or newly created), scan the NAS directory and use the real file count as the display floor for nasArchiveTotal without adding orphan entries to the manifest (that was the 7421 regression) - BackupManifest: add ManifestIndex.init(manifest:overrideTotalCount:) for the validity check path — keeps localIdentifier matching intact while correcting the displayed archive count - SMBService: add os.log around connect/auth/listDirectory with actual error reason instead of always surfacing authenticationFailed; distinguish network errors (timeout, host unreachable) from auth errors in thrown BackupError Co-Authored-By: Kutesir <kutesir@provoc.ug> Co-Authored-By: Sentry <sentry@provoc.ug>
This commit is contained in:
@@ -54,6 +54,27 @@ struct ManifestIndex: Sendable {
|
||||
self.totalCount = count
|
||||
}
|
||||
|
||||
// Same as init(manifest:) but with a corrected totalCount for display.
|
||||
// Used when the manifest has too few entries to trust but localIdentifiers are still valid.
|
||||
init(manifest: BackupManifest, overrideTotalCount: Int) {
|
||||
var ids = Set<String>(minimumCapacity: manifest.entries.count)
|
||||
var names = Set<String>(minimumCapacity: manifest.entries.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)
|
||||
}
|
||||
}
|
||||
self.localIdentifiers = ids
|
||||
self.lowercasedFilenames = names
|
||||
self.unclaimedFilenames = unclaimed
|
||||
self.totalCount = overrideTotalCount
|
||||
}
|
||||
|
||||
// Build from NAS directory listing (bootstrap — no localIdentifiers known)
|
||||
init(nasListing items: [NASItem]) {
|
||||
var names = Set<String>()
|
||||
|
||||
@@ -141,12 +141,6 @@ struct BackupView: View {
|
||||
// Re-check every time the app returns to the foreground.
|
||||
if phase == .active { coordinator.onActive() }
|
||||
}
|
||||
.onChange(of: lanMonitor.nasReachable) { reachable in
|
||||
// Belt-and-suspenders: coordinator handles this via Combine,
|
||||
// but an explicit refresh here ensures the stats strip updates
|
||||
// even when the coordinator's phase guard is not .disconnected.
|
||||
if reachable == true { statusService.refresh(force: false) }
|
||||
}
|
||||
.onChange(of: store.savedConnection?.remotePath) { _ in
|
||||
statusService.refresh(force: true)
|
||||
}
|
||||
|
||||
@@ -30,6 +30,8 @@ final class AutoBackupCoordinator: ObservableObject {
|
||||
@Published private(set) var pendingAtDisconnect: Int = 0
|
||||
|
||||
private var lastTriggerWasLAN = false
|
||||
private var lastAutoCheckAt: Date = .distantPast
|
||||
private var recheckTimer: AnyCancellable?
|
||||
|
||||
private let engine = BackupEngine.shared
|
||||
private let statusService = BackupStatusService.shared
|
||||
@@ -42,6 +44,36 @@ final class AutoBackupCoordinator: ObservableObject {
|
||||
|
||||
private init() {
|
||||
setupObservation()
|
||||
setupAppLifecycle()
|
||||
}
|
||||
|
||||
private func setupAppLifecycle() {
|
||||
NotificationCenter.default.addObserver(
|
||||
self,
|
||||
selector: #selector(appWillResignActive),
|
||||
name: UIApplication.willResignActiveNotification,
|
||||
object: nil
|
||||
)
|
||||
}
|
||||
|
||||
// Stop the timer and reset the debounce window on every background transition.
|
||||
// This ensures the next foreground session always gets an immediate status check.
|
||||
@objc private nonisolated func appWillResignActive() {
|
||||
Task { @MainActor [weak self] in
|
||||
self?.recheckTimer = nil
|
||||
self?.lastAutoCheckAt = .distantPast
|
||||
log.debug("App resigned — recheck timer stopped, debounce reset")
|
||||
}
|
||||
}
|
||||
|
||||
private func startPeriodicRecheckIfNeeded() {
|
||||
guard recheckTimer == nil else { return }
|
||||
recheckTimer = Timer.publish(every: 60, on: .main, in: .common)
|
||||
.autoconnect()
|
||||
.sink { [weak self] _ in
|
||||
log.debug("Periodic 60s recheck fired")
|
||||
self?.onActive()
|
||||
}
|
||||
}
|
||||
|
||||
private func setupObservation() {
|
||||
@@ -69,13 +101,28 @@ final class AutoBackupCoordinator: ObservableObject {
|
||||
// MARK: — External triggers
|
||||
|
||||
/// Call when the app becomes active: on first launch, foreground resume, or session unlock.
|
||||
/// Safe to call repeatedly — guards prevent redundant reconciliation.
|
||||
/// Safe to call repeatedly — debounce prevents redundant reconciliation within 55s.
|
||||
func onActive(lanTriggered: Bool = false) {
|
||||
lastTriggerWasLAN = lanTriggered
|
||||
guard !engine.job.status.isActive else { return }
|
||||
guard store.savedConnection != nil else { return }
|
||||
guard phase == .idle else { return } // Don't interrupt an in-progress check
|
||||
guard phase == .idle else { return }
|
||||
|
||||
// Non-LAN triggers are debounced: .task {} fires on every navigation appear,
|
||||
// and scenePhase fires on every foreground. Only allow one check per 55 seconds
|
||||
// so the user can background+foreground without restarting a new backup job.
|
||||
if !lanTriggered {
|
||||
let elapsed = Date().timeIntervalSince(lastAutoCheckAt)
|
||||
guard elapsed > 55 else {
|
||||
log.debug("onActive debounced — \(Int(elapsed))s since last check (need >55s)")
|
||||
return
|
||||
}
|
||||
}
|
||||
|
||||
lastAutoCheckAt = Date()
|
||||
phase = .checking
|
||||
startPeriodicRecheckIfNeeded()
|
||||
log.info("onActive: trigger=\(lanTriggered ? "LAN" : "appOpen") — status check starting")
|
||||
statusService.refresh(force: false)
|
||||
}
|
||||
|
||||
|
||||
@@ -182,12 +182,22 @@ final class BackupStatusService: NSObject, ObservableObject {
|
||||
username: conn.username, password: conn.password
|
||||
)
|
||||
} catch {
|
||||
let reason = (error as? BackupError).flatMap { $0.errorDescription } ?? error.localizedDescription
|
||||
log.warning("NAS connect failed: \(reason, privacy: .public) — retaining cached counts")
|
||||
refreshError = reason
|
||||
|
||||
// Restore last-known NAS archive count from manifest cache so the dashboard
|
||||
// doesn't show 0 when the NAS is temporarily offline.
|
||||
if let cached = await NASManifestCache.shared.manifest {
|
||||
let cachedCount = ManifestIndex(manifest: cached).totalCount
|
||||
snapshot.nasArchiveTotal = max(snapshot.nasArchiveTotal, cachedCount)
|
||||
log.info("NAS offline — restored nasArchiveTotal=\(self.snapshot.nasArchiveTotal) from cache")
|
||||
}
|
||||
snapshot.phoneTotal = phoneTotal
|
||||
snapshot.alreadySafe = min(snapshot.alreadySafe, phoneTotal)
|
||||
snapshot.connectionState = .offline
|
||||
snapshot.lastCheckedAt = Date()
|
||||
saveSnapshot()
|
||||
log.warning("NAS offline — cached numbers retained")
|
||||
return
|
||||
}
|
||||
defer { transfer.disconnect() }
|
||||
@@ -250,8 +260,21 @@ final class BackupStatusService: NSObject, ObservableObject {
|
||||
}.value
|
||||
if let (index, manifest) = decoded {
|
||||
lastManifest = manifest
|
||||
// Persist to local cache so future launches skip the NAS round-trip.
|
||||
Task { await NASManifestCache.shared.update(manifest) }
|
||||
|
||||
// Validity check: if the manifest has ≤ 1 entries it may be corrupt or newly
|
||||
// created. Scan the directory to get the real file count for the NAS Archive
|
||||
// display — but do NOT add directory files as orphan manifest entries.
|
||||
if index.totalCount <= 1 {
|
||||
let items = (try? await transfer.listDirectory(at: conn.remotePath)) ?? []
|
||||
let dirCount = items.filter {
|
||||
!$0.isDirectory && !$0.name.hasPrefix(".") && $0.name != BackupManifest.remoteFilename
|
||||
}.count
|
||||
log.info("buildManifestIndex: manifest=\(index.totalCount) dir=\(dirCount) — using max for nasArchiveTotal")
|
||||
if dirCount > index.totalCount {
|
||||
return ManifestIndex(manifest: manifest, overrideTotalCount: dirCount)
|
||||
}
|
||||
}
|
||||
return index
|
||||
}
|
||||
} catch {
|
||||
|
||||
@@ -1,5 +1,8 @@
|
||||
import Foundation
|
||||
import SMBClient
|
||||
import os.log
|
||||
|
||||
private let log = Logger(subsystem: "com.albert.nasbackup", category: "SMBService")
|
||||
|
||||
final class SMBService: NASTransferProtocol {
|
||||
private var client: SMBClient?
|
||||
@@ -16,12 +19,24 @@ final class SMBService: NASTransferProtocol {
|
||||
}
|
||||
|
||||
func connect(to host: String, port: Int, username: String, password: String) async throws {
|
||||
log.info("SMB connecting — host=\(host, privacy: .public):\(port) user=\(username, privacy: .private)")
|
||||
let c = SMBClient(host: host, port: port)
|
||||
do {
|
||||
try await c.login(username: username, password: password)
|
||||
} catch {
|
||||
log.error("SMB connect failed — host=\(host, privacy: .public):\(port) error=\(error.localizedDescription, privacy: .public)")
|
||||
// Propagate a connection error rather than always claiming auth failed —
|
||||
// the real cause could be host unreachable, wrong port, or network timeout.
|
||||
let nsErr = error as NSError
|
||||
if nsErr.domain == NSURLErrorDomain ||
|
||||
nsErr.code == NSURLErrorTimedOut ||
|
||||
nsErr.code == NSURLErrorCannotConnectToHost ||
|
||||
nsErr.code == NSURLErrorNetworkConnectionLost {
|
||||
throw BackupError.connectionFailed("\(host):\(port) — \(error.localizedDescription)")
|
||||
}
|
||||
throw BackupError.authenticationFailed
|
||||
}
|
||||
log.info("SMB auth OK — \(host, privacy: .public):\(port)")
|
||||
self.client = c
|
||||
self.isConnected = true
|
||||
}
|
||||
@@ -47,9 +62,10 @@ final class SMBService: NASTransferProtocol {
|
||||
func listDirectory(at path: String) async throws -> [NASItem] {
|
||||
guard let client else { throw BackupError.connectionFailed("Not connected") }
|
||||
let (share, rel) = splitPath(path)
|
||||
log.info("SMB listDirectory — share=\(share, privacy: .public) path=\(rel.isEmpty ? "/" : rel, privacy: .public)")
|
||||
try await client.connectShare(share)
|
||||
let files = try await client.listDirectory(path: rel.isEmpty ? "" : rel)
|
||||
return files.compactMap { file -> NASItem? in
|
||||
let items = files.compactMap { file -> NASItem? in
|
||||
guard file.name != "." && file.name != ".." else { return nil }
|
||||
let fullPath = rel.isEmpty ? "/\(share)/\(file.name)" : "/\(share)/\(rel)/\(file.name)"
|
||||
return NASItem(
|
||||
@@ -60,6 +76,8 @@ final class SMBService: NASTransferProtocol {
|
||||
modifiedDate: file.lastWriteTime
|
||||
)
|
||||
}
|
||||
log.info("SMB listDirectory result — \(items.count) items at \(path, privacy: .public)")
|
||||
return items
|
||||
}
|
||||
|
||||
func createDirectory(at path: String) async throws {
|
||||
|
||||
Reference in New Issue
Block a user