diff --git a/app/models/submission.rb b/app/models/submission.rb index e4f344ee25a..6da1b4d034f 100644 --- a/app/models/submission.rb +++ b/app/models/submission.rb @@ -17,6 +17,7 @@ # require 'atom' +require 'anonymity' class Submission < ActiveRecord::Base include Canvas::GradeValidations @@ -227,29 +228,6 @@ class Submission < ActiveRecord::Base anonymized.for_assignment(assignment).pluck(:anonymous_id) end - # Returns a unique short id to be used for anonymous_id. If the - # generated short id is already in use, loop until an available - # one is generated. `anonymous_ids` are unique per assignment. - # This method will throw a unique constraint error from the - # database if it has used all unique ids. - # An optional argument of existing_anonymous_ids can be supplied - # to customize the handling of existing anonymous_ids. E.g. bulk - # generation of anonymous ids where you wouldn't want to - # continuously query the database - def self.generate_unique_anonymous_id(assignment:, existing_anonymous_ids: anonymous_ids_for(assignment)) - loop do - short_id = Submission.generate_short_id - break short_id unless existing_anonymous_ids.include?(short_id) - end - end - - # base58 to avoid literal problems with prefixed 0 (i.e. when 0x123 - # is interpreted as a hex value `0x123 == 291`), and similar looking - # characters: 0/O, I/l - def self.generate_short_id - SecureRandom.base58(5) - end - # see #needs_grading? # When changing these conditions, update index_submissions_needs_grading to # maintain performance. @@ -2459,6 +2437,6 @@ class Submission < ActiveRecord::Base private def set_anonymous_id - self.anonymous_id = Submission.generate_unique_anonymous_id(assignment: anonymous_id) + self.anonymous_id = Anonymity.generate_id(existing_ids: Submission.anonymous_ids_for(assignment)) end end diff --git a/lib/anonymity.rb b/lib/anonymity.rb new file mode 100644 index 00000000000..de2ddb4a83b --- /dev/null +++ b/lib/anonymity.rb @@ -0,0 +1,50 @@ +# +# Copyright (C) 2018 - present Instructure, Inc. +# +# This file is part of Canvas. +# +# Canvas is free software: you can redistribute it and/or modify it under +# the terms of the GNU Affero General Public License as published by the Free +# Software Foundation, version 3 of the License. +# +# Canvas is distributed in the hope that it will be useful, but WITHOUT ANY +# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR +# A PARTICULAR PURPOSE. See the GNU Affero General Public License for more +# details. +# +# You should have received a copy of the GNU Affero General Public License along +# with this program. If not, see . + +# This file contains routines for creating IDs used for graders and submission +# in place of the objects' actual IDs for Anonymous Moderated Marking. +# The IDs are 5 characters long and in base58 format. These IDs are meant to be +# unique across the graders or submissions for a given assignment: a grader +# for an assignment, for example, can be referenced by their "anonymous" ID +# for that specific assignment so that their default ID is concealed. +module Anonymity + + # Returns a unique short id to be used as an anonymous ID. If the + # generated short id is already in use, loop until an available + # one is generated. + # This method will throw a unique constraint error from the + # database if it has used all unique ids. + # An optional argument of existing_ids can be supplied + # to customize the handling of existing ids. E.g. bulk + # generation of anonymous ids where you wouldn't want to + # continuously query the database + def self.generate_id(existing_ids: []) + loop do + short_id = self.generate_short_id + break short_id unless existing_ids.include?(short_id) + end + end + + private_class_method + + # base58 to avoid literal problems with prefixed 0 (i.e. when 0x123 + # is interpreted as a hex value `0x123 == 291`), and similar looking + # characters: 0/O, I/l + def self.generate_short_id + SecureRandom.base58(5) + end +end diff --git a/lib/due_date_cacher.rb b/lib/due_date_cacher.rb index 286743eed58..a569773066d 100644 --- a/lib/due_date_cacher.rb +++ b/lib/due_date_cacher.rb @@ -15,6 +15,8 @@ # You should have received a copy of the GNU Affero General Public License along # with this program. If not, see . +require 'anonymity' + class DueDateCacher INFER_SUBMISSION_WORKFLOW_STATE_SQL = <<~SQL_FRAGMENT CASE @@ -109,10 +111,7 @@ class DueDateCacher due_date = submission_info[:due_at] ? "'#{submission_info[:due_at].iso8601}'::timestamptz" : 'NULL' grading_period_id = submission_info[:grading_period_id] || 'NULL' - anonymous_id = Submission.generate_unique_anonymous_id( - assignment: assignment_id, - existing_anonymous_ids: existing_anonymous_ids - ) + anonymous_id = Anonymity.generate_id(existing_ids: existing_anonymous_ids) existing_anonymous_ids << anonymous_id sql_ready_anonymous_id = Submission.connection.quote(anonymous_id) values << [assignment_id, student_id, due_date, grading_period_id, sql_ready_anonymous_id] diff --git a/spec/lib/anonymity_spec.rb b/spec/lib/anonymity_spec.rb new file mode 100644 index 00000000000..5bfdf4c9d20 --- /dev/null +++ b/spec/lib/anonymity_spec.rb @@ -0,0 +1,70 @@ +# +# Copyright (C) 2018 - present Instructure, Inc. +# +# This file is part of Canvas. +# +# Canvas is free software: you can redistribute it and/or modify it under +# the terms of the GNU Affero General Public License as published by the Free +# Software Foundation, version 3 of the License. +# +# Canvas is distributed in the hope that it will be useful, but WITHOUT ANY +# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR +# A PARTICULAR PURPOSE. See the GNU Affero General Public License for more +# details. +# +# You should have received a copy of the GNU Affero General Public License along +# with this program. If not, see . + +require_relative '../spec_helper' + +describe Anonymity do + describe '.generate_id' do + let(:short_id) { 'aB123' } + + context 'given no existing_anonymous_ids' do + subject(:generate_id) do + -> { Anonymity.generate_id } + end + + it 'creates an anonymous_id' do + allow(Anonymity).to receive(:generate_short_id).and_return(short_id) + expect(generate_id.call).to eql short_id + end + + it 'creates a unique anonymous_id when collisions happen' do + first_anonymous_id = short_id + colliding_anonymous_id = first_anonymous_id + unused_anonymous_id = 'eeeee' + + allow(Anonymity).to receive(:generate_short_id).exactly(3).times.and_return( + first_anonymous_id, colliding_anonymous_id, unused_anonymous_id + ) + + first_returned_id = Anonymity.generate_id + second_returned_id = Anonymity.generate_id(existing_ids: [first_returned_id]) + expect(second_returned_id).to eql(unused_anonymous_id) + end + end + + context 'given a list of existing_anonymous_ids' do + subject do + Anonymity.generate_id(existing_ids: existing_anonymous_ids_fake) + end + + let(:existing_anonymous_ids_fake) { double('Array') } + + it 'queries the passed in existing_anonymous_ids' do + allow(Anonymity).to receive(:generate_short_id).and_return(short_id) + expect(existing_anonymous_ids_fake).to receive(:include?).with(short_id).and_return(false) + is_expected.to eql short_id + end + end + end + + describe '.generate_short_id' do + it 'generates a short id' do + expect(SecureRandom).to receive(:base58).with(5) + Anonymity.generate_short_id + end + end +end diff --git a/spec/models/submission_spec.rb b/spec/models/submission_spec.rb index 4d56391261b..bde794dc48a 100644 --- a/spec/models/submission_spec.rb +++ b/spec/models/submission_spec.rb @@ -63,78 +63,6 @@ describe Submission do end end - describe '.generate_unique_anonymous_id' do - let(:assignment) { @assignment } - let(:short_id) { 'aB123' } - - context 'given no existing_anonymous_ids' do - subject(:generate_unique_anonymous_id) do - -> { Submission.generate_unique_anonymous_id(assignment: assignment) } - end - - it 'creates an anonymous_id' do - allow(Submission).to receive(:generate_short_id).and_return(short_id) - expect(generate_unique_anonymous_id.call).to eql short_id - end - - it 'fetches existing_anonymous_ids' do - expect(Submission).to receive(:anonymous_ids_for).with(assignment).and_call_original - generate_unique_anonymous_id.call - end - - it 'creates a unique anonymous_id when collisions happen' do - first_student = @student - second_student = student_in_course(course: @course, active_all: true).user - first_submission = submission_model(assignment: assignment, user: first_student) - second_submission = submission_model(assignment: assignment, user: second_student) - Submission.update_all(anonymous_id: nil) - - first_anonymous_id = short_id - colliding_anonymous_id = first_anonymous_id - unused_anonymous_id = 'eeeee' - - allow(Submission).to receive(:generate_short_id).exactly(3).times.and_return( - first_anonymous_id, colliding_anonymous_id, unused_anonymous_id - ) - - anonymous_ids = [first_submission, second_submission].map do |submission| - submission.update!(anonymous_id: generate_unique_anonymous_id.call) - submission.anonymous_id - end - - expect(anonymous_ids).to contain_exactly(first_anonymous_id, unused_anonymous_id) - end - end - - context 'given a list of existing_anonymous_ids' do - subject do - Submission.generate_unique_anonymous_id( - assignment: assignment, - existing_anonymous_ids: existing_anonymous_ids_fake - ) - end - - let(:existing_anonymous_ids_fake) { double('Array') } - - before do - student_in_course(course: @course, active_all: true) - end - - it 'queries the passed in existing_anonymous_ids' do - allow(Submission).to receive(:generate_short_id).and_return(short_id) - expect(existing_anonymous_ids_fake).to receive(:include?).with(short_id).and_return(false) - is_expected.to eql short_id - end - end - end - - describe '.generate_short_id' do - it 'generates a short id' do - expect(SecureRandom).to receive(:base58).with(5) - Submission.generate_short_id - end - end - describe '.anonymous_ids_for' do subject { Submission.anonymous_ids_for(@first_assignment) }