From ba691ce94c97b93bdc72033addc3fad5f04b4b94 Mon Sep 17 00:00:00 2001 From: Gianni Carlo Date: Sat, 27 Jun 2026 09:57:02 -0500 Subject: [PATCH 1/6] guard against progress invalid values --- .../Library/ItemList/Views/PercentageProgressView.swift | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/BookPlayer/Library/ItemList/Views/PercentageProgressView.swift b/BookPlayer/Library/ItemList/Views/PercentageProgressView.swift index e069b26c1..92751f0a6 100644 --- a/BookPlayer/Library/ItemList/Views/PercentageProgressView.swift +++ b/BookPlayer/Library/ItemList/Views/PercentageProgressView.swift @@ -20,7 +20,11 @@ struct PercentageProgressView: View { } var body: some View { - Group { + // `progress` can arrive non-finite (e.g. an item whose duration is still + // 0 makes `percentCompleted` 0/0 = NaN). `Int(NaN)` traps at runtime, so + // collapse any non-finite value to 0 and clamp into the expected 0...1. + let progress = self.progress.isFinite ? min(max(self.progress, 0), 1) : 0 + return Group { if progress == 0 { EmptyView() } else if progress == 1 { From 1f9e25595967185f76f6fc5afd7f0aeb640d14b6 Mon Sep 17 00:00:00 2001 From: Gianni Carlo Date: Sat, 27 Jun 2026 10:41:25 -0500 Subject: [PATCH 2/6] Fix logout not clearing tasks --- Shared/Services/Sync/SyncService.swift | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/Shared/Services/Sync/SyncService.swift b/Shared/Services/Sync/SyncService.swift index 36839b003..9c41feff2 100644 --- a/Shared/Services/Sync/SyncService.swift +++ b/Shared/Services/Sync/SyncService.swift @@ -183,11 +183,10 @@ public final class SyncService: SyncServiceProtocol, BPLogger { func bindObservers() { NotificationCenter.default.publisher(for: .logout, object: nil) - .sink(receiveValue: { _ in - UserDefaults.standard.set( - false, - forKey: Constants.UserDefaults.hasScheduledLibraryContents - ) + .sink(receiveValue: { [weak self] _ in + /// Covers every logout path (iOS sign-out, account deletion, Watch), since + /// they all post `.logout`. Tears down the queue, the flag, and `isActive`. + Task { await self?.logout() } }) .store(in: &disposeBag) @@ -486,6 +485,18 @@ public final class SyncService: SyncServiceProtocol, BPLogger { public func resetAllJobs() async { await jobManager.resetAllJobs() } + + /// Tears down sync state when the account logs out (or is deleted): stops syncing, + /// clears the persisted task queue, and resets the "scheduled library contents" flag + /// so the next login runs a fresh initial sync from an empty queue. Idempotent. + public func logout() async { + await MainActor.run { self.isActive = false } + UserDefaults.standard.set( + false, + forKey: Constants.UserDefaults.hasScheduledLibraryContents + ) + await resetAllJobs() + } } extension SyncService { From 9b79e16c288f6a08445606cdb878593e6b29c8d1 Mon Sep 17 00:00:00 2001 From: Gianni Carlo Date: Sat, 27 Jun 2026 10:43:17 -0500 Subject: [PATCH 3/6] fix concurrency warning --- BookPlayer/Library/ItemList/ItemListView.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/BookPlayer/Library/ItemList/ItemListView.swift b/BookPlayer/Library/ItemList/ItemListView.swift index a8915abba..143b4c92f 100644 --- a/BookPlayer/Library/ItemList/ItemListView.swift +++ b/BookPlayer/Library/ItemList/ItemListView.swift @@ -83,7 +83,7 @@ struct ItemListView: View { preferencesService.register(folderUuid: uuid) } } - .onReceive(preferencesService.preferencesChanged) { key in + .onReceive(preferencesService.preferencesChanged.receive(on: DispatchQueue.main)) { key in // Server-driven sort change for the location currently on screen: // PreferencesSyncService has already rewritten orderRank in CoreData // via dispatchResort → sortContents. The cached `[SimpleLibraryItem]` From 8014a9989a8533b3a600994d6043036f3a228dd4 Mon Sep 17 00:00:00 2001 From: Gianni Carlo Date: Sat, 27 Jun 2026 10:50:50 -0500 Subject: [PATCH 4/6] fix delete bug --- Shared/SwiftData/TasksDataManager.swift | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/Shared/SwiftData/TasksDataManager.swift b/Shared/SwiftData/TasksDataManager.swift index 7da449e92..80647f52c 100644 --- a/Shared/SwiftData/TasksDataManager.swift +++ b/Shared/SwiftData/TasksDataManager.swift @@ -67,6 +67,8 @@ public final class TasksDataManager { } public func deleteAllTasks(with context: ModelContext) throws { + // Task payload models are standalone (no relationships), so a store-level + // batch delete is safe and fast. try context.delete(model: UploadTaskModel.self) try context.delete(model: UpdateTaskModel.self) try context.delete(model: MoveTaskModel.self) @@ -76,9 +78,21 @@ public final class TasksDataManager { try context.delete(model: RenameFolderTaskModel.self) try context.delete(model: ArtworkUploadTaskModel.self) try context.delete(model: MatchUuidsTaskModel.self) - try context.delete(model: SyncTaskReferenceModel.self) - try context.delete(model: SyncTasksContainer.self) - + + // SyncTaskReferenceModel has a mandatory inverse to SyncTasksContainer, which a + // store-level batch delete can't nullify — it trips a constraint-trigger / + // optimistic-lock error. Delete through the object graph instead: removing each + // container cascades to its task references and maintains the inverse in memory. + let containers = try context.fetch(FetchDescriptor()) + for container in containers { + context.delete(container) + } + // Defensively clear any references that aren't attached to a container. + let orphanedReferences = try context.fetch(FetchDescriptor()) + for reference in orphanedReferences { + context.delete(reference) + } + try context.save() } From 31e7b7055846dfda36906bbc168a7ceea3ae1440 Mon Sep 17 00:00:00 2001 From: Gianni Carlo Date: Sat, 27 Jun 2026 12:18:30 -0500 Subject: [PATCH 5/6] address feedback --- .../Generated/AutoMockable.generated.swift | 26 ++++++++++ .../Library/ItemList/Views/BookView.swift | 3 ++ BookPlayer/MainView.swift | 25 +++------ .../Profile/ProfileSyncTasksSectionView.swift | 4 +- BookPlayer/Utils/AppServices.swift | 24 +++++++-- BookPlayerWatch/CoreServices.swift | 2 +- BookPlayerWatch/ExtensionDelegate.swift | 1 + Shared/Services/Sync/SyncService.swift | 52 +++++++++++++++++-- Shared/SwiftData/TasksDataManager.swift | 9 ++-- 9 files changed, 115 insertions(+), 31 deletions(-) diff --git a/BookPlayer/Generated/AutoMockable.generated.swift b/BookPlayer/Generated/AutoMockable.generated.swift index fd3b9e1b6..12bf1ccc7 100644 --- a/BookPlayer/Generated/AutoMockable.generated.swift +++ b/BookPlayer/Generated/AutoMockable.generated.swift @@ -1687,6 +1687,32 @@ class SyncServiceProtocolMock: SyncServiceProtocol { set(value) { underlyingDownloadErrorPublisher = value } } var underlyingDownloadErrorPublisher: PassthroughSubject<(String, Error), Never>! + //MARK: - updateSyncEnabled + + var updateSyncEnabledCallsCount = 0 + var updateSyncEnabledCalled: Bool { + return updateSyncEnabledCallsCount > 0 + } + var updateSyncEnabledReceivedEnabled: Bool? + var updateSyncEnabledReceivedInvocations: [Bool] = [] + var updateSyncEnabledClosure: ((Bool) -> Void)? + func updateSyncEnabled(_ enabled: Bool) { + updateSyncEnabledCallsCount += 1 + updateSyncEnabledReceivedEnabled = enabled + updateSyncEnabledReceivedInvocations.append(enabled) + updateSyncEnabledClosure?(enabled) + } + //MARK: - logout + + var logoutCallsCount = 0 + var logoutCalled: Bool { + return logoutCallsCount > 0 + } + var logoutClosure: (() async -> Void)? + func logout() async { + logoutCallsCount += 1 + await logoutClosure?() + } //MARK: - queuedJobsCount var queuedJobsCountCallsCount = 0 diff --git a/BookPlayer/Library/ItemList/Views/BookView.swift b/BookPlayer/Library/ItemList/Views/BookView.swift index 9eb1004f4..e20ee2203 100644 --- a/BookPlayer/Library/ItemList/Views/BookView.swift +++ b/BookPlayer/Library/ItemList/Views/BookView.swift @@ -86,9 +86,12 @@ struct BookView: View { let audioMetadataService = AudioMetadataService() let libraryService = LibraryService() libraryService.setup(dataManager: dataManager, audioMetadataService: audioMetadataService) + let accountService = AccountService() + accountService.setup(dataManager: dataManager) syncService.setup( isActive: true, libraryService: libraryService, + accountService: accountService, dataManager: dataManager ) diff --git a/BookPlayer/MainView.swift b/BookPlayer/MainView.swift index 7f3a87fc0..2e491c2c0 100644 --- a/BookPlayer/MainView.swift +++ b/BookPlayer/MainView.swift @@ -19,7 +19,6 @@ struct MainView: View { @Environment(\.libraryService) private var libraryService @Environment(\.playerState) private var playerState @Environment(\.syncService) private var syncService - @Environment(\.accountService) private var accountService @Environment(\.jellyfinService) private var jellyfinService @Environment(\.audiobookshelfService) private var audiobookshelfService @Environment(\.playbackService) private var playbackService @@ -124,22 +123,14 @@ struct MainView: View { playerState.showPlayer = false } } - .onReceive( - NotificationCenter.default.publisher(for: .accountUpdate, object: nil) - ) { _ in - guard accountService.hasAccount() else { return } - - if accountService.hasSyncEnabled() { - if !syncService.isActive { - syncService.isActive = true - Task { - try? await listSyncRefreshService.syncList(at: nil) - listState.reloadAll() - } - } - } else if syncService.isActive { - syncService.isActive = false - syncService.cancelAllJobs() + // SyncService owns the active/inactive decision (it observes account/subscription + // changes itself). The view only reacts to sync becoming active to refresh the + // library list — it no longer writes `isActive`. + .onChange(of: syncService.isActive) { _, isActive in + guard isActive else { return } + Task { + try? await listSyncRefreshService.syncList(at: nil) + listState.reloadAll() } } } diff --git a/BookPlayer/Profile/Profile/ProfileSyncTasksSectionView.swift b/BookPlayer/Profile/Profile/ProfileSyncTasksSectionView.swift index c09e5dbc9..9d363608c 100644 --- a/BookPlayer/Profile/Profile/ProfileSyncTasksSectionView.swift +++ b/BookPlayer/Profile/Profile/ProfileSyncTasksSectionView.swift @@ -96,8 +96,10 @@ struct ProfileSyncTasksSectionView: View { let audioMetadataService = AudioMetadataService() let libraryService = LibraryService() libraryService.setup(dataManager: dataManager, audioMetadataService: audioMetadataService) + let accountService = AccountService() + accountService.setup(dataManager: dataManager) let syncService = SyncService() - syncService.setup(isActive: true, libraryService: libraryService, dataManager: dataManager) + syncService.setup(isActive: true, libraryService: libraryService, accountService: accountService, dataManager: dataManager) return syncService }() diff --git a/BookPlayer/Utils/AppServices.swift b/BookPlayer/Utils/AppServices.swift index 465271e63..7f3319b2c 100644 --- a/BookPlayer/Utils/AppServices.swift +++ b/BookPlayer/Utils/AppServices.swift @@ -76,7 +76,11 @@ final class AppServices: BPLogger { let accountService = makeAccountService(dataManager: dataManager) let audioMetadataService = makeAudioMetadataService() let libraryService = makeLibraryService(dataManager: dataManager, audioMetadataService: audioMetadataService) - let syncService = makeSyncService(accountService: accountService, libraryService: libraryService, dataManager: dataManager) + let syncService = makeSyncService( + accountService: accountService, + libraryService: libraryService, + dataManager: dataManager + ) let playbackService = makePlaybackService(libraryService: libraryService) let playerManager = PlayerManager( libraryService: libraryService, @@ -214,15 +218,27 @@ final class AppServices: BPLogger { return AudioMetadataService() } - private func makeLibraryService(dataManager: DataManager, audioMetadataService: AudioMetadataServiceProtocol) -> LibraryService { + private func makeLibraryService( + dataManager: DataManager, + audioMetadataService: AudioMetadataServiceProtocol + ) -> LibraryService { let service = LibraryService() service.setup(dataManager: dataManager, audioMetadataService: audioMetadataService) return service } - private func makeSyncService(accountService: AccountService, libraryService: LibraryService, dataManager: DataManager) -> SyncService { + private func makeSyncService( + accountService: AccountService, + libraryService: LibraryService, + dataManager: DataManager + ) -> SyncService { let service = SyncService() - service.setup(isActive: accountService.hasSyncEnabled(), libraryService: libraryService, dataManager: dataManager) + service.setup( + isActive: accountService.hasSyncEnabled(), + libraryService: libraryService, + accountService: accountService, + dataManager: dataManager + ) return service } diff --git a/BookPlayerWatch/CoreServices.swift b/BookPlayerWatch/CoreServices.swift index 49e8ba797..a37c700b4 100644 --- a/BookPlayerWatch/CoreServices.swift +++ b/BookPlayerWatch/CoreServices.swift @@ -48,6 +48,6 @@ class CoreServices: ObservableObject { func updateSyncEnabled(_ enabled: Bool) { hasSyncEnabled = enabled - syncService.isActive = enabled + syncService.updateSyncEnabled(enabled) } } diff --git a/BookPlayerWatch/ExtensionDelegate.swift b/BookPlayerWatch/ExtensionDelegate.swift index 49abef38e..dfefad392 100644 --- a/BookPlayerWatch/ExtensionDelegate.swift +++ b/BookPlayerWatch/ExtensionDelegate.swift @@ -75,6 +75,7 @@ class ExtensionDelegate: NSObject, WKApplicationDelegate, ObservableObject { syncService.setup( isActive: accountService.hasSyncEnabled(), libraryService: libraryService, + accountService: accountService, dataManager: dataManager ) let playbackService = PlaybackService() diff --git a/Shared/Services/Sync/SyncService.swift b/Shared/Services/Sync/SyncService.swift index 9c41feff2..5d40c8a0f 100644 --- a/Shared/Services/Sync/SyncService.swift +++ b/Shared/Services/Sync/SyncService.swift @@ -21,8 +21,15 @@ public enum BPSyncError: Error { /// sourcery: AutoMockable public protocol SyncServiceProtocol { - /// Flag to check if it can sync or not - var isActive: Bool { get set } + /// Flag to check if it can sync or not. Owned by `SyncService`; mutate it through + /// `updateSyncEnabled(_:)` / `logout()` rather than assigning directly. + var isActive: Bool { get } + /// Enable or disable syncing in response to account/subscription state. Disabling + /// also cancels any queued jobs. + func updateSyncEnabled(_ enabled: Bool) + /// Tear down sync state on logout/account deletion (stop syncing, clear the queue + /// and the scheduled-contents flag). + func logout() async /// Completion publisher for ongoing-download tasks var downloadCompletedPublisher: PassthroughSubject<(String, String, String?), Never> { get } /// Progress publisher for ongoing-download tasks @@ -101,10 +108,16 @@ public protocol SyncServiceProtocol { @Observable public final class SyncService: SyncServiceProtocol, BPLogger { private var libraryService: LibrarySyncProtocol! + private var accountService: AccountServiceProtocol! private var tasksCountService: SyncTasksCountService! var jobManager: JobSchedulerProtocol! private var client: NetworkClientProtocol! - public var isActive: Bool = false + /// Owned here: writes go through `updateSyncEnabled(_:)` / `logout()`, mutated on the + /// main actor. External callers read only. + public private(set) var isActive: Bool = false + /// In-flight logout teardown, awaited before an initial library sync re-schedules, + /// so a re-login can't begin scheduling until the queue reset has finished. + private var teardownTask: Task? /// Dictionary holding the initiating item relative path as key and the download tasks as value private var downloadTasksDictionary = [String: [URLSessionTask]]() @@ -132,11 +145,13 @@ public final class SyncService: SyncServiceProtocol, BPLogger { public func setup( isActive: Bool, libraryService: LibrarySyncProtocol, + accountService: AccountServiceProtocol, client: NetworkClientProtocol = NetworkClient(), dataManager: DataManager ) { self.isActive = isActive self.libraryService = libraryService + self.accountService = accountService let tasksDataManager = TasksDataManager() self.tasksCountService = SyncTasksCountService(tasksDataManager: tasksDataManager) self.jobManager = SyncJobScheduler(tasksDataManager: tasksDataManager, dataManager: dataManager) @@ -186,7 +201,18 @@ public final class SyncService: SyncServiceProtocol, BPLogger { .sink(receiveValue: { [weak self] _ in /// Covers every logout path (iOS sign-out, account deletion, Watch), since /// they all post `.logout`. Tears down the queue, the flag, and `isActive`. - Task { await self?.logout() } + /// Held in `teardownTask` so a re-login's initial sync awaits it first. + self?.teardownTask = Task { await self?.logout() } + }) + .store(in: &disposeBag) + + /// Sync ownership lives here, not in the views: any account/subscription change + /// re-derives whether syncing should be active. All logout paths post `.logout` + /// (handled above), so account-present-but-sync-disabled is the case we map here. + NotificationCenter.default.publisher(for: .accountUpdate, object: nil) + .sink(receiveValue: { [weak self] _ in + guard let self, self.accountService.hasAccount() else { return } + self.updateSyncEnabled(self.accountService.hasSyncEnabled()) }) .store(in: &disposeBag) @@ -254,6 +280,10 @@ public final class SyncService: SyncServiceProtocol, BPLogger { throw BookPlayerError.networkError("Sync is not enabled") } + /// Wait for any in-flight logout teardown to finish before scheduling, so a fast + /// logout→login can't have a late `resetAllJobs()` wipe freshly-scheduled jobs. + await teardownTask?.value + if await queuedJobsCount() > 0 { Self.logger.trace("Clearing orphaned tasks before initial library sync") await resetAllJobs() @@ -486,6 +516,20 @@ public final class SyncService: SyncServiceProtocol, BPLogger { await jobManager.resetAllJobs() } + /// Enables or disables syncing in response to account/subscription state. Disabling + /// also cancels any queued jobs. Idempotent — a no-op when the state is unchanged. + /// `isActive` is mutated on the main actor since it's an `@Observable` value read by + /// SwiftUI; the actual sync-content refresh is triggered by observers of `isActive`. + public func updateSyncEnabled(_ enabled: Bool) { + Task { @MainActor in + guard self.isActive != enabled else { return } + self.isActive = enabled + if !enabled { + self.cancelAllJobs() + } + } + } + /// Tears down sync state when the account logs out (or is deleted): stops syncing, /// clears the persisted task queue, and resets the "scheduled library contents" flag /// so the next login runs a fresh initial sync from an empty queue. Idempotent. diff --git a/Shared/SwiftData/TasksDataManager.swift b/Shared/SwiftData/TasksDataManager.swift index 80647f52c..99637860b 100644 --- a/Shared/SwiftData/TasksDataManager.swift +++ b/Shared/SwiftData/TasksDataManager.swift @@ -79,10 +79,11 @@ public final class TasksDataManager { try context.delete(model: ArtworkUploadTaskModel.self) try context.delete(model: MatchUuidsTaskModel.self) - // SyncTaskReferenceModel has a mandatory inverse to SyncTasksContainer, which a - // store-level batch delete can't nullify — it trips a constraint-trigger / - // optimistic-lock error. Delete through the object graph instead: removing each - // container cascades to its task references and maintains the inverse in memory. + // SyncTaskReferenceModel.container participates in a cascade relationship with + // SyncTasksContainer. A store-level batch delete runs below the object graph and + // skips relationship-maintenance (cascade/nullify) entirely, which trips a + // constraint-trigger / optimistic-lock error on that inverse. Delete through the + // object graph instead: removing each container cascades to its task references. let containers = try context.fetch(FetchDescriptor()) for container in containers { context.delete(container) From e32784db2571fb5a4dcf488c10bf62e969e469a2 Mon Sep 17 00:00:00 2001 From: Gianni Carlo Date: Sat, 27 Jun 2026 12:27:23 -0500 Subject: [PATCH 6/6] more feedback --- Shared/Services/Sync/SyncService.swift | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Shared/Services/Sync/SyncService.swift b/Shared/Services/Sync/SyncService.swift index 5d40c8a0f..b9ea20c80 100644 --- a/Shared/Services/Sync/SyncService.swift +++ b/Shared/Services/Sync/SyncService.swift @@ -265,6 +265,11 @@ public final class SyncService: SyncServiceProtocol, BPLogger { ) async throws { Self.logger.trace("Fetching list of contents") + /// Same gate as `syncLibraryContents()`: don't reconcile while a logout teardown + /// is still clearing the queue, in case a fast re-login routed here before the + /// `hasScheduledLibraryContents` flag was reset. + await teardownTask?.value + let response = try await fetchContents(at: relativePath) try await processContentsResponse(response, parentFolder: relativePath, canDelete: true)