Skip to content

Invalidate the cache of `personal projects count` correctly

Manoj M J requested to merge 271632-administrator-can-inadvertently-brick-user-project-creation-ability into master

What does this MR do and why?

Premise

There are only two hard things in Computer Science: cache invalidation and naming things. -- Phil Karlton

This issue once again proves that the above statement is right 😄

We store the count of user.personal_projects.count in cache (as user.personal_projects_count) and use it when necessary instead of issuing another SQL query again. Personal project counts are necessary to figure out whether a user can create their next personal project (eg: if their limit is set to 2 by instance admin, they should not be able to create a 3rd personal project)

While we do clear this cache upon creation and deletion of projects, we weren't considering 100% of the possible set of cases. Because, a project can be deleted in 2 ways:

  • The creator of the project itself deletes the project - we have no problem in this scenario as cache invalidation works correctly.
  • An instance admin can delete the project created by any other user - we have a problem here now because we were invalidating the cache by calling current_user.invalidate_personal_projects_count, and when this happens, the current_user is the admin, and clearing the admin's personal projects cache does no good with respect to the cache of the project's creator.

Solution

Cache shouldn't be cleared based on current_user, because current_user could be anyone with enough privilege to perform the action, and not necessarily the project's creator/owner.

Thus, it has been moved to a project.invalidate_personal_projects_count_of_owner call.

Other points:

  • current_user.invalidate_personal_projects_count used to clear cache even for non-personal projects, ie, projects in groups, which is unnecessary. project.invalidate_personal_projects_count_of_owner only clears the cache for personal projects.
  • Fixes cache clearing process during project creation and project deletion.
  • The project transfer process hasn't been clearing caches at all till now, which has now been fixed.

Screenshots or screen recordings

Screenshots are required for UI changes, and strongly recommended for all other merge requests.

How to set up and validate locally

Steps are in the description of #271632 (closed)

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Related to #271632 (closed)

Edited by Manoj M J

Merge request reports