From 900bf2db5a1b52a036f46ded7645be258410abbc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Mon, 17 Apr 2017 10:36:33 +0300 Subject: [PATCH] MXS-1220: Take the resource handler into use The resource handler system is now usable but it doesn't perform anything useful. Although, this will allows it to be tested for correctness. Minor fixes to HttpResponse output and renaming of functions. --- server/core/adminclient.cc | 25 +++++++++++-------------- server/core/httpresponse.cc | 6 +++--- server/core/maxscale/http.hh | 2 +- server/core/maxscale/httprequest.hh | 28 +++++++++++++++++++++------- server/core/maxscale/httpresponse.hh | 7 ++----- server/core/test/testhttp.cc | 16 ++++++++-------- 6 files changed, 46 insertions(+), 38 deletions(-) diff --git a/server/core/adminclient.cc b/server/core/adminclient.cc index ca34a5f19..d15aa9fe0 100644 --- a/server/core/adminclient.cc +++ b/server/core/adminclient.cc @@ -12,8 +12,6 @@ */ #include "maxscale/adminclient.hh" -#include "maxscale/httprequest.hh" -#include "maxscale/httpresponse.hh" #include #include @@ -24,6 +22,10 @@ #include #include +#include "maxscale/httprequest.hh" +#include "maxscale/httpresponse.hh" +#include "maxscale/resource.hh" + using std::string; using std::stringstream; using mxs::SpinLockGuard; @@ -85,26 +87,21 @@ static bool write_response(int fd, const string& body) void AdminClient::process() { - string request; + string data; atomic_write_int64(&m_last_activity, hkheartbeat); - if (read_request(m_fd, request)) + if (read_request(m_fd, data)) { - string response; - SHttpRequest parser(HttpRequest::parse(request)); + HttpResponse response(HTTP_400_BAD_REQUEST); + SHttpRequest request(HttpRequest::parse(data)); atomic_write_int64(&m_last_activity, hkheartbeat); - if (parser.get()) + if (request.get()) { - /** Valid request */ - response = HttpResponse(parser->get_json_str()).get_response(); - } - else - { - request = HttpResponse("", HTTP_400_BAD_REQUEST).get_response(); + response = resource_handle_request(*request); } - if (!write_response(m_fd, response)) + if (!write_response(m_fd, response.get_response())) { MXS_ERROR("Failed to write response to client: %d, %s", errno, mxs_strerror(errno)); } diff --git a/server/core/httpresponse.cc b/server/core/httpresponse.cc index 18bf34bc1..68d9c5d36 100644 --- a/server/core/httpresponse.cc +++ b/server/core/httpresponse.cc @@ -23,11 +23,11 @@ using std::string; using std::stringstream; -HttpResponse::HttpResponse(string response, enum http_code code): +HttpResponse::HttpResponse(enum http_code code, string response): m_body(response), m_code(code) { - m_headers["Date"] = get_http_date(); + m_headers["Date"] = http_get_date(); // TODO: Add proper modification timestamps m_headers["Last-Modified"] = m_headers["Date"]; @@ -54,7 +54,7 @@ void HttpResponse::add_header(string name, string value) string HttpResponse::get_response() const { stringstream response; - response << "HTTP/1.1 " << http_code_to_string(m_code); + response << "HTTP/1.1 " << http_code_to_string(m_code) << "\r\n"; for (map::const_iterator it = m_headers.begin(); it != m_headers.end(); it++) diff --git a/server/core/maxscale/http.hh b/server/core/maxscale/http.hh index 2f3ff5e14..4f8b9d1d4 100644 --- a/server/core/maxscale/http.hh +++ b/server/core/maxscale/http.hh @@ -264,7 +264,7 @@ static inline const char* http_code_to_string(enum http_code code) * * @return The RFC 1123 compliant date */ -static inline string get_http_date() +static inline string http_get_date() { time_t now = time(NULL); struct tm tm; diff --git a/server/core/maxscale/httprequest.hh b/server/core/maxscale/httprequest.hh index 52aaa5a60..69c086bd2 100644 --- a/server/core/maxscale/httprequest.hh +++ b/server/core/maxscale/httprequest.hh @@ -18,6 +18,7 @@ #include #include #include +#include #include #include @@ -132,24 +133,37 @@ public: } /** - * @brief Get request resource + * @brief Get complete request URI * - * @return The request resource + * @return The complete request URI */ - const string& get_resource() const + const string& get_uri() const { return m_resource; } /** - * @brief Get request resource parts + * @brief Get URI part * - * @return The request resource split into parts + * @param idx Zero indexed part number in URI + * + * @return The request URI part or empty string if no part was found */ - const deque& get_resource_parts() const + const string uri_part(uint32_t idx) const { - return m_resource_parts; + return m_resource_parts.size() > idx ? m_resource_parts[idx] : ""; } + + /** + * @brief Return how many parts are in the URI + * + * @return Number of URI parts + */ + size_t uri_part_count() const + { + return m_resource_parts.size(); + } + private: HttpRequest(); HttpRequest(const HttpRequest&); diff --git a/server/core/maxscale/httpresponse.hh b/server/core/maxscale/httpresponse.hh index 3f03f9e40..9a0d71ccf 100644 --- a/server/core/maxscale/httpresponse.hh +++ b/server/core/maxscale/httpresponse.hh @@ -39,7 +39,7 @@ public: * @param response Response body * @param code HTTP return code */ - HttpResponse(string response = "", enum http_code code = HTTP_200_OK); + HttpResponse(enum http_code code = HTTP_200_OK, string response = ""); ~HttpResponse(); @@ -59,10 +59,7 @@ public: string get_response() const; private: - HttpResponse(const HttpResponse&); - HttpResponse& operator = (const HttpResponse&); - string m_body; /**< Message body */ map m_headers; /**< Message headers */ - http_code m_code; /**< The HTTP code for the response */ + enum http_code m_code; /**< The HTTP code for the response */ }; diff --git a/server/core/test/testhttp.cc b/server/core/test/testhttp.cc index 16a81ed22..1fa68cbcc 100644 --- a/server/core/test/testhttp.cc +++ b/server/core/test/testhttp.cc @@ -96,9 +96,9 @@ int test_basic() ss << verbs_pass[i] << " " << paths_pass[j].input << " " << proto_pass[k] << "\r\n\r\n"; SHttpRequest parser(HttpRequest::parse(ss.str())); TEST(parser.get() != NULL, "Valid HTTP request should be parsed: %s", ss.str().c_str()); - TEST(parser->get_resource() == string(paths_pass[j].output), + TEST(parser->get_uri() == string(paths_pass[j].output), "The request path '%s' should be correct: %s", - paths_pass[j].output, parser->get_resource().c_str()); + paths_pass[j].output, parser->get_uri().c_str()); } } } @@ -372,12 +372,12 @@ int test_response() { TEST(HttpResponse().get_response().find("200 OK") != string::npos, "Default constructor should return a 200 OK with no body"); - TEST(HttpResponse("Test").get_response().find("\r\n\r\nTest") != string::npos, + TEST(HttpResponse(HTTP_200_OK, "Test").get_response().find("\r\n\r\nTest") != string::npos, "Custom payload should be found in the response"); - TEST(HttpResponse("", HTTP_204_NO_CONTENT).get_response().find("204 No Content") != string::npos, + TEST(HttpResponse(HTTP_204_NO_CONTENT).get_response().find("204 No Content") != string::npos, "Using custom header should generate correct response"); - HttpResponse response("A Bad gateway", HTTP_502_BAD_GATEWAY); + HttpResponse response(HTTP_502_BAD_GATEWAY, "A Bad gateway"); TEST(response.get_response().find("\r\n\r\nA Bad gateway") != string::npos && response.get_response().find("502 Bad Gateway") != string::npos, "Both custom response body and return code should be found"); @@ -440,11 +440,11 @@ int test_resource_parts() TEST(request.get(), "Request should be OK: %s", ss.str().c_str()); - TEST(request->get_resource_parts().size() == resource_parts[i].size, + TEST(request->uri_part_count() == resource_parts[i].size, "Request should have %lu parts: %lu", resource_parts[i].size, - request->get_resource_parts().size()); + request->uri_part_count()); - string value = request->get_resource_parts()[resource_parts[i].offset]; + string value = request->uri_part(resource_parts[i].offset); TEST(value == resource_parts[i].value, "Request part at %d should be '%s': %s", resource_parts[i].offset, resource_parts[i].value, value.c_str());