Skip to content

fix: memoise n1 query - #74

Merged
shavonn merged 3 commits into
mainfrom
fix/memoise-n1
Sep 15, 2026
Merged

shavonn merged 3 commits into
mainfrom
fix/memoise-n1

Conversation

@shavonn

@shavonn shavonn commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

The fix

  • hasPermission() now resolves against a preloaded set of every permission name the subject holds — direct, via roles, via capabilities — built once per context from the existing getAllPermissions() and filtered by guard, since a name is only unique within one. Sixteen different abilities cost the same few joins as one.
  • An answer memo sits in front of it for genuinely repeated checks.
  • The same admin request now issues 8 queries, down from 72, with nothing repeating.

What is deliberately not cached

Feature access. A handler decides it, and it can flip between two checks without touching the model, so checkFeatureAccess() runs before the memo is consulted on every call. Caching across it broke three FeatureIntegrationTest cases in an early attempt. $bypassFeatureCheck is not part of the cache key because that check is the only thing it affects.

Invalidation

  • Cleared by all 13 grant/revoke/sync/ assign/remove methods across HasPermissions and HasRoles, which also drop the loaded permissions, roles, and capabilities relations.
  • That last part is load-bearing. The exists queries this replaces always hit the database, so a grant was visible to the very next check; building the set from relations reintroduced a snapshot and broke five tests until the relations were dropped too.
  • forgetMandateAnswers() is public for the one case the package cannot observe: changing what a role carries while holding a subject instance. That write happens on another model and cannot reach this one.

Risk

This changes the path every authorization check takes, so review it as a security change rather than a performance change. The key question is whether a stale true is reachable—the answer depends on the invalidation list above being complete and feature access staying outside the cache.

Verification

  • 614 tests / 1,235 assertions passing, including 5 new ones in tests/Feature/PermissionMemoTest.php: the query-count claim, invalidation on grant/revoke and on role assign/remove, that distinct abilities do not contaminate each other, and the manual forgetMandateAnswers() case.
  • Pint and PHPStan clean.

@shavonn
shavonn merged commit 5475971 into main Sep 15, 2026
20 checks passed
@shavonn
shavonn deleted the fix/memoise-n1 branch September 15, 2026 12:17
@github-actions github-actions Bot mentioned this pull request Sep 15, 2026
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