Merge pull request #3848 from alphagov/remove-unused-queue-option-fro…

…m-service-listeners Remove unused queue option from service listeners
alphagov · Mar 13, 2018 · bda339b · bda339b
2 parents 8ca6f58 + baa0dcd
commit bda339b
Show file tree

Hide file tree

Showing 11 changed files with 16 additions and 144 deletions.
diff --git a/app/services/service_listeners/attachment_access_limited_updater.rb b/app/services/service_listeners/attachment_access_limited_updater.rb
@@ -1,10 +1,9 @@
 module ServiceListeners
   class AttachmentAccessLimitedUpdater
-    attr_reader :attachment_data, :queue
+    attr_reader :attachment_data
 
-    def initialize(attachment_data, queue: nil)
+    def initialize(attachment_data)
       @attachment_data = attachment_data
-      @queue = queue
     end
 
     def update!
@@ -25,12 +24,7 @@ def update!
 
     def enqueue_job(uploader, access_limited)
       legacy_url_path = uploader.asset_manager_path
-      worker.perform_async(legacy_url_path, access_limited: access_limited)
-    end
-
-    def worker
-      worker = AssetManagerUpdateAssetWorker
-      queue.present? ? worker.set(queue: queue) : worker
+      AssetManagerUpdateAssetWorker.perform_async(legacy_url_path, access_limited: access_limited)
     end
   end
 end
diff --git a/app/services/service_listeners/attachment_draft_status_updater.rb b/app/services/service_listeners/attachment_draft_status_updater.rb
@@ -1,10 +1,9 @@
 module ServiceListeners
   class AttachmentDraftStatusUpdater
-    attr_reader :attachment_data, :queue
+    attr_reader :attachment_data
 
-    def initialize(attachment_data, queue: nil)
+    def initialize(attachment_data)
       @attachment_data = attachment_data
-      @queue = queue
     end
 
     def update!
@@ -20,12 +19,7 @@ def update!
 
     def enqueue_job(uploader, draft)
       legacy_url_path = uploader.asset_manager_path
-      worker.perform_async(legacy_url_path, draft: draft)
-    end
-
-    def worker
-      worker = AssetManagerUpdateAssetWorker
-      queue.present? ? worker.set(queue: queue) : worker
+      AssetManagerUpdateAssetWorker.perform_async(legacy_url_path, draft: draft)
     end
   end
 end
diff --git a/app/services/service_listeners/attachment_link_header_updater.rb b/app/services/service_listeners/attachment_link_header_updater.rb
@@ -1,10 +1,9 @@
 module ServiceListeners
   class AttachmentLinkHeaderUpdater
-    attr_reader :attachment_data, :queue
+    attr_reader :attachment_data
 
-    def initialize(attachment_data, queue: nil)
+    def initialize(attachment_data)
       @attachment_data = attachment_data
-      @queue = queue
     end
 
     def update!
@@ -25,12 +24,7 @@ def update!
 
     def enqueue_job(uploader, parent_document_url)
       legacy_url_path = uploader.asset_manager_path
-      worker.perform_async(legacy_url_path, parent_document_url: parent_document_url)
-    end
-
-    def worker
-      worker = AssetManagerUpdateAssetWorker
-      queue.present? ? worker.set(queue: queue) : worker
+      AssetManagerUpdateAssetWorker.perform_async(legacy_url_path, parent_document_url: parent_document_url)
     end
   end
 end
diff --git a/app/services/service_listeners/attachment_redirect_url_updater.rb b/app/services/service_listeners/attachment_redirect_url_updater.rb
@@ -3,11 +3,10 @@ class AttachmentRedirectUrlUpdater
     include Rails.application.routes.url_helpers
     include PublicDocumentRoutesHelper
 
-    attr_reader :attachment_data, :queue
+    attr_reader :attachment_data
 
-    def initialize(attachment_data, queue: nil)
+    def initialize(attachment_data)
       @attachment_data = attachment_data
-      @queue = queue
     end
 
     def update!
@@ -26,12 +25,7 @@ def update!
 
     def enqueue_job(uploader, redirect_url)
       legacy_url_path = uploader.asset_manager_path
-      worker.perform_async(legacy_url_path, redirect_url: redirect_url)
-    end
-
-    def worker
-      worker = AssetManagerUpdateAssetWorker
-      queue.present? ? worker.set(queue: queue) : worker
+      AssetManagerUpdateAssetWorker.perform_async(legacy_url_path, redirect_url: redirect_url)
     end
   end
 end
diff --git a/app/services/service_listeners/attachment_replacement_id_updater.rb b/app/services/service_listeners/attachment_replacement_id_updater.rb
@@ -1,23 +1,15 @@
 module ServiceListeners
   class AttachmentReplacementIdUpdater
-    attr_reader :attachment_data, :queue
+    attr_reader :attachment_data
 
-    def initialize(attachment_data, queue: nil)
+    def initialize(attachment_data)
       @attachment_data = attachment_data
-      @queue = queue
     end
 
     def update!
       return unless attachment_data.present?
 
-      worker.perform_async(attachment_data.id)
-    end
-
-  private
-
-    def worker
-      worker = AssetManagerAttachmentReplacementIdUpdateWorker
-      queue.present? ? worker.set(queue: queue) : worker
+      AssetManagerAttachmentReplacementIdUpdateWorker.perform_async(attachment_data.id)
     end
   end
 end
diff --git a/lib/tasks/asset_manager.rake b/lib/tasks/asset_manager.rake
@@ -27,22 +27,6 @@ namespace :asset_manager do
     end
   end
 
-  namespace :attachments do
-    desc 'Update draft status for Asset Manager assets associated with attachments'
-    task :update_draft_status, %i(first_id last_id) => :environment do |_, args|
-      first_id = args[:first_id]
-      last_id = args[:last_id]
-      abort(update_draft_status_usage_string) unless first_id && last_id
-      options = { start: first_id, finish: last_id }
-      updater = ServiceListeners::AttachmentDraftStatusUpdater
-      FileAttachment.includes(:attachment_data).find_each(options) do |attachment|
-        if File.exist?(attachment.attachment_data.file.path)
-          updater.new(attachment.attachment_data, queue: 'asset_migration').update!
-        end
-      end
-    end
-  end
-
   private
 
   def usage_string
@@ -58,11 +42,4 @@ namespace :asset_manager do
       Where <batch_start> and <batch_end> are integers corresponding to the directory names under `system/uploads/attachment_data/file`. e.g. `system/uploads/attachment_data/file/100` will be migrated if <batch_start> <= 100 and <batch_end> >= 100.
     }
   end
-
-  def update_draft_status_usage_string
-    %{Usage: asset_manager:attachments:update_draft_status[<first_id>,<last_id>]
-
-      Where <first_id> and <last_id> are Attachment database IDs.
-    }
-  end
 end
diff --git a/test/unit/services/service_listeners/attachment_access_limited_updater_test.rb b/test/unit/services/service_listeners/attachment_access_limited_updater_test.rb
@@ -4,9 +4,8 @@ module ServiceListeners
   class AttachmentAccessLimitedUpdaterTest < ActiveSupport::TestCase
     extend Minitest::Spec::DSL
 
-    let(:updater) { AttachmentAccessLimitedUpdater.new(attachment_data, queue: queue) }
+    let(:updater) { AttachmentAccessLimitedUpdater.new(attachment_data) }
     let(:attachment_data) { attachment.attachment_data }
-    let(:queue) { nil }
 
     context 'when attachment has no associated attachment data' do
       let(:attachment) { FactoryBot.create(:html_attachment) }
@@ -93,20 +92,5 @@ class AttachmentAccessLimitedUpdaterTest < ActiveSupport::TestCase
         updater.update!
       end
     end
-
-    context 'when a different queue is specified' do
-      let(:queue) { 'alternative-queue' }
-      let(:worker) { stub('worker', perform_async: nil) }
-      let(:attachment) { FactoryBot.create(:file_attachment) }
-
-      it 'uses that queue' do
-        AssetManagerUpdateAssetWorker.expects(:set)
-          .with(queue: queue).at_least_once.returns(worker)
-        worker.expects(:perform_async)
-          .with(attachment.file.asset_manager_path, anything)
-
-        updater.update!
-      end
-    end
   end
 end
diff --git a/test/unit/services/service_listeners/attachment_draft_status_updater_test.rb b/test/unit/services/service_listeners/attachment_draft_status_updater_test.rb
@@ -32,21 +32,6 @@ class AttachmentDraftStatusUpdaterTest < ActiveSupport::TestCase
 
         updater.update!
       end
-
-      context 'and queue is specified' do
-        let(:queue) { 'alternative_queue' }
-        let(:updater) { AttachmentDraftStatusUpdater.new(attachment_data, queue: queue) }
-        let(:worker) { stub('worker') }
-
-        it 'marks corresponding asset as draft using specified queue' do
-          AssetManagerUpdateAssetWorker.expects(:set)
-            .with(queue: queue).returns(worker)
-          worker.expects(:perform_async)
-            .with(attachment.file.asset_manager_path, draft: true)
-
-          updater.update!
-        end
-      end
     end
 
     context 'when attachment is a PDF' do

diff --git a/test/unit/services/service_listeners/attachment_link_header_updater_test.rb b/test/unit/services/service_listeners/attachment_link_header_updater_test.rb
@@ -41,21 +41,6 @@ class AttachmentLinkHeaderUpdaterTest < ActiveSupport::TestCase
 
         updater.update!
       end
-
-      context 'and queue is specified' do
-        let(:queue) { 'alternative_queue' }
-        let(:updater) { AttachmentLinkHeaderUpdater.new(attachment_data, queue: queue) }
-        let(:worker) { stub('worker') }
-
-        it 'sets parent_document_url of corresponding asset using specified queue' do
-          AssetManagerUpdateAssetWorker.expects(:set)
-            .with(queue: queue).returns(worker)
-          worker.expects(:perform_async)
-            .with(attachment.file.asset_manager_path, parent_document_url: parent_document_url)
-
-          updater.update!
-        end
-      end
     end
 
     context 'when attachment is a PDF' do

diff --git a/test/unit/services/service_listeners/attachment_redirect_url_updater_test.rb b/test/unit/services/service_listeners/attachment_redirect_url_updater_test.rb
@@ -37,21 +37,6 @@ class AttachmentRedirectUrlUpdaterTest < ActiveSupport::TestCase
 
         updater.update!
       end
-
-      context 'and queue is specified' do
-        let(:queue) { 'alternative_queue' }
-        let(:updater) { AttachmentRedirectUrlUpdater.new(attachment_data, queue: queue) }
-        let(:worker) { stub('worker') }
-
-        it 'updates redirect URL of corresponding asset using specified queue' do
-          AssetManagerUpdateAssetWorker.expects(:set)
-            .with(queue: queue).returns(worker)
-          worker.expects(:perform_async)
-            .with(attachment.file.asset_manager_path, redirect_url: redirect_url)
-
-          updater.update!
-        end
-      end
     end
 
     context 'when attachment is a PDF' do

diff --git a/test/unit/services/service_listeners/attachment_replacement_id_updater_test.rb b/test/unit/services/service_listeners/attachment_replacement_id_updater_test.rb
@@ -16,18 +16,6 @@ class AttachmentReplacementIdUpdaterTest < ActiveSupport::TestCase
       end
     end
 
-    context 'when a queue is specified' do
-      let(:updater) { AttachmentReplacementIdUpdater.new(attachment_data, queue: 'a-queue') }
-      let(:attachment_data) { mock('attachment_data', id: 'attachment-data-id') }
-
-      it 'sets the queue on the worker' do
-        worker = mock('worker', perform_async: nil)
-        AssetManagerAttachmentReplacementIdUpdateWorker.expects(:set).with(queue: 'a-queue').returns(worker)
-
-        updater.update!
-      end
-    end
-
     context 'when attachment data is nil' do
       let(:attachment_data) { nil }