Refactor serializer::stream

Author: ashtum

include/boost/http_proto/serializer.hpp

@@ -159,7 +159,7 @@ class serializer
         std::size_t max_type_erase = 1024;
     };
 
-    struct stream;
+    class stream;
 
     /** Destructor
     */
@@ -401,124 +401,76 @@ install_serializer_service(
 
 //------------------------------------------------
 
-/** The type used for caller-provided body data during
-    serialization.
-
-    @code{.cpp}
-    http_proto::serializer sr(128);
-
-    http_proto::request req;
-    auto stream = sr.start_stream(req);
-
-    std::string_view msg = "Hello, world!";
-    auto n = buffers::copy(
-        stream.prepare(),
-        buffers::make_buffer(
-            msg.data(), msg.size()));
-
-    stream.commit(n);
-
-    auto cbs = sr.prepare().value();
-    (void)cbs;
-    @endcode
+/** Used for streaming body data during serialization.
 */
-struct serializer::stream
+class serializer::stream
 {
-    /** Constructor.
+public:
+    /** Constructor
 
-        The only valid operations on default constructed
-        streams are assignment and destruction.
+        A default-constructed stream is considered closed.
     */
     stream() = default;
 
-    /** Constructor.
-
-        The constructed stream will share the same
-        serializer as `other`.
+    /** Move constructor
     */
-    stream(stream const& other) = default;
-
-    /** Assignment.
+    stream(stream&& other)
+        : sr_(other.sr_)
+    {
+        other.sr_ = nullptr;
+    }
 
-        The current stream will share the same serializer
-        as `other`.
+    /** Move assignment
     */
-    stream& operator= (
-        stream const& other) = default;
+    stream&
+    operator=(stream&& other)
+    {
+        std::swap(sr_, other.sr_);
+        return *this;
+    }
 
-    /** A MutableBufferSequence consisting of a buffer pair.
+    /** The buffer type returned by @ref prepare
      */
     using buffers_type =
         buffers::mutable_buffer_pair;
 
-    /** Returns the remaining available capacity.
-
-        The returned value represents the available free
-        space in the backing fixed-sized buffers used by the
-        serializer associated with this stream.
-
-        The capacity is absolute and does not do any
-        accounting for any octets required by a chunked
-        transfer encoding.
+    /** Returns `true` if the stream is open
     */
     BOOST_HTTP_PROTO_DECL
-    std::size_t
-    capacity() const noexcept;
-
-    /** Return true if the stream cannot currently hold
-        additional output data.
-
-        The fixed-sized buffers maintained by the associated
-        serializer can be sufficiently full from previous
-        calls to @ref stream::commit.
+    bool
+    is_open() const noexcept;
 
-        This function can be called to determine if the caller
-        should drain the serializer via @ref serializer::consume calls
-        before attempting to fill the buffer sequence
-        returned from @ref stream::prepare.
+    /** Returns the available capacity
     */
     BOOST_HTTP_PROTO_DECL
-    bool
-    is_full() const noexcept;
-
-    /** Returns a MutableBufferSequence for storing
-        serializer input. If `n` bytes are written to the
-        buffer sequence, @ref stream::commit must be called
-        with `n` to update the backing serializer's buffers.
+    std::size_t
+    capacity() const noexcept;
 
-        The returned buffer sequence is as wide as is
-        possible.
+    /** Prepares the buffer for writing
 
-        @exception std::length_error Thrown if the stream
-        has insufficient capacity and a chunked transfer
-        encoding is being used
+        Call @ref commit to make data available
+        to the serializer.
     */
     BOOST_HTTP_PROTO_DECL
     buffers_type
-    prepare() const;
+    prepare() const noexcept;
 
-    /** Make `n` bytes available to the serializer.
+    /** Commits data to the serializer
 
-        Once the buffer sequence returned from @ref stream::prepare
-        has been filled, the input can be marked as ready
-        for serialization by using this function.
+        @param n Number of bytes to commit.
 
-        @exception std::logic_error Thrown if `commit` is
-        called with 0.
+        @throw std::invalid_argument if `n > capacity()`.
+        @throw std::logic_error if `!is_open()`.
     */
     BOOST_HTTP_PROTO_DECL
     void
     commit(std::size_t n) const;
 
-    /** Indicate that no more data is coming and that the
-        body should be treated as complete.
-
-        @excpeption std::logic_error Thrown if the stream
-        has been previously closed.
+    /** Closes the stream
     */
     BOOST_HTTP_PROTO_DECL
     void
-    close() const;
+    close() noexcept;
 
 private:
     friend class serializer;

src/serializer.cpp

@@ -107,66 +107,74 @@ write_final_chunk(
     BOOST_ASSERT(n == 5);
 }
 
+buffers::mutable_buffer_pair
+prepare_chunked(
+    buffers::circular_buffer& cb) noexcept
+{
+    return
+        buffers::sans_suffix(
+            buffers::sans_prefix(
+                cb.prepare(cb.capacity()),
+                chunk_header_len),
+            final_chunk_len + crlf_len);
+}
+
 //------------------------------------------------
 
 class appender
 {
     buffers::circular_buffer& cb_;
     buffers::mutable_buffer_pair mbp_;
-    std::size_t n_ = 0;
     bool is_chunked_ = false;
     bool more_input_ = true;
 
 public:
     appender(
         buffers::circular_buffer& cb,
-        bool is_chunked)
+        bool is_chunked) noexcept
         : cb_(cb)
-        , mbp_(cb.prepare(cb.capacity()))
+        , mbp_(is_chunked ?
+            prepare_chunked(cb) : cb.prepare(cb.capacity()))
         , is_chunked_(is_chunked)
     {
     }
 
     bool
     is_full() const noexcept
     {
-        auto remaining = cb_.capacity() - n_;
-        if(is_chunked_)
-            return remaining <= chunked_overhead_;
-
-        return remaining == 0;
+        return buffers::size(mbp_) == 0;
     }
 
     buffers::mutable_buffer_pair
     prepare() noexcept
     {
-        if(is_chunked_)
-        {
-            return buffers::sans_suffix(
-                buffers::sans_prefix(
-                    mbp_,
-                    chunk_header_len + n_)
-                , final_chunk_len + crlf_len);
-        }
-        return buffers::sans_prefix(mbp_, n_);
+        return mbp_;
     }
 
     void
     commit(std::size_t n, bool more) noexcept
     {
         BOOST_ASSERT(more_input_);
-        n_ += n;
+        mbp_ = buffers::sans_prefix(mbp_, n);
         more_input_ = more;
     }
 
     ~appender()
     {
         if(is_chunked_)
         {
-            if(n_)
+            auto consumed = cb_.capacity() -
+                buffers::size(mbp_) - chunked_overhead_;
+
+            if(consumed != 0)
             {
-                write_chunk_header(mbp_, n_);
-                cb_.commit(n_ + chunk_header_len);
+                write_chunk_header(
+                    cb_.prepare(chunk_header_len),
+                    consumed);
+                cb_.commit(chunk_header_len);
+
+                cb_.prepare(consumed);
+                cb_.commit(consumed);
 
                 write_crlf(
                     cb_.prepare(crlf_len));
@@ -182,7 +190,7 @@ class appender
         }
         else // is_chunked_ == false
         {
-            cb_.commit(n_);
+            cb_.commit(cb_.capacity() - buffers::size(mbp_));
         }
     }
 };
@@ -1038,11 +1046,25 @@ start_stream(
 
 //------------------------------------------------
 
+bool
+serializer::
+stream::
+is_open() const noexcept
+{
+    if(sr_ == nullptr)
+        return false;
+
+    return sr_->more_input_;
+}
+
 std::size_t
 serializer::
 stream::
 capacity() const noexcept
 {
+    if(!is_open())
+        return 0;
+
     if(sr_->filter_)
         return sr_->cb1_.capacity();
 
@@ -1057,20 +1079,15 @@ capacity() const noexcept
     return 0;
 }
 
-bool
-serializer::
-stream::
-is_full() const noexcept
-{
-    return capacity() == 0;
-}
-
 auto
 serializer::
 stream::
-prepare() const ->
+prepare() const noexcept ->
     buffers_type
 {
+    if(!is_open())
+        return {};
+
     if(sr_->filter_)
         return sr_->cb1_.prepare(
             sr_->cb1_.capacity());
@@ -1080,21 +1097,25 @@ prepare() const ->
             sr_->cb0_.capacity());
 
     // chunked with no filter
-    const auto cap = sr_->cb0_.capacity();
-    if(cap <= chunked_overhead_)
-        detail::throw_length_error();
-
-    return buffers::sans_prefix(
-        sr_->cb0_.prepare(
-            cap - crlf_len - final_chunk_len),
-        chunk_header_len);
+    return prepare_chunked(sr_->cb0_);
 }
 
 void
 serializer::
 stream::
 commit(std::size_t n) const
 {
+    // Precondition violation
+    if(!is_open())
+        detail::throw_logic_error();
+
+    if(n > capacity())
+    {
+        // n can't be greater than size of
+        // the buffers returned by prepare()
+        detail::throw_invalid_argument();
+    }
+
     if(sr_->filter_)
         return sr_->cb1_.commit(n);
 
@@ -1123,11 +1144,10 @@ commit(std::size_t n) const
 void
 serializer::
 stream::
-close() const
+close() noexcept
 {
-    // Precondition violation
-    if(!sr_->more_input_)
-        detail::throw_logic_error();
+    if(!is_open())
+        return; // no-op;
 
     sr_->more_input_ = false;