Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-26229

Invalid input syntax caused by repeated tag elements on restore (quiz, lesson)

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Blocker
    • Resolution: Fixed
    • Affects Version/s: 2.0, 2.0.1
    • Fix Version/s: 2.0.2
    • Component/s: Backup, Quiz
    • Labels:
    • Affected Branches:
      MOODLE_20_STABLE
    • Fixed Branches:
      MOODLE_20_STABLE

      Description

      It looks like this error only occurs when you try to restore a course with quizzes and a quiz has no attempts. An invalid input syntax is returned with the full error looking like:

      ERROR: invalid input syntax for integer: "
      "
      INSERT INTO mdl_quiz
      (name,intro,introformat,timeopen,timeclose,optionflags,penaltyscheme,attempts,attemptonlast,grademethod,decimalpoints,questiondecimalpoints,review,questionsperpage,shufflequestions,shuffleanswers,questions,sumgrades,grade,timecreated,timemodified,timelimit,password,subnet,popup,delay1,delay2,showuserpicture,showblocks,course)
      VALUES($1,$2,$3,$4,$5,$6,$7,$8,$9,$10,$11,$12,$13,$14,$15,$16,$17,$18,$19,$20,$21,$22,$23,$24,$25,$26,$27,$28,$29,$30)
      RETURNING id
      [array (
      'name' => 'arrays',
      'intro' => '',
      'introformat' => '1',
      'timeopen' => 1292338151,
      'timeclose' => '0',
      'optionflags' => '0',
      'penaltyscheme' => '0',
      'attempts' => '
      ',
      'attemptonlast' => '0',
      'grademethod' => '3',
      'decimalpoints' => '2',
      'questiondecimalpoints' => '2',
      'review' => '71760879',
      'questionsperpage' => '0',
      'shufflequestions' => '0',
      'shuffleanswers' => '0',
      'questions' => '',
      'sumgrades' => '0.00000',
      'grade' => '0.00000',
      'timecreated' => 1295837491,
      'timemodified' => 1295837491,
      'timelimit' => '3600',
      'password' => '',
      'subnet' => '',
      'popup' => '1',
      'delay1' => '0',
      'delay2' => '0',
      'showuserpicture' => '0',
      'showblocks' => '0',
      'course' => 48,
      )]

      You can see where the 'attempts' elements is, it seems to contain a new line escape character which breaks the input rule of it only allowing an integer.

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            jaseeey Jason Ilicic added a comment -

            Added an if statement in the restore library which checks if the 'attempts' element is numeric... if not, then it sets it to false because it's not needed.

            Patch can be found at: https://github.com/jasonilicic/moodle/commit/c9cf46bfa84afe86970695f56cdfc7e2aaafeef5

            Show
            jaseeey Jason Ilicic added a comment - Added an if statement in the restore library which checks if the 'attempts' element is numeric... if not, then it sets it to false because it's not needed. Patch can be found at: https://github.com/jasonilicic/moodle/commit/c9cf46bfa84afe86970695f56cdfc7e2aaafeef5
            Hide
            tsala Helen Foster added a comment -

            Jason, thanks for your report and patch. Increasing priority to blocker as the problem results in an error.

            Show
            tsala Helen Foster added a comment - Jason, thanks for your report and patch. Increasing priority to blocker as the problem results in an error.
            Hide
            ashleyholman Ashley Holman added a comment -

            The problem occurs because the quiz XML has two <attempts> elements. One set to an integer (the quiz.attempts settings), and the other one is a nested tag with child <attempt> tags.

            Depending on the order which they appear in the XML, the restore parser might grab the nested tag which will return an empty string if there are no attempts in the backup file.

            Perhaps need to rename these tags to use different names.

            Show
            ashleyholman Ashley Holman added a comment - The problem occurs because the quiz XML has two <attempts> elements. One set to an integer (the quiz.attempts settings), and the other one is a nested tag with child <attempt> tags. Depending on the order which they appear in the XML, the restore parser might grab the nested tag which will return an empty string if there are no attempts in the backup file. Perhaps need to rename these tags to use different names.
            Hide
            ashleyholman Ashley Holman added a comment -

            note the linked patch is a workaround not a fix since it will force the attempts setting to be 0 when it could have been some other integer.

            Show
            ashleyholman Ashley Holman added a comment - note the linked patch is a workaround not a fix since it will force the attempts setting to be 0 when it could have been some other integer.
            Hide
            dougiamas Martin Dougiamas added a comment -

            Tim and Eloy, can you advise on some good names?

            Show
            dougiamas Martin Dougiamas added a comment - Tim and Eloy, can you advise on some good names?
            Hide
            aolley Adam Olley added a comment -

            Example xml from a quiz.xml generated during a course backup:

            <?xml version="1.0" encoding="UTF-8"?>
            <activity id="959" moduleid="43766" modulename="quiz" contextid="44032">
            <quiz id="959">
            <name>coursename</name>
            <intro></intro>
            <introformat>1</introformat>
            <timeopen>1266212040</timeopen>
            <timeclose>1267941600</timeclose>
            <optionflags>0</optionflags>
            <penaltyscheme>0</penaltyscheme>
            <attempts>1</attempts>
            <attemptonlast>0</attemptonlast>
            <grademethod>4</grademethod>
            <decimalpoints>2</decimalpoints>
            <questiondecimalpoints>2</questiondecimalpoints>
            <review>71698464</review>
            <questionsperpage>0</questionsperpage>
            <shufflequestions>0</shufflequestions>
            <shuffleanswers>0</shuffleanswers>
            <questions></questions>
            <sumgrades>0.00000</sumgrades>
            <grade>70.00000</grade>
            <timecreated>1287216250</timecreated>
            <timemodified>1287216250</timemodified>
            <timelimit>3600</timelimit>
            <password></password>
            <subnet></subnet>
            <popup>1</popup>
            <delay1>0</delay1>
            <delay2>0</delay2>
            <showuserpicture>0</showuserpicture>
            <showblocks>0</showblocks>
            <question_instances>
            </question_instances>
            <feedbacks>
            </feedbacks>
            <overrides>
            </overrides>
            <grades>
            </grades>
            <attempts>
            </attempts>
            </quiz>
            </activity>

            The second set of attempts tags are used for for any attempts associated with the quiz, when not backing up userinfo, or if there are none, it's just empty (well, newline).

            Show
            aolley Adam Olley added a comment - Example xml from a quiz.xml generated during a course backup: <?xml version="1.0" encoding="UTF-8"?> <activity id="959" moduleid="43766" modulename="quiz" contextid="44032"> <quiz id="959"> <name>coursename</name> <intro></intro> <introformat>1</introformat> <timeopen>1266212040</timeopen> <timeclose>1267941600</timeclose> <optionflags>0</optionflags> <penaltyscheme>0</penaltyscheme> <attempts>1</attempts> <attemptonlast>0</attemptonlast> <grademethod>4</grademethod> <decimalpoints>2</decimalpoints> <questiondecimalpoints>2</questiondecimalpoints> <review>71698464</review> <questionsperpage>0</questionsperpage> <shufflequestions>0</shufflequestions> <shuffleanswers>0</shuffleanswers> <questions></questions> <sumgrades>0.00000</sumgrades> <grade>70.00000</grade> <timecreated>1287216250</timecreated> <timemodified>1287216250</timemodified> <timelimit>3600</timelimit> <password></password> <subnet></subnet> <popup>1</popup> <delay1>0</delay1> <delay2>0</delay2> <showuserpicture>0</showuserpicture> <showblocks>0</showblocks> <question_instances> </question_instances> <feedbacks> </feedbacks> <overrides> </overrides> <grades> </grades> <attempts> </attempts> </quiz> </activity> The second set of attempts tags are used for for any attempts associated with the quiz, when not backing up userinfo, or if there are none, it's just empty (well, newline).
            Hide
            ashleyholman Ashley Holman added a comment -

            I suppose changing the XML tags will break backwards compatability. So might need to add some extra logic to the XML parsing to differentiate between the two <attempts> tags based on whether it's an integer or contains nested elements / newline.

            Show
            ashleyholman Ashley Holman added a comment - I suppose changing the XML tags will break backwards compatability. So might need to add some extra logic to the XML parsing to differentiate between the two <attempts> tags based on whether it's an integer or contains nested elements / newline.
            Hide
            timhunt Tim Hunt added a comment -

            What happens when backing up and restoring in 1.9? Ah, in 1.9, the first <ATTEMPTS> element, the one that corresponds to the quiz.attempts column, was put into an <ATTEMPTS_NUMBER> tag.

            So, this is definitely new breakage in 2.0. I have no yet wrapped my head around the 2.0 backup system, so I'm really looking for advice from Eloy here. Please.

            Show
            timhunt Tim Hunt added a comment - What happens when backing up and restoring in 1.9? Ah, in 1.9, the first <ATTEMPTS> element, the one that corresponds to the quiz.attempts column, was put into an <ATTEMPTS_NUMBER> tag. So, this is definitely new breakage in 2.0. I have no yet wrapped my head around the 2.0 backup system, so I'm really looking for advice from Eloy here. Please.
            Hide
            aolley Adam Olley added a comment -

            The problem seems to be with the quiz backup and restore stepslibs.

            When creating a backup with or without userinfo, it creates the second set of 'attempts' tags that relate to the quiz_attempts table.
            When restoring a backup without userinfo, the restore stepslib doesn't add the 'attempts' path to set of paths the parser needs to know about, so when the xml parser gets to the second set of tags, it doesn't see it as part of the parentpaths and decides it must be part of the current "/activity/quiz" path.

            I've made a patch to the backup_quiz_stepslib that alleviates this issue when you're making a backup without userinfo by it not adding the extra 'attempts' tags at all since you're not backing that info up.

            Patch follows:

            diff --git a/mod/quiz/backup/moodle2/backup_quiz_stepslib.php b/mod/quiz/backup/moodle2/backup_quiz_stepslib.php
            index 7a05c5a..8d6b931 100644
            — a/mod/quiz/backup/moodle2/backup_quiz_stepslib.php
            +++ b/mod/quiz/backup/moodle2/backup_quiz_stepslib.php
            @@ -68,7 +68,9 @@ class backup_quiz_activity_structure_step extends backup_questions_activity_stru
            $grade = new backup_nested_element('grade', array('id'), array(
            'userid', 'gradeval', 'timemodified'));

            • $attempts = new backup_nested_element('attempts');
              + if ($userinfo) { + $attempts = new backup_nested_element('attempts'); + }

            $attempt = new backup_nested_element('attempt', array('id'), array(
            'uniqueid', 'userid', 'attemptnum', 'sumgrades',
            @@ -94,8 +96,10 @@ class backup_quiz_activity_structure_step extends backup_questions_activity_stru
            $quiz->add_child($grades);
            $grades->add_child($grade);

            • $quiz->add_child($attempts);
            • $attempts->add_child($attempt);
              + if ($userinfo) { + $quiz->add_child($attempts); + $attempts->add_child($attempt); + }
            Show
            aolley Adam Olley added a comment - The problem seems to be with the quiz backup and restore stepslibs. When creating a backup with or without userinfo, it creates the second set of 'attempts' tags that relate to the quiz_attempts table. When restoring a backup without userinfo, the restore stepslib doesn't add the 'attempts' path to set of paths the parser needs to know about, so when the xml parser gets to the second set of tags, it doesn't see it as part of the parentpaths and decides it must be part of the current "/activity/quiz" path. I've made a patch to the backup_quiz_stepslib that alleviates this issue when you're making a backup without userinfo by it not adding the extra 'attempts' tags at all since you're not backing that info up. Patch follows: diff --git a/mod/quiz/backup/moodle2/backup_quiz_stepslib.php b/mod/quiz/backup/moodle2/backup_quiz_stepslib.php index 7a05c5a..8d6b931 100644 — a/mod/quiz/backup/moodle2/backup_quiz_stepslib.php +++ b/mod/quiz/backup/moodle2/backup_quiz_stepslib.php @@ -68,7 +68,9 @@ class backup_quiz_activity_structure_step extends backup_questions_activity_stru $grade = new backup_nested_element('grade', array('id'), array( 'userid', 'gradeval', 'timemodified')); $attempts = new backup_nested_element('attempts'); + if ($userinfo) { + $attempts = new backup_nested_element('attempts'); + } $attempt = new backup_nested_element('attempt', array('id'), array( 'uniqueid', 'userid', 'attemptnum', 'sumgrades', @@ -94,8 +96,10 @@ class backup_quiz_activity_structure_step extends backup_questions_activity_stru $quiz->add_child($grades); $grades->add_child($grade); $quiz->add_child($attempts); $attempts->add_child($attempt); + if ($userinfo) { + $quiz->add_child($attempts); + $attempts->add_child($attempt); + }
            Hide
            aolley Adam Olley added a comment -

            Naturally comments messes up copy/pasted code.

            Attached the patch as a file to the ticket.

            Show
            aolley Adam Olley added a comment - Naturally comments messes up copy/pasted code. Attached the patch as a file to the ticket.
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Hi,

            my fault! I never should have named both elements the same way and forgot to alias the first one to attempts_num or whatever (like in 1.9). I remember myself doing tons of tests but surely I did them restoring with/without user info, but always with quizzes having attempts, so the problem wasn't reproducible.

            The main problem here is that the "second" attempts tag overwrites the 1st one and it's not fair to assume it will be 0/false, a.k.a. "unlimited attempts".

            So I will be:

            1) changing backup & restore from now to start using the "attempts_num" tag (as was in 1.9).
            2) try to go deeper in the XML parser to avoid the substitution commented above, so old (current) backups will restore ok. I'm not sure if this is going to be possible or no (perhaps some nasty harcoded trick could do the job, let's see).

            Ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Hi, my fault! I never should have named both elements the same way and forgot to alias the first one to attempts_num or whatever (like in 1.9). I remember myself doing tons of tests but surely I did them restoring with/without user info, but always with quizzes having attempts, so the problem wasn't reproducible. The main problem here is that the "second" attempts tag overwrites the 1st one and it's not fair to assume it will be 0/false, a.k.a. "unlimited attempts". So I will be: 1) changing backup & restore from now to start using the "attempts_num" tag (as was in 1.9). 2) try to go deeper in the XML parser to avoid the substitution commented above, so old (current) backups will restore ok. I'm not sure if this is going to be possible or no (perhaps some nasty harcoded trick could do the job, let's see). Ciao
            Hide
            timhunt Tim Hunt added a comment -

            Sounds like the right solution Eloy.

            Show
            timhunt Tim Hunt added a comment - Sounds like the right solution Eloy.
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Can anybody try this patch:

            https://github.com/stronk7/moodle/commit/9b97f1857773a4a5adf80ae3b6ad15593e41e68e

            and report if it causes the correct (the first) "attempts" tag to be applied properly in some of the offending backups?

            In the meantime, I'll be working on 1) above, renaming the tag and also adding some checks in backup to detect repetitions like this @ development stages.

            Ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Can anybody try this patch: https://github.com/stronk7/moodle/commit/9b97f1857773a4a5adf80ae3b6ad15593e41e68e and report if it causes the correct (the first) "attempts" tag to be applied properly in some of the offending backups? In the meantime, I'll be working on 1) above, renaming the tag and also adding some checks in backup to detect repetitions like this @ development stages. Ciao
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Aha, detection now seems to work, also detected another wrongly-reused tag in lesson:

            /activity/lesson/highscores

            Improving a bit more the detection... going to change this issue title to cover both lesson and quiz.

            Ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Aha, detection now seems to work, also detected another wrongly-reused tag in lesson: /activity/lesson/highscores Improving a bit more the detection... going to change this issue title to cover both lesson and quiz. Ciao
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Done,

            now quiz and lesson both use non-conflicting tags (lesson->showhighscores and quiz->attempts_number) on backup. And restore is able to handle both the old and the new tags.

            Also, when defining backup structures, those conflicts are detected @ development time, so this problem shouldn't happen anymore.

            Tested both with old and new course backups.

            Complete changes: https://github.com/stronk7/moodle/compare/master...MDL-26229_restore_attempts_wip
            Sent to integration as: PULL-268

            Thanks for the report and research, ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Done, now quiz and lesson both use non-conflicting tags (lesson->showhighscores and quiz->attempts_number) on backup. And restore is able to handle both the old and the new tags. Also, when defining backup structures, those conflicts are detected @ development time, so this problem shouldn't happen anymore. Tested both with old and new course backups. Complete changes: https://github.com/stronk7/moodle/compare/master...MDL-26229_restore_attempts_wip Sent to integration as: PULL-268 Thanks for the report and research, ciao
            Hide
            timhunt Tim Hunt added a comment -

            Thanks Eloy. The patch looks good to me.

            Show
            timhunt Tim Hunt added a comment - Thanks Eloy. The patch looks good to me.
            Hide
            tsala Helen Foster added a comment -

            This issue is fixed in the latest 2.0.1+ weekly. Thanks everyone!

            Show
            tsala Helen Foster added a comment - This issue is fixed in the latest 2.0.1+ weekly. Thanks everyone!

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  21/Feb/11