diff options
| -rw-r--r-- | actionpack/CHANGELOG | 2 | ||||
| -rw-r--r-- | actionpack/lib/action_controller/routing.rb | 37 | ||||
| -rw-r--r-- | actionpack/test/controller/routing_test.rb | 2 | ||||
| -rw-r--r-- | actionpack/test/controller/url_rewriter_test.rb | 61 | ||||
| -rw-r--r-- | activesupport/lib/active_support/core_ext/hash/conversions.rb | 6 |
5 files changed, 71 insertions, 37 deletions
diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG index ce27a28188..4b5657a579 100644 --- a/actionpack/CHANGELOG +++ b/actionpack/CHANGELOG @@ -1,5 +1,7 @@ *SVN* +* Allow array and hash query parameters. Array route parameters are converted/to/a/path as before. #6765, #7047, #7462 [bgipsy, Jeremy McAnally, Dan Kubb, brendan] + # Add a #dbman attr_reader for CGI::Session and make CGI::Session::CookieStore#generate_digest public so it's easy to generate digests using the cookie store's secret. [Rick] diff --git a/actionpack/lib/action_controller/routing.rb b/actionpack/lib/action_controller/routing.rb index 723aaada83..b44dc0f901 100644 --- a/actionpack/lib/action_controller/routing.rb +++ b/actionpack/lib/action_controller/routing.rb @@ -452,26 +452,18 @@ module ActionController # is given (as an array), only the keys indicated will be used to build # the query string. The query string will correctly build array parameter # values. - def build_query_string(hash, only_keys=nil) + def build_query_string(hash, only_keys = nil) elements = [] - only_keys ||= hash.keys - - only_keys.each do |key| - value = hash[key] or next - key = CGI.escape key.to_s - if value.class == Array - key << '[]' - else - value = [ value ] - end - value.each { |val| elements << "#{key}=#{CGI.escape(val.to_param.to_s)}" } - end - - query_string = "?#{elements.join("&")}" unless elements.empty? - query_string || "" + (only_keys || hash.keys).each do |key| + if value = hash[key] + elements << value.to_query(key) + end + end + + elements.empty? ? '' : "?#{elements.sort * '&'}" end - + # Write the real recognition implementation and then resend the message. def recognize(path, environment={}) write_recognition @@ -545,7 +537,7 @@ module ActionController end nil end - + end class Segment #:nodoc: @@ -669,7 +661,7 @@ module ActionController end def extract_value - "#{local_name} = hash[:#{key}] #{"|| #{default.inspect}" if default}" + "#{local_name} = hash[:#{key}] && hash[:#{key}].to_param #{"|| #{default.inspect}" if default}" end def value_check if default # Then we know it won't be nil @@ -1195,10 +1187,9 @@ module ActionController # # great fun, eh? - options_as_params = options[:controller] ? { :action => "index" } : {} - options.each do |k, value| - options_as_params[k] = value.to_param - end + options_as_params = options.clone + options_as_params[:action] ||= 'index' if options[:controller] + options_as_params[:action] = options_as_params[:action].to_s if options_as_params[:action] options_as_params end diff --git a/actionpack/test/controller/routing_test.rb b/actionpack/test/controller/routing_test.rb index f9e4de7dfb..defc64bba2 100644 --- a/actionpack/test/controller/routing_test.rb +++ b/actionpack/test/controller/routing_test.rb @@ -1008,7 +1008,7 @@ class RouteTest < Test::Unit::TestCase end def test_expand_array_build_query_string - assert_equal '?x[]=1&x[]=2', order_query_string(@route.build_query_string(:x => [1, 2])) + assert_equal '?x%5B%5D=1&x%5B%5D=2', order_query_string(@route.build_query_string(:x => [1, 2])) end def test_escape_spaces_build_query_string_selected_keys diff --git a/actionpack/test/controller/url_rewriter_test.rb b/actionpack/test/controller/url_rewriter_test.rb index fb3e318ffd..1e27c53512 100644 --- a/actionpack/test/controller/url_rewriter_test.rb +++ b/actionpack/test/controller/url_rewriter_test.rb @@ -56,15 +56,6 @@ class UrlRewriterTests < Test::Unit::TestCase u = @rewriter.rewrite(:only_path => false, :overwrite_params => {:list_page => 2}) assert_equal 'http://test.host/search/list?list_page=2', u end - - private - def split_query_string(str) - [str[0].chr] + str[1..-1].split(/&/).sort - end - - def assert_query_equal(q1, q2) - assert_equal(split_query_string(q1), split_query_string(q2)) - end end class UrlWriterTests < Test::Unit::TestCase @@ -160,5 +151,55 @@ class UrlWriterTests < Test::Unit::TestCase ensure ActionController::Routing::Routes.load! end - + + def test_one_parameter + assert_equal('/c/a?param=val', + W.new.url_for(:only_path => true, :controller => 'c', :action => 'a', :param => 'val') + ) + end + + def test_two_parameters + url = W.new.url_for(:only_path => true, :controller => 'c', :action => 'a', :p1 => 'X1', :p2 => 'Y2') + params = extract_params(url) + assert_equal params[0], { :p1 => 'X1' }.to_query + assert_equal params[1], { :p2 => 'Y2' }.to_query + end + + def test_hash_parameter + url = W.new.url_for(:only_path => true, :controller => 'c', :action => 'a', :query => {:name => 'Bob', :category => 'prof'}) + params = extract_params(url) + assert_equal params[0], { 'query[category]' => 'prof' }.to_query + assert_equal params[1], { 'query[name]' => 'Bob' }.to_query + end + + def test_array_parameter + url = W.new.url_for(:only_path => true, :controller => 'c', :action => 'a', :query => ['Bob', 'prof']) + params = extract_params(url) + assert_equal params[0], { 'query[]' => 'Bob' }.to_query + assert_equal params[1], { 'query[]' => 'prof' }.to_query + end + + def test_hash_recursive_parameters + url = W.new.url_for(:only_path => true, :controller => 'c', :action => 'a', :query => {:person => {:name => 'Bob', :position => 'prof'}, :hobby => 'piercing'}) + params = extract_params(url) + assert_equal params[0], { 'query[hobby]' => 'piercing' }.to_query + assert_equal params[1], { 'query[person][name]' => 'Bob' }.to_query + assert_equal params[2], { 'query[person][position]' => 'prof' }.to_query + end + + def test_hash_recursive_and_array_parameters + url = W.new.url_for(:only_path => true, :controller => 'c', :action => 'a', :id => 101, :query => {:person => {:name => 'Bob', :position => ['prof', 'art director']}, :hobby => 'piercing'}) + assert_match %r(^/c/a/101), url + params = extract_params(url) + assert_equal params[0], { 'query[hobby]' => 'piercing' }.to_query + assert_equal params[1], { 'query[person][name]' => 'Bob' }.to_query + assert_equal params[2], { 'query[person][position][]' => 'art director' }.to_query + assert_equal params[3], { 'query[person][position][]' => 'prof' }.to_query + end + + private + def extract_params(url) + url.split('?', 2).last.split('&') + end + end diff --git a/activesupport/lib/active_support/core_ext/hash/conversions.rb b/activesupport/lib/active_support/core_ext/hash/conversions.rb index d3ab3b3590..1cdf7d5c54 100644 --- a/activesupport/lib/active_support/core_ext/hash/conversions.rb +++ b/activesupport/lib/active_support/core_ext/hash/conversions.rb @@ -9,13 +9,13 @@ class Object end def to_query(key) #:nodoc: - "#{CGI.escape(key.to_s)}=#{CGI.escape(to_param || "")}" + "#{CGI.escape(key.to_s)}=#{CGI.escape(to_param.to_s)}" end end class Array def to_query(key) #:nodoc: - collect { |value| value.to_query("#{key}[]") } * '&' + collect { |value| value.to_query("#{key}[]") }.sort * '&' end end @@ -48,7 +48,7 @@ module ActiveSupport #:nodoc: def to_query(namespace = nil) collect do |key, value| value.to_query(namespace ? "#{namespace}[#{key}]" : key) - end * '&' + end.sort * '&' end def to_xml(options = {}) |