Assigning an instance of a foreign class to a composed_of aggregate calls an optional conversion block. Refactor and simplify composed_of implementation. Closes #6322.

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

3 files changed, 30 insertions, 39 deletions

diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG
index d2cfdc6a4d..c9f5534215 100644
--- a/activerecord/CHANGELOG
+++ b/activerecord/CHANGELOG
@@ -1,5 +1,7 @@
 *SVN*
 
+* Assigning an instance of a foreign class to a composed_of aggregate calls an optional conversion block. Refactor and simplify composed_of implementation.  #6322 [brandon, Chris Cruft]
+
 * Assigning nil to a composed_of aggregate also sets its immediate value to nil.  #9843 [Chris Cruft]
 
 * Ensure that mysql quotes table names with database names correctly.  Closes #9911 [crayz]
diff --git a/activerecord/lib/active_record/aggregations.rb b/activerecord/lib/active_record/aggregations.rb
index 8ee9d8368d..c227893765 100644
--- a/activerecord/lib/active_record/aggregations.rb
+++ b/activerecord/lib/active_record/aggregations.rb
@@ -122,68 +122,57 @@ module ActiveRecord
       #   attributes are +nil+.  Setting the aggregate class to +nil+ has the effect of writing +nil+ to all mapped attributes.
       #   This defaults to +false+.
       #
+      # An optional block can be passed to convert the argument that is passed to the writer method into an instance of
+      # <tt>:class_name</tt>. The block will only be called if the arguement is not already an instance of <tt>:class_name</tt>.
+      #
       # Option examples:
       #   composed_of :temperature, :mapping => %w(reading celsius)
-      #   composed_of :balance, :class_name => "Money", :mapping => %w(balance amount)
+      #   composed_of(:balance, :class_name => "Money", :mapping => %w(balance amount)) {|balance| balance.to_money }
       #   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 = {})
+      def composed_of(part_id, options = {}, &block)
         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     = [ mapping ] unless mapping.first.is_a?(Array)
         allow_nil   = options[:allow_nil]  || false
 
         reader_method(name, class_name, mapping, allow_nil)
-        writer_method(name, class_name, mapping, allow_nil)
+        writer_method(name, class_name, mapping, allow_nil, block)
         
         create_reflection(:composed_of, part_id, options, self)
       end
 
       private
         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, __FILE__, __LINE__
-            def #{name}(force_reload = false)
-              if (@#{name}.nil? || force_reload) && #{allow_nil_condition}
-                @#{name} = #{class_name}.new(#{mapping.collect { |pair| "read_attribute(\"#{pair.first}\")"}.join(", ")})
+          module_eval do
+            define_method(name) do |*args|
+              force_reload = args.first || false
+              if (instance_variable_get("@#{name}").nil? || force_reload) && (!allow_nil || mapping.any? {|pair| !read_attribute(pair.first).nil? })
+                instance_variable_set("@#{name}", class_name.constantize.new(*mapping.collect {|pair| read_attribute(pair.first)}))
               end
-              return @#{name}
+              return instance_variable_get("@#{name}")
             end
-          end_eval
-        end
+          end
 
-        def writer_method(name, class_name, mapping, allow_nil)
-          mapping = (Array === mapping.first ? mapping : [ mapping ])
+        end
 
-          if allow_nil
-            module_eval <<-end_eval, __FILE__, __LINE__
-              def #{name}=(part)
-                @#{name} = part.freeze
-                if part.nil?
-                  #{mapping.collect { |pair| "@attributes[\"#{pair.first}\"] = nil" }.join("\n")}
-                else
-                  #{mapping.collect { |pair| "@attributes[\"#{pair.first}\"] = part.#{pair.last}" }.join("\n")}
-                end
+        def writer_method(name, class_name, mapping, allow_nil, conversion)
+          module_eval do
+            define_method("#{name}=") do |part|
+              if part.nil? && allow_nil
+                mapping.each { |pair| @attributes[pair.first] = nil }
+                instance_variable_set("@#{name}", nil)
+              else
+                part = conversion.call(part) unless part.is_a?(class_name.constantize) || conversion.nil?
+                mapping.each { |pair| @attributes[pair.first] = part.send(pair.last) }
+                instance_variable_set("@#{name}", part.freeze)
               end
-            end_eval
-          else
-            module_eval <<-end_eval, __FILE__, __LINE__
-              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/fixtures/customer.rb b/activerecord/test/fixtures/customer.rb
index ccbe035931..3d8d644f12 100644
--- a/activerecord/test/fixtures/customer.rb
+++ b/activerecord/test/fixtures/customer.rb
@@ -1,6 +1,6 @@
 class Customer < ActiveRecord::Base
   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(:balance, :class_name => "Money", :mapping => %w(balance amount)) { |balance| balance.to_money }
   composed_of :gps_location, :allow_nil => true
 end
 
@@ -52,4 +52,4 @@ class GpsLocation
   def ==(other)
     self.latitude == other.latitude && self.longitude == other.longitude
   end
-end
\ No newline at end of file
+end