Merge pull request #41 from salsify/ms/revert-version-13

Reverts Version 0.13

Author: marcoserrato

CHANGELOG.md

@@ -1,5 +1,8 @@
 # Changelog
 
+## 0.14.0
+- Reverts changes made in version 0.13.
+
 ## 0.13.0
 - Moves `on_cancellation` logic from the before delayed job lifecycle hook to the after hook.
 - Gem will now fail to load if `Delayed::Worker.destroy_failed_jobs` is set to true.

lib/delayed/job_groups/errors.rb

@@ -62,18 +62,8 @@ def unblock
       end
 
       def cancel
-        with_lock do
-          return if destroyed?
-
-          Delayed::Job.enqueue(on_cancellation_job, on_cancellation_job_options || {}) if on_cancellation_job
-          destroy
-        end
-      rescue ActiveRecord::RecordNotFound
-        # The first failing job to attempt cancelling the job group will enqueue the
-        # on cancellation job and destroy the group. Any other concurrently failing job
-        # in the same group can therefore silently return if the job group has already
-        # been destroyed.
-        nil
+        Delayed::Job.enqueue(on_cancellation_job, on_cancellation_job_options || {}) if on_cancellation_job
+        destroy
       end
 
       def check_for_completion(skip_pending_jobs_check: false)

lib/delayed/job_groups/job_group.rb

@@ -17,13 +17,7 @@ def job.max_attempts
           end
         end
 
-        # In order to allow individual jobs in the JobGroup to perform cleanup activities upon
-        # job failure, a JobGroups on cancellation job must be enqueued in the after failure
-        # delayed job lifecycle hook. If both were to be performed in the same hook, the job's
-        # failure hook and the on cancellation job may run at the same time since hook execution
-        # order is never guaranteed. This is particular important if the on cancellation job
-        # depends on state set in the failure hook of an individual job.
-        lifecycle.after(:failure) do |_worker, job|
+        lifecycle.before(:failure) do |_worker, job|
           # If a job in the job group fails, then cancel the whole job group.
           # Need to check that the job group is present since another
           # job may have concurrently cancelled it.

lib/delayed/job_groups/plugin.rb

@@ -4,15 +4,6 @@ module Delayed
   module JobGroups
     class Railtie < ::Rails::Railtie
       config.after_initialize do
-
-        # On cancellation checks are performed in the after failure delayed job lifecycle, however
-        # https://github.com/collectiveidea/delayed_job/blob/master/lib/delayed/worker.rb#L268 may
-        # delete jobs before the hook runs. This could cause a successful job in the same group to
-        # complete the group instead of the group being cancelled. Therefore, we must ensure that
-        # the Delayed::Worker.destroy_failed_jobs is set to false, guaranteeing that the group is
-        # never empty if failure occurs.
-        raise Delayed::JobGroups::IncompatibleWithDelayedJobError if Delayed::Worker.destroy_failed_jobs
-
         Delayed::Backend::ActiveRecord::Job.include(Delayed::JobGroups::JobExtensions)
       end
     end

lib/delayed/job_groups/railtie.rb

@@ -2,6 +2,6 @@
 
 module Delayed
   module JobGroups
-    VERSION = '0.13.0'
+    VERSION = '0.14.0'
   end
 end

lib/delayed/job_groups/version.rb

@@ -4,7 +4,6 @@
 require 'active_record'
 require 'delayed_job'
 require 'delayed_job_active_record'
-require 'delayed/job_groups/errors'
 require 'delayed/job_groups/compatibility'
 require 'delayed/job_groups/complete_stuck_job_groups_job'
 require 'delayed/job_groups/job_extensions'
@@ -19,7 +18,6 @@
 else
   # Do the same as in the railtie
   Delayed::Backend::ActiveRecord::Job.include(Delayed::JobGroups::JobExtensions)
-  raise Delayed::JobGroups::IncompatibleWithDelayedJobError if Delayed::Worker.destroy_failed_jobs
 end
 
 Delayed::Worker.plugins << Delayed::JobGroups::Plugin

lib/delayed_job_groups_plugin.rb

@@ -299,28 +299,11 @@
   end
 
   describe "#cancel" do
-    subject(:job_group) do
-      create(
-        :job_group,
-        on_completion_job: on_completion_job,
-        on_completion_job_options: on_completion_job_options,
-        on_cancellation_job: on_cancellation_job,
-        on_cancellation_job_options: on_cancellation_job_options,
-        blocked: blocked
-      )
-    end
-
     let!(:queued_job) { Delayed::Job.create!(job_group_id: job_group.id) }
     let!(:running_job)  { Delayed::Job.create!(job_group_id: job_group.id, locked_at: Time.now, locked_by: 'test') }
-    let(:before_hook) {}
-    let(:on_cancellation_job) { 'dummy job' }
-    let(:on_cancellation_job_options) do
-      { foo: 'bar' }
-    end
-    let(:cancel) { true }
 
     before do
-      job_group.cancel if cancel
+      job_group.cancel
     end
 
     it "destroys the job group" do
@@ -334,21 +317,6 @@
     it "does not destroy running jobs" do
       expect(running_job).not_to have_been_destroyed
     end
-
-    context "when already cancelled" do
-      let(:cancel) { false }
-
-      it "skips cancel block if already cancelled" do
-        other = Delayed::JobGroups::JobGroup.find(job_group.id)
-        job_group.cancel
-
-        other.cancel
-        other.cancel
-
-        expect(Delayed::Job)
-          .to have_received(:enqueue).with(on_cancellation_job, on_cancellation_job_options).once
-      end
-    end
   end
 
   describe "#failure_cancels_group?" do

spec/delayed/job_groups/job_group_spec.rb

@@ -13,11 +13,6 @@
 
 require 'rspec/its'
 require 'database_cleaner'
-require 'delayed_job'
-
-Delayed::Worker.read_ahead = 1
-Delayed::Worker.destroy_failed_jobs = false
-
 require 'delayed_job_groups_plugin'
 require 'factory_bot'
 require 'yaml'
@@ -28,6 +23,9 @@
 
 FileUtils.makedirs('log')
 
+Delayed::Worker.read_ahead = 1
+Delayed::Worker.destroy_failed_jobs = false
+
 Delayed::Worker.logger = Logger.new('log/test.log')
 Delayed::Worker.logger.level = Logger::DEBUG
 ActiveRecord::Base.logger = Delayed::Worker.logger