Merge commit 'mainstream/master'

2 files changed, 101 insertions, 47 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