Merge pull request #15503 from sgrif/sg-json-hstore-storage

Bring type casting behavior of hstore/json in line with serialized

9 files changed, 29 insertions, 20 deletions

diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/oid/hstore.rb b/activerecord/lib/active_record/connection_adapters/postgresql/oid/hstore.rb
index bf680b6624..a65ca83f77 100644
--- a/activerecord/lib/active_record/connection_adapters/postgresql/oid/hstore.rb
+++ b/activerecord/lib/active_record/connection_adapters/postgresql/oid/hstore.rb
@@ -8,12 +8,6 @@ module ActiveRecord
           end
 
           def type_cast_for_write(value)
-            # roundtrip to ensure uniform uniform types
-            # TODO: This is not an efficient solution.
-            cast_value(type_cast_for_database(value))
-          end
-
-          def type_cast_for_database(value)
             ConnectionAdapters::PostgreSQLColumn.hstore_to_string(value)
           end
 
diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/oid/json.rb b/activerecord/lib/active_record/connection_adapters/postgresql/oid/json.rb
index 42a5110ffd..c87422fe32 100644
--- a/activerecord/lib/active_record/connection_adapters/postgresql/oid/json.rb
+++ b/activerecord/lib/active_record/connection_adapters/postgresql/oid/json.rb
@@ -8,12 +8,6 @@ module ActiveRecord
           end
 
           def type_cast_for_write(value)
-            # roundtrip to ensure uniform uniform types
-            # TODO: This is not an efficient solution.
-            cast_value(type_cast_for_database(value))
-          end
-
-          def type_cast_for_database(value)
             ConnectionAdapters::PostgreSQLColumn.json_to_string(value)
           end
 
diff --git a/activerecord/lib/active_record/core.rb b/activerecord/lib/active_record/core.rb
index d6849fef2e..c996e93076 100644
--- a/activerecord/lib/active_record/core.rb
+++ b/activerecord/lib/active_record/core.rb
@@ -278,12 +278,13 @@ module ActiveRecord
     #   post.init_with('attributes' => { 'title' => 'hello world' })
     #   post.title # => 'hello world'
     def init_with(coder)
-      @raw_attributes = coder['attributes']
+      @raw_attributes = coder['raw_attributes']
       @column_types_override = coder['column_types']
       @column_types = self.class.column_types
 
       init_internals
 
+      @attributes = coder['attributes'] if coder['attributes']
       @new_record = coder['new_record']
 
       self.class.define_attribute_methods
@@ -326,12 +327,13 @@ module ActiveRecord
 
       @raw_attributes = cloned_attributes
       @raw_attributes[self.class.primary_key] = nil
+      @attributes = other.clone_attributes(:read_attribute)
+      @attributes[self.class.primary_key] = nil
 
       run_callbacks(:initialize) unless _initialize_callbacks.empty?
 
       @aggregation_cache = {}
       @association_cache = {}
-      @attributes        = {}
 
       @new_record  = true
       @destroyed   = false
@@ -352,7 +354,8 @@ module ActiveRecord
     #   Post.new.encode_with(coder)
     #   coder # => {"attributes" => {"id" => nil, ... }}
     def encode_with(coder)
-      coder['attributes'] = @raw_attributes
+      coder['raw_attributes'] = @raw_attributes
+      coder['attributes'] = @attributes
       coder['new_record'] = new_record?
     end
 
diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb
index 2e3bcc0956..f1f0d3e57f 100644
--- a/activerecord/lib/active_record/persistence.rb
+++ b/activerecord/lib/active_record/persistence.rb
@@ -50,7 +50,7 @@ module ActiveRecord
         klass = discriminate_class_for_record(attributes)
         column_types = klass.decorate_columns(column_types.dup)
         klass.allocate.init_with(
-          'attributes' => attributes,
+          'raw_attributes' => attributes,
           'column_types' => column_types,
           'new_record' => false,
         )
diff --git a/activerecord/test/cases/adapters/postgresql/hstore_test.rb b/activerecord/test/cases/adapters/postgresql/hstore_test.rb
index 1fef4899be..78134ff316 100644
--- a/activerecord/test/cases/adapters/postgresql/hstore_test.rb
+++ b/activerecord/test/cases/adapters/postgresql/hstore_test.rb
@@ -152,6 +152,16 @@ class PostgresqlHstoreTest < ActiveRecord::TestCase
       assert_equal "GMT", y.timezone
     end
 
+    def test_yaml_round_trip_with_store_accessors
+      x = Hstore.new(language: "fr", timezone: "GMT")
+      assert_equal "fr", x.language
+      assert_equal "GMT", x.timezone
+
+      y = YAML.load(YAML.dump(x))
+      assert_equal "fr", y.language
+      assert_equal "GMT", y.timezone
+    end
+
     def test_gen1
       assert_equal(%q(" "=>""), @column.class.hstore_to_string({' '=>''}))
     end
diff --git a/activerecord/test/cases/adapters/postgresql/json_test.rb b/activerecord/test/cases/adapters/postgresql/json_test.rb
index 03b546119d..61d4e2b8ae 100644
--- a/activerecord/test/cases/adapters/postgresql/json_test.rb
+++ b/activerecord/test/cases/adapters/postgresql/json_test.rb
@@ -147,6 +147,14 @@ class PostgresqlJSONTest < ActiveRecord::TestCase
     assert_equal "320×480", y.resolution
   end
 
+  def test_yaml_round_trip_with_store_accessors
+    x = JsonDataType.new(resolution: "320×480")
+    assert_equal "320×480", x.resolution
+
+    y = YAML.load(YAML.dump(x))
+    assert_equal "320×480", y.resolution
+  end
+
   def test_update_all
     json = JsonDataType.create! payload: { "one" => "two" }
 
diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb
index d65c4b0638..f01901514e 100644
--- a/activerecord/test/cases/base_test.rb
+++ b/activerecord/test/cases/base_test.rb
@@ -1495,7 +1495,7 @@ class BasicsTest < ActiveRecord::TestCase
     }
 
     types = { 'author_name' => typecast.new }
-    topic = Topic.allocate.init_with 'attributes' => attrs,
+    topic = Topic.allocate.init_with 'raw_attributes' => attrs,
                                      'column_types' => types
 
     assert_equal 't.lo', topic.author_name
diff --git a/activerecord/test/cases/persistence_test.rb b/activerecord/test/cases/persistence_test.rb
index 16f481c289..7f5d0d66d3 100644
--- a/activerecord/test/cases/persistence_test.rb
+++ b/activerecord/test/cases/persistence_test.rb
@@ -251,7 +251,7 @@ class PersistenceTest < ActiveRecord::TestCase
 
   def test_create_columns_not_equal_attributes
     topic = Topic.allocate.init_with(
-      'attributes' => {
+      'raw_attributes' => {
         'title'          => 'Another New Topic',
         'does_not_exist' => 'test'
       }
@@ -302,7 +302,7 @@ class PersistenceTest < ActiveRecord::TestCase
 
     topic_reloaded = Topic.allocate
     topic_reloaded.init_with(
-      'attributes' => topic.attributes.merge('does_not_exist' => 'test')
+      'raw_attributes' => topic.attributes.merge('does_not_exist' => 'test')
     )
     topic_reloaded.title = 'A New Topic'
     assert_nothing_raised { topic_reloaded.save }
diff --git a/activerecord/test/cases/serialized_attribute_test.rb b/activerecord/test/cases/serialized_attribute_test.rb
index 5ea62c9f59..debb227303 100644
--- a/activerecord/test/cases/serialized_attribute_test.rb
+++ b/activerecord/test/cases/serialized_attribute_test.rb
@@ -31,7 +31,7 @@ class SerializedAttributeTest < ActiveRecord::TestCase
 
   def test_serialized_attribute_init_with
     topic = Topic.allocate
-    topic.init_with('attributes' => { 'content' => '--- foo' })
+    topic.init_with('raw_attributes' => { 'content' => '--- foo' })
     assert_equal 'foo', topic.content
   end