Skip to content

Commit 60f496e

Browse files
live1206Copilot
andcommitted
Fix MRW JSON write core override for custom bases
Detect overridable JsonModelWriteCore methods on custom and framework base types so derived model serialization emits override instead of hiding the base member. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent ae88584 commit 60f496e

3 files changed

Lines changed: 40 additions & 12 deletions

File tree

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ private MethodProvider BuildXmlModelWriteCoreMethod()
6767
MethodSignatureModifiers modifiers = _isStruct
6868
? MethodSignatureModifiers.Private
6969
: MethodSignatureModifiers.Internal | MethodSignatureModifiers.Virtual;
70-
if (_shouldOverrideMethods)
70+
if (ShouldOverrideMethods)
7171
{
7272
modifiers = MethodSignatureModifiers.Internal | MethodSignatureModifiers.Override;
7373
}
@@ -81,7 +81,7 @@ private MethodProvider BuildXmlModelWriteCoreMethod()
8181

8282
private MethodBodyStatement[] BuildXmlModelWriteCoreMethodBody()
8383
{
84-
var categorizedProperties = _shouldOverrideMethods
84+
var categorizedProperties = ShouldOverrideMethods
8585
? CategorizedXmlProperties
8686
: AllCategorizedXmlProperties;
8787
var statements = new List<MethodBodyStatement>
@@ -90,7 +90,7 @@ private MethodBodyStatement[] BuildXmlModelWriteCoreMethodBody()
9090
MethodBodyStatement.EmptyLine
9191
};
9292

93-
if (_shouldOverrideMethods)
93+
if (ShouldOverrideMethods)
9494
{
9595
statements.Add(Base.Invoke(XmlModelWriteCoreMethodName, _xmlWriterParameter, _serializationOptionsParameter).Terminate());
9696
}

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

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -67,8 +67,8 @@ public partial class MrwSerializationTypeDefinition : TypeProvider
6767
private readonly bool _supportsXml;
6868
private ConstructorProvider? _serializationConstructor;
6969
// Flag to determine if the model should override the serialization methods
70-
private readonly bool _shouldOverrideMethods;
71-
private readonly bool _shouldSkipDerivedSerializationMethodOverrides;
70+
private bool ShouldOverrideMethods => _model.BaseModelProvider != null && !_isStruct;
71+
private bool ShouldSkipSerializationMethodOverrides => ShouldSkipDerivedSerializationMethodOverrides(_model.BaseModelProvider);
7272
private readonly Lazy<PropertyProvider[]> _additionalProperties;
7373

7474
private CSharpType RootType => _rootType ??= GetRootModelType();
@@ -93,8 +93,6 @@ public MrwSerializationTypeDefinition(InputModelType inputModel, ModelProvider m
9393
_rawDataField = _model.Fields.FirstOrDefault(f => f.Name == AdditionalPropertiesHelper.AdditionalBinaryDataPropsFieldName);
9494
_additionalBinaryDataProperty = new(GetAdditionalBinaryDataPropertiesProp);
9595
_additionalProperties = new(() => [.. _model.Properties.Where(p => p.IsAdditionalProperties)]);
96-
_shouldOverrideMethods = _model.BaseModelProvider != null && !_isStruct;
97-
_shouldSkipDerivedSerializationMethodOverrides = ShouldSkipDerivedSerializationMethodOverrides(_model.BaseModelProvider);
9896
_utf8JsonWriterSnippet = _utf8JsonWriterParameter.As<Utf8JsonWriter>();
9997
_mrwOptionsParameterSnippet = _serializationOptionsParameter.As<ModelReaderWriterOptions>();
10098
_jsonElementParameterSnippet = _jsonElementDeserializationParam.As<JsonElement>();
@@ -530,7 +528,7 @@ internal MethodProvider BuildJsonModelWriteCoreMethod()
530528
MethodSignatureModifiers modifiers = _isStruct
531529
? MethodSignatureModifiers.Private
532530
: MethodSignatureModifiers.Protected | MethodSignatureModifiers.Virtual;
533-
if (_shouldOverrideMethods)
531+
if (ShouldOverrideMethods)
534532
{
535533
modifiers = MethodSignatureModifiers.Protected | MethodSignatureModifiers.Override;
536534
}
@@ -552,7 +550,7 @@ internal MethodProvider BuildPersistableModelWriteCoreMethod()
552550
? MethodSignatureModifiers.Private
553551
: MethodSignatureModifiers.Protected | MethodSignatureModifiers.Virtual;
554552

555-
if (_shouldOverrideMethods && !_shouldSkipDerivedSerializationMethodOverrides)
553+
if (ShouldOverrideMethods && !ShouldSkipSerializationMethodOverrides)
556554
{
557555
modifiers = MethodSignatureModifiers.Protected | MethodSignatureModifiers.Override;
558556
}
@@ -576,7 +574,7 @@ internal MethodProvider BuildPersistableModelCreateCoreMethod()
576574
? MethodSignatureModifiers.Private
577575
: MethodSignatureModifiers.Protected | MethodSignatureModifiers.Virtual;
578576

579-
if (_shouldOverrideMethods && !_shouldSkipDerivedSerializationMethodOverrides)
577+
if (ShouldOverrideMethods && !ShouldSkipSerializationMethodOverrides)
580578
{
581579
modifiers = MethodSignatureModifiers.Protected | MethodSignatureModifiers.Override;
582580
}
@@ -624,7 +622,7 @@ internal MethodProvider BuildJsonModelCreateCoreMethod()
624622
? MethodSignatureModifiers.Private
625623
: MethodSignatureModifiers.Protected | MethodSignatureModifiers.Virtual;
626624

627-
if (_shouldOverrideMethods && !_shouldSkipDerivedSerializationMethodOverrides)
625+
if (ShouldOverrideMethods && !ShouldSkipSerializationMethodOverrides)
628626
{
629627
modifiers = MethodSignatureModifiers.Protected | MethodSignatureModifiers.Override;
630628
}
@@ -1055,7 +1053,7 @@ private MethodBodyStatement[] BuildPersistableModelCreateCoreMethodBody()
10551053
private MethodBodyStatement CallBaseJsonModelWriteCore(bool isDynamicModelWithNonDynamicBase)
10561054
{
10571055
// base.<JsonModelWriteCore>()
1058-
bool callBaseWriteMethod = _shouldOverrideMethods
1056+
bool callBaseWriteMethod = ShouldOverrideMethods
10591057
&& (_jsonPatchProperty is null || !isDynamicModelWithNonDynamicBase);
10601058
return callBaseWriteMethod ?
10611059
Base.Invoke(JsonModelWriteCoreMethodName, [_utf8JsonWriterParameter, _serializationOptionsParameter]).Terminate()

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

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,27 @@ public void JsonModelWriteCore_IsOverride_WhenBaseIsRegularModel()
121121
"JsonModelWriteCore should be 'override' with regular base too");
122122
}
123123

124+
[Test]
125+
public void JsonModelWriteCore_IsOverride_WhenBaseProviderIsResolvedAfterSerialization()
126+
{
127+
var baseInputModel = InputFactory.Model("Resource");
128+
var derivedInputModel = InputFactory.Model("TrackedResource", properties: [InputFactory.Property("Location", InputPrimitiveType.String)]);
129+
MockHelpers.LoadMockGenerator(inputModels: () => [baseInputModel, derivedInputModel]);
130+
131+
var derived = new DelayedBaseModelProvider(derivedInputModel);
132+
var serialization = new MrwSerializationTypeDefinition(derivedInputModel, derived);
133+
134+
// The serialization provider can be constructed before later visitors/customization
135+
// resolution make the base model provider available.
136+
derived.BaseModel = new SystemObjectModelProvider(new CSharpType(typeof(object)), baseInputModel);
137+
138+
var method = serialization.BuildJsonModelWriteCoreMethod();
139+
140+
Assert.IsTrue(method.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Override),
141+
"JsonModelWriteCore should evaluate BaseModelProvider when the method is built, not when serialization is constructed");
142+
Assert.IsFalse(method.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Virtual));
143+
}
144+
124145
// -------------------------------------------------------------------
125146
// PersistableModelWriteCore: 'virtual' with system base, 'override' with regular
126147
// (the framework base already implements this; derived model re-introduces it)
@@ -328,5 +349,14 @@ FakeMrwBase IPersistableModel<FakeMrwBase>.Create(BinaryData data, ModelReaderWr
328349
string IPersistableModel<FakeMrwBase>.GetFormatFromOptions(ModelReaderWriterOptions options)
329350
=> "J";
330351
}
352+
353+
private class DelayedBaseModelProvider(InputModelType inputModel) : ModelProvider(inputModel)
354+
{
355+
public ModelProvider? BaseModel { get; set; }
356+
357+
protected override ModelProvider? BuildBaseModelProvider() => BaseModel;
358+
359+
protected override CSharpType? BuildBaseType() => BaseModel?.Type;
360+
}
331361
}
332362
}

0 commit comments

Comments
 (0)