Merge pull request #13512 from gsamokovarov/hstore_arrays_fix

Hstore arrays fix (follow up for #11444)
rails · Feb 16, 2014 · 7c32db1 · 7c32db1
2 parents 3e3ed1e + da3fec2
commit 7c32db1
Show file tree

Hide file tree

Showing 4 changed files with 52 additions and 7 deletions.
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
@@ -1,3 +1,9 @@
+*   Perform necessary deeper encoding when hstore is inside an array.
+
+    Fixes #11135.
+
+    *Josh Goodall*, *Genadi Samokovarov*
+
 *   Properly detect if a connection is still active before using it
     in multi-threaded environments.
 

diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/cast.rb b/activerecord/lib/active_record/connection_adapters/postgresql/cast.rb
@@ -35,11 +35,11 @@ def string_to_bit(value)
           end
         end
 
-        def hstore_to_string(object)
+        def hstore_to_string(object, array_member = false)
           if Hash === object
-            object.map { |k,v|
-              "#{escape_hstore(k)}=>#{escape_hstore(v)}"
-            }.join ','
+            string = object.map { |k, v| "#{escape_hstore(k)}=>#{escape_hstore(v)}" }.join(',')
+            string = escape_hstore(string) if array_member
+            string
           else
             object
           end
@@ -49,10 +49,10 @@ def string_to_hstore(string)
           if string.nil?
             nil
           elsif String === string
-            Hash[string.scan(HstorePair).map { |k,v|
+            Hash[string.scan(HstorePair).map { |k, v|
               v = v.upcase == 'NULL' ? nil : v.gsub(/\A"(.*)"\Z/m,'\1').gsub(/\\(.)/, '\1')
               k = k.gsub(/\A"(.*)"\Z/m,'\1').gsub(/\\(.)/, '\1')
-              [k,v]
+              [k, v]
             }]
           else
             string

diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/quoting.rb b/activerecord/lib/active_record/connection_adapters/postgresql/quoting.rb
@@ -121,7 +121,7 @@ def type_cast(value, column, array_member = false)
             end
           when Hash
             case column.sql_type
-            when 'hstore' then PostgreSQLColumn.hstore_to_string(value)
+            when 'hstore' then PostgreSQLColumn.hstore_to_string(value, array_member)
             when 'json' then PostgreSQLColumn.json_to_string(value)
             else super(value, column)
             end

diff --git a/activerecord/test/cases/adapters/postgresql/hstore_test.rb b/activerecord/test/cases/adapters/postgresql/hstore_test.rb
@@ -24,6 +24,7 @@ def setup
     @connection.transaction do
       @connection.create_table('hstores') do |t|
         t.hstore 'tags', :default => ''
+        t.hstore 'payload', array: true
         t.hstore 'settings'
       end
     end
@@ -182,6 +183,30 @@ def test_select
       assert_equal({'1' => '2'}, x.tags)
     end
 
+    def test_array_cycle
+      assert_array_cycle([{"AA" => "BB", "CC" => "DD"}, {"AA" => nil}])
+    end
+
+    def test_array_strings_with_quotes
+      assert_array_cycle([{'this has' => 'some "s that need to be escaped"'}])
+    end
+
+    def test_array_strings_with_commas
+      assert_array_cycle([{'this,has' => 'many,values'}])
+    end
+
+    def test_array_strings_with_array_delimiters
+      assert_array_cycle(['{' => '}'])
+    end
+
+    def test_array_strings_with_null_strings
+      assert_array_cycle([{'NULL' => 'NULL'}])
+    end
+
+    def test_contains_nils
+      assert_array_cycle([{'NULL' => nil}])
+    end
+
     def test_select_multikey
       @connection.execute "insert into hstores (tags) VALUES ('1=>2,2=>3')"
       x = Hstore.first
@@ -237,6 +262,20 @@ def test_update_all
 
   private
 
+    def assert_array_cycle(array)
+      # test creation
+      x = Hstore.create!(payload: array)
+      x.reload
+      assert_equal(array, x.payload)
+
+      # test updating
+      x = Hstore.create!(payload: [])
+      x.payload = array
+      x.save!
+      x.reload
+      assert_equal(array, x.payload)
+    end
+
     def assert_cycle(hash)
       # test creation
       x = Hstore.create!(:tags => hash)