Fix for an aborted transaction not being rolled-back

We were seeing errors in xapian_create_job that indicated that the
current transaction had been aborted but not rolled-back.  It seems
that this happened because sometimes xapian_create_job was called
from within another transaction (e.g. from the transaction block in
AdminIncomingMessageController#redeliver).  The default behaviour
of nested transactions in Rails is for them to behave as if all
but the outermost transaction blocks had no effect; this means
that the exception thrown in the inner transaction block in
xapian_create_job wasn't causing a rollback, and it was then being
caught before propagating up to the outermost transaction. Thus
the transaction was never being rolled-back so any subsequent
operation on this database connection would error.

This commit fixes this problem by using the :requires_new => true
option to transaction, which asks ActiveRecord to fake genuinely
nested transcations using SAVEPOINT.

Fixes #1124.

2 files changed, 12 insertions, 1 deletions

diff --git a/spec/controllers/admin_incoming_message_controller_spec.rb b/spec/controllers/admin_incoming_message_controller_spec.rb
index b969a8a3f..21c744e5b 100644
--- a/spec/controllers/admin_incoming_message_controller_spec.rb
+++ b/spec/controllers/admin_incoming_message_controller_spec.rb
@@ -50,6 +50,17 @@ describe AdminIncomingMessageController, "when administering incoming messages"
                              :url_title => destination_info_request.url_title
         end
 
+        it 'should succeed, even if a duplicate xapian indexing job is created' do
+
+            with_duplicate_xapian_job_creation do
+                current_info_request = info_requests(:fancy_dog_request)
+                destination_info_request = info_requests(:naughty_chicken_request)
+                incoming_message = incoming_messages(:useless_incoming_message)
+                post :redeliver, :redeliver_incoming_message_id => incoming_message.id,
+                                 :url_title => destination_info_request.url_title
+            end
+
+        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 fcf7a778d..2e486f328 100644
--- a/vendor/plugins/acts_as_xapian/lib/acts_as_xapian.rb
+++ b/vendor/plugins/acts_as_xapian/lib/acts_as_xapian.rb
@@ -928,7 +928,7 @@ module ActsAsXapian
 
         def xapian_create_job(action, model, model_id)
             begin
-                ActiveRecord::Base.transaction do
+                ActiveRecord::Base.transaction(:requires_new => true) do
                     ActsAsXapianJob.delete_all([ "model = ? and model_id = ?", model, model_id])
                     xapian_before_create_job_hook(action, model, model_id)
                     ActsAsXapianJob.create!(:model => model,