Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix numericality validator on decimal attribute with implicit scale #42125

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 17 additions & 18 deletions activemodel/lib/active_model/validations/numericality.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,10 @@ def check_validity!
end
end

def validate_each(record, attr_name, value, precision: Float::DIG, scale: nil)
unless is_number?(value, precision, scale)
def validate_each(record, attr_name, value)
maybe_decimal = maybe_decimal(record, attr_name)

unless is_number?(value, maybe_decimal)
record.errors.add(attr_name, :not_a_number, **filtered_options(value))
return
end
Expand All @@ -42,7 +44,7 @@ def validate_each(record, attr_name, value, precision: Float::DIG, scale: nil)
return
end

value = parse_as_number(value, precision, scale)
value = parse_as_number(value, maybe_decimal)

options.slice(*RESERVED_OPTIONS).each do |option, option_value|
if NUMBER_CHECKS.include?(option)
Expand All @@ -54,7 +56,7 @@ def validate_each(record, attr_name, value, precision: Float::DIG, scale: nil)
record.errors.add(attr_name, option, **filtered_options(value).merge!(count: option_value))
end
elsif COMPARE_CHECKS.include?(option)
option_value = option_as_number(record, option_value, precision, scale)
option_value = parse_as_number(option_value(record, option_value))
unless value.public_send(COMPARE_CHECKS[option], option_value)
record.errors.add(attr_name, option, **filtered_options(value).merge!(count: option_value))
end
Expand All @@ -63,34 +65,31 @@ def validate_each(record, attr_name, value, precision: Float::DIG, scale: nil)
end

private
def option_as_number(record, option_value, precision, scale)
parse_as_number(option_value(record, option_value), precision, scale)
def maybe_decimal(record, attr_name)
maybe_decimal = record.class.try(:type_for_attribute, attr_name)
maybe_decimal if maybe_decimal&.type == :decimal
end

def parse_as_number(raw_value, precision, scale)
def parse_as_number(raw_value, maybe_decimal = nil)
if raw_value.is_a?(Float)
parse_float(raw_value, precision, scale)
parse_float(raw_value, maybe_decimal)
elsif raw_value.is_a?(BigDecimal)
round(raw_value, scale)
maybe_decimal ? maybe_decimal.cast(raw_value) : raw_value
elsif raw_value.is_a?(Numeric)
raw_value
elsif is_integer?(raw_value)
raw_value.to_i
elsif !is_hexadecimal_literal?(raw_value)
parse_float(Kernel.Float(raw_value), precision, scale)
parse_float(Kernel.Float(raw_value), maybe_decimal)
end
end

def parse_float(raw_value, precision, scale)
round(raw_value, scale).to_d(precision)
end

def round(raw_value, scale)
scale ? raw_value.round(scale) : raw_value
def parse_float(raw_value, maybe_decimal)
maybe_decimal ? maybe_decimal.cast(raw_value) : raw_value.to_d
end

def is_number?(raw_value, precision, scale)
!parse_as_number(raw_value, precision, scale).nil?
def is_number?(raw_value, maybe_decimal)
!parse_as_number(raw_value, maybe_decimal).nil?
rescue ArgumentError, TypeError
false
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,19 @@ def test_validates_numericality_equality_for_float_and_big_decimal
Topic.validates_numericality_of :approved, equal_to: BigDecimal("65.6")

assert_invalid_values([Float("65.5"), BigDecimal("65.7")], "must be equal to 65.6")
assert_valid_values([Float("65.6"), BigDecimal("65.6")])
assert_valid_values([BigDecimal("65.6")])

# BigDecimal 3.0.0 has a rounding problem https://github.com/ruby/bigdecimal/issues/185
# caused by fixing https://github.com/ruby/bigdecimal/issues/70.
# It will be fixed by https://github.com/ruby/bigdecimal/pull/180
# in BigDecimal 3.0.1.
if "9.050000000000001" == 9.05.to_d.to_s("F")
assert_equal "65.59999999999999", 65.6.to_d.to_s("F")
assert_invalid_values([Float("65.6")])
else
assert_equal "65.6", 65.6.to_d.to_s("F")
assert_valid_values([Float("65.6")])
end
end

private
Expand Down
1 change: 0 additions & 1 deletion activerecord/lib/active_record/validations.rb
Original file line number Diff line number Diff line change
Expand Up @@ -91,4 +91,3 @@ def perform_validations(options = {})
require "active_record/validations/presence"
require "active_record/validations/absence"
require "active_record/validations/length"
require "active_record/validations/numericality"
35 changes: 0 additions & 35 deletions activerecord/lib/active_record/validations/numericality.rb

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -12,21 +12,23 @@ def setup

def test_column_with_precision
model_class.validates_numericality_of(
:unscaled_bank_balance, equal_to: 10_000_000.12
:unscaled_bank_balance, equal_to: 10_000_000.0
)

subject = model_class.new(unscaled_bank_balance: 10_000_000.121)

assert_equal BigDecimal("10_000_000.0"), subject.unscaled_bank_balance
assert_predicate subject, :valid?
end

def test_column_with_precision_higher_than_double_fig
model_class.validates_numericality_of(
:decimal_number_big_precision, equal_to: 10_000_000.3
:decimal_number_big_precision, equal_to: 10_000_000.0
)

subject = model_class.new(decimal_number_big_precision: 10_000_000.3)

assert_equal BigDecimal("10_000_000.0"), subject.decimal_number_big_precision
assert_predicate subject, :valid?
end

Expand All @@ -37,6 +39,7 @@ def test_column_with_scale

subject = model_class.new(bank_balance: 10.001)

assert_equal BigDecimal("10.00"), subject.bank_balance
assert_not_predicate subject, :valid?
end

Expand All @@ -47,7 +50,15 @@ def test_no_column_precision

subject = model_class.new(decimal_number: 1_000_000_000.1234545)

assert_predicate subject, :valid?
# In MySQL, if precision is omitted, the default is 10.
# https://dev.mysql.com/doc/refman/8.0/en/precision-math-decimal-characteristics.html
if current_adapter?(:Mysql2Adapter)
assert_equal BigDecimal("1_000_000_000"), subject.decimal_number
assert_not_predicate subject, :valid?
else
assert_equal 1_000_000_000.123454.to_d, subject.decimal_number
assert_predicate subject, :valid?
end
end

def test_virtual_attribute
Expand All @@ -58,6 +69,7 @@ def test_virtual_attribute

subject = model_class.new(virtual_decimal_number: 1_000_000_000.1234545)

assert_equal 1_000_000_000.123454.to_d, subject.virtual_decimal_number
assert_predicate subject, :valid?
end

Expand All @@ -78,6 +90,7 @@ def self.name
end
subject = klass.new(bank_balance: 10_000_000.12)

assert_equal BigDecimal("10_000_000.12"), subject.bank_balance
assert_predicate(subject, :valid?)
end

Expand All @@ -89,7 +102,17 @@ def test_virtual_attribute_without_precision

subject = model_class.new(virtual_decimal_number: 65.6)

assert_predicate subject, :valid?
# BigDecimal 3.0.0 has a rounding problem https://github.com/ruby/bigdecimal/issues/185
# caused by fixing https://github.com/ruby/bigdecimal/issues/70.
# It will be fixed by https://github.com/ruby/bigdecimal/pull/180
# in BigDecimal 3.0.1.
if "9.050000000000001" == 9.05.to_d.to_s("F")
assert_equal BigDecimal("65.59999999999999"), subject.virtual_decimal_number
assert_not_predicate subject, :valid?
else
assert_equal BigDecimal("65.6"), subject.virtual_decimal_number
assert_predicate subject, :valid?
end
end

def test_virtual_attribute_with_precision_round_down
Expand All @@ -100,6 +123,7 @@ def test_virtual_attribute_with_precision_round_down

subject = model_class.new(virtual_decimal_number: 123.454)

assert_equal BigDecimal("123.45"), subject.virtual_decimal_number
assert_predicate subject, :valid?
end

Expand All @@ -115,8 +139,10 @@ def test_virtual_attribute_with_precision_round_half_even
# BigDecimal's to_d behavior changed in BigDecimal 3.0.1, see https://github.com/ruby/bigdecimal/issues/70
# TODO: replace this with a check against BigDecimal::VERSION, currently
# we just check the behaviour because both versions of BigDecimal report "3.0.0"
assert_equal BigDecimal("123.46"), subject.virtual_decimal_number
assert_not_predicate subject, :valid?
else
assert_equal BigDecimal("123.45"), subject.virtual_decimal_number
assert_predicate subject, :valid?
end
end
Expand All @@ -129,6 +155,7 @@ def test_virtual_attribute_with_precision_round_up

subject = model_class.new(virtual_decimal_number: 123.456)

assert_equal BigDecimal("123.46"), subject.virtual_decimal_number
assert_not_predicate subject, :valid?
end

Expand All @@ -140,13 +167,14 @@ def test_virtual_attribute_with_scale

subject = model_class.new(virtual_decimal_number: 1.001)

assert_equal BigDecimal("1.00"), subject.virtual_decimal_number
assert_not_predicate subject, :valid?
end

def test_virtual_attribute_with_precision_and_scale
model_class.attribute(:virtual_decimal_number, :decimal, precision: 4, scale: 2)
model_class.validates_numericality_of(
:virtual_decimal_number, less_than_or_equal_to: 99.99
:virtual_decimal_number, less_than_or_equal_to: BigDecimal("99.99")
)

["99.994", 99.994, BigDecimal("99.994")].each do |raw_value|
Expand All @@ -167,6 +195,7 @@ def test_aliased_attribute

subject = model_class.new(new_bank_balance: "abcd")

assert_equal BigDecimal("0.0"), subject.new_bank_balance
assert_not_predicate subject, :valid?
end

Expand All @@ -175,6 +204,7 @@ def test_allow_nil_works_for_casted_value

subject = model_class.new(bank_balance: "")

assert_nil subject.new_bank_balance
assert_predicate subject, :valid?
end
end