diff --git a/CHANGELOG.md b/CHANGELOG.md index 7aad6d9..39b195a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,6 +36,17 @@ The long form, with what was wrong before and how it was found, is in - Export and Import in Settings say what they do. - The sign-in page shows the original icon large. - Nothing animates when your system asks for reduced motion. +- The pages are about 90 KB smaller: the icon is served once instead of written into each. +- A web token generated for a new install is 64 characters instead of 32. + +### Removed + +- The systemd units in `contrib/`. Run ipx with Docker, or point a unit of your own at + `ipx daemon`. +- Upgrading from before 0.3.0 directly: what was read, kept or part-played before accounts is no + longer carried over to the admin, and OPML feeds that old versions wrote into `config.toml` are + no longer moved out of it. Upgrade through 0.4.0 first. +- `interval_mins` in `config.toml` is ignored; use `schedule`. ### Fixed diff --git a/CLAUDE.md b/CLAUDE.md index 2856103..1be1ebd 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -99,9 +99,9 @@ Non-trivial logic leaves one runnable check behind. Pure functions (`merge_polic * **`enclosures.url` is globally UNIQUE.** It is the dedupe key and the reason one file serves every subscriber. Two feeds publishing the same URL means only the first one scanned shows it. -* **`entries.read`, `entries.flagged` and `entries.position` are dead columns.** Read state lives in - `entry_state` per user. Two bugs have already come from queries still reading the old ones - (retention, and the entry pruner) — grep before adding a third. +* **Read state lives in `entry_state`, per user, and nowhere else.** `entries` had `read`, `flagged` + and `position` columns from before accounts; two bugs came from queries still reading them + (retention, and the entry pruner), and `migrate()` now drops them. * **The catalogue is config.toml; the subscriptions are in the database.** A feed exists once; `subscriptions(user_id, feed_id)` says who wants it and with what settings. OPML children are derived and never written to config. diff --git a/Cargo.lock b/Cargo.lock index 3f6be29..4f4bf79 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -436,17 +436,6 @@ dependencies = [ "shlex", ] -[[package]] -name = "cfb" -version = "0.14.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a347dcabdae9c31b0825fd6a8bed285ec9c2acb89c47827126d52fa4f59cece3" -dependencies = [ - "fnv", - "uuid", - "web-time", -] - [[package]] name = "cfg-if" version = "1.0.4" @@ -867,15 +856,6 @@ dependencies = [ "dirs-sys", ] -[[package]] -name = "dirs" -version = "7.0.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8d57d423b3c82e89b9a24ca3091fee61f456a26edbd28d26c65906f4bc1dcd8f" -dependencies = [ - "dirs-sys", -] - [[package]] name = "dirs-sys" version = "0.5.0" @@ -1608,15 +1588,6 @@ dependencies = [ "serde_core", ] -[[package]] -name = "infer" -version = "0.22.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f4200d433cbd5178df7797c9c2e75b348b728e39631cf14520d1e2fc424201f4" -dependencies = [ - "cfb", -] - [[package]] name = "intervaltree" version = "0.2.7" @@ -1643,9 +1614,7 @@ dependencies = [ "axum", "chrono", "clap", - "dirs", "futures-util", - "infer", "librqbit", "opml", "percent-encoding", @@ -1656,7 +1625,6 @@ dependencies = [ "serde", "serde_json", "tokio", - "tokio-stream", "toml", "tower", "tower-http 0.7.1", diff --git a/Cargo.toml b/Cargo.toml index 2447ec3..79ba48c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -11,9 +11,7 @@ atom_syndication = "0.12.10" axum = "0.8.9" chrono = { version = "0.4.45", default-features = false, features = ["std", "clock"] } clap = { version = "4.6.6", features = ["derive"] } -dirs = "7.0.0" futures-util = { version = "0.3.34", default-features = false, features = ["std"] } -infer = "0.22.0" librqbit = { version = "9.0.1", default-features = false, features = ["rust-tls", "http-api-client"] } opml = "1.1.6" percent-encoding = "2.3.2" @@ -24,7 +22,6 @@ rusqlite = { version = "0.40.2", features = ["bundled"] } serde = { version = "1.0.229", features = ["derive"] } serde_json = "1.0.151" tokio = { version = "1.53.1", features = ["rt-multi-thread", "macros", "fs", "io-util", "net", "sync", "time", "signal"] } -tokio-stream = { version = "0.1.19", features = ["sync"] } toml = "1.1.5" tower = { version = "0.5.3", features = ["util"] } tower-http = { version = "0.7.1", features = ["fs"] } diff --git a/TODO.md b/TODO.md index 2139787..ff8ba0e 100644 --- a/TODO.md +++ b/TODO.md @@ -1 +1,30 @@ # To do + +## Cut what is no longer needed + +From a whole-repo audit for over-engineering on 2026-09-12. Biggest cut first. + +- [x] **Pre-accounts adoption and the dead `entries` columns.** The copy of the old read state into + `entry_state` and the `entries.read`, `flagged` and `position` columns are gone. Its other half, + subscribing the first admin to the catalogue, was not dead and stays as `adopt_catalogue`. + (`src/db.rs`, `src/main.rs`) +- [x] **`contrib/` systemd units.** From before the container; nothing points at them. +- [x] **`migrate_opml_children`.** A one-time move of OPML children out of `config.toml` that has + run. Delete it and its call. (`src/main.rs`) +- [x] **The legacy `interval_mins` key.** Production uses `schedule`. Delete the field, the fallback + in `General::interval` and its test. (`src/config.rs`) +- [x] **`Db::entries` and `Db::count_entries`.** One-line wrappers only the tests call; the tests + call `entries_in` and `count_in` instead. (`src/db.rs`) +- [x] **`web::generate_token`.** Repeats `auth::new_session_token`. Use that. (`src/web.rs`) +- [x] **Page leftovers.** `globalEvery`, `S.busy`, `S.limit`, `unitOptions`' `firstLabel`, `--r`, + `.ep.open`, the phone `.ep .art`, the duplicate phone `.fhead.slim{flex-wrap}`, the second + `#sidebar{z-index}`, and the `on()` helper. (`web/index.html`) +- [x] **`logbuf` visitors.** `record_i64`, `record_u64` and `record_bool` repeat what `Visit`'s + defaults already do through `record_debug`. (`src/logbuf.rs`) +- [x] **The `infer` dependency.** Its torrent check is the `d8:announce` test on the next line. +- [x] **The `dirs` dependency.** `XDG_CONFIG_HOME`, `XDG_DATA_HOME` and `HOME` from `std::env`. +- [x] **The `tokio-stream` dependency.** `futures_util::stream::unfold` over the broadcast receiver. +- [x] **The icon inlined four times.** About 94 KB of base64 across both pages; serve it once as + `/icon.png` from `include_bytes!`, open without signing in like `/login`. + +After these: `cargo test`, `node tests/page-smoke.js`, `npx playwright test`. diff --git a/contrib/ipx-scan.service b/contrib/ipx-scan.service deleted file mode 100644 index eafb932..0000000 --- a/contrib/ipx-scan.service +++ /dev/null @@ -1,7 +0,0 @@ -[Unit] -Description=ipx feed scan (one shot) - -[Service] -Type=oneshot -ExecStart=%h/.cargo/bin/ipx fetch -Environment=IPX_LOG=ipx=info diff --git a/contrib/ipx.service b/contrib/ipx.service deleted file mode 100644 index a166ac4..0000000 --- a/contrib/ipx.service +++ /dev/null @@ -1,18 +0,0 @@ -# User unit: install to ~/.config/systemd/user/ipx.service, then -# systemctl --user enable --now ipx -# The socket lands in $XDG_RUNTIME_DIR/ipx.sock by default, so a UI running as the -# same user can attach without extra configuration. -[Unit] -Description=ipx podcatcher -After=network-online.target -Wants=network-online.target - -[Service] -Type=simple -ExecStart=%h/.cargo/bin/ipx daemon -Restart=on-failure -RestartSec=30 -Environment=IPX_LOG=ipx=info - -[Install] -WantedBy=default.target diff --git a/contrib/ipx.timer b/contrib/ipx.timer deleted file mode 100644 index 89fe5b8..0000000 --- a/contrib/ipx.timer +++ /dev/null @@ -1,16 +0,0 @@ -# Alternative to the daemon: a periodic one-shot scan, closer to how the original -# iPodderX agent was driven. Use this OR ipx.service, not both -- with no daemon -# running there is no socket, so a UI cannot attach. -# -# Install ipx-scan.service and ipx.timer to ~/.config/systemd/user/, then -# systemctl --user enable --now ipx.timer -[Unit] -Description=Periodic ipx feed scan - -[Timer] -OnBootSec=5min -OnUnitActiveSec=1h -Persistent=true - -[Install] -WantedBy=timers.target diff --git a/docs/architecture.md b/docs/architecture.md index 3f4edb2..fdbae7c 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -58,9 +58,9 @@ entry_state user_id, feed_id, guid, read, flagged, position PK (user_id, feed_id, guid) ``` -`entries` still has `read`, `flagged` and `position` columns from before accounts existed. They are -**dead** — the migration copied them into `entry_state` and nothing reads them now. Anything found -querying them is a bug; two were. +Read state is `entry_state` alone. `entries` had `read`, `flagged` and `position` columns from +before accounts; two bugs came from queries still reading them, and `migrate()` drops them from an +older database. Schema changes: add the table or column to `SCHEMA`, and for a column also to the list in `migrate()`, which does `PRAGMA table_info` then `ALTER TABLE ADD COLUMN`. `Db::memory()` runs the diff --git a/docs/configuration.md b/docs/configuration.md index a04c457..6a57c10 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -43,8 +43,6 @@ media_types = ["audio", "video"] can be fetched by hand; blog feeds put each article's header image in an ``, and without this the disk fills with artwork. Empty takes everything. -`interval_mins` from older configs is still read, and `schedule` supersedes it. - ## `[torrent]` ```toml diff --git a/docs/history.md b/docs/history.md index 9190bdd..33b0279 100644 --- a/docs/history.md +++ b/docs/history.md @@ -6,6 +6,38 @@ reasoning lives. New write-ups go at the top. See [README.md](../README.md) for what the thing is. +## 2026-09-12 — Cutting what had outlived its reason + +A whole-repo audit for over-engineering listed twelve things to cut, and all of them went. + +- **Upgrades from before accounts.** `migrate_opml_children` moved OPML feeds that old versions + wrote into `config.toml` out to the database, and ran at every daemon start to do nothing after + the first. Production ran it in 0.3.0; anything older has to pass through 0.4.0. +- **Half of the adoption, and not the other half.** The audit called `adopt_existing_library` a + one-time migration and it was cut whole. It did two jobs: copy the old read state into + `entry_state`, which was dead, and subscribe the first admin to the whole catalogue while nobody + subscribed to anything, which is how a fresh install's first account gets `config.toml`'s feeds. + The browser suite caught it at once, signing in to an empty sidebar; `cargo test` had no idea. + The second job is back as `adopt_catalogue`, with a unit test of its own. +- **The dead `entries` columns.** `read`, `flagged` and `position` moved to `entry_state` with + accounts. The adoption's copy was their last reader, but `record_entry` still wrote them, and + still reset `read` when a title changed, which nothing looked at. Two bugs came from queries + reading them. `migrate()` now drops them from an existing database (SQLite has had `DROP COLUMN` + since 3.35), and a test builds an old table to prove it. +- **`interval_mins`**, which `schedule` replaced. An old config that still has the key loads; the + key is ignored, and the config test carries it to keep that true. +- **Three dependencies.** `infer` was only asked whether a file is a torrent, and the check after + it already looked for `d8:announce`, which is what `infer` looks for. `dirs` was three lookups of + `XDG_CONFIG_HOME`, `XDG_DATA_HOME` and `HOME`. `tokio-stream` wrapped the broadcast receiver for + the event stream; `futures_util::stream::unfold` does the same, lagging clients included. +- **Two token generators.** The web token came from a copy of the session-token code, with a + clock fallback on top. It uses `auth::new_session_token` now, and is 64 characters. +- **The icon inlined four times**, 23 KB of base64 each, into both pages. It is `/icon.png` now, + outside the sign-in wall with `/login`, since the sign-in page shows it. +- Also: the `contrib/` systemd units from before the container, `Db::entries` and + `Db::count_entries` that only the tests called, three `logbuf` visitors that repeated the trait's + defaults, and unused state, a helper and dead CSS in the page. + ## 2026-09-11 — A design pass on the web UI A review against screenshots of every view in all three themes found that Dark and Light read as a diff --git a/src/config.rs b/src/config.rs index 7d6605b..361fc60 100644 --- a/src/config.rs +++ b/src/config.rs @@ -26,9 +26,6 @@ pub struct General { /// How often to re-check feeds: "every 30m", "every 4h", "90" (minutes), "1d". /// A feed's own `schedule` overrides this. pub schedule: String, - /// Superseded by `schedule`. Still read so existing configs keep working. - #[serde(skip_serializing_if = "Option::is_none")] - pub interval_mins: Option, pub organize: Organize, /// 0 = unlimited. pub max_total_gb: f64, @@ -153,7 +150,6 @@ impl Default for General { download_dir: home().join("Podcasts"), socket: default_socket(), schedule: "every 60m".into(), - interval_mins: None, organize: Organize::Feed, max_total_gb: 0.0, max_age_days: 0, @@ -176,8 +172,8 @@ impl Default for Torrent { } impl General { - /// Minutes between checks. Falls back to the legacy `interval_mins`, then to an hour. - /// A malformed value warns rather than stopping the daemon. + /// Minutes between checks, or an hour when `schedule` is empty or unreadable. A malformed + /// value warns rather than stopping the daemon. pub fn interval(&self) -> u64 { if let Some(n) = parse_interval(&self.schedule) { return n; @@ -185,7 +181,7 @@ impl General { if !self.schedule.trim().is_empty() { tracing::warn!(schedule = %self.schedule, "unrecognised schedule; using the default"); } - self.interval_mins.filter(|n| *n > 0).unwrap_or(60) + 60 } } @@ -278,9 +274,7 @@ pub fn config_path() -> PathBuf { if let Ok(p) = std::env::var("IPX_CONFIG") { return PathBuf::from(p); } - dirs::config_dir() - .unwrap_or_else(|| home().join(".config")) - .join("ipx/config.toml") + xdg("XDG_CONFIG_HOME", ".config").join("ipx/config.toml") } /// `$IPX_DATA_DIR`, else `$XDG_DATA_HOME/ipx`. @@ -288,9 +282,7 @@ pub fn data_dir() -> PathBuf { if let Ok(p) = std::env::var("IPX_DATA_DIR") { return PathBuf::from(p); } - dirs::data_dir() - .unwrap_or_else(|| home().join(".local/share")) - .join("ipx") + xdg("XDG_DATA_HOME", ".local/share").join("ipx") } fn default_socket() -> PathBuf { @@ -346,8 +338,16 @@ pub fn unique_slug(text: &str, taken: &BTreeMap) -> String { (2..).map(|n| format!("{base}-{n}")).find(|s| !taken.contains_key(s)).unwrap() } +/// `$var`, or `~/fallback` when it is unset or empty, as the XDG base directory spec says. +fn xdg(var: &str, fallback: &str) -> PathBuf { + std::env::var_os(var) + .filter(|v| !v.is_empty()) + .map(PathBuf::from) + .unwrap_or_else(|| home().join(fallback)) +} + fn home() -> PathBuf { - dirs::home_dir().unwrap_or_else(|| PathBuf::from(".")) + std::env::var_os("HOME").map(PathBuf::from).unwrap_or_else(|| PathBuf::from(".")) } fn expand_tilde(p: &Path) -> PathBuf { @@ -367,6 +367,8 @@ mod tests { r#" [general] download_dir = "/tmp/pods" + # A key older versions read. An old config that still has it has to load. + interval_mins = 45 [feeds.example] url = "https://example.com/feed.xml" @@ -403,22 +405,17 @@ mod tests { } #[test] - fn interval_falls_back_through_legacy_then_default() { + fn interval_falls_back_to_an_hour() { let mut g = General::default(); assert_eq!(g.interval(), 60, "the default schedule"); g.schedule = "every 15m".into(); assert_eq!(g.interval(), 15); - // A config written before `schedule` existed still works. + // Empty or garbage must not stop the daemon. g.schedule = String::new(); - g.interval_mins = Some(45); - assert_eq!(g.interval(), 45); - - // Garbage must not stop the daemon. + assert_eq!(g.interval(), 60); g.schedule = "whenever".into(); - assert_eq!(g.interval(), 45); - g.interval_mins = None; assert_eq!(g.interval(), 60); } diff --git a/src/db.rs b/src/db.rs index 79f7473..55be6db 100644 --- a/src/db.rs +++ b/src/db.rs @@ -40,14 +40,10 @@ CREATE TABLE IF NOT EXISTS entries ( published INTEGER, description TEXT, first_seen INTEGER NOT NULL, - read INTEGER NOT NULL DEFAULT 0, - flagged INTEGER NOT NULL DEFAULT 0, image TEXT, duration INTEGER, episode INTEGER, season INTEGER, - -- Seconds into the audio, so playback resumes where it was left. - position INTEGER NOT NULL DEFAULT 0, PRIMARY KEY (feed_id, guid) ); @@ -93,7 +89,7 @@ CREATE TABLE IF NOT EXISTS subscriptions ( PRIMARY KEY (user_id, feed_id) ); --- Read, starred and how far in. One row per person per item, created on first touch; +-- Read, kept 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, @@ -143,8 +139,8 @@ pub struct Managed { pub orphaned: bool, } -/// Adds columns that later versions introduced. CREATE TABLE IF NOT EXISTS does nothing to -/// a table that already exists, so an installed database needs them added explicitly. +/// Adds the columns later versions introduced and drops the ones they retired. CREATE TABLE IF +/// NOT EXISTS does nothing to a table that already exists, so an installed database needs both. fn migrate(conn: &Connection) -> Result<()> { let wanted: &[(&str, &str, &str)] = &[ ("feeds", "image", "TEXT"), @@ -155,18 +151,29 @@ fn migrate(conn: &Connection) -> Result<()> { ("entries", "duration", "INTEGER"), ("entries", "episode", "INTEGER"), ("entries", "season", "INTEGER"), - ("entries", "position", "INTEGER NOT NULL DEFAULT 0"), ]; - for (table, column, ty) in wanted { + // Read state from before accounts, long since moved to entry_state. Two bugs came from + // queries still reading these after they stopped meaning anything, so they go. + let retired: &[(&str, &str)] = &[("entries", "read"), ("entries", "flagged"), ("entries", "position")]; + let has = |table: &str, column: &str| -> Result { let mut stmt = conn.prepare(&format!("PRAGMA table_info({table})"))?; - let existing: Vec = stmt + let names = stmt .query_map([], |r| r.get::<_, String>(1))? .collect::>>()?; - if !existing.iter().any(|c| c == column) { + Ok(names.iter().any(|c| c == column)) + }; + for (table, column, ty) in wanted { + if !has(table, column)? { tracing::info!(table, column, "adding column"); conn.execute_batch(&format!("ALTER TABLE {table} ADD COLUMN {column} {ty}"))?; } } + for (table, column) in retired { + if has(table, column)? { + tracing::info!(table, column, "dropping column"); + conn.execute_batch(&format!("ALTER TABLE {table} DROP COLUMN {column}"))?; + } + } Ok(()) } @@ -345,22 +352,19 @@ impl Db { /// Returns true when this entry had not been seen before. /// - /// A changed description or title flips `read` back to 0, which is what the original's - /// textDiff dance was ultimately for -- minus the diff markup, which the UI can do. pub fn record_entry(&self, feed_id: &str, e: &crate::feed::Entry) -> Result { let conn = self.conn.lock().unwrap(); let inserted = conn.execute( "INSERT OR IGNORE INTO entries - (feed_id, guid, title, link, published, description, first_seen, read, flagged, + (feed_id, guid, title, link, published, description, first_seen, image, duration, episode, season) - VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, 0, 0, ?8, ?9, ?10, ?11)", + VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10, ?11)", rusqlite::params![ feed_id, e.guid, e.title, e.link, e.published, e.description, now(), e.image, e.duration, e.episode, e.season ], )?; if inserted == 0 { - // The SET expressions see the pre-update row, so this compares old vs new. conn.execute( "UPDATE entries SET title = coalesce(?3, title), @@ -368,8 +372,7 @@ impl Db { image = coalesce(?5, image), duration = coalesce(?6, duration), episode = coalesce(?7, episode), - season = coalesce(?8, season), - read = CASE WHEN description IS NOT ?4 OR title IS NOT ?3 THEN 0 ELSE read END + season = coalesce(?8, season) WHERE feed_id = ?1 AND guid = ?2", rusqlite::params![ feed_id, e.guid, e.title, e.description, @@ -687,22 +690,9 @@ pub struct EncRow { } impl Db { - /// One page of a feed's entries, newest first, each with its enclosures attached. - /// `search` matches title and description, case-insensitively. - pub fn entries( - &self, - user_id: i64, - feed_id: &str, - filter: Filter, - search: Option<&str>, - offset: i64, - limit: i64, - ) -> Result> { - self.entries_in(user_id, Some(feed_id), filter, search, offset, limit, &order_sql("published", "desc")) - } - - /// `entries` for one feed, or across every feed the person subscribes to when `feed_id` - /// is None: the All Subscriptions view. + /// One page of entries, each with its enclosures attached: one feed's, or every feed the + /// person subscribes to when `feed_id` is None (All Subscriptions). `search` matches title + /// and description, case-insensitively. pub fn entries_in( &self, user_id: i64, @@ -793,18 +783,7 @@ impl Db { Ok(rows) } - /// How many entries match, so the UI knows whether there is another page. - pub fn count_entries( - &self, - user_id: i64, - feed_id: &str, - filter: Filter, - search: Option<&str>, - ) -> Result { - self.count_in(user_id, Some(feed_id), filter, search) - } - - /// `count_entries` for one feed, or across every feed the person subscribes to. + /// How many entries `entries_in` would page through, so the UI knows whether there is more. pub fn count_in( &self, user_id: i64, @@ -838,24 +817,16 @@ impl Db { 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 { + /// The first admin starts subscribed to the whole catalogue: whoever wrote config.toml meant + /// to read those feeds, and without this a fresh install signs in to an empty sidebar. Runs + /// only while nobody subscribes to anything, so an unsubscribe is never undone. + pub fn adopt_catalogue(&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)", @@ -868,7 +839,7 @@ impl Db { SELECT ?1, id, ?2 FROM feeds", params![user_id, now()], )?; - Ok(moved) + Ok(catalogue.len()) } // ---- subscriptions ---- @@ -1528,8 +1499,9 @@ mod tests { // 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 order = order_sql("published", "desc"); + let page = |user| db.entries_in(user, Some("f"), Filter::All, None, 0, 50, &order).unwrap(); + let (ray, sam) = (page(1), page(2)); 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); @@ -1553,6 +1525,49 @@ mod tests { assert_eq!(sum.downloaded, 0); } + #[test] + fn an_old_database_loses_the_retired_read_columns() { + let conn = Connection::open_in_memory().unwrap(); + conn.execute_batch( + "CREATE TABLE entries (feed_id TEXT NOT NULL, guid TEXT NOT NULL, + first_seen INTEGER NOT NULL, read INTEGER NOT NULL DEFAULT 0, + flagged INTEGER NOT NULL DEFAULT 0, position INTEGER NOT NULL DEFAULT 0, + PRIMARY KEY (feed_id, guid));", + ) + .unwrap(); + // The same order as open(): the schema leaves the old table alone, migrate() fixes it. + conn.execute_batch(SCHEMA).unwrap(); + migrate(&conn).unwrap(); + let cols: Vec = conn + .prepare("PRAGMA table_info(entries)") + .unwrap() + .query_map([], |r| r.get(1)) + .unwrap() + .collect::>() + .unwrap(); + assert!(!cols.iter().any(|c| ["read", "flagged", "position"].contains(&c.as_str())), "{cols:?}"); + assert!(cols.iter().any(|c| c == "image"), "and it still gains the newer ones"); + } + + #[test] + fn the_first_admin_starts_with_the_catalogue_and_only_once() { + // Cutting this along with the dead read columns left the browser suite's admin with an + // empty sidebar: it is how a fresh install's first account gets config.toml's feeds. + let db = Db::memory().unwrap(); + db.exec_for_test("INSERT INTO users (id, name, is_admin, created) VALUES (1,'admin',1,0);") + .unwrap(); + let subs = || -> i64 { + db.conn.lock().unwrap().query_row("SELECT count(*) FROM subscriptions", [], |r| r.get(0)).unwrap() + }; + let catalogue = ["a".to_string(), "b".to_string()]; + assert_eq!(db.adopt_catalogue(1, &catalogue).unwrap(), 2); + assert_eq!(subs(), 2); + // Once anyone subscribes to anything it never runs again, so an unsubscribe sticks. + db.unsubscribe(1, "a").unwrap(); + assert_eq!(db.adopt_catalogue(1, &catalogue).unwrap(), 0); + assert_eq!(subs(), 1); + } + #[test] fn every_filter_works_with_and_without_a_search_term() { // Regression: the search clause used to be omitted when no term was given, while @@ -1576,21 +1591,22 @@ mod tests { 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(7, "f", f, None, 0, 50).unwrap(); - let n = db.count_entries(7, "f", f, None).unwrap(); + let order = order_sql("published", "desc"); + let rows = db.entries_in(7, Some("f"), f, None, 0, 50, &order).unwrap(); + let n = db.count_in(7, Some("f"), f, None).unwrap(); assert_eq!(rows.len() as i64, n, "{f:?} count disagrees with the page"); - let rows = db.entries(7, "f", f, Some("dive"), 0, 50).unwrap(); - let n = db.count_entries(7, "f", f, Some("dive")).unwrap(); + let rows = db.entries_in(7, Some("f"), f, Some("dive"), 0, 50, &order).unwrap(); + let n = db.count_in(7, Some("f"), f, Some("dive")).unwrap(); assert_eq!(rows.len() as i64, n, "{f:?} with search disagrees"); } - 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, + assert_eq!(db.count_in(7, Some("f"), Filter::All, None).unwrap(), 3); + assert_eq!(db.count_in(7, Some("f"), Filter::Unread, None).unwrap(), 1); + assert_eq!(db.count_in(7, Some("f"), Filter::Downloaded, None).unwrap(), 1); + assert_eq!(db.count_in(7, Some("f"), Filter::Flagged, None).unwrap(), 1); + assert_eq!(db.count_in(7, Some("f"), Filter::All, Some("dive")).unwrap(), 2); + assert_eq!(db.count_in(7, Some("f"), Filter::All, Some("NOTES two")).unwrap(), 1, "search is case-insensitive and covers the description"); } diff --git a/src/download.rs b/src/download.rs index 7b9e145..b160f8b 100644 --- a/src/download.rs +++ b/src/download.rs @@ -223,7 +223,7 @@ enum Sniffed { /// 2008 and so always answered 'data'. async fn sniff(path: &Path) -> Result { let head = read_head(path, 512).await?; - if infer::is(&head, "torrent") || head.starts_with(b"d8:announce") || head.starts_with(b"d7:") { + if head.starts_with(b"d8:announce") || head.starts_with(b"d7:") { return Ok(Sniffed::Torrent); } let text = String::from_utf8_lossy(&head); diff --git a/src/logbuf.rs b/src/logbuf.rs index 443ec3f..ede2320 100644 --- a/src/logbuf.rs +++ b/src/logbuf.rs @@ -111,18 +111,11 @@ impl Visit for Collect { fn record_debug(&mut self, field: &Field, value: &dyn std::fmt::Debug) { self.add(field, format!("{value:?}")); } + // Numbers and bools reach record_debug through the trait's defaults, which prints them the + // same way. A string would print quoted there, hence its own method. fn record_str(&mut self, field: &Field, value: &str) { self.add(field, value.to_owned()); } - fn record_i64(&mut self, field: &Field, value: i64) { - self.add(field, value.to_string()); - } - fn record_u64(&mut self, field: &Field, value: u64) { - self.add(field, value.to_string()); - } - fn record_bool(&mut self, field: &Field, value: bool) { - self.add(field, value.to_string()); - } } #[cfg(test)] diff --git a/src/main.rs b/src/main.rs index e284cd2..7f9ece5 100644 --- a/src/main.rs +++ b/src/main.rs @@ -334,8 +334,7 @@ async fn daemon( anyhow::bail!("a daemon is already listening on {}", socket.display()); } - // A database with nobody in it cannot be signed into, and an install that predates - // accounts still has to serve its owner. Both get the same starting point. + // A database with nobody in it cannot be signed into. if ctx.db.users()?.is_empty() { ctx.db.create_user("admin", Some(&crate::auth::hash_password(DEFAULT_PASSWORD)?), true)?; tracing::warn!( @@ -344,22 +343,15 @@ 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) { + match ctx.db.adopt_catalogue(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"), + Ok(n) => tracing::info!(user = %admin.name, feeds = n, "subscribed the first admin to the catalogue"), + Err(e) => tracing::error!(error = %e, "could not subscribe the first admin to the catalogue"), } } - 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(_) => {} - Err(e) => tracing::warn!(error = ?e, "could not tidy OPML feeds out of the config"), - } - match ctx.db.requeue_interrupted() { Ok(n) if n > 0 => tracing::info!(count = n, "requeued downloads interrupted by a restart"), Ok(_) => {} @@ -468,7 +460,7 @@ async fn start_web( let mut fresh = (*cfg).clone(); fresh.web.enabled = true; fresh.web.bind = bind.clone(); - fresh.web.token = web::generate_token(); + fresh.web.token = crate::auth::new_session_token(); fresh.save(config_path)?; ctx.reload_cfg(config_path)?; println!("web ui token generated. Open:\n http://{bind}/?token={}", fresh.web.token); @@ -916,36 +908,6 @@ pub fn subscriptions(ctx: &Ctx) -> Result> { Ok(out) } -/// Moves OPML children that older versions wrote into config.toml over to the database. -/// They were never yours to edit, and 80-odd of them made the file unreadable. -fn migrate_opml_children(ctx: &Ctx) -> Result { - let cfg = (*ctx.cfg()).clone(); - let children: Vec<(String, config::Feed)> = cfg - .feeds - .iter() - .filter(|(_, f)| f.group.is_some()) - .map(|(id, f)| (id.clone(), f.clone())) - .collect(); - if children.is_empty() { - return Ok(0); - } - let mut fresh = cfg.clone(); - for (id, f) in &children { - let group = f.group.clone().unwrap_or_default(); - let title = ctx - .db - .feed_summary(id) - .ok() - .and_then(|s| s.title) - .unwrap_or_else(|| id.clone()); - ctx.db.upsert_managed(id, &f.url, &title, &group)?; - fresh.feeds.remove(id); - } - fresh.save(&ctx.config_path)?; - ctx.reload_cfg(&ctx.config_path)?; - Ok(children.len()) -} - /// Seconds to wait before re-checking a feed. /// /// A per-feed schedule is an explicit instruction and wins outright. Without one, the diff --git a/src/web.rs b/src/web.rs index 4e25845..d0349df 100644 --- a/src/web.rs +++ b/src/web.rs @@ -12,9 +12,7 @@ use axum::{ }, routing::{delete, get, patch, post}, }; -use futures_util::StreamExt; use serde::Deserialize; -use tokio_stream::wrappers::BroadcastStream; use tower::ServiceExt; use tower_http::services::ServeFile; use serde::Serialize; @@ -35,28 +33,6 @@ pub struct WebState { pub events: broadcast::Sender, } -/// A 32-hex-character shared secret, generated when config.toml has none. -/// -/// ponytail: /dev/urandom rather than a CSPRNG crate -- 16 bytes, once, on a Unix-only -/// binary. Falls back to the clock only if urandom is somehow unreadable, which would be a -/// weak token, so that case is logged loudly. -pub fn generate_token() -> String { - use std::io::Read; - let mut bytes = [0u8; 16]; - match std::fs::File::open("/dev/urandom").and_then(|mut f| f.read_exact(&mut bytes)) { - Ok(()) => {} - Err(e) => { - tracing::error!(error = %e, "could not read /dev/urandom; token is NOT secure"); - let n = std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .map(|d| d.as_nanos() as u64) - .unwrap_or(0); - bytes[..8].copy_from_slice(&n.to_le_bytes()); - } - } - bytes.iter().map(|b| format!("{b:02x}")).collect() -} - pub fn router(state: WebState) -> Router { Router::new() .route("/", get(index)) @@ -88,6 +64,7 @@ pub fn router(state: WebState) -> Router { // Signing in cannot require being signed in, so these sit outside the auth layer. .route("/login", get(login_page)) .route("/api/login", post(login)) + .route("/icon.png", get(icon)) .layer(middleware::from_fn(access_log)) .with_state(state) } @@ -433,6 +410,15 @@ async fn login_page() -> Html<&'static str> { Html(include_str!("../web/login.html")) } +/// The 2004 icon, served once for both pages rather than inlined as base64 into each. The +/// sign-in page shows it, so it sits outside the auth layer with /login. +async fn icon() -> impl IntoResponse { + ( + [(header::CONTENT_TYPE, "image/png"), (header::CACHE_CONTROL, "max-age=86400")], + include_bytes!("../web/ipodderx-icon.png").as_slice(), + ) +} + fn constant_time_eq(a: &str, b: &str) -> bool { let (a, b) = (a.as_bytes(), b.as_bytes()); if a.len() != b.len() { @@ -817,15 +803,6 @@ mod tests { "another feed already has that URL" ); } - - #[test] - fn generated_tokens_are_32_hex_chars_and_not_repeated() { - let a = generate_token(); - let b = generate_token(); - assert_eq!(a.len(), 32); - assert!(a.chars().all(|c| c.is_ascii_hexdigit())); - assert_ne!(a, b); - } } #[derive(Deserialize)] @@ -1247,9 +1224,20 @@ async fn fetch_now( /// The same broadcast the socket clients read, as server-sent events. async fn events(State(state): State) -> Sse>> { - let stream = BroadcastStream::new(state.events.subscribe()).filter_map(|ev| async move { - let ev = ev.ok()?; - Some(Ok(SseEvent::default().data(serde_json::to_string(&ev).ok()?))) + // A client that falls behind skips what it missed rather than being cut off. + let stream = futures_util::stream::unfold(state.events.subscribe(), |mut rx| async move { + loop { + match rx.recv().await { + Ok(ev) => { + if let Ok(data) = serde_json::to_string(&ev) { + let ev = Ok::<_, std::convert::Infallible>(SseEvent::default().data(data)); + return Some((ev, rx)); + } + } + Err(broadcast::error::RecvError::Lagged(_)) => {} + Err(broadcast::error::RecvError::Closed) => return None, + } + } }); Sse::new(stream).keep_alive(axum::response::sse::KeepAlive::default()) } @@ -1460,8 +1448,6 @@ async fn patch_settings( ))); } cfg.general.schedule = sched; - // The legacy key would otherwise keep shadowing intent in the file. - cfg.general.interval_mins = None; } if let Some(v) = body.max_new_per_check { cfg.general.max_new_per_check = v; diff --git a/tests/ui/app.spec.js b/tests/ui/app.spec.js index 6ffbaec..52c8fa6 100644 --- a/tests/ui/app.spec.js +++ b/tests/ui/app.spec.js @@ -348,6 +348,10 @@ test('a second person has their own feeds and their own read state', async ({ br const ctx = await browser.newContext(); const page = await ctx.newPage(); await page.goto('/login'); + // The sign-in page shows the icon, so it has to load before anyone has signed in. + const icon = await page.request.get('/icon.png'); + expect(icon.status()).toBe(200); + expect(icon.headers()['content-type']).toBe('image/png'); await page.locator('#name').fill('sam'); await page.locator('#pw').fill('sampassword'); await page.locator('button[type=submit]').click(); diff --git a/web/index.html b/web/index.html index e2915c6..b3a4df9 100644 --- a/web/index.html +++ b/web/index.html @@ -5,7 +5,7 @@ iPodderX - +
-

iPodderX

+

iPodderX