Merge pull request #13938 from marcandre/sized_enumerator

Sized enumerator

activerecord/CHANGELOG.md

+*   `find_in_batches`, `find_each`, `Result#each` and `Enumerable#index_by` now
+    return an `Enumerator` that can calculate its size.
+
+    See also #13938.
+
+    *Marc-André Lafortune*
+
 *   Make sure transaction state gets reset after a commit operation on the record.
 
     If a new transaction was open inside a callback, the record was loosing track

activerecord/lib/active_record/relation/batches.rb

@@ -52,7 +52,9 @@ def find_each(options = {})
           records.each { |record| yield record }
         end
       else
-        enum_for :find_each, options
+        enum_for :find_each, options do
+          options[:start] ? where(table[primary_key].gteq(options[:start])).size : size
+        end
       end
     end
 
@@ -96,17 +98,22 @@ def find_each(options = {})
     # the batch sizes.
     def find_in_batches(options = {})
       options.assert_valid_keys(:start, :batch_size)
-      return to_enum(:find_in_batches, options) unless block_given?
 
       relation = self
+      start = options[:start]
+      batch_size = options[:batch_size] || 1000
+
+      unless block_given?
+        return to_enum(:find_in_batches, options) do
+          total = start ? where(table[primary_key].gteq(start)).size : size
+          (total - 1).div(batch_size) + 1
+        end
+      end
 
       if logger && (arel.orders.present? || arel.taken.present?)
         logger.warn("Scoped order and limit are ignored, it's forced to be batch order and batch size")
       end
 
-      start = options[:start]
-      batch_size = options[:batch_size] || 1000
-
       relation = relation.reorder(batch_order).limit(batch_size)
       records = start ? relation.where(table[primary_key].gteq(start)).to_a : relation.to_a
 

activerecord/lib/active_record/result.rb

@@ -54,7 +54,7 @@ def each
       if block_given?
         hash_rows.each { |row| yield row }
       else
-        hash_rows.to_enum
+        hash_rows.to_enum { @rows.size }
       end
     end
 

activerecord/test/cases/batches_test.rb

@@ -35,6 +35,14 @@ def test_each_should_return_an_enumerator_if_no_block_is_present
     end
   end
 
+  if Enumerator.method_defined? :size
+    def test_each_should_return_a_sized_enumerator
+      assert_equal 11, Post.find_each(:batch_size => 1).size
+      assert_equal 5, Post.find_each(:batch_size => 2, :start => 7).size
+      assert_equal 11, Post.find_each(:batch_size => 10_000).size
+    end
+  end
+
   def test_each_enumerator_should_execute_one_query_per_batch
     assert_queries(@total + 1) do
       Post.find_each(:batch_size => 1).with_index do |post, index|
@@ -191,4 +199,14 @@ def test_find_in_batches_should_return_an_enumerator
       end
     end
   end
+
+  if Enumerator.method_defined? :size
+    def test_find_in_batches_should_return_a_sized_enumerator
+      assert_equal 11, Post.find_in_batches(:batch_size => 1).size
+      assert_equal 6, Post.find_in_batches(:batch_size => 2).size
+      assert_equal 4, Post.find_in_batches(:batch_size => 2, :start => 4).size
+      assert_equal 4, Post.find_in_batches(:batch_size => 3).size
+      assert_equal 1, Post.find_in_batches(:batch_size => 10_000).size
+    end
+  end
 end

activerecord/test/cases/result_test.rb

@@ -5,14 +5,16 @@ class ResultTest < ActiveRecord::TestCase
     def result
       Result.new(['col_1', 'col_2'], [
         ['row 1 col 1', 'row 1 col 2'],
-        ['row 2 col 1', 'row 2 col 2']
+        ['row 2 col 1', 'row 2 col 2'],
+        ['row 3 col 1', 'row 3 col 2'],
       ])
     end
 
     def test_to_hash_returns_row_hashes
       assert_equal [
         {'col_1' => 'row 1 col 1', 'col_2' => 'row 1 col 2'},
-        {'col_1' => 'row 2 col 1', 'col_2' => 'row 2 col 2'}
+        {'col_1' => 'row 2 col 1', 'col_2' => 'row 2 col 2'},
+        {'col_1' => 'row 3 col 1', 'col_2' => 'row 3 col 2'},
       ], result.to_hash
     end
 
@@ -28,5 +30,11 @@ def test_each_without_block_returns_an_enumerator
         assert_kind_of Integer, index
       end
     end
+
+    if Enumerator.method_defined? :size
+      def test_each_without_block_returns_a_sized_enumerator
+        assert_equal 3, result.each.size
+      end
+    end
   end
 end

activesupport/lib/active_support/core_ext/enumerable.rb

@@ -35,7 +35,7 @@ def index_by
     if block_given?
       Hash[map { |elem| [yield(elem), elem] }]
     else
-      to_enum :index_by
+      to_enum(:index_by) { size if respond_to?(:size) }
     end
   end
 

activesupport/test/core_ext/enumerable_test.rb

@@ -8,7 +8,6 @@ def +(p) self.class.new(price + p.price) end
 end
 
 class EnumerableTests < ActiveSupport::TestCase
-  Enumerator = [].each.class
 
   class GenericEnumerable
     include Enumerable
@@ -21,26 +20,6 @@ def each
     end
   end
 
-  def test_group_by
-    names = %w(marcel sam david jeremy)
-    klass = Struct.new(:name)
-    objects = (1..50).map do
-      klass.new names.sample
-    end
-
-    enum = GenericEnumerable.new(objects)
-    grouped = enum.group_by { |object| object.name }
-
-    grouped.each do |name, group|
-      assert group.all? { |person| person.name == name }
-    end
-
-    assert_equal objects.uniq.map(&:name), grouped.keys
-    assert({}.merge(grouped), "Could not convert ActiveSupport::OrderedHash into Hash")
-    assert_equal Enumerator, enum.group_by.class
-    assert_equal grouped, enum.group_by.each(&:name)
-  end
-
   def test_sums
     enum = GenericEnumerable.new([5, 15, 10])
     assert_equal 30, enum.sum
@@ -94,6 +73,10 @@ def test_index_by
     assert_equal({ 5 => Payment.new(5), 15 => Payment.new(15), 10 => Payment.new(10) },
                  payments.index_by { |p| p.price })
     assert_equal Enumerator, payments.index_by.class
+    if Enumerator.method_defined? :size
+      assert_equal nil, payments.index_by.size
+      assert_equal 42, (1..42).index_by.size
+    end
     assert_equal({ 5 => Payment.new(5), 15 => Payment.new(15), 10 => Payment.new(10) },
                  payments.index_by.each { |p| p.price })
   end