From 233df1d99d856ad95c89a8d31c66beff6e2cb34f Mon Sep 17 00:00:00 2001 From: funman300 Date: Fri, 21 Aug 2026 21:23:43 +0000 Subject: [PATCH] feat(core): dry-run mode for reclassify, and a per-kind tally MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Production is frozen and sits on schema 19, so the content_kind correction it needs cannot be applied yet. `--dry-run` runs the real UPDATEs and rolls the transaction back, so an operator can see exactly what a run would touch before touching a database they cannot easily restore — the counts are measured, not predicted. `updated_by_kind` reports the SHAPE of the change ("consumable 17, staff 3"), which is the thing worth sanity-checking: a different shape means the source profile moved and the mapping needs regenerating. Measured against a copy of prod-core.db: 20 rows would change (17 consumable, 3 staff), 1966 already correct, 0 unmatched, and the copy was verified unchanged afterwards. --- src/main.rs | 16 +++++---- src/services/import.rs | 31 ++++++++++++++-- tests/import_service_test.rs | 68 ++++++++++++++++++++++++++++++++++++ 3 files changed, 107 insertions(+), 8 deletions(-) diff --git a/src/main.rs b/src/main.rs index fabc562..23351f1 100644 --- a/src/main.rs +++ b/src/main.rs @@ -57,20 +57,24 @@ async fn main() -> Result<()> { return Ok(()); } - // `openfut-core reclassify ` — correct the content_kind of - // already-imported owned rows. A profile import is once-only, so a taxonomy - // fix cannot arrive by re-importing; the adapter supplies card_id -> kind - // because only it can map its own taxonomy. Idempotent. + // `openfut-core reclassify [--dry-run]` — correct the + // content_kind of already-imported owned rows. A profile import is + // once-only, so a taxonomy fix cannot arrive by re-importing; the adapter + // supplies card_id -> kind because only it can map its own taxonomy. + // Idempotent. `--dry-run` runs the same statements and rolls back, so an + // operator can see what a production run would touch before it touches it. if std::env::args().nth(1).as_deref() == Some("reclassify") { let path = std::env::args() .nth(2) - .context("usage: openfut-core reclassify ")?; + .context("usage: openfut-core reclassify [--dry-run]")?; + let dry_run = std::env::args().any(|a| a == "--dry-run"); let pool = db::init_pool(&cfg.database_url, cfg.max_connections).await?; db::run_migrations(&pool).await?; let raw = std::fs::read_to_string(&path) .with_context(|| format!("read reclassify request {path}"))?; - let req: openfut_core::services::import::ReclassifyRequest = + let mut req: openfut_core::services::import::ReclassifyRequest = serde_json::from_str(&raw).context("parse reclassify request JSON")?; + req.dry_run |= dry_run; let outcome = openfut_core::services::import::reclassify_owned_content(&pool, &req).await?; println!("{}", serde_json::to_string_pretty(&outcome)?); return Ok(()); diff --git a/src/services/import.rs b/src/services/import.rs index 8bfcf83..60f6382 100644 --- a/src/services/import.rs +++ b/src/services/import.rs @@ -19,6 +19,8 @@ //! non-imported profile is never clobbered. //! - The whole thing commits together or not at all. +use std::collections::BTreeMap; + use crate::db::Pool; use crate::models::card::ContentKind; use crate::models::game_ext::{MAX_EXT_NAMESPACE_LEN, MAX_EXT_PAYLOAD_BYTES}; @@ -385,16 +387,26 @@ pub struct ContentKindAssignment { pub struct ReclassifyRequest { pub game_id: String, pub assignments: Vec, + /// Compute the outcome and roll back instead of committing. Lets an operator + /// see exactly what a production run would touch before it touches it. + #[serde(default)] + pub dry_run: bool, } #[derive(Debug, Serialize, PartialEq, Eq)] pub struct ReclassifyOutcome { - /// Rows whose `content_kind` actually changed. + /// Rows whose `content_kind` actually changed. On a dry run, the rows that + /// WOULD change; nothing is committed. pub updated: usize, /// Rows already carrying the requested kind (a rerun updates nothing). pub unchanged: usize, /// Assignments naming a definition this game owns no copy of. pub unmatched_definitions: Vec, + /// True when the transaction was rolled back rather than committed. + pub dry_run: bool, + /// Per-kind tally of the rows that changed, so an operator can sanity-check + /// the shape of the change ("3 staff, 17 consumable") before committing. + pub updated_by_kind: BTreeMap, } /// Correct the `content_kind` of ALREADY-IMPORTED owned rows, in one transaction. @@ -419,6 +431,7 @@ pub async fn reclassify_owned_content( let mut updated = 0usize; let mut unchanged = 0usize; let mut unmatched = Vec::new(); + let mut by_kind: BTreeMap = BTreeMap::new(); for a in &req.assignments { // Scope by game through the owning club, so the same definition id in // another game is never touched. @@ -452,11 +465,25 @@ pub async fn reclassify_owned_content( .rows_affected() as usize; updated += changed; unchanged += present as usize - changed; + if changed > 0 { + *by_kind + .entry(a.content_kind.as_str().to_string()) + .or_default() += changed; + } + } + // A dry run does the real UPDATEs and then throws them away, so the counts + // it reports are measured rather than predicted — the same statements, the + // same WHERE clauses, just no commit. + if req.dry_run { + tx.rollback().await?; + } else { + tx.commit().await?; } - tx.commit().await?; Ok(ReclassifyOutcome { updated, unchanged, unmatched_definitions: unmatched, + dry_run: req.dry_run, + updated_by_kind: by_kind, }) } diff --git a/tests/import_service_test.rs b/tests/import_service_test.rs index 2999666..4ee6b4c 100644 --- a/tests/import_service_test.rs +++ b/tests/import_service_test.rs @@ -325,6 +325,7 @@ async fn reclassify_corrects_already_imported_rows_and_is_idempotent() { let rc = ReclassifyRequest { game_id: "g_reclass".into(), + dry_run: false, assignments: vec![ ContentKindAssignment { card_id: ids[0].clone(), @@ -388,6 +389,7 @@ async fn reclassify_never_crosses_a_game_boundary() { &pool, &ReclassifyRequest { game_id: "g_a".into(), + dry_run: false, assignments: vec![ContentKindAssignment { card_id: ids[0].clone(), content_kind: ContentKind::Kit, @@ -409,3 +411,69 @@ async fn reclassify_never_crosses_a_game_boundary() { .unwrap(); assert_eq!(kinds, vec!["player".to_string()], "game B untouched"); } + +/// A dry run must report the SAME counts a real run would, and leave the +/// database byte-for-byte unchanged. It runs the real UPDATEs and rolls back, so +/// the numbers are measured rather than predicted — which is the only reason an +/// operator can trust them before touching a frozen production database. +#[tokio::test] +async fn reclassify_dry_run_reports_the_real_counts_and_commits_nothing() { + use openfut_core::services::import::{ + reclassify_owned_content, ContentKindAssignment, ReclassifyRequest, + }; + + let pool = fresh_pool().await; + let db = CardDb::load("data").unwrap(); + let ids = valid_ids(3); + apply_profile_import(&pool, &db, &request("g_dry", "fp-dry", owned(&ids), None)) + .await + .expect("import"); + + let kinds = || { + let pool = pool.clone(); + async move { + sqlx::query_scalar::<_, String>("SELECT content_kind FROM owned_cards ORDER BY card_id") + .fetch_all(&pool) + .await + .unwrap() + } + }; + let before = kinds().await; + + let mut rc = ReclassifyRequest { + game_id: "g_dry".into(), + dry_run: true, + assignments: vec![ + ContentKindAssignment { + card_id: ids[0].clone(), + content_kind: ContentKind::Staff, + }, + ContentKindAssignment { + card_id: ids[1].clone(), + content_kind: ContentKind::Consumable, + }, + ], + }; + + let dry = reclassify_owned_content(&pool, &rc).await.expect("dry run"); + assert!(dry.dry_run); + assert_eq!(dry.updated, 2); + assert_eq!( + dry.updated_by_kind, + [("consumable".to_string(), 1), ("staff".to_string(), 1)] + .into_iter() + .collect(), + "the per-kind shape is what an operator sanity-checks" + ); + assert_eq!(kinds().await, before, "a dry run commits NOTHING"); + + // The real run then reports exactly what the dry run promised. + rc.dry_run = false; + let real = reclassify_owned_content(&pool, &rc) + .await + .expect("real run"); + assert!(!real.dry_run); + assert_eq!(real.updated, dry.updated); + assert_eq!(real.updated_by_kind, dry.updated_by_kind); + assert_ne!(kinds().await, before, "the real run DID commit"); +}