Use after_commit hook to protect against data inconsistencies - closes #111

Andrew Kane
1 parent 321b8e2c
Showing 3 changed files with 39 additions and 13 deletions Show diff stats
README.md
lib/searchkick/model.rb
test/index_test.rb
@@ -657,25 +657,20 @@ class Product &lt; ActiveRecord::Base
 end
 ```
-Reindex conditionally
+Asynchronous reindexing
+
+**Note:** Use `after_commit` for ActiveRecord and `after_save` and `after_destroy` for Mongoid
 ```ruby
 class Product < ActiveRecord::Base
   searchkick callbacks: false
   # add the callbacks manually
-  after_save :reindex, if: proc{|model| model.name_changed? } # use your own condition
-  after_destroy :reindex
-end
-```
-Asynchronous reindexing
-
-```ruby
-class Product < ActiveRecord::Base
-  searchkick callbacks: false
+  # ActiveRecord - one callback
+  after_commit :reindex_async
-  # add the callbacks manually
+  # Mongoid - two callbacks
   after_save :reindex_async
   after_destroy :reindex_async
@@ -686,6 +681,20 @@ class Product &lt; ActiveRecord::Base
 end
 ```
+Reindex conditionally
+
+**Note:** Use with caution - (transaction rollbacks can cause data inconstencies)[https://github.com/elasticsearch/elasticsearch-rails/blob/master/elasticsearch-model/README.md#custom-callbacks] with this feature
+
+```ruby
+class Product < ActiveRecord::Base
+  searchkick callbacks: false
+
+  # add the callbacks manually
+  after_save :reindex, if: proc{|model| model.name_changed? } # use your own condition
+  after_destroy :reindex
+end
+```
+
 Reindex all models (Rails only)
 ```sh
@@ -19,8 +19,12 @@ module Searchkick
         extend Searchkick::Reindex
         include Searchkick::Similar
-        after_save :reindex, if: proc { self.class.search_callbacks? }
-        after_destroy :reindex, if: proc { self.class.search_callbacks? }
+        if respond_to?(:after_commit)
+          after_commit :reindex, if: proc{ self.class.search_callbacks? }
+        else
+          after_save :reindex, if: proc{ self.class.search_callbacks? }
+          after_destroy :reindex, if: proc{ self.class.search_callbacks? }
+        end
         def self.enable_search_callbacks
           class_variable_set :@@searchkick_callbacks, true
@@ -32,4 +32,17 @@ class TestIndex &lt; Minitest::Unit::TestCase
     assert_equal ["Dollar Tree"], Store.search(query: {match: {name: "Dollar Tree"}}).map(&:name)
   end
+  if defined?(ActiveRecord)
+
+    def test_transaction
+      Product.transaction do
+        store_names ["Product A"]
+        raise ActiveRecord::Rollback
+      end
+
+      assert_search "product", []
+    end
+
+  end
+
 end
	@@ -32,4 +32,17 @@ class TestIndex < Minitest::Unit::TestCase		@@ -32,4 +32,17 @@ class TestIndex < Minitest::Unit::TestCase
32	assert_equal ["Dollar Tree"], Store.search(query: {match: {name: "Dollar Tree"}}).map(&:name)	32	assert_equal ["Dollar Tree"], Store.search(query: {match: {name: "Dollar Tree"}}).map(&:name)
33	end	33	end
34		34
		35	+ if defined?(ActiveRecord)
		36	+
		37	+ def test_transaction
		38	+ Product.transaction do
		39	+ store_names ["Product A"]
		40	+ raise ActiveRecord::Rollback
		41	+ end
		42	+
		43	+ assert_search "product", []
		44	+ end
		45	+
		46	+ end
		47	+
35	end	48	end