aboutsummaryrefslogtreecommitdiffstats
path: root/activerecord
diff options
context:
space:
mode:
authorSean Griffin <sean@thoughtbot.com>2015-01-23 14:22:53 -0700
committerSean Griffin <sean@thoughtbot.com>2015-01-23 14:30:23 -0700
commit7c6f3938dee47f0932c2a1d4924adaebc25517ac (patch)
tree89ce24319bf2159209fba8faefc4b80dafaee894 /activerecord
parent96e504ec8af149962312c13dd418e13e4c74ce86 (diff)
downloadrails-7c6f3938dee47f0932c2a1d4924adaebc25517ac.tar.gz
rails-7c6f3938dee47f0932c2a1d4924adaebc25517ac.tar.bz2
rails-7c6f3938dee47f0932c2a1d4924adaebc25517ac.zip
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: - https://github.com/thoughtbot/shoulda-matchers/blob/9ba21381d7caf045053a81f32df7de2f49687820/lib/shoulda/matchers/active_model/allow_value_matcher.rb#L261-L263 - https://github.com/rails/rails/issues/18653#issuecomment-71197026
Diffstat (limited to 'activerecord')
-rw-r--r--activerecord/CHANGELOG.md8
-rw-r--r--activerecord/lib/active_record/type/integer.rb14
-rw-r--r--activerecord/test/cases/type/integer_test.rb39
-rw-r--r--activerecord/test/cases/type/unsigned_integer_test.rb4
4 files changed, 45 insertions, 20 deletions
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index 2054c9573e..22b1c8e95b 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -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
diff --git a/activerecord/lib/active_record/type/integer.rb b/activerecord/lib/active_record/type/integer.rb
index 394fe008f9..90ca9f88da 100644
--- a/activerecord/lib/active_record/type/integer.rb
+++ b/activerecord/lib/active_record/type/integer.rb
@@ -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
diff --git a/activerecord/test/cases/type/integer_test.rb b/activerecord/test/cases/type/integer_test.rb
index ff956b7680..0c60f0690c 100644
--- a/activerecord/test/cases/type/integer_test.rb
+++ b/activerecord/test/cases/type/integer_test.rb
@@ -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
diff --git a/activerecord/test/cases/type/unsigned_integer_test.rb b/activerecord/test/cases/type/unsigned_integer_test.rb
index b6f673109e..4b8e2fb518 100644
--- a/activerecord/test/cases/type/unsigned_integer_test.rb
+++ b/activerecord/test/cases/type/unsigned_integer_test.rb
@@ -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