From db50e015bf16b908417c98f958850c6990442d60 Mon Sep 17 00:00:00 2001 From: FahdShalhoub Date: Sun, 30 Aug 2026 10:56:51 +0300 Subject: [PATCH] FEAT: Swicthed QUERY Method To GET --- CLAUDE.md | 42 +-- discovery/docs/docs.go | 83 +++++- discovery/docs/swagger.json | 83 +++++- discovery/docs/swagger.yaml | 72 +++++- discovery/internal/api/videos.go | 24 +- discovery/internal/handlers/videos.go | 126 ++++++--- discovery/main.go | 10 +- tests/README.md | 2 +- tests/client_test.go | 53 ++-- tests/steps_test.go | 2 +- upload-video-demo.sh | 357 ++++++++++++++++++++++++++ 11 files changed, 760 insertions(+), 94 deletions(-) create mode 100755 upload-video-demo.sh diff --git a/CLAUDE.md b/CLAUDE.md index 265ad9a..d7cc525 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -18,7 +18,7 @@ draining `discovery-catalogue-events`, the queue subscribed to the catalogue topic `cms` announces ready videos on, and keeps its own copy in its own database. Neither service reads the other's tables and nothing polls `cms`. -It serves `GET /health`, `GET /api/videos/{id}`, `QUERY /api/videos` (the +It serves `GET /health`, `GET /api/videos/{id}`, `GET /api/videos` (the catalogue search — see below) and the Swagger UI. Migration `0001_init` still creates nothing (it predates the domain and exists only because `internal/db/migrate.go` embeds `migrations/*.sql`, which will not compile @@ -87,24 +87,34 @@ not find "documentary". It is named explicitly in the column *and* in every query — a query built under a different configuration stems its terms differently and silently matches nothing. -### Catalogue search (`QUERY /api/videos`) +### Catalogue search (`GET /api/videos`) -Served on **QUERY**, not GET or POST: a search is safe and idempotent (which -POST is not), but its parameters are a structured document that does not belong -in a query string. Go's `ServeMux` routes the method fine. Two consequences to -know before touching it: +Served on **GET**, not POST: a search is safe and idempotent, which POST is not, +and its parameters fit a query string. It shares the path with +`GET /api/videos/{id}`; `ServeMux` keeps them apart, the more specific pattern +winning. Two things to know before touching it: -- It **cannot appear in the OpenAPI spec**. Swagger 2.0 and OpenAPI 3.x define - `PathItem` with a fixed field per method and there is no slot for QUERY; - `swag` rejects the annotation outright with `invalid method: QUERY`. So the - handler carries **no `@Router` annotation** and the endpoint is described in - prose in `main.go`'s `@description` instead. Adding a `@Router … [query]` - line breaks `swag init` for the whole service. -- QUERY is still an IETF draft (`draft-ietf-httpbis-safe-method-w-body`), so - expect intermediaries that have never heard of it, and no HTTP caching of the - kind a `GET` with a query string would get. +- This endpoint was on **QUERY** until the method was changed to GET. The + parameters are now a query string, not a JSON body, so there is no + `api.SearchRequest` any more — `handlers.parseSearchQuery` reads + `r.URL.Query()` into an unexported `searchQuery`. Anything still sending a + JSON body gets the whole catalogue, since every parameter is optional and it + supplied none of them. +- It **does** appear in the OpenAPI spec now, as `@Router /api/videos [get]` + with one `@Param … query` per parameter (`categories` carries + `collectionFormat(multi)`). That is the reason the change is worth anything + beyond method purity: QUERY had no `PathItem` slot in Swagger 2.0 or + OpenAPI 3.x, so `swag` rejected the annotation with `invalid method: QUERY` + and the endpoint had to be described in prose instead. -Request `{title, categories, limit, cursor}`, response `{videos, nextCursor}`. +`categories` is **repeated**, not comma-separated +(`?categories=news&categories=documentary`), so a category name containing a +comma stays one name; an empty value is dropped rather than treated as a +category named `""`. `limit` is the only parameter that can be malformed — a +non-numeric one is a 400 `Malformed Search`; zero or negative reads as "no +preference" and takes the default, as omitting it does. + +Response `{videos, nextCursor}`. `title` is matched lexically, ANDing the words with the **last one as a prefix** (`desert & fal:*`) so a part-typed word still matches; `categories` narrows to videos filed under **any** of the names (`&&`, not `@>`); `limit` defaults to 20 diff --git a/discovery/docs/docs.go b/discovery/docs/docs.go index 654da53..3b674cd 100644 --- a/discovery/docs/docs.go +++ b/discovery/docs/docs.go @@ -15,6 +15,71 @@ const docTemplate = `{ "host": "{{.Host}}", "basePath": "{{.BasePath}}", "paths": { + "/api/videos": { + "get": { + "description": "Finds catalogued videos by the words of their title and the categories they are filed under, most relevant first. Every parameter is optional — a request with none of them browses the whole catalogue, newest first. Paging is by cursor rather than by offset, so a page stays the same page while videos are being announced into the catalogue around it.", + "produces": [ + "application/json" + ], + "tags": [ + "videos" + ], + "summary": "Search the catalogue", + "parameters": [ + { + "type": "string", + "example": "desert fal", + "description": "Matched lexically against video titles: the words are all required, and the last is treated as a prefix so a part-typed word still finds something.", + "name": "title", + "in": "query" + }, + { + "type": "array", + "items": { + "type": "string" + }, + "collectionFormat": "multi", + "example": "documentary", + "description": "Narrows the search to videos filed under any one of these names — not all of them. Repeat the parameter per category. Omitted means every category.", + "name": "categories", + "in": "query" + }, + { + "type": "integer", + "default": 20, + "description": "How many videos to return. Absent, zero or negative takes the default of 20; anything above the maximum of 100 is capped to it rather than refused, and the cursor still reaches the rest.", + "name": "limit", + "in": "query" + }, + { + "type": "string", + "description": "Asks for the page after the one a previous search ended at. Send back the nextCursor from that search unchanged; it is opaque, and the only thing to do with it is return it.", + "name": "cursor", + "in": "query" + } + ], + "responses": { + "200": { + "description": "OK", + "schema": { + "$ref": "#/definitions/api.SearchResults" + } + }, + "400": { + "description": "Bad Request", + "schema": { + "$ref": "#/definitions/api.ProblemDetails" + } + }, + "500": { + "description": "Internal Server Error", + "schema": { + "$ref": "#/definitions/api.ProblemDetails" + } + } + } + } + }, "/api/videos/{id}": { "get": { "description": "Returns the catalogue's copy of a video: what a reader needs to show and play it. A video appears here only after the CMS has announced it as ready, so a video that is still transcoding — or one the CMS never made ready — is a 404.", @@ -108,6 +173,22 @@ const docTemplate = `{ } } }, + "api.SearchResults": { + "type": "object", + "properties": { + "nextCursor": { + "description": "NextCursor reaches the page after this one. Empty on the last page,\nwhich is how a reader knows there is no more to ask for.", + "type": "string" + }, + "videos": { + "description": "Videos are the matches, most relevant first. Never null: a search that\nmatches nothing is an empty list.", + "type": "array", + "items": { + "$ref": "#/definitions/api.Video" + } + } + } + }, "api.Video": { "type": "object", "properties": { @@ -145,7 +226,7 @@ var SwaggerInfo = &swag.Spec{ BasePath: "/", Schemes: []string{}, Title: "Thamanyah Discovery API", - Description: "JSON API for browsing the Thamanyah catalogue. Read-side counterpart to the CMS, which is what ingests videos.\n\nNot listed below: `QUERY /api/videos`, the catalogue search. It takes a JSON body `{title, categories, limit, cursor}` and answers `{videos, nextCursor}` — title is matched lexically with the last word as a prefix, categories narrow to videos filed under any one of them, limit defaults to 20 and is capped at 100, and cursor is the opaque `nextCursor` of a previous search. It is absent here because OpenAPI has no slot for the QUERY method, not because it is unsupported; see handlers.SearchVideos.", + Description: "JSON API for browsing the Thamanyah catalogue. Read-side counterpart to the CMS, which is what ingests videos.\n\nThe catalogue search is `GET /api/videos`, listed below: title is matched lexically with the last word as a prefix, categories narrow to videos filed under any one of them, limit defaults to 20 and is capped at 100, and cursor is the opaque `nextCursor` of a previous search.", InfoInstanceName: "swagger", SwaggerTemplate: docTemplate, LeftDelim: "{{", diff --git a/discovery/docs/swagger.json b/discovery/docs/swagger.json index 070b554..7870893 100644 --- a/discovery/docs/swagger.json +++ b/discovery/docs/swagger.json @@ -1,13 +1,78 @@ { "swagger": "2.0", "info": { - "description": "JSON API for browsing the Thamanyah catalogue. Read-side counterpart to the CMS, which is what ingests videos.\n\nNot listed below: `QUERY /api/videos`, the catalogue search. It takes a JSON body `{title, categories, limit, cursor}` and answers `{videos, nextCursor}` — title is matched lexically with the last word as a prefix, categories narrow to videos filed under any one of them, limit defaults to 20 and is capped at 100, and cursor is the opaque `nextCursor` of a previous search. It is absent here because OpenAPI has no slot for the QUERY method, not because it is unsupported; see handlers.SearchVideos.", + "description": "JSON API for browsing the Thamanyah catalogue. Read-side counterpart to the CMS, which is what ingests videos.\n\nThe catalogue search is `GET /api/videos`, listed below: title is matched lexically with the last word as a prefix, categories narrow to videos filed under any one of them, limit defaults to 20 and is capped at 100, and cursor is the opaque `nextCursor` of a previous search.", "title": "Thamanyah Discovery API", "contact": {}, "version": "1.0" }, "basePath": "/", "paths": { + "/api/videos": { + "get": { + "description": "Finds catalogued videos by the words of their title and the categories they are filed under, most relevant first. Every parameter is optional — a request with none of them browses the whole catalogue, newest first. Paging is by cursor rather than by offset, so a page stays the same page while videos are being announced into the catalogue around it.", + "produces": [ + "application/json" + ], + "tags": [ + "videos" + ], + "summary": "Search the catalogue", + "parameters": [ + { + "type": "string", + "example": "desert fal", + "description": "Matched lexically against video titles: the words are all required, and the last is treated as a prefix so a part-typed word still finds something.", + "name": "title", + "in": "query" + }, + { + "type": "array", + "items": { + "type": "string" + }, + "collectionFormat": "multi", + "example": "documentary", + "description": "Narrows the search to videos filed under any one of these names — not all of them. Repeat the parameter per category. Omitted means every category.", + "name": "categories", + "in": "query" + }, + { + "type": "integer", + "default": 20, + "description": "How many videos to return. Absent, zero or negative takes the default of 20; anything above the maximum of 100 is capped to it rather than refused, and the cursor still reaches the rest.", + "name": "limit", + "in": "query" + }, + { + "type": "string", + "description": "Asks for the page after the one a previous search ended at. Send back the nextCursor from that search unchanged; it is opaque, and the only thing to do with it is return it.", + "name": "cursor", + "in": "query" + } + ], + "responses": { + "200": { + "description": "OK", + "schema": { + "$ref": "#/definitions/api.SearchResults" + } + }, + "400": { + "description": "Bad Request", + "schema": { + "$ref": "#/definitions/api.ProblemDetails" + } + }, + "500": { + "description": "Internal Server Error", + "schema": { + "$ref": "#/definitions/api.ProblemDetails" + } + } + } + } + }, "/api/videos/{id}": { "get": { "description": "Returns the catalogue's copy of a video: what a reader needs to show and play it. A video appears here only after the CMS has announced it as ready, so a video that is still transcoding — or one the CMS never made ready — is a 404.", @@ -101,6 +166,22 @@ } } }, + "api.SearchResults": { + "type": "object", + "properties": { + "nextCursor": { + "description": "NextCursor reaches the page after this one. Empty on the last page,\nwhich is how a reader knows there is no more to ask for.", + "type": "string" + }, + "videos": { + "description": "Videos are the matches, most relevant first. Never null: a search that\nmatches nothing is an empty list.", + "type": "array", + "items": { + "$ref": "#/definitions/api.Video" + } + } + } + }, "api.Video": { "type": "object", "properties": { diff --git a/discovery/docs/swagger.yaml b/discovery/docs/swagger.yaml index 3d3dfb0..bd4312a 100644 --- a/discovery/docs/swagger.yaml +++ b/discovery/docs/swagger.yaml @@ -22,6 +22,21 @@ definitions: example: about:blank type: string type: object + api.SearchResults: + properties: + nextCursor: + description: |- + NextCursor reaches the page after this one. Empty on the last page, + which is how a reader knows there is no more to ask for. + type: string + videos: + description: |- + Videos are the matches, most relevant first. Never null: a search that + matches nothing is an empty list. + items: + $ref: '#/definitions/api.Video' + type: array + type: object api.Video: properties: categories: @@ -46,10 +61,65 @@ info: description: |- JSON API for browsing the Thamanyah catalogue. Read-side counterpart to the CMS, which is what ingests videos. - Not listed below: `QUERY /api/videos`, the catalogue search. It takes a JSON body `{title, categories, limit, cursor}` and answers `{videos, nextCursor}` — title is matched lexically with the last word as a prefix, categories narrow to videos filed under any one of them, limit defaults to 20 and is capped at 100, and cursor is the opaque `nextCursor` of a previous search. It is absent here because OpenAPI has no slot for the QUERY method, not because it is unsupported; see handlers.SearchVideos. + The catalogue search is `GET /api/videos`, listed below: title is matched lexically with the last word as a prefix, categories narrow to videos filed under any one of them, limit defaults to 20 and is capped at 100, and cursor is the opaque `nextCursor` of a previous search. title: Thamanyah Discovery API version: "1.0" paths: + /api/videos: + get: + description: Finds catalogued videos by the words of their title and the categories + they are filed under, most relevant first. Every parameter is optional — a + request with none of them browses the whole catalogue, newest first. Paging + is by cursor rather than by offset, so a page stays the same page while videos + are being announced into the catalogue around it. + parameters: + - description: 'Matched lexically against video titles: the words are all required, + and the last is treated as a prefix so a part-typed word still finds something.' + example: desert fal + in: query + name: title + type: string + - collectionFormat: multi + description: Narrows the search to videos filed under any one of these names + — not all of them. Repeat the parameter per category. Omitted means every + category. + example: documentary + in: query + items: + type: string + name: categories + type: array + - default: 20 + description: How many videos to return. Absent, zero or negative takes the + default of 20; anything above the maximum of 100 is capped to it rather + than refused, and the cursor still reaches the rest. + in: query + name: limit + type: integer + - description: Asks for the page after the one a previous search ended at. Send + back the nextCursor from that search unchanged; it is opaque, and the only + thing to do with it is return it. + in: query + name: cursor + type: string + produces: + - application/json + responses: + "200": + description: OK + schema: + $ref: '#/definitions/api.SearchResults' + "400": + description: Bad Request + schema: + $ref: '#/definitions/api.ProblemDetails' + "500": + description: Internal Server Error + schema: + $ref: '#/definitions/api.ProblemDetails' + summary: Search the catalogue + tags: + - videos /api/videos/{id}: get: description: 'Returns the catalogue''s copy of a video: what a reader needs diff --git a/discovery/internal/api/videos.go b/discovery/internal/api/videos.go index 3b29fea..85d19c0 100644 --- a/discovery/internal/api/videos.go +++ b/discovery/internal/api/videos.go @@ -12,26 +12,10 @@ type Video struct { Categories []string `json:"categories" example:"documentary,news"` } -// SearchRequest is the body of QUERY /api/videos: what a reader is looking -// for. Every member is optional — an empty body browses the whole catalogue. -type SearchRequest struct { - // Title is matched lexically against video titles: the words are all - // required, and the last is treated as a prefix so a part-typed word still - // finds something. - Title string `json:"title" example:"desert fal"` - // Categories narrows the search to videos filed under any one of these - // names — not all of them, so it widens the choice of category while - // narrowing the results. Empty means every category. - Categories []string `json:"categories" example:"documentary,news"` - // Limit is how many videos to return. Absent or zero takes the default of - // 20; anything above the maximum of 100 is capped to it rather than - // refused, and the cursor still reaches the rest. - Limit int `json:"limit" example:"20"` - // Cursor asks for the page after the one a previous search ended at. Send - // back the nextCursor from that search, unchanged; it is opaque, and the - // only thing to do with it is return it. - Cursor string `json:"cursor"` -} +// The search itself takes no body type: GET /api/videos reads its parameters +// — title, categories, limit and cursor — from the query string, so there is +// no request document to publish a schema for. They are documented as +// parameters on handlers.SearchVideos, and appear in the generated spec there. // SearchResults is one page of search results. type SearchResults struct { diff --git a/discovery/internal/handlers/videos.go b/discovery/internal/handlers/videos.go index 26fd6d8..0845abe 100644 --- a/discovery/internal/handlers/videos.go +++ b/discovery/internal/handlers/videos.go @@ -4,9 +4,12 @@ import ( "context" "encoding/json" "errors" + "fmt" "log" "net/http" + "net/url" "slices" + "strconv" "strings" "thamanyah/discovery/internal/api" "thamanyah/discovery/internal/db/repositories" @@ -111,9 +114,9 @@ func searchCacheKey(title string, categories []string, limit int, cursor string) // readers naming the same categories in a different order now share one // entry instead of building the same page twice. // - // Never nil, so that a client sending "categories": [] and one omitting - // the member entirely — the same search, since both mean "every category" - // — land on one entry rather than on `null` and `[]`. + // Never nil, so that a client sending no categories at all and one whose + // categories parsed away to none — the same search, since both mean "every + // category" — land on one entry rather than on `null` and `[]`. sorted := append([]string{}, categories...) slices.Sort(sorted) sorted = slices.Compact(sorted) @@ -175,55 +178,110 @@ func cacheSearch(ctx context.Context, key string, results api.SearchResults) { } } +// searchQuery is a search as the query string spells it out. Every parameter +// is optional: a request carrying none of them browses the whole catalogue, +// newest first. +type searchQuery struct { + title string + categories []string + limit int + cursor string +} + +// parseSearchQuery reads the search out of the URL, applying the default and +// the cap to the page size so that the rest of the handler works with a limit +// it can use as given. +// +// The only parameter that can be malformed is the limit: every other one is a +// string the search takes as written, so there is nothing to spell wrong. +func parseSearchQuery(values url.Values) (searchQuery, error) { + parsed := searchQuery{ + title: values.Get("title"), + // Repeated — ?categories=news&categories=documentary — rather than one + // comma-separated value, for the reason searchCacheKey encodes them as + // JSON rather than joining them: a category name is a name, and one + // containing a comma must not read as two. + // + // Empty values are dropped so that a client sending ?categories= means + // "every category", as omitting it does, rather than narrowing the + // search to a category named "". + categories: nonEmpty(values["categories"]), + cursor: values.Get("cursor"), + limit: searchPageSize, + } + + if raw := values.Get("limit"); raw != "" { + limit, err := strconv.Atoi(raw) + if err != nil { + return searchQuery{}, fmt.Errorf("limit %q is not a whole number", raw) + } + // Zero and negative are read as "no preference", the same as omitting + // the parameter. A page of nothing is not what anyone meant by it, and + // the request is still answerable, so answering it beats refusing it. + if limit > 0 { + parsed.limit = limit + } + } + + if parsed.limit > maxSearchPageSize { + parsed.limit = maxSearchPageSize + } + + return parsed, nil +} + +func nonEmpty(values []string) []string { + kept := make([]string, 0, len(values)) + for _, v := range values { + if v != "" { + kept = append(kept, v) + } + } + return kept +} + // SearchVideos serves the catalogue's lexical search. // -// It is served on QUERY rather than GET or POST: a search is safe and -// idempotent, which POST is not, but its parameters are a structured document -// that does not belong in a query string. QUERY says both. It is specified in -// draft-ietf-httpbis-safe-method-w-body, not in RFC 9110 — still a draft, so -// expect intermediaries that have never heard of it. Note it also cannot be -// expressed in the generated OpenAPI spec — Swagger 2.0 and OpenAPI 3.x define a fixed set of -// per-method fields with no slot for QUERY, and swag rejects the annotation -// outright — so this endpoint is deliberately absent from /swagger and -// documented here instead. +// It is served on GET: a search is safe and idempotent, which POST is not, and +// its parameters are small enough to say in a query string — which keeps the +// endpoint reachable from anything that speaks HTTP, describable in the +// OpenAPI spec, and cacheable by the intermediaries between the reader and +// this service. +// +// @Summary Search the catalogue +// @Description Finds catalogued videos by the words of their title and the categories they are filed under, most relevant first. Every parameter is optional — a request with none of them browses the whole catalogue, newest first. Paging is by cursor rather than by offset, so a page stays the same page while videos are being announced into the catalogue around it. +// @Tags videos +// @Produce json +// @Param title query string false "Matched lexically against video titles: the words are all required, and the last is treated as a prefix so a part-typed word still finds something." example(desert fal) +// @Param categories query []string false "Narrows the search to videos filed under any one of these names — not all of them. Repeat the parameter per category. Omitted means every category." collectionFormat(multi) example(documentary) +// @Param limit query int false "How many videos to return. Absent, zero or negative takes the default of 20; anything above the maximum of 100 is capped to it rather than refused, and the cursor still reaches the rest." default(20) +// @Param cursor query string false "Asks for the page after the one a previous search ended at. Send back the nextCursor from that search unchanged; it is opaque, and the only thing to do with it is return it." +// @Success 200 {object} api.SearchResults +// @Failure 400 {object} api.ProblemDetails +// @Failure 500 {object} api.ProblemDetails +// @Router /api/videos [get] func SearchVideos(w http.ResponseWriter, r *http.Request) { - var request api.SearchRequest - - if err := json.NewDecoder(r.Body).Decode(&request); err != nil { + request, err := parseSearchQuery(r.URL.Query()) + if err != nil { writeProblem(w, http.StatusBadRequest, "Malformed Search", - "The request body could not be read as JSON. Send a JSON object describing what to search for; an empty object browses the whole catalogue.") + "The search parameters could not be read: "+err.Error()+". Every parameter is optional; a request with none of them browses the whole catalogue.") return } - limit := request.Limit - if limit <= 0 { - limit = searchPageSize - } - if limit > maxSearchPageSize { - limit = maxSearchPageSize - } - - // Read through the cache before touching Postgres. A ranked search is the - // expensive request this service serves — ts_rank is computed per matching - // row, so deep pages scan rather than seek — and readers ask for the same - // few things over and over, which is exactly the shape a cache pays for. - // - // Only successful pages are ever stored, so a malformed cursor still - // reaches the repository and is still rejected below. - cacheKey := searchCacheKey(request.Title, request.Categories, limit, request.Cursor) + cacheKey := searchCacheKey(request.title, request.categories, request.limit, request.cursor) if cached, hit := cachedSearch(r.Context(), cacheKey); hit { writeJSON(w, http.StatusOK, cached) return } - found, next, err := repositories.VideoRepo.SearchVideos(r.Context(), request.Title, request.Categories, limit, request.Cursor) + found, next, err := repositories.VideoRepo.SearchVideos(r.Context(), request.title, request.categories, request.limit, request.cursor) if errors.Is(err, repositories.ErrInvalidCursor) { writeProblem(w, http.StatusBadRequest, "Malformed Search Cursor", "The cursor could not be read. Send back the nextCursor from a previous search unchanged, or omit it to start from the first page.") return } if err != nil { - log.Printf("Something Went Wrong Searching The Catalogue: title=%q: %s", request.Title, err) + log.Printf("Something Went Wrong Searching The Catalogue: title=%q: %s", request.title, err) writeProblem(w, http.StatusInternalServerError, "Something Went Wrong Searching The Catalogue", "The catalogue could not be searched. This is a server-side fault and the request was not processed; retrying in a few moments may succeed.") return diff --git a/discovery/main.go b/discovery/main.go index 433a4bb..3e914a0 100644 --- a/discovery/main.go +++ b/discovery/main.go @@ -26,7 +26,7 @@ import ( // @version 1.0 // @description JSON API for browsing the Thamanyah catalogue. Read-side counterpart to the CMS, which is what ingests videos. // @description -// @description Not listed below: `QUERY /api/videos`, the catalogue search. It takes a JSON body `{title, categories, limit, cursor}` and answers `{videos, nextCursor}` — title is matched lexically with the last word as a prefix, categories narrow to videos filed under any one of them, limit defaults to 20 and is capped at 100, and cursor is the opaque `nextCursor` of a previous search. It is absent here because OpenAPI has no slot for the QUERY method, not because it is unsupported; see handlers.SearchVideos. +// @description The catalogue search is `GET /api/videos`, listed below: title is matched lexically with the last word as a prefix, categories narrow to videos filed under any one of them, limit defaults to 20 and is capped at 100, and cursor is the opaque `nextCursor` of a previous search. // @BasePath / func main() { @@ -117,9 +117,11 @@ func runServer() { mux.HandleFunc("GET /health", handlers.Health) mux.HandleFunc("GET /api/videos/{id}", handlers.GetVideo) - // QUERY, not GET: a search is safe and idempotent but carries a structured - // body. See handlers.SearchVideos for why it is absent from the spec. - mux.HandleFunc("QUERY /api/videos", handlers.SearchVideos) + // The catalogue search. GET rather than POST because a search is safe and + // idempotent, and its parameters — title, categories, limit, cursor — fit + // a query string. ServeMux keeps this apart from the {id} pattern above: + // the more specific one wins. + mux.HandleFunc("GET /api/videos", handlers.SearchVideos) // Swagger UI and the generated spec. The UI assets are embedded in the // binary by swaggo/files, so this needs no static directory on disk. diff --git a/tests/README.md b/tests/README.md index 7985a55..5743e04 100644 --- a/tests/README.md +++ b/tests/README.md @@ -77,7 +77,7 @@ Both steps of the upload flow, end to end: not there at all, and a video announced a second time is updated rather than colliding with its own row -- searching that catalogue over `QUERY /api/videos`: finding a video by a word +- searching that catalogue over `GET /api/videos`: finding a video by a word in its title and by a part-typed one, narrowing to a category and being left out of another, several categories meaning "any of", paging by cursor so the pages tile the results exactly once, refusing a cursor no search issued, and diff --git a/tests/client_test.go b/tests/client_test.go index ea7359b..a634ff3 100644 --- a/tests/client_test.go +++ b/tests/client_test.go @@ -6,7 +6,9 @@ import ( "fmt" "io" "net/http" + "net/url" "os" + "strconv" "strings" "time" ) @@ -139,15 +141,37 @@ type catalogueVideoBody struct { Categories []string `json:"categories"` } -// searchRequest is the body of QUERY /api/videos on discovery: what a reader -// is looking for, plus where in the results to carry on from. +// searchRequest is a search of GET /api/videos on discovery: what a reader is +// looking for, plus where in the results to carry on from. // -// Every field is optional. An empty body is the whole catalogue, newest first. +// Every field is optional. A search with none of them set is the whole +// catalogue, newest first. type searchRequest struct { - Title string `json:"title,omitempty"` - Categories []string `json:"categories,omitempty"` - Limit int `json:"limit,omitempty"` - Cursor string `json:"cursor,omitempty"` + Title string + Categories []string + Limit int + Cursor string +} + +// query renders the search as the query string discovery reads it from. +// Categories are repeated rather than joined, since that is how the endpoint +// takes several of them, and an unset field is left out entirely so that the +// service applies its own default. +func (r searchRequest) query() url.Values { + values := url.Values{} + if r.Title != "" { + values.Set("title", r.Title) + } + for _, category := range r.Categories { + values.Add("categories", category) + } + if r.Limit != 0 { + values.Set("limit", strconv.Itoa(r.Limit)) + } + if r.Cursor != "" { + values.Set("cursor", r.Cursor) + } + return values } // searchResultsBody is what that query answers with: the page of videos, and @@ -177,15 +201,14 @@ func (b searchResultsBody) holds(id string) bool { return false } -// queryJSON sends a QUERY request — the method the catalogue search is served -// on, chosen because a search is safe and idempotent like a GET but carries a -// structured body no query string would hold comfortably. -func (c *client) queryJSON(path string, payload any) (response, error) { - encoded, err := json.Marshal(payload) - if err != nil { - return response{}, err +// search sends the catalogue search: a plain GET with the search spelled out +// in the query string. +func (c *client) search(path string, request searchRequest) (response, error) { + query := request.query().Encode() + if query != "" { + path += "?" + query } - return c.send("QUERY", c.base+path, "application/json", encoded) + return c.get(path) } // postJSON marshals payload and posts it to a path on the CMS. diff --git a/tests/steps_test.go b/tests/steps_test.go index 7e328ee..49db027 100644 --- a/tests/steps_test.go +++ b/tests/steps_test.go @@ -973,7 +973,7 @@ func searchTheCatalogue(t gobdd.StepTest, w *world, request searchRequest) { w.discovery = newDiscoveryClient() } - result, err := w.discovery.queryJSON("/api/videos", request) + result, err := w.discovery.search("/api/videos", request) if err != nil { t.Fatalf("could not search the catalogue: %s", err) return diff --git a/upload-video-demo.sh b/upload-video-demo.sh new file mode 100755 index 0000000..4019807 --- /dev/null +++ b/upload-video-demo.sh @@ -0,0 +1,357 @@ +#!/usr/bin/env bash +# +# upload-video.sh — drive the cms upload flow end to end from a local file. +# +# 1. GET /api/categories pick what the video is filed under +# 2. POST /api/videos/presign ask for an upload slot +# 3. PUT send the file straight to S3, not through cms +# 4. POST /api/videos register it, which queues the transcode +# 5. GET /api/videos/{id} poll until playbackUrl appears +# +# The playback URL is empty until the MediaConvert job reports success and +# cms's consumer records it, so step 5 is a wait, not a formality. A job that +# comes back "failed" never gets a URL — the script stops rather than polls on. +# +# Usage: scripts/upload-video.sh [options] +# Run with --help for the full list. With no options it asks for everything. + +set -euo pipefail + +readonly DEFAULT_BASE_URL="http://cms-alb-d478b02-648162889.us-east-1.elb.amazonaws.com" +readonly DEFAULT_POLL_INTERVAL=5 +readonly DEFAULT_POLL_TIMEOUT=900 + +base_url="${CMS_BASE_URL:-$DEFAULT_BASE_URL}" +file_path="" +title="" +description="" +tags="" +categories_arg="" +poll_interval="$DEFAULT_POLL_INTERVAL" +poll_timeout="$DEFAULT_POLL_TIMEOUT" +skip_poll=false + +usage() { + cat <<'USAGE' +Upload a video to the Thamanyah cms and wait for its playback URL. + +Usage: upload-video.sh [options] + +Options: + -f, --file PATH Video file to upload (.mp4 or .mov). Prompted for if omitted. + -t, --title TITLE Video title. Prompted for if omitted. + -d, --description TEXT Video description. Optional. + --tags TAGS Free-text comma-separated tags. Optional. + -c, --categories LIST Comma-separated category names or ids (e.g. "news,podcast" + or "2,4"). Prompted for if omitted. + -u, --url URL cms base URL. Default: $CMS_BASE_URL or http://localhost:8081 + -i, --interval SECONDS Seconds between status polls. Default: 5 + --timeout SECONDS Give up waiting after this long. Default: 900 + --no-poll Register the video and exit without waiting. + -h, --help Show this help. + +Requires: curl, jq. + +Exit codes: 0 ready, 1 usage/upload error, 2 transcode failed, 3 poll timed out. +USAGE +} + +die() { + printf '\nerror: %s\n' "$*" >&2 + exit 1 +} + +info() { printf '%s\n' "$*" >&2; } + +# The poll writes over one line on a terminal and one line per check when +# redirected — an escape-littered log file helps nobody. +progress() { + if [[ -t 2 ]]; then + printf '\r\033[K %s' "$*" >&2 + else + printf ' %s\n' "$*" >&2 + fi +} + +clear_progress() { [[ -t 2 ]] && printf '\r\033[K' >&2 || true; } + +require_tools() { + local missing=() + for tool in curl jq; do + command -v "$tool" >/dev/null 2>&1 || missing+=("$tool") + done + [[ ${#missing[@]} -eq 0 ]] || die "missing required command(s): ${missing[*]}" +} + +# The API reports failures as RFC 9457 problem details; surface the human parts +# of that rather than dumping the raw body. +report_api_error() { + local context="$1" status="$2" body="$3" problem_title problem_detail + + problem_title=$(jq -r '.title // empty' <<<"$body" 2>/dev/null || true) + problem_detail=$(jq -r '.detail // empty' <<<"$body" 2>/dev/null || true) + + if [[ -n "$problem_title" ]]; then + printf '\nerror: %s (HTTP %s)\n %s\n' "$context" "$status" "$problem_title" >&2 + [[ -n "$problem_detail" ]] && printf ' %s\n' "$problem_detail" >&2 + else + printf '\nerror: %s (HTTP %s)\n %s\n' "$context" "$status" "${body:-}" >&2 + fi + exit 1 +} + +# Splits curl's "body + trailing status line" into the two globals the callers +# read, so a request needs one subshell rather than two round trips. +http_status="" +http_body="" +request() { + local response + response=$(curl --silent --show-error --location --write-out $'\n%{http_code}' "$@") || + die "request to cms failed — is it running at $base_url ?" + http_status="${response##*$'\n'}" + http_body="${response%$'\n'*}" +} + +content_type_for() { + case "${1,,}" in + *.mp4) printf 'video/mp4' ;; + *.mov) printf 'video/quicktime' ;; + *) die "unsupported file type: $1 — cms accepts only .mp4 and .mov" ;; + esac +} + +prompt_required() { + local varname="$1" prompt="$2" value="" + [[ -t 0 ]] || die "$varname not given and stdin is not a terminal — pass it as an option" + while [[ -z "$value" ]]; do + read -r -e -p "$prompt" value + value="${value#"${value%%[![:space:]]*}"}" + done + printf '%s' "$value" +} + +while [[ $# -gt 0 ]]; do + case "$1" in + -f | --file) + file_path="${2:-}" + shift 2 + ;; + -t | --title) + title="${2:-}" + shift 2 + ;; + -d | --description) + description="${2:-}" + shift 2 + ;; + --tags) + tags="${2:-}" + shift 2 + ;; + -c | --categories) + categories_arg="${2:-}" + shift 2 + ;; + -u | --url) + base_url="${2:-}" + shift 2 + ;; + -i | --interval) + poll_interval="${2:-}" + shift 2 + ;; + --timeout) + poll_timeout="${2:-}" + shift 2 + ;; + --no-poll) + skip_poll=true + shift + ;; + -h | --help) + usage + exit 0 + ;; + *) + usage >&2 + die "unknown option: $1" + ;; + esac +done + +require_tools + +base_url="${base_url%/}" +[[ "$poll_interval" =~ ^[0-9]+$ && "$poll_interval" -gt 0 ]] || die "--interval must be a positive integer" +[[ "$poll_timeout" =~ ^[0-9]+$ && "$poll_timeout" -gt 0 ]] || die "--timeout must be a positive integer" + +# ---------------------------------------------------------------- the file --- + +if [[ -z "$file_path" ]]; then + # -e gives readline's filename completion, which is the whole point of + # asking here rather than making the flag mandatory. + file_path=$(prompt_required "--file" "Video file to upload: ") +fi +file_path="${file_path/#\~/$HOME}" + +[[ -f "$file_path" ]] || die "no such file: $file_path" +[[ -r "$file_path" ]] || die "file is not readable: $file_path" +[[ -s "$file_path" ]] || die "file is empty: $file_path" + +file_name=$(basename -- "$file_path") +content_type=$(content_type_for "$file_name") + +info "Checking cms at $base_url ..." +request "$base_url/health" +[[ "$http_status" == "200" ]] || report_api_error "cms is not healthy" "$http_status" "$http_body" + +# ---------------------------------------------------------- the categories --- + +request "$base_url/api/categories" +[[ "$http_status" == "200" ]] || report_api_error "could not list categories" "$http_status" "$http_body" +categories_json="$http_body" + +if [[ -z "$categories_arg" ]]; then + info "" + info "Available categories:" + jq -r '.categories[] | " \(.id)) \(.name)"' <<<"$categories_json" >&2 + info "" + categories_arg=$(prompt_required "--categories" "Categories (comma-separated ids or names): ") +fi + +# Accept either spelling — an id straight from the list, or the name it goes by +# — and resolve both to the ids POST /api/videos wants. Unmatched entries come +# back in their own member rather than as a jq error, so they can be named. +resolved=$(jq -c --arg raw "$categories_arg" ' + [$raw | split(",") | .[] | gsub("^\\s+|\\s+$"; "") | select(length > 0)] as $wanted + | { + ids: [ $wanted[] as $w + | $categories.categories[] + | select((.id | tostring) == $w or (.name | ascii_downcase) == ($w | ascii_downcase)) + | .id ] | unique, + unknown: [ $wanted[] as $w + | select([ $categories.categories[] + | select((.id | tostring) == $w or (.name | ascii_downcase) == ($w | ascii_downcase)) ] | length == 0) + | $w ] + } +' --argjson categories "$categories_json" -n) + +unknown_categories=$(jq -r '.unknown | join(", ")' <<<"$resolved") +[[ -z "$unknown_categories" ]] || die "no category is called \"$unknown_categories\" — pick from the list with GET $base_url/api/categories" + +category_ids_json=$(jq -c '.ids' <<<"$resolved") +[[ "$category_ids_json" != "[]" ]] || die "at least one category is required" + +category_names=$(jq -r --argjson ids "$category_ids_json" \ + '[.categories[] | select(.id as $i | $ids | index($i)) | .name] | join(", ")' <<<"$categories_json") + +# -------------------------------------------------------------- the metadata --- + +[[ -n "$title" ]] || title=$(prompt_required "--title" "Title: ") + +info "" +info "Uploading : $file_path ($content_type)" +info "Title : $title" +info "Categories : $category_names" +info "" + +# ------------------------------------------------------------- 1. presign --- + +info "Requesting an upload URL ..." +presign_body=$(jq -n --arg fileName "$file_name" --arg contentType "$content_type" \ + '{fileName: $fileName, contentType: $contentType}') + +request -X POST "$base_url/api/videos/presign" \ + -H 'Content-Type: application/json' \ + --data-binary "$presign_body" +[[ "$http_status" == "200" ]] || report_api_error "could not get an upload URL" "$http_status" "$http_body" + +upload_url=$(jq -r '.uploadUrl' <<<"$http_body") +storage_key=$(jq -r '.key' <<<"$http_body") +[[ -n "$upload_url" && "$upload_url" != "null" ]] || die "cms returned no upload URL" + +# ----------------------------------------------------- 2. PUT the file to S3 --- + +# The content type is signed into the URL, so this header has to match the one +# sent to /presign exactly or S3 rejects the PUT as a signature mismatch. +info "Uploading $(du -h -- "$file_path" | cut -f1) to storage ..." +upload_status=$(curl --silent --show-error --progress-bar \ + --request PUT \ + --header "Content-Type: $content_type" \ + --upload-file "$file_path" \ + --write-out '%{http_code}' \ + --output /dev/null \ + "$upload_url") || die "upload to storage failed" + +[[ "$upload_status" =~ ^2 ]] || die "storage rejected the upload (HTTP $upload_status)" +info "Upload complete: $storage_key" + +# ------------------------------------------- 3. register + queue transcoding --- + +info "Registering the video ..." +complete_body=$(jq -n \ + --arg title "$title" \ + --arg description "$description" \ + --arg tags "$tags" \ + --arg fileName "$file_name" \ + --arg key "$storage_key" \ + --argjson categoryIds "$category_ids_json" \ + '{title: $title, description: $description, categoryIds: $categoryIds, tags: $tags, fileName: $fileName, key: $key}') + +request -X POST "$base_url/api/videos" \ + -H 'Content-Type: application/json' \ + --data-binary "$complete_body" +[[ "$http_status" == "201" ]] || report_api_error "could not register the video" "$http_status" "$http_body" + +video_id=$(jq -r '.id' <<<"$http_body") +info "Registered as $video_id (status: $(jq -r '.status' <<<"$http_body"))" + +if [[ "$skip_poll" == true ]]; then + info "" + info "Not waiting for transcoding (--no-poll). Follow it with:" + info " curl -s $base_url/api/videos/$video_id | jq" + printf '%s\n' "$video_id" + exit 0 +fi + +# ------------------------------------------ 4. poll until the URL is there --- + +info "" +info "Waiting for transcoding to finish (timeout ${poll_timeout}s, checking every ${poll_interval}s) ..." + +started_at=$SECONDS +while :; do + request "$base_url/api/videos/$video_id" + [[ "$http_status" == "200" ]] || report_api_error "could not read the video" "$http_status" "$http_body" + + status=$(jq -r '.status' <<<"$http_body") + playback_url=$(jq -r '.playbackUrl // empty' <<<"$http_body") + elapsed=$((SECONDS - started_at)) + + # The URL is what the caller is actually waiting for, and it is written + # alongside the status — so wait for the URL, not merely for "ready". + if [[ -n "$playback_url" ]]; then + clear_progress + info "Ready after ${elapsed}s." + info "" + info "Playback URL:" + printf '%s\n' "$playback_url" + exit 0 + fi + + if [[ "$status" == "failed" ]]; then + clear_progress + printf '\nerror: transcoding failed for video %s after %ss — no playback URL will be produced.\n' "$video_id" "$elapsed" >&2 + exit 2 + fi + + if ((elapsed >= poll_timeout)); then + clear_progress + printf '\nerror: timed out after %ss with status %q and no playback URL.\n Check again with: curl -s %s/api/videos/%s | jq\n' \ + "$elapsed" "$status" "$base_url" "$video_id" >&2 + exit 3 + fi + + progress "status: $status — ${elapsed}s elapsed" + sleep "$poll_interval" +done