Skip to content

Respect JsonSerializerOptions in validation errors #62341

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 3 commits into from
Closed

Respect JsonSerializerOptions in validation errors #62341

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f29313d
Respect JsonSerializerOptions in validation errors
captainsafia Jun 13, 2025
e3c9162
Address feedback
captainsafia Jun 13, 2025
4517667
Merge branch 'main' of https://github.com/dotnet/aspnetcore into safi…
captainsafia Jun 16, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Address feedback
  • Loading branch information
@captainsafia
captainsafia committed Jun 13, 2025
commit e3c91626fe417edd39dca192cf583c8902b4c57a
6 changes: 3 additions & 3 deletions src/Validation/src/ValidatablePropertyInfo.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ protected ValidatablePropertyInfo(
DisplayName = displayName;

// Cache the HasDisplayAttribute result to avoid repeated reflection calls
Copy link
Member

@BrennanConroy BrennanConroy Jun 13, 2025

Choose a reason for hiding this comment

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

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

Copy link
Member Author

@captainsafia captainsafia Jun 13, 2025

Choose a reason for hiding this comment

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

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?

// We only check for the existence of the DisplayAttribute here and not the
// Name value itself since we rely on the source generator populating it
var property = DeclaringType.GetProperty(Name);
Copy link

Copilot AI Jun 13, 2025

Choose a reason for hiding this comment

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

[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);

Copilot uses AI. Check for mistakes.
_hasDisplayAttribute = property is not null && HasDisplayAttribute(property);
}
Expand Down Expand Up @@ -72,10 +74,8 @@ public virtual async Task ValidateAsync(object? value, ValidateContext context,
var propertyValue = property.GetValue(value);
var validationAttributes = GetValidationAttributes();

// Get JsonSerializerOptions from DI container
var namingPolicy = context.SerializerOptions?.PropertyNamingPolicy;

// Calculate and save the current path
var namingPolicy = context.SerializerOptions?.PropertyNamingPolicy;
var memberName = GetJsonPropertyName(Name, property, namingPolicy);
var originalPrefix = context.CurrentValidationPath;
if (string.IsNullOrEmpty(originalPrefix))
Expand Down
Loading