Add an unused implementation NoOpTransportSocketCallbacks#4181
Add an unused implementation NoOpTransportSocketCallbacks#4181alyssawilk merged 12 commits intoenvoyproxy:masterfrom
Conversation
setTransportSocketCallbacks(). Add a new class NoOpTransportSocketCallbacks. Add a utility class NoOpTransportSocketCallbacks to hold back callbacks from underlying socket in a TransportSocket implementation which wraps another socket. move files around Signed-off-by: Dan Zhang <danzh@google.com> add some implementation to mock class Signed-off-by: Dan Zhang <danzh@google.com> format Signed-off-by: Dan Zhang <danzh@google.com> header order Signed-off-by: Dan Zhang <danzh@google.com> format Signed-off-by: Dan Zhang <danzh@google.com>
Signed-off-by: Dan Zhang <danzh@google.com>
Signed-off-by: Dan Zhang <danzh@google.com>
Signed-off-by: Dan Zhang <danzh@google.com>
Signed-off-by: Dan Zhang <danzh@google.com>
Signed-off-by: Dan Zhang <danzh@google.com>
| */ | ||
| class NoOpTransportSocketCallbacks : public Network::TransportSocketCallbacks { | ||
| public: | ||
| explicit NoOpTransportSocketCallbacks(Network::TransportSocket* parent) : parent_(parent) {} |
There was a problem hiding this comment.
Yeah if you delay creating TransportSocketCallbacks, i.e. in parent setTransportSocketCallbacks, you should be able to just take TransportSocketCallbacks& here.
There was a problem hiding this comment.
That's a good point! Done.
I still keep the callbacks() in TransportSocket interface, as it's convenient to access to callbacks_ for underlying socket.
Signed-off-by: Dan Zhang <danzh@google.com>
test/mocks/network/mocks.h
Outdated
| ~MockTransportSocket(); | ||
|
|
||
| MOCK_METHOD1(setTransportSocketCallbacks, void(TransportSocketCallbacks& callbacks)); | ||
| void setTransportSocketCallbacks(TransportSocketCallbacks& callbacks) override { |
There was a problem hiding this comment.
Keep this (and callbacks below) as a MOCK and do ON_CALL(...).WillByDefault in constructor like:
https://github.com/envoyproxy/envoy/pull/4153/files#diff-f1d609046babf8223367556fbdda1721R206
This will allow test to do EXPECT_CALL etc.
| /** | ||
| * @return the TransportSocketCallbacks object passed in through setTransportSocketCallbacks(). | ||
| */ | ||
| virtual TransportSocketCallbacks* callbacks() PURE; |
There was a problem hiding this comment.
I would remove this as I think where you need get callbacks is in the scope you set it. No strong opinion on it though. If this is needed internally, ping me on hangout.
Signed-off-by: Dan Zhang <danzh@google.com>
Signed-off-by: Dan Zhang <danzh@google.com>
Signed-off-by: Dan Zhang <danzh@google.com>
|
Ping? |
|
|
||
| class TestTransportSocketCallbacks : public Network::TransportSocketCallbacks { | ||
| public: | ||
| TestTransportSocketCallbacks(Network::Connection* connection) : connection_(connection) {} |
There was a problem hiding this comment.
if connection_ couldn't be null, make it a reference. also make constructor explicit
Signed-off-by: Dan Zhang <danzh@google.com>
| * No-op for these two methods to hold back the callbacks. | ||
| */ | ||
| void setReadBufferReady() override {} | ||
| void raiseEvent(Network::ConnectionEvent) override {} |
There was a problem hiding this comment.
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.
@alyssawilk I will merge master and use this in my PR #4153, ASSERT cannot be added here. In #4153, it have to swallow Connected event from RawBufferSocket, and raise it when handshake is done.
4 participants
Signed-off-by: Dan Zhang danzh@chromium.com
Description:
Add an implementation of TransportSocketCallbacks which does nothing when setReadBufferReady() or raiseEvent() is call. This implementation is supposed to be used in TransportSocket implementation which wraps another socket object. In this case, the underlying socket's callbacks should be suppressed.
Also add callbacks() in TransportSocket interface which is used to get the callbacks passed to setTransportSocketCallbacks().
Risk Level: Low (not enabled in main)
Testing: bazel test //test/...
Docs Changes: N/A
Release Notes: N/A