diff --git a/.github/workflows/push.yml b/.github/workflows/push.yml index d84c61d6d..d4f897af6 100644 --- a/.github/workflows/push.yml +++ b/.github/workflows/push.yml @@ -27,8 +27,11 @@ env: DF_ENCRYPTION_DETERMINISTIC_KEY: "anlmuJ6cB3bN3biXRbYvmPsC5ALPFqGG" DF_ENCRYPTION_KEY_DERIVATION_SALT: "hzPR8D4qpOnAg7VeAhkhWw6JmmzKJB10" DF_REDIS_SIDEKIQ_URL: "redis://redis:6379/0" + DF_API_CONTAINER_NAME: doubtfire-api-development LATEX_CONTAINER_NAME: doubtfire-texlive + DF_LATEX_IMAGE_NAME: doubtfire-texlive LATEX_BUILD_PATH: /texlive/shell/latex_build.sh + DF_JPLAG_IMAGE_NAME: doubtfire-jplag DF_JPLAG_REPORT_DIR: /jplag/results LTI_SHARED_API_SECRET: "abc123" LTI_ENABLED: true @@ -61,7 +64,9 @@ jobs: file: texlive.Dockerfile push: false load: true - tags: doubtfire-texlive-development:local + tags: | + ${{ env.DF_LATEX_IMAGE_NAME }}:local + ${{ env.DF_LATEX_IMAGE_NAME }}:latest cache-from: type=gha,scope=texlive cache-to: type=gha,mode=max,scope=texlive - name: Build JPlag image @@ -71,7 +76,9 @@ jobs: file: jplag.Dockerfile push: false load: true - tags: doubtfire-jplag-development:local + tags: | + ${{ env.DF_JPLAG_IMAGE_NAME }}:local + ${{ env.DF_JPLAG_IMAGE_NAME }}:latest cache-from: type=gha,scope=jplag cache-to: type=gha,mode=max,scope=jplag - name: Build base doubtfire-api development image @@ -80,13 +87,13 @@ jobs: context: . push: false load: true - tags: doubtfire-api-development:local + tags: ${{ env.DF_API_CONTAINER_NAME }}:local cache-from: type=gha,scope=doubtfire-api cache-to: type=gha,mode=max,scope=doubtfire-api - name: Start TexLive service uses: addnab/docker-run-action@v3 with: - image: doubtfire-texlive-development:local + image: ${{ env.DF_LATEX_IMAGE_NAME }}:local options: > --name ${{ env.LATEX_CONTAINER_NAME }} -v ${{ github.workspace }}/student-work:/student-work @@ -98,7 +105,7 @@ jobs: - name: Test TexLive container uses: addnab/docker-run-action@v3 with: - image: doubtfire-api-development:local + image: ${{ env.DF_API_CONTAINER_NAME }}:local options: > -t -v ${{ github.workspace }}:/doubtfire @@ -107,9 +114,10 @@ jobs: - name: Start JPlag service uses: addnab/docker-run-action@v3 with: - image: doubtfire-jplag-development:local + image: ${{ env.DF_JPLAG_IMAGE_NAME }}:local options: > --name jplag + -v ${{ github.workspace }}:/doubtfire -v ${{ github.workspace }}/student-work:/student-work -v ${{ github.workspace }}/jplag/results:${{ env.DF_JPLAG_REPORT_DIR }} -v ${{ github.workspace }}/tmp/jplag:/tmp/jplag @@ -119,7 +127,7 @@ jobs: - name: Test JPlag service uses: addnab/docker-run-action@v3 with: - image: doubtfire-api-development:local + image: ${{ env.DF_API_CONTAINER_NAME }}:local options: > -t -v ${{ github.workspace }}:/doubtfire @@ -128,7 +136,7 @@ jobs: - name: Populate database uses: addnab/docker-run-action@v3 with: - image: doubtfire-api-development:local + image: ${{ env.DF_API_CONTAINER_NAME }}:local options: > -v ${{ github.workspace }}:/doubtfire -v ${{ github.workspace }}/student-work:/student-work @@ -153,14 +161,19 @@ jobs: -e LATEX_CONTAINER_NAME -e LATEX_BUILD_PATH -e DF_JPLAG_REPORT_DIR + -e DF_API_CONTAINER_NAME + -e DF_LATEX_IMAGE_NAME + -e DF_JPLAG_IMAGE_NAME -e LTI_SHARED_API_SECRET -e LTI_ENABLED + -e DF_LATEX_PATH_TO_WORKDIRS=/doubtfire/tmp/rails-latex run: bundle exec rake db:populate - name: Run unit tests uses: addnab/docker-run-action@v3 with: - image: doubtfire-api-development:local + image: ${{ env.DF_API_CONTAINER_NAME }}:local options: > + --name ${{ env.DF_API_CONTAINER_NAME }} -v ${{ github.workspace }}:/doubtfire -v ${{ github.workspace }}/student-work:/student-work -v /var/run/docker.sock:/var/run/docker.sock @@ -186,8 +199,13 @@ jobs: -e LATEX_CONTAINER_NAME -e LATEX_BUILD_PATH -e DF_JPLAG_REPORT_DIR + -e DF_API_CONTAINER_NAME + -e DF_LATEX_IMAGE_NAME + -e DF_JPLAG_IMAGE_NAME -e LTI_SHARED_API_SECRET -e LTI_ENABLED + -e DF_LATEX_PATH_TO_WORKDIRS=/doubtfire/tmp/rails-latex + -e DF_JPLAG_PATH_TO_WORKDIRS=/doubtfire/tmp/jplag run: TERM=xterm bundle exec rails test - name: Stop TexLive service run: docker rm -f ${{ env.LATEX_CONTAINER_NAME }} diff --git a/app/helpers/file_helper.rb b/app/helpers/file_helper.rb index 9557ff887..089acbb9f 100644 --- a/app/helpers/file_helper.rb +++ b/app/helpers/file_helper.rb @@ -409,35 +409,11 @@ def qpdf(path) # - retain_from = true if you want to keep from, otherwise it is deleted # - only_before = date for files to move (only if retain from is true) def move_files(from_path, to_path, retain_from = false, only_before = nil) - # move into the new dir - and mv files to the in_process_dir - begin - pwd = FileUtils.pwd - rescue - # if no pwd, reset to the root - pwd = Rails.root - end - - begin - FileUtils.mkdir_p(to_path) - Dir.chdir(from_path) - FileUtils.mv Dir.glob('*').filter{|fn| !retain_from || only_before.nil? || File.ctime(fn) < only_before}, to_path, force: true - Dir.chdir(to_path) - begin - # remove from_path as files are now "in process" - # these can be retained when the old folder wants to be kept - FileUtils.rm_rf(from_path) unless retain_from - rescue - logger.warn "failed to rm #{from_path}" - end - ensure - if FileUtils.pwd != pwd - if Dir.exist? pwd - FileUtils.chdir(pwd) - else - FileUtils.chdir(student_work_dir) - end - end - end + FileUtils.mkdir_p(to_path) + files = Dir.glob(File.join(from_path, '*')) + .filter { |fn| !retain_from || only_before.nil? || File.ctime(fn) < only_before } + FileUtils.mv(files, to_path, force: true) + FileUtils.rm_rf(from_path) unless retain_from end # diff --git a/app/models/pdf_generation/project_compile_portfolio_module.rb b/app/models/pdf_generation/project_compile_portfolio_module.rb index aed4c9cd0..fc1becae5 100644 --- a/app/models/pdf_generation/project_compile_portfolio_module.rb +++ b/app/models/pdf_generation/project_compile_portfolio_module.rb @@ -65,6 +65,7 @@ def init(project, is_retry) @institution_name = Doubtfire::Application.config.institution[:name] @doubtfire_product_name = Doubtfire::Application.config.institution[:product_name] @is_retry = is_retry + @work_id = "portfolio-#{project.id}#{'-retry' if is_retry}" end def make_pdf @@ -214,9 +215,8 @@ def move_to_portfolio(file, name, kind) self.uses_draft_learning_summary = false save else - Dir.chdir(portfolio_tmp_dir) - files = Dir.glob('*') - idx = files.map { |a_file| a_file.split('-').first.to_i }.max + files = Dir.glob(File.join(portfolio_tmp_dir, '*')) + idx = files.map { |a_file| File.basename(a_file).split('-').first.to_i }.max if idx.nil? || idx < 1 idx = 1 else @@ -237,10 +237,11 @@ def portfolio_files(ensure_valid: false, force_ascii: false) result = [] - Dir.chdir(portfolio_tmp_dir) - files = Dir.glob('*').select { |f| (f =~ /^\d{3}-(cover|document|code|image)/) == 0 } + files = Dir.glob(File.join(portfolio_tmp_dir, '*')) + .select { |f| File.basename(f) =~ /^\d{3}-(cover|document|code|image)/ } + files.each do |file| - parts = file.split('-') + parts = File.basename(file).split('-') idx = parts[0].to_i kind = parts[1] name = parts.drop(2).join('-') diff --git a/app/models/similarity/unit_similarity_module.rb b/app/models/similarity/unit_similarity_module.rb index 375950176..a0717681e 100644 --- a/app/models/similarity/unit_similarity_module.rb +++ b/app/models/similarity/unit_similarity_module.rb @@ -14,147 +14,157 @@ def last_plagarism_scan end # Pass tasks on to plagarism detection software and setup links between students - def check_moss_similarity(force: false) - # Get each task... - return unless active + # def check_moss_similarity(force: false) + # # Get each task... + # return unless active - # need pwd to restore after cding into submission folder (so the files do not have full path) - pwd = FileUtils.pwd + # # need pwd to restore after cding into submission folder (so the files do not have full path) + # pwd = FileUtils.pwd - begin - logger.info "Checking plagiarsm for unit #{code} - #{name} (id=#{id})" + # begin + # logger.info "Checking plagiarsm for unit #{code} - #{name} (id=#{id})" - task_definitions.each do |td| - next if td.similarity_language.nil? || td.upload_requirements.nil? || td.upload_requirements.select { |upreq| upreq['type'] == 'code' && upreq['tii_check'] }.empty? + # task_definitions.each do |td| + # next if td.similarity_language.nil? || td.upload_requirements.nil? || td.upload_requirements.select { |upreq| upreq['type'] == 'code' && upreq['tii_check'] }.empty? - type_data = td.similarity_language.split - next if type_data.nil? || (type_data.length != 2) || (type_data[0] != 'moss') + # type_data = td.similarity_language.split + # next if type_data.nil? || (type_data.length != 2) || (type_data[0] != 'moss') - # Is there anything to check? - logger.debug "Checking plagiarism for #{td.name} (id=#{td.id})" - tasks = tasks_for_definition(td) - tasks_with_files = tasks.select(&:has_pdf) + # # Is there anything to check? + # logger.debug "Checking plagiarism for #{td.name} (id=#{td.id})" + # tasks = tasks_for_definition(td) + # tasks_with_files = tasks.select(&:has_pdf) - # Skip if not due yet - next if td.due_date > Time.zone.now + # # Skip if not due yet + # next if td.due_date > Time.zone.now - # Skip if no files changed - next unless tasks_with_files.count > 1 && - ( - tasks.where('tasks.file_uploaded_at > ?', last_plagarism_scan).select(&:has_pdf).count > 0 || - td.updated_at > last_plagarism_scan || - force - ) + # # Skip if no files changed + # next unless tasks_with_files.count > 1 && + # ( + # tasks.where('tasks.file_uploaded_at > ?', last_plagarism_scan).select(&:has_pdf).count > 0 || + # td.updated_at > last_plagarism_scan || + # force + # ) - # There are new tasks, check these + # # There are new tasks, check these - logger.debug 'Contacting MOSS for new checks' + # logger.debug 'Contacting MOSS for new checks' - # Create the MossRuby object - moss_key = Doubtfire::Application.credentials.secret_key_moss - raise "No moss key set. Check ENV['DF_SECRET_KEY_MOSS'] first." if moss_key.nil? + # # Create the MossRuby object + # moss_key = Doubtfire::Application.credentials.secret_key_moss + # raise "No moss key set. Check ENV['DF_SECRET_KEY_MOSS'] first." if moss_key.nil? - moss = MossRuby.new(moss_key) + # moss = MossRuby.new(moss_key) - # Set options -- the options will already have these default values - moss.options[:max_matches] = 7 - moss.options[:directory_submission] = true - moss.options[:show_num_matches] = 500 - moss.options[:experimental_server] = false - moss.options[:comment] = '' - moss.options[:language] = type_data[1] + # # Set options -- the options will already have these default values + # moss.options[:max_matches] = 7 + # moss.options[:directory_submission] = true + # moss.options[:show_num_matches] = 500 + # moss.options[:experimental_server] = false + # moss.options[:comment] = '' + # moss.options[:language] = type_data[1] - tmp_path = File.join(Dir.tmpdir, 'doubtfire', "check-#{id}-#{td.id}") + # tmp_path = File.join(Dir.tmpdir, 'doubtfire', "check-#{id}-#{td.id}") - begin - # Create a file hash, with the files to be processed - to_check = MossRuby.empty_file_hash - add_done_files_for_plagiarism_check_of(td, tmp_path, to_check, tasks_with_files) + # begin + # # Create a file hash, with the files to be processed + # to_check = MossRuby.empty_file_hash + # add_done_files_for_plagiarism_check_of(td, tmp_path, to_check, tasks_with_files) - FileUtils.chdir(tmp_path) + # FileUtils.chdir(tmp_path) - # Get server to process files - logger.debug 'Sending to MOSS...' - url = moss.check(to_check, ->(_) { print '.' }) + # # Get server to process files + # logger.debug 'Sending to MOSS...' + # url = moss.check(to_check, ->(_) { print '.' }) - logger.info "MOSS check for #{code} #{td.abbreviation} url: #{url}" + # logger.info "MOSS check for #{code} #{td.abbreviation} url: #{url}" - td.plagiarism_report_url = url - td.plagiarism_updated = true - td.save - rescue StandardError => e - logger.error "Failed to check plagiarism for task #{td.name} (id=#{td.id}). Error: #{e.message}" - ensure - FileUtils.chdir(pwd) - FileUtils.rm_rf tmp_path - end - end - self.last_plagarism_scan = Time.zone.now - save! - ensure - FileUtils.chdir(pwd) if FileUtils.pwd != pwd - end + # td.plagiarism_report_url = url + # td.plagiarism_updated = true + # td.save + # rescue StandardError => e + # logger.error "Failed to check plagiarism for task #{td.name} (id=#{td.id}). Error: #{e.message}" + # ensure + # FileUtils.chdir(pwd) + # FileUtils.rm_rf tmp_path + # end + # end + # self.last_plagarism_scan = Time.zone.now + # save! + # ensure + # FileUtils.chdir(pwd) if FileUtils.pwd != pwd + # end - self - end + # self + # end # Pass tasks on to plagarism detection software and setup links between students def check_jplag_similarity(force: false) # Get each task... return unless active - # need pwd to restore after cding into submission folder (so the files do not have full path) - pwd = FileUtils.pwd - # making temp directory for unit - jplag - root_work_dir = Rails.root.join("tmp", "jplag", "#{code}-#{id}") - unit_code = "#{code}-#{id}" - - begin - logger.info "Checking plagiarsm for unit #{code} - #{name} (id=#{id})" - - task_definitions.each do |td| - next if td.similarity_language.nil? || td.upload_requirements.nil? || td.upload_requirements.select { |upreq| upreq['type'] == 'code' && upreq['tii_check'] }.empty? - - # Is there anything to check? - logger.debug "Checking plagiarism for #{td.name} (id=#{td.id})" - tasks = tasks_for_definition(td) - tasks_with_files = tasks.select(&:has_pdf) - - # Skip if no files changed - next unless tasks_with_files.count > 1 && - ( - # NOTE: `last_plagarism_scan` is currently tracked for each Unit, not each Task Definition - tasks.where('tasks.file_uploaded_at > ?', last_plagarism_scan).select(&:has_pdf).count > 0 || - td.updated_at > last_plagarism_scan || - force - ) - - # Ensure work directory for the unit is created - FileUtils.mkdir_p(root_work_dir) - - # Init work directory for each task definition - tasks_dir = root_work_dir.join(td.id.to_s) - FileUtils.mkdir_p(tasks_dir) - - # There are new tasks, check these with JPLAG - run_jplag_on_done_files(td, tasks_dir, tasks_with_files, unit_code) - report_path = "#{Doubtfire::Application.config.jplag_report_dir}/#{unit_code}/#{td.abbreviation}-result.jplag" - warn_pct = td.plagiarism_warn_pct || 50 - logger.debug "Warn PCT: #{warn_pct}" - - # Remove any existing plagiarism links that are below the threshold, in case it has been updated since the last analysis - JplagTaskSimilarity.joins(:task) - .where("pct < ? AND tasks.task_definition_id = ?", warn_pct, td.id) - .delete_all - - process_jplag_plagiarism_report(report_path, warn_pct, td.group_set) - end - self.last_plagarism_scan = Time.zone.now - save! - ensure - FileUtils.chdir(pwd) if FileUtils.pwd != pwd + # root_work_dir = Rails.root.join("tmp", "jplag", "#{code}-#{id}") + # unit_code = "#{code}-#{id}" + + logger.info "Checking plagiarsm for unit #{code} - #{name} (id=#{id})" + + task_definitions.each do |td| + next if td.similarity_language.nil? || td.upload_requirements.nil? || td.upload_requirements.select { |upreq| upreq['type'] == 'code' && upreq['tii_check'] }.empty? + + # Is there anything to check? + logger.debug "Checking plagiarism for #{td.name} (id=#{td.id})" + # tasks = tasks_for_definition(td) + tasks_with_files = tasks_for_definition(td).select(&:has_pdf) + + # If task is still being processed, Sidekiq will Re-queue the job until all PDFs are ready + # tasks_with_files = tasks.select { |t| t.has_pdf || t.processing_pdf? } + + # tasks_with_files = tasks.where(ask_definition_id: task_def.id) + + # Skip if no files changed + next unless tasks_with_files.count > 1 && + ( + # NOTE: `last_plagarism_scan` is currently tracked for each Unit, not each Task Definition + # tasks.where('tasks.file_uploaded_at > ?', last_plagarism_scan).select(&:has_pdf).count > 0 || + tasks_with_files.any? { |t| t.file_uploaded_at > last_plagarism_scan } || + td.updated_at > last_plagarism_scan || + force + ) + + # Ensure work directory for the unit is created + # FileUtils.mkdir_p(root_work_dir) + + # Init work directory for each task definition + # tasks_dir = root_work_dir.join(td.id.to_s) + # :workdir => -> { "#{@work_id}-#{Process.pid}-#{Thread.current.object_id}-#{Time.now.to_i}" } + + # TODO: create the work folder in the jplag sidekiq job + # TODO: running jplag similarity per task definition would be nice (instead of unit) + # + + # There are new tasks, check these with JPLAG + + # process_jplag_plagiarism_report(report_path, warn_pct, td.group_set) + # run_jplag_on_done_files(td, tasks_dir, tasks_with_files, unit_code) + # report_path = "#{Doubtfire::Application.config.jplag_report_dir}/#{unit_code}/#{td.abbreviation}-result.jplag" + warn_pct = td.plagiarism_warn_pct || 50 + # logger.debug "Warn PCT: #{warn_pct}" + + # Remove any existing plagiarism links that are below the threshold, in case it has been updated since the last analysis + JplagTaskSimilarity.joins(:task) + .where("pct < ? AND tasks.task_definition_id = ?", warn_pct, td.id) + .delete_all + + # byebug + + # TODO: cleanup + JplagSimilarityJob.perform_async(td.id) + + # process_jplag_plagiarism_report(report_path, warn_pct, td.group_set) end + self.last_plagarism_scan = Time.zone.now + save! self end @@ -212,7 +222,7 @@ def update_moss_plagiarism_stats self end - private + # private # Extract all done files related to a task definition matching a pattern into a given directory. # Returns an array of files @@ -235,12 +245,12 @@ def run_jplag_on_done_files(task_definition, tasks_dir, tasks_with_files, unit_c return if similarity_pct.nil? # Check if the directory exists and create it if it doesn't - results_dir = "/jplag/results/#{unit_code}" - system("docker exec -i jplag sh -c 'if [ ! -d \"#{results_dir}\" ]; then mkdir -p \"#{results_dir}\"; fi'") || raise('Failed to create JPlag results directory') + # results_dir = "/jplag/results/#{unit_code}" + # system("docker exec -i jplag sh -c 'if [ ! -d \"#{results_dir}\" ]; then mkdir -p \"#{results_dir}\"; fi'") || raise('Failed to create JPlag results directory') # Remove existing result file if it exists - result_file = "#{results_dir}/#{task_definition.abbreviation}-result.jplag" - system("docker exec -i jplag sh -c 'if [ -f \"#{result_file}\" ]; then rm \"#{result_file}\"; fi'") || raise('Failed to remove previous JPlag report') + # result_file = "#{results_dir}/#{task_definition.abbreviation}-result.jplag" + # system("docker exec -i jplag sh -c 'if [ -f \"#{result_file}\" ]; then rm \"#{result_file}\"; fi'") || raise('Failed to remove previous JPlag report') # get each code file for each task task_definition.upload_requirements.each_with_index do |upreq, idx| @@ -255,6 +265,7 @@ def run_jplag_on_done_files(task_definition, tasks_dir, tasks_with_files, unit_c tasks_with_files.each do |t| # "name" is {taskId}/{filename}, so it will create a subdir with the task id, but we use this later when processing the report + t.extract_file_from_done(tasks_dir, pattern, lambda { |task, to_path, name| names = name.split("/") if names.count >= 2 @@ -281,19 +292,39 @@ def run_jplag_on_done_files(task_definition, tasks_dir, tasks_with_files, unit_c # Convert pct to decimal similarity_threshold = similarity_pct.to_f / 100 - min_tokens = Doubtfire::Application.config.jplag_min_tokens.to_i + logger.debug tasks_dir + # TODO: clean this up... + unit_dir = File.dirname(tasks_dir) + # FileUtils.copy(Rails.root.join("lib/shell/jplag_run.sh"), unit_dir) + + # sudo chown -R $(whoami) /jplag/results + + # min_tokens = Doubtfire::Application.config.jplag_min_tokens.to_i # If empty, let JPlag set the default per-language - min_token_string = min_tokens <= 0 ? "" : "--min-tokens=#{min_tokens}" + # min_token_string = min_tokens <= 0 ? "" : "--min-tokens=#{min_tokens}" # Run JPLAG on the extracted files. JPlag container should already be in the /jplag/ workdir. - docker_command = "docker exec -i jplag java -jar jplag-jar-with-dependencies.jar #{tasks_dir_split} -l #{file_lang} --similarity-threshold=#{similarity_threshold} #{min_token_string} -M RUN -r #{results_dir}/#{task_definition.abbreviation}-result --overwrite" + # docker_command = "docker exec -i jplag java -jar jplag-jar-with-dependencies.jar #{tasks_dir_split} -l #{file_lang} --similarity-threshold=#{similarity_threshold} #{min_token_string} -M RUN -r #{results_dir}/#{task_definition.abbreviation}-result --overwrite" + + jplag_run_script = Rails.root.join("lib/shell/jplag_run.sh") + # byebug + docker_command = "#{jplag_run_script} #{File.basename(tasks_dir)} #{file_lang} #{similarity_threshold} #{tasks_dir_split} #{unit_code}-#{task_definition.id}-#{Thread.current.object_id}" + # docker_command = "docker exec -i jplag java -jar jplag-jar-with-dependencies.jar #{tasks_dir_split} -l #{file_lang} --similarity-threshold=#{similarity_threshold} #{min_token_string} -M RUN -r #{results_dir}/#{task_definition.abbreviation}-result" logger.debug "Executing command: #{docker_command}" system(docker_command) + report_path = "#{Doubtfire::Application.config.jplag_report_dir}/#{unit_code}/#{task_definition.abbreviation}-result.jplag" + FileUtils.mkdir_p(File.dirname(report_path)) + FileUtils.copy("#{tasks_dir}/result.jplag", report_path) + + # logger.debug "Executing command: #{docker_command}" + # system(docker_command) + # Delete the extracted code files from tmp tmp_dir = Rails.root.join("tmp/jplag") logger.info "Deleting files in: #{tmp_dir}" logger.info "Files to delete: #{Dir.glob("#{tmp_dir}/*")}" - FileUtils.rm_rf(Dir.glob("#{tmp_dir}/*")) + FileUtils.rm_rf(tasks_dir) + # FileUtils.rm_rf(Dir.glob("#{tmp_dir}/*")) self end diff --git a/app/models/task.rb b/app/models/task.rb index 321ee3f35..fb3e0d48d 100644 --- a/app/models/task.rb +++ b/app/models/task.rb @@ -1013,11 +1013,7 @@ def copy_done_to(path) def clear_in_process in_process_dir = student_work_dir(:in_process, false) - if Dir.exist? in_process_dir - Dir.chdir(FileHelper.student_work_root) if FileUtils.pwd == in_process_dir - FileUtils.rm_rf in_process_dir - end - + FileUtils.rm_rf(in_process_dir) if Dir.exist?(in_process_dir) rescue StandardError => e logger.error "Error clearing in process directory for task #{log_details} - #{e.message}" end @@ -1050,12 +1046,9 @@ def move_files_to_in_process(source_folder = FileHelper.student_work_dir(:new)) return false if in_process_dir.nil? - if Dir.exist? in_process_dir - pwd = FileUtils.pwd - Dir.chdir(in_process_dir) - # move all files to the enq dir - FileUtils.rm Dir.glob('*') - Dir.chdir(pwd) + if Dir.exist?(in_process_dir) + files = Dir.glob(File.join(in_process_dir, '*')) + FileUtils.rm(files) end # Zip new submission and store in done files (will remove from_dir) - ensure trailing / @@ -1093,23 +1086,18 @@ def move_files_on_abbreviation_change(old_abbreviation) end def __output_filename__(in_dir, idx, type) - pwd = FileUtils.pwd - Dir.chdir(in_dir) - begin - # Rename files with 000.type.* to 000-type-* - result = Dir.glob("#{idx.to_s.rjust(3, '0')}.#{type}.*").first - - if !result.nil? && File.exist?(result) - FileUtils.mv result, "#{idx.to_s.rjust(3, '0')}-#{type}#{File.extname(result)}" - end - result = Dir.glob("#{idx.to_s.rjust(3, '0')}-#{type}.*").first - ensure - Dir.chdir(pwd) + prefix = idx.to_s.rjust(3, '0') + result = Dir.glob(File.join(in_dir, "#{prefix}.#{type}.*")).first + # Rename files with 000.type.* to 000-type-* + if result && File.exist?(result) + new_name = "#{prefix}-#{type}#{File.extname(result)}" + FileUtils.mv(result, File.join(in_dir, new_name)) + result = File.join(in_dir, new_name) + else + result = Dir.glob(File.join(in_dir, "#{prefix}-#{type}.*")).first end - return File.join(in_dir, result) unless result.nil? - - nil + result end def in_process_files_for_task(is_retry) @@ -1167,6 +1155,7 @@ def init(task, is_retry) @institution_name = Doubtfire::Application.config.institution[:name] @doubtfire_product_name = Doubtfire::Application.config.institution[:product_name] @include_pax = !is_retry + @work_id = "task-#{task.id}#{'-retry' if is_retry}" end def make_pdf @@ -1289,7 +1278,7 @@ def convert_submission_to_pdf(source_folder: FileHelper.student_work_dir(:new), # Try again... # Without newpax # Ensure latex aux file is removed - Dir.glob(Rails.root.join('tmp/rails-latex/**/input.aux')).each { |f| File.delete(f) } + # Dir.glob(Rails.root.join('tmp/rails-latex/**/input.aux')).each { |f| File.delete(f) } tac2 = TaskAppController.new tac2.init(self, true) @@ -1346,7 +1335,7 @@ def convert_submission_to_pdf(source_folder: FileHelper.student_work_dir(:new), raise e ensure # Ensure latex aux file is removed - if broken will cause issues for next submission in sidekiq - Dir.glob(Rails.root.join('tmp/rails-latex/**/input.aux')).each { |f| File.delete(f) } + # Dir.glob(Rails.root.join('tmp/rails-latex/**/input.aux')).each { |f| File.delete(f) } clear_in_process end diff --git a/app/sidekiq/accept_submission_job.rb b/app/sidekiq/accept_submission_job.rb index b843f66cf..e08333587 100644 --- a/app/sidekiq/accept_submission_job.rb +++ b/app/sidekiq/accept_submission_job.rb @@ -2,22 +2,41 @@ class AcceptSubmissionJob include Sidekiq::Job include LogHelper + sidekiq_options queue: :task_pdf_gen, + lock: :until_and_while_executing, + lock_args_method: ->(args) { [args.first] }, + on_conflict: :reject, + # use `retry: false` to execute this once and only once + # use `retry: 0` to send the job to the dead queue + # We can then look at the dead queue and identify the task ID and user ID + retry: 0 + + # TODO: errors should clear the in_process files + # TODO: this job itself should handle the moving of files + # TODO: sidekiq should handle the retrying of a failed compile (done in convert_submission_to_pdf) + def perform(task_id, user_id, accepted_tii_eula) begin # Ensure cwd is valid... FileUtils.cd(Rails.root) rescue StandardError => e logger.error e + raise "cwd is invalid" end - begin - task = Task.find(task_id) - user = User.find(user_id) - rescue StandardError => e - logger.error e - return + # begin + task = Task.find(task_id) + user = User.find(user_id) + + unless task.processing_pdf? + raise "No files to process?" end + # rescue StandardError => e + # logger.error e + # return + # end + begin logger.info "Accepting submission for task #{task.id} by user #{user.id}" # Convert submission to PDF @@ -41,8 +60,7 @@ def perform(task_id, user_id, accepted_tii_eula) rescue StandardError => e logger.error "Failed to send error log to admin" end - - return + raise "#{e} - #{task.log_details}" end # When converted, we can now send documents to turn it in for checking @@ -51,6 +69,7 @@ def perform(task_id, user_id, accepted_tii_eula) end rescue StandardError => e # to raise error message to avoid unnecessary retry logger.error e - task.clear_in_process + task.clear_in_process if task&.valid? + raise e end end diff --git a/app/sidekiq/generate_portfolio_job.rb b/app/sidekiq/generate_portfolio_job.rb new file mode 100644 index 000000000..dcc0d44f0 --- /dev/null +++ b/app/sidekiq/generate_portfolio_job.rb @@ -0,0 +1,61 @@ +class GeneratePortfolioJob + include Sidekiq::Job + include LogHelper + + sidekiq_options queue: :portfolio_pdf_gen, + lock: :until_and_while_executing, + lock_args_method: ->(args) { [args.first] }, + on_conflict: :reject, + # use `retry: false` to execute this once and only once + # use `retry: 0` to send the job to the dead queue + # We can then look at the dead queue and identify the task ID and user ID + retry: 0 + + def perform(project_id) + begin + # Ensure cwd is valid... + FileUtils.cd(Rails.root) + rescue StandardError => e + logger.error e + raise e + end + + begin + project = Project.find(project_id) + rescue StandardError => e + logger.error e + raise e + + # return + end + + begin + # logger.info "Accepting submission for task #{task.id} by user #{user.id}" + project.create_portfolio + rescue StandardError => e + logger.error e + raise e + # # Send email to student if task pdf failed + # if task.project.student.receive_task_notifications + # begin + # PortfolioEvidenceMailer.task_pdf_failed(task.project, [task]).deliver + # rescue StandardError => e + # logger.error "Failed to send task pdf failed email for project #{task.project.id}!\n#{e.message}" + # end + # end + + # begin + # # Notify system admin + # mail = ErrorLogMailer.error_message('Accept Submission', "Failed to convert submission to PDF for task #{task.log_details}", e) + # mail.deliver if mail.present? + # rescue StandardError => e + # logger.error "Failed to send error log to admin" + # end + + # nil + end + rescue StandardError => e # to raise error message to avoid unnecessary retry + logger.error e + raise e + end +end diff --git a/app/sidekiq/jplag_similarity_job.rb b/app/sidekiq/jplag_similarity_job.rb new file mode 100644 index 000000000..93a29370d --- /dev/null +++ b/app/sidekiq/jplag_similarity_job.rb @@ -0,0 +1,72 @@ +class JplagSimilarityJob + include Sidekiq::Job + include LogHelper + + sidekiq_options queue: :jplag, + lock: :until_and_while_executing, + lock_args_method: ->(args) { [args.first] }, + on_conflict: :reject + # use `retry: false` to execute this once and only once + # use `retry: 0` to send the job to the dead queue + # We can then look at the dead queue and identify the task ID and user ID + # retry: 0 + + # TODO: errors should clear the in_process files + # TODO: this job itself should handle the moving of files + def perform(task_def_id) + begin + # Ensure cwd is valid... + FileUtils.cd(Rails.root) + rescue StandardError => e + logger.error e + raise "cwd is invalid" + end + + # unit = Unit.find(unit_id) + td = TaskDefinition.find(task_def_id) + unit = td.unit + + tasks = unit.tasks_for_definition(td) + + processing = tasks.select(&:processing_pdf?) + if processing.any? + # We can raise errors which will force Sidekiq to retry this job later + raise "Some tasks are still processing PDFs: Task IDs: [#{processing.map(&:id).join(', ')}]" + end + + tasks_with_files = tasks.select(&:has_pdf) + + unit_code = "#{unit.code}-#{unit.id}" + + # TODO: jplag_move_files_job + # - If this one fails, we can retry + # TODO: jplag_run_job + + begin + root_work_dir = Rails.root.join("tmp", "jplag", "#{unit.code}-#{unit.id}") + tasks_dir = "#{root_work_dir}-#{Process.pid}-#{Thread.current.object_id}-#{Time.now.to_i}-#{td.id}" + FileUtils.mkdir_p(tasks_dir) + unit.run_jplag_on_done_files(td, tasks_dir, tasks_with_files, unit_code) + + # run_jplag_on_done_files(td, tasks_dir, tasks_with_files, unit_code) + report_path = "#{Doubtfire::Application.config.jplag_report_dir}/#{unit_code}/#{td.abbreviation}-result.jplag" + warn_pct = td.plagiarism_warn_pct || 50 + logger.debug "Warn PCT: #{warn_pct}" + unit.process_jplag_plagiarism_report(report_path, warn_pct, td.group_set) + rescue StandardError => e + logger.error e + + # begin + # # Notify system admin + # mail = ErrorLogMailer.error_message('JPlag Similarity', "Failed to convert submission to PDF for task #{task.log_details}", e) + # mail.deliver if mail.present? + # rescue StandardError => e + # logger.error "Failed to send error log to admin" + # end + raise "#{e} - JPlag failed on unit" + end + rescue StandardError => e # to raise error message to avoid unnecessary retry + logger.error e + raise e + end +end diff --git a/app/views/layouts/application.pdf.erbtex b/app/views/layouts/application.pdf.erbtex index 7d0aa43bc..972b8142c 100644 --- a/app/views/layouts/application.pdf.erbtex +++ b/app/views/layouts/application.pdf.erbtex @@ -8,7 +8,7 @@ { :command => './latex_run.sh', :runs => 1 } ], :preservework => false, - :workdir => -> { "#{Process.pid}-#{Thread.current.object_id}-#{Time.now.to_i}" } + :workdir => -> { "#{@work_id}-#{Process.pid}-#{Thread.current.object_id}-#{Time.now.to_i}" } } %> diff --git a/config/sidekiq.yml b/config/sidekiq.yml index 0515ae218..a5dad749d 100644 --- a/config/sidekiq.yml +++ b/config/sidekiq.yml @@ -1 +1,13 @@ -:concurrency: 1 +# :concurrency: <%= ENV.fetch("SIDEKIQ_CONCURRENCY", 5) %> +:concurrency: 5 +:queues: + - [task_pdf_gen, 10] + - [portfolio_pdf_gen, 5] + - [portfolio_download, 1] + - [jplag, 2] + +:capsules: + :single_threaded: + :queues: + - [default, 10] + :concurrency: 1 diff --git a/lib/helpers/database_populator.rb b/lib/helpers/database_populator.rb index 4134bb34b..9b55f6acb 100644 --- a/lib/helpers/database_populator.rb +++ b/lib/helpers/database_populator.rb @@ -553,7 +553,7 @@ def self.generate_portfolio(project) lsr_path = File.join(portfolio_tmp_dir, '000-document-LearningSummaryReport.pdf') FileUtils.ln_s(Rails.root.join('test_files/unit_files/sample-learning-summary.pdf'), lsr_path) unless File.exist? lsr_path project.compile_portfolio = true - project.create_portfolio + GeneratePortfolioJob.perform_async(project.id) end private diff --git a/lib/shell/jplag_run.sh b/lib/shell/jplag_run.sh new file mode 100755 index 000000000..8d5e8854d --- /dev/null +++ b/lib/shell/jplag_run.sh @@ -0,0 +1,22 @@ +#!/bin/sh + +# WORK_DIR=$(basename "$PWD") + +WORK_DIR_NAME=$1 +LANGUAGE=$2 +THRESHOLD=$3 +TASKS_DIR_SPLIT=$4 +WORK_ID=$5 + +IMAGE_NAME="$DF_JPLAG_IMAGE_NAME" +CONTAINER_NAME="jplag-$WORK_ID" + +PATH_TO_JPLAG_FILES="$DF_JPLAG_PATH_TO_WORKDIRS/$WORK_DIR_NAME" + +docker run --rm \ + --network none \ + -e TERM=xterm \ + --volumes-from $DF_API_CONTAINER_NAME \ + --name "$CONTAINER_NAME" \ + "$IMAGE_NAME" \ + java -jar /jplag/jplag-jar-with-dependencies.jar "$PATH_TO_JPLAG_FILES" -l "$LANGUAGE" --similarity-threshold="$THRESHOLD" -M RUN -r "$PATH_TO_JPLAG_FILES"/result.jplag --overwrite diff --git a/lib/shell/latex_build.sh b/lib/shell/latex_build.sh index c853bdb20..b03877065 100644 --- a/lib/shell/latex_build.sh +++ b/lib/shell/latex_build.sh @@ -4,7 +4,7 @@ OUTPUT_DIR=$1 -cd /workdir/texlive-latex/${OUTPUT_DIR} +cd "$DF_LATEX_PATH_TO_WORKDIRS/${OUTPUT_DIR}" # Initialise work subfolder mkdir -p work diff --git a/lib/shell/latex_run.sh b/lib/shell/latex_run.sh index 9e546d4de..9290bafc5 100755 --- a/lib/shell/latex_run.sh +++ b/lib/shell/latex_run.sh @@ -1,5 +1,21 @@ #!/bin/sh -# This script is copied into tmp/rails-latex/$WORK_DIR/ and executed by rails-latex + +# You can mount your own script onto lib/shell/latex_run.sh +# and run custom PDF generation +# +# eg. If you have lualatex installed in the same container +# You can call it directly, instead of `docker exec` +# +# $: lualatex input.tex WORK_DIR=$(basename "$PWD") -docker exec -e TERM=xterm $LATEX_CONTAINER_NAME ${LATEX_BUILD_PATH:-/texlive/shell/latex_build.sh} $WORK_DIR +IMAGE_NAME="$DF_LATEX_IMAGE_NAME" +CONTAINER_NAME="texlive-job-$WORK_DIR" + +docker run --rm \ + -e TERM=xterm \ + -e DF_LATEX_PATH_TO_WORKDIRS="$DF_LATEX_PATH_TO_WORKDIRS" \ + --volumes-from $DF_API_CONTAINER_NAME \ + --name "$CONTAINER_NAME" \ + "$IMAGE_NAME" \ + ${LATEX_BUILD_PATH:-/texlive/shell/latex_build.sh} $WORK_DIR diff --git a/lib/tasks/generate_pdfs.rake b/lib/tasks/generate_pdfs.rake index 4daf14bf9..039d7594f 100644 --- a/lib/tasks/generate_pdfs.rake +++ b/lib/tasks/generate_pdfs.rake @@ -83,7 +83,7 @@ namespace :submission do puts "Project #{project.id} has a learning summary but no portfolio" project.update compile_portfolio: true - project.create_portfolio + GeneratePortfolioJob.perform_async(project.id) end end end diff --git a/lib/tasks/pause_sidekiq.rake b/lib/tasks/pause_sidekiq.rake new file mode 100644 index 000000000..d11ffba12 --- /dev/null +++ b/lib/tasks/pause_sidekiq.rake @@ -0,0 +1,7 @@ +# lib/tasks/pause_sidekiq.rake +namespace :db do + desc 'Pauses Sidekiq Queue' + task pause_sidekiq: [:environment] do + Sidekiq::ProcessSet.new.each(&:quiet!) + end +end diff --git a/test/api/projects_api_test.rb b/test/api/projects_api_test.rb index 4e49eced6..8daa63544 100644 --- a/test/api/projects_api_test.rb +++ b/test/api/projects_api_test.rb @@ -113,35 +113,37 @@ def test_projects_works_with_inactive_units end def test_submitted_grade_cant_change_after_submission - user = FactoryBot.create(:user, :student, enrol_in: 1) - project = user.projects.first + Sidekiq::Testing.inline! do + user = FactoryBot.create(:user, :student, enrol_in: 1) + project = user.projects.first - data_to_put = { - id: project.id, - submitted_grade: 2 - } + data_to_put = { + id: project.id, + submitted_grade: 2 + } - add_auth_header_for(user: user) + add_auth_header_for(user: user) - put_json "/api/projects/#{project.id}", data_to_put - project.reload + put_json "/api/projects/#{project.id}", data_to_put + project.reload - assert_equal 200, last_response.status, last_response_body - assert_equal user.projects.find(project.id).submitted_grade, 2 + assert_equal 200, last_response.status, last_response_body + assert_equal user.projects.find(project.id).submitted_grade, 2 - keys = %w(campus_id target_grade submitted_grade compile_portfolio portfolio_available uses_draft_learning_summary) + keys = %w[campus_id target_grade submitted_grade compile_portfolio portfolio_available uses_draft_learning_summary] - assert_json_limit_keys_to_exactly keys, last_response_body - assert_json_matches_model project, last_response_body, keys + assert_json_limit_keys_to_exactly keys, last_response_body + assert_json_matches_model project, last_response_body, keys - DatabasePopulator.generate_portfolio(project) + DatabasePopulator.generate_portfolio(project) - data_to_put['submitted_grade'] = 1 + data_to_put['submitted_grade'] = 1 - put_json "/api/projects/#{project.id}", data_to_put + put_json "/api/projects/#{project.id}", data_to_put - assert_not_equal user.projects.find(project.id).submitted_grade, 1 - assert_equal 403, last_response.status + assert_not_equal user.projects.find(project.id).submitted_grade, 1 + assert_equal 403, last_response.status + end end def test_download_portfolio diff --git a/test/api/tasks_api_test.rb b/test/api/tasks_api_test.rb index 48d8b73a4..9b07eddb5 100644 --- a/test/api/tasks_api_test.rb +++ b/test/api/tasks_api_test.rb @@ -590,102 +590,107 @@ def test_cant_submit_until_prerequisites_submitted end def test_prerequisites_task_status - # Create a unit and two task definitions - unit = FactoryBot.create(:unit, student_count: 1, task_count: 10) - td1 = unit.task_definitions.first - td2 = unit.task_definitions.second - - td1.update( - upload_requirements: [{ "key" => 'file0', "name" => 'Shape Class', "type" => 'code' }], - target_grade: 0, # Pass - start_date: Time.zone.now - 2.weeks, - target_date: Time.zone.now + 1.week - ) - - td2.update( - upload_requirements: [{ "key" => 'file0', "name" => 'Shape Class', "type" => 'code' }], - target_grade: 3, # HD - start_date: Time.zone.now - 2.weeks, - target_date: Time.zone.now + 1.week - ) + Sidekiq::Testing.inline! do + # Create a unit and two task definitions + unit = FactoryBot.create(:unit, student_count: 1, task_count: 10) + td1 = unit.task_definitions.first + td2 = unit.task_definitions.second - project = unit.active_projects.first + assert_not td1.nil? + assert_not td2.nil? - # Add username and auth_token to Header - add_auth_header_for(user: project.user) + td1.update!( + upload_requirements: [], + target_grade: 0, # Pass + start_date: Time.zone.now - 2.weeks, + target_date: Time.zone.now + 1.week + ) - data_to_post = { - trigger: 'ready_for_feedback' - } + td2.update!( + # upload_requirements: [{ "key" => 'file0', "name" => 'Shape Class', "type" => 'code' }], + upload_requirements: [], + target_grade: 3, # HD + start_date: Time.zone.now - 2.weeks, + target_date: Time.zone.now + 1.week + ) + + project = unit.active_projects.first - # Create a prerequisite on the second taskDef that adds the first taskDef as a prereq - prereq = TaskPrerequisite.create!( - task_definition: td2, # Before you can submit td2... - prerequisite: td1, # You need to submit td1 - task_status_id: TaskStatus.ready_for_feedback.id - ) - - assert prereq.valid? - - tests = [ - { - prerequisite_status: TaskStatus.ready_for_feedback, - required_status: TaskStatus.ready_for_feedback, - expected_status: 201, - expected_error: nil - }, - { - prerequisite_status: TaskStatus.discuss, - required_status: TaskStatus.discuss, - expected_status: 201, - expected_error: nil - }, - { - prerequisite_status: TaskStatus.demonstrate, - required_status: TaskStatus.demonstrate, - expected_status: 201, - expected_error: nil - }, - { - prerequisite_status: TaskStatus.discuss, - required_status: TaskStatus.demonstrate, - expected_status: 201, - expected_error: nil - }, - { - prerequisite_status: TaskStatus.demonstrate, - required_status: TaskStatus.discuss, - expected_status: 201, - expected_error: nil - }, - { - prerequisite_status: TaskStatus.complete, - required_status: TaskStatus.complete, - expected_status: 201, - expected_error: nil - }, - { - prerequisite_status: TaskStatus.complete, - required_status: TaskStatus.ready_for_feedback, - expected_status: 201, - expected_error: nil - }, - { - prerequisite_status: TaskStatus.discuss, - required_status: TaskStatus.ready_for_feedback, - expected_status: 201, - expected_error: nil + # Add username and auth_token to Header + add_auth_header_for(user: project.user) + + data_to_post = { + trigger: 'ready_for_feedback' } - ] - Sidekiq::Testing.inline! do + # Create a prerequisite on the second taskDef that adds the first taskDef as a prereq + prereq = TaskPrerequisite.create!( + task_definition: td2, # Before you can submit td2... + prerequisite: td1, # You need to submit td1 + task_status_id: TaskStatus.ready_for_feedback.id + ) + + assert prereq.valid? + + tests = [ + { + prerequisite_status: TaskStatus.ready_for_feedback, + required_status: TaskStatus.ready_for_feedback, + expected_status: 201, + expected_error: nil + }, + { + prerequisite_status: TaskStatus.discuss, + required_status: TaskStatus.discuss, + expected_status: 201, + expected_error: nil + }, + { + prerequisite_status: TaskStatus.demonstrate, + required_status: TaskStatus.demonstrate, + expected_status: 201, + expected_error: nil + }, + { + prerequisite_status: TaskStatus.discuss, + required_status: TaskStatus.demonstrate, + expected_status: 201, + expected_error: nil + }, + { + prerequisite_status: TaskStatus.demonstrate, + required_status: TaskStatus.discuss, + expected_status: 201, + expected_error: nil + }, + { + prerequisite_status: TaskStatus.complete, + required_status: TaskStatus.complete, + expected_status: 201, + expected_error: nil + }, + { + prerequisite_status: TaskStatus.complete, + required_status: TaskStatus.ready_for_feedback, + expected_status: 201, + expected_error: nil + }, + { + prerequisite_status: TaskStatus.discuss, + required_status: TaskStatus.ready_for_feedback, + expected_status: 201, + expected_error: nil + } + ] + prereq_task = project.task_for_task_definition(td1) task = project.task_for_task_definition(td2) - data_to_post = with_file('test_files/submissions/program.cs', 'application/json', data_to_post) + # data_to_post = with_file('test_files/submissions/program.cs', 'application/json', data_to_post) tests.each do |test| prereq_task.update(task_status_id: test[:prerequisite_status].id) task.update(task_status_id: TaskStatus.not_started.id) + td2.reload post "/api/projects/#{project.id}/task_def_id/#{td2.id}/submission", data_to_post assert_equal test[:expected_status], last_response.status, last_response_body @@ -697,6 +702,11 @@ def test_prerequisites_task_status # Ensure submission was denied assert_equal TaskStatus.not_started, task.task_status end + task.clear_in_process + task.reload + + # HACK: the accept submission job doesnt quite finish in time, so we clear all the files before the next loop + FileUtils.rm_rf(File.join(FileHelper.student_work_dir(:new), task.id.to_s)) end prereq.destroy diff --git a/test/models/task_similarity_test.rb b/test/models/task_similarity_test.rb index 9e3e421ac..75cc4ec55 100644 --- a/test/models/task_similarity_test.rb +++ b/test/models/task_similarity_test.rb @@ -53,64 +53,66 @@ def test_jplag_similarity_pct post "/api/projects/#{student2_project.id}/task_def_id/#{td.id}/submission", data_to_post assert_equal 201, last_response.status - student2_task = student2_project.task_for_task_definition(td) - student2_task.convert_submission_to_pdf(log_to_stdout: false) - assert File.exist? student2_task.final_pdf_path - assert student2_task.has_pdf + Sidekiq::Testing.inline! do + student2_task = student2_project.task_for_task_definition(td) + student2_task.convert_submission_to_pdf(log_to_stdout: false) + assert File.exist? student2_task.final_pdf_path + assert student2_task.has_pdf - # Run jplag - unit.check_jplag_similarity(force: true) + # Run jplag + unit.check_jplag_similarity(force: true) - # Validate similarities - similarity1 = JplagTaskSimilarity.find_by(task_id: student1_task.id) - similarity2 = JplagTaskSimilarity.find_by(task_id: student2_task.id) + # Validate similarities + similarity1 = JplagTaskSimilarity.find_by(task_id: student1_task.id) + similarity2 = JplagTaskSimilarity.find_by(task_id: student2_task.id) - assert_not_nil similarity1, "Similarity1 does not exist" - assert_not_nil similarity2, "Similarity2 does not exist" + assert_not_nil similarity1, "Similarity1 does not exist" + assert_not_nil similarity2, "Similarity2 does not exist" - assert similarity1.valid?, similarity1.errors.full_messages - assert similarity2.valid?, similarity2.errors.full_messages + assert similarity1.valid?, similarity1.errors.full_messages + assert similarity2.valid?, similarity2.errors.full_messages - assert_not_nil similarity1.other_task, "Similarity1 'other_task' is nil" - assert_not_nil similarity2.other_task, "Similarity2 'other_task' is nil" + assert_not_nil similarity1.other_task, "Similarity1 'other_task' is nil" + assert_not_nil similarity2.other_task, "Similarity2 'other_task' is nil" - assert similarity1.other_task.valid?, similarity1.errors.full_messages - assert similarity2.other_task.valid?, similarity2.errors.full_messages + assert similarity1.other_task.valid?, similarity1.errors.full_messages + assert similarity2.other_task.valid?, similarity2.errors.full_messages - assert_not_nil similarity1.other_student, "Similarity1 'other_student' is nil" - assert_not_nil similarity2.other_student, "Similarity2 'other_student' is nil" + assert_not_nil similarity1.other_student, "Similarity1 'other_student' is nil" + assert_not_nil similarity2.other_student, "Similarity2 'other_student' is nil" - assert similarity1.other_student.valid?, similarity1.errors.full_messages - assert similarity2.other_student.valid?, similarity2.errors.full_messages + assert similarity1.other_student.valid?, similarity1.errors.full_messages + assert similarity2.other_student.valid?, similarity2.errors.full_messages - assert_equal similarity1.other_task_id, similarity2.task_id - assert_equal similarity2.other_task_id, similarity1.task_id + assert_equal similarity1.other_task_id, similarity2.task_id + assert_equal similarity2.other_task_id, similarity1.task_id - assert_equal 100, similarity1.pct - assert_equal 100, similarity2.pct + assert_equal 100, similarity1.pct + assert_equal 100, similarity2.pct - assert td.has_jplag_report?, "Expected task definition to have a JPlag report" + assert td.has_jplag_report?, "Expected task definition to have a JPlag report" - # Create a similarity below the threshold - similarity1 = JplagTaskSimilarity.create( - task: student1_task, - other_task: student2_task, - pct: 10, - flagged: true - ) + # Create a similarity below the threshold + similarity1 = JplagTaskSimilarity.create( + task: student1_task, + other_task: student2_task, + pct: 10, + flagged: true + ) - # Create a similarity above the threshold - similarity2 = JplagTaskSimilarity.create( - task: student1_task, - other_task: student2_task, - pct: 99, - flagged: true - ) + # Create a similarity above the threshold + similarity2 = JplagTaskSimilarity.create( + task: student1_task, + other_task: student2_task, + pct: 99, + flagged: true + ) - unit.check_jplag_similarity(force: true) + unit.check_jplag_similarity(force: true) - assert_not JplagTaskSimilarity.exists?(similarity1.id), "Similarity with lower threshold whould have been deleted" - assert JplagTaskSimilarity.exists?(similarity2.id), "Similarity with higher threshold should not have been deleted" + assert_not JplagTaskSimilarity.exists?(similarity1.id), "Similarity with lower threshold whould have been deleted" + assert JplagTaskSimilarity.exists?(similarity2.id), "Similarity with higher threshold should not have been deleted" + end end # Test that when you create a plagiarism match link, that a moss test needs the other task @@ -122,7 +124,7 @@ def test_other_details pct: 10 ) - refute similarity.valid?, similarity.errors.full_messages + assert_not similarity.valid?, similarity.errors.full_messages similarity.other_task = task assert similarity.valid?, similarity.errors.full_messages @@ -140,7 +142,6 @@ def test_other_details ) assert tii_similarity.valid?, tii_similarity.errors.full_messages - ensure task&.project&.unit&.destroy end @@ -158,9 +159,9 @@ def test_similarity_pct assert similarity.valid?, similarity.errors.full_messages similarity.pct = -1 - refute similarity.valid? + assert_not similarity.valid? similarity.pct = 101 - refute similarity.valid? + assert_not similarity.valid? similarity.pct = 0 assert similarity.valid?, similarity.errors.full_messages @@ -239,10 +240,9 @@ def test_fetch_viewer_url add_auth_header_for(user: task.unit.main_convenor_user) # This will post to get the viewer url - viewer_url_request = stub_request(:post, "https://#{ENV['TCA_HOST']}/api/v1/submissions/1223/viewer-url"). - with(tii_headers). - to_return(status: 200, body: TCAClient::SimilarityViewerUrlResponse.new(viewer_url: 'https://viewer.url').to_hash.to_json, headers: {} - ) + viewer_url_request = stub_request(:post, "https://#{ENV.fetch('TCA_HOST', nil)}/api/v1/submissions/1223/viewer-url") + .with(tii_headers) + .to_return(status: 200, body: TCAClient::SimilarityViewerUrlResponse.new(viewer_url: 'https://viewer.url').to_hash.to_json, headers: {}) get "/api/tasks/#{task.id}/similarities/#{sim.id}/viewer_url" assert_equal 200, last_response.status diff --git a/test/models/unit_model_test.rb b/test/models/unit_model_test.rb index 3aed08123..d9f84b1e7 100644 --- a/test/models/unit_model_test.rb +++ b/test/models/unit_model_test.rb @@ -782,28 +782,31 @@ def test_change_main_convenor_does_not_allow_students_to_be_epmployed end def test_portfolio_zip - unit = FactoryBot.create :unit, campus_count: 2, tutorials:2, stream_count:2, task_count:1, student_count:1, unenrolled_student_count: 0, part_enrolled_student_count: 1 + Sidekiq::Testing.inline! do + unit = FactoryBot.create :unit, campus_count: 2, tutorials: 2, stream_count: 2, task_count: 1, student_count: 1, unenrolled_student_count: 0, part_enrolled_student_count: 1 - paths = [] + paths = [] - unit.active_projects.each do |p| - DatabasePopulator.generate_portfolio(p) - assert p.portfolio_exists? - assert File.exist?(p.portfolio_path) - paths << p.portfolio_path - end + unit.active_projects.each do |p| + DatabasePopulator.generate_portfolio(p) + p.reload + assert p.portfolio_exists? + assert File.exist?(p.portfolio_path) + paths << p.portfolio_path + end - filename = unit.get_portfolio_zip(unit.main_convenor_user) - assert File.exist? filename - Zip::File.open(filename) do |zip_file| - assert_equal unit.active_projects.count, zip_file.count - end - FileUtils.rm filename + filename = unit.get_portfolio_zip(unit.main_convenor_user) + assert File.exist? filename + Zip::File.open(filename) do |zip_file| + assert_equal unit.active_projects.count, zip_file.count + end + FileUtils.rm filename - unit.destroy! + unit.destroy! - paths.each do |path| - refute File.exist?(path) + paths.each do |path| + assert_not File.exist?(path) + end end end @@ -911,81 +914,85 @@ def test_change_unit_code_moves_files def test_archive_unit Doubtfire::Application.config.archive_units = true - unit = FactoryBot.create :unit, student_count: 1, unenrolled_student_count: 0, inactive_student_count: 0, task_count: 1, tutorials: 1, outcome_count: 0, staff_count: 0, campus_count: 1 - - td = unit.task_definitions.first - assert_not File.exist?(td.task_sheet) - FileUtils.touch(td.task_sheet) - assert File.exist?(td.task_sheet) + Sidekiq::Testing.inline! do + unit = FactoryBot.create :unit, student_count: 1, unenrolled_student_count: 0, inactive_student_count: 0, task_count: 1, tutorials: 1, outcome_count: 0, staff_count: 0, campus_count: 1 - old_path = td.task_sheet + td = unit.task_definitions.first + assert_not File.exist?(td.task_sheet) + FileUtils.touch(td.task_sheet) + assert File.exist?(td.task_sheet) - # also check tasks - p = unit.projects.first - task = p.task_for_task_definition(td) - task_pdf = task.final_pdf_path - FileUtils.touch(task_pdf) + old_path = td.task_sheet - DatabasePopulator.generate_portfolio(p) - old_portfolio_path = p.portfolio_path + # also check tasks + p = unit.projects.first + task = p.task_for_task_definition(td) + task_pdf = task.final_pdf_path + FileUtils.touch(task_pdf) - old_submission_history_path = FileHelper.task_submission_identifier_path_with_timestamp(:done, task, '123/45') - FileUtils.mkdir_p(old_submission_history_path) - FileUtils.touch(File.join(old_submission_history_path, 'output.txt')) + old_portfolio_path = p.portfolio_path - assert File.exist?(old_path) - assert File.exist?(task_pdf) - assert File.exist?(old_portfolio_path) - assert File.exist?(old_submission_history_path) - assert File.exist?(File.join(old_submission_history_path, 'output.txt')) - - unit.move_files_to_archive - unit.archived = true - unit.save! - - td.reload - task.reload - - assert_not File.exist?(old_path), "Old file still exists" - assert File.exist?(td.task_sheet), "New file does not exist - #{td.task_sheet}" - assert_not File.exist?(task_pdf), "Old task file still exists" - assert File.exist?(task.final_pdf_path), "New task file does not exist" - assert_not File.exist?(old_portfolio_path), "Old portfolio file still exists - #{old_portfolio_path}" - assert File.exist?(p.portfolio_path), "New portfolio file does not exist" - assert_not File.exist?(old_submission_history_path), "Old submission history still exists - #{old_submission_history_path}" - assert File.exist?(FileHelper.task_submission_identifier_path(:done, task)) - assert File.exist?(File.join(FileHelper.task_submission_identifier_path_with_timestamp(:done, task, '123_45'), 'output.txt')) - - assert File.exist?(task.final_pdf_path), "Portfolio evidence file does not exist - #{task.final_pdf_path}" - - td.abbreviation = 'NEW' - td.save - task.reload - - # File exists after rename - assert File.exist?(task.final_pdf_path), "Portfolio evidence file does not exist - #{task.final_pdf_path}" - assert File.exist?(FileHelper.task_submission_identifier_path(:done, task)) - assert File.exist?(File.join(FileHelper.task_submission_identifier_path_with_timestamp(:done, task, '123_45'), 'output.txt')) - - p.student.update(username: 'NEW_USERNAME') - task.reload - assert File.exist?(task.final_pdf_path), "Portfolio evidence file does not exist after username change - #{task.final_pdf_path}" - assert File.exist?(p.portfolio_path), "New portfolio file does not exist" - assert File.exist?(FileHelper.task_submission_identifier_path(:done, task)) - assert File.exist?(File.join(FileHelper.task_submission_identifier_path_with_timestamp(:done, task, '123_45'), 'output.txt')) + DatabasePopulator.generate_portfolio(p) - new_tp = FactoryBot.create :teaching_period - new_unit = unit.rollover(new_tp, nil, nil, nil) + old_submission_history_path = FileHelper.task_submission_identifier_path_with_timestamp(:done, task, '123/45') + FileUtils.mkdir_p(old_submission_history_path) + FileUtils.touch(File.join(old_submission_history_path, 'output.txt')) - assert_not new_unit.archived + assert File.exist?(old_path) + assert File.exist?(task_pdf) + assert File.exist?(old_portfolio_path) + assert File.exist?(old_submission_history_path) + assert File.exist?(File.join(old_submission_history_path, 'output.txt')) - unit.destroy! + unit.move_files_to_archive + unit.archived = true + unit.save! - assert_not File.exist?(td.task_sheet), "New file exists after delete - #{td.task_sheet}" - assert_not File.exist?(task.final_pdf_path), "New task file exists after delete - #{task.final_pdf_path}" - assert_not File.exist?(p.portfolio_path), "New portfolio exists after delete - #{p.portfolio_path}" - assert_not File.exist?(FileHelper.task_submission_identifier_path(:done, task)) - assert_not File.exist?(File.join(FileHelper.task_submission_identifier_path_with_timestamp(:done, task, '123_45'), 'output.txt')) + td.reload + task.reload + p.reload + + assert_not File.exist?(old_path), "Old file still exists" + assert File.exist?(td.task_sheet), "New file does not exist - #{td.task_sheet}" + assert_not File.exist?(task_pdf), "Old task file still exists" + assert File.exist?(task.final_pdf_path), "New task file does not exist" + assert_not File.exist?(old_portfolio_path), "Old portfolio file still exists - #{old_portfolio_path}" + assert File.exist?(p.portfolio_path), "New portfolio file does not exist" + assert_not File.exist?(old_submission_history_path), "Old submission history still exists - #{old_submission_history_path}" + assert File.exist?(FileHelper.task_submission_identifier_path(:done, task)) + assert File.exist?(File.join(FileHelper.task_submission_identifier_path_with_timestamp(:done, task, '123_45'), 'output.txt')) + + assert File.exist?(task.final_pdf_path), "Portfolio evidence file does not exist - #{task.final_pdf_path}" + + td.abbreviation = 'NEW' + td.save + task.reload + + # File exists after rename + assert File.exist?(task.final_pdf_path), "Portfolio evidence file does not exist - #{task.final_pdf_path}" + assert File.exist?(FileHelper.task_submission_identifier_path(:done, task)) + assert File.exist?(File.join(FileHelper.task_submission_identifier_path_with_timestamp(:done, task, '123_45'), 'output.txt')) + + p.student.update(username: 'NEW_USERNAME') + task.reload + assert File.exist?(task.final_pdf_path), "Portfolio evidence file does not exist after username change - #{task.final_pdf_path}" + assert File.exist?(p.portfolio_path), "New portfolio file does not exist" + assert File.exist?(FileHelper.task_submission_identifier_path(:done, task)) + assert File.exist?(File.join(FileHelper.task_submission_identifier_path_with_timestamp(:done, task, '123_45'), 'output.txt')) + + new_tp = FactoryBot.create :teaching_period + new_unit = unit.rollover(new_tp, nil, nil, nil) + + assert_not new_unit.archived + + unit.destroy! + + assert_not File.exist?(td.task_sheet), "New file exists after delete - #{td.task_sheet}" + assert_not File.exist?(task.final_pdf_path), "New task file exists after delete - #{task.final_pdf_path}" + assert_not File.exist?(p.portfolio_path), "New portfolio exists after delete - #{p.portfolio_path}" + assert_not File.exist?(FileHelper.task_submission_identifier_path(:done, task)) + assert_not File.exist?(File.join(FileHelper.task_submission_identifier_path_with_timestamp(:done, task, '123_45'), 'output.txt')) + end ensure Doubtfire::Application.config.archive_units = false end