Skip to content

Commit

Permalink
Update in-memory cache expiry to take into account user-provided value (
Browse files Browse the repository at this point in the history
#2514)

* Update in-memory cache expiry to take into account user-provided value. Add tests. Update comments. Rename variables.

* Typo.

* Update comment.
  • Loading branch information
pmaytak authored Oct 20, 2023
1 parent eb5233f commit 51de97e
Show file tree
Hide file tree
Showing 7 changed files with 123 additions and 48 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ public partial class MsalDistributedTokenCacheAdapter : MsalAbstractTokenCachePr
internal /*for tests*/ readonly IDistributedCache _distributedCache;
internal /*for tests*/ readonly MemoryCache? _memoryCache;
private readonly ILogger<MsalDistributedTokenCacheAdapter> _logger;
private readonly TimeSpan? _expirationTime;
private readonly TimeSpan? _memoryCacheExpirationTime;
private readonly string _distributedCacheType = "DistributedCache"; // for logging
private readonly string _memoryCacheType = "MemoryCache"; // for logging
private const string DefaultPurpose = "msal_cache";
Expand Down Expand Up @@ -69,7 +69,7 @@ public MsalDistributedTokenCacheAdapter(
throw new ArgumentOutOfRangeException(nameof(_distributedCacheOptions.L1ExpirationTimeRatio), "L1ExpirationTimeRatio must be greater than 0, less than 1. ");
}

_expirationTime = TimeSpan.FromMilliseconds(_distributedCacheOptions.AbsoluteExpirationRelativeToNow.Value.TotalMilliseconds * _distributedCacheOptions.L1ExpirationTimeRatio);
_memoryCacheExpirationTime = TimeSpan.FromMilliseconds(_distributedCacheOptions.AbsoluteExpirationRelativeToNow.Value.TotalMilliseconds * _distributedCacheOptions.L1ExpirationTimeRatio);
}
}

Expand Down Expand Up @@ -184,7 +184,7 @@ await L2OperationWithRetryOnFailureAsync(
{
MemoryCacheEntryOptions memoryCacheEntryOptions = new MemoryCacheEntryOptions
{
AbsoluteExpirationRelativeToNow = _expirationTime,
AbsoluteExpirationRelativeToNow = _memoryCacheExpirationTime,
Size = result?.Length,
};

Expand Down Expand Up @@ -245,7 +245,7 @@ protected override async Task WriteCacheBytesAsync(
MemoryCacheEntryOptions memoryCacheEntryOptions = new MemoryCacheEntryOptions
{
AbsoluteExpiration = cacheExpiry ?? _distributedCacheOptions.AbsoluteExpiration,
AbsoluteExpirationRelativeToNow = _expirationTime,
AbsoluteExpirationRelativeToNow = _memoryCacheExpirationTime,
Size = bytes?.Length,
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,15 @@ namespace Microsoft.Identity.Web.TokenCacheProviders.Distributed
{
/// <summary>
/// Options for the MSAL token cache serialization adapter,
/// which delegates the serialization to the IDistributedCache implementations
/// which delegates the serialization to the <see cref="IDistributedCache"/> implementations
/// available with .NET Core.
/// </summary>
public class MsalDistributedTokenCacheAdapterOptions : DistributedCacheEntryOptions
{
internal const int FiveHundredMb = 500 * 1024 * 1024;

/// <summary>
/// Options of the In Memory (L1) cache.
/// Options of the in-memory (L1) cache.
/// </summary>
public MemoryCacheOptions L1CacheOptions { get; set; } = new MemoryCacheOptions
{
Expand All @@ -27,17 +27,19 @@ public class MsalDistributedTokenCacheAdapterOptions : DistributedCacheEntryOpti
/// <summary>
/// Callback offered to the app to be notified when the L2 cache fails.
/// This way the app is given the possibility to act on the L2 cache,
/// for instance, in the case of Redis, to reconnect. This is left to the application as it's
/// for instance, in the case of Redis exception, to reconnect. This is left to the application as it's
/// the only one that knows about the real implementation of the L2 cache.
/// The handler should return <c>true</c> if the cache should try again the operation, and
/// The handler should return <c>true</c> if the cache should try the operation again, and
/// <c>false</c> otherwise. When <c>true</c> is passed and the retry fails, an exception
/// will be thrown.
/// </summary>
public Func<Exception, bool>? OnL2CacheFailure { get; set; }

/// <summary>
/// Value more than 0, less than 1, to set the In Memory (L1) cache
/// expiration time values relative to the Distributed (L2) cache.
/// Value must be more than 0 and less than or equal to 1.
/// Sets the ratio of the in-memory (L1) cache expiration time
/// relative to the distributed (L2) cache (e.g. when set to .5,
/// the L1 cache entry expiry is half the time of the L2 cache expiry).
/// Default is 1.
/// </summary>
internal double L1ExpirationTimeRatio { get; set; } = 1;
Expand All @@ -49,15 +51,15 @@ public class MsalDistributedTokenCacheAdapterOptions : DistributedCacheEntryOpti
public bool Encrypt { get; set; }

/// <summary>
/// Disable the L1 (InMemory) cache.
/// Disable the in-memory (L1) cache.
/// Useful in scenarios where multiple apps share the same
/// L2 cache.
/// distributed (L2) cache.
/// </summary>
/// The default is <c>false.</c>
public bool DisableL1Cache { get; set; }

/// <summary>
/// Enable writing to the L2 cache to be async (fire and forget).
/// Enable writing to the distributed (L2) cache to be async (i.e. fire-and-forget).
/// This improves performance as the MSAL.NET will not have to wait
/// for the write to complete.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,13 @@ namespace Microsoft.Identity.Web.TokenCacheProviders.InMemory
/// </summary>
public class MsalMemoryTokenCacheOptions
{
internal static TimeSpan DefaultAbsoluteExpirationRelativeToNow = TimeSpan.FromDays(14);

/// <summary>Initializes a new instance of the <see cref="MsalMemoryTokenCacheOptions"/> class.
/// By default, the sliding expiration is set for 14 days.</summary>
public MsalMemoryTokenCacheOptions()
{
AbsoluteExpirationRelativeToNow = TimeSpan.FromDays(14);
AbsoluteExpirationRelativeToNow = DefaultAbsoluteExpirationRelativeToNow;
}

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,22 @@ protected override Task WriteCacheBytesAsync(
byte[] bytes,
CacheSerializerHints cacheSerializerHints)
{
MemoryCacheEntryOptions memoryCacheEntryOptions = new MemoryCacheEntryOptions
{
AbsoluteExpirationRelativeToNow = DetermineCacheEntryExpiry(cacheSerializerHints),
Size = bytes?.Length,
};

_memoryCache.Set(cacheKey, bytes, memoryCacheEntryOptions);
return Task.CompletedTask;
}

/// <summary>
/// _cacheOptions.AbsoluteExpirationRelativeToNow represents either a user-provided expiration or the default, if not set.
/// Between the suggested expiry and expiry from options, the shorter one takes precedence.
/// </summary>
internal TimeSpan DetermineCacheEntryExpiry(CacheSerializerHints cacheSerializerHints)
{
TimeSpan? cacheExpiry = null;
if (cacheSerializerHints != null && cacheSerializerHints.SuggestedCacheExpiry != null)
{
Expand All @@ -113,14 +129,9 @@ protected override Task WriteCacheBytesAsync(
}
}

MemoryCacheEntryOptions memoryCacheEntryOptions = new MemoryCacheEntryOptions
{
AbsoluteExpirationRelativeToNow = cacheExpiry ?? _cacheOptions.AbsoluteExpirationRelativeToNow,
Size = bytes?.Length,
};

_memoryCache.Set(cacheKey, bytes, memoryCacheEntryOptions);
return Task.CompletedTask;
return cacheExpiry is null || _cacheOptions.AbsoluteExpirationRelativeToNow < cacheExpiry
? _cacheOptions.AbsoluteExpirationRelativeToNow
: cacheExpiry.Value;
}
}
}
22 changes: 22 additions & 0 deletions tests/Microsoft.Identity.Web.Test.Common/AssertExtensions.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

using System;
using Xunit;

namespace Microsoft.Identity.Web.Test.Common
{
public class Asserts
{
/// <summary>
/// Verifies that a value is within a given range.
/// </summary>
/// <param name="actual">The actual value to be evaluated</param>
/// <param name="expected">The expected middle of the range.</param>
/// <param name="variance">The variance below and above the expected value.</param>
public static void WithinVariance(TimeSpan actual, TimeSpan expected, TimeSpan variance)
{
Assert.InRange(actual, expected.Add(-variance), expected.Add(variance));
}
}
}
8 changes: 8 additions & 0 deletions tests/Microsoft.Identity.Web.Test/L1L2CacheOptionsTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using Microsoft.Extensions.Options;
using Microsoft.Identity.Web.Test.Common.TestHelpers;
using Microsoft.Identity.Web.TokenCacheProviders.Distributed;
using Microsoft.Identity.Web.TokenCacheProviders.InMemory;
using Xunit;

namespace Microsoft.Identity.Web.Test
Expand Down Expand Up @@ -101,6 +102,13 @@ public void MsalDistributedTokenCacheAdapterOptions_DisableL1Cache(bool disableL
}
}

[Fact]
public void MsalMemoryTokenCacheOptions_SetsDefault_Test()
{
var options = new MsalMemoryTokenCacheOptions();
Assert.Equal(MsalMemoryTokenCacheOptions.DefaultAbsoluteExpirationRelativeToNow, options.AbsoluteExpirationRelativeToNow);
}

private void BuildTheRequiredServices()
{
var services = new ServiceCollection();
Expand Down
82 changes: 56 additions & 26 deletions tests/Microsoft.Identity.Web.Test/MsalTokenCacheProviderTests.cs
Original file line number Diff line number Diff line change
@@ -1,46 +1,76 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

using System;
using System.Collections.Generic;
using Microsoft.Extensions.Caching.Memory;
using Microsoft.Extensions.Options;
using Microsoft.Identity.Web.Test.Common;
using Microsoft.Identity.Web.Test.Common.TestHelpers;
using Microsoft.Identity.Web.TokenCacheProviders;
using Microsoft.Identity.Web.TokenCacheProviders.InMemory;
using Xunit;


namespace Microsoft.Identity.Web.Test
{
public class MsalTokenCacheProviderTests
{
/// <summary>
/// Implementation based on a Memory cache, But could be Redis, SQL, ...
/// </summary>
/// <returns>In memory cache.</returns>
private static IMemoryCache GetMemoryCache()
{
if (s_memoryCache == null)
{
IOptions<MemoryCacheOptions> options = Options.Create(new MemoryCacheOptions());
s_memoryCache = new MemoryCache(options);
}

return s_memoryCache;
}

private static IMemoryCache? s_memoryCache;

private static IMsalTokenCacheProvider CreateTokenCacheSerializer()
{
IOptions<MsalMemoryTokenCacheOptions> msalCacheOptions = Options.Create(new MsalMemoryTokenCacheOptions());

MsalMemoryTokenCacheProvider memoryTokenCacheProvider = new MsalMemoryTokenCacheProvider(GetMemoryCache(), msalCacheOptions);
return memoryTokenCacheProvider;
}

[Fact]
public void CreateTokenCacheSerializerTest()
{
IMsalTokenCacheProvider tokenCacheProvider = CreateTokenCacheSerializer();
Assert.NotNull(tokenCacheProvider);
}

[Theory, MemberData(nameof(CacheExpiryData))]
public void CacheEntryExpiry_SetCorrectly_Test(TimeSpan? optionsCacheExpiry, DateTimeOffset? suggestedCacheExpiry, TimeSpan expectedCacheExpiry)
{
// Arrange
var msalCacheOptions = new MsalMemoryTokenCacheOptions();
if (optionsCacheExpiry.HasValue)
{
msalCacheOptions.AbsoluteExpirationRelativeToNow = optionsCacheExpiry.Value;
}

var msalCacheProvider = new MsalMemoryTokenCacheProvider(GetMemoryCache(), Options.Create(msalCacheOptions));
var cacheHints = new CacheSerializerHints()
{
SuggestedCacheExpiry = suggestedCacheExpiry,
};

// Act
TimeSpan? calculatedCacheExpiry = msalCacheProvider.DetermineCacheEntryExpiry(cacheHints);

// Assert
Assert.NotNull(calculatedCacheExpiry);
// Using InRange because internal logic uses current date/time, which is variable and not constant.
Asserts.WithinVariance(calculatedCacheExpiry.Value, expectedCacheExpiry, TimeSpan.FromMinutes(2));
}

// TimeSpan? optionsCacheExpiry, DateTimeOffset? suggestedCacheExpiry, TimeSpan expectedCacheExpiry
public static IEnumerable<object?[]> CacheExpiryData =>
new List<object?[]>
{
new object?[] { null, null, MsalMemoryTokenCacheOptions.DefaultAbsoluteExpirationRelativeToNow }, // no user-provided expiry, no suggested expiry (e.g. user flows)
new object?[] { null, DateTimeOffset.Now.AddHours(5), TimeSpan.FromHours(5) }, // no user-provided expiry, suggested expiry (e.g. app flows)
new object?[] { TimeSpan.FromHours(1), null, TimeSpan.FromHours(1) }, // user-provided expiry, no suggested expiry (e.g. user flows)
new object?[] { TimeSpan.FromHours(1), DateTimeOffset.Now.AddHours(5), TimeSpan.FromHours(1) }, // user-provided expiry shorter than suggested
new object?[] { TimeSpan.FromHours(10), DateTimeOffset.Now.AddHours(5), TimeSpan.FromHours(5) }, // user-provided expiry longer than suggested
new object?[] { null, DateTimeOffset.Now.AddHours(-5), TimeSpan.Zero }, // Negative suggested expiry
};

private IMemoryCache GetMemoryCache()
{
IOptions<MemoryCacheOptions> options = Options.Create(new MemoryCacheOptions());
return new MemoryCache(options);
}

private IMsalTokenCacheProvider CreateTokenCacheSerializer()
{
IOptions<MsalMemoryTokenCacheOptions> msalCacheOptions = Options.Create(new MsalMemoryTokenCacheOptions());

MsalMemoryTokenCacheProvider memoryTokenCacheProvider = new MsalMemoryTokenCacheProvider(GetMemoryCache(), msalCacheOptions);
return memoryTokenCacheProvider;
}
}
}

0 comments on commit 51de97e

Please sign in to comment.