diff --git a/src/Aspire.Cli/Program.cs b/src/Aspire.Cli/Program.cs index c4de57968b6..5ffcfda9776 100644 --- a/src/Aspire.Cli/Program.cs +++ b/src/Aspire.Cli/Program.cs @@ -299,7 +299,7 @@ internal static async Task BuildApplicationAsync(string[] args, CliStartu var globalSettingsFilePath = GetGlobalSettingsPath(startupContext.Logger); var globalSettingsFile = new FileInfo(globalSettingsFilePath); var workingDirectory = new DirectoryInfo(Environment.CurrentDirectory); - ConfigurationHelper.RegisterSettingsFiles(builder.Configuration, workingDirectory, globalSettingsFile); + ConfigurationHelper.RegisterSettingsFiles(builder.Configuration, workingDirectory, globalSettingsFile, startupContext.Logger); TrySetLocaleOverride(LocaleHelpers.GetLocaleOverride(builder.Configuration), startupContext.Logger, startupContext.ErrorWriter); WarnIfGlobalSettingsContainAppHostPath(globalSettingsFile, startupContext.ErrorWriter); diff --git a/src/Aspire.Cli/Utils/ConfigurationHelper.cs b/src/Aspire.Cli/Utils/ConfigurationHelper.cs index cc8229ffc2f..042d20c694e 100644 --- a/src/Aspire.Cli/Utils/ConfigurationHelper.cs +++ b/src/Aspire.Cli/Utils/ConfigurationHelper.cs @@ -7,6 +7,7 @@ using Aspire.Cli.Configuration; using Aspire.Cli.Resources; using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.Logging; namespace Aspire.Cli.Utils; @@ -23,7 +24,7 @@ internal static class ConfigurationHelper AllowTrailingCommas = true }; - internal static void RegisterSettingsFiles(IConfigurationBuilder configuration, DirectoryInfo workingDirectory, FileInfo globalSettingsFile) + internal static void RegisterSettingsFiles(IConfigurationBuilder configuration, DirectoryInfo workingDirectory, FileInfo globalSettingsFile, ILogger logger) { var currentDirectory = workingDirectory; @@ -46,6 +47,42 @@ internal static void RegisterSettingsFiles(IConfigurationBuilder configuration, var legacySettingsPath = BuildPathToSettingsJsonFile(currentDirectory.FullName); if (File.Exists(legacySettingsPath)) { + // Eagerly migrate the legacy layout to aspire.config.json on startup so that + // even read-only commands (aspire doctor, aspire ls, aspire --version, etc.) + // move the workspace forward on the user's first run of a newer CLI. Without + // this, migration only happens when a command actively writes settings + // (aspire run/add/update/pipeline), leaving workspaces stuck on the legacy + // layout indefinitely for users who never invoke those commands. + // See https://github.com/microsoft/aspire/issues/15488. + if (LegacySettingsFileHasMigratableData(legacySettingsPath)) + { + try + { + _ = AspireConfigFile.LoadOrCreate(currentDirectory.FullName); + var migratedPath = Path.Combine(currentDirectory.FullName, AspireConfigFile.FileName); + if (File.Exists(migratedPath)) + { + logger.LogInformation( + "Migrated legacy {LegacyPath} to {MigratedPath} on CLI startup.", + legacySettingsPath, + migratedPath); + localSettingsFile = new FileInfo(migratedPath); + break; + } + } + catch (Exception ex) + { + // Migration is best-effort during startup. If it fails (read-only + // directory, IO error, malformed legacy JSON), fall back to using the + // legacy file directly so the CLI still works. The next command that + // writes settings will retry the migration through its normal path. + logger.LogWarning( + ex, + "Failed to migrate legacy {LegacyPath} to aspire.config.json on startup. Falling back to the legacy file.", + legacySettingsPath); + } + } + localSettingsFile = new FileInfo(legacySettingsPath); break; } @@ -71,6 +108,47 @@ internal static string BuildPathToSettingsJsonFile(string workingDirectory) return Path.Combine(workingDirectory, ".aspire", "settings.json"); } + /// + /// Returns true when a legacy .aspire/settings.json file contains an + /// appHostPath entry — the signal that this is a real AppHost workspace worth + /// migrating to aspire.config.json. Files that exist purely as directory-walking + /// stop markers (empty JSON object, whitespace, comment-only, or files that only carry + /// flat colon-keys awaiting in-place normalization) are not migrated because doing so + /// would needlessly materialize an aspire.config.json alongside them with no + /// meaningful content. + /// + private static bool LegacySettingsFileHasMigratableData(string legacySettingsPath) + { + try + { + var content = File.ReadAllText(legacySettingsPath); + if (string.IsNullOrWhiteSpace(content)) + { + return false; + } + + // Read appHostPath directly with JsonDocument to avoid coupling this check to the + // strict-typed AspireJsonConfiguration deserializer, which would fail for files + // that contain loosely-typed values (e.g. string "false" in a bool dictionary). + using var doc = JsonDocument.Parse(content, ParseOptions); + if (doc.RootElement.ValueKind != JsonValueKind.Object) + { + return false; + } + + return doc.RootElement.TryGetProperty("appHostPath", out var appHostPathElement) + && appHostPathElement.ValueKind == JsonValueKind.String + && !string.IsNullOrEmpty(appHostPathElement.GetString()); + } + catch + { + // Treat unreadable or malformed legacy files as not-migratable so startup does + // not throw. The user will see the same JSON parse error from the regular + // configuration registration path below if the file is genuinely broken. + return false; + } + } + internal static DirectoryInfo? GetLegacySettingsRootDirectory(FileInfo settingsFile) { if (!string.Equals(settingsFile.Name, AspireJsonConfiguration.FileName, StringComparison.OrdinalIgnoreCase)) diff --git a/tests/Aspire.Cli.Tests/Configuration/ConfigurationHelperTests.cs b/tests/Aspire.Cli.Tests/Configuration/ConfigurationHelperTests.cs index 5a142fa026f..0f6a189c035 100644 --- a/tests/Aspire.Cli.Tests/Configuration/ConfigurationHelperTests.cs +++ b/tests/Aspire.Cli.Tests/Configuration/ConfigurationHelperTests.cs @@ -7,6 +7,7 @@ using Aspire.Cli.Tests.Utils; using Aspire.Cli.Utils; using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.Logging.Abstractions; namespace Aspire.Cli.Tests.Configuration; @@ -22,7 +23,7 @@ private static IConfiguration BuildConfigurationFromSettingsFile( var globalSettingsFile = new FileInfo(Path.Combine(globalDir.FullName, AspireConfigFile.FileName)); var builder = new ConfigurationBuilder(); - ConfigurationHelper.RegisterSettingsFiles(builder, workspace.WorkspaceRoot, globalSettingsFile); + ConfigurationHelper.RegisterSettingsFiles(builder, workspace.WorkspaceRoot, globalSettingsFile, NullLogger.Instance); return builder.Build(); } @@ -171,4 +172,148 @@ public void GetWorkspaceAspireDirectory_UsesLegacySettingsParentDirectory() Path.Combine(workspace.WorkspaceRoot.FullName, AspireJsonConfiguration.SettingsFolder), aspireDirectory.FullName); } + + [Fact] + public void RegisterSettingsFiles_MigratesLegacySettingsToAspireConfigJsonOnStartup() + { + // Reproduces https://github.com/microsoft/aspire/issues/15488: a user upgrades the + // CLI and runs it against an existing AppHost workspace that only has the legacy + // .aspire/settings.json. The CLI must eagerly migrate the workspace to + // aspire.config.json on startup, regardless of which command the user runs (even + // read-only commands that never pass createSettingsFile: true to ProjectLocator). + using var workspace = TemporaryWorkspace.Create(outputHelper); + + var legacyDir = workspace.CreateDirectory(AspireJsonConfiguration.SettingsFolder); + var legacySettingsPath = Path.Combine(legacyDir.FullName, AspireJsonConfiguration.FileName); + File.WriteAllText(legacySettingsPath, """ + { + "appHostPath": "MyApp.csproj", + "channel": "stable" + } + """); + + var aspireConfigPath = Path.Combine(workspace.WorkspaceRoot.FullName, AspireConfigFile.FileName); + Assert.False(File.Exists(aspireConfigPath), "Precondition: aspire.config.json should not yet exist."); + + var globalDir = workspace.CreateDirectory("global-aspire"); + var globalSettingsFile = new FileInfo(Path.Combine(globalDir.FullName, AspireConfigFile.FileName)); + + var builder = new ConfigurationBuilder(); + ConfigurationHelper.RegisterSettingsFiles(builder, workspace.WorkspaceRoot, globalSettingsFile, NullLogger.Instance); + + Assert.True( + File.Exists(aspireConfigPath), + "aspire.config.json should have been created by eager migration during RegisterSettingsFiles."); + } + + [Fact] + public void RegisterSettingsFiles_DoesNotOverwriteExistingAspireConfigJson() + { + using var workspace = TemporaryWorkspace.Create(outputHelper); + + // Both files present: the workspace was already migrated but the legacy file was + // retained (this is the documented transition state — see AspireConfigFile.LoadOrCreate + // and https://github.com/microsoft/aspire/issues/15239). Startup migration must not + // clobber the existing aspire.config.json or re-migrate from stale legacy data. + var aspireConfigPath = Path.Combine(workspace.WorkspaceRoot.FullName, AspireConfigFile.FileName); + var existingContent = """ + { + "appHost": { "path": "Current.csproj" }, + "channel": "daily" + } + """; + File.WriteAllText(aspireConfigPath, existingContent); + + var legacyDir = workspace.CreateDirectory(AspireJsonConfiguration.SettingsFolder); + var legacySettingsPath = Path.Combine(legacyDir.FullName, AspireJsonConfiguration.FileName); + File.WriteAllText(legacySettingsPath, """ + { "appHostPath": "Stale.csproj", "channel": "stable" } + """); + + var globalDir = workspace.CreateDirectory("global-aspire"); + var globalSettingsFile = new FileInfo(Path.Combine(globalDir.FullName, AspireConfigFile.FileName)); + + var builder = new ConfigurationBuilder(); + ConfigurationHelper.RegisterSettingsFiles(builder, workspace.WorkspaceRoot, globalSettingsFile, NullLogger.Instance); + + Assert.Equal(existingContent, File.ReadAllText(aspireConfigPath)); + + var config = builder.Build(); + Assert.Equal("Current.csproj", config["appHost:path"]); + Assert.Equal("daily", config["channel"]); + } + + [Fact] + public void RegisterSettingsFiles_GuardRejectsUnparseableLegacyFile() + { + using var workspace = TemporaryWorkspace.Create(outputHelper); + + var legacyDir = workspace.CreateDirectory(AspireJsonConfiguration.SettingsFolder); + var legacySettingsPath = Path.Combine(legacyDir.FullName, AspireJsonConfiguration.FileName); + // Unparseable JSON fails JsonDocument.Parse inside LegacySettingsFileHasMigratableData, + // so the guard short-circuits and returns false before migration is attempted. The + // downstream JSON registration via AddSettingsFile is what surfaces the parse error + // to the user, which is the pre-existing "your settings.json is broken" signal. The + // migration step itself must not introduce a new crash path on top of that. + File.WriteAllText(legacySettingsPath, "{ this is not valid json"); + + var globalDir = workspace.CreateDirectory("global-aspire"); + var globalSettingsFile = new FileInfo(Path.Combine(globalDir.FullName, AspireConfigFile.FileName)); + + var builder = new ConfigurationBuilder(); + var ex = Record.Exception(() => + ConfigurationHelper.RegisterSettingsFiles(builder, workspace.WorkspaceRoot, globalSettingsFile, NullLogger.Instance)); + + // The guard rejected the file before migration ran, so aspire.config.json must not have + // been materialized at the workspace root. + var migratedPath = Path.Combine(workspace.WorkspaceRoot.FullName, AspireConfigFile.FileName); + Assert.False(File.Exists(migratedPath), "Unparseable legacy file should not produce a partial aspire.config.json."); + // Either no exception (graceful), or the same InvalidOperationException previously + // thrown by AddSettingsFile for malformed JSON. Both are acceptable; what we're + // proving is that the guard prevented us from crashing inside the new migration step. + Assert.True(ex is null or InvalidOperationException, $"Unexpected exception type: {ex?.GetType().FullName}"); + } + + [Fact] + public void RegisterSettingsFiles_FallsBackToLegacyWhenMigrationLoadThrows() + { + using var workspace = TemporaryWorkspace.Create(outputHelper); + + var legacyDir = workspace.CreateDirectory(AspireJsonConfiguration.SettingsFolder); + var legacySettingsPath = Path.Combine(legacyDir.FullName, AspireJsonConfiguration.FileName); + // This file is *parseable* JSON with a valid string appHostPath, so + // LegacySettingsFileHasMigratableData returns true and we enter the migration try + // block. However, "features" is typed Dictionary with a strict + // FlexibleBooleanDictionaryConverter, so passing a string for it causes + // AspireJsonConfiguration.Load (invoked by AspireConfigFile.LoadOrCreate) to throw a + // JsonException. That exception must be caught and we must fall back to registering + // the legacy file directly. + File.WriteAllText(legacySettingsPath, """ + { + "appHostPath": "MyApp.csproj", + "features": "not-an-object" + } + """); + + var globalDir = workspace.CreateDirectory("global-aspire"); + var globalSettingsFile = new FileInfo(Path.Combine(globalDir.FullName, AspireConfigFile.FileName)); + + var builder = new ConfigurationBuilder(); + var ex = Record.Exception(() => + ConfigurationHelper.RegisterSettingsFiles(builder, workspace.WorkspaceRoot, globalSettingsFile, NullLogger.Instance)); + + Assert.Null(ex); + + // Migration failed inside LoadOrCreate, so aspire.config.json must not exist at the + // workspace root. Its absence proves we hit the catch block and continued past the + // failed migration rather than half-writing a new config file. + var migratedPath = Path.Combine(workspace.WorkspaceRoot.FullName, AspireConfigFile.FileName); + Assert.False(File.Exists(migratedPath), "Failed migration should not produce a partial aspire.config.json."); + + // The fallback registered the legacy file directly, so appHostPath remains readable + // from configuration via its flat key (the JSON source flattens nested objects with ':', + // but appHostPath is a root-level scalar so its key is unchanged). + var config = builder.Build(); + Assert.Equal("MyApp.csproj", config["appHostPath"]); + } } diff --git a/tests/Aspire.Cli.Tests/Utils/CliTestHelper.cs b/tests/Aspire.Cli.Tests/Utils/CliTestHelper.cs index df318da4eaa..60e263717aa 100644 --- a/tests/Aspire.Cli.Tests/Utils/CliTestHelper.cs +++ b/tests/Aspire.Cli.Tests/Utils/CliTestHelper.cs @@ -80,7 +80,7 @@ public static IServiceCollection CreateServiceCollection(TemporaryWorkspace work var globalSettingsFilePath = Path.Combine(options.WorkingDirectory.FullName, ".aspire", "settings.global.json"); var globalSettingsFile = new FileInfo(globalSettingsFilePath); - ConfigurationHelper.RegisterSettingsFiles(configBuilder, options.WorkingDirectory, globalSettingsFile); + ConfigurationHelper.RegisterSettingsFiles(configBuilder, options.WorkingDirectory, globalSettingsFile, NullLogger.Instance); var configuration = configBuilder.Build(); services.AddSingleton(configuration);