Fixes when eager loading many different types

.rubocop.yml

@@ -0,0 +1,11 @@
+require:
+  - rubocop-rspec
+
+AllCops:
+  NewCops: enable
+
+Metrics/BlockLength:
+  IgnoredMethods: ['describe', 'context']
+
+Style/Documentation:
+  Enabled: No

lib/no_brainer/document/association/eager_loader.rb

@@ -1,17 +1,25 @@
+# frozen_string_literal: true
+
 module NoBrainer::Document::Association::EagerLoader
   extend self
 
   module Generic
     # Used in associations to declare generic eager loading capabilities
     # The association should implement loaded?, preload,
     # eager_load_owner_key and eager_load_target_key.
-    def eager_load(docs, additional_criteria=nil)
+    def eager_load(docs, additional_criteria = nil)
       owner_key  = eager_load_owner_key
       owner_type = eager_load_owner_type
       target_key = eager_load_target_key
 
       if is_a?(NoBrainer::Document::Association::BelongsTo::Metadata) && owner_type
         target_class = docs.first.__send__(owner_type)
+
+        if docs.detect { |doc| doc.__send__(owner_type) != target_class }
+          raise NoBrainer::Error::PolymorphicAssociationWithDifferentTypes,
+                "The documents to be eager loaded doesn't have the same " \
+                'type, which is not supported'
+        end
       end
 
       criteria = target_class ? base_criteria(target_class) : base_criteria

lib/no_brainer/error.rb

@@ -1,19 +1,21 @@
+# frozen_string_literal: true
+
 module NoBrainer::Error
-  class Connection              < RuntimeError; end
-  class DocumentNotFound        < RuntimeError; end
-  class DocumentNotPersisted    < RuntimeError; end
-  class ChildrenExist           < RuntimeError; end
-  class CannotUseIndex          < RuntimeError; end
-  class MissingIndex            < RuntimeError; end
-  class AssociationNotPersisted < RuntimeError; end
-  class ReadonlyField           < RuntimeError; end
-  class MissingAttribute        < RuntimeError; end
-  class UnknownAttribute        < RuntimeError; end
-  class AtomicBlock             < RuntimeError; end
-  class LostLock                < RuntimeError; end
-  class LockInvalidOp           < RuntimeError; end
-  class LockUnavailable         < RuntimeError; end
-  class InvalidPolymorphicType  < RuntimeError; end
+  class AssociationNotPersisted                   < RuntimeError; end
+  class AtomicBlock                               < RuntimeError; end
+  class ChildrenExist                             < RuntimeError; end
+  class Connection                                < RuntimeError; end
+  class DocumentNotFound                          < RuntimeError; end
+  class DocumentNotPersisted                      < RuntimeError; end
+  class InvalidPolymorphicType                    < RuntimeError; end
+  class LockInvalidOp                             < RuntimeError; end
+  class LostLock                                  < RuntimeError; end
+  class LockUnavailable                           < RuntimeError; end
+  class MissingAttribute                          < RuntimeError; end
+  class MissingIndex                              < RuntimeError; end
+  class PolymorphicAssociationWithDifferentTypes  < RuntimeError; end
+  class ReadonlyField                             < RuntimeError; end
+  class UnknownAttribute                          < RuntimeError; end
 
   class DocumentInvalid < RuntimeError
     attr_accessor :instance

spec/integration/associations/belongs_to_polymorphic_spec.rb

@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 require 'spec_helper'
 
 describe 'belongs_to polymorphic' do
@@ -30,15 +32,28 @@
     end
   end
 
-  context 'when eager loading on a belongs_to association' do
+  context 'when eager loading on a belongs_to association with all documents ' \
+          'from the same root class' do
     it 'eagers load' do
-      expect(NoBrainer).to receive(:run).and_call_original.exactly(2).times
+      expect(NoBrainer).to receive(:run).and_call_original.twice
+
       Picture.eager_load(:imageable).each do |picture|
         expect(picture.imageable).to eql(restaurant)
       end
     end
   end
 
+  context 'when eager loading on a belongs_to association with at least one ' \
+          'document with a different root class' do
+    before { Picture.create(imageable: event, mime: 'image/png') }
+
+    it 'eagers load' do
+      expect do
+        Image.eager_load(:imageable).to_a
+      end.to raise_error(NoBrainer::Error::PolymorphicAssociationWithDifferentTypes)
+    end
+  end
+
   context 'foreign_type is nil' do
     context 'target is loaded' do
       it 'returns the target' do

spec/integration/criteria/eager_loading_spec.rb

@@ -1,133 +1,172 @@
+# frozen_string_literal: true
+
 require 'spec_helper'
 
 describe 'eager_loading' do
   before { load_blog_models }
 
-  let!(:author)   { Author.create  }
-  let!(:posts)    { 3.times.map { |i| Post.create(:author => author, :title => i) } }
-  let!(:comments) { 3.times.map { |i| 3.times.map { |j| Comment.create(:post => author.posts[i], :body => j) } }.flatten }
+  let!(:author)   { Author.create! }
+  let!(:posts)    do
+    3.times.map { |i| Post.create!(:author => author, :title => i) }
+  end
+  let!(:comments) do
+    3.times.map do |i|
+      3.times.map do |j|
+        Comment.create!(:post => author.posts[i], :body => j)
+      end
+    end.flatten
+  end
 
   context 'when eager loading on a belongs_to association' do
     it 'eagers load' do
-      expect(NoBrainer).to receive(:run).and_call_original.exactly(2).times
+      expect(NoBrainer).to receive(:run).and_call_original.twice
+
       Comment.eager_load(:post).each do |comment|
-        comment.post.should == comments.select { |c| c == comment }.first.post
+        expect(comment.post).to eql(comments.select { |c| c == comment }.first.post)
       end
     end
   end
 
   context 'when eager loading on a has_many association' do
     it 'eagers load' do
-      expect(NoBrainer).to receive(:run).and_call_original.exactly(2).times
+      expect(NoBrainer).to receive(:run).and_call_original.twice
+
       Post.eager_load(:comments).each do |post|
-        post.comments.to_a.should == comments.select { |c| c.post == post }
+        expect(post.comments.to_a).to eql(comments.select { |c| c.post == post })
       end
     end
   end
 
   context 'when eager loading nested associations' do
     it 'eager loads' do
-      expect(NoBrainer).to receive(:run).and_call_original.exactly(3).times
+      expect(NoBrainer).to receive(:run).and_call_original.thrice
+
       a = Author.eager_load(:posts => [:author, :comments]).first
-      a.should == author
-      a.posts.to_a.should == posts
+
+      expect(a).to eql(author)
+      expect(a.posts.to_a).to eql(posts)
       a.posts.each do |post|
-        post.author.should == author
-        post.comments.to_a.should == comments.select { |c| c.post == post }
+        expect(post.author).to eql(author)
+        expect(post.comments.to_a).to eql(comments.select { |c| c.post == post })
       end
     end
   end
 
   context 'when eager loading nested associations with multiple eager_load' do
     it 'eager loads' do
-      expect(NoBrainer).to receive(:run).and_call_original.exactly(3).times
+      expect(NoBrainer).to receive(:run).and_call_original.thrice
+
       a = Author.eager_load(:posts => :author).eager_load(:posts => :comments).first
-      a.should == author
-      a.posts.to_a.should == posts
+
+      expect(a).to eql(author)
+      expect(a.posts.to_a).to eql(posts)
       a.posts.each do |post|
-        post.author.should == author
-        post.comments.to_a.should == comments.select { |c| c.post == post }
+        expect(post.author).to eql(author)
+        expect(post.comments.to_a).to eql(comments.select { |c| c.post == post })
       end
     end
   end
 
   context 'when eager loading after the fact' do
     it 'eager loads' do
-      expect(NoBrainer).to receive(:run).and_call_original.exactly(3).times
+      expect(NoBrainer).to receive(:run).and_call_original.thrice
+
       a = Author.eager_load(:posts => :comments).first
-      a.should == author
-      a.posts.to_a.should == posts
+
+      expect(a).to eql(author)
+      expect(a.posts.to_a).to eql(posts)
       a.posts.eager_load(:author).each do |post|
-        post.author.should == author
-        post.comments.to_a.should == comments.select { |c| c.post == post }
+        expect(post.author).to eql(author)
+        expect(post.comments.to_a).to eql(comments.select { |c| c.post == post })
       end
     end
   end
 
   context 'when eager loading after the fact on top of an existing eager load' do
     it 'eager loads' do
-      expect(NoBrainer).to receive(:run).and_call_original.exactly(3).times
+      expect(NoBrainer).to receive(:run).and_call_original.thrice
+
       a = Author.eager_load(:posts => [:author, :comments]).first
-      a.should == author
-      a.posts.to_a.should == posts
+
+      expect(a).to eql(author)
+      expect(a.posts.to_a).to eql(posts)
       a.posts.eager_load(:comments).each do |post|
-        post.author.should == author
-        post.comments.to_a.should == comments.select { |c| c.post == post }
+        expect(post.author).to eql(author)
+        expect(post.comments.to_a).to eql(comments.select { |c| c.post == post })
       end
     end
   end
 
   context 'when eager loading after the fact on top of a cached criteria' do
     it 'eager loads' do
-      expect(NoBrainer).to receive(:run).and_call_original.exactly(3).times
+      expect(NoBrainer).to receive(:run).and_call_original.thrice
+
       a = Author.first
-      a.should == author
-      a.posts.to_a.should == posts
+
+      expect(a).to eql(author)
+      expect(a.posts.to_a).to eql(posts)
+
       criteria = a.posts.eager_load(:comments)
+
       ([criteria.first] + criteria.to_a).each do |post|
-        post.comments.to_a.should == comments.select { |c| c.post == post }
+        expect(post.comments.to_a).to eql(comments.select { |c| c.post == post })
       end
     end
   end
 
   context 'when eager loading nested associations with criterias' do
     it 'eager loads' do
-      expect(NoBrainer).to receive(:run).and_call_original.exactly(3).times
-      a = Author.eager_load(:posts => Post.where(:title.gte => 1).eager_load(
-                            :author, :comments => Comment.where(:body.gte => 1))).first
-      a.should == author
-      a.posts.to_a.should == posts.select { |p| p.title >= 1 }
+      expect(NoBrainer).to receive(:run).and_call_original.thrice
+
+      a = Author.eager_load(
+        :posts => Post.where(:title.gte => 1).eager_load(
+          :author,
+          :comments => Comment.where(:body.gte => 1)
+        )
+      ).first
+
+      expect(a).to eql(author)
+      expect(a.posts.to_a).to eql(posts.select { |p| p.title >= 1 })
+
       a.posts.each do |post|
-        post.author.should == author
-        post.comments.to_a.should == comments.select { |c| c.post == post && c.body >= 1 }
+        expect(post.author).to eql(author)
+        expect(post.comments.to_a).to eql(comments.select { |c| c.post == post && c.body >= 1 })
       end
     end
   end
 
   context 'when eager loading scoped associations' do
-    before { Author.has_many :posts, :scope => ->{ where(:title.gte 1) } }
-    before { Post.has_many :comments, :scope => ->{ where(:body.gte 1) } }
+    before do
+      Author.has_many :posts, :scope => ->{ where(:title.gte 1) }
+      Post.has_many :comments, :scope => ->{ where(:body.gte 1) }
+    end
 
     it 'eager loads' do
-      expect(NoBrainer).to receive(:run).and_call_original.exactly(3).times
+      expect(NoBrainer).to receive(:run).and_call_original.thrice
+
       a = Author.eager_load(:posts => [:author, :comments]).first
-      a.should == author
-      a.posts.to_a.should == posts[1..2]
+
+      expect(a).to eql(author)
+      expect(a.posts.to_a).to eql(posts[1..2])
+
       a.posts.each do |post|
-        post.author.should == author
-        post.comments.to_a.should == comments.select { |c| c.post == post }[1..2]
+        expect(post.author).to eql(author)
+        expect(post.comments.to_a).to eql(comments.select { |c| c.post == post }[1..2])
       end
     end
   end
 
   context 'when eager loading an array of docs with NoBrainer.eager_load' do
     it 'eager loads' do
-      expect(NoBrainer).to receive(:run).and_call_original.exactly(2).times
+      expect(NoBrainer).to receive(:run).and_call_original.twice
+
       comments = Comment.all.to_a
-      comments = comments + comments
+
+      comments += comments
       NoBrainer.eager_load(comments, :post)
+
       comments.each do |comment|
-        comment.post.should == comments.select { |c| c == comment }.first.post
+        expect(comment.post).to eql(comments.select { |c| c == comment }.first.post)
       end
     end
   end

spec/support/models.rb

@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 require 'yaml'
 
 module ModelsHelper
@@ -124,33 +126,33 @@ def load(s)
       field :partner_name
       field :partner_birthday
 
-      store :params, accessors: [ :token ], coder: YAML
-      store :settings, accessors: [ :color, :homepage ]
+      store :params, accessors: [:token], coder: YAML
+      store :settings, accessors: %i[color homepage]
       store_accessor :settings, :favorite_food
-      store :parent, accessors: [:birthday, :name], prefix: true
+      store :parent, accessors: %i[birthday name], prefix: true
       store :spouse, accessors: [:birthday], prefix: :partner
       store_accessor :spouse, :name, prefix: :partner
-      store :configs, accessors: [ :secret_question ]
-      store :configs, accessors: [ :two_factor_auth ], suffix: true
+      store :configs, accessors: [:secret_question]
+      store :configs, accessors: [:two_factor_auth], suffix: true
       store_accessor :configs, :login_retry, suffix: :config
-      store :preferences, accessors: [ :remember_login ]
-      store :json_data, accessors: [ :height, :weight ], coder: Coder.new
-      store :json_data_empty, accessors: [ :is_a_good_guy ], coder: Coder.new
+      store :preferences, accessors: [:remember_login]
+      store :json_data, accessors: %i[height weight], coder: Coder.new
+      store :json_data_empty, accessors: [:is_a_good_guy], coder: Coder.new
 
       def phone_number
         read_store_attribute(:settings, :phone_number).gsub(/(\d{3})(\d{3})(\d{4})/, '(\1) \2-\3')
       end
 
       def phone_number=(value)
-        write_store_attribute(:settings, :phone_number, value && value.gsub(/[^\d]/, ""))
+        write_store_attribute(:settings, :phone_number, value && value.gsub(/[^\d]/, ''))
       end
 
       def color
-        super || "red"
+        super || 'red'
       end
 
       def color=(value)
-        value = "blue" unless %w(black red green blue).include?(value)
+        value = 'blue' unless %w[black red green blue].include?(value)
         super
       end
     end