Moodle
  1. Moodle
  2. MDL-32104

Cannot restore backups with 'update post' forum logs

    Details

    • Testing Instructions:
      Hide
      1. This needs to be tested in ALL supported databases
      2. Goto a forum in a course
      3. update any post
      4. Goto site admin>reports>live logs and make sure the log is in the format discuss.php?d=3213#p17623&parent=17623
      5. Backup the course with Include course logs checked
      6. Restore the course
      7. Make sure you get no errors during restore and the entries in the log report for this course are still in the format discuss.php?d=3213#p17623&parent=17623
        (Note:- the urls in logs may be encoded so # appears as %23)
      Show
      This needs to be tested in ALL supported databases Goto a forum in a course update any post Goto site admin>reports>live logs and make sure the log is in the format discuss.php?d=3213#p17623&parent=17623 Backup the course with Include course logs checked Restore the course Make sure you get no errors during restore and the entries in the log report for this course are still in the format discuss.php?d=3213#p17623&parent=17623 (Note:- the urls in logs may be encoded so # appears as %23)
    • Affected Branches:
      MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE
    • Pull Master Branch:
      MDL-32104-master

      Description

      The "update post" log action in the forum module fails to parse properly when restoring a course.
      eg:

       module |   action    |                  url                   
      --------+-------------+----------------------------------------
       forum  | update post | discuss.php?d=3237#p13137&parent=13137
       forum  | update post | discuss.php?d=1532#p6295&parent=6295
       forum  | update post | discuss.php?d=3213#p17623&parent=17623

      It logs to the backup XML like this:

        <log id="3806754">
          <time>1331694499</time>
          <userid>44</userid>
          <ip>129.94.52.74</ip>
          <module>forum</module>
          <action>update post</action>
          <url>discuss.php?d=9020#p31092&amp;parent=31092</url>
          <info>31092</info>
        </log>

      mod/forum/backup/moodle2/restore_forum_activity_task.class.php:114:

              $rules[] = new restore_log_rule('forum', 'update post', 'discuss.php?d={forum_discussion}&parent={forum_post}', '{forum_post}');

      Parses the 'd' parameter as a non-integer which crashes the restore:

      Default exception handler: Error reading from database Debug: ERROR:  invalid input syntax for integer: "9020#p31092" SELECT * FROM mdl_backup_ids_temp WHERE backupid = $1 AND itemname = $2 AND itemid = $3\n[array (\n  0 => '0b3f2f773e7ac2546690019215ff867a',\n  1 => 'forum_discussion',  2 => '9020#p31092',\n)]
      * line 394 of /lib/dml/moodle_database.php: dml_read_exception thrown\n* line 232 of /lib/dml/pgsql_native_moodle_database.php: call to moodle_database->query_end()
      * line 678 of /lib/dml/pgsql_native_moodle_database.php: call to pgsql_native_moodle_database->query_end()\n* line 1297 of /lib/dml/moodle_database.php: call to pgsql_native_moodle_database->get_records_sql()
      * line 1269 of /lib/dml/moodle_database.php: call to moodle_database->get_record_sql()
      * line 1249 of /lib/dml/moodle_database.php: call to moodle_database->get_record_select()\n* line 1261 of /backup/util/dbops/restore_dbops.class.php: call to moodle_database->get_record()
      * line 197 of /backup/util/helper/restore_log_rule.class.php: call to restore_dbops::get_backup_ids_record()\n* line 137 of /backup/util/helper/restore_log_rule.class.php: call to restore_log_rule->parse_tokens_and_matches()
      * line 84 of /backup/util/helper/restore_logs_processor.class.php: call to restore_
      log_rule->process()
      * line 1933 of /backup/moodle2/restore_stepslib.php: call to restore_logs_processor->process_log_record()\n* line 131 of /backup/util/plan/restore_structure_step.class.php: call to restore_activity_logs_structure_step->process_log()
      * line 103 of /backup/util/helper/restore_structure_parser_processor.class.php: call to restore_structure_step->process()
      * line 130 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()\n* line 148 of /backup/util/xml/parser/processors/simplified_parser_processor.class.php: call to restore_structure_parser_processor->postprocess_chunk()

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            Michael de Raadt added a comment -

            Thanks for reporting this.

            I noted that you didn't add a "partner" tag. You still can if you wish.

            Show
            Michael de Raadt added a comment - Thanks for reporting this. I noted that you didn't add a "partner" tag. You still can if you wish.
            Hide
            Ankit Agarwal added a comment -

            We can either change the url format when adding logs or change the rule during restore to fix this issue.
            Changing the restore log rule makes more sense to me, so I have added a solution with that approach.
            Sending for peer-review.
            Thanks

            Show
            Ankit Agarwal added a comment - We can either change the url format when adding logs or change the rule during restore to fix this issue. Changing the restore log rule makes more sense to me, so I have added a solution with that approach. Sending for peer-review. Thanks
            Hide
            Rossiani Wijaya added a comment -

            looks good ankit.

            Show
            Rossiani Wijaya added a comment - looks good ankit.
            Hide
            Ankit Agarwal added a comment -

            Thanks Rosie for the review.
            Sending for integration
            Thanks

            Show
            Ankit Agarwal added a comment - Thanks Rosie for the review. Sending for integration Thanks
            Hide
            Dan Poltawski added a comment -

            The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

            TIA and ciao

            Show
            Dan Poltawski added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
            Hide
            Ankit Agarwal added a comment -

            rebased!

            Show
            Ankit Agarwal added a comment - rebased!
            Hide
            Dan Poltawski added a comment -

            Thanks, that has been integrated now!

            Show
            Dan Poltawski added a comment - Thanks, that has been integrated now!
            Hide
            Michael de Raadt added a comment -

            So far I have tested this in 2.1, 2.2 and master on MySQL.

            I have also tested it on PostgreSQL and MS SQL in master.

            I'm waiting for my Oracle instance to finish PHPUnit tests before I can test it there.

            Show
            Michael de Raadt added a comment - So far I have tested this in 2.1, 2.2 and master on MySQL. I have also tested it on PostgreSQL and MS SQL in master. I'm waiting for my Oracle instance to finish PHPUnit tests before I can test it there.
            Hide
            Michael de Raadt added a comment -

            Test result: Success

            Tested in master under all DBs. Also tested in 2.1 and 2.2 under MySQL.

            Show
            Michael de Raadt added a comment - Test result: Success Tested in master under all DBs. Also tested in 2.1 and 2.2 under MySQL.
            Hide
            Eloy Lafuente (stronk7) added a comment -

            This has been near becoming rejected, because it's not the best code you are able to produce.

            But, luckily, at the end, it has landed and has been spread to all repos out there.

            Many thanks and, don't forget it, keep improving your skills, you can!

            Closing, ciao

            Show
            Eloy Lafuente (stronk7) added a comment - This has been near becoming rejected, because it's not the best code you are able to produce. But, luckily, at the end, it has landed and has been spread to all repos out there. Many thanks and, don't forget it, keep improving your skills, you can! Closing, ciao

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved: