Skip to content

feat(cache): add scope option to confine invalidation to a tenant - #58

Draft
fratzinger wants to merge 1 commit into
mainfrom
feat/cache-scope
Draft

fratzinger wants to merge 1 commit into
mainfrom
feat/cache-scope

Conversation

@fratzinger

Copy link
Copy Markdown
Member

Warning

WIP — opened for early feedback on the API shape and the safety rules. Not ready to merge.

Summary

In a multi-tenant app every row carries a companyId (or userId, or locale, …) and every call is effectively confined to one of them. Today any mutation invalidates every cached find of every tenant, so with many tenants the cache barely ever hits.

scope is one function, branched on context.method:

cache({
  map,
  transformParams: ({ query }) => ({ query }),
  scope: (context) => {
    if (context.method === 'get' || context.method === 'find') {
      // only scope when the query really pins the tenant down
      return context.params.query?.companyId
    }

    // `update` replaces the row, so the company may change or be dropped
    if (context.method === 'update') return undefined

    // a `patch` could move items into another company
    if (context.method === 'patch' && mayChangeCompany(context.data)) {
      return undefined
    }

    return getResultIsArray(context).result.map((item) => item.companyId)
  },
})

A create in company a now drops company a's cached find entries and leaves company b's alone. The scope also becomes the first key segment, which closes a leak that exists today: derive the tenant from params.user.companyId while transformParams keeps only query, and two tenants collide on the same key.

Never caching too much

The guiding rule was: a false hit is unacceptable, a false miss is merely wasteful. So there are three levels, and the middle one is exactly what the hook does without the option:

Level Effect When
scoped drops the affected scopes' find entries plus every unscoped one; get entries by id the scope is known
unscoped drops every find entry; get entries by id scope unknown, resolver threw, or you returned undefined
full clear map.clear() the affected ids are unknown

Enabling scope therefore never invalidates less than the default. An entry stored without a resolvable scope is invalidated by every mutation, and a scoped mutation still clears those unscoped entries — otherwise an entry written once would go stale forever.

Because the common fallback does not call clear() and emits the same invalidate events a healthy scoped run does, silent under-scoping would be invisible. Hence a new degrade event carrying a reason (no-scope | scope-error | no-ids), and a reason on clear.

Three bugs fixed along the way

Found while reworking the key parser, each one a stale-data hole:

  • an id containing : (composite or urn:-style keys) was truncated at the first delimiter, never matched the mutated ids, and its cached get entry was never invalidated
  • partially missing ids — a mutation whose result items lacked ids (e.g. $select) invalidated only the known ones and left the rest silently stale; now a single unknown id forces a full clear
  • the id 0 fell through a filter(Boolean) and triggered an unnecessary full wipe

Each has a regression test, and I verified every one fails against the old implementation.

Other Information

Test refactor in the same diff (sorry). The three near-identical storage blocks (lru-cache / @isaacs/ttlcache / native Map) became one describe.each. I diffed them first to confirm the lru-cache block was a strict superset, so nothing was lost — and TTLCache and Map now run 12 tests each instead of 7. That is where the ~800 deletions come from; the file shrank from 1976 to 1682 lines while gaining coverage. Happy to split this into its own commit if that makes review easier.

Key format changes only when scope is set: <scope>:<id>:<params>, with the scope and id segments URI-encoded. Non-adopters keep their format. Entries written before enabling it are never served — a new read computes a three-segment key, which can never equal a two-segment one — but in a persistent store they linger until they expire, so bump your key prefix or flush on deploy. Documented.

Type-level only, no runtime break: CacheEvent gains a required reason on clear and a new degrade member. That breaks an exhaustive switch with a never default, nothing else.

pnpm test is green: 1514 tests, coverage of cache.hook.ts at 99.21% statements / 94.94% branches.

Open questions

  • API shape: one context-level function, or a { read, mutation } pair? I went with one because a per-item resolver cannot express "this patch may move rows, give up", which is the decision that actually protects against stale data.
  • Multi-scope reads ({ companyId: { $in: ['a','b'] } }) currently fall back to unscoped. Supporting them means joining scopes into the key segment and splitting/intersecting per key in the invalidation loop. I don't think it earns the complexity — an unscoped read is exactly today's caching, so nothing regresses.

🤖 Generated with Claude Code

Without it, any mutation invalidates every cached `find` of every tenant,
so a service where each row carries a `companyId` barely ever hits the
cache. `scope` is one function branched on `context.method`: for `get`/
`find` it returns the scope the result is confined to, for mutations every
scope the mutation may affect. The scope becomes the first key segment, so
two tenants can never be served each other's entries.

Safety is one-directional by construction. Whenever the scope cannot be
determined — the resolver returned nothing usable or threw, `$select`
stripped the field, a `patch` may move a row between tenants — the hook
falls back to the behaviour it has without the option, so enabling `scope`
never invalidates less than before. Every fallback emits a new `degrade`
event, because the common fallback does not call `clear()` and would
otherwise look exactly like a healthy scoped run.

Also fixes three invalidation holes found while reworking the key parser:

- an id containing `:` was truncated by the parser, never matched, and its
  cached `get` entry was never invalidated
- a mutation whose result items partially lacked ids invalidated only the
  known ones and left the rest silently stale
- the id `0` was treated as unknown and triggered a full wipe

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@cloudflare-workers-and-pages

Copy link
Copy Markdown

Deploying feathers-utils with  Cloudflare Pages  Cloudflare Pages

Latest commit: 2373c0d
Status: ✅  Deploy successful!
Preview URL: https://4c6b91ae.feathers-utils.pages.dev
Branch Preview URL: https://feat-cache-scope.feathers-utils.pages.dev

View logs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant