diff --git a/CHANGELOG.md b/CHANGELOG.md index 455d5fcb..bd0f246a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ Version 49 +API Changes: + +* Refactor method and verb + -------------------------------------------------------------------------------- Version 48 diff --git a/doc/6_1_http_message.qbk b/doc/6_1_http_message.qbk index aa9f6087..08f4c4bd 100644 --- a/doc/6_1_http_message.qbk +++ b/doc/6_1_http_message.qbk @@ -210,11 +210,16 @@ template struct header { int version; - string_view method() const; + verb method() const; + string_view method_string() const; + void method(verb); void method(string_view); string_view target(); const; void target(string_view); Fields fields; + +private: + verb method_; }; /// An HTTP response header @@ -230,12 +235,16 @@ struct header ``` The start-line data members are replaced traditional accessors using -non-owning references to string buffers. Now we make a concession: -management of the corresponding string is delegated to the [*Fields] -container, which can already be allocator aware and constructed with the -necessary allocator parameter via the provided constructor overloads for -`message`. The delegation implementation looks like this (only the -response header specialization is shown): +non-owning references to string buffers. The method is stored using +a simple integer instead of the entire string, for the case where +the method is recognized from the set of known verb strings. + +Now we make a concession: management of the corresponding string is +delegated to the [*Fields] container, which can already be allocator +aware and constructed with the necessary allocator parameter via the +provided constructor overloads for `message`. The delegation +implementation looks like this (only the response header specialization +is shown): ``` /// An HTTP response header template @@ -244,28 +253,22 @@ struct header int version; int status; - auto - reason() const -> decltype(std::declval().reason()) const + string_view + reason() const { return fields.reason(); } - template void - reason(Value&& value) + reason(string_view s) { - fields.reason(std::forward(value)); + fields.reason(s); } Fields fields; }; ``` -An advantage of this technique is that user-provided implementations of -[*Fields] may use arbitrary representation strategies. For example, instead -of explicitly storing the method as a string, store it as an enumerated -integer with a lookup table of static strings for known message types. - Now that we've accomplished our initial goals and more, there is one small quality of life improvement to make. Users will choose different types for `Body` far more often than they will for `Fields`. Thus, we swap the order diff --git a/doc/images/message.png b/doc/images/message.png index fc750f8d..6e964762 100644 Binary files a/doc/images/message.png and b/doc/images/message.png differ diff --git a/include/beast/http/fields.hpp b/include/beast/http/fields.hpp index 7774fe77..3784c9de 100644 --- a/include/beast/http/fields.hpp +++ b/include/beast/http/fields.hpp @@ -11,10 +11,8 @@ #include #include #include -#include #include #include -#include #include #include #include @@ -61,8 +59,6 @@ class basic_fields : using size_type = typename std::allocator_traits::size_type; - boost::optional verb_; - void delete_all(); @@ -290,13 +286,10 @@ private: #endif string_view - method() const; + method_string() const; void - method(verb v); - - void - method(string_view const& s); + method_string(string_view s); string_view target() const @@ -305,7 +298,7 @@ private: } void - target(string_view const& s) + target(string_view s) { return this->replace(":target", s); } @@ -317,7 +310,7 @@ private: } void - reason(string_view const& s) + reason(string_view s) { return this->replace(":reason", s); } diff --git a/include/beast/http/impl/fields.ipp b/include/beast/http/impl/fields.ipp index 39b695ad..ef78d693 100644 --- a/include/beast/http/impl/fields.ipp +++ b/include/beast/http/impl/fields.ipp @@ -100,7 +100,6 @@ basic_fields(basic_fields&& other) std::move(other.member())) , detail::basic_fields_base( std::move(other.set_), std::move(other.list_)) - , verb_(other.verb_) { } @@ -115,7 +114,6 @@ operator=(basic_fields&& other) -> clear(); move_assign(other, std::integral_constant{}); - verb_ = other.verb_; return *this; } @@ -126,7 +124,6 @@ basic_fields(basic_fields const& other) select_on_container_copy_construction(other.member())) { copy_from(other); - verb_ = other.verb_; } template @@ -138,7 +135,6 @@ operator=(basic_fields const& other) -> clear(); copy_assign(other, std::integral_constant{}); - verb_ = other.verb_; return *this; } @@ -148,7 +144,6 @@ basic_fields:: basic_fields(basic_fields const& other) { copy_from(other); - verb_ = other.verb_; } template @@ -160,7 +155,6 @@ operator=(basic_fields const& other) -> { clear(); copy_from(other); - verb_ = other.verb_; return *this; } @@ -260,29 +254,17 @@ replace(string_view const& name, template string_view basic_fields:: -method() const +method_string() const { - if(verb_) - return to_string(*verb_); return (*this)[":method"]; } template void basic_fields:: -method(verb v) +method_string(string_view s) { - verb_ = v; - this->erase(":method"); -} - -template -void -basic_fields:: -method(string_view const& s) -{ - verb_ = string_to_verb(s); - if(verb_) + if(s.empty()) this->erase(":method"); else this->replace(":method", s); diff --git a/include/beast/http/impl/message.ipp b/include/beast/http/impl/message.ipp index d951f951..71df4b0f 100644 --- a/include/beast/http/impl/message.ipp +++ b/include/beast/http/impl/message.ipp @@ -22,6 +22,43 @@ namespace beast { namespace http { +template +template +string_view +header:: +get_method_string() const +{ + if(method_ != verb::unknown) + return to_string(method_); + return fields.method_string(); +} + +template +template +void +header:: +set_method(verb v) +{ + if(v == verb::unknown) + BOOST_THROW_EXCEPTION( + std::invalid_argument{"unknown verb"}); + method_ = v; + fields.method_string({}); +} + +template +template +void +header:: +set_method(string_view s) +{ + method_ = string_to_verb(s); + if(method_ != verb::unknown) + fields.method_string({}); + else + fields.method_string(s); +} + template void swap( @@ -31,6 +68,7 @@ swap( using std::swap; swap(m1.version, m2.version); swap(m1.fields, m2.fields); + swap(m1.method_, m2.method_); } template @@ -193,7 +231,7 @@ prepare(message& msg, { using beast::detail::ci_equal; if(*pi.content_length > 0 || - ci_equal(msg.method(), "POST")) + msg.method() == verb::post) { msg.fields.insert( "Content-Length", *pi.content_length); diff --git a/include/beast/http/impl/verb.ipp b/include/beast/http/impl/verb.ipp index 7f261b6d..f72e1089 100644 --- a/include/beast/http/impl/verb.ipp +++ b/include/beast/http/impl/verb.ipp @@ -62,13 +62,16 @@ verb_to_string(verb v) case verb::link: return "LINK"; case verb::unlink: return "UNLINK"; + + case verb::unknown: + return ""; } - BOOST_THROW_EXCEPTION(std::logic_error{"unknown method"}); + BOOST_THROW_EXCEPTION(std::invalid_argument{"unknown verb"}); } template -boost::optional +verb string_to_verb(string_view v) { /* @@ -107,7 +110,8 @@ string_to_verb(string_view v) UNSUBSCRIBE */ if(v.size() < 3) - return boost::none; + return verb::unknown; + // s must be null terminated auto const eq = [](string_view sv, char const* s) { @@ -118,8 +122,8 @@ string_to_verb(string_view v) return false; ++s; ++p; - if(*s == 0) - return *p == 0; + if(! *s) + return p == sv.end(); } }; auto c = v[0]; @@ -303,7 +307,7 @@ string_to_verb(string_view v) break; } - return boost::none; + return verb::unknown; } } // detail @@ -316,7 +320,7 @@ to_string(verb v) } inline -boost::optional +verb string_to_verb(string_view s) { return detail::string_to_verb(s); diff --git a/include/beast/http/message.hpp b/include/beast/http/message.hpp index 629fe72e..bb15d847 100644 --- a/include/beast/http/message.hpp +++ b/include/beast/http/message.hpp @@ -10,9 +10,12 @@ #include #include +#include #include #include +#include #include +#include #include #include #include @@ -66,28 +69,64 @@ struct header */ int version; - /** Return the Request Method + /** Return the request-method verb. + + If the request-method is not one of the recognized verbs, + @ref verb::unknown is returned. Callers may use @ref method_string + to retrieve the exact text. @note This function is only available if `isRequest == true`. + + @see @ref method_string */ - auto - method() const -> - decltype(std::declval().method()) const + verb + method() const { - return fields.method(); + return method_; } - /** Set the Request Method + /** Set the request-method verb. - @param value A value that represents the request method. + This function will set the method for requests to one + of the known verbs. + + @param v The request method verb to set. + This may not be @ref verb::unknown. + + @throw std::invalid_argument when `v == verb::unknown`. + */ + void + method(verb v) + { + set_method(v); + } + + /** Return the request-method as a string. + + @note This function is only available if `isRequest == true`. + + @see @ref method + */ + string_view + method_string() const + { + return get_method_string(); + } + + /** Set the request-method using a string. + + This function will set the method for requests to a verb + if the string matches a known verb, otherwise it will + store a copy of the passed string as the method. + + @param s A string representing the request method. @note This function is only available if `isRequest == true`. */ - template void - method(Value&& value) + method(string_view s) { - fields.method(std::forward(value)); + set_method(s); } /** Return the Request Target @@ -158,6 +197,28 @@ struct header std::forward(argn)...) { } + +private: + template + friend + void + swap( + header& m1, + header& m2); + + template + string_view + get_method_string() const; + + template + void + set_method(verb v); + + template + void + set_method(string_view v); + + verb method_ = verb::unknown; }; /** A container for an HTTP request or response header. diff --git a/include/beast/http/verb.hpp b/include/beast/http/verb.hpp index 8ff64967..a77020b9 100644 --- a/include/beast/http/verb.hpp +++ b/include/beast/http/verb.hpp @@ -10,7 +10,6 @@ #include #include -#include #include namespace beast { @@ -23,8 +22,16 @@ namespace http { */ enum class verb { + /** An unknown method. + + This value indicates that the request method string is not + one of the recognized verbs. Callers interested in the method + should use an interface which returns the original string. + */ + unknown = 0, + /// The DELETE method deletes the specified resource - delete_ = 1, + delete_, /** The GET method requests a representation of the specified resource. @@ -123,7 +130,7 @@ enum class verb If the string does not match a known request method, `boost::none` is returned. */ -boost::optional +verb string_to_verb(string_view s); /// Returns the text representation of a request method verb. diff --git a/include/beast/websocket/impl/rfc6455.ipp b/include/beast/websocket/impl/rfc6455.ipp index e04bd11c..d8153785 100644 --- a/include/beast/websocket/impl/rfc6455.ipp +++ b/include/beast/websocket/impl/rfc6455.ipp @@ -19,7 +19,7 @@ is_upgrade(http::header const& req) { if(req.version < 11) return false; - if(req.method() != "GET") + if(req.method() != http::verb::get) return false; if(! http::is_upgrade(req)) return false; diff --git a/include/beast/websocket/impl/stream.ipp b/include/beast/websocket/impl/stream.ipp index d1894ce1..8ce4422c 100644 --- a/include/beast/websocket/impl/stream.ipp +++ b/include/beast/websocket/impl/stream.ipp @@ -225,7 +225,7 @@ build_response(http::header const& req, }; if(req.version < 11) return err("HTTP version 1.1 required"); - if(req.method() != "GET") + if(req.method() != http::verb::get) return err("Wrong method"); if(! is_upgrade(req)) return err("Expected Upgrade request"); diff --git a/test/http/fields.cpp b/test/http/fields.cpp index c6bb2c1a..3dfd23e7 100644 --- a/test/http/fields.cpp +++ b/test/http/fields.cpp @@ -100,17 +100,15 @@ public: } void - testMethod() + testMethodString() { f_t f; - f.method(verb::get); - BEAST_EXPECTS(f.method() == "GET", f.method()); - f.method("CRY"); - BEAST_EXPECTS(f.method() == "CRY", f.method()); - f.method("PUT"); - BEAST_EXPECTS(f.method() == "PUT", f.method()); - f.method("CONNECT"); - BEAST_EXPECTS(f.method() == "CONNECT", f.method()); + f.method_string("CRY"); + BEAST_EXPECTS(f.method_string() == "CRY", f.method_string()); + f.method_string("PUT"); + BEAST_EXPECTS(f.method_string() == "PUT", f.method_string()); + f.method_string({}); + BEAST_EXPECTS(f.method_string().empty(), f.method_string()); } void run() override @@ -118,7 +116,7 @@ public: testHeaders(); testRFC2616(); testErase(); - testMethod(); + testMethodString(); } }; diff --git a/test/http/message.cpp b/test/http/message.cpp index 14b817b6..0de2542b 100644 --- a/test/http/message.cpp +++ b/test/http/message.cpp @@ -137,8 +137,8 @@ public: m2.method("G"); m2.body = "2"; swap(m1, m2); - BEAST_EXPECT(m1.method() == "G"); - BEAST_EXPECT(m2.method().empty()); + BEAST_EXPECT(m1.method_string() == "G"); + BEAST_EXPECT(m2.method_string().empty()); BEAST_EXPECT(m1.target().empty()); BEAST_EXPECT(m2.target() == "u"); BEAST_EXPECT(m1.body == "2"); @@ -296,6 +296,31 @@ public: }(); } + void + testMethod() + { + header h; + auto const vcheck = + [&](verb v) + { + h.method(v); + BEAST_EXPECT(h.method() == v); + BEAST_EXPECT(h.method_string() == to_string(v)); + }; + auto const scheck = + [&](string_view s) + { + h.method(s); + BEAST_EXPECT(h.method() == string_to_verb(s)); + BEAST_EXPECT(h.method_string() == s); + }; + vcheck(verb::get); + vcheck(verb::head); + scheck("GET"); + scheck("HEAD"); + scheck("XYZ"); + } + void run() override { @@ -305,6 +330,7 @@ public: testPrepare(); testSwap(); testSpecialMembers(); + testMethod(); } }; diff --git a/test/http/parser.cpp b/test/http/parser.cpp index 6bbfe384..8ace8e0f 100644 --- a/test/http/parser.cpp +++ b/test/http/parser.cpp @@ -237,7 +237,7 @@ public: [&](parser_type const& p) { auto const& m = p.get(); - BEAST_EXPECT(m.method() == "GET"); + BEAST_EXPECT(m.method() == verb::get); BEAST_EXPECT(m.target() == "/"); BEAST_EXPECT(m.version == 11); BEAST_EXPECT(! p.need_eof()); @@ -274,7 +274,7 @@ public: BEAST_EXPECT(p.is_done()); BEAST_EXPECT(p.is_header_done()); BEAST_EXPECT(! p.need_eof()); - BEAST_EXPECT(m.method() == "GET"); + BEAST_EXPECT(m.method() == verb::get); BEAST_EXPECT(m.target() == "/"); BEAST_EXPECT(m.version == 11); BEAST_EXPECT(m.fields["User-Agent"] == "test"); diff --git a/test/http/verb.cpp b/test/http/verb.cpp index b0a83cf5..ec90033d 100644 --- a/test/http/verb.cpp +++ b/test/http/verb.cpp @@ -26,6 +26,8 @@ public: BEAST_EXPECT(string_to_verb(to_string(v)) == v); }; + good(verb::unknown); + good(verb::delete_); good(verb::get); good(verb::head); @@ -64,7 +66,7 @@ public: [&](string_view s) { auto const v = string_to_verb(s); - BEAST_EXPECTS(! v, to_string(*v)); + BEAST_EXPECTS(v == verb::unknown, to_string(v)); }; bad("AC_");