From cdd92e9951bc13d13b5f4d0bce97a959b145d659 Mon Sep 17 00:00:00 2001
From: Arun Kulshreshtha <kulshrax@fb.com>
Date: Mon, 20 May 2019 11:49:20 -0700
Subject: [PATCH] types: make DataEntry fields private

Summary: Change the API so that the act of getting the data from a data entry (optionally) forces hash validation.

Reviewed By: xavierd

Differential Revision: D15405742

fbshipit-source-id: c127286322c707eb066f89021ac45315e89aa718
---
 lib/edenapi/src/curl.rs    |  5 ++---
 lib/types/src/dataentry.rs | 28 +++++++++++++++++++++++-----
 lib/types/src/testutil.rs  |  6 +-----
 3 files changed, 26 insertions(+), 13 deletions(-)

diff --git a/lib/edenapi/src/curl.rs b/lib/edenapi/src/curl.rs
index 5c3658f15e..9bbcdbb378 100644
--- a/lib/edenapi/src/curl.rs
+++ b/lib/edenapi/src/curl.rs
@@ -163,10 +163,9 @@ impl EdenApi for EdenApiCurlClient {
         let mut files = Vec::new();
         for entry in responses.into_iter().flatten() {
             if self.validate_files {
-                log::trace!("Validating file: {}", &entry.key);
-                entry.validate()?;
+                log::trace!("Validating file: {}", entry.key());
             }
-            files.push((entry.key, entry.data));
+            files.push((entry.key().clone(), entry.data(self.validate_files)?));
         }
 
         write_to_deltastore(store, files)
diff --git a/lib/types/src/dataentry.rs b/lib/types/src/dataentry.rs
index 8aecedfb8c..0a59790b45 100644
--- a/lib/types/src/dataentry.rs
+++ b/lib/types/src/dataentry.rs
@@ -25,15 +25,33 @@ use crate::{key::Key, node::Node, parents::Parents};
     Deserialize
 )]
 pub struct DataEntry {
-    pub key: Key,
-    pub data: Bytes,
-    pub parents: Parents,
+    key: Key,
+    data: Bytes,
+    parents: Parents,
 }
 
 impl DataEntry {
-    /// Compute the filenode hash of this `DataEntry` using the parents and
+    pub fn new(key: Key, data: Bytes, parents: Parents) -> Self {
+        Self { key, data, parents }
+    }
+
+    pub fn key(&self) -> &Key {
+        &self.key
+    }
+
+    /// Get this entry's data content. If validate is set to true, this method
+    /// will recompute the entry's node hash and verify that it matches the
+    /// expected node hash in the entry's key, returning an error otherwise.
+    pub fn data(&self, validate: bool) -> Fallible<Bytes> {
+        if validate {
+            self.validate()?;
+        }
+        Ok(self.data.clone())
+    }
+
+    /// Compute the filenode hash of this `DataEntry` using its parents and
     /// content, and compare it with the known node hash from the entry's `Key`.
-    pub fn validate(&self) -> Fallible<()> {
+    fn validate(&self) -> Fallible<()> {
         // Mercurial hashes the parent nodes in sorted order
         // when computing the node hash.
         let (p1, p2) = match self.parents.clone().into_nodes() {
diff --git a/lib/types/src/testutil.rs b/lib/types/src/testutil.rs
index 8165425e94..14aa3c6cb6 100644
--- a/lib/types/src/testutil.rs
+++ b/lib/types/src/testutil.rs
@@ -62,9 +62,5 @@ pub fn null_key(path: &str) -> Key {
 }
 
 pub fn data_entry(key: Key, data: impl AsRef<[u8]>) -> DataEntry {
-    DataEntry {
-        key,
-        data: data.as_ref().into(),
-        parents: Parents::None,
-    }
+    DataEntry::new(key, data.as_ref().into(), Parents::None)
 }