Skip to content

Commit

Permalink
update_column take ruby-land input, not database-land input
Browse files Browse the repository at this point in the history
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
  • Loading branch information
sgrif committed Dec 16, 2014
1 parent e4c9bd9 commit dd8b5fb
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 3 deletions.
18 changes: 18 additions & 0 deletions activerecord/lib/active_record/attribute.rb
Expand Up @@ -9,6 +9,10 @@ def from_user(name, value, type)
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
Expand Down Expand Up @@ -58,6 +62,10 @@ def with_value_from_database(value)
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
Expand Down Expand Up @@ -93,6 +101,16 @@ def type_cast(value)
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)
Expand Down
2 changes: 1 addition & 1 deletion activerecord/lib/active_record/attribute_methods/write.rb
Expand Up @@ -73,7 +73,7 @@ def write_attribute_with_type_cast(attr_name, value, should_type_cast)
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
Expand Down
4 changes: 4 additions & 0 deletions activerecord/lib/active_record/attribute_set.rb
Expand Up @@ -39,6 +39,10 @@ def write_from_user(name, value)
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
Expand Down
6 changes: 4 additions & 2 deletions activerecord/test/cases/serialized_attribute_test.rb
Expand Up @@ -243,8 +243,9 @@ def test_serialized_column_should_unserialize_after_update_column
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
Expand All @@ -253,5 +254,6 @@ def test_serialized_column_should_unserialize_after_update_attribute

t.update_attribute(:content, "second")
assert_equal("second", t.content)
assert_equal("second", t.reload.content)
end
end

1 comment on commit dd8b5fb

@jonatack
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

Please sign in to comment.