Merge pull request #15629 from akshay-vishnoi/test-to_param

Define Hash#to_query and set Hash#to_param as alias to it; with test cases

4 files changed, 81 insertions, 66 deletions

diff --git a/activesupport/lib/active_support/core_ext/object/to_param.rb b/activesupport/lib/active_support/core_ext/object/to_param.rb
index e65fc5bac1..684d4ef57e 100644
--- a/activesupport/lib/active_support/core_ext/object/to_param.rb
+++ b/activesupport/lib/active_support/core_ext/object/to_param.rb
@@ -1,60 +1 @@
-class Object
-  # Alias of <tt>to_s</tt>.
-  def to_param
-    to_s
-  end
-end
-
-class NilClass
-  # Returns +self+.
-  def to_param
-    self
-  end
-end
-
-class TrueClass
-  # Returns +self+.
-  def to_param
-    self
-  end
-end
-
-class FalseClass
-  # Returns +self+.
-  def to_param
-    self
-  end
-end
-
-class Array
-  # Calls <tt>to_param</tt> on all its elements and joins the result with
-  # slashes. This is used by <tt>url_for</tt> in Action Pack.
-  def to_param
-    collect { |e| e.to_param }.join '/'
-  end
-end
-
-class Hash
-  # Returns a string representation of the receiver suitable for use as a URL
-  # query string:
-  #
-  #   {name: 'David', nationality: 'Danish'}.to_param
-  #   # => "name=David&nationality=Danish"
-  #
-  # An optional namespace can be passed to enclose the param names:
-  #
-  #   {name: 'David', nationality: 'Danish'}.to_param('user')
-  #   # => "user[name]=David&user[nationality]=Danish"
-  #
-  # The string pairs "key=value" that conform the query string
-  # are sorted lexicographically in ascending order.
-  #
-  # This method is also aliased as +to_query+.
-  def to_param(namespace = nil)
-    collect do |key, value|
-      unless (value.is_a?(Hash) || value.is_a?(Array)) && value.empty?
-        value.to_query(namespace ? "#{namespace}[#{key}]" : key)
-      end
-    end.compact.sort! * '&'
-  end
-end
+require 'active_support/core_ext/object/to_query'
diff --git a/activesupport/lib/active_support/core_ext/object/to_query.rb b/activesupport/lib/active_support/core_ext/object/to_query.rb
index 172f06ed64..ccd568bbf5 100644
--- a/activesupport/lib/active_support/core_ext/object/to_query.rb
+++ b/activesupport/lib/active_support/core_ext/object/to_query.rb
@@ -1,17 +1,46 @@
-require 'active_support/core_ext/object/to_param'
 require 'cgi'
 
 class Object
-  # Converts an object into a string suitable for use as a URL query string, using the given <tt>key</tt> as the
-  # param name.
-  #
-  # Note: This method is defined as a default implementation for all Objects for Hash#to_query to work.
+  # Alias of <tt>to_s</tt>.
+  def to_param
+    to_s
+  end
+
+  # Converts an object into a string suitable for use as a URL query string,
+  # using the given <tt>key</tt> as the param name.
   def to_query(key)
     "#{CGI.escape(key.to_param)}=#{CGI.escape(to_param.to_s)}"
   end
 end
 
+class NilClass
+  # Returns +self+.
+  def to_param
+    self
+  end
+end
+
+class TrueClass
+  # Returns +self+.
+  def to_param
+    self
+  end
+end
+
+class FalseClass
+  # Returns +self+.
+  def to_param
+    self
+  end
+end
+
 class Array
+  # Calls <tt>to_param</tt> on all its elements and joins the result with
+  # slashes. This is used by <tt>url_for</tt> in Action Pack.
+  def to_param
+    collect { |e| e.to_param }.join '/'
+  end
+
   # Converts an array into a string suitable for use as a URL query string,
   # using the given +key+ as the param name.
   #
@@ -28,5 +57,28 @@ class Array
 end
 
 class Hash
-  alias_method :to_query, :to_param
+  # Returns a string representation of the receiver suitable for use as a URL
+  # query string:
+  #
+  #   {name: 'David', nationality: 'Danish'}.to_query
+  #   # => "name=David&nationality=Danish"
+  #
+  # An optional namespace can be passed to enclose key names:
+  #
+  #   {name: 'David', nationality: 'Danish'}.to_query('user')
+  #   # => "user%5Bname%5D=David&user%5Bnationality%5D=Danish"
+  #
+  # The string pairs "key=value" that conform the query string
+  # are sorted lexicographically in ascending order.
+  #
+  # This method is also aliased as +to_param+.
+  def to_query(namespace = nil)
+    collect do |key, value|
+      unless (value.is_a?(Hash) || value.is_a?(Array)) && value.empty?
+        value.to_query(namespace ? "#{namespace}[#{key}]" : key)
+      end
+    end.compact.sort! * '&'
+  end
+
+  alias_method :to_param, :to_query
 end
diff --git a/activesupport/test/core_ext/object/to_param_test.rb b/activesupport/test/core_ext/object/to_param_test.rb
index bd7c6c422a..25c3bf22c3 100644
--- a/activesupport/test/core_ext/object/to_param_test.rb
+++ b/activesupport/test/core_ext/object/to_param_test.rb
@@ -16,4 +16,16 @@ class ToParamTest < ActiveSupport::TestCase
     assert_equal true, true.to_param
     assert_equal false, false.to_param
   end
+
+  def test_array
+    # Empty Array
+    assert_equal '', [].to_param
+
+    array = [1, 2, 3, 4]
+    assert_equal "1/2/3/4", array.to_param
+
+    # Array of different objects
+    array = [1, '3', { a: 1, b: 2 }, nil, true, false]
+    assert_equal "1/3/a=1&b=2//true/false", array.to_param
+  end
 end
diff --git a/activesupport/test/core_ext/object/to_query_test.rb b/activesupport/test/core_ext/object/to_query_test.rb
index 47220e9509..09cab3ed35 100644
--- a/activesupport/test/core_ext/object/to_query_test.rb
+++ b/activesupport/test/core_ext/object/to_query_test.rb
@@ -65,6 +65,16 @@ class ToQueryTest < ActiveSupport::TestCase
       {a: [], b: 3}
   end
 
+  def test_hash_with_namespace
+    hash = { name: 'Nakshay', nationality: 'Indian' }
+    assert_equal "user%5Bname%5D=Nakshay&user%5Bnationality%5D=Indian", hash.to_query('user')
+  end
+
+  def test_hash_sorted_lexicographically
+    hash = { type: 'human', name: 'Nakshay' }
+    assert_equal "name=Nakshay&type=human", hash.to_query
+  end
+
   private
     def assert_query_equal(expected, actual)
       assert_equal expected.split('&'), actual.to_query.split('&')