diff options
| -rw-r--r-- | actionpack/lib/action_controller/routing.rb | 10 | ||||
| -rw-r--r-- | actionpack/lib/action_controller/url_rewriter.rb | 24 | ||||
| -rw-r--r-- | actionpack/test/controller/routing_test.rb | 62 | ||||
| -rw-r--r-- | actionpack/test/controller/url_rewriter_tests.rb | 27 |
4 files changed, 70 insertions, 53 deletions
diff --git a/actionpack/lib/action_controller/routing.rb b/actionpack/lib/action_controller/routing.rb index 82b12f3a7b..8356b8ce55 100644 --- a/actionpack/lib/action_controller/routing.rb +++ b/actionpack/lib/action_controller/routing.rb @@ -397,13 +397,7 @@ module ActionController merged = recall.merge(options) expire_on = Routing.expiry_hash(options, recall) - path, keys = generate_path(merged, options, expire_on) - - # Factor out? - extras = {} - k = nil - keys.each {|k| extras[k] = options[k]} - [path, extras] + generate_path(merged, options, expire_on) end def generate_path(merged, options, expire_on) @@ -590,7 +584,7 @@ module ActionController end def extra_keys(options, recall = {}) - generate(options.dup, recall).last.keys + generate(options.dup, recall).last end end diff --git a/actionpack/lib/action_controller/url_rewriter.rb b/actionpack/lib/action_controller/url_rewriter.rb index 8ae6f3e57b..546bd13eb7 100644 --- a/actionpack/lib/action_controller/url_rewriter.rb +++ b/actionpack/lib/action_controller/url_rewriter.rb @@ -31,31 +31,27 @@ module ActionController rewritten_url end - def rewrite_path(options) - options = options.symbolize_keys - options.update((options[:params] || {}).symbolize_keys) + def rewrite_path(original_options) + options = original_options.symbolize_keys + options.update(params.symbolize_keys) if (params = options[:params]) RESERVED_OPTIONS.each {|k| options.delete k} - path, extras = Routing::Routes.generate(options, @request) + path, extra_keys = Routing::Routes.generate(options, @request) # Warning: Routes will mutate and violate the options hash - if extras[:overwrite_params] - params_copy = @request.parameters.reject { |k,v| %w(controller action).include? k } - params_copy.update extras[:overwrite_params] - extras.delete(:overwrite_params) - extras.update(params_copy) - end - - path << build_query_string(extras) unless extras.empty? + path << build_query_string(original_options.symbolize_keys, extra_keys) unless extra_keys.empty? path end # Returns a query string with escaped keys and values from the passed hash. If the passed hash contains an "id" it'll # be added as a path element instead of a regular parameter pair. - def build_query_string(hash) + def build_query_string(hash, only_keys = nil) elements = [] query_string = "" + + only_keys ||= hash.keys - hash.each do |key, value| + only_keys.each do |key| + value = hash[key] key = CGI.escape key.to_s key << '[]' if value.class == Array value = [ value ] unless value.class == Array diff --git a/actionpack/test/controller/routing_test.rb b/actionpack/test/controller/routing_test.rb index cec1b0313d..27433624e9 100644 --- a/actionpack/test/controller/routing_test.rb +++ b/actionpack/test/controller/routing_test.rb @@ -554,13 +554,13 @@ class RouteSetTests < Test::Unit::TestCase assert_equal({:controller => ::Controllers::Admin::UserController, :action => 'show', :id => '10'}.stringify_keys, rs.recognize_path(%w(admin user show 10))) - assert_equal ['/admin/user/show/10', {}], rs.generate({:controller => 'admin/user', :action => 'show', :id => 10}) + assert_equal ['/admin/user/show/10', []], rs.generate({:controller => 'admin/user', :action => 'show', :id => 10}) - assert_equal ['/admin/user/show', {}], rs.generate({:action => 'show'}, {:controller => 'admin/user', :action => 'list', :id => '10'}) - assert_equal ['/admin/user/list/10', {}], rs.generate({}, {:controller => 'admin/user', :action => 'list', :id => '10'}) + assert_equal ['/admin/user/show', []], rs.generate({:action => 'show'}, {:controller => 'admin/user', :action => 'list', :id => '10'}) + assert_equal ['/admin/user/list/10', []], rs.generate({}, {:controller => 'admin/user', :action => 'list', :id => '10'}) - assert_equal ['/admin/stuff', {}], rs.generate({:controller => 'stuff'}, {:controller => 'admin/user', :action => 'list', :id => '10'}) - assert_equal ['/stuff', {}], rs.generate({:controller => '/stuff'}, {:controller => 'admin/user', :action => 'list', :id => '10'}) + assert_equal ['/admin/stuff', []], rs.generate({:controller => 'stuff'}, {:controller => 'admin/user', :action => 'list', :id => '10'}) + assert_equal ['/stuff', []], rs.generate({:controller => '/stuff'}, {:controller => 'admin/user', :action => 'list', :id => '10'}) end def test_ignores_leading_slash @@ -689,7 +689,7 @@ class RouteSetTests < Test::Unit::TestCase end def test_changing_controller - assert_equal ['/admin/stuff/show/10', {}], rs.generate( + assert_equal ['/admin/stuff/show/10', []], rs.generate( {:controller => 'stuff', :action => 'show', :id => 10}, {:controller => 'admin/user', :action => 'index'} ) @@ -730,9 +730,9 @@ class RouteSetTests < Test::Unit::TestCase rs.connect ':controller/:action/:id' end - assert_equal ['/page/20', {}], rs.generate({:id => 20}, {:controller => 'pages'}) - assert_equal ['/page/20', {}], rs.generate(:controller => 'pages', :id => 20, :action => 'show') - assert_equal ['/pages/boo', {}], rs.generate(:controller => 'pages', :action => 'boo') + assert_equal ['/page/20', []], rs.generate({:id => 20}, {:controller => 'pages'}) + assert_equal ['/page/20', []], rs.generate(:controller => 'pages', :id => 20, :action => 'show') + assert_equal ['/pages/boo', []], rs.generate(:controller => 'pages', :action => 'boo') end def test_route_with_fixnum_default @@ -741,10 +741,10 @@ class RouteSetTests < Test::Unit::TestCase rs.connect ':controller/:action/:id' end - assert_equal ['/page', {}], rs.generate(:controller => 'content', :action => 'show_page') - assert_equal ['/page', {}], rs.generate(:controller => 'content', :action => 'show_page', :id => 1) - assert_equal ['/page', {}], rs.generate(:controller => 'content', :action => 'show_page', :id => '1') - assert_equal ['/page/10', {}], rs.generate(:controller => 'content', :action => 'show_page', :id => 10) + assert_equal ['/page', []], rs.generate(:controller => 'content', :action => 'show_page') + assert_equal ['/page', []], rs.generate(:controller => 'content', :action => 'show_page', :id => 1) + assert_equal ['/page', []], rs.generate(:controller => 'content', :action => 'show_page', :id => '1') + assert_equal ['/page/10', []], rs.generate(:controller => 'content', :action => 'show_page', :id => 10) ctrl = ::Controllers::ContentController @@ -754,7 +754,7 @@ class RouteSetTests < Test::Unit::TestCase end def test_action_expiry - assert_equal ['/content', {}], rs.generate({:controller => 'content'}, {:controller => 'content', :action => 'show'}) + assert_equal ['/content', []], rs.generate({:controller => 'content'}, {:controller => 'content', :action => 'show'}) end def test_recognition_with_uppercase_controller_name @@ -775,8 +775,8 @@ class RouteSetTests < Test::Unit::TestCase rs.connect ':controller/:action/:id' end - assert_equal ['/test', {}], rs.generate(:controller => 'post', :action => 'show') - assert_equal ['/test', {}], rs.generate(:controller => 'post', :action => 'show', :year => nil) + assert_equal ['/test', []], rs.generate(:controller => 'post', :action => 'show') + assert_equal ['/test', []], rs.generate(:controller => 'post', :action => 'show', :year => nil) x = setup_for_named_route assert_equal({:controller => '/post', :action => 'show'}, @@ -789,20 +789,20 @@ class RouteSetTests < Test::Unit::TestCase rs.connect ':controller/:action/:id' end - assert_equal ['/pages/2005', {}], + assert_equal ['/pages/2005', []], rs.generate(:controller => 'content', :action => 'list_pages', :year => 2005) - assert_equal ['/pages/2005/6', {}], + assert_equal ['/pages/2005/6', []], rs.generate(:controller => 'content', :action => 'list_pages', :year => 2005, :month => 6) - assert_equal ['/pages/2005/6/12', {}], + assert_equal ['/pages/2005/6/12', []], rs.generate(:controller => 'content', :action => 'list_pages', :year => 2005, :month => 6, :day => 12) - assert_equal ['/pages/2005/6/4', {}], + assert_equal ['/pages/2005/6/4', []], rs.generate({:day => 4}, {:controller => 'content', :action => 'list_pages', :year => '2005', :month => '6', :day => '12'}) - assert_equal ['/pages/2005/6', {}], + assert_equal ['/pages/2005/6', []], rs.generate({:day => nil}, {:controller => 'content', :action => 'list_pages', :year => '2005', :month => '6', :day => '12'}) - assert_equal ['/pages/2005', {}], + assert_equal ['/pages/2005', []], rs.generate({:day => nil, :month => nil}, {:controller => 'content', :action => 'list_pages', :year => '2005', :month => '6', :day => '12'}) end @@ -812,8 +812,8 @@ class RouteSetTests < Test::Unit::TestCase rs.connect ':controller/:action/:id' end - assert_equal ['/', {}], rs.generate(:controller => 'content', :action => 'index') - assert_equal ['/', {}], rs.generate(:controller => 'content') + assert_equal ['/', []], rs.generate(:controller => 'content', :action => 'index') + assert_equal ['/', []], rs.generate(:controller => 'content') end def test_named_url_with_no_action_specified @@ -822,8 +822,8 @@ class RouteSetTests < Test::Unit::TestCase rs.connect ':controller/:action/:id' end - assert_equal ['/', {}], rs.generate(:controller => 'content', :action => 'index') - assert_equal ['/', {}], rs.generate(:controller => 'content') + assert_equal ['/', []], rs.generate(:controller => 'content', :action => 'index') + assert_equal ['/', []], rs.generate(:controller => 'content') x = setup_for_named_route assert_equal({:controller => '/content', :action => 'index'}, @@ -836,9 +836,9 @@ class RouteSetTests < Test::Unit::TestCase rs.root '', hash rs.connect ':controller/:action/:id' end - assert_equal ['/', {}], rs.generate({:action => nil}, {:controller => 'content', :action => 'hello'}) - assert_equal ['/', {}], rs.generate({:controller => 'content'}) - assert_equal ['/content/hi', {}], rs.generate({:controller => 'content', :action => 'hi'}) + assert_equal ['/', []], rs.generate({:action => nil}, {:controller => 'content', :action => 'hello'}) + assert_equal ['/', []], rs.generate({:controller => 'content'}) + assert_equal ['/content/hi', []], rs.generate({:controller => 'content', :action => 'hi'}) end end @@ -850,8 +850,8 @@ class RouteSetTests < Test::Unit::TestCase rs.connect ':controller/:action/:id' end - assert_equal ['/categories', {}], rs.generate(:controller => 'content', :action => 'categories') - assert_equal ['/content/hi', {}], rs.generate({:controller => 'content', :action => 'hi'}) + assert_equal ['/categories', []], rs.generate(:controller => 'content', :action => 'categories') + assert_equal ['/content/hi', []], rs.generate({:controller => 'content', :action => 'hi'}) end end diff --git a/actionpack/test/controller/url_rewriter_tests.rb b/actionpack/test/controller/url_rewriter_tests.rb new file mode 100644 index 0000000000..392024f03d --- /dev/null +++ b/actionpack/test/controller/url_rewriter_tests.rb @@ -0,0 +1,27 @@ +require File.dirname(__FILE__) + '/../abstract_unit' + +class UrlRewriterTests < Test::Unit::TestCase + def setup + @request = ActionController::TestRequest.new + @params = {} + @rewriter = ActionController::UrlRewriter.new(@request, @params) + end + + def test_simple_build_query_string + assert_equal '?x=1&y=2', @rewriter.send(:build_query_string, :x => '1', :y => '2') + end + def test_convert_ints_build_query_string + assert_equal '?x=1&y=2', @rewriter.send(:build_query_string, :x => 1, :y => 2) + end + def test_escape_spaces_build_query_string + assert_equal '?x=hello+world&y=goodbye+world', @rewriter.send(:build_query_string, :x => 'hello world', :y => 'goodbye world') + end + def test_expand_array_build_query_string + assert_equal '?x[]=1&x[]=2', @rewriter.send(:build_query_string, :x => [1, 2]) + end + + def test_escape_spaces_build_query_string_selected_keys + assert_equal '?x=hello+world', @rewriter.send(:build_query_string, {:x => 'hello world', :y => 'goodbye world'}, [:x]) + end + +end |