New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use auto-incrementing IDs with Eloquent users instead of UUIDs #2194

Open
rdarcy1 opened this Issue Oct 9, 2018 · 2 comments

Comments

Projects
None yet
2 participants
@rdarcy1

rdarcy1 commented Oct 9, 2018

Describe the bug
Applies when using the Eloquent user driver.

When creating a new user, if the generated UUID can be cast to integer it will be written to the database instead of the database's auto-increment value. This pushes up the auto-increment value and has led to issues where the maximum integer value is reached in MySQL.

Affected UUIDs are those which start with integers (as PHP will cast these to a non-zero integer), e.g. 7336336d-6f3b-4f1f-9eff-b1dfa4a148a1.

To Reproduce
This is difficult to reproduce 'naturally' as it only happens with a specific subset of UUIDs. We can simulate it happening by manually setting the UUID when creating a user.

In Statamic\CP\Publish\Publisher add the following $this->id assignment to line 111.

 public function publish()
    {
        // ...

        // Add the fieldset to the data if it was specified on the fly.
        $this->appendFieldset();

        // Commit any changes made by the user and/or the fieldtype processors back to the content object.
        $this->id = "7336336d-6f3b-4f1f-9eff-b1dfa4a148a1";
        $this->updateContent();

        // Save the file and any run any supplementary tasks like updating the cache, firing events, etc.
        $this->save();

        return $this->content;
    }

Save a user.
See it has the ID 7336336 (the first part of the UUID).

Expected behavior
The ID should not be set for Eloquent users, instead they should be assigned one by the database.

Fix
We can check to see if the content is an eloquent user when updating the content in Statamic\CP\Publish\Publisher.

protected function updateContent()
    {
        if (! $this->content instanceof \Statamic\Data\Users\Eloquent\User) {
            $this->fields['id'] = $this->id;
        }

        // ...
    }

Environment details

  • Statamic Version 2.10.4
  • Fresh Install or Upgrade: Upgrade
  • PHP Version: 7.1

@jackmcdade jackmcdade changed the title from Eloquent users receive ID based on UUID rather than auto-incrementing to Use auto-incrementing IDs with Eloquent users instead of UUIDs Oct 26, 2018

@rdarcy1

This comment has been minimized.

rdarcy1 commented Oct 29, 2018

@jackmcdade This has been labelled as a feature request but I'd say it's more of a bug – it's causing the integer ID column in the database to reach its max value very quickly (due to the auto incremement value being pushed up unecessarily).

For a site with only a few thousand users I've had to manually re-assign IDs above a threshold and reset the auto-incr value on the table three times because it hit its limit.

@jackmcdade

This comment has been minimized.

Member

jackmcdade commented Oct 29, 2018

I think our original intentions with the feature was for new sites, but yeah I guess you're right on that one.

@jackmcdade jackmcdade added the bug label Oct 29, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment