Write some test cases for OnDiskBlob and also change implementation to adapt them

src/blobstore/implementations/ondisk/CMakeLists.txt

@@ -1,3 +1,3 @@
-add_library(blobstore_ondisk Data.cpp OnDiskBlob.cpp OnDiskBlobStore.cpp)
+add_library(blobstore_ondisk Data.cpp OnDiskBlob.cpp OnDiskBlobStore.cpp FileAlreadyExistsException.cpp)
 
 target_link_libraries(blobstore_ondisk boost_filesystem boost_system)

src/blobstore/implementations/ondisk/FileAlreadyExistsException.cpp

@@ -0,0 +1,19 @@
+#include "FileAlreadyExistsException.h"
+
+namespace bf = boost::filesystem;
+
+using std::runtime_error;
+using std::string;
+
+namespace blobstore {
+namespace ondisk {
+
+FileAlreadyExistsException::FileAlreadyExistsException(const bf::path &filepath)
+: runtime_error(string("The file ")+filepath.c_str()+" already exists") {
+}
+
+FileAlreadyExistsException::~FileAlreadyExistsException() {
+}
+
+} /* namespace ondisk */
+} /* namespace blobstore */

src/blobstore/implementations/ondisk/FileAlreadyExistsException.h

@@ -0,0 +1,21 @@
+#pragma once
+#ifndef BLOBSTORE_IMPLEMENTATIONS_ONDISK_FILEALREADYEXISTSEXCEPTION_H_
+#define BLOBSTORE_IMPLEMENTATIONS_ONDISK_FILEALREADYEXISTSEXCEPTION_H_
+
+#include <boost/filesystem/path.hpp>
+
+#include <stdexcept>
+
+namespace blobstore {
+namespace ondisk {
+
+class FileAlreadyExistsException: public std::runtime_error {
+public:
+  FileAlreadyExistsException(const boost::filesystem::path &filepath);
+  virtual ~FileAlreadyExistsException();
+};
+
+} /* namespace ondisk */
+} /* namespace blobstore */
+
+#endif

src/blobstore/implementations/ondisk/OnDiskBlob.cpp

@@ -1,17 +1,27 @@
 #include "OnDiskBlob.h"
 
 #include "OnDiskBlobStore.h"
+#include "blobstore/implementations/ondisk/FileAlreadyExistsException.h"
 
 #include <cstring>
+#include <fstream>
+#include <boost/filesystem.hpp>
 
+using std::unique_ptr;
+using std::make_unique;
 using std::istream;
 using std::ostream;
+using std::ifstream;
+using std::ofstream;
+using std::ios;
+
+namespace bf = boost::filesystem;
 
 namespace blobstore {
 namespace ondisk {
 
-OnDiskBlob::OnDiskBlob(size_t size)
+OnDiskBlob::OnDiskBlob(const bf::path &filepath, size_t size)
- : _size(size), _data(size) {
+ : _filepath(filepath), _size(size), _data(size) {
 }
 
 OnDiskBlob::~OnDiskBlob() {
@@ -29,17 +39,58 @@ size_t OnDiskBlob::size() const {
   return _size;
 }
 
-void OnDiskBlob::LoadDataFromStream(istream &stream) {
+unique_ptr<OnDiskBlob> OnDiskBlob::LoadFromDisk(const bf::path &filepath) {
+  ifstream file(filepath.c_str(), ios::binary);
+  size_t size = _getStreamSize(file);
+
+  auto blob = make_unique<OnDiskBlob>(filepath, size);
+  blob->_loadDataFromStream(file);
+  return blob;
+}
+
+size_t OnDiskBlob::_getStreamSize(istream &stream) {
+  auto current_pos = stream.tellg();
+
+  //Retrieve length
+  stream.seekg(0, stream.end);
+  auto endpos = stream.tellg();
+
+  //Restore old position
+  stream.seekg(current_pos, stream.beg);
+
+  return endpos - current_pos;
+}
+
+void OnDiskBlob::_loadDataFromStream(istream &stream) {
   stream.read((char*)_data.data(), _size);
 }
 
-void OnDiskBlob::StoreDataToStream(ostream &stream) const {
+unique_ptr<OnDiskBlob> OnDiskBlob::CreateOnDisk(const bf::path &filepath, size_t size) {
-  stream.write((const char*)_data.data(), _size);
+  _assertFileDoesntExist(filepath);
+  auto blob = make_unique<OnDiskBlob>(filepath, size);
+  blob->_fillDataWithZeroes();
+  blob->_storeToDisk();
+  return std::move(blob);
 }
 
-void OnDiskBlob::FillDataWithZeroes() {
+void OnDiskBlob::_assertFileDoesntExist(const bf::path &filepath) {
+  if (bf::exists(filepath)) {
+    throw FileAlreadyExistsException(filepath);
+  }
+}
+
+void OnDiskBlob::_fillDataWithZeroes() {
   std::memset(_data.data(), 0, _size);
 }
 
+void OnDiskBlob::_storeToDisk() const {
+  ofstream file(_filepath.c_str(), ios::binary | ios::trunc);
+  _storeDataToStream(file);
+}
+
+void OnDiskBlob::_storeDataToStream(ostream &stream) const {
+  stream.write((const char*)_data.data(), _size);
+}
+
 } /* namespace ondisk */
 } /* namespace blobstore */

src/blobstore/implementations/ondisk/OnDiskBlob.h

@@ -5,6 +5,7 @@
 #include "blobstore/interface/Blob.h"
 #include "Data.h"
 
+#include <boost/filesystem/path.hpp>
 #include <iostream>
 
 namespace blobstore {
@@ -13,21 +14,28 @@ class OnDiskBlobStore;
 
 class OnDiskBlob: public Blob {
 public:
-  OnDiskBlob(size_t size);
+  OnDiskBlob(const boost::filesystem::path &filepath, size_t size);
   virtual ~OnDiskBlob();
 
+  static std::unique_ptr<OnDiskBlob> LoadFromDisk(const boost::filesystem::path &filepath);
+  static std::unique_ptr<OnDiskBlob> CreateOnDisk(const boost::filesystem::path &filepath, size_t size);
+
   void *data() override;
   const void *data() const override;
 
   size_t size() const override;
 
-  void LoadDataFromStream(std::istream &stream);
-  void StoreDataToStream(std::ostream &stream) const;
-  void FillDataWithZeroes();
-
 private:
+  const boost::filesystem::path _filepath;
   size_t _size;
   Data _data;
+
+  static void _assertFileDoesntExist(const boost::filesystem::path &filepath);
+  static size_t _getStreamSize(std::istream &stream);
+  void _loadDataFromStream(std::istream &stream);
+  void _storeDataToStream(std::ostream &stream) const;
+  void _fillDataWithZeroes();
+  void _storeToDisk() const;
 };
 
 } /* namespace ondisk */

src/blobstore/implementations/ondisk/OnDiskBlobStore.cpp

@@ -2,14 +2,7 @@
 
 #include "OnDiskBlob.h"
 
-#include <fstream>
-
 using std::unique_ptr;
-using std::make_unique;
-using std::istream;
-using std::ifstream;
-using std::ofstream;
-using std::ios;
 
 namespace blobstore {
 namespace ondisk {
@@ -18,45 +11,13 @@ OnDiskBlobStore::OnDiskBlobStore(const boost::filesystem::path &rootdir)
  : _rootdir(rootdir) {}
 
 unique_ptr<Blob> OnDiskBlobStore::create(const std::string &key, size_t size) {
-  auto blob = make_unique<OnDiskBlob>(size);
-  blob->FillDataWithZeroes();
-  _storeBlobData(key, blob.get());
-  return std::move(blob);
-}
-
-void OnDiskBlobStore::_storeBlobData(const std::string &key, const OnDiskBlob *blob) {
   auto file_path = _rootdir / key;
-  ofstream file(file_path.c_str(), ios::binary | ios::trunc);
+  return OnDiskBlob::CreateOnDisk(file_path, size);
-
-  blob->StoreDataToStream(file);
 }
 
 unique_ptr<Blob> OnDiskBlobStore::load(const std::string &key) {
   auto file_path = _rootdir / key;
-  ifstream file(file_path.c_str(), ios::binary);
+  return OnDiskBlob::LoadFromDisk(file_path);
-
-  return _createBlobFromStream(file);
-}
-
-unique_ptr<Blob> OnDiskBlobStore::_createBlobFromStream(istream &stream) {
-  size_t size = _getStreamSize(stream);
-
-  auto blob = make_unique<OnDiskBlob>(size);
-  blob->LoadDataFromStream(stream);
-  return std::move(blob);
-}
-
-size_t OnDiskBlobStore::_getStreamSize(istream &stream) {
-  auto current_pos = stream.tellg();
-
-  //Retrieve length
-  stream.seekg(0, stream.end);
-  auto endpos = stream.tellg();
-
-  //Restore old position
-  stream.seekg(current_pos, stream.beg);
-
-  return endpos - current_pos;
 }
 
 } /* namespace ondisk */

src/blobstore/implementations/ondisk/OnDiskBlobStore.h

@@ -5,7 +5,6 @@
 #include "blobstore/interface/BlobStore.h"
 
 #include <boost/filesystem/path.hpp>
-#include <iostream>
 
 namespace blobstore {
 namespace ondisk {
@@ -20,10 +19,6 @@ public:
 
 private:
   const boost::filesystem::path _rootdir;
-
-  void _storeBlobData(const std::string &key, const OnDiskBlob *blob);
-  std::unique_ptr<Blob> _createBlobFromStream(std::istream &stream);
-  size_t _getStreamSize(std::istream &stream);
 };
 
 } /* namespace ondisk */

src/test/blobstore/ondisk/OnDiskBlobCreateTest.cpp

@@ -0,0 +1,55 @@
+#include "gtest/gtest.h"
+
+#include "test/testutils/TempFile.h"
+
+#include "blobstore/implementations/ondisk/OnDiskBlob.h"
+#include "blobstore/implementations/ondisk/FileAlreadyExistsException.h"
+
+using ::testing::Test;
+using ::testing::WithParamInterface;
+using ::testing::Values;
+
+using std::unique_ptr;
+
+using namespace blobstore::ondisk;
+
+namespace bf = boost::filesystem;
+
+class OnDiskBlobCreateTest: public Test {
+public:
+  OnDiskBlobCreateTest()
+  // Don't create the temp file yet (therefore pass false to the TempFile constructor)
+  : file(false) {
+  }
+  TempFile file;
+};
+
+TEST_F(OnDiskBlobCreateTest, CreatingBlobCreatesFile) {
+  EXPECT_FALSE(bf::exists(file.path()));
+
+  auto blob = OnDiskBlob::CreateOnDisk(file.path(), 0);
+
+  EXPECT_TRUE(bf::exists(file.path()));
+  EXPECT_TRUE(bf::is_regular_file(file.path()));
+}
+
+TEST_F(OnDiskBlobCreateTest, CreatingExistingBlobThrowsException) {
+  auto blob1 = OnDiskBlob::CreateOnDisk(file.path(), 0);
+  EXPECT_THROW(OnDiskBlob::CreateOnDisk(file.path(), 0), FileAlreadyExistsException);
+}
+
+class OnDiskBlobCreateSizeTest: public OnDiskBlobCreateTest, public WithParamInterface<size_t> {};
+INSTANTIATE_TEST_CASE_P(OnDiskBlobCreateSizeTest, OnDiskBlobCreateSizeTest, Values(0, 1, 5, 1024, 10*1024*1024));
+
+TEST_P(OnDiskBlobCreateSizeTest, FileSizeIsCorrect) {
+  auto blob = OnDiskBlob::CreateOnDisk(file.path(), GetParam());
+
+  EXPECT_EQ(GetParam(), bf::file_size(file.path()));
+}
+
+TEST_P(OnDiskBlobCreateSizeTest, InMemorySizeIsCorrect) {
+  auto blob = OnDiskBlob::CreateOnDisk(file.path(), GetParam());
+
+  EXPECT_EQ(GetParam(), blob->size());
+}
+

src/test/blobstore/ondisk/OnDiskBlobLoadTest.cpp

@@ -0,0 +1,19 @@
+#include "gtest/gtest.h"
+
+#include "test/testutils/TempFile.h"
+
+#include "blobstore/implementations/ondisk/OnDiskBlob.h"
+
+using ::testing::Test;
+
+using std::unique_ptr;
+
+using blobstore::ondisk::OnDiskBlob;
+
+namespace bf = boost::filesystem;
+
+class OnDiskBlobLoadTest: public Test {
+public:
+  TempFile file;
+};
+