Skip to content

MVC / Razor Pages support for unions#67005

Open
DeagleGross wants to merge 1 commit into
dotnet:mainfrom
DeagleGross:dmkorolev/unions/mvc+razor
Open

MVC / Razor Pages support for unions#67005
DeagleGross wants to merge 1 commit into
dotnet:mainfrom
DeagleGross:dmkorolev/unions/mvc+razor

Conversation

@DeagleGross
Copy link
Copy Markdown
Member

A follow-up of #66951 and #67001.

This PR adds tests to verify MVC and Razor Pages flow of union processing works, and different union usage cases are covered. MVC is a lower priority than Minimal API, but since the support is cheap, I am proposing to add coverage for MVC / Razor Pages as well.

Closes #66545

@DeagleGross DeagleGross self-assigned this Jun 3, 2026
Copilot AI review requested due to automatic review settings June 3, 2026 17:46
@DeagleGross DeagleGross requested a review from a team as a code owner June 3, 2026 17:46
@DeagleGross DeagleGross added the area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates label Jun 3, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds functional coverage to ensure C# union types flow correctly through MVC’s System.Text.Json input/output formatters and through Razor Pages handler binding + JsonResult responses, complementing the existing Minimal API union coverage.

Changes:

  • Adds a new FormatterWebSite controller + model types to exercise union binding/serialization scenarios via MVC.
  • Adds MVC functional tests for union request-body binding (input formatter) and union return-type serialization (output formatter).
  • Adds a Razor Pages smoke test site page + functional test to validate unions behave similarly in Razor Pages.
Show a summary per file
File Description
src/Mvc/test/WebSites/RazorPagesWebSite/Pages/Unions.cshtml.cs Adds a Razor Page handler that returns and accepts a union via [FromBody].
src/Mvc/test/WebSites/RazorPagesWebSite/Pages/Unions.cshtml Adds the Razor Page endpoint for the unions smoke test.
src/Mvc/test/WebSites/FormatterWebSite/Models/UnionTypes.cs Defines a set of union types (primitive/object/nested/classifier) used by MVC functional coverage.
src/Mvc/test/WebSites/FormatterWebSite/Controllers/UnionsController.cs Adds MVC endpoints to serialize unions and echo unions back (input + output path).
src/Mvc/test/Mvc.FunctionalTests/SystemTextJsonOutputFormatterTest.Unions.cs Adds output-formatter functional tests for union return types.
src/Mvc/test/Mvc.FunctionalTests/SystemTextJsonOutputFormatterTest.cs Marks the test class partial to split union tests into a separate file.
src/Mvc/test/Mvc.FunctionalTests/SystemTextJsonInputFormatterTest.Unions.cs Adds input-formatter functional tests for union request bodies (including ambiguous cases).
src/Mvc/test/Mvc.FunctionalTests/SystemTextJsonInputFormatterTest.cs Marks the test class partial to split union tests into a separate file.
src/Mvc/test/Mvc.FunctionalTests/RazorPagesUnionsTest.cs Adds functional tests validating union behavior through the Razor Pages pipeline.

Copilot's findings

  • Files reviewed: 9/9 changed files
  • Comments generated: 1

// [Theory]
// [InlineData("42", "42")]
// [InlineData("\"hi\"", "\"hi\"")]
// // TODO: enable null case after https://github.com/dotnet/runtime/issues/128688 is fixed.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we uncomment and skip the test instead ([Theory(Skip = "...")])?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll uncomment this fully when #67024 merges, since it contains runtime fixes

{
JsonTokenType.Number => typeof(int),
JsonTokenType.String => typeof(string),
JsonTokenType.Null => typeof(int),
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
JsonTokenType.Null => typeof(int),
JsonTokenType.Null => typeof(int?),

public override JsonTypeClassifier CreateJsonClassifier(JsonTypeClassifierContext context, JsonSerializerOptions options) =>
static (ref Utf8JsonReader reader) => reader.TokenType switch
{
JsonTokenType.Number => typeof(int),
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
JsonTokenType.Number => typeof(int),
JsonTokenType.Number => typeof(int?),

Copy link
Copy Markdown
Member

@BrennanConroy BrennanConroy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Change title, this is not adding support, it's adding tests

@Youssef1313
Copy link
Copy Markdown
Member

#67024 merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates

Projects

None yet

Development

Successfully merging this pull request may close these issues.

MVC / Razor Pages: Verify union types work in model binding and responses

4 participants