From bcd3589f9828c302988a08ed733eb506d807757f Mon Sep 17 00:00:00 2001 From: Andreas Reischuck Date: Wed, 25 Oct 2023 11:31:44 +0200 Subject: [PATCH] Fix casting PG money with comma as radix point The `/^-?\D*+[\d.]+,\d{2}$/` regexp in the guard does not guarantee a match for the subsequent `gsub!`. For example, when casting `"3,50"`. Thus `gsub!` could return `nil`, and the chained `sub!` call would raise `NoMethodError`. This commit splits up the call chain. It also replaces `gsub!` with `delete!` and `sub!` with `tr!` since regexps are unnecessary for these cases. Co-authored-by: Jonathan Hefner --- .../postgresql/oid/money.rb | 5 ++-- .../cases/adapters/postgresql/money_test.rb | 24 ++++++++++++------- 2 files changed, 19 insertions(+), 10 deletions(-) diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/oid/money.rb b/activerecord/lib/active_record/connection_adapters/postgresql/oid/money.rb index 3703e9a646dfc..86310407bfe75 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/oid/money.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/oid/money.rb @@ -27,9 +27,10 @@ def cast_value(value) value = value.sub(/^\((.+)\)$/, '-\1') # (4) case value when /^-?\D*+[\d,]+\.\d{2}$/ # (1) - value.gsub!(/[^-\d.]/, "") + value.delete!("^-0-9.") when /^-?\D*+[\d.]+,\d{2}$/ # (2) - value.gsub!(/[^-\d,]/, "").sub!(/,/, ".") + value.delete!("^-0-9,") + value.tr!(",", ".") end super(value) diff --git a/activerecord/test/cases/adapters/postgresql/money_test.rb b/activerecord/test/cases/adapters/postgresql/money_test.rb index da3643e57f02a..7e18dce1b9d7c 100644 --- a/activerecord/test/cases/adapters/postgresql/money_test.rb +++ b/activerecord/test/cases/adapters/postgresql/money_test.rb @@ -54,14 +54,22 @@ def test_money_values def test_money_type_cast type = PostgresqlMoney.type_for_attribute("wealth") - assert_equal(12345678.12, type.cast(+"$12,345,678.12")) - assert_equal(12345678.12, type.cast(+"$12.345.678,12")) - assert_equal(12345678.12, type.cast(+"12,345,678.12")) - assert_equal(12345678.12, type.cast(+"12.345.678,12")) - assert_equal(-1.15, type.cast(+"-$1.15")) - assert_equal(-2.25, type.cast(+"($2.25)")) - assert_equal(-1.15, type.cast(+"-1.15")) - assert_equal(-2.25, type.cast(+"(2.25)")) + + { + "12,345,678.12" => 12345678.12, + "12.345.678,12" => 12345678.12, + "0.12" => 0.12, + "0,12" => 0.12, + }.each do |string, number| + assert_equal number, type.cast(string) + assert_equal number, type.cast("$#{string}") + + assert_equal(-number, type.cast("-#{string}")) + assert_equal(-number, type.cast("-$#{string}")) + + assert_equal(-number, type.cast("(#{string})")) + assert_equal(-number, type.cast("($#{string})")) + end end def test_money_regex_backtracking