User role permission assignments are not deleted when a role is deleted

Issue was reproduced and confirmed to still be happening. Looking at this for SprintWeekend2015.

Here's a patch that calls RoleStorage::deleteRoleReferences() upon deletion of a role.

However for the first pass at this I've left it in RoleStorage rather than UserStorage. It could be argued that since cleaning up these references would be the responsibility of the Role the method should live there. Furthermore the methods in UserStorage seem to mostly be related so singular User entities and not the multiple as this method would address.

However it could absolutely run from UserStorage as well, you'd just swap:

$user_storage = \Drupal::entityManager()->getStorage('user');
$user_storage->resetCache();

for

$this->resetCache();

I've successfully tested the patch in #3.

You can create a role, assign it to a user, delete the role, recreate the role, and it doesn't 're-inherit' the original assignment.

Thanks @trevortwining, @adamwhite and @iPat for working on this!

1. +++ b/core/modules/user/src/Entity/Role.php
   @@ -154,4 +154,14 @@ public function postSave(EntityStorageInterface $storage, $update = TRUE) {
   +      $user_storage = \Drupal::entityManager()->getStorage('user');
   

   We could use $this->entityManager() here, it is already injected.

2. +++ b/core/modules/user/src/UserStorage.php
   @@ -116,4 +116,16 @@ public function updateLastAccessTimestamp(AccountInterface $account, $timestamp)
   +    $this->resetCache();
   

   @berdir: Do you think this enough? Shouldn't the role cache be deleted too?

Talked with @Berdir on irc, and for #11.2 we don't need anything else.

Instead of #11.1, let's do user_role_delete() in user.module as suggested in #10.

Looks great, let's see what the testbot thinks!

Only minor issues:

1. +++ b/core/modules/user/src/Entity/Role.php
   @@ -153,5 +153,4 @@ public function postSave(EntityStorageInterface $storage, $update = TRUE) {
   -
   

   We shouldn't remove this space :/

2. +++ b/core/modules/user/src/Tests/UserRoleDeleteTest.php
   @@ -0,0 +1,77 @@
   +    // Create new role with same role as test role one
   

   "with same name"

3. +++ b/core/modules/user/src/Tests/UserRoleDeleteTest.php
   @@ -0,0 +1,77 @@
   +    // Check that user still does not have role one.
   

   Remove the "still"

4. +++ b/core/modules/user/src/Tests/UserRoleDeleteTest.php
   @@ -0,0 +1,77 @@
   +  }
   +}
   

   Needs a blank line before the class end bracket per coding standards.

Looks good to me. Awesome work @iPat!
Filled beta evaluation form.

Thanks for taking the ball on this @iPat, I've tested on here and it looks great to me.

Committed 99377f9 and pushed to 8.0.x. Thanks!

Thanks for adding the beta evaluation to the issue summary.

I discussed this with @Berdir in IRC - he pointed out that the general problem of entity reference cleanup is more a problem for config entities than content, because there is no risk of accidental re-assignment for them. I think we should open a followup to work on a generic fix.

I think we should open a followup to work on a generic fix.

That follow-up now exists: #2723323: [META] Clean up references to deleted entities.