Skip to content

Commit 6913cb1

Browse files
yotsudayotsuda
committed
Address code review feedback: use custom converters and simplify SerializePrimitive
1 parent 385cf51 commit 6913cb1

File tree

2 files changed

+96
-66
lines changed

2 files changed

+96
-66
lines changed

‎src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/ConvertToJsonCommandV2.cs‎

Lines changed: 72 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -204,9 +204,11 @@ internal static class SystemTextJsonSerializer
204204
}
205205

206206
options.Converters.Add(new JsonConverterBigInteger());
207+
options.Converters.Add(new JsonConverterDouble());
208+
options.Converters.Add(new JsonConverterFloat());
207209
options.Converters.Add(new JsonConverterNullString());
208210
options.Converters.Add(new JsonConverterDBNull());
209-
options.Converters.Add(new JsonConverterPSObject(cmdlet, maxDepth));
211+
options.Converters.Add(new JsonConverterPSObject(cmdlet, maxDepth, basePropertiesOnly: false));
210212
options.Converters.Add(new JsonConverterRawObject(cmdlet, maxDepth));
211213
options.Converters.Add(new JsonConverterJObject());
212214

@@ -248,7 +250,7 @@ internal sealed class JsonConverterPSObject : System.Text.Json.Serialization.Jso
248250

249251
private bool _warningWritten;
250252

251-
public JsonConverterPSObject(PSCmdlet? cmdlet, int maxDepth, bool basePropertiesOnly = false)
253+
public JsonConverterPSObject(PSCmdlet? cmdlet, int maxDepth, bool basePropertiesOnly)
252254
{
253255
_cmdlet = cmdlet;
254256
_maxDepth = maxDepth;
@@ -599,6 +601,68 @@ public override void Write(Utf8JsonWriter writer, BigInteger value, JsonSerializ
599601
}
600602
}
601603

604+
/// <summary>
605+
/// JsonConverter for double to serialize Infinity and NaN as strings for V1 compatibility.
606+
/// </summary>
607+
internal sealed class JsonConverterDouble : System.Text.Json.Serialization.JsonConverter<double>
608+
{
609+
public override double Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
610+
{
611+
throw new NotImplementedException();
612+
}
613+
614+
public override void Write(Utf8JsonWriter writer, double value, JsonSerializerOptions options)
615+
{
616+
if (double.IsPositiveInfinity(value))
617+
{
618+
writer.WriteStringValue("Infinity");
619+
}
620+
else if (double.IsNegativeInfinity(value))
621+
{
622+
writer.WriteStringValue("-Infinity");
623+
}
624+
else if (double.IsNaN(value))
625+
{
626+
writer.WriteStringValue("NaN");
627+
}
628+
else
629+
{
630+
writer.WriteNumberValue(value);
631+
}
632+
}
633+
}
634+
635+
/// <summary>
636+
/// JsonConverter for float to serialize Infinity and NaN as strings for V1 compatibility.
637+
/// </summary>
638+
internal sealed class JsonConverterFloat : System.Text.Json.Serialization.JsonConverter<float>
639+
{
640+
public override float Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
641+
{
642+
throw new NotImplementedException();
643+
}
644+
645+
public override void Write(Utf8JsonWriter writer, float value, JsonSerializerOptions options)
646+
{
647+
if (float.IsPositiveInfinity(value))
648+
{
649+
writer.WriteStringValue("Infinity");
650+
}
651+
else if (float.IsNegativeInfinity(value))
652+
{
653+
writer.WriteStringValue("-Infinity");
654+
}
655+
else if (float.IsNaN(value))
656+
{
657+
writer.WriteStringValue("NaN");
658+
}
659+
else
660+
{
661+
writer.WriteNumberValue(value);
662+
}
663+
}
664+
}
665+
602666
/// <summary>
603667
/// Custom JsonConverter for Newtonsoft.Json.Linq.JObject to isolate Newtonsoft-related code.
604668
/// </summary>
@@ -638,16 +702,13 @@ private static void WriteJTokenValue(Utf8JsonWriter writer, Newtonsoft.Json.Linq
638702

639703
/// <summary>
640704
/// Wrapper class for raw .NET objects to distinguish them from PSObjects at the type level.
641-
/// This enables separate JsonConverter handling for raw objects (Base properties only).
705+
/// Inherits from PSObject to avoid rewrapping when passed to JsonConverterPSObject.
642706
/// </summary>
643-
internal sealed class RawObjectWrapper
707+
internal sealed class RawObjectWrapper : PSObject
644708
{
645-
public RawObjectWrapper(object value)
709+
public RawObjectWrapper(object value) : base(value)
646710
{
647-
Value = value;
648711
}
649-
650-
public object Value { get; }
651712
}
652713

653714
/// <summary>
@@ -673,8 +734,8 @@ public JsonConverterRawObject(PSCmdlet? cmdlet, int maxDepth)
673734

674735
public override void Write(Utf8JsonWriter writer, RawObjectWrapper wrapper, JsonSerializerOptions options)
675736
{
676-
var pso = PSObject.AsPSObject(wrapper.Value);
677-
_psoConverter.Write(writer, pso, options);
737+
// RawObjectWrapper inherits from PSObject, so no rewrapping needed
738+
_psoConverter.Write(writer, wrapper, options);
678739
}
679740
}
680741

@@ -727,53 +788,7 @@ public static bool IsStjNativeScalarType(object obj)
727788

728789
public static void SerializePrimitive(Utf8JsonWriter writer, object obj, JsonSerializerOptions options)
729790
{
730-
// Handle special floating-point values (Infinity, NaN) as strings for V1 compatibility
731-
if (obj is double d)
732-
{
733-
if (double.IsPositiveInfinity(d))
734-
{
735-
writer.WriteStringValue("Infinity");
736-
return;
737-
}
738-
739-
if (double.IsNegativeInfinity(d))
740-
{
741-
writer.WriteStringValue("-Infinity");
742-
return;
743-
}
744-
745-
if (double.IsNaN(d))
746-
{
747-
writer.WriteStringValue("NaN");
748-
return;
749-
}
750-
}
751-
else if (obj is float f)
752-
{
753-
if (float.IsPositiveInfinity(f))
754-
{
755-
writer.WriteStringValue("Infinity");
756-
return;
757-
}
758-
759-
if (float.IsNegativeInfinity(f))
760-
{
761-
writer.WriteStringValue("-Infinity");
762-
return;
763-
}
764-
765-
if (float.IsNaN(f))
766-
{
767-
writer.WriteStringValue("NaN");
768-
return;
769-
}
770-
}
771-
else if (obj is BigInteger bi)
772-
{
773-
writer.WriteRawValue(bi.ToString(CultureInfo.InvariantCulture));
774-
return;
775-
}
776-
791+
// Custom converters handle special cases (BigInteger, double/float Infinity/NaN)
777792
System.Text.Json.JsonSerializer.Serialize(writer, obj, obj.GetType(), options);
778793
}
779794

‎test/powershell/Modules/Microsoft.PowerShell.Utility/ConvertTo-Json.PSJsonSerializerV2.Tests.ps1‎

Lines changed: 24 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,37 @@
11
# Copyright (c) Microsoft Corporation.
22
# Licensed under the MIT License.
33

4-
BeforeDiscovery {
5-
$isV2Enabled = $EnabledExperimentalFeatures.Contains('PSJsonSerializerV2')
6-
}
4+
Describe 'ConvertTo-Json PSJsonSerializerV2 specific behavior' -Tags "CI" {
5+
6+
BeforeAll {
7+
$skipTest = -not $EnabledExperimentalFeatures.Contains('PSJsonSerializerV2')
8+
9+
if ($skipTest) {
10+
Write-Verbose "Test Suite Skipped. The test suite requires the experimental feature 'PSJsonSerializerV2' to be enabled." -Verbose
11+
$originalDefaultParameterValues = $PSDefaultParameterValues.Clone()
12+
$PSDefaultParameterValues["it:skip"] = $true
13+
}
14+
}
15+
16+
AfterAll {
17+
if ($skipTest) {
18+
$global:PSDefaultParameterValues = $originalDefaultParameterValues
19+
}
20+
}
721

8-
Describe 'ConvertTo-Json PSJsonSerializerV2 specific behavior' -Tags "CI" -Skip:(-not $isV2Enabled) {
922
It 'Should serialize dictionary with integer keys' {
1023
$dict = @{ 1 = "one"; 2 = "two" }
1124
$json = $dict | ConvertTo-Json -Compress
1225
$json | Should -BeIn @('{"1":"one","2":"two"}', '{"2":"two","1":"one"}')
1326
}
1427

15-
It 'Should serialize Exception.Data with non-string keys' {
16-
$ex = [System.Exception]::new("test")
17-
$ex.Data.Add(1, "value1")
18-
$ex.Data.Add("key", "value2")
19-
{ $ex | ConvertTo-Json -Depth 1 } | Should -Not -Throw
28+
It 'Should serialize dictionary with non-string keys converting keys to strings' {
29+
$dict = [System.Collections.Hashtable]::new()
30+
$dict.Add(1, "one")
31+
$dict.Add([guid]"12345678-1234-1234-1234-123456789abc", "guid-value")
32+
$json = $dict | ConvertTo-Json -Compress
33+
$json | Should -Match '"1":\s*"one"'
34+
$json | Should -Match '"12345678-1234-1234-1234-123456789abc":\s*"guid-value"'
2035
}
2136

2237
It 'Should not serialize hidden properties in PowerShell class' {

0 commit comments

Comments
 (0)