Refactoring the redirect method for the router api.

rails · Nov 18, 2011 · 99d94f1 · 99d94f1
1 parent d34efdd
commit 99d94f1
Show file tree

Hide file tree

Showing 2 changed files with 24 additions and 25 deletions.
diff --git a/actionpack/lib/action_dispatch/routing/redirection.rb b/actionpack/lib/action_dispatch/routing/redirection.rb
@@ -3,7 +3,7 @@
 
 module ActionDispatch
   module Routing
-    class Redirect
+    class Redirect # :nodoc:
       attr_reader :status, :block
 
       def initialize(status, block)
@@ -35,7 +35,7 @@ def path(params, request)
       end
     end
 
-    class OptionRedirect < Redirect
+    class OptionRedirect < Redirect # :nodoc:
       alias :options :block
 
       def path(params, request)
@@ -89,33 +89,27 @@ def redirect(*args, &block)
         options = args.last.is_a?(Hash) ? args.pop : {}
         status  = options.delete(:status) || 301
 
+        return OptionRedirect.new(status, options) if options.any?
+
         path = args.shift
 
-        if path.is_a?(String)
+        block = lambda { |params, request|
-          block_redirect status, lambda { |params, request|
+          (params.empty? || !path.match(/%\{\w*\}/)) ? path : (path % params)
-            (params.empty? || !path.match(/%\{\w*\}/)) ? path : (path % params)
+        } if String === path
-          }
+
-        elsif options.any?
+        block = path if path.respond_to? :call
-          OptionRedirect.new(status, options)
-        elsif path.respond_to?(:call)
-          block_redirect status, path
-        elsif block
-          if block.arity < 2
-            msg = "redirect blocks with arity of #{block.arity} are deprecated. Your block must take 2 parameters: the environment, and a request object"
-            ActiveSupport::Deprecation.warn msg
-            block_redirect status, lambda { |params, _| block.call(params) }
-          else
-            block_redirect status, block
-          end
-        else
-          raise ArgumentError, "redirection argument not supported"
-        end
-      end
 
-      private
+        # :FIXME: remove in Rails 4.0
-        def block_redirect(status, path_proc)
+        if block && block.respond_to?(:arity) && block.arity < 2
-          Redirect.new status, path_proc
+          msg = "redirect blocks with arity of #{block.arity} are deprecated. Your block must take 2 parameters: the environment, and a request object"
+          ActiveSupport::Deprecation.warn msg
+          block = lambda { |params, _| block.call(params) }
         end
+
+        raise ArgumentError, "redirection argument not supported" unless block
+
+        Redirect.new status, block
+      end
     end
   end
 end
diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb
@@ -2299,6 +2299,11 @@ def test_named_routes_collision_is_avoided_unless_explicitly_given_as
     assert_equal "/forced_collision", routes_forced_collision_path
   end
 
+  def test_redirect_argument_error
+    routes = Class.new { include ActionDispatch::Routing::Redirection }.new
+    assert_raises(ArgumentError) { routes.redirect Object.new }
+  end
+
   def test_explicitly_avoiding_the_named_route
     assert !respond_to?(:routes_no_collision_path)
   end