Skip to content

Commit

Permalink
Properly cast input in update_all
Browse files Browse the repository at this point in the history
The documentation claims that given values go through "normal AR type
casting and serialization", which to me implies
`serialize(cast(value))`, not just serialization. The docs were changed
to use this wording in #22492. The tests I cited in that PR (which is
the same test modified in this commit), is worded in a way that implies
it should be using `cast` as well.

It's possible that I originally meant "normal type casting" to imply
just the call to `serialize`, but given that `update_all(archived:
params['archived'])` seems to be pretty common, I'm inclined to make
this change as long as no tests are broken from it.
  • Loading branch information
sgrif committed Nov 13, 2017
1 parent f49d594 commit 68fe6b0
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 2 deletions.
6 changes: 6 additions & 0 deletions activerecord/CHANGELOG.md
@@ -1,3 +1,9 @@
* `update_all` will now pass its values to `Type#cast` before passing them to
`Type#serialize`. This means that `update_all(foo: 'true')` will properly
persist a boolean.

*Sean Griffin*

* Add new error class `StatementTimeout` which will be raised
when statement timeout exceeded.

Expand Down
3 changes: 2 additions & 1 deletion activerecord/lib/active_record/sanitization.rb
Expand Up @@ -110,7 +110,8 @@ def expand_hash_conditions_for_aggregates(attrs) # :doc:
def sanitize_sql_hash_for_assignment(attrs, table) # :doc:
c = connection
attrs.map do |attr, value|
value = type_for_attribute(attr.to_s).serialize(value)
type = type_for_attribute(attr.to_s)
value = type.serialize(type.cast(value))
"#{c.quote_table_name_for_assignment(table, attr)} = #{c.quote(value)}"
end.join(", ")
end
Expand Down
7 changes: 6 additions & 1 deletion activerecord/test/cases/relation_test.rb
Expand Up @@ -288,13 +288,18 @@ def type
:string
end

def cast(value)
raise value unless value == "value from user"
"cast value"
end

def deserialize(value)
raise value unless value == "type cast for database"
"type cast from database"
end

def serialize(value)
raise value unless value == "value from user"
raise value unless value == "cast value"
"type cast for database"
end
end
Expand Down

0 comments on commit 68fe6b0

Please sign in to comment.