From 7f4347182f851fd5e9f59869578d5c67507e0361 Mon Sep 17 00:00:00 2001 From: maliming Date: Sat, 24 Jan 2026 15:02:41 +0800 Subject: [PATCH] Fix key-value mapping in GetOrAddMany and add tests Refactored GetOrAddMany and GetOrAddManyAsync in DistributedCache to ensure returned key-value pairs are correctly mapped by key, regardless of order from the factory. Added unit tests to verify correct mapping and concurrency behavior. --- .../Volo/Abp/Caching/DistributedCache.cs | 64 ++++++++--------- .../Abp/Caching/DistributedCache_Tests.cs | 69 +++++++++++++++++++ 2 files changed, 96 insertions(+), 37 deletions(-) diff --git a/framework/src/Volo.Abp.Caching/Volo/Abp/Caching/DistributedCache.cs b/framework/src/Volo.Abp.Caching/Volo/Abp/Caching/DistributedCache.cs index ae571cb5f9..6ffaa96ecf 100644 --- a/framework/src/Volo.Abp.Caching/Volo/Abp/Caching/DistributedCache.cs +++ b/framework/src/Volo.Abp.Caching/Volo/Abp/Caching/DistributedCache.cs @@ -19,7 +19,7 @@ namespace Volo.Abp.Caching; /// Represents a distributed cache of type. /// /// The type of cache item being cached. -public class DistributedCache : +public class DistributedCache : IDistributedCache where TCacheItem : class { @@ -683,35 +683,30 @@ public class DistributedCache : IDistributedCache x.Value != null)) - { - return result!; - } + var resultMap = result + .Where(x => x.Value != null) + .ToDictionary(x => x.Key, x => x.Value); - var missingKeys = new List(); - var missingValuesIndex = new List(); - for (var i = 0; i < keyArray.Length; i++) + if (resultMap.Count == keyArray.Length) { - if (result[i].Value != null) - { - continue; - } - - missingKeys.Add(keyArray[i]); - missingValuesIndex.Add(i); + return keyArray + .Select(key => new KeyValuePair(key, resultMap[key])) + .ToArray(); } + var missingKeys = keyArray.Where(key => !resultMap.ContainsKey(key)).ToList(); var missingValues = factory.Invoke(missingKeys).ToArray(); - var valueQueue = new Queue>(missingValues); SetMany(missingValues, optionsFactory?.Invoke(), hideErrors, considerUow); - foreach (var index in missingValuesIndex) + foreach (var pair in missingValues) { - result[index] = valueQueue.Dequeue()!; + resultMap[pair.Key] = pair.Value; } - return result; + return keyArray + .Select(key => new KeyValuePair(key, resultMap.GetOrDefault(key))) + .ToArray(); } @@ -779,35 +774,30 @@ public class DistributedCache : IDistributedCache x.Value != null)) - { - return result; - } + var resultMap = result + .Where(x => x.Value != null) + .ToDictionary(x => x.Key, x => x.Value); - var missingKeys = new List(); - var missingValuesIndex = new List(); - for (var i = 0; i < keyArray.Length; i++) + if (resultMap.Count == keyArray.Length) { - if (result[i].Value != null) - { - continue; - } - - missingKeys.Add(keyArray[i]); - missingValuesIndex.Add(i); + return keyArray + .Select(key => new KeyValuePair(key, resultMap[key])) + .ToArray(); } + var missingKeys = keyArray.Where(key => !resultMap.ContainsKey(key)).ToList(); var missingValues = (await factory.Invoke(missingKeys)).ToArray(); - var valueQueue = new Queue>(missingValues); await SetManyAsync(missingValues, optionsFactory?.Invoke(), hideErrors, considerUow, token); - foreach (var index in missingValuesIndex) + foreach (var pair in missingValues) { - result[index] = valueQueue.Dequeue()!; + resultMap[pair.Key] = pair.Value; } - return result; + return keyArray + .Select(key => new KeyValuePair(key, resultMap.GetOrDefault(key))) + .ToArray(); } /// diff --git a/framework/test/Volo.Abp.Caching.Tests/Volo/Abp/Caching/DistributedCache_Tests.cs b/framework/test/Volo.Abp.Caching.Tests/Volo/Abp/Caching/DistributedCache_Tests.cs index c76f5be9ea..25cc4c9784 100644 --- a/framework/test/Volo.Abp.Caching.Tests/Volo/Abp/Caching/DistributedCache_Tests.cs +++ b/framework/test/Volo.Abp.Caching.Tests/Volo/Abp/Caching/DistributedCache_Tests.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Threading; using System.Threading.Tasks; using Shouldly; using Volo.Abp.Testing; @@ -751,6 +752,74 @@ public class DistributedCache_Tests : AbpIntegratedTest cacheValue[1].Value.Name.ShouldBe("jack"); } + [Fact] + public async Task GetOrAddManyAsync_Should_Return_Values_By_Key_With_Uow() + { + var key1 = "testkey"; + var key2 = "testkey2"; + var keys = new[] { key1, key2 }; + + using (GetRequiredService().Begin()) + { + var personCache = GetRequiredService>(); + + await personCache.SetAsync(key2, new PersonCacheItem("cached"), considerUow: true); + + var result = await personCache.GetOrAddManyAsync(keys, missingKeys => + { + missingKeys.ToArray().ShouldBe(new[] { key1 }); + return Task.FromResult(new List> + { + new(key1, new PersonCacheItem("factory")) + }); + }, considerUow: true); + + result.Length.ShouldBe(2); + result[0].Key.ShouldBe(key1); + result[0].Value.ShouldNotBeNull(); + result[0].Value.Name.ShouldBe("factory"); + result[1].Key.ShouldBe(key2); + result[1].Value.ShouldNotBeNull(); + result[1].Value.Name.ShouldBe("cached"); + } + } + + [Fact] + public async Task GetOrAddManyAsync_Should_Map_By_Key_Under_Concurrency() + { + var key1 = "testkey"; + var key2 = "testkey2"; + var keys = new[] { key1, key2 }; + + var personCache = GetRequiredService>(); + + async Task>> Factory(IEnumerable missingKeys) + { + await Task.Yield(); + + return missingKeys + .Reverse() + .Select(x => new KeyValuePair(x, new PersonCacheItem(x == key1 ? "v1" : "v2"))) + .ToList(); + } + + var task1 = personCache.GetOrAddManyAsync(keys, Factory); + var task2 = personCache.GetOrAddManyAsync(keys, Factory); + + var results = await Task.WhenAll(task1, task2); + + foreach (var result in results) + { + result.Length.ShouldBe(2); + + result[0].Key.ShouldBe(key1); + result[0].Value!.Name.ShouldBe("v1"); + + result[1].Key.ShouldBe(key2); + result[1].Value!.Name.ShouldBe("v2"); + } + } + [Fact] public async Task Cache_Should_Only_Available_In_Uow_For_GetOrAddManyAsync() {