after_transaction_commit gem extraction

refs CNVS-15881

our delayed_job depends on this, so I'm pulling it out into a gem first,
so that we can fully gemify our fork of delayed_job and use it in other
apps.

I modified the code to be built on top of the after_commit callback
functionality added in rails3, and I pulled in the test_after_commit gem
(after making a fix to it), which emulates transaction callbacks in test
mode. This makes it easier to write tests that interacts with these
callbacks, and cleans up some code in the app that was checking for test
mode and changing behavior. Fortunately, it behaves well with once-ler.

Side note: now that we are on rails3, we could replace some, but not
all, of our usage of after_transaction_commit with after_commit AR
callbacks. I didn't do that in this commit, to minimize churn.

test plan: things that happen in commit callbacks should still happen as
expected. for instance, with caching enabled, changing a /plugins
setting should still take effect immediately, because the cache is
cleared in a commit callback.

Change-Id: I66af5f66bf9fa9354352ed37edec8bd92d8e39c8
Reviewed-on: https://gerrit.instructure.com/41819
Reviewed-by: Cody Cutrer <cody@instructure.com>
Tested-by: Jenkins <jenkins@instructure.com>
QA-Review: August Thornton <august@instructure.com>
Product-Review: Brian Palmer <brianp@instructure.com>

Gemfile.d/other_stuff.rb

@@ -24,6 +24,7 @@ gem 'switchman', '1.2.21'
 gem 'folio-pagination', '0.0.7', :require => 'folio/rails'
 gem 'will_paginate', '3.0.4', :require => false
 
+gem "after_transaction_commit", '1.0.1'
 gem "aws-sdk", '1.52.0'
   gem 'uuidtools', '2.1.4'
 gem 'barby', '0.5.0'

Gemfile.d/test.rb

@@ -25,6 +25,7 @@ group :test do
   gem 'selenium-webdriver', '2.43.0'
     gem 'childprocess', '0.5.0'
     gem 'websocket', '1.0.7'
+  gem 'test_after_commit', '0.3.0', github: 'codekitchen/test_after_commit', branch: 'nested-commit-callbacks'
   gem 'webmock', '1.16.1', :require => false
     gem 'addressable', '2.3.5'
     gem 'crack', '0.4.1'

config/initializers/active_record.rb

@@ -940,69 +940,12 @@ class ActiveRecord::ConnectionAdapters::AbstractAdapter
           col
     }
   end
-
-  def after_transaction_commit(&block)
-    if open_transactions <= base_transaction_level
-      block.call
-    else
-      @after_transaction_commit ||= []
-      @after_transaction_commit << block
-    end
-  end
-
-  def after_transaction_commit_callbacks
-    @after_transaction_commit || []
-  end
-
-  # the alias_method_chain needs to happen in the subclass, since they all
-  # override commit_db_transaction
-  def commit_db_transaction_with_callbacks
-    commit_db_transaction_without_callbacks
-    run_transaction_commit_callbacks
-  end
-
-  # this will only be chained in in Rails.env.test?, but we still
-  # sometimes stub Rails.env.test? in specs to specifically
-  # test behavior like this, so leave the check in this code
-  def release_savepoint_with_callbacks
-    release_savepoint_without_callbacks
-    return unless Rails.env.test?
-    return if open_transactions > base_transaction_level
-    run_transaction_commit_callbacks
-  end
-
-  def base_transaction_level
-    return 0 unless Rails.env.test?
-    return 1 unless defined?(Onceler)
-    Onceler.base_transactions
-  end
-
-  def rollback_db_transaction_with_callbacks
-    rollback_db_transaction_without_callbacks
-    @after_transaction_commit = [] if @after_transaction_commit
-  end
-
-  def run_transaction_commit_callbacks
-    return unless @after_transaction_commit.present?
-    # the callback could trigger a new transaction on this connection,
-    # and leaving the callbacks in @after_transaction_commit could put us in an
-    # infinite loop.
-    # so we store off the callbacks to a local var here.
-    callbacks = @after_transaction_commit
-    @after_transaction_commit = []
-    callbacks.each { |cb| cb.call() }
-  ensure
-    @after_transaction_commit = [] if @after_transaction_commit
-  end
 end
 
 module MySQLAdapterExtensions
   def self.included(klass)
     klass::NATIVE_DATABASE_TYPES[:primary_key] = "bigint DEFAULT NULL auto_increment PRIMARY KEY".freeze
     klass.alias_method_chain :configure_connection, :pg_compat
-    klass.alias_method_chain :commit_db_transaction, :callbacks
-    klass.alias_method_chain :rollback_db_transaction, :callbacks
-    klass.alias_method_chain :release_savepoint, :callbacks if Rails.env.test?
   end
 
   def rename_index(table_name, old_name, new_name)
@@ -1212,14 +1155,6 @@ if defined?(ActiveRecord::ConnectionAdapters::PostgreSQLAdapter)
   end
 end
 
-if defined?(ActiveRecord::ConnectionAdapters::SQLiteAdapter)
-  ActiveRecord::ConnectionAdapters::SQLiteAdapter.class_eval do
-    alias_method_chain :commit_db_transaction, :callbacks
-    alias_method_chain :rollback_db_transaction, :callbacks
-    alias_method_chain :release_savepoint, :callbacks if Rails.env.test?
-  end
-end
-
 if defined?(ActiveRecord::ConnectionAdapters::PostgreSQLAdapter)
   ActiveRecord::ConnectionAdapters::PostgreSQLAdapter.class_eval do
     def func(name, *args)
@@ -1278,10 +1213,6 @@ if defined?(ActiveRecord::ConnectionAdapters::PostgreSQLAdapter)
         ActiveRecord::ConnectionAdapters::IndexDefinition.new(table_name, index_name, unique, column_names, [], orders)
       end
     end
-
-    alias_method_chain :commit_db_transaction, :callbacks
-    alias_method_chain :rollback_db_transaction, :callbacks
-    alias_method_chain :release_savepoint, :callbacks if Rails.env.test?
   end
 end
 

gems/plugins/delayed_job/lib/delayed/message_sending.rb

@@ -19,18 +19,12 @@ module Delayed
         # transaction are visible
         connection = self.connection if respond_to?(:connection)
         connection ||= ActiveRecord::Base.connection
-        transactions = 0
-        # specs run in an outer transaction that needs to be ignored
-        transactions += 1 if Rails.env.test?
 
-        if connection.open_transactions > transactions
-          if !Rails.env.test? && (Delayed::Job != Delayed::Backend::ActiveRecord::Job ||
-              connection != Delayed::Job.connection)
-            connection.after_transaction_commit do
-              Delayed::Job.enqueue(Delayed::PerformableMethod.new(self, method.to_sym, args), enqueue_args)
-            end
-            return nil
+        if (Delayed::Job != Delayed::Backend::ActiveRecord::Job || connection != Delayed::Job.connection)
+          connection.after_transaction_commit do
+            Delayed::Job.enqueue(Delayed::PerformableMethod.new(self, method.to_sym, args), enqueue_args)
           end
+          return nil
         end
       end
 

gems/plugins/delayed_job/spec_canvas/redis_job_spec.rb

@@ -71,12 +71,12 @@ describe 'Delayed::Backend::Redis::Job' do
 
   describe "send_later" do
     it "should schedule job on transaction commit" do
-      Rails.env.stubs(:test?).returns(false)
       before_count = Delayed::Job.jobs_count(:current)
-      job = "string".send_later :reverse
-      expect(job).to be_nil
-      expect(Delayed::Job.jobs_count(:current)).to eq before_count
-      ActiveRecord::Base.connection.run_transaction_commit_callbacks
+      User.transaction do
+        job = "string".send_later :reverse
+        expect(job).to be_nil
+        expect(Delayed::Job.jobs_count(:current)).to eq before_count
+      end
       Delayed::Job.jobs_count(:current) == before_count + 1
     end
   end

spec/models/active_record_base_spec.rb

@@ -428,52 +428,6 @@ describe ActiveRecord::Base do
     end
   end
 
-  context "after_transaction_commit" do
-    self.use_transactional_fixtures = false
-
-    before do
-      Rails.env.stubs(:test?).returns(false)
-    end
-
-    it "should execute the callback immediately if not in a transaction" do
-      a = 0
-      User.connection.after_transaction_commit { a += 1 }
-      expect(a).to eq 1
-    end
-
-    it "should execute the callback after commit if in a transaction" do
-      a = 0
-      User.connection.transaction do
-        User.connection.after_transaction_commit { a += 1 }
-        expect(a).to eq 0
-      end
-      expect(a).to eq 1
-    end
-
-    it "should not execute the callbacks on rollback" do
-      a = 0
-      User.connection.transaction do
-        User.connection.after_transaction_commit { a += 1 }
-        expect(a).to eq 0
-        raise ActiveRecord::Rollback
-      end
-      expect(a).to eq 0
-      User.connection.transaction do
-        # verify that the callback gets cleared out, so this second transaction won't trigger it
-      end
-      expect(a).to eq 0
-    end
-
-    it "should avoid loops due to callbacks causing a new transaction" do
-      a = 0
-      User.connection.transaction do
-        User.connection.after_transaction_commit { User.connection.transaction { a += 1 } }
-        expect(a).to eq 0
-      end
-      expect(a).to eq 1
-    end
-  end
-
   context "Finder tests" do
     before :once do
       @user = user_model

spec/models/content_tag_spec.rb

@@ -466,14 +466,14 @@ describe ContentTag do
   end
 
   it "should touch the module after committing the save" do
-    Rails.env.stubs(:test?).returns(false)
     course
     mod = @course.context_modules.create!
     yesterday = 1.day.ago
     ContextModule.where(:id => mod).update_all(:updated_at => yesterday)
-    tag = mod.add_item :type => 'context_module_sub_header', :title => 'blah'
-    expect(mod.reload.updated_at.to_i).to eq yesterday.to_i
-    mod.connection.run_transaction_commit_callbacks
+    ContextModule.transaction do
+      tag = mod.add_item :type => 'context_module_sub_header', :title => 'blah'
+      expect(mod.reload.updated_at.to_i).to eq yesterday.to_i
+    end
     expect(mod.reload.updated_at).to be > 5.seconds.ago
   end