fluxer/packages/cache/BEST_PRACTICES.md
2026-02-17 12:22:36 +00:00

11 KiB
Raw Blame History

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:

// KVCacheProvider.tsx defines AND exports CacheLogger
export interface CacheLogger { ... }
export class KVCacheProvider { ... }

After:

// 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:

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:

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.

// 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:

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:

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.

// 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:

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.

// 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:

// 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:

// 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 1619)