Merge remote branch 'eloy/master'

4 files changed, 67 insertions, 62 deletions

diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb
index b2ae447d5e..1320f5b624 100755
--- a/activerecord/lib/active_record/associations.rb
+++ b/activerecord/lib/active_record/associations.rb
@@ -1770,7 +1770,7 @@ module ActiveRecord
         end
 
         def using_limitable_reflections?(reflections)
-          reflections.reject { |r| [ :belongs_to, :has_one ].include?(r.macro) }.length.zero?
+          reflections.collect(&:collection_association?).length.zero?
         end
 
         def column_aliases(join_dependency)
@@ -1843,7 +1843,7 @@ module ActiveRecord
             case associations
               when Symbol, String
                 reflection = base.reflections[associations]
-                if reflection && [:has_many, :has_and_belongs_to_many].include?(reflection.macro)
+                if reflection && reflection.collection_association?
                   records.each { |record| record.send(reflection.name).target.uniq! }
                 end
               when Array
@@ -1853,12 +1853,11 @@ module ActiveRecord
               when Hash
                 associations.keys.each do |name|
                   reflection = base.reflections[name]
-                  is_collection = [:has_many, :has_and_belongs_to_many].include?(reflection.macro)
 
                   parent_records = []
                   records.each do |record|
                     if descendant = record.send(reflection.name)
-                      if is_collection
+                      if reflection.collection_association?
                         parent_records.concat descendant.target.uniq
                       else
                         parent_records << descendant
diff --git a/activerecord/lib/active_record/autosave_association.rb b/activerecord/lib/active_record/autosave_association.rb
index 98ab64537e..f01d0903cd 100644
--- a/activerecord/lib/active_record/autosave_association.rb
+++ b/activerecord/lib/active_record/autosave_association.rb
@@ -158,47 +158,39 @@ module ActiveRecord
       #
       # For performance reasons, we don't check whether to validate at runtime,
       # but instead only define the method and callback when needed. However,
-      # this can change, for instance, when using nested attributes. Since we
-      # don't want the callbacks to get defined multiple times, there are
-      # guards that check if the save or validation methods have already been
-      # defined before actually defining them.
+      # this can change, for instance, when using nested attributes, which is
+      # called _after_ the association has been defined. Since we don't want
+      # the callbacks to get defined multiple times, there are guards that
+      # check if the save or validation methods have already been defined
+      # before actually defining them.
       def add_autosave_association_callbacks(reflection)
-        save_method = "autosave_associated_records_for_#{reflection.name}"
-        validation_method = "validate_associated_records_for_#{reflection.name}"
-        force_validation = (reflection.options[:validate] == true || reflection.options[:autosave] == true)
+        save_method = :"autosave_associated_records_for_#{reflection.name}"
+        validation_method = :"validate_associated_records_for_#{reflection.name}"
+        collection = reflection.collection_association?
 
-        case reflection.macro
-        when :has_many, :has_and_belongs_to_many
-          unless method_defined?(save_method)
+        unless method_defined?(save_method)
+          if collection
             before_save :before_save_collection_association
 
             define_method(save_method) { save_collection_association(reflection) }
             # Doesn't use after_save as that would save associations added in after_create/after_update twice
             after_create save_method
             after_update save_method
-          end
-
-          if !method_defined?(validation_method) &&
-              (force_validation || (reflection.macro == :has_many && reflection.options[:validate] != false))
-            define_method(validation_method) { validate_collection_association(reflection) }
-            validate validation_method
-          end
-        else
-          unless method_defined?(save_method)
-            case reflection.macro
-            when :has_one
+          else
+            if reflection.macro == :has_one
               define_method(save_method) { save_has_one_association(reflection) }
               after_save save_method
-            when :belongs_to
+            else
               define_method(save_method) { save_belongs_to_association(reflection) }
               before_save save_method
             end
           end
+        end
 
-          if !method_defined?(validation_method) && force_validation
-            define_method(validation_method) { validate_single_association(reflection) }
-            validate validation_method
-          end
+        if reflection.validate? && !method_defined?(validation_method)
+          method = (collection ? :validate_collection_association : :validate_single_association)
+          define_method(validation_method) { send(method, reflection) }
+          validate validation_method
         end
       end
     end
diff --git a/activerecord/lib/active_record/nested_attributes.rb b/activerecord/lib/active_record/nested_attributes.rb
index ff3a51d5c0..9038888d22 100644
--- a/activerecord/lib/active_record/nested_attributes.rb
+++ b/activerecord/lib/active_record/nested_attributes.rb
@@ -188,6 +188,8 @@ module ActiveRecord
     # the parent model is saved. This happens inside the transaction initiated
     # by the parents save method. See ActiveRecord::AutosaveAssociation.
     module ClassMethods
+      REJECT_ALL_BLANK_PROC = proc { |attributes| attributes.all? { |_, value| value.blank? } }
+
       # Defines an attributes writer for the specified association(s). If you
       # are using <tt>attr_protected</tt> or <tt>attr_accessible</tt>, then you
       # will need to add the attribute writer to the allowed list.
@@ -229,23 +231,14 @@ module ActiveRecord
         options = { :allow_destroy => false, :update_only => false }
         options.update(attr_names.extract_options!)
         options.assert_valid_keys(:allow_destroy, :reject_if, :limit, :update_only)
+        options[:reject_if] = REJECT_ALL_BLANK_PROC if options[:reject_if] == :all_blank
 
         attr_names.each do |association_name|
           if reflection = reflect_on_association(association_name)
-            type = case reflection.macro
-            when :has_one, :belongs_to
-              :one_to_one
-            when :has_many, :has_and_belongs_to_many
-              :collection
-            end
-
             reflection.options[:autosave] = true
             add_autosave_association_callbacks(reflection)
-            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
+            nested_attributes_options[association_name.to_sym] = options
+            type = (reflection.collection_association? ? :collection : :one_to_one)
 
             # def pirate_attributes=(attributes)
             #   assign_nested_attributes_for_one_to_one_association(:pirate, attributes)
@@ -271,21 +264,11 @@ module ActiveRecord
       marked_for_destruction?
     end
 
-    # Deal with deprecated _delete.
-    #
-    def _delete #:nodoc:
-      ActiveSupport::Deprecation.warn "_delete is deprecated in nested attributes. Use _destroy instead."
-      _destroy
-    end
-
     private
 
     # Attribute hash keys that should not be assigned as normal attributes.
     # These hash keys are nested attributes implementation details.
-    #
-    # TODO Remove _delete from UNASSIGNABLE_KEYS when deprecation warning are
-    # removed.
-    UNASSIGNABLE_KEYS = %w( id _destroy _delete )
+    UNASSIGNABLE_KEYS = %w( id _destroy )
 
     # Assigns the given attributes to the association.
     #
@@ -298,13 +281,17 @@ module ActiveRecord
     # update_only is true, 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)
-      options = self.nested_attributes_options[association_name]
+      options = nested_attributes_options[association_name]
       attributes = attributes.with_indifferent_access
       check_existing_record = (options[:update_only] || !attributes['id'].blank?)
 
       if check_existing_record && (record = send(association_name)) &&
           (options[:update_only] || record.id.to_s == attributes['id'].to_s)
         assign_to_or_mark_for_destruction(record, attributes, options[:allow_destroy])
+
+      elsif attributes['id']
+        raise_nested_attributes_record_not_found(association_name, attributes['id'])
+
       elsif !reject_new_record?(association_name, attributes)
         method = "build_#{association_name}"
         if respond_to?(method)
@@ -343,7 +330,7 @@ module ActiveRecord
     #     { :id => '2', :_destroy => true }
     #   ])
     def assign_nested_attributes_for_collection_association(association_name, attributes_collection)
-      options = self.nested_attributes_options[association_name]
+      options = 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})"
@@ -366,6 +353,8 @@ module ActiveRecord
           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, options[:allow_destroy])
+        else
+          raise_nested_attributes_record_not_found(association_name, attributes['id'])
         end
       end
     end
@@ -382,8 +371,7 @@ module ActiveRecord
 
     # Determines if a hash contains a truthy _destroy key.
     def has_destroy_flag?(hash)
-      ConnectionAdapters::Column.value_to_boolean(hash['_destroy']) ||
-      ConnectionAdapters::Column.value_to_boolean(hash['_delete']) # TODO Remove after deprecation.
+      ConnectionAdapters::Column.value_to_boolean(hash['_destroy'])
     end
 
     # Determines if a new record should be build by checking for
@@ -394,14 +382,17 @@ module ActiveRecord
     end
 
     def call_reject_if(association_name, attributes)
-      callback = self.nested_attributes_options[association_name][:reject_if]
-
-      case callback
+      case callback = nested_attributes_options[association_name][:reject_if]
       when Symbol
         method(callback).arity == 0 ? send(callback) : send(callback, attributes)
       when Proc
-        callback.try(:call, attributes)
+        callback.call(attributes)
       end
     end
+
+    def raise_nested_attributes_record_not_found(association_name, record_id)
+      reflection = self.class.reflect_on_association(association_name)
+      raise RecordNotFound, "Couldn't find #{reflection.klass.name} with ID=#{record_id} for #{self.class.name} with ID=#{id}"
+    end
   end
 end
diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb
index b751c9ad68..d6fad5cf29 100644
--- a/activerecord/lib/active_record/reflection.rb
+++ b/activerecord/lib/active_record/reflection.rb
@@ -252,10 +252,33 @@ module ActiveRecord
         end
       end
 
+      # Returns whether or not this association reflection is for a collection
+      # association. Returns +true+ if the +macro+ is one of +has_many+ or
+      # +has_and_belongs_to_many+, +false+ otherwise.
+      def collection_association?
+        if @collection_association.nil?
+          @collection_association = [:has_many, :has_and_belongs_to_many].include?(macro)
+        end
+        @collection_association
+      end
+
+      # Returns whether or not the association should be validated as part of
+      # the parent's validation.
+      #
+      # Unless you explicitely disable validation with
+      # <tt>:validate => false</tt>, it will take place when:
+      #
+      # * you explicitely enable validation; <tt>:validate => true</tt>
+      # * you use autosave; <tt>:autosave => true</tt>
+      # * the association is a +has_many+ association
+      def validate?
+        !options[:validate].nil? ? options[:validate] : (options[:autosave] == true || macro == :has_many)
+      end
+
       private
         def derive_class_name
           class_name = name.to_s.camelize
-          class_name = class_name.singularize if [ :has_many, :has_and_belongs_to_many ].include?(macro)
+          class_name = class_name.singularize if collection_association?
           class_name
         end