Repository navigation
perf(user): let authorization accessors honour eager-loaded relations (12 → 0 queries/row) - #251
Conversation
`User::$role`, `$roles`, `$policies` and `$permissions` each execute a fresh query on every read, because they call the relation as a query builder (`$this->companyUser->roles()->first()`) rather than reading the loaded relation. Eager-loading `companyUser.roles` therefore does nothing: the data is loaded and the accessor queries anyway. `Http\Resources\User::toArray()` compounds it by evaluating `$this->role` four times per row (twice for `role`, twice for `role_name`). Measured on a real page with `DB::listen`, 5 users: today 12 queries/row resource reads `role` once 8 queries/row + eager-load `companyUser` 7 queries/row + deep eager-load (roles/policies/permissions) 7 queries/row (no change) accessors honour the loaded relation + eager-load 0 queries/row Each accessor now prefers the loaded relation and falls back to the query when it is absent, so behaviour is unchanged for callers that do not eager-load. The `instanceof Model` guard keeps duck-typed pivots working -- the suite's own UserModelAuthorizationPivotFake is one. No response shape changes, no memoisation, and nothing new appears in the model's array output. Adds a test whose pivot throws from roles()/policies()/permissions(), so the suite fails loudly if an accessor ever queries past a loaded relation again.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## release/v1.6.61 #251 +/- ##
===================================================
Coverage 100.00% 100.00%
- Complexity 6734 6750 +16
===================================================
Files 397 398 +1
Lines 22470 22502 +32
===================================================
+ Hits 22470 22502 +32
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Patched in 7982bd5 to make the eager-loading optimization work with real company memberships. The original The patch adds a Three database-backed regression tests cover:
All three tests fail when the original relationship is restored and pass with this patch. The full local Pest run completed with 1,437 passed, 22 deprecated, 5 warnings, 10,064 assertions, and no failures. The repository-wide PHP style check and date-drift check also passed. Validation used the installed local dependencies and SQLite; the GitHub PHP CI and Postman runs are currently awaiting fork-workflow approval. This removes the membership/authorization N+1 path for users with a matching company membership; it does not claim that the entire serialized response executes zero per-user queries. Other resource lookups and the existing missing-membership fallback remain separate paths. Created |
The problem
User::$role,$roles,$policiesand$permissionseach execute a fresh query on every read, because they call the relation as a query builder rather than reading the loaded relation:The consequence is that eager-loading does nothing.
with('companyUser.roles')loads the data and the accessor queries anyway.Http\Resources\User::toArray()compounds it by evaluating$this->rolefour times per row — twice forrole, twice forrole_name— each one paying the full accessor cost.Measurements
Taken with
DB::listenover a real page of users (5 rows), on a deployed 1.6.55 install. The four methods are byte-identical onmainatb7691c0, so these apply to HEAD.roleoncecompanyUsercompanyUser.roles/policies/permissions)The fourth row is the interesting one: deep eager-loading currently buys nothing, which is what identifies the accessors rather than the query as the cause.
For scale, on the install this was found on the IAM Users list took ~12 s for 18 rows and grew linearly with row count.
The change
Models/User.php— the four authorization accessors prefer the eager-loaded relation and fall back to the query when it is absent. Behaviour is unchanged for callers that do not eager-load.Http/Controllers/Internal/v1/UserController.php—onQueryRecord()eager-loadscompanyUser.roles,companyUser.policies,companyUser.permissions. Placed before the early return so it applies on both branches.Http/Resources/User.php— reads theroleaccessor once into a local.What it deliberately does not do
setRelationon the accessors. Caching into a relation slot would makerole/policies/permissionsappear in the model's array output, which would be a silent serialization change. This approach has no such side effect.instanceof Modelguard.companyUseris not always an Eloquent model — this repo's ownUserModelAuthorizationPivotFakeis duck-typed — so the loaded-relation path is guarded and those callers keep the query path. Without the guard,Tests\Unit\Models\UserModelTestfails.Test
Adds
it reads eager-loaded authorization relations without re-querying them. Its pivot throws fromroles()/policies()/permissions(), so the suite fails loudly if an accessor ever queries past a loaded relation again.Verified the test has teeth: reverting the accessor change makes it fail.
Deprecations and warnings are unchanged from the baseline run.
Noted, not addressed here
Traits/ProxiesAuthorizationMethods— its__callproxy forwards any role/policy/permission-named method to the pivot and queries per call, sogetRoleName()and friends still pay per-row. The new test found this; it is a separate path and out of scope for this change.Http/Resources/RoleandHttp/Resources/Policyalways serialize their fullpermissionsarray. On the same install this made the IAM Roles and Policies lists ~357 KB and ~332 KB per page. A list-contextpermissions_countwould fix it, but that is a breaking response-shape change, so I have left it out rather than bundle it here. Happy to raise it separately if you would take it.Setting::lookup('user.<uuid>.locale')and thecompanyUser()fallback whenusers.company_uuidis null are both further per-row costs, also left out to keep this reviewable.