Make MsappSerialization and MsaprSerialization public and immutable Json options

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;
+
+    /// <summary>
+    /// Verifies that a PackedJson with LoadFromYaml=true survives a serialize/deserialize round-trip.
+    /// </summary>
+    [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<PackedJson>(json, Options);
+
+        deserialized.Should().NotBeNull();
+        deserialized!.LoadConfiguration.LoadFromYaml.Should().BeTrue();
+        deserialized.PackedStructureVersion.Should().Be(PackedJson.CurrentPackedStructureVersion);
+    }
+
+    /// <summary>
+    /// 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.
+    /// </summary>
+    [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<PackedJson>(json, Options);
+
+        deserialized.Should().NotBeNull();
+        deserialized!.LoadConfiguration.LoadFromYaml.Should().BeFalse();
+        deserialized.PackedStructureVersion.Should().Be(PackedJson.CurrentPackedStructureVersion);
+    }
+
+    /// <summary>
+    /// Round-trips a fully-populated PackedJson (all optional fields set) to ensure nothing is lost.
+    /// </summary>
+    [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<PackedJson>(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();
+    }
+
+    /// <summary>
+    /// Verifies that a PackedJson with LoadFromYaml=false and a PackingClient survives round-trip.
+    /// </summary>
+    [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<PackedJson>(json, Options);
+        deserialized.Should().NotBeNull();
+        deserialized!.LoadConfiguration.LoadFromYaml.Should().BeFalse();
+        deserialized.PackingClient!.Name.Should().Be("MyCli");
+    }
+}

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
+{
+    /// <summary>
+    /// Verifies that a minimal MsaprHeaderJson survives a serialize/deserialize round-trip.
+    /// </summary>
+    [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<MsaprHeaderJson>(json, MsaprSerialization.DefaultJsonSerializeOptions);
+
+        deserialized.Should().NotBeNull();
+        deserialized!.MsaprStructureVersion.Should().Be(MsaprHeaderJson.CurrentMsaprStructureVersion);
+        deserialized.UnpackedConfiguration.Should().NotBeNull();
+        deserialized.UnpackedConfiguration.ContentTypes.Should().BeEmpty();
+    }
+
+    /// <summary>
+    /// Verifies that a fully-populated MsaprHeaderJson (all fields set) survives a serialize/deserialize round-trip.
+    /// </summary>
+    [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<MsaprHeaderJson>(json, MsaprSerialization.DefaultJsonSerializeOptions);
+
+        deserialized.Should().NotBeNull();
+        deserialized!.MsaprStructureVersion.Should().Be(MsaprHeaderJson.CurrentMsaprStructureVersion);
+        deserialized.UnpackedConfiguration.ContentTypes.Should().BeEquivalentTo(["Yaml", "Assets"]);
+    }
+
+    /// <summary>
+    /// Verifies that unknown/additional properties in the JSON are ignored (forward-compatible deserialization).
+    /// </summary>
+    [TestMethod]
+    public void MsaprHeaderJson_RoundTrip_IgnoresUnknownProperties()
+    {
+        var jsonWithExtraFields = """
+            {
+                "MsaprStructureVersion": "0.1",
+                "UnpackedConfiguration": {
+                    "ContentTypes": ["Yaml"],
+                    "FutureProperty": "some value"
+                },
+                "AnotherFutureTopLevelProperty": 42
+            }
+            """;
+
+        var deserialized = JsonSerializer.Deserialize<MsaprHeaderJson>(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);
+    }
+}

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()
     {
     }
+
+    /// <summary>
+    /// Asserts that two JSON strings are structurally equivalent by comparing their <see cref="JsonNode"/> representations.
+    /// </summary>
+    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}");
+    }
+
+    /// <summary>
+    /// Utility to create a <see cref="JsonElement"/> from a JSON string, which can be useful for constructing test inputs for models that use <see cref="JsonElement"/> properties.
+    /// </summary>
+    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();
+    }
 }

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
+{
+    /// <summary>
+    /// A fluent way of making a <see cref="JsonSerializerOptions"/> instance immutable.
+    /// Especially useful for shared static instances.
+    /// </summary>
+    public static JsonSerializerOptions ToReadOnly(this JsonSerializerOptions options)
+    {
+        options.MakeReadOnly(populateMissingResolver: true);
+        return options;
+    }
+}

src/Persistence/MsApp/Serialization/MsappSerialization.cs

@@ -17,13 +17,13 @@ namespace Microsoft.PowerPlatform.PowerApps.Persistence.MsApp.Serialization;
 /// <summary>
 /// Shared constants used for .msapp serialization and deserialization.
 /// </summary>
-internal static class MsappSerialization
+public static class MsappSerialization
 {
     /// <summary>
     /// This should match the options used in DocumentServer for deserializing msapp json files.
     /// See: JsonDocumentSerializer.SerializerOptions in DocumentServer.Core.
     /// </summary>
-    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();
 
     /// <summary>
     /// This should match the options used in DocumentServer for deserializing msapp json files.
     /// See: JsonDocumentSerializer.SerializerOptions in DocumentServer.Core.
     /// </summary>
-    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();
 
     /// <summary>
     /// Serialization options used for the 'packed.json' file in the msapp archive.
     /// </summary>
-    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();
 }

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.
     /// </summary>
     [JsonExtensionData]
-    public ImmutableDictionary<string, JsonElement>? AdditionalProperties { get; init; }
+    public IDictionary<string, JsonElement>? 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.
     /// </summary>
     [JsonExtensionData]
-    public ImmutableDictionary<string, JsonElement>? AdditionalProperties { get; init; }
+    public IDictionary<string, JsonElement>? AdditionalProperties { get; init; }
 }

src/Persistence/MsappPacking/Serialization/MsaprSerialization.cs

@@ -17,22 +17,27 @@ namespace Microsoft.PowerPlatform.PowerApps.Persistence.MsappPacking.Serializati
 /// <summary>
 /// Shared constants used for .msapr serialization and deserialization.
 /// </summary>
-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();
 }