-
Notifications
You must be signed in to change notification settings - Fork 5.3k
Add an unused implementation NoOpTransportSocketCallbacks #4181
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
bf1cee8
Add getter for callbacks in TransportSocket which is set via
danzh1989 dffd2d1
format again
danzh1989 d1a9c25
add override
danzh1989 a85ec49
add back BUILD file
danzh1989 b8605bd
add override
danzh1989 41f9978
fix connection_impl_test
danzh1989 d97d11f
remove socket pointer
danzh1989 dd1f97a
format
danzh1989 2bb3bbe
mock callbacks()
danzh1989 d2755dc
remove callback from interface
danzh1989 de8caf3
add back constructor
danzh1989 f0aeb4a
use reference
danzh1989 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
35 changes: 35 additions & 0 deletions
35
source/extensions/transport_sockets/alts/noop_transport_socket_callbacks.h
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| #include "envoy/network/transport_socket.h" | ||
|
|
||
| namespace Envoy { | ||
| namespace Extensions { | ||
| namespace TransportSockets { | ||
| namespace Alts { | ||
|
|
||
| /** | ||
| * A TransportSocketCallbacks for wrapped TransportSocket object. Some | ||
| * TransportSocket implementation wraps another socket which does actual I/O. | ||
| * This class is used by the wrapped socket as its callbacks instead of the real | ||
| * connection to hold back callbacks from the underlying socket to connection. | ||
| */ | ||
| class NoOpTransportSocketCallbacks : public Network::TransportSocketCallbacks { | ||
| public: | ||
| explicit NoOpTransportSocketCallbacks(Network::TransportSocketCallbacks& parent) | ||
| : parent_(parent) {} | ||
|
|
||
| int fd() const override { return parent_.fd(); } | ||
| Network::Connection& connection() override { return parent_.connection(); } | ||
| bool shouldDrainReadBuffer() override { return false; } | ||
| /* | ||
| * No-op for these two methods to hold back the callbacks. | ||
| */ | ||
| void setReadBufferReady() override {} | ||
| void raiseEvent(Network::ConnectionEvent) override {} | ||
|
|
||
| private: | ||
| Network::TransportSocketCallbacks& parent_; | ||
| }; | ||
|
|
||
| } // namespace Alts | ||
| } // namespace TransportSockets | ||
| } // namespace Extensions | ||
| } // namespace Envoy | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
58 changes: 58 additions & 0 deletions
58
test/extensions/transport_sockets/alts/noop_transport_socket_callbacks_test.cc
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,58 @@ | ||
| #include "extensions/transport_sockets/alts/noop_transport_socket_callbacks.h" | ||
|
|
||
| #include "test/mocks/network/mocks.h" | ||
|
|
||
| #include "gtest/gtest.h" | ||
|
|
||
| namespace Envoy { | ||
| namespace Extensions { | ||
| namespace TransportSockets { | ||
| namespace Alts { | ||
| namespace { | ||
|
|
||
| class TestTransportSocketCallbacks : public Network::TransportSocketCallbacks { | ||
| public: | ||
| explicit TestTransportSocketCallbacks(Network::Connection& connection) | ||
| : connection_(connection) {} | ||
|
|
||
| int fd() const override { return 1; } | ||
| Network::Connection& connection() override { return connection_; } | ||
| bool shouldDrainReadBuffer() override { return false; } | ||
| void setReadBufferReady() override { set_read_buffer_ready_ = true; } | ||
| void raiseEvent(Network::ConnectionEvent) override { event_raised_ = true; } | ||
|
|
||
| bool event_raised() const { return event_raised_; } | ||
| bool set_read_buffer_ready() const { return set_read_buffer_ready_; } | ||
|
|
||
| private: | ||
| bool event_raised_{false}; | ||
| bool set_read_buffer_ready_{false}; | ||
| Network::Connection& connection_; | ||
| }; | ||
|
|
||
| class NoOpTransportSocketCallbacksTest : public testing::Test { | ||
| protected: | ||
| NoOpTransportSocketCallbacksTest() | ||
| : wrapper_callbacks_(connection_), wrapped_callbacks_(wrapper_callbacks_) {} | ||
|
|
||
| Network::MockConnection connection_; | ||
| TestTransportSocketCallbacks wrapper_callbacks_; | ||
| NoOpTransportSocketCallbacks wrapped_callbacks_; | ||
| }; | ||
|
|
||
| TEST_F(NoOpTransportSocketCallbacksTest, TestAllCallbacks) { | ||
| EXPECT_EQ(wrapper_callbacks_.fd(), wrapped_callbacks_.fd()); | ||
| EXPECT_EQ(&connection_, &wrapped_callbacks_.connection()); | ||
| EXPECT_FALSE(wrapped_callbacks_.shouldDrainReadBuffer()); | ||
|
|
||
| wrapped_callbacks_.setReadBufferReady(); | ||
| EXPECT_FALSE(wrapper_callbacks_.set_read_buffer_ready()); | ||
| wrapped_callbacks_.raiseEvent(Network::ConnectionEvent::Connected); | ||
| EXPECT_FALSE(wrapper_callbacks_.event_raised()); | ||
| } | ||
|
|
||
| } // namespace | ||
| } // namespace Alts | ||
| } // namespace TransportSockets | ||
| } // namespace Extensions | ||
| } // namespace Envoy |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm fine with this landing as is, but can you please loop me in when it is used? I have some concerns about these particular events being swallowed. Maybe we can add ASSERTs that they're not used so we'll at least catch it in debug builds if a mistake is made? Hopefully seeing where the code is used will allay my concerns :-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK'ed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@alyssawilk I will merge master and use this in my PR #4153, ASSERT cannot be added here. In #4153, it have to swallow
Connectedevent from RawBufferSocket, and raise it when handshake is done.