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

3 files changed, 62 insertions, 15 deletions

diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb
index 4705d9359c..37c3ca0e07 100755
--- a/activerecord/lib/active_record/base.rb
+++ b/activerecord/lib/active_record/base.rb
@@ -982,12 +982,13 @@ module ActiveRecord #:nodoc:
 
       # 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 @@ module ActiveRecord #:nodoc:
 
       # 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 @@ module ActiveRecord #:nodoc:
       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
diff --git a/activerecord/test/base_test.rb b/activerecord/test/base_test.rb
index 9185d05c8a..043fc6ea6e 100755
--- a/activerecord/test/base_test.rb
+++ b/activerecord/test/base_test.rb
@@ -2,6 +2,7 @@ require 'abstract_unit'
 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 @@ end
 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 @@ class BasicsTest < Test::Unit::TestCase
   
   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 @@ class BasicsTest < Test::Unit::TestCase
     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
diff --git a/activerecord/test/fixtures/developer.rb b/activerecord/test/fixtures/developer.rb
index 78f8f54db7..6d01490844 100644
--- a/activerecord/test/fixtures/developer.rb
+++ b/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