diff options
author | Ryuta Kamizono <kamipo@gmail.com> | 2018-10-08 16:45:05 +0900 |
---|---|---|
committer | Ryuta Kamizono <kamipo@gmail.com> | 2018-10-09 13:03:08 +0900 |
commit | 136b738cd261a7f54f478f9fb160afb9f5e50a02 (patch) | |
tree | bc8d267763c061625939810c32d5abc3d38fcbbb | |
parent | a1ee4a9ff9d4a3cb255365310ead0dc7b739c6be (diff) | |
download | rails-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.
-rw-r--r-- | activerecord/lib/active_record/relation/delegation.rb | 30 | ||||
-rw-r--r-- | activerecord/lib/active_record/scoping/named.rb | 2 | ||||
-rw-r--r-- | activerecord/test/cases/associations/has_many_associations_test.rb | 1 | ||||
-rw-r--r-- | activerecord/test/cases/relations_test.rb | 10 | ||||
-rw-r--r-- | activerecord/test/models/account.rb | 13 | ||||
-rw-r--r-- | activerecord/test/models/post.rb | 8 |
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 |