Fix the protocol abstraction so different protocols can be supported (#48)

Author: BrennanConroy

include/signalrclient/connection.h

@@ -11,6 +11,7 @@
 #include "trace_level.h"
 #include "log_writer.h"
 #include "signalr_client_config.h"
+#include "transfer_format.h"
 
 namespace signalr
 {
@@ -31,7 +32,7 @@ namespace signalr
 
         SIGNALRCLIENT_API void __cdecl start(std::function<void(std::exception_ptr)> callback) noexcept;
 
-        SIGNALRCLIENT_API void __cdecl send(const std::string& data, std::function<void(std::exception_ptr)> callback) noexcept;
+        SIGNALRCLIENT_API void __cdecl send(const std::string& data, transfer_format transfer_format, std::function<void(std::exception_ptr)> callback) noexcept;
 
         SIGNALRCLIENT_API void __cdecl set_message_received(const message_received_handler& message_received_callback);
         SIGNALRCLIENT_API void __cdecl set_disconnected(const std::function<void __cdecl()>& disconnected_callback);

include/signalrclient/signalr_value.h

@@ -7,6 +7,7 @@
 #include <string>
 #include <vector>
 #include <map>
+#include <cstddef>
 
 namespace signalr
 {
@@ -34,6 +35,11 @@ namespace signalr
          */
         value();
 
+        /**
+         * Create an object representing a value_type::null value.
+         */
+        value(std::nullptr_t);
+
         /**
          * Create an object representing a default value for the given value_type.
          */
@@ -64,6 +70,11 @@ namespace signalr
          */
         value(const char* val);
 
+        /**
+         * Create an object representing a value_type::string with the given string value.
+         */
+        value(const char* val, size_t length);
+
         /**
          * Create an object representing a value_type::array with the given vector of value's.
          */

include/signalrclient/websocket_client.h

@@ -14,11 +14,11 @@ namespace signalr
     public:
         virtual ~websocket_client() {};
 
-        virtual void start(const std::string& url, transfer_format format, std::function<void(std::exception_ptr)> callback) = 0;
+        virtual void start(const std::string& url, std::function<void(std::exception_ptr)> callback) = 0;
 
         virtual void stop(std::function<void(std::exception_ptr)> callback) = 0;
 
-        virtual void send(const std::string& payload, std::function<void(std::exception_ptr)> callback) = 0;
+        virtual void send(const std::string& payload, transfer_format transfer_format, std::function<void(std::exception_ptr)> callback) = 0;
 
         virtual void receive(std::function<void(const std::string&, std::exception_ptr)> callback) = 0;
     };

src/signalrclient/callback_manager.cpp

@@ -10,17 +10,17 @@ namespace signalr
 {
     // dtor_clear_arguments will be passed when closing any pending callbacks when the `callback_manager` is
     // destroyed (i.e. in the dtor)
-    callback_manager::callback_manager(const signalr::value& dtor_clear_arguments)
+    callback_manager::callback_manager(const char* dtor_clear_arguments)
         : m_dtor_clear_arguments(dtor_clear_arguments)
     { }
 
     callback_manager::~callback_manager()
     {
-        clear(m_dtor_clear_arguments);
+        clear(m_dtor_clear_arguments.data());
     }
 
     // note: callback must not throw except for the `on_progress` callback which will never be invoked from the dtor
-    std::string callback_manager::register_callback(const std::function<void(const signalr::value&)>& callback)
+    std::string callback_manager::register_callback(const std::function<void(const char*, const signalr::value&)>& callback)
     {
         auto callback_id = get_callback_id();
 
@@ -35,9 +35,9 @@ namespace signalr
 
 
     // invokes a callback and stops tracking it if remove callback set to true
-    bool callback_manager::invoke_callback(const std::string& callback_id, const signalr::value& arguments, bool remove_callback)
+    bool callback_manager::invoke_callback(const std::string& callback_id, const char* error, const signalr::value& arguments, bool remove_callback)
     {
-        std::function<void(const signalr::value& arguments)> callback;
+        std::function<void(const char*, const signalr::value& arguments)> callback;
 
         {
             std::lock_guard<std::mutex> lock(m_map_lock);
@@ -56,7 +56,7 @@ namespace signalr
             }
         }
 
-        callback(arguments);
+        callback(error, arguments);
         return true;
     }
 
@@ -69,14 +69,14 @@ namespace signalr
         }
     }
 
-    void callback_manager::clear(const signalr::value& arguments)
+    void callback_manager::clear(const char* error)
     {
         {
             std::lock_guard<std::mutex> lock(m_map_lock);
 
             for (auto& kvp : m_callbacks)
             {
-                kvp.second(arguments);
+                kvp.second(error, signalr::value());
             }
 
             m_callbacks.clear();

src/signalrclient/callback_manager.h

@@ -15,22 +15,22 @@ namespace signalr
     class callback_manager
     {
     public:
-        explicit callback_manager(const signalr::value& dtor_error);
+        explicit callback_manager(const char* dtor_error);
         ~callback_manager();
 
         callback_manager(const callback_manager&) = delete;
         callback_manager& operator=(const callback_manager&) = delete;
 
-        std::string register_callback(const std::function<void(const signalr::value&)>& callback);
-        bool invoke_callback(const std::string& callback_id, const signalr::value& arguments, bool remove_callback);
+        std::string register_callback(const std::function<void(const char*, const signalr::value&)>& callback);
+        bool invoke_callback(const std::string& callback_id, const char* error, const signalr::value& arguments, bool remove_callback);
         bool remove_callback(const std::string& callback_id);
-        void clear(const signalr::value& arguments);
+        void clear(const char* error);
 
     private:
         std::atomic<int> m_id { 0 };
-        std::unordered_map<std::string, std::function<void(const signalr::value&)>> m_callbacks;
+        std::unordered_map<std::string, std::function<void(const char*, const signalr::value&)>> m_callbacks;
         std::mutex m_map_lock;
-        const signalr::value m_dtor_clear_arguments;
+        std::string m_dtor_clear_arguments;
 
         std::string get_callback_id();
     };

src/signalrclient/connection.cpp

@@ -22,9 +22,9 @@ namespace signalr
         m_pImpl->start(callback);
     }
 
-    void connection::send(const std::string& data, std::function<void(std::exception_ptr)> callback) noexcept
+    void connection::send(const std::string& data, transfer_format transfer_format, std::function<void(std::exception_ptr)> callback) noexcept
     {
-        m_pImpl->send(data, callback);
+        m_pImpl->send(data, transfer_format, callback);
     }
 
     void connection::set_message_received(const message_received_handler& message_received_callback)

src/signalrclient/connection_impl.cpp

@@ -455,7 +455,7 @@ namespace signalr
         auto query_string = "id=" + m_connection_token;
         auto connect_url = url_builder::build_connect(url, transport->get_transport_type(), query_string);
 
-        transport->start(connect_url, transfer_format::text, [callback, logger](std::exception_ptr exception)
+        transport->start(connect_url, [callback, logger](std::exception_ptr exception)
             mutable {
                 try
                 {
@@ -479,6 +479,7 @@ namespace signalr
 
     void connection_impl::process_response(std::string&& response)
     {
+        // TODO: log binary data better
         m_logger.log(trace_level::messages,
             std::string("processing message: ").append(response));
 
@@ -504,7 +505,7 @@ namespace signalr
         }
     }
 
-    void connection_impl::send(const std::string& data, std::function<void(std::exception_ptr)> callback) noexcept
+    void connection_impl::send(const std::string& data, transfer_format transfer_format, std::function<void(std::exception_ptr)> callback) noexcept
     {
         // To prevent an (unlikely) condition where the transport is nulled out after we checked the connection_state
         // and before sending data we store the pointer in the local variable. In this case `send()` will throw but
@@ -524,7 +525,7 @@ namespace signalr
 
         logger.log(trace_level::info, std::string("sending data: ").append(data));
 
-        transport->send(data, [logger, callback](std::exception_ptr exception)
+        transport->send(data, transfer_format, [logger, callback](std::exception_ptr exception)
             mutable {
                 try
                 {

src/signalrclient/connection_impl.h

@@ -39,7 +39,7 @@ namespace signalr
         ~connection_impl();
 
         void start(std::function<void(std::exception_ptr)> callback) noexcept;
-        void send(const std::string &data, std::function<void(std::exception_ptr)> callback) noexcept;
+        void send(const std::string &data, transfer_format transfer_format, std::function<void(std::exception_ptr)> callback) noexcept;
         void stop(std::function<void(std::exception_ptr)> callback) noexcept;
 
         connection_state get_connection_state() const noexcept;

src/signalrclient/default_websocket_client.cpp

@@ -6,6 +6,7 @@
 
 #ifdef USE_CPPRESTSDK
 #include "default_websocket_client.h"
+#include <signalrclient/signalr_exception.h>
 
 namespace signalr
 {
@@ -28,7 +29,7 @@ namespace signalr
         : m_underlying_client(create_client_config(signalr_client_config))
     { }
 
-    void default_websocket_client::start(const std::string& url, transfer_format, std::function<void(std::exception_ptr)> callback)
+    void default_websocket_client::start(const std::string& url, std::function<void(std::exception_ptr)> callback)
     {
         m_underlying_client.connect(utility::conversions::to_string_t(url))
             .then([callback](pplx::task<void> task)
@@ -62,10 +63,18 @@ namespace signalr
             });
     }
 
-    void default_websocket_client::send(const std::string& payload, std::function<void(std::exception_ptr)> callback)
+    void default_websocket_client::send(const std::string& payload, signalr::transfer_format transfer_format, std::function<void(std::exception_ptr)> callback)
     {
         web::websockets::client::websocket_outgoing_message msg;
-        msg.set_utf8_message(payload);
+
+        if (transfer_format == signalr::transfer_format::binary)
+        {
+            throw signalr_exception("binary isn't supported currently");
+        }
+        else
+        {
+            msg.set_utf8_message(payload);
+        }
         m_underlying_client.send(msg)
             .then([callback](pplx::task<void> task)
             {
@@ -88,8 +97,15 @@ namespace signalr
             {
                 try
                 {
+
+                    std::string msg;
                     auto response = task.get();
-                    auto msg = response.extract_string().get();
+                    if (response.message_type() == web::websockets::client::websocket_message_type::binary_message)
+                    {
+                        throw signalr_exception("binary isn't supported currently");
+                    }
+                    msg = response.extract_string().get();
+
                     callback(msg, nullptr);
                 }
                 catch (...)

src/signalrclient/default_websocket_client.h

@@ -20,9 +20,9 @@ namespace signalr
     public:
         explicit default_websocket_client(const signalr_client_config& signalr_client_config = {}) noexcept;
 
-        void start(const std::string& url, transfer_format format, std::function<void(std::exception_ptr)> callback);
+        void start(const std::string& url, std::function<void(std::exception_ptr)> callback);
         void stop(std::function<void(std::exception_ptr)> callback);
-        void send(const std::string& payload, std::function<void(std::exception_ptr)> callback);
+        void send(const std::string& payload, transfer_format transfer_format, std::function<void(std::exception_ptr)> callback);
         void receive(std::function<void(const std::string&, std::exception_ptr)> callback);
     private:
         web::websockets::client::websocket_client m_underlying_client;