ARROW-7306: [C++] Add Result-returning version of FileSystemFromUri

cpp/src/arrow/filesystem/filesystem.cc

@@ -333,8 +333,8 @@ Result<std::shared_ptr<io::OutputStream>> SlowFileSystem::OpenAppendStream(
   return base_fs_->OpenAppendStream(path);
 }
 
-Status FileSystemFromUri(const std::string& uri_string,
-                         std::shared_ptr<FileSystem>* out_fs, std::string* out_path) {
+Result<std::shared_ptr<FileSystem>> FileSystemFromUri(const std::string& uri_string,
+                                                      std::string* out_path) {
   Uri uri;
   RETURN_NOT_OK(uri.Parse(uri_string));
   if (out_path != nullptr) {
@@ -348,21 +348,18 @@ Status FileSystemFromUri(const std::string& uri_string,
     if (out_path != nullptr) {
       *out_path = uri_string;
     }
-    *out_fs = std::make_shared<LocalFileSystem>();
-    return Status::OK();
+    return std::make_shared<LocalFileSystem>();
   }
 #endif
   if (scheme == "" || scheme == "file") {
-    *out_fs = std::make_shared<LocalFileSystem>();
-    return Status::OK();
+    return std::make_shared<LocalFileSystem>();
   }
 
   if (scheme == "hdfs") {
 #ifdef ARROW_HDFS
     ARROW_ASSIGN_OR_RAISE(auto options, HdfsOptions::FromUri(uri));
     ARROW_ASSIGN_OR_RAISE(auto hdfs, HadoopFileSystem::Make(options));
-    *out_fs = hdfs;
-    return Status::OK();
+    return hdfs;
 #else
     return Status::NotImplemented("Arrow compiled without HDFS support");
 #endif
@@ -375,8 +372,7 @@ Status FileSystemFromUri(const std::string& uri_string,
     *out_path = std::string(RemoveLeadingSlash(*out_path));
   }
   if (scheme == "mock") {
-    *out_fs = std::make_shared<internal::MockFileSystem>(internal::CurrentTimePoint());
-    return Status::OK();
+    return std::make_shared<internal::MockFileSystem>(internal::CurrentTimePoint());
   }
 
   // TODO add support for S3 URIs

cpp/src/arrow/filesystem/filesystem.h

@@ -316,6 +316,18 @@ class ARROW_EXPORT SlowFileSystem : public FileSystem {
   std::shared_ptr<io::LatencyGenerator> latencies_;
 };
 
+/// \brief Create a new FileSystem by URI
+///
+/// A scheme-less URI is considered a local filesystem path.
+/// Recognized schemes are "file", "mock" and "hdfs".
+///
+/// \param[in] uri a URI-based path, ex: file:///some/local/path
+/// \param[out] out_path (optional) Path inside the filesystem.
+/// \return out_fs FileSystem instance.
+ARROW_EXPORT
+Result<std::shared_ptr<FileSystem>> FileSystemFromUri(const std::string& uri,
+                                                      std::string* out_path = NULLPTR);
+
 /// \brief Create a new FileSystem by URI
 ///
 /// A scheme-less URI is considered a local filesystem path.
@@ -325,9 +337,12 @@ class ARROW_EXPORT SlowFileSystem : public FileSystem {
 /// \param[out] out_fs FileSystem instance.
 /// \param[out] out_path (optional) Path inside the filesystem.
 /// \return Status
-ARROW_EXPORT
-Status FileSystemFromUri(const std::string& uri, std::shared_ptr<FileSystem>* out_fs,
-                         std::string* out_path = NULLPTR);
+ARROW_DEPRECATED("Use Result-returning version")
+inline Status FileSystemFromUri(const std::string& uri,
+                                std::shared_ptr<FileSystem>* out_fs,
+                                std::string* out_path = NULLPTR) {
+  return FileSystemFromUri(uri, out_path).Value(out_fs);
+}
 
 }  // namespace fs
 }  // namespace arrow

cpp/src/arrow/filesystem/filesystem_test.cc

@@ -423,22 +423,22 @@ TEST_F(TestMockFS, Make) {
 
 TEST_F(TestMockFS, FileSystemFromUri) {
   std::string path;
-  ASSERT_OK(FileSystemFromUri("mock:", &fs_, &path));
+  ASSERT_OK_AND_ASSIGN(fs_, FileSystemFromUri("mock:", &path));
   ASSERT_EQ(path, "");
   CheckDirs({});  // Ensures it's a MockFileSystem
-  ASSERT_OK(FileSystemFromUri("mock:foo/bar", &fs_, &path));
+  ASSERT_OK_AND_ASSIGN(fs_, FileSystemFromUri("mock:foo/bar", &path));
   ASSERT_EQ(path, "foo/bar");
   CheckDirs({});
-  ASSERT_OK(FileSystemFromUri("mock:/foo/bar", &fs_, &path));
+  ASSERT_OK_AND_ASSIGN(fs_, FileSystemFromUri("mock:/foo/bar", &path));
   ASSERT_EQ(path, "foo/bar");
   CheckDirs({});
-  ASSERT_OK(FileSystemFromUri("mock:/foo/bar/?q=xxx", &fs_, &path));
+  ASSERT_OK_AND_ASSIGN(fs_, FileSystemFromUri("mock:/foo/bar/?q=xxx", &path));
   ASSERT_EQ(path, "foo/bar/");
   CheckDirs({});
-  ASSERT_OK(FileSystemFromUri("mock:///foo/bar", &fs_, &path));
+  ASSERT_OK_AND_ASSIGN(fs_, FileSystemFromUri("mock:///foo/bar", &path));
   ASSERT_EQ(path, "foo/bar");
   CheckDirs({});
-  ASSERT_OK(FileSystemFromUri("mock:///foo/bar?q=zzz", &fs_, &path));
+  ASSERT_OK_AND_ASSIGN(fs_, FileSystemFromUri("mock:///foo/bar?q=zzz", &path));
   ASSERT_EQ(path, "foo/bar");
   CheckDirs({});
 }

cpp/src/arrow/filesystem/hdfs_test.cc

@@ -119,7 +119,7 @@ class TestHadoopFileSystem : public ::testing::Test {
     std::shared_ptr<FileSystem> uri_fs;
     std::string path;
     ARROW_LOG(INFO) << "!!! uri = " << ss.str();
-    ASSERT_OK(FileSystemFromUri(ss.str(), &uri_fs, &path));
+    ASSERT_OK_AND_ASSIGN(uri_fs, FileSystemFromUri(ss.str(), &path));
     ASSERT_EQ(path, "/");
 
     // Sanity check

cpp/src/arrow/filesystem/localfs_test.cc

@@ -114,7 +114,7 @@ class TestLocalFS : public LocalFSTestMixin {
 
   void TestFileSystemFromUri(const std::string& uri) {
     std::string path;
-    ASSERT_OK(FileSystemFromUri(uri, &fs_, &path));
+    ASSERT_OK_AND_ASSIGN(fs_, FileSystemFromUri(uri, &path));
 
     // Test that the right location on disk is accessed
     CreateFile(fs_.get(), local_path_ + "abc", "some data");