Add convenience method to open a file

Summary: These are convenience methods to open a file with FileHandlerFactory, without having to know about FileHandlerFactory. Reviewed By: hanghu Differential Revision: D64868465 fbshipit-source-id: 908d1a6f1ad47b6c16864ec5a1a4900a45ac97e6
facebookresearch · Oct 28, 2024 · 4bc3a73 · 4bc3a73
1 parent 48a30b7
commit 4bc3a73
Show file tree

Hide file tree

Showing 5 changed files with 32 additions and 14 deletions.
diff --git a/tools/vrs/VrsCommand.cpp b/tools/vrs/VrsCommand.cpp
@@ -498,8 +498,7 @@ int VrsCommand::runCommands() {
       FileSpec spec;
       unique_ptr<FileHandler> file;
       if (RecordFileReader::vrsFilePathToFileSpec(filteredReader.getPathOrUri(), spec) != 0 ||
-          FileHandlerFactory::getInstance().delegateOpen(spec, file) != 0 ||
-          !FileFormat::printVRSFileInternals(*file)) {
+          !(file = FileHandler::makeOpen(spec)) || !FileFormat::printVRSFileInternals(*file)) {
         statusCode = EXIT_FAILURE;
       }
     } break;
@@ -624,11 +623,8 @@ int VrsCommand::doCopyMerge() {
 }
 
 bool VrsCommand::isRemoteFileSystem(const string& path) {
-  unique_ptr<FileHandler> filehandler;
-  if (FileHandlerFactory::getInstance().delegateOpen(path, filehandler) != SUCCESS) {
-    return false;
-  }
-  return filehandler->isRemoteFileSystem();
+  unique_ptr<FileHandler> filehandler = FileHandler::makeOpen(path);
+  return filehandler && filehandler->isRemoteFileSystem();
 }
 
 DecimationParams& VrsCommand::getDecimatorParams() {

diff --git a/vrs/FileHandler.cpp b/vrs/FileHandler.cpp
@@ -57,6 +57,26 @@ struct CachingStrategyConverter : public EnumStringConverter<
 
 namespace vrs {
 
+unique_ptr<FileHandler> FileHandler::makeOpen(const string& filePath) {
+  unique_ptr<FileHandler> delegate;
+  int status = FileHandlerFactory::getInstance().delegateOpen(filePath, delegate);
+  if (status != 0) {
+    XR_LOGE("Could not open '{}': {}", filePath, errorCodeToMessage(status));
+    return nullptr;
+  }
+  return delegate;
+}
+
+unique_ptr<FileHandler> FileHandler::makeOpen(const FileSpec& fileSpec) {
+  unique_ptr<FileHandler> delegate;
+  int status = FileHandlerFactory::getInstance().delegateOpen(fileSpec, delegate);
+  if (status != 0) {
+    XR_LOGE("Could not open '{}': {}", fileSpec.toPathJsonUri(), errorCodeToMessage(status));
+    return nullptr;
+  }
+  return delegate;
+}
+
 int FileHandler::open(const string& filePath) {
   FileSpec fileSpec;
   int status = fileSpec.fromPathJsonUri(filePath, getFileHandlerName());

diff --git a/vrs/FileHandler.h b/vrs/FileHandler.h
@@ -84,6 +84,10 @@ class FileHandler : public FileDelegator {
 
   FileHandler() = default;
 
+  /// Open a file in read-only mode. Returns an open file handler, or nullptr on error.
+  static unique_ptr<FileHandler> makeOpen(const string& filePath);
+  static unique_ptr<FileHandler> makeOpen(const FileSpec& fileSpec);
+
   /// 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.

diff --git a/vrs/utils/Validation.cpp b/vrs/utils/Validation.cpp
@@ -26,7 +26,6 @@
 #include <logging/Log.h>
 #include <logging/Verify.h>
 
-#include <vrs/FileHandlerFactory.h>
 #include <vrs/helpers/Rapidjson.hpp>
 #include <vrs/helpers/Strings.h>
 #include <vrs/helpers/Throttler.h>
@@ -493,8 +492,8 @@ string recordsChecksum(const string& path, bool showProgress) {
 string verbatimChecksum(const string& path, bool showProgress) {
   const char* kStatus = "Calculating ";
   const char* kReset = showProgress ? kResetCurrentLine : "";
-  unique_ptr<FileHandler> file;
-  if (FileHandlerFactory::getInstance().delegateOpen(path, file) != 0) {
+  unique_ptr<FileHandler> file = FileHandler::makeOpen(path);
+  if (!file) {
     return "<file open error>";
   }
   XXH64Digester digester;

diff --git a/vrs/utils/test/ImageIndexerLoaderTest.cpp b/vrs/utils/test/ImageIndexerLoaderTest.cpp
@@ -18,7 +18,6 @@
 
 #include <TestDataDir/TestDataDir.h>
 
-#include <vrs/FileHandlerFactory.h>
 #include <vrs/os/Utils.h>
 #include <vrs/utils/ImageIndexer.h>
 #include <vrs/utils/ImageLoader.h>
@@ -81,8 +80,8 @@ TEST_F(ImageIndexerLoaderTest, ImageIndexerLoaderTest) {
       {4215175, 2106022, format, CompressionType::Zstd, 52, 3760128},
   };
   ASSERT_EQ(readImages, expectedImages);
-  unique_ptr<FileHandler> file;
-  ASSERT_EQ(FileHandlerFactory::getInstance().delegateOpen(kRgbFile, file), 0);
+  unique_ptr<FileHandler> file = FileHandler::makeOpen(kRgbFile);
+  ASSERT_NE(file, nullptr);
   EXPECT_EQ(loadFrameFromFile(*file, readImages[0], format, 4114475262886596638ULL), 0);
   EXPECT_EQ(loadFrameFromFile(*file, readImages[1], format, 16026781315276957005ULL), 0);
   EXPECT_EQ(loadFrameFromFile(*file, readImages[2], format, 8098506684566711634ULL), 0);
@@ -93,7 +92,7 @@ TEST_F(ImageIndexerLoaderTest, ImageIndexerLoaderTest) {
       {6046, 1985655, "jpg", CompressionType::None, 0, 0},
   };
   EXPECT_EQ(readImages, expectedImages);
-  ASSERT_EQ(FileHandlerFactory::getInstance().delegateOpen(kJpgFile, file), 0);
+  ASSERT_NE(file = FileHandler::makeOpen(kJpgFile), nullptr);
   EXPECT_EQ(loadFrameFromFile(*file, readImages[0], "jpg", 10323177114171200117ULL), 0);
   EXPECT_EQ(loadFrameFromMemory(*file, readImages[0], "jpg", 10323177114171200117ULL), 0);
 }