Merge branch 'in-transaction' into 'master'

Refactor AfterCommitQueue.inside_transaction?

See merge request gitlab-org/gitlab-ce!29903

3 files changed, 18 insertions, 16 deletions

diff --git a/config/initializers/forbid_sidekiq_in_transactions.rb b/config/initializers/forbid_sidekiq_in_transactions.rb
index deb94d7dbce..a69f1ba090e 100644
--- a/config/initializers/forbid_sidekiq_in_transactions.rb
+++ b/config/initializers/forbid_sidekiq_in_transactions.rb
@@ -17,7 +17,7 @@ module Sidekiq
       module NoEnqueueingFromTransactions
         %i(perform_async perform_at perform_in).each do |name|
           define_method(name) do |*args|
-            if !Sidekiq::Worker.skip_transaction_check && AfterCommitQueue.inside_transaction?
+            if !Sidekiq::Worker.skip_transaction_check && Gitlab::Database.inside_transaction?
               begin
                 raise Sidekiq::Worker::EnqueueFromTransactionError, <<~MSG
                 `#{self}.#{name}` cannot be called inside a transaction as this can lead to
diff --git a/lib/after_commit_queue.rb b/lib/after_commit_queue.rb
index 6fb7985f955..6a180fdf338 100644
--- a/lib/after_commit_queue.rb
+++ b/lib/after_commit_queue.rb
@@ -15,7 +15,7 @@ module AfterCommitQueue
   end
 
   def run_after_commit_or_now(&block)
-    if AfterCommitQueue.inside_transaction?
+    if Gitlab::Database.inside_transaction?
       if ActiveRecord::Base.connection.current_transaction.records.include?(self)
         run_after_commit(&block)
       else
@@ -32,18 +32,6 @@ module AfterCommitQueue
     true
   end
 
-  def self.open_transactions_baseline
-    if ::Rails.env.test?
-      return DatabaseCleaner.connections.count { |conn| conn.strategy.is_a?(DatabaseCleaner::ActiveRecord::Transaction) }
-    end
-
-    0
-  end
-
-  def self.inside_transaction?
-    ActiveRecord::Base.connection.open_transactions > open_transactions_baseline
-  end
-
   protected
 
   def _run_after_commit_queue
diff --git a/lib/gitlab/database.rb b/lib/gitlab/database.rb
index 8da98cc3909..e4d4779ba9a 100644
--- a/lib/gitlab/database.rb
+++ b/lib/gitlab/database.rb
@@ -234,6 +234,7 @@ module Gitlab
     def self.connection
       ActiveRecord::Base.connection
     end
+    private_class_method :connection
 
     def self.cached_column_exists?(table_name, column_name)
       connection.schema_cache.columns_hash(table_name).has_key?(column_name.to_s)
@@ -243,8 +244,6 @@ module Gitlab
       connection.schema_cache.data_source_exists?(table_name)
     end
 
-    private_class_method :connection
-
     def self.database_version
       row = connection.execute("SELECT VERSION()").first
 
@@ -272,5 +271,20 @@ module Gitlab
         end
       end
     end
+
+    # inside_transaction? will return true if the caller is running within a transaction. Handles special cases
+    # when running inside a test environment, in which the entire test is running with a DatabaseCleaner transaction
+    def self.inside_transaction?
+      ActiveRecord::Base.connection.open_transactions > open_transactions_baseline
+    end
+
+    def self.open_transactions_baseline
+      if ::Rails.env.test?
+        return DatabaseCleaner.connections.count { |conn| conn.strategy.is_a?(DatabaseCleaner::ActiveRecord::Transaction) }
+      end
+
+      0
+    end
+    private_class_method :open_transactions_baseline
   end
 end