remove the deprecated ObjectStore::getBlob() API

Summary:
Remove the blocking getBlob() API.

There were a few call sites in FileData still using this blocking API.  For now
I simply updated them to use getBlobFuture() and make a blocking get() call on
the returned future.  These call sites already had TODO comments documenting
the blocking behavior.

I plan to rename getBlobFuture() to getBlob() in a subsequent diff.

Reviewed By: wez

Differential Revision: D5295726

fbshipit-source-id: 748fd7a140b9b59da339a330071f732bba38cb35

eden/fs/inodes/FileData.cpp

@@ -311,17 +311,18 @@ Future<Unit> FileData::ensureDataLoaded() {
   }
 
   // Load the blob data.
-  //
-  // TODO: We really should use a Future-based API for this rather than
-  // blocking until the load completes.  However, for that to work we will need
-  // to add some extra data tracking whether or not we are already in the
-  // process of loading the data.  We need to avoid multiple threads all trying
-  // to load the data at the same time.
+  auto blobFuture = getObjectStore()->getBlobFuture(state->hash.value());
+
+  // TODO: We really should defer this using a Future rather than calling get()
+  // here and blocking until the load completes.  However, for that to work we
+  // will need to add some extra data tracking whether or not we are already in
+  // the process of loading the data.  We need to avoid multiple threads all
+  // trying to load the data at the same time.
   //
   // For now doing a blocking load with the inode_->state_ lock held ensures
   // that only one thread can load the data at a time.  It's pretty unfortunate
   // to block with the lock held, though :-(
-  blob_ = getObjectStore()->getBlob(state->hash.value());
+  blob_ = blobFuture.get();
   return makeFuture();
 }
 
@@ -355,7 +356,8 @@ Future<Unit> FileData::materializeForWrite(int openFlags) {
       // TODO: Load the blob using the non-blocking Future APIs.
       // However, just as in ensureDataLoaded() above we will also need
       // to add a mechanism to wait for already in-progress loads.
-      blob_ = getObjectStore()->getBlob(state->hash.value());
+      auto blobFuture = getObjectStore()->getBlobFuture(state->hash.value());
+      blob_ = blobFuture.get();
     }
 
     // Write the blob contents out to the overlay

eden/fs/store/IObjectStore.h

@@ -28,8 +28,6 @@ class IObjectStore {
  public:
   virtual ~IObjectStore() {}
 
-  virtual std::unique_ptr<Blob> getBlob(const Hash& id) const = 0;
-
   /**
    * Return the SHA1 hash of the blob contents.
    *

eden/fs/store/ObjectStore.cpp

@@ -74,10 +74,6 @@ Future<unique_ptr<Tree>> ObjectStore::getTreeFuture(const Hash& id) const {
   });
 }
 
-unique_ptr<Blob> ObjectStore::getBlob(const Hash& id) const {
-  return getBlobFuture(id).get();
-}
-
 Future<unique_ptr<Blob>> ObjectStore::getBlobFuture(const Hash& id) const {
   auto blob = localStore_->getBlob(id);
   if (blob) {

eden/fs/store/ObjectStore.h

@@ -38,16 +38,6 @@ class ObjectStore : public IObjectStore {
       std::shared_ptr<BackingStore> backingStore);
   ~ObjectStore() override;
 
-  /**
-   * Get a Blob by ID.
-   *
-   * This function never returns nullptr.  It throws std::domain_error if the
-   * specified blob ID does not exist, or possibly other exceptions on error.
-   *
-   * TODO: This API will be deprecated in favor of getBlobFuture()
-   */
-  std::unique_ptr<Blob> getBlob(const Hash& id) const override;
-
   /**
    * Get a Tree by commit ID.
    *

eden/fs/testharness/FakeObjectStore.cpp

@@ -63,10 +63,6 @@ Future<std::unique_ptr<Tree>> FakeObjectStore::getTreeFuture(
   return makeFuture(make_unique<Tree>(iter->second));
 }
 
-unique_ptr<Blob> FakeObjectStore::getBlob(const Hash& id) const {
-  return getBlobFuture(id).get();
-}
-
 Future<std::unique_ptr<Blob>> FakeObjectStore::getBlobFuture(
     const Hash& id) const {
   auto iter = blobs_.find(id);

eden/fs/testharness/FakeObjectStore.h

@@ -33,7 +33,6 @@ class FakeObjectStore : public IObjectStore {
   void addBlob(Blob&& blob);
   void setTreeForCommit(const Hash& commitID, Tree&& tree);
 
-  std::unique_ptr<Blob> getBlob(const Hash& id) const override;
   Hash getSha1ForBlob(const Hash& id) const override;
 
   folly::Future<std::unique_ptr<Tree>> getTreeFuture(

eden/fs/testharness/test/FakeObjectStoreTest.cpp

@@ -42,11 +42,11 @@ TEST(FakeObjectStore, getObjectsOfAllTypesFromStore) {
   EXPECT_TRUE(foundTree);
   EXPECT_EQ(tree1Hash, foundTree->getHash());
 
-  // Test getBlob().
+  // Test getBlobFuture().
   auto buf1 = IOBuf();
   Blob blob1(blobHash, buf1);
   store.addBlob(std::move(blob1));
-  auto foundBlob = store.getBlob(blobHash);
+  auto foundBlob = store.getBlobFuture(blobHash).get();
   EXPECT_TRUE(foundBlob);
   EXPECT_EQ(blobHash, foundBlob->getHash());
 
@@ -76,7 +76,7 @@ TEST(FakeObjectStore, getMissingObjectThrows) {
   FakeObjectStore store;
   Hash hash("4242424242424242424242424242424242424242");
   EXPECT_THROW(store.getTreeFuture(hash).get(), std::domain_error);
-  EXPECT_THROW(store.getBlob(hash), std::domain_error);
+  EXPECT_THROW(store.getBlobFuture(hash).get(), std::domain_error);
   EXPECT_THROW(store.getTreeForCommit(hash).get(), std::domain_error);
   EXPECT_THROW(store.getSha1ForBlob(hash), std::domain_error);
 }