Repository navigation
ottypes/docs spec is not supported correctly wrt (de)serialisation #214
Description
Spec: https://github.com/ottypes/docs#standard-properties
First of all, according to the spec, initialData, snapshot and data may all be completely different data types in the following function specs create([initialData]) -> snapshot, serialize(snapshot) -> data, deserialize(data) -> snapshot. text-tp2 is an example of such a type. Please note also that create([initialData]) -> snapshot returns a non-serialized snapshot.
Secondly, apply(snapshot, op) -> snapshot' expects a non-serialized snapshot as the first parameter. Again, text-tp2 is an example of a type which relies on the first param being a non-serialized snapshot.
I found a number of problems in ShareDB related to the above:
- ShareDB applies an operation to
initialData, instead of asnapshot, in tryCompose. I have already opened a PR with a fix. - ShareDB passes a non-serialized snapshot to
deserializehere. - ShareDB uses createDeserialized, which is not in the spec and is not needed because the standard
createalready returns a non-serialized snapshot. - ShareDB never calls
serializeand relies on the OT types to definetoJSONinstead. This is not documented anywhere and violates the spec. - ShareDB applies operations to serialized snapshots on the server-side because it does not deserialize the data coming from the database.
Reacted by Kieran Mann
Activity
DetweilerRyan commentedon Jun 15, 2018
The contract for custom (de)serialization of ot types in ShareDB differs from the spec you reference. This was done for good reason and it is "documented" here #108.
I'm always glad to hear of new use cases for custom ot types. At Retrium we have been using custom ot types with custom serialization for about three years in production and I assure you the current implementation in sharedb works without fail.
gkubisa commentedon Jun 15, 2018
From reading ShareDB source code, I see the differences affect the following functions:
create,createDeserialized,applyandserialize. The biggest issue for me is that those differences are not documented clearly and in a place where users would actually find out about them.As I understand, the reason is performance, which is a good reason indeed. :-) However, couldn't the same benefits be achieved while maintaining compatibility with ottypes/docs? For example, add a new optional function to the spec:
applySerialized(serializedSnapshot, op) -> serializedSnapshot'and use it only on the server as follows:applySerializedis available, callapplySerialized. (fast for types with custom (de)serialization)serializeanddeserializeare available, calldeserialize->apply->serialize. (MAY be slow for types with custom (de)serialization but compatible with ottypes/docs)apply. (fast for types without custom serialization)I never questioned that it works. I just pointed out that it implements a different spec than the one referenced in the readme file.
IMHO, the written ottypes/docs spec is clearer and simpler than the implied spec implemented by ShareDB, so I'd still argue for maintaining compatibility with ottypes/docs.