Skip to content

Commit 27010fc

Browse files
committed
Migrate DotnetToolResource to use RequiredCommandValidator
- Migrated `dotnet tool` version validation to use `RequiredCommandValidator` - Updated `RequiredCommandValidator` to include the callback in teh cache key, so that dotnet tools resources with different working directories are evaluated separately.
1 parent 73082ee commit 27010fc

4 files changed

Lines changed: 67 additions & 41 deletions

File tree

src/Aspire.Hosting/ApplicationModel/RequiredCommandValidator.cs

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,8 @@ internal sealed class RequiredCommandValidator : IRequiredCommandValidator, IDis
2121
private readonly IInteractionService _interactionService;
2222
private readonly ILogger<RequiredCommandValidator> _logger;
2323

24-
// Track validation state per command to coalesce notifications
25-
private readonly ConcurrentDictionary<string, CommandValidationState> _commandStates = new(StringComparer.OrdinalIgnoreCase);
24+
// Track validation state per command/callback pair to coalesce notifications and validation work.
25+
private readonly ConcurrentDictionary<CommandValidationCacheKey, CommandValidationState> _commandStates = new();
2626

2727
public RequiredCommandValidator(
2828
IServiceProvider serviceProvider,
@@ -59,8 +59,9 @@ public async Task<RequiredCommandValidationResult> ValidateAsync(
5959
throw new InvalidOperationException($"Required command on resource '{resource.Name}' cannot be null or empty.");
6060
}
6161

62-
// Get or create state for this command
63-
var state = _commandStates.GetOrAdd(command, _ => new CommandValidationState());
62+
// Get or create state for this command/callback combination.
63+
var cacheKey = new CommandValidationCacheKey(command, annotation.ValidationCallback);
64+
var state = _commandStates.GetOrAdd(cacheKey, _ => new CommandValidationState());
6465

6566
await state.Gate.WaitAsync(cancellationToken).ConfigureAwait(false);
6667
try
@@ -179,6 +180,16 @@ private sealed class CommandValidationState : IDisposable
179180
public void Dispose() => Gate.Dispose();
180181
}
181182

183+
/// <summary>
184+
/// Cache key for command validation state.
185+
/// </summary>
186+
private readonly record struct CommandValidationCacheKey(string command, Func<RequiredCommandValidationContext, Task<RequiredCommandValidationResult>>? callback)
187+
{
188+
public string Command { get; } = command;
189+
190+
public Func<RequiredCommandValidationContext, Task<RequiredCommandValidationResult>>? Callback { get; } = callback;
191+
}
192+
182193
/// <summary>
183194
/// Attempts to resolve a command (file name or path) to a full path.
184195
/// </summary>

src/Aspire.Hosting/DotnetToolResourceExtensions.cs

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ public static IResourceBuilder<T> AddDotnetTool<T>(this IDistributedApplicationB
5050
.WithIconName("Toolbox")
5151
.WithCommand("dotnet")
5252
.WithArgs(BuildToolExecArguments)
53+
.WithRequiredCommand("dotnet", context => ValidateDotnetSdkVersionAsync(context, resource.WorkingDirectory))
5354
.OnBeforeResourceStarted(BeforeResourceStarted);
5455

5556
void BuildToolExecArguments(CommandLineArgsCallbackContext x)
@@ -110,9 +111,6 @@ await rns.PublishUpdateAsync(resource, x => x with
110111
new (KnownProperties.Resource.Source, resource.ToolConfiguration?.PackageId)
111112
])
112113
}).ConfigureAwait(false);
113-
114-
var version = await DotnetSdkUtils.TryGetVersionAsync(resource.WorkingDirectory).ConfigureAwait(false);
115-
ValidateDotnetSdkVersion(version, resource.WorkingDirectory);
116114
}
117115

118116
}
@@ -204,18 +202,15 @@ public static IResourceBuilder<T> WithToolIgnoreFailedSources<T>(this IResourceB
204202
return builder;
205203
}
206204

207-
internal static void ValidateDotnetSdkVersion(Version? version, string workingDirectory)
205+
internal static async Task<RequiredCommandValidationResult> ValidateDotnetSdkVersionAsync(RequiredCommandValidationContext _, string workingDirectory)
208206
{
209-
if (version is null)
210-
{
211-
// This most likely means something is majorly wrong with the dotnet sdk
212-
// Which will show up as an error once dcp runs `dotnet tool exec`
213-
return;
214-
}
207+
var version = await DotnetSdkUtils.TryGetVersionAsync(workingDirectory).ConfigureAwait(false);
215208

216-
if (version.Major < 10)
209+
if (version?.Major < 10)
217210
{
218-
throw new DistributedApplicationException($"DotnetToolResource requires dotnet SDK 10 or higher to run. Detected version: {version} for working directory {workingDirectory}");
211+
return RequiredCommandValidationResult.Failure($"DotnetToolResource requires dotnet SDK 10 or higher to run. Detected version: {version}");
219212
}
213+
214+
return RequiredCommandValidationResult.Success();
220215
}
221216
}

tests/Aspire.Hosting.DotnetTool.Tests/AddDotnetToolTests.cs

Lines changed: 9 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -378,29 +378,16 @@ public async Task AddDotnetToolGeneratesCorrectManifest()
378378
Assert.Equal(expectedManifest, manifest.ToString());
379379
}
380380

381-
[Theory]
382-
[InlineData("11.1.0", true)]
383-
[InlineData("10.0.0", true)]
384-
[InlineData("9.0.999", false)]
385-
public void ValidateDotnetSdkVersion_ValidatesVersionCorrectly(string versionString, bool isAllowed)
386-
{
387-
var version = Version.Parse(versionString);
388-
389-
if (isAllowed)
390-
{
391-
DotnetToolResourceExtensions.ValidateDotnetSdkVersion(version, "");
392-
}
393-
else
394-
{
395-
Assert.Throws<DistributedApplicationException>(() =>
396-
DotnetToolResourceExtensions.ValidateDotnetSdkVersion(version, ""));
397-
}
398-
}
399-
400381
[Fact]
401-
public void ValidateDotnetSdkVersion_WithNullVersion_DoesNotThrow()
382+
public void AddDotnetTool_IncludesRequiredCommandAnnotation()
402383
{
403-
// Should not throw - null is treated as "unable to determine version"
404-
DotnetToolResourceExtensions.ValidateDotnetSdkVersion(null, "");
384+
var builder = DistributedApplication.CreateBuilder();
385+
var tool = builder.AddDotnetTool("mytool", "dotnet-ef");
386+
387+
#pragma warning disable ASPIRECOMMAND001
388+
var annotation = Assert.Single(tool.Resource.Annotations.OfType<RequiredCommandAnnotation>());
389+
#pragma warning restore ASPIRECOMMAND001
390+
Assert.Equal("dotnet", annotation.Command);
391+
Assert.NotNull(annotation.ValidationCallback);
405392
}
406393
}

tests/Aspire.Hosting.Tests/RequiredCommandAnnotationTests.cs

Lines changed: 36 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -318,15 +318,48 @@ public async Task RequiredCommandValidationLifecycleHook_CachesSuccessfulValidat
318318
var command = OperatingSystem.IsWindows() ? "cmd" : "sh";
319319
var callbackCount = 0;
320320

321+
Task<RequiredCommandValidationResult> callback(RequiredCommandValidationContext _)
322+
{
323+
Interlocked.Increment(ref callbackCount);
324+
return Task.FromResult(RequiredCommandValidationResult.Success());
325+
}
326+
321327
builder.AddContainer("test1", "image")
322-
.WithRequiredCommand(command, ctx =>
328+
.WithRequiredCommand(command, callback);
329+
330+
builder.AddContainer("test2", "image")
331+
.WithRequiredCommand(command, callback);
332+
333+
await using var app = builder.Build();
334+
await SubscribeHooksAsync(app);
335+
336+
var appModel = app.Services.GetRequiredService<DistributedApplicationModel>();
337+
var resource1 = appModel.Resources.Single(r => r.Name == "test1");
338+
var resource2 = appModel.Resources.Single(r => r.Name == "test2");
339+
var eventing = app.Services.GetRequiredService<IDistributedApplicationEventing>();
340+
341+
await eventing.PublishAsync(new BeforeResourceStartedEvent(resource1, app.Services));
342+
await eventing.PublishAsync(new BeforeResourceStartedEvent(resource2, app.Services));
343+
344+
Assert.Equal(1, callbackCount);
345+
}
346+
347+
[Fact]
348+
public async Task RequiredCommandValidationLifecycleHook_DoesNotCacheAcrossDifferentValidationCallbacks()
349+
{
350+
var builder = DistributedApplication.CreateBuilder();
351+
var command = OperatingSystem.IsWindows() ? "cmd" : "sh";
352+
var callbackCount = 0;
353+
354+
builder.AddContainer("test1", "image")
355+
.WithRequiredCommand(command, _ =>
323356
{
324357
Interlocked.Increment(ref callbackCount);
325358
return Task.FromResult(RequiredCommandValidationResult.Success());
326359
});
327360

328361
builder.AddContainer("test2", "image")
329-
.WithRequiredCommand(command, ctx =>
362+
.WithRequiredCommand(command, _ =>
330363
{
331364
Interlocked.Increment(ref callbackCount);
332365
return Task.FromResult(RequiredCommandValidationResult.Success());
@@ -343,7 +376,7 @@ public async Task RequiredCommandValidationLifecycleHook_CachesSuccessfulValidat
343376
await eventing.PublishAsync(new BeforeResourceStartedEvent(resource1, app.Services));
344377
await eventing.PublishAsync(new BeforeResourceStartedEvent(resource2, app.Services));
345378

346-
Assert.Equal(1, callbackCount);
379+
Assert.Equal(2, callbackCount);
347380
}
348381

349382
[Fact]

0 commit comments

Comments
 (0)