From e508d3dbd10662df93a4cb65e81884459d32ae6c Mon Sep 17 00:00:00 2001 From: Roly Gutierrez Date: Fri, 17 Jul 2026 17:18:11 -0400 Subject: [PATCH] FOUR-32256 [SPIKE] Analisis in the pages mentioned in the IDEA-1400 and add the reason for the delay in the pages mentioned. Description: Reduce duplicate permission queries on menu and auth checks. Cache permission names via Permission::cachedNames() and user permission groups via User::cachedPermissionGroups(), with invalidation through PermissionCacheService. Replaces repeated permissions()->pluck('group') and Permission::all() calls in GenerateMenus, User, and AuthServiceProvider. Going forward: memoize repeated reads per request with once(), batch related data with eager load/preload, and use Cache::remember only for cross-request data with explicit invalidation on writes. Related tickets: https://processmaker.atlassian.net/browse/FOUR-32256 --- .../Http/Middleware/GenerateMenus.php | 2 +- ProcessMaker/Models/Permission.php | 14 +++++-- ProcessMaker/Models/User.php | 2 +- .../Providers/AuthServiceProvider.php | 4 +- .../Services/PermissionCacheService.php | 38 +++++++++++++++++++ ProcessMaker/Traits/HasAuthorization.php | 9 +++++ .../Services/PermissionCacheServiceTest.php | 3 ++ 7 files changed, 63 insertions(+), 9 deletions(-) diff --git a/ProcessMaker/Http/Middleware/GenerateMenus.php b/ProcessMaker/Http/Middleware/GenerateMenus.php index bd8339fa39..f60536d283 100644 --- a/ProcessMaker/Http/Middleware/GenerateMenus.php +++ b/ProcessMaker/Http/Middleware/GenerateMenus.php @@ -353,7 +353,7 @@ public static function userHasPermission($permission) return $user && $user->can($permission) && $user->hasPermission($permission); } - $userPermissions = $user->permissions()->pluck('group')->unique()->toArray(); + $userPermissions = $user->cachedPermissionGroups(); $defaultPermissions = Permission::DEFAULT_PERMISSIONS; $userWithDefaultPermissions = empty(array_diff($userPermissions, $defaultPermissions)); diff --git a/ProcessMaker/Models/Permission.php b/ProcessMaker/Models/Permission.php index aa58674614..534dbcb678 100644 --- a/ProcessMaker/Models/Permission.php +++ b/ProcessMaker/Models/Permission.php @@ -98,6 +98,13 @@ public static function byResource($resource) return $filtered; } + public static function cachedNames(): array + { + return Cache::remember('permissions', 86400, function () { + return self::pluck('name')->toArray(); + }); + } + public static function byName($name) { try { @@ -139,7 +146,7 @@ public static function getUsersByGroup(array $groups) ->on('assignables.assignable_id', '=', 'users.id'); }) ->select('users.*') - ->union(\ProcessMaker\Models\User::where('is_administrator', '=', true)) + ->union(User::where('is_administrator', '=', true)) ->groupBy('users.id') ->get(); @@ -148,8 +155,7 @@ public static function getUsersByGroup(array $groups) private static function clearAndRebuildCache() { - // Rebuild and update the permissions cache - $permissions = self::pluck('name')->toArray(); - Cache::put('permissions', $permissions, 86400); + Cache::forget('permissions'); + self::cachedNames(); } } diff --git a/ProcessMaker/Models/User.php b/ProcessMaker/Models/User.php index c13c133659..e149cb73d4 100644 --- a/ProcessMaker/Models/User.php +++ b/ProcessMaker/Models/User.php @@ -267,7 +267,7 @@ public function getFullName() public function hasPermissionsFor(...$resources) { if ($this->is_administrator) { - $perms = Permission::all(['name'])->pluck('name'); + $perms = collect(Permission::cachedNames()); } else { $perms = collect(session('permissions')); } diff --git a/ProcessMaker/Providers/AuthServiceProvider.php b/ProcessMaker/Providers/AuthServiceProvider.php index fc9f68c952..36858333c5 100644 --- a/ProcessMaker/Providers/AuthServiceProvider.php +++ b/ProcessMaker/Providers/AuthServiceProvider.php @@ -92,9 +92,7 @@ public function defineGates() try { // Cache the permissions for a day to improve performance - $permissions = Cache::remember('permissions', 86400, function () { - return Permission::pluck('name')->toArray(); - }); + $permissions = Permission::cachedNames(); foreach ($permissions as $permission) { Gate::define($permission, function ($user) use ($permission) { return $user->hasPermission($permission); diff --git a/ProcessMaker/Services/PermissionCacheService.php b/ProcessMaker/Services/PermissionCacheService.php index 36f03adba0..7973008185 100644 --- a/ProcessMaker/Services/PermissionCacheService.php +++ b/ProcessMaker/Services/PermissionCacheService.php @@ -18,6 +18,8 @@ class PermissionCacheService implements PermissionCacheInterface private const LEGACY_USER_PERMISSIONS_KEY = 'user'; + private const USER_PERMISSION_GROUPS_KEY = 'user_permission_groups'; + private const TRACKED_PERMISSION_KEYS = 'permission_cache_keys'; private const TRACKED_PERMISSION_KEYS_LOCK = 'permission_cache_keys_lock'; @@ -91,11 +93,42 @@ public function cacheGroupPermissions(int $groupId, array $permissions): void /** * Invalidate user permissions cache */ + public function rememberUserPermissionGroups(int $userId, int $ttl, callable $callback): array + { + $key = $this->getUserPermissionGroupsKey($userId); + + try { + $groups = Cache::remember($key, $ttl, $callback); + $this->trackPermissionKey($key); + + return is_array($groups) ? $groups : []; + } catch (\Exception $e) { + Log::warning("Failed to remember user permission groups for user {$userId}: " . $e->getMessage()); + + $groups = $callback(); + + return is_array($groups) ? $groups : []; + } + } + + public function forgetUserPermissionGroups(int $userId): void + { + $key = $this->getUserPermissionGroupsKey($userId); + + try { + Cache::forget($key); + $this->untrackPermissionKey($key); + } catch (\Exception $e) { + Log::warning("Failed to forget user permission groups for user {$userId}: " . $e->getMessage()); + } + } + public function invalidateUserPermissions(int $userId): void { $keys = [ $this->getUserPermissionsKey($userId), $this->getLegacyUserPermissionsKey($userId), + $this->getUserPermissionGroupsKey($userId), ]; try { @@ -216,6 +249,11 @@ private function getLegacyUserPermissionsKey(int $userId): string return self::LEGACY_USER_PERMISSIONS_KEY . "_{$userId}_permissions"; } + private function getUserPermissionGroupsKey(int $userId): string + { + return self::USER_PERMISSION_GROUPS_KEY . ":{$userId}"; + } + /** * Warm up cache for a user */ diff --git a/ProcessMaker/Traits/HasAuthorization.php b/ProcessMaker/Traits/HasAuthorization.php index 2314b7e8cd..8ed42f3ca1 100644 --- a/ProcessMaker/Traits/HasAuthorization.php +++ b/ProcessMaker/Traits/HasAuthorization.php @@ -44,6 +44,15 @@ function () use ($user) { return $this->addCategoryViewPermissions($permissions); } + public function cachedPermissionGroups(): array + { + return app(PermissionCacheService::class)->rememberUserPermissionGroups( + $this->id, + 86400, + fn () => $this->permissions()->pluck('group')->unique()->values()->toArray() + ); + } + public function loadGroupPermissions() { $processedGroups = []; diff --git a/tests/unit/ProcessMaker/Services/PermissionCacheServiceTest.php b/tests/unit/ProcessMaker/Services/PermissionCacheServiceTest.php index cb26cda6da..7f634d55ed 100644 --- a/tests/unit/ProcessMaker/Services/PermissionCacheServiceTest.php +++ b/tests/unit/ProcessMaker/Services/PermissionCacheServiceTest.php @@ -127,10 +127,12 @@ public function test_invalidate_user_permissions_clears_cache_correctly() // Cache user permissions $this->cacheService->cacheUserPermissions($this->userId, $this->userPermissions); $this->cacheService->putLegacyUserPermissions($this->userId, $this->userPermissions, 3600); + $this->cacheService->rememberUserPermissionGroups($this->userId, 3600, fn () => ['Projects', 'Process Catalog']); // Verify cache exists $this->assertNotNull(Cache::get("user_permissions:{$this->userId}")); $this->assertNotNull(Cache::get("user_{$this->userId}_permissions")); + $this->assertNotNull(Cache::get("user_permission_groups:{$this->userId}")); // Invalidate cache $this->cacheService->invalidateUserPermissions($this->userId); @@ -138,6 +140,7 @@ public function test_invalidate_user_permissions_clears_cache_correctly() // Verify cache was cleared $this->assertNull(Cache::get("user_permissions:{$this->userId}")); $this->assertNull(Cache::get("user_{$this->userId}_permissions")); + $this->assertNull(Cache::get("user_permission_groups:{$this->userId}")); } /**