From c2e96e51d9d5ff160c114c5581a80374af4e5731 Mon Sep 17 00:00:00 2001 From: Joe Mayo Date: Thu, 12 Mar 2026 11:35:37 -0700 Subject: [PATCH] Make `MsappSerialization` public and immutable Json options - fix DefaultIgnoreCondition so 'required' properties with default values (e.g. bool=false) are properly round-tripped without being omitted from the serialized JSON --- .../Serialization/MsappSerializationTests.cs | 115 ++++++++++++++++++ .../Serialization/MsaprSerializationTests.cs | 94 ++++++++++++++ src/Persistence.Tests/TestBase.cs | 25 +++- src/Persistence/Extensions/JsonExtensions.cs | 19 +++ .../MsApp/Serialization/MsappSerialization.cs | 35 ++++-- .../MsappPacking/Models/MsaprHeaderJson.cs | 4 +- .../Serialization/MsaprSerialization.cs | 19 +-- 7 files changed, 289 insertions(+), 22 deletions(-) create mode 100644 src/Persistence.Tests/MsApp/Serialization/MsappSerializationTests.cs create mode 100644 src/Persistence.Tests/MsappPacking/Serialization/MsaprSerializationTests.cs create mode 100644 src/Persistence/Extensions/JsonExtensions.cs diff --git a/src/Persistence.Tests/MsApp/Serialization/MsappSerializationTests.cs b/src/Persistence.Tests/MsApp/Serialization/MsappSerializationTests.cs new file mode 100644 index 00000000..307f4412 --- /dev/null +++ b/src/Persistence.Tests/MsApp/Serialization/MsappSerializationTests.cs @@ -0,0 +1,115 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using System.Text.Json; +using Microsoft.PowerPlatform.PowerApps.Persistence.MsApp.Models; +using Microsoft.PowerPlatform.PowerApps.Persistence.MsApp.Serialization; + +namespace Persistence.Tests.MsApp.Serialization; + +[TestClass] +public class MsappSerializationTests : TestBase +{ + private static readonly JsonSerializerOptions Options = MsappSerialization.PackedJsonSerializeOptions; + + /// + /// Verifies that a PackedJson with LoadFromYaml=true survives a serialize/deserialize round-trip. + /// + [TestMethod] + public void PackedJson_RoundTrip_LoadFromYaml_True() + { + var original = new PackedJson + { + PackedStructureVersion = PackedJson.CurrentPackedStructureVersion, + LoadConfiguration = new() { LoadFromYaml = true }, + }; + + var json = JsonSerializer.Serialize(original, Options); + var deserialized = JsonSerializer.Deserialize(json, Options); + + deserialized.Should().NotBeNull(); + deserialized!.LoadConfiguration.LoadFromYaml.Should().BeTrue(); + deserialized.PackedStructureVersion.Should().Be(PackedJson.CurrentPackedStructureVersion); + } + + /// + /// Verifies that a PackedJson with LoadFromYaml=false survives a serialize/deserialize round-trip. + /// With WhenWritingDefault, the 'false' value is the default for bool and will be omitted during + /// serialization. Because LoadFromYaml is marked 'required', deserialization then fails unless + /// the serializer options are corrected. + /// + [TestMethod] + public void PackedJson_RoundTrip_LoadFromYaml_False() + { + var original = new PackedJson + { + PackedStructureVersion = PackedJson.CurrentPackedStructureVersion, + LoadConfiguration = new() { LoadFromYaml = false }, + }; + + var json = JsonSerializer.Serialize(original, Options); + + // The serialized JSON must contain the LoadFromYaml property, even when false, + // because it is a 'required' property on deserialization. + json.Should().Contain("LoadFromYaml", "required bool properties must not be omitted when their value is the default (false)"); + + var deserialized = JsonSerializer.Deserialize(json, Options); + + deserialized.Should().NotBeNull(); + deserialized!.LoadConfiguration.LoadFromYaml.Should().BeFalse(); + deserialized.PackedStructureVersion.Should().Be(PackedJson.CurrentPackedStructureVersion); + } + + /// + /// Round-trips a fully-populated PackedJson (all optional fields set) to ensure nothing is lost. + /// + [TestMethod] + public void PackedJson_RoundTrip_FullyPopulated() + { + var utcNow = new DateTime(2024, 6, 15, 12, 0, 0, DateTimeKind.Utc); + var original = new PackedJson + { + PackedStructureVersion = PackedJson.CurrentPackedStructureVersion, + LastPackedDateTimeUtc = utcNow, + PackingClient = new PackedJsonPackingClient + { + Name = "TestClient", + Version = "1.2.3", + }, + LoadConfiguration = new() { LoadFromYaml = true }, + }; + + var json = JsonSerializer.Serialize(original, Options); + var deserialized = JsonSerializer.Deserialize(json, Options); + + deserialized.Should().NotBeNull(); + deserialized!.PackedStructureVersion.Should().Be(PackedJson.CurrentPackedStructureVersion); + deserialized.LastPackedDateTimeUtc.Should().Be(utcNow); + deserialized.PackingClient.Should().NotBeNull(); + deserialized.PackingClient!.Name.Should().Be("TestClient"); + deserialized.PackingClient.Version.Should().Be("1.2.3"); + deserialized.LoadConfiguration.LoadFromYaml.Should().BeTrue(); + } + + /// + /// Verifies that a PackedJson with LoadFromYaml=false and a PackingClient survives round-trip. + /// + [TestMethod] + public void PackedJson_RoundTrip_LoadFromYaml_False_WithPackingClient() + { + var original = new PackedJson + { + PackedStructureVersion = PackedJson.CurrentPackedStructureVersion, + PackingClient = new PackedJsonPackingClient { Name = "MyCli", Version = "0.0.1" }, + LoadConfiguration = new() { LoadFromYaml = false }, + }; + + var json = JsonSerializer.Serialize(original, Options); + json.Should().Contain("LoadFromYaml", "required bool must be present in JSON even when false"); + + var deserialized = JsonSerializer.Deserialize(json, Options); + deserialized.Should().NotBeNull(); + deserialized!.LoadConfiguration.LoadFromYaml.Should().BeFalse(); + deserialized.PackingClient!.Name.Should().Be("MyCli"); + } +} diff --git a/src/Persistence.Tests/MsappPacking/Serialization/MsaprSerializationTests.cs b/src/Persistence.Tests/MsappPacking/Serialization/MsaprSerializationTests.cs new file mode 100644 index 00000000..a922f0cc --- /dev/null +++ b/src/Persistence.Tests/MsappPacking/Serialization/MsaprSerializationTests.cs @@ -0,0 +1,94 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using System.Collections.Immutable; +using System.Text.Json; +using Microsoft.PowerPlatform.PowerApps.Persistence.MsappPacking.Models; +using Microsoft.PowerPlatform.PowerApps.Persistence.MsappPacking.Serialization; + +namespace Persistence.Tests.MsappPacking.Serialization; + +[TestClass] +public class MsaprSerializationTests : TestBase +{ + /// + /// Verifies that a minimal MsaprHeaderJson survives a serialize/deserialize round-trip. + /// + [TestMethod] + public void MsaprHeaderJson_RoundTrip_Minimal() + { + var original = new MsaprHeaderJson + { + MsaprStructureVersion = MsaprHeaderJson.CurrentMsaprStructureVersion, + UnpackedConfiguration = new() + { + ContentTypes = [], + }, + }; + + var json = JsonSerializer.Serialize(original, MsaprSerialization.DefaultJsonSerializeOptions); + var deserialized = JsonSerializer.Deserialize(json, MsaprSerialization.DefaultJsonSerializeOptions); + + deserialized.Should().NotBeNull(); + deserialized!.MsaprStructureVersion.Should().Be(MsaprHeaderJson.CurrentMsaprStructureVersion); + deserialized.UnpackedConfiguration.Should().NotBeNull(); + deserialized.UnpackedConfiguration.ContentTypes.Should().BeEmpty(); + } + + /// + /// Verifies that a fully-populated MsaprHeaderJson (all fields set) survives a serialize/deserialize round-trip. + /// + [TestMethod] + public void MsaprHeaderJson_RoundTrip_FullyPopulated() + { + var original = new MsaprHeaderJson + { + MsaprStructureVersion = MsaprHeaderJson.CurrentMsaprStructureVersion, + UnpackedConfiguration = new() + { + ContentTypes = ["Yaml", "Assets"], + }, + }; + + var json = JsonSerializer.Serialize(original, MsaprSerialization.DefaultJsonSerializeOptions); + var deserialized = JsonSerializer.Deserialize(json, MsaprSerialization.DefaultJsonSerializeOptions); + + deserialized.Should().NotBeNull(); + deserialized!.MsaprStructureVersion.Should().Be(MsaprHeaderJson.CurrentMsaprStructureVersion); + deserialized.UnpackedConfiguration.ContentTypes.Should().BeEquivalentTo(["Yaml", "Assets"]); + } + + /// + /// Verifies that unknown/additional properties in the JSON are ignored (forward-compatible deserialization). + /// + [TestMethod] + public void MsaprHeaderJson_RoundTrip_IgnoresUnknownProperties() + { + var jsonWithExtraFields = """ + { + "MsaprStructureVersion": "0.1", + "UnpackedConfiguration": { + "ContentTypes": ["Yaml"], + "FutureProperty": "some value" + }, + "AnotherFutureTopLevelProperty": 42 + } + """; + + var deserialized = JsonSerializer.Deserialize(jsonWithExtraFields, MsaprSerialization.DefaultJsonSerializeOptions); + + deserialized.Should().NotBeNull(); + deserialized!.MsaprStructureVersion.Should().Be(new Version(0, 1)); + deserialized.UnpackedConfiguration.ContentTypes.Should().BeEquivalentTo(["Yaml"]); + + // And we should see the unknown properties still captured: + deserialized.AdditionalProperties.Should().NotBeNull() + .And.Subject.Keys.Should().BeEquivalentTo(["AnotherFutureTopLevelProperty"]); + deserialized.UnpackedConfiguration.AdditionalProperties.Should().NotBeNull() + .And.Subject.Keys.Should().BeEquivalentTo(["FutureProperty"]); + + // Re-serializing should produce JSON node-equivalent to the original input. + var reserialized = JsonSerializer.Serialize(deserialized, MsaprSerialization.DefaultJsonSerializeOptions); + JsonShouldBeEquivalentTo(reserialized, jsonWithExtraFields); + } +} diff --git a/src/Persistence.Tests/TestBase.cs b/src/Persistence.Tests/TestBase.cs index 43d9354b..2cfc4c76 100644 --- a/src/Persistence.Tests/TestBase.cs +++ b/src/Persistence.Tests/TestBase.cs @@ -1,9 +1,12 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -using System.Globalization; using Microsoft.Extensions.DependencyInjection; using Microsoft.PowerPlatform.PowerApps.Persistence.MsApp; +using System.Diagnostics.CodeAnalysis; +using System.Globalization; +using System.Text.Json; +using System.Text.Json.Nodes; namespace Persistence.Tests; @@ -12,4 +15,24 @@ public abstract class TestBase : VSTestBase protected TestBase() { } + + /// + /// Asserts that two JSON strings are structurally equivalent by comparing their representations. + /// + protected static void JsonShouldBeEquivalentTo(string actualJson, string expectedJson) + { + // While we're detecting equality correct here, the failure message isn't particularly useful, but can be improved in the future. + JsonNode.DeepEquals(JsonNode.Parse(actualJson), JsonNode.Parse(expectedJson)) + .Should().BeTrue($"actual JSON should be node-equivalent to expected JSON.\nActual:\n{actualJson}\nExpected:\n{expectedJson}"); + } + + /// + /// Utility to create a from a JSON string, which can be useful for constructing test inputs for models that use properties. + /// + public static JsonElement ToJsonElement([StringSyntax(StringSyntaxAttribute.Json)] string json) + { + using var doc = JsonDocument.Parse(json); + // We need to Clone so the element outlives 'doc' being disposed + return doc.RootElement.Clone(); + } } diff --git a/src/Persistence/Extensions/JsonExtensions.cs b/src/Persistence/Extensions/JsonExtensions.cs new file mode 100644 index 00000000..efb3977b --- /dev/null +++ b/src/Persistence/Extensions/JsonExtensions.cs @@ -0,0 +1,19 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using System.Text.Json; + +namespace Microsoft.PowerPlatform.PowerApps.Persistence.Extensions; + +public static class JsonExtensions +{ + /// + /// A fluent way of making a instance immutable. + /// Especially useful for shared static instances. + /// + public static JsonSerializerOptions ToReadOnly(this JsonSerializerOptions options) + { + options.MakeReadOnly(populateMissingResolver: true); + return options; + } +} diff --git a/src/Persistence/MsApp/Serialization/MsappSerialization.cs b/src/Persistence/MsApp/Serialization/MsappSerialization.cs index 1d200b72..9691acfc 100644 --- a/src/Persistence/MsApp/Serialization/MsappSerialization.cs +++ b/src/Persistence/MsApp/Serialization/MsappSerialization.cs @@ -17,13 +17,13 @@ namespace Microsoft.PowerPlatform.PowerApps.Persistence.MsApp.Serialization; /// /// Shared constants used for .msapp serialization and deserialization. /// -internal static class MsappSerialization +public static class MsappSerialization { /// /// This should match the options used in DocumentServer for deserializing msapp json files. /// See: JsonDocumentSerializer.SerializerOptions in DocumentServer.Core. /// - private static readonly JsonSerializerOptions DefaultSharedJsonSerializeOptions = new() + private static readonly JsonSerializerOptions DefaultSharedJsonSerializeOptions = new JsonSerializerOptions() { PropertyNameCaseInsensitive = true, AllowTrailingCommas = true, @@ -38,32 +38,43 @@ internal static class MsappSerialization // But this may have impact on other code which depends on this property. new JsonDateTimeAssumesUtcConverter(), }, - }; + }.ToReadOnly(); - public static readonly JsonSerializerOptions DocumentJsonSerializeOptions = new(DefaultSharedJsonSerializeOptions); + internal static readonly JsonSerializerOptions DocumentJsonSerializeOptions = new JsonSerializerOptions(DefaultSharedJsonSerializeOptions) + .ToReadOnly(); /// /// This should match the options used in DocumentServer for deserializing msapp json files. /// See: JsonDocumentSerializer.SerializerOptions in DocumentServer.Core. /// - public readonly static JsonSerializerOptions HeaderJsonSerializeOptions = new(DefaultSharedJsonSerializeOptions) + public readonly static JsonSerializerOptions HeaderJsonSerializeOptions = new JsonSerializerOptions(DefaultSharedJsonSerializeOptions) { + // Note: The docsvr doesn't indent the Header.json file. WriteIndented = false, - }; + }.ToReadOnly(); /// /// Serialization options used for the 'packed.json' file in the msapp archive. /// - public static readonly JsonSerializerOptions PackedJsonSerializeOptions = new() + public static readonly JsonSerializerOptions PackedJsonSerializeOptions = new JsonSerializerOptions() { // Note: We explicitly don't derive from the default, since this is a net-new file which is fully owned by this library. - PropertyNameCaseInsensitive = true, - AllowTrailingCommas = true, - DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingDefault, - UnmappedMemberHandling = JsonUnmappedMemberHandling.Skip, Converters = { new JsonDateTimeUtcConverter(), }, - }; + + // Use WhenWritingNull so that non-nullable value types (e.g. bool) are always written, + // which is required for round-tripping 'required' properties whose value equals the type default (e.g. LoadFromYaml=false). + // WhenWritingDefault would silently omit those properties, causing deserialization failures. + DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull, + WriteIndented = true, + + // Deserialization only options: + PropertyNameCaseInsensitive = true, + AllowTrailingCommas = true, + // In order to ensure forward-compatible deserialization, we ignore unknown members + // Any object model that wants to also survive round-tripping, must use JsonExtensionData to capture those unknown members. + UnmappedMemberHandling = JsonUnmappedMemberHandling.Skip, + }.ToReadOnly(); } diff --git a/src/Persistence/MsappPacking/Models/MsaprHeaderJson.cs b/src/Persistence/MsappPacking/Models/MsaprHeaderJson.cs index 647b54e7..5c435343 100644 --- a/src/Persistence/MsappPacking/Models/MsaprHeaderJson.cs +++ b/src/Persistence/MsappPacking/Models/MsaprHeaderJson.cs @@ -25,7 +25,7 @@ public sealed record MsaprHeaderJson /// In order to support forward-compatible deserialization, we alllow arbitrary additional properties. /// [JsonExtensionData] - public ImmutableDictionary? AdditionalProperties { get; init; } + public IDictionary? AdditionalProperties { get; init; } } public sealed record MsaprHeaderJsonUnpackedConfiguration @@ -39,5 +39,5 @@ public sealed record MsaprHeaderJsonUnpackedConfiguration /// In order to support forward-compatible deserialization, we alllow arbitrary additional properties. /// [JsonExtensionData] - public ImmutableDictionary? AdditionalProperties { get; init; } + public IDictionary? AdditionalProperties { get; init; } } diff --git a/src/Persistence/MsappPacking/Serialization/MsaprSerialization.cs b/src/Persistence/MsappPacking/Serialization/MsaprSerialization.cs index c0de1437..833f6bd9 100644 --- a/src/Persistence/MsappPacking/Serialization/MsaprSerialization.cs +++ b/src/Persistence/MsappPacking/Serialization/MsaprSerialization.cs @@ -17,22 +17,27 @@ namespace Microsoft.PowerPlatform.PowerApps.Persistence.MsappPacking.Serializati /// /// Shared constants used for .msapr serialization and deserialization. /// -internal static class MsaprSerialization +public static class MsaprSerialization { - public static readonly JsonSerializerOptions DefaultJsonSerializeOptions = new() + public static readonly JsonSerializerOptions DefaultJsonSerializeOptions = new JsonSerializerOptions() { - WriteIndented = true, - DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingDefault, - Converters = { - // TODO: ensure we save date-times in UTC round-tripable format + // If we ever need to save DateTime values, we should do so using the following converter to ensure correct serialization as UTC time: + //new JsonDateTimeUtcConverter(), }, + // Use WhenWritingNull so that non-nullable value types (e.g. bool) are always written, + // which is required for round-tripping 'required' properties whose value equals the type default (e.g. LoadFromYaml=false). + // WhenWritingDefault would silently omit those properties, causing deserialization failures. + DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull, + WriteIndented = true, + // Deserialization only options: PropertyNameCaseInsensitive = true, AllowTrailingCommas = true, // In order to ensure forward-compatible deserialization, we ignore unknown members + // Any object model that wants to also survive round-tripping, must use JsonExtensionData to capture those unknown members. UnmappedMemberHandling = JsonUnmappedMemberHandling.Skip, - }; + }.ToReadOnly(); }