Remove global configuration setters from GoodJob class

lib/good_job.rb

@@ -34,7 +34,7 @@ module GoodJob
   #   @return [Logger, nil]
   #   @example Output GoodJob logs to a file:
   #     GoodJob.logger = ActiveSupport::TaggedLogging.new(ActiveSupport::Logger.new("log/my_logs.log"))
-  mattr_accessor :logger, default: ActiveSupport::TaggedLogging.new(ActiveSupport::Logger.new($stdout))
+  mattr_accessor :logger, default: nil
 
   # @!attribute [rw] preserve_job_records
   #   @!scope class
@@ -45,7 +45,7 @@ module GoodJob
   #   If +true+, you will need to clean out jobs using the +good_job cleanup_preserved_jobs+ CLI command or
   #   by using +Goodjob.cleanup_preserved_jobs+.
   #   @return [Boolean, nil]
-  mattr_accessor :preserve_job_records, default: false
+  mattr_accessor :preserve_job_records, default: nil
 
   # @!attribute [rw] retry_on_unhandled_error
   #   @!scope class
@@ -54,7 +54,7 @@ module GoodJob
   #   If +false+, jobs will be discarded or marked as finished if they raise an instance of +StandardError+.
   #   Instances of +Exception+, like +SIGINT+, will *always* be retried, regardless of this attribute's value.
   #   @return [Boolean, nil]
-  mattr_accessor :retry_on_unhandled_error, default: true
+  mattr_accessor :retry_on_unhandled_error, default: nil
 
   # @!attribute [rw] on_thread_error
   #   @!scope class
@@ -66,6 +66,28 @@ module GoodJob
   #   @return [Proc, nil]
   mattr_accessor :on_thread_error, default: nil
 
+  [
+    :logger,
+    :preserve_job_records,
+    :retry_on_unhandled_error,
+    :on_thread_error,
+  ].each do |dep_method|
+    define_method("#{dep_method}=") do |value|
+      ActiveSupport::Deprecation.warn(<<~DEPRECATION)
+        `GoodJob.#{dep_method} =` is deprecated and will be removed in GoodJob 3.0.
+        Please use Rails-configuration style `config.good_job.#{dep_method} =` instead.
+      DEPRECATION
+
+      super(value)
+    end
+  end
+
+  # GoodJob global configuration
+  # @return [Configuration]
+  def self.configuration
+    @_configuration ||= Configuration.new({})
+  end
+
   # Stop executing jobs.
   # GoodJob does its work in pools of background threads.
   # When forking processes you should shut down these background threads before forking, and restart them after forking.

lib/good_job/cli.rb

@@ -156,7 +156,7 @@ def cleanup_preserved_jobs
       # Rails or from the application can be set up here.
       def set_up_application!
         require RAILS_ENVIRONMENT_RB
-        return unless GoodJob::CLI.log_to_stdout? && !ActiveSupport::Logger.logger_outputs_to?(GoodJob.logger, $stdout)
+        return unless GoodJob::CLI.log_to_stdout? && !ActiveSupport::Logger.logger_outputs_to?(GoodJob.configuration.logger, $stdout)
 
         GoodJob::LogSubscriber.loggers << ActiveSupport::TaggedLogging.new(ActiveSupport::Logger.new($stdout))
         GoodJob::LogSubscriber.reset_logger

lib/good_job/configuration.rb

@@ -69,6 +69,49 @@ def execution_mode
       end
     end
 
+    # The logger used by GoodJob (default: +Rails.logger+).
+    # Use this to redirect logs to a special location or file.
+    # @return [Logger]
+    def logger
+      GoodJob.logger ||
+        rails_config[:logger] ||
+        Rails.logger ||
+        (@_default_logger ||= ActiveSupport::TaggedLogging.new(ActiveSupport::Logger.new($stdout)))
+    end
+
+    #   Whether to preserve job records in the database after they have finished (default: +false+).
+    #   By default, GoodJob deletes job records after the job is completed successfully.
+    #   If you want to preserve jobs for latter inspection, set this to +true+.
+    #   If you want to preserve only jobs that finished with error for latter inspection, set this to +:on_unhandled_error+.
+    #   If +true+, you will need to clean out jobs using the +good_job cleanup_preserved_jobs+ CLI command or
+    #   by using +Goodjob.cleanup_preserved_jobs+.
+    #   @return [Boolean]
+    def preserve_job_records
+      GoodJob.preserve_job_records ||
+        rails_config[:preserve_job_records] ||
+        false
+    end
+
+    #   Whether to re-perform a job when a type of +StandardError+ is raised to GoodJob (default: +true+).
+    #   If +true+, causes jobs to be re-queued and retried if they raise an instance of +StandardError+.
+    #   If +false+, jobs will be discarded or marked as finished if they raise an instance of +StandardError+.
+    #   Instances of +Exception+, like +SIGINT+, will *always* be retried, regardless of this attribute's value.
+    #   @return [Boolean]
+    def retry_on_unhandled_error
+      existing_value = GoodJob.retry_on_unhandled_error || rails_config[:retry_on_unhandled_error]
+      existing_value.nil? ? true : existing_value
+    end
+
+    #   This callable will be called when an exception reaches GoodJob (default: +nil+).
+    #   It can be useful for logging errors to bug tracking services, like Sentry or Airbrake.
+    #   @example Send errors to Sentry
+    #     # config/application.rb
+    #     config.good_job.on_thread_error = -> (exception) { Raven.capture_exception(exception) }
+    #   @return [Proc, nil]
+    def on_thread_error
+      GoodJob.on_thread_error || rails_config[:on_thread_error]
+    end
+
     # Indicates the number of threads to use per {Scheduler}. Note that
     # {#queue_string} may provide more specific thread counts to use with
     # individual schedulers.

lib/good_job/execution.rb

@@ -108,7 +108,7 @@ def self._migration_pending_warning
     # Get Jobs were completed before the given timestamp. If no timestamp is
     # provided, get all jobs that have been completed. By default, GoodJob
     # deletes jobs after they are completed and this will find no jobs.
-    # However, if you have changed {GoodJob.preserve_job_records}, this may
+    # However, if you have changed +config.good_job.preserve_job_records+, this may
     # find completed Jobs.
     # @!method finished(timestamp = nil)
     # @!scope class
@@ -252,16 +252,16 @@ def perform
       raise PreviouslyPerformedError, 'Cannot perform a job that has already been performed' if finished_at
 
       self.performed_at = Time.current
-      save! if GoodJob.preserve_job_records
+      save! if GoodJob.configuration.preserve_job_records
 
       result = execute
 
       job_error = result.handled_error || result.unhandled_error
       self.error = [job_error.class, ERROR_MESSAGE_SEPARATOR, job_error.message].join if job_error
 
-      if result.unhandled_error && GoodJob.retry_on_unhandled_error
+      if result.unhandled_error && GoodJob.configuration.retry_on_unhandled_error
         save!
-      elsif GoodJob.preserve_job_records == true || (result.unhandled_error && GoodJob.preserve_job_records == :on_unhandled_error)
+      elsif GoodJob.configuration.preserve_job_records == true || (result.unhandled_error && GoodJob.configuration.preserve_job_records == :on_unhandled_error)
         self.finished_at = Time.current
         save!
       else

lib/good_job/job_performer.rb

@@ -65,11 +65,11 @@ def next_at(after: nil, limit: nil, now_limit: nil)
     attr_reader :queue_string
 
     def job_query
-      @job_query.value
+      @job_query.value!
     end
 
     def parsed_queues
-      @parsed_queues.value
+      @parsed_queues.value!
     end
   end
 end

lib/good_job/log_subscriber.rb

@@ -168,7 +168,7 @@ class << self
       #   GoodJob::LogSubscriber.loggers << ActiveSupport::TaggedLogging.new(ActiveSupport::Logger.new("log/my_logs.log"))
       #   GoodJob::LogSubscriber.reset_logger
       def loggers
-        @_loggers ||= [GoodJob.logger]
+        @_loggers ||= [GoodJob.configuration.logger]
       end
 
       # Represents all the loggers attached to {LogSubscriber} with a single

lib/good_job/notifier.rb

@@ -120,7 +120,7 @@ def listen_observer(_time, _result, thread_error)
       return if thread_error.is_a? AdapterCannotListenError
 
       if thread_error
-        GoodJob.on_thread_error.call(thread_error) if GoodJob.on_thread_error.respond_to?(:call)
+        GoodJob.configuration.on_thread_error.call(thread_error) if GoodJob.configuration.on_thread_error.respond_to?(:call)
         ActiveSupport::Notifications.instrument("notifier_notify_error.good_job", { error: thread_error })
 
         connection_error = CONNECTION_ERRORS.any? do |error_string|

lib/good_job/probe_server.rb

@@ -16,7 +16,7 @@ def initialize(port:)
 
     def start
       @handler = Rack::Handler.get(RACK_SERVER)
-      @future = Concurrent::Future.new(args: [@handler, @port, GoodJob.logger]) do |thr_handler, thr_port, thr_logger|
+      @future = Concurrent::Future.new(args: [@handler, @port, GoodJob.configuration.logger]) do |thr_handler, thr_port, thr_logger|
         thr_handler.run(self, Port: thr_port, Logger: thr_logger, AccessLog: [])
       end
       @future.add_observer(self.class, :task_observer)

lib/good_job/railtie.rb

@@ -6,9 +6,6 @@ class Railtie < ::Rails::Railtie
     config.good_job.cron = {}
 
     initializer "good_job.logger" do |_app|
-      ActiveSupport.on_load(:good_job) do
-        self.logger = ::Rails.logger
-      end
       GoodJob::LogSubscriber.attach_to :good_job
     end
 

lib/good_job/scheduler.rb

@@ -169,7 +169,7 @@ def create_thread(state = nil)
     # @return [void]
     def task_observer(time, output, thread_error)
       error = thread_error || (output.is_a?(GoodJob::ExecutionResult) ? output.unhandled_error : nil)
-      GoodJob.on_thread_error.call(error) if error && GoodJob.on_thread_error.respond_to?(:call)
+      GoodJob.configuration.on_thread_error.call(error) if error && GoodJob.configuration.on_thread_error.respond_to?(:call)
 
       instrument("finished_job_task", { result: output, error: thread_error, time: time })
       create_task if output

spec/app/jobs/example_job_spec.rb

@@ -3,7 +3,7 @@
 
 describe ExampleJob do
   before do
-    allow(GoodJob).to receive(:preserve_job_records).and_return(true)
+    allow(GoodJob.configuration).to receive(:preserve_job_records).and_return(true)
     ActiveJob::Base.queue_adapter = GoodJob::Adapter.new(execution_mode: :inline)
   end
 

spec/engine/filters/good_job/executions_filter_spec.rb

@@ -7,8 +7,8 @@
   let(:params) { {} }
 
   before do
-    allow(GoodJob).to receive(:retry_on_unhandled_error).and_return(false)
-    allow(GoodJob).to receive(:preserve_job_records).and_return(true)
+    allow(GoodJob.configuration).to receive(:retry_on_unhandled_error).and_return(false)
+    allow(GoodJob.configuration).to receive(:preserve_job_records).and_return(true)
     ActiveJob::Base.queue_adapter = GoodJob::Adapter.new(execution_mode: :inline)
 
     ExampleJob.set(queue: 'default').perform_later('success')

spec/engine/filters/good_job/jobs_filter_spec.rb

@@ -7,8 +7,8 @@
   let(:params) { {} }
 
   before do
-    allow(GoodJob).to receive(:retry_on_unhandled_error).and_return(false)
-    allow(GoodJob).to receive(:preserve_job_records).and_return(true)
+    allow(GoodJob.configuration).to receive(:retry_on_unhandled_error).and_return(false)
+    allow(GoodJob.configuration).to receive(:preserve_job_records).and_return(true)
     ActiveJob::Base.queue_adapter = GoodJob::Adapter.new(execution_mode: :inline)
 
     ExampleJob.set(queue: 'default').perform_later('success')

spec/integration/scheduler_spec.rb

@@ -6,7 +6,7 @@
 
   before do
     ActiveJob::Base.queue_adapter = adapter
-    GoodJob.preserve_job_records = true
+    allow(GoodJob.configuration).to receive(:preserve_job_records).and_return(true)
 
     stub_const "RUN_JOBS", Concurrent::Array.new
     stub_const "THREAD_JOBS", Concurrent::Hash.new(Concurrent::Array.new)

spec/lib/good_job/active_job_extensions/concurrency_spec.rb

@@ -73,7 +73,7 @@ def perform(name:)
 
     describe 'perform_limit:' do
       before do
-        allow(GoodJob).to receive(:preserve_job_records).and_return(true)
+        allow(GoodJob.configuration).to receive(:preserve_job_records).and_return(true)
 
         TestJob.good_job_control_concurrency_with(
           perform_limit: 0,

spec/lib/good_job/active_job_job_spec.rb

@@ -5,7 +5,7 @@
   subject(:job) { described_class.find(head_execution.active_job_id) }
 
   before do
-    allow(GoodJob).to receive(:preserve_job_records).and_return(true)
+    allow(GoodJob.configuration).to receive(:preserve_job_records).and_return(true)
     ActiveJob::Base.queue_adapter = GoodJob::Adapter.new(execution_mode: :external)
 
     stub_const 'TestJob', (Class.new(ActiveJob::Base) do

spec/lib/good_job/execution_spec.rb

@@ -234,7 +234,7 @@ def perform(result_value = nil, raise_error: false)
         context 'when there is a retry handler' do
           before do
             ActiveJob::Base.queue_adapter = GoodJob::Adapter.new(execution_mode: :inline)
-            allow(GoodJob).to receive(:preserve_job_records).and_return(true)
+            allow(GoodJob.configuration).to receive(:preserve_job_records).and_return(true)
             TestJob.retry_on(TestJob::ExpectedError, attempts: 2)
           end
 
@@ -325,13 +325,13 @@ def perform(result_value = nil, raise_error: false)
     end
 
     it 'destroys the job when preserving record only on error' do
-      allow(GoodJob).to receive(:preserve_job_records).and_return(:on_unhandled_error)
+      allow(GoodJob.configuration).to receive(:preserve_job_records).and_return(:on_unhandled_error)
       good_job.perform
       expect { good_job.reload }.to raise_error ActiveRecord::RecordNotFound
     end
 
     it 'can preserve the job' do
-      allow(GoodJob).to receive(:preserve_job_records).and_return(true)
+      allow(GoodJob.configuration).to receive(:preserve_job_records).and_return(true)
 
       good_job.perform
       expect(good_job.reload).to have_attributes(
@@ -366,7 +366,7 @@ def perform(result_value = nil, raise_error: false)
       end
 
       it 'can preserve the job' do
-        allow(GoodJob).to receive(:preserve_job_records).and_return(true)
+        allow(GoodJob.configuration).to receive(:preserve_job_records).and_return(true)
 
         good_job.perform
 
@@ -391,11 +391,11 @@ def perform(result_value = nil, raise_error: false)
       describe 'GoodJob.retry_on_unhandled_error behavior' do
         context 'when true' do
           before do
-            allow(GoodJob).to receive(:retry_on_unhandled_error).and_return(true)
+            allow(GoodJob.configuration).to receive(:retry_on_unhandled_error).and_return(true)
           end
 
           it 'leaves the job record unfinished' do
-            allow(GoodJob).to receive(:preserve_job_records).and_return(true)
+            allow(GoodJob.configuration).to receive(:preserve_job_records).and_return(true)
 
             good_job.perform
 
@@ -407,7 +407,7 @@ def perform(result_value = nil, raise_error: false)
           end
 
           it 'does not destroy the job record' do
-            allow(GoodJob).to receive(:preserve_job_records).and_return(false)
+            allow(GoodJob.configuration).to receive(:preserve_job_records).and_return(false)
 
             good_job.perform
             expect { good_job.reload }.not_to raise_error
@@ -416,18 +416,18 @@ def perform(result_value = nil, raise_error: false)
 
         context 'when false' do
           before do
-            allow(GoodJob).to receive(:retry_on_unhandled_error).and_return(false)
+            allow(GoodJob.configuration).to receive(:retry_on_unhandled_error).and_return(false)
           end
 
           it 'destroys the job' do
-            allow(GoodJob).to receive(:preserve_job_records).and_return(false)
+            allow(GoodJob.configuration).to receive(:preserve_job_records).and_return(false)
 
             good_job.perform
             expect { good_job.reload }.to raise_error ActiveRecord::RecordNotFound
           end
 
           it 'can preserve the job' do
-            allow(GoodJob).to receive(:preserve_job_records).and_return(true)
+            allow(GoodJob.configuration).to receive(:preserve_job_records).and_return(true)
 
             good_job.perform
 
@@ -439,7 +439,7 @@ def perform(result_value = nil, raise_error: false)
           end
 
           it 'preserves the job when preserving record only on error' do
-            allow(GoodJob).to receive(:preserve_job_records).and_return(:on_unhandled_error)
+            allow(GoodJob.configuration).to receive(:preserve_job_records).and_return(:on_unhandled_error)
             good_job.perform
 
             expect(good_job.reload).to have_attributes(

spec/lib/good_job/notifier_spec.rb

@@ -37,7 +37,7 @@
     it 'raises exception to GoodJob.on_thread_error' do
       stub_const('ExpectedError', Class.new(StandardError))
       on_thread_error = instance_double(Proc, call: nil)
-      allow(GoodJob).to receive(:on_thread_error).and_return(on_thread_error)
+      allow(GoodJob.configuration).to receive(:on_thread_error).and_return(on_thread_error)
       allow(JSON).to receive(:parse).and_raise ExpectedError
 
       notifier = described_class.new

spec/lib/good_job/railtie_spec.rb

@@ -3,6 +3,6 @@
 
 RSpec.describe GoodJob::Railtie do
   it 'copies over the Rails logger by default' do
-    expect(GoodJob.logger).to eq Rails.logger
+    expect(GoodJob.configuration.logger).to eq Rails.logger
   end
 end

spec/lib/good_job/scheduler_spec.rb

@@ -12,7 +12,7 @@
     let(:error_proc) { double("Error Collector", call: nil) } # rubocop:disable RSpec/VerifiedDoubles
 
     before do
-      allow(GoodJob).to receive(:on_thread_error).and_return(error_proc)
+      allow(GoodJob.configuration).to receive(:on_thread_error).and_return(error_proc)
       stub_const 'THREAD_HAS_RUN', Concurrent::AtomicBoolean.new(false)
       stub_const 'ERROR_TRIGGERED', Concurrent::AtomicBoolean.new(false)
     end

spec/support/reset_good_job.rb

@@ -4,7 +4,6 @@
 RSpec.configure do |config|
   config.before do
     GoodJob::CurrentThread.reset
-    GoodJob.preserve_job_records = false
 
     PgLock.current_database.advisory_lock.owns.all?(&:unlock) if PgLock.advisory_lock.owns.count > 0
     PgLock.current_database.advisory_lock.others.each(&:unlock!) if PgLock.advisory_lock.others.count > 0
@@ -15,7 +14,8 @@
     THREAD_ERRORS.clear
 
     Thread.current.name = "RSpec: #{example.description}"
-    GoodJob.on_thread_error = lambda do |exception|
+
+    Rails.application.config.good_job.on_thread_error = lambda do |exception|
       THREAD_ERRORS << [Thread.current.name, exception]
     end