Negotiate version support (#16)

BrennanConroy · web-flow · commit d0f42f35e743 · 2019-11-19T13:03:31.000-08:00
diff --git a/src/signalrclient/connection_impl.cpp b/src/signalrclient/connection_impl.cpp
@@ -136,7 +136,7 @@ namespace signalr
         const auto& token = m_disconnect_cts;
 
         negotiate::negotiate(*m_http_client, url, m_signalr_client_config,
-            [callback, weak_connection, redirect_count, token, url](const negotiation_response& response, std::exception_ptr exception)
+            [callback, weak_connection, redirect_count, token, url](negotiation_response&& response, std::exception_ptr exception)
             {
                 auto connection = weak_connection.lock();
                 if (!connection)
@@ -182,6 +182,7 @@ namespace signalr
                 }
 
                 connection->m_connection_id = std::move(response.connectionId);
+                connection->m_connection_token = std::move(response.connectionToken);
 
                 // TODO: fallback logic
 
@@ -407,7 +408,7 @@ namespace signalr
     void connection_impl::send_connect_request(const std::shared_ptr<transport>& transport, const std::string& url, std::function<void(std::exception_ptr)> callback)
     {
         auto logger = m_logger;
-        auto query_string = "id=" + m_connection_id;
+        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)
diff --git a/src/signalrclient/connection_impl.h b/src/signalrclient/connection_impl.h
@@ -61,6 +61,7 @@ namespace signalr
         std::mutex m_stop_lock;
         cancellation_token m_start_completed_event;
         std::string m_connection_id;
+        std::string m_connection_token;
         std::unique_ptr<http_client> m_http_client;
 
         connection_impl(const std::string& url, trace_level trace_level, const std::shared_ptr<log_writer>& log_writer,
diff --git a/src/signalrclient/negotiate.cpp b/src/signalrclient/negotiate.cpp
@@ -10,14 +10,17 @@ namespace signalr
 {
     namespace negotiate
     {
+        const int negotiate_version = 1;
+
         void negotiate(http_client& client, const std::string& base_url,
             const signalr_client_config& config,
-            std::function<void(const negotiation_response&, std::exception_ptr)> callback) noexcept
+            std::function<void(negotiation_response&&, std::exception_ptr)> callback) noexcept
         {
             std::string negotiate_url;
             try
             {
                 negotiate_url = url_builder::build_negotiate(base_url);
+                negotiate_url = url_builder::add_query_string(negotiate_url, "negotiateVersion=" + std::to_string(negotiate_version));
             }
             catch (...)
             {
@@ -58,15 +61,31 @@ namespace signalr
                     if (negotiation_response_json.has_field(_XPLATSTR("error")))
                     {
                         response.error = utility::conversions::to_utf8string(negotiation_response_json[_XPLATSTR("error")].as_string());
-                        callback(response, nullptr);
+                        callback(std::move(response), nullptr);
                         return;
                     }
 
+                    int server_negotiate_version = 0;
+                    if (negotiation_response_json.has_field(_XPLATSTR("negotiateVersion")))
+                    {
+                        server_negotiate_version = negotiation_response_json[_XPLATSTR("negotiateVersion")].as_integer();
+                    }
+
                     if (negotiation_response_json.has_field(_XPLATSTR("connectionId")))
                     {
                         response.connectionId = utility::conversions::to_utf8string(negotiation_response_json[_XPLATSTR("connectionId")].as_string());
                     }
 
+                    if (negotiation_response_json.has_field(_XPLATSTR("connectionToken")))
+                    {
+                        response.connectionToken = utility::conversions::to_utf8string(negotiation_response_json[_XPLATSTR("connectionToken")].as_string());
+                    }
+
+                    if (server_negotiate_version <= 0)
+                    {
+                        response.connectionToken = response.connectionId;
+                    }
+
                     if (negotiation_response_json.has_field(_XPLATSTR("availableTransports")))
                     {
                         for (auto transportData : negotiation_response_json[_XPLATSTR("availableTransports")].as_array())
@@ -100,7 +119,7 @@ namespace signalr
                         return;
                     }
 
-                    callback(response, nullptr);
+                    callback(std::move(response), nullptr);
                 }
                 catch (...)
                 {
diff --git a/src/signalrclient/negotiate.h b/src/signalrclient/negotiate.h
@@ -13,6 +13,6 @@ namespace signalr
     {
         void negotiate(http_client& client, const std::string& base_url,
             const signalr_client_config& signalr_client_config,
-            std::function<void(const negotiation_response&, std::exception_ptr)> callback) noexcept;
+            std::function<void(negotiation_response&&, std::exception_ptr)> callback) noexcept;
     }
 }
diff --git a/src/signalrclient/negotiation_response.h b/src/signalrclient/negotiation_response.h
@@ -14,6 +14,7 @@ namespace signalr
     struct negotiation_response
     {
         std::string connectionId;
+        std::string connectionToken;
         std::vector<available_transport> availableTransports;
         std::string url;
         std::string accessToken;
diff --git a/src/signalrclient/url_builder.cpp b/src/signalrclient/url_builder.cpp
@@ -49,5 +49,11 @@ namespace signalr
             auto builder = build_uri(base_url, "", query_string);
             return convert_to_websocket_url(builder, transport).to_string();
         }
+
+        std::string add_query_string(const std::string& base_url, const std::string& query_string)
+        {
+            uri_builder builder(base_url);
+            return builder.append_query(query_string).to_string();
+        }
     }
 }
diff --git a/src/signalrclient/url_builder.h b/src/signalrclient/url_builder.h
@@ -11,5 +11,6 @@ namespace signalr
     {
         std::string build_negotiate(const std::string& base_url);
         std::string build_connect(const std::string& base_url, transport_type transport, const std::string& query_string);
+        std::string add_query_string(const std::string& base_url, const std::string& query_string);
     }
 }
diff --git a/test/signalrclienttests/CMakeLists.txt b/test/signalrclienttests/CMakeLists.txt
@@ -1,13 +1,14 @@
 
 
-set (SOURCES 
+set (SOURCES
   callback_manager_tests.cpp
   case_insensitive_comparison_utils_tests.cpp
   connection_impl_tests.cpp
   hub_connection_impl_tests.cpp
   hub_exception_tests.cpp
   logger_tests.cpp
   memory_log_writer.cpp
+  negotiate_tests.cpp
   signalrclienttests.cpp
   stdafx.cpp
   test_http_client.cpp
@@ -20,7 +21,7 @@ set (SOURCES
 
 include_directories(
     ../../src/signalrclient)
- 
+
 add_executable (signalrclienttests ${SOURCES})
 
 target_link_libraries(signalrclienttests gtest signalrclient ${CPPREST_LIB})
diff --git a/test/signalrclienttests/connection_impl_tests.cpp b/test/signalrclienttests/connection_impl_tests.cpp
@@ -891,6 +891,92 @@ TEST(connection_impl_start, negotiate_redirect_uses_own_query_string)
     ASSERT_EQ("customQuery=1&id=f7707523-307d-4cba-9abf-3eef701241e8", query_string);
 }
 
+TEST(connection_impl_start, negotiate_with_negotiateVersion_uses_connectionToken)
+{
+    std::shared_ptr<log_writer> writer(std::make_shared<memory_log_writer>());
+    std::string query_string;
+
+    auto websocket_client = create_test_websocket_client(
+        /* receive function */ [](std::function<void(std::string, std::exception_ptr)> callback) { callback("", std::make_exception_ptr(std::runtime_error("should not be invoked"))); },
+        /* send function */ [](const std::string&, std::function<void(std::exception_ptr)> callback) { callback(std::make_exception_ptr(std::runtime_error("should not be invoked"))); },
+        /* connect function */[&query_string](const std::string& url, std::function<void(std::exception_ptr)> callback)
+        {
+            query_string = url.substr(url.find('?') + 1);
+            callback(std::make_exception_ptr(std::runtime_error("connecting failed")));
+        });
+
+    auto http_client = std::make_unique<test_http_client>([](const std::string& url, http_request)
+        {
+            std::string response_body = "";
+            if (url.find("/negotiate") != std::string::npos)
+            {
+                response_body = "{\"connectionId\" : \"f7707523-307d-4cba-9abf-3eef701241e8\", "
+                    "\"connectionToken\":\"1122334455\", \"negotiateVersion\": 1, "
+                    "\"availableTransports\" : [ { \"transport\": \"WebSockets\", \"transferFormats\": [ \"Text\", \"Binary\" ] } ] }";
+            }
+
+            return http_response{ 200, response_body };
+        });
+
+    auto connection = connection_impl::create(create_uri(), trace_level::errors, writer, std::move(http_client), std::make_unique<test_transport_factory>(websocket_client));
+
+    auto mre = manual_reset_event<void>();
+    connection->start([&mre](std::exception_ptr exception)
+        {
+            mre.set(exception);
+        });
+
+    try
+    {
+        mre.get();
+        ASSERT_TRUE(false);
+    }
+    catch (const std::runtime_error & ex)
+    {
+        ASSERT_STREQ("connecting failed", ex.what());
+    }
+
+    ASSERT_EQ("id=1122334455", query_string);
+}
+
+TEST(connection_impl_start, correct_connection_id_returned_with_negotiateVersion_one)
+{
+    std::shared_ptr<log_writer> writer(std::make_shared<memory_log_writer>());
+
+    auto websocket_client = create_test_websocket_client(
+        /* receive function */ [](std::function<void(std::string, std::exception_ptr)> callback) { callback("", std::make_exception_ptr(std::runtime_error("should not be invoked"))); },
+        /* send function */ [](const std::string&, std::function<void(std::exception_ptr)> callback) { callback(std::make_exception_ptr(std::runtime_error("should not be invoked"))); },
+        /* connect function */[](const std::string& url, std::function<void(std::exception_ptr)> callback)
+        {
+            callback(nullptr);
+        });
+
+    auto http_client = std::make_unique<test_http_client>([](const std::string& url, http_request)
+        {
+            std::string response_body = "";
+            if (url.find("/negotiate") != std::string::npos)
+            {
+                response_body = "{\"connectionId\" : \"f7707523-307d-4cba-9abf-3eef701241e8\", "
+                    "\"connectionToken\":\"1122334455\", \"negotiateVersion\": 1, "
+                    "\"availableTransports\" : [ { \"transport\": \"WebSockets\", \"transferFormats\": [ \"Text\", \"Binary\" ] } ] }";
+            }
+
+            return http_response{ 200, response_body };
+        });
+
+    auto connection = connection_impl::create(create_uri(), trace_level::errors, writer, std::move(http_client), std::make_unique<test_transport_factory>(websocket_client));
+
+    auto mre = manual_reset_event<void>();
+    connection->start([&mre](std::exception_ptr exception)
+        {
+            mre.set(exception);
+        });
+
+    mre.get();
+
+    ASSERT_EQ("f7707523-307d-4cba-9abf-3eef701241e8", connection->get_connection_id());
+}
+
 TEST(connection_impl_start, start_fails_if_connect_request_times_out)
 {
     std::shared_ptr<log_writer> writer(std::make_shared<memory_log_writer>());
diff --git a/test/signalrclienttests/hub_connection_impl_tests.cpp b/test/signalrclienttests/hub_connection_impl_tests.cpp
@@ -49,7 +49,7 @@ TEST(url, negotiate_appended_to_url)
         }
         catch (const std::exception&) {}
 
-        ASSERT_EQ("http://fakeuri/negotiate", requested_url);
+        ASSERT_EQ("http://fakeuri/negotiate?negotiateVersion=1", requested_url);
     }
 }
 
diff --git a/test/signalrclienttests/negotiate_tests.cpp b/test/signalrclienttests/negotiate_tests.cpp
@@ -4,6 +4,7 @@
 #include "stdafx.h"
 #include "negotiate.h"
 #include "test_http_client.h"
+#include "test_utils.h"
 
 using namespace signalr;
 
@@ -20,9 +21,15 @@ TEST(negotiate, request_created_with_correct_url)
         return http_response(200, response_body);
     });
 
-    negotiate::negotiate(http_client, "http://fake/signalr").get();
+    auto mre = manual_reset_event<void>();
+    negotiate::negotiate(http_client, "http://fake/signalr", signalr_client_config(),
+        [&mre](signalr::negotiation_response&& response, std::exception_ptr exception)
+        {
+            mre.set();
+        });
 
-    ASSERT_EQ("http://fake/signalr/negotiate", requested_url);
+    mre.get();
+    ASSERT_EQ("http://fake/signalr/negotiate?negotiateVersion=1", requested_url);
 }
 
 TEST(negotiate, negotiation_request_sent_and_response_serialized)
@@ -37,9 +44,17 @@ TEST(negotiate, negotiation_request_sent_and_response_serialized)
         return http_response(200, response_body);
     });
 
-    auto response = negotiate::negotiate(request_factory, "http://fake/signalr").get();
+    auto mre = manual_reset_event<negotiation_response>();
+    negotiate::negotiate(request_factory, "http://fake/signalr", signalr_client_config(),
+        [&mre](negotiation_response&& response, std::exception_ptr exception)
+        {
+            mre.set(response);
+        });
+
+    auto response = mre.get();
 
     ASSERT_EQ("f7707523-307d-4cba-9abf-3eef701241e8", response.connectionId);
+    ASSERT_EQ("f7707523-307d-4cba-9abf-3eef701241e8", response.connectionToken);
     ASSERT_EQ(2u, response.availableTransports.size());
     ASSERT_EQ(2u, response.availableTransports[0].transfer_formats.size());
     ASSERT_EQ("Text", response.availableTransports[0].transfer_formats[0]);
@@ -59,8 +74,69 @@ TEST(negotiate, negotiation_response_with_redirect)
         return http_response(200, response_body);
     });
 
-    auto response = negotiate::negotiate(request_factory, "http://fake/signalr").get();
+    auto mre = manual_reset_event<negotiation_response>();
+    negotiate::negotiate(request_factory, "http://fake/signalr", signalr_client_config(),
+        [&mre](negotiation_response&& response, std::exception_ptr exception)
+        {
+            mre.set(response);
+        });
+
+    auto response = mre.get();
 
     ASSERT_EQ("http://redirect", response.url);
     ASSERT_EQ("secret", response.accessToken);
 }
+
+TEST(negotiate, negotiation_response_with_negotiateVersion)
+{
+    auto request_factory = test_http_client([](const std::string&, http_request request)
+        {
+            std::string response_body(
+                "{\"connectionId\" : \"f7707523-307d-4cba-9abf-3eef701241e8\", "
+                "\"negotiateVersion\": 1, "
+                "\"connectionToken\": \"42\", "
+                "\"availableTransports\" : [ { \"transport\": \"WebSockets\", \"transferFormats\": [ \"Text\", \"Binary\" ] },"
+                "{ \"transport\": \"ServerSentEvents\", \"transferFormats\": [ \"Text\" ] } ] }");
+
+            return http_response(200, response_body);
+        });
+
+    auto mre = manual_reset_event<negotiation_response>();
+    negotiate::negotiate(request_factory, "http://fake/signalr", signalr_client_config(),
+        [&mre](negotiation_response&& response, std::exception_ptr exception)
+        {
+            mre.set(response);
+        });
+
+    auto response = mre.get();
+
+    ASSERT_EQ("42", response.connectionToken);
+    ASSERT_EQ("f7707523-307d-4cba-9abf-3eef701241e8", response.connectionId);
+}
+
+TEST(negotiate, negotiation_response_with_future_negotiateVersion)
+{
+    auto request_factory = test_http_client([](const std::string&, http_request request)
+        {
+            std::string response_body(
+                "{\"connectionId\" : \"f7707523-307d-4cba-9abf-3eef701241e8\", "
+                "\"negotiateVersion\": 99, "
+                "\"connectionToken\": \"42\", "
+                "\"availableTransports\" : [ { \"transport\": \"WebSockets\", \"transferFormats\": [ \"Text\", \"Binary\" ] },"
+                "{ \"transport\": \"ServerSentEvents\", \"transferFormats\": [ \"Text\" ] } ] }");
+
+            return http_response(200, response_body);
+        });
+
+    auto mre = manual_reset_event<negotiation_response>();
+    negotiate::negotiate(request_factory, "http://fake/signalr", signalr_client_config(),
+        [&mre](negotiation_response&& response, std::exception_ptr exception)
+        {
+            mre.set(response);
+        });
+
+    auto response = mre.get();
+
+    ASSERT_EQ("42", response.connectionToken);
+    ASSERT_EQ("f7707523-307d-4cba-9abf-3eef701241e8", response.connectionId);
+}

Original file line number	Diff line number	Diff line change
`@@ -136,7 +136,7 @@ namespace signalr`
`136`	`136`	`const auto& token = m_disconnect_cts;`
`137`	`137`
`138`	`138`	`negotiate::negotiate(*m_http_client, url, m_signalr_client_config,`
`139`		`- [callback, weak_connection, redirect_count, token, url](const negotiation_response& response, std::exception_ptr exception)`
	`139`	`+ [callback, weak_connection, redirect_count, token, url](negotiation_response&& response, std::exception_ptr exception)`
`140`	`140`	`{`
`141`	`141`	`auto connection = weak_connection.lock();`
`142`	`142`	`if (!connection)`
`@@ -182,6 +182,7 @@ namespace signalr`
`182`	`182`	`}`
`183`	`183`
`184`	`184`	`connection->m_connection_id = std::move(response.connectionId);`
	`185`	`+ connection->m_connection_token = std::move(response.connectionToken);`
`185`	`186`
`186`	`187`	`// TODO: fallback logic`
`187`	`188`
`@@ -407,7 +408,7 @@ namespace signalr`
`407`	`408`	`void connection_impl::send_connect_request(const std::shared_ptr<transport>& transport, const std::string& url, std::function<void(std::exception_ptr)> callback)`
`408`	`409`	`{`
`409`	`410`	`auto logger = m_logger;`
`410`		`- auto query_string = "id=" + m_connection_id;`
	`411`	`+ auto query_string = "id=" + m_connection_token;`
`411`	`412`	`auto connect_url = url_builder::build_connect(url, transport->get_transport_type(), query_string);`
`412`	`413`
`413`	`414`	`transport->start(connect_url, transfer_format::text, [callback, logger](std::exception_ptr exception)`
Original file line number	Diff line number	Diff line change
`@@ -10,14 +10,17 @@ namespace signalr`
`10`	`10`	`{`
`11`	`11`	`namespace negotiate`
`12`	`12`	`{`
	`13`	`+ const int negotiate_version = 1;`
	`14`	`+`
`13`	`15`	`void negotiate(http_client& client, const std::string& base_url,`
`14`	`16`	`const signalr_client_config& config,`
`15`		`- std::function<void(const negotiation_response&, std::exception_ptr)> callback) noexcept`
	`17`	`+ std::function<void(negotiation_response&&, std::exception_ptr)> callback) noexcept`
`16`	`18`	`{`
`17`	`19`	`std::string negotiate_url;`
`18`	`20`	`try`
`19`	`21`	`{`
`20`	`22`	`negotiate_url = url_builder::build_negotiate(base_url);`
	`23`	`+ negotiate_url = url_builder::add_query_string(negotiate_url, "negotiateVersion=" + std::to_string(negotiate_version));`
`21`	`24`	`}`
`22`	`25`	`catch (...)`
`23`	`26`	`{`
`@@ -58,15 +61,31 @@ namespace signalr`
`58`	`61`	`if (negotiation_response_json.has_field(_XPLATSTR("error")))`
`59`	`62`	`{`
`60`	`63`	`response.error = utility::conversions::to_utf8string(negotiation_response_json[_XPLATSTR("error")].as_string());`
`61`		`- callback(response, nullptr);`
	`64`	`+ callback(std::move(response), nullptr);`
`62`	`65`	`return;`
`63`	`66`	`}`
`64`	`67`
	`68`	`+ int server_negotiate_version = 0;`
	`69`	`+ if (negotiation_response_json.has_field(_XPLATSTR("negotiateVersion")))`
	`70`	`+ {`
	`71`	`+ server_negotiate_version = negotiation_response_json[_XPLATSTR("negotiateVersion")].as_integer();`
	`72`	`+ }`
	`73`	`+`
`65`	`74`	`if (negotiation_response_json.has_field(_XPLATSTR("connectionId")))`
`66`	`75`	`{`
`67`	`76`	`response.connectionId = utility::conversions::to_utf8string(negotiation_response_json[_XPLATSTR("connectionId")].as_string());`
`68`	`77`	`}`
`69`	`78`
	`79`	`+ if (negotiation_response_json.has_field(_XPLATSTR("connectionToken")))`
	`80`	`+ {`
	`81`	`+ response.connectionToken = utility::conversions::to_utf8string(negotiation_response_json[_XPLATSTR("connectionToken")].as_string());`
	`82`	`+ }`
	`83`	`+`
	`84`	`+ if (server_negotiate_version <= 0)`
	`85`	`+ {`
	`86`	`+ response.connectionToken = response.connectionId;`
	`87`	`+ }`
	`88`	`+`
`70`	`89`	`if (negotiation_response_json.has_field(_XPLATSTR("availableTransports")))`
`71`	`90`	`{`
`72`	`91`	`for (auto transportData : negotiation_response_json[_XPLATSTR("availableTransports")].as_array())`
`@@ -100,7 +119,7 @@ namespace signalr`
`100`	`119`	`return;`
`101`	`120`	`}`
`102`	`121`
`103`		`- callback(response, nullptr);`
	`122`	`+ callback(std::move(response), nullptr);`
`104`	`123`	`}`
`105`	`124`	`catch (...)`
`106`	`125`	`{`
Original file line number	Diff line number	Diff line change
`@@ -13,6 +13,6 @@ namespace signalr`
`13`	`13`	`{`
`14`	`14`	`void negotiate(http_client& client, const std::string& base_url,`
`15`	`15`	`const signalr_client_config& signalr_client_config,`
`16`		`- std::function<void(const negotiation_response&, std::exception_ptr)> callback) noexcept;`
	`16`	`+ std::function<void(negotiation_response&&, std::exception_ptr)> callback) noexcept;`
`17`	`17`	`}`
`18`	`18`	`}`
Original file line number	Diff line number	Diff line change
`@@ -14,6 +14,7 @@ namespace signalr`
`14`	`14`	`struct negotiation_response`
`15`	`15`	`{`
`16`	`16`	`std::string connectionId;`
	`17`	`+ std::string connectionToken;`
`17`	`18`	`std::vector<available_transport> availableTransports;`
`18`	`19`	`std::string url;`
`19`	`20`	`std::string accessToken;`
Original file line number	Diff line number	Diff line change
`@@ -49,5 +49,11 @@ namespace signalr`
`49`	`49`	`auto builder = build_uri(base_url, "", query_string);`
`50`	`50`	`return convert_to_websocket_url(builder, transport).to_string();`
`51`	`51`	`}`
	`52`	`+`
	`53`	`+ std::string add_query_string(const std::string& base_url, const std::string& query_string)`
	`54`	`+ {`
	`55`	`+ uri_builder builder(base_url);`
	`56`	`+ return builder.append_query(query_string).to_string();`
	`57`	`+ }`
`52`	`58`	`}`
`53`	`59`	`}`
Original file line number	Diff line number	Diff line change
`@@ -11,5 +11,6 @@ namespace signalr`
`11`	`11`	`{`
`12`	`12`	`std::string build_negotiate(const std::string& base_url);`
`13`	`13`	`std::string build_connect(const std::string& base_url, transport_type transport, const std::string& query_string);`
	`14`	`+ std::string add_query_string(const std::string& base_url, const std::string& query_string);`
`14`	`15`	`}`
`15`	`16`	`}`
Original file line number	Diff line number	Diff line change
`@@ -49,7 +49,7 @@ TEST(url, negotiate_appended_to_url)`
`49`	`49`	`}`
`50`	`50`	`catch (const std::exception&) {}`
`51`	`51`
`52`		`- ASSERT_EQ("http://fakeuri/negotiate", requested_url);`
	`52`	`+ ASSERT_EQ("http://fakeuri/negotiate?negotiateVersion=1", requested_url);`
`53`	`53`	`}`
`54`	`54`	`}`
`55`	`55`