Discrete cleanup (#1401)

* Remove a bit of code around discrete distinction

* Ignore the `is_discrete` column

For a safe migration, this column must be ignored first

* Deprecate `GoodJob.v4_ready?`

If you're reading this deprecation you're already all good (:

app/models/good_job/base_execution.rb

@@ -25,6 +25,8 @@ class BaseExecution < BaseRecord
     self.advisory_lockable_column = 'active_job_id'
     self.implicit_order_column = 'created_at'
 
+    self.ignored_columns += ["is_discrete"]
+
     define_model_callbacks :perform
     define_model_callbacks :perform_unlocked, only: :after
 
@@ -216,14 +218,6 @@ def params_execution_count
       def coalesce_scheduled_at_created_at
         arel_table.coalesce(arel_table['scheduled_at'], arel_table['created_at'])
       end
-
-      def discrete_support?
-        true
-      end
-    end
-
-    def discrete?
-      is_discrete?
     end
 
     # Build an ActiveJob instance and deserialize the arguments, using `#active_job_data`.
@@ -545,17 +539,6 @@ def executable?
       false
     end
 
-    def make_discrete
-      self.is_discrete = true
-      self.id = active_job_id
-      self.job_class = serialized_params['job_class']
-      self.executions_count ||= 0
-
-      current_time = Time.current
-      self.created_at ||= current_time
-      self.scheduled_at ||= current_time
-    end
-
     # Return formatted serialized_params for display in the dashboard
     # @return [Hash]
     def display_serialized_params

app/models/good_job/job.rb

@@ -46,8 +46,6 @@ class Job < BaseExecution
     # Errored but will not be retried
     scope :discarded, -> { finished.where.not(error: nil) }
 
-    scope :unfinished_undiscrete, -> { where(finished_at: nil, retried_good_job_id: nil, is_discrete: [nil, false]) }
-
     # TODO: it would be nice to enforce these values at the model
     # validates :active_job_id, presence: true
     # validates :scheduled_at, presence: true

demo/db/seeds.rb

@@ -2,7 +2,6 @@
 time_increments = (1.minute..10.minutes).to_a
 job_classes = ['ExampleJob', 'OtherJob']
 queue_names = ["default", "mice", "elephants"]
-discrete_is_supported = GoodJob::BaseExecution.discrete_support?
 
 jobs_data = []
 loop do
@@ -36,8 +35,7 @@
     scheduled_at: nil,
     performed_at: nil,
     finished_at: nil,
-    error: nil,
-    is_discrete: discrete_is_supported
+    error: nil
   }
 
   start_date += time_increments.sample

lib/good_job.rb

@@ -269,7 +269,11 @@ def self.perform_inline(queue_string = "*", limit: nil)
   # Tests whether GoodJob can be safely upgraded to v4
   # @return [Boolean]
   def self.v4_ready?
-    GoodJob::Job.discrete_support? && GoodJob::Job.where(finished_at: nil).where(is_discrete: [nil, false]).none?
+    GoodJob.deprecator.warn(<<~MSG)
+      Calling `GoodJob.v4_ready?` is deprecated and will be removed in GoodJob v5.
+      If you are reading this deprecation you are already on v4.
+    MSG
+    true
   end
 
   # Deprecator for providing deprecation warnings.

spec/app/models/good_job/job_spec.rb

@@ -359,8 +359,6 @@ def perform(*)
     end
 
     before do
-      allow(described_class).to receive(:discrete_support?).and_return(false)
-
       stub_const "RUN_JOBS", Concurrent::Array.new
       stub_const 'TestJob', (Class.new(ActiveJob::Base) do
         self.queue_name = 'test'
@@ -951,8 +949,6 @@ def job_params
       context 'when Discrete' do
         before do
           ActiveJob::Base.queue_adapter = GoodJob::Adapter.new(execution_mode: :inline)
-          allow(described_class).to receive(:discrete_support?).and_return(true)
-          good_job.update!(is_discrete: true)
         end
 
         it 'updates the Execution record and creates a DiscreteExecution record' do
@@ -983,10 +979,8 @@ def job_params
           let!(:good_job) { described_class.enqueue(active_job) }
 
           before do
-            allow(described_class).to receive(:discrete_support?).and_return(true)
             allow(GoodJob).to receive(:preserve_job_records).and_return(true)
             TestJob.retry_on(StandardError, wait: 1.hour, attempts: 2) { nil }
-            good_job.update!(is_discrete: true)
           end
 
           it 'updates the existing Execution/Job record instead of creating a new one' do

spec/lib/good_job/active_job_extensions/concurrency_spec.rb

@@ -248,24 +248,6 @@ def perform(*)
           expect(GoodJob::Job.first.concurrency_key).to be_present
           expect(GoodJob::Job.first).not_to be_finished
         end
-
-        context 'when not discrete' do
-          it 'preserves the key value across retries' do
-            TestJob.set(wait_until: 5.minutes.ago).perform_later(name: "Alice")
-            GoodJob::Job.first.update!(is_discrete: false)
-
-            original_concurrency_key = GoodJob::Job.last.concurrency_key
-            expect(original_concurrency_key).to be_present
-
-            begin
-              GoodJob.perform_inline
-            rescue StandardError
-              nil
-            end
-
-            expect(GoodJob::Job.last.concurrency_key).to eq original_concurrency_key
-          end
-        end
       end
     end
 

spec/lib/good_job/adapter_spec.rb

@@ -27,8 +27,6 @@
 
   describe '#enqueue' do
     it 'sets default values' do
-      allow(GoodJob::Job).to receive(:discrete_support?).and_return(true)
-
       active_job = ExampleJob.new
       adapter.enqueue(active_job)
 
@@ -37,16 +35,6 @@
         priority: 0,
         scheduled_at: be_within(1.second).of(Time.current)
       )
-
-      allow(GoodJob::Job).to receive(:discrete_support?).and_return(false)
-      active_job = ExampleJob.new
-      adapter.enqueue(active_job)
-
-      expect(GoodJob::Job.last).to have_attributes(
-        queue_name: 'default',
-        priority: 0
-        # scheduled_at: be_within(1.second).of(Time.current) # TODO: this should be expected
-      )
     end
 
     it 'calls GoodJob::Job.enqueue with parameters' do
@@ -129,7 +117,6 @@ def perform(succeed: true)
     end
 
     it 'sets default values' do
-      allow(GoodJob::Job).to receive(:discrete_support?).and_return(true)
       active_job = ExampleJob.new
       adapter.enqueue_at(active_job, nil)
 
@@ -138,16 +125,6 @@ def perform(succeed: true)
         priority: 0,
         scheduled_at: be_within(1.second).of(Time.current)
       )
-
-      allow(GoodJob::Job).to receive(:discrete_support?).and_return(false)
-      active_job = ExampleJob.new
-      adapter.enqueue_at(active_job, nil)
-
-      expect(GoodJob::Job.last).to have_attributes(
-        queue_name: 'default',
-        priority: 0
-        # scheduled_at: be_within(1.second).of(Time.current) # TODO: this should be expected
-      )
     end
   end
 
@@ -181,7 +158,6 @@ def perform(succeed: true)
     end
 
     it 'sets default values' do
-      allow(GoodJob::Job).to receive(:discrete_support?).and_return(true)
       active_job = ExampleJob.new
       adapter.enqueue_all([active_job])
 
@@ -190,16 +166,6 @@ def perform(succeed: true)
         priority: 0,
         scheduled_at: be_within(1.second).of(Time.current)
       )
-
-      allow(GoodJob::Job).to receive(:discrete_support?).and_return(false)
-      active_job = ExampleJob.new
-      adapter.enqueue_at(active_job, nil)
-
-      expect(GoodJob::Job.last).to have_attributes(
-        queue_name: 'default',
-        priority: 0
-        # scheduled_at: be_within(1.second).of(Time.current) # TODO: this should be expected
-      )
     end
 
     context 'when a job fails to enqueue' do

spec/lib/good_job_spec.rb

@@ -164,24 +164,10 @@ def perform(succeed: true)
   end
 
   describe '#v4_ready?' do
-    it "is true when all jobs are discrete" do
+    it "is true" do
+      allow(described_class.deprecator).to receive(:warn)
       expect(described_class.v4_ready?).to eq true
-    end
-
-    it "is false when a job is not discrete" do
-      GoodJob::BaseRecord.uncached do
-        job = GoodJob::Job.create!(finished_at: nil, is_discrete: false)
-        expect(described_class.v4_ready?).to eq false
-
-        job.update! finished_at: nil, is_discrete: nil
-        expect(described_class.v4_ready?).to eq false
-
-        job.update! finished_at: nil, is_discrete: true
-        expect(described_class.v4_ready?).to eq true
-
-        job.update! finished_at: Time.current, is_discrete: nil
-        expect(described_class.v4_ready?).to eq true
-      end
+      expect(described_class.deprecator).to have_received(:warn)
     end
   end
 end