Merge branch 'danielwestendorf-fix-uuid-async-reindex'

Andrew Kane
2 parents 131ed0bb 8b6d067e
Showing 3 changed files with 63 additions and 8 deletions Show diff stats
lib/searchkick/index.rb
test/reindex_test.rb
test/test_helper.rb
@@ -420,14 +420,25 @@ module Searchkick
       if scope.respond_to?(:primary_key)
         # TODO expire Redis key
         primary_key = scope.primary_key
-        starting_id = scope.minimum(primary_key) || 0
-        max_id = scope.maximum(primary_key) || 0
-        batches_count = ((max_id - starting_id + 1) / batch_size.to_f).ceil
-
-        batches_count.times do |i|
-          batch_id = i + 1
-          min_id = starting_id + (i * batch_size)
-          bulk_reindex_job scope, batch_id, min_id: min_id, max_id: min_id + batch_size - 1
+
+        starting_id = scope.minimum(primary_key)
+        if starting_id.nil?
+          # no records, do nothing
+        elsif starting_id.is_a?(Numeric)
+          max_id = scope.maximum(primary_key)
+          batches_count = ((max_id - starting_id + 1) / batch_size.to_f).ceil
+
+          batches_count.times do |i|
+            batch_id = i + 1
+            min_id = starting_id + (i * batch_size)
+            bulk_reindex_job scope, batch_id, min_id: min_id, max_id: min_id + batch_size - 1
+          end
+        else
+          scope.find_in_batches(batch_size: batch_size).each_with_index do |batch, i|
+            batch_id = i + 1
+
+            bulk_reindex_job scope, batch_id, record_ids: batch.map { |record| record.id.to_s }
+          end
         end
       else
         batch_id = 1
@@ -39,6 +39,18 @@ class ReindexTest &lt; Minitest::Test
     assert_search "product", ["Product A"]
   end
  
+  def test_async_non_integer_pk
+    skip if !defined?(ActiveJob)
+
+    Sku.create(id: SecureRandom.hex, name: "Test")
+    reindex = Sku.reindex(async: true)
+    assert_search "sku", [], conversions: false
+
+    index = Searchkick::Index.new(reindex[:index_name])
+    index.refresh
+    assert_equal 1, index.total_docs
+  end
+
   def test_refresh_interval
     reindex = Product.reindex(refresh_interval: "30s", async: true, import: false)
     index = Searchkick::Index.new(reindex[:index_name])
@@ -111,6 +111,12 @@ if defined?(Mongoid)
  
   class Cat < Animal
   end
+
+  class Sku
+    include Mongoid::Document
+
+    field :name
+  end
 elsif defined?(NoBrainer)
   NoBrainer.configure do |config|
     config.app_name = :searchkick
@@ -171,6 +177,13 @@ elsif defined?(NoBrainer)
  
   class Cat < Animal
   end
+
+  class Sku
+    include NoBrainer::Document
+
+    field :id,   type: String
+    field :name, type: String
+  end
 elsif defined?(Cequel)
   cequel =
     Cequel.connect(
@@ -252,6 +265,13 @@ elsif defined?(Cequel)
   class Cat < Animal
   end
  
+  class Sku
+    include Cequel::Record
+
+    key :id, :uuid
+    column :name, :text
+  end
+
   [Product, Store, Region, Speaker, Animal].each(&:synchronize_schema)
 else
   require "active_record"
@@ -339,6 +359,10 @@ else
     t.string :type
   end
  
+  ActiveRecord::Migration.create_table :skus, id: :uuid do |t|
+    t.string :name
+  end
+
   class Product < ActiveRecord::Base
     belongs_to :store
   end
@@ -361,6 +385,9 @@ else
  
   class Cat < Animal
   end
+
+  class Sku < ActiveRecord::Base
+  end
 end
  
 class Product
@@ -477,6 +504,10 @@ class Animal
     # wordnet: true
 end
  
+class Sku
+  searchkick callbacks: defined?(ActiveJob) ? :async : true
+end
+
 Product.searchkick_index.delete if Product.searchkick_index.exists?
 Product.reindex
 Product.reindex # run twice for both index paths
@@ -493,6 +524,7 @@ class Minitest::Test
     Store.destroy_all
     Animal.destroy_all
     Speaker.destroy_all
+    Sku.destroy_all
   end
  
   protected
...	...	@@ -420,14 +420,25 @@ module Searchkick
420	420	if scope.respond_to?(:primary_key)
421	421	# TODO expire Redis key
422	422	primary_key = scope.primary_key
423		- starting_id = scope.minimum(primary_key) \|\| 0
424		- max_id = scope.maximum(primary_key) \|\| 0
425		- batches_count = ((max_id - starting_id + 1) / batch_size.to_f).ceil
426		-
427		- batches_count.times do \|i\|
428		- batch_id = i + 1
429		- min_id = starting_id + (i * batch_size)
430		- bulk_reindex_job scope, batch_id, min_id: min_id, max_id: min_id + batch_size - 1
	423	+
	424	+ starting_id = scope.minimum(primary_key)
	425	+ if starting_id.nil?
	426	+ # no records, do nothing
	427	+ elsif starting_id.is_a?(Numeric)
	428	+ max_id = scope.maximum(primary_key)
	429	+ batches_count = ((max_id - starting_id + 1) / batch_size.to_f).ceil
	430	+
	431	+ batches_count.times do \|i\|
	432	+ batch_id = i + 1
	433	+ min_id = starting_id + (i * batch_size)
	434	+ bulk_reindex_job scope, batch_id, min_id: min_id, max_id: min_id + batch_size - 1
	435	+ end
	436	+ else
	437	+ scope.find_in_batches(batch_size: batch_size).each_with_index do \|batch, i\|
	438	+ batch_id = i + 1
	439	+
	440	+ bulk_reindex_job scope, batch_id, record_ids: batch.map { \|record\| record.id.to_s }
	441	+ end
431	442	end
432	443	else
433	444	batch_id = 1
...	...
...	...	@@ -39,6 +39,18 @@ class ReindexTest < Minitest::Test
39	39	assert_search "product", ["Product A"]
40	40	end
41	41
	42	+ def test_async_non_integer_pk
	43	+ skip if !defined?(ActiveJob)
	44	+
	45	+ Sku.create(id: SecureRandom.hex, name: "Test")
	46	+ reindex = Sku.reindex(async: true)
	47	+ assert_search "sku", [], conversions: false
	48	+
	49	+ index = Searchkick::Index.new(reindex[:index_name])
	50	+ index.refresh
	51	+ assert_equal 1, index.total_docs
	52	+ end
	53	+
42	54	def test_refresh_interval
43	55	reindex = Product.reindex(refresh_interval: "30s", async: true, import: false)
44	56	index = Searchkick::Index.new(reindex[:index_name])
...	...
...	...	@@ -111,6 +111,12 @@ if defined?(Mongoid)
111	111
112	112	class Cat < Animal
113	113	end
	114	+
	115	+ class Sku
	116	+ include Mongoid::Document
	117	+
	118	+ field :name
	119	+ end
114	120	elsif defined?(NoBrainer)
115	121	NoBrainer.configure do \|config\|
116	122	config.app_name = :searchkick
...	...	@@ -171,6 +177,13 @@ elsif defined?(NoBrainer)
171	177
172	178	class Cat < Animal
173	179	end
	180	+
	181	+ class Sku
	182	+ include NoBrainer::Document
	183	+
	184	+ field :id, type: String
	185	+ field :name, type: String
	186	+ end
174	187	elsif defined?(Cequel)
175	188	cequel =
176	189	Cequel.connect(
...	...	@@ -252,6 +265,13 @@ elsif defined?(Cequel)
252	265	class Cat < Animal
253	266	end
254	267
	268	+ class Sku
	269	+ include Cequel::Record
	270	+
	271	+ key :id, :uuid
	272	+ column :name, :text
	273	+ end
	274	+
255	275	[Product, Store, Region, Speaker, Animal].each(&:synchronize_schema)
256	276	else
257	277	require "active_record"
...	...	@@ -339,6 +359,10 @@ else
339	359	t.string :type
340	360	end
341	361
	362	+ ActiveRecord::Migration.create_table :skus, id: :uuid do \|t\|
	363	+ t.string :name
	364	+ end
	365	+
342	366	class Product < ActiveRecord::Base
343	367	belongs_to :store
344	368	end
...	...	@@ -361,6 +385,9 @@ else
361	385
362	386	class Cat < Animal
363	387	end
	388	+
	389	+ class Sku < ActiveRecord::Base
	390	+ end
364	391	end
365	392
366	393	class Product
...	...	@@ -477,6 +504,10 @@ class Animal
477	504	# wordnet: true
478	505	end
479	506
	507	+class Sku
	508	+ searchkick callbacks: defined?(ActiveJob) ? :async : true
	509	+end
	510	+
480	511	Product.searchkick_index.delete if Product.searchkick_index.exists?
481	512	Product.reindex
482	513	Product.reindex # run twice for both index paths
...	...	@@ -493,6 +524,7 @@ class Minitest::Test
493	524	Store.destroy_all
494	525	Animal.destroy_all
495	526	Speaker.destroy_all
	527	+ Sku.destroy_all
496	528	end
497	529
498	530	protected
...	...