diff options
| -rw-r--r-- | activerecord/lib/active_record/aggregations.rb | 15 | ||||
| -rw-r--r-- | activerecord/test/cases/aggregations_test.rb | 18 | ||||
| -rw-r--r-- | activerecord/test/models/customer.rb | 5 |
3 files changed, 31 insertions, 7 deletions
diff --git a/activerecord/lib/active_record/aggregations.rb b/activerecord/lib/active_record/aggregations.rb index 90d3b58c78..f7dcc019c8 100644 --- a/activerecord/lib/active_record/aggregations.rb +++ b/activerecord/lib/active_record/aggregations.rb @@ -187,7 +187,8 @@ module ActiveRecord # * <tt>:converter</tt> - A symbol specifying the name of a class method of <tt>:class_name</tt> # or a Proc that is called when a new value is assigned to the value object. The converter is # passed the single value that is used in the assignment and is only called if the new value is - # not an instance of <tt>:class_name</tt>. + # not an instance of <tt>:class_name</tt>. If <tt>:allow_nil</tt> is set to true, the converter + # can return nil to skip the assignment. # # Option examples: # composed_of :temperature, :mapping => %w(reading celsius) @@ -234,16 +235,16 @@ module ActiveRecord def writer_method(name, class_name, mapping, allow_nil, converter) define_method("#{name}=") do |part| + unless part.is_a?(class_name.constantize) || converter.nil? || part.nil? + part = converter.respond_to?(:call) ? + converter.call(part) : + class_name.constantize.send(converter, part) + end + if part.nil? && allow_nil mapping.each { |pair| self[pair.first] = nil } @aggregation_cache[name] = nil else - unless part.is_a?(class_name.constantize) || converter.nil? - part = converter.respond_to?(:call) ? - converter.call(part) : - class_name.constantize.send(converter, part) - end - mapping.each { |pair| self[pair.first] = part.send(pair.last) } @aggregation_cache[name] = part.freeze end diff --git a/activerecord/test/cases/aggregations_test.rb b/activerecord/test/cases/aggregations_test.rb index 3e0e6dce2c..5bd8f76ba2 100644 --- a/activerecord/test/cases/aggregations_test.rb +++ b/activerecord/test/cases/aggregations_test.rb @@ -109,6 +109,24 @@ class AggregationsTest < ActiveRecord::TestCase assert_nil customers(:david).gps_location end + def test_nil_return_from_converter_is_respected_when_allow_nil_is_true + customers(:david).non_blank_gps_location = "" + customers(:david).save + customers(:david).reload + assert_nil customers(:david).non_blank_gps_location + end + + def test_nil_return_from_converter_results_in_failure_when_allow_nil_is_false + assert_raises(NoMethodError) do + customers(:barney).gps_location = "" + end + end + + def test_do_not_run_the_converter_when_nil_was_set + customers(:david).non_blank_gps_location = nil + assert_nil Customer.gps_conversion_was_run + end + def test_custom_constructor assert_equal 'Barney GUMBLE', customers(:barney).fullname.to_s assert_kind_of Fullname, customers(:barney).fullname diff --git a/activerecord/test/models/customer.rb b/activerecord/test/models/customer.rb index 777f6b5ba0..807f25b687 100644 --- a/activerecord/test/models/customer.rb +++ b/activerecord/test/models/customer.rb @@ -1,7 +1,12 @@ class Customer < ActiveRecord::Base + + cattr_accessor :gps_conversion_was_run + composed_of :address, :mapping => [ %w(address_street street), %w(address_city city), %w(address_country country) ], :allow_nil => true composed_of :balance, :class_name => "Money", :mapping => %w(balance amount), :converter => Proc.new { |balance| balance.to_money } composed_of :gps_location, :allow_nil => true + composed_of :non_blank_gps_location, :class_name => "GpsLocation", :allow_nil => true, :mapping => %w(gps_location gps_location), + :converter => lambda { |gps| self.gps_conversion_was_run = true; gps.blank? ? nil : GpsLocation.new(gps)} composed_of :fullname, :mapping => %w(name to_s), :constructor => Proc.new { |name| Fullname.parse(name) }, :converter => :parse end |