From dfba0bad285a47304fd8f64f107cfeb625bab093 Mon Sep 17 00:00:00 2001 From: David Ebbo Date: Tue, 3 Sep 2024 10:53:11 +0200 Subject: [PATCH] Add AddParameter overloads that take a constant and a ParameterDefault Fixes #5410 --- .../ParameterResourceBuilderExtensions.cs | 41 ++++++- src/Aspire.Hosting/PublicAPI.Unshipped.txt | 2 + .../Aspire.Hosting.Tests/AddParameterTests.cs | 101 ++++++++++++++++++ 3 files changed, 143 insertions(+), 1 deletion(-) diff --git a/src/Aspire.Hosting/ParameterResourceBuilderExtensions.cs b/src/Aspire.Hosting/ParameterResourceBuilderExtensions.cs index b9190f57be2..57317e62199 100644 --- a/src/Aspire.Hosting/ParameterResourceBuilderExtensions.cs +++ b/src/Aspire.Hosting/ParameterResourceBuilderExtensions.cs @@ -27,6 +27,43 @@ public static IResourceBuilder AddParameter(this IDistributed return builder.AddParameter(name, parameterDefault => GetParameterValue(builder.Configuration, name, parameterDefault), secret: secret); } + /// + /// Adds a parameter resource to the application, providing a default value to beused as a fallback. + /// + /// Distributed application builder + /// Name of parameter resource + /// + /// Optional flag indicating whether the parameter should be regarded as secret. + /// + [System.Diagnostics.CodeAnalysis.SuppressMessage("ApiDesign", "RS0026:Do not add multiple public overloads with optional parameters", + Justification = "third parameters are mutually exclusive.")] + public static IResourceBuilder AddParameter(this IDistributedApplicationBuilder builder, string name, string defaultValue, bool secret = false) + { + // An alternate implementation is to use some ConstantParameterDefault implementation that always returns the default value. + // However, doing this causes a "default": {} to be written to the manifest, which is not valid. + // And note that we ignore parameterDefault in the callback, because it can never be non-null, and we want our own default. + return builder.AddParameter(name, parameterDefault => builder.Configuration[$"Parameters:{name}"] ?? defaultValue, secret: secret); + } + + /// + /// Adds a parameter resource to the application, providing a ParameterDefault to be used as a fallback. + /// + /// Distributed application builder + /// Name of parameter resource + /// + /// Optional flag indicating whether the parameter should be regarded as secret. + /// + [System.Diagnostics.CodeAnalysis.SuppressMessage("ApiDesign", "RS0026:Do not add multiple public overloads with optional parameters", + Justification = "third parameters are mutually exclusive.")] + public static IResourceBuilder AddParameter(this IDistributedApplicationBuilder builder, string name, ParameterDefault defaultValue, bool secret = false) + { + return builder.AddParameter( + name, + parameterDefault => GetParameterValue(builder.Configuration, name, parameterDefault), + secret: secret, + parameterDefault: defaultValue); + } + private static string GetParameterValue(IConfiguration configuration, string name, ParameterDefault? parameterDefault) { var configurationKey = $"Parameters:{name}"; @@ -39,10 +76,12 @@ internal static IResourceBuilder AddParameter(this IDistribut string name, Func callback, bool secret = false, - bool connectionString = false) + bool connectionString = false, + ParameterDefault? parameterDefault = null) { var resource = new ParameterResource(name, callback, secret); resource.IsConnectionString = connectionString; + resource.Default = parameterDefault; var state = new CustomResourceSnapshot() { diff --git a/src/Aspire.Hosting/PublicAPI.Unshipped.txt b/src/Aspire.Hosting/PublicAPI.Unshipped.txt index a0ff5f8815b..510a77a266c 100644 --- a/src/Aspire.Hosting/PublicAPI.Unshipped.txt +++ b/src/Aspire.Hosting/PublicAPI.Unshipped.txt @@ -50,6 +50,8 @@ static Aspire.Hosting.ApplicationModel.ResourceExtensions.GetEnvironmentVariable Aspire.Hosting.ApplicationModel.ResourceNotificationService.WaitForResourceAsync(string! resourceName, string? targetState = null, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task! Aspire.Hosting.ApplicationModel.ResourceNotificationService.WaitForResourceAsync(string! resourceName, System.Collections.Generic.IEnumerable! targetStates, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task! static Aspire.Hosting.ContainerResourceBuilderExtensions.WithContainerLifetime(this Aspire.Hosting.ApplicationModel.IResourceBuilder! builder, Aspire.Hosting.ApplicationModel.ContainerLifetimeType lifetimeType) -> Aspire.Hosting.ApplicationModel.IResourceBuilder! +static Aspire.Hosting.ParameterResourceBuilderExtensions.AddParameter(this Aspire.Hosting.IDistributedApplicationBuilder! builder, string! name, Aspire.Hosting.ApplicationModel.ParameterDefault! defaultValue, bool secret = false) -> Aspire.Hosting.ApplicationModel.IResourceBuilder! +static Aspire.Hosting.ParameterResourceBuilderExtensions.AddParameter(this Aspire.Hosting.IDistributedApplicationBuilder! builder, string! name, string! defaultValue, bool secret = false) -> Aspire.Hosting.ApplicationModel.IResourceBuilder! static Aspire.Hosting.ProjectResourceBuilderExtensions.WithEndpointsInEnvironment(this Aspire.Hosting.ApplicationModel.IResourceBuilder! builder, System.Func! filter) -> Aspire.Hosting.ApplicationModel.IResourceBuilder! Aspire.Hosting.DistributedApplicationExecutionContext.DistributedApplicationExecutionContext(Aspire.Hosting.DistributedApplicationExecutionContextOptions! options) -> void Aspire.Hosting.DistributedApplicationExecutionContext.ServiceProvider.get -> System.IServiceProvider! diff --git a/tests/Aspire.Hosting.Tests/AddParameterTests.cs b/tests/Aspire.Hosting.Tests/AddParameterTests.cs index 5bbb1b30e52..81e315c56da 100644 --- a/tests/Aspire.Hosting.Tests/AddParameterTests.cs +++ b/tests/Aspire.Hosting.Tests/AddParameterTests.cs @@ -3,6 +3,7 @@ using Aspire.Hosting.Lifecycle; using Aspire.Hosting.Publishing; +using Aspire.Hosting.Utils; using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; using Xunit; @@ -111,6 +112,106 @@ public void ParametersWithConfigurationValueDoNotGetDefaultValue() Assert.Equal("ValueFromConfiguration", parameterResource.Value); } + [Fact] + public async Task ParametersWithDefaultValueStringOverloadOnlyUsedIfNoConfigurationValue() + { + var appBuilder = DistributedApplication.CreateBuilder(); + + appBuilder.Configuration.AddInMemoryCollection(new Dictionary + { + ["Parameters:val1"] = "ValueFromConfiguration", + }); + + // We have 2 params, one with a config value and one without. Both get a default value. + var parameter1 = appBuilder.AddParameter("val1", "DefaultValue1"); + var parameter2 = appBuilder.AddParameter("val2", "DefaultValue2"); + + using var app = appBuilder.Build(); + + var appModel = app.Services.GetRequiredService(); + + // Make sure the config value is used for the first parameter + var parameterResource1 = Assert.Single(appModel.Resources.OfType(), r => r.Name == "val1"); + Assert.Equal("ValueFromConfiguration", parameterResource1.Value); + + // Make sure the default value is used for the second parameter, since there is no config value + var parameterResource2 = Assert.Single(appModel.Resources.OfType(), r => r.Name == "val2"); + Assert.Equal("DefaultValue2", parameterResource2.Value); + + // Note that the manifest should not include anything about the default value + var param1Manifest = await ManifestUtils.GetManifest(parameter1.Resource); + var expectedManifest = $$""" + { + "type": "parameter.v0", + "value": "{val1.inputs.value}", + "inputs": { + "value": { + "type": "string" + } + } + } + """; + Assert.Equal(expectedManifest, param1Manifest.ToString()); + } + + [Fact] + public async Task ParametersWithDefaultValueObjectOverloadOnlyUsedIfNoConfigurationValue() + { + var appBuilder = DistributedApplication.CreateBuilder(); + + appBuilder.Configuration.AddInMemoryCollection(new Dictionary + { + ["Parameters:val1"] = "ValueFromConfiguration", + }); + + var genParam = new GenerateParameterDefault + { + MinLength = 10, + // just use letters for the username since it can't start with a number + Numeric = false, + Special = false + }; + + // We have 2 params, one with a config value and one without. Both get a generated param default value. + var parameter1 = appBuilder.AddParameter("val1", genParam); + var parameter2 = appBuilder.AddParameter("val2", genParam); + + using var app = appBuilder.Build(); + + var appModel = app.Services.GetRequiredService(); + + // Make sure the config value is used for the first parameter + var parameterResource1 = Assert.Single(appModel.Resources.OfType(), r => r.Name == "val1"); + Assert.Equal("ValueFromConfiguration", parameterResource1.Value); + + // Make sure the generated default value is used for the second parameter, since there is no config value + // We can't test the exact value since it's random, but we can test the length + var parameterResource2 = Assert.Single(appModel.Resources.OfType(), r => r.Name == "val2"); + Assert.Equal(10, parameterResource2.Value.Length); + + // The manifest should include the fields for the generated default value + var param1Manifest = await ManifestUtils.GetManifest(parameter1.Resource); + var expectedManifest = $$""" + { + "type": "parameter.v0", + "value": "{val1.inputs.value}", + "inputs": { + "value": { + "type": "string", + "default": { + "generate": { + "minLength": 10, + "numeric": false, + "special": false + } + } + } + } + } + """; + Assert.Equal(expectedManifest, param1Manifest.ToString()); + } + private sealed class TestParameterDefault(string defaultValue) : ParameterDefault { public override string GetDefaultValue() => defaultValue;