Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Various refactoring to association scopes and joins #28276

Merged
merged 5 commits into from
Mar 4, 2017
Merged

Various refactoring to association scopes and joins #28276

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a5651eb
Move join scopes on to the reflection object
tenderlove Mar 3, 2017
c8e9466
ask reflection for klass join reflection
tenderlove Mar 3, 2017
4be4272
refactor so there is only one JoinKeys factory method
tenderlove Mar 3, 2017
a553302
`join_keys` no longer needs a class passed to it
tenderlove Mar 3, 2017
47b2ddd
remove unused parameters
tenderlove Mar 3, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
16 changes: 8 additions & 8 deletions activerecord/lib/active_record/associations/association_scope.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ def scope(association, connection)
chain_head, chain_tail = get_chain(reflection, association, alias_tracker)

scope.extending! Array(reflection.options[:extend])
add_constraints(scope, owner, klass, reflection, chain_head, chain_tail)
add_constraints(scope, owner, reflection, chain_head, chain_tail)
end

def join_type
Expand Down Expand Up @@ -60,8 +60,8 @@ def join(table, constraint)
table.create_join(table, table.create_on(constraint), join_type)
end

def last_chain_scope(scope, table, reflection, owner, association_klass)
join_keys = reflection.join_keys(association_klass)
def last_chain_scope(scope, table, reflection, owner)
join_keys = reflection.join_keys
key = join_keys.key
foreign_key = join_keys.foreign_key

Expand All @@ -80,8 +80,8 @@ def transform_value(value)
value_transformation.call(value)
end

def next_chain_scope(scope, table, reflection, association_klass, foreign_table, next_reflection)
join_keys = reflection.join_keys(association_klass)
def next_chain_scope(scope, table, reflection, foreign_table, next_reflection)
join_keys = reflection.join_keys
key = join_keys.key
foreign_key = join_keys.foreign_key

Expand Down Expand Up @@ -120,10 +120,10 @@ def get_chain(reflection, association, tracker)
[runtime_reflection, previous_reflection]
end

def add_constraints(scope, owner, association_klass, refl, chain_head, chain_tail)
def add_constraints(scope, owner, refl, chain_head, chain_tail)
owner_reflection = chain_tail
table = owner_reflection.alias_name
scope = last_chain_scope(scope, table, owner_reflection, owner, association_klass)
scope = last_chain_scope(scope, table, owner_reflection, owner)

reflection = chain_head
while reflection
Expand All @@ -132,7 +132,7 @@ def add_constraints(scope, owner, association_klass, refl, chain_head, chain_tai

unless reflection == chain_tail
foreign_table = next_reflection.alias_name
scope = next_chain_scope(scope, table, reflection, association_klass, foreign_table, next_reflection)
scope = next_chain_scope(scope, table, reflection, foreign_table, next_reflection)
end

# Exclude the scope of the association itself, because that
Expand Down
25 changes: 3 additions & 22 deletions activerecord/lib/active_record/associations/join_dependency/join_association.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,35 +34,16 @@ def join_constraints(foreign_table, foreign_klass, join_type, tables, chain)
table = tables.shift
klass = reflection.klass

join_keys = reflection.join_keys(klass)
join_keys = reflection.join_keys
key = join_keys.key
foreign_key = join_keys.foreign_key

constraint = build_constraint(klass, table, key, foreign_table, foreign_key)

predicate_builder = PredicateBuilder.new(TableMetadata.new(klass, table))
scope_chain_items = reflection.scopes.map do |item|
if item.is_a?(Relation)
item
else
ActiveRecord::Relation.create(klass, table, predicate_builder)
.instance_exec(&item)
end
end
scope_chain_items = reflection.join_scopes(table, predicate_builder)
klass_scope = reflection.klass_join_scope(table, predicate_builder)

klass_scope =
if klass.current_scope
klass.current_scope.clone.tap { |scope|
scope.joins_values = []
}
else
relation = ActiveRecord::Relation.create(
klass,
table,
predicate_builder,
)
klass.send(:build_default_scope, relation)
end
scope_chain_items.concat [klass_scope].compact

rel = scope_chain_items.inject(scope_chain_items.shift) do |left, right|
Expand Down
83 changes: 67 additions & 16 deletions activerecord/lib/active_record/reflection.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,8 +172,8 @@ def class_name

JoinKeys = Struct.new(:key, :foreign_key) # :nodoc:

def join_keys(association_klass)
JoinKeys.new(foreign_key, active_record_primary_key)
def join_keys
get_join_keys klass
end

# Returns a list of scopes that should be applied for this Reflection
Expand All @@ -187,6 +187,30 @@ def scope_chain
end
deprecate :scope_chain

def join_scopes(table, predicate_builder) # :nodoc:
if scope
[ActiveRecord::Relation.create(klass, table, predicate_builder)
.instance_exec(&scope)]
else
[]
end
end

def klass_join_scope(table, predicate_builder) # :nodoc:
if klass.current_scope
klass.current_scope.clone.tap { |scope|
scope.joins_values = []
}
else
relation = ActiveRecord::Relation.create(
klass,
table,
predicate_builder,
)
klass.send(:build_default_scope, relation)
end
end

def constraints
chain.map(&:scopes).flatten
end
Expand Down Expand Up @@ -260,6 +284,20 @@ def alias_candidate(name)
def chain
collect_join_chain
end

def get_join_keys(association_klass)
JoinKeys.new(join_pk(association_klass), join_fk)
end

private

def join_pk(_)
foreign_key
end

def join_fk
active_record_primary_key
end
end

# Base class for AggregateReflection and AssociationReflection. Objects of
Expand Down Expand Up @@ -687,11 +725,6 @@ def association_class
end
end

def join_keys(association_klass)
key = polymorphic? ? association_primary_key(association_klass) : association_primary_key
JoinKeys.new(key, foreign_key)
end

def join_id_for(owner) # :nodoc:
owner[foreign_key]
end
Expand All @@ -701,6 +734,14 @@ def join_id_for(owner) # :nodoc:
def calculate_constructable(macro, options)
!polymorphic?
end

def join_fk
foreign_key
end

def join_pk(klass)
polymorphic? ? association_primary_key(klass) : association_primary_key
end
end

class HasAndBelongsToManyReflection < AssociationReflection # :nodoc:
Expand All @@ -720,7 +761,7 @@ def collection?
class ThroughReflection < AbstractReflection #:nodoc:
attr_reader :delegate_reflection
delegate :foreign_key, :foreign_type, :association_foreign_key,
:active_record_primary_key, :type, to: :source_reflection
:active_record_primary_key, :type, :get_join_keys, to: :source_reflection

def initialize(delegate_reflection)
@delegate_reflection = delegate_reflection
Expand Down Expand Up @@ -806,6 +847,10 @@ def scopes
source_reflection.scopes + super
end

def join_scopes(table, predicate_builder) # :nodoc:
source_reflection.join_scopes(table, predicate_builder) + super
end

def source_type_scope
through_reflection.klass.where(foreign_type => options[:source_type])
end
Expand All @@ -816,10 +861,6 @@ def has_scope?
through_reflection.has_scope?
end

def join_keys(association_klass)
source_reflection.join_keys(association_klass)
end

# A through association is nested if there would be more than one join table
def nested?
source_reflection.through_reflection? || through_reflection.through_reflection?
Expand Down Expand Up @@ -954,6 +995,7 @@ def collect_join_reflections(seed)
end

private

def actual_source_reflection # FIXME: this is a horrible name
source_reflection.send(:actual_source_reflection)
end
Expand Down Expand Up @@ -990,6 +1032,15 @@ def scopes
end
end

def join_scopes(table, predicate_builder) # :nodoc:
scopes = @previous_reflection.join_scopes(table, predicate_builder) + super
if @previous_reflection.options[:source_type]
scopes + [@previous_reflection.source_type_scope]
else
scopes
end
end

def klass
@reflection.klass
end
Expand All @@ -1006,10 +1057,6 @@ def plural_name
@reflection.plural_name
end

def join_keys(association_klass)
@reflection.join_keys(association_klass)
end

def type
@reflection.type
end
Expand All @@ -1023,6 +1070,10 @@ def source_type_info
source_type = @previous_reflection.options[:source_type]
lambda { |object| where(type => source_type) }
end

def get_join_keys(association_klass)
@reflection.get_join_keys(association_klass)
end
end

class RuntimeReflection < PolymorphicReflection # :nodoc:
Expand Down