agatha
f953c88984
- 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>
187 lines
13 KiB
Markdown
187 lines
13 KiB
Markdown
# Tasks: Upload Thumbnails
|
||
|
||
**Input**: Design documents from `specs/003-upload-thumbnails/`
|
||
**Prerequisites**: plan.md ✅, spec.md ✅, research.md ✅, data-model.md ✅, contracts/api.md ✅
|
||
|
||
**TDD**: Tests are non-negotiable per constitution §5.1. Every test task MUST be written and confirmed failing before its implementation task runs.
|
||
|
||
**Organization**: Tasks grouped by user story to enable independent implementation and testing.
|
||
|
||
## Format: `[ID] [P?] [Story] Description`
|
||
|
||
- **[P]**: Can run in parallel (different files, no dependencies on incomplete tasks)
|
||
- **[Story]**: Which user story this task belongs to (US1, US2, US3)
|
||
|
||
---
|
||
|
||
## Phase 1: Setup
|
||
|
||
- [X] T001 Add `pillow>=10.0` to `[project.dependencies]` in `api/pyproject.toml`; rebuild the Docker API image (`docker compose build api`) so Pillow is available inside the container for all subsequent test runs
|
||
|
||
---
|
||
|
||
## Phase 2: Foundational (Blocking Prerequisites)
|
||
|
||
**Purpose**: Thumbnail generation logic and the DB schema change that all three user stories depend on.
|
||
|
||
**⚠️ CRITICAL**: All tasks in this phase must complete before any user story work begins.
|
||
|
||
### Tests for thumbnail utility (write FIRST — must FAIL before T005) ⚠️
|
||
|
||
- [X] T002 Write 6 unit tests in `api/tests/unit/test_thumbnail.py` — `test_thumbnail_is_webp` (output begins with WebP magic bytes `RIFF...WEBP`), `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 → static WebP, no animation), `test_thumbnail_handles_png_with_alpha` (RGBA PNG → valid WebP output), `test_thumbnail_does_not_upscale` (100×100 input stays ≤ 100×100); to confirm the TDD red state, first create an empty stub `api/app/thumbnail.py` (so pytest can collect tests), then run `pytest api/tests/unit/test_thumbnail.py` and confirm all 6 **fail** with assertion errors (not import errors)
|
||
|
||
### Thumbnail utility implementation
|
||
|
||
- [X] T003 Create `api/app/thumbnail.py` with `generate_thumbnail(data: bytes, mime_type: str) -> bytes`: open bytes with `PIL.Image.open(BytesIO(data))`, call `.seek(0)` to target frame 0 (GIF support), convert mode to RGB or RGBA as needed for WebP, call `.thumbnail((400, 400), PIL.Image.LANCZOS)` (never upscales), save to a `BytesIO` buffer as WebP quality=80, return bytes; run `pytest api/tests/unit/test_thumbnail.py` and confirm all 6 pass
|
||
|
||
### Database migration
|
||
|
||
- [X] T004 [P] Create `api/alembic/versions/002_add_thumbnail_key.py` with `upgrade()` calling `op.add_column("images", sa.Column("thumbnail_key", sa.String(70), nullable=True))` and `downgrade()` calling `op.drop_column("images", "thumbnail_key")`; set `revision="002"`, `down_revision="001"`
|
||
- [X] T005 [P] Add `thumbnail_key: Mapped[str | None] = mapped_column(String(70), nullable=True, default=None)` to the `Image` class in `api/app/models.py`
|
||
- [X] T006 Add `thumbnail_key: str | None = None` keyword argument to `ImageRepository.create()` in `api/app/repositories/image_repo.py`; include it in the `Image(...)` constructor call; run `docker compose run --rm api alembic upgrade head` inside the container and confirm migration applies cleanly; run `pytest api/` to confirm all 46 existing tests still pass
|
||
|
||
**Checkpoint**: Pillow available, thumbnail.py works, schema migrated, all existing tests green.
|
||
|
||
---
|
||
|
||
## Phase 3: User Story 3 — Thumbnails Generated Automatically on Upload (Priority: P1) 🎯
|
||
|
||
**Goal**: Every new upload triggers thumbnail generation and storage as part of the same request. No extra step required from the user.
|
||
|
||
**Independent Test**: Upload any supported image. Immediately check the upload response — it includes a non-null `thumbnail_key`. Call `GET /api/v1/images/{id}/thumbnail` — it returns 200 with `content-type: image/webp`.
|
||
|
||
### Tests for User Story 3 (write FIRST — must FAIL before T008) ⚠️
|
||
|
||
- [X] T007 [US3] Add three tests to `api/tests/integration/test_upload.py`: `test_upload_returns_thumbnail_key` (upload a JPEG/PNG/WebP, assert response JSON contains `thumbnail_key` ending in `-thumb`), `test_duplicate_upload_reuses_thumbnail_key` (upload same file twice, assert both responses have equal, non-null `thumbnail_key`), and `test_upload_succeeds_when_thumbnail_fails` (patch `generate_thumbnail` to raise an exception, upload an image, assert response is 200/201 with `thumbnail_key: null` — upload must not be blocked by thumbnail failure); run `pytest api/tests/integration/test_upload.py` and confirm all three new tests **fail**
|
||
|
||
### Implementation for User Story 3
|
||
|
||
- [X] T008 [US3] In `api/app/routers/images.py` `upload_image()`: import `asyncio` and `generate_thumbnail` from `app.thumbnail`; after `await storage.put(hash_hex, data, mime_type)`, wrap thumbnail generation in a try/except — call `thumb_bytes = await asyncio.to_thread(generate_thumbnail, data, mime_type)` (runs CPU-bound Pillow work off the async event loop), store result via `await storage.put(f"{hash_hex}-thumb", thumb_bytes, "image/webp")`, set `thumbnail_key = f"{hash_hex}-thumb"`; on any exception log a warning and leave `thumbnail_key = None`; pass `thumbnail_key=thumbnail_key` to `image_repo.create()`
|
||
- [X] T009 [US3] Add `"thumbnail_key": image.thumbnail_key` to the dict returned by `_image_to_dict()` in `api/app/routers/images.py`
|
||
- [X] T010 [US3] Run `pytest api/tests/integration/test_upload.py` to confirm both new tests pass; then run `pytest api/` to confirm no regressions
|
||
|
||
**Checkpoint**: Upload generates and stores a thumbnail. Duplicate uploads reuse the existing thumbnail. `thumbnail_key` appears in all image metadata responses.
|
||
|
||
---
|
||
|
||
## Phase 4: User Story 1 — Fast Library Load (Priority: P1)
|
||
|
||
**Goal**: The library grid fetches compact WebP thumbnails instead of full-size originals, dramatically reducing page-load bandwidth.
|
||
|
||
**Independent Test**: Open the library in a browser with DevTools network tab open. All grid `<img>` elements request `/api/v1/images/{id}/thumbnail`. Total bytes transferred for a 20-image grid is a small fraction of what the originals would cost.
|
||
|
||
### Tests for User Story 1 (write FIRST — must FAIL before T013) ⚠️
|
||
|
||
- [X] T011 [US1] Add `test_thumbnail_returns_webp` (upload image, GET `/thumbnail`, assert 200, `content-type: image/webp`, ETag header matches `f'"{image_hash}"'`, `"immutable"` in `cache-control`, non-empty content), `test_thumbnail_fallback_returns_original` (manually set `thumbnail_key=None` on a DB record via the session fixture, GET `/thumbnail`, assert 200 with original `mime_type` in content-type), and `test_thumbnail_unknown_id_returns_404` (unknown UUID, assert 404 `image_not_found`) to `api/tests/integration/test_serving.py`; run and confirm all three **fail**
|
||
- [X] T012 [P] [US1] In `ui/src/app/services/image.service.ts`: add `thumbnail_key: string | null` field to the `ImageRecord` interface; add `getThumbnailUrl(id: string): string { return \`${this.base}/images/${id}/thumbnail\`; }` method to `ImageService`
|
||
|
||
### Implementation for User Story 1
|
||
|
||
- [X] T013 [US1] Add `GET /api/v1/images/{image_id}/thumbnail` route to `api/app/routers/images.py`: look up image (404 if missing), select `key = image.thumbnail_key or image.storage_key` and `media_type = "image/webp" if image.thumbnail_key else image.mime_type`, call `await storage.get(key)` in a try/except (500 `storage_error` on failure), return `Response(content=data, media_type=media_type, headers={"ETag": f'"{image.hash}"', "Cache-Control": "public, max-age=31536000, immutable"})`
|
||
- [X] T014 [US1] In `ui/src/app/library/library.component.ts` and its HTML template: replace every use of `imageService.getFileUrl(image.id)` (or equivalent) with `imageService.getThumbnailUrl(image.id)` for grid cell `<img src>` bindings
|
||
- [X] T015 [US1] Add `thumbnail_key: null` to every `ImageRecord` mock/stub object in `ui/src/app/services/image.service.spec.ts`, `ui/src/app/library/library.component.spec.ts`, `ui/src/app/detail/detail.component.spec.ts`, and `ui/src/app/upload/upload.component.spec.ts`
|
||
- [X] T016 [US1] Run `pytest api/tests/integration/test_serving.py` to confirm all three new thumbnail tests pass and no existing serving tests regress
|
||
|
||
**Checkpoint**: `GET /api/v1/images/{id}/thumbnail` serves WebP with caching headers. Falls back to original for legacy images. Library grid `<img>` elements all use the thumbnail endpoint.
|
||
|
||
---
|
||
|
||
## Phase 5: User Story 2 — Full-Size Image on Detail Page (Priority: P1)
|
||
|
||
**Goal**: The detail page continues to display the full-size original. No regression introduced by the thumbnail work.
|
||
|
||
**Independent Test**: Navigate to any image detail page. The image displayed is full-resolution. Browser DevTools shows the detail `<img>` requests `/api/v1/images/{id}/file`, not `/thumbnail`.
|
||
|
||
### Verification for User Story 2
|
||
|
||
- [X] T017 [US2] Confirm `ui/src/app/detail/detail.component.ts` still calls `imageService.getFileUrl(image.id)` (not `getThumbnailUrl`) for its `<img src>` — no code change expected; if the file was accidentally updated in T014 or T015, revert the detail component to `getFileUrl`
|
||
- [X] T018 [US2] Run `ng test` (inside the UI container or locally) and confirm all Angular unit tests pass including the detail component spec; run `ng build` to confirm the Angular build succeeds
|
||
|
||
**Checkpoint**: Detail page verified unchanged. Angular build and tests clean.
|
||
|
||
---
|
||
|
||
## Phase 6: Polish & Cross-Cutting Concerns
|
||
|
||
**Purpose**: Delete cleanup, final test run, linting.
|
||
|
||
### Delete thumbnail on image deletion
|
||
|
||
- [X] T019 Write `test_delete_removes_thumbnail` in `api/tests/integration/test_delete.py`: upload an image, delete it, then `GET /api/v1/images/{id}/thumbnail` and assert 404; run and confirm it **fails** (currently delete does not remove the thumbnail object)
|
||
- [X] T020 In `api/app/routers/images.py` `delete_image()`: capture `thumbnail_key = image.thumbnail_key` before `image_repo.delete(image)`; after deleting the original via `await storage.delete(storage_key)`, add `if thumbnail_key: await storage.delete(thumbnail_key)`; run `pytest api/tests/integration/test_delete.py` to confirm new test and all existing delete tests pass
|
||
|
||
### Final validation
|
||
|
||
- [X] T021 [P] Run `~/.local/bin/ruff check api/app/thumbnail.py api/app/routers/images.py api/app/models.py api/app/repositories/image_repo.py api/tests/unit/test_thumbnail.py` and fix any lint issues in the changed files
|
||
- [X] T022 Run `pytest api/ -v` and confirm all tests pass; record final count (expected: 46 existing + ~10 new = ~56 total)
|
||
|
||
---
|
||
|
||
## Dependencies & Execution Order
|
||
|
||
### Phase Dependencies
|
||
|
||
- **Setup (Phase 1)**: No dependencies — start immediately
|
||
- **Foundational (Phase 2)**: Depends on Phase 1 (Pillow must be installed before tests can import it)
|
||
- **US3 (Phase 3)**: Depends on Phase 2 complete (T002–T006 all done)
|
||
- **US1 (Phase 4)**: Depends on Phase 3 complete (upload must set `thumbnail_key` before the endpoint can serve one)
|
||
- **US2 (Phase 5)**: Depends on Phase 4 complete (Angular changes in T014/T015 must be done before verifying no regression)
|
||
- **Polish (Phase 6)**: Depends on Phases 3, 4, and 5 complete
|
||
|
||
### Within Each Phase
|
||
|
||
- T002 (write failing tests) MUST precede T003 (implement thumbnail.py)
|
||
- T004 and T005 can run in parallel (different files)
|
||
- T006 (repo change) depends on T005 (model must compile first)
|
||
- T007 (write failing upload tests) MUST precede T008 (implement upload change)
|
||
- T011 (write failing serving tests) and T012 (UI service) can run in parallel
|
||
- T011 MUST precede T013 (implement thumbnail route)
|
||
- T019 (write failing delete test) MUST precede T020 (implement delete cleanup)
|
||
|
||
---
|
||
|
||
## Parallel Example: Phase 2 (Foundational)
|
||
|
||
```bash
|
||
# After T003 is done, run T004 and T005 together:
|
||
Task: "Create 002_add_thumbnail_key.py migration in api/alembic/versions/"
|
||
Task: "Add thumbnail_key column to Image ORM in api/app/models.py"
|
||
```
|
||
|
||
## Parallel Example: Phase 4 (US1)
|
||
|
||
```bash
|
||
# T011 and T012 touch different layers — run together:
|
||
Task: "Write 3 failing thumbnail serving tests in api/tests/integration/test_serving.py"
|
||
Task: "Add getThumbnailUrl() and thumbnail_key field to ui/src/app/services/image.service.ts"
|
||
```
|
||
|
||
---
|
||
|
||
## Implementation Strategy
|
||
|
||
### MVP (All three user stories — tightly coupled)
|
||
|
||
All three user stories are P1 and interdependent: US3 (generation) enables US1 (grid) which proves US2 (detail unchanged) by contrast. Complete all phases in order.
|
||
|
||
1. Phase 1: T001 (Pillow setup)
|
||
2. Phase 2: T002–T006 (core infrastructure)
|
||
3. Phase 3: T007–T010 (upload generates thumbnail)
|
||
4. Phase 4: T011–T016 (thumbnail endpoint + UI)
|
||
5. Phase 5: T017–T018 (detail page verification)
|
||
6. Phase 6: T019–T022 (delete cleanup + polish)
|
||
7. **STOP and VALIDATE**: Open library in browser; DevTools shows `/thumbnail` requests; bandwidth used is a fraction of original file sizes
|
||
|
||
### Total tasks: 22 (T001–T022)
|
||
|
||
---
|
||
|
||
## Notes
|
||
|
||
- [P] tasks touch different files and have no mutual dependencies within their phase
|
||
- T002 must be run **before** T003 and confirmed failing — this is the TDD red step
|
||
- T007, T011, T019 are all "write failing test" steps — confirm failure before implementing
|
||
- `thumbnail_key` in the API response is informational; the UI always calls `/thumbnail` and lets the endpoint handle the fallback — no client-side conditional logic needed
|
||
- Existing images (pre-dating this feature) will have `thumbnail_key: null`; the `/thumbnail` endpoint serves their original transparently
|
||
- The backfill migration for existing images is explicitly out of scope for this feature
|