Skip to content
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

Remove RedisContainerResource and containerize RedisResource. #2073

Merged
merged 3 commits into from
Feb 5, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
1 change: 1 addition & 0 deletions Directory.Packages.props
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@
<PackageVersion Include="Microsoft.Extensions.Http.Resilience" Version="$(MicrosoftExtensionsHttpResiliencePackageVersion)" />
<!-- external dependencies -->
<PackageVersion Include="Confluent.Kafka" Version="2.3.0" />
<PackageVersion Include="Dapper" Version="2.1.28" />
<PackageVersion Include="Dapr.AspNetCore" Version="1.12.0" />
<PackageVersion Include="DnsClient" Version="1.7.0" />
<PackageVersion Include="Grpc.AspNetCore" Version="2.59.0" />
Expand Down
6 changes: 4 additions & 2 deletions src/Aspire.Hosting/ApplicationModel/IResourceBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,16 @@ public interface IResourceBuilder<out T> where T : IResource
/// Adds an annotation to the resource being built.
/// </summary>
/// <typeparam name="TAnnotation">The type of the annotation to add.</typeparam>
/// <param name="behavior">The behavior to use when adding the annotation.</param>
/// <returns>The resource builder instance.</returns>
IResourceBuilder<T> WithAnnotation<TAnnotation>() where TAnnotation : IResourceAnnotation, new() => WithAnnotation(new TAnnotation());
IResourceBuilder<T> WithAnnotation<TAnnotation>(ResourceAnnotationMutationBehavior behavior = ResourceAnnotationMutationBehavior.AddAppend) where TAnnotation : IResourceAnnotation, new() => WithAnnotation(new TAnnotation(), behavior);

/// <summary>
/// Adds an annotation to the resource being built.
/// </summary>
/// <typeparam name="TAnnotation">The type of the annotation to add.</typeparam>
/// <param name="annotation">The annotation to add.</param>
/// <param name="behavior">The behavior to use when adding the annotation.</param>
/// <returns>The resource builder instance.</returns>
IResourceBuilder<T> WithAnnotation<TAnnotation>(TAnnotation annotation) where TAnnotation : IResourceAnnotation;
IResourceBuilder<T> WithAnnotation<TAnnotation>(TAnnotation annotation, ResourceAnnotationMutationBehavior behavior = ResourceAnnotationMutationBehavior.AddAppend) where TAnnotation : IResourceAnnotation;
}
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using Aspire.Hosting.ApplicationModel;
namespace Aspire.Hosting.ApplicationModel;

namespace Aspire.Hosting.Redis;
public interface IRedisResource : IResourceWithConnectionString
public enum ResourceAnnotationMutationBehavior
{
AddAppend,
AddReplace
mitchdenny marked this conversation as resolved.
Show resolved Hide resolved
}
17 changes: 16 additions & 1 deletion src/Aspire.Hosting/DistributedApplicationResourceBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,23 @@ internal sealed class DistributedApplicationResourceBuilder<T>(IDistributedAppli
public T Resource { get; } = resource;
public IDistributedApplicationBuilder ApplicationBuilder { get; } = applicationBuilder;

public IResourceBuilder<T> WithAnnotation<TAnnotation>(TAnnotation annotation) where TAnnotation : IResourceAnnotation
/// <inheritdoc />
public IResourceBuilder<T> WithAnnotation<TAnnotation>(TAnnotation annotation, ResourceAnnotationMutationBehavior behavior = ResourceAnnotationMutationBehavior.AddAppend) where TAnnotation : IResourceAnnotation
{
// Some defensive code to protect against introducing a new enumeration value without first updating
// this code to accomodate it.
if (behavior != ResourceAnnotationMutationBehavior.AddAppend && behavior != ResourceAnnotationMutationBehavior.AddReplace)
{
throw new ArgumentOutOfRangeException(nameof(behavior), behavior, "ResourceAnnotationMutationBehavior must be either AddAppend or AddReplace.");
Copy link
Member

Choose a reason for hiding this comment

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

Does Add prefix need to be dropped here? If so, then maybe use nameof(..) so it is in sync.

}

// If the behavior is AddReplace then there should never be more than one annotation present. The following call will result in an exception which
// allows us to easily spot these bugs.
if (behavior == ResourceAnnotationMutationBehavior.AddReplace && Resource.Annotations.OfType<TAnnotation>().SingleOrDefault() is { } existingAnnotation)
{
Resource.Annotations.Remove(existingAnnotation);
}

Resource.Annotations.Add(annotation);
return this;
}
Expand Down
3 changes: 2 additions & 1 deletion src/Aspire.Hosting/Extensions/ResourceBuilderExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,8 @@ public static IResourceBuilder<T> WithEnvironment<T>(this IResourceBuilder<T> bu
/// <returns>A reference to the <see cref="IResourceBuilder{T}"/>.</returns>
public static IResourceBuilder<T> WithManifestPublishingCallback<T>(this IResourceBuilder<T> builder, Action<ManifestPublishingContext> callback) where T : IResource
{
return builder.WithAnnotation(new ManifestPublishingCallbackAnnotation(callback));
// You can only ever have one manifest publishing callback, so it must be a replace operation.
return builder.WithAnnotation(new ManifestPublishingCallbackAnnotation(callback), ResourceAnnotationMutationBehavior.AddReplace);
}

private static bool ContainsAmbiguousEndpoints(IEnumerable<AllocatedEndpointAnnotation> endpoints)
Expand Down
26 changes: 8 additions & 18 deletions src/Aspire.Hosting/Redis/RedisBuilderExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,22 +14,6 @@ namespace Aspire.Hosting;
/// </summary>
public static class RedisBuilderExtensions
{
/// <summary>
/// Adds a Redis container to the application model. The default image is "redis" and tag is "latest".
/// </summary>
/// <param name="builder">The <see cref="IDistributedApplicationBuilder"/>.</param>
/// <param name="name">The name of the resource. This name will be used as the connection string name when referenced in a dependency.</param>
/// <param name="port">The host port for the redis server.</param>
/// <returns>A reference to the <see cref="IResourceBuilder{T}"/>.</returns>
public static IResourceBuilder<RedisContainerResource> AddRedisContainer(this IDistributedApplicationBuilder builder, string name, int? port = null)
{
var redis = new RedisContainerResource(name);
return builder.AddResource(redis)
.WithManifestPublishingCallback(context => WriteRedisContainerResourceToManifest(context, redis))
.WithAnnotation(new EndpointAnnotation(ProtocolType.Tcp, port: port, containerPort: 6379))
.WithAnnotation(new ContainerImageAnnotation { Image = "redis", Tag = "latest" });
}

/// <summary>
/// Adds a Redis container to the application model. The default image is "redis" and tag is "latest".
/// </summary>
Expand All @@ -45,7 +29,8 @@ public static IResourceBuilder<RedisResource> AddRedis(this IDistributedApplicat
.WithAnnotation(new ContainerImageAnnotation { Image = "redis", Tag = "latest" });
}

public static IResourceBuilder<T> WithRedisCommander<T>(this IResourceBuilder<T> builder, string? containerName = null, int? hostPort = null) where T: IRedisResource

public static IResourceBuilder<RedisResource> WithRedisCommander(this IResourceBuilder<RedisResource> builder, string? containerName = null, int? hostPort = null)
{
if (builder.ApplicationBuilder.Resources.OfType<RedisCommanderResource>().Any())
{
Expand All @@ -70,7 +55,12 @@ private static void WriteRedisResourceToManifest(ManifestPublishingContext conte
context.Writer.WriteString("type", "redis.v0");
}

private static void WriteRedisContainerResourceToManifest(ManifestPublishingContext context, RedisContainerResource resource)
public static IResourceBuilder<RedisResource> PublishAsContainer(this IResourceBuilder<RedisResource> builder)
{
return builder.WithManifestPublishingCallback(context => WriteRedisContainerResourceToManifest(context, builder.Resource));
}

private static void WriteRedisContainerResourceToManifest(ManifestPublishingContext context, RedisResource resource)
{
context.WriteContainer(resource);
context.Writer.WriteString( // "connectionString": "...",
Expand Down
2 changes: 1 addition & 1 deletion src/Aspire.Hosting/Redis/RedisCommanderConfigWriterHook.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ public Task AfterEndpointsAllocatedAsync(DistributedApplicationModel appModel, C
return Task.CompletedTask;
}

var redisInstances = appModel.Resources.OfType<IRedisResource>();
var redisInstances = appModel.Resources.OfType<RedisResource>();

if (!redisInstances.Any())
{
Expand Down
29 changes: 0 additions & 29 deletions src/Aspire.Hosting/Redis/RedisContainerResource.cs

This file was deleted.

4 changes: 1 addition & 3 deletions src/Aspire.Hosting/Redis/RedisResource.cs
Original file line number Diff line number Diff line change
@@ -1,15 +1,13 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using Aspire.Hosting.Redis;

namespace Aspire.Hosting.ApplicationModel;

/// <summary>
/// A resource that represents a Redis resource independent of the hosting model.
/// </summary>
/// <param name="name">The name of the resource.</param>
public class RedisResource(string name) : Resource(name), IResourceWithConnectionString, IRedisResource
public class RedisResource(string name) : ContainerResource(name), IResourceWithConnectionString
{
/// <summary>
/// Gets the connection string for the Redis server.
Expand Down
23 changes: 22 additions & 1 deletion tests/Aspire.Hosting.Tests/ManifestGenerationTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ public void EnsureAllRedisManifestTypesHaveVersion0Suffix()
var program = CreateTestProgramJsonDocumentManifestPublisher();

program.AppBuilder.AddRedis("redisabstract");
program.AppBuilder.AddRedisContainer("rediscontainer");
program.AppBuilder.AddRedis("rediscontainer").PublishAsContainer();

// Build AppHost so that publisher can be resolved.
program.Build();
Expand All @@ -211,6 +211,27 @@ public void EnsureAllRedisManifestTypesHaveVersion0Suffix()
Assert.Equal("container.v0", container.GetProperty("type").GetString());
}

[Fact]
public void PublishingRedisResourceAsContainerResultsInConnectionStringProperty()
{
var program = CreateTestProgramJsonDocumentManifestPublisher();

program.AppBuilder.AddRedis("rediscontainer").PublishAsContainer();

// Build AppHost so that publisher can be resolved.
program.Build();
var publisher = program.GetManifestPublisher();

program.Run();

var resources = publisher.ManifestDocument.RootElement.GetProperty("resources");

var container = resources.GetProperty("rediscontainer");
Assert.Equal("container.v0", container.GetProperty("type").GetString());
Assert.Equal("{rediscontainer.bindings.tcp.host}:{rediscontainer.bindings.tcp.port}", container.GetProperty("connectionString").GetString());

}

[Fact]
public void EnsureAllPostgresManifestTypesHaveVersion0Suffix()
{
Expand Down
Loading
Loading