From 617a32f3f0cc5fc3d2b5ec77653e75f3a8b0ca63 Mon Sep 17 00:00:00 2001 From: Matthew Hilton Date: Thu, 6 Oct 2022 13:12:13 +1000 Subject: [PATCH 1/2] MDL-74911 assignfeedback_editpdf: unlock session when polling conversion Polling conversions will run the document conversion immediately if the conversion has not already been completed. Releasing the session lock while doing this ensures large conversions do not unnecessarily hold the users session. --- mod/assign/feedback/editpdf/ajax.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/mod/assign/feedback/editpdf/ajax.php b/mod/assign/feedback/editpdf/ajax.php index 624033302f3..a9960c6096f 100644 --- a/mod/assign/feedback/editpdf/ajax.php +++ b/mod/assign/feedback/editpdf/ajax.php @@ -52,6 +52,9 @@ if (!$assignment->can_view_submission($userid)) { } if ($action === 'pollconversions') { + // Poll conversions does not require session lock. + \core\session\manager::write_close(); + $draft = true; if (!has_capability('mod/assign:grade', $context)) { // A student always sees the readonly version. From e02aa3ad9a3daf4a3c5bbaeae92f304361743960 Mon Sep 17 00:00:00 2001 From: Matthew Hilton Date: Mon, 12 Sep 2022 08:30:52 +1000 Subject: [PATCH 2/2] MDL-74911 assignfeedback_editpdf: per-assignment conversion poll lock The lock ensures that multiple conversions do not happen for the same assignment submission. Otherwise, subsequent conversions will fail when trying to save the converted file. --- mod/assign/feedback/editpdf/ajax.php | 129 ++++++++++++++++----------- 1 file changed, 75 insertions(+), 54 deletions(-) diff --git a/mod/assign/feedback/editpdf/ajax.php b/mod/assign/feedback/editpdf/ajax.php index a9960c6096f..07f1188d57e 100644 --- a/mod/assign/feedback/editpdf/ajax.php +++ b/mod/assign/feedback/editpdf/ajax.php @@ -68,7 +68,22 @@ if ($action === 'pollconversions') { $draft = false; } - $response = (object) [ + // Get a lock for the PDF/Image conversion of the assignment files. + $lockfactory = \core\lock\lock_config::get_lock_factory('assignfeedback_editpdf_pollconversions'); + $resource = "user:${userid},assignmentid:${assignmentid},attemptnumber:${attemptnumber}"; + $lock = $lockfactory->get_lock($resource, 0); + + // Could not get lock, send back JSON to poll again. + if (!$lock) { + echo json_encode([ + 'status' => 0 + ]); + die(); + } + + // Obtained lock, now process the assignment conversion. + try { + $response = (object) [ 'status' => -1, 'filecount' => 0, 'pagecount' => 0, @@ -77,70 +92,76 @@ if ($action === 'pollconversions') { 'pages' => [], ]; - $combineddocument = document_services::get_combined_document_for_attempt($assignment, $userid, $attemptnumber); - $response->status = $combineddocument->get_status(); - $response->filecount = $combineddocument->get_document_count(); - - $readystatuslist = [combined_document::STATUS_READY, combined_document::STATUS_READY_PARTIAL]; - $completestatuslist = [combined_document::STATUS_COMPLETE, combined_document::STATUS_FAILED]; - - if (in_array($response->status, $readystatuslist)) { - // It seems that the files for this submission haven't been combined in cron yet. - // Try to combine them in the user session. - $combineddocument = document_services::get_combined_pdf_for_attempt($assignment, $userid, $attemptnumber); + $combineddocument = document_services::get_combined_document_for_attempt($assignment, $userid, $attemptnumber); $response->status = $combineddocument->get_status(); $response->filecount = $combineddocument->get_document_count(); - } - if (in_array($response->status, $completestatuslist)) { - $pages = document_services::get_page_images_for_attempt($assignment, - $userid, - $attemptnumber, - $readonly); + $readystatuslist = [combined_document::STATUS_READY, combined_document::STATUS_READY_PARTIAL]; + $completestatuslist = [combined_document::STATUS_COMPLETE, combined_document::STATUS_FAILED]; - $response->pagecount = $combineddocument->get_page_count(); - - $grade = $assignment->get_user_grade($userid, true, $attemptnumber); - - // The readonly files are stored in a different file area. - $filearea = document_services::PAGE_IMAGE_FILEAREA; - if ($readonly) { - $filearea = document_services::PAGE_IMAGE_READONLY_FILEAREA; + if (in_array($response->status, $readystatuslist)) { + // It seems that the files for this submission haven't been combined in cron yet. + // Try to combine them in the user session. + $combineddocument = document_services::get_combined_pdf_for_attempt($assignment, $userid, $attemptnumber); + $response->status = $combineddocument->get_status(); + $response->filecount = $combineddocument->get_document_count(); } - $response->partial = $combineddocument->is_partial_conversion(); - foreach ($pages as $id => $pagefile) { - $index = count($response->pages); - $page = new stdClass(); - $comments = page_editor::get_comments($grade->id, $index, $draft); - $page->url = moodle_url::make_pluginfile_url($context->id, - 'assignfeedback_editpdf', - $filearea, - $grade->id, - '/', - $pagefile->get_filename())->out(); - $page->comments = $comments; - if ($imageinfo = $pagefile->get_imageinfo()) { - $page->width = $imageinfo['width']; - $page->height = $imageinfo['height']; - } else { - $page->width = 0; - $page->height = 0; + if (in_array($response->status, $completestatuslist)) { + $pages = document_services::get_page_images_for_attempt($assignment, + $userid, + $attemptnumber, + $readonly); + + $response->pagecount = $combineddocument->get_page_count(); + + $grade = $assignment->get_user_grade($userid, true, $attemptnumber); + + // The readonly files are stored in a different file area. + $filearea = document_services::PAGE_IMAGE_FILEAREA; + if ($readonly) { + $filearea = document_services::PAGE_IMAGE_READONLY_FILEAREA; } - $annotations = page_editor::get_annotations($grade->id, $index, $draft); - $page->annotations = $annotations; - $response->pages[] = $page; - } + $response->partial = $combineddocument->is_partial_conversion(); - $component = 'assignfeedback_editpdf'; - $filearea = document_services::PAGE_IMAGE_FILEAREA; - $filepath = '/'; - $fs = get_file_storage(); - $files = $fs->get_directory_files($context->id, $component, $filearea, $grade->id, $filepath); - $response->pageready = count($files); + foreach ($pages as $id => $pagefile) { + $index = count($response->pages); + $page = new stdClass(); + $comments = page_editor::get_comments($grade->id, $index, $draft); + $page->url = moodle_url::make_pluginfile_url($context->id, + 'assignfeedback_editpdf', + $filearea, + $grade->id, + '/', + $pagefile->get_filename())->out(); + $page->comments = $comments; + if ($imageinfo = $pagefile->get_imageinfo()) { + $page->width = $imageinfo['width']; + $page->height = $imageinfo['height']; + } else { + $page->width = 0; + $page->height = 0; + } + $annotations = page_editor::get_annotations($grade->id, $index, $draft); + $page->annotations = $annotations; + $response->pages[] = $page; + } + + $component = 'assignfeedback_editpdf'; + $filearea = document_services::PAGE_IMAGE_FILEAREA; + $filepath = '/'; + $fs = get_file_storage(); + $files = $fs->get_directory_files($context->id, $component, $filearea, $grade->id, $filepath); + $response->pageready = count($files); + } + } catch (\Throwable $e) { + // Release lock, and re-throw exception. + $lock->release(); + throw $e; } echo json_encode($response); + $lock->release(); die(); } else if ($action == 'savepage') { require_capability('mod/assign:grade', $context);