diff --git a/source/common/access_log/access_log_manager_impl.cc b/source/common/access_log/access_log_manager_impl.cc index dd2615421db1..cf5a98daaf47 100644 --- a/source/common/access_log/access_log_manager_impl.cc +++ b/source/common/access_log/access_log_manager_impl.cc @@ -42,9 +42,9 @@ AccessLogFileImpl::AccessLogFileImpl(Filesystem::FilePtr&& file, Event::Dispatch } Filesystem::FlagSet AccessLogFileImpl::defaultFlags() { - static constexpr Filesystem::FlagSet default_flags{ - 1 << Filesystem::File::Operation::Read | 1 << Filesystem::File::Operation::Write | - 1 << Filesystem::File::Operation::Create | 1 << Filesystem::File::Operation::Append}; + static constexpr Filesystem::FlagSet default_flags{1 << Filesystem::File::Operation::Write | + 1 << Filesystem::File::Operation::Create | + 1 << Filesystem::File::Operation::Append}; return default_flags; } diff --git a/test/common/access_log/access_log_manager_impl_test.cc b/test/common/access_log/access_log_manager_impl_test.cc index e7bf863df009..d8d9446d856f 100644 --- a/test/common/access_log/access_log_manager_impl_test.cc +++ b/test/common/access_log/access_log_manager_impl_test.cc @@ -14,6 +14,7 @@ using testing::_; using testing::ByMove; +using testing::Invoke; using testing::NiceMock; using testing::Return; using testing::ReturnNew; @@ -49,14 +50,28 @@ class AccessLogManagerImplTest : public testing::Test { TEST_F(AccessLogManagerImplTest, BadFile) { EXPECT_CALL(dispatcher_, createTimer_(_)); - EXPECT_CALL(*file_, open_()).WillOnce(Return(ByMove(Filesystem::resultFailure(false, 0)))); + EXPECT_CALL(*file_, open_(_)).WillOnce(Return(ByMove(Filesystem::resultFailure(false, 0)))); EXPECT_THROW(access_log_manager_.createAccessLog("foo"), EnvoyException); } +TEST_F(AccessLogManagerImplTest, OpenFileWithRightFlags) { + EXPECT_CALL(dispatcher_, createTimer_(_)); + EXPECT_CALL(*file_, open_(_)) + .WillOnce(Invoke([](Filesystem::FlagSet flags) -> Api::IoCallBoolResult { + EXPECT_FALSE(flags[Filesystem::File::Operation::Read]); + EXPECT_TRUE(flags[Filesystem::File::Operation::Write]); + EXPECT_TRUE(flags[Filesystem::File::Operation::Append]); + EXPECT_TRUE(flags[Filesystem::File::Operation::Create]); + return Filesystem::resultSuccess(true); + })); + EXPECT_NE(nullptr, access_log_manager_.createAccessLog("foo")); + EXPECT_CALL(*file_, close_()).WillOnce(Return(ByMove(Filesystem::resultSuccess(true)))); +} + TEST_F(AccessLogManagerImplTest, flushToLogFilePeriodically) { NiceMock* timer = new NiceMock(&dispatcher_); - EXPECT_CALL(*file_, open_()).WillOnce(Return(ByMove(Filesystem::resultSuccess(true)))); + EXPECT_CALL(*file_, open_(_)).WillOnce(Return(ByMove(Filesystem::resultSuccess(true)))); AccessLogFileSharedPtr log_file = access_log_manager_.createAccessLog("foo"); EXPECT_CALL(*timer, enableTimer(timeout_40ms_, _)); @@ -98,7 +113,7 @@ TEST_F(AccessLogManagerImplTest, flushToLogFilePeriodically) { TEST_F(AccessLogManagerImplTest, flushToLogFileOnDemand) { NiceMock* timer = new NiceMock(&dispatcher_); - EXPECT_CALL(*file_, open_()).WillOnce(Return(ByMove(Filesystem::resultSuccess(true)))); + EXPECT_CALL(*file_, open_(_)).WillOnce(Return(ByMove(Filesystem::resultSuccess(true)))); AccessLogFileSharedPtr log_file = access_log_manager_.createAccessLog("foo"); EXPECT_CALL(*timer, enableTimer(timeout_40ms_, _)); @@ -163,7 +178,7 @@ TEST_F(AccessLogManagerImplTest, reopenFile) { NiceMock* timer = new NiceMock(&dispatcher_); Sequence sq; - EXPECT_CALL(*file_, open_()) + EXPECT_CALL(*file_, open_(_)) .InSequence(sq) .WillOnce(Return(ByMove(Filesystem::resultSuccess(true)))); AccessLogFileSharedPtr log_file = access_log_manager_.createAccessLog("foo"); @@ -188,7 +203,7 @@ TEST_F(AccessLogManagerImplTest, reopenFile) { EXPECT_CALL(*file_, close_()) .InSequence(sq) .WillOnce(Return(ByMove(Filesystem::resultSuccess(true)))); - EXPECT_CALL(*file_, open_()) + EXPECT_CALL(*file_, open_(_)) .InSequence(sq) .WillOnce(Return(ByMove(Filesystem::resultSuccess(true)))); @@ -224,7 +239,7 @@ TEST_F(AccessLogManagerImplTest, reopenThrows) { })); Sequence sq; - EXPECT_CALL(*file_, open_()) + EXPECT_CALL(*file_, open_(_)) .InSequence(sq) .WillOnce(Return(ByMove(Filesystem::resultSuccess(true)))); @@ -232,7 +247,7 @@ TEST_F(AccessLogManagerImplTest, reopenThrows) { EXPECT_CALL(*file_, close_()) .InSequence(sq) .WillOnce(Return(ByMove(Filesystem::resultSuccess(true)))); - EXPECT_CALL(*file_, open_()) + EXPECT_CALL(*file_, open_(_)) .InSequence(sq) .WillOnce(Return(ByMove(Filesystem::resultFailure(false, 0)))); @@ -262,7 +277,7 @@ TEST_F(AccessLogManagerImplTest, reopenThrows) { } TEST_F(AccessLogManagerImplTest, bigDataChunkShouldBeFlushedWithoutTimer) { - EXPECT_CALL(*file_, open_()).WillOnce(Return(ByMove(Filesystem::resultSuccess(true)))); + EXPECT_CALL(*file_, open_(_)).WillOnce(Return(ByMove(Filesystem::resultSuccess(true)))); AccessLogFileSharedPtr log_file = access_log_manager_.createAccessLog("foo"); EXPECT_CALL(*file_, write_(_)) @@ -305,7 +320,7 @@ TEST_F(AccessLogManagerImplTest, reopenAllFiles) { EXPECT_CALL(dispatcher_, createTimer_(_)).WillRepeatedly(ReturnNew>()); Sequence sq; - EXPECT_CALL(*file_, open_()) + EXPECT_CALL(*file_, open_(_)) .InSequence(sq) .WillOnce(Return(ByMove(Filesystem::resultSuccess(true)))); AccessLogFileSharedPtr log = access_log_manager_.createAccessLog("foo"); @@ -315,7 +330,7 @@ TEST_F(AccessLogManagerImplTest, reopenAllFiles) { .WillOnce(Return(ByMove(std::unique_ptr>(file2)))); Sequence sq2; - EXPECT_CALL(*file2, open_()) + EXPECT_CALL(*file2, open_(_)) .InSequence(sq2) .WillOnce(Return(ByMove(Filesystem::resultSuccess(true)))); AccessLogFileSharedPtr log2 = access_log_manager_.createAccessLog("bar"); @@ -342,10 +357,10 @@ TEST_F(AccessLogManagerImplTest, reopenAllFiles) { .InSequence(sq2) .WillOnce(Return(ByMove(Filesystem::resultSuccess(true)))); - EXPECT_CALL(*file_, open_()) + EXPECT_CALL(*file_, open_(_)) .InSequence(sq) .WillOnce(Return(ByMove(Filesystem::resultSuccess(true)))); - EXPECT_CALL(*file2, open_()) + EXPECT_CALL(*file2, open_(_)) .InSequence(sq2) .WillOnce(Return(ByMove(Filesystem::resultSuccess(true)))); diff --git a/test/extensions/access_loggers/file/BUILD b/test/extensions/access_loggers/file/BUILD index 3feb9c7509a6..461c4d5bdaea 100644 --- a/test/extensions/access_loggers/file/BUILD +++ b/test/extensions/access_loggers/file/BUILD @@ -18,5 +18,6 @@ envoy_extension_cc_test( deps = [ "//source/extensions/access_loggers/file:config", "//test/mocks/server:server_mocks", + "//test/test_common:environment_lib", ], ) diff --git a/test/mocks/filesystem/mocks.cc b/test/mocks/filesystem/mocks.cc index 7cf733315030..af5d9fcddc7c 100644 --- a/test/mocks/filesystem/mocks.cc +++ b/test/mocks/filesystem/mocks.cc @@ -9,10 +9,10 @@ namespace Filesystem { MockFile::MockFile() : num_opens_(0), num_writes_(0), is_open_(false) {} MockFile::~MockFile() = default; -Api::IoCallBoolResult MockFile::open(FlagSet) { +Api::IoCallBoolResult MockFile::open(FlagSet flag) { Thread::LockGuard lock(open_mutex_); - Api::IoCallBoolResult result = open_(); + Api::IoCallBoolResult result = open_(flag); is_open_ = result.rc_; num_opens_++; open_event_.notifyOne(); diff --git a/test/mocks/filesystem/mocks.h b/test/mocks/filesystem/mocks.h index 459cdfd8ce65..c4ae006a83ad 100644 --- a/test/mocks/filesystem/mocks.h +++ b/test/mocks/filesystem/mocks.h @@ -25,7 +25,8 @@ class MockFile : public File { bool isOpen() const override { return is_open_; }; MOCK_CONST_METHOD0(path, std::string()); - MOCK_METHOD0(open_, Api::IoCallBoolResult()); + // The first parameter here must be `const FlagSet&` otherwise it doesn't compile with libstdc++ + MOCK_METHOD1(open_, Api::IoCallBoolResult(const FlagSet& flag)); MOCK_METHOD1(write_, Api::IoCallSizeResult(absl::string_view buffer)); MOCK_METHOD0(close_, Api::IoCallBoolResult());