Merge branch 'feature/issue-555-fix' into rails-3-develop

2 files changed, 54 insertions, 20 deletions

diff --git a/spec/models/xapian_spec.rb b/spec/models/xapian_spec.rb
index 7aab9cdc6..c7c21e3a0 100644
--- a/spec/models/xapian_spec.rb
+++ b/spec/models/xapian_spec.rb
@@ -400,3 +400,34 @@ describe ActsAsXapian::Search, "#words_to_highlight" do
     end
 
 end
+
+describe InfoRequestEvent, " when faced with a race condition during xapian_mark_needs_index" do
+
+    before(:each) do
+        load_raw_emails_data
+        get_fixtures_xapian_index
+        # Use the before create job hook to simulate a race condition with another process
+        # by creating an acts_as_xapian_job record for the same model
+        class InfoRequestEvent
+            def xapian_before_create_job_hook(action, model, model_id)
+                ActsAsXapian::ActsAsXapianJob.create!(:model => model,
+                                                      :model_id => model_id,
+                                                      :action => action)
+            end
+        end
+    end
+
+    after(:each) do
+        # Reset the before create job hook
+        class InfoRequestEvent
+            def xapian_before_create_job_hook(action, model, model_id)
+            end
+        end
+    end
+
+    it 'should not raise an error but should fail silently' do
+        ir = info_requests(:naughty_chicken_request)
+        ir.reindex_request_events
+    end
+
+end
diff --git a/vendor/plugins/acts_as_xapian/lib/acts_as_xapian.rb b/vendor/plugins/acts_as_xapian/lib/acts_as_xapian.rb
index f2cd1075c..fcf7a778d 100644
--- a/vendor/plugins/acts_as_xapian/lib/acts_as_xapian.rb
+++ b/vendor/plugins/acts_as_xapian/lib/acts_as_xapian.rb
@@ -913,16 +913,11 @@ module ActsAsXapian
 
         # Used to mark changes needed by batch indexer
         def xapian_mark_needs_index
-            model = self.class.base_class.to_s
-            model_id = self.id
-            ActiveRecord::Base.transaction do
-                found = ActsAsXapianJob.delete_all([ "model = ? and model_id = ?", model, model_id])
-                job = ActsAsXapianJob.new
-                job.model = model
-                job.model_id = model_id
-                job.action = 'update'
-                job.save!
-            end
+            xapian_create_job('update', self.class.base_class.to_s, self.id)
+        end
+
+        def xapian_mark_needs_destroy
+            xapian_create_job('destroy', self.class.base_class.to_s, self.id)
         end
 
         # Allow reindexing to be skipped if a flag is set
@@ -931,18 +926,26 @@ module ActsAsXapian
             xapian_mark_needs_index
         end
 
-        def xapian_mark_needs_destroy
-            model = self.class.base_class.to_s
-            model_id = self.id
-            ActiveRecord::Base.transaction do
-                found = ActsAsXapianJob.delete_all([ "model = ? and model_id = ?", model, model_id])
-                job = ActsAsXapianJob.new
-                job.model = model
-                job.model_id = model_id
-                job.action = 'destroy'
-                job.save!
+        def xapian_create_job(action, model, model_id)
+            begin
+                ActiveRecord::Base.transaction do
+                    ActsAsXapianJob.delete_all([ "model = ? and model_id = ?", model, model_id])
+                    xapian_before_create_job_hook(action, model, model_id)
+                    ActsAsXapianJob.create!(:model => model,
+                                            :model_id => model_id,
+                                            :action => action)
+                end
+            rescue ActiveRecord::RecordNotUnique => e
+                # Given the error handling in ActsAsXapian::update_index, we can just fail silently if
+                # another process has inserted an acts_as_xapian_jobs record for this model.
+                raise unless (e.message =~ /duplicate key value violates unique constraint "index_acts_as_xapian_jobs_on_model_and_model_id"/)
             end
         end
+
+        # A hook method that can be used in tests to simulate e.g. an external process inserting a record
+        def xapian_before_create_job_hook(action, model, model_id)
+        end
+
      end
 
     ######################################################################