diff options
author | Sean Griffin <sean@thoughtbot.com> | 2014-12-16 15:19:47 -0700 |
---|---|---|
committer | Sean Griffin <sean@thoughtbot.com> | 2014-12-16 15:23:05 -0700 |
commit | dd8b5fb9d30f355d4eab6376b8d9e025e90b14d3 (patch) | |
tree | 887b7851abeb0432bc013f362e396e2b2f0a0723 | |
parent | e4c9bd9828e107214d03a66679a89f669e3722fa (diff) | |
download | rails-dd8b5fb9d30f355d4eab6376b8d9e025e90b14d3.tar.gz rails-dd8b5fb9d30f355d4eab6376b8d9e025e90b14d3.tar.bz2 rails-dd8b5fb9d30f355d4eab6376b8d9e025e90b14d3.zip |
`update_column` take ruby-land input, not database-land input
In the case of serialized columns, we would expect the unserialized
value as input, not the serialized value. The original issue which made
this distinction, #14163, introduced a bug. If you passed serialized
input to the method, it would double serialize when it was sent to the
database. You would see the wrong input upon reloading, or get an error
if you had a specific type on the serialized column.
To put it another way, `update_column` is a special case of
`update_all`, which would take `['a']` and not `['a'].to_yaml`, but you
would not pass data from `params` to it.
Fixes #18037
4 files changed, 27 insertions, 3 deletions
diff --git a/activerecord/lib/active_record/attribute.rb b/activerecord/lib/active_record/attribute.rb index 8cc1904575..88536eaac0 100644 --- a/activerecord/lib/active_record/attribute.rb +++ b/activerecord/lib/active_record/attribute.rb @@ -9,6 +9,10 @@ module ActiveRecord FromUser.new(name, value, type) end + def with_cast_value(name, value, type) + WithCastValue.new(name, value, type) + end + def null(name) Null.new(name) end @@ -58,6 +62,10 @@ module ActiveRecord self.class.from_database(name, value, type) end + def with_cast_value(value) + self.class.with_cast_value(name, value, type) + end + def type_cast(*) raise NotImplementedError end @@ -93,6 +101,16 @@ module ActiveRecord end end + class WithCastValue < Attribute # :nodoc: + def type_cast(value) + value + end + + def changed_in_place_from?(old_value) + false + end + end + class Null < Attribute # :nodoc: def initialize(name) super(name, nil, Type::Value.new) diff --git a/activerecord/lib/active_record/attribute_methods/write.rb b/activerecord/lib/active_record/attribute_methods/write.rb index b3c8209a74..16804f86bf 100644 --- a/activerecord/lib/active_record/attribute_methods/write.rb +++ b/activerecord/lib/active_record/attribute_methods/write.rb @@ -73,7 +73,7 @@ module ActiveRecord if should_type_cast @attributes.write_from_user(attr_name, value) else - @attributes.write_from_database(attr_name, value) + @attributes.write_cast_value(attr_name, value) end value diff --git a/activerecord/lib/active_record/attribute_set.rb b/activerecord/lib/active_record/attribute_set.rb index 6b1d7ea79e..66fcaf6945 100644 --- a/activerecord/lib/active_record/attribute_set.rb +++ b/activerecord/lib/active_record/attribute_set.rb @@ -39,6 +39,10 @@ module ActiveRecord attributes[name] = self[name].with_value_from_user(value) end + def write_cast_value(name, value) + attributes[name] = self[name].with_cast_value(value) + end + def freeze @attributes.freeze super diff --git a/activerecord/test/cases/serialized_attribute_test.rb b/activerecord/test/cases/serialized_attribute_test.rb index 66f345d805..56a0e92e1d 100644 --- a/activerecord/test/cases/serialized_attribute_test.rb +++ b/activerecord/test/cases/serialized_attribute_test.rb @@ -243,8 +243,9 @@ class SerializedAttributeTest < ActiveRecord::TestCase t = Topic.create(content: "first") assert_equal("first", t.content) - t.update_column(:content, Topic.type_for_attribute('content').type_cast_for_database("second")) - assert_equal("second", t.content) + t.update_column(:content, ["second"]) + assert_equal(["second"], t.content) + assert_equal(["second"], t.reload.content) end def test_serialized_column_should_unserialize_after_update_attribute @@ -253,5 +254,6 @@ class SerializedAttributeTest < ActiveRecord::TestCase t.update_attribute(:content, "second") assert_equal("second", t.content) + assert_equal("second", t.reload.content) end end |