New class FileRotatingStreamReader

When landed, the FileRotatingStream class can be made write-only.

Bug: webrtc:7811
Change-Id: I6dcd2a869301b9b8273b48d47df51a1065767ffd
Reviewed-on: https://webrtc-review.googlesource.com/c/115302
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Sami Kalliomäki <sakal@webrtc.org>
Reviewed-by: Kári Helgason <kthelgason@webrtc.org>
Commit-Queue: Niels Moller <nisse@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#26126}

rtc_base/filerotatingstream.cc

@@ -36,6 +36,8 @@ namespace rtc {
 
 namespace {
 
+const char kCallSessionLogPrefix[] = "webrtc_log";
+
 std::string AddTrailingPathDelimiterIfNeeded(std::string directory);
 
 // |dir| must have a trailing delimiter. |prefix| must not include wild card
@@ -458,7 +460,7 @@ std::string FileRotatingStream::GetFilePath(size_t index,
 
 CallSessionFileRotatingStream::CallSessionFileRotatingStream(
     const std::string& dir_path)
-    : FileRotatingStream(dir_path, kLogPrefix),
+    : FileRotatingStream(dir_path, kCallSessionLogPrefix),
       max_total_log_size_(0),
       num_rotations_(0) {}
 
@@ -466,7 +468,7 @@ CallSessionFileRotatingStream::CallSessionFileRotatingStream(
     const std::string& dir_path,
     size_t max_total_log_size)
     : FileRotatingStream(dir_path,
-                         kLogPrefix,
+                         kCallSessionLogPrefix,
                          max_total_log_size / 2,
                          GetNumRotatingLogFiles(max_total_log_size) + 1),
       max_total_log_size_(max_total_log_size),
@@ -474,7 +476,6 @@ CallSessionFileRotatingStream::CallSessionFileRotatingStream(
   RTC_DCHECK_GE(max_total_log_size, 4);
 }
 
-const char* CallSessionFileRotatingStream::kLogPrefix = "webrtc_log";
 const size_t CallSessionFileRotatingStream::kRotatingLogFileDefaultSize =
     1024 * 1024;
 
@@ -508,4 +509,47 @@ size_t CallSessionFileRotatingStream::GetNumRotatingLogFiles(
                   (max_total_log_size / 2) / kRotatingLogFileDefaultSize);
 }
 
+FileRotatingStreamReader::FileRotatingStreamReader(
+    const std::string& dir_path,
+    const std::string& file_prefix) {
+  file_names_ = GetFilesWithPrefix(AddTrailingPathDelimiterIfNeeded(dir_path),
+                                   file_prefix);
+
+  // Plain sort of the file names would sort by age, i.e., oldest last. Using
+  // std::greater gives us the desired chronological older, oldest first.
+  std::sort(file_names_.begin(), file_names_.end(),
+            std::greater<std::string>());
+}
+
+FileRotatingStreamReader::~FileRotatingStreamReader() = default;
+
+size_t FileRotatingStreamReader::GetSize() const {
+  size_t total_size = 0;
+  for (const auto& file_name : file_names_) {
+    total_size += GetFileSize(file_name).value_or(0);
+  }
+  return total_size;
+}
+
+size_t FileRotatingStreamReader::ReadAll(void* buffer, size_t size) const {
+  size_t done = 0;
+  for (const auto& file_name : file_names_) {
+    if (done < size) {
+      FILE* f = fopen(file_name.c_str(), "rb");
+      if (!f) {
+        break;
+      }
+      done += fread(static_cast<char*>(buffer) + done, 1, size - done, f);
+      fclose(f);
+    } else {
+      break;
+    }
+  }
+  return done;
+}
+
+CallSessionFileRotatingStreamReader::CallSessionFileRotatingStreamReader(
+    const std::string& dir_path)
+    : FileRotatingStreamReader(dir_path, kCallSessionLogPrefix) {}
+
 }  // namespace rtc

rtc_base/filerotatingstream.h

@@ -158,7 +158,6 @@ class CallSessionFileRotatingStream : public FileRotatingStream {
  private:
   static size_t GetRotatingLogSize(size_t max_total_log_size);
   static size_t GetNumRotatingLogFiles(size_t max_total_log_size);
-  static const char* kLogPrefix;
   static const size_t kRotatingLogFileDefaultSize;
 
   const size_t max_total_log_size_;
@@ -167,6 +166,27 @@ class CallSessionFileRotatingStream : public FileRotatingStream {
   RTC_DISALLOW_COPY_AND_ASSIGN(CallSessionFileRotatingStream);
 };
 
+// This is a convenience class, to read all files produced by a
+// FileRotatingStream, all in one go. Typical use calls GetSize and ReadData
+// only once. The list of file names to read is based on the contents of the log
+// directory at construction time.
+class FileRotatingStreamReader {
+ public:
+  FileRotatingStreamReader(const std::string& dir_path,
+                           const std::string& file_prefix);
+  ~FileRotatingStreamReader();
+  size_t GetSize() const;
+  size_t ReadAll(void* buffer, size_t size) const;
+
+ private:
+  std::vector<std::string> file_names_;
+};
+
+class CallSessionFileRotatingStreamReader : public FileRotatingStreamReader {
+ public:
+  CallSessionFileRotatingStreamReader(const std::string& dir_path);
+};
+
 }  // namespace rtc
 
 #endif  // RTC_BASE_FILEROTATINGSTREAM_H_

rtc_base/filerotatingstream_unittest.cc

@@ -80,6 +80,8 @@ class MAYBE_FileRotatingStreamTest : public ::testing::Test {
                         const size_t expected_length,
                         const std::string& dir_path,
                         const char* file_prefix) {
+    // TODO(nisse): Delete the part of this method using read-mode
+    // FileRotatingStream, together with support for read-mode.
     std::unique_ptr<FileRotatingStream> stream;
     stream.reset(new FileRotatingStream(dir_path, file_prefix));
     ASSERT_TRUE(stream->Open());
@@ -92,6 +94,13 @@ class MAYBE_FileRotatingStreamTest : public ::testing::Test {
     EXPECT_EQ(0, memcmp(expected_contents, buffer.get(), expected_length));
     EXPECT_EQ(SR_EOS, stream->ReadAll(buffer.get(), 1, nullptr, nullptr));
     EXPECT_EQ(stream_size, read);
+
+    // Test also with the FileRotatingStreamReader class.
+    FileRotatingStreamReader reader(dir_path, file_prefix);
+    EXPECT_EQ(reader.GetSize(), expected_length);
+    memset(buffer.get(), 0, expected_length);
+    EXPECT_EQ(expected_length, reader.ReadAll(buffer.get(), expected_length));
+    EXPECT_EQ(0, memcmp(expected_contents, buffer.get(), expected_length));
   }
 
   void VerifyFileContents(const char* expected_contents,
@@ -293,6 +302,8 @@ class MAYBE_CallSessionFileRotatingStreamTest : public ::testing::Test {
   void VerifyStreamRead(const char* expected_contents,
                         const size_t expected_length,
                         const std::string& dir_path) {
+    // TODO(nisse): Delete the part of this method using read-mode
+    // CallSessionFileRotatingStream, together with support for read-mode.
     std::unique_ptr<CallSessionFileRotatingStream> stream(
         new CallSessionFileRotatingStream(dir_path));
     ASSERT_TRUE(stream->Open());
@@ -305,6 +316,13 @@ class MAYBE_CallSessionFileRotatingStreamTest : public ::testing::Test {
     EXPECT_EQ(0, memcmp(expected_contents, buffer.get(), expected_length));
     EXPECT_EQ(SR_EOS, stream->ReadAll(buffer.get(), 1, nullptr, nullptr));
     EXPECT_EQ(stream_size, read);
+
+    // Test also with the CallSessionFileRotatingStreamReader class.
+    CallSessionFileRotatingStreamReader reader(dir_path);
+    EXPECT_EQ(reader.GetSize(), expected_length);
+    memset(buffer.get(), 0, expected_length);
+    EXPECT_EQ(expected_length, reader.ReadAll(buffer.get(), expected_length));
+    EXPECT_EQ(0, memcmp(expected_contents, buffer.get(), expected_length));
   }
 
   std::unique_ptr<CallSessionFileRotatingStream> stream_;

sdk/android/src/jni/pc/callsessionfilerotatinglogsink.cc

@@ -50,23 +50,17 @@ JNI_CallSessionFileRotatingLogSink_GetLogData(
     JNIEnv* jni,
     const JavaParamRef<jstring>& j_dirPath) {
   std::string dir_path = JavaToStdString(jni, j_dirPath);
-  std::unique_ptr<rtc::CallSessionFileRotatingStream> stream(
-      new rtc::CallSessionFileRotatingStream(dir_path));
-  if (!stream->Open()) {
-    RTC_LOG_V(rtc::LoggingSeverity::LS_WARNING)
-        << "Failed to open CallSessionFileRotatingStream for path " << dir_path;
-    return ScopedJavaLocalRef<jbyteArray>(jni, jni->NewByteArray(0));
-  }
-  size_t log_size = 0;
-  if (!stream->GetSize(&log_size) || log_size == 0) {
+  rtc::CallSessionFileRotatingStreamReader file_reader(dir_path);
+  size_t log_size = file_reader.GetSize();
+  if (log_size == 0) {
     RTC_LOG_V(rtc::LoggingSeverity::LS_WARNING)
         << "CallSessionFileRotatingStream returns 0 size for path " << dir_path;
     return ScopedJavaLocalRef<jbyteArray>(jni, jni->NewByteArray(0));
   }
 
-  size_t read = 0;
+  // TODO(nisse, sakal): To avoid copying, change api to use ByteBuffer.
   std::unique_ptr<jbyte> buffer(static_cast<jbyte*>(malloc(log_size)));
-  stream->ReadAll(buffer.get(), log_size, &read, nullptr);
+  size_t read = file_reader.ReadAll(buffer.get(), log_size);
 
   ScopedJavaLocalRef<jbyteArray> result =
       ScopedJavaLocalRef<jbyteArray>(jni, jni->NewByteArray(read));

sdk/objc/api/peerconnection/RTCFileLogger.mm

@@ -129,29 +129,24 @@ const char *kRTCFileLoggerRotatingLogPrefix = "rotating_log";
     return nil;
   }
   NSMutableData* logData = [NSMutableData data];
-  std::unique_ptr<rtc::FileRotatingStream> stream;
+  std::unique_ptr<rtc::FileRotatingStreamReader> stream;
   switch(_rotationType) {
     case RTCFileLoggerTypeApp:
-      stream.reset(
-          new rtc::FileRotatingStream(_dirPath.UTF8String,
-                                      kRTCFileLoggerRotatingLogPrefix));
+      stream = absl::make_unique<rtc::FileRotatingStreamReader>(_dirPath.UTF8String,
+                                                                kRTCFileLoggerRotatingLogPrefix);
       break;
     case RTCFileLoggerTypeCall:
-      stream.reset(new rtc::CallSessionFileRotatingStream(_dirPath.UTF8String));
+      stream = absl::make_unique<rtc::CallSessionFileRotatingStreamReader>(_dirPath.UTF8String);
       break;
   }
-  if (!stream->Open()) {
+  size_t bufferSize = stream->GetSize();
+  if (bufferSize == 0) {
     return logData;
   }
-  size_t bufferSize = 0;
-  if (!stream->GetSize(&bufferSize) || bufferSize == 0) {
-    return logData;
-  }
-  size_t read = 0;
   // Allocate memory using malloc so we can pass it direcly to NSData without
   // copying.
   std::unique_ptr<uint8_t[]> buffer(static_cast<uint8_t*>(malloc(bufferSize)));
-  stream->ReadAll(buffer.get(), bufferSize, &read, nullptr);
+  size_t read = stream->ReadAll(buffer.get(), bufferSize);
   logData = [[NSMutableData alloc] initWithBytesNoCopy:buffer.release()
                                                 length:read];
   return logData;