Merge pull request #1559 from TortugaPower/fix/crash-library-display

Fix crash and logout sequence

Author: GianniCarlo

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

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]`

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
     )
 

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 {

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()
       }
     }
   }

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
   }()

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
   }
 

BookPlayerWatch/CoreServices.swift

@@ -48,6 +48,6 @@ class CoreServices: ObservableObject {
 
   func updateSyncEnabled(_ enabled: Bool) {
     hasSyncEnabled = enabled
-    syncService.isActive = enabled
+    syncService.updateSyncEnabled(enabled)
   }
 }

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()

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<Void, Never>?
 
   /// 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)
@@ -183,11 +198,21 @@ 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`.
+        /// 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)
 
@@ -240,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)
@@ -255,6 +285,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 +520,32 @@ public final class SyncService: SyncServiceProtocol, BPLogger {
   public func resetAllJobs() async {
     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.
+  public func logout() async {
+    await MainActor.run { self.isActive = false }
+    UserDefaults.standard.set(
+      false,
+      forKey: Constants.UserDefaults.hasScheduledLibraryContents
+    )
+    await resetAllJobs()
+  }
 }
 
 extension SyncService {