Refactoring

Rakefile

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

features/open_flow13/packet_out.feature

@@ -51,5 +51,5 @@ Feature: PacketOut
       | operation               |                 1 |
       | sender_hardware_address | ac:5d:10:31:37:79 |
       | sender_protocol_address |     192.168.2.254 |
-      | target_hardware_address | ff:ff:ff:ff:ff:ff |
+      | target_hardware_address | 00:00:00:00:00:00 |
       | target_protocol_address |       192.168.2.5 |

features/open_flow13/set_arp_operation.feature

@@ -4,7 +4,7 @@ Feature: SetArpOperation
   Scenario: new(Pio::Arp::Reply::OPERATION)
     When I create an OpenFlow action with:
       """
-      Pio::SetArpOperation.new(Pio::Arp::Reply::OPERATION)
+      Pio::SetArpOperation.new(Pio::Arp::Reply.operation)
       """
     Then the action has the following fields and values:
       | field         | value |

lib/pio/arp.rb

@@ -1,14 +1,18 @@
 require 'pio/arp/format'
-require 'pio/arp/request'
 require 'pio/arp/reply'
-require 'pio/message_type_selector'
+require 'pio/arp/request'
+require 'pio/parse_error'
 
 # Packet parser and generator library.
 module Pio
   # ARP parser and generator.
   class Arp
-    extend MessageTypeSelector
-    message_type Request::OPERATION => Request, Reply::OPERATION => Reply
+    def self.read(raw_data)
+      format = Format.read(raw_data)
+      { Request.operation => Request,
+        Reply.operation => Reply }.fetch(format.operation).create(format)
+    rescue
+      raise Pio::ParseError, $ERROR_INFO.message
+    end
   end
-  ARP = Arp
 end

lib/pio/arp/format.rb

@@ -23,10 +23,6 @@ class Format < BinData::Record
       mac_address :target_hardware_address
       ip_address :target_protocol_address
 
-      def message_type
-        operation
-      end
-
       # rubocop:disable MethodLength
       def to_exact_match(in_port)
         match_options = {

lib/pio/arp/message.rb

@@ -1,16 +1,18 @@
 require 'pio/arp/format'
-require 'pio/ruby_dumper'
+require 'pio/message'
 
 module Pio
   class Arp
     # Base class of ARP Request and Reply
-    class Message
-      include RubyDumper
-      private_class_method :new
+    class Message < Pio::Message
+      def self.create(format)
+        allocate.tap do |message|
+          message.instance_variable_set :@format, format
+        end
+      end
 
       def initialize(user_options)
-        options = self.class.const_get(:Options).new(user_options.dup.freeze)
-        @format = Arp::Format.new(options.to_hash)
+        @format = Arp::Format.new(parse_options(user_options))
       end
 
       def method_missing(method, *args)

lib/pio/arp/reply.rb

@@ -1,46 +1,20 @@
 require 'pio/arp/message'
 require 'pio/instance_inspector'
 require 'pio/mac'
-require 'pio/options'
 
 module Pio
   class Arp
     # ARP Reply packet generator
     class Reply < Message
       include InstanceInspector
 
-      OPERATION = 2
-      public_class_method :new
-
-      # User options for creating an Arp Reply.
-      class Options < Pio::Options
-        mandatory_option :source_mac
-        mandatory_option :destination_mac
-        mandatory_option :sender_protocol_address
-        mandatory_option :target_protocol_address
-
-        def initialize(options)
-          validate options
-          @source_mac = Mac.new(options[:source_mac]).freeze
-          @destination_mac = Mac.new(options[:destination_mac]).freeze
-          @sender_protocol_address =
-            IPv4Address.new(options[:sender_protocol_address]).freeze
-          @target_protocol_address =
-            IPv4Address.new(options[:target_protocol_address]).freeze
-        end
-
-        def to_hash
-          {
-            operation: OPERATION,
-            source_mac: @source_mac,
-            destination_mac: @destination_mac,
-            sender_hardware_address: @source_mac,
-            target_hardware_address: @destination_mac,
-            sender_protocol_address: @sender_protocol_address,
-            target_protocol_address: @target_protocol_address
-          }.freeze
-        end
-      end
+      options operation: 2,
+              source_mac: :source_mac,
+              destination_mac: :destination_mac,
+              sender_hardware_address: :source_mac,
+              target_hardware_address: :destination_mac,
+              sender_protocol_address: :sender_protocol_address,
+              target_protocol_address: :target_protocol_address
     end
   end
 end

lib/pio/arp/request.rb

@@ -1,47 +1,20 @@
 require 'pio/arp/message'
 require 'pio/instance_inspector'
 require 'pio/mac'
-require 'pio/options'
 
 module Pio
   class Arp
     # ARP Request packet generator
     class Request < Message
       include InstanceInspector
 
-      OPERATION = 1
-      public_class_method :new
-
-      # User options for creating an Arp Request.
-      class Options < Pio::Options
-        mandatory_option :source_mac
-        mandatory_option :sender_protocol_address
-        mandatory_option :target_protocol_address
-
-        BROADCAST_MAC = Mac.new(0xffffffffffff).freeze
-        ALL_ZERO_MAC = Mac.new(0).freeze
-
-        def initialize(options)
-          validate options
-          @source_mac = Mac.new(options[:source_mac]).freeze
-          @sender_protocol_address =
-            IPv4Address.new(options[:sender_protocol_address]).freeze
-          @target_protocol_address =
-            IPv4Address.new(options[:target_protocol_address]).freeze
-        end
-
-        def to_hash
-          {
-            operation: OPERATION,
-            source_mac: @source_mac,
-            destination_mac: BROADCAST_MAC,
-            sender_hardware_address: @source_mac,
-            target_hardware_address: ALL_ZERO_MAC,
-            sender_protocol_address: @sender_protocol_address,
-            target_protocol_address: @target_protocol_address
-          }.freeze
-        end
-      end
+      options operation: 1,
+              source_mac: :source_mac,
+              destination_mac: 'ff:ff:ff:ff:ff:ff'.freeze,
+              sender_hardware_address: :source_mac,
+              target_hardware_address: '00:00:00:00:00:00'.freeze,
+              sender_protocol_address: :sender_protocol_address,
+              target_protocol_address: :target_protocol_address
     end
   end
 end

lib/pio/message.rb

@@ -0,0 +1,40 @@
+require 'active_support/core_ext/class/attribute'
+require 'pio/ruby_dumper'
+
+module Pio
+  # Base message class
+  class Message
+    include RubyDumper
+
+    class_attribute :options
+
+    def self.options(options)
+      options.each_pair do |key, value|
+        next if value.is_a?(Symbol)
+        class_eval { class_attribute key }
+        class_eval { __send__("#{key}=", value) }
+      end
+      self.options = options
+    end
+
+    private
+
+    # rubocop:disable MethodLength
+    def parse_options(user_options)
+      options = {}
+      self.class.options.each_pair do |key, value|
+        if value.is_a?(Symbol)
+          begin
+            options[key] = user_options.fetch(value)
+          rescue
+            raise "#{key} option is a mandatory"
+          end
+        else
+          options[key] = user_options[key] || value
+        end
+      end
+      options
+    end
+    # rubocop:enable MethodLength
+  end
+end

lib/pio/open_flow10/packet_in.rb

@@ -1,3 +1,4 @@
+require 'active_support/core_ext/object/try'
 require 'pio/open_flow/message'
 require 'pio/open_flow10/packet_in/reason'
 require 'pio/parser'
@@ -46,7 +47,8 @@ def inspect
       # rubocop:enable LineLength
 
       def method_missing(method, *args)
-        data.__send__(method, *args).snapshot
+        bindata_value = data.__send__(method, *args)
+        bindata_value.try(:snapshot) || bindata_value
       end
 
       attr_accessor :datapath_id

lib/pio/open_flow13/packet_in.rb

@@ -1,3 +1,4 @@
+require 'active_support/core_ext/object/try'
 require 'pio/open_flow'
 require 'pio/open_flow13/match'
 require 'pio/parser'
@@ -49,7 +50,8 @@ def in_port
       end
 
       def method_missing(method, *args)
-        data.__send__(method, *args).snapshot
+        bindata_value = data.__send__(method, *args)
+        bindata_value.try(:snapshot) || bindata_value
       end
     end
   end

lib/pio/open_flow13/packet_out.rb

@@ -1,3 +1,4 @@
+require 'active_support/core_ext/object/try'
 require 'pio/open_flow'
 require 'pio/open_flow/buffer_id'
 require 'pio/open_flow13/actions'
@@ -38,7 +39,8 @@ def data
       end
 
       def method_missing(method, *args)
-        data.__send__(method, *args).snapshot
+        bindata_value = data.__send__(method, *args)
+        bindata_value.try(:snapshot) || bindata_value
       end
     end
   end

lib/pio/options.rb

@@ -14,6 +14,11 @@ def self.option(name)
       const_get(:OPTIONS) << name
     end
 
+    def initialize(options)
+      validate options
+      @options = options
+    end
+
     private
 
     def validate(user_options)

lib/pio/ruby_dumper.rb

@@ -41,7 +41,7 @@ def to_ruby
             bit_names = []
             bit = false
           end
-          list = __send__(each).to_bytes
+          list = (@format || self).__send__(each).to_bytes
           next if list.empty?
           bytes << "  #{list}, # #{each}\n"
           pack_template << 'C' * (list.count(',') + 1)