association methods are now generated in modules #3636

Merged
merged 6 commits into from Nov 29, 2011
+112 −23
Split
@@ -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.
@@ -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
@@ -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
@@ -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?
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
josevalim
josevalim Nov 28, 2011 Member

Do we set this constant in order to have a readable output on Model.ancestors?

josevalim
josevalim Nov 28, 2011 Member

If I am not wrong, we were used to do the same thing for generated_attribute_methods and it caused memory leaks in development. I will try to find the relevant commits.

joshsusser
joshsusser Nov 28, 2011 Contributor

Yes, the module gets named MyModel::GeneratedFeatureMethods. But ActiveModel doesn't name the attributes module.

+ 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
@@ -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
@@ -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
@@ -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[/[^:]*$/]
@@ -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',
@@ -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'} }