165 lines
12 KiB
Markdown
165 lines
12 KiB
Markdown
# TODO: привести Go-бэкенд к стандарту
|
||
|
||
Аудит проведён по `evening_detective_server` 2026-10-02.
|
||
|
||
## Результат проверки
|
||
|
||
- `gofmt -l` — нарушений форматирования не найдено.
|
||
- `go vet ./...` — замечаний нет.
|
||
- `go test ./...` — не проходит в sandbox: тесты gateway и MCP не могут открыть
|
||
локальный TCP-порт (`operation not permitted`). Остальные пакеты проходят.
|
||
- Сгенерированные `proto/*.pb.go`, Swagger, `.gotmp`, `.gocache` и
|
||
`.VSCodeCounter` не входят в ручной аудит.
|
||
|
||
## Приоритет P0 — сначала исправить границы приложения
|
||
|
||
### GO-001 — убрать прямые зависимости handler-ов от repos и технических modules
|
||
|
||
- Статус: `DONE` (этапы GO-001.1–GO-001.7 выполнены)
|
||
- Результат: runtime `internal/app` не импортирует `internal/repos`; error
|
||
mapping перенесён на границы services; одноиспользуемый `formatter_utils`
|
||
удалён.
|
||
- Проверка: `gofmt`, `go vet ./...`, `go test ./...` проходят.
|
||
|
||
- Стандарт нарушен: `internal/app` должен вызывать прикладные сервисы и делать
|
||
только transport mapping.
|
||
- Прямые repository-зависимости: `app/scenario_archive.go`, `app/team_qr.go`,
|
||
`app/team_registration.go`, `app/users.go`.
|
||
- Прямые module-зависимости с прикладной логикой/хранилищем: `app/files.go`,
|
||
`app/story_mapper.go`, `app/game_mapper.go`, `app/team_mapper.go`.
|
||
- Перенести операции в существующие сервисы, оставив в handler-е извлечение
|
||
запроса, вызов одного сервиса и преобразование ответа.
|
||
- Проверка: `go vet ./...`, targeted tests `internal/app/...`, затем `go test ./...`.
|
||
|
||
### GO-002 — разорвать неправильные зависимости modules от application layers
|
||
|
||
- `internal/modules/game_model` импортирует `internal/repos` и
|
||
`internal/services/scenarios_service`.
|
||
- `internal/modules/scenario_archive` импортирует типы `internal/repos`.
|
||
- Это нарушает правило: module не знает конкретный use case, service и storage
|
||
models не должны протекать в технический module.
|
||
- Оставить доменные типы у владельца; mapper-ы перенести на границы service или
|
||
app. Не вводить `common`/`utils` для обхода цикла.
|
||
- Проверка: `go list -deps`/ревью импортов и `go test ./...`.
|
||
|
||
### GO-003 — привести ошибки handler-ов к единому gRPC/gateway-механизму
|
||
|
||
- Во многих handler-ах (`answers.go`, `games.go`, `scenarios.go`, `teams.go`,
|
||
`users.go`, `game_reviews.go`, `tickets.go`) внутренняя ошибка превращается в
|
||
`err.Error()` внутри успешного protobuf-ответа.
|
||
- Это теряет тип ошибки и ведёт к разным правилам HTTP/gRPC mapping; стандарт
|
||
требует возвращать ошибку через существующий transport-механизм.
|
||
- Ввести/переиспользовать единый mapping sentinel-ошибок в одном месте; не
|
||
дублировать проверки по handler-ам и не отдавать внутренние тексты наружу.
|
||
- Добавить минимальные тесты на коды `NotFound`, `InvalidArgument`, `Conflict` и
|
||
`Internal` для representative endpoint-ов.
|
||
|
||
## Приоритет P1 — механические нарушения стандарта
|
||
|
||
### GO-004 — отформатировать SQL и вызовы pgx по правилам проекта
|
||
|
||
- Нарушены многострочный raw SQL, вертикальные clauses и многострочные вызовы.
|
||
- Зафиксированные примеры: `repos/screen_sessions_repo/repo.go` (несколько
|
||
`Exec` в одну строку), `repos/teams_repo/repo.go`,
|
||
`repos/teams_repo/participation.go`, `repos/tickets_repo/repo.go`,
|
||
`repos/users_repo/repo.go`.
|
||
- Пройти все runtime-файлы `internal/repos`, не менять SQL-смысл.
|
||
- Одновременно проверить для каждого `Query`: `defer rows.Close()` сразу после
|
||
успешного запроса и обязательный `rows.Err()`.
|
||
- Проверка: `gofmt -w` изменённых файлов, `go test ./internal/repos/...`.
|
||
|
||
### GO-005 — исправить имена конструкторов и интерфейсов
|
||
|
||
- Generic-конструкторы `New` находятся в `repos/health_repo/repo.go` и
|
||
`repos/tickets_repo/repo.go`; переименовать в предметные
|
||
`NewHealthRepo`/`NewTicketsRepo` и обновить wiring.
|
||
- Интерфейсы без обязательного префикса `I`: `GamePlayer`, `AnswerSubmitter`,
|
||
`usersRepository`; проверить также дублирующий `ICleaner` в
|
||
`modules/storytelling/dependency.go`.
|
||
- Не создавать новые интерфейсы: только переименовать или удалить дубликаты,
|
||
если это не ломает границу подмены.
|
||
- Проверка: `go test ./...`.
|
||
|
||
### GO-006 — убрать generic-пакеты `*_utils`
|
||
|
||
- Стандарт запрещает `utils` для несвязанных функций; найдены
|
||
`modules/formatter_utils` и `modules/network_utils`.
|
||
- Переименовать их по ответственности и обновить импорты: например, пакет
|
||
форматирования результата и пакет определения локальных адресов.
|
||
- Не переносить код и не добавлять общий helper-пакет без конкретного владельца.
|
||
- Проверка: поиск `utils|helpers|common|misc` среди пакетов и `go test ./...`.
|
||
|
||
### GO-007 — привести комментарии исходников к русскому стандарту
|
||
|
||
- Найдены английские комментарии в runtime/test-коде, например
|
||
`modules/processor_jwt/context.go` и комментарий в
|
||
`services/gameplay_service/finished_story_test.go`.
|
||
- Перевести только комментарии проекта; директивы `go:embed` и сгенерированный
|
||
код не трогать.
|
||
- Заодно документировать публичные символы, у которых комментарий отсутствует
|
||
и назначение неочевидно.
|
||
- Проверка: ручной проход `go doc`-символов и `go vet ./...`.
|
||
|
||
## Приоритет P1 — привести тесты к зафиксированному стандарту
|
||
|
||
### GO-008 — убрать прямую подготовку и проверку SQL из тестовых сценариев
|
||
|
||
- `repos/tickets_repo/accept_test.go` и `repos/teams_repo/registration_test.go`
|
||
напрямую вызывают `Exec`/`QueryRow` и создают схему в тесте.
|
||
- Подготовку выполнять миграциями инфраструктурного слоя и публичными методами
|
||
repository/service; проверки поведения делать через API тестируемого слоя.
|
||
- Сохранить отдельную БД через `TEST_DATABASE_URL`; не подключаться к production.
|
||
- Проверка: targeted integration tests с отдельной PostgreSQL-базой.
|
||
|
||
### GO-009 — стабилизировать сетевые тесты и общий тестовый gate
|
||
|
||
- `cmd/evening_detective_server/gateway_test.go` и
|
||
`services/mcp_service/http_test.go` требуют loopback TCP; в текущем окружении
|
||
они завершаются `operation not permitted`.
|
||
- Сделать сетевой слой тестов запускаемым в поддерживаемом CI-окружении либо
|
||
отделить такие тесты явным integration/build tag; не ослаблять проверки
|
||
поведения и не маскировать ошибки `t.Skip` без условия окружения.
|
||
- После этого зафиксировать обязательный зелёный gate `go test ./...`.
|
||
|
||
### GO-010 — выровнять структуру unit-тестов
|
||
|
||
- Проверить `*_test.go` на правило: тесты рядом с владельцем поведения,
|
||
бизнес-правила — без PostgreSQL, новая ветка — с минимальным регрессионным
|
||
тестом, кейсы — table-driven.
|
||
- В первую очередь пройти тесты с прямым `panic`, сравнением текста ошибки и
|
||
неописательными сценариями; например `users_service/service_test.go`,
|
||
`users_service/change_password_test.go`, `scenarios_service/service_test.go`.
|
||
- Не переписывать существующие тесты механически, если это не повышает
|
||
проверяемость конкретного правила.
|
||
|
||
## Приоритет P2 — завершить миграцию без изменения поведения
|
||
|
||
### GO-011 — инвентаризировать mapping и модели по владельцам
|
||
|
||
- Часть mapper-ов находится в `app`, часть в service, часть в modules; есть
|
||
mapping через repository-типы и потенциальные дубли моделей.
|
||
- Для каждого mapping выбрать единственного владельца по контракту стандарта:
|
||
transport — `app`, use-case/storage — соответствующий service/repo.
|
||
- Литералы mapping оставить вертикальными; I/O и бизнес-решения из mapper-ов
|
||
убрать.
|
||
- Проверка: review зависимостей и targeted tests mapper-ов.
|
||
|
||
### GO-012 — ввести лёгкий автоматический guard стандарта
|
||
|
||
- После исправления кода добавить только минимальные проверки без новой
|
||
зависимости: `gofmt -l`, `go vet ./...`, проверка запрещённых импортов
|
||
`app → repos`, `services → proto`, `repos → proto`, а также `go test ./...`.
|
||
- Разместить команду в существующем `Makefile`, если она не дублирует уже
|
||
имеющуюся проверку.
|
||
- Не подключать golangci-lint/staticcheck только ради этого списка.
|
||
|
||
## Порядок выполнения сегодня
|
||
|
||
1. `GO-001` → `GO-002` → `GO-003`.
|
||
2. `GO-004` → `GO-005` → `GO-006` → `GO-007`.
|
||
3. `GO-008` → `GO-009` → `GO-010`.
|
||
4. `GO-011` → `GO-012`.
|
||
|
||
Сгенерированные protobuf-файлы не форматировать вручную. Коммиты и push не
|
||
выполнять.
|