Skip to content
Browse files

Fixed construction of get parameters for arrays

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@1857 5ecf4fe2-1ee6-0310-87b1-e25e094e27de
  • Loading branch information...
1 parent 2cdecff commit 741316dc7123e8cdd750642dff6f39257d0ddbec @seckar seckar committed Jul 18, 2005
View
10 actionpack/lib/action_controller/routing.rb
@@ -397,13 +397,7 @@ def generate(options, request_or_recall_hash = {})
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 @@ def method_missing(name, *args)
end
def extra_keys(options, recall = {})
- generate(options.dup, recall).last.keys
+ generate(options.dup, recall).last
end
end
View
24 actionpack/lib/action_controller/url_rewriter.rb
@@ -31,31 +31,27 @@ def rewrite_url(path, options)
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
View
62 actionpack/test/controller/routing_test.rb
@@ -554,13 +554,13 @@ def test_default_setup
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 @@ def test_named_route_with_regexps
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 @@ def test_backwards
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 @@ def test_route_with_fixnum_default
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 @@ def test_route_with_fixnum_default
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 @@ def test_both_requirement_and_optional
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 @@ def test_set_to_nil_forgets
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 @@ def test_url_with_no_action_specified
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 @@ def test_named_url_with_no_action_specified
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 @@ def test_url_generated_when_forgetting_action
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 @@ def test_named_route_method
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
View
27 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

0 comments on commit 741316d

Please sign in to comment.
Something went wrong with that request. Please try again.