From 692f34d2ce9706683ce3066392073acb42d54f08 Mon Sep 17 00:00:00 2001 From: Ilyas Hallak Date: Wed, 20 Aug 2025 20:35:53 +0200 Subject: [PATCH] fix: Prevent redundant timer creation in offline sync state machine - Guard against multiple concurrent completion timers in sync state handling - Only trigger completion timer when transitioning from actual syncing state - Remove debug logging that impacted performance during scroll operations This resolves scroll performance issues introduced by excessive timer creation in the offline bookmark synchronization workflow. --- .../UI/Menu/OfflineBookmarksViewModel.swift | 215 ++++++++++-------- 1 file changed, 121 insertions(+), 94 deletions(-) diff --git a/readeck/UI/Menu/OfflineBookmarksViewModel.swift b/readeck/UI/Menu/OfflineBookmarksViewModel.swift index 011cf34..183d863 100644 --- a/readeck/UI/Menu/OfflineBookmarksViewModel.swift +++ b/readeck/UI/Menu/OfflineBookmarksViewModel.swift @@ -8,76 +8,16 @@ class OfflineBookmarksViewModel { private let syncUseCase: POfflineBookmarkSyncUseCase private var cancellables = Set() - private var successTimer: Timer? + private let successDelaySubject = PassthroughSubject() + private var completionTimerActive = false init(syncUseCase: POfflineBookmarkSyncUseCase = OfflineBookmarkSyncUseCase()) { self.syncUseCase = syncUseCase setupBindings() - updateState() + refreshState() } - private func setupBindings() { - // Observe sync state changes - syncUseCase.isSyncing - .receive(on: DispatchQueue.main) - .sink { [weak self] isSyncing in - self?.handleSyncStateChange(isSyncing: isSyncing) - } - .store(in: &cancellables) - - // Observe sync status changes - syncUseCase.syncStatus - .receive(on: DispatchQueue.main) - .sink { [weak self] status in - self?.handleSyncStatusChange(status: status) - } - .store(in: &cancellables) - - // Update count on app lifecycle events - NotificationCenter.default.publisher(for: UIApplication.willEnterForegroundNotification) - .receive(on: DispatchQueue.main) - .sink { [weak self] _ in - self?.updateState() - } - .store(in: &cancellables) - - NotificationCenter.default.publisher(for: UIApplication.didBecomeActiveNotification) - .receive(on: DispatchQueue.main) - .sink { [weak self] _ in - self?.updateState() - } - .store(in: &cancellables) - } - - func updateState() { - let count = syncUseCase.getOfflineBookmarksCount() - - switch state { - case .idle: - if count > 0 { - state = .pending(count: count) - } - case .pending: - if count > 0 { - state = .pending(count: count) - } else { - state = .idle - } - case .syncing: - // Keep syncing state, will be updated by handleSyncStateChange - break - case .success: - // Success state is temporary, handled by timer - break - case .error: - // Update count even in error state - if count > 0 { - state = .pending(count: count) - } else { - state = .idle - } - } - } + // MARK: - Public Methods func syncOfflineBookmarks() async { guard case .pending(let count) = state else { return } @@ -86,46 +26,133 @@ class OfflineBookmarksViewModel { await syncUseCase.syncOfflineBookmarks() } - private func handleSyncStateChange(isSyncing: Bool) { - if isSyncing { - // If we're not already in syncing state, transition to it - if case .pending(let count) = state { - state = .syncing(count: count, status: nil) + func refreshState() { + let currentCount = syncUseCase.getOfflineBookmarksCount() + updateStateWithCount(currentCount) + } + + // MARK: - Private Setup + + private func setupBindings() { + setupSyncBindings() + setupAppLifecycleBindings() + } + + private func setupSyncBindings() { + syncUseCase.isSyncing + .receive(on: DispatchQueue.main) + .sink { [weak self] isSyncing in + self?.handleSyncingStateChange(isSyncing) } + .store(in: &cancellables) + + syncUseCase.syncStatus + .receive(on: DispatchQueue.main) + .sink { [weak self] status in + self?.handleSyncStatusUpdate(status) + } + .store(in: &cancellables) + + // Auto-reset success state after 2 seconds + successDelaySubject + .delay(for: .seconds(2), scheduler: DispatchQueue.main) + .sink { [weak self] _ in + self?.state = .idle + } + .store(in: &cancellables) + } + + private func setupAppLifecycleBindings() { + let foregroundPublisher = NotificationCenter.default.publisher(for: UIApplication.willEnterForegroundNotification) + let activePublisher = NotificationCenter.default.publisher(for: UIApplication.didBecomeActiveNotification) + + Publishers.Merge(foregroundPublisher, activePublisher) + .receive(on: DispatchQueue.main) + .sink { [weak self] _ in + self?.refreshState() + } + .store(in: &cancellables) + } + + // MARK: - State Management + + private func updateStateWithCount(_ count: Int) { + switch state { + case .idle: + if count > 0 { + state = .pending(count: count) + } + case .pending: + state = count > 0 ? .pending(count: count) : .idle + case .syncing: + // Keep syncing state - will be updated by sync handlers + break + case .success: + // Success state is temporary - handled by timer + break + case .error: + state = count > 0 ? .pending(count: count) : .idle + } + } + + // MARK: - Sync Event Handlers + + private func handleSyncingStateChange(_ isSyncing: Bool) { + if isSyncing { + transitionToSyncingIfPending() } else { - // Sync completed - Task { @MainActor in - // Small delay to ensure count is updated - try await Task.sleep(nanoseconds: 500_000_000) - - let currentCount = syncUseCase.getOfflineBookmarksCount() - - if case .syncing(let originalCount, _) = state { - if currentCount == 0 { - // Success - all bookmarks synced - state = .success(syncedCount: originalCount) - - // Auto-hide success message after 2 seconds - successTimer?.invalidate() - successTimer = Timer.scheduledTimer(withTimeInterval: 2.0, repeats: false) { [weak self] _ in - self?.state = .idle - } - } else { - // Some bookmarks remain - state = .pending(count: currentCount) - } - } + // Only handle completion if we were actually syncing + if case .syncing = state { + handleSyncCompletion() } } } - private func handleSyncStatusChange(status: String?) { + private func transitionToSyncingIfPending() { + if case .pending(let count) = state { + state = .syncing(count: count, status: nil) + } + } + + private func handleSyncCompletion() { + guard !completionTimerActive else { + return + } + + completionTimerActive = true + + // wait for 0.5 seconds + Timer.publish(every: 0.5, on: .main, in: .common) + .autoconnect() + .first() + .sink { [weak self] _ in + guard let self = self else { return } + + self.completionTimerActive = false + + guard case .syncing(let originalCount, _) = self.state else { + return + } + + let remainingCount = self.syncUseCase.getOfflineBookmarksCount() + + if remainingCount == 0 { + self.state = .success(syncedCount: originalCount) + self.successDelaySubject.send(originalCount) + } else { + self.state = .pending(count: remainingCount) + } + } + .store(in: &cancellables) + } + + private func handleSyncStatusUpdate(_ status: String?) { if case .syncing(let count, _) = state { state = .syncing(count: count, status: status) } } deinit { - successTimer?.invalidate() + cancellables.removeAll() } -} \ No newline at end of file +}