From 9d73add9ed65a6307d815ad7a9c89cdacb551999 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Halil=20=C4=B0brahim=20Kalkan?= <hi_kalkan@yahoo.com>
Date: Tue, 16 Aug 2022 11:53:51 +0300
Subject: [PATCH] Use a common distributed lock while updating permissions

Because different applications may try to change the same permissions (when permissions are located in a shared project).
---
 .../StaticPermissionSaver.cs                  | 59 +++++++++++++------
 1 file changed, 40 insertions(+), 19 deletions(-)

diff --git a/modules/permission-management/src/Volo.Abp.PermissionManagement.Domain/Volo/Abp/PermissionManagement/StaticPermissionSaver.cs b/modules/permission-management/src/Volo.Abp.PermissionManagement.Domain/Volo/Abp/PermissionManagement/StaticPermissionSaver.cs
index ed55e6f50e..f3f8674755 100644
--- a/modules/permission-management/src/Volo.Abp.PermissionManagement.Domain/Volo/Abp/PermissionManagement/StaticPermissionSaver.cs
+++ b/modules/permission-management/src/Volo.Abp.PermissionManagement.Domain/Volo/Abp/PermissionManagement/StaticPermissionSaver.cs
@@ -53,11 +53,10 @@ public class StaticPermissionSaver : IStaticPermissionSaver, ITransientDependenc
     {
         /* TODO: We may double check and lock for optimization 
          */
-        await using var handle = await DistributedLock.TryAcquireAsync(GetDistributedLockKey());
-
-        if (handle == null)
+        await using var applicationLockHandle = await DistributedLock.TryAcquireAsync(GetApplicationDistributedLockKey());
+        if (applicationLockHandle == null)
         {
-            /* Another instance already did it */
+            /* Another application instance is already doing it */
             return;
         }
         
@@ -84,28 +83,41 @@ public class StaticPermissionSaver : IStaticPermissionSaver, ITransientDependenc
         {
             return;
         }
-        
-        await UpdateChangedPermissionGroupsAsync(permissionGroupRecords);
-        await UpdateChangedPermissionsAsync(permissionRecords);
 
-        await Cache.SetStringAsync(
-            cacheKey,
-            currentHash,
-            new DistributedCacheEntryOptions {
-                SlidingExpiration = TimeSpan.FromDays(30) //TODO: Make it configurable?
+        await using (var commonLockHandle = await DistributedLock
+                         .TryAcquireAsync(GetCommonDistributedLockKey(), TimeSpan.FromMinutes(5)))
+        {
+            if (commonLockHandle == null)
+            {
+                /* It will re-try */
+                throw new AbpException("Could not acquire distributed lock for saving static permissions!");
             }
-        );
+
+            var hasChangesInGroups = await UpdateChangedPermissionGroupsAsync(permissionGroupRecords);
+            var hasChangesInPermissions = await UpdateChangedPermissionsAsync(permissionRecords);
+
+            if (hasChangesInGroups ||hasChangesInPermissions)
+            {
+                await Cache.SetStringAsync(
+                    GetCommonStampCacheKey(),
+                    Guid.NewGuid().ToString(),
+                    new DistributedCacheEntryOptions {
+                        SlidingExpiration = TimeSpan.FromDays(30) //TODO: Make it configurable?
+                    }
+                );
+            }
+        }
 
         await Cache.SetStringAsync(
-            GetCommonStampCacheKey(),
-            Guid.NewGuid().ToString(),
+            cacheKey,
+            currentHash,
             new DistributedCacheEntryOptions {
                 SlidingExpiration = TimeSpan.FromDays(30) //TODO: Make it configurable?
             }
         );
     }
 
-    private async Task UpdateChangedPermissionGroupsAsync(
+    private async Task<bool> UpdateChangedPermissionGroupsAsync(
         IEnumerable<PermissionGroupDefinitionRecord> permissionGroupRecords)
     {
         var newRecords = new List<PermissionGroupDefinitionRecord>();
@@ -156,9 +168,11 @@ public class StaticPermissionSaver : IStaticPermissionSaver, ITransientDependenc
         {
             await PermissionGroupRepository.DeleteManyAsync(deletedRecords);
         }
+        
+        return newRecords.Any() || changedRecords.Any() || deletedRecords.Any();
     }
     
-    private async Task UpdateChangedPermissionsAsync(
+    private async Task<bool> UpdateChangedPermissionsAsync(
         IEnumerable<PermissionDefinitionRecord> permissionRecords)
     {
         var newRecords = new List<PermissionDefinitionRecord>();
@@ -221,11 +235,18 @@ public class StaticPermissionSaver : IStaticPermissionSaver, ITransientDependenc
         {
             await PermissionRepository.DeleteManyAsync(deletedRecords);
         }
+        
+        return newRecords.Any() || changedRecords.Any() || deletedRecords.Any();
+    }
+
+    private string GetApplicationDistributedLockKey()
+    {
+        return $"{CacheOptions.KeyPrefix}_{ApplicationNameAccessor.ApplicationName}_AbpPermissionUpdateLock";
     }
 
-    private string GetDistributedLockKey()
+    private string GetCommonDistributedLockKey()
     {
-        return $"{ApplicationNameAccessor.ApplicationName}_AbpPermissionUpdateLock";
+        return $"{CacheOptions.KeyPrefix}_Common_AbpPermissionUpdateLock";
     }
 
     private string GetApplicationHashCacheKey()