From d1e0f11b49c9c7598efffbc44a94539b6e982e5c Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Mon, 18 Jul 2016 22:11:35 +0900 Subject: Delegate to `scope` rather than `merge!` for collection proxy `merge! association.scope(nullify: false)` is expensive but most methods do not need the merge. --- .../associations/collection_association.rb | 6 ++-- .../active_record/associations/collection_proxy.rb | 32 ++++++++++++++-------- 2 files changed, 24 insertions(+), 14 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb index 46923f690a..f747491a12 100644 --- a/activerecord/lib/active_record/associations/collection_association.rb +++ b/activerecord/lib/active_record/associations/collection_association.rb @@ -313,9 +313,9 @@ module ActiveRecord record end - def scope(opts = {}) - scope = super() - scope.none! if opts.fetch(:nullify, true) && null_scope? + def scope + scope = super + scope.none! if null_scope? scope end diff --git a/activerecord/lib/active_record/associations/collection_proxy.rb b/activerecord/lib/active_record/associations/collection_proxy.rb index 9f91f2b536..63f746688c 100644 --- a/activerecord/lib/active_record/associations/collection_proxy.rb +++ b/activerecord/lib/active_record/associations/collection_proxy.rb @@ -28,12 +28,9 @@ module ActiveRecord # is computed directly through SQL and does not trigger by itself the # instantiation of the actual post records. class CollectionProxy < Relation - delegate :exists?, :update_all, :arel, to: :scope - def initialize(klass, association) #:nodoc: @association = association super klass, klass.arel_table, klass.predicate_builder - merge! association.scope(nullify: false) end def target @@ -956,14 +953,6 @@ module ActiveRecord @association end - # We don't want this object to be put on the scoping stack, because - # that could create an infinite loop where we call an @association - # method, which gets the current scope, which is this object, which - # delegates to @association, and so on. - def scoping - @association.scope.scoping { yield } - end - # Returns a Relation object for the records in this association def scope @association.scope @@ -1126,6 +1115,19 @@ module ActiveRecord self end + def respond_to?(name, include_private = false) + super || scope.respond_to?(name, include_private) + end + + delegate_methods = [ + QueryMethods, + SpawnMethods, + ].flat_map { |klass| + klass.public_instance_methods(false) + } - self.public_instance_methods(false) + [:scoping] + + delegate(*delegate_methods, to: :scope) + private def find_nth_with_limit(index, limit) # :doc: @@ -1149,6 +1151,14 @@ module ActiveRecord def exec_queries load_target end + + def method_missing(method, *args, &block) + if scope.respond_to?(method) + scope.public_send(method, *args, &block) + else + super + end + end end end end -- cgit v1.2.3 From f1217c605de2b089fd4fccb14c4d98b41a532c60 Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Fri, 22 Jul 2016 13:03:41 +0900 Subject: No need to cache collection proxies separately Because merging the association scope was removed. --- .../lib/active_record/associations/collection_association.rb | 8 +------- .../test/cases/associations/has_many_associations_test.rb | 5 ----- activerecord/test/cases/associations_test.rb | 5 ----- 3 files changed, 1 insertion(+), 17 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb index f747491a12..7d43a984d1 100644 --- a/activerecord/lib/active_record/associations/collection_association.rb +++ b/activerecord/lib/active_record/associations/collection_association.rb @@ -38,13 +38,7 @@ module ActiveRecord reload end - if null_scope? - # Cache the proxy separately before the owner has an id - # or else a post-save proxy will still lack the id - @null_proxy ||= CollectionProxy.create(klass, self) - else - @proxy ||= CollectionProxy.create(klass, self) - end + CollectionProxy.create(klass, self) end # Implements the writer method, e.g. foo.items= for Foo.has_many :items diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index 558c4b3ed1..ff008dd3b2 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -611,21 +611,16 @@ class HasManyAssociationsTest < ActiveRecord::TestCase def test_update_all_on_association_accessed_before_save firm = Firm.new(name: "Firm") - clients_proxy_id = firm.clients.object_id firm.clients << Client.first firm.save! assert_equal firm.clients.count, firm.clients.update_all(description: "Great!") - assert_not_equal clients_proxy_id, firm.clients.object_id end def test_update_all_on_association_accessed_before_save_with_explicit_foreign_key - # We can use the same cached proxy object because the id is available for the scope firm = Firm.new(name: "Firm", id: 100) - clients_proxy_id = firm.clients.object_id firm.clients << Client.first firm.save! assert_equal firm.clients.count, firm.clients.update_all(description: "Great!") - assert_equal clients_proxy_id, firm.clients.object_id end def test_belongs_to_sanity diff --git a/activerecord/test/cases/associations_test.rb b/activerecord/test/cases/associations_test.rb index c095b3a91c..e847af892c 100644 --- a/activerecord/test/cases/associations_test.rb +++ b/activerecord/test/cases/associations_test.rb @@ -233,11 +233,6 @@ class AssociationProxyTest < ActiveRecord::TestCase assert_equal david.projects, david.projects.scope end - test "proxy object is cached" do - david = developers(:david) - assert david.projects.equal?(david.projects) - end - test "inverses get set of subsets of the association" do man = Man.create man.interests.create -- cgit v1.2.3 From 673ed05ee96a9259ab329d909718240b9d63cc71 Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Tue, 19 Jul 2016 01:18:08 +0900 Subject: Cache target scope for collection proxy --- activerecord/lib/active_record/associations/association.rb | 2 +- activerecord/lib/active_record/associations/collection_proxy.rb | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/associations/association.rb b/activerecord/lib/active_record/associations/association.rb index 84d0493a60..1cb2b2d7c6 100644 --- a/activerecord/lib/active_record/associations/association.rb +++ b/activerecord/lib/active_record/associations/association.rb @@ -83,7 +83,7 @@ module ActiveRecord end def scope - target_scope.merge(association_scope) + target_scope.merge!(association_scope) end # The scope for this association. diff --git a/activerecord/lib/active_record/associations/collection_proxy.rb b/activerecord/lib/active_record/associations/collection_proxy.rb index 63f746688c..5b84351ef1 100644 --- a/activerecord/lib/active_record/associations/collection_proxy.rb +++ b/activerecord/lib/active_record/associations/collection_proxy.rb @@ -955,9 +955,8 @@ module ActiveRecord # Returns a Relation object for the records in this association def scope - @association.scope + @scope ||= @association.scope end - alias spawn scope # Equivalent to Array#==. Returns +true+ if the two arrays # contain the same number of elements and if each element is equal @@ -1089,6 +1088,7 @@ module ActiveRecord # person.pets(true) # fetches pets from the database # # => [#] def reload + @scope = nil proxy_association.reload self end @@ -1110,6 +1110,7 @@ module ActiveRecord # person.pets # fetches pets from the database # # => [#] def reset + @scope = nil proxy_association.reset proxy_association.reset_scope self -- cgit v1.2.3 From 38c4ff377260265d96c737c51541e6ca630ea707 Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Tue, 21 Feb 2017 02:50:05 +0900 Subject: Define `respond_to_missing?` instead of `respond_to?` --- activerecord/lib/active_record/associations/collection_proxy.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/associations/collection_proxy.rb b/activerecord/lib/active_record/associations/collection_proxy.rb index 5b84351ef1..6616de9f7f 100644 --- a/activerecord/lib/active_record/associations/collection_proxy.rb +++ b/activerecord/lib/active_record/associations/collection_proxy.rb @@ -1116,10 +1116,6 @@ module ActiveRecord self end - def respond_to?(name, include_private = false) - super || scope.respond_to?(name, include_private) - end - delegate_methods = [ QueryMethods, SpawnMethods, @@ -1153,6 +1149,10 @@ module ActiveRecord load_target end + def respond_to_missing?(method, _) + scope.respond_to?(method) || super + end + def method_missing(method, *args, &block) if scope.respond_to?(method) scope.public_send(method, *args, &block) -- cgit v1.2.3