From 84a449853fdd7da3b96fe40bcd7959bee34136b2 Mon Sep 17 00:00:00 2001 From: mjollnir_ Date: Wed, 6 Aug 2008 15:27:42 +0000 Subject: [PATCH] MDL-15777 - move all portfolio storage to portfolio_tempdata rather than session. This allows us to always have a unique id for a portfolio export which can be used as itemid for files api. Also it makes the session smaller :) --- lang/en_utf8/portfolio.php | 1 + lib/db/install.xml | 9 +-- lib/db/upgrade.php | 15 +++++ lib/portfoliolib.php | 114 ++++++++++++++++++++++++------------- mod/data/lib.php | 14 ++--- mod/glossary/lib.php | 31 +++++----- portfolio/add.php | 31 +++++----- version.php | 2 +- 8 files changed, 134 insertions(+), 83 deletions(-) diff --git a/lang/en_utf8/portfolio.php b/lang/en_utf8/portfolio.php index 82bc41382e..69dc3b71a0 100644 --- a/lang/en_utf8/portfolio.php +++ b/lang/en_utf8/portfolio.php @@ -31,6 +31,7 @@ $string['instanceismisconfigured'] = 'Portfolio instance is misconfigured, skipp $string['instancenotsaved'] = 'Failed to save portfolio'; $string['instancenotdelete'] = 'Failed to delete portfolio'; $string['instancesaved'] = 'Portfolio saved successfully'; +$string['invalidtempid'] = 'Invalid temporary id'; $string['invalidformat'] = 'Something is exporting an invalid format, $a'; $string['invalidinstance'] = 'Could not find that portfolio instance'; $string['manageportfolios'] = 'Manage portfolios'; diff --git a/lib/db/install.xml b/lib/db/install.xml index 5e999c9df6..82849ca627 100644 --- a/lib/db/install.xml +++ b/lib/db/install.xml @@ -1,5 +1,5 @@ - @@ -1764,10 +1764,11 @@ - +
- + + @@ -1877,4 +1878,4 @@ - + \ No newline at end of file diff --git a/lib/db/upgrade.php b/lib/db/upgrade.php index 5de1c13c62..cf219adcef 100644 --- a/lib/db/upgrade.php +++ b/lib/db/upgrade.php @@ -595,6 +595,21 @@ function xmldb_main_upgrade($oldversion=0) { upgrade_main_savepoint($result, 2008080500); } + if ($result && $oldversion < 2008080600) { + + $DB->delete_records('portfolio_tempdata'); // there shouldnt' be any, and it will cause problems with this upgrade. + /// Define field expirytime to be added to portfolio_tempdata + $table = new xmldb_table('portfolio_tempdata'); + $field = new xmldb_field('expirytime', XMLDB_TYPE_INTEGER, '10', XMLDB_UNSIGNED, XMLDB_NOTNULL, null, null, null, null, 'data'); + + /// Conditionally launch add field expirytime + if (!$dbman->field_exists($table, $field)) { + $dbman->add_field($table, $field); + } + + /// Main savepoint reached + upgrade_main_savepoint($result, 2008080600); + } return $result; } diff --git a/lib/portfoliolib.php b/lib/portfoliolib.php index 117cdc29e3..95390f366c 100644 --- a/lib/portfoliolib.php +++ b/lib/portfoliolib.php @@ -148,12 +148,16 @@ function portfolio_add_button($callbackclass, $callbackargs, $callbackfile=null, return; } - if (!empty($SESSION->portfoliointernal)) { + if (defined('PORTFOLIO_INTERNAL')) { // something somewhere has detected a risk of this being called during inside the preparation // eg forum_print_attachments return; } + if (isset($SESSION->portfolioexport)) { + print_error('alreadyexporting', 'portfolio'); + } + if (empty($callbackfile)) { $backtrace = debug_backtrace(); if (!array_key_exists(0, $backtrace) || !array_key_exists('file', $backtrace[0]) || !is_readable($backtrace[0]['file'])) { @@ -173,10 +177,6 @@ function portfolio_add_button($callbackclass, $callbackargs, $callbackfile=null, $callersupports = call_user_func(array($callbackclass, 'supported_formats')); - if (isset($SESSION->portfolio)) { - return portfolio_exporter::raise_error('alreadyexporting', 'portfolio'); - } - $output = '
' . "\n"; foreach ($callbackargs as $key => $value) { if (!empty($value) && !is_string($value) && !is_numeric($value)) { @@ -572,10 +572,11 @@ abstract class portfolio_caller_base { */ protected $user; + /** - * id if any of tempdata + * */ - private $tempdataid; + private $stage; /** * if this caller wants any additional config items @@ -710,23 +711,6 @@ abstract class portfolio_caller_base { } - protected function set_export_data($data) { - global $DB; - if ($this->tempdataid) { - $DB->set_field('portfolio_tempdata', 'data', serialize($data), array('id' => $this->tempdataid)); - } - $this->tempdataid = $DB->insert_record('portfolio_tempdata', (object)array('data' => serialize($data))); - } - - protected function get_export_data() { - global $DB; - if ($this->tempdataid) { - if ($data = $DB->get_field('portfolio_tempdata', 'data', array('id' => $this->tempdataid))) { - return unserialize($data); - } - } - return false; - } /** * Similar to the other allowed_config functions @@ -1654,6 +1638,11 @@ final class portfolio_exporter { public $instancefile; public $callerfile; + /** + * id of this export - matches record in portfolio_tempdata + */ + private $id; + /** * construct a new exporter for use * @@ -1715,9 +1704,10 @@ final class portfolio_exporter { * @return boolean whether or not to process the next stage. this is important as the function is called recursively. */ public function process_stage($stage, $alreadystolen=false) { - global $SESSION; if (!$alreadystolen && $url = $this->instance->steal_control($stage)) { - $SESSION->portfolio->stagepresteal = $stage; + $this->set('stage', $stage); + $this->save(); + //$SESSION->portfolio->stagepresteal = $stage; redirect($url); break; } @@ -1744,6 +1734,7 @@ final class portfolio_exporter { $stage++; return $this->process_stage($stage); } + $this->save(); return false; } @@ -1772,7 +1763,7 @@ final class portfolio_exporter { */ public function process_stage_config() { - global $SESSION; + //global $SESSION; $pluginobj = $callerobj = null; if ($this->instance->has_export_config()) { @@ -1806,9 +1797,12 @@ final class portfolio_exporter { ); $mform = new portfolio_export_form('', $customdata); if ($mform->is_cancelled()){ + $this->cancel_request(); + /* unset($SESSION->portfolio); redirect($this->caller->get_return_url()); exit; + */ } else if ($fromform = $mform->get_data()){ if (!confirm_sesskey()) { return $this->raise_error('confirmsesskeybad', '', $caller->get_return_url()); @@ -1924,8 +1918,8 @@ final class portfolio_exporter { global $SESSION; $wait = $this->instance->get_export_config('wait'); if (empty($wait)) { - events_trigger('portfolio_send', $this); - unset($SESSION->portfolio); + events_trigger('portfolio_send', $this->id); + unset($SESSION->portfolioexport); return $this->process_stage_finished(true); } return true; @@ -1959,15 +1953,13 @@ final class portfolio_exporter { * @return boolean whether or not to process the next stage. this is important as the control function is called recursively. */ public function process_stage_cleanup() { - global $CFG, $DB; + global $CFG, $DB, $SESSION; // @todo this is unpleasant. fix it. require_once($CFG->dirroot . '/backup/lib.php'); delete_dir_contents($this->tempdir); // @todo maybe add a hook in the plugin(s) - if ($this->caller->get('tempdataid')) { - $DB->delete_records('portfolio_tempdata', array('id' => $this->caller->get('tempdataid'))); - } - + $DB->delete_records('portfolio_tempdata', array('id' => $this->id)); + unset($SESSION->portfolioexport); return true; } @@ -2001,7 +1993,7 @@ final class portfolio_exporter { * @return boolean whether or not to process the next stage. this is important as the control function is called recursively. */ public function process_stage_finished($queued=false) { - global $SESSION; + //global $SESSION; $returnurl = $this->caller->get_return_url(); $continueurl = $this->instance->get_continue_url(); $extras = $this->instance->get_extra_finish_options(); @@ -2024,7 +2016,7 @@ final class portfolio_exporter { } } print_footer(); - unset($SESSION->portfolio); + //unset($SESSION->portfolio); return false; } @@ -2051,10 +2043,52 @@ final class portfolio_exporter { debugging(get_string($string, $module)); return false; } - global $SESSION; - unset($SESSION->portfolio); + if (isset($this)) { + $this->process_stage_cleanup(); + } + //global $SESSION; + //unset($SESSION->portfolio); print_error($string, $module, $continue); } + + public function cancel_request() { + if (!isset($this)) { + return; + } + $this->process_stage_cleanup(); + redirect($this->caller->get_return_url()); + exit; + } + + /** + * writes out the contents of this object and all its data to the portfolio_tempdata table and sets the 'id' field. + */ + public function save() { + global $DB; + if (empty($this->id)) { + $r = (object)array( + 'data' => base64_encode(serialize($this)), + 'expirytime' => time() + (60*60*24), + ); + $this->id = $DB->insert_record('portfolio_tempdata', $r); + } else { + $DB->set_field('portfolio_tempdata', 'data', base64_encode(serialize($this)), array('id' => $this->id)); + } + } + + public static function rewaken_object($id) { + global $DB, $CFG; + if (!$data = $DB->get_record('portfolio_tempdata', array('id' => $id))) { + portfolio_exporer::raise_error('invalidtempid', 'portfolio'); + } + $exporter = unserialize(base64_decode($data->data)); + if ($exporter->instancefile) { + require_once($CFG->dirroot . '/' . $exporter->instancefile); + } + require_once($CFG->dirroot . '/' . $exporter->callerfile); + $exporter = unserialize(serialize($exporter)); + return $exporter; + } } class portfolio_instance_select extends moodleform { @@ -2085,9 +2119,7 @@ class portfolio_instance_select extends moodleform { */ function portfolio_handle_event($eventdata) { global $CFG; - require_once($CFG->dirroot . '/' . $eventdata->instancefile); - require_once($CFG->dirroot . '/' . $eventdata->callerfile); - $exporter = unserialize(serialize($eventdata)); + $exporter = portfolio_exporter::rewaken_object($eventdata); $exporter->process_stage_package(); $exporter->process_stage_send(); $exporter->process_stage_cleanup(); diff --git a/mod/data/lib.php b/mod/data/lib.php index 50babb8bd1..05d7db0f21 100755 --- a/mod/data/lib.php +++ b/mod/data/lib.php @@ -2432,6 +2432,7 @@ class data_portfolio_caller extends portfolio_module_caller_base { private $fields; private $fieldtypes; private $delimiter; + private $exportdata; public function __construct($callbackargs) { global $DB; @@ -2455,7 +2456,7 @@ class data_portfolio_caller extends portfolio_module_caller_base { $this->fields[] = $tmp; $this->fieldtypes[] = $tmp->type; } - $this->set_export_data(data_get_exportdata($this->cm->instance, $this->fields, $this->selectedfields)); + $this->exportdata = data_get_exportdata($this->cm->instance, $this->fields, $this->selectedfields); } public function expected_time() { @@ -2465,7 +2466,7 @@ class data_portfolio_caller extends portfolio_module_caller_base { public function get_sha1() { $str = ''; - foreach ($this->get_export_data() as $data) { + foreach ($this->exportdata as $data) { $str .= implode(',', $data); } return sha1($str . ',' . $this->exporttype); @@ -2473,17 +2474,16 @@ class data_portfolio_caller extends portfolio_module_caller_base { public function prepare_package($tempdir) { global $DB; - $exportdata = $this->get_export_data(); - $count = count($exportdata); + $count = count($this->exportdata); switch ($this->exporttype) { case 'csv': - $return = data_export_csv($exportdata, $this->delimiter, $this->cm->name, $count, $tempdir); + $return = data_export_csv($this->exportdata, $this->delimiter, $this->cm->name, $count, $tempdir); break; case 'xls': - $return = data_export_xls($exportdata, $this->cm->name, $count, $tempdir); + $return = data_export_xls($this->exportdata, $this->cm->name, $count, $tempdir); break; case 'ods': - $return = data_export_ods($exportdata, $this->cm->name, $count, $tempdir); + $return = data_export_ods($this->exportdata, $this->cm->name, $count, $tempdir); break; } return $return; diff --git a/mod/glossary/lib.php b/mod/glossary/lib.php index 9de5062a56..5f46db4b5e 100644 --- a/mod/glossary/lib.php +++ b/mod/glossary/lib.php @@ -2384,6 +2384,7 @@ require_once($CFG->libdir . '/portfoliolib.php'); class glossary_csv_portfolio_caller extends portfolio_module_caller_base { private $glossary; + private $exportdata; public function __construct($callbackargs) { global $DB; @@ -2402,7 +2403,7 @@ class glossary_csv_portfolio_caller extends portfolio_module_caller_base { ON c.id = ec.categoryid WHERE ec.entryid ' . $where, $params); - $this->set_export_data(array('entries' => $entries, 'aliases' => $aliases, 'categoryentries' => $categoryentries)); + $this->exportdata = array('entries' => $entries, 'aliases' => $aliases, 'categoryentries' => $categoryentries); } public function expected_time() { @@ -2411,24 +2412,23 @@ class glossary_csv_portfolio_caller extends portfolio_module_caller_base { } public function get_sha1() { - return sha1(serialize($this->get_export_data())); + return sha1(serialize($this->exportdata)); } public function prepare_package($tempdir) { - $data = $this->get_export_data(); - $entries = $data['entries']; + $entries = $this->exportdata['entries']; $aliases = array(); $categories = array(); - if (is_array($data['aliases'])) { - foreach ($data['aliases'] as $alias) { + if (is_array($this->exportdata['aliases'])) { + foreach ($this->exportdata['aliases'] as $alias) { if (!array_key_exists($alias->entryid, $aliases)) { $aliases[$alias->entryid] = array(); } $aliases[$alias->entryid][] = $alias->alias; } } - if (is_array($data['categoryentries'])) { - foreach ($data['categoryentries'] as $cat) { + if (is_array($this->exportdata['categoryentries'])) { + foreach ($this->exportdata['categoryentries'] as $cat) { if (!array_key_exists($cat->entryid, $categories)) { $categories[$cat->entryid] = array(); } @@ -2470,13 +2470,14 @@ class glossary_entry_portfolio_caller extends portfolio_module_caller_base { || !$this->entry = $DB->get_record('glossary_entries', array('id' => $callbackargs['entryid']))) { portfolio_exporter::raise_error('noentry', 'glossary'); } + /* $aliases = $DB->get_records('glossary_alias', array('entryid' => $this->entry->id)); $categories = $DB->get_records_sql('SELECT ec.entryid, c.name FROM {glossary_entries_categories} ec JOIN {glossary_categories} c ON c.id = ec.categoryid WHERE ec.entryid = ?', array($this->entry->id)); - $this->set_export_data(array('entry' => $this->entry, 'aliases' => $aliases, 'categories' => $categories)); + */ } public function expected_time() { @@ -2493,14 +2494,11 @@ class glossary_entry_portfolio_caller extends portfolio_module_caller_base { } public function prepare_package($tempdir) { - global $SESSION; - $data = $this->get_export_data(); - $data['entry']->approved = true; // in case we don't have $USER which this function checks - $SESSION->portfoliointernal = true; + $this->entry->approved = true; // in case we don't have $USER which this function checks + define('PORTFOLIO_INTERNAL', true); ob_start(); - glossary_print_entry($this->get('course'), $this->cm, $this->glossary, $data['entry'], null, null, false); + glossary_print_entry($this->get('course'), $this->cm, $this->glossary, $this->entry, null, null, false); $content = ob_get_clean(); - $SESSION->portfoliointernal = false; // @todo - convert to files api. $status = ($handle = fopen($tempdir . '/' . clean_filename($this->entry->concept) . '.html', 'w')); $status = $status && fwrite($handle, $content); @@ -2509,8 +2507,7 @@ class glossary_entry_portfolio_caller extends portfolio_module_caller_base { } public function get_sha1() { - $data = $this->get_export_data(); - return sha1(serialize($data['entry']) . serialize($data['aliases']) . serialize($data['categories'])); + return sha1(serialize($this->entry)); } } diff --git a/portfolio/add.php b/portfolio/add.php index 91026ae1a3..240cc71963 100644 --- a/portfolio/add.php +++ b/portfolio/add.php @@ -8,18 +8,22 @@ if (empty($CFG->portfolioenabled)) { require_once($CFG->libdir . '/portfoliolib.php'); require_once($CFG->libdir . '/formslib.php'); $exporter = null; -if (isset($SESSION->portfolio) && isset($SESSION->portfolio->exporter)) { - $exporter = unserialize(serialize($SESSION->portfolio->exporter)); - if ($exporter->instancefile) { - require_once($CFG->dirroot . '/' . $exporter->instancefile); +$dataid = 0; + +if (!$dataid = optional_param('id') ) { + if (isset($SESSION->portfolioexport)) { + $dataid = $SESSION->portfolioexport; } - require_once($CFG->dirroot . '/' . $exporter->callerfile); - $exporter = unserialize(serialize($SESSION->portfolio->exporter)); - $SESSION->portfolio->exporter =& $exporter; +} +if ($dataid) { + $exporter = portfolio_exporter::rewaken_object($dataid); if ($cancel = optional_param('cancel', 0, PARAM_RAW)) { + $exporter->cancel_request(); + /* $returnurl = $exporter->get('caller')->get_return_url(); unset($SESSION->portfolio); redirect($returnurl); + */ } if (!$exporter->get('instance')) { if ($instance = optional_param('instance', '', PARAM_INT)) { @@ -70,12 +74,10 @@ if (isset($SESSION->portfolio) && isset($SESSION->portfolio->exporter)) { // for build navigation if (!$course = $caller->get('course')) { - echo 1; $course = optional_param('course', 0, PARAM_INT); } if (!empty($course) && is_numeric($course)) { - echo 2; $course = $DB->get_record('course', array('id' => $course), 'id,shortname,fullname'); // this is yuk but used in build_navigation } @@ -88,8 +90,8 @@ if (isset($SESSION->portfolio) && isset($SESSION->portfolio->exporter)) { $exporter = new portfolio_exporter($instance, $caller, $callbackfile, $navigation); $exporter->set('user', $USER); - $SESSION->portfolio = new StdClass; - $SESSION->portfolio->exporter =& $exporter; + $exporter->save(); + $SESSION->portfolioexport = $exporter->get('id'); } @@ -98,7 +100,7 @@ $alreadystolen = false; // for places returning control to pass (rather than PORTFOLIO_STAGE_PACKAGE // which is unstable if they can't get to the constant (eg external system) if ($postcontrol = optional_param('postcontrol', 0, PARAM_INT)) { - $stage = $SESSION->portfolio->stagepresteal; + $stage = $exporter->get('stage'); $exporter->instance()->post_control($stage, array_merge($_GET, $_POST)); $alreadystolen = true; } @@ -111,12 +113,15 @@ if (!$exporter->get('instance')) { // for the next block to catch $mform = new portfolio_instance_select('', array('caller' => $exporter->get('caller'))); if ($mform->is_cancelled()) { + $exporter->cancel_request(); + /* $returnurl = $caller->get_return_url(); unset($SESSION->portfolio); redirect($returnurl); exit; + */ } else if ($fromform = $mform->get_data()){ - redirect($CFG->wwwroot . '/portfolio/add.php?instance=' . $fromform->instance); + redirect($CFG->wwwroot . '/portfolio/add.php?instance=' . $fromform->instance . '&id=' . $exporter->get('id')); exit; } else { diff --git a/version.php b/version.php index 32a3dcdb71..5fb2bc6009 100644 --- a/version.php +++ b/version.php @@ -6,7 +6,7 @@ // This is compared against the values stored in the database to determine // whether upgrades should be performed (see lib/db/*.php) - $version = 2008080500; // YYYYMMDD = date of the last version bump + $version = 2008080600; // YYYYMMDD = date of the last version bump // XX = daily increments $release = '2.0 dev (Build: 20080806)'; // Human-friendly version name -- 2.39.5