diff --git a/src/api.rs b/src/api.rs index 3e4f280..9eed127 100644 --- a/src/api.rs +++ b/src/api.rs @@ -13,7 +13,7 @@ use crate::engine::{ EngineResponse, }; use crate::alerts::{deliver_ntfy, AlertError}; -use crate::profile::{self, Profile, ProfileError, ProfileStore}; +use crate::profile::{self, Dashboard, Profile, ProfileError}; use crate::settings::{CssError, SettingsHandle}; use axum::extract::{Path as AxumPath, Query, State}; use axum::http::{header, StatusCode}; @@ -43,11 +43,11 @@ impl ApiState { /// Best-effort: a no-op if no profile is active, and I/O failures are only logged, since the engine declaration is already the source of truth. fn persist_param_declaration(&self, request: &DeclareParamRequest) { - let Some((store_path, profile_name)) = self.active_profile_location() else { + let mut settings = self.settings.lock().unwrap(); + let Some(profile_name) = settings.active_profile.clone() else { return; }; - let mut store = ProfileStore::load(&store_path); - if let Err(err) = store.record_param_declaration(&store_path, &profile_name, request) { + if let Err(err) = settings.record_param_declaration(&self.settings_path, &profile_name, request) { eprintln!( "warning: failed to persist param declaration '{}' into profile '{profile_name}': {err}", request.name @@ -57,11 +57,11 @@ impl ApiState { /// Same as `persist_param_declaration`, for metric declarations. fn persist_metric_declaration(&self, request: &DeclareMetricRequest) { - let Some((store_path, profile_name)) = self.active_profile_location() else { + let mut settings = self.settings.lock().unwrap(); + let Some(profile_name) = settings.active_profile.clone() else { return; }; - let mut store = ProfileStore::load(&store_path); - if let Err(err) = store.record_metric_declaration(&store_path, &profile_name, request) { + if let Err(err) = settings.record_metric_declaration(&self.settings_path, &profile_name, request) { eprintln!( "warning: failed to persist metric declaration '{}' into profile '{profile_name}': {err}", request.name @@ -71,26 +71,22 @@ impl ApiState { /// Same as `persist_metric_declaration`, for an in-place edit (`PUT /api/metrics/{name}`) — also drops the persisted record under `old_name` when the edit renames the metric, so a rename doesn't leave a stale duplicate declaration in the profile file. fn persist_metric_declaration_update(&self, old_name: &str, request: &DeclareMetricRequest) { - let Some((store_path, profile_name)) = self.active_profile_location() else { + let mut settings = self.settings.lock().unwrap(); + let Some(profile_name) = settings.active_profile.clone() else { return; }; - let mut store = ProfileStore::load(&store_path); - if let Err(err) = - store.record_metric_declaration_update(&store_path, &profile_name, old_name, request) - { + if let Err(err) = settings.record_metric_declaration_update( + &self.settings_path, + &profile_name, + old_name, + request, + ) { eprintln!( "warning: failed to persist metric declaration update '{}' into profile '{profile_name}': {err}", request.name ); } } - - fn active_profile_location(&self) -> Option<(PathBuf, String)> { - let settings = self.settings.lock().unwrap(); - let profile_name = settings.profiles.active_profile.clone()?; - let profiles_dir = profile::resolve_profiles_dir(&settings, &self.settings_path); - Some((profile::profiles_store_path(&profiles_dir), profile_name)) - } } pub fn router(engine: EngineHandle, settings: SettingsHandle, settings_path: PathBuf) -> Router { @@ -560,9 +556,7 @@ fn profile_error_response(err: ProfileError) -> axum::response::Response { async fn list_profiles(State(state): State) -> impl IntoResponse { let settings = state.settings.lock().unwrap(); - let profiles_dir = profile::resolve_profiles_dir(&settings, &state.settings_path); - let store = ProfileStore::load(&profile::profiles_store_path(&profiles_dir)); - let summaries: Vec = store.list().iter().map(ProfileSummaryDto::from).collect(); + let summaries: Vec = settings.profiles.iter().map(ProfileSummaryDto::from).collect(); Json(summaries).into_response() } @@ -571,12 +565,9 @@ async fn create_profile( Json(body): Json, ) -> impl IntoResponse { let mut settings = state.settings.lock().unwrap(); - let profiles_dir = profile::resolve_profiles_dir(&settings, &state.settings_path); - let store_path = profile::profiles_store_path(&profiles_dir); - let mut store = ProfileStore::load(&store_path); - match store.create(&store_path, &mut settings, &state.settings_path, body.name.clone()) { + match settings.create_profile(&state.settings_path, body.name.clone()) { Ok(()) => { - let profile = store.get(&body.name).expect("profile was just created by store.create"); + let profile = settings.profile(&body.name).expect("profile was just created by create_profile"); (StatusCode::CREATED, Json(ProfileSummaryDto::from(profile))).into_response() } Err(err) => profile_error_response(err), @@ -589,10 +580,7 @@ async fn delete_profile( AxumPath(name): AxumPath, ) -> impl IntoResponse { let mut settings = state.settings.lock().unwrap(); - let profiles_dir = profile::resolve_profiles_dir(&settings, &state.settings_path); - let store_path = profile::profiles_store_path(&profiles_dir); - let mut store = ProfileStore::load(&store_path); - match store.delete(&store_path, &mut settings, &state.settings_path, &name) { + match settings.delete_profile(&state.settings_path, &name) { Ok(()) => StatusCode::NO_CONTENT.into_response(), Err(err) => profile_error_response(err), } @@ -637,11 +625,8 @@ async fn activate_profile( ) -> impl IntoResponse { let activated_profile = { let mut settings = state.settings.lock().unwrap(); - let profiles_dir = profile::resolve_profiles_dir(&settings, &state.settings_path); - let store_path = profile::profiles_store_path(&profiles_dir); - let store = ProfileStore::load(&store_path); - match store.set_active(&mut settings, &state.settings_path, Some(name.clone())) { - Ok(()) => Ok(store.get(&name).cloned()), + match settings.set_active_profile(&state.settings_path, Some(name.clone())) { + Ok(()) => Ok(settings.profile(&name).cloned()), Err(err) => Err(err), } }; @@ -663,9 +648,7 @@ async fn list_dashboards( AxumPath(name): AxumPath, ) -> impl IntoResponse { let settings = state.settings.lock().unwrap(); - let profiles_dir = profile::resolve_profiles_dir(&settings, &state.settings_path); - let store = ProfileStore::load(&profile::profiles_store_path(&profiles_dir)); - match store.list_dashboards(&name) { + match settings.list_dashboards(&name) { Ok(dashboards) => { let dtos: Vec = dashboards.iter().map(DashboardDto::from).collect(); Json(dtos).into_response() @@ -679,11 +662,8 @@ async fn create_dashboard( AxumPath(name): AxumPath, Json(body): Json, ) -> impl IntoResponse { - let settings = state.settings.lock().unwrap(); - let profiles_dir = profile::resolve_profiles_dir(&settings, &state.settings_path); - let store_path = profile::profiles_store_path(&profiles_dir); - let mut store = ProfileStore::load(&store_path); - match store.create_dashboard(&store_path, &name, body.name) { + let mut settings = state.settings.lock().unwrap(); + match settings.create_dashboard(&state.settings_path, &name, body.name) { Ok(dashboard) => (StatusCode::CREATED, Json(DashboardDto::from(&dashboard))).into_response(), Err(err) => profile_error_response(err), } @@ -694,9 +674,7 @@ async fn get_dashboard( AxumPath((name, dashboard_id)): AxumPath<(String, String)>, ) -> impl IntoResponse { let settings = state.settings.lock().unwrap(); - let profiles_dir = profile::resolve_profiles_dir(&settings, &state.settings_path); - let store = ProfileStore::load(&profile::profiles_store_path(&profiles_dir)); - match store.list_dashboards(&name) { + match settings.list_dashboards(&name) { Ok(dashboards) => match dashboards.iter().find(|d| d.id == dashboard_id) { Some(dashboard) => Json(DashboardDto::from(dashboard)).into_response(), None => profile_error_response(ProfileError::NotFound), @@ -710,16 +688,13 @@ async fn save_dashboard( AxumPath((name, dashboard_id)): AxumPath<(String, String)>, Json(body): Json, ) -> impl IntoResponse { - let settings = state.settings.lock().unwrap(); - let profiles_dir = profile::resolve_profiles_dir(&settings, &state.settings_path); - let store_path = profile::profiles_store_path(&profiles_dir); - let mut store = ProfileStore::load(&store_path); - let dashboard = crate::profile::Dashboard { + let mut settings = state.settings.lock().unwrap(); + let dashboard = Dashboard { id: dashboard_id.clone(), name: body.name, panels: body.panels.iter().map(|p| p.into()).collect(), }; - match store.save_dashboard(&store_path, &name, &dashboard_id, dashboard) { + match settings.save_dashboard(&state.settings_path, &name, &dashboard_id, dashboard) { Ok(dashboard) => Json(DashboardDto::from(&dashboard)).into_response(), Err(err) => profile_error_response(err), } @@ -729,11 +704,8 @@ async fn delete_dashboard( State(state): State, AxumPath((name, dashboard_id)): AxumPath<(String, String)>, ) -> impl IntoResponse { - let settings = state.settings.lock().unwrap(); - let profiles_dir = profile::resolve_profiles_dir(&settings, &state.settings_path); - let store_path = profile::profiles_store_path(&profiles_dir); - let mut store = ProfileStore::load(&store_path); - match store.delete_dashboard(&store_path, &name, &dashboard_id) { + let mut settings = state.settings.lock().unwrap(); + match settings.delete_dashboard(&state.settings_path, &name, &dashboard_id) { Ok(()) => StatusCode::NO_CONTENT.into_response(), Err(err) => profile_error_response(err), } @@ -1158,20 +1130,10 @@ mod tests { // Use the same shared settings handle the router was built with — a separately-loaded Settings wouldn't be visible to the running app. let settings_path = dir.path().join("settings.json"); - let profiles_dir = { - let settings = settings_handle.lock().unwrap(); - profile::resolve_profiles_dir(&settings, &settings_path) - }; - let store_path = profile::profiles_store_path(&profiles_dir); - let mut store = ProfileStore::load(&store_path); { let mut settings = settings_handle.lock().unwrap(); - store - .create(&store_path, &mut settings, &settings_path, "web".to_string()) - .unwrap(); - store - .set_active(&mut settings, &settings_path, Some("web".to_string())) - .unwrap(); + settings.create_profile(&settings_path, "web".to_string()).unwrap(); + settings.set_active_profile(&settings_path, Some("web".to_string())).unwrap(); } let (status, _body) = send_json( @@ -1188,8 +1150,8 @@ mod tests { .await; assert_eq!(status, StatusCode::CREATED); - let reloaded_store = ProfileStore::load(&store_path); - let profile = reloaded_store.get("web").unwrap(); + let reloaded_settings = Settings::load(&settings_path); + let profile = reloaded_settings.profile("web").unwrap(); assert_eq!(profile.params.len(), 1); assert_eq!(profile.params[0].name, "level"); } @@ -1271,7 +1233,7 @@ mod tests { #[tokio::test] async fn saved_dashboard_persists_across_simulated_restart() { - let (app, _engine, settings_handle, dir) = test_app(); + let (app, _engine, _settings_handle, dir) = test_app(); let (status, _body) = send_json(&app, "POST", "/api/settings/profiles", json!({ "name": "web" })).await; assert_eq!(status, StatusCode::CREATED); @@ -1298,15 +1260,10 @@ mod tests { ) .await; - // Simulate a restart by reloading the profile store fresh from disk, independent of the running app/engine. + // Simulate a restart by reloading settings fresh from disk, independent of the running app/engine. let settings_path = dir.path().join("settings.json"); - let profiles_dir = { - let settings = settings_handle.lock().unwrap(); - profile::resolve_profiles_dir(&settings, &settings_path) - }; - let store_path = profile::profiles_store_path(&profiles_dir); - let reloaded_store = ProfileStore::load(&store_path); - let dashboards = reloaded_store.list_dashboards("web").unwrap(); + let reloaded_settings = Settings::load(&settings_path); + let dashboards = reloaded_settings.list_dashboards("web").unwrap(); assert_eq!(dashboards.len(), 1); assert_eq!(dashboards[0].name, "Overview"); assert_eq!(dashboards[0].panels.len(), 1); diff --git a/src/dto.rs b/src/dto.rs index d42be87..d94dc0f 100644 --- a/src/dto.rs +++ b/src/dto.rs @@ -190,7 +190,7 @@ impl From<&Settings> for SettingsDto { fn from(settings: &Settings) -> Self { SettingsDto { color_scheme: settings.color_scheme, - active_profile: settings.profiles.active_profile.clone(), + active_profile: settings.active_profile.clone(), max_logs: settings.max_logs, custom_theme_id: settings.custom_theme_id.clone(), ntfy_topic: settings.ntfy_topic.clone(), diff --git a/src/main.rs b/src/main.rs index bb3fdf4..2fe8617 100644 --- a/src/main.rs +++ b/src/main.rs @@ -20,7 +20,6 @@ use clap::Parser; use tokio::net::TcpListener; use tokio::sync::mpsc; -use profile::ProfileStore; use settings::Settings; /// The resolved default settings path, rendered into `--help` so `--settings-path`'s help text @@ -74,28 +73,28 @@ async fn main() { // Reload the active profile's declarations before ingest starts: there's no way to // reprocess early lines later, so declaring after ingest starts could miss lines that raced ahead. { - let (active_profile_name, profiles_dir) = { + let (active_profile_name, active_profile) = { let mut settings_guard = settings.lock().unwrap(); - let profiles_dir = profile::resolve_profiles_dir(&settings_guard, &settings_path); - let mut store = ProfileStore::load(&profile::profiles_store_path(&profiles_dir)); - store - .ensure_default(&profile::profiles_store_path(&profiles_dir), &mut settings_guard, &settings_path) + settings_guard + .ensure_default_profile(&settings_path) .expect("failed to ensure the Default profile exists"); - (settings_guard.profiles.active_profile.clone(), profiles_dir) + let active_profile_name = settings_guard.active_profile.clone(); + let active_profile = active_profile_name + .as_deref() + .and_then(|name| settings_guard.profile(name).cloned()); + (active_profile_name, active_profile) }; - if let Some(profile_name) = active_profile_name { - let store = ProfileStore::load(&profile::profiles_store_path(&profiles_dir)); - match store.get(&profile_name) { - Some(active_profile) => { - profile::load_into_engine(&engine_tx, active_profile).await; - } - None => { - eprintln!( - "warning: active profile '{profile_name}' not found under {}; skipping", - profiles_dir.display() - ); - } + match (active_profile_name, active_profile) { + (Some(_), Some(active_profile)) => { + profile::load_into_engine(&engine_tx, &active_profile).await; } + (Some(profile_name), None) => { + eprintln!( + "warning: active profile '{profile_name}' not found in {}; skipping", + settings_path.display() + ); + } + (None, _) => {} } } diff --git a/src/profile.rs b/src/profile.rs index 4ef2c81..6a45363 100644 --- a/src/profile.rs +++ b/src/profile.rs @@ -1,5 +1,8 @@ -//! Named, persistable bundles of parsing/indexing rules and dashboards for a CLI app, persisted as a single `profiles.json`. -//! `Settings.profiles` only tracks lightweight name/active pointers; this module owns the actual content. +//! Named, persistable bundles of parsing/indexing rules and dashboards for a CLI app. +//! Ticket 049 folded profile storage inline into `Settings` (`settings.json`) instead of a separate +//! `profiles.json`; this module still owns the `Profile`/`Dashboard`/declaration data model, plus the +//! CRUD methods on `Settings` (`impl Settings` below `ProfileError`) that used to live on the +//! now-removed `ProfileStore`. #![allow(dead_code)] use std::collections::hash_map::DefaultHasher; @@ -8,7 +11,7 @@ use std::fmt; use std::fs; use std::hash::{Hash, Hasher}; use std::io; -use std::path::{Path, PathBuf}; +use std::path::Path; use std::time::{SystemTime, UNIX_EPOCH}; use serde::{Deserialize, Serialize}; @@ -18,7 +21,7 @@ use crate::engine::{DeclareMetricRequest, DeclareParamRequest, EngineHandle, Eng use crate::settings::Settings; use tokio::sync::oneshot; -/// Name of the profile that always exists and can't be deleted (see `ProfileStore::ensure_default`). +/// Name of the profile that always exists and can't be deleted (see `Settings::ensure_default_profile`). pub const DEFAULT_PROFILE_NAME: &str = "Default"; /// Mirrors `data_value::MetricPrimitiveKind` with `Serialize`/`Deserialize` so profiles can round-trip through JSON, since the engine-internal type only derives `Serialize`. @@ -325,7 +328,7 @@ pub async fn load_into_engine(engine: &EngineHandle, profile: &Profile) { } } -/// Error returned by `ProfileStore` mutations and lookups. +/// Error returned by `Settings`'s profile-related mutations and lookups. #[derive(Debug)] pub enum ProfileError { /// No profile with the given name exists. @@ -357,168 +360,106 @@ impl From for ProfileError { } } -/// Resolves `settings.profiles.profiles_dir` relative to `settings_path`'s parent directory (or as absolute if it already is one). -pub fn resolve_profiles_dir(settings: &Settings, settings_path: &Path) -> PathBuf { - let profiles_dir = Path::new(&settings.profiles.profiles_dir); - if profiles_dir.is_absolute() { - profiles_dir.to_path_buf() - } else { - settings_path - .parent() - .map(|parent| parent.join(profiles_dir)) - .unwrap_or_else(|| profiles_dir.to_path_buf()) - } -} - -/// Path to the single file holding all profiles, within `profiles_dir`. -pub fn profiles_store_path(profiles_dir: &Path) -> PathBuf { - profiles_dir.join("profiles.json") -} - -/// On-disk collection of all known profiles, persisted as a single JSON file. -#[derive(Debug, Clone, Default)] -pub struct ProfileStore { - profiles: Vec, -} - -impl ProfileStore { - /// Loads the store from `store_path`; a missing or unreadable/unparsable file is treated as an empty store rather than failing startup. - pub fn load(store_path: &Path) -> ProfileStore { - match fs::read_to_string(store_path) { - Ok(contents) => match serde_json::from_str::>(&contents) { - Ok(profiles) => ProfileStore { profiles }, - Err(err) => { - eprintln!( - "warning: failed to parse profile store at {}: {err}; using empty store", - store_path.display() - ); - ProfileStore::default() - } - }, - Err(err) if err.kind() == io::ErrorKind::NotFound => ProfileStore::default(), - Err(err) => { - eprintln!( - "warning: failed to read profile store at {}: {err}; using empty store", - store_path.display() - ); - ProfileStore::default() - } +/// Reads a legacy pre-049 `profiles.json` file (back when profiles lived in a separate file rather +/// than inline in `settings.json`) for `Settings::migrate_legacy_profiles_file`. Returns `None` if +/// the file doesn't exist or can't be read/parsed — treated as "nothing to migrate" rather than +/// failing settings load. +pub(crate) fn load_legacy_profile_store(path: &Path) -> Option> { + let contents = fs::read_to_string(path).ok()?; + match serde_json::from_str::>(&contents) { + Ok(profiles) => Some(profiles), + Err(err) => { + eprintln!( + "warning: failed to parse legacy profile store at {}: {err}; skipping migration", + path.display() + ); + None } } +} - /// Persists the store to `store_path` as pretty-printed JSON, creating any missing parent directories first. - pub fn save(&self, store_path: &Path) -> io::Result<()> { - if let Some(parent) = store_path.parent() { - fs::create_dir_all(parent)?; - } - let contents = serde_json::to_string_pretty(&self.profiles) - .map_err(|err| io::Error::new(io::ErrorKind::InvalidData, err))?; - fs::write(store_path, contents) - } - - pub fn list(&self) -> &[Profile] { - &self.profiles - } - - pub fn get(&self, name: &str) -> Option<&Profile> { +/// Profile CRUD, previously on the standalone `ProfileStore` (removed by ticket 049): every mutation +/// here reads/writes `self.profiles` directly and persists via `Settings::save`, so `settings.json` is +/// the single source of truth instead of a separate file kept in sync alongside it. +impl Settings { + pub fn profile(&self, name: &str) -> Option<&Profile> { self.profiles.iter().find(|p| p.name == name) } - /// Creates a new profile named `name` (persisting store and settings), copying the active profile's params/metrics/dashboards as a starting point rather than starting blank. Fails with `DuplicateName` if the name exists. - pub fn create( - &mut self, - store_path: &Path, - settings: &mut Settings, - settings_path: &Path, - name: String, - ) -> Result<(), ProfileError> { - if self.get(&name).is_some() { + /// Creates a new profile named `name`, persisting settings, copying the active profile's + /// params/metrics/dashboards as a starting point rather than starting blank. Fails with + /// `DuplicateName` if the name exists. + pub fn create_profile(&mut self, settings_path: &Path, name: String) -> Result<(), ProfileError> { + if self.profile(&name).is_some() { return Err(ProfileError::DuplicateName); } let mut profile = Profile::new(name.clone()); - if let Some(active) = settings.profiles.active_profile.as_deref().and_then(|n| self.get(n)) { + if let Some(active) = self.active_profile.as_deref().and_then(|n| self.profile(n)) { profile.params = active.params.clone(); profile.metrics = active.metrics.clone(); profile.dashboards = active.dashboards.clone(); } self.profiles.push(profile); - self.save(store_path)?; - - if !settings.profiles.names.contains(&name) { - settings.profiles.names.push(name); - } - settings.save(settings_path)?; + self.save(settings_path)?; Ok(()) } - /// Deletes the profile named `name` (persisting store and settings), clearing `active_profile` if it was active. Fails with `NotFound`, or `CannotDeleteDefault` for the `Default` profile. - pub fn delete( - &mut self, - store_path: &Path, - settings: &mut Settings, - settings_path: &Path, - name: &str, - ) -> Result<(), ProfileError> { + /// Deletes the profile named `name`, persisting settings, clearing `active_profile` if it was + /// active. Fails with `NotFound`, or `CannotDeleteDefault` for the `Default` profile. + pub fn delete_profile(&mut self, settings_path: &Path, name: &str) -> Result<(), ProfileError> { if name == DEFAULT_PROFILE_NAME { return Err(ProfileError::CannotDeleteDefault); } let index = self.profiles.iter().position(|p| p.name == name).ok_or(ProfileError::NotFound)?; self.profiles.remove(index); - self.save(store_path)?; - - settings.profiles.names.retain(|n| n != name); - if settings.profiles.active_profile.as_deref() == Some(name) { - settings.profiles.active_profile = None; + if self.active_profile.as_deref() == Some(name) { + self.active_profile = None; } - settings.save(settings_path)?; + self.save(settings_path)?; Ok(()) } - /// Ensures a `Default` profile exists (seeding a starter dashboard/metric only on first creation), is tracked in `settings.profiles.names`, and falls back to active if nothing else is. Called once on startup before anything else touches profiles. - pub fn ensure_default( - &mut self, - store_path: &Path, - settings: &mut Settings, - settings_path: &Path, - ) -> Result<(), ProfileError> { - if self.get(DEFAULT_PROFILE_NAME).is_none() { + /// Ensures a `Default` profile exists (seeding a starter dashboard/metric only on first + /// creation), and falls back to active if nothing else is. Called once on startup before anything + /// else touches profiles. + pub fn ensure_default_profile(&mut self, settings_path: &Path) -> Result<(), ProfileError> { + if self.profile(DEFAULT_PROFILE_NAME).is_none() { let mut profile = Profile::new(DEFAULT_PROFILE_NAME.to_string()); profile.metrics.push(seeded_error_split_metric()); profile.dashboards.push(seeded_default_dashboard()); self.profiles.push(profile); - self.save(store_path)?; } - if !settings.profiles.names.iter().any(|n| n == DEFAULT_PROFILE_NAME) { - settings.profiles.names.push(DEFAULT_PROFILE_NAME.to_string()); + if self.active_profile.is_none() { + self.active_profile = Some(DEFAULT_PROFILE_NAME.to_string()); } - if settings.profiles.active_profile.is_none() { - settings.profiles.active_profile = Some(DEFAULT_PROFILE_NAME.to_string()); - } - settings.save(settings_path)?; + self.save(settings_path)?; Ok(()) } - /// Sets (or clears, with `None`) the active profile in `settings`. Fails with `NotFound` if `name` is `Some` and no such profile exists. Doesn't itself reload declarations into a running engine — that happens on next startup via `load_into_engine`. - pub fn set_active( - &self, - settings: &mut Settings, + /// Sets (or clears, with `None`) the active profile. Fails with `NotFound` if `name` is `Some` + /// and no such profile exists. Doesn't itself reload declarations into a running engine — that + /// happens on next startup via `load_into_engine`. + pub fn set_active_profile( + &mut self, settings_path: &Path, name: Option, ) -> Result<(), ProfileError> { if let Some(name) = &name { - if self.get(name).is_none() { + if self.profile(name).is_none() { return Err(ProfileError::NotFound); } } - settings.profiles.active_profile = name; - settings.save(settings_path)?; + self.active_profile = name; + self.save(settings_path)?; Ok(()) } - /// Records/updates a declared param into `profile_name`'s stored declarations (matched by param name), persisting the store. Intended to be called by the HTTP API alongside declaring against the engine, so declarations survive a restart. + /// Records/updates a declared param into `profile_name`'s stored declarations (matched by param + /// name), persisting settings. Intended to be called by the HTTP API alongside declaring against + /// the engine, so declarations survive a restart. pub fn record_param_declaration( &mut self, - store_path: &Path, + settings_path: &Path, profile_name: &str, request: &DeclareParamRequest, ) -> Result<(), ProfileError> { @@ -532,14 +473,15 @@ impl ProfileStore { Some(existing) => *existing = record, None => profile.params.push(record), } - self.save(store_path)?; + self.save(settings_path)?; Ok(()) } - /// Records/updates a declared metric into `profile_name`'s stored declarations (matched by metric name), persisting the store; same integration intent as `record_param_declaration`. + /// Records/updates a declared metric into `profile_name`'s stored declarations (matched by metric + /// name), persisting settings; same integration intent as `record_param_declaration`. pub fn record_metric_declaration( &mut self, - store_path: &Path, + settings_path: &Path, profile_name: &str, request: &DeclareMetricRequest, ) -> Result<(), ProfileError> { @@ -553,14 +495,16 @@ impl ProfileStore { Some(existing) => *existing = record, None => profile.metrics.push(record), } - self.save(store_path)?; + self.save(settings_path)?; Ok(()) } - /// Same as `record_metric_declaration`, for an in-place edit that may rename the metric: drops any persisted record under `old_name` first (when it differs from the new name) so a rename doesn't leave a stale duplicate declaration behind, then upserts by the new name as usual. + /// Same as `record_metric_declaration`, for an in-place edit that may rename the metric: drops + /// any persisted record under `old_name` first (when it differs from the new name) so a rename + /// doesn't leave a stale duplicate declaration behind, then upserts by the new name as usual. pub fn record_metric_declaration_update( &mut self, - store_path: &Path, + settings_path: &Path, profile_name: &str, old_name: &str, request: &DeclareMetricRequest, @@ -578,20 +522,21 @@ impl ProfileStore { Some(existing) => *existing = record, None => profile.metrics.push(record), } - self.save(store_path)?; + self.save(settings_path)?; Ok(()) } /// Lists the dashboards saved on `profile_name`; fails with `NotFound` if it doesn't exist. pub fn list_dashboards(&self, profile_name: &str) -> Result<&[Dashboard], ProfileError> { - let profile = self.get(profile_name).ok_or(ProfileError::NotFound)?; + let profile = self.profile(profile_name).ok_or(ProfileError::NotFound)?; Ok(&profile.dashboards) } - /// Creates a new, empty dashboard named `name` on `profile_name`, persisting the store; fails with `NotFound` if the profile doesn't exist. + /// Creates a new, empty dashboard named `name` on `profile_name`, persisting settings; fails + /// with `NotFound` if the profile doesn't exist. pub fn create_dashboard( &mut self, - store_path: &Path, + settings_path: &Path, profile_name: &str, name: String, ) -> Result { @@ -602,14 +547,16 @@ impl ProfileStore { .ok_or(ProfileError::NotFound)?; let dashboard = Dashboard::new(name); profile.dashboards.push(dashboard.clone()); - self.save(store_path)?; + self.save(settings_path)?; Ok(dashboard) } - /// Replaces the dashboard identified by `dashboard_id` with `dashboard`, persisting the store; `dashboard.id` is forced to `dashboard_id` so a caller can't change a dashboard's identity through this call. + /// Replaces the dashboard identified by `dashboard_id` with `dashboard`, persisting settings; + /// `dashboard.id` is forced to `dashboard_id` so a caller can't change a dashboard's identity + /// through this call. pub fn save_dashboard( &mut self, - store_path: &Path, + settings_path: &Path, profile_name: &str, dashboard_id: &str, mut dashboard: Dashboard, @@ -626,14 +573,15 @@ impl ProfileStore { .ok_or(ProfileError::NotFound)?; dashboard.id = dashboard_id.to_string(); *existing = dashboard.clone(); - self.save(store_path)?; + self.save(settings_path)?; Ok(dashboard) } - /// Deletes the dashboard identified by `dashboard_id` from `profile_name`, persisting the store; fails with `NotFound` if either doesn't exist. + /// Deletes the dashboard identified by `dashboard_id` from `profile_name`, persisting settings; + /// fails with `NotFound` if either doesn't exist. pub fn delete_dashboard( &mut self, - store_path: &Path, + settings_path: &Path, profile_name: &str, dashboard_id: &str, ) -> Result<(), ProfileError> { @@ -648,7 +596,7 @@ impl ProfileStore { .position(|d| d.id == dashboard_id) .ok_or(ProfileError::NotFound)?; profile.dashboards.remove(index); - self.save(store_path)?; + self.save(settings_path)?; Ok(()) } } @@ -694,92 +642,64 @@ mod tests { response.await.unwrap() } - #[test] - fn resolve_profiles_dir_is_relative_to_settings_parent() { - let settings_path = Path::new("/config/ottyl/settings.json"); - let mut settings = Settings::default(); - settings.profiles.profiles_dir = "profiles".to_string(); - - let dir = resolve_profiles_dir(&settings, settings_path); - assert_eq!(dir, Path::new("/config/ottyl/profiles")); - } - - #[test] - fn resolve_profiles_dir_respects_absolute_path() { - let settings_path = Path::new("/config/ottyl/settings.json"); - let mut settings = Settings::default(); - settings.profiles.profiles_dir = "/elsewhere/profiles".to_string(); - - let dir = resolve_profiles_dir(&settings, settings_path); - assert_eq!(dir, Path::new("/elsewhere/profiles")); - } - #[test] fn create_list_get_delete_round_trip() { let dir = tempfile::tempdir().unwrap(); - let store_path = dir.path().join("profiles.json"); let settings_path = dir.path().join("settings.json"); let mut settings = Settings::default(); - let mut store = ProfileStore::load(&store_path); - store.create(&store_path, &mut settings, &settings_path, "web".to_string()).unwrap(); - store.create(&store_path, &mut settings, &settings_path, "worker".to_string()).unwrap(); + settings.create_profile(&settings_path, "web".to_string()).unwrap(); + settings.create_profile(&settings_path, "worker".to_string()).unwrap(); - assert_eq!(store.list().len(), 2); - assert!(store.get("web").is_some()); - assert_eq!(settings.profiles.names, vec!["web".to_string(), "worker".to_string()]); + assert_eq!(settings.profiles.len(), 2); + assert!(settings.profile("web").is_some()); - let dup = store.create(&store_path, &mut settings, &settings_path, "web".to_string()); + let dup = settings.create_profile(&settings_path, "web".to_string()); assert!(matches!(dup, Err(ProfileError::DuplicateName))); - // Reload from disk: profiles and settings both persisted. - let reloaded = ProfileStore::load(&store_path); - assert_eq!(reloaded.list().len(), 2); + // Reload from disk: profiles persisted. let reloaded_settings = Settings::load(&settings_path); - assert_eq!(reloaded_settings.profiles.names, settings.profiles.names); + assert_eq!(reloaded_settings.profiles.len(), 2); + assert!(reloaded_settings.profile("web").is_some()); + assert!(reloaded_settings.profile("worker").is_some()); - store.delete(&store_path, &mut settings, &settings_path, "worker").unwrap(); - assert!(store.get("worker").is_none()); - assert_eq!(settings.profiles.names, vec!["web".to_string()]); + settings.delete_profile(&settings_path, "worker").unwrap(); + assert!(settings.profile("worker").is_none()); + assert_eq!(settings.profiles.len(), 1); - let missing = store.delete(&store_path, &mut settings, &settings_path, "worker"); + let missing = settings.delete_profile(&settings_path, "worker"); assert!(matches!(missing, Err(ProfileError::NotFound))); } #[test] fn ensure_default_creates_default_and_activates_it_once() { let dir = tempfile::tempdir().unwrap(); - let store_path = dir.path().join("profiles.json"); let settings_path = dir.path().join("settings.json"); let mut settings = Settings::default(); - let mut store = ProfileStore::load(&store_path); - store.ensure_default(&store_path, &mut settings, &settings_path).unwrap(); + settings.ensure_default_profile(&settings_path).unwrap(); - assert!(store.get(DEFAULT_PROFILE_NAME).is_some()); - assert!(settings.profiles.names.contains(&DEFAULT_PROFILE_NAME.to_string())); - assert_eq!(settings.profiles.active_profile, Some(DEFAULT_PROFILE_NAME.to_string())); + assert!(settings.profile(DEFAULT_PROFILE_NAME).is_some()); + assert_eq!(settings.active_profile, Some(DEFAULT_PROFILE_NAME.to_string())); // Idempotent: doesn't duplicate the profile or clobber an already-active (non-Default) profile. - store.create(&store_path, &mut settings, &settings_path, "web".to_string()).unwrap(); - store.set_active(&mut settings, &settings_path, Some("web".to_string())).unwrap(); - store.ensure_default(&store_path, &mut settings, &settings_path).unwrap(); + settings.create_profile(&settings_path, "web".to_string()).unwrap(); + settings.set_active_profile(&settings_path, Some("web".to_string())).unwrap(); + settings.ensure_default_profile(&settings_path).unwrap(); - assert_eq!(store.list().iter().filter(|p| p.name == DEFAULT_PROFILE_NAME).count(), 1); - assert_eq!(settings.profiles.active_profile, Some("web".to_string())); + assert_eq!(settings.profiles.iter().filter(|p| p.name == DEFAULT_PROFILE_NAME).count(), 1); + assert_eq!(settings.active_profile, Some("web".to_string())); } #[test] fn ensure_default_seeds_starter_dashboard_only_on_creation() { let dir = tempfile::tempdir().unwrap(); - let store_path = dir.path().join("profiles.json"); let settings_path = dir.path().join("settings.json"); let mut settings = Settings::default(); - let mut store = ProfileStore::load(&store_path); - store.ensure_default(&store_path, &mut settings, &settings_path).unwrap(); + settings.ensure_default_profile(&settings_path).unwrap(); - let default_profile = store.get(DEFAULT_PROFILE_NAME).unwrap(); + let default_profile = settings.profile(DEFAULT_PROFILE_NAME).unwrap(); assert_eq!(default_profile.metrics.len(), 1); assert_eq!(default_profile.metrics[0].name, SEEDED_METRIC_NAME); assert_eq!( @@ -800,9 +720,9 @@ mod tests { assert!(dashboard.panels.iter().any(|p| p.chart_type == ChartType::Pie && p.promql == SEEDED_METRIC_NAME)); // Reload (simulating a restart) and ensure_default again: must not re-seed or duplicate. - let mut store = ProfileStore::load(&store_path); - store.ensure_default(&store_path, &mut settings, &settings_path).unwrap(); - let default_profile = store.get(DEFAULT_PROFILE_NAME).unwrap(); + let mut reloaded = Settings::load(&settings_path); + reloaded.ensure_default_profile(&settings_path).unwrap(); + let default_profile = reloaded.profile(DEFAULT_PROFILE_NAME).unwrap(); assert_eq!(default_profile.metrics.len(), 1); assert_eq!(default_profile.dashboards.len(), 1); } @@ -814,12 +734,10 @@ mod tests { use tokio::sync::mpsc; let dir = tempfile::tempdir().unwrap(); - let store_path = dir.path().join("profiles.json"); let settings_path = dir.path().join("settings.json"); let mut settings = Settings::default(); - let mut store = ProfileStore::load(&store_path); - store.ensure_default(&store_path, &mut settings, &settings_path).unwrap(); - let default_profile = store.get(DEFAULT_PROFILE_NAME).unwrap().clone(); + settings.ensure_default_profile(&settings_path).unwrap(); + let default_profile = settings.profile(DEFAULT_PROFILE_NAME).unwrap().clone(); let (engine_tx, engine_rx) = mpsc::channel(128); let settings_handle = std::sync::Arc::new(std::sync::Mutex::new(settings.clone())); @@ -857,41 +775,37 @@ mod tests { #[test] fn default_profile_cannot_be_deleted() { let dir = tempfile::tempdir().unwrap(); - let store_path = dir.path().join("profiles.json"); let settings_path = dir.path().join("settings.json"); let mut settings = Settings::default(); - let mut store = ProfileStore::load(&store_path); - store.ensure_default(&store_path, &mut settings, &settings_path).unwrap(); + settings.ensure_default_profile(&settings_path).unwrap(); - let err = store.delete(&store_path, &mut settings, &settings_path, DEFAULT_PROFILE_NAME); + let err = settings.delete_profile(&settings_path, DEFAULT_PROFILE_NAME); assert!(matches!(err, Err(ProfileError::CannotDeleteDefault))); - assert!(store.get(DEFAULT_PROFILE_NAME).is_some()); + assert!(settings.profile(DEFAULT_PROFILE_NAME).is_some()); } #[test] fn create_copies_the_active_profile() { let dir = tempfile::tempdir().unwrap(); - let store_path = dir.path().join("profiles.json"); let settings_path = dir.path().join("settings.json"); let mut settings = Settings::default(); - let mut store = ProfileStore::load(&store_path); - store.create(&store_path, &mut settings, &settings_path, "web".to_string()).unwrap(); + settings.create_profile(&settings_path, "web".to_string()).unwrap(); let param_req = param_request("level", r"level=(?P\w+)", "level"); - store.record_param_declaration(&store_path, "web", ¶m_req).unwrap(); - store.create_dashboard(&store_path, "web", "Overview".to_string()).unwrap(); - store.set_active(&mut settings, &settings_path, Some("web".to_string())).unwrap(); + settings.record_param_declaration(&settings_path, "web", ¶m_req).unwrap(); + settings.create_dashboard(&settings_path, "web", "Overview".to_string()).unwrap(); + settings.set_active_profile(&settings_path, Some("web".to_string())).unwrap(); - store.create(&store_path, &mut settings, &settings_path, "web-copy".to_string()).unwrap(); - let copy = store.get("web-copy").unwrap(); - assert_eq!(copy.params, store.get("web").unwrap().params); - assert_eq!(copy.dashboards, store.get("web").unwrap().dashboards); + settings.create_profile(&settings_path, "web-copy".to_string()).unwrap(); + let copy = settings.profile("web-copy").unwrap().clone(); + assert_eq!(copy.params, settings.profile("web").unwrap().params); + assert_eq!(copy.dashboards, settings.profile("web").unwrap().dashboards); // With no active profile, new profiles start empty as before. - store.set_active(&mut settings, &settings_path, None).unwrap(); - store.create(&store_path, &mut settings, &settings_path, "blank".to_string()).unwrap(); - let blank = store.get("blank").unwrap(); + settings.set_active_profile(&settings_path, None).unwrap(); + settings.create_profile(&settings_path, "blank".to_string()).unwrap(); + let blank = settings.profile("blank").unwrap(); assert!(blank.params.is_empty()); assert!(blank.dashboards.is_empty()); } @@ -899,44 +813,40 @@ mod tests { #[test] fn set_active_updates_and_persists_settings() { let dir = tempfile::tempdir().unwrap(); - let store_path = dir.path().join("profiles.json"); let settings_path = dir.path().join("settings.json"); let mut settings = Settings::default(); - let mut store = ProfileStore::load(&store_path); - store.create(&store_path, &mut settings, &settings_path, "web".to_string()).unwrap(); - store.create(&store_path, &mut settings, &settings_path, "worker".to_string()).unwrap(); + settings.create_profile(&settings_path, "web".to_string()).unwrap(); + settings.create_profile(&settings_path, "worker".to_string()).unwrap(); - store.set_active(&mut settings, &settings_path, Some("web".to_string())).unwrap(); - assert_eq!(settings.profiles.active_profile, Some("web".to_string())); + settings.set_active_profile(&settings_path, Some("web".to_string())).unwrap(); + assert_eq!(settings.active_profile, Some("web".to_string())); // Switching to another known profile works and persists. - store.set_active(&mut settings, &settings_path, Some("worker".to_string())).unwrap(); + settings.set_active_profile(&settings_path, Some("worker".to_string())).unwrap(); let reloaded = Settings::load(&settings_path); - assert_eq!(reloaded.profiles.active_profile, Some("worker".to_string())); + assert_eq!(reloaded.active_profile, Some("worker".to_string())); // Setting an unknown profile active is rejected, leaving the previous value. - let err = store.set_active(&mut settings, &settings_path, Some("ghost".to_string())); + let err = settings.set_active_profile(&settings_path, Some("ghost".to_string())); assert!(matches!(err, Err(ProfileError::NotFound))); - assert_eq!(settings.profiles.active_profile, Some("worker".to_string())); + assert_eq!(settings.active_profile, Some("worker".to_string())); // Clearing the active profile works. - store.set_active(&mut settings, &settings_path, None).unwrap(); - assert_eq!(settings.profiles.active_profile, None); + settings.set_active_profile(&settings_path, None).unwrap(); + assert_eq!(settings.active_profile, None); } #[test] fn record_param_and_metric_declarations_persist_into_profile() { let dir = tempfile::tempdir().unwrap(); - let store_path = dir.path().join("profiles.json"); let settings_path = dir.path().join("settings.json"); let mut settings = Settings::default(); - let mut store = ProfileStore::load(&store_path); - store.create(&store_path, &mut settings, &settings_path, "web".to_string()).unwrap(); + settings.create_profile(&settings_path, "web".to_string()).unwrap(); let param_req = param_request("level", r"level=(?P\w+)", "level"); - store.record_param_declaration(&store_path, "web", ¶m_req).unwrap(); + settings.record_param_declaration(&settings_path, "web", ¶m_req).unwrap(); let metric_req = metric_request( "requests", @@ -944,9 +854,9 @@ mod tests { "duration", "duration", ); - store.record_metric_declaration(&store_path, "web", &metric_req).unwrap(); + settings.record_metric_declaration(&settings_path, "web", &metric_req).unwrap(); - let profile = store.get("web").unwrap(); + let profile = settings.profile("web").unwrap(); assert_eq!(profile.params.len(), 1); assert_eq!(profile.params[0].name, "level"); assert_eq!(profile.metrics.len(), 1); @@ -954,39 +864,37 @@ mod tests { // Re-declaring the same name updates in place rather than duplicating. let param_req_v2 = param_request("level", r"lvl=(?P\w+)", "level"); - store.record_param_declaration(&store_path, "web", ¶m_req_v2).unwrap(); - let profile = store.get("web").unwrap(); + settings.record_param_declaration(&settings_path, "web", ¶m_req_v2).unwrap(); + let profile = settings.profile("web").unwrap(); assert_eq!(profile.params.len(), 1); assert_eq!(profile.params[0].message_match_regex, r"lvl=(?P\w+)"); // Reload from disk: recorded declarations survive a restart. - let reloaded = ProfileStore::load(&store_path); - let profile = reloaded.get("web").unwrap(); + let reloaded = Settings::load(&settings_path); + let profile = reloaded.profile("web").unwrap(); assert_eq!(profile.params.len(), 1); assert_eq!(profile.metrics.len(), 1); } - /// Acceptance criterion: declaring params/metrics, "restarting" (reloading the store from disk), and replaying into a fresh engine reproduces the same declarations and still ingests lines, without re-declaring by hand. + /// Acceptance criterion: declaring params/metrics, "restarting" (reloading settings from disk), and replaying into a fresh engine reproduces the same declarations and still ingests lines, without re-declaring by hand. #[tokio::test] async fn profile_round_trip_reproduces_declarations_in_fresh_engine() { let dir = tempfile::tempdir().unwrap(); - let store_path = dir.path().join("profiles.json"); let settings_path = dir.path().join("settings.json"); let mut settings = Settings::default(); // Declare directly into a profile, as the HTTP API will do via the `record_*_declaration` hooks. - let mut store = ProfileStore::load(&store_path); - store.create(&store_path, &mut settings, &settings_path, "web".to_string()).unwrap(); - store + settings.create_profile(&settings_path, "web".to_string()).unwrap(); + settings .record_param_declaration( - &store_path, + &settings_path, "web", ¶m_request("level", r"level=(?P\w+)", "level"), ) .unwrap(); - store + settings .record_metric_declaration( - &store_path, + &settings_path, "web", &metric_request( "requests", @@ -996,15 +904,13 @@ mod tests { ), ) .unwrap(); - store.set_active(&mut settings, &settings_path, Some("web".to_string())).unwrap(); + settings.set_active_profile(&settings_path, Some("web".to_string())).unwrap(); // Simulate a restart: drop everything in memory, reload from disk. - drop(store); drop(settings); let settings = Settings::load(&settings_path); - let active_name = settings.profiles.active_profile.clone().unwrap(); - let reloaded_store = ProfileStore::load(&store_path); - let profile = reloaded_store.get(&active_name).unwrap().clone(); + let active_name = settings.active_profile.clone().unwrap(); + let profile = settings.profile(&active_name).unwrap().clone(); // Spawn a fresh engine and replay the profile's declarations into it. let (engine_tx, engine_rx) = mpsc::channel(16); @@ -1061,18 +967,16 @@ mod tests { assert_eq!(logs[0].text, "level=info request duration=42"); } - /// Acceptance criterion: a dashboard (with a panel) survives reloading the profile store from disk. + /// Acceptance criterion: a dashboard (with a panel) survives reloading settings from disk. #[test] fn dashboard_create_save_delete_round_trip_and_persists_across_reload() { let dir = tempfile::tempdir().unwrap(); - let store_path = dir.path().join("profiles.json"); let settings_path = dir.path().join("settings.json"); let mut settings = Settings::default(); - let mut store = ProfileStore::load(&store_path); - store.create(&store_path, &mut settings, &settings_path, "web".to_string()).unwrap(); + settings.create_profile(&settings_path, "web".to_string()).unwrap(); - let dashboard = store.create_dashboard(&store_path, "web", "Overview".to_string()).unwrap(); + let dashboard = settings.create_dashboard(&settings_path, "web", "Overview".to_string()).unwrap(); assert_eq!(dashboard.name, "Overview"); assert!(dashboard.panels.is_empty()); @@ -1083,15 +987,15 @@ mod tests { promql: r#"requests{path="/a"}"#.to_string(), chart_type: ChartType::Line, }); - let saved = store - .save_dashboard(&store_path, "web", &dashboard.id, with_panel) + let saved = settings + .save_dashboard(&settings_path, "web", &dashboard.id, with_panel) .unwrap(); assert_eq!(saved.panels.len(), 1); assert_eq!(saved.id, dashboard.id); // Simulate a restart: drop and reload from disk. - drop(store); - let reloaded = ProfileStore::load(&store_path); + drop(settings); + let mut reloaded = Settings::load(&settings_path); let dashboards = reloaded.list_dashboards("web").unwrap(); assert_eq!(dashboards.len(), 1); assert_eq!(dashboards[0].name, "Overview"); @@ -1099,10 +1003,9 @@ mod tests { assert_eq!(dashboards[0].panels[0].promql, r#"requests{path="/a"}"#); // Deleting removes it, and that too persists. - let mut reloaded = reloaded; - reloaded.delete_dashboard(&store_path, "web", &dashboard.id).unwrap(); + reloaded.delete_dashboard(&settings_path, "web", &dashboard.id).unwrap(); assert!(reloaded.list_dashboards("web").unwrap().is_empty()); - let reloaded_again = ProfileStore::load(&store_path); + let reloaded_again = Settings::load(&settings_path); assert!(reloaded_again.list_dashboards("web").unwrap().is_empty()); // Unknown profile/dashboard both fail with NotFound. diff --git a/src/settings.rs b/src/settings.rs index 7e1eee9..22dbb5a 100644 --- a/src/settings.rs +++ b/src/settings.rs @@ -1,4 +1,5 @@ -//! Process-wide settings model: colour scheme, CSS entries, and profile pointers, persisted as JSON. +//! Process-wide settings model: colour scheme, CSS entries, and profiles (ticket 049 folded +//! profile storage inline here instead of a separate `profiles.json`), persisted as JSON. use std::collections::hash_map::DefaultHasher; use std::fmt; @@ -13,6 +14,7 @@ use directories::ProjectDirs; use serde::{Deserialize, Serialize}; use crate::alerts::AlertRule; +use crate::profile::Profile; /// Shared, process-wide settings handle. Cheap to clone. pub type SettingsHandle = Arc>; @@ -51,27 +53,101 @@ pub enum CssEntry { Linked { path_or_url: String }, } -/// Tracks known profile names and which one is active; the profile files themselves live elsewhere. -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, Default)] -pub struct Profiles { - /// Names of known profiles. - pub names: Vec, - /// Name of the currently active profile, if any. - pub active_profile: Option, - /// Directory (relative to the settings file's directory, or absolute) - /// where individual profile files are expected to live. - pub profiles_dir: String, +/// Legacy on-disk shape of the (pre-049) `Profiles` pointer struct: only names/active-profile/dir +/// pointers, with the actual profile content living in a separate `profiles.json`. Kept only so an +/// old `settings.json` still parses; see `ProfilesOnDisk` and `Settings::migrate_legacy_profiles_file`. +#[derive(Debug, Clone, Deserialize, Default)] +struct LegacyProfilesPointer { + // `names` is intentionally not captured: it was always redundant with the legacy + // `profiles.json`'s actual profile list, which is what migration reads instead. + #[serde(default)] + active_profile: Option, + /// Directory (relative to the settings file's directory, or absolute) the legacy `profiles.json` + /// lived under. Every version of this codebase ever wrote `"profiles"` here (no API ever let a + /// user change it), so migration doesn't bother re-deriving a custom value from this field. + #[serde(default)] + #[allow(dead_code)] + profiles_dir: Option, +} + +/// On-disk shape of the `profiles` field: either the new inline `Vec` (ticket 049) or the +/// old pointer-struct shape, so a `settings.json` written before this ticket still deserializes. +#[derive(Debug, Clone, Deserialize)] +#[serde(untagged)] +enum ProfilesOnDisk { + Inline(Vec), + Legacy(LegacyProfilesPointer), +} + +impl Default for ProfilesOnDisk { + fn default() -> Self { + ProfilesOnDisk::Inline(Vec::new()) + } +} + +/// Raw on-disk shape of `Settings`, used only to support deserializing an old `settings.json` whose +/// `profiles` field was the pointer struct and had no top-level `active_profile` — see `Settings`'s +/// `#[serde(from = ...)]`. +#[derive(Debug, Clone, Deserialize)] +struct SettingsOnDisk { + color_scheme: ColorScheme, + #[serde(default)] + css_entries: Vec, + #[serde(default)] + custom_theme_id: Option, + #[serde(default)] + profiles: ProfilesOnDisk, + #[serde(default)] + active_profile: Option, + #[serde(default)] + max_logs: Option, + #[serde(default)] + alert_rules: Vec, + #[serde(default)] + ntfy_topic: Option, + #[serde(default)] + ntfy_server: Option, +} + +impl From for Settings { + fn from(on_disk: SettingsOnDisk) -> Self { + let (profiles, legacy_active_profile) = match on_disk.profiles { + ProfilesOnDisk::Inline(profiles) => (profiles, None), + ProfilesOnDisk::Legacy(pointer) => (Vec::new(), pointer.active_profile), + }; + Settings { + color_scheme: on_disk.color_scheme, + css_entries: on_disk.css_entries, + custom_theme_id: on_disk.custom_theme_id, + profiles, + active_profile: on_disk.active_profile.or(legacy_active_profile), + max_logs: on_disk.max_logs, + alert_rules: on_disk.alert_rules, + ntfy_topic: on_disk.ntfy_topic, + ntfy_server: on_disk.ntfy_server, + } + } } /// The single, process-wide settings instance. Only `PartialEq` (not `Eq`): `alert_rules` carries `f64` fields, which aren't `Eq`. -#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +/// +/// `profiles`/`active_profile` used to be a separate `Profiles` pointer struct referencing a +/// standalone `profiles.json` (see `crate::profile::ProfileStore`, removed by ticket 049); profile +/// data now lives inline here, as the single source of truth. `Deserialize` goes through +/// `SettingsOnDisk` so a `settings.json` written before this ticket still loads (see +/// `migrate_legacy_profiles_file`, which then imports the old `profiles.json`, if any, on `load`). +#[derive(Debug, Clone, Default, PartialEq, Serialize, Deserialize)] +#[serde(from = "SettingsOnDisk")] pub struct Settings { pub color_scheme: ColorScheme, pub css_entries: Vec, /// Id of the selected custom theme entry, if any; `None` means `color_scheme` alone determines the active palette. #[serde(default)] pub custom_theme_id: Option, - pub profiles: Profiles, + /// Every known profile, inline (ticket 049). + pub profiles: Vec, + /// Name of the currently active profile, if any. + pub active_profile: Option, /// Maximum number of ingested logs kept in memory; `None` means unlimited. #[serde(default)] pub max_logs: Option, @@ -86,36 +162,20 @@ pub struct Settings { pub ntfy_server: Option, } -impl Default for Settings { - fn default() -> Self { - Settings { - color_scheme: ColorScheme::default(), - css_entries: Vec::new(), - custom_theme_id: None, - profiles: Profiles { - names: Vec::new(), - active_profile: None, - profiles_dir: "profiles".to_string(), - }, - max_logs: None, - alert_rules: Vec::new(), - ntfy_topic: None, - ntfy_server: None, - } - } -} - /// Computes the default settings file path (`/settings.json`) from OS-conventional config directories; `None` if none could be determined. pub fn default_settings_path() -> Option { ProjectDirs::from("", "", "ottyl").map(|dirs| dirs.config_dir().join("settings.json")) } impl Settings { - /// Loads settings from `path`, creating and persisting defaults if missing. Any I/O or parse error on an existing file is logged and treated as defaults, without overwriting the unreadable file. + /// Loads settings from `path`, creating and persisting defaults if missing. Any I/O or parse error on an existing file is logged and treated as defaults, without overwriting the unreadable file. Also runs the one-time legacy `profiles.json` migration (see `migrate_legacy_profiles_file`). pub fn load(path: &Path) -> Settings { match fs::read_to_string(path) { - Ok(contents) => match serde_json::from_str(&contents) { - Ok(settings) => settings, + Ok(contents) => match serde_json::from_str::(&contents) { + Ok(mut settings) => { + settings.migrate_legacy_profiles_file(path); + settings + } Err(err) => { eprintln!( "warning: failed to parse settings file at {}: {err}; using defaults", @@ -125,7 +185,8 @@ impl Settings { } }, Err(err) if err.kind() == io::ErrorKind::NotFound => { - let settings = Settings::default(); + let mut settings = Settings::default(); + settings.migrate_legacy_profiles_file(path); if let Err(save_err) = settings.save(path) { eprintln!( "warning: failed to create default settings file at {}: {save_err}", @@ -153,6 +214,43 @@ impl Settings { .map_err(|err| io::Error::new(io::ErrorKind::InvalidData, err))?; fs::write(path, contents) } + + /// One-time migration (ticket 049): if `self.profiles` is empty (a fresh `Settings`, or one + /// parsed from a pre-049 `settings.json` whose profiles lived in a separate file) and a legacy + /// `/profiles/profiles.json` exists, reads it in, merges it into + /// `self.profiles`, persists `self`, and — only once that save succeeds — renames the old file + /// (`.migrated` suffix) so it isn't re-imported on a later load. Every prior version of this + /// codebase always used the fixed `"profiles"` subdirectory name (no API ever let it be + /// reconfigured), so that's the only location checked. Best-effort: any failure is logged and + /// left for a future load to retry rather than panicking. + fn migrate_legacy_profiles_file(&mut self, settings_path: &Path) { + if !self.profiles.is_empty() { + return; + } + let legacy_path = settings_path + .parent() + .map(|parent| parent.join("profiles")) + .unwrap_or_else(|| PathBuf::from("profiles")) + .join("profiles.json"); + let Some(profiles) = crate::profile::load_legacy_profile_store(&legacy_path) else { + return; + }; + self.profiles = profiles; + if let Err(err) = self.save(settings_path) { + eprintln!( + "warning: failed to persist profiles migrated from {}: {err}", + legacy_path.display() + ); + return; + } + let migrated_path = legacy_path.with_extension("json.migrated"); + if let Err(err) = fs::rename(&legacy_path, &migrated_path) { + eprintln!( + "warning: migrated profiles from {} into settings.json, but failed to rename the old file: {err}", + legacy_path.display() + ); + } + } } /// Error returned by CSS-entry mutations and lookups. @@ -315,6 +413,20 @@ mod tests { assert_eq!(settings.color_scheme, ColorScheme::Dark); } + /// A `settings.json` written before ticket 049 (nested `Profiles` pointer struct, no top-level + /// `active_profile`) still parses: the pointer's `active_profile` carries over, and `profiles` + /// starts empty (the real content lived in a separate `profiles.json`, migrated separately by + /// `Settings::load`, not by plain deserialization). + #[test] + fn legacy_profiles_pointer_shape_deserializes_with_empty_inline_profiles() { + let settings: Settings = serde_json::from_str( + r#"{"color_scheme":"dark","css_entries":[],"custom_theme_id":null,"profiles":{"names":["web"],"active_profile":"web","profiles_dir":"profiles"},"max_logs":null,"alert_rules":[],"ntfy_topic":null,"ntfy_server":null}"#, + ) + .unwrap(); + assert!(settings.profiles.is_empty()); + assert_eq!(settings.active_profile, Some("web".to_string())); + } + #[test] fn round_trips_through_save_and_load() { let dir = tempfile::tempdir().unwrap(); @@ -329,8 +441,8 @@ mod tests { settings.css_entries.push(CssEntry::Linked { path_or_url: "https://example.com/style.css".to_string(), }); - settings.profiles.names.push("default".to_string()); - settings.profiles.active_profile = Some("default".to_string()); + settings.profiles.push(crate::profile::Profile::new("default".to_string())); + settings.active_profile = Some("default".to_string()); settings.alert_rules.push(crate::alerts::AlertRule { id: "rule1".to_string(), name: "error spike".to_string(), @@ -458,4 +570,55 @@ mod tests { let result = settings.css_content(&css_dir, &linked_id); assert!(matches!(result, Err(CssError::NotImported))); } + + /// Acceptance criterion (ticket 049): a pre-existing legacy `profiles_dir/profiles.json` is + /// merged into `settings.json` on the next load, and the old file is renamed (not re-imported on + /// a subsequent load). + #[test] + fn load_migrates_legacy_profiles_file_into_inline_settings() { + let dir = tempfile::tempdir().unwrap(); + let settings_path = dir.path().join("settings.json"); + let profiles_dir = dir.path().join("profiles"); + fs::create_dir_all(&profiles_dir).unwrap(); + let legacy_path = profiles_dir.join("profiles.json"); + fs::write( + &legacy_path, + r#"[{"name":"web","params":[],"metrics":[],"dashboards":[]}]"#, + ) + .unwrap(); + // No settings.json yet: simulates upgrading a pre-049 install that never wrote one either, + // or one that did but whose nested pointer already carried an active_profile name. + fs::write( + &settings_path, + r#"{"color_scheme":"system","css_entries":[],"custom_theme_id":null,"profiles":{"names":["web"],"active_profile":"web","profiles_dir":"profiles"},"max_logs":null,"alert_rules":[],"ntfy_topic":null,"ntfy_server":null}"#, + ) + .unwrap(); + + let settings = Settings::load(&settings_path); + assert_eq!(settings.profiles.len(), 1); + assert_eq!(settings.profiles[0].name, "web"); + assert_eq!(settings.active_profile, Some("web".to_string())); + + // The legacy file is gone (renamed), so it isn't re-imported. + assert!(!legacy_path.exists()); + let migrated_path = legacy_path.with_extension("json.migrated"); + assert!(migrated_path.exists()); + + // Reload from disk: the migrated settings.json now carries profiles inline directly. + let reloaded = Settings::load(&settings_path); + assert_eq!(reloaded.profiles.len(), 1); + assert_eq!(reloaded.profiles[0].name, "web"); + } + + /// A fresh install (no legacy file at all) never creates `profiles.json` — it's not even looked + /// for beyond the one migration check, and `profiles` simply stays empty. + #[test] + fn load_without_legacy_file_leaves_profiles_empty() { + let dir = tempfile::tempdir().unwrap(); + let settings_path = dir.path().join("settings.json"); + + let settings = Settings::load(&settings_path); + assert!(settings.profiles.is_empty()); + assert!(!dir.path().join("profiles").exists()); + } }