From 5eae0a5cddb87c971d17c01fad9ec7750a138c1c Mon Sep 17 00:00:00 2001 From: Classic298 <27028174+Classic298@users.noreply.github.com> Date: Fri, 17 Apr 2026 04:23:08 +0200 Subject: [PATCH] perf(users): drop redundant get_user_by_id refetch in session-user endpoints (#23794) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * perf(users): drop redundant get_user_by_id refetch in session-user endpoints Five /user/* handlers refetched the user row via Users.get_user_by_id(user.id) immediately after receiving an identical UserModel from Depends(get_verified_user). Since get_verified_user already populated the user within the same request microseconds earlier, the refetch is pure overhead. The dead else branches (unreachable — get_verified_user raises 401 on missing user) are removed as a natural consequence. Affected endpoints: - GET /user/settings - GET /user/status - POST /user/status/update - GET /user/info - POST /user/info/update Eliminates one SELECT per request to each of these endpoints with no behavioral change. * fix(users): preserve USER_NOT_FOUND error on status update failure update_user_status_by_id returns None when the target user is missing or the update raises. The previous commit removed the pre-update existence gate (get_user_by_id) and returned the update result directly, which turned not-found/failure cases into 200 OK with a null body instead of the expected 400 USER_NOT_FOUND. Guard the update result explicitly to preserve the original API contract, matching the equivalent pattern already applied in /user/info/update. * docs(users): note lost-update tradeoff on /user/info/update Make the concurrency tradeoff explicit: merging against the auth-time snapshot slightly widens the lost-update window compared to the previous pre-merge refetch, but the refetch only narrowed (did not eliminate) that window. Real safety requires row locking or a version column. --------- Co-authored-by: Claude --- backend/open_webui/routers/users.py | 67 +++++++++-------------------- 1 file changed, 21 insertions(+), 46 deletions(-) diff --git a/backend/open_webui/routers/users.py b/backend/open_webui/routers/users.py index 9fd2479ad..d8bfcefac 100644 --- a/backend/open_webui/routers/users.py +++ b/backend/open_webui/routers/users.py @@ -276,14 +276,8 @@ async def update_default_user_permissions(request: Request, form_data: UserPermi async def get_user_settings_by_session_user( user=Depends(get_verified_user), db: AsyncSession = Depends(get_async_session) ): - user = await Users.get_user_by_id(user.id, db=db) - if user: - return user.settings - else: - raise HTTPException( - status_code=status.HTTP_400_BAD_REQUEST, - detail=ERROR_MESSAGES.USER_NOT_FOUND, - ) + # user already fetched by get_verified_user — no need to refetch + return user.settings ############################ @@ -339,14 +333,8 @@ async def get_user_status_by_session_user( status_code=status.HTTP_403_FORBIDDEN, detail=ERROR_MESSAGES.ACTION_PROHIBITED, ) - user = await Users.get_user_by_id(user.id, db=db) - if user: - return user - else: - raise HTTPException( - status_code=status.HTTP_400_BAD_REQUEST, - detail=ERROR_MESSAGES.USER_NOT_FOUND, - ) + # user already fetched by get_verified_user — no need to refetch + return user ############################ @@ -366,15 +354,14 @@ async def update_user_status_by_session_user( status_code=status.HTTP_403_FORBIDDEN, detail=ERROR_MESSAGES.ACTION_PROHIBITED, ) - user = await Users.get_user_by_id(user.id, db=db) - if user: - user = await Users.update_user_status_by_id(user.id, form_data, db=db) - return user - else: - raise HTTPException( - status_code=status.HTTP_400_BAD_REQUEST, - detail=ERROR_MESSAGES.USER_NOT_FOUND, - ) + # user already fetched by get_verified_user — no need to refetch + updated = await Users.update_user_status_by_id(user.id, form_data, db=db) + if updated: + return updated + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail=ERROR_MESSAGES.USER_NOT_FOUND, + ) ############################ @@ -384,14 +371,8 @@ async def update_user_status_by_session_user( @router.get('/user/info', response_model=Optional[dict]) async def get_user_info_by_session_user(user=Depends(get_verified_user), db: AsyncSession = Depends(get_async_session)): - user = await Users.get_user_by_id(user.id, db=db) - if user: - return user.info - else: - raise HTTPException( - status_code=status.HTTP_400_BAD_REQUEST, - detail=ERROR_MESSAGES.USER_NOT_FOUND, - ) + # user already fetched by get_verified_user — no need to refetch + return user.info ############################ @@ -403,19 +384,13 @@ async def get_user_info_by_session_user(user=Depends(get_verified_user), db: Asy async def update_user_info_by_session_user( form_data: dict, user=Depends(get_verified_user), db: AsyncSession = Depends(get_async_session) ): - user = await Users.get_user_by_id(user.id, db=db) - if user: - if user.info is None: - user.info = {} - - user = await Users.update_user_by_id(user.id, {'info': {**user.info, **form_data}}, db=db) - if user: - return user.info - else: - raise HTTPException( - status_code=status.HTTP_400_BAD_REQUEST, - detail=ERROR_MESSAGES.USER_NOT_FOUND, - ) + # Merges against the auth-time snapshot of user.info. The previous pre-merge + # refetch only narrowed (did not eliminate) the lost-update window on concurrent + # same-user writes; real safety needs row locking or a version column. + existing_info = user.info or {} + updated = await Users.update_user_by_id(user.id, {'info': {**existing_info, **form_data}}, db=db) + if updated: + return updated.info else: raise HTTPException( status_code=status.HTTP_400_BAD_REQUEST,