diff --git a/src/actions/admin-radio-extra.ts b/src/actions/admin-radio-extra.ts index ca8ec201..9dbca337 100644 --- a/src/actions/admin-radio-extra.ts +++ b/src/actions/admin-radio-extra.ts @@ -2,11 +2,12 @@ import { eq } from "drizzle-orm"; import { revalidatePath } from "next/cache"; +import { redirect } from "next/navigation"; +import { executeLegacyHotelMutation } from "@/features/housekeeping/domains/hotel/services/mutations"; import { requirePermission } from "@/lib/admin/guard"; -import { db, RadioBanners, RadioRanks, WebsiteSetting } from "@/lib/db"; +import { db, RadioBanners, RadioRanks } from "@/lib/db"; import { logger } from "@/lib/logger"; import { PERMS } from "@/lib/permissions"; -import { siteSettings } from "@/lib/services/site-settings"; // ── Helpers ──────────────────────────────────────────────────────────────── @@ -39,22 +40,32 @@ function bool(raw: FormDataEntryValue | null): boolean { * siteSettings cache so the public radio pages pick the change up immediately. */ export async function saveRadioSetting(formData: FormData): Promise { - await requirePermission(PERMS.RADIO_EDIT); - const key = str(formData.get("key")).trim().slice(0, 255); + const staff = await requirePermission(PERMS.RADIO_EDIT); + const key = str(formData.get("key")).trim(); const value = str(formData.get("value")); - const comment = str(formData.get("comment")).trim().slice(0, 255); - if (!key) return; + const comment = str(formData.get("comment")).trim(); + let partial = false; try { - await db - .insert(WebsiteSetting) - .values({ key, value, comment: comment || null }) - .onDuplicateKeyUpdate({ set: { value } }); - siteSettings.reload(); - } catch (err) { - logger.error("Failed to save radio setting", { err, key }); + const snapshot = await executeLegacyHotelMutation( + staff, + "radio.settings.save-one", + { key, value, comment }, + ); + partial = snapshot.completion?.cache === "unavailable"; + } catch { + return redirect("/admin/radio/settings?error=1"); } - revalidatePath("/admin/radio/settings"); + try { + revalidatePath("/admin/radio/settings"); + } catch { + partial = true; + } + redirect( + partial + ? "/admin/radio/settings?partial=1" + : "/admin/radio/settings?saved=1", + ); } /** @@ -63,29 +74,32 @@ export async function saveRadioSetting(formData: FormData): Promise { * only touch those (and never wipe unrelated settings). */ export async function saveRadioSettings(formData: FormData): Promise { - await requirePermission(PERMS.RADIO_EDIT); + const staff = await requirePermission(PERMS.RADIO_EDIT); const keysRaw = str(formData.get("__keys")); - const keys = keysRaw - .split(",") - .map((k) => k.trim()) - .filter((k) => k.startsWith("radio_") || k.startsWith("auto_dj_")); - if (keys.length === 0) return; + const keys = keysRaw.split(",").map((key) => key.trim()); + const entries = keys.map((key) => ({ key, value: str(formData.get(key)) })); + let partial = false; try { - await Promise.all( - keys.map((key) => { - const value = str(formData.get(key)); - return db - .insert(WebsiteSetting) - .values({ key, value, comment: null }) - .onDuplicateKeyUpdate({ set: { value } }); - }), + const snapshot = await executeLegacyHotelMutation( + staff, + "radio.settings.save-many", + { entries }, ); - siteSettings.reload(); - } catch (err) { - logger.error("Failed to bulk-save radio settings", { err, keys }); + partial = snapshot.completion?.cache === "unavailable"; + } catch { + return redirect("/admin/radio/settings?error=1"); } - revalidatePath("/admin/radio/settings"); + try { + revalidatePath("/admin/radio/settings"); + } catch { + partial = true; + } + redirect( + partial + ? "/admin/radio/settings?partial=1" + : "/admin/radio/settings?saved=1", + ); } // ── Radio banners CRUD (radio_banners) ───────────────────────────────────── diff --git a/src/actions/admin-radio-points.ts b/src/actions/admin-radio-points.ts index 44d9feba..e1c23ec4 100644 --- a/src/actions/admin-radio-points.ts +++ b/src/actions/admin-radio-points.ts @@ -2,93 +2,41 @@ import { revalidatePath } from "next/cache"; import { redirect } from "next/navigation"; +import { executeLegacyHotelMutation } from "@/features/housekeeping/domains/hotel/services/mutations"; import { requirePermission } from "@/lib/admin/guard"; -import { db, WebsiteSetting } from "@/lib/db"; import { PERMS } from "@/lib/permissions"; -import { siteSettings } from "@/lib/services/site-settings"; -import { logStaffActivity } from "@/lib/services/staff-activity"; - -// Radio listener-points settings (website_settings radio_points_* keys). -// Mirrors AtomCMS's RadioPoints Filament page: key/value rows in -// website_settings that reward listeners for time spent on the radio. Booleans -// use the string '0' / '1'. Busts the siteSettings cache so the public radio -// pages pick the change up immediately. - -const POINTS_KEYS = [ - "radio_points_enabled", - "radio_points_per_minute", - "radio_points_currency", - "radio_points_max_per_day", - "radio_points_min_listeners", -] as const; - -const ALLOWED_CURRENCIES = new Set([ - "credits", - "duckets", - "diamonds", - "points", -]); function str(raw: FormDataEntryValue | null): string { return typeof raw === "string" ? raw : ""; } -/** Checkbox/select truthiness → '1' / '0'. */ -function boolStr(raw: FormDataEntryValue | null): "0" | "1" { - const v = str(raw).trim().toLowerCase(); - return v === "1" || v === "true" || v === "on" ? "1" : "0"; -} - -/** Clamp a form value to a non-negative integer string (defaulting to 0). */ -function intStr(raw: FormDataEntryValue | null): string { - const n = Number(str(raw).trim()); - if (!Number.isFinite(n) || n < 0) return "0"; - return String(Math.floor(n)); -} - export async function savePoints(formData: FormData): Promise { const staff = await requirePermission(PERMS.RADIO_EDIT); - - const currencyRaw = str(formData.get("radio_points_currency")) - .trim() - .toLowerCase(); - const currency = ALLOWED_CURRENCIES.has(currencyRaw) - ? currencyRaw - : "credits"; - - const values: Record<(typeof POINTS_KEYS)[number], string> = { - radio_points_enabled: boolStr(formData.get("radio_points_enabled")), - radio_points_per_minute: intStr(formData.get("radio_points_per_minute")), - radio_points_currency: currency, - radio_points_max_per_day: intStr(formData.get("radio_points_max_per_day")), - radio_points_min_listeners: intStr( - formData.get("radio_points_min_listeners"), - ), + const values = { + radio_points_enabled: str(formData.get("radio_points_enabled")), + radio_points_per_minute: str(formData.get("radio_points_per_minute")), + radio_points_currency: str(formData.get("radio_points_currency")), + radio_points_max_per_day: str(formData.get("radio_points_max_per_day")), + radio_points_min_listeners: str(formData.get("radio_points_min_listeners")), }; + let partial = false; try { - await Promise.all( - POINTS_KEYS.map((key) => - db - .insert(WebsiteSetting) - // eslint-disable-next-line security/detect-object-injection -- key from POINTS_KEYS const - .values({ key, value: values[key], comment: "Radio points" }) - .onDuplicateKeyUpdate({ - // eslint-disable-next-line security/detect-object-injection -- key from POINTS_KEYS const - set: { value: values[key] }, - }), - ), + const snapshot = await executeLegacyHotelMutation( + staff, + "radio.points.save", + values, ); - siteSettings.reload(); - await logStaffActivity({ - staffId: staff.id, - action: "radio_points_update", - description: `Updated radio listener-points settings (enabled=${values.radio_points_enabled}, ${values.radio_points_per_minute}/min ${currency})`, - }); + partial = snapshot.completion?.cache === "unavailable"; } catch { - // DB unavailable — fail soft so the action does not throw. + return redirect("/admin/radio/points?error=1"); } - - revalidatePath("/admin/radio/points"); - redirect("/admin/radio/points?saved=1"); + try { + revalidatePath("/admin/radio/points"); + } catch { + partial = true; + } + redirect( + partial ? "/admin/radio/points?partial=1" : "/admin/radio/points?saved=1", + ); } diff --git a/src/actions/admin-radio-settings.test.ts b/src/actions/admin-radio-settings.test.ts new file mode 100644 index 00000000..f524caf5 --- /dev/null +++ b/src/actions/admin-radio-settings.test.ts @@ -0,0 +1,145 @@ +import { revalidatePath } from "next/cache"; +import { redirect } from "next/navigation"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { executeLegacyHotelMutation } from "@/features/housekeeping/domains/hotel/services/mutations"; + +const mocks = vi.hoisted(() => ({ + execute: vi.fn(), + revalidate: vi.fn(), + redirect: vi.fn(), +})); + +vi.mock("next/cache", () => ({ revalidatePath: mocks.revalidate })); +vi.mock("next/navigation", () => ({ redirect: mocks.redirect })); +vi.mock("@/lib/permissions", async () => import("@/lib/permission-slugs")); +vi.mock("@/lib/admin/guard", () => ({ + requirePermission: async () => ({ id: 42, rank: 7, username: "operator" }), +})); +vi.mock("@/features/housekeeping/domains/hotel/services/mutations", () => ({ + executeLegacyHotelMutation: mocks.execute, +})); + +import { saveRadioSetting, saveRadioSettings } from "./admin-radio-extra"; +import { savePoints } from "./admin-radio-points"; + +function form(entries: Record): FormData { + const data = new FormData(); + for (const [key, value] of Object.entries(entries)) data.set(key, value); + return data; +} + +beforeEach(() => { + vi.clearAllMocks(); + mocks.execute.mockResolvedValue({ before: null, after: { saved: true } }); +}); + +describe("legacy radio setting actions", () => { + it("delegates one setting with its useful metadata", async () => { + await saveRadioSetting( + form({ key: "radio_name", value: "Test Radio", comment: "Display name" }), + ); + + expect(executeLegacyHotelMutation).toHaveBeenCalledWith( + expect.objectContaining({ id: 42 }), + "radio.settings.save-one", + { key: "radio_name", value: "Test Radio", comment: "Display name" }, + ); + expect(revalidatePath).toHaveBeenCalledWith("/admin/radio/settings"); + expect(redirect).toHaveBeenCalledWith("/admin/radio/settings?saved=1"); + }); + + it("passes forged bulk keys to strict Hotel validation instead of silently filtering them", async () => { + mocks.execute.mockRejectedValueOnce(new Error("validation")); + + await saveRadioSettings( + form({ + __keys: "radio_name,cms_secret", + radio_name: "Test Radio", + cms_secret: "must-not-write", + }), + ); + + expect(executeLegacyHotelMutation).toHaveBeenCalledWith( + expect.objectContaining({ id: 42 }), + "radio.settings.save-many", + { + entries: [ + { key: "radio_name", value: "Test Radio" }, + { key: "cms_secret", value: "must-not-write" }, + ], + }, + ); + expect(revalidatePath).not.toHaveBeenCalled(); + expect(redirect).toHaveBeenCalledWith("/admin/radio/settings?error=1"); + }); + + it("submits all 102 curated-size settings in one bounded Hotel operation", async () => { + const keys = Array.from( + { length: 102 }, + (_, index) => `radio_key_${index}`, + ); + const data = form({ + __keys: keys.join(","), + ...Object.fromEntries(keys.map((key) => [key, `value-${key}`])), + }); + + await saveRadioSettings(data); + + expect(executeLegacyHotelMutation).toHaveBeenCalledWith( + expect.objectContaining({ id: 42 }), + "radio.settings.save-many", + { + entries: keys.map((key) => ({ key, value: `value-${key}` })), + }, + ); + expect(redirect).toHaveBeenCalledWith("/admin/radio/settings?saved=1"); + }); + + it("shows a partial notice after a committed write whose cache invalidation failed", async () => { + mocks.execute.mockResolvedValueOnce({ + before: null, + after: { key: "radio_name" }, + completion: { + status: "partial", + external: "not-required", + audit: "persisted", + cache: "unavailable", + }, + }); + + await saveRadioSetting(form({ key: "radio_name", value: "Test" })); + + expect(redirect).toHaveBeenCalledWith("/admin/radio/settings?partial=1"); + }); +}); + +describe("legacy radio points action", () => { + const validPoints = { + radio_points_enabled: "1", + radio_points_per_minute: "2", + radio_points_currency: "duckets", + radio_points_max_per_day: "200", + radio_points_min_listeners: "3", + }; + + it("delegates the complete points form to the Hotel operation", async () => { + await savePoints(form(validPoints)); + + expect(executeLegacyHotelMutation).toHaveBeenCalledWith( + expect.objectContaining({ id: 42 }), + "radio.points.save", + validPoints, + ); + expect(redirect).toHaveBeenCalledWith("/admin/radio/points?saved=1"); + }); + + it("never reports saved after a failed write", async () => { + mocks.execute.mockRejectedValueOnce(new Error("database unavailable")); + + await savePoints(form(validPoints)); + + expect(revalidatePath).not.toHaveBeenCalled(); + expect(redirect).toHaveBeenCalledTimes(1); + expect(redirect).toHaveBeenCalledWith("/admin/radio/points?error=1"); + }); +}); diff --git a/src/app/admin/radio/points/page.tsx b/src/app/admin/radio/points/page.tsx index 8247f2a9..56f17fdd 100644 --- a/src/app/admin/radio/points/page.tsx +++ b/src/app/admin/radio/points/page.tsx @@ -19,11 +19,23 @@ const CURRENCIES = ["credits", "duckets", "diamonds", "points"] as const; export default async function AdminRadioPointsPage({ searchParams, }: { - searchParams: Promise<{ saved?: string }>; + searchParams: Promise<{ + saved?: string; + partial?: string; + error?: string; + }>; }) { const t = await getTranslations("pages.admin.radio"); - const { saved } = await searchParams; + const { saved, partial, error } = await searchParams; + const notice = + error === "1" + ? "Radio points settings were not saved. Check the submitted fields and try again." + : partial === "1" + ? "Radio points settings were saved, but cached data could not be refreshed. Refresh the page before saving again." + : saved === "1" + ? t("pointsForm.saved") + : null; const values = { radio_points_enabled: await siteSettings @@ -61,12 +73,10 @@ export default async function AdminRadioPointsPage({ return (
- {saved === "1" ? ( -

- - {t("pointsForm.saved")} - -

+ {notice ? ( +
+

{notice}

+
) : null}
diff --git a/src/app/admin/radio/settings/page.tsx b/src/app/admin/radio/settings/page.tsx index c8e80d76..3d9987f5 100644 --- a/src/app/admin/radio/settings/page.tsx +++ b/src/app/admin/radio/settings/page.tsx @@ -356,8 +356,25 @@ const GROUPS: Group[] = [ const CURATED_KEYS = new Set(GROUPS.flatMap((g) => g.fields.map((f) => f.key))); -export default async function AdminRadioSettingsPage() { +export default async function AdminRadioSettingsPage({ + searchParams, +}: { + searchParams: Promise<{ + saved?: string; + partial?: string; + error?: string; + }>; +}) { const t = await getTranslations("pages.admin.radio"); + const noticeParams = await searchParams; + const notice = + noticeParams.error === "1" + ? "Radio settings were not saved. Check the submitted fields and try again." + : noticeParams.partial === "1" + ? "Radio settings were saved, but cached data could not be refreshed. Refresh the page before saving again." + : noticeParams.saved === "1" + ? "Radio settings saved." + : null; let values: Map = new Map(); let dbError = false; @@ -398,6 +415,11 @@ export default async function AdminRadioSettingsPage() { return (
+ {notice ? ( +
+

{notice}

+
+ ) : null} {dbError ? (

diff --git a/src/features/housekeeping/backend-audit-integrity.test.ts b/src/features/housekeeping/backend-audit-integrity.test.ts index a671bf96..cf631269 100644 --- a/src/features/housekeeping/backend-audit-integrity.test.ts +++ b/src/features/housekeeping/backend-audit-integrity.test.ts @@ -37,6 +37,7 @@ describe("Backend audit reason integrity", () => { before: null, after: { dispatched: true }, }), + invalidateSiteSettings: async () => ({ invalidated: true }), }), async () => capability, ); diff --git a/src/features/housekeeping/domains/hotel/services/mutations-production.test.ts b/src/features/housekeeping/domains/hotel/services/mutations-production.test.ts index 596cc54b..03168694 100644 --- a/src/features/housekeeping/domains/hotel/services/mutations-production.test.ts +++ b/src/features/housekeeping/domains/hotel/services/mutations-production.test.ts @@ -23,6 +23,12 @@ const context = { function dependencies() { const transactionToken = { transaction: true }; + const invalidateSiteSettings = vi.fn( + async (): Promise<{ + invalidated: boolean; + redis: "invalidated" | "unavailable"; + }> => ({ invalidated: true, redis: "invalidated" }), + ); const writeAudit = vi.fn( async (_entry: AuditEntry, _transaction?: unknown) => undefined, ); @@ -36,10 +42,12 @@ function dependencies() { writeAudit, executeOperation, transaction, + invalidateSiteSettings, adapter: createHotelProductionMutationAdapter({ transaction, writeAudit, executeOperation, + invalidateSiteSettings, }), }; } @@ -121,6 +129,78 @@ describe("Hotel production mutation adapter", () => { ); }); + it.each([ + "radio.settings.save-one", + "radio.settings.save-many", + "radio.points.save", + ] as const)( + "invalidates settings only after committed %s", + async (operation) => { + const deps = dependencies(); + const events: string[] = []; + deps.executeOperation.mockImplementation(async (currentOperation) => { + events.push("write"); + return { + before: { operation: currentOperation, state: "before" }, + after: { operation: currentOperation, state: "after" }, + }; + }); + deps.writeAudit.mockImplementation(async () => { + events.push("audit"); + }); + deps.transaction.mockImplementation(async (run) => { + const result = await run(deps.transactionToken); + events.push("commit"); + return result; + }); + deps.invalidateSiteSettings.mockImplementation(async () => { + events.push("invalidate"); + return { invalidated: true, redis: "invalidated" }; + }); + + await deps.adapter.execute(operation, {}, context); + + expect(events).toEqual(["write", "audit", "commit", "invalidate"]); + }, + ); + + it("does not invalidate settings when the radio transaction rolls back", async () => { + const deps = dependencies(); + deps.writeAudit.mockRejectedValueOnce(new Error("audit unavailable")); + + await expect( + deps.adapter.execute("radio.settings.save-one", {}, context), + ).rejects.toThrow("audit unavailable"); + + expect(deps.invalidateSiteSettings).not.toHaveBeenCalled(); + }); + + it("reports cache invalidation failure as partial after the radio write commits", async () => { + const deps = dependencies(); + deps.invalidateSiteSettings.mockResolvedValueOnce({ + invalidated: false, + redis: "unavailable", + }); + + const result = await deps.adapter.execute( + "radio.settings.save-many", + {}, + context, + ); + + expect(result).toMatchObject({ + after: { operation: "radio.settings.save-many" }, + completion: { + status: "partial", + external: "not-required", + audit: "persisted", + cache: "unavailable", + }, + }); + expect(deps.transaction).toHaveBeenCalledTimes(1); + expect(deps.executeOperation).toHaveBeenCalledTimes(1); + }); + it("persists intent and outcome around RCON operations", async () => { const deps = dependencies(); await deps.adapter.execute( diff --git a/src/features/housekeeping/domains/hotel/services/mutations-production.ts b/src/features/housekeeping/domains/hotel/services/mutations-production.ts index d283e7c9..6587b314 100644 --- a/src/features/housekeeping/domains/hotel/services/mutations-production.ts +++ b/src/features/housekeeping/domains/hotel/services/mutations-production.ts @@ -15,6 +15,12 @@ export const HOTEL_EXTERNAL_OPERATIONS = [ "room.runtime", ] as const satisfies readonly HotelMutationOperation[]; +const SITE_SETTINGS_OPERATIONS = new Set([ + "radio.settings.save-one", + "radio.settings.save-many", + "radio.points.save", +]); + type TransactionToken = unknown; export interface HotelProductionMutationDependencies { @@ -28,6 +34,7 @@ export interface HotelProductionMutationDependencies { context: HotelMutationContext, transaction?: TransactionToken, ): Promise; + invalidateSiteSettings(): Promise<{ readonly invalidated: boolean }>; } function auditEntry( @@ -55,7 +62,7 @@ export function createHotelProductionMutationAdapter( return { async execute(operation, input, context) { if (operation !== "room.runtime") { - return dependencies.transaction(async (transaction) => { + const snapshot = await dependencies.transaction(async (transaction) => { const snapshot = await dependencies.executeOperation( operation, input, @@ -68,6 +75,22 @@ export function createHotelProductionMutationAdapter( ); return snapshot; }); + if (!SITE_SETTINGS_OPERATIONS.has(operation)) return snapshot; + try { + const invalidation = await dependencies.invalidateSiteSettings(); + if (invalidation.invalidated) return snapshot; + } catch { + // The transaction is already committed; expose partial completion. + } + return { + ...snapshot, + completion: { + status: "partial", + external: "not-required", + audit: "persisted", + cache: "unavailable", + }, + }; } await dependencies.writeAudit(auditEntry(operation, context, "intent")); @@ -128,4 +151,8 @@ export const hotelProductionMutationAdapter = transaction, ); }, + async invalidateSiteSettings() { + const { siteSettings } = await import("@/lib/services/site-settings"); + return siteSettings.reload(); + }, }); diff --git a/src/features/housekeeping/domains/hotel/services/mutations-runtime-radio.test.ts b/src/features/housekeeping/domains/hotel/services/mutations-runtime-radio.test.ts new file mode 100644 index 00000000..14d5924e --- /dev/null +++ b/src/features/housekeeping/domains/hotel/services/mutations-runtime-radio.test.ts @@ -0,0 +1,165 @@ +import type { SQL } from "drizzle-orm"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import type { HousekeepingCapabilityContext } from "../../../foundation/contracts"; +import { HotelMutationFailure } from "./mutations"; + +const state = vi.hoisted(() => ({ + rows: [] as Array>, + writes: [] as Array>, +})); + +vi.mock("@/lib/db", async () => { + const schema = await import("@/db/schema"); + const database = { + select: vi.fn(() => { + const query = { + from: vi.fn(() => query), + where: vi.fn((_condition: SQL | undefined) => { + const result = Promise.resolve(state.rows) as Promise< + Array> + > & { + limit: (count: number) => Promise>>; + }; + result.limit = vi.fn(() => result); + return result; + }), + }; + return query; + }), + insert: vi.fn(() => ({ + values: (values: Record) => ({ + onDuplicateKeyUpdate: async () => { + state.writes.push(values); + }, + }), + })), + }; + return { ...schema, db: database }; +}); + +import { executeHotelMutationOperation } from "./mutations-runtime"; + +const capability: HousekeepingCapabilityContext = { + actor: { id: 42, username: "operator", rank: 7 }, + isSuperAdmin: false, + has: () => true, + hasAny: () => true, + hasAll: () => true, +}; +const context = { + capability, + correlationId: "radio-runtime", + legacy: true, +}; + +beforeEach(() => { + state.rows = []; + state.writes = []; +}); + +describe("Hotel radio settings runtime", () => { + it("rejects an unrelated setting key before any write", async () => { + await expect( + executeHotelMutationOperation( + "radio.settings.save-one", + { key: "cms_secret", value: "private" }, + context, + ), + ).rejects.toBeInstanceOf(HotelMutationFailure); + expect(state.writes).toEqual([]); + }); + + it("rejects duplicate and invalid bulk entries atomically", async () => { + for (const entries of [ + [ + { key: "radio_name", value: "one" }, + { key: "radio_name", value: "two" }, + ], + [ + { key: "radio_name", value: "one" }, + { key: "site_name", value: "forged" }, + ], + ]) { + await expect( + executeHotelMutationOperation( + "radio.settings.save-many", + { entries }, + context, + ), + ).rejects.toBeInstanceOf(HotelMutationFailure); + expect(state.writes).toEqual([]); + } + }); + + it("accepts the complete 102-entry curated form within the 500-entry bound", async () => { + const entries = Array.from({ length: 102 }, (_, index) => ({ + key: `radio_curated_${index}`, + value: `value-${index}`, + })); + + await executeHotelMutationOperation( + "radio.settings.save-many", + { entries }, + context, + ); + + expect(state.writes).toHaveLength(102); + }); + + it("rejects a bulk request above 500 entries", async () => { + const entries = Array.from({ length: 501 }, (_, index) => ({ + key: `radio_setting_${index}`, + value: "value", + })); + + await expect( + executeHotelMutationOperation( + "radio.settings.save-many", + { entries }, + context, + ), + ).rejects.toBeInstanceOf(HotelMutationFailure); + expect(state.writes).toEqual([]); + }); + + it.each([ + [ + "radio.settings.save-one" as const, + { key: "radio_stream_url", value: "https://private.example/stream" }, + ], + [ + "radio.settings.save-many" as const, + { + entries: [{ key: "radio_name", value: "private station name" }], + }, + ], + [ + "radio.points.save" as const, + { + radio_points_enabled: "1", + radio_points_per_minute: "17", + radio_points_currency: "credits", + radio_points_max_per_day: "34", + radio_points_min_listeners: "2", + }, + ], + ])( + "does not expose private values in %s audit snapshots", + async (operation, input) => { + const snapshot = await executeHotelMutationOperation( + operation, + input, + context, + ); + const serialized = JSON.stringify(snapshot); + for (const privateValue of [ + "https://private.example/stream", + "private station name", + "17", + "34", + ]) { + expect(serialized).not.toContain(privateValue); + } + }, + ); +}); diff --git a/src/features/housekeeping/domains/hotel/services/mutations-runtime.ts b/src/features/housekeeping/domains/hotel/services/mutations-runtime.ts index bb190b05..8dba17db 100644 --- a/src/features/housekeeping/domains/hotel/services/mutations-runtime.ts +++ b/src/features/housekeeping/domains/hotel/services/mutations-runtime.ts @@ -72,12 +72,12 @@ function requireFound( return row; } -function isSensitiveSetting(key: string): boolean { - return /secret|token|password|api[_-]?key|webhook|custom_js/i.test(key); -} - -function settingSnapshot(key: string, value: string) { - return { key, value: isSensitiveSetting(key) ? "[Redacted]" : value }; +function settingSnapshot(key: string, _value: string, comment?: string | null) { + return { + key, + value: "[Redacted]", + ...(comment ? { comment } : {}), + }; } function safeImagePath(value: string): boolean { @@ -127,8 +127,8 @@ const roomRuntimeSchema = z }) .strict(); -const settingKey = requiredText(255).refine( - (key) => key.startsWith("radio_") || key.startsWith("auto_dj_"), +const settingKey = requiredText(255).refine((key) => + /^(?:radio_|auto_dj_)[a-z0-9_]+$/i.test(key), ); const settingSchema = z .object({ @@ -144,7 +144,7 @@ const settingsSchema = z z.object({ key: settingKey, value: z.string().max(20_000) }).strict(), ) .min(1) - .max(100), + .max(500), }) .strict(); const idSchema = z.object({ id: positiveBigInt }).strict(); @@ -368,8 +368,10 @@ async function executeRadioMutation( set: { value: value.value, comment: value.comment || null }, }); return { - before: before ? settingSnapshot(before.key, before.value) : null, - after: settingSnapshot(value.key, value.value), + before: before + ? settingSnapshot(before.key, before.value, before.comment) + : null, + after: settingSnapshot(value.key, value.value, value.comment), }; } if (operation === "radio.settings.save-many") { @@ -627,7 +629,9 @@ async function executeRadioMutation( before: { settings: before.map((entry) => settingSnapshot(entry.key, entry.value)), }, - after: { settings: entries }, + after: { + settings: entries.map((entry) => settingSnapshot(entry.key, entry.value)), + }, }; } diff --git a/src/features/housekeeping/foundation/contracts/result.ts b/src/features/housekeeping/foundation/contracts/result.ts index 1ccbc87d..34b8e956 100644 --- a/src/features/housekeeping/foundation/contracts/result.ts +++ b/src/features/housekeeping/foundation/contracts/result.ts @@ -21,6 +21,7 @@ export interface HousekeepingPartialCompletion { readonly status: "partial"; readonly external: "not-required" | "completed" | "failed"; readonly audit: "persisted" | "unavailable"; + readonly cache?: "invalidated" | "unavailable"; } export type HousekeepingResult = diff --git a/src/lib/services/site-settings.test.ts b/src/lib/services/site-settings.test.ts index 3d80e7e8..95518e9a 100644 --- a/src/lib/services/site-settings.test.ts +++ b/src/lib/services/site-settings.test.ts @@ -1,6 +1,15 @@ -import { beforeEach, describe, expect, it, vi } from "vitest"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +const { selectFrom, redisMocks, loggerMocks } = vi.hoisted(() => ({ + selectFrom: vi.fn(), + redisMocks: { + get: vi.fn(), + setex: vi.fn(), + del: vi.fn(), + }, + loggerMocks: { warn: vi.fn() }, +})); -const { selectFrom } = vi.hoisted(() => ({ selectFrom: vi.fn() })); vi.mock("@/lib/db", () => ({ db: { select: () => ({ @@ -14,27 +23,37 @@ vi.mock("@/lib/db", () => ({ }, WebsiteSetting: { key: "WebsiteSetting.key", value: "WebsiteSetting.value" }, })); +vi.mock("@/lib/redis", () => ({ redis: redisMocks })); +vi.mock("@/lib/logger", () => ({ logger: loggerMocks })); -import { siteSettings } from "./site-settings"; +import { createSiteSettings } from "./site-settings"; -beforeEach(async () => { +beforeEach(() => { selectFrom.mockReset(); - await siteSettings.reload(); + redisMocks.get.mockReset().mockResolvedValue(null); + redisMocks.setex.mockReset().mockResolvedValue("OK"); + redisMocks.del.mockReset().mockResolvedValue(1); + loggerMocks.warn.mockReset(); }); +afterEach(() => vi.useRealTimers()); + describe("siteSettings", () => { it("returns a value by key", async () => { + const siteSettings = createSiteSettings(null); selectFrom.mockResolvedValue([{ key: "hotel_name", value: "AtomHotel" }]); expect(await siteSettings.get("hotel_name")).toBe("AtomHotel"); }); it("returns the fallback when key is missing", async () => { + const siteSettings = createSiteSettings(null); selectFrom.mockResolvedValue([]); expect(await siteSettings.get("missing", "fallback")).toBe("fallback"); expect(await siteSettings.get("missing")).toBeNull(); }); it("coerces '1'/'0' string booleans", async () => { + const siteSettings = createSiteSettings(null); selectFrom.mockResolvedValue([ { key: "maintenance_enabled", value: "1" }, { key: "radio_enabled", value: "0" }, @@ -45,6 +64,7 @@ describe("siteSettings", () => { }); it("fetches once and caches until reload", async () => { + const siteSettings = createSiteSettings(null); selectFrom.mockResolvedValue([{ key: "hotel_name", value: "AtomHotel" }]); await siteSettings.get("hotel_name"); await siteSettings.getBool("hotel_name"); @@ -53,4 +73,124 @@ describe("siteSettings", () => { await siteSettings.get("hotel_name"); expect(selectFrom).toHaveBeenCalledTimes(2); }); + + it("refreshes expired memory from the database when Redis is not configured", async () => { + vi.useFakeTimers(); + vi.setSystemTime(new Date("2026-09-05T08:00:00.000Z")); + const siteSettings = createSiteSettings(null); + selectFrom + .mockResolvedValueOnce([{ key: "hotel_name", value: "Before" }]) + .mockResolvedValueOnce([{ key: "hotel_name", value: "After" }]); + + expect(await siteSettings.get("hotel_name")).toBe("Before"); + vi.advanceTimersByTime(60_001); + expect(await siteSettings.get("hotel_name")).toBe("After"); + expect(selectFrom).toHaveBeenCalledTimes(2); + }); + + it("retains expired memory only when the database refresh fails", async () => { + vi.useFakeTimers(); + vi.setSystemTime(new Date("2026-09-05T08:00:00.000Z")); + const siteSettings = createSiteSettings(null); + selectFrom + .mockResolvedValueOnce([{ key: "hotel_name", value: "Stale" }]) + .mockRejectedValueOnce(new Error("database unavailable")); + + expect(await siteSettings.get("hotel_name")).toBe("Stale"); + vi.advanceTimersByTime(60_001); + expect(await siteSettings.get("hotel_name")).toBe("Stale"); + expect(selectFrom).toHaveBeenCalledTimes(2); + expect(loggerMocks.warn).toHaveBeenCalledWith( + "Failed to load site settings from database, using fallback", + ); + }); + + it("blocks reads during reload and discards stale Redis data from an older flight", async () => { + let resolveRedisRead!: (value: string) => void; + let resolveDelete!: (value: number) => void; + redisMocks.get.mockReturnValueOnce( + new Promise((resolve) => { + resolveRedisRead = resolve; + }), + ); + redisMocks.del.mockReturnValueOnce( + new Promise((resolve) => { + resolveDelete = resolve; + }), + ); + selectFrom.mockResolvedValue([{ key: "hotel_name", value: "Fresh" }]); + const siteSettings = createSiteSettings(redisMocks); + + const oldRead = siteSettings.get("hotel_name"); + await vi.waitFor(() => expect(redisMocks.get).toHaveBeenCalledTimes(1)); + const reload = siteSettings.reload(); + const newRead = siteSettings.get("hotel_name"); + await Promise.resolve(); + expect(redisMocks.get).toHaveBeenCalledTimes(1); + + resolveRedisRead(JSON.stringify({ hotel_name: "Stale Redis" })); + resolveDelete(1); + + expect(await reload).toEqual({ + invalidated: true, + redis: "invalidated", + }); + expect(await oldRead).toBe("Fresh"); + expect(await newRead).toBe("Fresh"); + expect(selectFrom).toHaveBeenCalledTimes(1); + }); + + it("does not let an old flight clear the newer single flight", async () => { + let resolveOldDb!: (rows: Array<{ key: string; value: string }>) => void; + let resolveNewDb!: (rows: Array<{ key: string; value: string }>) => void; + selectFrom + .mockReturnValueOnce( + new Promise((resolve) => { + resolveOldDb = resolve; + }), + ) + .mockReturnValueOnce( + new Promise((resolve) => { + resolveNewDb = resolve; + }), + ); + const siteSettings = createSiteSettings(null); + + const oldRead = siteSettings.get("hotel_name"); + await vi.waitFor(() => expect(selectFrom).toHaveBeenCalledTimes(1)); + await siteSettings.reload(); + const newRead = siteSettings.get("hotel_name"); + await vi.waitFor(() => expect(selectFrom).toHaveBeenCalledTimes(2)); + resolveOldDb([{ key: "hotel_name", value: "Old" }]); + await Promise.resolve(); + const joinedRead = siteSettings.get("hotel_name"); + expect(selectFrom).toHaveBeenCalledTimes(2); + resolveNewDb([{ key: "hotel_name", value: "New" }]); + + expect(await Promise.all([oldRead, newRead, joinedRead])).toEqual([ + "New", + "New", + "New", + ]); + }); + + it("reports Redis invalidation failure without rejection and bypasses stale Redis", async () => { + redisMocks.get.mockResolvedValueOnce( + JSON.stringify({ hotel_name: "Stale Redis" }), + ); + redisMocks.del.mockRejectedValueOnce(new Error("Redis unavailable")); + selectFrom.mockResolvedValue([{ key: "hotel_name", value: "Fresh DB" }]); + const siteSettings = createSiteSettings(redisMocks); + + expect(await siteSettings.get("hotel_name")).toBe("Stale Redis"); + await expect(siteSettings.reload()).resolves.toEqual({ + invalidated: false, + redis: "unavailable", + }); + expect(await siteSettings.get("hotel_name")).toBe("Fresh DB"); + expect(redisMocks.get).toHaveBeenCalledTimes(1); + expect(loggerMocks.warn).toHaveBeenCalledWith( + "Failed to invalidate Redis cache for site settings", + ); + }); }); diff --git a/src/lib/services/site-settings.ts b/src/lib/services/site-settings.ts index a008e288..6e2b0e61 100644 --- a/src/lib/services/site-settings.ts +++ b/src/lib/services/site-settings.ts @@ -13,41 +13,70 @@ const DEFAULTS: Record = { const CACHE_TTL_MS = 300_000; const REDIS_CACHE_KEY = "site_settings"; -// Short in-process window so repeated getters in one request (header, nav, -// footer all read logo and other settings) don't each pay a Redis round-trip. -// Redis stays the source of truth across instances. const MEMORY_TTL_MS = 60_000; -// During `next build`, pages are prerendered and `Date.now()` is treated as an -// unstable prerender value — always serve the in-process cache then (settings -// cannot change mid-build). Runtime keeps the normal TTL check. const IS_PRERENDER = process.env.NEXT_PHASE === "phase-production-build" || process.env.NEXT_PHASE === "phase-production-compile"; +type RedisClient = Pick, "get" | "setex" | "del">; + +export interface SiteSettingsInvalidationResult { + readonly invalidated: boolean; + readonly redis: "invalidated" | "not-configured" | "unavailable"; +} + class SiteSettings { private cache: { map: Map; expiresAt: number } | null = null; - // Single-flight: one request (re)loads the map, the rest await it — the - // root layout reads settings on every render, so concurrent misses must - // not each hammer Redis/DB (cache-stampede protection). - private inFlight: Promise> | null = null; + private inFlight: { + generation: number; + promise: Promise>; + } | null = null; + private invalidationFlight: Promise | null = + null; + private readonly pendingRedisWrites = new Set>(); + private generation = 0; + private bypassRedis = false; + + constructor(private readonly redisClient: RedisClient | null) {} private async loadFromDb(): Promise> { + const rows = await db + .select({ key: WebsiteSetting.key, value: WebsiteSetting.value }) + .from(WebsiteSetting); + return new Map(rows.map((row) => [row.key, row.value])); + } + + private async writeRedis( + map: Map, + generation: number, + ): Promise { + if (!this.redisClient || generation !== this.generation) return; + const write = Promise.resolve( + this.redisClient.setex( + REDIS_CACHE_KEY, + Math.ceil(CACHE_TTL_MS / 1000), + JSON.stringify(Object.fromEntries(map.entries())), + ), + ).then(() => undefined); + this.pendingRedisWrites.add(write); try { - const rows = await db - .select({ key: WebsiteSetting.key, value: WebsiteSetting.value }) - .from(WebsiteSetting); - return new Map(rows.map((r) => [r.key, r.value])); + await write; } catch { - logger.warn("Failed to load site settings from database, using defaults"); - return new Map(Object.entries(DEFAULTS)); + logger.warn("Failed to write site settings to Redis cache"); + } finally { + this.pendingRedisWrites.delete(write); } } - private async loadFromCacheOrDb(): Promise> { - if (redis) { + private async loadForGeneration( + generation: number, + stale: Map | null, + ): Promise> { + if (this.redisClient && !this.bypassRedis) { try { - const cached = await redis.get(REDIS_CACHE_KEY); + const cached = await this.redisClient.get(REDIS_CACHE_KEY); + if (generation !== this.generation) return this.load(); if (cached) { const parsed = JSON.parse(cached) as Record; const map = new Map(Object.entries(parsed)); @@ -59,42 +88,42 @@ class SiteSettings { } } - // Expired but usable fallback — keeps the site up if both Redis and DB fail. - if (this.cache !== null) return this.cache.map; - - const map = await this.loadFromDb(); - this.cache = { map, expiresAt: Date.now() + MEMORY_TTL_MS }; - - if (redis) { - try { - const obj = Object.fromEntries(map.entries()); - await redis.setex( - REDIS_CACHE_KEY, - Math.ceil(CACHE_TTL_MS / 1000), - JSON.stringify(obj), - ); - } catch { - logger.warn("Failed to write site settings to Redis cache"); - } + let map: Map; + try { + map = await this.loadFromDb(); + } catch { + logger.warn("Failed to load site settings from database, using fallback"); + if (generation !== this.generation) return this.load(); + if (stale) return stale; + map = new Map(Object.entries(DEFAULTS)); } + if (generation !== this.generation) return this.load(); + this.cache = { map, expiresAt: Date.now() + MEMORY_TTL_MS }; + await this.writeRedis(map, generation); + if (generation !== this.generation) return this.load(); return map; } private async load(): Promise> { - if (this.cache !== null) { - if (IS_PRERENDER || this.cache.expiresAt > Date.now()) { - return this.cache.map; - } + while (this.invalidationFlight) await this.invalidationFlight; + + if ( + this.cache !== null && + (IS_PRERENDER || this.cache.expiresAt > Date.now()) + ) { + return this.cache.map; } - if (this.inFlight) return this.inFlight; - - const run = this.loadFromCacheOrDb().finally(() => { - this.inFlight = null; + const generation = this.generation; + if (this.inFlight?.generation === generation) return this.inFlight.promise; + const stale = this.cache?.map ?? null; + let promise!: Promise>; + promise = this.loadForGeneration(generation, stale).finally(() => { + if (this.inFlight?.promise === promise) this.inFlight = null; }); - this.inFlight = run; - return run; + this.inFlight = { generation, promise }; + return promise; } async getAll(): Promise> { @@ -123,10 +152,10 @@ class SiteSettings { } async getBool(key: string, fallback = false): Promise { - const v = await this.get(key, null); - if (v === null) return fallback; - const s = v.toLowerCase(); - return s === "1" || s === "true"; + const value = await this.get(key, null); + if (value === null) return fallback; + const normalized = value.toLowerCase(); + return normalized === "1" || normalized === "true"; } async update(key: string, value: string): Promise { @@ -137,17 +166,39 @@ class SiteSettings { await this.reload(); } - async reload(): Promise { + reload(): Promise { + const generation = ++this.generation; this.cache = null; this.inFlight = null; - if (redis) { + this.bypassRedis = true; + const pendingWrites = [...this.pendingRedisWrites]; + let promise!: Promise; + promise = (async (): Promise => { + if (!this.redisClient) { + if (generation === this.generation) this.bypassRedis = false; + return { invalidated: true, redis: "not-configured" }; + } + await Promise.allSettled(pendingWrites); try { - await redis.del(REDIS_CACHE_KEY); + await this.redisClient.del(REDIS_CACHE_KEY); + if (generation === this.generation) this.bypassRedis = false; + return { invalidated: true, redis: "invalidated" }; } catch { logger.warn("Failed to invalidate Redis cache for site settings"); + return { invalidated: false, redis: "unavailable" }; } - } + })().finally(() => { + if (this.invalidationFlight === promise) this.invalidationFlight = null; + }); + this.invalidationFlight = promise; + return promise; } } -export const siteSettings = new SiteSettings(); +export function createSiteSettings( + redisClient: RedisClient | null = redis, +): SiteSettings { + return new SiteSettings(redisClient); +} + +export const siteSettings = createSiteSettings();