Merge 9397e5b into 6f4272f

CHANGELOG.md

@@ -2,6 +2,8 @@
 
 ## master
 
+- [PR [#1](https://github.com/DmitryTsepelev/ar_lazy_preload/pull/1)] Cleanup code, fix initiating child context for preload associations ([@DmitryTsepelev][])
+
 ## 0.1.0 (2018-08-01)
 
 - Initial version. ([@DmitryTsepelev][])

lib/ar_lazy_preload/associated_context_builder.rb

@@ -7,52 +7,42 @@ module ArLazyPreload
   # belonging to the same context and association name it will create and attach a new context to
   # the associated records based on the parent association tree.
   class AssociatedContextBuilder
+    # Initiates lazy preload context the records loaded lazily
+    def self.prepare(*args)
+      new(*args).perform
+    end
+
     attr_reader :parent_context, :association_name
 
+    # :parent_context - root context
+    # :association_name - lazily preloaded association name
     def initialize(parent_context:, association_name:)
       @parent_context = parent_context
       @association_name = association_name
     end
 
-    delegate :records, :association_tree, :model, to: :parent_context
-
     # Takes all the associated records for the records, attached to the :parent_context and creates
     # a preloading context for them
     def perform
-      return if child_association_tree.blank? || associated_records.blank?
+      records_by_class = parent_context.records.group_by(&:class)
 
-      Context.new(
-        model: reflection.klass,
-        records: associated_records,
-        association_tree: child_association_tree
-      )
+      associated_records = records_by_class.map do |klass, klass_records|
+        associated_records_for(klass, klass_records)
+      end.flatten
+
+      Context.register(records: associated_records, association_tree: child_association_tree)
     end
 
     private
 
     def child_association_tree
-      @child_association_tree ||= association_tree_builder.subtree_for(association_name)
-    end
-
-    def association_tree_builder
-      @association_tree_builder ||= AssociationTreeBuilder.new(association_tree)
-    end
-
-    def associated_records
-      @associated_records ||=
-        if reflection.collection?
-          record_associations.map(&:target).flatten
-        else
-          record_associations
-        end
-    end
-
-    def reflection
-      @reflection = model.reflect_on_association(association_name)
+      AssociationTreeBuilder.new(parent_context.association_tree).subtree_for(association_name)
     end
 
-    def record_associations
-      @record_associations ||= records.map { |record| record.send(association_name) }
+    def associated_records_for(klass, records)
+      record_associations = records.map { |record| record.send(association_name) }
+      reflection = klass.reflect_on_association(association_name)
+      reflection.collection? ? record_associations.map(&:target).flatten : record_associations
     end
   end
 end

lib/ar_lazy_preload/context.rb

@@ -9,13 +9,17 @@ module ArLazyPreload
   # Calling #preload_association method will cause loading of ALL associated objects for EACH
   # ecord when requested association is found in the association tree.
   class Context
-    attr_reader :model, :records, :association_tree
+    # Initiates lazy preload context for given records
+    def self.register(records:, association_tree:)
+      return if records.empty? || association_tree.empty?
+      ArLazyPreload::Context.new(records: records, association_tree: association_tree)
+    end
+
+    attr_reader :records, :association_tree
 
-    # :model - ActiveRecord class which records belong to
     # :records - array of ActiveRecord instances
     # :association_tree - list of symbols or hashes representing a tree of preloadable associations
-    def initialize(model:, records:, association_tree:)
-      @model = model
+    def initialize(records:, association_tree:)
       @records = records.compact
       @association_tree = association_tree
 
@@ -26,13 +30,8 @@ def initialize(model:, records:, association_tree:)
     # objects in the context it if needed.
     def try_preload_lazily(association_name)
       return unless association_needs_preload?(association_name)
-
       preloader.preload(records, association_name)
-
-      AssociatedContextBuilder.new(
-        parent_context: self,
-        association_name: association_name
-      ).perform
+      AssociatedContextBuilder.prepare(parent_context: self, association_name: association_name)
     end
 
     private

lib/ar_lazy_preload/ext/association.rb

@@ -4,14 +4,8 @@ module ArLazyPreload
   # ActiveRecord::Association patch with a hook for lazy preloading
   module Association
     def load_target
-      owner.try_preload_lazily(association_name)
+      owner.try_preload_lazily(reflection.name)
       super
     end
-
-    private
-
-    def association_name
-      @association_name ||= reflection.name
-    end
   end
 end

lib/ar_lazy_preload/ext/association_relation.rb

@@ -8,7 +8,7 @@ def initialize(*args)
       super(*args)
 
       context = owner.lazy_preload_context
-      return if context.blank?
+      return if context.nil?
 
       association_tree_builder = AssociationTreeBuilder.new(context.association_tree)
       subtree = association_tree_builder.subtree_for(reflection.name)

lib/ar_lazy_preload/ext/base.rb

@@ -9,9 +9,6 @@ def self.included(base)
 
     attr_accessor :lazy_preload_context
 
-    # When context has been set, this method would cause preloading association with a given name
-    def try_preload_lazily(association_name)
-      lazy_preload_context.try_preload_lazily(association_name) if lazy_preload_context.present?
-    end
+    delegate :try_preload_lazily, to: :lazy_preload_context, allow_nil: true
   end
 end

lib/ar_lazy_preload/ext/relation.rb

@@ -9,9 +9,9 @@ module Relation
     # for lazy preloading to loaded each record
     def load
       need_context = !loaded?
-      old_load_result = super
-      setup_lazy_preload_context if need_context
-      old_load_result
+      result = super
+      Context.register(records: @records, association_tree: lazy_preload_values) if need_context
+      result
     end
 
     # Specify relationships to be loaded lazily when association is loaded for the first time. For
@@ -49,15 +49,5 @@ def lazy_preload_values
     private
 
     attr_writer :lazy_preload_values
-
-    def setup_lazy_preload_context
-      return if lazy_preload_values.blank? || @records.blank?
-
-      ArLazyPreload::Context.new(
-        model: model,
-        records: @records,
-        association_tree: lazy_preload_values
-      )
-    end
   end
 end

spec/ar_lazy_preload_spec.rb

@@ -136,7 +136,7 @@
     # SELECT "posts".* FROM "posts" WHERE "posts"."user_id" IN (...)
     it "loads lazy_preloaded association" do
       expect do
-        subject.each { |comment| comment.user.posts.map(&:id) if comment.user.present? }
+        subject.each { |comment| comment.user.posts.map(&:id) }
       end.to make_database_queries(count: 3)
     end
 
@@ -147,7 +147,7 @@
     it "loads embedded lazy_preloaded association" do
       expect do
         subject.map do |comment|
-          comment.user.posts.map { |p| p.comments.map(&:id) } if comment.user.present?
+          comment.user.posts.map { |p| p.comments.map(&:id) }
         end
       end.to make_database_queries(count: 4)
     end
@@ -156,14 +156,23 @@
   describe "polymorphic" do
     include_examples "check initial loading"
 
-    subject { Vote.lazy_preload(:voteable) }
+    subject { Vote.lazy_preload(voteable: :user) }
 
     # SELECT "votes".* FROM "votes"
     # SELECT "posts".* FROM "posts" WHERE "posts"."id" IN (...)
     # SELECT "comments".* FROM "comments" WHERE "comments"."id" IN (...)
     it "loads lazy_preloaded association" do
       expect { subject.map { |vote| vote.voteable.id } }.to make_database_queries(count: 3)
     end
+
+    # SELECT "votes".* FROM "votes"
+    # SELECT "posts".* FROM "posts" WHERE "posts"."id" IN (...)
+    # SELECT "comments".* FROM "comments" WHERE "comments"."id" IN (...)
+    # SELECT "users".* FROM "users" WHERE "users"."id" IN (...) - for posts
+    # SELECT "users".* FROM "users" WHERE "users"."id" IN (...) - for comments
+    it "loads embedded lazy_preloaded association" do
+      expect { subject.map { |vote| vote.voteable.user.id } }.to make_database_queries(count: 5)
+    end
   end
 
   describe "self_join" do

spec/associated_context_builder_spec.rb

@@ -13,7 +13,6 @@
     expect(user_with_post.account).not_to be_nil
 
     parent_context = ArLazyPreload::Context.new(
-      model: User,
       records: [user_with_post, user_without_posts],
       association_tree: [{ account: :account_history }]
     )
@@ -28,7 +27,6 @@
 
   it "supports collection associations" do
     parent_context = ArLazyPreload::Context.new(
-      model: User,
       records: [user_with_post, user_without_posts],
       association_tree: [{ posts: :comments }]
     )
@@ -41,9 +39,30 @@
     user_with_post.posts.each { |post| expect(post.lazy_preload_context).not_to be_nil }
   end
 
+  it "supports polymorphic associations" do
+    post = user_with_post.posts.first
+    vote_for_post = build(:vote, user: user_without_posts, voteable: post)
+    comment = build(:comment, user: user_without_posts, post: post)
+    vote_for_comment = build(:vote, user: user_with_post, voteable: comment)
+
+    records = [vote_for_post, vote_for_comment]
+    records.each { |vote| expect(vote.voteable).not_to be_nil }
+
+    parent_context = ArLazyPreload::Context.new(
+      records: records,
+      association_tree: [voteable: :user]
+    )
+
+    described_class.new(
+      parent_context: parent_context,
+      association_name: :voteable
+    ).perform
+
+    [post, comment].each { |voteable| expect(voteable.lazy_preload_context).not_to be_nil }
+  end
+
   it "skips creating context when child association tree is blank" do
     parent_context = ArLazyPreload::Context.new(
-      model: User,
       records: [user_with_post, user_without_posts],
       association_tree: [:posts]
     )
@@ -60,7 +79,6 @@
 
   it "skips creating context when list of associated records is blank" do
     parent_context = ArLazyPreload::Context.new(
-      model: User,
       records: [user_without_posts],
       association_tree: [{ posts: :comments }]
     )

spec/context_spec.rb

@@ -13,7 +13,6 @@
 
   subject! do
     described_class.new(
-      model: User,
       records: [user_with_post, user_without_posts, nil],
       association_tree: [comments: :post]
     )

spec/helpers/factories.rb

@@ -26,4 +26,6 @@
   factory :comment do
     user
   end
+
+  factory :vote
 end

spec/helpers/schema.rb

@@ -3,45 +3,47 @@
 ActiveRecord::Schema.define do
   self.verbose = false
 
-  create_table :users, force: true, &:timestamps
+  create_table :users, force: true do |t|
+    t.timestamps null: false
+  end
 
   create_table :posts do |t|
     t.references :user, foreign_key: true
 
-    t.timestamps
+    t.timestamps null: false
   end
 
   create_table :comments do |t|
     t.references :post, foreign_key: true
     t.references :user, foreign_key: true
     t.integer :parent_comment_id, foreign_key: true, table_name: :comments
 
-    t.timestamps
+    t.timestamps null: false
   end
 
   create_table :accounts do |t|
     t.references :user, foreign_key: true
 
-    t.timestamps
+    t.timestamps null: false
   end
 
   create_table :account_histories do |t|
     t.references :account, foreign_key: true
 
-    t.timestamps
+    t.timestamps null: false
   end
 
   create_table :user_mentions do |t|
     t.references :comment
     t.references :user
 
-    t.timestamps
+    t.timestamps null: false
   end
 
   create_table :votes do |t|
     t.references :voteable, polymorphic: true, index: true
     t.references :user, foreign_key: true
 
-    t.timestamps
+    t.timestamps null: false
   end
 end