Integrity constraint violation when saving a user account after creation

While saving a user again upon user-insert is something not nice to do, it's sometimes required to fulfill some use-cases like auto-populating fields based on information that required the user be already saved.

Then, having user roles inserted after the hook violates the hook documentation, thus is a bug. Also see #1729812: Separate storage operations from reactions.

Related rule issue: #1771666: Integrity constraint violation when saving a user account after creation.

Roles should be written before the hook is invoked, it's that way already in d8.

ops, moved the hooks a bit too far ;)

#5: user_insert_roles_d7.patch queued for re-testing.

I remember this from Drupal 6! The fix was to just add to the user objects role array, so with this in mind, contrib modules are likely to be unaffected if they use user_presave but will stop working if they use the user_insert hook.

http://drupal.org/project/autoassignrole

This uses presave hook so is ok

http://drupal.org/project/regcode

This uses the insert hook, so will probably break :(

http://drupal.org/project/registration_role
http://drupal.org/project/user_selectable_roles
http://drupal.org/project/og_joinrole

No Drupal 7 port

I didn't check the e-commerce modules, and logintoboggan kinda does it own thing before calling user_save(), so this shouldn't have any issues...

Or to paraphrase, this is an API change that affects at least one contrib module.

Patch http://drupal.org/node/1433288#5 tested in Drupal 7.15 with Rules 2.x-dev successfully.

Patch from #5 works also great with Drupal 7.16. Also saw no issues using regcode module, but haven't tested all regcode functionalities.

http://drupal.org/project/regcode

This uses the insert hook, so will probably break :(

Actually, it's the contrary. I wrote the patch to make regcode work in a scenario where a user role is added as well. Recode triggers an user_save upon user-insert what results in the PDO-Exception (without a fix like the one I provided in #5).

Cool, ignore my concerns then :)

Is this patch being committed? When we update to a newer 7.x core version for security reasons we need to apply the patch all over again... Or is this something that won't be fixed in core?

tested #5 with d7.19 without problems.
(using rules)

patch worked, thank you!

Patch works fine and still necessary for Drupal 7.20

This definitely makes the code more consistent, but is it really safe for Drupal 7? If anyone is modifying $account->roles in one of these hooks expecting it to be saved to the database, this patch will break their code...

I think a patch that made the database query more fail-safe (i.e., refusing to insert roles if they're already there) or perhaps some kind of "$account->roles_already_saved" flag (i.e., hack) to signal to it not to save again would be worth considering?

BTW I don't know if it is related after applying this patch, but I have a strange and terrible behavior that I can't always reproduce:
some new users are granted with ALL roles (including administrator O_o) after verifying their email address.
Still debugging and probably related to a Rules involved with roles.
Just in case removed patch from prod... of course.

In general I would agree with you. However, nobody should modify $account->roles in hook_entity_insert() as this hook is documented without referencing parameters (in contrast to i.e. hook_form_alter()).

So, if someone implemented this hook with references and modifies the object his implementation is not api compatible. Therefore I see no need to take respect of such a scenario.

@haggins, objects in PHP are always modified by reference; it doesn't matter if they're marked that way in the function signature or not.

And in general, if you make a change in hook_entity_insert() that affects the user account but don't update the $entity object to reflect that, that's a bug because it means the calling code (i.e., whoever called user_save() in the first place) will get back an object in the end that doesn't correctly reflect the current state of the account.

Ah yes, I forgot that. Ok, here's a new patch based on your suggestion from #18.

It saves the user roles already in the database before hook_entity_insert() is called. Before the user roles are inserted into the db again it checks whether the role id is in the list of already saved roles.

#22: drupal-fix-user-insert-roles-1433288-22.patch queued for re-testing.

Another try:

...

Just solved a problem I was having with a Rule that attempted to set a role on account registration. I'm also using the Profile2 Registration Path module to set some roles, and I think the combination was causing the issue. #27 looks like it fixed my problem :-)

Glad to see it fixed your issue!

Did anyone else try the patch?

Hi all,
I can confirm the patch worked perfectly for me too.

Thanks

I ran into the same issue, when I tried to use a rule to set a variable to a user field after saving a new user.

The patch worked perfectly!

Thanks a lot!

haggins Patch worked great for me too. Thanks a lot .. was getting a headache trying to approve and add a role to a user with rules.

Marking RTBC because of #28, #30, #31, and #32.

I think calling user_load() here is pretty heavy, and also could cause problems since modules which respond to the load event (via a hook) might reasonably expect all the user's roles to already be attached to the object they are working with, but in this case it looks like they won't be.

Can't we just do this check via a direct database query? Or maybe even by doing a db_merge() rather than a db_insert() (not sure about that one)?

Also, minor point:

+          if (!in_array($rid, array(DRUPAL_ANONYMOUS_RID, DRUPAL_AUTHENTICATED_RID)) && !array_key_exists($rid, $roles_db)) {

This could just use isset() rather than array_key_exists().

Thanks again for your feedback, David.

Here's a smarter solution with a direct database query.

Removed spaces...

#36 Works for me

Thanks

adding info that user is not created when we get this error

Thank you so much for this patch! What should I do with the user.module file when I upgrade Drupal in the future?

You need to apply this patch again if it's still not committed until next release.

Setting to RTBC because of #37 and #38.

Okay, I'll do that. Thanks for your answer.

Would be fantastic if this one could come with next bug release of Drupal. It would for sure let me get rid of endless workarounds.

In testing this out, it seems to make user_save() attempt a database query (i.e. an empty insert) even in cases where there are no roles to insert. This apparently works (at least with the MySQL database driver) but leads to PHP notices when printing the query (which is INSERT INTO {users_roles} (uid, rid) VALUES (), i.e. nonsensical) and I have no idea what happens for other databases.

The whole point of the previous code is to avoid ever attempting a database query in those situations, so we should do that here too.

Something like the attached.

This patch also seems like it could benefit from tests....

I am using rules to create a user. And I seem to be running into the issue here, its been quite a ride to find this issue.

@David Thanks for your patch, but I still seem to get the following error:

PDOException: SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry '322-20-acl' for key 'PRIMARY': INSERT INTO {node_access} (nid, realm, gid, grant_view, grant_update, grant_delete) VALUES (:db_insert_placeholder_0, :db_insert_placeholder_1, :db_insert_placeholder_2, :db_insert_placeholder_3, :db_insert_placeholder_4, :db_insert_placeholder_5), (:db_insert_placeholder_6, :db_insert_placeholder_7, :db_insert_placeholder_8, :db_insert_placeholder_9, :db_insert_placeholder_10, :db_insert_placeholder_11); Array ( [:db_insert_placeholder_0] => 322 [:db_insert_placeholder_1] => acl [:db_insert_placeholder_2] => 20 [:db_insert_placeholder_3] => 1 [:db_insert_placeholder_4] => 0 [:db_insert_placeholder_5] => 0 [:db_insert_placeholder_6] => 322 [:db_insert_placeholder_7] => content_access_rid [:db_insert_placeholder_8] => 3 [:db_insert_placeholder_9] => 1 [:db_insert_placeholder_10] => 0 [:db_insert_placeholder_11] => 0 ) in node_access_write_grants() (line 3548 of /modules/node/node.module).

Not sure if we have the same problem there. But it seems like it still occurs, when carrying out a node_acces_write_grants. I have a small rule that creates a user, than tries to set a message and a field. It basically fails when I try to add set data to a field.

I have tried all the patches, above none seem to resolve the problem also noted in https://drupal.org/node/1771666

#42 Worked for me. Thanks David

#42 worked for me too. Thank you for having this here- saved my day!

No code has been committed yet, so the issue is not fixed.

#42 worked for me, thank you very much.

I have now experienced this problem (very similar) while using Entity Registration 7x1.3 and 7x1-dev with Rules 7x2-7.

see: https://www.drupal.org/node/2311263#comment-9005795

I applied the user.module update in #42 but this has not fixed the issue. Submitting a registration cause the PHP error:

PDOException: SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry 'registration-6-0-0-und' for key 'PRIMARY': INSERT INTO..

Not until I disable the rules that affect user entity will registrations work.

Edit: The patch does work in that once the rules are disabled you can submit the registration. When I tried to submit registration using the unpatched user.module version I got the error again.

I just applied #42 to 7.32 and everything works as intended.

#45, #46, #47, #49 and me tested the patch successfully -> RTBC.

To #43 and #50: This issue doesn't seem to be related (or just in parts) to your issue. This one is about user roles on user registration while you have additional problems with the {node_access} table.
Please try to track down your issue to either contrib (rules?) or core and open a new topic at the appropriate project.

This patch already missed some possibilities for getting committed and I would be glad to see it in the next version :)

This was RTBC but then bumped back due to a bogus testbot failure. So I think we can commit it.

It would still be ideal to have tests for this, but I guess it's not absolutely critical for this particular issue and the patch has been here a while... plus it might make sense to have similar tests for Drupal 8 too. So I think they could be a followup.

Committed to 7.x - thanks!

Any patch for mine? Because #42 is only for file "/modules/user/user.module" and is already committed and integrated with my current version of Drupal.

Mine is an issue with the file "/includes/common.inc" and relates to the "commerce license billing" module

PDOException: SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry '' for key 'name': INSERT INTO {cl_billing_cycle_type} (engine, name, title) VALUES (:db_insert_placeholder_0, :db_insert_placeholder_1, :db_insert_placeholder_2); Array ( [:db_insert_placeholder_0] => periodic [:db_insert_placeholder_1] => [:db_insert_placeholder_2] => Quarterly ) in drupal_write_record() (line 7239 of /includes/common.inc)

@kebz
Different issue, try posting in the commerce license billing modules issue queue. A duplicate entry key is fairly common issue in a lot of different modules, due to a lot of different unrelated issues.

@Alan D

Yes, I understand. I posted there before here. Nobody is responding, so I thought I'd reach out elsewhere in case there's a generic answer/solution available.

@kebz
I had the same issue.
By setting $account->is_new = FALSE solved issue.

@adil2345

Good morning! Thank you for the suggestion.... I'm not quite sure where to put it... lol

Are you suggesting that I should paste it in the patch file here? > https://www.drupal.org/files/issues/drupal-fix-user-insert-roles-1433288...

Thanks in advance and a Happy New Year to all =)