diff --git a/src/Http/Controllers/Internal/v1/UserController.php b/src/Http/Controllers/Internal/v1/UserController.php index fe6a416e..241046ba 100644 --- a/src/Http/Controllers/Internal/v1/UserController.php +++ b/src/Http/Controllers/Internal/v1/UserController.php @@ -114,6 +114,11 @@ public function queryRecord(Request $request) */ public function onQueryRecord($query, Request $request): void { + // Eager-load what the `role` / `roles` / `policies` / `permissions` accessors + // read. Without this each row costs a fresh query per accessor; with it the + // whole page costs a fixed number of queries regardless of row count. + $query->with(['companyUser.roles', 'companyUser.policies', 'companyUser.permissions']); + if ($this->canAccessUsersAcrossCompanies($request)) { return; } diff --git a/src/Http/Resources/User.php b/src/Http/Resources/User.php index c8af56cb..f01b108d 100644 --- a/src/Http/Resources/User.php +++ b/src/Http/Resources/User.php @@ -17,6 +17,11 @@ class User extends FleetbaseResource */ public function toArray($request) { + // Read the `role` accessor once. It is not memoised — every read re-queries + // through `companyUser` — and this resource previously evaluated it four times + // per row (twice below, twice for `role_name`). + $role = Http::isInternalRequest() ? $this->role : null; + $data = [ 'id' => $this->when(Http::isInternalRequest(), $this->id, $this->public_id), 'uuid' => $this->when(Http::isInternalRequest(), $this->uuid), @@ -31,10 +36,10 @@ public function toArray($request) 'timezone' => $this->timezone, 'avatar_url' => $this->avatar_url, 'meta' => data_get($this, 'meta', Utils::createObject()), - 'role' => $this->when(Http::isInternalRequest(), $this->role ? new Role($this->role) : null, null), + 'role' => $this->when(Http::isInternalRequest(), $role ? new Role($role) : null, null), 'policies' => $this->when(Http::isInternalRequest(), Policy::collection($this->policies), []), 'permissions' => $this->when(Http::isInternalRequest(), $this->serializePermissions($this->permissions), []), - 'role_name' => $this->when(Http::isInternalRequest(), $this->role ? $this->role->name : null), + 'role_name' => $this->when(Http::isInternalRequest(), $role ? $role->name : null), 'type' => $this->type, 'locale' => $this->getLocale(), 'types' => $this->when(Http::isInternalRequest(), $this->types ?? []), diff --git a/src/Models/User.php b/src/Models/User.php index 0ff6b829..213fe593 100644 --- a/src/Models/User.php +++ b/src/Models/User.php @@ -622,7 +622,13 @@ public function getRoleAttribute(): ?Role return null; } - return $this->companyUser->roles()->first(); + // Prefer the eager-loaded relation when the caller has loaded it, so a list + // query that eager-loads `companyUser.roles` pays no per-row query. Falls back + // to the query for callers that have not, and for a `companyUser` that is not + // an Eloquent model (the suite's UserModelAuthorizationPivotFake is duck-typed). + return $this->companyUser instanceof Model && $this->companyUser->relationLoaded('roles') + ? $this->companyUser->roles->first() + : $this->companyUser->roles()->first(); } /** @@ -639,7 +645,9 @@ public function getRolesAttribute(): Collection return collect(); } - return $this->companyUser->roles()->get(); + return $this->companyUser instanceof Model && $this->companyUser->relationLoaded('roles') + ? $this->companyUser->roles + : $this->companyUser->roles()->get(); } /** @@ -656,7 +664,9 @@ public function getPoliciesAttribute(): Collection return collect(); } - return $this->companyUser->policies()->get(); + return $this->companyUser instanceof Model && $this->companyUser->relationLoaded('policies') + ? $this->companyUser->policies + : $this->companyUser->policies()->get(); } /** @@ -673,7 +683,9 @@ public function getPermissionsAttribute(): Collection return collect(); } - return $this->companyUser->permissions()->get(); + return $this->companyUser instanceof Model && $this->companyUser->relationLoaded('permissions') + ? $this->companyUser->permissions + : $this->companyUser->permissions()->get(); } /** diff --git a/tests/Unit/Models/UserModelTest.php b/tests/Unit/Models/UserModelTest.php index 428fff3d..880b5a6f 100644 --- a/tests/Unit/Models/UserModelTest.php +++ b/tests/Unit/Models/UserModelTest.php @@ -22,6 +22,29 @@ use Illuminate\Support\Facades\Cache; use Illuminate\Support\Facades\Facade; +/** + * A `companyUser` pivot whose relations are already eager-loaded, and whose relation + * QUERY methods throw. If an authorization accessor falls back to querying when the + * relation is present, these throws are what surfaces it. + */ +class UserModelEagerLoadedPivotFake extends Model +{ + public function roles(): object + { + throw new RuntimeException('roles() was queried despite an eager-loaded relation'); + } + + public function policies(): object + { + throw new RuntimeException('policies() was queried despite an eager-loaded relation'); + } + + public function permissions(): object + { + throw new RuntimeException('permissions() was queried despite an eager-loaded relation'); + } +} + class UserModelSaveSpy extends User { public int $saves = 0; @@ -825,6 +848,41 @@ public function save(): bool ->and($userWithoutRoles->getRoleName())->toBeNull(); }); +it('reads eager-loaded authorization relations without re-querying them', function () { + user_model_container(); + config([ + 'auth.defaults.guard' => 'web', + 'auth.guards.web' => [ + 'driver' => 'session', + 'provider' => 'users', + ], + ]); + + $role = new Role(); + $role->setRawAttributes(['name' => 'Dispatcher'], true); + + $policy = new Policy(); + $policy->setRawAttributes(['name' => 'Orders Read'], true); + + $permission = new Permission(); + $permission->setRawAttributes(['name' => 'orders.read'], true); + + // The pivot's roles()/policies()/permissions() QUERY methods throw, so this test + // fails loudly if an accessor ignores the loaded relation and queries anyway. + $pivot = new UserModelEagerLoadedPivotFake(); + $pivot->setRelation('roles', collect([$role])); + $pivot->setRelation('policies', collect([$policy])); + $pivot->setRelation('permissions', collect([$permission])); + + $user = new UserModelSaveSpy(); + $user->setRelation('companyUser', $pivot); + + expect($user->role)->toBe($role) + ->and($user->roles)->toEqual(collect([$role])) + ->and($user->policies)->toEqual(collect([$policy])) + ->and($user->permissions)->toEqual(collect([$permission])); +}); + it('enriches new and existing users from request timezone data without calling missing helpers', function () { user_model_container();