ML-176.1: plan
This commit is contained in:
Claudio Ortolina
+90
@@ -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
|
||||
<!-- AC:END -->
|
||||
|
||||
## Implementation Plan
|
||||
|
||||
<!-- SECTION:PLAN:BEGIN -->
|
||||
|
||||
## 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 <token>" "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 <escaped_query>` 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=<search term>` as an optional parameter, but this is self-documenting via the REST pattern.
|
||||
|
||||
<!-- SECTION:PLAN:END -->
|
||||
|
||||
Reference in New Issue
Block a user