Refactor Enqueue class

Delete Enqueue::Format class using Pio::OpenFlow::Action. Refs #253.
trema · Oct 26, 2015 · c0b641b · c0b641b
1 parent b3a20d8
commit c0b641b
Show file tree

Hide file tree

Showing 5 changed files with 43 additions and 60 deletions.
diff --git a/features/open_flow10/enqueue.feature b/features/open_flow10/enqueue.feature
@@ -1,10 +1,10 @@
 @open_flow10
-Feature: Pio::Enqueue
+Feature: Pio::OpenFlow10::Enqueue
 
   Scenario: new(port: 1, queue_id: 2)
     When I try to create an OpenFlow action with:
       """
-      Pio::Enqueue.new(port: 1, queue_id: 2)
+      Pio::OpenFlow10::Enqueue.new(port: 1, queue_id: 2)
       """
     Then it should finish successfully
     And the action has the following fields and values:

diff --git a/lib/pio/open_flow10/actions.rb b/lib/pio/open_flow10/actions.rb
@@ -26,7 +26,7 @@ class Actions < BinData::Primitive
         8 => Pio::OpenFlow10::SetIpTos,
         9 => Pio::OpenFlow10::SetTransportSourcePort,
         10 => Pio::OpenFlow10::SetTransportDestinationPort,
-        11 => Pio::Enqueue,
+        11 => Pio::OpenFlow10::Enqueue,
         0xffff => Pio::VendorAction
       }
 

diff --git a/lib/pio/open_flow10/enqueue.rb b/lib/pio/open_flow10/enqueue.rb
@@ -1,62 +1,43 @@
-require 'bindata'
-require 'forwardable'
 require 'pio/monkey_patch/integer'
+require 'pio/open_flow/action'
 require 'pio/open_flow10/port16'
 
 module Pio
-  # An action to enqueue the packet on the specified queue attached to
-  # a port.
-  class Enqueue
-    # OpenFlow 1.0 OFPAT_ENQUEUE action format.
-    class Format < BinData::Record
-      endian :big
-
-      uint16 :action_type, value: 11
-      uint16 :action_length, value: 16
+  module OpenFlow10
+    # An action to enqueue the packet on the specified queue attached
+    # to a port.
+    class Enqueue < OpenFlow::Action
+      action_header action_type: 11, action_length: 16
       port16 :port
-      uint48 :padding
+      string :padding, length: 6
       hide :padding
       uint32 :queue_id
-    end
-
-    def self.read(raw_data)
-      enqueue = allocate
-      enqueue.instance_variable_set :@format, Format.read(raw_data)
-      enqueue
-    end
-
-    extend Forwardable
 
-    def_delegators :@format, :action_type
-    def_delegator :@format, :action_length, :length
-    def_delegators :@format, :port
-    def_delegators :@format, :queue_id
-    def_delegator :@format, :to_binary_s, :to_binary
-
-    def initialize(user_options)
-      validate_port user_options
-      validate_queue_id user_options
-      @format = Format.new(user_options)
-    end
+      def initialize(user_options)
+        validate_port user_options
+        validate_queue_id user_options
+        super(user_options)
+      end
 
-    private
+      private
 
-    def validate_port(user_options)
-      port = user_options.fetch(:port)
-      if port.is_a?(Symbol) && port != :in_port
-        fail(ArgumentError,
-             ':port must be a valid physical port or :in_port')
+      def validate_port(user_options)
+        port = user_options.fetch(:port)
+        if port.is_a?(Symbol) && port != :in_port
+          fail(ArgumentError,
+               ':port must be a valid physical port or :in_port')
+        end
+      rescue KeyError
+        raise ArgumentError, ':port is a mandatory option'
       end
-    rescue KeyError
-      raise ArgumentError, ':port is a mandatory option'
-    end
 
-    def validate_queue_id(user_options)
-      unless user_options.fetch(:queue_id).unsigned_32bit?
-        fail ArgumentError, ':queue_id must be an unsigned 32-bit integer'
+      def validate_queue_id(user_options)
+        unless user_options.fetch(:queue_id).unsigned_32bit?
+          fail ArgumentError, ':queue_id must be an unsigned 32-bit integer'
+        end
+      rescue KeyError
+        raise ArgumentError, ':queue_id is a mandatory option'
       end
-    rescue KeyError
-      raise ArgumentError, ':queue_id is a mandatory option'
     end
   end
 end
diff --git a/spec/pio/open_flow10/enqueue_spec.rb b/spec/pio/open_flow10/enqueue_spec.rb
@@ -1,9 +1,9 @@
 require 'pio/open_flow10/enqueue'
 
-describe Pio::Enqueue do
+describe Pio::OpenFlow10::Enqueue do
   describe '.new' do
     context 'with port: 1, queue_id: 2' do
-      When(:enqueue) { Pio::Enqueue.new(port: 1, queue_id: 2) }
+      When(:enqueue) { Pio::OpenFlow10::Enqueue.new(port: 1, queue_id: 2) }
 
       describe '#port' do
         Then { enqueue.port == 1 }
@@ -27,40 +27,42 @@
     end
 
     context 'with port: :in_port, queue_id: 2' do
-      When(:enqueue) { Pio::Enqueue.new(port: :in_port, queue_id: 2) }
+      When(:enqueue) do
+        Pio::OpenFlow10::Enqueue.new(port: :in_port, queue_id: 2)
+      end
 
       describe '#port' do
         Then { enqueue.port == :in_port }
       end
     end
 
     context 'with port: :local, queue_id: 2' do
-      When(:enqueue) { Pio::Enqueue.new(port: :local, queue_id: 2) }
+      When(:enqueue) { Pio::OpenFlow10::Enqueue.new(port: :local, queue_id: 2) }
       Then { enqueue == Failure(ArgumentError) }
     end
 
     context 'with port: -1, queue_id: 2' do
-      When(:enqueue) { Pio::Enqueue.new(port: -1, queue_id: 2) }
+      When(:enqueue) { Pio::OpenFlow10::Enqueue.new(port: -1, queue_id: 2) }
       Then { enqueue == Failure(ArgumentError) }
     end
 
     context 'with port: 0xff00, queue_id: 2' do
-      When(:enqueue) { Pio::Enqueue.new(port: 0xff00, queue_id: 2) }
+      When(:enqueue) { Pio::OpenFlow10::Enqueue.new(port: 0xff00, queue_id: 2) }
       Then { enqueue == Failure(ArgumentError) }
     end
 
     context 'with port: 1, queue_id: -2' do
-      When(:enqueue) { Pio::Enqueue.new(port: 1, queue_id: -2) }
+      When(:enqueue) { Pio::OpenFlow10::Enqueue.new(port: 1, queue_id: -2) }
       Then { enqueue == Failure(ArgumentError) }
     end
 
     context 'with port: 1' do
-      When(:enqueue) { Pio::Enqueue.new(port: 1) }
+      When(:enqueue) { Pio::OpenFlow10::Enqueue.new(port: 1) }
       Then { enqueue == Failure(ArgumentError) }
     end
 
     context 'with queue_id: 2' do
-      When(:enqueue) { Pio::Enqueue.new(queue_id: 2) }
+      When(:enqueue) { Pio::OpenFlow10::Enqueue.new(queue_id: 2) }
       Then { enqueue == Failure(ArgumentError) }
     end
   end

diff --git a/spec/pio/open_flow10/packet_out_spec.rb b/spec/pio/open_flow10/packet_out_spec.rb
@@ -317,15 +317,15 @@
           transaction_id: 0x16,
           buffer_id: 0xffffffff,
           in_port: 0xffff,
-          actions: Pio::Enqueue.new(port: 1, queue_id: 2),
+          actions: Pio::OpenFlow10::Enqueue.new(port: 1, queue_id: 2),
           raw_data: data_dump
         }
       end
 
       Then { result.message_length == 0x60 }
       Then { result.actions_len == 0x10 }
       Then { result.actions.length == 1 }
-      Then { result.actions[0].is_a? Pio::Enqueue }
+      Then { result.actions[0].is_a? Pio::OpenFlow10::Enqueue }
       Then { result.actions[0].port == 1 }
       Then { result.actions[0].queue_id == 2 }
     end