From cd9fe75554d06aeb9b16631d44ac3c7be89614b8 Mon Sep 17 00:00:00 2001 From: RhodeCode Admin Date: Tue, 23 Apr 2024 15:25:21 +0200 Subject: [PATCH] fix(caching): fixed problems with Cache query for users. The old way of querying caused the user get query to be always cached, and returning old results even in 2fa forms. The new limited query doesn't cache the user object resolving issues --- rhodecode/apps/my_account/views/my_account.py | 13 ++++-- rhodecode/model/db.py | 45 ++++++++++--------- rhodecode/model/user.py | 11 +++-- .../my_account/my_account_bookmarks.mako | 4 +- rhodecode/templates/base/base.mako | 14 +++--- 5 files changed, 52 insertions(+), 35 deletions(-) diff --git a/rhodecode/apps/my_account/views/my_account.py b/rhodecode/apps/my_account/views/my_account.py index f17088c9..254be2ba 100644 --- a/rhodecode/apps/my_account/views/my_account.py +++ b/rhodecode/apps/my_account/views/my_account.py @@ -40,7 +40,7 @@ from rhodecode.lib.utils2 import safe_int, md5, str2bool from rhodecode.model.auth_token import AuthTokenModel from rhodecode.model.comment import CommentsModel from rhodecode.model.db import ( - IntegrityError, or_, in_filter_generator, + IntegrityError, or_, in_filter_generator, select, Repository, UserEmailMap, UserApiKeys, UserFollowing, PullRequest, UserBookmark, RepoGroup, ChangesetStatus) from rhodecode.model.meta import Session @@ -511,8 +511,15 @@ class MyAccountView(BaseAppView, DataGridAppView): def my_account_bookmarks(self): c = self.load_default_context() c.active = 'bookmarks' - c.bookmark_items = UserBookmark.get_bookmarks_for_user( - self._rhodecode_db_user.user_id, cache=False) + + user_bookmarks = \ + select(UserBookmark, Repository, RepoGroup) \ + .where(UserBookmark.user_id == self._rhodecode_user.user_id) \ + .outerjoin(Repository, Repository.repo_id == UserBookmark.bookmark_repo_id) \ + .outerjoin(RepoGroup, RepoGroup.group_id == UserBookmark.bookmark_repo_group_id) \ + .order_by(UserBookmark.position.asc()) + + c.user_bookmark_items = Session().execute(user_bookmarks).all() return self._get_template_context(c) def _process_bookmark_entry(self, entry, user_id): diff --git a/rhodecode/model/db.py b/rhodecode/model/db.py index c7330451..e479f99a 100644 --- a/rhodecode/model/db.py +++ b/rhodecode/model/db.py @@ -884,17 +884,6 @@ class User(Base, BaseModel): Session().commit() return new_recovery_codes - @classmethod - def get(cls, user_id, cache=False): - if not user_id: - return - - user = cls.query() - if cache: - user = user.options( - FromCache("sql_cache_short", f"get_users_{user_id}")) - return user.get(user_id) - @classmethod def extra_valid_auth_tokens(cls, user, role=None): tokens = UserApiKeys.query().filter(UserApiKeys.user == user)\ @@ -1021,12 +1010,23 @@ class User(Base, BaseModel): @user_data.setter def user_data(self, val): if not isinstance(val, dict): - raise Exception('user_data must be dict, got %s' % type(val)) + raise Exception(f'user_data must be dict, got {type(val)}') try: self._user_data = safe_bytes(json.dumps(val)) except Exception: log.error(traceback.format_exc()) + @classmethod + def get(cls, user_id, cache=False): + if not user_id: + return + + user = cls.query() + if cache: + user = user.options( + FromCache("sql_cache_short", f"get_users_{user_id}")) + return user.get(user_id) + @classmethod def get_by_username(cls, username, case_insensitive=False, cache=False): @@ -1611,7 +1611,7 @@ class UserGroup(Base, BaseModel): user_group = cls.query() if cache: user_group = user_group.options( - FromCache("sql_cache_short", "get_users_group_%s" % user_group_id)) + FromCache("sql_cache_short", f"get_users_group_{user_group_id}")) return user_group.get(user_group_id) def permissions(self, with_admins=True, with_owner=True, @@ -2005,7 +2005,7 @@ class Repository(Base, BaseModel): if val: return val else: - cache_key = "get_repo_by_name_%s" % _hash_key(repo_name) + cache_key = f"get_repo_by_name_{_hash_key(repo_name)}" q = q.options( FromCache("sql_cache_short", cache_key)) @@ -5643,18 +5643,23 @@ class UserBookmark(Base, BaseModel): @classmethod def get_bookmarks_for_user(cls, user_id, cache=True): - bookmarks = cls.query() \ - .filter(UserBookmark.user_id == user_id) \ - .options(joinedload(UserBookmark.repository)) \ - .options(joinedload(UserBookmark.repository_group)) \ + bookmarks = select( + UserBookmark.title, + UserBookmark.position, + ) \ + .add_columns(Repository.repo_id, Repository.repo_type, Repository.repo_name) \ + .add_columns(RepoGroup.group_id, RepoGroup.group_name) \ + .where(UserBookmark.user_id == user_id) \ + .outerjoin(Repository, Repository.repo_id == UserBookmark.bookmark_repo_id) \ + .outerjoin(RepoGroup, RepoGroup.group_id == UserBookmark.bookmark_repo_group_id) \ .order_by(UserBookmark.position.asc()) if cache: bookmarks = bookmarks.options( - FromCache("sql_cache_short", "get_user_{}_bookmarks".format(user_id)) + FromCache("sql_cache_short", f"get_user_{user_id}_bookmarks") ) - return bookmarks.all() + return Session().execute(bookmarks).all() def __repr__(self): return f'' diff --git a/rhodecode/model/user.py b/rhodecode/model/user.py index 18e33254..69af24c0 100644 --- a/rhodecode/model/user.py +++ b/rhodecode/model/user.py @@ -114,6 +114,7 @@ class UserModel(BaseModel): else: user = self.sa.query(User)\ .filter(User.username == username) + if cache: name_key = _hash_key(username) user = user.options( @@ -765,25 +766,29 @@ class UserModel(BaseModel): 'AuthUser: fill data execution based on: ' 'user_id:%s api_key:%s username:%s', user_id, api_key, username) try: + found_with = '' dbuser = None if user_id: dbuser = self.get(user_id) + found_with = 'user_id' elif api_key: dbuser = self.get_by_auth_token(api_key) + found_with = 'auth_token' elif username: dbuser = self.get_by_username(username) + found_with = 'username' if not dbuser: log.warning( - 'Unable to lookup user by id:%s api_key:%s username:%s', - user_id, token_obfuscate(api_key), username) + 'Unable to lookup user by id:%s api_key:%s username:%s, found with: %s', + user_id, token_obfuscate(api_key), username, found_with) return False if not dbuser.active: log.debug('User `%s:%s` is inactive, skipping fill data', username, user_id) return False - log.debug('AuthUser: filling found user:%s data', dbuser) + log.debug('AuthUser: filling found user:%s data, found with: %s', dbuser, found_with) attrs = { 'user_id': dbuser.user_id, diff --git a/rhodecode/templates/admin/my_account/my_account_bookmarks.mako b/rhodecode/templates/admin/my_account/my_account_bookmarks.mako index 5e0a6e8b..559d8593 100644 --- a/rhodecode/templates/admin/my_account/my_account_bookmarks.mako +++ b/rhodecode/templates/admin/my_account/my_account_bookmarks.mako @@ -83,14 +83,14 @@ ## generate always 10 entries - % for item in (c.bookmark_items + [None for i in range(10)])[:10]: + % for item in (c.user_bookmark_items + [None for i in range(10)])[:10]: % if item is None: ## empty placehodlder ${form_item()} % else: ## actual entry - ${form_item(position=item.position, title=item.title, redirect_url=item.redirect_url, repo=item.repository, repo_group=item.repository_group)} + ${form_item(position=item[0].position, title=item[0].title, redirect_url=item[0].redirect_url, repo=item[1], repo_group=item[2])} % endif % endfor diff --git a/rhodecode/templates/base/base.mako b/rhodecode/templates/base/base.mako index 3b81decc..51a7a0c7 100644 --- a/rhodecode/templates/base/base.mako +++ b/rhodecode/templates/base/base.mako @@ -651,26 +651,26 @@ % endif % for item in c.bookmark_items:
  • - % if item.repository: + % if item.repo_id:
    ${item.position} - % if item.repository.repo_type == 'hg': + % if item.repo_type == 'hg': - % elif item.repository.repo_type == 'git': + % elif item.repo_type == 'git': - % elif item.repository.repo_type == 'svn': + % elif item.repo_type == 'svn': % endif - ${(item.title or h.shorter(item.repository.repo_name, 30))} + ${(item.title or h.shorter(item.repo_name, 30))}
    - % elif item.repository_group: + % elif item.group_id:
    ${item.position} - ${(item.title or h.shorter(item.repository_group.group_name, 30))} + ${(item.title or h.shorter(item.group_name, 30))}
    % else: