From eb78f0632da3203d609f46bdff903f7ce963a67e Mon Sep 17 00:00:00 2001 From: "Eloy Lafuente (stronk7)" Date: Mon, 23 May 2011 00:58:45 +0200 Subject: [PATCH] MDL-27594 backup - fix start/end notifications on empty chunks --- .../grouped_parser_processor.class.php | 21 ++- .../simplified_parser_processor.class.php | 81 ++++++++++- .../xml/parser/simpletest/fixtures/test5.xml | 24 ++++ .../util/xml/parser/simpletest/testparser.php | 127 +++++++++++++++++- 4 files changed, 242 insertions(+), 11 deletions(-) create mode 100644 backup/util/xml/parser/simpletest/fixtures/test5.xml diff --git a/backup/util/xml/parser/processors/grouped_parser_processor.class.php b/backup/util/xml/parser/processors/grouped_parser_processor.class.php index 46391e62975..d8e260ad01a 100644 --- a/backup/util/xml/parser/processors/grouped_parser_processor.class.php +++ b/backup/util/xml/parser/processors/grouped_parser_processor.class.php @@ -71,10 +71,23 @@ abstract class grouped_parser_processor extends simplified_parser_processor { } /** - * Dispatch grouped chunks safely once their end tag happens. - * Also notify end of path if selected and not under grouped + * The parser fires this each time one path is going to be parsed + * + * @param string $path xml path which parsing has started + */ + public function before_path($path) { + if (!$this->grouped_parent_exists($path)) { + parent::before_path($path); + } + } + + /** + * The parser fires this each time one path has been parsed + * + * @param string $path xml path which parsing has ended */ public function after_path($path) { + // Have finished one grouped path, dispatch it if ($this->path_is_grouped($path)) { // Any accumulated information must be in // currentdata, properly built @@ -85,7 +98,7 @@ abstract class grouped_parser_processor extends simplified_parser_processor { } // Normal notification of path end // Only if path is selected and not child of grouped - if ($this->path_is_selected($path) && !$this->grouped_parent_exists($path)) { + if (!$this->grouped_parent_exists($path)) { parent::after_path($path); } } @@ -100,7 +113,6 @@ abstract class grouped_parser_processor extends simplified_parser_processor { $path = $data['path']; // If the chunk is a grouped one, simply put it into currentdata if ($this->path_is_grouped($path)) { - $this->notify_path_start($path); $this->currentdata[$path] = $data; // If the chunk is child of grouped one, add it to currentdata @@ -110,7 +122,6 @@ abstract class grouped_parser_processor extends simplified_parser_processor { // No grouped nor child of grouped, dispatch it } else { - $this->notify_path_start($path); $this->dispatch_chunk($data); } } diff --git a/backup/util/xml/parser/processors/simplified_parser_processor.class.php b/backup/util/xml/parser/processors/simplified_parser_processor.class.php index 0544b436445..c65a4be093f 100644 --- a/backup/util/xml/parser/processors/simplified_parser_processor.class.php +++ b/backup/util/xml/parser/processors/simplified_parser_processor.class.php @@ -41,12 +41,14 @@ abstract class simplified_parser_processor extends progressive_parser_processor protected $paths; // array of paths we are interested on protected $parentpaths; // array of parent paths of the $paths protected $parentsinfo; // array of parent attributes to be added as child tags + protected $startendinfo;// array (stack) of startend information - public function __construct(array $paths) { + public function __construct(array $paths = array()) { parent::__construct(); $this->paths = array(); $this->parentpaths = array(); $this->parentsinfo = array(); + $this->startendinfo = array(); // Add paths and parentpaths. We are looking for attributes there foreach ($paths as $key => $path) { $this->add_path($path); @@ -95,6 +97,10 @@ abstract class simplified_parser_processor extends progressive_parser_processor // If the path is a registered one, let's process it if ($this->path_is_selected($path)) { + + // Send all the pending notify_path_start/end() notifications + $this->process_pending_startend_notifications($path, 'start'); + // First of all, look for attributes available at parentsinfo // in order to get them available as normal tags if (isset($this->parentsinfo[$parentpath][$tag]['attrs'])) { @@ -146,22 +152,87 @@ abstract class simplified_parser_processor extends progressive_parser_processor } else { $this->chunks--; // Chunk skipped } + return true; } /** - * The parser fires this each time one path has been parsed + * The parser fires this each time one path is going to be parsed + * + * @param string $path xml path which parsing has started */ - public function after_path($path) { + public function before_path($path) { if ($this->path_is_selected($path)) { - $this->notify_path_end($path); + $this->startendinfo[] = array('path' => $path, 'action' => 'start'); } } + /** + * The parser fires this each time one path has been parsed + * + * @param string $path xml path which parsing has ended + */ + public function after_path($path) { + $toprocess = false; + // If the path being closed matches (same or parent) the last path in the stack + // we process pending startend notifications until one matching end is found + if ($element = reset($this->startendinfo)) { + $elepath = $element['path']; + $eleaction = $element['action']; + if ($eleaction = 'end' && strpos($elepath, $path) === 0) { + $toprocess = true; + } + + // Also, if the stack of startend notifications is empty, we can process current end + // path safely + } else { + $toprocess = true; + } + if ($this->path_is_selected($path)) { + $this->startendinfo[] = array('path' => $path, 'action' => 'end'); + } + // Send all the pending startend notifications if decided to do so + if ($toprocess) { + $this->process_pending_startend_notifications($path, 'end'); + } + } + + // Protected API starts here + /** + * Adjust start/end til finding one match start/end path (included) + * + * This will trigger all the pending {@see notify_path_start} and + * {@see notify_path_end} calls for one given path and action + * + * @param string path the path to look for as limit + * @param string action the action to look for as limit + */ + protected function process_pending_startend_notifications($path, $action) { + + // Iterate until one matching path and action is found (or the array is empty) + $elecount = count($this->startendinfo); + $elematch = false; + while ($elecount > 0 && !$elematch) { + $element = array_shift($this->startendinfo); + $elecount--; + $elepath = $element['path']; + $eleaction = $element['action']; + + if ($elepath == $path && $eleaction == $action) { + $elematch = true; + } + + if ($eleaction == 'start') { + $this->notify_path_start($elepath); + } else { + $this->notify_path_end($elepath); + } + } + } + protected function postprocess_chunk($data) { - $this->notify_path_start($data['path']); $this->dispatch_chunk($data); } diff --git a/backup/util/xml/parser/simpletest/fixtures/test5.xml b/backup/util/xml/parser/simpletest/fixtures/test5.xml new file mode 100644 index 00000000000..9904c3ba8e0 --- /dev/null +++ b/backup/util/xml/parser/simpletest/fixtures/test5.xml @@ -0,0 +1,24 @@ + + + +
+ 200 + + + 300 + + + + + + + + 500 + + + + +
+ + + diff --git a/backup/util/xml/parser/simpletest/testparser.php b/backup/util/xml/parser/simpletest/testparser.php index 02983041833..589f8c939b9 100644 --- a/backup/util/xml/parser/simpletest/testparser.php +++ b/backup/util/xml/parser/simpletest/testparser.php @@ -339,6 +339,73 @@ class progressive_parser_test extends UnitTestCase { $this->assertEqual($errcount, 0); // No errors found, plz } + /** + * test how the simplified processor and the order of start/process/end events happens + * with one real fragment of one backup 1.9 file, where some problems + * were found by David, hence we honor him in the name of the test ;-) + */ + function test_simplified_david_backup19_file_fragment() { + global $CFG; + // Instantiate progressive_parser + $pp = new progressive_parser(); + // Instantiate grouped_parser_processor + $pr = new mock_simplified_parser_processor(); + // Add interesting paths + $pr->add_path('/MOODLE_BACKUP/COURSE'); + $pr->add_path('/MOODLE_BACKUP/COURSE/SECTIONS/SECTION'); + $pr->add_path('/MOODLE_BACKUP/COURSE/SECTIONS/SECTION/MODS/MOD'); + $pr->add_path('/MOODLE_BACKUP/COURSE/SECTIONS/SECTION/MODS/MOD/ROLES_OVERRIDES'); + $this->assertTrue($pr instanceof progressive_parser_processor); + // Assign processor to parser + $pp->set_processor($pr); + // Set file from fixtures + $pp->set_file($CFG->dirroot . '/backup/util/xml/parser/simpletest/fixtures/test5.xml'); + // Process the file + $pp->process(); + + // Get all the simplified chunks and perform various validations + $chunks = $pr->get_chunks(); + $this->assertEqual(count($chunks), 3); // Only 3, because 7 (COURSE, ROLES_OVERRIDES and 5 MOD) are empty, aka no chunk + + // Now check start notifications + $snotifs = $pr->get_start_notifications(); + // Check we have received the correct number of notifications + $this->assertEqual(count($snotifs), 10); // Start tags are dispatched for empties (ROLES_OVERRIDES) + // Check first and last notifications + $this->assertEqual($snotifs[0], '/MOODLE_BACKUP/COURSE'); + $this->assertEqual($snotifs[1], '/MOODLE_BACKUP/COURSE/SECTIONS/SECTION'); + $this->assertEqual($snotifs[2], '/MOODLE_BACKUP/COURSE/SECTIONS/SECTION/MODS/MOD'); + $this->assertEqual($snotifs[3], '/MOODLE_BACKUP/COURSE/SECTIONS/SECTION/MODS/MOD/ROLES_OVERRIDES'); + $this->assertEqual($snotifs[7], '/MOODLE_BACKUP/COURSE/SECTIONS/SECTION/MODS/MOD'); + $this->assertEqual($snotifs[8], '/MOODLE_BACKUP/COURSE/SECTIONS/SECTION/MODS/MOD'); + $this->assertEqual($snotifs[9], '/MOODLE_BACKUP/COURSE/SECTIONS/SECTION/MODS/MOD'); + + // Now check end notifications + $enotifs = $pr->get_end_notifications(); + // Check we have received the correct number of notifications + $this->assertEqual(count($snotifs), 10); // End tags are dispatched for empties (ROLES_OVERRIDES) + // Check first, and last notifications + $this->assertEqual($enotifs[0], '/MOODLE_BACKUP/COURSE/SECTIONS/SECTION/MODS/MOD/ROLES_OVERRIDES'); + $this->assertEqual($enotifs[1], '/MOODLE_BACKUP/COURSE/SECTIONS/SECTION/MODS/MOD'); + $this->assertEqual($enotifs[2], '/MOODLE_BACKUP/COURSE/SECTIONS/SECTION/MODS/MOD'); + $this->assertEqual($enotifs[3], '/MOODLE_BACKUP/COURSE/SECTIONS/SECTION/MODS/MOD'); + $this->assertEqual($enotifs[7], '/MOODLE_BACKUP/COURSE/SECTIONS/SECTION/MODS/MOD'); + $this->assertEqual($enotifs[8], '/MOODLE_BACKUP/COURSE/SECTIONS/SECTION'); + $this->assertEqual($enotifs[9], '/MOODLE_BACKUP/COURSE'); + + // Check start and end notifications are balanced + sort($snotifs); + sort($enotifs); + $this->assertEqual($snotifs, $enotifs); + + // Now verify that the start/process/end order is correct + $allnotifs = $pr->get_all_notifications(); + $this->assertEqual(count($allnotifs), count($snotifs) + count($enotifs) + count($chunks)); // The count + // Check integrity of the notifications + $errcount = $this->helper_check_notifications_order_integrity($allnotifs); + $this->assertEqual($errcount, 0); // No errors found, plz + } + /* * test progressive_parser parsing results using grouped_parser_processor and test4.xml * (one simple glossary backup file example) @@ -514,6 +581,64 @@ class progressive_parser_test extends UnitTestCase { $this->assertEqual($errcount, 0); // No errors found, plz } + /** + * test how the grouped processor and the order of start/process/end events happens + * with one real fragment of one backup 1.9 file, where some problems + * were found by David, hence we honor him in the name of the test ;-) + */ + function test_grouped_david_backup19_file_fragment() { + global $CFG; + // Instantiate progressive_parser + $pp = new progressive_parser(); + // Instantiate grouped_parser_processor + $pr = new mock_grouped_parser_processor(); + // Add interesting paths + $pr->add_path('/MOODLE_BACKUP/COURSE'); + $pr->add_path('/MOODLE_BACKUP/COURSE/SECTIONS/SECTION', true); + $pr->add_path('/MOODLE_BACKUP/COURSE/SECTIONS/SECTION/MODS/MOD'); + $pr->add_path('/MOODLE_BACKUP/COURSE/SECTIONS/SECTION/MODS/MOD/ROLES_OVERRIDES'); + $this->assertTrue($pr instanceof progressive_parser_processor); + // Assign processor to parser + $pp->set_processor($pr); + // Set file from fixtures + $pp->set_file($CFG->dirroot . '/backup/util/xml/parser/simpletest/fixtures/test5.xml'); + // Process the file + $pp->process(); + + // Get all the simplified chunks and perform various validations + $chunks = $pr->get_chunks(); + $this->assertEqual(count($chunks), 1); // Only 1, the SECTION one + + // Now check start notifications + $snotifs = $pr->get_start_notifications(); + // Check we have received the correct number of notifications + $this->assertEqual(count($snotifs), 2); + // Check first and last notifications + $this->assertEqual($snotifs[0], '/MOODLE_BACKUP/COURSE'); + $this->assertEqual($snotifs[1], '/MOODLE_BACKUP/COURSE/SECTIONS/SECTION'); + + // Now check end notifications + $enotifs = $pr->get_end_notifications(); + // Check we have received the correct number of notifications + $this->assertEqual(count($snotifs), 2); // End tags are dispatched for empties (ROLES_OVERRIDES) + // Check first, and last notifications + $this->assertEqual($enotifs[0], '/MOODLE_BACKUP/COURSE/SECTIONS/SECTION'); + $this->assertEqual($enotifs[1], '/MOODLE_BACKUP/COURSE'); + + // Check start and end notifications are balanced + sort($snotifs); + sort($enotifs); + $this->assertEqual($snotifs, $enotifs); + + // Now verify that the start/process/end order is correct + $allnotifs = $pr->get_all_notifications(); + $this->assertEqual(count($allnotifs), count($snotifs) + count($enotifs) + count($chunks)); // The count + // Check integrity of the notifications + $errcount = $this->helper_check_notifications_order_integrity($allnotifs); + $this->assertEqual($errcount, 0); // No errors found, plz + } + + /** * Helper function that given one array of ordered start/process/end notifications will * check it of integrity like: @@ -545,7 +670,7 @@ class progressive_parser_test extends UnitTestCase { $notifpile[$notifpath] = 'process'; // Update the status in the pile break; case 'end': - if ($lastpilepath != $notifpath or $lastpiletype != 'process') { + if ($lastpilepath != $notifpath or ($lastpiletype != 'process' and $lastpiletype != 'start')) { $numerrors++; // Only process for same path is allowed before end } unset($notifpile[$notifpath]); // Delete from the pile