Allow array and hash query parameters. Array route parameters are converted/to/a/path as before. References #6765, #7462. Closes #7047.

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@6343 5ecf4fe2-1ee6-0310-87b1-e25e094e27de

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]
 

actionpack/lib/action_controller/routing.rb

@@ -452,24 +452,16 @@ 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 ] 
+        (only_keys || hash.keys).each do |key|
+          if value = hash[key]
+            elements << value.to_query(key)
           end
-          value.each { |val| elements << "#{key}=#{CGI.escape(val.to_param.to_s)}" }
         end
 
-        query_string = "?#{elements.join("&")}" unless elements.empty?
-        query_string || ""
+        elements.empty? ? '' : "?#{elements.sort * '&'}"
       end
 
       # Write the real recognition implementation and then resend the message.
@@ -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
   

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

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
@@ -161,4 +152,54 @@ class UrlWriterTests < Test::Unit::TestCase
     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

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 = {})