fix(http-client-csharp): override MRW *Core methods when external base declares them#11044
Merged
JoshLove-msft merged 2 commits intoJun 22, 2026
Conversation
…e declares them Issue microsoft#11042: derived models of an external/referenced MRW-generated base (e.g. OpenAI's ResponseItem) generated PersistableModelCreateCore and the other serialization *Core methods as `protected virtual` instead of `protected override`, causing CS0114. PR microsoft#11002 routed external models through SystemObjectModelProvider, whose ShouldSkipDerivedSerializationMethodOverrides unconditionally returns true (added in microsoft#9862 for hand-authored bases like ResourceData that lack the *Core methods). That forced derived models to hide rather than override the base's *Core methods. Now MrwSerializationTypeDefinition only skips the override when the wrapped framework type does not declare all four generated *Core methods; when it does, derived models correctly emit `protected override`. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
JoshLove-msft
requested review from
ShivangiReja,
jorgerangel-msft,
joseharriaga,
jsquire and
m-nash
as code owners
microsoft-github-policy-service
Bot
added
the
emitter:client:csharp
commit: |
Replace the reflective *Core method-name probe with a check of whether the external base type implements IJsonModel<T> or IPersistableModel<T>. This keys off the public model-reader-writer contract instead of protected implementation details, and drops the System.Reflection BindingFlags scan. The test's FakeMrwBase now implements IJsonModel<T> to mirror a generated model. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
|
No changes needing a change description found. |
jsquire
approved these changes
Jun 22, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #11042
Problem
Models derived from an external/referenced MRW-generated base type (e.g.
Azure.AI.Extensions.OpenAItypes deriving from OpenAI'sResponseItem) generatedPersistableModelCreateCoreand the other serialization*Coremethods asprotected virtualinstead ofprotected override. The compiler then reports CS0114 (hides inherited member ... add the override keyword).Root cause
PR #11002 routes external
InputModelTypes throughSystemObjectModelProviderso derived models get a realModelProviderbase. HoweverSystemObjectModelProvider.ShouldSkipDerivedSerializationMethodOverridesunconditionally returnstrue(introduced in #9862 for hand-authored ARM bases likeResourceDatathat do not follow the generated MRW pattern). Reusing it for all external bases forced derived models to hide rather than override even when the external base is itself MRW-generated and exposes the overridable*Coremethods.Fix
MrwSerializationTypeDefinitionnow decides whether to skip the derived overrides based on whether the external base actually participates in model-reader-writer serialization. When the base is aSystemObjectModelProvider, it inspects the wrapped framework/referenced type: if that type implementsIJsonModel<T>orIPersistableModel<T>, it follows the generated MRW pattern and exposes the overridable*Coremethods, so the derived model emitsprotected override. Otherwise existing behavior is preserved (bases that don't implement those interfaces, e.g.object/ResourceData, still emitvirtual).The check keys off the public model-reader-writer contract (the
IJsonModel<T>/IPersistableModel<T>interfaces) rather than probing for protected*Coremethod names, so it doesn't depend on serialization implementation details.Tests
Added regression tests in
SystemObjectModelSerializationTestscovering an external base that implementsIJsonModel<T>, asserting the four*Coremethods are emitted asoverride. Existing tests (object base -> virtual) remain unchanged. Full ClientModel suite (1435 tests) passes.