From 8be5c56f6f6c095f5f6fd9fdb92f450846fd52c7 Mon Sep 17 00:00:00 2001 From: dev_agent Date: Sat, 3 Oct 2026 11:50:15 -0400 Subject: [PATCH] Feature 04 task 3 (rebased): Household settings seeding and API (#27) Co-authored-by: dev_agent Co-committed-by: dev_agent --- README.md | 10 +- cmd/server/boot_test.go | 128 ++++++ cmd/server/main.go | 19 +- internal/config/config.go | 16 + internal/config/config_test.go | 15 + internal/db/models.go | 1 + internal/db/settings.sql.go | 13 +- internal/household/settings.go | 340 ++++++++++++++++ internal/household/settings_test.go | 374 ++++++++++++++++++ internal/household/zone.go | 28 ++ internal/server/csrf_test.go | 5 +- internal/server/login_test.go | 2 +- internal/server/origin_test.go | 2 +- internal/server/server.go | 27 +- internal/server/server_test.go | 44 ++- internal/server/settings.go | 135 +++++++ internal/server/settings_test.go | 278 +++++++++++++ internal/store/migrate_test.go | 11 +- .../0002_household_settings_updated_by.sql | 4 + migrations/queries/settings.sql | 5 +- 20 files changed, 1423 insertions(+), 34 deletions(-) create mode 100644 internal/household/settings.go create mode 100644 internal/household/settings_test.go create mode 100644 internal/household/zone.go create mode 100644 internal/server/settings.go create mode 100644 internal/server/settings_test.go create mode 100644 migrations/0002_household_settings_updated_by.sql diff --git a/README.md b/README.md index d374368..f9f43e2 100644 --- a/README.md +++ b/README.md @@ -44,6 +44,7 @@ export MEALQUEST_OIDC_ALLOWED_GROUPS=mealquest-admin # X-Forwarded-For/-Proto headers are honoured (production: Traefik). Unset # trusts no peer, which is right for local development. # export MEALQUEST_TRUSTED_PROXY=10.60.1.33 +export MEALQUEST_DEFAULT_TZ=America/New_York make dev ``` @@ -51,11 +52,16 @@ make dev (which proxies `/api`, `/auth` and `/media` to `:8080`). Open http://localhost:5173. Configuration is environment-only and fails fast: every variable above is required except the commented-out session -lifetimes (when set they must be positive, with MAX at least IDLE) and -trusted proxy list (every entry must be an IP or CIDR), and +lifetimes (when set they must be positive, with MAX at least IDLE), +trusted proxy list (every entry must be an IP or CIDR) and +`MEALQUEST_DEFAULT_TZ`, and secrets are read only from files named by `*_FILE` variables. `MEALQUEST_PUBLIC_URL`, `MEALQUEST_OIDC_ISSUER` and the endpoints the issuer advertises must be `https://`; plain `http://` is accepted only for `localhost`, `127.0.0.1` and `[::1]`. +`MEALQUEST_DEFAULT_TZ` is the IANA zone the household settings are created +with on first boot: until those settings exist, boot fails unless it is a +valid IANA zone. Later boots ignore it, set or not (change the zone in +household settings instead). ## Common commands diff --git a/cmd/server/boot_test.go b/cmd/server/boot_test.go index 157d39f..37dc639 100644 --- a/cmd/server/boot_test.go +++ b/cmd/server/boot_test.go @@ -16,6 +16,7 @@ import ( "time" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/config" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/household" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/secretbox" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/store" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/migrations" @@ -52,6 +53,11 @@ func loadConfig(t *testing.T, keyFile string) config.Config { t.Setenv(config.EnvOIDCClientID, "mealquest") t.Setenv(config.EnvOIDCClientSecretFile, clientSecretPath) t.Setenv(config.EnvOIDCAllowedGroups, "mealquest-admin") + // MEALQUEST_DEFAULT_TZ is left unset; t.Setenv restores any outer value. + t.Setenv(config.EnvDefaultTZ, "") + if err := os.Unsetenv(config.EnvDefaultTZ); err != nil { + t.Fatal(err) + } cfg, err := config.Load() if err != nil { t.Fatal(err) @@ -202,3 +208,125 @@ func TestLoadMissingSecretKeyFileStopsBoot(t *testing.T) { t.Fatalf("Load error = %v, want a config error naming %s", err, config.EnvSecretKeyFile) } } + +func TestRunWithoutValidDefaultZoneOnFirstBootStops(t *testing.T) { + tests := []struct { + name string + defaultTZ *string // nil leaves MEALQUEST_DEFAULT_TZ unset + want error + }{ + {"unset", nil, household.ErrNoDefaultZone}, + {"not an IANA zone", ptr("Mars/Olympus_Mons"), household.ErrInvalidDefaultZone}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + loadConfig(t, validKeyFile(t)) + if tt.defaultTZ != nil { + t.Setenv(config.EnvDefaultTZ, *tt.defaultTZ) + } + cfg, err := config.Load() + if err != nil { + t.Fatal(err) + } + var logs bytes.Buffer + errc := make(chan error, 1) + go func() { errc <- run(cfg, slog.New(slog.NewJSONHandler(&logs, nil))) }() + + select { + case err = <-errc: + case <-time.After(10 * time.Second): + t.Fatal("run kept going with no household settings and no valid default zone") + } + if !errors.Is(err, tt.want) || !strings.Contains(err.Error(), config.EnvDefaultTZ) { + t.Fatalf("run error = %v, want %v naming %s", err, tt.want, config.EnvDefaultTZ) + } + if strings.Contains(logs.String(), `"listening"`) { + t.Errorf("run started listening without household settings: %s", logs.String()) + } + }) + } +} + +// A reboot over existing household settings must not depend on +// MEALQUEST_DEFAULT_TZ: config.Load accepts an invalid value and seeding +// ignores it, keeping the stored zone. +func TestRebootWithSettingsIgnoresInvalidDefaultZone(t *testing.T) { + ctx := context.Background() + cfg := loadConfig(t, validKeyFile(t)) + + db, err := openStore(ctx, cfg.DataDir) + if err != nil { + t.Fatal(err) + } + if err := seedSettings(ctx, db, "America/Denver"); err != nil { + t.Fatal(err) + } + if err := db.Close(); err != nil { + t.Fatal(err) + } + + t.Setenv(config.EnvDefaultTZ, "Mars/Olympus_Mons") + cfg, err = config.Load() + if err != nil { + t.Fatalf("Load with an invalid %s and existing settings: %v", config.EnvDefaultTZ, err) + } + db, err = openStore(ctx, cfg.DataDir) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { + if err := db.Close(); err != nil { + t.Error(err) + } + }) + if err := seedSettings(ctx, db, cfg.DefaultTZ); err != nil { + t.Fatalf("reboot with %s=%q: %v", config.EnvDefaultTZ, cfg.DefaultTZ, err) + } + got, err := household.NewSettingsService(db).Get(ctx) + if err != nil { + t.Fatal(err) + } + if got.Timezone != "America/Denver" { + t.Errorf("timezone after reboot = %q, want America/Denver", got.Timezone) + } +} + +func ptr[T any](v T) *T { return &v } + +func TestSeedSettingsAtBoot(t *testing.T) { + ctx := context.Background() + dir := t.TempDir() + + // First boot seeds from the default zone. + db, err := openStore(ctx, dir) + if err != nil { + t.Fatal(err) + } + if err := seedSettings(ctx, db, "America/Denver"); err != nil { + t.Fatal(err) + } + if err := db.Close(); err != nil { + t.Fatal(err) + } + + // A later boot needs no default zone and keeps the stored one. + db, err = openStore(ctx, dir) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { + if err := db.Close(); err != nil { + t.Error(err) + } + }) + if err := seedSettings(ctx, db, ""); err != nil { + t.Fatalf("reboot without %s: %v", config.EnvDefaultTZ, err) + } + got, err := household.NewSettingsService(db).Get(ctx) + if err != nil { + t.Fatal(err) + } + if got.Timezone != "America/Denver" { + t.Errorf("timezone after reboot = %q, want America/Denver", got.Timezone) + } +} diff --git a/cmd/server/main.go b/cmd/server/main.go index 666cce7..e6feacc 100644 --- a/cmd/server/main.go +++ b/cmd/server/main.go @@ -17,6 +17,7 @@ import ( "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/config" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/forwarded" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/frontend" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/household" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/secretbox" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/server" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/store" @@ -58,6 +59,11 @@ func run(cfg config.Config, logger *slog.Logger) error { } }() + if err := seedSettings(ctx, db, cfg.DefaultTZ); err != nil { + return err + } + settings := household.NewSettingsService(db) + sessions, err := auth.NewSessionManager(db, cfg.SessionIdle, cfg.SessionMax) if err != nil { return err @@ -74,7 +80,7 @@ func run(cfg config.Config, logger *slog.Logger) error { return err } - router, err := server.NewRouter(logger, version, cfg.TrustedProxy, authFlow, sessions, frontend.Assets()) + router, err := server.NewRouter(logger, version, cfg.TrustedProxy, authFlow, sessions, settings, frontend.Assets()) if err != nil { return err } @@ -136,6 +142,17 @@ func openStore(ctx context.Context, dataDir string) (*store.Store, error) { return db, nil } +// seedSettings creates the household settings on first boot, naming the +// variable that supplies the zone when it is missing or invalid. Once the +// settings exist the variable is ignored. +func seedSettings(ctx context.Context, db *store.Store, defaultTZ string) error { + err := household.SeedSettings(ctx, db, defaultTZ) + if errors.Is(err, household.ErrNoDefaultZone) || errors.Is(err, household.ErrInvalidDefaultZone) { + return fmt.Errorf("%s: a valid IANA zone is required on first boot to create the household settings: %w", config.EnvDefaultTZ, err) + } + return err +} + // logListening logs the startup line. Only the public URL's origin is logged // so no credential-bearing URL component can reach the log. func logListening(logger *slog.Logger, cfg config.Config) { diff --git a/internal/config/config.go b/internal/config/config.go index d3ab5e2..1c1dfd7 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -33,6 +33,8 @@ const ( EnvSessionMax = "MEALQUEST_SESSION_MAX" EnvTrustedProxy = "MEALQUEST_TRUSTED_PROXY" + + EnvDefaultTZ = "MEALQUEST_DEFAULT_TZ" ) // Session lifetime defaults (D-004): long enough that the kitchen tablet @@ -65,6 +67,12 @@ type Config struct { // TrustedProxy lists the reverse proxies whose X-Forwarded-For and // X-Forwarded-Proto headers are honoured. Empty trusts no peer. TrustedProxy forwarded.Trusted + + // DefaultTZ is the IANA zone the household settings are created with on + // first boot. It is optional and not validated here: boot requires and + // validates it only while the settings do not exist yet, and ignores it + // once they do. Empty when unset. + DefaultTZ string } // PublicOrigin returns the scheme and host of PublicURL, the only parts of it @@ -97,6 +105,8 @@ func load(lookup func(string) (string, bool)) (Config, error) { SessionMax: r.sessionDuration(EnvSessionMax, DefaultSessionMax), TrustedProxy: r.trustedProxy(EnvTrustedProxy), + + DefaultTZ: r.optional(EnvDefaultTZ), } if cfg.SessionIdle > 0 && cfg.SessionMax > 0 && cfg.SessionMax < cfg.SessionIdle { r.fail(EnvSessionMax, "must be at least %s (%s)", EnvSessionIdle, cfg.SessionIdle) @@ -323,6 +333,12 @@ func (r *reader) parseDuration(name, v string) time.Duration { return d } +// optional returns the variable's value, or "" when it is unset. +func (r *reader) optional(name string) string { + v, _ := r.lookup(name) + return v +} + // secretFile reads a secret from the file named by name, which must end in // _FILE. Setting the secret inline (name without _FILE) is rejected. func (r *reader) secretFile(name string) Secret { diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 8816bf7..937474d 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -68,6 +68,16 @@ func TestLoad(t *testing.T) { name: "all required vars present", env: valid(), }, + { + name: "default time zone set", + env: with(map[string]string{EnvDefaultTZ: "America/New_York"}), + }, + { + // Validated only when seeding the household settings, so a + // stale value cannot stop a boot that no longer uses it. + name: "default time zone not an IANA zone", + env: with(map[string]string{EnvDefaultTZ: "Mars/Olympus_Mons"}), + }, { name: "one missing", env: with(nil, EnvHTTPAddr), @@ -410,6 +420,8 @@ func TestLoadValues(t *testing.T) { EnvOIDCClientID: "mealquest", EnvOIDCClientSecretFile: clientSecretPath, EnvOIDCAllowedGroups: " mealquest-admin ,family", + + EnvDefaultTZ: "Europe/London", } cfg, err := load(func(k string) (string, bool) { v, ok := env[k]; return v, ok }) if err != nil { @@ -465,6 +477,9 @@ func TestLoadValues(t *testing.T) { if !slices.Equal(cfg.TrustedProxy, want) { t.Errorf("TrustedProxy = %v, want %v", cfg.TrustedProxy, want) } + if cfg.DefaultTZ != "Europe/London" { + t.Errorf("DefaultTZ = %q, want Europe/London", cfg.DefaultTZ) + } } func TestSecretRedacted(t *testing.T) { diff --git a/internal/db/models.go b/internal/db/models.go index 78206b0..a2f8121 100644 --- a/internal/db/models.go +++ b/internal/db/models.go @@ -85,6 +85,7 @@ type HouseholdSetting struct { UpdatesSeenAt sql.NullString CreatedAt string UpdatedAt string + UpdatedBy sql.NullString } type IntegrationsGoogle struct { diff --git a/internal/db/settings.sql.go b/internal/db/settings.sql.go index 5bdc156..10c89fd 100644 --- a/internal/db/settings.sql.go +++ b/internal/db/settings.sql.go @@ -57,7 +57,7 @@ func (q *Queries) GetChatSettings(ctx context.Context) (ChatSetting, error) { } const getHouseholdSettings = `-- name: GetHouseholdSettings :one -SELECT id, timezone, dinner_time, reminder_time, review_time, plan_draft_dow, plan_draft_time, suggestions_dow, suggestions_time, chat_autosend_dow, chat_autosend_time, onboarding_completed_at, updates_seen_at, created_at, updated_at FROM household_settings +SELECT id, timezone, dinner_time, reminder_time, review_time, plan_draft_dow, plan_draft_time, suggestions_dow, suggestions_time, chat_autosend_dow, chat_autosend_time, onboarding_completed_at, updates_seen_at, created_at, updated_at, updated_by FROM household_settings WHERE id = 1 ` @@ -81,6 +81,7 @@ func (q *Queries) GetHouseholdSettings(ctx context.Context) (HouseholdSetting, e &i.UpdatesSeenAt, &i.CreatedAt, &i.UpdatedAt, + &i.UpdatedBy, ) return i, err } @@ -248,12 +249,12 @@ INSERT INTO household_settings ( id, timezone, dinner_time, reminder_time, review_time, plan_draft_dow, plan_draft_time, suggestions_dow, suggestions_time, chat_autosend_dow, chat_autosend_time, onboarding_completed_at, updates_seen_at, - created_at, updated_at + updated_by, created_at, updated_at ) VALUES ( 1, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, - ?, ? + ?, ?, ? ) ON CONFLICT (id) DO UPDATE SET timezone = excluded.timezone, @@ -268,8 +269,9 @@ ON CONFLICT (id) DO UPDATE SET chat_autosend_time = excluded.chat_autosend_time, onboarding_completed_at = excluded.onboarding_completed_at, updates_seen_at = excluded.updates_seen_at, + updated_by = excluded.updated_by, updated_at = excluded.updated_at -RETURNING id, timezone, dinner_time, reminder_time, review_time, plan_draft_dow, plan_draft_time, suggestions_dow, suggestions_time, chat_autosend_dow, chat_autosend_time, onboarding_completed_at, updates_seen_at, created_at, updated_at +RETURNING id, timezone, dinner_time, reminder_time, review_time, plan_draft_dow, plan_draft_time, suggestions_dow, suggestions_time, chat_autosend_dow, chat_autosend_time, onboarding_completed_at, updates_seen_at, created_at, updated_at, updated_by ` type UpsertHouseholdSettingsParams struct { @@ -285,6 +287,7 @@ type UpsertHouseholdSettingsParams struct { ChatAutosendTime string OnboardingCompletedAt sql.NullString UpdatesSeenAt sql.NullString + UpdatedBy sql.NullString CreatedAt string UpdatedAt string } @@ -305,6 +308,7 @@ func (q *Queries) UpsertHouseholdSettings(ctx context.Context, arg UpsertHouseho arg.ChatAutosendTime, arg.OnboardingCompletedAt, arg.UpdatesSeenAt, + arg.UpdatedBy, arg.CreatedAt, arg.UpdatedAt, ) @@ -325,6 +329,7 @@ func (q *Queries) UpsertHouseholdSettings(ctx context.Context, arg UpsertHouseho &i.UpdatesSeenAt, &i.CreatedAt, &i.UpdatedAt, + &i.UpdatedBy, ) return i, err } diff --git a/internal/household/settings.go b/internal/household/settings.go new file mode 100644 index 0000000..1e1df89 --- /dev/null +++ b/internal/household/settings.go @@ -0,0 +1,340 @@ +package household + +import ( + "context" + "database/sql" + "errors" + "fmt" + "regexp" + "strings" + "sync" + + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/db" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/store" +) + +// Seed values for a new household (CTXD CONTEXT.MD). Days count from +// Sunday = 0, as time.Weekday does. +const ( + SeedDinnerTime = "17:30" + SeedReminderTime = "16:30" + SeedReviewTime = "09:00" // rotation check + SeedPlanDraftDow = 5 // Friday + SeedPlanDraftTime = "09:00" + SeedSuggestionsDow = 0 // Sunday + SeedSuggestionsTime = "08:00" + SeedChatAutosendDow = 6 // Saturday + SeedChatAutosendTime = "08:00" +) + +// ErrNoDefaultZone is returned by SeedSettings when the household settings +// do not exist yet and no default zone was configured to create them with. +var ErrNoDefaultZone = errors.New("household settings do not exist yet and no default time zone is configured") + +// ErrInvalidDefaultZone is returned by SeedSettings when the household +// settings do not exist yet and the configured default zone cannot be loaded. +var ErrInvalidDefaultZone = errors.New("household settings do not exist yet and the default time zone is invalid") + +// Settings are the shared household settings. Times are household-local +// 24-hour HH:MM in Timezone; days of the week count from Sunday = 0. +type Settings struct { + Timezone string `json:"timezone"` + DinnerTime string `json:"dinner_time"` + ReminderTime string `json:"reminder_time"` + ReviewTime string `json:"review_time"` + PlanDraftDow int `json:"plan_draft_dow"` + PlanDraftTime string `json:"plan_draft_time"` + SuggestionsDow int `json:"suggestions_dow"` + SuggestionsTime string `json:"suggestions_time"` + ChatAutosendDow int `json:"chat_autosend_dow"` + ChatAutosendTime string `json:"chat_autosend_time"` + OnboardingCompletedAt *string `json:"onboarding_completed_at"` + UpdatesSeenAt *string `json:"updates_seen_at"` + UpdatedBy *string `json:"updated_by"` + CreatedAt string `json:"created_at"` + UpdatedAt string `json:"updated_at"` +} + +// SettingsPatch changes the household settings. Nil fields are left as they +// are. +type SettingsPatch struct { + Timezone *string + DinnerTime *string + ReminderTime *string + ReviewTime *string + PlanDraftDow *int + PlanDraftTime *string + SuggestionsDow *int + SuggestionsTime *string + ChatAutosendDow *int + ChatAutosendTime *string +} + +// Problem is one invalid field of a SettingsPatch, named by its JSON key. +type Problem struct { + Field string + Reason string +} + +// ValidationError lists every invalid field of a SettingsPatch. +type ValidationError struct { + Problems []Problem +} + +func (e *ValidationError) Error() string { + parts := make([]string, len(e.Problems)) + for i, p := range e.Problems { + parts[i] = p.Field + ": " + p.Reason + } + return "invalid household settings: " + strings.Join(parts, "; ") +} + +// SeedSettings creates the household settings row with zone defaultTZ and the +// seed times and days if it does not exist. An existing row is never changed, +// and defaultTZ is then ignored, valid or not. With no row, an empty +// defaultTZ is ErrNoDefaultZone and an unloadable one ErrInvalidDefaultZone. +func SeedSettings(ctx context.Context, st *store.Store, defaultTZ string) error { + return st.WithTx(ctx, func(tx *sql.Tx) error { + q := db.New(tx) + _, err := q.GetHouseholdSettings(ctx) + if err == nil { + return nil + } + if !errors.Is(err, sql.ErrNoRows) { + return fmt.Errorf("household: read settings: %w", err) + } + if defaultTZ == "" { + return ErrNoDefaultZone + } + if _, err := LoadZone(defaultTZ); err != nil { + return fmt.Errorf("%w: %v", ErrInvalidDefaultZone, err) + } + now := store.Now().String() + _, err = q.UpsertHouseholdSettings(ctx, db.UpsertHouseholdSettingsParams{ + Timezone: defaultTZ, + DinnerTime: SeedDinnerTime, + ReminderTime: SeedReminderTime, + ReviewTime: SeedReviewTime, + PlanDraftDow: SeedPlanDraftDow, + PlanDraftTime: SeedPlanDraftTime, + SuggestionsDow: SeedSuggestionsDow, + SuggestionsTime: SeedSuggestionsTime, + ChatAutosendDow: SeedChatAutosendDow, + ChatAutosendTime: SeedChatAutosendTime, + CreatedAt: now, + UpdatedAt: now, + }) + if err != nil { + return fmt.Errorf("household: seed settings: %w", err) + } + return nil + }) +} + +// SettingsService reads and changes the household settings, and tells +// in-process subscribers (the scheduler) about every change. +type SettingsService struct { + st *store.Store + + // updateMu serialises Update with its publish, so subscribers see + // changes in commit order. + updateMu sync.Mutex + + subsMu sync.Mutex + subs map[int]func(Settings) + nextID int +} + +// NewSettingsService returns a service over st. Call SeedSettings first. +func NewSettingsService(st *store.Store) *SettingsService { + return &SettingsService{st: st, subs: make(map[int]func(Settings))} +} + +// Get returns the household settings. +func (s *SettingsService) Get(ctx context.Context) (Settings, error) { + row, err := db.New(s.st.Reader).GetHouseholdSettings(ctx) + if err != nil { + return Settings{}, fmt.Errorf("household: read settings: %w", err) + } + return fromRow(row), nil +} + +// Update validates patch, applies it, records updatedBy (the signed-in +// member's subject, which must not be empty) and publishes the new settings +// to every subscriber. An invalid patch is a *ValidationError and changes +// nothing. +func (s *SettingsService) Update(ctx context.Context, patch SettingsPatch, updatedBy string) (Settings, error) { + if updatedBy == "" { + return Settings{}, errors.New("household: update settings: no member to record as updated_by") + } + if err := patch.validate(); err != nil { + return Settings{}, err + } + + s.updateMu.Lock() + defer s.updateMu.Unlock() + + var row db.HouseholdSetting + err := s.st.WithTx(ctx, func(tx *sql.Tx) error { + q := db.New(tx) + cur, err := q.GetHouseholdSettings(ctx) + if err != nil { + return fmt.Errorf("household: read settings: %w", err) + } + p := db.UpsertHouseholdSettingsParams{ + Timezone: cur.Timezone, + DinnerTime: cur.DinnerTime, + ReminderTime: cur.ReminderTime, + ReviewTime: cur.ReviewTime, + PlanDraftDow: cur.PlanDraftDow, + PlanDraftTime: cur.PlanDraftTime, + SuggestionsDow: cur.SuggestionsDow, + SuggestionsTime: cur.SuggestionsTime, + ChatAutosendDow: cur.ChatAutosendDow, + ChatAutosendTime: cur.ChatAutosendTime, + OnboardingCompletedAt: cur.OnboardingCompletedAt, + UpdatesSeenAt: cur.UpdatesSeenAt, + UpdatedBy: sql.NullString{String: updatedBy, Valid: true}, + CreatedAt: cur.CreatedAt, + UpdatedAt: store.Now().String(), + } + setString(&p.Timezone, patch.Timezone) + setString(&p.DinnerTime, patch.DinnerTime) + setString(&p.ReminderTime, patch.ReminderTime) + setString(&p.ReviewTime, patch.ReviewTime) + setDow(&p.PlanDraftDow, patch.PlanDraftDow) + setString(&p.PlanDraftTime, patch.PlanDraftTime) + setDow(&p.SuggestionsDow, patch.SuggestionsDow) + setString(&p.SuggestionsTime, patch.SuggestionsTime) + setDow(&p.ChatAutosendDow, patch.ChatAutosendDow) + setString(&p.ChatAutosendTime, patch.ChatAutosendTime) + row, err = q.UpsertHouseholdSettings(ctx, p) + if err != nil { + return fmt.Errorf("household: update settings: %w", err) + } + return nil + }) + if err != nil { + return Settings{}, err + } + + settings := fromRow(row) + s.publish(settings) + return settings, nil +} + +// Subscribe registers fn to receive the new settings after every successful +// Update. fn runs synchronously on the updating goroutine, so it must return +// promptly and must not call Update. The returned function unsubscribes fn. +func (s *SettingsService) Subscribe(fn func(Settings)) (unsubscribe func()) { + s.subsMu.Lock() + defer s.subsMu.Unlock() + id := s.nextID + s.nextID++ + s.subs[id] = fn + return func() { + s.subsMu.Lock() + defer s.subsMu.Unlock() + delete(s.subs, id) + } +} + +func (s *SettingsService) publish(settings Settings) { + s.subsMu.Lock() + fns := make([]func(Settings), 0, len(s.subs)) + for _, fn := range s.subs { + fns = append(fns, fn) + } + s.subsMu.Unlock() + for _, fn := range fns { + fn(settings) + } +} + +func (p SettingsPatch) validate() error { + var problems []Problem + fail := func(field, reason string) { + problems = append(problems, Problem{Field: field, Reason: reason}) + } + if p == (SettingsPatch{}) { + fail("settings", "no settings to change") + } + if p.Timezone != nil { + if _, err := LoadZone(*p.Timezone); err != nil { + fail("timezone", err.Error()) + } + } + for _, t := range []struct { + field string + value *string + }{ + {"dinner_time", p.DinnerTime}, + {"reminder_time", p.ReminderTime}, + {"review_time", p.ReviewTime}, + {"plan_draft_time", p.PlanDraftTime}, + {"suggestions_time", p.SuggestionsTime}, + {"chat_autosend_time", p.ChatAutosendTime}, + } { + if t.value != nil && !clockTime.MatchString(*t.value) { + fail(t.field, fmt.Sprintf("invalid time %q (want 24-hour HH:MM, 00:00-23:59)", *t.value)) + } + } + for _, d := range []struct { + field string + value *int + }{ + {"plan_draft_dow", p.PlanDraftDow}, + {"suggestions_dow", p.SuggestionsDow}, + {"chat_autosend_dow", p.ChatAutosendDow}, + } { + if d.value != nil && (*d.value < 0 || *d.value > 6) { + fail(d.field, fmt.Sprintf("invalid day of the week %d (want 0-6, Sunday = 0)", *d.value)) + } + } + if len(problems) > 0 { + return &ValidationError{Problems: problems} + } + return nil +} + +// clockTime matches a zero-padded 24-hour HH:MM time. +var clockTime = regexp.MustCompile(`^([01][0-9]|2[0-3]):[0-5][0-9]$`) + +func setString(dst *string, v *string) { + if v != nil { + *dst = *v + } +} + +func setDow(dst *int64, v *int) { + if v != nil { + *dst = int64(*v) + } +} + +func fromRow(r db.HouseholdSetting) Settings { + return Settings{ + Timezone: r.Timezone, + DinnerTime: r.DinnerTime, + ReminderTime: r.ReminderTime, + ReviewTime: r.ReviewTime, + PlanDraftDow: int(r.PlanDraftDow), + PlanDraftTime: r.PlanDraftTime, + SuggestionsDow: int(r.SuggestionsDow), + SuggestionsTime: r.SuggestionsTime, + ChatAutosendDow: int(r.ChatAutosendDow), + ChatAutosendTime: r.ChatAutosendTime, + OnboardingCompletedAt: nullable(r.OnboardingCompletedAt), + UpdatesSeenAt: nullable(r.UpdatesSeenAt), + UpdatedBy: nullable(r.UpdatedBy), + CreatedAt: r.CreatedAt, + UpdatedAt: r.UpdatedAt, + } +} + +func nullable(s sql.NullString) *string { + if !s.Valid { + return nil + } + return &s.String +} diff --git a/internal/household/settings_test.go b/internal/household/settings_test.go new file mode 100644 index 0000000..a01120c --- /dev/null +++ b/internal/household/settings_test.go @@ -0,0 +1,374 @@ +package household + +import ( + "context" + "errors" + "slices" + "sync" + "testing" + + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/store" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/migrations" +) + +// migratedStore opens a temp store in dir migrated with the embedded +// migrations, closed when the test ends. +func migratedStore(t *testing.T, dir string) *store.Store { + t.Helper() + ctx := context.Background() + st, err := store.Open(ctx, dir) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { + if err := st.Close(); err != nil { + t.Error(err) + } + }) + if err := store.Migrate(ctx, st.Writer, migrations.FS); err != nil { + t.Fatal(err) + } + return st +} + +// seededService returns a settings service over a migrated temp database +// seeded with zone America/New_York. +func seededService(t *testing.T) *SettingsService { + t.Helper() + st := migratedStore(t, t.TempDir()) + if err := SeedSettings(context.Background(), st, "America/New_York"); err != nil { + t.Fatal(err) + } + return NewSettingsService(st) +} + +func ptr[T any](v T) *T { return &v } + +func TestSeedSettingsCreatesRow(t *testing.T) { + t.Parallel() + ctx := context.Background() + st := migratedStore(t, t.TempDir()) + + if err := SeedSettings(ctx, st, "America/Chicago"); err != nil { + t.Fatal(err) + } + got, err := NewSettingsService(st).Get(ctx) + if err != nil { + t.Fatal(err) + } + want := Settings{ + Timezone: "America/Chicago", + DinnerTime: "17:30", + ReminderTime: "16:30", + ReviewTime: "09:00", + PlanDraftDow: 5, + PlanDraftTime: "09:00", + SuggestionsDow: 0, + SuggestionsTime: "08:00", + ChatAutosendDow: 6, + ChatAutosendTime: "08:00", + CreatedAt: got.CreatedAt, + UpdatedAt: got.UpdatedAt, + } + if !equalSettings(got, want) { + t.Errorf("seeded settings = %+v\nwant %+v", got, want) + } + var created store.Timestamp + if err := created.Scan(got.CreatedAt); err != nil { + t.Errorf("created_at: %v", err) + } + if got.UpdatedAt != got.CreatedAt { + t.Errorf("updated_at = %q, want created_at %q", got.UpdatedAt, got.CreatedAt) + } +} + +func TestSeedSettingsNeverOverwrites(t *testing.T) { + t.Parallel() + ctx := context.Background() + dir := t.TempDir() + + // First boot seeds; the household then changes a setting. + first := migratedStore(t, dir) + if err := SeedSettings(ctx, first, "America/Chicago"); err != nil { + t.Fatal(err) + } + changed, err := NewSettingsService(first).Update(ctx, SettingsPatch{DinnerTime: ptr("18:15")}, "sam") + if err != nil { + t.Fatal(err) + } + if err := first.Close(); err != nil { + t.Fatal(err) + } + + // Later boots reopen the same database with a different default zone, + // an invalid one and none at all. + for _, defaultTZ := range []string{"Europe/London", "Mars/Olympus_Mons", ""} { + st := migratedStore(t, dir) + if err := SeedSettings(ctx, st, defaultTZ); err != nil { + t.Fatalf("reboot with default zone %q: %v", defaultTZ, err) + } + got, err := NewSettingsService(st).Get(ctx) + if err != nil { + t.Fatal(err) + } + if !equalSettings(got, changed) { + t.Errorf("reboot with default zone %q changed settings:\n got %+v\nwant %+v", defaultTZ, got, changed) + } + if err := st.Close(); err != nil { + t.Fatal(err) + } + } +} + +func TestSeedSettingsRequiresDefaultZone(t *testing.T) { + t.Parallel() + ctx := context.Background() + + tests := []struct { + name string + defaultTZ string + is error + }{ + {"unset", "", ErrNoDefaultZone}, + {"not an IANA zone", "Mars/Olympus_Mons", ErrInvalidDefaultZone}, + {"Local", "Local", ErrInvalidDefaultZone}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + st := migratedStore(t, t.TempDir()) + err := SeedSettings(ctx, st, tt.defaultTZ) + if err == nil { + t.Fatal("SeedSettings succeeded") + } + if !errors.Is(err, tt.is) { + t.Errorf("error = %v, want %v", err, tt.is) + } + if _, err := NewSettingsService(st).Get(ctx); err == nil { + t.Error("a settings row was created despite the error") + } + }) + } +} + +func TestUpdateAppliesPatchAndRecordsUpdatedBy(t *testing.T) { + t.Parallel() + ctx := context.Background() + svc := seededService(t) + before, err := svc.Get(ctx) + if err != nil { + t.Fatal(err) + } + + got, err := svc.Update(ctx, SettingsPatch{ + Timezone: ptr("Europe/Paris"), + DinnerTime: ptr("19:00"), + ReminderTime: ptr("00:00"), + ReviewTime: ptr("23:59"), + PlanDraftDow: ptr(4), + PlanDraftTime: ptr("10:30"), + SuggestionsDow: ptr(1), + SuggestionsTime: ptr("07:45"), + ChatAutosendDow: ptr(0), + ChatAutosendTime: ptr("12:00"), + }, "sam") + if err != nil { + t.Fatal(err) + } + want := Settings{ + Timezone: "Europe/Paris", + DinnerTime: "19:00", + ReminderTime: "00:00", + ReviewTime: "23:59", + PlanDraftDow: 4, + PlanDraftTime: "10:30", + SuggestionsDow: 1, + SuggestionsTime: "07:45", + ChatAutosendDow: 0, + ChatAutosendTime: "12:00", + UpdatedBy: ptr("sam"), + CreatedAt: before.CreatedAt, + UpdatedAt: got.UpdatedAt, + } + if !equalSettings(got, want) { + t.Errorf("Update = %+v\nwant %+v", got, want) + } + if got.UpdatedAt < before.UpdatedAt { + t.Errorf("updated_at went back: %s < %s", got.UpdatedAt, before.UpdatedAt) + } + stored, err := svc.Get(ctx) + if err != nil { + t.Fatal(err) + } + if !equalSettings(stored, got) { + t.Errorf("stored = %+v\nwant %+v", stored, got) + } + + // A partial patch changes only its fields and records its own member. + got, err = svc.Update(ctx, SettingsPatch{DinnerTime: ptr("18:00")}, "alex") + if err != nil { + t.Fatal(err) + } + want.DinnerTime = "18:00" + want.UpdatedBy = ptr("alex") + want.UpdatedAt = got.UpdatedAt + if !equalSettings(got, want) { + t.Errorf("partial Update = %+v\nwant %+v", got, want) + } +} + +func TestUpdateRequiresMember(t *testing.T) { + t.Parallel() + ctx := context.Background() + svc := seededService(t) + notified := 0 + svc.Subscribe(func(Settings) { notified++ }) + + if _, err := svc.Update(ctx, SettingsPatch{DinnerTime: ptr("18:00")}, ""); err == nil { + t.Fatal("Update with no member succeeded") + } + got, err := svc.Get(ctx) + if err != nil { + t.Fatal(err) + } + if got.DinnerTime != SeedDinnerTime || got.UpdatedBy != nil || notified != 0 { + t.Errorf("Update with no member changed settings or notified: %+v, notified %d", got, notified) + } +} + +func TestUpdateValidation(t *testing.T) { + t.Parallel() + ctx := context.Background() + + tests := []struct { + name string + patch SettingsPatch + fields []string + }{ + {"empty patch", SettingsPatch{}, []string{"settings"}}, + {"unknown zone", SettingsPatch{Timezone: ptr("Mars/Olympus_Mons")}, []string{"timezone"}}, + {"empty zone", SettingsPatch{Timezone: ptr("")}, []string{"timezone"}}, + {"Local zone", SettingsPatch{Timezone: ptr("Local")}, []string{"timezone"}}, + {"zone path traversal", SettingsPatch{Timezone: ptr("../../etc/passwd")}, []string{"timezone"}}, + {"hour 24", SettingsPatch{DinnerTime: ptr("24:00")}, []string{"dinner_time"}}, + {"minute 60", SettingsPatch{ReminderTime: ptr("16:60")}, []string{"reminder_time"}}, + {"unpadded hour", SettingsPatch{ReviewTime: ptr("9:00")}, []string{"review_time"}}, + {"seconds", SettingsPatch{PlanDraftTime: ptr("09:00:00")}, []string{"plan_draft_time"}}, + {"12-hour", SettingsPatch{SuggestionsTime: ptr("08:00 AM")}, []string{"suggestions_time"}}, + {"empty time", SettingsPatch{ChatAutosendTime: ptr("")}, []string{"chat_autosend_time"}}, + {"dow 7", SettingsPatch{PlanDraftDow: ptr(7)}, []string{"plan_draft_dow"}}, + {"dow -1", SettingsPatch{SuggestionsDow: ptr(-1)}, []string{"suggestions_dow"}}, + {"dow 100", SettingsPatch{ChatAutosendDow: ptr(100)}, []string{"chat_autosend_dow"}}, + { + "every problem is reported", + SettingsPatch{Timezone: ptr("Nowhere"), DinnerTime: ptr("5pm"), PlanDraftDow: ptr(9)}, + []string{"timezone", "dinner_time", "plan_draft_dow"}, + }, + { + "valid fields do not mask an invalid one", + SettingsPatch{DinnerTime: ptr("18:00"), ChatAutosendDow: ptr(8)}, + []string{"chat_autosend_dow"}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + svc := seededService(t) + before, err := svc.Get(ctx) + if err != nil { + t.Fatal(err) + } + published := 0 + svc.Subscribe(func(Settings) { published++ }) + + _, err = svc.Update(ctx, tt.patch, "sam") + var verr *ValidationError + if !errors.As(err, &verr) { + t.Fatalf("Update error = %v, want *ValidationError", err) + } + var fields []string + for _, p := range verr.Problems { + fields = append(fields, p.Field) + } + if !slices.Equal(fields, tt.fields) { + t.Errorf("problem fields = %v, want %v", fields, tt.fields) + } + + after, err := svc.Get(ctx) + if err != nil { + t.Fatal(err) + } + if !equalSettings(after, before) { + t.Errorf("invalid patch changed settings:\n got %+v\nwant %+v", after, before) + } + if published != 0 { + t.Errorf("invalid patch published %d times", published) + } + }) + } +} + +func TestUpdatePublishesToSubscribers(t *testing.T) { + t.Parallel() + ctx := context.Background() + svc := seededService(t) + + var mu sync.Mutex + var first, second []Settings + unsubscribe := svc.Subscribe(func(s Settings) { + mu.Lock() + defer mu.Unlock() + first = append(first, s) + }) + svc.Subscribe(func(s Settings) { + mu.Lock() + defer mu.Unlock() + second = append(second, s) + }) + + updated, err := svc.Update(ctx, SettingsPatch{Timezone: ptr("Asia/Tokyo")}, "sam") + if err != nil { + t.Fatal(err) + } + mu.Lock() + if len(first) != 1 || !equalSettings(first[0], updated) { + t.Errorf("first subscriber got %+v, want [%+v]", first, updated) + } + if len(second) != 1 || !equalSettings(second[0], updated) { + t.Errorf("second subscriber got %+v, want [%+v]", second, updated) + } + mu.Unlock() + + unsubscribe() + updated, err = svc.Update(ctx, SettingsPatch{DinnerTime: ptr("18:45")}, "sam") + if err != nil { + t.Fatal(err) + } + mu.Lock() + defer mu.Unlock() + if len(first) != 1 { + t.Errorf("unsubscribed subscriber got %d notifications, want 1", len(first)) + } + if len(second) != 2 || !equalSettings(second[1], updated) { + t.Errorf("second subscriber got %+v, want a second notification %+v", second, updated) + } +} + +func TestUpdateWithoutSeedFails(t *testing.T) { + t.Parallel() + svc := NewSettingsService(migratedStore(t, t.TempDir())) + if _, err := svc.Update(context.Background(), SettingsPatch{DinnerTime: ptr("18:00")}, ""); err == nil { + t.Fatal("Update succeeded with no settings row") + } +} + +// equalSettings compares settings by value, following the nullable pointers. +func equalSettings(a, b Settings) bool { + eq := func(x, y *string) bool { return (x == nil) == (y == nil) && (x == nil || *x == *y) } + if !eq(a.OnboardingCompletedAt, b.OnboardingCompletedAt) || !eq(a.UpdatesSeenAt, b.UpdatesSeenAt) || !eq(a.UpdatedBy, b.UpdatedBy) { + return false + } + a.OnboardingCompletedAt, a.UpdatesSeenAt, a.UpdatedBy = nil, nil, nil + b.OnboardingCompletedAt, b.UpdatesSeenAt, b.UpdatedBy = nil, nil, nil + return a == b +} diff --git a/internal/household/zone.go b/internal/household/zone.go new file mode 100644 index 0000000..23e34c0 --- /dev/null +++ b/internal/household/zone.go @@ -0,0 +1,28 @@ +package household + +import ( + "errors" + "fmt" + "time" + + // Embeds the IANA zone database in the binary, so zones load the same + // way in a minimal image with no /usr/share/zoneinfo. + _ "time/tzdata" +) + +// LoadZone loads the IANA zone name, such as "America/New_York". The empty +// name and "Local", which time.LoadLocation maps to UTC and the process zone, +// are refused: the household zone is never implied. +func LoadZone(name string) (*time.Location, error) { + if name == "" { + return nil, errors.New("time zone is empty (want an IANA zone such as America/New_York)") + } + if name == "Local" { + return nil, errors.New(`time zone "Local" is not an IANA zone (want e.g. America/New_York)`) + } + loc, err := time.LoadLocation(name) + if err != nil { + return nil, fmt.Errorf("unknown time zone %q (want an IANA zone such as America/New_York)", name) + } + return loc, nil +} diff --git a/internal/server/csrf_test.go b/internal/server/csrf_test.go index 7e1dd54..53a69b9 100644 --- a/internal/server/csrf_test.go +++ b/internal/server/csrf_test.go @@ -34,9 +34,10 @@ func TestCSRFThroughRouter(t *testing.T) { {"PATCH without header", http.MethodPatch, "/api/v1/settings/household", nil, http.StatusForbidden, csrfJSON}, {"DELETE with wrong header", http.MethodDelete, "/api/v1/me", []string{"wrong-token"}, http.StatusForbidden, csrfJSON}, // The right token passes the check and reaches the router, which has - // no POST /api/v1/me. + // no POST /api/v1/me, and the settings handler, which refuses the + // empty body. {"POST with right header", http.MethodPost, "/api/v1/me", []string{testSession.CSRFToken}, http.StatusMethodNotAllowed, methodNotAllowedJSON}, - {"PATCH with right header", http.MethodPatch, "/api/v1/settings/household", []string{testSession.CSRFToken}, http.StatusNotFound, notFoundJSON}, + {"PATCH with right header", http.MethodPatch, "/api/v1/settings/household", []string{testSession.CSRFToken}, http.StatusBadRequest, `{"error":{"code":"invalid_request","message":"request body must be a JSON object","retryable":false}}`}, {"GET without header", http.MethodGet, "/api/v1/me", nil, http.StatusOK, ""}, // Only GET is exempt; HEAD and OPTIONS need the header too. {"HEAD without header", http.MethodHead, "/api/v1/me", nil, http.StatusForbidden, csrfJSON}, diff --git a/internal/server/login_test.go b/internal/server/login_test.go index d928de7..0b8ada9 100644 --- a/internal/server/login_test.go +++ b/internal/server/login_test.go @@ -82,7 +82,7 @@ func newLoginRouter(t *testing.T, issuer string) http.Handler { if err != nil { t.Fatal(err) } - r, err := NewRouter(logger, "dev", trusted, flow, sessions, + r, err := NewRouter(logger, "dev", trusted, flow, sessions, newTestSettings(t), fstest.MapFS{"index.html": {Data: []byte(fixtureIndex)}}) if err != nil { t.Fatal(err) diff --git a/internal/server/origin_test.go b/internal/server/origin_test.go index 1df8212..e4d0030 100644 --- a/internal/server/origin_test.go +++ b/internal/server/origin_test.go @@ -44,7 +44,7 @@ func newOriginRouter(t *testing.T, logs io.Writer) http.Handler { t.Fatal(err) } logger := slog.New(forwarded.LogHandler(slog.NewJSONHandler(logs, nil))) - r, err := NewRouter(logger, "dev", trusted, probeAuth{}, stubSessions{}, + r, err := NewRouter(logger, "dev", trusted, probeAuth{}, stubSessions{}, newTestSettings(t), fstest.MapFS{"index.html": {Data: []byte(fixtureIndex)}}) if err != nil { t.Fatal(err) diff --git a/internal/server/server.go b/internal/server/server.go index 46520c2..c296893 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -3,6 +3,7 @@ package server import ( "encoding/json" + "errors" "io/fs" "log/slog" "net/http" @@ -11,6 +12,7 @@ import ( "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/forwarded" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/frontend" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/household" ) // reservedPrefixes are owned by the API router and are never answered with the @@ -26,9 +28,13 @@ type RouteMounter interface { // has its client address and scheme resolved, honouring forwarded headers // only from a peer in trusted (see resolveOrigin). version is reported by // GET /health; auth mounts the /auth routes; sessions guards /api/v1 and -// /media; assets is the web build served for every other GET. /health, /auth -// and the web build are public. Requests to /api/v1 other than GET also need the session's CSRF token in CSRFHeader. -func NewRouter(logger *slog.Logger, version string, trusted forwarded.Trusted, auth RouteMounter, sessions SessionLookup, assets fs.FS) (http.Handler, error) { +// /media; settings serves /api/v1/settings/household; assets is the web build +// served for every other GET. /health, /auth and the web build are public. +// Requests to /api/v1 other than GET also need the session's CSRF token in CSRFHeader. +func NewRouter(logger *slog.Logger, version string, trusted forwarded.Trusted, auth RouteMounter, sessions SessionLookup, settings *household.SettingsService, assets fs.FS) (http.Handler, error) { + if settings == nil { + return nil, errors.New("server: nil household settings service") + } notFound := apiErrorHandler(logger, http.StatusNotFound, "not_found", "not found") methodNotAllowed := apiErrorHandler(logger, http.StatusMethodNotAllowed, "method_not_allowed", "method not allowed") @@ -55,6 +61,8 @@ func NewRouter(logger *slog.Logger, version string, trusted forwarded.Trusted, a sub.NotFound(notFound) } apiV1.Get("/me", me(logger)) + apiV1.Get("/settings/household", getHouseholdSettings(logger, settings)) + apiV1.Patch("/settings/household", patchHouseholdSettings(logger, settings)) r.Route("/api", func(r chi.Router) { r.MethodNotAllowed(methodNotAllowed) r.NotFound(notFound) @@ -80,19 +88,8 @@ type apiError struct { // apiErrorHandler responds with status and a fixed, non-retryable API error. func apiErrorHandler(logger *slog.Logger, status int, code, message string) http.HandlerFunc { - body, err := json.Marshal(errorBody{Error: apiError{ - Code: code, - Message: message, - }}) - if err != nil { - panic(err) // fixed string and bool fields cannot fail to marshal - } return func(w http.ResponseWriter, r *http.Request) { - w.Header().Set("Content-Type", "application/json") - w.WriteHeader(status) - if _, err := w.Write(body); err != nil { - logger.WarnContext(r.Context(), "write error response", "code", code, "error", err) - } + writeError(w, r, logger, status, code, message) } } diff --git a/internal/server/server_test.go b/internal/server/server_test.go index 7419658..5b59d2d 100644 --- a/internal/server/server_test.go +++ b/internal/server/server_test.go @@ -1,6 +1,7 @@ package server import ( + "context" "encoding/json" "io" "log/slog" @@ -12,6 +13,10 @@ import ( "testing/fstest" "github.com/go-chi/chi/v5" + + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/household" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/store" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/migrations" ) const ( @@ -21,13 +26,18 @@ const ( ) func newTestRouter(t *testing.T, version string) http.Handler { + t.Helper() + return newTestRouterWith(t, version, newTestSettings(t)) +} + +func newTestRouterWith(t *testing.T, version string, settings *household.SettingsService) http.Handler { t.Helper() logger := slog.New(slog.NewTextHandler(io.Discard, nil)) assets := fstest.MapFS{ "index.html": {Data: []byte(fixtureIndex)}, "assets/index-AbC123.js": {Data: []byte(fixtureAsset)}, } - r, err := NewRouter(logger, version, nil, stubAuth{}, stubSessions{}, assets) + r, err := NewRouter(logger, version, nil, stubAuth{}, stubSessions{}, settings, assets) if err != nil { t.Fatal(err) } @@ -60,6 +70,38 @@ func TestAuthRoutesMounted(t *testing.T) { } } +// newTestSettings returns a settings service over a migrated temp database +// seeded with zone America/New_York. +func newTestSettings(t *testing.T) *household.SettingsService { + t.Helper() + ctx := context.Background() + st, err := store.Open(ctx, t.TempDir()) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { + if err := st.Close(); err != nil { + t.Error(err) + } + }) + if err := store.Migrate(ctx, st.Writer, migrations.FS); err != nil { + t.Fatal(err) + } + if err := household.SeedSettings(ctx, st, "America/New_York"); err != nil { + t.Fatal(err) + } + return household.NewSettingsService(st) +} + +func TestNewRouterRequiresSettings(t *testing.T) { + t.Parallel() + + logger := slog.New(slog.NewTextHandler(io.Discard, nil)) + if _, err := NewRouter(logger, "dev", nil, stubAuth{}, stubSessions{}, nil, fstest.MapFS{"index.html": {}}); err == nil { + t.Fatal("NewRouter accepted a nil settings service") + } +} + func TestHealth(t *testing.T) { t.Parallel() diff --git a/internal/server/settings.go b/internal/server/settings.go new file mode 100644 index 0000000..c29846a --- /dev/null +++ b/internal/server/settings.go @@ -0,0 +1,135 @@ +package server + +import ( + "bytes" + "encoding/json" + "errors" + "fmt" + "io" + "log/slog" + "net/http" + + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/auth" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/household" +) + +// maxSettingsBody bounds a PATCH /settings/household request body. +const maxSettingsBody = 16 << 10 + +// settingsPatch is the PATCH /settings/household body. Every field is +// optional; unknown fields and null values are refused. +type settingsPatch struct { + Timezone *string `json:"timezone"` + DinnerTime *string `json:"dinner_time"` + ReminderTime *string `json:"reminder_time"` + ReviewTime *string `json:"review_time"` + PlanDraftDow *int `json:"plan_draft_dow"` + PlanDraftTime *string `json:"plan_draft_time"` + SuggestionsDow *int `json:"suggestions_dow"` + SuggestionsTime *string `json:"suggestions_time"` + ChatAutosendDow *int `json:"chat_autosend_dow"` + ChatAutosendTime *string `json:"chat_autosend_time"` +} + +func getHouseholdSettings(logger *slog.Logger, svc *household.SettingsService) http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + settings, err := svc.Get(r.Context()) + if err != nil { + logger.ErrorContext(r.Context(), "get household settings", "error", err) + writeError(w, r, logger, http.StatusInternalServerError, "internal", "internal error") + return + } + writeJSON(w, r, logger, http.StatusOK, settings) + } +} + +// patchHouseholdSettings records the session's actor subject as updated_by. +func patchHouseholdSettings(logger *slog.Logger, svc *household.SettingsService) http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + actor, ok := auth.ActorFrom(r.Context()) + if !ok { + logger.ErrorContext(r.Context(), "PATCH /api/v1/settings/household without a session") + writeError(w, r, logger, http.StatusInternalServerError, "internal", "internal error") + return + } + patch, err := decodeSettingsPatch(http.MaxBytesReader(w, r.Body, maxSettingsBody)) + if err != nil { + writeError(w, r, logger, http.StatusBadRequest, "invalid_request", err.Error()) + return + } + settings, err := svc.Update(r.Context(), household.SettingsPatch(patch), actor.Subject) + var invalid *household.ValidationError + if errors.As(err, &invalid) { + writeError(w, r, logger, http.StatusBadRequest, "invalid_settings", invalid.Error()) + return + } + if err != nil { + logger.ErrorContext(r.Context(), "update household settings", "error", err) + writeError(w, r, logger, http.StatusInternalServerError, "internal", "internal error") + return + } + writeJSON(w, r, logger, http.StatusOK, settings) + } +} + +// decodeSettingsPatch reads a single JSON object, refusing unknown fields, +// null values and trailing data. +func decodeSettingsPatch(body io.Reader) (settingsPatch, error) { + data, err := io.ReadAll(body) + if err != nil { + var tooLarge *http.MaxBytesError + if errors.As(err, &tooLarge) { + return settingsPatch{}, fmt.Errorf("request body exceeds %d bytes", tooLarge.Limit) + } + return settingsPatch{}, errors.New("cannot read request body") + } + + var fields map[string]json.RawMessage + if err := json.Unmarshal(data, &fields); err != nil || fields == nil { + return settingsPatch{}, errors.New("request body must be a JSON object") + } + for name, raw := range fields { + if bytes.Equal(raw, []byte("null")) { + return settingsPatch{}, fmt.Errorf("%s: must not be null", name) + } + } + + var patch settingsPatch + dec := json.NewDecoder(bytes.NewReader(data)) + dec.DisallowUnknownFields() + if err := dec.Decode(&patch); err != nil { + var typeErr *json.UnmarshalTypeError + if errors.As(err, &typeErr) { + return settingsPatch{}, fmt.Errorf("%s: want a JSON %s", typeErr.Field, typeErr.Type) + } + return settingsPatch{}, fmt.Errorf("invalid settings: %v", err) + } + return patch, nil +} + +func writeJSON(w http.ResponseWriter, r *http.Request, logger *slog.Logger, status int, v any) { + body, err := json.Marshal(v) + if err != nil { + logger.ErrorContext(r.Context(), "encode response", "error", err) + writeError(w, r, logger, http.StatusInternalServerError, "internal", "internal error") + return + } + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(status) + if _, err := w.Write(body); err != nil { + logger.WarnContext(r.Context(), "write response", "error", err) + } +} + +// writeError responds with status and a non-retryable API error. +func writeError(w http.ResponseWriter, r *http.Request, logger *slog.Logger, status int, code, message string) { + body, err := json.Marshal(errorBody{Error: apiError{Code: code, Message: message}}) + if err != nil { + panic(err) // string and bool fields cannot fail to marshal + } + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(status) + if _, err := w.Write(body); err != nil { + logger.WarnContext(r.Context(), "write error response", "code", code, "error", err) + } +} diff --git a/internal/server/settings_test.go b/internal/server/settings_test.go new file mode 100644 index 0000000..2fdfbca --- /dev/null +++ b/internal/server/settings_test.go @@ -0,0 +1,278 @@ +package server + +import ( + "context" + "encoding/json" + "io" + "log/slog" + "net/http" + "net/http/httptest" + "strings" + "testing" + "testing/fstest" + "time" + + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/auth" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/household" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/store" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/migrations" +) + +const settingsPath = "/api/v1/settings/household" + +// signedIn returns a request carrying stubSessions' valid session cookie and +// its CSRF token. +func signedIn(method, target string, body io.Reader) *http.Request { + req := httptest.NewRequest(method, target, body) + req.AddCookie(validSessionCookie) + req.Header.Set(CSRFHeader, testSession.CSRFToken) + return req +} + +func serve(t *testing.T, h http.Handler, req *http.Request) (*httptest.ResponseRecorder, map[string]any) { + t.Helper() + rec := httptest.NewRecorder() + h.ServeHTTP(rec, req) + if ct := rec.Header().Get("Content-Type"); ct != "application/json" { + t.Errorf("Content-Type = %q, want application/json", ct) + } + var body map[string]any + if err := json.Unmarshal(rec.Body.Bytes(), &body); err != nil { + t.Fatalf("decode body %q: %v", rec.Body.String(), err) + } + return rec, body +} + +func TestGetHouseholdSettings(t *testing.T) { + t.Parallel() + + rec, body := serve(t, newTestRouter(t, "dev"), signedIn(http.MethodGet, settingsPath, nil)) + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want 200: %s", rec.Code, rec.Body) + } + want := map[string]any{ + "timezone": "America/New_York", + "dinner_time": "17:30", + "reminder_time": "16:30", + "review_time": "09:00", + "plan_draft_dow": float64(5), + "plan_draft_time": "09:00", + "suggestions_dow": float64(0), + "suggestions_time": "08:00", + "chat_autosend_dow": float64(6), + "chat_autosend_time": "08:00", + "onboarding_completed_at": nil, + "updates_seen_at": nil, + "updated_by": nil, + } + for k, v := range want { + got, ok := body[k] + if !ok || got != v { + t.Errorf("%s = %#v (present %v), want %#v", k, got, ok, v) + } + } + for _, k := range []string{"created_at", "updated_at"} { + if s, ok := body[k].(string); !ok || s == "" { + t.Errorf("%s = %#v, want a timestamp", k, body[k]) + } + } + if len(body) != len(want)+2 { + t.Errorf("body has %d fields, want %d: %v", len(body), len(want)+2, body) + } +} + +func TestPatchHouseholdSettings(t *testing.T) { + t.Parallel() + settings := newTestSettings(t) + router := newTestRouterWith(t, "dev", settings) + + var notified []household.Settings + settings.Subscribe(func(s household.Settings) { notified = append(notified, s) }) + + rec, body := serve(t, router, signedIn(http.MethodPatch, settingsPath, + strings.NewReader(`{"timezone":"Europe/Berlin","dinner_time":"18:00","plan_draft_dow":4}`))) + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want 200: %s", rec.Code, rec.Body) + } + for k, v := range map[string]any{ + "timezone": "Europe/Berlin", + "dinner_time": "18:00", + "plan_draft_dow": float64(4), + "reminder_time": "16:30", // unchanged + "updated_by": testSession.Subject, + } { + if body[k] != v { + t.Errorf("%s = %#v, want %#v", k, body[k], v) + } + } + if len(notified) != 1 || notified[0].Timezone != "Europe/Berlin" { + t.Errorf("subscriber notified with %+v, want one change to Europe/Berlin", notified) + } + + _, body = serve(t, router, signedIn(http.MethodGet, settingsPath, nil)) + if body["timezone"] != "Europe/Berlin" || body["updated_by"] != testSession.Subject { + t.Errorf("GET after PATCH = %v", body) + } +} + +func TestPatchHouseholdSettingsRejected(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + body string + code string + in string // the error message must contain this + }{ + {"bad timezone", `{"timezone":"Mars/Olympus_Mons"}`, "invalid_settings", "timezone"}, + {"bad time", `{"dinner_time":"25:00"}`, "invalid_settings", "dinner_time"}, + {"unpadded time", `{"reminder_time":"9:30"}`, "invalid_settings", "reminder_time"}, + {"bad dow", `{"suggestions_dow":7}`, "invalid_settings", "suggestions_dow"}, + {"negative dow", `{"chat_autosend_dow":-1}`, "invalid_settings", "chat_autosend_dow"}, + {"empty object", `{}`, "invalid_settings", "no settings to change"}, + {"fractional dow", `{"plan_draft_dow":5.5}`, "invalid_request", "plan_draft_dow"}, + {"string dow", `{"plan_draft_dow":"5"}`, "invalid_request", "plan_draft_dow"}, + {"numeric time", `{"dinner_time":1730}`, "invalid_request", "dinner_time"}, + {"null value", `{"timezone":null}`, "invalid_request", "timezone"}, + {"unknown field", `{"dinner":"17:30"}`, "invalid_request", "dinner"}, + {"read-only field", `{"updated_by":"someone"}`, "invalid_request", "updated_by"}, + {"not an object", `["timezone"]`, "invalid_request", "JSON object"}, + {"null body", `null`, "invalid_request", "JSON object"}, + {"trailing data", `{"dinner_time":"18:00"} {}`, "invalid_request", "JSON object"}, + {"malformed", `{"dinner_time":`, "invalid_request", "JSON object"}, + {"too large", `{"timezone":"` + strings.Repeat("a", maxSettingsBody) + `"}`, "invalid_request", "exceeds"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + settings := newTestSettings(t) + router := newTestRouterWith(t, "dev", settings) + notified := 0 + settings.Subscribe(func(household.Settings) { notified++ }) + + rec, body := serve(t, router, signedIn(http.MethodPatch, settingsPath, strings.NewReader(tt.body))) + if rec.Code != http.StatusBadRequest { + t.Fatalf("status = %d, want 400: %s", rec.Code, rec.Body) + } + apiErr, ok := body["error"].(map[string]any) + if !ok || len(body) != 1 || len(apiErr) != 3 { + t.Fatalf("body = %s, want {\"error\":{code,message,retryable}}", rec.Body) + } + if apiErr["code"] != tt.code || apiErr["retryable"] != false { + t.Errorf("error = %v, want code %q, retryable false", apiErr, tt.code) + } + if msg, _ := apiErr["message"].(string); !strings.Contains(msg, tt.in) { + t.Errorf("message %q does not contain %q", msg, tt.in) + } + if notified != 0 { + t.Errorf("rejected PATCH notified subscribers %d times", notified) + } + _, after := serve(t, router, signedIn(http.MethodGet, settingsPath, nil)) + if after["timezone"] != "America/New_York" || after["dinner_time"] != "17:30" || after["updated_by"] != nil { + t.Errorf("rejected PATCH changed settings: %v", after) + } + }) + } +} + +func TestHouseholdSettingsMethodNotAllowed(t *testing.T) { + t.Parallel() + + rec, body := serve(t, newTestRouter(t, "dev"), signedIn(http.MethodPut, settingsPath, strings.NewReader(`{}`))) + if rec.Code != http.StatusMethodNotAllowed { + t.Errorf("status = %d, want 405", rec.Code) + } + if apiErr, _ := body["error"].(map[string]any); apiErr["code"] != "method_not_allowed" { + t.Errorf("body = %v", body) + } +} + +// TestHouseholdSettingsWithRealSession drives the settings API through the +// production router and session manager: a member signs in, reads the CSRF +// token from GET /api/v1/me and PATCHes; updated_by comes from the session. +// Without a session, or without the CSRF token, the PATCH is refused and +// nothing changes. +func TestHouseholdSettingsWithRealSession(t *testing.T) { + t.Parallel() + ctx := context.Background() + st, err := store.Open(ctx, t.TempDir()) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { + if err := st.Close(); err != nil { + t.Error(err) + } + }) + if err := store.Migrate(ctx, st.Writer, migrations.FS); err != nil { + t.Fatal(err) + } + if err := household.SeedSettings(ctx, st, "America/New_York"); err != nil { + t.Fatal(err) + } + sessions, err := auth.NewSessionManager(st, time.Hour, 24*time.Hour) + if err != nil { + t.Fatal(err) + } + logger := slog.New(slog.NewTextHandler(io.Discard, nil)) + router, err := NewRouter(logger, "dev", nil, stubAuth{}, sessions, household.NewSettingsService(st), + fstest.MapFS{"index.html": {Data: []byte(fixtureIndex)}}) + if err != nil { + t.Fatal(err) + } + + // Sign in as the callback would. + login := httptest.NewRecorder() + if err := sessions.StartSession(login, httptest.NewRequest(http.MethodGet, "/auth/callback", nil), auth.Claims{ + Subject: "authentik-sub-42", + Email: "cook@example.com", + Name: "Home Cook", + Groups: []string{"mealquest-admin"}, + }); err != nil { + t.Fatal(err) + } + cookies := login.Result().Cookies() + if len(cookies) != 1 || cookies[0].Name != auth.SessionCookieName { + t.Fatalf("StartSession set cookies %v, want the session cookie", cookies) + } + cookie := cookies[0] + withCookie := func(req *http.Request) *http.Request { + req.AddCookie(cookie) + return req + } + + rec, me := serve(t, router, withCookie(httptest.NewRequest(http.MethodGet, "/api/v1/me", nil))) + token, _ := me["csrfToken"].(string) + if rec.Code != http.StatusOK || token == "" { + t.Fatalf("GET /api/v1/me = %d %v, want 200 with a CSRF token", rec.Code, me) + } + + patch := `{"dinner_time":"18:45"}` + refused := []struct { + name string + req *http.Request + status int + }{ + {"no session", httptest.NewRequest(http.MethodPatch, settingsPath, strings.NewReader(patch)), http.StatusUnauthorized}, + {"no CSRF token", withCookie(httptest.NewRequest(http.MethodPatch, settingsPath, strings.NewReader(patch))), http.StatusForbidden}, + } + for _, tt := range refused { + if rec, body := serve(t, router, tt.req); rec.Code != tt.status { + t.Errorf("%s: PATCH status = %d, want %d: %v", tt.name, rec.Code, tt.status, body) + } + } + _, body := serve(t, router, withCookie(httptest.NewRequest(http.MethodGet, settingsPath, nil))) + if body["dinner_time"] != household.SeedDinnerTime || body["updated_by"] != nil { + t.Fatalf("refused PATCH changed settings: %v", body) + } + + req := withCookie(httptest.NewRequest(http.MethodPatch, settingsPath, strings.NewReader(patch))) + req.Header.Set(CSRFHeader, token) + rec, body = serve(t, router, req) + if rec.Code != http.StatusOK { + t.Fatalf("PATCH status = %d, want 200: %v", rec.Code, body) + } + if body["dinner_time"] != "18:45" || body["updated_by"] != "authentik-sub-42" { + t.Errorf("PATCH = %v, want dinner_time 18:45 updated_by authentik-sub-42", body) + } +} diff --git a/internal/store/migrate_test.go b/internal/store/migrate_test.go index bff84c5..cc4f356 100644 --- a/internal/store/migrate_test.go +++ b/internal/store/migrate_test.go @@ -175,8 +175,8 @@ func TestMigrateEmbeddedMigrations(t *testing.T) { if err := Migrate(context.Background(), s.Writer, migrations.FS); err != nil { t.Fatal(err) } - if got := versions(migrationRows(t, s)); got != "0000_bootstrap,0001_init" { - t.Errorf("versions = %q, want 0000_bootstrap,0001_init", got) + if got := versions(migrationRows(t, s)); got != "0000_bootstrap,0001_init,0002_household_settings_updated_by" { + t.Errorf("versions = %q, want 0000_bootstrap,0001_init,0002_household_settings_updated_by", got) } } @@ -188,15 +188,16 @@ func TestMigrateBootstrapThenEmbeddedInitApplies(t *testing.T) { t.Fatal(err) } // A database migrated by the bootstrap-only build upgrades to the - // embedded set, which adds 0001_init.sql alongside the unchanged bootstrap. + // embedded set, which adds the later migrations alongside the unchanged + // bootstrap. if err := Migrate(ctx, s.Writer, fstest.MapFS{"0000_bootstrap.sql": {Data: bootstrap}}); err != nil { t.Fatal(err) } if err := Migrate(ctx, s.Writer, migrations.FS); err != nil { t.Fatal(err) } - if got := versions(migrationRows(t, s)); got != "0000_bootstrap,0001_init" { - t.Errorf("versions = %q, want 0000_bootstrap,0001_init", got) + if got := versions(migrationRows(t, s)); got != "0000_bootstrap,0001_init,0002_household_settings_updated_by" { + t.Errorf("versions = %q, want 0000_bootstrap,0001_init,0002_household_settings_updated_by", got) } if !tableExists(t, s, "dinners") { t.Error("table from 0001_init was not created") diff --git a/migrations/0002_household_settings_updated_by.sql b/migrations/0002_household_settings_updated_by.sql new file mode 100644 index 0000000..b75042a --- /dev/null +++ b/migrations/0002_household_settings_updated_by.sql @@ -0,0 +1,4 @@ +-- 0002_household_settings_updated_by: record who last changed the household +-- settings, as dinners and suggestion_batches already do. NULL until the +-- first change made by a signed-in member. +ALTER TABLE household_settings ADD COLUMN updated_by TEXT; diff --git a/migrations/queries/settings.sql b/migrations/queries/settings.sql index 2b39e69..171be84 100644 --- a/migrations/queries/settings.sql +++ b/migrations/queries/settings.sql @@ -10,12 +10,12 @@ INSERT INTO household_settings ( id, timezone, dinner_time, reminder_time, review_time, plan_draft_dow, plan_draft_time, suggestions_dow, suggestions_time, chat_autosend_dow, chat_autosend_time, onboarding_completed_at, updates_seen_at, - created_at, updated_at + updated_by, created_at, updated_at ) VALUES ( 1, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, - ?, ? + ?, ?, ? ) ON CONFLICT (id) DO UPDATE SET timezone = excluded.timezone, @@ -30,6 +30,7 @@ ON CONFLICT (id) DO UPDATE SET chat_autosend_time = excluded.chat_autosend_time, onboarding_completed_at = excluded.onboarding_completed_at, updates_seen_at = excluded.updates_seen_at, + updated_by = excluded.updated_by, updated_at = excluded.updated_at RETURNING *;