mirror of
https://github.com/rails/rails.git
synced 2022-11-09 12:12:34 -05:00
Move integer range validation to never raise on assignment
Given that this was originally added to normalize an error that would
have otherwise come from the database (inconsistently), it's more
natural for us to raise in `type_cast_for_database`, rather than
`type_cast_from_user`. This way, things like numericality validators can
handle it instead if the user chooses to do so. It also fixes an issue
where assigning an out of range value would make it impossible to assign
a new value later.
This fixes several vague issues, none of which were ever directly
reported, so I have no issue number to give. Places it was mentioned
which I can remember:
- 9ba21381d7/lib/shoulda/matchers/active_model/allow_value_matcher.rb (L261-L263)
- https://github.com/rails/rails/issues/18653#issuecomment-71197026
This commit is contained in:
parent
96e504ec8a
commit
7c6f3938de
4 changed files with 45 additions and 20 deletions
|
@ -1,3 +1,11 @@
|
|||
* Integer types will no longer raise a `RangeError` when assigning an
|
||||
attribute, but will instead raise when going to the database.
|
||||
|
||||
Fixes several vague issues which were never reported directly. See the
|
||||
commit message from the commit which added this line for some examples.
|
||||
|
||||
*Sean Griffin*
|
||||
|
||||
* Values which would error while being sent to the database (such as an
|
||||
ASCII-8BIT string with invalid UTF-8 bytes on Sqlite3), no longer error on
|
||||
assignment. They will still error when sent to the database, but you are
|
||||
|
|
|
@ -16,13 +16,19 @@ module ActiveRecord
|
|||
:integer
|
||||
end
|
||||
|
||||
alias type_cast_for_database type_cast
|
||||
|
||||
def type_cast_from_database(value)
|
||||
return if value.nil?
|
||||
value.to_i
|
||||
end
|
||||
|
||||
def type_cast_for_database(value)
|
||||
result = type_cast(value)
|
||||
if result
|
||||
ensure_in_range(result)
|
||||
end
|
||||
result
|
||||
end
|
||||
|
||||
protected
|
||||
|
||||
attr_reader :range
|
||||
|
@ -34,9 +40,7 @@ module ActiveRecord
|
|||
when true then 1
|
||||
when false then 0
|
||||
else
|
||||
result = value.to_i rescue nil
|
||||
ensure_in_range(result) if result
|
||||
result
|
||||
value.to_i rescue nil
|
||||
end
|
||||
end
|
||||
|
||||
|
|
|
@ -60,55 +60,68 @@ module ActiveRecord
|
|||
|
||||
test "values below int min value are out of range" do
|
||||
assert_raises(::RangeError) do
|
||||
Integer.new.type_cast_from_user("-2147483649")
|
||||
Integer.new.type_cast_for_database(-2147483649)
|
||||
end
|
||||
end
|
||||
|
||||
test "values above int max value are out of range" do
|
||||
assert_raises(::RangeError) do
|
||||
Integer.new.type_cast_from_user("2147483648")
|
||||
Integer.new.type_cast_for_database(2147483648)
|
||||
end
|
||||
end
|
||||
|
||||
test "very small numbers are out of range" do
|
||||
assert_raises(::RangeError) do
|
||||
Integer.new.type_cast_from_user("-9999999999999999999999999999999")
|
||||
Integer.new.type_cast_for_database(-9999999999999999999999999999999)
|
||||
end
|
||||
end
|
||||
|
||||
test "very large numbers are out of range" do
|
||||
assert_raises(::RangeError) do
|
||||
Integer.new.type_cast_from_user("9999999999999999999999999999999")
|
||||
Integer.new.type_cast_for_database(9999999999999999999999999999999)
|
||||
end
|
||||
end
|
||||
|
||||
test "normal numbers are in range" do
|
||||
type = Integer.new
|
||||
assert_equal(0, type.type_cast_from_user("0"))
|
||||
assert_equal(-1, type.type_cast_from_user("-1"))
|
||||
assert_equal(1, type.type_cast_from_user("1"))
|
||||
assert_equal(0, type.type_cast_for_database(0))
|
||||
assert_equal(-1, type.type_cast_for_database(-1))
|
||||
assert_equal(1, type.type_cast_for_database(1))
|
||||
end
|
||||
|
||||
test "int max value is in range" do
|
||||
assert_equal(2147483647, Integer.new.type_cast_from_user("2147483647"))
|
||||
assert_equal(2147483647, Integer.new.type_cast_for_database(2147483647))
|
||||
end
|
||||
|
||||
test "int min value is in range" do
|
||||
assert_equal(-2147483648, Integer.new.type_cast_from_user("-2147483648"))
|
||||
assert_equal(-2147483648, Integer.new.type_cast_for_database(-2147483648))
|
||||
end
|
||||
|
||||
test "columns with a larger limit have larger ranges" do
|
||||
type = Integer.new(limit: 8)
|
||||
|
||||
assert_equal(9223372036854775807, type.type_cast_from_user("9223372036854775807"))
|
||||
assert_equal(-9223372036854775808, type.type_cast_from_user("-9223372036854775808"))
|
||||
assert_equal(9223372036854775807, type.type_cast_for_database(9223372036854775807))
|
||||
assert_equal(-9223372036854775808, type.type_cast_for_database(-9223372036854775808))
|
||||
assert_raises(::RangeError) do
|
||||
type.type_cast_from_user("-9999999999999999999999999999999")
|
||||
type.type_cast_for_database(-9999999999999999999999999999999)
|
||||
end
|
||||
assert_raises(::RangeError) do
|
||||
type.type_cast_from_user("9999999999999999999999999999999")
|
||||
type.type_cast_for_database(9999999999999999999999999999999)
|
||||
end
|
||||
end
|
||||
|
||||
test "values which are out of range can be re-assigned" do
|
||||
klass = Class.new(ActiveRecord::Base) do
|
||||
self.table_name = 'posts'
|
||||
attribute :foo, Type::Integer.new
|
||||
end
|
||||
model = klass.new
|
||||
|
||||
model.foo = 2147483648
|
||||
model.foo = 1
|
||||
|
||||
assert_equal 1, model.foo
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -4,12 +4,12 @@ module ActiveRecord
|
|||
module Type
|
||||
class UnsignedIntegerTest < ActiveRecord::TestCase
|
||||
test "unsigned int max value is in range" do
|
||||
assert_equal(4294967295, UnsignedInteger.new.type_cast_from_user("4294967295"))
|
||||
assert_equal(4294967295, UnsignedInteger.new.type_cast_for_database(4294967295))
|
||||
end
|
||||
|
||||
test "minus value is out of range" do
|
||||
assert_raises(::RangeError) do
|
||||
UnsignedInteger.new.type_cast_from_user("-1")
|
||||
UnsignedInteger.new.type_cast_for_database(-1)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
Loading…
Reference in a new issue