Make FileRotatingStream independent of StreamInterface

Bug: webrtc:7811
Change-Id: Ia5c07ad00e90d5b982750004eeb2c8e1cfbae4eb
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/212969
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Commit-Queue: Niels Moller <nisse@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33579}
This commit is contained in:
Niels Möller 2021-03-25 15:29:02 +01:00 committed by Commit Bot
parent a4b2c2b207
commit a9311b6761
4 changed files with 48 additions and 72 deletions

View file

@ -193,49 +193,40 @@ FileRotatingStream::FileRotatingStream(const std::string& dir_path,
FileRotatingStream::~FileRotatingStream() {} FileRotatingStream::~FileRotatingStream() {}
StreamState FileRotatingStream::GetState() const { bool FileRotatingStream::IsOpen() const {
return (file_.is_open() ? SS_OPEN : SS_CLOSED); return file_.is_open();
} }
StreamResult FileRotatingStream::Read(void* buffer, bool FileRotatingStream::Write(const void* data, size_t data_len) {
size_t buffer_len,
size_t* read,
int* error) {
RTC_DCHECK(buffer);
RTC_NOTREACHED();
return SR_EOS;
}
StreamResult FileRotatingStream::Write(const void* data,
size_t data_len,
size_t* written,
int* error) {
if (!file_.is_open()) { if (!file_.is_open()) {
std::fprintf(stderr, "Open() must be called before Write.\n"); std::fprintf(stderr, "Open() must be called before Write.\n");
return SR_ERROR; return false;
} }
// Write as much as will fit in to the current file. while (data_len > 0) {
RTC_DCHECK_LT(current_bytes_written_, max_file_size_); // Write as much as will fit in to the current file.
size_t remaining_bytes = max_file_size_ - current_bytes_written_; RTC_DCHECK_LT(current_bytes_written_, max_file_size_);
size_t write_length = std::min(data_len, remaining_bytes); size_t remaining_bytes = max_file_size_ - current_bytes_written_;
size_t write_length = std::min(data_len, remaining_bytes);
if (!file_.Write(data, write_length)) { if (!file_.Write(data, write_length)) {
return SR_ERROR; return false;
} }
if (disable_buffering_ && !file_.Flush()) { if (disable_buffering_ && !file_.Flush()) {
return SR_ERROR; return false;
} }
current_bytes_written_ += write_length; current_bytes_written_ += write_length;
if (written) {
*written = write_length; // If we're done with this file, rotate it out.
if (current_bytes_written_ >= max_file_size_) {
RTC_DCHECK_EQ(current_bytes_written_, max_file_size_);
RotateFiles();
}
data_len -= write_length;
data =
static_cast<const void*>(static_cast<const char*>(data) + write_length);
} }
// If we're done with this file, rotate it out. return true;
if (current_bytes_written_ >= max_file_size_) {
RTC_DCHECK_EQ(current_bytes_written_, max_file_size_);
RotateFiles();
}
return SR_SUCCESS;
} }
bool FileRotatingStream::Flush() { bool FileRotatingStream::Flush() {

View file

@ -27,13 +27,8 @@ namespace rtc {
// constructor. It rotates the files once the current file is full. The // constructor. It rotates the files once the current file is full. The
// individual file size and the number of files used is configurable in the // individual file size and the number of files used is configurable in the
// constructor. Open() must be called before using this stream. // constructor. Open() must be called before using this stream.
class FileRotatingStream : public StreamInterface { class FileRotatingStream {
public: public:
// Use this constructor for reading a directory previously written to with
// this stream.
FileRotatingStream(const std::string& dir_path,
const std::string& file_prefix);
// Use this constructor for writing to a directory. Files in the directory // Use this constructor for writing to a directory. Files in the directory
// matching the prefix will be deleted on open. // matching the prefix will be deleted on open.
FileRotatingStream(const std::string& dir_path, FileRotatingStream(const std::string& dir_path,
@ -41,20 +36,13 @@ class FileRotatingStream : public StreamInterface {
size_t max_file_size, size_t max_file_size,
size_t num_files); size_t num_files);
~FileRotatingStream() override; virtual ~FileRotatingStream();
// StreamInterface methods. bool IsOpen() const;
StreamState GetState() const override;
StreamResult Read(void* buffer, bool Write(const void* data, size_t data_len);
size_t buffer_len, bool Flush();
size_t* read, void Close();
int* error) override;
StreamResult Write(const void* data,
size_t data_len,
size_t* written,
int* error) override;
bool Flush() override;
void Close() override;
// Opens the appropriate file(s). Call this before using the stream. // Opens the appropriate file(s). Call this before using the stream.
bool Open(); bool Open();
@ -63,6 +51,8 @@ class FileRotatingStream : public StreamInterface {
// enabled by default for performance. // enabled by default for performance.
bool DisableBuffering(); bool DisableBuffering();
// Below two methods are public for testing only.
// Returns the path used for the i-th newest file, where the 0th file is the // Returns the path used for the i-th newest file, where the 0th file is the
// newest file. The file may or may not exist, this is just used for // newest file. The file may or may not exist, this is just used for
// formatting. Index must be less than GetNumFiles(). // formatting. Index must be less than GetNumFiles().
@ -72,8 +62,6 @@ class FileRotatingStream : public StreamInterface {
size_t GetNumFiles() const { return file_names_.size(); } size_t GetNumFiles() const { return file_names_.size(); }
protected: protected:
size_t GetMaxFileSize() const { return max_file_size_; }
void SetMaxFileSize(size_t size) { max_file_size_ = size; } void SetMaxFileSize(size_t size) { max_file_size_ = size; }
size_t GetRotationIndex() const { return rotation_index_; } size_t GetRotationIndex() const { return rotation_index_; }

View file

@ -72,7 +72,7 @@ class MAYBE_FileRotatingStreamTest : public ::testing::Test {
// Writes the data to the stream and flushes it. // Writes the data to the stream and flushes it.
void WriteAndFlush(const void* data, const size_t data_len) { void WriteAndFlush(const void* data, const size_t data_len) {
EXPECT_EQ(SR_SUCCESS, stream_->WriteAll(data, data_len, nullptr, nullptr)); EXPECT_EQ(SR_SUCCESS, stream_->Write(data, data_len));
EXPECT_TRUE(stream_->Flush()); EXPECT_TRUE(stream_->Flush());
} }
@ -114,11 +114,11 @@ const size_t MAYBE_FileRotatingStreamTest::kMaxFileSize = 2;
TEST_F(MAYBE_FileRotatingStreamTest, State) { TEST_F(MAYBE_FileRotatingStreamTest, State) {
Init("FileRotatingStreamTestState", kFilePrefix, kMaxFileSize, 3); Init("FileRotatingStreamTestState", kFilePrefix, kMaxFileSize, 3);
EXPECT_EQ(SS_CLOSED, stream_->GetState()); EXPECT_FALSE(stream_->IsOpen());
ASSERT_TRUE(stream_->Open()); ASSERT_TRUE(stream_->Open());
EXPECT_EQ(SS_OPEN, stream_->GetState()); EXPECT_TRUE(stream_->IsOpen());
stream_->Close(); stream_->Close();
EXPECT_EQ(SS_CLOSED, stream_->GetState()); EXPECT_FALSE(stream_->IsOpen());
} }
// Tests that nothing is written to file when data of length zero is written. // Tests that nothing is written to file when data of length zero is written.
@ -277,7 +277,7 @@ class MAYBE_CallSessionFileRotatingStreamTest : public ::testing::Test {
// Writes the data to the stream and flushes it. // Writes the data to the stream and flushes it.
void WriteAndFlush(const void* data, const size_t data_len) { void WriteAndFlush(const void* data, const size_t data_len) {
EXPECT_EQ(SR_SUCCESS, stream_->WriteAll(data, data_len, nullptr, nullptr)); EXPECT_TRUE(stream_->Write(data, data_len));
EXPECT_TRUE(stream_->Flush()); EXPECT_TRUE(stream_->Flush());
} }
@ -334,8 +334,7 @@ TEST_F(MAYBE_CallSessionFileRotatingStreamTest, WriteAndReadLarge) {
std::unique_ptr<uint8_t[]> buffer(new uint8_t[buffer_size]); std::unique_ptr<uint8_t[]> buffer(new uint8_t[buffer_size]);
for (int i = 0; i < 8; i++) { for (int i = 0; i < 8; i++) {
memset(buffer.get(), i, buffer_size); memset(buffer.get(), i, buffer_size);
EXPECT_EQ(SR_SUCCESS, EXPECT_EQ(SR_SUCCESS, stream_->Write(buffer.get(), buffer_size));
stream_->WriteAll(buffer.get(), buffer_size, nullptr, nullptr));
} }
const int expected_vals[] = {0, 1, 2, 6, 7}; const int expected_vals[] = {0, 1, 2, 6, 7};
@ -369,8 +368,7 @@ TEST_F(MAYBE_CallSessionFileRotatingStreamTest, WriteAndReadFirstHalf) {
std::unique_ptr<uint8_t[]> buffer(new uint8_t[buffer_size]); std::unique_ptr<uint8_t[]> buffer(new uint8_t[buffer_size]);
for (int i = 0; i < 2; i++) { for (int i = 0; i < 2; i++) {
memset(buffer.get(), i, buffer_size); memset(buffer.get(), i, buffer_size);
EXPECT_EQ(SR_SUCCESS, EXPECT_EQ(SR_SUCCESS, stream_->Write(buffer.get(), buffer_size));
stream_->WriteAll(buffer.get(), buffer_size, nullptr, nullptr));
} }
const int expected_vals[] = {0, 1}; const int expected_vals[] = {0, 1};

View file

@ -16,7 +16,6 @@
#include <string> #include <string>
#include "rtc_base/checks.h" #include "rtc_base/checks.h"
#include "rtc_base/stream.h"
namespace rtc { namespace rtc {
@ -37,23 +36,23 @@ FileRotatingLogSink::FileRotatingLogSink(FileRotatingStream* stream)
FileRotatingLogSink::~FileRotatingLogSink() {} FileRotatingLogSink::~FileRotatingLogSink() {}
void FileRotatingLogSink::OnLogMessage(const std::string& message) { void FileRotatingLogSink::OnLogMessage(const std::string& message) {
if (stream_->GetState() != SS_OPEN) { if (!stream_->IsOpen()) {
std::fprintf(stderr, "Init() must be called before adding this sink.\n"); std::fprintf(stderr, "Init() must be called before adding this sink.\n");
return; return;
} }
stream_->WriteAll(message.c_str(), message.size(), nullptr, nullptr); stream_->Write(message.c_str(), message.size());
} }
void FileRotatingLogSink::OnLogMessage(const std::string& message, void FileRotatingLogSink::OnLogMessage(const std::string& message,
LoggingSeverity sev, LoggingSeverity sev,
const char* tag) { const char* tag) {
if (stream_->GetState() != SS_OPEN) { if (!stream_->IsOpen()) {
std::fprintf(stderr, "Init() must be called before adding this sink.\n"); std::fprintf(stderr, "Init() must be called before adding this sink.\n");
return; return;
} }
stream_->WriteAll(tag, strlen(tag), nullptr, nullptr); stream_->Write(tag, strlen(tag));
stream_->WriteAll(": ", 2, nullptr, nullptr); stream_->Write(": ", 2);
stream_->WriteAll(message.c_str(), message.size(), nullptr, nullptr); stream_->Write(message.c_str(), message.size());
} }
bool FileRotatingLogSink::Init() { bool FileRotatingLogSink::Init() {