dotnet / aspire

Tools, templates, and packages to accelerate building observable, production-ready apps
https://learn.microsoft.com/dotnet/aspire
MIT License
3.95k stars 485 forks source link

Consider adding methods to easily get a typed resource builder via the resource name #3022

Open DamianEdwards opened 8 months ago

DamianEdwards commented 8 months ago

In the following test, notice the complicated line required to get an existing resource from the builder and modify it using the extension methods on the specific resource type:

[Fact]
public async Task GetWebResourceHealthReturnsUnhealthyWhenRedisUnavailable()
{
    // Arrange
    var appHost = await DistributedApplicationTestingBuilder.CreateAsync<Projects.AspireApp41_AppHost>();
    var redis = appHost.CreateResourceBuilder((RedisResource)appHost.Resources.First(r => r.Name == "cache"));
    redis.WithEntrypoint("sleep 1d");

    await using var app = await appHost.BuildAsync();
    await app.StartAsync();

    // Act
    var httpClient = new HttpClient { BaseAddress = app.GetEndpoint("webfrontend") };
    var response = await httpClient.GetAsync("/health");

    // Assert
    Assert.Equal(HttpStatusCode.ServiceUnavailable, response.StatusCode);
    Assert.Equal("Unhealthy", await response.Content.ReadAsStringAsync());
}

This line:

var redis = appHost.CreateResourceBuilder((RedisResource)appHost.Resources.First(r => r.Name == "cache"));

We should consider adding extension methods to make it easy to get an IResourceBuilder<TResource> for an existing resource in a builder by the resource name. For example, for Redis:

public static class DistributedApplicationTestingBuilderExtensions
{
    public static IResourceBuilder<RedisResource> GetRedis(this IDistributedApplicationTestingBuilder builder, string name)
    {
        var resource = builder.Resources.FirstOrDefault(r => string.Equals(r.Name, name, StringComparison.OrdinalIgnoreCase))
            ?? throw new ArgumentException($"Resource with name '{name}' was not found.", nameof(name));

        var redisResource = resource as RedisResource
            ?? throw new ArgumentException($"Resource with name '{name}' is not a Redis resource.", nameof(name));

        return builder.CreateResourceBuilder(redisResource);
    }
}

Then the line in the test above could become:

appHost.GetRedis("cache").WithEntrypoint("sleep 1d");

This has a nice symmetry with the builder.AddRedis("cache") call in the AppHost project's Program.cs that added the resource.

One challenge will be that if we want these methods to be on IDistributedApplicationTestingBuilder, they would need to exist in new packages for each resource type, as we recently split all the hosting resources into their own packages. So e.g. for Redis we'd need an Aspire.Hosting.Testing.Redis package that depends on Aspire.Hosting.Redis and Aspire.Hosting.Testing, and then the test project would need to depend on that.

mitchdenny commented 7 months ago

There is no reason we couldn't hang these extensions off IDistributedApplicationBuilder?

DamianEdwards commented 7 months ago

The IDistributedApplicationTestingBuilder interface doesn't expose IDistributedApplicationBuilder so we'd need to expose it in both places explicitly.

mitchdenny commented 7 months ago

We already have CreateResourceBuilder(...) that does this on IDistributedApplicationBuilder. Is this more about aligning these two method names across IDAB and IDATB?

We already implement CreateResourceBuilder(...) in both places. The downside is that this would polute the intellisense interface for IDAB and IDATB.

DamianEdwards commented 7 months ago

Pollute in what way?

mitchdenny commented 7 months ago

I take that back. I might be a huge fan of this idea ;) This might provide an elegant solution to this:

var builder = DistributedApplication.CreateBuilder(args);
var sql = builder.AddSqlServer("sql").PublishAsAzureSqlDatabase((resource, construct, sqlServer, databases) => {
  sqlServer.AddOutput("someOutput", blah);
});

// Currently this doesn't work because sql is of type IResourceBuilder<SqlServerServerResource>.
var app = builder.AddProject<Projects.MyApp>("myapp")
                 .WithReference(sql.GetOutput("someOutput"));

// ... but with this API, this might be viable.
var app = builder.AddProject<Projects.MyApp>("myapp")
                 .WithReference(builder.GetAzureSql("sql").GetOutput("someOutput"));

This would be instead of doing something like this:

var sql = builder.AddSqlServer("sql").PublishAsAzureSqlDatabase(out var azureSql);