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]

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...

BrennanConroy · 2025-06-13T22:36:38Z

src/Validation/src/ValidateContext.cs

+    /// Attempts to resolve the <see cref="JsonSerializerOptions"/> used for serialization
+    /// using reflection to access JsonOptions from the ASP.NET Core shared framework.
+    /// </summary>
+    private JsonSerializerOptions? ResolveSerializerOptions()


How AOT friendly is this?

I think it's generally safe. The main issue would be the resolution of the IOptions<JsonOptions> type but since that's always configured in the DI container for ASP.NET Core apps, I don't expect it to be trimmed away. The other issue would beJsonOptions.SerializerOptions but I don't think that's likely to be trimmed away given how it's referenced in the framework.

I am long overdue for adding a native AoT test project to this package... :/

…a/valid-jsonoptions

captainsafia · 2025-06-19T00:15:11Z

@javiercn @oroztocil Would appreciate your thoughts here. The goal here is to support using the STJ naming policy for both the property keys and the member names in error messages.

The OnValidationError API that is used by Blazor isn't sufficient here because AFAIK that only influences the key that is used and not the member name that is in the errors.

This does introduce an STJ dependency into the extensions package but I think it is crafted such that there should be any impact for the WASM scenario.

This maybe have an impact for the Blazor Server scenario since I believe JsonOptions might be loaded there. In a previous iteration of this API, we explicitly set a SerializerOptions property on ValidationOptions to opt into this behavior. Let me know if you prefer this design.

javiercn · 2025-06-19T09:48:29Z

@captainsafia I'm not sure I understand all the details about this but let me put it in my own words to see if I understood correctly.

This adds support for DisplayName (or that existed, and I wasn't aware of) and S.T.J conventions.

Does the DisplayName attribute show up in any way when reporting the errors? Are the path/key on the OnValidateError callback or the ValidationResult error messages affected by the the displayName attribute? (that likely break us)
For the STJ case, I see that the key at least presents a different casing.

We currently rely on the mapping being 1:1 between key/path and object path, so any transform there is something we would need to account for and potentially "undo".

For us there is no "value" in having such transforms, on the contrary, it gets in our way because we have to do extra work to reverse the mapping (camel casing might work, but snake_casing or kebab-casing won't).

One alternative way to go about this might be to abstract the naming policy away into something like an (IErrorNamingPolicy) that can then be added to the ValidationOptions. You could then source generate the STJ version of that into the user app along with the glue you need to wire things up when you replace AddValidation. Would something like that work?

The other bit would be that I would not mind this being "on by default" but I would like a way to "turn off" this behavior from within the framework at the time we are going to run the validation. For example, maybe ValidateContext has a flag IgnoreErrorNamingPolicy or something like that which we can set.

Ideally, we shouldn't require the users to do additional work for the framework to work in the way it is expected to do so.

If we want to be all or nothing, instead of IgnoreErrorNamingPolicy we can have an enum that allows us to pick "only messages" for example but not apply it to keys or paths.

I'm happy to discuss this further.

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

BrennanConroy reviewed Jun 16, 2025

View reviewed changes

Merge branch 'main' of https://github.com/dotnet/aspnetcore into safi…

4517667

…a/valid-jsonoptions

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

captainsafia 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!

BrennanConroy Jun 13, 2025

Uh oh!

captainsafia Jun 16, 2025

Uh oh!

captainsafia commented Jun 19, 2025

Uh oh!

javiercn commented Jun 19, 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

Conversation

captainsafia commented Jun 13, 2025

Enhancements to JSON Serialization and Validation:

Dependency and Context Updates:

Test Case Updates:

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

captainsafia commented Jun 19, 2025

Uh oh!

javiercn commented Jun 19, 2025

Uh oh!

Uh oh!