Addressed most PR feedback from graebm.

JonathanHenson · JonathanHenson · commit 160288657121 · 2018-11-21T11:37:21.000-08:00
diff --git a/include/aws/crt/io/Bootstrap.h b/include/aws/crt/io/Bootstrap.h
@@ -29,7 +29,7 @@ namespace Aws
             class AWS_CRT_CPP_API ClientBootstrap final
             {
             public:
-                ClientBootstrap(const EventLoopGroup& elGroup = EventLoopGroup(),
+                ClientBootstrap(EventLoopGroup& elGroup,
                         Allocator* allocator = DefaultAllocator()) noexcept;
                 ~ClientBootstrap();
                 ClientBootstrap(const ClientBootstrap&) = delete;
@@ -40,7 +40,7 @@ namespace Aws
                 operator bool() const noexcept;
                 int LastError() const noexcept;
 
-                const aws_client_bootstrap* GetUnderlyingHandle() const;
+                aws_client_bootstrap* GetUnderlyingHandle() noexcept;
             private:
                 aws_client_bootstrap m_bootstrap;
                 int m_lastError;
diff --git a/include/aws/crt/io/EventLoopGroup.h b/include/aws/crt/io/EventLoopGroup.h
@@ -49,7 +49,7 @@ namespace Aws
                 operator bool() const;
                 int LastError() const;
 
-                const aws_event_loop_group* GetUnderlyingHandle() const;
+                aws_event_loop_group* GetUnderlyingHandle() noexcept;
 
             private:
                 aws_event_loop_group m_eventLoopGroup;
diff --git a/include/aws/crt/mqtt/MqttClient.h b/include/aws/crt/mqtt/MqttClient.h
@@ -36,6 +36,15 @@ namespace Aws
             class MqttClient;
             class MqttConnection;
 
+            enum class ConnectionState
+            {
+                Init,
+                Connecting,
+                Connected,
+                Disconnected,
+                Error,
+            };
+
             /**
              * Invoked Upon Connection failure.
              */
@@ -69,12 +78,13 @@ namespace Aws
             public:
                 ~MqttConnection();
                 MqttConnection(const MqttConnection&) = delete;
-                MqttConnection(MqttConnection&&) = default;
+                MqttConnection(MqttConnection&&);
                 MqttConnection& operator =(const MqttConnection&) = delete;
-                MqttConnection& operator =(MqttConnection&&) = default;
+                MqttConnection& operator =(MqttConnection&&);
 
                 operator bool() const noexcept;
                 int LastError() const noexcept;
+                inline ConnectionState GetConnectionState() const noexcept { return m_connectionState; }
 
                 inline void SetOnConnectionFailedHandler(OnConnectionFailedHandler&& onConnectionFailed) noexcept
                 {
@@ -154,7 +164,7 @@ namespace Aws
                 OnConnAckHandler m_onConnAck;
                 OnDisconnectHandler m_onDisconnect;
                 int m_lastError;
-                bool m_isInit;
+                ConnectionState m_connectionState;
 
                 static void s_onConnectionFailed(aws_mqtt_client_connection* connection, int errorCode, void* userData);
                 static void s_onConnAck(aws_mqtt_client_connection* connection,
@@ -180,7 +190,7 @@ namespace Aws
                 /**
                  * Initialize an MqttClient using bootstrap and allocator
                  */
-                MqttClient(const Io::ClientBootstrap& bootstrap, Allocator* allocator = DefaultAllocator()) noexcept;
+                MqttClient(Io::ClientBootstrap& bootstrap, Allocator* allocator = DefaultAllocator()) noexcept;
 
                 ~MqttClient();
                 MqttClient(const MqttClient&) = delete;
diff --git a/samples/mqtt_pub_sub/main.cpp b/samples/mqtt_pub_sub/main.cpp
@@ -200,6 +200,7 @@ int main(int argc, char* argv[])
     {
         {
             fprintf(stdout, "Connection completed with return code %d\n", returnCode);
+            fprintf(stdout, "Conneciton state %d\n", connection.GetConnectionState());
             std::lock_guard<std::mutex> lockGuard(mutex);
             connectionSucceeded = true;
         }
@@ -226,6 +227,7 @@ int main(int argc, char* argv[])
     {
         {
             fprintf(stdout, "Connection closed\n");
+            fprintf(stdout, "Conneciton state %d\n", connection.GetConnectionState());
             std::lock_guard<std::mutex> lockGuard(mutex);
             connectionClosed = true;
         }
diff --git a/source/io/Bootstrap.cpp b/source/io/Bootstrap.cpp
@@ -20,12 +20,12 @@ namespace Aws
     {
         namespace Io
         {
-            ClientBootstrap::ClientBootstrap(const EventLoopGroup& elGroup, Allocator* allocator) noexcept :
+            ClientBootstrap::ClientBootstrap(EventLoopGroup& elGroup, Allocator* allocator) noexcept :
                 m_lastError(AWS_ERROR_SUCCESS)
             {
                 AWS_ZERO_STRUCT(m_bootstrap);
                 if (aws_client_bootstrap_init(&m_bootstrap, allocator,
-                        (aws_event_loop_group*)elGroup.GetUnderlyingHandle(), nullptr, nullptr))
+                        elGroup.GetUnderlyingHandle(), nullptr, nullptr))
                 {
                     m_lastError = aws_last_error();
                 }
@@ -74,7 +74,7 @@ namespace Aws
                 return m_lastError;
             }
 
-            const aws_client_bootstrap* ClientBootstrap::GetUnderlyingHandle() const
+            aws_client_bootstrap* ClientBootstrap::GetUnderlyingHandle() noexcept
             {
                 if (*this)
                 {
diff --git a/source/io/EventLoopGroup.cpp b/source/io/EventLoopGroup.cpp
@@ -81,7 +81,7 @@ namespace Aws
                 return m_lastError == AWS_ERROR_SUCCESS;
             }
 
-            const aws_event_loop_group *EventLoopGroup::GetUnderlyingHandle() const
+            aws_event_loop_group *EventLoopGroup::GetUnderlyingHandle() noexcept
             {
                 if (*this)
                 {
diff --git a/source/mqtt/MqttClient.cpp b/source/mqtt/MqttClient.cpp
@@ -30,6 +30,7 @@ namespace Aws
                 auto connWrapper = reinterpret_cast<MqttConnection*>(userData);
                 connWrapper->m_lastError = errorCode;
 
+                connWrapper->m_connectionState = ConnectionState::Disconnected;
                 if (connWrapper->m_onConnectionFailed)
                 {
                     connWrapper->m_onConnectionFailed(*connWrapper);
@@ -43,6 +44,15 @@ namespace Aws
             {
                 auto connWrapper = reinterpret_cast<MqttConnection*>(userData);
 
+                if (returnCode == AWS_MQTT_CONNECT_ACCEPTED)
+                {
+                    connWrapper->m_connectionState = ConnectionState::Connected;
+                }
+                else
+                {
+                    connWrapper->m_connectionState = ConnectionState::Error;
+                }
+
                 if (connWrapper->m_onConnAck)
                 {
                     connWrapper->m_onConnAck(*connWrapper, returnCode, sessionPresent);
@@ -54,9 +64,15 @@ namespace Aws
                 auto connWrapper = reinterpret_cast<MqttConnection*>(userData);
                 connWrapper->m_lastError = errorCode;
 
+                connWrapper->m_connectionState = ConnectionState::Disconnected;
+
                 if (connWrapper->m_onDisconnect)
                 {
-                    return connWrapper->m_onDisconnect(*connWrapper);
+                    if (connWrapper->m_onDisconnect(*connWrapper))
+                    {
+                        connWrapper->m_connectionState = ConnectionState::Connecting;
+                        return true;
+                    }                    
                 }
 
                 return false;
@@ -125,7 +141,7 @@ namespace Aws
                         Io::TlsConnectionOptions&& tlsConnOptions) noexcept :
                            m_owningClient(client),
                            m_lastError(AWS_ERROR_SUCCESS),
-                           m_isInit(false)
+                           m_connectionState(ConnectionState::Init)
             {
                 aws_mqtt_client_connection_callbacks callbacks;
                 AWS_ZERO_STRUCT(callbacks);
@@ -144,12 +160,9 @@ namespace Aws
 
                 if (!m_underlyingConnection)
                 {
+                    m_connectionState = ConnectionState::Error;
                     m_lastError = aws_last_error();
-                }
-                else
-                {
-                    m_isInit = true;
-                }
+                }                
             }
 
             MqttConnection::~MqttConnection()
@@ -158,11 +171,52 @@ namespace Aws
                 {
                     aws_mqtt_client_connection_destroy(m_underlyingConnection);
                 }
+
+                AWS_ZERO_STRUCT(*this);
+            }
+
+            MqttConnection::MqttConnection(MqttConnection&& toMove) :
+                m_owningClient(toMove.m_owningClient),
+                m_underlyingConnection(toMove.m_underlyingConnection),
+                m_onConnectionFailed(std::move(toMove.m_onConnectionFailed)),
+                m_onConnAck(std::move(toMove.m_onConnAck)),
+                m_onDisconnect(std::move(toMove.m_onDisconnect)),
+                m_lastError(toMove.m_lastError),
+                m_connectionState(toMove.m_connectionState)
+            {
+                toMove.m_owningClient = nullptr;
+                toMove.m_underlyingConnection = nullptr;
+                toMove.m_lastError = AWS_ERROR_UNKNOWN;
+                toMove.m_connectionState = ConnectionState::Error;
+            }
+
+            MqttConnection& MqttConnection::operator =(MqttConnection&& toMove)
+            {
+                if (this == &toMove)
+                {
+                    return *this;
+                }
+
+                m_owningClient = toMove.m_owningClient;
+                m_underlyingConnection = toMove.m_underlyingConnection;
+                m_onConnectionFailed = std::move(toMove.m_onConnectionFailed);
+                m_onConnAck = std::move(toMove.m_onConnAck);
+                m_onDisconnect = std::move(toMove.m_onDisconnect);
+                m_lastError = toMove.m_lastError;
+                m_connectionState = toMove.m_connectionState;
+
+                toMove.m_owningClient = nullptr;
+                toMove.m_underlyingConnection = nullptr;
+                toMove.m_lastError = AWS_ERROR_UNKNOWN;
+                toMove.m_connectionState = ConnectionState::Error;
+
+                return *this;
             }
 
+
             MqttConnection::operator bool() const noexcept
             {
-                return m_isInit;
+                return m_connectionState != ConnectionState::Error;
             }
 
             int MqttConnection::LastError() const noexcept
@@ -206,6 +260,10 @@ namespace Aws
                 {
                     m_lastError = aws_last_error();
                 }
+                else
+                {
+                    m_connectionState = ConnectionState::Connecting;
+                }
             }
 
             void MqttConnection::Disconnect() noexcept
@@ -365,13 +423,12 @@ namespace Aws
                 aws_mqtt_client_connection_ping(m_underlyingConnection);
             }
 
-            MqttClient::MqttClient(const Io::ClientBootstrap &bootstrap, Allocator *allocator) noexcept :
+            MqttClient::MqttClient(Io::ClientBootstrap &bootstrap, Allocator *allocator) noexcept :
                     m_lastError(AWS_ERROR_SUCCESS),
                     m_isInit(false)
             {
                 AWS_ZERO_STRUCT(m_client);
-                if (aws_mqtt_client_init(&m_client, allocator,
-                                         const_cast<aws_client_bootstrap*>(bootstrap.GetUnderlyingHandle())))
+                if (aws_mqtt_client_init(&m_client, allocator,  bootstrap.GetUnderlyingHandle()))
                 {
                     m_lastError = aws_last_error();
                 }

Original file line number	Diff line number	Diff line change
`@@ -200,6 +200,7 @@ int main(int argc, char* argv[])`
`200`	`200`	`{`
`201`	`201`	`{`
`202`	`202`	`fprintf(stdout, "Connection completed with return code %d\n", returnCode);`
	`203`	`+ fprintf(stdout, "Conneciton state %d\n", connection.GetConnectionState());`
`203`	`204`	`std::lock_guard<std::mutex> lockGuard(mutex);`
`204`	`205`	`connectionSucceeded = true;`
`205`	`206`	`}`
`@@ -226,6 +227,7 @@ int main(int argc, char* argv[])`
`226`	`227`	`{`
`227`	`228`	`{`
`228`	`229`	`fprintf(stdout, "Connection closed\n");`
	`230`	`+ fprintf(stdout, "Conneciton state %d\n", connection.GetConnectionState());`
`229`	`231`	`std::lock_guard<std::mutex> lockGuard(mutex);`
`230`	`232`	`connectionClosed = true;`
`231`	`233`	`}`
Original file line number	Diff line number	Diff line change
`@@ -20,12 +20,12 @@ namespace Aws`
`20`	`20`	`{`
`21`	`21`	`namespace Io`
`22`	`22`	`{`
`23`		`- ClientBootstrap::ClientBootstrap(const EventLoopGroup& elGroup, Allocator* allocator) noexcept :`
	`23`	`+ ClientBootstrap::ClientBootstrap(EventLoopGroup& elGroup, Allocator* allocator) noexcept :`
`24`	`24`	`m_lastError(AWS_ERROR_SUCCESS)`
`25`	`25`	`{`
`26`	`26`	`AWS_ZERO_STRUCT(m_bootstrap);`
`27`	`27`	`if (aws_client_bootstrap_init(&m_bootstrap, allocator,`
`28`		`- (aws_event_loop_group*)elGroup.GetUnderlyingHandle(), nullptr, nullptr))`
	`28`	`+ elGroup.GetUnderlyingHandle(), nullptr, nullptr))`
`29`	`29`	`{`
`30`	`30`	`m_lastError = aws_last_error();`
`31`	`31`	`}`
`@@ -74,7 +74,7 @@ namespace Aws`
`74`	`74`	`return m_lastError;`
`75`	`75`	`}`
`76`	`76`
`77`		`- const aws_client_bootstrap* ClientBootstrap::GetUnderlyingHandle() const`
	`77`	`+ aws_client_bootstrap* ClientBootstrap::GetUnderlyingHandle() noexcept`
`78`	`78`	`{`
`79`	`79`	`if (*this)`
`80`	`80`	`{`
Original file line number	Diff line number	Diff line change
`@@ -81,7 +81,7 @@ namespace Aws`
`81`	`81`	`return m_lastError == AWS_ERROR_SUCCESS;`
`82`	`82`	`}`
`83`	`83`
`84`		`- const aws_event_loop_group *EventLoopGroup::GetUnderlyingHandle() const`
	`84`	`+ aws_event_loop_group *EventLoopGroup::GetUnderlyingHandle() noexcept`
`85`	`85`	`{`
`86`	`86`	`if (*this)`
`87`	`87`	`{`