From 9d9e4a345acd074ebd6c87dd2faca42379d25507 Mon Sep 17 00:00:00 2001 From: NotBigGhost Date: Wed, 9 Sep 2026 15:24:52 +0300 Subject: [PATCH] =?UTF-8?q?=D0=A0=D0=B5=D0=B2=D1=8C=D1=8E:=20=D0=B1=D0=B5?= =?UTF-8?q?=D0=B7=D0=BE=D0=BF=D0=B0=D1=81=D0=BD=D0=BE=D1=81=D1=82=D1=8C=20?= =?UTF-8?q?=D0=B8=20=D0=BA=D0=BE=D1=80=D1=80=D0=B5=D0=BA=D1=82=D0=BD=D0=BE?= =?UTF-8?q?=D1=81=D1=82=D1=8C=20=D0=B2=20=D1=81=D0=B5=D1=80=D0=B2=D0=B8?= =?UTF-8?q?=D1=81=D0=B0=D1=85=20=D0=B1=D1=8D=D0=BA=D0=B5=D0=BD=D0=B4=D0=B0?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Находки прохода /code-review high по backend/app: - achievement_service: slug из URL шёл в путь без проверки, из-за чего DELETE /api/admin/achievements/%2E%2E удалял rmtree'ом родительскую папку каталога ачивок (в проде это /data — БД, uploads, ачивки целиком). - match_service/attachment_service: версия партии = updated_at, но onupdate срабатывает лишь при реальном UPDATE строки matches. Правка одних участников и работа с вложениями его не вызывали, и оптимистичная блокировка молча пропускала конкурентную запись — бампаем updated_at явно. - admin_service: удаление группы с партиями упиралось в RESTRICT и уходило наружу голым 500; теперь понятная ошибка. Админское удаление партии не чистило файлы вложений с тома — они оставались навсегда. - user_service: при повторной загрузке аватара с тем же расширением avatar_path не менялся, updated_at не двигался, и кэш-бастер оставлял старую картинку до часа. Плюс версия считалась из наивного времени как из локального и разъезжалась с лидербордом, где то же поле считает SQL. - membership_service: единственный владелец мог разжаловать сам себя и группа оставалась без владельца навсегда. - notification_service: mark_read не слал SSE-сигнал, и бейдж непрочитанных на других устройствах висел до перезагрузки. - routers/admin: created_at после правки пользователя отдавался без смещения, и дата «создан» прыгала на часовой пояс до следующего обновления списка. #8 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_0186Fk74jkkszahEHSjBzTjD --- backend/app/routers/admin.py | 2 +- backend/app/services/achievement_service.py | 9 +++++++++ backend/app/services/admin_service.py | 10 ++++++++++ backend/app/services/attachment_service.py | 11 +++++++++++ backend/app/services/match_service.py | 4 ++++ backend/app/services/membership_service.py | 10 ++++++++++ backend/app/services/notification_service.py | 3 +++ backend/app/services/user_service.py | 16 ++++++++++++++-- 8 files changed, 62 insertions(+), 3 deletions(-) diff --git a/backend/app/routers/admin.py b/backend/app/routers/admin.py index 44b889f..ff006ff 100644 --- a/backend/app/routers/admin.py +++ b/backend/app/routers/admin.py @@ -114,7 +114,7 @@ def update_user( is_active=u.is_active, auth_provider=u.auth_provider, telegram_id=u.telegram_id, - created_at=u.created_at.isoformat(), + created_at=iso_utc(u.created_at), ) diff --git a/backend/app/services/achievement_service.py b/backend/app/services/achievement_service.py index c01ec48..3bc1839 100644 --- a/backend/app/services/achievement_service.py +++ b/backend/app/services/achievement_service.py @@ -42,6 +42,10 @@ def _root() -> Path: return Path(settings.achievements_dir) +# Формат slug — то, что выдаёт _slugify: только латиница, цифры и дефис. +_SLUG_RE = re.compile(r"[a-z0-9][a-z0-9-]{0,63}") + + def _slugify(name: str) -> str: text = "".join(_TRANSLIT.get(ch, ch) for ch in (name or "").strip().lower()) slug = re.sub(r"[^a-z0-9]+", "-", text).strip("-") @@ -49,6 +53,11 @@ def _slugify(name: str) -> str: def _dir(slug: str) -> Path: + """Папка ачивки. Slug приходит из URL, поэтому формат проверяем здесь: без этого + `..` или `a/b` увели бы файловые операции (вплоть до rmtree в delete) за пределы + каталога ачивок.""" + if not _SLUG_RE.fullmatch(slug or ""): + raise NotFoundError("Ачивка не найдена.") return _root() / slug diff --git a/backend/app/services/admin_service.py b/backend/app/services/admin_service.py index 4616ea1..552f21d 100644 --- a/backend/app/services/admin_service.py +++ b/backend/app/services/admin_service.py @@ -6,6 +6,7 @@ from typing import Any from sqlmodel import Session, select from app.core.errors import ( + ConflictError, InvalidCredentialsError, NicknameTakenError, NotFoundError, @@ -75,6 +76,10 @@ def delete_group(session: Session, group_id: int) -> None: group = session.get(Group, group_id) if group is None: raise NotFoundError("Группа не найдена.") + # matches.group_id — ON DELETE RESTRICT, поэтому группу с партиями БД не отдаст. + # Проверяем сами, иначе IntegrityError уходит наружу голым 500 без AppError-конверта. + if session.exec(select(Match.id).where(Match.group_id == group_id)).first() is not None: + raise ConflictError("Нельзя удалить группу, в которой есть партии. Сначала удалите их.") session.delete(group) session.commit() @@ -134,11 +139,16 @@ def rename_faction(session: Session, faction_id: int, name_ru: str) -> Faction: def delete_match(session: Session, match_id: int) -> None: + from app.services import attachment_service # избегаем цикла импорта + match = session.get(Match, match_id) if match is None: raise NotFoundError("Партия не найдена.") session.delete(match) session.commit() + # Как и в игроцком пути (match_service.delete_match): строки вложений уходят + # каскадом, а файлы с тома нужно убрать руками, иначе они остаются навсегда. + attachment_service.delete_match_files(match_id) # ─── Журнал аудита ─────────────────────────────────────────────────────────── diff --git a/backend/app/services/attachment_service.py b/backend/app/services/attachment_service.py index 5491e33..eabfe2e 100644 --- a/backend/app/services/attachment_service.py +++ b/backend/app/services/attachment_service.py @@ -11,6 +11,7 @@ from sqlmodel import Session, select from app.core.config import settings from app.core.errors import ConflictError, NotFoundError +from app.core.timeutil import utcnow from app.models import Match, MatchAttachment, User MAX_ATTACHMENTS = 10 @@ -39,6 +40,14 @@ def file_path(att: MatchAttachment) -> Path: return Path(settings.upload_dir) / att.storage_path +def _touch(session: Session, match: Match) -> None: + """Двинуть версию партии: набор вложений виден в MatchRead, а сама строка matches + при работе с ними не меняется — без этого оптимистичная блокировка проспала бы + конкурентную правку.""" + match.updated_at = utcnow() + session.add(match) + + def add_photo( session: Session, match: Match, user: User, content: bytes, ext: str, mime: str ) -> MatchAttachment: @@ -60,6 +69,7 @@ def add_photo( abs_path.write_bytes(content) att.storage_path = rel session.add(att) + _touch(session, match) session.commit() session.refresh(att) return att @@ -81,6 +91,7 @@ def delete(session: Session, match: Match, att_id: int) -> None: except OSError: pass session.delete(att) + _touch(session, match) session.commit() diff --git a/backend/app/services/match_service.py b/backend/app/services/match_service.py index 00ead8e..e3d7ecb 100644 --- a/backend/app/services/match_service.py +++ b/backend/app/services/match_service.py @@ -351,6 +351,10 @@ def update_match( ) match.player_count = len(participants) + # Версия партии = updated_at, а onupdate срабатывает только при реальном UPDATE + # строки matches. Правка одних участников его не вызывает, и тогда оптимистичная + # блокировка молча пропускала бы конкурентную запись — поэтому бампаем явно. + match.updated_at = _utcnow() session.add(match) session.commit() session.refresh(match) diff --git a/backend/app/services/membership_service.py b/backend/app/services/membership_service.py index d4a83b6..12b4dd4 100644 --- a/backend/app/services/membership_service.py +++ b/backend/app/services/membership_service.py @@ -85,6 +85,16 @@ def change_role(session: Session, group: Group, user_id: int, role: str) -> Grou ).first() if member is None: raise NotFoundError("Игрок не состоит в группе.") + if member.role == "owner" and role != "owner": + # Без этого единственный владелец мог разжаловать сам себя, и группа + # оставалась без владельца навсегда: назначить нового уже некому. + owners = session.exec( + select(GroupMember).where( + GroupMember.group_id == group.id, GroupMember.role == "owner" + ) + ).all() + if len(owners) <= 1: + raise ForbiddenError("Нельзя снять роль с последнего владельца группы.") member.role = role session.add(member) session.commit() diff --git a/backend/app/services/notification_service.py b/backend/app/services/notification_service.py index cd3c80d..c7e33b2 100644 --- a/backend/app/services/notification_service.py +++ b/backend/app/services/notification_service.py @@ -137,6 +137,9 @@ def mark_read(session: Session, user_id: int, ids: list[int] | None = None) -> i session.add(row) if rows: session.commit() + # Счётчик непрочитанных изменился — толкаем тот же сигнал, что и create_for, + # иначе вкладка на другом устройстве держит устаревший бейдж до перезагрузки. + notify.notifications_changed(user_id) return len(rows) diff --git a/backend/app/services/user_service.py b/backend/app/services/user_service.py index ea6d552..1be90f3 100644 --- a/backend/app/services/user_service.py +++ b/backend/app/services/user_service.py @@ -3,7 +3,7 @@ from __future__ import annotations import os import re -from datetime import datetime +from datetime import datetime, timezone from pathlib import Path from sqlmodel import Session, select @@ -11,6 +11,7 @@ from sqlmodel import Session, select from app.auth.provider import ExternalIdentity from app.core.config import settings from app.core.errors import NicknameTakenError, NotFoundError, ValidationError +from app.core.timeutil import utcnow from app.models import AuthIdentity, Faction, GroupMember, User _NICK_RE = re.compile(r"^[\w .\-]{2,64}$", re.UNICODE) @@ -170,7 +171,13 @@ def avatar_url_for(user_id: int, avatar_path: str | None, updated_at: datetime | подтягивал новую картинку после смены (файл перезаписывается по тому же пути).""" if not avatar_path: return None - version = int(updated_at.timestamp()) if updated_at else 0 + # В БД время наивное и хранится в UTC. .timestamp() у наивного значения считает + # его локальным, и версия разъезжалась с лидербордом, где то же поле считает SQL + # (strftime('%s') читает его как UTC) — один аватар качался браузером дважды. + version = 0 + if updated_at is not None: + aware = updated_at if updated_at.tzinfo else updated_at.replace(tzinfo=timezone.utc) + version = int(aware.timestamp()) return f"/api/users/{user_id}/avatar?v={version}" @@ -253,6 +260,10 @@ def set_avatar(session: Session, user: User, content: bytes, ext: str) -> User: rel = f"{_AVATAR_SUBDIR}/{user.id}.{ext}" (Path(settings.upload_dir) / rel).write_bytes(content) user.avatar_path = rel + # Файл перезаписывается по тому же пути, поэтому при том же расширении avatar_path + # не меняется, UPDATE не эмитится и onupdate не срабатывает. Без явного бампа + # кэш-бастер остаётся прежним, и браузер час показывает старую картинку. + user.updated_at = utcnow() session.add(user) session.commit() session.refresh(user) @@ -267,6 +278,7 @@ def clear_avatar(session: Session, user: User) -> User: except OSError: pass user.avatar_path = None + user.updated_at = utcnow() session.add(user) session.commit() session.refresh(user)