Fix: generate JsonPatch for @dynamicModel with ...Record<unknown> + backcompat support

packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ScmModelProvider.cs

@@ -39,6 +39,11 @@ public sealed class ScmModelProvider : ModelProvider
         internal bool HasDynamicProperties => _hasDynamicProperties ??= BuildHasDynamicProperties();
         private bool? _hasDynamicProperties;
 
+        // When true, the model needs to generate both JsonPatch and AdditionalProperties for
+        // backward compatibility (the model was previously shipped with AdditionalProperties).
+        private bool NeedsBackCompatAdditionalProperties => _needsBackCompatAdditionalProperties ??= BuildNeedsBackCompatAdditionalProperties();
+        private bool? _needsBackCompatAdditionalProperties;
+
         internal static SuppressionStatement JsonPatchSuppression = new SuppressionStatement(null,
             Literal(ScmEvaluationTypeDiagnosticId),
             ScmEvaluationTypeSuppressionJustification);
@@ -62,10 +67,12 @@ protected override FieldProvider[] BuildFields()
 
             foreach (var field in fields)
             {
-                if (!field.Equals(RawDataField))
+                // Keep the RawDataField when backcompat requires AdditionalProperties to be generated alongside JsonPatch
+                if (field.Equals(RawDataField) && !NeedsBackCompatAdditionalProperties)
                 {
-                    updatedFields.Add(field);
+                    continue;
                 }
+                updatedFields.Add(field);
             }
 
             return [JsonPatchField, .. updatedFields];
@@ -78,6 +85,14 @@ protected override PropertyProvider[] BuildProperties()
                 return base.BuildProperties();
             }
 
+            // For dynamic models with BinaryData additional properties (Record<unknown>),
+            // skip generating AdditionalProperties since JsonPatch handles dynamic properties.
+            // Exception: when backcompat requires preserving AdditionalProperties from the last contract.
+            if (SupportsBinaryDataAdditionalProperties && !NeedsBackCompatAdditionalProperties)
+            {
+                return [JsonPatchProperty, .. base.BuildProperties().Where(p => !p.IsAdditionalProperties)];
+            }
+
             return [JsonPatchProperty, .. base.BuildProperties()];
         }
 
@@ -122,7 +137,9 @@ protected override ConstructorProvider[] BuildConstructors()
                             foreach (var statement in constructor.BodyStatements)
                             {
                                 if (statement is ExpressionStatement { Expression: AssignmentExpression assignmentExpression }
-                                    && assignmentExpression.Value.Equals(RawDataField.AsParameter))
+                                    && (assignmentExpression.Value.Equals(RawDataField.AsParameter) ||
+                                        (!NeedsBackCompatAdditionalProperties &&
+                                         assignmentExpression.Variable is MemberExpression { MemberName: AdditionalPropertiesHelper.AdditionalBinaryDataPropsFieldName })))
                                 {
                                     continue;
                                 }
@@ -152,6 +169,20 @@ protected override ConstructorProvider[] BuildConstructors()
                         constructor.Update(bodyStatements: updatedBody);
                     }
                 }
+                else if (JsonPatchField != null && SupportsBinaryDataAdditionalProperties && !NeedsBackCompatAdditionalProperties && constructor.BodyStatements != null)
+                {
+                    // Remove the additional binary data properties initialization from the init constructor
+                    var updatedBody = constructor.BodyStatements
+                        .Where(s => s is not ExpressionStatement
+                        {
+                            Expression: AssignmentExpression
+                            {
+                                Variable: MemberExpression { MemberName: AdditionalPropertiesHelper.AdditionalBinaryDataPropsFieldName }
+                            }
+                        })
+                        .ToList();
+                    constructor.Update(bodyStatements: updatedBody);
+                }
                 updatedConstructors.Add(constructor);
             }
 
@@ -166,7 +197,7 @@ protected override ConstructorProvider[] BuildConstructors()
 
         private FieldProvider? BuildJsonPatchField()
         {
-            if (!IsDynamicModel || SupportsBinaryDataAdditionalProperties)
+            if (!IsDynamicModel)
             {
                 return null;
             }
@@ -232,10 +263,13 @@ private bool ShouldUpdateFullConstructor()
                 return true;
             }
 
-            return FullConstructor.Signature.Parameters
-                .Any(p => p.Field?.Name.Equals(AdditionalPropertiesHelper.AdditionalBinaryDataPropsFieldName) == true);
+            return FullConstructor.Signature.Parameters.Any(IsAdditionalBinaryDataParameter);
         }
 
+        private static bool IsAdditionalBinaryDataParameter(ParameterProvider p) =>
+            p.Field?.Name.Equals(AdditionalPropertiesHelper.AdditionalBinaryDataPropsFieldName) == true ||
+            p.Property?.BackingField?.Name.Equals(AdditionalPropertiesHelper.AdditionalBinaryDataPropsFieldName) == true;
+
         private void UpdateFullConstructorParameters()
         {
             if (BaseJsonPatchProperty.Value is null)
@@ -249,8 +283,13 @@ private void UpdateFullConstructorParameters()
 
             foreach (var parameter in FullConstructor.Signature.Parameters)
             {
-                if (parameter.Field?.Name.Equals(AdditionalPropertiesHelper.AdditionalBinaryDataPropsFieldName) == true)
+                if (IsAdditionalBinaryDataParameter(parameter))
                 {
+                    if (NeedsBackCompatAdditionalProperties)
+                    {
+                        // Backcompat: keep the additionalBinaryData parameter
+                        updatedParameters.Add(parameter);
+                    }
                     updatedParameters.Add(jsonPatchParameter);
                 }
                 else
@@ -342,5 +381,16 @@ private bool BuildHasDynamicProperties()
 
             return null;
         }
+
+        private bool BuildNeedsBackCompatAdditionalProperties()
+        {
+            if (!IsDynamicModel || !SupportsBinaryDataAdditionalProperties || LastContractView == null)
+            {
+                return false;
+            }
+
+            return LastContractView.Properties.Any(p =>
+                p.Name == AdditionalPropertiesHelper.DefaultAdditionalPropertiesPropertyName);
+        }
     }
 }

packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/MrwSerializationTypeDefinitions/DynamicModelSerializationTests.cs

@@ -1489,5 +1489,81 @@ public void DeserializeDynamicDerivedModelWithNonDiscriminatedBase()
             var file = writer.Write();
             Assert.AreEqual(Helpers.GetExpectedFromFile(), file.Content);
         }
+
+        [Test]
+        public async Task WriteModelWithBinaryDataAdditionalPropsBackCompat()
+        {
+            var inputModel = InputFactory.Model(
+                "dynamicModel",
+                isDynamicModel: true,
+                additionalProperties: InputPrimitiveType.Any,
+                properties:
+                [
+                    InputFactory.Property("p1", InputPrimitiveType.String, isRequired: true)
+                ]);
+
+            await MockHelpers.LoadMockGeneratorAsync(
+                lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync(),
+                inputModels: () => [inputModel]);
+            var model = ScmCodeModelGenerator.Instance.TypeFactory.CreateModel(inputModel) as ClientModel.Providers.ScmModelProvider;
+
+            Assert.IsNotNull(model);
+            Assert.IsTrue(model!.IsDynamicModel);
+            Assert.IsTrue(model.Properties.Any(p => p.IsAdditionalProperties),
+                "Backcompat model should still generate AdditionalProperties");
+
+            var serialization = model.SerializationProviders.SingleOrDefault();
+            Assert.IsNotNull(serialization);
+
+            // Ensure Constructors have been built (which updates FullConstructor parameters
+            // used by the serialization provider). In the full pipeline, the model's Constructors
+            // are always written before the serialization file.
+            Assert.AreEqual(2, model.Constructors.Count);
+
+            var writer = new TypeProviderWriter(new FilteredMethodsTypeProvider(
+                serialization!,
+                name => name is "JsonModelWriteCore" or "Write"));
+
+            var file = writer.Write();
+            Assert.AreEqual(Helpers.GetExpectedFromFile(), file.Content);
+        }
+
+        [Test]
+        public async Task DeserializeModelWithBinaryDataAdditionalPropsBackCompat()
+        {
+            var inputModel = InputFactory.Model(
+                "dynamicModel",
+                isDynamicModel: true,
+                additionalProperties: InputPrimitiveType.Any,
+                properties:
+                [
+                    InputFactory.Property("p1", InputPrimitiveType.String, isRequired: true)
+                ]);
+
+            await MockHelpers.LoadMockGeneratorAsync(
+                lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync(),
+                inputModels: () => [inputModel]);
+            var model = ScmCodeModelGenerator.Instance.TypeFactory.CreateModel(inputModel) as ClientModel.Providers.ScmModelProvider;
+
+            Assert.IsNotNull(model);
+            Assert.IsTrue(model!.IsDynamicModel);
+            Assert.IsTrue(model.Properties.Any(p => p.IsAdditionalProperties),
+                "Backcompat model should still generate AdditionalProperties");
+
+            var serialization = model.SerializationProviders.SingleOrDefault();
+            Assert.IsNotNull(serialization);
+
+            // Ensure Constructors have been built (which updates FullConstructor parameters
+            // used by the serialization provider). In the full pipeline, the model's Constructors
+            // are always written before the serialization file.
+            Assert.AreEqual(2, model.Constructors.Count);
+
+            var writer = new TypeProviderWriter(new FilteredMethodsTypeProvider(
+                serialization!,
+                name => name.StartsWith("Deserialize")));
+
+            var file = writer.Write();
+            Assert.AreEqual(Helpers.GetExpectedFromFile(), file.Content);
+        }
     }
 }

packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/MrwSerializationTypeDefinitions/TestData/DynamicModelSerializationTests/DeserializeModelWithBinaryDataAdditionalPropsBackCompat.cs

@@ -0,0 +1,38 @@
+// <auto-generated/>
+
+#nullable disable
+
+using System;
+using System.ClientModel.Primitives;
+using System.Collections.Generic;
+using System.Text.Json;
+using Sample.Models;
+
+namespace Sample
+{
+    public partial class DynamicModel
+    {
+        internal static global::Sample.Models.DynamicModel DeserializeDynamicModel(global::System.Text.Json.JsonElement element, global::System.BinaryData data, global::System.ClientModel.Primitives.ModelReaderWriterOptions options)
+        {
+            if ((element.ValueKind == global::System.Text.Json.JsonValueKind.Null))
+            {
+                return null;
+            }
+            string p1 = default;
+            global::System.Collections.Generic.IDictionary<string, global::System.BinaryData> additionalProperties = new global::Sample.ChangeTrackingDictionary<string, global::System.BinaryData>();
+#pragma warning disable SCME0001 // Type is for evaluation purposes only and is subject to change or removal in future updates.
+            global::System.ClientModel.Primitives.JsonPatch patch = new global::System.ClientModel.Primitives.JsonPatch((data is null) ? global::System.ReadOnlyMemory<byte>.Empty : data.ToMemory());
+#pragma warning restore SCME0001 // Type is for evaluation purposes only and is subject to change or removal in future updates.
+            foreach (var prop in element.EnumerateObject())
+            {
+                if (prop.NameEquals("p1"u8))
+                {
+                    p1 = prop.Value.GetString();
+                    continue;
+                }
+                additionalProperties.Add(prop.Name, global::System.BinaryData.FromString(prop.Value.GetRawText()));
+            }
+            return new global::Sample.Models.DynamicModel(p1, additionalProperties, patch);
+        }
+    }
+}

packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/MrwSerializationTypeDefinitions/TestData/DynamicModelSerializationTests/DeserializeModelWithBinaryDataAdditionalPropsBackCompat/DynamicModel.cs

@@ -0,0 +1,14 @@
+// This file represents the previous (last contract) state of the model before @dynamicModel was added.
+// It has AdditionalProperties but no JsonPatch, which triggers the backcompat path.
+
+using System.Collections.Generic;
+
+namespace Sample.Models
+{
+    public partial class DynamicModel
+    {
+        public string P1 { get; set; }
+
+        public global::System.Collections.Generic.IDictionary<string, global::System.BinaryData> AdditionalProperties { get; }
+    }
+}

packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/MrwSerializationTypeDefinitions/TestData/DynamicModelSerializationTests/WriteModelWithBinaryDataAdditionalPropsBackCompat.cs

@@ -0,0 +1,61 @@
+// <auto-generated/>
+
+#nullable disable
+
+using System;
+using System.ClientModel.Primitives;
+using System.Text.Json;
+using Sample.Models;
+
+namespace Sample
+{
+    public partial class DynamicModel
+    {
+        global::System.BinaryData global::System.ClientModel.Primitives.IPersistableModel<global::Sample.Models.DynamicModel>.Write(global::System.ClientModel.Primitives.ModelReaderWriterOptions options) => this.PersistableModelWriteCore(options);
+
+        void global::System.ClientModel.Primitives.IJsonModel<global::Sample.Models.DynamicModel>.Write(global::System.Text.Json.Utf8JsonWriter writer, global::System.ClientModel.Primitives.ModelReaderWriterOptions options)
+        {
+#pragma warning disable SCME0001 // Type is for evaluation purposes only and is subject to change or removal in future updates.
+            if (Patch.Contains("$"u8))
+            {
+                writer.WriteRawValue(Patch.GetJson("$"u8));
+                return;
+            }
+#pragma warning restore SCME0001 // Type is for evaluation purposes only and is subject to change or removal in future updates.
+
+            writer.WriteStartObject();
+            this.JsonModelWriteCore(writer, options);
+            writer.WriteEndObject();
+        }
+
+        protected virtual void JsonModelWriteCore(global::System.Text.Json.Utf8JsonWriter writer, global::System.ClientModel.Primitives.ModelReaderWriterOptions options)
+        {
+            string format = (options.Format == "W") ? ((global::System.ClientModel.Primitives.IPersistableModel<global::Sample.Models.DynamicModel>)this).GetFormatFromOptions(options) : options.Format;
+            if ((format != "J"))
+            {
+                throw new global::System.FormatException($"The model {nameof(global::Sample.Models.DynamicModel)} does not support writing '{format}' format.");
+            }
+#pragma warning disable SCME0001 // Type is for evaluation purposes only and is subject to change or removal in future updates.
+            if (!Patch.Contains("$.p1"u8))
+            {
+                writer.WritePropertyName("p1"u8);
+                writer.WriteStringValue(P1);
+            }
+            foreach (var item in AdditionalProperties)
+            {
+                writer.WritePropertyName(item.Key);
+#if NET6_0_OR_GREATER
+                writer.WriteRawValue(item.Value);
+#else
+                using (global::System.Text.Json.JsonDocument document = global::System.Text.Json.JsonDocument.Parse(item.Value))
+                {
+                    global::System.Text.Json.JsonSerializer.Serialize(writer, document.RootElement);
+                }
+#endif
+            }
+
+            Patch.WriteTo(writer);
+#pragma warning restore SCME0001 // Type is for evaluation purposes only and is subject to change or removal in future updates.
+        }
+    }
+}

packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/MrwSerializationTypeDefinitions/TestData/DynamicModelSerializationTests/WriteModelWithBinaryDataAdditionalPropsBackCompat/DynamicModel.cs

@@ -0,0 +1,14 @@
+// This file represents the previous (last contract) state of the model before @dynamicModel was added.
+// It has AdditionalProperties but no JsonPatch, which triggers the backcompat path.
+
+using System.Collections.Generic;
+
+namespace Sample.Models
+{
+    public partial class DynamicModel
+    {
+        public string P1 { get; set; }
+
+        public global::System.Collections.Generic.IDictionary<string, global::System.BinaryData> AdditionalProperties { get; }
+    }
+}

packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ScmModelProvider/ScmModelProviderTests.cs

@@ -157,6 +157,45 @@ public void TestDynamicModelWithBinaryDataAdditionalProps()
 
             Assert.IsNotNull(model);
             Assert.IsTrue(model!.IsDynamicModel);
+            // Dynamic models with Record<unknown> (BinaryData additional properties) should generate
+            // JsonPatch instead of AdditionalProperties.
+            Assert.IsNotNull(model.JsonPatchProperty, "Dynamic models with Record<unknown> should generate JsonPatch");
+            Assert.IsFalse(model.Properties.Any(p => p.IsAdditionalProperties),
+                "Dynamic models with Record<unknown> should not generate AdditionalProperties");
+            AssertJsonIgnoreAttributeOnPatchProperty(model);
+
+            var writer = new TypeProviderWriter(model);
+            var file = writer.Write();
+
+            Assert.AreEqual(Helpers.GetExpectedFromFile(), file.Content);
+        }
+
+        [Test]
+        public async Task TestDynamicModelWithBinaryDataAdditionalPropsBackCompat()
+        {
+            // Scenario: A model was previously shipped with AdditionalProperties (IDictionary<string, BinaryData>)
+            // but the model has now been updated to use @dynamicModel. Both JsonPatch and AdditionalProperties
+            // should be generated to maintain backward compatibility.
+            var inputModel = InputFactory.Model(
+                "dynamicModel",
+                isDynamicModel: true,
+                additionalProperties: InputPrimitiveType.Any,
+                properties:
+                [
+                    InputFactory.Property("p1", InputPrimitiveType.String, isRequired: true)
+                ]);
+
+            await MockHelpers.LoadMockGeneratorAsync(
+                lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync(),
+                inputModels: () => [inputModel]);
+            var model = ScmCodeModelGenerator.Instance.TypeFactory.CreateModel(inputModel) as ScmModel;
+
+            Assert.IsNotNull(model);
+            Assert.IsTrue(model!.IsDynamicModel);
+            // Backcompat: both JsonPatch and AdditionalProperties should be generated
+            Assert.IsNotNull(model.JsonPatchProperty, "Dynamic model should generate JsonPatch");
+            Assert.IsTrue(model.Properties.Any(p => p.IsAdditionalProperties),
+                "Dynamic model should still generate AdditionalProperties for backcompat");
             AssertJsonIgnoreAttributeOnPatchProperty(model);
 
             var writer = new TypeProviderWriter(model);