Added :allow_nil option for aggregations (closes #5091) [ian.w.white@gmail.com]

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@4353 5ecf4fe2-1ee6-0310-87b1-e25e094e27de

5 files changed, 85 insertions, 18 deletions

diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG
index 09960b6650..06d9db76fb 100644
--- a/activerecord/CHANGELOG
+++ b/activerecord/CHANGELOG
@@ -1,5 +1,7 @@
 *SVN*
 
+* Added :allow_nil option for aggregations #5091 [ian.w.white@gmail.com]
+
 * Fix Oracle boolean support and tests. Closes #5139. [schoenm@earthlink.net]
 
 * create! no longer blows up when no attributes are passed and a :create scope is in effect (e.g. foo.bars.create! failed whereas foo.bars.create!({}) didn't.) [Jeremy Kemper]
diff --git a/activerecord/lib/active_record/aggregations.rb b/activerecord/lib/active_record/aggregations.rb
index 3f524f591c..f6b604c118 100644
--- a/activerecord/lib/active_record/aggregations.rb
+++ b/activerecord/lib/active_record/aggregations.rb
@@ -118,46 +118,73 @@ module ActiveRecord
       #   if the real class name is +CompanyAddress+, you'll have to specify it with this option.
       # * <tt>:mapping</tt> - specifies a number of mapping arrays (attribute, parameter) that bind an attribute name
       #   to a constructor parameter on the value class.
+      # * <tt>:allow_nil</tt> - specifies that the aggregate object will not be instantiated when all mapped
+      #   attributes are nil.  Setting the aggregate class to nil has the effect of writing nil to all mapped attributes.
+      #   This defaults to false.
       #
       # Option examples:
       #   composed_of :temperature, :mapping => %w(reading celsius)
       #   composed_of :balance, :class_name => "Money", :mapping => %w(balance amount)
       #   composed_of :address, :mapping => [ %w(address_street street), %w(address_city city) ]
       #   composed_of :gps_location
+      #   composed_of :gps_location, :allow_nil => true
+      #
       def composed_of(part_id, options = {})
-        options.assert_valid_keys(:class_name, :mapping)
+        options.assert_valid_keys(:class_name, :mapping, :allow_nil)
 
         name        = part_id.id2name
         class_name  = options[:class_name] || name.camelize
-        mapping     = options[:mapping] || [ name, name ]
+        mapping     = options[:mapping]    || [ name, name ]
+        allow_nil   = options[:allow_nil]  || false
 
-        reader_method(name, class_name, mapping)
-        writer_method(name, class_name, mapping)
+        reader_method(name, class_name, mapping, allow_nil)
+        writer_method(name, class_name, mapping, allow_nil)
         
         create_reflection(:composed_of, part_id, options, self)
       end
 
       private
-        
-        def reader_method(name, class_name, mapping)
+        def reader_method(name, class_name, mapping, allow_nil)
+          mapping = (Array === mapping.first ? mapping : [ mapping ])
+
+          allow_nil_condition = if allow_nil
+            mapping.collect { |pair| "!read_attribute(\"#{pair.first}\").nil?"}.join(" && ")
+          else
+            "true"
+          end
+
           module_eval <<-end_eval
             def #{name}(force_reload = false)
-              if @#{name}.nil? || force_reload
-                @#{name} = #{class_name}.new(#{(Array === mapping.first ? mapping : [ mapping ]).collect{ |pair| "read_attribute(\"#{pair.first}\")"}.join(", ")})
+              if (@#{name}.nil? || force_reload) && #{allow_nil_condition}
+                @#{name} = #{class_name}.new(#{mapping.collect { |pair| "read_attribute(\"#{pair.first}\")"}.join(", ")})
               end
-              
               return @#{name}
             end
           end_eval
         end        
         
-        def writer_method(name, class_name, mapping)
-          module_eval <<-end_eval
-            def #{name}=(part)
-              @#{name} = part.freeze
-              #{(Array === mapping.first ? mapping : [ mapping ]).collect{ |pair| "@attributes[\"#{pair.first}\"] = part.#{pair.last}" }.join("\n")}
-            end
-          end_eval
+        def writer_method(name, class_name, mapping, allow_nil)
+          mapping = (Array === mapping.first ? mapping : [ mapping ])
+
+          if allow_nil
+            module_eval <<-end_eval
+              def #{name}=(part)
+                if part.nil?
+                  #{mapping.collect { |pair| "@attributes[\"#{pair.first}\"] = nil" }.join("\n")}
+                else
+                  @#{name} = part.freeze
+                  #{mapping.collect { |pair| "@attributes[\"#{pair.first}\"] = part.#{pair.last}" }.join("\n")}
+                end
+              end
+            end_eval
+          else
+            module_eval <<-end_eval
+              def #{name}=(part)
+                @#{name} = part.freeze
+                #{mapping.collect{ |pair| "@attributes[\"#{pair.first}\"] = part.#{pair.last}" }.join("\n")}
+              end
+            end_eval
+          end
         end
     end
   end
diff --git a/activerecord/test/aggregations_test.rb b/activerecord/test/aggregations_test.rb
index 39adedae53..8cd4bfe481 100644
--- a/activerecord/test/aggregations_test.rb
+++ b/activerecord/test/aggregations_test.rb
@@ -63,4 +63,33 @@ class AggregationsTest < Test::Unit::TestCase
   def test_gps_inequality
     assert GpsLocation.new('39x110') != GpsLocation.new('39x111')
   end
+  
+  def test_allow_nil_gps_is_nil
+    assert_equal nil, customers(:zaphod).gps_location
+  end
+  
+  def test_allow_nil_gps_set_to_nil
+    customers(:david).gps_location = nil
+    customers(:david).save
+    customers(:david).reload
+    assert_equal nil, customers(:david).gps_location
+  end
+  
+  def test_allow_nil_set_address_attributes_to_nil
+    customers(:zaphod).address = nil
+    assert_equal nil, customers(:zaphod).attributes[:address_street]
+    assert_equal nil, customers(:zaphod).attributes[:address_city]
+    assert_equal nil, customers(:zaphod).attributes[:address_country]
+  end
+  
+  def test_allow_nil_address_set_to_nil
+    customers(:zaphod).address = nil
+    customers(:zaphod).save
+    customers(:zaphod).reload
+    assert_equal nil, customers(:zaphod).address
+  end
+  
+  def test_nil_raises_error_when_allow_nil_is_false
+    assert_raises(NoMethodError) { customers(:david).balance = nil }
+  end
 end
diff --git a/activerecord/test/fixtures/customer.rb b/activerecord/test/fixtures/customer.rb
index e23fd03ade..ccbe035931 100644
--- a/activerecord/test/fixtures/customer.rb
+++ b/activerecord/test/fixtures/customer.rb
@@ -1,7 +1,7 @@
 class Customer < ActiveRecord::Base
-  composed_of :address, :mapping => [ %w(address_street street), %w(address_city city), %w(address_country country) ]
+  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)
-  composed_of :gps_location
+  composed_of :gps_location, :allow_nil => true
 end
 
 class Address
diff --git a/activerecord/test/fixtures/customers.yml b/activerecord/test/fixtures/customers.yml
index 9169d7d413..f802aac5ea 100644
--- a/activerecord/test/fixtures/customers.yml
+++ b/activerecord/test/fixtures/customers.yml
@@ -6,3 +6,12 @@ david:
   address_city: Scary Town
   address_country: Loony Land
   gps_location: 35.544623640962634x-105.9309951055148
+  
+zaphod:
+  id: 2
+  name: Zaphod
+  balance: 62
+  address_street: Avenue Road
+  address_city: Hamlet Town
+  address_country: Nation Land
+  gps_location: NULL
\ No newline at end of file