diff --git a/GO-001-plan.md b/GO-001-plan.md new file mode 100644 index 0000000..0ccfe9d --- /dev/null +++ b/GO-001-plan.md @@ -0,0 +1,217 @@ +# План GO-001: убрать прямые зависимости handler-ов от repos и modules + +## Цель + +`internal/app` принимает transport-вход, извлекает пользователя и параметры, +вызывает прикладной service и преобразует результат в protobuf/HTTP. Handler не +импортирует repository и не владеет прикладной операцией. + +Не трогаем в этой задаче: + +- `proto/`, API-контракт и protobuf-генерацию; +- wiring в `cmd`, где repositories и modules должны собираться; +- `roles` и `processor_jwt`, если они используются только для извлечения + identity/проверки transport-доступа; +- единое исправление `err.Error()` во всех handler-ах — это GO-003. + +## Текущие точки нарушения + +| Участок | Прямые зависимости app | Что оставить в app | Что должно уйти в service | +| --- | --- | --- | --- | +| `app/team_registration.go` | `teams_repo` | HTTP/JSON, auth token, timeout | смысл ошибок регистрации | +| `app/team_qr.go` | `games_repo`, `team_qr_pdf` | auth, headers, `HttpBody` | ошибки игры и PDF | +| `app/users.go` | `users_repo` | claims, metadata, transport auth | ошибка отсутствующего пользователя | +| `app/scenario_archive.go` | `scenarios_repo`, archive/string modules | auth, ZIP headers, `HttpBody` | лимиты/ошибки сценарного архива | +| `app/files.go` | `file_storage` | auth, headers, `HttpBody` | ошибка отсутствующего файла | +| mapper-ы app | `game_model`, `formatter_utils` | protobuf mapping | модели и преобразования use-case | + +## Маленькие этапы + +### GO-001.0 — зафиксировать baseline и границы + +1. Проверить текущий список импортов: + + ```sh + rg -n 'internal/(repos|modules)' evening_detective_server/internal/app --glob '*.go' + cd evening_detective_server && go test ./internal/app ./internal/services/... + ``` + +2. Выписать в diff только файлы из таблицы выше. +3. После каждого следующего этапа проверять, что импорт repository исчез только + из app, а wiring в `cmd` остался. + +Готово, если до изменений есть зафиксированный список импортов и понятен +targeted test для каждого участка. + +### GO-001.1 — webhook регистрации команды + +Файлы: `internal/app/team_registration.go`, +`internal/services/team_service/registration.go` и связанные тесты. + +1. Найти, какие sentinel-ошибки `TeamService.RegisterTeam` уже возвращает. +2. Сделать ошибку закрытой регистрации частью контракта `team_service`, чтобы + handler не импортировал `teams_repo` только ради `errors.Is`. +3. Оставить `TeamRegistrationHandler` ответственным только за проверку bearer + token, разбор JSON/JSON-RPC, timeout и HTTP-код. +4. Обновить тесты handler-а на публичную ошибку service; не переносить JSON + parsing в service. + +Проверка этапа: + +```sh +cd evening_detective_server +go test ./internal/app ./internal/services/team_service +! rg 'internal/repos' internal/app/team_registration.go +``` + +Ожидаемый результат: регистрация с закрытым окном по-прежнему даёт `409`, +ошибочный payload — `400`, а app больше не знает `teams_repo`. + +### GO-001.2 — выдача QR-кодов команд + +Файлы: `internal/app/team_qr.go`, +`internal/services/team_service/service.go`, `team_qr.go` и тесты. + +1. Перенести наружный смысл ошибок из repository/PDF-модуля на границу + `team_service`: игра не найдена и у команды нет карточек. +2. Не переносить в service формирование `HttpBody` и HTTP-заголовков. +3. В handler оставить проверку claims/ролей, валидацию `id`, mapping ошибок в + gRPC-коды и headers. +4. Проверить существующие тесты `team_qr_test.go` и добавить только недостающий + regression case на обе ошибки. + +Проверка этапа: + +```sh +cd evening_detective_server +go test ./internal/app ./internal/services/team_service +! rg 'internal/repos|team_qr_pdf' internal/app/team_qr.go +``` + +Ожидаемый результат: `DownloadTeamQR` сохраняет PDF и заголовки без прямой +зависимости app от repository/PDF implementation. + +### GO-001.3 — операции пользователей + +Файлы: `internal/app/users.go`, `internal/services/users_service/*` и тесты. + +1. Найти, почему app использует `users_repo.ErrUserNotFound` только в + `ChangeUserPassword`. +2. Передать наружу ошибку через контракт `users_service` (владелец прикладного + смысла), не раскрывая в app storage-пакет. +3. Не менять здесь многочисленные protobuf-поля `Error`: их единое исправление + относится к GO-003. +4. Оставить в app извлечение claims, пароля из metadata, роли и вызов service. +5. Обновить unit-тесты на `NotFound` через service-owned error. + +Проверка этапа: + +```sh +cd evening_detective_server +go test ./internal/app ./internal/services/users_service +! rg 'internal/repos' internal/app/users.go +``` + +Ожидаемый результат: app не импортирует `users_repo`, а код gRPC для +отсутствующего пользователя не меняется. + +### GO-001.4 — архив сценария + +Файлы: `internal/app/scenario_archive.go`, +`internal/services/scenarios_service/*` и тесты. + +1. Отделить transport-часть от прикладной: app оставляет auth, `HttpBody`, + `Content-Disposition` и проверку размера запроса. +2. Ошибку сценария не найдено получать через `scenarios_repo` в app; использовать + service-owned контракт ошибки. +3. Проверить, нужен ли app доступ к `scenario_archive.MaxArchiveSize()` только + как к transport-лимиту. Если да, предоставить лимит через service-owned + константу/функцию; не тащить repository или storage-модель. +4. `string_tools` оставить только если это чистая transport-санитизация имени; + иначе вернуть готовое безопасное имя из service. Не переносить бизнес-логику + в mapper. +5. Не менять формат ZIP и правила авторизации. + +Проверка этапа: + +```sh +cd evening_detective_server +go test ./internal/app ./internal/services/scenarios_service +! rg 'internal/repos' internal/app/scenario_archive.go +``` + +Ожидаемый результат: archive handler знает HTTP-представление, а не repository +и внутреннюю реализацию архива. + +### GO-001.5 — скачивание файлов + +Файлы: `internal/app/files.go`, `internal/services/file_service/*` и тесты. + +1. Перенести `ErrFileNotFound` на публичную границу `file_service` (или сделать + service-owned alias без нового слоя), чтобы handler не импортировал + `file_storage`. +2. Оставить в app авторизацию автора, `nosniff`, `HttpBody` и mapping MIME-типа. +3. Проверить, что service уже возвращает полную модель файла и контекст; + новых DTO и интерфейсов не добавлять. +4. Добавить/обновить тест на `404` скачивания отсутствующего файла. + +Проверка этапа: + +```sh +cd evening_detective_server +go test ./internal/app ./internal/services/file_service +! rg 'internal/modules/file_storage' internal/app/files.go +``` + +### GO-001.6 — app mapper-ы и чистка импортов + +Файлы: `internal/app/game_mapper.go`, `team_mapper.go`, `story_mapper.go`, +`internal/app/*`. + +1. Не переносить protobuf mapping в service: он остаётся в `app`. +2. Проверить, что входные типы mapper-ов являются контрактом service, а не + типами repository/module. Если mapper использует `game_model` или + `formatter_utils` только как промежуточный use-case тип, перенести границу + в service минимальным изменением, сохранив существующий protobuf output. +3. Не переименовывать пакеты `*_utils` в рамках GO-001 — это GO-006. +4. После каждого переноса удалить неиспользуемые импорты и проверить callers. + +Проверка этапа: + +```sh +cd evening_detective_server +find internal/app internal/services -name '*.go' -print0 | xargs -0 gofmt -l +go vet ./... +go test ./internal/app ./internal/services/... +``` + +### GO-001.7 — финальная проверка архитектурного правила + +1. Runtime-файлы `internal/app` не должны импортировать `internal/repos`. +2. Каждый RPC/HTTP handler должен вызывать service, а не repository/module + с бизнес-операцией. +3. `cmd/evening_detective_server/main.go` продолжает собирать repositories, + modules и services — его зависимости не удалять. +4. Проверить все callers изменённых ошибок и конструкторов. +5. Запустить полный набор проверок: + + ```sh + cd evening_detective_server + find internal/app internal/services -name '*.go' -print0 | xargs -0 gofmt -l + go vet ./... + go test ./... + ``` + +6. В `todo.md` отметить GO-001 выполненной только после успешной проверки + за пределами sandbox, где разрешён loopback TCP. + +## Критерий готовности GO-001 + +- Runtime `internal/app` не импортирует `internal/repos`. +- В app не осталось прямого вызова repository или технического module для + выполнения бизнес-операции. +- Существующие transport-коды, headers, JSON payloads и protobuf responses не + изменились намеренно. +- Для каждого перенесённого error mapping есть targeted test. +- `gofmt`, `go vet ./...` и полный `go test ./...` проходят в поддерживаемом + окружении. diff --git a/todo.md b/todo.md new file mode 100644 index 0000000..ee89e3f --- /dev/null +++ b/todo.md @@ -0,0 +1,164 @@ +# 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 не +выполнять.