frame_factory: specify flags by kwargs

10 files changed, 46 insertions, 37 deletions

diff --git a/lib/plum/frame_factory.rb b/lib/plum/frame_factory.rb
index b4c1c01..de5a0b4 100644
--- a/lib/plum/frame_factory.rb
+++ b/lib/plum/frame_factory.rb
@@ -54,37 +54,45 @@ module Plum
     # Creates a DATA frame.
     # @param stream_id [Integer] The stream ID.
     # @param payload [String] Payload.
-    # @param flags [Array<Symbol>] Flags.
-    def data(stream_id, payload, *flags)
+    # @param end_stream [Boolean] add END_STREAM flag
+    def data(stream_id, payload, end_stream: false)
       payload = payload.b if payload && payload.encoding != Encoding::BINARY
-      Frame.new(type: :data, stream_id: stream_id, flags: flags, payload: payload)
+      fval = 0
+      fval += 1 if end_stream
+      Frame.new(type_value: 0, stream_id: stream_id, flags_value: fval, payload: payload)
     end
 
     # Creates a HEADERS frame.
     # @param stream_id [Integer] The stream ID.
     # @param encoded [String] Headers.
-    # @param flags [Array<Symbol>] Flags.
-    def headers(stream_id, encoded, *flags)
-      Frame.new(type: :headers, stream_id: stream_id, flags: flags, payload: encoded)
+    # @param end_stream [Boolean] add END_STREAM flag
+    # @param end_headers [Boolean] add END_HEADERS flag
+    def headers(stream_id, encoded, end_stream: false, end_headers: false)
+      fval = 0
+      fval += 1 if end_stream
+      fval += 4 if end_headers
+      Frame.new(type_value: 1, stream_id: stream_id, flags_value: fval, payload: encoded)
     end
 
     # Creates a PUSH_PROMISE frame.
     # @param stream_id [Integer] The stream ID.
     # @param new_id [Integer] The stream ID to create.
     # @param encoded [String] Request headers.
-    # @param flags [Array<Symbol>] Flags.
-    def push_promise(stream_id, new_id, encoded, *flags)
+    # @param end_headers [Boolean] add END_HEADERS flag
+    def push_promise(stream_id, new_id, encoded, end_headers: false)
       payload = String.new.push_uint32(new_id)
                           .push(encoded)
-      Frame.new(type: :push_promise, stream_id: stream_id, flags: flags, payload: payload)
+      fval = 0
+      fval += 4 if end_headers
+      Frame.new(type: :push_promise, stream_id: stream_id, flags_value: fval, payload: payload)
     end
 
     # Creates a CONTINUATION frame.
     # @param stream_id [Integer] The stream ID.
     # @param payload [String] Payload.
-    # @param flags [Array<Symbol>] Flags.
-    def continuation(stream_id, payload, *flags)
-      Frame.new(type: :continuation, stream_id: stream_id, flags: flags, payload: payload)
+    # @param end_headers [Boolean] add END_HEADERS flag
+    def continuation(stream_id, payload, end_headers: false)
+      Frame.new(type: :continuation, stream_id: stream_id, flags_value: (end_headers && 4 || 0), payload: payload)
     end
   end
 end
diff --git a/lib/plum/hpack/constants.rb b/lib/plum/hpack/constants.rb
index 4eac05c..6993735 100644
--- a/lib/plum/hpack/constants.rb
+++ b/lib/plum/hpack/constants.rb
@@ -67,6 +67,8 @@ module Plum
       ["www-authenticate", ""],
     ].freeze
 
+    STATIC_TABLE_SIZE = STATIC_TABLE.size
+
     HUFFMAN_TABLE = [
       "1111111111000",
       "11111111111111111011000",
diff --git a/lib/plum/hpack/context.rb b/lib/plum/hpack/context.rb
index 622fe36..1d7f7d6 100644
--- a/lib/plum/hpack/context.rb
+++ b/lib/plum/hpack/context.rb
@@ -26,10 +26,10 @@ module Plum
       def fetch(index)
         if index == 0
           raise HPACKError.new("index can't be 0")
-        elsif index <= STATIC_TABLE.size
-          STATIC_TABLE[index - 1]
+        elsif index <= STATIC_TABLE_SIZE
+            STATIC_TABLE[index - 1]
         elsif index <= STATIC_TABLE.size + @dynamic_table.size
-          @dynamic_table[index - STATIC_TABLE.size - 1]
+          @dynamic_table[index - STATIC_TABLE_SIZE - 1]
         else
           raise HPACKError.new("invalid index: #{index}")
         end
@@ -43,7 +43,7 @@ module Plum
         si = STATIC_TABLE.index &pr
         return si + 1 if si
         di = @dynamic_table.index &pr
-        return di + STATIC_TABLE.size + 1 if di
+        return di + STATIC_TABLE_SIZE + 1 if di
       end
 
       def evict
diff --git a/lib/plum/server/http_connection.rb b/lib/plum/server/http_connection.rb
index 116fdcb..8f91ca4 100644
--- a/lib/plum/server/http_connection.rb
+++ b/lib/plum/server/http_connection.rb
@@ -77,8 +77,8 @@ module Plum
                                   ":authority" => @_headers["host"] })
                          .reject {|n, v| ["connection", "http2-settings", "upgrade", "host"].include?(n) }
 
-      stream.receive_frame Frame.headers(1, encoder.encode(headers), :end_headers)
-      stream.receive_frame Frame.data(1, @_body, :end_stream)
+      stream.receive_frame Frame.headers(1, encoder.encode(headers), end_headers: true)
+      stream.receive_frame Frame.data(1, @_body, end_stream: true)
     end
   end
 end
diff --git a/lib/plum/stream_utils.rb b/lib/plum/stream_utils.rb
index ff14c7a..dbb8d96 100644
--- a/lib/plum/stream_utils.rb
+++ b/lib/plum/stream_utils.rb
@@ -9,7 +9,7 @@ module Plum
     def promise(headers)
       stream = @connection.reserve_stream(weight: self.weight + 1, parent: self)
       encoded = @connection.hpack_encoder.encode(headers)
-      frame = Frame.push_promise(id, stream.id, encoded, :end_headers)
+      frame = Frame.push_promise(id, stream.id, encoded, end_headers: true)
       send frame
       stream
     end
@@ -20,7 +20,7 @@ module Plum
     def send_headers(headers, end_stream:)
       max = @connection.remote_settings[:max_frame_size]
       encoded = @connection.hpack_encoder.encode(headers)
-      frame = Frame.headers(id, encoded, :end_headers, (end_stream && :end_stream || nil))
+      frame = Frame.headers(id, encoded, end_headers: true, end_stream: end_stream)
       send frame
       @state = :half_closed_local if end_stream
     end
@@ -33,11 +33,10 @@ module Plum
       if data.is_a?(IO)
         until data.eof?
           fragment = data.readpartial(max)
-          send Frame.data(id, fragment, (end_stream && data.eof? && :end_stream))
+          send Frame.data(id, fragment, end_stream: end_stream && data.eof?)
         end
       else
-        frame = Frame.data(id, data, (end_stream && :end_stream))
-        send frame
+        send Frame.data(id, data, end_stream: end_stream)
       end
       @state = :half_closed_local if end_stream
     end
diff --git a/test/plum/client/test_upgrade_client_session.rb b/test/plum/client/test_upgrade_client_session.rb
index d695142..3cc97b4 100644
--- a/test/plum/client/test_upgrade_client_session.rb
+++ b/test/plum/client/test_upgrade_client_session.rb
@@ -24,8 +24,8 @@ class UpgradeClientSessionTest < Minitest::Test
     sock.rio.string << Frame.settings().assemble
     sock.rio.string << Frame.settings(:ack).assemble
     res = session.request({ ":method" => "GET", ":path" => "/aa" }, "aa", {})
-    sock.rio.string << Frame.headers(3, HPACK::Encoder.new(3).encode(":status" => "200", "content-length" => "3"), :end_headers).assemble
-    sock.rio.string << Frame.data(3, "aaa", :end_stream).assemble
+    sock.rio.string << Frame.headers(3, HPACK::Encoder.new(3).encode(":status" => "200", "content-length" => "3"), end_headers: true).assemble
+    sock.rio.string << Frame.data(3, "aaa", end_stream: true).assemble
     session.succ until res.finished?
     assert(res.finished?)
     assert_equal("aaa", res.body)
diff --git a/test/plum/test_connection_utils.rb b/test/plum/test_connection_utils.rb
index 0051117..35ab7b8 100644
--- a/test/plum/test_connection_utils.rb
+++ b/test/plum/test_connection_utils.rb
@@ -16,7 +16,7 @@ class ServerConnectionUtilsTest < Minitest::Test
 
   def test_server_goaway
     open_server_connection {|con|
-      con << Frame.headers(3, "", :end_stream, :end_headers).assemble
+      con << Frame.headers(3, "", end_stream: true, end_headers: true).assemble
       con.goaway(:stream_closed)
 
       last = sent_frames.last
diff --git a/test/plum/test_flow_control.rb b/test/plum/test_flow_control.rb
index 758472d..8133e0e 100644
--- a/test/plum/test_flow_control.rb
+++ b/test/plum/test_flow_control.rb
@@ -135,17 +135,17 @@ class FlowControlTest < Minitest::Test
 
     prepare.call {|con, stream|
       con.window_update(500) # extend only connection
-      con << Frame.headers(stream.id, "", :end_headers).assemble
+      con << Frame.headers(stream.id, "", end_headers: true).assemble
       assert_stream_error(:flow_control_error) {
-        con << Frame.data(stream.id, "\x00" * 30, :end_stream).assemble
+        con << Frame.data(stream.id, "\x00" * 30, end_stream: true).assemble
       }
     }
 
     prepare.call {|con, stream|
       stream.window_update(500) # extend only stream
-      con << Frame.headers(stream.id, "", :end_headers).assemble
+      con << Frame.headers(stream.id, "", end_headers: true).assemble
       assert_connection_error(:flow_control_error) {
-        con << Frame.data(stream.id, "\x00" * 30, :end_stream).assemble
+        con << Frame.data(stream.id, "\x00" * 30, end_stream: true).assemble
       }
     }
   end
@@ -155,8 +155,8 @@ class FlowControlTest < Minitest::Test
       con = stream.connection
       con.settings(initial_window_size: 24)
       stream.window_update(1)
-      con << Frame.headers(stream.id, "", :end_headers).assemble
-      con << Frame.data(stream.id, "\x00" * 20, :end_stream).assemble
+      con << Frame.headers(stream.id, "", end_headers: true).assemble
+      con << Frame.data(stream.id, "\x00" * 20, end_stream: true).assemble
       assert_equal(4, con.recv_remaining_window)
       assert_equal(5, stream.recv_remaining_window)
       con.settings(initial_window_size: 60)
diff --git a/test/plum/test_frame_factory.rb b/test/plum/test_frame_factory.rb
index ccaa56c..109fb95 100644
--- a/test/plum/test_frame_factory.rb
+++ b/test/plum/test_frame_factory.rb
@@ -55,7 +55,7 @@ class FrameFactoryTest < Minitest::Test
   end
 
   def test_continuation
-    frame = Frame.continuation(123, "abc", :end_headers)
+    frame = Frame.continuation(123, "abc", end_headers: true)
     assert_frame(frame,
                  type: :continuation,
                  stream_id: 123,
@@ -74,7 +74,7 @@ class FrameFactoryTest < Minitest::Test
   end
 
   def test_headers
-    frame = Frame.headers(123, "abc", :end_stream)
+    frame = Frame.headers(123, "abc", end_stream: true)
     assert_frame(frame,
                  type: :headers,
                  stream_id: 123,
@@ -83,7 +83,7 @@ class FrameFactoryTest < Minitest::Test
   end
 
   def test_push_promise
-    frame = Frame.push_promise(345, 2, "abc", :end_headers)
+    frame = Frame.push_promise(345, 2, "abc", end_headers: true)
     assert_frame(frame,
                  type: :push_promise,
                  stream_id: 345,
diff --git a/test/plum/test_stream.rb b/test/plum/test_stream.rb
index a9de58e..9b06214 100644
--- a/test/plum/test_stream.rb
+++ b/test/plum/test_stream.rb
@@ -28,7 +28,7 @@ class StreamTest < Minitest::Test
       }
 
       assert_stream_error(:frame_size_error) {
-        con << Frame.headers(1, "", :end_headers).assemble
+        con << Frame.headers(1, "", end_headers: true).assemble
       }
 
       last = sent_frames.last
@@ -43,7 +43,7 @@ class StreamTest < Minitest::Test
       stream = nil
       con.on(:headers) { |s| stream = s }
 
-      con << Frame.headers(1, "", :end_headers).assemble
+      con << Frame.headers(1, "", end_headers: true).assemble
       assert_raises(LocalStreamError) {
         con << Frame.rst_stream(1, :frame_size_error).assemble
       }