Direct Deserialization from Binary Driver when using BoltGraphClient

[arturosevilla]

Right now the deserializar for BoltGraphClient serializes (on a per-record basis) the results into JSON, which is then sent to the existing CypherJsonDeserializer.

This set of changes removes that serialization into JSON by extracting the common functionality of CypherJsonDeserializer and creating a new DriverDeserializer. In this way a change in the deserialization algorithm is not dependent on the underlying serialization.

Summary of changes:

• Most of CommonDeserializerMethods and the structure of CypherJsonDeserializer was transformed into BaseDeserializer which is a generic abstract class that holds the skeleton of the deserializer.
• Both DriverSerializer and CypherJsonDeserializer implement BaseDeserializer.
• CustomJsonDeserializer uses CypherJsonDeserializer instead of CommonDeserializerMethods.
• Removed StatementResultHelper extension methods.
• Added DriverDeserializerTests.
• Added support for DataMemberAttribute property "renaming" while deserializing and IgnoreDataMemberAttribute when serializing.
• Added support for DataMemberAttribute and JsonProperty while serializing and using BoltGraphClient.
• Created a new interface called ITypeSerializer which substitutes JsonConverter for BoltGraphClient.

[cskardon]

Hey Arturo,

Thanks for this - obviously It'll take some time to go through this - but - when I opened it, one of the tests was failing here , so that'll need to be resolved.

I'll try to review the pull request as soon as I can!

[arturosevilla]

@cskardon I rebased but I forgot to run the tests again afterwards. The fix was rather simple: the new code uses .Values of a Dictionary object so instead of defining the values in a mock through the keys, they should be defined by setting a mock value for .Values.

[arturosevilla]

This should be labeled as Work In Progress.

Right now missing:

• CypherDeserializerTests equivalent. Working on them (called DriverDeserializerTests). (DONE)
• JsonPropertyAttribute equivalent during deserialization (serialization is handled differently and not in this scope of this PR). I intend to support DataMemberAttribute as this is also supported by Newtonsoft.Json. (DONE)
• JsonConverter is not supported for the Bolt Driver Deserializer (it makes no sense as it requires a JToken). I intend to create a ITypeSerializer interface that provides the same functionality. (DONE)

[arturosevilla]

@cskardon This is no longer Work in Progress. It could be merged as is, if the given functionality completely meets the expectations.

[cskardon]

Hey Arturo - sorry with the huge pile of GDPR emails this slipped through - I'll look at it today / tomorrow and see what I can do! - Sorry for the delay

[arturosevilla]

Hi @cskardon any update on this?

[cskardon]

Hey @arturosevilla - I am truly sorry for the time on this - it's a large change and I've unfortunately had work/life get in the way :/

[cskardon]

I need to work out what to do with this - the problem is that it's a Major version change type pull request as it breaks existing functionality - now - there's not necessarily a huge problem with that kind of thing - but - I'm wary.

[arturosevilla]

Alright. May I suggest that independently if this would make a major release, or at least a new minor with some deprecated functionality, then maybe see if these changes at least (from your point of view and the project's objectives), meet the following:

• Expected functionality. I believe it does. However, do the deprecated functionality can still be fulfilled by the new code?
• Code style.
• Readability. Would it be easy to make changes in the future?

[cskardon]

Hi @arturosevilla ,

I have a question about the TypeSerializers, in particular a use-case.
One of the original uses for them is if you're storing a class with a complex type as a property, or a dictionary etc, how would you do this, let's say I have this class:

public class MyNode {
    public string Name {get;set;}
    public ComplexType Complex {get;set;}
}

My understanding is that I would have:

public class MyNodeTypeSerializer : ITypeSerializer {
 /* Only looking at Deserializer in this case */
   public object Deserialize(Type objectType, object value){
        var node = value as INode;
        var myNode = new MyNode {
                   Name = node.Properties[nameof(MyNode.Name)],
                   ComplexType = JsonConvert.DeserializeObject<ComplexType>(node.Properties[nameof(MyNode.ComplexType)])
        };

       //etc
   }
}

My reason for using JsonConvert is that I'm working on a basis of how to translate existing JsonConverters, and those will likely use JsonConvert to serialize / deserialize - but in essence that bit doesn't matter too much.

The main bit I'm concerned with is the Name = node.Properties[nameof(MyNode.Name)] line, as these are standard properties that if I didn't have complex types, I'd be able to deserialize using your default code easily - is there a way to do that within a TypeSerializer?

In the old JsonConverter way - I would remove the complex properties from the JsonObject I had, and after that I could just do a simple JsonConvert.Deserialize - is there an equivalent?

[arturosevilla]

@cskardon I'm sorry that I hadn't followed as I was busy with other projects.

I am not sure I exactly follow your example. First of all, instead of doing

var node = value as INode;

You would do:

var record = value as IRecord;

And of course modify the rest of the code accordingly. I didn't hard coded the IRecord type into the ITypeSerializer as the purpose of having this interface is to be available for even the Http client if it really works as expected.

Now, returning to your example, I think you are trying to ask if there's a way of doing the following:

1. At first you would have an object in the graph as follows: {name:"Some name", complexType: "{\"a\":3}"}. That is you have JSON stored in the complex type, but the other property is just a regular (or standard as you put it) old-fashioned property.
2. You would do the following with JsonConverter:

var complex = JsonConvert.DeserializeObject<ComplexType>(jsonObject["complexType"]);
jsonObject.Remove("complexType");
var deserialized = JsonConvert.Deserialize(jsonObject);
deserialized.ComplexType = complex;
return deserialized;

I'm not sure if I followed exactly, but I would argue that although that might look somewhat cleaner for standard/regular properties (specially if you have a few, or at least more than the complex types), it is not a strategy that would work for everyone. For example, if instead of JSON another encoding would be used, it would depend on that other library.

I would even argue that might not be the responsibility of Neo4jClient as that is a functionality provided by the underlying library, that is, Json.NET in this case. In this case IRecord does not provide a way to delete properties (everything is read-only version of a collection).

In summary, there is no way of doing the exact same thing (if I understand your use-case correctly), but it can definitely still be used to deserialize a complex type.