aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--actionpack/CHANGELOG2
-rw-r--r--actionpack/lib/action_controller/routing.rb37
-rw-r--r--actionpack/test/controller/routing_test.rb2
-rw-r--r--actionpack/test/controller/url_rewriter_test.rb61
-rw-r--r--activesupport/lib/active_support/core_ext/hash/conversions.rb6
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 = {})