Use json converters collection instead of property converters (#423)

helenkzhang · web-flow · commit 73f3221ad4e6 · 2025-04-04T10:57:11.000-07:00
* Fix serialization

* Remove white space

* Fix namespace ordering

* Add tests

* Address comments
diff --git a/src/PluginsSystem/Microsoft.Performance.Toolkit.Plugins.Core.Tests/Microsoft.Performance.Toolkit.Plugins.Core.Tests.csproj b/src/PluginsSystem/Microsoft.Performance.Toolkit.Plugins.Core.Tests/Microsoft.Performance.Toolkit.Plugins.Core.Tests.csproj
@@ -9,6 +9,7 @@
   </PropertyGroup>
 
   <ItemGroup>
+    <PackageReference Include="AutoFixture" Version="4.18.1" />
     <PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.1.0" />
     <PackageReference Include="Moq" Version="4.18.4" />
     <PackageReference Include="MSTest.TestAdapter" Version="2.2.8" />
diff --git a/src/PluginsSystem/Microsoft.Performance.Toolkit.Plugins.Core.Tests/PluginMetadataSerializationTests.cs b/src/PluginsSystem/Microsoft.Performance.Toolkit.Plugins.Core.Tests/PluginMetadataSerializationTests.cs
@@ -0,0 +1,32 @@
+﻿// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT License.
+
+using Microsoft.Performance.Toolkit.Plugins.Core.Metadata;
+using Microsoft.VisualStudio.TestTools.UnitTesting;
+using Fixture = AutoFixture.Fixture;
+
+namespace Microsoft.Performance.Toolkit.Plugins.Core.Tests;
+
+[TestClass]
+public sealed class PluginMetadataSerializationTests
+{
+    private Fixture? fixture = null;
+
+    [TestInitialize]
+    public void Setup()
+    {
+        this.fixture = new Fixture();
+    }
+
+    [TestMethod]
+    public void PluginMetadata_Serialization_RoundTrip()
+    {
+        SerializationTestsHelper.RunSerializationTest<PluginMetadata>(this.fixture!);
+    }
+
+    [TestMethod]
+    public void PluginContentsMetadata_Serialization_RoundTrip()
+    {
+        SerializationTestsHelper.RunSerializationTest<PluginContentsMetadata>(this.fixture!);
+    }
+}
diff --git a/src/PluginsSystem/Microsoft.Performance.Toolkit.Plugins.Core.Tests/SerializationTestsHelper.cs b/src/PluginsSystem/Microsoft.Performance.Toolkit.Plugins.Core.Tests/SerializationTestsHelper.cs
@@ -0,0 +1,28 @@
+﻿// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT License.
+
+using AutoFixture;
+using Microsoft.Performance.Toolkit.Plugins.Core.Serialization;
+using Microsoft.VisualStudio.TestTools.UnitTesting;
+
+namespace Microsoft.Performance.Toolkit.Plugins.Core.Tests;
+
+public class SerializationTestsHelper
+{
+    public static void RunSerializationTest<T>(Fixture fixture) where T : class
+    {
+        ISerializer<T> serializer = SerializationUtils.GetJsonSerializerWithDefaultOptions<T>();
+
+        T original = fixture.Create<T>();
+
+        using var stream = new MemoryStream();
+        serializer.Serialize(stream, original);
+
+        stream.Position = 0;
+
+        T deserialized = serializer.Deserialize(stream);
+
+        Assert.IsNotNull(deserialized);
+        Assert.IsTrue(original.Equals(deserialized));
+    }
+}
diff --git a/src/PluginsSystem/Microsoft.Performance.Toolkit.Plugins.Core/EnumerableExtensions.cs b/src/PluginsSystem/Microsoft.Performance.Toolkit.Plugins.Core/EnumerableExtensions.cs
@@ -11,7 +11,7 @@ namespace Microsoft.Performance.Toolkit.Plugins.Core
     /// <summary>
     ///     Provides extension methods for <see cref="IEnumerable{T}"/>.
     /// </summary>
-    internal static class EnumerableExtensions
+    public static class EnumerableExtensions
     {
         /// <summary>
         ///     Determines whether two sequences are equal by comparing their elements by using the default equality comparer for their type.
diff --git a/src/PluginsSystem/Microsoft.Performance.Toolkit.Plugins.Core/Metadata/PluginMetadata.cs b/src/PluginsSystem/Microsoft.Performance.Toolkit.Plugins.Core/Metadata/PluginMetadata.cs
@@ -5,7 +5,6 @@
 using System.Collections.Generic;
 using System.Text.Json.Serialization;
 using Microsoft.Performance.SDK;
-using Microsoft.Performance.Toolkit.Plugins.Core.Serialization;
 using NuGet.Versioning;
 
 namespace Microsoft.Performance.Toolkit.Plugins.Core.Metadata
@@ -87,7 +86,6 @@ public PluginMetadata(
         /// <summary>
         ///     Gets the version of the performance SDK which this plugin depends upon.
         /// </summary>
-        [JsonConverter(typeof(SemanticVersionJsonConverter))]
         public SemanticVersion SdkVersion { get; }
 
         /// <summary>
diff --git a/src/PluginsSystem/Microsoft.Performance.Toolkit.Plugins.Core/PluginIdentity.cs b/src/PluginsSystem/Microsoft.Performance.Toolkit.Plugins.Core/PluginIdentity.cs
@@ -36,7 +36,7 @@ public PluginIdentity(string id, PluginVersion version)
         }
 
         /// <summary>
-        ///     Gets the identifer of this plugin.
+        ///     Gets the identifier of this plugin.
         /// </summary>
         public string Id { get; }
 
diff --git a/src/PluginsSystem/Microsoft.Performance.Toolkit.Plugins.Core/Serialization/PluginVersionJsonConverter.cs b/src/PluginsSystem/Microsoft.Performance.Toolkit.Plugins.Core/Serialization/PluginVersionJsonConverter.cs
@@ -7,13 +7,19 @@
 
 namespace Microsoft.Performance.Toolkit.Plugins.Core.Serialization
 {
+    /// <summary>
+    ///     Converts a <see cref="PluginVersion"/> to and from JSON.
+    /// </summary>
     public class PluginVersionJsonConverter : JsonConverter<PluginVersion>
     {
+        /// <inheritdoc/>
         public override PluginVersion Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
         {
             var versionString = reader.GetString();
             return versionString is null ? null : PluginVersion.Parse(versionString);
         }
+
+        /// <inheritdoc/>
         public override void Write(Utf8JsonWriter writer, PluginVersion value, JsonSerializerOptions options)
         {
             writer.WriteStringValue(value.ToString());
diff --git a/src/PluginsSystem/Microsoft.Performance.Toolkit.Plugins.Core/Serialization/SemanticVersionConverter.cs b/src/PluginsSystem/Microsoft.Performance.Toolkit.Plugins.Core/Serialization/SemanticVersionConverter.cs
@@ -8,14 +8,19 @@
 
 namespace Microsoft.Performance.Toolkit.Plugins.Core.Serialization
 {
+    /// <summary>
+    ///     Converts a <see cref="SemanticVersion"/> to and from JSON.
+    /// </summary>
     public class SemanticVersionJsonConverter : JsonConverter<SemanticVersion>
     {
+        /// <inheritdoc/>
         public override SemanticVersion Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
         {
             var versionString = reader.GetString();
             return versionString is null ? null : SemanticVersion.Parse(versionString);
         }
 
+        /// <inheritdoc/>
         public override void Write(Utf8JsonWriter writer, SemanticVersion value, JsonSerializerOptions options)
         {
             writer.WriteStringValue(value.ToString());
diff --git a/src/PluginsSystem/Microsoft.Performance.Toolkit.Plugins.Core/Serialization/SerializationUtils.cs b/src/PluginsSystem/Microsoft.Performance.Toolkit.Plugins.Core/Serialization/SerializationUtils.cs
@@ -54,28 +54,9 @@ public static ISerializer<T> GetJsonSerializer<T>(JsonSerializerOptions serializ
             Converters =
             {
                 new JsonStringEnumConverter(),
-                new VersionStringConverter(),
+                new PluginVersionJsonConverter(),
+                new SemanticVersionJsonConverter(),
             }
         };
-
-        internal class VersionStringConverter
-            : JsonConverter<Version>
-        {
-            public override Version Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
-            {
-                if (reader.TokenType == JsonTokenType.String)
-                {
-                    string stringValue = reader.GetString();
-                    return new Version(stringValue);
-                }
-
-                throw new JsonException();
-            }
-
-            public override void Write(Utf8JsonWriter writer, Version value, JsonSerializerOptions options)
-            {
-                writer.WriteStringValue(value.ToString());
-            }
-        }
     }
 }
diff --git a/src/PluginsSystem/Tools/Microsoft.Performance.Toolkit.Plugins.Cli.Tests/Microsoft.Performance.Toolkit.Plugins.Cli.Tests.csproj b/src/PluginsSystem/Tools/Microsoft.Performance.Toolkit.Plugins.Cli.Tests/Microsoft.Performance.Toolkit.Plugins.Cli.Tests.csproj
@@ -0,0 +1,28 @@
+<Project Sdk="Microsoft.NET.Sdk">
+
+  <PropertyGroup>
+    <TargetFramework>net8.0</TargetFramework>
+    <ImplicitUsings>enable</ImplicitUsings>
+    <Nullable>enable</Nullable>
+  </PropertyGroup>
+
+  <ItemGroup>
+	<PackageReference Include="AutoFixture" Version="4.18.1" />
+    <PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.12.0" />
+    <PackageReference Include="MSTest.TestAdapter" Version="2.2.8" />
+    <PackageReference Include="MSTest.TestFramework" Version="2.2.8" />
+    <PackageReference Include="coverlet.collector" Version="3.1.2" />
+  </ItemGroup>
+
+  <ItemGroup>
+    <ProjectReference Include="..\..\..\Microsoft.Performance.SDK\Microsoft.Performance.SDK.csproj" />
+    <ProjectReference Include="..\..\Microsoft.Performance.Toolkit.Plugins.Core.Tests\Microsoft.Performance.Toolkit.Plugins.Core.Tests.csproj" />
+    <ProjectReference Include="..\..\Microsoft.Performance.Toolkit.Plugins.Core\Microsoft.Performance.Toolkit.Plugins.Core.csproj" />
+    <ProjectReference Include="..\Microsoft.Performance.Toolkit.Plugins.Cli\Microsoft.Performance.Toolkit.Plugins.Cli.csproj" />
+  </ItemGroup>
+
+  <ItemGroup>
+    <Using Include="Microsoft.VisualStudio.TestTools.UnitTesting" />
+  </ItemGroup>
+
+</Project>
diff --git a/src/PluginsSystem/Tools/Microsoft.Performance.Toolkit.Plugins.Cli.Tests/PluginManifestSerializationTests.cs b/src/PluginsSystem/Tools/Microsoft.Performance.Toolkit.Plugins.Cli.Tests/PluginManifestSerializationTests.cs
@@ -0,0 +1,26 @@
+﻿// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT License.
+
+using Microsoft.Performance.Toolkit.Plugins.Cli.Manifest;
+using Microsoft.Performance.Toolkit.Plugins.Core.Tests;
+using Fixture = AutoFixture.Fixture;
+
+namespace Microsoft.Performance.Toolkit.Plugins.Cli.Tests;
+
+[TestClass]
+public sealed class PluginManifestSerializationTest
+{
+    private Fixture? fixture = null;
+
+    [TestInitialize]
+    public void Setup()
+    {
+        this.fixture = new Fixture();
+    }
+
+    [TestMethod]
+    public void PluginManifest_Serialization_RoundTrip()
+    {
+        SerializationTestsHelper.RunSerializationTest<PluginManifest>(this.fixture!);
+    }
+}
diff --git a/src/PluginsSystem/Tools/Microsoft.Performance.Toolkit.Plugins.Cli/Manifest/PluginIdentityManifest.cs b/src/PluginsSystem/Tools/Microsoft.Performance.Toolkit.Plugins.Cli/Manifest/PluginIdentityManifest.cs
@@ -1,16 +1,16 @@
 ﻿// Copyright (c) Microsoft Corporation.
 // Licensed under the MIT License.
 
+using Microsoft.Performance.SDK;
 using Microsoft.Performance.Toolkit.Plugins.Core;
-using Microsoft.Performance.Toolkit.Plugins.Core.Serialization;
-using System.Text.Json.Serialization;
 
 namespace Microsoft.Performance.Toolkit.Plugins.Cli.Manifest
 {
     /// <summary>
     ///     Represents the identity of a plugin in a manifest.
     /// </summary>
     public sealed class PluginIdentityManifest
+        : IEquatable<PluginIdentityManifest>
     {
         /// <summary>
         ///     Initializes a new instance of the <see cref="PluginIdentityManifest"/>
@@ -25,6 +25,9 @@ public PluginIdentityManifest(
             string id,
             PluginVersion version)
         {
+            Guard.NotNull(id, nameof(id));
+            Guard.NotNull(version, nameof(version));
+
             this.Id = id;
             this.Version = version;
         }
@@ -37,7 +40,55 @@ public PluginIdentityManifest(
         /// <summary>
         ///     Gets or sets the version of this plugin.
         /// </summary>
-        [JsonConverter(typeof(PluginVersionJsonConverter))]
         public PluginVersion Version { get; }
+
+        /// <inheritdoc />
+        public override bool Equals(object? obj)
+        {
+            return Equals(obj as PluginIdentityManifest);
+        }
+
+        /// <inheritdoc />
+        public bool Equals(PluginIdentityManifest? other)
+        {
+            return Equals(this, other);
+        }
+
+        /// <summary>
+        ///     Determines whether the specified <see cref="PluginIdentityManifest"/> instances are considered equal.
+        /// </summary>
+        /// <param name="a">
+        ///     The first plugin identity to compare.  
+        /// </param>
+        /// <param name="b">
+        ///     The second plugin identity to compare.
+        /// </param>
+        /// <returns>
+        ///     <c>true</c> if a and b are considered equal; <c>false</c> otherwise.
+        ///     If a or b is null, the method returns <c>false</c>.
+        /// </returns>
+        public static bool Equals(PluginIdentityManifest? a, PluginIdentityManifest? b)
+        {
+            if (ReferenceEquals(a, b))
+            {
+                return true;
+            }
+
+            if (a is null || b is null)
+            {
+                return false;
+            }
+
+            return string.Equals(a.Id, b.Id, StringComparison.Ordinal) &&
+                PluginVersion.Equals(a.Version, b.Version);
+        }
+
+        /// <inheritdoc />
+        public override int GetHashCode()
+        {
+            return HashCodeUtils.CombineHashCodeValues(
+               this.Id.GetHashCode(),
+               this.Version.GetHashCode());
+        }
     }
 }
diff --git a/src/PluginsSystem/Tools/Microsoft.Performance.Toolkit.Plugins.Cli/Manifest/PluginManifest.cs b/src/PluginsSystem/Tools/Microsoft.Performance.Toolkit.Plugins.Cli/Manifest/PluginManifest.cs
@@ -1,12 +1,17 @@
 ﻿// Copyright (c) Microsoft Corporation.
 // Licensed under the MIT License.
 
+using Microsoft.Performance.Toolkit.Plugins.Core;
+using Microsoft.Performance.SDK;
+using System.Text.Json.Serialization;
+
 namespace Microsoft.Performance.Toolkit.Plugins.Cli.Manifest
 {
     /// <summary>
     ///     Represents the manifest for a plugin.
     /// </summary>
     public sealed class PluginManifest
+        : IEquatable<PluginManifest>
     {
         /// <summary>
         ///     Initializes a new instance of the <see cref="PluginManifest"/>s
@@ -26,6 +31,7 @@ public sealed class PluginManifest
         /// <param name="projectUrl">
         ///     The URL of the project that owns this plugin.
         /// </param>
+        [JsonConstructor]
         public PluginManifest(
             PluginIdentityManifest identity,
             string displayName,
@@ -65,5 +71,51 @@ public PluginManifest(
         ///     Gets the URL of the project that owns this plugin.
         /// </summary>
         public Uri ProjectUrl { get; }
+
+        /// <inheritdoc />
+        public override bool Equals(object? obj)
+        {
+            return Equals(obj as PluginManifest);
+        }
+
+        /// <inheritdoc />
+        public bool Equals(PluginManifest? other)
+        {
+            if (other is null)
+            {
+                return false;
+            }
+
+            if (ReferenceEquals(this, other))
+            {
+                return true;
+            }
+
+            return this.Identity.Equals(other.Identity)
+                && this.DisplayName == other.DisplayName
+                && this.Description == other.Description
+                && this.Owners.EnumerableEqual(other.Owners)
+                && this.ProjectUrl == other.ProjectUrl;
+        }
+
+        /// <inheritdoc />
+        public override int GetHashCode()
+        {
+            int result = HashCodeUtils.CombineHashCodeValues(
+                this.Identity?.GetHashCode() ?? 0,
+                this.DisplayName?.GetHashCode() ?? 0,
+                this.Description?.GetHashCode() ?? 0,
+                this.ProjectUrl?.GetHashCode() ?? 0);
+
+            if (this.Owners != null)
+            {
+                foreach (PluginOwnerInfoManifest owner in this.Owners)
+                {
+                    result = HashCodeUtils.CombineHashCodeValues(result, owner?.GetHashCode() ?? 0);
+                }
+            }
+
+            return result;
+        }
     }
 }
diff --git a/src/PluginsSystem/Tools/Microsoft.Performance.Toolkit.Plugins.Cli/Manifest/PluginOwnerInfoManifest.cs b/src/PluginsSystem/Tools/Microsoft.Performance.Toolkit.Plugins.Cli/Manifest/PluginOwnerInfoManifest.cs
diff --git a/src/PluginsSystem/Tools/Microsoft.Performance.Toolkit.Plugins.Cli/Program.cs b/src/PluginsSystem/Tools/Microsoft.Performance.Toolkit.Plugins.Cli/Program.cs
diff --git a/src/SDK.sln b/src/SDK.sln

Original file line number	Diff line number	Diff line change
`@@ -11,7 +11,7 @@ namespace Microsoft.Performance.Toolkit.Plugins.Core`
`11`	`11`	`/// <summary>`
`12`	`12`	`/// Provides extension methods for <see cref="IEnumerable{T}"/>.`
`13`	`13`	`/// </summary>`
`14`		`- internal static class EnumerableExtensions`
	`14`	`+ public static class EnumerableExtensions`
`15`	`15`	`{`
`16`	`16`	`/// <summary>`
`17`	`17`	`/// Determines whether two sequences are equal by comparing their elements by using the default equality comparer for their type.`
Original file line number	Diff line number	Diff line change
`@@ -36,7 +36,7 @@ public PluginIdentity(string id, PluginVersion version)`
`36`	`36`	`}`
`37`	`37`
`38`	`38`	`/// <summary>`
`39`		`- /// Gets the identifer of this plugin.`
	`39`	`+ /// Gets the identifier of this plugin.`
`40`	`40`	`/// </summary>`
`41`	`41`	`public string Id { get; }`
`42`	`42`
Original file line number	Diff line number	Diff line change
`@@ -7,13 +7,19 @@`
`7`	`7`
`8`	`8`	`namespace Microsoft.Performance.Toolkit.Plugins.Core.Serialization`
`9`	`9`	`{`
	`10`	`+ /// <summary>`
	`11`	`+ /// Converts a <see cref="PluginVersion"/> to and from JSON.`
	`12`	`+ /// </summary>`
`10`	`13`	`public class PluginVersionJsonConverter : JsonConverter<PluginVersion>`
`11`	`14`	`{`
	`15`	`+ /// <inheritdoc/>`
`12`	`16`	`public override PluginVersion Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)`
`13`	`17`	`{`
`14`	`18`	`var versionString = reader.GetString();`
`15`	`19`	`return versionString is null ? null : PluginVersion.Parse(versionString);`
`16`	`20`	`}`
	`21`	`+`
	`22`	`+ /// <inheritdoc/>`
`17`	`23`	`public override void Write(Utf8JsonWriter writer, PluginVersion value, JsonSerializerOptions options)`
`18`	`24`	`{`
`19`	`25`	`writer.WriteStringValue(value.ToString());`
Original file line number	Diff line number	Diff line change
`@@ -8,14 +8,19 @@`
`8`	`8`
`9`	`9`	`namespace Microsoft.Performance.Toolkit.Plugins.Core.Serialization`
`10`	`10`	`{`
	`11`	`+ /// <summary>`
	`12`	`+ /// Converts a <see cref="SemanticVersion"/> to and from JSON.`
	`13`	`+ /// </summary>`
`11`	`14`	`public class SemanticVersionJsonConverter : JsonConverter<SemanticVersion>`
`12`	`15`	`{`
	`16`	`+ /// <inheritdoc/>`
`13`	`17`	`public override SemanticVersion Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)`
`14`	`18`	`{`
`15`	`19`	`var versionString = reader.GetString();`
`16`	`20`	`return versionString is null ? null : SemanticVersion.Parse(versionString);`
`17`	`21`	`}`
`18`	`22`
	`23`	`+ /// <inheritdoc/>`
`19`	`24`	`public override void Write(Utf8JsonWriter writer, SemanticVersion value, JsonSerializerOptions options)`
`20`	`25`	`{`
`21`	`26`	`writer.WriteStringValue(value.ToString());`