Skip to content
This repository was archived by the owner on Dec 14, 2018. It is now read-only.

Ability to have different JsonSerializerSettings for input and output missing #4562

Closed
kwaclaw opened this issue Apr 29, 2016 · 15 comments
Closed
Milestone

Comments

@kwaclaw
Copy link

kwaclaw commented Apr 29, 2016

Until recently I was able to have different settings, but now the serializer settings on the Json formatters are not accessible anymore and I have to use AddJsonOptions(). This would be fine if MvcJsonOptions had different SerializerSettings for input and output.

What are the alternatives? Issue #4492 does not really explain it in a way that an "outsider" can easily understand.

@kichalla
Copy link
Member

kichalla commented Apr 29, 2016

Until recently I was able to have different settings

Curious what settings are you setting differently between input & output formatters?

One of the reasons for both the input & output formatters to share the same settings is performance. Json.net serializer builds json contracts for types it serializes/deserializes. Having separate settings means having this cache in 2 places.

You can new up the input and output formatters with your custom serializer settings and add them to the global formatters list.

You can use JsonSerializerSettingsProvider to create the default serializer settings that MVC creates.
https://github.com/aspnet/Mvc/blob/dev/src/Microsoft.AspNetCore.Mvc.Formatters.Json/JsonSerializerSettingsProvider.cs

@kwaclaw
Copy link
Author

kwaclaw commented Apr 29, 2016

In my case I have different TypeNameHandlig settings.
On input I use TypeNameHandling.All with a custom SerializationBinder that can process type meta-data for certain scenarios. But I dont want to add any type meta-data to the output. So currently my application is broken because the Javascript client does not expect this.

I would expect that input and output are sufficiently different scenarios to warrant differences in settings.

@kwaclaw
Copy link
Author

kwaclaw commented Apr 30, 2016

Newing up my own formatter instances poses the question: what to supply for the other constructor arguments.
Any guidance? Any way to use the same argument instances already used by the defaults?

@dougbu
Copy link
Member

dougbu commented May 1, 2016

Any guidance? Any way to use the same argument instances already used by the defaults?

In general, you should set JsonSerializerSettings as your application requires in its Startup class.

If you need to create non-default JsonInputFormatter or JsonOutputFormatter instances, use JsonSerializerSettingsProvider (in RC2) to initialize the JsonSerializerSettings instances and get the remaining constructor parameters from DI e.g. adding parameters to the controller constructor.

BTW what settings would you like to set differently for input and output formatters?

@kwaclaw
Copy link
Author

kwaclaw commented May 1, 2016

TypeNameHandling and SerializationBinder need to be different in my case, as described above. I would think this is not an uncommon scenario.

@JamesNK
Copy link
Member

JamesNK commented May 2, 2016

One of the reasons for both the input & output formatters to share the same settings is performance.

If you're worried about that then by default the input and output formatter could have the same instance of JsonSerializerSettings, and if a dev wants to they can set a different JsonSerializerSettings on the inputter/outputter then they can go ahead and do that.

Also by default the contract resolver on JsonSerializerSettings is null so it won't override the default contract resolver and there won't be any perf hit for reflection over the same object multiple times. You will only get that situation if a user explicitly sets a new contract resolver on their JsonSerializerSettings.

@Eilon Eilon added this to the Backlog milestone May 2, 2016
@Eilon
Copy link
Member

Eilon commented May 2, 2016

Putting in Backlog for now. We will reconsider this in a future release (we can easily add new properties that are backwards-compatible).

@jods4
Copy link

jods4 commented Jun 14, 2016

I was going to open an issue exactly for that.

My use-case is the reverse of @kwaclaw.
I want to set TypeNameHandling to Objects for output because inside our SPA, a JSON reviver instantiates the correct class, complete with methods, validation, proper date deserialization, etc.
On the other hand I want TypeNameHandling to None for input because ASP.NET doesn't need this information and it is a security risk to let JSON.NET blindly process it.

@javiercn
Copy link
Member

I think these scenarios can be achieved today.

public class CustomSerializerSettingsSetup : IConfigureOptions<MvcOptions>
{
    private readonly ILoggerFactory _loggerFactory;
    private readonly ArrayPool<char> _charPool;
    private readonly ObjectPoolProvider _objectPoolProvider;

    public CustomSerializerSettingsSetup(
        ILoggerFactory loggerFactory,
        ArrayPool<char> charPool,
        ObjectPoolProvider objectPoolProvider)
    {
        _loggerFactory = loggerFactory;
        _charPool = charPool;
        _objectPoolProvider = objectPoolProvider;
    }

    public void Configure(MvcOptions options)
    {
        options.OutputFormatters.RemoveType<JsonOutputFormatter>();
        options.InputFormatters.RemoveType<JsonInputFormatter>();
        options.InputFormatters.RemoveType<JsonPatchInputFormatter>();

        var outputSettings = new JsonSerializerSettings();
        options.OutputFormatters.Add(new JsonOutputFormatter(outputSettings, _charPool));

        var inputSettings = new JsonSerializerSettings();
        var jsonInputLogger = _loggerFactory.CreateLogger<JsonInputFormatter>();
        options.InputFormatters.Add(new JsonInputFormatter(
            jsonInputLogger,
            inputSettings,
            _charPool,
            _objectPoolProvider));

        var jsonInputPatchLogger = _loggerFactory.CreateLogger<JsonPatchInputFormatter>();
        options.InputFormatters.Add(new JsonPatchInputFormatter(
            jsonInputPatchLogger,
            inputSettings,
            _charPool,
            _objectPoolProvider));
    }
}

On your startup class you simply do this when configuring services:

services.TryAddEnumerable(
    ServiceDescriptor.Transient<IConfigureOptions<MvcOptions>, CustomSerializerSettingsSetup>());

After you called AddMvc()

@dougbu
Copy link
Member

dougbu commented Jun 15, 2016

One correction: Do not use new JsonSerializerSettings(); always call JsonSerialierSettingsProvider.CreateSerializerSettings() instead.

@javiercn
Copy link
Member

@DougHu is correct, I was oversimplifying.

dougbu referenced this issue Jul 17, 2016
- #4339: remove non-recommended JSON formatter constructors
 - affects `JsonInputFormatter`, `JsonOutputFormatter`, `JsonPatchInputFormatter`
 - `JsonOutputFormatter` cleanup also impacts `JsonHelper`
 - rename and make `SerializerSettingsProvider` class public; use it as appropriate
- #4409: make `SerializerSetings` properties get-only and `protected`
 - affects `JsonInputFormatter`, `JsonOutputFormatter`

Recommended patterns:
- change `JsonSerializerSettings` values in `MvcJsonOptions` for almost all customizations
- find `JsonOutputFormatter` in `MvcOptions.OutputFormatters` when limiting per-result formatters
- start with `JsonSerializerSettingsProvider.CreateSerializerSettings()` when customizing a per-result formatter
@weitzhandler
Copy link

@dougbu
If I have a subclass of JsonSerializerSettings, how can I clone all the properties provided by JsonSerializerSettingsProvider.CreateSerializerSettings()?

@dougbu
Copy link
Member

dougbu commented May 22, 2017

@weitzhandler either copy the properties of the JsonSerializerSettings instance JsonSerializerSettingsProvider.CreateSerializerSettings() returns into your subclass or file a request at https://github.com/JamesNK/Newtonsoft.Json/issues for a protected copy constructor in the JsonSerializerSettings class.

@weitzhandler
Copy link

weitzhandler commented May 22, 2017

Here you go, please participate, thanks.

@rynowak
Copy link
Member

rynowak commented Jul 17, 2017

I'm not sure that there's anything left to do here, it's possible use different sets of settings for input and output as shown here #4562 (comment)

Recommend Closing /cc @Eilon

@Eilon Eilon closed this as completed Jul 17, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants