Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions src/Http/Controllers/Internal/v1/UserController.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
9 changes: 7 additions & 2 deletions src/Http/Resources/User.php
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand All @@ -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 ?? []),
Expand Down
20 changes: 16 additions & 4 deletions src/Models/User.php
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

/**
Expand All @@ -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();
}

/**
Expand All @@ -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();
}

/**
Expand All @@ -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();
}

/**
Expand Down
58 changes: 58 additions & 0 deletions tests/Unit/Models/UserModelTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();

Expand Down
Loading