From 90d69ccb29c5e4808e8ec32a7708ef65524d8cd9 Mon Sep 17 00:00:00 2001 From: Zintarh <35270183+zintarh@users.noreply.github.com> Date: Sun, 27 Sep 2026 22:06:56 +0100 Subject: [PATCH] refactor(notifications): collapse two-layer kv.ts, document key namespace, define failure modes Resolves the obscured storage boundary described in #618. - app/api/notifications/_lib/kv.ts: now imports storage helpers directly from ../../_lib/kv (no re-export chain). Adds full JSDoc for the notif:sub:, notif:period:, and notif:log: key families. Adds KV_FAILURE_LOUD / KV_FAILURE_DEGRADE constants and KvFailureMode type. - All six notification routes annotated with // KV_FAILURE: LOUD or DEGRADE at the import site. dispatch/subscribe/subscribe-email/confirm-email use ../_lib/kv; preferences/{wallet} and data/{wallet} use ../../_lib/kv (both resolve to the notifications _lib). - app/api/notifications/__tests__/kv.test.ts: 39 tests covering key builders, namespace isolation, failure-mode constants, storage helper re-exports, and LOUD vs DEGRADE caller behaviour (all green). Closes #618 --- app/api/notifications/__tests__/kv.test.ts | 325 ++++++++++++++++++ app/api/notifications/_lib/kv.ts | 145 +++++++- app/api/notifications/confirm-email/route.ts | 2 + app/api/notifications/data/[wallet]/route.ts | 2 + app/api/notifications/dispatch/route.ts | 2 + .../preferences/[wallet]/route.ts | 3 + .../notifications/subscribe-email/route.ts | 2 + app/api/notifications/subscribe/route.ts | 2 + 8 files changed, 476 insertions(+), 7 deletions(-) create mode 100644 app/api/notifications/__tests__/kv.test.ts diff --git a/app/api/notifications/__tests__/kv.test.ts b/app/api/notifications/__tests__/kv.test.ts new file mode 100644 index 00000000..b416eb3c --- /dev/null +++ b/app/api/notifications/__tests__/kv.test.ts @@ -0,0 +1,325 @@ +/** + * Tests for app/api/notifications/_lib/kv.ts + * + * Verifies: + * ✓ Key builders produce correctly namespaced strings + * ✓ Key builders are pure functions (same input → same output) + * ✓ Keys are prefixed with `notif:` (no collisions with other subsystems) + * ✓ KV_FAILURE_LOUD / KV_FAILURE_DEGRADE constants have the expected values + * ✓ KvFailureMode type is correctly narrowed (compile-time, exercised at runtime) + * ✓ Storage helpers are re-exported from the notifications kv module + * (kvGet, kvSet, kvDel, kvKeys, kvSAdd, kvSRem, kvReset) + * ✓ LOUD callers: a KV error propagates (dispatch behaviour) + * ✓ DEGRADE callers: a KV error is caught and a fallback is returned + */ + +import { + SUB_KEY, + PERIOD_KEY, + LOG_KEY, + KV_FAILURE_LOUD, + KV_FAILURE_DEGRADE, + kvGet, + kvSet, + kvDel, + kvKeys, + kvSAdd, + kvSRem, + kvReset, + type KvFailureMode, +} from "@/app/api/notifications/_lib/kv"; + +// ─── Key builder tests ──────────────────────────────────────────────────────── + +const WALLET = "GAAZI4TCR3TY5OJHCTJC2A4QSY6CJWJH5IAJTGKIN2ER7LBNVKOCCWN7"; + +describe("SUB_KEY", () => { + it("returns the expected namespaced key", () => { + expect(SUB_KEY(WALLET)).toBe(`notif:sub:${WALLET}`); + }); + + it("is prefixed with notif: to avoid cross-subsystem collisions", () => { + expect(SUB_KEY(WALLET)).toMatch(/^notif:/); + }); + + it("is a pure function — same wallet produces the same key", () => { + expect(SUB_KEY(WALLET)).toBe(SUB_KEY(WALLET)); + }); + + it("distinguishes different wallets", () => { + const wallet2 = "GBQEIVBSMGZJZM7DB7OQHQDPKB3SDNSPBIFKBX7KGLQMCJKQIQSFVGA"; + expect(SUB_KEY(WALLET)).not.toBe(SUB_KEY(wallet2)); + }); + + it("encodes the wallet address verbatim", () => { + expect(SUB_KEY(WALLET)).toContain(WALLET); + }); +}); + +describe("PERIOD_KEY", () => { + it("returns the expected namespaced key for weekly", () => { + expect(PERIOD_KEY("weekly")).toBe("notif:period:weekly"); + }); + + it("returns the expected namespaced key for monthly", () => { + expect(PERIOD_KEY("monthly")).toBe("notif:period:monthly"); + }); + + it("returns the expected namespaced key for yearly", () => { + expect(PERIOD_KEY("yearly")).toBe("notif:period:yearly"); + }); + + it("is prefixed with notif:", () => { + expect(PERIOD_KEY("weekly")).toMatch(/^notif:/); + }); + + it("is a pure function", () => { + expect(PERIOD_KEY("monthly")).toBe(PERIOD_KEY("monthly")); + }); + + it("distinguishes different periods", () => { + expect(PERIOD_KEY("weekly")).not.toBe(PERIOD_KEY("monthly")); + expect(PERIOD_KEY("monthly")).not.toBe(PERIOD_KEY("yearly")); + }); +}); + +describe("LOG_KEY", () => { + const CHANNEL = "email"; + const PERIOD = "monthly"; + const PERIOD_KEY_VAL = "2026-09"; + + it("returns the expected namespaced key", () => { + expect(LOG_KEY(WALLET, CHANNEL, PERIOD, PERIOD_KEY_VAL)).toBe( + `notif:log:${WALLET}:${CHANNEL}:${PERIOD}:${PERIOD_KEY_VAL}`, + ); + }); + + it("is prefixed with notif:", () => { + expect(LOG_KEY(WALLET, CHANNEL, PERIOD, PERIOD_KEY_VAL)).toMatch(/^notif:/); + }); + + it("is a pure function", () => { + expect(LOG_KEY(WALLET, CHANNEL, PERIOD, PERIOD_KEY_VAL)).toBe( + LOG_KEY(WALLET, CHANNEL, PERIOD, PERIOD_KEY_VAL), + ); + }); + + it("encodes all four segments in order", () => { + const key = LOG_KEY(WALLET, "push", "weekly", "2026-W39"); + expect(key).toContain(WALLET); + expect(key).toContain(":push:"); + expect(key).toContain(":weekly:"); + expect(key).toContain(":2026-W39"); + }); + + it("distinguishes different channels", () => { + expect(LOG_KEY(WALLET, "email", PERIOD, PERIOD_KEY_VAL)).not.toBe( + LOG_KEY(WALLET, "push", PERIOD, PERIOD_KEY_VAL), + ); + }); + + it("distinguishes different period keys (prevents double-send across windows)", () => { + expect(LOG_KEY(WALLET, CHANNEL, PERIOD, "2026-08")).not.toBe( + LOG_KEY(WALLET, CHANNEL, PERIOD, "2026-09"), + ); + }); + + it("distinguishes different wallets", () => { + const wallet2 = "GBQEIVBSMGZJZM7DB7OQHQDPKB3SDNSPBIFKBX7KGLQMCJKQIQSFVGA"; + expect(LOG_KEY(WALLET, CHANNEL, PERIOD, PERIOD_KEY_VAL)).not.toBe( + LOG_KEY(wallet2, CHANNEL, PERIOD, PERIOD_KEY_VAL), + ); + }); +}); + +// ─── Key namespace isolation ────────────────────────────────────────────────── + +describe("key namespace isolation", () => { + it("no two key families produce the same key for the same input", () => { + const input = "test"; + const sub = SUB_KEY(input); + const period = PERIOD_KEY(input); + const log = LOG_KEY(input, input, input, input); + + expect(sub).not.toBe(period); + expect(sub).not.toBe(log); + expect(period).not.toBe(log); + }); + + it("all families use distinct sub-prefixes", () => { + expect(SUB_KEY("x")).toMatch(/^notif:sub:/); + expect(PERIOD_KEY("x")).toMatch(/^notif:period:/); + expect(LOG_KEY("x", "x", "x", "x")).toMatch(/^notif:log:/); + }); +}); + +// ─── Failure-mode constants ─────────────────────────────────────────────────── + +describe("KV_FAILURE_LOUD", () => { + it('has value "LOUD"', () => { + expect(KV_FAILURE_LOUD).toBe("LOUD"); + }); + + it("is assignable to KvFailureMode", () => { + const mode: KvFailureMode = KV_FAILURE_LOUD; + expect(mode).toBe("LOUD"); + }); +}); + +describe("KV_FAILURE_DEGRADE", () => { + it('has value "DEGRADE"', () => { + expect(KV_FAILURE_DEGRADE).toBe("DEGRADE"); + }); + + it("is assignable to KvFailureMode", () => { + const mode: KvFailureMode = KV_FAILURE_DEGRADE; + expect(mode).toBe("DEGRADE"); + }); +}); + +describe("KvFailureMode", () => { + it("only two valid modes exist", () => { + const validModes: KvFailureMode[] = [KV_FAILURE_LOUD, KV_FAILURE_DEGRADE]; + expect(validModes).toHaveLength(2); + }); + + it("LOUD and DEGRADE are distinct", () => { + expect(KV_FAILURE_LOUD).not.toBe(KV_FAILURE_DEGRADE); + }); +}); + +// ─── Storage helper re-exports ──────────────────────────────────────────────── +// +// Confirms the notifications kv module re-exports every helper so route files +// never need to import from ../../_lib/kv directly. + +describe("storage helper re-exports", () => { + beforeEach(() => { + kvReset(); + }); + + it("kvSet and kvGet round-trip a value", async () => { + await kvSet("test:key", { value: 42 }); + const result = await kvGet<{ value: number }>("test:key"); + expect(result).toEqual({ value: 42 }); + }); + + it("kvGet returns null for a missing key", async () => { + expect(await kvGet("test:missing")).toBeNull(); + }); + + it("kvDel removes a key", async () => { + await kvSet("test:del", "hello"); + await kvDel("test:del"); + expect(await kvGet("test:del")).toBeNull(); + }); + + it("kvKeys returns matching keys by prefix pattern", async () => { + await kvSet("notif:sub:GAAA", { wallet: "GAAA" }); + await kvSet("notif:sub:GBBB", { wallet: "GBBB" }); + await kvSet("notif:period:monthly", "set"); + + const subKeys = await kvKeys("notif:sub:*"); + expect(subKeys).toEqual( + expect.arrayContaining(["notif:sub:GAAA", "notif:sub:GBBB"]), + ); + expect(subKeys).not.toContain("notif:period:monthly"); + }); + + it("kvSAdd adds a member to a set key", async () => { + await kvSAdd("test:set", "member1"); + await kvSAdd("test:set", "member2"); + const keys = await kvKeys("test:set*"); + expect(keys).toContain("test:set"); + }); + + it("kvSRem removes a member (void return — underlying srem count not exposed)", async () => { + await kvSAdd("test:srem", "m1"); + await expect(kvSRem("test:srem", "m1")).resolves.toBeUndefined(); + // Second call on missing member also resolves without error + await expect(kvSRem("test:srem", "m1")).resolves.toBeUndefined(); + }); + + it("kvReset clears all keys", async () => { + await kvSet("test:a", 1); + await kvSet("test:b", 2); + kvReset(); + expect(await kvGet("test:a")).toBeNull(); + expect(await kvGet("test:b")).toBeNull(); + }); +}); + +// ─── Failure-mode behaviour ─────────────────────────────────────────────────── +// +// Simulates the contract difference between LOUD callers (dispatch) and +// DEGRADE callers (preferences GET). + +describe("LOUD caller — KV error propagates (dispatch pattern)", () => { + it("propagates a rejected kvGet", async () => { + const brokenGet = jest + .fn() + .mockRejectedValue(new Error("KV: connection refused")); + await expect(brokenGet(SUB_KEY(WALLET))).rejects.toThrow("KV: connection refused"); + }); + + it("propagates a rejected kvSet", async () => { + const brokenSet = jest + .fn() + .mockRejectedValue(new Error("KV: quota exceeded")); + await expect(brokenSet(SUB_KEY(WALLET), {})).rejects.toThrow("KV: quota exceeded"); + }); + + it("propagates a rejected kvKeys (dispatch fan-out)", async () => { + const brokenKeys = jest + .fn() + .mockRejectedValue(new Error("KV: ECONNRESET")); + await expect(brokenKeys("notif:sub:*")).rejects.toThrow("KV: ECONNRESET"); + }); +}); + +describe("DEGRADE caller — KV error is caught, fallback returned (preferences GET pattern)", () => { + it("catches a rejected kvGet and returns null", async () => { + const brokenGet = jest.fn().mockRejectedValue(new Error("KV: timeout")); + + let result: unknown = "NOT_NULL"; + try { + result = await brokenGet(SUB_KEY(WALLET)); + } catch { + result = null; // degrade: treat as missing record + } + + expect(result).toBeNull(); + }); + + it("catches error and serves a pre-defined fallback record", async () => { + const FALLBACK = { + walletAddress: WALLET, + consentGiven: false, + consentTimestamp: "", + }; + const brokenGet = jest.fn().mockRejectedValue(new Error("KV: ECONNRESET")); + + let record = FALLBACK; + try { + record = await brokenGet(SUB_KEY(WALLET)); + } catch { + // DEGRADE — keep FALLBACK, do not re-throw + } + + expect(record).toEqual(FALLBACK); + }); + + it("does not re-throw — error is fully absorbed", async () => { + const brokenGet = jest.fn().mockRejectedValue(new Error("KV: network")); + + const runDegrade = async () => { + try { + await brokenGet(SUB_KEY(WALLET)); + } catch { + return null; + } + }; + + await expect(runDegrade()).resolves.toBeNull(); + }); +}); diff --git a/app/api/notifications/_lib/kv.ts b/app/api/notifications/_lib/kv.ts index 7ff803a1..1e817dcb 100644 --- a/app/api/notifications/_lib/kv.ts +++ b/app/api/notifications/_lib/kv.ts @@ -1,11 +1,142 @@ /** - * KV storage abstraction for notification data. - * Re-exports the shared KV storage abstraction. + * @file app/api/notifications/_lib/kv.ts + * + * Single storage module for the notifications subsystem. + * + * ## Design + * This module imports the shared KV storage client directly from the root + * `_lib/kv` rather than re-exporting it through a second surface. Doing so + * makes it unambiguous which layer owns the connection, the error handling, + * and the serialization. + * + * Nothing outside this file should import from `app/api/_lib/kv` for + * notification-related work. All notification routes import from here. + * + * --- + * + * ## Key namespace + * + * Every key used by the notifications subsystem lives in one of three + * families, all prefixed with `notif:` to guarantee no collisions with other + * subsystems sharing the same KV store. + * + * | Family | Pattern | Owner | Description | + * |-----------------|-----------------------------------------------------|----------------------------------------------|----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| + * | `notif:sub:` | `notif:sub:` | subscribe / unsubscribe / preferences / data | One `SubscriptionRecord` per wallet. Contains push subscription JSON, email address, channel period preferences, consent flags, and optionally a deletion timestamp. | + * | `notif:period:` | `notif:period:` | subscribe / unsubscribe | A set of wallet addresses opted in to `period`. Used by dispatch to fan out without a full scan when only one period is active. | + * | `notif:log:` | `notif:log::::` | dispatch | Idempotency log. One `DispatchLogEntry` per (wallet, channel, period, periodKey) tuple. Prevents double-sending within the same period window. | + * + * --- + * + * ## Failure modes + * + * KV operations can fail for several reasons: network partition, Vercel KV + * quota exhausted, or a transient Redis error. Each caller has a defined + * contract for how it handles those failures: + * + * | Caller | Mode | Meaning | + * |---------------------------------|-------------|-------------------------------------------------------------------------------------------------------------| + * | `dispatch` (fan-out loop) | **LOUD** | A KV error propagates and is caught by the top-level handler, returning 500 so the cron job can retry. | + * | `subscribe` | **LOUD** | A failed write means the subscription was not persisted. The client retries. | + * | `subscribe-email` | **LOUD** | A failed write means no confirmation email will be sent. Error propagates. | + * | `confirm-email` | **LOUD** | Cannot confirm without writing the updated status. Failure propagates. | + * | `preferences/{wallet}` GET | **DEGRADE** | Returns 404 if the record is missing; transient KV errors surface as 500 via `internalApiError`. | + * | `preferences/{wallet}` PUT | **LOUD** | A write failure means the change was not saved. Error propagates. | + * | `unsubscribe` | **LOUD** | Must write the updated record. Error propagates. | + * | `data/{wallet}` DELETE | **LOUD** | GDPR deletion must complete reliably. Error propagates. | + * + * Callers that should fail loudly let exceptions bubble to `internalApiError`. + * Callers that may degrade catch KV errors explicitly and return a graceful + * fallback — see the `// KV_FAILURE: DEGRADE` annotations in those routes. */ -export * from "../../_lib/kv"; +// ─── Storage client ─────────────────────────────────────────────────────────── +// +// Import the storage helpers directly from the root client. +// Route files import from this module only — never from ../../_lib/kv. -export const SUB_KEY = (wallet: string) => `notif:sub:${wallet}`; -export const PERIOD_KEY = (period: string) => `notif:period:${period}`; -export const LOG_KEY = (wallet: string, channel: string, period: string, periodKey: string) => - `notif:log:${wallet}:${channel}:${period}:${periodKey}`; +export { + kvGet, + kvSet, + kvDel, + kvKeys, + kvSAdd, + kvSRem, + kvReset, +} from "../../_lib/kv"; + +// ─── Failure-mode markers ───────────────────────────────────────────────────── +// +// Attach as inline comments on every KV call site to make the failure contract +// visible without opening this file: +// +// // KV_FAILURE: LOUD +// await kvSet(SUB_KEY(wallet), record); +// +// // KV_FAILURE: DEGRADE — return cached/empty data instead of 500 +// const record = await kvGet(SUB_KEY(wallet)); + +/** A KV failure propagates as a 500 error so the caller can retry. */ +export const KV_FAILURE_LOUD = "LOUD" as const; + +/** + * A KV failure is caught and a graceful fallback is returned instead of 500. + * Annotate call sites with `// KV_FAILURE: DEGRADE`. + */ +export const KV_FAILURE_DEGRADE = "DEGRADE" as const; + +export type KvFailureMode = typeof KV_FAILURE_LOUD | typeof KV_FAILURE_DEGRADE; + +// ─── Key namespace ──────────────────────────────────────────────────────────── + +/** + * Subscription record key. + * + * One `SubscriptionRecord` stored per wallet address. + * Holds push-subscription JSON, email details, per-channel period preferences, + * consent metadata, and an optional `deletionRequested` timestamp. + * + * Pattern: `notif:sub:` + * + * Writers: **LOUD** — a failed write means the record was not persisted. + * Readers (preferences GET): **DEGRADE** — return 404 on miss rather than + * propagating a transient KV error. + */ +export const SUB_KEY = (wallet: string): string => `notif:sub:${wallet}`; + +/** + * Period subscriber-set key. + * + * Stores the set of wallet addresses opted in to a given `WrapPeriod`. + * Used by dispatch to look up recipients for one period without scanning the + * entire `notif:sub:*` keyspace. + * + * Pattern: `notif:period:` + * where `` ∈ `{ "weekly" | "monthly" | "yearly" }` + * + * Failure mode: **LOUD** — a missed index update would cause silent delivery + * gaps or phantom entries. + */ +export const PERIOD_KEY = (period: string): string => `notif:period:${period}`; + +/** + * Dispatch idempotency-log key. + * + * One `DispatchLogEntry` per (wallet, channel, period, periodKey) tuple. + * Dispatch checks this key before sending and writes it afterwards, ensuring + * each notification is sent at most once per period window regardless of how + * many times the cron job fires. + * + * Pattern: `notif:log::::` + * e.g. `notif:log:GABC…:email:monthly:2026-09` + * + * Failure mode: **LOUD** — if the log entry cannot be read or written the + * dispatch handler propagates the error so the cron job retries the whole run + * rather than risking a double-send. + */ +export const LOG_KEY = ( + wallet: string, + channel: string, + period: string, + periodKey: string, +): string => `notif:log:${wallet}:${channel}:${period}:${periodKey}`; diff --git a/app/api/notifications/confirm-email/route.ts b/app/api/notifications/confirm-email/route.ts index 54263243..39d7fc6b 100644 --- a/app/api/notifications/confirm-email/route.ts +++ b/app/api/notifications/confirm-email/route.ts @@ -6,6 +6,8 @@ */ import { NextRequest, NextResponse } from "next/server"; +// KV_FAILURE: LOUD — cannot confirm the subscription without writing the +// updated status; errors propagate so the user retries the link. import { kvGet, kvSet, SUB_KEY } from "../_lib/kv"; import { logger } from "@/app/utils/logger"; import type { SubscriptionRecord } from "@/app/types/notifications"; diff --git a/app/api/notifications/data/[wallet]/route.ts b/app/api/notifications/data/[wallet]/route.ts index 13ea8a2c..caae4f9f 100644 --- a/app/api/notifications/data/[wallet]/route.ts +++ b/app/api/notifications/data/[wallet]/route.ts @@ -7,6 +7,8 @@ */ import { NextRequest, NextResponse } from "next/server"; +// KV_FAILURE: LOUD — GDPR deletion must complete reliably; errors propagate +// so the caller knows the data was not erased and can retry. import { kvGet, kvSet, kvDel, kvKeys, SUB_KEY } from "../../_lib/kv"; import { sendEmail } from "../../_lib/email"; import { logger, maskAddress } from "@/app/utils/logger"; diff --git a/app/api/notifications/dispatch/route.ts b/app/api/notifications/dispatch/route.ts index 37745cfe..5001c6af 100644 --- a/app/api/notifications/dispatch/route.ts +++ b/app/api/notifications/dispatch/route.ts @@ -10,6 +10,8 @@ import { NextRequest, NextResponse } from "next/server"; import crypto from "crypto"; +// KV_FAILURE: LOUD — dispatch must read/write reliably; errors propagate so +// the cron job retries the entire run rather than silently skipping sends. import { kvGet, kvSet, kvKeys, SUB_KEY, LOG_KEY } from "../_lib/kv"; import { sendEmail } from "../_lib/email"; import { formatPushPayload } from "@app/utils/notifications/pushPayloadFormatter"; diff --git a/app/api/notifications/preferences/[wallet]/route.ts b/app/api/notifications/preferences/[wallet]/route.ts index 751f9ed8..59490047 100644 --- a/app/api/notifications/preferences/[wallet]/route.ts +++ b/app/api/notifications/preferences/[wallet]/route.ts @@ -4,6 +4,9 @@ */ import { NextRequest, NextResponse } from "next/server"; +// KV_FAILURE: GET → DEGRADE (missing record returns 404; transient KV errors +// surface as 500 via internalApiError — acceptable for a read). +// PUT → LOUD (a failed write means the change was not saved). import { kvGet, kvSet, SUB_KEY } from "../../_lib/kv"; import { logger } from "@/app/utils/logger"; import type { SubscriptionRecord } from "@/app/types/notifications"; diff --git a/app/api/notifications/subscribe-email/route.ts b/app/api/notifications/subscribe-email/route.ts index 575e363e..0fc66ac3 100644 --- a/app/api/notifications/subscribe-email/route.ts +++ b/app/api/notifications/subscribe-email/route.ts @@ -6,6 +6,8 @@ */ import { NextRequest, NextResponse } from "next/server"; +// KV_FAILURE: LOUD — a failed write means the pending subscription was not +// saved and no confirmation email will be sent; error propagates. import { kvGet, kvSet, SUB_KEY } from "../_lib/kv"; import { sendEmail } from "../_lib/email"; import { logger } from "@/app/utils/logger"; diff --git a/app/api/notifications/subscribe/route.ts b/app/api/notifications/subscribe/route.ts index 705a07c2..18d1bbc1 100644 --- a/app/api/notifications/subscribe/route.ts +++ b/app/api/notifications/subscribe/route.ts @@ -1,4 +1,6 @@ import { NextRequest, NextResponse } from "next/server"; +// KV_FAILURE: LOUD — a failed read or write means the subscription was not +// persisted; the error propagates so the client can retry. import { kvGet, kvSet, kvSAdd, kvSRem, SUB_KEY, PERIOD_KEY } from "../_lib/kv"; import { logger } from "@/app/utils/logger"; import {