From ff1725c4a833a81b54626f0d6c847edab9e8dd74 Mon Sep 17 00:00:00 2001 From: Jorge Rangel <102122018+jorgerangel-msft@users.noreply.github.com> Date: Fri, 11 Oct 2024 11:37:47 -0500 Subject: [PATCH] Don't generate prop if custom prop exists in base (#4682) This PR address the following bugs: - If a model contains a spec property that was also added as a custom model in it's base model, the derived model will not generate the property. - if a custom property is added to a base model and it includes the `CodeGenSerialization` attribute, that property is included in serialization ctor for the derived model. fixes: https://github.com/microsoft/typespec/issues/4629 --- .../MrwSerializationTypeDefinition.cs | 30 +++- .../ModelCustomizationTests.cs | 47 ++++++ .../SerializationCustomizationTests.cs | 32 +++++ .../BaseModel.cs | 15 ++ ...izeSerializationMethodForPropertyInBase.cs | 136 ++++++++++++++++++ .../BaseModel.cs | 15 ++ .../src/Providers/ModelProvider.cs | 1 + .../src/Providers/TypeProvider.cs | 46 +++++- .../ModelProviders/ModelCustomizationTests.cs | 97 +++++++++++++ .../BaseModel.cs | 11 ++ .../MockInputModel.cs | 10 ++ 11 files changed, 434 insertions(+), 6 deletions(-) create mode 100644 packages/http-client-csharp/generator/Microsoft.Generator.CSharp.ClientModel/test/Providers/MrwSerializationTypeDefinitions/TestData/ModelCustomizationTests/CanSerializeCustomPropertyFromBase/BaseModel.cs create mode 100644 packages/http-client-csharp/generator/Microsoft.Generator.CSharp.ClientModel/test/Providers/MrwSerializationTypeDefinitions/TestData/SerializationCustomizationTests/CanCustomizeSerializationMethodForPropertyInBase.cs create mode 100644 packages/http-client-csharp/generator/Microsoft.Generator.CSharp.ClientModel/test/Providers/MrwSerializationTypeDefinitions/TestData/SerializationCustomizationTests/CanCustomizeSerializationMethodForPropertyInBase/BaseModel.cs create mode 100644 packages/http-client-csharp/generator/Microsoft.Generator.CSharp/test/Providers/ModelProviders/TestData/ModelCustomizationTests/DoesNotGenerateCustomPropertyFromBase/BaseModel.cs create mode 100644 packages/http-client-csharp/generator/Microsoft.Generator.CSharp/test/Providers/ModelProviders/TestData/ModelCustomizationTests/DoesNotGenerateExistingProperty/MockInputModel.cs diff --git a/packages/http-client-csharp/generator/Microsoft.Generator.CSharp.ClientModel/src/Providers/MrwSerializationTypeDefinition.cs b/packages/http-client-csharp/generator/Microsoft.Generator.CSharp.ClientModel/src/Providers/MrwSerializationTypeDefinition.cs index 7399051b5e..4d33051b0f 100644 --- a/packages/http-client-csharp/generator/Microsoft.Generator.CSharp.ClientModel/src/Providers/MrwSerializationTypeDefinition.cs +++ b/packages/http-client-csharp/generator/Microsoft.Generator.CSharp.ClientModel/src/Providers/MrwSerializationTypeDefinition.cs @@ -10,6 +10,7 @@ using System.Linq; using System.Net; using System.Text.Json; +using Microsoft.CodeAnalysis; using Microsoft.Generator.CSharp.ClientModel.Snippets; using Microsoft.Generator.CSharp.Expressions; using Microsoft.Generator.CSharp.Input; @@ -710,6 +711,24 @@ private List BuildDeserializePropertiesStatements(ScopedApi Dictionary> additionalPropsValueKindBodyStatements = []; var parameters = SerializationConstructor.Signature.Parameters; + // Parse the custom serialization attributes + List serializationAttributes = _model.CustomCodeView?.GetAttributes() + .Where(a => a.AttributeClass?.Name == CodeGenAttributes.CodeGenSerializationAttributeName) + .ToList() ?? []; + var baseModelProvider = _model.BaseModelProvider; + + while (baseModelProvider != null) + { + var customCodeView = baseModelProvider.CustomCodeView; + if (customCodeView != null) + { + serializationAttributes + .AddRange(customCodeView.GetAttributes() + .Where(a => a.AttributeClass?.Name == CodeGenAttributes.CodeGenSerializationAttributeName)); + } + baseModelProvider = baseModelProvider.BaseModelProvider; + } + // Create each property's deserialization statement for (int i = 0; i < parameters.Count; i++) { @@ -731,7 +750,7 @@ private List BuildDeserializePropertiesStatements(ScopedApi var propertySerializationName = wireInfo.SerializedName; var checkIfJsonPropEqualsName = new IfStatement(jsonProperty.NameEquals(propertySerializationName)) { - DeserializeProperty(property, jsonProperty) + DeserializeProperty(property, jsonProperty, serializationAttributes) }; propertyDeserializationStatements.Add(checkIfJsonPropEqualsName); } @@ -752,7 +771,7 @@ private List BuildDeserializePropertiesStatements(ScopedApi var rawBinaryData = _rawDataField; if (rawBinaryData == null) { - var baseModelProvider = _model.BaseModelProvider; + baseModelProvider = _model.BaseModelProvider; while (baseModelProvider != null) { var field = baseModelProvider.Fields.FirstOrDefault(f => f.Name == AdditionalPropertiesHelper.AdditionalBinaryDataPropsFieldName); @@ -1033,7 +1052,8 @@ private static SwitchStatement CreateDeserializeAdditionalPropsValueKindCheck( private MethodBodyStatement[] DeserializeProperty( PropertyProvider property, - ScopedApi jsonProperty) + ScopedApi jsonProperty, + IEnumerable serializationAttributes) { var serializationFormat = property.WireInfo?.SerializationFormat ?? SerializationFormat.Default; var propertyVarReference = property.AsVariableExpression; @@ -1043,8 +1063,7 @@ private MethodBodyStatement[] DeserializeProperty( propertyVarReference.Assign(value).Terminate() }; - foreach (var attribute in _model.CustomCodeView?.GetAttributes() - .Where(a => a.AttributeClass?.Name == CodeGenAttributes.CodeGenSerializationAttributeName) ?? []) + foreach (var attribute in serializationAttributes) { if (CodeGenAttributes.TryGetCodeGenSerializationAttributeValue( attribute, @@ -1059,6 +1078,7 @@ private MethodBodyStatement[] DeserializeProperty( deserializationHook, jsonProperty, ByRef(propertyVarReference)).Terminate()]; + break; } } diff --git a/packages/http-client-csharp/generator/Microsoft.Generator.CSharp.ClientModel/test/Providers/MrwSerializationTypeDefinitions/ModelCustomizationTests.cs b/packages/http-client-csharp/generator/Microsoft.Generator.CSharp.ClientModel/test/Providers/MrwSerializationTypeDefinitions/ModelCustomizationTests.cs index 3191a5287b..bcc736508d 100644 --- a/packages/http-client-csharp/generator/Microsoft.Generator.CSharp.ClientModel/test/Providers/MrwSerializationTypeDefinitions/ModelCustomizationTests.cs +++ b/packages/http-client-csharp/generator/Microsoft.Generator.CSharp.ClientModel/test/Providers/MrwSerializationTypeDefinitions/ModelCustomizationTests.cs @@ -38,5 +38,52 @@ public async Task CanChangePropertyName() var expected = Helpers.GetExpectedFromFile(); Assert.AreEqual(Helpers.GetExpectedFromFile(), file.Content); } + + // Validates that if a custom property is added to the base model, and the CodeGenSerialization attribute is used, + // then the derived model includes the custom property in the serialization ctor. + [Test] + public async Task CanSerializeCustomPropertyFromBase() + { + var baseModel = InputFactory.Model( + "baseModel", + usage: InputModelTypeUsage.Input, + properties: [InputFactory.Property("BaseProp", InputPrimitiveType.Int32, isRequired: true)]); + var plugin = await MockHelpers.LoadMockPluginAsync( + inputModels: () => [ + InputFactory.Model( + "mockInputModel", + // use Input so that we generate a public ctor + usage: InputModelTypeUsage.Input, + properties: + [ + InputFactory.Property("OtherProp", InputPrimitiveType.Int32, isRequired: true), + ], + baseModel: baseModel), + ], + compilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); + + var modelTypeProvider = plugin.Object.OutputLibrary.TypeProviders.FirstOrDefault(t => t is ModelProvider && t.Name == "MockInputModel"); + Assert.IsNotNull(modelTypeProvider); + + var baseModelTypeProvider = (modelTypeProvider as ModelProvider)?.BaseModelProvider; + Assert.IsNotNull(baseModelTypeProvider); + var customCodeView = baseModelTypeProvider!.CustomCodeView; + Assert.IsNotNull(customCodeView); + Assert.IsNull(modelTypeProvider!.CustomCodeView); + + Assert.AreEqual(1, baseModelTypeProvider!.Properties.Count); + Assert.AreEqual("BaseProp", baseModelTypeProvider.Properties[0].Name); + Assert.AreEqual(new CSharpType(typeof(int)), baseModelTypeProvider.Properties[0].Type); + Assert.AreEqual(1, customCodeView!.Properties.Count); + Assert.AreEqual("Prop1", customCodeView.Properties[0].Name); + + Assert.AreEqual(1, modelTypeProvider.Properties.Count); + Assert.AreEqual("OtherProp", modelTypeProvider.Properties[0].Name); + + // the custom property should exist in the full ctor + var fullCtor = modelTypeProvider.Constructors.FirstOrDefault(c => c.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Internal)); + Assert.IsNotNull(fullCtor); + Assert.IsTrue(fullCtor!.Signature.Parameters.Any(p => p.Name == "prop1")); + } } } diff --git a/packages/http-client-csharp/generator/Microsoft.Generator.CSharp.ClientModel/test/Providers/MrwSerializationTypeDefinitions/SerializationCustomizationTests.cs b/packages/http-client-csharp/generator/Microsoft.Generator.CSharp.ClientModel/test/Providers/MrwSerializationTypeDefinitions/SerializationCustomizationTests.cs index 3f8ff4525e..62566bf7d1 100644 --- a/packages/http-client-csharp/generator/Microsoft.Generator.CSharp.ClientModel/test/Providers/MrwSerializationTypeDefinitions/SerializationCustomizationTests.cs +++ b/packages/http-client-csharp/generator/Microsoft.Generator.CSharp.ClientModel/test/Providers/MrwSerializationTypeDefinitions/SerializationCustomizationTests.cs @@ -113,6 +113,38 @@ public async Task CanCustomizeSerializationMethodForRenamedProperty() Assert.AreEqual(Helpers.GetExpectedFromFile(), file.Content); } + // Validates that the custom serialization method is used in the serialization provider + // for the custom property that exists in the base model. + [Test] + public async Task CanCustomizeSerializationMethodForPropertyInBase() + { + var baseModel = InputFactory.Model( + "baseModel", + usage: InputModelTypeUsage.Input, + properties: [InputFactory.Property("Prop1", InputPrimitiveType.Int32, isRequired: true)]); + var plugin = await MockHelpers.LoadMockPluginAsync( + inputModels: () => [ + InputFactory.Model( + "mockInputModel", + usage: InputModelTypeUsage.Json, + properties: + [ + InputFactory.Property("OtherProp", InputPrimitiveType.Int32, isRequired: true), + ], + baseModel: baseModel), + ], + compilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); + + var modelProvider = plugin.Object.OutputLibrary.TypeProviders.FirstOrDefault(t => t is ModelProvider); + Assert.IsNotNull(modelProvider); + var serializationProvider = modelProvider!.SerializationProviders.Single(t => t is MrwSerializationTypeDefinition); + Assert.IsNotNull(serializationProvider); + + var writer = new TypeProviderWriter(serializationProvider); + var file = writer.Write(); + Assert.AreEqual(Helpers.GetExpectedFromFile(), file.Content); + } + // Validates that a properties serialization name can be changed using custom code. [Test] public async Task CanChangePropertySerializedName() diff --git a/packages/http-client-csharp/generator/Microsoft.Generator.CSharp.ClientModel/test/Providers/MrwSerializationTypeDefinitions/TestData/ModelCustomizationTests/CanSerializeCustomPropertyFromBase/BaseModel.cs b/packages/http-client-csharp/generator/Microsoft.Generator.CSharp.ClientModel/test/Providers/MrwSerializationTypeDefinitions/TestData/ModelCustomizationTests/CanSerializeCustomPropertyFromBase/BaseModel.cs new file mode 100644 index 0000000000..9d73402453 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.Generator.CSharp.ClientModel/test/Providers/MrwSerializationTypeDefinitions/TestData/ModelCustomizationTests/CanSerializeCustomPropertyFromBase/BaseModel.cs @@ -0,0 +1,15 @@ +using Sample; +using System; +using System.Collections.Generic; +using Microsoft.Generator.CSharp.Customization; + +namespace Sample.Models; + +[CodeGenSerialization(nameof(Prop1), DeserializationValueHook = nameof(DeserializationMethod))] +public partial class BaseModel +{ + internal string Prop1 { get; set; } + + private static void DeserializationMethod(JsonProperty property, ref string fieldValue) + => fieldValue = property.Value.GetString(); +} diff --git a/packages/http-client-csharp/generator/Microsoft.Generator.CSharp.ClientModel/test/Providers/MrwSerializationTypeDefinitions/TestData/SerializationCustomizationTests/CanCustomizeSerializationMethodForPropertyInBase.cs b/packages/http-client-csharp/generator/Microsoft.Generator.CSharp.ClientModel/test/Providers/MrwSerializationTypeDefinitions/TestData/SerializationCustomizationTests/CanCustomizeSerializationMethodForPropertyInBase.cs new file mode 100644 index 0000000000..35f4485dc7 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.Generator.CSharp.ClientModel/test/Providers/MrwSerializationTypeDefinitions/TestData/SerializationCustomizationTests/CanCustomizeSerializationMethodForPropertyInBase.cs @@ -0,0 +1,136 @@ +// + +#nullable disable + +using System; +using System.ClientModel; +using System.ClientModel.Primitives; +using System.Collections.Generic; +using System.Text.Json; +using Sample; + +namespace Sample.Models +{ + /// + public partial class MockInputModel : global::System.ClientModel.Primitives.IJsonModel + { + internal MockInputModel() + { + } + + void global::System.ClientModel.Primitives.IJsonModel.Write(global::System.Text.Json.Utf8JsonWriter writer, global::System.ClientModel.Primitives.ModelReaderWriterOptions options) + { + writer.WriteStartObject(); + this.JsonModelWriteCore(writer, options); + writer.WriteEndObject(); + } + + /// The JSON writer. + /// The client options for reading and writing models. + protected override void JsonModelWriteCore(global::System.Text.Json.Utf8JsonWriter writer, global::System.ClientModel.Primitives.ModelReaderWriterOptions options) + { + string format = (options.Format == "W") ? ((global::System.ClientModel.Primitives.IPersistableModel)this).GetFormatFromOptions(options) : options.Format; + if ((format != "J")) + { + throw new global::System.FormatException($"The model {nameof(global::Sample.Models.MockInputModel)} does not support writing '{format}' format."); + } + base.JsonModelWriteCore(writer, options); + writer.WritePropertyName("otherProp"u8); + writer.WriteNumberValue(OtherProp); + } + + global::Sample.Models.MockInputModel global::System.ClientModel.Primitives.IJsonModel.Create(ref global::System.Text.Json.Utf8JsonReader reader, global::System.ClientModel.Primitives.ModelReaderWriterOptions options) => ((global::Sample.Models.MockInputModel)this.JsonModelCreateCore(ref reader, options)); + + /// The JSON reader. + /// The client options for reading and writing models. + protected override global::Sample.Models.BaseModel JsonModelCreateCore(ref global::System.Text.Json.Utf8JsonReader reader, global::System.ClientModel.Primitives.ModelReaderWriterOptions options) + { + string format = (options.Format == "W") ? ((global::System.ClientModel.Primitives.IPersistableModel)this).GetFormatFromOptions(options) : options.Format; + if ((format != "J")) + { + throw new global::System.FormatException($"The model {nameof(global::Sample.Models.MockInputModel)} does not support reading '{format}' format."); + } + using global::System.Text.Json.JsonDocument document = global::System.Text.Json.JsonDocument.ParseValue(ref reader); + return global::Sample.Models.MockInputModel.DeserializeMockInputModel(document.RootElement, options); + } + + internal static global::Sample.Models.MockInputModel DeserializeMockInputModel(global::System.Text.Json.JsonElement element, global::System.ClientModel.Primitives.ModelReaderWriterOptions options) + { + if ((element.ValueKind == global::System.Text.Json.JsonValueKind.Null)) + { + return null; + } + int otherProp = default; + int prop1 = default; + global::System.Collections.Generic.IDictionary additionalBinaryDataProperties = new global::Sample.ChangeTrackingDictionary(); + foreach (var prop in element.EnumerateObject()) + { + if (prop.NameEquals("otherProp"u8)) + { + otherProp = prop.Value.GetInt32(); + continue; + } + if (prop.NameEquals("prop1"u8)) + { + DeserializationMethod(prop, ref prop1); + continue; + } + if ((options.Format != "W")) + { + additionalBinaryDataProperties.Add(prop.Name, global::System.BinaryData.FromString(prop.Value.GetRawText())); + } + } + return new global::Sample.Models.MockInputModel(otherProp, prop1, additionalBinaryDataProperties); + } + + global::System.BinaryData global::System.ClientModel.Primitives.IPersistableModel.Write(global::System.ClientModel.Primitives.ModelReaderWriterOptions options) => this.PersistableModelWriteCore(options); + + /// The client options for reading and writing models. + protected override global::System.BinaryData PersistableModelWriteCore(global::System.ClientModel.Primitives.ModelReaderWriterOptions options) + { + string format = (options.Format == "W") ? ((global::System.ClientModel.Primitives.IPersistableModel)this).GetFormatFromOptions(options) : options.Format; + switch (format) + { + case "J": + return global::System.ClientModel.Primitives.ModelReaderWriter.Write(this, options); + default: + throw new global::System.FormatException($"The model {nameof(global::Sample.Models.MockInputModel)} does not support writing '{options.Format}' format."); + } + } + + global::Sample.Models.MockInputModel global::System.ClientModel.Primitives.IPersistableModel.Create(global::System.BinaryData data, global::System.ClientModel.Primitives.ModelReaderWriterOptions options) => ((global::Sample.Models.MockInputModel)this.PersistableModelCreateCore(data, options)); + + /// The data to parse. + /// The client options for reading and writing models. + protected override global::Sample.Models.BaseModel PersistableModelCreateCore(global::System.BinaryData data, global::System.ClientModel.Primitives.ModelReaderWriterOptions options) + { + string format = (options.Format == "W") ? ((global::System.ClientModel.Primitives.IPersistableModel)this).GetFormatFromOptions(options) : options.Format; + switch (format) + { + case "J": + using (global::System.Text.Json.JsonDocument document = global::System.Text.Json.JsonDocument.Parse(data)) + { + return global::Sample.Models.MockInputModel.DeserializeMockInputModel(document.RootElement, options); + } + default: + throw new global::System.FormatException($"The model {nameof(global::Sample.Models.MockInputModel)} does not support reading '{options.Format}' format."); + } + } + + string global::System.ClientModel.Primitives.IPersistableModel.GetFormatFromOptions(global::System.ClientModel.Primitives.ModelReaderWriterOptions options) => "J"; + + /// The to serialize into . + public static implicit operator BinaryContent(global::Sample.Models.MockInputModel mockInputModel) + { + return global::System.ClientModel.BinaryContent.Create(mockInputModel, global::Sample.ModelSerializationExtensions.WireOptions); + } + + /// The to deserialize the from. + public static explicit operator MockInputModel(global::System.ClientModel.ClientResult result) + { + using global::System.ClientModel.Primitives.PipelineResponse response = result.GetRawResponse(); + using global::System.Text.Json.JsonDocument document = global::System.Text.Json.JsonDocument.Parse(response.Content); + return global::Sample.Models.MockInputModel.DeserializeMockInputModel(document.RootElement, global::Sample.ModelSerializationExtensions.WireOptions); + } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.Generator.CSharp.ClientModel/test/Providers/MrwSerializationTypeDefinitions/TestData/SerializationCustomizationTests/CanCustomizeSerializationMethodForPropertyInBase/BaseModel.cs b/packages/http-client-csharp/generator/Microsoft.Generator.CSharp.ClientModel/test/Providers/MrwSerializationTypeDefinitions/TestData/SerializationCustomizationTests/CanCustomizeSerializationMethodForPropertyInBase/BaseModel.cs new file mode 100644 index 0000000000..feb7205576 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.Generator.CSharp.ClientModel/test/Providers/MrwSerializationTypeDefinitions/TestData/SerializationCustomizationTests/CanCustomizeSerializationMethodForPropertyInBase/BaseModel.cs @@ -0,0 +1,15 @@ + +using Microsoft.Generator.CSharp.Customization; + +namespace Sample.Models +{ + [CodeGenSerialization(nameof(Prop1), SerializationValueHook = nameof(SerializationMethod), DeserializationValueHook = nameof(DeserializationMethod))] + public partial class BaseModel + { + private void SerializationMethod(Utf8JsonWriter writer, ModelReaderWriterOptions options) + => writer.WriteObjectValue(Prop1, options); + + private static void DeserializationMethod(JsonProperty property, ref string fieldValue) + => fieldValue = property.Value.GetString(); + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/src/Providers/ModelProvider.cs b/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/src/Providers/ModelProvider.cs index 030367ebff..5db762628e 100644 --- a/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/src/Providers/ModelProvider.cs +++ b/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/src/Providers/ModelProvider.cs @@ -75,6 +75,7 @@ private IReadOnlyList BuildDerivedModels() return [.. derivedModels]; } + internal override TypeProvider? BaseTypeProvider => BaseModelProvider; public ModelProvider? BaseModelProvider => _baseModelProvider ??= (_baseTypeProvider?.Value is ModelProvider baseModelProvider ? baseModelProvider : null); diff --git a/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/src/Providers/TypeProvider.cs b/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/src/Providers/TypeProvider.cs index 996b686736..c16966262c 100644 --- a/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/src/Providers/TypeProvider.cs +++ b/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/src/Providers/TypeProvider.cs @@ -7,6 +7,7 @@ using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp; using Microsoft.Generator.CSharp.Expressions; +using Microsoft.Generator.CSharp.Input; using Microsoft.Generator.CSharp.Primitives; using Microsoft.Generator.CSharp.SourceInput; using Microsoft.Generator.CSharp.Statements; @@ -119,6 +120,8 @@ private TypeSignatureModifiers GetDeclarationModifiersInternal() return modifiers; } + internal virtual TypeProvider? BaseTypeProvider => null; + protected virtual CSharpType? GetBaseType() => null; public virtual WhereExpression? WhereClause { get; protected init; } @@ -159,17 +162,58 @@ private PropertyProvider[] BuildPropertiesInternal() var properties = new List(); var customProperties = new Dictionary(); var renamedProperties = new Dictionary(); - foreach (var customProperty in CustomCodeView?.Properties ?? []) + var allCustomProperties = CustomCodeView?.Properties != null + ? new List(CustomCodeView.Properties) + : []; + var baseTypeCustomCodeView = BaseTypeProvider?.CustomCodeView; + + // add all custom properties from base types + while (baseTypeCustomCodeView != null) + { + allCustomProperties.AddRange(baseTypeCustomCodeView.Properties); + baseTypeCustomCodeView = baseTypeCustomCodeView.BaseTypeProvider?.CustomCodeView; + } + + foreach (var customProperty in allCustomProperties) { customProperties.Add(customProperty.Name, customProperty); + bool isRenamedProperty = false; + foreach (var attribute in customProperty.Attributes ?? []) { if (CodeGenAttributes.TryGetCodeGenMemberAttributeValue(attribute, out var originalName)) { renamedProperties.Add(originalName, customProperty); + isRenamedProperty = true; + } + } + + // Handle custom serializable properties + if (!isRenamedProperty && customProperty.WireInfo == null) + { + foreach (var attribute in GetCodeGenSerializationAttributes()) + { + if (CodeGenAttributes.TryGetCodeGenSerializationAttributeValue( + attribute, + out var propertyName, + out string? serializationName, + out _, + out _, + out _) && propertyName == customProperty.Name) + { + customProperty.WireInfo = new( + SerializationFormat.Default, + false, + !customProperty.Body.HasSetter, + customProperty.Type.IsNullable, + false, + serializationName ?? customProperty.Name.ToVariableName()); + break; + } } } } + foreach (var property in BuildProperties()) { if (ShouldGenerate(property, customProperties, renamedProperties)) diff --git a/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/test/Providers/ModelProviders/ModelCustomizationTests.cs b/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/test/Providers/ModelProviders/ModelCustomizationTests.cs index 348fbf4813..26136558eb 100644 --- a/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/test/Providers/ModelProviders/ModelCustomizationTests.cs +++ b/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/test/Providers/ModelProviders/ModelCustomizationTests.cs @@ -1,6 +1,7 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. +using System.Collections.Generic; using System.Linq; using System.Threading.Tasks; using Microsoft.Generator.CSharp.Input; @@ -326,6 +327,102 @@ await MockHelpers.LoadMockPluginAsync( Assert.AreEqual("SkyBlue", customCodeView?.Fields[2].Name); } + // Validates that if a spec property is customized, then the property is not generated and the custom property + // is used instead + [Test] + public async Task DoesNotGenerateExistingProperty() + { + var plugin = await MockHelpers.LoadMockPluginAsync( + inputModelTypes: [ + InputFactory.Model( + "mockInputModel", + // use Input so that we generate a public ctor + usage: InputModelTypeUsage.Input, + properties: + [ + InputFactory.Property("Prop1", InputFactory.Array(InputPrimitiveType.String), isRequired: true), + ]) + ], + compilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); + var csharpGen = new CSharpGen(); + await csharpGen.ExecuteAsync(); + + var modelTypeProvider = plugin.Object.OutputLibrary.TypeProviders.FirstOrDefault(t => t is ModelProvider); + Assert.IsNotNull(modelTypeProvider); + var customCodeView = modelTypeProvider!.CustomCodeView; + + AssertCommon(modelTypeProvider, "Sample.Models", "MockInputModel"); + + // the custom property shouldn't be added to the model provider + Assert.AreEqual(0, modelTypeProvider.Properties.Count); + + // the custom property should still be parameters of the model's ctor + var modelCtors = modelTypeProvider.Constructors; + foreach (var ctor in modelCtors) + { + Assert.IsTrue(ctor.Signature.Parameters.Any(p => p.Name == "prop1")); + } + + // the custom property should be added to the custom code view + Assert.AreEqual(1, customCodeView!.Properties.Count); + Assert.AreEqual("Prop1", customCodeView.Properties[0].Name); + Assert.AreEqual(new CSharpType(typeof(IList)), customCodeView.Properties[0].Type); + Assert.AreEqual(MethodSignatureModifiers.Internal, customCodeView.Properties[0].Modifiers); + Assert.IsTrue(customCodeView.Properties[0].Body.HasSetter); + } + + // Validates that if a custom property is added to the base model, and a property with the same name exists in the derived model, + // then the derived model property is not generated and the custom property is used instead. + [Test] + public async Task DoesNotGenerateCustomPropertyFromBase() + { + var baseModel = InputFactory.Model( + "baseModel", + usage: InputModelTypeUsage.Input, + properties: [InputFactory.Property("BaseProp", InputPrimitiveType.Int32, isRequired: true)]); + var plugin = await MockHelpers.LoadMockPluginAsync( + inputModelTypes: [ + InputFactory.Model( + "mockInputModel", + // use Input so that we generate a public ctor + usage: InputModelTypeUsage.Input, + properties: + [ + InputFactory.Property("Prop1", InputPrimitiveType.Int32, isRequired: true), + ], + baseModel: baseModel), + ], + compilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); + var csharpGen = new CSharpGen(); + await csharpGen.ExecuteAsync(); + + var modelTypeProvider = plugin.Object.OutputLibrary.TypeProviders.FirstOrDefault(t => t is ModelProvider && t.Name == "MockInputModel"); + Assert.IsNotNull(modelTypeProvider); + + var baseModelTypeProvider = (modelTypeProvider as ModelProvider)?.BaseModelProvider; + Assert.IsNotNull(baseModelTypeProvider); + var customCodeView = baseModelTypeProvider!.CustomCodeView; + Assert.IsNotNull(customCodeView); + Assert.IsNull(modelTypeProvider!.CustomCodeView); + + AssertCommon(baseModelTypeProvider, "Sample.Models", "BaseModel"); + + Assert.AreEqual(1, baseModelTypeProvider!.Properties.Count); + Assert.AreEqual("BaseProp", baseModelTypeProvider.Properties[0].Name); + Assert.AreEqual(new CSharpType(typeof(int)), baseModelTypeProvider.Properties[0].Type); + Assert.AreEqual(1, customCodeView!.Properties.Count); + Assert.AreEqual("Prop1", customCodeView.Properties[0].Name); + // the spec property shouldn't be added to the model provider since a custom property with the same name exists + Assert.AreEqual(0, modelTypeProvider.Properties.Count); + + // the custom property should not be parameters of the model's ctor + var modelCtors = modelTypeProvider.Constructors; + foreach (var ctor in modelCtors) + { + Assert.IsFalse(ctor.Signature.Parameters.Any(p => p.Name == "prop1")); + } + } + [Test] public async Task CanReplaceConstructor() { diff --git a/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/test/Providers/ModelProviders/TestData/ModelCustomizationTests/DoesNotGenerateCustomPropertyFromBase/BaseModel.cs b/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/test/Providers/ModelProviders/TestData/ModelCustomizationTests/DoesNotGenerateCustomPropertyFromBase/BaseModel.cs new file mode 100644 index 0000000000..6a1966c559 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/test/Providers/ModelProviders/TestData/ModelCustomizationTests/DoesNotGenerateCustomPropertyFromBase/BaseModel.cs @@ -0,0 +1,11 @@ +using Sample; +using System; +using System.Collections.Generic; +using Microsoft.Generator.CSharp.Customization; + +namespace Sample.Models; + +public partial class BaseModel +{ + internal int Prop1 { get; set; } +} diff --git a/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/test/Providers/ModelProviders/TestData/ModelCustomizationTests/DoesNotGenerateExistingProperty/MockInputModel.cs b/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/test/Providers/ModelProviders/TestData/ModelCustomizationTests/DoesNotGenerateExistingProperty/MockInputModel.cs new file mode 100644 index 0000000000..584f468555 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/test/Providers/ModelProviders/TestData/ModelCustomizationTests/DoesNotGenerateExistingProperty/MockInputModel.cs @@ -0,0 +1,10 @@ +using System; +using System.Collections.Generic; +using Microsoft.Generator.CSharp.Customization; + +namespace Sample.Models; + +public partial class MockInputModel +{ + internal IList Prop1 { get; set; } +}