simplify the Middleware constructor

We should do the hard work outside the constructor.  Also fix the tests
to not directly construct middleware objects, but to go through the
stack object.

2 files changed, 30 insertions, 18 deletions

diff --git a/actionpack/lib/action_controller/metal.rb b/actionpack/lib/action_controller/metal.rb
index dc88557e3a..817339dadf 100644
--- a/actionpack/lib/action_controller/metal.rb
+++ b/actionpack/lib/action_controller/metal.rb
@@ -11,18 +11,16 @@ module ActionController
   #
   class MiddlewareStack < ActionDispatch::MiddlewareStack #:nodoc:
     class Middleware < ActionDispatch::MiddlewareStack::Middleware #:nodoc:
-      def initialize(klass, *args, &block)
-        options = args.extract_options!
-        @only   = Array(options.delete(:only)).map(&:to_s)
-        @except = Array(options.delete(:except)).map(&:to_s)
-        args << options unless options.empty?
-        super
+      def initialize(klass, args, only, except, block)
+        @only = only
+        @except = except
+        super(klass, args, block)
       end
 
       def valid?(action)
-        if @only.present?
+        if @only.any?
           @only.include?(action)
-        elsif @except.present?
+        elsif @except.any?
           !@except.include?(action)
         else
           true
@@ -37,6 +35,17 @@ module ActionController
         middleware.valid?(action) ? middleware.build(a) : a
       end
     end
+
+    private
+
+    def build_middleware(klass, args, block)
+      options = args.extract_options!
+      only   = Array(options.delete(:only)).map(&:to_s)
+      except = Array(options.delete(:except)).map(&:to_s)
+      args << options unless options.empty?
+
+      Middleware.new(klass, args, only, except, block)
+    end
   end
 
   # <tt>ActionController::Metal</tt> is the simplest possible controller, providing a
diff --git a/actionpack/lib/action_dispatch/middleware/stack.rb b/actionpack/lib/action_dispatch/middleware/stack.rb
index 87e627319e..b39a502d77 100644
--- a/actionpack/lib/action_dispatch/middleware/stack.rb
+++ b/actionpack/lib/action_dispatch/middleware/stack.rb
@@ -6,7 +6,7 @@ module ActionDispatch
     class Middleware
       attr_reader :args, :block, :name, :classcache
 
-      def initialize(klass_or_name, *args, &block)
+      def initialize(klass_or_name, args, block)
         @klass = nil
 
         if klass_or_name.respond_to?(:name)
@@ -75,19 +75,17 @@ module ActionDispatch
       middlewares[i]
     end
 
-    def unshift(*args, &block)
-      middleware = self.class::Middleware.new(*args, &block)
-      middlewares.unshift(middleware)
+    def unshift(klass, *args, &block)
+      middlewares.unshift(build_middleware(klass, args, block))
     end
 
     def initialize_copy(other)
       self.middlewares = other.middlewares.dup
     end
 
-    def insert(index, *args, &block)
+    def insert(index, klass, *args, &block)
       index = assert_index(index, :before)
-      middleware = self.class::Middleware.new(*args, &block)
-      middlewares.insert(index, middleware)
+      middlewares.insert(index, build_middleware(klass, args, block))
     end
 
     alias_method :insert_before, :insert
@@ -107,9 +105,8 @@ module ActionDispatch
       middlewares.delete target
     end
 
-    def use(*args, &block)
-      middleware = self.class::Middleware.new(*args, &block)
-      middlewares.push(middleware)
+    def use(klass, *args, &block)
+      middlewares.push(build_middleware(klass, args, block))
     end
 
     def build(app = Proc.new)
@@ -123,5 +120,11 @@ module ActionDispatch
       raise "No such middleware to insert #{where}: #{index.inspect}" unless i
       i
     end
+
+    private
+
+    def build_middleware(klass, args, block)
+      Middleware.new(klass, args, block)
+    end
   end
 end