feat: extend admin operations and media review
This commit is contained in:
@@ -0,0 +1,389 @@
|
||||
# Полный аудит проекта RF4 Spotter — 10 сентября 2026
|
||||
|
||||
База: `13e04e6`. Проверены: архитектура, backend, frontend, security, performance, testing, deployment, documentation.
|
||||
|
||||
---
|
||||
|
||||
## 1. Архитектура и высокоуровневый обзор
|
||||
|
||||
### Стек
|
||||
| Слой | Технология | Статус |
|
||||
|------|-----------|--------|
|
||||
| **Proxy** | Caddy 2.10 | Production-ready, TLS завершение, Basic Auth на admin |
|
||||
| **Frontend** | Astro 7 SSR (Node) | Сборка 0 errors, адаптивный дизайн |
|
||||
| **Backend** | FastAPI + SQLAlchemy 2 + PostgreSQL 17 | 130 тестов, 1 skipped |
|
||||
| **Хранилище** | PostgreSQL 17 + MinIO/S3 | Volumes, backup/restore |
|
||||
| **CI** | Gitea Actions | Python, Astro, Compose E2E, pip-audit |
|
||||
| **Scheduler** | Official import + Community scheduler | Опциональный профиль |
|
||||
|
||||
### Оценка архитектуры: ✅ Хорошо (8/10)
|
||||
- Чёткое разделение: Caddy → Astro SSR → FastAPI → Postgres/MinIO
|
||||
- Нет точки отказа в виде единого контейнера
|
||||
- Profiles в docker-compose для scheduler/importer
|
||||
- Production compose отдельно от dev
|
||||
|
||||
---
|
||||
|
||||
## 2. Backend (FastAPI) — детальный разбор
|
||||
|
||||
### 2.1 `main.py` (~1100 строк) — 🔴 КРИТИЧЕСКАЯ ПРОБЛЕМА
|
||||
|
||||
**Проблемы:**
|
||||
- Все endpoints в одном файле — нарушение SRP
|
||||
- `create_catch_report` — 50+ строк бизнес-логики inline
|
||||
- `_check_rate_limit` и `_is_trusted_proxy` — утилиты должны быть в отдельном модуле
|
||||
- `_admin`, `_spot_or_404`, `_report_source` — общие функции смешаны с endpoints
|
||||
- Нет APIRouter — всё на `app.get/post`
|
||||
|
||||
**Рекомендация:** Разделить на `routes/` с `APIRouter`:
|
||||
```
|
||||
apps/api/app/routes/
|
||||
├── activity.py # /api/v1/activity, /spots/*
|
||||
├── submissions.py # catch-reports, screenshots
|
||||
├── admin.py # /admin/*
|
||||
├── community.py # observations, source-status
|
||||
├── records.py # /records, /imports
|
||||
└── catalog.py # fishes, waterbodies, baits
|
||||
```
|
||||
|
||||
### 2.2 `activity.py` — ⚠️ Есть риски при scale
|
||||
|
||||
**Плюсы:**
|
||||
- Детерминированная формула, покрыта тестами
|
||||
- Cap confidence для 1-2 игроков (A07)
|
||||
- Exponential decay freshness (18h half-life)
|
||||
|
||||
**Риски:**
|
||||
- Нет индекса на `(spot_id, fish_id)` — `groups` агрегирует в памяти
|
||||
- При 10k+ approved reports за 72h — полный load + sort в памяти
|
||||
- `total = len(rows)` — нет `COUNT` запроса, полная выборка для pagination
|
||||
|
||||
**Рекомендация:**
|
||||
1. Добавить composite index: `ix_catch_report_spot_fish_approved`
|
||||
2. Для activity endpoint — materialized view или pre-aggregation table
|
||||
3. Для total — отдельный `COUNT` query вместо `len(rows)`
|
||||
|
||||
### 2.3 `models.py` — ✅ Хорошо структурирован
|
||||
|
||||
**Плюсы:**
|
||||
- Чёткие enum для SourceType, ModerationStatus, ImportStatus
|
||||
- Unique constraints на slug, external_id
|
||||
- JSON поля для raw_payload, changes, provenance
|
||||
- Foreign keys с relationships
|
||||
|
||||
**Проблемы:**
|
||||
- `source_external_id` unique=True — конфликт при повторном импорте разных sources с одинаковым ID
|
||||
- `SubmissionAttempt` — нет cleanup cron, растёт бесконечно (хотя есть cleanup в `_check_rate_limit`)
|
||||
- `ImportRecordEvent` — нет индекса на `(catch_report_id, created_at)` для history query
|
||||
|
||||
### 2.4 `importer.py` — ✅ Хорошо, но есть edge cases
|
||||
|
||||
**Плюсы:**
|
||||
- Advisory lock для cross-process mutex
|
||||
- ETag/Last-Modified conditional requests
|
||||
- Idempotent по external_id SHA-256
|
||||
- A12: meaningful changes/provenance в ImportRecordEvent
|
||||
|
||||
**Риски:**
|
||||
- `html: str | None = None` — тестовый параметр в production signature
|
||||
- `_unique_slug` — N+1 query loop при коллизиях
|
||||
- Нет retry policy для HTTP 5xx с exponential backoff
|
||||
|
||||
### 2.5 `retention.py` — ✅ Хорошо
|
||||
|
||||
**Плюсы:**
|
||||
- Dry-run mode по умолчанию
|
||||
- Configurable policy через RetentionPolicy
|
||||
- Handles screenshots, payloads, moderation events
|
||||
- Автоматический rejection pending reports по TTL
|
||||
|
||||
**Проблемы:**
|
||||
- `delete_object` callback — должен быть injected, не passed per-call
|
||||
- Нет bulk delete — loop по reports
|
||||
|
||||
### 2.6 `scheduler.py` vs `community_scheduler.py` — ⚠️ Дублирование
|
||||
|
||||
Оба используют pattern:
|
||||
```python
|
||||
while True:
|
||||
try:
|
||||
run_due_import()
|
||||
except Exception:
|
||||
logger.exception(...)
|
||||
time.sleep(interval)
|
||||
```
|
||||
|
||||
**Рекомендация:** Общий base class или use APScheduler/celery-beat
|
||||
|
||||
---
|
||||
|
||||
## 3. Frontend (Astro) — детальный разбор
|
||||
|
||||
### 3.1 `index.astro` — ⚠️ Сложная логика в template
|
||||
|
||||
**Проблемы:**
|
||||
- 50+ строк бизнес-логики в `.astro` файле
|
||||
- Нет error boundary — catch all делает 503
|
||||
- `offset + items.length < totalItems` — работает, но fragile при partial last page
|
||||
- Нет optimistic UI / loading state
|
||||
|
||||
**Рекомендация:**
|
||||
1. Вынести fetch logic в `src/lib/fetchers.ts`
|
||||
2. Добавить `isLoading` state и skeleton UI
|
||||
3. Error boundary per-section (filters, activity, signals)
|
||||
|
||||
### 3.2 `Layout.astro` — ✅ Хорошо
|
||||
|
||||
**Плюсы:**
|
||||
- A08: errorPage prop для skip structuredData
|
||||
- Canonical, OG, Twitter Card, JSON-LD
|
||||
- robots/noindex для admin/error pages
|
||||
- Skip link для accessibility
|
||||
|
||||
**Минусы:**
|
||||
- 12 CSS imports — можно aggregate в `global.css`
|
||||
- `replaceAll("<", "\\u003c")` — hack для JSON escaping
|
||||
|
||||
### 3.3 `global.css` (~1200 строк) — ⚠️ Все стили вместе
|
||||
|
||||
**Проблемы:**
|
||||
- Все компоненты в одном файле — сложно navigate
|
||||
- Нет CSS modules или scoped styles
|
||||
|
||||
**Рекомендация:**
|
||||
- Разделить на `components/`, `pages/`, `utilities/`
|
||||
- Astro scoped styles для компонентов
|
||||
|
||||
---
|
||||
|
||||
## 4. Security — ✅ В целом хорошо, есть улучшения
|
||||
|
||||
### ✅ Реализовано
|
||||
| Мера | Статус |
|
||||
|------|--------|
|
||||
| Rate limiting (PostgreSQL advisory lock) | ✅ |
|
||||
| Idempotency-Key header | ✅ (A05) |
|
||||
| HMAC client_hash вместо IP | ✅ |
|
||||
| Screenshot validation (MIME, size, EXIF removal) | ✅ |
|
||||
| Admin Bearer token + Caddy Basic Auth | ✅ |
|
||||
| Security headers (HSTS, X-Frame-Deny, etc.) | ✅ |
|
||||
| Production settings validation | ✅ |
|
||||
| pip-audit в CI | ✅ |
|
||||
|
||||
### ⚠️ Улучшения
|
||||
| Проблема | Приоритет |
|
||||
|----------|----------|
|
||||
| **CORS origins** — dev defaults `localhost:4321`, production требует HTTPS | Medium |
|
||||
| **ADMIN_TOKEN** — default `change-me-in-production` в compose.yaml | High (docs) |
|
||||
| **X-Content-Type-Options** — только на admin/catch-reports, не на всех | Low |
|
||||
| **Content-Security-Policy** — отсутствует | Medium |
|
||||
| **Rate limit** — 5 requests per 10 min, нет per-endpoint limits | Low |
|
||||
| **Screenshot upload** — нет CSRF protection (POST без token) | Medium |
|
||||
|
||||
---
|
||||
|
||||
## 5. Performance — ⚠️ Есть узкие места
|
||||
|
||||
### 5.1 Database
|
||||
|
||||
**Проблемы:**
|
||||
- `activity` endpoint: full table scan + in-memory sort
|
||||
- `spot_detail`: `count_since` — Python loop по reports
|
||||
- `public_spot_pages`: 3-way JOIN + DISTINCT + offset pagination
|
||||
|
||||
**Рекомендации — SQL индексы:**
|
||||
```sql
|
||||
-- Добавить индексы
|
||||
CREATE INDEX ix_catch_report_spot_fish_approved
|
||||
ON catch_report(spot_id, fish_id, moderation_status, reported_at);
|
||||
|
||||
CREATE INDEX ix_catch_report_waterbody_approved_reported
|
||||
ON catch_report(waterbody_id, moderation_status, reported_at);
|
||||
|
||||
CREATE INDEX ix_catch_report_official_records
|
||||
ON catch_report(source_type, caught_at, weight_g)
|
||||
WHERE source_type = 'official_record';
|
||||
```
|
||||
|
||||
### 5.2 Caching
|
||||
|
||||
**Текущее:**
|
||||
- In-memory cache: 20s TTL, 128 keys, process-local
|
||||
- `public_cache.invalidate()` на moderation/publish
|
||||
|
||||
**Проблемы:**
|
||||
- Multi-process: каждый API worker имеет свой cache
|
||||
- Нет cache warming на restart
|
||||
- Community observations не кэшируются
|
||||
|
||||
**Рекомендация:** Redis или shared memory (SHM) для multi-process
|
||||
|
||||
### 5.3 Frontend
|
||||
|
||||
**Плюсы:**
|
||||
- Astro SSR — no client JS for initial render
|
||||
- `fetchpriority="high"` для hero image
|
||||
- Reduced motion support
|
||||
|
||||
**Улучшения:**
|
||||
- Нет lazy loading для нижеfold images
|
||||
- Нет prefetch для `/spots/[id]`
|
||||
- SignalFeed — нет intersection observer для "load more"
|
||||
|
||||
---
|
||||
|
||||
## 6. Testing — ✅ 130 passed, 1 skipped
|
||||
|
||||
### ✅ Хорошее покрытие
|
||||
- API endpoints (test_api.py)
|
||||
- Import idempotency (test_importer.py)
|
||||
- Community CLI state/validation (test_community_cli.py)
|
||||
- Readiness aggregation (test_readiness.py)
|
||||
- Rate limit with Docker chain (test_rate_limit.py)
|
||||
|
||||
### ⚠️ Пропущенные сценарии
|
||||
| Тест | Приоритет |
|
||||
|------|----------|
|
||||
| **Retention cleanup** — нет интеграционного теста | High |
|
||||
| **Screenshot validation** — edge cases (PNG vs JPEG magic bytes) | Medium |
|
||||
| **Community scheduler retry backoff** — 30min → 24h | Medium |
|
||||
| **Activity calculation** — edge cases (0 players, all same player) | Low |
|
||||
| **Bootstrap E2E** — test-production-bootstrap.sh не в CI | Medium |
|
||||
| **Astro accessibility** — audit:axe есть, но нет automated checks | Low |
|
||||
|
||||
---
|
||||
|
||||
## 7. Deployment & Operations
|
||||
|
||||
### ✅ Хорошее
|
||||
- Compose production с healthchecks
|
||||
- Backup/restore scripts
|
||||
- Monitoring docs (disk, TLS, readiness)
|
||||
- Data retention policy
|
||||
- Preflight checks before deploy
|
||||
|
||||
### ⚠️ Улучшения
|
||||
| Проблема | Решение |
|
||||
|----------|---------|
|
||||
| **Нет blue-green/canary** | Добавить rolling update в deploy script |
|
||||
| **Database migration rollback** | Нет test-rollback в bootstrap |
|
||||
| **Log rotation** — Docker json-file, 10MB × 5 | OK, но нет centralized logging |
|
||||
| **No distributed tracing** | Добавить OpenTelemetry для request_id propagation |
|
||||
| **No metrics export** | Нет Prometheus metrics (request count, latency, error rate) |
|
||||
|
||||
---
|
||||
|
||||
## 8. Documentation
|
||||
|
||||
### ✅ Хорошее
|
||||
- README.md — comprehensive (150+ строк)
|
||||
- docs/ — recovery plans, audit reports, roadmap
|
||||
- deploy/README.md — production steps
|
||||
- data-policy.md, retention, monitoring
|
||||
|
||||
### ⚠️ Улучшения
|
||||
| Проблема | Решение |
|
||||
|----------|---------|
|
||||
| **API documentation** — FastAPI auto-docs, но нет OpenAPI spec file | Добавить `openapi.json` в repo |
|
||||
| **Data model ER diagram** | Добавить визуальную схему |
|
||||
| **Architecture decision records (ADR)** | Для key decisions (Astro vs Next, in-memory cache, etc.) |
|
||||
| **Runbook for incidents** | Что делать при Postgres full, MinIO down, etc. |
|
||||
|
||||
---
|
||||
|
||||
## 9. Код-стиль и maintainability
|
||||
|
||||
### ✅ Хорошее
|
||||
- Type hints everywhere
|
||||
- `from __future__ import annotations`
|
||||
- Pydantic v2 models
|
||||
- Alembic migrations
|
||||
|
||||
### ⚠️ Улучшения
|
||||
| Проблема | Пример |
|
||||
|----------|--------|
|
||||
| **Long lines** | `create_catch_report` — 70+ char lines |
|
||||
| **Magic numbers** | `55 * min(1, weighted / 12)` — вынести константы |
|
||||
| **Inline SQL** | `text("SELECT pg_try_advisory_lock(:key)")` — вынести в constants |
|
||||
| **Duplicate URL patterns** | `"/api/v1/admin/"` repeated in middleware and endpoints |
|
||||
|
||||
---
|
||||
|
||||
## 10. Приоритизированный список улучшений
|
||||
|
||||
### 🔴 High Priority (блокирующие/рисковые)
|
||||
|
||||
1. **Разделить main.py на routers** — SRP, testability, reviewability
|
||||
2. **Добавить database indexes** — `spot_id + fish_id + moderation_status`, `source_type + official_records`
|
||||
3. **Retention integration test** — verify cleanup works end-to-end
|
||||
4. **Screenshot CSRF protection** — form submission без CSRF token
|
||||
5. **Bootstrap E2E в CI** — `test-production-bootstrap.sh` должен run на push
|
||||
|
||||
### 🟡 Medium Priority (улучшения)
|
||||
|
||||
6. **Shared cache (Redis)** — вместо in-memory для multi-process
|
||||
7. **Content-Security-Policy header** — добавить base-uri, script-src
|
||||
8. **Error boundaries в Astro** — per-section error handling
|
||||
9. **Materialized view для activity** — pre-aggregation вместо in-memory sort
|
||||
10. **OpenAPI spec export** — `openapi.json` в repo + swagger UI
|
||||
11. **Metrics export (Prometheus)** — request latency, error rate, queue depth
|
||||
12. **Screenshot validation test** — magic bytes, MIME mismatch
|
||||
|
||||
### 🟢 Low Priority (косметика/технический долг)
|
||||
|
||||
13. **CSS modularization** — split global.css into components
|
||||
14. **Extract constants** — 55, 25, 20, 12, 6, 3 → named constants
|
||||
15. **Scheduler base class** — DRY для official + community scheduler
|
||||
16. **Prefetch links** — `/spots/[id]` prefetch на hover
|
||||
17. **ADR documentation** — record architecture decisions
|
||||
18. **Incident runbook** — playbooks для common failures
|
||||
|
||||
---
|
||||
|
||||
## Итоговая оценка
|
||||
|
||||
| Категория | Оценка | Комментарий |
|
||||
|-----------|--------|-------------|
|
||||
| **Архитектура** | 8/10 | Чистое разделение, но main.py — god object |
|
||||
| **Безопасность** | 7.5/10 | Хорошие основы, нужно CSP + CSRF |
|
||||
| **Производительность** | 6.5/10 | Indexes + caching critical для scale |
|
||||
| **Тестирование** | 8/10 | 130 tests, но пропущены retention + bootstrap |
|
||||
| **Deploy/Ops** | 7.5/10 | Good compose, no metrics/tracing |
|
||||
| **Документация** | 8.5/10 | Comprehensive README, missing runbook |
|
||||
| **Code Quality** | 7.5/10 | Type hints, но long files + magic numbers |
|
||||
|
||||
**Общий балл: 7.6/10** — крепкий проект с хорошими основами, требует refactor main.py и database indexes для production scale.
|
||||
|
||||
---
|
||||
|
||||
## История ревизий
|
||||
|
||||
| Дата | Автор | Изменения |
|
||||
|------|-------|----------|
|
||||
| 2026-09-10 | AI | Полный аудит: архитектура, backend, frontend, security, performance, testing, deploy, docs |
|
||||
| 2026-09-08 | (предыдущий) | Базовый аудит UI/UX, T01/T02 fixes |
|
||||
|
||||
---
|
||||
|
||||
## Что делать дальше (рекомендации)
|
||||
|
||||
### Немедленно (до продакшена)
|
||||
1. Разделить main.py на routers (минимум 3 файла)
|
||||
2. Создать и применить SQL индексы
|
||||
3. Добавить retention integration test
|
||||
4. Добавить CSRF token для screenshot upload
|
||||
5. Добавить `test-production-bootstrap.sh` в CI workflow
|
||||
|
||||
### В течение спринта
|
||||
6. Внедрить Redis для shared caching
|
||||
7. Добавить CSP headers
|
||||
8. Вынести fetch logic из index.astro в fetchers.ts
|
||||
9. Добавить OpenAPI spec export
|
||||
10. Написать incident runbook
|
||||
|
||||
### По возможности
|
||||
11. Materialized view для activity
|
||||
12. Prometheus metrics
|
||||
13. CSS modularization
|
||||
14. ADR documentation
|
||||
15. Scheduler base class
|
||||
Reference in New Issue
Block a user