From c033786ac3353768e4a8204c42ec0470a18efb55 Mon Sep 17 00:00:00 2001 From: Adam Daley Date: Wed, 24 Dec 2025 00:22:03 +0000 Subject: [PATCH 1/2] Remove error wrapping and verbose docs from Node adapters Simplifies the Node.js SQLite cache and database adapters by removing custom error wrapping and extensive JSDoc comments. Error handling now relies on native exceptions, and test cases are updated to expect generic errors. The Cloudflare KV adapter is also streamlined by removing the binding name and related error checks. --- src/lib/adapters/cloudflare/cache.ts | 11 +- src/lib/adapters/cloudflare/index.ts | 4 +- src/lib/adapters/node/cache.ts | 279 ++------------------------- src/lib/adapters/node/database.ts | 131 ++++--------- src/lib/adapters/node/index.ts | 20 -- test/lib/adapters/node/cache.test.ts | 12 +- 6 files changed, 60 insertions(+), 397 deletions(-) diff --git a/src/lib/adapters/cloudflare/cache.ts b/src/lib/adapters/cloudflare/cache.ts index 83da5af..e620be3 100644 --- a/src/lib/adapters/cloudflare/cache.ts +++ b/src/lib/adapters/cloudflare/cache.ts @@ -1,16 +1,7 @@ import { ICache, CacheOptions } from "../../interfaces"; export class CloudflareKVAdapter implements ICache { - constructor( - private kv: KVNamespace, - private bindingName: string = "UNKNOWN_KV" - ) { - if (!kv) { - throw new Error( - `CloudflareKVAdapter initialized with undefined KVNamespace for binding: ${bindingName}. Check your wrangler.toml or environment variables.` - ); - } - } + constructor(private kv: KVNamespace) {} async get(key: string): Promise { return this.kv.get(key); diff --git a/src/lib/adapters/cloudflare/index.ts b/src/lib/adapters/cloudflare/index.ts index 78332bf..079ee57 100644 --- a/src/lib/adapters/cloudflare/index.ts +++ b/src/lib/adapters/cloudflare/index.ts @@ -11,8 +11,8 @@ export function createCloudflareBindings( DB_CENTRAL_ALERTS: new CloudflareD1Adapter(env.DB_CENTRAL_ALERTS) }, caches: { - CACHE_KV: new CloudflareKVAdapter(env.CACHE_KV, "CACHE_KV"), - AUTH_KV: new CloudflareKVAdapter(env.AUTH_KV, "AUTH_KV") + CACHE_KV: new CloudflareKVAdapter(env.CACHE_KV), + AUTH_KV: new CloudflareKVAdapter(env.AUTH_KV) }, environment: new CloudflareEnvironmentAdapter( env as unknown as Record diff --git a/src/lib/adapters/node/cache.ts b/src/lib/adapters/node/cache.ts index f5ac19f..36220e6 100644 --- a/src/lib/adapters/node/cache.ts +++ b/src/lib/adapters/node/cache.ts @@ -1,27 +1,6 @@ import { DatabaseSync } from "node:sqlite"; import { ICache, CacheOptions } from "../../interfaces"; -/** - * SQLite-based cache adapter for Node.js environments. - * - * Provides persistent or in-memory caching with TTL support using Node.js - * built-in SQLite module. Requires Node.js 22.5+ for node:sqlite support. - * - * Cache entries support two expiration modes: - * - expirationTtl: seconds from now - * - expiration: Unix timestamp (seconds since epoch) - * - * Permanent entries (no expiration) are stored with NULL expire_at. - * - * @example - * ```ts - * import { SQLiteCacheAdapter, createFileCache } from "./cache"; - * - * const cache = createFileCache("./cache.db"); - * await cache.put("key", "value", { expirationTtl: 3600 }); - * const value = await cache.get("key"); // "value" - * ``` - */ export class SQLiteCacheAdapter implements ICache { private db: DatabaseSync; private stmtGet: ReturnType; @@ -29,12 +8,6 @@ export class SQLiteCacheAdapter implements ICache { private stmtDelete: ReturnType; private stmtClearExpired: ReturnType; - /** - * Creates a new SQLite cache adapter. - * - * @param database - A DatabaseSync instance from node:sqlite. Can be - * in-memory (":memory:") or file-based. - */ constructor(database: DatabaseSync) { this.db = database; this.db.exec(` @@ -63,265 +36,49 @@ export class SQLiteCacheAdapter implements ICache { ); } - /** - * Retrieves a value from the cache. - * - * Returns null if the key doesn't exist or has expired. Expired entries - * are not automatically removed on get - use clearExpired() for cleanup. - * - * @param key - The cache key to retrieve - * @returns The cached value or null if not found/expired - * @throws Error if the database operation fails - */ async get(key: string): Promise { - try { - const result = this.stmtGet.get(key, Date.now()) as - | { value: string } - | undefined; - return result?.value ?? null; - } catch (error) { - const message = error instanceof Error ? error.message : String(error); - throw new Error( - `Failed to get cache entry for key "${key}": ${message}`, - error instanceof Error ? { cause: error } : undefined - ); - } + const result = this.stmtGet.get(key, Date.now()) as + | { value: string } + | undefined; + return result?.value ?? null; } - /** - * Stores a value in the cache with optional expiration. - * - * Overwrites existing values. Supports two expiration modes: - * - expirationTtl: seconds from current time - * - expiration: Unix timestamp in seconds - * - * If no expiration options provided, entry persists until manually deleted. - * - * @param key - The cache key - * @param value - The value to store - * @param options - Optional expiration settings - * @throws Error if the database operation fails - */ async put(key: string, value: string, options?: CacheOptions): Promise { - try { - let expireAt: number | null = null; - const now = Date.now(); - - if (options?.expirationTtl) { - expireAt = now + options.expirationTtl * 1000; - } else if (options?.expiration) { - expireAt = options.expiration * 1000; - } + let expireAt: number | null = null; + const now = Date.now(); - this.stmtPut.run(key, value, expireAt); - } catch (error) { - const message = error instanceof Error ? error.message : String(error); - throw new Error( - `Failed to put cache entry for key "${key}": ${message}`, - error instanceof Error ? { cause: error } : undefined - ); + if (options?.expirationTtl) { + expireAt = now + options.expirationTtl * 1000; + } else if (options?.expiration) { + expireAt = options.expiration * 1000; } + + this.stmtPut.run(key, value, expireAt); } - /** - * Removes a value from the cache. - * - * Silently succeeds if key doesn't exist. - * - * @param key - The cache key to delete - * @throws Error if the database operation fails - */ async delete(key: string): Promise { - try { - this.stmtDelete.run(key); - } catch (error) { - const message = error instanceof Error ? error.message : String(error); - throw new Error( - `Failed to delete cache entry for key "${key}": ${message}`, - error instanceof Error ? { cause: error } : undefined - ); - } + this.stmtDelete.run(key); } - /** - * Removes all expired entries from the cache. - * - * Affects only entries with expire_at timestamp in the past. - * Permanent entries (NULL expire_at) are preserved. - * - * @throws Error if the database operation fails - */ clearExpired(): void { - try { - this.stmtClearExpired.run(Date.now()); - } catch (error) { - const message = error instanceof Error ? error.message : String(error); - throw new Error( - `Failed to clear expired cache entries: ${message}`, - error instanceof Error ? { cause: error } : undefined - ); - } + this.stmtClearExpired.run(Date.now()); } - /** - * Determines whether running VACUUM is likely to reclaim a significant - * amount of space. - * - * Uses SQLite PRAGMAs page_count and freelist_count to estimate the ratio - * of free pages. If any error occurs while querying, this method returns - * true to preserve the previous behavior of always vacuuming. - */ - private shouldVacuum(): boolean { - try { - const pageCountRow = this.db.prepare("PRAGMA page_count").get() as - | { page_count: number } - | Record - | undefined; - const freelistRow = this.db.prepare("PRAGMA freelist_count").get() as - | { freelist_count: number } - | Record - | undefined; - - const pageCount = this.extractPragmaValue(pageCountRow, "page_count"); - const freelistCount = this.extractPragmaValue( - freelistRow, - "freelist_count" - ); - - if (pageCount === 0 || freelistCount === 0) { - return false; - } - - const freeRatio = freelistCount / pageCount; - // Only vacuum if at least 20% of pages are free. - return freeRatio >= 0.2; - } catch { - // If we can't determine the freelist, fall back to vacuuming to keep - // behavior close to the original implementation. - return true; - } - } - - /** - * Extracts a numeric value from a PRAGMA result row. - * Handles both named property access and Record fallback. - */ - private extractPragmaValue( - row: { [key: string]: number } | Record | undefined, - propertyName: string - ): number { - if (!row) { - return 0; - } - - // Check if the property exists directly - if (propertyName in row && typeof row[propertyName] === "number") { - return row[propertyName]; - } - - // Fallback: try to get the first numeric value - const values = Object.values(row).filter( - (v): v is number => typeof v === "number" - ); - return values[0] ?? 0; - } - - /** - * Removes all entries from the cache. - * - * Deletes both permanent and expired entries. Optionally performs VACUUM - * to reclaim disk space for file-based databases when it is likely to - * recover a significant amount of free space. - * - * @throws Error if the database operation fails - */ clearAll(): void { - try { - this.db.exec(`DELETE FROM cache`); - if (this.shouldVacuum()) { - this.db.exec(`VACUUM`); - } - } catch (error) { - const message = error instanceof Error ? error.message : String(error); - throw new Error( - `Failed to clear cache: ${message}`, - error instanceof Error ? { cause: error } : undefined - ); - } + this.db.exec(`DELETE FROM cache`); } - /** - * Closes the underlying SQLite database connection. - * - * For file-based databases, this releases file descriptors and ensures - * all pending changes are flushed to disk. After calling this method, - * the cache instance should no longer be used. - * - * @throws Error if closing the database fails - */ close(): void { - try { - this.db.close(); - } catch (error) { - const message = error instanceof Error ? error.message : String(error); - throw new Error( - `Failed to close cache database: ${message}`, - error instanceof Error ? { cause: error } : undefined - ); - } + this.db.close(); } } -/** - * Creates an in-memory SQLite cache adapter. - * - * Cache contents are lost when the process exits. Suitable for testing - * or temporary caching where persistence isn't required. - * - * @returns A new SQLiteCacheAdapter using an in-memory database - * @throws Error if database creation fails - * - * @example - * ```ts - * const cache = createMemoryCache(); - * await cache.put("temp", "data"); - * ``` - */ export function createMemoryCache(): SQLiteCacheAdapter { const db = new DatabaseSync(":memory:"); return new SQLiteCacheAdapter(db); } -/** - * Creates a file-based SQLite cache adapter. - * - * Cache contents persist across process restarts. Creates the database - * file and cache table if they don't exist. The file is created in the - * directory specified by dbPath - ensure the directory exists and is writable. - * - * @param dbPath - Path to the SQLite database file - * @returns A new SQLiteCacheAdapter using a file-based database - * @throws Error if the path is invalid or database creation fails - * - * @example - * ```ts - * const cache = createFileCache("./cache/myapp.db"); - * await cache.put("key", "value", { expirationTtl: 86400 }); - * ``` - */ export function createFileCache(dbPath: string): SQLiteCacheAdapter { - if (typeof dbPath !== "string" || dbPath.trim() === "") { - throw new Error("Invalid database path provided to createFileCache"); - } - - try { - const db = new DatabaseSync(dbPath); - return new SQLiteCacheAdapter(db); - } catch (error) { - const message = error instanceof Error ? error.message : String(error); - throw new Error( - `Failed to create file cache at path "${dbPath}": ${message}`, - error instanceof Error ? { cause: error } : undefined - ); - } + const db = new DatabaseSync(dbPath); + return new SQLiteCacheAdapter(db); } diff --git a/src/lib/adapters/node/database.ts b/src/lib/adapters/node/database.ts index 40d1999..8d5f67f 100644 --- a/src/lib/adapters/node/database.ts +++ b/src/lib/adapters/node/database.ts @@ -1,23 +1,6 @@ import { IDatabase, IPreparedStatement } from "../../interfaces"; -import { DatabaseSync } from "node:sqlite"; - -/** - * SQLite database adapter for Node.js environments. - * - * Provides a database interface using Node.js built-in SQLite module. - * Requires Node.js 22.5+ for node:sqlite support. - * - * @example - * ```ts - * import { DatabaseSync } from "node:sqlite"; - * import { SQLiteAdapter } from "./database"; - * - * const db = new DatabaseSync("mydb.sqlite"); - * const adapter = new SQLiteAdapter(db); - * const stmt = adapter.prepare("SELECT * FROM users WHERE id = ?"); - * const result = await stmt.bind(1).first(); - * ``` - */ +import { DatabaseSync, type SQLInputValue } from "node:sqlite"; + export class SQLiteAdapter implements IDatabase { constructor(private db: DatabaseSync) {} @@ -26,60 +9,46 @@ export class SQLiteAdapter implements IDatabase { } async batch(statements: IPreparedStatement[]): Promise { + this.db.exec("BEGIN IMMEDIATE"); + const results = []; + try { - this.db.exec("BEGIN IMMEDIATE"); - const results = []; - - try { - for (const stmt of statements) { - if (stmt instanceof SQLiteStatement) { - const result = await stmt.run(); - - if (!result.success) { - throw new Error(result.error ?? "Statement execution failed"); - } - - results.push(result); - } else { - throw new Error("Invalid statement type for SQLite batch"); - } - } + for (const stmt of statements) { + if (stmt instanceof SQLiteStatement) { + const result = await stmt.run(); - this.db.exec("COMMIT"); - return results; - } catch (error) { - this.db.exec("ROLLBACK"); - throw error; + results.push(result); + } else { + throw new Error("Invalid statement type for SQLite batch"); + } } + + this.db.exec("COMMIT"); + return results; } catch (error) { - const message = error instanceof Error ? error.message : String(error); - throw new Error( - `Failed to execute batch: ${message}`, - error instanceof Error ? { cause: error } : undefined - ); + this.db.exec("ROLLBACK"); + throw error; } } } +type SQLiteParams = SQLInputValue[]; + class SQLiteStatement implements IPreparedStatement { - private params: unknown[] = []; + private params: SQLiteParams = []; private statement: ReturnType; - constructor( - private db: DatabaseSync, - private query: string - ) { - this.statement = this.db.prepare(this.query); + constructor(db: DatabaseSync, query: string) { + this.statement = db.prepare(query); } bind(...params: unknown[]): IPreparedStatement { - this.params = params; + this.params = params as SQLiteParams; return this; } async all(): Promise<{ results?: T[]; success: boolean }> { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const results = this.statement.all(...(this.params as any[])); + const results = this.statement.all(...this.params); return { results: results as T[], @@ -88,18 +57,8 @@ class SQLiteStatement implements IPreparedStatement { } async first(): Promise { - try { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const result = this.statement.get(...(this.params as any[])); - return (result as T) ?? null; - } catch (error) { - console.error( - "SQLiteStatement.first: failed to execute query", - { query: this.query, params: this.params }, - error - ); - throw error; - } + const result = this.statement.get(...this.params); + return (result as T) ?? null; } async run(): Promise<{ @@ -111,23 +70,15 @@ class SQLiteStatement implements IPreparedStatement { [key: string]: unknown; }; }> { - try { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const result = this.statement.run(...(this.params as any[])); - - return { - success: true, - meta: { - changes: Number(result.changes), - last_row_id: Number(result.lastInsertRowid) - } - }; - } catch (error) { - return { - success: false, - error: error instanceof Error ? error.message : String(error) - }; - } + const result = this.statement.run(...this.params); + + return { + success: true, + meta: { + changes: Number(result.changes), + last_row_id: Number(result.lastInsertRowid) + } + }; } } @@ -136,17 +87,7 @@ export function createInMemoryDatabase(): DatabaseSync { } export function createFileDatabase(filename: string): DatabaseSync { - try { - return new DatabaseSync(filename); - } catch (error) { - const message = error instanceof Error ? error.message : String(error); - throw new Error( - `Failed to create SQLite database at "${filename}": ${message}`, - { - cause: error instanceof Error ? error : undefined - } - ); - } + return new DatabaseSync(filename); } export function createDefaultAdapter(): SQLiteAdapter { diff --git a/src/lib/adapters/node/index.ts b/src/lib/adapters/node/index.ts index 1d4e6f0..5e2f298 100644 --- a/src/lib/adapters/node/index.ts +++ b/src/lib/adapters/node/index.ts @@ -2,17 +2,6 @@ import { IPlatformBindings } from "../../interfaces"; import { createMemoryCache, createFileCache } from "./cache"; import { NodeEnvironmentAdapter } from "./environment"; -/** - * Creates platform bindings for the Node.js environment. - * - * @param cacheDbPath Optional base file path (without extension) for - * persistent cache databases. When provided, two file-backed caches - * are created using `${cacheDbPath}.kv` for general caching and - * `${cacheDbPath}.auth` for auth-related caching. When omitted or - * `undefined`, in-memory caches are used instead. - * @returns Platform bindings configured with either file-backed or - * in-memory cache adapters for CACHE_KV and AUTH_KV. - */ export function createNodeBindings(cacheDbPath?: string): IPlatformBindings { const cacheKv = cacheDbPath ? createFileCache(`${normalizePath(cacheDbPath)}.kv`) @@ -31,18 +20,9 @@ export function createNodeBindings(cacheDbPath?: string): IPlatformBindings { }; } -/** - * Normalizes a file path by removing trailing dots, slashes, and specific - * SQLite-related extensions (".sqlite", ".db", ".sqlite3") to prevent - * malformed paths when appending suffixes. - */ function normalizePath(path: string): string { - // Remove trailing slashes let normalized = path.replace(/\/+$/, ""); - // Then remove trailing dots normalized = normalized.replace(/\.+$/, ""); - - // Remove common extensions if present normalized = normalized.replace(/\.(?:sqlite|db|sqlite3)$/i, ""); return normalized; diff --git a/test/lib/adapters/node/cache.test.ts b/test/lib/adapters/node/cache.test.ts index f758f0c..4e7804f 100644 --- a/test/lib/adapters/node/cache.test.ts +++ b/test/lib/adapters/node/cache.test.ts @@ -79,27 +79,21 @@ describe("SQLiteCacheAdapter - Memory", () => { const db = new DatabaseSync(":memory:"); const badCache = new SQLiteCacheAdapter(db); db.close(); - await expect(badCache.get("key")).rejects.toThrow( - "Failed to get cache entry" - ); + await expect(badCache.get("key")).rejects.toThrow(); }); it("should throw on put error", async () => { const db = new DatabaseSync(":memory:"); const badCache = new SQLiteCacheAdapter(db); db.close(); - await expect(badCache.put("key", "value")).rejects.toThrow( - "Failed to put cache entry" - ); + await expect(badCache.put("key", "value")).rejects.toThrow(); }); it("should throw on delete error", async () => { const db = new DatabaseSync(":memory:"); const badCache = new SQLiteCacheAdapter(db); db.close(); - await expect(badCache.delete("key")).rejects.toThrow( - "Failed to delete cache entry" - ); + await expect(badCache.delete("key")).rejects.toThrow(); }); }); From 865f6ee38967fdcac571a063eb65aed97360a956 Mon Sep 17 00:00:00 2001 From: Adam Daley Date: Wed, 24 Dec 2025 00:27:02 +0000 Subject: [PATCH 2/2] Update token storage instructions in documentation Clarified that the update token should be stored in the AUTH_KV namespace under the key UPDATE_TOKEN, and removed outdated references to UPDATE_TOKEN environment variable. Updated setup steps and endpoint documentation for consistency. --- README.md | 14 ++++++++------ src/services/versions/v1/README.md | 2 +- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index b64d0d4..246cc19 100644 --- a/README.md +++ b/README.md @@ -37,8 +37,6 @@ We've structured the app to separate the core logic from the specific runtime en ### Central Alerts (`/central-alerts/v1`) - `GET /central-alerts/v1/list` - Public endpoint for fetching active alerts. -- `GET /central-alerts/v1/version/:version` - Fetch alerts targeted at a specific FOSSBilling version. -- **Admin Endpoints**: `POST`, `PUT`, `DELETE` exist but require authentication (controlled by the admin interface). ## Configuration @@ -50,12 +48,11 @@ We use [Cloudflare D1](https://developers.cloudflare.com/d1/) and [KV](https://d - **D1 Database** (`DB_CENTRAL_ALERTS`): Stores the alert messages. - **KV Namespace** (`CACHE_KV`): Caches GitHub API responses so we don't hit rate limits. -- **KV Namespace** (`AUTH_KV`): Stores the `update_token` for secured endpoints. +- **KV Namespace** (`AUTH_KV`): Stores the `UPDATE_TOKEN` value for `/versions/v1/update`. ### Environment Variables - `GITHUB_TOKEN`: A GitHub Personal Access Token (classic) with public repo read access. -- `UPDATE_TOKEN`: A secret token you define to secure release cache updates. ## Development @@ -71,7 +68,6 @@ npm install ```env GITHUB_TOKEN="your-token" - UPDATE_TOKEN="dev-secret" ``` 2. Initialize the local D1 database: @@ -80,7 +76,13 @@ npm install npm run init:db ``` -3. Spin up the dev server: +3. (Optional) Store an update token in KV for `/versions/v1/update`: + + ```bash + npx wrangler kv:key put --binding AUTH_KV UPDATE_TOKEN "dev-secret" --local + ``` + +4. Spin up the dev server: ```bash npm run dev ``` diff --git a/src/services/versions/v1/README.md b/src/services/versions/v1/README.md index f8b3328..c126217 100644 --- a/src/services/versions/v1/README.md +++ b/src/services/versions/v1/README.md @@ -148,7 +148,7 @@ GET /versions/v1/update Authorization: Bearer YOUR_UPDATE_TOKEN ``` -The update token must be stored in the `AUTH_KV` namespace under the key `update_token`. +The update token must be stored in the `AUTH_KV` namespace under the key `UPDATE_TOKEN`. **Response:**