Actually remove deprecated configuration for `cleanup_interval_second…

…s`, `cleanup_interval_jobs`; remove deprecated Lockable` (#1406)

lib/good_job.rb

@@ -281,9 +281,6 @@ def self.deprecator
     end
   end
 
-  include ActiveSupport::Deprecation::DeprecatedConstantAccessor
-  deprecate_constant :Lockable, 'GoodJob::AdvisoryLockable', deprecator: deprecator
-
   # Whether all GoodJob migrations have been applied.
   # For use in tests/CI to validate GoodJob is up-to-date.
   # @return [Boolean]

lib/good_job/cleanup_tracker.rb

@@ -9,7 +9,7 @@ class CleanupTracker
                   :last_at
 
     def initialize(cleanup_interval_seconds: false, cleanup_interval_jobs: false)
-      raise ArgumentError, "Do not use `0`. Use `false` to disable, or -1 to always run" if cleanup_interval_seconds == 0 || cleanup_interval_jobs == 0 # rubocop:disable Style/NumericPredicate
+      raise ArgumentError, "Do not use `0` for cleanup intervals. Use `false` to disable, or -1 to always run" if cleanup_interval_seconds == 0 || cleanup_interval_jobs == 0 # rubocop:disable Style/NumericPredicate
 
       self.cleanup_interval_seconds = cleanup_interval_seconds
       self.cleanup_interval_jobs = cleanup_interval_jobs

lib/good_job/configuration.rb

@@ -230,8 +230,8 @@ def cron_entries
     def queue_select_limit
       (
         options[:queue_select_limit] ||
-        rails_config[:queue_select_limit] ||
-        env['GOOD_JOB_QUEUE_SELECT_LIMIT']
+          rails_config[:queue_select_limit] ||
+          env['GOOD_JOB_QUEUE_SELECT_LIMIT']
       )&.to_i
     end
 
@@ -240,8 +240,8 @@ def queue_select_limit
     def idle_timeout
       (
         options[:idle_timeout] ||
-        rails_config[:idle_timeout] ||
-        env['GOOD_JOB_IDLE_TIMEOUT']
+          rails_config[:idle_timeout] ||
+          env['GOOD_JOB_IDLE_TIMEOUT']
       )&.to_i || nil
     end
 
@@ -269,69 +269,38 @@ def cleanup_preserved_jobs_before_seconds_ago
     # Positive values will clean up after that many jobs have run, false or 0 will disable, and -1 will clean up after every job.
     # @return [Integer, Boolean, nil]
     def cleanup_interval_jobs
-      if rails_config.key?(:cleanup_interval_jobs)
-        value = rails_config[:cleanup_interval_jobs]
-        if value.nil?
-          GoodJob.deprecator.warn(
-            %(Setting `config.good_job.cleanup_interval_jobs` to `nil` will no longer disable count-based cleanups in GoodJob v4. Set to `false` to disable, or `-1` to run every time.)
-          )
-          value = false
-        elsif value == 0 # rubocop:disable Style/NumericPredicate
-          GoodJob.deprecator.warn(
-            %(Setting `config.good_job.cleanup_interval_jobs` to `0` will disable count-based cleanups in GoodJob v4. Set to `false` to disable, or `-1` to run every time.)
-          )
-          value = -1
-        end
-      elsif env.key?('GOOD_JOB_CLEANUP_INTERVAL_JOBS')
-        value = env['GOOD_JOB_CLEANUP_INTERVAL_JOBS']
-        if value.blank?
-          GoodJob.deprecator.warn(
-            %(Setting `GOOD_JOB_CLEANUP_INTERVAL_JOBS` to `""` will no longer disable count-based cleanups in GoodJob v4. Set to `0` to disable, or `-1` to run every time.)
-          )
-          value = false
-        elsif value == '0'
-          value = false
-        end
+      value = if rails_config.key?(:cleanup_interval_jobs)
+                rails_config[:cleanup_interval_jobs]
+              elsif env.key?('GOOD_JOB_CLEANUP_INTERVAL_JOBS')
+                env['GOOD_JOB_CLEANUP_INTERVAL_JOBS']
+              end
+
+      if value.in? [nil, "", true]
+        DEFAULT_CLEANUP_INTERVAL_JOBS
+      elsif value.in? [0, "0", false, "false"]
+        false
       else
-        value = DEFAULT_CLEANUP_INTERVAL_JOBS
+        value ? value.to_i : false
       end
-
-      value ? value.to_i : false
     end
 
     # Number of seconds a {Scheduler} will wait before automatically cleaning up preserved jobs.
     # Positive values will clean up after that many jobs have run, false or 0 will disable, and -1 will clean up after every job.
-    # @return [Integer, nil]
+    # @return [Integer, Boolean, nil]
     def cleanup_interval_seconds
-      if rails_config.key?(:cleanup_interval_seconds)
-        value = rails_config[:cleanup_interval_seconds]
-
-        if value.nil?
-          GoodJob.deprecator.warn(
-            %(Setting `config.good_job.cleanup_interval_seconds` to `nil` will no longer disable time-based cleanups in GoodJob v4. Set to `false` to disable, or `-1` to run every time.)
-          )
-          value = false
-        elsif value == 0 # rubocop:disable Style/NumericPredicate
-          GoodJob.deprecator.warn(
-            %(Setting `config.good_job.cleanup_interval_seconds` to `0` will disable time-based cleanups in GoodJob v4. Set to `false` to disable, or `-1` to run every time.)
-          )
-          value = -1
-        end
-      elsif env.key?('GOOD_JOB_CLEANUP_INTERVAL_SECONDS')
-        value = env['GOOD_JOB_CLEANUP_INTERVAL_SECONDS']
-        if value.blank?
-          GoodJob.deprecator.warn(
-            %(Setting `GOOD_JOB_CLEANUP_INTERVAL_SECONDS` to `""` will no longer disable time-based cleanups in GoodJob v4. Set to `0` to disable, or `-1` to run every time.)
-          )
-          value = false
-        elsif value == '0'
-          value = false
-        end
+      value = if rails_config.key?(:cleanup_interval_seconds)
+                rails_config[:cleanup_interval_seconds]
+              elsif env.key?('GOOD_JOB_CLEANUP_INTERVAL_SECONDS')
+                env['GOOD_JOB_CLEANUP_INTERVAL_SECONDS']
+              end
+
+      if value.nil? || value == "" || value == true
+        DEFAULT_CLEANUP_INTERVAL_SECONDS
+      elsif value.in? [0, "0", false, "false"]
+        false
       else
-        value = DEFAULT_CLEANUP_INTERVAL_SECONDS
+        value ? value.to_i : false
       end
-
-      value ? value.to_i : false
     end
 
     # Tests whether to daemonize the process.

spec/lib/good_job/configuration_spec.rb

@@ -134,22 +134,18 @@
         expect(configuration.cleanup_interval_jobs).to be false
       end
 
-      it 'accepts 0, with deprecation' do
+      it 'coerces 0 to false' do
         allow(Rails.application.config).to receive(:good_job).and_return({ cleanup_interval_jobs: 0 })
-        allow(GoodJob.deprecator).to receive(:warn)
 
         configuration = described_class.new({})
-        expect(configuration.cleanup_interval_jobs).to eq(-1)
-        expect(GoodJob.deprecator).to have_received(:warn)
+        expect(configuration.cleanup_interval_jobs).to eq false
       end
 
-      it 'accepts nil, with deprecation' do
+      it 'coerces nil to default' do
         allow(Rails.application.config).to receive(:good_job).and_return({ cleanup_interval_jobs: nil })
-        allow(GoodJob.deprecator).to receive(:warn)
 
         configuration = described_class.new({})
-        expect(configuration.cleanup_interval_jobs).to be false
-        expect(GoodJob.deprecator).to have_received(:warn)
+        expect(configuration.cleanup_interval_jobs).to be described_class::DEFAULT_CLEANUP_INTERVAL_JOBS
       end
     end
 
@@ -168,20 +164,18 @@
         expect(configuration.cleanup_interval_jobs).to eq(-1)
       end
 
-      it 'accepts 0, without deprecation' do
+      it 'coerces 0 to false' do
         stub_const 'ENV', ENV.to_hash.merge({ 'GOOD_JOB_CLEANUP_INTERVAL_JOBS' => '0' })
 
         configuration = described_class.new({})
         expect(configuration.cleanup_interval_jobs).to be false
       end
 
-      it 'accepts an empty value, with deprecation' do
+      it 'coerces empty value to default' do
         stub_const 'ENV', ENV.to_hash.merge({ 'GOOD_JOB_CLEANUP_INTERVAL_JOBS' => '' })
-        allow(GoodJob.deprecator).to receive(:warn)
 
         configuration = described_class.new({})
-        expect(configuration.cleanup_interval_jobs).to be false
-        expect(GoodJob.deprecator).to have_received(:warn)
+        expect(configuration.cleanup_interval_jobs).to be described_class::DEFAULT_CLEANUP_INTERVAL_JOBS
       end
     end
   end
@@ -207,22 +201,18 @@
         expect(configuration.cleanup_interval_seconds).to be false
       end
 
-      it 'accepts 0, with deprecation' do
+      it 'coerces 0 to false' do
         allow(Rails.application.config).to receive(:good_job).and_return({ cleanup_interval_seconds: 0 })
-        allow(GoodJob.deprecator).to receive(:warn)
 
         configuration = described_class.new({})
-        expect(configuration.cleanup_interval_seconds).to be(-1)
-        expect(GoodJob.deprecator).to have_received(:warn)
+        expect(configuration.cleanup_interval_seconds).to be false
       end
 
-      it 'accepts nil, with deprecation' do
+      it 'coerces nil to default value' do
         allow(Rails.application.config).to receive(:good_job).and_return({ cleanup_interval_seconds: nil })
-        allow(GoodJob.deprecator).to receive(:warn)
 
         configuration = described_class.new({})
-        expect(configuration.cleanup_interval_seconds).to be false
-        expect(GoodJob.deprecator).to have_received(:warn)
+        expect(configuration.cleanup_interval_seconds).to be described_class::DEFAULT_CLEANUP_INTERVAL_SECONDS
       end
     end
 
@@ -241,20 +231,18 @@
         expect(configuration.cleanup_interval_seconds).to eq(-1)
       end
 
-      it 'accepts 0, with deprecation' do
+      it 'coerces 0 to false' do
         stub_const 'ENV', ENV.to_hash.merge({ 'GOOD_JOB_CLEANUP_INTERVAL_SECONDS' => '0' })
 
         configuration = described_class.new({})
         expect(configuration.cleanup_interval_seconds).to be false
       end
 
-      it 'accepts an empty value, with deprecation' do
+      it 'coerces empty value to default' do
         stub_const 'ENV', ENV.to_hash.merge({ 'GOOD_JOB_CLEANUP_INTERVAL_SECONDS' => '' })
-        allow(GoodJob.deprecator).to receive(:warn)
 
         configuration = described_class.new({})
-        expect(configuration.cleanup_interval_seconds).to be false
-        expect(GoodJob.deprecator).to have_received(:warn)
+        expect(configuration.cleanup_interval_seconds).to be described_class::DEFAULT_CLEANUP_INTERVAL_SECONDS
       end
     end
   end