3 Commits

Author SHA1 Message Date
funman300 20e281e0cf feat(squad): support a role-only partial update distinct from replacement
CI / Build, lint & test (push) Successful in 3m28s
`/squad/replace` is a full replacement: it deletes every assignment and
reinserts the supplied slots, and 9bdc163 correctly made it refuse a
replacement carrying no slots so a broken client cannot write its emptiness
back. That guard is load-bearing and is not touched here.

But FIFA 17 sends TWO operations down one wire path. Its captain/kick-taker
screen emits a body with no `players` at all -- `{id, custom, captain,
kicktakers}` -- and the host presented that to `/squad/replace` as a
replacement with zero slots. The guard did exactly its job and refused it, so
every captain/kick-taker change died with a 400 (surfaced to the client as a
502) and the user's edit was silently lost. Confirmed by bisect against the
captures: the same body returned 200 on 08-24 18:06:59 and 502 at 20:05:30,
either side of the Core deploy carrying the guard.

The operation was mis-described, so the fix is to stop mis-describing it, not
to relax the guard. `patch_squad_roles` updates only the captain flag and the
opaque extension, in one transaction, issuing no statement that can insert,
delete or reorder an assignment row -- player slots, the squad manager and club
actives are untouched by construction rather than by care.

Two details that matter:

  * the captain is part of `squad_fingerprint`, so a captain move MUST
    re-anchor the extension or every later read reports it stale;
  * the captain is validated against THIS squad's assignments before any
    write, so an invalid target leaves captain AND extension unapplied rather
    than half-applying the patch.

Tests cover both halves: the captain moves without disturbing assignments and
re-anchors the fingerprint, and an unfielded captain is refused with the prior
captain and the prior extension payload both intact. The empty-replacement
guard regression test continues to pass unchanged.
2026-08-25 01:52:36 +00:00
funman300 8819cc76a1 fix(core): keep an absent manager field distinct from an explicit removal
CI / Build, lint & test (push) Successful in 3m24s
PUT /club/manager took `owned_card_id: Option<String>`, so serde collapsed
"field absent" and "field explicitly null" into the same None, and the route
treated both as a clear. A caller that simply had nothing to say about the
manager therefore DELETED the assignment.

That is the second destructive squad-save path. WAL forensics on the staging
DB pin it to commit frame 468, squad_managers 1 row -> 0, in a transaction
touching only squad_managers and its indexes - disjoint from the player wipe
at frame 465, which touched squads/squad_players/game_entity_ext. The two
wipes came from two different writes, and only the first was guarded.

The three states are now distinct:

  {}                            leave the manager exactly as it is
  {"owned_card_id": null}       explicitly remove it (still supported)
  {"owned_card_id": "<id>"}     assign that owned card

A deliberate removal is a legitimate operation and is preserved; only the
"absent means delete" reading is gone.

set_squad_manager_for_squad now runs its two existence checks and the insert
in ONE transaction. Validating on the pool and then inserting left a window in
which the squad or the card could disappear between check and write.

Tests cover assign, reassign, idempotent re-assign, absent-is-a-no-op,
explicit-null-still-removes, unowned-manager-refused, absent-against-no-manager
not over-guarded, and that no manager write disturbs player assignments. A
malformed body is asserted to be a parser rejection, distinguishable from the
guard. With the fix reverted the absent-field test fails.
2026-08-24 19:58:54 +00:00
funman300 9bdc1633a0 fix(core): refuse a squad replacement that would empty a populated squad
CI / Build, lint & test (push) Successful in 3m38s
/squad/replace is a full replacement: it deletes every assignment and
reinserts the supplied slots. Nothing validated that the supplied list was
non-empty, so a caller sending no slots silently wiped the squad and got
200/ok back.

This happened for real. A FIFA 17 client whose in-memory squad had been
destroyed by a bad parse wrote its emptiness back twice; WAL forensics on the
staging DB pin the damage to commit frame 465, squad_players 18 rows -> 0,
logged as route=squad-replace status=200 outcome=ok. The squad is the
authority's state, so mirroring a broken client's model is unrecoverable.

No product flow empties a squad: a full-replacement client sends its complete
slot array, and no caller or test in the tree builds an empty slot list. So an
empty list means the caller's model is broken, and the write is refused with
BadRequest. The check runs inside the transaction, so a concurrent write
cannot slip between the count and the delete, and a newly created squad
counts zero and is unaffected.

The regression test asserts both halves: the empty replacement is rejected,
and the existing assignments survive it. With the guard removed the test fails
with 200 and slots_written 0 - the exact production symptom.
2026-08-24 19:27:44 +00:00
6 changed files with 640 additions and 10 deletions
+1
View File
@@ -241,6 +241,7 @@ pub async fn build(pool: Pool, cfg: Config) -> Result<Router> {
.route("/squad", post(routes::squad::post_squad))
.route("/squad/ext", get(routes::squad::get_squad_ext))
.route("/squad/replace", put(routes::squad::put_squad_replace))
.route("/squad/roles", put(routes::squad::put_squad_roles))
.route("/squads", get(routes::squad::get_squads))
.route("/squads/:squad_id", get(routes::squad::get_squad_by_id))
.route("/squads/:squad_id", delete(routes::squad::delete_squad))
+31 -7
View File
@@ -139,15 +139,36 @@ pub async fn get_squad_manager(
Ok(Json(json!({ "manager": manager })))
}
/// A manager write. The three states are DISTINCT and must stay that way:
///
/// | body | meaning |
/// | --- | --- |
/// | `{}` — field absent | say nothing about the manager; leave it as it is |
/// | `{"owned_card_id": null}` | explicitly remove the current manager |
/// | `{"owned_card_id": "<id>"}` | assign that owned card |
///
/// A plain `Option<String>` collapsed the first two into `None`, so a caller
/// that simply had nothing to say silently deleted the assignment. That is how a
/// FIFA 17 client with a destroyed squad model wiped a real manager row. The
/// double option keeps "absent" and "null" apart.
#[derive(Deserialize)]
pub struct SetManagerRequest {
/// The owned card to assign as manager, or `null`/absent to clear it.
pub owned_card_id: Option<String>,
#[serde(default, deserialize_with = "deserialize_present_option")]
pub owned_card_id: Option<Option<String>>,
}
/// Assign (or, with a null/absent `owned_card_id`, clear) the active squad's
/// manager. Fail-closed: the card must be owned by this club and the club must
/// have a squad. Returns the resulting assignment.
/// Deserialize a field that is present-but-null into `Some(None)`, leaving an
/// absent field as `None` (supplied by `#[serde(default)]`).
fn deserialize_present_option<'de, D>(d: D) -> Result<Option<Option<String>>, D::Error>
where
D: serde::Deserializer<'de>,
{
Option::<String>::deserialize(d).map(Some)
}
/// Assign, explicitly remove, or leave unchanged the active squad's manager.
/// Fail-closed: the card must be owned by this club and the club must have a
/// squad. Returns the resulting assignment.
pub async fn put_squad_manager(
State(state): State<AppState>,
game: GameId,
@@ -156,10 +177,13 @@ pub async fn put_squad_manager(
let profile = profile_svc::get_active_profile(&state.pool, game.as_str()).await?;
let club = club_svc::get_club_by_profile(&state.pool, &profile.id).await?;
match req.owned_card_id {
Some(owned_card_id) => {
Some(Some(owned_card_id)) => {
club_svc::set_squad_manager(&state.pool, &club.id, &owned_card_id).await?
}
None => club_svc::clear_squad_manager(&state.pool, &club.id).await?,
// Explicit null: a deliberate removal, which is a legitimate operation.
Some(None) => club_svc::clear_squad_manager(&state.pool, &club.id).await?,
// Absent: this request expresses no manager decision. Touch nothing.
None => {}
}
let manager = club_svc::get_squad_manager(&state.pool, &club.id).await?;
Ok(Json(json!({ "manager": manager })))
+42
View File
@@ -235,3 +235,45 @@ pub async fn put_squad_replace(
"slots_written": out.slots_written,
})))
}
#[derive(Deserialize)]
pub struct RolePatchReq {
/// Owned card to flag as captain. Omitted means "leave the captain alone" —
/// it is NEVER a request to clear it. Clearing has no established client
/// semantics and is deliberately not invented here.
#[serde(default)]
pub captain_owned_card_id: Option<String>,
pub extension: OpaqueExtensionWrite,
}
/// `PUT /squad/roles` — patch ONLY role assignments (captain) plus the opaque
/// game extension, atomically.
///
/// Distinct from `/squad/replace` on purpose. A role-only update carries no slot
/// array, and describing it as a replacement with zero slots trips the
/// empty-replacement guard — which is correct behaviour for a replacement and
/// wrong for a patch. This route never touches player assignments, the squad
/// manager, or club actives.
pub async fn put_squad_roles(
State(state): State<AppState>,
game: GameId,
Json(req): Json<RolePatchReq>,
) -> AppResult<Json<Value>> {
let profile = profile_svc::get_active_profile(&state.pool, game.as_str()).await?;
let club = club_svc::get_club_by_profile(&state.pool, &profile.id).await?;
let out = squad_svc::patch_squad_roles(
&state.pool,
game.as_str(),
&club.id,
req.captain_owned_card_id.as_deref(),
&req.extension,
)
.await?;
Ok(Json(json!({
"squad_id": out.squad.id,
"canonical_fingerprint": out.canonical_fingerprint,
"captain_changed": out.captain_changed,
})))
}
+9 -3
View File
@@ -195,11 +195,16 @@ pub async fn set_squad_manager_for_squad(
squad_id: &str,
owned_card_id: &str,
) -> AppResult<()> {
// One transaction: both existence checks and the write. Validating on the
// pool and then inserting left a window in which the squad or the card could
// be removed between the check and the write, persisting an assignment whose
// preconditions no longer held.
let mut tx = pool.begin().await?;
let squad_ok =
sqlx::query_scalar::<_, String>("SELECT id FROM squads WHERE id = ? AND club_id = ?")
.bind(squad_id)
.bind(club_id)
.fetch_optional(pool)
.fetch_optional(&mut *tx)
.await?;
if squad_ok.is_none() {
return Err(AppError::NotFound(format!("squad '{squad_id}' not found")));
@@ -208,7 +213,7 @@ pub async fn set_squad_manager_for_squad(
sqlx::query_scalar::<_, String>("SELECT id FROM owned_cards WHERE id = ? AND club_id = ?")
.bind(owned_card_id)
.bind(club_id)
.fetch_optional(pool)
.fetch_optional(&mut *tx)
.await?;
if card_ok.is_none() {
return Err(AppError::NotFound(format!(
@@ -223,8 +228,9 @@ pub async fn set_squad_manager_for_squad(
.bind(squad_id)
.bind(owned_card_id)
.bind(&now)
.execute(pool)
.execute(&mut *tx)
.await?;
tx.commit().await?;
Ok(())
}
+151
View File
@@ -345,6 +345,32 @@ async fn replace_squad_inner(
}
};
// A replacement carrying no slots would DELETE every assignment below and
// insert nothing, silently emptying the squad. No product flow does that:
// a full-replacement client sends its COMPLETE slot array, so an empty list
// means the caller's own model was destroyed, not that the user emptied
// their squad. Mirroring that damage into the authority is unrecoverable,
// so refuse it.
//
// Observed for real: a FIFA 17 client whose in-memory squad had been
// destroyed by a bad parse wrote its emptiness back twice, taking
// `squad_players` from 18 rows to 0 while the request logged 200/ok.
//
// Checked inside the transaction so a concurrent write cannot slip between
// the count and the delete. A newly created squad counts 0 and is unaffected.
if replacement.slots.is_empty() {
let existing =
sqlx::query_scalar::<_, i64>("SELECT COUNT(*) FROM squad_players WHERE squad_id = ?")
.bind(&squad_id)
.fetch_one(&mut *tx)
.await?;
if existing > 0 {
return Err(AppError::BadRequest(format!(
"refusing to empty a populated squad: replacement carried no slots, but squad '{squad_id}' holds {existing} assignments"
)));
}
}
sqlx::query("DELETE FROM squad_players WHERE squad_id = ?")
.bind(&squad_id)
.execute(&mut *tx)
@@ -560,6 +586,131 @@ pub async fn read_squad_with_ext(
Ok((squad, players, state))
}
/// Outcome of a role-only squad patch.
pub struct SquadRolesPatched {
pub squad: Squad,
/// Re-anchored fingerprint of the committed canonical state. The captain
/// flag is part of the fingerprint, so a captain change MUST re-anchor the
/// extension or every later read reports it stale.
pub canonical_fingerprint: String,
/// Whether the captain flag actually moved (false when it was already set).
pub captain_changed: bool,
}
/// Patch ONLY a squad's role assignments plus its opaque game extension, in one
/// transaction. Never inserts, deletes or reorders a single assignment row.
///
/// This exists because a full replacement and a role-only update are different
/// operations that the FIFA 17 client sends down the same wire path. Routing a
/// role-only update through [`replace_squad_with_extension`] means presenting it
/// as a replacement carrying zero slots, which the empty-replacement guard
/// correctly refuses — the client's captain/kick-taker change was being lost
/// with a 400. The fix is to stop mis-describing the operation, NOT to relax the
/// guard: that guard is load-bearing and stays exactly as strict.
///
/// Player assignments, the squad manager and club actives are untouched by
/// construction — this function issues no statement that can affect them.
///
/// `captain_owned_card_id` must already be assigned to this squad. Anything else
/// is refused before any write, so an invalid target leaves the whole patch
/// unapplied (captain AND extension), never half-applied.
pub async fn patch_squad_roles(
pool: &Pool,
game_id: &str,
club_id: &str,
captain_owned_card_id: Option<&str>,
ext: &OpaqueExtensionWrite,
) -> AppResult<SquadRolesPatched> {
let now = chrono::Utc::now().to_rfc3339();
let mut tx = pool.begin().await?;
// Resolve the club's active squad. A role patch NEVER creates a squad: with
// no squad there is nothing to assign a captain within, and inventing one
// here would let a stray patch materialise empty canonical state.
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(&mut *tx)
.await?
.ok_or_else(|| AppError::NotFound("no squad found for this club".into()))?;
let assigned = sqlx::query_as::<_, (String, i64, bool, bool)>(
"SELECT owned_card_id, position_index, is_captain, is_on_bench \
FROM squad_players WHERE squad_id = ?",
)
.bind(&squad.id)
.fetch_all(&mut *tx)
.await?;
let mut captain_changed = false;
if let Some(captain) = captain_owned_card_id {
// Validate against THIS squad's assignments, not the whole collection:
// a captain the user does not field is not a captain, and accepting an
// arbitrary owned card here would let a patch reference any inventory
// item.
// Validated BEFORE any write, so an invalid target aborts the whole
// patch — captain and extension both — rather than half-applying it.
if !assigned.iter().any(|(owned, _, _, _)| owned == captain) {
return Err(AppError::BadRequest(format!(
"captain '{captain}' is not assigned to squad '{}'",
squad.id
)));
}
let already_captain = assigned
.iter()
.any(|(owned, _, cap, _)| owned == captain && *cap);
let someone_else_captain = assigned
.iter()
.any(|(owned, _, cap, _)| *cap && owned != captain);
captain_changed = !already_captain || someone_else_captain;
sqlx::query("UPDATE squad_players SET is_captain = (owned_card_id = ?) WHERE squad_id = ?")
.bind(captain)
.bind(&squad.id)
.execute(&mut *tx)
.await?;
}
// Re-anchor to the state as it now stands, applying the captain move to the
// in-memory view rather than re-reading: same transaction, same result, one
// fewer round trip.
let canonical_fingerprint = squad_fingerprint(
&squad.id,
&squad.formation,
assigned.iter().map(|(owned, slot, cap, bench)| {
let is_cap = match captain_owned_card_id {
Some(c) => owned.as_str() == c,
None => *cap,
};
(*slot, owned.as_str(), is_cap, *bench)
}),
);
sqlx::query(
"INSERT OR REPLACE INTO game_entity_ext \
(game_id, entity_kind, entity_id, namespace, schema_version, canonical_fingerprint, payload, updated_at) \
VALUES (?, 'squad', ?, ?, ?, ?, ?, ?)",
)
.bind(game_id)
.bind(&squad.id)
.bind(&ext.namespace)
.bind(ext.schema_version)
.bind(&canonical_fingerprint)
.bind(&ext.payload)
.bind(&now)
.execute(&mut *tx)
.await?;
tx.commit().await?;
Ok(SquadRolesPatched {
squad,
canonical_fingerprint,
captain_changed,
})
}
/// Compatibility wrapper over [`replace_squad`].
///
/// Kept so the existing Core REST route keeps working, but it no longer has its
+406
View File
@@ -3016,6 +3016,412 @@ async fn test_squad_ext_replace_read_roundtrip_and_idempotency() {
assert_eq!(other["extension"]["state"], "missing");
}
/// A full replacement that carries no slots MUST NOT empty a populated squad.
///
/// Regression: a FIFA 17 client whose in-memory squad had been destroyed by a
/// bad parse wrote that emptiness back through `/squad/replace`, taking the
/// canonical squad from 18 assignments to 0 while the request logged 200/ok.
/// The squad is the authority's state, so mirroring a broken client's model is
/// unrecoverable data loss.
#[tokio::test]
async fn test_squad_replace_refuses_to_empty_a_populated_squad() {
let app = build_test_app().await;
auth(&app, "SquadWipeGuardUser").await;
let (_, packs) = json_get(&app, "/packs").await;
let pack_id = packs["packs"][0]["pack_id"].as_str().unwrap().to_string();
json_post(
&app,
&format!("/packs/open/{pack_id}"),
serde_json::json!({}),
)
.await;
let (_, coll) = json_get(&app, "/collection").await;
let ids: Vec<String> = coll["collection"]
.as_array()
.unwrap()
.iter()
.take(2)
.map(|c| c["owned_card_id"].as_str().unwrap().to_string())
.collect();
let ext_write = serde_json::json!({
"namespace": "fifa17.squad", "schema_version": 1, "payload": "{\"custom\":\"[1]\"}"
});
let client_reported = serde_json::json!({
"client_reported_chemistry": 52,
"client_reported_rating": 90,
"client_reported_star_rating": 90
});
let populate = serde_json::json!({
"name": "OpenFUT",
"formation": "f442",
"slots": [
{"owned_card_id": ids[0], "slot": 0, "is_captain": true, "is_on_bench": false},
{"owned_card_id": ids[1], "slot": 1, "is_captain": false, "is_on_bench": false},
],
"client_reported": client_reported,
"extension": ext_write,
});
let (s, put) = json_put(&app, "/squad/replace", populate).await;
assert_eq!(s, StatusCode::OK, "{put}");
assert_eq!(put["slots_written"], 2);
// The destructive write: a well-formed replacement that simply carries no
// slots. It must be REFUSED, not applied — this is the exact shape that
// emptied a real squad.
let (s, err) = json_put(
&app,
"/squad/replace",
serde_json::json!({
"name": "OpenFUT",
"formation": "f442",
"slots": [],
"client_reported": client_reported,
"extension": ext_write,
}),
)
.await;
assert_eq!(
s,
StatusCode::BAD_REQUEST,
"an empty replacement must be refused, not applied: {err}"
);
// The squad is untouched — the refusal rolled back, it did not half-apply.
let (s, ext) = json_get(&app, "/squad/ext?namespace=fifa17.squad").await;
assert_eq!(s, StatusCode::OK);
assert_eq!(
ext["players"].as_array().unwrap().len(),
2,
"both assignments survive the refused replacement"
);
}
/// A role-only patch must move the captain and re-anchor the extension WITHOUT
/// disturbing a single assignment.
///
/// Regression: FIFA 17's captain/kick-taker screen sends a body with no
/// `players`, which the host presented to `/squad/replace` as a replacement
/// carrying zero slots. The empty-replacement guard correctly refused it, so
/// every captain change died with a 400 (surfaced to the client as 502). The
/// operation, not the guard, was wrong.
#[tokio::test]
async fn test_squad_roles_patch_moves_captain_without_touching_assignments() {
let app = build_test_app().await;
auth(&app, "RolePatchUser").await;
let (_, packs) = json_get(&app, "/packs").await;
let pack_id = packs["packs"][0]["pack_id"].as_str().unwrap().to_string();
json_post(
&app,
&format!("/packs/open/{pack_id}"),
serde_json::json!({}),
)
.await;
let (_, coll) = json_get(&app, "/collection").await;
let ids: Vec<String> = coll["collection"]
.as_array()
.unwrap()
.iter()
.take(2)
.map(|c| c["owned_card_id"].as_str().unwrap().to_string())
.collect();
let client_reported = serde_json::json!({
"client_reported_chemistry": 52,
"client_reported_rating": 90,
"client_reported_star_rating": 90
});
let (s, put) = json_put(
&app,
"/squad/replace",
serde_json::json!({
"name": "OpenFUT",
"formation": "f442",
"slots": [
{"owned_card_id": ids[0], "slot": 0, "is_captain": true, "is_on_bench": false},
{"owned_card_id": ids[1], "slot": 1, "is_captain": false, "is_on_bench": false},
],
"client_reported": client_reported,
"extension": {"namespace": "fifa17.squad", "schema_version": 1,
"payload": "{\"custom\":\"[1]\",\"kit_numbers\":{\"a\":7}}"},
}),
)
.await;
assert_eq!(s, StatusCode::OK, "{put}");
let before_fp = put["canonical_fingerprint"].as_str().unwrap().to_string();
// Move the captain to the second player, carrying a new opaque payload.
let (s, patched) = json_put(
&app,
"/squad/roles",
serde_json::json!({
"captain_owned_card_id": ids[1],
"extension": {"namespace": "fifa17.squad", "schema_version": 1,
"payload": "{\"custom\":\"[0,8,16]\",\"kit_numbers\":{\"a\":7}}"},
}),
)
.await;
assert_eq!(s, StatusCode::OK, "{patched}");
assert_eq!(patched["captain_changed"], true);
assert_ne!(
patched["canonical_fingerprint"].as_str().unwrap(),
before_fp,
"the captain is part of the fingerprint, so a captain move MUST re-anchor it"
);
let (s, ext) = json_get(&app, "/squad/ext?namespace=fifa17.squad").await;
assert_eq!(s, StatusCode::OK);
let players = ext["players"].as_array().unwrap();
assert_eq!(players.len(), 2, "a role patch must not add or drop slots");
let captain_of = |owned: &str| -> bool {
players
.iter()
.find(|p| p["owned_card_id"] == owned)
.map(|p| p["is_captain"] == true)
.unwrap_or(false)
};
assert!(captain_of(&ids[1]), "the new captain is flagged");
assert!(!captain_of(&ids[0]), "the previous captain is cleared");
// Fresh, not stale: the patch re-anchored the extension it wrote.
assert_eq!(
ext["extension"]["payload"], "{\"custom\":\"[0,8,16]\",\"kit_numbers\":{\"a\":7}}",
"the patch's payload is the one stored"
);
}
/// A role patch naming a captain who is not in the squad must change NOTHING —
/// not the captain, not the extension. All-or-nothing, validated before any write.
#[tokio::test]
async fn test_squad_roles_patch_rejects_unfielded_captain_and_rolls_back() {
let app = build_test_app().await;
auth(&app, "RolePatchRollbackUser").await;
let (_, packs) = json_get(&app, "/packs").await;
let pack_id = packs["packs"][0]["pack_id"].as_str().unwrap().to_string();
json_post(
&app,
&format!("/packs/open/{pack_id}"),
serde_json::json!({}),
)
.await;
let (_, coll) = json_get(&app, "/collection").await;
let ids: Vec<String> = coll["collection"]
.as_array()
.unwrap()
.iter()
.take(3)
.map(|c| c["owned_card_id"].as_str().unwrap().to_string())
.collect();
let original_payload = "{\"custom\":\"[1]\"}";
let (s, _) = json_put(
&app,
"/squad/replace",
serde_json::json!({
"name": "OpenFUT",
"formation": "f442",
"slots": [
{"owned_card_id": ids[0], "slot": 0, "is_captain": true, "is_on_bench": false},
{"owned_card_id": ids[1], "slot": 1, "is_captain": false, "is_on_bench": false},
],
"client_reported": serde_json::json!({}),
"extension": {"namespace": "fifa17.squad", "schema_version": 1,
"payload": original_payload},
}),
)
.await;
assert_eq!(s, StatusCode::OK);
// ids[2] is owned but NOT fielded — a patch must not accept it.
let (s, err) = json_put(
&app,
"/squad/roles",
serde_json::json!({
"captain_owned_card_id": ids[2],
"extension": {"namespace": "fifa17.squad", "schema_version": 1,
"payload": "{\"custom\":\"[9,9,9]\"}"},
}),
)
.await;
assert_eq!(
s,
StatusCode::BAD_REQUEST,
"a captain not assigned to the squad must be refused: {err}"
);
let (s, ext) = json_get(&app, "/squad/ext?namespace=fifa17.squad").await;
assert_eq!(s, StatusCode::OK);
let players = ext["players"].as_array().unwrap();
assert!(
players
.iter()
.any(|p| p["owned_card_id"] == ids[0].as_str() && p["is_captain"] == true),
"the original captain survives a refused patch"
);
assert_eq!(
ext["extension"]["payload"], original_payload,
"the extension must NOT be written when the captain is refused"
);
}
/// `PUT /club/manager` must keep three states apart: absent = say nothing,
/// explicit null = remove, id = assign.
///
/// Regression: `owned_card_id` was a plain `Option<String>`, so serde collapsed
/// "field absent" and "field null" into the same `None` and the route treated
/// both as a clear. A caller with nothing to say therefore DELETED the manager —
/// how a FIFA 17 client with a destroyed squad model wiped a real manager row
/// (WAL commit 468, squad_managers 1 -> 0).
#[tokio::test]
async fn test_manager_absent_field_leaves_assignment_untouched() {
let app = build_test_app().await;
auth(&app, "ManagerGuardUser").await;
let (_, packs) = json_get(&app, "/packs").await;
let pack_id = packs["packs"][0]["pack_id"].as_str().unwrap().to_string();
json_post(
&app,
&format!("/packs/open/{pack_id}"),
serde_json::json!({}),
)
.await;
let (_, coll) = json_get(&app, "/collection").await;
let ids: Vec<String> = coll["collection"]
.as_array()
.unwrap()
.iter()
.take(3)
.map(|c| c["owned_card_id"].as_str().unwrap().to_string())
.collect();
// A squad must exist for a manager to attach to.
let (s, _) = json_put(
&app,
"/squad/replace",
serde_json::json!({
"name": "OpenFUT", "formation": "f442",
"slots": [{"owned_card_id": ids[0], "slot": 0, "is_captain": true, "is_on_bench": false}],
"client_reported": {"client_reported_chemistry": 50, "client_reported_rating": 80,
"client_reported_star_rating": 80},
"extension": {"namespace": "fifa17.squad", "schema_version": 1, "payload": "{}"},
}),
)
.await;
assert_eq!(s, StatusCode::OK);
// Assign.
let (s, body) = json_put(
&app,
"/club/manager",
serde_json::json!({"owned_card_id": ids[1]}),
)
.await;
assert_eq!(s, StatusCode::OK, "{body}");
assert_eq!(body["manager"]["id"], ids[1].as_str());
// ABSENT field: the destructive shape. Must change nothing.
let (s, body) = json_put(&app, "/club/manager", serde_json::json!({})).await;
assert_eq!(s, StatusCode::OK, "{body}");
assert_eq!(
body["manager"]["id"],
ids[1].as_str(),
"an absent owned_card_id must LEAVE the manager, never clear it"
);
// Reassign to a different owned card: authentic, still allowed.
let (s, body) = json_put(
&app,
"/club/manager",
serde_json::json!({"owned_card_id": ids[2]}),
)
.await;
assert_eq!(s, StatusCode::OK, "{body}");
assert_eq!(body["manager"]["id"], ids[2].as_str());
// Same manager again: idempotent no-op, still assigned.
let (s, body) = json_put(
&app,
"/club/manager",
serde_json::json!({"owned_card_id": ids[2]}),
)
.await;
assert_eq!(s, StatusCode::OK, "{body}");
assert_eq!(body["manager"]["id"], ids[2].as_str());
// A card this club does not own is refused.
let (s, _) = json_put(
&app,
"/club/manager",
serde_json::json!({"owned_card_id": "not-a-real-owned-card"}),
)
.await;
assert_eq!(
s,
StatusCode::NOT_FOUND,
"an unowned manager must be refused"
);
let (_, body) = json_get(&app, "/club/manager").await;
assert_eq!(
body["manager"]["id"],
ids[2].as_str(),
"a refused assignment must not disturb the current manager"
);
// EXPLICIT null: a deliberate removal is legitimate and still works.
let (s, body) = json_put(
&app,
"/club/manager",
serde_json::json!({"owned_card_id": null}),
)
.await;
assert_eq!(s, StatusCode::OK, "{body}");
assert!(
body["manager"].is_null(),
"an explicit null must still remove the manager: {body}"
);
// Absent against a squad with NO manager: not over-guarded, plain no-op.
let (s, body) = json_put(&app, "/club/manager", serde_json::json!({})).await;
assert_eq!(s, StatusCode::OK, "{body}");
assert!(body["manager"].is_null());
// The squad's player assignment survived every one of those manager writes.
let (_, ext) = json_get(&app, "/squad/ext?namespace=fifa17.squad").await;
assert_eq!(
ext["players"].as_array().unwrap().len(),
1,
"manager writes must never disturb player assignments"
);
}
/// A malformed manager body is a PARSER rejection, distinguishable from the
/// guard's behaviour: a wrong-typed field is refused outright rather than being
/// silently treated as "absent" and passed through as a no-op.
#[tokio::test]
async fn test_manager_malformed_body_is_rejected_not_treated_as_absent() {
let app = build_test_app().await;
auth(&app, "ManagerMalformedUser").await;
let resp = app
.clone()
.oneshot(
Request::builder()
.method("PUT")
.uri("/club/manager")
.header("content-type", "application/json")
.body(Body::from(r#"{"owned_card_id": 12345}"#))
.unwrap(),
)
.await
.unwrap();
let s = resp.status();
assert!(
s == StatusCode::UNPROCESSABLE_ENTITY || s == StatusCode::BAD_REQUEST,
"a non-string owned_card_id must be a parser rejection, got {s}"
);
}
// ─────────────────────────── economy HTTP boundary ──────────────────────────
#[tokio::test]