Skip to content

Commit

Permalink
Don't end resource notifications until service is disposed (#7108)
Browse files Browse the repository at this point in the history
Contributes to #4878
  • Loading branch information
DamianEdwards authored Jan 14, 2025
1 parent b02fa42 commit c000b49
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 10 deletions.
19 changes: 13 additions & 6 deletions src/Aspire.Hosting/ApplicationModel/ResourceNotificationService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,13 @@ namespace Aspire.Hosting.ApplicationModel;
/// <summary>
/// A service that allows publishing and subscribing to changes in the state of a resource.
/// </summary>
public class ResourceNotificationService
public class ResourceNotificationService : IDisposable
{
// Resource state is keyed by the resource and the unique name of the resource. This could be the name of the resource, or a replica ID.
private readonly ConcurrentDictionary<(IResource, string), ResourceNotificationState> _resourceNotificationStates = new();
private readonly ILogger<ResourceNotificationService> _logger;
private readonly IServiceProvider _serviceProvider;
private readonly CancellationToken _applicationStopping;
private readonly CancellationTokenSource _disposing = new();
private readonly ResourceLoggerService _resourceLoggerService;

private Action<ResourceEvent>? OnResourceUpdated { get; set; }
Expand All @@ -43,7 +43,6 @@ public ResourceNotificationService(ILogger<ResourceNotificationService> logger,
{
_logger = logger ?? throw new ArgumentNullException(nameof(logger));
_serviceProvider = new NullServiceProvider();
_applicationStopping = hostApplicationLifetime?.ApplicationStopping ?? throw new ArgumentNullException(nameof(hostApplicationLifetime));
_resourceLoggerService = new ResourceLoggerService();
}

Expand All @@ -58,8 +57,10 @@ public ResourceNotificationService(ILogger<ResourceNotificationService> logger,
{
_logger = logger ?? throw new ArgumentNullException(nameof(logger));
_serviceProvider = serviceProvider;
_applicationStopping = hostApplicationLifetime?.ApplicationStopping ?? throw new ArgumentNullException(nameof(hostApplicationLifetime));
_resourceLoggerService = resourceLoggerService ?? throw new ArgumentNullException(nameof(resourceLoggerService));

// The IHostApplicationLifetime parameter is not used anymore, but we keep it for backwards compatibility.
// Notfication updates will be cancelled when the service is disposed.
}

private class NullServiceProvider : IServiceProvider
Expand Down Expand Up @@ -105,7 +106,7 @@ public Task WaitForResourceAsync(string resourceName, string? targetState = null
Justification = "targetState(s) parameters are mutually exclusive.")]
public async Task<string> WaitForResourceAsync(string resourceName, IEnumerable<string> targetStates, CancellationToken cancellationToken = default)
{
using var watchCts = CancellationTokenSource.CreateLinkedTokenSource(_applicationStopping, cancellationToken);
using var watchCts = CancellationTokenSource.CreateLinkedTokenSource(_disposing.Token, cancellationToken);
var watchToken = watchCts.Token;
await foreach (var resourceEvent in WatchAsync(watchToken).ConfigureAwait(false))
{
Expand Down Expand Up @@ -273,7 +274,7 @@ public async Task WaitForDependenciesAsync(IResource resource, CancellationToken
Justification = "predicate and targetState(s) parameters are mutually exclusive.")]
public async Task<ResourceEvent> WaitForResourceAsync(string resourceName, Func<ResourceEvent, bool> predicate, CancellationToken cancellationToken = default)
{
using var watchCts = CancellationTokenSource.CreateLinkedTokenSource(_applicationStopping, cancellationToken);
using var watchCts = CancellationTokenSource.CreateLinkedTokenSource(_disposing.Token, cancellationToken);
var watchToken = watchCts.Token;
await foreach (var resourceEvent in WatchAsync(watchToken).ConfigureAwait(false))
{
Expand Down Expand Up @@ -502,6 +503,12 @@ private static CustomResourceSnapshot GetCurrentSnapshot(IResource resource, Res
private ResourceNotificationState GetResourceNotificationState(IResource resource, string resourceId) =>
_resourceNotificationStates.GetOrAdd((resource, resourceId), _ => new ResourceNotificationState());

/// <inheritdoc/>
public void Dispose()
{
_disposing.Cancel();
}

/// <summary>
/// The annotation that allows publishing and subscribing to changes in the state of a resource.
/// </summary>
Expand Down
1 change: 1 addition & 0 deletions src/Aspire.Hosting/PublicAPI.Unshipped.txt
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@ Aspire.Hosting.ApplicationModel.ResourceCommandState
Aspire.Hosting.ApplicationModel.ResourceCommandState.Disabled = 1 -> Aspire.Hosting.ApplicationModel.ResourceCommandState
Aspire.Hosting.ApplicationModel.ResourceCommandState.Enabled = 0 -> Aspire.Hosting.ApplicationModel.ResourceCommandState
Aspire.Hosting.ApplicationModel.ResourceCommandState.Hidden = 2 -> Aspire.Hosting.ApplicationModel.ResourceCommandState
Aspire.Hosting.ApplicationModel.ResourceNotificationService.Dispose() -> void
Aspire.Hosting.ApplicationModel.ResourceNotificationService.ResourceNotificationService(Microsoft.Extensions.Logging.ILogger<Aspire.Hosting.ApplicationModel.ResourceNotificationService!>! logger, Microsoft.Extensions.Hosting.IHostApplicationLifetime! hostApplicationLifetime) -> void
Aspire.Hosting.ApplicationModel.ResourceNotificationService.ResourceNotificationService(Microsoft.Extensions.Logging.ILogger<Aspire.Hosting.ApplicationModel.ResourceNotificationService!>! logger, Microsoft.Extensions.Hosting.IHostApplicationLifetime! hostApplicationLifetime, System.IServiceProvider! serviceProvider, Aspire.Hosting.ApplicationModel.ResourceLoggerService! resourceLoggerService) -> void
Aspire.Hosting.ApplicationModel.ResourceNotificationService.WaitForDependenciesAsync(Aspire.Hosting.ApplicationModel.IResource! resource, System.Threading.CancellationToken cancellationToken) -> System.Threading.Tasks.Task!
Expand Down
26 changes: 22 additions & 4 deletions tests/Aspire.Hosting.Tests/ResourceNotificationTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,24 @@ public async Task WaitingOnResourceReturnsCorrectStateWhenResourceReachesOneOfTa
Assert.Equal("SomeOtherState", reachedState);
}

[Fact]
public async Task WaitingOnResourceReturnsItReachesStateAfterApplicationStoppingCancellationTokenSignaled()
{
var resource1 = new CustomResource("myResource1");

using var hostApplicationLifetime = new TestHostApplicationLifetime();
var notificationService = ResourceNotificationServiceTestHelpers.Create(hostApplicationLifetime: hostApplicationLifetime);

var waitTask = notificationService.WaitForResourceAsync("myResource1", "SomeState");
hostApplicationLifetime.StopApplication();

await notificationService.PublishUpdateAsync(resource1, snapshot => snapshot with { State = "SomeState" }).DefaultTimeout();

await waitTask.DefaultTimeout();

Assert.True(waitTask.IsCompletedSuccessfully);
}

[Fact]
public async Task WaitingOnResourceThrowsOperationCanceledExceptionIfResourceDoesntReachStateBeforeCancellationTokenSignaled()
{
Expand All @@ -261,13 +279,13 @@ await Assert.ThrowsAsync<OperationCanceledException>(async () =>
}

[Fact]
public async Task WaitingOnResourceThrowsOperationCanceledExceptionIfResourceDoesntReachStateBeforeApplicationStoppingCancellationTokenSignaled()
public async Task WaitingOnResourceThrowsOperationCanceledExceptionIfResourceDoesntReachStateBeforeServiceIsDisposed()
{
using var hostApplicationLifetime = new TestHostApplicationLifetime();
var notificationService = ResourceNotificationServiceTestHelpers.Create(hostApplicationLifetime: hostApplicationLifetime);
var notificationService = ResourceNotificationServiceTestHelpers.Create();

var waitTask = notificationService.WaitForResourceAsync("myResource1", "SomeState");
hostApplicationLifetime.StopApplication();

notificationService.Dispose();

await Assert.ThrowsAsync<OperationCanceledException>(async () =>
{
Expand Down

0 comments on commit c000b49

Please sign in to comment.