Specialized transcoding to/from Json for specific types

[GoogleCodeExporter]

I have a custom DateTimeProto and GuidProto classes that are binary structures 
which is great for talking to an embedded device running protocol buffers in 
C++.

On the C# side, I use the same protocol buffer messages and output Json to a 
web service.  For those specific types, I would like to input/output specific 
strings in standard string formats instead of a structure.

For example, instead Json like 
"CreateDate": {
    "year": 2009,
    "month": 2,
    "day": 11,
    "hour": 10,
    "minute": 35,
    "second": 19,
    "millisecond": 510,
    "kind": "UNSPECIFIED"
}

I want something that jscript libraries can understand (rfc3339) and is compact:
CreateDate : "2009-02-11T10:35:19.510-00:00"

The same for GUID, input/output a string on the Json side and binary numbers on 
the protocol buffers side.  The reason is that a formatted string in Json is 
much more compact and readable than in the structure format.

I have been looking at subclassing JsonFormatReader and JsonFormatWriter to 
implement exceptions for my specific cases.  I'm not sure if this is the 
correct approach or not.  It looks pretty abstract and changing it for such a 
specific case would not be very reusable.

Another option would be to have a "custom format" interface and then add that 
as a partial to the generated DateTimeProto and GuidProto and then skip the 
reading and writing of all the fields and use the interface functions to 
directly handle the serialization and deserialization to/from Json for those 
objects.

Any thoughts?

Original issue reported on code.google.com by nat...@nkbrown.us on 15 May 2012 at 5:24

[GoogleCodeExporter]

I'm afraid I don't have any time to implement this in the short/medium term. We 
*could* potentially add some sort of optional interface, implemented via 
partial classes on the message and its builder, but it's not terribly ideal.

Original comment by jonathan.skeet on 15 May 2012 at 5:27

[GoogleCodeExporter]

I was going to go with locally subclassing the JsonFormatWriter/Reader and add 
whatever I need to there, but was looking for some feedback on what would be 
ideal.

Original comment by nat...@nkbrown.us on 15 May 2012 at 5:47

[GoogleCodeExporter]

I tried with the interface, and I didn't like it that much.  Instead I tried 
again with a JsonConverter class, like Newtonsoft.Json (Json.Net) has and it 
seemed to work pretty well.

I did it by extending JsonFormatReader and JsonFormatWriter in my local project 
and adding the converter stuff there.  The trouble is that JsonFormatReader and 
JsonFormatWriter have key extension points locked down, like private inheriting 
concrete classes that make it impossible to extend without modification.

The first change to protobuf-csharp should be to just open those classes and 
move JsonTextWriter and JsonStreamWriter into a separate class so they don't 
inherit from JsonFormatWriter.  I'll put together a patch for that.

Original comment by nat...@nkbrown.us on 16 May 2012 at 8:24

[GoogleCodeExporter]

I'm certainly happy to look at a patch, but I personally generally prefer 
composition over inheritance in various places. I think I'll have to have a 
look at various options before putting anything into the code base.

Of course, you'd be welcome to use your own fork in the meantime :)

Original comment by jonathan.skeet on 16 May 2012 at 8:51

[GoogleCodeExporter]

Can you please review the two commits I made?  I separated them, so you can 
decide how far you want to go with this.

The first commit, eb6d0ea94e05, enables the extensability needed, some of it is 
required even if the JsonConverter functionality is built in.
http://code.google.com/r/nathan-protobuf-csharp-port/source/detail?r=eb6d0ea94e0
58fdd31a1524df449f172e99f995c&name=issue-44

The second commit, 9d5c01bc5e20, builds on that and builds JsonConverter 
functionality in.
http://code.google.com/r/nathan-protobuf-csharp-port/source/detail?r=9d5c01bc5e2
00823c084c1773db7c3a49007d6b5&name=issue-44

Thanks

Original comment by nat...@nkbrown.us on 17 May 2012 at 7:52

[GoogleCodeExporter]

Nathan,

Not sure if we want to expose the inner workings of these classes as they could 
change drastically in the future.  This was the reason they are essentially 
sealed classes (you can't derive from them).

One viable option would be to replicate the entire JsonReader/Writer classes in 
your local project.  You should have no issues deriving from the 
AbstractTextReader/Writer classes to perform whatever magic you need.  This 
really isn't that much code and will isolate from changes to those classes in 
the future.

The other option of course is to implement the ICodedInput/OutputStream 
interfaces and aggregate the behavior of the json/xml reader/writer.  Mostly 
you will pass-through the calls directly to the underlying implementation.  The 
special casing should be able to be handled in WriteMessage(...) for the 
ICodedOutputStream, and ReadMessage(...) for ICodedInputStream.  I think this 
option would prove the most durable against future changes if you can make it 
work for you.

If you what to pursue the later option here let me know, I'll lend you what aid 
I can.

Original comment by Grig...@gmail.com on 18 May 2012 at 12:27

[GoogleCodeExporter]

I agree with Roger - I'd rather not start making the guts internal. Once folks 
(such as yourself) have started taking dependencies on the implementation 
details, it's going to make it much harder to change them later.

Given that Roger's worked much more on the JSON than I have, I'd suggest you 
take him up on his offer of help ;)

Original comment by jonathan.skeet on 18 May 2012 at 7:42

[GoogleCodeExporter]

Nathan,

The attached zip file contains three sources files than can be added to the 
"ProtocolBuffers.Test" project.  Two of these, AggregateInputStream.cs and 
AggregateOutputStream.cs are simple aggregations of the reader/writer 
interfaces.  The last file, TestCustomReaderWriter.cs, contains derivations 
that customize the behavior of ReadMessage and WriteMessage to perform the same 
behavior as your example unit test.  The test also demonstrates that this 
technique can be applied to any format.

If Jon agrees, I would be very willing to include the AggregateXxxxStream 
objects as these are quite difficult to produce due to the verbosity of the 
interfaces.  Additionally having these defined in the library would isolate 
implementations from minor interface changes.  

Anyway, take a look at the CustomMessageReader/CustomMessageWriter 
implementations for your example and let me know what you think.

Original comment by Grig...@gmail.com on 22 May 2012 at 12:21

Attachments:

• AggregateStreamExample.zip

[GoogleCodeExporter]

I think I'd called it DelegatingMessageReader (etc) instead of Aggregate - it's 
not aggregating multiple streams, after all. But the overall approach seems 
sound, yes. I think such code will always have to be written pretty carefully, 
mind you.

I'd also prefer the InnerStream to be a property instead of a protected field, 
but that's a minor detail :)

Original comment by jonathan.skeet on 23 May 2012 at 7:20

[GoogleCodeExporter]

@Jon, No problem on the rename and use of property.

@Nathan, Have you had the chance to try this out and see if it works for you?

Original comment by Grig...@gmail.com on 29 May 2012 at 3:55

[GoogleCodeExporter]

Original comment by Grig...@gmail.com on 11 Oct 2012 at 12:07

• Changed state: Accepted

[GoogleCodeExporter]

Original comment by Grig...@gmail.com on 11 Oct 2012 at 12:07