Backport OffsetDateTime serializers in GLVs to enable deserialization from server #3168
Conversation
… from server. Date remains the default serializer for GLV native date types.
Do we not need to also support serialization of |
| const string graphSon = "{\"@type\":\"gx:OffsetDateTime\",\"@value\":\"2016-10-04T12:17:22.5520000+00:00\"}"; | ||
| var reader = CreateStandardGraphSONReader(version); | ||
|
|
||
| var jsonElement = JsonSerializer.Deserialize<JsonElement>(graphSon); | ||
| var deserializedValue = reader.ToObject(jsonElement); | ||
|
|
||
| var expectedDateTimeOffset = TestUtils.FromJavaTime(1475583442552); |
There was a problem hiding this comment.
| const string graphSon = "{\"@type\":\"gx:OffsetDateTime\",\"@value\":\"2016-10-04T12:17:22.5520000+00:00\"}"; | |
| var reader = CreateStandardGraphSONReader(version); | |
| var jsonElement = JsonSerializer.Deserialize<JsonElement>(graphSon); | |
| var deserializedValue = reader.ToObject(jsonElement); | |
| var expectedDateTimeOffset = TestUtils.FromJavaTime(1475583442552); | |
| const string dateTimeString = "2016-10-04T12:17:22.5520000+00:00"; | |
| const string graphSon = "{\"@type\":\"gx:OffsetDateTime\",\"@value\":\"" + dateTimeString + "\"}"; | |
| var reader = CreateStandardGraphSONReader(version); | |
| var jsonElement = JsonSerializer.Deserialize<JsonElement>(graphSon); | |
| var deserializedValue = reader.ToObject(jsonElement); | |
| var expectedDateTimeOffset = DateTimeOffset.Parse(dateTimeString); |
There was a problem hiding this comment.
I just want to clarify that I'm understanding this correctly. This OffsetDateTimeSerializer will not be used until 3.8 and is not being used in 3.7.4. This was not added to the GraphSONWriter.cs logic because for 3.7.4 we are still serializing DateTimeOffset as Date and there is no way to write OffsetDateTime to a request.
Is my understanding correct? If so, is there any value with adding this OffsetDateTimeSerializer?
There was a problem hiding this comment.
Yes, that is correct. This is more of a connivence back-port for the ability to deserialize potential OffsetDateTime from server in the GLVs (Java has the support already). I don't quite expect this to be used per se, but the difference would be a deserialization error vs successful deserialization.
Potential downside is that OffsetDateTime will not be able to round-trip, given GLVs still use Date as default type for native date supports.
| const obj = { "@type" : "gx:OffsetDateTime", "@value" : "2016-12-14T21:14:36.295Z" }; | ||
| const reader = new GraphSONReader(); | ||
| const result = reader.read(obj); | ||
| assert.ok(result instanceof Date); |
There was a problem hiding this comment.
Can you add some verification of the day, month, year values etc?
| @@ -343,6 +343,43 @@ def objectify(cls, buff, reader, nullable=True): | |||
| nullable) | |||
|
|
|||
|
|
|||
| class OffsetDateTimeDeserializer(_GraphBinaryTypeIO): | |||
There was a problem hiding this comment.
| class OffsetDateTimeDeserializer(_GraphBinaryTypeIO): | |
| class OffsetDateTimeIO(_GraphBinaryTypeIO): |
Given OffsetDateTime serializers were implemented in master/3.8, this is a back-port for convenience of the ability to deserialize from OffsetDateTime into native date types. Do note that while the serializers were backported for some GLVs h, Date remains the default serialized types for GLV native date types.