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
This commit is contained in:
RhodeCode Admin
parent
12af6697b8
commit
cd9fe75554
5 changed files with 52 additions and 35 deletions
|
|
@ -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):
|
||||
|
|
|
|||
|
|
@ -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'<UserBookmark({self.position} @ {self.redirect_url!r})>'
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -83,14 +83,14 @@
|
|||
<table class="rctable">
|
||||
## generate always 10 entries
|
||||
<input type="hidden" name="__start__" value="bookmarks:sequence"/>
|
||||
% 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]:
|
||||
<input type="hidden" name="__start__" value="bookmark:mapping"/>
|
||||
% 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
|
||||
<input type="hidden" name="__end__" value="bookmark:mapping"/>
|
||||
% endfor
|
||||
|
|
|
|||
|
|
@ -651,26 +651,26 @@
|
|||
% endif
|
||||
% for item in c.bookmark_items:
|
||||
<li>
|
||||
% if item.repository:
|
||||
% if item.repo_id:
|
||||
<div>
|
||||
<a class="bookmark-item" href="${h.route_path('my_account_goto_bookmark', bookmark_id=item.position)}">
|
||||
<code>${item.position}</code>
|
||||
% if item.repository.repo_type == 'hg':
|
||||
% if item.repo_type == 'hg':
|
||||
<i class="icon-hg" title="${_('Repository')}" style="font-size: 16px"></i>
|
||||
% elif item.repository.repo_type == 'git':
|
||||
% elif item.repo_type == 'git':
|
||||
<i class="icon-git" title="${_('Repository')}" style="font-size: 16px"></i>
|
||||
% elif item.repository.repo_type == 'svn':
|
||||
% elif item.repo_type == 'svn':
|
||||
<i class="icon-svn" title="${_('Repository')}" style="font-size: 16px"></i>
|
||||
% endif
|
||||
${(item.title or h.shorter(item.repository.repo_name, 30))}
|
||||
${(item.title or h.shorter(item.repo_name, 30))}
|
||||
</a>
|
||||
</div>
|
||||
% elif item.repository_group:
|
||||
% elif item.group_id:
|
||||
<div>
|
||||
<a class="bookmark-item" href="${h.route_path('my_account_goto_bookmark', bookmark_id=item.position)}">
|
||||
<code>${item.position}</code>
|
||||
<i class="icon-repo-group" title="${_('Repository group')}" style="font-size: 14px"></i>
|
||||
${(item.title or h.shorter(item.repository_group.group_name, 30))}
|
||||
${(item.title or h.shorter(item.group_name, 30))}
|
||||
</a>
|
||||
</div>
|
||||
% else:
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue