diff --git a/lib/db/upgrade.php b/lib/db/upgrade.php index 3b9f6d8c992..d70037f0d16 100644 --- a/lib/db/upgrade.php +++ b/lib/db/upgrade.php @@ -2857,5 +2857,20 @@ function xmldb_main_upgrade($oldversion) { upgrade_main_savepoint(true, 2017042600.01); } + if ($oldversion < 2017050300.01) { + // MDL-58684: + // Remove all portfolio_tempdata records as these may contain serialized \file_system type objects, which are now unable to + // be unserialized because of changes to the file storage API made in MDL-46375. Portfolio now stores an id reference to + // files instead of the object. + // These records are normally removed after a successful export, however, can be left behind if the user abandons the + // export attempt (a stale record). Additionally, each stale record cannot be reused and is normally cleaned up by the cron + // task core\task\portfolio_cron_task. Since the cron task tries to unserialize them, and generates a warning, we'll remove + // all records here. + $DB->delete_records_select('portfolio_tempdata', 'id > ?', [0]); + + // Main savepoint reached. + upgrade_main_savepoint(true, 2017050300.01); + } + return true; } diff --git a/lib/portfolio/plugin.php b/lib/portfolio/plugin.php index 34c40c8d3fd..18572f1e98e 100644 --- a/lib/portfolio/plugin.php +++ b/lib/portfolio/plugin.php @@ -637,6 +637,11 @@ abstract class portfolio_plugin_base { * @return array|string|int|boolean value of the field */ public final function get($field) { + // This is a legacy change to the way files are get/set. + // We now only set $this->file to the id of the \stored_file. So, we need to convert that id back to a \stored_file here. + if ($field === 'file') { + return $this->get_file(); + } if (property_exists($this, $field)) { return $this->{$field}; } @@ -654,6 +659,12 @@ abstract class portfolio_plugin_base { * @return bool */ public final function set($field, $value) { + // This is a legacy change to the way files are get/set. + // Make sure we never save the \stored_file object. Instead, use the id from $file->get_id() - set_file() does this for us. + if ($field === 'file') { + $this->set_file($value); + return true; + } if (property_exists($this, $field)) { $this->{$field} =& $value; $this->dirty = true; @@ -776,7 +787,7 @@ abstract class portfolio_plugin_push_base extends portfolio_plugin_base { */ abstract class portfolio_plugin_pull_base extends portfolio_plugin_base { - /** @var stdclass single file */ + /** @var int $file the id of a single file */ protected $file; /** @@ -827,4 +838,36 @@ abstract class portfolio_plugin_pull_base extends portfolio_plugin_base { $this->get('exporter')->log_transfer(); } + /** + * Sets the $file instance var to the id of the supplied \stored_file. + + * This helper allows the $this->get('file') call to return a \stored_file, but means that we only ever record an id reference + * in the $file instance var. + * + * @param \stored_file $file The stored_file instance. + * @return void + */ + protected function set_file(\stored_file $file) { + $fileid = $file->get_id(); + if (empty($fileid)) { + debugging('stored_file->id should not be empty'); + $this->file = null; + } else { + $this->file = $fileid; + } + } + + /** + * Gets the \stored_file object from the file id in the $file instance var. + * + * @return stored_file|null the \stored_file object if it exists, null otherwise. + */ + protected function get_file() { + if (!$this->file) { + return null; + } + // The get_file_by_id call can return false, so normalise to null. + $file = get_file_storage()->get_file_by_id($this->file); + return ($file) ? $file : null; + } } diff --git a/version.php b/version.php index bcd308b50fc..b631505ddbc 100644 --- a/version.php +++ b/version.php @@ -29,7 +29,7 @@ defined('MOODLE_INTERNAL') || die(); -$version = 2017050300.00; // YYYYMMDD = weekly release date of this DEV branch. +$version = 2017050300.01; // YYYYMMDD = weekly release date of this DEV branch. // RR = release increments - 00 in DEV branches. // .XX = incremental changes.