diff --git a/tensorflow/c/experimental/filesystem/filesystem_interface.h b/tensorflow/c/experimental/filesystem/filesystem_interface.h index 947dc48150c..2340078afa9 100644 --- a/tensorflow/c/experimental/filesystem/filesystem_interface.h +++ b/tensorflow/c/experimental/filesystem/filesystem_interface.h @@ -527,11 +527,13 @@ typedef struct TF_FilesystemOps { /// of type `TF_Status*`. /// /// If `statuses` is not null, plugins must fill each element with detailed - /// status for each file, as if calling `path_exists` on each one. + /// status for each file, as if calling `path_exists` on each one. Core + /// TensorFlow initializes the `statuses` array and plugins must use + /// `TF_SetStatus` to set each element instead of dirrectly assigning. /// /// DEFAULT IMPLEMENTATION: Checks existence of every file. Needs /// `path_exists`. - bool (*paths_exist)(const TF_Filesystem* filesystem, const char** paths, + bool (*paths_exist)(const TF_Filesystem* filesystem, char** paths, int num_files, TF_Status** statuses); /// Obtains statistics for the given `path`. diff --git a/tensorflow/c/experimental/filesystem/modular_filesystem.cc b/tensorflow/c/experimental/filesystem/modular_filesystem.cc index 9e51e4bc9e8..35bec2feb80 100644 --- a/tensorflow/c/experimental/filesystem/modular_filesystem.cc +++ b/tensorflow/c/experimental/filesystem/modular_filesystem.cc @@ -14,6 +14,7 @@ limitations under the License. ==============================================================================*/ #include "tensorflow/c/experimental/filesystem/modular_filesystem.h" +#include #include #include @@ -111,15 +112,47 @@ Status ModularFileSystem::NewReadOnlyMemoryRegionFromFile( } Status ModularFileSystem::FileExists(const std::string& fname) { - // TODO(mihaimaruseac): Implementation to come in a new change - return Status(error::UNIMPLEMENTED, - "Modular filesystem stub not implemented yet"); + if (ops_->path_exists == nullptr) + return errors::Unimplemented(tensorflow::strings::StrCat( + "Filesystem for ", fname, " does not support FileExists()")); + + UniquePtrTo_TF_Status plugin_status(TF_NewStatus(), TF_DeleteStatus); + const std::string translated_name = TranslateName(fname); + ops_->path_exists(filesystem_.get(), translated_name.c_str(), + plugin_status.get()); + return StatusFromTF_Status(plugin_status.get()); } bool ModularFileSystem::FilesExist(const std::vector& files, std::vector* status) { - // TODO(mihaimaruseac): Implementation to come in a new change - return true; + if (ops_->paths_exist == nullptr) + return FileSystem::FilesExist(files, status); + + std::vector translated_names; + translated_names.reserve(files.size()); + for (int i = 0; i < files.size(); i++) + translated_names.push_back(strdup(TranslateName(files[i]).c_str())); + + bool result; + if (status == nullptr) { + result = ops_->paths_exist(filesystem_.get(), translated_names.data(), + files.size(), nullptr); + } else { + std::vector plugin_status; + plugin_status.reserve(files.size()); + for (int i = 0; i < files.size(); i++) + plugin_status.push_back(TF_NewStatus()); + result = ops_->paths_exist(filesystem_.get(), translated_names.data(), + files.size(), plugin_status.data()); + for (int i = 0; i < files.size(); i++) { + status->push_back(StatusFromTF_Status(plugin_status[i])); + TF_DeleteStatus(plugin_status[i]); + } + } + + for (int i = 0; i < files.size(); i++) free(translated_names[i]); + + return result; } Status ModularFileSystem::GetChildren(const std::string& dir, diff --git a/tensorflow/c/experimental/filesystem/modular_filesystem_test.cc b/tensorflow/c/experimental/filesystem/modular_filesystem_test.cc index 9394072c21b..659a07e9dee 100644 --- a/tensorflow/c/experimental/filesystem/modular_filesystem_test.cc +++ b/tensorflow/c/experimental/filesystem/modular_filesystem_test.cc @@ -539,6 +539,95 @@ TEST_P(ModularFileSystemTest, TestDeleteDirectoryPathIsInvalid) { EXPECT_PRED2(UninmplementedOrReturnsCode, status, Code::FAILED_PRECONDITION); } +TEST_P(ModularFileSystemTest, TestFileExists) { + const std::string filepath = GetURIForPath("a_file"); + std::unique_ptr file; + Status status = env_->NewWritableFile(filepath, &file); + if (!status.ok()) GTEST_SKIP() << "NewWritableFile() not supported"; + + status = env_->FileExists(filepath); + EXPECT_PRED2(UninmplementedOrReturnsCode, status, Code::OK); +} + +TEST_P(ModularFileSystemTest, TestFileExistsButIsDirectory) { + const std::string filepath = GetURIForPath("a_file"); + Status status = env_->CreateDir(filepath); + if (!status.ok()) GTEST_SKIP() << "CreateDir() not supported"; + + status = env_->FileExists(filepath); + EXPECT_PRED2(UninmplementedOrReturnsCode, status, Code::OK); +} + +TEST_P(ModularFileSystemTest, TestFileExistsNotFound) { + const std::string filepath = GetURIForPath("a_file"); + Status status = env_->FileExists(filepath); + EXPECT_PRED2(UninmplementedOrReturnsCode, status, Code::NOT_FOUND); +} + +TEST_P(ModularFileSystemTest, TestFileExistsPathIsInvalid) { + const std::string filepath = GetURIForPath("a_file"); + std::unique_ptr file; + Status status = env_->NewWritableFile(filepath, &file); + if (!status.ok()) GTEST_SKIP() << "NewWritableFile() not supported"; + + const std::string target_path = GetURIForPath("a_file/a_new_file"); + status = env_->FileExists(target_path); + EXPECT_PRED2(UninmplementedOrReturnsCode, status, Code::FAILED_PRECONDITION); +} + +TEST_P(ModularFileSystemTest, TestFilesExist) { + const std::vector filenames = {GetURIForPath("a"), + GetURIForPath("b")}; + for (const auto& filename : filenames) { + std::unique_ptr file; + Status status = env_->NewWritableFile(filename, &file); + if (!status.ok()) GTEST_SKIP() << "NewWritableFile() not supported"; + } + + EXPECT_TRUE(env_->FilesExist(filenames, /*status=*/nullptr)); + + std::vector statuses; + EXPECT_TRUE(env_->FilesExist(filenames, &statuses)); + EXPECT_EQ(statuses.size(), filenames.size()); + for (const auto& status : statuses) + EXPECT_PRED2(UninmplementedOrReturnsCode, status, Code::OK); +} + +TEST_P(ModularFileSystemTest, TestFilesExistAllFailureModes) { + // if reordering these, make sure to reorder checks at the end + const std::vector filenames = { + GetURIForPath("a_dir"), + GetURIForPath("a_file"), + GetURIForPath("a_file/a_new_file"), + GetURIForPath("file_not_found"), + }; + + Status status = env_->CreateDir(filenames[0]); + if (!status.ok()) GTEST_SKIP() << "CreateDir() not supported"; + + std::unique_ptr file; + status = env_->NewWritableFile(filenames[1], &file); + if (!status.ok()) GTEST_SKIP() << "NewWritableFile() not supported"; + + std::vector statuses; + EXPECT_FALSE(env_->FilesExist(filenames, &statuses)); + EXPECT_EQ(statuses.size(), filenames.size()); + EXPECT_PRED2(UninmplementedOrReturnsCode, statuses[0], Code::OK); + EXPECT_PRED2(UninmplementedOrReturnsCode, statuses[1], Code::OK); + EXPECT_PRED2(UninmplementedOrReturnsCode, statuses[2], + Code::FAILED_PRECONDITION); + EXPECT_PRED2(UninmplementedOrReturnsCode, statuses[3], Code::NOT_FOUND); +} + +TEST_P(ModularFileSystemTest, TestFilesExistsNoFiles) { + const std::vector filenames = {}; + EXPECT_TRUE(env_->FilesExist(filenames, /*status=*/nullptr)); + + std::vector statuses; + EXPECT_TRUE(env_->FilesExist(filenames, &statuses)); + EXPECT_TRUE(statuses.empty()); +} + TEST_P(ModularFileSystemTest, TestAppendAndTell) { const std::string filename = GetURIForPath("a_file"); std::unique_ptr file; diff --git a/tensorflow/c/experimental/filesystem/plugins/posix/posix_filesystem.cc b/tensorflow/c/experimental/filesystem/plugins/posix/posix_filesystem.cc index 353190a1601..52348eb946a 100644 --- a/tensorflow/c/experimental/filesystem/plugins/posix/posix_filesystem.cc +++ b/tensorflow/c/experimental/filesystem/plugins/posix/posix_filesystem.cc @@ -287,7 +287,13 @@ static void DeleteDir(const TF_Filesystem* filesystem, const char* path, TF_SetStatus(status, TF_OK, ""); } -// TODO(mihaimaruseac): More implementations to follow in subsequent changes. +static void PathExists(const TF_Filesystem* filesystem, const char* path, + TF_Status* status) { + if (access(path, F_OK) != 0) + TF_SetStatusFromIOError(status, errno, path); + else + TF_SetStatus(status, TF_OK, ""); +} } // namespace tf_posix_filesystem @@ -317,6 +323,11 @@ void TF_InitPlugin(TF_Status* status) { /*recursively_create_dir=*/nullptr, tf_posix_filesystem::DeleteFile, tf_posix_filesystem::DeleteDir, + /*delete_recursively=*/nullptr, + /*rename_file=*/nullptr, + /*copy_file=*/nullptr, + tf_posix_filesystem::PathExists, + /*paths_exist=*/nullptr, nullptr, };