diff --git a/.dockerfiles/Dockerfile b/.dockerfiles/Dockerfile index 122d558c04..390b16cc39 100644 --- a/.dockerfiles/Dockerfile +++ b/.dockerfiles/Dockerfile @@ -1,7 +1,7 @@ -FROM ubuntu:focal AS base +FROM ubuntu:jammy AS base ARG NODE_MAJOR=18 -ARG BUNDLER_VERSION='2.3.17' +ARG BUNDLER_VERSION='2.4.13' ARG USER=markus # Required in order to ensure bind-mounts are owned by the correct user inside the container @@ -69,6 +69,7 @@ ENV PATH="$GEM_HOME/bin:$GEM_HOME/gems/bin:$PATH" RUN apt-get update -qq && \ DEBIAN_FRONTEND=noninteractive apt-get install -yq --no-install-recommends openssh-server \ python3 \ + python3-dev \ python3-venv \ equivs diff --git a/.dockerfiles/entrypoint-dev-rails.sh b/.dockerfiles/entrypoint-dev-rails.sh index 2b311aec08..130c59bfc0 100755 --- a/.dockerfiles/entrypoint-dev-rails.sh +++ b/.dockerfiles/entrypoint-dev-rails.sh @@ -11,6 +11,7 @@ npm list &> /dev/null || npm ci ./venv/bin/python3 -m pip install --upgrade pip > /dev/null ./venv/bin/python3 -m pip install -r requirements-jupyter.txt > /dev/null ./venv/bin/python3 -m pip install -r requirements-scanner.txt > /dev/null +./venv/bin/python3 -m pip install -r requirements-qr.txt > /dev/null # setup the database (checks for db existence first) until pg_isready -q; do diff --git a/.github/workflows/test_ci.yml b/.github/workflows/test_ci.yml index 663cfb563a..3557be50f0 100644 --- a/.github/workflows/test_ci.yml +++ b/.github/workflows/test_ci.yml @@ -9,10 +9,10 @@ on: jobs: test: if: github.event.pull_request.draft == false - runs-on: ubuntu-20.04 + runs-on: ubuntu-22.04 services: postgres: - image: postgres:12 + image: postgres:14 env: POSTGRES_USER: postgres POSTGRES_PASSWORD: postgres @@ -49,7 +49,7 @@ jobs: - name: Set up ruby and cache gems uses: ruby/setup-ruby@v1 with: - ruby-version: ruby-2.7 + ruby-version: ruby-3.0 bundler-cache: true - name: Set up node and cache packages uses: actions/setup-node@v3 @@ -66,13 +66,13 @@ jobs: uses: actions/cache@v3 with: path: ~/.cache/pip - key: ${{ runner.os }}-pip-${{ hashFiles('requirements-jupyter.txt') }}-${{ hashFiles('requirements-scanner.txt') }} + key: ${{ runner.os }}-pip-${{ hashFiles('requirements-jupyter.txt') }}-${{ hashFiles('requirements-scanner.txt') }}-${{ hashFiles('requirements-qr.txt') }} restore-keys: | ${{ runner.os }}-pip- - name: Install python packages run: | python3.9 -m venv venv - ./venv/bin/pip install -r requirements-jupyter.txt -r requirements-scanner.txt + ./venv/bin/pip install -r requirements-jupyter.txt -r requirements-scanner.txt -r requirements-qr.txt - name: Configure server run: | sudo rm -f /etc/localtime @@ -113,7 +113,7 @@ jobs: finish: needs: test if: github.event.pull_request.draft == false - runs-on: ubuntu-20.04 + runs-on: ubuntu-22.04 steps: - name: Coveralls Finished uses: coverallsapp/github-action@v2 diff --git a/Gemfile b/Gemfile index 37118719dd..b2cd6c12f6 100644 --- a/Gemfile +++ b/Gemfile @@ -47,9 +47,8 @@ gem 'redis', '~> 4.8.1' gem 'combine_pdf' gem 'prawn' gem 'prawn-qrcode' -gem 'rmagick' +gem 'rmagick', '~> 5.2.0' gem 'rtesseract' -gem 'zxing_cpp', require: 'zxing' # Ruby miscellany gem 'json' diff --git a/Gemfile.lock b/Gemfile.lock index 9d345d80ae..c3cfd42e6b 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -117,7 +117,7 @@ GEM rack-test (>= 0.6.3) regexp_parser (>= 1.5, < 3.0) xpath (~> 3.2) - chunky_png (1.3.12) + chunky_png (1.4.0) combine_pdf (1.0.23) matrix ruby-rc4 (>= 0.1.5) @@ -260,6 +260,7 @@ GEM ast (~> 2.4.1) pdf-core (0.9.0) pg (1.5.3) + pkg-config (1.5.2) pluck_to_hash (1.0.2) activerecord (>= 4.0.2) activesupport (>= 4.0.2) @@ -349,12 +350,13 @@ GEM resque (>= 1.27) rufus-scheduler (~> 3.2, != 3.3) rexml (3.2.5) - rmagick (2.16.0) + rmagick (5.2.0) + pkg-config (~> 1.4) rouge (4.1.1) - rqrcode (1.1.2) + rqrcode (2.2.0) chunky_png (~> 1.0) - rqrcode_core (~> 0.1) - rqrcode_core (0.1.2) + rqrcode_core (~> 1.0) + rqrcode_core (1.2.0) rspec-core (3.12.2) rspec-support (~> 3.12.0) rspec-expectations (3.12.3) @@ -451,9 +453,6 @@ GEM xpath (3.2.0) nokogiri (~> 1.8) zeitwerk (2.6.8) - zxing_cpp (0.1.1) - ffi (~> 1.1) - rmagick (~> 2.13) PLATFORMS ruby @@ -509,7 +508,7 @@ DEPENDENCIES responders resque resque-scheduler - rmagick + rmagick (~> 5.2.0) rspec-rails (~> 6.0.3) rtesseract rubyzip @@ -526,7 +525,6 @@ DEPENDENCIES time-warp unicorn webmock - zxing_cpp BUNDLED WITH - 2.3.17 + 2.4.13 diff --git a/app/jobs/split_pdf_job.rb b/app/jobs/split_pdf_job.rb index f8c68679d8..a28be8f1eb 100644 --- a/app/jobs/split_pdf_job.rb +++ b/app/jobs/split_pdf_job.rb @@ -42,16 +42,23 @@ def perform(exam_template, _path, split_pdf_log, _original_filename = nil, _curr original_pdf = File.binread(File.join(raw_dir, "#{split_page.id}.pdf")) # convert PDF to an image - img = Magick::Image.from_blob(original_pdf) do - self.quality = 100 - self.density = '200' + img = Magick::Image.from_blob(original_pdf) do |options| + options.quality = 100 + options.density = '200' end.first qr_file_location = File.join(raw_dir, "#{split_page.id}.jpg") img.crop(Magick::NorthWestGravity, img.columns, img.rows / 5.0).write(qr_file_location) img.destroy! code_regex = /(?[\w-]+)-(?\d+)-(?\d+)/ - m = code_regex.match(ZXing.decode(qr_file_location)) || code_regex.match(RTesseract.new(qr_file_location).to_s) + python_exe = Rails.application.config.python + read_qr_py_file = Rails.root.join('lib/scanner/read_qr_code.py').to_s + stdout, status = Open3.capture2(python_exe, read_qr_py_file, qr_file_location) + if status.success? + m = code_regex.match(stdout) + else + m = code_regex.match(RTesseract.new(qr_file_location).to_s) + end status = '' if m.nil? @@ -234,9 +241,9 @@ def save_pages(exam_template, partial_exams, filename = nil, split_pdf_log = nil next unless exam_template.automatic_parsing && Rails.application.config.scanner_enabled begin # convert PDF to an image - imglist = Magick::Image.from_blob(cover_pdf.to_pdf) do - self.quality = 100 - self.density = '300' + imglist = Magick::Image.from_blob(cover_pdf.to_pdf) do |options| + options.quality = 100 + options.density = '300' end rescue StandardError next diff --git a/app/lib/markus_csv.rb b/app/lib/markus_csv.rb index 94778261cd..0bb4e6d01d 100644 --- a/app/lib/markus_csv.rb +++ b/app/lib/markus_csv.rb @@ -31,7 +31,7 @@ def self.parse(input, **options) if options[:encoding] input = input.encode(Encoding::UTF_8, options[:encoding]) end - CSV.parse(input, options) do |row| + CSV.parse(input, **options) do |row| yield row valid_line_count += 1 rescue CsvInvalidLineError => e diff --git a/app/models/assignment.rb b/app/models/assignment.rb index 0c6e97a96d..9e9d8e0398 100644 --- a/app/models/assignment.rb +++ b/app/models/assignment.rb @@ -634,7 +634,7 @@ def summary_test_result_json # Generate a CSV summary of the most recent test results associated with an assignment. def summary_test_result_csv results = {} - headers = SortedSet.new + headers = Set.new summary_test_results = self.summary_test_results.as_json summary_test_results.each do |test_result| @@ -648,6 +648,7 @@ def summary_test_result_csv headers << header end + headers = headers.sort CSV.generate do |csv| csv << [nil, *headers] diff --git a/app/models/exam_template.rb b/app/models/exam_template.rb index 75efbd79d5..023b564907 100644 --- a/app/models/exam_template.rb +++ b/app/models/exam_template.rb @@ -242,9 +242,9 @@ def save_cover cover = pdf.pages[0] cover_page = CombinePDF.new cover_page << cover - imglist = Magick::Image.from_blob(cover_page.to_pdf) do - self.quality = 100 - self.density = '300' + imglist = Magick::Image.from_blob(cover_page.to_pdf) do |options| + options.quality = 100 + options.density = '300' end imglist.first.write(File.join(self.base_path, 'cover.jpg')) end diff --git a/app/views/annotation_categories/_annotation_text.html.erb b/app/views/annotation_categories/_annotation_text.html.erb index f57a70120b..7c06b256da 100644 --- a/app/views/annotation_categories/_annotation_text.html.erb +++ b/app/views/annotation_categories/_annotation_text.html.erb @@ -66,7 +66,7 @@ <% end %> <% if annotation_text[:annotation_category].nil? %>
- <%= t('activerecord.models.submission', {'count': 1}) + ': ' %> + <%= t('activerecord.models.submission', count: 1) + ': ' %> <%= link_to annotation_text[:group_name], edit_course_result_url(@current_course, annotation_text[:result_id]) %>
diff --git a/db/migrate/20090219150618_fix_annotation_label_null_category_ok.rb b/db/migrate/20090219150618_fix_annotation_label_null_category_ok.rb index 92794c0222..9fff8bc1c7 100644 --- a/db/migrate/20090219150618_fix_annotation_label_null_category_ok.rb +++ b/db/migrate/20090219150618_fix_annotation_label_null_category_ok.rb @@ -1,9 +1,9 @@ class FixAnnotationLabelNullCategoryOk < ActiveRecord::Migration[4.2] def self.up - change_column :annotation_labels, :annotation_category_id, :integer, {:null => true} + change_column :annotation_labels, :annotation_category_id, :integer, null: true end def self.down - change_column :annotation_labels, :annotation_category_id, :integer, {:null => false} + change_column :annotation_labels, :annotation_category_id, :integer, null: true end end diff --git a/db/migrate/20090219151533_annotation_categories_assigned_to_assignments.rb b/db/migrate/20090219151533_annotation_categories_assigned_to_assignments.rb index f17cd736d3..cb9871d55a 100644 --- a/db/migrate/20090219151533_annotation_categories_assigned_to_assignments.rb +++ b/db/migrate/20090219151533_annotation_categories_assigned_to_assignments.rb @@ -1,6 +1,6 @@ class AnnotationCategoriesAssignedToAssignments < ActiveRecord::Migration[4.2] def self.up - add_column :annotation_categories, :assignment_id, :integer, {:null => false} + add_column :annotation_categories, :assignment_id, :integer, null: false end def self.down diff --git a/db/migrate/20091105182703_improve_db_indexes.rb b/db/migrate/20091105182703_improve_db_indexes.rb index 8704f4e93b..540d9ba985 100644 --- a/db/migrate/20091105182703_improve_db_indexes.rb +++ b/db/migrate/20091105182703_improve_db_indexes.rb @@ -10,7 +10,7 @@ def self.up add_index :results, :submission_id, :unique => true, :name => "results_u1" - change_column :groups, :group_name, :string, {:limit => 30} + change_column :groups, :group_name, :string, limit: 30 add_index :groups, :group_name, :name => "groups_n1" end diff --git a/db/migrate/20091116141905_make_criterion_weight_not_null_again.rb b/db/migrate/20091116141905_make_criterion_weight_not_null_again.rb index b0312ae8a5..957f438dd3 100644 --- a/db/migrate/20091116141905_make_criterion_weight_not_null_again.rb +++ b/db/migrate/20091116141905_make_criterion_weight_not_null_again.rb @@ -1,6 +1,6 @@ class MakeCriterionWeightNotNullAgain < ActiveRecord::Migration[4.2] def self.up - change_column :rubric_criteria, :weight, :float, {:null => false} + change_column :rubric_criteria, :weight, :float, null: false end def self.down diff --git a/db/migrate/20091116195456_fix_precision_of_flexible_criterion_max.rb b/db/migrate/20091116195456_fix_precision_of_flexible_criterion_max.rb index 00932e0bb4..72e367ba6b 100644 --- a/db/migrate/20091116195456_fix_precision_of_flexible_criterion_max.rb +++ b/db/migrate/20091116195456_fix_precision_of_flexible_criterion_max.rb @@ -1,9 +1,9 @@ class FixPrecisionOfFlexibleCriterionMax < ActiveRecord::Migration[4.2] def self.up - change_column :flexible_criteria, :max, :decimal, {:precision => 10, :scale => 1, :null => false} + change_column :flexible_criteria, :max, :decimal, precision: 10, scale: 1, null: false end def self.down - change_column :flexible_criteria, :max, :decimal, {:precision => 10, :scale => 0, :null => false} + change_column :flexible_criteria, :max, :decimal, precision: 10, scale: 1, null: false end end diff --git a/db/migrate/20121028211448_testing.rb b/db/migrate/20121028211448_testing.rb index 0e93109443..8e456733a9 100644 --- a/db/migrate/20121028211448_testing.rb +++ b/db/migrate/20121028211448_testing.rb @@ -13,9 +13,9 @@ def self.up end create_table :test_support_files do |t| - t.string :file_name, { :null => false } - t.references :assignment, { :null => false } - t.text :description, { :null => false } + t.string :file_name, null: false + t.references :assignment, null: false + t.text :description, null: false end add_index :test_support_files, @@ -23,20 +23,20 @@ def self.up :name => "index_test_files_on_assignment_id" create_table :test_scripts do |t| - t.integer "assignment_id", { :null => false } - t.float "seq_num", { :null => false } - t.string "script_name", { :null => false } - t.text "description", { :null => false } - t.integer "max_marks", { :null => false } + t.integer "assignment_id", null: false + t.float "seq_num", null: false + t.string "script_name", null: false + t.text "description", null: false + t.integer "max_marks", null: false t.boolean "run_on_submission" t.boolean "run_on_request" t.boolean "halts_testing" - t.string "display_description", { :null => false } - t.string "display_run_status", { :null => false } - t.string "display_marks_earned", { :null => false } - t.string "display_input", { :null => false } - t.string "display_expected_output", { :null => false } - t.string "display_actual_output", { :null => false } + t.string "display_description", null: false + t.string "display_run_status", null: false + t.string "display_marks_earned", null: false + t.string "display_input", null: false + t.string "display_expected_output", null: false + t.string "display_actual_output", null: false end add_index :test_scripts, @@ -49,12 +49,12 @@ def self.up t.references :test_script t.references :test_script_result t.string "name" - t.string "completion_status", { :null => false } - t.integer "marks_earned", { :null => false } + t.string "completion_status", null: false + t.integer "marks_earned", null: false t.integer "repo_revision" - t.text "input_description", { :null => false } - t.text "actual_output", { :null => false } - t.text "expected_output", { :null => false } + t.text "input_description", null: false + t.text "actual_output", null: false + t.text "expected_output", null: false end add_index :test_results, diff --git a/db/migrate/20230713153536_increase_group_name_length.rb b/db/migrate/20230713153536_increase_group_name_length.rb index 90eba04f12..b071f0b966 100644 --- a/db/migrate/20230713153536_increase_group_name_length.rb +++ b/db/migrate/20230713153536_increase_group_name_length.rb @@ -1,5 +1,5 @@ class IncreaseGroupNameLength < ActiveRecord::Migration[7.0] def change - change_column :groups, :group_name, :string, {:limit => nil} + change_column :groups, :group_name, :string, limit: nil end end diff --git a/lib/scanner/read_qr_code.py b/lib/scanner/read_qr_code.py new file mode 100644 index 0000000000..00590b1862 --- /dev/null +++ b/lib/scanner/read_qr_code.py @@ -0,0 +1,15 @@ +import sys +import cv2 +import zxingcpp + + +if __name__ == '__main__': + input_filename = sys.argv[1] + img = cv2.imread(input_filename) + results = zxingcpp.read_barcodes(img) + if len(results) == 0: + print("Could not find any barcode.") + sys.exit(1) + else: + result = results[0] + print(result.text) diff --git a/markus.control b/markus.control index 7091eaf4c2..701b486a1e 100644 --- a/markus.control +++ b/markus.control @@ -29,9 +29,9 @@ Depends: # nodejs: for serving/managing javascript nodejs (>= 18), nodejs (<< 19), # ruby: required to run Rails - ruby (>= 1:2.7) | ruby2.7, ruby (<< 1:3) | ruby2.7, + ruby (>= 1:2.7) | ruby3.0, ruby (<< 1:3) | ruby3.0, # ruby-dev: required to run rails - ruby-dev (>= 1:2.7) | ruby2.7-dev, ruby-dev (<< 1:3) | ruby2.7-dev, + ruby-dev (>= 1:2.7) | ruby3.0-dev, ruby-dev (<< 1:3) | ruby3.0-dev, # rubygems-integration: required to install ruby gems rubygems-integration, # tesseract-ocr: required for scanned exams diff --git a/markus_1.0_all.deb b/markus_1.0_all.deb index 9777a53dfc..bc0f1e72a4 100644 Binary files a/markus_1.0_all.deb and b/markus_1.0_all.deb differ diff --git a/requirements-qr.txt b/requirements-qr.txt new file mode 100644 index 0000000000..0cf57c7ace --- /dev/null +++ b/requirements-qr.txt @@ -0,0 +1 @@ +zxing-cpp==2.0.0 diff --git a/spec/controllers/assignments_controller_spec.rb b/spec/controllers/assignments_controller_spec.rb index 85046b9ee2..744a28ba75 100644 --- a/spec/controllers/assignments_controller_spec.rb +++ b/spec/controllers/assignments_controller_spec.rb @@ -177,7 +177,7 @@ assignment_results = assignment.summary_test_results - headers = SortedSet.new(test_results.headers) + headers = Set.new(test_results.headers.drop(1)).sort assignment_results.each do |result| expect(headers.include?("#{result['name']}:#{result['test_result_name']}")).to eq true end @@ -188,7 +188,7 @@ test_results = CSV.parse(response.body, headers: true) headers = test_results.headers.drop(1) - sorted_headers = SortedSet.new(headers) + sorted_headers = Set.new(headers).sort sorted_headers.each_with_index do |header, i| expect(header).to eq headers[i] end diff --git a/spec/fixtures/files/dummy_invalidate.sh b/spec/fixtures/files/dummy_invalidate.sh new file mode 100755 index 0000000000..425142a8a0 --- /dev/null +++ b/spec/fixtures/files/dummy_invalidate.sh @@ -0,0 +1,53 @@ +#!/bin/bash + +######################################################################## +# Basic structure as to how a password validation script might look like +# The preferred way is to write a small C program for this though. +######################################################################## + +# Check that no username/passwords are passed on the command line +if [ "$#" -ne 0 ]; then + # HACK-ALARM?! + echo "usage: $0" 1>&2 + exit 1 +fi + +# Username is passed on the first line from stdin, the users +# password as the second line. ip address might optionally be passed +# as a third line +# WARNING: username and/or password may contain any characters except \n +# and \0. I.e. don't trust that data in any case. Thus, make +# sure your script/program accounts for that! +read user +read password +read ip + +######################################################################## +# Do your password validation here +######################################################################## + +if [ $user == 'exit1' ]; then + exit 1 +fi + +if [ $user == 'exit2' ]; then + exit 2 +fi + +if [ $user == 'exit3' ]; then + echo "custom message with error 3" + exit 3 +fi + +if [ $user == 'exit3nomsg' ]; then + exit 3 +fi + +if [ $user == 'exit4' ]; then + echo "custom message with error 4" + exit 4 +fi + +# Exit with 0 return code, if and only if user/password combination +# is valid +exit 0 diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index e5ee4da1cd..45952395ae 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -76,45 +76,44 @@ end end context 'without a custom exit status messages' do + before do + allow(Settings).to receive(:validate_file).and_return(Rails.root + .join('spec/fixtures/files/dummy_invalidate.sh')) + end context 'a successful login' do it 'should return a success message' do - allow_any_instance_of(Process::Status).to receive(:exitstatus).and_return(0) expect(User.authenticate('ab', '123')).to eq User::AUTHENTICATE_SUCCESS end end context 'an unsuccessful login' do it 'should return a failure message' do - allow_any_instance_of(Process::Status).to receive(:exitstatus).and_return(1) - expect(User.authenticate('ab', '123')).to eq User::AUTHENTICATE_ERROR + expect(User.authenticate('exit3', '123')).to eq User::AUTHENTICATE_ERROR end end end context 'with a custom exit status message' do before do allow(Settings).to receive(:validate_custom_status_message).and_return('2' => 'a two!', '3' => 'a three!') + allow(Settings).to receive(:validate_file).and_return(Rails.root + .join('spec/fixtures/files/dummy_invalidate.sh')) end context 'a successful login' do it 'should return a success message' do - allow_any_instance_of(Process::Status).to receive(:exitstatus).and_return(0) expect(User.authenticate('ab', '123')).to eq User::AUTHENTICATE_SUCCESS end end context 'an unsuccessful login' do it 'should return a failure message with a 1' do - allow_any_instance_of(Process::Status).to receive(:exitstatus).and_return(1) - expect(User.authenticate('ab', '123')).to eq User::AUTHENTICATE_ERROR + expect(User.authenticate('exit1', '123')).to eq User::AUTHENTICATE_ERROR end it 'should return a failure message with a 4' do - allow_any_instance_of(Process::Status).to receive(:exitstatus).and_return(4) - expect(User.authenticate('ab', '123')).to eq User::AUTHENTICATE_ERROR + expect(User.authenticate('exit4', '123')).to eq User::AUTHENTICATE_ERROR end it 'should return a custom message with a 2' do - allow_any_instance_of(Process::Status).to receive(:exitstatus).and_return(2) - expect(User.authenticate('ab', '123')).to eq '2' + expect(User.authenticate('exit2', '123')).to eq '2' end it 'should return a custom message with a 3' do - allow_any_instance_of(Process::Status).to receive(:exitstatus).and_return(3) - expect(User.authenticate('ab', '123')).to eq '3' + expect(User.authenticate('exit3nomsg', '123')).to eq '3' end end end