Problem
When generating the $user->roles array, we used to be able to grab the rids and the names in one query. With the move of roles to CMI (#1479454: Convert user roles to configurables), this is no longer possible. We can grab the rids from {user_roles} but then we need to open a config object to match up the names. This happens on cached page loads in _drupal_session_read(), so it would be nice not to have that overhead.
Solution
Remove the role names from the $user->roles array, and just store the rids. This shouldn't be a problem in D8 now that the rids are machine names and human readable.
Unsure what kind of chaos this causes, gonna throw together a patch here and just see what fails.
Comments
Comment #1
gddSeeing what the bot thinks
Comment #2
sunDo eeeeet!
Comment #3
sunhah! Two brilliant minds and so on... ;)
Comment #5
gddDuh.
Comment #6
gddComment #7
gddNo that's broken too, hang on
Comment #8
gddComment #10
gddThat one is broken too because of debug code I left in, I'm just being dumb now. Although it appears the testbot is being a bit weird today too.
Comment #11
Tor Arne Thune CreditAttribution: Tor Arne Thune commentedComment #13
gddOK, indexed arrays in roles is bad. This is the same as the first one, it just uses fetchAllKeyed(0,0) to make the array key and value both be the rid. Sun had actually already done that in his patch with drupal_map_assoc() and I just made them both consistent.
Comment #14
gddComment #16
gddThis passes for me locally.
I don't think we actually should change user_roles() as that function is used in a lot of places to display a list of roles, and as such should include the role names (that is where the above test failures came from.) We only really want to change the $roles array on the user object, and user_roles() is never actually used for that purpose.
This patch also cleans up a few more hardcoded role names.
Comment #18
gddThat failure isn't happening for me locally, but I have a cleanup patch anyways that
- Reverts the removal of the 'Administrative features' comment
- Fixes a small spacing issue
Hopefully a retest will go ok
Comment #19
sunAwesome!
Comment #20
Dries CreditAttribution: Dries commentedCommitted to 8.x. Thanks!
Comment #21.0
(not verified) CreditAttribution: commentedUpdated issue summary.