Newtonsoft.json: Unable to extend default serializer logic in JsonConverter.WriteJson without "Self referencing loop" exception

[johncrim]

I believe this is a bug in the implementation, b/c I have been unable to find a reasonable implementation for JsonConverter.WriteJson that extends the default serializer logic, and does not throw "JsonSerializationException : Self referencing loop detected with type XXX".

My use-case is that I am using JSON and Json.NET to parse and format config files. I want to use an additional Json property value to determine the type of each object - and this property name and value are specific to the object type. This requires intercepting the deserialization and pulling out the property (no problem - I have a working JsonConverter.ReadJson implementation); and it requires intercepting the serialization and inserting the property (big problem here - all implementations of JsonConverter.WriteJson I've tried result in the "Self referencing loop" exception).

Here's a working xunit test that fails due to this problem:

``` C#
using System;

using Newtonsoft.Json;
using Newtonsoft.Json.Linq;
using Newtonsoft.Json.Serialization;

using Xunit;

public class A
{
public string Id { get; set; }
public A Child { get; set; }
}

public class B : A {}

public class C : A {}

///

/// Shows the problem I'm having serializing classes with Json.
///

public sealed class JsonTypeConverterProblem
{
[Fact]
public void ShowSerializationBug()
{
A a = new B()
{
Id = "foo",
Child = new C() { Id = "bar" }
};

    JsonSerializerSettings jsonSettings = new JsonSerializerSettings();
    jsonSettings.ContractResolver = new TypeHintContractResolver();
    string json = JsonConvert.SerializeObject(a, Formatting.Indented, jsonSettings);
    Console.WriteLine(json);

    Assert.Contains(@"""Target"": ""B""", json);
    Assert.Contains(@"""Is"": ""C""", json);
}

[Fact]
public void DeserializationWorks()
{
    string json =

@"{
""Target"": ""B"",
""Id"": ""foo"",
""Child"": {
""Is"": ""C"",
""Id"": ""bar"",
}
}";

    JsonSerializerSettings jsonSettings = new JsonSerializerSettings();
    jsonSettings.ContractResolver = new TypeHintContractResolver();
    A a = JsonConvert.DeserializeObject<A>(json, jsonSettings);

    Assert.IsType<B>(a);
    Assert.IsType<C>(a.Child);
}

}

public class TypeHintContractResolver : DefaultContractResolver
{
public override JsonContract ResolveContract(Type type)
{
JsonContract contract = base.ResolveContract(type);
if ((contract is JsonObjectContract)
&& ((type == typeof(A)) || (type == typeof(B))) ) // In the real implementation, this is checking against a registry of types
{
contract.Converter = new TypeHintJsonConverter(type);
}
return contract;
}
}

public class TypeHintJsonConverter : JsonConverter
{
private readonly Type _declaredType;

public TypeHintJsonConverter(Type declaredType)
{
    _declaredType = declaredType;
}

public override bool CanConvert(Type objectType)
{
    return objectType == _declaredType;
}

// The real implementation of the next 2 methods uses reflection on concrete types to determine the declaredType hint.
// TypeFromTypeHint and TypeHintPropertyForType are the inverse of each other.

private Type TypeFromTypeHint(JObject jo)
{
    if (new JValue("B").Equals(jo["Target"]))
    {
        return typeof(B);
    }
    else if (new JValue("A").Equals(jo["Hint"]))
    {
        return typeof(A);
    }
    else if (new JValue("C").Equals(jo["Is"]))
    {
        return typeof(C);
    }
    else
    {
        throw new ArgumentException("Type not recognized from JSON");
    }
}

private JProperty TypeHintPropertyForType(Type type)
{
    if (type == typeof(A))
    {
        return new JProperty("Hint", "A");
    }
    else if (type == typeof(B))
    {
        return new JProperty("Target", "B");
    }
    else if (type == typeof(C))
    {
        return new JProperty("Is", "C");
    }
    else
    {
        return null;
    }
}

public override object ReadJson(JsonReader reader, Type objectType, object existingValue, JsonSerializer serializer)
{
    if (! CanConvert(objectType))
    {
        throw new InvalidOperationException("Can't convert declaredType " + objectType + "; expected " + _declaredType);
    }

    // Load JObject from stream.  Turns out we're also called for null arrays of our objects,
    // so handle a null by returning one.
    var jToken = JToken.Load(reader);
    if (jToken.Type == JTokenType.Null)
        return null;
    if (jToken.Type != JTokenType.Object)
    {
        throw new InvalidOperationException("Json: expected " + _declaredType + "; got " + jToken.Type);
    }
    JObject jObject = (JObject) jToken;

    // Select the declaredType based on TypeHint
    Type deserializingType = TypeFromTypeHint(jObject);

    var target = Activator.CreateInstance(deserializingType);
    serializer.Populate(jObject.CreateReader(), target);
    return target;
}

public override bool CanWrite
{
    get { return true; }
}

public override void WriteJson(JsonWriter writer, object value, JsonSerializer serializer)
{
    JProperty typeHintProperty = TypeHintPropertyForType(value.GetType());

    //BUG: JsonSerializationException : Self referencing loop detected with type 'B'. Path ''.
    // Same error occurs whether I use the serializer parameter or a separate serializer.
    JObject jo = JObject.FromObject(value, serializer); 
    if (typeHintProperty != null)
    {
        jo.AddFirst(typeHintProperty);
    }
    writer.WriteToken(jo.CreateReader());
}

}
```

[JamesNK]

The serializer is calling into your converter which is then calling into the serializer which is calling into your converter, etc.

[johncrim]

I understand that. The problem is that there is no apparent way to re-use the default serialization logic from within a WriteJson() method. If there is a way, can you shine some light on it?

[JamesNK]

Ask on stackoverflow

[johncrim]

Thanks for the follow up...

I did ask on SO - http://stackoverflow.com/questions/26129448/json-net-how-to-customize-serialization-to-insert-a-json-property

So far, no takers. I also spent some time in the JSON.NET source code - no dice so far.

[Lenne231]

Any progress on this? Is there a solution to reuse the serialization logic? Rewriting the whole serialization logic is not an option for us.

[StrangeWill]

@johncrim @Lenne231 So I have a rough solution on a new project I'm working on, much like you I need to preserve default behavior, including support for Newtonsoft.Json attributes, but have custom properties added to it. What I have _just_ started passing it's unit tests so it's pretty experimental and needs some additional work: https://github.com/RoushTech/SegmentDotNet/pull/1/commits/c21d30c9d429d2b0985ce3b6ad0e2272e522b50a#diff-6a5f310d2329fc14cc9507f20415ab69R27

I noticed here: https://github.com/JamesNK/Newtonsoft.Json/blob/b23ead8d23046da1edf475e8ff60caecb0d1c78f/Src/Newtonsoft.Json/Serialization/JsonSerializerInternalWriter.cs#L164 that the only way to get the serializer to kick over is to cause CanWrite to return false... so we prevent that from returning true on nested serialization calls.

Problems include: for thread safety I have to lock CanWrite which will lead to performance barriers, and that nesting this behavior doesn't work, but this at least gets me where I _need_.

It's late and I'm going to play with ripping out a minimum of what is required to get SerializeObject working outside of the class and implement basically the thinnest JsonSerializer for this very specific use case another day (I got some other projects that need some time over the next few days before I can take a swing at this again).

Edit: I may just be able to override ContractResolver to a new Resolver that always returns null and specify it as part of JToken.FromObject(...)? So the new JsonSerializer may be pretty thin.

[StrangeWill]

As of this commit: https://github.com/RoushTech/SegmentDotNet/pull/1/commits/ac514d0c270a048c243d3ea913f7c065d42d0b10 I now use a custom ContractResolver that forces the Converter to null for this specific instance, it allows nested overridden objects (see Serialization_With_Nested_Custom_Params test). Code required is minimum just unraveling how to do this is the difficult part.

[buzzware]

This issue is a real problem. I simply want to customise the writing of properties for those classes my JsonConverter is attached to, but I get the stack overflow or self referencing loop crash.
The solutions I've seen implement property writing from first principles, or create and use a new serializer, but I suspect the various attributes and options won't be observed for embedded relationships. ContractResolvers don't seem to be the answer either, as they don't relate to a particular class. It is particularly good that JSON.Net allows us to associate a JsonConverter per model class that can be looked up and used deep into a heirarchical json document.

How are we supposed to implement WriteJson() in a way that ?

1. doesn't crash
2. has all the features we get from the non-overidden code, especially for embedded records
3. calls parts of the default code so we aren't reinventing the wheel

[StrangeWill]

@buzzware I'm pretty sure my code example above from SegmentDotNet covers all of those cases.

[buzzware]

@StrangeWill Thanks for replying.
By "Converter to null for this specific instance" are you losing custom configuration in any way for embedded records? By nulling the converter I'm wondering what abilities would be lost, or what internal logic would have to be duplicated (error prone) for the current model or embedded models.
I couldn't find the test you refer to.
Would you recommend your solution as the current best practice, or is it more of a hack that won't work for all scenarios?
I've given up overriding WriteJson for now, and refactored client & server to avoid it - just too hard for me. I've gotta say I'm surprised that custom writing of JSON is so hard on this, the most popular .Net library. Its way easier on the Ruby based libraries I've used, like https://github.com/rails-api/active_model_serializers.
A core ability of a JSON library when writing models with embedding should be the ability to associate serializer classes per model class like JSON.Net already does with eg. [JsonConverter(typeof(IModelConverter))] on a class, and then we need to translate values in each direction with both the json and model class in scope. I've achieved this in IModelConverter for reading, adding an AfterPopulate hook, but so far I've failed for writing.

    public override object ReadJson(JsonReader reader, Type objectType, object existingValue,
        JsonSerializer serializer)
    {
        if (reader.TokenType == JsonToken.Null)
        {
            return null;
        }
        else if (reader.TokenType == JsonToken.StartArray)
        {
            JArray array = JArray.Load(reader);
            var typed = array.ToObject<T[]>(); // serializer); this blows up with a deep stack trace - why?
            return typed;
        }

        JObject jObject = JObject.Load(reader);
        T value = Create(objectType, jObject);
        if (value == null)
            throw new JsonSerializationException("No object created.");
        using (JsonReader jObjectReader = CopyReaderForObject(reader, jObject))
        {
            serializer.Populate(jObjectReader, value);
        }
        AfterPopulate(value, jObject);
        return value;
    }

    protected virtual void AfterPopulate(T aModel, JObject aJObject)
    {
    }

JSON.Net has global customisation in ContractResolver, but I don't know how to access the model converter from there, and property type based JsonConverters() don't have access to the model - severely limiting their ability when the result involves other properties.

[StrangeWill]

@buzzware

By "Converter to null for this specific instance" are you losing custom configuration in any way for embedded records? By nulling the converter I'm wondering what abilities would be lost, or what internal logic would have to be duplicated (error prone) for the current model or embedded models.

Nulling the converter causes the JsonSerializer to force default JSON.net object serialization over our custom serialization for all children, so it won't apply our custom converters to the rest of the object. In my experience all the normal JSON.Net attributes continue to work as expected. Just take whatever you want to manually serialize and don't serialize it or use [JsonIgnore] (depending on how you have the serializer configured).

I couldn't find the test you refer to.

This is the test that covers my use case:

https://github.com/StrangeWill/SegmentDotNet/blob/ac514d0c270a048c243d3ea913f7c065d42d0b10/test/SegmentDotNet.Tests/Contexts/ContextCollectionTests.cs

You can see in the following files how I use a "ForcedObjectResolver" and "ForcedObjectSerializer" to do the leg work:

https://github.com/StrangeWill/SegmentDotNet/blob/ac514d0c270a048c243d3ea913f7c065d42d0b10/src/SegmentDotNet/Json/ForcedObjectResolver.cs

https://github.com/StrangeWill/SegmentDotNet/blob/ac514d0c270a048c243d3ea913f7c065d42d0b10/src/SegmentDotNet/Json/ForcedObjectSerializer.cs

https://github.com/StrangeWill/SegmentDotNet/blob/ac514d0c270a048c243d3ea913f7c065d42d0b10/src/SegmentDotNet/Contexts/Serializers/ContextBaseSerializer.cs

They're some really basic serializers so not much going on other than "serialize this custom object, one property non-standard and the rest standard JSON.Net configuration".

Would you recommend your solution as the current best practice, or is it more of a hack that won't work for all scenarios?

I recommend my solution as the _only known working solution_, I'm not _happy_ with what I have to do to make it work, and while it's working within the rules of JSON.Net, it is very much exploiting internal behaviors to accomplish this.

Being as I'm not a member of the JSON.Net team, I can't speak on best practice.

I wish I had a better way...

I've given up overriding WriteJson for now, and refactored client & server to avoid it - just too hard for me. I've gotta say I'm surprised that custom writing of JSON is so hard on this, the most popular .Net library. Its way easier on the Ruby based libraries I've used, like https://github.com/rails-api/active_model_serializers.

This really just appears to be a problem with how Serializers and Converters co-exist and interact, I bet it _could_ be fixed but this use case is so edge-case (at least that's what the popularity of this problem shows) that reworking these tools is probably not worth it.

[buzzware]

Thankyou for your indepth reply. It will help those battling with WriteJson. I got my task done by the following :

1. override CreateProperties to add a "type" property with the class name on serialize, and use a custom JsonConverter to create the right class on deserialize. This means that deserialization will create the right model classes to arbitrary depth (first major goal, ridiculously hard to figure out - surely this is no edge case? No I don't want a $type property with a full .Net class path)
2. also in the CreateProperties override I disable serialization of complex properties generally, but allow deserialization.
3. For serialize, I discovered the power of JToken.fromObject. It means we can convert our models into the generic JToken/JObject/JArray objects for manipulation _at runtime_ before writing to a string, and (I'm guessing) _no performance penalty_ since JSON.Net would internally convert to these objects anyway.
   Ideally we would be able to do something like :

JsonConvert.Serialize(order,include = new string[] {"product,customer"}) 

to selectively include children at run time, and that probably isn't hard to write a utility function for.
For now I use JToken.fromObject to convert my models to JTokens. Then I manually convert (via same method) and attach child models eg.

orderOutput["product"] = JToken.fromObject(product)

Then

JsonConvert.Serialize(orderOutput)

I haven't seen any tutorial or blog promoting this method. From my research, JSON.Net & associated docs assume that models will be serialized/deserialized the same way every time. This is simply not the case - wait till GraphQL takes off.

[aaronhoffman]

I thought I'd leave this here just in case it helps others on their journey.

( writing as of this commit: d5d338b49e37bcc8c3cd0a8ac784ce91155760eb 2018-09-13)

If you're wanting to take a look at the "default serialization", you can find it here: https://github.com/JamesNK/Newtonsoft.Json/blob/master/Src/Newtonsoft.Json/Serialization/JsonSerializerInternalWriter.cs#L149

JsonSerializerInternalWriter.SerializeValue(JsonWriter writer, object value, JsonContract valueContract, JsonProperty member, JsonContainerContract containerContract, JsonProperty containerProperty)

An extremely trivial JsonConverter might look like this:

    public class TrivialJsonConverter : JsonConverter
    {
        public override bool CanConvert(Type objectType)
        {
            return true;
        }

        public override object ReadJson(JsonReader reader, Type objectType, object existingValue, JsonSerializer serializer)
        {
            var jt = JToken.Load(reader);
            return jt.ToObject(objectType);
        }

        public override void WriteJson(JsonWriter writer, object value, JsonSerializer serializer)
        {
            var jt = JToken.FromObject(value);
            jt.WriteTo(writer);
        }

    }

[DalSoft]

You still get a JsonSerializationException: Self referencing loop detected with... that @johncrim describes in his SO if you don't override CanWrite and CanRead.

Credit to https://github.com/RicoSuter/NJsonSchema/blob/master/src/NJsonSchema/Converters/JsonInheritanceConverter.cs

    public class DefaultJsonConverter : JsonConverter
    {
        [ThreadStatic]
        private static bool _isReading;

        [ThreadStatic]
        private static bool _isWriting;

        public override void WriteJson(JsonWriter writer, object value, JsonSerializer serializer)
        {
            try
            {
                _isWriting = true;

                serializer.Serialize(writer, value);
            }
            finally
            {
                _isWriting = false;
            }
        }

        public override bool CanWrite
        {
            get
            {
                if (!_isWriting)
                    return true;

                _isWriting = false;

                return false;
            }
        }

        public override bool CanRead
        {
            get
            {
                if (!_isReading)
                    return true;

                _isReading = false;

                return false;
            }
        }

        public override bool CanConvert(Type objectType)
        {
            return true;
        }

        public override object ReadJson(JsonReader reader, Type objectType, object existingValue, JsonSerializer serializer)
        {
            try
            {
                _isReading = true;
                return serializer.Deserialize(reader, objectType);
            }
            finally
            {
                _isReading = false;
            }
        }
    }