Report a missing or blank Command Id or Name in JSON as a JsonException [patch]

Keybinding.Test/CommandJsonValidationTests.cs

@@ -0,0 +1,36 @@
+// Copyright (c) 2023-2026 ktsu-dev contributors
+
+namespace ktsu.Keybinding.Test;
+
+using System.Text.Json;
+using ktsu.Keybinding.Core.Models;
+
+[TestClass]
+public class CommandJsonValidationTests
+{
+	[TestMethod]
+	[DataRow("""{"Name":"Save"}""")]
+	[DataRow("""{"Id":null,"Name":"Save"}""")]
+	[DataRow("""{"Id":"  ","Name":"Save"}""")]
+	[DataRow("""{"Id":"file.save"}""")]
+	[DataRow("""{"Id":"file.save","Name":null}""")]
+	[DataRow("""{"Id":"file.save","Name":""}""")]
+	public void MissingOrBlankIdOrName_ThrowsJsonException(string json) =>
+		Assert.ThrowsExactly<JsonException>(() => JsonSerializer.Deserialize<Command>(json));
+
+	[TestMethod]
+	public void ValidJson_StillNormalizesLikeThePublicConstructor()
+	{
+		Command? command = JsonSerializer.Deserialize<Command>("""{"Id":" file.save ","Name":" Save ","Description":"  ","Category":" File "}""");
+
+		Assert.IsNotNull(command);
+		Assert.AreEqual("file.save", command.Id.ToString());
+		Assert.AreEqual("Save", command.Name.ToString());
+		Assert.IsNull(command.Description);
+		Assert.AreEqual("File", command.Category?.ToString());
+	}
+
+	[TestMethod]
+	public void PublicConstructor_StillThrowsArgumentException() =>
+		Assert.ThrowsExactly<ArgumentException>(() => new Command(CommandId.Create("  "), CommandName.Create("Save")));
+}

Keybinding/Models/Command.cs

@@ -17,7 +17,6 @@ public sealed class Command : IEquatable<Command>
 	/// <param name="description">Optional description of what the command does</param>
 	/// <param name="category">Optional category for grouping commands</param>
 	/// <exception cref="ArgumentException">Thrown when id or name is null or whitespace</exception>
-	[JsonConstructor]
 	public Command(CommandId id, CommandName name, CommandDescription? description = null, CommandCategory? category = null)
 	{
 		Ensure.NotNull(id);
@@ -69,6 +68,22 @@ public Command(string id, string name, string? description = null, string? categ
 		Category = TrimToNull(category) is string trimmedCategory ? CommandCategory.Create(trimmedCategory) : null;
 	}
 
+	// The serializer binds constructor parameters by name, not position, so this takes name before id only to
+	// differ in signature from the public constructor. It reports a missing or blank id or name as malformed JSON,
+	// as Chord, Phrase and Profile do, rather than with the public constructor's ArgumentException.
+	[JsonConstructor]
+	[System.Diagnostics.CodeAnalysis.SuppressMessage("CodeQuality", "IDE0051:Remove unused private members", Justification = "Called by System.Text.Json through [JsonConstructor].")]
+	private Command(CommandName? name, CommandId? id, CommandDescription? description, CommandCategory? category)
+		: this(RequireJsonText(id, nameof(Id)), RequireJsonText(name, nameof(Name)), description, category)
+	{
+	}
+
+	private static T RequireJsonText<T>(T? value, string propertyName) where T : class
+	{
+		JsonArguments.RequireText(value?.ToString(), nameof(Command), propertyName);
+		return value!;
+	}
+
 	private static string? TrimToNull(string? value) =>
 		string.IsNullOrWhiteSpace(value) ? null : value.Trim();