From 6490d65234b89d4d28308b72b13d4834fd44bbb3 Mon Sep 17 00:00:00 2001 From: Jon Leighton Date: Thu, 10 Mar 2011 19:04:00 +0000 Subject: [PATCH] Move the code which builds a scope for through associations into a generic AssociationScope class which is capable of building a scope for any association. --- .../lib/active_record/associations.rb | 7 +- .../active_record/associations/association.rb | 44 +----- .../associations/association_scope.rb | 149 ++++++++++++++++++ .../associations/collection_association.rb | 13 -- .../has_and_belongs_to_many_association.rb | 22 --- .../associations/has_many_association.rb | 2 - .../associations/has_one_association.rb | 6 - .../associations/through_association.rb | 128 --------------- activerecord/lib/active_record/reflection.rb | 6 +- 9 files changed, 162 insertions(+), 215 deletions(-) create mode 100644 activerecord/lib/active_record/associations/association_scope.rb diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index ec5b41a3e7..90745112b1 100644 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -140,9 +140,10 @@ module ActiveRecord autoload :HasAndBelongsToMany, 'active_record/associations/builder/has_and_belongs_to_many' end - autoload :Preloader, 'active_record/associations/preloader' - autoload :JoinDependency, 'active_record/associations/join_dependency' - autoload :AliasTracker, 'active_record/associations/alias_tracker' + autoload :Preloader, 'active_record/associations/preloader' + autoload :JoinDependency, 'active_record/associations/join_dependency' + autoload :AssociationScope, 'active_record/associations/association_scope' + autoload :AliasTracker, 'active_record/associations/alias_tracker' # Clears out the association cache. def clear_association_cache #:nodoc: diff --git a/activerecord/lib/active_record/associations/association.rb b/activerecord/lib/active_record/associations/association.rb index 25b4b9d90d..27c446b12c 100644 --- a/activerecord/lib/active_record/associations/association.rb +++ b/activerecord/lib/active_record/associations/association.rb @@ -93,23 +93,9 @@ module ActiveRecord # by scope.scoping { ... } or with_scope { ... } etc, which affects the scope which # actually gets built. def construct_scope - @association_scope = association_scope if klass - end - - def association_scope - scope = klass.unscoped - scope = scope.create_with(creation_attributes) - scope = scope.apply_finder_options(options.slice(:readonly, :include)) - scope = scope.where(interpolate(options[:conditions])) - if select = select_value - scope = scope.select(select) + if klass + @association_scope = AssociationScope.new(self).scope end - scope = scope.extending(*Array.wrap(options[:extend])) - scope.where(construct_owner_conditions) - end - - def aliased_table - klass.arel_table end # Set the inverse association, if possible @@ -174,42 +160,24 @@ module ActiveRecord end end - def select_value - options[:select] - end - - # Implemented by (some) subclasses def creation_attributes - { } - end - - # Returns a hash linking the owner to the association represented by the reflection - def construct_owner_attributes(reflection = reflection) attributes = {} - if reflection.macro == :belongs_to - attributes[reflection.association_primary_key] = owner[reflection.foreign_key] - else + + if [:has_one, :has_many].include?(reflection.macro) && !options[:through] attributes[reflection.foreign_key] = owner[reflection.active_record_primary_key] if reflection.options[:as] attributes[reflection.type] = owner.class.base_class.name end end - attributes - end - # Builds an array of arel nodes from the owner attributes hash - def construct_owner_conditions(table = aliased_table, reflection = reflection) - conditions = construct_owner_attributes(reflection).map do |attr, value| - table[attr].eq(value) - end - table.create_and(conditions) + attributes end # Sets the owner attributes on the given record def set_owner_attributes(record) if owner.persisted? - construct_owner_attributes.each { |key, value| record[key] = value } + creation_attributes.each { |key, value| record[key] = value } end end diff --git a/activerecord/lib/active_record/associations/association_scope.rb b/activerecord/lib/active_record/associations/association_scope.rb new file mode 100644 index 0000000000..5df2e964be --- /dev/null +++ b/activerecord/lib/active_record/associations/association_scope.rb @@ -0,0 +1,149 @@ +module ActiveRecord + module Associations + class AssociationScope #:nodoc: + attr_reader :association, :alias_tracker + + delegate :klass, :owner, :reflection, :interpolate, :to => :association + delegate :through_reflection_chain, :through_conditions, :options, :source_options, :to => :reflection + + def initialize(association) + @association = association + @alias_tracker = AliasTracker.new + end + + def scope + scope = klass.unscoped + scope = scope.extending(*Array.wrap(options[:extend])) + + # It's okay to just apply all these like this. The options will only be present if the + # association supports that option; this is enforced by the association builder. + scope = scope.apply_finder_options(options.slice( + :readonly, :include, :order, :limit, :joins, :group, :having, :offset)) + + if options[:through] && !options[:include] + scope = scope.includes(source_options[:include]) + end + + if select = select_value + scope = scope.select(select) + end + + add_constraints(scope) + end + + private + + def select_value + select_value = options[:select] + + if reflection.collection? + select_value ||= options[:uniq] && "DISTINCT #{reflection.quoted_table_name}.*" + end + + if reflection.macro == :has_and_belongs_to_many + select_value ||= reflection.klass.arel_table[Arel.star] + end + + select_value + end + + def add_constraints(scope) + tables = construct_tables + + through_reflection_chain.each_with_index do |reflection, i| + table, foreign_table = tables.shift, tables.first + + if reflection.source_macro == :has_and_belongs_to_many + join_table = tables.shift + + scope = scope.joins(inner_join( + join_table, reflection, + table[reflection.active_record_primary_key]. + eq(join_table[reflection.association_foreign_key]) + )) + + table, foreign_table = join_table, tables.first + end + + if reflection.source_macro == :belongs_to + key = reflection.association_primary_key + foreign_key = reflection.foreign_key + else + key = reflection.foreign_key + foreign_key = reflection.active_record_primary_key + end + + if reflection == through_reflection_chain.last + scope = scope.where(table[key].eq(owner[foreign_key])) + + through_conditions[i].each do |condition| + if options[:through] && condition.is_a?(Hash) + condition = { table.name => condition } + end + + scope = scope.where(interpolate(condition)) + end + else + constraint = table[key].eq foreign_table[foreign_key] + + join = inner_join(foreign_table, reflection, constraint, *through_conditions[i]) + scope = scope.joins(join) + end + end + + scope + end + + def construct_tables + tables = [] + through_reflection_chain.each do |reflection| + tables << alias_tracker.aliased_table_for( + table_name_for(reflection), + table_alias_for(reflection, reflection != self.reflection) + ) + + if reflection.source_macro == :has_and_belongs_to_many + tables << alias_tracker.aliased_table_for( + (reflection.source_reflection || reflection).options[:join_table], + table_alias_for(reflection, true) + ) + end + end + tables + end + + def table_name_for(reflection) + if reflection == self.reflection + # If this is a polymorphic belongs_to, we want to get the klass from the + # association because it depends on the polymorphic_type attribute of + # the owner + klass.table_name + else + reflection.table_name + end + end + + def table_alias_for(reflection, join = false) + name = alias_tracker.pluralize(reflection.name) + name << "_#{self.reflection.name}" + name << "_join" if join + name + end + + def inner_join(table, reflection, *conditions) + conditions = sanitize_conditions(reflection, conditions) + table.create_join(table, table.create_on(conditions)) + end + + def sanitize_conditions(reflection, conditions) + conditions = conditions.map do |condition| + condition = reflection.klass.send(:sanitize_sql, interpolate(condition), reflection.table_name) + condition = Arel.sql(condition) unless condition.is_a?(Arel::Node) + condition + end + + conditions.length == 1 ? conditions.first : Arel::Nodes::And.new(conditions) + end + end + end +end diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb index f3761bd2c7..9f4fc44cc6 100644 --- a/activerecord/lib/active_record/associations/collection_association.rb +++ b/activerecord/lib/active_record/associations/collection_association.rb @@ -331,11 +331,6 @@ module ActiveRecord @scopes_cache[method][args] ||= scoped.readonly(nil).send(method, *args) end - def association_scope - options = reflection.options.slice(:order, :limit, :joins, :group, :having, :offset) - super.apply_finder_options(options) - end - def load_target if find_target? targets = [] @@ -373,14 +368,6 @@ module ActiveRecord private - def select_value - super || uniq_select_value - end - - def uniq_select_value - options[:uniq] && "DISTINCT #{reflection.quoted_table_name}.*" - end - def custom_counter_sql if options[:counter_sql] interpolate(options[:counter_sql]) diff --git a/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb b/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb index 028630977d..217213808b 100644 --- a/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb +++ b/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb @@ -26,10 +26,6 @@ module ActiveRecord record end - def association_scope - super.joins(construct_joins) - end - private def count_records @@ -48,24 +44,6 @@ module ActiveRecord end end - def construct_joins - right = join_table - left = reflection.klass.arel_table - - condition = left[reflection.klass.primary_key].eq( - right[reflection.association_foreign_key]) - - right.create_join(right, right.create_on(condition)) - end - - def construct_owner_conditions - super(join_table) - end - - def select_value - super || reflection.klass.arel_table[Arel.star] - end - def invertible_for?(record) false end diff --git a/activerecord/lib/active_record/associations/has_many_association.rb b/activerecord/lib/active_record/associations/has_many_association.rb index cebf3e477a..78c5c4b870 100644 --- a/activerecord/lib/active_record/associations/has_many_association.rb +++ b/activerecord/lib/active_record/associations/has_many_association.rb @@ -94,8 +94,6 @@ module ActiveRecord end end end - - alias creation_attributes construct_owner_attributes end end end diff --git a/activerecord/lib/active_record/associations/has_one_association.rb b/activerecord/lib/active_record/associations/has_one_association.rb index e13f97125f..1d2e8667e4 100644 --- a/activerecord/lib/active_record/associations/has_one_association.rb +++ b/activerecord/lib/active_record/associations/has_one_association.rb @@ -39,14 +39,8 @@ module ActiveRecord end end - def association_scope - super.order(options[:order]) - end - private - alias creation_attributes construct_owner_attributes - # The reason that the save param for replace is false, if for create (not just build), # is because the setting of the foreign keys is actually handled by the scoping when # the record is instantiated, and so they are set straight away and do not need to be diff --git a/activerecord/lib/active_record/associations/through_association.rb b/activerecord/lib/active_record/associations/through_association.rb index ae2c8b65ed..408237c689 100644 --- a/activerecord/lib/active_record/associations/through_association.rb +++ b/activerecord/lib/active_record/associations/through_association.rb @@ -1,5 +1,3 @@ -require 'enumerator' - module ActiveRecord # = Active Record Through Association module Associations @@ -26,77 +24,8 @@ module ActiveRecord scope end - def association_scope - scope = join_to(super) - - unless options[:include] - scope = scope.includes(source_options[:include]) - end - - scope - end - private - # This scope affects the creation of the associated records (not the join records). At the - # moment we only support creating on a :through association when the source reflection is a - # belongs_to. Thus it's not necessary to set a foreign key on the associated record(s), so - # this scope has can legitimately be empty. - def creation_attributes - { } - end - - # TODO: Needed? - def aliased_through_table - name = through_reflection.table_name - - reflection.table_name == name ? - through_reflection.klass.arel_table.alias(name + "_join") : - through_reflection.klass.arel_table - end - - def construct_owner_conditions - end - - def join_to(scope) - joins = [] - tables = tables().dup # FIXME: Ugly - - through_reflection_chain.each_with_index do |reflection, i| - table, foreign_table = tables.shift, tables.first - - if reflection.source_macro == :has_and_belongs_to_many - join_table = tables.shift - - joins << inner_join( - join_table, - table[reflection.active_record_primary_key]. - eq(join_table[reflection.association_foreign_key]) - ) - - table, foreign_table = join_table, tables.first - end - - if reflection.source_macro == :belongs_to - key = reflection.association_primary_key - foreign_key = reflection.foreign_key - else - key = reflection.foreign_key - foreign_key = reflection.active_record_primary_key - end - - if reflection == through_reflection_chain.last - constraint = table[key].eq owner[foreign_key] - scope = scope.where(constraint).where(reflection_conditions(i)) - else - constraint = table[key].eq foreign_table[foreign_key] - joins << inner_join(foreign_table, constraint, reflection_conditions(i)) - end - end - - scope.joins(joins) - end - # Construct attributes for :through pointing to owner and associate. This is used by the # methods which create and delete records on the association. # @@ -133,63 +62,6 @@ module ActiveRecord end end - def alias_tracker - @alias_tracker ||= AliasTracker.new - end - - def tables - @tables ||= begin - tables = [] - through_reflection_chain.each do |reflection| - tables << alias_tracker.aliased_table_for( - reflection.table_name, - table_alias_for(reflection, reflection != self.reflection) - ) - - if reflection.macro == :has_and_belongs_to_many || - (reflection.source_reflection && - reflection.source_reflection.macro == :has_and_belongs_to_many) - - tables << alias_tracker.aliased_table_for( - (reflection.source_reflection || reflection).options[:join_table], - table_alias_for(reflection, true) - ) - end - end - tables - end - end - - def table_alias_for(reflection, join = false) - name = alias_tracker.pluralize(reflection.name) - name << "_#{self.reflection.name}" - name << "_join" if join - name - end - - def inner_join(table, *conditions) - table.create_join( - table, - table.create_on(table.create_and(conditions.flatten.compact))) - end - - def reflection_conditions(index) - reflection = through_reflection_chain[index] - conditions = through_conditions[index] - - unless conditions.empty? - Arel::Nodes::And.new(process_conditions(conditions, reflection)) - end - end - - def process_conditions(conditions, reflection) - conditions.map do |condition| - condition = reflection.klass.send(:sanitize_sql, interpolate(condition), reflection.table_name) - condition = Arel.sql(condition) unless condition.is_a?(Arel::Node) - condition - end - end - # TODO: Think about this in the context of nested associations def stale_state if through_reflection.macro == :belongs_to diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index 0a9855ec25..8c73e49e74 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -270,9 +270,9 @@ module ActiveRecord end def through_conditions - through_conditions = [Array.wrap(options[:conditions])] - through_conditions.first << { type => active_record.base_class.name } if options[:as] - through_conditions + conditions = [options[:conditions]].compact + conditions << { type => active_record.base_class.name } if options[:as] + [conditions] end def source_reflection