Permalink
Browse files

remove state from the preloader

  • Loading branch information...
1 parent 5b43281 commit 6e5a2cb9519aab568ea0cfea2f42364de8ccf655 @tenderlove tenderlove committed Sep 23, 2013
@@ -46,8 +46,6 @@ class Preloader #:nodoc:
autoload :BelongsTo, 'active_record/associations/preloader/belongs_to'
end
- attr_reader :records, :associations, :preload_scope, :model
-
# Eager loads the named associations for the given Active Record record(s).
#
# In this description, 'association name' shall refer to the name passed
@@ -82,28 +80,20 @@ class Preloader #:nodoc:
# [ :books, :author ]
# { author: :avatar }
# [ :books, { author: :avatar } ]
- def initialize(records, associations, preload_scope = nil)
- @records = Array.wrap(records).compact.uniq
- @associations = Array.wrap(associations)
- @preload_scope = preload_scope || NULL_RELATION
- @preloaders = nil
- end
NULL_RELATION = Struct.new(:values).new({})
- def run
- preloaders.each(&:run)
- end
-
- def preloaders
- return @preloaders if @preloaders
+ def preload(records, associations, preload_scope = nil)
@WizardOfOgz

WizardOfOgz Jul 30, 2014

@tenderlove Is there any reason this method should not be a class or module method? The same question applies to the other instance methods in this class.

@rafaelfranca

rafaelfranca Jul 31, 2014

Owner

Yes, there is. Keeping it as class method make easier to us support the activerecord-deprecated_finders gem until Rails 5. I already have a branch changing this to modules that will be merged when we start Rails 5.

+ records = Array.wrap(records).compact.uniq
+ associations = Array.wrap(associations)
+ preload_scope = preload_scope || NULL_RELATION
if records.empty?
- @preloaders = []
+ []
else
- @preloaders = associations.flat_map { |association|
+ associations.flat_map { |association|
preloaders_on association, records, preload_scope
- }
+ }.each(&:run)
end
end
@@ -16,10 +16,10 @@ def associated_records_by_owner
return @associated_records_by_owner if @associated_records_by_owner
- left_loader = Preloader.new(owners,
- through_reflection.name,
- through_scope)
- left_loader.run
+ left_loader = Preloader.new
+ left_loader.preload(owners,
+ through_reflection.name,
+ through_scope)
should_reset = (through_scope != through_reflection.klass.unscoped) ||
(reflection.options[:source_type] && through_reflection.collection?)
@@ -37,13 +37,12 @@ def associated_records_by_owner
middle_records = through_records.map { |(_,rec,_)| rec }.flatten
- preloader = Preloader.new(middle_records,
- source_reflection.name,
- reflection_scope)
+ preloader = Preloader.new
+ preloaders = preloader.preload(middle_records,
+ source_reflection.name,
+ reflection_scope)
- preloader.run
-
- middle_to_pl = preloader.preloaders.each_with_object({}) do |pl,h|
+ middle_to_pl = preloaders.each_with_object({}) do |pl,h|
pl.owners.each { |middle|
h[middle] = pl
}
@@ -600,7 +600,8 @@ def exec_queries
preload = preload_values
preload += includes_values unless eager_loading?
preload.each do |associations|
- ActiveRecord::Associations::Preloader.new(@records, associations).run
+ pl = ActiveRecord::Associations::Preloader.new
+ pl.preload @records, associations
end
@records.each { |record| record.readonly! } if readonly_value

0 comments on commit 6e5a2cb

Please sign in to comment.