Refactor Features messages.

Rakefile

@@ -1,7 +1,7 @@
 require 'bundler/gem_tasks'
 
 RELISH_PROJECT = 'trema/pio'
-FLAY_THRESHOLD = 1467
+FLAY_THRESHOLD = 1030
 
 task default: :travis
 task test: [:spec, :cucumber]

features/open_flow10/features_request.feature

@@ -47,20 +47,6 @@ Feature: Pio::Features::Request
       | xid            |                    123 |
       | user_data      |                        |
 
-  Scenario: new(xid: -1) and error
-    When I try to create an OpenFlow message with:
-      """
-      Pio::Features::Request.new(xid: -1)
-      """
-    Then it should fail with "ArgumentError", "Transaction ID should be an unsigned 32-bit integer."
-
-  Scenario: new(xid: 2**32) and error
-    When I try to create an OpenFlow message with:
-      """
-      Pio::Features::Request.new(xid: 2**32)
-      """
-    Then it should fail with "ArgumentError", "Transaction ID should be an unsigned 32-bit integer."
-
   Scenario: read
     When I try to parse a file named "open_flow10/features_request.raw" with "Pio::Features::Request" class
     Then it should finish successfully

features/open_flow13/features_request.feature

@@ -50,20 +50,6 @@ Feature: Pio::Features::Request
       | xid            |                    123 |
       | body           |                        |
 
-  Scenario: new(xid: -1) and error
-    When I try to create an OpenFlow message with:
-      """
-      Pio::Features::Request.new(xid: -1)
-      """
-    Then it should fail with "ArgumentError", "Transaction ID should be an unsigned 32-bit integer."
-
-  Scenario: new(xid: 2**32) and error
-    When I try to create an OpenFlow message with:
-      """
-      Pio::Features::Request.new(xid: 2**32)
-      """
-    Then it should fail with "ArgumentError", "Transaction ID should be an unsigned 32-bit integer."
-
   Scenario: new(unknown_attr: 'foo') and error
     When I try to create an OpenFlow message with:
       """

lib/pio/open_flow/echo.rb

@@ -36,18 +36,8 @@ def self.message_type(message_type)
         end
       end
 
-      user_option :transaction_id
-      user_option :xid
-      user_option :body
-      user_option :user_data
-
-      def initialize(user_options = {})
-        validate_user_options user_options
-        header_options = OpenFlowHeader::Options.parse(user_options)
-        body_options = user_options[:body] || user_options[:user_data] || ''
-        @format = self.class.const_get(:Format).new(header: header_options,
-                                                    body: body_options)
-      end
+      body_option :body
+      body_option :user_data
     end
   end
 end

lib/pio/open_flow/message.rb

@@ -19,12 +19,20 @@ def self.read(raw_data)
       end
       # rubocop:enable MethodLength
 
-      def self.user_option(name)
-        unless class_variable_defined?(:@@valid_options)
-          class_variable_set(:@@valid_options, [])
+      def self.body_option(name)
+        unless class_variable_defined?(:@@valid_body_options)
+          class_variable_set(:@@valid_body_options, [])
         end
-        class_variable_set(:@@valid_options,
-                           class_variable_get(:@@valid_options) + [name])
+        class_variable_set(:@@valid_body_options,
+                           class_variable_get(:@@valid_body_options) + [name])
+      end
+
+      def initialize(user_options = {})
+        validate_user_options user_options
+        @format =
+          self.class.const_get(:Format).
+          new(header: parse_header_options(user_options),
+              body: parse_body_options(user_options))
       end
 
       def method_missing(method, *args, &block)
@@ -34,13 +42,40 @@ def method_missing(method, *args, &block)
       private
 
       def validate_user_options(user_options)
-        unknown_options = user_options.keys - valid_options
+        unknown_options =
+          user_options.keys - valid_header_options - valid_body_options
         return if unknown_options.empty?
         fail "Unknown option: #{unknown_options.first}"
       end
 
-      def valid_options
-        self.class.class_variable_get(:@@valid_options)
+      def parse_header_options(user_options)
+        transaction_id =
+          user_options[:transaction_id] || user_options[:xid] || 0
+        { transaction_id: transaction_id }
+      end
+
+      def parse_body_options(user_options)
+        if valid_body_options.include?(:body)
+          return user_options[:body] || user_options[:user_data] || ''
+        end
+        options = user_options.dup
+        options.delete :transaction_id
+        options.delete :xid
+        dpid = options[:dpid]
+        options[:datapath_id] = dpid if dpid
+        options
+      end
+
+      def valid_header_options
+        [:transaction_id, :xid]
+      end
+
+      def valid_body_options
+        if self.class.class_variable_defined?(:@@valid_body_options)
+          self.class.class_variable_get(:@@valid_body_options)
+        else
+          []
+        end
       end
     end
   end

lib/pio/open_flow/open_flow_header.rb

@@ -22,19 +22,5 @@ class OpenFlowHeader < BinData::Record
     virtual assert: -> { message_type == message_type_value }
     uint16 :message_length, initial_value: -> { 8 + body.length }
     transaction_id :transaction_id, initial_value: 0
-
-    # parse header options
-    class Options
-      def self.parse(options)
-        xid = if options.respond_to?(:to_i)
-                options.to_i
-              elsif options.respond_to?(:fetch)
-                options[:transaction_id] || options[:xid] || 0
-              else
-                fail TypeError
-              end
-        { transaction_id: xid }
-      end
-    end
   end
 end

lib/pio/open_flow10/features.rb

@@ -10,17 +10,16 @@ class Format < BinData::Record
         extend OpenFlow::Format
 
         header version: 1, message_type: OpenFlow::FEATURES_REQUEST
-        string :body
+        string :body, value: ''
 
         def user_data
           body
         end
       end
 
       def initialize(user_options = {})
-        header_options = OpenFlowHeader::Options.parse(user_options)
-        body_options = user_options[:body] || user_options[:user_data] || ''
-        @format = Format.new(header: header_options, body: body_options)
+        validate_user_options user_options
+        @format = Format.new(header: parse_header_options(user_options))
       end
     end
 
@@ -101,21 +100,20 @@ def physical_ports
         end
       end
 
-      # rubocop:disable MethodLength
+      body_option :dpid
+      body_option :datapath_id
+      body_option :n_buffers
+      body_option :n_tables
+      body_option :capabilities
+      body_option :actions
+      body_option :ports
+
       def initialize(user_options = {})
-        header_options = OpenFlowHeader::Options.parse(user_options)
-        body_options = if user_options.respond_to?(:fetch)
-                         user_options.delete :transaction_id
-                         user_options.delete :xid
-                         dpid = user_options[:dpid]
-                         user_options[:datapath_id] = dpid if dpid
-                         user_options
-                       else
-                         ''
-                       end
+        validate_user_options user_options
+        header_options = parse_header_options(user_options)
+        body_options = parse_body_options(user_options)
         @format = Format.new(header: header_options, body: body_options)
       end
-      # rubocop:enable MethodLength
     end
   end
 end

lib/pio/open_flow10/flow_mod.rb

@@ -84,7 +84,7 @@ class Format < BinData::Record
 
     # rubocop:disable MethodLength
     def initialize(user_options = {})
-      header_options = OpenFlowHeader::Options.parse(user_options)
+      header_options = parse_header_options(user_options)
       body_options = if user_options.respond_to?(:fetch)
                        user_options.delete :transaction_id
                        user_options.delete :xid

lib/pio/open_flow10/hello.rb

@@ -16,16 +16,7 @@ def user_data
       end
     end
 
-    user_option :transaction_id
-    user_option :xid
-    user_option :body
-    user_option :user_data
-
-    def initialize(user_options = {})
-      validate_user_options user_options
-      header_options = OpenFlowHeader::Options.parse(user_options)
-      body_options = user_options[:body] || user_options[:user_data] || ''
-      @format = Format.new(header: header_options, body: body_options)
-    end
+    body_option :body
+    body_option :user_data
   end
 end

lib/pio/open_flow10/packet_in.rb

@@ -71,7 +71,7 @@ class Format < BinData::Record
 
     # rubocop:disable MethodLength
     def initialize(user_options = {})
-      header_options = OpenFlowHeader::Options.parse(user_options)
+      header_options = parse_header_options(user_options)
       body_options = if user_options.respond_to?(:fetch)
                        user_options.delete :transaction_id
                        user_options.delete :xid

lib/pio/open_flow10/packet_out.rb

@@ -33,7 +33,7 @@ class Format < BinData::Record
 
     # rubocop:disable MethodLength
     def initialize(user_options = {})
-      header_options = OpenFlowHeader::Options.parse(user_options)
+      header_options = parse_header_options(user_options)
       body_options = if user_options.respond_to?(:fetch)
                        user_options.delete :transaction_id
                        user_options.delete :xid

lib/pio/open_flow10/port_status.rb

@@ -47,7 +47,7 @@ def desc
 
     # rubocop:disable MethodLength
     def initialize(user_options = {})
-      header_options = OpenFlowHeader::Options.parse(user_options)
+      header_options = parse_header_options(user_options)
       body_options = if user_options.respond_to?(:fetch)
                        user_options.delete :transaction_id
                        user_options.delete :xid

lib/pio/open_flow13/features_reply.rb

@@ -51,13 +51,11 @@ def dpid
         end
       end
 
-      def initialize(user_attrs = {})
-        header_options = OpenFlowHeader::Options.parse(user_attrs)
-        body_options = user_attrs.dup
-        body_options[:datapath_id] =
-          body_options[:dpid] || body_options[:datapath_id]
-        @format = Format.new(header: header_options, body: body_options)
-      end
+      body_option :dpid
+      body_option :datapath_id
+      body_option :n_buffers
+      body_option :n_tables
+      body_option :capabilities
     end
   end
 end

lib/pio/open_flow13/features_request.rb

@@ -4,7 +4,7 @@
 module Pio
   # OpenFlow 1.3 Features Request and Reply message.
   class Features
-    remove_const :Request
+    remove_const(:Request) if const_defined?(:Request)
 
     # OpenFlow 1.3 Features Request message.
     class Request < OpenFlow::Message
@@ -14,15 +14,10 @@ class Format < BinData::Record
 
         header version: 4, message_type: 5
         string :body, value: ''
-      end
 
-      def initialize(user_attrs = {})
-        unknown_attrs = user_attrs.keys - [:transaction_id, :xid]
-        unless unknown_attrs.empty?
-          fail "Unknown option: #{unknown_attrs.first}"
+        def user_data
+          body
         end
-        header_options = OpenFlowHeader::Options.parse(user_attrs)
-        @format = Format.new(header: header_options)
       end
     end
   end

lib/pio/open_flow13/flow_mod.rb

@@ -146,7 +146,7 @@ class Format < BinData::Record
     end
 
     def initialize(user_attrs = {})
-      header_attrs = OpenFlowHeader::Options.parse(user_attrs)
+      header_attrs = parse_header_options(user_attrs)
       body_attrs = { table_id: user_attrs[:table_id],
                      match: user_attrs[:match],
                      priority: user_attrs[:priority],

lib/pio/open_flow13/hello.rb

@@ -70,12 +70,9 @@ def version_bitmap
       end
     end
 
-    user_option :transaction_id
-    user_option :xid
-
     def initialize(user_options = {})
       validate_user_options user_options
-      header_options = OpenFlowHeader::Options.parse(user_options)
+      header_options = parse_header_options(user_options)
       body_options = { elements: [{ element_type: VERSION_BITMAP,
                                     element_length: 8,
                                     element_value: 0b10000 }] }

lib/pio/open_flow13/packet_in.rb

@@ -68,7 +68,7 @@ def method_missing(method, *args)
     end
 
     def initialize(user_attrs = {})
-      header_attrs = OpenFlowHeader::Options.parse(user_attrs)
+      header_attrs = parse_header_options(user_attrs)
       body_attrs = { raw_data: user_attrs[:raw_data] }
       @format = Format.new(header: header_attrs, body: body_attrs)
     end

lib/pio/open_flow13/packet_out.rb

@@ -57,7 +57,7 @@ class Format < BinData::Record
     end
 
     def initialize(user_attrs = {})
-      header_attrs = OpenFlowHeader::Options.parse(user_attrs)
+      header_attrs = parse_header_options(user_attrs)
       body_attrs = { actions: user_attrs[:actions],
                      raw_data: user_attrs[:raw_data] }
       @format = Format.new(header: header_attrs, body: body_attrs)

spec/pio/open_flow10/features_request_spec.rb

@@ -0,0 +1,12 @@
+require 'pio/open_flow10/features'
+
+describe Pio::Features::Request do
+  describe '.new' do
+    it_should_behave_like('an OpenFlow message', Pio::Features::Request)
+
+    context "with body: 'abcde'" do
+      When(:message) { Pio::Features::Request.new(body: 'abcde') }
+      Then { message == Failure(RuntimeError, 'Unknown option: body') }
+    end
+  end
+end

spec/pio/open_flow13/features_request_spec.rb

@@ -0,0 +1,12 @@
+require 'pio/open_flow13/features_request'
+
+describe Pio::Features::Request do
+  describe '.new' do
+    it_should_behave_like('an OpenFlow message', Pio::Features::Request)
+
+    context "with body: 'abcde'" do
+      When(:message) { Pio::Features::Request.new(body: 'abcde') }
+      Then { message == Failure(RuntimeError, 'Unknown option: body') }
+    end
+  end
+end