diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/MrwSerializationTypeDefinitions/AdditionalPropertiesTest.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/MrwSerializationTypeDefinitions/AdditionalPropertiesTest.cs index 756013f0fac..ae643bc374b 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/MrwSerializationTypeDefinitions/AdditionalPropertiesTest.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/MrwSerializationTypeDefinitions/AdditionalPropertiesTest.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Threading.Tasks; using Microsoft.TypeSpec.Generator.ClientModel.Providers; using Microsoft.TypeSpec.Generator.Input; using Microsoft.TypeSpec.Generator.Input.Extensions; @@ -159,5 +160,132 @@ public static IEnumerable TestBuildJsonModelWriteCoreTestCases yield return new TestCaseData(new InputUnionType("union", [InputPrimitiveType.String, InputPrimitiveType.Float64])); } } + + [Test] + public async Task TestBuildDeserializationMethod_WithObjectAdditionalPropertiesBackwardCompatibility() + { + // Create a model with unknown additional properties + var inputModel = InputFactory.Model( + "TestModel", + properties: [InputFactory.Property("Name", InputPrimitiveType.String, isRequired: true)], + additionalProperties: InputPrimitiveType.Any); + + await MockHelpers.LoadMockGeneratorAsync( + inputModels: () => [inputModel], + lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); + + var model = ScmCodeModelGenerator.Instance.TypeFactory.CreateModel(inputModel); + Assert.IsNotNull(model, "Model should be created"); + + var serializations = model!.SerializationProviders.FirstOrDefault() as MrwSerializationTypeDefinition; + Assert.IsNotNull(serializations, "Serialization provider should exist"); + + var deserializationMethod = serializations!.BuildDeserializationMethod(); + Assert.IsNotNull(deserializationMethod, "Deserialization method should be created"); + + var signature = deserializationMethod?.Signature; + Assert.IsNotNull(signature, "Method signature should exist"); + Assert.AreEqual(model.Type, signature?.ReturnType, "Return type should match model type"); + + var methodBody = deserializationMethod?.BodyStatements; + Assert.IsNotNull(methodBody, "Method body should exist"); + + var methodBodyString = methodBody!.ToDisplayString(); + + // Verify variable declaration with object type for backward compatibility + Assert.IsTrue(methodBodyString.Contains("global::System.Collections.Generic.IDictionary additionalProperties"), + "Should declare IDictionary variable for backward compatibility"); + + // Verify dictionary initialization with ChangeTrackingDictionary + Assert.IsTrue(methodBodyString.Contains("new global::Sample.ChangeTrackingDictionary()"), + "Should initialize ChangeTrackingDictionary with object type"); + + // Verify foreach loop over JsonElement properties + Assert.IsTrue(methodBodyString.Contains("foreach (var prop in element.EnumerateObject())"), + "Should enumerate over JsonElement properties"); + + // Verify property name check for known property + Assert.IsTrue(methodBodyString.Contains("prop.NameEquals(\"name\"u8)") || methodBodyString.Contains("prop.NameEquals(\"Name\"u8)"), + "Should check for property name"); + + // Verify GetString() is used for the Name property + Assert.IsTrue(methodBodyString.Contains("prop.Value.GetString()"), + "Should use GetString() for string property deserialization"); + + // Verify GetObject() method is used for additional properties deserialization + Assert.IsTrue(methodBodyString.Contains("additionalProperties.Add(prop.Name, prop.Value.GetObject())"), + "Should use GetObject() for deserializing object values into additionalProperties"); + + // Verify return statement with proper constructor call + Assert.IsTrue(methodBodyString.Contains("return new global::Sample.Models.TestModel(name, additionalProperties)"), + "Return statement should call constructor with name and additionalProperties"); + + // Verify no additionalBinaryDataProperties parameter in constructor call for object type + Assert.IsFalse(methodBodyString.Contains("return new global::Sample.Models.TestModel(name, additionalProperties, additionalBinaryDataProperties)"), + "Should not include additionalBinaryDataProperties parameter when using object type"); + } + + [Test] + public async Task TestBuildJsonModelWriteCore_WithObjectAdditionalPropertiesBackwardCompatibility() + { + // Create a model with unknown additional properties + var inputModel = InputFactory.Model( + "TestModel", + properties: [InputFactory.Property("Name", InputPrimitiveType.String, isRequired: true)], + additionalProperties: InputPrimitiveType.Any); + + await MockHelpers.LoadMockGeneratorAsync( + inputModels: () => [inputModel], + lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); + + var model = ScmCodeModelGenerator.Instance.TypeFactory.CreateModel(inputModel); + Assert.IsNotNull(model, "Model should be created"); + + var serializations = model!.SerializationProviders.FirstOrDefault() as MrwSerializationTypeDefinition; + Assert.IsNotNull(serializations, "Serialization provider should exist"); + + var writeCoreMethod = serializations!.BuildJsonModelWriteCoreMethod(); + Assert.IsNotNull(writeCoreMethod, "Write method should be created"); + + var methodBody = writeCoreMethod?.BodyStatements; + Assert.IsNotNull(methodBody, "Method body should exist"); + + var methodBodyString = methodBody!.ToDisplayString(); + + // Verify that AdditionalProperties property exists and has object type + var additionalPropertiesProperty = model.Properties.FirstOrDefault(p => p.Name == "AdditionalProperties"); + Assert.IsNotNull(additionalPropertiesProperty, "AdditionalProperties property should exist"); + Assert.IsTrue(additionalPropertiesProperty!.Type.IsDictionary, "Property should be a dictionary"); + Assert.AreEqual(typeof(object), additionalPropertiesProperty.Type.Arguments[1].FrameworkType, + "Value type should be object for backward compatibility"); + + // Verify format check + Assert.IsTrue(methodBodyString.Contains("options.Format"), + "Should check options.Format"); + Assert.IsTrue(methodBodyString.Contains("if ((format != \"J\"))"), + "Should validate JSON format"); + Assert.IsTrue(methodBodyString.Contains("throw new global::System.FormatException"), + "Should throw FormatException for unsupported formats"); + + // Verify serialization of the Name property + Assert.IsTrue(methodBodyString.Contains("writer.WritePropertyName(\"name\"u8)") || + methodBodyString.Contains("writer.WritePropertyName(\"Name\"u8)"), + "Should write Name property name"); + Assert.IsTrue(methodBodyString.Contains("writer.WriteStringValue(Name)"), + "Should write Name property value"); + + // Verify foreach loop over AdditionalProperties + var expectedForeachStatement = $"foreach (var item in {additionalPropertiesProperty.Name})"; + Assert.IsTrue(methodBodyString.Contains(expectedForeachStatement), + "Should iterate over AdditionalProperties"); + + // Verify property key is written + Assert.IsTrue(methodBodyString.Contains("writer.WritePropertyName(item.Key)"), + "Should write additional property key"); + + // Verify property value is written with generic WriteObjectValue for object type + Assert.IsTrue(methodBodyString.Contains("writer.WriteObjectValue(item.Value, options)"), + "Should use WriteObjectValue for object-typed additional properties"); + } } } diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/MrwSerializationTypeDefinitions/TestData/AdditionalPropertiesTest/TestBuildDeserializationMethod_WithObjectAdditionalPropertiesBackwardCompatibility/TestModel.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/MrwSerializationTypeDefinitions/TestData/AdditionalPropertiesTest/TestBuildDeserializationMethod_WithObjectAdditionalPropertiesBackwardCompatibility/TestModel.cs new file mode 100644 index 00000000000..8faa5d5597b --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/MrwSerializationTypeDefinitions/TestData/AdditionalPropertiesTest/TestBuildDeserializationMethod_WithObjectAdditionalPropertiesBackwardCompatibility/TestModel.cs @@ -0,0 +1,20 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using System; +using System.Collections.Generic; + +namespace Sample.Models +{ + public partial class TestModel + { + public TestModel(string name, IDictionary additionalProperties) + { + Name = name; + AdditionalProperties = additionalProperties; + } + + public string Name { get; set; } + public IDictionary AdditionalProperties { get; } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/MrwSerializationTypeDefinitions/TestData/AdditionalPropertiesTest/TestBuildJsonModelWriteCore_WithObjectAdditionalPropertiesBackwardCompatibility/TestModel.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/MrwSerializationTypeDefinitions/TestData/AdditionalPropertiesTest/TestBuildJsonModelWriteCore_WithObjectAdditionalPropertiesBackwardCompatibility/TestModel.cs new file mode 100644 index 00000000000..8faa5d5597b --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/MrwSerializationTypeDefinitions/TestData/AdditionalPropertiesTest/TestBuildJsonModelWriteCore_WithObjectAdditionalPropertiesBackwardCompatibility/TestModel.cs @@ -0,0 +1,20 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using System; +using System.Collections.Generic; + +namespace Sample.Models +{ + public partial class TestModel + { + public TestModel(string name, IDictionary additionalProperties) + { + Name = name; + AdditionalProperties = additionalProperties; + } + + public string Name { get; set; } + public IDictionary AdditionalProperties { get; } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelProvider.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelProvider.cs index 2f1fa32e1c5..bd73a087201 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelProvider.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelProvider.cs @@ -54,7 +54,9 @@ protected override FormattableString BuildDescription() private readonly bool _isMultiLevelDiscriminator; private readonly CSharpType _additionalBinaryDataPropsFieldType = typeof(IDictionary); + private readonly CSharpType _additionalObjectPropsFieldType = typeof(IDictionary); private readonly Type _additionalPropsUnknownType = typeof(BinaryData); + private Lazy _useObjectAdditionalProperties; private FieldProvider? _rawDataField; private List? _additionalPropertyFields; private List? _additionalPropertyProperties; @@ -68,6 +70,7 @@ public ModelProvider(InputModelType inputModel) : base(inputModel) { _inputModel = inputModel; _isMultiLevelDiscriminator = ComputeIsMultiLevelDiscriminator(); + _useObjectAdditionalProperties = new Lazy(ShouldUseObjectAdditionalProperties); if (_inputModel.BaseModel is not null) { @@ -125,7 +128,9 @@ public ModelProvider? BaseModelProvider protected FieldProvider? RawDataField => _rawDataField ??= BuildRawDataField(); private List AdditionalPropertyFields => _additionalPropertyFields ??= BuildAdditionalPropertyFields(); private List AdditionalPropertyProperties => _additionalPropertyProperties ??= BuildAdditionalPropertyProperties(); - protected internal bool SupportsBinaryDataAdditionalProperties => AdditionalPropertyProperties.Any(p => p.Type.ElementType.Equals(_additionalPropsUnknownType)); + protected internal bool SupportsBinaryDataAdditionalProperties => AdditionalPropertyProperties.Any(p => + p.Type.ElementType.Equals(_additionalPropsUnknownType) || + (p.Type.ElementType.IsFrameworkType && p.Type.ElementType.FrameworkType == typeof(object))); public ConstructorProvider FullConstructor => _fullConstructor ??= BuildFullConstructor(); protected override string BuildNamespace() => string.IsNullOrEmpty(_inputModel.Namespace) ? @@ -367,9 +372,13 @@ private List BuildAdditionalPropertyProperties() var name = !containsAdditionalTypeProperties ? AdditionalPropertiesHelper.DefaultAdditionalPropertiesPropertyName : RawDataField.Name.ToIdentifierName(); + + // Use object type if backward compatibility requires it, otherwise use BinaryData type + var propertyType = _useObjectAdditionalProperties.Value ? _additionalObjectPropsFieldType : additionalPropsType; var type = !_inputModel.Usage.HasFlag(InputModelTypeUsage.Input) - ? additionalPropsType.OutputType - : additionalPropsType; + ? propertyType.OutputType + : propertyType; + var assignment = type.IsReadOnlyDictionary ? new ExpressionPropertyBody(New.ReadOnlyDictionary(type.Arguments[0], type.ElementType, RawDataField)) : new ExpressionPropertyBody(RawDataField); @@ -1146,9 +1155,12 @@ private ValueExpression GetConversion(PropertyProvider? property = default, Fiel } modifiers |= FieldModifiers.ReadOnly; + // Use object type for backward compatibility if needed, otherwise use BinaryData + var fieldType = _useObjectAdditionalProperties.Value ? _additionalObjectPropsFieldType : _additionalBinaryDataPropsFieldType; + var rawDataField = new FieldProvider( modifiers: modifiers, - type: _additionalBinaryDataPropsFieldType, + type: fieldType, description: FormattableStringHelpers.FromString(AdditionalBinaryDataPropsFieldDescription), name: AdditionalPropertiesHelper.AdditionalBinaryDataPropsFieldName, enclosingType: this); @@ -1175,6 +1187,45 @@ _ when type.Equals(_additionalPropsUnknownType, ignoreNullable: true) => type, }; } + /// + /// Determines whether to use object type for AdditionalProperties based on backward compatibility requirements. + /// Checks if the last contract (previous version) had an AdditionalProperties property of type IDictionary<string, object>. + /// + /// True if object type should be used for backward compatibility; otherwise false (uses BinaryData). + private bool ShouldUseObjectAdditionalProperties() + { + if (LastContractView == null || _inputModel.AdditionalProperties == null) + { + return false; + } + + // Check if the property exists in the last contract by name + var lastContractProperty = LastContractView.Properties.FirstOrDefault(p => + p.Name == AdditionalPropertiesHelper.DefaultAdditionalPropertiesPropertyName); + + if (lastContractProperty == null) + { + return false; + } + + // Check if it's IDictionary + var propertyType = lastContractProperty.Type; + if (propertyType.IsDictionary && propertyType.Arguments.Count == 2) + { + var keyType = propertyType.Arguments[0]; + var valueType = propertyType.Arguments[1]; + + // Check if key is string and value is object + if (keyType.IsFrameworkType && keyType.FrameworkType == typeof(string) && + valueType.IsFrameworkType && valueType.FrameworkType == typeof(object)) + { + return true; + } + } + + return false; + } + private static string BuildAdditionalTypePropertiesFieldName(CSharpType additionalPropertiesValueType) { var name = additionalPropertiesValueType.Name; diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/ModelProviderTests.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/ModelProviderTests.cs index 4ae6598c305..7f6ef7ca815 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/ModelProviderTests.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/ModelProviderTests.cs @@ -1807,5 +1807,37 @@ await MockHelpers.LoadMockGeneratorAsync( Assert.IsNotNull(publicConstructor, "Constructor modifier should be changed to public for backward compatibility"); Assert.AreEqual("baseProp", publicConstructor!.Signature.Parameters[0].Name); } + + [Test] + public async Task TestBuildProperties_WithObjectAdditionalPropertiesBackwardCompatibility() + { + // Create a model with unknown additional properties (which would normally generate BinaryData) + var inputModel = InputFactory.Model( + "TestModel", + usage: InputModelTypeUsage.Input, + properties: [InputFactory.Property("Name", InputPrimitiveType.String, isRequired: true)], + additionalProperties: InputPrimitiveType.Any); + + await MockHelpers.LoadMockGeneratorAsync( + inputModelTypes: [inputModel], + lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); + + var modelProvider = CodeModelGenerator.Instance.TypeFactory.CreateModel(inputModel); + + Assert.IsNotNull(modelProvider); + Assert.IsNotNull(modelProvider!.Properties); + + // Verify that AdditionalProperties property exists and has object type for backward compatibility + var additionalPropertiesProperty = modelProvider.Properties.FirstOrDefault(p => p.Name == "AdditionalProperties"); + Assert.IsNotNull(additionalPropertiesProperty, "AdditionalProperties property should be generated"); + Assert.IsTrue(additionalPropertiesProperty!.IsAdditionalProperties, "Property should be marked as additional properties"); + + // Verify the type is IDictionary for backward compatibility + var propertyType = additionalPropertiesProperty.Type; + Assert.IsTrue(propertyType.IsDictionary, "Property should be a dictionary type"); + Assert.AreEqual(2, propertyType.Arguments.Count, "Dictionary should have 2 type arguments"); + Assert.AreEqual(typeof(string), propertyType.Arguments[0].FrameworkType, "Key type should be string"); + Assert.AreEqual(typeof(object), propertyType.Arguments[1].FrameworkType, "Value type should be object for backward compatibility"); + } } } diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/TestBuildProperties_WithObjectAdditionalPropertiesBackwardCompatibility/TestModel.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/TestBuildProperties_WithObjectAdditionalPropertiesBackwardCompatibility/TestModel.cs new file mode 100644 index 00000000000..8faa5d5597b --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/TestBuildProperties_WithObjectAdditionalPropertiesBackwardCompatibility/TestModel.cs @@ -0,0 +1,20 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using System; +using System.Collections.Generic; + +namespace Sample.Models +{ + public partial class TestModel + { + public TestModel(string name, IDictionary additionalProperties) + { + Name = name; + AdditionalProperties = additionalProperties; + } + + public string Name { get; set; } + public IDictionary AdditionalProperties { get; } + } +} diff --git a/packages/http-client-csharp/generator/docs/backward-compatibility.md b/packages/http-client-csharp/generator/docs/backward-compatibility.md index 2b44139c827..10a5ebd15b1 100644 --- a/packages/http-client-csharp/generator/docs/backward-compatibility.md +++ b/packages/http-client-csharp/generator/docs/backward-compatibility.md @@ -7,6 +7,7 @@ - [Supported Scenarios](#supported-scenarios) - [Model Factory Methods](#model-factory-methods) - [Model Properties](#model-properties) + - [AdditionalProperties Type Preservation](#additionalproperties-type-preservation) - [API Version Enum](#api-version-enum) - [Non-abstract Base Models](#non-abstract-base-models) - [Model Constructors](#model-constructors) @@ -138,6 +139,87 @@ public IReadOnlyList Items { get; } - For read-write lists and dictionaries, if the previous type was different, the previous type is retained - A diagnostic message is logged: `"Changed property {ModelName}.{PropertyName} type to {LastContractType} to match last contract."` +### AdditionalProperties Type Preservation + +The generator maintains backward compatibility for the `AdditionalProperties` property type on models that extend or use `Record`. + +#### Scenario: AdditionalProperties Type Changed from Object to BinaryData + +**Description:** When a model with additional properties was previously generated with `IDictionary`, but the current generator would produce `IDictionary`, the generator preserves the previous `object` type to maintain backward compatibility. + +This commonly occurs when: + +- Migrating from an older generator version that used `object` for unknown additional property values +- Regenerating a library that was originally created with `IDictionary` for `Record` types + +**Example:** + +Previous version generated with object type: + +```csharp +public partial class MyModel +{ + private readonly IDictionary _additionalBinaryDataProperties; + + public MyModel(string name, IDictionary additionalProperties) + { + Name = name; + _additionalBinaryDataProperties = additionalProperties; + } + + public string Name { get; set; } + public IDictionary AdditionalProperties { get; } +} +``` + +Current TypeSpec would generate with BinaryData: + +```csharp +public partial class MyModel +{ + private readonly IDictionary _additionalBinaryDataProperties; + + public MyModel(string name, IDictionary additionalProperties) + { + Name = name; + _additionalBinaryDataProperties = additionalProperties; + } + + public string Name { get; set; } + public IDictionary AdditionalProperties { get; } +} +``` + +**Generated Compatibility Result:** + +When the last contract had `IDictionary`, the generator preserves the object type: + +```csharp +public partial class MyModel +{ + private readonly IDictionary _additionalBinaryDataProperties; + + public MyModel(string name, IDictionary additionalProperties) + { + Name = name; + _additionalBinaryDataProperties = additionalProperties; + } + + public string Name { get; set; } + public IDictionary AdditionalProperties { get; } +} +``` + +**Key Points:** + +- Applies to models with `AdditionalProperties` defined via `Record` or similar patterns +- The backing field type is changed from `IDictionary` to `IDictionary` +- The property type matches the backing field type to avoid compilation errors +- Serialization and deserialization automatically handle both `object` and `BinaryData` types +- For object types, deserialization uses `JsonElement.GetObject()` instead of wrapping in `BinaryData` +- For object types, serialization uses `Utf8JsonWriter.WriteObjectValue()` to handle arbitrary values +- Binary compatibility is fully maintained - existing client code continues to work without recompilation + ### API Version Enum Service version enums maintain backward compatibility by preserving version values from previous releases.