Skip to content

perf(user): let authorization accessors honour eager-loaded relations (12 → 0 queries/row) - #251

Merged
roncodes merged 3 commits into
fleetbase:release/v1.6.61from
dounisaur:perf/user-authorization-accessors-honour-eager-loading
Sep 9, 2026
Merged

roncodes merged 3 commits into
fleetbase:release/v1.6.61from
dounisaur:perf/user-authorization-accessors-honour-eager-loading

Conversation

@dounisaur

Copy link
Copy Markdown
Contributor

The problem

User::$role, $roles, $policies and $permissions each execute a fresh query on every read, because they call the relation as a query builder rather than reading the loaded relation:

return $this->companyUser->roles()->first();     // ->roles() then ->first() = a query
return $this->companyUser->policies()->get();
return $this->companyUser->permissions()->get();

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->role four times per row — twice for role, twice for role_name — each one paying the full accessor cost.

Measurements

Taken with DB::listen over a real page of users (5 rows), on a deployed 1.6.55 install. The four methods are byte-identical on main at b7691c0, so these apply to HEAD.

queries per row
today 12
resource reads role once 8
+ eager-load companyUser 7
+ deep eager-load (companyUser.roles/policies/permissions) 7 — no change
accessors honour the loaded relation + eager-load 0

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

  1. 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.
  2. Http/Controllers/Internal/v1/UserController.php — onQueryRecord() eager-loads companyUser.roles, companyUser.policies, companyUser.permissions. Placed before the early return so it applies on both branches.
  3. Http/Resources/User.php — reads the role accessor once into a local.

What it deliberately does not do

  • No memoisation and no setRelation on the accessors. Caching into a relation slot would make role / policies / permissions appear in the model's array output, which would be a silent serialization change. This approach has no such side effect.
  • No response shape change. Every key and value is identical.
  • instanceof Model guard. companyUser is not always an Eloquent model — this repo's own UserModelAuthorizationPivotFake is duck-typed — so the loaded-relation path is guarded and those callers keep the query path. Without the guard, Tests\Unit\Models\UserModelTest fails.

Test

Adds it reads eager-loaded authorization relations without re-querying them. Its pivot throws from roles() / 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.

Tests: 1429 passed (10024 assertions)   # 1428 before, + the new test

Deprecations and warnings are unchanged from the baseline run.

Noted, not addressed here

  • Traits/ProxiesAuthorizationMethods — its __call proxy forwards any role/policy/permission-named method to the pivot and queries per call, so getRoleName() 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/Role and Http/Resources/Policy always serialize their full permissions array. On the same install this made the IAM Roles and Policies lists ~357 KB and ~332 KB per page. A list-context permissions_count would 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.
  • Per-row Setting::lookup('user.<uuid>.locale') and the companyUser() fallback when users.company_uuid is null are both further per-row costs, also left out to keep this reviewable.

`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

codecov Bot commented Sep 5, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (b7691c0) to head (1666d7d).
⚠️ Report is 3 commits behind head on release/v1.6.61.

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     
Flag Coverage Δ
backend 100.00% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@roncodes
roncodes changed the base branch from main to release/v1.6.61 September 9, 2026 04:27
@roncodes

roncodes commented Sep 9, 2026

Copy link
Copy Markdown
Member

Patched in 7982bd5 to make the eager-loading optimization work with real company memberships.

The original companyUser() relation applied where('company_uuid', $this->company_uuid). Eloquent constructs eager-loading relations on an empty parent model, so the list query used company_uuid IS NULL. Users with normal memberships received a null loaded relation and fell back to per-user lookups; their authorization relations were never batch-loaded. The existing fake-pivot test bypassed this path.

The patch adds a HasOne specialization that builds eager constraints from the actual users and matches memberships by both user UUID and company UUID. This preserves lazy lookup using the user's own company_uuid, handles cross-company lists and users with multiple memberships, and correlates relationship-existence queries on both columns. It does not substitute the requester's session company for each listed user's company.

Three database-backed regression tests cover:

  • Constant authorization batch query counts for one versus multiple users, with zero additional queries when reading the four authorization accessors after loading.
  • Correct memberships and distinct authorization across companies, including the same user represented in two company contexts, plus identical serialized user-resource values between lazy and eager loading.
  • Missing and soft-deleted memberships, null company IDs, tenant-scoped queries, and relationship-existence queries.

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 release/v1.6.61 from current main and retargeted this PR to it. Release-branch commit 49f5f2f adds release/v* to the PHP CI and Postman branch filters so checks remain enabled for the new target.

@roncodes roncodes mentioned this pull request Sep 9, 2026
@roncodes
roncodes merged commit dc0dfa0 into fleetbase:release/v1.6.61 Sep 9, 2026
5 checks passed
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.

2 participants