protobuf-csharp-port
protobuf-csharp-port copied to clipboard
Published
20 hours ago •
jskeet
jskeet
Specialized transcoding to/from Json for specific types
trafficstars
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 [email protected] on 15 May 2012 at 5:24
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
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 [email protected] on 15 May 2012 at 5:47
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 [email protected] on 16 May 2012 at 8:24
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
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 [email protected] on 17 May 2012 at 7:52
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 [email protected] on 18 May 2012 at 12:27
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
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 [email protected] on 22 May 2012 at 12:21
Attachments:
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
@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 [email protected] on 29 May 2012 at 3:55