Merge commit 'rails/master'

6 files changed, 75 insertions, 29 deletions

diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb
index 1b7bf42b91..25e329c0c1 100644
--- a/activerecord/lib/active_record/associations/association_collection.rb
+++ b/activerecord/lib/active_record/associations/association_collection.rb
@@ -476,7 +476,7 @@ module ActiveRecord
 
         def callback(method, record)
           callbacks_for(method).each do |callback|
-            ActiveSupport::Callbacks::Callback.new(method, callback, record).call(@owner, record)
+            ActiveSupport::DeprecatedCallbacks::Callback.new(method, callback, record).call(@owner, record)
           end
         end
 
diff --git a/activerecord/lib/active_record/callbacks.rb b/activerecord/lib/active_record/callbacks.rb
index 40a25811c4..b25893a1c3 100644
--- a/activerecord/lib/active_record/callbacks.rb
+++ b/activerecord/lib/active_record/callbacks.rb
@@ -20,7 +20,7 @@ module ActiveRecord
   # * (6) <tt>after_save</tt>
   #
   # That's a total of eight callbacks, which gives you immense power to react and prepare for each state in the
-  # Active Record lifecycle. The sequence for calling <tt>Base#save</tt> for an existing record is similar, except that each 
+  # Active Record lifecycle. The sequence for calling <tt>Base#save</tt> for an existing record is similar, except that each
   # <tt>_on_create</tt> callback is replaced by the corresponding <tt>_on_update</tt> callback.
   #
   # Examples:
@@ -210,7 +210,7 @@ module ActiveRecord
   # instead of quietly returning +false+.
   module Callbacks
     extend ActiveSupport::Concern
-    include ActiveSupport::NewCallbacks
+    include ActiveSupport::Callbacks
 
     CALLBACKS = [
       :after_initialize, :after_find, :before_validation, :after_validation,
diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
index 78c7a4b697..cdf0aebfee 100755
--- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
@@ -32,7 +32,7 @@ module ActiveRecord
     class AbstractAdapter
       include Quoting, DatabaseStatements, SchemaStatements
       include QueryCache
-      include ActiveSupport::Callbacks
+      include ActiveSupport::DeprecatedCallbacks
       define_callbacks :checkout, :checkin
 
       @@row_even = true
@@ -75,7 +75,7 @@ module ActiveRecord
       def supports_ddl_transactions?
         false
       end
-      
+
       # Does this adapter support savepoints? PostgreSQL and MySQL do, SQLite
       # does not.
       def supports_savepoints?
diff --git a/activerecord/lib/active_record/nested_attributes.rb b/activerecord/lib/active_record/nested_attributes.rb
index ec6c02db38..edcf547e01 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
@@ -203,6 +206,12 @@ module ActiveRecord
       #   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=
@@ -214,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)
@@ -227,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)
@@ -238,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
@@ -282,7 +291,8 @@ 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)
+    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?
@@ -295,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
 
@@ -326,11 +336,17 @@ 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
@@ -343,7 +359,7 @@ module ActiveRecord
             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
@@ -372,7 +388,7 @@ module ActiveRecord
     end
 
     def call_reject_if(association_name, attributes)
-      callback = self.class.reject_new_nested_attributes_procs[association_name]
+      callback = self.nested_attributes_options[association_name][:reject_if]
 
       case callback
       when Symbol
diff --git a/activerecord/lib/active_record/validations.rb b/activerecord/lib/active_record/validations.rb
index ab79b520a2..e61b253192 100644
--- a/activerecord/lib/active_record/validations.rb
+++ b/activerecord/lib/active_record/validations.rb
@@ -59,15 +59,15 @@ module ActiveRecord
     end
 
     # Translates an error message in it's default scope (<tt>activerecord.errrors.messages</tt>).
-    # Error messages are first looked up in <tt>models.MODEL.attributes.ATTRIBUTE.MESSAGE</tt>, if it's not there, 
-    # it's looked up in <tt>models.MODEL.MESSAGE</tt> and if that is not there it returns the translation of the 
-    # default message (e.g. <tt>activerecord.errors.messages.MESSAGE</tt>). The translated model name, 
+    # Error messages are first looked up in <tt>models.MODEL.attributes.ATTRIBUTE.MESSAGE</tt>, if it's not there,
+    # it's looked up in <tt>models.MODEL.MESSAGE</tt> and if that is not there it returns the translation of the
+    # default message (e.g. <tt>activerecord.errors.messages.MESSAGE</tt>). The translated model name,
     # translated attribute name and the value are available for interpolation.
     #
     # When using inheritance in your models, it will check all the inherited models too, but only if the model itself
     # hasn't been found. Say you have <tt>class Admin < User; end</tt> and you wanted the translation for the <tt>:blank</tt>
     # error +message+ for the <tt>title</tt> +attribute+, it looks for these translations:
-    # 
+    #
     # <ol>
     # <li><tt>activerecord.errors.models.admin.attributes.title.blank</tt></li>
     # <li><tt>activerecord.errors.models.admin.blank</tt></li>
@@ -80,10 +80,10 @@ module ActiveRecord
       message, options[:default] = options[:default], message if options[:default].is_a?(Symbol)
 
       defaults = @base.class.self_and_descendants_from_active_record.map do |klass|
-        [ :"models.#{klass.name.underscore}.attributes.#{attribute}.#{message}", 
+        [ :"models.#{klass.name.underscore}.attributes.#{attribute}.#{message}",
           :"models.#{klass.name.underscore}.#{message}" ]
       end
-      
+
       defaults << options.delete(:default)
       defaults = defaults.compact.flatten << :"messages.#{message}"
 
@@ -104,7 +104,7 @@ module ActiveRecord
   module Validations
     extend ActiveSupport::Concern
 
-    include ActiveSupport::Callbacks
+    include ActiveSupport::DeprecatedCallbacks
     include ActiveModel::Validations
 
     included do
diff --git a/activerecord/test/cases/nested_attributes_test.rb b/activerecord/test/cases/nested_attributes_test.rb
index e57e361520..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
 
@@ -603,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