333 lines
11 KiB
Markdown
333 lines
11 KiB
Markdown
# Cache package – best practices
|
||
|
||
Working document. Append new lessons as they arise.
|
||
|
||
---
|
||
|
||
## 1. Extract shared logic into focused, single-responsibility files
|
||
|
||
When two or more providers (KV, Redis, InMemory) share identical logic, extract it
|
||
into a dedicated file rather than duplicating it. Each extracted file should own
|
||
exactly one concern:
|
||
|
||
| File | Concern |
|
||
|---|---|
|
||
| `CacheProviderTypes.tsx` | Shared interfaces consumed by providers and callers (`CacheLogger`, `CacheTelemetry`, `CacheKeyClassifier`) |
|
||
| `CacheSerialization.tsx` | JSON parse/stringify helpers with error handling and truncation |
|
||
| `CacheLockValidation.tsx` | Lock key/token format validation, token generation, key formatting |
|
||
| `CacheKeyClassification.tsx` | Mapping a cache key to a telemetry dimension string |
|
||
| `RedisClientTypes.tsx` | `RedisClient` and `RedisPipeline` interface contracts |
|
||
|
||
**Why:** duplicated code drifts apart over time, bugs get fixed in one copy but not
|
||
another, and readers have to mentally diff near-identical methods.
|
||
|
||
---
|
||
|
||
## 2. Separate public contract types from implementation files
|
||
|
||
Types that callers or tests need to import (`CacheLogger`, `CacheTelemetry`,
|
||
`RedisClient`) belong in their own files – not buried inside a provider class.
|
||
|
||
**Rule of thumb:** if a type appears in an `import type` statement in a file that is
|
||
not the defining file, it deserves its own home.
|
||
|
||
**Before:**
|
||
```tsx
|
||
// KVCacheProvider.tsx defines AND exports CacheLogger
|
||
export interface CacheLogger { ... }
|
||
export class KVCacheProvider { ... }
|
||
```
|
||
|
||
**After:**
|
||
```tsx
|
||
// CacheProviderTypes.tsx – sole owner of the type
|
||
export interface CacheLogger { ... }
|
||
|
||
// KVCacheProvider.tsx – imports the type
|
||
import type {CacheLogger} from '@fluxer/cache/src/CacheProviderTypes';
|
||
```
|
||
|
||
---
|
||
|
||
## 3. Use an `instrumented` helper to collapse telemetry boilerplate
|
||
|
||
When every method in a class follows the same start/try/record-success/catch/record-error
|
||
pattern, extract it into a single private method:
|
||
|
||
```tsx
|
||
private async instrumented<T>(
|
||
operation: string,
|
||
key: string,
|
||
fn: () => Promise<T>,
|
||
statusResolver?: (result: T) => string,
|
||
): Promise<T> { ... }
|
||
```
|
||
|
||
Each call site then becomes one line of intent instead of 25 lines of ceremony:
|
||
|
||
```tsx
|
||
async get<T>(key: string): Promise<T | null> {
|
||
return this.instrumented('get', key, async () => {
|
||
const value = await this.client.get(key);
|
||
if (value == null) return null;
|
||
return safeJsonParse<T>(value, this.logger);
|
||
}, (result) => (result == null ? 'miss' : 'hit'));
|
||
}
|
||
```
|
||
|
||
**Why:** the telemetry pattern was copy-pasted across `get`, `set`, `delete` with
|
||
only the operation name and status differing. A helper makes the unique logic visible
|
||
and the boilerplate invisible.
|
||
|
||
---
|
||
|
||
## 4. Keep serialisation consistent
|
||
|
||
Use `serializeValue()` and `safeJsonParse()` from `CacheSerialization.tsx` inside the
|
||
cache package rather than bare `JSON.stringify` / `JSON.parse`. Benefits:
|
||
|
||
- Parse errors are caught and logged with truncated values, not swallowed or thrown
|
||
raw.
|
||
- A single place to add future concerns (metrics on parse failures, encoding changes,
|
||
etc.).
|
||
- Callers outside the cache package doing their own `JSON.stringify` for HTTP bodies,
|
||
worker queues, etc. do **not** need these helpers – they are cache-specific.
|
||
|
||
---
|
||
|
||
## 5. Centralise validation and make it reusable
|
||
|
||
Lock key and token validation was copy-pasted across all three providers with identical
|
||
regexes and error messages. `CacheLockValidation.tsx` now owns:
|
||
|
||
- `validateLockKey(key)` – throws on bad format
|
||
- `validateLockToken(token)` – throws on bad format
|
||
- `generateLockToken()` – `randomBytes(16).toString('hex')`
|
||
- `formatLockKey(key)` – returns `lock:${key}`
|
||
|
||
**Guideline:** if the same regex or format string appears in more than one file, it
|
||
should be a named export in a shared module.
|
||
|
||
---
|
||
|
||
## 6. Import from the defining file ("horse's mouth")
|
||
|
||
Per project conventions: all callsites import directly from the file that defines the
|
||
symbol. Never re-export. Never create barrel files.
|
||
|
||
```tsx
|
||
// Good
|
||
import type {CacheLogger} from '@fluxer/cache/src/CacheProviderTypes';
|
||
|
||
// Bad – importing a type from a file that re-exports it
|
||
import type {CacheLogger} from '@fluxer/cache/src/providers/KVCacheProvider';
|
||
```
|
||
|
||
When moving a type to a new home, grep for every import and update it in the same
|
||
change. Do not leave the old export as a compatibility shim.
|
||
|
||
---
|
||
|
||
## 7. Provider-specific config stays in the provider file
|
||
|
||
`KVCacheProviderConfig` stays in `KVCacheProvider.tsx` because it references
|
||
`IKVProvider`, which is specific to that implementation. Only types shared across
|
||
multiple providers get extracted.
|
||
|
||
**Heuristic:** if a type is only imported by a single file and its tests, it belongs
|
||
in that file.
|
||
|
||
---
|
||
|
||
## 8. Verify refactors with the full test suite, formatter, and type checker
|
||
|
||
After any structural refactor, run in order:
|
||
|
||
```bash
|
||
cd packages/cache && pnpm test # all existing tests pass
|
||
pnpm biome check --write packages/cache/src/ # formatting (run from repo root)
|
||
cd packages/cache && pnpm typecheck # type checking
|
||
```
|
||
|
||
Then spot-check downstream consumers:
|
||
|
||
```bash
|
||
cd packages/api && pnpm typecheck
|
||
```
|
||
|
||
Pre-existing failures in downstream packages are acceptable so long as **none of the
|
||
errors reference the refactored package**.
|
||
|
||
---
|
||
|
||
## 9. Do not over-extract
|
||
|
||
Not every use of `randomBytes` or `JSON.stringify` in the codebase needs to use the
|
||
cache package's helpers. Cache serialisation helpers are for **cache values**. Other
|
||
domains (SSO tokens, CSP nonces, HTTP bodies, worker queues) have their own
|
||
serialisation needs and should not couple to the cache package.
|
||
|
||
**Rule:** extract shared code within a bounded context (the cache package), not across
|
||
unrelated domains.
|
||
|
||
---
|
||
|
||
## 10. Naming conventions for extracted files
|
||
|
||
Follow the project's descriptive-filename convention. Avoid generic names:
|
||
|
||
| Good | Bad |
|
||
|---|---|
|
||
| `CacheLockValidation.tsx` | `Validation.tsx` |
|
||
| `CacheKeyClassification.tsx` | `Utils.tsx` |
|
||
| `CacheProviderTypes.tsx` | `Types.tsx` |
|
||
| `RedisClientTypes.tsx` | `RedisTypes.tsx` |
|
||
|
||
Domain-prefix every file so it is unambiguous in search results and import
|
||
auto-complete.
|
||
|
||
---
|
||
|
||
## 11. Avoid backwards-compatibility shims
|
||
|
||
When moving exports to new files, do not leave behind re-exports or aliases in the old
|
||
location. This creates indirection and lets stale imports survive indefinitely.
|
||
Instead:
|
||
|
||
1. Move the definition.
|
||
2. Update every import site in the same commit.
|
||
3. Delete the old export entirely.
|
||
|
||
---
|
||
|
||
## 12. Keep the abstract contract stable
|
||
|
||
`ICacheService` and `CacheMSetEntry` stay in `ICacheService.tsx`. They define the
|
||
public contract that all providers implement and all consumers depend on. Refactoring
|
||
provider internals should never change this file.
|
||
|
||
---
|
||
|
||
## 13. One import block, no blank lines inside
|
||
|
||
Per project conventions, keep a single contiguous import block at the top of each file
|
||
with no blank lines or code between imports. Let biome handle ordering.
|
||
|
||
```tsx
|
||
// Good
|
||
import {classifyKeyType} from '@fluxer/cache/src/CacheKeyClassification';
|
||
import {formatLockKey, generateLockToken} from '@fluxer/cache/src/CacheLockValidation';
|
||
import type {CacheLogger} from '@fluxer/cache/src/CacheProviderTypes';
|
||
import {safeJsonParse, serializeValue} from '@fluxer/cache/src/CacheSerialization';
|
||
import {ICacheService} from '@fluxer/cache/src/ICacheService';
|
||
```
|
||
|
||
---
|
||
|
||
## 14. Scope of `safeJsonParse` logger parameter
|
||
|
||
`safeJsonParse` accepts an optional `CacheLogger`. The KV provider passes its logger
|
||
so parse errors are reported. The Redis provider passes nothing (logs to console as a
|
||
fallback in the provider itself – now removed in favour of silent null return). The
|
||
InMemory provider does not call `safeJsonParse` at all because it stores native values,
|
||
not serialised strings.
|
||
|
||
When adding a new provider, decide up front whether parse errors should be logged and
|
||
pass the logger accordingly.
|
||
|
||
---
|
||
|
||
## 15. Watch for non-atomic operations
|
||
|
||
The Redis provider's `acquireLock` was (and remains, pending a proper fix) non-atomic:
|
||
|
||
```tsx
|
||
await this.client.set(lockKey, token);
|
||
await this.client.expire(lockKey, ttlSeconds);
|
||
```
|
||
|
||
Between these two calls, the lock exists without a TTL. If the process crashes, the
|
||
lock is held forever. The KV provider uses `SET ... EX ... NX` in a single command,
|
||
which is correct. When implementing distributed primitives, always prefer atomic
|
||
operations.
|
||
|
||
---
|
||
|
||
## 16. Downstream callsites should use shared utilities
|
||
|
||
After extracting shared logic, audit the rest of the codebase for consumers that
|
||
duplicate the same logic inline. Common patterns:
|
||
|
||
- `lock:${key}` string templates → `formatLockKey(key)`
|
||
- `crypto.randomBytes(16).toString('hex')` → `generateLockToken()`
|
||
- `Math.random().toString(36)...` → `generateLockToken()` (also a security fix)
|
||
- Mock implementations with inline lock validation → import from `CacheLockValidation`
|
||
|
||
Update downstream callsites in the same change to prevent the shared code from
|
||
becoming an orphan.
|
||
|
||
---
|
||
|
||
## 17. Never use Math.random() for tokens
|
||
|
||
`Math.random()` is not cryptographically secure. For any token used in locking,
|
||
authentication, or session management, use `crypto.randomBytes()` or the shared
|
||
`generateLockToken()` utility.
|
||
|
||
```tsx
|
||
// bad
|
||
const token = Math.random().toString(36).substring(2, 15) + Math.random().toString(36).substring(2, 15);
|
||
|
||
// good
|
||
import {generateLockToken} from '@fluxer/cache/src/CacheLockValidation';
|
||
const token = generateLockToken();
|
||
```
|
||
|
||
---
|
||
|
||
## 18. Remove no-op string operations
|
||
|
||
Watch for `.replace()` calls that silently match nothing:
|
||
|
||
```tsx
|
||
// bad — 'deletion_queue:rebuild_lock' doesn't contain 'lock:' as a prefix
|
||
// .replace('lock:', '') matches nothing and returns the original string unchanged
|
||
REBUILD_LOCK_KEY.replace('lock:', '')
|
||
|
||
// good — pass the key directly
|
||
REBUILD_LOCK_KEY
|
||
```
|
||
|
||
No-op operations are a sign the author assumed a different key format. Verify the
|
||
format and remove the dead code.
|
||
|
||
---
|
||
|
||
## 19. Mock implementations should use shared utilities
|
||
|
||
Test mocks (like `MockCacheService`) that implement lock acquisition should use the
|
||
same shared utilities as real providers:
|
||
|
||
```tsx
|
||
// bad — inline duplication
|
||
const lockKey = `lock:${key}`;
|
||
const token = crypto.randomBytes(16).toString('hex');
|
||
|
||
// good — shared utilities keep mocks consistent
|
||
import {formatLockKey, generateLockToken} from '@fluxer/cache/src/CacheLockValidation';
|
||
const lockKey = formatLockKey(key);
|
||
const token = generateLockToken();
|
||
```
|
||
|
||
This ensures mocks produce tokens in the same format as real providers and that
|
||
format changes propagate automatically.
|
||
|
||
---
|
||
|
||
## Changelog
|
||
|
||
| Date | Change |
|
||
|---|---|
|
||
| 2026-02-06 | Initial version from cache package refactoring |
|
||
| 2026-02-06 | Added downstream cleanup lessons (sections 16–19) |
|