diff --git a/CHANGELOG.md b/CHANGELOG.md index 35a4e27..d4a505b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - ipx can keep its data in Postgres: set `IPX_DATABASE_URL` to a `postgres://` URL. Without it, it is the SQLite `state.db` as before. `ipx copy-db ` moves an existing database across, everything in one go. +- The catalogue of feeds and the server settings the admin page edits are kept in the database + rather than config.toml, which keeps where things are, the torrent settings and who may sign + in. The first start takes them from config.toml and trims it, keeping the original as + `config.toml.pre-database`; feeds added to config.toml after that are ignored, with a warning. ### Changed diff --git a/CLAUDE.md b/CLAUDE.md index d8ca15b..06333f9 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -13,7 +13,7 @@ Arcane project `content`: `/mnt/fast/arcane/projects/content/compose.yaml`. That | | Host | In the container | |---|---|---| | Image | `192.168.1.130:5000/ipodderx:latest` | | -| Config | `/mnt/fast/appdata/ipodderx/config.toml` | `/config/config.toml` | +| Config | `/mnt/fast/appdata/ipodderx/config.toml`: bind address, token, trusted proxies, torrent, paths. The feeds and server settings are in the database | `/config/config.toml` | | Database | Postgres 18, database `ipodderx`, login `ipodderx`, on the `postgres` container of the Arcane project `databases` (`192.168.1.130:5433`). The URL is in `ipodderx.env` beside the compose file (`/mnt/fast/arcane/projects/content/ipodderx.env`, mode 600), passed to the container as `IPX_DATABASE_URL`. A relative `env_file`: Arcane runs compose in its own container, where `/mnt/fast/appdata` does not exist | | | Old database | `/mnt/user/ipodderx/state.db`, SQLite, used until the move to Postgres on 2026-09-18 and kept for rollback | `/data/state.db` | | Downloads | `/mnt/user/ipodderx/downloads` | `/downloads` | @@ -127,9 +127,17 @@ Non-trivial logic leaves one runnable check behind. Pure functions (`merge_polic * **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 they were dropped in 0.5. -* **The catalogue is config.toml; the subscriptions are in the database.** A feed exists once; +* **The catalogue and the server settings are in the database, not config.toml** (issue #18): + tables `catalogue` (each feed's `config::Feed` as JSON) and `settings` (`general`: + `config::Stored`). ipx still runs from one in-memory `Config`, config.toml for where things are + and who gets in, the database for the rest (`assemble_config`); a change goes through + `Ctx::store_cfg`, never a write to the file. The first start on a database without them imports + config.toml's and trims the file, keeping `config.toml.pre-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. + derived and never in the catalogue. +* **Postgres connections ask for no notices** (`client_min_messages=warning`, `db::url_for`). + Postgres sends one for every `CREATE ... IF NOT EXISTS` on something existing, sqlx logs each, + and tracing-subscriber's per-layer filters then dropped the next line ipx logged. * **One fetch serves everyone**, so scan policy is a union of subscribers' wants (`merge_policy`). Anyone wanting an item is enough to fetch it. * **The UI hiding a control is not enforcement.** Admin-only actions check `user.is_admin` in the diff --git a/docs/configuration.md b/docs/configuration.md index 7deb090..8b5a528 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -1,7 +1,19 @@ # Configuration -One TOML file, read at startup and re-read whenever the web UI writes to it — most changes take -effect without a restart. Default location `$XDG_CONFIG_HOME/ipx/config.toml` +Two places. **config.toml** holds what ipx needs before it reaches its database, and what decides +who gets in: where things are (`download_dir`, `socket`, `organize`), `[torrent]` and `[web]`. +**The database** holds the catalogue of feeds (`[feeds.]` below) and the server settings the +admin page edits (`schedule`, `max_total_gb`, `max_age_days`, `max_new_per_check`, +`media_types`). Change those in the web UI, or with `ipx add`, `ipx rm` and `ipx import`; they +take effect without a restart. + +The first time ipx meets a database that holds no catalogue, it takes the feeds and those +settings from config.toml, then rewrites config.toml without them, keeping the original beside it +as `config.toml.pre-database`. After that, feeds or those settings written into config.toml are +ignored, with a warning in the log saying so. The sections below describe them as they were +written in config.toml, which is still how a fresh install begins. + +config.toml's default location is `$XDG_CONFIG_HOME/ipx/config.toml` (`~/.config/ipx/config.toml`), overridden with `--config` or `$IPX_CONFIG`. | What | Where | Override | @@ -30,6 +42,9 @@ max_new_per_check = 3 # per feed, per scan. 0 = unlimited media_types = ["audio", "video"] ``` +`schedule`, `max_total_gb`, `max_age_days`, `max_new_per_check` and `media_types` move into the +database as described above; `download_dir`, `socket` and `organize` stay in config.toml. + * **`schedule`** — how often feeds are re-checked. A feed's own `` still wins when it asks to be polled *less* often, and a per-feed `schedule` overrides both. Admin-only from the UI. * **`organize`** — `feed` files downloads under the feed's folder; `date` under `YYYY-MM-DD`. @@ -90,8 +105,9 @@ itself carry a credential. Put TLS in front of it if that matters. ## `[feeds.]` -The table key is the feed id: stable, human-readable, and used in paths and the API. `ipx add` -derives it from the feed title. +Kept in the database once ipx has moved them in: a feed's settings are changed in the web UI, and +feeds come and go with `ipx add`, `ipx rm` and `ipx import`. The table key is the feed id: stable, +human-readable, and used in paths and the API. `ipx add` derives it from the feed title. ```toml [feeds.atp] @@ -109,8 +125,8 @@ With more than one account, **`keywords`, `auto_download`, `allow_explicit` and config.toml are the fallback for a feed nobody has claimed. The keys above describe the feed itself and are the same for everyone. See [users.md](users.md). -Feeds derived from a subscribed OPML are **not** written here: the OPML is the source of truth and -they are re-derived on every scan. Editing one in the UI promotes it to a real config entry. +Feeds derived from a subscribed OPML are **not** in the catalogue: the OPML is the source of truth +and they are re-derived on every scan. Editing one in the UI promotes it to a catalogue entry. ## Environment diff --git a/src/config.rs b/src/config.rs index 659d66b..b63967f 100644 --- a/src/config.rs +++ b/src/config.rs @@ -261,21 +261,86 @@ impl Config { Ok(cfg) } - pub fn save(&self, path: &Path) -> Result<()> { - if let Some(dir) = path.parent() { - std::fs::create_dir_all(dir) - .with_context(|| format!("creating {}", dir.display()))?; +} + +/// What the database keeps of the configuration (issue #18): the server settings the admin page +/// edits, and, beside them in `Db::stored_config`, the catalogue of feeds. The rest -- where +/// things are, who may sign in, the torrent session -- is needed before the database is reached, +/// or decides who gets in, and stays in config.toml. +#[derive(Debug, Clone, PartialEq, Deserialize, Serialize)] +pub struct Stored { + pub schedule: String, + pub max_total_gb: f64, + pub max_age_days: u64, + pub max_new_per_check: usize, + pub media_types: Vec, +} + +/// `[general]` keys that live in the database once it holds the configuration. +const STORED_KEYS: [&str; 5] = ["schedule", "max_total_gb", "max_age_days", "max_new_per_check", "media_types"]; + +impl Stored { + pub fn of(cfg: &Config) -> Self { + let g = &cfg.general; + Self { + schedule: g.schedule.clone(), + max_total_gb: g.max_total_gb, + max_age_days: g.max_age_days, + max_new_per_check: g.max_new_per_check, + media_types: g.media_types.clone(), } - let text = toml::to_string_pretty(self)?; - std::fs::write(path, text).with_context(|| format!("writing {}", path.display()))?; - // Passwords may live in here. - #[cfg(unix)] - { - use std::os::unix::fs::PermissionsExt; - std::fs::set_permissions(path, std::fs::Permissions::from_mode(0o600))?; - } - Ok(()) } + + pub fn apply(self, cfg: &mut Config) { + let g = &mut cfg.general; + g.schedule = self.schedule; + g.max_total_gb = self.max_total_gb; + g.max_age_days = self.max_age_days; + g.max_new_per_check = self.max_new_per_check; + g.media_types = self.media_types; + } +} + +impl Config { + /// config.toml as it is kept once the database holds the feeds and server settings: the same + /// file without `[feeds]` or the `[general]` keys in `Stored`. + pub fn save_bootstrap(&self, path: &Path) -> Result<()> { + let mut v = toml::Value::try_from(self)?; + if let Some(t) = v.as_table_mut() { + t.remove("feeds"); + if let Some(g) = t.get_mut("general").and_then(|g| g.as_table_mut()) { + for k in STORED_KEYS { + g.remove(k); + } + } + } + write_private(path, &toml::to_string_pretty(&v)?) + } + + /// Whether config.toml still lists feeds or server settings, which the database now holds: + /// an edit there would otherwise go unnoticed. + pub fn file_holds_stored(path: &Path) -> bool { + let Ok(text) = std::fs::read_to_string(path) else { return false }; + let Ok(v) = text.parse::() else { return false }; + v.get("feeds").and_then(|f| f.as_table()).is_some_and(|f| !f.is_empty()) + || v.get("general") + .and_then(|g| g.as_table()) + .is_some_and(|g| STORED_KEYS.iter().any(|k| g.contains_key(*k))) + } +} + +/// Writes a config file readable by its owner alone: feed passwords have lived in it. +fn write_private(path: &Path, text: &str) -> Result<()> { + if let Some(dir) = path.parent() { + std::fs::create_dir_all(dir).with_context(|| format!("creating {}", dir.display()))?; + } + std::fs::write(path, text).with_context(|| format!("writing {}", path.display()))?; + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + std::fs::set_permissions(path, std::fs::Permissions::from_mode(0o600))?; + } + Ok(()) } /// `$IPX_CONFIG`, else `$XDG_CONFIG_HOME/ipx/config.toml`. @@ -370,6 +435,36 @@ fn expand_tilde(p: &Path) -> PathBuf { mod tests { use super::*; + #[test] + fn the_file_kept_beside_the_database_has_no_feeds_or_server_settings() { + let cfg: Config = toml::from_str( + r#" + [general] + download_dir = "/downloads" + schedule = "every 2h" + max_new_per_check = 7 + media_types = ["audio"] + [web] + bind = "0.0.0.0:8099" + token = "t" + [feeds.show] + url = "http://x/show.xml" + "#, + ) + .unwrap(); + let path = std::env::temp_dir().join(format!("ipx-bootstrap-{}.toml", std::process::id())); + std::fs::write(&path, toml::to_string(&cfg).unwrap()).unwrap(); + assert!(Config::file_holds_stored(&path), "a whole config.toml holds them"); + cfg.save_bootstrap(&path).unwrap(); + let text = std::fs::read_to_string(&path).unwrap(); + assert!(!Config::file_holds_stored(&path), "{text}"); + let back: Config = toml::from_str(&text).unwrap(); + assert!(back.feeds.is_empty()); + assert_eq!(back.web.token, "t", "who may sign in stays in the file"); + assert_eq!(back.general.download_dir, PathBuf::from("/downloads"), "where things are, too"); + std::fs::remove_file(&path).unwrap(); + } + #[test] fn parses_a_config_and_applies_defaults() { let cfg: Config = toml::from_str( diff --git a/src/db.rs b/src/db.rs index ac692d4..fb40291 100644 --- a/src/db.rs +++ b/src/db.rs @@ -2,7 +2,7 @@ //! per-feed .ipxd plists, history.dat and qmcache.dat. use anyhow::{Context, Result}; -use crate::entity::{enclosures, entries, feeds, sessions, subscriptions, users}; +use crate::entity::{catalogue, enclosures, entries, feeds, sessions, settings, subscriptions, users}; use sea_orm::sea_query::{Expr, Func}; use sea_orm::{ ActiveModelTrait, ColumnTrait, ConnectionTrait, EntityTrait, PaginatorTrait, QueryFilter, QueryOrder, Set, @@ -55,6 +55,8 @@ async fn create_missing(orm: &sea_orm::DatabaseConnection) -> Result<()> { schema.create_table_from_entity(subscriptions::Entity), schema.create_table_from_entity(entry_state::Entity), schema.create_table_from_entity(sessions::Entity), + schema.create_table_from_entity(catalogue::Entity), + schema.create_table_from_entity(settings::Entity), ] { orm.execute(table.if_not_exists()).await.context("creating the schema")?; } @@ -100,6 +102,9 @@ pub fn location() -> String { .unwrap_or_else(|| crate::config::data_dir().join("state.db").display().to_string()) } +/// The Postgres connection option that keeps notices from being sent at all; see `connect`. +const QUIET: &str = "options=-c%20client_min_messages%3Dwarning"; + /// A URL fit for a log or an error: the password taken out. fn redact(url: &str) -> String { match (url.find("://"), url.rfind('@')) { @@ -118,15 +123,30 @@ fn is_postgres(location: &str) -> bool { /// sqlx's defaults for SQLite are what ipx wants: foreign keys on, and a five-second wait for a /// lock, which is what rusqlite was set to. async fn connect(location: &str) -> Result { - let url = - if is_postgres(location) { location.to_owned() } else { format!("sqlite://{location}?mode=rwc") }; - let mut opts = sea_orm::ConnectOptions::new(url); + let mut opts = sea_orm::ConnectOptions::new(url_for(location)); opts.sqlx_logging(false); sea_orm::Database::connect(opts) .await .with_context(|| format!("opening {}", redact(location))) } +/// The URL `connect` hands sqlx for a location: a SQLite file, created if missing, or a Postgres +/// URL with notices turned off. +fn url_for(location: &str) -> String { + if !is_postgres(location) { + format!("sqlite://{location}?mode=rwc") + } else if location.contains("options=") { + location.to_owned() // someone chose their own; theirs stands + } else { + // Warnings and up only. Postgres answers every CREATE ... IF NOT EXISTS on an existing + // table with a notice, eleven on each open; sqlx logs each, no filter here wants them, + // and tracing-subscriber's per-layer filters then swallowed the next line ipx logged + // (0.3.23: "moved the feeds ... into the database" went missing that way). + let sep = if location.contains('?') { '&' } else { '?' }; + format!("{location}{sep}{QUIET}") + } +} + /// One person's wants for one feed. `None` in a field means the feed's own setting stands. #[derive(Debug, Clone, Default)] @@ -280,7 +300,9 @@ impl Db { admin.execute_unprepared(&format!("CREATE SCHEMA {schema}")).await?; // Every connection in the pool starts in it, so each test sees only its own tables. let sep = if url.contains('?') { '&' } else { '?' }; - let orm = connect(&format!("{url}{sep}options=-c%20search_path%3D{schema}")).await?; + let orm = + connect(&format!("{url}{sep}options=-c%20search_path%3D{schema}%20-c%20client_min_messages%3Dwarning")) + .await?; create_missing(&orm).await?; return Ok(Self { orm, tmp: None }); } @@ -963,6 +985,76 @@ impl Db { Ok(()) } + // ---- the configuration: the catalogue and the server settings ---- + + /// The feeds and server settings, once the database holds them; None before, when they are + /// still config.toml's. + pub async fn stored_config( + &self, + ) -> Result)>> { + let Some(general) = settings::Entity::find_by_id("general".to_owned()).one(&self.orm).await? else { + return Ok(None); + }; + let stored = serde_json::from_str(&general.value).context("reading the stored server settings")?; + let mut feeds = std::collections::BTreeMap::new(); + for row in catalogue::Entity::find().all(&self.orm).await? { + let feed = serde_json::from_str(&row.spec).with_context(|| format!("reading feed {}", row.id))?; + feeds.insert(row.id, feed); + } + Ok(Some((stored, feeds))) + } + + /// Takes the feeds and server settings from `cfg` (config.toml, as read) into a database that + /// has none. False when it already had them: another process got there first, and its copy + /// stands. One transaction, and the settings row goes in first, so two processes starting at + /// once cannot both import. + pub async fn import_config(&self, cfg: &crate::config::Config) -> Result { + use sea_orm::TransactionTrait; + let backend = self.orm.get_database_backend(); + let tx = self.orm.begin().await?; + let claimed = tx + .execute_raw(Statement::from_sql_and_values( + backend, + "INSERT INTO settings (name, value) VALUES ('general', $1) ON CONFLICT DO NOTHING", + vec![serde_json::to_string(&crate::config::Stored::of(cfg))?.into()], + )) + .await? + .rows_affected(); + if claimed == 0 { + return Ok(false); // dropped, so rolled back + } + for (id, feed) in &cfg.feeds { + catalogue::ActiveModel { id: Set(id.clone()), spec: Set(serde_json::to_string(feed)?) } + .insert(&tx) + .await?; + } + tx.commit().await?; + Ok(true) + } + + /// Writes the feeds and server settings as they now stand: what config.toml's save did. + /// The whole catalogue at once, in one transaction, so a feed removed goes too. + pub async fn store_config(&self, cfg: &crate::config::Config) -> Result<()> { + use sea_orm::TransactionTrait; + let backend = self.orm.get_database_backend(); + let tx = self.orm.begin().await?; + tx.execute_raw(Statement::from_sql_and_values( + backend, + "INSERT INTO settings (name, value) VALUES ('general', $1) + ON CONFLICT (name) DO UPDATE SET value = excluded.value", + vec![serde_json::to_string(&crate::config::Stored::of(cfg))?.into()], + )) + .await?; + catalogue::Entity::delete_many().exec(&tx).await?; + for (id, feed) in &cfg.feeds { + catalogue::ActiveModel { id: Set(id.clone()), spec: Set(serde_json::to_string(feed)?) } + .insert(&tx) + .await?; + } + tx.commit().await?; + Ok(()) + } + // ---- moving to another database ---- /// Copies every row of `from` into this database, which must be empty: the move from the @@ -985,6 +1077,8 @@ impl Db { ("subscriptions", copy_table::(&from.orm, &tx).await?), ("entry_state", copy_table::(&from.orm, &tx).await?), ("sessions", copy_table::(&from.orm, &tx).await?), + ("catalogue", copy_table::(&from.orm, &tx).await?), + ("settings", copy_table::(&from.orm, &tx).await?), ]; if self.orm.get_database_backend() == sea_orm::DbBackend::Postgres { // The copied ids came with the rows; the counters that hand out new ones start past @@ -2077,6 +2171,51 @@ mod tests { assert!(db.pinned_feeds(me).await.unwrap().is_empty()); } + #[test] + fn postgres_is_asked_for_no_notices_and_a_password_never_reaches_a_log() { + assert_eq!(url_for("postgres://u:p@h/d"), format!("postgres://u:p@h/d?{QUIET}")); + assert_eq!(url_for("postgres://u:p@h/d?sslmode=disable"), format!("postgres://u:p@h/d?sslmode=disable&{QUIET}")); + assert_eq!(url_for("postgres://u:p@h/d?options=-c%20x%3Dy"), "postgres://u:p@h/d?options=-c%20x%3Dy", "theirs stands"); + assert_eq!(url_for("/data/state.db"), "sqlite:///data/state.db?mode=rwc"); + assert_eq!(redact("postgres://ipodderx:s3cret@h:5433/ipodderx"), "postgres://ipodderx:***@h:5433/ipodderx"); + } + + #[tokio::test] + async fn the_configuration_goes_in_once_and_comes_back_as_it_went() { + let db = Db::memory().await.unwrap(); + assert!(db.stored_config().await.unwrap().is_none(), "nothing until it is taken in"); + let mut cfg: crate::config::Config = toml::from_str( + r#" + [general] + schedule = "every 2h" + max_total_gb = 1.5 + [feeds.a] + url = "http://x/a.xml" + keywords = ["one", "two"] + [feeds.b] + url = "http://x/b.xml" + password = "secret" + "#, + ) + .unwrap(); + assert!(db.import_config(&cfg).await.unwrap()); + assert!(!db.import_config(&cfg).await.unwrap(), "a second import finds the first and stands back"); + let (stored, feeds) = db.stored_config().await.unwrap().unwrap(); + assert_eq!(stored, crate::config::Stored::of(&cfg)); + assert_eq!(feeds.keys().collect::>(), ["a", "b"]); + assert_eq!(feeds["a"].keywords, ["one", "two"]); + assert_eq!(feeds["b"].password.as_deref(), Some("secret")); + + // Storing is the whole catalogue: a feed taken out goes, one put in arrives. + cfg.feeds.remove("a"); + cfg.feeds.insert("c".into(), cfg.feeds["b"].clone()); + cfg.general.schedule = "every 30m".into(); + db.store_config(&cfg).await.unwrap(); + let (stored, feeds) = db.stored_config().await.unwrap().unwrap(); + assert_eq!(stored.schedule, "every 30m"); + assert_eq!(feeds.keys().collect::>(), ["b", "c"]); + } + #[tokio::test] async fn enclosure_url_is_the_dedupe_key() { let db = Db::memory().await.unwrap(); diff --git a/src/entity.rs b/src/entity.rs index a8ff7bb..409307d 100644 --- a/src/entity.rs +++ b/src/entity.rs @@ -242,3 +242,44 @@ pub mod sessions { owned_by_user!(); } + +/// The catalogue: every feed configured, with its shared settings as `config::Feed` in JSON, so a +/// new setting on a feed needs no new column. It was config.toml's `[feeds]` (issue #18). +pub mod catalogue { + use sea_orm::entity::prelude::*; + + #[derive(Clone, Debug, PartialEq, Eq, DeriveEntityModel)] + #[sea_orm(table_name = "catalogue")] + pub struct Model { + #[sea_orm(primary_key, auto_increment = false, column_type = "Text")] + pub id: String, + #[sea_orm(column_type = "Text")] + pub spec: String, + } + + #[derive(Copy, Clone, Debug, EnumIter, DeriveRelation)] + pub enum Relation {} + + impl ActiveModelBehavior for ActiveModel {} +} + +/// The server's settings, by name, each a JSON value. `general` is `config::Stored`: what was in +/// config.toml's `[general]` and the admin page edits. Its row being there is what says the +/// configuration has moved in (issue #18). +pub mod settings { + use sea_orm::entity::prelude::*; + + #[derive(Clone, Debug, PartialEq, Eq, DeriveEntityModel)] + #[sea_orm(table_name = "settings")] + pub struct Model { + #[sea_orm(primary_key, auto_increment = false, column_type = "Text")] + pub name: String, + #[sea_orm(column_type = "Text")] + pub value: String, + } + + #[derive(Copy, Clone, Debug, EnumIter, DeriveRelation)] + pub enum Relation {} + + impl ActiveModelBehavior for ActiveModel {} +} diff --git a/src/main.rs b/src/main.rs index 9bcbd12..cdb9196 100644 --- a/src/main.rs +++ b/src/main.rs @@ -144,13 +144,19 @@ impl Ctx { } /// Re-reads config.toml into the live snapshot. - pub fn reload_cfg(&self, path: &std::path::Path) -> Result<()> { - let fresh = config::Config::load(path)?; - *self.cfg.write().unwrap() = std::sync::Arc::new(fresh); - tracing::info!("config reloaded"); + /// Keeps a changed catalogue or server settings: in the database, and for everything running + /// here from now on. What config.toml's save and reload did, before the database held them. + pub async fn store_cfg(&self, cfg: config::Config) -> Result<()> { + self.db.store_config(&cfg).await?; + self.set_cfg(cfg); Ok(()) } + fn set_cfg(&self, cfg: config::Config) { + *self.cfg.write().unwrap() = std::sync::Arc::new(cfg); + tracing::info!("config reloaded"); + } + async fn torrents(&self) -> Result<&torrent::Torrents> { let cfg = self.cfg(); self.torrents @@ -211,6 +217,14 @@ async fn main() -> Result<()> { return ipc::proxy(&cfg.general.socket, cmd).await; } + // copy-db fills an empty database from another, configuration included; taking config.toml + // into it first would have the copy collide with it. + let cfg = if matches!(cli.command, Command::CopyDb { .. }) { + cfg + } else { + assemble_config(&db, cfg, &config_path).await? + }; + let is_daemon = matches!(cli.command, Command::Daemon { .. }); let (events, _) = broadcast::channel(1024); let ctx = Arc::new(Ctx { @@ -227,14 +241,14 @@ async fn main() -> Result<()> { }); match cli.command { - Command::List => list(&ctx, &config_path).await, + Command::List => list(&ctx).await, Command::Daemon { web } => daemon(ctx, config_path, web, events).await, Command::Add { url, folder, keywords } => { - add(&ctx, &config_path, &url, folder, keywords).await + add(&ctx, &url, folder, keywords).await } - Command::Rm { feed } => rm(&ctx, &config_path, &feed).await, + Command::Rm { feed } => rm(&ctx, &feed).await, Command::User { cmd } => user_cmd(&ctx, cmd).await, - Command::Import { file } => import(&ctx, &config_path, &file).await, + Command::Import { file } => import(&ctx, &file).await, Command::Export { file } => export(&ctx, &file).await, Command::CopyDb { from } => copy_db(&ctx, &from).await, _ => run(&ctx, wire_cmd.expect("only List and Daemon have no wire form")).await, @@ -533,8 +547,9 @@ async fn start_web( fresh.web.enabled = true; fresh.web.bind = bind.clone(); fresh.web.token = crate::auth::new_session_token(); - fresh.save(config_path)?; - ctx.reload_cfg(config_path)?; + // The token is config.toml's, not the database's: it decides who gets in. + fresh.save_bootstrap(config_path)?; + ctx.set_cfg(fresh.clone()); println!("web ui token generated. Open:\n http://{bind}/?token={}", fresh.web.token); } else { println!( @@ -549,7 +564,6 @@ async fn start_web( let state = web::WebState { ctx: ctx.clone(), - config_path: config_path.to_path_buf(), cmds: cmds.clone(), events: events.clone(), }; @@ -575,7 +589,6 @@ async fn shutdown() { /// Subscribes to one feed, naming it from its own title. async fn add( ctx: &Ctx, - config_path: &std::path::Path, url: &str, folder: Option, keywords: Vec, @@ -587,7 +600,7 @@ async fn add( anyhow::bail!("already subscribed as {:?}", existing.id); } let id = add_one(ctx, &mut cfg, url, folder, keywords).await?; - cfg.save(config_path)?; + ctx.store_cfg(cfg).await?; println!("added {id}"); Ok(()) } @@ -659,7 +672,7 @@ fn url_stem(url: &str) -> String { .unwrap_or_else(|| url.to_owned()) } -async fn rm(ctx: &Ctx, config_path: &std::path::Path, feed: &str) -> Result<()> { +async fn rm(ctx: &Ctx, feed: &str) -> Result<()> { let mut cfg = (*ctx.cfg()).clone(); if cfg.feeds.remove(feed).is_none() { // Derived from an OPML: drop it here, though the subscription will list it again @@ -668,14 +681,14 @@ async fn rm(ctx: &Ctx, config_path: &std::path::Path, feed: &str) -> Result<()> println!("removed {feed}; it came from an OPML subscription and may return on the next read"); return Ok(()); } - cfg.save(config_path)?; + ctx.store_cfg(cfg).await?; // State and files stay: re-adding the feed should not re-download its back catalogue. println!("removed {feed}; downloads and history kept"); retire_group(ctx, feed).await?; Ok(()) } -async fn import(ctx: &Ctx, config_path: &std::path::Path, file: &std::path::Path) -> Result<()> { +async fn import(ctx: &Ctx, file: &std::path::Path) -> Result<()> { let text = std::fs::read_to_string(file) .with_context(|| format!("reading {}", file.display()))?; // The CLI speaks for the operator, as the shared web token does. @@ -687,7 +700,7 @@ async fn import(ctx: &Ctx, config_path: &std::path::Path, file: &std::path::Path .ok_or_else(|| anyhow::anyhow!("no admin account to subscribe: ipx user add --admin"))?; let doc = opml::OPML::from_str(&text) .map_err(|e| anyhow::anyhow!("{} is not OPML: {e}", file.display()))?; - let (added, had) = subscribe_opml(ctx, config_path, &doc, admin.id).await?; + let (added, had) = subscribe_opml(ctx, &doc, admin.id).await?; println!("subscribed {} to {added} feed(s); {had} already there", admin.name); Ok(()) } @@ -704,7 +717,6 @@ async fn import(ctx: &Ctx, config_path: &std::path::Path, file: &std::path::Path /// before anything is touched: a 400 from the web, a message from the CLI. pub async fn subscribe_opml( ctx: &Ctx, - config_path: &std::path::Path, doc: &opml::OPML, user_id: i64, ) -> Result<(usize, usize)> { @@ -751,8 +763,7 @@ pub async fn subscribe_opml( ids.push(id); } if grew { - cfg.save(config_path)?; - ctx.reload_cfg(config_path)?; + ctx.store_cfg(cfg).await?; } let (mut added, mut had) = (0, 0); @@ -778,6 +789,40 @@ pub fn collect_outlines(outlines: &[opml::Outline], out: &mut Vec<(String, Strin } } +/// The configuration ipx runs with: config.toml for where things are and who may sign in, the +/// database for the feeds and the server settings (issue #18). The first time a database holds +/// neither, it takes them from config.toml, which is then cut down to the rest, the original kept +/// beside it as config.toml.pre-database. +async fn assemble_config(db: &db::Db, mut cfg: config::Config, path: &std::path::Path) -> Result { + for _ in 0..2 { + if let Some((stored, feeds)) = db.stored_config().await? { + if config::Config::file_holds_stored(path) { + tracing::warn!( + "config.toml still lists feeds or server settings; they are ignored, since the \ + database holds them now. Change them in the web UI, or with ipx add and rm." + ); + } + stored.apply(&mut cfg); + cfg.feeds = feeds; + return Ok(cfg); + } + if db.import_config(&cfg).await? { + if path.exists() { + let original = path.with_extension("toml.pre-database"); + if !original.exists() { + std::fs::copy(path, &original) + .with_context(|| format!("keeping the original as {}", original.display()))?; + } + cfg.save_bootstrap(path)?; + } + tracing::info!(feeds = cfg.feeds.len(), "moved the feeds and server settings from config.toml into the database"); + return Ok(cfg); + } + // Another ipx imported between our look and our insert; the next pass reads its copy. + } + anyhow::bail!("the database says it holds the configuration and then that it does not") +} + async fn copy_db(ctx: &Ctx, from: &std::path::Path) -> Result<()> { anyhow::ensure!(from.exists(), "{} does not exist", from.display()); let source = db::Db::open(&from.display().to_string()).await?; @@ -808,10 +853,10 @@ async fn export(ctx: &Ctx, file: &std::path::Path) -> Result<()> { Ok(()) } -async fn list(ctx: &Ctx, config_path: &std::path::Path) -> Result<()> { +async fn list(ctx: &Ctx) -> Result<()> { let cfg = ctx.cfg(); if cfg.feeds.is_empty() { - println!("No feeds configured in {}", config_path.display()); + println!("No feeds configured. Add one with `ipx add `, or in the web UI."); return Ok(()); } for (id, feed) in &cfg.feeds { @@ -1670,6 +1715,32 @@ fn duration(secs: u64) -> String { mod tests { use super::*; + #[tokio::test] + async fn the_first_start_moves_the_configuration_in_and_trims_the_file() { + let dir = std::env::temp_dir().join(format!("ipx-assemble-{}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + let path = dir.join("config.toml"); + std::fs::write( + &path, + "[general]\nschedule = \"every 2h\"\n[web]\ntoken = \"t\"\n[feeds.show]\nurl = \"http://x/show.xml\"\n", + ) + .unwrap(); + let db = db::Db::memory().await.unwrap(); + + let first = assemble_config(&db, config::Config::load(&path).unwrap(), &path).await.unwrap(); + assert_eq!(first.feeds.keys().collect::>(), ["show"]); + assert_eq!(first.general.schedule, "every 2h"); + assert!(dir.join("config.toml.pre-database").exists(), "the original is kept"); + assert!(!config::Config::file_holds_stored(&path), "and the file no longer lists them"); + + // The next start reads them from the database, the trimmed file notwithstanding. + let next = assemble_config(&db, config::Config::load(&path).unwrap(), &path).await.unwrap(); + assert_eq!(next.feeds.keys().collect::>(), ["show"]); + assert_eq!(next.general.schedule, "every 2h"); + assert_eq!(next.web.token, "t"); + std::fs::remove_dir_all(&dir).unwrap(); + } + fn feed() -> config::Feed { // Whatever `ipx add` would write, which is the shape every code path sees. let mut cfg = config::Config::default(); diff --git a/src/web.rs b/src/web.rs index a8928b2..3381921 100644 --- a/src/web.rs +++ b/src/web.rs @@ -16,7 +16,6 @@ use serde::Deserialize; use tower::ServiceExt; use tower_http::services::ServeFile; use serde::Serialize; -use std::path::PathBuf; use std::sync::Arc; use tokio::sync::{broadcast, mpsc}; @@ -28,7 +27,6 @@ const COOKIE: &str = "ipx_token"; #[derive(Clone)] pub struct WebState { pub ctx: Arc, - pub config_path: PathBuf, pub cmds: mpsc::Sender, pub events: broadcast::Sender, } @@ -1229,8 +1227,7 @@ async fn add_feed( )); } let id = crate::add_one(&state.ctx, &mut cfg, &url, body.folder, body.keywords).await?; - cfg.save(&state.config_path)?; - state.ctx.reload_cfg(&state.config_path)?; + state.ctx.store_cfg(cfg).await?; state.ctx.db.subscribe(user.id, &id).await?; explicit_on_add(&state, user.id, &id, body.allow_explicit).await?; scan_soon(&state, Some(id.clone())).await; @@ -1378,8 +1375,7 @@ async fn patch_feed( if let Some(v) = body.category { feed.category = v.map(|s| s.trim().to_owned()).filter(|s| !s.is_empty()); } - cfg.save(&state.config_path)?; - state.ctx.reload_cfg(&state.config_path)?; + state.ctx.store_cfg(cfg).await?; if url_changed { // Refreshing a rotated auth token is the common case; entries and download history // are keyed by feed id, so they survive the change. @@ -1415,8 +1411,7 @@ async fn remove_feed( state.ctx.db.drop_managed(&id).await?; return Ok(StatusCode::NO_CONTENT); } - cfg.save(&state.config_path)?; - state.ctx.reload_cfg(&state.config_path)?; + state.ctx.store_cfg(cfg).await?; crate::retire_group(&state.ctx, &id).await?; Ok(StatusCode::NO_CONTENT) } @@ -1713,7 +1708,7 @@ async fn import_opml( // file arrives as text, is read here, and is gone when the request ends. let doc = opml::OPML::from_str(&body.xml) .map_err(|e| ApiError::bad_request(format!("that is not an OPML file: {e}")))?; - let (added, already) = crate::subscribe_opml(&state.ctx, &state.config_path, &doc, user.id).await?; + let (added, already) = crate::subscribe_opml(&state.ctx, &doc, user.id).await?; if added > 0 { scan_soon(&state, None).await; } @@ -1787,8 +1782,7 @@ async fn patch_settings( if let Some(v) = body.max_age_days { cfg.general.max_age_days = v; } - cfg.save(&state.config_path)?; - state.ctx.reload_cfg(&state.config_path)?; + state.ctx.store_cfg(cfg).await?; Ok(StatusCode::NO_CONTENT) }