Use method compilation for association methods

Method compilation provides better performance and I think the code
comes out cleaner as well.

A knock on effect is that methods that get redefined produce warnings. I
think this is a good thing. I had to deal with a bunch of warnings
coming from our tests, though.

activerecord/lib/active_record/associations/builder/association.rb

@@ -63,17 +63,19 @@ module ActiveRecord::Associations::Builder
     end
 
     def define_readers
-      name = self.name
-      mixin.redefine_method(name) do |*params|
-        association(name).reader(*params)
-      end
+      mixin.class_eval <<-CODE, __FILE__, __LINE__ + 1
+        def #{name}(*args)
+          association(:#{name}).reader(*args)
+        end
+      CODE
     end
 
     def define_writers
-      name = self.name
-      mixin.redefine_method("#{name}=") do |value|
-        association(name).writer(value)
-      end
+      mixin.class_eval <<-CODE, __FILE__, __LINE__ + 1
+        def #{name}=(value)
+          association(:#{name}).writer(value)
+        end
+      CODE
     end
 
     def configure_dependency
@@ -88,10 +90,11 @@ module ActiveRecord::Associations::Builder
         )
       end
 
-      name = self.name
-      mixin.redefine_method("#{macro}_dependent_for_#{name}") do
-        association(name).handle_dependency
-      end
+      mixin.class_eval <<-CODE, __FILE__, __LINE__ + 1
+        def #{macro}_dependent_for_#{name}
+          association(:#{name}).handle_dependency
+        end
+      CODE
 
       model.before_destroy "#{macro}_dependent_for_#{name}"
     end

activerecord/lib/active_record/associations/builder/belongs_to.rb

@@ -21,49 +21,42 @@ module ActiveRecord::Associations::Builder
 
     def add_counter_cache_callbacks(reflection)
       cache_column = reflection.counter_cache_column
-      name         = self.name
 
-      method_name = "belongs_to_counter_cache_after_create_for_#{name}"
-      mixin.redefine_method(method_name) do
-        record = send(name)
-        record.class.increment_counter(cache_column, record.id) unless record.nil?
-      end
-      model.after_create(method_name)
-
-      method_name = "belongs_to_counter_cache_before_destroy_for_#{name}"
-      mixin.redefine_method(method_name) do
-        unless marked_for_destruction?
-          record = send(name)
-          record.class.decrement_counter(cache_column, record.id) unless record.nil?
+      mixin.class_eval <<-CODE, __FILE__, __LINE__ + 1
+        def belongs_to_counter_cache_after_create_for_#{name}
+          record = #{name}
+          record.class.increment_counter(:#{cache_column}, record.id) unless record.nil?
         end
-      end
-      model.before_destroy(method_name)
 
-      model.send(:module_eval,
-        "#{reflection.class_name}.send(:attr_readonly,\"#{cache_column}\".intern) if defined?(#{reflection.class_name}) && #{reflection.class_name}.respond_to?(:attr_readonly)", __FILE__, __LINE__
-      )
+        def belongs_to_counter_cache_before_destroy_for_#{name}
+          unless marked_for_destruction?
+            record = #{name}
+            record.class.decrement_counter(:#{cache_column}, record.id) unless record.nil?
+          end
+        end
+      CODE
+
+      model.after_create   "belongs_to_counter_cache_after_create_for_#{name}"
+      model.before_destroy "belongs_to_counter_cache_before_destroy_for_#{name}"
+
+      klass = reflection.class_name.safe_constantize
+      klass.attr_readonly cache_column if klass && klass.respond_to?(:attr_readonly)
     end
 
     def add_touch_callbacks(reflection)
-      name        = self.name
-      method_name = "belongs_to_touch_after_save_or_destroy_for_#{name}"
-      touch       = options[:touch]
+      mixin.class_eval <<-CODE, __FILE__, __LINE__ + 1
+        def belongs_to_touch_after_save_or_destroy_for_#{name}
+          record = #{name}
 
-      mixin.redefine_method(method_name) do
-        record = send(name)
-
-        unless record.nil?
-          if touch == true
-            record.touch
-          else
-            record.touch(touch)
+          unless record.nil?
+            record.touch #{options[:touch].inspect if options[:touch] != true}
           end
         end
-      end
+      CODE
 
-      model.after_save(method_name)
-      model.after_touch(method_name)
-      model.after_destroy(method_name)
+      model.after_save    "belongs_to_touch_after_save_or_destroy_for_#{name}"
+      model.after_touch   "belongs_to_touch_after_save_or_destroy_for_#{name}"
+      model.after_destroy "belongs_to_touch_after_save_or_destroy_for_#{name}"
     end
 
     def valid_dependent_options

activerecord/lib/active_record/associations/builder/collection_association.rb

@@ -65,19 +65,21 @@ module ActiveRecord::Associations::Builder
     def define_readers
       super
 
-      name = self.name
-      mixin.redefine_method("#{name.to_s.singularize}_ids") do
-        association(name).ids_reader
-      end
+      mixin.class_eval <<-CODE, __FILE__, __LINE__ + 1
+        def #{name.to_s.singularize}_ids
+          association(:#{name}).ids_reader
+        end
+      CODE
     end
 
     def define_writers
       super
 
-      name = self.name
-      mixin.redefine_method("#{name.to_s.singularize}_ids=") do |ids|
-        association(name).ids_writer(ids)
-      end
+      mixin.class_eval <<-CODE, __FILE__, __LINE__ + 1
+        def #{name.to_s.singularize}_ids=(ids)
+          association(:#{name}).ids_writer(ids)
+        end
+      CODE
     end
   end
 end

activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb

@@ -29,7 +29,7 @@ module ActiveRecord::Associations::Builder
       model.send(:include, Module.new {
         class_eval <<-RUBY, __FILE__, __LINE__ + 1
           def destroy_associations
-            association(#{name.to_sym.inspect}).delete_all
+            association(:#{name}).delete_all
             super
           end
         RUBY

activerecord/lib/active_record/associations/builder/singular_association.rb

@@ -14,19 +14,19 @@ module ActiveRecord::Associations::Builder
     end
 
     def define_constructors
-      name = self.name
+      mixin.class_eval <<-CODE, __FILE__, __LINE__ + 1
+        def build_#{name}(*args, &block)
+          association(:#{name}).build(*args, &block)
+        end
 
-      mixin.redefine_method("build_#{name}") do |*params, &block|
-        association(name).build(*params, &block)
-      end
+        def create_#{name}(*args, &block)
+          association(:#{name}).create(*args, &block)
+        end
 
-      mixin.redefine_method("create_#{name}") do |*params, &block|
-        association(name).create(*params, &block)
-      end
-
-      mixin.redefine_method("create_#{name}!") do |*params, &block|
-        association(name).create!(*params, &block)
-      end
+        def create_#{name}!(*args, &block)
+          association(:#{name}).create!(*args, &block)
+        end
+      CODE
     end
   end
 end

activerecord/test/cases/associations/belongs_to_associations_test.rb

@@ -524,13 +524,13 @@ class BelongsToAssociationsTest < ActiveRecord::TestCase
 
   def test_invalid_belongs_to_dependent_option_nullify_raises_exception
     assert_raise ArgumentError do
-      Author.belongs_to :special_author_address, :dependent => :nullify
+      Class.new(Author).belongs_to :special_author_address, :dependent => :nullify
     end
   end
 
   def test_invalid_belongs_to_dependent_option_restrict_raises_exception
     assert_raise ArgumentError do
-      Author.belongs_to :special_author_address, :dependent => :restrict
+      Class.new(Author).belongs_to :special_author_address, :dependent => :restrict
     end
   end
 

activerecord/test/cases/associations/join_model_test.rb

@@ -385,7 +385,7 @@ class AssociationsJoinModelTest < ActiveRecord::TestCase
   end
 
   def test_has_many_through_polymorphic_has_one
-    assert_equal Tagging.find(1,2).sort_by { |t| t.id }, authors(:david).tagging
+    assert_equal Tagging.find(1,2).sort_by { |t| t.id }, authors(:david).taggings_2
   end
 
   def test_has_many_through_polymorphic_has_many

activerecord/test/cases/multiple_db_test.rb

@@ -1,9 +1,7 @@
 require "cases/helper"
 require 'models/entrant'
 require 'models/bird'
-
-# So we can test whether Course.connection survives a reload.
-require_dependency 'models/course'
+require 'models/course'
 
 class MultipleDbTest < ActiveRecord::TestCase
   self.use_transactional_fixtures = false

activerecord/test/cases/timestamp_test.rb

@@ -114,9 +114,12 @@ class TimestampTest < ActiveRecord::TestCase
   end
 
   def test_saving_a_record_with_a_belongs_to_that_specifies_touching_a_specific_attribute_the_parent_should_update_that_attribute
-    Pet.belongs_to :owner, :touch => :happy_at
+    klass = Class.new(ActiveRecord::Base) do
+      def self.name; 'Pet'; end
+      belongs_to :owner, :touch => :happy_at
+    end
 
-    pet   = Pet.first
+    pet   = klass.first
     owner = pet.owner
     previously_owner_happy_at = owner.happy_at
 
@@ -124,14 +127,15 @@ class TimestampTest < ActiveRecord::TestCase
     pet.save
 
     assert_not_equal previously_owner_happy_at, pet.owner.happy_at
-  ensure
-    Pet.belongs_to :owner, :touch => true
   end
 
   def test_touching_a_record_with_a_belongs_to_that_uses_a_counter_cache_should_update_the_parent
-    Pet.belongs_to :owner, :counter_cache => :use_count, :touch => true
+    klass = Class.new(ActiveRecord::Base) do
+      def self.name; 'Pet'; end
+      belongs_to :owner, :counter_cache => :use_count, :touch => true
+    end
 
-    pet = Pet.first
+    pet = klass.first
     owner = pet.owner
     owner.update_columns(happy_at: 3.days.ago)
     previously_owner_updated_at = owner.updated_at
@@ -140,15 +144,15 @@ class TimestampTest < ActiveRecord::TestCase
     pet.save
 
     assert_not_equal previously_owner_updated_at, pet.owner.updated_at
-  ensure
-    Pet.belongs_to :owner, :touch => true
   end
 
   def test_touching_a_record_touches_parent_record_and_grandparent_record
-    Toy.belongs_to :pet, :touch => true
-    Pet.belongs_to :owner, :touch => true
+    klass = Class.new(ActiveRecord::Base) do
+      def self.name; 'Toy'; end
+      belongs_to :pet, :touch => true
+    end
 
-    toy = Toy.first
+    toy = klass.first
     pet = toy.pet
     owner = pet.owner
     time = 3.days.ago
@@ -158,8 +162,6 @@ class TimestampTest < ActiveRecord::TestCase
     owner.reload
 
     assert_not_equal time, owner.updated_at
-  ensure
-    Toy.belongs_to :pet
   end
 
   def test_timestamp_attributes_for_create

activerecord/test/models/author.rb

@@ -93,8 +93,8 @@ class Author < ActiveRecord::Base
   has_many :author_favorites
   has_many :favorite_authors, -> { order('name') }, :through => :author_favorites
 
-  has_many :tagging,         :through => :posts
   has_many :taggings,        :through => :posts
+  has_many :taggings_2,      :through => :posts, :source => :tagging
   has_many :tags,            :through => :posts
   has_many :post_categories, :through => :posts, :source => :categories
   has_many :tagging_tags,    :through => :taggings, :source => :tag

activerecord/test/models/member.rb

@@ -24,11 +24,10 @@ class Member < ActiveRecord::Base
 
   has_one :club_category, :through => :club, :source => :category
 
-  has_many :current_memberships
-  has_one :club_through_many, :through => :current_memberships, :source => :club
-
   has_many :current_memberships, -> { where :favourite => true }
   has_many :clubs, :through => :current_memberships
+
+  has_one :club_through_many, :through => :current_memberships, :source => :club
 end
 
 class SelfMember < ActiveRecord::Base

activerecord/test/support/connection.rb

@@ -1,6 +1,6 @@
 require 'active_support/logger'
-require_dependency 'models/college'
-require_dependency 'models/course'
+require 'models/college'
+require 'models/course'
 
 module ARTest
   def self.connection_name