Commit ba3aedb3 authored by Cristian Lumezanu's avatar Cristian Lumezanu Committed by Facebook GitHub Bot

Add connectAttempt and connectSuccess AsyncSocket::LifecycleObserver callbacks

Summary:
Made the following changes AsyncTransport::LifecycleObserver callback
- added `connectAttempt()`, called for each observer when connect is invoked;
- changed `connect()` to `connectSuccess()`, called for each observer when connect is successful;

Reviewed By: bschlinker

Differential Revision: D29962337

fbshipit-source-id: f1d2c0e48b6d7e263466f74c97d701bb9df9b313
parent 81c70338
...@@ -750,6 +750,7 @@ void AsyncSocket::connect( ...@@ -750,6 +750,7 @@ void AsyncSocket::connect(
assert(fd_ == NetworkSocket()); assert(fd_ == NetworkSocket());
state_ = StateEnum::CONNECTING; state_ = StateEnum::CONNECTING;
connectCallback_ = callback; connectCallback_ = callback;
invokeConnectAttempt();
sockaddr_storage addrStorage; sockaddr_storage addrStorage;
auto saddr = reinterpret_cast<sockaddr*>(&addrStorage); auto saddr = reinterpret_cast<sockaddr*>(&addrStorage);
...@@ -3697,10 +3698,7 @@ void AsyncSocket::invokeConnectErr(const AsyncSocketException& ex) { ...@@ -3697,10 +3698,7 @@ void AsyncSocket::invokeConnectErr(const AsyncSocketException& ex) {
// back wants. This seems like a bug but work around here while we explore // back wants. This seems like a bug but work around here while we explore
// it independently // it independently
for (const auto& cb : lifecycleObservers_) { for (const auto& cb : lifecycleObservers_) {
if (auto observer = dynamic_cast<AsyncSocket::LifecycleObserver*>(cb)) { cb->connectError(this, ex);
// inform any lifecycle observes that the connection failed
observer->connectError(this, ex);
}
} }
} }
if (connectCallback_) { if (connectCallback_) {
...@@ -3715,9 +3713,9 @@ void AsyncSocket::invokeConnectSuccess() { ...@@ -3715,9 +3713,9 @@ void AsyncSocket::invokeConnectSuccess() {
<< "): connect success invoked"; << "): connect success invoked";
connectEndTime_ = std::chrono::steady_clock::now(); connectEndTime_ = std::chrono::steady_clock::now();
bool enableByteEventsForObserver = false; bool enableByteEventsForObserver = false;
for (const auto& observer : lifecycleObservers_) { for (const auto& cb : lifecycleObservers_) {
observer->connect(this); cb->connectSuccess(this);
enableByteEventsForObserver |= ((observer->getConfig().byteEvents) ? 1 : 0); enableByteEventsForObserver |= ((cb->getConfig().byteEvents) ? 1 : 0);
} }
if (enableByteEventsForObserver) { if (enableByteEventsForObserver) {
enableByteEvents(); enableByteEvents();
...@@ -3729,6 +3727,14 @@ void AsyncSocket::invokeConnectSuccess() { ...@@ -3729,6 +3727,14 @@ void AsyncSocket::invokeConnectSuccess() {
} }
} }
void AsyncSocket::invokeConnectAttempt() {
VLOG(5) << "AsyncSocket(this=" << this << ", fd=" << fd_
<< "): connect attempt";
for (const auto& cb : lifecycleObservers_) {
cb->connectAttempt(this);
}
}
void AsyncSocket::invalidState(ReadCallback* callback) { void AsyncSocket::invalidState(ReadCallback* callback) {
VLOG(4) << "AsyncSocket(this=" << this << ", fd=" << fd_ VLOG(4) << "AsyncSocket(this=" << this << ", fd=" << fd_
<< "): setReadCallback(" << callback << ") called in invalid state " << "): setReadCallback(" << callback << ") called in invalid state "
......
...@@ -1481,6 +1481,7 @@ class AsyncSocket : public AsyncTransport { ...@@ -1481,6 +1481,7 @@ class AsyncSocket : public AsyncTransport {
void failByteEvents(const AsyncSocketException& ex); void failByteEvents(const AsyncSocketException& ex);
virtual void invokeConnectErr(const AsyncSocketException& ex); virtual void invokeConnectErr(const AsyncSocketException& ex);
virtual void invokeConnectSuccess(); virtual void invokeConnectSuccess();
virtual void invokeConnectAttempt();
void invalidState(ConnectCallback* callback); void invalidState(ConnectCallback* callback);
void invalidState(ErrMessageCallback* callback); void invalidState(ErrMessageCallback* callback);
void invalidState(ReadCallback* callback); void invalidState(ReadCallback* callback);
......
...@@ -1000,13 +1000,22 @@ class AsyncTransport : public DelayedDestruction, ...@@ -1000,13 +1000,22 @@ class AsyncTransport : public DelayedDestruction,
virtual void close(AsyncTransport* /* transport */) noexcept = 0; virtual void close(AsyncTransport* /* transport */) noexcept = 0;
/** /**
* connect() will be invoked when connect() returns successfully. * connectAttempt() will be invoked when connect() is called.
*
* Triggered before any application connection callback.
*
* @param transport Transport that attempts to connect.
*/
virtual void connectAttempt(AsyncTransport* /* transport */) noexcept {}
/**
* connectSuccess() will be invoked when connect() returns successfully.
* *
* Triggered before any application connection callback. * Triggered before any application connection callback.
* *
* @param transport Transport that has connected. * @param transport Transport that has connected.
*/ */
virtual void connect(AsyncTransport* /* transport */) noexcept = 0; virtual void connectSuccess(AsyncTransport* /* transport */) noexcept {}
/** /**
* connectError() will be invoked when connect() returns an error. * connectError() will be invoked when connect() returns an error.
......
...@@ -7598,8 +7598,9 @@ TEST(AsyncSocket, LifecycleObserverAttachThenDestroySocket) { ...@@ -7598,8 +7598,9 @@ TEST(AsyncSocket, LifecycleObserverAttachThenDestroySocket) {
EXPECT_THAT(socket->getLifecycleObservers(), UnorderedElementsAre(cb.get())); EXPECT_THAT(socket->getLifecycleObservers(), UnorderedElementsAre(cb.get()));
Mock::VerifyAndClearExpectations(cb.get()); Mock::VerifyAndClearExpectations(cb.get());
EXPECT_CALL(*cb, connectMock(socket.get())); EXPECT_CALL(*cb, connectAttemptMock(socket.get()));
EXPECT_CALL(*cb, fdAttachMock(socket.get())); EXPECT_CALL(*cb, fdAttachMock(socket.get()));
EXPECT_CALL(*cb, connectSuccessMock(socket.get()));
socket->connect(nullptr, server.getAddress(), 30); socket->connect(nullptr, server.getAddress(), 30);
evb.loop(); evb.loop();
Mock::VerifyAndClearExpectations(cb.get()); Mock::VerifyAndClearExpectations(cb.get());
...@@ -7625,6 +7626,7 @@ TEST(AsyncSocket, LifecycleObserverAttachThenConnectError) { ...@@ -7625,6 +7626,7 @@ TEST(AsyncSocket, LifecycleObserverAttachThenConnectError) {
// the current state machine calls AsyncSocket::invokeConnectionError() twice // the current state machine calls AsyncSocket::invokeConnectionError() twice
// for this use-case... // for this use-case...
EXPECT_CALL(*cb, connectAttemptMock(socket.get()));
EXPECT_CALL(*cb, fdAttachMock(socket.get())); EXPECT_CALL(*cb, fdAttachMock(socket.get()));
EXPECT_CALL(*cb, connectErrorMock(socket.get(), _)).Times(2); EXPECT_CALL(*cb, connectErrorMock(socket.get(), _)).Times(2);
EXPECT_CALL(*cb, closeMock(socket.get())); EXPECT_CALL(*cb, closeMock(socket.get()));
...@@ -7658,16 +7660,18 @@ TEST(AsyncSocket, LifecycleObserverMultipleAttachThenDestroySocket) { ...@@ -7658,16 +7660,18 @@ TEST(AsyncSocket, LifecycleObserverMultipleAttachThenDestroySocket) {
Mock::VerifyAndClearExpectations(cb1.get()); Mock::VerifyAndClearExpectations(cb1.get());
Mock::VerifyAndClearExpectations(cb2.get()); Mock::VerifyAndClearExpectations(cb2.get());
EXPECT_CALL(*cb1, connectMock(socket.get())); InSequence s;
EXPECT_CALL(*cb2, connectMock(socket.get())); EXPECT_CALL(*cb1, connectAttemptMock(socket.get()));
EXPECT_CALL(*cb2, connectAttemptMock(socket.get()));
EXPECT_CALL(*cb1, fdAttachMock(socket.get())); EXPECT_CALL(*cb1, fdAttachMock(socket.get()));
EXPECT_CALL(*cb2, fdAttachMock(socket.get())); EXPECT_CALL(*cb2, fdAttachMock(socket.get()));
EXPECT_CALL(*cb1, connectSuccessMock(socket.get()));
EXPECT_CALL(*cb2, connectSuccessMock(socket.get()));
socket->connect(nullptr, server.getAddress(), 30); socket->connect(nullptr, server.getAddress(), 30);
evb.loop(); evb.loop();
Mock::VerifyAndClearExpectations(cb1.get()); Mock::VerifyAndClearExpectations(cb1.get());
Mock::VerifyAndClearExpectations(cb2.get()); Mock::VerifyAndClearExpectations(cb2.get());
InSequence s;
EXPECT_CALL(*cb1, closeMock(socket.get())); EXPECT_CALL(*cb1, closeMock(socket.get()));
EXPECT_CALL(*cb2, closeMock(socket.get())); EXPECT_CALL(*cb2, closeMock(socket.get()));
EXPECT_CALL(*cb1, destroyMock(socket.get())); EXPECT_CALL(*cb1, destroyMock(socket.get()));
...@@ -7803,8 +7807,9 @@ TEST(AsyncSocket, LifecycleObserverDetach) { ...@@ -7803,8 +7807,9 @@ TEST(AsyncSocket, LifecycleObserverDetach) {
EXPECT_THAT(socket1->getLifecycleObservers(), UnorderedElementsAre(cb.get())); EXPECT_THAT(socket1->getLifecycleObservers(), UnorderedElementsAre(cb.get()));
Mock::VerifyAndClearExpectations(cb.get()); Mock::VerifyAndClearExpectations(cb.get());
EXPECT_CALL(*cb, connectMock(socket1.get())); EXPECT_CALL(*cb, connectAttemptMock(socket1.get()));
EXPECT_CALL(*cb, fdAttachMock(socket1.get())); EXPECT_CALL(*cb, fdAttachMock(socket1.get()));
EXPECT_CALL(*cb, connectSuccessMock(socket1.get()));
socket1->connect(nullptr, server.getAddress(), 30); socket1->connect(nullptr, server.getAddress(), 30);
evb.loop(); evb.loop();
Mock::VerifyAndClearExpectations(cb.get()); Mock::VerifyAndClearExpectations(cb.get());
...@@ -7832,8 +7837,9 @@ TEST(AsyncSocket, LifecycleObserverMoveResubscribe) { ...@@ -7832,8 +7837,9 @@ TEST(AsyncSocket, LifecycleObserverMoveResubscribe) {
EXPECT_THAT(socket1->getLifecycleObservers(), UnorderedElementsAre(cb.get())); EXPECT_THAT(socket1->getLifecycleObservers(), UnorderedElementsAre(cb.get()));
Mock::VerifyAndClearExpectations(cb.get()); Mock::VerifyAndClearExpectations(cb.get());
EXPECT_CALL(*cb, connectMock(socket1.get())); EXPECT_CALL(*cb, connectAttemptMock(socket1.get()));
EXPECT_CALL(*cb, fdAttachMock(socket1.get())); EXPECT_CALL(*cb, fdAttachMock(socket1.get()));
EXPECT_CALL(*cb, connectSuccessMock(socket1.get()));
socket1->connect(nullptr, server.getAddress(), 30); socket1->connect(nullptr, server.getAddress(), 30);
evb.loop(); evb.loop();
Mock::VerifyAndClearExpectations(cb.get()); Mock::VerifyAndClearExpectations(cb.get());
...@@ -7879,8 +7885,9 @@ TEST(AsyncSocket, LifecycleObserverMoveDoNotResubscribe) { ...@@ -7879,8 +7885,9 @@ TEST(AsyncSocket, LifecycleObserverMoveDoNotResubscribe) {
EXPECT_THAT(socket1->getLifecycleObservers(), UnorderedElementsAre(cb.get())); EXPECT_THAT(socket1->getLifecycleObservers(), UnorderedElementsAre(cb.get()));
Mock::VerifyAndClearExpectations(cb.get()); Mock::VerifyAndClearExpectations(cb.get());
EXPECT_CALL(*cb, connectMock(socket1.get())); EXPECT_CALL(*cb, connectAttemptMock(socket1.get()));
EXPECT_CALL(*cb, fdAttachMock(socket1.get())); EXPECT_CALL(*cb, fdAttachMock(socket1.get()));
EXPECT_CALL(*cb, connectSuccessMock(socket1.get()));
socket1->connect(nullptr, server.getAddress(), 30); socket1->connect(nullptr, server.getAddress(), 30);
evb.loop(); evb.loop();
Mock::VerifyAndClearExpectations(cb.get()); Mock::VerifyAndClearExpectations(cb.get());
...@@ -7931,8 +7938,9 @@ TEST(AsyncSocket, LifecycleObserverDetachCallbackAfterConnect) { ...@@ -7931,8 +7938,9 @@ TEST(AsyncSocket, LifecycleObserverDetachCallbackAfterConnect) {
socket->addLifecycleObserver(cb.get()); socket->addLifecycleObserver(cb.get());
Mock::VerifyAndClearExpectations(cb.get()); Mock::VerifyAndClearExpectations(cb.get());
EXPECT_CALL(*cb, connectMock(socket.get())); EXPECT_CALL(*cb, connectAttemptMock(socket.get()));
EXPECT_CALL(*cb, fdAttachMock(socket.get())); EXPECT_CALL(*cb, fdAttachMock(socket.get()));
EXPECT_CALL(*cb, connectSuccessMock(socket.get()));
socket->connect(nullptr, server.getAddress(), 30); socket->connect(nullptr, server.getAddress(), 30);
evb.loop(); evb.loop();
Mock::VerifyAndClearExpectations(cb.get()); Mock::VerifyAndClearExpectations(cb.get());
...@@ -7952,8 +7960,9 @@ TEST(AsyncSocket, LifecycleObserverDetachCallbackAfterClose) { ...@@ -7952,8 +7960,9 @@ TEST(AsyncSocket, LifecycleObserverDetachCallbackAfterClose) {
socket->addLifecycleObserver(cb.get()); socket->addLifecycleObserver(cb.get());
Mock::VerifyAndClearExpectations(cb.get()); Mock::VerifyAndClearExpectations(cb.get());
EXPECT_CALL(*cb, connectMock(socket.get())); EXPECT_CALL(*cb, connectAttemptMock(socket.get()));
EXPECT_CALL(*cb, fdAttachMock(socket.get())); EXPECT_CALL(*cb, fdAttachMock(socket.get()));
EXPECT_CALL(*cb, connectSuccessMock(socket.get()));
socket->connect(nullptr, server.getAddress(), 30); socket->connect(nullptr, server.getAddress(), 30);
evb.loop(); evb.loop();
Mock::VerifyAndClearExpectations(cb.get()); Mock::VerifyAndClearExpectations(cb.get());
...@@ -7977,8 +7986,9 @@ TEST(AsyncSocket, LifecycleObserverDetachCallbackcloseDuringDestroy) { ...@@ -7977,8 +7986,9 @@ TEST(AsyncSocket, LifecycleObserverDetachCallbackcloseDuringDestroy) {
socket->addLifecycleObserver(cb.get()); socket->addLifecycleObserver(cb.get());
Mock::VerifyAndClearExpectations(cb.get()); Mock::VerifyAndClearExpectations(cb.get());
EXPECT_CALL(*cb, connectMock(socket.get())); EXPECT_CALL(*cb, connectAttemptMock(socket.get()));
EXPECT_CALL(*cb, fdAttachMock(socket.get())); EXPECT_CALL(*cb, fdAttachMock(socket.get()));
EXPECT_CALL(*cb, connectSuccessMock(socket.get()));
socket->connect(nullptr, server.getAddress(), 30); socket->connect(nullptr, server.getAddress(), 30);
evb.loop(); evb.loop();
Mock::VerifyAndClearExpectations(cb.get()); Mock::VerifyAndClearExpectations(cb.get());
...@@ -8006,7 +8016,8 @@ TEST(AsyncSocket, LifecycleObserverBaseClassMoveNoCrash) { ...@@ -8006,7 +8016,8 @@ TEST(AsyncSocket, LifecycleObserverBaseClassMoveNoCrash) {
EXPECT_THAT(socket1->getLifecycleObservers(), UnorderedElementsAre(cb.get())); EXPECT_THAT(socket1->getLifecycleObservers(), UnorderedElementsAre(cb.get()));
Mock::VerifyAndClearExpectations(cb.get()); Mock::VerifyAndClearExpectations(cb.get());
EXPECT_CALL(*cb, connectMock(socket1.get())); EXPECT_CALL(*cb, connectAttemptMock(socket1.get()));
EXPECT_CALL(*cb, connectSuccessMock(socket1.get()));
socket1->connect(nullptr, server.getAddress(), 30); socket1->connect(nullptr, server.getAddress(), 30);
evb.loop(); evb.loop();
Mock::VerifyAndClearExpectations(cb.get()); Mock::VerifyAndClearExpectations(cb.get());
......
...@@ -37,7 +37,8 @@ class MockAsyncSocketLifecycleObserver : public AsyncSocket::LifecycleObserver { ...@@ -37,7 +37,8 @@ class MockAsyncSocketLifecycleObserver : public AsyncSocket::LifecycleObserver {
MOCK_METHOD1(observerDetachMock, void(AsyncTransport*)); MOCK_METHOD1(observerDetachMock, void(AsyncTransport*));
MOCK_METHOD1(destroyMock, void(AsyncTransport*)); MOCK_METHOD1(destroyMock, void(AsyncTransport*));
MOCK_METHOD1(closeMock, void(AsyncTransport*)); MOCK_METHOD1(closeMock, void(AsyncTransport*));
MOCK_METHOD1(connectMock, void(AsyncTransport*)); MOCK_METHOD1(connectAttemptMock, void(AsyncTransport*));
MOCK_METHOD1(connectSuccessMock, void(AsyncTransport*));
MOCK_METHOD2( MOCK_METHOD2(
connectErrorMock, void(AsyncTransport*, const AsyncSocketException&)); connectErrorMock, void(AsyncTransport*, const AsyncSocketException&));
MOCK_METHOD2(evbAttachMock, void(AsyncTransport*, EventBase*)); MOCK_METHOD2(evbAttachMock, void(AsyncTransport*, EventBase*));
...@@ -63,7 +64,12 @@ class MockAsyncSocketLifecycleObserver : public AsyncSocket::LifecycleObserver { ...@@ -63,7 +64,12 @@ class MockAsyncSocketLifecycleObserver : public AsyncSocket::LifecycleObserver {
} }
void destroy(AsyncTransport* trans) noexcept override { destroyMock(trans); } void destroy(AsyncTransport* trans) noexcept override { destroyMock(trans); }
void close(AsyncTransport* trans) noexcept override { closeMock(trans); } void close(AsyncTransport* trans) noexcept override { closeMock(trans); }
void connect(AsyncTransport* trans) noexcept override { connectMock(trans); } void connectAttempt(AsyncTransport* trans) noexcept override {
connectAttemptMock(trans);
}
void connectSuccess(AsyncTransport* trans) noexcept override {
connectSuccessMock(trans);
}
void connectError( void connectError(
AsyncTransport* trans, const AsyncSocketException& ex) noexcept override { AsyncTransport* trans, const AsyncSocketException& ex) noexcept override {
connectErrorMock(trans, ex); connectErrorMock(trans, ex);
......
...@@ -31,7 +31,10 @@ class MockAsyncTransportLifecycleObserver ...@@ -31,7 +31,10 @@ class MockAsyncTransportLifecycleObserver
MOCK_METHOD1(observerDetachMock, void(AsyncTransport*)); MOCK_METHOD1(observerDetachMock, void(AsyncTransport*));
MOCK_METHOD1(destroyMock, void(AsyncTransport*)); MOCK_METHOD1(destroyMock, void(AsyncTransport*));
MOCK_METHOD1(closeMock, void(AsyncTransport*)); MOCK_METHOD1(closeMock, void(AsyncTransport*));
MOCK_METHOD1(connectMock, void(AsyncTransport*)); MOCK_METHOD1(connectAttemptMock, void(AsyncTransport*));
MOCK_METHOD1(connectSuccessMock, void(AsyncTransport*));
MOCK_METHOD2(
connectErrorMock, void(AsyncTransport*, const AsyncSocketException&));
MOCK_METHOD2(evbAttachMock, void(AsyncTransport*, EventBase*)); MOCK_METHOD2(evbAttachMock, void(AsyncTransport*, EventBase*));
MOCK_METHOD2(evbDetachMock, void(AsyncTransport*, EventBase*)); MOCK_METHOD2(evbDetachMock, void(AsyncTransport*, EventBase*));
MOCK_METHOD2( MOCK_METHOD2(
...@@ -52,7 +55,16 @@ class MockAsyncTransportLifecycleObserver ...@@ -52,7 +55,16 @@ class MockAsyncTransportLifecycleObserver
} }
void destroy(AsyncTransport* trans) noexcept override { destroyMock(trans); } void destroy(AsyncTransport* trans) noexcept override { destroyMock(trans); }
void close(AsyncTransport* trans) noexcept override { closeMock(trans); } void close(AsyncTransport* trans) noexcept override { closeMock(trans); }
void connect(AsyncTransport* trans) noexcept override { connectMock(trans); } void connectAttempt(AsyncTransport* trans) noexcept override {
connectAttemptMock(trans);
}
void connectSuccess(AsyncTransport* trans) noexcept override {
connectSuccessMock(trans);
}
void connectError(
AsyncTransport* trans, const AsyncSocketException& ex) noexcept override {
connectErrorMock(trans, ex);
}
void evbAttach(AsyncTransport* trans, EventBase* eb) noexcept override { void evbAttach(AsyncTransport* trans, EventBase* eb) noexcept override {
evbAttachMock(trans, eb); evbAttachMock(trans, eb);
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment