diff --git a/PROGRESS.md b/PROGRESS.md index 5cc634a..6e71ea6 100644 --- a/PROGRESS.md +++ b/PROGRESS.md @@ -56,6 +56,38 @@ and until now nothing set them. --- +## 2026-09-11 — Steps B and C: what is yours, what is everyone's + +Read, starred and playback position moved out of `entries` into `entry_state (user_id, feed_id, +guid, ...)`; subscriptions became rows in `subscriptions (user_id, feed_id, ...)` carrying **your** +keywords, auto-download, explicit and per-scan limit. The feed list, unread counts, filters and +mark-all-read are all per person now. On first start the existing library is adopted by the admin: +2438 read/starred items and all 86 feeds, so nothing was lost. + +The split follows from the file being shared: + +* **Yours**: read state, starred, position, keywords, auto-download, explicit, per-scan limit, + and which feeds you see at all. +* **Everyone's**: the feed URL, its download folder, and when it is scanned -- there is one copy of + a file however many people subscribe, so those describe the file, not a preference. Admin-only, + refused with a 403 for anyone else rather than merely hidden. + +Scanning merges the subscribers' wants, because one fetch and one file serve them all: an item is +downloaded if **anyone** wants it (any one person's keyword set matching is enough, and one person +taking everything removes the filter), auto-download is on if anyone has it on, and the per-scan cap +is the largest anyone asked for. `merge_policy` is a pure function with a test covering each of +those. Subscribing to a feed someone already has costs no second fetch and no second copy on disk; +unsubscribing takes it off your list alone, and only when the last subscriber leaves does the feed +stop being scanned. + +**A test-harness bug worth naming**: Playwright imports the config in every worker, so the fixture's +`prepare()` ran again mid-run and deleted the data directory out from under the daemon. The daemon +kept serving from the unlinked inode while the CLI and any query opened a fresh empty database at +the same path -- which looked exactly like sign-in being broken. Only the launching process wipes +now (a worker has `TEST_WORKER_INDEX`). + +--- + ## 2026-09-10 — Scanning is the operator's decision The per-feed **Check schedule** picker is gone from feed settings, and the global Settings page is @@ -123,10 +155,10 @@ on disk. `enclosures.url` is already globally UNIQUE, so the file half is nearly optionally creates a user -- honoured only from a `trusted_proxies` address, so a LAN client cannot simply assert it. The existing shared token keeps working and resolves to the admin, so the healthcheck and any scripts survive. Login page for direct access. -- [ ] **B. Per-user read state.** `entry_state(user_id, feed_id, guid, read, flagged, position)`; +- [x] **B. Per-user read state.** `entry_state(user_id, feed_id, guid, read, flagged, position)`; the current columns on `entries` migrate into the first user's rows. Unread counts, filters and playback position all become per user. -- [ ] **C. Per-user subscriptions.** `subscriptions(user_id, feed_id)`. config.toml stays the feed +- [x] **C. Per-user subscriptions.** `subscriptions(user_id, feed_id)`. config.toml stays the feed catalogue; the UI lists only what you subscribe to. Adding a feed someone else already has costs nothing. A feed nobody subscribes to stops being scanned but keeps its files. - [ ] **D. One file, many users.** Auto-download when *any* subscriber wants it; retention never diff --git a/src/db.rs b/src/db.rs index 7428fd3..45f9f73 100644 --- a/src/db.rs +++ b/src/db.rs @@ -80,6 +80,31 @@ CREATE TABLE IF NOT EXISTS users ( created INTEGER NOT NULL ); +-- What one person wants from a feed. The feed, its items and its files are shared; this +-- is the part that is not. NULL in a column means: follow the feed's own setting. +CREATE TABLE IF NOT EXISTS subscriptions ( + user_id INTEGER NOT NULL REFERENCES users(id) ON DELETE CASCADE, + feed_id TEXT NOT NULL, + keywords TEXT, + auto_download INTEGER, + allow_explicit INTEGER, + max_new_per_check INTEGER, + created INTEGER NOT NULL, + PRIMARY KEY (user_id, feed_id) +); + +-- Read, starred and how far in. One row per person per item, created on first touch; +-- an item nobody has touched has no row at all, which is what unread means. +CREATE TABLE IF NOT EXISTS entry_state ( + user_id INTEGER NOT NULL REFERENCES users(id) ON DELETE CASCADE, + feed_id TEXT NOT NULL, + guid TEXT NOT NULL, + read INTEGER NOT NULL DEFAULT 0, + flagged INTEGER NOT NULL DEFAULT 0, + position INTEGER NOT NULL DEFAULT 0, + PRIMARY KEY (user_id, feed_id, guid) +); + CREATE TABLE IF NOT EXISTS sessions ( token TEXT PRIMARY KEY, user_id INTEGER NOT NULL REFERENCES users(id) ON DELETE CASCADE, @@ -88,6 +113,16 @@ CREATE TABLE IF NOT EXISTS sessions ( ); "; +/// One person's wants for one feed. `None` in a field means the feed's own setting stands. +#[derive(Debug, Clone, Default)] +pub struct Sub { + pub feed_id: String, + pub keywords: Option>, + pub auto_download: Option, + pub allow_explicit: Option, + pub max_new_per_check: Option, +} + /// Someone who can sign in. `pass_hash` is None for an account that only ever arrives /// through the proxy. #[derive(Debug, Clone)] @@ -506,11 +541,14 @@ impl Db { } impl Db { - pub fn unread_count(&self, feed_id: &str) -> Result { + pub fn unread_count(&self, user_id: i64, feed_id: &str) -> Result { let conn = self.conn.lock().unwrap(); Ok(conn.query_row( - "SELECT count(*) FROM entries WHERE feed_id = ?1 AND read = 0", - [feed_id], + "SELECT count(*) FROM entries e + LEFT JOIN entry_state s + ON s.user_id = ?2 AND s.feed_id = e.feed_id AND s.guid = e.guid + WHERE e.feed_id = ?1 AND coalesce(s.read, 0) = 0", + rusqlite::params![feed_id, user_id], |r| r.get(0), )?) } @@ -574,8 +612,8 @@ impl Filter { fn sql(self) -> &'static str { match self { Self::All => "1=1", - Self::Unread => "e.read = 0", - Self::Flagged => "e.flagged = 1", + Self::Unread => "coalesce(s.read, 0) = 0", + Self::Flagged => "coalesce(s.flagged, 0) = 1", Self::Downloaded => { "EXISTS (SELECT 1 FROM enclosures x WHERE x.feed_id = e.feed_id AND x.guid = e.guid AND x.path IS NOT NULL)" @@ -602,6 +640,7 @@ impl Db { /// `search` matches title and description, case-insensitively. pub fn entries( &self, + user_id: i64, feed_id: &str, filter: Filter, search: Option<&str>, @@ -613,9 +652,12 @@ impl Db { .map(|q| format!("%{}%", q.trim().to_lowercase())) .unwrap_or_default(); let sql = format!( - "SELECT e.guid, e.feed_id, e.title, e.link, e.published, e.description, e.read, - e.flagged, e.image, e.duration, e.episode, e.season, e.position + "SELECT e.guid, e.feed_id, e.title, e.link, e.published, e.description, + coalesce(s.read, 0), coalesce(s.flagged, 0), e.image, e.duration, + e.episode, e.season, coalesce(s.position, 0) FROM entries e + LEFT JOIN entry_state s + ON s.user_id = ?5 AND s.feed_id = e.feed_id AND s.guid = e.guid WHERE e.feed_id = ?1 AND {} AND {SEARCH} ORDER BY coalesce(e.published, e.first_seen) DESC, e.rowid DESC LIMIT ?4 OFFSET ?3", @@ -641,7 +683,7 @@ impl Db { }) }; let mut rows: Vec = stmt - .query_map(rusqlite::params![feed_id, like, offset, limit], map)? + .query_map(rusqlite::params![feed_id, like, offset, limit, user_id], map)? .collect::>>()?; if rows.is_empty() { @@ -685,29 +727,210 @@ impl Db { } /// How many entries match, so the UI knows whether there is another page. - pub fn count_entries(&self, feed_id: &str, filter: Filter, search: Option<&str>) -> Result { + pub fn count_entries( + &self, + user_id: i64, + feed_id: &str, + filter: Filter, + search: Option<&str>, + ) -> Result { let conn = self.conn.lock().unwrap(); let like = search .map(|q| format!("%{}%", q.trim().to_lowercase())) .unwrap_or_default(); let sql = format!( - "SELECT count(*) FROM entries e WHERE e.feed_id = ?1 AND {} AND {SEARCH}", + "SELECT count(*) FROM entries e + LEFT JOIN entry_state s + ON s.user_id = ?3 AND s.feed_id = e.feed_id AND s.guid = e.guid + WHERE e.feed_id = ?1 AND {} AND {SEARCH}", filter.sql() ); - Ok(conn.query_row(&sql, rusqlite::params![feed_id, like], |r| r.get(0))?) + Ok(conn.query_row(&sql, rusqlite::params![feed_id, like, user_id], |r| r.get(0))?) } - /// Where playback got to, so it resumes there next time. - pub fn set_position(&self, feed_id: &str, guid: &str, secs: i64) -> Result<()> { + /// Where playback got to, so it resumes there next time -- for this listener only. + pub fn set_position(&self, user_id: i64, feed_id: &str, guid: &str, secs: i64) -> Result<()> { let conn = self.conn.lock().unwrap(); conn.execute( - "UPDATE entries SET position = ?3 WHERE feed_id = ?1 AND guid = ?2", - rusqlite::params![feed_id, guid, secs.max(0)], + "INSERT INTO entry_state (user_id, feed_id, guid, position) VALUES (?1, ?2, ?3, ?4) + ON CONFLICT(user_id, feed_id, guid) DO UPDATE SET position = excluded.position", + rusqlite::params![user_id, feed_id, guid, secs.max(0)], )?; Ok(()) } /// Marks every entry in a feed read, for the "mark all read" button. + /// Moves a single-user library onto an account: everything read, starred or part-played + /// becomes that person's, and they subscribe to every feed already in the catalogue. + /// Runs once -- the moment there is a first account and no subscriptions yet. + pub fn adopt_existing_library(&self, user_id: i64, catalogue: &[String]) -> Result { + let conn = self.conn.lock().unwrap(); + let already: i64 = + conn.query_row("SELECT count(*) FROM subscriptions", [], |r| r.get(0))?; + if already > 0 { + return Ok(0); + } + let moved = conn.execute( + "INSERT INTO entry_state (user_id, feed_id, guid, read, flagged, position) + SELECT ?1, feed_id, guid, read, flagged, position FROM entries + WHERE read = 1 OR flagged = 1 OR position > 0 + ON CONFLICT(user_id, feed_id, guid) DO NOTHING", + [user_id], + )?; + for id in catalogue { + conn.execute( + "INSERT OR IGNORE INTO subscriptions (user_id, feed_id, created) VALUES (?1, ?2, ?3)", + params![user_id, id, now()], + )?; + } + // Feeds that exist only in the database (OPML children) count too. + conn.execute( + "INSERT OR IGNORE INTO subscriptions (user_id, feed_id, created) + SELECT ?1, id, ?2 FROM feeds", + params![user_id, now()], + )?; + Ok(moved) + } + + // ---- subscriptions ---- + + /// What this person wants from a feed. Absent means they do not subscribe at all. + pub fn subscription(&self, user_id: i64, feed_id: &str) -> Result> { + let conn = self.conn.lock().unwrap(); + let mut stmt = conn.prepare( + "SELECT keywords, auto_download, allow_explicit, max_new_per_check + FROM subscriptions WHERE user_id = ?1 AND feed_id = ?2", + )?; + let mut rows = stmt.query(params![user_id, feed_id])?; + Ok(match rows.next()? { + Some(r) => Some(Sub { + feed_id: feed_id.to_string(), + keywords: r + .get::<_, Option>(0)? + .and_then(|j| serde_json::from_str(&j).ok()), + auto_download: r.get::<_, Option>(1)?.map(|v| v != 0), + allow_explicit: r.get::<_, Option>(2)?.map(|v| v != 0), + max_new_per_check: r.get(3)?, + }), + None => None, + }) + } + + /// Every feed this person subscribes to, with their settings. + pub fn subscriptions_for(&self, user_id: i64) -> Result> { + let conn = self.conn.lock().unwrap(); + let mut stmt = conn.prepare( + "SELECT feed_id, keywords, auto_download, allow_explicit, max_new_per_check + FROM subscriptions WHERE user_id = ?1", + )?; + let out = stmt + .query_map([user_id], |r| { + Ok(Sub { + feed_id: r.get(0)?, + keywords: r + .get::<_, Option>(1)? + .and_then(|j| serde_json::from_str(&j).ok()), + auto_download: r.get::<_, Option>(2)?.map(|v| v != 0), + allow_explicit: r.get::<_, Option>(3)?.map(|v| v != 0), + max_new_per_check: r.get(4)?, + }) + })? + .collect::>>()?; + Ok(out) + } + + /// Everyone's settings for one feed. The scanner merges these into what it fetches + /// and downloads, since one file serves the lot. + pub fn subscribers(&self, feed_id: &str) -> Result> { + let conn = self.conn.lock().unwrap(); + let mut stmt = conn.prepare( + "SELECT keywords, auto_download, allow_explicit, max_new_per_check + FROM subscriptions WHERE feed_id = ?1", + )?; + let out = stmt + .query_map([feed_id], |r| { + Ok(Sub { + feed_id: feed_id.to_string(), + keywords: r + .get::<_, Option>(0)? + .and_then(|j| serde_json::from_str(&j).ok()), + auto_download: r.get::<_, Option>(1)?.map(|v| v != 0), + allow_explicit: r.get::<_, Option>(2)?.map(|v| v != 0), + max_new_per_check: r.get(3)?, + }) + })? + .collect::>>()?; + Ok(out) + } + + pub fn subscribe(&self, user_id: i64, feed_id: &str) -> Result<()> { + let conn = self.conn.lock().unwrap(); + conn.execute( + "INSERT OR IGNORE INTO subscriptions (user_id, feed_id, created) VALUES (?1, ?2, ?3)", + params![user_id, feed_id, now()], + )?; + Ok(()) + } + + pub fn unsubscribe(&self, user_id: i64, feed_id: &str) -> Result<()> { + let conn = self.conn.lock().unwrap(); + conn.execute( + "DELETE FROM subscriptions WHERE user_id = ?1 AND feed_id = ?2", + params![user_id, feed_id], + )?; + Ok(()) + } + + /// How many people want this feed. Nobody means it stops being scanned. + pub fn subscriber_count(&self, feed_id: &str) -> Result { + let conn = self.conn.lock().unwrap(); + Ok(conn.query_row( + "SELECT count(*) FROM subscriptions WHERE feed_id = ?1", + [feed_id], + |r| r.get(0), + )?) + } + + /// Overwrites one person's settings for a feed. A None field means: follow the feed. + pub fn set_subscription(&self, user_id: i64, sub: &Sub) -> Result<()> { + let conn = self.conn.lock().unwrap(); + let kw = sub + .keywords + .as_ref() + .map(|k| serde_json::to_string(k)) + .transpose()?; + conn.execute( + "INSERT INTO subscriptions + (user_id, feed_id, keywords, auto_download, allow_explicit, max_new_per_check, created) + VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7) + ON CONFLICT(user_id, feed_id) DO UPDATE SET + keywords = excluded.keywords, + auto_download = excluded.auto_download, + allow_explicit = excluded.allow_explicit, + max_new_per_check = excluded.max_new_per_check", + params![ + user_id, + sub.feed_id, + kw, + sub.auto_download.map(|v| v as i64), + sub.allow_explicit.map(|v| v as i64), + sub.max_new_per_check, + now() + ], + )?; + Ok(()) + } + + /// Feeds with at least one subscriber. What the scanner walks. + pub fn subscribed_feed_ids(&self) -> Result> { + let conn = self.conn.lock().unwrap(); + let mut stmt = conn.prepare("SELECT DISTINCT feed_id FROM subscriptions")?; + let out = stmt + .query_map([], |r| r.get::<_, String>(0))? + .collect::>>()?; + Ok(out) + } + // ---- users and sessions ---- pub fn create_user(&self, name: &str, pass_hash: Option<&str>, admin: bool) -> Result { @@ -827,11 +1050,19 @@ impl Db { /// Marks every entry of the given feeds read. Takes a list because an OPML subscription /// holds no entries itself -- marking it read means the feeds inside it. - pub fn mark_all_read(&self, feed_ids: &[String]) -> Result { + pub fn mark_all_read(&self, user_id: i64, feed_ids: &[String]) -> Result { let conn = self.conn.lock().unwrap(); let mut n = 0; for id in feed_ids { - n += conn.execute("UPDATE entries SET read = 1 WHERE feed_id = ?1 AND read = 0", [id])?; + n += conn.execute( + "INSERT INTO entry_state (user_id, feed_id, guid, read) + SELECT ?1, e.feed_id, e.guid, 1 FROM entries e + LEFT JOIN entry_state s + ON s.user_id = ?1 AND s.feed_id = e.feed_id AND s.guid = e.guid + WHERE e.feed_id = ?2 AND coalesce(s.read, 0) = 0 + ON CONFLICT(user_id, feed_id, guid) DO UPDATE SET read = 1", + rusqlite::params![user_id, id], + )?; } Ok(n) } @@ -876,14 +1107,28 @@ impl Db { .optional()?) } - /// `read` and `flagged` finally get a writer: retention orders by them. - pub fn set_entry_flag(&self, feed_id: &str, guid: &str, field: EntryFlag, on: bool) -> Result<()> { + /// Read and starred, per person. The row is created on first touch. + pub fn set_entry_flag( + &self, + user_id: i64, + feed_id: &str, + guid: &str, + field: EntryFlag, + on: bool, + ) -> Result<()> { let conn = self.conn.lock().unwrap(); - let sql = match field { - EntryFlag::Read => "UPDATE entries SET read = ?3 WHERE feed_id = ?1 AND guid = ?2", - EntryFlag::Flagged => "UPDATE entries SET flagged = ?3 WHERE feed_id = ?1 AND guid = ?2", + let col = match field { + EntryFlag::Read => "read", + EntryFlag::Flagged => "flagged", }; - conn.execute(sql, rusqlite::params![feed_id, guid, on as i64])?; + conn.execute( + &format!( + "INSERT INTO entry_state (user_id, feed_id, guid, {col}) + VALUES (?1, ?2, ?3, ?4) + ON CONFLICT(user_id, feed_id, guid) DO UPDATE SET {col} = excluded.{col}" + ), + rusqlite::params![user_id, feed_id, guid, on as i64], + )?; Ok(()) } @@ -1021,6 +1266,39 @@ pub fn now() -> i64 { mod tests { use super::*; + #[test] + fn read_state_belongs_to_one_person() { + let db = Db::memory().unwrap(); + db.exec_for_test( + "INSERT INTO users (id, name, is_admin, created) VALUES (1,'ray',1,0),(2,'sam',0,0); + INSERT INTO entries (feed_id, guid, title, first_seen) VALUES + ('f','a','One',100),('f','b','Two',200);", + ) + .unwrap(); + + assert_eq!(db.unread_count(1, "f").unwrap(), 2); + assert_eq!(db.unread_count(2, "f").unwrap(), 2); + + db.set_entry_flag(1, "f", "a", EntryFlag::Read, true).unwrap(); + assert_eq!(db.unread_count(1, "f").unwrap(), 1, "ray read one of them"); + assert_eq!(db.unread_count(2, "f").unwrap(), 2, "sam has read nothing"); + + // Starring and position are just as private. + db.set_entry_flag(1, "f", "b", EntryFlag::Flagged, true).unwrap(); + db.set_position(2, "f", "b", 42).unwrap(); + let ray = db.entries(1, "f", Filter::All, None, 0, 50).unwrap(); + let sam = db.entries(2, "f", Filter::All, None, 0, 50).unwrap(); + let ray_b = ray.iter().find(|e| e.guid == "b").unwrap(); + let sam_b = sam.iter().find(|e| e.guid == "b").unwrap(); + assert!(ray_b.flagged && ray_b.position == 0); + assert!(!sam_b.flagged && sam_b.position == 42); + + // Marking a whole feed read is likewise one person's business. + assert_eq!(db.mark_all_read(2, &["f".to_string()]).unwrap(), 2); + assert_eq!(db.unread_count(2, "f").unwrap(), 0); + assert_eq!(db.unread_count(1, "f").unwrap(), 1); + } + #[test] fn schema_is_idempotent_and_summary_handles_unknown_feeds() { let db = Db::memory().unwrap(); @@ -1040,32 +1318,37 @@ mod tests { // so plain filtering failed with "Wrong number of parameters passed to query". let db = Db::memory().unwrap(); db.exec_for_test( - "INSERT INTO entries (feed_id, guid, title, description, first_seen, read, flagged) VALUES - ('f','a','Alpha dive','notes one',100,0,0), - ('f','b','Beta', 'notes two',200,1,0), - ('f','c','Gamma dive','notes three',300,1,1); + "INSERT INTO entries (feed_id, guid, title, description, first_seen) VALUES + ('f','a','Alpha dive','notes one',100), + ('f','b','Beta', 'notes two',200), + ('f','c','Gamma dive','notes three',300); INSERT INTO enclosures (id, feed_id, guid, url, path, state) VALUES - (1,'f','b','u1','/tmp/b','done');", + (1,'f','b','u1','/tmp/b','done'); + -- Read and starred belong to a person now, so say which one. + INSERT INTO users (id, name, is_admin, created) VALUES (7,'reader',1,0); + INSERT INTO entry_state (user_id, feed_id, guid, read, flagged) VALUES + (7,'f','b',1,0), + (7,'f','c',1,1);", ) .unwrap(); for f in [Filter::All, Filter::Unread, Filter::Downloaded, Filter::Flagged] { // Both paths must run without erroring, and agree with each other. - let rows = db.entries("f", f, None, 0, 50).unwrap(); - let n = db.count_entries("f", f, None).unwrap(); + let rows = db.entries(7, "f", f, None, 0, 50).unwrap(); + let n = db.count_entries(7, "f", f, None).unwrap(); assert_eq!(rows.len() as i64, n, "{f:?} count disagrees with the page"); - let rows = db.entries("f", f, Some("dive"), 0, 50).unwrap(); - let n = db.count_entries("f", f, Some("dive")).unwrap(); + let rows = db.entries(7, "f", f, Some("dive"), 0, 50).unwrap(); + let n = db.count_entries(7, "f", f, Some("dive")).unwrap(); assert_eq!(rows.len() as i64, n, "{f:?} with search disagrees"); } - assert_eq!(db.count_entries("f", Filter::All, None).unwrap(), 3); - assert_eq!(db.count_entries("f", Filter::Unread, None).unwrap(), 1); - assert_eq!(db.count_entries("f", Filter::Downloaded, None).unwrap(), 1); - assert_eq!(db.count_entries("f", Filter::Flagged, None).unwrap(), 1); - assert_eq!(db.count_entries("f", Filter::All, Some("dive")).unwrap(), 2); - assert_eq!(db.count_entries("f", Filter::All, Some("NOTES two")).unwrap(), 1, + assert_eq!(db.count_entries(7, "f", Filter::All, None).unwrap(), 3); + assert_eq!(db.count_entries(7, "f", Filter::Unread, None).unwrap(), 1); + assert_eq!(db.count_entries(7, "f", Filter::Downloaded, None).unwrap(), 1); + assert_eq!(db.count_entries(7, "f", Filter::Flagged, None).unwrap(), 1); + assert_eq!(db.count_entries(7, "f", Filter::All, Some("dive")).unwrap(), 2); + assert_eq!(db.count_entries(7, "f", Filter::All, Some("NOTES two")).unwrap(), 1, "search is case-insensitive and covers the description"); } diff --git a/src/main.rs b/src/main.rs index 7cd5cc2..580ae9f 100644 --- a/src/main.rs +++ b/src/main.rs @@ -344,6 +344,16 @@ async fn daemon( ); } + // A library that predates accounts belongs to whoever was using it: the admin. + if let Some(admin) = ctx.db.users()?.into_iter().find(|u| u.is_admin) { + let catalogue: Vec = ctx.cfg().feeds.keys().cloned().collect(); + match ctx.db.adopt_existing_library(admin.id, &catalogue) { + Ok(0) => {} + Ok(n) => tracing::info!(user = %admin.name, entries = n, "adopted the existing library"), + Err(e) => tracing::error!(error = %e, "could not adopt the existing library"), + } + } + match migrate_opml_children(&ctx) { Ok(n) if n > 0 => tracing::info!(count = n, "moved OPML feeds out of config.toml into the database"), Ok(_) => {} @@ -957,6 +967,7 @@ async fn scan_one( parsed.image.as_deref(), )?; + let policy = policy_for(ctx, id, feed_cfg)?; let mut scan = Scan::default(); for entry in &parsed.entries { if ctx.db.record_entry(id, entry)? { @@ -968,18 +979,14 @@ async fn scan_one( } // Filters run once, at discovery, and are recorded in `state`. The download // queue below is then just "everything still pending". - if let Some(reason) = reject(&ctx.cfg(), feed_cfg, entry, enc) { + if let Some(reason) = reject(&ctx.cfg(), feed_cfg, &policy, entry, enc) { ctx.db.mark_enclosure(&enc.url, "skipped", Some(reason))?; } } } - // An unset per-feed cap follows the global one; 0 there means unlimited. - let budget = feed_cfg.max_new_per_check.unwrap_or_else(|| { - let g = ctx.cfg().general.max_new_per_check; - if g == 0 { usize::MAX } else { g } - }); - if feed_cfg.auto_download && budget > 0 { + let budget = policy.budget; + if policy.auto_download && budget > 0 { let cfg = ctx.cfg(); let folder = download::folder_for(&cfg, id, feed_cfg, parsed.title.as_deref()); let dest_dir = cfg.general.download_dir.join(&folder); @@ -1099,6 +1106,23 @@ async fn sync_opml( added.push(id); } + // Whoever subscribes to the OPML subscribes to what it lists: that is what taking a + // subscription means. Their own feeds are untouched. + for id in ctx + .db + .managed_feeds()? + .iter() + .filter(|m| m.group_id == parent_id) + .map(|m| m.id.clone()) + .chain(std::iter::once(parent_id.to_string())) + { + for user in ctx.db.users()? { + if ctx.db.subscription(user.id, parent_id)?.is_some() { + ctx.db.subscribe(user.id, &id)?; + } + } + } + // Anything in this group the OPML no longer lists. let mut removed = 0; let mut kept = 0; @@ -1125,11 +1149,12 @@ async fn sync_opml( fn reject( cfg: &config::Config, feed_cfg: &config::Feed, + policy: &Policy, entry: &feed::Entry, enc: &feed::Enclosure, ) -> Option<&'static str> { let url = enc.url.as_str(); - if !feed_cfg.auto_download { + if !policy.auto_download { return Some("auto_download is off"); } // Blog feeds put the article's header image in an ; without this a text @@ -1141,7 +1166,7 @@ fn reject( if !config::wanted_media(enc.mime.as_deref(), wanted) { return Some("not audio or video"); } - if entry.explicit && !feed_cfg.allow_explicit { + if entry.explicit && !policy.allow_explicit { return Some("explicit"); } let categories = entry.categories.join(" "); @@ -1151,12 +1176,79 @@ fn reject( entry.description.as_deref().unwrap_or(""), categories.as_str(), ]; - if !download::matches_keywords(&feed_cfg.keywords, &haystacks) { + // One file serves everyone subscribed, so an item is wanted if it is wanted by + // anyone: any one person's keyword set matching is enough. + let wanted_by_someone = policy.keyword_sets.is_empty() + || policy + .keyword_sets + .iter() + .any(|set| download::matches_keywords(set, &haystacks)); + if !wanted_by_someone { return Some("no keyword match"); } None } +/// What the scanner should do for a feed, merged across everyone subscribed to it. The +/// feed is fetched once and its files are downloaded once, so the merge is a union: if +/// one person wants a thing, it is fetched, and everyone else simply sees it listed. +/// +/// With no subscribers at all -- a hand-written config entry nobody has claimed yet -- +/// the feed's own settings stand, which is how a single-user install behaves. +pub struct Policy { + pub auto_download: bool, + pub allow_explicit: bool, + /// Empty means take everything. Otherwise one set per subscriber who filters. + pub keyword_sets: Vec>, + pub budget: usize, +} + +fn policy_for(ctx: &Ctx, id: &str, feed_cfg: &config::Feed) -> Result { + let global = ctx.cfg().general.max_new_per_check; + Ok(merge_policy(&ctx.db.subscribers(id)?, feed_cfg, global)) +} + +fn merge_policy(subs: &[db::Sub], feed_cfg: &config::Feed, global: usize) -> Policy { + let cap = |n: Option| n.unwrap_or(if global == 0 { usize::MAX } else { global }); + + if subs.is_empty() { + return Policy { + auto_download: feed_cfg.auto_download, + allow_explicit: feed_cfg.allow_explicit, + keyword_sets: if feed_cfg.keywords.is_empty() { + vec![] + } else { + vec![feed_cfg.keywords.clone()] + }, + budget: cap(feed_cfg.max_new_per_check), + }; + } + + let mut policy = Policy { + auto_download: false, + allow_explicit: false, + keyword_sets: vec![], + budget: 0, + }; + for sub in subs { + if !sub.auto_download.unwrap_or(feed_cfg.auto_download) { + continue; // Not fetching for this person, so their wants add nothing. + } + policy.auto_download = true; + policy.allow_explicit |= sub.allow_explicit.unwrap_or(feed_cfg.allow_explicit); + policy.budget = policy + .budget + .max(cap(sub.max_new_per_check.map(|n| n as usize).or(feed_cfg.max_new_per_check))); + let kw = sub.keywords.clone().unwrap_or_else(|| feed_cfg.keywords.clone()); + if kw.is_empty() { + // Somebody takes everything, so no filter can apply to the shared copy. + return Policy { keyword_sets: vec![], ..policy }; + } + policy.keyword_sets.push(kw); + } + policy +} + async fn fetch_one( ctx: &Arc, feed_id: &str, @@ -1352,3 +1444,79 @@ fn duration(secs: u64) -> String { s => format!("{}d", s / 86_400), } } + +#[cfg(test)] +mod tests { + use super::*; + + fn feed() -> config::Feed { + // Whatever `ipx add` would write, which is the shape every code path sees. + let mut cfg = config::Config::default(); + let f = add_one_cfg(&mut cfg, "http://x/f.xml", None, vec![]); + f + } + + /// The feed entry `add` builds, without the network round trip it does for a title. + fn add_one_cfg( + _cfg: &mut config::Config, + url: &str, + folder: Option, + keywords: Vec, + ) -> config::Feed { + config::Feed { + url: url.into(), + folder, + keywords, + allow_explicit: false, + auto_download: true, + group: None, + media_types: None, + schedule: None, + max_new_per_check: None, + username: None, + password: None, + password_env: None, + } + } + + fn sub(kw: Option<&[&str]>, auto: Option, max: Option) -> db::Sub { + db::Sub { + feed_id: "f".into(), + keywords: kw.map(|k| k.iter().map(|s| s.to_string()).collect()), + auto_download: auto, + allow_explicit: None, + max_new_per_check: max, + } + } + + #[test] + fn a_shared_feed_is_fetched_for_whoever_wants_the_most() { + // Nobody subscribed: the feed's own settings stand, as in a single-user install. + let p = merge_policy(&[], &feed(), 3); + assert!(p.auto_download); + assert_eq!(p.budget, 3); + assert!(p.keyword_sets.is_empty()); + + // Two filters: an item wanted by either of them is fetched, since one file serves + // both. The larger per-scan cap wins for the same reason. + let p = merge_policy( + &[sub(Some(&["rust"]), None, Some(2)), sub(Some(&["sqlite"]), None, Some(9))], + &feed(), + 3, + ); + assert_eq!(p.keyword_sets.len(), 2); + assert_eq!(p.budget, 9); + + // One person taking everything removes the filter for the shared copy. + let p = merge_policy(&[sub(Some(&["rust"]), None, None), sub(Some(&[]), None, None)], &feed(), 3); + assert!(p.keyword_sets.is_empty()); + + // Everyone has auto-download off: nothing is fetched automatically. + let p = merge_policy(&[sub(None, Some(false), None), sub(None, Some(false), None)], &feed(), 3); + assert!(!p.auto_download); + + // One of them wants it, so it is fetched. + let p = merge_policy(&[sub(None, Some(false), None), sub(None, Some(true), None)], &feed(), 3); + assert!(p.auto_download); + } +} diff --git a/src/web.rs b/src/web.rs index 6d2fdb2..6e1c88b 100644 --- a/src/web.rs +++ b/src/web.rs @@ -348,13 +348,25 @@ struct FeedRow { unread: i64, } -async fn feeds(State(state): State) -> Result>, ApiError> { +async fn feeds( + State(state): State, + user: crate::db::User, +) -> Result>, ApiError> { let cfg = state.ctx.cfg(); - // Config entries plus the feeds derived from OPML subscriptions. + // Config entries plus the feeds derived from OPML subscriptions -- the catalogue. + // What comes back is only the part of it this person subscribes to. let subs = crate::subscriptions(&state.ctx)?; - let mut out = Vec::with_capacity(subs.len()); + let mine: std::collections::HashMap = state + .ctx + .db + .subscriptions_for(user.id)? + .into_iter() + .map(|s| (s.feed_id.clone(), s)) + .collect(); + let mut out = Vec::with_capacity(mine.len()); for sub in &subs { let (id, feed) = (&sub.id, &sub.cfg); + let Some(mine) = mine.get(id) else { continue }; let s = state.ctx.db.feed_summary(id)?; let st = state.ctx.db.http_state(id)?; out.push(FeedRow { @@ -363,10 +375,13 @@ async fn feeds(State(state): State) -> Result>, ApiE title: s.title, image: s.image, folder: feed.folder.clone(), - keywords: feed.keywords.clone(), - allow_explicit: feed.allow_explicit, - auto_download: feed.auto_download, - max_new_per_check: feed.max_new_per_check, + keywords: mine.keywords.clone().unwrap_or_else(|| feed.keywords.clone()), + allow_explicit: mine.allow_explicit.unwrap_or(feed.allow_explicit), + auto_download: mine.auto_download.unwrap_or(feed.auto_download), + max_new_per_check: mine + .max_new_per_check + .map(|n| n as usize) + .or(feed.max_new_per_check), group: feed.group.clone(), orphaned: s.orphaned, // Derived from an OPML and not written to config until you change something. @@ -384,7 +399,7 @@ async fn feeds(State(state): State) -> Result>, ApiE last_error: s.last_error, entries: s.entries, downloaded: s.downloaded, - unread: state.ctx.db.unread_count(id)?, + unread: state.ctx.db.unread_count(user.id, id)?, }); } Ok(Json(out)) @@ -548,6 +563,7 @@ struct EntryPage { async fn entries( State(state): State, Path(id): Path, + user: crate::db::User, Query(page): Query, ) -> Result, ApiError> { let filter = crate::db::Filter::parse(page.filter.as_deref().unwrap_or("all")); @@ -555,14 +571,14 @@ async fn entries( let mut rows = state .ctx .db - .entries(&id, filter, search, page.offset, page.limit.clamp(1, 200))?; + .entries(user.id, &id, filter, search, page.offset, page.limit.clamp(1, 200))?; // Feed HTML is untrusted: it reaches the page only after ammonia has been through it. for row in &mut rows { if let Some(d) = &row.description { row.description = Some(ammonia::clean(d)); } } - let total = state.ctx.db.count_entries(&id, filter, search)?; + let total = state.ctx.db.count_entries(user.id, &id, filter, search)?; Ok(Json(EntryPage { total, entries: rows })) } @@ -577,19 +593,26 @@ struct NewFeed { async fn add_feed( State(state): State, + user: crate::db::User, Json(body): Json, ) -> Result, ApiError> { let mut cfg = (*state.ctx.cfg()).clone(); - // Derived feeds count as subscribed: adding one an OPML already lists would duplicate it. + // Someone else may already have it. Then adding costs nothing: no second fetch, no + // second copy on disk, just another name against the same feed. if let Some(existing) = crate::subscriptions(&state.ctx)? .into_iter() .find(|s| s.cfg.url == body.url) { - return Ok(Json(serde_json::json!({ "id": existing.id, "existing": true }))); + let already = state.ctx.db.subscription(user.id, &existing.id)?.is_some(); + state.ctx.db.subscribe(user.id, &existing.id)?; + return Ok(Json( + serde_json::json!({ "id": existing.id, "existing": already }), + )); } let id = crate::add_one(&state.ctx, &mut cfg, &body.url, body.folder, body.keywords).await?; cfg.save(&state.config_path)?; state.ctx.reload_cfg(&state.config_path)?; + state.ctx.db.subscribe(user.id, &id)?; Ok(Json(serde_json::json!({ "id": id, "existing": false }))) } @@ -626,10 +649,46 @@ async fn patch_feed( user: crate::db::User, Json(body): Json, ) -> Result { - // How often a feed is polled is the operator's call: it costs bandwidth, it is what - // publishers notice, and one impatient setting affects everyone reading the feed. - if body.schedule.is_some() && !user.is_admin { - return Err(ApiError::forbidden("only an admin sets when feeds are scanned")); + // What one person wants -- which items, whether to fetch them, how many at a time -- + // is theirs. It goes on their subscription and nobody else sees the change. + if state.ctx.db.subscription(user.id, &id)?.is_some() { + let mut mine = state + .ctx + .db + .subscription(user.id, &id)? + .unwrap_or_else(|| crate::db::Sub { feed_id: id.clone(), ..Default::default() }); + let mut touched = false; + if let Some(v) = body.keywords.clone() { + mine.keywords = Some(v.into_iter().filter(|k| !k.trim().is_empty()).collect()); + touched = true; + } + if let Some(v) = body.allow_explicit { + mine.allow_explicit = Some(v); + touched = true; + } + if let Some(v) = body.auto_download { + mine.auto_download = Some(v); + touched = true; + } + if let Some(v) = body.max_new_per_check { + mine.max_new_per_check = v.map(|n| n as i64); + touched = true; + } + if touched { + state.ctx.db.set_subscription(user.id, &mine)?; + } + } + + // The rest describes the feed itself -- where its files land, its address, when it is + // polled -- and there is one of those however many people read it. + let feed_level = body.url.is_some() || body.folder.is_some() || body.schedule.is_some(); + if !feed_level { + return Ok(StatusCode::NO_CONTENT); + } + if !user.is_admin { + return Err(ApiError::forbidden( + "the feed's address, folder and schedule are the same for everyone, so only an admin changes them", + )); } let mut cfg = (*state.ctx.cfg()).clone(); @@ -675,18 +734,6 @@ async fn patch_feed( if let Some(v) = body.folder { feed.folder = v.filter(|s| !s.trim().is_empty()); } - if let Some(v) = body.keywords { - feed.keywords = v.into_iter().filter(|k| !k.trim().is_empty()).collect(); - } - if let Some(v) = body.allow_explicit { - feed.allow_explicit = v; - } - if let Some(v) = body.auto_download { - feed.auto_download = v; - } - if let Some(v) = body.max_new_per_check { - feed.max_new_per_check = v; - } cfg.save(&state.config_path)?; state.ctx.reload_cfg(&state.config_path)?; if url_changed { @@ -700,7 +747,23 @@ async fn patch_feed( async fn remove_feed( State(state): State, Path(id): Path, + user: crate::db::User, ) -> Result { + // Unsubscribing is personal: it takes the feed off your list and leaves everyone + // else's alone. + state.ctx.db.unsubscribe(user.id, &id)?; + for child in crate::subscriptions(&state.ctx)? + .iter() + .filter(|s| s.cfg.group.as_deref() == Some(id.as_str())) + { + state.ctx.db.unsubscribe(user.id, &child.id)?; + } + if state.ctx.db.subscriber_count(&id)? > 0 { + return Ok(StatusCode::NO_CONTENT); + } + + // Nobody is left: the feed stops being scanned. Its files and history stay, so if + // someone subscribes again they do not pull the back catalogue a second time. let mut cfg = (*state.ctx.cfg()).clone(); if cfg.feeds.remove(&id).is_none() { // A derived feed: forget it here, though the OPML will list it again on the next @@ -708,7 +771,6 @@ async fn remove_feed( state.ctx.db.drop_managed(&id)?; return Ok(StatusCode::NO_CONTENT); } - // Downloads and history stay, so re-adding does not re-pull the back catalogue. cfg.save(&state.config_path)?; state.ctx.reload_cfg(&state.config_path)?; Ok(StatusCode::NO_CONTENT) @@ -723,14 +785,15 @@ struct Flags { async fn set_flags( State(state): State, Path((feed_id, guid)): Path<(String, String)>, + user: crate::db::User, Json(body): Json, ) -> Result { use crate::db::EntryFlag; if let Some(v) = body.read { - state.ctx.db.set_entry_flag(&feed_id, &guid, EntryFlag::Read, v)?; + state.ctx.db.set_entry_flag(user.id, &feed_id, &guid, EntryFlag::Read, v)?; } if let Some(v) = body.flagged { - state.ctx.db.set_entry_flag(&feed_id, &guid, EntryFlag::Flagged, v)?; + state.ctx.db.set_entry_flag(user.id, &feed_id, &guid, EntryFlag::Flagged, v)?; } Ok(StatusCode::NO_CONTENT) } @@ -838,15 +901,17 @@ struct Position { async fn set_position( State(state): State, Path((feed_id, guid)): Path<(String, String)>, + user: crate::db::User, Json(body): Json, ) -> Result { - state.ctx.db.set_position(&feed_id, &guid, body.secs)?; + state.ctx.db.set_position(user.id, &feed_id, &guid, body.secs)?; Ok(StatusCode::NO_CONTENT) } async fn read_all( State(state): State, Path(id): Path, + user: crate::db::User, ) -> Result, ApiError> { // A subscription's own row has no entries, so marking it read means everything under it. let mut ids = vec![id.clone()]; @@ -856,7 +921,7 @@ async fn read_all( .filter(|s| s.cfg.group.as_deref() == Some(id.as_str())) .map(|s| s.id), ); - let n = state.ctx.db.mark_all_read(&ids)?; + let n = state.ctx.db.mark_all_read(user.id, &ids)?; Ok(Json(serde_json::json!({ "marked": n }))) } diff --git a/tests/ui/app.spec.js b/tests/ui/app.spec.js index e8b3a9a..bdf4bf6 100644 --- a/tests/ui/app.spec.js +++ b/tests/ui/app.spec.js @@ -256,3 +256,47 @@ test('opening an item marks it read, and the toggle flips it back', async ({ pag expect(errors).toEqual([]); }); + +test('a second person has their own feeds and their own read state', async ({ browser }) => { + const { execFileSync } = require('child_process'); + const setup = require('./global-setup'); + const env = { + ...process.env, + IPX_CONFIG: `${setup.root}/config/config.toml`, + IPX_DATA_DIR: `${setup.root}/data`, + }; + try { + execFileSync('./target/debug/ipx', ['user', 'add', 'sam'], { input: 'sampassword', env }); + } catch (e) { + if (!String(e.stderr || e.stdout).includes('already exists')) throw e; + } + + // A fresh context, so none of the admin's cookies come along. + const ctx = await browser.newContext(); + const page = await ctx.newPage(); + await page.goto('/login'); + await page.locator('#name').fill('sam'); + await page.locator('#pw').fill('sampassword'); + await page.locator('button[type=submit]').click(); + await expect(page.locator('#feedlist')).toBeVisible(); + + // Sam subscribes to nothing yet, so sees nothing -- the admin's feeds are not theirs. + await expect(page.locator('#feedlist')).toContainText('No feeds.'); + await expect(page.locator('#prefs')).toBeHidden(); // not an admin + + // Subscribing to a feed the admin already has costs no second fetch: same feed, same + // files, but Sam's own read state. + await page.locator('#addFeed').click(); + await page.locator('#nurl').fill('http://127.0.0.1:8792/show.xml'); + await page.locator('#nsave').click(); + await expect(page.locator('.feed', { hasText: 'Test Show' })).toBeVisible({ timeout: 20_000 }); + + await page.locator('.feed', { hasText: 'Test Show' }).click(); + await expect(page.locator('.ep').first()).toBeVisible({ timeout: 20_000 }); + // The admin read these earlier in this file; for Sam they are all still unread. + const rows = await page.locator('.ep').count(); + await page.locator('.tabs button', { hasText: 'Unread' }).click(); + await expect(page.locator('.ep')).toHaveCount(rows); + + await ctx.close(); +}); diff --git a/tests/ui/global-setup.js b/tests/ui/global-setup.js index c09e795..b777560 100644 --- a/tests/ui/global-setup.js +++ b/tests/ui/global-setup.js @@ -10,7 +10,15 @@ const TOKEN = 'testtokentesttokentesttoken12345'; // fixed, so tests need not // Called from playwright.config.js at load time, NOT as globalSetup: Playwright starts // webServer *before* globalSetup, so a config written there does not exist yet when the // daemon launches -- it would fall back to the real config and fight the live daemon. +// Playwright imports this config again in every worker process, so prepare() runs more +// than once per suite. Wiping on the second call deleted the data directory out from under +// the running daemon: it kept serving from the unlinked inode, while anything else opening +// that path -- the CLI, a query -- got a brand new empty database and disagreed with it. function prepare() { + // Only the process that launches the run may wipe. A worker gets TEST_WORKER_INDEX. + if (process.env.TEST_WORKER_INDEX !== undefined || process.env.PW_WORKER_INDEX !== undefined) { + return; + } fs.rmSync(root, { recursive: true, force: true }); for (const d of ['config', 'data', 'downloads']) { fs.mkdirSync(path.join(root, d), { recursive: true }); diff --git a/web/index.html b/web/index.html index 92c167d..d4054f8 100644 --- a/web/index.html +++ b/web/index.html @@ -45,6 +45,8 @@ --shadow:0 8px 28px rgba(45,83,145,.14); } *{box-sizing:border-box} +/* A rule that sets display beats the UA's [hidden], and several below do. */ +[hidden]{display:none!important} html,body{height:100%} body{ margin:0;background:var(--bg);color:var(--fg); @@ -1230,8 +1232,8 @@ function settingsModal(f){ ${f.managed?`

This feed comes from an OPML subscription and follows its settings. Saving anything here gives it its own entry in config.toml, and it stops following the subscription's settings.

`:''} -
-
+

These are your settings for this feed. + Everyone else keeps their own.

Comma separated. Empty takes everything.
@@ -1243,23 +1245,34 @@ function settingsModal(f){
- +
- Editing this keeps every item and download — handy when an auth token - in the URL is rotated. The feed is re-checked from scratch on the next scan.
+ ${S.me&&S.me.admin + ? `Shared with everyone reading this feed. Editing it keeps every item and download — + handy when an auth token in the URL is rotated. The feed is re-checked from scratch + on the next scan.` + : `The same for everyone reading this feed, so only an admin can change it.`} + ${S.me&&S.me.admin?`
+ + Where the files land. There is one copy however many people + subscribe, so this is the same for everyone.
`:''}
`); $('#scopy').onclick=()=>copyText($('#surl').value,$('#scopy')); $('#ssave').onclick=async()=>{ const max=$('#smax').value; try{ - await api(`/api/feeds/${encodeURIComponent(f.id)}`,{method:'PATCH',body:JSON.stringify({ - url:$('#surl').value.trim(), - folder:$('#sfolder').value.trim()||null, + const patch={ keywords:$('#skw').value.split(',').map(s=>s.trim()).filter(Boolean), max_new_per_check:max===''?null:Number(max), - auto_download:$('#sauto').checked, allow_explicit:$('#sexp').checked})}); + auto_download:$('#sauto').checked, allow_explicit:$('#sexp').checked}; + // The shared half is an admin's to change, and the API refuses it from anyone else. + if(S.me&&S.me.admin){ + patch.url=$('#surl').value.trim(); + patch.folder=$('#sfolder').value.trim()||null; + } + await api(`/api/feeds/${encodeURIComponent(f.id)}`,{method:'PATCH',body:JSON.stringify(patch)}); closeModal(); toast('Saved — applies on the next scan'); await loadFeeds(true); renderFeed(); loadEntries(); }catch(e){ toast(e.message,true); } @@ -1285,7 +1298,8 @@ function downloadLatestModal(f){ function removeFeed(f){ openModal(`

Unsubscribe?

-

Removes ${esc(f.title||f.id)} from your feeds. +

Removes ${esc(f.title||f.id)} from your feeds. Anyone else + reading it keeps it, along with their own read state. Downloaded files and history are kept, so re-adding it will not pull the back catalogue again.

`);