Problem/Motivation

The User data service offers a public API to store additional data related to individual users. The related UserDataInterface exposes two methods that perform storage writes: set() and delete(). The default implementation does not take cache tag invalidation into account, which may cause stale cache entries not to be invalidated.

Proposed resolution

Add cache invalidation to UserData::set() and UserData::delete().

Issue fork drupal-2477903

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

plach’s picture

Status: Active » Needs review
Issue tags: +Needs tests
FileSize
2.27 KB

This is a quick patch I put together, it has the problem that we'd be invalidating cache tags each time ::set() is called, which would perform very bed when multiple properties need to be set. Not sure whether that's a common case, but we have such an example in User::preSave(). One possible fix could adding a parameter to skip cache tag invalidation (defaulting to FALSE).

Status: Needs review » Needs work

The last submitted patch, 1: user-data_cache_invalidate-2477903-1.patch, failed testing.

plach’s picture

Status: Needs work » Needs review
FileSize
1.64 KB

Removed unrelated hunk

Fabianx’s picture

I was under the impression that at least in the mysql cache tags invalidator we have protection against multiple invalidations of the same tag in the same request.

If not however, this should be fixed in the invalidator and not here in my opinion.

plach’s picture

That's entirely possible :)

dawehner’s picture

+++ b/core/modules/user/src/UserData.php
@@ -119,4 +125,25 @@ public function delete($module = NULL, $uid = NULL, $name = NULL) {
+   * @param int $uids
+   *   The user account ID the data is associated with.

Its a bit odd that we allow multiple $uids, even the set method gets the $uid as singular.

plach’s picture

The delete method allows multiple ids to be specified, although the parameter is called $uid. If Fabian is correct about invalidating tags multiple times in the same request being ok, we should add the call also on delete.

Fabianx’s picture

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

dawehner’s picture

Status: Needs review » Needs work
  1. +++ b/core/modules/user/src/UserData.php
    @@ -99,12 +101,16 @@ public function set($module, $uid, $name, $value) {
    +    // We do not need to invalidate cache tags here as this is invoked in
    +    // User::postDelete().
    

    I'm not sure this makes sense ... User::postDelete() is just executed when the entire user is removed. In this method you can also remove SOME of the user data.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

Bug reports should be targeted against the 8.6.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Hardik_Patel_12’s picture

Version: 8.6.x-dev » 8.9.x-dev
Status: Needs work » Needs review
FileSize
2.38 KB
1.53 KB

Re-rolled patch for D8.9.X and removing cache invalidation from testContactLink function.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

BramDriesen’s picture

Status: Needs review » Reviewed & tested by the community

I tested this patch and it seems to work fine.

Berdir’s picture

+++ b/core/modules/user/src/UserData.php
@@ -89,12 +91,15 @@ public function set($module, $uid, $name, $value) {
    */
   public function delete($module = NULL, $uid = NULL, $name = NULL) {
+    // We do not need to invalidate cache tags here as this is invoked in
+    // User::postDelete().

What #10 said. There are also no tests yet.

catch’s picture

Status: Reviewed & tested by the community » Needs work

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

acbramley’s picture

This issue came up as part of the BSI daily triage.

The issue summary needs an update to reflect the changes mentioned in #10.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

apaderno’s picture

Issue summary: View changes
Issue tags: -Needs issue summary update

Prashant.c made their first commit to this issue’s fork.

Prashant.c’s picture

Took the changes from #16 to create a MR.

It seems still a lot of work is left to be done in this. Keeping in NW.

Thanks