From ba7bbd390b6b4d69342a69d246b41b7abe4c6fa1 Mon Sep 17 00:00:00 2001 From: Tim Bleck Date: Thu, 15 Jun 2017 09:57:35 +0200 Subject: [PATCH] [MODEL] Fix import when preprocess returns empty collection Fixes #719 --- .../model/adapters/active_record.rb | 3 +- .../test/unit/adapter_active_record_test.rb | 37 ++++++++++++++----- 2 files changed, 29 insertions(+), 11 deletions(-) diff --git a/elasticsearch-model/lib/elasticsearch/model/adapters/active_record.rb b/elasticsearch-model/lib/elasticsearch/model/adapters/active_record.rb index 23c4267fa..aa74833d4 100644 --- a/elasticsearch-model/lib/elasticsearch/model/adapters/active_record.rb +++ b/elasticsearch-model/lib/elasticsearch/model/adapters/active_record.rb @@ -103,7 +103,8 @@ def __find_in_batches(options={}, &block) scope = scope.instance_exec(&query) if query scope.find_in_batches(options) do |batch| - yield (preprocess ? self.__send__(preprocess, batch) : batch) + batch = self.__send__(preprocess, batch) if preprocess + yield(batch) if batch.present? end end diff --git a/elasticsearch-model/test/unit/adapter_active_record_test.rb b/elasticsearch-model/test/unit/adapter_active_record_test.rb index 335e3bd10..152526e47 100644 --- a/elasticsearch-model/test/unit/adapter_active_record_test.rb +++ b/elasticsearch-model/test/unit/adapter_active_record_test.rb @@ -121,20 +121,37 @@ def ids DummyClassForActiveRecord.__find_in_batches(query: -> { where(color: "red") }) do; end end - should "preprocess the batch if option provided" do - class << DummyClassForActiveRecord - # Updates/transforms the batch while fetching it from the database - # (eg. with information from an external system) - # - def update_batch(batch) - batch.collect { |b| b.to_s + '!' } + context "when preprocessing batches" do + setup do + class << DummyClassForActiveRecord + def find_in_batches(options = {}, &block) + yield [:a, :b] + end + end + end + + should "preprocess the batch if option provided" do + class << DummyClassForActiveRecord + def update_batch(batch) + batch.collect { |b| b.to_s + '!' } + end + end + + DummyClassForActiveRecord.__find_in_batches(preprocess: :update_batch) do |batch| + assert_same_elements ["a!", "b!"], batch end end - DummyClassForActiveRecord.expects(:__find_in_batches).returns( [:a, :b] ) + should "skip batch if preprocess option provided and returns empty collection" do + class << DummyClassForActiveRecord + def update_batch(batch) + [] + end + end - DummyClassForActiveRecord.__find_in_batches(preprocess: :update_batch) do |batch| - assert_same_elements ["a!", "b!"], batch + DummyClassForActiveRecord.__find_in_batches(preprocess: :update_batch) do |batch| + flunk("block should not have been called on empty batch") + end end end