# Code Review: находки и предложения > Статус: **только чтение** — ничего не изменено. Каждый пункт сопровождается предлагаемым исправлением. --- ## 1. Дублирование кода — `_safe_name` / `_safe_chapter_name` **Файлы:** `src/api.py:251`, `src/worker.py:26–32`, `src/cli.py` (аналогичные функции) Одна и та же функция определена в трёх местах. При изменении логики нужно менять сразу в трёх файлах — риск расхождения. **Исправление:** вынести в `src/utils.py`, импортировать везде: ```python # src/utils.py import re from .sources.base import Chapter def safe_name(s: str) -> str: return re.sub(r'[^\w\s\-]', '', s).strip().replace(" ", "_")[:80] def safe_chapter_name(ch: Chapter) -> str: vol = f"v{ch.volume:02d}_" if ch.volume else "" return f"{vol}ch{ch.number:06.1f}" ``` --- ## 2. Прямой доступ к `db.conn` в API-эндпоинтах **Файлы:** `src/api.py` | Место | Строки | |-------|--------| | `_enrich_manga` | 269–278 | | `retry_errors` | 680–688 | | `force_redownload` | 819–823 | | `delete_manga` | 882–885 | | `rename_folder` | 801–803 | `api.py` напрямую исполняет SQL через `db.conn.execute(...)` вместо методов `StateDB`. Это означает, что логика разбросана между двумя слоями, и рефакторинг `StateDB` может незаметно сломать эндпоинты. Для `retry_errors` и `force_redownload` в `StateDB` уже есть готовый метод `reset_failed_chapters` — он просто не используется в API. **Исправление для `retry_errors`:** ```python # api.py — было: now = db.conn.execute("SELECT datetime('now')").fetchone()[0] db.conn.execute("UPDATE chapters SET status='pending' ...", (now, url)) db.conn.commit() # стало: db.reset_failed_chapters(url) ``` Для `delete_manga` и `rename_folder` добавить соответствующие методы в `StateDB`. --- ## 3. `datetime.utcnow()` устарел **Файлы:** `src/api.py:369`, `src/state.py:628` `datetime.utcnow()` объявлен deprecated в Python 3.12. Используется в двух местах. **Исправление:** ```python # src/state.py from datetime import datetime, timezone def _now() -> str: return datetime.now(timezone.utc).replace(tzinfo=None).isoformat() # src/api.py — в login(): expires_at = (datetime.now(timezone.utc) + timedelta(days=30)).replace(tzinfo=None).isoformat() ``` --- ## 4. `check_for_updates` не использует `db_lock` **Файл:** `src/worker.py:343–400` `download_manga` тщательно оборачивает все обращения к БД в `db_lock` (`asyncio.Lock`). `check_for_updates` вызывает `db.update_manga_info`, `db.get_all_chapters`, `db.set_last_checked` напрямую без блокировки. При параллельном запуске нескольких авто-обновлений возможна конкуренция записей в одну и ту же строку SQLite. **Исправление:** добавить `db_lock` в `check_for_updates` по аналогии с `download_manga`, либо убедиться, что авто-обновления всегда запускаются последовательно (сейчас в `_run_auto_updates` они идут через `for manga in candidates` — это последовательно, но ручной `check_now` и авто-обновление могут пересечься). --- ## 5. Хак `pages_done_count = [0]` **Файл:** `src/worker.py:196` ```python pages_done_count = [0] # мутабельный список вместо nonlocal async def on_page(page_idx: int, pages_total: int): pages_done_count[0] += 1 ``` Это классический обходной путь для `nonlocal` в Python 2. В Python 3 достаточно `nonlocal`. **Исправление:** ```python pages_done = 0 async def on_page(page_idx: int, pages_total: int): nonlocal pages_done pages_done += 1 await db_call(db.update_chapter_pages, ch.url, pages_total, pages_done) ``` --- ## 6. Мёртвый код в `StateDB` **Файл:** `src/state.py:405–407` ```python def increment_manga_chapters_done(self, url: str): # Оставлен для совместимости, но не используется в воркере pass ``` Метод ничего не делает и нигде не вызывается. **Исправление:** удалить. --- ## 7. Отложенный импорт `BrowserManager` в `_fetch_preview` **Файл:** `src/api.py:548` ```python async def _fetch_preview(url: str): try: from .browser import BrowserManager # импорт внутри функции ``` `BrowserManager` уже импортируется в `worker.py`. Импорт внутри функции — признак того, что его пытались скрыть из-за каких-то проблем при старте, но сейчас оснований для этого нет. Это замедляет первый вызов и затрудняет поиск зависимостей. **Исправление:** добавить `from .browser import BrowserManager` в топ-уровневые импорты `api.py`. Аналогично — `import shutil` внутри тел `rename_folder` (строка 789) и `delete_manga` (строка 879). Вынести в топ. --- ## 8. O(n²) назначение позиций в очереди **Файл:** `src/api.py:486–491` ```python queue_list = list(download_queue._queue) for i, job in enumerate(queue_list): for r in result: # ← внутренний цикл по всем мангам if r["url"] == job["url"]: r["queue_position"] = i + 1 ``` При 100 мангах в очереди и 100 в БД — 10 000 итераций на каждый запрос `/api/mangas`. **Исправление:** ```python queue_positions = {job["url"]: i + 1 for i, job in enumerate(queue_list)} for r in result: r["queue_position"] = queue_positions.get(r["url"]) ``` --- ## 9. Утечка памяти в `_export_pdf_pillow` **Файл:** `src/exporter.py:131–135` ```python def _export_pdf_pillow(images: list[Path], out: Path): from PIL import Image pil_images = [Image.open(p).convert("RGB") for p in images] if pil_images: pil_images[0].save(out, save_all=True, append_images=pil_images[1:], format="PDF") # pil_images не закрываются — файловые дескрипторы висят до GC ``` **Исправление:** ```python def _export_pdf_pillow(images: list[Path], out: Path): from PIL import Image pil_images = [Image.open(p).convert("RGB") for p in images] try: if pil_images: pil_images[0].save(out, save_all=True, append_images=pil_images[1:], format="PDF") finally: for img in pil_images: img.close() ``` --- ## 10. Потенциальный SQL-инъекционный путь в `mark_done` **Файл:** `src/state.py:453–459` ```python def mark_done(self, chapter_url: str, fmt: str, output_path: str): col = f"output_{fmt}" self.conn.execute(f""" UPDATE chapters SET status='done', {col}=?, updated_at=? WHERE chapter_url=? """, (output_path, _now(), chapter_url)) ``` Сейчас `fmt` всегда приходит из `["cbz", "pdf", "epub"]` в `worker.py`, поэтому эксплуатации нет. Но паттерн хрупкий: если завтра `fmt` придёт от пользователя через API, это станет инъекцией. **Исправление:** ```python _ALLOWED_FMTS = {"cbz", "pdf", "epub"} def mark_done(self, chapter_url: str, fmt: str, output_path: str): if fmt not in _ALLOWED_FMTS: raise ValueError(f"Unknown format: {fmt}") col = f"output_{fmt}" ... ``` --- ## 11. Неиспользуемый метод `BrowserManager.navigate()` **Файл:** `src/browser.py` `BrowserManager` содержит метод `navigate()`, который нигде не вызывается — `readmanga.py` определяет собственный `_navigate`. Это мёртвый код. **Исправление:** удалить `navigate()` из `BrowserManager` или убрать `_navigate` из `readmanga.py` и использовать единый метод. --- ## 12. `cli.py` использует устаревший шим вместо реестра источников **Файл:** `src/cli.py` ```python from .scraper import get_manga_info, get_chapter_images_and_download # shim ``` `scraper.py` — это обёртка-пустышка, делегирующая в `ReadmangaSource`. CLI не использует `SourceRegistry`, не поддерживает `MangaMeta` полноценно. При добавлении нового источника CLI не заработает автоматически. **Исправление:** переписать `cli.py` на использование `registry` и `get_source_for_url`, как это сделано в `worker.py`. --- ## 13. Двойное чтение тела ответа в `saveRenameFolder` **Файл:** `frontend/index.html` ```javascript async function saveRenameFolder() { const r = await fetch('/api/mangas/rename_folder', ...); if (!r.ok) { const err = await r.json(); // ← первое чтение ... } const data = await r.json(); // ← второе чтение (тело уже прочитано!) ``` `Response.body` — это `ReadableStream`, читается один раз. Второй `r.json()` вернёт ошибку или пустой объект. **Исправление:** ```javascript const data = await r.json(); if (!r.ok) { showError(data.detail || 'Ошибка'); return; } ``` --- ## 14. `escHtml()` не защищает от JS-инъекции в `onclick` **Файл:** `frontend/index.html` — различные места типа: ```javascript `