diff --git a/README.md b/README.md index 5111ecbc..d53b4c50 100644 --- a/README.md +++ b/README.md @@ -212,6 +212,10 @@ If you prefer running Silo without Docker: ## Reporting Issues +Client implementers can use the [Canonical Settings API guide](docs/settings-api.md) +for contract discovery, contextual headers, remote scopes, effective reads, and +the admin projection. + If you are reporting a bug, install problem, or performance issue, start with the admin workflow and reproduction steps, not Claude/Codex analysis. Please include: diff --git a/cmd/settingsgen/main.go b/cmd/settingsgen/main.go index d1cc5c59..9a257b93 100644 --- a/cmd/settingsgen/main.go +++ b/cmd/settingsgen/main.go @@ -157,6 +157,7 @@ func generateTypeScript(contract *settingscontract.Manifest) ([]byte, error) { } out.WriteString(" */\n\n") + fmt.Fprintf(&out, "export const SETTINGS_API_VERSION = %d;\n", contract.APIVersion) fmt.Fprintf(&out, "export const SETTINGS_REVISION = %d;\n\n", contract.Revision) out.WriteString("export interface SettingSuggestedOption {\n") diff --git a/contracts/settings/v1/conformance.json b/contracts/settings/v1/conformance.json index 1fd60cbd..fc8f4769 100644 --- a/contracts/settings/v1/conformance.json +++ b/contracts/settings/v1/conformance.json @@ -1,8 +1,82 @@ { "fixture_version": 1, - "manifest_revision": 4, + "manifest_revision": 5, "description": "Cross-platform conformance cases for settings resolution. Every case runs against the shipped manifest in this directory: definitions are referenced by key, never restated, so an expectation can only be satisfied by resolving the real contract. Each platform's resolver (Go in internal/settingsresolve, TypeScript in web/src/lib/settingsResolve.ts, Kotlin and Swift in the client repos) runs every case through a hand-written runner; a runner must fail on any fixture field it does not know, because schema drift in the fixture itself is drift. A case's constraint_bindings attach a constraint to a copy of a real definition so constraint semantics stay testable even while no shipped definition carries that constraint kind. In expected entries, constrained:true requires stored_value and constraint_kind to be present, and stored_value may be null to mean the authored value was JSON null.", "cases": [ + { + "name": "profile_client_sits_between_device_and_profile", + "description": "Presentation values roam across like clients, while an exact-device override remains more specific and a profile value remains the cross-family fallback.", + "keys": ["ui.card_presentation"], + "context": { "profile_id": "p1", "client_family": "tv", "device_id": "living-room" }, + "stored": [ + { + "key": "ui.card_presentation", + "scope": "profile", + "profile_id": "p1", + "value": { "poster_size": "standard", "caption": "title" } + }, + { + "key": "ui.card_presentation", + "scope": "profile_client", + "profile_id": "p1", + "client_family": "tv", + "value": { "poster_size": "large", "caption": "artwork" } + }, + { + "key": "ui.card_presentation", + "scope": "profile_device", + "profile_id": "p1", + "device_id": "living-room", + "value": { "poster_size": "compact", "caption": "title_metadata" } + } + ], + "expected": [ + { + "key": "ui.card_presentation", + "value": { "poster_size": "compact", "caption": "title_metadata" }, + "source": "profile_device" + } + ] + }, + { + "name": "profile_client_roams_only_within_its_family", + "description": "A television menu applies to another television identity but not to mobile; the family is explicit resolution context rather than inferred from device metadata.", + "keys": ["nav.primary_menu"], + "context": { "profile_id": "p1", "client_family": "tv", "device_id": "bedroom-tv" }, + "stored": [ + { + "key": "nav.primary_menu", + "scope": "profile_client", + "profile_id": "p1", + "client_family": "mobile", + "value": { "items": [{ "type": "builtin", "destination": "home" }] } + }, + { + "key": "nav.primary_menu", + "scope": "profile_client", + "profile_id": "p1", + "client_family": "tv", + "value": { + "items": [ + { "type": "builtin", "destination": "home" }, + { "type": "builtin", "destination": "movies" } + ] + } + } + ], + "expected": [ + { + "key": "nav.primary_menu", + "value": { + "items": [ + { "type": "builtin", "destination": "home" }, + { "type": "builtin", "destination": "movies" } + ] + }, + "source": "profile_client" + } + ] + }, { "name": "resolution_order_series_wins", "description": "The full ladder: with values stored at every scope a content key allows, profile_series beats profile_library, profile_device, and profile.", diff --git a/contracts/settings/v1/manifest.json b/contracts/settings/v1/manifest.json index 91445620..186d922f 100644 --- a/contracts/settings/v1/manifest.json +++ b/contracts/settings/v1/manifest.json @@ -1,6 +1,6 @@ { "api_version": 1, - "revision": 4, + "revision": 5, "option_sets": { "playback_audio_languages": { "type": "language_tag", @@ -827,6 +827,59 @@ "recommended_control": "select", "notes": "Registered from the legacy unprefixed key next_up_mode. The server reads it directly when assembling home sections, so it cannot be client-local; that read moves to the canonical resolver at cutover. The legacy value was untyped and absent meant combined, which is why combined is the default rather than a third \"unset\" member." }, + { + "key": "nav.primary_menu", + "introduced_in": 5, + "persistence": "remote", + "allowed_scopes": ["profile_client", "profile_device"], + "resolution_order": ["profile_device", "profile_client", "default"], + "value_schema": { + "type": "object", + "schema_ref": "primary-menu.json", + "nullable": true + }, + "default_value": null, + "platforms": ["web", "ios", "tvos", "macos", "android", "android_tv"], + "category": "navigation", + "label": "Primary menu", + "description": "The ordered visible destinations shared by like clients.", + "recommended_control": "panel", + "notes": "Search and profile remain fixed client utilities. Home is required by primary-menu.json; omitting any other supported built-in hides it. Semantic destination identities are unique even when labels differ. A null default lets each family keep its native baseline until the user customizes it. Family scope synchronizes like clients while profile_device remains an explicit escape hatch for one screen; clients ignore built-in destinations they do not support." + }, + { + "key": "nav.shortcuts", + "introduced_in": 5, + "persistence": "remote", + "allowed_scopes": ["profile"], + "resolution_order": ["profile", "default"], + "value_schema": { + "type": "object", + "schema_ref": "navigation-shortcuts.json" + }, + "default_value": { "items": [] }, + "category": "navigation", + "label": "Navigation shortcuts", + "description": "Libraries, sections, and collections pinned for use across navigation surfaces.", + "notes": "Profile-wide catalog; individual client families decide which shortcuts to place in their primary menu. Semantic destination identities are unique even when labels differ. The profile_client migration keeps ui.sidebar_pins unchanged and seeds this key from convertible legacy web pins only when this key has no authored row." + }, + { + "key": "ui.card_presentation", + "introduced_in": 5, + "persistence": "remote", + "allowed_scopes": ["profile", "profile_client", "profile_device"], + "resolution_order": ["profile_device", "profile_client", "profile", "default"], + "value_schema": { + "type": "object", + "schema_ref": "card-presentation.json" + }, + "default_value": { "poster_size": "standard", "caption": "title_metadata" }, + "platforms": ["web", "ios", "tvos", "macos", "android", "android_tv"], + "category": "appearance", + "label": "Media cards", + "description": "Poster size and caption detail used by media cards.", + "recommended_control": "panel", + "notes": "Semantic presets roam between like devices without forcing identical pixel dimensions across platforms. A profile fallback can opt into one presentation everywhere; family and exact-device values remain more specific." + }, { "key": "ui.sidebar_pins", "introduced_in": 1, diff --git a/contracts/settings/v1/manifest.schema.json b/contracts/settings/v1/manifest.schema.json index 0ef0d44f..96d1ab05 100644 --- a/contracts/settings/v1/manifest.schema.json +++ b/contracts/settings/v1/manifest.schema.json @@ -76,6 +76,7 @@ "enum": [ "account", "profile", + "profile_client", "profile_device", "profile_library", "profile_series", @@ -291,6 +292,7 @@ "enum": [ "account", "profile", + "profile_client", "profile_device", "profile_library", "profile_series", diff --git a/contracts/settings/v1/schemas/card-presentation.json b/contracts/settings/v1/schemas/card-presentation.json new file mode 100644 index 00000000..ef7e46ea --- /dev/null +++ b/contracts/settings/v1/schemas/card-presentation.json @@ -0,0 +1,19 @@ +{ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "$id": "https://silo-server.dev/contracts/settings/v1/schemas/card-presentation.json", + "title": "Media card presentation", + "description": "Semantic density and caption presets that each client family maps to platform-appropriate card dimensions.", + "type": "object", + "additionalProperties": false, + "required": ["poster_size", "caption"], + "properties": { + "poster_size": { + "type": "string", + "enum": ["compact", "standard", "large"] + }, + "caption": { + "type": "string", + "enum": ["title_metadata", "title", "artwork"] + } + } +} diff --git a/contracts/settings/v1/schemas/navigation-shortcuts.json b/contracts/settings/v1/schemas/navigation-shortcuts.json new file mode 100644 index 00000000..1920a24c --- /dev/null +++ b/contracts/settings/v1/schemas/navigation-shortcuts.json @@ -0,0 +1,62 @@ +{ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "$id": "https://silo-server.dev/contracts/settings/v1/schemas/navigation-shortcuts.json", + "title": "Navigation shortcuts", + "description": "Profile-wide catalog of pinned libraries, sections, and collections that client-family menus may surface.", + "$comment": "Contract validation also rejects repeated semantic destination identities even when labels differ: library(library_id), section(library_id, section_id), collection(optional library_id, collection_id).", + "type": "object", + "additionalProperties": false, + "required": ["items"], + "properties": { + "items": { + "description": "Pinned targets. Each semantic destination identity may appear at most once; changing its label does not create a distinct destination.", + "type": "array", + "maxItems": 256, + "uniqueItems": true, + "items": { + "oneOf": [ + { "$ref": "#/$defs/library" }, + { "$ref": "#/$defs/section" }, + { "$ref": "#/$defs/collection" } + ] + } + } + }, + "$defs": { + "label": { "type": "string", "minLength": 1, "maxLength": 256, "pattern": "\\S" }, + "libraryId": { "type": "integer", "minimum": 1 }, + "targetId": { "type": "string", "minLength": 1, "maxLength": 128, "pattern": "\\S" }, + "library": { + "type": "object", + "additionalProperties": false, + "required": ["type", "library_id", "label"], + "properties": { + "type": { "const": "library" }, + "library_id": { "$ref": "#/$defs/libraryId" }, + "label": { "$ref": "#/$defs/label" } + } + }, + "section": { + "type": "object", + "additionalProperties": false, + "required": ["type", "library_id", "section_id", "label"], + "properties": { + "type": { "const": "section" }, + "library_id": { "$ref": "#/$defs/libraryId" }, + "section_id": { "$ref": "#/$defs/targetId" }, + "label": { "$ref": "#/$defs/label" } + } + }, + "collection": { + "type": "object", + "additionalProperties": false, + "required": ["type", "collection_id", "label"], + "properties": { + "type": { "const": "collection" }, + "collection_id": { "$ref": "#/$defs/targetId" }, + "library_id": { "$ref": "#/$defs/libraryId" }, + "label": { "$ref": "#/$defs/label" } + } + } + } +} diff --git a/contracts/settings/v1/schemas/primary-menu.json b/contracts/settings/v1/schemas/primary-menu.json new file mode 100644 index 00000000..f53c60dc --- /dev/null +++ b/contracts/settings/v1/schemas/primary-menu.json @@ -0,0 +1,86 @@ +{ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "$id": "https://silo-server.dev/contracts/settings/v1/schemas/primary-menu.json", + "title": "Primary navigation menu", + "description": "Ordered visible destinations for one client family. Search and profile controls are fixed client utilities and are intentionally absent.", + "$comment": "Contract validation also rejects repeated semantic destination identities even when labels differ: builtin(destination), library(library_id), section(library_id, section_id), collection(optional library_id, collection_id).", + "type": "object", + "additionalProperties": false, + "required": ["items"], + "properties": { + "items": { + "description": "Ordered destinations. Each semantic destination identity may appear at most once; changing its label does not create a distinct destination.", + "type": "array", + "minItems": 1, + "maxItems": 64, + "uniqueItems": true, + "contains": { + "type": "object", + "required": ["type", "destination"], + "properties": { + "type": { "const": "builtin" }, + "destination": { "const": "home" } + } + }, + "minContains": 1, + "maxContains": 1, + "items": { + "oneOf": [ + { "$ref": "#/$defs/builtin" }, + { "$ref": "#/$defs/library" }, + { "$ref": "#/$defs/section" }, + { "$ref": "#/$defs/collection" } + ] + } + } + }, + "$defs": { + "label": { "type": "string", "minLength": 1, "maxLength": 256, "pattern": "\\S" }, + "libraryId": { "type": "integer", "minimum": 1 }, + "targetId": { "type": "string", "minLength": 1, "maxLength": 128, "pattern": "\\S" }, + "builtin": { + "type": "object", + "additionalProperties": false, + "required": ["type", "destination"], + "properties": { + "type": { "const": "builtin" }, + "destination": { + "type": "string", + "enum": ["home", "movies", "series", "music", "audiobooks", "for_you", "calendar"] + } + } + }, + "library": { + "type": "object", + "additionalProperties": false, + "required": ["type", "library_id", "label"], + "properties": { + "type": { "const": "library" }, + "library_id": { "$ref": "#/$defs/libraryId" }, + "label": { "$ref": "#/$defs/label" } + } + }, + "section": { + "type": "object", + "additionalProperties": false, + "required": ["type", "library_id", "section_id", "label"], + "properties": { + "type": { "const": "section" }, + "library_id": { "$ref": "#/$defs/libraryId" }, + "section_id": { "$ref": "#/$defs/targetId" }, + "label": { "$ref": "#/$defs/label" } + } + }, + "collection": { + "type": "object", + "additionalProperties": false, + "required": ["type", "collection_id", "label"], + "properties": { + "type": { "const": "collection" }, + "collection_id": { "$ref": "#/$defs/targetId" }, + "library_id": { "$ref": "#/$defs/libraryId" }, + "label": { "$ref": "#/$defs/label" } + } + } + } +} diff --git a/docs/settings-api.md b/docs/settings-api.md new file mode 100644 index 00000000..fb8ec5ad --- /dev/null +++ b/docs/settings-api.md @@ -0,0 +1,198 @@ +# Canonical Settings API + +The canonical settings API stores typed user preferences from the shared +settings contract. Client implementations should discover the server contract +before rendering controls or writing a value; do not keep a separate list of +keys, scopes, enum members, or defaults. + +All paths below are relative to `/api/v1`. + +## Contract discovery + +| Method and path | Purpose | +| -------------------------- | ------------------------------------------------------------------------------------------------------- | +| `GET /settings/manifest` | Public client projection of the current manifest. Supports `If-None-Match`. | +| `GET /settings/capability` | Contract API version, revision, remote scopes, supported client families, and batch/write capabilities. | + +`/settings/contract` and `/settings/contract/capabilities` are equivalent +aliases. A client whose vendored contract is newer than the advertised server +revision must hide definitions and features introduced after that revision. +Navigation shortcut mutation controls additionally require +`supports_atomic_shortcuts: true`. Revision-5 customization also requires +`supports_batched_effective: true` and `supports_idempotent_writes: true` so +batched resolution and replayed writes have the semantics the clients depend +on. Any missing flag fails closed. + +## Request identity headers + +Authenticated settings routes use the active account and profile from the +normal Silo session. The contextual headers below identify which client is +resolving or writing an override. + +| Header | When required | Meaning | +| ---------------------- | ----------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------- | +| `X-Profile-Id` | All `/settings/values` routes | Active household profile. | +| `X-Silo-Device-Id` | A `profile_device` explicit request, or an effective request containing a key that permits `profile_device` | Stable exact-device identity. | +| `X-Silo-Client-Family` | Required for a `profile_client` explicit request; optional on effective reads | Closed like-client identity: `tv`, `mobile`, `tablet`, `desktop`, or `web`. A valid value includes the like-client layer; absence skips it. | +| `X-Silo-Mutation-Id` | Optional on `PUT` | Idempotency key for safe retries. Reusing it for a different identity or value returns a conflict. | + +Client family is intentionally independent of the free-form +`X-Silo-Device-Platform` display metadata. The server never guesses one from +the other. Send the exact lower-case family: + +| Client | Family | +| ----------------------- | --------- | +| tvOS or Android TV | `tv` | +| iPhone or Android phone | `mobile` | +| iPad or Android tablet | `tablet` | +| macOS | `desktop` | +| Browser | `web` | + +## Remote scopes + +Every stored value has exactly one identity. Context fields not named by the +selected scope must be absent. + +| Scope | Identity after account | +| ----------------- | ----------------------------- | +| `account` | none | +| `profile` | `profile_id` | +| `profile_client` | `profile_id`, `client_family` | +| `profile_device` | `profile_id`, `device_id` | +| `profile_library` | `profile_id`, `library_id` | +| `profile_series` | `profile_id`, `series_id` | + +The manifest's `allowed_scopes` decides where each key may be written, and its +`resolution_order` decides precedence. `profile_client` values roam only among +like clients. For example, a TV value applies to tvOS and Android TV but not to +a phone or browser. + +## Explicit values + +Use explicit endpoints to edit or clear one scope, without resolving inherited +values: + +- `GET /settings/values?keys=&scope=` returns every requested key + with `is_set`; unset rows remain in the response. +- `GET /settings/values/{key}?scope=` returns one stored value or `404`. +- `PUT /settings/values/{key}?scope=` accepts `{"value": }`. +- `DELETE /settings/values/{key}?scope=` removes the row so inheritance + applies again. + +`library_id` and `series_id` are query parameters for their matching scopes. +An explicitly managed device may be named with `device_id`, subject to the +profile/device authorization checks. The self-service `profile_client` scope +takes its family only from `X-Silo-Client-Family`. + +Example: share a TV menu between tvOS and Android TV clients. + +```http +PUT /api/v1/settings/values/nav.primary_menu?scope=profile_client +Authorization: Bearer +X-Profile-Id: +X-Silo-Client-Family: tv +Content-Type: application/json + +{"value":{"items":[{"type":"builtin","destination":"home"},{"type":"library","library_id":7,"label":"Movies"}]}} +``` + +Stored-value responses include `client_family` when the source or explicit row +is at `profile_client`. + +### Atomic navigation shortcuts + +`nav.shortcuts` is a profile-wide catalog shared by TV, mobile, desktop, and +web clients. Self-service clients must mutate one destination at a time instead +of replacing that shared document: + +```http +PUT /api/v1/settings/values/nav.shortcuts/item +Authorization: Bearer +X-Profile-Id: +X-Silo-Mutation-Id: +Content-Type: application/json + +{"item":{"type":"section","library_id":7,"section_id":"recent","label":"Recently Added"},"present":true} +``` + +The item is one member of `navigation-shortcuts.json`: a library, section, or +collection (whose `collection_id` is a string and whose `library_id` is +optional). Identity is exactly the schema's semantic identity: + +- library: `type + library_id` +- section: `type + library_id + section_id` +- collection: `type + optional library_id + collection_id` + +`present: true` appends an absent item or refreshes the label of an existing +identity in place without reordering it. `present: false` removes that identity; +the supplied label is validated but ignored for matching. The server atomically +rebases on concurrent edits, enforces the full schema and 256-item cap, and +returns the normal stored-value object containing the complete resulting +`value`, row `revision`, and `updated_at`. An already-satisfied operation is a +successful no-op and does not increment the revision. Removing from a catalog +that has never been stored returns `{"items":[]}` at revision `0` with no +`updated_at`. + +Retry with the same `X-Silo-Mutation-Id`. A recorded retry returns the original +response with `X-Silo-Idempotent-Replay: true`; reusing the id for a different +semantic operation returns `409 mutation_id_conflict`. The mutation ID is +serialized before the setting write, and the setting plus its replay receipt +commit in one database transaction, so concurrent reuse cannot apply twice and +a crash cannot persist only one half. A rare exhausted +contention loop returns retryable `409 setting_update_conflict`. Malformed +envelopes or unknown envelope fields return `400 bad_request`, while item +schema failures (including unknown item fields) and cap failures return +`400 invalid_value`. + +Ordinary session `PUT` and `DELETE` at +`/settings/values/nav.shortcuts?scope=profile` are rejected with +`400 atomic_update_required` so a whole-document mutation cannot erase +concurrent item edits or reset the row revision. The admin endpoint retains +whole-document `PUT` for explicit repair work, but rejects physical `DELETE` +for the same revision-history reason. An admin clears the catalog with +`{"value":{"items":[]}}`, which advances the row revision, or replaces it with +another validated document. + +## Effective values + +- `GET /settings/values/effective?keys=` resolves several keys for the + request profile, device, and client family. Omitting `keys` resolves all + remote definitions. +- `POST /settings/values/effective` resolves a bounded list of content contexts + in one store read. Use it for grids or lists rather than issuing one request + per item. + +An effective request must include the device header when any requested +definition permits an exact-device override. The family header is optional for +backward compatibility: absence skips the `profile_client` layer, while a +non-empty invalid family is rejected. First-party clients should send their +family so like-device preferences participate in resolution. Each response +includes its source scope and source context; `client_family` is included for a +family-scoped winner. + +## Admin projection + +Admin routes are mounted behind the normal acting-admin authorization: + +- `GET /admin/users/{id}/settings/values` lists every stored row across all + scopes. +- `PUT /admin/users/{id}/settings/values/{key}?scope=` writes through the + same contract validation and normalization as the self-service endpoint. +- `DELETE /admin/users/{id}/settings/values/{key}?scope=` clears the + selected row. + +Admin requests name profile/context identity with query parameters because the +target user is not the admin's active session. In particular, +`scope=profile_client` requires both `profile_id` and `client_family` query +parameters; it does not use `X-Silo-Client-Family`. + +```http +PUT /api/v1/admin/users/42/settings/values/ui.card_presentation?scope=profile_client&profile_id=main&client_family=tv +Authorization: Bearer +Content-Type: application/json + +{"value":{"poster_size":"large","caption":"artwork"}} +``` + +The five accepted `client_family` values are also returned by the capability +endpoint so admin tooling does not need to invent them. diff --git a/docs/superpowers/specs/2026-07-10-cross-platform-user-settings-contract-design.md b/docs/superpowers/specs/2026-07-10-cross-platform-user-settings-contract-design.md index 55674d05..1c050b91 100644 --- a/docs/superpowers/specs/2026-07-10-cross-platform-user-settings-contract-design.md +++ b/docs/superpowers/specs/2026-07-10-cross-platform-user-settings-contract-design.md @@ -416,6 +416,7 @@ The remote scopes are: |---|---|---| | `account` | `(user_id)` | Same for every profile and signed-in client on the account. | | `profile` | `(user_id, profile_id)` | Roams with one profile. | +| `profile_client` | `(user_id, profile_id, client_family)` | Roams between like clients: `tv`, `mobile`, `tablet`, `desktop`, or `web`. | | `profile_device` | `(user_id, profile_id, device_id)` | Override for one profile on one device identity. | | `profile_library` | `(user_id, profile_id, library_id)` | Content preference for one library. | | `profile_series` | `(user_id, profile_id, series_id)` | Content preference for one series. | @@ -423,6 +424,10 @@ The remote scopes are: `client_local` definitions use a single logical `client_local` scope and are never addressed by the server values API. +Clients send the family explicitly as `X-Silo-Client-Family`; the server never infers it from the +free-form `X-Silo-Device-Platform` registry label. tvOS and Android TV use `tv`, phones use +`mobile`, tablets use `tablet`, native desktop clients use `desktop`, and browsers use `web`. + All remote mutations carry their complete identity explicitly. The server authorizes that the profile, library, series, and device belong to the authenticated user. A queued operation must not derive its profile or server from whichever account happens to be active when the retry runs. @@ -448,6 +453,8 @@ Examples: |---|---| | Audio/subtitle selection | series → library → device → profile → default | | Playback behavior with device override | device → profile → default | +| Family-synchronized presentation | device → client family → profile → default | +| Family-synchronized navigation | device → client family → default | | Device playback capability | device → default | | Account UI preference | account → default | | Client-local OS behavior | local value → default | @@ -625,8 +632,10 @@ to check compatibility without transferring the manifest body. - Returns the explicit value and revision at exactly one requested scope; it does not resolve fallbacks. -- Context parameters are required by scope: `profile_id`, `device_id`, `library_id`, or `series_id` - as defined by the identity table above. +- Context parameters are required by scope: `profile_id`, `client_family`, `device_id`, + `library_id`, or `series_id` as defined by the identity table above. Self-service + `profile_client` requests take client family from `X-Silo-Client-Family`; admin explicit-value + routes name it with the `client_family` query parameter. - An unset value is represented as `is_set: false` with no `value` member, never as an empty string or JSON `null`. - Settings screens use this endpoint to show profile defaults and device overrides independently. @@ -637,7 +646,8 @@ to check compatibility without transferring the manifest body. `GET /api/v1/settings/values/effective?keys=` - Requires the active profile and device identity headers for definitions that can resolve those - scopes. + scopes. `X-Silo-Client-Family` is optional for effective reads so older callers remain compatible: + absence skips `profile_client`, a valid value includes it, and a non-empty invalid value is rejected. - Rejects unknown keys rather than fabricating defaults. - Returns native typed values, resolution source, source context, definition revision, `updated_at`, and any policy constraint. @@ -789,6 +799,7 @@ CREATE TABLE user_setting_values ( key text NOT NULL, scope text NOT NULL, profile_id text, + client_family text, device_id text, library_id integer, series_id text, @@ -796,13 +807,15 @@ CREATE TABLE user_setting_values ( revision bigint NOT NULL DEFAULT 1, created_at timestamptz NOT NULL DEFAULT now(), updated_at timestamptz NOT NULL DEFAULT now(), - CHECK (scope IN ('account', 'profile', 'profile_device', 'profile_library', 'profile_series')), + CHECK (scope IN ('account', 'profile', 'profile_client', 'profile_device', 'profile_library', 'profile_series')), + CHECK (client_family IS NULL OR client_family IN ('tv', 'mobile', 'tablet', 'desktop', 'web')), CHECK ( - (scope = 'account' AND profile_id IS NULL AND device_id IS NULL AND library_id IS NULL AND series_id IS NULL) OR - (scope = 'profile' AND profile_id IS NOT NULL AND device_id IS NULL AND library_id IS NULL AND series_id IS NULL) OR - (scope = 'profile_device' AND profile_id IS NOT NULL AND device_id IS NOT NULL AND library_id IS NULL AND series_id IS NULL) OR - (scope = 'profile_library' AND profile_id IS NOT NULL AND device_id IS NULL AND library_id IS NOT NULL AND series_id IS NULL) OR - (scope = 'profile_series' AND profile_id IS NOT NULL AND device_id IS NULL AND library_id IS NULL AND series_id IS NOT NULL) + (scope = 'account' AND profile_id IS NULL AND client_family IS NULL AND device_id IS NULL AND library_id IS NULL AND series_id IS NULL) OR + (scope = 'profile' AND profile_id IS NOT NULL AND client_family IS NULL AND device_id IS NULL AND library_id IS NULL AND series_id IS NULL) OR + (scope = 'profile_client' AND profile_id IS NOT NULL AND client_family IS NOT NULL AND device_id IS NULL AND library_id IS NULL AND series_id IS NULL) OR + (scope = 'profile_device' AND profile_id IS NOT NULL AND client_family IS NULL AND device_id IS NOT NULL AND library_id IS NULL AND series_id IS NULL) OR + (scope = 'profile_library' AND profile_id IS NOT NULL AND client_family IS NULL AND device_id IS NULL AND library_id IS NOT NULL AND series_id IS NULL) OR + (scope = 'profile_series' AND profile_id IS NOT NULL AND client_family IS NULL AND device_id IS NULL AND library_id IS NULL AND series_id IS NOT NULL) ) ); ``` @@ -819,6 +832,8 @@ CREATE UNIQUE INDEX user_setting_values_account_uq ON user_setting_values (user_id, key) WHERE scope = 'account'; CREATE UNIQUE INDEX user_setting_values_profile_uq ON user_setting_values (user_id, profile_id, key) WHERE scope = 'profile'; +CREATE UNIQUE INDEX user_setting_values_profile_client_uq + ON user_setting_values (user_id, profile_id, client_family, key) WHERE scope = 'profile_client'; CREATE UNIQUE INDEX user_setting_values_profile_device_uq ON user_setting_values (user_id, profile_id, device_id, key) WHERE scope = 'profile_device'; CREATE UNIQUE INDEX user_setting_values_profile_library_uq @@ -949,8 +964,19 @@ client. The following decisions resolve today's duplicate semantics: | Date/time format | remote: **profile** | Existing account rows are copied to every profile. | | Search media scope | remote: profile | Preserve strict enums. | | `ui.library_page_state` | remote: profile_device | Keep navigation state tied to one profile/device. | +| `nav.primary_menu` | remote: profile_client, profile_device | Family menu ordering/visibility with an optional exact-device escape hatch; null means use the family-native baseline. | +| `nav.shortcuts` | remote: profile | Profile-wide catalog of library, section, and collection pins; seed convertible legacy `ui.sidebar_pins` without deleting the old row. | +| `ui.card_presentation` | remote: profile, profile_client, profile_device | Semantic size/caption presets; device → family → profile → default. | | OS caption mirroring, platform decoder diagnostics, temporary sleep timers | client_local or private `local.*` | Production caption-mirroring UI is contract-known local; diagnostics/timers remain private local. | +Navigation destinations are unique by semantic identity, not by the complete JSON object: built-ins +use `destination`, libraries use `library_id`, sections use `(library_id, section_id)`, and collections +use `(optional library_id, collection_id)`. Changing a label therefore cannot create a duplicate menu +entry. Labels and string target ids must contain a non-whitespace character; labels are bounded to +256 characters and target ids to 128. The JSON schemas document these rules and the canonical +validator enforces the cross-item identity check that JSON Schema `uniqueItems` cannot express by +itself. + ### Appearance belongs to the profile, not the account Theme, text scale, contrast, custom CSS, and date/time format are stored today in `user_settings` diff --git a/internal/api/handlers/settings.go b/internal/api/handlers/settings.go index c6d299e5..6c219e17 100644 --- a/internal/api/handlers/settings.go +++ b/internal/api/handlers/settings.go @@ -46,6 +46,7 @@ const ( deviceIDHeader = "X-Silo-Device-Id" deviceNameHeader = "X-Silo-Device-Name" devicePlatformHeader = "X-Silo-Device-Platform" + clientFamilyHeader = "X-Silo-Client-Family" ) // ServerSettingReader reads individual keys from the server_settings table. diff --git a/internal/api/handlers/settings_audit.go b/internal/api/handlers/settings_audit.go index fa30cdfd..2839c58d 100644 --- a/internal/api/handlers/settings_audit.go +++ b/internal/api/handlers/settings_audit.go @@ -23,7 +23,10 @@ import ( // activitylog is an HTTP request-log middleware whose schema has no profile or // body, so it cannot express "actor P changed key K for profile Q". Persisting // these needs its own table and migration. -const settingsAuditMsg = "settings changed for another profile" +const ( + settingsAuditMsg = "settings changed for another profile" + settingsAuditActionSet = "set" +) // logComponentKey is the structured-log attribute every handler in this package // tags itself with. @@ -37,6 +40,7 @@ type settingsAuditRecord struct { // actor's own account only on the admin routes, where profile ids alone // would not say whose settings moved. TargetUserID int + ClientFamily string DeviceID string Key string Scope string @@ -67,6 +71,9 @@ func auditSettingsForOther(ctx context.Context, record settingsAuditRecord) { if record.Scope != "" { attrs = append(attrs, "scope", record.Scope) } + if record.ClientFamily != "" { + attrs = append(attrs, "client_family", record.ClientFamily) + } if record.DeviceID != "" { attrs = append(attrs, "device_id", record.DeviceID) } diff --git a/internal/api/handlers/settings_values.go b/internal/api/handlers/settings_values.go index 0a12d695..548e9e22 100644 --- a/internal/api/handlers/settings_values.go +++ b/internal/api/handlers/settings_values.go @@ -7,6 +7,7 @@ import ( "encoding/hex" "encoding/json" "errors" + "fmt" "io" "log/slog" "net/http" @@ -118,35 +119,99 @@ const fieldValues = "values" // the request boundary keeps a crafted batch from failing resolution outright. const maxEffectiveContentIDs = 200 +// maxShortcutMutationRetries bounds contention retries while still making a +// normal burst of edits effectively wait-free for clients. Each failed CAS +// means another writer made progress; exhausting this limit is therefore a +// retryable conflict, never permission to overwrite the newer document. +const maxShortcutMutationRetries = 32 + const jsonNullLiteral = "null" +const navigationShortcutAtomicUpdateMessage = "Use PUT /settings/values/nav.shortcuts/item to change navigation shortcuts" + +var ( + errMutationIDConflict = errors.New("setting mutation id conflict") + errMutationReplayRollback = errors.New("setting mutation replay requires rollback") + errMutationTransactionRequired = errors.New("settings store does not support atomic idempotent mutations") + errShortcutMutationContention = errors.New("navigation shortcuts changed too quickly") + errShortcutMutationInvalidValue = errors.New("invalid navigation shortcut value") +) + +type idempotentSettingMutationOutcome struct { + result json.RawMessage + stored *userstore.SettingValue + replay bool + changed bool +} + +type shortcutMutationStore interface { + GetSettingValue(context.Context, userstore.SettingIdentity) (*userstore.SettingValue, error) + CompareAndSetSettingValue( + context.Context, + userstore.SettingIdentity, + json.RawMessage, + int64, + ) (*userstore.SettingValue, error) +} + // settingValueResponse is one explicit stored value. type settingValueResponse struct { - Key string `json:"key"` - Scope string `json:"scope"` - ProfileID string `json:"profile_id,omitempty"` - DeviceID string `json:"device_id,omitempty"` - LibraryID int `json:"library_id,omitempty"` - SeriesID string `json:"series_id,omitempty"` - Value json.RawMessage `json:"value"` - Revision int64 `json:"revision"` - UpdatedAt string `json:"updated_at,omitempty"` + Key string `json:"key"` + Scope string `json:"scope"` + ProfileID string `json:"profile_id,omitempty"` + ClientFamily string `json:"client_family,omitempty"` + DeviceID string `json:"device_id,omitempty"` + LibraryID int `json:"library_id,omitempty"` + SeriesID string `json:"series_id,omitempty"` + Value json.RawMessage `json:"value"` + Revision int64 `json:"revision"` + UpdatedAt string `json:"updated_at,omitempty"` +} + +type navigationShortcutMutationRequest struct { + Item json.RawMessage `json:"item"` + Present *bool `json:"present"` +} + +type navigationShortcutDocument struct { + Items []navigationShortcutItem `json:"items"` +} + +// navigationShortcutItem mirrors navigation-shortcuts.json. LibraryID is a +// pointer because its presence is part of collection identity: a global +// collection and a library-specific collection with the same collection_id +// are different destinations. +type navigationShortcutItem struct { + Type string `json:"type"` + LibraryID *int `json:"library_id,omitempty"` + SectionID string `json:"section_id,omitempty"` + CollectionID string `json:"collection_id,omitempty"` + Label string `json:"label"` +} + +type navigationShortcutIdentity struct { + Type string + LibraryID int + HasLibraryID bool + SectionID string + CollectionID string } // explicitSettingValueResponse is one entry from the collection GET. Unset is // represented explicitly rather than as a 404 so a settings screen can fetch // several profile defaults or device overrides in one request. type explicitSettingValueResponse struct { - Key string `json:"key"` - Scope string `json:"scope"` - ProfileID string `json:"profile_id,omitempty"` - DeviceID string `json:"device_id,omitempty"` - LibraryID int `json:"library_id,omitempty"` - SeriesID string `json:"series_id,omitempty"` - IsSet bool `json:"is_set"` - Value json.RawMessage `json:"value,omitempty"` - Revision int64 `json:"revision,omitempty"` - UpdatedAt string `json:"updated_at,omitempty"` + Key string `json:"key"` + Scope string `json:"scope"` + ProfileID string `json:"profile_id,omitempty"` + ClientFamily string `json:"client_family,omitempty"` + DeviceID string `json:"device_id,omitempty"` + LibraryID int `json:"library_id,omitempty"` + SeriesID string `json:"series_id,omitempty"` + IsSet bool `json:"is_set"` + Value json.RawMessage `json:"value,omitempty"` + Revision int64 `json:"revision,omitempty"` + UpdatedAt string `json:"updated_at,omitempty"` } // effectiveSettingValueResponse is one resolved value plus where it came from. @@ -176,18 +241,20 @@ type effectiveSettingValueResponse struct { // Scope locates the row the value came from, so a client can offer a reset // against exactly that scope. Empty for a contract default. - Scope string `json:"scope,omitempty"` - ProfileID string `json:"profile_id,omitempty"` - DeviceID string `json:"device_id,omitempty"` - LibraryID int `json:"library_id,omitempty"` - SeriesID string `json:"series_id,omitempty"` + Scope string `json:"scope,omitempty"` + ProfileID string `json:"profile_id,omitempty"` + ClientFamily string `json:"client_family,omitempty"` + DeviceID string `json:"device_id,omitempty"` + LibraryID int `json:"library_id,omitempty"` + SeriesID string `json:"series_id,omitempty"` } type effectiveSourceContextResponse struct { - ProfileID string `json:"profile_id,omitempty"` - DeviceID string `json:"device_id,omitempty"` - LibraryID int `json:"library_id,omitempty"` - SeriesID string `json:"series_id,omitempty"` + ProfileID string `json:"profile_id,omitempty"` + ClientFamily string `json:"client_family,omitempty"` + DeviceID string `json:"device_id,omitempty"` + LibraryID int `json:"library_id,omitempty"` + SeriesID string `json:"series_id,omitempty"` } // HandleGetContract serves the public manifest. @@ -235,12 +302,21 @@ func (h *SettingValuesHandler) HandleGetCapabilities(w http.ResponseWriter, r *h "scopes": []string{ string(settingscontract.ScopeAccount), string(settingscontract.ScopeProfile), + string(settingscontract.ScopeProfileClient), string(settingscontract.ScopeProfileDevice), string(settingscontract.ScopeProfileLibrary), string(settingscontract.ScopeProfileSeries), }, + "client_families": []string{ + string(settingscontract.ClientFamilyTV), + string(settingscontract.ClientFamilyMobile), + string(settingscontract.ClientFamilyTablet), + string(settingscontract.ClientFamilyDesktop), + string(settingscontract.ClientFamilyWeb), + }, "supports_batched_effective": true, "supports_idempotent_writes": true, + "supports_atomic_shortcuts": true, }) } @@ -306,6 +382,9 @@ func (h *SettingValuesHandler) HandleGetValues(w http.ResponseWriter, r *http.Re switch identity.Scope { case settingscontract.ScopeProfile: query.ProfileIDs = []string{identity.ProfileID} + case settingscontract.ScopeProfileClient: + query.ProfileIDs = []string{identity.ProfileID} + query.ClientFamily = identity.ClientFamily case settingscontract.ScopeProfileDevice: query.ProfileIDs = []string{identity.ProfileID} query.DeviceID = identity.DeviceID @@ -332,7 +411,8 @@ func (h *SettingValuesHandler) HandleGetValues(w http.ResponseWriter, r *http.Re for _, key := range keys { entry := explicitSettingValueResponse{ Key: key, Scope: string(identity.Scope), ProfileID: identity.ProfileID, - DeviceID: identity.DeviceID, LibraryID: identity.LibraryID, SeriesID: identity.SeriesID, + ClientFamily: string(identity.ClientFamily), DeviceID: identity.DeviceID, + LibraryID: identity.LibraryID, SeriesID: identity.SeriesID, } if value, exists := byKey[key]; exists { entry.IsSet = true @@ -363,9 +443,333 @@ func (h *SettingValuesHandler) HandleSetValue(w http.ResponseWriter, r *http.Req if !ok { return } + if identity.Key == settingskeys.NavShortcuts { + writeError(w, http.StatusBadRequest, "atomic_update_required", + navigationShortcutAtomicUpdateMessage) + return + } h.setValueAt(w, r, store, apimw.GetUserID(r.Context()), identity) } +// HandleSetNavigationShortcut applies one desired-state edit to the shared +// profile shortcut catalog. Unlike the generic whole-document PUT, two clients +// adding different destinations cannot overwrite one another: the handler +// rebases after an internal compare-and-set conflict until its semantic edit +// lands on the newest document. +func (h *SettingValuesHandler) HandleSetNavigationShortcut(w http.ResponseWriter, r *http.Request) { + store, ok := h.storeFor(w, r) + if !ok { + return + } + shortcutStore, ok := store.(shortcutMutationStore) + if !ok { + writeError(w, http.StatusInternalServerError, "internal_error", + "This settings store does not support atomic shortcut updates") + return + } + + profileID := strings.TrimSpace(apimw.GetProfileID(r.Context())) + identity := userstore.SettingIdentity{ + Key: settingskeys.NavShortcuts, Scope: settingscontract.ScopeProfile, ProfileID: profileID, + } + if err := identity.Validate(); err != nil { + writeError(w, http.StatusBadRequest, "bad_request", "X-Profile-Id header is required") + return + } + def, ok := h.definitionFor(w, identity.Key) + if !ok { + return + } + + var body navigationShortcutMutationRequest + decoder := json.NewDecoder(http.MaxBytesReader(w, r.Body, 1<<20)) + decoder.DisallowUnknownFields() + if err := decoder.Decode(&body); err != nil { + writeError(w, http.StatusBadRequest, "bad_request", + "Body must be {\"item\": {…}, \"present\": true|false}") + return + } + if _, err := decoder.Token(); !errors.Is(err, io.EOF) { + writeError(w, http.StatusBadRequest, "bad_request", "Body must be a single JSON document") + return + } + if body.Present == nil { + writeError(w, http.StatusBadRequest, "bad_request", "present is required") + return + } + + item, err := normalizeNavigationShortcutItem(def, body.Item) + if err != nil { + writeError(w, http.StatusBadRequest, "invalid_value", err.Error()) + return + } + requestHash := hashNavigationShortcutMutation(identity, item, *body.Present) + mutationID := strings.TrimSpace(r.Header.Get(mutationIDHeader)) + + var stored *userstore.SettingValue + var idempotentResult json.RawMessage + var changed bool + if mutationID == "" { + stored, changed, err = mutateNavigationShortcut( + r.Context(), shortcutStore, def, identity, item, *body.Present) + } else { + var outcome idempotentSettingMutationOutcome + outcome, err = runIdempotentSettingMutation( + r.Context(), store, mutationID, requestHash, + func(writer userstore.SettingMutationWriter) (*userstore.SettingValue, bool, error) { + return mutateNavigationShortcut(r.Context(), writer, def, identity, item, *body.Present) + }, + ) + if err == nil && outcome.replay { + w.Header().Set("X-Silo-Idempotent-Replay", "true") + writeRawJSON(w, http.StatusOK, outcome.result) + return + } + stored, changed = outcome.stored, outcome.changed + idempotentResult = outcome.result + } + if err != nil { + switch { + case errors.Is(err, errMutationIDConflict): + writeError(w, http.StatusConflict, "mutation_id_conflict", + "This mutation id was used for a different write") + case errors.Is(err, errShortcutMutationContention): + writeError(w, http.StatusConflict, "setting_update_conflict", + "Navigation shortcuts changed too quickly; retry this mutation") + case errors.Is(err, errShortcutMutationInvalidValue): + writeError(w, http.StatusBadRequest, "invalid_value", err.Error()) + case errors.Is(err, userstore.ErrInvalidSettingIdentity), + errors.Is(err, userstore.ErrInvalidSettingValue): + writeError(w, http.StatusBadRequest, "bad_request", err.Error()) + default: + writeError(w, http.StatusInternalServerError, "internal_error", "Failed to store navigation shortcuts") + } + return + } + + response := settingValueToResponse(*stored) + if changed { + publishUserSettingsEvent(r.Context(), h.EventsHub, + apimw.GetUserID(r.Context()), identity.ProfileID, identity.Key, string(identity.Scope)) + auditSettingsForOther(r.Context(), settingsAuditRecord{ + Action: settingsAuditActionSet, + ActorProfileID: actingProfileID(r.Context()), + TargetProfileID: identity.ProfileID, + TargetUserID: apimw.GetUserID(r.Context()), + Key: identity.Key, + Scope: string(identity.Scope), + }) + } + if idempotentResult != nil { + writeRawJSON(w, http.StatusOK, idempotentResult) + } else { + writeJSON(w, http.StatusOK, response) + } +} + +func normalizeNavigationShortcutItem( + def *settingscontract.Definition, + item json.RawMessage, +) (navigationShortcutItem, error) { + if len(item) == 0 { + return navigationShortcutItem{}, errors.New("item is required") + } + raw, err := json.Marshal(struct { + Items []json.RawMessage `json:"items"` + }{Items: []json.RawMessage{item}}) + if err != nil { + return navigationShortcutItem{}, fmt.Errorf("encoding shortcut: %w", err) + } + normalized, err := def.ValueSchema.NormalizeValue(raw, settingscontract.ObjectSchemas()) + if err != nil { + return navigationShortcutItem{}, err + } + var document navigationShortcutDocument + if err := json.Unmarshal(normalized, &document); err != nil || len(document.Items) != 1 { + return navigationShortcutItem{}, errors.New("shortcut did not normalize to one item") + } + return document.Items[0], nil +} + +func applyNavigationShortcutMutation( + def *settingscontract.Definition, + current *userstore.SettingValue, + item navigationShortcutItem, + present bool, +) (json.RawMessage, bool, error) { + document := navigationShortcutDocument{Items: []navigationShortcutItem{}} + if current != nil { + if err := json.Unmarshal(current.Value, &document); err != nil { + return nil, false, fmt.Errorf("decoding stored navigation shortcuts: %w", err) + } + } + + target := item.identity() + match := -1 + for index, candidate := range document.Items { + if candidate.identity() == target { + match = index + break + } + } + + if present { + if match >= 0 { + if document.Items[match].equal(item) { + return current.Value, false, nil + } + document.Items[match] = item + } else { + document.Items = append(document.Items, item) + } + } else { + if match < 0 { + if current == nil { + return json.RawMessage(`{"items":[]}`), false, nil + } + return current.Value, false, nil + } + document.Items = append(document.Items[:match], document.Items[match+1:]...) + } + + raw, err := json.Marshal(document) + if err != nil { + return nil, false, fmt.Errorf("encoding navigation shortcuts: %w", err) + } + normalized, err := def.ValueSchema.NormalizeValue(raw, settingscontract.ObjectSchemas()) + if err != nil { + return nil, false, fmt.Errorf("%w: %w", errShortcutMutationInvalidValue, err) + } + return normalized, true, nil +} + +func (item navigationShortcutItem) identity() navigationShortcutIdentity { + identity := navigationShortcutIdentity{ + Type: item.Type, SectionID: item.SectionID, CollectionID: item.CollectionID, + } + if item.LibraryID != nil { + identity.LibraryID = *item.LibraryID + identity.HasLibraryID = true + } + return identity +} + +func (item navigationShortcutItem) equal(other navigationShortcutItem) bool { + return item.identity() == other.identity() && item.Label == other.Label +} + +func mutateNavigationShortcut( + ctx context.Context, + store shortcutMutationStore, + def *settingscontract.Definition, + identity userstore.SettingIdentity, + item navigationShortcutItem, + present bool, +) (*userstore.SettingValue, bool, error) { + for attempt := 0; attempt < maxShortcutMutationRetries; attempt++ { + current, err := store.GetSettingValue(ctx, identity) + if err != nil { + return nil, false, fmt.Errorf("reading navigation shortcuts: %w", err) + } + + next, changed, err := applyNavigationShortcutMutation(def, current, item, present) + if err != nil { + return nil, false, err + } + if !changed { + if current != nil { + return current, false, nil + } + return &userstore.SettingValue{ + SettingIdentity: identity, + Value: json.RawMessage(`{"items":[]}`), + }, false, nil + } + + expectedRevision := int64(0) + if current != nil { + expectedRevision = current.Revision + } + stored, err := store.CompareAndSetSettingValue(ctx, identity, next, expectedRevision) + if errors.Is(err, userstore.ErrSettingValueRevisionConflict) { + continue + } + if err != nil { + return nil, false, err + } + return stored, true, nil + } + return nil, false, errShortcutMutationContention +} + +func runIdempotentSettingMutation( + ctx context.Context, + store userstore.UserStore, + mutationID string, + requestHash string, + mutate func(userstore.SettingMutationWriter) (*userstore.SettingValue, bool, error), +) (idempotentSettingMutationOutcome, error) { + transactioner, ok := store.(userstore.SettingMutationTransactioner) + if !ok { + return idempotentSettingMutationOutcome{}, errMutationTransactionRequired + } + + var outcome idempotentSettingMutationOutcome + err := transactioner.WithSettingMutationTransaction(ctx, mutationID, + func(writer userstore.SettingMutationWriter) error { + prior, err := writer.GetSettingMutation(ctx, mutationID) + if err != nil { + return fmt.Errorf("checking setting mutation: %w", err) + } + if prior != nil { + if prior.RequestHash != requestHash { + return errMutationIDConflict + } + outcome.result = slices.Clone(prior.Result) + outcome.replay = true + return nil + } + + stored, changed, err := mutate(writer) + if err != nil { + return err + } + response := settingValueToResponse(*stored) + result, err := json.Marshal(response) + if err != nil { + return fmt.Errorf("encoding setting mutation receipt: %w", err) + } + record, inserted, err := writer.PutSettingMutation(ctx, userstore.SettingMutationRecord{ + MutationID: mutationID, + RequestHash: requestHash, + Result: result, + ExpiresAt: time.Now().UTC().Add(30 * 24 * time.Hour), + }) + if err != nil { + return fmt.Errorf("recording setting mutation: %w", err) + } + if !inserted { + if record.RequestHash != requestHash { + return errMutationIDConflict + } + outcome.result = slices.Clone(record.Result) + outcome.replay = true + // A legacy writer could have inserted between the initial read and + // this insert. Roll back our setting write before serving its receipt. + return errMutationReplayRollback + } + + outcome.result = slices.Clone(record.Result) + outcome.stored = stored + outcome.changed = changed + return nil + }) + if errors.Is(err, errMutationReplayRollback) { + return outcome, nil + } + return outcome, err +} + // setValueAt is the write path shared by the session route and the admin // route: validation, normalization, idempotency and the change event are one // implementation regardless of who addresses the store. eventUserID names the @@ -413,27 +817,34 @@ func (h *SettingValuesHandler) setValueAt( // not double-apply it, and must be able to tell "already done" from "that // id means something else". mutationID := strings.TrimSpace(r.Header.Get(mutationIDHeader)) - var requestHash string - if mutationID != "" { - requestHash = hashMutationRequest(identity, normalized) - prior, err := store.GetSettingMutation(r.Context(), mutationID) - if err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", "Failed to check the mutation id") + var stored *userstore.SettingValue + var idempotentResult json.RawMessage + if mutationID == "" { + stored, err = store.UpsertSettingValue(r.Context(), identity, normalized) + } else { + outcome, mutationErr := runIdempotentSettingMutation( + r.Context(), store, mutationID, hashMutationRequest(identity, normalized), + func(writer userstore.SettingMutationWriter) (*userstore.SettingValue, bool, error) { + value, err := writer.UpsertSettingValue(r.Context(), identity, normalized) + return value, true, err + }, + ) + if errors.Is(mutationErr, errMutationIDConflict) { + writeError(w, http.StatusConflict, "mutation_id_conflict", + "This mutation id was used for a different write") return } - if prior != nil { - if prior.RequestHash != requestHash { - writeError(w, http.StatusConflict, "mutation_id_conflict", - "This mutation id was used for a different write") - return - } + if mutationErr != nil { + err = mutationErr + } else if outcome.replay { w.Header().Set("X-Silo-Idempotent-Replay", "true") - writeRawJSON(w, http.StatusOK, prior.Result) + writeRawJSON(w, http.StatusOK, outcome.result) return + } else { + stored = outcome.stored + idempotentResult = outcome.result } } - - stored, err := store.UpsertSettingValue(r.Context(), identity, normalized) if err != nil { if errors.Is(err, userstore.ErrInvalidSettingIdentity) || errors.Is(err, userstore.ErrInvalidSettingValue) { @@ -445,14 +856,6 @@ func (h *SettingValuesHandler) setValueAt( } response := settingValueToResponse(*stored) - if mutationID != "" { - // Recorded only now, after the write landed: a receipt written for a - // failed upsert would turn the client's retry of a 500 into a silent - // 200 replay of a write that never happened. Storing the actual - // response also makes the replay byte-identical — revision and - // updated_at included — rather than a reconstruction of the input. - h.recordMutation(r, store, mutationID, requestHash, response) - } acting := actingProfileID(r.Context()) if identity.Scope == settingscontract.ScopeProfileDevice { // A device that only ever writes canonically must still appear in @@ -472,15 +875,20 @@ func (h *SettingValuesHandler) setValueAt( publishUserSettingsEvent(r.Context(), h.EventsHub, eventUserID, identity.ProfileID, identity.Key, string(identity.Scope)) auditSettingsForOther(r.Context(), settingsAuditRecord{ - Action: "set", + Action: settingsAuditActionSet, ActorProfileID: acting, TargetProfileID: identity.ProfileID, TargetUserID: eventUserID, + ClientFamily: string(identity.ClientFamily), DeviceID: identity.DeviceID, Key: identity.Key, Scope: string(identity.Scope), }) - writeJSON(w, http.StatusOK, response) + if idempotentResult != nil { + writeRawJSON(w, http.StatusOK, idempotentResult) + } else { + writeJSON(w, http.StatusOK, response) + } } // registerWritingDevice refreshes the device registry from the request's @@ -531,6 +939,11 @@ func (h *SettingValuesHandler) HandleDeleteValue(w http.ResponseWriter, r *http. if !ok { return } + if identity.Key == settingskeys.NavShortcuts { + writeError(w, http.StatusBadRequest, "atomic_update_required", + navigationShortcutAtomicUpdateMessage) + return + } h.deleteValueAt(w, r, store, apimw.GetUserID(r.Context()), identity) } @@ -557,6 +970,7 @@ func (h *SettingValuesHandler) deleteValueAt( ActorProfileID: actingProfileID(r.Context()), TargetProfileID: identity.ProfileID, TargetUserID: eventUserID, + ClientFamily: string(identity.ClientFamily), DeviceID: identity.DeviceID, Key: identity.Key, Scope: string(identity.Scope), @@ -608,6 +1022,11 @@ func (h *SettingValuesHandler) HandleGetEffective(w http.ResponseWriter, r *http LibraryIDs: parseIntCSV(r.URL.Query().Get("library_ids")), SeriesIDs: splitCSV(r.URL.Query().Get("series_ids")), } + if family, needed, ok := h.clientFamilyForKeys(w, r, keys); !ok { + return + } else if needed { + rc.ClientFamily = family + } // A device-settings screen resolves what some *other* device sees, so this // read accepts the same explicit identity the write path does, under the @@ -714,6 +1133,10 @@ func (h *SettingValuesHandler) HandlePostEffective(w http.ResponseWriter, r *htt profileID := strings.TrimSpace(apimw.GetProfileID(r.Context())) deviceID := deviceMetadataFromRequest(r).DeviceID + clientFamily, familyNeeded, ok := h.clientFamilyForKeys(w, r, keys) + if !ok { + return + } if deviceID == "" { for _, key := range keys { if def, exists := h.contract.Lookup(key); exists && def.AllowsScope(settingscontract.ScopeProfileDevice) { @@ -749,6 +1172,9 @@ func (h *SettingValuesHandler) HandlePostEffective(w http.ResponseWriter, r *htt return } rc := settingsresolve.Context{ProfileID: profileID, DeviceID: deviceID} + if familyNeeded { + rc.ClientFamily = clientFamily + } if libraryID > 0 { rc.LibraryIDs = []int{libraryID} contentIDs++ @@ -862,28 +1288,6 @@ func (h *SettingValuesHandler) constraintsFor(r *http.Request) settingsresolve.C return settingsresolve.Constraints{policyInputMaxPlaybackQuality: limit} } -// recordMutation stores the idempotency receipt after a successful write. The -// receipt is the response the original request returned, so a replay serves -// exactly what the client would have received. -func (h *SettingValuesHandler) recordMutation( - r *http.Request, - store userstore.UserStore, - mutationID, requestHash string, - response settingValueResponse, -) { - receipt, _ := json.Marshal(response) - // Best effort: the write already succeeded, and failing the request now - // would tell the client the opposite of the truth. A missing receipt costs - // at most a duplicate write on retry, which upsert makes harmless. - _, _, _ = store.PutSettingMutation(r.Context(), userstore.SettingMutationRecord{ - MutationID: mutationID, - RequestHash: requestHash, - Result: receipt, - CreatedAt: time.Now().UTC(), - ExpiresAt: time.Now().UTC().Add(30 * 24 * time.Hour), - }) -} - func (h *SettingValuesHandler) storeFor(w http.ResponseWriter, r *http.Request) (userstore.UserStore, bool) { store, err := h.storeProvider.ForUser(r.Context(), apimw.GetUserID(r.Context())) if err != nil { @@ -908,6 +1312,37 @@ func (h *SettingValuesHandler) definitionFor(w http.ResponseWriter, key string) return def, true } +// clientFamilyForKeys validates an optional family header for effective reads. +// An absent header deliberately drops the profile_client layer so pre-revision +// 5 callers keep resolving broader fallbacks; explicit profile_client reads and +// writes still require the header in identityForSessionKey. The server never +// guesses this identity from X-Silo-Device-Platform: that header is free-form +// display metadata, while client_family is a closed storage key shared by like +// clients. +func (h *SettingValuesHandler) clientFamilyForKeys( + w http.ResponseWriter, r *http.Request, keys []string, +) (settingscontract.ClientFamily, bool, bool) { + eligible := false + for _, key := range keys { + if def, ok := h.contract.Lookup(key); ok && def.AllowsScope(settingscontract.ScopeProfileClient) { + eligible = true + break + } + } + + value := strings.TrimSpace(r.Header.Get(clientFamilyHeader)) + if value == "" { + return "", false, true + } + family := settingscontract.ClientFamily(value) + if !family.Valid() { + writeError(w, http.StatusBadRequest, "bad_request", + "X-Silo-Client-Family header must be one of tv, mobile, tablet, desktop or web") + return "", false, false + } + return family, eligible, true +} + // identityFromRequest builds and validates the scope identity a request names. // // Scope comes from the query string rather than the path so one route serves @@ -947,6 +1382,15 @@ func (h *SettingValuesHandler) identityForSessionKey( identity.ProfileID = named } } + if scope == settingscontract.ScopeProfileClient { + family := settingscontract.ClientFamily(strings.TrimSpace(r.Header.Get(clientFamilyHeader))) + if !family.Valid() { + writeError(w, http.StatusBadRequest, "bad_request", + "X-Silo-Client-Family header must be one of tv, mobile, tablet, desktop or web") + return userstore.SettingIdentity{}, false + } + identity.ClientFamily = family + } if scope == settingscontract.ScopeProfileDevice { // A device may be named explicitly so one device can manage another's // settings — the screen that lists your devices edits them in place. @@ -1064,7 +1508,7 @@ func (h *SettingValuesHandler) keyedScope( scope := settingscontract.Scope(strings.TrimSpace(query.Get("scope"))) if scope == "" { writeError(w, http.StatusBadRequest, "bad_request", - "A scope is required: account, profile, profile_device, profile_library or profile_series") + "A scope is required: account, profile, profile_client, profile_device, profile_library or profile_series") return "", "", false } return key, scope, true @@ -1133,20 +1577,22 @@ func (h *SettingValuesHandler) libraryContextExists( func sameSettingContext(a, b userstore.SettingIdentity) bool { return a.Scope == b.Scope && a.ProfileID == b.ProfileID && - a.DeviceID == b.DeviceID && a.LibraryID == b.LibraryID && a.SeriesID == b.SeriesID + a.ClientFamily == b.ClientFamily && a.DeviceID == b.DeviceID && + a.LibraryID == b.LibraryID && a.SeriesID == b.SeriesID } func settingValueToResponse(value userstore.SettingValue) settingValueResponse { return settingValueResponse{ - Key: value.Key, - Scope: string(value.Scope), - ProfileID: value.ProfileID, - DeviceID: value.DeviceID, - LibraryID: value.LibraryID, - SeriesID: value.SeriesID, - Value: value.Value, - Revision: value.Revision, - UpdatedAt: value.UpdatedAt, + Key: value.Key, + Scope: string(value.Scope), + ProfileID: value.ProfileID, + ClientFamily: string(value.ClientFamily), + DeviceID: value.DeviceID, + LibraryID: value.LibraryID, + SeriesID: value.SeriesID, + Value: value.Value, + Revision: value.Revision, + UpdatedAt: value.UpdatedAt, } } @@ -1169,14 +1615,16 @@ func effectiveToResponse(eff settingsresolve.Effective) effectiveSettingValueRes if eff.Identity != nil { out.Scope = string(eff.Identity.Scope) out.ProfileID = eff.Identity.ProfileID + out.ClientFamily = string(eff.Identity.ClientFamily) out.DeviceID = eff.Identity.DeviceID out.LibraryID = eff.Identity.LibraryID out.SeriesID = eff.Identity.SeriesID out.SourceContext = &effectiveSourceContextResponse{ - ProfileID: eff.Identity.ProfileID, - DeviceID: eff.Identity.DeviceID, - LibraryID: eff.Identity.LibraryID, - SeriesID: eff.Identity.SeriesID, + ProfileID: eff.Identity.ProfileID, + ClientFamily: string(eff.Identity.ClientFamily), + DeviceID: eff.Identity.DeviceID, + LibraryID: eff.Identity.LibraryID, + SeriesID: eff.Identity.SeriesID, } } return out @@ -1307,9 +1755,36 @@ func hashMutationRequest(identity userstore.SettingIdentity, value json.RawMessa sum.Write([]byte(identity.SeriesID)) sum.Write([]byte{0}) sum.Write(value) + // Preserve the established fingerprint for the five pre-existing scopes so + // an in-flight retry made across this server upgrade still replays. Only the + // new profile_client identity appends a family discriminator. + if identity.ClientFamily != "" { + sum.Write([]byte{0}) + sum.Write([]byte(identity.ClientFamily)) + } return hex.EncodeToString(sum.Sum(nil)) } +func hashNavigationShortcutMutation( + identity userstore.SettingIdentity, + item navigationShortcutItem, + present bool, +) string { + // Removing a destination ignores presentation fields, so the fingerprint + // does too. Reusing one mutation id for the same remove with a refreshed + // label is the same operation; adding includes the label because it can + // update that field in place. + if !present { + item.Label = "" + } + canonical, _ := json.Marshal(struct { + Operation string `json:"operation"` + Item navigationShortcutItem `json:"item"` + Present bool `json:"present"` + }{Operation: "set_navigation_shortcut_presence", Item: item, Present: present}) + return hashMutationRequest(identity, canonical) +} + func writeRawJSON(w http.ResponseWriter, status int, body []byte) { w.Header().Set("Content-Type", "application/json") w.WriteHeader(status) diff --git a/internal/api/handlers/settings_values_admin.go b/internal/api/handlers/settings_values_admin.go index 079088ed..355e37e0 100644 --- a/internal/api/handlers/settings_values_admin.go +++ b/internal/api/handlers/settings_values_admin.go @@ -5,9 +5,12 @@ import ( "strings" "github.com/Silo-Server/silo-server/internal/settingscontract" + "github.com/Silo-Server/silo-server/internal/settingskeys" "github.com/Silo-Server/silo-server/internal/userstore" ) +const adminNavigationShortcutRepairMessage = "Use whole-document PUT on this admin resource to clear or replace navigation shortcuts" + // Admin projections of the canonical settings API. These replace the string // registry's /admin/users/{id}/settings* and device-settings* routes with the // same typed surface clients use on /settings/values: the same validation, the @@ -81,6 +84,11 @@ func (h *SettingValuesHandler) HandleAdminDeleteUserSettingValue(w http.Response if !ok { return } + if identity.Key == settingskeys.NavShortcuts { + writeError(w, http.StatusBadRequest, "atomic_update_required", + adminNavigationShortcutRepairMessage) + return + } h.deleteValueAt(w, r, store, userID, identity) } @@ -128,6 +136,14 @@ func (h *SettingValuesHandler) adminIdentityFromRequest( return userstore.SettingIdentity{}, false } } + if scope == settingscontract.ScopeProfileClient { + identity.ClientFamily = settingscontract.ClientFamily(strings.TrimSpace(query.Get("client_family"))) + if !identity.ClientFamily.Valid() { + writeError(w, http.StatusBadRequest, "bad_request", + "client_family must be one of tv, mobile, tablet, desktop or web") + return userstore.SettingIdentity{}, false + } + } if scope == settingscontract.ScopeProfileDevice { identity.DeviceID = strings.TrimSpace(query.Get("device_id")) if identity.DeviceID == "" { diff --git a/internal/api/handlers/settings_values_admin_test.go b/internal/api/handlers/settings_values_admin_test.go index 8bfad24a..d8e9b819 100644 --- a/internal/api/handlers/settings_values_admin_test.go +++ b/internal/api/handlers/settings_values_admin_test.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "encoding/json" + "errors" "net/http" "net/http/httptest" "testing" @@ -129,6 +130,10 @@ func TestAdminListShowsAnotherUsersValuesAcrossScopes(t *testing.T) { settingscontract.ScopeProfile: { Key: "playback.subtitle_mode", Scope: settingscontract.ScopeProfile, ProfileID: "profile-1", }, + settingscontract.ScopeProfileClient: { + Key: "nav.primary_menu", Scope: settingscontract.ScopeProfileClient, + ProfileID: "profile-1", ClientFamily: settingscontract.ClientFamilyTV, + }, settingscontract.ScopeProfileDevice: { Key: "playback.subtitle_language", Scope: settingscontract.ScopeProfileDevice, ProfileID: "profile-1", DeviceID: "tv-1", @@ -145,6 +150,7 @@ func TestAdminListShowsAnotherUsersValuesAcrossScopes(t *testing.T) { values := map[settingscontract.Scope]string{ settingscontract.ScopeAccount: `"de"`, settingscontract.ScopeProfile: `"always"`, + settingscontract.ScopeProfileClient: `{"items":[{"type":"builtin","destination":"home"}]}`, settingscontract.ScopeProfileDevice: `"en"`, settingscontract.ScopeProfileLibrary: `"fr"`, settingscontract.ScopeProfileSeries: `"ja"`, @@ -186,6 +192,7 @@ func TestAdminListShowsAnotherUsersValuesAcrossScopes(t *testing.T) { continue } if got.Key != want.Key || got.ProfileID != want.ProfileID || + got.ClientFamily != string(want.ClientFamily) || got.DeviceID != want.DeviceID || got.LibraryID != want.LibraryID || got.SeriesID != want.SeriesID { t.Errorf("listed identity at %s = %+v, want %+v", got.Scope, got, want) @@ -243,6 +250,83 @@ func TestAdminSetAndDeleteAtExplicitScopeRoundTrip(t *testing.T) { } } +func TestAdminNavigationShortcutRepairPreservesRevisionHistory(t *testing.T) { + env := newAdminValuesEnv(t) + ctx := context.Background() + identity := userstore.SettingIdentity{ + Key: "nav.shortcuts", Scope: settingscontract.ScopeProfile, ProfileID: "profile-1", + } + seeded, err := env.targetStore.UpsertSettingValue(ctx, identity, + json.RawMessage(`{"items":[{"type":"library","library_id":42,"label":"Movies"}]}`)) + if err != nil { + t.Fatalf("seed shortcuts: %v", err) + } + if seeded.Revision != 1 { + t.Fatalf("seed revision = %d, want 1", seeded.Revision) + } + + target := "/admin/users/7/settings/values/nav.shortcuts?scope=profile&profile_id=profile-1" + if rec := env.do(t, "admin", http.MethodDelete, target, nil); rec.Code != http.StatusBadRequest { + t.Fatalf("admin repair DELETE = %d, want 400: %s", rec.Code, rec.Body.String()) + } else if !bytes.Contains(rec.Body.Bytes(), []byte(`"error":"atomic_update_required"`)) { + t.Fatalf("admin repair DELETE body = %s, want atomic_update_required", rec.Body.String()) + } + if got, err := env.targetStore.GetSettingValue(ctx, identity); err != nil || got == nil || got.Revision != 1 { + t.Fatalf("shortcut after rejected DELETE = %+v err=%v, want revision 1 intact", got, err) + } + + rec := env.do(t, "admin", http.MethodPut, target, []byte(`{"value":{"items":[]}}`)) + if rec.Code != http.StatusOK { + t.Fatalf("admin repair PUT empty = %d: %s", rec.Code, rec.Body.String()) + } + var repaired settingValueResponse + if err := json.Unmarshal(rec.Body.Bytes(), &repaired); err != nil { + t.Fatalf("decode repair response: %v", err) + } + if repaired.Revision != 2 || string(repaired.Value) != `{"items":[]}` { + t.Fatalf("repair response = revision %d value %s, want empty revision 2", + repaired.Revision, repaired.Value) + } + + cas, ok := env.targetStore.(userstore.SettingValueCompareAndSetter) + if !ok { + t.Fatal("target store does not support compare-and-set") + } + _, err = cas.CompareAndSetSettingValue(ctx, identity, + json.RawMessage(`{"items":[{"type":"library","library_id":99,"label":"Shows"}]}`), + seeded.Revision) + if !errors.Is(err, userstore.ErrSettingValueRevisionConflict) { + t.Fatalf("stale CAS after admin repair = %v, want revision conflict", err) + } + if got, err := env.targetStore.GetSettingValue(ctx, identity); err != nil || got == nil || + got.Revision != 2 || string(got.Value) != `{"items":[]}` { + t.Fatalf("shortcut after stale CAS = %+v err=%v, want empty revision 2", got, err) + } +} + +func TestAdminProfileClientUsesExplicitQueryIdentity(t *testing.T) { + env := newAdminValuesEnv(t) + target := "/admin/users/7/settings/values/nav.primary_menu" + + "?scope=profile_client&profile_id=profile-1&client_family=tv" + body := []byte(`{"value":{"items":[{"type":"builtin","destination":"home"}]}}`) + rec := env.do(t, "admin", http.MethodPut, target, body) + if rec.Code != http.StatusOK { + t.Fatalf("PUT = %d: %s", rec.Code, rec.Body.String()) + } + var stored settingValueResponse + if err := json.Unmarshal(rec.Body.Bytes(), &stored); err != nil { + t.Fatalf("decoding: %v", err) + } + if stored.Scope != "profile_client" || stored.ProfileID != "profile-1" || stored.ClientFamily != "tv" { + t.Fatalf("stored identity = %+v, want profile-1/tv", stored) + } + + missing := "/admin/users/7/settings/values/nav.primary_menu?scope=profile_client&profile_id=profile-1" + if rec := env.do(t, "admin", http.MethodPut, missing, body); rec.Code != http.StatusBadRequest { + t.Fatalf("PUT without family = %d, want 400: %s", rec.Code, rec.Body.String()) + } +} + // TestAdminSetRejectsInvalidValueLikeTheSessionRoute pins that the admin write // is the same validation path as /settings/values, not a second validator: an // invalid value fails with the identical status, code and message. diff --git a/internal/api/handlers/settings_values_test.go b/internal/api/handlers/settings_values_test.go index 56b04eee..0a719207 100644 --- a/internal/api/handlers/settings_values_test.go +++ b/internal/api/handlers/settings_values_test.go @@ -3,14 +3,19 @@ package handlers import ( "bytes" "context" + "crypto/sha256" "database/sql" + "encoding/hex" "encoding/json" "errors" + "fmt" "log/slog" "net/http" "net/http/httptest" "slices" "strings" + "sync" + "sync/atomic" "testing" "github.com/go-chi/chi/v5" @@ -20,6 +25,7 @@ import ( "github.com/Silo-Server/silo-server/internal/auth" "github.com/Silo-Server/silo-server/internal/catalog" "github.com/Silo-Server/silo-server/internal/models" + "github.com/Silo-Server/silo-server/internal/notifications" "github.com/Silo-Server/silo-server/internal/settingscontract" "github.com/Silo-Server/silo-server/internal/settingskeys" "github.com/Silo-Server/silo-server/internal/settingsresolve" @@ -36,6 +42,7 @@ func newValuesTestHandler(t *testing.T) (*SettingValuesHandler, userstore.UserSt if err != nil { t.Fatalf("open sqlite: %v", err) } + db.SetMaxOpenConns(1) t.Cleanup(func() { _ = db.Close() }) if err := userdb.InitSchema(db); err != nil { t.Fatalf("init schema: %v", err) @@ -65,6 +72,7 @@ func valuesRequest(method, target string, body []byte) *http.Request { req = httptest.NewRequest(method, target, bytes.NewReader(body)) } req.Header.Set(deviceIDHeader, "device-1") + req.Header.Set(clientFamilyHeader, "web") ctx := apimw.SetClaims(req.Context(), &auth.Claims{UserID: 1}) return req.WithContext(apimw.SetProfileID(ctx, "profile-1")) } @@ -94,6 +102,499 @@ func routeValues(t *testing.T, h *SettingValuesHandler, method, key, query strin return rec } +func routeNavigationShortcutMutation( + t *testing.T, + h *SettingValuesHandler, + body string, + mutationID string, +) *httptest.ResponseRecorder { + t.Helper() + req := valuesRequest(http.MethodPut, "/settings/values/nav.shortcuts/item", []byte(body)) + if mutationID != "" { + req.Header.Set(mutationIDHeader, mutationID) + } + rec := httptest.NewRecorder() + h.HandleSetNavigationShortcut(rec, req) + return rec +} + +func decodeShortcutResponse(t *testing.T, rec *httptest.ResponseRecorder) (settingValueResponse, navigationShortcutDocument) { + t.Helper() + var response settingValueResponse + if err := json.Unmarshal(rec.Body.Bytes(), &response); err != nil { + t.Fatalf("decode shortcut response: %v; body=%s", err, rec.Body.String()) + } + var document navigationShortcutDocument + if err := json.Unmarshal(response.Value, &document); err != nil { + t.Fatalf("decode shortcut document: %v; value=%s", err, response.Value) + } + return response, document +} + +func TestNavigationShortcutMutationLifecycle(t *testing.T) { + handler, _ := newValuesTestHandler(t) + + addLibrary := `{"item":{"type":"library","library_id":42,"label":"Movies"},"present":true}` + rec := routeNavigationShortcutMutation(t, handler, addLibrary, "") + if rec.Code != http.StatusOK { + t.Fatalf("add library = %d: %s", rec.Code, rec.Body.String()) + } + response, document := decodeShortcutResponse(t, rec) + if response.Key != settingskeys.NavShortcuts || response.Scope != "profile" || response.Revision != 1 { + t.Fatalf("add response = %+v, want profile nav.shortcuts revision 1", response) + } + if len(document.Items) != 1 || document.Items[0].Label != "Movies" { + t.Fatalf("add document = %+v, want Movies", document) + } + + addSection := `{"item":{"type":"section","library_id":42,"section_id":"recent","label":"Recent"},"present":true}` + rec = routeNavigationShortcutMutation(t, handler, addSection, "") + if rec.Code != http.StatusOK { + t.Fatalf("add section = %d: %s", rec.Code, rec.Body.String()) + } + response, document = decodeShortcutResponse(t, rec) + if response.Revision != 2 || len(document.Items) != 2 || document.Items[1].SectionID != "recent" { + t.Fatalf("add section = revision %d document %+v, want both items at revision 2", response.Revision, document) + } + + refreshLibrary := `{"item":{"type":"library","library_id":42,"label":"Films"},"present":true}` + rec = routeNavigationShortcutMutation(t, handler, refreshLibrary, "") + response, document = decodeShortcutResponse(t, rec) + if response.Revision != 3 || len(document.Items) != 2 || document.Items[0].Label != "Films" || document.Items[1].SectionID != "recent" { + t.Fatalf("label refresh reordered or dropped items: revision %d document %+v", response.Revision, document) + } + + // Exact desired state is a no-op: no revision churn and no duplicate. + rec = routeNavigationShortcutMutation(t, handler, refreshLibrary, "") + response, document = decodeShortcutResponse(t, rec) + if response.Revision != 3 || len(document.Items) != 2 { + t.Fatalf("repeat add = revision %d document %+v, want unchanged revision 3", response.Revision, document) + } + + // Removal matches semantic identity only; a stale label cannot stop it. + removeLibrary := `{"item":{"type":"library","library_id":42,"label":"Old label"},"present":false}` + rec = routeNavigationShortcutMutation(t, handler, removeLibrary, "") + response, document = decodeShortcutResponse(t, rec) + if response.Revision != 4 || len(document.Items) != 1 || document.Items[0].SectionID != "recent" { + t.Fatalf("remove library = revision %d document %+v, want section preserved", response.Revision, document) + } + + rec = routeNavigationShortcutMutation(t, handler, removeLibrary, "") + response, _ = decodeShortcutResponse(t, rec) + if response.Revision != 4 { + t.Fatalf("repeat remove revision = %d, want unchanged 4", response.Revision) + } +} + +func TestNavigationShortcutCollectionIdentityIncludesOptionalLibrary(t *testing.T) { + handler, _ := newValuesTestHandler(t) + global := `{"item":{"type":"collection","collection_id":"favorites","label":"All Favorites"},"present":true}` + withinLibrary := `{"item":{"type":"collection","library_id":42,"collection_id":"favorites","label":"Movie Favorites"},"present":true}` + for _, body := range []string{global, withinLibrary} { + if rec := routeNavigationShortcutMutation(t, handler, body, ""); rec.Code != http.StatusOK { + t.Fatalf("add collection = %d: %s", rec.Code, rec.Body.String()) + } + } + + removeGlobal := `{"item":{"type":"collection","collection_id":"favorites","label":"Ignored"},"present":false}` + rec := routeNavigationShortcutMutation(t, handler, removeGlobal, "") + response, document := decodeShortcutResponse(t, rec) + if response.Revision != 3 || len(document.Items) != 1 || document.Items[0].LibraryID == nil || *document.Items[0].LibraryID != 42 { + t.Fatalf("global removal = revision %d document %+v, want scoped collection preserved", response.Revision, document) + } +} + +func TestNavigationShortcutCollectionLibraryIDValidationCannotMutateExistingTargets(t *testing.T) { + handler, store := newValuesTestHandler(t) + global := `{"item":{"type":"collection","collection_id":"favorites","label":"All Favorites"},"present":true}` + withinLibrary := `{"item":{"type":"collection","library_id":42,"collection_id":"favorites","label":"Movie Favorites"},"present":true}` + for _, tc := range []struct { + name string + body string + }{ + {"omitted library id is global", global}, + {"positive library id", withinLibrary}, + } { + t.Run(tc.name, func(t *testing.T) { + rec := routeNavigationShortcutMutation(t, handler, tc.body, "") + if rec.Code != http.StatusOK { + t.Fatalf("valid collection = %d: %s", rec.Code, rec.Body.String()) + } + }) + } + + invalidLibraryIDs := map[string]string{ + "explicit null": `null`, + "string": `"42"`, + "zero": `0`, + } + for name, libraryID := range invalidLibraryIDs { + t.Run(name, func(t *testing.T) { + body := fmt.Sprintf( + `{"item":{"type":"collection","library_id":%s,"collection_id":"favorites","label":"Ignored"},"present":false}`, + libraryID, + ) + rec := routeNavigationShortcutMutation(t, handler, body, "") + if rec.Code != http.StatusBadRequest || !strings.Contains(rec.Body.String(), "invalid_value") { + t.Fatalf("invalid collection library_id = %d: %s", rec.Code, rec.Body.String()) + } + + stored, err := store.GetSettingValue(context.Background(), userstore.SettingIdentity{ + Key: settingskeys.NavShortcuts, Scope: settingscontract.ScopeProfile, ProfileID: "profile-1", + }) + if err != nil || stored == nil { + t.Fatalf("read catalog after rejected remove: value=%+v err=%v", stored, err) + } + var document navigationShortcutDocument + if err := json.Unmarshal(stored.Value, &document); err != nil { + t.Fatalf("decode catalog after rejected remove: %v", err) + } + if stored.Revision != 2 || len(document.Items) != 2 { + t.Fatalf("rejected remove mutated catalog: revision %d items %+v", stored.Revision, document.Items) + } + if document.Items[0].LibraryID != nil { + t.Fatalf("global collection gained library identity: %+v", document.Items[0]) + } + if document.Items[1].LibraryID == nil || *document.Items[1].LibraryID != 42 { + t.Fatalf("library-specific collection changed identity: %+v", document.Items[1]) + } + }) + } +} + +func TestNavigationShortcutMutationIdempotency(t *testing.T) { + handler, _ := newValuesTestHandler(t) + const mutationID = "d615e91d-988f-4928-bb32-8956a26c7608" + body := `{"item":{"type":"collection","collection_id":"watchlist","label":"Watchlist"},"present":true}` + first := routeNavigationShortcutMutation(t, handler, body, mutationID) + if first.Code != http.StatusOK { + t.Fatalf("first mutation = %d: %s", first.Code, first.Body.String()) + } + replay := routeNavigationShortcutMutation(t, handler, body, mutationID) + if replay.Code != http.StatusOK || replay.Header().Get("X-Silo-Idempotent-Replay") != "true" { + t.Fatalf("replay = %d header %q: %s", replay.Code, replay.Header().Get("X-Silo-Idempotent-Replay"), replay.Body.String()) + } + if strings.TrimSpace(replay.Body.String()) != strings.TrimSpace(first.Body.String()) { + t.Fatalf("replay body = %s, want exact %s", replay.Body.String(), first.Body.String()) + } + + conflict := routeNavigationShortcutMutation(t, handler, + `{"item":{"type":"collection","collection_id":"watchlist","label":"Renamed"},"present":true}`, + mutationID) + if conflict.Code != http.StatusConflict || !strings.Contains(conflict.Body.String(), "mutation_id_conflict") { + t.Fatalf("mutation id reuse = %d: %s", conflict.Code, conflict.Body.String()) + } + + // A remove's label is not part of its semantics or request fingerprint. + const removeID = "f2f65f5e-918a-46bf-bee0-65a84491d298" + removed := routeNavigationShortcutMutation(t, handler, + `{"item":{"type":"collection","collection_id":"watchlist","label":"First"},"present":false}`, + removeID) + removeReplay := routeNavigationShortcutMutation(t, handler, + `{"item":{"type":"collection","collection_id":"watchlist","label":"Second"},"present":false}`, + removeID) + if removed.Code != http.StatusOK || removeReplay.Code != http.StatusOK || removeReplay.Header().Get("X-Silo-Idempotent-Replay") != "true" { + t.Fatalf("semantic remove replay = %d/%d header %q", removed.Code, removeReplay.Code, removeReplay.Header().Get("X-Silo-Idempotent-Replay")) + } +} + +func TestNavigationShortcutMutationThroughNotificationWrappedProvider(t *testing.T) { + baseHandler, store := newValuesTestHandler(t) + provider := notifications.WrapUserStoreProvider( + testUserStoreProvider{store: store}, + ¬ifications.System{}, + ) + handler := NewSettingValuesHandler(provider, baseHandler.contract) + const mutationID = "wrapped-provider-mutation" + body := `{"item":{"type":"collection","collection_id":"watchlist","label":"Watchlist"},"present":true}` + + first := routeNavigationShortcutMutation(t, handler, body, mutationID) + if first.Code != http.StatusOK { + t.Fatalf("wrapped first mutation = %d: %s", first.Code, first.Body.String()) + } + replay := routeNavigationShortcutMutation(t, handler, body, mutationID) + if replay.Code != http.StatusOK || replay.Header().Get("X-Silo-Idempotent-Replay") != "true" { + t.Fatalf("wrapped replay = %d header %q: %s", + replay.Code, replay.Header().Get("X-Silo-Idempotent-Replay"), replay.Body.String()) + } + if !bytes.Equal(bytes.TrimSpace(first.Body.Bytes()), bytes.TrimSpace(replay.Body.Bytes())) { + t.Fatalf("wrapped replay body = %s, want %s", replay.Body.String(), first.Body.String()) + } +} + +func TestNavigationShortcutConcurrentSameMutationID(t *testing.T) { + for _, tc := range []struct { + name string + bodies []string + wantConflicts int + }{ + { + name: "same hash replays one receipt", + bodies: []string{ + `{"item":{"type":"library","library_id":42,"label":"Movies"},"present":true}`, + `{"item":{"type":"library","library_id":42,"label":"Movies"},"present":true}`, + }, + }, + { + name: "different hash conflicts before CAS", + bodies: []string{ + `{"item":{"type":"library","library_id":42,"label":"Movies"},"present":true}`, + `{"item":{"type":"library","library_id":99,"label":"Shows"},"present":true}`, + }, + wantConflicts: 1, + }, + } { + t.Run(tc.name, func(t *testing.T) { + handler, store := newValuesTestHandler(t) + const mutationID = "concurrent-same-id" + start := make(chan struct{}) + responses := make(chan *httptest.ResponseRecorder, len(tc.bodies)) + var ready sync.WaitGroup + ready.Add(len(tc.bodies)) + for _, body := range tc.bodies { + body := body + go func() { + ready.Done() + <-start + responses <- routeNavigationShortcutMutation(t, handler, body, mutationID) + }() + } + ready.Wait() + close(start) + + conflicts := 0 + replays := 0 + var successes [][]byte + for range tc.bodies { + rec := <-responses + switch rec.Code { + case http.StatusOK: + successes = append(successes, bytes.TrimSpace(rec.Body.Bytes())) + if rec.Header().Get("X-Silo-Idempotent-Replay") == "true" { + replays++ + } + case http.StatusConflict: + if !strings.Contains(rec.Body.String(), "mutation_id_conflict") { + t.Fatalf("conflict = %s, want mutation_id_conflict", rec.Body.String()) + } + conflicts++ + default: + t.Fatalf("concurrent mutation = %d: %s", rec.Code, rec.Body.String()) + } + } + if conflicts != tc.wantConflicts { + t.Fatalf("conflicts = %d, want %d", conflicts, tc.wantConflicts) + } + if tc.wantConflicts == 0 { + if len(successes) != 2 || replays != 1 || !bytes.Equal(successes[0], successes[1]) { + t.Fatalf("same-hash outcomes: successes=%d replays=%d bodies=%q", len(successes), replays, successes) + } + } else if len(successes) != 1 || replays != 0 { + t.Fatalf("different-hash outcomes: successes=%d replays=%d", len(successes), replays) + } + + stored, err := store.GetSettingValue(context.Background(), userstore.SettingIdentity{ + Key: settingskeys.NavShortcuts, Scope: settingscontract.ScopeProfile, ProfileID: "profile-1", + }) + if err != nil || stored == nil || stored.Revision != 1 { + t.Fatalf("stored shortcut = %+v (%v), want exactly one CAS revision", stored, err) + } + receipt, err := store.GetSettingMutation(context.Background(), mutationID) + if err != nil || receipt == nil { + t.Fatalf("stored receipt = %+v (%v)", receipt, err) + } + if len(successes) > 0 && !bytes.Equal(successes[0], bytes.TrimSpace(receipt.Result)) { + t.Fatalf("success = %s, receipt = %s", successes[0], receipt.Result) + } + }) + } +} + +func TestNavigationShortcutMutationValidationAndWholeDocumentGuard(t *testing.T) { + handler, store := newValuesTestHandler(t) + + wholeDocument := routeValues(t, handler, http.MethodPut, settingskeys.NavShortcuts, + "scope=profile", []byte(`{"value":{"items":[]}}`)) + if wholeDocument.Code != http.StatusBadRequest || !strings.Contains(wholeDocument.Body.String(), "atomic_update_required") { + t.Fatalf("whole-document PUT = %d: %s", wholeDocument.Code, wholeDocument.Body.String()) + } + if value, err := store.GetSettingValue(context.Background(), userstore.SettingIdentity{ + Key: settingskeys.NavShortcuts, Scope: settingscontract.ScopeProfile, ProfileID: "profile-1", + }); err != nil || value != nil { + t.Fatalf("guarded PUT stored %+v, err=%v", value, err) + } + + invalidBodies := []string{ + `{"item":{"type":"builtin","destination":"home","label":"Home"},"present":true}`, + `{"item":{"type":"library","library_id":42,"label":"Movies","extra":true},"present":true}`, + `{"item":{"type":"library","library_id":42,"label":"Movies"},"present":true,"extra":true}`, + `{"item":{"type":"section","library_id":42,"section_id":"","label":"Recent"},"present":true}`, + `{"item":{"type":"collection","collection_id":"favorites","label":" "},"present":true}`, + `{"item":{"type":"library","library_id":0,"label":"Movies"},"present":true}`, + `{"item":{"type":"library","library_id":42,"label":"Movies"}}`, + } + for _, body := range invalidBodies { + rec := routeNavigationShortcutMutation(t, handler, body, "") + if rec.Code != http.StatusBadRequest { + t.Errorf("invalid body %s = %d: %s", body, rec.Code, rec.Body.String()) + } + } +} + +func TestNavigationShortcutWholeDocumentDeleteCannotCreateRevisionABA(t *testing.T) { + handler, store := newValuesTestHandler(t) + first := routeNavigationShortcutMutation(t, handler, + `{"item":{"type":"library","library_id":42,"label":"Movies"},"present":true}`, "") + firstResponse, _ := decodeShortcutResponse(t, first) + if first.Code != http.StatusOK || firstResponse.Revision != 1 { + t.Fatalf("seed shortcut = %d revision %d: %s", first.Code, firstResponse.Revision, first.Body.String()) + } + + identity := userstore.SettingIdentity{ + Key: settingskeys.NavShortcuts, Scope: settingscontract.ScopeProfile, ProfileID: "profile-1", + } + beforeDelete, err := store.GetSettingValue(context.Background(), identity) + if err != nil || beforeDelete == nil { + t.Fatalf("read shortcut before guarded delete: value=%+v err=%v", beforeDelete, err) + } + deleteRec := routeValues(t, handler, http.MethodDelete, settingskeys.NavShortcuts, + "scope=profile", nil) + if deleteRec.Code != http.StatusBadRequest || !strings.Contains(deleteRec.Body.String(), "atomic_update_required") { + t.Fatalf("whole-document DELETE = %d: %s", deleteRec.Code, deleteRec.Body.String()) + } + + second := routeNavigationShortcutMutation(t, handler, + `{"item":{"type":"library","library_id":99,"label":"Shows"},"present":true}`, "") + secondResponse, secondDocument := decodeShortcutResponse(t, second) + if second.Code != http.StatusOK || secondResponse.Revision != 2 || len(secondDocument.Items) != 2 { + t.Fatalf("second shortcut = %d revision %d document %+v", second.Code, secondResponse.Revision, secondDocument) + } + + // If session DELETE had removed and recreated the row, its revision would + // return to one and this stale writer could pass an ABA-shaped precondition. + // Keeping the row alive makes the stale revision conflict instead. + cas := store.(userstore.SettingValueCompareAndSetter) + if _, err := cas.CompareAndSetSettingValue(context.Background(), identity, + beforeDelete.Value, beforeDelete.Revision); !errors.Is(err, userstore.ErrSettingValueRevisionConflict) { + t.Fatalf("stale CAS after guarded DELETE error = %v, want revision conflict", err) + } + final, err := store.GetSettingValue(context.Background(), identity) + if err != nil || final == nil || final.Revision != 2 { + t.Fatalf("final shortcut row = %+v err=%v, want revision 2", final, err) + } +} + +func TestNavigationShortcutMutationEnforcesDocumentCap(t *testing.T) { + handler, store := newValuesTestHandler(t) + items := make([]navigationShortcutItem, 0, 256) + for id := 1; id <= 256; id++ { + libraryID := id + items = append(items, navigationShortcutItem{Type: "library", LibraryID: &libraryID, Label: fmt.Sprintf("Library %d", id)}) + } + raw, err := json.Marshal(navigationShortcutDocument{Items: items}) + if err != nil { + t.Fatal(err) + } + def, _ := handler.contract.Lookup(settingskeys.NavShortcuts) + normalized, err := def.ValueSchema.NormalizeValue(raw, settingscontract.ObjectSchemas()) + if err != nil { + t.Fatalf("normalize seed: %v", err) + } + identity := userstore.SettingIdentity{ + Key: settingskeys.NavShortcuts, Scope: settingscontract.ScopeProfile, ProfileID: "profile-1", + } + seed, err := store.UpsertSettingValue(context.Background(), identity, normalized) + if err != nil { + t.Fatalf("seed full catalog: %v", err) + } + + rec := routeNavigationShortcutMutation(t, handler, + `{"item":{"type":"library","library_id":257,"label":"Too Many"},"present":true}`, "") + if rec.Code != http.StatusBadRequest || !strings.Contains(rec.Body.String(), "invalid_value") { + t.Fatalf("257th shortcut = %d: %s", rec.Code, rec.Body.String()) + } + got, err := store.GetSettingValue(context.Background(), identity) + if err != nil || got == nil || got.Revision != seed.Revision { + t.Fatalf("catalog after rejected add = %+v, err=%v; want unchanged revision %d", got, err, seed.Revision) + } +} + +type synchronizedShortcutStore struct { + userstore.UserStore + cas userstore.SettingValueCompareAndSetter + reads atomic.Int32 + ready chan struct{} + once sync.Once + casMu sync.Mutex +} + +func (s *synchronizedShortcutStore) GetSettingValue(ctx context.Context, id userstore.SettingIdentity) (*userstore.SettingValue, error) { + value, err := s.UserStore.GetSettingValue(ctx, id) + if err == nil && id.Key == settingskeys.NavShortcuts { + read := s.reads.Add(1) + if read <= 2 { + if read == 2 { + s.once.Do(func() { close(s.ready) }) + } + <-s.ready + } + } + return value, err +} + +func (s *synchronizedShortcutStore) CompareAndSetSettingValue( + ctx context.Context, + id userstore.SettingIdentity, + value json.RawMessage, + expectedRevision int64, +) (*userstore.SettingValue, error) { + s.casMu.Lock() + defer s.casMu.Unlock() + return s.cas.CompareAndSetSettingValue(ctx, id, value, expectedRevision) +} + +func TestNavigationShortcutConcurrentAddsMerge(t *testing.T) { + baseHandler, baseStore := newValuesTestHandler(t) + wrapped := &synchronizedShortcutStore{ + UserStore: baseStore, + cas: baseStore.(userstore.SettingValueCompareAndSetter), + ready: make(chan struct{}), + } + handler := NewSettingValuesHandler(testUserStoreProvider{store: wrapped}, baseHandler.contract) + bodies := []string{ + `{"item":{"type":"library","library_id":42,"label":"Movies"},"present":true}`, + `{"item":{"type":"library","library_id":99,"label":"Shows"},"present":true}`, + } + + responses := make(chan *httptest.ResponseRecorder, len(bodies)) + for _, body := range bodies { + body := body + go func() { + responses <- routeNavigationShortcutMutation(t, handler, body, "") + }() + } + for range bodies { + rec := <-responses + if rec.Code != http.StatusOK { + t.Fatalf("concurrent add = %d: %s", rec.Code, rec.Body.String()) + } + } + + got, err := baseStore.GetSettingValue(context.Background(), userstore.SettingIdentity{ + Key: settingskeys.NavShortcuts, Scope: settingscontract.ScopeProfile, ProfileID: "profile-1", + }) + if err != nil || got == nil { + t.Fatalf("read merged catalog: value=%+v err=%v", got, err) + } + var document navigationShortcutDocument + if err := json.Unmarshal(got.Value, &document); err != nil { + t.Fatalf("decode merged catalog: %v", err) + } + if got.Revision != 2 || len(document.Items) != 2 { + t.Fatalf("merged catalog = revision %d items %+v, want both adds at revision 2", got.Revision, document.Items) + } +} + func TestSettingValuesRoundTrip(t *testing.T) { handler, _ := newValuesTestHandler(t) @@ -131,6 +632,161 @@ func TestSettingValuesRoundTrip(t *testing.T) { } } +func TestProfileClientValueUsesExplicitFamilyHeader(t *testing.T) { + handler, _ := newValuesTestHandler(t) + key := settingskeys.NavPrimaryMenu + body := []byte(`{"value":{"items":[{"type":"builtin","destination":"home"},` + + `{"type":"library","library_id":42,"label":"Movies"}]}}`) + + rec := routeValues(t, handler, http.MethodPut, key, "scope=profile_client", body) + if rec.Code != http.StatusOK { + t.Fatalf("PUT profile_client = %d: %s", rec.Code, rec.Body.String()) + } + var stored settingValueResponse + if err := json.Unmarshal(rec.Body.Bytes(), &stored); err != nil { + t.Fatalf("decoding response: %v", err) + } + if stored.Scope != "profile_client" || stored.ClientFamily != "web" { + t.Fatalf("stored scope/family = %q/%q, want profile_client/web", stored.Scope, stored.ClientFamily) + } + var menu navigationShortcutDocument + if err := json.Unmarshal(stored.Value, &menu); err != nil { + t.Fatalf("decoding stored primary menu: %v", err) + } + if len(menu.Items) != 2 || menu.Items[1].Label != "Movies" { + t.Fatalf("stored non-builtin menu item = %+v, want labeled Movies library", menu.Items) + } + + for _, tc := range []struct { + name string + header string + }{ + {"missing", ""}, + {"not canonical", "TV"}, + {"unknown", "car"}, + } { + t.Run(tc.name, func(t *testing.T) { + req := valuesRequest(http.MethodPut, + "/settings/values/"+key+"?scope=profile_client", body) + if tc.header == "" { + req.Header.Del(clientFamilyHeader) + } else { + req.Header.Set(clientFamilyHeader, tc.header) + } + routeCtx := chi.NewRouteContext() + routeCtx.URLParams.Add("key", key) + req = req.WithContext(context.WithValue(req.Context(), chi.RouteCtxKey, routeCtx)) + rec := httptest.NewRecorder() + handler.HandleSetValue(rec, req) + if rec.Code != http.StatusBadRequest { + t.Fatalf("PUT with family %q = %d, want 400: %s", tc.header, rec.Code, rec.Body.String()) + } + }) + } +} + +func TestMutationHashPreservesExistingScopesAndSeparatesClientFamilies(t *testing.T) { + identity := userstore.SettingIdentity{ + Key: "playback.subtitle_mode", Scope: settingscontract.ScopeProfileDevice, + ProfileID: "profile-1", DeviceID: "device-1", + } + value := json.RawMessage(`"always"`) + legacyPayload := append([]byte( + "playback.subtitle_mode\x00profile_device\x00profile-1\x00device-1\x000\x00\x00", + ), value...) + want := sha256.Sum256(legacyPayload) + if got := hashMutationRequest(identity, value); got != hex.EncodeToString(want[:]) { + t.Fatalf("existing-scope mutation hash changed across the profile_client rollout: %s", got) + } + + client := userstore.SettingIdentity{ + Key: settingskeys.NavPrimaryMenu, Scope: settingscontract.ScopeProfileClient, + ProfileID: "profile-1", ClientFamily: settingscontract.ClientFamilyTV, + } + tv := hashMutationRequest(client, json.RawMessage(`null`)) + client.ClientFamily = settingscontract.ClientFamilyMobile + if mobile := hashMutationRequest(client, json.RawMessage(`null`)); mobile == tv { + t.Fatal("profile_client mutation hashes do not distinguish client families") + } +} + +func TestEffectiveClientFamilyIsOptionalButValidated(t *testing.T) { + handler, _ := newValuesTestHandler(t) + key := settingskeys.UiCardPresentation + if rec := routeValues(t, handler, http.MethodPut, key, "scope=profile", + []byte(`{"value":{"poster_size":"compact","caption":"title"}}`)); rec.Code != http.StatusOK { + t.Fatalf("seed profile fallback = %d: %s", rec.Code, rec.Body.String()) + } + if rec := routeValues(t, handler, http.MethodPut, key, "scope=profile_client", + []byte(`{"value":{"poster_size":"large","caption":"artwork"}}`)); rec.Code != http.StatusOK { + t.Fatalf("seed family value = %d: %s", rec.Code, rec.Body.String()) + } + + effective := func(t *testing.T, family *string) (int, effectiveSettingValueResponse, string) { + t.Helper() + req := valuesRequest(http.MethodGet, "/settings/values/effective?keys="+key, nil) + if family == nil { + req.Header.Del(clientFamilyHeader) + } else { + req.Header.Set(clientFamilyHeader, *family) + } + rec := httptest.NewRecorder() + handler.HandleGetEffective(rec, req) + if rec.Code != http.StatusOK { + return rec.Code, effectiveSettingValueResponse{}, rec.Body.String() + } + var body struct { + Settings []effectiveSettingValueResponse `json:"settings"` + } + if err := json.Unmarshal(rec.Body.Bytes(), &body); err != nil { + t.Fatalf("decode effective response: %v", err) + } + if len(body.Settings) != 1 { + t.Fatalf("settings = %d, want 1", len(body.Settings)) + } + return rec.Code, body.Settings[0], rec.Body.String() + } + + t.Run("absent skips profile client layer", func(t *testing.T) { + code, got, body := effective(t, nil) + if code != http.StatusOK { + t.Fatalf("effective without client family = %d: %s", code, body) + } + if got.Source != string(settingscontract.ScopeProfile) || string(got.Value) != `{"poster_size":"compact","caption":"title"}` { + t.Fatalf("effective without family = %s from %s, want profile fallback", got.Value, got.Source) + } + }) + + t.Run("absent remains compatible with resolve all", func(t *testing.T) { + req := valuesRequest(http.MethodGet, "/settings/values/effective", nil) + req.Header.Del(clientFamilyHeader) + rec := httptest.NewRecorder() + handler.HandleGetEffective(rec, req) + if rec.Code != http.StatusOK { + t.Fatalf("resolve all without client family = %d: %s", rec.Code, rec.Body.String()) + } + }) + + t.Run("valid selects profile client layer", func(t *testing.T) { + family := "web" + code, got, body := effective(t, &family) + if code != http.StatusOK { + t.Fatalf("effective with client family = %d: %s", code, body) + } + if got.Source != string(settingscontract.ScopeProfileClient) || got.ClientFamily != family { + t.Fatalf("effective with family = %s/%s, want profile_client/web", got.Source, got.ClientFamily) + } + }) + + t.Run("nonempty invalid is rejected", func(t *testing.T) { + family := "TV" + code, _, _ := effective(t, &family) + if code != http.StatusBadRequest { + t.Fatalf("effective with invalid client family = %d, want 400", code) + } + }) +} + func TestGetSettingValuesReportsSetAndUnsetAtOneScope(t *testing.T) { handler, store := newValuesTestHandler(t) if _, err := store.UpsertSettingValue(context.Background(), userstore.SettingIdentity{ @@ -697,12 +1353,34 @@ type failingUpsertStore struct { userstore.UserStore } +type failingUpsertMutationWriter struct { + userstore.SettingMutationWriter +} + func (failingUpsertStore) UpsertSettingValue( context.Context, userstore.SettingIdentity, json.RawMessage, ) (*userstore.SettingValue, error) { return nil, errors.New("simulated write failure") } +func (s failingUpsertStore) WithSettingMutationTransaction( + ctx context.Context, + mutationID string, + fn func(userstore.SettingMutationWriter) error, +) error { + transactioner := s.UserStore.(userstore.SettingMutationTransactioner) + return transactioner.WithSettingMutationTransaction(ctx, mutationID, + func(writer userstore.SettingMutationWriter) error { + return fn(failingUpsertMutationWriter{SettingMutationWriter: writer}) + }) +} + +func (failingUpsertMutationWriter) UpsertSettingValue( + context.Context, userstore.SettingIdentity, json.RawMessage, +) (*userstore.SettingValue, error) { + return nil, errors.New("simulated write failure") +} + // TestFailedWritesLeaveNoReceipt: a receipt for a write that never landed // would turn the client's retry of a 500 into a silent success replay. func TestFailedWritesLeaveNoReceipt(t *testing.T) { @@ -815,9 +1493,10 @@ func TestCapabilitiesReportTheContractRevision(t *testing.T) { } var body struct { - APIVersion int `json:"api_version"` - Revision int `json:"revision"` - Scopes []string `json:"scopes"` + APIVersion int `json:"api_version"` + Revision int `json:"revision"` + Scopes []string `json:"scopes"` + ClientFamilies []string `json:"client_families"` } if err := json.Unmarshal(rec.Body.Bytes(), &body); err != nil { t.Fatalf("decoding: %v", err) @@ -827,8 +1506,15 @@ func TestCapabilitiesReportTheContractRevision(t *testing.T) { t.Errorf("reported %d/%d, want %d/%d", body.APIVersion, body.Revision, contract.APIVersion, contract.Revision) } - if len(body.Scopes) != 5 { - t.Errorf("reported %d scopes, want 5", len(body.Scopes)) + wantScopes := []string{ + "account", "profile", "profile_client", "profile_device", "profile_library", "profile_series", + } + if !slices.Equal(body.Scopes, wantScopes) { + t.Errorf("reported scopes %v, want %v", body.Scopes, wantScopes) + } + wantFamilies := []string{"tv", "mobile", "tablet", "desktop", "web"} + if !slices.Equal(body.ClientFamilies, wantFamilies) { + t.Errorf("reported client families %v, want %v", body.ClientFamilies, wantFamilies) } } diff --git a/internal/api/router.go b/internal/api/router.go index 81681c3c..c02c1733 100644 --- a/internal/api/router.go +++ b/internal/api/router.go @@ -2402,6 +2402,7 @@ func NewRouter(deps Dependencies) chi.Router { r.Get("/values", settingValuesHandler.HandleGetValues) r.Get("/values/effective", settingValuesHandler.HandleGetEffective) r.Post("/values/effective", settingValuesHandler.HandlePostEffective) + r.Put("/values/nav.shortcuts/item", settingValuesHandler.HandleSetNavigationShortcut) r.Get("/values/{key}", settingValuesHandler.HandleGetValue) r.Put("/values/{key}", settingValuesHandler.HandleSetValue) r.Delete("/values/{key}", settingValuesHandler.HandleDeleteValue) diff --git a/internal/database/settings_backfill_test.go b/internal/database/settings_backfill_test.go index 91a7f46d..9837d756 100644 --- a/internal/database/settings_backfill_test.go +++ b/internal/database/settings_backfill_test.go @@ -19,7 +19,7 @@ import ( // The planner's rules are unit-tested in internal/settingsmigrate. What this // covers is everything only a live database can show: that the Go migration is // registered and actually runs, that the rows satisfy the scope CHECK, the -// composite profile foreign key and the five partial unique indexes, and that +// composite profile foreign key and the six partial unique indexes, and that // jsonb accepts the values the planner encodes. func TestPostgresSettingsBackfill(t *testing.T) { dsn := os.Getenv("SILO_TEST_DATABASE_URL") diff --git a/internal/notifications/interest_hooks.go b/internal/notifications/interest_hooks.go index e6fb16cd..ef12e229 100644 --- a/internal/notifications/interest_hooks.go +++ b/internal/notifications/interest_hooks.go @@ -2,6 +2,7 @@ package notifications import ( "context" + "encoding/json" "fmt" "log/slog" "time" @@ -64,6 +65,11 @@ type interestTrackingStoreWithDevices struct { userstore.DeviceRegistry } +var _ userstore.SettingValueCompareAndSetter = (*interestTrackingStore)(nil) +var _ userstore.SettingMutationTransactioner = (*interestTrackingStore)(nil) +var _ userstore.SettingValueCompareAndSetter = (*interestTrackingStoreWithDevices)(nil) +var _ userstore.SettingMutationTransactioner = (*interestTrackingStoreWithDevices)(nil) + // WithPreferenceSettingsTransaction preserves the optional atomic-settings // capability of the wrapped store. Preference writes do not affect interest // signals, so the transaction can pass through unchanged; keeping the method @@ -80,6 +86,37 @@ func (s *interestTrackingStore) WithPreferenceSettingsTransaction( return transactioner.WithPreferenceSettingsTransaction(ctx, fn) } +// CompareAndSetSettingValue preserves the semantic-document CAS capability of +// the wrapped store. Settings writes do not affect notification interests, so +// this decorator must not intercept or downgrade the backend primitive. +func (s *interestTrackingStore) CompareAndSetSettingValue( + ctx context.Context, + id userstore.SettingIdentity, + value json.RawMessage, + expectedRevision int64, +) (*userstore.SettingValue, error) { + cas, ok := s.UserStore.(userstore.SettingValueCompareAndSetter) + if !ok { + return nil, fmt.Errorf("wrapped user store does not support atomic setting updates") + } + return cas.CompareAndSetSettingValue(ctx, id, value, expectedRevision) +} + +// WithSettingMutationTransaction preserves the durable setting+receipt +// transaction used by mutation IDs. Passing the transaction-scoped writer +// through unchanged keeps both operations on the concrete backend transaction. +func (s *interestTrackingStore) WithSettingMutationTransaction( + ctx context.Context, + mutationID string, + fn func(userstore.SettingMutationWriter) error, +) error { + transactioner, ok := s.UserStore.(userstore.SettingMutationTransactioner) + if !ok { + return fmt.Errorf("wrapped user store does not support atomic idempotent setting mutations") + } + return transactioner.WithSettingMutationTransaction(ctx, mutationID, fn) +} + // progressState is the transition-relevant projection of a progress row. type progressState struct { exists bool diff --git a/internal/notifications/interest_hooks_test.go b/internal/notifications/interest_hooks_test.go index 96ff9eda..9396ea7b 100644 --- a/internal/notifications/interest_hooks_test.go +++ b/internal/notifications/interest_hooks_test.go @@ -3,8 +3,11 @@ package notifications import ( "context" "database/sql" + "encoding/json" "testing" + "time" + "github.com/Silo-Server/silo-server/internal/settingscontract" "github.com/Silo-Server/silo-server/internal/userdb" "github.com/Silo-Server/silo-server/internal/userstore" ) @@ -19,7 +22,7 @@ func (p preferenceTransactionTestProvider) ForUser(context.Context, int) (userst func (preferenceTransactionTestProvider) Close() error { return nil } -func TestInterestTrackingStorePreservesPreferenceSettingsTransactions(t *testing.T) { +func TestInterestTrackingStorePreservesSettingCapabilities(t *testing.T) { db, err := sql.Open("sqlite3", ":memory:") if err != nil { t.Fatalf("open sqlite: %v", err) @@ -53,4 +56,47 @@ func TestInterestTrackingStorePreservesPreferenceSettingsTransactions(t *testing if !called { t.Fatal("transaction callback was not invoked") } + + cas, ok := wrapped.(userstore.SettingValueCompareAndSetter) + if !ok { + t.Fatal("interest-tracking wrapper dropped SettingValueCompareAndSetter") + } + identity := userstore.SettingIdentity{Key: "ui.test", Scope: settingscontract.ScopeAccount} + first, err := cas.CompareAndSetSettingValue( + context.Background(), identity, json.RawMessage(`"first"`), 0) + if err != nil { + t.Fatalf("CompareAndSetSettingValue: %v", err) + } + + mutationTx, ok := wrapped.(userstore.SettingMutationTransactioner) + if !ok { + t.Fatal("interest-tracking wrapper dropped SettingMutationTransactioner") + } + called = false + if err := mutationTx.WithSettingMutationTransaction(context.Background(), "wrapped-mutation", + func(writer userstore.SettingMutationWriter) error { + called = true + if _, err := writer.CompareAndSetSettingValue( + context.Background(), identity, json.RawMessage(`"second"`), first.Revision); err != nil { + return err + } + _, _, err := writer.PutSettingMutation(context.Background(), userstore.SettingMutationRecord{ + MutationID: "wrapped-mutation", RequestHash: "hash", + Result: json.RawMessage(`{"ok":true}`), ExpiresAt: time.Now().UTC().Add(time.Hour), + }) + return err + }); err != nil { + t.Fatalf("WithSettingMutationTransaction: %v", err) + } + if !called { + t.Fatal("mutation transaction callback was not invoked") + } + stored, err := wrapped.GetSettingValue(context.Background(), identity) + if err != nil || stored == nil || stored.Revision != 2 { + t.Fatalf("wrapped setting after transaction = %+v (%v), want revision 2", stored, err) + } + receipt, err := wrapped.GetSettingMutation(context.Background(), "wrapped-mutation") + if err != nil || receipt == nil || receipt.RequestHash != "hash" { + t.Fatalf("wrapped receipt = %+v (%v)", receipt, err) + } } diff --git a/internal/settingscontract/contract.go b/internal/settingscontract/contract.go index 17785f77..fa7578c6 100644 --- a/internal/settingscontract/contract.go +++ b/internal/settingscontract/contract.go @@ -22,6 +22,7 @@ type Scope string const ( ScopeAccount Scope = "account" ScopeProfile Scope = "profile" + ScopeProfileClient Scope = "profile_client" ScopeProfileDevice Scope = "profile_device" ScopeProfileLibrary Scope = "profile_library" ScopeProfileSeries Scope = "profile_series" @@ -37,6 +38,7 @@ const ( var remoteScopes = map[Scope]struct{}{ ScopeAccount: {}, ScopeProfile: {}, + ScopeProfileClient: {}, ScopeProfileDevice: {}, ScopeProfileLibrary: {}, ScopeProfileSeries: {}, @@ -48,6 +50,31 @@ func (s Scope) IsRemote() bool { return ok } +// ClientFamily identifies a class of like clients whose presentation and +// navigation preferences should roam together. It is explicit request +// metadata rather than an inference from the device platform string: platform +// names are free-form registry metadata, while this value participates in a +// canonical storage identity. +type ClientFamily string + +const ( + ClientFamilyTV ClientFamily = "tv" + ClientFamilyMobile ClientFamily = "mobile" + ClientFamilyTablet ClientFamily = "tablet" + ClientFamilyDesktop ClientFamily = "desktop" + ClientFamilyWeb ClientFamily = "web" +) + +// Valid reports whether f is one of the canonical lower-case wire values. +func (f ClientFamily) Valid() bool { + switch f { + case ClientFamilyTV, ClientFamilyMobile, ClientFamilyTablet, ClientFamilyDesktop, ClientFamilyWeb: + return true + default: + return false + } +} + // Persistence declares who stores a setting's value. type Persistence string diff --git a/internal/settingscontract/contract_test.go b/internal/settingscontract/contract_test.go index 67893f43..21785f8f 100644 --- a/internal/settingscontract/contract_test.go +++ b/internal/settingscontract/contract_test.go @@ -1588,6 +1588,119 @@ func TestObjectSchemasAcceptTheShapesClientsStore(t *testing.T) { } } +func TestUICustomizationObjectSchemasAreStrict(t *testing.T) { + manifest, err := Load() + if err != nil { + t.Fatalf("loading manifest: %v", err) + } + + valid := map[string]string{ + "nav.primary_menu": `{"items":[` + + `{"type":"builtin","destination":"home"},` + + `{"type":"library","library_id":7,"label":"Movies"},` + + `{"type":"section","library_id":7,"section_id":"recently-added","label":"Recently Added"},` + + `{"type":"collection","collection_id":"horror","library_id":7,"label":"Horror"}]}`, + "nav.shortcuts": `{"items":[` + + `{"type":"library","library_id":7,"label":"Movies"},` + + `{"type":"section","library_id":7,"section_id":"recently-added","label":"Recently Added"},` + + `{"type":"collection","collection_id":"horror","label":"Horror"}]}`, + "ui.card_presentation": `{"poster_size":"large","caption":"artwork"}`, + } + for key, value := range valid { + t.Run(key+" accepts canonical shape", func(t *testing.T) { + def, ok := manifest.Lookup(key) + if !ok { + t.Fatalf("%s is not registered", key) + } + if err := def.ValueSchema.ValidateValue(json.RawMessage(value), objSchemas); err != nil { + t.Fatalf("valid %s value rejected: %v", key, err) + } + }) + } + + invalid := []struct { + name string + key string + value string + }{ + { + name: "primary menu requires home", + key: "nav.primary_menu", + value: `{"items":[{"type":"builtin","destination":"movies"}]}`, + }, + { + name: "primary menu allows home exactly once", + key: "nav.primary_menu", + value: `{"items":[{"type":"builtin","destination":"home"},{"type":"builtin","destination":"home"}]}`, + }, + { + name: "menu labels cannot be empty", + key: "nav.primary_menu", + value: `{"items":[{"type":"builtin","destination":"home"},{"type":"library","library_id":7,"label":""}]}`, + }, + { + name: "menu labels cannot be only whitespace", + key: "nav.primary_menu", + value: `{"items":[{"type":"builtin","destination":"home"},{"type":"library","library_id":7,"label":" "}]}`, + }, + { + name: "menu target ids cannot be only whitespace", + key: "nav.primary_menu", + value: `{"items":[{"type":"builtin","destination":"home"},{"type":"section","library_id":7,"section_id":" ","label":"Recent"}]}`, + }, + { + name: "menu destination identity ignores labels", + key: "nav.primary_menu", + value: `{"items":[{"type":"builtin","destination":"home"},` + + `{"type":"library","library_id":7,"label":"Movies"},` + + `{"type":"library","library_id":7,"label":"Films"}]}`, + }, + { + name: "shortcuts cannot contain builtins", + key: "nav.shortcuts", + value: `{"items":[{"type":"builtin","destination":"home"}]}`, + }, + { + name: "shortcuts require positive library ids", + key: "nav.shortcuts", + value: `{"items":[{"type":"library","library_id":0,"label":"Movies"}]}`, + }, + { + name: "shortcut target ids cannot be only whitespace", + key: "nav.shortcuts", + value: `{"items":[{"type":"collection","collection_id":"\t ","label":"Collection"}]}`, + }, + { + name: "shortcut destination identity includes collection context", + key: "nav.shortcuts", + value: `{"items":[` + + `{"type":"collection","collection_id":"horror","library_id":7,"label":"Horror"},` + + `{"type":"collection","collection_id":"horror","library_id":7,"label":"Scary"}]}`, + }, + { + name: "card presets reject unknown sizes", + key: "ui.card_presentation", + value: `{"poster_size":"huge","caption":"artwork"}`, + }, + { + name: "card presets reject extension fields", + key: "ui.card_presentation", + value: `{"poster_size":"large","caption":"artwork","columns":8}`, + }, + } + for _, tc := range invalid { + t.Run(tc.name, func(t *testing.T) { + def, ok := manifest.Lookup(tc.key) + if !ok { + t.Fatalf("%s is not registered", tc.key) + } + if err := def.ValueSchema.ValidateValue(json.RawMessage(tc.value), objSchemas); err == nil { + t.Fatalf("invalid %s value was accepted: %s", tc.key, tc.value) + } + }) + } +} + // TestQualityIsTwoIndependentAxes pins the split that replaced the compound // ladder values. A client composes its own presets from these two, so the // contract must not constrain them jointly. diff --git a/internal/settingscontract/validate.go b/internal/settingscontract/validate.go index 6a2ed7c1..07cb4cd7 100644 --- a/internal/settingscontract/validate.go +++ b/internal/settingscontract/validate.go @@ -553,6 +553,9 @@ func (v *ValueSchema) ValidateValue(raw json.RawMessage, objectSchemas map[strin if err := schema.Validate(doc); err != nil { return fmt.Errorf("does not satisfy %s: %w", v.SchemaRef, err) } + if err := validateObjectSemantics(v.SchemaRef, trimmed); err != nil { + return fmt.Errorf("does not satisfy %s: %w", v.SchemaRef, err) + } default: return fmt.Errorf("unknown value type %q", v.Type) @@ -561,6 +564,86 @@ func (v *ValueSchema) ValidateValue(raw json.RawMessage, objectSchemas map[strin return nil } +type navigationDocument struct { + Items []navigationItem `json:"items"` +} + +type navigationItem struct { + Type string `json:"type"` + Destination string `json:"destination"` + LibraryID *int `json:"library_id"` + SectionID string `json:"section_id"` + CollectionID string `json:"collection_id"` + Label string `json:"label"` +} + +type navigationIdentity struct { + Type string + Destination string + LibraryID int + HasLibraryID bool + SectionID string + CollectionID string +} + +// validateObjectSemantics holds the few cross-item invariants JSON Schema +// cannot express. uniqueItems rejects byte-for-byte duplicate objects, but a +// renamed library is still the same destination and must not appear twice in a +// menu. Keeping this beside ValidateValue means API writes, migrations, and +// manifest defaults all enforce the same identity rule. +func validateObjectSemantics(schemaRef string, raw json.RawMessage) error { + if schemaRef != "primary-menu.json" && schemaRef != "navigation-shortcuts.json" { + return nil + } + + var document navigationDocument + if err := strictUnmarshal(raw, &document); err != nil { + return fmt.Errorf("decoding navigation document: %w", err) + } + seen := make(map[navigationIdentity]int, len(document.Items)) + for index, item := range document.Items { + identity, err := item.identity() + if err != nil { + return fmt.Errorf("items[%d]: %w", index, err) + } + if first, duplicate := seen[identity]; duplicate { + return fmt.Errorf("items[%d] repeats the destination from items[%d]", index, first) + } + seen[identity] = index + } + return nil +} + +func (item navigationItem) identity() (navigationIdentity, error) { + identity := navigationIdentity{Type: item.Type} + switch item.Type { + case "builtin": + identity.Destination = item.Destination + case "library": + if item.LibraryID == nil { + return navigationIdentity{}, errors.New("library is missing library_id") + } + identity.LibraryID = *item.LibraryID + identity.HasLibraryID = true + case "section": + if item.LibraryID == nil { + return navigationIdentity{}, errors.New("section is missing library_id") + } + identity.LibraryID = *item.LibraryID + identity.HasLibraryID = true + identity.SectionID = item.SectionID + case "collection": + identity.CollectionID = item.CollectionID + if item.LibraryID != nil { + identity.LibraryID = *item.LibraryID + identity.HasLibraryID = true + } + default: + return navigationIdentity{}, fmt.Errorf("unknown navigation item type %q", item.Type) + } + return identity, nil +} + // NormalizeValue validates a value and returns the form that should be stored. // // Anything that persists a value goes through here rather than ValidateValue, diff --git a/internal/settingskeys/keys.go b/internal/settingskeys/keys.go index d34c4b37..52691e06 100644 --- a/internal/settingskeys/keys.go +++ b/internal/settingskeys/keys.go @@ -9,7 +9,7 @@ package settingskeys // Revision is the manifest revision these bindings were generated from. -const Revision = 4 +const Revision = 5 // Setting keys, one constant per definition. const ( @@ -23,6 +23,10 @@ const ( DownloadsKeepWatched = "downloads.keep_watched" // Download over Wi-Fi only DownloadsWifiOnly = "downloads.wifi_only" + // Primary menu + NavPrimaryMenu = "nav.primary_menu" + // Navigation shortcuts + NavShortcuts = "nav.shortcuts" // Show audiobooks NavShowAudiobooks = "nav.show_audiobooks" // Preferred audio language @@ -85,6 +89,8 @@ const ( SubtitleMatchesDevice = "subtitle.matches_device" // Poster badges UiCardOverlays = "ui.card_overlays" + // Media cards + UiCardPresentation = "ui.card_presentation" // Custom CSS UiCustomCss = "ui.custom_css" // Custom theme variables @@ -119,6 +125,8 @@ const ( var Remote = []string{ CatalogMetadataLanguage, CatalogMetadataLanguageOverrides, + NavPrimaryMenu, + NavShortcuts, PlaybackAudioLanguage, PlaybackAutoPlayNext, PlaybackAutoPlayNextPreview, @@ -145,6 +153,7 @@ var Remote = []string{ PlayerVideoGravity, SearchMediaScope, UiCardOverlays, + UiCardPresentation, UiCustomCss, UiCustomThemeVars, UiDateFormat, diff --git a/internal/settingsresolve/conformance_test.go b/internal/settingsresolve/conformance_test.go index 6965fcb8..fe8a011d 100644 --- a/internal/settingsresolve/conformance_test.go +++ b/internal/settingsresolve/conformance_test.go @@ -47,20 +47,22 @@ type conformanceCase struct { } type conformanceContext struct { - ProfileID string `json:"profile_id"` - DeviceID string `json:"device_id"` - LibraryIDs []int `json:"library_ids"` - SeriesIDs []string `json:"series_ids"` + ProfileID string `json:"profile_id"` + ClientFamily string `json:"client_family"` + DeviceID string `json:"device_id"` + LibraryIDs []int `json:"library_ids"` + SeriesIDs []string `json:"series_ids"` } type conformanceRow struct { - Key string `json:"key"` - Scope string `json:"scope"` - ProfileID string `json:"profile_id"` - DeviceID string `json:"device_id"` - LibraryID int `json:"library_id"` - SeriesID string `json:"series_id"` - Value json.RawMessage `json:"value"` + Key string `json:"key"` + Scope string `json:"scope"` + ProfileID string `json:"profile_id"` + ClientFamily string `json:"client_family"` + DeviceID string `json:"device_id"` + LibraryID int `json:"library_id"` + SeriesID string `json:"series_id"` + Value json.RawMessage `json:"value"` } type conformanceBinding struct { @@ -146,10 +148,11 @@ func runConformanceCase(t *testing.T, manifest *settingscontract.Manifest, tc co rc := Context{} if tc.Context != nil { rc = Context{ - ProfileID: tc.Context.ProfileID, - DeviceID: tc.Context.DeviceID, - LibraryIDs: tc.Context.LibraryIDs, - SeriesIDs: tc.Context.SeriesIDs, + ProfileID: tc.Context.ProfileID, + ClientFamily: settingscontract.ClientFamily(tc.Context.ClientFamily), + DeviceID: tc.Context.DeviceID, + LibraryIDs: tc.Context.LibraryIDs, + SeriesIDs: tc.Context.SeriesIDs, } } @@ -160,12 +163,13 @@ func runConformanceCase(t *testing.T, manifest *settingscontract.Manifest, tc co } rows = append(rows, userstore.SettingValue{ SettingIdentity: userstore.SettingIdentity{ - Key: stored.Key, - Scope: settingscontract.Scope(stored.Scope), - ProfileID: stored.ProfileID, - DeviceID: stored.DeviceID, - LibraryID: stored.LibraryID, - SeriesID: stored.SeriesID, + Key: stored.Key, + Scope: settingscontract.Scope(stored.Scope), + ProfileID: stored.ProfileID, + ClientFamily: settingscontract.ClientFamily(stored.ClientFamily), + DeviceID: stored.DeviceID, + LibraryID: stored.LibraryID, + SeriesID: stored.SeriesID, }, Value: stored.Value, }) diff --git a/internal/settingsresolve/resolve.go b/internal/settingsresolve/resolve.go index 97c3cc3d..07c7b6dc 100644 --- a/internal/settingsresolve/resolve.go +++ b/internal/settingsresolve/resolve.go @@ -32,8 +32,9 @@ import ( // profile_series. That is what lets one code path serve an identified client, // an anonymous jellycompat seed, and a batch spanning many series. type Context struct { - ProfileID string - DeviceID string + ProfileID string + ClientFamily settingscontract.ClientFamily + DeviceID string // LibraryIDs and SeriesIDs are the content contexts in play. A batch // resolving a season passes every id once rather than resolving per item. LibraryIDs []int @@ -131,11 +132,12 @@ func (r *Resolver) Resolve( } stored, err := store.ListSettingValuesForResolution(ctx, userstore.SettingResolutionQuery{ - Keys: known, - ProfileIDs: nonEmpty(rc.ProfileID), - DeviceID: rc.DeviceID, - LibraryIDs: rc.LibraryIDs, - SeriesIDs: rc.SeriesIDs, + Keys: known, + ProfileIDs: nonEmpty(rc.ProfileID), + ClientFamily: rc.ClientFamily, + DeviceID: rc.DeviceID, + LibraryIDs: rc.LibraryIDs, + SeriesIDs: rc.SeriesIDs, }) if err != nil { return nil, fmt.Errorf("settingsresolve: reading candidates: %w", err) @@ -168,11 +170,15 @@ func (r *Resolver) ResolveContexts( } deviceID := contexts[0].DeviceID - query := userstore.SettingResolutionQuery{Keys: known, DeviceID: deviceID} + clientFamily := contexts[0].ClientFamily + query := userstore.SettingResolutionQuery{Keys: known, ClientFamily: clientFamily, DeviceID: deviceID} for _, rc := range contexts { if rc.DeviceID != deviceID { return nil, fmt.Errorf("settingsresolve: batched contexts use different devices") } + if rc.ClientFamily != clientFamily { + return nil, fmt.Errorf("settingsresolve: batched contexts use different client families") + } if rc.ProfileID != "" { query.ProfileIDs = append(query.ProfileIDs, rc.ProfileID) } @@ -352,6 +358,10 @@ func pickForScope( if row.ProfileID == rc.ProfileID { matches = append(matches, row) } + case settingscontract.ScopeProfileClient: + if row.ProfileID == rc.ProfileID && row.ClientFamily == rc.ClientFamily && rc.ClientFamily.Valid() { + matches = append(matches, row) + } case settingscontract.ScopeProfileDevice: if row.ProfileID == rc.ProfileID && row.DeviceID == rc.DeviceID && rc.DeviceID != "" { matches = append(matches, row) diff --git a/internal/userdb/conformance_test.go b/internal/userdb/conformance_test.go index 22e867e5..f6a2b32d 100644 --- a/internal/userdb/conformance_test.go +++ b/internal/userdb/conformance_test.go @@ -2,7 +2,7 @@ package userdb import ( "context" - "database/sql" + "path/filepath" "testing" "time" @@ -12,15 +12,12 @@ import ( func newConformanceStore(t *testing.T) userstore.UserStore { t.Helper() - db, err := sql.Open("sqlite3", ":memory:") + db, err := NewUserDB(filepath.Join(t.TempDir(), "user.db"), 1) if err != nil { - t.Fatalf("open sqlite: %v", err) + t.Fatalf("open user database: %v", err) } t.Cleanup(func() { _ = db.Close() }) - if err := InitSchema(db); err != nil { - t.Fatalf("InitSchema: %v", err) - } - return NewSQLiteUserStore(db) + return NewSQLiteUserStore(db.DB) } // TestSQLiteProgressSince runs the offline-sync progress-reconciliation diff --git a/internal/userdb/migrate.go b/internal/userdb/migrate.go index 8849b9a0..4e1344a5 100644 --- a/internal/userdb/migrate.go +++ b/internal/userdb/migrate.go @@ -5,7 +5,7 @@ import ( "fmt" ) -const schemaVersion = 17 +const schemaVersion = 18 func runMigrations(db *sql.DB) error { version, err := userVersion(db) @@ -169,9 +169,27 @@ func runMigrations(db *sql.DB) error { } } + if version < 18 { + if err := migrateToV18(tx); err != nil { + return err + } + if _, err := tx.Exec("PRAGMA user_version = 18"); err != nil { + return fmt.Errorf("setting sqlite user_version 18: %w", err) + } + } + return tx.Commit() } +// migrateToV18 seeds the family-neutral navigation shortcut catalog from the +// existing web sidebar pins. InitSchema has already rebuilt +// user_setting_values with the profile_client identity before this versioned +// migration runs; keeping the data conversion here makes it one-time and +// transactional with the schema-version bump. +func migrateToV18(tx *sql.Tx) error { + return migrateSidebarPinsToNavigationShortcuts(tx) +} + // migrateToV17 rehomes the Jellyfin DisplayPreferences blobs from // user_settings (jellycompat:* keys) into the dedicated // jellycompat_displayprefs table, removing the last non-settings tenant of the diff --git a/internal/userdb/migrate_v18_test.go b/internal/userdb/migrate_v18_test.go new file mode 100644 index 00000000..7e140ce7 --- /dev/null +++ b/internal/userdb/migrate_v18_test.go @@ -0,0 +1,220 @@ +package userdb + +import ( + "context" + "database/sql" + "encoding/json" + "reflect" + "testing" + + "github.com/Silo-Server/silo-server/internal/settingscontract" + "github.com/Silo-Server/silo-server/internal/userstore" +) + +const settingValuesSchemaV17 = ` +CREATE TABLE user_setting_values ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + key TEXT NOT NULL, + scope TEXT NOT NULL, + profile_id TEXT, + device_id TEXT, + library_id INTEGER, + series_id TEXT, + value TEXT NOT NULL CHECK (json_valid(value)), + revision INTEGER NOT NULL DEFAULT 1, + created_at TEXT NOT NULL, + updated_at TEXT NOT NULL, + CHECK (scope IN ('account', 'profile', 'profile_device', 'profile_library', 'profile_series')), + CHECK ( + (scope = 'account' AND profile_id IS NULL AND device_id IS NULL AND library_id IS NULL AND series_id IS NULL) OR + (scope = 'profile' AND profile_id IS NOT NULL AND device_id IS NULL AND library_id IS NULL AND series_id IS NULL) OR + (scope = 'profile_device' AND profile_id IS NOT NULL AND device_id IS NOT NULL AND library_id IS NULL AND series_id IS NULL) OR + (scope = 'profile_library' AND profile_id IS NOT NULL AND device_id IS NULL AND library_id IS NOT NULL AND series_id IS NULL) OR + (scope = 'profile_series' AND profile_id IS NOT NULL AND device_id IS NULL AND library_id IS NULL AND series_id IS NOT NULL) + ) +);` + +func TestMigrateToV18AddsClientFamilyAndSeedsShortcuts(t *testing.T) { + db, err := sql.Open("sqlite3", ":memory:") + if err != nil { + t.Fatalf("open sqlite: %v", err) + } + t.Cleanup(func() { _ = db.Close() }) + if err := InitSchema(db); err != nil { + t.Fatalf("initial InitSchema: %v", err) + } + + // Replace only the canonical values table with its released v17 shape. On a + // real open, InitSchema sees exactly this table before runMigrations. + for _, index := range []string{ + "user_setting_values_account_uq", "user_setting_values_profile_uq", + "user_setting_values_profile_client_uq", "user_setting_values_profile_device_uq", + "user_setting_values_profile_library_uq", "user_setting_values_profile_series_uq", + "user_setting_values_resolution_idx", "user_setting_values_series_idx", + "user_setting_values_library_idx", + } { + if _, err := db.Exec("DROP INDEX IF EXISTS " + index); err != nil { + t.Fatalf("drop %s: %v", index, err) + } + } + if _, err := db.Exec("DROP TABLE user_setting_values"); err != nil { + t.Fatalf("drop current values table: %v", err) + } + if _, err := db.Exec(settingValuesSchemaV17); err != nil { + t.Fatalf("create v17 values table: %v", err) + } + if _, err := db.Exec("PRAGMA user_version = 17"); err != nil { + t.Fatalf("set v17: %v", err) + } + + const created = "2026-07-28T10:00:00Z" + const updated = "2026-08-01T12:00:00Z" + legacy := `{"12":[{"type":"collection","id":"classics","label":"Classics"}],"7":[{"type":"section","id":"recently-added","label":"Recently Added"},{"type":"section","id":" ","label":"Whitespace ID"},{"type":"collection","id":"horror","label":"Horror"}],"favorites":[{"type":"collection","id":"keep-me","label":"Legacy group"}]}` + encodedLegacy, err := json.Marshal(legacy) + if err != nil { + t.Fatalf("encode legacy JSON string: %v", err) + } + if _, err := db.Exec(` +INSERT INTO user_setting_values + (key, scope, profile_id, value, revision, created_at, updated_at) +VALUES ('ui.sidebar_pins', 'profile', 'p1', ?, 4, ?, ?), + ('ui.sidebar_pins', 'profile', 'p2', ?, 2, ?, ?), + ('ui.sidebar_pins', 'profile', 'p3', ?, 5, ?, ?), + ('nav.shortcuts', 'profile', 'p2', '{"items":[]}', 3, ?, ?)`, + legacy, created, updated, + legacy, created, updated, + string(encodedLegacy), created, updated, + created, updated, + ); err != nil { + t.Fatalf("seed v17 rows: %v", err) + } + + if err := InitSchema(db); err != nil { + t.Fatalf("upgrade InitSchema: %v", err) + } + if err := runMigrations(db); err != nil { + t.Fatalf("runMigrations: %v", err) + } + version, err := userVersion(db) + if err != nil || version != schemaVersion { + t.Fatalf("user_version = %d (%v), want %d", version, err, schemaVersion) + } + + var columnCount int + if err := db.QueryRow(`SELECT COUNT(*) FROM pragma_table_info('user_setting_values') + WHERE name = 'client_family'`).Scan(&columnCount); err != nil || columnCount != 1 { + t.Fatalf("client_family column count = %d (%v), want 1", columnCount, err) + } + + // The legacy row is preserved byte-for-byte and keeps its revision. + var kept string + var revision int + if err := db.QueryRow(`SELECT value, revision FROM user_setting_values + WHERE key = 'ui.sidebar_pins' AND profile_id = 'p1'`).Scan(&kept, &revision); err != nil { + t.Fatalf("read legacy row: %v", err) + } + if kept != legacy || revision != 4 { + t.Fatalf("legacy row changed to %s at revision %d", kept, revision) + } + + var migrated string + if err := db.QueryRow(`SELECT value FROM user_setting_values + WHERE key = 'nav.shortcuts' AND profile_id = 'p1'`).Scan(&migrated); err != nil { + t.Fatalf("read migrated shortcuts: %v", err) + } + assertJSONValueEqual(t, migrated, `{"items":[{"label":"Recently Added","library_id":7,"section_id":"recently-added","type":"section"},{"collection_id":"horror","label":"Horror","library_id":7,"type":"collection"},{"collection_id":"classics","label":"Classics","library_id":12,"type":"collection"}]}`) + + // Legacy web builds also wrote the grouped object as one JSON string. It is + // preserved as authored while seeding the same canonical shortcut document. + if err := db.QueryRow(`SELECT value, revision FROM user_setting_values + WHERE key = 'ui.sidebar_pins' AND profile_id = 'p3'`).Scan(&kept, &revision); err != nil { + t.Fatalf("read JSON-string legacy row: %v", err) + } + if kept != string(encodedLegacy) || revision != 5 { + t.Fatalf("JSON-string legacy row changed to %s at revision %d", kept, revision) + } + if err := db.QueryRow(`SELECT value FROM user_setting_values + WHERE key = 'nav.shortcuts' AND profile_id = 'p3'`).Scan(&migrated); err != nil { + t.Fatalf("read shortcuts migrated from JSON string: %v", err) + } + assertJSONValueEqual(t, migrated, `{"items":[{"label":"Recently Added","library_id":7,"section_id":"recently-added","type":"section"},{"collection_id":"horror","label":"Horror","library_id":7,"type":"collection"},{"collection_id":"classics","label":"Classics","library_id":12,"type":"collection"}]}`) + + // Existing new-format data wins rather than being overwritten by legacy. + if err := db.QueryRow(`SELECT value, revision FROM user_setting_values + WHERE key = 'nav.shortcuts' AND profile_id = 'p2'`).Scan(&migrated, &revision); err != nil { + t.Fatalf("read pre-existing shortcuts: %v", err) + } + assertJSONValueEqual(t, migrated, `{"items":[]}`) + if revision != 3 { + t.Fatalf("pre-existing shortcut revision = %d, want 3", revision) + } + + store := NewSQLiteUserStore(db) + id := userstore.SettingIdentity{ + Key: "nav.primary_menu", Scope: settingscontract.ScopeProfileClient, + ProfileID: "p1", ClientFamily: settingscontract.ClientFamilyTV, + } + if _, err := store.UpsertSettingValue(context.Background(), id, + json.RawMessage(`{"items":[{"type":"builtin","destination":"home"}]}`)); err != nil { + t.Fatalf("write profile_client value: %v", err) + } + got, err := store.GetSettingValue(context.Background(), id) + if err != nil || got == nil || got.ClientFamily != settingscontract.ClientFamilyTV { + t.Fatalf("profile_client round trip = %+v (%v)", got, err) + } +} + +func TestNavigationShortcutsFromSidebarPinsSkipsMalformedSiblingsAndBoundsLibraryIDs(t *testing.T) { + raw := `{ + "7":[ + {"type":"section","id":"recent","label":"Recent"}, + {"type":"section","id":99,"label":"Wrong ID type"}, + {"type":"collection","id":"horror","label":"Horror"}, + {"type":"collection","id":"bad-label","label":42} + ], + "8":{"type":"collection","id":"not-an-array","label":"Skipped group"}, + "9":[null,{"type":"collection","id":"drama","label":"Drama"}], + "2147483647":[{"type":"section","id":"boundary","label":"Boundary"}], + "2147483648":[{"type":"section","id":"overflow","label":"Overflow"}], + "999999999999999999999999999999":[{"type":"section","id":"huge","label":"Huge"}], + "07":[{"type":"section","id":"leading-zero","label":"Leading Zero"}] + }` + + value, ok := navigationShortcutsFromSidebarPins(raw) + if !ok { + t.Fatal("migration rejected valid siblings because malformed groups/items were present") + } + assertJSONValueEqual(t, string(value), `{"items":[ + {"type":"section","library_id":7,"section_id":"recent","label":"Recent"}, + {"type":"collection","library_id":7,"collection_id":"horror","label":"Horror"}, + {"type":"collection","library_id":9,"collection_id":"drama","label":"Drama"}, + {"type":"section","library_id":2147483647,"section_id":"boundary","label":"Boundary"} + ]}`) +} + +func TestNavigationShortcutsFromSidebarPinsRejectsOnlyOverflowLibraryIDs(t *testing.T) { + for name, raw := range map[string]string{ + "one over int32": `{"2147483648":[{"type":"section","id":"overflow","label":"Overflow"}]}`, + "parse overflow": `{"999999999999999999999999999999":[{"type":"section","id":"huge","label":"Huge"}]}`, + } { + t.Run(name, func(t *testing.T) { + if value, ok := navigationShortcutsFromSidebarPins(raw); ok || value != nil { + t.Fatalf("overflow-only pins migrated as %s", value) + } + }) + } +} + +func assertJSONValueEqual(t *testing.T, got, want string) { + t.Helper() + var gotValue, wantValue any + if err := json.Unmarshal([]byte(got), &gotValue); err != nil { + t.Fatalf("decode got JSON: %v", err) + } + if err := json.Unmarshal([]byte(want), &wantValue); err != nil { + t.Fatalf("decode want JSON: %v", err) + } + if !reflect.DeepEqual(gotValue, wantValue) { + t.Fatalf("JSON = %s, want %s", got, want) + } +} diff --git a/internal/userdb/navigation_shortcuts_migrate.go b/internal/userdb/navigation_shortcuts_migrate.go new file mode 100644 index 00000000..4f36debd --- /dev/null +++ b/internal/userdb/navigation_shortcuts_migrate.go @@ -0,0 +1,171 @@ +package userdb + +import ( + "database/sql" + "encoding/json" + "fmt" + "sort" + "strconv" + "strings" +) + +const ( + maxNavigationShortcuts = 256 + maxMigratedNavigationLibraryID = int64(2_147_483_647) +) + +type legacySidebarPin struct { + Type string `json:"type"` + ID string `json:"id"` + Label string `json:"label"` +} + +// migrateSidebarPinsToNavigationShortcuts copies the subset of the legacy web +// map that has a concrete positive library id into the new cross-client item +// catalog. The old ui.sidebar_pins row remains untouched, including +// non-numeric well-known groups; a pre-existing nav.shortcuts row always wins. +func migrateSidebarPinsToNavigationShortcuts(tx *sql.Tx) error { + rows, err := tx.Query(` +SELECT legacy.profile_id, legacy.value, legacy.created_at, legacy.updated_at + FROM user_setting_values AS legacy + WHERE legacy.key = 'ui.sidebar_pins' + AND legacy.scope = 'profile' + AND legacy.value <> 'null' + AND NOT EXISTS ( + SELECT 1 + FROM user_setting_values AS current + WHERE current.key = 'nav.shortcuts' + AND current.scope = 'profile' + AND current.profile_id = legacy.profile_id + ) + ORDER BY legacy.profile_id`) + if err != nil { + return fmt.Errorf("reading sidebar pins for navigation shortcut migration: %w", err) + } + type source struct { + profileID, raw, createdAt, updatedAt string + } + var sources []source + for rows.Next() { + var entry source + if err := rows.Scan(&entry.profileID, &entry.raw, &entry.createdAt, &entry.updatedAt); err != nil { + _ = rows.Close() + return fmt.Errorf("scanning sidebar pins for navigation shortcut migration: %w", err) + } + sources = append(sources, entry) + } + if err := rows.Close(); err != nil { + return fmt.Errorf("closing sidebar pins migration rows: %w", err) + } + if err := rows.Err(); err != nil { + return fmt.Errorf("iterating sidebar pins for navigation shortcut migration: %w", err) + } + + for _, source := range sources { + value, ok := navigationShortcutsFromSidebarPins(source.raw) + if !ok { + continue + } + if _, err := tx.Exec(` +INSERT OR IGNORE INTO user_setting_values + (key, scope, profile_id, client_family, device_id, library_id, series_id, + value, revision, created_at, updated_at) +VALUES ('nav.shortcuts', 'profile', ?, NULL, NULL, NULL, NULL, ?, 1, ?, ?)`, + source.profileID, string(value), source.createdAt, source.updatedAt, + ); err != nil { + return fmt.Errorf("seeding navigation shortcuts for profile %q: %w", source.profileID, err) + } + } + return nil +} + +func navigationShortcutsFromSidebarPins(raw string) (json.RawMessage, bool) { + payload := []byte(raw) + // The legacy web client sometimes persisted the whole object as a JSON + // string. Match parseSidebarPins by unwrapping exactly one such layer before + // interpreting the grouped pin map. + var encoded string + if err := json.Unmarshal(payload, &encoded); err == nil { + payload = []byte(encoded) + } + + // Decode the object one group at a time. A typed map[string][]T makes one + // non-array group or one malformed member reject every valid sibling in the + // profile. Raw group/member boundaries match PostgreSQL's jsonb_each plus + // jsonb_array_elements behavior: malformed siblings are skipped in place. + var legacy map[string]json.RawMessage + if err := json.Unmarshal(payload, &legacy); err != nil { + return nil, false + } + + items := make([]map[string]any, 0) + seen := make(map[string]struct{}) + // JSON object order is not semantic and Go deliberately randomizes map + // iteration. Sort valid numeric groups by library id, matching the + // PostgreSQL migration's ORDER BY rather than lexical string order. + type libraryGroup struct { + key string + id int + } + groups := make([]libraryGroup, 0, len(legacy)) + for group := range legacy { + libraryID, err := strconv.ParseInt(group, 10, 32) + if err != nil || libraryID <= 0 || libraryID > maxMigratedNavigationLibraryID || + strconv.FormatInt(libraryID, 10) != group { + continue + } + groups = append(groups, libraryGroup{key: group, id: int(libraryID)}) + } + sort.Slice(groups, func(i, j int) bool { return groups[i].id < groups[j].id }) + + for _, group := range groups { + var encodedPins []json.RawMessage + if err := json.Unmarshal(legacy[group.key], &encodedPins); err != nil { + continue + } + for _, encodedPin := range encodedPins { + var pin legacySidebarPin + if err := json.Unmarshal(encodedPin, &pin); err != nil { + continue + } + id := pin.ID + label := pin.Label + if strings.TrimSpace(id) == "" || len([]rune(id)) > 128 || + strings.TrimSpace(label) == "" || len([]rune(label)) > 256 { + continue + } + identity := pin.Type + "\x00" + strconv.Itoa(group.id) + "\x00" + id + if _, duplicate := seen[identity]; duplicate { + continue + } + var item map[string]any + switch pin.Type { + case "section": + item = map[string]any{ + "type": "section", "library_id": group.id, + "section_id": id, "label": label, + } + case "collection": + item = map[string]any{ + "type": "collection", "library_id": group.id, + "collection_id": id, "label": label, + } + default: + continue + } + seen[identity] = struct{}{} + items = append(items, item) + if len(items) == maxNavigationShortcuts { + break + } + } + if len(items) == maxNavigationShortcuts { + break + } + } + if len(items) == 0 { + return nil, false + } + value, err := json.Marshal(map[string]any{"items": items}) + return value, err == nil +} diff --git a/internal/userdb/preference_settings_tx.go b/internal/userdb/preference_settings_tx.go index e4065275..446b1045 100644 --- a/internal/userdb/preference_settings_tx.go +++ b/internal/userdb/preference_settings_tx.go @@ -16,8 +16,11 @@ import ( // ordinary store calls. type preferenceSettingsExecutor interface { Exec(string, ...any) (sql.Result, error) + ExecContext(context.Context, string, ...any) (sql.Result, error) Query(string, ...any) (*sql.Rows, error) + QueryContext(context.Context, string, ...any) (*sql.Rows, error) QueryRow(string, ...any) *sql.Row + QueryRowContext(context.Context, string, ...any) *sql.Row } type preferenceSettingsTx struct { diff --git a/internal/userdb/schema.go b/internal/userdb/schema.go index 3f07b4a3..8e2c656f 100644 --- a/internal/userdb/schema.go +++ b/internal/userdb/schema.go @@ -301,6 +301,7 @@ CREATE TABLE IF NOT EXISTS user_setting_values ( key TEXT NOT NULL, scope TEXT NOT NULL, profile_id TEXT, + client_family TEXT, device_id TEXT, library_id INTEGER, series_id TEXT, @@ -308,13 +309,15 @@ CREATE TABLE IF NOT EXISTS user_setting_values ( revision INTEGER NOT NULL DEFAULT 1, created_at TEXT NOT NULL, updated_at TEXT NOT NULL, - CHECK (scope IN ('account', 'profile', 'profile_device', 'profile_library', 'profile_series')), + CHECK (scope IN ('account', 'profile', 'profile_client', 'profile_device', 'profile_library', 'profile_series')), + CHECK (client_family IS NULL OR client_family IN ('tv', 'mobile', 'tablet', 'desktop', 'web')), CHECK ( - (scope = 'account' AND profile_id IS NULL AND device_id IS NULL AND library_id IS NULL AND series_id IS NULL) OR - (scope = 'profile' AND profile_id IS NOT NULL AND device_id IS NULL AND library_id IS NULL AND series_id IS NULL) OR - (scope = 'profile_device' AND profile_id IS NOT NULL AND device_id IS NOT NULL AND library_id IS NULL AND series_id IS NULL) OR - (scope = 'profile_library' AND profile_id IS NOT NULL AND device_id IS NULL AND library_id IS NOT NULL AND series_id IS NULL) OR - (scope = 'profile_series' AND profile_id IS NOT NULL AND device_id IS NULL AND library_id IS NULL AND series_id IS NOT NULL) + (scope = 'account' AND profile_id IS NULL AND client_family IS NULL AND device_id IS NULL AND library_id IS NULL AND series_id IS NULL) OR + (scope = 'profile' AND profile_id IS NOT NULL AND client_family IS NULL AND device_id IS NULL AND library_id IS NULL AND series_id IS NULL) OR + (scope = 'profile_client' AND profile_id IS NOT NULL AND client_family IS NOT NULL AND device_id IS NULL AND library_id IS NULL AND series_id IS NULL) OR + (scope = 'profile_device' AND profile_id IS NOT NULL AND client_family IS NULL AND device_id IS NOT NULL AND library_id IS NULL AND series_id IS NULL) OR + (scope = 'profile_library' AND profile_id IS NOT NULL AND client_family IS NULL AND device_id IS NULL AND library_id IS NOT NULL AND series_id IS NULL) OR + (scope = 'profile_series' AND profile_id IS NOT NULL AND client_family IS NULL AND device_id IS NULL AND library_id IS NULL AND series_id IS NOT NULL) ) ); @@ -367,6 +370,9 @@ func InitSchema(db *sql.DB) error { if err != nil { return err } + if err := ensureSettingValuesClientFamily(db); err != nil { + return err + } if err := ensureProfileSectionOverridesRemovedColumn(db); err != nil { return err } @@ -400,6 +406,95 @@ func InitSchema(db *sql.DB) error { return backfillUserDevices(db) } +// ensureSettingValuesClientFamily upgrades the canonical settings table before +// runMigrations reads it. InitSchema runs first on every open, and SQLite +// cannot widen a table CHECK constraint with ALTER TABLE, so an existing table +// must be rebuilt here rather than merely gaining a nullable column. The +// rebuild is transactional and preserves ids, revisions, values, and +// timestamps verbatim. +func ensureSettingValuesClientFamily(db *sql.DB) error { + var count int + if err := db.QueryRow( + "SELECT COUNT(*) FROM pragma_table_info(?) WHERE name = ?", + "user_setting_values", "client_family", + ).Scan(&count); err != nil { + return fmt.Errorf("checking user_setting_values.client_family column: %w", err) + } + if count > 0 { + _, err := db.Exec(`CREATE UNIQUE INDEX IF NOT EXISTS user_setting_values_profile_client_uq + ON user_setting_values (profile_id, client_family, key) WHERE scope = 'profile_client'`) + if err != nil { + return fmt.Errorf("creating profile_client setting index: %w", err) + } + return nil + } + + tx, err := db.Begin() + if err != nil { + return fmt.Errorf("beginning profile_client settings migration: %w", err) + } + defer tx.Rollback() //nolint:errcheck + + if _, err := tx.Exec(` +CREATE TABLE user_setting_values_profile_client_new ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + key TEXT NOT NULL, + scope TEXT NOT NULL, + profile_id TEXT, + client_family TEXT, + device_id TEXT, + library_id INTEGER, + series_id TEXT, + value TEXT NOT NULL CHECK (json_valid(value)), + revision INTEGER NOT NULL DEFAULT 1, + created_at TEXT NOT NULL, + updated_at TEXT NOT NULL, + CHECK (scope IN ('account', 'profile', 'profile_client', 'profile_device', 'profile_library', 'profile_series')), + CHECK (client_family IS NULL OR client_family IN ('tv', 'mobile', 'tablet', 'desktop', 'web')), + CHECK ( + (scope = 'account' AND profile_id IS NULL AND client_family IS NULL AND device_id IS NULL AND library_id IS NULL AND series_id IS NULL) OR + (scope = 'profile' AND profile_id IS NOT NULL AND client_family IS NULL AND device_id IS NULL AND library_id IS NULL AND series_id IS NULL) OR + (scope = 'profile_client' AND profile_id IS NOT NULL AND client_family IS NOT NULL AND device_id IS NULL AND library_id IS NULL AND series_id IS NULL) OR + (scope = 'profile_device' AND profile_id IS NOT NULL AND client_family IS NULL AND device_id IS NOT NULL AND library_id IS NULL AND series_id IS NULL) OR + (scope = 'profile_library' AND profile_id IS NOT NULL AND client_family IS NULL AND device_id IS NULL AND library_id IS NOT NULL AND series_id IS NULL) OR + (scope = 'profile_series' AND profile_id IS NOT NULL AND client_family IS NULL AND device_id IS NULL AND library_id IS NULL AND series_id IS NOT NULL) + ) +); +INSERT INTO user_setting_values_profile_client_new + (id, key, scope, profile_id, client_family, device_id, library_id, series_id, + value, revision, created_at, updated_at) +SELECT id, key, scope, profile_id, NULL, device_id, library_id, series_id, + value, revision, created_at, updated_at + FROM user_setting_values; +DROP TABLE user_setting_values; +ALTER TABLE user_setting_values_profile_client_new RENAME TO user_setting_values; +CREATE UNIQUE INDEX user_setting_values_account_uq + ON user_setting_values (key) WHERE scope = 'account'; +CREATE UNIQUE INDEX user_setting_values_profile_uq + ON user_setting_values (profile_id, key) WHERE scope = 'profile'; +CREATE UNIQUE INDEX user_setting_values_profile_client_uq + ON user_setting_values (profile_id, client_family, key) WHERE scope = 'profile_client'; +CREATE UNIQUE INDEX user_setting_values_profile_device_uq + ON user_setting_values (profile_id, device_id, key) WHERE scope = 'profile_device'; +CREATE UNIQUE INDEX user_setting_values_profile_library_uq + ON user_setting_values (profile_id, library_id, key) WHERE scope = 'profile_library'; +CREATE UNIQUE INDEX user_setting_values_profile_series_uq + ON user_setting_values (profile_id, series_id, key) WHERE scope = 'profile_series'; +CREATE INDEX user_setting_values_resolution_idx + ON user_setting_values (profile_id, key, scope); +CREATE INDEX user_setting_values_series_idx + ON user_setting_values (profile_id, series_id); +CREATE INDEX user_setting_values_library_idx + ON user_setting_values (profile_id, library_id); +`); err != nil { + return fmt.Errorf("rebuilding user_setting_values for profile_client: %w", err) + } + if err := tx.Commit(); err != nil { + return fmt.Errorf("committing profile_client settings migration: %w", err) + } + return nil +} + // watchProgressSyncTriggers stamps the server-owned cursor (synced_seq) on every // watch_progress write and owns the LWW key: event_at defaults to the write // time, and an UPDATE that changed updated_at without explicitly changing diff --git a/internal/userdb/setting_mutation_tx.go b/internal/userdb/setting_mutation_tx.go new file mode 100644 index 00000000..2b1ec3d4 --- /dev/null +++ b/internal/userdb/setting_mutation_tx.go @@ -0,0 +1,136 @@ +package userdb + +import ( + "context" + "database/sql" + "database/sql/driver" + "encoding/json" + "fmt" + "log/slog" + + "github.com/Silo-Server/silo-server/internal/userstore" +) + +// settingMutationConnExecutor pins every statement to one database/sql +// connection. SQLite cannot request BEGIN IMMEDIATE through sql.Tx without a +// DSN-wide policy, so this adapter lets the mutation transaction acquire its +// write reservation before it reads the receipt. +type settingMutationConnExecutor struct { + ctx context.Context + conn *sql.Conn +} + +func (e settingMutationConnExecutor) Exec(query string, args ...any) (sql.Result, error) { + return e.conn.ExecContext(e.ctx, query, args...) +} + +func (e settingMutationConnExecutor) ExecContext(ctx context.Context, query string, args ...any) (sql.Result, error) { + return e.conn.ExecContext(ctx, query, args...) +} + +func (e settingMutationConnExecutor) Query(query string, args ...any) (*sql.Rows, error) { + return e.conn.QueryContext(e.ctx, query, args...) +} + +func (e settingMutationConnExecutor) QueryContext(ctx context.Context, query string, args ...any) (*sql.Rows, error) { + return e.conn.QueryContext(ctx, query, args...) +} + +func (e settingMutationConnExecutor) QueryRow(query string, args ...any) *sql.Row { + return e.conn.QueryRowContext(e.ctx, query, args...) +} + +func (e settingMutationConnExecutor) QueryRowContext(ctx context.Context, query string, args ...any) *sql.Row { + return e.conn.QueryRowContext(ctx, query, args...) +} + +type sqliteSettingMutationWriter struct { + exec preferenceSettingsExecutor +} + +var _ userstore.SettingMutationTransactioner = (*SQLiteUserStore)(nil) +var _ userstore.SettingMutationWriter = (*sqliteSettingMutationWriter)(nil) + +// WithSettingMutationTransaction uses BEGIN IMMEDIATE so same-database +// contenders serialize before checking mutation_id. The setting row and its +// receipt then commit together; rollback covers callback errors and crashes. +func (s *SQLiteUserStore) WithSettingMutationTransaction( + ctx context.Context, + mutationID string, + fn func(userstore.SettingMutationWriter) error, +) (err error) { + if mutationID == "" { + return fmt.Errorf("setting mutation transaction requires a mutation id") + } + conn, err := s.db.Conn(ctx) + if err != nil { + return fmt.Errorf("opening setting mutation connection: %w", err) + } + defer conn.Close() //nolint:errcheck + + exec := settingMutationConnExecutor{ctx: ctx, conn: conn} + if _, err := exec.ExecContext(ctx, "BEGIN IMMEDIATE"); err != nil { + return fmt.Errorf("beginning setting mutation transaction: %w", err) + } + committed := false + defer func() { + if !committed { + rollbackCtx := context.WithoutCancel(ctx) + if _, rollbackErr := exec.ExecContext(rollbackCtx, "ROLLBACK"); rollbackErr != nil { + slog.ErrorContext(rollbackCtx, "rolling back setting mutation transaction failed", + "component", "userdb.settings", "error", rollbackErr) + // A failed rollback can leave SQLite's write reservation attached to + // the pooled driver connection. Mark it bad so database/sql discards + // it instead of returning a poisoned connection to the pool. + _ = conn.Raw(func(any) error { return driver.ErrBadConn }) + } + } + }() + + if err := fn(&sqliteSettingMutationWriter{exec: exec}); err != nil { + return err + } + if _, err := exec.ExecContext(ctx, "COMMIT"); err != nil { + return fmt.Errorf("committing setting mutation transaction: %w", err) + } + committed = true + return nil +} + +func (w *sqliteSettingMutationWriter) GetSettingValue( + _ context.Context, + id userstore.SettingIdentity, +) (*userstore.SettingValue, error) { + return getSettingValue(w.exec, id) +} + +func (w *sqliteSettingMutationWriter) UpsertSettingValue( + _ context.Context, + id userstore.SettingIdentity, + value json.RawMessage, +) (*userstore.SettingValue, error) { + return upsertSettingValue(w.exec, id, value) +} + +func (w *sqliteSettingMutationWriter) CompareAndSetSettingValue( + ctx context.Context, + id userstore.SettingIdentity, + value json.RawMessage, + expectedRevision int64, +) (*userstore.SettingValue, error) { + return compareAndSetSettingValue(ctx, w.exec, id, value, expectedRevision) +} + +func (w *sqliteSettingMutationWriter) GetSettingMutation( + _ context.Context, + mutationID string, +) (*userstore.SettingMutationRecord, error) { + return getSettingMutation(w.exec, mutationID) +} + +func (w *sqliteSettingMutationWriter) PutSettingMutation( + _ context.Context, + record userstore.SettingMutationRecord, +) (userstore.SettingMutationRecord, bool, error) { + return putSettingMutation(w.exec, record) +} diff --git a/internal/userdb/setting_values.go b/internal/userdb/setting_values.go index 6c11dfe5..fcccbd4c 100644 --- a/internal/userdb/setting_values.go +++ b/internal/userdb/setting_values.go @@ -1,6 +1,7 @@ package userdb import ( + "context" "database/sql" "encoding/json" "errors" @@ -14,7 +15,7 @@ import ( // settingValueColumns is the projection every read shares, in the order // scanSettingValue expects. -const settingValueColumns = `key, scope, profile_id, device_id, library_id, series_id, +const settingValueColumns = `key, scope, profile_id, client_family, device_id, library_id, series_id, value, revision, created_at, updated_at` // settingConflictTargets maps a scope to the partial unique index that enforces @@ -23,6 +24,7 @@ const settingValueColumns = `key, scope, profile_id, device_id, library_id, seri var settingConflictTargets = map[settingscontract.Scope]string{ settingscontract.ScopeAccount: "(key) WHERE scope = 'account'", settingscontract.ScopeProfile: "(profile_id, key) WHERE scope = 'profile'", + settingscontract.ScopeProfileClient: "(profile_id, client_family, key) WHERE scope = 'profile_client'", settingscontract.ScopeProfileDevice: "(profile_id, device_id, key) WHERE scope = 'profile_device'", settingscontract.ScopeProfileLibrary: "(profile_id, library_id, key) WHERE scope = 'profile_library'", settingscontract.ScopeProfileSeries: "(profile_id, series_id, key) WHERE scope = 'profile_series'", @@ -38,6 +40,9 @@ func settingIdentityPredicate(id userstore.SettingIdentity) (string, []any) { case settingscontract.ScopeProfile: args = append(args, id.ProfileID) clause += " AND profile_id = ?" + case settingscontract.ScopeProfileClient: + args = append(args, id.ProfileID, string(id.ClientFamily)) + clause += " AND profile_id = ? AND client_family = ?" case settingscontract.ScopeProfileDevice: args = append(args, id.ProfileID, id.DeviceID) clause += " AND profile_id = ? AND device_id = ?" @@ -54,11 +59,15 @@ func settingIdentityPredicate(id userstore.SettingIdentity) (string, []any) { // GetSettingValue returns the explicit value at exactly one scope, or nil when // that identity is unset. func GetSettingValue(db *sql.DB, id userstore.SettingIdentity) (*userstore.SettingValue, error) { + return getSettingValue(db, id) +} + +func getSettingValue(exec preferenceSettingsExecutor, id userstore.SettingIdentity) (*userstore.SettingValue, error) { if err := id.Validate(); err != nil { return nil, err } clause, args := settingIdentityPredicate(id) - row := db.QueryRow("SELECT "+settingValueColumns+" FROM user_setting_values WHERE "+clause, args...) + row := exec.QueryRow("SELECT "+settingValueColumns+" FROM user_setting_values WHERE "+clause, args...) value, err := scanSettingValue(row) if errors.Is(err, sql.ErrNoRows) { return nil, nil @@ -70,9 +79,9 @@ func GetSettingValue(db *sql.DB, id userstore.SettingIdentity) (*userstore.Setti } // ListSettingValuesForResolution collects every candidate row for a resolution -// request in one query. The predicate covers all five scopes at once: ranking by -// each definition's resolution order happens in Go, so a four-scope chain still -// costs one round trip rather than four lookups per key. +// request in one query. The predicate covers all six scopes at once: ranking by +// each definition's resolution order happens in Go, so a multi-scope chain still +// costs one round trip rather than one lookup per scope and key. func ListSettingValuesForResolution( db *sql.DB, query userstore.SettingResolutionQuery, @@ -82,14 +91,14 @@ func ListSettingValuesForResolution( return nil, nil } - args := make([]any, 0, len(q.Keys)+len(q.ProfileIDs)+len(q.LibraryIDs)+len(q.SeriesIDs)+1) + args := make([]any, 0, len(q.Keys)+len(q.ProfileIDs)+len(q.LibraryIDs)+len(q.SeriesIDs)+2) for _, key := range q.Keys { args = append(args, key) } for _, profileID := range q.ProfileIDs { args = append(args, profileID) } - args = append(args, q.DeviceID) + args = append(args, string(q.ClientFamily), q.DeviceID) for _, libraryID := range q.LibraryIDs { args = append(args, libraryID) } @@ -123,13 +132,14 @@ func ListSettingValuesForResolution( `+profileClause+` AND ( scope = 'profile' + OR (scope = 'profile_client' AND client_family = ?) OR (scope = 'profile_device' AND device_id = ?) OR (`+libraryClause+`) OR (`+seriesClause+`) ) ) ) - ORDER BY key, scope, COALESCE(profile_id, ''), COALESCE(device_id, ''), + ORDER BY key, scope, COALESCE(profile_id, ''), COALESCE(client_family, ''), COALESCE(device_id, ''), COALESCE(library_id, 0), COALESCE(series_id, '')`, args..., ) @@ -157,7 +167,7 @@ func ListAllSettingValues(db *sql.DB) ([]userstore.SettingValue, error) { rows, err := db.Query(` SELECT ` + settingValueColumns + ` FROM user_setting_values - ORDER BY key, scope, COALESCE(profile_id, ''), COALESCE(device_id, ''), + ORDER BY key, scope, COALESCE(profile_id, ''), COALESCE(client_family, ''), COALESCE(device_id, ''), COALESCE(library_id, 0), COALESCE(series_id, '')`) if err != nil { return nil, fmt.Errorf("listing all setting values: %w", err) @@ -204,15 +214,15 @@ func upsertSettingValue( now := nowRFC3339() row := exec.QueryRow(fmt.Sprintf(` INSERT INTO user_setting_values - (key, scope, profile_id, device_id, library_id, series_id, value, created_at, updated_at) - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?) + (key, scope, profile_id, client_family, device_id, library_id, series_id, value, created_at, updated_at) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?) ON CONFLICT %s DO UPDATE SET value = excluded.value, revision = user_setting_values.revision + 1, updated_at = excluded.updated_at RETURNING %s`, target, settingValueColumns), id.Key, string(id.Scope), - nullableText(id.ProfileID), nullableText(id.DeviceID), + nullableText(id.ProfileID), nullableText(string(id.ClientFamily)), nullableText(id.DeviceID), nullableInt(id.LibraryID), nullableText(id.SeriesID), string(value), now, now, ) @@ -223,6 +233,76 @@ func upsertSettingValue( return &stored, nil } +// CompareAndSetSettingValue writes only when expectedRevision still names the +// current row. Revision zero is the insert-if-absent case. Each branch is one +// SQLite statement, so the comparison and write are indivisible even when two +// clients read the same starting document. +func CompareAndSetSettingValue( + ctx context.Context, + db *sql.DB, + id userstore.SettingIdentity, + value json.RawMessage, + expectedRevision int64, +) (*userstore.SettingValue, error) { + return compareAndSetSettingValue(ctx, db, id, value, expectedRevision) +} + +func compareAndSetSettingValue( + ctx context.Context, + exec preferenceSettingsExecutor, + id userstore.SettingIdentity, + value json.RawMessage, + expectedRevision int64, +) (*userstore.SettingValue, error) { + if err := id.Validate(); err != nil { + return nil, err + } + if err := userstore.ValidateSettingValueJSON(value); err != nil { + return nil, err + } + if expectedRevision < 0 { + return nil, fmt.Errorf("%w: expected revision must be non-negative", userstore.ErrInvalidSettingIdentity) + } + + now := nowRFC3339() + var row *sql.Row + if expectedRevision == 0 { + target, ok := settingConflictTargets[id.Scope] + if !ok { + return nil, fmt.Errorf("%w: %q has no storage identity", userstore.ErrInvalidSettingIdentity, id.Scope) + } + row = exec.QueryRowContext(ctx, fmt.Sprintf(` + INSERT INTO user_setting_values + (key, scope, profile_id, client_family, device_id, library_id, series_id, value, created_at, updated_at) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?) + ON CONFLICT %s DO NOTHING + RETURNING %s`, target, settingValueColumns), + id.Key, string(id.Scope), + nullableText(id.ProfileID), nullableText(string(id.ClientFamily)), nullableText(id.DeviceID), + nullableInt(id.LibraryID), nullableText(id.SeriesID), + string(value), now, now, + ) + } else { + clause, args := settingIdentityPredicate(id) + args = append([]any{string(value), now}, args...) + args = append(args, expectedRevision) + row = exec.QueryRowContext(ctx, ` + UPDATE user_setting_values + SET value = ?, revision = revision + 1, updated_at = ? + WHERE `+clause+` AND revision = ? + RETURNING `+settingValueColumns, args...) + } + + stored, err := scanSettingValue(row) + if errors.Is(err, sql.ErrNoRows) { + return nil, userstore.ErrSettingValueRevisionConflict + } + if err != nil { + return nil, fmt.Errorf("compare-and-setting value %q at %s: %w", id.Key, id.Scope, err) + } + return &stored, nil +} + // DeleteSettingValue removes the explicit value at one scope — the `unset` // operation — and reports whether a row existed. func DeleteSettingValue(db *sql.DB, id userstore.SettingIdentity) (bool, error) { @@ -286,7 +366,11 @@ func execSettingValueDelete(db *sql.DB, query, subject string, args ...any) (int // GetSettingMutation returns a recorded idempotency receipt, or nil. func GetSettingMutation(db *sql.DB, mutationID string) (*userstore.SettingMutationRecord, error) { - row := db.QueryRow(` + return getSettingMutation(db, mutationID) +} + +func getSettingMutation(exec preferenceSettingsExecutor, mutationID string) (*userstore.SettingMutationRecord, error) { + row := exec.QueryRow(` SELECT mutation_id, request_hash, result, created_at, expires_at FROM user_setting_mutations WHERE mutation_id = ?`, mutationID) @@ -307,12 +391,19 @@ func GetSettingMutation(db *sql.DB, mutationID string) (*userstore.SettingMutati func PutSettingMutation( db *sql.DB, record userstore.SettingMutationRecord, +) (userstore.SettingMutationRecord, bool, error) { + return putSettingMutation(db, record) +} + +func putSettingMutation( + exec preferenceSettingsExecutor, + record userstore.SettingMutationRecord, ) (userstore.SettingMutationRecord, bool, error) { if err := record.Validate(); err != nil { return userstore.SettingMutationRecord{}, false, err } - row := db.QueryRow(` + row := exec.QueryRow(` INSERT INTO user_setting_mutations (mutation_id, request_hash, result, created_at, expires_at) VALUES (?, ?, ?, ?, ?) ON CONFLICT (mutation_id) DO NOTHING @@ -328,7 +419,7 @@ func PutSettingMutation( return userstore.SettingMutationRecord{}, false, fmt.Errorf("recording setting mutation %q: %w", record.MutationID, err) } - existing, err := GetSettingMutation(db, record.MutationID) + existing, err := getSettingMutation(exec, record.MutationID) if err != nil { return userstore.SettingMutationRecord{}, false, err } @@ -365,22 +456,24 @@ type sqlRow interface { func scanSettingValue(row sqlRow) (userstore.SettingValue, error) { var ( - value userstore.SettingValue - scope string - profileID sql.NullString - deviceID sql.NullString - libraryID sql.NullInt64 - seriesID sql.NullString - raw string + value userstore.SettingValue + scope string + profileID sql.NullString + clientFamily sql.NullString + deviceID sql.NullString + libraryID sql.NullInt64 + seriesID sql.NullString + raw string ) if err := row.Scan( - &value.Key, &scope, &profileID, &deviceID, &libraryID, &seriesID, + &value.Key, &scope, &profileID, &clientFamily, &deviceID, &libraryID, &seriesID, &raw, &value.Revision, &value.CreatedAt, &value.UpdatedAt, ); err != nil { return userstore.SettingValue{}, err } value.Scope = settingscontract.Scope(scope) value.ProfileID = profileID.String + value.ClientFamily = settingscontract.ClientFamily(clientFamily.String) value.DeviceID = deviceID.String value.LibraryID = int(libraryID.Int64) value.SeriesID = seriesID.String diff --git a/internal/userdb/sqlitestore.go b/internal/userdb/sqlitestore.go index 29f6d5d6..f532abb6 100644 --- a/internal/userdb/sqlitestore.go +++ b/internal/userdb/sqlitestore.go @@ -466,6 +466,10 @@ func (s *SQLiteUserStore) UpsertSettingValue(_ context.Context, id userstore.Set return UpsertSettingValue(s.db, id, value) } +func (s *SQLiteUserStore) CompareAndSetSettingValue(ctx context.Context, id userstore.SettingIdentity, value json.RawMessage, expectedRevision int64) (*userstore.SettingValue, error) { + return CompareAndSetSettingValue(ctx, s.db, id, value, expectedRevision) +} + func (s *SQLiteUserStore) DeleteSettingValue(_ context.Context, id userstore.SettingIdentity) (bool, error) { return DeleteSettingValue(s.db, id) } diff --git a/internal/userstore/pgstore/setting_mutation_tx.go b/internal/userstore/pgstore/setting_mutation_tx.go new file mode 100644 index 00000000..742bce10 --- /dev/null +++ b/internal/userstore/pgstore/setting_mutation_tx.go @@ -0,0 +1,96 @@ +package pgstore + +import ( + "context" + "encoding/json" + "fmt" + "hash/fnv" + "strconv" + + "github.com/Silo-Server/silo-server/internal/userstore" +) + +const settingMutationAdvisoryClass int32 = 0x534d5554 // "SMUT" + +type postgresSettingMutationWriter struct { + exec preferenceSettingsExecutor + userID int +} + +var _ userstore.SettingMutationTransactioner = (*PostgresUserStore)(nil) +var _ userstore.SettingMutationWriter = (*postgresSettingMutationWriter)(nil) + +// WithSettingMutationTransaction takes a transaction-scoped advisory lock +// before reading mutation_id. The hash includes the user, so unrelated +// accounts and mutation ids remain concurrent; a hash collision only causes +// harmless extra serialization. The setting and receipt commit together. +func (s *PostgresUserStore) WithSettingMutationTransaction( + ctx context.Context, + mutationID string, + fn func(userstore.SettingMutationWriter) error, +) error { + if mutationID == "" { + return fmt.Errorf("setting mutation transaction requires a mutation id") + } + tx, err := s.pool.Begin(ctx) + if err != nil { + return fmt.Errorf("beginning setting mutation transaction: %w", err) + } + defer tx.Rollback(ctx) //nolint:errcheck + + lockHash := fnv.New32a() + _, _ = lockHash.Write([]byte(strconv.Itoa(s.userID))) + _, _ = lockHash.Write([]byte{0}) + _, _ = lockHash.Write([]byte(mutationID)) + if _, err := tx.Exec(ctx, "SELECT pg_advisory_xact_lock($1, $2)", + settingMutationAdvisoryClass, int32(lockHash.Sum32())); err != nil { + return fmt.Errorf("locking setting mutation transaction: %w", err) + } + + writer := &postgresSettingMutationWriter{exec: tx, userID: s.userID} + if err := fn(writer); err != nil { + return err + } + if err := tx.Commit(ctx); err != nil { + return fmt.Errorf("committing setting mutation transaction: %w", err) + } + return nil +} + +func (w *postgresSettingMutationWriter) GetSettingValue( + ctx context.Context, + id userstore.SettingIdentity, +) (*userstore.SettingValue, error) { + return getSettingValue(ctx, w.exec, w.userID, id) +} + +func (w *postgresSettingMutationWriter) UpsertSettingValue( + ctx context.Context, + id userstore.SettingIdentity, + value json.RawMessage, +) (*userstore.SettingValue, error) { + return upsertSettingValue(ctx, w.exec, w.userID, id, value) +} + +func (w *postgresSettingMutationWriter) CompareAndSetSettingValue( + ctx context.Context, + id userstore.SettingIdentity, + value json.RawMessage, + expectedRevision int64, +) (*userstore.SettingValue, error) { + return compareAndSetSettingValue(ctx, w.exec, w.userID, id, value, expectedRevision) +} + +func (w *postgresSettingMutationWriter) GetSettingMutation( + ctx context.Context, + mutationID string, +) (*userstore.SettingMutationRecord, error) { + return getSettingMutation(ctx, w.exec, w.userID, mutationID) +} + +func (w *postgresSettingMutationWriter) PutSettingMutation( + ctx context.Context, + record userstore.SettingMutationRecord, +) (userstore.SettingMutationRecord, bool, error) { + return putSettingMutation(ctx, w.exec, w.userID, record) +} diff --git a/internal/userstore/pgstore/setting_values.go b/internal/userstore/pgstore/setting_values.go index f93373f8..126da1d0 100644 --- a/internal/userstore/pgstore/setting_values.go +++ b/internal/userstore/pgstore/setting_values.go @@ -15,7 +15,7 @@ import ( // settingValueColumns is the projection every read shares, in the order // scanSettingValue expects. -const settingValueColumns = `key, scope, profile_id, device_id, library_id, series_id, +const settingValueColumns = `key, scope, profile_id, client_family, device_id, library_id, series_id, value, revision, created_at, updated_at` // settingConflictTargets maps a scope to the partial unique index that enforces @@ -24,6 +24,7 @@ const settingValueColumns = `key, scope, profile_id, device_id, library_id, seri var settingConflictTargets = map[settingscontract.Scope]string{ settingscontract.ScopeAccount: "(user_id, key) WHERE scope = 'account'", settingscontract.ScopeProfile: "(user_id, profile_id, key) WHERE scope = 'profile'", + settingscontract.ScopeProfileClient: "(user_id, profile_id, client_family, key) WHERE scope = 'profile_client'", settingscontract.ScopeProfileDevice: "(user_id, profile_id, device_id, key) WHERE scope = 'profile_device'", settingscontract.ScopeProfileLibrary: "(user_id, profile_id, library_id, key) WHERE scope = 'profile_library'", settingscontract.ScopeProfileSeries: "(user_id, profile_id, series_id, key) WHERE scope = 'profile_series'", @@ -39,6 +40,9 @@ func settingIdentityPredicate(userID int, id userstore.SettingIdentity) (string, case settingscontract.ScopeProfile: args = append(args, id.ProfileID) clause += " AND profile_id = $4" + case settingscontract.ScopeProfileClient: + args = append(args, id.ProfileID, string(id.ClientFamily)) + clause += " AND profile_id = $4 AND client_family = $5" case settingscontract.ScopeProfileDevice: args = append(args, id.ProfileID, id.DeviceID) clause += " AND profile_id = $4 AND device_id = $5" @@ -55,12 +59,21 @@ func settingIdentityPredicate(userID int, id userstore.SettingIdentity) (string, func (s *PostgresUserStore) GetSettingValue( ctx context.Context, id userstore.SettingIdentity, +) (*userstore.SettingValue, error) { + return getSettingValue(ctx, s.pool, s.userID, id) +} + +func getSettingValue( + ctx context.Context, + exec preferenceSettingsExecutor, + userID int, + id userstore.SettingIdentity, ) (*userstore.SettingValue, error) { if err := id.Validate(); err != nil { return nil, err } - clause, args := settingIdentityPredicate(s.userID, id) - row := s.pool.QueryRow(ctx, + clause, args := settingIdentityPredicate(userID, id) + row := exec.QueryRow(ctx, "SELECT "+settingValueColumns+" FROM user_setting_values WHERE "+clause, args..., ) @@ -75,9 +88,9 @@ func (s *PostgresUserStore) GetSettingValue( } // ListSettingValuesForResolution collects every candidate row for a resolution -// request in one query. The predicate covers all five scopes at once: ranking by -// each definition's resolution order happens in Go, so a four-scope chain still -// costs one round trip and one index scan rather than four lookups per key. +// request in one query. The predicate covers all six scopes at once: ranking by +// each definition's resolution order happens in Go, so a multi-scope chain still +// costs one round trip and one index scan rather than one lookup per scope and key. func (s *PostgresUserStore) ListSettingValuesForResolution( ctx context.Context, query userstore.SettingResolutionQuery, @@ -96,17 +109,18 @@ func (s *PostgresUserStore) ListSettingValuesForResolution( scope = 'account' OR ( profile_id = ANY($3::text[]) - AND ( - scope = 'profile' - OR (scope = 'profile_device' AND device_id = $4) - OR (scope = 'profile_library' AND library_id = ANY($5::int[])) - OR (scope = 'profile_series' AND series_id = ANY($6::text[])) - ) - ) - ) - ORDER BY key, scope, COALESCE(profile_id, ''), COALESCE(device_id, ''), + AND ( + scope = 'profile' + OR (scope = 'profile_client' AND client_family = $4) + OR (scope = 'profile_device' AND device_id = $5) + OR (scope = 'profile_library' AND library_id = ANY($6::int[])) + OR (scope = 'profile_series' AND series_id = ANY($7::text[])) + ) + ) + ) + ORDER BY key, scope, COALESCE(profile_id, ''), COALESCE(client_family, ''), COALESCE(device_id, ''), COALESCE(library_id, 0), COALESCE(series_id, '')`, - s.userID, q.Keys, q.ProfileIDs, q.DeviceID, q.LibraryIDs, q.SeriesIDs, + s.userID, q.Keys, q.ProfileIDs, string(q.ClientFamily), q.DeviceID, q.LibraryIDs, q.SeriesIDs, ) if err != nil { return nil, fmt.Errorf("listing setting values for resolution: %w", err) @@ -133,7 +147,7 @@ func (s *PostgresUserStore) ListAllSettingValues(ctx context.Context) ([]usersto SELECT `+settingValueColumns+` FROM user_setting_values WHERE user_id = $1 - ORDER BY key, scope, COALESCE(profile_id, ''), COALESCE(device_id, ''), + ORDER BY key, scope, COALESCE(profile_id, ''), COALESCE(client_family, ''), COALESCE(device_id, ''), COALESCE(library_id, 0), COALESCE(series_id, '')`, s.userID, ) @@ -181,15 +195,15 @@ func upsertSettingValue( row := exec.QueryRow(ctx, fmt.Sprintf(` INSERT INTO user_setting_values - (user_id, key, scope, profile_id, device_id, library_id, series_id, value) - VALUES ($1, $2, $3, $4, $5, $6, $7, $8) + (user_id, key, scope, profile_id, client_family, device_id, library_id, series_id, value) + VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9) ON CONFLICT %s DO UPDATE SET value = excluded.value, revision = user_setting_values.revision + 1, updated_at = now() RETURNING %s`, target, settingValueColumns), userID, id.Key, string(id.Scope), - nullableText(id.ProfileID), nullableText(id.DeviceID), + nullableText(id.ProfileID), nullableText(string(id.ClientFamily)), nullableText(id.DeviceID), nullableInt(id.LibraryID), nullableText(id.SeriesID), []byte(value), ) @@ -200,6 +214,75 @@ func upsertSettingValue( return &stored, nil } +// CompareAndSetSettingValue writes only when expectedRevision still names the +// current row. Revision zero is insert-if-absent. PostgreSQL performs either +// comparison and write in one statement, so concurrent document mutations can +// retry without overwriting one another. +func (s *PostgresUserStore) CompareAndSetSettingValue( + ctx context.Context, + id userstore.SettingIdentity, + value json.RawMessage, + expectedRevision int64, +) (*userstore.SettingValue, error) { + return compareAndSetSettingValue(ctx, s.pool, s.userID, id, value, expectedRevision) +} + +func compareAndSetSettingValue( + ctx context.Context, + exec preferenceSettingsExecutor, + userID int, + id userstore.SettingIdentity, + value json.RawMessage, + expectedRevision int64, +) (*userstore.SettingValue, error) { + if err := id.Validate(); err != nil { + return nil, err + } + if err := userstore.ValidateSettingValueJSON(value); err != nil { + return nil, err + } + if expectedRevision < 0 { + return nil, fmt.Errorf("%w: expected revision must be non-negative", userstore.ErrInvalidSettingIdentity) + } + + var row pgx.Row + if expectedRevision == 0 { + target, ok := settingConflictTargets[id.Scope] + if !ok { + return nil, fmt.Errorf("%w: %q has no storage identity", userstore.ErrInvalidSettingIdentity, id.Scope) + } + row = exec.QueryRow(ctx, fmt.Sprintf(` + INSERT INTO user_setting_values + (user_id, key, scope, profile_id, client_family, device_id, library_id, series_id, value) + VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9) + ON CONFLICT %s DO NOTHING + RETURNING %s`, target, settingValueColumns), + userID, id.Key, string(id.Scope), + nullableText(id.ProfileID), nullableText(string(id.ClientFamily)), nullableText(id.DeviceID), + nullableInt(id.LibraryID), nullableText(id.SeriesID), []byte(value), + ) + } else { + clause, args := settingIdentityPredicate(userID, id) + valuePosition := len(args) + 1 + revisionPosition := valuePosition + 1 + args = append(args, []byte(value), expectedRevision) + row = exec.QueryRow(ctx, fmt.Sprintf(` + UPDATE user_setting_values + SET value = $%d, revision = revision + 1, updated_at = now() + WHERE %s AND revision = $%d + RETURNING %s`, valuePosition, clause, revisionPosition, settingValueColumns), args...) + } + + stored, err := scanSettingValue(row) + if errors.Is(err, pgx.ErrNoRows) { + return nil, userstore.ErrSettingValueRevisionConflict + } + if err != nil { + return nil, fmt.Errorf("compare-and-setting value %q at %s: %w", id.Key, id.Scope, err) + } + return &stored, nil +} + func (s *PostgresUserStore) DeleteSettingValue(ctx context.Context, id userstore.SettingIdentity) (bool, error) { return deleteSettingValue(ctx, s.pool, s.userID, id) } @@ -275,11 +358,20 @@ func (s *PostgresUserStore) GetSettingMutation( ctx context.Context, mutationID string, ) (*userstore.SettingMutationRecord, error) { - row := s.pool.QueryRow(ctx, ` + return getSettingMutation(ctx, s.pool, s.userID, mutationID) +} + +func getSettingMutation( + ctx context.Context, + exec preferenceSettingsExecutor, + userID int, + mutationID string, +) (*userstore.SettingMutationRecord, error) { + row := exec.QueryRow(ctx, ` SELECT mutation_id, request_hash, result, created_at, expires_at FROM user_setting_mutations WHERE user_id = $1 AND mutation_id = $2`, - s.userID, mutationID, + userID, mutationID, ) record, err := scanSettingMutation(row) if errors.Is(err, pgx.ErrNoRows) { @@ -298,17 +390,26 @@ func (s *PostgresUserStore) GetSettingMutation( func (s *PostgresUserStore) PutSettingMutation( ctx context.Context, record userstore.SettingMutationRecord, +) (userstore.SettingMutationRecord, bool, error) { + return putSettingMutation(ctx, s.pool, s.userID, record) +} + +func putSettingMutation( + ctx context.Context, + exec preferenceSettingsExecutor, + userID int, + record userstore.SettingMutationRecord, ) (userstore.SettingMutationRecord, bool, error) { if err := record.Validate(); err != nil { return userstore.SettingMutationRecord{}, false, err } - row := s.pool.QueryRow(ctx, ` + row := exec.QueryRow(ctx, ` INSERT INTO user_setting_mutations (user_id, mutation_id, request_hash, result, expires_at) VALUES ($1, $2, $3, $4, $5) ON CONFLICT (user_id, mutation_id) DO NOTHING RETURNING mutation_id, request_hash, result, created_at, expires_at`, - s.userID, record.MutationID, record.RequestHash, []byte(record.Result), record.ExpiresAt, + userID, record.MutationID, record.RequestHash, []byte(record.Result), record.ExpiresAt, ) stored, err := scanSettingMutation(row) if err == nil { @@ -318,7 +419,7 @@ func (s *PostgresUserStore) PutSettingMutation( return userstore.SettingMutationRecord{}, false, fmt.Errorf("recording setting mutation %q: %w", record.MutationID, err) } - existing, err := s.GetSettingMutation(ctx, record.MutationID) + existing, err := getSettingMutation(ctx, exec, userID, record.MutationID) if err != nil { return userstore.SettingMutationRecord{}, false, err } @@ -349,18 +450,19 @@ type pgxRow interface { func scanSettingValue(row pgxRow) (userstore.SettingValue, error) { var ( - value userstore.SettingValue - scope string - profileID *string - deviceID *string - libraryID *int - seriesID *string - raw []byte - createdAt time.Time - updatedAt time.Time + value userstore.SettingValue + scope string + profileID *string + clientFamily *string + deviceID *string + libraryID *int + seriesID *string + raw []byte + createdAt time.Time + updatedAt time.Time ) if err := row.Scan( - &value.Key, &scope, &profileID, &deviceID, &libraryID, &seriesID, + &value.Key, &scope, &profileID, &clientFamily, &deviceID, &libraryID, &seriesID, &raw, &value.Revision, &createdAt, &updatedAt, ); err != nil { return userstore.SettingValue{}, err @@ -369,6 +471,9 @@ func scanSettingValue(row pgxRow) (userstore.SettingValue, error) { if profileID != nil { value.ProfileID = *profileID } + if clientFamily != nil { + value.ClientFamily = settingscontract.ClientFamily(*clientFamily) + } if deviceID != nil { value.DeviceID = *deviceID } diff --git a/internal/userstore/settingvalues.go b/internal/userstore/settingvalues.go index 6d0516fb..a7b5a84f 100644 --- a/internal/userstore/settingvalues.go +++ b/internal/userstore/settingvalues.go @@ -1,6 +1,7 @@ package userstore import ( + "context" "encoding/json" "errors" "fmt" @@ -23,6 +24,13 @@ var ErrInvalidSettingIdentity = errors.New("invalid setting identity") // validation path. var ErrInvalidSettingValue = errors.New("invalid setting value") +// ErrSettingValueRevisionConflict reports that a compare-and-set write did +// not observe the revision it expected. It is deliberately a storage-level +// primitive rather than a public settings precondition: semantic mutation +// endpoints use it to merge concurrent edits without making every client +// implement a read/rebase/retry loop. +var ErrSettingValueRevisionConflict = errors.New("setting value revision conflict") + // SettingIdentity addresses exactly one canonical setting row: the key plus the // context columns its scope requires. // @@ -33,10 +41,11 @@ type SettingIdentity struct { Key string Scope settingscontract.Scope - ProfileID string - DeviceID string - LibraryID int - SeriesID string + ProfileID string + ClientFamily settingscontract.ClientFamily + DeviceID string + LibraryID int + SeriesID string } // Validate reports whether the identity is addressable. It mirrors the scope @@ -51,10 +60,11 @@ type SettingIdentity struct { // p1 — a silently orphaned row. func (id SettingIdentity) Validate() error { for field, value := range map[string]string{ - "key": id.Key, - "profile id": id.ProfileID, - "device id": id.DeviceID, - "series id": id.SeriesID, + "key": id.Key, + "profile id": id.ProfileID, + "client family": string(id.ClientFamily), + "device id": id.DeviceID, + "series id": id.SeriesID, } { if value != strings.TrimSpace(value) { return fmt.Errorf("%w: %s %q has surrounding whitespace", @@ -76,6 +86,15 @@ func (id SettingIdentity) Validate() error { return fmt.Errorf("%w: scope %q must not carry a profile id", ErrInvalidSettingIdentity, id.Scope) } + wantClientFamily := id.Scope == settingscontract.ScopeProfileClient + if wantClientFamily && !id.ClientFamily.Valid() { + return fmt.Errorf("%w: scope %q requires client family tv, mobile, tablet, desktop or web", + ErrInvalidSettingIdentity, id.Scope) + } + if !wantClientFamily && id.ClientFamily != "" { + return fmt.Errorf("%w: scope %q must not carry a client family", ErrInvalidSettingIdentity, id.Scope) + } + wantDevice := id.Scope == settingscontract.ScopeProfileDevice if wantDevice && id.DeviceID == "" { return fmt.Errorf("%w: scope %q requires a device id", ErrInvalidSettingIdentity, id.Scope) @@ -117,6 +136,55 @@ type SettingValue struct { UpdatedAt string } +// SettingValueCompareAndSetter is the optional atomic-write capability used +// by settings whose public API mutates one member of a shared document. An +// expected revision of zero means the row must not exist; a positive revision +// means that exact row revision must still be current. +// +// UserStore intentionally does not embed this interface. Transaction-scoped +// and test-only stores that never serve semantic mutation endpoints need not +// expose an operation they cannot perform, while both production backends do. +type SettingValueCompareAndSetter interface { + CompareAndSetSettingValue( + ctx context.Context, + id SettingIdentity, + value json.RawMessage, + expectedRevision int64, + ) (*SettingValue, error) +} + +// SettingMutationWriter is the transaction-scoped settings surface used by an +// idempotent mutation. Implementations must keep every call made by one +// WithSettingMutationTransaction callback on the same database transaction. +// That makes the setting write and its replay receipt one durable unit. +type SettingMutationWriter interface { + GetSettingValue(ctx context.Context, id SettingIdentity) (*SettingValue, error) + UpsertSettingValue(ctx context.Context, id SettingIdentity, value json.RawMessage) (*SettingValue, error) + CompareAndSetSettingValue( + ctx context.Context, + id SettingIdentity, + value json.RawMessage, + expectedRevision int64, + ) (*SettingValue, error) + GetSettingMutation(ctx context.Context, mutationID string) (*SettingMutationRecord, error) + PutSettingMutation(ctx context.Context, record SettingMutationRecord) (SettingMutationRecord, bool, error) +} + +// SettingMutationTransactioner atomically serializes one mutation id, applies +// its setting write, and records its receipt. The callback commits only when it +// returns nil. A crash or callback error therefore leaves neither half applied. +// Concurrent callbacks for the same mutationID must execute one at a time. +// +// This is optional rather than embedded in UserStore because only production +// stores serving idempotent mutation routes need to expose transaction state. +type SettingMutationTransactioner interface { + WithSettingMutationTransaction( + ctx context.Context, + mutationID string, + fn func(SettingMutationWriter) error, + ) error +} + // SettingResolutionQuery describes one resolution request: the keys to resolve // and every identity they may resolve against. // @@ -138,6 +206,10 @@ type SettingResolutionQuery struct { // DeviceID drops profile_device candidates when empty, which is what an // unidentified client (jellycompat's DisplayPreferences seed) needs. DeviceID string + // ClientFamily drops profile_client candidates when empty. Unlike the + // device registry's platform metadata, this is a closed canonical enum and + // is supplied explicitly by first-party clients. + ClientFamily settingscontract.ClientFamily // LibraryIDs and SeriesIDs carry the content contexts of a batch. Empty // slices drop their scope from the candidate set. LibraryIDs []int @@ -150,11 +222,12 @@ type SettingResolutionQuery struct { // the same predicate for the same request. func (q SettingResolutionQuery) Normalized() SettingResolutionQuery { return SettingResolutionQuery{ - Keys: compactStrings(q.Keys), - ProfileIDs: compactStrings(q.ProfileIDs), - DeviceID: strings.TrimSpace(q.DeviceID), - LibraryIDs: compactPositiveInts(q.LibraryIDs), - SeriesIDs: compactStrings(q.SeriesIDs), + Keys: compactStrings(q.Keys), + ProfileIDs: compactStrings(q.ProfileIDs), + ClientFamily: settingscontract.ClientFamily(strings.TrimSpace(string(q.ClientFamily))), + DeviceID: strings.TrimSpace(q.DeviceID), + LibraryIDs: compactPositiveInts(q.LibraryIDs), + SeriesIDs: compactStrings(q.SeriesIDs), } } diff --git a/internal/userstore/storetest/settingvalues.go b/internal/userstore/storetest/settingvalues.go index 0d30e84a..bc632616 100644 --- a/internal/userstore/storetest/settingvalues.go +++ b/internal/userstore/storetest/settingvalues.go @@ -7,6 +7,7 @@ import ( "fmt" "reflect" "sort" + "sync" "testing" "time" @@ -28,6 +29,9 @@ func RunSettingValues(t *testing.T, newStore func(t *testing.T) userstore.UserSt t.Run("RevisionIncrements", func(t *testing.T) { testSettingValueRevisions(t, newStore) }) + t.Run("CompareAndSet", func(t *testing.T) { + testSettingValueCompareAndSet(t, newStore) + }) t.Run("PartialUniqueness", func(t *testing.T) { testSettingValuePartialUniqueness(t, newStore) }) @@ -49,11 +53,62 @@ func RunSettingValues(t *testing.T, newStore func(t *testing.T) userstore.UserSt t.Run("MutationIdempotency", func(t *testing.T) { testSettingMutationIdempotency(t, newStore) }) + t.Run("MutationTransaction", func(t *testing.T) { + testSettingMutationTransaction(t, newStore) + }) t.Run("DeviceExists", func(t *testing.T) { testDeviceExists(t, newStore) }) } +// testSettingValueCompareAndSet pins the internal primitive used by semantic +// document mutations. A stale writer must lose without changing the row, and +// an insert-if-absent race must have exactly one winner. +func testSettingValueCompareAndSet(t *testing.T, newStore func(t *testing.T) userstore.UserStore) { + ctx := context.Background() + store := newStore(t) + cas, ok := store.(userstore.SettingValueCompareAndSetter) + if !ok { + t.Skip("store does not implement SettingValueCompareAndSetter") + } + seedSettingProfiles(t, ctx, store, "p1") + id := profileID(audioKey, "p1") + + first, err := cas.CompareAndSetSettingValue(ctx, id, json.RawMessage(`"en"`), 0) + if err != nil { + t.Fatalf("insert-if-absent: %v", err) + } + if first == nil || first.Revision != 1 || !jsonEqual(first.Value, json.RawMessage(`"en"`)) { + t.Fatalf("insert-if-absent = %+v, want en at revision 1", first) + } + + if _, err := cas.CompareAndSetSettingValue(ctx, id, json.RawMessage(`"fr"`), 0); !errors.Is(err, userstore.ErrSettingValueRevisionConflict) { + t.Fatalf("second insert-if-absent error = %v, want ErrSettingValueRevisionConflict", err) + } + if _, err := cas.CompareAndSetSettingValue(ctx, id, json.RawMessage(`"de"`), first.Revision+1); !errors.Is(err, userstore.ErrSettingValueRevisionConflict) { + t.Fatalf("future revision error = %v, want ErrSettingValueRevisionConflict", err) + } + + second, err := cas.CompareAndSetSettingValue(ctx, id, json.RawMessage(`"ja"`), first.Revision) + if err != nil { + t.Fatalf("compare-and-set current revision: %v", err) + } + if second == nil || second.Revision != 2 || !jsonEqual(second.Value, json.RawMessage(`"ja"`)) { + t.Fatalf("compare-and-set = %+v, want ja at revision 2", second) + } + if _, err := cas.CompareAndSetSettingValue(ctx, id, json.RawMessage(`"it"`), first.Revision); !errors.Is(err, userstore.ErrSettingValueRevisionConflict) { + t.Fatalf("stale revision error = %v, want ErrSettingValueRevisionConflict", err) + } + + got, err := store.GetSettingValue(ctx, id) + if err != nil { + t.Fatalf("GetSettingValue: %v", err) + } + if got == nil || got.Revision != 2 || !jsonEqual(got.Value, json.RawMessage(`"ja"`)) { + t.Fatalf("row after stale CAS = %+v, want ja at revision 2", got) + } +} + // testDeviceExists pins the check that authorizes a device named in a request // rather than taken from its headers. It must be scoped to the profile, not // merely to the account: two profiles in one household register the same TV @@ -204,6 +259,12 @@ func profileID(key, profile string) userstore.SettingIdentity { return userstore.SettingIdentity{Key: key, Scope: settingscontract.ScopeProfile, ProfileID: profile} } +func clientID(key, profile string, family settingscontract.ClientFamily) userstore.SettingIdentity { + return userstore.SettingIdentity{ + Key: key, Scope: settingscontract.ScopeProfileClient, ProfileID: profile, ClientFamily: family, + } +} + func deviceID(key, profile, device string) userstore.SettingIdentity { return userstore.SettingIdentity{ Key: key, Scope: settingscontract.ScopeProfileDevice, ProfileID: profile, DeviceID: device, @@ -261,6 +322,7 @@ func testSettingValueScopes(t *testing.T, newStore func(t *testing.T) userstore. }{ {"account", accountID(audioKey), `"en"`}, {"profile", profileID(audioKey, "p1"), `"fr"`}, + {"profile_client", clientID(audioKey, "p1", settingscontract.ClientFamilyTV), `"it"`}, {"profile_device", deviceID(audioKey, "p1", deviceApple), `"de"`}, {"profile_library", libraryID(audioKey, "p1", 42), `"es"`}, {"profile_series", seriesID(audioKey, "p1", "series-1"), `"ja"`}, @@ -307,6 +369,7 @@ func testSettingValueScopes(t *testing.T, newStore func(t *testing.T) userstore. seedSettingProfiles(t, ctx, store, "p2") for _, id := range []userstore.SettingIdentity{ profileID(audioKey, "p2"), + clientID(audioKey, "p1", settingscontract.ClientFamilyMobile), deviceID(audioKey, "p1", "iphone"), libraryID(audioKey, "p1", 43), seriesID(audioKey, "p1", "series-2"), @@ -338,12 +401,13 @@ func testSettingValueListAll(t *testing.T, newStore func(t *testing.T) userstore seedSettingProfiles(t, ctx, store, "p1", "p2") seeded := map[userstore.SettingIdentity]string{ - accountID(audioKey): `"en"`, - profileID(audioKey, "p1"): `"fr"`, - profileID(subtitleKey, "p2"): `"always"`, - deviceID(audioKey, "p1", deviceApple): `"de"`, - libraryID(audioKey, "p1", 42): `"es"`, - seriesID(audioKey, "p1", seriesOne): `"ja"`, + accountID(audioKey): `"en"`, + profileID(audioKey, "p1"): `"fr"`, + profileID(subtitleKey, "p2"): `"always"`, + clientID(audioKey, "p1", settingscontract.ClientFamilyTV): `"it"`, + deviceID(audioKey, "p1", deviceApple): `"de"`, + libraryID(audioKey, "p1", 42): `"es"`, + seriesID(audioKey, "p1", seriesOne): `"ja"`, } for id, value := range seeded { mustUpsert(t, ctx, store, id, value) @@ -476,7 +540,7 @@ func testSettingValueRevisions(t *testing.T, newStore func(t *testing.T) usersto } } -// testSettingValuePartialUniqueness pins the five partial unique indexes: one +// testSettingValuePartialUniqueness pins the six partial unique indexes: one // explicit value per identity, and identities that differ in any one context // column coexist. func testSettingValuePartialUniqueness(t *testing.T, newStore func(t *testing.T) userstore.UserStore) { @@ -488,6 +552,9 @@ func testSettingValuePartialUniqueness(t *testing.T, newStore func(t *testing.T) accountID(audioKey), profileID(audioKey, "p1"), profileID(audioKey, "p2"), + clientID(audioKey, "p1", settingscontract.ClientFamilyTV), + clientID(audioKey, "p1", settingscontract.ClientFamilyMobile), + clientID(audioKey, "p2", settingscontract.ClientFamilyTV), deviceID(audioKey, "p1", deviceApple), deviceID(audioKey, "p1", "iphone"), deviceID(audioKey, "p2", deviceApple), @@ -505,19 +572,22 @@ func testSettingValuePartialUniqueness(t *testing.T, newStore func(t *testing.T) } rows, err := store.ListSettingValuesForResolution(ctx, userstore.SettingResolutionQuery{ - Keys: []string{audioKey}, - ProfileIDs: []string{"p1"}, - DeviceID: deviceApple, - LibraryIDs: []int{1, 2}, - SeriesIDs: []string{seriesOne, seriesTwo}, + Keys: []string{audioKey}, + ProfileIDs: []string{"p1"}, + ClientFamily: settingscontract.ClientFamilyTV, + DeviceID: deviceApple, + LibraryIDs: []int{1, 2}, + SeriesIDs: []string{seriesOne, seriesTwo}, }) if err != nil { t.Fatalf("ListSettingValuesForResolution: %v", err) } - // p1's candidates: account, profile, one device, two libraries, two series. + // p1's candidates: account, profile, one client family, one device, two + // libraries, and two series. want := []userstore.SettingIdentity{ accountID(audioKey), profileID(audioKey, "p1"), + clientID(audioKey, "p1", settingscontract.ClientFamilyTV), deviceID(audioKey, "p1", deviceApple), libraryID(audioKey, "p1", 1), libraryID(audioKey, "p1", 2), @@ -555,6 +625,15 @@ func testSettingValueIdentityValidation(t *testing.T, newStore func(t *testing.T {"account scope with profile", userstore.SettingIdentity{ Key: audioKey, Scope: settingscontract.ScopeAccount, ProfileID: "p1", }}, + {"client scope without family", userstore.SettingIdentity{ + Key: audioKey, Scope: settingscontract.ScopeProfileClient, ProfileID: "p1", + }}, + {"client scope with unknown family", userstore.SettingIdentity{ + Key: audioKey, Scope: settingscontract.ScopeProfileClient, ProfileID: "p1", ClientFamily: "car", + }}, + {"profile scope with family", userstore.SettingIdentity{ + Key: audioKey, Scope: settingscontract.ScopeProfile, ProfileID: "p1", ClientFamily: settingscontract.ClientFamilyTV, + }}, {"device scope without device", userstore.SettingIdentity{ Key: audioKey, Scope: settingscontract.ScopeProfileDevice, ProfileID: "p1", }}, @@ -582,6 +661,9 @@ func testSettingValueIdentityValidation(t *testing.T, newStore func(t *testing.T {"padded device id", userstore.SettingIdentity{ Key: audioKey, Scope: settingscontract.ScopeProfileDevice, ProfileID: "p1", DeviceID: deviceApple + " ", }}, + {"padded client family", userstore.SettingIdentity{ + Key: audioKey, Scope: settingscontract.ScopeProfileClient, ProfileID: "p1", ClientFamily: " tv ", + }}, {"padded series id", userstore.SettingIdentity{ Key: audioKey, Scope: settingscontract.ScopeProfileSeries, ProfileID: "p1", SeriesID: " " + seriesOne, }}, @@ -624,6 +706,7 @@ func testSettingValueResolution(t *testing.T, newStore func(t *testing.T) userst mustUpsert(t, ctx, store, accountID(audioKey), `"en"`) mustUpsert(t, ctx, store, profileID(audioKey, "p1"), `"fr"`) + mustUpsert(t, ctx, store, clientID(audioKey, "p1", settingscontract.ClientFamilyTV), `"it"`) mustUpsert(t, ctx, store, deviceID(audioKey, "p1", deviceApple), `"de"`) mustUpsert(t, ctx, store, libraryID(audioKey, "p1", 42), `"es"`) mustUpsert(t, ctx, store, seriesID(audioKey, "p1", seriesOne), `"ja"`) @@ -633,6 +716,8 @@ func testSettingValueResolution(t *testing.T, newStore func(t *testing.T) userst // Decoys: another profile, another device, another library, another series, // and a key nobody asked for. mustUpsert(t, ctx, store, profileID(audioKey, "p2"), `"pt"`) + mustUpsert(t, ctx, store, clientID(audioKey, "p1", settingscontract.ClientFamilyMobile), `"pl"`) + mustUpsert(t, ctx, store, clientID(audioKey, "p2", settingscontract.ClientFamilyTV), `"cs"`) mustUpsert(t, ctx, store, deviceID(audioKey, "p1", "iphone"), `"nl"`) mustUpsert(t, ctx, store, libraryID(audioKey, "p1", 43), `"sv"`) mustUpsert(t, ctx, store, seriesID(audioKey, "p1", "s-3"), `"da"`) @@ -640,11 +725,12 @@ func testSettingValueResolution(t *testing.T, newStore func(t *testing.T) userst // The batched shape: two content contexts resolved in one call. rows, err := store.ListSettingValuesForResolution(ctx, userstore.SettingResolutionQuery{ - Keys: []string{audioKey, subtitleKey}, - ProfileIDs: []string{"p1"}, - DeviceID: deviceApple, - LibraryIDs: []int{42}, - SeriesIDs: []string{seriesOne, seriesTwo}, + Keys: []string{audioKey, subtitleKey}, + ProfileIDs: []string{"p1"}, + ClientFamily: settingscontract.ClientFamilyTV, + DeviceID: deviceApple, + LibraryIDs: []int{42}, + SeriesIDs: []string{seriesOne, seriesTwo}, }) if err != nil { t.Fatalf("ListSettingValuesForResolution: %v", err) @@ -652,6 +738,7 @@ func testSettingValueResolution(t *testing.T, newStore func(t *testing.T) userst assertIdentitySet(t, rows, []userstore.SettingIdentity{ accountID(audioKey), profileID(audioKey, "p1"), + clientID(audioKey, "p1", settingscontract.ClientFamilyTV), deviceID(audioKey, "p1", deviceApple), libraryID(audioKey, "p1", 42), seriesID(audioKey, "p1", seriesOne), @@ -663,10 +750,11 @@ func testSettingValueResolution(t *testing.T, newStore func(t *testing.T) userst // lets a list view make one round trip instead of one per item. for _, series := range []string{seriesOne, seriesTwo} { single, err := store.ListSettingValuesForResolution(ctx, userstore.SettingResolutionQuery{ - Keys: []string{audioKey}, - ProfileIDs: []string{"p1"}, - DeviceID: deviceApple, - SeriesIDs: []string{series}, + Keys: []string{audioKey}, + ProfileIDs: []string{"p1"}, + ClientFamily: settingscontract.ClientFamilyTV, + DeviceID: deviceApple, + SeriesIDs: []string{series}, }) if err != nil { t.Fatalf("ListSettingValuesForResolution(%s): %v", series, err) @@ -674,13 +762,30 @@ func testSettingValueResolution(t *testing.T, newStore func(t *testing.T) userst assertIdentitySet(t, single, []userstore.SettingIdentity{ accountID(audioKey), profileID(audioKey, "p1"), + clientID(audioKey, "p1", settingscontract.ClientFamilyTV), deviceID(audioKey, "p1", deviceApple), seriesID(audioKey, "p1", series), }) } - // No device identity — an incognito window, or jellycompat's seed — drops - // profile_device candidates without touching the roaming ones. + // A family without a device still sees the like-client value while dropping + // exact-device candidates. + familyOnly, err := store.ListSettingValuesForResolution(ctx, userstore.SettingResolutionQuery{ + Keys: []string{audioKey}, + ProfileIDs: []string{"p1"}, + ClientFamily: settingscontract.ClientFamilyTV, + }) + if err != nil { + t.Fatalf("ListSettingValuesForResolution(family only): %v", err) + } + assertIdentitySet(t, familyOnly, []userstore.SettingIdentity{ + accountID(audioKey), + profileID(audioKey, "p1"), + clientID(audioKey, "p1", settingscontract.ClientFamilyTV), + }) + + // No device or family identity — an incognito window, or jellycompat's seed + // — drops both contextual candidates without touching profile roaming. noDevice, err := store.ListSettingValuesForResolution(ctx, userstore.SettingResolutionQuery{ Keys: []string{audioKey}, ProfileIDs: []string{"p1"}, @@ -722,11 +827,12 @@ func testSettingValueResolution(t *testing.T, newStore func(t *testing.T) userst // Blank and duplicate context ids are compacted rather than bound as // literals, so they neither match a '' row nor multiply the result set. dirty, err := store.ListSettingValuesForResolution(ctx, userstore.SettingResolutionQuery{ - Keys: []string{audioKey, "", audioKey, " "}, - ProfileIDs: []string{"p1", "p1", "", " "}, - DeviceID: deviceApple, - LibraryIDs: []int{42, 42, 0, -1}, - SeriesIDs: []string{seriesOne, seriesOne, "", " "}, + Keys: []string{audioKey, "", audioKey, " "}, + ProfileIDs: []string{"p1", "p1", "", " "}, + ClientFamily: settingscontract.ClientFamilyTV, + DeviceID: deviceApple, + LibraryIDs: []int{42, 42, 0, -1}, + SeriesIDs: []string{seriesOne, seriesOne, "", " "}, }) if err != nil { t.Fatalf("ListSettingValuesForResolution(dirty): %v", err) @@ -734,6 +840,7 @@ func testSettingValueResolution(t *testing.T, newStore func(t *testing.T) userst assertIdentitySet(t, dirty, []userstore.SettingIdentity{ accountID(audioKey), profileID(audioKey, "p1"), + clientID(audioKey, "p1", settingscontract.ClientFamilyTV), deviceID(audioKey, "p1", deviceApple), libraryID(audioKey, "p1", 42), seriesID(audioKey, "p1", seriesOne), @@ -780,6 +887,9 @@ func testSettingValueDeletePaths(t *testing.T, newStore func(t *testing.T) users accountID(audioKey), profileID(audioKey, "p1"), profileID(audioKey, "p2"), + clientID(audioKey, "p1", settingscontract.ClientFamilyTV), + clientID(audioKey, "p1", settingscontract.ClientFamilyMobile), + clientID(audioKey, "p2", settingscontract.ClientFamilyTV), deviceID(audioKey, "p1", deviceApple), deviceID(audioKey, "p1", "iphone"), deviceID(audioKey, "p2", deviceApple), @@ -887,11 +997,13 @@ func testSettingValueDeletePaths(t *testing.T, newStore func(t *testing.T) users if err != nil { t.Fatalf("DeleteSettingValuesForProfile: %v", err) } - if removed != 7 { - t.Fatalf("DeleteSettingValuesForProfile removed %d rows, want 7", removed) + if removed != 9 { + t.Fatalf("DeleteSettingValuesForProfile removed %d rows, want 9", removed) } assertRemaining(t, store, all, []userstore.SettingIdentity{ profileID(audioKey, "p1"), + clientID(audioKey, "p1", settingscontract.ClientFamilyTV), + clientID(audioKey, "p1", settingscontract.ClientFamilyMobile), deviceID(audioKey, "p1", deviceApple), deviceID(audioKey, "p1", "iphone"), libraryID(audioKey, "p1", 1), @@ -909,6 +1021,8 @@ func testSettingValueDeletePaths(t *testing.T, newStore func(t *testing.T) users // Account scope belongs to the account, not to any one household member. assertRemaining(t, store, all, []userstore.SettingIdentity{ profileID(audioKey, "p1"), + clientID(audioKey, "p1", settingscontract.ClientFamilyTV), + clientID(audioKey, "p1", settingscontract.ClientFamilyMobile), deviceID(audioKey, "p1", deviceApple), deviceID(audioKey, "p1", "iphone"), libraryID(audioKey, "p1", 1), @@ -1029,6 +1143,152 @@ func testSettingMutationIdempotency(t *testing.T, newStore func(t *testing.T) us } } +// testSettingMutationTransaction proves the storage primitive that handlers +// rely on: setting + receipt roll back together, and same-id contenders check +// the winning receipt before either can apply a second setting write. +func testSettingMutationTransaction(t *testing.T, newStore func(t *testing.T) userstore.UserStore) { + ctx := context.Background() + + t.Run("RollbackIsAtomic", func(t *testing.T) { + store := newStore(t) + transactioner, ok := store.(userstore.SettingMutationTransactioner) + if !ok { + t.Skip("store does not implement SettingMutationTransactioner") + } + seedSettingProfiles(t, ctx, store, "p1") + id := profileID(audioKey, "p1") + rollbackErr := errors.New("simulate process failure before commit") + err := transactioner.WithSettingMutationTransaction(ctx, "tx-rollback", + func(writer userstore.SettingMutationWriter) error { + if _, err := writer.UpsertSettingValue(ctx, id, json.RawMessage(`"en"`)); err != nil { + return err + } + if _, _, err := writer.PutSettingMutation(ctx, userstore.SettingMutationRecord{ + MutationID: "tx-rollback", RequestHash: mutationHash, + Result: json.RawMessage(`{"winner":"en"}`), ExpiresAt: time.Now().UTC().Add(time.Hour), + }); err != nil { + return err + } + return rollbackErr + }) + if !errors.Is(err, rollbackErr) { + t.Fatalf("transaction error = %v, want rollback sentinel", err) + } + if got, err := store.GetSettingValue(ctx, id); err != nil || got != nil { + t.Fatalf("setting after rollback = %+v (%v), want nil", got, err) + } + if got, err := store.GetSettingMutation(ctx, "tx-rollback"); err != nil || got != nil { + t.Fatalf("receipt after rollback = %+v (%v), want nil", got, err) + } + }) + + for _, tc := range []struct { + name string + secondHash string + second json.RawMessage + wantReplay int + wantConflict int + }{ + {name: "SameHashReplays", secondHash: "hash-a", second: json.RawMessage(`"en"`), wantReplay: 1}, + {name: "DifferentHashConflicts", secondHash: "hash-b", second: json.RawMessage(`"fr"`), wantConflict: 1}, + } { + t.Run(tc.name, func(t *testing.T) { + store := newStore(t) + transactioner, ok := store.(userstore.SettingMutationTransactioner) + if !ok { + t.Skip("store does not implement SettingMutationTransactioner") + } + seedSettingProfiles(t, ctx, store, "p1") + id := profileID(audioKey, "p1") + type attempt struct { + hash string + value json.RawMessage + } + attempts := []attempt{{hash: "hash-a", value: json.RawMessage(`"en"`)}, {hash: tc.secondHash, value: tc.second}} + type result struct { + status string + receipt json.RawMessage + err error + } + results := make(chan result, len(attempts)) + start := make(chan struct{}) + var ready sync.WaitGroup + ready.Add(len(attempts)) + for _, candidate := range attempts { + candidate := candidate + go func() { + ready.Done() + <-start + out := result{} + out.err = transactioner.WithSettingMutationTransaction(ctx, "tx-concurrent", + func(writer userstore.SettingMutationWriter) error { + prior, err := writer.GetSettingMutation(ctx, "tx-concurrent") + if err != nil { + return err + } + if prior != nil { + out.receipt = prior.Result + if prior.RequestHash == candidate.hash { + out.status = "replay" + } else { + out.status = "conflict" + } + return nil + } + stored, err := writer.UpsertSettingValue(ctx, id, candidate.value) + if err != nil { + return err + } + out.receipt, err = json.Marshal(map[string]any{"revision": stored.Revision, "value": candidate.value}) + if err != nil { + return err + } + _, inserted, err := writer.PutSettingMutation(ctx, userstore.SettingMutationRecord{ + MutationID: "tx-concurrent", RequestHash: candidate.hash, + Result: out.receipt, ExpiresAt: time.Now().UTC().Add(time.Hour), + }) + if err == nil && !inserted { + return errors.New("serialized transaction unexpectedly lost receipt insert") + } + out.status = "applied" + return err + }) + results <- out + }() + } + ready.Wait() + close(start) + + counts := map[string]int{} + var receipts []json.RawMessage + for range attempts { + out := <-results + if out.err != nil { + t.Fatalf("concurrent transaction: %v", out.err) + } + counts[out.status]++ + receipts = append(receipts, out.receipt) + } + if counts["applied"] != 1 || counts["replay"] != tc.wantReplay || counts["conflict"] != tc.wantConflict { + t.Fatalf("outcomes = %+v, want one applied, %d replay, %d conflict", counts, tc.wantReplay, tc.wantConflict) + } + stored, err := store.GetSettingValue(ctx, id) + if err != nil || stored == nil || stored.Revision != 1 { + t.Fatalf("stored setting = %+v (%v), want exactly one revision", stored, err) + } + receipt, err := store.GetSettingMutation(ctx, "tx-concurrent") + if err != nil || receipt == nil { + t.Fatalf("stored receipt = %+v (%v)", receipt, err) + } + for _, got := range receipts { + if !jsonEqual(got, receipt.Result) { + t.Fatalf("attempt receipt = %s, want coherent winner %s", got, receipt.Result) + } + } + }) + } +} + // assertIdentitySet compares the returned rows to the expected identities as a // set. Row order is deliberately not asserted: the two backends sort text under // different collations, and ranking is the resolver's job anyway. @@ -1050,8 +1310,8 @@ func assertIdentitySet(t *testing.T, rows []userstore.SettingValue, want []users } func identityToken(id userstore.SettingIdentity) string { - return fmt.Sprintf("%s|%s|%s|%s|%d|%s", - id.Key, id.Scope, id.ProfileID, id.DeviceID, id.LibraryID, id.SeriesID) + return fmt.Sprintf("%s|%s|%s|%s|%s|%d|%s", + id.Key, id.Scope, id.ProfileID, id.ClientFamily, id.DeviceID, id.LibraryID, id.SeriesID) } // jsonEqual compares two JSON documents by value. PostgreSQL stores jsonb, which diff --git a/migrations/sql/20260803191207_add_profile_client_settings_scope.sql b/migrations/sql/20260803191207_add_profile_client_settings_scope.sql new file mode 100644 index 00000000..04766bea --- /dev/null +++ b/migrations/sql/20260803191207_add_profile_client_settings_scope.sql @@ -0,0 +1,200 @@ +-- Add a profile-and-client-family identity to the canonical settings store. +-- Unlike device_platform, client_family is a closed request-supplied enum and +-- therefore safe to use as part of resolution and uniqueness. + +-- +goose Up +ALTER TABLE public.user_setting_values + ADD COLUMN client_family text; + +ALTER TABLE public.user_setting_values + DROP CONSTRAINT user_setting_values_scope_check, + DROP CONSTRAINT user_setting_values_identity_check, + ADD CONSTRAINT user_setting_values_client_family_check + CHECK (client_family IS NULL OR client_family IN ('tv', 'mobile', 'tablet', 'desktop', 'web')), + ADD CONSTRAINT user_setting_values_scope_check + CHECK (scope IN ('account', 'profile', 'profile_client', 'profile_device', 'profile_library', 'profile_series')), + ADD CONSTRAINT user_setting_values_identity_check CHECK ( + (scope = 'account' AND profile_id IS NULL AND client_family IS NULL AND device_id IS NULL AND library_id IS NULL AND series_id IS NULL) OR + (scope = 'profile' AND profile_id IS NOT NULL AND client_family IS NULL AND device_id IS NULL AND library_id IS NULL AND series_id IS NULL) OR + (scope = 'profile_client' AND profile_id IS NOT NULL AND client_family IS NOT NULL AND device_id IS NULL AND library_id IS NULL AND series_id IS NULL) OR + (scope = 'profile_device' AND profile_id IS NOT NULL AND client_family IS NULL AND device_id IS NOT NULL AND library_id IS NULL AND series_id IS NULL) OR + (scope = 'profile_library' AND profile_id IS NOT NULL AND client_family IS NULL AND device_id IS NULL AND library_id IS NOT NULL AND series_id IS NULL) OR + (scope = 'profile_series' AND profile_id IS NOT NULL AND client_family IS NULL AND device_id IS NULL AND library_id IS NULL AND series_id IS NOT NULL) + ); + +CREATE UNIQUE INDEX user_setting_values_profile_client_uq + ON public.user_setting_values (user_id, profile_id, client_family, key) + WHERE scope = 'profile_client'; + +-- Decode either the canonical object or the one-layer JSON-string encoding +-- written by legacy web clients. Invalid string contents return NULL instead +-- of aborting the whole migration. +-- +goose StatementBegin +CREATE FUNCTION pg_temp.decode_legacy_sidebar_pins(candidate jsonb) +RETURNS jsonb +LANGUAGE plpgsql +IMMUTABLE +STRICT +AS $$ +DECLARE + decoded jsonb; +BEGIN + IF jsonb_typeof(candidate) = 'object' THEN + RETURN candidate; + END IF; + IF jsonb_typeof(candidate) = 'string' THEN + BEGIN + decoded := (candidate #>> '{}')::jsonb; + EXCEPTION WHEN invalid_text_representation THEN + RETURN NULL; + END; + + IF jsonb_typeof(decoded) = 'object' THEN + RETURN decoded; + END IF; + END IF; + RETURN NULL; +END; +$$; +-- +goose StatementEnd + +-- Seed the new profile-wide shortcut catalog from convertible legacy web +-- sidebar pins. ui.sidebar_pins remains untouched, rows with non-numeric +-- well-known groups remain there, and an already-authored nav.shortcuts row is +-- never overwritten. The new schema bounds the catalog at 256 entries. +WITH legacy AS ( + SELECT value_row.id AS legacy_id, + value_row.user_id, + value_row.profile_id, + value_row.created_at, + value_row.updated_at, + decoded.value + FROM public.user_setting_values AS value_row + CROSS JOIN LATERAL ( + SELECT pg_temp.decode_legacy_sidebar_pins(value_row.value) AS value + ) AS decoded + WHERE value_row.key = 'ui.sidebar_pins' + AND value_row.scope = 'profile' + AND decoded.value IS NOT NULL + AND NOT EXISTS ( + SELECT 1 + FROM public.user_setting_values AS current + WHERE current.user_id = value_row.user_id + AND current.profile_id = value_row.profile_id + AND current.key = 'nav.shortcuts' + AND current.scope = 'profile' + ) +), library_groups AS ( + SELECT legacy.legacy_id, + legacy.user_id, + legacy.profile_id, + legacy.created_at, + legacy.updated_at, + groups.pins, + CASE + WHEN groups.group_key ~ '^[1-9][0-9]{0,9}$' THEN + CASE + WHEN groups.group_key::bigint <= 2147483647 THEN groups.group_key::integer + END + END AS library_id + FROM legacy + CROSS JOIN LATERAL jsonb_each(legacy.value) AS groups(group_key, pins) +), valid_pins AS ( + SELECT group_row.legacy_id, + group_row.user_id, + group_row.profile_id, + group_row.created_at, + group_row.updated_at, + group_row.library_id, + pin.pin_value->>'type' AS pin_type, + pin.pin_value->>'id' AS pin_id, + pin.pin_value->>'label' AS pin_label, + pin.ordinality + FROM library_groups AS group_row + CROSS JOIN LATERAL jsonb_array_elements( + CASE WHEN jsonb_typeof(group_row.pins) = 'array' THEN group_row.pins ELSE '[]'::jsonb END + ) WITH ORDINALITY AS pin(pin_value, ordinality) + WHERE group_row.library_id IS NOT NULL + AND jsonb_typeof(pin.pin_value) = 'object' + AND pin.pin_value->>'type' IN ('section', 'collection') + AND jsonb_typeof(pin.pin_value->'id') = 'string' + AND jsonb_typeof(pin.pin_value->'label') = 'string' + AND char_length(COALESCE(pin.pin_value->>'id', '')) BETWEEN 1 AND 128 + AND pin.pin_value->>'id' ~ '[^[:space:]]' + AND char_length(COALESCE(pin.pin_value->>'label', '')) BETWEEN 1 AND 256 + AND pin.pin_value->>'label' ~ '[^[:space:]]' +), deduplicated AS ( + SELECT valid_pins.*, + row_number() OVER ( + PARTITION BY legacy_id, library_id, pin_type, pin_id + ORDER BY ordinality + ) AS duplicate_rank + FROM valid_pins +), ranked AS ( + SELECT deduplicated.*, + row_number() OVER ( + PARTITION BY legacy_id + ORDER BY library_id, ordinality, pin_type, pin_id + ) AS item_rank + FROM deduplicated + WHERE duplicate_rank = 1 +), shortcut_values AS ( + SELECT legacy_id, + user_id, + profile_id, + created_at, + updated_at, + jsonb_build_object( + 'items', + jsonb_agg( + CASE pin_type + WHEN 'section' THEN jsonb_build_object( + 'type', 'section', + 'library_id', library_id, + 'section_id', pin_id, + 'label', pin_label + ) + ELSE jsonb_build_object( + 'type', 'collection', + 'library_id', library_id, + 'collection_id', pin_id, + 'label', pin_label + ) + END + ORDER BY item_rank + ) + ) AS value + FROM ranked + WHERE item_rank <= 256 + GROUP BY legacy_id, user_id, profile_id, created_at, updated_at +) +INSERT INTO public.user_setting_values + (user_id, key, scope, profile_id, client_family, device_id, library_id, series_id, + value, revision, created_at, updated_at) +SELECT user_id, 'nav.shortcuts', 'profile', profile_id, NULL, NULL, NULL, NULL, + value, 1, created_at, updated_at + FROM shortcut_values +ON CONFLICT (user_id, profile_id, key) WHERE scope = 'profile' DO NOTHING; + +DROP FUNCTION pg_temp.decode_legacy_sidebar_pins(jsonb); + +-- +goose Down +DELETE FROM public.user_setting_values WHERE scope = 'profile_client'; + +DROP INDEX IF EXISTS public.user_setting_values_profile_client_uq; + +ALTER TABLE public.user_setting_values + DROP CONSTRAINT user_setting_values_client_family_check, + DROP CONSTRAINT user_setting_values_scope_check, + DROP CONSTRAINT user_setting_values_identity_check, + ADD CONSTRAINT user_setting_values_scope_check + CHECK (scope IN ('account', 'profile', 'profile_device', 'profile_library', 'profile_series')), + ADD CONSTRAINT user_setting_values_identity_check CHECK ( + (scope = 'account' AND profile_id IS NULL AND device_id IS NULL AND library_id IS NULL AND series_id IS NULL) OR + (scope = 'profile' AND profile_id IS NOT NULL AND device_id IS NULL AND library_id IS NULL AND series_id IS NULL) OR + (scope = 'profile_device' AND profile_id IS NOT NULL AND device_id IS NOT NULL AND library_id IS NULL AND series_id IS NULL) OR + (scope = 'profile_library' AND profile_id IS NOT NULL AND device_id IS NULL AND library_id IS NOT NULL AND series_id IS NULL) OR + (scope = 'profile_series' AND profile_id IS NOT NULL AND device_id IS NULL AND library_id IS NULL AND series_id IS NOT NULL) + ); + +ALTER TABLE public.user_setting_values DROP COLUMN client_family; diff --git a/migrations/user_setting_values_test.go b/migrations/user_setting_values_test.go index ea1c4cb8..a4477a12 100644 --- a/migrations/user_setting_values_test.go +++ b/migrations/user_setting_values_test.go @@ -6,9 +6,10 @@ import ( ) // TestUserSettingValuesMigrationContract pins the parts of the canonical -// settings storage that the store code and the design both depend on: the scope -// CHECK constraints, the five partial unique indexes that enforce one explicit -// value per identity, and the covering indexes the one-query read path needs. +// settings storage that the store code and the design both depend on: the five +// original scope CHECK/index identities, and the covering indexes the one-query +// read path needs. TestProfileClientSettingsMigrationContract pins the sixth +// identity added by the later extension migration. // A silent edit to any of them would not fail a store test until a duplicate row // or a sequential-scan regression reached production. func TestUserSettingValuesMigrationContract(t *testing.T) { @@ -68,6 +69,49 @@ func TestUserSettingValuesMigrationContract(t *testing.T) { } } +func TestProfileClientSettingsMigrationContract(t *testing.T) { + migration := readMigration(t, "sql/20260803191207_add_profile_client_settings_scope.sql") + + for _, want := range []string{ + "ADD COLUMN client_family text", + "CHECK (client_family IS NULL OR client_family IN ('tv', 'mobile', 'tablet', 'desktop', 'web'))", + "CHECK (scope IN ('account', 'profile', 'profile_client', 'profile_device', 'profile_library', 'profile_series'))", + "(scope = 'profile_client' AND profile_id IS NOT NULL AND client_family IS NOT NULL AND device_id IS NULL AND library_id IS NULL AND series_id IS NULL)", + "CREATE UNIQUE INDEX user_setting_values_profile_client_uq\n ON public.user_setting_values (user_id, profile_id, client_family, key)", + + // Legacy pins are copied only when the new profile row is absent. Blank + // labels never become values that the new contract would reject, and the + // old row is not deleted. + "current.key = 'nav.shortcuts'", + "CREATE FUNCTION pg_temp.decode_legacy_sidebar_pins(candidate jsonb)", + "jsonb_typeof(candidate) = 'string'", + "decoded := (candidate #>> '{}')::jsonb", + "EXCEPTION WHEN invalid_text_representation", + "pg_temp.decode_legacy_sidebar_pins(value_row.value)", + "WHEN groups.group_key ~ '^[1-9][0-9]{0,9}$' THEN", + "WHEN groups.group_key::bigint <= 2147483647 THEN groups.group_key::integer", + "group_row.library_id IS NOT NULL", + "CASE WHEN jsonb_typeof(group_row.pins) = 'array' THEN group_row.pins ELSE '[]'::jsonb END", + "jsonb_typeof(pin.pin_value) = 'object'", + "jsonb_typeof(pin.pin_value->'id') = 'string'", + "jsonb_typeof(pin.pin_value->'label') = 'string'", + "pin.pin_value->>'id' ~ '[^[:space:]]'", + "pin.pin_value->>'label' ~ '[^[:space:]]'", + "ON CONFLICT (user_id, profile_id, key) WHERE scope = 'profile' DO NOTHING", + } { + if !strings.Contains(migration, want) { + t.Fatalf("profile_client migration missing %q", want) + } + } + + if strings.Contains(migration, "DELETE FROM public.user_setting_values WHERE key = 'ui.sidebar_pins'") { + t.Fatal("profile_client migration deletes the legacy sidebar pins it must preserve") + } + if strings.Contains(migration, "999999999") || strings.Contains(migration, "{0,8}") { + t.Fatal("profile_client migration retains the obsolete nine-digit library id cap") + } +} + func readMigration(t *testing.T, path string) string { t.Helper() contents, err := FS.ReadFile(path) diff --git a/web/src/App.tsx b/web/src/App.tsx index b5a5cd0d..25ed9dc4 100644 --- a/web/src/App.tsx +++ b/web/src/App.tsx @@ -1,4 +1,4 @@ -import { useEffect, useRef, useState } from "react"; +import { useEffect, useRef, useState, type ReactNode } from "react"; import { BrowserRouter, Routes, @@ -18,6 +18,7 @@ import { ThemeProvider } from "@/hooks/useTheme"; import { DateTimeFormatProvider, useDateTimeFormat } from "@/hooks/useDateTimeFormat"; import { CustomThemeProvider } from "@/contexts/CustomThemeProvider"; import { BrandingProvider } from "@/contexts/BrandingProvider"; +import { UICustomizationProvider } from "@/contexts/UICustomizationProvider"; import { ErrorBoundary } from "@/components/ErrorBoundary"; import ImpersonationBanner from "@/components/ImpersonationBanner"; import { loadStoredImpersonationAdminSession } from "@/lib/impersonationSession"; @@ -100,6 +101,7 @@ import ThemeEditorSettings from "@/pages/settings/ThemeEditorSettings"; import CardOverlaySettings from "@/pages/settings/CardOverlaySettings"; import PersonalizeSettings from "@/pages/settings/PersonalizeSettings"; import ConnectAppsSettings from "@/pages/settings/ConnectAppsSettings"; +import InterfaceSettings from "@/pages/settings/InterfaceSettings"; import WatchTogetherJoin from "@/pages/WatchTogetherJoin"; import WatchTogetherRoomPage from "@/pages/WatchTogetherRoomPage"; import WatchRoute from "@/pages/WatchRoute"; @@ -110,7 +112,6 @@ import { WatchPlaybackProvider, } from "@/playback/WatchPlaybackChrome"; import { AudiobookPlaybackProvider } from "@/pages/audiobooks/player/audiobookPlaybackContext"; -import type { ReactNode } from "react"; import { buildLegacyBrowseCatalogHref, buildPersonalCatalogHref, @@ -375,6 +376,14 @@ function ReactiveAppRoutes() { return ; } +function UICustomizedLayout({ children }: { children: ReactNode }) { + return ( + + {children} + + ); +} + function AppRoutes() { return ( @@ -462,14 +471,15 @@ function AppRoutes() { path="/settings/*" element={ - + - + } > } /> + } /> } /> } /> } /> @@ -499,7 +509,7 @@ function AppRoutes() { path="/*" element={ - + } /> - + } /> diff --git a/web/src/api/client.test.ts b/web/src/api/client.test.ts index 0ffa92f7..aa65f61b 100644 --- a/web/src/api/client.test.ts +++ b/web/src/api/client.test.ts @@ -2,8 +2,11 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import { api, apiBlob, + apiWithProfileRequestContext, bootstrapAccessToken, + captureProfileRequestContext, getAccessToken, + getProfileToken, getPersonCatalogItems, onProfileUnverified, setAccessToken, @@ -42,13 +45,13 @@ describe("bootstrapAccessToken", () => { }); it("refreshes the access token before protected requests on startup", async () => { - setRefreshToken("startup-refresh"); + setRefreshToken("fake"); const fetchMock = vi.fn(async (input) => { expect(String(input)).toBe("/api/v1/auth/refresh"); return new Response( JSON.stringify({ - access_token: "startup-access", - refresh_token: "rotated-refresh", + access_token: "dummy", + refresh_token: "example", expires_in: 3600, }), { @@ -61,19 +64,19 @@ describe("bootstrapAccessToken", () => { await expect(bootstrapAccessToken(fetchMock)).resolves.toBe(true); expect(fetchMock).toHaveBeenCalledTimes(1); - expect(getAccessToken()).toBe("startup-access"); - expect(localStorage.getItem("refresh_token")).toBe("rotated-refresh"); + expect(getAccessToken()).toBe("dummy"); + expect(localStorage.getItem("refresh_token")).toBe("example"); }); it("does not refresh when an access token is already present", async () => { - setAccessToken("already-present"); - setRefreshToken("startup-refresh"); + setAccessToken("sample"); + setRefreshToken("fake"); const fetchMock = vi.fn(); await expect(bootstrapAccessToken(fetchMock)).resolves.toBe(true); expect(fetchMock).not.toHaveBeenCalled(); - expect(getAccessToken()).toBe("already-present"); + expect(getAccessToken()).toBe("sample"); }); }); @@ -265,6 +268,340 @@ describe("api", () => { onProfileUnverified(null); }); + it("preserves an explicitly captured profile identity", async () => { + Object.defineProperty(globalThis, "sessionStorage", { + value: { + getItem: () => null, + setItem: () => {}, + removeItem: () => {}, + clear: () => {}, + }, + configurable: true, + }); + setProfileId("profile-new"); + const fetchMock = vi.fn( + async () => + new Response(JSON.stringify({ ok: true }), { + status: 200, + headers: { "Content-Type": "application/json" }, + }), + ); + vi.stubGlobal("fetch", fetchMock); + + await api("/test", { headers: { "X-Profile-Id": "profile-old" } }); + + const headers = fetchMock.mock.calls[0]![1]!.headers as Record; + expect(headers["X-Profile-Id"]).toBe("profile-old"); + }); + + it("preserves the PIN token captured with an explicit profile identity", async () => { + Object.defineProperty(globalThis, "sessionStorage", { + value: { + getItem: () => null, + setItem: () => {}, + removeItem: () => {}, + clear: () => {}, + }, + configurable: true, + }); + setProfileId("profile-new"); + setProfileToken("dummy"); + const fetchMock = vi.fn( + async () => + new Response(JSON.stringify({ ok: true }), { + status: 200, + headers: { "Content-Type": "application/json" }, + }), + ); + vi.stubGlobal("fetch", fetchMock); + + try { + await api("/test", { + headers: { + "X-Profile-Id": "profile-old", + "X-Profile-Token": "fake", + }, + }); + + const headers = fetchMock.mock.calls[0]![1]!.headers as Record; + expect(headers["X-Profile-Id"]).toBe("profile-old"); + expect(headers["X-Profile-Token"]).toBe("fake"); + } finally { + setProfileToken(null); + } + }); + + it("safely refreshes a captured request without rebasing its profile authority", async () => { + Object.defineProperty(globalThis, "sessionStorage", { + value: { + getItem: () => null, + setItem: () => {}, + removeItem: () => {}, + clear: () => {}, + }, + configurable: true, + }); + setAccessToken("fake"); + setProfileId("profile-old"); + setProfileToken("fake"); + setRefreshToken("dummy"); + const snapshot = captureProfileRequestContext(); + expect(snapshot).not.toBeNull(); + const fetchMock = vi.fn(async (input, init) => { + const url = String(input); + if (url === "/api/v1/auth/refresh") { + expect(JSON.parse(String(init?.body))).toEqual({ refresh_token: "dummy" }); + return Response.json({ + access_token: "example", + refresh_token: "sample", + expires_in: 3600, + }); + } + const headers = init?.headers as Record; + expect(headers["X-Profile-Id"]).toBe("profile-old"); + expect(headers["X-Profile-Token"]).toBe("fake"); + if (headers.Authorization === "Bearer fake") { + return Response.json({ error: "unauthorized", message: "expired" }, { status: 401 }); + } + expect(headers.Authorization).toBe("Bearer example"); + return Response.json({ ok: true }); + }); + vi.stubGlobal("fetch", fetchMock); + + try { + await expect( + apiWithProfileRequestContext("/test", snapshot!, { method: "PUT" }), + ).resolves.toEqual({ ok: true }); + + expect(fetchMock).toHaveBeenCalledTimes(3); + expect(getAccessToken()).toBe("example"); + expect(localStorage.getItem("refresh_token")).toBe("sample"); + } finally { + setRefreshToken(null); + setProfileToken(null); + setAccessToken(null); + } + }); + + it("clears the active PIN when its captured profile authority is rejected", async () => { + setAccessToken("fake"); + setProfileId("profile-old"); + setProfileToken("pin-old"); + const snapshot = captureProfileRequestContext(); + expect(snapshot).not.toBeNull(); + const listener = vi.fn(); + onProfileUnverified(listener); + vi.stubGlobal( + "fetch", + vi.fn(async () => + Response.json({ error: "profile_unverified", message: "PIN required" }, { status: 403 }), + ), + ); + + try { + await expect(apiWithProfileRequestContext("/test", snapshot!)).rejects.toMatchObject({ + status: 403, + code: "profile_unverified", + }); + expect(getProfileToken()).toBeNull(); + expect(listener).toHaveBeenCalledOnce(); + } finally { + onProfileUnverified(null); + setProfileToken(null); + setProfileId(null); + setAccessToken(null); + } + }); + + it.each([ + { + authorityChange: "active profile", + changeAuthority: () => { + setProfileId("profile-new"); + setProfileToken("pin-new"); + }, + expectedToken: "pin-new", + }, + { + authorityChange: "active PIN", + changeAuthority: () => setProfileToken("pin-new"), + expectedToken: "pin-new", + }, + ])( + "does not clear the $authorityChange for a delayed rejection of captured authority", + async ({ changeAuthority, expectedToken }) => { + setAccessToken("fake"); + setProfileId("profile-old"); + setProfileToken("pin-old"); + const snapshot = captureProfileRequestContext(); + expect(snapshot).not.toBeNull(); + const listener = vi.fn(); + onProfileUnverified(listener); + let resolveResponse!: (response: Response) => void; + const response = new Promise((resolve) => { + resolveResponse = resolve; + }); + const fetchMock = vi.fn(() => response); + vi.stubGlobal("fetch", fetchMock); + + try { + const request = apiWithProfileRequestContext("/test", snapshot!); + await vi.waitFor(() => expect(fetchMock).toHaveBeenCalledOnce()); + changeAuthority(); + resolveResponse( + Response.json({ error: "profile_unverified", message: "PIN required" }, { status: 403 }), + ); + + await expect(request).rejects.toMatchObject({ + status: 403, + code: "profile_unverified", + }); + expect(getProfileToken()).toBe(expectedToken); + expect(listener).not.toHaveBeenCalled(); + } finally { + onProfileUnverified(null); + setProfileToken(null); + setProfileId(null); + setAccessToken(null); + } + }, + ); + + it.each(["account", "origin"] as const)( + "cancels captured retry when the %s changes during refresh", + async (changedContext) => { + Object.defineProperty(globalThis, "sessionStorage", { + value: { + getItem: () => null, + setItem: () => {}, + removeItem: () => {}, + clear: () => {}, + }, + configurable: true, + }); + const originalLocation = globalThis.location; + if (changedContext === "origin") { + Object.defineProperty(globalThis, "location", { + value: { origin: "https://server-old.example" }, + configurable: true, + }); + } + setAccessToken("fake"); + setProfileId("profile-old"); + setProfileToken("fake"); + setRefreshToken("dummy"); + const snapshot = captureProfileRequestContext(); + expect(snapshot).not.toBeNull(); + + let resolveRefresh!: (response: Response) => void; + const refreshResponse = new Promise((resolve) => { + resolveRefresh = resolve; + }); + const fetchMock = vi.fn(async (input) => { + if (String(input) === "/api/v1/auth/refresh") return refreshResponse; + return Response.json({ error: "unauthorized", message: "expired" }, { status: 401 }); + }); + vi.stubGlobal("fetch", fetchMock); + + try { + const request = apiWithProfileRequestContext("/test", snapshot!, { method: "PUT" }); + await vi.waitFor(() => expect(fetchMock).toHaveBeenCalledTimes(2)); + if (changedContext === "account") { + setAccessToken("sample"); + setRefreshToken("placeholder"); + } else { + Object.defineProperty(globalThis, "location", { + value: { origin: "https://server-new.example" }, + configurable: true, + }); + } + resolveRefresh( + Response.json({ + access_token: "example", + refresh_token: "redacted", + expires_in: 3600, + }), + ); + + await expect(request).rejects.toMatchObject({ name: "StaleApiRequestContextError" }); + expect(fetchMock).toHaveBeenCalledTimes(2); + expect(getAccessToken()).toBe(changedContext === "account" ? "sample" : "fake"); + expect(localStorage.getItem("refresh_token")).toBe( + changedContext === "account" ? "placeholder" : "dummy", + ); + } finally { + if (changedContext === "origin") { + Object.defineProperty(globalThis, "location", { + value: originalLocation, + configurable: true, + }); + } + setRefreshToken(null); + setProfileToken(null); + setAccessToken(null); + } + }, + ); + + it("refuses a captured request after the account context changes", async () => { + Object.defineProperty(globalThis, "sessionStorage", { + value: { + getItem: () => null, + setItem: () => {}, + removeItem: () => {}, + clear: () => {}, + }, + configurable: true, + }); + setAccessToken("fake"); + setProfileId("profile-old"); + setProfileToken("fake"); + const snapshot = captureProfileRequestContext(); + expect(snapshot).not.toBeNull(); + setAccessToken("dummy"); + setProfileId("profile-new"); + setProfileToken("dummy"); + const fetchMock = vi.fn(); + vi.stubGlobal("fetch", fetchMock); + + try { + await expect(apiWithProfileRequestContext("/test", snapshot!)).rejects.toMatchObject({ + name: "StaleApiRequestContextError", + }); + expect(fetchMock).not.toHaveBeenCalled(); + } finally { + setProfileToken(null); + } + }); + + it("keeps the browser client family canonical", async () => { + Object.defineProperty(globalThis, "sessionStorage", { + value: { + getItem: () => null, + setItem: () => {}, + removeItem: () => {}, + clear: () => {}, + }, + configurable: true, + }); + const fetchMock = vi.fn( + async () => + new Response(JSON.stringify({ ok: true }), { + status: 200, + headers: { "Content-Type": "application/json" }, + }), + ); + vi.stubGlobal("fetch", fetchMock); + + await api("/test", { headers: { "x-silo-client-family": "tv" } }); + + const headers = fetchMock.mock.calls[0]![1]!.headers as Record; + expect(headers["X-Silo-Client-Family"]).toBe("web"); + expect( + Object.keys(headers).filter((key) => key.toLowerCase() === "x-silo-client-family"), + ).toEqual(["X-Silo-Client-Family"]); + }); + it("forwards AbortSignal from options to fetch", async () => { Object.defineProperty(globalThis, "sessionStorage", { value: { diff --git a/web/src/api/client.ts b/web/src/api/client.ts index bc4c55b6..cc961361 100644 --- a/web/src/api/client.ts +++ b/web/src/api/client.ts @@ -10,9 +10,18 @@ export function onProfileUnverified(listener: ProfileUnverifiedListener | null) } let accessToken: string | null = null; +let authContextVersion = 0; let refreshPromise: Promise | null = null; export function setAccessToken(token: string | null) { + if (accessToken !== token) authContextVersion += 1; + accessToken = token; +} + +function refreshCurrentAccessToken(token: string): void { + // Token rotation preserves the authenticated account/server context. A + // queued request may use its captured predecessor once, safely refresh, and + // retry with this successor without changing account authority. accessToken = token; } @@ -83,6 +92,59 @@ export function getProfileToken(): string | null { return profileToken; } +/** + * Complete request authority for one queued profile intent. It is deliberately + * an in-memory value: access and PIN tokens must never enter storage, query + * caches, logs, or persisted mutation state through this snapshot. + */ +export interface ProfileRequestContextSnapshot { + accessToken: string; + authContextVersion: number; + serverOrigin: string; + profileId: string; + profileToken: string | null; +} + +function currentServerOrigin(): string { + return typeof globalThis.location === "undefined" ? "" : globalThis.location.origin; +} + +/** Capture account, server, profile, and PIN authority in one synchronous turn. */ +export function captureProfileRequestContext(): ProfileRequestContextSnapshot | null { + const profileId = getProfileId(); + if (!accessToken || !profileId) return null; + return { + accessToken, + authContextVersion, + serverOrigin: currentServerOrigin(), + profileId, + profileToken: getProfileToken(), + }; +} + +/** + * A session-authority change advances authContextVersion even if an account + * later happens to return to the same token. Queued work can therefore never + * cross a logout, impersonation, account switch, or server-origin switch + * unnoticed. Automatic token rotation deliberately preserves the context. + * Profile/PIN changes are excluded so an already-created intent remains bound + * to its captured profile authority through a same-account refresh. + */ +export function isProfileRequestContextCurrent(snapshot: ProfileRequestContextSnapshot): boolean { + return ( + snapshot.authContextVersion === authContextVersion && + snapshot.serverOrigin === currentServerOrigin() + ); +} + +function isCapturedProfileAuthorityActive(snapshot: ProfileRequestContextSnapshot): boolean { + return ( + isProfileRequestContextCurrent(snapshot) && + getProfileId() === snapshot.profileId && + getProfileToken() === snapshot.profileToken + ); +} + function getOrCreateDeviceId(): string { const existing = storage.get(storage.KEYS.DEVICE_ID); if (existing) { @@ -131,6 +193,9 @@ function getDeviceHeaders(): Record { "X-Silo-Device-Id": deviceId, "X-Silo-Device-Name": detectDeviceName(), "X-Silo-Device-Platform": detectDevicePlatform(), + // Browser preferences roam between browsers without changing TV, mobile, + // tablet, or desktop-native layouts. + "X-Silo-Client-Family": "web", }; } @@ -138,10 +203,22 @@ async function attemptRefresh(): Promise { const rt = getRefreshToken(); if (!rt) return false; + // A refresh response belongs only to the account/server that started it. + // Discarding it after a context switch prevents a delayed old-account + // response from overwriting the new account's access or refresh token. + const startingAuthContextVersion = authContextVersion; + const startingServerOrigin = currentServerOrigin(); + try { const data = await refreshAccessToken(rt, fetch); if (!data) return false; - setAccessToken(data.access_token); + if ( + startingAuthContextVersion !== authContextVersion || + startingServerOrigin !== currentServerOrigin() + ) { + return false; + } + refreshCurrentAccessToken(data.access_token); setRefreshToken(data.refresh_token); return true; } catch { @@ -251,6 +328,14 @@ function hasHeader(headers: Record, name: string): boolean { return Object.keys(headers).some((key) => key.toLowerCase() === target); } +function setHeader(headers: Record, name: string, value: string): void { + const target = name.toLowerCase(); + for (const key of Object.keys(headers)) { + if (key.toLowerCase() === target) delete headers[key]; + } + headers[name] = value; +} + interface ParsedApiError { /** Normalized error with guaranteed `error`/`message` fields. */ apiErr: ApiError; @@ -326,9 +411,7 @@ export async function restoreUserSession({ }; } -export async function api(path: string, options: RequestInit = {}): Promise { - const res = await apiResponse(path, options); - +async function readApiResponse(res: Response): Promise { // Handle empty successful responses. if (res.status === 204 || res.status === 205) { return undefined as T; @@ -340,22 +423,89 @@ export async function api(path: string, options: RequestInit = {}): Promise(path: string, options: RequestInit = {}): Promise { + return readApiResponse(await apiResponse(path, options)); +} + +/** + * Sends a request with one captured account/profile authority. The explicit + * headers cannot be replaced by the current session, and a stale snapshot is + * rejected before fetch. + */ +export async function apiWithProfileRequestContext( + path: string, + snapshot: ProfileRequestContextSnapshot, + options: RequestInit = {}, +): Promise { + if (!isProfileRequestContextCurrent(snapshot)) { + throw new StaleApiRequestContextError(); + } + const headers = { ...(options.headers as Record) }; + setHeader(headers, "Authorization", `Bearer ${snapshot.accessToken}`); + setHeader(headers, "X-Profile-Id", snapshot.profileId); + setHeader(headers, "X-Profile-Token", snapshot.profileToken ?? ""); + const response = await apiResponseInternal(path, { ...options, headers }, snapshot); + if (!isProfileRequestContextCurrent(snapshot)) { + throw new StaleApiRequestContextError(); + } + return readApiResponse(response); +} + +export class StaleApiRequestContextError extends Error { + constructor() { + super("The account or server changed before the queued request could be sent."); + this.name = "StaleApiRequestContextError"; + } +} + /** Performs an authenticated API request while leaving the successful body unread. */ export async function apiResponse(path: string, options: RequestInit = {}): Promise { + return apiResponseInternal(path, options); +} + +async function apiResponseInternal( + path: string, + options: RequestInit, + snapshot?: ProfileRequestContextSnapshot, +): Promise { + if (snapshot && !isProfileRequestContextCurrent(snapshot)) { + throw new StaleApiRequestContextError(); + } + const explicitAuthorization = hasHeader( + (options.headers as Record | undefined) ?? {}, + "Authorization", + ); const headers = buildApiHeaders(options); const requestProfileId = headers["X-Profile-Id"] ?? null; const requestProfileToken = headers["X-Profile-Token"] ?? null; let res = await fetch(`/api/v1${path}`, { ...options, headers }); - // Auto-refresh on 401 - if (res.status === 401 && getRefreshToken()) { + if (snapshot && !isProfileRequestContextCurrent(snapshot)) { + throw new StaleApiRequestContextError(); + } + + // Auto-refresh on 401. An ordinary explicit Authorization header opts out, + // but a captured profile request is a stronger contract: it may rotate the + // token only while its account/server generation remains current, then retry + // with the new access token and the exact captured profile/PIN headers. + if ( + res.status === 401 && + getRefreshToken() && + (snapshot !== undefined || !explicitAuthorization) + ) { + if (snapshot && !isProfileRequestContextCurrent(snapshot)) { + throw new StaleApiRequestContextError(); + } if (!refreshPromise) { refreshPromise = attemptRefresh().finally(() => { refreshPromise = null; }); } const refreshed = await refreshPromise; + if (snapshot && !isProfileRequestContextCurrent(snapshot)) { + throw new StaleApiRequestContextError(); + } if (refreshed) { // Keep the profile and device identity captured for the original // request. A household profile can change while refresh is pending; @@ -364,11 +514,16 @@ export async function apiResponse(path: string, options: RequestInit = {}): Prom // is allowed to change for this retry. const refreshedHeaders = { ...headers }; if (accessToken) { - refreshedHeaders.Authorization = `Bearer ${accessToken}`; + setHeader(refreshedHeaders, "Authorization", `Bearer ${accessToken}`); + } else if (snapshot) { + throw new StaleApiRequestContextError(); } else { delete refreshedHeaders.Authorization; } res = await fetch(`/api/v1${path}`, { ...options, headers: refreshedHeaders }); + if (snapshot && !isProfileRequestContextCurrent(snapshot)) { + throw new StaleApiRequestContextError(); + } } } @@ -377,8 +532,9 @@ export async function apiResponse(path: string, options: RequestInit = {}): Prom if ( res.status === 403 && parsed.apiErr.error === "profile_unverified" && - getProfileId() === requestProfileId && - getProfileToken() === requestProfileToken + (snapshot + ? isCapturedProfileAuthorityActive(snapshot) + : getProfileId() === requestProfileId && getProfileToken() === requestProfileToken) ) { setProfileToken(null); profileUnverifiedListener?.(); @@ -395,18 +551,20 @@ function buildApiHeaders(options: RequestInit = {}): Record { if (!(options.body instanceof FormData) && !hasHeader(headers, "Content-Type")) { headers["Content-Type"] = "application/json"; } - if (accessToken) { + if (accessToken && !hasHeader(headers, "Authorization")) { headers["Authorization"] = `Bearer ${accessToken}`; } const profileId = getProfileId(); - if (profileId) { + if (profileId && !hasHeader(headers, "X-Profile-Id")) { headers["X-Profile-Id"] = profileId; } const profToken = getProfileToken(); - if (profToken) { + if (profToken && !hasHeader(headers, "X-Profile-Token")) { headers["X-Profile-Token"] = profToken; } - Object.assign(headers, getDeviceHeaders()); + for (const [name, value] of Object.entries(getDeviceHeaders())) { + setHeader(headers, name, value); + } return headers; } diff --git a/web/src/components/AppSidebar.test.tsx b/web/src/components/AppSidebar.test.tsx index afea715d..5f7d1487 100644 --- a/web/src/components/AppSidebar.test.tsx +++ b/web/src/components/AppSidebar.test.tsx @@ -21,6 +21,22 @@ import { const mockLogout = vi.fn(); const mockClearProfile = vi.fn(); const mockTogglePin = vi.fn(); +let mockPrimaryMenu: { + items: Array< + | { + type: "builtin"; + destination: "home" | "movies" | "series" | "music" | "audiobooks"; + } + | { type: "section"; library_id: number; section_id: string; label: string } + | { + type: "collection"; + library_id?: number; + collection_id: string; + label: string; + } + >; +} | null = null; +let mockLibraries = [{ id: 7, name: "Movies", type: "movies" }]; vi.mock("@/hooks/useAuth", () => { const useAuth = () => ({ @@ -39,7 +55,7 @@ vi.mock("@/hooks/useCurrentProfile", () => ({ vi.mock("@/hooks/queries/libraries", () => ({ useUserLibraries: () => ({ - data: [{ id: 7, name: "Movies", type: "movies" }], + data: mockLibraries, }), })); @@ -51,6 +67,16 @@ vi.mock("@/hooks/queries/sidebarPins", () => ({ }), useToggleSidebarPin: () => ({ togglePin: mockTogglePin, + canToggle: true, + }), +})); + +vi.mock("@/hooks/useUICustomization", () => ({ + useUICustomization: () => ({ + primaryMenu: mockPrimaryMenu, + shortcuts: { items: [] }, + cardPresentation: { poster_size: "standard", caption: "title_metadata" }, + isLoading: false, }), })); @@ -161,6 +187,8 @@ describe("AppSidebar", () => { mockClearProfile.mockReset(); mockTogglePin.mockReset(); mockPluginInstallations = []; + mockPrimaryMenu = null; + mockLibraries = [{ id: 7, name: "Movies", type: "movies" }]; }); it("uses the cinema highlight text color for active catalog source links", () => { @@ -228,6 +256,124 @@ describe("AppSidebar", () => { expect(markup).not.toContain("text-sidebar-primary-foreground bg-sidebar-accent"); }); + it("drops library-scoped menu targets when their library is not visible", () => { + mockPrimaryMenu = { + items: [ + { type: "builtin", destination: "home" }, + { + type: "section", + library_id: 99, + section_id: "hidden-section", + label: "Hidden section", + }, + { + type: "collection", + library_id: 99, + collection_id: "hidden-collection", + label: "Hidden collection", + }, + { + type: "collection", + collection_id: "global-collection", + label: "Global collection", + }, + ], + }; + + const markup = renderSidebar("/"); + + expect(markup).not.toContain("Hidden section"); + expect(markup).not.toContain("Hidden collection"); + expect(markup).toContain("Global collection"); + }); + + it("does not reinterpret global media built-ins as the first matching library", () => { + mockLibraries = [ + { id: 7, name: "Movies A", type: "movies" }, + { id: 8, name: "Movies B", type: "movie" }, + { id: 9, name: "TV A", type: "series" }, + { id: 10, name: "TV B", type: "shows" }, + { id: 11, name: "Books A", type: "audiobooks" }, + { id: 12, name: "Books B", type: "audiobook" }, + { id: 13, name: "Music A", type: "music" }, + { id: 14, name: "Music B", type: "music" }, + ]; + mockPrimaryMenu = { + items: [ + { type: "builtin", destination: "home" }, + { type: "builtin", destination: "movies" }, + { type: "builtin", destination: "series" }, + { type: "builtin", destination: "audiobooks" }, + { type: "builtin", destination: "music" }, + ], + }; + + const document = parseMarkup(renderSidebar("/")); + const primaryMenu = document.querySelector('nav[aria-label="Main navigation"] > ul'); + const primaryHrefs = [...(primaryMenu?.querySelectorAll("a") ?? [])].map((link) => + link.getAttribute("href"), + ); + + expect(primaryHrefs).toEqual(["/"]); + expect(document.querySelector('a[href="/library/7"]')).not.toBeNull(); + expect(document.querySelector('a[href="/library/14"]')).not.toBeNull(); + }); + + it("keeps a custom section active while sort and filter params change", () => { + mockPrimaryMenu = { + items: [ + { + type: "section", + library_id: 7, + section_id: "recent", + label: "Custom Recently Added", + }, + ], + }; + + const document = parseMarkup( + renderSidebar( + "/catalog?source=section&scope=library&library_id=7§ion_id=recent&sort=title&order=asc&genre=Drama", + ), + ); + const link = [...document.querySelectorAll("a")].find((candidate) => + candidate.textContent?.includes("Custom Recently Added"), + ); + + expect(link?.getAttribute("aria-current")).toBe("page"); + }); + + it("matches custom collections by source, library, and collection id", () => { + mockPrimaryMenu = { + items: [ + { + type: "collection", + library_id: 7, + collection_id: "favorites", + label: "Custom Favorites", + }, + ], + }; + + const filtered = parseMarkup( + renderSidebar( + "/catalog?source=library_collection&library_id=7&collection_id=favorites&type=movie&sort=year&order=desc", + ), + ); + const otherLibrary = parseMarkup( + renderSidebar( + "/catalog?source=library_collection&library_id=8&collection_id=favorites&sort=year", + ), + ); + const findCustomLink = (document: Document) => + [...document.querySelectorAll("a")].find((candidate) => + candidate.textContent?.includes("Custom Favorites"), + ); + + expect(findCustomLink(filtered)?.getAttribute("aria-current")).toBe("page"); + expect(findCustomLink(otherLibrary)?.hasAttribute("aria-current")).toBe(false); + }); + it("preserves the current library query when linking to the active library", () => { const markup = renderSidebar("/library/7?tab=library&sort=year&order=desc"); diff --git a/web/src/components/AppSidebar.tsx b/web/src/components/AppSidebar.tsx index 712fe2fd..a41b581d 100644 --- a/web/src/components/AppSidebar.tsx +++ b/web/src/components/AppSidebar.tsx @@ -29,7 +29,9 @@ import { buildPersonalCatalogHref, buildQueryCatalogHref, buildSectionCatalogHref, + buildUserCollectionCatalogHref, parseCatalogSearchParams, + sameCatalogDestination, } from "@/pages/catalogSearchParams"; import { Avatar, AvatarFallback, AvatarImage } from "@/components/ui/avatar"; import { @@ -69,6 +71,8 @@ import { import { useTheme } from "@/hooks/useTheme"; import { CURATED_THEME_IDS, THEMES } from "@/lib/themes"; import { cn } from "@/lib/utils"; +import { useUICustomization } from "@/hooks/useUICustomization"; +import { menuItemKey } from "@/lib/uiCustomization"; function getLibraryIcon(type: string) { switch (type) { @@ -176,6 +180,13 @@ interface AppSidebarProps { collapsed?: boolean; } +interface ResolvedPrimaryMenuLink { + key: string; + href: string; + label: string; + icon: ReactNode; +} + export default function AppSidebar({ onNavigate, collapsed = false }: AppSidebarProps) { const location = useLocation(); const navigate = useViewTransitionNavigate(); @@ -186,7 +197,8 @@ export default function AppSidebar({ onNavigate, collapsed = false }: AppSidebar const showAdminNav = useIsActingAdmin(); const { data: libraries } = useUserLibraries(); const { pins } = useSidebarPins(); - const { togglePin } = useToggleSidebarPin(); + const { togglePin, canToggle } = useToggleSidebarPin(); + const { primaryMenu } = useUICustomization(); const { data: pluginSettings } = usePluginSettingsList(); const requestStatus = useRequestFeatureStatus(); const showRequestsNav = requestStatus.data?.requests_enabled === true; @@ -220,6 +232,104 @@ export default function AppSidebar({ onNavigate, collapsed = false }: AppSidebar // Grouped view of the Apps entries (null → keep the flat list under the // single "Apps" header). See groupAppNavLinks for the SDK category contract. const pluginNavGroups = useMemo(() => groupAppNavLinks(pluginNavLinks), [pluginNavLinks]); + const primaryMenuLinks = useMemo(() => { + if (!primaryMenu) return null; + + return primaryMenu.items.flatMap((item): ResolvedPrimaryMenuLink[] => { + const key = menuItemKey(item); + if (item.type === "library") { + const library = libraries?.find((candidate) => candidate.id === item.library_id); + if (!library) return []; + return [ + { + key, + href: `/library/${library.id}`, + label: item.label || library.name, + icon: getLibraryIcon(library.type), + }, + ]; + } + if (item.type === "section") { + if (!libraries?.some((candidate) => candidate.id === item.library_id)) return []; + return [ + { + key, + href: buildSectionCatalogHref({ + scope: "library", + libraryId: item.library_id, + sectionId: item.section_id, + title: item.label, + }), + label: item.label, + icon: , + }, + ]; + } + if (item.type === "collection") { + if ( + item.library_id !== undefined && + !libraries?.some((candidate) => candidate.id === item.library_id) + ) { + return []; + } + return [ + { + key, + href: + item.library_id === undefined + ? buildUserCollectionCatalogHref(item.collection_id, item.label) + : buildLibraryCollectionCatalogHref( + item.collection_id, + item.label, + item.library_id, + ), + label: item.label, + icon: , + }, + ]; + } + + switch (item.destination) { + case "home": + return [ + { + key, + href: "/", + label: "Home", + icon: , + }, + ]; + case "for_you": + return [ + { + key, + href: "/recommendations", + label: "For You", + icon: , + }, + ]; + case "calendar": + return [ + { + key, + href: "/calendar", + label: "Calendar", + icon: , + }, + ]; + // These contract built-ins identify global media destinations, not a + // particular library. Web does not currently have complete global + // routes for all four media families (notably music), so omit them + // instead of silently changing their meaning to the first matching + // library. An explicit `library` menu item remains fully supported. + case "movies": + case "series": + case "music": + case "audiobooks": + return []; + } + }); + }, [libraries, primaryMenu]); const catalogState = useMemo( () => @@ -300,7 +410,11 @@ export default function AppSidebar({ onNavigate, collapsed = false }: AppSidebar } if (pin.type === "collection") { - return catalogState.source === "library_collection" && catalogState.collection_id === pin.id; + return ( + catalogState.source === "library_collection" && + catalogState.collection_id === pin.id && + catalogState.library_id === libId + ); } return ( @@ -382,26 +496,67 @@ export default function AppSidebar({ onNavigate, collapsed = false }: AppSidebar aria-label="Main navigation" className="sidebar-nav sidebar-scroll flex-1 space-y-7 overflow-y-auto px-3 pb-5" > - {/* Home */} -
    -
  • - - {isActive("/", true) && ( - - )} - - Home - -
    • -
+ {/* A customized primary menu is an ordered shortcut rail. Search and + profile controls remain fixed elsewhere, and the full library + tree remains available below as a safe browsing fallback. */} + {primaryMenuLinks ? ( +
    + {primaryMenuLinks.map((link) => { + const [targetPath, targetQuery] = link.href.split("?", 2); + const active = + targetPath === "/" + ? isActive("/", true) + : targetPath === "/catalog" && targetQuery !== undefined && catalogState + ? sameCatalogDestination( + catalogState, + parseCatalogSearchParams(new URLSearchParams(targetQuery)), + ) + : targetQuery === undefined + ? location.pathname === targetPath || + location.pathname.startsWith(`${targetPath}/`) + : location.pathname === targetPath && location.search === `?${targetQuery}`; + return ( +
  • + + {active ? ( + + ) : null} + {link.icon} + {link.label} + +
    • + ); + })} +
+ ) : ( +
    +
  • + + {isActive("/", true) && ( + + )} + + Home + +
    • +
+ )} {/* Libraries */} {libraries && libraries.length > 0 && ( @@ -497,7 +652,7 @@ export default function AppSidebar({ onNavigate, collapsed = false }: AppSidebar {libraryPins.map((pin) => { const pinHref = pin.type === "collection" - ? buildLibraryCollectionCatalogHref(pin.id, pin.label) + ? buildLibraryCollectionCatalogHref(pin.id, pin.label, lib.id) : buildSectionCatalogHref({ scope: "library", libraryId: lib.id, @@ -527,20 +682,22 @@ export default function AppSidebar({ onNavigate, collapsed = false }: AppSidebar )} {pin.label} - + {canToggle ? ( + + ) : null} ); })} @@ -583,23 +740,25 @@ export default function AppSidebar({ onNavigate, collapsed = false }: AppSidebar -
  • - - {isActive("/recommendations") && ( - - )} - - Recommendations - -
    • + {primaryMenuLinks === null ? ( +
  • + + {isActive("/recommendations") && ( + + )} + + Recommendations + +
    • + ) : null} {showRequestsNav && (
    • )} -
  • - - {isActive("/calendar") && ( - - )} - - Calendar - -
    • + {primaryMenuLinks === null ? ( +
  • + + {isActive("/calendar") && ( + + )} + + Calendar + +
    • + ) : null} {showNotificationsNav && (
  • {/* Info */} -
    - - {heading} - - {episodeMeta && ( + {showCaption ? ( +
    - {episodeMeta} + {heading} - )} - {premiereBadge && ( -
    - - {upcomingBadgeLabel(premiereBadge)} - -
    - )} - {timeLeftLabel && - (timeLeftLabel === "\u00A0" ? ( -
    {timeLeftLabel}
    - ) : ( + {showMetadata && episodeMeta && ( - {timeLeftLabel} + {episodeMeta} - ))} -
    + )} + {showMetadata && premiereBadge && ( +
    + + {upcomingBadgeLabel(premiereBadge)} + +
    + )} + {showMetadata && + timeLeftLabel && + (timeLeftLabel === "\u00A0" ? ( +
    {timeLeftLabel}
    + ) : ( + + {timeLeftLabel} + + ))} +
    + ) : null} ); } diff --git a/web/src/components/ItemCard.tsx b/web/src/components/ItemCard.tsx index f7703707..e9ac98ef 100644 --- a/web/src/components/ItemCard.tsx +++ b/web/src/components/ItemCard.tsx @@ -10,6 +10,7 @@ import { overlayDataFromBrowseItem, type CardOverlayPrefs } from "@/lib/overlays import { buildEpisodeCardLabels } from "@/lib/episodeCardLabels"; import { formatDate as formatPreferredDate } from "@/lib/datetime"; import { formatBitrate } from "@/lib/mediaFormat"; +import { useUICustomization } from "@/hooks/useUICustomization"; const DATE_ONLY_PATTERN = /^\d{4}-\d{2}-\d{2}$/; @@ -190,6 +191,9 @@ export default function ItemCard({ const displayTitle = episodeLabels ? episodeLabels.seriesTitle : item.title; const mangaCountLabel = mangaCountChipLabel(item); const mangaStatus = mangaStatusChip(item); + const { cardPresentation } = useUICustomization(); + const showCaption = cardPresentation.caption !== "artwork"; + const showMetadata = cardPresentation.caption === "title_metadata"; return (
    @@ -302,17 +306,21 @@ export default function ItemCard({ variant="poster" />
    - -
    {displayTitle}
    - {episodeLabels?.episodeTitle ? ( -
    - {episodeLabels.episodeTitle} -
    - ) : null} -
    - -
    -     + {showCaption ? ( + +
    {displayTitle}
    + {showMetadata && episodeLabels?.episodeTitle ? ( +
    + {episodeLabels.episodeTitle} +
    + ) : null} + {showMetadata ? ( +
    + +
    + ) : null} +     + ) : null} ); } diff --git a/web/src/components/ItemGrid.tsx b/web/src/components/ItemGrid.tsx index c75d6384..6118d828 100644 --- a/web/src/components/ItemGrid.tsx +++ b/web/src/components/ItemGrid.tsx @@ -5,6 +5,8 @@ import ItemCard from "./ItemCard"; import { Skeleton } from "@/components/ui/skeleton"; import { useGridLayout } from "@/hooks/useGridLayout"; import { useOverlayPrefs } from "@/hooks/useOverlayPrefs"; +import { useUICustomization } from "@/hooks/useUICustomization"; +import { cardGridClasses, cardTextAreaHeight } from "@/lib/uiCustomization"; interface SharedItemGridProps { loading?: boolean; @@ -37,11 +39,6 @@ function hasStaticItems(props: ItemGridProps): props is StaticItemGridProps { return Array.isArray((props as StaticItemGridProps).items); } -const GRID_GAP = 12; -const TEXT_AREA_HEIGHT = 44; -const GRID_CLASSES = - "grid grid-cols-3 sm:grid-cols-4 md:grid-cols-5 lg:grid-cols-7 xl:grid-cols-8 gap-3"; - export default function ItemGrid(props: ItemGridProps) { const { loading, @@ -52,6 +49,9 @@ export default function ItemGrid(props: ItemGridProps) { onToggleSelect, } = props; const { prefs: overlayPrefs } = useOverlayPrefs(); + const { cardPresentation } = useUICustomization(); + const gridGap = cardPresentation.poster_size === "large" ? 16 : 12; + const gridClasses = cardGridClasses(cardPresentation.poster_size); const totalItems = hasStaticItems(props) ? props.items.length : props.totalItems; const pages = hasStaticItems(props) ? new Map([[0, props.items]]) @@ -59,8 +59,9 @@ export default function ItemGrid(props: ItemGridProps) { const pageSize = hasStaticItems(props) ? Math.max(props.items.length, 1) : props.pageSize; const onVisibleRangeChange = hasStaticItems(props) ? () => undefined : props.onVisibleRangeChange; const { containerRef, layout } = useGridLayout({ - gap: GRID_GAP, - textAreaHeight: TEXT_AREA_HEIGHT, + gap: gridGap, + textAreaHeight: cardTextAreaHeight(cardPresentation.caption), + layoutKey: cardPresentation.poster_size, }); const [anchorEl, setAnchorEl] = useState(null); const { columnCount, rowHeight } = layout; @@ -79,6 +80,12 @@ export default function ItemGrid(props: ItemGridProps) { scrollMargin, }); + // The estimate function closes over rowHeight. Explicitly invalidate the + // cached measurements when a poster/caption preset changes that height. + useEffect(() => { + virtualizer.measure(); + }, [rowHeight, virtualizer]); + const virtualRows = virtualizer.getVirtualItems(); // Report visible item range to parent for page fetching @@ -103,7 +110,7 @@ export default function ItemGrid(props: ItemGridProps) { return (
    {loading ? ( -
    +
    {Array.from({ length: 24 }).map((_, i) => (
    @@ -124,7 +131,7 @@ export default function ItemGrid(props: ItemGridProps) {
    ({ @@ -30,4 +31,37 @@ describe("RecommendationGrid", () => { expect(markup).toContain('href="/item/ebook%201"'); expect(mocks.useCatalogItemDetail).toHaveBeenCalledWith("ebook 1"); }); + + it("uses the selected density and omits artwork-only caption rows", () => { + mocks.useCatalogItemDetail.mockReturnValue({ + data: { + content_id: "ebook 1", + title: "A Reader", + poster_url: "/cover.jpg", + }, + }); + + const markup = renderToStaticMarkup( + + + + + , + ); + + expect(markup).toContain("grid-cols-3 sm:grid-cols-5"); + expect(markup).not.toContain(">A Reader

    "); + }); }); diff --git a/web/src/components/RecommendationGrid.tsx b/web/src/components/RecommendationGrid.tsx index d096bb1e..469c6503 100644 --- a/web/src/components/RecommendationGrid.tsx +++ b/web/src/components/RecommendationGrid.tsx @@ -1,5 +1,7 @@ import ViewTransitionLink from "@/components/ViewTransitionLink"; import { useCatalogItemDetail } from "@/hooks/queries/catalogRead"; +import { useUICustomization } from "@/hooks/useUICustomization"; +import { cardGridClasses } from "@/lib/uiCustomization"; interface RecommendationGridProps { items: Array<{ media_item_id: string }>; @@ -12,6 +14,7 @@ interface RecommendationItemCardProps { function RecommendationItemCard({ itemId }: RecommendationItemCardProps) { const { data: item } = useCatalogItemDetail(itemId); + const { cardPresentation } = useUICustomization(); if (!item) { return
    ; } @@ -30,14 +33,17 @@ function RecommendationItemCard({ itemId }: RecommendationItemCardProps) {
    )}
    -

    {item.title}

    + {cardPresentation.caption !== "artwork" ? ( +

    {item.title}

    + ) : null} ); } export default function RecommendationGrid({ items, maxItems = 12 }: RecommendationGridProps) { + const { cardPresentation } = useUICustomization(); return ( -
    +
    {items.slice(0, maxItems).map((si) => ( ))} diff --git a/web/src/components/SectionItemCard.tsx b/web/src/components/SectionItemCard.tsx index 3cd8bc52..203f0e54 100644 --- a/web/src/components/SectionItemCard.tsx +++ b/web/src/components/SectionItemCard.tsx @@ -14,6 +14,7 @@ import { upcomingBadgeLabel, } from "@/lib/upcomingEventPresentation"; import type { SectionItem } from "@/api/types"; +import { useUICustomization } from "@/hooks/useUICustomization"; interface SectionItemCardProps { item: SectionItem; @@ -32,6 +33,9 @@ export default function SectionItemCard({ item, libraryId }: SectionItemCardProp const airDateLabel = upcomingEvent ? formatUpcomingDate(upcomingEvent.air_date) : ""; const airTimeLabel = upcomingEvent ? formatUpcomingTime(upcomingEvent.air_time) : null; const episodeLabels = !upcomingEvent ? buildEpisodeCardLabels(item) : null; + const { cardPresentation } = useUICustomization(); + const showCaption = cardPresentation.caption !== "artwork"; + const showMetadata = cardPresentation.caption === "title_metadata"; return (
    @@ -97,45 +101,47 @@ export default function SectionItemCard({ item, libraryId }: SectionItemCardProp variant="poster" />
    - -
    - {episodeLabels ? episodeLabels.seriesTitle : item.title} -
    - {upcomingEvent ? ( - <> - {subtitle && ( -
    - {subtitle} + {showCaption ? ( + +
    + {episodeLabels ? episodeLabels.seriesTitle : item.title} +
    + {showMetadata && upcomingEvent ? ( + <> + {subtitle && ( +
    + {subtitle} +
    + )} +
    + {airDateLabel} + {airTimeLabel && {airTimeLabel}}
    - )} -
    - {airDateLabel} - {airTimeLabel && {airTimeLabel}} + + ) : showMetadata && episodeLabels ? ( + <> + {episodeLabels.episodeTitle ? ( +
    + {episodeLabels.episodeTitle} +
    + ) : null} +
    + {episodeLabels.episodeCode} +
    + + ) : showMetadata && item.item_source === "next_in_series" && item.series_title ? ( +
    + {[item.badges?.find((badge) => badge.startsWith("Book ")), item.series_title] + .filter(Boolean) + .join(" · ")}
    - - ) : episodeLabels ? ( - <> - {episodeLabels.episodeTitle ? ( -
    - {episodeLabels.episodeTitle} -
    - ) : null} + ) : showMetadata ? (
    - {episodeLabels.episodeCode} + {item.year ? `${item.year}` : ""} {item.type === "series" ? "Series" : ""}
    - - ) : item.item_source === "next_in_series" && item.series_title ? ( -
    - {[item.badges?.find((badge) => badge.startsWith("Book ")), item.series_title] - .filter(Boolean) - .join(" · ")} -
    - ) : ( -
    - {item.year ? `${item.year}` : ""} {item.type === "series" ? "Series" : ""} -
    - )} - + ) : null} + + ) : null}
    ); } diff --git a/web/src/components/SectionRow.test.tsx b/web/src/components/SectionRow.test.tsx index f95a7e55..008751f4 100644 --- a/web/src/components/SectionRow.test.tsx +++ b/web/src/components/SectionRow.test.tsx @@ -50,6 +50,7 @@ vi.mock("@/hooks/queries/sidebarPins", () => ({ useToggleSidebarPin: () => ({ togglePin: mockTogglePin, isPinned: mockIsPinned, + canToggle: true, }), })); diff --git a/web/src/components/SectionRow.tsx b/web/src/components/SectionRow.tsx index a55c18c1..3350bde9 100644 --- a/web/src/components/SectionRow.tsx +++ b/web/src/components/SectionRow.tsx @@ -7,6 +7,8 @@ import { useOverlayPrefs } from "@/hooks/useOverlayPrefs"; import { buildSectionCatalogHref, isSectionBrowseSupported } from "@/pages/catalogSearchParams"; import type { ResolvedSection } from "@/api/types"; import { Pin, PinOff } from "lucide-react"; +import { useUICustomization } from "@/hooks/useUICustomization"; +import { carouselCardWidthClasses } from "@/lib/uiCustomization"; interface SectionRowProps { section: ResolvedSection; @@ -23,9 +25,11 @@ function SectionPinButton({ sectionTitle: string; libraryId: number; }) { - const { togglePin, isPinned } = useToggleSidebarPin(); + const { togglePin, isPinned, canToggle } = useToggleSidebarPin(); const pinned = isPinned(libraryId, "section", sectionId); + if (!canToggle) return null; + return ( {/* Pin to sidebar button — only for library collections */} - {!isUserCollection && ( + {!isUserCollection && canToggle && ( +
    + ) : isLoading ? ( ) : ( ({ + useUICustomization: vi.fn(), + useUserLibraries: vi.fn(), + useSetSettingValue: vi.fn(), + useClearSettingValue: vi.fn(), +})); + +vi.mock("@/hooks/useUICustomization", () => ({ + useUICustomization: (...args: unknown[]) => mocks.useUICustomization(...args), +})); + +vi.mock("@/hooks/queries/libraries", () => ({ + useUserLibraries: (...args: unknown[]) => mocks.useUserLibraries(...args), +})); + +vi.mock("@/hooks/queries/settingValues", () => ({ + useSetSettingValue: (...args: unknown[]) => mocks.useSetSettingValue(...args), + useClearSettingValue: (...args: unknown[]) => mocks.useClearSettingValue(...args), +})); + +vi.mock("sonner", () => ({ + toast: { success: vi.fn(), error: vi.fn() }, +})); + +import InterfaceSettings from "./InterfaceSettings"; + +describe("InterfaceSettings card resets", () => { + let clearMutateAsync: ReturnType; + let setMutateAsync: ReturnType; + + beforeEach(() => { + mocks.useUICustomization.mockReset(); + mocks.useUserLibraries.mockReset(); + mocks.useSetSettingValue.mockReset(); + mocks.useClearSettingValue.mockReset(); + clearMutateAsync = vi.fn().mockResolvedValue(undefined); + setMutateAsync = vi.fn().mockResolvedValue(undefined); + + mocks.useUICustomization.mockReturnValue({ + cardPresentation: { poster_size: "large", caption: "title" }, + cardPresentationSource: "profile_client", + primaryMenu: defaultWebPrimaryMenu(), + primaryMenuSource: "profile_client", + shortcuts: { items: [] }, + isSupported: true, + supportsAtomicShortcuts: true, + isLoading: false, + isUnavailable: false, + }); + mocks.useUserLibraries.mockReturnValue({ data: [] }); + mocks.useSetSettingValue.mockReturnValue({ isPending: false, mutateAsync: setMutateAsync }); + mocks.useClearSettingValue.mockReturnValue({ + isPending: false, + mutateAsync: clearMutateAsync, + }); + }); + + afterEach(cleanup); + + it("resets the web-family card preference at profile_client scope", async () => { + render(); + + const reset = screen.getByRole("button", { name: "Reset web-family card layout" }); + expect(reset).toHaveAccessibleDescription( + "Remove the layout shared by web browsers and inherit the profile or app default.", + ); + fireEvent.click(reset); + + await waitFor(() => + expect(clearMutateAsync).toHaveBeenCalledWith({ + key: SETTING_KEYS.UI_CARD_PRESENTATION, + identity: { scope: "profile_client" }, + }), + ); + }); + + it("keeps the device reset distinct from the web-family reset", async () => { + mocks.useUICustomization.mockReturnValue({ + cardPresentation: { poster_size: "compact", caption: "artwork" }, + cardPresentationSource: "profile_device", + primaryMenu: defaultWebPrimaryMenu(), + primaryMenuSource: "profile_client", + shortcuts: { items: [] }, + isSupported: true, + supportsAtomicShortcuts: true, + isLoading: false, + }); + + render(); + + expect( + screen.queryByRole("button", { name: "Reset web-family card layout" }), + ).not.toBeInTheDocument(); + fireEvent.click(screen.getByRole("button", { name: "Use web-family layout" })); + + await waitFor(() => + expect(clearMutateAsync).toHaveBeenCalledWith({ + key: SETTING_KEYS.UI_CARD_PRESENTATION, + identity: { scope: "profile_device" }, + }), + ); + }); + + it("follows the effective menu after clearing a web-family override", async () => { + const effectiveMenu = { + items: [ + { type: "builtin" as const, destination: "home" as const }, + { type: "library" as const, library_id: 7, label: "Movies" }, + ], + }; + let source = "profile_client"; + mocks.useUICustomization.mockImplementation(() => ({ + cardPresentation: { poster_size: "large", caption: "title" }, + cardPresentationSource: "profile_client", + primaryMenu: effectiveMenu, + primaryMenuSource: source, + shortcuts: { items: [] }, + isSupported: true, + supportsAtomicShortcuts: true, + isLoading: false, + })); + clearMutateAsync.mockImplementation(async () => { + source = "profile"; + }); + + render(); + fireEvent.click(screen.getByRole("button", { name: "Remove Movies · Library" })); + expect(screen.queryByText("Movies · Library")).not.toBeInTheDocument(); + + fireEvent.click(screen.getByRole("button", { name: "Reset to default" })); + + await waitFor(() => + expect(clearMutateAsync).toHaveBeenCalledWith({ + key: SETTING_KEYS.NAV_PRIMARY_MENU, + identity: { scope: "profile_client" }, + }), + ); + expect(await screen.findByText("Movies · Library")).toBeInTheDocument(); + expect(screen.getByRole("button", { name: "Save menu" })).toBeDisabled(); + }); + + it("keeps a rapid post-save edit when the saved baseline refetch arrives", async () => { + let effectiveMenu = { + items: [ + { type: "builtin" as const, destination: "home" as const }, + { type: "library" as const, library_id: 7, label: "Movies" }, + { type: "builtin" as const, destination: "calendar" as const }, + ], + }; + mocks.useUICustomization.mockImplementation(() => ({ + cardPresentation: { poster_size: "large", caption: "title" }, + cardPresentationSource: "profile_client", + primaryMenu: effectiveMenu, + primaryMenuSource: "profile_client", + shortcuts: { items: [] }, + isSupported: true, + supportsAtomicShortcuts: true, + isLoading: false, + })); + + const view = render(); + fireEvent.click(screen.getByRole("button", { name: "Remove Movies · Library" })); + fireEvent.click(screen.getByRole("button", { name: "Save menu" })); + await waitFor(() => expect(setMutateAsync).toHaveBeenCalledTimes(1)); + await waitFor(() => expect(screen.getByRole("button", { name: "Save menu" })).toBeDisabled()); + + fireEvent.click(screen.getByRole("button", { name: "Remove Calendar" })); + effectiveMenu = { + items: [ + { type: "builtin" as const, destination: "home" as const }, + { type: "builtin" as const, destination: "calendar" as const }, + ], + }; + view.rerender(); + + expect(screen.queryByText("Calendar")).not.toBeInTheDocument(); + expect(screen.getByRole("button", { name: "Save menu" })).toBeEnabled(); + }); + + it("clears a just-saved override before its effective refetch arrives", async () => { + mocks.useUICustomization.mockReturnValue({ + cardPresentation: { poster_size: "large", caption: "title" }, + cardPresentationSource: "profile_client", + primaryMenu: { + items: [ + { type: "builtin" as const, destination: "home" as const }, + { type: "library" as const, library_id: 7, label: "Movies" }, + ], + }, + primaryMenuSource: "profile", + shortcuts: { items: [] }, + isSupported: true, + supportsAtomicShortcuts: true, + isLoading: false, + }); + + render(); + fireEvent.click(screen.getByRole("button", { name: "Remove Movies · Library" })); + fireEvent.click(screen.getByRole("button", { name: "Save menu" })); + await waitFor(() => expect(setMutateAsync).toHaveBeenCalledTimes(1)); + await waitFor(() => expect(screen.getByRole("button", { name: "Save menu" })).toBeDisabled()); + + fireEvent.click(screen.getByRole("button", { name: "Reset to default" })); + + await waitFor(() => + expect(clearMutateAsync).toHaveBeenCalledWith({ + key: SETTING_KEYS.NAV_PRIMARY_MENU, + identity: { scope: "profile_client" }, + }), + ); + }); + + it("discards an unsaved draft locally when no web-family override exists", async () => { + const effectiveMenu = { + items: [ + { type: "builtin" as const, destination: "home" as const }, + { type: "library" as const, library_id: 7, label: "Movies" }, + ], + }; + mocks.useUICustomization.mockReturnValue({ + cardPresentation: { poster_size: "large", caption: "title" }, + cardPresentationSource: "profile_client", + primaryMenu: effectiveMenu, + primaryMenuSource: "profile", + shortcuts: { items: [] }, + isSupported: true, + supportsAtomicShortcuts: true, + isLoading: false, + }); + + render(); + fireEvent.click(screen.getByRole("button", { name: "Remove Movies · Library" })); + fireEvent.click(screen.getByRole("button", { name: "Reset to default" })); + + expect(clearMutateAsync).not.toHaveBeenCalled(); + expect(await screen.findByText("Movies · Library")).toBeInTheDocument(); + expect(screen.getByRole("button", { name: "Save menu" })).toBeDisabled(); + }); + + it("hides revision-five controls when the server does not support them", () => { + mocks.useUICustomization.mockReturnValue({ + cardPresentation: { poster_size: "standard", caption: "title_metadata" }, + cardPresentationSource: "default", + primaryMenu: null, + primaryMenuSource: "default", + shortcuts: { items: [] }, + isSupported: false, + supportsAtomicShortcuts: false, + isLoading: false, + isUnavailable: false, + }); + + render(); + + expect(screen.getByRole("alert")).toHaveTextContent("Server upgrade required"); + expect(screen.queryByRole("radiogroup", { name: "Poster size" })).not.toBeInTheDocument(); + expect(screen.queryByRole("button", { name: "Save menu" })).not.toBeInTheDocument(); + }); + + it("keeps editors closed when effective customization could not be loaded", () => { + mocks.useUICustomization.mockReturnValue({ + cardPresentation: { poster_size: "standard", caption: "title_metadata" }, + cardPresentationSource: "default", + primaryMenu: null, + primaryMenuSource: "default", + shortcuts: { items: [] }, + isSupported: true, + supportsAtomicShortcuts: true, + isLoading: false, + isUnavailable: true, + }); + + render(); + + expect(screen.getByRole("alert")).toHaveTextContent("Customization unavailable"); + expect(screen.getByRole("alert")).toHaveTextContent("Editing stays disabled"); + expect(screen.queryByRole("radiogroup", { name: "Poster size" })).not.toBeInTheDocument(); + expect(screen.queryByRole("button", { name: "Save menu" })).not.toBeInTheDocument(); + expect(setMutateAsync).not.toHaveBeenCalled(); + expect(clearMutateAsync).not.toHaveBeenCalled(); + }); +}); diff --git a/web/src/pages/settings/InterfaceSettings.tsx b/web/src/pages/settings/InterfaceSettings.tsx new file mode 100644 index 00000000..1889d263 --- /dev/null +++ b/web/src/pages/settings/InterfaceSettings.tsx @@ -0,0 +1,575 @@ +import { useMemo, useState } from "react"; +import { ArrowDown, ArrowUp, Check, Monitor, RotateCcw, X } from "lucide-react"; +import { toast } from "sonner"; + +import { SettingsGroup } from "@/components/settings/SettingsGroup"; +import { Button } from "@/components/ui/button"; +import { + Select, + SelectContent, + SelectItem, + SelectTrigger, + SelectValue, +} from "@/components/ui/select"; +import { useUICustomization } from "@/hooks/useUICustomization"; +import { useUserLibraries } from "@/hooks/queries/libraries"; +import { useClearSettingValue, useSetSettingValue } from "@/hooks/queries/settingValues"; +import { SETTING_KEYS } from "@/lib/settingsContract"; +import { + CARD_PRESENTATION_PRESETS, + defaultWebPrimaryMenu, + menuItemKey, + moveMenuItem, + type CardCaption, + type CardPresentation, + type PosterSize, + type PrimaryMenuItem, +} from "@/lib/uiCustomization"; +import { cn } from "@/lib/utils"; + +const CLIENT_SCOPE = { scope: "profile_client" } as const; + +const BUILTIN_LABELS: Record = { + home: "Home", + movies: "Movies", + series: "TV Shows", + music: "Music", + audiobooks: "Audiobooks", + for_you: "For You", + calendar: "Calendar", +}; + +const ADDABLE_WEB_BUILTINS: PrimaryMenuItem[] = [ + // Media-family built-ins are global by contract. Until web has global + // routes for every family, users can add the explicit library destinations + // below without giving a global item first-library semantics. + { type: "builtin", destination: "for_you" }, + { type: "builtin", destination: "calendar" }, +]; + +function menuItemLabel(item: PrimaryMenuItem): string { + if (item.type === "builtin") return BUILTIN_LABELS[item.destination] ?? item.destination; + if (item.type === "library") return `${item.label} · Library`; + if (item.type === "section") return `${item.label} · Section`; + return `${item.label} · Collection`; +} + +function samePresentation(left: CardPresentation, right: CardPresentation) { + return left.poster_size === right.poster_size && left.caption === right.caption; +} + +function CardPreview({ presentation }: { presentation: CardPresentation }) { + const widths = + presentation.poster_size === "compact" + ? ["w-12", "w-12", "w-12", "w-12"] + : presentation.poster_size === "large" + ? ["w-20", "w-20"] + : ["w-16", "w-16", "w-16"]; + return ( +
    + {widths.map((width, index) => ( +
    +
    + {presentation.caption !== "artwork" ? ( + <> +
    + {presentation.caption === "title_metadata" ? ( +
    + ) : null} + + ) : null} +
    + ))} +
    + ); +} + +function InterfaceHeader() { + return ( +
    +

    Navigation & cards

    +

    + These choices sync between web browsers signed into this profile. TV, mobile, tablet, and + desktop-native apps keep their own matching-device layouts. +

    +
    + ); +} + +export default function InterfaceSettings() { + const { data: libraries = [] } = useUserLibraries(); + const customization = useUICustomization(); + const setValue = useSetSettingValue(); + const clearValue = useClearSettingValue(); + const baselineMenu = useMemo( + () => customization.primaryMenu ?? defaultWebPrimaryMenu(), + [customization.primaryMenu], + ); + const baselineKey = useMemo(() => JSON.stringify(baselineMenu.items), [baselineMenu.items]); + const [menuDraft, setMenuDraft] = useState<{ + baselineKey: string; + /** The just-saved baseline expected from the asynchronous query refresh. */ + pendingBaselineKey?: string; + items: PrimaryMenuItem[]; + dirty: boolean; + } | null>(null); + const [addItemKey, setAddItemKey] = useState(""); + const menuDraftMatchesBaseline = + menuDraft?.baselineKey === baselineKey || menuDraft?.pendingBaselineKey === baselineKey; + const menuItems = menuDraftMatchesBaseline ? menuDraft.items : baselineMenu.items; + const menuDirty = menuDraftMatchesBaseline === true && menuDraft.dirty; + const cardPresentation = customization.cardPresentation; + const cardDeviceOverride = customization.cardPresentationSource === "profile_device"; + const cardClientOverride = customization.cardPresentationSource === "profile_client"; + const menuDeviceOverride = customization.primaryMenuSource === "profile_device"; + const menuClientOverride = customization.primaryMenuSource === "profile_client"; + const menuMutationPending = setValue.isPending || clearValue.isPending; + const cardMutationPending = menuMutationPending || cardDeviceOverride; + const menuAtLimit = menuItems.length >= 64; + + const availableItems = useMemo(() => { + const current = new Set(menuItems.map(menuItemKey)); + const visibleLibraryIds = new Set(libraries.map((library) => library.id)); + const candidates: PrimaryMenuItem[] = [ + ...ADDABLE_WEB_BUILTINS, + ...libraries.map( + (library): PrimaryMenuItem => ({ + type: "library", + library_id: library.id, + label: library.name, + }), + ), + ...customization.shortcuts.items.filter( + (item) => item.library_id === undefined || visibleLibraryIds.has(item.library_id), + ), + ]; + const unique = new Map(); + for (const candidate of candidates) { + const key = menuItemKey(candidate); + if (!current.has(key)) unique.set(key, candidate); + } + return [...unique.values()]; + }, [customization.shortcuts.items, libraries, menuItems]); + const selectedAddItem = availableItems.find((item) => menuItemKey(item) === addItemKey); + + async function saveCardPresentation(next: CardPresentation) { + try { + await setValue.mutateAsync({ + key: SETTING_KEYS.UI_CARD_PRESENTATION, + value: next, + identity: CLIENT_SCOPE, + }); + } catch (error) { + toast.error(error instanceof Error ? error.message : "Could not save card layout"); + } + } + + async function resetCardPresentation() { + try { + await clearValue.mutateAsync({ + key: SETTING_KEYS.UI_CARD_PRESENTATION, + identity: CLIENT_SCOPE, + }); + toast.success("Web-family card layout reset"); + } catch (error) { + toast.error(error instanceof Error ? error.message : "Could not reset card layout"); + } + } + + async function clearDeviceOverride( + key: typeof SETTING_KEYS.UI_CARD_PRESENTATION | typeof SETTING_KEYS.NAV_PRIMARY_MENU, + label: string, + ) { + try { + await clearValue.mutateAsync({ key, identity: { scope: "profile_device" } }); + toast.success(`${label} now follows the web-family preference`); + } catch (error) { + toast.error( + error instanceof Error ? error.message : `Could not clear ${label.toLowerCase()}`, + ); + } + } + + function updateMenu(next: PrimaryMenuItem[]) { + setMenuDraft((current) => { + if (current?.pendingBaselineKey === baselineKey) { + return { baselineKey, items: next, dirty: true }; + } + if (current?.baselineKey === baselineKey) { + return { ...current, items: next, dirty: true }; + } + return { baselineKey, items: next, dirty: true }; + }); + } + + async function saveMenu() { + const savedItems = menuItems; + const savedBaselineKey = JSON.stringify(savedItems); + try { + await setValue.mutateAsync({ + key: SETTING_KEYS.NAV_PRIMARY_MENU, + value: { items: savedItems }, + identity: CLIENT_SCOPE, + }); + setMenuDraft((current) => { + const currentItemsKey = current ? JSON.stringify(current.items) : null; + if (current?.dirty && currentItemsKey !== savedBaselineKey) { + return { ...current, pendingBaselineKey: savedBaselineKey }; + } + return { + baselineKey, + pendingBaselineKey: savedBaselineKey, + items: savedItems, + dirty: false, + }; + }); + toast.success("Web navigation saved"); + } catch (error) { + toast.error(error instanceof Error ? error.message : "Could not save navigation"); + } + } + + async function resetMenu() { + const pendingClientOverride = menuDraft?.pendingBaselineKey !== undefined; + if (!menuClientOverride && !pendingClientOverride) { + setMenuDraft(null); + toast.success("Web navigation reset"); + return; + } + try { + await clearValue.mutateAsync({ + key: SETTING_KEYS.NAV_PRIMARY_MENU, + identity: CLIENT_SCOPE, + }); + setMenuDraft(null); + toast.success("Web navigation reset"); + } catch (error) { + toast.error(error instanceof Error ? error.message : "Could not reset navigation"); + } + } + + if (customization.isLoading) { + return ( +
    + +
    + Checking server support… +
    +
    + ); + } + + if (customization.isUnavailable) { + return ( +
    + +
    +

    Customization unavailable

    +

    + Saved navigation and card settings could not be loaded. Editing stays disabled to + protect your existing choices. +

    +
    +
    + ); + } + + if (!customization.isSupported) { + return ( +
    + +
    +

    Server upgrade required

    +

    + This server does not support synchronized navigation and card customization yet. +

    +
    +
    + ); + } + + return ( +
    + + + + {cardDeviceOverride ? ( +
    +

    + This browser has a higher-priority device override. Clear it before editing the + preference shared by web browsers. +

    + +
    + ) : null} +
    + {CARD_PRESENTATION_PRESETS.map((preset) => { + const active = samePresentation(cardPresentation, preset.value); + return ( + + ); + })} +
    +     + + +
    +
    +
    +

    Poster size

    +
    + {( + [ + ["compact", "Compact"], + ["standard", "Standard"], + ["large", "Large"], + ] as const + ).map(([value, label]) => ( + + ))} +
    +
    +
    +

    Caption

    +
    + {( + [ + ["title_metadata", "Title & metadata"], + ["title", "Title only"], + ["artwork", "Artwork only"], + ] as const + ).map(([value, label]) => ( + + ))} +
    +
    +
    + +
    + {cardClientOverride ? ( +
    +

    + Remove the layout shared by web browsers and inherit the profile or app default. +

    + +
    + ) : null} +     + + +
    +
    + + Web browsers +
    + {menuDeviceOverride ? ( +
    +

    + This browser has a higher-priority device menu. Clear it before editing the menu + shared by web browsers. +

    + +
    + ) : null} +
      + {menuItems.map((item, index) => { + const home = item.type === "builtin" && item.destination === "home"; + return ( +
    1. + + {index + 1} + + + {menuItemLabel(item)} + +
      + + + +
      +
      2. + ); + })} +
    + + {availableItems.length > 0 ? ( +
    + + +
    + ) : null} + +
    + + +
    +
    +     +
    + ); +}