Skip to content
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

Add snake case test #35

Merged
merged 1 commit into from
Oct 18, 2024
Merged

Add snake case test #35

merged 1 commit into from
Oct 18, 2024

Conversation

sguryev
Copy link
Contributor

@sguryev sguryev commented Oct 11, 2024

Here is the test describing true snake case approach using for both path and nested object value values.

{op:"replace", path:"/nested_property", value:"{first_prop:1, second_prop:2}"}

PocoAdapter using snake case handling tries to convert snake_case_name to snake case again and then fails to find the property.
image

NET8 has no explicit PascalCase naming policy which is default (null). The condition above returns just a name which is sanka_case_name as it was passed and then fails to find the property.

I have tried to create PascalCase naming policy myself. It works for path and we can find the target property but it leads to fail during value application which is snake_case as well.

Test works for case when path has PascalCase and value has snake_case though:
{op:"replace", path:"/NestedProperty", value:"{first_prop:1, second_prop:2}"}

We have truly snake_case API where client uses snake_case for both path and value. And it works with Newtonsoft. What is the best way to make it work with SystemText?

@Havunen
Copy link
Owner

Havunen commented Oct 18, 2024

Yep, this is bug, thanks for the test case will fix!

@Havunen Havunen merged commit 17bfc79 into Havunen:main Oct 18, 2024
2 of 3 checks passed
@sguryev sguryev deleted the serj/main-snake-test branch October 31, 2024 20:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants