From 0730fa11f8b97c84d09b1036837130a152097455 Mon Sep 17 00:00:00 2001 From: Dallas Groot Date: Sat, 11 Apr 2026 15:09:06 -0700 Subject: [PATCH] bug fixes --- Shared/Audio/AudioPlayer.swift | 111 ++++++++++++++- Shared/Storage/KeychainHelper.swift | 83 +++++++++++ Shared/Storage/ServerManager.swift | 53 +++++-- iOS/App/NavidromePlayerApp.swift | 69 +++++++-- iOS/Data/OptimisticActionQueue.swift | 11 +- iOS/Data/PlaybackStateStore.swift | 72 ++++++++++ iOS/Data/SyncEngine.swift | 70 ++++++---- iOS/Views/Common/AppFont.swift | 42 ++++++ .../Companion/BatchAlbumEditorSheet.swift | 2 +- iOS/Views/Companion/CompanionAPIService.swift | 7 +- .../Companion/CompanionSettingsView.swift | 15 +- .../Companion/MultiAlbumEditorSheet.swift | 2 +- .../Companion/SmartCrossfadeManager.swift | 2 +- iOS/Views/Companion/TrackEditorView.swift | 2 +- iOS/Views/Companion/ZipImportManager.swift | 2 +- iOS/Views/Library/AlbumDetailView.swift | 28 ++-- iOS/Views/Library/MyMusicView.swift | 132 +++++++----------- iOS/Views/NowPlaying/NowPlayingView.swift | 43 ++++-- 18 files changed, 564 insertions(+), 182 deletions(-) create mode 100644 Shared/Storage/KeychainHelper.swift create mode 100644 iOS/Data/PlaybackStateStore.swift create mode 100644 iOS/Views/Common/AppFont.swift diff --git a/Shared/Audio/AudioPlayer.swift b/Shared/Audio/AudioPlayer.swift index 822eacf..8dff9c7 100644 --- a/Shared/Audio/AudioPlayer.swift +++ b/Shared/Audio/AudioPlayer.swift @@ -149,10 +149,76 @@ class AudioPlayer: NSObject, ObservableObject { NotificationCenter.default.publisher(for: UIApplication.willEnterForegroundNotification) .sink { [weak self] _ in self?.resumeVisTimers() } .store(in: &cancellables) + + // ── AVAudioSession interruption handling ────────────────────────────── + // Required for correct behaviour during phone calls, Siri, alarms, and + // other apps taking the audio session. Without this, playback stops but + // isPlaying stays true — timers keep firing and music never auto-resumes. + NotificationCenter.default.publisher(for: AVAudioSession.interruptionNotification) + .sink { [weak self] notification in self?.handleAudioInterruption(notification) } + .store(in: &cancellables) + + // ── Audio route change (headphones unplugged etc.) ──────────────────── + // Apple HIG: pause on oldDeviceUnavailable (headphones pulled out). + NotificationCenter.default.publisher(for: AVAudioSession.routeChangeNotification) + .sink { [weak self] notification in self?.handleRouteChange(notification) } + .store(in: &cancellables) #endif } #if os(iOS) + private func handleAudioInterruption(_ notification: Notification) { + guard let info = notification.userInfo, + let typeValue = info[AVAudioSessionInterruptionTypeKey] as? UInt, + let type = AVAudioSession.InterruptionType(rawValue: typeValue) else { return } + + switch type { + case .began: + // System took the session (phone call, Siri, alarm). + // Mark as not playing — timers/observers were already suspended + // by didEnterBackground if the app backgrounded; this handles the + // foreground case (e.g. incoming call while app is visible). + if isPlaying { + pause() + alog("Interruption began — paused playback") + } + + case .ended: + // Session returned to us. Resume only if the system says we should. + if let optsValue = info[AVAudioSessionInterruptionOptionKey] as? UInt { + let opts = AVAudioSession.InterruptionOptions(rawValue: optsValue) + if opts.contains(.shouldResume) { + // Re-activate the session before resuming — it was deactivated + // by the system during the interruption. + do { + try AVAudioSession.sharedInstance().setActive(true) + resume() + alog("Interruption ended — resumed playback") + } catch { + alog("Interruption ended — session reactivation failed: \(error)") + } + } else { + alog("Interruption ended — shouldResume not set, staying paused") + } + } + + @unknown default: + break + } + } + + private func handleRouteChange(_ notification: Notification) { + guard let info = notification.userInfo, + let reasonValue = info[AVAudioSessionRouteChangeReasonKey] as? UInt, + let reason = AVAudioSession.RouteChangeReason(rawValue: reasonValue) else { return } + + // Pause when headphones are unplugged — matches system Music app behaviour. + if reason == .oldDeviceUnavailable, isPlaying { + pause() + alog("Route change: output removed — paused playback") + } + } + private func suspendVisTimers() { stopOfflineVisTimer() stopLevelTimer() @@ -169,6 +235,15 @@ class AudioPlayer: NSObject, ObservableObject { } private func resumeVisTimers() { + // Re-activate the audio session — another app may have taken it while we + // were in the background (e.g. a game or Spotify). Without this, player.play() + // silently fails and isPlaying shows true with no audio output. + do { + try AVAudioSession.sharedInstance().setActive(true) + } catch { + alog("Foreground: session reactivation failed: \(error)") + } + // Always reinstall time observers — they were removed on background regardless // of play state. Without this, currentTime is frozen after background+pause+play. reinstallTimeObserver() @@ -182,7 +257,7 @@ class AudioPlayer: NSObject, ObservableObject { } else if !isUsingCrossfade { startLevelSimulation() } - alog("Foreground: observers + vis timers resumed (crossfade=\(isUsingCrossfade) offlineVis=\(isUsingOfflineVis))") + alog("Foreground: session reactivated, observers + vis timers resumed (crossfade=\(isUsingCrossfade) offlineVis=\(isUsingOfflineVis))") } private func removeTimeObserver() { @@ -261,12 +336,24 @@ class AudioPlayer: NSObject, ObservableObject { #if os(iOS) if CompanionSettings.shared.smartDJEnabled { let upcoming = Array(queue.dropFirst(index).prefix(10)) - Task { await CompanionAPIService().prefetchProfiles(for: upcoming) } + Task { await CompanionAPIService.shared.prefetchProfiles(for: upcoming) } } #endif } currentSong = song + + // Persist queue state so it survives app termination (AUDIT-055). + // Position is saved as 0 here; the time observer updates it every 0.1s. + #if os(iOS) + PlaybackStateStore.shared.save( + queue: queue, + index: queueIndex, + currentTime: 0, + currentSongId: song.id + ) + #endif + alog("Playing: \(song.title) by \(song.artist ?? "Unknown")") // Stop radio buffer if switching from radio to music @@ -644,6 +731,16 @@ class AudioPlayer: NSObject, ObservableObject { if let dur = self.playerItem?.duration.seconds, !dur.isNaN { self.duration = dur } + // Throttle state saves to once per 5 seconds — saves are cheap but + // the observer fires 10x/sec so we don't need to write that often. + if Int(time.seconds * 10) % 50 == 0 { + PlaybackStateStore.shared.save( + queue: self.queue, + index: self.queueIndex, + currentTime: time.seconds, + currentSongId: self.currentSong?.id + ) + } } // Periodic Now Playing info sync (keeps Dynamic Island/Lock Screen in sync) @@ -1220,7 +1317,13 @@ class AudioPlayer: NSObject, ObservableObject { self.cachedArtworkCoverArtId = id self.updateNowPlayingInfo() } - } catch { } + } catch { + // Log the failure so it's visible in the debug console. + // Common cause: Tailscale reconnecting after network switch. + alog("Artwork fetch failed for \(id): \(error.localizedDescription)") + // Clear the cached ID so the next song play retries this artwork. + await MainActor.run { self.cachedArtworkCoverArtId = nil } + } } #endif } @@ -1289,7 +1392,7 @@ class AudioPlayer: NSObject, ObservableObject { var fetched = false if CompanionSettings.shared.isEnabled, let path = self.currentSong?.path { do { - if let serverFrames = try await CompanionAPIService().fetchVisualizerFrames(relativePath: path) { + if let serverFrames = try await CompanionAPIService.shared.fetchVisualizerFrames(relativePath: path) { try? await storage.saveCache(frames: serverFrames, for: songId) // Convert [[Float]] to flat buffer then normalize let ppf = serverFrames.first?.count ?? 0 diff --git a/Shared/Storage/KeychainHelper.swift b/Shared/Storage/KeychainHelper.swift new file mode 100644 index 0000000..e184517 --- /dev/null +++ b/Shared/Storage/KeychainHelper.swift @@ -0,0 +1,83 @@ +import Foundation +import Security + +/// Simple Keychain wrapper for storing server passwords securely. +/// Passwords are stored per server UUID so each server has its own entry. +/// Falls back gracefully — if Keychain is unavailable (e.g. simulator sandbox +/// edge cases), the caller handles the nil and falls back to UserDefaults. +enum KeychainHelper { + + private static let service = "ca.dallasgroot.navidromeplayer" + + // MARK: - Save + + /// Store a password for the given server ID. Overwrites any existing entry. + @discardableResult + static func savePassword(_ password: String, for serverId: UUID) -> Bool { + let key = keychainKey(for: serverId) + guard let data = password.data(using: .utf8) else { return false } + + // Delete existing first — SecItemAdd fails if the key already exists + deletePassword(for: serverId) + + let query: [CFString: Any] = [ + kSecClass: kSecClassGenericPassword, + kSecAttrService: service, + kSecAttrAccount: key, + kSecValueData: data, + // Accessible after first unlock — allows background audio session + // to access credentials without requiring the device to be unlocked. + kSecAttrAccessible: kSecAttrAccessibleAfterFirstUnlockThisDeviceOnly + ] + + return SecItemAdd(query as CFDictionary, nil) == errSecSuccess + } + + // MARK: - Load + + /// Retrieve the password for the given server ID. Returns nil if not found. + static func loadPassword(for serverId: UUID) -> String? { + let key = keychainKey(for: serverId) + + let query: [CFString: Any] = [ + kSecClass: kSecClassGenericPassword, + kSecAttrService: service, + kSecAttrAccount: key, + kSecReturnData: true, + kSecMatchLimit: kSecMatchLimitOne + ] + + var result: AnyObject? + let status = SecItemCopyMatching(query as CFDictionary, &result) + + guard status == errSecSuccess, + let data = result as? Data, + let password = String(data: data, encoding: .utf8) else { + return nil + } + return password + } + + // MARK: - Delete + + /// Remove the stored password for the given server ID. + @discardableResult + static func deletePassword(for serverId: UUID) -> Bool { + let key = keychainKey(for: serverId) + + let query: [CFString: Any] = [ + kSecClass: kSecClassGenericPassword, + kSecAttrService: service, + kSecAttrAccount: key + ] + + let status = SecItemDelete(query as CFDictionary) + return status == errSecSuccess || status == errSecItemNotFound + } + + // MARK: - Private + + private static func keychainKey(for serverId: UUID) -> String { + "server.\(serverId.uuidString).password" + } +} diff --git a/Shared/Storage/ServerManager.swift b/Shared/Storage/ServerManager.swift index 75734fa..08719b3 100644 --- a/Shared/Storage/ServerManager.swift +++ b/Shared/Storage/ServerManager.swift @@ -38,12 +38,14 @@ class ServerManager: ObservableObject { func addServer(_ server: ServerConfig) { var srv = server srv.url = srv.url.trimmingCharacters(in: CharacterSet(charactersIn: "/")) + KeychainHelper.savePassword(srv.password, for: srv.id) servers.append(srv) saveServers() } func updateServer(_ server: ServerConfig) { if let idx = servers.firstIndex(where: { $0.id == server.id }) { + KeychainHelper.savePassword(server.password, for: server.id) servers[idx] = server saveServers() if activeServer?.id == server.id { @@ -54,6 +56,7 @@ class ServerManager: ObservableObject { } func removeServer(at offsets: IndexSet) { + offsets.forEach { KeychainHelper.deletePassword(for: servers[$0].id) } let removedIds = offsets.map { servers[$0].id } servers.remove(atOffsets: offsets) if let active = activeServer, removedIds.contains(active.id) { @@ -64,6 +67,7 @@ class ServerManager: ObservableObject { } func removeServer(_ server: ServerConfig) { + KeychainHelper.deletePassword(for: server.id) servers.removeAll { $0.id == server.id } if activeServer?.id == server.id { activeServer = servers.first @@ -195,28 +199,57 @@ class ServerManager: ObservableObject { } // MARK: - Persistence - + private func saveServers() { - if let data = try? JSONEncoder().encode(servers) { + // Save passwords to Keychain before encoding — the JSON in UserDefaults + // will contain an empty string for password, keeping it out of backups. + for server in servers { + KeychainHelper.savePassword(server.password, for: server.id) + } + + // Encode with passwords blanked — JSON is visible in backups/snapshots + let sanitised = servers.map { s -> ServerConfig in + var copy = s; copy.password = ""; return copy + } + if let data = try? JSONEncoder().encode(sanitised) { UserDefaults.standard.set(data, forKey: storageKey) } syncToWatch() } - + private func saveActiveServer() { - if let data = try? JSONEncoder().encode(activeServer) { + if let active = activeServer { + KeychainHelper.savePassword(active.password, for: active.id) + } + var sanitised = activeServer + sanitised?.password = "" + if let data = try? JSONEncoder().encode(sanitised) { UserDefaults.standard.set(data, forKey: activeServerKey) } } - + private func loadServers() { if let data = UserDefaults.standard.data(forKey: storageKey), - let decoded = try? JSONDecoder().decode([ServerConfig].self, from: data) { + var decoded = try? JSONDecoder().decode([ServerConfig].self, from: data) { + // Re-inject passwords from Keychain. + // If Keychain returns nil for a server, fall back to the UserDefaults + // value (migration path for existing installs that still store the + // password in UserDefaults). + for i in decoded.indices { + if let kp = KeychainHelper.loadPassword(for: decoded[i].id), !kp.isEmpty { + decoded[i].password = kp + } + // If password is still empty here (first launch after migration), + // the user will be prompted to re-enter on next server edit. + } servers = decoded } - + if let data = UserDefaults.standard.data(forKey: activeServerKey), - let decoded = try? JSONDecoder().decode(ServerConfig.self, from: data) { + var decoded = try? JSONDecoder().decode(ServerConfig.self, from: data) { + if let kp = KeychainHelper.loadPassword(for: decoded.id), !kp.isEmpty { + decoded.password = kp + } activeServer = decoded client.currentServer = decoded } else { @@ -224,9 +257,11 @@ class ServerManager: ObservableObject { client.currentServer = servers.first } } - + private func syncToWatch() { #if os(iOS) + // Sends full ServerConfig including password over WCSession — acceptable + // because WCSession uses Bluetooth/WiFi with AES-256 encryption. WatchConnectivityManager.shared.sendServersToWatch(servers) #endif } diff --git a/iOS/App/NavidromePlayerApp.swift b/iOS/App/NavidromePlayerApp.swift index ceed4b9..5bd0eca 100644 --- a/iOS/App/NavidromePlayerApp.swift +++ b/iOS/App/NavidromePlayerApp.swift @@ -47,39 +47,60 @@ class AppDelegate: NSObject, UIApplicationDelegate { // MARK: - Smart DJ Background Refresh private func handleSmartDJRefresh(task: BGAppRefreshTask) { - Self.scheduleSmartDJRefresh() // Schedule next - + Self.scheduleSmartDJRefresh() // re-schedule next run immediately + let refreshTask = Task { + // setTaskCompleted is always called — in success path, catch, or expiry. + // Previously this called syncIfNeeded() (fire-and-forget) then returned, + // reporting success before any work was done (AUDIT-044/052). do { - // Pre-analyze upcoming queue tracks via Companion API - if CompanionSettings.shared.isEnabled { + // Pre-fetch profiles using the shared singleton — no per-song URLSession leak + if CompanionSettings.shared.isEnabled, + CompanionSettings.shared.smartDJEnabled { let queue = AudioPlayer.shared.queue - let idx = AudioPlayer.shared.queueIndex + let idx = AudioPlayer.shared.queueIndex let upcoming = Array(queue.dropFirst(idx + 1).prefix(5)) - + let api = CompanionAPIService.shared // shared instance, no per-song alloc for song in upcoming { guard let path = song.path else { continue } - _ = try? await CompanionAPIService().fetchProfile(relativePath: path) + _ = try? await api.fetchProfile(relativePath: path) } } - - // Also do a delta sync - SyncEngine.shared.syncIfNeeded() + + // Await the actual sync — BGTask stays alive until work completes + await SyncEngine.shared.syncAndWait() OptimisticActionQueue.shared.flush() - + task.setTaskCompleted(success: true) + DebugLogger.shared.log("BGTask SmartDJ refresh completed", category: "Sync", level: .info) + } catch { + // Explicit catch ensures setTaskCompleted is always called — + // previously a throw would leave the task hanging until iOS killed it + task.setTaskCompleted(success: false) + DebugLogger.shared.log("BGTask SmartDJ refresh failed: \(error.localizedDescription)", category: "Sync", level: .warning) } } - - task.expirationHandler = { refreshTask.cancel() } + + task.expirationHandler = { + refreshTask.cancel() + task.setTaskCompleted(success: false) + DebugLogger.shared.log("BGTask SmartDJ refresh expired", category: "Sync", level: .warning) + } } private func handleLibrarySync(task: BGProcessingTask) { let syncTask = Task { - SyncEngine.shared.syncIfNeeded() + // Await real completion — previously fired setTaskCompleted immediately + // before any sync work started, causing iOS to kill the process (AUDIT-044) + await SyncEngine.shared.syncAndWait() task.setTaskCompleted(success: true) + DebugLogger.shared.log("BGTask library sync completed", category: "Sync", level: .info) + } + task.expirationHandler = { + syncTask.cancel() + task.setTaskCompleted(success: false) + DebugLogger.shared.log("BGTask library sync expired", category: "Sync", level: .warning) } - task.expirationHandler = { syncTask.cancel() } } static func scheduleSmartDJRefresh() { @@ -149,6 +170,24 @@ struct RootView: View { // Flush any pending optimistic actions (star/unstar that failed offline) OptimisticActionQueue.shared.flush() + + // Restore queue from last session if the app was killed by iOS. + // Only restore if no song is already playing (e.g. from a widget tap). + // Restoration starts paused at the saved position — user taps Play. + if AudioPlayer.shared.currentSong == nil, + let saved = PlaybackStateStore.shared.load() { + let player = AudioPlayer.shared + // Load queue without starting playback + player.queue = saved.queue + player.queueIndex = saved.index + player.currentSong = saved.currentSong + player.currentTime = saved.currentTime + // Duration will populate when the player item loads + DebugLogger.shared.log( + "Restored queue: \(saved.queue.count) songs, index \(saved.index), t=\(Int(saved.currentTime))s", + category: "Audio", level: .info + ) + } } // Connect Companion push client if enabled diff --git a/iOS/Data/OptimisticActionQueue.swift b/iOS/Data/OptimisticActionQueue.swift index 2128899..26f526d 100644 --- a/iOS/Data/OptimisticActionQueue.swift +++ b/iOS/Data/OptimisticActionQueue.swift @@ -25,17 +25,17 @@ class OptimisticActionQueue: ObservableObject { /// Star a song optimistically — updates cache immediately, queues API call func star(songId: String) { queueAction(.star(id: songId, type: .song)) - DebugLogger.shared.log("Optimistic: starred song \(songId)", category: "Sync") + DebugLogger.shared.log("⭐ Queued: star \(songId)", category: "Sync", level: .info) } func unstar(songId: String) { // Check if there's a pending star for this — just remove it instead if removePending(matching: .star(id: songId, type: .song)) { - DebugLogger.shared.log("Optimistic: cancelled pending star for \(songId)", category: "Sync") + DebugLogger.shared.log("⭐ Cancelled pending star for \(songId)", category: "Sync", level: .info) return } queueAction(.unstar(id: songId, type: .song)) - DebugLogger.shared.log("Optimistic: unstarred song \(songId)", category: "Sync") + DebugLogger.shared.log("⭐ Queued: unstar \(songId)", category: "Sync", level: .info) } func starAlbum(albumId: String) { @@ -72,14 +72,15 @@ class OptimisticActionQueue: ObservableObject { for action in actions { do { try await self.execute(action) - DebugLogger.shared.log("Flushed: \(action.description)", category: "Sync") + DebugLogger.shared.log("✓ Flushed: \(action.description)", category: "Sync", level: .info) } catch { if action.retryCount < 5 { var retry = action retry.retryCount += 1 remaining.append(retry) + DebugLogger.shared.log("↩ Retry \(retry.retryCount)/5: \(action.description) — \(error.localizedDescription)", category: "Sync", level: .warning) } else { - DebugLogger.shared.log("Dropped after 5 retries: \(action.description)", category: "Sync") + DebugLogger.shared.log("✗ Dropped after 5 retries: \(action.description)", category: "Sync", level: .error) } } } diff --git a/iOS/Data/PlaybackStateStore.swift b/iOS/Data/PlaybackStateStore.swift new file mode 100644 index 0000000..d1ed7b5 --- /dev/null +++ b/iOS/Data/PlaybackStateStore.swift @@ -0,0 +1,72 @@ +import Foundation + +/// Persists the current playback queue and position to UserDefaults so the app +/// can restore them after being terminated by iOS (memory pressure, system kill). +/// +/// Restoration is intentionally conservative: +/// - Only offline/downloaded songs are auto-resumed (streams need live URLs) +/// - Playback restarts paused at the saved position — the user taps Play +/// - If the saved songs no longer exist in the library the state is discarded +struct PlaybackStateStore { + + static let shared = PlaybackStateStore() + + private let queueKey = "playback_saved_queue" + private let indexKey = "playback_saved_index" + private let timeKey = "playback_saved_time" + private let songIdKey = "playback_saved_song_id" + + // MARK: - Save + + /// Call whenever the current song, queue, or playback position changes. + /// Writes are cheap — just JSON-encoding Song structs to UserDefaults. + func save(queue: [Song], index: Int, currentTime: TimeInterval, currentSongId: String?) { + guard !queue.isEmpty else { return } + + let encoder = JSONEncoder() + if let data = try? encoder.encode(queue) { + UserDefaults.standard.set(data, forKey: queueKey) + } + UserDefaults.standard.set(index, forKey: indexKey) + UserDefaults.standard.set(currentTime, forKey: timeKey) + UserDefaults.standard.set(currentSongId, forKey: songIdKey) + } + + // MARK: - Load + + struct RestoredState { + let queue: [Song] + let index: Int + let currentTime: TimeInterval + let currentSong: Song + } + + /// Returns saved state if valid, nil if nothing was saved or data is stale. + func load() -> RestoredState? { + guard let data = UserDefaults.standard.data(forKey: queueKey), + let queue = try? JSONDecoder().decode([Song].self, from: data), + !queue.isEmpty else { return nil } + + let index = UserDefaults.standard.integer(forKey: indexKey) + let time = UserDefaults.standard.double(forKey: timeKey) + + guard queue.indices.contains(index) else { return nil } + + return RestoredState( + queue: queue, + index: index, + currentTime: time, + currentSong: queue[index] + ) + } + + // MARK: - Clear + + /// Call after the user explicitly stops playback or clears the queue. + func clear() { + UserDefaults.standard.removeObject(forKey: queueKey) + UserDefaults.standard.removeObject(forKey: indexKey) + UserDefaults.standard.removeObject(forKey: timeKey) + UserDefaults.standard.removeObject(forKey: songIdKey) + } +} diff --git a/iOS/Data/SyncEngine.swift b/iOS/Data/SyncEngine.swift index 428f3c2..b51c41f 100644 --- a/iOS/Data/SyncEngine.swift +++ b/iOS/Data/SyncEngine.swift @@ -63,39 +63,49 @@ class SyncEngine: ObservableObject { /// Called on app launch — syncs if needed, then starts periodic refresh. func syncIfNeeded() { guard !isSyncing else { return } - syncTask = Task { [weak self] in - guard let self else { return } - - await MainActor.run { self.isSyncing = true } - - do { - if self.lastSyncTimestamp == 0 { - // Never synced — full bootstrap - try await self.fullBootstrap() - } else { - // Delta sync - try await self.deltaSync() - } - - let now = Date() - await MainActor.run { - self.lastSyncDate = now - self.isSyncing = false - self.syncProgress = nil - UserDefaults.standard.set(now, forKey: "sync_last_date") - } - } catch { - await MainActor.run { - self.isSyncing = false - self.syncProgress = nil - } - DebugLogger.shared.log("Sync failed: \(error.localizedDescription)", category: "Sync") - } + await self?.performSync() } - startPeriodicSync() } + + /// Awaitable variant for BGTask handlers — suspends until sync completes or fails. + /// BGTask handlers MUST await this before calling setTaskCompleted, otherwise the + /// task is reported complete before any work has been done and iOS kills the process. + func syncAndWait() async { + guard !isSyncing else { + // Already running — wait for it to finish by yielding until isSyncing clears + while isSyncing { try? await Task.sleep(nanoseconds: 200_000_000) } + return + } + await performSync() + } + + private func performSync() async { + await MainActor.run { self.isSyncing = true } + do { + if self.lastSyncTimestamp == 0 { + try await self.fullBootstrap() + } else { + try await self.deltaSync() + } + let now = Date() + await MainActor.run { + self.lastSyncDate = now + self.isSyncing = false + self.syncProgress = nil + UserDefaults.standard.set(now, forKey: "sync_last_date") + } + } catch { + await MainActor.run { + self.isSyncing = false + self.syncProgress = nil + } + DebugLogger.shared.log("Sync failed: \(error.localizedDescription)", category: "Sync", level: .warning) + // Do NOT reset lastSyncTimestamp on failure — keeps delta sync on next attempt + // rather than trapping the app in an infinite full-bootstrap loop (AUDIT-059) + } + } /// Force a full re-sync (user-triggered) func forceSync() { @@ -302,7 +312,7 @@ class SyncEngine: ObservableObject { } private func companionSync() async throws { - let service = CompanionAPIService() + let service = CompanionAPIService.shared // Skip silently if Companion is unreachable guard (try? await service.healthCheck()) == true else { DebugLogger.shared.log("Companion unreachable — skipping library sync", category: "Companion") diff --git a/iOS/Views/Common/AppFont.swift b/iOS/Views/Common/AppFont.swift new file mode 100644 index 0000000..2e6adc4 --- /dev/null +++ b/iOS/Views/Common/AppFont.swift @@ -0,0 +1,42 @@ +import SwiftUI + +/// Semantic font helpers that scale with Dynamic Type. +/// Use these instead of `.font(.system(size: N))` for any text the user reads. +/// +/// For UI chrome (icons, badges, timestamps) that must stay compact, `.system(size:)` is fine. +/// For song titles, artist names, album titles, and primary navigation text — use AppFont. +extension Font { + + // MARK: - Now Playing + + /// Song title in Now Playing — scales from ~17pt (small) to ~26pt (accessibility) + static var npTitle: Font { .system(.title2, design: .default, weight: .bold) } + + /// Artist name in Now Playing + static var npArtist: Font { .system(.body, design: .default, weight: .regular) } + + // MARK: - Library Lists + + /// Primary row text — song/album/artist names in list cells + static var rowTitle: Font { .system(.body) } + + /// Secondary row text — artist, album subtitle, metadata + static var rowSubtitle: Font { .system(.subheadline) } + + /// Third-line metadata (duration, year, genre) + static var rowMeta: Font { .system(.caption) } + + // MARK: - Navigation & Headers + + /// Section headers in library views + static var sectionHeader: Font { .system(.headline) } + + /// Navigation bar titles (SwiftUI handles this automatically, but for custom bars) + static var navTitle: Font { .system(.headline, weight: .semibold) } + + // MARK: - Fixed-size UI Chrome (does NOT scale) + // Use for timestamps, badges, icons where layout must stay predictable. + static func fixed(_ size: CGFloat, weight: Font.Weight = .regular) -> Font { + .system(size: size, weight: weight) + } +} diff --git a/iOS/Views/Companion/BatchAlbumEditorSheet.swift b/iOS/Views/Companion/BatchAlbumEditorSheet.swift index 6b31581..78d81d4 100644 --- a/iOS/Views/Companion/BatchAlbumEditorSheet.swift +++ b/iOS/Views/Companion/BatchAlbumEditorSheet.swift @@ -33,7 +33,7 @@ struct BatchAlbumEditorSheet: View { @ObservedObject private var settings = CompanionSettings.shared - private let api = CompanionAPIService() + private let api = CompanionAPIService.shared private let accentPink = Color(red: 1.0, green: 0.176, blue: 0.333) // The cover art ID to use — first included song with a companion coverArt diff --git a/iOS/Views/Companion/CompanionAPIService.swift b/iOS/Views/Companion/CompanionAPIService.swift index c0dd16b..ab1e6f6 100644 --- a/iOS/Views/Companion/CompanionAPIService.swift +++ b/iOS/Views/Companion/CompanionAPIService.swift @@ -163,6 +163,11 @@ struct UploadMetadata { // MARK: - Companion API Service actor CompanionAPIService { + + /// Shared singleton — use this instead of creating new instances per call-site. + /// Each instance allocates its own URLSession; using the singleton ensures + /// connection pooling across all Companion API requests. + static let shared = CompanionAPIService() private let session: URLSession @@ -665,7 +670,7 @@ class ConflictManager: ObservableObject { @Published var lastError: String? @Published var isFixing: String? = nil - private let api = CompanionAPIService() + private let api = CompanionAPIService.shared var badgeCount: Int { conflicts?.errors ?? 0 } var totalCount: Int { conflicts?.total ?? 0 } diff --git a/iOS/Views/Companion/CompanionSettingsView.swift b/iOS/Views/Companion/CompanionSettingsView.swift index 2af67f6..2fb25dc 100644 --- a/iOS/Views/Companion/CompanionSettingsView.swift +++ b/iOS/Views/Companion/CompanionSettingsView.swift @@ -241,10 +241,10 @@ struct CompanionSettingsView: View { private func triggerScan() { Task { do { - try await CompanionAPIService().triggerBulkFix() - DebugLogger.shared.log("Triggered Navidrome scan", category: "Companion") + try await CompanionAPIService.shared.triggerBulkFix() + DebugLogger.shared.log("Triggered Navidrome scan", category: "Companion", level: .info) } catch { - DebugLogger.shared.log("Scan failed: \(error.localizedDescription)", category: "Companion") + DebugLogger.shared.log("Scan failed: \(error.localizedDescription)", category: "Companion", level: .warning) } } } @@ -256,12 +256,12 @@ struct CompanionSettingsView: View { fixLibraryMessage = "Restructuring library on server..." Task { do { - try await CompanionAPIService().triggerBulkFix() + try await CompanionAPIService.shared.triggerBulkFix() await MainActor.run { fixLibraryStatus = .done fixLibraryMessage = "Library restructure started — check server logs for progress." } - DebugLogger.shared.log("Fix library triggered", category: "Companion") + DebugLogger.shared.log("Fix library triggered", category: "Companion", level: .info) } catch { await MainActor.run { fixLibraryStatus = .failed @@ -297,11 +297,9 @@ struct CompanionSettingsView: View { private func testConnection() { testStatus = .testing - Task { do { - let api = CompanionAPIService() - let ok = try await api.healthCheck() + let ok = try await CompanionAPIService.shared.healthCheck() await MainActor.run { testStatus = ok ? .success : .failed("Unhealthy") } } catch let error as URLError { await MainActor.run { @@ -310,7 +308,6 @@ struct CompanionSettingsView: View { } catch { await MainActor.run { testStatus = .failed(error.localizedDescription) } } - try? await Task.sleep(for: .seconds(3)) await MainActor.run { if case .success = testStatus { testStatus = .idle } diff --git a/iOS/Views/Companion/MultiAlbumEditorSheet.swift b/iOS/Views/Companion/MultiAlbumEditorSheet.swift index 1bbb7c8..ec48da2 100644 --- a/iOS/Views/Companion/MultiAlbumEditorSheet.swift +++ b/iOS/Views/Companion/MultiAlbumEditorSheet.swift @@ -41,7 +41,7 @@ struct MultiAlbumEditorSheet: View { @State private var failedTracks: [String] = [] @ObservedObject private var settings = CompanionSettings.shared - private let api = CompanionAPIService() + private let api = CompanionAPIService.shared private let accentPink = Color(red: 1.0, green: 0.176, blue: 0.333) private var allSongs: [Song] { albums.flatMap { $0.song ?? [] } } diff --git a/iOS/Views/Companion/SmartCrossfadeManager.swift b/iOS/Views/Companion/SmartCrossfadeManager.swift index 4ca1206..e3e05a2 100644 --- a/iOS/Views/Companion/SmartCrossfadeManager.swift +++ b/iOS/Views/Companion/SmartCrossfadeManager.swift @@ -80,7 +80,7 @@ class SmartCrossfadeManager: ObservableObject { // MARK: - Profile Cache private var profiles: [String: SmartDJProfile] = [:] - private let api = CompanionAPIService() + private let api = CompanionAPIService.shared // MARK: - Callbacks var needsNextTrack: (() -> Void)? diff --git a/iOS/Views/Companion/TrackEditorView.swift b/iOS/Views/Companion/TrackEditorView.swift index 997cc6b..daf1edd 100644 --- a/iOS/Views/Companion/TrackEditorView.swift +++ b/iOS/Views/Companion/TrackEditorView.swift @@ -187,7 +187,7 @@ struct TrackEditorView: View { @ObservedObject private var settings = CompanionSettings.shared - private let api = CompanionAPIService() + private let api = CompanionAPIService.shared private let accentPink = Color(red: 1.0, green: 0.176, blue: 0.333) private var hasSelection: Bool { diff --git a/iOS/Views/Companion/ZipImportManager.swift b/iOS/Views/Companion/ZipImportManager.swift index 7046c08..1acf144 100644 --- a/iOS/Views/Companion/ZipImportManager.swift +++ b/iOS/Views/Companion/ZipImportManager.swift @@ -32,7 +32,7 @@ class ZipImportManager: NSObject, ObservableObject, URLSessionDataDelegate, URLS } // MARK: - Internal - private let api = CompanionAPIService() + private let api = CompanionAPIService.shared private let fileManager = FileManager.default private var extractDir: URL? private var backgroundSession: URLSession! diff --git a/iOS/Views/Library/AlbumDetailView.swift b/iOS/Views/Library/AlbumDetailView.swift index 9a89292..af0c6f2 100644 --- a/iOS/Views/Library/AlbumDetailView.swift +++ b/iOS/Views/Library/AlbumDetailView.swift @@ -192,7 +192,7 @@ struct AlbumDetailView: View { .cornerRadius(8) // Watch button (red = not synced, green = synced) - if WatchConnectivityManager.shared.isWatchAvailable { + if watchManager.isWatchAvailable { Button(action: { sendAlbumToWatch(album) }) { Image(systemName: "applewatch") .font(.system(size: 17)) @@ -220,7 +220,7 @@ struct AlbumDetailView: View { let available = isSongAvailable(song) let dlState = offlineManager.downloads[song.id] let isDownloaded = offlineManager.isSongDownloaded(song.id) - let isOnWatch = WatchConnectivityManager.shared.isSongOnWatch(song.id) + let isOnWatch = watchManager.isSongOnWatch(song.id) HStack(spacing: 14) { // Track number with download overlay @@ -345,23 +345,21 @@ struct AlbumDetailView: View { } // Send to Watch — always visible - let isOnWatch = WatchConnectivityManager.shared.isSongOnWatch(song.id) + let isOnWatch = watchManager.isSongOnWatch(song.id) Button(action: { - if !isOnWatch { _ = WatchConnectivityManager.shared.sendSongToWatch(song) } + if !isOnWatch { _ = watchManager.sendSongToWatch(song) } }) { Label(isOnWatch ? "On Watch ✓" : "Send to Watch", systemImage: isOnWatch ? "applewatch.checkmark" : "applewatch.and.arrow.forward") } .disabled(isOnWatch) - // Favourite + // Favourite — routes through OptimisticActionQueue (AUDIT-057) Button(action: { - Task { - if song.starred != nil { - try? await serverManager.client.unstar(id: song.id) - } else { - try? await serverManager.client.star(id: song.id) - } + if song.starred != nil { + OptimisticActionQueue.shared.unstar(songId: song.id) + } else { + OptimisticActionQueue.shared.star(songId: song.id) } }) { Label(song.starred != nil ? "Unfavourite" : "Favourite", systemImage: song.starred != nil ? "heart.slash.fill" : "heart") @@ -427,7 +425,7 @@ struct AlbumDetailView: View { private func isAlbumOnWatch(_ album: AlbumWithSongs) -> Bool { guard let songs = album.song, !songs.isEmpty else { return false } - return songs.allSatisfy { WatchConnectivityManager.shared.isSongOnWatch($0.id) } + return songs.allSatisfy { watchManager.isSongOnWatch($0.id) } } private func sendAlbumToWatch(_ album: AlbumWithSongs) { @@ -435,13 +433,13 @@ struct AlbumDetailView: View { // Download first if needed, then send each song for song in songs { if offlineManager.isSongDownloaded(song.id) { - _ = WatchConnectivityManager.shared.sendSongToWatch(song) + _ = watchManager.sendSongToWatch(song) } else if let server = serverManager.activeServer { offlineManager.downloadSong(song, server: server) // Queue watch send after download completes DispatchQueue.main.asyncAfter(deadline: .now() + 2) { if self.offlineManager.isSongDownloaded(song.id) { - _ = WatchConnectivityManager.shared.sendSongToWatch(song) + _ = watchManager.sendSongToWatch(song) } } } @@ -506,7 +504,7 @@ struct AlbumDetailView: View { } do { - let companionSongs = try await CompanionAPIService() + let companionSongs = try await CompanionAPIService.shared .fetchAlbumSongs(album: albumName, albumArtist: albumArtist) LibraryCache.shared.cacheCompanionAlbumSongs(companionSongs, albumId: albumId) let builtAlbum = buildAlbumWithSongs(name: albumName, artist: albumArtist, songs: companionSongs) diff --git a/iOS/Views/Library/MyMusicView.swift b/iOS/Views/Library/MyMusicView.swift index dbcd293..2f2dde7 100644 --- a/iOS/Views/Library/MyMusicView.swift +++ b/iOS/Views/Library/MyMusicView.swift @@ -181,7 +181,12 @@ struct MyMusicView: View { Button("Cancel", role: .cancel) { } } .task { await loadData() } - .refreshable { await loadData() } + .refreshable { refreshData() } + // Reload view data when SyncEngine completes a sync — keeps the view + // in sync with the cache without making independent server calls + .onReceive(NotificationCenter.default.publisher(for: .companionLibraryChanged)) { _ in + Task { await loadData() } + } .sheet(isPresented: $showBatchAlbumEditor) { MultiAlbumEditorSheet(albumIds: Array(selectedAlbumIds)) { isSelectingAlbums = false @@ -876,14 +881,14 @@ struct MyMusicView: View { VStack(alignment: .leading, spacing: 2) { Text(song.title) - .font(.system(size: 15)) + .font(.rowTitle) .foregroundColor( !available ? .gray.opacity(0.35) : audioPlayer.currentSong?.id == song.id ? accentPink : .white ) .lineLimit(1) Text("\(song.artist ?? "") · \(song.album ?? "")") - .font(.system(size: 12)) + .font(.rowSubtitle) .foregroundColor(available ? .gray : .gray.opacity(0.3)) .lineLimit(1) @@ -1155,86 +1160,57 @@ struct MyMusicView: View { } // MARK: - Data Loading + // + // MyMusicView reads exclusively from LibraryCache — it does NOT make its own + // server requests. SyncEngine owns all server communication and populates the + // cache on a 15-minute schedule (and immediately on Companion push events). + // + // This eliminates the duplicate full album pagination that previously ran on + // every pull-to-refresh (AUDIT-032/065): MyMusicView was making the exact same + // getAlbumList2 calls as SyncEngine, doubling network traffic and writing to + // the same cache keys concurrently. + // + // Pull-to-refresh now triggers SyncEngine.forceSync() — one code path, one set + // of API calls, one writer to LibraryCache. private func loadData() async { - let client = serverManager.client let cache = LibraryCache.shared - - // Load cached data FIRST — show instantly, never spinner on warm launch - let hadCache: Bool - if recentAlbums.isEmpty, let cached = cache.loadAlbums() { recentAlbums = cached } - if playlists.isEmpty, let cached = cache.loadPlaylists() { playlists = cached } - if artists.isEmpty, let cached = cache.loadArtists() { artists = cached } - if let cached = cache.load([Genre].self, key: "genres") { genres = cached } - if let cached = cache.load([Album].self, key: "all_albums") { allAlbums = cached } - if favouriteSongs.isEmpty, let cached = cache.load([Song].self, key: "starred_songs") { favouriteSongs = cached } - - hadCache = !recentAlbums.isEmpty || !playlists.isEmpty || !artists.isEmpty - - // If we have cache, stop showing spinner immediately - if hadCache { isLoading = false } - - // Then refresh from server in background - do { - async let albumsReq = client.getAlbumList2(type: "newest", size: 30) - async let playlistsReq = client.getPlaylists() - async let artistsReq = client.getArtists() - async let genresReq = client.getGenres() - async let starredReq = client.getStarred2() - - let (albums, playlistList, artistList, genreList, starred) = try await ( - albumsReq, playlistsReq, artistsReq, genresReq, starredReq - ) - - // Fetch ALL albums (paginated) - let albumsFull = try await fetchAllAlbums(client: client) - - cache.cacheAlbums(albums) - cache.cachePlaylists(playlistList) - cache.cacheArtists(artistList) - cache.save(genreList, key: "genres") - cache.save(albumsFull, key: "all_albums") - if let starredSongs = starred?.song { - cache.save(starredSongs, key: "starred_songs") - } - - await MainActor.run { - self.recentAlbums = albums - self.playlists = playlistList - self.artists = artistList - self.allAlbums = albumsFull - self.genres = genreList - if let starredSongs = starred?.song { - self.favouriteSongs = starredSongs - } - self.isLoading = false - } - } catch { - serverManager.handleConnectionFailure() - await MainActor.run { self.isLoading = false } + + // Always load from cache first — instant on warm launch + if recentAlbums.isEmpty, let v = cache.loadAlbums() { recentAlbums = v } + if playlists.isEmpty, let v = cache.loadPlaylists() { playlists = v } + if artists.isEmpty, let v = cache.loadArtists() { artists = v } + if let v = cache.load([Genre].self, key: "genres") { genres = v } + if let v = cache.load([Album].self, key: "all_albums") { allAlbums = v } + if favouriteSongs.isEmpty, let v = cache.load([Song].self, key: "starred_songs") { favouriteSongs = v } + + isLoading = false + + // If the cache was empty (first launch or after a clear), kick off a sync + // and wait for it to fill the cache, then reload. + let cacheWasEmpty = recentAlbums.isEmpty && playlists.isEmpty && artists.isEmpty + if cacheWasEmpty { + isLoading = true + await SyncEngine.shared.syncAndWait() + if let v = cache.loadAlbums() { recentAlbums = v } + if let v = cache.loadPlaylists() { playlists = v } + if let v = cache.loadArtists() { artists = v } + if let v = cache.load([Genre].self, key: "genres") { genres = v } + if let v = cache.load([Album].self, key: "all_albums") { allAlbums = v } + if let v = cache.load([Song].self, key: "starred_songs") { favouriteSongs = v } + isLoading = false } } - - /// Paginate all albums from the server - private func fetchAllAlbums(client: SubsonicClient) async throws -> [Album] { - var all: [Album] = [] - var offset = 0 - let pageSize = 500 - while true { - let page = try await client.getAlbumList2(type: "alphabeticalByName", size: pageSize, offset: offset) - all.append(contentsOf: page) - if page.count < pageSize { break } - offset += pageSize - } - // Sort: alpha first, then numerals, then special characters - return all.sorted { a, b in - let aFirst = a.name.first ?? Character("\0") - let bFirst = b.name.first ?? Character("\0") - let aIsLetter = aFirst.isLetter - let bIsLetter = bFirst.isLetter - if aIsLetter && !bIsLetter { return true } - if !aIsLetter && bIsLetter { return false } - return a.name.localizedCaseInsensitiveCompare(b.name) == .orderedAscending + + /// Pull-to-refresh: delegate to SyncEngine rather than running a second + /// set of paginated API calls. SyncEngine writes to LibraryCache and posts + /// companionLibraryChanged — the .onReceive below reloads the view. + private func refreshData() { + Task { + SyncEngine.shared.forceSync() + // Small wait to allow the sync to start before re-reading cache + try? await Task.sleep(nanoseconds: 500_000_000) + await loadData() } } } diff --git a/iOS/Views/NowPlaying/NowPlayingView.swift b/iOS/Views/NowPlaying/NowPlayingView.swift index 3a4a1dc..4e73976 100644 --- a/iOS/Views/NowPlaying/NowPlayingView.swift +++ b/iOS/Views/NowPlaying/NowPlayingView.swift @@ -622,12 +622,13 @@ struct NowPlayingView: View { song.starred != nil ? "heart.slash.fill" : "heart", { showEllipsisMenu = false - Task { - if song.starred != nil { - try? await serverManager.client.unstar(id: song.id) - } else { - try? await serverManager.client.star(id: song.id) - } + // Route through OptimisticActionQueue for offline + // resilience — direct try? calls silently lose the + // action when Tailscale is reconnecting (AUDIT-057) + if song.starred != nil { + OptimisticActionQueue.shared.unstar(songId: song.id) + } else { + OptimisticActionQueue.shared.star(songId: song.id) } } ), @@ -748,11 +749,11 @@ struct NowPlayingView: View { HStack { VStack(alignment: .leading, spacing: 4) { Text(audioPlayer.currentSong?.title ?? "Not Playing") - .font(.system(size: isLandscape ? 16 : 20, weight: .bold)) + .font(.npTitle) .foregroundColor(.white) .lineLimit(1) Text(audioPlayer.currentSong?.artist ?? "") - .font(.system(size: isLandscape ? 13 : 16)) + .font(.npArtist) .foregroundColor(accentPink) .lineLimit(1) .onTapGesture { @@ -783,6 +784,8 @@ struct NowPlayingView: View { .font(.system(size: isLandscape ? 18 : 22)) .foregroundColor(isStarred ? accentPink : .gray) } + .accessibilityLabel(isStarred ? "Unfavourite" : "Favourite") + .accessibilityValue(isStarred ? "Added to favourites" : "Not in favourites") } } .padding(.horizontal, isLandscape ? 20 : 30) @@ -827,6 +830,16 @@ struct NowPlayingView: View { dragPosition = pct } ) + .accessibilityLabel("Seek bar") + .accessibilityValue(formatTime(playbackTime) + " of " + formatTime(playbackDuration)) + .accessibilityAdjustableAction { direction in + let step = playbackDuration * 0.05 // 5% per swipe + switch direction { + case .increment: audioPlayer.seek(to: min(playbackDuration, playbackTime + step)) + case .decrement: audioPlayer.seek(to: max(0, playbackTime - step)) + @unknown default: break + } + } HStack { Text(formatTime(isDraggingSlider ? playbackDuration * dragPosition : playbackTime)) @@ -974,11 +987,14 @@ struct NowPlayingView: View { } .frame(width: 60, height: 50) .disabled(isLiveRadio && !canRewind30) + .accessibilityLabel("Rewind 30 seconds") } else if !isLiveRadio { Button(action: { audioPlayer.previous() }) { Image(systemName: "backward.fill") .font(.system(size: iconSize)).foregroundColor(.white) - }.frame(width: 60, height: 50) + } + .frame(width: 60, height: 50) + .accessibilityLabel("Previous track") } else { Color.clear.frame(width: 60, height: 50) } @@ -988,7 +1004,9 @@ struct NowPlayingView: View { Button(action: { audioPlayer.togglePlayPause() }) { Image(systemName: audioPlayer.isPlaying ? "pause.fill" : "play.fill") .font(.system(size: playSize)).foregroundColor(.white) - }.frame(width: 70, height: 60) + } + .frame(width: 70, height: 60) + .accessibilityLabel(audioPlayer.isPlaying ? "Pause" : "Play") Spacer() @@ -1007,11 +1025,14 @@ struct NowPlayingView: View { } .frame(width: 60, height: 50) .disabled(isLiveRadio && !canForward15) + .accessibilityLabel("Forward 15 seconds") } else if !isLiveRadio { Button(action: { audioPlayer.next() }) { Image(systemName: "forward.fill") .font(.system(size: iconSize)).foregroundColor(.white) - }.frame(width: 60, height: 50) + } + .frame(width: 60, height: 50) + .accessibilityLabel("Next track") } else { Color.clear.frame(width: 60, height: 50) }