Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Comparing changes

Choose two branches to see what's changed or to start a new pull request. If you need to, you can also compare across forks.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also compare across forks.
base fork: joshsusser/rails
base: 9cdf33af0b
...
head fork: joshsusser/rails
compare: 124c97fbe2
Checking mergeability… Don't worry, you can still create the pull request.
  • 3 commits
  • 14 files changed
  • 0 commit comments
  • 1 contributor
Commits on Nov 27, 2011
@joshsusser use GeneratedFeatureMethods module for associations 61bcc31
@joshsusser changelog & docs for GeneratedFeatureMethods 10834e9
@joshsusser avoid warnings
This change uses Module.redefine_method as defined in ActiveSupport.
Making Module.define_method public would be as clean in the code, and
would also emit warnings when redefining an association. That is pretty
messy given current tests, so I'm leaving it for someone else to decide
what approach is better.
124c97f
View
6 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.
View
20 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
View
12 activerecord/lib/active_record/associations/builder/association.rb
@@ -6,7 +6,7 @@ class Association #:nodoc:
# Set by subclasses
class_attribute :macro
- attr_reader :model, :name, :options, :reflection, :mixin
+ attr_reader :model, :name, :options, :reflection
def self.build(model, name, options)
new(model, name, options).build
@@ -14,8 +14,10 @@ def self.build(model, name, options)
def initialize(model, name, options)
@model, @name, @options = model, name, options
- @mixin = Module.new
- @model.__send__(:include, @mixin)
+ end
+
+ def mixin
+ @model.generated_feature_methods
end
def build
@@ -38,14 +40,14 @@ def define_accessors
def define_readers
name = self.name
- mixin.send(:define_method, name) do |*params|
+ mixin.redefine_method(name) do |*params|
association(name).reader(*params)
end
end
def define_writers
name = self.name
- mixin.send(:define_method, "#{name}=") do |value|
+ mixin.redefine_method("#{name}=") do |value|
association(name).writer(value)
end
end
View
6 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}"
- mixin.send(:define_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}"
- mixin.send(:define_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]
- mixin.send(:define_method, method_name) do
+ mixin.redefine_method(method_name) do
record = send(name)
unless record.nil?
View
4 activerecord/lib/active_record/associations/builder/collection_association.rb
@@ -58,7 +58,7 @@ def define_readers
super
name = self.name
- mixin.send(:define_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
- mixin.send(:define_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
View
12 activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb
@@ -15,10 +15,14 @@ def build
def define_destroy_hook
name = self.name
- mixin.send(:define_method, :destroy_associations) do
- association(name).delete_all
- super()
- end
+ model.send(:include, Module.new {
+ class_eval <<-RUBY, __FILE__, __LINE__ + 1
+ def destroy_associations
+ association(#{name.to_sym.inspect}).delete_all
+ super
+ end
+ RUBY
+ })
end
# TODO: These checks should probably be moved into the Reflection, and we should not be
View
6 activerecord/lib/active_record/associations/builder/has_many.rb
@@ -28,7 +28,7 @@ def configure_dependency
def define_destroy_dependency_method
name = self.name
- mixin.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,7 +45,7 @@ class << o
def define_delete_all_dependency_method
name = self.name
- mixin.send(:define_method, dependency_method_name) do
+ mixin.redefine_method(dependency_method_name) do
send(name).delete_all
end
end
@@ -53,7 +53,7 @@ def define_delete_all_dependency_method
def define_restrict_dependency_method
name = self.name
- mixin.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
View
4 activerecord/lib/active_record/associations/builder/has_one.rb
@@ -45,7 +45,7 @@ def dependency_method_name
def define_destroy_dependency_method
name = self.name
- mixin.send(:define_method, dependency_method_name) do
+ mixin.redefine_method(dependency_method_name) do
association(name).delete
end
end
@@ -54,7 +54,7 @@ def define_destroy_dependency_method
def define_restrict_dependency_method
name = self.name
- mixin.send(:define_method, dependency_method_name) do
+ mixin.redefine_method(dependency_method_name) do
raise ActiveRecord::DeleteRestrictionError.new(name) unless send(name).nil?
end
end
View
6 activerecord/lib/active_record/associations/builder/singular_association.rb
@@ -16,15 +16,15 @@ def define_accessors
def define_constructors
name = self.name
- mixin.send(:define_method, "build_#{name}") do |*params, &block|
+ mixin.redefine_method("build_#{name}") do |*params, &block|
association(name).build(*params, &block)
end
- mixin.send(:define_method, "create_#{name}") do |*params, &block|
+ mixin.redefine_method("create_#{name}") do |*params, &block|
association(name).create(*params, &block)
end
- mixin.send(:define_method, "create_#{name}!") do |*params, &block|
+ mixin.redefine_method("create_#{name}!") do |*params, &block|
association(name).create!(*params, &block)
end
end
View
6 activerecord/lib/active_record/attribute_methods.rb
@@ -8,12 +8,6 @@ module AttributeMethods #:nodoc:
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
View
14 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
View
22 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
View
9 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[/[^:]*$/]
View
1  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',

No commit comments for this range

Something went wrong with that request. Please try again.