Represent association scope options as AR::Relations insternally.

This commit is contained in:
Jon Leighton 2012-07-06 18:20:20 +01:00
parent 76d3397061
commit 65843e1acc
15 changed files with 190 additions and 164 deletions

View File

@ -82,7 +82,7 @@ module ActiveRecord
end
def scoped
target_scope.merge(association_scope).merge(reflection_scope)
target_scope.merge(association_scope)
end
# The scope for this association.
@ -101,10 +101,6 @@ module ActiveRecord
@association_scope = nil
end
def reflection_scope
reflection.scope && klass.instance_exec(&reflection.scope)
end
# Set the inverse association, if possible
def set_inverse_instance(record)
if record && invertible_for?(record)
@ -152,6 +148,21 @@ module ActiveRecord
end
end
# We can't dump @reflection since it contains the scope proc
def marshal_dump
reflection = @reflection
@reflection = nil
ivars = instance_variables.map { |name| [name, instance_variable_get(name)] }
[reflection.name, ivars]
end
def marshal_load(data)
reflection_name, ivars = data
ivars.each { |name, val| instance_variable_set(name, val) }
@reflection = @owner.class.reflect_on_association(reflection_name)
end
private
def find_target?

View File

@ -6,7 +6,7 @@ module ActiveRecord
attr_reader :association, :alias_tracker
delegate :klass, :owner, :reflection, :interpolate, :to => :association
delegate :chain, :conditions, :options, :source_options, :active_record, :to => :reflection
delegate :chain, :scope_chain, :options, :source_options, :active_record, :to => :reflection
def initialize(association)
@association = association
@ -15,19 +15,8 @@ module ActiveRecord
def scope
scope = klass.unscoped
scope.extending!(*Array(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.merge!(options.slice(
:readonly, :references, :order, :limit, :joins, :group, :having, :offset, :select, :uniq))
if options[:include]
scope.includes! options[:include]
elsif options[:through]
scope.includes! source_options[:include]
end
scope.merge! eval_scope(klass, reflection.scope) if reflection.scope
add_constraints(scope)
end
@ -82,8 +71,6 @@ module ActiveRecord
foreign_key = reflection.active_record_primary_key
end
conditions = self.conditions[i]
if reflection == chain.last
bind_val = bind scope, table.table_name, key.to_s, owner[foreign_key]
scope = scope.where(table[key].eq(bind_val))
@ -93,14 +80,6 @@ module ActiveRecord
bind_val = bind scope, table.table_name, reflection.type.to_s, value
scope = scope.where(table[reflection.type].eq(bind_val))
end
conditions.each do |condition|
if options[:through] && condition.is_a?(Hash)
condition = disambiguate_condition(table, condition)
end
scope = scope.where(interpolate(condition))
end
else
constraint = table[key].eq(foreign_table[foreign_key])
@ -110,13 +89,15 @@ module ActiveRecord
end
scope = scope.joins(join(foreign_table, constraint))
end
conditions.each do |condition|
condition = interpolate(condition)
condition = disambiguate_condition(table, condition) unless i == 0
# Exclude the scope of the association itself, because that
# was already merged in the #scope method.
(scope_chain[i] - [self.reflection.scope]).each do |scope_chain_item|
item = eval_scope(reflection.klass, scope_chain_item)
scope = scope.where(condition)
end
scope.includes! item.includes_values
scope.where_values += item.where_values
end
end
@ -138,19 +119,13 @@ module ActiveRecord
end
end
def disambiguate_condition(table, condition)
if condition.is_a?(Hash)
Hash[
condition.map do |k, v|
if v.is_a?(Hash)
[k, v]
else
[table.table_alias || table.name, { k => v }]
end
end
]
def eval_scope(klass, scope)
return scope if scope.is_a?(Relation)
if scope.arity == 0
klass.unscoped.instance_exec(&scope)
else
condition
klass.unscoped.instance_exec(owner, &scope)
end
end
end

View File

@ -22,6 +22,49 @@ module ActiveRecord::Associations::Builder
else
@scope = nil
@options = scope
convert_deprecated_options_to_scope!
end
end
# FIXME: references should not be in this list
DEPRECATED_OPTIONS = [:readonly, :references, :order, :limit, :joins, :group, :having,
:offset, :select, :uniq, :include, :conditions]
class DeprecatedOptionsProc
attr_reader :options
def initialize(options)
@options = options
end
def to_proc
options = self.options
proc do |owner|
if options[:where].respond_to?(:to_proc)
context = owner || self
where(context.instance_eval(&options[:where]))
.merge!(options.except(:where))
else
scoped(options)
end
end
end
def arity
1
end
end
def convert_deprecated_options_to_scope!
deprecated_options = options.slice(*DEPRECATED_OPTIONS)
unless deprecated_options.empty?
deprecated_options[:includes] = deprecated_options.delete(:include) if deprecated_options[:include]
deprecated_options[:where] = deprecated_options.delete(:conditions) if deprecated_options[:conditions]
@scope = DeprecatedOptionsProc.new(deprecated_options)
@options = options.except(*DEPRECATED_OPTIONS)
end
end

View File

@ -183,7 +183,7 @@ module ActiveRecord
reflection.klass.count_by_sql(custom_counter_sql)
else
if options[:uniq]
if association_scope.uniq_value
# This is needed because 'SELECT count(DISTINCT *)..' is not valid SQL.
column_name ||= reflection.klass.primary_key
count_options[:distinct] = true
@ -246,14 +246,14 @@ module ActiveRecord
# +count_records+, which is a method descendants have to provide.
def size
if !find_target? || loaded?
if options[:uniq]
if association_scope.uniq_value
target.uniq.size
else
target.size
end
elsif !loaded? && options[:group]
elsif !loaded? && !association_scope.group_values.empty?
load_target.size
elsif !loaded? && !options[:uniq] && target.is_a?(Array)
elsif !loaded? && !association_scope.uniq_value && target.is_a?(Array)
unsaved_records = target.select { |r| r.new_record? }
unsaved_records.size + count_records
else
@ -344,7 +344,7 @@ module ActiveRecord
callback(:before_add, record)
yield(record) if block_given?
if options[:uniq] && index = @target.index(record)
if association_scope.uniq_value && index = @target.index(record)
@target[index] = record
else
@target << record

View File

@ -46,7 +46,7 @@ module ActiveRecord
# documented side-effect of the method that may avoid an extra SELECT.
@target ||= [] and loaded! if count == 0
[options[:limit], count].compact.min
[association_scope.limit_value, count].compact.min
end
def has_cached_counter?(reflection = reflection)

View File

@ -92,14 +92,21 @@ module ActiveRecord
constraint = build_constraint(reflection, table, key, foreign_table, foreign_key)
conditions = self.conditions[i].dup
conditions << { reflection.type => foreign_klass.base_class.name } if reflection.type
scope_chain_items = scope_chain[i]
conditions.each do |condition|
condition = active_record.send(:sanitize_sql, interpolate(condition), table.table_alias || table.name)
condition = Arel.sql(condition) unless condition.is_a?(Arel::Node)
if reflection.type
scope_chain_items += [
ActiveRecord::Relation.new(reflection.klass, table)
.where(reflection.type => foreign_klass.base_class.name)
]
end
constraint = constraint.and(condition)
scope_chain_items.each do |item|
unless item.is_a?(Relation)
item = ActiveRecord::Relation.new(reflection.klass, table).instance_exec(self, &item)
end
constraint = constraint.and(item.arel.constraints) unless item.arel.constraints.empty?
end
relation.from(join(table, constraint))
@ -137,18 +144,8 @@ module ActiveRecord
table.table_alias || table.name
end
def conditions
@conditions ||= reflection.conditions.reverse
end
private
def interpolate(conditions)
if conditions.respond_to?(:to_proc)
instance_eval(&conditions)
else
conditions
end
def scope_chain
@scope_chain ||= reflection.scope_chain.reverse
end
end

View File

@ -42,7 +42,7 @@ module ActiveRecord
autoload :HasAndBelongsToMany, 'active_record/associations/preloader/has_and_belongs_to_many'
autoload :BelongsTo, 'active_record/associations/preloader/belongs_to'
attr_reader :records, :associations, :options, :model
attr_reader :records, :associations, :preload_scope, :model
# Eager loads the named associations for the given Active Record record(s).
#
@ -78,15 +78,10 @@ module ActiveRecord
# [ :books, :author ]
# { :author => :avatar }
# [ :books, { :author => :avatar } ]
#
# +options+ contains options that will be passed to ActiveRecord::Base#find
# (which is called under the hood for preloading records). But it is passed
# only one level deep in the +associations+ argument, i.e. it's not passed
# to the child associations when +associations+ is a Hash.
def initialize(records, associations, options = {})
@records = Array.wrap(records).compact.uniq
@associations = Array.wrap(associations)
@options = options
def initialize(records, associations, preload_scope = nil)
@records = Array.wrap(records).compact.uniq
@associations = Array.wrap(associations)
@preload_scope = preload_scope || Relation.new(nil, nil)
end
def run
@ -110,7 +105,7 @@ module ActiveRecord
def preload_hash(association)
association.each do |parent, child|
Preloader.new(records, parent, options).run
Preloader.new(records, parent, preload_scope).run
Preloader.new(records.map { |record| record.send(parent) }.flatten, child).run
end
end
@ -125,7 +120,7 @@ module ActiveRecord
def preload_one(association)
grouped_records(association).each do |reflection, klasses|
klasses.each do |klass, records|
preloader_for(reflection).new(klass, records, reflection, options).run
preloader_for(reflection).new(klass, records, reflection, preload_scope).run
end
end
end

View File

@ -2,16 +2,16 @@ module ActiveRecord
module Associations
class Preloader
class Association #:nodoc:
attr_reader :owners, :reflection, :preload_options, :model, :klass
attr_reader :owners, :reflection, :preload_scope, :model, :klass
def initialize(klass, owners, reflection, preload_options)
@klass = klass
@owners = owners
@reflection = reflection
@preload_options = preload_options || {}
@model = owners.first && owners.first.class
@scoped = nil
@owners_by_key = nil
def initialize(klass, owners, reflection, preload_scope)
@klass = klass
@owners = owners
@reflection = reflection
@preload_scope = preload_scope
@model = owners.first && owners.first.class
@scoped = nil
@owners_by_key = nil
end
def run
@ -92,34 +92,29 @@ module ActiveRecord
records_by_owner
end
def reflection_scope
@reflection_scope ||= reflection.scope ? klass.unscoped.instance_exec(&reflection.scope) : klass.unscoped
end
def build_scope
scope = klass.unscoped
scope.default_scoped = true
scope = scope.where(interpolate(options[:conditions]))
scope = scope.where(interpolate(preload_options[:conditions]))
values = reflection_scope.values
preload_values = preload_scope.values
scope = scope.select(preload_options[:select] || options[:select] || table[Arel.star])
scope = scope.includes(preload_options[:include] || options[:include])
scope.where_values = Array(values[:where]) + Array(preload_values[:where])
scope.references_values = Array(values[:references]) + Array(preload_values[:references])
scope.select! preload_values[:select] || values[:select] || table[Arel.star]
scope.includes! preload_values[:includes] || values[:includes]
if options[:as]
scope = scope.where(
klass.table_name => {
reflection.type => model.base_class.sti_name
}
)
scope.where!(klass.table_name => { reflection.type => model.base_class.sti_name })
end
scope
end
def interpolate(conditions)
if conditions.respond_to?(:to_proc)
klass.send(:instance_eval, &conditions)
else
conditions
end
end
end
end
end

View File

@ -6,7 +6,7 @@ module ActiveRecord
private
def build_scope
super.order(preload_options[:order] || options[:order])
super.order(preload_scope.values[:order] || reflection_scope.values[:order])
end
def preload

View File

@ -6,7 +6,7 @@ module ActiveRecord
def associated_records_by_owner
super.each do |owner, records|
records.uniq! if options[:uniq]
records.uniq! if reflection_scope.uniq_value
end
end
end

View File

@ -14,7 +14,7 @@ module ActiveRecord
private
def build_scope
super.order(preload_options[:order] || options[:order])
super.order(preload_scope.values[:order] || reflection_scope.values[:order])
end
end

View File

@ -14,10 +14,7 @@ module ActiveRecord
def associated_records_by_owner
through_records = through_records_by_owner
ActiveRecord::Associations::Preloader.new(
through_records.values.flatten,
source_reflection.name, options
).run
Preloader.new(through_records.values.flatten, source_reflection.name, reflection_scope).run
through_records.each do |owner, records|
records.map! { |r| r.send(source_reflection.name) }.flatten!
@ -28,10 +25,7 @@ module ActiveRecord
private
def through_records_by_owner
ActiveRecord::Associations::Preloader.new(
owners, through_reflection.name,
through_options
).run
Preloader.new(owners, through_reflection.name, through_scope).run
Hash[owners.map do |owner|
through_records = Array.wrap(owner.send(through_reflection.name))
@ -45,21 +39,22 @@ module ActiveRecord
end]
end
def through_options
through_options = {}
def through_scope
through_scope = through_reflection.klass.unscoped
if options[:source_type]
through_options[:conditions] = { reflection.foreign_type => options[:source_type] }
through_scope.where! reflection.foreign_type => options[:source_type]
else
if options[:conditions]
through_options[:include] = options[:include] || options[:source]
through_options[:conditions] = options[:conditions]
unless reflection_scope.where_values.empty?
through_scope.includes_values = reflection_scope.values[:includes] || options[:source]
through_scope.where_values = reflection_scope.values[:where]
end
through_options[:order] = options[:order]
through_scope.order! reflection_scope.values[:order]
through_scope.references! reflection_scope.values[:references]
end
through_options
through_scope
end
end
end

View File

@ -247,11 +247,10 @@ module ActiveRecord
false
end
# An array of arrays of conditions. Each item in the outside array corresponds to a reflection
# in the #chain. The inside arrays are simply conditions (and each condition may itself be
# a hash, array, arel predicate, etc...)
def conditions
[[options[:conditions]].compact]
# 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
alias :source_macro :macro
@ -419,28 +418,25 @@ module ActiveRecord
# has_many :tags
# end
#
# There may be conditions on Person.comment_tags, Article.comment_tags and/or Comment.tags,
# 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 conditions corresponding to the chain. Each item in the #conditions array corresponds
# to an item in the #chain, and is itself an array of conditions from an arbitrary number
# of relevant reflections, plus any :source_type or polymorphic :as constraints.
def conditions
@conditions ||= begin
conditions = source_reflection.conditions.map { |c| c.dup }
# of scopes corresponding to the chain.
def scope_chain
@scope_chain ||= begin
scope_chain = source_reflection.scope_chain.map(&:dup)
# Add to it the conditions from this reflection if necessary.
conditions.first << options[:conditions] if options[:conditions]
# Add to it the scope from this reflection (if any)
scope_chain.first << scope if scope
through_conditions = through_reflection.conditions
through_scope_chain = through_reflection.scope_chain
if options[:source_type]
through_conditions.first << { foreign_type => options[:source_type] }
through_scope_chain.first <<
through_reflection.klass.where(foreign_type => options[:source_type])
end
# Recursively fill out the rest of the array from the through reflection
conditions += through_conditions
conditions
scope_chain + through_scope_chain
end
end

View File

@ -0,0 +1,15 @@
require 'cases/helper'
require 'models/post'
require 'models/author'
module ActiveRecord
module Associations
class AssociationScopeTest < ActiveRecord::TestCase
test 'does not duplicate conditions' do
association_scope = AssociationScope.new(Author.new.association(:welcome_posts))
wheres = association_scope.scope.where_values.map(&:right)
assert_equal wheres.uniq, wheres
end
end
end
end

View File

@ -190,21 +190,25 @@ class ReflectionTest < ActiveRecord::TestCase
assert_equal expected, actual
end
def test_conditions
def test_scope_chain
expected = [
[{ :tags => { :name => 'Blue' } }],
[{ :taggings => { :comment => 'first' } }],
[{ :posts => { :title => ['misc post by bob', 'misc post by mary'] } }]
[Tagging.reflect_on_association(:tag).scope, Post.reflect_on_association(:first_blue_tags).scope],
[Post.reflect_on_association(:first_taggings).scope],
[Author.reflect_on_association(:misc_posts).scope]
]
actual = Author.reflect_on_association(:misc_post_first_blue_tags).conditions
actual = Author.reflect_on_association(:misc_post_first_blue_tags).scope_chain
assert_equal expected, actual
expected = [
[{ :tags => { :name => 'Blue' } }, { :taggings => { :comment => 'first' } }, { :posts => { :title => ['misc post by bob', 'misc post by mary'] } }],
[
Tagging.reflect_on_association(:blue_tag).scope,
Post.reflect_on_association(:first_blue_tags_2).scope,
Author.reflect_on_association(:misc_post_first_blue_tags_2).scope
],
[],
[]
]
actual = Author.reflect_on_association(:misc_post_first_blue_tags_2).conditions
actual = Author.reflect_on_association(:misc_post_first_blue_tags_2).scope_chain
assert_equal expected, actual
end
@ -295,10 +299,10 @@ class ReflectionTest < ActiveRecord::TestCase
assert_equal "category_id", Post.reflect_on_association(:categorizations).foreign_key.to_s
end
def test_through_reflection_conditions_do_not_modify_other_reflections
orig_conds = Post.reflect_on_association(:first_blue_tags_2).conditions.inspect
Author.reflect_on_association(:misc_post_first_blue_tags_2).conditions
assert_equal orig_conds, Post.reflect_on_association(:first_blue_tags_2).conditions.inspect
def test_through_reflection_scope_chain_does_not_modify_other_reflections
orig_conds = Post.reflect_on_association(:first_blue_tags_2).scope_chain.inspect
Author.reflect_on_association(:misc_post_first_blue_tags_2).scope_chain
assert_equal orig_conds, Post.reflect_on_association(:first_blue_tags_2).scope_chain.inspect
end
def test_symbol_for_class_name
@ -309,11 +313,11 @@ class ReflectionTest < ActiveRecord::TestCase
category = Struct.new(:table_name, :pluralize_table_names).new('categories', true)
product = Struct.new(:table_name, :pluralize_table_names).new('products', true)
reflection = AssociationReflection.new(:has_and_belongs_to_many, :categories, {}, product)
reflection = AssociationReflection.new(:has_and_belongs_to_many, :categories, nil, {}, product)
reflection.stubs(:klass).returns(category)
assert_equal 'categories_products', reflection.join_table
reflection = AssociationReflection.new(:has_and_belongs_to_many, :products, {}, category)
reflection = AssociationReflection.new(:has_and_belongs_to_many, :products, nil, {}, category)
reflection.stubs(:klass).returns(product)
assert_equal 'categories_products', reflection.join_table
end
@ -322,11 +326,11 @@ class ReflectionTest < ActiveRecord::TestCase
category = Struct.new(:table_name, :pluralize_table_names).new('catalog_categories', true)
product = Struct.new(:table_name, :pluralize_table_names).new('catalog_products', true)
reflection = AssociationReflection.new(:has_and_belongs_to_many, :categories, {}, product)
reflection = AssociationReflection.new(:has_and_belongs_to_many, :categories, nil, {}, product)
reflection.stubs(:klass).returns(category)
assert_equal 'catalog_categories_products', reflection.join_table
reflection = AssociationReflection.new(:has_and_belongs_to_many, :products, {}, category)
reflection = AssociationReflection.new(:has_and_belongs_to_many, :products, nil, {}, category)
reflection.stubs(:klass).returns(product)
assert_equal 'catalog_categories_products', reflection.join_table
end
@ -335,11 +339,11 @@ class ReflectionTest < ActiveRecord::TestCase
category = Struct.new(:table_name, :pluralize_table_names).new('catalog_categories', true)
page = Struct.new(:table_name, :pluralize_table_names).new('content_pages', true)
reflection = AssociationReflection.new(:has_and_belongs_to_many, :categories, {}, page)
reflection = AssociationReflection.new(:has_and_belongs_to_many, :categories, nil, {}, page)
reflection.stubs(:klass).returns(category)
assert_equal 'catalog_categories_content_pages', reflection.join_table
reflection = AssociationReflection.new(:has_and_belongs_to_many, :pages, {}, category)
reflection = AssociationReflection.new(:has_and_belongs_to_many, :pages, nil, {}, category)
reflection.stubs(:klass).returns(page)
assert_equal 'catalog_categories_content_pages', reflection.join_table
end
@ -348,11 +352,11 @@ class ReflectionTest < ActiveRecord::TestCase
category = Struct.new(:table_name, :pluralize_table_names).new('categories', true)
product = Struct.new(:table_name, :pluralize_table_names).new('products', true)
reflection = AssociationReflection.new(:has_and_belongs_to_many, :categories, { :join_table => 'product_categories' }, product)
reflection = AssociationReflection.new(:has_and_belongs_to_many, :categories, nil, { :join_table => 'product_categories' }, product)
reflection.stubs(:klass).returns(category)
assert_equal 'product_categories', reflection.join_table
reflection = AssociationReflection.new(:has_and_belongs_to_many, :products, { :join_table => 'product_categories' }, category)
reflection = AssociationReflection.new(:has_and_belongs_to_many, :products, nil, { :join_table => 'product_categories' }, category)
reflection.stubs(:klass).returns(product)
assert_equal 'product_categories', reflection.join_table
end