edenapi: use DataEntry

Summary: Update the Eden API client and server to use the new DataEntry type for file content downloads.

Reviewed By: quark-zju

Differential Revision: D14907958

fbshipit-source-id: 8a7b1cbb54bdc119dda11179ff94d3efdb7e85c9

lib/edenapi/src/curl.rs

@@ -39,6 +39,7 @@ pub struct EdenApiCurlClient {
     creds: Option<ClientCreds>,
     data_batch_size: Option<usize>,
     history_batch_size: Option<usize>,
+    validate: bool,
 }
 
 // Public API.
@@ -71,6 +72,7 @@ impl EdenApiCurlClient {
             creds: config.creds,
             data_batch_size: config.data_batch_size,
             history_batch_size: config.history_batch_size,
+            validate: true,
         };
 
         // Create repo/packs directory in cache if it doesn't already exist.
@@ -123,13 +125,22 @@ impl EdenApi for EdenApiCurlClient {
             responses.len(),
             responses
                 .iter()
-                .map(|entry| entry.files.len())
+                .map(|response| response.entries.len())
                 .sum::<usize>(),
         );
 
+        let mut files = Vec::new();
+        for entry in responses.into_iter().flatten() {
+            if self.validate {
+                log::trace!("Validating file: {}", &entry.key);
+                entry.validate()?;
+            }
+            files.push((entry.key, entry.data));
+        }
+
         let cache_path = self.pack_cache_path();
         log::debug!("Writing pack file in directory: {:?}", &cache_path);
-        write_datapack(cache_path, responses.into_iter().flatten())
+        write_datapack(cache_path, files)
     }
 
     fn get_history(&self, keys: Vec<Key>, max_depth: Option<u32>) -> Fallible<PathBuf> {

lib/types/src/api.rs

@@ -2,10 +2,10 @@
 
 //! Types for data interchange between the Mononoke API Server and the Mercurial client.
 
-use bytes::Bytes;
 use serde_derive::{Deserialize, Serialize};
 
 use crate::{
+    dataentry::DataEntry,
     historyentry::{HistoryEntry, WireHistoryEntry},
     key::Key,
     path::RepoPathBuf,
@@ -18,23 +18,23 @@ pub struct FileDataRequest {
 
 #[derive(Debug, Serialize, Deserialize)]
 pub struct FileDataResponse {
-    pub files: Vec<(Key, Bytes)>,
+    pub entries: Vec<DataEntry>,
 }
 
 impl FileDataResponse {
-    pub fn new(data: impl IntoIterator<Item = (Key, Bytes)>) -> Self {
+    pub fn new(data: impl IntoIterator<Item = DataEntry>) -> Self {
         Self {
-            files: data.into_iter().collect(),
+            entries: data.into_iter().collect(),
         }
     }
 }
 
 impl IntoIterator for FileDataResponse {
-    type Item = (Key, Bytes);
-    type IntoIter = std::vec::IntoIter<(Key, Bytes)>;
+    type Item = DataEntry;
+    type IntoIter = std::vec::IntoIter<DataEntry>;
 
     fn into_iter(self) -> Self::IntoIter {
-        self.files.into_iter()
+        self.entries.into_iter()
     }
 }
 
@@ -85,9 +85,9 @@ mod tests {
     #[test]
     fn data_iter() {
         let data = vec![
-            (FOO_KEY.clone(), Bytes::from(&b"foo data"[..])),
-            (BAR_KEY.clone(), Bytes::from(&b"bar data"[..])),
-            (BAZ_KEY.clone(), Bytes::from(&b"baz data"[..])),
+            data_entry(FOO_KEY.clone(), b"foo data"),
+            data_entry(BAR_KEY.clone(), b"bar data"),
+            data_entry(BAZ_KEY.clone(), b"baz data"),
         ];
         let response = FileDataResponse::new(data.clone());
 

lib/types/src/dataentry.rs

@@ -34,10 +34,15 @@ impl DataEntry {
     /// file content, and compare it with the known filenode hash from
     /// the entry's `Key`.
     pub fn validate(&self) -> Fallible<()> {
-        let (p1, p2) = self.parents.clone().into_nodes();
+        // Mercurial hashes the parent filesnodes in sorted order
+        // when computing the filenode hash.
+        let (p1, p2) = match self.parents.clone().into_nodes() {
+            (p1, p2) if p1 > p2 => (p2, p1),
+            (p1, p2) => (p1, p2),
+        };
+
         let mut hash = [0u8; 20];
         let mut hasher = Sha1::new();
-
         hasher.input(p1.as_ref());
         hasher.input(p2.as_ref());
         hasher.input(&self.data);

lib/types/src/parents.rs

@@ -14,17 +14,7 @@ use crate::node::{Node, NULL_ID};
 ///
 /// In Rust, these restrictions can be enforced with an enum that makes invalid
 /// states unrepresentable.
-#[derive(
-    Clone,
-    Debug,
-    Eq,
-    Hash,
-    Ord,
-    PartialEq,
-    PartialOrd,
-    Serialize,
-    Deserialize
-)]
+#[derive(Clone, Debug, Eq, Hash, Ord, PartialEq, PartialOrd, Serialize, Deserialize)]
 #[serde(untagged)]
 pub enum Parents {
     None,

lib/types/src/path.rs

@@ -41,8 +41,18 @@ use failure::{bail, Fallible};
 use serde_derive::{Deserialize, Serialize};
 
 /// An owned version of a `RepoPath`.
-#[derive(Clone, Debug, Default, Ord, PartialOrd, Eq, PartialEq, Hash)]
-#[derive(Serialize, Deserialize)]
+#[derive(
+    Clone,
+    Debug,
+    Default,
+    Ord,
+    PartialOrd,
+    Eq,
+    PartialEq,
+    Hash,
+    Serialize,
+    Deserialize
+)]
 pub struct RepoPathBuf(String);
 
 /// A normalized path starting from the root of the repository. Paths can be broken into
@@ -61,8 +71,7 @@ pub struct RepoPathBuf(String);
 /// characters and reseved words.
 ///
 /// It should be noted that `RepoPathBuf` and `RepoPath` implement `AsRef<RepoPath>`.
-#[derive(Debug, Ord, PartialOrd, Eq, PartialEq, Hash)]
-#[derive(Serialize)]
+#[derive(Debug, Ord, PartialOrd, Eq, PartialEq, Hash, Serialize)]
 pub struct RepoPath(str);
 
 /// An owned version of a `PathComponent`. Not intended for mutation. RepoPathBuf is probably

lib/types/src/testutil.rs

@@ -4,8 +4,10 @@
 // GNU General Public License version 2 or any later version.
 
 use crate::{
+    dataentry::DataEntry,
     key::Key,
     node::Node,
+    parents::Parents,
     path::{PathComponent, PathComponentBuf, RepoPath, RepoPathBuf},
 };
 
@@ -58,3 +60,11 @@ pub fn key(path: &str, hexnode: &str) -> Key {
 pub fn null_key(path: &str) -> Key {
     Key::new(repo_path_buf(path), Node::null_id().clone())
 }
+
+pub fn data_entry(key: Key, data: impl AsRef<[u8]>) -> DataEntry {
+    DataEntry {
+        key,
+        data: data.as_ref().into(),
+        parents: Parents::None,
+    }
+}