Refactor method and verb (API Change):

The verb interfaces now use verb::unknown instead of
boost::optional<verb> == boost::none to indicate that
the request-method is an unrecognized string.

The http::header interface is modified to focus more on the
verb enum rather than the string. For recognized verbs, the
implementation stores an integer instead of the string.
Unknown verbs are still stored as strings.

* header::method now returns a verb
* header::method_string returns the method text

CHANGELOG.md

@@ -1,5 +1,9 @@
 Version 49
 
+API Changes:
+
+* Refactor method and verb
+
 --------------------------------------------------------------------------------
 
 Version 48

doc/6_1_http_message.qbk

@@ -210,11 +210,16 @@ template<class Fields>
 struct header<true, Fields>
 {
     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<false, Fields>
 ```
 
 The start-line data members are replaced traditional accessors using
-non-owning references to string buffers. Now we make a concession:
+non-owning references to string buffers. The method is stored using
-management of the corresponding string is delegated to the [*Fields]
+a simple integer instead of the entire string, for the case where
-container, which can already be allocator aware and constructed with the
+the method is recognized from the set of known verb strings.
-necessary allocator parameter via the provided constructor overloads for
+
-`message`. The delegation implementation looks like this (only the
+Now we make a concession: management of the corresponding string is
-response header specialization is shown):
+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<class Fields>
@@ -244,28 +253,22 @@ struct header<false, Fields>
     int version;
     int status;
 
-    auto
+    string_view
-    reason() const -> decltype(std::declval<Fields>().reason()) const
+    reason() const
     {
         return fields.reason();
     }
 
-    template<class Value>
     void
-    reason(Value&& value)
+    reason(string_view s)
     {
-        fields.reason(std::forward<Value>(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

include/beast/http/fields.hpp

@@ -11,10 +11,8 @@
 #include <beast/config.hpp>
 #include <beast/core/string_view.hpp>
 #include <beast/core/detail/empty_base_optimization.hpp>
-#include <beast/http/verb.hpp>
 #include <beast/http/detail/fields.hpp>
 #include <boost/lexical_cast.hpp>
-#include <boost/optional.hpp>
 #include <algorithm>
 #include <cctype>
 #include <memory>
@@ -61,8 +59,6 @@ class basic_fields :
     using size_type =
         typename std::allocator_traits<Allocator>::size_type;
 
-    boost::optional<verb> verb_;
-
     void
     delete_all();
 
@@ -290,13 +286,10 @@ private:
 #endif
 
     string_view
-    method() const;
+    method_string() const;
 
     void
-    method(verb v);
+    method_string(string_view s);
-
-    void
-    method(string_view const& 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);
     }

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<bool,
         alloc_traits::propagate_on_container_move_assignment::value>{});
-    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<class Allocator>
@@ -138,7 +135,6 @@ operator=(basic_fields const& other) ->
     clear();
     copy_assign(other, std::integral_constant<bool,
         alloc_traits::propagate_on_container_copy_assignment::value>{});
-    verb_ = other.verb_;
     return *this;
 }
 
@@ -148,7 +144,6 @@ basic_fields<Allocator>::
 basic_fields(basic_fields<OtherAlloc> const& other)
 {
     copy_from(other);
-    verb_ = other.verb_;
 }
 
 template<class Allocator>
@@ -160,7 +155,6 @@ operator=(basic_fields<OtherAlloc> const& other) ->
 {
     clear();
     copy_from(other);
-    verb_ = other.verb_;
     return *this;
 }
 
@@ -260,29 +254,17 @@ replace(string_view const& name,
 template<class Allocator>
 string_view
 basic_fields<Allocator>::
-method() const
+method_string() const
 {
-    if(verb_)
-        return to_string(*verb_);
     return (*this)[":method"];
 }
 
 template<class Allocator>
 void
 basic_fields<Allocator>::
-method(verb v)
+method_string(string_view s)
 {
-    verb_ = v;
+    if(s.empty())
-    this->erase(":method");
-}
-
-template<class Allocator>
-void
-basic_fields<Allocator>::
-method(string_view const& s)
-{
-    verb_ = string_to_verb(s);
-    if(verb_)
         this->erase(":method");
     else
         this->replace(":method", s);

include/beast/http/impl/message.ipp

@@ -22,6 +22,43 @@
 namespace beast {
 namespace http {
 
+template<class Fields>
+template<class>
+string_view
+header<true, Fields>::
+get_method_string() const
+{
+    if(method_ != verb::unknown)
+        return to_string(method_);
+    return fields.method_string();
+}
+
+template<class Fields>
+template<class>
+void
+header<true, Fields>::
+set_method(verb v)
+{
+    if(v == verb::unknown)
+        BOOST_THROW_EXCEPTION(
+            std::invalid_argument{"unknown verb"});
+    method_ = v;
+    fields.method_string({});
+}
+
+template<class Fields>
+template<class>
+void
+header<true, Fields>::
+set_method(string_view s)
+{
+    method_ = string_to_verb(s);
+    if(method_ != verb::unknown)
+        fields.method_string({});
+    else
+        fields.method_string(s);
+}
+
 template<class Fields>
 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<class Fields>
@@ -193,7 +231,7 @@ prepare(message<isRequest, Body, Fields>& 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);

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 "<unknown>";
     }
 
-    BOOST_THROW_EXCEPTION(std::logic_error{"unknown method"});
+    BOOST_THROW_EXCEPTION(std::invalid_argument{"unknown verb"});
 }
 
 template<class = void>
-boost::optional<verb>
+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)
+                if(! *s)
-                    return *p == 0;
+                    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>
+verb
 string_to_verb(string_view s)
 {
     return detail::string_to_verb(s);

include/beast/http/message.hpp

@@ -10,9 +10,12 @@
 
 #include <beast/config.hpp>
 #include <beast/http/fields.hpp>
+#include <beast/http/verb.hpp>
 #include <beast/core/string_view.hpp>
 #include <beast/core/detail/integer_sequence.hpp>
+#include <boost/throw_exception.hpp>
 #include <memory>
+#include <stdexcept>
 #include <string>
 #include <tuple>
 #include <utility>
@@ -66,28 +69,64 @@ struct header<true, Fields>
     */
     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
+    verb
-    method() const ->
+    method() const
-        decltype(std::declval<Fields>().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<class Value>
     void
-    method(Value&& value)
+    method(string_view s)
     {
-        fields.method(std::forward<Value>(value));
+        set_method(s);
     }
 
     /** Return the Request Target
@@ -158,6 +197,28 @@ struct header<true, Fields>
             std::forward<ArgN>(argn)...)
     {
     }
+
+private:
+    template<class Fields>
+    friend
+    void
+    swap(
+        header<true, Fields>& m1,
+        header<true, Fields>& m2);
+
+    template<class = void>
+    string_view
+    get_method_string() const;
+
+    template<class = void>
+    void
+    set_method(verb v);
+
+    template<class = void>
+    void
+    set_method(string_view v);
+
+    verb method_ = verb::unknown;
 };
 
 /** A container for an HTTP request or response header.

include/beast/http/verb.hpp

@@ -10,7 +10,6 @@
 
 #include <beast/config.hpp>
 #include <beast/core/string_view.hpp>
-#include <boost/optional.hpp>
 #include <ostream>
 
 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>
+verb
 string_to_verb(string_view s);
 
 /// Returns the text representation of a request method verb.

include/beast/websocket/impl/rfc6455.ipp

@@ -19,7 +19,7 @@ is_upgrade(http::header<true, Fields> 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;

include/beast/websocket/impl/stream.ipp

@@ -225,7 +225,7 @@ build_response(http::header<true, Fields> 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");

test/http/fields.cpp

@@ -100,17 +100,15 @@ public:
     }
 
     void
-    testMethod()
+    testMethodString()
     {
         f_t f;
-        f.method(verb::get);
+        f.method_string("CRY");
-        BEAST_EXPECTS(f.method() == "GET", f.method());
+        BEAST_EXPECTS(f.method_string() == "CRY", f.method_string());
-        f.method("CRY");
+        f.method_string("PUT");
-        BEAST_EXPECTS(f.method() == "CRY", f.method());
+        BEAST_EXPECTS(f.method_string() == "PUT", f.method_string());
-        f.method("PUT");
+        f.method_string({});
-        BEAST_EXPECTS(f.method() == "PUT", f.method());
+        BEAST_EXPECTS(f.method_string().empty(), f.method_string());
-        f.method("CONNECT");
-        BEAST_EXPECTS(f.method() == "CONNECT", f.method());
     }
 
     void run() override
@@ -118,7 +116,7 @@ public:
         testHeaders();
         testRFC2616();
         testErase();
-        testMethod();
+        testMethodString();
     }
 };
 

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(m1.method_string() == "G");
-        BEAST_EXPECT(m2.method().empty());
+        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<true> 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();
     }
 };
 

test/http/parser.cpp

@@ -237,7 +237,7 @@ public:
             [&](parser_type<true> 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");

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_");