Fixed that clone would break when an aggregate had the same name as one of its...

Fixed that clone would break when an aggregate had the same name as one of its attributes #1307 [bitsweat]

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

activerecord/lib/active_record/base.rb

@@ -982,12 +982,13 @@ def destroy
 
       # Returns a clone of the record that hasn't been assigned an id yet and is treated as a new record.
       def clone
-        attrs = self.attributes
+        attrs = self.attributes_before_type_cast
         attrs.delete(self.class.primary_key)
-        cloned_record = self.class.new(attrs)
-        cloned_record
+        self.class.new do |record|
+          record.send :instance_variable_set, '@attributes', attrs
+        end
       end
-            
+
       # Updates a single attribute and saves the record. This is especially useful for boolean flags on existing records.
       # Note: This method is overwritten by the Validation module that'll make sure that updates made with this method
       # doesn't get subjected to validation checks. Hence, attributes can be updated even if the full object isn't valid.
@@ -1076,14 +1077,12 @@ def attributes=(attributes)
 
       # Returns a hash of all the attributes with their names as keys and clones of their objects as values.
       def attributes
-        self.attribute_names.inject({}) do |attributes, name|
-          begin
-            attributes[name] = read_attribute(name).clone
-          rescue TypeError, NoMethodError
-            attributes[name] = read_attribute(name)
-          end
-          attributes
-        end
+        clone_attributes :read_attribute
+      end
+
+      # Returns a hash of cloned attributes before typecasting and deserialization.
+      def attributes_before_type_cast
+        clone_attributes :read_attribute_before_type_cast
       end
 
       # Returns true if the specified +attribute+ has been set by the user or by a database load and is neither
@@ -1420,5 +1419,19 @@ def object_from_yaml(string)
       def has_yaml_encoding_header?(string)
         string[0..3] == "--- "
       end
+
+      def clone_attributes(reader_method = :read_attribute, attributes = {})
+        self.attribute_names.inject(attributes) do |attributes, name|
+          attributes[name] = clone_attribute_value(reader_method, name)
+          attributes
+        end
+      end
+
+      def clone_attribute_value(reader_method, attribute_name)
+        value = send(reader_method, attribute_name)
+        value.clone
+      rescue TypeError, NoMethodError
+        value
+      end
   end
 end

activerecord/test/base_test.rb

@@ -2,6 +2,7 @@
 require 'fixtures/topic'
 require 'fixtures/reply'
 require 'fixtures/company'
+require 'fixtures/developer'
 require 'fixtures/project'
 require 'fixtures/default'
 require 'fixtures/auto_id'
@@ -33,7 +34,7 @@ class TightDescendant < TightPerson
 class Booleantest < ActiveRecord::Base; end
 
 class BasicsTest < Test::Unit::TestCase
-  fixtures :topics, :companies, :projects, :computers
+  fixtures :topics, :companies, :developers, :projects, :computers
 
   def test_set_attributes
     topic = Topic.find(1)
@@ -568,7 +569,8 @@ def test_boolean
   
   def test_clone
     topic = Topic.find(1)
-    cloned_topic = topic.clone
+    cloned_topic = nil
+    assert_nothing_raised { cloned_topic = topic.clone }
     assert_equal topic.title, cloned_topic.title
     assert cloned_topic.new_record?
 
@@ -588,7 +590,33 @@ def test_clone
     assert !cloned_topic.new_record?
     assert cloned_topic.id != topic.id
   end
-  
+
+  def test_clone_with_aggregate_of_same_name_as_attribute
+    dev = DeveloperWithAggregate.find(1)
+    assert_kind_of DeveloperSalary, dev.salary
+
+    clone = nil
+    assert_nothing_raised { clone = dev.clone }
+    assert_kind_of DeveloperSalary, clone.salary
+    assert_equal dev.salary.amount, clone.salary.amount
+    assert clone.new_record?
+
+    # test if the attributes have been cloned
+    original_amount = clone.salary.amount
+    dev.salary.amount = 1
+    assert_equal original_amount, clone.salary.amount
+
+    assert clone.save
+    assert !clone.new_record?
+    assert clone.id != dev.id
+  end
+
+  def test_clone_preserves_subtype
+    clone = nil
+    assert_nothing_raised { clone = Company.find(3).clone }
+    assert_kind_of Client, clone
+  end
+
   def test_bignum
     company = Company.find(1)
     company.rating = 2147483647

activerecord/test/fixtures/developer.rb

@@ -4,3 +4,9 @@ class Developer < ActiveRecord::Base
   validates_inclusion_of :salary, :in => 50000..200000
   validates_length_of    :name, :within => 3..20
 end
+
+DeveloperSalary = Struct.new(:amount)
+class DeveloperWithAggregate < ActiveRecord::Base
+  self.table_name = 'developers'
+  composed_of :salary, :class_name => 'DeveloperSalary', :mapping => [%w(salary amount)]
+end