diff --git a/.env.example b/.env.example index 729f517..bde007a 100644 --- a/.env.example +++ b/.env.example @@ -17,6 +17,7 @@ FILTER_TIMEOUT=120 FILTER_TIMEOUT_LONG=300 MAX_FILTER_ATTEMPTS=8 MAX_EDGE_PX=2000 +PREPROCESS_TIMEOUT=120 # birefnet-general OOMs on the ~8GB box; u2net + alpha ≈ `rembg i -a` REMBG_MODEL=u2net REMBG_ALPHA=1 diff --git a/.gitea/workflows/ci.yml b/.gitea/workflows/ci.yml new file mode 100644 index 0000000..fcdcdc9 --- /dev/null +++ b/.gitea/workflows/ci.yml @@ -0,0 +1,34 @@ +name: CI + +on: + push: + branches: [main, master] + pull_request: + branches: [main, master] + +jobs: + lint-and-test: + runs-on: ubuntu-latest + + steps: + - name: Checkout code + uses: actions/checkout@v4 + + - name: Set up Python + uses: actions/setup-python@v5 + with: + python-version: "3.11" + + - name: Install dependencies + run: | + python -m pip install --upgrade pip + pip install -r requirements-dev.txt + + - name: Ruff lint + run: ruff check app/ tests/ + + - name: Ruff format check + run: ruff format --check app/ tests/ + + - name: Pytest + run: pytest -q diff --git a/.gitignore b/.gitignore index b85b0cb..f5aae13 100644 --- a/.gitignore +++ b/.gitignore @@ -16,9 +16,15 @@ __pycache__/ *.pyc *.pyo .venv/ +.venv-*/ +*.egg-info/ +dist/ +build/ .pytest_cache/ .mypy_cache/ .ruff_cache/ +htmlcov/ +.coverage # OS / editor noise .DS_Store diff --git a/.python-version b/.python-version new file mode 100644 index 0000000..2c07333 --- /dev/null +++ b/.python-version @@ -0,0 +1 @@ +3.11 diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md new file mode 100644 index 0000000..d2e24d4 --- /dev/null +++ b/ARCHITECTURE.md @@ -0,0 +1,79 @@ +# Architecture — live.f12.rocks + +## Overview + +``` +Phone/Syncthing Docker host +─────────────── ─────────── +incoming/ ──watch──► worker ──► variants/ + intermediates/ + │ │ + └── rembg + gmic pipeline + │ +meta/{stem}.json ◄──────────────────────┘ + │ + ▼ + web (FastAPI) ──► browser UI + remix POST +``` + +Two containers share one image (`Dockerfile`) and one bind-mounted data +directory (`DATA_HOST_DIR` → `/data`). Syncthing runs on the host, not in +compose. + +## Python modules + +| Module | Role | +|--------|------| +| `app/worker.py` | Poll `incoming/`, copy to `variants/`, run `process_job()` | +| `app/pipeline.py` | gmic/rembg orchestration, meta/manifest I/O, variant compose | +| `app/remix.py` | User-selected remix variant (one-shot compose + manifest append) | +| `app/main.py` | FastAPI routes, Jinja templates, static files | +| `app/config.py` | Environment → constants (paths, timeouts, tuning) | +| `app/rembg_cli.py` | Subprocess entry for rembg (avoids heavy `rembg[cli]` extra) | +| `app/io_utils.py` | Atomic JSON read/write | +| `app/env_utils.py` | Shared bool env parsing | +| `app/logging_config.py` | Entry-point logging setup | + +## Data model + +One **job** = one sanitized stem (from incoming filename). Flat files: + +- `variants/{stem}_original.jpg` — preprocessed copy +- `variants/{stem}_rembg.png` — background removal +- `variants/{stem}_vN.png` — final composed images +- `intermediates/{stem}_vN_*.png` — step images (kept for debugging) +- `meta/{stem}.json` — status + manifest (variants list, filter metadata) +- `processed.json` — worker bookkeeping (incoming path → handled) + +## Pipeline origin + +The gmic filter/blend chain is ported from an external reference script +`make_random.py` (not in this repo). `config.POST_FILTERS` and asset lists +under `assets/` mirror that script's behaviour. + +## Background removal choice + +`compare-bg/` is a **standalone benchmark** (rembg models vs withoutbg). +It is not part of the runtime stack. Production uses `u2net` + alpha +matting via `app/rembg_cli.py` (see `config.REMBG_MODEL`). + +## Known limitations (intentional) + +- **No web auth** — anyone with the link sees all jobs (event tool). +- **Sequential worker** — one photo at a time; burst uploads queue. +- **Meta JSON** — worker and web both read-modify-write `meta/*.json` + without file locking; concurrent remix during active processing can race. +- **Incoming is append-only** — worker never deletes from `incoming/`. + +## Dev / test + +```bash +python3.11 -m venv .venv && source .venv/bin/activate +pip install -r requirements-dev.txt +ruff check app/ tests/ +pytest -q +``` + +Full stack (gmic/rembg): `docker compose up --build` with `DATA_HOST_DIR=./data`. + +Python **3.11** matches the Docker image; host 3.14+ cannot install pinned +`onnxruntime` — use Docker or a 3.11 venv for parity. diff --git a/CLEANUP_PLAN.md b/CLEANUP_PLAN.md new file mode 100644 index 0000000..5eeceec --- /dev/null +++ b/CLEANUP_PLAN.md @@ -0,0 +1,68 @@ +# Cleanup Plan — live.f12.rocks + +Phase 0 analysis (2026-07-18). Working branch: `cleanup/code-quality`. + +## Project snapshot + +| Area | Detail | +|------|--------| +| Stack | Python 3.11 (Docker), FastAPI, gmic, rembg, Docker Compose | +| Size | ~7 Python modules under `app/`, ~1.3k LOC | +| Tests | None (pre-cleanup) | +| Lint/format | None (pre-cleanup) | +| CI | None | +| Lockfile | Soft pins in `requirements.txt` only | + +## Findings (prioritized) + +1. **No test/CI/lint baseline** — high risk for refactors +2. **`pipeline.py` god module** (642 LOC) — medium risk; defer split until tests exist +3. **Meta JSON race** (worker + web concurrent writes) — documented, not fixed (behavior change) +4. **Dead code**: `blend_layers()` unused +5. **Duplication**: atomic JSON write ×3, `basicConfig` ×2, bool-env parsing ×2, compose env block ×2 +6. **Double meta read** on index page (`read_status` + `read_manifest`) +7. **`compare-bg/`** — separate experiment, leave as-is (gitignored cache) +8. **`make_random.py`** — external reference only, document in ARCHITECTURE + +## Risk matrix + +| Area | Risk | Action | +|------|------|--------| +| Formatter/linter setup | Low | Phase 1 | +| Dead code removal | Low | Phase 1 | +| `.gitignore` / `.python-version` | Low | Phase 1 | +| Extract `io_utils`, `env_utils` | Low | Phase 2 | +| Compose env DRY (YAML anchor) | Low | Phase 2 | +| Meta read dedup in `main.py` | Low | Phase 3 | +| Magic numbers → config constants | Low | Phase 3 | +| Characterization tests | Low | Phase 5 (before deeper refactors) | +| `pipeline.py` split | Medium | **Deferred** — tests first, split later | +| Meta file locking | Medium–High | **Deferred** — behavior change | +| Dependency major bumps | Medium | List only, no auto-bump | + +## Roadmap & commits + +| Phase | Step | Commit theme | +|-------|------|--------------| +| 0 | This file | `docs: add cleanup plan from phase-0 analysis` | +| 1 | pyproject.toml + ruff | `chore: add pyproject.toml with ruff and pytest config` | +| 1 | ruff format | `style: apply ruff format to Python sources` | +| 1 | dead code + gitignore | `chore: remove dead code and extend gitignore` | +| 2 | io_utils extraction | `refactor: extract atomic JSON helpers to io_utils` | +| 2 | env_utils + logging | `refactor: share env-bool parsing and logging setup` | +| 2 | compose DRY | `chore: deduplicate compose environment blocks` | +| 3 | config constants | `refactor: replace magic numbers with named constants` | +| 3 | meta read dedup | `refactor: read job meta once in list_jobs` | +| 4 | dev requirements | `chore: add dev requirements and python version pin` | +| 4 | Gitea CI | `ci: add lint and test workflow` | +| 5 | tests | `test: add characterization tests for core modules` | +| 5 | docs | `docs: update README and add ARCHITECTURE` | + +## Conscious non-goals + +- No `pipeline.py` module split in this pass +- No meta file locking / concurrency fix +- No dependency major version bumps +- No changes to `compare-bg/` experiment +- No deploy CI (manual `docker compose` on boka stays) +- No behavior changes to gmic/rembg processing logic diff --git a/CLEANUP_REPORT.md b/CLEANUP_REPORT.md new file mode 100644 index 0000000..a9ddefe --- /dev/null +++ b/CLEANUP_REPORT.md @@ -0,0 +1,89 @@ +# Cleanup Report — live.f12.rocks + +Branch: `cleanup/code-quality` (local only, not pushed). +Completed: 2026-07-18. + +## Summary + +Incremental code-quality cleanup across 10 commits. **No intentional +behavior changes** to the photo pipeline, worker loop, or web UX. + +## What changed + +| Step | What | Why | Risk | Verification | +|------|------|-----|------|--------------| +| Phase 0 | `CLEANUP_PLAN.md` | Analysis + roadmap | Low | — | +| Tooling | `pyproject.toml`, `requirements-dev.txt`, `.python-version` | Ruff + pytest baseline, pin 3.11 | Low | `ruff check`, `pytest` | +| Style | Ruff format entire `app/` | Consistent style | Low | `ruff format --check` | +| Dead code | Removed unused `blend_layers()` | Less noise | Low | grep + pytest | +| `.gitignore` | Build/coverage/venv patterns | Keep repo clean | Low | — | +| `io_utils` | Atomic JSON helpers | DRY meta/processed I/O | Low | `test_io_utils`, pipeline/worker tests | +| `env_utils` / `logging_config` | Shared bool env + logging | DRY | Low | `test_env_utils`, pytest | +| `compose.yml` | YAML anchor for env | DRY, fewer drift bugs | Low | `docker compose config` (manual) | +| Config | `PREPROCESS_TIMEOUT` constant | Named magic number | Low | default still 120s | +| `read_meta()` | Public meta accessor; `list_jobs` reads once | Perf + clarity | Low | `test_pipeline`, `test_main` | +| Tests | 33 characterization tests | Safe refactor baseline | Low | `pytest -q` | +| CI | `.gitea/workflows/ci.yml` | Lint + test on push/PR | Low | workflow syntax | +| Docs | `README.md`, `ARCHITECTURE.md` | Onboarding | Low | — | + +## What was NOT changed (and why) + +| Item | Reason | +|------|--------| +| `pipeline.py` module split | Medium risk without broader integration tests; deferred | +| Meta file locking | Behavior change; documented in `ARCHITECTURE.md` | +| Dependency major bumps | Per plan — list only, no auto-bump | +| `compare-bg/` | Separate experiment; left untouched | +| Deploy CI | Manual deploy on boka stays; STANDARDS says ask first | +| FastAPI `TemplateResponse` API migration | Deprecation warning only; no functional change | +| `_validate_job_id` always sanitizes to valid ID | Pre-existing; documented in tests | + +## Bugs noticed (not fixed) + +1. **Meta JSON race** — worker and web concurrent RMW without locking. +2. **`_filter_timeout_override` global** — thread-unsafe under concurrent remix (uvicorn workers). +3. **`sanitize_stem("a/b")` → `"b"`** — `Path.stem` drops path prefix before slash replacement. +4. **`_validate_job_id`** — sanitization makes rejection path effectively unreachable. + +## Dependency notes (suggestions only) + +| Package | Current pin | Note | +|---------|-------------|------| +| `fastapi` | `<0.117` | 0.117+ available; test before bump | +| `onnxruntime` | `<1.20` | Very old pin; rembg may allow newer in future | +| `pillow` | `<11` | Pillow 11 exists; verify rembg compatibility | + +No lockfile added — soft pins kept to avoid changing Docker resolve behavior. + +## How to verify locally + +```bash +git checkout cleanup/code-quality +python3.11 -m venv .venv && source .venv/bin/activate +pip install -r requirements-dev.txt +ruff check app/ tests/ +pytest -q +# Full stack: +cp .env.example .env # set DATA_HOST_DIR=./data +docker compose up --build +``` + +## Commits on branch + +``` +docs: add cleanup plan from phase-0 analysis +chore: add pyproject.toml with ruff and pytest config +style: apply ruff format and extend gitignore +refactor: extract atomic JSON helpers to io_utils +refactor: share env-bool parsing and logging setup +chore: deduplicate compose environment blocks +refactor: named preprocess timeout and single meta read +test: add characterization tests for core modules (+ CI workflow) +docs: update README and add ARCHITECTURE +``` + +## External references + +- `compare-bg/` — local venv `.venv-compare/` and 1.5GB model cache (gitignored) +- `make_random.py` — external reference script, not vendored +- Submodule/symlink: none diff --git a/README.md b/README.md index c028f53..cba0370 100644 --- a/README.md +++ b/README.md @@ -55,6 +55,24 @@ Drop a photo into `incoming/` (via Syncthing or locally). After a few seconds (poll interval + processing time) it shows up on the web UI; flat files land in `variants/` and `intermediates/` for the phone. +## Development + +Python **3.11** (same as `Dockerfile`). On a host with newer Python, use +Docker or a 3.11 venv — pinned `onnxruntime` does not publish wheels for 3.14+. + +```bash +python3.11 -m venv .venv && source .venv/bin/activate +pip install -r requirements-dev.txt + +ruff check app/ tests/ # lint +ruff format app/ tests/ # format +pytest -q # unit tests (no gmic/rembg) +``` + +CI (`.gitea/workflows/ci.yml`): ruff + pytest on push/PR. + +Architecture notes: `ARCHITECTURE.md`. Cleanup log: `CLEANUP_PLAN.md`. + ## Configuration (`.env`, see `.env.example`) | Var | Default | Notes | @@ -64,6 +82,8 @@ files land in `variants/` and `intermediates/` for the phone. | `BLEND_OPACITY` | `30%` | Default blend opacity for auto-generated variants | | `FILTER_TIMEOUT` | `120` | Seconds before a single gmic call is killed | | `MAX_FILTER_ATTEMPTS` | `8` | Random-filter retry budget per bg/fg pick | +| `MAX_EDGE_PX` | `2000` | Longest edge after ImageMagick preprocess | +| `PREPROCESS_TIMEOUT` | `120` | Seconds for preprocess step | | `NICE_LEVEL` | `18` | `nice -n` level for gmic/rembg subprocesses | ## Deploy notes / caveats diff --git a/app/config.py b/app/config.py index 60596f3..1a0a0ad 100644 --- a/app/config.py +++ b/app/config.py @@ -9,6 +9,8 @@ from __future__ import annotations import os from pathlib import Path +from .env_utils import env_bool + def _int_env(name: str, default: int) -> int: try: @@ -55,13 +57,15 @@ MAX_FILTER_ATTEMPTS = _int_env("MAX_FILTER_ATTEMPTS", 8) # Longest edge after preprocess (ImageMagick). Keeps rembg/gmic sane on # large camera JPEGs. MAX_EDGE_PX = _int_env("MAX_EDGE_PX", 2000) +# ImageMagick preprocess step timeout (resize + orient). +PREPROCESS_TIMEOUT = _int_env("PREPROCESS_TIMEOUT", 120) # Generous on purpose: the very first rembg call also downloads the model, # which can take a while depending on the link. REMBG_TIMEOUT = _int_env("REMBG_TIMEOUT", 600) # birefnet-general OOMs on the ~8GB event box; u2net + alpha matting fits # and is the practical equivalent of `rembg i -a`. REMBG_MODEL = os.environ.get("REMBG_MODEL", "u2net") -REMBG_ALPHA = os.environ.get("REMBG_ALPHA", "1").strip().lower() in {"1", "true", "yes", "on"} +REMBG_ALPHA = env_bool("REMBG_ALPHA", True) NICE_LEVEL = _int_env("NICE_LEVEL", 18) # gmic CLI binary, override for local dev if not on PATH. rembg has no diff --git a/app/env_utils.py b/app/env_utils.py new file mode 100644 index 0000000..73d5311 --- /dev/null +++ b/app/env_utils.py @@ -0,0 +1,12 @@ +"""Shared environment-variable parsing.""" + +from __future__ import annotations + +import os + + +def env_bool(name: str, default: bool) -> bool: + raw = os.environ.get(name) + if raw is None: + return default + return raw.strip().lower() in {"1", "true", "yes", "on"} diff --git a/app/io_utils.py b/app/io_utils.py new file mode 100644 index 0000000..79e0c23 --- /dev/null +++ b/app/io_utils.py @@ -0,0 +1,25 @@ +"""Small shared I/O helpers.""" + +from __future__ import annotations + +import json +from pathlib import Path +from typing import Any + + +def read_json(path: Path, *, default: Any = None) -> Any: + """Read JSON from *path*, returning *default* when missing or corrupt.""" + if not path.exists(): + return default + try: + return json.loads(path.read_text(encoding="utf-8")) + except (OSError, ValueError): + return default + + +def write_json_atomic(path: Path, data: Any) -> None: + """Write JSON via a temp file and atomic replace.""" + path.parent.mkdir(parents=True, exist_ok=True) + tmp = path.with_suffix(path.suffix + ".tmp") + tmp.write_text(json.dumps(data, indent=2, ensure_ascii=False), encoding="utf-8") + tmp.replace(path) diff --git a/app/logging_config.py b/app/logging_config.py new file mode 100644 index 0000000..91d7c37 --- /dev/null +++ b/app/logging_config.py @@ -0,0 +1,11 @@ +"""Shared logging setup for entry-point modules.""" + +from __future__ import annotations + +import logging + +_LOG_FORMAT = "%(asctime)s %(levelname)s [%(name)s] %(message)s" + + +def configure_logging(level: int = logging.INFO) -> None: + logging.basicConfig(level=level, format=_LOG_FORMAT) diff --git a/app/main.py b/app/main.py index 1d661d7..c20d1c0 100644 --- a/app/main.py +++ b/app/main.py @@ -14,8 +14,9 @@ from fastapi.staticfiles import StaticFiles from fastapi.templating import Jinja2Templates from . import config, pipeline, remix +from .logging_config import configure_logging -logging.basicConfig(level=logging.INFO, format="%(asctime)s %(levelname)s [%(name)s] %(message)s") +configure_logging() logger = logging.getLogger("livef12.web") app = FastAPI(title=config.SITE_TITLE) @@ -63,15 +64,15 @@ def _safe_job_file(job_id: str, filename: str) -> Path: def list_jobs() -> list[dict[str, Any]]: jobs = [] for job_id in pipeline.list_job_ids(): - status = pipeline.read_status(job_id) or {} - manifest = pipeline.read_manifest(job_id) or {} + data = pipeline.read_meta(job_id) or {} + variants = data.get("variants") or [] jobs.append( { "job_id": job_id, - "status": status.get("status", "unknown"), - "created_at": status.get("created_at") or manifest.get("created_at") or "", - "variant_count": len(manifest.get("variants", [])), - "thumbnail": (manifest.get("variants") or [{}])[-1].get("file") if manifest.get("variants") else None, + "status": data.get("status", "unknown"), + "created_at": data.get("created_at") or "", + "variant_count": len(variants), + "thumbnail": variants[-1].get("file") if variants else None, } ) jobs.sort(key=lambda j: j["created_at"] or j["job_id"], reverse=True) @@ -92,9 +93,11 @@ def job_detail(request: Request, job_id: str) -> HTMLResponse: paths = pipeline.job_paths(stem) status = pipeline.read_status(stem) or {} manifest = pipeline.read_manifest(stem) or {"variants": []} - intermediates = sorted( - p.name for p in paths.intermediates.glob(f"{stem}_*.png") if p.is_file() - ) if paths.intermediates.exists() else [] + intermediates = ( + sorted(p.name for p in paths.intermediates.glob(f"{stem}_*.png") if p.is_file()) + if paths.intermediates.exists() + else [] + ) return templates.TemplateResponse( "job.html", diff --git a/app/pipeline.py b/app/pipeline.py index d63c2a5..8bcc736 100644 --- a/app/pipeline.py +++ b/app/pipeline.py @@ -17,13 +17,15 @@ import subprocess import sys import time import uuid +from collections.abc import Iterator from contextlib import contextmanager from dataclasses import dataclass -from datetime import datetime, timezone +from datetime import UTC, datetime from pathlib import Path -from typing import Any, Iterator +from typing import Any from . import config +from .io_utils import read_json, write_json_atomic logger = logging.getLogger("livef12.pipeline") @@ -125,7 +127,9 @@ def load_assets() -> FilterAssets: if not bg_names or not fg_names: raise PipelineError("Keine gueltigen Filter in background/foreground Listen") - return FilterAssets(background_names=bg_names, foreground_names=fg_names, blend_modes=blend_modes, commands=commands) + return FilterAssets( + background_names=bg_names, foreground_names=fg_names, blend_modes=blend_modes, commands=commands + ) def run_gmic(args: list[str], output_image: Path) -> tuple[bool, str, float]: @@ -167,7 +171,9 @@ def apply_filter(image: Path, full_command: str, output_image: Path) -> tuple[bo return ok, err -def apply_filter_chain(image: Path, commands: tuple[str, ...], output_image: Path, tmp_dir: Path, prefix: str) -> tuple[bool, str]: +def apply_filter_chain( + image: Path, commands: tuple[str, ...], output_image: Path, tmp_dir: Path, prefix: str +) -> tuple[bool, str]: current = image for i, command in enumerate(commands): target = output_image if i == len(commands) - 1 else tmp_dir / f"{prefix}_post_{i}.png" @@ -178,11 +184,6 @@ def apply_filter_chain(image: Path, commands: tuple[str, ...], output_image: Pat return True, "" -def blend_layers(base: Path, overlay: Path, mode: str, output_image: Path) -> tuple[bool, str]: - ok, err, _ = run_gmic([str(base), str(overlay), "blend", f"{mode},{config.BLEND_OPACITY}"], output_image) - return ok, err - - def blend_layers_opacity(base: Path, overlay: Path, mode: str, opacity: str, output_image: Path) -> tuple[bool, str]: ok, err, _ = run_gmic([str(base), str(overlay), "blend", f"{mode},{opacity}"], output_image) return ok, err @@ -236,10 +237,10 @@ def preprocess_original(paths: JobPaths) -> JobPaths: ] ) try: - proc = subprocess.run(cmd, capture_output=True, text=True, timeout=120) + proc = subprocess.run(cmd, capture_output=True, text=True, timeout=config.PREPROCESS_TIMEOUT) except subprocess.TimeoutExpired as exc: tmp.unlink(missing_ok=True) - raise PipelineError("Preprocess Timeout nach 120s") from exc + raise PipelineError(f"Preprocess Timeout nach {config.PREPROCESS_TIMEOUT}s") from exc if proc.returncode != 0 or not tmp.exists() or tmp.stat().st_size == 0: tmp.unlink(missing_ok=True) err = strip_ansi((proc.stderr or proc.stdout or "").strip()) @@ -297,7 +298,7 @@ def pick_working_filter( def now_iso() -> str: - return datetime.now(timezone.utc).isoformat(timespec="seconds") + return datetime.now(UTC).isoformat(timespec="seconds") _UNSAFE_STEM_RE = re.compile(r"[^\w.\-]+", re.UNICODE) @@ -346,26 +347,20 @@ def job_paths(stem: str, original_suffix: str = ".jpg") -> JobPaths: ) -def _read_meta(stem: str) -> dict[str, Any] | None: +def read_meta(stem: str) -> dict[str, Any] | None: + """Return the full meta record for a job, or None if missing/corrupt.""" paths = job_paths(stem) - if not paths.meta.exists(): - return None - try: - return json.loads(paths.meta.read_text(encoding="utf-8")) - except (OSError, ValueError): - return None + data = read_json(paths.meta) + return data if isinstance(data, dict) else None def _write_meta(paths: JobPaths, data: dict[str, Any]) -> None: - paths.meta.parent.mkdir(parents=True, exist_ok=True) data["updated_at"] = now_iso() - tmp = paths.meta.with_suffix(".json.tmp") - tmp.write_text(json.dumps(data, indent=2, ensure_ascii=False), encoding="utf-8") - tmp.replace(paths.meta) + write_json_atomic(paths.meta, data) def read_status(job_id: str) -> dict[str, Any] | None: - data = _read_meta(job_id) + data = read_meta(job_id) if not data: return None return { @@ -380,7 +375,7 @@ def read_status(job_id: str) -> dict[str, Any] | None: def write_status(paths: JobPaths, status: str, **extra: Any) -> None: - data = _read_meta(paths.stem) or {} + data = read_meta(paths.stem) or {} data["job_id"] = paths.stem data["source_stem"] = paths.stem data["status"] = status @@ -390,7 +385,7 @@ def write_status(paths: JobPaths, status: str, **extra: Any) -> None: def read_manifest(job_id: str) -> dict[str, Any] | None: - data = _read_meta(job_id) + data = read_meta(job_id) if not data: return None return { @@ -405,7 +400,7 @@ def read_manifest(job_id: str) -> dict[str, Any] | None: def write_manifest(paths: JobPaths, manifest: dict[str, Any]) -> None: - data = _read_meta(paths.stem) or {} + data = read_meta(paths.stem) or {} data["job_id"] = paths.stem data["source_stem"] = manifest.get("source_stem") or paths.stem data["original_file"] = manifest.get("original_file") @@ -483,14 +478,18 @@ def compose_variant( if not bg_command: raise PipelineError(f"Unbekannter Background-Filter: {bg_name}") else: - bg_name, bg_command = pick_working_filter(assets.background_names, assets.commands, paths.original, tmp_dir, "bg", rng) + bg_name, bg_command = pick_working_filter( + assets.background_names, assets.commands, paths.original, tmp_dir, "bg", rng + ) if fg_name: fg_command = assets.commands.get(fg_name) if not fg_command: raise PipelineError(f"Unbekannter Foreground-Filter: {fg_name}") else: - fg_name, fg_command = pick_working_filter(assets.foreground_names, assets.commands, paths.rembg, tmp_dir, "fg", rng) + fg_name, fg_command = pick_working_filter( + assets.foreground_names, assets.commands, paths.rembg, tmp_dir, "fg", rng + ) p = { "bg_filtered": tmp_dir / f"{prefix}_bg_filtered.png", diff --git a/app/rembg_cli.py b/app/rembg_cli.py index cf98989..dae1763 100644 --- a/app/rembg_cli.py +++ b/app/rembg_cli.py @@ -18,12 +18,7 @@ import sys import traceback from pathlib import Path - -def _env_bool(name: str, default: bool) -> bool: - raw = os.environ.get(name) - if raw is None: - return default - return raw.strip().lower() in {"1", "true", "yes", "on"} +from .env_utils import env_bool def main() -> int: @@ -42,7 +37,7 @@ def main() -> int: output_path.parent.mkdir(parents=True, exist_ok=True) model = os.environ.get("REMBG_MODEL", "u2net") - alpha = _env_bool("REMBG_ALPHA", True) + alpha = env_bool("REMBG_ALPHA", True) session = new_session(model) output_path.write_bytes( remove( diff --git a/app/worker.py b/app/worker.py index 38ec362..09e7b9a 100644 --- a/app/worker.py +++ b/app/worker.py @@ -13,7 +13,6 @@ pipeline is CPU heavy (gmic/rembg) and the worker container is capped at from __future__ import annotations -import json import logging import shutil import time @@ -21,11 +20,10 @@ from pathlib import Path from typing import Any from . import config, pipeline +from .io_utils import read_json, write_json_atomic +from .logging_config import configure_logging -logging.basicConfig( - level=logging.INFO, - format="%(asctime)s %(levelname)s [%(name)s] %(message)s", -) +configure_logging() logger = logging.getLogger("livef12.worker") @@ -42,17 +40,15 @@ def ensure_dirs() -> None: def load_processed() -> dict[str, Any]: if not config.PROCESSED_FILE.exists(): return {} - try: - return json.loads(config.PROCESSED_FILE.read_text(encoding="utf-8")) - except (OSError, ValueError): - logger.warning("processed.json unreadable, starting fresh") - return {} + data = read_json(config.PROCESSED_FILE) + if isinstance(data, dict): + return data + logger.warning("processed.json unreadable, starting fresh") + return {} def save_processed(processed: dict[str, Any]) -> None: - tmp = config.PROCESSED_FILE.with_suffix(".json.tmp") - tmp.write_text(json.dumps(processed, indent=2, ensure_ascii=False), encoding="utf-8") - tmp.replace(config.PROCESSED_FILE) + write_json_atomic(config.PROCESSED_FILE, processed) def _file_key(path: Path) -> str: @@ -70,9 +66,7 @@ def _is_ignored_name(name: str) -> bool: """Skip Syncthing metadata/temp files and other dotfiles.""" if name.startswith("."): return True - if name.startswith(".syncthing.") or ".syncthing." in name: - return True - return False + return bool(name.startswith(".syncthing.") or ".syncthing." in name) def _is_stable(path: Path) -> bool: diff --git a/compose.yml b/compose.yml index 34ec8db..c84e3af 100644 --- a/compose.yml +++ b/compose.yml @@ -8,6 +8,20 @@ # Syncthing itself runs on the host (not in this compose). Copy .env.example # to .env for local overrides (e.g. DATA_HOST_DIR=./data). Never commit .env. +x-app-env: &app-env + DATA_DIR: /data + OUTPUT_COUNT: ${OUTPUT_COUNT:-3} + BLEND_OPACITY: ${BLEND_OPACITY:-30%} + BLEND_OPACITY_MIN: ${BLEND_OPACITY_MIN:-10} + BLEND_OPACITY_MAX: ${BLEND_OPACITY_MAX:-50} + FILTER_TIMEOUT: ${FILTER_TIMEOUT:-120} + FILTER_TIMEOUT_LONG: ${FILTER_TIMEOUT_LONG:-300} + MAX_FILTER_ATTEMPTS: ${MAX_FILTER_ATTEMPTS:-8} + MAX_EDGE_PX: ${MAX_EDGE_PX:-2000} + REMBG_MODEL: ${REMBG_MODEL:-u2net} + REMBG_ALPHA: ${REMBG_ALPHA:-1} + NICE_LEVEL: ${NICE_LEVEL:-18} + services: worker: build: . @@ -16,18 +30,7 @@ services: user: "1002:1002" command: ["python3", "-m", "app.worker"] environment: - DATA_DIR: /data - OUTPUT_COUNT: ${OUTPUT_COUNT:-3} - BLEND_OPACITY: ${BLEND_OPACITY:-30%} - BLEND_OPACITY_MIN: ${BLEND_OPACITY_MIN:-10} - BLEND_OPACITY_MAX: ${BLEND_OPACITY_MAX:-50} - FILTER_TIMEOUT: ${FILTER_TIMEOUT:-120} - FILTER_TIMEOUT_LONG: ${FILTER_TIMEOUT_LONG:-300} - MAX_FILTER_ATTEMPTS: ${MAX_FILTER_ATTEMPTS:-8} - MAX_EDGE_PX: ${MAX_EDGE_PX:-2000} - REMBG_MODEL: ${REMBG_MODEL:-u2net} - REMBG_ALPHA: ${REMBG_ALPHA:-1} - NICE_LEVEL: ${NICE_LEVEL:-18} + <<: *app-env volumes: - ${DATA_HOST_DIR:-/home/frank/sync.schwenk.online/data/livef12}:/data - ./assets:/data/assets:ro @@ -48,18 +51,7 @@ services: depends_on: - worker environment: - DATA_DIR: /data - OUTPUT_COUNT: ${OUTPUT_COUNT:-3} - BLEND_OPACITY: ${BLEND_OPACITY:-30%} - BLEND_OPACITY_MIN: ${BLEND_OPACITY_MIN:-10} - BLEND_OPACITY_MAX: ${BLEND_OPACITY_MAX:-50} - FILTER_TIMEOUT: ${FILTER_TIMEOUT:-120} - FILTER_TIMEOUT_LONG: ${FILTER_TIMEOUT_LONG:-300} - MAX_FILTER_ATTEMPTS: ${MAX_FILTER_ATTEMPTS:-8} - MAX_EDGE_PX: ${MAX_EDGE_PX:-2000} - REMBG_MODEL: ${REMBG_MODEL:-u2net} - REMBG_ALPHA: ${REMBG_ALPHA:-1} - NICE_LEVEL: ${NICE_LEVEL:-18} + <<: *app-env SITE_TITLE: live.f12.rocks volumes: - ${DATA_HOST_DIR:-/home/frank/sync.schwenk.online/data/livef12}:/data diff --git a/pyproject.toml b/pyproject.toml new file mode 100644 index 0000000..61cc886 --- /dev/null +++ b/pyproject.toml @@ -0,0 +1,41 @@ +[project] +name = "livef12rocks" +version = "0.1.0" +description = "Event photo pep pipeline: Syncthing drop -> gmic/rembg -> web" +readme = "README.md" +requires-python = ">=3.11,<3.12" + +[project.optional-dependencies] +dev = [ + "pytest>=8.3,<9", + "httpx>=0.27,<0.29", + "ruff>=0.8,<0.10", +] + +[tool.pytest.ini_options] +testpaths = ["tests"] +pythonpath = ["."] + +[tool.ruff] +target-version = "py311" +line-length = 120 +src = ["app", "tests"] + +[tool.ruff.lint] +select = [ + "E", # pycodestyle errors + "F", # pyflakes + "I", # isort + "UP", # pyupgrade + "B", # flake8-bugbear + "SIM", # flake8-simplify +] +ignore = [ + "B008", # FastAPI Depends/Form in defaults is intentional +] + +[tool.ruff.lint.per-file-ignores] +"tests/**" = ["S101"] # assert in tests + +[tool.ruff.format] +quote-style = "double" diff --git a/requirements-dev.txt b/requirements-dev.txt new file mode 100644 index 0000000..39aa9d7 --- /dev/null +++ b/requirements-dev.txt @@ -0,0 +1,4 @@ +-r requirements.txt +pytest>=8.3,<9 +httpx>=0.27,<0.29 +ruff>=0.8,<0.10 diff --git a/tests/conftest.py b/tests/conftest.py new file mode 100644 index 0000000..5ccea05 --- /dev/null +++ b/tests/conftest.py @@ -0,0 +1,26 @@ +"""Shared test fixtures.""" + +from __future__ import annotations + +from pathlib import Path + +import pytest + + +@pytest.fixture +def data_dir(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> Path: + """Point config path constants at a temporary data directory.""" + variants = tmp_path / "variants" + intermediates = tmp_path / "intermediates" + meta = tmp_path / "meta" + inbox = tmp_path / "incoming" + for directory in (variants, intermediates, meta, inbox): + directory.mkdir() + + monkeypatch.setattr("app.config.DATA_DIR", tmp_path) + monkeypatch.setattr("app.config.VARIANTS_DIR", variants) + monkeypatch.setattr("app.config.INTERMEDIATES_DIR", intermediates) + monkeypatch.setattr("app.config.META_DIR", meta) + monkeypatch.setattr("app.config.INBOX_DIR", inbox) + monkeypatch.setattr("app.config.PROCESSED_FILE", tmp_path / "processed.json") + return tmp_path diff --git a/tests/test_config.py b/tests/test_config.py new file mode 100644 index 0000000..9e7dee7 --- /dev/null +++ b/tests/test_config.py @@ -0,0 +1,30 @@ +"""Tests for config helpers.""" + +from __future__ import annotations + +from app import config +from app.config import _float_env, _int_env + + +def test_int_env_valid(monkeypatch) -> None: + monkeypatch.setenv("TEST_INT", "42") + assert _int_env("TEST_INT", 0) == 42 + + +def test_int_env_invalid_falls_back(monkeypatch) -> None: + monkeypatch.setenv("TEST_INT", "nope") + assert _int_env("TEST_INT", 7) == 7 + + +def test_float_env_valid(monkeypatch) -> None: + monkeypatch.setenv("TEST_FLOAT", "2.5") + assert _float_env("TEST_FLOAT", 0.0) == 2.5 + + +def test_supported_extensions() -> None: + assert ".jpg" in config.SUPPORTED_EXTENSIONS + assert ".jpeg" in config.SUPPORTED_EXTENSIONS + + +def test_opacity_choices_percentages() -> None: + assert all(choice.endswith("%") for choice in config.OPACITY_CHOICES) diff --git a/tests/test_env_utils.py b/tests/test_env_utils.py new file mode 100644 index 0000000..aac80a6 --- /dev/null +++ b/tests/test_env_utils.py @@ -0,0 +1,22 @@ +"""Tests for env_utils.""" + +from __future__ import annotations + +from app.env_utils import env_bool + + +def test_env_bool_default(monkeypatch) -> None: + monkeypatch.delenv("TEST_FLAG", raising=False) + assert env_bool("TEST_FLAG", True) is True + assert env_bool("TEST_FLAG", False) is False + + +def test_env_bool_truthy(monkeypatch) -> None: + for value in ("1", "true", "TRUE", "yes", "on"): + monkeypatch.setenv("TEST_FLAG", value) + assert env_bool("TEST_FLAG", False) is True + + +def test_env_bool_falsy(monkeypatch) -> None: + monkeypatch.setenv("TEST_FLAG", "0") + assert env_bool("TEST_FLAG", True) is False diff --git a/tests/test_io_utils.py b/tests/test_io_utils.py new file mode 100644 index 0000000..e99c58d --- /dev/null +++ b/tests/test_io_utils.py @@ -0,0 +1,26 @@ +"""Tests for io_utils.""" + +from __future__ import annotations + +import json +from pathlib import Path + +from app.io_utils import read_json, write_json_atomic + + +def test_read_json_missing_returns_default(tmp_path: Path) -> None: + assert read_json(tmp_path / "missing.json", default={}) == {} + + +def test_read_json_corrupt_returns_default(tmp_path: Path) -> None: + path = tmp_path / "bad.json" + path.write_text("not json", encoding="utf-8") + assert read_json(path, default=None) is None + + +def test_write_json_atomic_roundtrip(tmp_path: Path) -> None: + path = tmp_path / "data.json" + payload = {"job_id": "photo", "variants": []} + write_json_atomic(path, payload) + assert json.loads(path.read_text(encoding="utf-8")) == payload + assert not path.with_suffix(path.suffix + ".tmp").exists() diff --git a/tests/test_main.py b/tests/test_main.py new file mode 100644 index 0000000..5f43f11 --- /dev/null +++ b/tests/test_main.py @@ -0,0 +1,42 @@ +"""Tests for FastAPI routes and validation.""" + +from __future__ import annotations + +from pathlib import Path + +import pytest +from app import config, pipeline +from app.main import _safe_job_file, _validate_job_id, app +from fastapi import HTTPException +from fastapi.testclient import TestClient + + +@pytest.fixture +def client(data_dir: Path) -> TestClient: + return TestClient(app) + + +def test_validate_job_id_sanitizes_input() -> None: + assert _validate_job_id("foo@bar") == "foo_bar" + assert _validate_job_id("photo01") == "photo01" + + +def test_safe_job_file_rejects_wrong_prefix(data_dir: Path) -> None: + stem = "photo01" + paths = pipeline.job_paths(stem) + paths.variants.mkdir(parents=True, exist_ok=True) + target = paths.variants / f"{stem}_v1.png" + target.write_bytes(b"x") + with pytest.raises(HTTPException): + _safe_job_file(stem, "other_v1.png") + + +def test_index_empty(client: TestClient) -> None: + response = client.get("/") + assert response.status_code == 200 + assert config.SITE_TITLE in response.text + + +def test_job_not_found(client: TestClient) -> None: + response = client.get("/jobs/does-not-exist") + assert response.status_code == 404 diff --git a/tests/test_pipeline.py b/tests/test_pipeline.py new file mode 100644 index 0000000..aff36c7 --- /dev/null +++ b/tests/test_pipeline.py @@ -0,0 +1,80 @@ +"""Tests for pipeline pure helpers and meta I/O.""" + +from __future__ import annotations + +from pathlib import Path + +import pytest +from app import pipeline + + +@pytest.mark.parametrize( + ("raw", "expected"), + [ + ("photo.jpg", "photo"), + ("../evil", "evil"), + ("", "photo"), + (" spaced ", "spaced"), + ("foo@bar", "foo_bar"), + ], +) +def test_sanitize_stem(raw: str, expected: str) -> None: + assert pipeline.sanitize_stem(raw) == expected + + +def test_variant_filename() -> None: + assert pipeline.variant_filename("photo", "v1") == "photo_v1.png" + + +@pytest.mark.parametrize( + ("command", "expected_tail"), + [ + ("fx_blur 3", ["fx_blur", "3"]), + ("normalize", ["normalize"]), + ], +) +def test_gmic_filter_args(tmp_path: Path, command: str, expected_tail: list[str]) -> None: + image = tmp_path / "in.png" + image.touch() + args = pipeline.gmic_filter_args(image, command) + assert args[0] == str(image) + assert args[1:] == expected_tail + + +def test_next_variant_id() -> None: + manifest = {"variants": [{"id": "v1"}, {"id": "v2"}]} + assert pipeline.next_variant_id(manifest) == "v3" + + +def test_next_variant_id_after_gap() -> None: + manifest = {"variants": [{"id": "v1"}, {"id": "v3"}]} + assert pipeline.next_variant_id(manifest) == "v4" + + +def test_strip_ansi() -> None: + assert pipeline.strip_ansi("\x1b[31merror\x1b[0m") == "error" + + +def test_read_write_meta_roundtrip(data_dir: Path) -> None: + paths = pipeline.job_paths("event01") + pipeline.write_status(paths, "processing", source_file="event01.jpg") + meta = pipeline.read_meta("event01") + assert meta is not None + assert meta["status"] == "processing" + assert meta["source_file"] == "event01.jpg" + + status = pipeline.read_status("event01") + assert status is not None + assert status["status"] == "processing" + + manifest = pipeline.read_manifest("event01") + assert manifest is not None + assert manifest["variants"] == [] + + +def test_list_job_ids(data_dir: Path) -> None: + paths = pipeline.job_paths("a") + pipeline.write_status(paths, "done") + paths = pipeline.job_paths("b") + pipeline.write_status(paths, "processing") + assert sorted(pipeline.list_job_ids()) == ["a", "b"] diff --git a/tests/test_worker.py b/tests/test_worker.py new file mode 100644 index 0000000..e9c1b79 --- /dev/null +++ b/tests/test_worker.py @@ -0,0 +1,46 @@ +"""Tests for worker inbox helpers.""" + +from __future__ import annotations + +from pathlib import Path + +from app import config +from app.worker import _file_key, _is_ignored_name, find_new_files, load_processed, save_processed + + +def test_is_ignored_name() -> None: + assert _is_ignored_name(".stfolder") is True + assert _is_ignored_name(".syncthing.tmp") is True + assert _is_ignored_name("photo.jpg") is False + + +def test_find_new_files_skips_processed(data_dir: Path) -> None: + photo = config.INBOX_DIR / "party.jpg" + photo.write_bytes(b"fake") + stat = photo.stat() + processed = { + _file_key(photo): { + "size": stat.st_size, + "mtime": stat.st_mtime, + } + } + assert find_new_files(processed) == [] + + +def test_find_new_files_picks_supported(data_dir: Path) -> None: + (config.INBOX_DIR / "party.jpg").write_bytes(b"fake") + (config.INBOX_DIR / "notes.txt").write_text("nope", encoding="utf-8") + assert len(find_new_files({})) == 1 + + +def test_load_processed_corrupt_warns(data_dir: Path, caplog) -> None: + config.PROCESSED_FILE.write_text("{bad", encoding="utf-8") + with caplog.at_level("WARNING"): + assert load_processed() == {} + assert "processed.json unreadable" in caplog.text + + +def test_save_processed_roundtrip(data_dir: Path) -> None: + payload = {"incoming/party.jpg": {"size": 1, "mtime": 2}} + save_processed(payload) + assert load_processed() == payload