diff --git a/src-tauri/src/db.rs b/src-tauri/src/db.rs index 028bb4d..9c2137a 100644 --- a/src-tauri/src/db.rs +++ b/src-tauri/src/db.rs @@ -55,6 +55,22 @@ CREATE TABLE IF NOT EXISTS downloads ( ); "#; +/// A channel row that is not a subscription and has no video on disk or on its +/// way there. Written once and used by both the count and the sweep, so the +/// confirmation can never disagree with what happens. +const ORPHAN_WHERE: &str = "c.subscribed = 0 AND NOT EXISTS ( + SELECT 1 FROM videos v JOIN downloads d ON d.video_id = v.id + WHERE v.channel_id = c.id AND d.state IN ('done','queued','running'))"; + +const ORPHAN_COUNT_SQL: &str = "SELECT COUNT(*) FROM channels c WHERE c.subscribed = 0 + AND NOT EXISTS (SELECT 1 FROM videos v JOIN downloads d ON d.video_id = v.id + WHERE v.channel_id = c.id AND d.state IN ('done','queued','running'))"; + +const ORPHAN_VIDEO_COUNT_SQL: &str = "SELECT COUNT(*) FROM videos v WHERE v.channel_id IN ( + SELECT c.id FROM channels c WHERE c.subscribed = 0 + AND NOT EXISTS (SELECT 1 FROM videos v2 JOIN downloads d ON d.video_id = v2.id + WHERE v2.channel_id = c.id AND d.state IN ('done','queued','running')))"; + fn now() -> i64 { std::time::SystemTime::now() .duration_since(std::time::UNIX_EPOCH) @@ -214,10 +230,13 @@ impl Db { } } + // Bare rows that the replace would sweep count as removals too: saying + // "0 channels" while quietly taking one is worse than taking it. + let (orphan_channels, orphan_videos) = self.orphan_counts()?; Ok(ImportPreview { incoming: incoming.len() as i64, - removed_channels, - removed_videos, + removed_channels: removed_channels + orphan_channels, + removed_videos: removed_videos + orphan_videos, removed_downloads, }) } @@ -306,6 +325,9 @@ impl Db { } } tx.commit().map_err(|e| e.to_string())?; + // A replace is the moment to notice that a bare row has outlived its + // video. + self.prune_orphan_channels()?; Ok(channels.len()) } @@ -356,6 +378,18 @@ impl Db { tx.commit().map_err(|e| e.to_string()) } + /// True when a channel row exists at all, subscription or not. + pub fn has_any_channel(&self, id: &str) -> Result { + self.conn + .query_row("SELECT 1 FROM channels WHERE id = ?1", params![id], |_| Ok(())) + .map(|_| true) + .or_else(|e| match e { + rusqlite::Error::QueryReturnedNoRows => Ok(false), + other => Err(other), + }) + .map_err(|e| e.to_string()) + } + /// True when a channel is already a subscription. A bare row saved for a /// one-off video does not count: subscribing to it is a real change. pub fn is_subscribed(&self, id: &str) -> Result { @@ -447,6 +481,49 @@ impl Db { /// Upserts video metadata only. Deliberately never touches the `downloads` /// table, so a refresh cannot lose the record of what has been downloaded. /// `thumb_path` is preserved via COALESCE for the same reason. + /// How many bare rows, and their videos, a replace would sweep. + fn orphan_counts(&self) -> Result<(i64, i64), String> { + let channels: i64 = self + .conn + .query_row(ORPHAN_COUNT_SQL, [], |r| r.get(0)) + .map_err(|e| e.to_string())?; + let videos: i64 = self + .conn + .query_row(ORPHAN_VIDEO_COUNT_SQL, [], |r| r.get(0)) + .map_err(|e| e.to_string())?; + Ok((channels, videos)) + } + + /// Drops channel rows that are not subscriptions and have nothing left to + /// hold on to. + /// + /// A bare row exists only to parent a video saved on its own from the menu + /// bar. Once that video is no longer on disk — or on its way there — the + /// row is residue, and it is confusing residue: the channel is not in the + /// sidebar, yet its video still sits in the feed after everything else has + /// been removed. + pub fn prune_orphan_channels(&mut self) -> Result { + let tx = self.conn.transaction().map_err(|e| e.to_string())?; + tx.execute_batch( + &format!( + "CREATE TEMP VIEW IF NOT EXISTS orphans AS + SELECT c.id FROM channels c WHERE {ORPHAN_WHERE}; + DELETE FROM downloads WHERE video_id IN ( + SELECT id FROM videos WHERE channel_id IN (SELECT id FROM orphans)); + DELETE FROM playback WHERE video_id IN ( + SELECT id FROM videos WHERE channel_id IN (SELECT id FROM orphans)); + DELETE FROM videos WHERE channel_id IN (SELECT id FROM orphans);" + ), + ) + .map_err(|e| e.to_string())?; + let n = tx + .execute("DELETE FROM channels WHERE id IN (SELECT id FROM orphans)", []) + .map_err(|e| e.to_string())?; + tx.execute_batch("DROP VIEW orphans;").map_err(|e| e.to_string())?; + tx.commit().map_err(|e| e.to_string())?; + Ok(n) + } + /// Records a channel without subscribing to it, for a video saved on its /// own. An existing subscription is left exactly as it is. pub fn ensure_channel(&self, c: &Channel) -> Result<(), String> { @@ -1190,20 +1267,27 @@ mod tests { assert!(!db.list_channels().unwrap().iter().any(|c| c.id == "UCX")); assert!(!db.channel_ids().unwrap().contains(&"UCX".to_string())); - // And importing a CSV, which replaces subscriptions, leaves it alone. + // Importing a CSV, which replaces subscriptions, leaves it alone while + // it still has a downloaded video to hold on to. + db.upsert_videos(&[Video { + id: "x1".into(), + channel_id: "UCX".into(), + title: "A one-off".into(), + description: String::new(), + published: 10, + thumb_url: String::new(), + views: 0, + is_short: false, + }]) + .unwrap(); + db.set_download_state("x1", DownloadState::Done, None).unwrap(); let incoming = vec![Channel { id: "UC1".into(), title: "Alpha".into(), url: "https://youtube.com/channel/UC1".into(), }]; db.replace_channels(&incoming).unwrap(); - assert_eq!( - db.conn - .query_row("SELECT COUNT(*) FROM channels WHERE id='UCX'", [], |r| r - .get::<_, i64>(0)) - .unwrap(), - 1 - ); + assert!(db.has_any_channel("UCX").unwrap()); // Subscribing to it later promotes the row rather than being refused // as a duplicate — saving a video is not subscribing. @@ -1212,6 +1296,92 @@ mod tests { assert!(db.list_channels().unwrap().iter().any(|c| c.id == "UCX")); } + #[test] + fn a_bare_row_with_nothing_downloaded_is_swept_by_a_replace() { + let mut db = seeded(); + let one_off = Channel { + id: "UCX".into(), + title: "Saved Once".into(), + url: "https://youtube.com/channel/UCX".into(), + }; + db.ensure_channel(&one_off).unwrap(); + db.upsert_videos(&[Video { + id: "x1".into(), + channel_id: "UCX".into(), + title: "A one-off".into(), + description: String::new(), + published: 10, + thumb_url: String::new(), + views: 0, + is_short: false, + }]) + .unwrap(); + + // Nothing downloaded: emptying the list takes it too, or its video + // would sit in the feed with no channel behind it. + db.replace_channels(&[]).unwrap(); + assert!(!db.has_any_channel("UCX").unwrap()); + assert!(db.list_feed(&FeedFilter::default()).unwrap().is_empty()); + } + + #[test] + fn the_preview_counts_what_the_sweep_will_take() { + let mut db = seeded(); + db.ensure_channel(&Channel { + id: "UCX".into(), + title: "Saved Once".into(), + url: "https://youtube.com/channel/UCX".into(), + }) + .unwrap(); + db.upsert_videos(&[Video { + id: "x1".into(), + channel_id: "UCX".into(), + title: "A one-off".into(), + description: String::new(), + published: 10, + thumb_url: String::new(), + views: 0, + is_short: false, + }]) + .unwrap(); + + // Emptying the list takes both subscriptions and this bare row, and + // the confirmation has to say so before it happens. + let p = db.preview_replace(&[]).unwrap(); + assert_eq!(p.removed_channels, 3, "two subscriptions and the bare row"); + assert_eq!(p.removed_videos, 4, "their videos and the one-off"); + + db.replace_channels(&[]).unwrap(); + assert!(!db.has_any_channel("UCX").unwrap()); + } + + #[test] + fn a_bare_row_whose_video_is_downloaded_survives() { + let mut db = seeded(); + let one_off = Channel { + id: "UCX".into(), + title: "Saved Once".into(), + url: "https://youtube.com/channel/UCX".into(), + }; + db.ensure_channel(&one_off).unwrap(); + db.upsert_videos(&[Video { + id: "x1".into(), + channel_id: "UCX".into(), + title: "A one-off".into(), + description: String::new(), + published: 10, + thumb_url: String::new(), + views: 0, + is_short: false, + }]) + .unwrap(); + db.set_download_state("x1", DownloadState::Done, None).unwrap(); + + // Deliberately saved and still on disk: not this function's to delete. + db.replace_channels(&[]).unwrap(); + assert!(db.has_any_channel("UCX").unwrap()); + } + #[test] fn deleting_a_subscription_takes_its_videos_with_it() { let mut db = seeded(); diff --git a/src-tauri/src/tray.rs b/src-tauri/src/tray.rs index 226b6a6..debc946 100644 --- a/src-tauri/src/tray.rs +++ b/src-tauri/src/tray.rs @@ -1,8 +1,9 @@ //! The menu bar item. //! //! Its point is to act on what a browser is showing without leaving it: while a -//! video plays in Safari or Chrome, one click saves that video or subscribes to -//! its channel, and playback carries on undisturbed. +//! video plays in Safari or Chrome, one click saves it — and one reads the +//! whole subscription list off youtube.com/feed/channels — with playback +//! carrying on undisturbed. //! //! Reading the browser's address needs Apple events, which macOS gates behind a //! permission the user grants once. Without it the menu still opens and says @@ -199,6 +200,17 @@ async fn browser_on_subscriptions(app: &AppHandle) -> Result { pub async fn scrape_subscriptions(app: &AppHandle) -> Result { let browser = browser_on_subscriptions(app).await?; + // Bring it to the front first. Chromium browsers discard a background + // tab's DOM to save memory, so the page keeps reporting its address while + // having nothing on it — which reads exactly like an empty subscription + // list rather than like a sleeping tab. + let _ = tokio::process::Command::new("/usr/bin/osascript") + .arg("-e") + .arg(format!("tell application {} to activate", quote(&browser))) + .output() + .await; + tokio::time::sleep(std::time::Duration::from_millis(1200)).await; + let mut last = 0usize; let mut settled = 0; for round in 0..40 {