Партии: оптимистичная блокировка (version) — устаревшие правки/отмена отклоняются (фикс гонки завершить-vs-отменить)
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -229,6 +229,7 @@ def update_match(
|
|||||||
win_reason=body.win_reason,
|
win_reason=body.win_reason,
|
||||||
win_reason_set=("win_reason" in body.model_fields_set),
|
win_reason_set=("win_reason" in body.model_fields_set),
|
||||||
participants=participants,
|
participants=participants,
|
||||||
|
expected_version=body.expected_version,
|
||||||
)
|
)
|
||||||
audit_service.record(
|
audit_service.record(
|
||||||
session,
|
session,
|
||||||
|
|||||||
@@ -1,7 +1,7 @@
|
|||||||
"""Роутер партий: рандом фракции, старт, завершение, детали, правка, удаление."""
|
"""Роутер партий: рандом фракции, старт, завершение, детали, правка, удаление."""
|
||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
from fastapi import APIRouter, Depends, File, Request, UploadFile
|
from fastapi import APIRouter, Depends, File, Query, Request, UploadFile
|
||||||
from fastapi.responses import FileResponse
|
from fastapi.responses import FileResponse
|
||||||
from sqlmodel import Session
|
from sqlmodel import Session
|
||||||
|
|
||||||
@@ -69,6 +69,7 @@ def build_match_read(session: Session, match: Match, *, can_modify: bool = False
|
|||||||
overall_comment=match.overall_comment,
|
overall_comment=match.overall_comment,
|
||||||
created_by=match.created_by,
|
created_by=match.created_by,
|
||||||
can_modify=can_modify,
|
can_modify=can_modify,
|
||||||
|
version=match_service.match_version(match),
|
||||||
participants=parts,
|
participants=parts,
|
||||||
attachments=[
|
attachments=[
|
||||||
attachment_read(a, f"/api/matches/{match.id}")
|
attachment_read(a, f"/api/matches/{match.id}")
|
||||||
@@ -143,6 +144,7 @@ def finish_match(
|
|||||||
win_reason=body.win_reason,
|
win_reason=body.win_reason,
|
||||||
overall_comment=body.overall_comment,
|
overall_comment=body.overall_comment,
|
||||||
overall_comment_set=("overall_comment" in body.model_fields_set),
|
overall_comment_set=("overall_comment" in body.model_fields_set),
|
||||||
|
expected_version=body.expected_version,
|
||||||
)
|
)
|
||||||
audit_service.record(
|
audit_service.record(
|
||||||
session,
|
session,
|
||||||
@@ -200,6 +202,7 @@ def update_match(
|
|||||||
win_reason=body.win_reason,
|
win_reason=body.win_reason,
|
||||||
win_reason_set=("win_reason" in body.model_fields_set),
|
win_reason_set=("win_reason" in body.model_fields_set),
|
||||||
participants=participants,
|
participants=participants,
|
||||||
|
expected_version=body.expected_version,
|
||||||
)
|
)
|
||||||
audit_service.record(
|
audit_service.record(
|
||||||
session,
|
session,
|
||||||
@@ -278,6 +281,7 @@ def get_attachment(
|
|||||||
def delete_match(
|
def delete_match(
|
||||||
match_id: int,
|
match_id: int,
|
||||||
request: Request,
|
request: Request,
|
||||||
|
expected_version: str | None = Query(None),
|
||||||
session: Session = Depends(get_session),
|
session: Session = Depends(get_session),
|
||||||
user: User = Depends(get_current_user),
|
user: User = Depends(get_current_user),
|
||||||
) -> s.OkResponse:
|
) -> s.OkResponse:
|
||||||
@@ -285,7 +289,7 @@ def delete_match(
|
|||||||
match_service.assert_can_modify(session, match, user)
|
match_service.assert_can_modify(session, match, user)
|
||||||
match_id_val = match.id
|
match_id_val = match.id
|
||||||
group_id_val = match.group_id
|
group_id_val = match.group_id
|
||||||
match_service.delete_match(session, match)
|
match_service.delete_match(session, match, expected_version=expected_version)
|
||||||
audit_service.record(
|
audit_service.record(
|
||||||
session,
|
session,
|
||||||
actor_id=user.id,
|
actor_id=user.id,
|
||||||
|
|||||||
@@ -192,6 +192,8 @@ class MatchFinish(BaseModel):
|
|||||||
participants: list[MatchFinishParticipant]
|
participants: list[MatchFinishParticipant]
|
||||||
win_reason: WinReason
|
win_reason: WinReason
|
||||||
overall_comment: str | None = None
|
overall_comment: str | None = None
|
||||||
|
# Оптимистичная блокировка: версия партии, которую видел клиент (см. MatchRead.version).
|
||||||
|
expected_version: str | None = None
|
||||||
|
|
||||||
|
|
||||||
# Полный участник (правка завершённой партии админом).
|
# Полный участник (правка завершённой партии админом).
|
||||||
@@ -208,6 +210,7 @@ class MatchUpdate(BaseModel):
|
|||||||
overall_comment: str | None = None
|
overall_comment: str | None = None
|
||||||
win_reason: WinReason | None = None
|
win_reason: WinReason | None = None
|
||||||
participants: list[ParticipantInput] | None = None
|
participants: list[ParticipantInput] | None = None
|
||||||
|
expected_version: str | None = None # оптимистичная блокировка
|
||||||
|
|
||||||
|
|
||||||
class MatchParticipantRead(BaseModel):
|
class MatchParticipantRead(BaseModel):
|
||||||
@@ -242,6 +245,7 @@ class MatchRead(BaseModel):
|
|||||||
overall_comment: str | None = None
|
overall_comment: str | None = None
|
||||||
created_by: int
|
created_by: int
|
||||||
can_modify: bool = False # может ли текущий зритель править/завершать партию
|
can_modify: bool = False # может ли текущий зритель править/завершать партию
|
||||||
|
version: str # для оптимистичной блокировки (iso updated_at); клиент шлёт обратно
|
||||||
participants: list[MatchParticipantRead] = []
|
participants: list[MatchParticipantRead] = []
|
||||||
attachments: list[AttachmentRead] = []
|
attachments: list[AttachmentRead] = []
|
||||||
|
|
||||||
|
|||||||
@@ -16,7 +16,7 @@ from app.core.errors import (
|
|||||||
NotFoundError,
|
NotFoundError,
|
||||||
ValidationError,
|
ValidationError,
|
||||||
)
|
)
|
||||||
from app.core.timeutil import app_today
|
from app.core.timeutil import app_today, iso_utc
|
||||||
from app.models import Faction, GroupMember, Match, MatchParticipant, User
|
from app.models import Faction, GroupMember, Match, MatchParticipant, User
|
||||||
from app.services import group_service
|
from app.services import group_service
|
||||||
|
|
||||||
@@ -70,6 +70,20 @@ def get_match(session: Session, match_id: int) -> Match:
|
|||||||
return match
|
return match
|
||||||
|
|
||||||
|
|
||||||
|
def match_version(match: Match) -> str:
|
||||||
|
"""Версия партии для оптимистичной блокировки (меняется при любом изменении)."""
|
||||||
|
return iso_utc(match.updated_at)
|
||||||
|
|
||||||
|
|
||||||
|
def assert_version(match: Match, expected: str | None) -> None:
|
||||||
|
"""Если клиент прислал версию и она устарела — отказываем (кто-то изменил партию)."""
|
||||||
|
if expected is not None and expected != match_version(match):
|
||||||
|
raise ConflictError(
|
||||||
|
"Партия уже изменена на другом устройстве — обновите страницу.",
|
||||||
|
code="STALE_WRITE",
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
def participants_detail(
|
def participants_detail(
|
||||||
session: Session, match_id: int
|
session: Session, match_id: int
|
||||||
) -> list[tuple[MatchParticipant, User, Faction]]:
|
) -> list[tuple[MatchParticipant, User, Faction]]:
|
||||||
@@ -193,7 +207,9 @@ def finish_match(
|
|||||||
win_reason: str,
|
win_reason: str,
|
||||||
overall_comment: str | None = None,
|
overall_comment: str | None = None,
|
||||||
overall_comment_set: bool = False,
|
overall_comment_set: bool = False,
|
||||||
|
expected_version: str | None = None,
|
||||||
) -> Match:
|
) -> Match:
|
||||||
|
assert_version(match, expected_version)
|
||||||
if match.status != "in_progress":
|
if match.status != "in_progress":
|
||||||
raise ConflictError("Партия уже завершена.")
|
raise ConflictError("Партия уже завершена.")
|
||||||
if win_reason not in WIN_REASONS:
|
if win_reason not in WIN_REASONS:
|
||||||
@@ -274,8 +290,10 @@ def update_match(
|
|||||||
win_reason: str | None = None,
|
win_reason: str | None = None,
|
||||||
win_reason_set: bool = False,
|
win_reason_set: bool = False,
|
||||||
participants: list[ParticipantInput] | None = None,
|
participants: list[ParticipantInput] | None = None,
|
||||||
|
expected_version: str | None = None,
|
||||||
) -> Match:
|
) -> Match:
|
||||||
"""Правка завершённой партии (админ): полный список участников с местами."""
|
"""Правка завершённой партии (админ): полный список участников с местами."""
|
||||||
|
assert_version(match, expected_version)
|
||||||
if played_at is not None:
|
if played_at is not None:
|
||||||
match.played_at = played_at
|
match.played_at = played_at
|
||||||
if overall_comment_set:
|
if overall_comment_set:
|
||||||
@@ -317,9 +335,10 @@ def update_match(
|
|||||||
return match
|
return match
|
||||||
|
|
||||||
|
|
||||||
def delete_match(session: Session, match: Match) -> None:
|
def delete_match(session: Session, match: Match, expected_version: str | None = None) -> None:
|
||||||
from app.services import attachment_service # избегаем цикла импорта
|
from app.services import attachment_service # избегаем цикла импорта
|
||||||
|
|
||||||
|
assert_version(match, expected_version)
|
||||||
match_id = match.id
|
match_id = match.id
|
||||||
session.delete(match) # участники и вложения (БД) удалятся каскадом (FK ON DELETE CASCADE)
|
session.delete(match) # участники и вложения (БД) удалятся каскадом (FK ON DELETE CASCADE)
|
||||||
session.commit()
|
session.commit()
|
||||||
|
|||||||
@@ -0,0 +1,77 @@
|
|||||||
|
"""Оптимистичная блокировка партии: устаревшие правки/удаление отклоняются (STALE_WRITE)."""
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
from fastapi.testclient import TestClient
|
||||||
|
|
||||||
|
from tests.conftest import add_group_member, csrf_headers, finish_match, login, start_match
|
||||||
|
|
||||||
|
|
||||||
|
def _start(client: TestClient, engine) -> tuple[dict, int, int]:
|
||||||
|
me = login(client, "Хост")
|
||||||
|
exps = [e["id"] for e in client.get("/api/expansions").json()]
|
||||||
|
gid = client.post(
|
||||||
|
"/api/groups", json={"name": "Группа", "expansion_ids": exps}, headers=csrf_headers(client)
|
||||||
|
).json()["id"]
|
||||||
|
p2 = add_group_member(engine, gid, "Игрок2")
|
||||||
|
fids = [f["id"] for f in client.get(f"/api/groups/{gid}/factions").json()]
|
||||||
|
started = start_match(
|
||||||
|
client, gid,
|
||||||
|
[{"user_id": me["id"], "faction_id": fids[0]}, {"user_id": p2, "faction_id": fids[1]}],
|
||||||
|
)
|
||||||
|
assert started.status_code == 200, started.text
|
||||||
|
return me, p2, started.json()["id"]
|
||||||
|
|
||||||
|
|
||||||
|
def test_stale_delete_rejected(client: TestClient, engine):
|
||||||
|
"""Сценарий бага: ПК завершил, телефон со старой версией жмёт «Отменить»."""
|
||||||
|
me, p2, mid = _start(client, engine)
|
||||||
|
v1 = client.get(f"/api/matches/{mid}").json()["version"]
|
||||||
|
|
||||||
|
# «ПК» завершает партию — версия меняется.
|
||||||
|
fin = finish_match(
|
||||||
|
client, mid, [{"user_id": me["id"], "place": 1}, {"user_id": p2, "place": 2}],
|
||||||
|
win_reason="objectives",
|
||||||
|
)
|
||||||
|
assert fin.status_code == 200, fin.text
|
||||||
|
|
||||||
|
# «Телефон» со старой версией пытается отменить → 409 STALE_WRITE, партия НЕ удаляется.
|
||||||
|
stale = client.delete(
|
||||||
|
f"/api/matches/{mid}", params={"expected_version": v1}, headers=csrf_headers(client)
|
||||||
|
)
|
||||||
|
assert stale.status_code == 409, stale.text
|
||||||
|
assert stale.json()["error"]["code"] == "STALE_WRITE"
|
||||||
|
assert client.get(f"/api/matches/{mid}").status_code == 200 # жива
|
||||||
|
|
||||||
|
# С актуальной версией удаление проходит.
|
||||||
|
v2 = client.get(f"/api/matches/{mid}").json()["version"]
|
||||||
|
ok = client.delete(
|
||||||
|
f"/api/matches/{mid}", params={"expected_version": v2}, headers=csrf_headers(client)
|
||||||
|
)
|
||||||
|
assert ok.status_code == 200, ok.text
|
||||||
|
assert client.get(f"/api/matches/{mid}").status_code == 404
|
||||||
|
|
||||||
|
|
||||||
|
def test_stale_finish_rejected(client: TestClient, engine):
|
||||||
|
me, p2, mid = _start(client, engine)
|
||||||
|
v1 = client.get(f"/api/matches/{mid}").json()["version"]
|
||||||
|
|
||||||
|
# Партию изменили (правка комментария) — версия устарела.
|
||||||
|
bump = client.patch(
|
||||||
|
f"/api/matches/{mid}", json={"overall_comment": "правка"}, headers=csrf_headers(client)
|
||||||
|
)
|
||||||
|
assert bump.status_code == 200, bump.text
|
||||||
|
|
||||||
|
# Завершение со старой версией → 409 STALE_WRITE.
|
||||||
|
r = client.post(
|
||||||
|
f"/api/matches/{mid}/finish",
|
||||||
|
json={
|
||||||
|
"participants": [
|
||||||
|
{"user_id": me["id"], "place": 1},
|
||||||
|
{"user_id": p2, "place": 2},
|
||||||
|
],
|
||||||
|
"win_reason": "objectives",
|
||||||
|
"expected_version": v1,
|
||||||
|
},
|
||||||
|
headers=csrf_headers(client),
|
||||||
|
)
|
||||||
|
assert r.status_code == 409 and r.json()["error"]["code"] == "STALE_WRITE", r.text
|
||||||
Vendored
+9
-1
@@ -1507,6 +1507,8 @@ export interface components {
|
|||||||
win_reason: "objectives" | "worlds" | "plastic" | "resources";
|
win_reason: "objectives" | "worlds" | "plastic" | "resources";
|
||||||
/** Overall Comment */
|
/** Overall Comment */
|
||||||
overall_comment?: string | null;
|
overall_comment?: string | null;
|
||||||
|
/** Expected Version */
|
||||||
|
expected_version?: string | null;
|
||||||
};
|
};
|
||||||
/** MatchFinishParticipant */
|
/** MatchFinishParticipant */
|
||||||
MatchFinishParticipant: {
|
MatchFinishParticipant: {
|
||||||
@@ -1627,6 +1629,8 @@ export interface components {
|
|||||||
* @default false
|
* @default false
|
||||||
*/
|
*/
|
||||||
can_modify: boolean;
|
can_modify: boolean;
|
||||||
|
/** Version */
|
||||||
|
version: string;
|
||||||
/**
|
/**
|
||||||
* Participants
|
* Participants
|
||||||
* @default []
|
* @default []
|
||||||
@@ -1648,6 +1652,8 @@ export interface components {
|
|||||||
win_reason?: ("objectives" | "worlds" | "plastic" | "resources") | null;
|
win_reason?: ("objectives" | "worlds" | "plastic" | "resources") | null;
|
||||||
/** Participants */
|
/** Participants */
|
||||||
participants?: components["schemas"]["ParticipantInput"][] | null;
|
participants?: components["schemas"]["ParticipantInput"][] | null;
|
||||||
|
/** Expected Version */
|
||||||
|
expected_version?: string | null;
|
||||||
};
|
};
|
||||||
/** MeRead */
|
/** MeRead */
|
||||||
MeRead: {
|
MeRead: {
|
||||||
@@ -2882,7 +2888,9 @@ export interface operations {
|
|||||||
};
|
};
|
||||||
delete_match_api_matches__match_id__delete: {
|
delete_match_api_matches__match_id__delete: {
|
||||||
parameters: {
|
parameters: {
|
||||||
query?: never;
|
query?: {
|
||||||
|
expected_version?: string | null;
|
||||||
|
};
|
||||||
header?: never;
|
header?: never;
|
||||||
path: {
|
path: {
|
||||||
match_id: number;
|
match_id: number;
|
||||||
|
|||||||
@@ -70,10 +70,14 @@ export function useFinishMatch() {
|
|||||||
export function useDeleteMatch() {
|
export function useDeleteMatch() {
|
||||||
const qc = useQueryClient();
|
const qc = useQueryClient();
|
||||||
return useMutation({
|
return useMutation({
|
||||||
mutationFn: async (matchId: number) =>
|
// expectedVersion — оптимистичная блокировка: отмена устаревшей версии вернёт 409 STALE_WRITE.
|
||||||
|
mutationFn: async (args: { matchId: number; expectedVersion?: string }) =>
|
||||||
unwrap(
|
unwrap(
|
||||||
await api.DELETE("/api/matches/{match_id}", {
|
await api.DELETE("/api/matches/{match_id}", {
|
||||||
params: { path: { match_id: matchId } },
|
params: {
|
||||||
|
path: { match_id: args.matchId },
|
||||||
|
query: args.expectedVersion ? { expected_version: args.expectedVersion } : {},
|
||||||
|
},
|
||||||
}),
|
}),
|
||||||
),
|
),
|
||||||
onSuccess: () => qc.invalidateQueries(),
|
onSuccess: () => qc.invalidateQueries(),
|
||||||
|
|||||||
@@ -26,7 +26,7 @@ interface FinishRow {
|
|||||||
export function MatchDetailPage() {
|
export function MatchDetailPage() {
|
||||||
const { matchId } = useParams();
|
const { matchId } = useParams();
|
||||||
const id = matchId ? Number(matchId) : null;
|
const id = matchId ? Number(matchId) : null;
|
||||||
const { data: match, isLoading } = useMatch(id);
|
const { data: match, isLoading, refetch } = useMatch(id);
|
||||||
const finish = useFinishMatch();
|
const finish = useFinishMatch();
|
||||||
const del = useDeleteMatch();
|
const del = useDeleteMatch();
|
||||||
const uploadAtt = useUploadMatchAttachment(id ?? 0);
|
const uploadAtt = useUploadMatchAttachment(id ?? 0);
|
||||||
@@ -59,8 +59,11 @@ export function MatchDetailPage() {
|
|||||||
const upd = (i: number, patch: Partial<FinishRow>) =>
|
const upd = (i: number, patch: Partial<FinishRow>) =>
|
||||||
setRows(finishRows.map((r, idx) => (idx === i ? { ...r, ...patch } : r)));
|
setRows(finishRows.map((r, idx) => (idx === i ? { ...r, ...patch } : r)));
|
||||||
|
|
||||||
|
// Конфликт версий (кто-то изменил партию с другого устройства) → сообщаем и обновляем.
|
||||||
|
const isStale = (e: unknown) => e instanceof ApiError && e.code === "STALE_WRITE";
|
||||||
|
|
||||||
const submitFinish = async () => {
|
const submitFinish = async () => {
|
||||||
if (!id) return;
|
if (!id || !match) return;
|
||||||
setError(null);
|
setError(null);
|
||||||
try {
|
try {
|
||||||
await finish.mutateAsync({
|
await finish.mutateAsync({
|
||||||
@@ -73,19 +76,34 @@ export function MatchDetailPage() {
|
|||||||
})),
|
})),
|
||||||
win_reason: winReason,
|
win_reason: winReason,
|
||||||
overall_comment: overall.trim() || null,
|
overall_comment: overall.trim() || null,
|
||||||
|
expected_version: match.version,
|
||||||
},
|
},
|
||||||
});
|
});
|
||||||
toast.show("Партия завершена");
|
toast.show("Партия завершена");
|
||||||
} catch (e) {
|
} catch (e) {
|
||||||
|
if (isStale(e)) {
|
||||||
|
toast.show("Партия изменилась на другом устройстве — обновлено");
|
||||||
|
refetch();
|
||||||
|
return;
|
||||||
|
}
|
||||||
setError(e instanceof ApiError ? e.message : "Не удалось завершить партию");
|
setError(e instanceof ApiError ? e.message : "Не удалось завершить партию");
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
|
|
||||||
const remove = async () => {
|
const remove = async () => {
|
||||||
if (!id) return;
|
if (!id || !match) return;
|
||||||
await del.mutateAsync(id).catch(() => {});
|
try {
|
||||||
|
await del.mutateAsync({ matchId: id, expectedVersion: match.version });
|
||||||
toast.show(inProgress ? "Партия отменена" : "Партия удалена");
|
toast.show(inProgress ? "Партия отменена" : "Партия удалена");
|
||||||
navigate(-1);
|
navigate(-1);
|
||||||
|
} catch (e) {
|
||||||
|
if (isStale(e)) {
|
||||||
|
toast.show("Партия изменилась на другом устройстве — обновлено");
|
||||||
|
refetch();
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
toast.show(e instanceof ApiError ? e.message : "Не удалось выполнить");
|
||||||
|
}
|
||||||
};
|
};
|
||||||
|
|
||||||
const sorted = [...match.participants].sort(
|
const sorted = [...match.participants].sort(
|
||||||
|
|||||||
Reference in New Issue
Block a user