fix(matches): close the second, unguarded match-economy authority
CI / Build, lint & test (push) Successful in 3m15s
CI / Build, lint & test (push) Successful in 3m15s
`POST /matches/result` granted coins, XP, level-ups, statistics, four objective metrics, loan expiry, season progression and achievements across a dozen SEPARATE writes with no transaction and no idempotency key. Every call re-credited the same match, and any mid-way failure half-applied it. It sat beside `/matches/complete`, so nothing stopped one match being paid twice through two different doors. It cannot be made exactly-once in place: that needs a caller-supplied match identity, and this request shape has none. Deriving one from the body would collapse two legitimate matches with the same scoreline into one — the under-credit trap already documented for the `fp:` fallback. So the route fails closed: it rejects with a message naming `/matches/complete`, rather than 404, so a caller learns why. The behaviour it uniquely drove is kept, not deleted. `process_match` was the ONLY caller of loan expiry and Core's season model, so both move into `complete_match`'s transaction behind opt-in `expire_loans` / `advance_season` flags. Both default OFF, which keeps the FIFA 17 retail path byte-identical: FIFA 17 has its own loan and Seasons models, and Core's season END GRANTS coins and a pack — invisible economy on a path that never asked for it. Their pooled implementations are replaced by `expire_loans_tx` and `season::record_match_tx`, so a loan that expires or a season that ends commits with the match that caused it. Notifications (level-up / objective / loan / season) were pooled side effects of the removed path. They now emit from the route AFTER the commit — never inside the transaction, since a failed notification must not roll back a completed match — and only when `applied`, so a replay no longer re-notifies. The pooled path had no replay concept and notified every time. Also fixes a real bug this surfaced: `/auth/reset` never deleted `match_completions`, which carries un-cascaded foreign keys to BOTH `matches` and `profiles`. Any profile that completed a match through the authoritative route — i.e. every FIFA 17 profile after a retail match — failed to reset with a database error. It is now deleted first, and ordering is documented. Tests: the 20 integration call sites move to the authoritative route through one helper that mints a per-call identity (each call IS a distinct match). New coverage for the closed path: it rejects without moving the balance or writing history; Core progression stays off unless opted into; a replay does not duplicate notifications; and a profile that completed matches can still be reset.
This commit is contained in:
+34
-190
@@ -4,17 +4,11 @@ use crate::{
|
||||
models::{
|
||||
achievement::AchievementDefinition,
|
||||
card::OwnedCard,
|
||||
match_result::{
|
||||
CompleteMatchRequest, Match, MatchCompletionResult, MatchResultKind, MatchRewardResult,
|
||||
SubmitMatchRequest,
|
||||
},
|
||||
match_result::{CompleteMatchRequest, Match, MatchCompletionResult, MatchResultKind},
|
||||
objective::ObjectiveDefinition,
|
||||
profile::{coins_for_level, level_for_xp, pack_for_level, LevelUpEvent},
|
||||
},
|
||||
services::{
|
||||
achievement, card_db::CardDb, club, notification, objective, profile, season as season_svc,
|
||||
statistics,
|
||||
},
|
||||
services::{achievement, card_db::CardDb, objective, season as season_svc, statistics},
|
||||
};
|
||||
use rand::{seq::SliceRandom, Rng};
|
||||
use sqlx::{Sqlite, Transaction};
|
||||
@@ -127,182 +121,15 @@ const XP_WIN: i64 = 200;
|
||||
const XP_DRAW: i64 = 75;
|
||||
const XP_LOSS: i64 = 30;
|
||||
|
||||
pub async fn process_match(
|
||||
pool: &Pool,
|
||||
profile_id: &str,
|
||||
/// Decrement `loan_matches_remaining` for each loan card in the squad's starting
|
||||
/// XI, removing the cards whose loan ran out and returning their
|
||||
/// `owned_card_id`s. Runs inside the caller's transaction so a loan that expires
|
||||
/// commits (or rolls back) with the match that consumed it.
|
||||
async fn expire_loans_tx(
|
||||
tx: &mut Transaction<'_, Sqlite>,
|
||||
club_id: &str,
|
||||
req: &SubmitMatchRequest,
|
||||
obj_defs: &[ObjectiveDefinition],
|
||||
ach_defs: &[AchievementDefinition],
|
||||
) -> AppResult<MatchRewardResult> {
|
||||
if req.goals_for < 0 || req.goals_against < 0 || req.goals_for > 99 || req.goals_against > 99 {
|
||||
return Err(crate::error::AppError::BadRequest(
|
||||
"goals_for and goals_against must each be between 0 and 99".into(),
|
||||
));
|
||||
}
|
||||
|
||||
let outcome = if req.goals_for > req.goals_against {
|
||||
"win"
|
||||
} else if req.goals_for == req.goals_against {
|
||||
"draw"
|
||||
} else {
|
||||
"loss"
|
||||
};
|
||||
|
||||
let (coins, xp) = match outcome {
|
||||
"win" => (COINS_WIN, XP_WIN),
|
||||
"draw" => (COINS_DRAW, XP_DRAW),
|
||||
_ => (COINS_LOSS, XP_LOSS),
|
||||
};
|
||||
|
||||
let match_record = Match::new(
|
||||
profile_id,
|
||||
&req.squad_id,
|
||||
&req.opponent_name,
|
||||
req.goals_for,
|
||||
req.goals_against,
|
||||
&req.mode,
|
||||
coins,
|
||||
xp,
|
||||
);
|
||||
|
||||
sqlx::query(
|
||||
"INSERT INTO matches (id, profile_id, squad_id, opponent_name, goals_for, goals_against, outcome, coins_awarded, xp_awarded, mode, played_at) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)"
|
||||
)
|
||||
.bind(&match_record.id)
|
||||
.bind(&match_record.profile_id)
|
||||
.bind(&match_record.squad_id)
|
||||
.bind(&match_record.opponent_name)
|
||||
.bind(match_record.goals_for)
|
||||
.bind(match_record.goals_against)
|
||||
.bind(&match_record.outcome)
|
||||
.bind(match_record.coins_awarded)
|
||||
.bind(match_record.xp_awarded)
|
||||
.bind(&match_record.mode)
|
||||
.bind(&match_record.played_at)
|
||||
.execute(pool)
|
||||
.await?;
|
||||
|
||||
club::add_coins(pool, club_id, coins).await?;
|
||||
let level_ups = profile::add_xp_with_levelup(pool, profile_id, club_id, xp).await?;
|
||||
|
||||
for ev in &level_ups {
|
||||
let body = if let Some(ref pack) = ev.pack_granted {
|
||||
format!(
|
||||
"You reached level {}! Reward: {} coins + {pack}.",
|
||||
ev.new_level, ev.coins_granted
|
||||
)
|
||||
} else {
|
||||
format!(
|
||||
"You reached level {}! Reward: {} coins.",
|
||||
ev.new_level, ev.coins_granted
|
||||
)
|
||||
};
|
||||
let _ = notification::create(pool, "level_up", &format!("Level {}!", ev.new_level), &body)
|
||||
.await;
|
||||
}
|
||||
|
||||
statistics::record_match(
|
||||
pool,
|
||||
profile_id,
|
||||
outcome,
|
||||
req.goals_for,
|
||||
req.goals_against,
|
||||
coins,
|
||||
)
|
||||
.await?;
|
||||
|
||||
if let Some(positions) = &req.goal_positions {
|
||||
statistics::record_position_goals(pool, profile_id, positions).await?;
|
||||
}
|
||||
|
||||
let mut objectives_updated = Vec::new();
|
||||
|
||||
let mut completed =
|
||||
objective::increment_metric(pool, profile_id, obj_defs, "matchesplayed", 1).await?;
|
||||
objectives_updated.append(&mut completed);
|
||||
|
||||
if outcome == "win" {
|
||||
let mut c =
|
||||
objective::increment_metric(pool, profile_id, obj_defs, "matcheswon", 1).await?;
|
||||
objectives_updated.append(&mut c);
|
||||
}
|
||||
|
||||
let mut c =
|
||||
objective::increment_metric(pool, profile_id, obj_defs, "goalsscored", req.goals_for)
|
||||
.await?;
|
||||
objectives_updated.append(&mut c);
|
||||
|
||||
let mut c =
|
||||
objective::increment_metric(pool, profile_id, obj_defs, "coinsearned", coins).await?;
|
||||
objectives_updated.append(&mut c);
|
||||
|
||||
for obj_id in &objectives_updated {
|
||||
let display_name = obj_defs
|
||||
.iter()
|
||||
.find(|d| &d.id == obj_id)
|
||||
.map(|d| d.title.as_str())
|
||||
.unwrap_or(obj_id.as_str());
|
||||
let body = format!(
|
||||
"\"{}\" is now complete. Claim your reward in Objectives.",
|
||||
display_name
|
||||
);
|
||||
let _ =
|
||||
notification::create(pool, "objective_complete", "Objective complete!", &body).await;
|
||||
}
|
||||
|
||||
// Decrement loan matches remaining for squad starters; collect expired loan IDs.
|
||||
let expired_loans = process_loan_expiry(pool, club_id, &req.squad_id).await?;
|
||||
|
||||
for owned_id in &expired_loans {
|
||||
let body =
|
||||
format!("Loan card (id: {owned_id}) has expired and been removed from your club.");
|
||||
let _ = notification::create(pool, "loan_expired", "Loan card expired", &body).await;
|
||||
}
|
||||
|
||||
// Update season progress (creates the season row if it doesn't exist yet).
|
||||
season_svc::get_or_create(pool, profile_id).await?;
|
||||
let (_, season_end) = season_svc::record_match(pool, club_id, profile_id, outcome).await?;
|
||||
|
||||
if let Some(ref se) = season_end {
|
||||
use crate::models::season::SeasonResult;
|
||||
let direction = match se.result {
|
||||
SeasonResult::Promoted => "Promoted",
|
||||
SeasonResult::Relegated => "Relegated",
|
||||
SeasonResult::Maintained => "Maintained",
|
||||
};
|
||||
let body = format!(
|
||||
"{direction} — now in Division {}. Rewards: {} coins{}.",
|
||||
se.new_division,
|
||||
se.coins_awarded,
|
||||
se.pack_awarded
|
||||
.as_deref()
|
||||
.map(|p| format!(" + {p}"))
|
||||
.unwrap_or_default()
|
||||
);
|
||||
let _ = notification::create(pool, "season_end", "Season complete!", &body).await;
|
||||
}
|
||||
|
||||
let achievements_unlocked = achievement::check_and_unlock(pool, ach_defs, profile_id, club_id)
|
||||
.await
|
||||
.unwrap_or_default();
|
||||
|
||||
Ok(MatchRewardResult {
|
||||
match_record,
|
||||
coins_awarded: coins,
|
||||
xp_awarded: xp,
|
||||
objectives_updated,
|
||||
expired_loans,
|
||||
season_end,
|
||||
level_ups,
|
||||
achievements_unlocked,
|
||||
})
|
||||
}
|
||||
|
||||
/// Decrement `loan_matches_remaining` for each loan card in the squad's starting XI.
|
||||
/// Removes cards whose remaining count hits 0 and returns their owned_card_ids.
|
||||
async fn process_loan_expiry(pool: &Pool, club_id: &str, squad_id: &str) -> AppResult<Vec<String>> {
|
||||
// Get starters (is_on_bench = 0) for this squad
|
||||
squad_id: &str,
|
||||
) -> AppResult<Vec<String>> {
|
||||
let starters: Vec<(String, String)> = sqlx::query_as(
|
||||
"SELECT sp.id, sp.owned_card_id FROM squad_players sp \
|
||||
JOIN squads s ON s.id = sp.squad_id \
|
||||
@@ -310,39 +137,36 @@ async fn process_loan_expiry(pool: &Pool, club_id: &str, squad_id: &str) -> AppR
|
||||
)
|
||||
.bind(squad_id)
|
||||
.bind(club_id)
|
||||
.fetch_all(pool)
|
||||
.fetch_all(&mut **tx)
|
||||
.await?;
|
||||
|
||||
let mut expired = Vec::new();
|
||||
|
||||
for (_sp_id, owned_id) in starters {
|
||||
let card = sqlx::query_as::<_, OwnedCard>(
|
||||
"SELECT id, club_id, card_id, is_loan, loan_matches_remaining, acquired_at \
|
||||
FROM owned_cards WHERE id = ? AND is_loan = 1",
|
||||
)
|
||||
.bind(&owned_id)
|
||||
.fetch_optional(pool)
|
||||
.fetch_optional(&mut **tx)
|
||||
.await?;
|
||||
|
||||
if let Some(c) = card {
|
||||
let remaining = c.loan_matches_remaining.unwrap_or(0);
|
||||
if remaining <= 1 {
|
||||
// Loan expired — remove from collection
|
||||
sqlx::query("DELETE FROM owned_cards WHERE id = ?")
|
||||
.bind(&owned_id)
|
||||
.execute(pool)
|
||||
.execute(&mut **tx)
|
||||
.await?;
|
||||
expired.push(owned_id);
|
||||
} else {
|
||||
sqlx::query("UPDATE owned_cards SET loan_matches_remaining = ? WHERE id = ?")
|
||||
.bind(remaining - 1)
|
||||
.bind(&owned_id)
|
||||
.execute(pool)
|
||||
.execute(&mut **tx)
|
||||
.await?;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Ok(expired)
|
||||
}
|
||||
|
||||
@@ -526,6 +350,8 @@ async fn complete_match_inner(
|
||||
let mut objectives_updated = Vec::new();
|
||||
let mut level_ups = Vec::new();
|
||||
let mut achievements_unlocked = Vec::new();
|
||||
let mut expired_loans = Vec::new();
|
||||
let mut season_end = None;
|
||||
|
||||
// A no-contest is recorded (history + idempotency) but has ZERO economic
|
||||
// effect: no coins, XP, statistics, objectives, or achievements.
|
||||
@@ -617,6 +443,18 @@ async fn complete_match_inner(
|
||||
// 7. Achievements.
|
||||
achievements_unlocked =
|
||||
achievement::check_and_unlock_tx(&mut tx, ach_defs, profile_id, club_id, &now).await?;
|
||||
|
||||
// 8. Opt-in progression. Both default OFF: a game whose loans and
|
||||
// seasons are its own (FIFA 17) must not have Core's model advance —
|
||||
// and Core's season end GRANTS coins and a pack, which would be
|
||||
// invisible economy on a path that never asked for it.
|
||||
if req.expire_loans {
|
||||
expired_loans = expire_loans_tx(&mut tx, club_id, &req.squad_id).await?;
|
||||
}
|
||||
if req.advance_season {
|
||||
season_end =
|
||||
season_svc::record_match_tx(&mut tx, club_id, profile_id, outcome, &now).await?;
|
||||
}
|
||||
}
|
||||
inject_fault(fault, FaultPoint::BeforeCommit)?;
|
||||
|
||||
@@ -637,6 +475,8 @@ async fn complete_match_inner(
|
||||
objectives_updated,
|
||||
level_ups,
|
||||
achievements_unlocked,
|
||||
expired_loans,
|
||||
season_end,
|
||||
match_record,
|
||||
})
|
||||
}
|
||||
@@ -685,6 +525,8 @@ async fn already_completed(
|
||||
coins_balance,
|
||||
objectives_updated: vec![],
|
||||
level_ups: vec![],
|
||||
expired_loans: vec![],
|
||||
season_end: None,
|
||||
achievements_unlocked: vec![],
|
||||
match_record,
|
||||
})
|
||||
@@ -821,6 +663,8 @@ mod match_completion_tests {
|
||||
goals_against: ga,
|
||||
mode: "seasons".into(),
|
||||
goal_positions: None,
|
||||
expire_loans: false,
|
||||
advance_season: false,
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
+63
-33
@@ -2,8 +2,8 @@ use crate::{
|
||||
db::Pool,
|
||||
error::{AppError, AppResult},
|
||||
models::season::{Season, SeasonEndSummary, SeasonHistoryEntry, SeasonResult},
|
||||
services::{club, pack},
|
||||
};
|
||||
use sqlx::{Sqlite, Transaction};
|
||||
use uuid::Uuid;
|
||||
|
||||
/// Get the current season for a profile, creating it if it doesn't exist.
|
||||
@@ -38,21 +38,34 @@ async fn fetch(pool: &Pool, profile_id: &str) -> AppResult<Option<Season>> {
|
||||
Ok(s)
|
||||
}
|
||||
|
||||
/// Record a match result in the season; end the season if the quota is met.
|
||||
/// Record a match in Core's season model inside the caller's transaction,
|
||||
/// ending the season when the quota is met.
|
||||
///
|
||||
/// Returns the updated season and an optional end-of-season summary.
|
||||
pub async fn record_match(
|
||||
pool: &Pool,
|
||||
/// Mirrors [`record_match`] but every write — the season row, the rollover, the
|
||||
/// end-of-season coin and pack award, and the history entry — commits or rolls
|
||||
/// back with the match that caused it. Creates the season row if absent, so a
|
||||
/// first match does not need a separate call.
|
||||
pub async fn record_match_tx(
|
||||
tx: &mut Transaction<'_, Sqlite>,
|
||||
club_id: &str,
|
||||
profile_id: &str,
|
||||
outcome: &str,
|
||||
) -> AppResult<(Season, Option<SeasonEndSummary>)> {
|
||||
now: &str,
|
||||
) -> AppResult<Option<SeasonEndSummary>> {
|
||||
sqlx::query(
|
||||
"INSERT OR IGNORE INTO seasons (profile_id, division, season_number, season_points, \
|
||||
matches_played, wins, draws, losses, started_at) VALUES (?, 5, 1, 0, 0, 0, 0, 0, ?)",
|
||||
)
|
||||
.bind(profile_id)
|
||||
.bind(now)
|
||||
.execute(&mut **tx)
|
||||
.await?;
|
||||
|
||||
let points = match outcome {
|
||||
"win" => 3,
|
||||
"draw" => 1,
|
||||
_ => 0,
|
||||
};
|
||||
|
||||
sqlx::query(
|
||||
"UPDATE seasons SET \
|
||||
season_points = season_points + ?, \
|
||||
@@ -67,32 +80,28 @@ pub async fn record_match(
|
||||
.bind(outcome)
|
||||
.bind(outcome)
|
||||
.bind(profile_id)
|
||||
.execute(pool)
|
||||
.execute(&mut **tx)
|
||||
.await?;
|
||||
|
||||
let season = fetch(pool, profile_id).await?.ok_or_else(|| {
|
||||
let season = fetch_tx(tx, profile_id).await?.ok_or_else(|| {
|
||||
AppError::Internal(anyhow::anyhow!(
|
||||
"season row missing after record_match update"
|
||||
"season row missing after record_match_tx update"
|
||||
))
|
||||
})?;
|
||||
|
||||
if !season.is_complete() {
|
||||
return Ok((season, None));
|
||||
return Ok(None);
|
||||
}
|
||||
|
||||
// Season complete — calculate result and start next
|
||||
let result = season.end_result();
|
||||
let old_div = season.division;
|
||||
let coins = season.season_reward_coins();
|
||||
let pack_id = season.season_reward_pack();
|
||||
|
||||
let new_div = match result {
|
||||
SeasonResult::Promoted => (old_div - 1).max(1),
|
||||
SeasonResult::Relegated => (old_div + 1).min(10),
|
||||
SeasonResult::Maintained => old_div,
|
||||
};
|
||||
let new_season = season.season_number + 1;
|
||||
let now = chrono::Utc::now().to_rfc3339();
|
||||
|
||||
sqlx::query(
|
||||
"UPDATE seasons SET division = ?, season_number = ?, season_points = 0, \
|
||||
@@ -101,30 +110,46 @@ pub async fn record_match(
|
||||
)
|
||||
.bind(new_div)
|
||||
.bind(new_season)
|
||||
.bind(&now)
|
||||
.bind(now)
|
||||
.bind(profile_id)
|
||||
.execute(pool)
|
||||
.execute(&mut **tx)
|
||||
.await?;
|
||||
|
||||
// Grant rewards
|
||||
club::add_coins(pool, club_id, coins).await?;
|
||||
if coins > 0 {
|
||||
let credited =
|
||||
sqlx::query("UPDATE clubs SET coins = coins + ?, updated_at = ? WHERE id = ?")
|
||||
.bind(coins)
|
||||
.bind(now)
|
||||
.bind(club_id)
|
||||
.execute(&mut **tx)
|
||||
.await?;
|
||||
if credited.rows_affected() != 1 {
|
||||
return Err(AppError::NotFound("club not found".into()));
|
||||
}
|
||||
}
|
||||
if let Some(pack_def) = pack_id {
|
||||
pack::grant_pack(pool, club_id, pack_def).await?;
|
||||
sqlx::query(
|
||||
"INSERT INTO packs (id, club_id, definition_id, opened, created_at) VALUES (?, ?, ?, 0, ?)",
|
||||
)
|
||||
.bind(Uuid::new_v4().to_string())
|
||||
.bind(club_id)
|
||||
.bind(pack_def)
|
||||
.bind(now)
|
||||
.execute(&mut **tx)
|
||||
.await?;
|
||||
}
|
||||
|
||||
// Persist history entry before rolling over
|
||||
let result_str = match result {
|
||||
SeasonResult::Promoted => "promoted",
|
||||
SeasonResult::Maintained => "maintained",
|
||||
SeasonResult::Relegated => "relegated",
|
||||
};
|
||||
let history_id = Uuid::new_v4().to_string();
|
||||
let _ = sqlx::query(
|
||||
sqlx::query(
|
||||
"INSERT INTO season_history (id, profile_id, season_number, division, season_points, \
|
||||
wins, draws, losses, result, new_division, coins_awarded, pack_awarded, ended_at) \
|
||||
VALUES (?,?,?,?,?,?,?,?,?,?,?,?,?)",
|
||||
)
|
||||
.bind(&history_id)
|
||||
.bind(Uuid::new_v4().to_string())
|
||||
.bind(profile_id)
|
||||
.bind(season.season_number)
|
||||
.bind(old_div)
|
||||
@@ -136,23 +161,28 @@ pub async fn record_match(
|
||||
.bind(new_div)
|
||||
.bind(coins)
|
||||
.bind(pack_id)
|
||||
.bind(&now)
|
||||
.execute(pool)
|
||||
.await;
|
||||
.bind(now)
|
||||
.execute(&mut **tx)
|
||||
.await?;
|
||||
|
||||
let summary = SeasonEndSummary {
|
||||
Ok(Some(SeasonEndSummary {
|
||||
result,
|
||||
old_division: old_div,
|
||||
new_division: new_div,
|
||||
new_season_number: new_season,
|
||||
coins_awarded: coins,
|
||||
pack_awarded: pack_id.map(String::from),
|
||||
};
|
||||
}))
|
||||
}
|
||||
|
||||
let updated = fetch(pool, profile_id).await?.ok_or_else(|| {
|
||||
AppError::Internal(anyhow::anyhow!("season row missing after season rollover"))
|
||||
})?;
|
||||
Ok((updated, Some(summary)))
|
||||
async fn fetch_tx(tx: &mut Transaction<'_, Sqlite>, profile_id: &str) -> AppResult<Option<Season>> {
|
||||
Ok(sqlx::query_as::<_, Season>(
|
||||
"SELECT profile_id, division, season_number, season_points, matches_played, \
|
||||
wins, draws, losses, started_at FROM seasons WHERE profile_id = ?",
|
||||
)
|
||||
.bind(profile_id)
|
||||
.fetch_optional(&mut **tx)
|
||||
.await?)
|
||||
}
|
||||
|
||||
/// Return past seasons for a profile, newest first (max 20).
|
||||
|
||||
Reference in New Issue
Block a user