agatha
aaacfae653
API responses now include file_url and thumbnail_url fields. When S3_PUBLIC_BASE_URL is configured, these point to the CDN domain; when unset, they fall back to the existing API proxy paths so local dev requires no additional setup. UI updated to use response URL fields directly instead of constructing proxy URLs client-side. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
117 lines
8.8 KiB
Markdown
117 lines
8.8 KiB
Markdown
# Tasks: CDN Image Serving
|
||
|
||
**Input**: Design documents from `specs/014-r2-cdn-serving/`
|
||
**Prerequisites**: plan.md ✅, spec.md ✅, research.md ✅, contracts/image-response.md ✅, quickstart.md ✅
|
||
|
||
**Tests**: Unit tests for URL construction logic; integration tests asserting `file_url` and `thumbnail_url` in all image responses. Tests accompany each implementation task per §5.1.
|
||
|
||
**Organization**: Phase 1 adds the config value (foundational — blocks everything). Phase 2 implements US1 (CDN URL serving in API + UI consumption). Phase 3 verifies US2 (local dev fallback). Polish runs the full suite and manual end-to-end check.
|
||
|
||
## Format: `[ID] [P?] [Story] Description`
|
||
|
||
- **[P]**: Can run in parallel (different files, no dependencies)
|
||
- **[Story]**: Which user story this task belongs to
|
||
|
||
---
|
||
|
||
## Phase 1: Foundational (Config)
|
||
|
||
**Goal**: Add `s3_public_base_url` to config and `.env.example`. All US1 and US2 tasks depend on this.
|
||
|
||
**⚠️ CRITICAL**: No user story work can begin until this phase is complete.
|
||
|
||
- [X] T001 Add `s3_public_base_url: str | None = None` to the `Settings` class in `api/app/config.py` (after `api_base_url`); add `S3_PUBLIC_BASE_URL=` with comment "# CDN base URL for serving images (e.g. https://cdn.example.com). Leave empty in local dev to use API proxy fallback." to `.env.example` after the `API_BASE_URL` line
|
||
|
||
**Checkpoint**: Config in place — user story work can begin.
|
||
|
||
---
|
||
|
||
## Phase 2: User Story 1 — Images Load Directly from CDN (Priority: P1) 🎯 MVP
|
||
|
||
**Goal**: API returns `file_url` and `thumbnail_url` in all image responses; UI uses those fields to render images rather than constructing proxy URLs client-side.
|
||
|
||
**Independent Test**: With `S3_PUBLIC_BASE_URL=https://cdn.reactbin.juggalol.com` set, call `GET /api/v1/images` and confirm each item has `file_url` starting with `https://cdn.reactbin.juggalol.com/` and `thumbnail_url` starting with `https://cdn.reactbin.juggalol.com/` (or null). Open the library page in a browser and confirm image requests go to the CDN domain in the network panel.
|
||
|
||
- [X] T002 [US1] Write unit tests in `api/tests/unit/test_url_construction.py` covering four cases: (1) CDN base set, image has thumbnail — `file_url` and `thumbnail_url` are CDN URLs; (2) CDN base set, image has no thumbnail — `thumbnail_url` is None; (3) CDN base not set, image has thumbnail — `file_url` is `/api/v1/images/{id}/file` and `thumbnail_url` is `/api/v1/images/{id}/thumbnail`; (4) CDN base not set, no thumbnail — `thumbnail_url` is None. Test the trailing-slash normalisation case (CDN base with trailing slash produces no double-slash). Import and call `_image_to_dict` directly with a mock `Image` object.
|
||
|
||
- [X] T003 [US1] Update `_image_to_dict` in `api/app/routers/images.py`: add `cdn_base: str | None = None` keyword parameter; compute `_base = cdn_base.rstrip("/") if cdn_base else None`; set `file_url = f"{_base}/{image.storage_key}" if _base else f"/api/v1/images/{image.id}/file"`; set `thumbnail_url = (f"{_base}/{image.thumbnail_key}" if _base else f"/api/v1/images/{image.id}/thumbnail") if image.thumbnail_key else None`; add `"file_url": file_url` and `"thumbnail_url": thumbnail_url` to the returned dict. Run `make test-unit` and confirm T002 tests pass.
|
||
|
||
- [X] T004 [US1] Update every `_image_to_dict(...)` call site in `api/app/routers/images.py`: at the top of each endpoint function that calls `_image_to_dict`, add `_cdn_base = get_settings().s3_public_base_url` (import `get_settings` is already present); pass `cdn_base=_cdn_base` to every `_image_to_dict` call in that endpoint. Affected endpoints: `upload_image`, `list_images`, `get_image`, `patch_image_tags`. Confirm `get_settings()` is called once per endpoint, not once per image in a loop (for `list_images`, call it before the list comprehension).
|
||
|
||
- [X] T005 [US1] Update integration tests: in `api/tests/integration/test_upload.py`, add assertions after existing response checks that `"file_url"` is present in the response body and starts with `/api/v1/images/` (since no CDN is configured in test env); add the same assertion for `"thumbnail_url"` in `test_upload_returns_thumbnail_key`; add assertion that `thumbnail_url` is None in the test that expects `thumbnail_key` to be None. Run `make test-integration` and confirm all pass.
|
||
|
||
- [X] T006 [P] [US1] Update `ui/src/app/services/image.service.ts`: add `file_url: string` and `thumbnail_url: string | null` to the `ImageRecord` interface; remove the `getFileUrl(id: string): string` method; remove the `getThumbnailUrl(id: string): string` method.
|
||
|
||
- [X] T007 [P] [US1] Update `ui/src/app/library/library.component.ts`: replace `[src]="imageService.getThumbnailUrl(img.id)"` (line 77) with `[src]="img.thumbnail_url ?? img.file_url"` — fall back to `file_url` when thumbnail is absent (FR-009); update `ui/src/app/library/library.component.spec.ts` to add `file_url` and `thumbnail_url` to any mock `ImageRecord` objects and remove any references to `getThumbnailUrl()`.
|
||
|
||
- [X] T008 [P] [US1] Update `ui/src/app/detail/detail.component.ts`: replace `[src]="imageService.getFileUrl(image.id)"` (line 52) with `[src]="image.file_url"`; update `ui/src/app/detail/detail.component.spec.ts` to add `file_url` and `thumbnail_url` to any mock `ImageRecord` objects and remove any references to `getFileUrl()`.
|
||
|
||
- [X] T009 [US1] Update `ui/src/app/services/image.service.spec.ts`: add `file_url` and `thumbnail_url` fields to any mock `ImageRecord` objects used in tests; remove any test cases that test `getFileUrl()` or `getThumbnailUrl()` (these methods no longer exist). Run UI tests and confirm they pass.
|
||
|
||
**Checkpoint**: US1 complete. API returns CDN URLs when configured; UI uses response fields to render images.
|
||
|
||
---
|
||
|
||
## Phase 3: User Story 2 — Local Development Works Without CDN (Priority: P2)
|
||
|
||
**Goal**: Confirm that with no `S3_PUBLIC_BASE_URL` configured, `file_url` and `thumbnail_url` fall back to API proxy paths and images load correctly in local dev.
|
||
|
||
**Independent Test**: Run `make test-unit && make test-integration` with no `S3_PUBLIC_BASE_URL` set (the default). Confirm all tests pass and that `file_url` values in integration test responses begin with `/api/v1/images/`.
|
||
|
||
- [X] T010 [US2] Verify US2: run `make test-unit` and confirm the url-construction unit tests for the "no CDN base" case (T002 cases 3 and 4) pass; run `make test-integration` and confirm the updated upload tests (T005) pass — they already assert relative proxy paths since the test environment has no `S3_PUBLIC_BASE_URL`. Confirm `docker compose up` starts cleanly and images load in the browser via the proxy paths with no console errors.
|
||
|
||
**Checkpoint**: US2 verified. Local development requires no additional configuration.
|
||
|
||
---
|
||
|
||
## Phase 4: Polish & Cross-Cutting Concerns
|
||
|
||
- [X] T011 [P] Run `ruff check api/app/routers/images.py api/app/config.py` and fix any lint issues; run `ruff format --check` and format if needed.
|
||
|
||
- [X] T012 Run end-to-end verification per `specs/014-r2-cdn-serving/quickstart.md`: in production with `S3_PUBLIC_BASE_URL` set, call `GET /api/v1/images` and confirm `file_url` and `thumbnail_url` begin with `https://cdn.reactbin.juggalol.com/`; open the library page in a browser and confirm image requests in the network panel go to `cdn.reactbin.juggalol.com`, not `/api/`.
|
||
|
||
---
|
||
|
||
## Dependencies & Execution Order
|
||
|
||
- T001 must complete before any other task
|
||
- T002 before T003 (tests before implementation — unit test first)
|
||
- T003 before T004 (update helper before call sites)
|
||
- T004 before T005 (implementation before integration tests)
|
||
- T006, T007, T008 can run in parallel after T001 (different files)
|
||
- T009 after T006 (spec depends on updated interface)
|
||
- T010 after T003–T009 (verification requires full implementation)
|
||
- T011 after T003–T004 (lint the changed files)
|
||
- T012 last (manual end-to-end)
|
||
|
||
### Execution Order Summary
|
||
|
||
```
|
||
Step 1: T001 (foundational: config)
|
||
Step 2: T002 (US1: unit tests first)
|
||
Step 3: T003 (US1: implement _image_to_dict)
|
||
Step 4: T004 ∥ T006 ∥ T007 ∥ T008 (US1: call sites + UI in parallel)
|
||
Step 5: T005 ∥ T009 (US1: integration tests + service spec)
|
||
Step 6: T010 (US2: verify local dev fallback)
|
||
Step 7: T011 (polish: lint)
|
||
Step 8: T012 (polish: manual end-to-end)
|
||
```
|
||
|
||
---
|
||
|
||
## Implementation Strategy
|
||
|
||
### MVP (US1 only — CDN URLs in API + UI)
|
||
|
||
1. T001 — config
|
||
2. T002–T005 — API implementation and tests
|
||
3. T006–T009 — UI updates
|
||
4. **STOP and VALIDATE**: `make test-unit && make test-integration`, check browser network panel
|
||
|
||
### Incremental Delivery
|
||
|
||
1. T001–T005 (API only) → deploy → verify CDN URLs appear in API responses
|
||
2. T006–T009 (UI) → deploy → verify browser fetches images from CDN
|
||
3. T010 (local dev verification) → confirm fallback intact
|
||
4. T011–T012 (polish + end-to-end) → ship
|