Merge branch 'master' into habtm

* master:
  we can define callbacks without a builder instance
  push methods that don't depend on the instance to the class
  get the name from the reflection
  cache the name and options on the stack
  decouple belongs_to callback definition from the builder instance.
  add_before_destroy_callbacks doesn't depend on the instance, so push it to the class.
  valid_options doesn't depend on the instance, so push it to the class
  use the information on the reflection to determine whether callbacks should be added
  Running all isolated test for actionview
  association builder is no longer needed for counter cache, so remove it
  push constructable? on to the reflection
  Adds the Rails maintenance policy to the Guides
  name is on the reflection, so just use the reflection
  decouple define_callback from the instance
  Added activemodel as a explicit dependency
  unify `Fixes` notes in AR changelog. [ci skip].
  improving `reload` doc wording. #12418 [ci skip]
  Update AR reload doc for the case of manually set primary key attribute [ci skip]

9 files changed, 71 insertions, 53 deletions

diff --git a/activerecord/lib/active_record/associations/builder/association.rb b/activerecord/lib/active_record/associations/builder/association.rb
index 6de0331851..783a86d4a3 100644
--- a/activerecord/lib/active_record/associations/builder/association.rb
+++ b/activerecord/lib/active_record/associations/builder/association.rb
@@ -31,8 +31,8 @@ module ActiveRecord::Associations::Builder
 
       builder = new(name, scope, options, &block)
       reflection = builder.build(model)
-      builder.define_accessors model
-      builder.define_callbacks model, reflection
+      builder.define_accessors model, reflection
+      define_callbacks model, reflection
       builder.define_extensions model
       reflection
     end
@@ -79,8 +79,8 @@ module ActiveRecord::Associations::Builder
     def define_extensions(model)
     end
 
-    def define_callbacks(model, reflection)
-      add_before_destroy_callbacks(model, name) if options[:dependent]
+    def self.define_callbacks(model, reflection)
+      add_before_destroy_callbacks(model, reflection) if reflection.options[:dependent]
       Association.extensions.each do |extension|
         extension.build model, reflection
       end
@@ -93,7 +93,7 @@ module ActiveRecord::Associations::Builder
     #
     # Post.first.comments and Post.first.comments= methods are defined by this method...
 
-    def define_accessors(model)
+    def define_accessors(model, reflection)
       mixin = model.generated_feature_methods
       define_readers(mixin)
       define_writers(mixin)
@@ -115,17 +115,18 @@ module ActiveRecord::Associations::Builder
       CODE
     end
 
-    def valid_dependent_options
+    def self.valid_dependent_options
       raise NotImplementedError
     end
 
     private
 
-    def add_before_destroy_callbacks(model, name)
-      unless valid_dependent_options.include? options[:dependent]
-        raise ArgumentError, "The :dependent option must be one of #{valid_dependent_options}, but is :#{options[:dependent]}"
+    def self.add_before_destroy_callbacks(model, reflection)
+      unless valid_dependent_options.include? reflection.options[:dependent]
+        raise ArgumentError, "The :dependent option must be one of #{valid_dependent_options}, but is :#{reflection.options[:dependent]}"
       end
 
+      name = reflection.name
       model.before_destroy lambda { |o| o.association(name).handle_dependency }
     end
   end
diff --git a/activerecord/lib/active_record/associations/builder/belongs_to.rb b/activerecord/lib/active_record/associations/builder/belongs_to.rb
index 4e88b50ec5..9d55ec850e 100644
--- a/activerecord/lib/active_record/associations/builder/belongs_to.rb
+++ b/activerecord/lib/active_record/associations/builder/belongs_to.rb
@@ -8,21 +8,17 @@ module ActiveRecord::Associations::Builder
       super + [:foreign_type, :polymorphic, :touch]
     end
 
-    def constructable?
-      !options[:polymorphic]
-    end
-
-    def valid_dependent_options
+    def self.valid_dependent_options
       [:destroy, :delete]
     end
 
-    def define_callbacks(model, reflection)
+    def self.define_callbacks(model, reflection)
       super
-      add_counter_cache_callbacks(model, reflection) if options[:counter_cache]
-      add_touch_callbacks(model, reflection)         if options[:touch]
+      add_counter_cache_callbacks(model, reflection) if reflection.options[:counter_cache]
+      add_touch_callbacks(model, reflection)         if reflection.options[:touch]
     end
 
-    def define_accessors(mixin)
+    def define_accessors(mixin, reflection)
       super
       add_counter_cache_methods mixin
     end
@@ -33,18 +29,18 @@ module ActiveRecord::Associations::Builder
       return if mixin.method_defined? :belongs_to_counter_cache_after_create
 
       mixin.class_eval do
-        def belongs_to_counter_cache_after_create(association, reflection)
-          if record = send(association.name)
+        def belongs_to_counter_cache_after_create(reflection)
+          if record = send(reflection.name)
             cache_column = reflection.counter_cache_column
             record.class.increment_counter(cache_column, record.id)
             @_after_create_counter_called = true
           end
         end
 
-        def belongs_to_counter_cache_before_destroy(association, reflection)
+        def belongs_to_counter_cache_before_destroy(reflection)
           foreign_key = reflection.foreign_key.to_sym
           unless destroyed_by_association && destroyed_by_association.foreign_key.to_sym == foreign_key
-            record = send association.name
+            record = send reflection.name
             if record && !self.destroyed?
               cache_column = reflection.counter_cache_column
               record.class.decrement_counter(cache_column, record.id)
@@ -52,13 +48,13 @@ module ActiveRecord::Associations::Builder
           end
         end
 
-        def belongs_to_counter_cache_after_update(association, reflection)
+        def belongs_to_counter_cache_after_update(reflection)
           foreign_key  = reflection.foreign_key
           cache_column = reflection.counter_cache_column
 
           if (@_after_create_counter_called ||= false)
             @_after_create_counter_called = false
-          elsif attribute_changed?(foreign_key) && !new_record? && association.constructable?
+          elsif attribute_changed?(foreign_key) && !new_record? && reflection.constructable?
             model           = reflection.klass
             foreign_key_was = attribute_was foreign_key
             foreign_key     = attribute foreign_key
@@ -74,20 +70,19 @@ module ActiveRecord::Associations::Builder
       end
     end
 
-    def add_counter_cache_callbacks(model, reflection)
+    def self.add_counter_cache_callbacks(model, reflection)
       cache_column = reflection.counter_cache_column
-      association = self
 
       model.after_create lambda { |record|
-        record.belongs_to_counter_cache_after_create(association, reflection)
+        record.belongs_to_counter_cache_after_create(reflection)
       }
 
       model.before_destroy lambda { |record|
-        record.belongs_to_counter_cache_before_destroy(association, reflection)
+        record.belongs_to_counter_cache_before_destroy(reflection)
       }
 
       model.after_update lambda { |record|
-        record.belongs_to_counter_cache_after_update(association, reflection)
+        record.belongs_to_counter_cache_after_update(reflection)
       }
 
       klass = reflection.class_name.safe_constantize
@@ -120,10 +115,10 @@ module ActiveRecord::Associations::Builder
       end
     end
 
-    def add_touch_callbacks(model, reflection)
+    def self.add_touch_callbacks(model, reflection)
       foreign_key = reflection.foreign_key
-      n           = name
-      touch       = options[:touch]
+      n           = reflection.name
+      touch       = reflection.options[:touch]
 
       callback = lambda { |record|
         BelongsTo.touch_record(record, foreign_key, n, touch)
diff --git a/activerecord/lib/active_record/associations/builder/collection_association.rb b/activerecord/lib/active_record/associations/builder/collection_association.rb
index 7bd0687c0b..15f9f9a65f 100644
--- a/activerecord/lib/active_record/associations/builder/collection_association.rb
+++ b/activerecord/lib/active_record/associations/builder/collection_association.rb
@@ -23,9 +23,13 @@ module ActiveRecord::Associations::Builder
       end
     end
 
-    def define_callbacks(model, reflection)
+    def self.define_callbacks(model, reflection)
       super
-      CALLBACKS.each { |callback_name| define_callback(model, callback_name) }
+      name    = reflection.name
+      options = reflection.options
+      CALLBACKS.each { |callback_name|
+        define_callback(model, callback_name, name, options)
+      }
     end
 
     def define_extensions(model)
@@ -35,7 +39,7 @@ module ActiveRecord::Associations::Builder
       end
     end
 
-    def define_callback(model, callback_name)
+    def self.define_callback(model, callback_name, name, options)
       full_callback_name = "#{callback_name}_for_#{name}"
 
       # TODO : why do i need method_defined? I think its because of the inheritance chain
diff --git a/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb b/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb
index f3bf406196..4b47824148 100644
--- a/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb
+++ b/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb
@@ -117,9 +117,9 @@ module ActiveRecord::Associations::Builder
       super + [:join_table, :association_foreign_key]
     end
 
-    def define_callbacks(model, reflection)
+    def self.define_callbacks(model, reflection)
       super
-      name = self.name
+      name = reflection.name
       model.send(:include, Module.new {
         class_eval <<-RUBY, __FILE__, __LINE__ + 1
           def destroy_associations
diff --git a/activerecord/lib/active_record/associations/builder/has_many.rb b/activerecord/lib/active_record/associations/builder/has_many.rb
index a60cb4769a..7909b93622 100644
--- a/activerecord/lib/active_record/associations/builder/has_many.rb
+++ b/activerecord/lib/active_record/associations/builder/has_many.rb
@@ -8,7 +8,7 @@ module ActiveRecord::Associations::Builder
       super + [:primary_key, :dependent, :as, :through, :source, :source_type, :inverse_of, :counter_cache]
     end
 
-    def valid_dependent_options
+    def self.valid_dependent_options
       [:destroy, :delete_all, :nullify, :restrict_with_error, :restrict_with_exception]
     end
   end
diff --git a/activerecord/lib/active_record/associations/builder/has_one.rb b/activerecord/lib/active_record/associations/builder/has_one.rb
index 62d454ce55..f359efd496 100644
--- a/activerecord/lib/active_record/associations/builder/has_one.rb
+++ b/activerecord/lib/active_record/associations/builder/has_one.rb
@@ -10,18 +10,14 @@ module ActiveRecord::Associations::Builder
       valid
     end
 
-    def constructable?
-      !options[:through]
-    end
-
-    def valid_dependent_options
+    def self.valid_dependent_options
       [:destroy, :delete, :nullify, :restrict_with_error, :restrict_with_exception]
     end
 
     private
 
-    def add_before_destroy_callbacks(model, name)
-      super unless options[:through]
+    def self.add_before_destroy_callbacks(model, reflection)
+      super unless reflection.options[:through]
     end
   end
 end
diff --git a/activerecord/lib/active_record/associations/builder/singular_association.rb b/activerecord/lib/active_record/associations/builder/singular_association.rb
index d97c0e9afd..10d568ebc0 100644
--- a/activerecord/lib/active_record/associations/builder/singular_association.rb
+++ b/activerecord/lib/active_record/associations/builder/singular_association.rb
@@ -6,13 +6,9 @@ module ActiveRecord::Associations::Builder
       super + [:remote, :dependent, :counter_cache, :primary_key, :inverse_of]
     end
 
-    def constructable?
-      true
-    end
-
-    def define_accessors(model)
+    def define_accessors(model, reflection)
       super
-      define_constructors(model.generated_feature_methods) if constructable?
+      define_constructors(model.generated_feature_methods) if reflection.constructable?
     end
 
     # Defines the (build|create)_association methods for belongs_to or has_one association
diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb
index bdd00ee259..267aa28058 100644
--- a/activerecord/lib/active_record/persistence.rb
+++ b/activerecord/lib/active_record/persistence.rb
@@ -335,8 +335,18 @@ module ActiveRecord
 
     # Reloads the record from the database.
     #
-    # This method modifies the receiver in-place. Attributes are updated, and
-    # caches busted, in particular the associations cache.
+    # This method finds record by its primary key (which could be assigned manually) and
+    # modifies the receiver in-place:
+    #
+    #   account = Account.new
+    #   # => #<Account id: nil, email: nil>
+    #   account.id = 1
+    #   account.reload
+    #   # Account Load (1.2ms)  SELECT "accounts".* FROM "accounts" WHERE "accounts"."id" = $1 LIMIT 1  [["id", 1]]
+    #   # => #<Account id: 1, email: 'account@example.com'>
+    #
+    # Attributes are reloaded from the database, and caches busted, in
+    # particular the associations cache.
     #
     # If the record no longer exists in the database <tt>ActiveRecord::RecordNotFound</tt>
     # is raised. Otherwise, in addition to the in-place modification the method
diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb
index f47282b7fd..263ba922da 100644
--- a/activerecord/lib/active_record/reflection.rb
+++ b/activerecord/lib/active_record/reflection.rb
@@ -200,6 +200,7 @@ module ActiveRecord
         @automatic_inverse_of = nil
         @type         = options[:as] && "#{options[:as]}_type"
         @foreign_type = options[:foreign_type] || "#{name}_type"
+        @constructable = calculate_constructable(macro, options)
       end
 
       # Returns a new, unsaved instance of the associated class. +attributes+ will
@@ -208,6 +209,10 @@ module ActiveRecord
         klass.new(attributes, &block)
       end
 
+      def constructable? # :nodoc:
+        @constructable
+      end
+
       def table_name
         klass.table_name
       end
@@ -379,6 +384,17 @@ module ActiveRecord
       end
 
       private
+      def calculate_constructable(macro, options)
+        case macro
+        when :belongs_to
+          !options[:polymorphic]
+        when :has_one
+          !options[:through]
+        else
+          true
+        end
+      end
+
         # Attempts to find the inverse association name automatically.
         # If it cannot find a suitable inverse association name, it returns
         # nil.