Contrib.social

Rename the 'user.permissions' context to 'user.roles.permissions', to ensure correct cache context optimization

Open on Drupal.org →
Created on 16 June 2016, over 8 years ago
Updated 7 November 2023, about 1 year ago

Summary

I've been talking to Wim Leers about cache contexts and during our discussion I came to the conclusion that user.permissions and user.roles aren't really sibling cache contexts. Instead, user.permissions is a logical child of user.roles and should thus become user.roles.permissions.

Explanation

Suppose you have a block on the page that shows a list of your permissions. This block obviously varies by the current user's permissions. Keep in mind that there could be two users with totally different roles that end up with the same permissions because the combination of their roles so happens to add up to the same permission set.

Great, so now we want to add a block to that page that shows a list of your user roles. Again, this block obviously varies by the current user's roles. Now here's the kicker: Any two users with the same roles automatically have the same permissions. In light of that, user.roles is a logical parent to user.roles.permissions because it's more granular.

Finally, for argument's sake, let's add a block showing your user ID to the page. This block naturally varies per user. Should the previous two blocks now also vary per user, then that would be fine as they would most definitely show the right roles and permissions. So user is still a logical parent to both user.roles and user.roles.permissions

What needs to be done?

The permission cache context needs to move to user.roles.permissions

That's it! Permissions already correctly set the roles' metadata when optimized away, so in a sense it already knew it was a more logical child of roles than it was of users.

📌 Task
Status

Closed: won't fix

Version

11.0 🔥

Component
Cache  →

Last updated 3 days ago

Created by

🇧🇪Belgium kristiaanvandeneynde Antwerp, Belgium

Live updates comments and jobs are added and updated live.
  • Needs change record

    A change record needs to be drafted before an issue is committed. Note: Change records used to be called change notifications.

  • Needs documentation

    A documentation change is requested elsewhere. For Drupal core (and possibly other projects), once the change has been committed, this status should be recorded in a change record node.

Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • Comment almost 2 years ago →
    needs-review-queue-bot

    The Needs Review Queue Bot → tested this issue. It either no longer applies to Drupal core, or fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

    Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.

    Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

  • Comment about 1 year ago →
    🇧🇪Belgium kristiaanvandeneynde Antwerp, Belgium

    With 🌱 [Meta, Plan] Pitch-Burgh: Policy based access in core Active around the corner (hopefully?), this issue no longer makes sense.

    Current situation: Your permissions are not calculated based merely on your role, but also on whether or not you are user 1, as rightfully pointed out in #33. So until that was fixed, we could never resolve this issue.

    Future situation (again, hopefully): Your permissions are calculated based on an unpredictable amount of access policies, some of which being your user roles and your UID1 status. So as soon as we have access policies in core, we will never be able to fold the user.permissions cache context into user.roles and for good reason.

    So if everyone agrees, we can close this issue as won't fix as it's looking more likely that access policies will turn this into a moot point. Furthermore, access policies will allow us to land 📌 Add a container parameter that can remove the special behavior of UID#1 Fixed after ~14-15 years :)

  • Status changed to Closed: won't fix about 1 year ago
  • Comment about 1 year ago →
    🇬🇧United Kingdom catch

    Yes agreed, let's won't fix it. user.roles is not often used so even if we hadn't broken the optimization possibility, it would have been a relatively marginal one.

contrib.social Blog FAQ Discussions
Production build 0.71.5 2024