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

eden/scm/lib/edenapi/types/src/complete_tree.rs

@@ -9,6 +9,8 @@ use serde_derive::{Deserialize, Serialize};
 
 use types::{hgid::HgId, path::RepoPathBuf};
 
+use crate::is_default;
+
 /// Struct reprenting the arguments to a "gettreepack" operation, which
 /// is used by Mercurial to prefetch treemanifests. This struct is intended
 /// to provide a way to support requests compatible with Mercurial's existing
@@ -18,12 +20,19 @@ use types::{hgid::HgId, path::RepoPathBuf};
 /// In general, trees can be requested from the API server using a `TreeRequest`
 /// containing the keys of the desired tree nodes.
 ///
-/// In all cases, trees will be returned in a `DataResponse`.
+/// In all cases, trees will be returned in a `TreeResponse`.
 #[derive(Clone, Debug, Serialize, Deserialize, Eq, PartialEq)]
 pub struct CompleteTreeRequest {
+    #[serde(rename = "0", default, skip_serializing_if = "is_default")]
     pub rootdir: RepoPathBuf,
+
+    #[serde(rename = "1", default, skip_serializing_if = "is_default")]
     pub mfnodes: Vec<HgId>,
+
+    #[serde(rename = "2", default, skip_serializing_if = "is_default")]
     pub basemfnodes: Vec<HgId>,
+
+    #[serde(rename = "3", default, skip_serializing_if = "is_default")]
     pub depth: Option<usize>,
 }
 

eden/scm/lib/edenapi/types/src/file.rs

@@ -14,7 +14,7 @@ use thiserror::Error;
 use revisionstore_types::Metadata;
 use types::{hgid::HgId, key::Key, parents::Parents};
 
-use crate::InvalidHgId;
+use crate::{is_default, InvalidHgId};
 
 /// Tombstone string that replaces the content of redacted files.
 /// TODO(T48685378): Handle redacted content in a less hacky way.
@@ -51,11 +51,17 @@ impl FileError {
 /// Includes the information required to add the data to a mutable store,
 /// along with the parents for hash validation.
 #[derive(Clone, Debug, Default, Eq, PartialEq, Serialize, Deserialize)]
-#[serde(default)]
 pub struct FileEntry {
+    #[serde(rename = "0", default, skip_serializing_if = "is_default")]
     key: Key,
+
+    #[serde(rename = "1", default, skip_serializing_if = "is_default")]
     data: Bytes,
+
+    #[serde(rename = "2", default, skip_serializing_if = "is_default")]
     parents: Parents,
+
+    #[serde(rename = "3", default, skip_serializing_if = "is_default")]
     metadata: Metadata,
 }
 
@@ -116,13 +122,15 @@ impl FileEntry {
     }
 }
 
-#[derive(Clone, Debug, Serialize, Deserialize, Eq, PartialEq)]
+#[derive(Clone, Default, Debug, Serialize, Deserialize, Eq, PartialEq)]
 pub struct FileRequest {
+    #[serde(rename = "0", default, skip_serializing_if = "is_default")]
     pub keys: Vec<Key>,
 }
 
-#[derive(Debug, Serialize, Deserialize)]
+#[derive(Debug, Default, Serialize, Deserialize)]
 pub struct FileResponse {
+    #[serde(rename = "0", default, skip_serializing_if = "is_default")]
     pub entries: Vec<FileEntry>,
 }
 

eden/scm/lib/edenapi/types/src/lib.rs

@@ -55,3 +55,7 @@ pub struct InvalidHgId {
     data: Bytes,
     parents: Parents,
 }
+
+fn is_default<T: Default + PartialEq>(v: &T) -> bool {
+    v == &T::default()
+}

eden/scm/lib/edenapi/types/src/tree.rs

@@ -14,7 +14,7 @@ use thiserror::Error;
 use revisionstore_types::Metadata;
 use types::{hgid::HgId, key::Key, parents::Parents};
 
-use crate::InvalidHgId;
+use crate::{is_default, InvalidHgId};
 
 #[derive(Debug, Error)]
 pub enum TreeError {
@@ -45,11 +45,17 @@ impl TreeError {
 /// Includes the information required to add the data to a mutable store,
 /// along with the parents for hash validation.
 #[derive(Clone, Debug, Default, Eq, PartialEq, Serialize, Deserialize)]
-#[serde(default)]
 pub struct TreeEntry {
+    #[serde(rename = "0", default, skip_serializing_if = "is_default")]
     key: Key,
+
+    #[serde(rename = "1", default, skip_serializing_if = "is_default")]
     data: Bytes,
+
+    #[serde(rename = "2", default, skip_serializing_if = "is_default")]
     parents: Parents,
+
+    #[serde(rename = "3", default, skip_serializing_if = "is_default")]
     metadata: Metadata,
 }
 
@@ -109,13 +115,15 @@ impl TreeEntry {
     }
 }
 
-#[derive(Clone, Debug, Serialize, Deserialize, Eq, PartialEq)]
+#[derive(Clone, Debug, Default, Serialize, Deserialize, Eq, PartialEq)]
 pub struct TreeRequest {
+    #[serde(rename = "0", default, skip_serializing_if = "is_default")]
     pub keys: Vec<Key>,
 }
 
-#[derive(Debug, Serialize, Deserialize)]
+#[derive(Debug, Default, Serialize, Deserialize)]
 pub struct TreeResponse {
+    #[serde(rename = "0", default, skip_serializing_if = "is_default")]
     pub entries: Vec<TreeEntry>,
 }