diff --git a/eden/fs/store/hg/HgProxyHash.cpp b/eden/fs/store/hg/HgProxyHash.cpp index 8a8bb453d2..b8b12a3008 100644 --- a/eden/fs/store/hg/HgProxyHash.cpp +++ b/eden/fs/store/hg/HgProxyHash.cpp @@ -38,7 +38,7 @@ HgProxyHash::HgProxyHash( } value_ = infoResult.extractValue(); - parseValue(edenBlobHash); + validate(edenBlobHash); } folly::Future>> 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 diff --git a/eden/fs/store/hg/HgProxyHash.h b/eden/fs/store/hg/HgProxyHash.h index c986859988..0b443527b9 100644 --- a/eden/fs/store/hg/HgProxyHash.h +++ b/eden/fs/store/hg/HgProxyHash.h @@ -7,6 +7,7 @@ #pragma once +#include #include #include #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().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>> getBatch( LocalStore* store, const std::vector& 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 diff --git a/eden/fs/store/hg/test/HgProxyHashTest.cpp b/eden/fs/store/hg/test/HgProxyHashTest.cpp new file mode 100644 index 0000000000..50043a2ed6 --- /dev/null +++ b/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 +#include +#include +#include + +#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(); + 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"}}); +}