Skip to content

Commit

Permalink
Moving work reload_snapshot from the show to a background job (#1631)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
carolyncole authored Dec 8, 2023
1 parent 4978040 commit 1f52c04
Show file tree
Hide file tree
Showing 5 changed files with 64 additions and 1 deletion.
2 changes: 1 addition & 1 deletion app/controllers/works_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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|
Expand Down
15 changes: 15 additions & 0 deletions app/jobs/update_snapshot_job.rb
Original file line number Diff line number Diff line change
@@ -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
1 change: 1 addition & 0 deletions config/sidekiq.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,5 @@ production:
:concurrency: 5
:queues:
- default
- low
:max_retries: 25
36 changes: 36 additions & 0 deletions spec/jobs/update_snapshot_job_spec.rb
Original file line number Diff line number Diff line change
@@ -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
11 changes: 11 additions & 0 deletions spec/requests/works_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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"
Expand Down

0 comments on commit 1f52c04

Please sign in to comment.