Merge commit 'mainstream/master'

5 files changed, 168 insertions, 52 deletions

diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb
index 502fe0442e..150e3fc263 100755
--- a/activerecord/lib/active_record/base.rb
+++ b/activerecord/lib/active_record/base.rb
@@ -2427,6 +2427,29 @@ module ActiveRecord #:nodoc:
         result
       end
 
+      # Cloned objects have no id assigned and are treated as new records. Note that this is a "shallow" clone
+      # as it copies the object's attributes only, not its associations. The extent of a "deep" clone is
+      # application specific and is therefore left to the application to implement according to its need.
+      def initialize_copy(other)
+        # Think the assertion which fails if the after_initialize callback goes at the end of the method is wrong. The
+        # deleted clone method called new which therefore called the after_initialize callback. It then went on to copy
+        # over the attributes. But if it's copying the attributes afterwards then it hasn't finished initializing right?
+        # For example in the test suite the topic model's after_initialize method sets the author_email_address to
+        # test@test.com. I would have thought this would mean that all cloned models would have an author email address
+        # of test@test.com. However the test_clone test method seems to test that this is not the case. As a result the
+        # after_initialize callback has to be run *before* the copying of the atrributes rather than afterwards in order
+        # for all tests to pass. This makes no sense to me.
+        callback(:after_initialize) if respond_to_without_attributes?(:after_initialize)
+        cloned_attributes = other.clone_attributes(:read_attribute_before_type_cast)
+        cloned_attributes.delete(self.class.primary_key)
+        @attributes = cloned_attributes
+        clear_aggregation_cache
+        @attributes_cache = {}
+        @new_record = true
+        ensure_proper_type
+        self.class.send(:scope, :create).each { |att, value| self.send("#{att}=", value) } if self.class.send(:scoped?, :create)
+      end
+
       # Returns a String, which Action Pack uses for constructing an URL to this
       # object. The default implementation returns this record's id as a String,
       # or nil if this record's unsaved.
@@ -2555,19 +2578,6 @@ module ActiveRecord #:nodoc:
         freeze
       end
 
-      # Returns a clone of the record that hasn't been assigned an id yet and
-      # is treated as a new record.  Note that this is a "shallow" clone:
-      # it copies the object's attributes only, not its associations.
-      # The extent of a "deep" clone is application-specific and is therefore
-      # left to the application to implement according to its need.
-      def clone
-        attrs = clone_attributes(:read_attribute_before_type_cast)
-        attrs.delete(self.class.primary_key)
-        record = self.class.new
-        record.send :instance_variable_set, '@attributes', attrs
-        record
-      end
-
       # Returns an instance of the specified +klass+ with the attributes of the current record. This is mostly useful in relation to
       # single-table inheritance structures where you want a subclass to appear as the superclass. This can be used along with record
       # identification in Action Pack to allow, say, <tt>Client < Company</tt> to do something like render <tt>:partial => @client.becomes(Company)</tt>
@@ -2831,6 +2841,21 @@ module ActiveRecord #:nodoc:
         "#<#{self.class} #{attributes_as_nice_string}>"
       end
 
+    protected
+      def clone_attributes(reader_method = :read_attribute, attributes = {})
+        self.attribute_names.inject(attributes) do |attrs, name|
+          attrs[name] = clone_attribute_value(reader_method, name)
+          attrs
+        end
+      end
+
+      def clone_attribute_value(reader_method, attribute_name)
+        value = send(reader_method, attribute_name)
+        value.duplicable? ? value.clone : value
+      rescue TypeError, NoMethodError
+        value
+      end
+
     private
       def create_or_update
         raise ReadOnlyRecord if readonly?
@@ -3093,20 +3118,6 @@ module ActiveRecord #:nodoc:
         return string unless string.is_a?(String) && string =~ /^---/
         YAML::load(string) rescue string
       end
-
-      def clone_attributes(reader_method = :read_attribute, attributes = {})
-        self.attribute_names.inject(attributes) do |attrs, name|
-          attrs[name] = clone_attribute_value(reader_method, name)
-          attrs
-        end
-      end
-
-      def clone_attribute_value(reader_method, attribute_name)
-        value = send(reader_method, attribute_name)
-        value.duplicable? ? value.clone : value
-      rescue TypeError, NoMethodError
-        value
-      end
   end
 
   Base.class_eval do
diff --git a/activerecord/lib/active_record/nested_attributes.rb b/activerecord/lib/active_record/nested_attributes.rb
index 9c29f32639..f65ee9465e 100644
--- a/activerecord/lib/active_record/nested_attributes.rb
+++ b/activerecord/lib/active_record/nested_attributes.rb
@@ -3,11 +3,14 @@ require 'active_support/core_ext/object/try'
 
 module ActiveRecord
   module NestedAttributes #:nodoc:
+    class TooManyRecords < ActiveRecordError
+    end
+
     extend ActiveSupport::Concern
 
     included do
-      class_inheritable_accessor :reject_new_nested_attributes_procs, :instance_writer => false
-      self.reject_new_nested_attributes_procs = {}
+      class_inheritable_accessor :nested_attributes_options, :instance_writer => false
+      self.nested_attributes_options = {}
     end
 
     # == Nested Attributes
@@ -127,6 +130,22 @@ module ActiveRecord
     #   member.posts.first.title # => 'Kari, the awesome Ruby documentation browser!'
     #   member.posts.second.title # => 'The egalitarian assumption of the modern citizen'
     #
+    #  Alternatively, :reject_if also accepts a symbol for using methods:
+    #
+    #    class Member < ActiveRecord::Base
+    #      has_many :posts
+    #      accepts_nested_attributes_for :posts, :reject_if => :new_record?
+    #    end
+    #
+    #    class Member < ActiveRecord::Base
+    #      has_many :posts
+    #      accepts_nested_attributes_for :posts, :reject_if => :reject_posts
+    #
+    #      def reject_posts(attributed)
+    #        attributed['title].blank?
+    #      end
+    #    end
+    #
     # If the hash contains an <tt>id</tt> key that matches an already
     # associated record, the matching record will be modified:
     #
@@ -179,13 +198,20 @@ module ActiveRecord
       #   <tt>_destroy</tt> key and a value that evaluates to +true+
       #   (eg. 1, '1', true, or 'true'). This option is off by default.
       # [:reject_if]
-      #   Allows you to specify a Proc that checks whether a record should be
-      #   built for a certain attribute hash. The hash is passed to the Proc
-      #   and the Proc should return either +true+ or +false+. When no Proc
-      #   is specified a record will be built for all attribute hashes that
+      #   Allows you to specify a Proc or a Symbol pointing to a method
+      #   that checks whether a record should be built for a certain attribute
+      #   hash. The hash is passed to the supplied Proc or the method
+      #   and it should return either +true+ or +false+. When no :reject_if
+      #   is specified, a record will be built for all attribute hashes that
       #   do not have a <tt>_destroy</tt> value that evaluates to true.
       #   Passing <tt>:all_blank</tt> instead of a Proc will create a proc
       #   that will reject a record where all the attributes are blank.
+      # [:limit]
+      #   Allows you to specify the maximum number of the associated records that
+      #   can be processes with the nested attributes. If the size of the
+      #   nested attributes array exceeds the specified limit, NestedAttributes::TooManyRecords
+      #   exception is raised. If omitted, any number associations can be processed.
+      #   Note that the :limit option is only applicable to one-to-many associations.
       #
       # Examples:
       #   # creates avatar_attributes=
@@ -197,7 +223,7 @@ module ActiveRecord
       def accepts_nested_attributes_for(*attr_names)
         options = { :allow_destroy => false }
         options.update(attr_names.extract_options!)
-        options.assert_valid_keys(:allow_destroy, :reject_if)
+        options.assert_valid_keys(:allow_destroy, :reject_if, :limit)
 
         attr_names.each do |association_name|
           if reflection = reflect_on_association(association_name)
@@ -210,10 +236,10 @@ module ActiveRecord
 
             reflection.options[:autosave] = true
 
-            self.reject_new_nested_attributes_procs[association_name.to_sym] = if options[:reject_if] == :all_blank
-              proc { |attributes| attributes.all? {|k,v| v.blank?} }
-            else
-              options[:reject_if]
+            self.nested_attributes_options[association_name.to_sym] = options
+
+            if options[:reject_if] == :all_blank
+              self.nested_attributes_options[association_name.to_sym][:reject_if] = proc { |attributes| attributes.all? {|k,v| v.blank?} }
             end
 
             # def pirate_attributes=(attributes)
@@ -221,7 +247,7 @@ module ActiveRecord
             # end
             class_eval %{
               def #{association_name}_attributes=(attributes)
-                assign_nested_attributes_for_#{type}_association(:#{association_name}, attributes, #{options[:allow_destroy]})
+                assign_nested_attributes_for_#{type}_association(:#{association_name}, attributes)
               end
             }, __FILE__, __LINE__
           else
@@ -265,8 +291,9 @@ module ActiveRecord
     # If the given attributes include a matching <tt>:id</tt> attribute _and_ a
     # <tt>:_destroy</tt> key set to a truthy value, then the existing record
     # will be marked for destruction.
-    def assign_nested_attributes_for_one_to_one_association(association_name, attributes, allow_destroy)
-      attributes = attributes.stringify_keys
+    def assign_nested_attributes_for_one_to_one_association(association_name, attributes)
+      options = self.nested_attributes_options[association_name]
+      attributes = attributes.with_indifferent_access
 
       if attributes['id'].blank?
         unless reject_new_record?(association_name, attributes)
@@ -278,7 +305,7 @@ module ActiveRecord
           end
         end
       elsif (existing_record = send(association_name)) && existing_record.id.to_s == attributes['id'].to_s
-        assign_to_or_mark_for_destruction(existing_record, attributes, allow_destroy)
+        assign_to_or_mark_for_destruction(existing_record, attributes, options[:allow_destroy])
       end
     end
 
@@ -309,24 +336,30 @@ module ActiveRecord
     #     { :name => 'John' },
     #     { :id => '2', :_destroy => true }
     #   ])
-    def assign_nested_attributes_for_collection_association(association_name, attributes_collection, allow_destroy)
+    def assign_nested_attributes_for_collection_association(association_name, attributes_collection)
+      options = self.nested_attributes_options[association_name]
+
       unless attributes_collection.is_a?(Hash) || attributes_collection.is_a?(Array)
         raise ArgumentError, "Hash or Array expected, got #{attributes_collection.class.name} (#{attributes_collection.inspect})"
       end
 
+      if options[:limit] && attributes_collection.size > options[:limit]
+        raise TooManyRecords, "Maximum #{options[:limit]} records are allowed. Got #{attributes_collection.size} records instead."
+      end
+
       if attributes_collection.is_a? Hash
         attributes_collection = attributes_collection.sort_by { |index, _| index.to_i }.map { |_, attributes| attributes }
       end
 
       attributes_collection.each do |attributes|
-        attributes = attributes.stringify_keys
+        attributes = attributes.with_indifferent_access
 
         if attributes['id'].blank?
           unless reject_new_record?(association_name, attributes)
             send(association_name).build(attributes.except(*UNASSIGNABLE_KEYS))
           end
         elsif existing_record = send(association_name).detect { |record| record.id.to_s == attributes['id'].to_s }
-          assign_to_or_mark_for_destruction(existing_record, attributes, allow_destroy)
+          assign_to_or_mark_for_destruction(existing_record, attributes, options[:allow_destroy])
         end
       end
     end
@@ -351,8 +384,18 @@ module ActiveRecord
     # has_destroy_flag? or if a <tt>:reject_if</tt> proc exists for this
     # association and evaluates to +true+.
     def reject_new_record?(association_name, attributes)
-      has_destroy_flag?(attributes) ||
-        self.class.reject_new_nested_attributes_procs[association_name].try(:call, attributes)
+      has_destroy_flag?(attributes) || call_reject_if(association_name, attributes)
+    end
+
+    def call_reject_if(association_name, attributes)
+      callback = self.nested_attributes_options[association_name][:reject_if]
+
+      case callback
+      when Symbol
+        method(callback).arity == 0 ? send(callback) : send(callback, attributes)
+      when Proc
+        callback.try(:call, attributes)
+      end
     end
   end
 end
diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb
index 8421a8fb07..1e2d903492 100755
--- a/activerecord/test/cases/base_test.rb
+++ b/activerecord/test/cases/base_test.rb
@@ -1352,7 +1352,7 @@ class BasicsTest < ActiveRecord::TestCase
     cloned_topic.title["a"] = "c"
     assert_equal "b", topic.title["a"]
 
-    #test if attributes set as part of after_initialize are cloned correctly
+    # test if attributes set as part of after_initialize are cloned correctly
     assert_equal topic.author_email_address, cloned_topic.author_email_address
 
     # test if saved clone object differs from original
diff --git a/activerecord/test/cases/nested_attributes_test.rb b/activerecord/test/cases/nested_attributes_test.rb
index 721792132c..53fd168e1b 100644
--- a/activerecord/test/cases/nested_attributes_test.rb
+++ b/activerecord/test/cases/nested_attributes_test.rb
@@ -29,13 +29,13 @@ class TestNestedAttributesInGeneral < ActiveRecord::TestCase
     Pirate.accepts_nested_attributes_for :ship, :allow_destroy => true, :reject_if => proc { |attributes| attributes.empty? }
   end
 
-  def test_base_should_have_an_empty_reject_new_nested_attributes_procs
-    assert_equal Hash.new, ActiveRecord::Base.reject_new_nested_attributes_procs
+  def test_base_should_have_an_empty_nested_attributes_options
+    assert_equal Hash.new, ActiveRecord::Base.nested_attributes_options
   end
 
-  def test_should_add_a_proc_to_reject_new_nested_attributes_procs
+  def test_should_add_a_proc_to_nested_attributes_options
     [:parrots, :birds, :birds_with_reject_all_blank].each do |name|
-      assert_instance_of Proc, Pirate.reject_new_nested_attributes_procs[name]
+      assert_instance_of Proc, Pirate.nested_attributes_options[name][:reject_if]
     end
   end
 
@@ -84,6 +84,34 @@ class TestNestedAttributesInGeneral < ActiveRecord::TestCase
     ship = Ship.create!(:name => 'Nights Dirty Lightning')
     ship._delete
   end
+
+  def test_reject_if_method_without_arguments
+    Pirate.accepts_nested_attributes_for :ship, :reject_if => :new_record?
+
+    pirate = Pirate.new(:catchphrase => "Stop wastin' me time")
+    pirate.ship_attributes = { :name => 'Black Pearl' }
+    assert_no_difference('Ship.count') { pirate.save! }
+  end
+
+  def test_reject_if_method_with_arguments
+    Pirate.accepts_nested_attributes_for :ship, :reject_if => :reject_empty_ships_on_create
+
+    pirate = Pirate.new(:catchphrase => "Stop wastin' me time")
+    pirate.ship_attributes = { :name => 'Red Pearl', :_reject_me_if_new => true }
+    assert_no_difference('Ship.count') { pirate.save! }
+
+    # pirate.reject_empty_ships_on_create returns false for saved records
+    pirate.ship_attributes = { :name => 'Red Pearl', :_reject_me_if_new => true }
+    assert_difference('Ship.count') { pirate.save! }
+  end
+
+  def test_reject_if_with_indifferent_keys
+    Pirate.accepts_nested_attributes_for :ship, :reject_if => proc {|attributes| attributes[:name].blank? }
+
+    pirate = Pirate.new(:catchphrase => "Stop wastin' me time")
+    pirate.ship_attributes = { :name => 'Hello Pearl' }
+    assert_difference('Ship.count') { pirate.save! }
+  end
 end
 
 class TestNestedAttributesOnAHasOneAssociation < ActiveRecord::TestCase
@@ -575,3 +603,33 @@ class TestNestedAttributesOnAHasAndBelongsToManyAssociation < ActiveRecord::Test
 
   include NestedAttributesOnACollectionAssociationTests
 end
+
+class TestNestedAttributesLimit < ActiveRecord::TestCase
+  def setup
+    Pirate.accepts_nested_attributes_for :parrots, :limit => 2
+
+    @pirate = Pirate.create!(:catchphrase => "Don' botharrr talkin' like one, savvy?")
+  end
+
+  def teardown
+    Pirate.accepts_nested_attributes_for :parrots, :allow_destroy => true, :reject_if => proc { |attributes| attributes.empty? }
+  end
+
+  def test_limit_with_less_records
+    @pirate.attributes = { :parrots_attributes => { 'foo' => { :name => 'Big Big Love' } } }
+    assert_difference('Parrot.count') { @pirate.save! }
+  end
+
+  def test_limit_with_number_exact_records
+    @pirate.attributes = { :parrots_attributes => { 'foo' => { :name => 'Lovely Day' }, 'bar' => { :name => 'Blown Away' } } }
+    assert_difference('Parrot.count', 2) { @pirate.save! }
+  end
+
+  def test_limit_with_exceeding_records
+    assert_raises(ActiveRecord::NestedAttributes::TooManyRecords) do
+      @pirate.attributes = { :parrots_attributes => { 'foo' => { :name => 'Lovely Day' },
+                                                      'bar' => { :name => 'Blown Away' },
+                                                      'car' => { :name => 'The Happening' }} }
+    end
+  end
+end
diff --git a/activerecord/test/models/pirate.rb b/activerecord/test/models/pirate.rb
index 3d7c4bc48a..05c5b666ae 100644
--- a/activerecord/test/models/pirate.rb
+++ b/activerecord/test/models/pirate.rb
@@ -45,6 +45,10 @@ class Pirate < ActiveRecord::Base
     @ship_log ||= []
   end
 
+  def reject_empty_ships_on_create(attributes)
+    attributes.delete('_reject_me_if_new').present? && new_record?
+  end
+
   private
     def log_before_add(record)
       log(record, "before_adding_method")