refactor: consolidate bulk actions, URL assembly, and case-artefact filenames
Pulls three pattern groups into shared helpers so a rename or wire-format tweak edits one file instead of 10+: - BulkAction enum in doctate-common replaces the "close"/"analyze"/"reset" string literals that were duplicated between the bulk handler and 3 attack/CSRF test files. FromStr preserves the exact "Unbekannte Aktion" error shape; the handler match is now exhaustive over the enum. - join_url helper in doctate-common absorbs 7 identical trim_end_matches+format! call sites across client-core, client-desktop, server/transcribe (ollama, whisper), and server/analyze (llm). - server/tests/common/artefacts.rs re-exports ONELINER_FILENAME (doctate-common), DOCUMENT_FILE + ANALYSIS_INPUT_FILE (doctate-server::analyze), and CLOSE_MARKER (doctate-server::paths) so test files reference the canonical name instead of inlining literals. Also lifts client-desktop local duplication: - paths.rs: project_path helper collapses 4 identical ProjectDirs chains - main.rs: or_die helper replaces 4 eprintln!+exit(1) blocks - app.rs: named RecordingContext struct replaces (Uuid, String) tuple at 3 sites around the ffmpeg-flush finalization path Verification: 397 tests pass (baseline was 389; +8 for new unit tests on BulkAction and join_url), 0 failed, 4 ignored (unchanged). clippy clean.
This commit is contained in:
@@ -18,6 +18,7 @@ use doctate_client_core::{
|
||||
PendingUpload, ServerSync, SnapshotCache, SyncConfig, UploadEvent, WorkerSnapshot, WorkerState,
|
||||
pick_footer_status, run_startup_cleanup, write_sidecar,
|
||||
};
|
||||
use doctate_common::join_url;
|
||||
use doctate_common::oneliners::OnelinerState;
|
||||
use doctate_common::timestamp::{extract_hhmm, now_rfc3339, recorded_at_to_filename_stem};
|
||||
use eframe::egui;
|
||||
@@ -55,7 +56,15 @@ pub struct DoctateApp {
|
||||
/// for `AppState::Idle` on Stop (so the UI stays responsive), but we
|
||||
/// still need `case_id` + `recorded_at` when the recorder emits
|
||||
/// `Finished`. `None` when no flush is in progress.
|
||||
finalizing: Option<(Uuid, String)>,
|
||||
finalizing: Option<RecordingContext>,
|
||||
}
|
||||
|
||||
/// Enough recording identity to finalize a pending flush or emit an
|
||||
/// upload event once the recorder signals `Finished`.
|
||||
#[derive(Debug, Clone)]
|
||||
struct RecordingContext {
|
||||
case_id: Uuid,
|
||||
recorded_at: String,
|
||||
}
|
||||
|
||||
enum UiAction {
|
||||
@@ -273,7 +282,10 @@ impl DoctateApp {
|
||||
}
|
||||
// UI is ready for the next recording immediately — the finalizing
|
||||
// ffmpeg flush shows up as a footer spinner, not a blocking state.
|
||||
self.finalizing = Some((case_id, recorded_at));
|
||||
self.finalizing = Some(RecordingContext {
|
||||
case_id,
|
||||
recorded_at,
|
||||
});
|
||||
self.state = AppState::Idle;
|
||||
}
|
||||
|
||||
@@ -281,11 +293,11 @@ impl DoctateApp {
|
||||
let Some(cfg) = &self.config else {
|
||||
return;
|
||||
};
|
||||
let server_url = cfg.server_url.trim_end_matches('/').to_owned();
|
||||
let server_url = cfg.server_url.clone();
|
||||
let api_key = cfg.api_key.clone();
|
||||
let http = self.http_client.clone();
|
||||
let return_to = format!("/web/cases/{case_id}");
|
||||
let fallback_url = format!("{server_url}{return_to}");
|
||||
let fallback_url = join_url(&server_url, &return_to);
|
||||
|
||||
// Fire-and-forget on the runtime. Blocking the egui UI thread on
|
||||
// an HTTP round-trip would freeze the window for ~100–500ms.
|
||||
@@ -337,17 +349,20 @@ impl DoctateApp {
|
||||
// Prefer the explicit finalizing context (Stop-flow). Fall back
|
||||
// to AppState::Recording (recorder terminated before Stop — rare
|
||||
// but possible).
|
||||
let (case_id, recorded_at) = if let Some((id, rec)) = self.finalizing.take() {
|
||||
(id, rec)
|
||||
let ctx = if let Some(ctx) = self.finalizing.take() {
|
||||
ctx
|
||||
} else if let AppState::Recording {
|
||||
case_id,
|
||||
recorded_at,
|
||||
..
|
||||
} = &self.state
|
||||
{
|
||||
let pair = (*case_id, recorded_at.clone());
|
||||
let ctx = RecordingContext {
|
||||
case_id: *case_id,
|
||||
recorded_at: recorded_at.clone(),
|
||||
};
|
||||
self.state = AppState::Idle;
|
||||
pair
|
||||
ctx
|
||||
} else {
|
||||
warn!(state = ?self.state, "recorder finished in unexpected state");
|
||||
self.recorder_events = None;
|
||||
@@ -355,6 +370,11 @@ impl DoctateApp {
|
||||
return;
|
||||
};
|
||||
|
||||
let RecordingContext {
|
||||
case_id,
|
||||
recorded_at,
|
||||
} = ctx;
|
||||
|
||||
let store = self.case_store.clone();
|
||||
let now = time::OffsetDateTime::now_utc();
|
||||
self.runtime.block_on(async move {
|
||||
|
||||
@@ -11,7 +11,7 @@
|
||||
//! plain case URL if this fails — the doctor lands on the login page and
|
||||
//! signs in manually rather than seeing a dead button.
|
||||
|
||||
use doctate_common::API_KEY_HEADER;
|
||||
use doctate_common::{API_KEY_HEADER, join_url};
|
||||
use serde::{Deserialize, Serialize};
|
||||
use thiserror::Error;
|
||||
|
||||
@@ -42,9 +42,8 @@ pub async fn build_magic_url(
|
||||
api_key: &str,
|
||||
return_to: &str,
|
||||
) -> Result<String, MagicLinkError> {
|
||||
let base = server_url.trim_end_matches('/');
|
||||
let resp = http
|
||||
.post(format!("{base}/api/auth/magic-link"))
|
||||
.post(join_url(server_url, "/api/auth/magic-link"))
|
||||
.header(API_KEY_HEADER, api_key)
|
||||
.json(&CreateRequest { return_to })
|
||||
.send()
|
||||
@@ -55,7 +54,10 @@ pub async fn build_magic_url(
|
||||
}
|
||||
|
||||
let parsed: CreateResponse = resp.json().await?;
|
||||
Ok(format!("{base}/web/magic?token={}", parsed.token))
|
||||
Ok(join_url(
|
||||
server_url,
|
||||
&format!("/web/magic?token={}", parsed.token),
|
||||
))
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
|
||||
+20
-28
@@ -16,6 +16,19 @@ use std::fs::{File, OpenOptions, TryLockError};
|
||||
use crate::app::DoctateApp;
|
||||
use crate::paths::{cases_dir, lock_file_path, pending_dir, snapshot_cache_path};
|
||||
|
||||
/// Unwrap a startup `Result` or print `context: {err}` to stderr and
|
||||
/// `exit(1)`. Used for unrecoverable bootstrap failures before the
|
||||
/// egui event loop runs — there is no UI to show an error in.
|
||||
fn or_die<T, E: std::fmt::Display>(result: Result<T, E>, context: &str) -> T {
|
||||
match result {
|
||||
Ok(v) => v,
|
||||
Err(e) => {
|
||||
eprintln!("{context}: {e}");
|
||||
std::process::exit(1);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn main() -> eframe::Result {
|
||||
tracing_subscriber::fmt()
|
||||
.with_env_filter(
|
||||
@@ -34,33 +47,18 @@ fn main() -> eframe::Result {
|
||||
.expect("build tokio runtime"),
|
||||
);
|
||||
|
||||
let pending = match pending_dir() {
|
||||
Ok(p) => p,
|
||||
Err(e) => {
|
||||
eprintln!("cannot determine pending directory: {e}");
|
||||
std::process::exit(1);
|
||||
}
|
||||
};
|
||||
let pending = or_die(pending_dir(), "cannot determine pending directory");
|
||||
|
||||
// Single-instance guard. The lock is held by the `_lock_file` binding
|
||||
// for the entire `main` lifetime — OS releases it automatically on
|
||||
// process exit or crash, so a stale lock file after a crash is
|
||||
// harmless (the next launch re-acquires).
|
||||
let _lock_file = acquire_single_instance_lock();
|
||||
let cases = match cases_dir() {
|
||||
Ok(p) => p,
|
||||
Err(e) => {
|
||||
eprintln!("cannot determine cases directory: {e}");
|
||||
std::process::exit(1);
|
||||
}
|
||||
};
|
||||
let cache_path = match snapshot_cache_path() {
|
||||
Ok(p) => p,
|
||||
Err(e) => {
|
||||
eprintln!("cannot determine snapshot cache path: {e}");
|
||||
std::process::exit(1);
|
||||
}
|
||||
};
|
||||
let cases = or_die(cases_dir(), "cannot determine cases directory");
|
||||
let cache_path = or_die(
|
||||
snapshot_cache_path(),
|
||||
"cannot determine snapshot cache path",
|
||||
);
|
||||
|
||||
let options = eframe::NativeOptions {
|
||||
viewport: egui::ViewportBuilder::default()
|
||||
@@ -84,13 +82,7 @@ fn main() -> eframe::Result {
|
||||
/// handle; dropping it (process exit) releases the lock. If another
|
||||
/// instance already holds it, prints a friendly message and exits.
|
||||
fn acquire_single_instance_lock() -> File {
|
||||
let path = match lock_file_path() {
|
||||
Ok(p) => p,
|
||||
Err(e) => {
|
||||
eprintln!("cannot determine lock file path: {e}");
|
||||
std::process::exit(1);
|
||||
}
|
||||
};
|
||||
let path = or_die(lock_file_path(), "cannot determine lock file path");
|
||||
if let Some(parent) = path.parent()
|
||||
&& let Err(e) = std::fs::create_dir_all(parent)
|
||||
{
|
||||
|
||||
+28
-12
@@ -5,45 +5,61 @@ use std::path::PathBuf;
|
||||
use directories::ProjectDirs;
|
||||
use thiserror::Error;
|
||||
|
||||
/// Directory name for pending (not-yet-uploaded) recordings inside the
|
||||
/// data-local root.
|
||||
const PENDING_DIR_NAME: &str = "pending";
|
||||
|
||||
/// Directory name for per-case marker files inside the data-local root.
|
||||
const CASES_DIR_NAME: &str = "cases";
|
||||
|
||||
/// File name for the oneliners snapshot cache inside the data-local root.
|
||||
const SNAPSHOT_CACHE_FILENAME: &str = "oneliners_cache.json";
|
||||
|
||||
/// File name for the single-instance advisory lock inside the data-local
|
||||
/// root.
|
||||
const LOCK_FILENAME: &str = "client.lock";
|
||||
|
||||
#[derive(Debug, Error)]
|
||||
pub enum PathError {
|
||||
#[error("cannot determine OS data directory (is $HOME set?)")]
|
||||
NoDataDir,
|
||||
}
|
||||
|
||||
/// Resolve `{data_local_dir}/{rel}` under the doctate project scope, or
|
||||
/// error if the OS won't give us a data directory at all. Every public
|
||||
/// path helper here delegates to this so the `ProjectDirs::from(…)`
|
||||
/// triple stays in one place.
|
||||
fn project_path(rel: &str) -> Result<PathBuf, PathError> {
|
||||
ProjectDirs::from("", "", "doctate")
|
||||
.map(|dirs| dirs.data_local_dir().join(rel))
|
||||
.ok_or(PathError::NoDataDir)
|
||||
}
|
||||
|
||||
/// Directory where recordings wait for upload. Persists across app
|
||||
/// restarts and crashes.
|
||||
/// Linux: `~/.local/share/doctate/pending/`
|
||||
/// Windows: `%LOCALAPPDATA%\doctate\pending\`
|
||||
/// macOS: `~/Library/Application Support/doctate/pending/`
|
||||
pub fn pending_dir() -> Result<PathBuf, PathError> {
|
||||
ProjectDirs::from("", "", "doctate")
|
||||
.map(|dirs| dirs.data_local_dir().join("pending"))
|
||||
.ok_or(PathError::NoDataDir)
|
||||
project_path(PENDING_DIR_NAME)
|
||||
}
|
||||
|
||||
/// Directory holding one JSON marker per known case (locally created
|
||||
/// and/or server-synced). Persists across app restarts.
|
||||
/// Linux: `~/.local/share/doctate/cases/`
|
||||
pub fn cases_dir() -> Result<PathBuf, PathError> {
|
||||
ProjectDirs::from("", "", "doctate")
|
||||
.map(|dirs| dirs.data_local_dir().join("cases"))
|
||||
.ok_or(PathError::NoDataDir)
|
||||
project_path(CASES_DIR_NAME)
|
||||
}
|
||||
|
||||
/// File path for the last-successful `/api/oneliners` response. Feeds
|
||||
/// the UI at cold start before the first live poll completes.
|
||||
pub fn snapshot_cache_path() -> Result<PathBuf, PathError> {
|
||||
ProjectDirs::from("", "", "doctate")
|
||||
.map(|dirs| dirs.data_local_dir().join("oneliners_cache.json"))
|
||||
.ok_or(PathError::NoDataDir)
|
||||
project_path(SNAPSHOT_CACHE_FILENAME)
|
||||
}
|
||||
|
||||
/// File path for the single-instance advisory lock. Held by the running
|
||||
/// process for its entire lifetime; a second launch attempting to lock
|
||||
/// the same file gets `WouldBlock` and exits cleanly.
|
||||
pub fn lock_file_path() -> Result<PathBuf, PathError> {
|
||||
ProjectDirs::from("", "", "doctate")
|
||||
.map(|dirs| dirs.data_local_dir().join("client.lock"))
|
||||
.ok_or(PathError::NoDataDir)
|
||||
project_path(LOCK_FILENAME)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user