From 300e0a34fbfd519e7a140332d8be5c0ce7932e71 Mon Sep 17 00:00:00 2001 From: Chris Roos Date: Mon, 12 Mar 2018 11:42:00 +0000 Subject: [PATCH 01/11] Remove queue option from AttachmentRedirectUrlUpdater We're not using this option yet and removing it will make it easier to move the logic from this listener into a worker. --- .../attachment_redirect_url_updater.rb | 8 +++----- .../attachment_redirect_url_updater_test.rb | 15 --------------- 2 files changed, 3 insertions(+), 20 deletions(-) diff --git a/app/services/service_listeners/attachment_redirect_url_updater.rb b/app/services/service_listeners/attachment_redirect_url_updater.rb index 133fdebee4b..205a83cdd9b 100644 --- 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! @@ -30,8 +29,7 @@ def enqueue_job(uploader, redirect_url) end def worker - worker = AssetManagerUpdateAssetWorker - queue.present? ? worker.set(queue: queue) : worker + AssetManagerUpdateAssetWorker end end end 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 index ca19c78412d..30e47a5db5e 100644 --- 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 From f4b9476ef35dc4f9c00184fa76cad43410ab0cac Mon Sep 17 00:00:00 2001 From: Chris Roos Date: Mon, 12 Mar 2018 11:45:56 +0000 Subject: [PATCH 02/11] Inline AttachmentRedirectUrlUpdater#worker --- .../service_listeners/attachment_redirect_url_updater.rb | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/app/services/service_listeners/attachment_redirect_url_updater.rb b/app/services/service_listeners/attachment_redirect_url_updater.rb index 205a83cdd9b..4f11534254f 100644 --- a/app/services/service_listeners/attachment_redirect_url_updater.rb +++ b/app/services/service_listeners/attachment_redirect_url_updater.rb @@ -25,11 +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 - AssetManagerUpdateAssetWorker + AssetManagerUpdateAssetWorker.perform_async(legacy_url_path, redirect_url: redirect_url) end end end From 640af8f7fe3e3742645916f5e16ed7759fb8e185 Mon Sep 17 00:00:00 2001 From: Chris Roos Date: Mon, 12 Mar 2018 11:42:00 +0000 Subject: [PATCH 03/11] Remove queue option from AttachmentLinkHeaderUpdater We're not using this option yet and removing it will make it easier to move the logic from this listener into a worker. --- .../attachment_link_header_updater.rb | 8 +++----- .../attachment_link_header_updater_test.rb | 15 --------------- 2 files changed, 3 insertions(+), 20 deletions(-) diff --git a/app/services/service_listeners/attachment_link_header_updater.rb b/app/services/service_listeners/attachment_link_header_updater.rb index b159fc013cb..0895461ff85 100644 --- 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! @@ -29,8 +28,7 @@ def enqueue_job(uploader, parent_document_url) end def worker - worker = AssetManagerUpdateAssetWorker - queue.present? ? worker.set(queue: queue) : worker + AssetManagerUpdateAssetWorker end end end 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 index 619b38b40f4..c3fda5c5b8e 100644 --- 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 From f5ce01f20a474df7d7e9b9dc397d731e93efe691 Mon Sep 17 00:00:00 2001 From: Chris Roos Date: Mon, 12 Mar 2018 11:49:01 +0000 Subject: [PATCH 04/11] Inline AttachmentLinkHeaderUpdater#worker --- .../service_listeners/attachment_link_header_updater.rb | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/app/services/service_listeners/attachment_link_header_updater.rb b/app/services/service_listeners/attachment_link_header_updater.rb index 0895461ff85..c2ef18b63a4 100644 --- a/app/services/service_listeners/attachment_link_header_updater.rb +++ b/app/services/service_listeners/attachment_link_header_updater.rb @@ -24,11 +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 - AssetManagerUpdateAssetWorker + AssetManagerUpdateAssetWorker.perform_async(legacy_url_path, parent_document_url: parent_document_url) end end end From 3f6e65bd48fc28b2ae48450cd9f0f8cde09512f7 Mon Sep 17 00:00:00 2001 From: Chris Roos Date: Mon, 12 Mar 2018 11:42:00 +0000 Subject: [PATCH 05/11] Remove queue option from AttachmentAccessLimitedUpdater We're not using this option yet and removing it will make it easier to move the logic from this listener into a worker. --- .../attachment_access_limited_updater.rb | 8 +++----- .../attachment_access_limited_updater_test.rb | 18 +----------------- 2 files changed, 4 insertions(+), 22 deletions(-) diff --git a/app/services/service_listeners/attachment_access_limited_updater.rb b/app/services/service_listeners/attachment_access_limited_updater.rb index c70be4c688d..7439583ca4b 100644 --- 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! @@ -29,8 +28,7 @@ def enqueue_job(uploader, access_limited) end def worker - worker = AssetManagerUpdateAssetWorker - queue.present? ? worker.set(queue: queue) : worker + AssetManagerUpdateAssetWorker end 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 index a5b83ab2d6e..efb82b87a31 100644 --- 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 From 3cee6523fe21cc0e1b17c1b76c49bd76fc19ca2f Mon Sep 17 00:00:00 2001 From: Chris Roos Date: Mon, 12 Mar 2018 11:51:31 +0000 Subject: [PATCH 06/11] Inline AttachmentAccessLimitedUpdater#worker --- .../service_listeners/attachment_access_limited_updater.rb | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/app/services/service_listeners/attachment_access_limited_updater.rb b/app/services/service_listeners/attachment_access_limited_updater.rb index 7439583ca4b..ca9e8aca058 100644 --- a/app/services/service_listeners/attachment_access_limited_updater.rb +++ b/app/services/service_listeners/attachment_access_limited_updater.rb @@ -24,11 +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 - AssetManagerUpdateAssetWorker + AssetManagerUpdateAssetWorker.perform_async(legacy_url_path, access_limited: access_limited) end end end From 81056f173b4b526b7b8eaa4ebde7478c647e9cb5 Mon Sep 17 00:00:00 2001 From: Chris Roos Date: Mon, 12 Mar 2018 11:54:44 +0000 Subject: [PATCH 07/11] Remove attachments:update_draft_status Rake task We don't have any plans to use this Rake task and removing it will allow us to simplify the `AttachmentDraftStatusUpdater` by removing the `queue` option. --- lib/tasks/asset_manager.rake | 23 ----------------------- 1 file changed, 23 deletions(-) diff --git a/lib/tasks/asset_manager.rake b/lib/tasks/asset_manager.rake index e9fb282574d..93e698889d2 100644 --- 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 and 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 <= 100 and >= 100. } end - - def update_draft_status_usage_string - %{Usage: asset_manager:attachments:update_draft_status[,] - - Where and are Attachment database IDs. - } - end end From 7df9929c324fdff53f732b865668c9957a13f664 Mon Sep 17 00:00:00 2001 From: Chris Roos Date: Mon, 12 Mar 2018 11:42:00 +0000 Subject: [PATCH 08/11] Remove queue option from AttachmentDraftStatusUpdater We're not using this option yet and removing it will make it easier to move the logic from this listener into a worker. --- .../attachment_draft_status_updater.rb | 8 +++----- .../attachment_draft_status_updater_test.rb | 15 --------------- 2 files changed, 3 insertions(+), 20 deletions(-) diff --git a/app/services/service_listeners/attachment_draft_status_updater.rb b/app/services/service_listeners/attachment_draft_status_updater.rb index 074e4b4bc11..18295a5aa0c 100644 --- 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! @@ -24,8 +23,7 @@ def enqueue_job(uploader, draft) end def worker - worker = AssetManagerUpdateAssetWorker - queue.present? ? worker.set(queue: queue) : worker + AssetManagerUpdateAssetWorker 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 index e4be1cde477..b433c2dcf74 100644 --- 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 From 80e87d6df926bbbbd68d345334479203d782c4a2 Mon Sep 17 00:00:00 2001 From: Chris Roos Date: Mon, 12 Mar 2018 11:57:21 +0000 Subject: [PATCH 09/11] Inline AttachmentDraftStatusUpdater#worker --- .../service_listeners/attachment_draft_status_updater.rb | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/app/services/service_listeners/attachment_draft_status_updater.rb b/app/services/service_listeners/attachment_draft_status_updater.rb index 18295a5aa0c..fa728f455c7 100644 --- a/app/services/service_listeners/attachment_draft_status_updater.rb +++ b/app/services/service_listeners/attachment_draft_status_updater.rb @@ -19,11 +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 - AssetManagerUpdateAssetWorker + AssetManagerUpdateAssetWorker.perform_async(legacy_url_path, draft: draft) end end end From 37bb8e50f32dc1f9b8d37a662dcfdf1a11ef3e10 Mon Sep 17 00:00:00 2001 From: Chris Roos Date: Mon, 12 Mar 2018 11:42:00 +0000 Subject: [PATCH 10/11] Remove queue option from AttachmentReplacementIdUpdater We're not using this option yet and don't have any plans to use it in future. --- .../attachment_replacement_id_updater.rb | 8 +++----- .../attachment_replacement_id_updater_test.rb | 12 ------------ 2 files changed, 3 insertions(+), 17 deletions(-) diff --git a/app/services/service_listeners/attachment_replacement_id_updater.rb b/app/services/service_listeners/attachment_replacement_id_updater.rb index e7db36e626b..dc67ca35270 100644 --- a/app/services/service_listeners/attachment_replacement_id_updater.rb +++ b/app/services/service_listeners/attachment_replacement_id_updater.rb @@ -1,10 +1,9 @@ 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! @@ -16,8 +15,7 @@ def update! private def worker - worker = AssetManagerAttachmentReplacementIdUpdateWorker - queue.present? ? worker.set(queue: queue) : worker + AssetManagerAttachmentReplacementIdUpdateWorker end end end 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 index 464efbea325..7e3f630f92f 100644 --- 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 } From baa0dcde7cbbfa4105f9d3d6ec38d17ea44930d1 Mon Sep 17 00:00:00 2001 From: Chris Roos Date: Mon, 12 Mar 2018 12:00:53 +0000 Subject: [PATCH 11/11] Inline AttachmentReplacementIdUpdater#worker --- .../attachment_replacement_id_updater.rb | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/app/services/service_listeners/attachment_replacement_id_updater.rb b/app/services/service_listeners/attachment_replacement_id_updater.rb index dc67ca35270..6ebf06f20fa 100644 --- a/app/services/service_listeners/attachment_replacement_id_updater.rb +++ b/app/services/service_listeners/attachment_replacement_id_updater.rb @@ -9,13 +9,7 @@ def initialize(attachment_data) def update! return unless attachment_data.present? - worker.perform_async(attachment_data.id) - end - - private - - def worker - AssetManagerAttachmentReplacementIdUpdateWorker + AssetManagerAttachmentReplacementIdUpdateWorker.perform_async(attachment_data.id) end end end