From 1f52c04513dbb09cc9dccab47e3386d90f099586 Mon Sep 17 00:00:00 2001 From: carolyncole <1599081+carolyncole@users.noreply.github.com> Date: Fri, 8 Dec 2023 09:11:05 -0500 Subject: [PATCH] Moving work reload_snapshot from the show to a background job (#1631) This should allow the page to display for very large sets of files and the prvenance to be updated later Also putting this job on it's own queue since it could take a very long time we do not want it subverting other jobs refs #1621 --- app/controllers/works_controller.rb | 2 +- app/jobs/update_snapshot_job.rb | 15 +++++++++++ config/sidekiq.yml | 1 + spec/jobs/update_snapshot_job_spec.rb | 36 +++++++++++++++++++++++++++ spec/requests/works_spec.rb | 11 ++++++++ 5 files changed, 64 insertions(+), 1 deletion(-) create mode 100644 app/jobs/update_snapshot_job.rb create mode 100644 spec/jobs/update_snapshot_job_spec.rb diff --git a/app/controllers/works_controller.rb b/app/controllers/works_controller.rb index 88efa09c..8802bd4c 100644 --- a/app/controllers/works_controller.rb +++ b/app/controllers/works_controller.rb @@ -58,7 +58,7 @@ def new_submission # When requested as .json, return the internal json resource def show @work = Work.find(params[:id]) - @work.reload_snapshots + UpdateSnapshotJob.perform_later(work_id: @work.id, last_snapshot_id: work.upload_snapshots.first&.id) @work_decorator = WorkDecorator.new(@work, current_user) respond_to do |format| diff --git a/app/jobs/update_snapshot_job.rb b/app/jobs/update_snapshot_job.rb new file mode 100644 index 00000000..176e771f --- /dev/null +++ b/app/jobs/update_snapshot_job.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true +class UpdateSnapshotJob < ApplicationJob + queue_as :low + + def perform(work_id:, last_snapshot_id:) + work = Work.find(work_id) + + # do nothing if there was another snapshot saved to the work between when this one was queued and when it started + # Becuse this job can take so long lets not keep running it every time someone looks at the work + # Unless this is a completely new visit after the last queued job completed + return if work.upload_snapshots.first&.id != last_snapshot_id + + work.reload_snapshots + end +end diff --git a/config/sidekiq.yml b/config/sidekiq.yml index e42a9d24..9767767b 100644 --- a/config/sidekiq.yml +++ b/config/sidekiq.yml @@ -6,4 +6,5 @@ production: :concurrency: 5 :queues: - default + - low :max_retries: 25 \ No newline at end of file diff --git a/spec/jobs/update_snapshot_job_spec.rb b/spec/jobs/update_snapshot_job_spec.rb new file mode 100644 index 00000000..3da1a2b4 --- /dev/null +++ b/spec/jobs/update_snapshot_job_spec.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true +require "rails_helper" + +RSpec.describe UpdateSnapshotJob, type: :job do + include ActiveJob::TestHelper + + let(:user) { FactoryBot.create :user } + + let(:s3_file) { FactoryBot.build :s3_file, work:, filename: "#{work.prefix}/test_key" } + + describe "perform" do + subject(:job) do + described_class.perform_later(work_id: work.id, last_snapshot_id: nil) + end + let(:fake_s3_service) { stub_s3 prefix: "10.34770/ackh-7y71/#{work.id}", data: [s3_file] } + let(:work) { FactoryBot.create :approved_work, doi: "10.34770/ackh-7y71" } + + before do + fake_s3_service + end + + it "creates an upload snapshot" do + expect { perform_enqueued_jobs { job } }.to change { UploadSnapshot.count }.by(1) + end + + context "when last snapshot is different" do + subject(:job) do + described_class.perform_later(work_id: work.id, last_snapshot_id: 123) + end + + it "skips processing" do + expect { perform_enqueued_jobs { job } }.to change { UploadSnapshot.count }.by(0) + end + end + end +end diff --git a/spec/requests/works_spec.rb b/spec/requests/works_spec.rb index 7b4801b2..48e1ce4a 100644 --- a/spec/requests/works_spec.rb +++ b/spec/requests/works_spec.rb @@ -2,6 +2,7 @@ require "rails_helper" RSpec.describe "/works", type: :request do + include ActiveJob::TestHelper let(:user) { FactoryBot.create :user } describe "GET /work" do @@ -36,6 +37,7 @@ allow(stubbed).to receive(:reload_snapshots) stubbed_resource = instance_double(PDCMetadata::Resource) allow(stubbed).to receive(:resource).and_return(stubbed_resource) + allow(stubbed).to receive(:upload_snapshots).and_return([]) stubbed end @@ -235,6 +237,15 @@ end it "renders messages generated for the replaced files" do + expect { get work_url(work) }.to have_enqueued_job(UpdateSnapshotJob) + + # Job gets queued so the changes are not present + expect(response.code).to eq "200" + expect(response.body).not_to include("Files Replaced: 1") + + perform_enqueued_jobs + + # a second get will show the updates that occured get work_url(work) expect(response.code).to eq "200"