association methods are now generated in modules

Instead of generating association methods directly in the model
class, they are generated in an anonymous module which
is then included in the model class. There is one such module
for each association. The only subtlety is that the
generated_attributes_methods module (from ActiveModel) must
be forced to be included before association methods are created
so that attribute methods will not shadow association methods.

9 files changed, 43 insertions, 30 deletions

diff --git a/activerecord/lib/active_record/associations/builder/association.rb b/activerecord/lib/active_record/associations/builder/association.rb
index 96fca97440..686db0725d 100644
--- a/activerecord/lib/active_record/associations/builder/association.rb
+++ b/activerecord/lib/active_record/associations/builder/association.rb
@@ -6,7 +6,7 @@ module ActiveRecord::Associations::Builder
     # Set by subclasses
     class_attribute :macro
 
-    attr_reader :model, :name, :options, :reflection
+    attr_reader :model, :name, :options, :reflection, :mixin
 
     def self.build(model, name, options)
       new(model, name, options).build
@@ -14,6 +14,8 @@ module ActiveRecord::Associations::Builder
 
     def initialize(model, name, options)
       @model, @name, @options = model, name, options
+      @mixin = Module.new
+      @model.__send__(:include, @mixin)
     end
 
     def build
@@ -36,16 +38,14 @@ module ActiveRecord::Associations::Builder
 
       def define_readers
         name = self.name
-
-        model.redefine_method(name) do |*params|
+        mixin.send(:define_method, name) do |*params|
           association(name).reader(*params)
         end
       end
 
       def define_writers
         name = self.name
-
-        model.redefine_method("#{name}=") do |value|
+        mixin.send(:define_method, "#{name}=") do |value|
           association(name).writer(value)
         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 f6d26840c2..0ca107035f 100644
--- a/activerecord/lib/active_record/associations/builder/belongs_to.rb
+++ b/activerecord/lib/active_record/associations/builder/belongs_to.rb
@@ -25,14 +25,14 @@ module ActiveRecord::Associations::Builder
         name         = self.name
 
         method_name = "belongs_to_counter_cache_after_create_for_#{name}"
-        model.redefine_method(method_name) do
+        mixin.send(:define_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}"
-        model.redefine_method(method_name) do
+        mixin.send(:define_method, method_name) do
           record = send(name)
           record.class.decrement_counter(cache_column, record.id) unless record.nil?
         end
@@ -48,7 +48,7 @@ module ActiveRecord::Associations::Builder
         method_name = "belongs_to_touch_after_save_or_destroy_for_#{name}"
         touch       = options[:touch]
 
-        model.redefine_method(method_name) do
+        mixin.send(:define_method, method_name) do
           record = send(name)
 
           unless record.nil?
diff --git a/activerecord/lib/active_record/associations/builder/collection_association.rb b/activerecord/lib/active_record/associations/builder/collection_association.rb
index f62209a226..1805ea2c1e 100644
--- a/activerecord/lib/active_record/associations/builder/collection_association.rb
+++ b/activerecord/lib/active_record/associations/builder/collection_association.rb
@@ -58,7 +58,7 @@ module ActiveRecord::Associations::Builder
         super
 
         name = self.name
-        model.redefine_method("#{name.to_s.singularize}_ids") do
+        mixin.send(:define_method, "#{name.to_s.singularize}_ids") do
           association(name).ids_reader
         end
       end
@@ -67,7 +67,7 @@ module ActiveRecord::Associations::Builder
         super
 
         name = self.name
-        model.redefine_method("#{name.to_s.singularize}_ids=") do |ids|
+        mixin.send(:define_method, "#{name.to_s.singularize}_ids=") do |ids|
           association(name).ids_writer(ids)
         end
       end
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 30fc44b4c2..f3391eba13 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
@@ -15,14 +15,10 @@ module ActiveRecord::Associations::Builder
 
       def define_destroy_hook
         name = self.name
-        model.send(:include, Module.new {
-          class_eval <<-RUBY, __FILE__, __LINE__ + 1
-            def destroy_associations
-              association(#{name.to_sym.inspect}).delete_all
-              super
-            end
-          RUBY
-        })
+        mixin.send(:define_method, :destroy_associations) do
+          association(name).delete_all
+          super()
+        end
       end
 
       # TODO: These checks should probably be moved into the Reflection, and we should not be
diff --git a/activerecord/lib/active_record/associations/builder/has_many.rb b/activerecord/lib/active_record/associations/builder/has_many.rb
index ecbc70888f..8a6f5a87e7 100644
--- a/activerecord/lib/active_record/associations/builder/has_many.rb
+++ b/activerecord/lib/active_record/associations/builder/has_many.rb
@@ -28,7 +28,7 @@ module ActiveRecord::Associations::Builder
 
       def define_destroy_dependency_method
         name = self.name
-        model.send(:define_method, dependency_method_name) do
+        mixin.send(:define_method, dependency_method_name) do
           send(name).each do |o|
             # No point in executing the counter update since we're going to destroy the parent anyway
             counter_method = ('belongs_to_counter_cache_before_destroy_for_' + self.class.name.downcase).to_sym
@@ -45,7 +45,7 @@ module ActiveRecord::Associations::Builder
 
       def define_delete_all_dependency_method
         name = self.name
-        model.send(:define_method, dependency_method_name) do
+        mixin.send(:define_method, dependency_method_name) do
           send(name).delete_all
         end
       end
@@ -53,7 +53,7 @@ module ActiveRecord::Associations::Builder
 
       def define_restrict_dependency_method
         name = self.name
-        model.send(:define_method, dependency_method_name) do
+        mixin.send(:define_method, dependency_method_name) do
           raise ActiveRecord::DeleteRestrictionError.new(name) unless send(name).empty?
         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 88c0d3e90f..2cea8b9805 100644
--- a/activerecord/lib/active_record/associations/builder/has_one.rb
+++ b/activerecord/lib/active_record/associations/builder/has_one.rb
@@ -44,18 +44,17 @@ module ActiveRecord::Associations::Builder
       end
 
       def define_destroy_dependency_method
-        model.send(:class_eval, <<-eoruby, __FILE__, __LINE__ + 1)
-          def #{dependency_method_name}
-            association(#{name.to_sym.inspect}).delete
-          end
-        eoruby
+        name = self.name
+        mixin.send(:define_method, dependency_method_name) do
+          association(name).delete
+        end
       end
       alias :define_delete_dependency_method :define_destroy_dependency_method
       alias :define_nullify_dependency_method :define_destroy_dependency_method
 
       def define_restrict_dependency_method
         name = self.name
-        model.redefine_method(dependency_method_name) do
+        mixin.send(:define_method, dependency_method_name) do
           raise ActiveRecord::DeleteRestrictionError.new(name) unless send(name).nil?
         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 0cbbba041a..020e9157b3 100644
--- a/activerecord/lib/active_record/associations/builder/singular_association.rb
+++ b/activerecord/lib/active_record/associations/builder/singular_association.rb
@@ -16,15 +16,15 @@ module ActiveRecord::Associations::Builder
       def define_constructors
         name = self.name
 
-        model.redefine_method("build_#{name}") do |*params, &block|
+        mixin.send(:define_method, "build_#{name}") do |*params, &block|
           association(name).build(*params, &block)
         end
 
-        model.redefine_method("create_#{name}") do |*params, &block|
+        mixin.send(:define_method, "create_#{name}") do |*params, &block|
           association(name).create(*params, &block)
         end
 
-        model.redefine_method("create_#{name}!") do |*params, &block|
+        mixin.send(:define_method, "create_#{name}!") do |*params, &block|
           association(name).create!(*params, &block)
         end
       end
diff --git a/activerecord/lib/active_record/attribute_methods.rb b/activerecord/lib/active_record/attribute_methods.rb
index d7bfaa5655..2d720fe700 100644
--- a/activerecord/lib/active_record/attribute_methods.rb
+++ b/activerecord/lib/active_record/attribute_methods.rb
@@ -8,6 +8,12 @@ module ActiveRecord
     include ActiveModel::AttributeMethods
 
     module ClassMethods
+      def inherited(child_class)
+        # force creation + include before accessor method modules
+        child_class.generated_attribute_methods
+        super
+      end
+
       # Generates all the attribute related methods for columns in the database
       # accessors, mutators and query methods.
       def define_attribute_methods
diff --git a/activerecord/test/cases/associations_test.rb b/activerecord/test/cases/associations_test.rb
index ffe2993e0f..a9094b7a8b 100644
--- a/activerecord/test/cases/associations_test.rb
+++ b/activerecord/test/cases/associations_test.rb
@@ -1,4 +1,5 @@
 require "cases/helper"
+require 'models/computer'
 require 'models/developer'
 require 'models/project'
 require 'models/company'
@@ -273,3 +274,14 @@ class OverridingAssociationsTest < ActiveRecord::TestCase
     )
   end
 end
+
+class GeneratedMethodsTest < ActiveRecord::TestCase
+  fixtures :developers, :computers
+  def test_association_methods_override_attribute_methods_of_same_name
+    assert_equal(developers(:david), computers(:workstation).developer)
+    # this next line will fail if the attribute methods module is generated lazily
+    # after the association methods module is generated
+    assert_equal(developers(:david), computers(:workstation).developer)
+    assert_equal(developers(:david).id, computers(:workstation)[:developer])
+  end
+end
\ No newline at end of file