aboutsummaryrefslogtreecommitdiffstats
path: root/activerecord
diff options
context:
space:
mode:
authorRyuta Kamizono <kamipo@gmail.com>2018-10-08 16:45:05 +0900
committerRyuta Kamizono <kamipo@gmail.com>2018-10-09 13:03:08 +0900
commit136b738cd261a7f54f478f9fb160afb9f5e50a02 (patch)
treebc8d267763c061625939810c32d5abc3d38fcbbb /activerecord
parenta1ee4a9ff9d4a3cb255365310ead0dc7b739c6be (diff)
downloadrails-136b738cd261a7f54f478f9fb160afb9f5e50a02.tar.gz
rails-136b738cd261a7f54f478f9fb160afb9f5e50a02.tar.bz2
rails-136b738cd261a7f54f478f9fb160afb9f5e50a02.zip
Generate delegation methods to named scope in the definition time
The delegation methods to named scope are defined when `method_missing` is invoked on the relation. Since #29301, the receiver in the named scope is changed to the relation like others (e.g. `default_scope`, etc) for consistency. Most named scopes would be delegated from relation by `method_missing`, since we don't allow scopes to be defined which conflict with instance methods on `Relation` (#31179). But if a named scope is defined with the same name as any method on the `superclass` (e.g. `Kernel.open`), the `method_missing` on the relation is not invoked. To address the issue, make the delegation methods to named scope is generated in the definition time. Fixes #34098.
Diffstat (limited to 'activerecord')
-rw-r--r--activerecord/lib/active_record/relation/delegation.rb30
-rw-r--r--activerecord/lib/active_record/scoping/named.rb2
-rw-r--r--activerecord/test/cases/associations/has_many_associations_test.rb1
-rw-r--r--activerecord/test/cases/relations_test.rb10
-rw-r--r--activerecord/test/models/account.rb13
-rw-r--r--activerecord/test/models/post.rb8
6 files changed, 59 insertions, 5 deletions
diff --git a/activerecord/lib/active_record/relation/delegation.rb b/activerecord/lib/active_record/relation/delegation.rb
index 8f657840f5..383dc1bf4b 100644
--- a/activerecord/lib/active_record/relation/delegation.rb
+++ b/activerecord/lib/active_record/relation/delegation.rb
@@ -17,6 +17,7 @@ module ActiveRecord
delegate = Class.new(klass) {
include ClassSpecificRelation
}
+ include_relation_methods(delegate)
mangled_name = klass.name.gsub("::", "_")
const_set mangled_name, delegate
private_constant mangled_name
@@ -29,6 +30,35 @@ module ActiveRecord
child_class.initialize_relation_delegate_cache
super
end
+
+ protected
+ def include_relation_methods(delegate)
+ superclass.include_relation_methods(delegate) unless base_class?
+ delegate.include generated_relation_methods
+ end
+
+ private
+ def generated_relation_methods
+ @generated_relation_methods ||= Module.new.tap do |mod|
+ mod_name = "GeneratedRelationMethods"
+ const_set mod_name, mod
+ private_constant mod_name
+ end
+ end
+
+ def generate_relation_method(method)
+ if /\A[a-zA-Z_]\w*[!?]?\z/.match?(method)
+ generated_relation_methods.module_eval <<-RUBY, __FILE__, __LINE__ + 1
+ def #{method}(*args, &block)
+ scoping { klass.#{method}(*args, &block) }
+ end
+ RUBY
+ else
+ generated_relation_methods.send(:define_method, method) do |*args, &block|
+ scoping { klass.public_send(method, *args, &block) }
+ end
+ end
+ end
end
extend ActiveSupport::Concern
diff --git a/activerecord/lib/active_record/scoping/named.rb b/activerecord/lib/active_record/scoping/named.rb
index 573d97b819..d5cc5db97e 100644
--- a/activerecord/lib/active_record/scoping/named.rb
+++ b/activerecord/lib/active_record/scoping/named.rb
@@ -191,6 +191,8 @@ module ActiveRecord
scope
end
end
+
+ generate_relation_method(name)
end
private
diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb
index eb65f9e74f..2d16815da8 100644
--- a/activerecord/test/cases/associations/has_many_associations_test.rb
+++ b/activerecord/test/cases/associations/has_many_associations_test.rb
@@ -2391,6 +2391,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
def test_collection_association_with_private_kernel_method
firm = companies(:first_firm)
assert_equal [accounts(:signals37)], firm.accounts.open
+ assert_equal [accounts(:signals37)], firm.accounts.available
end
def test_association_with_or_doesnt_set_inverse_instance_key
diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb
index 9914a61033..e471ee8039 100644
--- a/activerecord/test/cases/relations_test.rb
+++ b/activerecord/test/cases/relations_test.rb
@@ -1812,6 +1812,16 @@ class RelationTest < ActiveRecord::TestCase
assert_equal "Thank you for the welcome,Thank you again for the welcome", Post.first.comments.join(",")
end
+ def test_relation_with_private_kernel_method
+ accounts = Account.all
+ assert_equal [accounts(:signals37)], accounts.open
+ assert_equal [accounts(:signals37)], accounts.available
+
+ sub_accounts = SubAccount.all
+ assert_equal [accounts(:signals37)], sub_accounts.open
+ assert_equal [accounts(:signals37)], sub_accounts.available
+ end
+
test "#skip_query_cache!" do
Post.cache do
assert_queries(1) do
diff --git a/activerecord/test/models/account.rb b/activerecord/test/models/account.rb
index 0c3cd45a81..639e395743 100644
--- a/activerecord/test/models/account.rb
+++ b/activerecord/test/models/account.rb
@@ -11,9 +11,8 @@ class Account < ActiveRecord::Base
end
# Test private kernel method through collection proxy using has_many.
- def self.open
- where("firm_name = ?", "37signals")
- end
+ scope :open, -> { where("firm_name = ?", "37signals") }
+ scope :available, -> { open }
before_destroy do |account|
if account.firm
@@ -32,3 +31,11 @@ class Account < ActiveRecord::Base
"Sir, yes sir!"
end
end
+
+class SubAccount < Account
+ def self.instantiate_instance_of(klass, attributes, column_types = {}, &block)
+ klass = superclass
+ super
+ end
+ private_class_method :instantiate_instance_of
+end
diff --git a/activerecord/test/models/post.rb b/activerecord/test/models/post.rb
index 528585fb75..710a75ad30 100644
--- a/activerecord/test/models/post.rb
+++ b/activerecord/test/models/post.rb
@@ -297,8 +297,6 @@ end
class FakeKlass
extend ActiveRecord::Delegation::DelegateCache
- inherited self
-
class << self
def connection
Post.connection
@@ -335,5 +333,11 @@ class FakeKlass
def predicate_builder
Post.predicate_builder
end
+
+ def base_class?
+ true
+ end
end
+
+ inherited self
end