Service XF:User\ChangeContent runs twice when renaming a user upon deletion

Kirby

Well-known member
Affected version
2.2.13
This is a rather complicated bug, but I hope it's understandable.

Method XF\Service\User\Delete::delete sets entity options to avoid running jobs XF:UserRenameCleanup and XF:UserDeleteCleanup when renaming a user before deletion:
PHP:
if ($this->renameTo)
{
    $user->reset();
    $user->setOption('admin_edit', true);
    $user->setOption('enqueue_rename_cleanup', false);
    $user->setOption('enqueue_delete_cleanup', false);

    $user->set('username', $this->renameTo);

After deletion,. those jobs are enqueued in method runPostDeleteJobs
PHP:
if ($this->renameTo)
{
    $jobList = [
        [
            'XF:UserRenameCleanUp',
            [
                'originalUserId' => $user->user_id,
                'originalUserName' => $this->originalUserName,
                'newUserName' => $this->renameTo
            ]
        ],
        [
            'XF:UserDeleteCleanUp',
            [
                'userId' => $user->user_id,
                'username' => $this->renameTo
            ]
        ]
    ];
    $this->app->jobManager()->enqueueUnique('userRenameDelete' . $user->user_id, 'XF:Atomic', [
        'execute' => $jobList
    ]);

Job XF:UserRenameCleanUpcalls service XF:User\ContentChange in method run
PHP:
/** @var \XF\Service\User\ContentChange $contentChanger */
$contentChanger = $this->app->service(
        'XF:User\ContentChange', $this->data['originalUserId'], $this->data['originalUserName']
);
$contentChanger->setupForNameChange($this->data['newUserName']);
$contentChanger->restoreState($this->data['currentStep'], $this->data['lastOffset']);

$result = $contentChanger->apply($maxRunTime);

Job XF:UserDeleteCleanUp calls service XF:User\DeleteCleanUp in method run
PHP:
$deleter = $this->app->service(
        'XF:User\DeleteCleanUp', $this->data['userId'], $this->data['username']
);
$deleter->restoreState($this->data['currentStep'], $this->data['lastOffset']);

$result = $deleter->cleanUp($maxRunTime);

Now finally service XF:User\DeleteCleanUp calls service XF:User\ContentChange in method stepChangeOwne
PHP:
$contentChanger = $this->service('XF:User\ContentChange', $this->userId, $this->userName);
$contentChanger->setupForDelete();

if (is_array($lastOffset))
{
    list($changeStep, $changeLastOffset) = $lastOffset;
    $contentChanger->restoreState($changeStep, $changeLastOffset);
}

$result = $contentChanger->apply($maxRunTime);

The overall effect of this call chain is that content is changed twice:

  1. First, only the username is changed to $newUsername via rename cleanup
  2. Afterwards the userid is changed to 0 via delete cleanup

This is inefficient and can cause quite some performance issues as noted in

In case a user account is renamed before deletion there should be only one content change:
Update userid to 0 and set username to $newUsername
 
Last edited:
This is still an issue, the queries generated by the second change can be quite slow if there are many reactions by deleted users:
Code:
# Thread_id: <redacted>  Schema: <redacted>  QC_hit: No
# Query_time: 18.361660  Lock_time: 0.000031  Rows_sent: 0  Rows_examined: 2144963
# Rows_affected: 0  Bytes_sent: 0
UPDATE (
    SELECT content_id
    FROM xf_reaction_content
    WHERE content_type = 'post'
    AND reaction_user_id = '0'
) AS temp
INNER JOIN xf_post AS reaction_table ON (reaction_table.`post_id` = temp.content_id)
SET reaction_table.`reaction_users` = REPLACE(reaction_table.`reaction_users`, '\"user_id\":<userid>,\"username\":\"<newusername>\"', '\"user_id\":0,\"username\":\"<newusername>\"')
WHERE reaction_table.`reaction_users` LIKE '%\"user\\_id\":<userid>,\"username\":\"<newusername>\"%';
 
Top Bottom