From facad78d09bbeddf4645d52f2b5f3a5a41e097db Mon Sep 17 00:00:00 2001 From: Quick <31828688+Quick104@users.noreply.github.com> Date: Tue, 28 Jul 2026 13:47:10 +0000 Subject: [PATCH] feat(settings): serve admin user-settings through the canonical API MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the ten string-registry /admin/users/{id}/settings* and device-settings* routes with the canonical contract surface: one list of every explicit value the target user has stored across all scopes, and set/delete at an explicit scope named in the query string. The admin handlers live on SettingValuesHandler and share the session routes' implementation rather than duplicating it — the same key/scope parsing, identity validation, contract scope allowance, value normalization and mutation-receipt idempotency, factored into keyedScopeFromRequest/completeIdentity and setValueAt/deleteValueAt. The only admin-specific parts are the target user coming from the path, profile and device ids coming from the query (an admin holds no session claim to the user being inspected, so its named profile is checked to exist), and change events attributed to the target user so their clients refresh. The list is a new UserStore read, ListAllSettingValues, implemented in both backends and pinned by the shared storetest conformance suite: the admin surface wants the stored truth (which overrides exist, for a per-row reset affordance), which no resolution-shaped read answers. The ten removed routes are recorded in the pre-lock removals table in docs/architecture/v1-scope.md per the v1 API rules; the web admin device-overrides page moves onto the new surface in the Phase B rewrite inside this same unmerged PR. Co-Authored-By: Claude Fable 5 --- docs/architecture/v1-scope.md | 1 + internal/access/resolver_test.go | 3 + internal/api/handlers/admin.go | 362 ------------------ internal/api/handlers/settings_device_test.go | 95 ----- internal/api/handlers/settings_values.go | 91 ++++- .../api/handlers/settings_values_admin.go | 141 +++++++ .../handlers/settings_values_admin_test.go | 301 +++++++++++++++ internal/api/router.go | 21 +- internal/jellycompat/content_direct_test.go | 3 + internal/userdb/setting_values.go | 26 ++ internal/userdb/sqlitestore.go | 4 + internal/userstore/pgstore/setting_values.go | 29 ++ internal/userstore/store.go | 5 + internal/userstore/storetest/settingvalues.go | 53 +++ 14 files changed, 647 insertions(+), 488 deletions(-) create mode 100644 internal/api/handlers/settings_values_admin.go create mode 100644 internal/api/handlers/settings_values_admin_test.go diff --git a/docs/architecture/v1-scope.md b/docs/architecture/v1-scope.md index 04a2b2a3..3366b0d9 100644 --- a/docs/architecture/v1-scope.md +++ b/docs/architecture/v1-scope.md @@ -32,6 +32,7 @@ justification and falls back to the Deprecation/Sunset flow like anything else. | Removed | Release | Rationale | |---|---|---| | String `GET`/`PUT`/`DELETE /api/v1/settings…`, the unknown-key extension bag, preference fields on profile/library/series DTOs | Cross-platform settings contract, [design](../superpowers/specs/2026-07-10-cross-platform-user-settings-contract-design.md) | Replaced wholesale by the typed settings contract. Deferring past lock would mean carrying the Deprecation/Sunset surface *and* the untyped key bag — which lets any client invent a production setting the server stores unvalidated — through the deprecation window, which is the exact surface the contract exists to close. | +| The ten string-registry admin user-settings routes: `GET /api/v1/admin/users/{id}/settings`, `GET /api/v1/admin/users/{id}/settings/{key}`, `PUT /api/v1/admin/users/{id}/settings/{key}`, `DELETE /api/v1/admin/users/{id}/settings/{key}`, `GET /api/v1/admin/users/{id}/device-settings`, `GET /api/v1/admin/users/{id}/device-settings/{key}`, `DELETE /api/v1/admin/users/{id}/device-settings/{key}`, `PUT /api/v1/admin/users/{id}/profiles/{profile_id}/device-settings/{key}/{device_id}`, `DELETE /api/v1/admin/users/{id}/profiles/{profile_id}/device-settings/{key}/{device_id}`, `DELETE /api/v1/admin/users/{id}/profiles/{profile_id}/devices/{device_id}/settings` | Cross-platform settings contract, [design](../superpowers/specs/2026-07-10-cross-platform-user-settings-contract-design.md) | The admin projection of the removal above: these routes read and wrote the string registry the contract replaces. Their canonical successors are `GET /api/v1/admin/users/{id}/settings/values` (every stored value across all scopes) and `PUT`/`DELETE /api/v1/admin/users/{id}/settings/values/{key}` at an explicit scope, sharing the session routes' validation. Keeping the string routes past lock would preserve an admin-only write path into the untyped bag after the user-facing one closed. | Feature-detection precedent: clients discover which metadata providers (including the built-in NFO provider, #216) apply to a library type via diff --git a/internal/access/resolver_test.go b/internal/access/resolver_test.go index d7eec222..8827757a 100644 --- a/internal/access/resolver_test.go +++ b/internal/access/resolver_test.go @@ -290,6 +290,9 @@ func (s stubStore) GetSettingValue(context.Context, userstore.SettingIdentity) ( func (s stubStore) ListSettingValuesForResolution(context.Context, userstore.SettingResolutionQuery) ([]userstore.SettingValue, error) { return s.settingValues, nil } +func (s stubStore) ListAllSettingValues(context.Context) ([]userstore.SettingValue, error) { + panic("unused") +} func (s stubStore) UpsertSettingValue(context.Context, userstore.SettingIdentity, json.RawMessage) (*userstore.SettingValue, error) { panic("unused") } diff --git a/internal/api/handlers/admin.go b/internal/api/handlers/admin.go index 0f6ac304..23624f60 100644 --- a/internal/api/handlers/admin.go +++ b/internal/api/handlers/admin.go @@ -1355,10 +1355,6 @@ type adminSettingResponse struct { RestartRequired bool `json:"restart_required,omitempty"` } -type adminSettingsListResponse struct { - Settings []adminSettingResponse `json:"settings"` -} - type adminDeviceSettingResponse struct { UserID int `json:"user_id"` ProfileID string `json:"profile_id"` @@ -1413,345 +1409,6 @@ type adminDeviceDetailResponse struct { Settings []adminDeviceSettingResponse `json:"settings"` } -// HandleListUserSettings handles GET /admin/users/{id}/settings. -func (h *AdminHandler) HandleListUserSettings(w http.ResponseWriter, r *http.Request) { - userID, ok := parseAdminUserIDParam(w, r) - if !ok { - return - } - store, ok := h.adminUserStore(w, r, userID) - if !ok { - return - } - entries, err := store.ListSettings(r.Context()) - if err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", "Failed to list settings") - return - } - resp := adminSettingsListResponse{ - Settings: make([]adminSettingResponse, 0, len(entries)), - } - for _, entry := range entries { - if !keyUsesUserScope(entry.Key) { - continue - } - resp.Settings = append(resp.Settings, adminSettingResponse{ - Key: entry.Key, - Value: entry.Value, - }) - } - writeJSON(w, http.StatusOK, resp) -} - -// HandleGetUserSetting handles GET /admin/users/{id}/settings/{key}. -func (h *AdminHandler) HandleGetUserSetting(w http.ResponseWriter, r *http.Request) { - userID, ok := parseAdminUserIDParam(w, r) - if !ok { - return - } - key := strings.TrimSpace(chi.URLParam(r, "key")) - if key == "" { - writeError(w, http.StatusBadRequest, "bad_request", "Setting key is required") - return - } - if !keyUsesUserScope(key) { - writeError(w, http.StatusBadRequest, "bad_request", fmt.Sprintf("%s is not a %s setting", key, scopeUser)) - return - } - store, ok := h.adminUserStore(w, r, userID) - if !ok { - return - } - value, err := store.GetSetting(r.Context(), key) - if err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", "Failed to load setting") - return - } - if value == "" { - entries, err := store.ListSettings(r.Context()) - if err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", "Failed to load setting") - return - } - found := false - for _, entry := range entries { - if entry.Key == key { - found = true - break - } - } - if !found { - writeError(w, http.StatusNotFound, "not_found", "Setting not found") - return - } - } - writeJSON(w, http.StatusOK, adminSettingResponse{Key: key, Value: value}) -} - -// HandleUpdateUserSetting handles PUT /admin/users/{id}/settings/{key}. -func (h *AdminHandler) HandleUpdateUserSetting(w http.ResponseWriter, r *http.Request) { - userID, ok := parseAdminUserIDParam(w, r) - if !ok { - return - } - key := strings.TrimSpace(chi.URLParam(r, "key")) - if key == "" { - writeError(w, http.StatusBadRequest, "bad_request", "Setting key is required") - return - } - if !keyUsesUserScope(key) { - writeError(w, http.StatusBadRequest, "bad_request", fmt.Sprintf("%s is not a %s setting", key, scopeUser)) - return - } - var req updateSettingRequest - if err := json.NewDecoder(r.Body).Decode(&req); err != nil { - writeError(w, http.StatusBadRequest, "bad_request", "Invalid request body") - return - } - if err := validateRegisteredSetting(key, req.Value, scopeUser); err != nil { - writeError(w, http.StatusBadRequest, "bad_request", err.Error()) - return - } - store, ok := h.adminUserStore(w, r, userID) - if !ok { - return - } - if err := store.SetSetting(r.Context(), key, req.Value); err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", "Failed to update setting") - return - } - writeJSON(w, http.StatusOK, adminSettingResponse{Key: key, Value: req.Value}) -} - -// HandleDeleteUserSetting handles DELETE /admin/users/{id}/settings/{key}. -func (h *AdminHandler) HandleDeleteUserSetting(w http.ResponseWriter, r *http.Request) { - userID, ok := parseAdminUserIDParam(w, r) - if !ok { - return - } - key := strings.TrimSpace(chi.URLParam(r, "key")) - if key == "" { - writeError(w, http.StatusBadRequest, "bad_request", "Setting key is required") - return - } - if !keyUsesUserScope(key) { - writeError(w, http.StatusBadRequest, "bad_request", fmt.Sprintf("%s is not a %s setting", key, scopeUser)) - return - } - store, ok := h.adminUserStore(w, r, userID) - if !ok { - return - } - if err := store.DeleteSetting(r.Context(), key); err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", "Failed to delete setting") - return - } - w.WriteHeader(http.StatusNoContent) -} - -// HandleListUserDeviceSettings handles GET /admin/users/{id}/device-settings. -func (h *AdminHandler) HandleListUserDeviceSettings(w http.ResponseWriter, r *http.Request) { - userID, ok := parseAdminUserIDParam(w, r) - if !ok { - return - } - store, ok := h.adminUserStore(w, r, userID) - if !ok { - return - } - entries, err := store.ListAllDeviceSettings(r.Context()) - if err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", "Failed to list device settings") - return - } - profileNames, err := listProfileNamesByID(r.Context(), store) - if err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", "Failed to list profiles") - return - } - writeJSON(w, http.StatusOK, buildAdminDeviceSettingsResponse(userID, profileNames, entries)) -} - -// HandleListUserDeviceSettingsByKey handles GET /admin/users/{id}/device-settings/{key}. -func (h *AdminHandler) HandleListUserDeviceSettingsByKey(w http.ResponseWriter, r *http.Request) { - userID, ok := parseAdminUserIDParam(w, r) - if !ok { - return - } - key := strings.TrimSpace(chi.URLParam(r, "key")) - if key == "" { - writeError(w, http.StatusBadRequest, "bad_request", "Setting key is required") - return - } - store, ok := h.adminUserStore(w, r, userID) - if !ok { - return - } - entries, err := store.ListDeviceSettings(r.Context(), key) - if err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", "Failed to list device settings") - return - } - profileNames, err := listProfileNamesByID(r.Context(), store) - if err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", "Failed to list profiles") - return - } - writeJSON(w, http.StatusOK, buildAdminDeviceSettingsResponse(userID, profileNames, entries)) -} - -// HandleUpdateUserDeviceSetting handles PUT /admin/users/{id}/profiles/{profile_id}/device-settings/{key}/{device_id}. -func (h *AdminHandler) HandleUpdateUserDeviceSetting(w http.ResponseWriter, r *http.Request) { - userID, ok := parseAdminUserIDParam(w, r) - if !ok { - return - } - profileID := strings.TrimSpace(chi.URLParam(r, "profile_id")) - key := strings.TrimSpace(chi.URLParam(r, "key")) - deviceID := strings.TrimSpace(chi.URLParam(r, "device_id")) - if profileID == "" { - writeError(w, http.StatusBadRequest, "bad_request", "Profile id is required") - return - } - if key == "" { - writeError(w, http.StatusBadRequest, "bad_request", "Setting key is required") - return - } - if deviceID == "" { - writeError(w, http.StatusBadRequest, "bad_request", "Device id is required") - return - } - var req updateSettingRequest - if err := json.NewDecoder(r.Body).Decode(&req); err != nil { - writeError(w, http.StatusBadRequest, "bad_request", "Invalid request body") - return - } - if err := validateRegisteredSetting(key, req.Value, scopeDevice); err != nil { - writeError(w, http.StatusBadRequest, "bad_request", err.Error()) - return - } - store, ok := h.adminUserStore(w, r, userID) - if !ok { - return - } - if !adminProfileExists(w, r, store, profileID) { - return - } - existing, err := store.GetDeviceSetting(r.Context(), profileID, deviceID, key) - if err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", "Failed to load device setting") - return - } - entry := userstore.DeviceSettingEntry{ - ProfileID: profileID, - DeviceID: deviceID, - Key: key, - Value: req.Value, - } - if existing != nil { - entry.DeviceName = existing.DeviceName - entry.DevicePlatform = existing.DevicePlatform - } else if registered, err := registeredDeviceForProfile(r.Context(), store, profileID, deviceID); err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", "Failed to load device") - return - } else if registered != nil { - entry.DeviceName = registered.DeviceName - entry.DevicePlatform = registered.DevicePlatform - } - if err := store.SetDeviceSetting(r.Context(), entry); err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", "Failed to update device setting") - return - } - writeJSON(w, http.StatusOK, adminSettingResponse{Key: key, Value: req.Value}) -} - -// HandleDeleteUserDeviceSetting handles DELETE /admin/users/{id}/profiles/{profile_id}/device-settings/{key}/{device_id}. -func (h *AdminHandler) HandleDeleteUserDeviceSetting(w http.ResponseWriter, r *http.Request) { - userID, ok := parseAdminUserIDParam(w, r) - if !ok { - return - } - profileID := strings.TrimSpace(chi.URLParam(r, "profile_id")) - key := strings.TrimSpace(chi.URLParam(r, "key")) - deviceID := strings.TrimSpace(chi.URLParam(r, "device_id")) - if profileID == "" { - writeError(w, http.StatusBadRequest, "bad_request", "Profile id is required") - return - } - if key == "" { - writeError(w, http.StatusBadRequest, "bad_request", "Setting key is required") - return - } - if deviceID == "" { - writeError(w, http.StatusBadRequest, "bad_request", "Device id is required") - return - } - store, ok := h.adminUserStore(w, r, userID) - if !ok { - return - } - if !adminProfileExists(w, r, store, profileID) { - return - } - if err := store.DeleteDeviceSetting(r.Context(), profileID, deviceID, key); err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", "Failed to delete device setting") - return - } - w.WriteHeader(http.StatusNoContent) -} - -// HandleDeleteAllUserDeviceSettings handles DELETE /admin/users/{id}/profiles/{profile_id}/devices/{device_id}/settings. -func (h *AdminHandler) HandleDeleteAllUserDeviceSettings(w http.ResponseWriter, r *http.Request) { - userID, ok := parseAdminUserIDParam(w, r) - if !ok { - return - } - profileID := strings.TrimSpace(chi.URLParam(r, "profile_id")) - deviceID := strings.TrimSpace(chi.URLParam(r, "device_id")) - if profileID == "" { - writeError(w, http.StatusBadRequest, "bad_request", "Profile id is required") - return - } - if deviceID == "" { - writeError(w, http.StatusBadRequest, "bad_request", "Device id is required") - return - } - store, ok := h.adminUserStore(w, r, userID) - if !ok { - return - } - if !adminProfileExists(w, r, store, profileID) { - return - } - if err := store.DeleteAllDeviceSettings(r.Context(), profileID, deviceID); err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", "Failed to delete device settings") - return - } - w.WriteHeader(http.StatusNoContent) -} - -// HandleDeleteUserDeviceSettingsByKey handles DELETE /admin/users/{id}/device-settings/{key}. -func (h *AdminHandler) HandleDeleteUserDeviceSettingsByKey(w http.ResponseWriter, r *http.Request) { - userID, ok := parseAdminUserIDParam(w, r) - if !ok { - return - } - key := strings.TrimSpace(chi.URLParam(r, "key")) - if key == "" { - writeError(w, http.StatusBadRequest, "bad_request", "Setting key is required") - return - } - store, ok := h.adminUserStore(w, r, userID) - if !ok { - return - } - if err := store.DeleteDeviceSettingsByKey(r.Context(), key); err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", "Failed to delete device settings") - return - } - w.WriteHeader(http.StatusNoContent) -} - // HandleListDevices handles GET /admin/devices. func (h *AdminHandler) HandleListDevices(w http.ResponseWriter, r *http.Request) { if h.userRepo == nil || h.storeProv == nil { @@ -1922,25 +1579,6 @@ func listRegisteredDevices(ctx context.Context, store userstore.UserStore) ([]us return registry.ListDevices(ctx) } -func registeredDeviceForProfile( - ctx context.Context, - store userstore.UserStore, - profileID string, - deviceID string, -) (*userstore.DeviceEntry, error) { - devices, err := listRegisteredDevices(ctx, store) - if err != nil { - return nil, err - } - for _, device := range devices { - if device.ProfileID == profileID && device.DeviceID == deviceID { - matched := device - return &matched, nil - } - } - return nil, nil -} - func buildAdminDeviceSettingsResponse(userID int, profileNames map[string]string, entries []userstore.DeviceSettingEntry) adminDeviceSettingsListResponse { resp := adminDeviceSettingsListResponse{ Settings: make([]adminDeviceSettingResponse, 0, len(entries)), diff --git a/internal/api/handlers/settings_device_test.go b/internal/api/handlers/settings_device_test.go index f2846697..074ac406 100644 --- a/internal/api/handlers/settings_device_test.go +++ b/internal/api/handlers/settings_device_test.go @@ -691,53 +691,6 @@ func TestRememberLibraryPageStateIsDeviceScopedBoolSetting(t *testing.T) { } } -func TestAdminCanResetSubtitleAppearanceDeviceOverrides(t *testing.T) { - store := newProfileTestStore(t) - for _, deviceID := range []string{"apple-tv", "iphone"} { - if err := store.SetDeviceSetting(context.Background(), userstore.DeviceSettingEntry{ - ProfileID: "profile-1", - DeviceID: deviceID, - Key: subtitleAppearanceSettingKey, - Value: `{"fontSize":"small"}`, - }); err != nil { - t.Fatalf("SetDeviceSetting(%s): %v", deviceID, err) - } - } - handler := &AdminHandler{storeProv: testUserStoreProvider{store: store}} - - req := httptest.NewRequest(http.MethodDelete, "/admin/users/7/profiles/profile-1/device-settings/subtitle_appearance/apple-tv", nil) - req = withRouteParams(req, map[string]string{ - "id": "7", "profile_id": "profile-1", "key": subtitleAppearanceSettingKey, "device_id": "apple-tv", - }) - rec := httptest.NewRecorder() - handler.HandleDeleteUserDeviceSetting(rec, req) - if rec.Code != http.StatusNoContent { - t.Fatalf("delete one status = %d body=%s", rec.Code, rec.Body.String()) - } - remaining, err := store.ListDeviceSettings(context.Background(), subtitleAppearanceSettingKey) - if err != nil { - t.Fatalf("ListDeviceSettings: %v", err) - } - if len(remaining) != 1 || remaining[0].DeviceID != "iphone" { - t.Fatalf("remaining = %#v", remaining) - } - - req = httptest.NewRequest(http.MethodDelete, "/admin/users/7/device-settings/subtitle_appearance", nil) - req = withRouteParams(req, map[string]string{"id": "7", "key": subtitleAppearanceSettingKey}) - rec = httptest.NewRecorder() - handler.HandleDeleteUserDeviceSettingsByKey(rec, req) - if rec.Code != http.StatusNoContent { - t.Fatalf("delete all status = %d body=%s", rec.Code, rec.Body.String()) - } - remaining, err = store.ListDeviceSettings(context.Background(), subtitleAppearanceSettingKey) - if err != nil { - t.Fatalf("ListDeviceSettings after delete all: %v", err) - } - if len(remaining) != 0 { - t.Fatalf("remaining after delete all = %#v", remaining) - } -} - func TestEffectiveSettingsAreIsolatedPerProfileOnSameDevice(t *testing.T) { store := newProfileTestStore(t) if err := store.CreateProfile(context.Background(), userstore.Profile{ID: "profile-2", Name: "Guest"}); err != nil { @@ -911,54 +864,6 @@ func TestAdminCanListAndInspectDevicesAcrossUsers(t *testing.T) { } } -func TestAdminCanResetAllOverridesForOneDevice(t *testing.T) { - store := newProfileTestStore(t) - for _, entry := range []userstore.DeviceSettingEntry{ - {ProfileID: "profile-1", DeviceID: "living-room", Key: "player.playback_speed", Value: "1.25"}, - {ProfileID: "profile-1", DeviceID: "living-room", Key: "player.audio_sync_ms", Value: "120"}, - {ProfileID: "profile-1", DeviceID: "phone", Key: "player.hdr_enabled", Value: "false"}, - } { - if err := store.SetDeviceSetting(context.Background(), entry); err != nil { - t.Fatalf("SetDeviceSetting: %v", err) - } - } - - handler := &AdminHandler{storeProv: testUserStoreProvider{store: store}} - req := httptest.NewRequest(http.MethodDelete, "/admin/users/7/profiles/profile-1/devices/living-room/settings", nil) - req = withRouteParams(req, map[string]string{"id": "7", "profile_id": "profile-1", "device_id": "living-room"}) - rec := httptest.NewRecorder() - handler.HandleDeleteAllUserDeviceSettings(rec, req) - if rec.Code != http.StatusNoContent { - t.Fatalf("delete status = %d body=%s", rec.Code, rec.Body.String()) - } - - entries, err := store.ListAllDeviceSettings(context.Background()) - if err != nil { - t.Fatalf("ListAllDeviceSettings: %v", err) - } - if len(entries) != 1 || entries[0].DeviceID != "phone" { - t.Fatalf("entries after delete = %#v", entries) - } - registry, ok := store.(userstore.DeviceRegistry) - if !ok { - t.Fatalf("store does not support device registry") - } - devices, err := registry.ListDevices(context.Background()) - if err != nil { - t.Fatalf("ListDevices: %v", err) - } - foundLivingRoom := false - for _, device := range devices { - if device.ProfileID == "profile-1" && device.DeviceID == "living-room" { - foundLivingRoom = true - break - } - } - if !foundLivingRoom { - t.Fatalf("registry devices after delete = %#v", devices) - } -} - func withRouteParams(req *http.Request, params map[string]string) *http.Request { routeCtx := chi.NewRouteContext() for key, value := range params { diff --git a/internal/api/handlers/settings_values.go b/internal/api/handlers/settings_values.go index deb6e326..360f1a80 100644 --- a/internal/api/handlers/settings_values.go +++ b/internal/api/handlers/settings_values.go @@ -6,6 +6,7 @@ import ( "encoding/json" "errors" "net/http" + "net/url" "strconv" "strings" "time" @@ -189,6 +190,22 @@ func (h *SettingValuesHandler) HandleSetValue(w http.ResponseWriter, r *http.Req if !ok { return } + h.setValueAt(w, r, store, apimw.GetUserID(r.Context()), identity) +} + +// 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 +// account whose settings changed — the session owner on the self-service +// route, the target user on the admin route — so change events always reach +// the clients whose settings moved. +func (h *SettingValuesHandler) setValueAt( + w http.ResponseWriter, + r *http.Request, + store userstore.UserStore, + eventUserID int, + identity userstore.SettingIdentity, +) { def, ok := h.definitionFor(w, identity.Key) if !ok { return @@ -247,7 +264,7 @@ func (h *SettingValuesHandler) HandleSetValue(w http.ResponseWriter, r *http.Req return } publishUserSettingsEvent(r.Context(), h.EventsHub, - apimw.GetUserID(r.Context()), identity.ProfileID, identity.Key, string(identity.Scope)) + eventUserID, identity.ProfileID, identity.Key, string(identity.Scope)) writeJSON(w, http.StatusOK, settingValueToResponse(*stored)) } @@ -262,7 +279,18 @@ func (h *SettingValuesHandler) HandleDeleteValue(w http.ResponseWriter, r *http. if !ok { return } + h.deleteValueAt(w, r, store, apimw.GetUserID(r.Context()), identity) +} +// deleteValueAt is the unset path shared by the session and admin routes. See +// setValueAt for what eventUserID means. +func (h *SettingValuesHandler) deleteValueAt( + w http.ResponseWriter, + r *http.Request, + store userstore.UserStore, + eventUserID int, + identity userstore.SettingIdentity, +) { removed, err := store.DeleteSettingValue(r.Context(), identity) if err != nil { writeError(w, http.StatusInternalServerError, "internal_error", "Failed to clear the setting") @@ -273,7 +301,7 @@ func (h *SettingValuesHandler) HandleDeleteValue(w http.ResponseWriter, r *http. return } publishUserSettingsEvent(r.Context(), h.EventsHub, - apimw.GetUserID(r.Context()), identity.ProfileID, identity.Key, string(identity.Scope)) + eventUserID, identity.ProfileID, identity.Key, string(identity.Scope)) w.WriteHeader(http.StatusNoContent) } @@ -417,20 +445,8 @@ func (h *SettingValuesHandler) definitionFor(w http.ResponseWriter, key string) func (h *SettingValuesHandler) identityFromRequest( w http.ResponseWriter, r *http.Request, ) (userstore.SettingIdentity, bool) { - key := chi.URLParam(r, "key") - if strings.TrimSpace(key) == "" { - writeError(w, http.StatusBadRequest, "bad_request", "A setting key is required") - return userstore.SettingIdentity{}, false - } - if _, ok := h.definitionFor(w, key); !ok { - return userstore.SettingIdentity{}, false - } - - query := r.URL.Query() - 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") + key, scope, ok := h.keyedScopeFromRequest(w, r) + if !ok { return userstore.SettingIdentity{}, false } @@ -454,7 +470,40 @@ func (h *SettingValuesHandler) identityFromRequest( return userstore.SettingIdentity{}, false } } - if scope == settingscontract.ScopeProfileLibrary { + + return h.completeIdentity(w, r.URL.Query(), identity) +} + +// keyedScopeFromRequest parses the parts every scoped request names: a key +// that exists in the contract and is remote, plus an explicit scope. +func (h *SettingValuesHandler) keyedScopeFromRequest( + w http.ResponseWriter, r *http.Request, +) (string, settingscontract.Scope, bool) { + key := chi.URLParam(r, "key") + if strings.TrimSpace(key) == "" { + writeError(w, http.StatusBadRequest, "bad_request", "A setting key is required") + return "", "", false + } + if _, ok := h.definitionFor(w, key); !ok { + return "", "", false + } + + scope := settingscontract.Scope(strings.TrimSpace(r.URL.Query().Get("scope"))) + if scope == "" { + writeError(w, http.StatusBadRequest, "bad_request", + "A scope is required: account, profile, profile_device, profile_library or profile_series") + return "", "", false + } + return key, scope, true +} + +// completeIdentity fills the content-scope ids from the query, then runs the +// checks the session and admin routes share: the identity matches its scope's +// columns and the contract allows the key at that scope. +func (h *SettingValuesHandler) completeIdentity( + w http.ResponseWriter, query url.Values, identity userstore.SettingIdentity, +) (userstore.SettingIdentity, bool) { + if identity.Scope == settingscontract.ScopeProfileLibrary { libraryID, err := strconv.Atoi(strings.TrimSpace(query.Get("library_id"))) if err != nil || libraryID <= 0 { writeError(w, http.StatusBadRequest, "bad_request", @@ -463,7 +512,7 @@ func (h *SettingValuesHandler) identityFromRequest( } identity.LibraryID = libraryID } - if scope == settingscontract.ScopeProfileSeries { + if identity.Scope == settingscontract.ScopeProfileSeries { identity.SeriesID = strings.TrimSpace(query.Get("series_id")) if identity.SeriesID == "" { writeError(w, http.StatusBadRequest, "bad_request", @@ -479,10 +528,10 @@ func (h *SettingValuesHandler) identityFromRequest( // The contract decides where a setting may be written, independently of // whether the identity is well formed. - def, _ := h.contract.Lookup(key) - if !def.AllowsScope(scope) { + def, _ := h.contract.Lookup(identity.Key) + if !def.AllowsScope(identity.Scope) { writeError(w, http.StatusBadRequest, "scope_not_allowed", - key+" cannot be set at "+string(scope)) + identity.Key+" cannot be set at "+string(identity.Scope)) return userstore.SettingIdentity{}, false } diff --git a/internal/api/handlers/settings_values_admin.go b/internal/api/handlers/settings_values_admin.go new file mode 100644 index 00000000..32b67be9 --- /dev/null +++ b/internal/api/handlers/settings_values_admin.go @@ -0,0 +1,141 @@ +package handlers + +import ( + "net/http" + "strings" + + "github.com/Silo-Server/silo-server/internal/settingscontract" + "github.com/Silo-Server/silo-server/internal/userstore" +) + +// 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 +// same scopes, the same response shapes. The only differences are that the +// target user comes from the path instead of the session, and that profile and +// device ids come from the query string — an admin has no session claim to the +// user they are inspecting. +// +// Mounted behind requireActingAdmin next to the other /admin/users routes, so +// authorization is the router group's, not re-checked here. + +// HandleAdminListUserSettingValues handles +// GET /admin/users/{id}/settings/values: every explicit value the target user +// has stored, across all scopes. It deliberately lists stored rows rather than +// resolving: the admin surface answers "what overrides exist" (and offers a +// reset per row), which is the same question the session route's per-scope GET +// answers for one identity. +func (h *SettingValuesHandler) HandleAdminListUserSettingValues(w http.ResponseWriter, r *http.Request) { + store, _, ok := h.adminTargetStore(w, r) + if !ok { + return + } + + values, err := store.ListAllSettingValues(r.Context()) + if err != nil { + writeError(w, http.StatusInternalServerError, "internal_error", "Failed to list settings") + return + } + out := make([]settingValueResponse, 0, len(values)) + for _, value := range values { + out = append(out, settingValueToResponse(value)) + } + writeJSON(w, http.StatusOK, map[string]any{ + "values": out, + fieldRevision: h.contract.Revision, + }) +} + +// HandleAdminSetUserSettingValue handles +// PUT /admin/users/{id}/settings/values/{key}: write an explicit value at one +// scope on behalf of the target user, through the same validation and +// idempotency path as the session route. +func (h *SettingValuesHandler) HandleAdminSetUserSettingValue(w http.ResponseWriter, r *http.Request) { + store, userID, ok := h.adminTargetStore(w, r) + if !ok { + return + } + identity, ok := h.adminIdentityFromRequest(w, r) + if !ok { + return + } + // The session route's profile is validated by middleware; the admin names + // one in the query, so its existence is checked here. Postgres would refuse + // an orphan row on its profile FK anyway — checking first turns that 500 + // into a 404 and gives SQLite the same behavior. + if identity.ProfileID != "" && !adminProfileExists(w, r, store, identity.ProfileID) { + return + } + h.setValueAt(w, r, store, userID, identity) +} + +// HandleAdminDeleteUserSettingValue handles +// DELETE /admin/users/{id}/settings/values/{key}: remove the target user's +// explicit value at one scope so inheritance applies again. +func (h *SettingValuesHandler) HandleAdminDeleteUserSettingValue(w http.ResponseWriter, r *http.Request) { + store, userID, ok := h.adminTargetStore(w, r) + if !ok { + return + } + identity, ok := h.adminIdentityFromRequest(w, r) + if !ok { + return + } + h.deleteValueAt(w, r, store, userID, identity) +} + +// adminTargetStore resolves the {id} path parameter to the target user's +// store. +func (h *SettingValuesHandler) adminTargetStore( + w http.ResponseWriter, r *http.Request, +) (userstore.UserStore, int, bool) { + userID, ok := parseAdminUserIDParam(w, r) + if !ok { + return nil, 0, false + } + store, err := h.storeProvider.ForUser(r.Context(), userID) + if err != nil { + writeError(w, http.StatusInternalServerError, "internal_error", "Failed to access user store") + return nil, 0, false + } + if store == nil { + writeError(w, http.StatusNotFound, "not_found", "User store not found") + return nil, 0, false + } + return store, userID, true +} + +// adminIdentityFromRequest is identityFromRequest with the profile and device +// taken from the query string instead of the session: the admin is not the +// user being addressed, so there are no session headers to trust. Everything +// after that — content-scope ids, identity validation, the contract's scope +// allowance — is the shared completeIdentity path. +func (h *SettingValuesHandler) adminIdentityFromRequest( + w http.ResponseWriter, r *http.Request, +) (userstore.SettingIdentity, bool) { + key, scope, ok := h.keyedScopeFromRequest(w, r) + if !ok { + return userstore.SettingIdentity{}, false + } + + query := r.URL.Query() + identity := userstore.SettingIdentity{Key: key, Scope: scope} + if scope != settingscontract.ScopeAccount { + identity.ProfileID = strings.TrimSpace(query.Get("profile_id")) + if identity.ProfileID == "" { + writeError(w, http.StatusBadRequest, "bad_request", + "profile_id is required for this scope") + return userstore.SettingIdentity{}, false + } + } + if scope == settingscontract.ScopeProfileDevice { + identity.DeviceID = strings.TrimSpace(query.Get("device_id")) + if identity.DeviceID == "" { + writeError(w, http.StatusBadRequest, "bad_request", + "device_id is required for a device override") + return userstore.SettingIdentity{}, false + } + } + + return h.completeIdentity(w, query, identity) +} diff --git a/internal/api/handlers/settings_values_admin_test.go b/internal/api/handlers/settings_values_admin_test.go new file mode 100644 index 00000000..02643d5b --- /dev/null +++ b/internal/api/handlers/settings_values_admin_test.go @@ -0,0 +1,301 @@ +package handlers + +import ( + "bytes" + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + + "github.com/go-chi/chi/v5" + + apimw "github.com/Silo-Server/silo-server/internal/api/middleware" + "github.com/Silo-Server/silo-server/internal/auth" + "github.com/Silo-Server/silo-server/internal/settingscontract" + "github.com/Silo-Server/silo-server/internal/userstore" +) + +const ( + adminValuesAdminID = 1 + adminValuesTargetID = 7 +) + +// adminValuesEnv mounts the admin projection exactly as the router does: the +// canonical handler behind RequireActingAdmin, with the target user's store +// distinct from the admin's own so a route that resolved the wrong user is +// caught rather than masked by a shared store. +type adminValuesEnv struct { + router chi.Router + handler *SettingValuesHandler + adminStore userstore.UserStore + targetStore userstore.UserStore +} + +func newAdminValuesEnv(t *testing.T) adminValuesEnv { + t.Helper() + + adminStore := newIsolatedProfileTestStore(t, "admin") + targetStore := newIsolatedProfileTestStore(t, "target") + contract, err := settingscontract.Load() + if err != nil { + t.Fatalf("loading contract: %v", err) + } + handler := NewSettingValuesHandler(mappedTestUserStoreProvider{ + stores: map[int]userstore.UserStore{ + adminValuesAdminID: adminStore, + adminValuesTargetID: targetStore, + }, + }, contract) + + router := chi.NewRouter() + router.Group(func(r chi.Router) { + r.Use(apimw.RequireActingAdmin(nil)) + r.Get("/admin/users/{id}/settings/values", handler.HandleAdminListUserSettingValues) + r.Put("/admin/users/{id}/settings/values/{key}", handler.HandleAdminSetUserSettingValue) + r.Delete("/admin/users/{id}/settings/values/{key}", handler.HandleAdminDeleteUserSettingValue) + }) + return adminValuesEnv{router: router, handler: handler, adminStore: adminStore, targetStore: targetStore} +} + +// do sends a request through the mounted routes as a caller with the given +// role; an empty role sends no session at all. +func (env adminValuesEnv) do(t *testing.T, role, method, target string, body []byte) *httptest.ResponseRecorder { + t.Helper() + var req *http.Request + if body == nil { + req = httptest.NewRequest(method, target, nil) + } else { + req = httptest.NewRequest(method, target, bytes.NewReader(body)) + } + if role != "" { + req = req.WithContext(apimw.SetClaims(req.Context(), &auth.Claims{UserID: adminValuesAdminID, Role: role})) + } + rec := httptest.NewRecorder() + env.router.ServeHTTP(rec, req) + return rec +} + +func TestAdminSettingValuesRefuseNonAdmins(t *testing.T) { + env := newAdminValuesEnv(t) + + for name, req := range map[string]struct { + method, target string + body []byte + }{ + "list": {http.MethodGet, "/admin/users/7/settings/values", nil}, + "set": {http.MethodPut, "/admin/users/7/settings/values/playback.subtitle_mode?scope=account", []byte(`{"value":"always"}`)}, + "delete": {http.MethodDelete, "/admin/users/7/settings/values/playback.subtitle_mode?scope=account", nil}, + } { + t.Run(name, func(t *testing.T) { + if rec := env.do(t, "user", req.method, req.target, req.body); rec.Code != http.StatusForbidden { + t.Errorf("non-admin %s = %d, want 403: %s", name, rec.Code, rec.Body.String()) + } + if rec := env.do(t, "", req.method, req.target, req.body); rec.Code != http.StatusUnauthorized { + t.Errorf("anonymous %s = %d, want 401: %s", name, rec.Code, rec.Body.String()) + } + }) + } +} + +func TestAdminListShowsAnotherUsersValuesAcrossScopes(t *testing.T) { + env := newAdminValuesEnv(t) + ctx := context.Background() + + seeded := map[settingscontract.Scope]userstore.SettingIdentity{ + settingscontract.ScopeAccount: { + Key: "catalog.metadata_language", Scope: settingscontract.ScopeAccount, + }, + settingscontract.ScopeProfile: { + Key: "playback.subtitle_mode", Scope: settingscontract.ScopeProfile, ProfileID: "profile-1", + }, + settingscontract.ScopeProfileDevice: { + Key: "playback.subtitle_language", Scope: settingscontract.ScopeProfileDevice, + ProfileID: "profile-1", DeviceID: "tv-1", + }, + settingscontract.ScopeProfileLibrary: { + Key: "playback.subtitle_language", Scope: settingscontract.ScopeProfileLibrary, + ProfileID: "profile-1", LibraryID: 42, + }, + settingscontract.ScopeProfileSeries: { + Key: "playback.subtitle_language", Scope: settingscontract.ScopeProfileSeries, + ProfileID: "profile-1", SeriesID: "s-1", + }, + } + values := map[settingscontract.Scope]string{ + settingscontract.ScopeAccount: `"de"`, + settingscontract.ScopeProfile: `"always"`, + settingscontract.ScopeProfileDevice: `"en"`, + settingscontract.ScopeProfileLibrary: `"fr"`, + settingscontract.ScopeProfileSeries: `"ja"`, + } + for scope, id := range seeded { + if _, err := env.targetStore.UpsertSettingValue(ctx, id, json.RawMessage(values[scope])); err != nil { + t.Fatalf("seeding %s: %v", scope, err) + } + } + // A value in the admin's own store must not leak into the target's list. + if _, err := env.adminStore.UpsertSettingValue(ctx, userstore.SettingIdentity{ + Key: "playback.subtitle_mode", Scope: settingscontract.ScopeAccount, + }, json.RawMessage(`"off"`)); err != nil { + t.Fatalf("seeding admin store: %v", err) + } + + rec := env.do(t, "admin", http.MethodGet, "/admin/users/7/settings/values", nil) + if rec.Code != http.StatusOK { + t.Fatalf("list = %d: %s", rec.Code, rec.Body.String()) + } + var body struct { + Values []settingValueResponse `json:"values"` + Revision int `json:"revision"` + } + if err := json.Unmarshal(rec.Body.Bytes(), &body); err != nil { + t.Fatalf("decoding: %v", err) + } + if len(body.Values) != len(seeded) { + t.Fatalf("listed %d values, want %d: %s", len(body.Values), len(seeded), rec.Body.String()) + } + contract, _ := settingscontract.Load() + if body.Revision != contract.Revision { + t.Errorf("revision = %d, want %d", body.Revision, contract.Revision) + } + for _, got := range body.Values { + want, ok := seeded[settingscontract.Scope(got.Scope)] + if !ok { + t.Errorf("unexpected scope %q in list", got.Scope) + continue + } + if got.Key != want.Key || got.ProfileID != want.ProfileID || + 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) + } + if string(got.Value) != values[settingscontract.Scope(got.Scope)] { + t.Errorf("value at %s = %s, want %s", got.Scope, got.Value, values[settingscontract.Scope(got.Scope)]) + } + } + + // A user with no store is a 404, not an empty list pretending to be truth. + if rec := env.do(t, "admin", http.MethodGet, "/admin/users/99/settings/values", nil); rec.Code != http.StatusNotFound { + t.Errorf("list for unknown user = %d, want 404", rec.Code) + } +} + +func TestAdminSetAndDeleteAtExplicitScopeRoundTrip(t *testing.T) { + env := newAdminValuesEnv(t) + ctx := context.Background() + target := "/admin/users/7/settings/values/playback.subtitle_language" + + "?scope=profile_device&profile_id=profile-1&device_id=tv-1" + identity := userstore.SettingIdentity{ + Key: "playback.subtitle_language", Scope: settingscontract.ScopeProfileDevice, + ProfileID: "profile-1", DeviceID: "tv-1", + } + + rec := env.do(t, "admin", http.MethodPut, target, []byte(`{"value":"de"}`)) + 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 PUT response: %v", err) + } + if string(stored.Value) != `"de"` || stored.Scope != "profile_device" || + stored.ProfileID != "profile-1" || stored.DeviceID != "tv-1" { + t.Errorf("PUT stored %+v, want \"de\" at profile-1/tv-1", stored) + } + + // The write landed in the target user's store and only there. + if got, err := env.targetStore.GetSettingValue(ctx, identity); err != nil || got == nil { + t.Fatalf("target store value = %+v, %v; want stored", got, err) + } + if got, err := env.adminStore.GetSettingValue(ctx, identity); err != nil || got != nil { + t.Errorf("admin store value = %+v, %v; want none", got, err) + } + + if rec := env.do(t, "admin", http.MethodDelete, target, nil); rec.Code != http.StatusNoContent { + t.Fatalf("DELETE = %d: %s", rec.Code, rec.Body.String()) + } + if got, err := env.targetStore.GetSettingValue(ctx, identity); err != nil || got != nil { + t.Errorf("value after delete = %+v, %v; want gone", got, err) + } + if rec := env.do(t, "admin", http.MethodDelete, target, nil); rec.Code != http.StatusNotFound { + t.Errorf("second DELETE = %d, want 404", rec.Code) + } +} + +// 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. +func TestAdminSetRejectsInvalidValueLikeTheSessionRoute(t *testing.T) { + env := newAdminValuesEnv(t) + invalid := []byte(`{"value":"sideways"}`) + + adminRec := env.do(t, "admin", http.MethodPut, + "/admin/users/7/settings/values/playback.subtitle_mode?scope=profile&profile_id=profile-1", invalid) + if adminRec.Code != http.StatusBadRequest { + t.Fatalf("admin PUT = %d, want 400: %s", adminRec.Code, adminRec.Body.String()) + } + + // The same write through the session route, as the target user. + sessionReq := httptest.NewRequest(http.MethodPut, + "/settings/values/playback.subtitle_mode?scope=profile", bytes.NewReader(invalid)) + sessionCtx := apimw.SetClaims(sessionReq.Context(), &auth.Claims{UserID: adminValuesTargetID}) + sessionReq = sessionReq.WithContext(apimw.SetProfileID(sessionCtx, "profile-1")) + routeCtx := chi.NewRouteContext() + routeCtx.URLParams.Add("key", "playback.subtitle_mode") + sessionReq = sessionReq.WithContext(context.WithValue(sessionReq.Context(), chi.RouteCtxKey, routeCtx)) + sessionRec := httptest.NewRecorder() + env.handler.HandleSetValue(sessionRec, sessionReq) + + if sessionRec.Code != adminRec.Code { + t.Errorf("status: session %d, admin %d", sessionRec.Code, adminRec.Code) + } + var adminErr, sessionErr errorResponse + if err := json.Unmarshal(adminRec.Body.Bytes(), &adminErr); err != nil { + t.Fatalf("decoding admin error: %v", err) + } + if err := json.Unmarshal(sessionRec.Body.Bytes(), &sessionErr); err != nil { + t.Fatalf("decoding session error: %v", err) + } + if adminErr.Error != "invalid_value" { + t.Errorf("admin error code = %q, want invalid_value", adminErr.Error) + } + if adminErr != sessionErr { + t.Errorf("error bodies differ: admin %+v, session %+v", adminErr, sessionErr) + } +} + +func TestAdminSetRefusesUnknownKeysAndProfiles(t *testing.T) { + env := newAdminValuesEnv(t) + + rec := env.do(t, "admin", http.MethodPut, + "/admin/users/7/settings/values/totally.invented.key?scope=profile&profile_id=profile-1", + []byte(`{"value":"x"}`)) + if rec.Code != http.StatusNotFound { + t.Errorf("unknown key = %d, want 404: %s", rec.Code, rec.Body.String()) + } + var unknownErr errorResponse + if err := json.Unmarshal(rec.Body.Bytes(), &unknownErr); err != nil { + t.Fatalf("decoding unknown-key error: %v", err) + } + if unknownErr.Error != "unknown_setting" { + t.Errorf("unknown key code = %q, want unknown_setting", unknownErr.Error) + } + + // A client_local key is refused as server storage, same as the session route. + rec = env.do(t, "admin", http.MethodPut, + "/admin/users/7/settings/values/downloads.wifi_only?scope=profile&profile_id=profile-1", + []byte(`{"value":true}`)) + if rec.Code != http.StatusBadRequest { + t.Errorf("client_local key = %d, want 400: %s", rec.Code, rec.Body.String()) + } + + // A profile the target user does not have is a 404, which also keeps + // Postgres's profile FK from turning the typo into a 500. + rec = env.do(t, "admin", http.MethodPut, + "/admin/users/7/settings/values/playback.subtitle_mode?scope=profile&profile_id=ghost", + []byte(`{"value":"always"}`)) + if rec.Code != http.StatusNotFound { + t.Errorf("unknown profile = %d, want 404: %s", rec.Code, rec.Body.String()) + } +} diff --git a/internal/api/router.go b/internal/api/router.go index aa652185..13a91d5f 100644 --- a/internal/api/router.go +++ b/internal/api/router.go @@ -2563,16 +2563,17 @@ func NewRouter(deps Dependencies) chi.Router { r.Delete("/users/{id}", adminHandler.HandleDeleteUser) r.Post("/users/{id}/impersonate", adminHandler.HandleImpersonateUser) r.Get("/users/{id}/profiles", adminHandler.HandleListUserProfiles) - r.Get("/users/{id}/settings", adminHandler.HandleListUserSettings) - r.Get("/users/{id}/settings/{key}", adminHandler.HandleGetUserSetting) - r.Put("/users/{id}/settings/{key}", adminHandler.HandleUpdateUserSetting) - r.Delete("/users/{id}/settings/{key}", adminHandler.HandleDeleteUserSetting) - r.Get("/users/{id}/device-settings", adminHandler.HandleListUserDeviceSettings) - r.Get("/users/{id}/device-settings/{key}", adminHandler.HandleListUserDeviceSettingsByKey) - r.Put("/users/{id}/profiles/{profile_id}/device-settings/{key}/{device_id}", adminHandler.HandleUpdateUserDeviceSetting) - r.Delete("/users/{id}/device-settings/{key}", adminHandler.HandleDeleteUserDeviceSettingsByKey) - r.Delete("/users/{id}/profiles/{profile_id}/device-settings/{key}/{device_id}", adminHandler.HandleDeleteUserDeviceSetting) - r.Delete("/users/{id}/profiles/{profile_id}/devices/{device_id}/settings", adminHandler.HandleDeleteAllUserDeviceSettings) + // The canonical settings API's admin projection. It + // replaced the string-registry /users/{id}/settings* + // and device-settings* routes (see the pre-lock + // removals table in docs/architecture/v1-scope.md): + // one list across every scope, and set/delete at an + // explicit scope named in the query string. + if settingValuesHandler != nil { + r.Get("/users/{id}/settings/values", settingValuesHandler.HandleAdminListUserSettingValues) + r.Put("/users/{id}/settings/values/{key}", settingValuesHandler.HandleAdminSetUserSettingValue) + r.Delete("/users/{id}/settings/values/{key}", settingValuesHandler.HandleAdminDeleteUserSettingValue) + } r.Get("/devices", adminHandler.HandleListDevices) r.Get("/devices/{user_id}/{device_id}", adminHandler.HandleGetDevice) if accessGroupHandler != nil { diff --git a/internal/jellycompat/content_direct_test.go b/internal/jellycompat/content_direct_test.go index e3b830ae..9292538e 100644 --- a/internal/jellycompat/content_direct_test.go +++ b/internal/jellycompat/content_direct_test.go @@ -538,6 +538,9 @@ func (s *progressCountingStore) GetSettingValue(context.Context, userstore.Setti func (s *progressCountingStore) ListSettingValuesForResolution(context.Context, userstore.SettingResolutionQuery) ([]userstore.SettingValue, error) { panic("unused") } +func (s *progressCountingStore) ListAllSettingValues(context.Context) ([]userstore.SettingValue, error) { + panic("unused") +} func (s *progressCountingStore) UpsertSettingValue(context.Context, userstore.SettingIdentity, json.RawMessage) (*userstore.SettingValue, error) { panic("unused") } diff --git a/internal/userdb/setting_values.go b/internal/userdb/setting_values.go index 2751a0a5..cabe5f0f 100644 --- a/internal/userdb/setting_values.go +++ b/internal/userdb/setting_values.go @@ -139,6 +139,32 @@ func ListSettingValuesForResolution( return values, rows.Err() } +// ListAllSettingValues returns every stored explicit value across all scopes, +// ordered by (key, scope, identity) so repeated reads page through the same +// sequence. It backs the admin inspection surface, which wants the stored +// truth rather than a resolution. +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, ''), + COALESCE(library_id, 0), COALESCE(series_id, '')`) + if err != nil { + return nil, fmt.Errorf("listing all setting values: %w", err) + } + defer func() { _ = rows.Close() }() + + var values []userstore.SettingValue + for rows.Next() { + value, err := scanSettingValue(rows) + if err != nil { + return nil, fmt.Errorf("scanning setting value: %w", err) + } + values = append(values, value) + } + return values, rows.Err() +} + // UpsertSettingValue writes the explicit value at one scope and increments that // row's revision. func UpsertSettingValue( diff --git a/internal/userdb/sqlitestore.go b/internal/userdb/sqlitestore.go index 28a31977..7fc45e9e 100644 --- a/internal/userdb/sqlitestore.go +++ b/internal/userdb/sqlitestore.go @@ -440,6 +440,10 @@ func (s *SQLiteUserStore) ListSettingValuesForResolution(_ context.Context, quer return ListSettingValuesForResolution(s.db, query) } +func (s *SQLiteUserStore) ListAllSettingValues(_ context.Context) ([]userstore.SettingValue, error) { + return ListAllSettingValues(s.db) +} + func (s *SQLiteUserStore) UpsertSettingValue(_ context.Context, id userstore.SettingIdentity, value json.RawMessage) (*userstore.SettingValue, error) { return UpsertSettingValue(s.db, id, value) } diff --git a/internal/userstore/pgstore/setting_values.go b/internal/userstore/pgstore/setting_values.go index c40d60c8..477cdf1d 100644 --- a/internal/userstore/pgstore/setting_values.go +++ b/internal/userstore/pgstore/setting_values.go @@ -124,6 +124,35 @@ func (s *PostgresUserStore) ListSettingValuesForResolution( return values, rows.Err() } +// ListAllSettingValues returns every stored explicit value across all scopes, +// ordered by (key, scope, identity) so repeated reads page through the same +// sequence. It backs the admin inspection surface, which wants the stored +// truth rather than a resolution. +func (s *PostgresUserStore) ListAllSettingValues(ctx context.Context) ([]userstore.SettingValue, error) { + rows, err := s.pool.Query(ctx, ` + SELECT `+settingValueColumns+` + FROM user_setting_values + WHERE user_id = $1 + ORDER BY key, scope, COALESCE(profile_id, ''), COALESCE(device_id, ''), + COALESCE(library_id, 0), COALESCE(series_id, '')`, + s.userID, + ) + if err != nil { + return nil, fmt.Errorf("listing all setting values: %w", err) + } + defer rows.Close() + + var values []userstore.SettingValue + for rows.Next() { + value, err := scanSettingValue(rows) + if err != nil { + return nil, fmt.Errorf("scanning setting value: %w", err) + } + values = append(values, value) + } + return values, rows.Err() +} + func (s *PostgresUserStore) UpsertSettingValue( ctx context.Context, id userstore.SettingIdentity, diff --git a/internal/userstore/store.go b/internal/userstore/store.go index bc97b129..4e6cef1f 100644 --- a/internal/userstore/store.go +++ b/internal/userstore/store.go @@ -154,6 +154,11 @@ type UserStore interface { // definition's resolution order in Go; issuing one lookup per scope is a // rejected implementation. ListSettingValuesForResolution(ctx context.Context, query SettingResolutionQuery) ([]SettingValue, error) + // ListAllSettingValues returns every explicit value this user has stored, + // across all scopes, in a stable (key, scope, identity) order. It serves + // the admin inspection surface; resolution reads keep going through + // ListSettingValuesForResolution. + ListAllSettingValues(ctx context.Context) ([]SettingValue, error) // UpsertSettingValue writes the explicit value at one scope and increments // that row's revision. Concurrent writes to one identity are // last-write-wins in server receipt order; there is no compare-and-set diff --git a/internal/userstore/storetest/settingvalues.go b/internal/userstore/storetest/settingvalues.go index 5d29cf49..acd5e5cf 100644 --- a/internal/userstore/storetest/settingvalues.go +++ b/internal/userstore/storetest/settingvalues.go @@ -37,6 +37,9 @@ func RunSettingValues(t *testing.T, newStore func(t *testing.T) userstore.UserSt t.Run("ResolutionCandidates", func(t *testing.T) { testSettingValueResolution(t, newStore) }) + t.Run("ListAll", func(t *testing.T) { + testSettingValueListAll(t, newStore) + }) t.Run("DeletePaths", func(t *testing.T) { testSettingValueDeletePaths(t, newStore) }) @@ -194,6 +197,56 @@ func testSettingValueScopes(t *testing.T, newStore func(t *testing.T) userstore. } } +// testSettingValueListAll pins the admin inspection read: every stored value +// comes back regardless of scope, with its identity intact, and nothing is +// invented for identities that were never written. +func testSettingValueListAll(t *testing.T, newStore func(t *testing.T) userstore.UserStore) { + ctx := context.Background() + store := newStore(t) + + empty, err := store.ListAllSettingValues(ctx) + if err != nil { + t.Fatalf("ListAllSettingValues(empty): %v", err) + } + if len(empty) != 0 { + t.Fatalf("ListAllSettingValues(empty) = %+v, want none", empty) + } + + 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"`, + } + for id, value := range seeded { + mustUpsert(t, ctx, store, id, value) + } + + all, err := store.ListAllSettingValues(ctx) + if err != nil { + t.Fatalf("ListAllSettingValues: %v", err) + } + if len(all) != len(seeded) { + t.Fatalf("ListAllSettingValues returned %d rows, want %d: %+v", len(all), len(seeded), all) + } + for _, got := range all { + want, ok := seeded[got.SettingIdentity] + if !ok { + t.Fatalf("ListAllSettingValues invented identity %+v", got.SettingIdentity) + } + if !jsonEqual(got.Value, json.RawMessage(want)) { + t.Fatalf("ListAllSettingValues(%+v) = %s, want %s", got.SettingIdentity, got.Value, want) + } + if got.Revision != 1 || got.UpdatedAt == "" { + t.Fatalf("ListAllSettingValues(%+v) revision/updated_at = %d/%q, want 1/set", + got.SettingIdentity, got.Revision, got.UpdatedAt) + } + } +} + // testSettingValueUnsetIsNotFalsy pins the distinction the whole contract rests // on: false, 0, "" and JSON null are stored values, and only deleting the row // makes a setting unset.