diff --git a/bincio/edit/server.py b/bincio/edit/server.py index c919bce..21e6f70 100644 --- a/bincio/edit/server.py +++ b/bincio/edit/server.py @@ -43,19 +43,9 @@ def _check_id(activity_id: str) -> str: return activity_id -_ALLOWED_IMAGE_TYPES = {"image/jpeg", "image/png", "image/webp", "image/gif"} -_MAX_IMAGE_BYTES = 10 * 1024 * 1024 # 10 MB - - -def _unique_image_name(directory: Path, filename: str) -> str: - """Return a filename that does not collide with existing files in directory.""" - stem, suffix = Path(filename).stem, Path(filename).suffix - candidate = filename - counter = 1 - while (directory / candidate).exists(): - candidate = f"{stem}_{counter}{suffix}" - counter += 1 - return candidate +from bincio.shared.images import ALLOWED_IMAGE_TYPES as _ALLOWED_IMAGE_TYPES +from bincio.shared.images import MAX_IMAGE_BYTES as _MAX_IMAGE_BYTES +from bincio.shared.images import unique_image_name as _unique_image_name # ── HTML UI ─────────────────────────────────────────────────────────────────── diff --git a/bincio/serve/server.py b/bincio/serve/server.py index 403db96..2d2be7b 100644 --- a/bincio/serve/server.py +++ b/bincio/serve/server.py @@ -332,21 +332,9 @@ def _set_session_cookie(response: Response, token: str) -> None: response.set_cookie(**kwargs) -# ── Image upload constants ──────────────────────────────────────────────────── - -_ALLOWED_IMAGE_TYPES = {"image/jpeg", "image/png", "image/webp", "image/gif"} -_MAX_IMAGE_BYTES = 10 * 1024 * 1024 # 10 MB - - -def _unique_image_name(directory: Path, filename: str) -> str: - """Return a filename that does not collide with existing files in directory.""" - stem, suffix = Path(filename).stem, Path(filename).suffix - candidate = filename - counter = 1 - while (directory / candidate).exists(): - candidate = f"{stem}_{counter}{suffix}" - counter += 1 - return candidate +from bincio.shared.images import ALLOWED_IMAGE_TYPES as _ALLOWED_IMAGE_TYPES +from bincio.shared.images import MAX_IMAGE_BYTES as _MAX_IMAGE_BYTES +from bincio.shared.images import unique_image_name as _unique_image_name # ── Post-write rebuild ──────────────────────────────────────────────────────── diff --git a/bincio/shared/__init__.py b/bincio/shared/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/bincio/shared/images.py b/bincio/shared/images.py new file mode 100644 index 0000000..7466166 --- /dev/null +++ b/bincio/shared/images.py @@ -0,0 +1,23 @@ +"""Shared image upload utilities used by both the edit and serve servers.""" +from __future__ import annotations + +from pathlib import Path + +ALLOWED_IMAGE_TYPES: frozenset[str] = frozenset({ + "image/jpeg", + "image/png", + "image/webp", + "image/gif", +}) +MAX_IMAGE_BYTES: int = 10 * 1024 * 1024 # 10 MB + + +def unique_image_name(directory: Path, filename: str) -> str: + """Return a filename that does not collide with existing files in directory.""" + stem, suffix = Path(filename).stem, Path(filename).suffix + candidate = filename + counter = 1 + while (directory / candidate).exists(): + candidate = f"{stem}_{counter}{suffix}" + counter += 1 + return candidate diff --git a/refactoring.md b/refactoring.md new file mode 100644 index 0000000..7b72d33 --- /dev/null +++ b/refactoring.md @@ -0,0 +1,497 @@ +# Refactoring Plan + +Branch: `refactoring` +Approach: test-first — each step starts with tests that prove correctness of the current behaviour, then the refactor makes those tests pass against the new structure. + +--- + +## Step 1 — Extract shared image utilities + +### Problem +`_ALLOWED_IMAGE_TYPES`, `_MAX_IMAGE_BYTES`, and `_unique_image_name()` are defined identically in two files: + +| File | Lines | +|---|---| +| `bincio/edit/server.py` | 46–58 | +| `bincio/serve/server.py` | 337–357 | + +Any change (e.g. adding `image/avif`) must be made in both places. + +### Target +New module: `bincio/shared/images.py` + +```python +# bincio/shared/images.py +from pathlib import Path + +ALLOWED_IMAGE_TYPES: frozenset[str] = frozenset({ + "image/jpeg", "image/png", "image/webp", "image/gif" +}) +MAX_IMAGE_BYTES: int = 10 * 1024 * 1024 # 10 MB + +def unique_image_name(directory: Path, filename: str) -> str: + """Return a filename that does not collide with existing files in directory.""" + stem, suffix = Path(filename).stem, Path(filename).suffix + candidate = filename + counter = 1 + while (directory / candidate).exists(): + candidate = f"{stem}_{counter}{suffix}" + counter += 1 + return candidate +``` + +### Test plan + +**New file**: `tests/test_shared_images.py` + +Write these tests first (they will fail until the module exists): + +```python +# tests/test_shared_images.py +from pathlib import Path +import pytest +from bincio.shared.images import ALLOWED_IMAGE_TYPES, MAX_IMAGE_BYTES, unique_image_name + +def test_constants(): + assert "image/jpeg" in ALLOWED_IMAGE_TYPES + assert "image/png" in ALLOWED_IMAGE_TYPES + assert "image/webp" in ALLOWED_IMAGE_TYPES + assert "image/gif" in ALLOWED_IMAGE_TYPES + assert "image/avif" not in ALLOWED_IMAGE_TYPES # guard against accidental expansion + assert MAX_IMAGE_BYTES == 10 * 1024 * 1024 + +def test_unique_name_no_collision(tmp_path): + assert unique_image_name(tmp_path, "photo.jpg") == "photo.jpg" + +def test_unique_name_single_collision(tmp_path): + (tmp_path / "photo.jpg").touch() + assert unique_image_name(tmp_path, "photo.jpg") == "photo_1.jpg" + +def test_unique_name_multiple_collisions(tmp_path): + (tmp_path / "photo.jpg").touch() + (tmp_path / "photo_1.jpg").touch() + assert unique_image_name(tmp_path, "photo.jpg") == "photo_2.jpg" + +def test_unique_name_no_suffix(tmp_path): + (tmp_path / "photo").touch() + assert unique_image_name(tmp_path, "photo") == "photo_1" + +def test_unique_name_preserves_case(tmp_path): + assert unique_image_name(tmp_path, "MyPhoto.PNG") == "MyPhoto.PNG" +``` + +### Implementation steps + +1. Create `bincio/shared/__init__.py` (empty). +2. Create `bincio/shared/images.py` with the public constants and function (no leading underscore). +3. In `bincio/edit/server.py`: replace lines 46–58 with: + ```python + from bincio.shared.images import ALLOWED_IMAGE_TYPES as _ALLOWED_IMAGE_TYPES + from bincio.shared.images import MAX_IMAGE_BYTES as _MAX_IMAGE_BYTES + from bincio.shared.images import unique_image_name as _unique_image_name + ``` +4. In `bincio/serve/server.py`: same replacement at lines 337–357. +5. Run `pytest tests/test_shared_images.py` — all pass. +6. Run full test suite to confirm no regressions. + +--- + +## Step 2 — Extract embedded HTML template from `edit/server.py` + +### Problem +`bincio/edit/server.py` contains 285 lines of static HTML/CSS/JS as a Python string literal (`_HTML`, lines 63–347). This: + +- Makes the file 30% larger than it needs to be. +- Prevents syntax highlighting and linting of the HTML/JS. +- Cannot be tested in isolation (template substitution is done inline by the route handler). + +### Target +New file: `bincio/edit/templates/edit.html` + +The template already uses three placeholder tokens: +- `__SITE_URL__` — replaced with `site_url` at request time +- `__SPORT_OPTIONS__` — replaced with generated `