From 4fef7c2a56ee183e720e634bd23d674f46949c1a Mon Sep 17 00:00:00 2001 From: eileencodes Date: Mon, 30 Jan 2017 14:11:53 -0500 Subject: Implement `scopes` method on each Reflection Each reflection should be responsible for returning the scopes needed to query against the db. --- activerecord/lib/active_record/reflection.rb | 52 +++++++++++++++++++++++++--- 1 file changed, 47 insertions(+), 5 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index 2c8c4b6297..face8e9200 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -175,6 +175,10 @@ module ActiveRecord JoinKeys.new(foreign_key, active_record_primary_key) end + def scopes + scope ? [scope] : [] + end + def constraints scope_chain.flatten end @@ -815,6 +819,40 @@ module ActiveRecord # There may be scopes on Person.comment_tags, Article.comment_tags and/or Comment.tags, # but only Comment.tags will be represented in the #chain. So this method creates an array # of scopes corresponding to the chain. + def scopes + @sc ||= if scope + source_reflection.scopes + through_reflection.scopes + [scope] + else + source_reflection.scopes + through_reflection.scopes + end + + return @sc + scope_chain = source_reflection.scopes + scope_chain = through_reflection.scopes + + # Add to it the scope from this reflection (if any) + scope_chain.first << scope if scope + + through_scope_chain = through_reflection.scope_chain.map(&:dup) + + if options[:source_type] + through_scope_chain.first << source_type_lambda + end + + # Recursively fill out the rest of the array from the through reflection + scope_chain + through_scope_chain + end + + def source_type_lambda + @source_type_lambda ||= begin + type = foreign_type + source_type = options[:source_type] + lambda { |object| + where(type => source_type) + } + end + end + def scope_chain @scope_chain ||= begin scope_chain = source_reflection.scope_chain.map(&:dup) @@ -825,11 +863,7 @@ module ActiveRecord through_scope_chain = through_reflection.scope_chain.map(&:dup) if options[:source_type] - type = foreign_type - source_type = options[:source_type] - through_scope_chain.first << lambda { |object| - where(type => source_type) - } + through_scope_chain.first << source_type_lambda end # Recursively fill out the rest of the array from the through reflection @@ -1008,6 +1042,14 @@ module ActiveRecord @previous_reflection = previous_reflection end + def scopes + if @previous_reflection.options[:source_type] + return super + [@previous_reflection.source_type_lambda] + else + return super + end + end + def klass @reflection.klass end -- cgit v1.2.3 From e8be3a9016b6c27ead6d5608c3f988c5f72b385b Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 30 Jan 2017 13:51:48 -0800 Subject: Fix `scopes` implementation on `PolymorphicReflection` `PolymorphicReflection` needs to be custom for handling scope lambdas --- .../active_record/associations/join_dependency.rb | 2 +- .../join_dependency/join_association.rb | 8 +-- activerecord/lib/active_record/reflection.rb | 59 +++++----------------- 3 files changed, 15 insertions(+), 54 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/associations/join_dependency.rb b/activerecord/lib/active_record/associations/join_dependency.rb index a79eb03acc..87e0847ec1 100644 --- a/activerecord/lib/active_record/associations/join_dependency.rb +++ b/activerecord/lib/active_record/associations/join_dependency.rb @@ -171,7 +171,7 @@ module ActiveRecord chain = child.reflection.chain foreign_table = parent.table foreign_klass = parent.base_klass - child.join_constraints(foreign_table, foreign_klass, child, join_type, tables, child.reflection.scope_chain, chain) + child.join_constraints(foreign_table, foreign_klass, child, join_type, tables, chain) end def make_outer_joins(parent, child) diff --git a/activerecord/lib/active_record/associations/join_dependency/join_association.rb b/activerecord/lib/active_record/associations/join_dependency/join_association.rb index a5705951f3..f5fcba1236 100644 --- a/activerecord/lib/active_record/associations/join_dependency/join_association.rb +++ b/activerecord/lib/active_record/associations/join_dependency/join_association.rb @@ -23,14 +23,11 @@ module ActiveRecord JoinInformation = Struct.new :joins, :binds - def join_constraints(foreign_table, foreign_klass, node, join_type, tables, scope_chain, chain) + def join_constraints(foreign_table, foreign_klass, node, join_type, tables, chain) joins = [] binds = [] tables = tables.reverse - scope_chain_index = 0 - scope_chain = scope_chain.reverse - # The chain starts with the target table, but we want to end with it here (makes # more sense in this context), so we reverse chain.reverse_each do |reflection| @@ -44,7 +41,7 @@ module ActiveRecord constraint = build_constraint(klass, table, key, foreign_table, foreign_key) predicate_builder = PredicateBuilder.new(TableMetadata.new(klass, table)) - scope_chain_items = scope_chain[scope_chain_index].map do |item| + scope_chain_items = reflection.scopes.map do |item| if item.is_a?(Relation) item else @@ -52,7 +49,6 @@ module ActiveRecord .instance_exec(node, &item) end end - scope_chain_index += 1 klass_scope = if klass.current_scope diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index face8e9200..70e2487891 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -179,8 +179,12 @@ module ActiveRecord scope ? [scope] : [] end + def scope_chain + chain.map(&:scopes) + end + def constraints - scope_chain.flatten + chain.map(&:scopes).flatten end def counter_cache_column @@ -465,12 +469,6 @@ module ActiveRecord false end - # An array of arrays of scopes. Each item in the outside array corresponds to a reflection - # in the #chain. - def scope_chain - scope ? [[scope]] : [[]] - end - def has_scope? scope end @@ -820,30 +818,14 @@ module ActiveRecord # but only Comment.tags will be represented in the #chain. So this method creates an array # of scopes corresponding to the chain. def scopes - @sc ||= if scope - source_reflection.scopes + through_reflection.scopes + [scope] + if scope + source_reflection.scopes + [scope] else - source_reflection.scopes + through_reflection.scopes - end - - return @sc - scope_chain = source_reflection.scopes - scope_chain = through_reflection.scopes - - # Add to it the scope from this reflection (if any) - scope_chain.first << scope if scope - - through_scope_chain = through_reflection.scope_chain.map(&:dup) - - if options[:source_type] - through_scope_chain.first << source_type_lambda + source_reflection.scopes end - - # Recursively fill out the rest of the array from the through reflection - scope_chain + through_scope_chain end - def source_type_lambda + def source_type_scope @source_type_lambda ||= begin type = foreign_type source_type = options[:source_type] @@ -853,24 +835,6 @@ module ActiveRecord end end - def scope_chain - @scope_chain ||= begin - scope_chain = source_reflection.scope_chain.map(&:dup) - - # Add to it the scope from this reflection (if any) - scope_chain.first << scope if scope - - through_scope_chain = through_reflection.scope_chain.map(&:dup) - - if options[:source_type] - through_scope_chain.first << source_type_lambda - end - - # Recursively fill out the rest of the array from the through reflection - scope_chain + through_scope_chain - end - end - def has_scope? scope || options[:source_type] || source_reflection.has_scope? || @@ -1043,10 +1007,11 @@ module ActiveRecord end def scopes + scopes = @previous_reflection.scopes if @previous_reflection.options[:source_type] - return super + [@previous_reflection.source_type_lambda] + scopes + [@previous_reflection.source_type_scope] else - return super + scopes end end -- cgit v1.2.3 From d3851621de4efd74e34777aca8626a146c03d4b1 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 30 Jan 2017 14:06:58 -0800 Subject: remove caching until we prove it is required --- activerecord/lib/active_record/reflection.rb | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index 70e2487891..cb240229a5 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -826,13 +826,9 @@ module ActiveRecord end def source_type_scope - @source_type_lambda ||= begin - type = foreign_type - source_type = options[:source_type] - lambda { |object| - where(type => source_type) - } - end + type = foreign_type + source_type = options[:source_type] + lambda { |object| where(type => source_type) } end def has_scope? -- cgit v1.2.3 From f91bc245978f9e42ae48a553e481fcc48d2a97bd Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 30 Jan 2017 14:10:33 -0800 Subject: update comments and call `super` --- activerecord/lib/active_record/reflection.rb | 27 +++------------------------ 1 file changed, 3 insertions(+), 24 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index cb240229a5..71993a4fe4 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -175,6 +175,8 @@ module ActiveRecord JoinKeys.new(foreign_key, active_record_primary_key) end + # Returns a list of scopes that should be applied for this Reflection + # object when querying the database. def scopes scope ? [scope] : [] end @@ -798,31 +800,8 @@ module ActiveRecord through_reflection.clear_association_scope_cache end - # Consider the following example: - # - # class Person - # has_many :articles - # has_many :comment_tags, through: :articles - # end - # - # class Article - # has_many :comments - # has_many :comment_tags, through: :comments, source: :tags - # end - # - # class Comment - # has_many :tags - # end - # - # There may be scopes on Person.comment_tags, Article.comment_tags and/or Comment.tags, - # but only Comment.tags will be represented in the #chain. So this method creates an array - # of scopes corresponding to the chain. def scopes - if scope - source_reflection.scopes + [scope] - else - source_reflection.scopes - end + source_reflection.scopes + super end def source_type_scope -- cgit v1.2.3 From 233630a175f6e6f1ab8c99af380f550038e04b3d Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 30 Jan 2017 14:17:19 -0800 Subject: deprecate `scope_chain` --- activerecord/lib/active_record/reflection.rb | 2 ++ 1 file changed, 2 insertions(+) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index 71993a4fe4..d923050822 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -1,5 +1,6 @@ require "thread" require "active_support/core_ext/string/filters" +require "active_support/deprecation" module ActiveRecord # = Active Record Reflection @@ -184,6 +185,7 @@ module ActiveRecord def scope_chain chain.map(&:scopes) end + deprecate :scope_chain def constraints chain.map(&:scopes).flatten -- cgit v1.2.3