Handle duplicate setting, feature and permission records

modules/feature-management/src/Volo.Abp.FeatureManagement.Domain/Volo/Abp/FeatureManagement/FeatureManagementStore.cs

@@ -1,6 +1,8 @@
 using System.Collections.Generic;
 using System.Linq;
 using System.Threading.Tasks;
+using Microsoft.Extensions.Logging;
+using Microsoft.Extensions.Logging.Abstractions;
 using Volo.Abp.Caching;
 using Volo.Abp.DependencyInjection;
 using Volo.Abp.Features;
@@ -11,6 +13,8 @@ namespace Volo.Abp.FeatureManagement;
 
 public class FeatureManagementStore : IFeatureManagementStore, ITransientDependency
 {
+    public ILogger<FeatureManagementStore> Logger { get; set; }
+
     protected IDistributedCache<FeatureValueCacheItem> Cache { get; }
     protected IFeatureDefinitionManager FeatureDefinitionManager { get; }
     protected IFeatureValueRepository FeatureValueRepository { get; }
@@ -26,6 +30,7 @@ public FeatureManagementStore(
         GuidGenerator = guidGenerator;
         Cache = cache;
         FeatureDefinitionManager = featureDefinitionManager;
+        Logger = NullLogger<FeatureManagementStore>.Instance;
     }
 
     [UnitOfWork]
@@ -38,7 +43,7 @@ public virtual async Task<string> GetOrNullAsync(string name, string providerNam
     [UnitOfWork]
     public virtual async Task SetAsync(string name, string value, string providerName, string providerKey)
     {
-        var featureValue = await FeatureValueRepository.FindAsync(name, providerName, providerKey);
+        var featureValue = await FindAndDeleteDuplicatesAsync(name, providerName, providerKey);
         if (featureValue == null)
         {
             featureValue = new FeatureValue(GuidGenerator.Create(), name, value, providerName, providerKey);
@@ -88,8 +93,10 @@ private async Task SetCacheItemsAsync(
         FeatureValueCacheItem currentCacheItem)
     {
         var featureDefinitions = await FeatureDefinitionManager.GetAllAsync();
-        var featuresDictionary = (await FeatureValueRepository.GetListAsync(providerName, providerKey))
-            .ToDictionary(s => s.Name, s => s.Value);
+        var featuresDictionary = await CreateFeatureValueDictionaryAsync(
+            await FeatureValueRepository.GetListAsync(providerName, providerKey),
+            providerName,
+            providerKey);
 
         var cacheItems = new List<KeyValuePair<string, FeatureValueCacheItem>>();
 
@@ -113,6 +120,51 @@ private async Task SetCacheItemsAsync(
         await Cache.SetManyAsync(cacheItems, considerUow: true);
     }
 
+    protected virtual async Task<FeatureValue> FindAndDeleteDuplicatesAsync(string name, string providerName, string providerKey)
+    {
+        var featureValue = await FeatureValueRepository.FindAsync(name, providerName, providerKey);
+        if (featureValue == null)
+        {
+            return null;
+        }
+
+        var duplicates = (await FeatureValueRepository.FindAllAsync(name, providerName, providerKey))
+            .Where(x => x.Id != featureValue.Id);
+        foreach (var duplicate in duplicates)
+        {
+            await FeatureValueRepository.DeleteAsync(duplicate, true);
+        }
+
+        return featureValue;
+    }
+
+    protected virtual async Task<Dictionary<string, string>> CreateFeatureValueDictionaryAsync(
+        List<FeatureValue> featureValues,
+        string providerName,
+        string providerKey)
+    {
+        var featuresDictionary = new Dictionary<string, string>();
+
+        foreach (var featureValueGroup in featureValues.GroupBy(s => s.Name))
+        {
+            if (featureValueGroup.Count() == 1)
+            {
+                featuresDictionary[featureValueGroup.Key] = featureValueGroup.First().Value;
+                continue;
+            }
+
+            // FindAsync returns the same record that SetAsync updates, so reads and writes agree until the duplicates are deleted.
+            Logger.LogWarning(
+                "Found {Count} feature value records for Name = {Name}, ProviderName = {ProviderName}, ProviderKey = {ProviderKey}. The duplicates will be deleted the next time the feature value is saved.",
+                featureValueGroup.Count(), featureValueGroup.Key, providerName, providerKey);
+
+            var featureValue = await FeatureValueRepository.FindAsync(featureValueGroup.Key, providerName, providerKey);
+            featuresDictionary[featureValueGroup.Key] = (featureValue ?? featureValueGroup.First()).Value;
+        }
+
+        return featuresDictionary;
+    }
+
     protected virtual string CalculateCacheKey(string name, string providerName, string providerKey)
     {
         return FeatureValueCacheItem.CalculateCacheKey(name, providerName, providerKey);

modules/feature-management/test/Volo.Abp.FeatureManagement.TestBase/Volo/Abp/FeatureManagement/FeatureManagementStore_Tests.cs

@@ -141,4 +141,52 @@ await FeatureManagementStore.DeleteAsync(TestFeatureDefinitionProvider.SocialLog
                 TestEditionIds.Regular.ToString())).ShouldBeNull();
         }
     }
+
+    [Fact]
+    public async Task GetOrNullAsync_Should_Not_Fail_When_Duplicate_Host_Values_Exist()
+    {
+        // Arrange
+        await InsertDuplicateHostFeatureValuesAsync(TestFeatureDefinitionProvider.SocialLogins);
+
+        // Act & Assert
+        (await FeatureManagementStore.GetOrNullAsync(TestFeatureDefinitionProvider.UserCount,
+            TenantFeatureValueProvider.ProviderName,
+            null)).ShouldBeNull();
+
+        (await FeatureManagementStore.GetOrNullAsync(TestFeatureDefinitionProvider.SocialLogins,
+            TenantFeatureValueProvider.ProviderName,
+            null)).ShouldBe((await FeatureValueRepository.FindAsync(TestFeatureDefinitionProvider.SocialLogins,
+            TenantFeatureValueProvider.ProviderName,
+            null)).Value);
+    }
+
+    [Fact]
+    public async Task SetAsync_Should_Remove_Duplicate_Host_Values()
+    {
+        // Arrange
+        await InsertDuplicateHostFeatureValuesAsync(TestFeatureDefinitionProvider.SocialLogins);
+
+        // Act
+        await FeatureManagementStore.SetAsync(TestFeatureDefinitionProvider.SocialLogins,
+            false.ToString().ToUpperInvariant(),
+            TenantFeatureValueProvider.ProviderName,
+            null);
+
+        // Assert
+        var featureValues = await FeatureValueRepository.FindAllAsync(TestFeatureDefinitionProvider.SocialLogins,
+            TenantFeatureValueProvider.ProviderName,
+            null);
+        featureValues.Count.ShouldBe(1);
+        featureValues[0].Value.ShouldBe(false.ToString().ToUpperInvariant());
+    }
+
+    private async Task InsertDuplicateHostFeatureValuesAsync(string name)
+    {
+        await FeatureValueRepository.InsertAsync(new FeatureValue(Guid.NewGuid(), name,
+            true.ToString().ToLowerInvariant(), TenantFeatureValueProvider.ProviderName, null));
+        await FeatureValueRepository.InsertAsync(new FeatureValue(Guid.NewGuid(), name,
+            false.ToString().ToLowerInvariant(), TenantFeatureValueProvider.ProviderName, null));
+
+        (await FeatureValueRepository.FindAllAsync(name, TenantFeatureValueProvider.ProviderName, null)).Count.ShouldBe(2);
+    }
 }

modules/permission-management/src/Volo.Abp.PermissionManagement.Domain/Volo/Abp/PermissionManagement/PermissionManagementProvider.cs

@@ -78,12 +78,10 @@ protected virtual async Task GrantAsync(string name, string providerKey)
 
     protected virtual async Task RevokeAsync(string name, string providerKey)
     {
-        var permissionGrant = await PermissionGrantRepository.FindAsync(name, Name, providerKey);
-        if (permissionGrant == null)
+        var permissionGrants = await PermissionGrantRepository.GetListAsync(new[] { name }, Name, providerKey);
+        foreach (var permissionGrant in permissionGrants)
         {
-            return;
+            await PermissionGrantRepository.DeleteAsync(permissionGrant, true);
         }
-
-        await PermissionGrantRepository.DeleteAsync(permissionGrant, true);
     }
 }

modules/permission-management/src/Volo.Abp.PermissionManagement.Domain/Volo/Abp/PermissionManagement/ResourcePermissionManagementProvider.cs

@@ -81,12 +81,10 @@ protected virtual async Task GrantAsync(string name, string resourceName, string
 
     protected virtual async Task RevokeAsync(string name, string resourceName,string resourceKey, string providerKey)
     {
-        var resourcePermissionGrants = await ResourcePermissionGrantRepository.FindAsync(name, resourceName, resourceKey, Name, providerKey);
-        if (resourcePermissionGrants == null)
+        var resourcePermissionGrants = await ResourcePermissionGrantRepository.GetListAsync(new[] { name }, resourceName, resourceKey, Name, providerKey);
+        foreach (var resourcePermissionGrant in resourcePermissionGrants)
         {
-            return;
+            await ResourcePermissionGrantRepository.DeleteAsync(resourcePermissionGrant, true);
         }
-
-        await ResourcePermissionGrantRepository.DeleteAsync(resourcePermissionGrants, true);
     }
 }

modules/permission-management/src/Volo.Abp.PermissionManagement.Domain/Volo/Abp/PermissionManagement/ResourcePermissionManager.cs

@@ -375,8 +375,8 @@ public virtual async Task DeleteAsync(string name, string resourceName, string r
             throw new AbpException($"The resource permission management provider '{providerName}' is not available in the current context.");
         }
 
-        var permissionGrant = await ResourcePermissionGrantRepository.FindAsync(name, resourceName, resourceKey, providerName, providerKey);
-        if (permissionGrant != null)
+        var permissionGrants = await ResourcePermissionGrantRepository.GetListAsync(new[] { name }, resourceName, resourceKey, providerName, providerKey);
+        foreach (var permissionGrant in permissionGrants)
         {
             await ResourcePermissionGrantRepository.DeleteAsync(permissionGrant, true);
         }

modules/permission-management/test/Volo.Abp.PermissionManagement.Domain.Tests/Volo/Abp/PermissionManagement/PermissionManagementProvider_Tests.cs

@@ -75,4 +75,19 @@ await _permissionManagementProvider.SetAsync("MyPermission1",
             "Test")).ShouldBeNull();
     }
 
+    [Fact]
+    public async Task SetAsync_Should_Revoke_Duplicate_Host_Grants()
+    {
+        await _permissionGrantRepository.InsertAsync(new PermissionGrant(Guid.NewGuid(), "MyPermission1", "Test", "Test"));
+        await _permissionGrantRepository.InsertAsync(new PermissionGrant(Guid.NewGuid(), "MyPermission1", "Test", "Test"));
+        (await _permissionGrantRepository.GetListAsync(new[] { "MyPermission1" }, "Test", "Test")).Count.ShouldBe(2);
+
+        await _permissionManagementProvider.SetAsync("MyPermission1",
+            "Test",
+            false);
+
+        (await _permissionGrantRepository.FindAsync("MyPermission1",
+            "Test",
+            "Test")).ShouldBeNull();
+    }
 }

modules/permission-management/test/Volo.Abp.PermissionManagement.Domain.Tests/Volo/Abp/PermissionManagement/ResourcePermissionManagementProvider_Tests.cs

@@ -87,4 +87,39 @@ await _resourcePermissionManagementProvider.SetAsync("MyPermission1",
             "Test",
             "Test")).ShouldBeNull();
     }
+
+    [Fact]
+    public async Task SetAsync_Should_Revoke_Duplicate_Host_Grants()
+    {
+        for (var i = 0; i < 2; i++)
+        {
+            await _resourcePermissionGrantRepository.InsertAsync(
+                new ResourcePermissionGrant(
+                    Guid.NewGuid(),
+                    "MyPermission1",
+                    TestEntityResource.ResourceName,
+                    TestEntityResource.ResourceKey1,
+                    "Test",
+                    "Test"
+                )
+            );
+        }
+        (await _resourcePermissionGrantRepository.GetListAsync(new[] { "MyPermission1" },
+            TestEntityResource.ResourceName,
+            TestEntityResource.ResourceKey1,
+            "Test",
+            "Test")).Count.ShouldBe(2);
+
+        await _resourcePermissionManagementProvider.SetAsync("MyPermission1",
+            TestEntityResource.ResourceName,
+            TestEntityResource.ResourceKey1,
+            "Test",
+            false);
+
+        (await _resourcePermissionGrantRepository.FindAsync("MyPermission1",
+            TestEntityResource.ResourceName,
+            TestEntityResource.ResourceKey1,
+            "Test",
+            "Test")).ShouldBeNull();
+    }
 }

modules/permission-management/test/Volo.Abp.PermissionManagement.Domain.Tests/Volo/Abp/PermissionManagement/ResourcePermissionManager_Tests.cs

@@ -341,6 +341,41 @@ await _resourcePermissionManager.DeleteAsync(
             "Test")).ShouldBeNull();
     }
 
+    [Fact]
+    public async Task DeleteByNameAsync_Should_Delete_Duplicate_Host_Grants()
+    {
+        for (var i = 0; i < 2; i++)
+        {
+            await _resourcePermissionGrantRepository.InsertAsync(new ResourcePermissionGrant(
+                Guid.NewGuid(),
+                "MyResourcePermission1",
+                TestEntityResource.ResourceName,
+                TestEntityResource.ResourceKey1,
+                "Test",
+                "Test")
+            );
+        }
+        (await _resourcePermissionGrantRepository.GetListAsync(new[] { "MyResourcePermission1" },
+            TestEntityResource.ResourceName,
+            TestEntityResource.ResourceKey1,
+            "Test",
+            "Test")).Count.ShouldBe(2);
+
+        await _resourcePermissionManager.DeleteAsync(
+            "MyResourcePermission1",
+            TestEntityResource.ResourceName,
+            TestEntityResource.ResourceKey1,
+            "Test",
+            "Test");
+
+        (await _resourcePermissionGrantRepository.FindAsync(
+            "MyResourcePermission1",
+            TestEntityResource.ResourceName,
+            TestEntityResource.ResourceKey1,
+            "Test",
+            "Test")).ShouldBeNull();
+    }
+
     [Fact]
     public async Task GetProviderKeyLookupServicesAsync_Should_Not_Return_Unavailable_Services()
     {