JamesNK / Newtonsoft.Json

Json.NET is a popular high-performance JSON framework for .NET
https://www.newtonsoft.com/json
MIT License
10.73k stars 3.25k forks source link

System.ArgumentException : Could not determine JSON object type for type System.Collections.Generic.KeyValuePair`2[Newtonsoft.Json.Linq.JObject,Newtonsoft.Json.Linq.JObject]. #2844

Open cschuchardt88 opened 1 year ago

cschuchardt88 commented 1 year ago

Source/destination types

using Neo.VM.Types;
using Newtonsoft.Json;
using Newtonsoft.Json.Linq;
using Array = Neo.VM.Types.Array;
using Boolean = Neo.VM.Types.Boolean;
using Buffer = Neo.VM.Types.Buffer;

namespace Neo.Events.Tools.Json
{
    public class StackItemJsonConverter : JsonConverter<StackItem>
    {
        public override StackItem ReadJson(JsonReader reader, Type objectType, StackItem existingValue, bool hasExistingValue, JsonSerializer serializer)
        {
            throw new NotImplementedException();
        }

        public override void WriteJson(JsonWriter writer, StackItem value, JsonSerializer serializer)
        {
            var o = ToJObject(value, null);
            o.WriteTo(writer);
        }

        private JObject ToJObject(StackItem item, IList<(StackItem, JObject)> context)
        {
            JObject o = null;
            switch (item)
            {
                case Array array:
                    if (context is null)
                        context = new List<(StackItem, JObject)>();
                    else
                        (_, o) = context.FirstOrDefault(f => ReferenceEquals(f.Item1, item));
                    if (o is null)
                    {
                        o = new()
                        {
                            new JProperty("type", StackItemType.Array.ToString()),
                            new JProperty("size", array.Count),
                        };
                        context.Add((item, o));
                        o.Add(array.Select(s => ToJObject(s, context)));
                    }
                    break;
                case Map map:
                    if (context is null)
                        context = new List<(StackItem, JObject)>();
                    else
                        (_, o) = context.FirstOrDefault(f => ReferenceEquals(f.Item1, item));
                    if (o is null)
                    {
                        context.Add((item, o));
                        var jo = map.Select(s => new KeyValuePair<JObject, JObject>(ToJObject(s.Key, context), ToJObject(s.Value, context)));
                        o = new()
                        {
                            new JProperty("type", StackItemType.Map.ToString()),
                            new JProperty("size", map.Count),
                            new JProperty("value", jo),
                        };
                    }
                    break;
                case Boolean _:
                    o = new()
                    {
                        new JProperty("type", StackItemType.Boolean.ToString()),
                        new JProperty("value", item.GetBoolean()),
                    };
                    break;
                case Buffer buffer:
                    o = new()
                    {
                        new JProperty("type", StackItemType.Buffer.ToString()),
                        new JProperty("value", buffer.GetSpan().ToArray()),
                    };
                    break;
                case ByteString array:
                    switch (array.Size)
                    {
                        case UInt160.Length:
                            o = new()
                            {
                                new JProperty("type", nameof(UInt160)),
                                new JProperty("value", new UInt160(array.GetSpan()).ToString()),
                            };
                            break;
                        case UInt256.Length:
                            o = new()
                            {
                                new JProperty("type", nameof(UInt256)),
                                new JProperty("value", new UInt256(array.GetSpan()).ToString()),
                            };
                            break;
                        default:
                            o = new()
                            {
                                new JProperty("type", StackItemType.ByteString.ToString()),
                                new JProperty("value", array.GetSpan().ToHexString()),
                            };
                            break;
                    }
                    break;
                case Integer i:
                    o = new()
                    {
                        new JProperty("type", StackItemType.Integer.ToString()),
                        new JProperty("value", i.GetInteger()),
                    };
                    break;
                case InteropInterface i:
                    o = new()
                    {
                        new JProperty("type", StackItemType.InteropInterface.ToString()),
                    };
                    break;
                case Pointer pointer:
                    o = new()
                    {
                        new JProperty("type", StackItemType.Pointer.ToString()),
                        new JProperty("vaule", pointer.Position),
                    };
                    break;
                case Null _:
                    o = new()
                    {
                        new JProperty("type", StackItemType.Any.ToString()),
                    };
                    break;
                default:
                    throw new NotImplementedException($"StackItemType({item.Type}) is not supported to JSON.");
            }
            return o;
        }
    }
}

expected Output

{
  "type": "Map",
  "size": 2,
  "value": [
    {
      "Key": {
        "type": "Integer",
        "value": 0
      },
      "Value": {
        "type": "Integer",
        "value": 1
      }
    },
    {
      "Key": {
        "type": "Integer",
        "value": 2
      },
      "Value": {
        "type": "Integer",
        "value": 3
      }
    }
  ]
}

Expected behavior

See output

Actual behavior

Get Error System.ArgumentException : Could not determine JSON object type for type System.Collections.Generic.KeyValuePair`2[Newtonsoft.Json.Linq.JObject,Newtonsoft.Json.Linq.JObject].

Steps to reproduce

[Fact]
        public void MapOuput_StackItemJsonConverter()
        {
            var map = new Map()
            {
                [0] = 1,
                [2] = 3,
            };

            var json = JsonConvert.SerializeObject(
                                    map, Formatting.Indented,
                                    new StackItemJsonConverter()
                                );

            Console.WriteLine(json);
        }

This is a work around.

var jo = map.Select(s => new KeyValuePair<JObject, JObject>(ToJObject(s.Key, context), ToJObject(s.Value, context)));
                        var j = JsonConvert.SerializeObject(jo);
                        o = new()
                        {
                            new JProperty("type", StackItemType.Map.ToString()),
                            new JProperty("size", map.Count),
                            new JProperty("value", JArray.Parse(j)),
                        };
elgonzo commented 1 year ago

Okay, lets start with distilling your issue report to the essential code necessary to reproduce the problem:

var jobjectKeyValuePair = new KeyValuePair<JObject, JObject>(
    new JObject { new JProperty("type", "foo"), new JProperty("value", 1) },
    new JObject { new JProperty("type", "bar"), new JProperty("value", 2) }
);

//
// Creating this JObject instance with jobjectKeyValuePair as value for one of
// its properties goes kaboom!
//
var jo = new JObject
{
    new JProperty("foobar", jobjectKeyValuePair)
};

The issue is in you using a data type like KeyValuePair that is not supported by JValue (well, it's not the real issue, but more on that below). When you pass a content object that's not a JToken (or derived from it) to a JProperty, that content object is basically wrapped in a JValue instance when the content object is not an IEnumerable. If the content object is an IEnumerable, then the elements of the IEnumerable are wrapped individually in JValue instances and put into an JArray instance. But JValue does not (and therefore neither JProperty) attempt to convert a given content object (or the elements of an IEnumerable passed as content object) if it is not one of the data types it supports. It requires the content object to be one of the supported data types.

And that's where the actual and real issue manifests: Newtonsoft.Json's documentation. Nowhere in its documentation could i find any mention of what is and what is not allowed to pass as content object to a JValue, and therefore as content to a JProperty. Only by studying its source code (specifically the JValue.GetValueType method here: https://github.com/JamesNK/Newtonsoft.Json/blob/0a2e291c0d9c0c7675d445703e51750363a549ef/Src/Newtonsoft.Json/Linq/JValue.cs). As said, i found nothing in Newtonsoft.Json's documentation that describes these restrictions on the data types usable as content objects for JValue/JProperty.

One workaround, as you found out, is avoiding the use of KeyValuePair as content object by simply serializing the KeyValuePair enumerable and deserializing/parsing the resulting json string into a JToken representation, thus the JToken representation being built never encountering unsupported KeyValuePair instances at all.

Another workaround that would perform better than serialization followed by deserialization/parsing would be to avoid KeyValuePair instances by "simply" creating JObject instances (with "key" and "value" properties) instead of KeyValuePairs in your converter.

Still, despite these workarounds, the documentation needs to be improved. Badly. But i don't hang my hopes up high for that to happen, sadly...