diff --git a/custom_templates/auth/OAuthAuthenticator.mustache b/custom_templates/auth/OAuthAuthenticator.mustache index f726c092..82c896b0 100644 --- a/custom_templates/auth/OAuthAuthenticator.mustache +++ b/custom_templates/auth/OAuthAuthenticator.mustache @@ -2,6 +2,7 @@ using System; using System.Collections.Concurrent; +using System.Threading; using System.Threading.Tasks; using Newtonsoft.Json; using RestSharp; @@ -32,6 +33,11 @@ namespace {{packageName}}.Client.Auth /// static readonly ConcurrentDictionary _tokenCache = new ConcurrentDictionary(); + /// + /// One lock per cache key so concurrent cache misses share a single token request instead of each fetching. + /// + static readonly ConcurrentDictionary> _fetchLocks = new ConcurrentDictionary>(); + sealed class CachedToken { public string Token { get; set; } @@ -92,13 +98,40 @@ namespace {{packageName}}.Client.Auth /// /// An authentication token. async Task GetCachedOrFetchToken() + { + if (TryGetCachedToken(out var token)) + { + return token; + } + + var fetchLock = _fetchLocks.GetOrAdd(CacheKey, _ => new Lazy(() => new SemaphoreSlim(1, 1))).Value; + await fetchLock.WaitAsync().ConfigureAwait(false); + try + { + // Re-check: a caller that held the lock before us may have already populated the cache. + if (TryGetCachedToken(out token)) + { + return token; + } + + return await GetToken().ConfigureAwait(false); + } + finally + { + fetchLock.Release(); + } + } + + bool TryGetCachedToken(out string token) { if (_tokenCache.TryGetValue(CacheKey, out var cached) && cached.ExpiresAtUtc > DateTime.UtcNow) { - return cached.Token; + token = cached.Token; + return true; } - return await GetToken().ConfigureAwait(false); + token = null; + return false; } /// diff --git a/src/Bandwidth.Standard.Test/Unit/Client/OAuthAuthenticatorTests.cs b/src/Bandwidth.Standard.Test/Unit/Client/OAuthAuthenticatorTests.cs index dd499d6c..6059b08a 100644 --- a/src/Bandwidth.Standard.Test/Unit/Client/OAuthAuthenticatorTests.cs +++ b/src/Bandwidth.Standard.Test/Unit/Client/OAuthAuthenticatorTests.cs @@ -10,6 +10,7 @@ using System; +using System.Linq; using System.Net; using System.Net.Sockets; using System.Text; @@ -42,6 +43,9 @@ public class OAuthAuthenticatorTests : IDisposable private int _expiresInSeconds = 3600; private bool _includeExpiresIn = true; + // Artificial latency on the fake endpoint so concurrent callers overlap on a cache miss. + private int _responseDelayMs = 0; + public OAuthAuthenticatorTests() { // Unique client id per test keeps the authenticator's process-wide static @@ -110,6 +114,22 @@ public async Task GetAuthenticationParameter_WithoutExpiresIn_FetchesEachTime() Assert.Equal(2, _tokenRequestCount); } + /// + /// Concurrent callers hitting a cold cache should share a single token request rather than each fetching. + /// + [Fact] + public async Task GetAuthenticationParameter_ConcurrentColdCache_FetchesOnlyOnce() + { + _responseDelayMs = 200; + var authenticator = CreateAuthenticator(); + + var results = await Task.WhenAll( + Enumerable.Range(0, 50).Select(_ => authenticator.GetAuthHeaderAsync())); + + Assert.All(results, r => Assert.Equal($"Bearer {AccessToken}", r)); + Assert.Equal(1, _tokenRequestCount); + } + private TestableOAuthAuthenticator CreateAuthenticator() { return new TestableOAuthAuthenticator( @@ -139,6 +159,9 @@ private async Task ServeTokenRequestsAsync() Interlocked.Increment(ref _tokenRequestCount); _lastAuthorizationHeader = context.Request.Headers["Authorization"]; + if (_responseDelayMs > 0) + await Task.Delay(_responseDelayMs); + string body = _includeExpiresIn ? $"{{\"token_type\":\"Bearer\",\"access_token\":\"{AccessToken}\",\"expires_in\":{_expiresInSeconds}}}" : $"{{\"token_type\":\"Bearer\",\"access_token\":\"{AccessToken}\"}}"; diff --git a/src/Bandwidth.Standard/Client/Auth/OAuthAuthenticator.cs b/src/Bandwidth.Standard/Client/Auth/OAuthAuthenticator.cs index 9c9da17c..f53d6f08 100644 --- a/src/Bandwidth.Standard/Client/Auth/OAuthAuthenticator.cs +++ b/src/Bandwidth.Standard/Client/Auth/OAuthAuthenticator.cs @@ -11,6 +11,7 @@ using System; using System.Collections.Concurrent; +using System.Threading; using System.Threading.Tasks; using Newtonsoft.Json; using RestSharp; @@ -41,6 +42,11 @@ public class OAuthAuthenticator : AuthenticatorBase /// static readonly ConcurrentDictionary _tokenCache = new ConcurrentDictionary(); + /// + /// One lock per cache key so concurrent cache misses share a single token request instead of each fetching. + /// + static readonly ConcurrentDictionary> _fetchLocks = new ConcurrentDictionary>(); + sealed class CachedToken { public string Token { get; set; } @@ -101,13 +107,40 @@ protected override async ValueTask GetAuthenticationParameter(string /// /// An authentication token. async Task GetCachedOrFetchToken() + { + if (TryGetCachedToken(out var token)) + { + return token; + } + + var fetchLock = _fetchLocks.GetOrAdd(CacheKey, _ => new Lazy(() => new SemaphoreSlim(1, 1))).Value; + await fetchLock.WaitAsync().ConfigureAwait(false); + try + { + // Re-check: a caller that held the lock before us may have already populated the cache. + if (TryGetCachedToken(out token)) + { + return token; + } + + return await GetToken().ConfigureAwait(false); + } + finally + { + fetchLock.Release(); + } + } + + bool TryGetCachedToken(out string token) { if (_tokenCache.TryGetValue(CacheKey, out var cached) && cached.ExpiresAtUtc > DateTime.UtcNow) { - return cached.Token; + token = cached.Token; + return true; } - return await GetToken().ConfigureAwait(false); + token = null; + return false; } ///