MONGOID-4315 Eager load objects when criteria docs are enumerated multiple times

lib/mongoid/contextual/memory.rb

@@ -130,9 +130,7 @@ def exists?
       #
       # @since 3.0.0
       def first
-        doc = documents.first
-        eager_load_one(doc)
-        doc
+        eager_load([documents.first]).first
       end
       alias :one :first
       alias :find_first :first
@@ -165,9 +163,7 @@ def initialize(criteria)
       #
       # @since 3.0.0
       def last
-        doc = documents.last
-        eager_load_one(doc)
-        doc
+        eager_load([documents.last]).first
       end
 
       # Get the length of matching documents in the context.

lib/mongoid/contextual/mongo.rb

@@ -237,7 +237,10 @@ def find_one_and_delete
       def first
         return documents.first if cached? && cache_loaded?
         try_cache(:first) do
-          with_eager_loading(view.limit(-1).first)
+          if raw_doc = view.limit(-1).first
+            doc = Factory.from_db(klass, raw_doc, criteria.options[:fields])
+            eager_load([doc]).first
+          end
         end
       end
       alias :one :first
@@ -249,7 +252,10 @@ def first
       # @since 4.0.2
       def find_first
         return documents.first if cached? && cache_loaded?
-        with_eager_loading(view.first)
+        if raw_doc = view.first
+          doc = Factory.from_db(klass, raw_doc, criteria.options[:fields])
+          eager_load([doc]).first
+        end
       end
 
       # Execute a $geoNear command against the database.
@@ -334,7 +340,10 @@ def initialize(criteria)
       def last
         try_cache(:last) do
           with_inverse_sorting do
-            with_eager_loading(view.limit(-1).first)
+            if raw_doc = view.limit(-1).first
+              doc = Factory.from_db(klass, raw_doc, criteria.options[:fields])
+              eager_load([doc]).first
+            end
           end
         end
       end
@@ -643,15 +652,9 @@ def documents
       #
       # @since 3.0.0
       def documents_for_iteration
-        if cached? && !documents.empty?
-          documents
-        elsif eager_loadable?
-          docs = view.map{ |doc| Factory.from_db(klass, doc, criteria.options[:fields]) }
-          eager_load(docs)
-          docs
-        else
-          view
-        end
+        return documents if cached? && !documents.empty?
+        docs = view.map{ |doc| Factory.from_db(klass, doc, criteria.options[:fields]) }
+        eager_load(docs)
       end
 
       # Yield to the document.
@@ -667,10 +670,8 @@ def documents_for_iteration
       #
       # @since 3.0.0
       def yield_document(document, &block)
-        doc = document.respond_to?(:_id) ?
-          document : Factory.from_db(klass, document, criteria.options[:fields])
-        yield(doc)
-        documents.push(doc) if cacheable?
+        yield(document)
+        documents.push(document) if cacheable?
       end
     end
   end

lib/mongoid/relations/eager.rb

@@ -9,28 +9,16 @@ module Mongoid
   module Relations
     module Eager
 
-      attr_accessor :eager_loaded
-
-      def with_eager_loading(document)
-        return nil unless document
-        doc = Factory.from_db(klass, document, criteria.options[:fields])
-        eager_load_one(doc)
-        doc
-      end
-
-      def eager_load_one(doc)
-        eager_load([doc])
-      end
-
-      def eager_loadable?(document = nil)
-        return false if criteria.inclusions.empty?
-        !eager_loaded
+      def eager_loadable?
+        !criteria.inclusions.empty?
       end
 
       def eager_load(docs)
-        return false unless eager_loadable?
-        preload(criteria.inclusions, docs)
-        self.eager_loaded = true
+        docs.tap do |docs|
+          if eager_loadable?
+            preload(criteria.inclusions, docs)
+          end
+        end
       end
 
       def preload(relations, docs)

spec/mongoid/criteria_spec.rb

@@ -1504,13 +1504,6 @@ class D
             end
           end
 
-          it "does not eager load the first document" do
-            doc = criteria.first
-            expect_query(1) do
-              expect(doc.person).to eq(person)
-            end
-          end
-
           it "returns the last document" do
             expect(document).to eq(post_two)
           end
@@ -1566,13 +1559,6 @@ class D
             end
           end
 
-          it "does not eager load the last document" do
-            doc = criteria.last
-            expect_query(1) do
-              expect(doc.band).to eq(tool)
-            end
-          end
-
           it "returns the document" do
             expect(document).to eq(address_one)
           end
@@ -1598,13 +1584,6 @@ class D
             end
           end
 
-          it "does not eager load the first document" do
-            doc = criteria.first
-            expect_query(1) do
-              expect(doc.band).to eq(depeche)
-            end
-          end
-
           it "returns the document" do
             expect(document).to eq(address_two)
           end
@@ -1698,7 +1677,7 @@ class D
           end
 
           before do
-            expect(new_context).to receive(:eager_load_one).with(person).once.and_call_original
+            expect(new_context).to receive(:eager_load).with([person]).once.and_call_original
           end
 
           let!(:from_db) do
@@ -1749,7 +1728,7 @@ class D
         end
 
         before do
-          expect(context).to receive(:eager_load_one).with(person).once.and_call_original
+          expect(context).to receive(:eager_load).with([person]).once.and_call_original
         end
 
         let!(:from_db) do
@@ -2128,7 +2107,7 @@ class D
         end
 
         before do
-          expect(context).to receive(:eager_load).with([ game_one, game_two ]).once.and_call_original
+          expect(context).to receive(:preload).twice.and_call_original
         end
 
         let!(:documents) do
@@ -2191,7 +2170,7 @@ class D
       end
 
       before do
-        expect(context).to receive(:eager_load).with([ person ]).once.and_call_original
+        expect(context).to receive(:preload).twice.and_call_original
       end
 
       let!(:documents) do