Merge pull request #331 from lardawge/ls/add-support-for-removed-records

Add support for deleted records before processing

Author: lardawge

CHANGELOG.md

@@ -1,3 +1,8 @@
+## 1.1.0
+
+### enhancements
+  * Suppress NotFoundError if a record gets deleted before it is processed. This is configurable and defaults to true. [lardawge]
+
 ## 1.0.3
 
 ### enhancements

README.md

@@ -3,11 +3,10 @@
 [![Build Status](https://github.com/lardawge/carrierwave_backgrounder/actions/workflows/ruby-ci.yml/badge.svg)](https://github.com/lardawge/carrierwave_backgrounder/actions/workflows/ruby-ci.yml)
 [![Code Climate](https://codeclimate.com/github/lardawge/carrierwave_backgrounder.png)](https://codeclimate.com/github/lardawge/carrierwave_backgrounder)
 
-NOTICE: Version 1.0.0 contains breaking changes if you are coming from an earlier version.
-The most notible change is the removal of queue backend options other than active_job and sidekiq.
-If you are using other backends, switch over to active_job which should support your preference.
-If you are using Sidekiq, there is nothing to change.
+---
+NOTICE: Version 1.1.0 contains a change in behavior from previous version. When a record is deleted before the job is picked up, it will no longer raise an error. Prior to this change, when using `process_in_background`, if a record was missing, an error was raised. Some users might have relied on that. By default, this will no longer happen. If you want to maintain that behavior, you must set the `suppress_record_not_found_errors` configuration to `false`. This will raise a RecordNotFound error.
 
+---
 I am a fan of CarrierWave. That being said, I don't like tying up requests waiting for images to process.
 
 This gem addresses that by offloading processing or storaging/processing to a background task.

lib/backgrounder/workers/base.rb

@@ -14,12 +14,13 @@ def perform(*args)
         set_args(*args) if args.present?
         self.record = constantized_resource.find id
       rescue *not_found_errors
+        raise not_found_errors.first unless CarrierWave::Backgrounder.suppress_not_found_errors
       end
 
       private
 
       def not_found_errors
-        [].tap do |errors|
+        @not_found_errors ||= [].tap do |errors|
           errors << ::ActiveRecord::RecordNotFound      if defined?(::ActiveRecord)
           errors << ::Mongoid::Errors::DocumentNotFound if defined?(::Mongoid)
         end

lib/backgrounder/workers/process_asset_mixin.rb

@@ -11,6 +11,9 @@ def self.included(base)
 
       def perform(*args)
         record = super(*args)
+
+        return unless record
+
         record.send(:"process_#{column}_upload=", true)
         asset = record.send(:"#{column}")
 

lib/carrierwave_backgrounder.rb

@@ -9,17 +9,17 @@ module Backgrounder
     include Support::Backends
 
     class << self
-      attr_reader :worker_klass
+      attr_reader :worker_klass, :suppress_not_found_errors
     end
 
     def self.configure
       yield self
 
+      @suppress_not_found_errors ||= true
+
       case backend
       when :active_job
         @worker_klass = 'CarrierWave::Workers::ActiveJob'
-
-        require 'active_job'
         require 'backgrounder/workers/active_job/process_asset'
         require 'backgrounder/workers/active_job/store_asset'
 
@@ -34,7 +34,6 @@ def self.configure
       when :sidekiq
         @worker_klass = 'CarrierWave::Workers'
 
-        require 'sidekiq'
         ::CarrierWave::Workers::ProcessAsset.class_eval do
           include ::Sidekiq::Worker
         end
@@ -43,6 +42,10 @@ def self.configure
         end
       end
     end
+
+    def self.suppress_record_not_found_errors(surpress_errors = true)
+      @suppress_not_found_errors = surpress_errors
+    end
   end
 end
 

lib/generators/carrierwave_backgrounder/templates/config/initializers/carrierwave_backgrounder.rb

@@ -1,4 +1,7 @@
 CarrierWave::Backgrounder.configure do |c|
   c.backend :active_job, queue: :carrierwave
   # c.backend :sidekiq, queue: :carrierwave
+
+  ## Uncomment if you would like a NotFoundError raised if a record is deleted before processing
+  # c.suppress_record_not_found_errors false
 end

spec/integrations/process_in_background_spec.rb

@@ -22,6 +22,35 @@
     end
   end
 
+  context 'when a record gets deleted before it is processed' do
+    context 'and suppress_record_not_found_errors is set to true' do
+      before do
+        admin.update(avatar: load_file('test-1.jpg'))
+      end
+
+      it 'does not raise an error' do
+        admin.delete
+        expect { process_latest_sidekiq_job }.not_to raise_error
+      end
+    end
+
+    context 'and suppress_record_not_found_errors is set to false' do
+      before do
+        admin.update(avatar: load_file('test-1.jpg'))
+        CarrierWave::Backgrounder.suppress_record_not_found_errors(false)
+      end
+
+      after do
+        CarrierWave::Backgrounder.suppress_record_not_found_errors(true)
+      end
+
+      it 'raises an error' do
+        admin.delete
+        expect { process_latest_sidekiq_job }.to raise_error(ActiveRecord::RecordNotFound)
+      end
+    end
+  end
+
   context 'when processing the worker' do
     before do
       admin.update(avatar: load_file('test-1.jpg'))

spec/integrations/store_in_background_spec.rb

@@ -68,6 +68,35 @@
     end
   end
 
+  context 'when a record gets deleted before it is processed' do
+    context 'and suppress_record_not_found_errors is set to true' do
+      before do
+        user.update(avatar: load_file('test-1.jpg'))
+      end
+
+      it 'does not raise an error' do
+        user.delete
+        expect { process_latest_sidekiq_job }.not_to raise_error
+      end
+    end
+
+    context 'and suppress_record_not_found_errors is set to false' do
+      before do
+        user.update(avatar: load_file('test-1.jpg'))
+        CarrierWave::Backgrounder.suppress_record_not_found_errors(false)
+      end
+
+      after do
+        CarrierWave::Backgrounder.suppress_record_not_found_errors(true)
+      end
+
+      it 'raises an error' do
+        user.delete
+        expect { process_latest_sidekiq_job }.to raise_error(ActiveRecord::RecordNotFound)
+      end
+    end
+  end
+
   context 'when setting a column for removal' do
     let!(:user) {
       Sidekiq::Testing.inline! do

spec/support/dummy_app/config/initializers/carrierwave_backgrounder.rb

@@ -1,3 +1,6 @@
+require 'active_job'
+require 'sidekiq'
+
 queue_adapter = ENV['QUEUE_ADAPTER'] || :active_job
 CarrierWave::Backgrounder.configure do |c|
   c.backend queue_adapter.to_sym, queue: :carrierwave