Files
agatha f953c88984 Feat: Pre-generate WebP thumbnails on upload for faster library load
- Add Pillow dependency and thumbnail.py with generate_thumbnail() — produces
  WebP ≤400px, preserves aspect ratio, never upscales, handles GIF frame 0
- Alembic migration 002 adds nullable thumbnail_key column to images table
- Upload route generates thumbnail via asyncio.to_thread (non-blocking),
  stores at {hash}-thumb; failure is tolerated and upload succeeds with null key
- New GET /api/v1/images/{id}/thumbnail endpoint: serves WebP thumbnail or
  falls back to original for pre-feature images; ETag + immutable cache headers
- Delete route cleans up thumbnail storage object alongside original
- Library grid switches from /file to /thumbnail for all image src bindings
- 59 tests passing (46 existing + 13 new across unit, upload, serving, delete)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-03 17:26:16 +00:00

247 lines
12 KiB
Markdown
Raw Permalink Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# Implementation Plan: Upload Thumbnails
**Branch**: `003-upload-thumbnails` | **Date**: 2026-05-03 | **Spec**: [spec.md](spec.md)
**Input**: Feature specification from `specs/003-upload-thumbnails/spec.md`
## Summary
When an image is uploaded, generate a WebP thumbnail (longest side ≤ 400 px,
aspect ratio preserved) and store it in S3 alongside the original. Add a
`GET /api/v1/images/{id}/thumbnail` endpoint that serves the thumbnail (or falls
back to the original for images that pre-date the feature). The Angular library
grid switches from `/file` to `/thumbnail`. The detail page is unchanged.
Changes span: a new Pillow dependency, a new `thumbnail.py` utility module, one
Alembic migration, the upload and delete routes, a new thumbnail serve route, and
the Angular image service and library component.
## Technical Context
**Language/Version**: Python 3.12+ (API); TypeScript strict mode (UI)
**Primary Dependencies**: FastAPI, Pillow (new — thumbnail generation), aiobotocore,
SQLAlchemy 2.x async, Alembic, Angular
**Storage**: S3-compatible object storage via `StorageBackend.put()` and `.get()`;
thumbnails stored at key `{sha256_hash}-thumb` in the same bucket
**Testing**: pytest + pytest-asyncio (API); Angular Karma/Jest + TestBed (UI)
**Target Platform**: Linux server (containerised); modern evergreen desktop browsers
**Project Type**: Web application — FastAPI API + Angular SPA
**Performance Goals**: 20-image grid transfers ≥ 80% less data than full-size;
first page of 1,000-image library loads in under 2 s
**Constraints**: Thumbnail generation is synchronous within the upload request;
thumbnail failure must not block upload success; no backfill of existing images in v1
**Scale/Scope**: Single-user personal application; upload frequency low
## Constitution Check
*GATE: Must pass before Phase 0 research. Re-checked after Phase 1 design below.*
| Principle | Check | Status |
|-----------|-------|--------|
| §2.1 Separation of concerns | `thumbnail.py` owns resize logic; router owns orchestration; UI knows nothing about S3 keys | ✅ |
| §2.2 Dependency direction | UI → API → Storage; thumbnail stored via `StorageBackend`; no upward imports | ✅ |
| §2.3 Storage abstraction | All thumbnail I/O via `StorageBackend.put()` and `.get()`; no raw S3 SDK calls in routes or `thumbnail.py` | ✅ |
| §2.4 Auth abstraction | No change to auth flow | ✅ |
| §2.5 DB abstraction | New `thumbnail_key` column accessed only through `ImageRepository`; migration added | ✅ |
| §2.6 No speculative abstraction | `thumbnail.py` is a concrete module-level function; no interface added because one implementation exists | ✅ |
| §3.1 API versioning | New route at `/api/v1/images/{id}/thumbnail` | ✅ |
| §3.3 Error shape | `image_not_found` and `storage_error` codes used consistently | ✅ |
| §4.2 Images immutable after upload | Thumbnail is generated at upload time only; never mutated | ✅ |
| §4.3 Dedup by hash | Duplicate upload returns existing record including existing `thumbnail_key`; no re-generation | ✅ |
| §5.1 TDD non-negotiable | Failing tests written before every implementation task | ✅ |
| §5.2 Test pyramid | Unit test for `thumbnail.py`; integration tests for new route + upload + delete | ✅ |
| §5.3 Test colocation | API tests in `api/tests/`; Angular spec files colocated with components | ✅ |
| §5.4 CI gate | All tests + ruff must pass before milestone is done | ✅ |
| §7.1 One-command start | No change to `docker-compose.yml` required | ✅ |
| §7.2 Env configuration | No new env vars; Pillow is a build dependency, not a runtime config | ✅ |
| §8 Scope boundaries | Backfill of existing images, multiple thumbnail sizes, animated WebP — all deferred | ✅ |
**Post-design re-check**: All gates still pass after Phase 1 design.
## Project Structure
### Documentation (this feature)
```text
specs/003-upload-thumbnails/
├── plan.md # This file
├── spec.md # Feature specification
├── research.md # Phase 0 decisions
├── data-model.md # Schema and module changes
├── contracts/
│ └── api.md # New endpoint + changed response shapes
├── checklists/
│ └── requirements.md # Spec quality checklist
└── tasks.md # Phase 2 output (/speckit-tasks — NOT created here)
```
### Files changed or created
```text
api/
├── pyproject.toml # Add Pillow dependency
├── app/
│ ├── thumbnail.py # NEW — generate_thumbnail()
│ ├── models.py # Add thumbnail_key column to Image
│ ├── repositories/
│ │ └── image_repo.py # Pass thumbnail_key on create
│ └── routers/
│ └── images.py # Upload: generate+store thumbnail
│ # Delete: remove thumbnail
│ # New route: GET /images/{id}/thumbnail
│ # _image_to_dict: add thumbnail_key field
└── alembic/
└── versions/
└── 002_add_thumbnail_key.py # NEW migration
ui/
└── src/
└── app/
├── services/
│ └── image.service.ts # Add getThumbnailUrl(); add thumbnail_key to ImageRecord
└── library/
└── library.component.ts # Use getThumbnailUrl() for grid image src
```
## Milestones
> **TDD ORDER IS MANDATORY** (constitution §5.1): For every milestone, write
> the failing test(s) first, confirm they fail, then implement until they pass.
---
### M1 — Thumbnail generation utility
**Goal**: A tested, self-contained function that produces a WebP thumbnail from
raw image bytes.
**Deliverables**:
- Add `pillow>=10.0` to `[project.dependencies]` in `api/pyproject.toml`
- Create `api/app/thumbnail.py` with `generate_thumbnail(data: bytes, mime_type: str) -> bytes`:
- Open image bytes with Pillow
- Seek to frame 0 (handles animated GIFs)
- Convert mode as needed for WebP output
- Resize to fit within 400×400 using LANCZOS resampling (never upscale)
- Encode as WebP quality 80 and return bytes
- Unit tests in `api/tests/unit/test_thumbnail.py`:
- `test_thumbnail_is_webp` — output starts with WebP magic bytes
- `test_thumbnail_fits_within_400px` — both dimensions ≤ 400
- `test_thumbnail_preserves_aspect_ratio` — ratio within 1% of original
- `test_thumbnail_handles_gif_first_frame` — GIF input produces static WebP
- `test_thumbnail_handles_png_with_alpha` — RGBA PNG produces valid WebP
- `test_thumbnail_does_not_upscale` — 100×100 image stays ≤ 100×100
**Done criterion**: All unit tests pass; `ruff check api/` passes.
---
### M2 — Database migration
**Goal**: The `images` table has a nullable `thumbnail_key` column; the ORM model
and repository reflect it.
**Deliverables**:
- `api/alembic/versions/002_add_thumbnail_key.py``upgrade` adds
`VARCHAR(70) NULLABLE` column; `downgrade` drops it
- `api/app/models.py`: add `thumbnail_key: Mapped[str | None]` mapped to
`String(70)`, `nullable=True`, `default=None`
- `api/app/repositories/image_repo.py`: add `thumbnail_key: str | None = None`
parameter to `create()`; persist it on the new `Image` instance
**Done criterion**: `alembic upgrade head` runs cleanly inside Docker; all
existing 46 integration tests still pass (new column is nullable, no existing
test breaks).
---
### M3 — Upload route: generate and store thumbnail
**Goal**: Every new upload generates a thumbnail; duplicates reuse the existing
record; failures are tolerated without blocking the upload.
**TDD first** — new tests in `api/tests/integration/test_upload.py`:
- `test_upload_returns_thumbnail_key` — upload response includes non-null
`thumbnail_key` ending in `-thumb`
- `test_duplicate_upload_reuses_thumbnail_key` — second upload of the same
file returns the same `thumbnail_key` as the first
- `test_upload_succeeds_when_thumbnail_fails` — patch `generate_thumbnail` to
raise, upload returns 200/201 with `thumbnail_key: null`
**Implementation** in `api/app/routers/images.py` `upload_image()`:
1. After `await storage.put(hash_hex, data, mime_type)`, attempt thumbnail generation
and storage in a try/except; catch any exception and leave `thumbnail_key` as `None`
2. Call `thumb_bytes = await asyncio.to_thread(generate_thumbnail, data, mime_type)`
`generate_thumbnail` is CPU-bound (Pillow); `asyncio.to_thread` runs it in the
default thread pool executor so it does not block the async event loop
3. Pass `thumbnail_key` to `image_repo.create()`
4. Add `"thumbnail_key": image.thumbnail_key` to `_image_to_dict()`
**Done criterion**: New tests pass; all 46 existing tests pass.
---
### M4 — New `GET /api/v1/images/{id}/thumbnail` endpoint
**Goal**: Clients fetch thumbnail content; falls back to original if no thumbnail exists.
**TDD first** — new tests in `api/tests/integration/test_serving.py`:
- `test_thumbnail_returns_webp` — upload image, call `/thumbnail`, assert
200, `content-type: image/webp`, ETag, `Cache-Control` with immutable
- `test_thumbnail_fallback_returns_original` — set `thumbnail_key=None` on a
record, call `/thumbnail`, assert 200 with original mime_type
- `test_thumbnail_unknown_id_returns_404` — unknown UUID → 404 `image_not_found`
**Implementation**: new route `GET /images/{image_id}/thumbnail` in
`api/app/routers/images.py` using `image.thumbnail_key or image.storage_key`
to select the key, and `"image/webp" if image.thumbnail_key else image.mime_type`
for the content type. Same `ETag` + `Cache-Control` headers as `/file`.
**Done criterion**: All new tests pass; all existing tests pass.
---
### M5 — Delete route: remove thumbnail from storage
**Goal**: Deleting an image also removes its thumbnail; no orphaned objects left.
**TDD first** — new test in `api/tests/integration/test_delete.py`:
- `test_delete_removes_thumbnail` — upload image, delete it, then verify
`GET /images/{id}/thumbnail` returns 404
**Implementation** in `api/app/routers/images.py` `delete_image()`: after
deleting the DB record and the original object, call `await storage.delete(image.thumbnail_key)`
if `image.thumbnail_key` is not None.
**Done criterion**: New test passes; all existing delete tests pass.
---
### M6 — UI: library grid uses thumbnail endpoint
**Goal**: Library grid fetches thumbnails instead of full-size originals; detail
page is unchanged.
**Deliverables**:
- `ui/src/app/services/image.service.ts`:
- Add `thumbnail_key: string | null` to `ImageRecord` interface
- Add `getThumbnailUrl(id: string): string` returning `/api/v1/images/${id}/thumbnail`
- `ui/src/app/library/library.component.ts` + template: replace
`getFileUrl(image.id)` with `getThumbnailUrl(image.id)` for grid `<img src>`
- Update Angular spec files: add `thumbnail_key: null` to all `ImageRecord`
mock objects
- Verify `ng test` passes and `ng build` succeeds
**Done criterion**: Angular build clean; all Angular tests pass; library grid
`<img>` elements reference `/thumbnail` not `/file`.
## Post-design Constitution Re-check
| Principle | Verdict |
|-----------|---------|
| §2.3 Storage abstraction | All thumbnail I/O via `StorageBackend`; `thumbnail.py` never touches S3 directly | ✅ |
| §2.5 DB abstraction | `thumbnail_key` persisted only through `ImageRepository.create()` | ✅ |
| §2.6 No speculative abstraction | One concrete function; no interface | ✅ |
| §4.2 Immutability | Thumbnail written once at upload; never mutated | ✅ |
| §5.1 TDD | Failing tests written before each milestone's implementation | ✅ |
All gates pass. Feature is ready for `/speckit-tasks`.