Skip to content

General refactor and cleanup #31

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
admdly merged 2 commits into from
Dec 24, 2025
Merged

General refactor and cleanup #31

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c033786
Remove error wrapping and verbose docs from Node adapters
admdly Dec 24, 2025
865f6ee
Update token storage instructions in documentation
admdly Dec 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 8 additions & 6 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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

Expand All @@ -71,7 +68,6 @@ npm install

```env
GITHUB_TOKEN="your-token"
UPDATE_TOKEN="dev-secret"
```

2. Initialize the local D1 database:
Expand All @@ -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
```
Expand Down
11 changes: 1 addition & 10 deletions src/lib/adapters/cloudflare/cache.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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) {}
Copy link

Copilot AI Dec 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The validation that checked for undefined KVNamespace and provided a helpful error message has been removed. If env.CACHE_KV or env.AUTH_KV are undefined (due to misconfiguration), the code will now fail with a less informative error when trying to call methods on undefined. The previous implementation provided clear guidance about checking wrangler.toml or environment variables, which would help developers diagnose configuration issues more quickly.

Suggested change
constructor(private kv: KVNamespace) {}
constructor(private kv: KVNamespace) {
if (!kv) {
throw new Error(
"CloudflareKVAdapter: KVNamespace is undefined. " +
"Ensure that the KV binding (e.g. CACHE_KV or AUTH_KV) is correctly configured in your wrangler.toml or environment variables."
);
}
}

Copilot uses AI. Check for mistakes.

async get(key: string): Promise<string | null> {
return this.kv.get(key);
Expand Down
4 changes: 2 additions & 2 deletions src/lib/adapters/cloudflare/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, unknown>
Expand Down
279 changes: 18 additions & 261 deletions src/lib/adapters/node/cache.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,40 +1,13 @@
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<DatabaseSync["prepare"]>;
private stmtPut: ReturnType<DatabaseSync["prepare"]>;
private stmtDelete: ReturnType<DatabaseSync["prepare"]>;
private stmtClearExpired: ReturnType<DatabaseSync["prepare"]>;

/**
* 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(`
Expand Down Expand Up @@ -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<string | null> {
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<void> {
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<void> {
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<string, number>
| undefined;
const freelistRow = this.db.prepare("PRAGMA freelist_count").get() as
| { freelist_count: number }
| Record<string, number>
| 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<string, number> fallback.
*/
private extractPragmaValue(
row: { [key: string]: number } | Record<string, number> | 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`);
}

Comment on lines +68 to 70
Copy link

Copilot AI Dec 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The VACUUM operation has been removed from clearAll(). The previous implementation would run VACUUM conditionally based on the ratio of free pages to total pages (when >= 20% free) to reclaim disk space in file-based databases. Without VACUUM, file-based cache databases will grow over time and never shrink, even after clearing all entries. This is particularly important for long-running applications. Consider either restoring the VACUUM call or documenting this behavior change.

Suggested change
this.db.exec(`DELETE FROM cache`);
}
this.db.exec(`DELETE FROM cache`);
this.vacuumIfNeeded();
}
private vacuumIfNeeded(): void {
try {
const pageCountRow = this.db.prepare(`PRAGMA page_count;`).get() as
| Record<string, unknown>
| undefined;
const freeListRow = this.db.prepare(`PRAGMA freelist_count;`).get() as
| Record<string, unknown>
| undefined;
const extractNumber = (row: Record<string, unknown> | undefined, key: string): number => {
if (!row) {
return 0;
}
const direct = row[key];
if (typeof direct === "number") {
return direct;
}
const firstValue = Object.values(row).find((v) => typeof v === "number");
return typeof firstValue === "number" ? firstValue : 0;
};
const totalPages = extractNumber(pageCountRow, "page_count");
const freePages = extractNumber(freeListRow, "freelist_count");
if (totalPages > 0) {
const freeRatio = freePages / totalPages;
if (freeRatio >= 0.2) {
this.db.exec(`VACUUM`);
}
}
} catch {
// Best-effort VACUUM; ignore any errors to avoid impacting cache operations.
}
}

Copilot uses AI. Check for mistakes.
/**
* 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();
}
Comment on lines 39 to 73
Copy link

Copilot AI Dec 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The SQLiteCacheAdapter constructor and methods can now throw exceptions from the underlying DatabaseSync operations without wrapping them in more descriptive error messages. In the previous implementation, errors included context like which key failed, the operation type (get/put/delete), and the underlying error message. Without this context, debugging issues in production will be more difficult. For example, if a database operation fails due to disk space or permissions, the error won't indicate which cache operation or key was involved.

Copilot uses AI. Check for mistakes.
}

/**
* 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 {
Copy link

Copilot AI Dec 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The createFileCache function no longer validates that dbPath is a non-empty string before attempting to create the database. If an empty string or whitespace-only path is passed, DatabaseSync will throw a less helpful error. The previous validation provided clear, immediate feedback about invalid input at the function boundary.

Suggested change
export function createFileCache(dbPath: string): SQLiteCacheAdapter {
export function createFileCache(dbPath: string): SQLiteCacheAdapter {
if (typeof dbPath !== "string" || dbPath.trim() === "") {
throw new TypeError("createFileCache: dbPath must be a non-empty string");
}

Copilot uses AI. Check for mistakes.
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);
}
Loading