Respect JsonSerializerOptions in validation errors #62341

captainsafia · 2025-06-13T05:23:25Z

This pull request introduces enhancements to JSON serialization and validation in the Microsoft.Extensions.Validation library. Key changes include improved handling of JSON naming policies, support for JsonPropertyName attributes, and caching of reflection results to optimize performance. Additionally, test cases have been updated to reflect these changes.

Enhancements to JSON Serialization and Validation:

Support for JSON Naming Policies and Attributes:
- Added a private method GetJsonPropertyName to determine effective property names during JSON serialization, considering JsonPropertyNameAttribute and naming policies.
- Updated validation logic to use JSON naming policies and JsonPropertyName attributes for formatting property names and paths. [1] [2]
Performance Optimization:
- Cached the presence of DisplayAttribute in the constructor of ValidatablePropertyInfo to avoid repeated reflection calls.
- Introduced the HasDisplayAttribute method to determine if a property or its corresponding record parameter has a DisplayAttribute.

Dependency and Context Updates:

New Dependencies:
- Added references to Microsoft.AspNetCore.Http.Extensions and System.Text.Json for JSON serialization support. [1] [2] [3]
- Exposed JsonSerializerOptions in ValidateContext to retrieve JSON naming policies from the DI container.

Test Case Updates:

Validation Tests:
- Updated test cases to verify the correct application of JSON naming policies and JsonPropertyName attributes in validation error messages. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11]

dotnet-policy-service · 2025-06-13T05:23:55Z

Hey @dotnet/aspnet-build, looks like this PR is something you want to take a look at.

Copilot

Pull Request Overview

This pull request enhances JSON serialization and validation by applying JSON naming policies and attributes consistently, while also improving performance through caching and updating corresponding tests. Key changes include updating expected validation error keys to match JSON naming policies, caching of DisplayAttribute checks in property validation, and exposing JsonSerializerOptions via dependency injection.

Reviewed Changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
src/Validation/test/Microsoft.Extensions.Validation.Tests/Microsoft.Extensions.Validation.Tests.csproj	Added dependency references for JSON serialization support.
src/Validation/test/Microsoft.Extensions.Validation.GeneratorTests/*.cs	Updated expected error message keys in tests to lowercase according to the new naming policy.
src/Validation/src/ValidateContext.cs	Exposed JsonSerializerOptions from DI and used new array literal initialization.
src/Validation/src/ValidatableTypeInfo.cs	Uses JSON naming policy to format member names in validation errors.
src/Validation/src/ValidatablePropertyInfo.cs	Integrated caching for DisplayAttribute and applied JSON naming policy consistently.
src/Validation/src/Microsoft.Extensions.Validation.csproj	Updated dependency references.

Comments suppressed due to low confidence (1)

src/Validation/src/ValidateContext.cs:71

[nitpick] Ensure that the empty array literal '[]' properly initializes ValidationErrors as intended; if the target type is not directly inferrable, consider using 'new SomeType[0]' for clarity.

ValidationErrors ??= [];

Copilot · 2025-06-13T05:31:49Z

src/Validation/src/ValidatablePropertyInfo.cs

@@ -28,12 +32,16 @@ protected ValidatablePropertyInfo(
        PropertyType = propertyType;
        Name = name;
        DisplayName = displayName;
+
+        // Cache the HasDisplayAttribute result to avoid repeated reflection calls
+        var property = DeclaringType.GetProperty(Name);


[nitpick] Consider specifying explicit BindingFlags (e.g. BindingFlags.Public | BindingFlags.Instance) with GetProperty to ensure the lookup behavior is unambiguous.

Suggested change

var property = DeclaringType.GetProperty(Name);

var property = DeclaringType.GetProperty(Name, BindingFlags.Public | BindingFlags.Instance);

BrennanConroy

Is there a test with a class with primary ctors and json attributes?

BrennanConroy · 2025-06-13T16:31:07Z

src/Validation/src/ValidatablePropertyInfo.cs

@@ -28,12 +32,16 @@ protected ValidatablePropertyInfo(
        PropertyType = propertyType;
        Name = name;
        DisplayName = displayName;
+
+        // Cache the HasDisplayAttribute result to avoid repeated reflection calls


nit: Comment about why we don't use the name from the attribute

And maybe we should set DisplayName to the attributes value if the provided value happens to be null? (I know the API definition says non-null but...)

And maybe we should set DisplayName to the attributes value if the provided value happens to be null? (I know the API definition says non-null but...)

What would this help with?

src/Validation/src/ValidatablePropertyInfo.cs

BrennanConroy · 2025-06-13T16:33:53Z

src/Validation/src/ValidatablePropertyInfo.cs

+        }
+
+        // Look for a constructor parameter matching the property name (case-insensitive)
+        // to account for the record scenario


and primary ctors?

Not quite since there isn't the same case-based 1:1 mapping between property names and parameter names when using primary constructors. It's a variation of #61526.

BrennanConroy · 2025-06-13T16:36:56Z

src/Validation/src/ValidateContext.cs

@@ -60,6 +64,8 @@ public sealed class ValidateContext
    /// </summary>
    public int CurrentDepth { get; set; }

+    internal JsonSerializerOptions? SerializerOptions => ValidationContext.GetService(typeof(IOptions<JsonOptions>)) is IOptions<JsonOptions> options ? options.Value.SerializerOptions : null;


Suggested change

internal JsonSerializerOptions? SerializerOptions => ValidationContext.GetService(typeof(IOptions<JsonOptions>)) is IOptions<JsonOptions> options ? options.Value.SerializerOptions : null;

internal JsonSerializerOptions? SerializerOptions => ValidationContext.GetService<IOptions<JsonOptions>>()>?.Value.SerializerOptions;

I'm not sure this works since you're embedding a local version of JsonOptions into the package.

You should make a web app outside of the repo and test that this resolves the JsonOptions from ConfigureHttpJsonOptions.

Yeah, I was testing with this last night and trying to figure out how to get it to work in a console app. It turns out that using private reflection might be the only way to do this.

I force pushed the change since I hadn't realized you started reviewing it...

github-actions bot added the area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework label Jun 13, 2025

captainsafia mentioned this pull request Jun 13, 2025

Respect JsonSerializerOptions casing for property names in validation errors #62036

Closed

captainsafia requested a review from Copilot June 13, 2025 05:31

Copilot AI reviewed Jun 13, 2025

View reviewed changes

captainsafia force-pushed the safia/valid-jsonoptions branch from b5217e2 to ee27af8 Compare June 13, 2025 06:27

This was referenced Jun 13, 2025

Known Build Error: error : Cannot create a file when that file already exists. #61178

Open

Known Build Error: ProcessBufferedRenderBatches_WritesRenders fails #61807

Open

BrennanConroy reviewed Jun 13, 2025

View reviewed changes

Respect JsonSerializerOptions in validation errors

f29313d

captainsafia force-pushed the safia/valid-jsonoptions branch from ee27af8 to f29313d Compare June 13, 2025 17:02

Address feedback

e3c9162

build-analysis bot mentioned this pull request Jun 13, 2025

The active test run was aborted. Reason: Test host process crashed dotnet/dnceng#451

Open

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Respect JsonSerializerOptions in validation errors #62341

Respect JsonSerializerOptions in validation errors #62341

Uh oh!

captainsafia commented Jun 13, 2025

Uh oh!

dotnet-policy-service bot commented Jun 13, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jun 13, 2025

Uh oh!

BrennanConroy left a comment

Uh oh!

BrennanConroy Jun 13, 2025

Uh oh!

captainsafia Jun 13, 2025

Uh oh!

Uh oh!

BrennanConroy Jun 13, 2025

Uh oh!

captainsafia Jun 13, 2025

Uh oh!

BrennanConroy Jun 13, 2025

Uh oh!

BrennanConroy Jun 13, 2025

Uh oh!

captainsafia Jun 13, 2025

Uh oh!

Uh oh!

	var property = DeclaringType.GetProperty(Name);
	var property = DeclaringType.GetProperty(Name, BindingFlags.Public \| BindingFlags.Instance);

	internal JsonSerializerOptions? SerializerOptions => ValidationContext.GetService(typeof(IOptions<JsonOptions>)) is IOptions<JsonOptions> options ? options.Value.SerializerOptions : null;
	internal JsonSerializerOptions? SerializerOptions => ValidationContext.GetService<IOptions<JsonOptions>>()>?.Value.SerializerOptions;

Respect JsonSerializerOptions in validation errors #62341

Are you sure you want to change the base?

Respect JsonSerializerOptions in validation errors #62341

Uh oh!

Conversation

captainsafia commented Jun 13, 2025

Enhancements to JSON Serialization and Validation:

Dependency and Context Updates:

Test Case Updates:

Uh oh!

dotnet-policy-service bot commented Jun 13, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Jun 13, 2025

Choose a reason for hiding this comment

Uh oh!

BrennanConroy left a comment

Choose a reason for hiding this comment

Uh oh!

BrennanConroy Jun 13, 2025

Choose a reason for hiding this comment

Uh oh!

captainsafia Jun 13, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

BrennanConroy Jun 13, 2025

Choose a reason for hiding this comment

Uh oh!

captainsafia Jun 13, 2025

Choose a reason for hiding this comment

Uh oh!

BrennanConroy Jun 13, 2025

Choose a reason for hiding this comment

Uh oh!

BrennanConroy Jun 13, 2025

Choose a reason for hiding this comment

Uh oh!

captainsafia Jun 13, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!