ylqjgm
/
canvas-lms
mirror of https://github.com/instructure/canvas-lms.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

remove a potential race in turnitin processing; fixes #8704 

Previously we spawned a new job to check the status of each asset sent to
turnitin.  If there were multiple files it was possible that two of the jobs
could run at the same time, with one overwriting the score computed by the
other.  This commit changes the logic so that only one job is spawned, which
checks the status on all outstanding assets.

test plan:
- turn in multiple files to a turnitin assignment
- ensure you get scores for both assignments

Change-Id: I78edb33584b8cf99d490681154b32559c0a7db25
Reviewed-on: https://gerrit.instructure.com/11722
Tested-by: Jenkins <jenkins@instructure.com>
Reviewed-by: Brian Palmer <brianp@instructure.com>
This commit is contained in:
Simon Williams 2012-06-20 15:23:07 -06:00
parent 4fb6d42ccb
commit 0108709c12
2 changed files with 60 additions and 33 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

71
app/models/submission.rb
View File

@ -205,44 +205,53 @@ class Submission < ActiveRecord::Base
strip_tags((self.body || "").gsub(/\<\s*br\s*\/\>/, "\n<br/>").gsub(/\<\/p\>/, "</p>\n"))
end
def check_turnitin_status(asset_string, attempt=1)
def check_turnitin_status(attempt=1)
self.turnitin_data ||= {}
data = self.turnitin_data[asset_string]
return unless data && data[:object_id]
if data[:similarity_score].blank?
if attempt < TURNITIN_RETRY
turnitin = Turnitin::Client.new(*self.context.turnitin_settings)
res = turnitin.generateReport(self, asset_string)
if res[:similarity_score]
data[:similarity_score] = res[:similarity_score].to_f
data[:web_overlap] = res[:web_overlap].to_f
data[:publication_overlap] = res[:publication_overlap].to_f
data[:student_overlap] = res[:student_overlap].to_f
data[:state] = 'failure'
data[:state] = 'problem' if data[:similarity_score] < 75
data[:state] = 'warning' if data[:similarity_score] < 50
data[:state] = 'acceptable' if data[:similarity_score] < 25
data[:state] = 'none' if data[:similarity_score] == 0
data[:status] = 'scored'
turnitin = nil
needs_retry = false
# check all assets in the turnitin_data (self.turnitin_assets is only the
# current assets) so that we get the status for assets of previous versions
# of the submission as well
self.turnitin_data.keys.each do |asset_string|
data = self.turnitin_data[asset_string]
next unless data && data.is_a?(Hash) && data[:object_id]
if data[:similarity_score].blank?
if attempt < TURNITIN_RETRY
turnitin ||= Turnitin::Client.new(*self.context.turnitin_settings)
res = turnitin.generateReport(self, asset_string)
if res[:similarity_score]
data[:similarity_score] = res[:similarity_score].to_f
data[:web_overlap] = res[:web_overlap].to_f
data[:publication_overlap] = res[:publication_overlap].to_f
data[:student_overlap] = res[:student_overlap].to_f
data[:state] = 'failure'
data[:state] = 'problem' if data[:similarity_score] < 75
data[:state] = 'warning' if data[:similarity_score] < 50
data[:state] = 'acceptable' if data[:similarity_score] < 25
data[:state] = 'none' if data[:similarity_score] == 0
data[:status] = 'scored'
else
needs_retry ||= true
end
else
send_at((5 * attempt).minutes.from_now, :check_turnitin_status, asset_string, attempt + 1)
data[:status] = 'error'
end
else
data[:status] = 'error'
data[:status] = 'scored'
end
else
data[:status] = 'scored'
self.turnitin_data[asset_string] = data
end
self.turnitin_data[asset_string] = data
send_at((5 * attempt).minutes.from_now, :check_turnitin_status, attempt + 1) if needs_retry
self.turnitin_data_changed!
self.save
data
end
def turnitin_report_url(asset_string, user)
if self.turnitin_data && self.turnitin_data[asset_string] && self.turnitin_data[asset_string][:similarity_score]
turnitin = Turnitin::Client.new(*self.context.turnitin_settings)
self.send_later(:check_turnitin_status, asset_string)
self.send_later(:check_turnitin_status)
if self.grants_right?(user, nil, :grade)
turnitin.submissionReportUrl(self, asset_string)
elsif self.grants_right?(user, nil, :view_turnitin_report)
@ -278,7 +287,7 @@ class Submission < ActiveRecord::Base
turnitin = Turnitin::Client.new(*self.context.turnitin_settings)
reset_turnitin_assets
# 1. Make sure the assignment exists and user is enrolled
# Make sure the assignment exists and user is enrolled
assign_status = self.assignment.create_in_turnitin
enroll_status = turnitin.enrollStudent(self.context, self.user)
unless assign_status && enroll_status
@ -296,18 +305,20 @@ class Submission < ActiveRecord::Base
return false
end
# 2. Submit the file(s)
# Submit the file(s)
submission_response = turnitin.submitPaper(self)
submission_response.each do |res_asset_string, response|
self.turnitin_data[res_asset_string].merge!(response)
self.turnitin_data_changed!
if response[:object_id]
self.send_at(5.minutes.from_now, :check_turnitin_status, res_asset_string)
elsif !(attempt < TURNITIN_RETRY)
if !response[:object_id] && !(attempt < TURNITIN_RETRY)
self.turnitin_data[res_asset_string][:status] = 'error'
end
end
self.send_at(5.minutes.from_now, :check_turnitin_status)
self.save
# Schedule retry if there were failures
submit_status = submission_response.present? && submission_response.values.all?{ |v| v[:object_id] }
unless submit_status
send_at(5.minutes.from_now, :submit_to_turnitin, attempt + 1) if attempt < TURNITIN_RETRY

22
spec/models/submission_spec.rb
View File

@ -355,7 +355,7 @@ describe Submission do
:student_overlap => 33
})
@submission.check_turnitin_status(@submission.asset_string)
@submission.check_turnitin_status
@submission.reload.turnitin_data[@submission.asset_string][:status].should == 'scored'
end
@ -365,13 +365,29 @@ describe Submission do
@submission.turnitin_data[@submission.asset_string] = { :object_id => '1234', :status => 'pending' }
@turnitin_api.expects(:generateReport).with(@submission, @submission.asset_string).returns({})
@submission.check_turnitin_status(@submission.asset_string, Submission::TURNITIN_RETRY-1)
@submission.check_turnitin_status(Submission::TURNITIN_RETRY-1)
@submission.reload.turnitin_data[@submission.asset_string][:status].should == 'pending'
Delayed::Job.find_by_tag('Submission#check_turnitin_status').should_not be_nil
@submission.check_turnitin_status(@submission.asset_string, Submission::TURNITIN_RETRY)
@submission.check_turnitin_status(Submission::TURNITIN_RETRY)
@submission.reload.turnitin_data[@submission.asset_string][:status].should == 'error'
end
it "should check status for all assets" do
init_turnitin_api
@submission.turnitin_data ||= {}
@submission.turnitin_data[@submission.asset_string] = { :object_id => '1234', :status => 'pending' }
@submission.turnitin_data["other_asset"] = { :object_id => 'xxyy', :status => 'pending' }
@turnitin_api.expects(:generateReport).with(@submission, @submission.asset_string).returns({
:similarity_score => 56, :web_overlap => 22, :publication_overlap => 0, :student_overlap => 33
})
@turnitin_api.expects(:generateReport).with(@submission, "other_asset").returns({ :similarity_score => 20 })
@submission.check_turnitin_status
@submission.reload
@submission.turnitin_data[@submission.asset_string][:status].should == 'scored'
@submission.turnitin_data["other_asset"][:status].should == 'scored'
end
end
context "report" do