diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index db18ab6..a8fb0ea 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -38,7 +38,14 @@ serialized Effects flow out. The core never touches UI or I/O directly. pollux/ ├── shared/ # Rust core (Crux app) │ ├── src/ -│ │ ├── app.rs # App trait impl: Event, Model, ViewModel, Effect +│ │ ├── app.rs # App trait impl: Event, update(), view() +│ │ ├── model.rs # Model — private core state +│ │ ├── view_model.rs # ViewModel — serializable projection for the shell +│ │ ├── effect.rs # Effect enum (Render / Storage / Http / Download) +│ │ ├── capabilities/ # Storage, Http, Download operation & result types +│ │ ├── domain/ # Episode, Subscription +│ │ ├── feed_parser.rs # RSS → Subscription + Episodes +│ │ ├── theme.rs # base16 themes → ThemeView │ │ ├── ffi.rs # CoreFFI — the C/UniFFI/WASM bridge struct │ │ ├── lib.rs # Crate root; re-exports; UniFFI scaffolding │ │ └── bin/ @@ -46,9 +53,12 @@ pollux/ │ └── Cargo.toml ├── iOS/ │ ├── Pollux/ # SwiftUI app -│ │ ├── Pollux.swift # @main entry point -│ │ ├── ContentView.swift -│ │ └── core.swift # Core ObservableObject: drives CoreFFI +│ │ ├── Pollux.swift # @main entry point +│ │ ├── ContentView.swift # library + subscribe flow +│ │ ├── core.swift # Core ObservableObject: drives CoreFFI +│ │ ├── DatabaseManager.swift # Storage effect handler (GRDB/SQLite) +│ │ ├── DownloadManager.swift # Download effect handler (URLSession) +│ │ └── Theme.swift # ThemeView → SwiftUI colors │ ├── generated/ # ← git-ignored; produced by `make typegen` + `make package` │ │ ├── App/ # Swift type stubs (Event, ViewModel, Request, …) │ │ └── Shared/ # Swift package wrapping the compiled Rust static lib @@ -89,10 +99,10 @@ Event → update() → Command | Type | Role | |---|---| | `Pollux` | Zero-sized struct implementing `crux_core::App` | -| `Event` | All actions the UI can trigger (`Increment`, `Decrement`, `Reset` today) | -| `Model` | Private application state (never leaves the core) | -| `ViewModel` | Public, serializable snapshot of state for the shell to render | -| `Effect` | Side-effects the shell must handle (`Render` today; later: network, storage, …) | +| `Event` | All inputs the shell feeds in — user actions (subscribe/refresh a feed, select a subscription, sort/download/cancel an episode, set the theme) and the async results of storage/HTTP/download effects | +| `Model` | Private application state (never leaves the core); see `model.rs` | +| `ViewModel` | Public, serializable snapshot of state for the shell to render; see `view_model.rs` | +| `Effect` | Side-effects the shell must handle: `Render`, `Storage` (SQLite via GRDB), `Http` (feed fetch), `Download` (episode audio); see `effect.rs` | `Model` and `ViewModel` are deliberately separate. `Model` is private and mutable; `ViewModel` is a derived, serializable read-only projection. @@ -108,9 +118,10 @@ mutable; `ViewModel` is a derived, serializable read-only projection. effect - `view() -> Vec` — get the current bincode-serialized `ViewModel` -The shell is responsible for deserializing effects and dispatching them. Simple -synchronous effects (like `Render`) are handled inline. Future async effects -(network, disk) will be dispatched to platform APIs and resolved via `resolve()`. +The shell is responsible for deserializing effects and dispatching them. The +synchronous `Render` effect is handled inline; the async `Storage`, `Http`, and +`Download` effects are dispatched to platform APIs and their results delivered +back via `resolve()`. ### iOS shell (`iOS/Pollux/core.swift`) @@ -119,8 +130,10 @@ synchronous effects (like `Render`) are handled inline. Future async effects 1. Serializes the `Event` with bincode 2. Calls `CoreFFI.update(data:)` 3. Deserializes the returned `[Request]` -4. Dispatches each `Request` via `processEffect(_:)` — currently only - `.render`, which deserializes the `ViewModel` and publishes it +4. Dispatches each `Request` via `processEffect(_:)`: `.render` deserializes and + publishes the `ViewModel`; `.storage`, `.http`, and `.download` run their + async work and feed the result back through `resolve()`. Storage runs on a + single serial consumer so writes apply in submission order. SwiftUI views observe `core.view` and re-render automatically. @@ -143,8 +156,9 @@ make generate-project → xcodegen --spec iOS/project.yml regenerates iOS/Pollux.xcodeproj ``` -`make ios-build` runs all three in order. Run it whenever `shared/src/app.rs` -changes (new events, new view model fields). +`make ios-build` runs all three in order. Run it whenever the core's public +types change — new `Event`/`Effect` variants, `ViewModel` fields, or capability +operation/result types in `shared/src/`. Type serialization uses **bincode** (compact binary) at the FFI boundary and **serde** for all types. `facet` is used to drive type generation. @@ -174,7 +188,7 @@ GitHub Actions runs on push and PR to `main`: - **Rust checks**: `cargo check`, `cargo test`, `cargo clippy -D warnings`, `cargo fmt --check`, `cargo check --locked` - **Swift checks**: `swiftlint lint --strict`, `swiftformat --lint` -- **Admin**: forbidden-pattern scan (no no lint suppressions, no `unsafe`, no +- **Admin**: forbidden-pattern scan (no lint suppressions, no `unsafe`, no force-unwrap calls, no force-try/cast in Swift, etc.) All CI targets in `.github/workflows/` are reusable `workflow_call` workflows @@ -184,14 +198,21 @@ composed by `rust.yml`, `swift.yml`, and `admin.yml`. ## Current State -The core is a counter app (Increment / Decrement / Reset). That is scaffolding -only — the real domain model has not been implemented yet. Feature intent and -priorities are documented in `docs/features/`; the implementation order is in -`docs/ROADMAP.md`. +The MVP foundation is in place: RSS feed parsing and subscribing, a library and +per-feed episode list, and an offline **download manager** (serial queue, live +progress, resume-on-launch, cancel). A base16 **theming** system is wired +through the `ViewModel`. State lives in the Rust core; the iOS shell handles +SQLite (GRDB), HTTP, downloads (URLSession), and rendering. + +Still to come (see `docs/ROADMAP.md`): audio playback, playlists (the core +differentiator), a Settings screen, and feed refresh. Sync is out of scope for +MVP and v1.0. Feature intent and priorities live in `docs/features/`. When adding a new domain feature, the typical change surface is: -1. Extend `Event`, `Model`, `ViewModel`, and/or `Effect` in `shared/src/app.rs` +1. Extend the core types in `shared/src/` — `Event`/`update()`/`view()` in + `app.rs`, plus `model.rs`, `view_model.rs`, `effect.rs`, and `capabilities/` + as needed 2. Run `make ios-build` to regenerate Swift types and rebuild the Xcode project 3. Update SwiftUI views in `iOS/Pollux/` to consume the new `ViewModel` fields and emit the new `Event` variants diff --git a/docs/ROADMAP.md b/docs/ROADMAP.md index a2973e2..411b56b 100644 --- a/docs/ROADMAP.md +++ b/docs/ROADMAP.md @@ -104,6 +104,9 @@ Rounding out MVP to a complete, shippable state. - **Storage UI**: usage breakdown by subscription, limit configuration - **Subscription unsubscribe**: warning with episode count, hard delete, playlist re-evaluation on removal +- **Download queue management**: downloads page, user re-ordering, and a + persisted enqueue order so the serial queue restores FIFO across relaunch + (today it rebuilds by pub date); groundwork for parallel downloads - **Subscription-level download rules**: cascading defaults (global → feed) - **Settings completion**: all known settings wired up, cascading defaults pattern throughout diff --git a/iOS/Pollux/DatabaseManager.swift b/iOS/Pollux/DatabaseManager.swift index 5bb14c7..722045b 100644 --- a/iOS/Pollux/DatabaseManager.swift +++ b/iOS/Pollux/DatabaseManager.swift @@ -63,7 +63,6 @@ actor DatabaseManager { t.column("playback_status", .text).notNull().defaults(to: "Unplayed") t.column("playback_position_secs", .integer).check(sql: "playback_position_secs >= 0") t.column("download_status", .text).notNull().defaults(to: "NotDownloaded") - t.column("download_progress", .integer).check(sql: "download_progress BETWEEN 0 AND 100") t.column("is_flagged", .boolean).notNull().defaults(to: false) t.column("file_size_bytes", .integer).check(sql: "file_size_bytes >= 0") t.column("local_path", .text) @@ -82,6 +81,9 @@ actor DatabaseManager { @discardableResult func execute(_ operation: StorageOperation) async throws -> StorageResult { + // Split by domain so neither dispatcher trips the cyclomatic-complexity + // limit as operations are added. Subscription/feed ops here; episode ops in + // `executeEpisode`. switch operation { case .listSubscriptions: try listSubscriptions() @@ -91,18 +93,35 @@ actor DatabaseManager { try await upsertSubscription(sub) case let .deleteSubscription(id): try await deleteSubscription(id: id) + case let .upsertFeedWithEpisodes(subscription, episodes): + try await upsertFeedWithEpisodes(subscription: subscription, episodes: episodes) + default: + try await executeEpisode(operation) + } + } + + /// Episode-scoped storage operations. `execute` routes everything it does not + /// handle here; the `default` is unreachable (every remaining case is covered). + private func executeEpisode(_ operation: StorageOperation) async throws -> StorageResult { + switch operation { case let .upsertEpisode(episode): try await upsertEpisode(episode) case let .getEpisode(id): try getEpisode(id: id) case let .listEpisodesBySubscription(subscriptionId): try listEpisodesBySubscription(subscriptionId: subscriptionId) + case .listPendingDownloads: + try listPendingDownloads() case let .getEpisodeByFeedGuid(subscriptionId, feedGuid): try getEpisodeByFeedGuid(subscriptionId: subscriptionId, feedGuid: feedGuid) case let .updatePlaybackStatus(episodeId, status, positionSecs): try await updatePlaybackStatus(episodeId: episodeId, status: status, positionSecs: positionSecs) - case let .upsertFeedWithEpisodes(subscription, episodes): - try await upsertFeedWithEpisodes(subscription: subscription, episodes: episodes) + case let .updateDownloadState(episodeId, status, localPath, sizeBytes): + try await updateDownloadState( + episodeId: episodeId, status: status, localPath: localPath, sizeBytes: sizeBytes, + ) + default: + fatalError("executeEpisode received a non-episode operation: \(operation)") } } @@ -191,6 +210,26 @@ actor DatabaseManager { return .episodes(rows.map(Self.episode(from:))) } + private func listPendingDownloads() throws -> StorageResult { + // Known limitation: enqueue order isn't persisted, so a relaunch rebuilds the + // queue by pub_date rather than the order the user queued them (and may start a + // previously-Queued item before the one that was actively Downloading). Fine for + // the serial MVP; revisit when the download queue gains explicit ordering — a + // persisted sequence column, restoring the active item first — alongside the + // downloads page / re-ordering / parallel-downloads work (see ROADMAP Phase 5). + let rows = try db.read { db -> [Row] in + try Row.fetchAll( + db, + sql: """ + SELECT * FROM episodes + WHERE download_status IN ('Downloading', 'Queued') + ORDER BY pub_date + """, + ) + } + return .episodes(rows.map(Self.episode(from:))) + } + private func getEpisodeByFeedGuid(subscriptionId: String, feedGuid: String) throws -> StorageResult { let row = try db.read { db -> Row? in try Row.fetchOne( @@ -220,146 +259,26 @@ actor DatabaseManager { } return .success } -} - -// MARK: - Row mapping + status conversion - -private extension DatabaseManager { - static func upsertSubscriptionRow(_ subscription: Subscription, db: Database) throws { - try db.execute( - sql: """ - INSERT INTO subscriptions - (id, feed_url, title, artwork_url, description, last_refreshed, created_at) - VALUES (?, ?, ?, ?, ?, ?, ?) - ON CONFLICT(feed_url) DO UPDATE SET - title = excluded.title, - artwork_url = excluded.artwork_url, - description = excluded.description, - last_refreshed = excluded.last_refreshed - """, - arguments: [ - subscription.id, subscription.feedUrl, subscription.title, - subscription.artworkUrl, subscription.description, - subscription.lastRefreshed, subscription.createdAt, - ], - ) - } - static func upsertEpisodeRow(_ episode: Episode, subscriptionId: String, db: Database) throws { - let playbackStr = playbackStatusString(episode.playbackStatus) - let downloadStr = downloadStatusString(episode.downloadStatus) - let downloadProgress = episode.downloadProgress.map { Int32($0) } - try db.execute( - sql: """ - INSERT INTO episodes - (id, feed_guid, subscription_id, title, description, pub_date, - duration_secs, enclosure_url, artwork_url, playback_status, - playback_position_secs, download_status, download_progress, - is_flagged, file_size_bytes, local_path) - VALUES (?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?) - ON CONFLICT(subscription_id, feed_guid) DO UPDATE SET - title = excluded.title, - description = excluded.description, - enclosure_url = excluded.enclosure_url, - artwork_url = excluded.artwork_url, - pub_date = excluded.pub_date, - duration_secs = excluded.duration_secs, - file_size_bytes = excluded.file_size_bytes - """, - arguments: [ - episode.id, episode.feedGuid, subscriptionId, - episode.title, episode.description, - episode.pubDate, episode.durationSecs, - episode.enclosureUrl, episode.artworkUrl, - playbackStr, episode.playbackPositionSecs, - downloadStr, downloadProgress, - episode.isFlagged, - episode.fileSizeBytes.flatMap { Int64(exactly: $0) }, - episode.localPath, - ], - ) - } - - static func subscription(from row: Row) -> Subscription { - Subscription( - id: row["id"], - feedUrl: row["feed_url"], - title: row["title"], - artworkUrl: row["artwork_url"], - description: row["description"], - lastRefreshed: row["last_refreshed"], - createdAt: row["created_at"], - ) - } - - static func episode(from row: Row) -> Episode { - // Checked conversions: a stored value outside the target type's range - // (written by a later schema or external tooling) reads back as nil - // rather than trapping or silently wrapping — matching the write path, - // which stores NULL on overflow (see fileSizeBytes in upsertEpisodeRow). - Episode( - id: row["id"], - feedGuid: row["feed_guid"], - subscriptionId: row["subscription_id"], - title: row["title"], - description: row["description"], - pubDate: row["pub_date"], - durationSecs: (row["duration_secs"] as Int64?).flatMap { UInt32(exactly: $0) }, - enclosureUrl: row["enclosure_url"], - artworkUrl: row["artwork_url"], - playbackStatus: playbackStatus(from: row["playback_status"]), - playbackPositionSecs: (row["playback_position_secs"] as Int64?).flatMap { UInt32(exactly: $0) }, - downloadStatus: downloadStatus(from: row["download_status"]), - downloadProgress: (row["download_progress"] as Int32?).flatMap { UInt8(exactly: $0) }, - isFlagged: row["is_flagged"], - fileSizeBytes: (row["file_size_bytes"] as Int64?).map { UInt64(bitPattern: $0) }, - localPath: row["local_path"], - ) - } - - static func playbackStatusString(_ status: PlaybackStatus) -> String { - switch status { - case .unplayed: "Unplayed" - case .inProgress: "InProgress" - case .played: "Played" - } - } - - static func playbackStatus(from string: String) -> PlaybackStatus { - switch string { - case "Unplayed": .unplayed - case "InProgress": .inProgress - case "Played": .played - default: fatalError( - "Unknown PlaybackStatus in DB: '\(string)' — add a case to " - + "playbackStatus(from:) and playbackStatusString(_:)", - ) - } - } - - static func downloadStatusString(_ status: DownloadStatus) -> String { - switch status { - case .notDownloaded: "NotDownloaded" - case .queued: "Queued" - case .downloading: "Downloading" - case .downloaded: "Downloaded" - case .failed: "Failed" - case .removedFromFeed: "RemovedFromFeed" - } - } - - static func downloadStatus(from string: String) -> DownloadStatus { - switch string { - case "NotDownloaded": .notDownloaded - case "Queued": .queued - case "Downloading": .downloading - case "Downloaded": .downloaded - case "Failed": .failed - case "RemovedFromFeed": .removedFromFeed - default: fatalError( - "Unknown DownloadStatus in DB: '\(string)' — add a case to " - + "downloadStatus(from:) and downloadStatusString(_:)", + /// Persists a download-state transition. `localPath`/`sizeBytes` are written + /// verbatim — passing nil clears the column to NULL, matching the core, which + /// sends nil to wipe stale file metadata on delete/failure. Size overflow stores + /// NULL (as in `upsertEpisodeRow`) rather than trapping. + private func updateDownloadState( + episodeId: String, status: DownloadStatus, localPath: String?, sizeBytes: UInt64?, + ) async throws -> StorageResult { + let statusStr = Self.downloadStatusString(status) + let size = sizeBytes.flatMap { Int64(exactly: $0) } + try await db.write { db in + try db.execute( + sql: """ + UPDATE episodes + SET download_status = ?, local_path = ?, file_size_bytes = ? + WHERE id = ? + """, + arguments: [statusStr, localPath, size, episodeId], ) } + return .success } } diff --git a/iOS/Pollux/DatabaseManagerRows.swift b/iOS/Pollux/DatabaseManagerRows.swift new file mode 100644 index 0000000..04723ea --- /dev/null +++ b/iOS/Pollux/DatabaseManagerRows.swift @@ -0,0 +1,151 @@ +import App +import Foundation +import GRDB + +/// Row mapping + status conversion for `DatabaseManager`. Split out of the main file +/// to keep each under the file-length limit; these are module-internal helpers, not +/// part of any public surface. +extension DatabaseManager { + static func upsertSubscriptionRow(_ subscription: Subscription, db: Database) throws { + try db.execute( + sql: """ + INSERT INTO subscriptions + (id, feed_url, title, artwork_url, description, last_refreshed, created_at) + VALUES (?, ?, ?, ?, ?, ?, ?) + ON CONFLICT(feed_url) DO UPDATE SET + title = excluded.title, + artwork_url = excluded.artwork_url, + description = excluded.description, + last_refreshed = excluded.last_refreshed + """, + arguments: [ + subscription.id, subscription.feedUrl, subscription.title, + subscription.artworkUrl, subscription.description, + subscription.lastRefreshed, subscription.createdAt, + ], + ) + } + + static func upsertEpisodeRow(_ episode: Episode, subscriptionId: String, db: Database) throws { + let playbackStr = playbackStatusString(episode.playbackStatus) + let downloadStr = downloadStatusString(episode.downloadStatus) + try db.execute( + sql: """ + INSERT INTO episodes + (id, feed_guid, subscription_id, title, description, pub_date, + duration_secs, enclosure_url, artwork_url, playback_status, + playback_position_secs, download_status, + is_flagged, file_size_bytes, local_path) + VALUES (?,?,?,?,?,?,?,?,?,?,?,?,?,?,?) + ON CONFLICT(subscription_id, feed_guid) DO UPDATE SET + title = excluded.title, + description = excluded.description, + enclosure_url = excluded.enclosure_url, + artwork_url = excluded.artwork_url, + pub_date = excluded.pub_date, + duration_secs = excluded.duration_secs, + -- note(refresh): download_status/local_path are preserved on + -- conflict, but file_size_bytes is overwritten with the feed's + -- advertised . For a downloaded episode this clobbers + -- the real on-disk size that updateDownloadState recorded. There's no + -- refresh feature yet, so it can't fire today; when feed refresh lands, + -- stop overwriting file_size_bytes for rows whose download_status is + -- Downloaded (or drop it from this SET and let the download path own it). + file_size_bytes = excluded.file_size_bytes + """, + arguments: [ + episode.id, episode.feedGuid, subscriptionId, + episode.title, episode.description, + episode.pubDate, episode.durationSecs, + episode.enclosureUrl, episode.artworkUrl, + playbackStr, episode.playbackPositionSecs, + downloadStr, + episode.isFlagged, + episode.fileSizeBytes.flatMap { Int64(exactly: $0) }, + episode.localPath, + ], + ) + } + + static func subscription(from row: Row) -> Subscription { + Subscription( + id: row["id"], + feedUrl: row["feed_url"], + title: row["title"], + artworkUrl: row["artwork_url"], + description: row["description"], + lastRefreshed: row["last_refreshed"], + createdAt: row["created_at"], + ) + } + + static func episode(from row: Row) -> Episode { + // Checked conversions: a stored value outside the target type's range + // (written by a later schema or external tooling) reads back as nil + // rather than trapping or silently wrapping — matching the write path, + // which stores NULL on overflow (see fileSizeBytes in upsertEpisodeRow). + Episode( + id: row["id"], + feedGuid: row["feed_guid"], + subscriptionId: row["subscription_id"], + title: row["title"], + description: row["description"], + pubDate: row["pub_date"], + durationSecs: (row["duration_secs"] as Int64?).flatMap { UInt32(exactly: $0) }, + enclosureUrl: row["enclosure_url"], + artworkUrl: row["artwork_url"], + playbackStatus: playbackStatus(from: row["playback_status"]), + playbackPositionSecs: (row["playback_position_secs"] as Int64?).flatMap { UInt32(exactly: $0) }, + downloadStatus: downloadStatus(from: row["download_status"]), + isFlagged: row["is_flagged"], + fileSizeBytes: (row["file_size_bytes"] as Int64?).map { UInt64(bitPattern: $0) }, + localPath: row["local_path"], + ) + } + + static func playbackStatusString(_ status: PlaybackStatus) -> String { + switch status { + case .unplayed: "Unplayed" + case .inProgress: "InProgress" + case .played: "Played" + } + } + + static func playbackStatus(from string: String) -> PlaybackStatus { + switch string { + case "Unplayed": .unplayed + case "InProgress": .inProgress + case "Played": .played + default: fatalError( + "Unknown PlaybackStatus in DB: '\(string)' — add a case to " + + "playbackStatus(from:) and playbackStatusString(_:)", + ) + } + } + + static func downloadStatusString(_ status: DownloadStatus) -> String { + switch status { + case .notDownloaded: "NotDownloaded" + case .queued: "Queued" + case .downloading: "Downloading" + case .downloaded: "Downloaded" + case .failed: "Failed" + case .removedFromFeed: "RemovedFromFeed" + } + } + + static func downloadStatus(from string: String) -> DownloadStatus { + switch string { + case "NotDownloaded": .notDownloaded + case "Queued": .queued + case "Downloading": .downloading + case "Downloaded": .downloaded + case "Failed": .failed + case "RemovedFromFeed": .removedFromFeed + default: fatalError( + "Unknown DownloadStatus in DB: '\(string)' — add a case to " + + "downloadStatus(from:) and downloadStatusString(_:)", + ) + } + } +} diff --git a/iOS/Pollux/DownloadManager.swift b/iOS/Pollux/DownloadManager.swift new file mode 100644 index 0000000..fdc71e6 --- /dev/null +++ b/iOS/Pollux/DownloadManager.swift @@ -0,0 +1,342 @@ +import App +import CryptoKit +import Foundation + +/// Errors from the download shell. Carried in `DownloadResult.error` back to the +/// core, which turns them into a `Failed` episode row. +enum DownloadManagerError: Error, LocalizedError { + case storageUnavailable + case invalidURL(String) + case badStatus(Int) + case incomplete + + var errorDescription: String? { + switch self { + case .storageUnavailable: + "device storage is unavailable" + case let .invalidURL(url): + "invalid download URL: \(url)" + case let .badStatus(code): + "download failed: HTTP \(code)" + case .incomplete: + "download ended without a file" + } + } +} + +/// Performs the actual episode-audio I/O for the `Download` capability: fetching a +/// file to local storage and removing it again. The core owns *when* to download +/// (and the serial queue); this type only carries out one operation at a time and +/// reports the outcome. +/// +/// Files live under `/Downloads/`. The path returned to the +/// core (and stored in the DB) is **relative** to Application Support, so it keeps +/// resolving after an OS container-path change; `absoluteURL(for:)` turns it back +/// into a usable URL. +actor DownloadManager { + private let storageRoot: URL + private let downloadsDir: URL + /// Configuration for the download session. Injectable so tests can register a + /// stub `URLProtocol` instead of hitting the network. + private let sessionConfiguration: URLSessionConfiguration + /// Progress-report throttle passed to the session delegate (see there). Injectable + /// so tests don't depend on a real one-second wall-clock gap. + private let reportInterval: TimeInterval + + /// The in-flight download's task and episode, so a Cancel operation can find and + /// cancel it. At most one at a time (downloads are serial). Set before the await + /// and cleared when it returns. + private var activeTask: (episodeId: String, task: URLSessionDownloadTask)? + + /// Subdirectory (and stored-path prefix) for downloaded audio. + private static let subdirectory = "Downloads" + + /// - Parameters: + /// - storageRoot: where downloads live; defaults to Application Support. Tests + /// pass a temporary directory. + /// - sessionConfiguration: URLSession configuration; tests register a stub + /// `URLProtocol` here. + /// - reportInterval: throttle for progress reports on unknown-size downloads. + init( + storageRoot: URL? = nil, + sessionConfiguration: URLSessionConfiguration = .default, + reportInterval: TimeInterval = 1, + ) throws { + let root: URL + if let storageRoot { + root = storageRoot + } else { + guard let support = FileManager.default.urls( + for: .applicationSupportDirectory, in: .userDomainMask, + ).first else { + throw DownloadManagerError.storageUnavailable + } + root = support + } + self.storageRoot = root + downloadsDir = root.appendingPathComponent(Self.subdirectory, isDirectory: true) + self.sessionConfiguration = sessionConfiguration + self.reportInterval = reportInterval + } + + /// `onProgress(received, total)` is called with byte counts while a download runs + /// (total is nil when the server didn't report a size). It never fires for a delete. + func perform( + _ operation: DownloadOperation, + onProgress: @escaping @Sendable (UInt64, UInt64?) -> Void, + ) async -> DownloadResult { + switch operation { + case let .download(episodeId, url): + await download(episodeId: episodeId, urlString: url, onProgress: onProgress) + case let .delete(localPath): + delete(localPath: localPath) + case let .cancel(episodeId): + cancel(episodeId: episodeId) + } + } + + // MARK: - Download + + private func download( + episodeId: String, urlString: String, + onProgress: @escaping @Sendable (UInt64, UInt64?) -> Void, + ) async -> DownloadResult { + guard let url = URL(string: urlString) else { + return .error(DownloadManagerError.invalidURL(urlString).localizedDescription) + } + do { + try ensureDownloadsDirectory() + let fileName = Self.fileName(episodeId: episodeId, url: url) + let destination = downloadsDir.appendingPathComponent(fileName) + + // A session-level delegate is required for byte progress: the async + // download(from:delegate:) convenience never invokes didWriteData. The + // delegate reports progress, moves the finished temp file to `destination` + // (the system temp file is valid only inside didFinishDownloadingTo), and + // resumes the await; a non-2xx response fails the download. + let delegate = DownloadSessionDelegate( + destination: destination, reportInterval: reportInterval, onProgress: onProgress, + ) + let session = URLSession( + configuration: sessionConfiguration, delegate: delegate, delegateQueue: nil, + ) + let task = session.downloadTask(with: url) + // Registered before the await so a Cancel can reach it; cleared after. + activeTask = (episodeId: episodeId, task: task) + defer { + activeTask = nil + session.finishTasksAndInvalidate() + } + + // The delegate reports the completed file's byte count, so there's no + // filesystem re-read that could fail and record a size of 0. + let (_, size): (URL, UInt64) = try await withCheckedThrowingContinuation { continuation in + delegate.attach(continuation) + task.resume() + } + + let relativePath = "\(Self.subdirectory)/\(fileName)" + return .completed(localPath: relativePath, sizeBytes: size) + } catch { + // A cancelled task throws URLError.cancelled; URLSession discards its own + // temp file, and we clear any file that reached `destination` to be sure. + if (error as? URLError)?.code == .cancelled { + let fileName = Self.fileName(episodeId: episodeId, url: url) + try? FileManager.default.removeItem( + at: downloadsDir.appendingPathComponent(fileName), + ) + return .cancelled + } + return .error(error.localizedDescription) + } + } + + // MARK: - Cancel + + private func cancel(episodeId: String) -> DownloadResult { + // Cancelling the task makes the in-flight download throw URLError.cancelled, + // which resolves the Download request as `.cancelled`; that is where the real + // state change happens, so this just acks. + if let activeTask, activeTask.episodeId == episodeId { + activeTask.task.cancel() + } + return .cancelled + } + + // MARK: - Delete + + private func delete(localPath: String) -> DownloadResult { + let target = absoluteURL(for: localPath) + // Idempotent: a file that is already gone is a successful delete. + guard FileManager.default.fileExists(atPath: target.path) else { + return .deleted + } + do { + try FileManager.default.removeItem(at: target) + return .deleted + } catch { + return .error(error.localizedDescription) + } + } + + // MARK: - Paths + + /// Creates the downloads directory if needed and marks it excluded from device + /// backups. Downloaded audio is reproducible, so it shouldn't consume the user's + /// iCloud backup quota or bloat their backups. The exclusion is best-effort — a + /// failure to set the flag doesn't fail the download. + private func ensureDownloadsDirectory() throws { + try FileManager.default.createDirectory( + at: downloadsDir, withIntermediateDirectories: true, + ) + var dir = downloadsDir + var values = URLResourceValues() + values.isExcludedFromBackup = true + try? dir.setResourceValues(values) + } + + private func absoluteURL(for relativePath: String) -> URL { + storageRoot.appendingPathComponent(relativePath) + } + + /// A filesystem-safe, stable, collision-free file name for an episode. The episode + /// id can in principle be any string (GUID, URL, hash), so the base is a SHA-256 + /// digest of the id rather than a lossy character replacement — two distinct ids + /// that would sanitize to the same name (e.g. `ep-1` and `ep_1`) get distinct + /// files. The same id always maps to the same name; the enclosure's file extension + /// is preserved when present so players see a familiar type. + /// Not `private` so it can be unit-tested directly. + static func fileName(episodeId: String, url: URL) -> String { + let base = SHA256.hash(data: Data(episodeId.utf8)) + .map { String(format: "%02x", $0) } + .joined() + let ext = url.pathExtension + return ext.isEmpty ? base : "\(base).\(ext)" + } +} + +/// Session delegate driving a single download. Reports byte progress via +/// didWriteData, moves the finished file to `destination` synchronously (the +/// system-supplied temp file is valid only inside didFinishDownloadingTo), and +/// resumes the awaiting continuation on completion. A non-2xx response fails it. +/// +/// The delegate callbacks run on URLSession's delegate queue while `attach` is +/// called from the caller's thread, so all mutable state is guarded by `lock` +/// (which is what makes the `@unchecked Sendable` sound). +private final class DownloadSessionDelegate: NSObject, URLSessionDownloadDelegate, @unchecked Sendable { + private let destination: URL + private let onProgress: @Sendable (UInt64, UInt64?) -> Void + private let lock = NSLock() + private var continuation: CheckedContinuation<(URL, UInt64), Error>? + private var movedURL: URL? + private var completionError: Error? + private var lastPercent = -1 + /// Last time an indeterminate-size download reported progress, for time throttling. + private var lastReportAt = Date.distantPast + /// Bytes written so far; the final value is the completed file's size, so it can be + /// reported without a filesystem re-read (which could fail and record a size of 0). + private var receivedBytes: UInt64 = 0 + + /// Minimum spacing between progress reports when the total size is unknown and + /// there is no percentage to coalesce on (~1 update per second in production). + private let reportInterval: TimeInterval + + init( + destination: URL, reportInterval: TimeInterval, + onProgress: @escaping @Sendable (UInt64, UInt64?) -> Void, + ) { + self.destination = destination + self.reportInterval = reportInterval + self.onProgress = onProgress + } + + func attach(_ continuation: CheckedContinuation<(URL, UInt64), Error>) { + lock.lock() + defer { lock.unlock() } + self.continuation = continuation + } + + func urlSession( + _: URLSession, downloadTask _: URLSessionDownloadTask, + didWriteData _: Int64, totalBytesWritten: Int64, totalBytesExpectedToWrite: Int64, + ) { + let received = UInt64(max(0, totalBytesWritten)) + let total: UInt64? = totalBytesExpectedToWrite > 0 + ? UInt64(totalBytesExpectedToWrite) : nil + // Record every callback (so the final value is the completed size), but coalesce + // onProgress so the core isn't flooded with an update per write: to whole-percent + // steps when the size is known (~100 updates), or to ~1/sec when it isn't (no + // percentage to step on). onProgress runs outside the lock. + lock.lock() + receivedBytes = received + let report: Bool + if let total, total > 0 { + let percent = Int(received * 100 / total) + report = percent != lastPercent + if report { + lastPercent = percent + } + } else { + let now = Date() + report = now.timeIntervalSince(lastReportAt) >= reportInterval + if report { + lastReportAt = now + } + } + lock.unlock() + if report { + onProgress(received, total) + } + } + + func urlSession( + _: URLSession, downloadTask: URLSessionDownloadTask, didFinishDownloadingTo location: URL, + ) { + let status = (downloadTask.response as? HTTPURLResponse)?.statusCode + if let status, !(200 ..< 300).contains(status) { + setCompletionError(DownloadManagerError.badStatus(status)) + return + } + do { + // A re-download replaces any previous file at the same path. + try? FileManager.default.removeItem(at: destination) + try FileManager.default.moveItem(at: location, to: destination) + lock.lock() + movedURL = destination + lock.unlock() + } catch { + setCompletionError(error) + } + } + + func urlSession( + _: URLSession, task _: URLSessionTask, didCompleteWithError error: Error?, + ) { + // Snapshot under the lock, then resume outside it (resume can run the awaiting + // task, which must not happen while we hold the lock). + lock.lock() + let continuation = continuation + self.continuation = nil + let finishError = completionError + let moved = movedURL + let bytes = receivedBytes + lock.unlock() + + guard let continuation else { return } + if let error { + continuation.resume(throwing: error) + } else if let finishError { + continuation.resume(throwing: finishError) + } else if let moved { + continuation.resume(returning: (moved, bytes)) + } else { + continuation.resume(throwing: DownloadManagerError.incomplete) + } + } + + private func setCompletionError(_ error: Error) { + lock.lock() + defer { lock.unlock() } + completionError = error + } +} diff --git a/iOS/Pollux/DownloadNoticeBanner.swift b/iOS/Pollux/DownloadNoticeBanner.swift new file mode 100644 index 0000000..b17d44b --- /dev/null +++ b/iOS/Pollux/DownloadNoticeBanner.swift @@ -0,0 +1,29 @@ +import SwiftUI + +/// Non-blocking banner for a failed download *operation* (a persistence write that +/// didn't commit, or a file that couldn't be removed). Unlike a list-load error it +/// leaves the surrounding content and controls in place; the core clears the notice +/// on the next download action or feed switch. Shown on both the subscription detail +/// list and a single episode's detail page. +struct DownloadNoticeBanner: View { + @Environment(\.themeColors) private var themeColors + let message: String + + var body: some View { + HStack(alignment: .top, spacing: 8) { + // Decorative: the message text conveys the warning, so keep VoiceOver from + // announcing the icon's name ahead of it. + Image(systemName: "exclamationmark.triangle.fill") + .accessibilityHidden(true) + Text(message) + .font(.caption) + .fixedSize(horizontal: false, vertical: true) + Spacer(minLength: 0) + } + .foregroundStyle(themeColors.error) + .padding(.horizontal) + .padding(.vertical, 8) + .frame(maxWidth: .infinity, alignment: .leading) + .background(themeColors.secondaryBackground) + } +} diff --git a/iOS/Pollux/EpisodeDetailView.swift b/iOS/Pollux/EpisodeDetailView.swift index 900651f..d64794e 100644 --- a/iOS/Pollux/EpisodeDetailView.swift +++ b/iOS/Pollux/EpisodeDetailView.swift @@ -1,23 +1,49 @@ import App import SwiftUI -/// Full-page view for a single episode. Renders the stored metadata and show -/// notes; the transport controls, chapters, and bookmarks are DEBUG-marked -/// placeholders because their engines (playback, chapters, bookmarks) don't exist -/// yet. The episode is passed in from the list rather than re-fetched — the list's -/// ViewModel already carries it. A dedicated `GetEpisode` round-trip could replace -/// this later if the detail page ever needs data the list doesn't ship. +/// Full-page view for a single episode. Renders the stored metadata, show notes, +/// and a working download control; transport controls, chapters, and bookmarks are +/// DEBUG-marked placeholders because their engines don't exist yet. Static metadata +/// is taken from the `episode` snapshot passed in from the list, but the download +/// state is read live from `core.view` so it updates while the page is open. struct EpisodeDetailView: View { + @ObservedObject var core: Core @Environment(\.themeColors) private var themeColors let episode: EpisodeSummary let feedTitle: String + /// The current version of this episode from the core, so download-state changes + /// show up here without re-navigating. Falls back to the injected snapshot if + /// the list is no longer the selected feed. + private var liveEpisode: EpisodeSummary { + core.view.subscriptionDetail.episodes.first { $0.id == episode.id } ?? episode + } + + /// The op-failure notice message, but only when it concerns this episode — the + /// notice is subscription-wide, so scope it so one episode's failure doesn't + /// surface on another's detail page. + private var episodeNotice: String? { + guard let notice = core.view.subscriptionDetail.downloadNotice, + notice.episodeId == episode.id + else { + return nil + } + return notice.message + } + var body: some View { ScrollView { VStack(alignment: .leading, spacing: 20) { artworkHeader titleBlock playbackControls + // A delete/cancel initiated here can fail; show the same non-blocking + // notice the list uses so the failure is visible without navigating back + // — but only when it's about this episode (see `episodeNotice`). + if let notice = episodeNotice { + DownloadNoticeBanner(message: notice) + } + downloadSection Divider() showNotes PlaceholderSection( @@ -87,6 +113,91 @@ struct EpisodeDetailView: View { .frame(maxWidth: .infinity) } + /// Working download control, driven by the live download status. + @ViewBuilder private var downloadSection: some View { + switch liveEpisode.downloadStatus { + case .notDownloaded: + Button { core.update(.downloadEpisode(episode.id)) } label: { + Label("Download episode", systemImage: "arrow.down.circle") + } + case .queued: + HStack { + Label("Queued for download", systemImage: "clock") + .foregroundStyle(themeColors.secondaryText) + Spacer() + cancelButton + } + case .downloading: + HStack(spacing: 12) { + VStack(alignment: .leading, spacing: 6) { + if let progress = EpisodeFormatting.downloadProgress( + received: liveEpisode.downloadReceivedBytes, + total: liveEpisode.downloadTotalBytes, + ) { + if let fraction = progress.fraction { + ProgressView(value: fraction) + } else { + ProgressView() + } + Text(progress.label) + .font(.caption) + .monospacedDigit() + .foregroundStyle(themeColors.secondaryText) + } else { + Label { + Text("Downloading…") + } icon: { + ProgressView() + } + } + } + .frame(maxWidth: .infinity, alignment: .leading) + cancelButton + } + case .downloaded: + HStack { + Label("Downloaded", systemImage: "checkmark.circle.fill") + .foregroundStyle(themeColors.success) + Spacer() + Button(role: .destructive) { + core.update(.deleteDownload(episode.id)) + } label: { + Label("Delete", systemImage: "trash") + } + } + case .failed: + VStack(alignment: .leading, spacing: 6) { + HStack { + Label("Download failed", systemImage: "exclamationmark.triangle") + .foregroundStyle(themeColors.error) + Spacer() + Button("Retry") { core.update(.downloadEpisode(episode.id)) } + } + if let reason = liveEpisode.downloadError { + Text(reason) + .font(.caption) + .foregroundStyle(themeColors.secondaryText) + .fixedSize(horizontal: false, vertical: true) + } + } + case .removedFromFeed: + Label("Removed from feed", systemImage: "xmark.circle") + .foregroundStyle(themeColors.secondaryText) + } + } + + /// Small cancel affordance shown beside the progress bar (and the queued row). + private var cancelButton: some View { + Button(role: .destructive) { + core.update(.cancelDownload(episode.id)) + } label: { + Image(systemName: "xmark.circle.fill") + .font(.title2) + } + .buttonStyle(.borderless) + .accessibilityLabel("Cancel download") + } + private var showNotes: some View { VStack(alignment: .leading, spacing: 6) { Text("Show notes") diff --git a/iOS/Pollux/EpisodeFormatting.swift b/iOS/Pollux/EpisodeFormatting.swift index 5fc1bec..65106f2 100644 --- a/iOS/Pollux/EpisodeFormatting.swift +++ b/iOS/Pollux/EpisodeFormatting.swift @@ -35,10 +35,33 @@ enum EpisodeFormatting { return "\(secs)s" } + /// Live download progress for display. Returns nil before any bytes are known. + /// `fraction` is 0...1 when the total size is known (nil → show an indeterminate + /// indicator); `label` is a byte summary like "12.3 MB / 45.6 MB", or just + /// "12.3 MB" when the server didn't report a total. + static func downloadProgress( + received: UInt64?, total: UInt64?, + ) -> (fraction: Double?, label: String)? { + guard let received else { return nil } + let receivedText = byteFormatter.string(fromByteCount: Int64(clamping: received)) + if let total, total > 0 { + let totalText = byteFormatter.string(fromByteCount: Int64(clamping: total)) + let fraction = min(1.0, Double(received) / Double(total)) + return (fraction, "\(receivedText) / \(totalText)") + } + return (nil, receivedText) + } + private static let pubDateFormatter: DateFormatter = { let formatter = DateFormatter() formatter.dateStyle = .medium formatter.timeStyle = .none return formatter }() + + private static let byteFormatter: ByteCountFormatter = { + let formatter = ByteCountFormatter() + formatter.countStyle = .file + return formatter + }() } diff --git a/iOS/Pollux/EpisodeRow.swift b/iOS/Pollux/EpisodeRow.swift index 160a423..830b5f5 100644 --- a/iOS/Pollux/EpisodeRow.swift +++ b/iOS/Pollux/EpisodeRow.swift @@ -1,13 +1,19 @@ import App import SwiftUI -/// One episode in the subscription details list. Shows only stored data (art, -/// title, date, duration, description, read-only played/download status). Controls -/// whose engines don't exist yet (play, download, more-actions) are rendered as -/// DEBUG-tinted, disabled placeholders — see `DebugStyle.swift`. +/// One episode in the subscription details list. Shows stored data (art, title, +/// date, duration, description, read-only played/download status) and a working +/// download control. Playback and more-actions remain DEBUG-tinted, disabled +/// placeholders because their engines don't exist yet — see `DebugStyle.swift`. struct EpisodeRow: View { @Environment(\.themeColors) private var themeColors let episode: EpisodeSummary + /// Queue this episode for download (or retry a failed one). + let onDownload: (String) -> Void + /// Remove this episode's downloaded file. + let onDeleteDownload: (String) -> Void + /// Stop this episode's in-flight or queued download. + let onCancelDownload: (String) -> Void var body: some View { HStack(alignment: .top, spacing: 12) { @@ -20,11 +26,7 @@ struct EpisodeRow: View { .foregroundStyle(themeColors.text) .lineLimit(2) - if let meta = metaLine { - Text(meta) - .font(.caption) - .foregroundStyle(themeColors.secondaryText) - } + metaRow if let description = episode.descriptionText, !description.isEmpty { Text(description) @@ -38,7 +40,7 @@ struct EpisodeRow: View { Spacer(minLength: 8) - placeholderControls + trailingControls } .padding(.vertical, 4) } @@ -47,10 +49,82 @@ struct EpisodeRow: View { EpisodeFormatting.metaLine(pubDate: episode.pubDate, durationSecs: episode.durationSecs) } + /// The date·duration line — except while downloading, when it is replaced by a + /// progress indicator. Non-not-downloaded states get a small glyph ahead of the + /// date so status reads inline instead of on its own badge line — otherwise + /// queued/failed/removed rows would look identical to a plain undownloaded one. + @ViewBuilder private var metaRow: some View { + if episode.downloadStatus == .downloading { + downloadingLine + } else { + HStack(spacing: 4) { + if let glyph = statusGlyph { + Image(systemName: glyph.icon) + .font(.caption) + .foregroundStyle(glyph.tint) + .accessibilityLabel(glyph.label) + } + if let text = metaText { + Text(text) + .font(.caption) + .lineLimit(1) + .foregroundStyle(themeColors.secondaryText) + } + } + } + } + + /// The date·duration line, replaced by the failure reason when a download failed + /// so the row says *why* (disk full, HTTP status, …) next to the warning glyph. + private var metaText: String? { + if episode.downloadStatus == .failed, let error = episode.downloadError { + return error + } + return metaLine + } + + /// The inline download-status glyph for the date line. `nil` for not-downloaded + /// (a plain date) and downloading (handled by `downloadingLine`). Failed uses the + /// error color to stand out; the rest are neutral. + private var statusGlyph: StatusGlyph? { + switch episode.downloadStatus { + case .downloaded: + StatusGlyph(icon: "arrow.down.circle.fill", tint: themeColors.secondaryText, label: "Downloaded") + case .queued: + StatusGlyph(icon: "clock", tint: themeColors.secondaryText, label: "Queued for download") + case .failed: + StatusGlyph(icon: "exclamationmark.triangle.fill", tint: themeColors.error, label: "Download failed") + case .removedFromFeed: + StatusGlyph(icon: "xmark.circle", tint: themeColors.secondaryText, label: "Removed from feed") + case .notDownloaded, .downloading: + nil + } + } + + /// In-progress download: a determinate bar when the size is known, otherwise a + /// spinner with whatever byte count we have. Replaces the date·duration line. + @ViewBuilder private var downloadingLine: some View { + let progress = EpisodeFormatting.downloadProgress( + received: episode.downloadReceivedBytes, + total: episode.downloadTotalBytes, + ) + if let progress, let fraction = progress.fraction { + ProgressView(value: fraction) + .accessibilityLabel("Downloading \(Int(fraction * 100)) percent") + } else { + HStack(spacing: 6) { + ProgressView() + Text(progress?.label ?? "Downloading…") + .font(.caption) + .foregroundStyle(themeColors.secondaryText) + } + .accessibilityLabel("Downloading \(progress?.label ?? "")") + } + } + @ViewBuilder private var statusRow: some View { let playback = playbackBadge - let download = downloadBadge - if playback != nil || download != nil || playbackPositionText != nil { + if playback != nil || playbackPositionText != nil { HStack(spacing: 10) { if let playback { StatusBadge(systemImage: playback.icon, text: playback.text) @@ -58,25 +132,60 @@ struct EpisodeRow: View { if let positionText = playbackPositionText { StatusBadge(systemImage: "clock.arrow.circlepath", text: positionText) } - if let download { - StatusBadge(systemImage: download.icon, text: download.text) - } } .padding(.top, 2) } } - /// Play + more-actions: no playback engine or download manager exists yet, so - /// these are inert placeholders (DEBUG tint + 🚫 overlay), not wired to fake - /// behavior. - private var placeholderControls: some View { + /// Trailing controls: the still-stubbed play button (no playback engine yet) and + /// the three-dots menu, which is where the download action now lives. Download + /// *status* stays inline (`metaRow`); this menu is the *action* surface. + private var trailingControls: some View { HStack(spacing: 12) { Image(systemName: "play.circle.fill") .font(.title2) .stubbed() + moreMenu + } + } + + /// The three-dots menu. Today it holds only the download action (its first, + /// context-sensitive item); Play, Mark played, Flag, etc. join it as their + /// engines land. Swipe shortcuts are a later, user-configurable addition. + private var moreMenu: some View { + Menu { + downloadMenuItem + } label: { Image(systemName: "ellipsis.circle") .font(.title2) - .stubbed() + } + .accessibilityLabel("More actions") + } + + /// The download entry, driven by status: Download when absent, Retry after a + /// failure, Delete when stored, Cancel Download while queued or downloading, and + /// a disabled indicator once removed from the feed. + @ViewBuilder private var downloadMenuItem: some View { + switch episode.downloadStatus { + case .notDownloaded: + Button { onDownload(episode.id) } label: { + Label("Download", systemImage: "arrow.down.circle") + } + case .failed: + Button { onDownload(episode.id) } label: { + Label("Retry Download", systemImage: "arrow.clockwise.circle") + } + case .downloaded: + Button(role: .destructive) { onDeleteDownload(episode.id) } label: { + Label("Delete Download", systemImage: "trash") + } + case .queued, .downloading: + Button(role: .destructive) { onCancelDownload(episode.id) } label: { + Label("Cancel Download", systemImage: "xmark.circle") + } + case .removedFromFeed: + Button {} label: { Label("Removed From Feed", systemImage: "xmark.circle") } + .disabled(true) } } @@ -101,23 +210,13 @@ struct EpisodeRow: View { } return "at \(position)" } +} - private var downloadBadge: (icon: String, text: String)? { - switch episode.downloadStatus { - case .notDownloaded: - nil - case .queued: - ("clock", "Queued") - case .downloading: - ("arrow.down.circle", "Downloading") - case .downloaded: - ("arrow.down.circle.fill", "Downloaded") - case .failed: - ("exclamationmark.triangle", "Failed") - case .removedFromFeed: - ("xmark.circle", "Removed") - } - } +/// Inline download-status glyph shown before the date on an episode row. +private struct StatusGlyph { + let icon: String + let tint: Color + let label: String } /// A compact read-only status indicator (real data, theme colors). diff --git a/iOS/Pollux/SubscriptionDetailScreen.swift b/iOS/Pollux/SubscriptionDetailScreen.swift index 4d70ca8..e8a7dd2 100644 --- a/iOS/Pollux/SubscriptionDetailScreen.swift +++ b/iOS/Pollux/SubscriptionDetailScreen.swift @@ -30,7 +30,7 @@ struct SubscriptionDetailScreen: View { } } .navigationDestination(for: EpisodeSummary.self) { episode in - EpisodeDetailView(episode: episode, feedTitle: subscription.title) + EpisodeDetailView(core: core, episode: episode, feedTitle: subscription.title) } .task(id: subscription.id) { core.update(.selectSubscription(subscription.id)) @@ -71,14 +71,72 @@ struct SubscriptionDetailScreen: View { .foregroundStyle(themeColors.secondaryText) } } else { - List(detail.episodes, id: \.id) { episode in - NavigationLink(value: episode) { - EpisodeRow(episode: episode) + VStack(spacing: 0) { + // The list spans every episode, so show a notice for any of them. + if let notice = detail.downloadNotice { + DownloadNoticeBanner(message: notice.message) } - .listRowBackground(themeColors.background) + List(detail.episodes, id: \.id) { episode in + NavigationLink(value: episode) { + EpisodeRow( + episode: episode, + onDownload: { core.update(.downloadEpisode($0)) }, + onDeleteDownload: { core.update(.deleteDownload($0)) }, + onCancelDownload: { core.update(.cancelDownload($0)) }, + ) + } + // Spec default: short swipe-right → Download / Delete. This is the + // one actionable swipe today (flag / mark-played have no engine yet); + // it becomes user-configurable when Settings lands. + .swipeActions(edge: .leading, allowsFullSwipe: fullSwipeAllowed(episode)) { + downloadSwipeButton(for: episode) + } + .listRowBackground(themeColors.background) + } + .listStyle(.plain) + .scrollContentBackground(.hidden) + } + } + } + + /// Leading-swipe download action, mirroring the row's menu: Download when absent, + /// Retry after a failure, Delete when stored, Cancel while queued or downloading, + /// and nothing once removed from the feed. + @ViewBuilder private func downloadSwipeButton(for episode: EpisodeSummary) -> some View { + switch episode.downloadStatus { + case .notDownloaded: + Button { core.update(.downloadEpisode(episode.id)) } label: { + Label("Download", systemImage: "arrow.down.circle") + } + .tint(themeColors.accent) + case .failed: + Button { core.update(.downloadEpisode(episode.id)) } label: { + Label("Retry", systemImage: "arrow.clockwise.circle") } - .listStyle(.plain) - .scrollContentBackground(.hidden) + .tint(themeColors.warning) + case .downloaded: + Button(role: .destructive) { core.update(.deleteDownload(episode.id)) } label: { + Label("Delete", systemImage: "trash") + } + case .queued, .downloading: + Button(role: .destructive) { core.update(.cancelDownload(episode.id)) } label: { + Label { + Text("Cancel\nDownload") + } icon: { + Image(systemName: "xmark.circle") + } + } + case .removedFromFeed: + EmptyView() + } + } + + /// Full-swipe commits the action without tapping. Allowed for downloading (safe, + /// re-downloadable) but not for deleting, so a stray long swipe can't wipe a file. + private func fullSwipeAllowed(_ episode: EpisodeSummary) -> Bool { + switch episode.downloadStatus { + case .notDownloaded, .failed: true + default: false } } diff --git a/iOS/Pollux/core.swift b/iOS/Pollux/core.swift index f3bbefc..9c43e6c 100644 --- a/iOS/Pollux/core.swift +++ b/iOS/Pollux/core.swift @@ -8,6 +8,9 @@ class Core: ObservableObject { private var core: CoreFfi private let db: DatabaseManager + private let downloads: DownloadManager + /// Storage operations are funnelled here and executed one at a time, in order. + private let storage: AsyncStream<(StorageOperation, UInt32)>.Continuation init() { core = CoreFfi() @@ -16,10 +19,34 @@ class Core: ObservableObject { } catch { fatalError("Failed to initialize DatabaseManager: \(error)") } + do { + downloads = try DownloadManager() + } catch { + fatalError("Failed to initialize DownloadManager: \(error)") + } + // One serial consumer so writes for a given episode apply in submission order + // (Queued → Downloading → terminal). A Task per op could reorder them — a fast + // failure/cancel racing the Downloading write would leave a stale status in the + // DB and re-download the episode after restart. + let (storageStream, storageContinuation) = + AsyncStream.makeStream(of: (StorageOperation, UInt32).self) + storage = storageContinuation guard let view = try? ViewModel.bincodeDeserialize(input: [UInt8](core.view())) else { fatalError("Failed to deserialize initial ViewModel from core") } self.view = view + Task { @MainActor [weak self] in + for await (operation, requestId) in storageStream { + guard let self else { return } + let result: StorageResult + do { + result = try await db.execute(operation) + } catch { + result = .error(error.localizedDescription) + } + resolveAndDispatch(requestId: requestId, result: result) + } + } update(.started) } @@ -48,15 +75,8 @@ class Core: ObservableObject { view = updatedView case let .storage(operation): - Task { @MainActor in - let result: StorageResult - do { - result = try await db.execute(operation) - } catch { - result = .error(error.localizedDescription) - } - resolveAndDispatch(requestId: request.id, result: result) - } + // Enqueue for the serial consumer set up in init (preserves write order). + storage.yield((operation, request.id)) case let .http(operation): let requestId = request.id @@ -66,6 +86,43 @@ class Core: ObservableObject { self?.resolveAndDispatch(requestId: requestId, result: result) } } + + case let .download(operation): + handleDownload(operation, requestId: request.id) + } + } + + /// Runs a download-capability operation on the DownloadManager actor (so the UI + /// isn't blocked) and resolves the request with its terminal result. Byte progress + /// is funnelled through a stream drained by a single consumer, so DownloadProgress + /// events reach the core in order — a Task-per-callback could reorder them. + private func handleDownload(_ operation: DownloadOperation, requestId: UInt32) { + let progressEpisodeId: String? = { + if case let .download(episodeId, _) = operation { + return episodeId + } + return nil + }() + Task { @MainActor in + let (progressStream, progressContinuation) = + AsyncStream.makeStream(of: (UInt64, UInt64?).self) + let consumer = Task { @MainActor [weak self] in + for await (received, total) in progressStream { + guard let progressEpisodeId else { continue } + self?.update(.downloadProgress( + episodeId: progressEpisodeId, + receivedBytes: received, + totalBytes: total, + )) + } + } + let result = await downloads.perform(operation) { received, total in + progressContinuation.yield((received, total)) + } + // Drain any buffered progress before the terminal result lands. + progressContinuation.finish() + await consumer.value + resolveAndDispatch(requestId: requestId, result: result) } } @@ -116,4 +173,11 @@ class Core: ObservableObject { } resolveBytes(requestId: requestId, bytes: bytes) } + + private func resolveAndDispatch(requestId: UInt32, result: DownloadResult) { + guard let bytes = try? result.bincodeSerialize() else { + fatalError("Failed to serialize DownloadResult for request \(requestId)") + } + resolveBytes(requestId: requestId, bytes: bytes) + } } diff --git a/iOS/PolluxTests/DatabaseManagerTests.swift b/iOS/PolluxTests/DatabaseManagerTests.swift index 7395e3c..dc8d09d 100644 --- a/iOS/PolluxTests/DatabaseManagerTests.swift +++ b/iOS/PolluxTests/DatabaseManagerTests.swift @@ -51,7 +51,6 @@ private func makeEpisode( playbackStatus: playbackStatus, playbackPositionSecs: nil, downloadStatus: downloadStatus, - downloadProgress: nil, isFlagged: false, fileSizeBytes: fileSizeBytes, localPath: nil @@ -230,6 +229,81 @@ struct DatabaseManagerTests { #expect(fetched.playbackPositionSecs == largePosition) } + @Test func updateDownloadState_recordsDownloadedFileMetadata() async throws { + let db = try makeManager() + try await db.execute(.upsertSubscription(makeSubscription(id: "sub-1"))) + try await db.execute(.upsertEpisode(makeEpisode(id: "ep-1", subscriptionId: "sub-1"))) + + try await db.execute( + .updateDownloadState( + episodeId: "ep-1", status: .downloaded, + localPath: "Downloads/ep-1.mp3", sizeBytes: 4096, + )) + + let result = try await db.execute(.getEpisode(id: "ep-1")) + guard case let .episode(fetched) = result else { + Issue.record("Expected .episode, got \(result)") + return + } + #expect(fetched.downloadStatus == .downloaded) + #expect(fetched.localPath == "Downloads/ep-1.mp3") + #expect(fetched.fileSizeBytes == 4096) + } + + @Test func updateDownloadState_clearsFileMetadataOnReset() async throws { + let db = try makeManager() + try await db.execute(.upsertSubscription(makeSubscription(id: "sub-1"))) + try await db.execute(.upsertEpisode(makeEpisode(id: "ep-1", subscriptionId: "sub-1"))) + + // Download, then delete: the reset must null out the path and size so no + // stale file reference survives. + try await db.execute( + .updateDownloadState( + episodeId: "ep-1", status: .downloaded, + localPath: "Downloads/ep-1.mp3", sizeBytes: 4096, + )) + try await db.execute( + .updateDownloadState( + episodeId: "ep-1", status: .notDownloaded, + localPath: nil, sizeBytes: nil, + )) + + let result = try await db.execute(.getEpisode(id: "ep-1")) + guard case let .episode(fetched) = result else { + Issue.record("Expected .episode, got \(result)") + return + } + #expect(fetched.downloadStatus == .notDownloaded) + #expect(fetched.localPath == nil) + #expect(fetched.fileSizeBytes == nil) + } + + @Test func listPendingDownloads_returnsOnlyInFlightEpisodes() async throws { + let db = try makeManager() + try await db.execute(.upsertSubscription(makeSubscription(id: "sub-1"))) + + // One of each download status; only Downloading + Queued should come back. + let statuses: [(String, DownloadStatus)] = [ + ("ep-notdl", .notDownloaded), + ("ep-queued", .queued), + ("ep-downloading", .downloading), + ("ep-downloaded", .downloaded), + ("ep-failed", .failed), + ] + for (id, status) in statuses { + try await db.execute(.upsertEpisode( + makeEpisode(id: id, subscriptionId: "sub-1", feedGuid: "g-\(id)", downloadStatus: status), + )) + } + + let result = try await db.execute(.listPendingDownloads) + guard case let .episodes(eps) = result else { + Issue.record("Expected .episodes, got \(result)") + return + } + #expect(Set(eps.map(\.id)) == ["ep-queued", "ep-downloading"]) + } + @Test func deleteSubscription_cascadesToEpisodes() async throws { let db = try makeManager() let sub = makeSubscription(id: "sub-1") @@ -430,7 +504,7 @@ struct DatabaseManagerTests { title: "Test", description: nil, pubDate: nil, durationSecs: nil, enclosureUrl: "https://example.com/ep.mp3", artworkUrl: nil, playbackStatus: .unplayed, playbackPositionSecs: nil, - downloadStatus: .notDownloaded, downloadProgress: nil, + downloadStatus: .notDownloaded, isFlagged: false, fileSizeBytes: UInt64.max, localPath: nil ) @@ -469,31 +543,4 @@ struct DatabaseManagerTests { } #expect(fetched.durationSecs == nil, "out-of-range duration should read as nil") } - - @Test func episode_outOfRangeDownloadProgressReadsAsNil() async throws { - // download_progress has a CHECK (0...100), so planting an out-of-range - // value requires bypassing it — standing in for a future schema that - // drops the bound or external tooling that ignores it. The read must - // degrade to nil, not silently wrap (300 -> 44) as truncation would. - let path = FileManager.default.temporaryDirectory - .appendingPathComponent(UUID().uuidString + ".sqlite").path - let db = try DatabaseManager(path: path) - try await db.execute(.upsertSubscription(makeSubscription(id: "sub-1"))) - try await db.execute(.upsertEpisode( - makeEpisode(id: "ep-1", subscriptionId: "sub-1", feedGuid: "g1"), - )) - - let raw = try DatabasePool(path: path) - try await raw.write { db in - try db.execute(sql: "PRAGMA ignore_check_constraints = ON") - try db.execute(sql: "UPDATE episodes SET download_progress = 300 WHERE id = 'ep-1'") - } - - let result = try await db.execute(.getEpisode(id: "ep-1")) - guard case let .episode(fetched) = result else { - Issue.record("Expected .episode, got \(result)") - return - } - #expect(fetched.downloadProgress == nil, "out-of-range progress should read as nil") - } } diff --git a/iOS/PolluxTests/DownloadManagerTests.swift b/iOS/PolluxTests/DownloadManagerTests.swift new file mode 100644 index 0000000..1b6c57a --- /dev/null +++ b/iOS/PolluxTests/DownloadManagerTests.swift @@ -0,0 +1,297 @@ +import App +import Foundation +import Testing + +@testable import Pollux + +// MARK: - Test doubles + +/// A stub `URLProtocol` so download tests never touch the network. A test configures +/// the canned response (status, whether a Content-Length is advertised, body bytes, +/// and whether to stall mid-flight for cancellation), and the loader delivers it in +/// small chunks so byte-progress callbacks fire. +private final class MockURLProtocol: URLProtocol, @unchecked Sendable { + struct Stub: Sendable { + var statusCode = 200 + var advertiseContentLength = true + var body = Data() + /// Deliver the response + body but never finish, so the request stays in + /// flight until the test cancels it. + var stall = false + } + + private static let lock = NSLock() + nonisolated(unsafe) private static var stub = Stub() + nonisolated(unsafe) private static var onStart: (@Sendable () -> Void)? + + static func configure(_ stub: Stub, onStart: (@Sendable () -> Void)? = nil) { + lock.lock() + defer { lock.unlock() } + self.stub = stub + self.onStart = onStart + } + + override class func canInit(with _: URLRequest) -> Bool { true } + override class func canonicalRequest(for request: URLRequest) -> URLRequest { request } + + override func startLoading() { + Self.lock.lock() + let stub = Self.stub + let onStart = Self.onStart + Self.lock.unlock() + + guard let url = request.url, let client else { return } + var headers: [String: String] = [:] + if stub.advertiseContentLength { + headers["Content-Length"] = String(stub.body.count) + } + guard let response = HTTPURLResponse( + url: url, statusCode: stub.statusCode, httpVersion: "HTTP/1.1", headerFields: headers, + ) else { return } + client.urlProtocol(self, didReceive: response, cacheStoragePolicy: .notAllowed) + + // Chunk the body so more than one write callback can fire. + let chunkSize = 64 + var offset = 0 + while offset < stub.body.count { + let end = min(offset + chunkSize, stub.body.count) + client.urlProtocol(self, didLoad: stub.body.subdata(in: offset ..< end)) + offset = end + } + + onStart?() + if stub.stall { return } + client.urlProtocolDidFinishLoading(self) + } + + override func stopLoading() {} +} + +/// Thread-safe collector for the `onProgress` callbacks, which arrive on the session's +/// delegate queue. +private final class ProgressRecorder: @unchecked Sendable { + private let lock = NSLock() + private var storage: [(received: UInt64, total: UInt64?)] = [] + + func record(_ received: UInt64, _ total: UInt64?) { + lock.lock() + defer { lock.unlock() } + storage.append((received, total)) + } + + var samples: [(received: UInt64, total: UInt64?)] { + lock.lock() + defer { lock.unlock() } + return storage + } +} + +/// One-shot signal a test awaits to learn the stub has begun loading (so it can cancel +/// the request while it is genuinely in flight). +private final class StartSignal: @unchecked Sendable { + private let lock = NSLock() + private var continuation: CheckedContinuation? + private var fired = false + + func fire() { + lock.lock() + let waiter = continuation + continuation = nil + fired = true + lock.unlock() + waiter?.resume() + } + + func wait() async { + await withCheckedContinuation { continuation in + lock.lock() + if fired { + lock.unlock() + continuation.resume() + } else { + self.continuation = continuation + lock.unlock() + } + } + } +} + +// MARK: - Helpers + +private func makeTempRoot() -> URL { + FileManager.default.temporaryDirectory.appendingPathComponent(UUID().uuidString, isDirectory: true) +} + +private func makeManager(root: URL, reportInterval: TimeInterval = 0) throws -> DownloadManager { + let configuration = URLSessionConfiguration.ephemeral + configuration.protocolClasses = [MockURLProtocol.self] + return try DownloadManager( + storageRoot: root, sessionConfiguration: configuration, reportInterval: reportInterval, + ) +} + +private let noProgress: @Sendable (UInt64, UInt64?) -> Void = { _, _ in } + +/// The stored path a download for `episodeId`/`url` resolves to, derived from the +/// same helper the manager uses so tests don't hard-code the (digest-based) name. +private func downloadsRelativePath(episodeId: String, url: URL) -> String { + "Downloads/" + DownloadManager.fileName(episodeId: episodeId, url: url) +} + +// MARK: - Tests + +@Suite("DownloadManager", .serialized) +struct DownloadManagerTests { + + @Test func successfulDownloadStoresTheFileAndReportsItsSize() async throws { + let root = makeTempRoot() + let manager = try makeManager(root: root) + let body = Data((0 ..< 300).map { UInt8($0 % 251) }) + MockURLProtocol.configure(.init(body: body)) + let url = try #require(URL(string: "https://example.com/a.mp3")) + + let result = await manager.perform( + .download(episodeId: "ep-1", url: url.absoluteString), onProgress: noProgress, + ) + + let rel = downloadsRelativePath(episodeId: "ep-1", url: url) + #expect(result == .completed(localPath: rel, sizeBytes: UInt64(body.count))) + let stored = try Data(contentsOf: root.appendingPathComponent(rel)) + #expect(stored == body) + } + + @Test func nonSuccessStatusFailsWithoutLeavingAFile() async throws { + let root = makeTempRoot() + let manager = try makeManager(root: root) + MockURLProtocol.configure(.init(statusCode: 404, body: Data("not found".utf8))) + let url = try #require(URL(string: "https://example.com/b.mp3")) + + let result = await manager.perform( + .download(episodeId: "ep-2", url: url.absoluteString), onProgress: noProgress, + ) + + guard case let .error(message) = result else { + Issue.record("expected .error, got \(result)") + return + } + #expect(message.contains("404")) + #expect(!FileManager.default.fileExists( + atPath: root.appendingPathComponent( + downloadsRelativePath(episodeId: "ep-2", url: url), + ).path, + )) + } + + @Test func cancellingAnInFlightDownloadReturnsCancelledAndLeavesNoFile() async throws { + let root = makeTempRoot() + let manager = try makeManager(root: root) + let started = StartSignal() + MockURLProtocol.configure( + .init(advertiseContentLength: false, body: Data(count: 128), stall: true), + onStart: { started.fire() }, + ) + let url = try #require(URL(string: "https://example.com/c.mp3")) + + let download = Task { + await manager.perform( + .download(episodeId: "ep-3", url: url.absoluteString), + onProgress: noProgress, + ) + } + await started.wait() + _ = await manager.perform(.cancel(episodeId: "ep-3"), onProgress: noProgress) + + let result = await download.value + #expect(result == .cancelled) + #expect(!FileManager.default.fileExists( + atPath: root.appendingPathComponent( + downloadsRelativePath(episodeId: "ep-3", url: url), + ).path, + )) + } + + @Test func deletingAStoredFileIsIdempotent() async throws { + let root = makeTempRoot() + let manager = try makeManager(root: root) + let relativePath = "Downloads/keep.mp3" + let fileURL = root.appendingPathComponent(relativePath) + try FileManager.default.createDirectory( + at: fileURL.deletingLastPathComponent(), withIntermediateDirectories: true, + ) + try Data("audio".utf8).write(to: fileURL) + + let first = await manager.perform(.delete(localPath: relativePath), onProgress: noProgress) + #expect(first == .deleted) + #expect(!FileManager.default.fileExists(atPath: fileURL.path)) + + // Deleting the now-absent file still succeeds. + let second = await manager.perform(.delete(localPath: relativePath), onProgress: noProgress) + #expect(second == .deleted) + } + + @Test func progressIsReportedWithMonotonicBytesAndTheKnownTotal() async throws { + let root = makeTempRoot() + let manager = try makeManager(root: root) + let body = Data(count: 5000) + MockURLProtocol.configure(.init(body: body)) + let recorder = ProgressRecorder() + let url = try #require(URL(string: "https://example.com/p.mp3")) + + let result = await manager.perform( + .download(episodeId: "ep-p", url: url.absoluteString), + ) { @Sendable received, total in + recorder.record(received, total) + } + + let rel = downloadsRelativePath(episodeId: "ep-p", url: url) + #expect(result == .completed(localPath: rel, sizeBytes: UInt64(body.count))) + let samples = recorder.samples + #expect(!samples.isEmpty) + // Every report carries the advertised total, and received bytes never go backwards. + #expect(samples.allSatisfy { $0.total == UInt64(body.count) }) + #expect(samples.map(\.received) == samples.map(\.received).sorted()) + #expect(samples.last?.received == UInt64(body.count)) + } + + @Test func fileNameAvoidsCollisionsBetweenSimilarIds() throws { + let url = try #require(URL(string: "https://example.com/a.mp3")) + // Ids that a lossy char-replacement scheme would collapse to the same name. + let first = DownloadManager.fileName(episodeId: "ep-1", url: url) + let second = DownloadManager.fileName(episodeId: "ep_1", url: url) + #expect(first != second, "distinct ids must map to distinct files") + // Deterministic for the same id, and the extension is preserved. + #expect(DownloadManager.fileName(episodeId: "ep-1", url: url) == first) + #expect(first.hasSuffix(".mp3")) + } + + @Test func fileNameOmitsExtensionWhenTheUrlHasNone() throws { + let url = try #require(URL(string: "https://example.com/audio")) + let name = DownloadManager.fileName(episodeId: "ep-1", url: url) + #expect(!name.contains("."), "no URL extension means no dotted suffix") + } + + @Test func unknownSizeProgressIsThrottled() async throws { + let root = makeTempRoot() + // A long interval means the time throttle should let at most the first report + // through, no matter how many write callbacks a chunked body produces. + let manager = try makeManager(root: root, reportInterval: 1000) + // ~1600 chunks of 64 bytes: without throttling this would report constantly. + MockURLProtocol.configure(.init(advertiseContentLength: false, body: Data(count: 100_000))) + let recorder = ProgressRecorder() + let url = try #require(URL(string: "https://example.com/u.mp3")) + + let result = await manager.perform( + .download(episodeId: "ep-u", url: url.absoluteString), + ) { @Sendable received, total in + recorder.record(received, total) + } + + let rel = downloadsRelativePath(episodeId: "ep-u", url: url) + #expect(result == .completed(localPath: rel, sizeBytes: 100_000)) + let samples = recorder.samples + // The throttle caps reporting at the single initial callback. + #expect(samples.count <= 1) + // Any report that did fire carries no total (the size was unknown). + #expect(samples.allSatisfy { $0.total == nil }) + } +} diff --git a/iOS/PolluxTests/EpisodeFormattingTests.swift b/iOS/PolluxTests/EpisodeFormattingTests.swift index 24b608d..4a451a5 100644 --- a/iOS/PolluxTests/EpisodeFormattingTests.swift +++ b/iOS/PolluxTests/EpisodeFormattingTests.swift @@ -46,4 +46,52 @@ struct EpisodeFormattingTests { #expect(rendered != nil) #expect(rendered?.isEmpty == false) } + + // MARK: Download progress + + @Test func nilReceivedHasNoProgress() { + #expect(EpisodeFormatting.downloadProgress(received: nil, total: 100) == nil) + } + + @Test func knownTotalGivesFractionAndCombinedLabel() { + let progress = EpisodeFormatting.downloadProgress(received: 50, total: 100) + #expect(progress?.fraction == 0.5) + #expect(progress?.label == "\(bytes(50)) / \(bytes(100))") + } + + @Test func unknownTotalHasNoFractionAndReceivedOnlyLabel() { + let progress = EpisodeFormatting.downloadProgress(received: 1234, total: nil) + #expect(progress?.fraction == nil) + #expect(progress?.label == bytes(1234)) + } + + @Test func zeroTotalIsTreatedAsUnknown() { + // A reported total of 0 carries no information, so it behaves like `nil`. + let progress = EpisodeFormatting.downloadProgress(received: 1234, total: 0) + #expect(progress?.fraction == nil) + #expect(progress?.label == bytes(1234)) + } + + @Test func receivedExceedingTotalClampsFractionToOne() { + // A late byte count past the advertised size shouldn't push the bar past full. + let progress = EpisodeFormatting.downloadProgress(received: 150, total: 100) + #expect(progress?.fraction == 1.0) + #expect(progress?.label == "\(bytes(150)) / \(bytes(100))") + } + + @Test func hugeByteCountsDoNotOverflow() { + // Int64(clamping:) keeps the ByteCountFormatter input in range, and the + // fraction still resolves to a sane clamped value. + let progress = EpisodeFormatting.downloadProgress(received: .max, total: .max) + #expect(progress?.fraction == 1.0) + #expect(progress?.label == "\(bytes(.max)) / \(bytes(.max))") + } +} + +/// Mirror of `EpisodeFormatting`'s private byte formatter so tests can assert exact +/// labels without depending on the current locale's spelling of a unit. +private func bytes(_ count: Int64) -> String { + let formatter = ByteCountFormatter() + formatter.countStyle = .file + return formatter.string(fromByteCount: count) } diff --git a/shared/src/app.rs b/shared/src/app.rs index 934056d..733aae2 100644 --- a/shared/src/app.rs +++ b/shared/src/app.rs @@ -2,16 +2,18 @@ use crux_core::{render::render, App, Command}; use facet::Facet; use serde::{Deserialize, Serialize}; +use crate::capabilities::download::{DownloadOperation, DownloadResult}; use crate::capabilities::http::{HttpOperation, HttpResult}; use crate::capabilities::storage::{StorageOperation, StorageResult}; -use crate::domain::{Episode, EpisodeSortOrder}; +use crate::domain::{DownloadStatus, Episode, EpisodeSortOrder}; use crate::effect::Effect; use crate::feed_parser::parse_feed; use crate::html::strip_html_preview; -use crate::model::Model; +use crate::model::{DownloadProgress, Model, QueuedDownload}; use crate::theme::{theme_view, ThemeId, ThemeMode}; use crate::view_model::{ - EpisodeSummary, LibraryView, SubscriptionDetailView, SubscriptionSummary, ViewModel, + DownloadNotice, EpisodeSummary, LibraryView, SubscriptionDetailView, SubscriptionSummary, + ViewModel, }; #[derive(Default)] @@ -27,9 +29,14 @@ impl App for Pollux { match event { Event::Started => { model.loading = true; - Command::request_from_shell(StorageOperation::ListSubscriptions) - .then_send(|r| Event::SubscriptionsLoaded(Box::new(r))) - .and(render()) + let subscriptions = + Command::request_from_shell(StorageOperation::ListSubscriptions) + .then_send(|r| Event::SubscriptionsLoaded(Box::new(r))); + // Rebuild the download queue for anything a previous session left + // mid-download, and resume it. + let pending = Command::request_from_shell(StorageOperation::ListPendingDownloads) + .then_send(|r| Event::PendingDownloadsLoaded(Box::new(r))); + subscriptions.and(pending).and(render()) } Event::SubscriptionsLoaded(result) => { model.loading = false; @@ -45,6 +52,45 @@ impl App for Pollux { } render() } + Event::PendingDownloadsLoaded(result) => { + // Rebuild the queue from downloads a prior session left in flight and + // resume them. Partial files don't survive a quit, so each restarts + // from scratch. Best-effort: an error here just leaves nothing queued. + let mut cmd = Command::done(); + if let StorageResult::Episodes(rows) = *result { + // Nothing is in flight at launch, so `maybe_start_next` will start + // the first re-enqueued item (persisting it as `Downloading`). + let will_start_head = model.downloading.is_none(); + for episode in rows { + let already = model.downloading.as_deref() == Some(episode.id.as_str()) + || model + .download_queue + .iter() + .any(|q| q.episode_id == episode.id); + if already { + continue; + } + // The head item is about to be started, so skip a `Queued` write + // that its `Downloading` write would immediately supersede. Items + // waiting behind it are normalized to `Queued` so one left + // "Downloading" by a prior session no longer reads that way. + let is_head = will_start_head && model.download_queue.is_empty(); + if !is_head { + cmd = cmd.and(persist_download_state( + &episode.id, + DownloadStatus::Queued, + None, + None, + )); + } + model.download_queue.push(QueuedDownload { + episode_id: episode.id, + url: episode.enclosure_url, + }); + } + } + cmd.and(maybe_start_next(model)).and(render()) + } Event::FetchFeed(url) => { model.loading = true; model.error = None; @@ -126,6 +172,8 @@ impl App for Pollux { model.episodes.clear(); model.detail_loading = true; model.detail_error = None; + // A notice from the previous feed doesn't apply to this one. + model.download_notice = None; Command::request_from_shell(StorageOperation::ListEpisodesBySubscription { subscription_id: id.clone(), }) @@ -170,6 +218,215 @@ impl App for Pollux { model.episode_sort = order; render() } + Event::DownloadEpisode(episode_id) => { + // A fresh user action supersedes any stale op-failure notice. + model.download_notice = None; + // Only a not-downloaded or failed (retry) episode can start a + // download. Every other state — already queued/downloading, already + // downloaded, removed from feed, or an episode we don't have loaded — + // is a no-op, so a stray or repeated event can't double-enqueue or + // re-download a file we already have. + let found = model + .episodes + .iter() + .find(|e| e.id == episode_id) + .map(|e| (e.download_status.clone(), e.enclosure_url.clone())); + match found { + Some((DownloadStatus::NotDownloaded, url)) + | Some((DownloadStatus::Failed, url)) => { + if !download_allowed(model) { + // Fail-on-full: unlimited today, so this is unreachable. + // When a storage cap exists this is where a download is + // refused up front and marked Failed instead of started. + set_download_state( + model, + &episode_id, + DownloadStatus::Failed, + None, + None, + ); + persist_download_state(&episode_id, DownloadStatus::Failed, None, None) + .and(render()) + } else { + set_download_state( + model, + &episode_id, + DownloadStatus::Queued, + None, + None, + ); + model.download_queue.push(QueuedDownload { + episode_id: episode_id.clone(), + url, + }); + let persist = persist_download_state( + &episode_id, + DownloadStatus::Queued, + None, + None, + ); + persist.and(maybe_start_next(model)).and(render()) + } + } + _ => render(), + } + } + Event::DownloadProgress { + episode_id, + received_bytes, + total_bytes, + } => { + // Track only the active download's progress; drop a late or stale + // report for an episode that already finished or was superseded. + if model.downloading.as_deref() == Some(episode_id.as_str()) { + model.active_download_progress = Some(DownloadProgress { + episode_id, + received_bytes, + total_bytes, + }); + render() + } else { + Command::done() + } + } + Event::DownloadFinished { episode_id, result } => match *result { + DownloadResult::Completed { + local_path, + size_bytes, + } => { + finish_active(model, &episode_id); + set_download_state( + model, + &episode_id, + DownloadStatus::Downloaded, + Some(local_path.clone()), + Some(size_bytes), + ); + let persist = persist_download_state( + &episode_id, + DownloadStatus::Downloaded, + Some(local_path), + Some(size_bytes), + ); + persist.and(maybe_start_next(model)).and(render()) + } + DownloadResult::Error(reason) => { + finish_active(model, &episode_id); + set_download_state(model, &episode_id, DownloadStatus::Failed, None, None); + // Keep the shell's specific reason so the UI can show it beside + // Retry (set_download_state cleared any older one above). + model.download_errors.insert(episode_id.clone(), reason); + let persist = + persist_download_state(&episode_id, DownloadStatus::Failed, None, None); + persist.and(maybe_start_next(model)).and(render()) + } + DownloadResult::Cancelled => { + // The shell cancelled the task and flushed the partial file, so + // the episode goes back to not-downloaded and the queue drains on. + // This is the *sole* owner of a cancelled download's state reset — + // the `Cancel` operation's own ack (DownloadCanceled) does nothing. + finish_active(model, &episode_id); + reset_to_not_downloaded(model, &episode_id) + .and(maybe_start_next(model)) + .and(render()) + } + // Deleted never arrives here (it resolves DeleteDownload instead). + DownloadResult::Deleted => render(), + }, + Event::CancelDownload(episode_id) => { + // A fresh user action supersedes any stale op-failure notice. + model.download_notice = None; + if model.downloading.as_deref() == Some(episode_id.as_str()) { + // Active download: ask the shell to cancel the task and flush the + // partial file. The in-flight Download request then resolves as + // Cancelled (handled above), which finalizes the state. + Command::request_from_shell(DownloadOperation::Cancel { + episode_id: episode_id.clone(), + }) + .then_send(|r| Event::DownloadCanceled(Box::new(r))) + .and(render()) + } else if let Some(pos) = model + .download_queue + .iter() + .position(|q| q.episode_id == episode_id) + { + // Queued but not started: just drop it from the queue and reset — + // nothing is running and no file exists yet, so no shell cancel is + // needed and the reset happens synchronously here. + model.download_queue.remove(pos); + reset_to_not_downloaded(model, &episode_id).and(render()) + } else { + // Not downloading and not queued — nothing to cancel. + render() + } + } + Event::DownloadCanceled(_result) => { + // The `Cancel` request must be resolved (the shell resolves every + // request), so this is its required sink — nothing more. The actual + // state reset is owned entirely by the in-flight `Download` request + // resolving `Cancelled` (see DownloadFinished above); if the cancel + // somehow didn't take, that download simply resolves normally, so + // there is nothing to surface here. + render() + } + Event::DeleteDownload(episode_id) => { + // A fresh user action supersedes any stale op-failure notice. + model.download_notice = None; + let local_path = model + .episodes + .iter() + .find(|e| e.id == episode_id) + .and_then(|e| e.local_path.clone()); + match local_path { + Some(path) => { + let id = episode_id.clone(); + Command::request_from_shell(DownloadOperation::Delete { local_path: path }) + .then_send(move |r| Event::DownloadDeleted { + episode_id: id.clone(), + result: Box::new(r), + }) + .and(render()) + } + None => { + // Nothing on disk (e.g. a Failed row): normalize to + // NotDownloaded without a filesystem round-trip. + reset_to_not_downloaded(model, &episode_id).and(render()) + } + } + } + Event::DownloadDeleted { episode_id, result } => match *result { + DownloadResult::Deleted => { + reset_to_not_downloaded(model, &episode_id).and(render()) + } + DownloadResult::Error(e) => { + // The file couldn't be removed; leave the row (still Downloaded) as + // it is and surface why through the non-blocking notice rather than + // the list-load error, which would hide every episode and control. + model.download_notice = Some(DownloadNotice { + episode_id, + message: format!("Couldn't remove the download: {e}"), + }); + render() + } + // Only Deleted/Error arrive from a Delete request; the rest are + // unreachable but keep the match total. + DownloadResult::Completed { .. } | DownloadResult::Cancelled => render(), + }, + Event::DownloadStatePersisted { episode_id, result } => { + // A durability failure: the UI already shows the correct optimistic + // state, so surface this as a non-blocking notice, not the list error. + // Success is silent — notably it does NOT clear an existing notice, so + // an unrelated episode's persist can't dismiss another episode's failure + // message. The notice clears on the next user download action (see + // DownloadEpisode/DeleteDownload/CancelDownload) or on a feed switch. + if let StorageResult::Error(e) = *result { + model.download_notice = Some(DownloadNotice { + episode_id, + message: format!("Couldn't save the download's state: {e}"), + }); + } + render() + } Event::SetTheme { id, mode } => { // Records the theme choice and re-renders; resolution to platform // colors happens shell-side. No UI emits this yet — it's the seam @@ -216,6 +473,24 @@ fn build_subscription_detail(model: &Model) -> SubscriptionDetailView { let mut episodes: Vec = model.episodes.iter().map(episode_summary).collect(); sort_episodes(&mut episodes, model.episode_sort); + // Overlay the in-flight download's live byte progress onto its row. Progress is + // transient runtime state (not stored on the Episode), so it is injected here at + // projection time for the one episode currently downloading. + if let Some(progress) = model.active_download_progress.as_ref() { + if let Some(summary) = episodes.iter_mut().find(|e| e.id == progress.episode_id) { + summary.download_received_bytes = Some(progress.received_bytes); + summary.download_total_bytes = progress.total_bytes; + } + } + + // Overlay each failed episode's reason (also transient, not stored on the Episode). + // Skipped entirely when nothing has failed, which is the common case. + if !model.download_errors.is_empty() { + for summary in episodes.iter_mut() { + summary.download_error = model.download_errors.get(&summary.id).cloned(); + } + } + let (subscription_id, title, artwork_url) = match &model.selected_subscription { Some(s) => (Some(s.id.clone()), s.title.clone(), s.artwork_url.clone()), None => (None, String::new(), None), @@ -229,6 +504,7 @@ fn build_subscription_detail(model: &Model) -> SubscriptionDetailView { sort_order: model.episode_sort, loading: model.detail_loading, error: model.detail_error.clone(), + download_notice: model.download_notice.clone(), } } @@ -256,7 +532,11 @@ fn episode_summary(e: &Episode) -> EpisodeSummary { playback_status: e.playback_status.clone(), playback_position_secs: e.playback_position_secs, download_status: e.download_status.clone(), - download_progress: e.download_progress, + // Live progress and the failure reason are overlaid in + // `build_subscription_detail`; every row ships without them here. + download_received_bytes: None, + download_total_bytes: None, + download_error: None, } } @@ -278,11 +558,122 @@ fn sort_episodes(episodes: &mut [EpisodeSummary], order: EpisodeSortOrder) { } } +/// Pre-download storage check. Unlimited today — the user-configurable soft cap +/// arrives with the Settings screen (see ROADMAP follow-ups). Device-full is not +/// predicted here; it surfaces as the shell's write failing (`DownloadResult::Error` +/// → `Failed`), which is the fail-on-full behavior the storage spec calls for. +fn download_allowed(_model: &Model) -> bool { + true +} + +/// Applies a download-state transition to the in-memory episode, if it's loaded. +/// +/// `SelectSubscription` is intentionally idempotent (it won't reload the feed +/// already on screen), so a status change written only to storage would not show +/// up on the details page. Mutating `model.episodes` in place is what lets `view()` +/// reproject the new state without a reload — see the note at `SelectSubscription`. +/// `local_path`/`size_bytes`/`progress` are set (or cleared) together with the +/// status so the row never keeps a path for a file that isn't there. +fn set_download_state( + model: &mut Model, + episode_id: &str, + status: DownloadStatus, + local_path: Option, + size_bytes: Option, +) { + // A recorded failure reason is only meaningful while the episode is Failed; every + // other transition drops it. The `Error` handler re-inserts after calling this. + if !matches!(status, DownloadStatus::Failed) { + model.download_errors.remove(episode_id); + } + if let Some(episode) = model.episodes.iter_mut().find(|e| e.id == episode_id) { + episode.download_status = status; + episode.local_path = local_path; + episode.file_size_bytes = size_bytes; + } +} + +/// Persists a download-state transition through the storage capability. The result +/// is checked for errors (via `DownloadStatePersisted`) but success is silent — the +/// UI already reflects the change from `set_download_state`, so persistence is only +/// about durability across restarts. +fn persist_download_state( + episode_id: &str, + status: DownloadStatus, + local_path: Option, + size_bytes: Option, +) -> Command { + // Capture the id so a persistence failure's notice can be tagged with its episode. + let id = episode_id.to_string(); + Command::request_from_shell(StorageOperation::UpdateDownloadState { + episode_id: episode_id.to_string(), + status, + local_path, + size_bytes, + }) + .then_send(move |r| Event::DownloadStatePersisted { + episode_id: id.clone(), + result: Box::new(r), + }) +} + +/// Resets an episode to not-downloaded (in the model) and returns the command that +/// persists it. The single shape used everywhere a download ends without a file — +/// cancelling an active or queued download, and deleting a stored one. +fn reset_to_not_downloaded(model: &mut Model, episode_id: &str) -> Command { + set_download_state(model, episode_id, DownloadStatus::NotDownloaded, None, None); + persist_download_state(episode_id, DownloadStatus::NotDownloaded, None, None) +} + +/// Clears the in-flight marker (and its live progress) when the active download +/// reaches a terminal state. +fn finish_active(model: &mut Model, episode_id: &str) { + if model.downloading.as_deref() == Some(episode_id) { + model.downloading = None; + model.active_download_progress = None; + } +} + +/// Starts the next queued download if nothing is in flight (serial: one at a time). +/// Returns the command that marks the episode `Downloading`, persists that, and +/// issues the shell download whose result comes back as `DownloadFinished`. A no-op +/// (empty command) when a download is already running or the queue is empty. +fn maybe_start_next(model: &mut Model) -> Command { + if model.downloading.is_some() { + return Command::done(); + } + if model.download_queue.is_empty() { + return Command::done(); + } + // The queue entry carries the URL, so a download can start without its feed's + // episodes being loaded (e.g. a resume at launch). + let QueuedDownload { episode_id, url } = model.download_queue.remove(0); + + model.downloading = Some(episode_id.clone()); + // A fresh download starts with no progress until the shell reports the first bytes. + model.active_download_progress = None; + set_download_state(model, &episode_id, DownloadStatus::Downloading, None, None); + + let persist = persist_download_state(&episode_id, DownloadStatus::Downloading, None, None); + let download = Command::request_from_shell(DownloadOperation::Download { + episode_id: episode_id.clone(), + url, + }) + .then_send(move |r| Event::DownloadFinished { + episode_id: episode_id.clone(), + result: Box::new(r), + }); + persist.and(download) +} + #[derive(Facet, Serialize, Deserialize, Clone, Debug)] #[repr(C)] pub enum Event { Started, SubscriptionsLoaded(Box), + /// Episodes left `Downloading`/`Queued` by a previous session, loaded at launch + /// so their downloads can be re-enqueued and resumed. + PendingDownloadsLoaded(Box), FetchFeed(String), FeedFetched { url: String, @@ -295,6 +686,38 @@ pub enum Event { result: Box, }, SetEpisodeSort(EpisodeSortOrder), + /// Queue an episode's audio for download (or retry a failed one). + DownloadEpisode(String), + /// Remove a downloaded (or failed) episode's local file. + DeleteDownload(String), + /// Stop an in-flight or queued download and reset the episode to not-downloaded. + CancelDownload(String), + /// Required resolution sink for the `Cancel` request — carries no state change. + /// The reset is owned by the in-flight `Download` request resolving `Cancelled`. + DownloadCanceled(Box), + /// Live byte progress for the in-flight download, reported by the shell while + /// the one-shot download request is still running. Transient — not persisted. + DownloadProgress { + episode_id: String, + received_bytes: u64, + total_bytes: Option, + }, + /// Terminal result of a shell download for `episode_id`. + DownloadFinished { + episode_id: String, + result: Box, + }, + /// Terminal result of a shell delete for `episode_id`. + DownloadDeleted { + episode_id: String, + result: Box, + }, + /// Result of persisting a download-state transition. Success is silent; an + /// error is surfaced on the details page (downloads are driven from there). + DownloadStatePersisted { + episode_id: String, + result: Box, + }, /// Change the active theme. Not yet emitted by any UI — the seam for the /// Settings appearance section (see `docs/features/theme.md`). SetTheme { @@ -335,7 +758,6 @@ mod tests { playback_status: PlaybackStatus::Unplayed, playback_position_secs: None, download_status: DownloadStatus::NotDownloaded, - download_progress: None, is_flagged: false, file_size_bytes: None, local_path: None, @@ -405,13 +827,22 @@ mod tests { assert!(model.loading); let effects: Vec = cmd.effects().collect(); - assert_eq!(effects.len(), 2); + // ListSubscriptions + ListPendingDownloads + render. + assert_eq!(effects.len(), 3); let has_storage = effects .iter() .any(|e| matches!(e, Effect::Storage(r) if matches!(r.operation, StorageOperation::ListSubscriptions))); assert!(has_storage, "expected a ListSubscriptions storage effect"); + let has_pending = effects + .iter() + .any(|e| matches!(e, Effect::Storage(r) if matches!(r.operation, StorageOperation::ListPendingDownloads))); + assert!( + has_pending, + "expected a ListPendingDownloads storage effect to resume interrupted downloads" + ); + let has_render = effects.iter().any(|e| matches!(e, Effect::Render(_))); assert!(has_render, "expected a render effect"); } @@ -1085,4 +1516,690 @@ mod tests { assert!(summary.description.is_none()); assert!(summary.description_text.is_none()); } + + // --- Download manager --- + + fn model_episode_status(model: &Model, id: &str) -> DownloadStatus { + model + .episodes + .iter() + .find(|e| e.id == id) + .map(|e| e.download_status.clone()) + .expect("episode should be loaded") + } + + fn queued_ids(model: &Model) -> Vec { + model + .download_queue + .iter() + .map(|q| q.episode_id.clone()) + .collect() + } + + fn has_download_effect_for(effects: &[Effect], episode_id: &str) -> bool { + effects.iter().any(|e| { + matches!( + e, + Effect::Download(r) if matches!(&r.operation, + DownloadOperation::Download { episode_id: id, .. } if id == episode_id) + ) + }) + } + + #[test] + fn download_episode_marks_downloading_and_issues_download_effect() { + let app = Pollux; + let mut model = Model::default(); + load_episodes(&app, &mut model, vec![make_episode("e1", "Ep", Some(1))]); + + let mut cmd = app.update(Event::DownloadEpisode("e1".to_string()), &mut model); + + // Serial: nothing else in flight, so it starts immediately. + assert_eq!(model.downloading.as_deref(), Some("e1")); + assert_eq!( + model_episode_status(&model, "e1"), + DownloadStatus::Downloading + ); + assert!( + model.download_queue.is_empty(), + "the only item should be in flight, not queued" + ); + + let effects: Vec = cmd.effects().collect(); + assert!( + has_download_effect_for(&effects, "e1"), + "expected a Download effect for e1" + ); + + // The details page reflects the new state without any re-selection — the + // in-place model update is what makes idempotent SelectSubscription safe. + let view = app.view(&model); + assert_eq!( + view.subscription_detail.episodes[0].download_status, + DownloadStatus::Downloading + ); + } + + #[test] + fn second_download_stays_queued_while_one_is_in_flight() { + let app = Pollux; + let mut model = Model::default(); + load_episodes( + &app, + &mut model, + vec![ + make_episode("e1", "First", Some(2)), + make_episode("e2", "Second", Some(1)), + ], + ); + + let _ = app.update(Event::DownloadEpisode("e1".to_string()), &mut model); + let mut cmd = app.update(Event::DownloadEpisode("e2".to_string()), &mut model); + + // e1 is still the one downloading; e2 waits its turn. + assert_eq!(model.downloading.as_deref(), Some("e1")); + assert_eq!(model_episode_status(&model, "e2"), DownloadStatus::Queued); + assert_eq!(queued_ids(&model), vec!["e2"]); + + let effects: Vec = cmd.effects().collect(); + assert!( + !has_download_effect_for(&effects, "e2"), + "a second download must not start while one is in flight (serial)" + ); + } + + #[test] + fn download_completed_marks_downloaded_and_starts_next() { + let app = Pollux; + let mut model = Model::default(); + load_episodes( + &app, + &mut model, + vec![ + make_episode("e1", "First", Some(2)), + make_episode("e2", "Second", Some(1)), + ], + ); + let _ = app.update(Event::DownloadEpisode("e1".to_string()), &mut model); + let _ = app.update(Event::DownloadEpisode("e2".to_string()), &mut model); + + let mut cmd = app.update( + Event::DownloadFinished { + episode_id: "e1".to_string(), + result: Box::new(DownloadResult::Completed { + local_path: "Downloads/e1.mp3".to_string(), + size_bytes: 4_096, + }), + }, + &mut model, + ); + + // e1 is done, with its file recorded. + assert_eq!( + model_episode_status(&model, "e1"), + DownloadStatus::Downloaded + ); + let e1 = model + .episodes + .iter() + .find(|e| e.id == "e1") + .expect("e1 should be loaded"); + assert_eq!(e1.local_path.as_deref(), Some("Downloads/e1.mp3")); + assert_eq!(e1.file_size_bytes, Some(4_096)); + + // The queue drains to e2. + assert_eq!(model.downloading.as_deref(), Some("e2")); + assert_eq!( + model_episode_status(&model, "e2"), + DownloadStatus::Downloading + ); + assert!(model.download_queue.is_empty()); + + let effects: Vec = cmd.effects().collect(); + assert!( + has_download_effect_for(&effects, "e2"), + "finishing one download should start the next queued one" + ); + } + + #[test] + fn download_error_marks_failed_and_still_drains_queue() { + let app = Pollux; + let mut model = Model::default(); + load_episodes( + &app, + &mut model, + vec![ + make_episode("e1", "First", Some(2)), + make_episode("e2", "Second", Some(1)), + ], + ); + let _ = app.update(Event::DownloadEpisode("e1".to_string()), &mut model); + let _ = app.update(Event::DownloadEpisode("e2".to_string()), &mut model); + + let mut cmd = app.update( + Event::DownloadFinished { + episode_id: "e1".to_string(), + result: Box::new(DownloadResult::Error("disk full".to_string())), + }, + &mut model, + ); + + assert_eq!(model_episode_status(&model, "e1"), DownloadStatus::Failed); + // A failure must not stall the queue — the next episode still starts. + assert_eq!(model.downloading.as_deref(), Some("e2")); + let effects: Vec = cmd.effects().collect(); + assert!(has_download_effect_for(&effects, "e2")); + } + + #[test] + fn download_failure_reason_is_exposed_on_the_episode() { + let app = Pollux; + let mut model = Model::default(); + load_episodes(&app, &mut model, vec![make_episode("e1", "Ep", Some(1))]); + let _ = app.update(Event::DownloadEpisode("e1".to_string()), &mut model); + + let _ = app.update( + Event::DownloadFinished { + episode_id: "e1".to_string(), + result: Box::new(DownloadResult::Error("disk full".to_string())), + }, + &mut model, + ); + + // The specific reason rides along on the failed episode's summary. + let summary = &app.view(&model).subscription_detail.episodes[0]; + assert_eq!(summary.download_status, DownloadStatus::Failed); + assert_eq!(summary.download_error.as_deref(), Some("disk full")); + } + + #[test] + fn retrying_a_failed_download_clears_the_reason() { + let app = Pollux; + let mut model = Model::default(); + load_episodes(&app, &mut model, vec![make_episode("e1", "Ep", Some(1))]); + let _ = app.update(Event::DownloadEpisode("e1".to_string()), &mut model); + let _ = app.update( + Event::DownloadFinished { + episode_id: "e1".to_string(), + result: Box::new(DownloadResult::Error("boom".to_string())), + }, + &mut model, + ); + assert!(!model.download_errors.is_empty()); + + // Retrying (a DownloadEpisode on the failed episode) clears the reason and + // starts over. + let _ = app.update(Event::DownloadEpisode("e1".to_string()), &mut model); + assert!(model.download_errors.is_empty()); + let summary = &app.view(&model).subscription_detail.episodes[0]; + assert!(summary.download_error.is_none()); + assert_eq!(summary.download_status, DownloadStatus::Downloading); + } + + #[test] + fn delete_download_issues_delete_effect_then_resets_on_result() { + let app = Pollux; + let mut model = Model::default(); + let mut downloaded = make_episode("e1", "Ep", Some(1)); + downloaded.download_status = DownloadStatus::Downloaded; + downloaded.local_path = Some("Downloads/e1.mp3".to_string()); + downloaded.file_size_bytes = Some(4_096); + load_episodes(&app, &mut model, vec![downloaded]); + + // Deleting a downloaded episode goes to the shell first (best-effort file + // removal), so the row stays Downloaded until the delete result comes back. + let mut cmd = app.update(Event::DeleteDownload("e1".to_string()), &mut model); + assert_eq!( + model_episode_status(&model, "e1"), + DownloadStatus::Downloaded + ); + let effects: Vec = cmd.effects().collect(); + assert!( + effects.iter().any(|e| matches!( + e, + Effect::Download(r) if matches!(&r.operation, + DownloadOperation::Delete { local_path } if local_path == "Downloads/e1.mp3") + )), + "expected a Delete effect carrying the stored path" + ); + + // The shell reports the file gone. + let _ = app.update( + Event::DownloadDeleted { + episode_id: "e1".to_string(), + result: Box::new(DownloadResult::Deleted), + }, + &mut model, + ); + assert_eq!( + model_episode_status(&model, "e1"), + DownloadStatus::NotDownloaded + ); + let e1 = model + .episodes + .iter() + .find(|e| e.id == "e1") + .expect("e1 should be loaded"); + assert!(e1.local_path.is_none(), "path must be cleared after delete"); + assert!(e1.file_size_bytes.is_none()); + } + + #[test] + fn delete_without_a_local_file_normalizes_immediately() { + let app = Pollux; + let mut model = Model::default(); + let mut failed = make_episode("e1", "Ep", Some(1)); + failed.download_status = DownloadStatus::Failed; + load_episodes(&app, &mut model, vec![failed]); + + // A Failed row has no file, so there's no shell round-trip — it just resets. + let mut cmd = app.update(Event::DeleteDownload("e1".to_string()), &mut model); + assert_eq!( + model_episode_status(&model, "e1"), + DownloadStatus::NotDownloaded + ); + let effects: Vec = cmd.effects().collect(); + assert!( + !effects.iter().any(|e| matches!(e, Effect::Download(_))), + "no file means no Delete effect" + ); + } + + #[test] + fn download_persist_error_surfaces_as_a_notice_not_the_list_error() { + let app = Pollux; + let mut model = Model::default(); + // A pre-existing list-load error must survive a download op-failure. + model.detail_error = Some("earlier list load failed".to_string()); + + let _ = app.update( + Event::DownloadStatePersisted { + episode_id: "e1".to_string(), + result: Box::new(StorageResult::Error("db locked".to_string())), + }, + &mut model, + ); + + // The op failure lands in the non-blocking notice, tagged with its episode, and + // the list error (which would blank the whole episode list) is untouched. + let notice = model.download_notice.as_ref().expect("a notice"); + assert_eq!(notice.episode_id, "e1"); + assert!(notice.message.contains("db locked")); + assert_eq!( + model.detail_error.as_deref(), + Some("earlier list load failed") + ); + } + + #[test] + fn an_unrelated_successful_persist_does_not_clear_the_notice() { + // A different episode's persist succeeding must NOT dismiss a standing + // op-failure notice — otherwise a background download completing could wipe + // the message before the user sees it. The notice clears on a user action or + // feed switch instead (covered by other tests). + let app = Pollux; + let mut model = Model::default(); + model.download_notice = Some(DownloadNotice { + episode_id: "e1".to_string(), + message: "Couldn't remove the download: disk busy".to_string(), + }); + + // A *different* episode's persist succeeds. + let _ = app.update( + Event::DownloadStatePersisted { + episode_id: "e2".to_string(), + result: Box::new(StorageResult::Success), + }, + &mut model, + ); + + let notice = model.download_notice.as_ref().expect("notice retained"); + assert_eq!(notice.episode_id, "e1"); + assert_eq!(notice.message, "Couldn't remove the download: disk busy"); + } + + #[test] + fn a_fresh_download_action_clears_a_stale_download_notice() { + let app = Pollux; + let mut model = Model::default(); + load_episodes(&app, &mut model, vec![make_episode("e1", "Ep", Some(1))]); + model.download_notice = Some(DownloadNotice { + episode_id: "e1".to_string(), + message: "Couldn't remove the download: busy".to_string(), + }); + + let _ = app.update(Event::DownloadEpisode("e1".to_string()), &mut model); + + assert!(model.download_notice.is_none()); + } + + #[test] + fn delete_failure_surfaces_as_a_notice_and_keeps_the_row() { + let app = Pollux; + let mut model = Model::default(); + let mut downloaded = make_episode("e1", "Ep", Some(1)); + downloaded.download_status = DownloadStatus::Downloaded; + downloaded.local_path = Some("Downloads/e1.mp3".to_string()); + load_episodes(&app, &mut model, vec![downloaded]); + let _ = app.update(Event::DeleteDownload("e1".to_string()), &mut model); + + // The shell couldn't remove the file. + let _ = app.update( + Event::DownloadDeleted { + episode_id: "e1".to_string(), + result: Box::new(DownloadResult::Error("disk busy".to_string())), + }, + &mut model, + ); + + // The row stays Downloaded (the file is presumably still there) and the + // failure lands in the non-blocking notice (tagged with e1), not the list error. + assert_eq!( + model_episode_status(&model, "e1"), + DownloadStatus::Downloaded + ); + let notice = model.download_notice.as_ref().expect("a notice"); + assert_eq!(notice.episode_id, "e1"); + assert!(notice.message.contains("disk busy")); + assert!(model.detail_error.is_none()); + } + + #[test] + fn selecting_a_subscription_clears_a_stale_download_notice() { + let app = Pollux; + let mut model = Model::default(); + model.subscriptions = vec![make_subscription("sub-1", "Feed")]; + model.download_notice = Some(DownloadNotice { + episode_id: "e1".to_string(), + message: "Couldn't save the download's state: db locked".to_string(), + }); + + let _ = app.update(Event::SelectSubscription("sub-1".to_string()), &mut model); + + assert!( + model.download_notice.is_none(), + "a notice from a previous feed shouldn't carry into another" + ); + } + + #[test] + fn download_progress_overlays_bytes_on_the_active_episode() { + let app = Pollux; + let mut model = Model::default(); + load_episodes(&app, &mut model, vec![make_episode("e1", "Ep", Some(1))]); + let _ = app.update(Event::DownloadEpisode("e1".to_string()), &mut model); + + let _ = app.update( + Event::DownloadProgress { + episode_id: "e1".to_string(), + received_bytes: 512, + total_bytes: Some(1024), + }, + &mut model, + ); + + let summary = &app.view(&model).subscription_detail.episodes[0]; + assert_eq!(summary.download_received_bytes, Some(512)); + assert_eq!(summary.download_total_bytes, Some(1024)); + } + + #[test] + fn download_progress_for_an_inactive_episode_is_ignored() { + let app = Pollux; + let mut model = Model::default(); + load_episodes(&app, &mut model, vec![make_episode("e1", "Ep", Some(1))]); + + // Nothing is downloading, so a stray progress report must not stick. + let _ = app.update( + Event::DownloadProgress { + episode_id: "e1".to_string(), + received_bytes: 10, + total_bytes: None, + }, + &mut model, + ); + + assert!(model.active_download_progress.is_none()); + let summary = &app.view(&model).subscription_detail.episodes[0]; + assert!(summary.download_received_bytes.is_none()); + } + + #[test] + fn progress_is_cleared_when_the_download_finishes() { + let app = Pollux; + let mut model = Model::default(); + load_episodes(&app, &mut model, vec![make_episode("e1", "Ep", Some(1))]); + let _ = app.update(Event::DownloadEpisode("e1".to_string()), &mut model); + let _ = app.update( + Event::DownloadProgress { + episode_id: "e1".to_string(), + received_bytes: 512, + total_bytes: Some(1024), + }, + &mut model, + ); + assert!(model.active_download_progress.is_some()); + + let _ = app.update( + Event::DownloadFinished { + episode_id: "e1".to_string(), + result: Box::new(DownloadResult::Completed { + local_path: "Downloads/e1.mp3".to_string(), + size_bytes: 1024, + }), + }, + &mut model, + ); + + assert!( + model.active_download_progress.is_none(), + "progress must be cleared once the download ends" + ); + let summary = &app.view(&model).subscription_detail.episodes[0]; + assert!(summary.download_received_bytes.is_none()); + } + + #[test] + fn pending_downloads_are_reenqueued_and_resumed_on_load() { + let app = Pollux; + let mut model = Model::default(); + + // Two episodes a previous session left mid-download (their feed is not + // loaded — mirroring a cold start before any subscription is selected). + let mut e1 = make_episode("e1", "First", Some(2)); + e1.download_status = DownloadStatus::Downloading; + let mut e2 = make_episode("e2", "Second", Some(1)); + e2.download_status = DownloadStatus::Queued; + + let mut cmd = app.update( + Event::PendingDownloadsLoaded(Box::new(StorageResult::Episodes(vec![e1, e2]))), + &mut model, + ); + + // Serial: the first resumes, the rest wait — even though model.episodes is + // empty, because the queue carries the URL. + assert_eq!(model.downloading.as_deref(), Some("e1")); + assert_eq!(queued_ids(&model), vec!["e2"]); + + let effects: Vec = cmd.effects().collect(); + assert!( + has_download_effect_for(&effects, "e1"), + "an interrupted download should resume at launch" + ); + + // The head item (e1) is started immediately, so it's persisted only as + // Downloading — no redundant Queued write that Downloading would supersede. + // The item waiting behind it (e2) is normalized to Queued. + let persisted: Vec<(&str, &DownloadStatus)> = effects + .iter() + .filter_map(|e| match e { + Effect::Storage(r) => match &r.operation { + StorageOperation::UpdateDownloadState { + episode_id, status, .. + } => Some((episode_id.as_str(), status)), + _ => None, + }, + _ => None, + }) + .collect(); + assert_eq!( + persisted, + vec![ + ("e2", &DownloadStatus::Queued), + ("e1", &DownloadStatus::Downloading), + ], + "head persists Downloading only; the rest persist Queued" + ); + } + + #[test] + fn pending_downloads_loaded_empty_is_a_noop() { + let app = Pollux; + let mut model = Model::default(); + + let mut cmd = app.update( + Event::PendingDownloadsLoaded(Box::new(StorageResult::Episodes(vec![]))), + &mut model, + ); + + assert!(model.downloading.is_none()); + assert!(model.download_queue.is_empty()); + let effects: Vec = cmd.effects().collect(); + assert!(!effects.iter().any(|e| matches!(e, Effect::Download(_)))); + } + + #[test] + fn cancel_active_download_requests_a_shell_cancel() { + let app = Pollux; + let mut model = Model::default(); + load_episodes(&app, &mut model, vec![make_episode("e1", "Ep", Some(1))]); + let _ = app.update(Event::DownloadEpisode("e1".to_string()), &mut model); + assert_eq!(model.downloading.as_deref(), Some("e1")); + + let mut cmd = app.update(Event::CancelDownload("e1".to_string()), &mut model); + let effects: Vec = cmd.effects().collect(); + assert!( + effects.iter().any(|e| matches!( + e, + Effect::Download(r) if matches!(&r.operation, + DownloadOperation::Cancel { episode_id } if episode_id == "e1") + )), + "cancelling the active download should ask the shell to cancel the task" + ); + } + + #[test] + fn download_finished_cancelled_resets_and_starts_next() { + let app = Pollux; + let mut model = Model::default(); + load_episodes( + &app, + &mut model, + vec![ + make_episode("e1", "First", Some(2)), + make_episode("e2", "Second", Some(1)), + ], + ); + let _ = app.update(Event::DownloadEpisode("e1".to_string()), &mut model); + let _ = app.update(Event::DownloadEpisode("e2".to_string()), &mut model); + let _ = app.update(Event::CancelDownload("e1".to_string()), &mut model); + + // The shell reports the cancellation back through the Download request. + let mut cmd = app.update( + Event::DownloadFinished { + episode_id: "e1".to_string(), + result: Box::new(DownloadResult::Cancelled), + }, + &mut model, + ); + + assert_eq!( + model_episode_status(&model, "e1"), + DownloadStatus::NotDownloaded, + "a cancelled download goes back to not-downloaded, not failed" + ); + assert!(model.active_download_progress.is_none()); + // The queue drains to e2. + assert_eq!(model.downloading.as_deref(), Some("e2")); + let effects: Vec = cmd.effects().collect(); + assert!(has_download_effect_for(&effects, "e2")); + } + + #[test] + fn cancel_queued_download_drops_it_without_a_shell_cancel() { + let app = Pollux; + let mut model = Model::default(); + load_episodes( + &app, + &mut model, + vec![ + make_episode("e1", "First", Some(2)), + make_episode("e2", "Second", Some(1)), + ], + ); + let _ = app.update(Event::DownloadEpisode("e1".to_string()), &mut model); + let _ = app.update(Event::DownloadEpisode("e2".to_string()), &mut model); + assert_eq!(queued_ids(&model), vec!["e2"]); + + let mut cmd = app.update(Event::CancelDownload("e2".to_string()), &mut model); + + // e2 leaves the queue and resets; e1 keeps downloading. + assert!(model.download_queue.is_empty()); + assert_eq!( + model_episode_status(&model, "e2"), + DownloadStatus::NotDownloaded + ); + assert_eq!(model.downloading.as_deref(), Some("e1")); + let effects: Vec = cmd.effects().collect(); + assert!( + !effects.iter().any(|e| matches!( + e, + Effect::Download(r) if matches!(&r.operation, DownloadOperation::Cancel { .. }) + )), + "a queued item needs no shell cancel — nothing is running" + ); + } + + #[test] + fn downloading_an_already_downloaded_episode_is_a_noop() { + let app = Pollux; + let mut model = Model::default(); + let mut downloaded = make_episode("e1", "Ep", Some(1)); + downloaded.download_status = DownloadStatus::Downloaded; + downloaded.local_path = Some("Downloads/e1.mp3".to_string()); + load_episodes(&app, &mut model, vec![downloaded]); + + // A stray DownloadEpisode must not re-download a file we already have. + let mut cmd = app.update(Event::DownloadEpisode("e1".to_string()), &mut model); + + assert_eq!( + model_episode_status(&model, "e1"), + DownloadStatus::Downloaded + ); + assert!(model.download_queue.is_empty()); + assert!(model.downloading.is_none()); + let effects: Vec = cmd.effects().collect(); + assert!( + !has_download_effect_for(&effects, "e1"), + "downloaded episode must not be re-fetched" + ); + } + + #[test] + fn redownloading_an_in_flight_episode_is_a_noop() { + let app = Pollux; + let mut model = Model::default(); + load_episodes(&app, &mut model, vec![make_episode("e1", "Ep", Some(1))]); + let _ = app.update(Event::DownloadEpisode("e1".to_string()), &mut model); + + // Tapping download again while it's downloading must not re-enqueue. + let mut cmd = app.update(Event::DownloadEpisode("e1".to_string()), &mut model); + assert!(model.download_queue.is_empty()); + let effects: Vec = cmd.effects().collect(); + assert!( + !has_download_effect_for(&effects, "e1"), + "a redundant download request must not issue a second fetch" + ); + } } diff --git a/shared/src/capabilities/download.rs b/shared/src/capabilities/download.rs new file mode 100644 index 0000000..1f8b9ea --- /dev/null +++ b/shared/src/capabilities/download.rs @@ -0,0 +1,47 @@ +use crux_core::capability::Operation; +use facet::Facet; +use serde::{Deserialize, Serialize}; + +/// Side-effects the shell performs against episode audio files. Actual network +/// and filesystem I/O lives in the shell; the core only decides *when* to fetch +/// or delete and records the outcome. Downloads are driven one at a time — the +/// serial guard is in the core (see `Model::downloading`), so each operation the +/// shell receives is an independent one-shot. +#[derive(Facet, Serialize, Deserialize, Clone, Debug)] +#[repr(C)] +pub enum DownloadOperation { + /// Fetch `url` and store it locally for `episode_id`. + Download { episode_id: String, url: String }, + /// Remove a previously downloaded file. Idempotent: deleting a file that is + /// already gone still succeeds. + Delete { local_path: String }, + /// Cancel the in-flight download for `episode_id` and flush any partial bytes. + /// The pending `Download` request then resolves as `Cancelled`. + Cancel { episode_id: String }, +} + +#[derive(Facet, Serialize, Deserialize, Clone, Debug)] +#[repr(C)] +pub enum DownloadResult { + /// The download finished; the file is at `local_path` (relative to the app's + /// storage root, resolved to absolute by the shell) and is `size_bytes` long. + Completed { + local_path: String, + size_bytes: u64, + }, + /// A delete finished (or the file was already absent). + Deleted, + /// The download was cancelled; any partial file has been flushed. + Cancelled, + Error(String), +} + +// Interim download progress is NOT a `DownloadResult`: it is reported by the shell +// as an ordinary `Event::DownloadProgress` (received/total bytes) while the one-shot +// `Download` request is still in flight. That keeps the download a simple one-shot +// (terminal Completed/Error) and keeps progress — which is transient and never +// persisted — out of the storage-backed result type. + +impl Operation for DownloadOperation { + type Output = DownloadResult; +} diff --git a/shared/src/capabilities/mod.rs b/shared/src/capabilities/mod.rs index c581df4..497baa4 100644 --- a/shared/src/capabilities/mod.rs +++ b/shared/src/capabilities/mod.rs @@ -1,2 +1,3 @@ +pub mod download; pub mod http; pub mod storage; diff --git a/shared/src/capabilities/storage.rs b/shared/src/capabilities/storage.rs index 870979a..860a1d5 100644 --- a/shared/src/capabilities/storage.rs +++ b/shared/src/capabilities/storage.rs @@ -2,7 +2,7 @@ use crux_core::capability::Operation; use facet::Facet; use serde::{Deserialize, Serialize}; -use crate::domain::{Episode, PlaybackStatus, Subscription}; +use crate::domain::{DownloadStatus, Episode, PlaybackStatus, Subscription}; #[derive(Facet, Serialize, Deserialize, Clone, Debug)] #[repr(C)] @@ -22,6 +22,10 @@ pub enum StorageOperation { ListEpisodesBySubscription { subscription_id: String, }, + /// Every episode currently marked `Downloading` or `Queued`, across all feeds. + /// Used at launch to rebuild the download queue for downloads interrupted by a + /// previous quit. + ListPendingDownloads, GetEpisodeByFeedGuid { subscription_id: String, feed_guid: String, @@ -31,6 +35,16 @@ pub enum StorageOperation { status: PlaybackStatus, position_secs: Option, }, + /// Persists a download-state transition (status plus the file metadata that + /// comes with it). `local_path`/`size_bytes` are cleared to NULL when `None` — + /// e.g. on delete or failure — so the row never keeps a stale path for a file + /// that is no longer there. (Live byte progress is transient and never stored.) + UpdateDownloadState { + episode_id: String, + status: DownloadStatus, + local_path: Option, + size_bytes: Option, + }, UpsertFeedWithEpisodes { subscription: Subscription, episodes: Vec, diff --git a/shared/src/domain/episode.rs b/shared/src/domain/episode.rs index 55e9f53..1f23086 100644 --- a/shared/src/domain/episode.rs +++ b/shared/src/domain/episode.rs @@ -46,7 +46,6 @@ pub struct Episode { pub playback_status: PlaybackStatus, pub playback_position_secs: Option, pub download_status: DownloadStatus, - pub download_progress: Option, pub is_flagged: bool, pub file_size_bytes: Option, pub local_path: Option, diff --git a/shared/src/effect.rs b/shared/src/effect.rs index 7f28ac9..242c756 100644 --- a/shared/src/effect.rs +++ b/shared/src/effect.rs @@ -3,6 +3,7 @@ use crux_core::{macros::effect, render::RenderOperation}; +use crate::capabilities::download::DownloadOperation; use crate::capabilities::http::HttpOperation; use crate::capabilities::storage::StorageOperation; @@ -12,4 +13,5 @@ pub enum Effect { Render(RenderOperation), Storage(StorageOperation), Http(HttpOperation), + Download(DownloadOperation), } diff --git a/shared/src/feed_parser.rs b/shared/src/feed_parser.rs index 8ab70e3..80b9c9a 100644 --- a/shared/src/feed_parser.rs +++ b/shared/src/feed_parser.rs @@ -96,7 +96,6 @@ pub fn parse_feed(url: &str, body: Vec) -> Result<(Subscription, Vec, + // Download manager. Serial for MVP: at most one episode downloads at a time + // (`downloading` holds its id), the rest wait in `download_queue` (front = next + // up). Queue entries carry the enclosure URL so a download can start without its + // feed's episodes being loaded — this is what lets `Started` re-enqueue downloads + // interrupted by a previous quit (see `PendingDownloadsLoaded`). In-memory only: + // the queue itself is rebuilt from the DB at launch. + pub download_queue: Vec, + pub downloading: Option, + + // Live progress of the in-flight download (the one in `downloading`). Transient + // and never persisted: a partial download can't resume across a restart, so a + // stored percentage would only be stale. Cleared when a download ends or the + // next one starts. + pub active_download_progress: Option, + + // Why a download failed, keyed by episode id, so the shell's specific reason + // (disk full, HTTP status, filesystem error) can be shown beside Retry instead + // of a generic "failed". In-memory only (like progress) and cleared as soon as + // the episode leaves the Failed state; the persisted status is enough to know it + // failed across a restart. + pub download_errors: HashMap, + + // A transient, non-blocking notice for a download *operation* that failed without + // changing the episode's state — a persistence write that didn't commit, or a file + // that couldn't be removed. Tagged with the failing episode so the detail page can + // scope it to that episode. Shown as a banner on the details page (NOT through + // `detail_error`, which replaces the whole episode list). Cleared when the user + // starts another download action or switches feeds. + pub download_notice: Option, + // Active theme selection. Defaults (System / FollowSystem) reproduce the OS's // native appearance. Hard-coded for now — no UI changes it until the Settings // appearance section lands and drives `Event::SetTheme`. pub theme_id: ThemeId, pub theme_mode: ThemeMode, } + +/// An episode waiting to be downloaded. Carries the enclosure URL so the download +/// can be issued from the queue alone, without the episode being loaded in +/// `model.episodes`. +#[derive(Clone, Debug)] +pub struct QueuedDownload { + pub episode_id: String, + pub url: String, +} + +/// Byte progress of the currently-downloading episode. `total_bytes` is optional +/// because a server may omit `Content-Length`; when it's `None` the shell can't +/// know the size, so the UI shows an indeterminate indicator rather than a bar. +#[derive(Clone, Debug)] +pub struct DownloadProgress { + pub episode_id: String, + pub received_bytes: u64, + pub total_bytes: Option, +} diff --git a/shared/src/view_model.rs b/shared/src/view_model.rs index cb3f20d..ef9ccca 100644 --- a/shared/src/view_model.rs +++ b/shared/src/view_model.rs @@ -38,6 +38,20 @@ pub struct SubscriptionDetailView { pub sort_order: EpisodeSortOrder, pub loading: bool, pub error: Option, + /// A transient, non-blocking notice for a failed download *operation* (a + /// persistence write or file removal that didn't take). Shown as a banner that + /// leaves the episode list and its controls intact, unlike `error`. Tagged with + /// the episode it concerns so an episode's detail page shows it only when it's + /// about that episode, while the list shows it for any. + pub download_notice: Option, +} + +/// A failed-download-operation notice, paired with the episode it's about. Transient +/// and non-blocking (see `SubscriptionDetailView::download_notice`). +#[derive(Facet, Serialize, Deserialize, Clone)] +pub struct DownloadNotice { + pub episode_id: String, + pub message: String, } /// Read-only projection of an `Episode` for display. Dates and durations stay raw @@ -57,5 +71,14 @@ pub struct EpisodeSummary { pub playback_status: PlaybackStatus, pub playback_position_secs: Option, pub download_status: DownloadStatus, - pub download_progress: Option, + /// Live download progress, present only for the episode currently downloading. + /// `received`/`total` are byte counts; `total` is absent when the server didn't + /// report a size, in which case the shell shows an indeterminate indicator. + /// Transient and never persisted (see `Model::active_download_progress`). + pub download_received_bytes: Option, + pub download_total_bytes: Option, + /// Why the download failed, present only for a `Failed` episode. Lets the shell + /// show the specific reason (disk full, HTTP status, …) beside Retry rather than + /// a generic message. Transient (see `Model::download_errors`). + pub download_error: Option, }