stop calling url_for with recall parameters and actually use a request

author: Aaron Patterson <aaron.patterson@gmail.com> 2014-07-02 15:02:36 -0700
committer: Aaron Patterson <aaron.patterson@gmail.com> 2014-07-15 15:19:41 -0700
commit: 8eb7bcb6e59e5720e3bd322511817804c8087043 (patch)
tree: ef088233869d43cbd05e8dec6603e602db8f2da9 /actionpack
parent: a3ca700c58fa59722288a12960c4a6cd1ade6aea (diff)
download: rails-8eb7bcb6e59e5720e3bd322511817804c8087043.tar.gz
rails-8eb7bcb6e59e5720e3bd322511817804c8087043.tar.bz2
rails-8eb7bcb6e59e5720e3bd322511817804c8087043.zip
2 files changed, 197 insertions, 85 deletions
diff --git a/actionpack/test/abstract_unit.rb b/actionpack/test/abstract_unit.rb
index 6584d20840..d027f1a88d 100644
--- a/actionpack/test/abstract_unit.rb
+++ b/actionpack/test/abstract_unit.rb
@@ -305,15 +305,44 @@ end
 
 module ActionDispatch
   module RoutingVerbs
-    def get(uri_or_host, path = nil)
+    def send_request(uri_or_host, method, path)
       host = uri_or_host.host unless path
       path ||= uri_or_host.path
 
       params = {'PATH_INFO'      => path,
-                'REQUEST_METHOD' => 'GET',
+                'REQUEST_METHOD' => method,
                 'HTTP_HOST'      => host}
 
-      routes.call(params)[2].join
+      routes.call(params)
+    end
+
+    def request_path_params(path, method: 'GET')
+      resp = send_request URI('http://localhost' + path), method.to_s.upcase, nil
+      status, headers, body = *resp
+      if status == 404
+        raise ActionController::RoutingError, "No route matches #{path.inspect}"
+      end
+      controller.request.path_parameters
+    end
+
+    def get(uri_or_host, path = nil)
+      send_request(uri_or_host, 'GET', path)[2].join
+    end
+
+    def post(uri_or_host, path = nil)
+      send_request(uri_or_host, 'POST', path)[2].join
+    end
+
+    def put(uri_or_host, path = nil)
+      send_request(uri_or_host, 'PUT', path)[2].join
+    end
+
+    def delete(uri_or_host, path = nil)
+      send_request(uri_or_host, 'DELETE', path)[2].join
+    end
+
+    def patch(uri_or_host, path = nil)
+      send_request(uri_or_host, 'PATCH', path)[2].join
     end
   end
 end
@@ -322,6 +351,48 @@ module RoutingTestHelpers
   def url_for(set, options, recall = {})
     set.url_for options.merge(:only_path => true, :_recall => recall)
   end
+
+  def make_set(strict = true)
+    tc = self
+    TestSet.new ->(c) { tc.controller = c }, strict
+  end
+
+  class TestSet < ActionDispatch::Routing::RouteSet
+    attr_reader :strict
+
+    def initialize(block, strict = false)
+      @block = block
+      @strict = strict
+      super()
+    end
+
+    class Dispatcher < ActionDispatch::Routing::RouteSet::Dispatcher
+      def initialize(defaults, set, block)
+        super(defaults)
+        @block = block
+        @set = set
+      end
+
+      def controller(params, default_controller=true)
+        super(params, @set.strict)
+      end
+
+      def controller_reference(controller_param)
+        block = @block
+        set = @set
+        super if @set.strict
+        Class.new(ActionController::Base) {
+          include set.url_helpers
+          define_method(:process) { |name| block.call(self) }
+          def to_a; [200, {}, []]; end
+        }
+      end
+    end
+
+    def dispatcher defaults
+      TestSet::Dispatcher.new defaults, self, @block
+    end
+  end
 end
 
 class ResourcesController < ActionController::Base
diff --git a/actionpack/test/controller/routing_test.rb b/actionpack/test/controller/routing_test.rb
index 1c40e0e723..c18914cc8e 100644
--- a/actionpack/test/controller/routing_test.rb
+++ b/actionpack/test/controller/routing_test.rb
@@ -77,10 +77,11 @@ class LegacyRouteSetTests < ActiveSupport::TestCase
   include ActionDispatch::RoutingVerbs
 
   attr_reader :rs
+  attr_accessor :controller
   alias :routes :rs
 
   def setup
-    @rs       = ::ActionDispatch::Routing::RouteSet.new
+    @rs = make_set
     @response = nil
   end
 
@@ -317,11 +318,16 @@ class LegacyRouteSetTests < ActiveSupport::TestCase
 
     assert_equal '/admin/user/show/10', url_for(rs, { :controller => 'admin/user', :action => 'show', :id => 10 })
 
-    assert_equal '/admin/user/show',    url_for(rs, { :action => 'show' }, { :controller => 'admin/user', :action => 'list', :id => '10' })
-    assert_equal '/admin/user/list/10', url_for(rs, {}, { :controller => 'admin/user', :action => 'list', :id => '10' })
+    get URI('http://test.host/admin/user/list/10')
 
-    assert_equal '/admin/stuff', url_for(rs, { :controller => 'stuff' }, { :controller => 'admin/user', :action => 'list', :id => '10' })
-    assert_equal '/stuff',       url_for(rs, { :controller => '/stuff' }, { :controller => 'admin/user', :action => 'list', :id => '10' })
+    assert_equal({ :controller => 'admin/user', :action => 'list', :id => '10' },
+                 controller.request.path_parameters)
+
+    assert_equal '/admin/user/show',    controller.url_for({ :action => 'show', :only_path => true })
+    assert_equal '/admin/user/list/10', controller.url_for({:only_path => true})
+
+    assert_equal '/admin/stuff', controller.url_for({ :controller => 'stuff', :only_path => true })
+    assert_equal '/stuff', controller.url_for({ :controller => '/stuff', :only_path => true })
   end
 
   def test_ignores_leading_slash
@@ -518,9 +524,10 @@ class LegacyRouteSetTests < ActiveSupport::TestCase
   def test_changing_controller
     rs.draw { get ':controller/:action/:id' }
 
+    get URI('http://test.host/admin/user/index/10')
+
     assert_equal '/admin/stuff/show/10',
-        url_for(rs, {:controller => 'stuff', :action => 'show', :id => 10},
-                    {:controller => 'admin/user', :action => 'index'})
+        controller.url_for({:controller => 'stuff', :action => 'show', :id => 10, :only_path => true})
   end
 
   def test_paths_escaped
@@ -579,8 +586,12 @@ class LegacyRouteSetTests < ActiveSupport::TestCase
       get '*path' => 'content#show_file'
     end
 
+    get URI('http://test.host/pages/boo')
+    assert_equal({:controller=>"content", :action=>"show_file", :path=>"pages/boo"},
+                 controller.request.path_parameters)
+
     assert_equal '/pages/boo',
-        url_for(rs, {}, { :controller => 'content', :action => 'show_file', :path => %w(pages boo) })
+      controller.url_for(:only_path => true)
   end
 
   def test_backwards
@@ -589,7 +600,8 @@ class LegacyRouteSetTests < ActiveSupport::TestCase
       get ':controller(/:action(/:id))'
     end
 
-    assert_equal '/page/20',   url_for(rs, { :id => 20 }, { :controller => 'pages', :action => 'show' })
+    get URI('http://test.host/pages/show')
+    assert_equal '/page/20',   controller.url_for({ :id => 20, :only_path => true })
     assert_equal '/page/20',   url_for(rs, { :controller => 'pages', :id => 20, :action => 'show' })
     assert_equal '/pages/boo', url_for(rs, { :controller => 'pages', :action => 'boo' })
   end
@@ -630,7 +642,8 @@ class LegacyRouteSetTests < ActiveSupport::TestCase
 
   def test_action_expiry
     rs.draw { get ':controller(/:action(/:id))' }
-    assert_equal '/content', url_for(rs, { :controller => 'content' }, { :controller => 'content', :action => 'show' })
+    get URI('http://test.host/content/show')
+    assert_equal '/content', controller.url_for(:controller => 'content', :only_path => true)
   end
 
   def test_requirement_should_prevent_optional_id
@@ -673,14 +686,18 @@ class LegacyRouteSetTests < ActiveSupport::TestCase
     assert_equal '/pages/2005/6/12',
       url_for(rs, { :controller => 'content', :action => 'list_pages', :year => 2005, :month => 6, :day => 12 })
 
+    get URI('http://test.host/pages/2005/6/12')
+    assert_equal({ :controller => 'content', :action => 'list_pages', :year => '2005', :month => '6', :day => '12' },
+                controller.request.path_parameters)
+
     assert_equal '/pages/2005/6/4',
-      url_for(rs, { :day => 4 },   { :controller => 'content', :action => 'list_pages', :year => '2005', :month => '6', :day => '12' })
+      controller.url_for({ :day => 4, :only_path => true })
 
     assert_equal '/pages/2005/6',
-      url_for(rs, { :day => nil }, { :controller => 'content', :action => 'list_pages', :year => '2005', :month => '6', :day => '12' })
+      controller.url_for({ :day => nil, :only_path => true })
 
     assert_equal '/pages/2005',
-      url_for(rs, { :day => nil, :month => nil }, { :controller => 'content', :action => 'list_pages', :year => '2005', :month => '6', :day => '12' })
+      controller.url_for({ :day => nil, :month => nil, :only_path => true })
   end
 
   def test_root_url_generation_with_controller_and_action
@@ -838,12 +855,15 @@ end
 
 class RouteSetTest < ActiveSupport::TestCase
   include RoutingTestHelpers
+  include ActionDispatch::RoutingVerbs
 
   attr_reader :set
+  alias :routes :set
+  attr_accessor :controller
 
   def setup
     super
-    @set = ROUTING::RouteSet.new
+    @set = make_set
   end
 
   def request
@@ -944,7 +964,8 @@ class RouteSetTest < ActiveSupport::TestCase
       get '/admin/users' => 'admin/users#index', :as => "users"
     end
 
-    MockController.build(set.url_helpers).new
+    get URI('http://test.host/people')
+    controller
   end
 
   def test_named_route_url_method
@@ -1041,12 +1062,12 @@ class RouteSetTest < ActiveSupport::TestCase
       get '/:controller(/:action(/:id))'
     end
 
-    assert_equal({:controller => 'pages', :action => 'index'}, set.recognize_path('/pages'))
-    assert_equal({:controller => 'pages', :action => 'index'}, set.recognize_path('/pages/index'))
-    assert_equal({:controller => 'pages', :action => 'list'}, set.recognize_path('/pages/list'))
+    assert_equal({:controller => 'pages', :action => 'index'}, request_path_params('/pages'))
+    assert_equal({:controller => 'pages', :action => 'index'}, request_path_params('/pages/index'))
+    assert_equal({:controller => 'pages', :action => 'list'}, request_path_params('/pages/list'))
 
-    assert_equal({:controller => 'pages', :action => 'show', :id => '10'}, set.recognize_path('/pages/show/10'))
-    assert_equal({:controller => 'pages', :action => 'show', :id => '10'}, set.recognize_path('/page/10'))
+    assert_equal({:controller => 'pages', :action => 'show', :id => '10'}, request_path_params('/pages/show/10'))
+    assert_equal({:controller => 'pages', :action => 'show', :id => '10'}, request_path_params('/page/10'))
   end
 
   def test_route_constraints_on_request_object_with_anchors_are_valid
@@ -1098,9 +1119,7 @@ class RouteSetTest < ActiveSupport::TestCase
       get    "/people" => "missing#index"
     end
 
-    assert_raise(ActionController::RoutingError) {
-      set.recognize_path("/people", :method => :get)
-    }
+    assert_raises(ActionController::RoutingError) { request_path_params '/people' }
   end
 
   def test_recognize_with_encoded_id_and_regex
@@ -1108,8 +1127,8 @@ class RouteSetTest < ActiveSupport::TestCase
       get 'page/:id' => 'pages#show', :id => /[a-zA-Z0-9\+]+/
     end
 
-    assert_equal({:controller => 'pages', :action => 'show', :id => '10'}, set.recognize_path('/page/10'))
-    assert_equal({:controller => 'pages', :action => 'show', :id => 'hello+world'}, set.recognize_path('/page/hello+world'))
+    assert_equal({:controller => 'pages', :action => 'show', :id => '10'}, request_path_params('/page/10'))
+    assert_equal({:controller => 'pages', :action => 'show', :id => 'hello+world'}, request_path_params('/page/hello+world'))
   end
 
   def test_recognize_with_http_methods
@@ -1122,40 +1141,40 @@ class RouteSetTest < ActiveSupport::TestCase
       delete "/people/:id" => "people#destroy"
     end
 
-    params = set.recognize_path("/people", :method => :get)
+    params = request_path_params("/people", :method => :get)
     assert_equal("index", params[:action])
 
-    params = set.recognize_path("/people", :method => :post)
+    params = request_path_params("/people", :method => :post)
     assert_equal("create", params[:action])
 
-    params = set.recognize_path("/people/5", :method => :put)
+    params = request_path_params("/people/5", :method => :put)
     assert_equal("update", params[:action])
 
-    params = set.recognize_path("/people/5", :method => :patch)
+    params = request_path_params("/people/5", :method => :patch)
     assert_equal("update", params[:action])
 
     assert_raise(ActionController::UnknownHttpMethod) {
-      set.recognize_path("/people", :method => :bacon)
+      request_path_params("/people", :method => :bacon)
     }
 
-    params = set.recognize_path("/people/5", :method => :get)
+    params = request_path_params("/people/5", :method => :get)
     assert_equal("show", params[:action])
     assert_equal("5", params[:id])
 
-    params = set.recognize_path("/people/5", :method => :put)
+    params = request_path_params("/people/5", :method => :put)
     assert_equal("update", params[:action])
     assert_equal("5", params[:id])
 
-    params = set.recognize_path("/people/5", :method => :patch)
+    params = request_path_params("/people/5", :method => :patch)
     assert_equal("update", params[:action])
     assert_equal("5", params[:id])
 
-    params = set.recognize_path("/people/5", :method => :delete)
+    params = request_path_params("/people/5", :method => :delete)
     assert_equal("destroy", params[:action])
     assert_equal("5", params[:id])
 
     assert_raise(ActionController::RoutingError) {
-      set.recognize_path("/people/5", :method => :post)
+      request_path_params("/people/5", :method => :post)
     }
   end
 
@@ -1165,11 +1184,11 @@ class RouteSetTest < ActiveSupport::TestCase
       root :to => "people#index"
     end
 
-    params = set.recognize_path("/people", :method => :get)
+    params = request_path_params("/people", :method => :get)
     assert_equal("people", params[:controller])
     assert_equal("index", params[:action])
 
-    params = set.recognize_path("/", :method => :get)
+    params = request_path_params("/", :method => :get)
     assert_equal("people", params[:controller])
     assert_equal("index", params[:action])
   end
@@ -1180,7 +1199,7 @@ class RouteSetTest < ActiveSupport::TestCase
              :year => /\d{4}/, :day => /\d{1,2}/, :month => /\d{1,2}/
     end
 
-    params = set.recognize_path("/articles/2005/11/05/a-very-interesting-article", :method => :get)
+    params = request_path_params("/articles/2005/11/05/a-very-interesting-article", :method => :get)
     assert_equal("permalink", params[:action])
     assert_equal("2005", params[:year])
     assert_equal("11", params[:month])
@@ -1194,7 +1213,7 @@ class RouteSetTest < ActiveSupport::TestCase
       get '/profile' => 'profile#index'
     end
 
-    set.recognize_path("/profile") rescue nil
+    request_path_params("/profile") rescue nil
 
     assert !Object.const_defined?("Profiler__"), "Profiler should not be loaded"
   end
@@ -1207,17 +1226,17 @@ class RouteSetTest < ActiveSupport::TestCase
       get "people/:id(.:format)" => "people#show"
     end
 
-    params = set.recognize_path("/people/5", :method => :get)
+    params = request_path_params("/people/5", :method => :get)
     assert_equal("show", params[:action])
     assert_equal("5", params[:id])
 
-    params = set.recognize_path("/people/5", :method => :put)
+    params = request_path_params("/people/5", :method => :put)
     assert_equal("update", params[:action])
 
-    params = set.recognize_path("/people/5", :method => :patch)
+    params = request_path_params("/people/5", :method => :patch)
     assert_equal("update", params[:action])
 
-    params = set.recognize_path("/people/5.png", :method => :get)
+    params = request_path_params("/people/5.png", :method => :get)
     assert_equal("show", params[:action])
     assert_equal("5", params[:id])
     assert_equal("png", params[:format])
@@ -1236,7 +1255,7 @@ class RouteSetTest < ActiveSupport::TestCase
   def test_root_map
     set.draw { root :to => 'people#index' }
 
-    params = set.recognize_path("", :method => :get)
+    params = request_path_params("", :method => :get)
     assert_equal("people", params[:controller])
     assert_equal("index", params[:action])
   end
@@ -1250,7 +1269,7 @@ class RouteSetTest < ActiveSupport::TestCase
 
     end
 
-    params = set.recognize_path("/api/inventory", :method => :get)
+    params = request_path_params("/api/inventory", :method => :get)
     assert_equal("api/products", params[:controller])
     assert_equal("inventory", params[:action])
   end
@@ -1262,7 +1281,7 @@ class RouteSetTest < ActiveSupport::TestCase
       end
     end
 
-    params = set.recognize_path("/api", :method => :get)
+    params = request_path_params("/api", :method => :get)
     assert_equal("api/products", params[:controller])
     assert_equal("index", params[:action])
   end
@@ -1274,7 +1293,7 @@ class RouteSetTest < ActiveSupport::TestCase
       end
     end
 
-    params = set.recognize_path("/prefix/inventory", :method => :get)
+    params = request_path_params("/prefix/inventory", :method => :get)
     assert_equal("api/products", params[:controller])
     assert_equal("inventory", params[:action])
   end
@@ -1286,38 +1305,36 @@ class RouteSetTest < ActiveSupport::TestCase
       end
     end
 
-    params = set.recognize_path("/inventory", :method => :get)
+    params = request_path_params("/inventory", :method => :get)
     assert_equal("api/products", params[:controller])
     assert_equal("inventory", params[:action])
   end
 
-  def test_generate_changes_controller_module
-    set.draw { get ':controller/:action/:id' }
-    current = { :controller => "bling/bloop", :action => "bap", :id => 9 }
-
-    assert_equal "/foo/bar/baz/7",
-        url_for(set, { :controller => "foo/bar", :action => "baz", :id => 7 }, current)
-  end
-
   def test_id_is_sticky_when_it_ought_to_be
+    @set = make_set false
+
     set.draw do
       get ':controller/:id/:action'
     end
 
-    url = url_for(set, { :action => "destroy" }, { :controller => "people", :action => "show", :id => "7" })
-    assert_equal "/people/7/destroy", url
+    get URI('http://test.host/people/7/show')
+
+    assert_equal "/people/7/destroy", controller.url_for(:action => 'destroy', :only_path => true)
   end
 
   def test_use_static_path_when_possible
+    @set = make_set false
+
     set.draw do
       get 'about' => "welcome#about"
       get ':controller/:action/:id'
     end
 
-    url = url_for(set, { :controller => "welcome", :action => "about" },
-      { :controller => "welcome", :action => "get", :id => "7" })
+    get URI('http://test.host/welcom/get/7')
 
-    assert_equal "/about", url
+    assert_equal "/about", controller.url_for(:controller => 'welcome',
+                                              :action => 'about',
+                                              :only_path => true)
   end
 
   def test_generate
@@ -1352,38 +1369,51 @@ class RouteSetTest < ActiveSupport::TestCase
   end
 
   def test_named_routes_are_never_relative_to_modules
+    @set = make_set false
+
     set.draw do
       get "/connection/manage(/:action)" => 'connection/manage#index'
       get "/connection/connection" => "connection/connection#index"
       get '/connection' => 'connection#index', :as => 'family_connection'
     end
 
-    url = url_for(set, { :controller => "connection" }, { :controller => 'connection/manage' })
+    assert_equal({ :controller => 'connection/manage',
+                   :action => 'index', }, request_path_params('/connection/manage'))
+
+    url = controller.url_for({ :controller => "connection", :only_path => true })
     assert_equal "/connection/connection", url
 
-    url = url_for(set, { :use_route => :family_connection, :controller => "connection" }, { :controller => 'connection/manage' })
+    url = controller.url_for({ :use_route => :family_connection,
+                               :controller => "connection", :only_path => true })
     assert_equal "/connection", url
   end
 
   def test_action_left_off_when_id_is_recalled
+    @set = make_set false
+
     set.draw do
       get ':controller(/:action(/:id))'
     end
-    assert_equal '/books', url_for(set,
-      {:controller => 'books', :action => 'index'},
-      {:controller => 'books', :action => 'show', :id => '10'}
-    )
+
+    get URI('http://test.host/books/show/10')
+
+    assert_equal '/books', controller.url_for(:controller => 'books',
+                                              :only_path => true,
+                                              :action => 'index')
   end
 
   def test_query_params_will_be_shown_when_recalled
+    @set = make_set false
+
     set.draw do
       get 'show_weblog/:parameter' => 'weblog#show'
       get ':controller(/:action(/:id))'
     end
-    assert_equal '/weblog/edit?parameter=1', url_for(set,
-      {:action => 'edit', :parameter => 1},
-      {:controller => 'weblog', :action => 'show', :parameter => 1}
-    )
+
+    get URI('http://test.host/weblog/show/1')
+
+    assert_equal '/weblog/edit?parameter=1', controller.url_for(
+      {:action => 'edit', :parameter => 1, :only_path => true})
   end
 
   def test_format_is_not_inherit
@@ -1391,22 +1421,30 @@ class RouteSetTest < ActiveSupport::TestCase
       get '/posts(.:format)' => 'posts#index'
     end
 
-    assert_equal '/posts', url_for(set,
-      {:controller => 'posts'},
-      {:controller => 'posts', :action => 'index', :format => 'xml'}
-    )
+    get URI('http://test.host/posts.xml')
+    assert_equal({:controller => 'posts', :action => 'index', :format => 'xml'},
+                 controller.request.path_parameters)
+
+    assert_equal '/posts', controller.url_for(
+      {:controller => 'posts', :only_path => true})
 
-    assert_equal '/posts.xml', url_for(set,
-      {:controller => 'posts', :format => 'xml'},
-      {:controller => 'posts', :action => 'index', :format => 'xml'}
-    )
+    assert_equal '/posts.xml', controller.url_for(
+      {:controller => 'posts', :format => 'xml', :only_path => true})
   end
 
   def test_expiry_determination_should_consider_values_with_to_param
+    @set = make_set false
+
     set.draw { get 'projects/:project_id/:controller/:action' }
-    assert_equal '/projects/1/weblog/show', url_for(set,
-      { :action => 'show', :project_id => 1 },
-      { :controller => 'weblog', :action => 'show', :project_id => '1' })
+
+    get URI('http://test.host/projects/1/weblog/show')
+
+    assert_equal(
+      { :controller => 'weblog', :action => 'show', :project_id => '1' },
+      controller.request.path_parameters)
+
+    assert_equal '/projects/1/weblog/show',
+      controller.url_for({ :action => 'show', :project_id => 1, :only_path => true })
   end
 
   def test_named_route_in_nested_resource
@@ -1852,6 +1890,9 @@ class RackMountIntegrationTests < ActiveSupport::TestCase
     root :to => "news#index"
   }
 
+  attr_reader :routes
+  attr_reader :controller
+
   def setup
     @routes = ActionDispatch::Routing::RouteSet.new
     @routes.draw(&Mapping)
author	Aaron Patterson <aaron.patterson@gmail.com>	2014-07-02 15:02:36 -0700
committer	Aaron Patterson <aaron.patterson@gmail.com>	2014-07-15 15:19:41 -0700
commit	8eb7bcb6e59e5720e3bd322511817804c8087043 (patch)
tree	ef088233869d43cbd05e8dec6603e602db8f2da9 /actionpack
parent	a3ca700c58fa59722288a12960c4a6cd1ade6aea (diff)
download	rails-8eb7bcb6e59e5720e3bd322511817804c8087043.tar.gz rails-8eb7bcb6e59e5720e3bd322511817804c8087043.tar.bz2 rails-8eb7bcb6e59e5720e3bd322511817804c8087043.zip