Merge pull request #3636 from joshsusser/master

association methods are now generated in modules

activerecord/CHANGELOG.md

@@ -1,5 +1,11 @@
 ## Rails 3.2.0 (unreleased) ##
 
+*   Generated association methods are created within a separate module to allow overriding and
+    composition using `super`. For a class named `MyModel`, the module is named
+    `MyModel::GeneratedFeatureMethods`. It is included into the model class immediately after
+    the `generated_attributes_methods` module defined in ActiveModel, so association methods
+    override attribute methods of the same name. *Josh Susser*
+
 *   Implemented ActiveRecord::Relation#explain. *fxn*
 
 *   Add ActiveRecord::Relation#uniq for generating unique queries.

activerecord/lib/active_record/associations.rb

@@ -196,6 +196,26 @@ def association_instance_set(name, association)
     # * <tt>Project#categories.empty?, Project#categories.size, Project#categories, Project#categories<<(category1),</tt>
     #   <tt>Project#categories.delete(category1)</tt>
     #
+    # === Overriding generated methods
+    #
+    # Association methods are generated in a module that is included into the model class,
+    # which allows you to easily override with your own methods and call the original
+    # generated method with +super+. For example:
+    #
+    #   class Car < ActiveRecord::Base
+    #     belongs_to :owner
+    #     belongs_to :old_owner
+    #     def owner=(new_owner)
+    #       self.old_owner = self.owner
+    #       super
+    #     end
+    #   end
+    #
+    # If your model class is <tt>Project</tt>, the module is
+    # named <tt>Project::GeneratedFeatureMethods</tt>. The GeneratedFeatureMethods module is
+    # is included in the model class immediately after the (anonymous) generated attributes methods
+    # module, meaning an association will override the methods for an attribute with the same name.
+    #
     # === A word of warning
     #
     # Don't create associations that have the same name as instance methods of

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

@@ -16,6 +16,10 @@ def initialize(model, name, options)
       @model, @name, @options = model, name, options
     end
 
+    def mixin
+      @model.generated_feature_methods
+    end
+
     def build
       validate_options
       reflection = model.create_reflection(self.class.macro, name, options, model)
@@ -36,16 +40,14 @@ def define_accessors
 
       def define_readers
         name = self.name
-
-        model.redefine_method(name) do |*params|
+        mixin.redefine_method(name) do |*params|
           association(name).reader(*params)
         end
       end
 
       def define_writers
         name = self.name
-
-        model.redefine_method("#{name}=") do |value|
+        mixin.redefine_method("#{name}=") do |value|
           association(name).writer(value)
         end
       end

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

@@ -25,14 +25,14 @@ def add_counter_cache_callbacks(reflection)
         name         = self.name
 
         method_name = "belongs_to_counter_cache_after_create_for_#{name}"
-        model.redefine_method(method_name) do
+        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}"
-        model.redefine_method(method_name) do
+        mixin.redefine_method(method_name) do
           record = send(name)
           record.class.decrement_counter(cache_column, record.id) unless record.nil?
         end
@@ -48,7 +48,7 @@ def add_touch_callbacks(reflection)
         method_name = "belongs_to_touch_after_save_or_destroy_for_#{name}"
         touch       = options[:touch]
 
-        model.redefine_method(method_name) do
+        mixin.redefine_method(method_name) do
           record = send(name)
 
           unless record.nil?

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

@@ -58,7 +58,7 @@ def define_readers
         super
 
         name = self.name
-        model.redefine_method("#{name.to_s.singularize}_ids") do
+        mixin.redefine_method("#{name.to_s.singularize}_ids") do
           association(name).ids_reader
         end
       end
@@ -67,7 +67,7 @@ def define_writers
         super
 
         name = self.name
-        model.redefine_method("#{name.to_s.singularize}_ids=") do |ids|
+        mixin.redefine_method("#{name.to_s.singularize}_ids=") do |ids|
           association(name).ids_writer(ids)
         end
       end

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

@@ -28,7 +28,7 @@ def configure_dependency
 
       def define_destroy_dependency_method
         name = self.name
-        model.send(:define_method, dependency_method_name) do
+        mixin.redefine_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,15 +45,15 @@ class << o
 
       def define_delete_all_dependency_method
         name = self.name
-        model.send(:define_method, dependency_method_name) do
+        mixin.redefine_method(dependency_method_name) do
           send(name).delete_all
         end
       end
       alias :define_nullify_dependency_method :define_delete_all_dependency_method
 
       def define_restrict_dependency_method
         name = self.name
-        model.send(:define_method, dependency_method_name) do
+        mixin.redefine_method(dependency_method_name) do
           raise ActiveRecord::DeleteRestrictionError.new(name) unless send(name).empty?
         end
       end

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

@@ -44,18 +44,17 @@ def dependency_method_name
       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.redefine_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.redefine_method(dependency_method_name) do
           raise ActiveRecord::DeleteRestrictionError.new(name) unless send(name).nil?
         end
       end

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

@@ -16,15 +16,15 @@ def define_accessors
       def define_constructors
         name = self.name
 
-        model.redefine_method("build_#{name}") do |*params, &block|
+        mixin.redefine_method("build_#{name}") do |*params, &block|
           association(name).build(*params, &block)
         end
 
-        model.redefine_method("create_#{name}") do |*params, &block|
+        mixin.redefine_method("create_#{name}") do |*params, &block|
           association(name).create(*params, &block)
         end
 
-        model.redefine_method("create_#{name}!") do |*params, &block|
+        mixin.redefine_method("create_#{name}!") do |*params, &block|
           association(name).create!(*params, &block)
         end
       end

activerecord/lib/active_record/base.rb

@@ -450,6 +450,20 @@ class << self # Class methods
                :having, :create_with, :uniq, :to => :scoped
       delegate :count, :average, :minimum, :maximum, :sum, :calculate, :to => :scoped
 
+      def inherited(child_class) #:nodoc:
+        # force attribute methods to be higher in inheritance hierarchy than other generated methods
+        child_class.generated_attribute_methods
+        child_class.generated_feature_methods
+        super
+      end
+
+      def generated_feature_methods
+        unless const_defined?(:GeneratedFeatureMethods, false)
+          include const_set(:GeneratedFeatureMethods, Module.new)
+        end
+        const_get(:GeneratedFeatureMethods)
+      end
+
       # Executes a custom SQL query against your database and returns all the results. The results will
       # be returned as an array with columns requested encapsulated as attributes of the model you call
       # this method from. If you call <tt>Product.find_by_sql</tt> then the results will be returned in

activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb

@@ -77,7 +77,7 @@ class DeveloperWithCounterSQL < ActiveRecord::Base
 
 class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase
   fixtures :accounts, :companies, :categories, :posts, :categories_posts, :developers, :projects, :developers_projects,
-           :parrots, :pirates, :treasures, :price_estimates, :tags, :taggings
+           :parrots, :pirates, :parrots_pirates, :treasures, :price_estimates, :tags, :taggings
 
   def setup_data_for_habtm_case
     ActiveRecord::Base.connection.execute('delete from countries_treaties')
@@ -445,6 +445,26 @@ def test_destroy_all
     assert david.projects(true).empty?
   end
 
+  def test_destroy_associations_destroys_multiple_associations
+    george = parrots(:george)
+    assert !george.pirates.empty?
+    assert !george.treasures.empty?
+
+    assert_no_difference "Pirate.count" do
+      assert_no_difference "Treasure.count" do
+        george.destroy_associations
+      end
+    end
+
+    join_records = Parrot.connection.select_all("SELECT * FROM parrots_pirates WHERE parrot_id = #{george.id}")
+    assert join_records.empty?
+    assert george.pirates(true).empty?
+
+    join_records = Parrot.connection.select_all("SELECT * FROM parrots_treasures WHERE parrot_id = #{george.id}")
+    assert join_records.empty?
+    assert george.treasures(true).empty?
+  end
+
   def test_deprecated_push_with_attributes_was_removed
     jamis = developers(:jamis)
     assert_raise(NoMethodError) do

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,18 @@ def test_has_one_association_redefinition_reflections_should_differ_and_not_inhe
     )
   end
 end
+
+class GeneratedMethodsTest < ActiveRecord::TestCase
+  fixtures :developers, :computers, :posts, :comments
+  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
+
+  def test_model_method_overrides_association_method
+    assert_equal(comments(:greetings).body, posts(:welcome).first_comment)
+  end
+end

activerecord/test/cases/base_test.rb

@@ -69,6 +69,15 @@ def setup
 class BasicsTest < ActiveRecord::TestCase
   fixtures :topics, :companies, :developers, :projects, :computers, :accounts, :minimalistics, 'warehouse-things', :authors, :categorizations, :categories, :posts
 
+  def test_generated_methods_modules
+    modules = Computer.ancestors
+    assert modules.include?(Computer::GeneratedFeatureMethods)
+    assert_equal(Computer::GeneratedFeatureMethods, Computer.generated_feature_methods)
+    assert(modules.index(Computer.generated_attribute_methods) > modules.index(Computer.generated_feature_methods),
+           "generated_attribute_methods must be higher in inheritance hierarchy than generated_feature_methods")
+    assert_not_equal Computer.generated_feature_methods, Post.generated_feature_methods
+  end
+
   def test_column_names_are_escaped
     conn      = ActiveRecord::Base.connection
     classname = conn.class.name[/[^:]*$/]

activerecord/test/models/author.rb

@@ -128,7 +128,6 @@ def testing_proxy_target
   belongs_to :author_address,       :dependent => :destroy
   belongs_to :author_address_extra, :dependent => :delete, :class_name => "AuthorAddress"
 
-  has_many :post_categories, :through => :posts, :source => :categories
   has_many :category_post_comments, :through => :categories, :source => :post_comments
 
   has_many :misc_posts, :class_name => 'Post',

activerecord/test/models/post.rb

@@ -24,6 +24,10 @@ def greeting
   belongs_to :author_with_posts, :class_name => "Author", :foreign_key => :author_id, :include => :posts
   belongs_to :author_with_address, :class_name => "Author", :foreign_key => :author_id, :include => :author_address
 
+  def first_comment
+    super.body
+  end
+  has_one :first_comment, :class_name => 'Comment', :order => 'id ASC'
   has_one :last_comment, :class_name => 'Comment', :order => 'id desc'
 
   scope :with_special_comments, :joins => :comments, :conditions => {:comments => {:type => 'SpecialComment'} }