reactbin/specs/002-api-image-proxy/plan.md

# Implementation Plan: API Image Proxy

**Branch**: `002-api-image-proxy` | **Date**: 2026-05-03 | **Spec**: [spec.md](spec.md)
**Input**: Feature specification from `specs/002-api-image-proxy/spec.md`

## Summary

Replace the presigned-URL redirect in `GET /api/v1/images/{id}/file` with a direct
proxy that fetches the image from object storage server-side and streams the bytes
back to the client. The browser never contacts the storage backend. This eliminates
the `/etc/hosts` workaround required in local development and keeps the storage
layer fully private.

Changes touch three files in the API and the integration test suite. The UI
requires no changes because the endpoint path is unchanged.

## Technical Context

**Language/Version**: Python 3.12+ (API); TypeScript strict mode (UI — no changes)
**Primary Dependencies**: FastAPI, aiobotocore (S3 retrieval), pytest-asyncio (tests)
**Storage**: S3-compatible object storage (MinIO locally, real S3 in production) via `StorageBackend` interface
**Testing**: pytest + pytest-asyncio; existing integration test fixture reused
**Target Platform**: Linux server (containerised); modern evergreen desktop browsers
**Project Type**: Web application — FastAPI API + Angular SPA
**Performance Goals**: Image load time via proxy within 20% of direct storage access on local network
**Constraints**: Max 50 MB per image; no storage details exposed in any error response
**Scale/Scope**: Single-user personal application; no concurrency or throughput targets for v1

## Constitution Check

*GATE: Must pass before Phase 0 research. Re-checked after Phase 1 design below.*

| Principle | Check | Status |
|-----------|-------|--------|
| §2.1 Separation of concerns | API proxies bytes; UI is unchanged; storage detail never reaches the browser | ✅ |
| §2.2 Dependency direction | UI → API → Storage; no new upward imports introduced | ✅ |
| §2.3 Storage abstraction | Retrieval goes through the `StorageBackend.get()` method; no raw S3 calls in the router | ✅ |
| §2.4 Auth abstraction | No change to auth flow | ✅ |
| §2.5 DB abstraction | No query logic moves outside repositories | ✅ |
| §2.6 No speculative abstraction | `get_presigned_url` removed (no callers); `get()` added (has one caller) | ✅ |
| §3.1 API versioning | Endpoint path `/api/v1/images/{id}/file` is unchanged | ✅ |
| §3.3 Error shape | Storage errors return `{"detail": "...", "code": "storage_error"}` | ✅ |
| §5.1 TDD non-negotiable | Failing tests written before implementation in every task | ✅ |
| §5.2 Test pyramid | Existing integration test updated; no unit test needed (thin router delegation) | ✅ |
| §5.4 CI gate | All tests + ruff must pass before milestone is done | ✅ |

**Post-design re-check**: Contracts and data model confirmed above; all gates still pass.

## Project Structure

### Documentation (this feature)

```text
specs/002-api-image-proxy/
├── plan.md              # This file
├── spec.md              # Feature specification
├── research.md          # Phase 0 decisions
├── data-model.md        # Interface change summary
├── contracts/
│   └── api.md           # Updated endpoint contract
├── checklists/
│   └── requirements.md  # Spec quality checklist
└── tasks.md             # Phase 2 output (/speckit-tasks — NOT created here)
```

### Files changed

```text
api/
├── app/
│   ├── storage/
│   │   ├── backend.py       # Remove get_presigned_url; add get(key) -> bytes
│   │   └── s3_backend.py    # Remove get_presigned_url impl; add get(key) -> bytes
│   └── routers/
│       └── images.py        # serve_image_file: RedirectResponse → Response with bytes + cache headers
└── tests/
    └── integration/
        └── test_serving.py  # T055: update assertion from 302 to 200 with content-type + ETag
```

No UI files change. `ImageService.getFileUrl()` already returns `/api/v1/images/{id}/file`.

## Milestones

> **TDD ORDER IS MANDATORY** (constitution §5.1): Write the failing test first,
> confirm it fails, then implement until it passes.

### M1 — Update StorageBackend interface and S3 implementation

**Goal**: The storage layer can retrieve object bytes; presigned-URL generation is removed.

**Tasks** (TDD order):

1. In `api/app/storage/backend.py`:
   - Remove the `get_presigned_url` abstract method
   - Add `async def get(self, key: str) -> bytes` as a new abstract method

2. In `api/app/storage/s3_backend.py`:
   - Remove the `get_presigned_url` implementation
   - Add `async def get(self, key: str) -> bytes`:
     ```
     async with self._client() as client:
         response = await client.get_object(
             Bucket=self._settings.s3_bucket_name,
             Key=key,
         )
         return await response["Body"].read()
     ```

3. Verify: `ruff check api/` passes; `mypy` or pyright (if configured) passes.

**Done criterion**: Interface and implementation compile cleanly; no existing tests break.

---

### M2 — Update the integration test (failing first)

**Goal**: T055 asserts the new contract (200 + bytes) before any router change.

**Tasks** (TDD order):

1. In `api/tests/integration/test_serving.py`, update `test_file_redirect_returns_302`:
   - Rename to `test_file_returns_200_with_content`
   - Change assertion from `status_code == 302` and `Location` header to:
     - `response.status_code == 200`
     - `response.headers["content-type"]` starts with `"image/"`
     - `response.headers["etag"]` is present and equals `f'"{image.hash}"'`
     - `response.headers["cache-control"]` contains `"immutable"`
     - `len(response.content) > 0`
   - Remove `follow_redirects=False` from the client call

2. Run `pytest api/tests/integration/test_serving.py` — confirm T055 **fails** (still 302).

**Done criterion**: Test file updated; new test fails with the current implementation.

---

### M3 — Update the router

**Goal**: `serve_image_file` proxies bytes from storage instead of redirecting.

**Tasks** (TDD order — tests already written in M2):

1. In `api/app/routers/images.py`:
   - Remove `from fastapi.responses import RedirectResponse` import
   - Add `from fastapi.responses import Response` (already present as `Response` from fastapi)
   - Update `serve_image_file`:

     ```python
     @router.get("/images/{image_id}/file")
     async def serve_image_file(
         image_id: uuid.UUID,
         db: AsyncSession = Depends(get_db),
         storage: StorageBackend = Depends(get_storage),
     ):
         image_repo = ImageRepository(db)
         image = await image_repo.get_by_id(image_id)
         if not image:
             raise HTTPException(
                 status_code=404,
                 detail={"detail": "Image not found", "code": "image_not_found"},
             )
         try:
             data = await storage.get(image.storage_key)
         except Exception:
             raise HTTPException(
                 status_code=500,
                 detail={"detail": "Failed to retrieve image content", "code": "storage_error"},
             )
         return Response(
             content=data,
             media_type=image.mime_type,
             headers={
                 "ETag": f'"{image.hash}"',
                 "Cache-Control": "public, max-age=31536000, immutable",
             },
         )
     ```

2. Run `pytest api/tests/integration/test_serving.py` — confirm all tests pass.
3. Run full test suite: `pytest api/` — confirm no regressions.
4. Run `ruff check api/ && ruff format --check api/`.

**Done criterion**: All 45+ tests pass; linter clean; the presigned-URL redirect is gone.

## Post-design Constitution Re-check

| Principle | Verdict |
|-----------|---------|
| §2.3 Storage abstraction | `get()` added to interface; router only calls `storage.get()` | ✅ |
| §2.6 No speculative abstraction | `get_presigned_url` removed with zero callers remaining | ✅ |
| §3.3 Error shape | `storage_error` code used for retrieval failures | ✅ |
| §5.1 TDD | Test updated to fail before router change | ✅ |

All gates pass. Feature is ready for `/speckit-tasks`.