feat(core): dry-run mode for reclassify, and a per-kind tally
CI / Build, lint & test (push) Successful in 3m7s
CI / Build, lint & test (push) Successful in 3m7s
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.
This commit is contained in:
+10
-6
@@ -57,20 +57,24 @@ async fn main() -> Result<()> {
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
// `openfut-core reclassify <request.json>` — 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 <request.json> [--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 <request.json>")?;
|
||||
.context("usage: openfut-core reclassify <request.json> [--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(());
|
||||
|
||||
+29
-2
@@ -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<ContentKindAssignment>,
|
||||
/// 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<String>,
|
||||
/// 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<String, usize>,
|
||||
}
|
||||
|
||||
/// 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<String, usize> = 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,
|
||||
})
|
||||
}
|
||||
|
||||
@@ -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");
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user