Redefined error behavior in blob loading when blob key doesn't exist

src/blobstore/implementations/inmemory/InMemoryBlobStore.cpp

@@ -25,13 +25,13 @@ unique_ptr<BlobWithKey> InMemoryBlobStore::create(const std::string &key, size_t
   return make_unique<BlobWithKey>(key, make_unique<InMemoryBlob>(insert_result.first->second));
 }
 
-bool InMemoryBlobStore::exists(const std::string &key) {
-  return _blobs.count(key) > 0;
-}
-
 unique_ptr<Blob> InMemoryBlobStore::load(const string &key) {
   //Return a copy of the stored InMemoryBlob
-  return make_unique<InMemoryBlob>(_blobs.at(key));
+  try {
+    return make_unique<InMemoryBlob>(_blobs.at(key));
+  } catch (const std::out_of_range &e) {
+    return nullptr;
+  }
 }
 
 } /* namespace ondisk */

src/blobstore/implementations/inmemory/InMemoryBlobStore.h

@@ -18,7 +18,6 @@ public:
   InMemoryBlobStore();
 
   std::unique_ptr<BlobWithKey> create(const std::string &key, size_t size) override;
-  bool exists(const std::string &key) override;
   std::unique_ptr<Blob> load(const std::string &key) override;
 
 private:

src/blobstore/implementations/ondisk/OnDiskBlob.cpp

@@ -2,6 +2,7 @@
 
 #include "OnDiskBlobStore.h"
 #include "blobstore/implementations/ondisk/FileAlreadyExistsException.h"
+#include "blobstore/utils/FileDoesntExistException.h"
 
 #include <cstring>
 #include <fstream>
@@ -45,9 +46,18 @@ size_t OnDiskBlob::size() const {
 }
 
 unique_ptr<OnDiskBlob> OnDiskBlob::LoadFromDisk(const bf::path &filepath) {
-  Data data = Data::LoadFromFile(filepath);
-
-  return unique_ptr<OnDiskBlob>(new OnDiskBlob(filepath, std::move(data)));
+  try {
+    //If it isn't a file, Data::LoadFromFile() would usually also crash. We still need this extra check
+    //upfront, because Data::LoadFromFile() doesn't crash if we give it the path of a directory
+    //instead the path of a file.
+    if(!bf::is_regular_file(filepath)) {
+      return nullptr;
+    }
+    Data data = Data::LoadFromFile(filepath);
+    return unique_ptr<OnDiskBlob>(new OnDiskBlob(filepath, std::move(data)));
+  } catch (const FileDoesntExistException &e) {
+    return nullptr;
+  }
 }
 
 unique_ptr<OnDiskBlob> OnDiskBlob::CreateOnDisk(const bf::path &filepath, size_t size) {

src/blobstore/implementations/ondisk/OnDiskBlobStore.cpp

@@ -27,11 +27,6 @@ unique_ptr<BlobWithKey> OnDiskBlobStore::create(const std::string &key, size_t s
   return make_unique<BlobWithKey>(key, std::move(blob));
 }
 
-bool OnDiskBlobStore::exists(const std::string &key) {
-  auto file_path = _rootdir / key;
-  return bf::exists(file_path);
-}
-
 unique_ptr<Blob> OnDiskBlobStore::load(const string &key) {
   auto file_path = _rootdir / key;
   return OnDiskBlob::LoadFromDisk(file_path);

src/blobstore/implementations/ondisk/OnDiskBlobStore.h

@@ -17,7 +17,6 @@ class OnDiskBlobStore: public BlobStoreWithRandomKeys {
 public:
   OnDiskBlobStore(const boost::filesystem::path &rootdir);
 
-  bool exists(const std::string &key) override;
   std::unique_ptr<BlobWithKey> create(const std::string &key, size_t size) override;
   std::unique_ptr<Blob> load(const std::string &key) override;
 

src/blobstore/interface/BlobStore.h

@@ -19,7 +19,6 @@ public:
   virtual BlobWithKey create(size_t size) = 0;
   // Return nullptr if blob with this key doesn't exists
   virtual std::unique_ptr<Blob> load(const std::string &key) = 0;
-  virtual bool exists(const std::string &key) = 0;
   //TODO Needed for performance? Or is deleting loaded blobs enough?
   //virtual void remove(const std::string &key) = 0;
 };

src/test/blobstore/implementations/ondisk/OnDiskBlobTest/OnDiskBlobLoadTest.cpp

@@ -68,8 +68,7 @@ TEST_P(OnDiskBlobLoadTest, LoadedDataIsCorrect) {
 
 TEST_F(OnDiskBlobLoadTest, LoadNotExistingBlob) {
   TempFile file2(false); // Pass false, so the file isn't created.
-  EXPECT_THROW(
-      OnDiskBlob::LoadFromDisk(file2.path()),
-      FileDoesntExistException
+  EXPECT_FALSE(
+      (bool)OnDiskBlob::LoadFromDisk(file2.path())
   );
 }

src/test/blobstore/testutils/BlobStoreTest.h

@@ -109,23 +109,20 @@ public:
   }
 
   void TestLoadNonExistingBlobWithDefinitelyValidKey() {
-    //TODO Specify loading error behavior more precise and test for concrete exception (or whatever behavior we choose)
-    EXPECT_ANY_THROW(
-        blobStore->load(blobstore::RandomKeyGenerator::singleton().create());
+    EXPECT_FALSE(
+        (bool)blobStore->load(blobstore::RandomKeyGenerator::singleton().create())
     );
   }
 
   void TestLoadNonExistingBlobWithMaybeInvalidKey() {
-    //TODO Specify loading error behavior more precise and test for concrete exception (or whatever behavior we choose)
-    EXPECT_ANY_THROW(
-        blobStore->load("not-existing-key");
+    EXPECT_FALSE(
+        (bool)blobStore->load("not-existing-key")
     );
   }
 
   void TestLoadNonExistingBlobWithEmptyKey() {
-    //TODO Specify loading error behavior more precise and test for concrete exception (or whatever behavior we choose)
-    EXPECT_ANY_THROW(
-        blobStore->load("");
+    EXPECT_FALSE(
+        (bool)blobStore->load("")
     );
   }