JsonNamingPolicy.Default internal property should be made public

[VassilAtanasov]

Why this is internal? Developers need to be able to use it pass through PascalCase

Originally posted by @VassilAtanasov in dotnet/corefx#41354

[VassilAtanasov]

the problem is in
https://github.com/dotnet/corefx/blob/93251806dcd25f0f59ea506698f46acdd7b93265/src/System.Text.Json/src/System/Text/Json/Serialization/JsonNamingPolicy.cs

    public abstract class JsonNamingPolicy
    {
        /// <summary>
        /// Initializes a new instance of <see cref="JsonNamingPolicy"/>.
        /// </summary>
        protected JsonNamingPolicy() { }

        /// <summary>
        /// Returns the naming policy for camel-casing.
        /// </summary>
        public static JsonNamingPolicy CamelCase { get; } = new JsonCamelCaseNamingPolicy();

        /// <summary>
        /// Returns the naming policy for snake-casing.
        /// </summary>
        public static JsonNamingPolicy SnakeCase { get; } = new JsonSnakeCaseNamingPolicy();

        internal static JsonNamingPolicy Default { get; } = new JsonDefaultNamingPolicy();

the request for change is for
public static JsonNamingPolicy Default { get; } = new JsonDefaultNamingPolicy();

[Zenexer]

Note: JsonDefaultNamingPolicy is internal, too.

[ahsonkhan]

Why this is internal? Developers need to be able to use it pass through PascalCase

I don't understand the question. Can you explain your use case/scenario, preferably with a code sample, where you'd use this to help me understand why the current behavior doesn't work for you?

For api change proposals like these, it would be good to follow the template (with rationale and sample usage):
https://github.com/dotnet/corefx/blob/master/Documentation/project-docs/api-review-process.md
Here's an example: https://github.com/dotnet/corefx/issues/271

If you don't specify a naming policy in the JsonSerializerOptions, then that default is used internally (or an indication of a null NamingPolicy means we don't do anything to the name which is equivalent). For example, look here:
https://github.com/dotnet/corefx/blob/e667c29636a622eb4f9493f75232b44e0ae90b29/src/System.Text.Json/src/System/Text/Json/Serialization/JsonPropertyInfo.cs#L106-L119

Why does this need to be public? What would you use this for?

[btecu]

@ahsonkhan I'm not sure if this is @VassilAtanasov's need, but how would you go about selecting a default naming policy?

Currently in JSON.NET:

JsonConvert.DefaultSettings = () => new JsonSerializerSettings {
    ContractResolver = new CamelCasePropertyNamesContractResolver()
}

[ahsonkhan]

but how would you go about selecting a default naming policy?

Overriding the "default" naming policy with camel case (or a custom naming policy) can be done in the JsonSerializerOptions being passed-in.

var options = new JsonSerializerOptions
{
    PropertyNamingPolicy = JsonNamingPolicy.CamelCase,
};
string str = JsonSerializer.Serialize<Foo>(obj, options);

If you want the default policy, just don't set one.

// Don't set any naming policy in options, but feel free to set other options that might be needed
var options = new JsonSerializerOptions();
string str = JsonSerializer.Serialize<Foo>(obj, options);

// OR just simply:
string str = JsonSerializer.Serialize<Foo>(obj);

So, I don't see why there is a need for the default naming policy to be public?

[btecu]

@ahsonkhan the question is about setting it globally, not passing in every single time.

If the application does a significant amount of serializations / deserializations, I'd like to set it in one place and not have to declare and pass every single time:

var options = new JsonSerializerOptions
{
    PropertyNamingPolicy = JsonNamingPolicy.CamelCase,
};

[ahsonkhan]

@btecu, even in the case of using a global, by not setting PropertyNamingPolicy, you can implicitly setting it to Default without having to explicitly set it to JsonNamingPolicy.Default (or set it to null if you really want to). That's why I am asking why the default is required to be made public and what scenario is it blocking otherwise. If exposing the JsonNamingPolicy.Default makes it more discover able, helps usability/documentation, we could make it public but I want to understand what the need is here.

// Global setting - can be re-used over and over
// Implicitly, we are using JsonNamingPolicy.Default because a custom 
// PropertyNamingPolicy wasn't set
var options = new JsonSerializerOptions();

// OR explicitly set the property to null - if for some reason it needs
// to be reset to undo some previous value
options = new JsonSerializerOptions
{
   PropertyNamingPolicy  = null
};

@steveharter - is there any reason we shouldn't expose the default naming policy?

[btecu]

@ahsonkhan this is not much better than inline. Now you need some class where you have a random property jsonOption or something like that and you have to remember to pass it every time you serialize or deserialize. This is nothing like what JSON.NET does, where you set it once and you're done, no need to pass any options:

JsonConvert.DefaultSettings = () => new JsonSerializerSettings {
    ContractResolver = new CamelCasePropertyNamesContractResolver()
}

[ahsonkhan]

This is nothing like what JSON.NET does, where you set it once and you're done, no need to pass any options

I understand, but there are by-design differences in the two libraries, and that design has already shipped. I think the concerns you brought up @btecu are unrelated to the issue at hand.

@VassilAtanasov, @Zenexer, can you please clarify your requirements and address my comment above:
https://github.com/dotnet/corefx/issues/41383#issuecomment-536085366
Are you explicitly looking for PascalCase support (regardless of the casing that your .NET object model has, and overrides it, similar to CamelCase)? Or are you looking for the default behavior (exact match) to be made public? And if it's the latter, why is that? What scenario is blocked by you not having access to the default naming policy?

Otherwise, I don't see this issue as being actionable.

[Zenexer]

@ahsonkhan I have no requirements; I just find it odd and confusing that the default isn't public. What is that trying to tell me? Should I be avoiding the default? Is there something wrong with the default? Is the default going to change in a future version, meaning that I should write my own implementation? It's unintuitive.

In general, throughout .NET, you can explicitly set default values--that is, if a property initializes to null, you can set it to null anyway--it isn't going to change anything, but sometimes it's helpful to clarify the state that you expect. When that's explicitly prevented by visibility constraints, it indicates that there's something wrong with or implementation-dependent about the default, and that I should be explicitly specifying something other than the default. If the current default is acceptable and not subject to change in the future, then it makes little sense to hide it.

[ahsonkhan]

Fair enough, @Zenexer - thanks for the input.

@steveharter - I don't see why the Default property is internal. Do you have any reason to keep it that way (or know why it was internal to begin with)?

I am marking this ready for review (otherwise, let me know if you think we shouldn't do this).

[steveharter]

@steveharter - I don't see why the Default property is internal. Do you have any reason to keep it that way (or know why it was internal to begin with)?

All public settable options by design are instance properties on JsonSerializerOptions including the casing policies.

The "default" internal static naming policy getter does nothing -- just returns the same string. It is an implementation detail for the Enum conversion code. It is not used anywhere else.

To clarify, as mentioned earlier, you can set the default on options:

var options = new JsonSerializerOptions
{
    PropertyNamingPolicy = JsonNamingPolicy.CamelCase,
};

but of course the "options" would need to be passed to every (de)serialize call. However, we did consider a feature (not implemented) where you can essentially set a static variable that defines a callback that returns what options instance is returned (a CallContext\thread variable, session variable, another static, etc), allowing use of the serializer methods that don't take "options" as a parameter. I can add that feature to track this if that is desired.

The reason the options are instance-based is because we want to support different options for different use cases and don't want one area of code to "randomly" change behavior when another area of code changes a global static variable. For this reason we also prevent options from being changed once (de)serialization has occurred.

Another benefit of preventing options from being changed once (de)serialization has occurred is that it allows us to effectively emit IL or code (for ahead-of-time scenarios) that can be optimized by not having to check to see if global options have changed on every (de)serialization.

Closing this issue by design. See above if we should create a new issue to specify where the global options can live (so the options don't need to be passed on every call).

[btecu]

@steveharter, please that feature (static variable with options) would be really useful.
After working on several larger applications I've noticed that usually there's a default across the entire application and for those one or two cases where you work with some third party api that has a different naming convention, you can just pass a specific JsonSerializerOptions.

[steveharter]

I see someone created a similar issue for configuring the default settings, so we will assume that for now: https://github.com/dotnet/corefx/issues/41612

[steveharter]

Clarification: there is no "default" naming policy -- just a lack of a naming policy which is represented by a null for the naming polices (e.g. PropertyNamingPolicy and DictionaryKeyPolicy properties).

The lack of a naming policy just means the JSON will use the names of the POCO properties as-is (which are typically Pascal-cased).

[msmolka]

In aspnetcore AddJsonOptions has already camelcase so how to change it back?

[khellang]

@msmolka Set the property to null, like @ahsonkhan mentioned in https://github.com/dotnet/corefx/issues/41383#issuecomment-536852829.