Extract s3 file operations to separate file
Preparation for reading from memory. It will be called from log backend.
Change-Id: Ie2b6fff413fae08cc66afccb17d02b99a987847d
Signed-off-by: James Kuszmaul <james.kuszmaul@bluerivertech.com>
diff --git a/aos/events/logging/BUILD b/aos/events/logging/BUILD
index bfa51fa..9f31b15 100644
--- a/aos/events/logging/BUILD
+++ b/aos/events/logging/BUILD
@@ -73,6 +73,27 @@
)
cc_library(
+ name = "file_operations",
+ srcs = ["file_operations.cc"],
+ hdrs = ["file_operations.h"],
+ target_compatible_with = ["@platforms//os:linux"],
+ deps = [
+ "@com_github_google_glog//:glog",
+ "@com_google_absl//absl/strings",
+ ],
+)
+
+cc_library(
+ name = "s3_operations",
+ srcs = ["s3_operations.cc"],
+ hdrs = ["s3_operations.h"],
+ deps = [
+ ":file_operations",
+ ":s3_fetcher",
+ ],
+)
+
+cc_library(
name = "log_backend",
srcs = ["log_backend.cc"],
hdrs = ["log_backend.h"],
@@ -141,6 +162,7 @@
target_compatible_with = ["@platforms//os:linux"],
visibility = ["//visibility:public"],
deps = [
+ ":file_operations",
":boot_timestamp",
":snappy_encoder",
":buffer_encoder",
@@ -162,6 +184,7 @@
] + select({
"//tools:cpu_k8": [
":s3_fetcher",
+ ":s3_operations",
":lzma_encoder",
],
"//tools:cpu_arm64": [":lzma_encoder"],
diff --git a/aos/events/logging/file_operations.cc b/aos/events/logging/file_operations.cc
new file mode 100644
index 0000000..75910e5
--- /dev/null
+++ b/aos/events/logging/file_operations.cc
@@ -0,0 +1,38 @@
+#include "aos/events/logging/file_operations.h"
+
+#include "absl/strings/match.h"
+#include "glog/logging.h"
+
+namespace aos::logger::internal {
+
+bool IsValidFilename(std::string_view filename) {
+ return absl::EndsWith(filename, ".bfbs") ||
+ absl::EndsWith(filename, ".bfbs.xz") ||
+ absl::EndsWith(filename, ".bfbs.sz");
+}
+
+void LocalFileOperations::FindLogs(std::vector<std::string> *files) {
+ auto MaybeAddFile = [&files](std::string_view filename) {
+ if (!IsValidFilename(filename)) {
+ VLOG(1) << "Ignoring " << filename << " with invalid extension.";
+ } else {
+ VLOG(1) << "Found log " << filename;
+ files->emplace_back(filename);
+ }
+ };
+ if (std::filesystem::is_directory(filename_)) {
+ VLOG(1) << "Searching in " << filename_;
+ for (const auto &file :
+ std::filesystem::recursive_directory_iterator(filename_)) {
+ if (!file.is_regular_file()) {
+ VLOG(1) << file << " is not file.";
+ continue;
+ }
+ MaybeAddFile(file.path().string());
+ }
+ } else {
+ MaybeAddFile(filename_);
+ }
+}
+
+} // namespace aos::logger::internal
diff --git a/aos/events/logging/file_operations.h b/aos/events/logging/file_operations.h
new file mode 100644
index 0000000..faf63cb
--- /dev/null
+++ b/aos/events/logging/file_operations.h
@@ -0,0 +1,40 @@
+#ifndef AOS_EVENTS_LOGGING_FILE_OPERATIONS_H_
+#define AOS_EVENTS_LOGGING_FILE_OPERATIONS_H_
+
+#include <filesystem>
+#include <string>
+#include <vector>
+
+namespace aos::logger::internal {
+
+// Predicate to include or exclude file to be considered as a log file.
+bool IsValidFilename(std::string_view filename);
+
+// Abstraction that supports listing of the logs on file system and S3. It is
+// associated with either a single file or directory that contains log files.
+class FileOperations {
+ public:
+ virtual ~FileOperations() = default;
+
+ virtual bool Exists() = 0;
+ virtual void FindLogs(std::vector<std::string> *files) = 0;
+};
+
+// Implements FileOperations with standard POSIX filesystem APIs. These work on
+// files local to the machine they're running on.
+class LocalFileOperations final : public FileOperations {
+ public:
+ explicit LocalFileOperations(std::string_view filename)
+ : filename_(filename) {}
+
+ bool Exists() override { return std::filesystem::exists(filename_); }
+
+ void FindLogs(std::vector<std::string> *files) override;
+
+ private:
+ std::string filename_;
+};
+
+} // namespace aos::logger::internal
+
+#endif // AOS_EVENTS_LOGGING_FILE_OPERATIONS_H_
diff --git a/aos/events/logging/log_reader_utils_test.cc b/aos/events/logging/log_reader_utils_test.cc
index 9977be9..911c4eb 100644
--- a/aos/events/logging/log_reader_utils_test.cc
+++ b/aos/events/logging/log_reader_utils_test.cc
@@ -1,5 +1,6 @@
#include "aos/events/logging/log_reader_utils.h"
+#include "aos/events/logging/file_operations.h"
#include "aos/events/logging/multinode_logger_test_lib.h"
#include "aos/events/ping_lib.h"
#include "aos/events/pong_lib.h"
@@ -126,4 +127,31 @@
EXPECT_EQ(pong_count, 0);
reader.Deregister();
}
+
+// Verify that it is OK to list single file.
+TEST(FileOperationTest, SingleFile) {
+ std::string log_file = aos::testing::TestTmpDir() + "/test.bfbs";
+ util::WriteStringToFileOrDie(log_file, "test");
+ internal::LocalFileOperations file_op(log_file);
+ EXPECT_TRUE(file_op.Exists());
+ std::vector<std::string> logs;
+ file_op.FindLogs(&logs);
+ ASSERT_EQ(logs.size(), 1);
+ EXPECT_EQ(logs.front(), log_file);
+}
+
+// Verify that it is OK to list folder with log file.
+TEST(FileOperationTest, ListDirectory) {
+ std::string log_folder = aos::testing::TestTmpDir() + "/log_folder/";
+ std::filesystem::create_directories(log_folder);
+ std::string log_file = log_folder + "test.bfbs";
+ util::WriteStringToFileOrDie(log_file, "test");
+ internal::LocalFileOperations file_op(log_folder);
+ EXPECT_TRUE(file_op.Exists());
+ std::vector<std::string> logs;
+ file_op.FindLogs(&logs);
+ ASSERT_EQ(logs.size(), 1);
+ EXPECT_EQ(logs.front(), log_file);
+}
+
} // namespace aos::logger::testing
diff --git a/aos/events/logging/logfile_sorting.cc b/aos/events/logging/logfile_sorting.cc
index f6d12da..81305d7 100644
--- a/aos/events/logging/logfile_sorting.cc
+++ b/aos/events/logging/logfile_sorting.cc
@@ -11,6 +11,7 @@
#include "absl/container/btree_map.h"
#include "absl/strings/str_join.h"
+#include "aos/events/logging/file_operations.h"
#include "aos/events/logging/logfile_utils.h"
#include "aos/flatbuffer_merge.h"
#include "aos/flatbuffers.h"
@@ -19,7 +20,7 @@
#include "sys/stat.h"
#if ENABLE_S3
-#include "aos/events/logging/s3_fetcher.h"
+#include "aos/events/logging/s3_operations.h"
#endif
DEFINE_bool(quiet_sorting, false,
@@ -30,69 +31,12 @@
namespace {
namespace chrono = std::chrono;
-// Check if string ends with ending
-bool EndsWith(std::string_view str, std::string_view ending) {
- return str.size() >= ending.size() &&
- str.substr(str.size() - ending.size()) == ending;
-}
-
-class FileOperations {
- public:
- virtual ~FileOperations() = default;
-
- virtual bool Exists() = 0;
- virtual void FindLogs(std::vector<std::string> *files) = 0;
-};
-
-// Implements FileOperations with standard POSIX filesystem APIs. These work on
-// files local to the machine they're running on.
-class LocalFileOperations final : public FileOperations {
- public:
- LocalFileOperations(std::string_view filename) : filename_(filename) {}
-
- bool Exists() override {
- struct stat stat_results;
- int error = stat(filename_.c_str(), &stat_results);
- return error == 0;
- }
-
- void FindLogs(std::vector<std::string> *files) override;
-
- private:
- std::string filename_;
-};
-
-bool IsValidFilename(std::string_view filename) {
- return EndsWith(filename, ".bfbs") || EndsWith(filename, ".bfbs.xz") ||
- EndsWith(filename, ".bfbs.sz");
-}
-
-#if ENABLE_S3
-class S3FileOperations final : public FileOperations {
- public:
- S3FileOperations(std::string_view url) : object_urls_(ListS3Objects(url)) {}
-
- bool Exists() override { return !object_urls_.empty(); }
- void FindLogs(std::vector<std::string> *files) override {
- // We already have a recursive listing, so just grab all the objects from
- // there.
- for (const std::string &object_url : object_urls_) {
- if (IsValidFilename(object_url)) {
- files->push_back(object_url);
- }
- }
- }
-
- private:
- const std::vector<std::string> object_urls_;
-};
-#endif
-
-std::unique_ptr<FileOperations> MakeFileOperations(std::string_view filename) {
+std::unique_ptr<internal::FileOperations> MakeFileOperations(
+ std::string_view filename) {
static constexpr std::string_view kS3 = "s3:";
if (filename.substr(0, kS3.size()) == kS3) {
#if ENABLE_S3
- return std::make_unique<S3FileOperations>(filename);
+ return std::make_unique<internal::S3FileOperations>(filename);
#else
LOG(FATAL) << "Reading files from S3 not supported on this platform";
#endif
@@ -100,31 +44,7 @@
if (filename.find("://") != filename.npos) {
LOG(FATAL) << "This looks like a URL of an unknown type: " << filename;
}
- return std::make_unique<LocalFileOperations>(filename);
-}
-
-void LocalFileOperations::FindLogs(std::vector<std::string> *files) {
- DIR *directory = opendir(filename_.c_str());
-
- if (directory == nullptr) {
- if (IsValidFilename(filename_)) {
- files->emplace_back(filename_);
- }
- return;
- }
-
- struct dirent *directory_entry;
- while ((directory_entry = readdir(directory)) != nullptr) {
- std::string next_filename = directory_entry->d_name;
- if (next_filename == "." || next_filename == "..") {
- continue;
- }
-
- std::string path = filename_ + "/" + next_filename;
- std::make_unique<LocalFileOperations>(path)->FindLogs(files);
- }
-
- closedir(directory);
+ return std::make_unique<internal::LocalFileOperations>(filename);
}
bool ConfigOnly(const LogFileHeader *header) {
diff --git a/aos/events/logging/s3_operations.cc b/aos/events/logging/s3_operations.cc
new file mode 100644
index 0000000..01b5dd2
--- /dev/null
+++ b/aos/events/logging/s3_operations.cc
@@ -0,0 +1,20 @@
+#include "aos/events/logging/s3_operations.h"
+
+#include "aos/events/logging/s3_fetcher.h"
+
+namespace aos::logger::internal {
+
+S3FileOperations::S3FileOperations(std::string_view url)
+ : object_urls_(ListS3Objects(url)) {}
+
+void S3FileOperations::FindLogs(std::vector<std::string> *files) {
+ // We already have a recursive listing, so just grab all the objects from
+ // there.
+ for (const std::string &object_url : object_urls_) {
+ if (IsValidFilename(object_url)) {
+ files->push_back(object_url);
+ }
+ }
+}
+
+} // namespace aos::logger::internal
\ No newline at end of file
diff --git a/aos/events/logging/s3_operations.h b/aos/events/logging/s3_operations.h
new file mode 100644
index 0000000..d1bc5b2
--- /dev/null
+++ b/aos/events/logging/s3_operations.h
@@ -0,0 +1,22 @@
+#ifndef AOS_EVENTS_LOGGING_S3_OPERATIONS_H_
+#define AOS_EVENTS_LOGGING_S3_OPERATIONS_H_
+
+#include "aos/events/logging/file_operations.h"
+
+namespace aos::logger::internal {
+
+class S3FileOperations final : public FileOperations {
+ public:
+ explicit S3FileOperations(std::string_view url);
+
+ bool Exists() final { return !object_urls_.empty(); }
+
+ void FindLogs(std::vector<std::string> *files) final;
+
+ private:
+ const std::vector<std::string> object_urls_;
+};
+
+} // namespace aos::logger::internal
+
+#endif // AOS_EVENTS_LOGGING_S3_OPERATIONS_H_