CARVIEW |
Navigation Menu
-
Notifications
You must be signed in to change notification settings - Fork 37
Description
Hi there,
First, thank you so much for all your work on this C# RethinkDB driver.
You write very clean code. I was able to examine the source code and figure it out. Your code was wonderfully easy to comprehend. Thank you for doing such a great job!
Now, some issues/suggestions/solutions/feedback:
Serialization
Conceptually, in my view, the restriction to explicitly mark every [DataMember]
and every [DataContract]
for each aggregate/domain object and every field is quite cumbersome. I understand it's a WCF concept, indeed, even the MVC crew in its early days required to explicitly mark every attribute, data contract, and HTTP action.
Eventually, MVC & WebAPI teams adopted the convention that everything was "on / opt-in" by default without having to explicitly mark every data contract, HTTP action, etc. When a developer explicitly sets up Attributes it usually conveys the notion of an "explicit override".
JSON.NET (Newtonsoft.Json) serializer, follows the same convention:
By default (without any attribute decorations), an object serialized by JsonConvert.Serialize()
all public members are selected for serialization.
This becomes really important when you don't have mutable access to object model definitions that exist inside the .NET framework. For example, suppose you had the following object model for ASP.NET's new User Identity framework:
[DataContract}
public class Account : Document<string>, IUser
{
public Account()
{
this.Logins = new List<UserLoginInfo>();
}
[DataMember]
public string PasswordHash { get; set; }
[DataMember]
public string UserName { get; set; }
[DataMember]
public string SecurityStamp { get; set; }
[DataMember]
public List<UserLoginInfo> Logins { get; set; }
}
//Microsoft.AspNet.Identity.Core.dll
public sealed class UserLoginInfo
{
public string LoginProvider { get; set; }
public string ProviderKey { get; set; }
}
Using [DataContract]
is great, up until the point where you need to serialize User.Logins
property which is of type UserLoginInfo
. UserLoginInfo
resides inside Microsoft.AspNet.Identity.Core.dll
and it is not decorated with [DataContract]
. Worse yet, UserLoginInfo
is sealed. This leaves me in a corner. I want to use RethinkDB serialization, but I can't decorate UserLoginInfo
with the required [DataContract]
.
This is why I think following the convention that all public properties are "opt-in" by default makes more sense. [DataMember]
should be thought of as supplementing the serialization process, not a requirement for serialization. IMHO.
Also, I'm not really a fan of decorating everything with [DataContract]
and [DataMember]
; since it only adds extra code to my domain models.
JSON.NET (Newtonsoft.Json) Datum Conversion
So, realizing this limitation, I set out to write my own custom serializer that leveraged the popular Newtonsoft.Json serializer. Basically, I wrote my own custom DatumReader
and DatumWriter
for Json.NET. In essence, instead of reading and writing JSON text, the Json.NET serializer reads and writes "datums" for RethinkDB.
The primary benefit, is I get to use all the serialization facilities, capabilities and customization that come with Json.NET https://james.newtonking.com/json.
Summary
I'd be more than happy to contribute my DatumReader
and DatumWriter
code to the RethinkDB C# driver project, but there are a few points:
Cons:
DatumReader
andDatumWriter
require a reference to Newtonsoft.Json.- Ultimately, the Json.NET converter is slower than raw
Reflection.Emit
IL gen-edDataContractDatumConveters
.
Pros:
- Alleviates tons of serialization issues. Dictionary serialization, etc.
- More developer friendly.
- Conventions based - IE: All public methods are serialized by default; not requiring explicit marker attributes.
- Leverages an already popular .NET serialization library.
Perhaps the Json.NET serializer code could live in a separate assembly for those who want it. I don't know, but I thought maybe you might be interested.
Thanks,
Brian