diff --git a/CLAUDE.md b/CLAUDE.md index fc647b9..fee33f5 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -19,6 +19,7 @@ deployable. ## Commands ### cms (Go 1.25, module `thamanyah/cms/v2`) + ```bash cd cms go run . # serves on :8081 (requires DB_*/S3_*/MEDIACONVERT_* env vars — see below) @@ -26,6 +27,7 @@ go run . migrate # applies pending DB migrations, then exits (no HTTP go build ./... go vet ./... ``` + There are no test files in this repo (`cms`, `discovery`, or `infrastructure`) — don't assume a test suite exists. `cms` is a JSON API only — it serves no HTML and has no static assets. The @@ -47,12 +49,14 @@ misreports itself as v1.16.4; `go version -m $(go env GOPATH)/bin/swag` gives the real one.) ### infrastructure (Go, Pulumi, module `thamanyah`) + ```bash cd infrastructure pulumi preview # plan changes against stack "main" pulumi up # apply — this touches real AWS resources, confirm with the user first pulumi stack output # e.g. ecsClusterArn, cmsServiceArn ``` + Deploys run in CI (`.gitea/workflows/infrastructure-deploy.yml`) on push to `main` touching `infrastructure/**`. Treat local `pulumi up` as something to confirm with the user, not a routine dev command — it mutates shared cloud @@ -60,18 +64,47 @@ state and Pulumi state isn't safe to update concurrently with CI. ## Architecture +### cms package layout + +Four `internal` packages, split by the kind of thing they hold — keep new code +on the same seams: + +| package | holds | +|---|---| +| `internal/api` | the **wire contract**: request/response structs with their JSON + swaggo tags. No logic. | +| `internal/models` | the **domain types** the rest of the code passes around: `Video`, `Category`, `VideoStatus`. No JSON tags, no SQL. | +| `internal/handlers` | HTTP: decode `api.*`, validate, call repositories/services, encode `api.*`. | +| `internal/db` | the `*sql.DB` connection lifecycle and migrations; `internal/db/repositories` holds all SQL. | +| `internal/services` | **AWS only** — S3 and MediaConvert. Nothing database-related lives here. | + +`api` and `models` are deliberately separate types even where the fields look +alike: `api.CompleteResponse` is the published schema, `models.Video` is the +row. Handlers translate between them field by field, so renaming a `models` +field doesn't move the published API and vice versa. + ### cms service + Plain `net/http` (Go 1.22+ pattern-based `ServeMux`), no framework. Entry -point `cms/main.go` wires up AWS SDK v2 clients (S3, MediaConvert) and a -Postgres connection from env vars and assigns them to package-level interface -vars in `internal/services` (`services.S3Client`, `services.MediaConvertClient`, -`services.DB`) — handlers call through these interfaces, and -`S3Concrete`/`MediaConvertConcrete`/`DBConcrete` are the only implementations, -which is what makes the handlers testable even though no tests exist yet. On -boot, `S3Concrete.AssertSuccessfulConnection` proactively exercises -head/put/get/presign against the bucket, and `DBConcrete.AssertSuccessfulConnection` -pings Postgres — both panic on failure rather than letting the service come -up in a broken state. +point `cms/main.go` builds the dependencies and assigns them to package-level +interface vars that the handlers call through: + +- `services.S3Client` (`services.S3`) ← `*services.S3Concrete` +- `services.MediaConvertClient` (`services.MediaConvert`) ← `*services.MediaConvertConcrete` +- `repositories.VideoRepo` (`repositories.VideoRepository`) ← `*repositories.ConcreteVideoRepository` +- `repositories.CatagoriesRepo` (`repositories.CatagoriesRepository`) ← `*repositories.ConcreteCatagoriesRepository` + +Each interface has exactly one implementation; the indirection is what makes +the handlers substitutable in tests, even though no tests exist yet. Note the +spelling: the categories repository is `Catagories`/`CatagoriesRepo` in +`repositories/catagories.go` (and `handlers.validateCatagoryIDS`) — the +misspelling is load-bearing for compilation, so match it rather than "fixing" +it piecemeal. + +On boot, `S3Concrete.AssertSuccessfulConnection` proactively exercises +head-bucket/put/get/presign against the bucket (writing a throwaway +`.s3-connectivity-check` object), and `db.AssertSuccessfulConnection` pings +Postgres — both panic on failure rather than letting the service come up in a +broken state. `cms/main.go` has two entry paths, dispatched on `os.Args[1]`: the default path (`runServer`) boots the HTTP server; `./cms migrate` (`runMigrate`) @@ -82,6 +115,11 @@ S3/MediaConvert or start the server. This is run as its own ECS container before the main container starts (see infrastructure below), so `runServer` never runs migrations itself, only `AssertSuccessfulConnection`. +The DB connection is opened once for the process lifetime by +`db.CreateDBConnection(connString)` and closed by `db.CloseConnection` — both +plain functions over `*sql.DB` in `internal/db/client.go`, not methods on a +wrapper type. Repositories take that `*sql.DB` as their `SQLDB` field. + Routes (`cms/main.go`): `GET /health`, `GET /api/categories`, `POST /api/videos/presign`, `POST /api/videos`, plus Swagger UI at `GET /swagger/` (`/swagger/doc.json` serves the spec). The UI assets are @@ -90,12 +128,13 @@ nothing is fetched from a CDN at runtime. Handlers live in `cms/internal/handlers` — `handlers.go` holds `Health` and the shared response writers, `videos.go` the categories and upload endpoints. -The request/response bodies they serve are *not* in that package: every wire -struct lives in `cms/internal/api` (`api.go` for `HealthResponse` and -`ProblemDetails`, `videos.go` for the rest), exported and referenced by the -handlers' swaggo annotations as `api.VideoResponse` and so on — so the -generated spec's definition names track that package, and renaming a type -there changes the published schema names. +The wire structs they serve live in `cms/internal/api` (`api.go` for +`HealthResponse` and `ProblemDetails`, `videos.go` for `Category`, +`CategoriesResponse`, `PresignRequest`/`PresignResponse` and +`CompleteRequest`/`CompleteResponse`), referenced by the handlers' swaggo +annotations as `api.CompleteResponse` and so on — so the generated spec's +definition names track that package, and renaming a type there changes the +published schema names. There is no view layer: the `internal/views` templ package, the `static/` directory, and the htmx frontend were all removed when the service became a @@ -113,48 +152,55 @@ just the HTTP status phrase — meaningful titles like these are supposed to carry a real `type` URI. Adding per-problem type URIs is the conforming fix if it ever matters. -### Data model (Postgres, `cms/internal/db/migrations/`) +### Data model (Postgres, `cms/internal/db/migrations/`, two migrations: `0001`, `0002`) + - `videos` — one row per uploaded video: `title`, `description`, `tags` (free-text, comma-separated — not normalized), `file_name`, `storage_key` (the S3 key, unique), `mediaconvert_job_id`, `status` (written once as `"processing"` on insert — see Known gaps), `size_bytes`, timestamps. - `status` is an enum on both sides: the `services.VideoStatus` string type - in `cms/internal/services/status.go` (`processing`, `ready`, `failed`) and - the `videos_status_check` CHECK constraint added by migration `0003`. - Extending it means adding a constant there, widening the constraint in a - new migration, and extending the `enums` annotation on - `api.VideoResponse.Status` before regenerating the spec. - `id` is a `UUID` generated application-side as a UUIDv7 - (`uuid.NewV7()` in `services.DBConcrete.CreateVideo`) rather than via the - column's own `DEFAULT gen_random_uuid()` (which generates v4 and is never - actually relied on) — v7 keeps primary-key inserts roughly time-ordered, - avoiding B-tree fragmentation as the table grows. + `id` is a `UUID` filled by the column's own `DEFAULT gen_random_uuid()` + (v4) and read back through `RETURNING id` — the application does not + generate it. (An earlier revision generated UUIDv7 application-side; that + was reverted in `785154c`, so primary keys are random, not time-ordered.) + `status` is a plain `TEXT NOT NULL DEFAULT 'processing'` column with **no + CHECK constraint** — the closed set exists only in Go, as the + `models.VideoStatus` string type in `cms/internal/models/video.go` + (`processing`, `ready`, `failed`). Extending it means adding a constant + there and extending the `enums` annotation on `api.CompleteResponse.Status` + before regenerating the spec. (The doc comment on `VideoStatus` claims a + `videos_status_check` constraint enforces it in the database — that + constraint does not exist; nothing has ever created it.) - `categories` — a small fixed lookup table (`documentary`, `news`, `entertainment`, `podcast`, `other`), seeded by migration `0001`. Its `SMALLSERIAL` ids are part of the public API: `GET /api/categories` returns - `{id, name}` pairs and `POST /api/videos` takes `categoryIds`, so the seed - order in migration `0001` is what fixes which id means which name — never - renumber it. + `{id, name}` pairs (ordered by name, not id) and `POST /api/videos` takes + `categoryIds`, so the seed order in migration `0001` is what fixes which id + means which name — never renumber it. - `video_categories` — join table (`video_id`, `category_id`, composite PK, `ON DELETE CASCADE`) added in migration `0002`: a video can belong to - *multiple* categories, not just one. `CreateVideo` inserts the `videos` row - and its `video_categories` links inside a single transaction, taking the - category ids as given — `handlers.resolveCategoryIDs` is what checks them - against `ListCategories` (422 on an unknown id) and drops duplicates before - the transcode job is queued, leaving the foreign key as a backstop. + *multiple* categories, not just one. `ConcreteVideoRepository.CreateVideo` + inserts the `videos` row and its `video_categories` links inside a single + transaction, taking the category ids as given — `handlers.validateCatagoryIDS` + is what checks them against `CatagoriesRepo.ListCategoriesIDs` before the + transcode job is queued, leaving the foreign key and the composite PK as + backstops (see Known gaps for how those failures surface). ### Video upload → transcode pipeline + 1. Client calls `POST /api/videos/presign` → cms returns a presigned S3 `PUT` - URL for `raw-uploads-bucket`, key `videos/.`. + URL for `raw-uploads-bucket`, key `videos/.`. The + submitted `contentType` (only `video/mp4` or `video/quicktime`) is signed + into the URL, so the client's `PUT` must send the identical header. 2. Client `PUT`s the file directly to S3 (from a browser this requires the - bucket's CORS rule, set up in `infrastructure/main.go` — see Known gaps, + bucket's CORS rule, set up in `infrastructure/main.go`, its allowed origin is now stale). The file never passes through cms. 3. Client calls `POST /api/videos` with the metadata (categories given as - `categoryIds` from `GET /api/categories`) + key → cms calls - `MediaConvertClient.QueueEncodingJob(key)`, submitting a MediaConvert job - `s3://raw-uploads-bucket/` → `s3://encoded-bucket/` (H.264/AAC → - MP4, QVBR rate control — QVBR requires `MaxBitrate` to be set explicitly), - then `services.DB.CreateVideo` persists the `videos` row (status + `categoryIds` from `GET /api/categories`) + key → cms validates the + category ids, calls `MediaConvertClient.QueueEncodingJob(key)`, submitting + a MediaConvert job `s3://raw-uploads-bucket/` → + `s3://encoded-bucket/` (H.264/AAC → MP4, QVBR rate control — QVBR + requires `MaxBitrate` to be set explicitly), then + `repositories.VideoRepo.CreateVideo` persists the `videos` row (status `"processing"`) and its `video_categories` links. 4. Finished output lands in `encoded-bucket`, served via CloudFront. @@ -166,6 +212,7 @@ vars in `deployFargateService`, `infrastructure/main.go`) — `main.go` panics on boot if any required var is empty. ### infrastructure (`infrastructure/main.go`, single Pulumi Go program, region `us-east-1`, stack `main`) + - **Postgres**: one shared RDS instance (`db.t3.micro`, single-AZ, no backups — intentionally minimal). Each app (`cms`, `discovery`) gets its own login role and same-named database via the `postgresql` provider @@ -218,6 +265,7 @@ on boot if any required var is empty. Never broaden this to `ecr:*`/`ecs:*`. ### CI/CD (`.gitea/workflows/`) + - **`cms-deploy.yml`**: push to `main` touching `cms/**`. Builds/pushes the Docker image to ECR using `gitea-ci-user`, installs the AWS CLI (not preinstalled on the runner image — via AWS's official install script, not @@ -233,32 +281,3 @@ on boot if any required var is empty. - The runner's `ubuntu-latest` label maps to a docker image configured on the runner host (outside this repo) — currently minimal, lacking the AWS CLI, hence the manual install step in `cms-deploy.yml`. - -## Known gaps - -- `cms/internal/services/mediaconvert.go` never calls `DescribeEndpoints` - and configures no custom MediaConvert endpoint — relies on the SDK's - default regional endpoint. -- No MediaConvert completion webhook or poller exists — a `videos.status` - row is written once as `VideoStatusProcessing` in `CreateVideo` and never - updated, even after the transcode job actually finishes or fails. The - `ready` and `failed` states are defined but nothing sets them yet. -- `discovery` has infra provisioned (ECR repo, ECS service, ALB, Postgres - DB/role) but no application code — it doesn't touch its database at all. -- No automated tests exist for `cms`, `discovery`, or `infrastructure`. -- **No browser client can reach the API yet.** Two separate CORS gaps, both - left over from cms dropping its own UI: cms sends no CORS headers of its - own, and `raw-uploads-bucket`'s CORS rule in `infrastructure/main.go` still - allows only the `cms` ALB origin, so a frontend served from anywhere else - fails preflight on the direct S3 `PUT`. Both need the real frontend origin - before a browser client works end to end. -- `videos.size_bytes` is hardcoded to `60` in `CompleteVideoUpload` — the - request body carries no size field, so every row stores 60 and the API - hands that back in `sizeBytes`. Fixing it means adding `sizeBytes` to the - `POST /api/videos` request contract. -- The generated spec is Swagger 2.0, which has no per-response media type, so - error responses are documented as `application/json` even though they are - actually sent as `application/problem+json`. The schemas themselves are - right. `swag init --v3.1` would emit OpenAPI 3.1 and resolve it. -- `cms/uploads/` holds a stray `.mov` from an older local-disk upload path. - It is untracked, unreferenced by any code, and safe to delete.