Skip to content

Commit ccfebc8

Browse files
baywetbaywet
authored
Merge pull request #2562 from microsoft/copilot/fix-yamlconverter-quote-issue
fix: YamlConverter adding extra quotes to string values when converting from JSON to YAML
2 parents 6e62de2 + ab5f73b commit ccfebc8

File tree

2 files changed

+237
-2
lines changed

2 files changed

+237
-2
lines changed

‎src/Microsoft.OpenApi.YamlReader/YamlConverter.cs‎

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
using System.Collections.Generic;
33
using System.Globalization;
44
using System.Linq;
5+
using System.Text.Json;
56
using System.Text.Json.Nodes;
67
using SharpYaml;
78
using SharpYaml.Serialization;
@@ -133,7 +134,31 @@ ScalarStyle.Plain when YamlNullRepresentations.Contains(yaml.Value) => null,
133134

134135
private static YamlScalarNode ToYamlScalar(this JsonValue val)
135136
{
136-
return new YamlScalarNode(val.ToJsonString());
137+
// Try to get the underlying value based on its actual type
138+
// First try to get it as a string
139+
if (val.GetValueKind() == JsonValueKind.String &&
140+
val.TryGetValue(out string? stringValue))
141+
{
142+
// For string values, we need to determine if they should be quoted in YAML
143+
// Strings that look like numbers, booleans, or null need to be quoted
144+
// to preserve their string type when round-tripping
145+
var needsQuoting = decimal.TryParse(stringValue, NumberStyles.Float, CultureInfo.InvariantCulture, out _) ||
146+
bool.TryParse(stringValue, out _) ||
147+
YamlNullRepresentations.Contains(stringValue);
148+
149+
return new YamlScalarNode(stringValue)
150+
{
151+
Style = needsQuoting ? ScalarStyle.DoubleQuoted : ScalarStyle.Plain
152+
};
153+
}
154+
155+
// For non-string values (numbers, booleans, null), use their string representation
156+
// These should remain unquoted in YAML
157+
var valueString = val.ToString();
158+
return new YamlScalarNode(valueString)
159+
{
160+
Style = ScalarStyle.Plain
161+
};
137162
}
138163
}
139164
}

‎test/Microsoft.OpenApi.Readers.Tests/YamlConverterTests.cs‎

Lines changed: 211 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
1-
using SharpYaml;
1+
using SharpYaml;
22
using SharpYaml.Serialization;
33
using Xunit;
44
using Microsoft.OpenApi.YamlReader;
5+
using System.IO;
6+
using System.Text.Json.Nodes;
57

68
namespace Microsoft.OpenApi.Readers.Tests;
79

@@ -26,4 +28,212 @@ public void YamlNullValuesReturnNullJsonNode(string value)
2628
// Then
2729
Assert.Null(jsonNode);
2830
}
31+
32+
[Fact]
33+
public void ToYamlNode_StringValue_NotQuotedInYaml()
34+
{
35+
// Arrange
36+
var json = Assert.IsType<JsonObject>(JsonNode.Parse(@"{""fooString"": ""fooStringValue""}"));
37+
38+
// Act
39+
var yamlNode = json.ToYamlNode();
40+
var yamlOutput = ConvertYamlNodeToString(yamlNode);
41+
42+
// Assert
43+
Assert.Contains("fooString: fooStringValue", yamlOutput);
44+
Assert.DoesNotContain("\"fooStringValue\"", yamlOutput);
45+
Assert.DoesNotContain("'fooStringValue'", yamlOutput);
46+
}
47+
48+
[Fact]
49+
public void ToYamlNode_StringThatLooksLikeNumber_QuotedInYaml()
50+
{
51+
// Arrange
52+
var json = Assert.IsType<JsonObject>(JsonNode.Parse(@"{""fooStringOfNumber"": ""200""}"));
53+
54+
// Act
55+
var yamlNode = json.ToYamlNode();
56+
var yamlOutput = ConvertYamlNodeToString(yamlNode);
57+
58+
// Assert
59+
Assert.Contains("fooStringOfNumber: \"200\"", yamlOutput);
60+
}
61+
62+
[Fact]
63+
public void ToYamlNode_ActualNumber_NotQuotedInYaml()
64+
{
65+
// Arrange
66+
var json = Assert.IsType<JsonObject>(JsonNode.Parse(@"{""actualNumber"": 200}"));
67+
68+
// Act
69+
var yamlNode = json.ToYamlNode();
70+
var yamlOutput = ConvertYamlNodeToString(yamlNode);
71+
72+
// Assert
73+
Assert.Contains("actualNumber: 200", yamlOutput);
74+
Assert.DoesNotContain("\"200\"", yamlOutput);
75+
}
76+
77+
[Fact]
78+
public void ToYamlNode_StringThatLooksLikeDecimal_QuotedInYaml()
79+
{
80+
// Arrange
81+
var json = Assert.IsType<JsonObject>(JsonNode.Parse(@"{""decimalString"": ""123.45""}"));
82+
83+
// Act
84+
var yamlNode = json.ToYamlNode();
85+
var yamlOutput = ConvertYamlNodeToString(yamlNode);
86+
87+
// Assert
88+
Assert.Contains("decimalString: \"123.45\"", yamlOutput);
89+
}
90+
91+
[Fact]
92+
public void ToYamlNode_ActualDecimal_NotQuotedInYaml()
93+
{
94+
// Arrange
95+
var json = Assert.IsType<JsonObject>(JsonNode.Parse(@"{""actualDecimal"": 123.45}"));
96+
97+
// Act
98+
var yamlNode = json.ToYamlNode();
99+
var yamlOutput = ConvertYamlNodeToString(yamlNode);
100+
101+
// Assert
102+
Assert.Contains("actualDecimal: 123.45", yamlOutput);
103+
Assert.DoesNotContain("\"123.45\"", yamlOutput);
104+
}
105+
106+
[Fact]
107+
public void ToYamlNode_StringThatLooksLikeBoolean_QuotedInYaml()
108+
{
109+
// Arrange
110+
var json = Assert.IsType<JsonObject>(JsonNode.Parse(@"{""boolString"": ""true""}"));
111+
112+
// Act
113+
var yamlNode = json.ToYamlNode();
114+
var yamlOutput = ConvertYamlNodeToString(yamlNode);
115+
116+
// Assert
117+
Assert.Contains("boolString: \"true\"", yamlOutput);
118+
}
119+
120+
[Fact]
121+
public void ToYamlNode_ActualBoolean_NotQuotedInYaml()
122+
{
123+
// Arrange
124+
var json = Assert.IsType<JsonObject>(JsonNode.Parse(@"{""actualBool"": true}"));
125+
126+
// Act
127+
var yamlNode = json.ToYamlNode();
128+
var yamlOutput = ConvertYamlNodeToString(yamlNode);
129+
130+
// Assert
131+
Assert.Contains("actualBool: true", yamlOutput);
132+
Assert.DoesNotContain("\"true\"", yamlOutput);
133+
}
134+
135+
[Fact]
136+
public void ToYamlNode_StringThatLooksLikeNull_QuotedInYaml()
137+
{
138+
// Arrange
139+
var json = Assert.IsType<JsonObject>(JsonNode.Parse(@"{""nullString"": ""null""}"));
140+
141+
// Act
142+
var yamlNode = json.ToYamlNode();
143+
var yamlOutput = ConvertYamlNodeToString(yamlNode);
144+
145+
// Assert
146+
Assert.Contains("nullString: \"null\"", yamlOutput);
147+
}
148+
149+
[Fact]
150+
public void ToYamlNode_MixedTypes_CorrectQuoting()
151+
{
152+
// Arrange
153+
var json = Assert.IsType<JsonObject>(JsonNode.Parse(@"{
154+
""str"": ""hello"",
155+
""numStr"": ""42"",
156+
""num"": 42,
157+
""boolStr"": ""false"",
158+
""bool"": false
159+
}"));
160+
161+
// Act
162+
var yamlNode = json.ToYamlNode();
163+
var yamlOutput = ConvertYamlNodeToString(yamlNode);
164+
165+
// Assert
166+
Assert.Contains("str: hello", yamlOutput);
167+
Assert.Contains("numStr: \"42\"", yamlOutput);
168+
Assert.Contains("num: 42", yamlOutput);
169+
Assert.DoesNotContain("num: \"42\"", yamlOutput);
170+
Assert.Contains("boolStr: \"false\"", yamlOutput);
171+
Assert.Contains("bool: false", yamlOutput);
172+
Assert.DoesNotContain("bool: \"false\"", yamlOutput);
173+
}
174+
175+
[Fact]
176+
public void ToYamlNode_FromIssueExample_CorrectOutput()
177+
{
178+
// Arrange - Example from issue #1951
179+
var json = Assert.IsType<JsonObject>(JsonNode.Parse(@"{
180+
""fooString"": ""fooStringValue"",
181+
""fooStringOfNumber"": ""200""
182+
}"));
183+
184+
// Act
185+
var yamlNode = json.ToYamlNode();
186+
var yamlOutput = ConvertYamlNodeToString(yamlNode);
187+
188+
// Assert
189+
Assert.Contains("fooString: fooStringValue", yamlOutput);
190+
Assert.Contains("fooStringOfNumber: \"200\"", yamlOutput);
191+
192+
// Ensure no extra quotes on regular strings
193+
Assert.DoesNotContain("\"fooStringValue\"", yamlOutput);
194+
Assert.DoesNotContain("'fooStringValue'", yamlOutput);
195+
}
196+
197+
[Fact]
198+
public void ToYamlNode_StringWithLineBreaks_PreservesLineBreaks()
199+
{
200+
// Arrange
201+
var json = Assert.IsType<JsonObject>(JsonNode.Parse(@"{
202+
""multiline"": ""Line 1\nLine 2\nLine 3"",
203+
""description"": ""This is a description\nwith line breaks\nin it""
204+
}"));
205+
206+
// Act
207+
var yamlNode = json.ToYamlNode();
208+
var yamlOutput = ConvertYamlNodeToString(yamlNode);
209+
210+
// Convert back to JSON to verify round-tripping
211+
var yamlStream = new YamlStream();
212+
using var sr = new StringReader(yamlOutput);
213+
yamlStream.Load(sr);
214+
var jsonBack = yamlStream.Documents[0].ToJsonNode();
215+
216+
// Assert - line breaks should be preserved during round-trip
217+
var originalMultiline = json["multiline"]?.GetValue<string>();
218+
var roundTripMultiline = jsonBack?["multiline"]?.GetValue<string>();
219+
Assert.Equal(originalMultiline, roundTripMultiline);
220+
Assert.Contains("\n", roundTripMultiline);
221+
222+
var originalDescription = json["description"]?.GetValue<string>();
223+
var roundTripDescription = jsonBack?["description"]?.GetValue<string>();
224+
Assert.Equal(originalDescription, roundTripDescription);
225+
Assert.Contains("\n", roundTripDescription);
226+
}
227+
228+
private static string ConvertYamlNodeToString(YamlNode yamlNode)
229+
{
230+
using var ms = new MemoryStream();
231+
var yamlStream = new YamlStream(new YamlDocument(yamlNode));
232+
var writer = new StreamWriter(ms);
233+
yamlStream.Save(writer);
234+
writer.Flush();
235+
ms.Seek(0, SeekOrigin.Begin);
236+
var reader = new StreamReader(ms);
237+
return reader.ReadToEnd();
238+
}
29239
}

0 commit comments

Comments
 (0)