Fixed removing records when should_index? is false when reindex called on relation [skip ci]

Andrew Kane
1 parent 89a2e697
Showing 4 changed files with 18 additions and 12 deletions Show diff stats
CHANGELOG.md
lib/searchkick/bulk_indexer.rb
test/reindex_test.rb
test/should_index_test.rb
@@ -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
@@ -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
@@ -61,8 +61,6 @@ class ReindexTest &lt; 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")
@@ -39,7 +39,6 @@ class ShouldIndexTest &lt; 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
...	...	@@ -8,6 +8,7 @@
8	8	- Raise `ArgumentError` instead of `RuntimeError` for unknown operators
9	9	- Updated `searchkick_index_options` to return symbol keys (instead of mix of strings and symbols)
10	10	- Fixed issue with `merge_mappings`
	11	+- Fixed removing records when `should_index?` is `false` when `reindex` called on relation
11	12	- Removed mapping of `id` to `_id` with `order` option
12	13	- Removed `wordnet` option
13	14	- Removed `elasticsearch` dependency
...	...
...	...	@@ -14,7 +14,7 @@ module Searchkick
14	14	end
15	15
16	16	if batch
17		- import_or_update relation.to_a, method_name, async
	17	+ import_or_update relation.to_a, method_name, async, full
18	18	Searchkick.with_redis { \|r\| r.srem(batches_key, batch_id) } if batch_id
19	19	elsif full && async
20	20	full_reindex_async(relation)
...	...	@@ -30,11 +30,11 @@ module Searchkick
30	30	relation = relation.select("id").except(:includes, :preload) if async
31	31
32	32	relation.find_in_batches batch_size: batch_size do \|items\|
33		- import_or_update items, method_name, async
	33	+ import_or_update items, method_name, async, full
34	34	end
35	35	else
36	36	each_batch(relation) do \|items\|
37		- import_or_update items, method_name, async
	37	+ import_or_update items, method_name, async, full
38	38	end
39	39	end
40	40	end
...	...	@@ -57,7 +57,7 @@ module Searchkick
57	57
58	58	private
59	59
60		- def import_or_update(records, method_name, async)
	60	+ def import_or_update(records, method_name, async, full)
61	61	if records.any?
62	62	if async
63	63	Searchkick::BulkReindexJob.perform_later(
...	...	@@ -67,17 +67,25 @@ module Searchkick
67	67	method_name: method_name ? method_name.to_s : nil
68	68	)
69	69	else
70		- records = records.select(&:should_index?)
71		- if records.any?
	70	+ index_records, other_records = records.partition(&:should_index?)
	71	+
	72	+ if index_records.any?
72	73	with_retries do
73	74	# call out to index for ActiveSupport notifications
74	75	if method_name
75		- index.bulk_update(records, method_name)
	76	+ index.bulk_update(index_records, method_name)
76	77	else
77		- index.bulk_index(records)
	78	+ index.bulk_index(index_records)
78	79	end
79	80	end
80	81	end
	82	+
	83	+ if other_records.any? && !full
	84	+ with_retries do
	85	+ # call out to index for ActiveSupport notifications
	86	+ index.bulk_delete(other_records)
	87	+ end
	88	+ end
81	89	end
82	90	end
83	91	end
...	...
...	...	@@ -61,8 +61,6 @@ class ReindexTest < Minitest::Test
61	61	end
62	62
63	63	def test_relation_should_index
64		- skip "TODO make pass in Searchkick 5"
65		-
66	64	store_names ["Product A", "Product B"]
67	65	Searchkick.callbacks(false) do
68	66	Product.find_by(name: "Product B").update!(name: "DO NOT INDEX")
...	...
...	...	@@ -39,7 +39,6 @@ class ShouldIndexTest < Minitest::Test
39	39	end
40	40	Product.where(id: product.id).reindex
41	41	Product.searchkick_index.refresh
42		- # TODO fix in Searchkick 5
43		- # assert_search "index", []
	42	+ assert_search "index", []
44	43	end
45	44	end
...	...