CBC Task 1.5 - Rewrite section for JSON-based serialization per PR #14530 / #3010 #56
Conversation
…zation based on the changes in the following PRs: 1. DynamoDS/Dynamo#14530 2. DynamoDS/DynamoRevit#3010 Changes made: Modified the code snippets Removed the warning message about not to rely on the serialized format of the data Modified the trace object serialization content. Added "Compatibility" section and the warning message displayed for legacy bindings.
|
@mjkkirschner As Aaron is away, are you able to review this? Weirdly, not able to add you as reviewer. |
|
I'll review yes. |
| * TraceSerializer | ||
| * Serializes `ISerializable` and `[Serializable]` marked classes into trace. | ||
| * Handles serialization and deserialization of data into trace. | ||
| * TraceBinder controls binding deserialized data to a runtime type. (creates instance of real class) |
There was a problem hiding this comment.
trace binder no longer exists.
| * TraceBinder controls binding deserialized data to a runtime type. (creates instance of real class) | |
| * JSON deserialization is generally used to read data from trace. |
There was a problem hiding this comment.
Removed TraceBinder and added content for TraceSerializer
| * DesignScript function | ||
| * Custom node (DS function) | ||
| * TraceSerializer | ||
| * Serializes `ISerializable` and `[Serializable]` marked classes into trace. |
There was a problem hiding this comment.
Added this to the TraceSerializer content
|
|
||
| ``` | ||
|
|
||
| it is _NOT_ advisable to depend on the format of the serialized base64encoded data. |
There was a problem hiding this comment.
you should not remove this. The data is still base64 encoded and additionally it's gzipped as well now.
There was a problem hiding this comment.
Reverted the delete
|
|
||
| It also allows you to serialize arbitrary data into the .dyn file when writing zero touch dynamo nodes. This is not generally advisable as it means the potentially transferable zero touch code now has a dependency on dynamo core. | ||
|
|
||
| Do not rely on the serialized format of the data in the .dyn file - instead use the \[Serializable] attribute and interface |
There was a problem hiding this comment.
you should not remove this completely. It's still true as I commented above that the serialized data is not to be read directly. It should be accessed through the trace apis.
There was a problem hiding this comment.
Reverted the delete and removed the following content 'instead use the [Serializable] attribute and interface' based on your feedback.
| ``` | ||
|
|
||
| [GetTraceDataForNodes](https://github.com/DynamoDS/Dynamo/blob/master/src/Engine/ProtoCore/RuntimeData.cs#L218) | ||
| [GetTraceDataForNodes](https://github.com/DynamoDS/Dynamo/blob/master/src/Engine/ProtoCore/RuntimeData.cs#L213) |
There was a problem hiding this comment.
it might be a good idea to swap all of these links to stable lines of code - for example like in the 4.0 or 3.0 release branches:
https://github.com/DynamoDS/Dynamo/blob/RC4.0.0_master/src/Engine/ProtoCore/RuntimeData.cs#L212
There was a problem hiding this comment.
changed the link as per your suggestions
| Each `TraceExampleItem` is serialized into trace represented as a `TraceableId` - this is just a class containing an `IntId` which is marked `[Serializeable]` so it can be serialized with `SOAP` Formatter. see [here for more info on the serializable attribute](https://docs.microsoft.com/en-us/dotnet/api/system.serializableattribute?view=netframework-4.8) | ||
|
|
||
| You must also implement the `ISerializable` interface defined [here](https://docs.microsoft.com/en-us/dotnet/api/system.runtime.serialization.iserializable?view=netframework-4.8) | ||
| Each `TraceExampleItem` is stored in a static TraceableObjectManager. The TraceableObjectManager class maintains a static dictionary of objects keyed by their trace id. |
There was a problem hiding this comment.
The TraceableObjectManager class maintains a static dictionary of objects keyed by their trace id.
I don't think this matters - what matters to communicate is that TraceableObjectManager serializes each TraceExampleItem using JSON to a string and then calls the trace apis.
There was a problem hiding this comment.
Modified the content. Kindly review.
| #### Efficiency | ||
|
|
||
| * Currently each serialize trace object is serialized using SOAP xml formatting - this is quite verbose and duplicates a lot of information. Then the data is base64 encoded twice - This is not efficient in terms of serialization or deserialization. This can be improved in the future if the internal format is not built on top of. Again, we repeat, do not rely on the format of the serialized data at rest. | ||
| * Currently each trace object uses JSON based serialization. This is an imporvement over the earlier SOAP xml formatting method and is more efficient in terms of serialization or deserialization. |
There was a problem hiding this comment.
| * Currently each trace object uses JSON based serialization. This is an imporvement over the earlier SOAP xml formatting method and is more efficient in terms of serialization or deserialization. | |
| * Currently each trace string is serialized using gzip compression and base64 encoding. This is an improvement over the legacy SOAP xml formatting. |
There was a problem hiding this comment.
Modified the content as per your suggestion.
Purpose:
Update the “Element Binding and Tracing” section to JSON based serialization based on the changes in the following PRs:
Changes: