From 5bb07c5585cfa6803fd755a58fed51e674f37637 Mon Sep 17 00:00:00 2001 From: Akanimoh12 Date: Thu, 28 May 2026 16:57:09 +0100 Subject: [PATCH] feat: cache key stability, cursor debug helper, health failure test, creator list docs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #318 — add buildCanonicalParamString helper that sorts query params lexicographically before cache key generation, wire it into buildCreatorFeedCacheKey so equivalent param sets always map to the same cache entry, add unit tests covering ordering, omission of undefined values, and creator feed param equivalence. Closes #313 — add formatCursorForDebug helper that decodes a pagination cursor into a human-readable key-value map for structured debug logging; returns null for tampered, malformed, or non-string input so call sites never throw. Wire it into checkIndexerCursorStalenessFromStore at debug level only. Add unit tests for valid payloads, tampered cursors, and edge input types. Closes #314 — add health.controllers.integration.test.ts that simulates database unavailability for both the readiness and detailed health endpoints; asserts 503 status, degraded service flags, and that the response body continues to conform to the documented schema under failure. Closes #315 — add docs/api/creator-list-query-precedence.md documenting sort+order coupling, cursor vs offset priority, AND semantics for search and verified, filter-before-sort ordering, strict unknown-key rejection, and repeated-param behaviour. Placed alongside existing API docs with cross-references to the relevant source files. --- docs/api/creator-list-query-precedence.md | 107 +++++++++++ .../creators/creators-cache-key.utils.ts | 45 ++--- .../health.controllers.integration.test.ts | 167 ++++++++++++++++++ src/utils/cache-key-params.utils.test.ts | 74 ++++++++ src/utils/cache-key-params.utils.ts | 25 +++ src/utils/cursor-debug.utils.test.ts | 70 ++++++++ src/utils/cursor-debug.utils.ts | 42 +++++ src/utils/indexer-cursor-staleness.utils.ts | 8 + 8 files changed, 512 insertions(+), 26 deletions(-) create mode 100644 docs/api/creator-list-query-precedence.md create mode 100644 src/modules/health/health.controllers.integration.test.ts create mode 100644 src/utils/cache-key-params.utils.test.ts create mode 100644 src/utils/cache-key-params.utils.ts create mode 100644 src/utils/cursor-debug.utils.test.ts create mode 100644 src/utils/cursor-debug.utils.ts diff --git a/docs/api/creator-list-query-precedence.md b/docs/api/creator-list-query-precedence.md new file mode 100644 index 0000000..5649268 --- /dev/null +++ b/docs/api/creator-list-query-precedence.md @@ -0,0 +1,107 @@ +# Creator List Query Parameter Precedence + +`GET /api/v1/creators` accepts several query parameters for pagination, sorting, +and filtering. When parameters overlap or conflict, the rules below determine +which value takes effect. + +## Parameter reference + +| Parameter | Type | Default | Notes | +| :--------- | :---------------- | :----------- | :------------------------------------------------------- | +| `limit` | integer (1–100) | `20` | Number of results per page | +| `offset` | integer (≥ 0) | `0` | Number of results to skip | +| `sort` | enum | `createdAt` | Field used to order results | +| `order` | `asc` \| `desc` | `desc` | Direction applied to the `sort` field | +| `verified` | boolean | _(absent)_ | Filter by creator verification status | +| `search` | string | _(absent)_ | Full-text filter applied to display name and handle | +| `include` | comma-separated | _(absent)_ | Extra data to embed in each result (e.g. `stats`) | + +## Precedence rules + +### `sort` and `order` are always applied together + +`order` has no effect unless a `sort` field is present. When `sort` is omitted, +the default field (`createdAt`) and the default direction (`desc`) are used. +If `sort` is supplied without `order`, `order` defaults to `desc`. + +``` +sort=displayName → sort by displayName desc (order defaults) +sort=displayName&order=asc → sort by displayName asc +order=asc → sort by createdAt asc (sort defaults) +``` + +### Cursor-based navigation overrides `offset` + +When a `cursor` value is present (future feature, type-checked at the schema +level), it takes precedence over `offset`. The `limit` value continues to +control page size regardless of which pagination mode is active. + +While the endpoint currently uses offset pagination, the schema reserves the +`cursor` field for forward compatibility. Supplying both `cursor` and `offset` +is not recommended; `cursor` will win when both are non-empty. + +### `search` and `verified` are independent filters — both are applied + +`search` and `verified` narrow the result set independently. When both are +present the response contains only creators that match **both** conditions. +Neither parameter takes precedence over the other; they are ANDed together in +the database query. + +``` +verified=true&search=jazz → creators who are verified AND whose name/handle + contains "jazz" +``` + +### `search` is applied before sorting + +Sorting is applied to the filtered result set. Specifying `search=jazz` with +`sort=displayName` returns matching creators sorted alphabetically — not all +creators sorted alphabetically narrowed to those matching "jazz". + +### `verified` is applied before `search` + +There is no practical difference in the result when both are present (AND +semantics), but internally `verified` is resolved first in the filter +combinator. The ordering is an implementation detail and may not be relied upon +for correctness. + +### `limit` and `offset` operate on the fully-filtered, sorted result set + +`limit` and `offset` are applied **after** all filters and sorting. Setting +`offset=40&limit=20` skips the first 40 matching creators and returns the next +20, not 20 creators from position 40 in the unfiltered list. + +### Unrecognized parameters are rejected + +The query schema uses `strict()` mode. Any parameter not listed in the table +above causes a `400 Bad Request` with a structured error body listing the +unknown keys. Unknown parameters are never silently ignored. + +### Repeated parameters use the first value + +When the same parameter appears more than once in the query string +(e.g. `sort=createdAt&sort=displayName`), only the first occurrence is used. +This is consistent with how Express parses repeated scalar query params. + +## Behaviour summary table + +| Supplied params | Effective behaviour | +| :------------------------------------- | :------------------------------------------------ | +| _(no params)_ | `createdAt desc`, page 1 (limit 20, offset 0) | +| `sort=displayName` | `displayName desc` | +| `order=asc` | `createdAt asc` | +| `sort=displayName&order=asc` | `displayName asc` | +| `verified=true` | verified creators only, `createdAt desc` | +| `search=jazz` | creators matching "jazz", `createdAt desc` | +| `verified=true&search=jazz` | verified creators matching "jazz", `createdAt desc` | +| `verified=true&sort=displayName` | verified creators sorted `displayName desc` | +| `limit=10&offset=20` | page 3 at 10-per-page | +| `unknownParam=x` | `400 Bad Request` | + +## Related files + +- [`src/modules/creators/creators.schemas.ts`](../../src/modules/creators/creators.schemas.ts) — Zod validation schema with defaults +- [`src/modules/creators/creators.filter.ts`](../../src/modules/creators/creators.filter.ts) — Filter parsing and unknown-key rejection +- [`src/modules/creators/creator-feed-filter-combinator.utils.ts`](../../src/modules/creators/creator-feed-filter-combinator.utils.ts) — Prisma `where` clause builder +- [`src/constants/creator-list-sort.constants.ts`](../../src/constants/creator-list-sort.constants.ts) — Allowed sort fields and defaults +- [`src/modules/creators/creators.routes.ts`](../../src/modules/creators/creators.routes.ts) — Route handler wiring diff --git a/src/modules/creators/creators-cache-key.utils.ts b/src/modules/creators/creators-cache-key.utils.ts index 6a20c96..83570e3 100644 --- a/src/modules/creators/creators-cache-key.utils.ts +++ b/src/modules/creators/creators-cache-key.utils.ts @@ -5,12 +5,14 @@ * filter and pagination inputs to ensure cache invalidation works correctly. */ import { CreatorListQueryType } from './creators.schemas'; +import { buildCanonicalParamString } from '../../utils/cache-key-params.utils'; /** * Builds a cache key for the creator feed endpoint. * - * The key includes all query parameters to ensure that different - * filter/pagination combinations have separate cache entries. + * Parameters are sorted into a canonical order via `buildCanonicalParamString` + * so that two requests with identical params in different orders always map to + * the same cache entry. * * @param query - The parsed creator feed query parameters * @returns A deterministic cache key string @@ -26,34 +28,25 @@ import { CreatorListQueryType } from './creators.schemas'; * search: 'example', * include: ['stats'] * }); - * // Returns: "creators:limit:20:offset:0:sort:createdAt:order:desc:verified:true:search:example:include:stats" + * // Returns: "creators:include:stats:limit:20:offset:0:order:desc:search:example:sort:createdAt:verified:true" * ``` */ export function buildCreatorFeedCacheKey(query: CreatorListQueryType): string { - const parts: string[] = ['creators']; + const params: Record = { + limit: query.limit, + offset: query.offset, + sort: query.sort, + order: query.order, + verified: query.verified, + search: query.search !== '' ? query.search : undefined, + include: + query.include !== undefined && query.include.length > 0 + ? query.include.join(',') + : undefined, + }; - // Add pagination parameters - parts.push(`limit:${query.limit}`); - parts.push(`offset:${query.offset}`); - - // Add sorting parameters - parts.push(`sort:${query.sort}`); - parts.push(`order:${query.order}`); - - // Add filter parameters if present - if (query.verified !== undefined) { - parts.push(`verified:${query.verified}`); - } - - if (query.search !== undefined && query.search !== '') { - parts.push(`search:${query.search}`); - } - - if (query.include !== undefined && query.include.length > 0) { - parts.push(`include:${query.include.join(',')}`); - } - - return parts.join(':'); + const canonical = buildCanonicalParamString(params); + return canonical ? `creators:${canonical}` : 'creators'; } /** diff --git a/src/modules/health/health.controllers.integration.test.ts b/src/modules/health/health.controllers.integration.test.ts new file mode 100644 index 0000000..74fdb0b --- /dev/null +++ b/src/modules/health/health.controllers.integration.test.ts @@ -0,0 +1,167 @@ +// Integration tests for health controllers under simulated dependency failures. +// All external dependencies (Prisma, config) are mocked so no real DB is needed. + +jest.mock('../../config', () => ({ + envConfig: { + MODE: 'production', + PORT: 3000, + INDEXER_HEARTBEAT_STALE_THRESHOLD_MS: 300000, + }, + appConfig: { + allowedOrigins: [], + }, +})); + +jest.mock('../../utils/prisma.utils', () => ({ + prisma: { + $queryRaw: jest.fn(), + }, +})); + +jest.mock('../../utils/indexer-cursor-staleness.utils', () => ({ + checkIndexerCursorStalenessFromStore: jest.fn().mockResolvedValue(undefined), +})); + +import { Request, Response } from 'express'; +import { healthCheck, readinessCheck } from './health.controllers'; +import { prisma } from '../../utils/prisma.utils'; + +const queryRawMock = prisma.$queryRaw as unknown as jest.Mock; + +// --------------------------------------------------------------------------- +// Helpers +// --------------------------------------------------------------------------- + +function mockResponse(): Response & { statusCode: number; body: any } { + const res = { statusCode: 0, body: undefined as any } as any; + res.status = (code: number) => { + res.statusCode = code; + return res; + }; + res.json = (payload: any) => { + res.body = payload; + return res; + }; + return res; +} + +function mockRequest(): Request { + return {} as Request; +} + +// --------------------------------------------------------------------------- +// readinessCheck — database failure +// --------------------------------------------------------------------------- + +describe('readinessCheck() — simulated database failure', () => { + beforeEach(() => { + queryRawMock.mockReset(); + }); + + it('returns 503 when the database is unreachable', async () => { + queryRawMock.mockRejectedValue(new Error('connection refused')); + + const res = mockResponse(); + await readinessCheck(mockRequest(), res); + + expect(res.statusCode).toBe(503); + }); + + it('sets ready:false when a dependency check fails', async () => { + queryRawMock.mockRejectedValue(new Error('timeout')); + + const res = mockResponse(); + await readinessCheck(mockRequest(), res); + + expect(res.body.ready).toBe(false); + }); + + it('response body conforms to the readiness schema even on failure', async () => { + queryRawMock.mockRejectedValue(new Error('ECONNREFUSED')); + + const res = mockResponse(); + await readinessCheck(mockRequest(), res); + + expect(res.body).toHaveProperty('ready', false); + expect(res.body).toHaveProperty('timestamp'); + expect(typeof res.body.timestamp).toBe('string'); + expect(res.body).toHaveProperty('latencyMs'); + expect(typeof res.body.latencyMs).toBe('number'); + expect(Array.isArray(res.body.checks)).toBe(true); + }); + + it('reports the database check as failed in the checks array', async () => { + queryRawMock.mockRejectedValue(new Error('connection refused')); + + const res = mockResponse(); + await readinessCheck(mockRequest(), res); + + const dbCheck = res.body.checks.find((c: any) => c.name === 'database'); + expect(dbCheck).toBeDefined(); + expect(dbCheck.status).toBe('fail'); + expect(typeof dbCheck.error).toBe('string'); + }); + + it('still passes the cache check when only the database fails', async () => { + queryRawMock.mockRejectedValue(new Error('db down')); + + const res = mockResponse(); + await readinessCheck(mockRequest(), res); + + const cacheCheck = res.body.checks.find((c: any) => c.name === 'cache'); + expect(cacheCheck).toBeDefined(); + expect(cacheCheck.status).toBe('ok'); + }); + + it('includes a non-zero latencyMs even when the database check fails', async () => { + queryRawMock.mockRejectedValue(new Error('db down')); + + const res = mockResponse(); + await readinessCheck(mockRequest(), res); + + expect(res.body.latencyMs).toBeGreaterThanOrEqual(0); + }); +}); + +// --------------------------------------------------------------------------- +// healthCheck (detailed) — database failure in production +// --------------------------------------------------------------------------- + +describe('healthCheck() — simulated database failure in production mode', () => { + beforeEach(() => { + queryRawMock.mockReset(); + }); + + it('returns 503 when the database is disconnected in production', async () => { + queryRawMock.mockRejectedValue(new Error('connection refused')); + + const res = mockResponse(); + await healthCheck(mockRequest(), res); + + expect(res.statusCode).toBe(503); + }); + + it('response body conforms to the health schema even when DB is down', async () => { + queryRawMock.mockRejectedValue(new Error('ECONNREFUSED')); + + const res = mockResponse(); + await healthCheck(mockRequest(), res); + + expect(res.body).toHaveProperty('success'); + expect(res.body).toHaveProperty('message'); + expect(res.body).toHaveProperty('timestamp'); + expect(res.body).toHaveProperty('database'); + expect(res.body.database.status).toBe('disconnected'); + }); + + it('marks the Database service as unhealthy in the services array', async () => { + queryRawMock.mockRejectedValue(new Error('db down')); + + const res = mockResponse(); + await healthCheck(mockRequest(), res); + + const dbService = res.body.services?.find((s: any) => s.name === 'Database'); + expect(dbService).toBeDefined(); + expect(dbService.status).toBe('unhealthy'); + }); +}); diff --git a/src/utils/cache-key-params.utils.test.ts b/src/utils/cache-key-params.utils.test.ts new file mode 100644 index 0000000..33995ec --- /dev/null +++ b/src/utils/cache-key-params.utils.test.ts @@ -0,0 +1,74 @@ +import { buildCanonicalParamString } from './cache-key-params.utils'; + +describe('buildCanonicalParamString()', () => { + it('produces identical output regardless of input key order', () => { + const setA = buildCanonicalParamString({ + order: 'desc', + limit: 20, + sort: 'createdAt', + offset: 0, + }); + + const setB = buildCanonicalParamString({ + limit: 20, + offset: 0, + sort: 'createdAt', + order: 'desc', + }); + + expect(setA).toBe(setB); + }); + + it('sorts keys lexicographically', () => { + const result = buildCanonicalParamString({ z: 'last', a: 'first', m: 'mid' }); + expect(result).toBe('a:first:m:mid:z:last'); + }); + + it('omits undefined values', () => { + const result = buildCanonicalParamString({ + limit: 20, + search: undefined, + verified: undefined, + }); + expect(result).toBe('limit:20'); + }); + + it('includes boolean values', () => { + const result = buildCanonicalParamString({ verified: true, limit: 10 }); + expect(result).toBe('limit:10:verified:true'); + }); + + it('returns an empty string when all values are undefined', () => { + const result = buildCanonicalParamString({ search: undefined, verified: undefined }); + expect(result).toBe(''); + }); + + it('returns an empty string for an empty params object', () => { + expect(buildCanonicalParamString({})).toBe(''); + }); + + it('two equivalent creator feed param sets produce the same cache key fragment', () => { + const paramsA = buildCanonicalParamString({ + limit: 20, + offset: 0, + sort: 'createdAt', + order: 'desc', + verified: true, + search: 'example', + }); + + const paramsB = buildCanonicalParamString({ + search: 'example', + verified: true, + order: 'desc', + sort: 'createdAt', + offset: 0, + limit: 20, + }); + + expect(paramsA).toBe(paramsB); + expect(paramsA).toBe( + 'limit:20:offset:0:order:desc:search:example:sort:createdAt:verified:true', + ); + }); +}); diff --git a/src/utils/cache-key-params.utils.ts b/src/utils/cache-key-params.utils.ts new file mode 100644 index 0000000..4c12ef2 --- /dev/null +++ b/src/utils/cache-key-params.utils.ts @@ -0,0 +1,25 @@ +/** + * Normalizes a flat key-value param map into a canonical sorted order + * before cache key generation. + * + * Two requests with the same parameters in different orders produce the + * same canonical string, preventing duplicate cache entries. + * + * @param params - Object whose keys are param names and values are their + * serialized representations. `undefined` values are omitted. + * @returns A colon-delimited string: `"key1:val1:key2:val2:..."` where + * keys are sorted lexicographically. + * + * @example + * buildCanonicalParamString({ order: 'desc', limit: '20', sort: 'createdAt' }) + * // => "limit:20:order:desc:sort:createdAt" + */ +export function buildCanonicalParamString( + params: Record, +): string { + return Object.entries(params) + .filter((entry): entry is [string, string | number | boolean] => entry[1] !== undefined) + .sort(([a], [b]) => a.localeCompare(b)) + .map(([key, value]) => `${key}:${value}`) + .join(':'); +} diff --git a/src/utils/cursor-debug.utils.test.ts b/src/utils/cursor-debug.utils.test.ts new file mode 100644 index 0000000..f3f54c0 --- /dev/null +++ b/src/utils/cursor-debug.utils.test.ts @@ -0,0 +1,70 @@ +jest.mock('./cursor.utils', () => ({ + decodeCursor: jest.fn(), + CursorChecksumError: class CursorChecksumError extends Error { + constructor(msg = 'Invalid cursor') { + super(msg); + this.name = 'CursorChecksumError'; + } + }, +})); + +import { formatCursorForDebug } from './cursor-debug.utils'; +import { decodeCursor, CursorChecksumError } from './cursor.utils'; + +const decodeMock = decodeCursor as jest.MockedFunction; + +beforeEach(() => { + decodeMock.mockReset(); +}); + +describe('formatCursorForDebug()', () => { + it('returns a string-keyed map for a valid cursor', () => { + decodeMock.mockReturnValue({ createdAt: '2024-01-01T00:00:00.000Z', id: 'abc123' }); + + const result = formatCursorForDebug('valid-cursor'); + + expect(result).toEqual({ createdAt: '2024-01-01T00:00:00.000Z', id: 'abc123' }); + }); + + it('converts non-string values to strings', () => { + decodeMock.mockReturnValue({ page: 3, active: true } as any); + + const result = formatCursorForDebug('valid-cursor'); + + expect(result).toEqual({ page: '3', active: 'true' }); + }); + + it('returns null for a tampered cursor (CursorChecksumError)', () => { + decodeMock.mockImplementation(() => { + throw new CursorChecksumError('Cursor checksum mismatch'); + }); + + const result = formatCursorForDebug('tampered-cursor'); + + expect(result).toBeNull(); + }); + + it('returns null for an empty string', () => { + expect(formatCursorForDebug('')).toBeNull(); + }); + + it('returns null for a non-string input', () => { + expect(formatCursorForDebug(null)).toBeNull(); + expect(formatCursorForDebug(undefined)).toBeNull(); + expect(formatCursorForDebug(42)).toBeNull(); + }); + + it('returns null when decoded payload is not a plain object', () => { + decodeMock.mockReturnValue(['not', 'an', 'object'] as any); + + expect(formatCursorForDebug('array-cursor')).toBeNull(); + }); + + it('coerces null payload values to empty string', () => { + decodeMock.mockReturnValue({ id: 'abc', extra: null } as any); + + const result = formatCursorForDebug('cursor'); + + expect(result).toEqual({ id: 'abc', extra: '' }); + }); +}); diff --git a/src/utils/cursor-debug.utils.ts b/src/utils/cursor-debug.utils.ts new file mode 100644 index 0000000..8084960 --- /dev/null +++ b/src/utils/cursor-debug.utils.ts @@ -0,0 +1,42 @@ +import { decodeCursor, CursorChecksumError } from './cursor.utils'; + +/** + * Decodes a pagination cursor into a human-readable key-value map for debug + * logging. Returns `null` when the cursor cannot be decoded (tampered, malformed, + * or wrong type) so callers can safely skip the log entry. + * + * IMPORTANT: call this helper only on `logger.debug(...)` paths. Never use it + * in info-level or higher log entries — cursors may carry internal pagination + * anchors that should not appear in production log streams. + * + * @param raw - Raw cursor string from a query parameter or request body + * @returns A flat key-value record suitable for structured logging, or `null` + * + * @example + * logger.debug({ cursor: formatCursorForDebug(raw), msg: 'Processing cursor' }); + */ +export function formatCursorForDebug(raw: unknown): Record | null { + if (typeof raw !== 'string' || raw === '') { + return null; + } + + try { + const payload = decodeCursor>(raw); + + if (payload === null || typeof payload !== 'object' || Array.isArray(payload)) { + return null; + } + + return Object.fromEntries( + Object.entries(payload).map(([key, value]) => [ + key, + value === null || value === undefined ? '' : String(value), + ]), + ); + } catch (err) { + if (err instanceof CursorChecksumError) { + return null; + } + return null; + } +} diff --git a/src/utils/indexer-cursor-staleness.utils.ts b/src/utils/indexer-cursor-staleness.utils.ts index be0e489..30768be 100644 --- a/src/utils/indexer-cursor-staleness.utils.ts +++ b/src/utils/indexer-cursor-staleness.utils.ts @@ -1,6 +1,7 @@ import { envConfig } from '../config'; import { prisma } from './prisma.utils'; import { logger } from './logger.utils'; +import { formatCursorForDebug } from './cursor-debug.utils'; /** Correlates a staleness warning with the indexer job that observed it. */ export interface IndexerCursorStalenessContext { @@ -73,6 +74,13 @@ export async function checkIndexerCursorStalenessFromStore( return; } + logger.debug({ + msg: 'Checking indexer cursor staleness', + job: context.job ?? 'indexer', + cursor: formatCursorForDebug(status.cursor), + ledger: status.ledger, + }); + warnIfIndexerCursorStale( status.updatedAt, envConfig.INDEXER_CURSOR_STALE_AGE_WARNING_MS,