From 702123a1dd250734fa1e6e33e5b15ce4554e72b7 Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Mon, 3 Dec 2018 15:04:46 +0800 Subject: [PATCH 1/6] MDL-49399 core: Add ability to specify a header col --- lib/tablelib.php | 33 +++++++++++++++++++++++++++++---- 1 file changed, 29 insertions(+), 4 deletions(-) diff --git a/lib/tablelib.php b/lib/tablelib.php index 4405bc60b5b..d1213b89b47 100644 --- a/lib/tablelib.php +++ b/lib/tablelib.php @@ -57,6 +57,11 @@ class flexible_table { var $attributes = array(); var $headers = array(); + /** + * @var string A column which should be considered as a header column. + */ + protected $headercolumn = null; + /** * @var string For create header with help icon. */ @@ -429,6 +434,17 @@ class flexible_table { $this->headers = $headers; } + /** + * Mark a specific column as being a table header using the column name defined in define_columns. + * + * Note: Only one column can be a header, and it will be rendered using a th tag. + * + * @param string $column + */ + public function define_header_column(string $column) { + $this->headercolumn = $column; + } + /** * Defines a help icon for the header * @@ -1098,6 +1114,18 @@ class flexible_table { foreach ($row as $index => $data) { $column = $colbyindex[$index]; + $attributes = [ + 'class' => "cell c{$index}" . $this->column_class[$column], + 'id' => "{$rowid}_c{$index}", + 'style' => $this->make_styles_string($this->column_style[$column]), + ]; + + $celltype = 'td'; + if ($this->headercolumn && $column == $this->headercolumn) { + $celltype = 'th'; + $attributes['scope'] = 'row'; + } + if (empty($this->prefs['collapse'][$column])) { if ($this->column_suppress[$column] && $suppress_lastrow !== NULL && $suppress_lastrow[$index] === $data) { $content = ' '; @@ -1108,10 +1136,7 @@ class flexible_table { $content = ' '; } - $html .= html_writer::tag('td', $content, array( - 'class' => 'cell c' . $index . $this->column_class[$column], - 'id' => $rowid . '_c' . $index, - 'style' => $this->make_styles_string($this->column_style[$column]))); + $html .= html_writer::tag($celltype, $content, $attributes); } } From af540d426def658e30691aaa44cf55d23493497a Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Tue, 4 Dec 2018 10:38:59 +0800 Subject: [PATCH 2/6] MDL-49399 task: Add task log table --- lang/en/moodle.php | 3 +++ lib/classes/privacy/provider.php | 7 +++++++ lib/db/install.xml | 23 ++++++++++++++++++++++- lib/db/upgrade.php | 32 ++++++++++++++++++++++++++++++++ version.php | 2 +- 5 files changed, 65 insertions(+), 2 deletions(-) diff --git a/lang/en/moodle.php b/lang/en/moodle.php index 5fe9a09cdcb..9e42a4336b9 100644 --- a/lang/en/moodle.php +++ b/lang/en/moodle.php @@ -1588,6 +1588,9 @@ $string['privacy:metadata:task_adhoc'] = 'The status of adhoc tasks.'; $string['privacy:metadata:task_adhoc:component'] = 'The component owning the task.'; $string['privacy:metadata:task_adhoc:nextruntime'] = 'The earliest time to run this task.'; $string['privacy:metadata:task_adhoc:userid'] = 'The user to run the task as.'; +$string['privacy:metadata:task_log'] = 'Log output for a log'; +$string['privacy:metadata:task_log:component'] = 'The component owning the task.'; +$string['privacy:metadata:task_log:userid'] = 'The user that the task belonged to.'; $string['privacy:metadata:upgrade_log'] = 'The upgrade log.'; $string['privacy:metadata:upgrade_log:backtrace'] = 'Any backtrace associated with this upgrade step.'; $string['privacy:metadata:upgrade_log:details'] = 'Extra information relating to the upgrade.'; diff --git a/lib/classes/privacy/provider.php b/lib/classes/privacy/provider.php index b9d1d670a81..bc9b8f8cf78 100644 --- a/lib/classes/privacy/provider.php +++ b/lib/classes/privacy/provider.php @@ -88,6 +88,13 @@ class provider implements 'userid' => 'privacy:metadata:task_adhoc:userid', ], 'privacy:metadata:task_adhoc'); + // The task_log table stores debugging data for tasks. + // These are cleaned regularly and intended purely for debugging. + $collection->add_database_table('task_log', [ + 'component' => 'privacy:metadata:task_log:component', + 'userid' => 'privacy:metadata:task_log:userid', + ], 'privacy:metadata:task_log'); + // The events_queue includes information about pending events tasks. // These are stored for short periods whilst being processed into other locations. $collection->add_database_table('events_queue', [ diff --git a/lib/db/install.xml b/lib/db/install.xml index a456c84e711..2c57cdaa3da 100644 --- a/lib/db/install.xml +++ b/lib/db/install.xml @@ -1,5 +1,5 @@ - @@ -3305,6 +3305,27 @@ + + + + + + + + + + + + + + + + + + + + +
diff --git a/lib/db/upgrade.php b/lib/db/upgrade.php index 5e7324d29a9..80dc8ec4c38 100644 --- a/lib/db/upgrade.php +++ b/lib/db/upgrade.php @@ -2516,5 +2516,37 @@ function xmldb_main_upgrade($oldversion) { upgrade_main_savepoint(true, 2018120301.02); } + if ($oldversion < 2019011500.00) { + // Define table task_log to be created. + $table = new xmldb_table('task_log'); + + // Adding fields to table task_log. + $table->add_field('id', XMLDB_TYPE_INTEGER, '10', null, XMLDB_NOTNULL, XMLDB_SEQUENCE, null); + $table->add_field('type', XMLDB_TYPE_INTEGER, '4', null, XMLDB_NOTNULL, null, null); + $table->add_field('component', XMLDB_TYPE_CHAR, '255', null, XMLDB_NOTNULL, null, null); + $table->add_field('classname', XMLDB_TYPE_CHAR, '255', null, XMLDB_NOTNULL, null, null); + $table->add_field('userid', XMLDB_TYPE_INTEGER, '10', null, XMLDB_NOTNULL, null, null); + $table->add_field('timestart', XMLDB_TYPE_NUMBER, '20, 10', null, XMLDB_NOTNULL, null, null); + $table->add_field('timeend', XMLDB_TYPE_NUMBER, '20, 10', null, XMLDB_NOTNULL, null, null); + $table->add_field('dbreads', XMLDB_TYPE_INTEGER, '10', null, XMLDB_NOTNULL, null, null); + $table->add_field('dbwrites', XMLDB_TYPE_INTEGER, '10', null, XMLDB_NOTNULL, null, null); + $table->add_field('result', XMLDB_TYPE_INTEGER, '2', null, XMLDB_NOTNULL, null, null); + + // Adding keys to table task_log. + $table->add_key('primary', XMLDB_KEY_PRIMARY, ['id']); + + // Adding indexes to table task_log. + $table->add_index('classname', XMLDB_INDEX_NOTUNIQUE, ['classname']); + $table->add_index('timestart', XMLDB_INDEX_NOTUNIQUE, ['timestart']); + + // Conditionally launch create table for task_log. + if (!$dbman->table_exists($table)) { + $dbman->create_table($table); + } + + // Main savepoint reached. + upgrade_main_savepoint(true, 2019011500.00); + } + return true; } diff --git a/version.php b/version.php index d0e192f9a23..be82e4bb506 100644 --- a/version.php +++ b/version.php @@ -29,7 +29,7 @@ defined('MOODLE_INTERNAL') || die(); -$version = 2019011100.00; // YYYYMMDD = weekly release date of this DEV branch. +$version = 2019011500.00; // YYYYMMDD = weekly release date of this DEV branch. // RR = release increments - 00 in DEV branches. // .XX = incremental changes. From 4b71596fc9552730053101a8deb21d4c3281b016 Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Tue, 4 Dec 2018 10:39:56 +0800 Subject: [PATCH 3/6] MDL-49399 task: Add task logging API --- admin/settings/server.php | 40 +- admin/tool/task/cli/schedule_task.php | 1 + config-dist.php | 9 + lang/en/admin.php | 13 + lib/classes/task/database_logger.php | 144 ++++++ lib/classes/task/logmanager.php | 334 ++++++++++++ lib/classes/task/manager.php | 12 + lib/classes/task/task_log_cleanup_task.php | 53 ++ lib/classes/task/task_logger.php | 57 +++ lib/cronlib.php | 5 + lib/db/install.xml | 3 +- lib/db/tasks.php | 9 + lib/db/upgrade.php | 14 + lib/moodlelib.php | 5 + lib/tests/task_database_logger_test.php | 505 +++++++++++++++++++ lib/tests/task_logging_test.php | 558 +++++++++++++++++++++ version.php | 2 +- 17 files changed, 1761 insertions(+), 3 deletions(-) create mode 100644 lib/classes/task/database_logger.php create mode 100644 lib/classes/task/logmanager.php create mode 100644 lib/classes/task/task_log_cleanup_task.php create mode 100644 lib/classes/task/task_logger.php create mode 100644 lib/tests/task_database_logger_test.php create mode 100644 lib/tests/task_logging_test.php diff --git a/admin/settings/server.php b/admin/settings/server.php index f4c24c0f3f0..73efce12e45 100644 --- a/admin/settings/server.php +++ b/admin/settings/server.php @@ -4,7 +4,6 @@ if ($hassiteconfig) { // speedup for non-admins, add all caps used on this page - // "systempaths" settingpage $temp = new admin_settingpage('systempaths', new lang_string('systempaths','admin')); $temp->add(new admin_setting_configexecutable('pathtophp', new lang_string('pathtophp', 'admin'), @@ -212,6 +211,45 @@ $temp->add(new admin_setting_configtext('curltimeoutkbitrate', new lang_string(' $ADMIN->add('server', $temp); +$ADMIN->add('server', new admin_category('taskconfig', new lang_string('taskadmintitle', 'admin'))); +$temp = new admin_settingpage('tasklogging', new lang_string('tasklogging','admin')); +$temp->add( + new admin_setting_configselect( + 'task_logmode', + new lang_string('task_logmode', 'admin'), + new lang_string('task_logmode_desc', 'admin'), + \core\task\logmanager::MODE_ALL, + [ + \core\task\logmanager::MODE_ALL => new lang_string('task_logmode_all', 'admin'), + \core\task\logmanager::MODE_FAILONLY => new lang_string('task_logmode_failonly', 'admin'), + \core\task\logmanager::MODE_NONE => new lang_string('task_logmode_none', 'admin'), + ] + ) +); + +if (\core\task\logmanager::uses_standard_settings()) { + $temp->add( + new admin_setting_configduration( + 'task_logretention', + new \lang_string('task_logretention', 'admin'), + new \lang_string('task_logretention_desc', 'admin'), + 28 * DAYSECS + ) + ); + + $temp->add( + new admin_setting_configtext( + 'task_logretainruns', + new \lang_string('task_logretainruns', 'admin'), + new \lang_string('task_logretainruns_desc', 'admin'), + 20, + PARAM_INT + ) + ); + +} +$ADMIN->add('taskconfig', $temp); + // E-mail settings. $ADMIN->add('server', new admin_category('email', new lang_string('categoryemail', 'admin'))); diff --git a/admin/tool/task/cli/schedule_task.php b/admin/tool/task/cli/schedule_task.php index 0de826c1655..cf03700bd4f 100644 --- a/admin/tool/task/cli/schedule_task.php +++ b/admin/tool/task/cli/schedule_task.php @@ -129,6 +129,7 @@ if ($execute = $options['execute']) { $predbqueries = $DB->perf_get_queries(); $pretime = microtime(true); + \core\task\logmanager::start_logging($task); $fullname = $task->get_name() . ' (' . get_class($task) . ')'; mtrace('Execute scheduled task: ' . $fullname); // NOTE: it would be tricky to move this code to \core\task\manager class, diff --git a/config-dist.php b/config-dist.php index 18cbe311524..29f9b799d76 100644 --- a/config-dist.php +++ b/config-dist.php @@ -545,6 +545,15 @@ $CFG->admin = 'admin'; // on a shared file system that supports locking. // $CFG->lock_file_root = $CFG->dataroot . '/lock'; // +// +// Alternative task logging. +// Since Moodle 3.7 the output of al scheduled and adhoc tasks is stored in the database and it is possible to use an +// alternative task logging mechanism. +// To set the alternative task logging mechanism in config.php you can use the following settings, providing the +// alternative class name that will be auto-loaded. +// +// $CFG->task_log_class = '\\local_mytasklogger\\logger'; +// // Moodle 2.9 allows administrators to customise the list of supported file types. // To add a new filetype or override the definition of an existing one, set the // customfiletypes variable like this: diff --git a/lang/en/admin.php b/lang/en/admin.php index c2aad42f376..86f77cd08a2 100644 --- a/lang/en/admin.php +++ b/lang/en/admin.php @@ -1164,6 +1164,16 @@ $string['tablesnosave'] = 'Changes in tables above are saved automatically.'; $string['tabselectedtofront'] = 'On tables with tabs, should the row with the currently selected tab be placed at the front'; $string['tabselectedtofronttext'] = 'Bring selected tab row to front'; $string['testsiteupgradewarning'] = 'You are currently using the {$a} test site, to upgrade it properly use the command line interface tool'; +$string['task_logmode'] = 'When to log'; +$string['task_logmode_desc'] = 'You can choose when you wish task logging to take place. By default logs are always captured. You can disable logging entirely, or change to only log tasks which fail.'; +$string['task_logmode_none'] = 'Do not log anything'; +$string['task_logmode_all'] = 'Store the log output of all jobs'; +$string['task_logmode_failonly'] = 'Only store logs for jobs which fail'; +$string['task_logretention'] = 'Retention period'; +$string['task_logretention_desc'] = 'The maximum period that logs should be kept for. This setting interacts with the \'Retain runs\' setting: whichever is reached first will apply'; +$string['task_logretainruns'] = 'Retain runs'; +$string['task_logretainruns_desc'] = 'The number of runs of each task to retain. This setting interacts with the \'Retention period\' setting: whichever is reached first will apply.'; +$string['taskadmintitle'] = 'Tasks'; $string['taskanalyticscleanup'] = 'Analytics cleanup'; $string['taskautomatedbackup'] = 'Automated backups'; $string['taskbackupcleanup'] = 'Clean backup tables and logs'; @@ -1176,6 +1186,7 @@ $string['taskcheckforupdates'] = 'Check for updates'; $string['taskcompletionregular'] = 'Calculate regular completion data'; $string['taskcompletiondaily'] = 'Completion mark as started'; $string['taskcontextcleanup'] = 'Cleanup contexts'; +$string['tasklogging'] = 'Task log configuration'; $string['taskcreatecontexts'] = 'Create missing contexts'; $string['taskdeletecachetext'] = 'Delete old text cache records'; $string['taskdeleteincompleteusers'] = 'Delete incomplete users'; @@ -1186,6 +1197,8 @@ $string['taskglobalsearchindex'] = 'Global search indexing'; $string['taskglobalsearchoptimize'] = 'Global search index optimization'; $string['taskgradecron'] = 'Background processing for gradebook'; $string['tasklegacycron'] = 'Legacy cron processing for plugins'; +$string['tasklogcleanup'] = 'Cleanup of task logs'; +$string['tasklogs'] = 'Task logs'; $string['taskmessagingcleanup'] = 'Background processing for messaging'; $string['taskpasswordresetcleanup'] = 'Cleanup password reset attempts'; $string['taskplagiarismcron'] = 'Background processing for legacy cron in plagiarism plugins'; diff --git a/lib/classes/task/database_logger.php b/lib/classes/task/database_logger.php new file mode 100644 index 00000000000..253e0d2645f --- /dev/null +++ b/lib/classes/task/database_logger.php @@ -0,0 +1,144 @@ +. + +/** + * Database logger for task logging. + * + * @package core + * @copyright 2018 Andrew Nicols + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +namespace core\task; + +defined('MOODLE_INTERNAL') || die(); + +/** + * Database logger for task logging. + * + * @copyright 2018 Andrew Nicols + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class database_logger implements task_logger { + + /** @var int Type constant for a scheduled task */ + const TYPE_SCHEDULED = 0; + + /** @var int Type constant for an adhoc task */ + const TYPE_ADHOC = 1; + + /** + * Whether the task is configured and ready to log. + * + * @return bool + */ + public static function is_configured() : bool { + return true; + } + + /** + * Store the log for the specified task. + * + * @param task_base $task The task that the log belongs to. + * @param string $logpath The path to the log on disk + * @param bool $failed Whether the task failed + * @param int $dbreads The number of DB reads + * @param int $dbwrites The number of DB writes + * @param float $timestart The start time of the task + * @param float $timeend The end time of the task + */ + public static function store_log_for_task(task_base $task, string $logpath, bool $failed, + int $dbreads, int $dbwrites, float $timestart, float $timeend) { + global $DB; + + // Write this log to the database. + $logdata = (object) [ + 'type' => is_a($task, scheduled_task::class) ? self::TYPE_SCHEDULED : self::TYPE_ADHOC, + 'component' => $task->get_component(), + 'classname' => get_class($task), + 'userid' => 0, + 'timestart' => $timestart, + 'timeend' => $timeend, + 'dbreads' => $dbreads, + 'dbwrites' => $dbwrites, + 'result' => (int) $failed, + 'output' => file_get_contents($logpath), + ]; + + if (is_a($task, adhoc_task::class) && $userid = $task->get_userid()) { + $logdata->userid = $userid; + } + + $logdata->id = $DB->insert_record('task_log', $logdata); + } + + /** + * Cleanup old task logs. + */ + public static function cleanup() { + global $CFG, $DB; + + // Delete logs older than the retention period. + $params = [ + 'retentionperiod' => time() - $CFG->task_logretention, + ]; + $logids = $DB->get_fieldset_select('task_log', 'id', 'timestart < :retentionperiod', $params); + self::delete_task_logs($logids); + + // Delete logs to retain a minimum number of logs. + $sql = "SELECT classname FROM {task_log} GROUP BY classname HAVING COUNT(classname) > :retaincount"; + $params = [ + 'retaincount' => $CFG->task_logretainruns, + ]; + $classes = $DB->get_fieldset_sql($sql, $params); + + foreach ($classes as $classname) { + $params = [ + 'classname' => $classname, + ]; + + $retaincount = (int) $CFG->task_logretainruns; + $keeplogs = $DB->get_fieldset_sql( + "SELECT id FROM {task_log} WHERE classname = :classname ORDER BY timestart DESC LIMIT {$retaincount}", + $params + ); + + $notinsql = ""; + if ($keeplogs) { + list($notinsql, $params) = $DB->get_in_or_equal($keeplogs, SQL_PARAMS_NAMED, 'p', false); + $params['classname'] = $classname; + $notinsql = " AND id {$notinsql}"; + } + + $logids = $DB->get_fieldset_select('task_log', 'id', "classname = :classname {$notinsql}", $params); + self::delete_task_logs($logids); + } + } + + /** + * Delete task logs for the specified logs. + * + * @param array $logids + */ + public static function delete_task_logs(array $logids) { + global $DB; + + if (empty($logids)) { + return; + } + + $DB->delete_records_list('task_log', 'id', $logids); + } +} diff --git a/lib/classes/task/logmanager.php b/lib/classes/task/logmanager.php new file mode 100644 index 00000000000..6e2e6820abd --- /dev/null +++ b/lib/classes/task/logmanager.php @@ -0,0 +1,334 @@ +. + +/** + * Task log manager. + * + * @package core + * @category task + * @copyright 2018 Andrew Nicols + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +namespace core\task; + +defined('MOODLE_INTERNAL') || die(); + +/** + * Task log manager. + * + * @copyright 2018 Andrew Nicols + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class logmanager { + + /** @var int Do not log anything */ + const MODE_NONE = 0; + + /** @var int Log all tasks */ + const MODE_ALL = 1; + + /** @var int Only log fails */ + const MODE_FAILONLY = 2; + + /** @var int The default chunksize to use in ob_start */ + const CHUNKSIZE = 1; + + /** + * @var \core\task\task_base The task being logged. + */ + protected static $task = null; + + /** + * @var \stdClass Metadata about the current log + */ + protected static $taskloginfo = null; + + /** + * @var \resource The current filehandle used for logging + */ + protected static $fh = null; + + /** + * @var string The path to the log file + */ + protected static $logpath = null; + + /** + * @var bool Whether the task logger has been registered with the shutdown handler + */ + protected static $tasklogregistered = false; + + /** + * @var int The level of output buffering in place before starting. + */ + protected static $oblevel = null; + + /** + * Create a new task logger for the specified task, and prepare for logging. + * + * @param \core\task\task_base $task The task being run + */ + public static function start_logging(task_base $task) { + global $DB; + + if (!self::should_log()) { + return; + } + + // We register a shutdown handler to ensure that logs causing any failures are correctly disposed of. + // Note: This must happen before the per-request directory is requested because the shutdown handler may delete + // the logfile. + if (!self::$tasklogregistered) { + \core_shutdown_manager::register_function(function() { + // These will only actually do anything if capturing is current active when the thread ended, which + // constitutes a failure. + \core\task\logmanager::finalise_log(true); + }); + + self::$tasklogregistered = true; + } + + if (self::is_current_output_buffer()) { + // We cannot capture when we are already capturing. + throw new \coding_exception('Logging is already in progress for task "' . get_class(self::$task) . '". ' . + 'Nested logging is not supported.'); + } + + // Store the initial data about the task and current state. + self::$task = $task; + self::$taskloginfo = (object) [ + 'dbread' => $DB->perf_get_reads(), + 'dbwrite' => $DB->perf_get_writes(), + 'timestart' => microtime(true), + ]; + + // For simplicity's sake we always store logs on disk and flush at the end. + self::$logpath = make_request_directory() . DIRECTORY_SEPARATOR . "task.log"; + self::$fh = fopen(self::$logpath, 'w+'); + + // Note the level of the current output buffer. + // Note: You cannot use ob_get_level() as it will return `1` when the default output buffer is enabled. + if ($obstatus = ob_get_status()) { + self::$oblevel = $obstatus['level']; + } else { + self::$oblevel = null; + } + + // Start capturing output. + ob_start([\core\task\logmanager::class, 'add_line'], self::CHUNKSIZE); + } + + /** + * Whether logging is possible and should be happening. + * + * @return bool + */ + protected static function should_log() : bool { + global $CFG; + + // Respect the config setting. + if (isset($CFG->task_logmode) && empty($CFG->task_logmode)) { + return false; + } + + return !empty(self::get_logger_classname()); + } + + /** + * Return the name of the logging class to use. + * + * @return string + */ + public static function get_logger_classname() : string { + global $CFG; + + if (!empty($CFG->task_log_class)) { + // Configuration is present to use an alternative task logging class. + return $CFG->task_log_class; + } + + // Fall back on the default database logger. + return database_logger::class; + } + + /** + * Whether this task logger has a report available. + * + * @return bool + */ + public static function has_log_report() : bool { + $loggerclass = self::get_logger_classname(); + + return $loggerclass::has_log_report(); + } + + /** + * Whether to use the standard settings fore + */ + public static function uses_standard_settings() : bool { + $classname = self::get_logger_classname(); + if (!class_exists($classname)) { + return false; + } + + if (is_a($classname, database_logger::class, true)) { + return true; + } + + return false; + } + + /** + * Get any URL available for viewing relevant task log reports. + * + * @param string $classname The task class to fetch for + * @return \moodle_url + */ + public static function get_url_for_task_class(string $classname) : \moodle_url { + $loggerclass = self::get_logger_classname(); + + return $loggerclass::get_url_for_task_class($classname); + } + + /** + * Whether we are the current log collector. + * + * @return bool + */ + protected static function is_current_output_buffer() : bool { + if (empty(self::$taskloginfo)) { + return false; + } + + if ($ob = ob_get_status()) { + return 'core\\task\\logmanager::add_line' == $ob['name']; + } + + return false; + } + + /** + * Whether we are capturing at all. + * + * @return bool + */ + protected static function is_capturing() : bool { + $buffers = ob_get_status(true); + foreach ($buffers as $ob) { + if ('core\\task\\logmanager::add_line' == $ob['name']) { + return true; + } + } + + return false; + } + + /** + * Finish writing for the current task. + * + * @param bool $failed + */ + public static function finalise_log(bool $failed = false) { + global $CFG, $DB, $PERF; + + if (!self::should_log()) { + return; + } + + if (!self::is_capturing()) { + // Not capturing anything. + return; + } + + // Ensure that all logs are closed. + $buffers = ob_get_status(true); + foreach (array_reverse($buffers) as $ob) { + if (null !== self::$oblevel) { + if ($ob['level'] <= self::$oblevel) { + // Only close as far as the initial output buffer level. + break; + } + } + + // End and flush this buffer. + ob_end_flush(); + + if ('core\\task\\logmanager::add_line' == $ob['name']) { + break; + } + } + self::$oblevel = null; + + // Flush any remaining buffer. + self::flush(); + + // Close and unset the FH. + fclose(self::$fh); + self::$fh = null; + + if ($failed || empty($CFG->task_logmode) || self::MODE_ALL == $CFG->task_logmode) { + // Finalise the log. + $loggerclass = self::get_logger_classname(); + $loggerclass::store_log_for_task( + self::$task, + self::$logpath, + $failed, + $DB->perf_get_reads() - self::$taskloginfo->dbread, + $DB->perf_get_writes() - self::$taskloginfo->dbwrite - $PERF->logwrites, + self::$taskloginfo->timestart, + microtime(true) + ); + } + + // Tidy up. + self::$logpath = null; + self::$taskloginfo = null; + } + + /** + * Flush the current output buffer. + * + * This function will ensure that we are the current output buffer handler. + */ + public static function flush() { + // We only call ob_flush if the current output buffer belongs to us. + if (self::is_current_output_buffer()) { + ob_flush(); + } + } + + /** + * Add a log record to the task log. + * + * @param string $log + * @return string + */ + public static function add_line(string $log) : string { + if (empty(self::$taskloginfo)) { + return $log; + } + + if (empty(self::$fh)) { + return $log; + } + + if (self::is_current_output_buffer()) { + fwrite(self::$fh, $log); + } + + return $log; + } +} diff --git a/lib/classes/task/manager.php b/lib/classes/task/manager.php index fe4124a39e5..6e8f467178c 100644 --- a/lib/classes/task/manager.php +++ b/lib/classes/task/manager.php @@ -590,6 +590,9 @@ class manager { $task->get_cron_lock()->release(); } $task->get_lock()->release(); + + // Finalise the log output. + \core\task\logmanager::finalise_log(true); } /** @@ -600,6 +603,9 @@ class manager { public static function adhoc_task_complete(adhoc_task $task) { global $DB; + // Finalise the log output. + \core\task\logmanager::finalise_log(); + // Delete the adhoc task record - it is finished. $DB->delete_records('task_adhoc', array('id' => $task->get_id())); @@ -643,6 +649,9 @@ class manager { $task->get_cron_lock()->release(); } $task->get_lock()->release(); + + // Finalise the log output. + \core\task\logmanager::finalise_log(true); } /** @@ -670,6 +679,9 @@ class manager { public static function scheduled_task_complete(scheduled_task $task) { global $DB; + // Finalise the log output. + \core\task\logmanager::finalise_log(); + $classname = self::get_canonical_class_name($task); $record = $DB->get_record('task_scheduled', array('classname' => $classname)); if ($record) { diff --git a/lib/classes/task/task_log_cleanup_task.php b/lib/classes/task/task_log_cleanup_task.php new file mode 100644 index 00000000000..188171a5efb --- /dev/null +++ b/lib/classes/task/task_log_cleanup_task.php @@ -0,0 +1,53 @@ +. + +/** + * Task to cleanup task logs. + * + * @package core + * @copyright 2018 Andrew Nicols + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +namespace core\task; + +defined('MOODLE_INTERNAL') || die(); + +/** + * A task to cleanup log entries for tasks. + * + * @copyright 2018 Andrew Nicols + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class task_log_cleanup_task extends scheduled_task { + + /** + * Get a descriptive name for this task (shown to admins). + * + * @return string + */ + public function get_name() { + return get_string('tasklogcleanup', 'admin'); + } + + /** + * Perform the cleanup task. + */ + public function execute() { + if (\core\task\database_logger::class == \core\task\logmanager::get_logger_classname()) { + \core\task\database_logger::cleanup(); + } + } +} diff --git a/lib/classes/task/task_logger.php b/lib/classes/task/task_logger.php new file mode 100644 index 00000000000..c7b322ac21c --- /dev/null +++ b/lib/classes/task/task_logger.php @@ -0,0 +1,57 @@ +. + +/** + * Interface for task logging. + * + * @package core + * @category task + * @copyright 2018 Andrew Nicols + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +namespace core\task; + +defined('MOODLE_INTERNAL') || die(); + +/** + * Interface for task logging. + * + * @copyright 2018 Andrew Nicols + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +interface task_logger { + /** + * Whether the task is configured and ready to log. + * + * @return bool + */ + public static function is_configured() : bool; + + /** + * Store the log for the specified task. + * + * @param task_base $task The task that the log belongs to. + * @param string $logpath The path to the log on disk + * @param bool $failed Whether the task failed + * @param int $dbreads The number of DB reads + * @param int $dbwrites The number of DB writes + * @param float $timestart The start time of the task + * @param float $timeend The end time of the task + */ + public static function store_log_for_task(task_base $task, string $logpath, bool $failed, + int $dbreads, int $dbwrites, float $timestart, float $timeend); + +} diff --git a/lib/cronlib.php b/lib/cronlib.php index 589af74503f..b11be3f45ba 100644 --- a/lib/cronlib.php +++ b/lib/cronlib.php @@ -93,6 +93,8 @@ function cron_run() { function cron_run_inner_scheduled_task(\core\task\task_base $task) { global $CFG, $DB; + \core\task\logmanager::start_logging($task); + $fullname = $task->get_name() . ' (' . get_class($task) . ')'; mtrace('Execute scheduled task: ' . $fullname); cron_trace_time_and_memory(); @@ -144,6 +146,9 @@ function cron_run_inner_scheduled_task(\core\task\task_base $task) { */ function cron_run_inner_adhoc_task(\core\task\adhoc_task $task) { global $DB, $CFG; + + \core\task\logmanager::start_logging($task); + mtrace("Execute adhoc task: " . get_class($task)); cron_trace_time_and_memory(); $predbqueries = null; diff --git a/lib/db/install.xml b/lib/db/install.xml index 2c57cdaa3da..7de20fcf304 100644 --- a/lib/db/install.xml +++ b/lib/db/install.xml @@ -1,5 +1,5 @@ - @@ -3317,6 +3317,7 @@ + diff --git a/lib/db/tasks.php b/lib/db/tasks.php index 38e8d85c4a7..93ffff03005 100644 --- a/lib/db/tasks.php +++ b/lib/db/tasks.php @@ -356,4 +356,13 @@ $tasks = array( 'dayofweek' => '*', 'month' => '*' ), + array( + 'classname' => 'core\task\task_log_cleanup_task', + 'blocking' => 0, + 'minute' => 'R', + 'hour' => 'R', + 'day' => '*', + 'dayofweek' => '*', + 'month' => '*' + ), ); diff --git a/lib/db/upgrade.php b/lib/db/upgrade.php index 80dc8ec4c38..bf81f99238a 100644 --- a/lib/db/upgrade.php +++ b/lib/db/upgrade.php @@ -2548,5 +2548,19 @@ function xmldb_main_upgrade($oldversion) { upgrade_main_savepoint(true, 2019011500.00); } + if ($oldversion < 2019011501.00) { + // Define field output to be added to task_log. + $table = new xmldb_table('task_log'); + $field = new xmldb_field('output', XMLDB_TYPE_TEXT, null, null, XMLDB_NOTNULL, null, null, 'result'); + + // Conditionally launch add field output. + if (!$dbman->field_exists($table, $field)) { + $dbman->add_field($table, $field); + } + + // Main savepoint reached. + upgrade_main_savepoint(true, 2019011501.00); + } + return true; } diff --git a/lib/moodlelib.php b/lib/moodlelib.php index 4cfc923df44..d3b85d2aac3 100644 --- a/lib/moodlelib.php +++ b/lib/moodlelib.php @@ -8953,10 +8953,15 @@ function mtrace($string, $eol="\n", $sleep=0) { return; } else if (defined('STDOUT') && !PHPUNIT_TEST && !defined('BEHAT_TEST')) { fwrite(STDOUT, $string.$eol); + + // We must explicitly call the add_line function here. + // Uses of fwrite to STDOUT are not picked up by ob_start. + \core\task\logmanager::add_line("{$string}{$eol}"); } else { echo $string . $eol; } + // Flush again. flush(); // Delay to keep message on user's screen in case of subsequent redirect. diff --git a/lib/tests/task_database_logger_test.php b/lib/tests/task_database_logger_test.php new file mode 100644 index 00000000000..d2c43b241a4 --- /dev/null +++ b/lib/tests/task_database_logger_test.php @@ -0,0 +1,505 @@ +. + +/** + * This file contains the unit tests for the database task logger. + * + * @package core + * @category phpunit + * @copyright 2018 Andrew Nicols + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +defined('MOODLE_INTERNAL') || die(); + +use \core\task\database_logger; + +/** + * This file contains the unit tests for the database task logger. + * + * @copyright 2018 Andrew Nicols + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class task_database_logger_testcase extends advanced_testcase { + + /** + * @var \moodle_database The original database prior to mocking + */ + protected $DB; + + /** + * Setup to backup the database before mocking. + */ + public function setUp() { + global $DB; + + $this->DB = $DB; + } + + /** + * Tear down to unmock the database where it was mocked. + */ + public function tearDown() { + global $DB; + + $DB = $this->DB; + $this->DB = null; + } + + /** + * Ensure that store_log_for_task works with a passing scheduled task. + */ + public function test_store_log_for_task_scheduled() { + global $DB; + + $this->resetAfterTest(); + + $endtime = microtime(true); + $starttime = $endtime - 4; + + $logdir = make_request_directory(); + $logpath = "{$logdir}/log.txt"; + file_put_contents($logpath, 'Example content'); + + $task = new \core\task\cache_cron_task(); + database_logger::store_log_for_task($task, $logpath, false, 1, 2, $starttime, $endtime); + + $logs = $DB->get_records('task_log'); + $this->assertCount(1, $logs); + + $log = reset($logs); + $this->assertEquals(file_get_contents($logpath), $log->output); + $this->assertEquals(0, $log->result); + $this->assertEquals(database_logger::TYPE_SCHEDULED, $log->type); + $this->assertEquals('core\task\cache_cron_task', $log->classname); + $this->assertEquals(0, $log->userid); + } + + /** + * Ensure that store_log_for_task works with a passing adhoc task. + */ + public function test_store_log_for_task_adhoc() { + global $DB; + + $this->resetAfterTest(); + + $endtime = microtime(true); + $starttime = $endtime - 4; + + $logdir = make_request_directory(); + $logpath = "{$logdir}/log.txt"; + file_put_contents($logpath, 'Example content'); + + $task = $this->getMockBuilder(\core\task\adhoc_task::class) + ->setMethods(['get_component', 'execute']) + ->getMock(); + + $task->method('get_component')->willReturn('core_test'); + + database_logger::store_log_for_task($task, $logpath, false, 1, 2, $starttime, $endtime); + + $logs = $DB->get_records('task_log'); + $this->assertCount(1, $logs); + + $log = reset($logs); + $this->assertEquals(file_get_contents($logpath), $log->output); + $this->assertEquals(0, $log->result); + $this->assertEquals(database_logger::TYPE_ADHOC, $log->type); + } + + /** + * Ensure that store_log_for_task works with a failing scheduled task. + */ + public function test_store_log_for_task_failed_scheduled() { + global $DB; + + $this->resetAfterTest(); + + $endtime = microtime(true); + $starttime = $endtime - 4; + + $logdir = make_request_directory(); + $logpath = "{$logdir}/log.txt"; + file_put_contents($logpath, 'Example content'); + + $task = new \core\task\cache_cron_task(); + database_logger::store_log_for_task($task, $logpath, true, 1, 2, $starttime, $endtime); + + $logs = $DB->get_records('task_log'); + $this->assertCount(1, $logs); + + $log = reset($logs); + $this->assertEquals(file_get_contents($logpath), $log->output); + $this->assertEquals(1, $log->result); + $this->assertEquals(database_logger::TYPE_SCHEDULED, $log->type); + $this->assertEquals('core\task\cache_cron_task', $log->classname); + $this->assertEquals(0, $log->userid); + } + + /** + * Ensure that store_log_for_task works with a failing adhoc task. + */ + public function test_store_log_for_task_failed_adhoc() { + global $DB; + + $this->resetAfterTest(); + + $endtime = microtime(true); + $starttime = $endtime - 4; + + $logdir = make_request_directory(); + $logpath = "{$logdir}/log.txt"; + file_put_contents($logpath, 'Example content'); + + $task = $this->getMockBuilder(\core\task\adhoc_task::class) + ->setMethods(['get_component', 'execute']) + ->getMock(); + + $task->method('get_component')->willReturn('core_test'); + + database_logger::store_log_for_task($task, $logpath, true, 1, 2, $starttime, $endtime); + + $logs = $DB->get_records('task_log'); + $this->assertCount(1, $logs); + + $log = reset($logs); + $this->assertEquals(file_get_contents($logpath), $log->output); + $this->assertEquals(1, $log->result); + $this->assertEquals(database_logger::TYPE_ADHOC, $log->type); + $this->assertEquals(0, $log->userid); + } + /** + * Ensure that store_log_for_task works with a passing adhoc task run as a specific user. + */ + public function test_store_log_for_task_adhoc_userid() { + global $DB; + + $this->resetAfterTest(); + + $endtime = microtime(true); + $starttime = $endtime - 4; + + $logdir = make_request_directory(); + $logpath = "{$logdir}/log.txt"; + file_put_contents($logpath, 'Example content'); + + $task = $this->getMockBuilder(\core\task\adhoc_task::class) + ->setMethods(['get_component', 'execute', 'get_userid']) + ->getMock(); + + $task->method('get_component')->willReturn('core_test'); + $task->method('get_userid')->willReturn(99); + + database_logger::store_log_for_task($task, $logpath, false, 1, 2, $starttime, $endtime); + + $logs = $DB->get_records('task_log'); + $this->assertCount(1, $logs); + + $log = reset($logs); + $this->assertEquals(file_get_contents($logpath), $log->output); + $this->assertEquals(0, $log->result); + $this->assertEquals(database_logger::TYPE_ADHOC, $log->type); + $this->assertEquals(99, $log->userid); + } + + /** + * Ensure that the delete_task_logs function performs necessary deletion tasks. + * + * @dataProvider delete_task_logs_provider + * @param mixed $ids + */ + public function test_delete_task_logs($ids) { + $DB = $this->mock_database(); + $DB->expects($this->once()) + ->method('delete_records_list') + ->with( + $this->equalTo('task_log'), + $this->equalTo('id'), + $this->callback(function($deletedids) use ($ids) { + sort($ids); + $idvalues = array_values($deletedids); + sort($idvalues); + + return $ids == $idvalues; + }) + ); + + database_logger::delete_task_logs($ids); + } + + /** + * Data provider for delete_task_logs tests. + * + * @return array + */ + public function delete_task_logs_provider() : array { + return [ + [ + [0], + [1], + [1, 2, 3, 4, 5], + ], + ]; + } + + /** + * Ensure that the retention period applies correctly. + */ + public function test_cleanup_retention() { + global $DB; + + $this->resetAfterTest(); + + // Set a high value for task_logretainruns so that it does no interfere. + set_config('task_logretainruns', 1000); + + // Create sample log data - 1 run per hour for 3 days - round down to the start of the hour to avoid time race conditions. + $date = new DateTime(); + $date->setTime($date->format('G'), 0); + $baselogtime = $date->getTimestamp(); + + for ($i = 0; $i < 3 * 24; $i++) { + $task = new \core\task\cache_cron_task(); + $logpath = __FILE__; + database_logger::store_log_for_task($task, $logpath, false, 1, 2, $date->getTimestamp(), $date->getTimestamp() + MINSECS); + + $date->sub(new \DateInterval('PT1H')); + } + + // Initially there should be 72 runs. + $this->assertCount(72, $DB->get_records('task_log')); + + // Note: We set the retention time to a period like DAYSECS minus an adjustment. + // The adjustment is to account for the time taken during setup. + + // With a retention period of 2 * DAYSECS, there should only be 47-48 left. + set_config('task_logretention', (2 * DAYSECS) - (time() - $baselogtime)); + \core\task\database_logger::cleanup(); + $this->assertGreaterThanOrEqual(47, $DB->count_records('task_log')); + $this->assertLessThanOrEqual(48, $DB->count_records('task_log')); + + // The oldest should be no more than 48 hours old. + $oldest = $DB->get_records('task_log', [], 'timestart DESC', 'timestart', 0, 1); + $oldest = reset($oldest); + $this->assertGreaterThan(time() - (48 * DAYSECS), $oldest->timestart); + + // With a retention period of DAYSECS, there should only be 23 left. + set_config('task_logretention', DAYSECS - (time() - $baselogtime)); + \core\task\database_logger::cleanup(); + $this->assertGreaterThanOrEqual(23, $DB->count_records('task_log')); + $this->assertLessThanOrEqual(24, $DB->count_records('task_log')); + + // The oldest should be no more than 24 hours old. + $oldest = $DB->get_records('task_log', [], 'timestart DESC', 'timestart', 0, 1); + $oldest = reset($oldest); + $this->assertGreaterThan(time() - (24 * DAYSECS), $oldest->timestart); + + // With a retention period of 0.5 DAYSECS, there should only be 11 left. + set_config('task_logretention', (DAYSECS / 2) - (time() - $baselogtime)); + \core\task\database_logger::cleanup(); + $this->assertGreaterThanOrEqual(11, $DB->count_records('task_log')); + $this->assertLessThanOrEqual(12, $DB->count_records('task_log')); + + // The oldest should be no more than 12 hours old. + $oldest = $DB->get_records('task_log', [], 'timestart DESC', 'timestart', 0, 1); + $oldest = reset($oldest); + $this->assertGreaterThan(time() - (12 * DAYSECS), $oldest->timestart); + } + + /** + * Ensure that the run-count retention applies. + */ + public function test_cleanup_retainruns() { + global $DB; + + $this->resetAfterTest(); + + // Set a high value for task_logretention so that it does not interfere. + set_config('task_logretention', YEARSECS); + + // Create sample log data - 2 tasks, once per hour for 3 days. + $date = new DateTime(); + $date->setTime($date->format('G'), 0); + $firstdate = $date->getTimestamp(); + + for ($i = 0; $i < 3 * 24; $i++) { + $task = new \core\task\cache_cron_task(); + $logpath = __FILE__; + database_logger::store_log_for_task($task, $logpath, false, 1, 2, $date->getTimestamp(), $date->getTimestamp() + MINSECS); + + $task = new \core\task\badges_cron_task(); + $logpath = __FILE__; + database_logger::store_log_for_task($task, $logpath, false, 1, 2, $date->getTimestamp(), $date->getTimestamp() + MINSECS); + + $date->sub(new \DateInterval('PT1H')); + } + $lastdate = $date->getTimestamp(); + + // Initially there should be 144 runs - 72 for each task. + $this->assertEquals(144, $DB->count_records('task_log')); + $this->assertEquals(72, $DB->count_records('task_log', ['classname' => \core\task\cache_cron_task::class])); + $this->assertEquals(72, $DB->count_records('task_log', ['classname' => \core\task\badges_cron_task::class])); + + // Grab the records for comparison. + $cachecronrecords = array_values($DB->get_records('task_log', ['classname' => \core\task\cache_cron_task::class], 'timestart DESC')); + $badgescronrecords = array_values($DB->get_records('task_log', ['classname' => \core\task\badges_cron_task::class], 'timestart DESC')); + + // Configured to retain 144 should have no effect. + set_config('task_logretainruns', 144); + \core\task\database_logger::cleanup(); + $this->assertEquals(144, $DB->count_records('task_log')); + $this->assertEquals(72, $DB->count_records('task_log', ['classname' => \core\task\cache_cron_task::class])); + $this->assertEquals(72, $DB->count_records('task_log', ['classname' => \core\task\badges_cron_task::class])); + + // The list of records should be identical. + $this->assertEquals($cachecronrecords, array_values($DB->get_records('task_log', ['classname' => \core\task\cache_cron_task::class], 'timestart DESC'))); + $this->assertEquals($badgescronrecords, array_values($DB->get_records('task_log', ['classname' => \core\task\badges_cron_task::class], 'timestart DESC'))); + + // Configured to retain 72 should have no effect either. + set_config('task_logretainruns', 72); + \core\task\database_logger::cleanup(); + $this->assertEquals(144, $DB->count_records('task_log')); + $this->assertEquals(72, $DB->count_records('task_log', ['classname' => \core\task\cache_cron_task::class])); + $this->assertEquals(72, $DB->count_records('task_log', ['classname' => \core\task\badges_cron_task::class])); + + // The list of records should now only contain the first 72 of each. + $this->assertEquals( + array_slice($cachecronrecords, 0, 72), + array_values($DB->get_records('task_log', ['classname' => \core\task\cache_cron_task::class], 'timestart DESC')) + ); + $this->assertEquals( + array_slice($badgescronrecords, 0, 72), + array_values($DB->get_records('task_log', ['classname' => \core\task\badges_cron_task::class], 'timestart DESC')) + ); + + // Configured to only retain 24 should bring that down to a total of 48, or 24 each. + set_config('task_logretainruns', 24); + \core\task\database_logger::cleanup(); + $this->assertEquals(48, $DB->count_records('task_log')); + $this->assertEquals(24, $DB->count_records('task_log', ['classname' => \core\task\cache_cron_task::class])); + $this->assertEquals(24, $DB->count_records('task_log', ['classname' => \core\task\badges_cron_task::class])); + + // The list of records should now only contain the first 24 of each. + $this->assertEquals( + array_slice($cachecronrecords, 0, 24), + array_values($DB->get_records('task_log', ['classname' => \core\task\cache_cron_task::class], 'timestart DESC')) + ); + $this->assertEquals( + array_slice($badgescronrecords, 0, 24), + array_values($DB->get_records('task_log', ['classname' => \core\task\badges_cron_task::class], 'timestart DESC')) + ); + + // Configured to only retain 5 should bring that down to a total of 10, or 5 each. + set_config('task_logretainruns', 5); + \core\task\database_logger::cleanup(); + $this->assertEquals(10, $DB->count_records('task_log')); + $this->assertEquals(5, $DB->count_records('task_log', ['classname' => \core\task\cache_cron_task::class])); + $this->assertEquals(5, $DB->count_records('task_log', ['classname' => \core\task\badges_cron_task::class])); + + // The list of records should now only contain the first 5 of each. + $this->assertEquals( + array_slice($cachecronrecords, 0, 5), + array_values($DB->get_records('task_log', ['classname' => \core\task\cache_cron_task::class], 'timestart DESC')) + ); + $this->assertEquals( + array_slice($badgescronrecords, 0, 5), + array_values($DB->get_records('task_log', ['classname' => \core\task\badges_cron_task::class], 'timestart DESC')) + ); + + // Configured to only retain 0 should bring that down to none. + set_config('task_logretainruns', 0); + \core\task\database_logger::cleanup(); + $this->assertEquals(0, $DB->count_records('task_log')); + } + + /** + * Ensure that the retention period applies correctly when combined with the run count retention. + */ + public function test_cleanup_combined() { + global $DB; + + $this->resetAfterTest(); + + // Create sample log data - 2 tasks, once per hour for 3 days. + $date = new DateTime(); + $date->setTime($date->format('G'), 0); + $baselogtime = $date->getTimestamp(); + + for ($i = 0; $i < 3 * 24; $i++) { + $task = new \core\task\cache_cron_task(); + $logpath = __FILE__; + database_logger::store_log_for_task($task, $logpath, false, 1, 2, $date->getTimestamp(), $date->getTimestamp() + MINSECS); + + $task = new \core\task\badges_cron_task(); + $logpath = __FILE__; + database_logger::store_log_for_task($task, $logpath, false, 1, 2, $date->getTimestamp(), $date->getTimestamp() + MINSECS); + + $date->sub(new \DateInterval('PT1H')); + } + + // Initially there should be 144 runs - 72 for each task. + $this->assertEquals(144, $DB->count_records('task_log')); + $this->assertEquals(72, $DB->count_records('task_log', ['classname' => \core\task\cache_cron_task::class])); + $this->assertEquals(72, $DB->count_records('task_log', ['classname' => \core\task\badges_cron_task::class])); + + // Note: We set the retention time to a period like DAYSECS minus an adjustment. + // The adjustment is to account for the time taken during setup. + + // With a retention period of 2 * DAYSECS, there should only be 94-96 left. + // The run count is a higher number so it will have no effect. + set_config('task_logretention', (2 * DAYSECS) - (time() - $baselogtime)); + set_config('task_logretainruns', 50); + \core\task\database_logger::cleanup(); + $this->assertGreaterThanOrEqual(94, $DB->count_records('task_log')); + $this->assertLessThanOrEqual(96, $DB->count_records('task_log')); + $this->assertGreaterThanOrEqual(47, $DB->count_records('task_log', ['classname' => \core\task\cache_cron_task::class])); + $this->assertLessThanOrEqual(48, $DB->count_records('task_log', ['classname' => \core\task\cache_cron_task::class])); + $this->assertGreaterThanOrEqual(47, $DB->count_records('task_log', ['classname' => \core\task\badges_cron_task::class])); + $this->assertLessThanOrEqual(48, $DB->count_records('task_log', ['classname' => \core\task\badges_cron_task::class])); + + // We should retain the most recent 48 so the oldest will be no more than 48 hours old. + $oldest = $DB->get_records('task_log', [], 'timestart DESC', 'timestart', 0, 1); + $oldest = reset($oldest); + $this->assertGreaterThan(time() - (48 * DAYSECS), $oldest->timestart); + + // Reducing the retain runs count to 10 should reduce the total logs to 20, overriding the time constraint. + set_config('task_logretainruns', 10); + \core\task\database_logger::cleanup(); + $this->assertEquals(20, $DB->count_records('task_log')); + $this->assertEquals(10, $DB->count_records('task_log', ['classname' => \core\task\cache_cron_task::class])); + $this->assertEquals(10, $DB->count_records('task_log', ['classname' => \core\task\badges_cron_task::class])); + + // We should retain the most recent 10 so the oldeste will be no more than 10 hours old. + $oldest = $DB->get_records('task_log', [], 'timestart DESC', 'timestart', 0, 1); + $oldest = reset($oldest); + $this->assertGreaterThan(time() - (10 * DAYSECS), $oldest->timestart); + } + + /** + * Mock the database. + */ + protected function mock_database() { + global $DB; + + $DB = $this->getMockBuilder(\moodle_database::class) + ->getMock(); + + $DB->method('get_record') + ->willReturn((object) []); + + return $DB; + } +} diff --git a/lib/tests/task_logging_test.php b/lib/tests/task_logging_test.php new file mode 100644 index 00000000000..aaed98c5de4 --- /dev/null +++ b/lib/tests/task_logging_test.php @@ -0,0 +1,558 @@ +. + +/** + * This file contains the unit tests for the task logging system. + * + * @package core + * @category phpunit + * @copyright 2018 Andrew Nicols + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +defined('MOODLE_INTERNAL') || die(); +require_once(__DIR__ . '/fixtures/task_fixtures.php'); + + +/** + * This file contains the unit tests for the task logging system. + * + * @copyright 2018 Andrew Nicols + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class core_task_logmanager extends advanced_testcase { + + /** + * @var \moodle_database The original database prior to mocking + */ + protected $DB; + + /** + * Relevant tearDown for logging tests. + */ + public function tearDown() { + global $DB; + + // Ensure that any logging is always ended. + \core\task\logmanager::finalise_log(); + + if (null !== $this->DB) { + $DB = $this->DB; + $this->DB = null; + } + } + + /** + * When the logmode is set to none, logging should not start. + */ + public function test_logmode_none() { + global $CFG; + $this->resetAfterTest(); + + $CFG->task_logmode = \core\task\logmanager::MODE_NONE; + + $initialbufferstate = ob_get_status(); + + $task = $this->get_test_adhoc_task(); + \core\task\logmanager::start_logging($task); + + // There will be no additional output buffer. + $this->assertEquals($initialbufferstate, ob_get_status()); + } + + /** + * When the logmode is set to all that log capture is started. + */ + public function test_start_logmode_all() { + global $CFG; + $this->resetAfterTest(); + + $CFG->task_logmode = \core\task\logmanager::MODE_ALL; + + $initialbufferstate = ob_get_status(); + + $task = $this->get_test_adhoc_task(); + \core\task\logmanager::start_logging($task); + + // Fetch the new output buffer state. + $state = ob_get_status(); + + // There will be no additional output buffer. + $this->assertNotEquals($initialbufferstate, $state); + } + + /** + * When the logmode is set to fail that log capture is started. + */ + public function test_start_logmode_fail() { + global $CFG; + $this->resetAfterTest(); + + $CFG->task_logmode = \core\task\logmanager::MODE_FAILONLY; + + $initialbufferstate = ob_get_status(); + + $task = $this->get_test_adhoc_task(); + \core\task\logmanager::start_logging($task); + + // Fetch the new output buffer state. + $state = ob_get_status(); + + // There will be no additional output buffer. + $this->assertNotEquals($initialbufferstate, $state); + } + + /** + * When the logmode is set to fail, passing adhoc tests should not be logged. + */ + public function test_logmode_fail_with_passing_adhoc_task() { + global $CFG; + $this->resetAfterTest(); + + $CFG->task_logmode = \core\task\logmanager::MODE_FAILONLY; + + $logger = $this->get_mocked_logger(); + + $initialbufferstate = ob_get_status(); + + $task = $this->get_test_adhoc_task(); + \core\task\logmanager::start_logging($task); + + \core\task\manager::adhoc_task_complete($task); + + $this->assertEmpty($logger::$storelogfortask); + } + + /** + * When the logmode is set to fail, passing scheduled tests should not be logged. + */ + public function test_logmode_fail_with_passing_scheduled_task() { + global $CFG; + $this->resetAfterTest(); + + $CFG->task_logmode = \core\task\logmanager::MODE_FAILONLY; + + $logger = $this->get_mocked_logger(); + + $initialbufferstate = ob_get_status(); + + $task = $this->get_test_scheduled_task(); + \core\task\logmanager::start_logging($task); + + \core\task\manager::scheduled_task_complete($task); + + $this->assertEmpty($logger::$storelogfortask); + } + + /** + * When the logmode is set to fail, failing adhoc tests should be logged. + */ + public function test_logmode_fail_with_failing_adhoc_task() { + global $CFG; + + $this->resetAfterTest(); + + // Mock the database. Marking jobs as failed updates a DB record which doesn't exist. + $this->mock_database(); + + $task = $this->get_test_adhoc_task(); + + $CFG->task_logmode = \core\task\logmanager::MODE_FAILONLY; + + $logger = $this->get_mocked_logger(); + + \core\task\logmanager::start_logging($task); + \core\task\manager::adhoc_task_failed($task); + + $this->assertCount(1, $logger::$storelogfortask); + $this->assertEquals($task, $logger::$storelogfortask[0][0]); + $this->assertTrue($logger::$storelogfortask[0][2]); + } + + /** + * When the logmode is set to fail, failing scheduled tests should be logged. + */ + public function test_logmode_fail_with_failing_scheduled_task() { + global $CFG; + + $this->resetAfterTest(); + + // Mock the database. Marking jobs as failed updates a DB record which doesn't exist. + $this->mock_database(); + + $task = $this->get_test_scheduled_task(); + + $CFG->task_logmode = \core\task\logmanager::MODE_FAILONLY; + + $logger = $this->get_mocked_logger(); + + \core\task\logmanager::start_logging($task); + \core\task\manager::scheduled_task_failed($task); + + $this->assertCount(1, $logger::$storelogfortask); + $this->assertEquals($task, $logger::$storelogfortask[0][0]); + $this->assertTrue($logger::$storelogfortask[0][2]); + } + + /** + * When the logmode is set to fail, failing adhoc tests should be logged. + */ + public function test_logmode_any_with_failing_adhoc_task() { + global $CFG; + + $this->resetAfterTest(); + + // Mock the database. Marking jobs as failed updates a DB record which doesn't exist. + $this->mock_database(); + + $task = $this->get_test_adhoc_task(); + + $CFG->task_logmode = \core\task\logmanager::MODE_FAILONLY; + + $logger = $this->get_mocked_logger(); + + \core\task\logmanager::start_logging($task); + \core\task\manager::adhoc_task_failed($task); + + $this->assertCount(1, $logger::$storelogfortask); + $this->assertEquals($task, $logger::$storelogfortask[0][0]); + $this->assertTrue($logger::$storelogfortask[0][2]); + } + + /** + * When the logmode is set to fail, failing scheduled tests should be logged. + */ + public function test_logmode_any_with_failing_scheduled_task() { + global $CFG; + + $this->resetAfterTest(); + + // Mock the database. Marking jobs as failed updates a DB record which doesn't exist. + $this->mock_database(); + + $task = $this->get_test_scheduled_task(); + + $CFG->task_logmode = \core\task\logmanager::MODE_FAILONLY; + + $logger = $this->get_mocked_logger(); + + \core\task\logmanager::start_logging($task); + \core\task\manager::scheduled_task_failed($task); + + $this->assertCount(1, $logger::$storelogfortask); + $this->assertEquals($task, $logger::$storelogfortask[0][0]); + $this->assertTrue($logger::$storelogfortask[0][2]); + } + + /** + * When the logmode is set to fail, passing adhoc tests should be logged. + */ + public function test_logmode_any_with_passing_adhoc_task() { + global $CFG; + + $this->resetAfterTest(); + + $this->mock_database(); + + $task = $this->get_test_adhoc_task(); + + $CFG->task_logmode = \core\task\logmanager::MODE_ALL; + + $logger = $this->get_mocked_logger(); + + \core\task\logmanager::start_logging($task); + \core\task\manager::adhoc_task_complete($task); + + $this->assertCount(1, $logger::$storelogfortask); + $this->assertEquals($task, $logger::$storelogfortask[0][0]); + $this->assertFalse($logger::$storelogfortask[0][2]); + } + + /** + * When the logmode is set to fail, passing scheduled tests should be logged. + */ + public function test_logmode_any_with_passing_scheduled_task() { + global $CFG; + + $this->resetAfterTest(); + + $this->mock_database(); + + $task = $this->get_test_scheduled_task(); + + $CFG->task_logmode = \core\task\logmanager::MODE_ALL; + + $logger = $this->get_mocked_logger(); + + \core\task\logmanager::start_logging($task); + \core\task\manager::scheduled_task_complete($task); + + $this->assertCount(1, $logger::$storelogfortask); + $this->assertEquals($task, $logger::$storelogfortask[0][0]); + $this->assertFalse($logger::$storelogfortask[0][2]); + } + + /** + * Ensure that start_logging cannot be called in a nested fashion. + */ + public function test_prevent_nested_logging() { + $this->resetAfterTest(); + + $task = $this->get_test_adhoc_task(); + \core\task\logmanager::start_logging($task); + + $this->expectException(\coding_exception::class); + \core\task\logmanager::start_logging($task); + } + + /** + * Ensure that logging can be called after a previous log has finished. + */ + public function test_repeated_usages() { + $this->resetAfterTest(); + + $logger = $this->get_mocked_logger(); + + $task = $this->get_test_adhoc_task(); + \core\task\logmanager::start_logging($task); + \core\task\logmanager::finalise_log(); + + \core\task\logmanager::start_logging($task); + \core\task\logmanager::finalise_log(); + + $this->assertCount(2, $logger::$storelogfortask); + $this->assertEquals($task, $logger::$storelogfortask[0][0]); + $this->assertFalse($logger::$storelogfortask[0][2]); + $this->assertEquals($task, $logger::$storelogfortask[1][0]); + $this->assertFalse($logger::$storelogfortask[1][2]); + } + + /** + * Enusre that when finalise_log is called when logging is not active, nothing happens. + */ + public function test_finalise_log_no_logging() { + $initialbufferstate = ob_get_status(); + + \core\task\logmanager::finalise_log(); + + // There will be no additional output buffer. + $this->assertEquals($initialbufferstate, ob_get_status()); + } + + /** + * When log capture is enabled, calls to the flush function should cause log output to be both returned and captured. + */ + public function test_flush_on_own_buffer() { + $this->resetAfterTest(); + + $logger = $this->get_mocked_logger(); + + $testoutput = "I am the output under test.\n"; + + $task = $this->get_test_adhoc_task(); + \core\task\logmanager::start_logging($task); + + echo $testoutput; + + $this->expectOutputString($testoutput); + \core\task\logmanager::flush(); + + // Finalise the log. + \core\task\logmanager::finalise_log(); + + $this->assertCount(1, $logger::$storelogfortask); + $this->assertEquals($testoutput, file_get_contents($logger::$storelogfortask[0][1])); + } + + /** + * When log capture is enabled, calls to the flush function should not affect any subsequent ob_start. + */ + public function test_flush_does_not_flush_inner_buffers() { + $this->resetAfterTest(); + + $logger = $this->get_mocked_logger(); + + $testoutput = "I am the output under test.\n"; + + $task = $this->get_test_adhoc_task(); + \core\task\logmanager::start_logging($task); + + ob_start(); + echo $testoutput; + ob_end_clean(); + + \core\task\logmanager::flush(); + + // Finalise the log. + \core\task\logmanager::finalise_log(); + + $this->assertCount(1, $logger::$storelogfortask); + + // The task logger should not have captured the content of the inner buffer. + $this->assertEquals('', file_get_contents($logger::$storelogfortask[0][1])); + } + + /** + * When log capture is enabled, calls to the flush function should not affect any subsequent ob_start. + */ + public function test_inner_flushed_buffers_are_logged() { + $this->resetAfterTest(); + + $logger = $this->get_mocked_logger(); + + $testoutput = "I am the output under test.\n"; + + $task = $this->get_test_adhoc_task(); + \core\task\logmanager::start_logging($task); + + // We are going to flush the inner buffer. That means that we should expect the output immediately. + $this->expectOutputString($testoutput); + + ob_start(); + echo $testoutput; + ob_end_flush(); + + // Finalise the log. + \core\task\logmanager::finalise_log(); + + $this->assertCount(1, $logger::$storelogfortask); + + // The task logger should not have captured the content of the inner buffer. + $this->assertEquals($testoutput, file_get_contents($logger::$storelogfortask[0][1])); + } + + /** + * Get an example adhoc task to use for testing. + * + * @return \core\task\adhoc_task + */ + protected function get_test_adhoc_task() : \core\task\adhoc_task { + $task = $this->getMockForAbstractClass(\core\task\adhoc_task::class); + + // Mock a lock on the task. + $lock = $this->getMockBuilder(\core\lock\lock::class) + ->disableOriginalConstructor() + ->getMock(); + $task->set_lock($lock); + + return $task; + } + + /** + * Get an example scheduled task to use for testing. + * + * @return \core\task\scheduled_task + */ + protected function get_test_scheduled_task() : \core\task\scheduled_task { + $task = $this->getMockForAbstractClass(\core\task\scheduled_task::class); + + // Mock a lock on the task. + $lock = $this->getMockBuilder(\core\lock\lock::class) + ->disableOriginalConstructor() + ->getMock(); + $task->set_lock($lock); + + return $task; + } + + /** + * Create and configure a mocked task logger. + * + * @return \core\task\task_logger + */ + protected function get_mocked_logger() { + global $CFG; + + // We will modify config for the alternate logging class therefore we mnust reset after the test. + $this->resetAfterTest(); + + // Note PHPUnit does not support mocking static functions. + $CFG->task_log_class = \task_logging_test_mocked_logger::class; + \task_logging_test_mocked_logger::test_reset(); + + return $CFG->task_log_class; + } + + /** + * Mock the database. + */ + protected function mock_database() { + global $DB; + + // Store the old Database for restoration in reset. + $this->DB = $DB; + + $DB = $this->getMockBuilder(\moodle_database::class) + ->getMock(); + + $DB->method('get_record') + ->willReturn((object) []); + } +} + +/** + * Mocked logger. + * + * @copyright 2018 Andrew Nicols + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class task_logging_test_mocked_logger implements \core\task\task_logger { + + /** + * @var bool Whether this is configured. + */ + public static $isconfigured = true; + + /** + * @var array Arguments that store_log_for_task was called with. + */ + public static $storelogfortask = []; + + /** + * Reset the test class. + */ + public static function test_reset() { + self::$isconfigured = true; + self::$storelogfortask = []; + self::$haslogreport = true; + } + + /** + * Whether the task is configured and ready to log. + * + * @return bool + */ + public static function is_configured() : bool { + return self::$isconfigured; + } + + /** + * Store the log for the specified task. + * + * @param \core\task\task_base $task The task that the log belongs to. + * @param string $logpath The path to the log on disk + * @param bool $failed Whether the task failed + * @param int $dbreads The number of DB reads + * @param int $dbwrites The number of DB writes + * @param float $timestart The start time of the task + * @param float $timeend The end time of the task + */ + public static function store_log_for_task(\core\task\task_base $task, string $logpath, bool $failed, + int $dbreads, int $dbwrites, float $timestart, float $timeend) { + self::$storelogfortask[] = func_get_args(); + } + +} diff --git a/version.php b/version.php index be82e4bb506..84b00cb36d2 100644 --- a/version.php +++ b/version.php @@ -29,7 +29,7 @@ defined('MOODLE_INTERNAL') || die(); -$version = 2019011500.00; // YYYYMMDD = weekly release date of this DEV branch. +$version = 2019011501.00; // YYYYMMDD = weekly release date of this DEV branch. // RR = release increments - 00 in DEV branches. // .XX = incremental changes. From 8c69e86cd4830d0246c1c0efa31aa9366342b114 Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Tue, 4 Dec 2018 10:46:25 +0800 Subject: [PATCH 4/6] MDL-49399 task: Add admin log viewer AMOS BEGIN CPY [eventstarttime,core_calendar],[task_starttime,core_admin] CPY [eventduration,core_calendar],[task_duration,core_admin] CPY [result,core_cache],[task_result,core_admin] CPY [database,install],[task_dbstats,core_admin] CPY [fail,install],[task_result:failed,core_admin] AMOS END --- admin/classes/task_log_table.php | 280 +++++++++++++++++++++++++++ admin/settings/server.php | 9 +- admin/tasklogs.php | 92 +++++++++ admin/templates/tasklogs.mustache | 34 ++++ lang/en/admin.php | 11 ++ lib/classes/task/database_logger.php | 23 +++ lib/classes/task/logmanager.php | 9 +- lib/classes/task/manager.php | 8 +- lib/classes/task/task_logger.php | 14 ++ lib/tests/task_logging_test.php | 24 +++ theme/boost/scss/moodle/admin.scss | 7 + theme/boost/style/moodle.css | 4 + 12 files changed, 508 insertions(+), 7 deletions(-) create mode 100644 admin/classes/task_log_table.php create mode 100644 admin/tasklogs.php create mode 100644 admin/templates/tasklogs.mustache diff --git a/admin/classes/task_log_table.php b/admin/classes/task_log_table.php new file mode 100644 index 00000000000..554d92db791 --- /dev/null +++ b/admin/classes/task_log_table.php @@ -0,0 +1,280 @@ +. + +/** + * Task log table. + * + * @package core_admin + * @copyright 2018 Andrew Nicols + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +namespace core_admin; + +defined('MOODLE_INTERNAL') || die(); + +require_once($CFG->libdir . '/tablelib.php'); + +/** + * Table to display list of task logs. + * + * @copyright 2018 Andrew Nicols + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class task_log_table extends \table_sql { + + /** + * Constructor for the task_log table. + * + * @param string $filter + * @param int $resultfilter + */ + public function __construct(string $filter = '', int $resultfilter = null) { + global $DB; + + if (-1 === $resultfilter) { + $resultfilter = null; + } + + parent::__construct('tasklogs'); + + $columnheaders = [ + 'classname' => get_string('name'), + 'type' => get_string('tasktype', 'admin'), + 'userid' => get_string('user', 'admin'), + 'timestart' => get_string('task_starttime', 'admin'), + 'duration' => get_string('task_duration', 'admin'), + 'db' => get_string('task_dbstats', 'admin'), + 'result' => get_string('task_result', 'admin'), + 'actions' => '', + ]; + $this->define_columns(array_keys($columnheaders)); + $this->define_headers(array_values($columnheaders)); + + // The name column is a header. + $this->define_header_column('classname'); + + // This table is not collapsible. + $this->collapsible(false); + + // The actions class should not wrap. Use the BS text utility class. + $this->column_class('actions', 'text-nowrap'); + + // Allow pagination. + $this->pageable(true); + + // Allow sorting. Default to sort by timestarted DESC. + $this->sortable(true, 'timestart', SORT_DESC); + + // Add filtering. + $where = []; + $params = []; + if (!empty($filter)) { + $where[] = $DB->sql_like('classname', ':filter', false, false); + $filter = str_replace('\\', '\\\\', $filter); + $params['filter'] = '%' . $DB->sql_like_escape($filter) . '%'; + } + + if (null !== $resultfilter) { + $where[] = 'tl.result = :result'; + $params['result'] = $resultfilter; + } + + $where = implode(' AND ', $where); + + $this->set_sql('', '', $where, $params); + } + + /** + * Query the db. Store results in the table object for use by build_table. + * + * @param int $pagesize size of page for paginated displayed table. + * @param bool $useinitialsbar do you want to use the initials bar. Bar + * will only be used if there is a fullname column defined for the table. + */ + public function query_db($pagesize, $useinitialsbar = true) { + global $DB; + + // Fetch the attempts. + $sort = $this->get_sql_sort(); + if ($sort) { + $sort = "ORDER BY $sort"; + } + + $extrafields = get_extra_user_fields(\context_system::instance()); + $userfields = \user_picture::fields('u', $extrafields, 'userid2', 'user'); + + $where = ''; + if (!empty($this->sql->where)) { + $where = "WHERE {$this->sql->where}"; + } + + $sql = "SELECT + tl.*, + tl.dbreads + tl.dbwrites AS db, + tl.timeend - tl.timestart AS duration, + {$userfields} + FROM {task_log} tl + LEFT JOIN {user} u ON u.id = tl.userid + {$where} + {$sort}"; + + $this->pagesize($pagesize, $DB->count_records_sql("SELECT COUNT('x') FROM {task_log} tl {$where}", $this->sql->params)); + if (!$this->is_downloading()) { + $this->rawdata = $DB->get_records_sql($sql, $this->sql->params, $this->get_page_start(), $this->get_page_size()); + } else { + $this->rawdata = $DB->get_records_sql($sql, $this->sql->params); + } + } + + /** + * Format the name cell. + * + * @param \stdClass $row + * @return string + */ + public function col_classname($row) : string { + $output = ''; + if (class_exists($row->classname)) { + $task = new $row->classname; + if ($task instanceof \core\task\scheduled_task) { + $output = $task->get_name(); + } + } + + $output .= \html_writer::tag('div', "\\{$row->classname}", [ + 'class' => 'task-class', + ]); + return $output; + } + + /** + * Format the type cell. + * + * @param \stdClass $row + * @return string + */ + public function col_type($row) : string { + if (\core\task\database_logger::TYPE_SCHEDULED == $row->type) { + return get_string('task_type:scheduled', 'admin'); + } else { + return get_string('task_type:adhoc', 'admin'); + } + } + + /** + * Format the timestart cell. + * + * @param \stdClass $row + * @return string + */ + public function col_result($row) : string { + if ($row->result) { + return get_string('task_result:failed', 'admin'); + } else { + return get_string('success'); + } + } + + /** + * Format the timestart cell. + * + * @param \stdClass $row + * @return string + */ + public function col_timestart($row) : string { + return userdate($row->timestart, get_string('strftimedatetimeshort', 'langconfig')); + } + + /** + * Format the duration cell. + * + * @param \stdClass $row + * @return string + */ + public function col_duration($row) : string { + $duration = round($row->timeend - $row->timestart, 2); + + if (empty($duration)) { + // The format_time function returns 'now' when the difference is exactly 0. + // Note: format_time performs concatenation in exactly this fashion so we should do this for consistency. + return '0 ' . get_string('secs', 'moodle'); + } + + return format_time($duration); + } + + /** + * Format the DB details cell. + * + * @param \stdClass $row + * @return string + */ + public function col_db($row) : string { + $output = ''; + + $output .= \html_writer::div(get_string('task_stats:dbreads', 'admin', $row->dbreads)); + $output .= \html_writer::div(get_string('task_stats:dbwrites', 'admin', $row->dbwrites)); + + return $output; + } + + /** + * Format the actions cell. + * + * @param \stdClass $row + * @return string + */ + public function col_actions($row) : string { + global $OUTPUT; + + $actions = []; + + $url = new \moodle_url('/admin/tasklogs.php', ['logid' => $row->id]); + + // Quick view. + $actions[] = $OUTPUT->action_icon( + $url, + new \pix_icon('e/search', get_string('view')), + new \popup_action('click', $url) + ); + + // Download. + $actions[] = $OUTPUT->action_icon( + new \moodle_url($url, ['download' => true]), + new \pix_icon('t/download', get_string('download')) + ); + + return implode(' ', $actions); + } + + /** + * Format the user cell. + * + * @param \stdClass $row + * @return string + */ + public function col_userid($row) : string { + if (empty($row->userid)) { + return ''; + } + + $user = (object) []; + username_load_fields_from_object($user, $row, 'user'); + + return fullname($user); + } +} diff --git a/admin/settings/server.php b/admin/settings/server.php index 73efce12e45..eaa318c72c5 100644 --- a/admin/settings/server.php +++ b/admin/settings/server.php @@ -246,10 +246,17 @@ if (\core\task\logmanager::uses_standard_settings()) { PARAM_INT ) ); - } $ADMIN->add('taskconfig', $temp); +if (empty($CFG->task_log_class) || '\\core\\task\\database_logger' == $CFG->task_log_class) { + $ADMIN->add('taskconfig', new admin_externalpage( + 'tasklogs', + new lang_string('tasklogs','admin'), + "{$CFG->wwwroot}/{$CFG->admin}/tasklogs.php" + )); +} + // E-mail settings. $ADMIN->add('server', new admin_category('email', new lang_string('categoryemail', 'admin'))); diff --git a/admin/tasklogs.php b/admin/tasklogs.php new file mode 100644 index 00000000000..bcebd581d38 --- /dev/null +++ b/admin/tasklogs.php @@ -0,0 +1,92 @@ +. + +/** + * Task log. + * + * @package admin + * @copyright 2018 Andrew Nicols + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +require_once(__DIR__ . '/../config.php'); +require_once("{$CFG->libdir}/adminlib.php"); +require_once("{$CFG->libdir}/tablelib.php"); +require_once("{$CFG->libdir}/filelib.php"); + +$filter = optional_param('filter', '', PARAM_ALPHANUMEXT); +$result = optional_param('result', null, PARAM_INT); + +$pageurl = new \moodle_url('/admin/tasklogs.php'); +$pageurl->param('filter', $filter); + +$PAGE->set_url($pageurl); +$PAGE->set_context(context_system::instance()); +$PAGE->set_pagelayout('admin'); +$strheading = get_string('tasklogs', 'tool_task'); +$PAGE->set_title($strheading); +$PAGE->set_heading($strheading); + +require_login(); + +require_capability('moodle/site:config', context_system::instance()); +admin_externalpage_setup('tasklogs'); + +$logid = optional_param('logid', null, PARAM_INT); +$download = optional_param('download', false, PARAM_BOOL); + +if (null !== $logid) { + $log = $DB->get_record('task_log', ['id' => $logid], '*', MUST_EXIST); + + $fs = get_file_storage(); + $file = $fs->get_file(\context_system::instance()->id, 'core', 'task_logs', $log->id, '/', 'log.txt'); + + $filename = str_replace('\\', '_', $log->classname) . "-{$log->id}.log"; + send_stored_file($file, null, 0, $download, [ + 'filename' => $filename, + ]); +} + +$renderer = $PAGE->get_renderer('tool_task'); + +echo $OUTPUT->header(); +echo $OUTPUT->render_from_template('core_admin/tasklogs', (object) [ + 'action' => $pageurl->out(), + 'filter' => $filter, + 'resultfilteroptions' => [ + (object) [ + 'value' => -1, + 'title' => get_string('all'), + 'selected' => (-1 === $result), + ], + (object) [ + 'value' => 0, + 'title' => get_string('success'), + 'selected' => (0 === $result), + ], + (object) [ + 'value' => 1, + 'title' => get_string('task_result:failed', 'admin'), + 'selected' => (1 === $result), + ], + ], +]); + +$table = new \core_admin\task_log_table($filter, $result); +$table->baseurl = $pageurl; +$table->out(100, false); + +echo $OUTPUT->footer(); diff --git a/admin/templates/tasklogs.mustache b/admin/templates/tasklogs.mustache new file mode 100644 index 00000000000..c3180a2cb22 --- /dev/null +++ b/admin/templates/tasklogs.mustache @@ -0,0 +1,34 @@ +{{! + 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 . +}} +{{! + @template core_admin/tasklogs + + Task Logs template. +}} +
+ + + + + + + + diff --git a/lang/en/admin.php b/lang/en/admin.php index 86f77cd08a2..e21455dc7f0 100644 --- a/lang/en/admin.php +++ b/lang/en/admin.php @@ -1030,6 +1030,7 @@ $string['requestcategoryselection'] = 'Enable category selection'; $string['restorecourse'] = 'Restore course'; $string['restorernewroleid'] = 'Restorers\' role in courses'; $string['restorernewroleid_help'] = 'If the user does not already have the permission to manage the newly restored course, the user is automatically assigned this role and enrolled if necessary. Select "None" if you do not want restorers to be able to manage every restored course.'; +$string['resultfilter'] = 'Filter by result'; $string['reverseproxy'] = 'Reverse proxy'; $string['riskconfig'] = 'Users could change site configuration and behaviour'; $string['riskconfigshort'] = 'Configuration risk'; @@ -1173,6 +1174,16 @@ $string['task_logretention'] = 'Retention period'; $string['task_logretention_desc'] = 'The maximum period that logs should be kept for. This setting interacts with the \'Retain runs\' setting: whichever is reached first will apply'; $string['task_logretainruns'] = 'Retain runs'; $string['task_logretainruns_desc'] = 'The number of runs of each task to retain. This setting interacts with the \'Retention period\' setting: whichever is reached first will apply.'; +$string['task_type:adhoc'] = 'Adhoc'; +$string['task_type:scheduled'] = 'Scheduled'; +$string['task_result:failed'] = 'Fail'; +$string['task_stats:dbreads'] = '{$a} reads'; +$string['task_stats:dbwrites'] = '{$a} writes'; +$string['task_starttime'] = 'Start time'; +$string['task_duration'] = 'Duration'; +$string['task_dbstats'] = 'Database'; +$string['task_result'] = 'Result'; +$string['tasktype'] = 'Type'; $string['taskadmintitle'] = 'Tasks'; $string['taskanalyticscleanup'] = 'Analytics cleanup'; $string['taskautomatedbackup'] = 'Automated backups'; diff --git a/lib/classes/task/database_logger.php b/lib/classes/task/database_logger.php index 253e0d2645f..65af0ea3c65 100644 --- a/lib/classes/task/database_logger.php +++ b/lib/classes/task/database_logger.php @@ -84,6 +84,29 @@ class database_logger implements task_logger { $logdata->id = $DB->insert_record('task_log', $logdata); } + /** + * Whether this task logger has a report available. + * + * @return bool + */ + public static function has_log_report() : bool { + return true; + } + + /** + * Get any URL available for viewing relevant task log reports. + * + * @param string $classname The task class to fetch for + * @return \moodle_url + */ + public static function get_url_for_task_class(string $classname) : \moodle_url { + global $CFG; + + return new \moodle_url("/{$CFG->admin}/tasklogs.php", [ + 'filter' => $classname, + ]); + } + /** * Cleanup old task logs. */ diff --git a/lib/classes/task/logmanager.php b/lib/classes/task/logmanager.php index 6e2e6820abd..fe89d25769f 100644 --- a/lib/classes/task/logmanager.php +++ b/lib/classes/task/logmanager.php @@ -144,7 +144,12 @@ class logmanager { return false; } - return !empty(self::get_logger_classname()); + $loggerclass = self::get_logger_classname(); + if (empty($loggerclass)) { + return false; + } + + return $loggerclass::is_configured(); } /** @@ -176,7 +181,7 @@ class logmanager { } /** - * Whether to use the standard settings fore + * Whether to use the standard settings form. */ public static function uses_standard_settings() : bool { $classname = self::get_logger_classname(); diff --git a/lib/classes/task/manager.php b/lib/classes/task/manager.php index 6e8f467178c..59520182142 100644 --- a/lib/classes/task/manager.php +++ b/lib/classes/task/manager.php @@ -592,7 +592,7 @@ class manager { $task->get_lock()->release(); // Finalise the log output. - \core\task\logmanager::finalise_log(true); + logmanager::finalise_log(true); } /** @@ -604,7 +604,7 @@ class manager { global $DB; // Finalise the log output. - \core\task\logmanager::finalise_log(); + logmanager::finalise_log(); // Delete the adhoc task record - it is finished. $DB->delete_records('task_adhoc', array('id' => $task->get_id())); @@ -651,7 +651,7 @@ class manager { $task->get_lock()->release(); // Finalise the log output. - \core\task\logmanager::finalise_log(true); + logmanager::finalise_log(true); } /** @@ -680,7 +680,7 @@ class manager { global $DB; // Finalise the log output. - \core\task\logmanager::finalise_log(); + logmanager::finalise_log(); $classname = self::get_canonical_class_name($task); $record = $DB->get_record('task_scheduled', array('classname' => $classname)); diff --git a/lib/classes/task/task_logger.php b/lib/classes/task/task_logger.php index c7b322ac21c..d9118b69e89 100644 --- a/lib/classes/task/task_logger.php +++ b/lib/classes/task/task_logger.php @@ -54,4 +54,18 @@ interface task_logger { public static function store_log_for_task(task_base $task, string $logpath, bool $failed, int $dbreads, int $dbwrites, float $timestart, float $timeend); + /** + * Whether this task logger has a report available. + * + * @return bool + */ + public static function has_log_report() : bool; + + /** + * Get any URL available for viewing relevant task log reports. + * + * @param string $classname The task class to fetch for + * @return \moodle_url + */ + public static function get_url_for_task_class(string $classname) : \moodle_url; } diff --git a/lib/tests/task_logging_test.php b/lib/tests/task_logging_test.php index aaed98c5de4..d4f4e12de8b 100644 --- a/lib/tests/task_logging_test.php +++ b/lib/tests/task_logging_test.php @@ -521,6 +521,11 @@ class task_logging_test_mocked_logger implements \core\task\task_logger { */ public static $storelogfortask = []; + /** + * @var bool Whether this logger has a report. + */ + public static $haslogreport = true; + /** * Reset the test class. */ @@ -555,4 +560,23 @@ class task_logging_test_mocked_logger implements \core\task\task_logger { self::$storelogfortask[] = func_get_args(); } + /** + * Whether this task logger has a report available. + * + * @return bool + */ + public static function has_log_report() : bool { + return self::$haslogreport; + } + + /** + * Get any URL available for viewing relevant task log reports. + * + * @param string $classname The task class to fetch for + * @return \moodle_url + */ + public static function get_url_for_task_class(string $classname) : \moodle_url { + return new \moodle_url(''); + } + } diff --git a/theme/boost/scss/moodle/admin.scss b/theme/boost/scss/moodle/admin.scss index 4faa33e8c3a..85d654a9ee5 100644 --- a/theme/boost/scss/moodle/admin.scss +++ b/theme/boost/scss/moodle/admin.scss @@ -827,3 +827,10 @@ } } } + +#page-admin-tasklogs { + .task-class { + font-size: $font-size-sm; + color: $gray-600; + } +} diff --git a/theme/boost/style/moodle.css b/theme/boost/style/moodle.css index 90b2cff93e3..46bba859d13 100644 --- a/theme/boost/style/moodle.css +++ b/theme/boost/style/moodle.css @@ -11151,6 +11151,10 @@ div.editor_atto_toolbar button .icon { padding-left: 0.5rem; content: "/"; } +#page-admin-tasklogs .task-class { + font-size: 0.8203125rem; + color: #868e96; } + .blockmovetarget .accesshide { position: relative; left: initial; } From 251ee97c352916fdac41517d148284cc20c12c90 Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Tue, 4 Dec 2018 10:56:48 +0800 Subject: [PATCH 5/6] MDL-49399 tool_task: Link to log viewer This commit also adds the fa-file-text icon from font-awesome. --- admin/classes/task_log_table.php | 12 ++++- admin/settings/server.php | 2 +- admin/tasklogs.php | 14 ++--- admin/tool/task/lang/en/tool_task.php | 3 +- admin/tool/task/renderer.php | 48 +++++++++++++----- admin/tool/task/settings.php | 9 +++- .../output/icon_system_fontawesome.php | 1 + pix/e/file-text.png | Bin 0 -> 285 bytes pix/e/file-text.svg | 1 + 9 files changed, 65 insertions(+), 25 deletions(-) create mode 100644 pix/e/file-text.png create mode 100644 pix/e/file-text.svg diff --git a/admin/classes/task_log_table.php b/admin/classes/task_log_table.php index 554d92db791..2bb892db2a4 100644 --- a/admin/classes/task_log_table.php +++ b/admin/classes/task_log_table.php @@ -83,9 +83,17 @@ class task_log_table extends \table_sql { $where = []; $params = []; if (!empty($filter)) { - $where[] = $DB->sql_like('classname', ':filter', false, false); + $orwhere = []; $filter = str_replace('\\', '\\\\', $filter); - $params['filter'] = '%' . $DB->sql_like_escape($filter) . '%'; + + // Check the class name. + $orwhere[] = $DB->sql_like('classname', ':classfilter', false, false); + $params['classfilter'] = '%' . $DB->sql_like_escape($filter) . '%'; + + $orwhere[] = $DB->sql_like('output', ':outputfilter', false, false); + $params['outputfilter'] = '%' . $DB->sql_like_escape($filter) . '%'; + + $where[] = "(" . implode(' OR ', $orwhere) . ")"; } if (null !== $resultfilter) { diff --git a/admin/settings/server.php b/admin/settings/server.php index eaa318c72c5..9102155b090 100644 --- a/admin/settings/server.php +++ b/admin/settings/server.php @@ -249,7 +249,7 @@ if (\core\task\logmanager::uses_standard_settings()) { } $ADMIN->add('taskconfig', $temp); -if (empty($CFG->task_log_class) || '\\core\\task\\database_logger' == $CFG->task_log_class) { +if (\core\task\logmanager::uses_standard_settings()) { $ADMIN->add('taskconfig', new admin_externalpage( 'tasklogs', new lang_string('tasklogs','admin'), diff --git a/admin/tasklogs.php b/admin/tasklogs.php index bcebd581d38..6684a7ebc4b 100644 --- a/admin/tasklogs.php +++ b/admin/tasklogs.php @@ -27,7 +27,7 @@ require_once("{$CFG->libdir}/adminlib.php"); require_once("{$CFG->libdir}/tablelib.php"); require_once("{$CFG->libdir}/filelib.php"); -$filter = optional_param('filter', '', PARAM_ALPHANUMEXT); +$filter = optional_param('filter', '', PARAM_RAW); $result = optional_param('result', null, PARAM_INT); $pageurl = new \moodle_url('/admin/tasklogs.php'); @@ -51,13 +51,13 @@ $download = optional_param('download', false, PARAM_BOOL); if (null !== $logid) { $log = $DB->get_record('task_log', ['id' => $logid], '*', MUST_EXIST); - $fs = get_file_storage(); - $file = $fs->get_file(\context_system::instance()->id, 'core', 'task_logs', $log->id, '/', 'log.txt'); + if ($download) { + $filename = str_replace('\\', '_', $log->classname) . "-{$log->id}.log"; + header("Content-Disposition: attachment; filename=\"{$filename}\""); + } - $filename = str_replace('\\', '_', $log->classname) . "-{$log->id}.log"; - send_stored_file($file, null, 0, $download, [ - 'filename' => $filename, - ]); + readstring_accel($log->output, 'text/plain', false); + exit; } $renderer = $PAGE->get_renderer('tool_task'); diff --git a/admin/tool/task/lang/en/tool_task.php b/admin/tool/task/lang/en/tool_task.php index 8ea9c706e85..a28b478eaeb 100644 --- a/admin/tool/task/lang/en/tool_task.php +++ b/admin/tool/task/lang/en/tool_task.php @@ -48,6 +48,7 @@ $string['runpattern'] = 'Run pattern'; $string['scheduledtasks'] = 'Scheduled tasks'; $string['scheduledtaskchangesdisabled'] = 'Modifications to the list of scheduled tasks have been prevented in Moodle configuration'; $string['taskdisabled'] = 'Task disabled'; +$string['tasklogs'] = 'Task logs'; $string['taskscheduleday'] = 'Day'; $string['taskscheduleday_help'] = 'Day of month field for task schedule. The field uses the same format as unix cron. Some examples are:
  • * Every day
  • */2 Every 2nd day
  • 1 The first of every month
  • 1,15 The first and fifteenth of every month
'; $string['taskscheduledayofweek'] = 'Day of week'; @@ -59,4 +60,4 @@ $string['taskscheduleminute_help'] = 'Minute field for task schedule. The field $string['taskschedulemonth'] = 'Month'; $string['taskschedulemonth_help'] = 'Month field for task schedule. The field uses the same format as unix cron. Some examples are:
  • * Every month
  • */2 Every second month
  • 1 Every January
  • 1,5 Every January and May
'; $string['privacy:metadata'] = 'The Scheduled task configuration plugin does not store any personal data.'; - +$string['viewlogs'] = 'View logs for {$a}'; diff --git a/admin/tool/task/renderer.php b/admin/tool/task/renderer.php index 88dbdd4971d..3afd20c6b67 100644 --- a/admin/tool/task/renderer.php +++ b/admin/tool/task/renderer.php @@ -41,20 +41,33 @@ class tool_task_renderer extends plugin_renderer_base { public function scheduled_tasks_table($tasks) { global $CFG; + $showloglink = \core\task\logmanager::has_log_report(); + $table = new html_table(); - $table->head = array(get_string('name'), - get_string('component', 'tool_task'), - get_string('edit'), - get_string('lastruntime', 'tool_task'), - get_string('nextruntime', 'tool_task'), - get_string('taskscheduleminute', 'tool_task'), - get_string('taskschedulehour', 'tool_task'), - get_string('taskscheduleday', 'tool_task'), - get_string('taskscheduledayofweek', 'tool_task'), - get_string('taskschedulemonth', 'tool_task'), - get_string('faildelay', 'tool_task'), - get_string('default', 'tool_task')); + $table->head = [ + get_string('name'), + get_string('component', 'tool_task'), + get_string('edit'), + get_string('logs'), + get_string('lastruntime', 'tool_task'), + get_string('nextruntime', 'tool_task'), + get_string('taskscheduleminute', 'tool_task'), + get_string('taskschedulehour', 'tool_task'), + get_string('taskscheduleday', 'tool_task'), + get_string('taskscheduledayofweek', 'tool_task'), + get_string('taskschedulemonth', 'tool_task'), + get_string('faildelay', 'tool_task'), + get_string('default', 'tool_task'), + ]; + $table->attributes['class'] = 'admintable generaltable'; + $table->colclasses = []; + + if (!$showloglink) { + // Hide the log links. + $table->colclasses['3'] = 'hidden'; + } + $data = array(); $yes = get_string('yes'); $no = get_string('no'); @@ -72,6 +85,14 @@ class tool_task_renderer extends plugin_renderer_base { $editlink = $this->render(new pix_icon('t/locked', get_string('scheduledtaskchangesdisabled', 'tool_task'))); } + $loglink = ''; + if ($showloglink) { + $loglink = $this->action_icon( + \core\task\logmanager::get_url_for_task_class(get_class($task)), + new pix_icon('e/file-text', get_string('viewlogs', 'tool_task', $task->get_name()) + )); + } + $namecell = new html_table_cell($task->get_name() . "\n" . html_writer::tag('span', '\\'.get_class($task), array('class' => 'task-class text-ltr'))); $namecell->header = true; @@ -125,6 +146,7 @@ class tool_task_renderer extends plugin_renderer_base { $namecell, $componentcell, new html_table_cell($editlink), + new html_table_cell($loglink), new html_table_cell($lastrun . $runnow), new html_table_cell($nextrun), new html_table_cell($task->get_minute()), @@ -136,11 +158,11 @@ class tool_task_renderer extends plugin_renderer_base { new html_table_cell($customised))); // Cron-style values must always be LTR. - $row->cells[5]->attributes['class'] = 'text-ltr'; $row->cells[6]->attributes['class'] = 'text-ltr'; $row->cells[7]->attributes['class'] = 'text-ltr'; $row->cells[8]->attributes['class'] = 'text-ltr'; $row->cells[9]->attributes['class'] = 'text-ltr'; + $row->cells[10]->attributes['class'] = 'text-ltr'; if ($disabled) { $row->attributes['class'] = 'disabled'; diff --git a/admin/tool/task/settings.php b/admin/tool/task/settings.php index ac75859b5fc..ac9858ed61e 100644 --- a/admin/tool/task/settings.php +++ b/admin/tool/task/settings.php @@ -25,5 +25,12 @@ defined('MOODLE_INTERNAL') || die; if ($hassiteconfig) { - $ADMIN->add('server', new admin_externalpage('scheduledtasks', new lang_string('scheduledtasks','tool_task'), "$CFG->wwwroot/$CFG->admin/tool/task/scheduledtasks.php")); + $ADMIN->add( + 'taskconfig', + new admin_externalpage( + 'scheduledtasks', + new lang_string('scheduledtasks', 'tool_task'), + "$CFG->wwwroot/$CFG->admin/tool/task/scheduledtasks.php" + ) + ); } diff --git a/lib/classes/output/icon_system_fontawesome.php b/lib/classes/output/icon_system_fontawesome.php index 1530895f952..e90efe8d0bf 100644 --- a/lib/classes/output/icon_system_fontawesome.php +++ b/lib/classes/output/icon_system_fontawesome.php @@ -99,6 +99,7 @@ class icon_system_fontawesome extends icon_system_font { 'core:e/document_properties' => 'fa-info', 'core:e/emoticons' => 'fa-smile-o', 'core:e/find_replace' => 'fa-search-plus', + 'core:e/file-text' => 'fa-file-text', 'core:e/forward' => 'fa-arrow-right', 'core:e/fullpage' => 'fa-arrows-alt', 'core:e/fullscreen' => 'fa-arrows-alt', diff --git a/pix/e/file-text.png b/pix/e/file-text.png new file mode 100644 index 0000000000000000000000000000000000000000..02c2f66a0aeb3311c9bac9e44dc1778f319e2d9b GIT binary patch literal 285 zcmeAS@N?(olHy`uVBq!ia0vp^0wB!61|;P_|4#%`oCO|{#S9GGLLkg|>2BR0px}8= z7sn8b(^DrO6l^jOaFy>X_cmC1fwSu7Zczd22Yb{nG0J*yPMaXKg=tTYOk1NvnVR^C z`al1LS7pu$xGI-%;=K3^mm}xp!`NmtFnp1IG@Vh;L11EZRpGqcE+wOND?>#WFw0z6 zp~YFWD8WaB=aDz#*3LyoHH!-T1)}92?0Tc{v~*gjo#2OS!6xc&wGOP=D(yFqE$OXD zP+;9oP4jHQnqvo@U3Td(NEpe>l*>jmer0=*``X@B{AB>UlF^$ZH&`q}*Ydx*BP8&q fJdf)RdxN||plj5T^-2$bUSsfd^>bP0l+XkK2lr>@ literal 0 HcmV?d00001 diff --git a/pix/e/file-text.svg b/pix/e/file-text.svg new file mode 100644 index 00000000000..25a8d4e2ad7 --- /dev/null +++ b/pix/e/file-text.svg @@ -0,0 +1 @@ +Artboard 1 \ No newline at end of file From cca12f68d084a2c77e08f05cb26c073ad71bacc3 Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Wed, 5 Dec 2018 15:20:20 +0800 Subject: [PATCH 6/6] MDL-49399 core: Allow creation of a new per-request basedir Shutdown handlers are processed in order. If something in a shutdown handler uses a file which is stored in a per-request directory, and another, unrelated, per-request directory was created before the handler started. then a fresh per-request directory will be required. --- lib/classes/task/logmanager.php | 6 ++++-- lib/setuplib.php | 36 +++++++++++++++++++++++---------- 2 files changed, 29 insertions(+), 13 deletions(-) diff --git a/lib/classes/task/logmanager.php b/lib/classes/task/logmanager.php index fe89d25769f..8b812e11c31 100644 --- a/lib/classes/task/logmanager.php +++ b/lib/classes/task/logmanager.php @@ -89,8 +89,7 @@ class logmanager { } // We register a shutdown handler to ensure that logs causing any failures are correctly disposed of. - // Note: This must happen before the per-request directory is requested because the shutdown handler may delete - // the logfile. + // Note: This must happen before the per-request directory is requested because the shutdown handler deletes the logfile. if (!self::$tasklogregistered) { \core_shutdown_manager::register_function(function() { // These will only actually do anything if capturing is current active when the thread ended, which @@ -98,6 +97,9 @@ class logmanager { \core\task\logmanager::finalise_log(true); }); + // Create a brand new per-request directory basedir. + get_request_storage_directory(true, true); + self::$tasklogregistered = true; } diff --git a/lib/setuplib.php b/lib/setuplib.php index 2550a146cf7..68c283a2832 100644 --- a/lib/setuplib.php +++ b/lib/setuplib.php @@ -1633,15 +1633,22 @@ function make_upload_directory($directory, $exceptiononerror = true) { * * The directory is automatically cleaned up during the shutdown handler. * - * @param bool $exceptiononerror throw exception if error encountered - * @return string|false Returns full path to directory if successful, false if not; may throw exception + * @param bool $exceptiononerror throw exception if error encountered + * @param bool $forcecreate Force creation of a new parent directory + * @return string Returns full path to directory if successful, false if not; may throw exception */ -function get_request_storage_directory($exceptiononerror = true) { +function get_request_storage_directory($exceptiononerror = true, bool $forcecreate = false) { global $CFG; static $requestdir = null; - if (!$requestdir || !file_exists($requestdir) || !is_dir($requestdir) || !is_writable($requestdir)) { + $writabledirectoryexists = (null !== $requestdir); + $writabledirectoryexists = $writabledirectoryexists && file_exists($requestdir); + $writabledirectoryexists = $writabledirectoryexists && is_dir($requestdir); + $writabledirectoryexists = $writabledirectoryexists && is_writable($requestdir); + $createnewdirectory = $forcecreate || !$writabledirectoryexists; + + if ($createnewdirectory) { if ($CFG->localcachedir !== "$CFG->dataroot/localcache") { check_dir_exists($CFG->localcachedir, true, true); protect_directory($CFG->localcachedir); @@ -1649,10 +1656,12 @@ function get_request_storage_directory($exceptiononerror = true) { protect_directory($CFG->dataroot); } - if ($requestdir = make_unique_writable_directory($CFG->localcachedir, $exceptiononerror)) { + if ($dir = make_unique_writable_directory($CFG->localcachedir, $exceptiononerror)) { // Register a shutdown handler to remove the directory. - \core_shutdown_manager::register_function('remove_dir', array($requestdir)); + \core_shutdown_manager::register_function('remove_dir', [$dir]); } + + $requestdir = $dir; } return $requestdir; @@ -1663,13 +1672,18 @@ function get_request_storage_directory($exceptiononerror = true) { * This can only be used during the current request and will be tidied away * automatically afterwards. * - * A new, unique directory is always created within the current request directory. + * A new, unique directory is always created within a shared base request directory. * - * @param bool $exceptiononerror throw exception if error encountered - * @return string full path to directory if successful, false if not; may throw exception + * In some exceptional cases an alternative base directory may be required. This can be accomplished using the + * $forcecreate parameter. Typically this will only be requried where the file may be required during a shutdown handler + * which may or may not be registered after a previous request directory has been created. + * + * @param bool $exceptiononerror throw exception if error encountered + * @param bool $forcecreate Force creation of a new parent directory + * @return string The full path to directory if successful, false if not; may throw exception */ -function make_request_directory($exceptiononerror = true) { - $basedir = get_request_storage_directory($exceptiononerror); +function make_request_directory($exceptiononerror = true, bool $forcecreate = false) { + $basedir = get_request_storage_directory($exceptiononerror, $forcecreate); return make_unique_writable_directory($basedir, $exceptiononerror); }