Skip to content

Commit

Permalink
MDL-51053 Restore: Static cache breaks unit tests
Browse files Browse the repository at this point in the history
  • Loading branch information
sammarshallou committed Aug 11, 2015
1 parent 57739a7 commit 27479b5
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 18 deletions.
16 changes: 12 additions & 4 deletions backup/moodle2/restore_stepslib.php
Original file line number Diff line number Diff line change
Expand Up @@ -1428,6 +1428,15 @@ protected function define_execution() {
* as needed, rebuilding course cache and other friends
*/
class restore_section_structure_step extends restore_structure_step {
/** @var array Cache: Array of id => course format */
private static $courseformats = array();

/**
* Resets a static cache of course formats. Required for unit testing.
*/
public static function reset_caches() {
self::$courseformats = array();
}

protected function define_structure() {
global $CFG;
Expand Down Expand Up @@ -1600,14 +1609,13 @@ public function process_availability_field($data) {

public function process_course_format_options($data) {
global $DB;
static $courseformats = array();
$courseid = $this->get_courseid();
if (!array_key_exists($courseid, $courseformats)) {
if (!array_key_exists($courseid, self::$courseformats)) {
// It is safe to have a static cache of course formats because format can not be changed after this point.
$courseformats[$courseid] = $DB->get_field('course', 'format', array('id' => $courseid));
self::$courseformats[$courseid] = $DB->get_field('course', 'format', array('id' => $courseid));
}
$data = (array)$data;
if ($courseformats[$courseid] === $data['format']) {
if (self::$courseformats[$courseid] === $data['format']) {
// Import section format options only if both courses (the one that was backed up
// and the one we are restoring into) have same formats.
$params = array(
Expand Down
48 changes: 34 additions & 14 deletions backup/moodle2/tests/moodle2_course_format_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ public function test_course_format_options_import_myself() {
'numdaystocomplete' => 2);
$courseobject->update_section_format_options($data);

$this->backup_and_restore($course, $course);
$this->backup_and_restore($course, $course, backup::TARGET_EXISTING_ADDING);

$sectionoptions = $courseobject->get_format_options(1);
$this->assertArrayHasKey('numdaystocomplete', $sectionoptions);
Expand All @@ -126,47 +126,61 @@ public function test_course_format_options_restore_new_format() {

$this->resetAfterTest(true);
$this->setAdminUser();
$CFG->enableavailability = true;
$CFG->enablecompletion = true;

// Create a course with some availability data set.
// Create a source course using the test_cs2_options format.
$generator = $this->getDataGenerator();
$course = $generator->create_course(
array('format' => 'test_cs2_options', 'numsections' => 3,
'enablecompletion' => COMPLETION_ENABLED),
array('createsections' => true));

// Create a target course using test_cs_options format.
$newcourse = $generator->create_course(
array('format' => 'test_cs_options', 'numsections' => 3,
'enablecompletion' => COMPLETION_ENABLED),
array('createsections' => true));

// Set section 2 to have both options, and a name.
$courseobject = format_base::instance($course->id);
$section = $DB->get_record('course_sections',
array('course' => $course->id, 'section' => 1), '*', MUST_EXIST);

array('course' => $course->id, 'section' => 2), '*', MUST_EXIST);
$data = array('id' => $section->id,
'numdaystocomplete' => 2,
'secondparameter' => 8);
'secondparameter' => 8
);
$courseobject->update_section_format_options($data);
// Backup and restore it.
$this->backup_and_restore($course, $newcourse);
$DB->set_field('course_sections', 'name', 'Frogs', array('id' => $section->id));

// Backup and restore to the new course using 'add to existing' so it
// keeps the current (test_cs_options) format.
$this->backup_and_restore($course, $newcourse, backup::TARGET_EXISTING_ADDING);

// Check that the section contains the options suitable for the new
// format and that even the one with the same name as from the old format
// has NOT been set.
$newcourseobject = format_base::instance($newcourse->id);
$sectionoptions = $newcourseobject->get_format_options(1);
$sectionoptions = $newcourseobject->get_format_options(2);
$this->assertArrayHasKey('numdaystocomplete', $sectionoptions);
$this->assertArrayHasKey('secondparameter', $sectionoptions);
$this->assertArrayNotHasKey('secondparameter', $sectionoptions);
$this->assertEquals(0, $sectionoptions['numdaystocomplete']);
$this->assertEquals(0, $sectionoptions['secondparameter']);

// However, the name should have been changed, as this does not depend
// on the format.
$modinfo = get_fast_modinfo($newcourse->id);
$section = $modinfo->get_section_info(2);
$this->assertEquals('Frogs', $section->name);
}

/**
* Backs a course up and restores it.
*
* @param stdClass $srccourse Course object to backup
* @param stdClass $dstcourse Course object to restore into
* @param int $target Target course mode (backup::TARGET_xx)
* @return int ID of newly restored course
*/
protected function backup_and_restore($srccourse, $dstcourse = null) {
protected function backup_and_restore($srccourse, $dstcourse = null,
$target = backup::TARGET_NEW_COURSE) {
global $USER, $CFG;

// Turn off file logging, otherwise it can't delete the file (Windows).
Expand All @@ -190,7 +204,7 @@ protected function backup_and_restore($srccourse, $dstcourse = null) {
}
$rc = new restore_controller($backupid, $newcourseid,
backup::INTERACTIVE_NO, backup::MODE_GENERAL, $USER->id,
backup::TARGET_NEW_COURSE);
$target);

$this->assertTrue($rc->execute_precheck());
$rc->execute_plan();
Expand Down Expand Up @@ -226,6 +240,12 @@ public function section_format_options($foreditform = false) {
class format_test_cs2_options extends format_test_cs_options {
public function section_format_options($foreditform = false) {
return array(
'numdaystocomplete' => array(
'type' => PARAM_INT,
'label' => 'Test days',
'element_type' => 'text',
'default' => 0,
),
'secondparameter' => array(
'type' => PARAM_INT,
'label' => 'Test Parmater',
Expand Down
5 changes: 5 additions & 0 deletions lib/phpunit/classes/util.php
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,11 @@ public static function reset_all_data($detectchanges = false) {
\core\update\deployer::reset_caches(true);
}

// Clear static cache within restore.
if (class_exists('restore_section_structure_step')) {
restore_section_structure_step::reset_caches();
}

// purge dataroot directory
self::reset_dataroot();

Expand Down

0 comments on commit 27479b5

Please sign in to comment.