Skip to content

Fix reliability of a couple of caching tests #359

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Jun 29, 2023
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,6 @@ public static void UseSystemWebAdapters(this IApplicationBuilder app)
{
ArgumentNullException.ThrowIfNull(app);

HttpRuntime.Current = app.ApplicationServices.GetRequiredService<IHttpRuntime>();

app.UseSystemWebAdapterFeatures();
app.UseAuthenticationEvents();
app.UseAuthorizationEvents();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,12 @@ public CacheDependency(string[] filenames, DateTime start) : this(filenames, nul
public CacheDependency(string[]? filenames, string[]? cachekeys) : this(filenames, cachekeys, null, DateTime.MaxValue) { }

public CacheDependency(string[]? filenames, string[]? cachekeys, DateTime start) :
this(filenames, cachekeys, null, start) { }
this(filenames, cachekeys, null, start)
{ }

public CacheDependency(string[]? filenames, string[]? cachekeys, CacheDependency? dependency) :
this(filenames, cachekeys, dependency, DateTime.MaxValue) { }
this(filenames, cachekeys, dependency, DateTime.MaxValue)
{ }

public CacheDependency(
string[]? filenames,
Expand Down Expand Up @@ -70,7 +72,7 @@ public CacheDependency(
protected internal void FinishInit()
{
HasChanged = changeMonitors.Any(cm => cm.HasChanged && (cm.GetLastModifiedUtc() > utcStart));
utcLastModified = changeMonitors.Count==0 ? DateTime.MinValue : changeMonitors.Max(cm => cm.GetLastModifiedUtc());
utcLastModified = changeMonitors.Count == 0 ? DateTime.MinValue : changeMonitors.Max(cm => cm.GetLastModifiedUtc());
if (HasChanged)
{
NotifyDependencyChanged(this, EventArgs.Empty);
Expand Down Expand Up @@ -135,14 +137,27 @@ protected virtual void Dispose(bool disposing)
{
if (disposing)
{
foreach (var changeMonitor in changeMonitors)
if (!disposedValue)
{
changeMonitor?.Dispose();
// Ensure that if the Dispose is called by different threads (i.e. the wrapping ChangeMonitor) that it won't enumerate the list at the same time
lock (changeMonitors)
{
if (!disposedValue)
{
foreach (var changeMonitor in changeMonitors)
{
changeMonitor?.Dispose();
}
changeMonitors.Clear();

DependencyDispose();

disposedValue = true;
}
}
}
changeMonitors.Clear();

DependencyDispose();
}

disposedValue = true;
}
}
Expand Down
9 changes: 2 additions & 7 deletions src/Microsoft.AspNetCore.SystemWebAdapters/HttpRuntime.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,22 +2,17 @@
// The .NET Foundation licenses this file to you under the MIT license.

using Microsoft.AspNetCore.SystemWebAdapters;
using Microsoft.Extensions.DependencyInjection;

namespace System.Web;

public sealed class HttpRuntime
{
private static IHttpRuntime? _current;

/// <summary>
/// Gets the current <see cref="IHttpRuntime"/>. This should not be used internally besides where is strictly necessary.
/// If this is needed, it should be retrieved through dependency injection.
/// </summary>
internal static IHttpRuntime Current
{
get => _current ?? throw new InvalidOperationException("HttpRuntime is not available in the current environment");
set => _current = value;
}
internal static IHttpRuntime Current => HttpContext.Current.UnwrapAdapter()?.RequestServices.GetRequiredService<IHttpRuntime>() ?? throw new InvalidOperationException("No runtime is currently available");

private HttpRuntime()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,11 @@

using System.Collections;
using System.Collections.Generic;
using System.IO;
using System.Runtime.Caching;
using System.Threading.Tasks;
using AutoFixture;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.SystemWebAdapters;
using Moq;
using Xunit;
Expand Down Expand Up @@ -340,6 +342,8 @@ public async Task DependentFileCallback()
var slidingExpiration = TimeSpan.FromMilliseconds(1);
CacheItemUpdateReason? updateReason = default;

var tcs = new TaskCompletionSource();

void Callback(string key, CacheItemUpdateReason reason, out object? expensiveObject, out CacheDependency? dependency, out DateTime absoluteExpiration, out TimeSpan slidingExpiration)
{
expensiveObject = null;
Expand All @@ -349,23 +353,25 @@ void Callback(string key, CacheItemUpdateReason reason, out object? expensiveObj

updated = true;
updateReason = reason;

tcs.SetResult();
}

var file = System.IO.Path.GetTempFileName();
await System.IO.File.WriteAllTextAsync(file, key);
var file = Path.GetTempFileName();
await File.WriteAllTextAsync(file, key);

using var cd = new CacheDependency(file);
// Act
cache.Insert(key, item, cd, Cache.NoAbsoluteExpiration, Cache.NoSlidingExpiration, Callback);

// Ensure file is updated
await System.IO.File.WriteAllTextAsync(file, DateTime.UtcNow.ToString("O"));
await File.WriteAllTextAsync(file, DateTime.UtcNow.ToString("O"));

// Small delay here to ensure that the file change notification happens (may fail tests if too fast)
await Task.Delay(10);
// Wait for callback to be called
await tcs.Task;

// Force cleanup to initiate callbacks on current thread
memCache.Trim(100);
// Wait for callback to be finalized
await Task.Delay(100);

// Assert
Assert.True(updated);
Expand All @@ -382,7 +388,8 @@ public async Task DependentItemCallback()
var cache = new Cache(memCache);
var httpRuntime = new Mock<IHttpRuntime>();
httpRuntime.Setup(s => s.Cache).Returns(cache);
HttpRuntime.Current = httpRuntime.Object;

SetHttpRuntime(httpRuntime.Object);

var item1 = new object();
var item2 = new object();
Expand Down Expand Up @@ -425,4 +432,28 @@ void Callback(string key, CacheItemUpdateReason reason, out object? expensiveObj
Assert.Equal(CacheItemUpdateReason.Expired, updateReason[key1]);
Assert.Equal(CacheItemUpdateReason.DependencyChanged, updateReason[key2]);
}

private static void SetHttpRuntime(IHttpRuntime runtime)
{
_ = new HttpContextAccessor
{
HttpContext = new DefaultHttpContext
{
RequestServices = new RuntimeServiceProvider(runtime)
}
};
}

private sealed class RuntimeServiceProvider : IServiceProvider
{
private readonly IHttpRuntime _runtime;

public RuntimeServiceProvider(IHttpRuntime runtime)
{
_runtime = runtime;
}

public object? GetService(Type serviceType)
=> serviceType == typeof(IHttpRuntime) ? _runtime : null;
}
}