refactor(clippy): fix the never-loop error and clear all lint warnings
`cargo clippy --all-targets` was failing on a deny-by-default `never_loop` in the analysis SSE handler (the `loop` always returned on the first iteration — the stream `unfold` already re-enters per event), plus ~28 warnings. All resolved with no behaviour change: - admin/analysis SSE: drop the dead `loop` wrapper. - app: match port literals directly instead of `if p == 80` guards. - repo/user: separate doc list from the following paragraphs. - repo/upload_history: `sort_by_key(Reverse(..))` over `sort_by`. - crawler/nav test: construct the error directly (no `unwrap_err` on a literal `Err`). - test helpers: build configs via struct-update syntax instead of `Default::default()` + field reassignment; add a type alias for a complex audit-row tuple. - plus the mechanical `deref`/etc. fixes from `cargo clippy --fix`. Note: not running `cargo fmt` — the backend uses a consistent hand-formatted style that differs from rustfmt-default across ~110 files, so a blanket reformat would be pure churn. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -1212,14 +1212,16 @@ mod tests {
|
||||
// VisionClient pointed at a closed local port. Reqwest fails the
|
||||
// POST immediately (connection refused), but the prep work runs
|
||||
// before the POST is even built.
|
||||
let mut cfg = AnalysisConfig::default();
|
||||
cfg.endpoint = "http://127.0.0.1:1/v1/chat/completions".into();
|
||||
cfg.model = "stub".into();
|
||||
cfg.request_timeout = Duration::from_secs(1);
|
||||
// Tune slice geometry to match the test image's shape.
|
||||
cfg.max_pixels = 1_000_000;
|
||||
cfg.min_slice_height = 100;
|
||||
cfg.tall_aspect_threshold = 1.8;
|
||||
let cfg = AnalysisConfig {
|
||||
endpoint: "http://127.0.0.1:1/v1/chat/completions".into(),
|
||||
model: "stub".into(),
|
||||
request_timeout: Duration::from_secs(1),
|
||||
// Tune slice geometry to match the test image's shape.
|
||||
max_pixels: 1_000_000,
|
||||
min_slice_height: 100,
|
||||
tall_aspect_threshold: 1.8,
|
||||
..Default::default()
|
||||
};
|
||||
let http = reqwest::Client::builder()
|
||||
.timeout(cfg.request_timeout)
|
||||
.no_proxy()
|
||||
|
||||
@@ -58,20 +58,20 @@ async fn stream_status(
|
||||
) -> Sse<impl Stream<Item = Result<Event, Infallible>>> {
|
||||
let rx = state.analysis_events.subscribe();
|
||||
let stream = futures_util::stream::unfold(rx, |mut rx| async move {
|
||||
loop {
|
||||
match rx.recv().await {
|
||||
Ok(ev) => {
|
||||
let event = Event::default()
|
||||
.event("analysis")
|
||||
.json_data(&ev)
|
||||
.unwrap_or_else(|_| Event::default().comment("serialize error"));
|
||||
return Some((Ok(event), rx));
|
||||
}
|
||||
Err(RecvError::Lagged(_)) => {
|
||||
return Some((Ok(Event::default().event("lagged").data("")), rx));
|
||||
}
|
||||
Err(RecvError::Closed) => return None,
|
||||
// One recv per unfold step; the stream driver re-enters for the next
|
||||
// event, so no explicit loop is needed here.
|
||||
match rx.recv().await {
|
||||
Ok(ev) => {
|
||||
let event = Event::default()
|
||||
.event("analysis")
|
||||
.json_data(&ev)
|
||||
.unwrap_or_else(|_| Event::default().comment("serialize error"));
|
||||
Some((Ok(event), rx))
|
||||
}
|
||||
Err(RecvError::Lagged(_)) => {
|
||||
Some((Ok(Event::default().event("lagged").data("")), rx))
|
||||
}
|
||||
Err(RecvError::Closed) => None,
|
||||
}
|
||||
});
|
||||
Sse::new(stream).keep_alive(KeepAlive::default())
|
||||
|
||||
@@ -255,8 +255,8 @@ async fn create(
|
||||
)
|
||||
.await?;
|
||||
|
||||
let author_refs = repo::author::set_for_manga(&mut *tx, manga.id, &authors).await?;
|
||||
repo::genre::set_for_manga(&mut *tx, manga.id, &metadata.genre_ids).await?;
|
||||
let author_refs = repo::author::set_for_manga(&mut tx, manga.id, &authors).await?;
|
||||
repo::genre::set_for_manga(&mut tx, manga.id, &metadata.genre_ids).await?;
|
||||
|
||||
if let Some(img) = cover {
|
||||
let key = format!("mangas/{}/cover.{}", manga.id, img.ext);
|
||||
@@ -321,7 +321,7 @@ async fn update(
|
||||
|
||||
let mut tx = state.db.begin().await?;
|
||||
let _updated = repo::manga::update_basics(
|
||||
&mut *tx,
|
||||
&mut tx,
|
||||
id,
|
||||
patch.title.as_deref().map(str::trim),
|
||||
patch.status.as_deref().map(str::trim),
|
||||
@@ -331,10 +331,10 @@ async fn update(
|
||||
)
|
||||
.await?;
|
||||
if let Some(ref names) = authors_owned {
|
||||
repo::author::set_for_manga(&mut *tx, id, names).await?;
|
||||
repo::author::set_for_manga(&mut tx, id, names).await?;
|
||||
}
|
||||
if let Some(ref ids) = patch.genre_ids {
|
||||
repo::genre::set_for_manga(&mut *tx, id, ids).await?;
|
||||
repo::genre::set_for_manga(&mut tx, id, ids).await?;
|
||||
}
|
||||
tx.commit().await?;
|
||||
|
||||
|
||||
@@ -1266,8 +1266,8 @@ fn parse_origin(raw: &str) -> Option<String> {
|
||||
let host = url.host_str()?;
|
||||
let scheme = url.scheme();
|
||||
let port_str = match (url.port(), scheme) {
|
||||
(Some(p), "http") if p == 80 => String::new(),
|
||||
(Some(p), "https") if p == 443 => String::new(),
|
||||
(Some(80), "http") => String::new(),
|
||||
(Some(443), "https") => String::new(),
|
||||
(Some(p), _) => format!(":{p}"),
|
||||
(None, _) => String::new(),
|
||||
};
|
||||
@@ -1509,7 +1509,6 @@ mod tests {
|
||||
let storage_dir = tempfile::tempdir().unwrap();
|
||||
let storage: Arc<dyn Storage> =
|
||||
Arc::new(LocalStorage::new(storage_dir.path()));
|
||||
let mut cfg = crate::config::AnalysisConfig::default();
|
||||
// The worker always runs OCR now (vision is dormant — see
|
||||
// `effective_backend`), and the `.rten` models aren't shipped to unit
|
||||
// CI. Point the engine at a path that can't exist so the *engine build*
|
||||
@@ -1518,10 +1517,13 @@ mod tests {
|
||||
// readiness, so the row must still be reclaimed even though spawn
|
||||
// returns Err. That's exactly the regression this test guards (an
|
||||
// analysis-only deploy must reclaim orphaned leases at startup).
|
||||
cfg.ocr_detection_model = "/nonexistent/text-detection.rten".to_string();
|
||||
cfg.ocr_recognition_model = "/nonexistent/text-recognition.rten".to_string();
|
||||
cfg.workers = 1;
|
||||
cfg.job_timeout = Duration::from_secs(1);
|
||||
let cfg = crate::config::AnalysisConfig {
|
||||
ocr_detection_model: "/nonexistent/text-detection.rten".to_string(),
|
||||
ocr_recognition_model: "/nonexistent/text-recognition.rten".to_string(),
|
||||
workers: 1,
|
||||
job_timeout: Duration::from_secs(1),
|
||||
..Default::default()
|
||||
};
|
||||
let events = Arc::new(crate::analysis::events::AnalysisEvents::new());
|
||||
|
||||
let spawned = spawn_analysis_daemon(pool.clone(), storage, &cfg, events).await;
|
||||
|
||||
@@ -876,8 +876,10 @@ mod tests {
|
||||
// what `ANALYSIS_BACKEND` parsed to. `backend` still reflects the raw
|
||||
// request (so the override is visible/loggable), but `effective_backend`
|
||||
// is the value the daemon actually dispatches through.
|
||||
let mut cfg = AnalysisConfig::default();
|
||||
cfg.backend = AnalysisBackend::Vision;
|
||||
let mut cfg = AnalysisConfig {
|
||||
backend: AnalysisBackend::Vision,
|
||||
..Default::default()
|
||||
};
|
||||
assert_eq!(cfg.effective_backend(), AnalysisBackend::Ocr);
|
||||
cfg.backend = AnalysisBackend::Ocr;
|
||||
assert_eq!(cfg.effective_backend(), AnalysisBackend::Ocr);
|
||||
|
||||
@@ -164,8 +164,7 @@ mod tests {
|
||||
|
||||
#[test]
|
||||
fn anyhow_with_nav_timeout_in_chain_is_flagged() {
|
||||
let inner: Result<(), NavError> = Err(NavError::Timeout(NAV_TIMEOUT));
|
||||
let outer = inner.unwrap_err();
|
||||
let outer = NavError::Timeout(NAV_TIMEOUT);
|
||||
let wrapped: anyhow::Error =
|
||||
anyhow::Error::new(outer).context("wait for chapter nav");
|
||||
assert!(anyhow_looks_browser_dead(&wrapped));
|
||||
|
||||
@@ -110,7 +110,7 @@ pub async fn list_for_user(
|
||||
});
|
||||
}
|
||||
// Newest first; trim to limit after the merge.
|
||||
entries.sort_by(|a, b| b.created_at().cmp(&a.created_at()));
|
||||
entries.sort_by_key(|b| std::cmp::Reverse(b.created_at()));
|
||||
entries.truncate(limit as usize);
|
||||
|
||||
let (manga_total, chapter_total): (i64, i64) = sqlx::query_as(
|
||||
|
||||
@@ -157,8 +157,10 @@ pub async fn set_is_admin_unchecked(pool: &PgPool, id: Uuid, value: bool) -> App
|
||||
/// - If a row already exists: flip `is_admin` to true if needed; **never**
|
||||
/// touch the existing `password_hash`. Lets the operator rotate the
|
||||
/// admin password through the UI without env-var conflict.
|
||||
///
|
||||
/// Wrapped in a transaction so a concurrent `register` for the same
|
||||
/// username can't slip an INSERT between the SELECT and UPDATE/INSERT.
|
||||
///
|
||||
/// Set `is_admin` on a user with full safety checks: rejects self-demote,
|
||||
/// rejects demoting the only remaining admin (under `ADMIN_INVARIANT_LOCK_KEY`
|
||||
/// to close the parallel-demote race), and writes an `admin_audit` row
|
||||
|
||||
@@ -567,11 +567,13 @@ mod tests {
|
||||
|
||||
#[test]
|
||||
fn crawler_round_trips_through_dto() {
|
||||
let mut base = CrawlerConfig::default();
|
||||
base.start_url = Some("https://example.com/".to_string());
|
||||
base.tz = Tz::Europe__Berlin;
|
||||
base.chapter_workers = 3;
|
||||
base.cookie_domain = Some("example.com".to_string());
|
||||
let base = CrawlerConfig {
|
||||
start_url: Some("https://example.com/".to_string()),
|
||||
tz: Tz::Europe__Berlin,
|
||||
chapter_workers: 3,
|
||||
cookie_domain: Some("example.com".to_string()),
|
||||
..Default::default()
|
||||
};
|
||||
let dto = CrawlerSettings::from_config(&base);
|
||||
let back = dto.to_config(&base).expect("valid");
|
||||
assert_eq!(back.tz, Tz::Europe__Berlin);
|
||||
@@ -597,10 +599,12 @@ mod tests {
|
||||
|
||||
#[test]
|
||||
fn crawler_overlay_preserves_env_only_fields() {
|
||||
let mut base = CrawlerConfig::default();
|
||||
base.proxy = Some("socks5://127.0.0.1:9050".to_string());
|
||||
base.tor_control_password = Some("secret".to_string());
|
||||
base.phpsessid = Some("abc123".to_string());
|
||||
let base = CrawlerConfig {
|
||||
proxy: Some("socks5://127.0.0.1:9050".to_string()),
|
||||
tor_control_password: Some("secret".to_string()),
|
||||
phpsessid: Some("abc123".to_string()),
|
||||
..Default::default()
|
||||
};
|
||||
// A DTO that knows nothing about the env-only fields.
|
||||
let dto = CrawlerSettings {
|
||||
rate_ms: 2000,
|
||||
@@ -704,8 +708,10 @@ mod tests {
|
||||
|
||||
#[test]
|
||||
fn analysis_captures_env_prompt_override() {
|
||||
let mut base = AnalysisConfig::default();
|
||||
base.system_prompt = "custom env prompt".to_string();
|
||||
let base = AnalysisConfig {
|
||||
system_prompt: "custom env prompt".to_string(),
|
||||
..Default::default()
|
||||
};
|
||||
let dto = AnalysisSettings::from_config(&base);
|
||||
assert_eq!(dto.system_prompt.as_deref(), Some("custom env prompt"));
|
||||
}
|
||||
@@ -729,8 +735,10 @@ mod tests {
|
||||
|
||||
#[test]
|
||||
fn analysis_overlay_preserves_api_key() {
|
||||
let mut base = AnalysisConfig::default();
|
||||
base.api_key = Some("sk-secret".to_string());
|
||||
let base = AnalysisConfig {
|
||||
api_key: Some("sk-secret".to_string()),
|
||||
..Default::default()
|
||||
};
|
||||
let dto = AnalysisSettings::from_config(&base);
|
||||
assert_eq!(dto.to_config(&base).unwrap().api_key.as_deref(), Some("sk-secret"));
|
||||
}
|
||||
@@ -872,8 +880,10 @@ mod tests {
|
||||
// a hostile/CSRF-able admin must NOT be able to point endpoint at
|
||||
// cloud metadata, loopback services, or RFC1918 hosts — when the
|
||||
// worker is enabled (toggling enabled=true later re-runs this gate).
|
||||
let mut base = AnalysisConfig::default();
|
||||
base.backend = AnalysisBackend::Vision;
|
||||
let base = AnalysisConfig {
|
||||
backend: AnalysisBackend::Vision,
|
||||
..Default::default()
|
||||
};
|
||||
for url in [
|
||||
"http://169.254.169.254/v1/chat/completions",
|
||||
"http://127.0.0.1:5432/",
|
||||
@@ -897,8 +907,10 @@ mod tests {
|
||||
// The documented default — docker DNS name resolving to a private IP
|
||||
// at runtime — must still validate, because the bearer recipient
|
||||
// identity is the operator-chosen hostname, not the underlying IP.
|
||||
let mut base = AnalysisConfig::default();
|
||||
base.backend = AnalysisBackend::Vision;
|
||||
let base = AnalysisConfig {
|
||||
backend: AnalysisBackend::Vision,
|
||||
..Default::default()
|
||||
};
|
||||
for url in [
|
||||
"http://mangalord-vision:8000/v1/chat/completions",
|
||||
"https://api.openai.com/v1/chat/completions",
|
||||
@@ -931,8 +943,10 @@ mod tests {
|
||||
fn analysis_requires_model_only_when_enabled() {
|
||||
// Vision base: the model id is required only when the vision worker
|
||||
// is actually live (see the OCR carve-out below).
|
||||
let mut base = AnalysisConfig::default();
|
||||
base.backend = AnalysisBackend::Vision;
|
||||
let base = AnalysisConfig {
|
||||
backend: AnalysisBackend::Vision,
|
||||
..Default::default()
|
||||
};
|
||||
let disabled = AnalysisSettings {
|
||||
enabled: false,
|
||||
model: "".to_string(),
|
||||
|
||||
@@ -279,7 +279,7 @@ async fn backfill_fills_unmeasured_pages_and_covers_idempotently(pool: PgPool) {
|
||||
let body2 = common::body_json(resp2).await;
|
||||
assert_eq!(body2["pages"].as_i64().unwrap(), 0);
|
||||
assert_eq!(body2["covers"].as_i64().unwrap(), 0);
|
||||
assert_eq!(body2["more_remaining"].as_bool().unwrap(), false);
|
||||
assert!(!body2["more_remaining"].as_bool().unwrap());
|
||||
}
|
||||
|
||||
#[sqlx::test(migrations = "./migrations")]
|
||||
@@ -362,7 +362,7 @@ async fn backfill_caps_per_run_and_reports_more_remaining(pool: PgPool) {
|
||||
assert_eq!(resp.status(), StatusCode::OK);
|
||||
let body = common::body_json(resp).await;
|
||||
// Capped at 20_000 attempts this run; one row left → run again.
|
||||
assert_eq!(body["more_remaining"].as_bool().unwrap(), true);
|
||||
assert!(body["more_remaining"].as_bool().unwrap());
|
||||
assert_eq!(body["missing"].as_i64().unwrap(), 20_000);
|
||||
assert_eq!(body["pages"].as_i64().unwrap(), 0);
|
||||
}
|
||||
@@ -403,9 +403,8 @@ async fn backfill_at_exact_cap_is_not_more_remaining(pool: PgPool) {
|
||||
.unwrap();
|
||||
let body = common::body_json(resp).await;
|
||||
assert_eq!(body["missing"].as_i64().unwrap(), 20_000);
|
||||
assert_eq!(
|
||||
body["more_remaining"].as_bool().unwrap(),
|
||||
false,
|
||||
assert!(
|
||||
!body["more_remaining"].as_bool().unwrap(),
|
||||
"exactly-cap backlog drains in one run; no follow-up needed"
|
||||
);
|
||||
}
|
||||
|
||||
@@ -381,10 +381,11 @@ async fn delete_writes_audit_row(pool: PgPool) {
|
||||
.unwrap();
|
||||
assert_eq!(resp.status(), StatusCode::NO_CONTENT);
|
||||
|
||||
let rows: Vec<(Option<Uuid>, String, String, Option<Uuid>, serde_json::Value)> =
|
||||
sqlx::query_as(
|
||||
"SELECT actor_user_id, action, target_kind, target_id, payload FROM admin_audit",
|
||||
)
|
||||
// (actor_user_id, action, target_kind, target_id, payload)
|
||||
type AuditRow = (Option<Uuid>, String, String, Option<Uuid>, serde_json::Value);
|
||||
let rows: Vec<AuditRow> = sqlx::query_as(
|
||||
"SELECT actor_user_id, action, target_kind, target_id, payload FROM admin_audit",
|
||||
)
|
||||
.fetch_all(&pool)
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
@@ -446,7 +446,7 @@ async fn list_tie_break_orders_equal_keys_by_ascending_id(pool: PgPool) {
|
||||
|
||||
// Paginating one row at a time reproduces that exact sequence — no overlap,
|
||||
// no gap — which only holds because the id tie-break makes the order total.
|
||||
let paged: Vec<String> = vec![page(1, 0).await, page(1, 1).await, page(1, 2).await]
|
||||
let paged: Vec<String> = [page(1, 0).await, page(1, 1).await, page(1, 2).await]
|
||||
.iter()
|
||||
.map(|b| b["items"][0]["title"].as_str().unwrap().to_string())
|
||||
.collect();
|
||||
|
||||
@@ -279,7 +279,7 @@ async fn tag_autocomplete_returns_matches_ordered_by_similarity(pool: PgPool) {
|
||||
.iter()
|
||||
.map(|t| t["name"].as_str().unwrap())
|
||||
.collect();
|
||||
assert!(names.iter().any(|n| *n == "Mystery"));
|
||||
assert!(names.iter().any(|n| *n == "Murder Mystery"));
|
||||
assert!(!names.iter().any(|n| *n == "Comedy"));
|
||||
assert!(names.contains(&"Mystery"));
|
||||
assert!(names.contains(&"Murder Mystery"));
|
||||
assert!(!names.contains(&"Comedy"));
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user