diff --git a/CHANGELOG.md b/CHANGELOG.md index 3e9249e..04e9a05 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ # Changes in Medidata.MAuth - +## v5.1.5 +- **[Core]** Fix bug in MAuth caching response ## v5.1.4 - **[Core]** Change the fallback behavior of caching from 1 hour to 5 minutes in case when there is no valid caching instruction provided by the MAuth server diff --git a/src/Medidata.MAuth.Core/CacheExtensions.cs b/src/Medidata.MAuth.Core/CacheExtensions.cs deleted file mode 100644 index 23375a7..0000000 --- a/src/Medidata.MAuth.Core/CacheExtensions.cs +++ /dev/null @@ -1,49 +0,0 @@ -using System; -using Microsoft.Extensions.Caching.Memory; - -namespace Medidata.MAuth.Core -{ - /// - /// Caching-related extension methods. - /// - public static class CacheExtensions - { - /// - /// Get or create an item in the cache. If the specified key does not exist, the factory method will be executed to - /// create a new item, and that item will be inserted into the cache with that key. Locking is based on the cache - /// key, to prevent multiple concurrent factory methods from executing for a single key, but allow multiple concurrent - /// factory methods to execute for different keys. - /// - /// The memory cache. - /// The cache key. - /// The factory method to create a new item if the key does not exist in the cache. - /// The cached item type. - /// The item that was retrieved from the cache or created. - public static TItem GetOrCreateWithLock( - this IMemoryCache cache, - object key, - Func factory) - { - if (cache.TryGetValue(key, out TItem item)) - { - return item; - } - - var lockStr = string.Intern("mutex_" + key.GetHashCode()); - - lock (lockStr) - { - if (cache.TryGetValue(key, out item)) - { - return item; - } - - ICacheEntry entry = cache.CreateEntry(key); - item = factory(entry); - entry.SetValue(item); - entry.Dispose(); - return item; - } - } - } -} diff --git a/src/Medidata.MAuth.Core/AsyncLazy.cs b/src/Medidata.MAuth.Core/Caching/AsyncLazy.cs similarity index 70% rename from src/Medidata.MAuth.Core/AsyncLazy.cs rename to src/Medidata.MAuth.Core/Caching/AsyncLazy.cs index f9cf378..7bf481f 100644 --- a/src/Medidata.MAuth.Core/AsyncLazy.cs +++ b/src/Medidata.MAuth.Core/Caching/AsyncLazy.cs @@ -1,9 +1,11 @@ -using System; +using System; +using System.Diagnostics.CodeAnalysis; using System.Runtime.CompilerServices; using System.Threading.Tasks; -namespace Medidata.MAuth.Core +namespace Medidata.MAuth.Core.Caching { + [ExcludeFromCodeCoverage] internal class AsyncLazy : Lazy> { public AsyncLazy(Func valueFactory) @@ -17,8 +19,5 @@ public AsyncLazy(Func> taskFactory) } public TaskAwaiter GetAwaiter() => Value.GetAwaiter(); - - public ConfiguredTaskAwaitable ConfigureAwait(bool continueOnCapturedContext) - => Value.ConfigureAwait(continueOnCapturedContext); } } diff --git a/src/Medidata.MAuth.Core/Caching/CacheResult.cs b/src/Medidata.MAuth.Core/Caching/CacheResult.cs new file mode 100644 index 0000000..03f6ef6 --- /dev/null +++ b/src/Medidata.MAuth.Core/Caching/CacheResult.cs @@ -0,0 +1,26 @@ +using System; + +namespace Medidata.MAuth.Core.Caching +{ + /// + /// Dto to store cache fields + /// + /// Cache result type. + public class CacheResult + { + /// + /// Item to store in cache. + /// + public TItem Result { get; set; } + + /// + /// Flag to determine if the result should be cache. + /// + public bool ShouldCache { get; set; } + + /// + /// Cache duration. + /// + public TimeSpan AbsoluteCacheDuration { get; set; } + } +} \ No newline at end of file diff --git a/src/Medidata.MAuth.Core/Caching/ICacheService.cs b/src/Medidata.MAuth.Core/Caching/ICacheService.cs new file mode 100644 index 0000000..fe2efb5 --- /dev/null +++ b/src/Medidata.MAuth.Core/Caching/ICacheService.cs @@ -0,0 +1,28 @@ +using System; +using System.Threading.Tasks; + +namespace Medidata.MAuth.Core.Caching +{ + /// + /// Caching abstraction + /// + public interface ICacheService + { + /// + /// Gets or create cache item with locking. + /// + /// The cached item type. + /// Cache Key + /// Factory method to create cached item + /// The item that was retrieved from the cache or created. + Task GetOrCreateWithLock( + string key, + Func>> factory); + + /// + /// Remove key from cache if exists. + /// + /// + void Remove(string key); + } +} diff --git a/src/Medidata.MAuth.Core/Caching/MemoryCacheService.cs b/src/Medidata.MAuth.Core/Caching/MemoryCacheService.cs new file mode 100644 index 0000000..c7b9e5e --- /dev/null +++ b/src/Medidata.MAuth.Core/Caching/MemoryCacheService.cs @@ -0,0 +1,127 @@ +using System; +using System.Diagnostics.CodeAnalysis; +using System.Threading.Tasks; +using Microsoft.Extensions.Caching.Memory; + +namespace Medidata.MAuth.Core.Caching +{ + /// + /// In-Memory Cache service + /// + public class MemoryCacheService : ICacheService + { + private readonly IMemoryCache _cache; + + /// + /// In Memory Cache Service + /// + /// + public MemoryCacheService(IMemoryCache cache) + { + _cache = cache; + } + + /// + /// Checks if key is in the cache + /// + /// + /// + /// + /// True when key found in cache. + public bool TryGetValue(string key, out T value) + => _cache.TryGetValue(key, out value); + + /// + /// + /// + /// + public void Remove(string key) => _cache.Remove(key); + + /// + /// Add an item to the cache. + /// + /// Cache Key + /// Item to cache + /// Cache Expiration + public void SetItem(string key, T value, DateTimeOffset expiration) + => _cache.Set(key, value, expiration); + + /// + /// Gets or create cache item with locking. + /// + /// The cached item type. + /// Cache Key + /// Factory method to create cached item + /// Cached Item + public async Task GetOrCreateWithLock( + string key, + Func>> factory) + { + if (TryGetValue(key, out TItem item)) + { + return item; + } + + return await CreateAsyncLazyWithLock(key, factory); + } + + [ExcludeFromCodeCoverage] + private AsyncLazy CreateAsyncLazyWithLock(string key, Func>> factory) + { + var asyncKey = CreateAsyncKey(key); + + if (TryGetValue(asyncKey, out AsyncLazy asyncItem)) + { + return asyncItem; + } + + var lockStr = string.Intern("mutex_" + asyncKey); + + lock (lockStr) + { + if (TryGetValue(asyncKey, out asyncItem)) + { + return asyncItem; + } + + if (TryGetValue(key, out TItem item)) + { + return new AsyncLazy(() => item); + } + + asyncItem = new AsyncLazy(() => CreateLazyFactory(key, factory)); + + _cache.Set(asyncKey, asyncItem); + } + + return asyncItem; + } + + private async Task CreateLazyFactory(string key, Func>> factory) + { + try + { + var result = await factory().ConfigureAwait(false); + + if (!result.ShouldCache) + { + return result.Result; + } + + using var entry = _cache.CreateEntry(key); + entry.AbsoluteExpiration = DateTimeOffset.UtcNow + result.AbsoluteCacheDuration; + entry.SetValue(result.Result); + return result.Result; + } + finally + { + var asyncKey = CreateAsyncKey(key); + _cache.Remove(asyncKey); + } + + } + + private string CreateAsyncKey(string key) => $"{key}-async"; + + } +} diff --git a/src/Medidata.MAuth.Core/Caching/NoopCacheService.cs b/src/Medidata.MAuth.Core/Caching/NoopCacheService.cs new file mode 100644 index 0000000..77dd1df --- /dev/null +++ b/src/Medidata.MAuth.Core/Caching/NoopCacheService.cs @@ -0,0 +1,36 @@ +using System; +using System.Diagnostics.CodeAnalysis; +using System.Threading.Tasks; + +namespace Medidata.MAuth.Core.Caching +{ + /// + /// Noop Cache Service + /// + [ExcludeFromCodeCoverage] + public class NoopCacheService : ICacheService + { + /// + /// Gets or create cache item with locking. + /// + /// The cached item type. + /// Cache Key + /// Factory method to create cached item + /// Cached Item + public async Task GetOrCreateWithLock( + string key, + Func>> factory) + { + var data = await factory().ConfigureAwait(false); + return data.Result; + } + + /// + /// Remove key from cache if exists. + /// + /// + public void Remove(string key) + { + } + } +} diff --git a/src/Medidata.MAuth.Core/MAuthAuthenticator.cs b/src/Medidata.MAuth.Core/MAuthAuthenticator.cs index cee6438..fc1bf39 100644 --- a/src/Medidata.MAuth.Core/MAuthAuthenticator.cs +++ b/src/Medidata.MAuth.Core/MAuthAuthenticator.cs @@ -1,6 +1,7 @@ using System; using System.Net.Http; using System.Threading.Tasks; +using Medidata.MAuth.Core.Caching; using Medidata.MAuth.Core.Exceptions; using Microsoft.Extensions.Caching.Memory; using Org.BouncyCastle.Crypto; @@ -12,14 +13,14 @@ namespace Medidata.MAuth.Core { internal class MAuthAuthenticator { - private readonly IMemoryCache _cache; + private readonly ICacheService _cache; private readonly MAuthOptionsBase _options; private readonly ILogger _logger; private readonly Lazy _lazyHttpClient; public Guid ApplicationUuid => _options.ApplicationUuid; - public MAuthAuthenticator(MAuthOptionsBase options, ILogger logger, IMemoryCache cache = null) + public MAuthAuthenticator(MAuthOptionsBase options, ILogger logger, ICacheService cacheService = null) { if (options.ApplicationUuid == default) throw new ArgumentException(nameof(options.ApplicationUuid)); @@ -30,7 +31,7 @@ public MAuthAuthenticator(MAuthOptionsBase options, ILogger logger, IMemoryCache if (string.IsNullOrWhiteSpace(options.PrivateKey)) throw new ArgumentNullException(nameof(options.PrivateKey)); - _cache = cache ?? new MemoryCache(new MemoryCacheOptions()); + _cache = cacheService ?? new MemoryCacheService(new MemoryCache(new MemoryCacheOptions())); _options = options; _logger = logger; _lazyHttpClient = new Lazy(() => CreateHttpClient(options)); @@ -103,29 +104,17 @@ private async Task Authenticate(HttpRequestMessage request, MAuthVersion v var mAuthCore = MAuthCoreFactory.Instantiate(version); var authInfo = GetAuthenticationInfo(request, mAuthCore); + + var appInfo = await _cache.GetOrCreateWithLock( + authInfo.ApplicationUuid.ToString(), + () => SendApplicationInfoRequest(authInfo.ApplicationUuid)).ConfigureAwait(false); - try - { - var appInfo = await GetApplicationInfo(authInfo.ApplicationUuid).ConfigureAwait(false); + var signature = await mAuthCore.GetSignature(request, authInfo).ConfigureAwait(false); + return mAuthCore.Verify(authInfo.Payload, signature, appInfo.PublicKey); - var signature = await mAuthCore.GetSignature(request, authInfo).ConfigureAwait(false); - return mAuthCore.Verify(authInfo.Payload, signature, appInfo.PublicKey); - } - catch (RetriedRequestException) - { - // If the appliation info could not be fetched, remove the lazy - // object from the cache to allow for another attempt. - _cache.Remove(authInfo.ApplicationUuid); - throw; } - } - - private AsyncLazy GetApplicationInfo(Guid applicationUuid) => - _cache.GetOrCreateWithLock( - applicationUuid, - entry => new AsyncLazy(() => SendApplicationInfoRequest(entry, applicationUuid))); - - private async Task SendApplicationInfoRequest(ICacheEntry entry, Guid applicationUuid) + + private async Task> SendApplicationInfoRequest(Guid applicationUuid) { var logMessage = "Mauth-client requesting from mAuth service application info not available " + $"in the local cache for app uuid {applicationUuid}."; @@ -139,16 +128,19 @@ private async Task SendApplicationInfoRequest(ICacheEntry entry ).ConfigureAwait(false); var result = await response.Content.FromResponse().ConfigureAwait(false); - - entry.SetOptions( - new MemoryCacheEntryOptions() - .SetAbsoluteExpiration(response.Headers.CacheControl?.MaxAge ?? TimeSpan.FromMinutes(5)) - ); - + logMessage = $"Mauth-client application info for app uuid {applicationUuid} cached in memory."; _logger.LogInformation(logMessage); - return result; + var cacheResult = new CacheResult + { + ShouldCache = true, + Result = result, + AbsoluteCacheDuration = response.Headers.CacheControl?.MaxAge ?? TimeSpan.FromMinutes(5), + }; + + return cacheResult; + } /// diff --git a/src/Medidata.MAuth.Core/UtilityExtensions.cs b/src/Medidata.MAuth.Core/UtilityExtensions.cs index 8a6c8b8..5b61ba9 100644 --- a/src/Medidata.MAuth.Core/UtilityExtensions.cs +++ b/src/Medidata.MAuth.Core/UtilityExtensions.cs @@ -1,6 +1,7 @@ using System; using System.Net.Http; using System.Threading.Tasks; +using Medidata.MAuth.Core.Caching; using Medidata.MAuth.Core.Models; using Microsoft.Extensions.Caching.Memory; using Microsoft.Extensions.Logging; @@ -13,7 +14,8 @@ namespace Medidata.MAuth.Core /// public static class UtilityExtensions { - private static readonly IMemoryCache _cache = new MemoryCache(new MemoryCacheOptions()); + private static readonly ICacheService _cache + = new MemoryCacheService(new MemoryCache(new MemoryCacheOptions())); /// /// Parses an MAuth authentication HTTP header value for the application uuid and the MAuth signature payload. diff --git a/version.props b/version.props index 17b6840..89f0dbf 100644 --- a/version.props +++ b/version.props @@ -1,6 +1,6 @@  - 5.1.4 + 5.1.5