sapling/eden/scm/lib/edenapi
Meyer Jacobs 656e3c90d6 edenapi: Introduce serde annotations for wire protocol compatibility and compact wire representation
Summary:
Aux data wire protocol part 1: field annotations & basic compatibility model.

Annotates fields in `file`, `tree`, and `complete_tree` wire structs with `#[serde(rename = "N", default, skip_serializing_if = "is_default")]`. I've avoided using `#[serde(default)]` on the container structs themselves because this can cause some confusion / incorrect behavior if not used carefully. Consider a wire struct `FooRequest` with a field of type `Option<Bar>`. `Option<Bar>` defaults to `None`. If `FooRequest`'s `Default` implenentation sets the field's default to `Some(bar)`, a `FooRequest` explicitly constructed with `None` for the field will be serialized with the field omitted (because it passes `is_default`) and will be deserialized on the server as `Some(bar)`, causing incorrect behavior. To address this, we'd need to change the `is_default` function used with `skip_serializing_if` to check against the field's default value as set by the container, which isn't trivially possible without some sort of reflection (please correct me if you know a good way to achieve this). This is unfortunate, as it'd be very desirable for the container to be able to set defaults different from the individual field type defaults, for cases where one boolean, for instance, should default to true. As-is, we'd need to address this with wrapper types instead, where we can fully control the `Default` implenentation.

We can, of course, address this by providing an alternate `skip_serializing_if` function to fields with default that doesn't match that set by the container. This will need to be done carefully, though, to avoid the issue I described above.

Currently the JSON module manually serializes and deserializes all the top-level request objects, so the rename annotation doesn't impact it. We can add `#[serde(alias = "rustfieldname")]` if we'd like the server and client to be able to accept manually-crafted requests and responses with explicit field names. This could also be useful to replace the manual parsing in the JSON module, but can't replace the manual serialization in a clean way. We'd need to introduce a second copy of the wire types, without the serde `rename` attribute, to allow serializing with the actual rust field name.

I've only modified the `tree`, `file`, and `complete_tree` modules. I intend to eventually update the rest of the edenapi protocol later on, when the implementation of `file` and `tree` are complete / stable. This will give us a chance to fix any mistakes before copying the design to more places.

Note: I do not intend to keep to proper wire protocol compatibility at this stage in the implementation. Expect field numbers to be re-used by non-compatible changes.

Reviewed By: kulshrax

Differential Revision: D23172756

fbshipit-source-id: 39976ed4bede892bd6981f9c3f23557a91f9028b
2020-08-18 13:44:35 -07:00
..
src edenapi: Split DataEntry into FileEntry and TreeEntry 2020-08-13 10:01:40 -07:00
tools edenapi: Split DataEntry into FileEntry and TreeEntry 2020-08-13 10:01:40 -07:00
types edenapi: Introduce serde annotations for wire protocol compatibility and compact wire representation 2020-08-18 13:44:35 -07:00
Cargo.toml edenapi: percent-encode repo names 2020-07-16 13:32:19 -07:00