Cut what the audit found: dead columns, one-time upgrades, three deps

Works through TODO.md from the 2026-09-12 over-engineering audit. Drops the
entries.read/flagged/position columns (migrate() removes them from older
databases), migrate_opml_children, the legacy interval_mins key, the
contrib/ systemd units, test-only Db wrappers, a duplicate token generator,
redundant logbuf visitors, unused page state and CSS, and the infer, dirs
and tokio-stream dependencies. The icon is served once as /icon.png instead
of inlined four times, taking about 94 KB off the two pages.

The adoption's subscription half was not dead: it gives a fresh install's
first admin the config's feeds. It stays as adopt_catalogue, now tested.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TAC7sLVqfKmY6rsTLXzNgk
This commit is contained in:
2026-09-12 01:55:44 +00:00
parent 8937f35f00
commit dc63d6acaf
20 changed files with 235 additions and 298 deletions

View File

@@ -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. - Export and Import in Settings say what they do.
- The sign-in page shows the original icon large. - The sign-in page shows the original icon large.
- Nothing animates when your system asks for reduced motion. - 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 ### Fixed

View File

@@ -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 * **`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. 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 * **Read state lives in `entry_state`, per user, and nowhere else.** `entries` had `read`, `flagged`
`entry_state` per user. Two bugs have already come from queries still reading the old ones and `position` columns from before accounts; two bugs came from queries still reading them
(retention, and the entry pruner) — grep before adding a third. (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; * **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 `subscriptions(user_id, feed_id)` says who wants it and with what settings. OPML children are
derived and never written to config. derived and never written to config.

32
Cargo.lock generated
View File

@@ -436,17 +436,6 @@ dependencies = [
"shlex", "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]] [[package]]
name = "cfg-if" name = "cfg-if"
version = "1.0.4" version = "1.0.4"
@@ -867,15 +856,6 @@ dependencies = [
"dirs-sys", "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]] [[package]]
name = "dirs-sys" name = "dirs-sys"
version = "0.5.0" version = "0.5.0"
@@ -1608,15 +1588,6 @@ dependencies = [
"serde_core", "serde_core",
] ]
[[package]]
name = "infer"
version = "0.22.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "f4200d433cbd5178df7797c9c2e75b348b728e39631cf14520d1e2fc424201f4"
dependencies = [
"cfb",
]
[[package]] [[package]]
name = "intervaltree" name = "intervaltree"
version = "0.2.7" version = "0.2.7"
@@ -1643,9 +1614,7 @@ dependencies = [
"axum", "axum",
"chrono", "chrono",
"clap", "clap",
"dirs",
"futures-util", "futures-util",
"infer",
"librqbit", "librqbit",
"opml", "opml",
"percent-encoding", "percent-encoding",
@@ -1656,7 +1625,6 @@ dependencies = [
"serde", "serde",
"serde_json", "serde_json",
"tokio", "tokio",
"tokio-stream",
"toml", "toml",
"tower", "tower",
"tower-http 0.7.1", "tower-http 0.7.1",

View File

@@ -11,9 +11,7 @@ atom_syndication = "0.12.10"
axum = "0.8.9" axum = "0.8.9"
chrono = { version = "0.4.45", default-features = false, features = ["std", "clock"] } chrono = { version = "0.4.45", default-features = false, features = ["std", "clock"] }
clap = { version = "4.6.6", features = ["derive"] } clap = { version = "4.6.6", features = ["derive"] }
dirs = "7.0.0"
futures-util = { version = "0.3.34", default-features = false, features = ["std"] } 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"] } librqbit = { version = "9.0.1", default-features = false, features = ["rust-tls", "http-api-client"] }
opml = "1.1.6" opml = "1.1.6"
percent-encoding = "2.3.2" percent-encoding = "2.3.2"
@@ -24,7 +22,6 @@ rusqlite = { version = "0.40.2", features = ["bundled"] }
serde = { version = "1.0.229", features = ["derive"] } serde = { version = "1.0.229", features = ["derive"] }
serde_json = "1.0.151" serde_json = "1.0.151"
tokio = { version = "1.53.1", features = ["rt-multi-thread", "macros", "fs", "io-util", "net", "sync", "time", "signal"] } 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" toml = "1.1.5"
tower = { version = "0.5.3", features = ["util"] } tower = { version = "0.5.3", features = ["util"] }
tower-http = { version = "0.7.1", features = ["fs"] } tower-http = { version = "0.7.1", features = ["fs"] }

29
TODO.md
View File

@@ -1 +1,30 @@
# To do # 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`.

View File

@@ -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

View File

@@ -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

View File

@@ -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

View File

@@ -58,9 +58,9 @@ entry_state user_id, feed_id, guid, read, flagged, position
PK (user_id, feed_id, guid) PK (user_id, feed_id, guid)
``` ```
`entries` still has `read`, `flagged` and `position` columns from before accounts existed. They are Read state is `entry_state` alone. `entries` had `read`, `flagged` and `position` columns from
**dead** — the migration copied them into `entry_state` and nothing reads them now. Anything found before accounts; two bugs came from queries still reading them, and `migrate()` drops them from an
querying them is a bug; two were. older database.
Schema changes: add the table or column to `SCHEMA`, and for a column also to the list in 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 `migrate()`, which does `PRAGMA table_info` then `ALTER TABLE ADD COLUMN`. `Db::memory()` runs the

View File

@@ -43,8 +43,6 @@ media_types = ["audio", "video"]
can be fetched by hand; blog feeds put each article's header image in an `<enclosure>`, and can be fetched by hand; blog feeds put each article's header image in an `<enclosure>`, and
without this the disk fills with artwork. Empty takes everything. without this the disk fills with artwork. Empty takes everything.
`interval_mins` from older configs is still read, and `schedule` supersedes it.
## `[torrent]` ## `[torrent]`
```toml ```toml

View File

@@ -6,6 +6,38 @@ reasoning lives. New write-ups go at the top.
See [README.md](../README.md) for what the thing is. 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 ## 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 A review against screenshots of every view in all three themes found that Dark and Light read as a

View File

@@ -26,9 +26,6 @@ pub struct General {
/// How often to re-check feeds: "every 30m", "every 4h", "90" (minutes), "1d". /// How often to re-check feeds: "every 30m", "every 4h", "90" (minutes), "1d".
/// A feed's own `schedule` overrides this. /// A feed's own `schedule` overrides this.
pub schedule: String, pub schedule: String,
/// Superseded by `schedule`. Still read so existing configs keep working.
#[serde(skip_serializing_if = "Option::is_none")]
pub interval_mins: Option<u64>,
pub organize: Organize, pub organize: Organize,
/// 0 = unlimited. /// 0 = unlimited.
pub max_total_gb: f64, pub max_total_gb: f64,
@@ -153,7 +150,6 @@ impl Default for General {
download_dir: home().join("Podcasts"), download_dir: home().join("Podcasts"),
socket: default_socket(), socket: default_socket(),
schedule: "every 60m".into(), schedule: "every 60m".into(),
interval_mins: None,
organize: Organize::Feed, organize: Organize::Feed,
max_total_gb: 0.0, max_total_gb: 0.0,
max_age_days: 0, max_age_days: 0,
@@ -176,8 +172,8 @@ impl Default for Torrent {
} }
impl General { impl General {
/// Minutes between checks. Falls back to the legacy `interval_mins`, then to an hour. /// Minutes between checks, or an hour when `schedule` is empty or unreadable. A malformed
/// A malformed value warns rather than stopping the daemon. /// value warns rather than stopping the daemon.
pub fn interval(&self) -> u64 { pub fn interval(&self) -> u64 {
if let Some(n) = parse_interval(&self.schedule) { if let Some(n) = parse_interval(&self.schedule) {
return n; return n;
@@ -185,7 +181,7 @@ impl General {
if !self.schedule.trim().is_empty() { if !self.schedule.trim().is_empty() {
tracing::warn!(schedule = %self.schedule, "unrecognised schedule; using the default"); 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") { if let Ok(p) = std::env::var("IPX_CONFIG") {
return PathBuf::from(p); return PathBuf::from(p);
} }
dirs::config_dir() xdg("XDG_CONFIG_HOME", ".config").join("ipx/config.toml")
.unwrap_or_else(|| home().join(".config"))
.join("ipx/config.toml")
} }
/// `$IPX_DATA_DIR`, else `$XDG_DATA_HOME/ipx`. /// `$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") { if let Ok(p) = std::env::var("IPX_DATA_DIR") {
return PathBuf::from(p); return PathBuf::from(p);
} }
dirs::data_dir() xdg("XDG_DATA_HOME", ".local/share").join("ipx")
.unwrap_or_else(|| home().join(".local/share"))
.join("ipx")
} }
fn default_socket() -> PathBuf { fn default_socket() -> PathBuf {
@@ -346,8 +338,16 @@ pub fn unique_slug(text: &str, taken: &BTreeMap<String, Feed>) -> String {
(2..).map(|n| format!("{base}-{n}")).find(|s| !taken.contains_key(s)).unwrap() (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 { 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 { fn expand_tilde(p: &Path) -> PathBuf {
@@ -367,6 +367,8 @@ mod tests {
r#" r#"
[general] [general]
download_dir = "/tmp/pods" download_dir = "/tmp/pods"
# A key older versions read. An old config that still has it has to load.
interval_mins = 45
[feeds.example] [feeds.example]
url = "https://example.com/feed.xml" url = "https://example.com/feed.xml"
@@ -403,22 +405,17 @@ mod tests {
} }
#[test] #[test]
fn interval_falls_back_through_legacy_then_default() { fn interval_falls_back_to_an_hour() {
let mut g = General::default(); let mut g = General::default();
assert_eq!(g.interval(), 60, "the default schedule"); assert_eq!(g.interval(), 60, "the default schedule");
g.schedule = "every 15m".into(); g.schedule = "every 15m".into();
assert_eq!(g.interval(), 15); 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.schedule = String::new();
g.interval_mins = Some(45); assert_eq!(g.interval(), 60);
assert_eq!(g.interval(), 45);
// Garbage must not stop the daemon.
g.schedule = "whenever".into(); g.schedule = "whenever".into();
assert_eq!(g.interval(), 45);
g.interval_mins = None;
assert_eq!(g.interval(), 60); assert_eq!(g.interval(), 60);
} }

158
src/db.rs
View File

@@ -40,14 +40,10 @@ CREATE TABLE IF NOT EXISTS entries (
published INTEGER, published INTEGER,
description TEXT, description TEXT,
first_seen INTEGER NOT NULL, first_seen INTEGER NOT NULL,
read INTEGER NOT NULL DEFAULT 0,
flagged INTEGER NOT NULL DEFAULT 0,
image TEXT, image TEXT,
duration INTEGER, duration INTEGER,
episode INTEGER, episode INTEGER,
season 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) PRIMARY KEY (feed_id, guid)
); );
@@ -93,7 +89,7 @@ CREATE TABLE IF NOT EXISTS subscriptions (
PRIMARY KEY (user_id, feed_id) 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. -- an item nobody has touched has no row at all, which is what unread means.
CREATE TABLE IF NOT EXISTS entry_state ( CREATE TABLE IF NOT EXISTS entry_state (
user_id INTEGER NOT NULL REFERENCES users(id) ON DELETE CASCADE, user_id INTEGER NOT NULL REFERENCES users(id) ON DELETE CASCADE,
@@ -143,8 +139,8 @@ pub struct Managed {
pub orphaned: bool, pub orphaned: bool,
} }
/// Adds columns that later versions introduced. CREATE TABLE IF NOT EXISTS does nothing to /// Adds the columns later versions introduced and drops the ones they retired. CREATE TABLE IF
/// a table that already exists, so an installed database needs them added explicitly. /// NOT EXISTS does nothing to a table that already exists, so an installed database needs both.
fn migrate(conn: &Connection) -> Result<()> { fn migrate(conn: &Connection) -> Result<()> {
let wanted: &[(&str, &str, &str)] = &[ let wanted: &[(&str, &str, &str)] = &[
("feeds", "image", "TEXT"), ("feeds", "image", "TEXT"),
@@ -155,18 +151,29 @@ fn migrate(conn: &Connection) -> Result<()> {
("entries", "duration", "INTEGER"), ("entries", "duration", "INTEGER"),
("entries", "episode", "INTEGER"), ("entries", "episode", "INTEGER"),
("entries", "season", "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<bool> {
let mut stmt = conn.prepare(&format!("PRAGMA table_info({table})"))?; let mut stmt = conn.prepare(&format!("PRAGMA table_info({table})"))?;
let existing: Vec<String> = stmt let names = stmt
.query_map([], |r| r.get::<_, String>(1))? .query_map([], |r| r.get::<_, String>(1))?
.collect::<rusqlite::Result<Vec<_>>>()?; .collect::<rusqlite::Result<Vec<_>>>()?;
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"); tracing::info!(table, column, "adding column");
conn.execute_batch(&format!("ALTER TABLE {table} ADD COLUMN {column} {ty}"))?; 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(()) Ok(())
} }
@@ -345,22 +352,19 @@ impl Db {
/// Returns true when this entry had not been seen before. /// 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<bool> { pub fn record_entry(&self, feed_id: &str, e: &crate::feed::Entry) -> Result<bool> {
let conn = self.conn.lock().unwrap(); let conn = self.conn.lock().unwrap();
let inserted = conn.execute( let inserted = conn.execute(
"INSERT OR IGNORE INTO entries "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) 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![ rusqlite::params![
feed_id, e.guid, e.title, e.link, e.published, e.description, now(), feed_id, e.guid, e.title, e.link, e.published, e.description, now(),
e.image, e.duration, e.episode, e.season e.image, e.duration, e.episode, e.season
], ],
)?; )?;
if inserted == 0 { if inserted == 0 {
// The SET expressions see the pre-update row, so this compares old vs new.
conn.execute( conn.execute(
"UPDATE entries SET "UPDATE entries SET
title = coalesce(?3, title), title = coalesce(?3, title),
@@ -368,8 +372,7 @@ impl Db {
image = coalesce(?5, image), image = coalesce(?5, image),
duration = coalesce(?6, duration), duration = coalesce(?6, duration),
episode = coalesce(?7, episode), episode = coalesce(?7, episode),
season = coalesce(?8, season), season = coalesce(?8, season)
read = CASE WHEN description IS NOT ?4 OR title IS NOT ?3 THEN 0 ELSE read END
WHERE feed_id = ?1 AND guid = ?2", WHERE feed_id = ?1 AND guid = ?2",
rusqlite::params![ rusqlite::params![
feed_id, e.guid, e.title, e.description, feed_id, e.guid, e.title, e.description,
@@ -687,22 +690,9 @@ pub struct EncRow {
} }
impl Db { impl Db {
/// One page of a feed's entries, newest first, each with its enclosures attached. /// One page of entries, each with its enclosures attached: one feed's, or every feed the
/// `search` matches title and description, case-insensitively. /// person subscribes to when `feed_id` is None (All Subscriptions). `search` matches title
pub fn entries( /// and description, case-insensitively.
&self,
user_id: i64,
feed_id: &str,
filter: Filter,
search: Option<&str>,
offset: i64,
limit: i64,
) -> Result<Vec<EntryRow>> {
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.
pub fn entries_in( pub fn entries_in(
&self, &self,
user_id: i64, user_id: i64,
@@ -793,18 +783,7 @@ impl Db {
Ok(rows) Ok(rows)
} }
/// How many entries match, so the UI knows whether there is another page. /// How many entries `entries_in` would page through, so the UI knows whether there is more.
pub fn count_entries(
&self,
user_id: i64,
feed_id: &str,
filter: Filter,
search: Option<&str>,
) -> Result<i64> {
self.count_in(user_id, Some(feed_id), filter, search)
}
/// `count_entries` for one feed, or across every feed the person subscribes to.
pub fn count_in( pub fn count_in(
&self, &self,
user_id: i64, user_id: i64,
@@ -838,24 +817,16 @@ impl Db {
Ok(()) Ok(())
} }
/// Marks every entry in a feed read, for the "mark all read" button. /// The first admin starts subscribed to the whole catalogue: whoever wrote config.toml meant
/// Moves a single-user library onto an account: everything read, starred or part-played /// to read those feeds, and without this a fresh install signs in to an empty sidebar. Runs
/// becomes that person's, and they subscribe to every feed already in the catalogue. /// only while nobody subscribes to anything, so an unsubscribe is never undone.
/// Runs once -- the moment there is a first account and no subscriptions yet. pub fn adopt_catalogue(&self, user_id: i64, catalogue: &[String]) -> Result<usize> {
pub fn adopt_existing_library(&self, user_id: i64, catalogue: &[String]) -> Result<usize> {
let conn = self.conn.lock().unwrap(); let conn = self.conn.lock().unwrap();
let already: i64 = let already: i64 =
conn.query_row("SELECT count(*) FROM subscriptions", [], |r| r.get(0))?; conn.query_row("SELECT count(*) FROM subscriptions", [], |r| r.get(0))?;
if already > 0 { if already > 0 {
return Ok(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 { for id in catalogue {
conn.execute( conn.execute(
"INSERT OR IGNORE INTO subscriptions (user_id, feed_id, created) VALUES (?1, ?2, ?3)", "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", SELECT ?1, id, ?2 FROM feeds",
params![user_id, now()], params![user_id, now()],
)?; )?;
Ok(moved) Ok(catalogue.len())
} }
// ---- subscriptions ---- // ---- subscriptions ----
@@ -1528,8 +1499,9 @@ mod tests {
// Starring and position are just as private. // Starring and position are just as private.
db.set_entry_flag(1, "f", "b", EntryFlag::Flagged, true).unwrap(); db.set_entry_flag(1, "f", "b", EntryFlag::Flagged, true).unwrap();
db.set_position(2, "f", "b", 42).unwrap(); db.set_position(2, "f", "b", 42).unwrap();
let ray = db.entries(1, "f", Filter::All, None, 0, 50).unwrap(); let order = order_sql("published", "desc");
let sam = db.entries(2, "f", Filter::All, None, 0, 50).unwrap(); 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 ray_b = ray.iter().find(|e| e.guid == "b").unwrap();
let sam_b = sam.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!(ray_b.flagged && ray_b.position == 0);
@@ -1553,6 +1525,49 @@ mod tests {
assert_eq!(sum.downloaded, 0); 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<String> = conn
.prepare("PRAGMA table_info(entries)")
.unwrap()
.query_map([], |r| r.get(1))
.unwrap()
.collect::<rusqlite::Result<_>>()
.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] #[test]
fn every_filter_works_with_and_without_a_search_term() { fn every_filter_works_with_and_without_a_search_term() {
// Regression: the search clause used to be omitted when no term was given, while // 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] { for f in [Filter::All, Filter::Unread, Filter::Downloaded, Filter::Flagged] {
// Both paths must run without erroring, and agree with each other. // Both paths must run without erroring, and agree with each other.
let rows = db.entries(7, "f", f, None, 0, 50).unwrap(); let order = order_sql("published", "desc");
let n = db.count_entries(7, "f", f, None).unwrap(); 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"); 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 rows = db.entries_in(7, Some("f"), f, Some("dive"), 0, 50, &order).unwrap();
let n = db.count_entries(7, "f", f, Some("dive")).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!(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_in(7, Some("f"), Filter::All, None).unwrap(), 3);
assert_eq!(db.count_entries(7, "f", Filter::Unread, None).unwrap(), 1); assert_eq!(db.count_in(7, Some("f"), Filter::Unread, None).unwrap(), 1);
assert_eq!(db.count_entries(7, "f", Filter::Downloaded, None).unwrap(), 1); assert_eq!(db.count_in(7, Some("f"), Filter::Downloaded, None).unwrap(), 1);
assert_eq!(db.count_entries(7, "f", Filter::Flagged, None).unwrap(), 1); assert_eq!(db.count_in(7, Some("f"), Filter::Flagged, None).unwrap(), 1);
assert_eq!(db.count_entries(7, "f", Filter::All, Some("dive")).unwrap(), 2); assert_eq!(db.count_in(7, Some("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, Some("NOTES two")).unwrap(), 1,
"search is case-insensitive and covers the description"); "search is case-insensitive and covers the description");
} }

View File

@@ -223,7 +223,7 @@ enum Sniffed {
/// 2008 and so always answered 'data'. /// 2008 and so always answered 'data'.
async fn sniff(path: &Path) -> Result<Sniffed> { async fn sniff(path: &Path) -> Result<Sniffed> {
let head = read_head(path, 512).await?; 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); return Ok(Sniffed::Torrent);
} }
let text = String::from_utf8_lossy(&head); let text = String::from_utf8_lossy(&head);

View File

@@ -111,18 +111,11 @@ impl Visit for Collect {
fn record_debug(&mut self, field: &Field, value: &dyn std::fmt::Debug) { fn record_debug(&mut self, field: &Field, value: &dyn std::fmt::Debug) {
self.add(field, format!("{value:?}")); 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) { fn record_str(&mut self, field: &Field, value: &str) {
self.add(field, value.to_owned()); 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)] #[cfg(test)]

View File

@@ -334,8 +334,7 @@ async fn daemon(
anyhow::bail!("a daemon is already listening on {}", socket.display()); 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 // A database with nobody in it cannot be signed into.
// accounts still has to serve its owner. Both get the same starting point.
if ctx.db.users()?.is_empty() { if ctx.db.users()?.is_empty() {
ctx.db.create_user("admin", Some(&crate::auth::hash_password(DEFAULT_PASSWORD)?), true)?; ctx.db.create_user("admin", Some(&crate::auth::hash_password(DEFAULT_PASSWORD)?), true)?;
tracing::warn!( 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) { if let Some(admin) = ctx.db.users()?.into_iter().find(|u| u.is_admin) {
let catalogue: Vec<String> = ctx.cfg().feeds.keys().cloned().collect(); let catalogue: Vec<String> = 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(0) => {}
Ok(n) => tracing::info!(user = %admin.name, entries = n, "adopted 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 adopt the existing library"), 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() { match ctx.db.requeue_interrupted() {
Ok(n) if n > 0 => tracing::info!(count = n, "requeued downloads interrupted by a restart"), Ok(n) if n > 0 => tracing::info!(count = n, "requeued downloads interrupted by a restart"),
Ok(_) => {} Ok(_) => {}
@@ -468,7 +460,7 @@ async fn start_web(
let mut fresh = (*cfg).clone(); let mut fresh = (*cfg).clone();
fresh.web.enabled = true; fresh.web.enabled = true;
fresh.web.bind = bind.clone(); fresh.web.bind = bind.clone();
fresh.web.token = web::generate_token(); fresh.web.token = crate::auth::new_session_token();
fresh.save(config_path)?; fresh.save(config_path)?;
ctx.reload_cfg(config_path)?; ctx.reload_cfg(config_path)?;
println!("web ui token generated. Open:\n http://{bind}/?token={}", fresh.web.token); println!("web ui token generated. Open:\n http://{bind}/?token={}", fresh.web.token);
@@ -916,36 +908,6 @@ pub fn subscriptions(ctx: &Ctx) -> Result<Vec<Sub>> {
Ok(out) 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<usize> {
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. /// Seconds to wait before re-checking a feed.
/// ///
/// A per-feed schedule is an explicit instruction and wins outright. Without one, the /// A per-feed schedule is an explicit instruction and wins outright. Without one, the

View File

@@ -12,9 +12,7 @@ use axum::{
}, },
routing::{delete, get, patch, post}, routing::{delete, get, patch, post},
}; };
use futures_util::StreamExt;
use serde::Deserialize; use serde::Deserialize;
use tokio_stream::wrappers::BroadcastStream;
use tower::ServiceExt; use tower::ServiceExt;
use tower_http::services::ServeFile; use tower_http::services::ServeFile;
use serde::Serialize; use serde::Serialize;
@@ -35,28 +33,6 @@ pub struct WebState {
pub events: broadcast::Sender<Event>, pub events: broadcast::Sender<Event>,
} }
/// 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 { pub fn router(state: WebState) -> Router {
Router::new() Router::new()
.route("/", get(index)) .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. // Signing in cannot require being signed in, so these sit outside the auth layer.
.route("/login", get(login_page)) .route("/login", get(login_page))
.route("/api/login", post(login)) .route("/api/login", post(login))
.route("/icon.png", get(icon))
.layer(middleware::from_fn(access_log)) .layer(middleware::from_fn(access_log))
.with_state(state) .with_state(state)
} }
@@ -433,6 +410,15 @@ async fn login_page() -> Html<&'static str> {
Html(include_str!("../web/login.html")) 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 { fn constant_time_eq(a: &str, b: &str) -> bool {
let (a, b) = (a.as_bytes(), b.as_bytes()); let (a, b) = (a.as_bytes(), b.as_bytes());
if a.len() != b.len() { if a.len() != b.len() {
@@ -817,15 +803,6 @@ mod tests {
"another feed already has that URL" "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)] #[derive(Deserialize)]
@@ -1247,9 +1224,20 @@ async fn fetch_now(
/// The same broadcast the socket clients read, as server-sent events. /// The same broadcast the socket clients read, as server-sent events.
async fn events(State(state): State<WebState>) -> Sse<impl futures_util::Stream<Item = Result<SseEvent, std::convert::Infallible>>> { async fn events(State(state): State<WebState>) -> Sse<impl futures_util::Stream<Item = Result<SseEvent, std::convert::Infallible>>> {
let stream = BroadcastStream::new(state.events.subscribe()).filter_map(|ev| async move { // A client that falls behind skips what it missed rather than being cut off.
let ev = ev.ok()?; let stream = futures_util::stream::unfold(state.events.subscribe(), |mut rx| async move {
Some(Ok(SseEvent::default().data(serde_json::to_string(&ev).ok()?))) 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()) Sse::new(stream).keep_alive(axum::response::sse::KeepAlive::default())
} }
@@ -1460,8 +1448,6 @@ async fn patch_settings(
))); )));
} }
cfg.general.schedule = sched; 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 { if let Some(v) = body.max_new_per_check {
cfg.general.max_new_per_check = v; cfg.general.max_new_per_check = v;

View File

@@ -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 ctx = await browser.newContext();
const page = await ctx.newPage(); const page = await ctx.newPage();
await page.goto('/login'); 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('#name').fill('sam');
await page.locator('#pw').fill('sampassword'); await page.locator('#pw').fill('sampassword');
await page.locator('button[type=submit]').click(); await page.locator('button[type=submit]').click();

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long