Skip to content

Remove IResourceWithoutLifetime on everything #10851

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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
2 changes: 1 addition & 1 deletion src/Aspire.Hosting/ApplicationModel/ParameterResource.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ namespace Aspire.Hosting.ApplicationModel;
/// <summary>
/// Represents a parameter resource.
/// </summary>
public class ParameterResource : Resource, IResourceWithoutLifetime, IManifestExpressionProvider, IValueProvider
public class ParameterResource : Resource, IManifestExpressionProvider, IValueProvider
{
private readonly Lazy<string> _lazyValue;
private readonly Func<ParameterDefault?, string> _valueGetter;
Expand Down
39 changes: 39 additions & 0 deletions src/Aspire.Hosting/ConnectionStringBuilderExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// The .NET Foundation licenses this file to you under the MIT license.
using Aspire.Dashboard.Model;
using Aspire.Hosting.ApplicationModel;
using Microsoft.Extensions.Logging;

namespace Aspire.Hosting;

Expand Down Expand Up @@ -46,6 +47,44 @@ public static IResourceBuilder<ConnectionStringResource> AddConnectionString(thi
ResourceType = KnownResourceTypes.ConnectionString,
State = KnownResourceStates.Waiting,
Properties = []
})
.OnInitializeResource(async (r, @evt, ct) =>
{
// Wait for any referenced resources in the connection string.
// We only look at top level resources with the assumption that they are transitive themselves.
var tasks = new List<Task>();
foreach (var referencedResource in r.ConnectionStringExpression.ValueProviders.OfType<IResource>())
{
tasks.Add(evt.Notifications.WaitForResourceHealthyAsync(referencedResource.Name, ct));
Copy link
Preview

Copilot AI Aug 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code waits for resources to be healthy, but connection string resolution might only need the referenced resources to have their connection strings available, not necessarily be healthy. This could cause unnecessary delays or failures if a referenced resource is running but not yet healthy.

Suggested change
tasks.Add(evt.Notifications.WaitForResourceHealthyAsync(referencedResource.Name, ct));
tasks.Add(evt.Notifications.WaitForResourceRunningAsync(referencedResource.Name, ct));

Copilot uses AI. Check for mistakes.

Copy link
Member Author

@davidfowl davidfowl Aug 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is a reasonable suggestion tbh. The resource does not need to be healthy to get the value of the connection string. That said, this affects consumers of the ConnectionStringAvailableEvent, not users of ConnectionStringExpression

}

try
{
// Connection string resolution is dependent on parameters being resolved
// We use this to wait for the parameters to be resolved before we can compute the connection string.
await Task.WhenAll(tasks).ConfigureAwait(false);

// Publish the update with the connection string value and the state as running.
// This will allow health checks to start running.
await evt.Notifications.PublishUpdateAsync(r, s => s with
{
State = KnownResourceStates.Running
}).ConfigureAwait(false);

// Publish the connection string available event for other resources that may depend on this resource.
await evt.Eventing.PublishAsync(new ConnectionStringAvailableEvent(r, evt.Services), ct)
.ConfigureAwait(false);
}
catch (Exception ex)
{
evt.Logger.LogError(ex, "Failed to resolve connection string for resource '{ResourceName}'", r.Name);

// If we fail to resolve the connection string, we set the state to failed.
await evt.Notifications.PublishUpdateAsync(r, s => s with
{
State = KnownResourceStates.FailedToStart
}).ConfigureAwait(false);
}
});
}

Expand Down
2 changes: 1 addition & 1 deletion src/Aspire.Hosting/ConnectionStringResource.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ namespace Aspire.Hosting;
/// </summary>
/// <param name="name">The name of the resource.</param>
/// <param name="connectionStringExpression">The connection string expression.</param>
public sealed class ConnectionStringResource(string name, ReferenceExpression connectionStringExpression) : Resource(name), IResourceWithConnectionString, IResourceWithoutLifetime
public sealed class ConnectionStringResource(string name, ReferenceExpression connectionStringExpression) : Resource(name), IResourceWithConnectionString
{
/// <summary>
/// Describes the connection string format string used for this resource.
Expand Down
2 changes: 1 addition & 1 deletion src/Aspire.Hosting/ExternalServiceResource.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ namespace Aspire.Hosting;
/// <summary>
/// Represents an external service resource with service discovery capabilities.
/// </summary>
public sealed class ExternalServiceResource : Resource, IResourceWithoutLifetime
public sealed class ExternalServiceResource : Resource
{
private readonly Uri? _uri;
private readonly ParameterResource? _urlParameter;
Expand Down
65 changes: 0 additions & 65 deletions src/Aspire.Hosting/Orchestrator/ApplicationOrchestrator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
using Aspire.Hosting.Dcp;
using Aspire.Hosting.Eventing;
using Aspire.Hosting.Lifecycle;
using Microsoft.Extensions.Logging;

namespace Aspire.Hosting.Orchestrator;

Expand Down Expand Up @@ -59,7 +58,6 @@ public ApplicationOrchestrator(DistributedApplicationModel model,
_eventing.Subscribe<ConnectionStringAvailableEvent>(PublishConnectionStringValue);
// Implement WaitFor functionality using BeforeResourceStartedEvent.
_eventing.Subscribe<BeforeResourceStartedEvent>(WaitForInBeforeResourceStartedEvent);
_eventing.Subscribe<InitializeResourceEvent>(OnResourceInitialized);
}

private async Task PublishConnectionStringValue(ConnectionStringAvailableEvent @event, CancellationToken token)
Expand Down Expand Up @@ -306,69 +304,6 @@ private async Task OnResourceEndpointsAllocated(ResourceEndpointsAllocatedEvent
await PublishResourceEndpointUrls(@event.Resource, cancellationToken).ConfigureAwait(false);
}

private Task OnResourceInitialized(InitializeResourceEvent @event, CancellationToken cancellationToken)
{
var resource = @event.Resource;

if (resource is ConnectionStringResource connectionStringResource)
{
InitializeConnectionString(connectionStringResource);
}

void InitializeConnectionString(ConnectionStringResource connectionStringResource)
{
var logger = _loggerService.GetLogger(resource);
var waitFor = new List<Task>();

var references = connectionStringResource.Annotations.OfType<ResourceRelationshipAnnotation>()
.Where(x => x.Type == KnownRelationshipTypes.Reference)
.Select(x => x.Resource);

foreach (var reference in references)
{
if (reference is IResourceWithEndpoints)
{
var tcs = new TaskCompletionSource();
logger.LogInformation("Waiting for endpoints to be allocated for resource {ResourceName}", reference.Name);
_eventing.Subscribe<ResourceEndpointsAllocatedEvent>(reference, (_, _) =>
{
logger.LogInformation("Endpoints allocated for resource {ResourceName}", reference.Name);
tcs.SetResult();
return Task.CompletedTask;
});

waitFor.Add(tcs.Task.WaitAsync(cancellationToken));
}

if (reference is IResourceWithConnectionString)
{
var tcs = new TaskCompletionSource();
logger.LogInformation("Waiting for connection string to be available for resource {ResourceName}", reference.Name);
_eventing.Subscribe<ConnectionStringAvailableEvent>(reference, (_, _) =>
{
logger.LogInformation("Connection string is available for resource {ResourceName}", reference.Name);
tcs.SetResult();
return Task.CompletedTask;
});

waitFor.Add(tcs.Task.WaitAsync(cancellationToken));
}
}

_ = Task.Run(async () =>
{
await Task.WhenAll(waitFor).ConfigureAwait(false);
await PublishConnectionStringAvailableEvent(connectionStringResource, cancellationToken).ConfigureAwait(false);
await _notificationService.PublishUpdateAsync(connectionStringResource, s => s with
{
State = new(KnownResourceStates.Active, KnownResourceStateStyles.Success),
}).ConfigureAwait(false);
}, cancellationToken);
}

return Task.CompletedTask;
}

private async Task OnResourceChanged(OnResourceChangedContext context)
{
await _notificationService.PublishUpdateAsync(context.Resource, context.DcpResourceName, context.UpdateSnapshot).ConfigureAwait(false);
Expand Down
4 changes: 2 additions & 2 deletions src/Aspire.Hosting/Orchestrator/ParameterProcessor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ await notificationService.PublishUpdateAsync(parameterResource, s =>
return s with
{
Properties = s.Properties.SetResourceProperty(KnownProperties.Parameter.Value, value, parameterResource.Secret),
State = new(KnownResourceStates.Active, KnownResourceStateStyles.Success)
State = KnownResourceStates.Running
};
})
.ConfigureAwait(false);
Expand Down Expand Up @@ -189,7 +189,7 @@ await notificationService.PublishUpdateAsync(parameter, s =>
return s with
{
Properties = s.Properties.SetResourceProperty(KnownProperties.Parameter.Value, inputValue, parameter.Secret),
State = new(KnownResourceStates.Active, KnownResourceStateStyles.Success)
State = KnownResourceStates.Running
};
})
.ConfigureAwait(false);
Expand Down
1 change: 0 additions & 1 deletion src/Aspire.Hosting/ParameterResourceBuilderExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,6 @@ internal static IResourceBuilder<T> AddParameter<T>(this IDistributedApplication
{
ResourceType = KnownResourceTypes.Parameter,
Properties = [
new("parameter.secret", resource.Secret.ToString()),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is removed but I'm not seeing where it is injected elsewhere, are we just relying on parameter value now?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't need to remove it but it does nothing today 😄

new(CustomResourceKnownProperties.Source, resource.ConfigurationKey)
],
State = KnownResourceStates.Waiting
Expand Down
10 changes: 0 additions & 10 deletions src/Aspire.Hosting/ResourceBuilderExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1202,16 +1202,6 @@ private static IResourceBuilder<T> WaitForCore<T>(this IResourceBuilder<T> build
builder.WaitForCore(parentBuilder, waitBehavior, addRelationship: false);
}

// Wait for any referenced resources in the connection string.
if (dependency.Resource is ConnectionStringResource cs)
{
// We only look at top level resources with the assumption that they are transitive themselves.
foreach (var referencedResource in cs.ConnectionStringExpression.ValueProviders.OfType<IResource>())
{
builder.WaitForCore(builder.ApplicationBuilder.CreateResourceBuilder(referencedResource), waitBehavior, addRelationship: false);
}
}

if (addRelationship)
{
builder.WithRelationship(dependency.Resource, KnownRelationshipTypes.WaitFor);
Expand Down
5 changes: 0 additions & 5 deletions tests/Aspire.Hosting.Tests/AddParameterTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,6 @@ public void ParametersAreVisibleByDefault()

Assert.False(state.IsHidden);
Assert.Collection(state.Properties,
prop =>
{
Assert.Equal("parameter.secret", prop.Name);
Assert.Equal("True", prop.Value);
},
prop =>
{
Assert.Equal(CustomResourceKnownProperties.Source, prop.Name);
Expand Down
11 changes: 0 additions & 11 deletions tests/Aspire.Hosting.Tests/ExternalServiceTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -246,17 +246,6 @@ public void ExternalServiceResourceHasExpectedInitialState()
Assert.Equal("ExternalService", snapshot.InitialSnapshot.ResourceType);
}

[Fact]
public void ExternalServiceResourceImplementsExpectedInterfaces()
{
using var builder = TestDistributedApplicationBuilder.Create();

var externalService = builder.AddExternalService("nuget", "https://nuget.org/");

// Verify the resource implements the expected interfaces
Assert.IsAssignableFrom<IResourceWithoutLifetime>(externalService.Resource);
}

[Fact]
public void ExternalServiceResourceIsExcludedFromPublishingManifest()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ namespace Aspire.Hosting.Tests.Orchestrator;
public class ParameterProcessorTests
{
[Fact]
public async Task InitializeParametersAsync_WithValidParameters_SetsActiveState()
public async Task InitializeParametersAsync_WithValidParameters_SetsRunningState()
{
// Arrange
var parameterProcessor = CreateParameterProcessor();
Expand All @@ -41,7 +41,7 @@ public async Task InitializeParametersAsync_WithValidParameters_SetsActiveState(
}

[Fact]
public async Task InitializeParametersAsync_WithValidParametersAndDashboardEnabled_SetsActiveState()
public async Task InitializeParametersAsync_WithValidParametersAndDashboardEnabled_SetsRunningState()
{
// Arrange
var interactionService = CreateInteractionService(disableDashboard: false);
Expand Down Expand Up @@ -93,8 +93,7 @@ public async Task InitializeParametersAsync_WithSecretParameter_MarksAsSecret()
// Assert
var (resource, snapshot) = Assert.Single(updates);
Assert.Same(secretParam, resource);
Assert.Equal(KnownResourceStates.Active, snapshot.State?.Text);
Assert.Equal(KnownResourceStateStyles.Success, snapshot.State?.Style);
Assert.Equal(KnownResourceStates.Running, snapshot.State?.Text);
}

[Fact]
Expand Down Expand Up @@ -246,17 +245,17 @@ public async Task HandleUnresolvedParametersAsync_WithMultipleUnresolvedParamete
Assert.Equal("secretValue", await secretParam.WaitForValueTcs.Task);

// Notification service should have received updates for each parameter
// Marking them as Active with the provided values
// Marking them as Running with the provided values
await updates.MoveNextAsync();
Assert.Equal(KnownResourceStates.Active, updates.Current.Snapshot.State?.Text);
Assert.Equal(KnownResourceStates.Running, updates.Current.Snapshot.State?.Text);
Assert.Equal("value1", updates.Current.Snapshot.Properties.FirstOrDefault(p => p.Name == KnownProperties.Parameter.Value)?.Value);

await updates.MoveNextAsync();
Assert.Equal(KnownResourceStates.Active, updates.Current.Snapshot.State?.Text);
Assert.Equal(KnownResourceStates.Running, updates.Current.Snapshot.State?.Text);
Assert.Equal("value2", updates.Current.Snapshot.Properties.FirstOrDefault(p => p.Name == KnownProperties.Parameter.Value)?.Value);

await updates.MoveNextAsync();
Assert.Equal(KnownResourceStates.Active, updates.Current.Snapshot.State?.Text);
Assert.Equal(KnownResourceStates.Running, updates.Current.Snapshot.State?.Text);
Assert.Equal("secretValue", updates.Current.Snapshot.Properties.FirstOrDefault(p => p.Name == KnownProperties.Parameter.Value)?.Value);
Assert.True(updates.Current.Snapshot.Properties.FirstOrDefault(p => p.Name == KnownProperties.Parameter.Value)?.IsSensitive ?? false);
}
Expand Down
Loading