extract Frame creation into FrameFactory

8 files changed, 81 insertions, 64 deletions

diff --git a/lib/plum.rb b/lib/plum.rb
index 7b22ff9..3e09eb0 100644
--- a/lib/plum.rb
+++ b/lib/plum.rb
@@ -9,7 +9,8 @@ require "plum/hpack/huffman"
 require "plum/hpack/context"
 require "plum/hpack/decoder"
 require "plum/hpack/encoder"
-require "plum/frame_helper"
+require "plum/frame_utils"
+require "plum/frame_factory"
 require "plum/frame"
 require "plum/flow_control"
 require "plum/stream_helper"
diff --git a/lib/plum/connection.rb b/lib/plum/connection.rb
index 6dac8c3..0d9916a 100644
--- a/lib/plum/connection.rb
+++ b/lib/plum/connection.rb
@@ -44,16 +44,10 @@ module Plum
 
     # Closes the connection and closes the io. Sends GOAWAY frame to the peer.
     #
-    # @param error_code [Integer] The error code to be contained in the GOAWAY frame.
-    def close(error_code = 0)
+    # @param error_type [Symbol] The error type to be contained in the GOAWAY frame.
+    def close(error_type = :no_error)
       last_id = @streams.keys.reverse_each.find {|id| id.odd? }
-      data = ""
-      data.push_uint32((last_id || 0) & ~(1 << 31))
-      data.push_uint32(error_code)
-      data.push("") # debug message
-      send_immediately Frame.new(type: :goaway,
-                                 stream_id: 0,
-                                 payload: data)
+      send_immediately Frame.goaway(last_id, error_type)
       # TODO: server MAY wait streams
       @io.close
     end
@@ -77,7 +71,7 @@ module Plum
       end
     rescue ConnectionError => e
       callback(:connection_error, e)
-      close(e.http2_error_code)
+      close(e.http2_error_type)
     end
     alias << receive
 
@@ -174,7 +168,7 @@ module Plum
 
       callback(:remote_settings, @remote_settings, old_remote_settings)
 
-      send_immediately Frame.new(type: :settings, stream_id: 0x00, flags: [:ack])
+      send_immediately Frame.settings(:ack)
     end
 
     def update_local_settings(new_settings)
@@ -198,10 +192,7 @@ module Plum
       else
         on(:ping)
         opaque_data = frame.payload
-        send_immediately Frame.new(type: :ping,
-                                   stream_id: 0,
-                                   flags: [:ack],
-                                   payload: opaque_data)
+        send_immediately Frame.ping(:ack, opaque_data)
       end
     end
 
diff --git a/lib/plum/connection_helper.rb b/lib/plum/connection_helper.rb
index c0db0a3..e3ab0ce 100644
--- a/lib/plum/connection_helper.rb
+++ b/lib/plum/connection_helper.rb
@@ -6,14 +6,7 @@ module Plum
     #
     # @param kwargs [Hash<Symbol, Integer>]
     def settings(**kwargs)
-      payload = kwargs.inject("") {|payload, (key, value)|
-        id = Frame::SETTINGS_TYPE[key] or raise ArgumentError.new("invalid settings type")
-        payload.push_uint16(id)
-        payload.push_uint32(value)
-      }
-      send Frame.new(type: :settings,
-                     stream_id: 0,
-                     payload: payload)
+      send_immediately Frame.settings(**kwargs)
       update_local_settings(kwargs)
     end
 
@@ -22,10 +15,7 @@ module Plum
     # @param data [String] Must be 8 octets.
     # @raise [ArgumentError] If the data is not 8 octets.
     def ping(data = "plum\x00\x00\x00\x00")
-      raise ArgumentError.new("data must be 8 octets") if data.bytesize != 8
-      send Frame.new(type: :ping,
-                     stream_id: 0,
-                     payload: data)
+      send_immediately Frame.ping(data)
     end
   end
 end
diff --git a/lib/plum/frame.rb b/lib/plum/frame.rb
index 67316b5..18b20a2 100644
--- a/lib/plum/frame.rb
+++ b/lib/plum/frame.rb
@@ -2,7 +2,8 @@ using Plum::BinaryString
 
 module Plum
   class Frame
-    include FrameHelper
+    extend FrameFactory
+    include FrameUtils
 
     FRAME_TYPES = {
       data:           0x00,
diff --git a/lib/plum/frame_factory.rb b/lib/plum/frame_factory.rb
new file mode 100644
index 0000000..c735512
--- /dev/null
+++ b/lib/plum/frame_factory.rb
@@ -0,0 +1,53 @@
+using Plum::BinaryString
+
+module Plum
+  module FrameFactory
+    def rst_stream(stream_id, error_type)
+      payload = "".push_uint32(ERROR_CODES[error_type])
+      Frame.new(type: :rst_stream, stream_id: stream_id, payload: payload)
+    end
+
+    def goaway(last_id, error_type, message = "")
+      payload = "".push_uint32((last_id || 0) & ~(1 << 31))
+                  .push_uint32(ERROR_CODES[error_type])
+                  .push(message)
+      Frame.new(type: :goaway, stream_id: 0, payload: payload)
+    end
+
+    def settings(ack = nil, **args)
+      payload = args.inject("") {|payload, (key, value)|
+        id = Frame::SETTINGS_TYPE[key] or raise ArgumentError.new("invalid settings type")
+        payload.push_uint16(id)
+        payload.push_uint32(value)
+      }
+      Frame.new(type: :settings, stream_id: 0, flags: [ack].compact, payload: payload)
+    end
+
+    def ping(arg1 = "plum\x00\x00\x00\x00", arg2 = nil)
+      if !arg2
+        raise ArgumentError.new("data must be 8 octets") if arg1.bytesize != 8
+        Frame.new(type: :ping, stream_id: 0, payload: arg1)
+      else
+        Frame.new(type: :ping, stream_id: 0, flags: [:ack], payload: arg2)
+      end
+    end
+
+    def data(stream_id, payload, *flags)
+      Frame.new(type: :data, stream_id: stream_id, flags: flags.compact, payload: payload.to_s)
+    end
+
+    def headers(stream_id, encoded, *flags)
+      Frame.new(type: :headers, stream_id: stream_id, flags: flags.compact, payload: encoded)
+    end
+
+    def push_promise(stream_id, new_id, encoded, *flags)
+      payload = "".push_uint32(0 << 31 | new_id)
+                  .push(encoded)
+      Frame.new(type: :push_promise, stream_id: stream_id, flags: flags.compact, payload: payload)
+    end
+
+    def continuation(stream_id, payload, *flags)
+      Frame.new(type: :continuation, stream_id: stream_id, flags: flags.compact, payload: payload)
+    end
+  end
+end
diff --git a/lib/plum/frame_helper.rb b/lib/plum/frame_utils.rb
index dd46ddd..2694f5a 100644
--- a/lib/plum/frame_helper.rb
+++ b/lib/plum/frame_utils.rb
@@ -1,7 +1,7 @@
 using Plum::BinaryString
 
 module Plum
-  module FrameHelper
+  module FrameUtils
     # Splits the frame into multiple frames if the payload size exceeds max size.
     #
     # @param max [Integer] The maximum size of a frame payload.
@@ -18,9 +18,9 @@ module Plum
       end
 
       frames = []
-      last = Frame.new(type: :data, flags: self.flags & [:end_stream], stream_id: self.stream_id, payload: fragments.pop)
+      last = Frame.data(stream_id, fragments.pop, *(self.flags & [:end_stream]))
       fragments.each do |fragment|
-        frames << Frame.new(type: :data, flags: self.flags - [:end_stream], stream_id: self.stream_id, payload: fragment)
+        frames << Frame.data(stream_id, fragment, *(self.flags - [:end_stream]))
       end
       frames << last
       frames
@@ -40,9 +40,9 @@ module Plum
       frames = []
       frames << Frame.new(type_value: self.type_value, flags: self.flags - [:end_headers], stream_id: self.stream_id, payload: fragments.shift)
       if fragments.size > 0
-        last = Frame.new(type: :continuation, flags: self.flags & [:end_headers], stream_id: self.stream_id, payload: fragments.pop)
+        last = Frame.continuation(stream_id, fragments.pop, *(self.flags & [:end_headers]))
         fragments.each do |fragment|
-          frames << Frame.new(type: :continuation, stream_id: self.stream_id, payload: fragment)
+          frames << Frame.continuation(stream_id, fragment)
         end
         frames << last
       end
@@ -56,9 +56,9 @@ module Plum
       (self.length / 6).times.map {|i|
         id = self.payload.uint16(6 * i)
         val = self.payload.uint32(6 * i + 2)
-        name = Frame::SETTINGS_TYPE.key(id)
+        name = Frame::SETTINGS_TYPE.key(id) or next nil
         [name, val]
-      }.select {|k, v| k }.to_h
+      }.compact.to_h
     end
   end
 end
diff --git a/lib/plum/stream.rb b/lib/plum/stream.rb
index aa8be7f..e88a70d 100644
--- a/lib/plum/stream.rb
+++ b/lib/plum/stream.rb
@@ -54,19 +54,15 @@ module Plum
       end
     rescue StreamError => e
       callback(:stream_error, e)
-      close(e.http2_error_code)
+      close(e.http2_error_type)
     end
 
     # Closes this stream. Sends RST_STREAM frame to the peer.
     #
-    # @param error_code [Integer] The error code to be contained in the RST_STREAM frame.
-    def close(error_code = 0)
+    # @param error_type [Symbol] The error type to be contained in the RST_STREAM frame.
+    def close(error_type = :no_error)
       @state = :closed
-      data = "".force_encoding(Encoding::BINARY)
-      data.push_uint32(error_code)
-      send_immediately Frame.new(type: :rst_stream,
-                                 stream_id: id,
-                                 payload: data)
+      send_immediately Frame.rst_stream(id, error_type)
     end
 
     private
diff --git a/lib/plum/stream_helper.rb b/lib/plum/stream_helper.rb
index 5dd2de9..5be440e 100644
--- a/lib/plum/stream_helper.rb
+++ b/lib/plum/stream_helper.rb
@@ -21,14 +21,8 @@ module Plum
     # @return [Stream] The stream to send push response.
     def promise(headers)
       stream = @connection.reserve_stream(weight: self.weight + 1, parent: self)
-      payload = "".force_encoding(Encoding::BINARY)
-      payload.push_uint32((0 << 31 | stream.id))
-      payload.push(@connection.hpack_encoder.encode(headers))
-
-      original = Frame.new(type: :push_promise,
-                           flags: [:end_headers],
-                           stream_id: id,
-                           payload: payload)
+      encoded = @connection.hpack_encoder.encode(headers)
+      original = Frame.push_promise(id, stream.id, encoded, :end_headers)
       original.split_headers(@connection.remote_settings[:max_frame_size]).each do |frame|
         send frame
       end
@@ -39,10 +33,7 @@ module Plum
     def send_headers(headers, end_stream:)
       max = @connection.remote_settings[:max_frame_size]
       encoded = @connection.hpack_encoder.encode(headers)
-      original_frame = Frame.new(type: :headers,
-                                 flags: [:end_headers, end_stream ? :end_stream : nil].compact,
-                                 stream_id: id,
-                                 payload: encoded)
+      original_frame = Frame.headers(id, encoded, :end_headers, (end_stream && :end_stream))
       original_frame.split_headers(max).each do |frame|
         send frame
       end
@@ -53,16 +44,10 @@ module Plum
       max = @connection.remote_settings[:max_frame_size]
       if data.is_a?(IO)
         while !data.eof? && fragment = data.readpartial(max)
-          send Frame.new(type: :data,
-                         stream_id: id,
-                         flags: (end_stream && data.eof? && [:end_stream]),
-                         payload: fragment)
+          send Frame.data(id, fragment, (end_stream && data.eof? && :end_stream))
         end
       else
-        original = Frame.new(type: :data,
-                             stream_id: id,
-                             flags: (end_stream && [:end_stream]),
-                             payload: data.to_s)
+        original = Frame.data(id, data, (end_stream && :end_stream))
         original.split_data(max).each do |frame|
           send frame
         end