diff --git a/docs/user_management/managing_users.md b/docs/user_management/managing_users.md index 3c321bd3d..597be53d6 100644 --- a/docs/user_management/managing_users.md +++ b/docs/user_management/managing_users.md @@ -79,6 +79,42 @@ $user->fill([ $users->save($user); ``` +### Listing Users + +When displaying a list of users - for example, in the admin panel - we typically use the standard `find*` methods. However, these methods only return basic user information. + +If you need additional details like email addresses, groups, or permissions, each piece of information will trigger a separate database query for every user. This happens because user entities lazy-load related data, which can quickly result in a large number of queries. + +To optimize this, you can use method scopes like `UserModel::withIdentities()`, `withGroups()`, and `withPermissions()`. These methods preload the related data in a single query (one per each method), drastically reducing the number of database queries and improving performance. + +```php +// Get the User Provider (UserModel by default) +$users = auth()->getProvider(); + +$usersList = $users + ->withIdentities() + ->withGroups() + ->withPermissions() + ->findAll(10); + +// The below code would normally trigger an additional +// DB queries, on every loop iteration, but now it won't + +foreach ($usersList as $u) { + // Because identities are preloaded + echo $u->email; + + // Because groups are preloaded + $u->inGroup('admin'); + + // Because permissions are preloaded + $u->hasPermission('users.delete'); + + // Because groups and permissions are preloaded + $u->can('users.delete'); +} +``` + ## Managing Users via CLI Shield has a CLI command to manage users. You can do the following actions: diff --git a/phpstan-baseline.php b/phpstan-baseline.php index 575162eb4..10ffdc6bc 100644 --- a/phpstan-baseline.php +++ b/phpstan-baseline.php @@ -405,6 +405,12 @@ $ignoreErrors[] = [ 'message' => '#^Call to function model with CodeIgniter\\\\Shield\\\\Models\\\\GroupModel\\:\\:class is discouraged\\.$#', 'identifier' => 'codeigniter.factoriesClassConstFetch', + 'count' => 2, + 'path' => __DIR__ . '/src/Models/UserModel.php', +]; +$ignoreErrors[] = [ + 'message' => '#^Call to function model with CodeIgniter\\\\Shield\\\\Models\\\\PermissionModel\\:\\:class is discouraged\\.$#', + 'identifier' => 'codeigniter.factoriesClassConstFetch', 'count' => 1, 'path' => __DIR__ . '/src/Models/UserModel.php', ]; @@ -489,7 +495,7 @@ $ignoreErrors[] = [ 'message' => '#^Call to an undefined method CodeIgniter\\\\Shield\\\\Models\\\\UserModel\\:\\:getLastQuery\\(\\)\\.$#', 'identifier' => 'method.notFound', - 'count' => 1, + 'count' => 7, 'path' => __DIR__ . '/tests/Unit/UserTest.php', ]; $ignoreErrors[] = [ @@ -498,5 +504,11 @@ 'count' => 1, 'path' => __DIR__ . '/tests/Unit/UserTest.php', ]; +$ignoreErrors[] = [ + 'message' => '#^Offset 1 does not exist on array\\{CodeIgniter\\\\Shield\\\\Entities\\\\User\\}\\.$#', + 'identifier' => 'offsetAccess.notFound', + 'count' => 5, + 'path' => __DIR__ . '/tests/Unit/UserTest.php', +]; return ['parameters' => ['ignoreErrors' => $ignoreErrors]]; diff --git a/psalm.xml b/psalm.xml index 5b3a32593..61677a083 100644 --- a/psalm.xml +++ b/psalm.xml @@ -11,6 +11,7 @@ errorBaseline="psalm-baseline.xml" findUnusedBaselineEntry="false" findUnusedCode="false" + ensureOverrideAttribute="false" > diff --git a/src/Authorization/Traits/Authorizable.php b/src/Authorization/Traits/Authorizable.php index 6e4f4b21b..b34b134b0 100644 --- a/src/Authorization/Traits/Authorizable.php +++ b/src/Authorization/Traits/Authorizable.php @@ -112,6 +112,22 @@ public function syncGroups(string ...$groups): self return $this; } + /** + * Set groups cache manually + */ + public function setGroupsCache(array $groups): void + { + $this->groupCache = $groups === [] ? null : $groups; + } + + /** + * Set permissions cache manually + */ + public function setPermissionsCache(array $permissions): void + { + $this->permissionsCache = $permissions === [] ? null : $permissions; + } + /** * Returns all groups this user is a part of. */ diff --git a/src/Models/GroupModel.php b/src/Models/GroupModel.php index 9d4e5b8ce..ea41d02a8 100644 --- a/src/Models/GroupModel.php +++ b/src/Models/GroupModel.php @@ -82,4 +82,28 @@ public function isValidGroup(string $group): bool return in_array($group, $allowedGroups, true); } + + /** + * @param list|list $userIds + * + * @return array + */ + public function getGroupsByUserIds(array $userIds): array + { + $groups = $this->builder() + ->select('user_id, group') + ->whereIn('user_id', $userIds) + ->orderBy($this->primaryKey) + ->get() + ->getResultArray(); + + return array_map( + 'array_keys', + array_reduce($groups, static function ($carry, $item) { + $carry[$item['user_id']][$item['group']] = true; + + return $carry; + }, []), + ); + } } diff --git a/src/Models/PermissionModel.php b/src/Models/PermissionModel.php index fc508ef73..553818640 100644 --- a/src/Models/PermissionModel.php +++ b/src/Models/PermissionModel.php @@ -72,4 +72,28 @@ public function deleteNotIn($userId, mixed $cache): void $this->checkQueryReturn($return); } + + /** + * @param list|list $userIds + * + * @return array + */ + public function getPermissionsByUserIds(array $userIds): array + { + $permissions = $this->builder() + ->select('user_id, permission') + ->whereIn('user_id', $userIds) + ->orderBy($this->primaryKey) + ->get() + ->getResultArray(); + + return array_map( + 'array_keys', + array_reduce($permissions, static function ($carry, $item) { + $carry[$item['user_id']][$item['permission']] = true; + + return $carry; + }, []), + ); + } } diff --git a/src/Models/UserModel.php b/src/Models/UserModel.php index e4a002fbb..7f6a9ebb8 100644 --- a/src/Models/UserModel.php +++ b/src/Models/UserModel.php @@ -39,7 +39,7 @@ class UserModel extends BaseModel 'last_active', ]; protected $useTimestamps = true; - protected $afterFind = ['fetchIdentities']; + protected $afterFind = ['fetchIdentities', 'fetchGroups', 'fetchPermissions']; protected $afterInsert = ['saveEmailIdentity']; protected $afterUpdate = ['saveEmailIdentity']; @@ -49,6 +49,18 @@ class UserModel extends BaseModel */ protected bool $fetchIdentities = false; + /** + * Whether groups should be included + * when user records are fetched from the database. + */ + protected bool $fetchGroups = false; + + /** + * Whether permissions should be included + * when user records are fetched from the database. + */ + protected bool $fetchPermissions = false; + /** * Save the User for afterInsert and afterUpdate */ @@ -73,6 +85,30 @@ public function withIdentities(): self return $this; } + /** + * Mark the next find* query to include groups + * + * @return $this + */ + public function withGroups(): self + { + $this->fetchGroups = true; + + return $this; + } + + /** + * Mark the next find* query to include permissions + * + * @return $this + */ + public function withPermissions(): self + { + $this->fetchPermissions = true; + + return $this; + } + /** * Populates identities for all records * returned from a find* method. Called @@ -147,6 +183,113 @@ private function assignIdentities(array $data, array $identities): array return $mappedUsers; } + /** + * Populates groups for all records + * returned from a find* method. Called + * automatically when $this->fetchGroups == true + * + * Model event callback called by `afterFind`. + */ + protected function fetchGroups(array $data): array + { + if (! $this->fetchGroups) { + return $data; + } + + $userIds = $data['singleton'] + ? array_column($data, 'id') + : array_column($data['data'], 'id'); + + if ($userIds === []) { + return $data; + } + + /** @var GroupModel $groupModel */ + $groupModel = model(GroupModel::class); + + // Get our groups for all users + $groups = $groupModel->getGroupsByUserIds($userIds); + + if ($groups === []) { + return $data; + } + + $mappedUsers = $this->assignProperties($data, $groups, 'groups'); + + $data['data'] = $data['singleton'] ? $mappedUsers[$data['id']] : $mappedUsers; + + return $data; + } + + /** + * Populates permissions for all records + * returned from a find* method. Called + * automatically when $this->fetchPermissions == true + * + * Model event callback called by `afterFind`. + */ + protected function fetchPermissions(array $data): array + { + if (! $this->fetchPermissions) { + return $data; + } + + $userIds = $data['singleton'] + ? array_column($data, 'id') + : array_column($data['data'], 'id'); + + if ($userIds === []) { + return $data; + } + + /** @var PermissionModel $permissionModel */ + $permissionModel = model(PermissionModel::class); + + $permissions = $permissionModel->getPermissionsByUserIds($userIds); + + if ($permissions === []) { + return $data; + } + + $mappedUsers = $this->assignProperties($data, $permissions, 'permissions'); + + $data['data'] = $data['singleton'] ? $mappedUsers[$data['id']] : $mappedUsers; + + return $data; + } + + /** + * Map our users by ID to make assigning simpler + * + * @param array $data Event $data + * @param list $properties + * @param string $type One of: 'groups' or 'permissions' + * + * @return list UserId => User object + */ + private function assignProperties(array $data, array $properties, string $type): array + { + $mappedUsers = []; + + $users = $data['singleton'] ? [$data['data']] : $data['data']; + + foreach ($users as $user) { + $mappedUsers[$user->id] = $user; + } + unset($users); + + // Build method name + $method = 'set' . ucfirst($type) . 'Cache'; + + // Now assign the properties to the user + foreach ($properties as $userId => $propertyArray) { + $mappedUsers[$userId]->{$method}($propertyArray); + } + unset($properties); + + return $mappedUsers; + } + /** * Adds a user to the default group. * Used during registration. diff --git a/tests/Unit/UserTest.php b/tests/Unit/UserTest.php index 2f6d7a440..dc1aa18d8 100644 --- a/tests/Unit/UserTest.php +++ b/tests/Unit/UserTest.php @@ -18,7 +18,9 @@ use CodeIgniter\Shield\Entities\Login; use CodeIgniter\Shield\Entities\User; use CodeIgniter\Shield\Entities\UserIdentity; +use CodeIgniter\Shield\Models\GroupModel; use CodeIgniter\Shield\Models\LoginModel; +use CodeIgniter\Shield\Models\PermissionModel; use CodeIgniter\Shield\Models\UserIdentityModel; use CodeIgniter\Shield\Models\UserModel; use Tests\Support\DatabaseTestCase; @@ -132,6 +134,162 @@ public function testModelFindByIdWithIdentitiesUserNotExists(): void $this->assertNull($user); } + public function testModelFindAllWithGroupsUserNotExists(): void + { + $users = model(UserModel::class)->where('active', 0)->withGroups()->findAll(); + + $this->assertSame([], $users); + } + + public function testModelFindAllWithGroups(): void + { + fake(GroupModel::class, ['user_id' => $this->user->id, 'group' => 'superadmin']); + fake(GroupModel::class, ['user_id' => $this->user->id, 'group' => 'admin']); + + $users = model(UserModel::class)->where('active', 1)->withGroups()->findAll(); + + $this->assertCount(1, $users); + + $this->assertTrue($users[1]->inGroup('admin')); + + // Check the last query and see if a proper type of query was used + $query = (string) model(UserModel::class)->getLastQuery(); + $this->assertMatchesRegularExpression( + '/WHERE\s+.*\s+IN\s+\([^)]+\)/i', + $query, + 'Groups were not obtained with the single query (missing "WHERE ... IN" condition)', + ); + } + + public function testModelFindByIdWithGroupsUserNotExists(): void + { + $user = model(UserModel::class)->where('active', 0)->withGroups()->findById(1); + + $this->assertNull($user); + } + + public function testModelFindByIdWithGroups(): void + { + fake(GroupModel::class, ['user_id' => $this->user->id, 'group' => 'superadmin']); + fake(GroupModel::class, ['user_id' => $this->user->id, 'group' => 'admin']); + + $user = model(UserModel::class)->where('active', 1)->withGroups()->findById(1); + + $this->assertInstanceOf(User::class, $user); + + $this->assertTrue($user->inGroup('admin')); + + // Check the last query and see if a proper type of query was used + $query = (string) model(UserModel::class)->getLastQuery(); + $this->assertMatchesRegularExpression( + '/WHERE\s+.*\s+IN\s+\([^)]+\)/i', + $query, + 'Groups were not obtained with the single query (missing "WHERE ... IN" condition)', + ); + } + + public function testModelFindAllWithPermissionsUserNotExists(): void + { + $users = model(UserModel::class)->where('active', 0)->withPermissions()->findAll(); + + $this->assertSame([], $users); + } + + public function testModelFindAllWithPermissions(): void + { + fake(PermissionModel::class, ['user_id' => $this->user->id, 'permission' => 'users.edit']); + fake(PermissionModel::class, ['user_id' => $this->user->id, 'permission' => 'users.delete']); + + $users = model(UserModel::class)->where('active', 1)->withPermissions()->findAll(); + + $this->assertCount(1, $users); + + $this->assertTrue($users[1]->hasPermission('users.delete')); + $this->assertFalse($users[1]->hasPermission('users.add')); + + // Check the last query and see if a proper type of query was used + $query = (string) model(UserModel::class)->getLastQuery(); + $this->assertMatchesRegularExpression( + '/WHERE\s+.*\s+IN\s+\([^)]+\)/i', + $query, + 'Permissions were not obtained with the single query (missing "WHERE ... IN" condition)', + ); + } + + public function testModelFindByIdWithPermissionsUserNotExists(): void + { + $user = model(UserModel::class)->where('active', 0)->withPermissions()->findById(1); + + $this->assertNull($user); + } + + public function testModelFindByIdWithPermissions(): void + { + fake(PermissionModel::class, ['user_id' => $this->user->id, 'permission' => 'users.edit']); + fake(PermissionModel::class, ['user_id' => $this->user->id, 'permission' => 'users.delete']); + + $user = model(UserModel::class)->where('active', 1)->withPermissions()->findById(1); + + $this->assertInstanceOf(User::class, $user); + + $this->assertTrue($user->hasPermission('users.delete')); + $this->assertFalse($user->hasPermission('users.add')); + + // Check the last query and see if a proper type of query was used + $query = (string) model(UserModel::class)->getLastQuery(); + $this->assertMatchesRegularExpression( + '/WHERE\s+.*\s+IN\s+\([^)]+\)/i', + $query, + 'Permissions were not obtained with the single query (missing "WHERE ... IN" condition)', + ); + } + + public function testModelFindByIdWithGroupsAndPermissions(): void + { + fake(GroupModel::class, ['user_id' => $this->user->id, 'group' => 'superadmin']); + fake(GroupModel::class, ['user_id' => $this->user->id, 'group' => 'admin']); + fake(PermissionModel::class, ['user_id' => $this->user->id, 'permission' => 'users.edit']); + fake(PermissionModel::class, ['user_id' => $this->user->id, 'permission' => 'users.delete']); + + $user = model(UserModel::class)->where('active', 1)->withGroups()->withPermissions()->findById(1); + + $this->assertInstanceOf(User::class, $user); + + $this->assertTrue($user->can('users.delete')); + $this->assertTrue($user->can('users.add')); + + // Check the last query and see if a proper type of query was used + $query = (string) model(UserModel::class)->getLastQuery(); + $this->assertMatchesRegularExpression( + '/WHERE\s+.*\s+IN\s+\([^)]+\)/i', + $query, + 'Groups and Permissions were not obtained with the single query (missing "WHERE ... IN" condition)', + ); + } + + public function testModelFindAllWithGroupsAndPermissions(): void + { + fake(GroupModel::class, ['user_id' => $this->user->id, 'group' => 'superadmin']); + fake(GroupModel::class, ['user_id' => $this->user->id, 'group' => 'admin']); + fake(PermissionModel::class, ['user_id' => $this->user->id, 'permission' => 'users.edit']); + fake(PermissionModel::class, ['user_id' => $this->user->id, 'permission' => 'users.delete']); + + $users = model(UserModel::class)->where('active', 1)->withGroups()->withPermissions()->findAll(); + + $this->assertCount(1, $users); + + $this->assertTrue($users[1]->can('users.delete')); + $this->assertTrue($users[1]->can('users.add')); + + // Check the last query and see if a proper type of query was used + $query = (string) model(UserModel::class)->getLastQuery(); + $this->assertMatchesRegularExpression( + '/WHERE\s+.*\s+IN\s+\([^)]+\)/i', + $query, + 'Groups and Permissions were not obtained with the single query (missing "WHERE ... IN" condition)', + ); + } + public function testLastLogin(): void { fake(