Skip to content

Commit

Permalink
DoiVerificationJob tweaks (#697)
Browse files Browse the repository at this point in the history
* DoiVerificationJob was running in sync with the changes previous to this commit.  These changes get it to run async and handled by DelayedJob.  I made some changes to the specs to handle this, and added a tests that confirms the job is added to the queue.

* Fixed up the OaWorkflowService and fixed some tests
  • Loading branch information
ajkiessl authored Feb 15, 2023
1 parent 1f8068b commit fb09c0a
Show file tree
Hide file tree
Showing 7 changed files with 39 additions and 29 deletions.
2 changes: 1 addition & 1 deletion app/importers/activity_insight_importer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ def call
file = ActivityInsightOaFile.create(location: activity_insight_file_location)
pub_record.activity_insight_oa_files << file
pub_record.save!
DoiVerificationJob.new.perform(pub_record)
DoiVerificationJob.perform_later(pub_record.id)
end
end
end
Expand Down
1 change: 1 addition & 0 deletions app/jobs/ai_publication_export_job.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# frozen_string_literal: true

# TODO: This does not work anymore. This syntax is used with SuckerPunch, not DelayedJob
class AiPublicationExportJob
def perform(publication_ids, target)
publications = Publication.find(publication_ids)
Expand Down
7 changes: 5 additions & 2 deletions app/jobs/doi_verification_job.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
# frozen_string_literal: true

class DoiVerificationJob
def perform(publication)
class DoiVerificationJob < ApplicationJob
queue_as 'default'

def perform(publication_id)
publication = Publication.find(publication_id)
return if publication.doi_verified == true

if publication.doi.present?
Expand Down
2 changes: 1 addition & 1 deletion app/services/oa_workflow_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ def workflow
Publication.needs_doi_verification.each do |pub|
pub.oa_workflow_state = 'automatic DOI verification pending'
pub.save!
DoiVerificationJob.new.perform(pub)
DoiVerificationJob.perform_later(pub.id)
rescue StandardError
pub.doi_verified = false
pub.save!
Expand Down
5 changes: 2 additions & 3 deletions spec/component/importers/activity_insight_importer_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

describe ActivityInsightImporter do
let(:importer) { described_class.new }
let(:doi_job) { instance_spy DoiVerificationJob }

before do
allow(HTTParty).to receive(:get).with('https://webservices.digitalmeasures.com/login/service/v4/User',
Expand All @@ -24,7 +23,7 @@
password: 'secret' }).and_return(
Rails.root.join('spec', 'fixtures', 'activity_insight_user_def45.xml').read
)
allow(DoiVerificationJob).to receive(:new).and_return(doi_job)
allow(DoiVerificationJob).to receive(:perform_later)
end

describe '#call' do
Expand Down Expand Up @@ -4951,7 +4950,7 @@
importer.call
p4 = PublicationImport.find_by(source: 'Activity Insight',
source_identifier: '171620739090').publication
expect(doi_job).to have_received(:perform).with(p4)
expect(DoiVerificationJob).to have_received(:perform_later).with(p4.id)
end

it 'does not import ActivityInsightOaFiles for imported publications without postprint/open access file locations' do
Expand Down
36 changes: 22 additions & 14 deletions spec/component/jobs/doi_verification_job_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,23 @@
require 'component/component_spec_helper'

describe DoiVerificationJob, type: :job do
describe '#perform' do
let(:job) { described_class }

describe '.perform_later' do
ActiveJob::Base.queue_adapter = :test
it 'enqueues a job' do
expect { job.perform_later(1) }.to have_enqueued_job.with(1).on_queue('default')
end
end

describe '#perform_now' do
let(:publication) { create(:publication,
doi: pub_doi,
title: pub_title,
doi_verified: doi_verified)}
let(:pub_title) { 'Psychotherapy integration and the need for better theories of change: A rejoinder to Alford' }
let(:pub_doi) { 'https://doi.org/10.1016/S0962-1849(05)80014-9' }
let(:doi_verified) { nil }
let(:job) { described_class.new }
let(:response) { instance_double(UnpaywallResponse,
title: 'Psychotherapy integration and the need for better theories of change: A rejoinder to Alford',
matchable_title: 'psychotherapyintegrationandtheneedforbettertheoriesofchangearejoindertoalford',
Expand All @@ -28,7 +36,7 @@

it 'calls the DOI verification service and updates doi verified to false' do
expect(service).to receive(:verify)
job.perform(publication)
job.perform_now(publication.id)
end
end

Expand All @@ -39,44 +47,44 @@
before { allow(UnpaywallClient).to receive(:query_unpaywall).with(publication).and_return(response) }

context 'when the publication title and unpaywall title match' do
before { job.perform(publication) }
before { job.perform_now(publication.id) }

it 'updates the publication doi' do
expect(publication.doi).to eq 'https://doi.org/10.1016/S0962-1849(05)80014-9'
expect(publication.reload.doi).to eq 'https://doi.org/10.1016/S0962-1849(05)80014-9'
end

it 'updates the doi verification to true' do
expect(publication.doi_verified).to be true
expect(publication.reload.doi_verified).to be true
end
end

context 'when the publication title and unpaywall title do not match' do
let(:pub_title) { 'Psychotherapy integration' }

before { job.perform(publication) }
before { job.perform_now(publication.id) }

it 'does not update the publication doi' do
expect(publication.doi).to be_nil
expect(publication.reload.doi).to be_nil
end

it 'does not update the doi verification' do
expect(publication.doi_verified).to be_nil
expect(publication.reload.doi_verified).to be_nil
end
end
end

context "when the publication's doi is not found in Unpaywall" do
before do
allow(UnpaywallClient).to receive(:query_unpaywall).with(publication).and_return(empty_response)
job.perform(publication)
job.perform_now(publication.id)
end

it 'does not update the publication doi' do
expect(publication.doi).to be_nil
expect(publication.reload.doi).to be_nil
end

it 'does not update the doi verification' do
expect(publication.doi_verified).to be_nil
expect(publication.reload.doi_verified).to be_nil
end
end
end
Expand All @@ -85,10 +93,10 @@
let(:doi_verified) { true }
let(:pub_title) { 'Psychotherapy integration' }

before { job.perform(publication) }
before { job.perform_now(publication.id) }

it 'does not update the publication' do
expect(publication.doi_verified).to be true
expect(publication.reload.doi_verified).to be true
end
end
end
Expand Down
15 changes: 7 additions & 8 deletions spec/component/services/oa_workflow_service_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,23 +27,22 @@
let!(:activity_insight_oa_file2) { create(:activity_insight_oa_file, publication: pub3) }
let!(:activity_insight_oa_file3) { create(:activity_insight_oa_file, publication: pub4) }
let!(:activity_insight_oa_file4) { create(:activity_insight_oa_file, publication: pub5) }
let(:doi_job) { instance_spy DoiVerificationJob }

context 'when publications need doi verification' do
before { allow(DoiVerificationJob).to receive(:new).and_return(doi_job) }
before { allow(DoiVerificationJob).to receive(:perform_later) }

it 'calls the doi verification job with that publication' do
service.workflow
expect(doi_job).not_to have_received(:perform).with(pub1)
expect(doi_job).not_to have_received(:perform).with(pub2)
expect(doi_job).not_to have_received(:perform).with(pub3)
expect(doi_job).to have_received(:perform).with(pub4)
expect(doi_job).not_to have_received(:perform).with(pub5)
expect(DoiVerificationJob).not_to have_received(:perform_later).with(pub1.id)
expect(DoiVerificationJob).not_to have_received(:perform_later).with(pub2.id)
expect(DoiVerificationJob).not_to have_received(:perform_later).with(pub3.id)
expect(DoiVerificationJob).to have_received(:perform_later).with(pub4.id)
expect(DoiVerificationJob).not_to have_received(:perform_later).with(pub5.id)
end
end

context 'when there is an error' do
before { allow(DoiVerificationJob).to receive(:new).and_raise(RuntimeError) }
before { allow(DoiVerificationJob).to receive(:perform_later).and_raise(RuntimeError) }

it 'saves doi verifed as false' do
service.workflow
Expand Down

0 comments on commit fb09c0a

Please sign in to comment.