From 12747738af09640dd5cbc6d84e20152ab2b984ba Mon Sep 17 00:00:00 2001 From: Claudio Ortolina Date: Sun, 10 May 2026 14:44:19 +0100 Subject: [PATCH] ML-176.1: plan --- ...uery-parameter-on-GET-api-v1-collection.md | 90 +++++++++++++++++++ 1 file changed, 90 insertions(+) diff --git a/backlog/tasks/ml-176.1 - API-accept-search-query-parameter-on-GET-api-v1-collection.md b/backlog/tasks/ml-176.1 - API-accept-search-query-parameter-on-GET-api-v1-collection.md index 3e81904f..582789be 100644 --- a/backlog/tasks/ml-176.1 - API-accept-search-query-parameter-on-GET-api-v1-collection.md +++ b/backlog/tasks/ml-176.1 - API-accept-search-query-parameter-on-GET-api-v1-collection.md @@ -4,6 +4,7 @@ title: "API: accept search query parameter on GET /api/v1/collection" status: To Do assignee: [] created_date: "2026-05-10 13:35" +updated_date: "2026-05-10 13:41" labels: - api - backend @@ -50,3 +51,92 @@ The existing FTS5 search in `Records.Search` handles the query — no new search - [ ] #5 Response format matches existing schema: {total, limit, offset, records} with all record fields - [ ] #6 Controller test covers: no q param, empty q, populated q, q with no matches, q with pagination + +## Implementation Plan + + + +## Implementation Plan + +### 1. Objective alignment + +Extend `GET /api/v1/collection` to accept an optional `q` query parameter for FTS5 full-text search. When `q` is present and non-empty, pass it to the existing `Collection.search_records/2` and `search_records_count/1` pipeline. When absent or empty, preserve the current behavior of returning all collection records. No new search logic is required — `Records.Search` already handles both empty and populated queries. + +### 2. Simplicity and alternatives considered + +**Chosen approach: extract `q` from conn params in the controller, pass it directly.** + +The controller's `index/2` action currently hardcodes `""` as the query. Changing it to read `params["q"]` and normalize empty/absent to `""` is a ~5-line change. The entire search pipeline (`Collection.search_records/2` → `Records.Search.search_records/3` → `build_search/3`) already supports passing a query string and treats `{:query, ""}` as a no-op. + +**Alternatives considered and rejected:** + +- **New dedicated endpoint (`GET /api/v1/collection/search`)**: Adds a route and controller action for no benefit. The existing endpoint already returns searchable data — adding a query parameter is simpler and REST-idiomatic. +- **Separate search action in the controller**: Would duplicate the pagination and render logic already in `index/2`. The only difference is the `q` value — a parameter is cleaner. +- **Validation/sanitization layer**: The `Records.Search` module already handles FTS5 escaping via `fts_query_escape/1` (splitting on whitespace, escaping quotes, wrapping in double-quote syntax with `*` suffix). No additional sanitization is needed. +- **New LiveView action/template**: Not applicable. This is API-only, consumed by the Presto client. + +### 3. Completeness and sequencing + +All steps are independent and can be done in a single pass: + +1. **Extract `q` param in `CollectionController.index/2`** — Add `query = normalize_query(params["q"])` before the existing `limit`/`offset` extraction, and pass `query` to `search_records_count/1` and `search_records/2` instead of `""`. Add a private `normalize_query/1` helper that returns `""` for `nil` or empty string input. + +2. **Add tests to `collection_controller_test.exs`** — Six test cases in the `describe "GET /api/v1/collection"` block: + - Existing test unchanged (no `q` param → returns all records) + - `q` with matching text → returns filtered results + - Empty `q` string → returns all records (same as absent) + - `q` with no matches → returns `total: 0`, empty `records` + - `q` with pagination (`limit`/`offset`) → respects both search and pagination + - Response shape validation → confirms the schema is unchanged + +3. **Run the full test suite** — `mix test test/music_library_web/controllers/collection_controller_test.exs` + +### 4. Verifiability + +| Step | Verification | +| ------------------- | ----------------------------------------------------------------------------------------------------------------- | +| Controller change | `mix test test/music_library_web/controllers/collection_controller_test.exs` — all tests pass | +| No regression | `mix test` full suite — all tests pass | +| Manual API check | `curl -H "Authorization: Bearer " "localhost:4002/api/v1/collection?q=beatles"` returns filtered results | +| Empty query | `curl ... "localhost:4002/api/v1/collection?q="` returns all records | +| Pagination + search | `curl ... "localhost:4002/api/v1/collection?q=beatles&limit=1&offset=0"` returns correct count with single result | + +### 5. Architecture impact analysis + +| Touchpoint | Impact | +| --------------------------------------------------- | ------------------------------------------------------------------------------------- | +| `MusicLibraryWeb.CollectionController` (controller) | **Changed** — `index/2` reads `q` param and a new `normalize_query/1` helper is added | +| `MusicLibrary.Collection` (context) | **Unchanged** — already accepts query string | +| `MusicLibrary.Records.Search` (search module) | **Unchanged** — already handles empty queries | +| `MusicLibraryWeb.CollectionJSON` (JSON view) | **Unchanged** — response format is identical | +| Router (`router.ex`) | **Unchanged** — query params don't affect routing | +| Schemas (`SearchIndex`, `Record`) | **Unchanged** — no schema changes | +| PubSub | **Unchanged** — no broadcast events involved | +| Supervision tree | **Unchanged** — no new processes | +| External APIs | **Unchanged** — no external calls | +| Presto client | **Unchanged** — the client already sends requests; the `q` param is opt-in | + +### 6. Performance profile + +- **Query complexity**: Unchanged from the existing FTS5 path. The query already runs `records_search_index MATCH ` with the same LIMIT/OFFSET pattern. An empty `q` produces the same query plan as today (no FTS clause → full scan with LIMIT/OFFSET on `purchased_at IS NOT NULL`). +- **N+1 risk**: None. `SearchIndex` is a pre-joined materialization that includes all needed fields (artists, genres as JSON). The `essential_fields()` macro selects all columns in a single query. +- **Memory**: Unchanged. Pagination caps results at 20 records by default, same as today. +- **Latency**: FTS5 queries on the `records_search_index` table are sub-millisecond for typical collection sizes. The index is maintained automatically by SQLite triggers. + +### 7. Benchmarking requirements + +No benchmarks needed. This change does not introduce new query patterns — it reuses the existing FTS5 search path. The performance characteristics are identical whether `q` is empty or populated (both paths already exist in `build_search/3`). + +### 8. Cost profile + +No cost impact. No external API calls, no additional storage, no compute changes. The FTS5 search runs entirely within the local SQLite database. + +### 9. Production infrastructure steps + +**No production changes required.** The route is unchanged (query parameters don't affect routing), no new environment variables, no migrations, no service provisioning. The change is a standard code deploy. + +### 10. Documentation updates + +No documentation updates needed. The API endpoint already exists in `router.ex`. The `q` parameter is additive and optional — existing API consumers are unaffected. If project API docs exist externally, note that `GET /api/v1/collection` now accepts `?q=` as an optional parameter, but this is self-documenting via the REST pattern. + +