MDL-71468 assignfeedback_editpdf: Convert submissions via adhoc tasks

config-dist.php

@@ -637,14 +637,6 @@ $CFG->admin = 'admin';
 //
 //      $CFG->upgradekey = 'put_some_password-like_value_here';
 //
-// Document conversion limit
-//
-// How many times the background task should attempt to convert a given attempt
-// before removing it from the queue. Currently this limit is only used by the
-// mod_assign conversion task.
-//
-//      $CFG->conversionattemptlimit = 3;
-//
 // Font used in exported PDF files. When generating a PDF, Moodle embeds a subset of
 // the font in the PDF file so it will be readable on the widest range of devices.
 // The default font is 'freesans' which is part of the GNU FreeFont collection.

lib/upgrade.txt

@@ -243,6 +243,8 @@ defined or can't be applied.
   user_can_edit_blocks() to return false where necessary. This makes it possible to remove block editing on a page
   from ALL users, including admins, where required on pages with multi region layouts exist, such as "My courses".
 * Add an early $CFG->session_redis_acquire_lock_warn option
+* Removed $CFG->conversionattemptlimit setting from config.php. assignfeedback_editpdf\task\convert_submissions task
+  is now replaced with adhoc tasks with standard fail delay approach.
 
 === 3.11.4 ===
 * A new option dontforcesvgdownload has been added to the $options parameter of the send_file() function.

mod/assign/feedback/editpdf/ajax.php

@@ -82,21 +82,11 @@ if ($action === 'pollconversions') {
     $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 by the
+        // It seems that the files for this submission haven't been combined in cron yet.
-        // "\assignfeedback_editpdf\task\convert_submissions" scheduled task.
         // 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();
-
-        // Check status of the combined document and remove the submission
-        // from the task queue if combination completed.
-        if (in_array($response->status, $completestatuslist)) {
-            $submission = $assignment->get_user_submission($userid, false, $attemptnumber);
-            if ($submission) {
-                $DB->delete_records('assignfeedback_editpdf_queue', array('submissionid' => $submission->id));
-            }
-        }
     }
 
     if (in_array($response->status, $completestatuslist)) {

mod/assign/feedback/editpdf/classes/event/observer.php

@@ -51,20 +51,12 @@ class observer {
      * @param \mod_assign\event\base $event The submission created/updated event.
      */
     protected static function queue_conversion($event) {
-        global $DB;
+        $data = [
-
+            'submissionid' => $event->other['submissionid'],
-        $submissionid = $event->other['submissionid'];
+            'submissionattempt' => $event->other['submissionattempt'],
-        $submissionattempt = $event->other['submissionattempt'];
+        ];
-        $fields = array( 'submissionid' => $submissionid, 'submissionattempt' => $submissionattempt);
+        $task = new \assignfeedback_editpdf\task\convert_submission;
-        $record = (object) $fields;
+        $task->set_custom_data($data);
-
+        \core\task\manager::queue_adhoc_task($task, true);
-        $exists = $DB->get_record('assignfeedback_editpdf_queue', $fields);
-        if (!$exists) {
-            $DB->insert_record('assignfeedback_editpdf_queue', $record);
-        } else {
-            // This submission attempt was already queued, so just reset the existing failure counter to ensure it gets processed.
-            $exists->attemptedconversions = 0;
-            $DB->update_record('assignfeedback_editpdf_queue', $exists);
-        }
     }
 }

mod/assign/feedback/editpdf/classes/privacy/provider.php

@@ -175,6 +175,5 @@ class provider implements
         $DB->delete_records_select('assignfeedback_editpdf_annot', "gradeid $sql", $params);
         $DB->delete_records_select('assignfeedback_editpdf_cmnt', "gradeid $sql", $params);
         $DB->delete_records_select('assignfeedback_editpdf_rot', "gradeid $sql", $params);
-        // Submission records in assignfeedback_editpdf_queue will be cleaned up in a scheduled task
     }
 }

mod/assign/feedback/editpdf/classes/task/convert_submission.php

@@ -0,0 +1,77 @@
+<?php
+// This file is part of Moodle - http://moodle.org/
+//
+// Moodle is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// Moodle is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with Moodle.  If not, see <http://www.gnu.org/licenses/>.
+
+namespace assignfeedback_editpdf\task;
+
+use core\task\adhoc_task;
+use assignfeedback_editpdf\document_services;
+use assignfeedback_editpdf\combined_document;
+use context_module;
+use assign;
+
+/**
+ * An adhoc task to convert submissions to pdf in the background.
+ *
+ * @copyright  2022 Mikhail Golenkov <mikhailgolenkov@catalyst-au.net>
+ * @package    assignfeedback_editpdf
+ * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+class convert_submission extends adhoc_task {
+
+    /**
+     * Run the task.
+     */
+    public function execute() {
+        global $CFG, $DB;
+        require_once($CFG->dirroot . '/mod/assign/locallib.php');
+
+        $data = $this->get_custom_data();
+
+        $submission = $DB->get_record('assign_submission', ['id' => $data->submissionid], '*', IGNORE_MISSING);
+        if (!$submission) {
+            mtrace('Submission no longer exists');
+            return;
+        }
+
+        $cm = get_coursemodule_from_instance('assign', $submission->assignment, 0, false, MUST_EXIST);
+        $context = context_module::instance($cm->id);
+        $assign = new assign($context, null, null);
+
+        if ($submission->userid) {
+            $users = [$submission->userid];
+        } else {
+            $users = [];
+            $members = $assign->get_submission_group_members($submission->groupid, true);
+            foreach ($members as $member) {
+                $users[] = $member->id;
+            }
+        }
+
+        foreach ($users as $userid) {
+            mtrace('Converting submission for user id ' . $userid);
+            $combineddocument = document_services::get_combined_pdf_for_attempt($assign, $userid, $data->submissionattempt);
+            $status = $combineddocument->get_status();
+            if ($status == combined_document::STATUS_COMPLETE) {
+                document_services::get_page_images_for_attempt($assign, $userid, $data->submissionattempt, false);
+                document_services::get_page_images_for_attempt($assign, $userid, $data->submissionattempt, true);
+
+                mtrace('The document has been successfully converted');
+            } else {
+                throw new \coding_exception('Document conversion completed with status ' . $status);
+            }
+        }
+    }
+}

mod/assign/feedback/editpdf/classes/task/convert_submissions.php

@@ -1,144 +0,0 @@
-<?php
-// This file is part of Moodle - http://moodle.org/
-//
-// Moodle is free software: you can redistribute it and/or modify
-// it under the terms of the GNU General Public License as published by
-// the Free Software Foundation, either version 3 of the License, or
-// (at your option) any later version.
-//
-// Moodle is distributed in the hope that it will be useful,
-// but WITHOUT ANY WARRANTY; without even the implied warranty of
-// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-// GNU General Public License for more details.
-//
-// You should have received a copy of the GNU General Public License
-// along with Moodle.  If not, see <http://www.gnu.org/licenses/>.
-
-/**
- * A scheduled task.
- *
- * @package    assignfeedback_editpdf
- * @copyright  2016 Damyon Wiese
- * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
- */
-namespace assignfeedback_editpdf\task;
-
-use core\task\scheduled_task;
-use assignfeedback_editpdf\document_services;
-use assignfeedback_editpdf\combined_document;
-use context_module;
-use assign;
-
-/**
- * Simple task to convert submissions to pdf in the background.
- * @copyright  2016 Damyon Wiese
- * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
- */
-class convert_submissions extends scheduled_task {
-
-    /**
-     * Get a descriptive name for this task (shown to admins).
-     *
-     * @return string
-     */
-    public function get_name() {
-        return get_string('preparesubmissionsforannotation', 'assignfeedback_editpdf');
-    }
-
-    /**
-     * Do the job.
-     * Throw exceptions on errors (the job will be retried).
-     */
-    public function execute() {
-        global $CFG, $DB;
-
-        require_once($CFG->dirroot . '/mod/assign/locallib.php');
-
-        // Conversion speed varies significantly and mostly depends on the documents content.
-        // We don't want the task to get stuck forever trying to process the whole queue in one go,
-        // so fetch 100 records only to make sure the task will be working for reasonable time.
-        // With the task's default schedule, 100 records per run means the task is capable to process
-        // 9600 conversions per day (100 * 4 * 24).
-        $records = $DB->get_records('assignfeedback_editpdf_queue', [], '', '*', 0, 100);
-
-        $assignmentcache = array();
-
-        $conversionattemptlimit = !empty($CFG->conversionattemptlimit) ? $CFG->conversionattemptlimit : 3;
-        foreach ($records as $record) {
-            $submissionid = $record->submissionid;
-            $submission = $DB->get_record('assign_submission', array('id' => $submissionid), '*', IGNORE_MISSING);
-            if (!$submission || $record->attemptedconversions >= $conversionattemptlimit) {
-                // Submission no longer exists; or we've exceeded the conversion attempt limit.
-                $DB->delete_records('assignfeedback_editpdf_queue', array('id' => $record->id));
-                continue;
-            }
-
-            // Record that we're attempting the conversion ahead of time.
-            // We can't do this afterwards as its possible for the conversion process to crash the script entirely.
-            $DB->set_field('assignfeedback_editpdf_queue', 'attemptedconversions',
-                    $record->attemptedconversions + 1, ['id' => $record->id]);
-
-            $assignmentid = $submission->assignment;
-            $attemptnumber = $record->submissionattempt;
-
-            if (empty($assignmentcache[$assignmentid])) {
-                $cm = get_coursemodule_from_instance('assign', $assignmentid, 0, false, MUST_EXIST);
-                $context = context_module::instance($cm->id);
-
-                $assignment = new assign($context, null, null);
-                $assignmentcache[$assignmentid] = $assignment;
-            } else {
-                $assignment = $assignmentcache[$assignmentid];
-            }
-
-            $users = array();
-            if ($submission->userid) {
-                array_push($users, $submission->userid);
-            } else {
-                $members = $assignment->get_submission_group_members($submission->groupid, true);
-
-                foreach ($members as $member) {
-                    array_push($users, $member->id);
-                }
-            }
-
-            mtrace('Convert ' . count($users) . ' submission attempt(s) for assignment ' . $assignmentid);
-            $conversionrequirespolling = false;
-
-            foreach ($users as $userid) {
-                try {
-                    $combineddocument = document_services::get_combined_pdf_for_attempt($assignment, $userid, $attemptnumber);
-                    switch ($combineddocument->get_status()) {
-                        case combined_document::STATUS_READY:
-                        case combined_document::STATUS_READY_PARTIAL:
-                        case combined_document::STATUS_PENDING_INPUT:
-                            // The document has not been converted yet or is somehow still ready.
-                            $conversionrequirespolling = true;
-                            continue 2;
-                    }
-                    document_services::get_page_images_for_attempt(
-                            $assignment,
-                            $userid,
-                            $attemptnumber,
-                            false
-                        );
-                    document_services::get_page_images_for_attempt(
-                            $assignment,
-                            $userid,
-                            $attemptnumber,
-                            true
-                        );
-                } catch (\moodle_exception $e) {
-                    mtrace('Conversion failed with error:' . $e->errorcode);
-                }
-            }
-
-            // Remove from queue.
-            if (!$conversionrequirespolling) {
-                $DB->delete_records('assignfeedback_editpdf_queue', array('id' => $record->id));
-            }
-
-        }
-    }
-
-}

mod/assign/feedback/editpdf/db/install.xml

@@ -59,18 +59,6 @@
         <KEY NAME="userid" TYPE="foreign" FIELDS="userid" REFTABLE="user" REFFIELDS="id"/>
       </KEYS>
     </TABLE>
-    <TABLE NAME="assignfeedback_editpdf_queue" COMMENT="Queue for processing.">
-      <FIELDS>
-        <FIELD NAME="id" TYPE="int" LENGTH="10" NOTNULL="true" SEQUENCE="true"/>
-        <FIELD NAME="submissionid" TYPE="int" LENGTH="10" NOTNULL="true" SEQUENCE="false"/>
-        <FIELD NAME="submissionattempt" TYPE="int" LENGTH="10" NOTNULL="true" SEQUENCE="false"/>
-        <FIELD NAME="attemptedconversions" TYPE="int" LENGTH="10" NOTNULL="true" DEFAULT="0" SEQUENCE="false"/>
-      </FIELDS>
-      <KEYS>
-        <KEY NAME="primary" TYPE="primary" FIELDS="id"/>
-        <KEY NAME="submissionid-submissionattempt" TYPE="unique" FIELDS="submissionid, submissionattempt"/>
-      </KEYS>
-    </TABLE>
     <TABLE NAME="assignfeedback_editpdf_rot" COMMENT="Stores rotation information of a page.">
       <FIELDS>
         <FIELD NAME="id" TYPE="int" LENGTH="10" NOTNULL="true" SEQUENCE="true"/>

mod/assign/feedback/editpdf/db/tasks.php

@@ -1,40 +0,0 @@
-<?php
-// This file is part of Moodle - http://moodle.org/
-//
-// Moodle is free software: you can redistribute it and/or modify
-// it under the terms of the GNU General Public License as published by
-// the Free Software Foundation, either version 3 of the License, or
-// (at your option) any later version.
-//
-// Moodle is distributed in the hope that it will be useful,
-// but WITHOUT ANY WARRANTY; without even the implied warranty of
-// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-// GNU General Public License for more details.
-//
-// You should have received a copy of the GNU General Public License
-// along with Moodle.  If not, see <http://www.gnu.org/licenses/>.
-
-/**
- * Definition of editpdf scheduled tasks.
- *
- * @package   assignfeedback_editpdf
- * @category  task
- * @copyright 2016 Damyon Wiese
- * @license   http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
- */
-
-defined('MOODLE_INTERNAL') || die();
-
-/* List of handlers */
-
-$tasks = array(
-    array(
-        'classname' => 'assignfeedback_editpdf\task\convert_submissions',
-        'blocking' => 0,
-        'minute' => '*/15',
-        'hour' => '*',
-        'day' => '*',
-        'dayofweek' => '*',
-        'month' => '*'
-    ),
-);

mod/assign/feedback/editpdf/db/upgrade.php

@@ -95,5 +95,29 @@ function xmldb_assignfeedback_editpdf_upgrade($oldversion) {
     // Automatically generated Moodle v4.0.0 release upgrade line.
     // Put any upgrade step following this.
 
+    if ($oldversion < 2022061000) {
+        $table = new xmldb_table('assignfeedback_editpdf_queue');
+        if ($dbman->table_exists($table)) {
+            // Convert not yet converted submissions into adhoc tasks.
+            $rs = $DB->get_recordset('assignfeedback_editpdf_queue');
+            foreach ($rs as $record) {
+                $data = [
+                    'submissionid' => $record->submissionid,
+                    'submissionattempt' => $record->submissionattempt,
+                ];
+                $task = new assignfeedback_editpdf\task\convert_submission;
+                $task->set_custom_data($data);
+                \core\task\manager::queue_adhoc_task($task, true);
+            }
+            $rs->close();
+
+            // Drop the table.
+            $dbman->drop_table($table);
+        }
+
+        // Editpdf savepoint reached.
+        upgrade_plugin_savepoint(true, 2022061000, 'assignfeedback', 'editpdf');
+    }
+
     return true;
 }

mod/assign/feedback/editpdf/tests/feedback_test.php

@@ -329,14 +329,16 @@ class feedback_test extends \advanced_testcase {
         $this->assertEmpty($file3);
     }
 
+    /**
+     * Test Convert submission ad-hoc task.
+     *
+     * @covers \assignfeedback_editpdf\task\convert_submission
+     */
     public function test_conversion_task() {
-        global $DB;
         $this->require_ghostscript();
         $this->resetAfterTest();
         cron_setup_user();
 
-        $task = new \assignfeedback_editpdf\task\convert_submissions;
-
         $course = $this->getDataGenerator()->create_course();
         $student = $this->getDataGenerator()->create_and_enrol($course, 'student');
         $assignopts = [
@@ -351,39 +353,35 @@ class feedback_test extends \advanced_testcase {
         $this->add_file_submission($student, $assign);
 
         // Run the conversion task.
+        $task = \core\task\manager::get_next_adhoc_task(time());
         ob_start();
         $task->execute();
+        \core\task\manager::adhoc_task_complete($task);
         $output = ob_get_clean();
 
-        // Verify it acted on both submissions in the queue.
+        // Confirm, that submission has been converted and the task queue is now empty.
-        $this->assertStringContainsString("Convert 1 submission attempt(s) for assignment {$assign->get_instance()->id}", $output);
+        $this->assertStringContainsString('Converting submission for user id ' . $student->id, $output);
-        $this->assertEquals(0, $DB->count_records('assignfeedback_editpdf_queue'));
+        $this->assertStringContainsString('The document has been successfully converted', $output);
-
+        $this->assertNull(\core\task\manager::get_next_adhoc_task(time()));
-        // Set a known limit.
-        set_config('conversionattemptlimit', 3);
 
         // Trigger a re-queue by 'updating' a submission.
         $submission = $assign->get_user_submission($student->id, true);
         $plugin = $assign->get_submission_plugin_by_type('file');
         $plugin->save($submission, (new \stdClass));
 
+        $task = \core\task\manager::get_next_adhoc_task(time());
         // Verify that queued a conversion task.
-        $this->assertEquals(1, $DB->count_records('assignfeedback_editpdf_queue'));
+        $this->assertNotNull($task);
-
-        // Fake some failed attempts for it.
-        $queuerecord = $DB->get_record('assignfeedback_editpdf_queue', ['submissionid' => $submission->id]);
-        $queuerecord->attemptedconversions = 3;
-        $DB->update_record('assignfeedback_editpdf_queue', $queuerecord);
 
         ob_start();
         $task->execute();
+        \core\task\manager::adhoc_task_complete($task);
         $output = ob_get_clean();
 
-        // Verify that the cron task skipped the submission.
+        // Confirm, that submission has been converted and the task queue is now empty.
-        $this->assertStringNotContainsString("Convert 1 submission attempt(s) for assignment {$assign->get_instance()->id}", $output);
+        $this->assertStringContainsString('Converting submission for user id ' . $student->id, $output);
-        // And it removed it from the queue.
+        $this->assertStringContainsString('The document has been successfully converted', $output);
-        $this->assertEquals(0, $DB->count_records('assignfeedback_editpdf_queue'));
+        $this->assertNull(\core\task\manager::get_next_adhoc_task(time()));
-
     }
 
     /**
@@ -517,41 +515,4 @@ class feedback_test extends \advanced_testcase {
         // No modification.
         $this->assertFalse($plugin->is_feedback_modified($grade, $data));
     }
-
-    /**
-     * Test Convert submissions scheduled task limit.
-     *
-     * @covers \assignfeedback_editpdf\task\convert_submissions
-     */
-    public function test_conversion_task_limit() {
-        global $DB;
-        $this->require_ghostscript();
-        $this->resetAfterTest();
-        cron_setup_user();
-
-        $course = $this->getDataGenerator()->create_course();
-        $assignopts = [
-            'assignsubmission_file_enabled' => 1,
-            'assignsubmission_file_maxfiles' => 1,
-            'assignfeedback_editpdf_enabled' => 1,
-            'assignsubmission_file_maxsizebytes' => 1000000,
-        ];
-        $assign = $this->create_instance($course, $assignopts);
-
-        // Generate 110 submissions.
-        for ($i = 0; $i < 110; $i++) {
-            $student = $this->getDataGenerator()->create_and_enrol($course, 'student');
-            $this->add_file_submission($student, $assign);
-        }
-        $this->assertEquals(110, $DB->count_records('assignfeedback_editpdf_queue'));
-
-        // Run the conversion task.
-        $task = new \assignfeedback_editpdf\task\convert_submissions;
-        ob_start();
-        $task->execute();
-        ob_end_clean();
-
-        // Confirm, that 100 records were processed and 10 were left for the next task run.
-        $this->assertEquals(10, $DB->count_records('assignfeedback_editpdf_queue'));
-    }
 }

mod/assign/feedback/editpdf/version.php

@@ -24,6 +24,6 @@
 
 defined('MOODLE_INTERNAL') || die();
 
-$plugin->version   = 2022041900;
+$plugin->version   = 2022061000;
 $plugin->requires  = 2022041200;
 $plugin->component = 'assignfeedback_editpdf';