Add tests for GameVersionConverter and fix edge case (#1726)

- Adds tests for GameVersionConverter
- Refactors GameVersionConverter to reduce nesting
- Fixes an edge case in GameVersion deserialization in which the JsonConstructor will be invoked even if no properties match
- Adds a test for the GameVersion deserialization edge case

Dalamud.Common/Game/GameVersion.cs

@@ -109,26 +109,31 @@ public sealed class GameVersion : ICloneable, IComparable, IComparable<GameVersi
     /// <summary>
     /// Gets the year component.
     /// </summary>
+    [JsonRequired]
     public int Year { get; } = -1;
 
     /// <summary>
     /// Gets the month component.
     /// </summary>
+    [JsonRequired]
     public int Month { get; } = -1;
 
     /// <summary>
     /// Gets the day component.
     /// </summary>
+    [JsonRequired]
     public int Day { get; } = -1;
 
     /// <summary>
     /// Gets the major version component.
     /// </summary>
+    [JsonRequired]
     public int Major { get; } = -1;
 
     /// <summary>
     /// Gets the minor version component.
     /// </summary>
+    [JsonRequired]
     public int Minor { get; } = -1;
 
     public static implicit operator GameVersion(string ver)

Dalamud.Common/Game/GameVersionConverter.cs

@@ -15,17 +15,16 @@ public sealed class GameVersionConverter : JsonConverter
     /// <param name="serializer">The calling serializer.</param>
     public override void WriteJson(JsonWriter writer, object? value, JsonSerializer serializer)
     {
-        if (value == null)
+        switch (value)
         {
-            writer.WriteNull();
+            case null:
-        }
+                writer.WriteNull();
-        else if (value is GameVersion)
+                break;
-        {
+            case GameVersion:
-            writer.WriteValue(value.ToString());
+                writer.WriteValue(value.ToString());
-        }
+                break;
-        else
+            default:
-        {
+                throw new JsonSerializationException("Expected GameVersion object value");
-            throw new JsonSerializationException("Expected GameVersion object value");
         }
     }
 
@@ -43,24 +42,20 @@ public sealed class GameVersionConverter : JsonConverter
         {
             return null;
         }
-        else
+
+        if (reader.TokenType == JsonToken.String)
         {
-            if (reader.TokenType == JsonToken.String)
+            try
             {
-                try
+                return new GameVersion((string)reader.Value!);
-                {
-                    return new GameVersion((string)reader.Value!);
-                }
-                catch (Exception ex)
-                {
-                    throw new JsonSerializationException($"Error parsing GameVersion string: {reader.Value}", ex);
-                }
             }
-            else
+            catch (Exception ex)
             {
-                throw new JsonSerializationException($"Unexpected token or value when parsing GameVersion. Token: {reader.TokenType}, Value: {reader.Value}");
+                throw new JsonSerializationException($"Error parsing GameVersion string: {reader.Value}", ex);
             }
         }
+
+        throw new JsonSerializationException($"Unexpected token or value when parsing GameVersion. Token: {reader.TokenType}, Value: {reader.Value}");
     }
 
     /// <summary>

Dalamud.Test/Game/GameVersionConverterTests.cs

@@ -0,0 +1,138 @@
+using Dalamud.Common.Game;
+
+using JetBrains.Annotations;
+
+using Newtonsoft.Json;
+
+using Xunit;
+
+namespace Dalamud.Test.Game;
+
+public class GameVersionConverterTests
+{
+    [Fact]
+    public void ReadJson_ConvertsFromString()
+    {
+        var serialized = """
+                         {
+                            "Version": "2020.06.15.0000.0000"
+                         }
+                         """;
+        var deserialized = JsonConvert.DeserializeObject<TestSerializationClass>(serialized);
+
+        Assert.NotNull(deserialized);
+        Assert.Equal(GameVersion.Parse("2020.06.15.0000.0000"), deserialized.Version);
+    }
+
+
+    [Fact]
+    public void ReadJson_ConvertsFromNull()
+    {
+        var serialized = """
+                         {
+                            "Version": null
+                         }
+                         """;
+        var deserialized = JsonConvert.DeserializeObject<TestSerializationClass>(serialized);
+
+        Assert.NotNull(deserialized);
+        Assert.Null(deserialized.Version);
+    }
+
+    [Fact]
+    public void ReadJson_WhenInvalidType_Throws()
+    {
+        var serialized = """
+                         {
+                            "Version": 2
+                         }
+                         """;
+        Assert.Throws<JsonSerializationException>(
+            () => JsonConvert.DeserializeObject<TestSerializationClass>(serialized));
+    }
+
+    [Fact]
+    public void ReadJson_WhenInvalidVersion_Throws()
+    {
+        var serialized = """
+                         {
+                            "Version": "junk"
+                         }
+                         """;
+        Assert.Throws<JsonSerializationException>(
+            () => JsonConvert.DeserializeObject<TestSerializationClass>(serialized));
+    }
+
+    [Fact]
+    public void WriteJson_ConvertsToString()
+    {
+        var deserialized = new TestSerializationClass
+        {
+            Version = GameVersion.Parse("2020.06.15.0000.0000"),
+        };
+        var serialized = JsonConvert.SerializeObject(deserialized);
+
+        Assert.Equal("""{"Version":"2020.06.15.0000.0000"}""", RemoveWhitespace(serialized));
+    }
+
+    [Fact]
+    public void WriteJson_ConvertsToNull()
+    {
+        var deserialized = new TestSerializationClass
+        {
+            Version = null,
+        };
+        var serialized = JsonConvert.SerializeObject(deserialized);
+
+        Assert.Equal("""{"Version":null}""", RemoveWhitespace(serialized));
+    }
+
+    [Fact]
+    public void WriteJson_WhenInvalidVersion_Throws()
+    {
+        var deserialized = new TestWrongTypeSerializationClass
+        {
+            Version = 42,
+        };
+        Assert.Throws<JsonSerializationException>(() => JsonConvert.SerializeObject(deserialized));
+    }
+
+    [Fact]
+    public void CanConvert_WhenGameVersion_ReturnsTrue()
+    {
+        var converter = new GameVersionConverter();
+        Assert.True(converter.CanConvert(typeof(GameVersion)));
+    }
+
+    [Fact]
+    public void CanConvert_WhenNotGameVersion_ReturnsFalse()
+    {
+        var converter = new GameVersionConverter();
+        Assert.False(converter.CanConvert(typeof(int)));
+    }
+
+    [Fact]
+    public void CanConvert_WhenNull_ReturnsFalse()
+    {
+        var converter = new GameVersionConverter();
+        Assert.False(converter.CanConvert(null!));
+    }
+
+    private static string RemoveWhitespace(string input)
+    {
+        return input.Replace(" ", "").Replace("\r", "").Replace("\n", "");
+    }
+
+    private class TestSerializationClass
+    {
+        [JsonConverter(typeof(GameVersionConverter))]
+        [CanBeNull]
+        public GameVersion Version { get; init; }
+    }
+
+    private class TestWrongTypeSerializationClass
+    {
+        [JsonConverter(typeof(GameVersionConverter))]
+        public int Version { get; init; }
+    }
+}

Dalamud.Test/Game/GameVersionTests.cs

@@ -225,6 +225,17 @@ namespace Dalamud.Test.Game
             Assert.Throws<ArgumentOutOfRangeException>(() => JsonConvert.DeserializeObject<GameVersion>(serialized));
         }
 
+        [Fact]
+        public void VersionInvalidTypeDeserialization()
+        {
+            var serialized = """
+                             {
+                                "Value": "Hello"
+                             }
+                             """;
+            Assert.Throws<JsonSerializationException>(() => JsonConvert.DeserializeObject<GameVersion>(serialized));
+        }
+
         [Fact]
         public void VersionConstructorNegativeYear()
         {