Files
OpenFUT-Core/src/services/squad.rs
T
funman300 eab522a1eb squad: transactional replace_squad + a game-rules boundary for evaluation
Driven by a retail FIFA 17 capture: the client sends the WHOLE squad on
every save (~2KB, every slot/item/kit number), and a user swapping two
players produced nine changed slots across two saves. Slot deltas
therefore do not describe intent, so the only honest semantic operation
is "this is the squad now".

replace_squad, and why save_squad no longer has its own write path
------------------------------------------------------------------
save_squad UPDATEd the squad, DELETEd every squad_players row, then
INSERTed the new ones one at a time -- all outside a transaction. A
failure part-way through left a squad with some old players deleted and
only some new ones written: a state nobody asked for and no client can
detect. It also never checked that the cards being placed belonged to the
club, and accepted the same card in two slots.

replace_squad validates BEFORE any write (so a rejection leaves the
stored squad untouched) and performs every write in one transaction:

  - card must exist AND belong to this club
  - a card may occupy at most one slot
  - a slot may hold at most one card
  - slot indices must not be negative
  - the squad being replaced must belong to this club

save_squad is now a thin wrapper over it. That deliberately tightens the
existing Core REST route -- it now validates ownership and rejects
duplicates. Those were bugs, and two write paths with different
guarantees is how the stricter one gets bypassed.

A cross-club card is reported as NotFound, not Forbidden: whether a card
exists in someone else's club is not the caller's business.

Game-rules boundary
-------------------
Core contained calculate_chemistry -- a full FUT-style link-scoring
formula. Chemistry is game-specific and changed between FIFA
generations, so a formula compiled into generic Core quietly makes Core a
FIFA-something server.

It now sits behind SquadRules, with the existing implementation preserved
byte-for-byte in behaviour as DefaultSquadRules ("openfut-default-v2").
Rules take a resolved SquadSnapshot of pure data rather than a pool and a
card database, so they are synchronous, testable without fixtures, and
cannot reach Core's storage. Fifa17SquadRules is deliberately NOT
written: the algorithm is unproven and inventing one is worse than having
none.

Client-reported values
----------------------
FIFA sends its own chemistry/rating/starRating. ClientReportedEvaluation
is a DIFFERENT TYPE from SquadEvaluation, so assigning one where the
other belongs does not compile. Disagreement is reported through
EvaluationComparison and never reconciled in either direction -- the
server's value stands and the mismatch is surfaced for investigation
against the exact squad that produced it.

Evidence
--------
17 unit tests, 113 in the crate, 7/7 mutations killed including
"ownership check removed", "replacement becomes a merge" and
"client-reported chemistry becomes the server value".

Scope note: `cargo fmt` without -p reformatted ~27 unrelated files; those
were reverted so this commit touches only the squad path.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-08-11 18:56:33 +00:00

731 lines
24 KiB
Rust

use crate::{
db::Pool,
error::{AppError, AppResult},
models::{
card::{CardDefinition, OwnedCard},
squad::{
SaveSquadRequest, SlotAssignment, Squad, SquadPlayer, SquadPlayerInput, SquadReplaced,
SquadReplacement,
},
},
services::{
card_db::CardDb,
squad_rules::{
ClientReportedEvaluation, DefaultSquadRules, SquadPlayerCard, SquadRules, SquadSnapshot,
},
},
};
use std::collections::HashSet;
use uuid::Uuid;
pub async fn get_squad(pool: &Pool, club_id: &str) -> AppResult<(Squad, Vec<SquadPlayer>)> {
let squad = sqlx::query_as::<_, Squad>(
"SELECT id, club_id, name, formation, created_at, updated_at FROM squads WHERE club_id = ? ORDER BY updated_at DESC LIMIT 1"
)
.bind(club_id)
.fetch_optional(pool)
.await?
.ok_or_else(|| AppError::NotFound("no squad found for this club".into()))?;
let players = get_players(pool, &squad.id).await?;
Ok((squad, players))
}
pub async fn get_squad_by_id(
pool: &Pool,
club_id: &str,
squad_id: &str,
) -> AppResult<(Squad, Vec<SquadPlayer>)> {
let squad = sqlx::query_as::<_, Squad>(
"SELECT id, club_id, name, formation, created_at, updated_at FROM squads WHERE id = ? AND club_id = ?",
)
.bind(squad_id)
.bind(club_id)
.fetch_optional(pool)
.await?
.ok_or_else(|| AppError::NotFound(format!("squad '{squad_id}' not found")))?;
let players = get_players(pool, &squad.id).await?;
Ok((squad, players))
}
pub async fn list_squads(pool: &Pool, club_id: &str) -> AppResult<Vec<Squad>> {
let squads = sqlx::query_as::<_, Squad>(
"SELECT id, club_id, name, formation, created_at, updated_at FROM squads WHERE club_id = ? ORDER BY updated_at DESC",
)
.bind(club_id)
.fetch_all(pool)
.await?;
Ok(squads)
}
async fn get_players(pool: &Pool, squad_id: &str) -> AppResult<Vec<SquadPlayer>> {
let players = sqlx::query_as::<_, SquadPlayer>(
"SELECT id, squad_id, owned_card_id, position_index, is_captain, is_on_bench FROM squad_players WHERE squad_id = ?",
)
.bind(squad_id)
.fetch_all(pool)
.await?;
Ok(players)
}
pub async fn validate_formation(
pool: &Pool,
card_db: &CardDb,
players: &[SquadPlayerInput],
) -> AppResult<()> {
let starters: Vec<&SquadPlayerInput> = players.iter().filter(|p| !p.is_on_bench).collect();
if starters.len() != 11 {
return Err(AppError::BadRequest(format!(
"need exactly 11 starters, got {}",
starters.len()
)));
}
let mut gk_count = 0usize;
for sp in &starters {
let owned = sqlx::query_as::<_, OwnedCard>(
"SELECT id, club_id, card_id, is_loan, loan_matches_remaining, acquired_at, chemistry_style, position_override, training_bonus FROM owned_cards WHERE id = ?",
)
.bind(&sp.owned_card_id)
.fetch_optional(pool)
.await?
.ok_or_else(|| AppError::NotFound(format!("owned card {} not found", sp.owned_card_id)))?;
if let Some(card) = card_db.get(&owned.card_id) {
if card.position == "GK" {
gk_count += 1;
}
}
}
if gk_count == 0 {
return Err(AppError::BadRequest(
"squad must include exactly one goalkeeper (GK)".into(),
));
}
if gk_count > 1 {
return Err(AppError::BadRequest(
"squad cannot have more than one starting goalkeeper".into(),
));
}
Ok(())
}
/// Chemistry v2: FUT-style link scoring.
///
/// Each player earns up to 10 chemistry from three link types:
/// - Club links: strongest signal — +3 per shared club teammate (max 6 pts)
/// - League links: medium signal — +1 per shared league mate (max 4 pts)
/// - Nation links: weakest signal — +1 per shared nation mate (max 3 pts)
///
/// Individual player chemistry is capped at 10.
/// Team chemistry = sum of all player chemistries, capped at 100.
pub async fn calculate_chemistry(
pool: &Pool,
card_db: &CardDb,
players: &[SquadPlayer],
) -> AppResult<serde_json::Value> {
let starters: Vec<&SquadPlayer> = players.iter().filter(|p| !p.is_on_bench).collect();
// Load all starter card definitions (N separate queries, fine for 11 players)
let mut player_cards: Vec<(String, CardDefinition)> = Vec::new();
for sp in &starters {
let owned = sqlx::query_as::<_, OwnedCard>(
"SELECT id, club_id, card_id, is_loan, loan_matches_remaining, acquired_at \
FROM owned_cards WHERE id = ?",
)
.bind(&sp.owned_card_id)
.fetch_optional(pool)
.await?;
if let Some(o) = owned {
if let Some(card) = card_db.get(&o.card_id) {
player_cards.push((sp.owned_card_id.clone(), card.clone()));
}
}
}
let mut player_chemistries = Vec::with_capacity(player_cards.len());
let mut total_chem: i64 = 0;
for (i, (owned_id, card)) in player_cards.iter().enumerate() {
let club_links = player_cards
.iter()
.enumerate()
.filter(|(j, (_, c))| *j != i && c.club == card.club)
.count() as i64;
let league_links = player_cards
.iter()
.enumerate()
.filter(|(j, (_, c))| *j != i && c.league == card.league)
.count() as i64;
let nation_links = player_cards
.iter()
.enumerate()
.filter(|(j, (_, c))| *j != i && c.nation == card.nation)
.count() as i64;
let club_pts = (club_links * 3).min(6);
let league_pts = league_links.min(4);
let nation_pts = nation_links.min(3);
let player_chem = (club_pts + league_pts + nation_pts).min(10);
total_chem += player_chem;
player_chemistries.push(serde_json::json!({
"owned_card_id": owned_id,
"card_id": &card.id,
"name": &card.name,
"chemistry": player_chem,
"breakdown": {
"club_links": club_links,
"league_links": league_links,
"nation_links": nation_links,
"club_pts": club_pts,
"league_pts": league_pts,
"nation_pts": nation_pts,
}
}));
}
let team_chemistry = total_chem.min(100);
Ok(serde_json::json!({
"total": team_chemistry,
"max": 100,
"player_count": player_chemistries.len(),
"player_chemistries": player_chemistries,
}))
}
/// Replace a squad's entire slot assignment, atomically.
///
/// # Why this exists alongside `save_squad`
///
/// `save_squad` wrote outside a transaction: it UPDATEd the squad, DELETEd every
/// row from `squad_players`, then INSERTed the new ones one at a time. A failure
/// part-way through left a squad with some of its old players deleted and only
/// some of its new ones written — a state no client asked for and none can
/// detect. It also never checked that the cards being placed belonged to the
/// club, and happily accepted the same card in two slots.
///
/// Those are acceptable in a single-user REST toy and not acceptable under a
/// real client, so this is the one write path now and `save_squad` delegates to
/// it.
///
/// # Order of work
///
/// Validation happens BEFORE any write, so a rejected replacement leaves the
/// existing squad exactly as it was. Everything that does write happens inside
/// one transaction.
pub async fn replace_squad(
pool: &Pool,
card_db: &CardDb,
rules: &dyn SquadRules,
club_id: &str,
squad_id: Option<&str>,
replacement: &SquadReplacement,
client_reported: &ClientReportedEvaluation,
) -> AppResult<SquadReplaced> {
// ── validate before touching anything ────────────────────────────────
let mut seen: HashSet<&str> = HashSet::new();
let mut slots_seen: HashSet<i64> = HashSet::new();
for s in &replacement.slots {
if s.slot < 0 {
return Err(AppError::BadRequest(format!(
"slot index must not be negative, got {}",
s.slot
)));
}
if !slots_seen.insert(s.slot) {
return Err(AppError::BadRequest(format!(
"slot {} assigned more than once",
s.slot
)));
}
if !seen.insert(s.owned_card_id.as_str()) {
return Err(AppError::BadRequest(format!(
"card {} assigned to more than one slot",
s.owned_card_id
)));
}
}
// Ownership: every card must belong to THIS club. Without this a client
// could place a card it does not own, and the squad would read back as
// though it did.
let mut resolved: Vec<(SlotAssignmentRef, OwnedCard)> = Vec::new();
for s in &replacement.slots {
let owned = sqlx::query_as::<_, OwnedCard>(
"SELECT id, club_id, card_id, is_loan, loan_matches_remaining, acquired_at, chemistry_style, position_override, training_bonus FROM owned_cards WHERE id = ?",
)
.bind(&s.owned_card_id)
.fetch_optional(pool)
.await?
.ok_or_else(|| AppError::NotFound(format!("owned card {} not found", s.owned_card_id)))?;
if owned.club_id != club_id {
// Deliberately the same message as "not found": whether a card
// exists in someone else's club is not this caller's business.
return Err(AppError::NotFound(format!(
"owned card {} not found",
s.owned_card_id
)));
}
resolved.push((
SlotAssignmentRef {
slot: s.slot,
is_captain: s.is_captain,
is_on_bench: s.is_on_bench,
},
owned,
));
}
// ── one transaction for every write ──────────────────────────────────
let now = chrono::Utc::now().to_rfc3339();
let mut tx = pool.begin().await?;
let squad_id = match squad_id {
Some(id) => {
let verified = sqlx::query_scalar::<_, String>(
"SELECT id FROM squads WHERE id = ? AND club_id = ?",
)
.bind(id)
.bind(club_id)
.fetch_optional(&mut *tx)
.await?
.ok_or_else(|| AppError::NotFound(format!("squad '{id}' not found")))?;
sqlx::query(
"UPDATE squads SET name = COALESCE(?, name), formation = COALESCE(?, formation), updated_at = ? WHERE id = ?",
)
.bind(replacement.name.as_deref())
.bind(replacement.formation.as_deref())
.bind(&now)
.bind(&verified)
.execute(&mut *tx)
.await?;
verified
}
None => {
let squad = Squad::new(
club_id,
replacement.name.as_deref().unwrap_or("My Squad"),
replacement.formation.as_deref().unwrap_or("4-4-2"),
);
sqlx::query(
"INSERT INTO squads (id, club_id, name, formation, created_at, updated_at) VALUES (?, ?, ?, ?, ?, ?)",
)
.bind(&squad.id)
.bind(&squad.club_id)
.bind(&squad.name)
.bind(&squad.formation)
.bind(&squad.created_at)
.bind(&squad.updated_at)
.execute(&mut *tx)
.await?;
squad.id
}
};
sqlx::query("DELETE FROM squad_players WHERE squad_id = ?")
.bind(&squad_id)
.execute(&mut *tx)
.await?;
for (slot, owned) in &resolved {
sqlx::query(
"INSERT INTO squad_players (id, squad_id, owned_card_id, position_index, is_captain, is_on_bench) VALUES (?, ?, ?, ?, ?, ?)",
)
.bind(Uuid::new_v4().to_string())
.bind(&squad_id)
.bind(&owned.id)
.bind(slot.slot)
.bind(slot.is_captain)
.bind(slot.is_on_bench)
.execute(&mut *tx)
.await?;
}
let squad = sqlx::query_as::<_, Squad>(
"SELECT id, club_id, name, formation, created_at, updated_at FROM squads WHERE id = ?",
)
.bind(&squad_id)
.fetch_one(&mut *tx)
.await?;
tx.commit().await?;
// ── evaluate with the game's rules, never with the client's numbers ──
let snapshot = SquadSnapshot {
formation: squad.formation.clone(),
players: resolved
.iter()
.filter_map(|(slot, owned)| {
card_db.get(&owned.card_id).map(|card| SquadPlayerCard {
owned_card_id: owned.id.clone(),
card_id: card.id.clone(),
name: card.name.clone(),
overall: card.overall,
position: card.position.clone(),
nation: card.nation.clone(),
league: card.league.clone(),
club: card.club.clone(),
slot: slot.slot,
on_bench: slot.is_on_bench,
})
})
.collect(),
};
let evaluation = rules.evaluate(&snapshot);
let client_disagreements = client_reported.compare(&evaluation);
Ok(SquadReplaced {
squad,
slots_written: resolved.len(),
evaluation,
client_disagreements,
})
}
struct SlotAssignmentRef {
slot: i64,
is_captain: bool,
is_on_bench: bool,
}
/// Compatibility wrapper over [`replace_squad`].
///
/// Kept so the existing Core REST route keeps working, but it no longer has its
/// own write path. That means this route now also validates ownership and
/// rejects duplicate cards — a deliberate tightening, not an accident: those
/// were bugs, and having two write paths with different guarantees is how the
/// stricter one gets bypassed.
pub async fn save_squad(
pool: &Pool,
card_db: &CardDb,
club_id: &str,
req: &SaveSquadRequest,
) -> AppResult<Squad> {
let replacement = SquadReplacement {
name: req.name.clone(),
formation: req.formation.clone(),
slots: req
.players
.iter()
.map(|p| SlotAssignment {
owned_card_id: p.owned_card_id.clone(),
slot: p.position_index,
is_captain: p.is_captain,
is_on_bench: p.is_on_bench,
})
.collect(),
};
let out = replace_squad(
pool,
card_db,
&DefaultSquadRules,
club_id,
req.squad_id.as_deref(),
&replacement,
&ClientReportedEvaluation::default(),
)
.await?;
Ok(out.squad)
}
pub async fn delete_squad(pool: &Pool, club_id: &str, squad_id: &str) -> AppResult<()> {
let deleted = sqlx::query("DELETE FROM squads WHERE id = ? AND club_id = ?")
.bind(squad_id)
.bind(club_id)
.execute(pool)
.await?;
if deleted.rows_affected() == 0 {
return Err(AppError::NotFound(format!("squad '{squad_id}' not found")));
}
Ok(())
}
#[cfg(test)]
mod tests {
use super::*;
use crate::models::squad::SlotAssignment;
/// A pool with the real schema, plus two clubs that own one card each.
///
/// Two clubs specifically: the guarantee under test is that a card
/// belonging to somebody else cannot be placed, and that cannot be
/// expressed with one club.
const TS: &str = "2026-01-01T00:00:00Z";
async fn fixture() -> (Pool, CardDb) {
let pool = sqlx::sqlite::SqlitePoolOptions::new()
.connect("sqlite::memory:")
.await
.expect("in-memory sqlite");
sqlx::migrate!("./migrations")
.run(&pool)
.await
.expect("migrations");
for (profile, club) in [("prof-a", "club-a"), ("prof-b", "club-b")] {
sqlx::query(
"INSERT INTO profiles (id, username, created_at, updated_at) VALUES (?, ?, ?, ?)",
)
.bind(profile)
.bind(profile)
.bind(TS)
.bind(TS)
.execute(&pool)
.await
.expect("profile");
sqlx::query("INSERT INTO clubs (id, profile_id, name, coins, created_at, updated_at) VALUES (?, ?, ?, ?, ?, ?)")
.bind(club).bind(profile).bind(club).bind(1000i64).bind(TS).bind(TS)
.execute(&pool).await.expect("club");
}
// club-a owns card-1 and card-2; club-b owns card-foreign.
for (id, club) in [
("card-1", "club-a"),
("card-2", "club-a"),
("card-foreign", "club-b"),
] {
sqlx::query("INSERT INTO owned_cards (id, club_id, card_id, is_loan, acquired_at) VALUES (?, ?, ?, 0, ?)")
.bind(id).bind(club).bind("def-1").bind(TS)
.execute(&pool).await.expect("owned card");
}
// An empty card database is enough: none of these guarantees consult it.
(
pool,
CardDb::load("/nonexistent-card-dir").expect("empty card db"),
)
}
fn slot(card: &str, n: i64) -> SlotAssignment {
SlotAssignment {
owned_card_id: card.into(),
slot: n,
is_captain: false,
is_on_bench: false,
}
}
async fn replace(
pool: &Pool,
db: &CardDb,
club: &str,
id: Option<&str>,
slots: Vec<SlotAssignment>,
) -> AppResult<SquadReplaced> {
replace_squad(
pool,
db,
&DefaultSquadRules,
club,
id,
&SquadReplacement {
name: Some("S".into()),
formation: Some("4-4-2".into()),
slots,
},
&ClientReportedEvaluation::default(),
)
.await
}
async fn slots_of(pool: &Pool, squad_id: &str) -> Vec<(String, i64)> {
sqlx::query_as::<_, (String, i64)>(
"SELECT owned_card_id, position_index FROM squad_players WHERE squad_id = ? ORDER BY position_index",
).bind(squad_id).fetch_all(pool).await.unwrap()
}
#[tokio::test]
async fn a_card_owned_by_another_club_cannot_be_placed() {
let (pool, db) = fixture().await;
let err = replace(&pool, &db, "club-a", None, vec![slot("card-foreign", 0)])
.await
.unwrap_err();
// Same message as a missing card: whether it exists elsewhere is not
// this caller's business.
assert!(
matches!(err, AppError::NotFound(ref m) if m.contains("card-foreign")),
"{err:?}"
);
}
#[tokio::test]
async fn the_same_card_cannot_occupy_two_slots() {
let (pool, db) = fixture().await;
let err = replace(
&pool,
&db,
"club-a",
None,
vec![slot("card-1", 0), slot("card-1", 1)],
)
.await
.unwrap_err();
assert!(
matches!(err, AppError::BadRequest(ref m) if m.contains("more than one slot")),
"{err:?}"
);
}
#[tokio::test]
async fn two_cards_cannot_occupy_the_same_slot() {
let (pool, db) = fixture().await;
let err = replace(
&pool,
&db,
"club-a",
None,
vec![slot("card-1", 3), slot("card-2", 3)],
)
.await
.unwrap_err();
assert!(
matches!(err, AppError::BadRequest(ref m) if m.contains("slot 3")),
"{err:?}"
);
}
#[tokio::test]
async fn a_negative_slot_is_refused() {
let (pool, db) = fixture().await;
let err = replace(&pool, &db, "club-a", None, vec![slot("card-1", -1)])
.await
.unwrap_err();
assert!(
matches!(err, AppError::BadRequest(ref m) if m.contains("negative")),
"{err:?}"
);
}
/// The atomicity guarantee, and the reason this operation exists.
///
/// The old implementation deleted every squad player before inserting the
/// new ones, outside a transaction. A replacement rejected part-way through
/// therefore destroyed the squad it failed to replace.
#[tokio::test]
async fn a_rejected_replacement_leaves_the_previous_squad_untouched() {
let (pool, db) = fixture().await;
let first = replace(
&pool,
&db,
"club-a",
None,
vec![slot("card-1", 0), slot("card-2", 1)],
)
.await
.expect("first save");
let before = slots_of(&pool, &first.squad.id).await;
assert_eq!(before.len(), 2);
// Valid card in slot 0, then one owned by another club.
let err = replace(
&pool,
&db,
"club-a",
Some(&first.squad.id),
vec![slot("card-1", 0), slot("card-foreign", 1)],
)
.await
.unwrap_err();
assert!(matches!(err, AppError::NotFound(_)), "{err:?}");
assert_eq!(
slots_of(&pool, &first.squad.id).await,
before,
"a rejected replacement must not disturb the stored squad"
);
}
/// Replacement means replacement: slots present before and absent from the
/// new assignment must be gone, not merged.
#[tokio::test]
async fn replacement_removes_slots_absent_from_the_new_assignment() {
let (pool, db) = fixture().await;
let first = replace(
&pool,
&db,
"club-a",
None,
vec![slot("card-1", 0), slot("card-2", 1)],
)
.await
.expect("first");
let second = replace(
&pool,
&db,
"club-a",
Some(&first.squad.id),
vec![slot("card-2", 5)],
)
.await
.expect("second");
assert_eq!(second.slots_written, 1);
assert_eq!(
slots_of(&pool, &first.squad.id).await,
vec![("card-2".to_string(), 5)]
);
}
#[tokio::test]
async fn a_squad_belonging_to_another_club_cannot_be_replaced() {
let (pool, db) = fixture().await;
let mine = replace(&pool, &db, "club-a", None, vec![slot("card-1", 0)])
.await
.expect("mine");
let err = replace(&pool, &db, "club-b", Some(&mine.squad.id), vec![])
.await
.unwrap_err();
assert!(matches!(err, AppError::NotFound(_)), "{err:?}");
assert_eq!(slots_of(&pool, &mine.squad.id).await.len(), 1);
}
/// The client's numbers must never become the server's.
#[tokio::test]
async fn client_reported_values_are_reported_as_disagreement_not_stored() {
let (pool, db) = fixture().await;
let out = replace_squad(
&pool,
&db,
&DefaultSquadRules,
"club-a",
None,
&SquadReplacement {
name: Some("S".into()),
formation: Some("4-4-2".into()),
slots: vec![slot("card-1", 0)],
},
&ClientReportedEvaluation {
client_reported_chemistry: Some(52),
client_reported_rating: Some(99),
client_reported_star_rating: None,
},
)
.await
.expect("save");
// The card db is empty, so the server derives nothing: 0.
assert_eq!(out.evaluation.chemistry, 0);
assert_eq!(out.evaluation.rating, 0);
// And the disagreement is surfaced rather than reconciled.
let fields: Vec<&str> = out
.client_disagreements
.iter()
.map(|d| d.field.as_str())
.collect();
assert_eq!(
fields,
vec!["chemistry", "rating"],
"{:?}",
out.client_disagreements
);
assert_eq!(out.evaluation.rules, "openfut-default-v2");
}
}