Refactor FileHandler name handling

Summary:
The way we were handling a FileHandler's name was not consistent and didn't make much sense, as most FileHandlers have a static name.
With a virtual function, we end up with the same behaviors, and more flexibility.

Reviewed By: kiminoue7

Differential Revision: D47580165

fbshipit-source-id: a2d1a2947ef772df3b09704cdbc9e57d8b771b2e

vrs/DiskFile.cpp

@@ -50,8 +50,6 @@ const string& DiskFile::staticName() {
   return sDiskFileHandlerName;
 }
 
-DiskFile::DiskFile() : WriteFileHandler(DiskFile::staticName()) {}
-
 DiskFile::~DiskFile() {
   DiskFile::close(); // overrides not available in constructors & destructors
 }
@@ -60,6 +58,10 @@ unique_ptr<FileHandler> DiskFile::makeNew() const {
   return make_unique<DiskFile>();
 }
 
+const string& DiskFile::getFileHandlerName() const {
+  return staticName();
+}
+
 int DiskFile::close() {
   lastError_ = 0;
   for (auto& chunk : chunks_) {

vrs/DiskFile.h

@@ -41,12 +41,14 @@ class DiskFile : public WriteFileHandler {
     }
   };
 
-  DiskFile();
+  DiskFile() = default;
   ~DiskFile() override;
 
   /// Make a new DiskFile object, with a default state.
   std::unique_ptr<FileHandler> makeNew() const override;
 
+  const string& getFileHandlerName() const override;
+
   /// Open a file in read-only mode.
   int openSpec(const FileSpec& fileSpec) override;
 

vrs/FileHandler.h

@@ -84,12 +84,13 @@ class FileHandler : public FileDelegator {
 
   using CacheStatsCallbackFunction = std::function<void(const CacheStats& stats)>;
 
-  FileHandler(const string& fileHandlerName) : fileHandlerName_{fileHandlerName} {}
+  FileHandler() = default;
 
   /// Make a new instance of the concrete class implementing this interface in its default state,
   /// no matter what this object's state is, so that we can access more files using the same method.
   /// @return A new object of the concrete type, ready to be used to open a new file.
   virtual unique_ptr<FileHandler> makeNew() const = 0;
+  virtual const string& getFileHandlerName() const = 0;
 
   /// Open a file in read-only mode.
   /// @param filePath: a disk path, or anything that the particular module recognizes.
@@ -224,10 +225,6 @@ class FileHandler : public FileDelegator {
     return true;
   }
 
-  string getFileHandlerName() const {
-    return fileHandlerName_;
-  }
-
   bool isFileHandlerMatch(const FileSpec& fileSpec) const;
 
   /// Tell if the file handler is handling remote data, that might need caching for instance.
@@ -237,9 +234,6 @@ class FileHandler : public FileDelegator {
   virtual bool showProgress() const {
     return isRemoteFileSystem();
   }
-
- protected:
-  string fileHandlerName_;
 };
 
 /// Helper class to temporarily modify a FileHandler's caching strategy.

vrs/WriteFileHandler.h

@@ -39,7 +39,7 @@ using std::vector;
 /// VRS file creation.
 class WriteFileHandler : public FileHandler {
  public:
-  WriteFileHandler(const string& fileHandlerName) : FileHandler(fileHandlerName) {}
+  WriteFileHandler() = default;
 
   /// Create a new file for writing.
   /// @param newFilePath: a disk path to create the file.

vrs/test/FileHandlerFactoryTest.cpp

@@ -39,9 +39,7 @@ struct FileHandlerFactoryTest : testing::Test {
 // Fake FileHandler class that just pretends to open any path
 class FakeHandler : public DiskFile {
  public:
-  FakeHandler(const string& name) {
-    fileHandlerName_ = name;
-  }
+  FakeHandler(const string& name) : fileHandlerName_{name} {}
   unique_ptr<FileHandler> makeNew() const override {
     return make_unique<FakeHandler>(fileHandlerName_);
   }
@@ -56,6 +54,13 @@ class FakeHandler : public DiskFile {
     outNewDelegate.reset();
     return 0;
   }
+
+  const string& getFileHandlerName() const override {
+    return fileHandlerName_;
+  }
+
+ private:
+  const string fileHandlerName_;
 };
 
 static int openVRSFile(const string& path, unique_ptr<FileHandler>& outFile) {

vrs/test/file_tests/UserRecordsFileHandlerTest.cpp

@@ -42,10 +42,13 @@ const string kUserFileRecordsFileHandler = "UserRecordsFileHandler";
 
 class UserRecordsFileHandler : public WriteFileHandler {
  public:
-  UserRecordsFileHandler() : WriteFileHandler(kUserFileRecordsFileHandler) {}
+  UserRecordsFileHandler() {}
   std::unique_ptr<FileHandler> makeNew() const override {
     return make_unique<UserRecordsFileHandler>();
   }
+  const string& getFileHandlerName() const override {
+    return kUserFileRecordsFileHandler;
+  }
 
   /// Minimal implementations needed for a custom WriteFileHandler used for data writes only
   /// It can only write forward, no seek operations, no read back

vrs/utils/BufferRecordReader.hpp

@@ -26,7 +26,7 @@ namespace vrs::utils {
 /// A FileHandler that reads data from a buffer
 class BufferFileHandler : public FileHandler {
  public:
-  BufferFileHandler() : FileHandler("BufferFileHandler") {}
+  BufferFileHandler() : fileHandlerName_{"BufferFileHandler"} {}
 
   void init(const vector<uint8_t>& buffer) {
     data_ = buffer.data();
@@ -39,6 +39,9 @@ class BufferFileHandler : public FileHandler {
   std::unique_ptr<FileHandler> makeNew() const override {
     return std::make_unique<BufferFileHandler>();
   }
+  const string& getFileHandlerName() const override {
+    return fileHandlerName_;
+  }
 
   int openSpec(const FileSpec& fileSpec) override {
     return FAILURE;
@@ -112,11 +115,12 @@ class BufferFileHandler : public FileHandler {
   }
 
  private:
-  const uint8_t* data_{};
-  int64_t totalSize_;
-  uint32_t readSize_;
-  uint32_t lastReadSize_;
-  int lastError_;
+  const string fileHandlerName_;
+  const uint8_t* data_{nullptr};
+  int64_t totalSize_{0};
+  uint32_t readSize_{0};
+  uint32_t lastReadSize_{0};
+  int lastError_{0};
 };
 
 /// A RecordReader that reads data from a buffer