make HgProxyHash copyable & moveable

Summary: This diff makes `HgProxyHash` moveable & copyable so we can return it from `getBatch` method instead returning a pair of `RelativePath` & `Hash`. See next diff for usage.

Reviewed By: chadaustin

Differential Revision: D20841434

fbshipit-source-id: c082c217f75d53bcbfb4853e2055d1458a040188

eden/fs/store/hg/HgProxyHash.cpp

@@ -38,7 +38,7 @@ HgProxyHash::HgProxyHash(
   }
 
   value_ = infoResult.extractValue();
-  parseValue(edenBlobHash);
+  validate(edenBlobHash);
 }
 
 folly::Future<std::vector<std::pair<RelativePath, Hash>>> HgProxyHash::getBatch(
@@ -108,7 +108,7 @@ HgProxyHash::HgProxyHash(
   }
 
   value_ = infoResult.extractValue();
-  parseValue(edenBlobHash);
+  validate(edenBlobHash);
 }
 
 IOBuf HgProxyHash::serialize(RelativePathPiece path, Hash hgRevHash) {
@@ -125,7 +125,19 @@ IOBuf HgProxyHash::serialize(RelativePathPiece path, Hash hgRevHash) {
   return buf;
 }
 
-void HgProxyHash::parseValue(Hash edenBlobHash) {
+RelativePathPiece HgProxyHash::path() const {
+  DCHECK_GE(value_.size(), Hash::RAW_SIZE + sizeof(uint32_t));
+  StringPiece data{value_.data(), value_.size()};
+  data.advance(Hash::RAW_SIZE + sizeof(uint32_t));
+  return RelativePathPiece{data};
+}
+
+Hash HgProxyHash::revHash() const {
+  DCHECK_GE(value_.size(), Hash::RAW_SIZE);
+  return Hash{ByteRange{StringPiece{value_.data(), Hash::RAW_SIZE}}};
+}
+
+void HgProxyHash::validate(Hash edenBlobHash) {
   ByteRange infoBytes = StringPiece(value_);
   // Make sure the data is long enough to contain the rev hash and path length
   if (infoBytes.size() < Hash::RAW_SIZE + sizeof(uint32_t)) {
@@ -139,8 +151,6 @@ void HgProxyHash::parseValue(Hash edenBlobHash) {
     throw std::length_error(msg);
   }
 
-  // Extract the revHash_
-  revHash_ = Hash(infoBytes.subpiece(0, Hash::RAW_SIZE));
   infoBytes.advance(Hash::RAW_SIZE);
 
   // Extract the path length
@@ -157,10 +167,6 @@ void HgProxyHash::parseValue(Hash edenBlobHash) {
     XLOG(ERR) << msg;
     throw std::length_error(msg);
   }
-
-  // Extract the path_
-  path_ = RelativePathPiece(StringPiece(infoBytes));
 }
-
 } // namespace eden
 } // namespace facebook

eden/fs/store/hg/HgProxyHash.h

@@ -7,6 +7,7 @@
 
 #pragma once
 
+#include <folly/FixedString.h>
 #include <string>
 #include <vector>
 #include "eden/fs/model/Hash.h"
@@ -21,6 +22,13 @@ class IOBuf;
 
 namespace facebook {
 namespace eden {
+namespace {
+// An empty ProxyHash. It contains an all zeros hash and zero length path.
+constexpr auto kDefaultProxyHash =
+    folly::FixedString<Hash::RAW_SIZE + sizeof(uint32_t)>().append(
+        Hash::RAW_SIZE + sizeof(uint32_t),
+        '\x00');
+} // namespace
 
 /**
  * HgProxyHash manages mercurial (path, revHash) data in the LocalStore.
@@ -30,10 +38,10 @@ namespace eden {
  * path.  To use the data in eden, we need to create a blob hash that we can
  * use instead.
  *
- * To do so, we hash the (path, revHash) tuple, and use this hash as the blob
- * hash in eden.  We store the eden_blob_hash --> (path, hgRevHash) mapping
- * in the LocalStore.  The HgProxyHash class helps store and retrieve these
- * mappings.
+ * To do so, we hash the (path, revHash) tuple, and use this hash as the
+ * blob hash in eden.  We store the eden_blob_hash --> (path, hgRevHash)
+ * mapping in the LocalStore.  The HgProxyHash class helps store and
+ * retrieve these mappings.
  */
 class HgProxyHash {
  public:
@@ -42,16 +50,25 @@ class HgProxyHash {
    */
   HgProxyHash(LocalStore* store, Hash edenBlobHash, folly::StringPiece context);
 
-  ~HgProxyHash() {}
+  ~HgProxyHash() = default;
 
-  const RelativePathPiece& path() const {
-    return path_;
+  HgProxyHash(const HgProxyHash& other) = default;
+  HgProxyHash& operator=(const HgProxyHash& other) = default;
+
+  HgProxyHash(HgProxyHash&& other) noexcept(false) {
+    value_.swap(other.value_);
   }
 
-  const Hash& revHash() const {
-    return revHash_;
+  HgProxyHash& operator=(HgProxyHash&& other) noexcept(false) {
+    value_.swap(other.value_);
+    other.value_ = kDefaultProxyHash;
+    return *this;
   }
 
+  RelativePathPiece path() const;
+
+  Hash revHash() const;
+
   static folly::Future<std::vector<std::pair<RelativePath, Hash>>> getBatch(
       LocalStore* store,
       const std::vector<Hash>& blobHashes);
@@ -89,15 +106,6 @@ class HgProxyHash {
       LocalStore::WriteBatch* writeBatch);
 
  private:
-  // Not movable or copyable.
-  // path_ points into value_, and would need to be updated after
-  // copying/moving the data.  Since no-one needs to copy or move HgProxyHash
-  // objects, we don't implement this for now.
-  HgProxyHash(const HgProxyHash&) = delete;
-  HgProxyHash& operator=(const HgProxyHash&) = delete;
-  HgProxyHash(HgProxyHash&&) = delete;
-  HgProxyHash& operator=(HgProxyHash&&) = delete;
-
   HgProxyHash(
       Hash edenBlobHash,
       StoreResult& infoResult,
@@ -110,30 +118,19 @@ class HgProxyHash {
   static folly::IOBuf serialize(RelativePathPiece path, Hash hgRevHash);
 
   /**
-   * Parse the serialized data found in value_, and set revHash_ and path_.
+   * Validate data found in value_.
    *
    * The value_ member variable should already contain the serialized data,
    * (as returned by serialize()).
    *
-   * Note that path_ will be set to a RelativePathPiece pointing into the
-   * string data owned by value_.  (This lets us avoid copying the string data
-   * out.)
+   * Note there will be an exception being thrown if `value_` is invalid.
    */
-  void parseValue(Hash edenBlobHash);
+  void validate(Hash edenBlobHash);
 
   /**
    * The serialized data as written in the LocalStore.
    */
-  std::string value_;
-  /**
-   * The revision hash.
-   */
-  Hash revHash_;
-  /**
-   * The path name.  Note that this points into the serialized value_ data.
-   * path_ itself does not own the data it points to.
-   */
-  RelativePathPiece path_;
+  std::string value_ = kDefaultProxyHash;
 };
 
 } // namespace eden

eden/fs/store/hg/test/HgProxyHashTest.cpp

@@ -0,0 +1,65 @@
+/*
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ *
+ * This software may be used and distributed according to the terms of the
+ * GNU General Public License version 2.
+ */
+
+#include <folly/Range.h>
+#include <folly/logging/xlog.h>
+#include <gtest/gtest.h>
+#include <memory>
+
+#include "eden/fs/model/Hash.h"
+#include "eden/fs/store/MemoryLocalStore.h"
+#include "eden/fs/store/hg/HgProxyHash.h"
+#include "eden/fs/utils/IDGen.h"
+#include "eden/fs/utils/PathFuncs.h"
+
+using namespace facebook::eden;
+
+TEST(HgProxyHashTest, testCopyMove) {
+  auto store = std::make_shared<MemoryLocalStore>();
+  Hash hash1, hash2;
+  {
+    auto write = store->beginWrite();
+    hash1 = HgProxyHash::store(
+        RelativePathPiece{"foobar"},
+        Hash{folly::StringPiece{"1111111111111111111111111111111111111111"}},
+        write.get());
+
+    hash2 = HgProxyHash::store(
+        RelativePathPiece{"barfoo"},
+        Hash{folly::StringPiece{"DDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDD"}},
+        write.get());
+
+    write->flush();
+  }
+  auto orig1 = HgProxyHash{store.get(), hash1, "test"};
+  auto orig2 = HgProxyHash{store.get(), hash2, "test"};
+  auto second = orig1;
+
+  EXPECT_EQ(orig1.path(), second.path());
+  EXPECT_EQ(orig1.revHash(), second.revHash());
+
+  second = orig2;
+  EXPECT_EQ(orig2.path(), second.path());
+  EXPECT_EQ(orig2.revHash(), second.revHash());
+
+  auto moved = std::move(second);
+
+  EXPECT_EQ(moved.path(), orig2.path());
+  EXPECT_EQ(moved.revHash(), orig2.revHash());
+
+  moved = std::move(orig1);
+
+  EXPECT_EQ(moved.path(), RelativePathPiece{"foobar"});
+  EXPECT_EQ(
+      moved.revHash(),
+      Hash{folly::StringPiece{"1111111111111111111111111111111111111111"}});
+
+  EXPECT_EQ(orig1.path(), RelativePathPiece{""});
+  EXPECT_EQ(
+      orig1.revHash(),
+      Hash{folly::StringPiece{"0000000000000000000000000000000000000000"}});
+}