Skip to content

Commit 99a2bde

Browse files
joperezrjoperezr
committed
Revert "Fix crash when reading boolean config values from aspire.config.json (microsoft#15383)"
This reverts commit aad1601.
1 parent aad1601 commit 99a2bde

File tree

6 files changed

+3
-206
lines changed

6 files changed

+3
-206
lines changed

‎src/Aspire.Cli/Configuration/FlexibleBooleanConverter.cs‎

Lines changed: 0 additions & 42 deletions
This file was deleted.

‎src/Aspire.Cli/JsonSourceGenerationContext.cs‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,7 @@ namespace Aspire.Cli;
1818
WriteIndented = true,
1919
DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull,
2020
AllowTrailingCommas = true,
21-
ReadCommentHandling = JsonCommentHandling.Skip,
22-
Converters = [typeof(FlexibleBooleanConverter)])]
21+
ReadCommentHandling = JsonCommentHandling.Skip)]
2322
[JsonSerializable(typeof(CliSettings))]
2423
[JsonSerializable(typeof(JsonObject))]
2524
[JsonSerializable(typeof(ListIntegrationsResponse))]

‎src/Aspire.Cli/Utils/ConfigurationHelper.cs‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -154,15 +154,13 @@ internal static bool TryNormalizeSettingsFile(string filePath)
154154
}
155155

156156
// Find all colon-separated keys at root level
157-
var colonKeys = new List<(string key, JsonNode? value)>();
157+
var colonKeys = new List<(string key, string? value)>();
158158

159159
foreach (var kvp in settings)
160160
{
161161
if (kvp.Key.Contains(':'))
162162
{
163-
// DeepClone preserves the original JSON type (boolean, number, etc.)
164-
// instead of converting to string via ToString().
165-
colonKeys.Add((kvp.Key, kvp.Value?.DeepClone()));
163+
colonKeys.Add((kvp.Key, kvp.Value?.ToString()));
166164
}
167165
}
168166

‎tests/Aspire.Cli.Tests/Configuration/AspireConfigFileTests.cs‎

Lines changed: 0 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -306,69 +306,6 @@ public void GetIntegrationReferences_IncludesPackagesAndBasePackage()
306306
Assert.Contains(refs, r => r.Name == "Aspire.Hosting.Redis");
307307
}
308308

309-
[Fact]
310-
public void Load_ReturnsConfig_WhenFeaturesAreBooleans()
311-
{
312-
using var workspace = TemporaryWorkspace.Create(outputHelper);
313-
314-
var configPath = Path.Combine(workspace.WorkspaceRoot.FullName, AspireConfigFile.FileName);
315-
File.WriteAllText(configPath, """
316-
{
317-
"features": { "polyglotSupportEnabled": true, "showAllTemplates": false }
318-
}
319-
""");
320-
321-
var result = AspireConfigFile.Load(workspace.WorkspaceRoot.FullName);
322-
323-
Assert.NotNull(result);
324-
Assert.NotNull(result.Features);
325-
Assert.True(result.Features["polyglotSupportEnabled"]);
326-
Assert.False(result.Features["showAllTemplates"]);
327-
}
328-
329-
[Fact]
330-
public void Load_ReturnsConfig_WhenFeaturesAreStringBooleans()
331-
{
332-
using var workspace = TemporaryWorkspace.Create(outputHelper);
333-
334-
// Simulates what happens when ConfigurationService.SetNestedValue wrote "true"/"false" as strings
335-
var configPath = Path.Combine(workspace.WorkspaceRoot.FullName, AspireConfigFile.FileName);
336-
File.WriteAllText(configPath, """
337-
{
338-
"features": { "polyglotSupportEnabled": "true", "showAllTemplates": "false" }
339-
}
340-
""");
341-
342-
var result = AspireConfigFile.Load(workspace.WorkspaceRoot.FullName);
343-
344-
Assert.NotNull(result);
345-
Assert.NotNull(result.Features);
346-
Assert.True(result.Features["polyglotSupportEnabled"]);
347-
Assert.False(result.Features["showAllTemplates"]);
348-
}
349-
350-
[Fact]
351-
public void Save_Load_RoundTrips_WithFeatures()
352-
{
353-
using var workspace = TemporaryWorkspace.Create(outputHelper);
354-
355-
var config = new AspireConfigFile
356-
{
357-
Features = new Dictionary<string, bool>
358-
{
359-
["polyglotSupportEnabled"] = true,
360-
["showAllTemplates"] = false
361-
}
362-
};
363-
364-
config.Save(workspace.WorkspaceRoot.FullName);
365-
var loaded = AspireConfigFile.Load(workspace.WorkspaceRoot.FullName);
366-
367-
Assert.NotNull(loaded?.Features);
368-
Assert.True(loaded.Features["polyglotSupportEnabled"]);
369-
Assert.False(loaded.Features["showAllTemplates"]);
370-
}
371-
372309
[Fact]
373310
public void Load_RoundTrips_WithProfiles()
374311
{

‎tests/Aspire.Cli.Tests/Configuration/ConfigurationHelperTests.cs‎

Lines changed: 0 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
// Licensed to the .NET Foundation under one or more agreements.
22
// The .NET Foundation licenses this file to you under the MIT license.
33

4-
using System.Text.Json;
5-
using System.Text.Json.Nodes;
64
using Aspire.Cli.Configuration;
75
using Aspire.Cli.Tests.Utils;
86
using Aspire.Cli.Utils;
@@ -110,35 +108,4 @@ public void RegisterSettingsFiles_HandlesCommentsAndTrailingCommas()
110108
Assert.Equal("MyApp.csproj", config["appHost:path"]);
111109
Assert.Equal("daily", config["channel"]);
112110
}
113-
114-
[Fact]
115-
public void TryNormalizeSettingsFile_PreservesBooleanTypes()
116-
{
117-
using var workspace = TemporaryWorkspace.Create(outputHelper);
118-
119-
var settingsPath = Path.Combine(workspace.WorkspaceRoot.FullName, AspireConfigFile.FileName);
120-
// File has a colon-separated key with a boolean value
121-
File.WriteAllText(settingsPath, """
122-
{
123-
"features:polyglotSupportEnabled": true,
124-
"features:showAllTemplates": false
125-
}
126-
""");
127-
128-
var normalized = ConfigurationHelper.TryNormalizeSettingsFile(settingsPath);
129-
130-
Assert.True(normalized);
131-
132-
var json = JsonNode.Parse(File.ReadAllText(settingsPath));
133-
var polyglotNode = json!["features"]!["polyglotSupportEnabled"];
134-
var templatesNode = json!["features"]!["showAllTemplates"];
135-
Assert.Equal(JsonValueKind.True, polyglotNode!.GetValueKind());
136-
Assert.Equal(JsonValueKind.False, templatesNode!.GetValueKind());
137-
138-
// Verify the file can be loaded by AspireConfigFile without error
139-
var config = AspireConfigFile.Load(workspace.WorkspaceRoot.FullName);
140-
Assert.NotNull(config?.Features);
141-
Assert.True(config.Features["polyglotSupportEnabled"]);
142-
Assert.False(config.Features["showAllTemplates"]);
143-
}
144111
}

‎tests/Aspire.Cli.Tests/Configuration/ConfigurationServiceTests.cs‎

Lines changed: 0 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
// Licensed to the .NET Foundation under one or more agreements.
22
// The .NET Foundation licenses this file to you under the MIT license.
33

4-
using System.Text.Json;
5-
using System.Text.Json.Nodes;
64
using Aspire.Cli.Configuration;
75
using Aspire.Cli.Tests.Utils;
86
using Microsoft.Extensions.Configuration;
@@ -227,64 +225,4 @@ public async Task SetConfigurationAsync_SetsNestedValues()
227225
Assert.Contains("appHost", result);
228226
Assert.Contains("MyApp/MyApp.csproj", result);
229227
}
230-
231-
[Fact]
232-
public async Task SetConfigurationAsync_WritesBooleanStringAsJsonString()
233-
{
234-
using var workspace = TemporaryWorkspace.Create(outputHelper);
235-
236-
var (service, settingsFilePath) = CreateService(workspace, "{}");
237-
238-
await service.SetConfigurationAsync("features.polyglotSupportEnabled", "true", isGlobal: false);
239-
240-
// Value is written as a JSON string "true", not a JSON boolean true.
241-
// The FlexibleBooleanConverter handles parsing "true" -> bool on read.
242-
var json = JsonNode.Parse(File.ReadAllText(settingsFilePath));
243-
var node = json!["features"]!["polyglotSupportEnabled"];
244-
Assert.Equal(JsonValueKind.String, node!.GetValueKind());
245-
Assert.Equal("true", node.GetValue<string>());
246-
247-
// Verify round-trip through AspireConfigFile.Load still works
248-
var config = AspireConfigFile.Load(workspace.WorkspaceRoot.FullName);
249-
Assert.NotNull(config?.Features);
250-
Assert.True(config.Features["polyglotSupportEnabled"]);
251-
}
252-
253-
[Fact]
254-
public async Task SetConfigurationAsync_ChannelWithBooleanLikeValue_StaysAsString()
255-
{
256-
using var workspace = TemporaryWorkspace.Create(outputHelper);
257-
258-
var (service, settingsFilePath) = CreateService(workspace, "{}");
259-
260-
// "true" is a valid channel value and must remain a string in JSON
261-
// to avoid corrupting the string-typed Channel property.
262-
await service.SetConfigurationAsync("channel", "true", isGlobal: false);
263-
264-
// Must be a JSON string "true", not a JSON boolean true
265-
var json = JsonNode.Parse(File.ReadAllText(settingsFilePath));
266-
var node = json!["channel"];
267-
Assert.Equal(JsonValueKind.String, node!.GetValueKind());
268-
Assert.Equal("true", node.GetValue<string>());
269-
270-
// Verify it round-trips correctly through AspireConfigFile.Load
271-
var config = AspireConfigFile.Load(workspace.WorkspaceRoot.FullName);
272-
Assert.NotNull(config);
273-
Assert.Equal("true", config.Channel);
274-
}
275-
276-
[Fact]
277-
public async Task SetConfigurationAsync_WritesStringValueAsString()
278-
{
279-
using var workspace = TemporaryWorkspace.Create(outputHelper);
280-
281-
var (service, settingsFilePath) = CreateService(workspace, "{}");
282-
283-
await service.SetConfigurationAsync("channel", "daily", isGlobal: false);
284-
285-
var json = JsonNode.Parse(File.ReadAllText(settingsFilePath));
286-
var node = json!["channel"];
287-
Assert.Equal(JsonValueKind.String, node!.GetValueKind());
288-
Assert.Equal("daily", node.GetValue<string>());
289-
}
290228
}

0 commit comments

Comments
 (0)