Commit 5985e72c authored by Samir Halilcevic's avatar Samir Halilcevic

Integrate review feedback

parent f81216be
...@@ -120,18 +120,26 @@ ptrdiff_t mock_stream_transport::handle_input() { ...@@ -120,18 +120,26 @@ ptrdiff_t mock_stream_transport::handle_input() {
// -- mock_web_socket_app ------------------------------------------------------ // -- mock_web_socket_app ------------------------------------------------------
mock_web_socket_app::mock_web_socket_app(bool behave_as_server) mock_web_socket_app::mock_web_socket_app(bool request_messages_on_start)
: behave_as_server(behave_as_server) { : request_messages_on_start(request_messages_on_start) {
// nop // nop
} }
caf::error mock_web_socket_app::start(caf::net::web_socket::lower_layer* ll) { caf::error mock_web_socket_app::start(caf::net::web_socket::lower_layer* ll) {
down = ll; down = ll;
if (behave_as_server) if (request_messages_on_start)
down->request_messages(); down->request_messages();
return caf::none; return caf::none;
} }
void mock_web_socket_app::prepare_send() {
// nop
}
bool mock_web_socket_app::done_sending() {
return true;
}
caf::error caf::error
mock_web_socket_app::accept(const caf::net::http::request_header& hdr) { mock_web_socket_app::accept(const caf::net::http::request_header& hdr) {
// Store the request information in cfg to evaluate them later. // Store the request information in cfg to evaluate them later.
...@@ -151,14 +159,8 @@ mock_web_socket_app::accept(const caf::net::http::request_header& hdr) { ...@@ -151,14 +159,8 @@ mock_web_socket_app::accept(const caf::net::http::request_header& hdr) {
} }
void mock_web_socket_app::abort(const caf::error& reason) { void mock_web_socket_app::abort(const caf::error& reason) {
if (expect_abort_) { abort_reason = reason;
has_aborted = true;
err = reason;
expect_abort_ = false;
down->shutdown(reason); down->shutdown(reason);
} else {
CAF_FAIL("app::abort called: " << reason);
}
} }
ptrdiff_t mock_web_socket_app::consume_text(std::string_view text) { ptrdiff_t mock_web_socket_app::consume_text(std::string_view text) {
......
...@@ -11,6 +11,10 @@ ...@@ -11,6 +11,10 @@
#include "caf/string_view.hpp" #include "caf/string_view.hpp"
#include "caf/test/bdd_dsl.hpp" #include "caf/test/bdd_dsl.hpp"
/// Implements a trivial transport layer that stores the contents of all
/// received frames in a respective output buffer, it can propagate the content
/// of the input buffer to the upper layer, and switch protocols if configured
/// so.
class mock_stream_transport : public caf::net::octet_stream::lower_layer { class mock_stream_transport : public caf::net::octet_stream::lower_layer {
public: public:
// -- member types ----------------------------------------------------------- // -- member types -----------------------------------------------------------
...@@ -106,16 +110,28 @@ private: ...@@ -106,16 +110,28 @@ private:
caf::net::multiplexer* mpx_; caf::net::multiplexer* mpx_;
}; };
/// Tag used to configure mock_web_socket_app to request messages on start
struct request_messages_on_start_t {};
constexpr auto request_messages_on_start = request_messages_on_start_t{};
/// Implements a trivial WebSocket application that stores the contents of all
/// received messages in respective text/binary buffers. It can take both
/// roles, server and client, request messages and track whether the
/// lower layer was aborted.
class mock_web_socket_app : public caf::net::web_socket::upper_layer::server { class mock_web_socket_app : public caf::net::web_socket::upper_layer::server {
public: public:
// -- constructor ------------------------------------------------------------ // -- constructor ------------------------------------------------------------
mock_web_socket_app(bool behave_as_server); explicit mock_web_socket_app(bool request_messages_on_start);
// -- factories -------------------------------------------------------------- // -- factories --------------------------------------------------------------
static auto make(bool behave_as_server = false) { static auto make(request_messages_on_start_t) {
return std::make_unique<mock_web_socket_app>(behave_as_server); return std::make_unique<mock_web_socket_app>(true);
}
static auto make() {
return std::make_unique<mock_web_socket_app>(false);
} }
// -- initialization --------------------------------------------------------- // -- initialization ---------------------------------------------------------
...@@ -126,13 +142,9 @@ public: ...@@ -126,13 +142,9 @@ public:
caf::error accept(const caf::net::http::request_header& hdr) override; caf::error accept(const caf::net::http::request_header& hdr) override;
void prepare_send() override { void prepare_send() override;
// nop
}
bool done_sending() override { bool done_sending() override;
return true;
}
void abort(const caf::error& reason) override; void abort(const caf::error& reason) override;
...@@ -140,8 +152,8 @@ public: ...@@ -140,8 +152,8 @@ public:
ptrdiff_t consume_binary(caf::byte_span bytes) override; ptrdiff_t consume_binary(caf::byte_span bytes) override;
void expect_abort() { bool has_aborted() const noexcept {
expect_abort_ = true; return !abort_reason.empty();
} }
// -- member variables ------------------------------------------------------- // -- member variables -------------------------------------------------------
...@@ -154,13 +166,9 @@ public: ...@@ -154,13 +166,9 @@ public:
caf::settings cfg; caf::settings cfg;
bool behave_as_server = false; bool request_messages_on_start = false;
caf::error err;
bool has_aborted = false;
bool expect_abort_ = false; caf::error abort_reason;
}; };
// Drop-in replacement for std::barrier (based on the TS API as of 2020). // Drop-in replacement for std::barrier (based on the TS API as of 2020).
......
...@@ -57,6 +57,7 @@ SCENARIO("the client performs the WebSocket handshake on startup") { ...@@ -57,6 +57,7 @@ SCENARIO("the client performs the WebSocket handshake on startup") {
GIVEN("valid WebSocket handshake data") { GIVEN("valid WebSocket handshake data") {
WHEN("starting a WebSocket client") { WHEN("starting a WebSocket client") {
auto app = mock_web_socket_app::make(); auto app = mock_web_socket_app::make();
auto app_ptr = app.get();
auto ws = net::web_socket::client::make(make_handshake(), std::move(app)); auto ws = net::web_socket::client::make(make_handshake(), std::move(app));
auto uut = mock_stream_transport::make(std::move(ws)); auto uut = mock_stream_transport::make(std::move(ws));
THEN("the client sends its HTTP request when initializing it") { THEN("the client sends its HTTP request when initializing it") {
...@@ -68,6 +69,9 @@ SCENARIO("the client performs the WebSocket handshake on startup") { ...@@ -68,6 +69,9 @@ SCENARIO("the client performs the WebSocket handshake on startup") {
CHECK_EQ(uut->handle_input(), CHECK_EQ(uut->handle_input(),
static_cast<ptrdiff_t>(http_response.size())); static_cast<ptrdiff_t>(http_response.size()));
} }
AND("the client did not abort") {
CHECK(!app_ptr->has_aborted());
}
} }
} }
} }
...@@ -54,6 +54,9 @@ SCENARIO("the client sends a ping and receives a pong response") { ...@@ -54,6 +54,9 @@ SCENARIO("the client sends a ping and receives a pong response") {
pong_frame); pong_frame);
CHECK_EQ(transport->output_buffer(), pong_frame); CHECK_EQ(transport->output_buffer(), pong_frame);
} }
AND("the client did not abort") {
CHECK(!app->has_aborted());
}
} }
transport->output_buffer().clear(); transport->output_buffer().clear();
WHEN("the client sends a ping with some data") { WHEN("the client sends a ping with some data") {
...@@ -68,6 +71,9 @@ SCENARIO("the client sends a ping and receives a pong response") { ...@@ -68,6 +71,9 @@ SCENARIO("the client sends a ping and receives a pong response") {
pong_frame); pong_frame);
CHECK_EQ(transport->output_buffer(), pong_frame); CHECK_EQ(transport->output_buffer(), pong_frame);
} }
AND("the client did not abort") {
CHECK(!app->has_aborted());
}
} }
transport->output_buffer().clear(); transport->output_buffer().clear();
WHEN("the client sends a ping with the maximum allowed 125 bytes") { WHEN("the client sends a ping with the maximum allowed 125 bytes") {
...@@ -82,6 +88,9 @@ SCENARIO("the client sends a ping and receives a pong response") { ...@@ -82,6 +88,9 @@ SCENARIO("the client sends a ping and receives a pong response") {
pong_frame); pong_frame);
CHECK_EQ(transport->output_buffer(), pong_frame); CHECK_EQ(transport->output_buffer(), pong_frame);
} }
AND("the client did not abort") {
CHECK(!app->has_aborted());
}
} }
} }
} }
...@@ -95,6 +104,7 @@ TEST_CASE("calling shutdown with protocol_error sets status in close header") { ...@@ -95,6 +104,7 @@ TEST_CASE("calling shutdown with protocol_error sets status in close header") {
auto status = (std::to_integer<int>(transport->output_buffer().at(2)) << 8) auto status = (std::to_integer<int>(transport->output_buffer().at(2)) << 8)
+ std::to_integer<int>(transport->output_buffer().at(3)); + std::to_integer<int>(transport->output_buffer().at(3));
CHECK_EQ(status, static_cast<int>(net::web_socket::status::protocol_error)); CHECK_EQ(status, static_cast<int>(net::web_socket::status::protocol_error));
CHECK(!app->has_aborted());
} }
SCENARIO("the client sends an invalid ping that closes the connection") { SCENARIO("the client sends an invalid ping that closes the connection") {
...@@ -106,11 +116,10 @@ SCENARIO("the client sends an invalid ping that closes the connection") { ...@@ -106,11 +116,10 @@ SCENARIO("the client sends an invalid ping that closes the connection") {
ping_frame); ping_frame);
transport->push(ping_frame); transport->push(ping_frame);
THEN("the server aborts the application") { THEN("the server aborts the application") {
app->expect_abort();
CHECK_EQ(transport->handle_input(), 0); CHECK_EQ(transport->handle_input(), 0);
CHECK(app->has_aborted); CHECK(app->has_aborted());
CHECK_EQ(app->err, sec::protocol_error); CHECK_EQ(app->abort_reason, sec::protocol_error);
MESSAGE("Aborted with: " << app->err); MESSAGE("Aborted with: " << app->abort_reason);
} }
AND("the server closes the connection with a protocol error") { AND("the server closes the connection with a protocol error") {
detail::rfc6455::header hdr; detail::rfc6455::header hdr;
......
...@@ -23,7 +23,7 @@ using svec = std::vector<std::string>; ...@@ -23,7 +23,7 @@ using svec = std::vector<std::string>;
struct fixture { struct fixture {
fixture() { fixture() {
using namespace caf::net; using namespace caf::net;
auto app_ptr = mock_web_socket_app::make(/*behave_as_server=*/true); auto app_ptr = mock_web_socket_app::make(request_messages_on_start);
app = app_ptr.get(); app = app_ptr.get();
auto ws_ptr = net::web_socket::server::make(std::move(app_ptr)); auto ws_ptr = net::web_socket::server::make(std::move(app_ptr));
transport = mock_stream_transport::make(std::move(ws_ptr)); transport = mock_stream_transport::make(std::move(ws_ptr));
...@@ -91,9 +91,9 @@ constexpr std::string_view opening_handshake ...@@ -91,9 +91,9 @@ constexpr std::string_view opening_handshake
if (CHECK(holds_alternative<std::string>(app->cfg, key))) \ if (CHECK(holds_alternative<std::string>(app->cfg, key))) \
CHECK_EQ(get<std::string>(app->cfg, key), expected_value); CHECK_EQ(get<std::string>(app->cfg, key), expected_value);
CAF_TEST_FIXTURE_SCOPE(web_socket_tests, fixture) BEGIN_FIXTURE_SCOPE(fixture)
CAF_TEST(applications receive handshake data via config) { TEST_CASE("applications receive handshake data via config") {
transport->push(opening_handshake); transport->push(opening_handshake);
{ {
auto consumed = transport->handle_input(); auto consumed = transport->handle_input();
...@@ -117,9 +117,10 @@ CAF_TEST(applications receive handshake data via config) { ...@@ -117,9 +117,10 @@ CAF_TEST(applications receive handshake data via config) {
using str_map = std::map<std::string, std::string>; using str_map = std::map<std::string, std::string>;
if (auto query = get_as<str_map>(app->cfg, "web-socket.query"); CHECK(query)) if (auto query = get_as<str_map>(app->cfg, "web-socket.query"); CHECK(query))
CHECK_EQ(*query, str_map({{"room"s, "lounge"s}})); CHECK_EQ(*query, str_map({{"room"s, "lounge"s}}));
CHECK(!app->has_aborted());
} }
CAF_TEST(the server responds with an HTTP response on success) { TEST_CASE("the server responds with an HTTP response on success") {
transport->push(opening_handshake); transport->push(opening_handshake);
CHECK_EQ(transport->handle_input(), CHECK_EQ(transport->handle_input(),
static_cast<ptrdiff_t>(opening_handshake.size())); static_cast<ptrdiff_t>(opening_handshake.size()));
...@@ -128,9 +129,10 @@ CAF_TEST(the server responds with an HTTP response on success) { ...@@ -128,9 +129,10 @@ CAF_TEST(the server responds with an HTTP response on success) {
"Upgrade: websocket\r\n" "Upgrade: websocket\r\n"
"Connection: Upgrade\r\n" "Connection: Upgrade\r\n"
"Sec-WebSocket-Accept: s3pPLMBiTxaQ9kYGzzhZRbK+xOo=\r\n\r\n"); "Sec-WebSocket-Accept: s3pPLMBiTxaQ9kYGzzhZRbK+xOo=\r\n\r\n");
CHECK(!app->has_aborted());
} }
CAF_TEST(handshakes may arrive in chunks) { TEST_CASE("handshakes may arrive in chunks") {
svec bufs; svec bufs;
size_t chunk_size = opening_handshake.size() / 3; size_t chunk_size = opening_handshake.size() / 3;
auto i = opening_handshake.begin(); auto i = opening_handshake.begin();
...@@ -146,9 +148,10 @@ CAF_TEST(handshakes may arrive in chunks) { ...@@ -146,9 +148,10 @@ CAF_TEST(handshakes may arrive in chunks) {
transport->push(bufs[2]); transport->push(bufs[2]);
CHECK_EQ(static_cast<size_t>(transport->handle_input()), CHECK_EQ(static_cast<size_t>(transport->handle_input()),
opening_handshake.size()); opening_handshake.size());
CHECK(!app->has_aborted());
} }
CAF_TEST(data may follow the handshake immediately) { TEST_CASE("data may follow the handshake immediately") {
auto hs_bytes = as_bytes(make_span(opening_handshake)); auto hs_bytes = as_bytes(make_span(opening_handshake));
byte_buffer buf{hs_bytes.begin(), hs_bytes.end()}; byte_buffer buf{hs_bytes.begin(), hs_bytes.end()};
rfc6455_append("Hello WebSocket!\n"sv, buf); rfc6455_append("Hello WebSocket!\n"sv, buf);
...@@ -156,18 +159,20 @@ CAF_TEST(data may follow the handshake immediately) { ...@@ -156,18 +159,20 @@ CAF_TEST(data may follow the handshake immediately) {
transport->push(buf); transport->push(buf);
CHECK_EQ(transport->handle_input(), static_cast<ptrdiff_t>(buf.size())); CHECK_EQ(transport->handle_input(), static_cast<ptrdiff_t>(buf.size()));
CHECK_EQ(app->text_input, "Hello WebSocket!\nBye WebSocket!\n"); CHECK_EQ(app->text_input, "Hello WebSocket!\nBye WebSocket!\n");
CHECK(!app->has_aborted());
} }
CAF_TEST(data may arrive later) { TEST_CASE("data may arrive later") {
transport->push(opening_handshake); transport->push(opening_handshake);
CHECK_EQ(transport->handle_input(), CHECK_EQ(transport->handle_input(),
static_cast<ptrdiff_t>(opening_handshake.size())); static_cast<ptrdiff_t>(opening_handshake.size()));
push("Hello WebSocket!\nBye WebSocket!\n"sv); push("Hello WebSocket!\nBye WebSocket!\n"sv);
transport->handle_input(); transport->handle_input();
CHECK_EQ(app->text_input, "Hello WebSocket!\nBye WebSocket!\n"); CHECK_EQ(app->text_input, "Hello WebSocket!\nBye WebSocket!\n");
CHECK(!app->has_aborted());
} }
CAF_TEST(data may arrive fragmented) { TEST_CASE("data may arrive fragmented") {
transport->push(opening_handshake); transport->push(opening_handshake);
CHECK_EQ(transport->handle_input(), CHECK_EQ(transport->handle_input(),
static_cast<ptrdiff_t>(opening_handshake.size())); static_cast<ptrdiff_t>(opening_handshake.size()));
...@@ -180,6 +185,7 @@ CAF_TEST(data may arrive fragmented) { ...@@ -180,6 +185,7 @@ CAF_TEST(data may arrive fragmented) {
transport->push(buf); transport->push(buf);
CHECK_EQ(transport->handle_input(), static_cast<ptrdiff_t>(buf.size())); CHECK_EQ(transport->handle_input(), static_cast<ptrdiff_t>(buf.size()));
CHECK_EQ(app->text_input, "Hello WebSocket!\nBye WebSocket!\n"); CHECK_EQ(app->text_input, "Hello WebSocket!\nBye WebSocket!\n");
CHECK(!app->has_aborted());
} }
CAF_TEST_FIXTURE_SCOPE_END() END_FIXTURE_SCOPE()
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