From 5ddb69302211e9a95c5e762ed8e68607e4384ba9 Mon Sep 17 00:00:00 2001 From: Andrew Kane Date: Sat, 19 Feb 2022 13:38:25 -0800 Subject: [PATCH] Fixed removing records when should_index? is false when reindex called on relation [skip ci] --- CHANGELOG.md | 1 + lib/searchkick/bulk_indexer.rb | 24 ++++++++++++++++-------- test/reindex_test.rb | 2 -- test/should_index_test.rb | 3 +-- 4 files changed, 18 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 678f0b1..fe1f422 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ - Raise `ArgumentError` instead of `RuntimeError` for unknown operators - Updated `searchkick_index_options` to return symbol keys (instead of mix of strings and symbols) - Fixed issue with `merge_mappings` +- Fixed removing records when `should_index?` is `false` when `reindex` called on relation - Removed mapping of `id` to `_id` with `order` option - Removed `wordnet` option - Removed `elasticsearch` dependency diff --git a/lib/searchkick/bulk_indexer.rb b/lib/searchkick/bulk_indexer.rb index dc45c1b..7bbef31 100644 --- a/lib/searchkick/bulk_indexer.rb +++ b/lib/searchkick/bulk_indexer.rb @@ -14,7 +14,7 @@ module Searchkick end if batch - import_or_update relation.to_a, method_name, async + import_or_update relation.to_a, method_name, async, full Searchkick.with_redis { |r| r.srem(batches_key, batch_id) } if batch_id elsif full && async full_reindex_async(relation) @@ -30,11 +30,11 @@ module Searchkick relation = relation.select("id").except(:includes, :preload) if async relation.find_in_batches batch_size: batch_size do |items| - import_or_update items, method_name, async + import_or_update items, method_name, async, full end else each_batch(relation) do |items| - import_or_update items, method_name, async + import_or_update items, method_name, async, full end end end @@ -57,7 +57,7 @@ module Searchkick private - def import_or_update(records, method_name, async) + def import_or_update(records, method_name, async, full) if records.any? if async Searchkick::BulkReindexJob.perform_later( @@ -67,17 +67,25 @@ module Searchkick method_name: method_name ? method_name.to_s : nil ) else - records = records.select(&:should_index?) - if records.any? + index_records, other_records = records.partition(&:should_index?) + + if index_records.any? with_retries do # call out to index for ActiveSupport notifications if method_name - index.bulk_update(records, method_name) + index.bulk_update(index_records, method_name) else - index.bulk_index(records) + index.bulk_index(index_records) end end end + + if other_records.any? && !full + with_retries do + # call out to index for ActiveSupport notifications + index.bulk_delete(other_records) + end + end end end end diff --git a/test/reindex_test.rb b/test/reindex_test.rb index 14e65f9..1ecbcc8 100644 --- a/test/reindex_test.rb +++ b/test/reindex_test.rb @@ -61,8 +61,6 @@ class ReindexTest < Minitest::Test end def test_relation_should_index - skip "TODO make pass in Searchkick 5" - store_names ["Product A", "Product B"] Searchkick.callbacks(false) do Product.find_by(name: "Product B").update!(name: "DO NOT INDEX") diff --git a/test/should_index_test.rb b/test/should_index_test.rb index 1dafed2..9d5083d 100644 --- a/test/should_index_test.rb +++ b/test/should_index_test.rb @@ -39,7 +39,6 @@ class ShouldIndexTest < Minitest::Test end Product.where(id: product.id).reindex Product.searchkick_index.refresh - # TODO fix in Searchkick 5 - # assert_search "index", [] + assert_search "index", [] end end -- libgit2 0.21.0