-
Notifications
You must be signed in to change notification settings - Fork 10.4k
Respect JsonSerializerOptions casing for property names in validation errors #62036
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.
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
base: main
Are you sure you want to change the base?
Conversation
…ation errors Co-authored-by: captainsafia <1857993+captainsafia@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good start! Left some comments inline.
src/Http/Http.Abstractions/test/Validation/ValidateContextTests.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: captainsafia <1857993+captainsafia@users.noreply.github.com>
I've addressed all the inline comments. Made the SerializerOptions property internal, removed the test file, and updated ValidationEndpointFilterFactory to use the internal property via reflection. All changes in commit 216406a. |
… from service provider Co-authored-by: captainsafia <1857993+captainsafia@users.noreply.github.com>
@captainsafia Unfortunately I hit an unexpected error while processing your comment. I've automatically reported this to GitHub. You can ask me to try again later by mentioning me in a new comment. If you want to contact GitHub about this error, please mention the following identifier so they can better serve you: Sorry for the inconvenience! |
@copilot Let's try to address the review comments I left for you earlier. You were on the right track with making the |
… complex keys Co-authored-by: captainsafia <1857993+captainsafia@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're almost there! One more comment. Also, be undo all the changes you made to the package.json
files and remove the package.json.bak
files.
…tions Co-authored-by: captainsafia <1857993+captainsafia@users.noreply.github.com>
@copilot Revert any changes made to package.json and package-lock.json files to their state in main. |
@copilot it looks like some tests in ValidatableTypeInfoTests and ValidationsGeneratorTests are failing. Can you fix them? |
@copilot Can you fix this build error by adding the correct attribute to the constructor?
|
…idatablePropertyInfo constructor Co-authored-by: captainsafia <1857993+captainsafia@users.noreply.github.com>
... Fixed the build error by adding Warning Firewall rules blocked me from connecting to one or more addressesI tried to connect to the following addresses, but was blocked by firewall rules:If you need me to access, download, or install something from one of these locations, you can either:
|
@copilot I've made some fixes to the implementation and want you to follow up on some things, including:
|
Co-authored-by: captainsafia <1857993+captainsafia@users.noreply.github.com>
I've implemented both performance improvements in commit 409bb34:
Warning Firewall rules blocked me from connecting to one or more addressesI tried to connect to the following addresses, but was blocked by firewall rules:If you need me to access, download, or install something from one of these locations, you can either:
|
src/Http/Http.Abstractions/src/Validation/ValidatableParameterInfo.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: captainsafia <1857993+captainsafia@users.noreply.github.com>
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addressesI tried to connect to the following addresses, but was blocked by firewall rules:If you need me to access, download, or install something from one of these locations, you can either:
|
4afa2bb
to
f035b35
Compare
@DeagleGross @BrennanConroy Would love your review n this PR. Outside of the prompting, I ended up making local changes on Some things here:
|
Summary
When configuring JSON serialization options with a custom property naming policy (like camelCase), validation error responses were not consistently following the same naming policy for property names. This created a disconnect between what clients send/expect and what the validation errors contained.
The implemented changes ensure validation errors respect the configured JSON naming policy, making them consistent with the rest of the API's JSON formatting.
Changes
SerializerOptions
property toValidateContext
to access JSON naming policiesValidationEndpointFilterFactory
to pass the JSON options from DI to the validation contextPropertyNamingPolicy
for error key namesitems[0].productName
→items[0].productName
)Before
After
The implementation preserves the full compatibility with existing usage while ensuring that property names in validation errors now follow the configured naming policy.
Fixes #61764.
Warning
Firewall rules blocked me from connecting to one or more addresses
I tried to connect to the following addresses, but was blocked by firewall rules:
cdn.fwupd.org
/usr/bin/fwupdmgr refresh
(dns block)If you need me to access, download, or install something from one of these locations, you can either:
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.