From d0fb170520ea16bb5c745041ba2352e5511aa140 Mon Sep 17 00:00:00 2001 From: David Mudrak Date: Tue, 8 Feb 2011 16:37:54 +0100 Subject: [PATCH 1/3] MDL-26312 Workshop: fixed upgrade of module instances from 1.9 to 2.0 The patch changes workshop_upgrade_module_instances() so that it processes old workshop records ordered by their id and it keeps the instances id by using import_record() instead of insert_record(). Thanks to this, there is no mapping of workshop ids needed so the function workshop_upgrade_workshop_id_mappings() is dropped and all callers use plain workshop record. --- mod/workshop/db/upgradelib.php | 39 +++++-------------- .../form/accumulative/db/upgradelib.php | 3 +- mod/workshop/form/comments/db/upgradelib.php | 3 +- mod/workshop/form/numerrors/db/upgradelib.php | 5 +-- mod/workshop/form/rubric/db/upgradelib.php | 26 ++++++------- 5 files changed, 26 insertions(+), 50 deletions(-) diff --git a/mod/workshop/db/upgradelib.php b/mod/workshop/db/upgradelib.php index 8bc77f44048..4b79846f45a 100644 --- a/mod/workshop/db/upgradelib.php +++ b/mod/workshop/db/upgradelib.php @@ -154,13 +154,13 @@ function workshop_upgrade_module_instances() { upgrade_set_timeout(); $moduleid = $DB->get_field('modules', 'id', array('name' => 'workshop'), MUST_EXIST); - $rs = $DB->get_recordset_select('workshop_old', 'newid IS NULL'); + $rs = $DB->get_recordset_select('workshop_old', 'newid IS NULL', null, 'id'); foreach ($rs as $old) { $new = workshop_upgrade_transform_instance($old); - $newid = $DB->insert_record('workshop', $new, true, true); - $DB->set_field('course_modules', 'instance', $newid, array('module' => $moduleid, 'instance' => $old->id)); + $new->id = $old->id; + $DB->import_record('workshop', $new); $DB->set_field('workshop_old', 'newplugin', 'workshop', array('id' => $old->id)); - $DB->set_field('workshop_old', 'newid', $newid, array('id' => $old->id)); + $DB->set_field('workshop_old', 'newid', $new->id, array('id' => $old->id)); } $rs->close(); } @@ -214,24 +214,6 @@ function workshop_upgrade_transform_instance(stdClass $old) { return $new; } -/** - * Returns the list of new workshop instances ids - * - * @return array (int)oldid => (int)newid - */ -function workshop_upgrade_workshop_id_mappings() { - global $DB; - - $oldrecords = $DB->get_records('workshop_old', null, 'id', 'id,newid'); - $newids = array(); - foreach ($oldrecords as $oldid => $oldrecord) { - if ($oldrecord->id and $oldrecord->newid) { - $newids[$oldid] = $oldrecord->newid; - } - } - return $newids; -} - /** * Copies records from workshop_submissions_old into workshop_submissions. Can be called after all workshop module instances * were correctly migrated and new ids are filled in workshop_old @@ -242,19 +224,18 @@ function workshop_upgrade_submissions() { global $CFG, $DB; upgrade_set_timeout(); - $newworkshopids = workshop_upgrade_workshop_id_mappings(); - // list of teachers in every workshop: array of (int)oldworkshopid => array of (int)userid => notused + // list of teachers in every workshop: array of (int)workshopid => array of (int)userid => notused $workshopteachers = array(); $rs = $DB->get_recordset_select('workshop_submissions_old', 'newid IS NULL'); foreach ($rs as $old) { if (!isset($workshopteachers[$old->workshopid])) { - $cm = get_coursemodule_from_instance('workshop', $newworkshopids[$old->workshopid], 0, false, MUST_EXIST); + $cm = get_coursemodule_from_instance('workshop', $old->workshopid, 0, false, MUST_EXIST); $context = get_context_instance(CONTEXT_MODULE, $cm->id); $workshopteachers[$old->workshopid] = get_users_by_capability($context, 'mod/workshop:manage', 'u.id'); } - $new = workshop_upgrade_transform_submission($old, $newworkshopids[$old->workshopid], $workshopteachers[$old->workshopid]); + $new = workshop_upgrade_transform_submission($old, $old->workshopid, $workshopteachers[$old->workshopid]); $newid = $DB->insert_record('workshop_submissions', $new, true, true); $DB->set_field('workshop_submissions_old', 'newplugin', 'submissions', array('id' => $old->id)); $DB->set_field('workshop_submissions_old', 'newid', $newid, array('id' => $old->id)); @@ -343,17 +324,17 @@ function workshop_upgrade_assessments() { global $CFG, $DB, $OUTPUT; upgrade_set_timeout(); - $newworkshopids = workshop_upgrade_workshop_id_mappings(); + $newsubmissionids = workshop_upgrade_submission_id_mappings(); $teacherweights = workshop_upgrade_legacy_teacher_weights(); - // list of teachers in every workshop: array of (int)oldworkshopid => array of (int)userid => notused + // list of teachers in every workshop: array of (int)workshopid => array of (int)userid => notused $workshopteachers = array(); $rs = $DB->get_recordset_select('workshop_assessments_old', 'newid IS NULL'); foreach ($rs as $old) { if (!isset($workshopteachers[$old->workshopid])) { - $cm = get_coursemodule_from_instance('workshop', $newworkshopids[$old->workshopid], 0, false, MUST_EXIST); + $cm = get_coursemodule_from_instance('workshop', $old->workshopid, 0, false, MUST_EXIST); $context = get_context_instance(CONTEXT_MODULE, $cm->id); $workshopteachers[$old->workshopid] = get_users_by_capability($context, 'mod/workshop:manage', 'u.id'); } diff --git a/mod/workshop/form/accumulative/db/upgradelib.php b/mod/workshop/form/accumulative/db/upgradelib.php index 3376c864067..1e7237ec86d 100644 --- a/mod/workshop/form/accumulative/db/upgradelib.php +++ b/mod/workshop/form/accumulative/db/upgradelib.php @@ -49,11 +49,10 @@ function workshopform_accumulative_upgrade_legacy() { WHERE workshopid $workshopids AND newid IS NULL"; $rs = $DB->get_recordset_sql($sql, $params); - $newworkshopids = workshop_upgrade_workshop_id_mappings(); // prepare system (global) scales to replace the legacy in-built ones $newscaleids = workshopform_accumulative_upgrade_scales(); foreach ($rs as $old) { - $new = workshopform_accumulative_upgrade_element($old, $newscaleids, $newworkshopids[$old->workshopid]); + $new = workshopform_accumulative_upgrade_element($old, $newscaleids, $old->workshopid); $newid = $DB->insert_record('workshopform_accumulative', $new); $DB->set_field('workshop_elements_old', 'newplugin', 'accumulative', array('id' => $old->id)); $DB->set_field('workshop_elements_old', 'newid', $newid, array('id' => $old->id)); diff --git a/mod/workshop/form/comments/db/upgradelib.php b/mod/workshop/form/comments/db/upgradelib.php index f1e537bb6e1..ee45f2ad142 100644 --- a/mod/workshop/form/comments/db/upgradelib.php +++ b/mod/workshop/form/comments/db/upgradelib.php @@ -49,9 +49,8 @@ function workshopform_comments_upgrade_legacy() { WHERE workshopid $workshopids AND newid IS NULL"; $rs = $DB->get_recordset_sql($sql, $params); - $newworkshopids = workshop_upgrade_workshop_id_mappings(); foreach ($rs as $old) { - $new = workshopform_comments_upgrade_element($old, $newworkshopids[$old->workshopid]); + $new = workshopform_comments_upgrade_element($old, $old->workshopid); $newid = $DB->insert_record('workshopform_comments', $new); $DB->set_field('workshop_elements_old', 'newplugin', 'comments', array('id' => $old->id)); $DB->set_field('workshop_elements_old', 'newid', $newid, array('id' => $old->id)); diff --git a/mod/workshop/form/numerrors/db/upgradelib.php b/mod/workshop/form/numerrors/db/upgradelib.php index f7b07f86ddb..e27d61e303d 100644 --- a/mod/workshop/form/numerrors/db/upgradelib.php +++ b/mod/workshop/form/numerrors/db/upgradelib.php @@ -53,11 +53,10 @@ function workshopform_numerrors_upgrade_legacy() { WHERE workshopid $workshopids AND newid IS NULL"; $rs = $DB->get_recordset_sql($sql, $params); - $newworkshopids = workshop_upgrade_workshop_id_mappings(); foreach ($rs as $old) { // process the information about mapping $newmapping = new stdclass(); - $newmapping->workshopid = $newworkshopids[$old->workshopid]; + $newmapping->workshopid = $old->workshopid; $newmapping->nonegative = $old->elementno; $newmapping->grade = $old->maxscore; if ($old->maxscore > 0) { @@ -70,7 +69,7 @@ function workshopform_numerrors_upgrade_legacy() { $DB->insert_record('workshopform_numerrors_map', $newmapping); // process the information about the element itself if (trim($old->description) and $old->description <> '@@ GRADE_MAPPING_ELEMENT @@') { - $new = workshopform_numerrors_upgrade_element($old, $newworkshopids[$old->workshopid]); + $new = workshopform_numerrors_upgrade_element($old, $old->workshopid); $newid = $DB->insert_record('workshopform_numerrors', $new); } else { $newid = 0; diff --git a/mod/workshop/form/rubric/db/upgradelib.php b/mod/workshop/form/rubric/db/upgradelib.php index 40df767619e..50c4fbde447 100644 --- a/mod/workshop/form/rubric/db/upgradelib.php +++ b/mod/workshop/form/rubric/db/upgradelib.php @@ -57,25 +57,24 @@ function workshopform_rubric_upgrade_legacy_criterion() { WHERE workshopid $workshopids AND newid IS NULL"; $rs = $DB->get_recordset_sql($sql, $params); - $newworkshopids = workshop_upgrade_workshop_id_mappings(); - $newdimensionids = array(); // (int)oldworkshopid => (int)dimensionid + $newdimensionids = array(); // (int)workshopid => (int)dimensionid foreach ($rs as $old) { // create rubric criterion and the configuration if necessary if (!isset($newdimensionids[$old->workshopid])) { - if (!$DB->record_exists('workshopform_rubric', array('workshopid' => $newworkshopids[$old->workshopid], 'sort' => 1))) { + if (!$DB->record_exists('workshopform_rubric', array('workshopid' => $old->workshopid, 'sort' => 1))) { $newdimension = new stdclass(); - $newdimension->workshopid = $newworkshopids[$old->workshopid]; + $newdimension->workshopid = $old->workshopid; $newdimension->sort = 1; $newdimension->description = trim(get_string('dimensionnumber', 'workshopform_rubric', '')); $newdimension->descriptionformat = FORMAT_HTML; $newdimensionids[$old->workshopid] = $DB->insert_record('workshopform_rubric', $newdimension); } else { $newdimensionids[$old->workshopid] = $DB->get_field('workshopform_rubric', 'id', - array('workshopid' => $newworkshopids[$old->workshopid], 'sort' => 1)); + array('workshopid' => $old->workshopid, 'sort' => 1)); } - if (!$DB->record_exists('workshopform_rubric_config', array('workshopid' => $newworkshopids[$old->workshopid]))) { + if (!$DB->record_exists('workshopform_rubric_config', array('workshopid' => $old->workshopid))) { $newconfig = new stdclass(); - $newconfig->workshopid = $newworkshopids[$old->workshopid]; + $newconfig->workshopid = $old->workshopid; $newconfig->layout = 'list'; $DB->insert_record('workshopform_rubric_config', $newconfig); } @@ -148,27 +147,26 @@ function workshopform_rubric_upgrade_legacy_rubric() { AND r.newid IS NULL ORDER BY e.workshopid, e.elementno, r.rubricno"; $rs = $DB->get_recordset_sql($sql, $params); - $newworkshopids = workshop_upgrade_workshop_id_mappings(); // array of (int)oldworkshopid => (int)newworkshopid - $newdimensionids = array(); // (int)oldworkshopid => (int)elementno => (int)dimensionid + $newdimensionids = array(); // (int)workshopid => (int)elementno => (int)dimensionid $newlevelids = array(); // (int)oldrubricid => (int)newlevelid $prevelement = null; foreach ($rs as $old) { // create rubric criterion and the configuration if necessary if (!isset($newdimensionids[$old->workshopid]) or !isset($newdimensionids[$old->workshopid][$old->esort])) { - if (!$DB->record_exists('workshopform_rubric', array('workshopid' => $newworkshopids[$old->workshopid], 'sort' => $old->esort))) { + if (!$DB->record_exists('workshopform_rubric', array('workshopid' => $old->workshopid, 'sort' => $old->esort))) { $newdimension = new stdclass(); - $newdimension->workshopid = $newworkshopids[$old->workshopid]; + $newdimension->workshopid = $old->workshopid; $newdimension->sort = $old->esort; $newdimension->description = $old->edesc; $newdimension->descriptionformat = FORMAT_HTML; $newdimensionids[$old->workshopid][$old->esort] = $DB->insert_record('workshopform_rubric', $newdimension); } else { $newdimensionids[$old->workshopid][$old->esort] = $DB->get_field('workshopform_rubric', 'id', - array('workshopid' => $newworkshopids[$old->workshopid], 'sort' => $old->esort)); + array('workshopid' => $old->workshopid, 'sort' => $old->esort)); } - if (!$DB->record_exists('workshopform_rubric_config', array('workshopid' => $newworkshopids[$old->workshopid]))) { + if (!$DB->record_exists('workshopform_rubric_config', array('workshopid' => $old->workshopid))) { $newconfig = new stdclass(); - $newconfig->workshopid = $newworkshopids[$old->workshopid]; + $newconfig->workshopid = $old->workshopid; $newconfig->layout = 'grid'; $DB->insert_record('workshopform_rubric_config', $newconfig); } From f593111b677162e247d1d237c1cc0348f4a3bbdb Mon Sep 17 00:00:00 2001 From: David Mudrak Date: Fri, 11 Feb 2011 15:41:35 +0100 Subject: [PATCH 2/3] MDL-26312 Workshop: data integrity check to detect eventual course_modules corruption Because of a bug in Workshop upgrade code, multiple workshop course_modules can potentially point to a single workshop instance. The chance is pretty low as in most cases, the upgrade failed. But under certain circumstances, workshop could be upgraded with this data integrity issue. We want to detect it now and let the admin know. --- mod/workshop/db/upgrade.php | 42 +++++++++++++++++++++++++++++++++++++ mod/workshop/version.php | 4 ++-- 2 files changed, 44 insertions(+), 2 deletions(-) diff --git a/mod/workshop/db/upgrade.php b/mod/workshop/db/upgrade.php index 13f02d84200..34883cc0a24 100644 --- a/mod/workshop/db/upgrade.php +++ b/mod/workshop/db/upgrade.php @@ -264,5 +264,47 @@ function xmldb_workshop_upgrade($oldversion) { upgrade_mod_savepoint(true, 2010111200, 'workshop'); } + /** + * Check the course_module integrity - see MDL-26312 for details + * + * Because of a bug in Workshop upgrade code, multiple workshop course_modules can + * potentially point to a single workshop instance. The chance is pretty low as in most cases, + * the upgrade failed. But under certain circumstances, workshop could be upgraded with + * this data integrity issue. We want to detect it now and let the admin know. + */ + if ($oldversion < 2011021100) { + $sql = "SELECT cm.id, cm.course, cm.instance + FROM {course_modules} cm + WHERE cm.module IN (SELECT id + FROM {modules} + WHERE name = ?)"; + $rs = $DB->get_recordset_sql($sql, array('workshop')); + $map = array(); // returned stdClasses by instance id + foreach ($rs as $cm) { + $map[$cm->instance][$cm->id] = $cm; + } + $rs->close(); + + $problems = array(); + foreach ($map as $instanceid => $cms) { + if (count($cms) > 1) { + $problems[] = 'workshop instance ' . $instanceid . ' referenced by course_modules ' . implode(', ', array_keys($cms)); + } + } + if ($problems) { + echo $OUTPUT->notification('¡Ay, caramba! Data integrity corruption has been detected in your workshop ' . PHP_EOL . + 'module database tables. This might be caused by a bug in workshop upgrade code. ' . PHP_EOL . + 'Please report this issue immediately in workshop module support forum at ' . PHP_EOL . + 'http://moodle.org so that we can help to fix this problem. Please copy and keep ' . PHP_EOL . + 'following information for future reference:'); + foreach ($problems as $problem) { + echo $OUTPUT->notification($problem); + upgrade_log(UPGRADE_LOG_NOTICE, 'mod_workshop', 'course_modules integrity problem', $problem); + } + } + + upgrade_mod_savepoint(true, 2011021100, 'workshop'); + } + return true; } diff --git a/mod/workshop/version.php b/mod/workshop/version.php index 9cc8d8871ef..11348c1c9f6 100644 --- a/mod/workshop/version.php +++ b/mod/workshop/version.php @@ -29,6 +29,6 @@ defined('MOODLE_INTERNAL') || die(); -$module->version = 2010111200; -$module->requires = 2010111002; // Requires this Moodle version +$module->version = 2011021100; +$module->requires = 2011020900; // Requires this Moodle version //$module->cron = 60; From 0c3c0de198569c80e55e9bd9fa987969744fa182 Mon Sep 17 00:00:00 2001 From: David Mudrak Date: Mon, 14 Feb 2011 21:38:50 +0100 Subject: [PATCH 3/3] MDL-26312 Workshop: unset temporary arrays in upgrade code Spotted by Petr Skoda during the code review. --- mod/workshop/db/upgrade.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/mod/workshop/db/upgrade.php b/mod/workshop/db/upgrade.php index 34883cc0a24..df614c921d9 100644 --- a/mod/workshop/db/upgrade.php +++ b/mod/workshop/db/upgrade.php @@ -303,6 +303,8 @@ function xmldb_workshop_upgrade($oldversion) { } } + unset($problems); + unset($map); upgrade_mod_savepoint(true, 2011021100, 'workshop'); }