Browse Source

Added reset of UserInfo.current_user_id for background jobs and transactions.

Martin Edenhofer 9 years ago
parent
commit
b9c24748a9

+ 20 - 27
app/models/scheduler.rb

@@ -1,5 +1,5 @@
 # Copyright (C) 2012-2014 Zammad Foundation, http://zammad-foundation.org/
-# rubocop:disable Rails/Output
+
 class Scheduler < ApplicationModel
 
   # rubocop:disable Style/ClassVars
@@ -114,9 +114,25 @@ class Scheduler < ApplicationModel
     end
   end
 
-  def self.worker
-    wait = 8
+  def self.worker(foreground = false)
+
+    # used for tests
+    if foreground
+      original_user_id = UserInfo.current_user_id
+      UserInfo.current_user_id = nil
+      loop do
+        success, failure = Delayed::Worker.new.work_off
+        if failure != 0
+          raise "ERROR: #{failure} failed background jobs: #{Delayed::Job.where('last_error IS NOT NULL').inspect}"
+        end
+        break if success == 0
+      end
+      UserInfo.current_user_id = original_user_id
+      return
+    end
 
+    # used for production
+    wait = 8
     Thread.new {
       sleep wait
 
@@ -126,6 +142,7 @@ class Scheduler < ApplicationModel
         result = nil
 
         realtime = Benchmark.realtime do
+          logger.debug "*** worker thread, #{Delayed::Job.all.count} in queue"
           result = Delayed::Worker.new.work_off
         end
 
@@ -145,28 +162,4 @@ class Scheduler < ApplicationModel
 
   end
 
-  def self.check(name, time_warning = 10, time_critical = 20)
-    time_warning_time  = Time.zone.now - time_warning.minutes
-    time_critical_time = Time.zone.now - time_critical.minutes
-    scheduler = Scheduler.find_by( name: name )
-    if !scheduler
-      puts "CRITICAL - no such scheduler jobs '#{name}'"
-      return true
-    end
-    logger.debug scheduler.inspect
-    if !scheduler.last_run
-      puts "CRITICAL - scheduler jobs never started '#{name}'"
-      exit 2
-    end
-    if scheduler.last_run < time_critical_time
-      puts "CRITICAL - scheduler jobs was not running in last '#{time_critical}' minutes - last run at '#{scheduler.last_run}' '#{name}'"
-      exit 2
-    end
-    if scheduler.last_run < time_warning_time
-      puts "CRITICAL - scheduler jobs was not running in last '#{time_warning}' minutes - last run at '#{scheduler.last_run}' '#{name}'"
-      exit 2
-    end
-    puts "ok - scheduler jobs was running at '#{scheduler.last_run}' '#{name}'"
-    exit 0
-  end
 end

+ 3 - 3
app/models/transaction/background_job.rb

@@ -24,12 +24,12 @@ class Transaction::BackgroundJob
     Setting.where(area: 'Transaction::Backend').order(:name).each {|setting|
       backend = Setting.get(setting.name)
       begin
-        UserInfo.current_user_id = 1
+        UserInfo.current_user_id = nil
         integration = Kernel.const_get(backend).new(@item, @params)
         integration.perform
       rescue => e
-        logger.error 'ERROR: ' + setting.inspect
-        logger.error 'ERROR: ' + e.inspect
+        Rails.logger.error 'ERROR: ' + setting.inspect
+        Rails.logger.error 'ERROR: ' + e.inspect
       end
     }
   end

+ 1 - 0
app/models/transaction/clearbit_enrichment.rb

@@ -47,6 +47,7 @@ class Transaction::ClearbitEnrichment
   end
 
   def self.sync_user(user)
+    UserInfo.current_user_id = 1
 
     return if user.email.empty?
     data = fetch(user.email)

+ 1 - 3
test/controllers/search_controller_test.rb

@@ -177,9 +177,7 @@ class SearchControllerTest < ActionDispatch::IntegrationTest
       system('rake searchindex:rebuild')
 
       # execute background jobs
-      # execute background jobs
-      #puts Delayed::Job.all.inspect
-      Delayed::Worker.new.work_off
+      Scheduler.worker(true)
 
       sleep 6
     end

+ 11 - 11
test/integration/clearbit_test.rb

@@ -48,7 +48,7 @@ class ClearbitTest < ActiveSupport::TestCase
     assert(customer1)
 
     Observer::Transaction.commit
-    Delayed::Worker.new.work_off
+    Scheduler.worker(true)
 
     assert(ExternalSync.find_by(source: 'clearbit', object: 'User', o_id: customer1.id))
 
@@ -76,7 +76,7 @@ class ClearbitTest < ActiveSupport::TestCase
     assert(customer2)
 
     Observer::Transaction.commit
-    Delayed::Worker.new.work_off
+    Scheduler.worker(true)
 
     assert(ExternalSync.find_by(source: 'clearbit', object: 'User', o_id: customer2.id))
 
@@ -99,7 +99,7 @@ class ClearbitTest < ActiveSupport::TestCase
     )
 
     Observer::Transaction.commit
-    Delayed::Worker.new.work_off
+    Scheduler.worker(true)
 
     assert(ExternalSync.find_by(source: 'clearbit', object: 'User', o_id: customer2.id))
 
@@ -111,7 +111,7 @@ class ClearbitTest < ActiveSupport::TestCase
     assert_equal('Norsk-Data-Straße 1, 61352 Bad Homburg vor der Höhe, Germany', customer2_lookup.address)
 
     Transaction::ClearbitEnrichment.sync_user(customer2)
-    Delayed::Worker.new.work_off
+    Scheduler.worker(true)
 
     customer2_lookup = User.lookup(id: customer2.id)
 
@@ -127,7 +127,7 @@ class ClearbitTest < ActiveSupport::TestCase
     )
 
     Transaction::ClearbitEnrichment.sync_user(customer2)
-    Delayed::Worker.new.work_off
+    Scheduler.worker(true)
 
     customer2_lookup = User.lookup(id: customer2.id)
 
@@ -142,7 +142,7 @@ class ClearbitTest < ActiveSupport::TestCase
     )
 
     Transaction::ClearbitEnrichment.sync_user(customer2)
-    Delayed::Worker.new.work_off
+    Scheduler.worker(true)
 
     customer2_lookup = User.lookup(id: customer2.id)
 
@@ -168,7 +168,7 @@ class ClearbitTest < ActiveSupport::TestCase
     assert(customer3)
 
     Observer::Transaction.commit
-    Delayed::Worker.new.work_off
+    Scheduler.worker(true)
 
     assert_not(ExternalSync.find_by(source: 'clearbit', object: 'User', o_id: customer3.id))
 
@@ -199,7 +199,7 @@ class ClearbitTest < ActiveSupport::TestCase
     assert(customer4)
 
     Observer::Transaction.commit
-    Delayed::Worker.new.work_off
+    Scheduler.worker(true)
 
     assert(ExternalSync.find_by(source: 'clearbit', object: 'User', o_id: customer4.id))
 
@@ -228,7 +228,7 @@ class ClearbitTest < ActiveSupport::TestCase
     assert(customer5)
 
     Observer::Transaction.commit
-    Delayed::Worker.new.work_off
+    Scheduler.worker(true)
 
     assert(ExternalSync.find_by(source: 'clearbit', object: 'User', o_id: customer5.id))
 
@@ -259,7 +259,7 @@ class ClearbitTest < ActiveSupport::TestCase
     assert(customer6)
 
     Observer::Transaction.commit
-    Delayed::Worker.new.work_off
+    Scheduler.worker(true)
 
     assert_not(ExternalSync.find_by(source: 'clearbit', object: 'User', o_id: customer6.id))
 
@@ -322,7 +322,7 @@ class ClearbitTest < ActiveSupport::TestCase
     assert(customer1)
 
     Observer::Transaction.commit
-    Delayed::Worker.new.work_off
+    Scheduler.worker(true)
 
     assert(ExternalSync.find_by(source: 'clearbit', object: 'User', o_id: customer1.id))
 

+ 1 - 2
test/integration/elasticsearch_test.rb

@@ -215,8 +215,7 @@ class ElasticsearchTest < ActiveSupport::TestCase
     )
 
     # execute background jobs
-    #puts Delayed::Job.all.inspect
-    Delayed::Worker.new.work_off
+    Scheduler.worker(true)
 
     sleep 6
 

+ 1 - 2
test/integration/report_test.rb

@@ -240,8 +240,7 @@ class ReportTest < ActiveSupport::TestCase
   )
 
   # execute background jobs
-  #puts Delayed::Job.all.inspect
-  Delayed::Worker.new.work_off
+  Scheduler.worker(true)
 
   sleep 6
 

+ 11 - 11
test/integration/slack_test.rb

@@ -69,7 +69,7 @@ class SlackTest < ActiveSupport::TestCase
     )
 
     Observer::Transaction.commit
-    Delayed::Worker.new.work_off
+    Scheduler.worker(true)
 
     # check if message exists
     assert_equal(0, slack_check(channel, hash))
@@ -78,7 +78,7 @@ class SlackTest < ActiveSupport::TestCase
     ticket1.save
 
     Observer::Transaction.commit
-    Delayed::Worker.new.work_off
+    Scheduler.worker(true)
 
     # check if message exists
     assert_equal(0, slack_check(channel, hash))
@@ -107,7 +107,7 @@ class SlackTest < ActiveSupport::TestCase
     )
 
     Observer::Transaction.commit
-    Delayed::Worker.new.work_off
+    Scheduler.worker(true)
 
     # check if message exists
     assert_equal(1, slack_check(channel, hash))
@@ -119,7 +119,7 @@ class SlackTest < ActiveSupport::TestCase
     ticket2.save
 
     Observer::Transaction.commit
-    Delayed::Worker.new.work_off
+    Scheduler.worker(true)
 
     # check if message exists
     assert_equal(1, slack_check(channel, hash))
@@ -129,7 +129,7 @@ class SlackTest < ActiveSupport::TestCase
     ticket2.save
 
     Observer::Transaction.commit
-    Delayed::Worker.new.work_off
+    Scheduler.worker(true)
 
     # check if message exists
     assert_equal(2, slack_check(channel, hash))
@@ -137,7 +137,7 @@ class SlackTest < ActiveSupport::TestCase
     Ticket.process_pending
 
     Observer::Transaction.commit
-    Delayed::Worker.new.work_off
+    Scheduler.worker(true)
 
     # check if message exists
     assert_equal(3, slack_check(channel, hash))
@@ -145,7 +145,7 @@ class SlackTest < ActiveSupport::TestCase
     Ticket.process_pending
 
     Observer::Transaction.commit
-    Delayed::Worker.new.work_off
+    Scheduler.worker(true)
 
     # check if message exists
     assert_equal(3, slack_check(channel, hash))
@@ -188,7 +188,7 @@ class SlackTest < ActiveSupport::TestCase
     )
 
     Observer::Transaction.commit
-    Delayed::Worker.new.work_off
+    Scheduler.worker(true)
 
     # check if message exists
     assert_equal(0, slack_check(channel, hash))
@@ -197,7 +197,7 @@ class SlackTest < ActiveSupport::TestCase
     ticket3.save
 
     Observer::Transaction.commit
-    Delayed::Worker.new.work_off
+    Scheduler.worker(true)
 
     # check if message exists
     assert_equal(0, slack_check(channel, hash))
@@ -226,7 +226,7 @@ class SlackTest < ActiveSupport::TestCase
     )
 
     Observer::Transaction.commit
-    Delayed::Worker.new.work_off
+    Scheduler.worker(true)
 
     # check if message exists
     assert_equal(1, slack_check(channel, hash))
@@ -238,7 +238,7 @@ class SlackTest < ActiveSupport::TestCase
     ticket4.save
 
     Observer::Transaction.commit
-    Delayed::Worker.new.work_off
+    Scheduler.worker(true)
 
     # check if message exists
     assert_equal(0, slack_check(channel, hash))

+ 3 - 0
test/integration_test_helper.rb

@@ -18,6 +18,9 @@ class ActiveSupport::TestCase
     # clear cache
     Cache.clear
 
+    # remove background jobs
+    Delayed::Job.destroy_all
+
     # set current user
     UserInfo.current_user_id = nil
   end

+ 3 - 15
test/test_helper.rb

@@ -24,9 +24,6 @@ class ActiveSupport::TestCase
   load "#{Rails.root}/db/seeds.rb"
   load "#{Rails.root}/test/fixtures/seeds.rb"
 
-  # proccess background jobs
-  Delayed::Worker.new.work_off
-
   # set system mode to done / to activate
   Setting.set('system_init_done', true)
 
@@ -35,21 +32,12 @@ class ActiveSupport::TestCase
     # clear cache
     Cache.clear
 
+    # remove background jobs
+    Delayed::Job.destroy_all
+
     # set current user
     UserInfo.current_user_id = nil
   end
 
-  # cleanup jobs
-  def teardown
-
-    # check if jobs are proccessed
-    return if Delayed::Job.all.empty?
-
-    Delayed::Job.where('failed_at != NULL').each {|job|
-      assert( false, "not processable job #{job.inspect}" )
-    }
-    Delayed::Job.all.destroy_all
-  end
-
   # Add more helper methods to be used by all tests here...
 end

Some files were not shown because too many files changed in this diff