From 715ccaf101655e63e9d081ef9b364cfccae62d13 Mon Sep 17 00:00:00 2001 From: Andrew Cain Date: Fri, 25 Oct 2024 20:57:37 +1100 Subject: [PATCH] fix: ensure broken aux file does not kill future pdf generation --- app/models/task.rb | 10 ++++-- test/models/task_test.rb | 52 +++++++++++++++++++++++++++++++ test_files/latex/input-broken.aux | 40 ++++++++++++++++++++++++ 3 files changed, 100 insertions(+), 2 deletions(-) create mode 100644 test_files/latex/input-broken.aux diff --git a/app/models/task.rb b/app/models/task.rb index ffa7f7a77..46759a522 100644 --- a/app/models/task.rb +++ b/app/models/task.rb @@ -1167,8 +1167,11 @@ def convert_submission_to_pdf(source_folder: FileHelper.student_work_dir(:new), begin pdf_text = tac.make_pdf rescue => e - # Try again... with convert to ascic - # + # 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) } + tac2 = TaskAppController.new tac2.init(self, true) @@ -1234,6 +1237,9 @@ def convert_submission_to_pdf(source_folder: FileHelper.student_work_dir(:new), add_text_comment project.tutor_for(task_definition), "**Automated Comment**: Something went wrong with your submission. Check the files and resubmit this task. #{e.message}" 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) } + clear_in_process end end diff --git a/test/models/task_test.rb b/test/models/task_test.rb index 212e75508..308b35a66 100644 --- a/test/models/task_test.rb +++ b/test/models/task_test.rb @@ -757,6 +757,58 @@ def test_pdf_creation_fails_on_invalid_pdf end end + def test_pax_crash_does_not_stop_pdf_creation + unit = FactoryBot.create(:unit, student_count: 1, task_count: 0) + td = TaskDefinition.new({ + unit_id: unit.id, + tutorial_stream: unit.tutorial_streams.first, + name: 'PDF Test Task', + description: 'Test task', + weighting: 4, + target_grade: 0, + start_date: unit.start_date + 1.week, + target_date: unit.start_date + 2.weeks, + abbreviation: 'PDFTestTask', + restrict_status_updates: false, + upload_requirements: [ { "key" => 'file0', "name" => 'A pdf file', "type" => 'document' } ], + plagiarism_warn_pct: 0.8, + is_graded: false, + max_quality_pts: 0 + }) + td.save! + + data_to_post = { + trigger: 'ready_for_feedback' + } + + # submit an encrypted (but valid) PDF file and ensure it's rejected immediately + data_to_post = with_file('test_files/submissions/valid.pdf', 'application/json', data_to_post) + + project = unit.active_projects.first + + add_auth_header_for user: unit.main_convenor_user + + post "/api/projects/#{project.id}/task_def_id/#{td.id}/submission", data_to_post + + assert_equal 201, last_response.status, last_response_body + + task = project.task_for_task_definition(td) + + rails_latex_path = Rails.root.join("tmp/rails-latex/#{Process.pid}-#{Thread.current.hash}") + FileUtils.mkdir_p(rails_latex_path) + FileUtils.cp(Rails.root.join('test_files/latex/input-broken.aux'), "#{rails_latex_path}/input.aux") + + assert task.convert_submission_to_pdf(log_to_stdout: false) + path = task.zip_file_path_for_done_task + assert path + assert File.exist? path + assert File.exist? task.final_pdf_path + + td.destroy + assert_not File.exist? path + unit.destroy! + end + def test_accept_files_checks_they_all_exist project = FactoryBot.create(:project) unit = project.unit diff --git a/test_files/latex/input-broken.aux b/test_files/latex/input-broken.aux new file mode 100644 index 000000000..13a13ad73 --- /dev/null +++ b/test_files/latex/input-broken.aux @@ -0,0 +1,40 @@ +\relax +\providecommand\hyper@newdestlabel[2]{} +\providecommand\HyField@AuxAddToFields[1]{} +\providecommand\HyField@AuxAddToCoFields[2]{} +\providecommand{\NEWPAX@DestReq}[2]{} +\providecommand{\NEWPAX@DestProv}[2]{} +\providecommand\BKM@entry[2]{} +\NEWPAX@DestProv{000-document.newpax}{Congratulations!} +\NEWPAX@DestProv{000-document.newpax}{Decision-Tree} +\NEWPAX@DestProv{000-document.newpax}{Explanation-using-LIME} +\NEWPAX@DestProv{000-document.newpax}{Feature-importance:} +\NEWPAX@DestProv{000-document.newpax}{LIME-for-explaining-prediction-images} +\NEWPAX@DestProv{000-document.newpax}{LIME:} +\NEWPAX@DestProv{000-document.newpax}{Model-Interpretation-Methods} +\NEWPAX@DestProv{000-document.newpax}{Model-Performance} +\NEWPAX@DestProv{000-document.newpax}{Pre-processing} +\NEWPAX@DestProv{000-document.newpax}{Predictions-on-the-test-data} +\NEWPAX@DestProv{000-document.newpax}{Split-Train-and-Test-Datasets} +\NEWPAX@DestProv{000-document.newpax}{Training-the-classification-model} +\NEWPAX@DestProv{000-document.newpax}{Understanding-the-Census-Income-Dataset} +\NEWPAX@DestProv{000-document.newpax}{Visualzing-the-Tree} +\NEWPAX@DestProv{000-document.newpax}{When-a-person's-income-%3C=-$50K} +\NEWPAX@DestProv{000-document.newpax}{When-a-person's-income-%3E-$50K} +\NEWPAX@DestProv{000-document.newpax}{When-a-person's-income-actual-is-different-than-predicted} +\NEWPAX@DestProv{000-document.newpax}{Your-own-test-example} +\NEWPAX@DestProv{000-document.newpax}{sk-container-id-1} +\NEWPAX@DestProv{000-document.newpax}{sk-container-id-2} +\NEWPAX@DestProv{000-document.newpax}{sk-estimator-id-1} +\NEWPAX@DestProv{000-document.newpax}{sk-estimator-id-2} +\NEWPAX@DestProv{000-document.newpax}{top_div8WD77G29ZHEFM8A} +\NEWPAX@DestProv{000-document.newpax}{top_divDR5YA6I6PZOKFNQ} +\NEWPAX@DestProv{000-document.newpax}{top_divGH11HTIE1SXJUDR} +\NEWPAX@DestProv{000-document.newpax}{top_divQ4ALLQ5KXQWP33K} +\NEWPAX@DestProv{000-document.newpax}{top_divR1OKF6JPUWMW89V} +\NEWPAX@DestProv{000-document.newpax}{top_divSJB1S48PH4T84MA} +\NEWPAX@DestProv{000-document.newpax}{top_divWA6RJ9LWIJMH2XT} +\newlabel{LastPage}{{}{27}{}{page.27}{}} +\gdef\lastpage@lastpage{27} +\gdef\lastpage@lastpageHy{27} +\gdef \@abspage@last{28}