Fix null property name handling logic in SystemTextJsonValidationMetadataProvider

[eiriktsarpalis]

Fixing what appears to be a negation bug causing null names to be passed to the underlying JsonNamingPolicy.

cf. dotnet/runtime#85002 (comment)

Fix #47835

[eerhardt]

I believe the existing code is correct. "If propertyName isn't set, read it from the context.Key.Name".

However, the bug here appears to be the null suppression on line 60:

_jsonNamingPolicy.ConvertName(context.Key.Name!)

If context.Key.Name can also be null, then I think we have a problem.

[eiriktsarpalis]

Good call, I misinterpreted the code as running the naming policy against propertyName. In that case I'll close the PR and transfer https://github.com/dotnet/runtime/issues/84830 to aspnetcore so that you folks can make the appropriate fix.

[eiriktsarpalis]

I ended up updating the PR so that it includes a null check for context.Key.Name. That should emulate the semantics of JsonNamingPolicy.CamelCase for other naming policies.

[captainsafia]

Can we add a test for this in https://github.com/dotnet/aspnetcore/blob/main/src/Mvc/Mvc.Core/test/ModelBinding/Metadata/SystemTextJsonValidationMetadataProviderTest.cs?

I think it should work if we use the following key in the test cases:

var key = ModelMetadataIdentity.ForType(typeof(SampleTestClass));

[captainsafia]

@eerhardt I added the test I recommended in #47775 (review). Can you review both changes?

Also, I observed something interesting which is that the bug only applies to scenarios that use the JsonSeperatorNamingPolicy. Things that use JsonCamelCasePolicy don't have the same bug because of this https://github.com/dotnet/runtime/blob/main/src/libraries/System.Text.Json/Common/JsonCamelCaseNamingPolicy.cs#L10-L13.

I was curious as to why this happens but I suppose it makes sense because you don't need to do any seperator-based concatenation for other policies?

[eerhardt]

Thanks!

[eerhardt]

Also, I observed something interesting which is that the bug only applies to scenarios that use the JsonSeperatorNamingPolicy. Things that use JsonCamelCasePolicy don't have the same bug because of this https://github.com/dotnet/runtime/blob/main/src/libraries/System.Text.Json/Common/JsonCamelCaseNamingPolicy.cs#L10-L13.

I was curious as to why this happens but I suppose it makes sense because you don't need to do any seperator-based concatenation for other policies?

This is a (now) known inconsistency between the policies. See the conversation at dotnet/runtime#85002 (comment).