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()
.
Comment | File | Size | Author |
---|---|---|---|
#16 | interdiff-3-16.txt | 1.53 KB | Hardik_Patel_12 |
#16 | user-data_cache_invalidate-2477903-16.patch | 2.38 KB | Hardik_Patel_12 |
Issue fork drupal-2477903
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:
Comments
Comment #1
plachThis 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 inUser::preSave()
. One possible fix could adding a parameter to skip cache tag invalidation (defaulting to FALSE).Comment #3
plachRemoved unrelated hunk
Comment #4
Fabianx CreditAttribution: Fabianx commentedI 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.
Comment #5
plachThat's entirely possible :)
Comment #6
dawehnerIts a bit odd that we allow multiple $uids, even the set method gets the $uid as singular.
Comment #7
plachThe 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.Comment #8
Fabianx CreditAttribution: Fabianx commentedComment #10
dawehnerI'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.Comment #16
Hardik_Patel_12 CreditAttribution: Hardik_Patel_12 at QED42 for Drupal India Association commentedRe-rolled patch for D8.9.X and removing cache invalidation from testContactLink function.
Comment #19
BramDriesenI tested this patch and it seems to work fine.
Comment #20
BerdirWhat #10 said. There are also no tests yet.
Comment #21
catchComment #26
acbramley CreditAttribution: acbramley at PreviousNext commentedThis issue came up as part of the BSI daily triage.
The issue summary needs an update to reflect the changes mentioned in #10.
Comment #28
apadernoComment #30
Prashant.cTook 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