Moodle
  1. Moodle
  2. MDL-33080

Restoring IMS-CC files, imsmanifest.xml is not looked within root folder

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.2, 2.3
    • Fix Version/s: 2.2.4
    • Component/s: Backup: IMS-CC
    • Labels:
      None
    • Testing Instructions:
      Hide

      1) Restore the attached course (backup-MDL-33080.mbz) into new course.
      2) Go to the restored course and backup it, ticking the "IMS Common Cartridge 1.1" checkbox.
      3) TEST: The process ends and one ".imscc" file is available.
      4) Restore that IMSCC package to new course.
      5) TEST: The format is properly detected and conversion happen.
      6) Continue with the restore process, ignoring any warning.
      5) TEST: The process ends without any fatal error. Warnings and borked results at the end can be ignored to tests this. Other issues have been created for them.

      Note: It's enough to test this under master only. For 22_STABLE the feature was hidden (via CFG setting) and no many changes are expected there.

      Show
      1) Restore the attached course (backup- MDL-33080 .mbz) into new course. 2) Go to the restored course and backup it, ticking the "IMS Common Cartridge 1.1" checkbox. 3) TEST: The process ends and one ".imscc" file is available. 4) Restore that IMSCC package to new course. 5) TEST: The format is properly detected and conversion happen. 6) Continue with the restore process, ignoring any warning. 5) TEST: The process ends without any fatal error. Warnings and borked results at the end can be ignored to tests this. Other issues have been created for them. Note: It's enough to test this under master only. For 22_STABLE the feature was hidden (via CFG setting) and no many changes are expected there.
    • Affected Branches:
      MOODLE_22_STABLE, MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_22_STABLE
    • Pull from Repository:
    • Pull Master Branch:
    • Rank:
      40298

      Description

      Hi,

      I was enabling IMS-CC export @ MDL-33079, and tried the obvious test:

      1) Create one course with a few activities (one scorm and one ims-cp resources included).
      2) Backup (export) it to IMS-CC format.
      3) Restore (import) it, to get the course back and compare.

      And 3 fails because cc112moodle::get_manifest() returns the scorm or ims-cp imsmanifest.xml files before looking for the root/imsmanifest.xml one (because of how DirectoryIterator returns the information).

      So I've created one small patch to enforce looking to the root directory before searching recursively. For your consideration.

      Ciao

        Issue Links

          Activity

          Hide
          Eloy Lafuente (stronk7) added a comment -

          Adding git source with the patch available. This requires to be fixed both in 22_STABLE and master.

          Show
          Eloy Lafuente (stronk7) added a comment - Adding git source with the patch available. This requires to be fixed both in 22_STABLE and master.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Darko, how does the patch sound?

          Show
          Eloy Lafuente (stronk7) added a comment - Darko, how does the patch sound?
          Hide
          Darko Miletic added a comment -

          What you did is correct and it can serve as patch for now but it does not really guarantee fulproof solution. We may have a case with 3 levels of directories where manifest file is on the third level, but there are also other manifests from some scorm content. And than the subdirectory where imsmanifest.xml from scorm has a name which is named in such way to appear earlier in scan than the real manifest and it will bomb again.

          The real solution would be to search for all instances of imsmanifest.xml and take one that works. Only in case where there is no imamanifest.xml at all or the existing instances are not common cartridge compliant than bomb.

          But let it go with this fix for now. There are still lot of changes awaiting to be integrated in the Moodle core. BTW when would you like to talk about next round of integrations?

          Show
          Darko Miletic added a comment - What you did is correct and it can serve as patch for now but it does not really guarantee fulproof solution. We may have a case with 3 levels of directories where manifest file is on the third level, but there are also other manifests from some scorm content. And than the subdirectory where imsmanifest.xml from scorm has a name which is named in such way to appear earlier in scan than the real manifest and it will bomb again. The real solution would be to search for all instances of imsmanifest.xml and take one that works. Only in case where there is no imamanifest.xml at all or the existing instances are not common cartridge compliant than bomb. But let it go with this fix for now. There are still lot of changes awaiting to be integrated in the Moodle core. BTW when would you like to talk about next round of integrations?
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Hi Darko,

          about this... I'going to create a new issue for the "real solution" and proceed by sending this to integration right now.

          About the changes awaiting... which ones are them? We are releasing 2.3 in weeks and, or we add them now... or they won't be available till 2.4? Bug fixes, improvements? Please point me to them.

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Hi Darko, about this... I'going to create a new issue for the "real solution" and proceed by sending this to integration right now. About the changes awaiting... which ones are them? We are releasing 2.3 in weeks and, or we add them now... or they won't be available till 2.4? Bug fixes, improvements? Please point me to them. Ciao
          Hide
          Eloy Lafuente (stronk7) added a comment -

          I've created MDL-33522 as followup for this, in order to perform complete detection of valid ims-cc manifests at any level.

          So I'm going to send this to integration as a quick fix for the root detection problem exclusively.

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - I've created MDL-33522 as followup for this, in order to perform complete detection of valid ims-cc manifests at any level. So I'm going to send this to integration as a quick fix for the root detection problem exclusively. Ciao
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Also, MDL-33523 has been created about the annoyances found when testing this, so can be ignored here.

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Also, MDL-33523 has been created about the annoyances found when testing this, so can be ignored here. Ciao
          Hide
          Sam Hemelryk added a comment -

          Thanks Eloy, integrated now

          Show
          Sam Hemelryk added a comment - Thanks Eloy, integrated now
          Hide
          Ankit Agarwal added a comment -

          I got the following error while trying to restore the attached course

          Debug info: Column 'itemid' cannot be null
          INSERT INTO mdl_backup_ids_temp (backupid,itemname,itemid,newitemid,parentitemid,info) VALUES(?,?,?,?,?,?)
          [array (
          0 => 'e1fe178b509d887f6cd4b3ea0e0432c8',
          1 => 'course_sectionnumber',
          2 => NULL,
          3 => 0,
          4 => NULL,
          5 => NULL,
          )]
          Error code: dmlwriteexception
          Stack trace:
          
              line 416 of /lib/dml/moodle_database.php: dml_write_exception thrown
              line 952 of /lib/dml/mysqli_native_moodle_database.php: call to moodle_database->query_end()
              line 994 of /lib/dml/mysqli_native_moodle_database.php: call to mysqli_native_moodle_database->insert_record_raw()
              line 269 of /backup/util/dbops/restore_dbops.class.php: call to mysqli_native_moodle_database->insert_record()
              line 1396 of /backup/util/dbops/restore_dbops.class.php: call to restore_dbops::set_backup_ids_cached()
              line 175 of /backup/util/plan/restore_structure_step.class.php: call to restore_dbops::set_backup_ids_record()
              line 1109 of /backup/moodle2/restore_stepslib.php: call to restore_structure_step->set_mapping()
              line 131 of /backup/util/plan/restore_structure_step.class.php: call to restore_section_structure_step->process_section()
              line 103 of /backup/util/helper/restore_structure_parser_processor.class.php: call to restore_structure_step->process()
              line 125 of /backup/util/xml/parser/processors/grouped_parser_processor.class.php: call to restore_structure_parser_processor->dispatch_chunk()
              line 91 of /backup/util/helper/restore_structure_parser_processor.class.php: call to grouped_parser_processor->postprocess_chunk()
              line 148 of /backup/util/xml/parser/processors/simplified_parser_processor.class.php: call to restore_structure_parser_processor->postprocess_chunk()
              line 92 of /backup/util/xml/parser/processors/progressive_parser_processor.class.php: call to simplified_parser_processor->process_chunk()
              line 169 of /backup/util/xml/parser/progressive_parser.class.php: call to progressive_parser_processor->receive_chunk()
              line 253 of /backup/util/xml/parser/progressive_parser.class.php: call to progressive_parser->publish()
              line ? of unknownfile: call to progressive_parser->end_tag()
              line 158 of /backup/util/xml/parser/progressive_parser.class.php: call to xml_parse()
              line 137 of /backup/util/xml/parser/progressive_parser.class.php: call to progressive_parser->parse()
              line 105 of /backup/util/plan/restore_structure_step.class.php: call to progressive_parser->process()
              line 153 of /backup/util/plan/base_task.class.php: call to restore_structure_step->execute()
              line 98 of /backup/moodle2/restore_section_task.class.php: call to base_task->execute()
              line 146 of /backup/util/plan/base_plan.class.php: call to restore_section_task->execute()
              line 157 of /backup/util/plan/restore_plan.class.php: call to base_plan->execute()
              line 315 of /backup/controller/restore_controller.class.php: call to restore_plan->execute()
              line 147 of /backup/util/ui/restore_ui.class.php: call to restore_controller->execute_plan()
              line 46 of /backup/restore.php: call to restore_ui->execute()
          
          Output buffer: Notice: Undefined variable: oldsection in /var/www/int/master/moodle/backup/moodle2/restore_stepslib.php on line 1109 
          

          Failing this sorry
          Thanks

          Show
          Ankit Agarwal added a comment - I got the following error while trying to restore the attached course Debug info: Column 'itemid' cannot be null INSERT INTO mdl_backup_ids_temp (backupid,itemname,itemid,newitemid,parentitemid,info) VALUES(?,?,?,?,?,?) [array ( 0 => 'e1fe178b509d887f6cd4b3ea0e0432c8', 1 => 'course_sectionnumber', 2 => NULL, 3 => 0, 4 => NULL, 5 => NULL, )] Error code: dmlwriteexception Stack trace: line 416 of /lib/dml/moodle_database.php: dml_write_exception thrown line 952 of /lib/dml/mysqli_native_moodle_database.php: call to moodle_database->query_end() line 994 of /lib/dml/mysqli_native_moodle_database.php: call to mysqli_native_moodle_database->insert_record_raw() line 269 of /backup/util/dbops/restore_dbops.class.php: call to mysqli_native_moodle_database->insert_record() line 1396 of /backup/util/dbops/restore_dbops.class.php: call to restore_dbops::set_backup_ids_cached() line 175 of /backup/util/plan/restore_structure_step.class.php: call to restore_dbops::set_backup_ids_record() line 1109 of /backup/moodle2/restore_stepslib.php: call to restore_structure_step->set_mapping() line 131 of /backup/util/plan/restore_structure_step.class.php: call to restore_section_structure_step->process_section() line 103 of /backup/util/helper/restore_structure_parser_processor.class.php: call to restore_structure_step->process() line 125 of /backup/util/xml/parser/processors/grouped_parser_processor.class.php: call to restore_structure_parser_processor->dispatch_chunk() line 91 of /backup/util/helper/restore_structure_parser_processor.class.php: call to grouped_parser_processor->postprocess_chunk() line 148 of /backup/util/xml/parser/processors/simplified_parser_processor.class.php: call to restore_structure_parser_processor->postprocess_chunk() line 92 of /backup/util/xml/parser/processors/progressive_parser_processor.class.php: call to simplified_parser_processor->process_chunk() line 169 of /backup/util/xml/parser/progressive_parser.class.php: call to progressive_parser_processor->receive_chunk() line 253 of /backup/util/xml/parser/progressive_parser.class.php: call to progressive_parser->publish() line ? of unknownfile: call to progressive_parser->end_tag() line 158 of /backup/util/xml/parser/progressive_parser.class.php: call to xml_parse() line 137 of /backup/util/xml/parser/progressive_parser.class.php: call to progressive_parser->parse() line 105 of /backup/util/plan/restore_structure_step.class.php: call to progressive_parser->process() line 153 of /backup/util/plan/base_task.class.php: call to restore_structure_step->execute() line 98 of /backup/moodle2/restore_section_task.class.php: call to base_task->execute() line 146 of /backup/util/plan/base_plan.class.php: call to restore_section_task->execute() line 157 of /backup/util/plan/restore_plan.class.php: call to base_plan->execute() line 315 of /backup/controller/restore_controller.class.php: call to restore_plan->execute() line 147 of /backup/util/ui/restore_ui.class.php: call to restore_controller->execute_plan() line 46 of /backup/restore.php: call to restore_ui->execute() Output buffer: Notice: Undefined variable: oldsection in /var/www/int/master/moodle/backup/moodle2/restore_stepslib.php on line 1109 Failing this sorry Thanks
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Oh my,

          that must be another regression introduced this week... that undefined $oldsection is the one leading to the error and, for sure, it's defined in moodle.git, but not in integration.git.

          Looking...

          Show
          Eloy Lafuente (stronk7) added a comment - Oh my, that must be another regression introduced this week... that undefined $oldsection is the one leading to the error and, for sure, it's defined in moodle.git, but not in integration.git. Looking...
          Hide
          Eloy Lafuente (stronk7) added a comment -

          The problem seems to be caused by one incorrect conflict resolution when merging MDL-33465 . It has re-introduced one line that should be deleted by that exact issue.

          So I'd recommend to:

          1) Move this back to "testable" status. Really the code introduced here is 100% independent of that error.
          2) Warn @ MDL-33465 because the incorrect conflict resolution is leading to all restore operations borked.

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - The problem seems to be caused by one incorrect conflict resolution when merging MDL-33465 . It has re-introduced one line that should be deleted by that exact issue. So I'd recommend to: 1) Move this back to "testable" status. Really the code introduced here is 100% independent of that error. 2) Warn @ MDL-33465 because the incorrect conflict resolution is leading to all restore operations borked. Ciao
          Hide
          Eloy Lafuente (stronk7) added a comment -

          MDL-33465 has been fixed and now this can be re-tested. Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - MDL-33465 has been fixed and now this can be re-tested. Ciao
          Hide
          Dan Marsden added a comment -

          passing test - no fatal errors occurred and course was restored.

          Show
          Dan Marsden added a comment - passing test - no fatal errors occurred and course was restored.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          We could celebrate it today... but better if we perform a bigger party after releasing Moodle 2.3.

          Print this message and come to Perth that day, it's valid for one beer, wine, coke or... water, as you wish.

          Many thanks for your collaboration!

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - We could celebrate it today... but better if we perform a bigger party after releasing Moodle 2.3. Print this message and come to Perth that day, it's valid for one beer, wine, coke or... water, as you wish. Many thanks for your collaboration! Ciao

            People

            • Votes:
              1 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: