NancyFx / Nancy.Serialization.JsonNet

NewtonSoft.Json serializer for Nancy
MIT License
39 stars 32 forks source link

Use JsonSettings for date formatting and casing #14

Closed xt0rted closed 6 years ago

xt0rted commented 9 years ago

This fixes https://github.com/NancyFx/Nancy/issues/1607

I wasn't sure how we wanted the settings from JsonConvert.DefaultSettings() and JsonSettings to take priority so that might need to be revised a bit.

It'd be helpful if Json.IsJsonContentType() could be made public since all of the serializers have their own copy which is out of sync.

keithnlarsen commented 9 years ago

Can this be merged in please?

PureKrome commented 9 years ago

@xt0rted - quick question. Can you show some example code how this PR can be used / is helpful? I was going to create another issue and wasn't sure if this PR handles my use case / scenario already.

xt0rted commented 9 years ago

@PureKrome by default the json.net serializer doesn't use the same default settings as the built-in one, it's also not controllable via Nancy.Json.JsonSettings. This meant if you wanted camelCase you'd have to add your own json.net serializer and override the one in your IoC container of choice like so:

public class CamelCaseJsonSerializer : JsonSerializer
{
    public CamelCaseJsonSerializer()
    {
        ContractResolver = new CamelCasePropertyNamesContractResolver();
#if DEBUG
        Formatting = Formatting.Indented;
#endif
    }
}
// Registration with StructureMap
public class RepositoryRegistry : Registry
{
    public RepositoryRegistry()
    {
        For<JsonSerializer>()
            .Use<CamelCaseJsonSerializer>();
    }
}

That's not very SDHP.

With these changes camelCase is now the default (a breaking change) since that's what Nancy's default is and then it could be turned off simply by setting JsonSettings.RetainCasing to true. As @khellang pointed out in the original issue the setting name should be changed, but that was left for another PR.

You'd also able to use JsonSettings.ISO8601DateFormat to toggle between the ISO format and Microsoft's "\/Date(1333645844276-0600)\/" format.

The last thing this added was the ability to use json.net's JsonConvert.DefaultSettings() builder in the event you have an existing setup that's used outside of Nancy (such as in other middleware or 3rd party components). Then the JsonSettings would be applied as needed.

Aside from review this also needs a good rebasing.

PureKrome commented 9 years ago

This meant if you wanted camelCase you'd have to add your own json.net serializer and override the one in your IoC container

I'm doing the exact same code and it's very very annoying.

That's not very SDHP.

The last thing this added was the ability to use json.net's JsonConvert.DefaultSettings()

Er.. can you show some sample code, what this means and why this might be helpful, pwease?

xt0rted commented 9 years ago

@PureKrome I don't use this (yet) but here's a modified version of one of my OWIN Startup classes that should help show it in use.

public partial class Startup : RaygunStartup
{
    public override void SetupConfiguration(IAppBuilder app, RaygunSettings settings)
    {
        JsonConvert.DefaultSettings = () =>
            new JsonSerializerSettings
            {
                NullValueHandling = NullValueHandling.Ignore,
#if DEBUG
                Formatting = Formatting.Indented,
                TraceWriter = new DebugJsonLogger(), // uses System.Diagnostics.Debug
#else
                TraceWriter = new JsonLogger(), // uses SimpleLogging
#endif
            };

        app.UseRaygun(options => { /* ... */ });
        app.Use<CleanHeadersMiddleware>();
        app.Use<HeartbeatMiddleware>();
        app.Use<SafeCrawlingMiddleware>();
        app.Use<PlaceholderMiddleware>();
        app.UseSimpleAuthentication();
        app.UseNancy(options => { /* ... */ });
    }
}

Given this setup as long as everything creates their json.net serializer like so then the system-wide settings will be used as the starting point.

var settings = JsonConvert.DefaultSettings();
var serializer = JsonSerializer.Create(settings);

So given the following model in debug mode:

var model = new UserData
{
    Id = 1234,
    Name = "A Test",
    Description = null
};

You'll get the following json in response:

{
  "Id": 1234,
  "Name": "A Test"
}

Along with json.net's logging written out to one of the custom loggers:

Info - Started serializing UserQuery+UserData. Path ''. 
Info - Finished serializing UserQuery+UserData. Path ''. 
Verbose - Serialized JSON: 
{
  "Id": 1234,
  "Name": "A Test"
} 

The response from Nancy will be slightly different and that's because we're overriding some of the settings with those from JsonSettings. This means the response coming from Nancy would actually look like:

{
  "id": 1234,
  "name": "A Test"
}

Here you get camelCase which came from JsonSettings but you also get the indented formatting, description is left out since it's null, and logging written to your custom logger.


These changes are to help with consistent defaults and ease of configuring common settings. If you want to use a custom ContractResolver that say filters out all SuperSecretId properties from your models then you'll need to do some overriding, but for most things you can now use Nancy's JsonSettings and json.net's JsonConvert.DefaultSettings.

To get around the overriding we could do one of the following:

  1. Skip applying Nancy's settings if JsonConvert.DefaultSettings is to be used
  2. Make the DefaultSerializer helper function public static similar to the one in json.net and let the user do whatever they want if they don't like the defaults

I'll rebase & update once I know what direction we want to go.

PureKrome commented 9 years ago

Gotcha!

(and thanks for answering the q with code and explanation, etc).

thecodejunkie commented 9 years ago

Hi, could you please rebase this? kntxbai

xt0rted commented 9 years ago

@thecodejunkie rebased. Hopefully I didn't miss anything.

xt0rted commented 9 years ago

@thecodejunkie I think the project I originally tested this on wasn't configured correctly. What I'm now seeing is even though JsonSerializer isn't registered with StructureMap the default constructors on JsonNetSerializer & JsonNetBodyDeserializer aren't being used.

If this is how all of the IoC containers are going to function then I don't think we can have JsonSerializer as a parameter unless we override its settings in each class. I'm also not sure why there's default constructors since they're not used anywhere (aside from the unit tests).

If I remove the constructors with the serializer parameter then everything works as intended and you can use Nancy's JsonSettings and Json.Net's JsonConvert.DefaultSettings to configure your serializers further. Is this the route we want to go?

Instead of making the Helper class public would it be better to rename Json.cs to something like JsonUtility and then make this instance of IsJsonType public? Nancy.Serialization.ServiceStack has the same helper.