diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 495fd90b..ddb051a3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -18,9 +18,9 @@ jobs: - name: Swiftlint run: swiftlint - name: Set Xcode version - run: sudo xcode-select -s "/Applications/Xcode_15.0.app/Contents/Developer" + run: sudo xcode-select -s "/Applications/Xcode_15.0.1.app/Contents/Developer" - name: Resolve dependencies run: xcodebuild -resolvePackageDependencies - name: Build and Run tests - run: xcodebuild -scheme BookPlayer test -testPlan Unit\ Tests -destination 'platform=iOS Simulator,name=iPhone 15,OS=17.0' + run: xcodebuild -scheme BookPlayer test -testPlan Unit\ Tests -destination 'platform=iOS Simulator,name=iPhone 15,OS=17.0.1' diff --git a/BookPlayer/Coordinators/LibraryListCoordinator.swift b/BookPlayer/Coordinators/LibraryListCoordinator.swift index 47698d8e..3dcc25c1 100644 --- a/BookPlayer/Coordinators/LibraryListCoordinator.swift +++ b/BookPlayer/Coordinators/LibraryListCoordinator.swift @@ -214,12 +214,10 @@ class LibraryListCoordinator: ItemListCoordinator, UINavigationControllerDelegat override func syncList() { Task { @MainActor in do { - let lastPlayed: SyncableItem? - if UserDefaults.standard.bool(forKey: Constants.UserDefaults.hasScheduledLibraryContents) == true { - lastPlayed = try await syncService.syncListContents(at: nil) + try await syncService.syncListContents(at: nil) } else { - lastPlayed = try await syncService.syncLibraryContents() + try await syncService.syncLibraryContents() UserDefaults.standard.set( true, @@ -228,9 +226,6 @@ class LibraryListCoordinator: ItemListCoordinator, UINavigationControllerDelegat } reloadItemsWithPadding() - if let lastPlayed { - reloadLastBook(relativePath: lastPlayed.relativePath) - } } catch BPSyncError.reloadLastBook(let relativePath) { reloadItemsWithPadding() reloadLastBook(relativePath: relativePath) diff --git a/BookPlayer/Generated/AutoMockable.generated.swift b/BookPlayer/Generated/AutoMockable.generated.swift index 00a0bdc2..21b6b135 100644 --- a/BookPlayer/Generated/AutoMockable.generated.swift +++ b/BookPlayer/Generated/AutoMockable.generated.swift @@ -1380,20 +1380,15 @@ class SyncServiceProtocolMock: SyncServiceProtocol { } var syncListContentsAtReceivedRelativePath: String? var syncListContentsAtReceivedInvocations: [String?] = [] - var syncListContentsAtReturnValue: SyncableItem? - var syncListContentsAtClosure: ((String?) async throws -> SyncableItem?)? - func syncListContents(at relativePath: String?) async throws -> SyncableItem? { + var syncListContentsAtClosure: ((String?) async throws -> Void)? + func syncListContents(at relativePath: String?) async throws { if let error = syncListContentsAtThrowableError { throw error } syncListContentsAtCallsCount += 1 syncListContentsAtReceivedRelativePath = relativePath syncListContentsAtReceivedInvocations.append(relativePath) - if let syncListContentsAtClosure = syncListContentsAtClosure { - return try await syncListContentsAtClosure(relativePath) - } else { - return syncListContentsAtReturnValue - } + try await syncListContentsAtClosure?(relativePath) } //MARK: - syncLibraryContents @@ -1402,18 +1397,13 @@ class SyncServiceProtocolMock: SyncServiceProtocol { var syncLibraryContentsCalled: Bool { return syncLibraryContentsCallsCount > 0 } - var syncLibraryContentsReturnValue: SyncableItem? - var syncLibraryContentsClosure: (() async throws -> SyncableItem?)? - func syncLibraryContents() async throws -> SyncableItem? { + var syncLibraryContentsClosure: (() async throws -> Void)? + func syncLibraryContents() async throws { if let error = syncLibraryContentsThrowableError { throw error } syncLibraryContentsCallsCount += 1 - if let syncLibraryContentsClosure = syncLibraryContentsClosure { - return try await syncLibraryContentsClosure() - } else { - return syncLibraryContentsReturnValue - } + try await syncLibraryContentsClosure?() } //MARK: - syncBookmarksList diff --git a/BookPlayer/Player/PlayerManager.swift b/BookPlayer/Player/PlayerManager.swift index a93820ec..63c69d39 100755 --- a/BookPlayer/Player/PlayerManager.swift +++ b/BookPlayer/Player/PlayerManager.swift @@ -64,6 +64,8 @@ final class PlayerManager: NSObject, PlayerManagerProtocol { @Published private var playbackQueued: Bool? /// Flag determining if it's in the process of fetching the URL for playback @Published private var isFetchingRemoteURL: Bool? + /// Prevent loop from automatic URL refreshes + private var canFetchRemoteURL = true private var hasObserverRegistered = false private var observeStatus: Bool = false { didSet { @@ -623,6 +625,9 @@ extension PlayerManager { /// Ignore play commands if there's no item loaded guard let currentItem else { return } + /// Allow refetching remote URL if the action was initiating by the user + canFetchRemoteURL = true + guard let playerItem else { /// Check if the playbable item is in the process of being set if observeStatus == false { @@ -724,37 +729,47 @@ extension PlayerManager { // swiftlint:disable block_based_kvo // Using this instead of new form, because the new one wouldn't work properly on AVPlayerItem override func observeValue(forKeyPath keyPath: String?, of object: Any?, change: [NSKeyValueChangeKey: Any]?, context: UnsafeMutableRawPointer?) { - guard let path = keyPath, - path == "status", - let item = object as? AVPlayerItem else { - super.observeValue(forKeyPath: keyPath, - of: object, - change: change, - context: context) - return - } - - guard item.status == .readyToPlay else { - if item.status == .failed { - if (item.error as? NSError)?.code == NSURLErrorResourceUnavailable, - let currentItem { - loadAndRefreshURL(item: currentItem) - } else { - playbackQueued = nil - observeStatus = false - showErrorAlert(title: "\("error_title".localized) AVPlayerItem", item.error?.localizedDescription) - } - } + guard + let path = keyPath, + path == "status", + let item = object as? AVPlayerItem + else { + super.observeValue( + forKeyPath: keyPath, + of: object, + change: change, + context: context + ) return } - self.observeStatus = false + switch item.status { + case .readyToPlay: + self.observeStatus = false - if self.playbackQueued == true { - self.play() + if self.playbackQueued == true { + self.play() + } + // Clean up flag + self.playbackQueued = nil + case .failed: + if canFetchRemoteURL, + (item.error as? NSError)?.code == NSURLErrorResourceUnavailable, + let currentItem { + loadAndRefreshURL(item: currentItem) + canFetchRemoteURL = false + } else { + playbackQueued = nil + observeStatus = false + playerItem = nil + showErrorAlert(title: "\("error_title".localized) AVPlayerItem", item.error?.localizedDescription) + } + case .unknown: + /// Do not handle .unknown states, as we're only interested in the success and failure states + fallthrough + @unknown default: + break } - // Clean up flag - self.playbackQueued = nil } // swiftlint:enable block_based_kvo diff --git a/Shared/Services/Sync/SyncService.swift b/Shared/Services/Sync/SyncService.swift index ac3eecd4..dcb84219 100644 --- a/Shared/Services/Sync/SyncService.swift +++ b/Shared/Services/Sync/SyncService.swift @@ -32,11 +32,11 @@ public protocol SyncServiceProtocol { var downloadProgressPublisher: PassthroughSubject<(String, String, String?, Double), Never> { get } /// Fetch the contents at the relativePath and override local contents with the remote repsonse - func syncListContents(at relativePath: String?) async throws -> SyncableItem? + func syncListContents(at relativePath: String?) async throws /// Fetch the synced identifiers and upload new local items /// Note: Should only be called once when the user logs in - func syncLibraryContents() async throws -> SyncableItem? + func syncLibraryContents() async throws func syncBookmarksList(relativePath: String) async throws -> [SimpleBookmark]? @@ -152,7 +152,7 @@ public final class SyncService: SyncServiceProtocol, BPLogger { public func syncListContents( at relativePath: String? - ) async throws -> SyncableItem? { + ) async throws { guard isActive else { throw BookPlayerError.networkError("Sync is not enabled") } @@ -180,11 +180,9 @@ public final class SyncService: SyncServiceProtocol, BPLogger { let response = try await fetchContents(at: relativePath) try await processContentsResponse(response, parentFolder: relativePath, canDelete: true) - - return response.lastItemPlayed } - public func syncLibraryContents() async throws -> SyncableItem? { + public func syncLibraryContents() async throws { guard isActive, UserDefaults.standard.bool(forKey: Constants.UserDefaults.hasQueuedJobs) == false @@ -205,8 +203,6 @@ public final class SyncService: SyncServiceProtocol, BPLogger { let response = try await fetchContents(at: nil) try await processContentsResponse(response, parentFolder: nil, canDelete: false) - - return response.lastItemPlayed } func processContentsResponse( @@ -250,14 +246,11 @@ public final class SyncService: SyncServiceProtocol, BPLogger { } /// Only update the time if the remote last played timestamp is greater than the local timestamp - guard - let remoteLastPlayDateTimestamp = item.lastPlayDateTimestamp, - remoteLastPlayDateTimestamp > localLastPlayDateTimestamp - else { - return + if let remoteLastPlayDateTimestamp = item.lastPlayDateTimestamp, + remoteLastPlayDateTimestamp > localLastPlayDateTimestamp { + await libraryService.updateInfo(for: item) + throw BPSyncError.reloadLastBook(item.relativePath) } - - await libraryService.updateInfo(for: item) } public func syncBookmarksList(relativePath: String) async throws -> [SimpleBookmark]? {