Files
wiki/tasks/enduro-trails/CODE_REVIEW.md
2026-05-14 23:30:01 +03:00

18 KiB
Raw Blame History

Code Review: Enduro Trails

Дата: 2026-05-14
Проект: Enduro Trails — веб-приложение для эндуро-маршрутов
Стек: Python (FastAPI) + MapLibre GL JS + SQLite
Ревьюер: Dev Agent


Общая оценка

Проект представляет собой рабочий прототип с хорошей функциональностью: векторные тайлы, роутинг через OSRM, разведка, красивые маршруты, линейка, метки. UI продуман для мобильных устройств. Однако есть серьёзные проблемы с архитектурой фронтенда (монолитный файл ~2800 строк), безопасностью, и потенциальные баги в работе с ресурсами.


1. Архитектура и структура кода

Critical

# Проблема Файл Описание
1 Монолитный фронтенд static/app.js ~2800 строк в одном файле без модулей. Невозможно тестировать, сложно поддерживать. Нужно разбить на модули (ES modules или bundler).

High

# Проблема Файл Описание
2 Глобальное состояние static/app.js Всё состояние хранится в глобальных переменных (routeMode, rulerPoints, scenicRoutes, etc.). Нет state management — любая функция может мутировать что угодно.
3 Смешение ответственностей в бэкенде app.py Один файл содержит: WKB-парсер, MVT-билдер, route stats, OSRM-клиент, scenic logic, tile cache. Нужно разбить на модули (tiles.py, routing.py, scenic.py, db.py).
4 Нет разделения API и статики app.py FastAPI раздаёт и API, и статику. В продакшене статику должен раздавать nginx/CDN.

Medium

# Проблема Файл Описание
5 package.json содержит только ssh2 package.json Зависимость ssh2 — это деплой-утилита, не имеет отношения к приложению. Нет build-системы для фронтенда.
6 Мусорные файлы в репозитории корень check_terrain.js, debug_terrain2.js, deploy_dark_style.js, fix_nginx_perms.js и т.д. — одноразовые скрипты, засоряют проект.
7 __pycache__ в репозитории корень Скомпилированные .pyc файлы не должны быть в VCS. Нужен .gitignore.

2. Качество кода

High

# Проблема Файл Строки Описание
8 Дублирование кода поиска app.js множество _doOnboardSearch, doWaypointSearch, _doMiniOnboardSearch, doSearch — 4 почти идентичные функции для Nominatim-поиска. Нужна одна универсальная.
9 Дублирование haversine app.js + app.py haversineKm, haversineM (JS) и haversine_m (Python) — одна и та же формула в 3 вариантах.
10 HTML в строках JS app.js множество Огромные HTML-шаблоны через template literals. Нет санитизации, нет компонентного подхода.

Medium

# Проблема Файл Описание
11 Magic numbers app.py 0.0015, 0.005, 500.0, 5000, 10000 — snap radius, delta, sampling step без именованных констант.
12 Inconsistent naming app.js Смесь camelCase (routeMode) и snake_case в комментариях. Функции то с underscore (_doOnboardSearch), то без.
13 Неиспользуемые импорты app.py from typing import List — в Python 3.11 можно использовать list[] напрямую. Мелочь, но показывает отсутствие линтера.
14 Отсутствие JSDoc/TypeScript app.js Ни одной аннотации типов. При 2800 строках это критично для поддержки.

Low

# Проблема Файл Описание
15 Комментарии на смеси языков оба Русские комментарии в коде — ок для команды, но docstrings API лучше на английском для совместимости с инструментами.

3. Потенциальные баги

Critical

# Проблема Файл Описание
16 SQLite connection не thread-safe app.py get_db() создаёт новое соединение при каждом вызове, но uvicorn с workers=4 запускает multiprocess. SQLite в WAL mode поддерживает concurrent reads, но без WAL mode возможны database is locked ошибки под нагрузкой.
17 Connection leak в calc_route_stats app.py В post_route и _build_segmented_route соединение открывается, но если исключение произойдёт до conn.close() — утечка. Нужен try/finally или context manager.

High

# Проблема Файл Описание
18 Tile cache без thread safety app.py _tile_cache — обычный dict, мутируется из async handlers. При workers=4 каждый процесс имеет свой кэш (ок), но внутри одного процесса asyncio event loop однопоточный — тоже ок. Однако FIFO-вытеснение через next(iter(...)) не гарантирует LRU-поведение.
19 initMiniRouteInteraction клонирует DOM app.js mini.cloneNode(true) + replaceChild — каждый вызов пересоздаёт элемент. Это теряет все event listeners на дочерних элементах и может вызвать утечки памяти при частом переключении маршрутов.
20 Race condition в buildRoute app.js debounceBuildRoute с 300ms debounce, но если пользователь быстро добавляет точки, предыдущий fetch не отменяется (нет AbortController). Результат старого запроса может перезаписать новый.

Medium

# Проблема Файл Описание
21 removeRulerPoint не обновляет индексы корректно app.js btn.onclick замыкает idx через updateRulerLabels, но после splice индексы сдвигаются. Повторное удаление может удалить не ту точку.
22 Бесконечная рекурсия в _osrm_segment_alternatives app.py depth < 2 ограничивает, но при itertools.product(alts_a[:3], alts_b[:3]) на глубине 2 это до 9 комбинаций × 9 = 81 маршрут. Комбинаторный взрыв при 3+ waypoints.
23 selectMarkerType использует prompt() app.js Блокирующий prompt() — плохой UX на мобильных. Некоторые браузеры ограничивают его.

4. Производительность

High

# Проблема Файл Описание
24 O(n) snap в getRouteSegmentDistances app.js Для каждого waypoint перебирает ВСЕ точки геометрии (for (let j = 0; j < n; j++)). При маршруте 500км с 10000 точек и 10 waypoints = 100000 итераций на каждый render.
25 Синхронный SQLite в async handler app.py get_db() + cur.execute() блокируют event loop. FastAPI запускает sync-функции в threadpool только если endpoint не async. Здесь endpoints async — значит блокировка event loop при каждом запросе тайла.
26 Нет HTTP caching для тайлов app.py Тайлы отдаются без Cache-Control header (кроме terrain). Браузер будет запрашивать их повторно при каждом pan/zoom.
27 reverseGeocode на каждый render app.js renderWaypointsList() вызывает reverseGeocode для каждого waypoint при каждом обновлении списка. Хотя есть кэш, это лишние async операции и потенциальные race conditions с DOM.

Medium

# Проблема Файл Описание
28 Tile cache 512 entries — мало app.py При активном использовании карты 512 тайлов заполняются быстро. FIFO-вытеснение неоптимально — лучше LRU.
29 simplify_coords создаёт Shapely объекты app.py Для каждого трека в тайле создаётся LineString + simplify(). На тайле с 15000 треков это дорого. Стоит кэшировать или упрощать на этапе импорта.
30 Nominatim rate limiting app.js Nominatim имеет лимит 1 req/sec. При быстром вводе в поиск (debounce 400ms) можно получить 429. Нет обработки rate limit ответов.

Low

# Проблема Файл Описание
31 Загрузка SunCalc для авто-темы index.html Внешняя библиотека загружается всегда, даже если пользователь выбрал фиксированную тему.

5. Безопасность

Critical

# Проблема Файл Описание
32 XSS через Nominatim данные app.js Результаты поиска вставляются через innerHTML без санитизации: <div class="wl-search-result-name">${name}</div>. Если Nominatim вернёт <script> в display_name — XSS.
33 XSS через marker name app.js prompt() результат используется в innerHTML popup через escapeXml() — это защищает, но drawNamedMarker использует markerData.name в popup HTML. escapeXml вызывается — ОК, но в generateGPX тоже — нужно проверить все пути.

High

# Проблема Файл Описание
34 CORS allow all origins app.py allow_origins=["*"] — любой сайт может делать запросы к API. Для прототипа допустимо, для продакшена — нет.
35 Path traversal в terrain endpoint app.py terrain_tile(layer, z, x, y) — layer проверяется на whitelist, но z/x/y — нет валидации на отрицательные значения или ../. os.path.join с отрицательным числом безопасен, но стоит добавить явную проверку.
36 Нет rate limiting app.py API открыт без ограничений. Один клиент может забить сервер запросами к OSRM или тяжёлыми scenic-маршрутами.
37 OSRM URL из env без валидации app.py OSRM_URL используется напрямую в httpx запросах. Если кто-то установит его в http://internal-service/, возможен SSRF. В контейнерной среде это менее критично.

Medium

# Проблема Файл Описание
38 localStorage без проверки app.js loadMarkers() парсит JSON из localStorage. Если данные повреждены — try/catch есть, но нет валидации структуры. Malformed data может вызвать runtime errors позже.
39 Внешние CDN без SRI index.html maplibre-gl@4.7.0 и suncalc@1.9.0 загружаются с unpkg без integrity атрибута. Supply chain attack vector.

6. UX/UI проблемы

High

# Проблема Файл Описание
40 Нет offline-режима Карта полностью зависит от сети. При потере связи (типично для эндуро) — белый экран. Нужен Service Worker + кэш тайлов.
41 prompt() для имени метки app.js Нативный prompt() — плохой UX на мобильных, особенно iOS. Нужен кастомный inline-input.
42 Нет undo/redo app.js Случайное удаление waypoint или метки необратимо.

Medium

# Проблема Файл Описание
43 Нет индикации ошибок сети app.js При сетевых ошибках пользователь видит только " Error" без деталей и retry-кнопки.
44 Toolbar переполнение на маленьких экранах app.css 6 кнопок в toolbar при ширине 320px — каждая ~53px. С учётом padding может быть тесно.
45 Нет accessibility index.html Кнопки без aria-label, нет keyboard navigation, нет skip-links, контраст не проверен.
46 Swipe-to-close конфликтует со scroll app.js Sheet swipe инициируется из top 50px, но sheet-handle маленький (36×4px). На практике пользователь может случайно закрыть sheet при скролле.

Low

# Проблема Файл Описание
47 Hardcoded countrycodes=ru app.js Поиск ограничен Россией. Если проект расширится — нужно параметризовать.
48 Нет favicon index.html Браузер запрашивает /favicon.ico — 404.

7. Рекомендации по улучшению

Приоритет 1 (Critical path)

  1. Разбить app.js на модули — минимум: map.js, routing.js, ruler.js, markers.js, search.js, ui.js, state.js. Использовать ES modules или Vite.
  2. Исправить XSS — использовать textContent вместо innerHTML для пользовательских данных, или DOMPurify.
  3. Исправить блокировку event loop — использовать run_in_executor для SQLite операций или сделать endpoints синхронными (FastAPI автоматически запустит в threadpool).
  4. Добавить AbortController — отменять предыдущие fetch-запросы при новых.

Приоритет 2 (High value)

  1. Добавить Cache-Control для тайловCache-Control: public, max-age=3600 для MVT тайлов.
  2. Connection pooling — использовать один connection per request с context manager.
  3. Добавить SRI для CDN-скриптов.
  4. Service Worker — кэширование тайлов и статики для offline.
  5. Заменить prompt() на inline-input в marker dialog.
  6. Добавить .gitignore — исключить __pycache__, node_modules, *.pyc, .env.

Приоритет 3 (Nice to have)

  1. TypeScript — постепенная миграция фронтенда.
  2. Тесты — pytest для API endpoints, unit tests для calc_route_stats, wkb_to_coords.
  3. Linting — ruff для Python, eslint для JS.
  4. LRU cache — заменить FIFO tile cache на functools.lru_cache или cachetools.LRUCache.
  5. Error boundaries — graceful degradation при недоступности OSRM.
  6. Accessibility audit — aria-labels, keyboard nav, focus management.

Резюме

Категория Critical High Medium Low
Архитектура 1 3 3
Качество кода 3 4 1
Баги 2 3 3
Производительность 4 3 1
Безопасность 2 4 2
UX/UI 3 4 2
Итого 5 20 19 4

Вердикт: Прототип функционально богатый и визуально качественный. Для перехода в продакшен необходимо: (1) модуляризация фронтенда, (2) исправление XSS, (3) async-safe работа с БД, (4) добавление тестов и линтеров. Текущее состояние — хороший MVP для демонстрации, но не для публичного деплоя.