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.