Moodle
  1. Moodle
  2. MDL-30192

Problem creating deleted users on restore

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: 2.2
    • Fix Version/s: 2.0.6, 2.1.3
    • Component/s: Backup
    • Labels:
    • Testing Instructions:
      Hide
      • Create one course and enrol 2-3 student.
      • Create 1 forum and add some posts.
      • Go to admin/users and delete one of the users.
      • Verify that their posts continue being shown in the course
      • Make a backup of the course
      • Restore the course into ANOTHER site (so the user will need to be created).
      • TEST: No error happens and restore ends ok.
      • TEST: Go to the restored course and verify that the forums still show the posts from the deleted user.
      • TEST: Clicking on the deleted user fullname leads to "Invalid user" error. This is in fact a regression that has been addressed @ MDL-30276 but that's the output provided right now.

      That is!

      Show
      Create one course and enrol 2-3 student. Create 1 forum and add some posts. Go to admin/users and delete one of the users. Verify that their posts continue being shown in the course Make a backup of the course Restore the course into ANOTHER site (so the user will need to be created). TEST: No error happens and restore ends ok. TEST: Go to the restored course and verify that the forums still show the posts from the deleted user. TEST: Clicking on the deleted user fullname leads to "Invalid user" error. This is in fact a regression that has been addressed @ MDL-30276 but that's the output provided right now. That is!
    • Affected Branches:
      MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_20_STABLE, MOODLE_21_STABLE
    • Pull from Repository:
    • Pull Master Branch:
    • Rank:
      26117

      Description

      I have a backup that I use for testing freshly installed sites.
      The backup was originally taken in August 2010, and I have last successfully restored it in August 2011.
      Today I've tried to restore it twice and both times have got the following:

      error/unknown_context_mapping

      More information about this error

      Stack trace:
      line 637 of /backup/util/dbops/restore_dbops.class.php: restore_dbops_exception thrown
      line 869 of /backup/util/dbops/restore_dbops.class.php: call to restore_dbops::send_files_to_pool()
      line 691 of /backup/moodle2/restore_stepslib.php: call to restore_dbops::create_included_users()
      line 34 of /backup/util/plan/restore_execution_step.class.php: call to restore_create_included_users->define_execution()
      line 153 of /backup/util/plan/base_task.class.php: call to restore_execution_step->execute()
      line 148 of /backup/util/plan/base_plan.class.php: call to base_task->execute()
      line 157 of /backup/util/plan/restore_plan.class.php: call to base_plan->execute()
      line 302 of /backup/controller/restore_controller.class.php: call to restore_plan->execute()
      line 144 of /backup/util/ui/restore_ui.class.php: call to restore_controller->execute_plan()
      line 45 of /backup/restore.php: call to restore_ui->execute()
      Output buffer: Notice: Trying to get property of non-object in /backup/util/dbops/restore_dbops.class.php on line 827

        Issue Links

          Activity

          Hide
          Sam Hemelryk added a comment -

          The backup I use is of the Demo world of water course from Tomaz's test site http://school.demo.moodle.net/course/view.php?id=115

          I've just confirmed that I get the above with a fresh backup of the course as well so looks like it is more than just an old backup issue.

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - The backup I use is of the Demo world of water course from Tomaz's test site http://school.demo.moodle.net/course/view.php?id=115 I've just confirmed that I get the above with a fresh backup of the course as well so looks like it is more than just an old backup issue. Cheers Sam
          Hide
          Sam Hemelryk added a comment -

          Ohh all settings default btw

          Show
          Sam Hemelryk added a comment - Ohh all settings default btw
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Oh, my... that notice in the call to get_context_instance() (line 827) is really suspicious... I guess it causes not to be set, hence the exception later...

          will try tomorrow...thanks & ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Oh, my... that notice in the call to get_context_instance() (line 827) is really suspicious... I guess it causes not to be set, hence the exception later... will try tomorrow...thanks & ciao
          Hide
          Eloy Lafuente (stronk7) added a comment -

          re.oh, it fails for user id=5 (ctxid = 2550) that is a deleted user! I bet accesslib does not create a context for those... epic problem. Tomorrow more...

          Show
          Eloy Lafuente (stronk7) added a comment - re.oh, it fails for user id=5 (ctxid = 2550) that is a deleted user! I bet accesslib does not create a context for those... epic problem. Tomorrow more...
          Hide
          Michael de Raadt added a comment -

          Epic problem - cool. I'll lend you my cape.

          Show
          Michael de Raadt added a comment - Epic problem - cool. I'll lend you my cape.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          The problem is only reproducible under master since 2-3 weeks ago because then, with the refactor of accesslib we stopped creating user contexts for deleted users. Before that (2.0, 2.1) the creation happened and the error was "impossible.

          In any case, taking a look to the code executed by delete_user() it seems that all the preferences, tags, custom profile fields and files are deleted when the user is deleted, so there is no point about to re-create that information for deleted users in restore, no matter if the context can be created or no.

          So the fix, is simply, about to enclose the creation of the context and all related information within one big:

          if (empty($user->deleted)) {
              ....
              ....
          

          so, for deleted users it won't be executed any more. And apply the fix to all the 2.x versions.

          Doing it now... ciao

          Show
          Eloy Lafuente (stronk7) added a comment - The problem is only reproducible under master since 2-3 weeks ago because then, with the refactor of accesslib we stopped creating user contexts for deleted users. Before that (2.0, 2.1) the creation happened and the error was "impossible. In any case, taking a look to the code executed by delete_user() it seems that all the preferences, tags, custom profile fields and files are deleted when the user is deleted, so there is no point about to re-create that information for deleted users in restore, no matter if the context can be created or no. So the fix, is simply, about to enclose the creation of the context and all related information within one big: if (empty($user->deleted)) { .... .... so, for deleted users it won't be executed any more. And apply the fix to all the 2.x versions. Doing it now... ciao
          Hide
          Eloy Lafuente (stronk7) added a comment -

          (sent to integration)

          Show
          Eloy Lafuente (stronk7) added a comment - (sent to integration)
          Hide
          Petr Škoda added a comment -

          Eloy: I agree, it does not make any sense to restore any info for deleted users, they should be imo completely skipped during restore. The trouble here is we do not have yet method to deal with full purging of user data after deleting. Also what happens when you restore posts from user that was deleted?

          Show
          Petr Škoda added a comment - Eloy: I agree, it does not make any sense to restore any info for deleted users, they should be imo completely skipped during restore. The trouble here is we do not have yet method to deal with full purging of user data after deleting. Also what happens when you restore posts from user that was deleted?
          Hide
          Eloy Lafuente (stronk7) added a comment -

          They are restored, obviously, as they would be in real life (user deleted but posts survive).

          Show
          Eloy Lafuente (stronk7) added a comment - They are restored, obviously, as they would be in real life (user deleted but posts survive).
          Hide
          Sam Hemelryk added a comment -

          Thanks Eloy - this has been integrated now

          Show
          Sam Hemelryk added a comment - Thanks Eloy - this has been integrated now
          Hide
          Jason Fowler added a comment -

          I get this error message after doing a successful restore and viewing the forum discussion.

          Stack trace:
          line 150 of /lib/outputcomponents.php: coding_exception thrown
          line 1774 of /lib/outputrenderers.php: call to user_picture->__construct()
          line 3350 of /mod/forum/lib.php: call to core_renderer->user_picture()
          line 5635 of /mod/forum/lib.php: call to forum_print_post()
          line 5636 of /mod/forum/lib.php: call to forum_print_posts_nested()
          line 5502 of /mod/forum/lib.php: call to forum_print_posts_nested()
          line 271 of /mod/forum/discuss.php: call to forum_print_discussion()

          Show
          Jason Fowler added a comment - I get this error message after doing a successful restore and viewing the forum discussion. Stack trace: line 150 of /lib/outputcomponents.php: coding_exception thrown line 1774 of /lib/outputrenderers.php: call to user_picture->__construct() line 3350 of /mod/forum/lib.php: call to core_renderer->user_picture() line 5635 of /mod/forum/lib.php: call to forum_print_post() line 5636 of /mod/forum/lib.php: call to forum_print_posts_nested() line 5502 of /mod/forum/lib.php: call to forum_print_posts_nested() line 271 of /mod/forum/discuss.php: call to forum_print_discussion()
          Hide
          Jason Fowler added a comment -

          the error occured under 2.2dev

          Show
          Jason Fowler added a comment - the error occured under 2.2dev
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Just guessing if you have there the backup file that caused the error testing to happen:

          http://tracker.moodle.org/browse/MDL-30192?focusedCommentId=131435&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-131435

          I've been tracing the error, and I cannot see any problem in restore code related with that, the user IS created ok. But it seems that, when printing the forum, somehow the user->id is banished.

          I've been trying here with some deleted users and cannot reproduce, so it would be great to get access to the backup file leading to that.

          Also, did you tried it in other branches with success? Or just 2.2 and failed?

          TIA!

          Show
          Eloy Lafuente (stronk7) added a comment - Just guessing if you have there the backup file that caused the error testing to happen: http://tracker.moodle.org/browse/MDL-30192?focusedCommentId=131435&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-131435 I've been tracing the error, and I cannot see any problem in restore code related with that, the user IS created ok. But it seems that, when printing the forum, somehow the user->id is banished. I've been trying here with some deleted users and cannot reproduce, so it would be great to get access to the backup file leading to that. Also, did you tried it in other branches with success? Or just 2.2 and failed? TIA!
          Hide
          Eloy Lafuente (stronk7) added a comment -

          I'm going to replicate the testing instructions here from scratch. I hope they will lead me to the same error (cross your fingers, heh).

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - I'm going to replicate the testing instructions here from scratch. I hope they will lead me to the same error (cross your fingers, heh). Ciao
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Step 4: Immediately after deleting one user, when navigating to the forum I'm getting:

          Notice: Trying to get property of non-object in /lib/outputcomponents.php on line 305
          Call Stack
          #	Time	Memory	Function	Location
          1	0.0001	637480	{main}( )	../view.php:0
          2	0.4719	16241480	forum_print_latest_discussions( )	../view.php:239
          3	0.4826	16304992	forum_print_discussion_header( )	../lib.php:5352
          4	0.4828	16306192	core_renderer->user_picture( )	../lib.php:3650
          5	0.4828	16308040	renderer_base->render( )	../outputrenderers.php:1780
          6	0.4828	16308128	core_renderer->render_user_picture( )	../outputrenderers.php:70
          7	0.4830	16308312	user_picture->get_url( )	../outputrenderers.php:1817
          

          (it breaks because the user context does not exist anymore! Has been deleted.

          I continue testing but I bet the problem is in output code not verifying if the user / context is ok...

          Show
          Eloy Lafuente (stronk7) added a comment - Step 4: Immediately after deleting one user, when navigating to the forum I'm getting: Notice: Trying to get property of non-object in /lib/outputcomponents.php on line 305 Call Stack # Time Memory Function Location 1 0.0001 637480 {main}( ) ../view.php:0 2 0.4719 16241480 forum_print_latest_discussions( ) ../view.php:239 3 0.4826 16304992 forum_print_discussion_header( ) ../lib.php:5352 4 0.4828 16306192 core_renderer->user_picture( ) ../lib.php:3650 5 0.4828 16308040 renderer_base->render( ) ../outputrenderers.php:1780 6 0.4828 16308128 core_renderer->render_user_picture( ) ../outputrenderers.php:70 7 0.4830 16308312 user_picture->get_url( ) ../outputrenderers.php:1817 (it breaks because the user context does not exist anymore! Has been deleted. I continue testing but I bet the problem is in output code not verifying if the user / context is ok...
          Hide
          Jason Fowler added a comment -

          The backup file I restored

          Show
          Jason Fowler added a comment - The backup file I restored
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Aha, found another problem, affecting to 2.2 backup containing deleted users... going to fix it... in the mean time... it would be great if you can perform the testing instructions above for the 20_STABLE and 21_STABLE branches, they should not be affected.

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Aha, found another problem, affecting to 2.2 backup containing deleted users... going to fix it... in the mean time... it would be great if you can perform the testing instructions above for the 20_STABLE and 21_STABLE branches, they should not be affected. Ciao
          Hide
          Eloy Lafuente (stronk7) added a comment - - edited

          Some related issues:

          • I've created MDL-30456 about the regression found in backup. It has been already integrated, so 2.2 can be tested here too.
          • I've created MDL-30457 about the notices detected above outputting deleted user icons.

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - - edited Some related issues: I've created MDL-30456 about the regression found in backup. It has been already integrated, so 2.2 can be tested here too. I've created MDL-30457 about the notices detected above outputting deleted user icons. Ciao
          Hide
          Eloy Lafuente (stronk7) added a comment -

          (issue reset to "testing" status so new results can come here)

          Show
          Eloy Lafuente (stronk7) added a comment - (issue reset to "testing" status so new results can come here)
          Hide
          Jason Fowler added a comment -

          works perfectly in all 2.X builds

          Show
          Jason Fowler added a comment - works perfectly in all 2.X builds
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Thanks for the extensive testing here, Jason, it helped discovering some interesting issues, yay!

          Show
          Eloy Lafuente (stronk7) added a comment - Thanks for the extensive testing here, Jason, it helped discovering some interesting issues, yay!
          Hide
          Eloy Lafuente (stronk7) added a comment -

          And this has landed upstream, just on time for the upcoming new releases week. Thanks for it!

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - And this has landed upstream, just on time for the upcoming new releases week. Thanks for it! Ciao

            People

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

              Dates

              • Created:
                Updated:
                Resolved: