Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

toJSON of a host object #122

Open
jsobell opened this issue Apr 12, 2018 · 18 comments
Open

toJSON of a host object #122

jsobell opened this issue Apr 12, 2018 · 18 comments
Labels

Comments

@jsobell
Copy link
Contributor

jsobell commented Apr 12, 2018

At the moment there doesn't seem to be any converting a native object to JSON. Adding toJSON to a .NET class results in a serialised string, and while you can do an external JSON.stringify of result.Value, this obviously fails for embedded javascript objects in the array.
It might be best to provide a hook on the context that is called by the JSON.cs when it finds a value is not a JSValue, then the user can decide the serialised content to return?
There are a few places this is an issue, such as Lists of values coming back as

{
  "0": {
  },
  "1": {
  },
  "2": {
  },
  "3": {
  },
  "4": {
:
:

which is unrelated to the data contained in the List.

@jsobell
Copy link
Contributor Author

jsobell commented Apr 12, 2018

After a little experimenting I can see that toJSON expects a JSValue to be returned, which of course means duplicating objects and collections to make them able to be stringifyd. This sort of thing works in some situations, but to make objects serialisable you have to do something like subclass them from a base class the exposes a public JSValue toJSON(), creates a JSObject, then uses reflection to add each property as a JSValue so it's parsed into JSON.
The biggest drawback here is that every nested object needs to go through the same process. If we're using DTO objects generated externally we can't add methods, so we probably either need a callback at the Context level such as string SerializeToJSON(object obj) or we need to have the existing JSON.serialize use reflection to identify serializable properties properly on non JSValue objects.
Any suggestions?

@nilproject
Copy link
Owner

A difficult situation. I'm thinking, you can add implementation of toJSON in prototype inside of JS environment. You can implement it in any other type, just create proxy for this type (with DefineConstructor or Marshal) and modify prototype.

@jsobell
Copy link
Contributor Author

jsobell commented Apr 13, 2018

It appears the main issue is that external JSON serializers don't understand the JSValue objects, even when they have base data types. For instance, calling NewtonSoft's serializer with a variable assigned with the number 9 results in [] because only your own JSON.stringify knows to drill into the .Value of the JSValue object to get the value to serialize.
So the question is whether we modify external serializers to recognise that JSValue instances need special handling, or modify JSON.stringify to understand that .NET objects are not enumerable and GetProperties must be used to iterate over the serializable properties.

@nilproject
Copy link
Owner

nilproject commented Apr 13, 2018

Because we cannot modify external serializer, need to use JSON.stringify. My solution allow do not modify this method.

public sealed class MyFirstDTO
{
    public int Field1 { get; set; }
    public int Field2 { get; set; }
}

public sealed class MySecondDTO
{
    public int Field3 { get; set; }
    public int Field4 { get; set; }
}

public sealed class MyDtoSerializer
{
    public static object SerializeToJSObject(MyFirstDTO myDto)
    {
        // Some serializer
        return serializeToJSObject(myDto);
    }

    public static object SerializeToJSObject(MySecondDTO myDto)
    {
        return serializeToJSObject(myDto);
    }

    private static object serializeToJSObject(object obj)
    {
        if (obj == null)
            return null;

        var result = JSObject.CreateObject();
        var properties = obj.GetType().GetProperties();
        foreach (var prop in properties)
        {
            result[prop.Name] = JSValue.Marshal(prop.GetValue(obj));
        }

        return result;
    }
}

static void Main(string[] args)
{
    var context = new Context();

    var jsObject = JSObject.CreateObject();
    jsObject["dto1"] = JSValue.Marshal(new MyFirstDTO { Field1 = 777, Field2 = 420 });
    jsObject["dto2"] = JSValue.Marshal(new MySecondDTO { Field3 = 123, Field4 = 456 });

    // this need to do only once
    var serializerConstrucor = context.GlobalContext.GetConstructor(typeof(MyDtoSerializer));
    var serializeFunction = serializerConstrucor["SerializeToJSObject"];
    context.DefineVariable("serializeDtoFunction").Assign(serializeFunction);
    var toJsonFunction = context.Eval("(function() { return serializeDtoFunction(this) })");
    context.GlobalContext.GetConstructor(typeof(MyFirstDTO)).As<Function>().prototype["toJSON"] = toJsonFunction;
    context.GlobalContext.GetConstructor(typeof(MySecondDTO)).As<Function>().prototype["toJSON"] = toJsonFunction;

    context.DefineVariable("test").Assign(jsObject);
    Console.WriteLine(context.Eval("JSON.stringify(test)")); // {"dto1":{"Field1":777,"Field2":420},"dto2":{"Field3":123,"Field4":456}}
}

Is this solution appropriate?

@jsobell
Copy link
Contributor Author

jsobell commented Apr 13, 2018

Not really. This assumes users are happy to create and assign an unknown number of these for every .NET object they want to be able to convert to JSON.
I think the main thing here is that this library is always going to be compared to ClearScript, and while your library is significantly faster than ClearScript when integrating .NET objects, situations like this mean special handling of object instances depending on their source.
I see this issue because I've been replacing ClearScript in our solution, but this is one example of where the whole underlying system would need reworking to provide compatibility, plus the above method will result in unexpected missing data when developers forget to add new class types.

@jsobell
Copy link
Contributor Author

jsobell commented Apr 14, 2018

Is there a problem with performing this within the JSON class? :

                res = new StringBuilder(obj is Array ? "[" : "{");

                string prevKey = null;

                if (!(obj.Value is JSValue) && obj.Value is object)
                {
                    JSObject native = JSObject.CreateObject();
                    foreach (var prop in obj.Value.GetType().GetProperties())
                    {
                        native[prop.Name] = JSValue.Marshal(prop.GetValue(obj.Value));
                    }
                    obj = native;
                }
                
                foreach (var member in obj)
                {

This recurses down the nested native objects properly, but will still have issues with things like Lists and Dictionaries which won't be serialised.

@nilproject
Copy link
Owner

nilproject commented Apr 14, 2018

It will break stringification for external types (and some from BaseLibrary), which not derived from JSValue. For example:

var s = new Set();
s[1] = 1;
JSON.stringify(s); // should be "{\"1\":1}", but will be "{\"size\":0}"

I have already begun work on support of external "stringificators". I need a few days.

@nilproject
Copy link
Owner

nilproject commented Apr 15, 2018

Do your DTOs have a common base type other than object? I cannot find a correct solution without a base type or list of types which should be serialized into json.

@jsobell
Copy link
Contributor Author

jsobell commented Apr 15, 2018

We can't expect everyone to have a base class for every object they want to serialise to JSON.
There is this option (not fully tested):

    public class JSValueConverter : JsonConverter<JSValue>
    {
        public override void WriteJson(JsonWriter writer, JSValue value, JsonSerializer serializer)
        {
            serializer.Serialize(writer, value.Value);
        }

        public override JSValue ReadJson(JsonReader reader, Type objectType, JSValue existingValue,
            bool hasExistingValue, JsonSerializer serializer)
        {
            return null;
        }
    }
    
    public class JSON2
    {
        public string stringify(object o)
        {
            return JsonConvert.SerializeObject(o, new JSValueConverter());
        }        
    }

This tells Newtonsoft to use the custom deserialize for JSValue objects, and that tells it to serialize the .Value property of any JSValue object.

@jsobell
Copy link
Contributor Author

jsobell commented Apr 15, 2018

Hmm, that still has an issue with Set. I'm going to leave this to you for now. I can't grasp the combinations of JSObject and things like Set to work out how to identify what each JSValue holds.

@nilproject
Copy link
Owner

nilproject commented Apr 16, 2018

This problem is very complex. At this moment for external types engine tries to simulate behaviour as a native js types (constructor, prototype, prototypeInstance and other), but DTOs does not fit into this behaviour. We should have a some sign for types, which instances should be serialized like a Dictionary-like objects. Attribute, interface, base type, list of this type, anything. We cannot change this behaviour for all external types, because this solution will break existed code.

@jsobell
Copy link
Contributor Author

jsobell commented Apr 16, 2018

Yes, although it doesn't matter if it's a DTO or any other type of .NET object, the same problem exists.
The fundamental problem is that javascript script engines like this are generally used to evaluate something, which means the user must always be able to get the result out of the library in some way.
Since results can be absolutely any type of structure, e.g. Array, number, string, object, dictionary, etc. there are a couple of ways to return results. One is to use dynamic, and the other is to wrap them as you do. Now the question is how to get the value out of the wrapper.
In our system we serialize any result to JSON, as that way we guarantee the consumer can read the values. We don't care that it's no longer in the original .NET object, because we can parse any JSON back into a dynamic object if we want to.
But... in NiL.js at the moment we can't do that. If we say object result = engine.Eval("...") we may get any combination of arrays, strings, numbers, or .NET objects in an ObjectWrapper. In theory we could use GetType to see if it's a JSValue then read the .Value, but the value in there gets messy, particularly if it's an ObjectWrapper because that's internal. We also can't get the internal fields such as _oValue as they are internal too, so it's not possible to replicate the JSON.cs code outside.
So we really should be using the built-in JSON.stringify() command to return the result, but as soon as we return a GUID, List<>, Dictionary<>, or any external .NET class it returns an empty object.
If an external class contains a property of type JSValue it should still serialize correctly in your code, which is why I think we need to have a hook in JSON.cs that says "This is an object that I don't know how to serialize", and calls a function delegate passing the StringBuffer, and allowing recursive calls for the function to pass back child JSValue fields if it finds them. We can also have a set of predefined serializers for the internal types such as Set (which doesn't actually serialise at all but includes the added properties).

The main issue I have is understanding what your stringify function does, particularly when the comments are in Russian :)

@jsobell
Copy link
Contributor Author

jsobell commented Apr 16, 2018

As a reference object, here's the one we use for testing:

            engine.DefineVariable("Info").Assign(JSValue.Marshal(new Info
            {
                FirstName = "Fred", 
                LastName = "Boulder", 
                Child= new Info
                {
                    FirstName = "Pebbles", LastName = "Boulder",
                    DOB = JSValue.Marshal(new Date(DateTime.Parse("27 Jan 2010")))
                },
                DOB = JSValue.Marshal(new Date(DateTime.Parse("21 Feb 1974"))),
                List = new List<Object> { "A", "B","C",1,2,3},
                Dict = new Dictionary<int, string> { {1, "A"}, {4,"B"},{99,"C"}},
                myset = new Set()
            }));
            
            engine.Eval("Info.myset['p1']='one'; Info.myset['p2']='two'; Info.myset.add(1);");

            Console.WriteLine(engine.Eval("JSON.stringify(Info)").Value);

Info:


    public class Info
    {
        public Info()
        {
            DOB = JSValue.Marshal(new Date(DateTime.UtcNow));
        }

        public string FirstName { get; set; }
        public string LastName { get; set; }
        public JSValue DOB { get; set; }
        public Info Child { get; set; }
        private bool Hidden { get; set; }
        public Set myset { get; set; } = new Set();
        public List<Object> List { get; set; } = new List<Object>();
        public Dictionary<int, string> Dict { get; set; } = new Dictionary<int, string>();
    }

@nilproject
Copy link
Owner

nilproject commented Apr 17, 2018

So, I implemented some solution. By default List, Dictionary and other complex types does not serialize, but support of them can be implemented simply. I tested this with code bellow

public class MyFirstDTO
{
    public int Field1 { get; set; }
    public int Field2 { get; set; }
}

public sealed class MySecondDTO : MyFirstDTO
{
    public int Field3 { get; set; }
    public int Field4 { get; set; }
    public MyFirstDTO Child { get; set; }
}

private static void serialization()
{
    var context = new Context();

    var jsObject = JSObject.CreateObject();
    jsObject["dto1"] = JSValue.Marshal(new MyFirstDTO { Field1 = 777, Field2 = 420 });
    jsObject["dto2"] = JSValue.Marshal(new MySecondDTO { Field3 = 123, Field4 = 456, Child = new MyFirstDTO { Field1 = 789, Field2 = 101112 } });

    context.GlobalContext.JsonSerializersRegistry.AddJsonSerializer(new JsonSerializer(typeof(MyFirstDTO)));
    context.GlobalContext.JsonSerializersRegistry.AddJsonSerializer(new JsonSerializer(typeof(MySecondDTO)));

    context.DefineVariable("test").Assign(jsObject);
    Console.WriteLine(context.Eval("JSON.stringify(test)"));
}

@jsobell
Copy link
Contributor Author

jsobell commented Apr 20, 2018

OK, but does this mean the developer has to add every framework object they may ever serialise to the JsonSerializerRegistry?

@nilproject
Copy link
Owner

Yes. Every object or them base class.

@jsobell
Copy link
Contributor Author

jsobell commented Apr 20, 2018

Yeah, this isn't going to be a very practical option. Why not add:

context.GlobalContext.JsonSerializersRegistry.AddJsonSerializer((obj) => mySerializer(obj))

Then the user can programmatically decide if they are able to serialize the object or not?

@nilproject
Copy link
Owner

nilproject commented Apr 20, 2018

Create your own serializer as a subclass of JsonSerializer, specify targetType as the object, override CanSerialize and Serialize and add it into registry. That's allows you to implement behaviour that you want

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants