From 9301df860eab8545b79010c051f82a595454c31b Mon Sep 17 00:00:00 2001 From: Wind-Explorer Date: Mon, 15 Dec 2025 20:00:55 +0800 Subject: [PATCH] user sync optinization --- src/auth/auth.service.spec.ts | 77 ++++++++++++++++++++++++++ src/auth/auth.service.ts | 48 ++++++++++++++++ src/friends/friends.controller.spec.ts | 2 + src/friends/friends.controller.ts | 16 +++--- src/users/users.controller.spec.ts | 11 ++-- src/users/users.controller.ts | 10 ++-- 6 files changed, 148 insertions(+), 16 deletions(-) diff --git a/src/auth/auth.service.spec.ts b/src/auth/auth.service.spec.ts index 27bbb0d..41aaaf4 100644 --- a/src/auth/auth.service.spec.ts +++ b/src/auth/auth.service.spec.ts @@ -8,9 +8,11 @@ describe('AuthService', () => { let service: AuthService; const mockCreateFromToken = jest.fn(); + const mockFindByKeycloakSub = jest.fn(); const mockUsersService = { createFromToken: mockCreateFromToken, + findByKeycloakSub: mockFindByKeycloakSub, }; const mockAuthUser: AuthenticatedUser = { @@ -202,6 +204,81 @@ describe('AuthService', () => { }); }); + describe('ensureUserExists', () => { + it('should return existing user without updating', async () => { + mockFindByKeycloakSub.mockResolvedValue(mockUser); + + const result = await service.ensureUserExists(mockAuthUser); + + expect(result).toEqual(mockUser); + expect(mockFindByKeycloakSub).toHaveBeenCalledWith('f:realm:user123'); + expect(mockCreateFromToken).not.toHaveBeenCalled(); + }); + + it('should create new user if user does not exist', async () => { + mockFindByKeycloakSub.mockResolvedValue(null); + mockCreateFromToken.mockResolvedValue(mockUser); + + const result = await service.ensureUserExists(mockAuthUser); + + expect(result).toEqual(mockUser); + expect(mockFindByKeycloakSub).toHaveBeenCalledWith('f:realm:user123'); + expect(mockCreateFromToken).toHaveBeenCalledWith({ + keycloakSub: 'f:realm:user123', + email: 'test@example.com', + name: 'Test User', + username: 'testuser', + picture: 'https://example.com/avatar.jpg', + roles: ['user', 'premium'], + }); + }); + + it('should handle user with no email when creating new user', async () => { + const authUserNoEmail: AuthenticatedUser = { + keycloakSub: 'f:realm:user456', + name: 'No Email User', + }; + + mockFindByKeycloakSub.mockResolvedValue(null); + mockCreateFromToken.mockResolvedValue({ + ...mockUser, + email: '', + name: 'No Email User', + }); + + await service.ensureUserExists(authUserNoEmail); + + expect(mockFindByKeycloakSub).toHaveBeenCalledWith('f:realm:user456'); + expect(mockCreateFromToken).toHaveBeenCalledWith( + expect.objectContaining({ + email: '', + name: 'No Email User', + }), + ); + }); + + it('should use "Unknown User" when creating user with no name or username', async () => { + const authUserMinimal: AuthenticatedUser = { + keycloakSub: 'f:realm:minimal', + }; + + mockFindByKeycloakSub.mockResolvedValue(null); + mockCreateFromToken.mockResolvedValue({ + ...mockUser, + name: 'Unknown User', + }); + + await service.ensureUserExists(authUserMinimal); + + expect(mockFindByKeycloakSub).toHaveBeenCalledWith('f:realm:minimal'); + expect(mockCreateFromToken).toHaveBeenCalledWith( + expect.objectContaining({ + name: 'Unknown User', + }), + ); + }); + }); + describe('hasRole', () => { it('should return true if user has the required role', () => { const result = service.hasRole(mockAuthUser, 'user'); diff --git a/src/auth/auth.service.ts b/src/auth/auth.service.ts index 6efcbfa..d1f9cb3 100644 --- a/src/auth/auth.service.ts +++ b/src/auth/auth.service.ts @@ -24,6 +24,11 @@ import { User } from '../users/users.entity'; * * For explicit profile sync (webhooks, admin operations), use: * - UsersService.syncProfileFromToken() - force sync regardless of changes + * + * ## Usage Guidelines + * + * - Use `syncUserFromToken()` for actual login events (WebSocket connections, /users/me) + * - Use `ensureUserExists()` for regular API calls that just need the user record */ @Injectable() export class AuthService { @@ -61,6 +66,49 @@ export class AuthService { return user; } + /** + * Ensures a user exists in the local database without updating profile data. + * This is optimized for regular API calls that need the user record but don't + * need to sync profile data from Keycloak on every request. + * + * - For new users: Creates with full profile data + * - For existing users: Returns existing record WITHOUT updating + * + * Use this for most API endpoints. Only use syncUserFromToken() for actual + * login events (WebSocket connections, /users/me endpoint). + * + * @param authenticatedUser - User data extracted from JWT token + * @returns The user entity + */ + async ensureUserExists(authenticatedUser: AuthenticatedUser): Promise { + const { keycloakSub, email, name, username, picture, roles } = + authenticatedUser; + + // Check if user exists + const existingUser = await this.usersService.findByKeycloakSub(keycloakSub); + + if (existingUser) { + // User exists - return without updating + return existingUser; + } + + // New user - create with full profile data + this.logger.log( + `Creating new user from token: ${keycloakSub} (via ensureUserExists)`, + ); + + const user = await this.usersService.createFromToken({ + keycloakSub, + email: email || '', + name: name || username || 'Unknown User', + username, + picture, + roles, + }); + + return user; + } + /** * Validates if a user has a specific role. * diff --git a/src/friends/friends.controller.spec.ts b/src/friends/friends.controller.spec.ts index 76d7d13..7d0cf87 100644 --- a/src/friends/friends.controller.spec.ts +++ b/src/friends/friends.controller.spec.ts @@ -82,6 +82,7 @@ describe('FriendsController', () => { const mockAuthService = { syncUserFromToken: jest.fn(), + ensureUserExists: jest.fn(), }; const mockStateGateway = { @@ -106,6 +107,7 @@ describe('FriendsController', () => { jest.clearAllMocks(); mockAuthService.syncUserFromToken.mockResolvedValue(mockUser1); + mockAuthService.ensureUserExists.mockResolvedValue(mockUser1); }); it('should be defined', () => { diff --git a/src/friends/friends.controller.ts b/src/friends/friends.controller.ts index 01f77dc..b8f5b1e 100644 --- a/src/friends/friends.controller.ts +++ b/src/friends/friends.controller.ts @@ -78,7 +78,7 @@ export class FriendsController { @Query() searchDto: SearchUsersDto, @CurrentUser() authUser: AuthenticatedUser, ): Promise { - const user = await this.authService.syncUserFromToken(authUser); + const user = await this.authService.ensureUserExists(authUser); this.logger.debug( `Searching users with username: ${searchDto.username || 'all'}`, @@ -126,7 +126,7 @@ export class FriendsController { @Body() sendRequestDto: SendFriendRequestDto, @CurrentUser() authUser: AuthenticatedUser, ): Promise { - const user = await this.authService.syncUserFromToken(authUser); + const user = await this.authService.ensureUserExists(authUser); this.logger.log( `User ${user.id} sending friend request to ${sendRequestDto.receiverId}`, @@ -161,7 +161,7 @@ export class FriendsController { async getReceivedRequests( @CurrentUser() authUser: AuthenticatedUser, ): Promise { - const user = await this.authService.syncUserFromToken(authUser); + const user = await this.authService.ensureUserExists(authUser); this.logger.debug(`Getting received friend requests for user ${user.id}`); @@ -188,7 +188,7 @@ export class FriendsController { async getSentRequests( @CurrentUser() authUser: AuthenticatedUser, ): Promise { - const user = await this.authService.syncUserFromToken(authUser); + const user = await this.authService.ensureUserExists(authUser); this.logger.debug(`Getting sent friend requests for user ${user.id}`); @@ -227,7 +227,7 @@ export class FriendsController { @Param('id') requestId: string, @CurrentUser() authUser: AuthenticatedUser, ): Promise { - const user = await this.authService.syncUserFromToken(authUser); + const user = await this.authService.ensureUserExists(authUser); this.logger.log(`User ${user.id} accepting friend request ${requestId}`); @@ -274,7 +274,7 @@ export class FriendsController { @Param('id') requestId: string, @CurrentUser() authUser: AuthenticatedUser, ): Promise { - const user = await this.authService.syncUserFromToken(authUser); + const user = await this.authService.ensureUserExists(authUser); this.logger.log(`User ${user.id} denying friend request ${requestId}`); @@ -307,7 +307,7 @@ export class FriendsController { async getFriends( @CurrentUser() authUser: AuthenticatedUser, ): Promise { - const user = await this.authService.syncUserFromToken(authUser); + const user = await this.authService.ensureUserExists(authUser); this.logger.debug(`Getting friends list for user ${user.id}`); @@ -355,7 +355,7 @@ export class FriendsController { @Param('friendId') friendId: string, @CurrentUser() authUser: AuthenticatedUser, ): Promise { - const user = await this.authService.syncUserFromToken(authUser); + const user = await this.authService.ensureUserExists(authUser); this.logger.log(`User ${user.id} unfriending user ${friendId}`); diff --git a/src/users/users.controller.spec.ts b/src/users/users.controller.spec.ts index 4aab09b..bc19d23 100644 --- a/src/users/users.controller.spec.ts +++ b/src/users/users.controller.spec.ts @@ -24,8 +24,11 @@ describe('UsersController', () => { findByKeycloakSub: mockFindByKeycloakSub, }; + const mockEnsureUserExists = jest.fn(); + const mockAuthService = { syncUserFromToken: mockSyncUserFromToken, + ensureUserExists: mockEnsureUserExists, }; const mockAuthUser: AuthenticatedUser = { @@ -90,7 +93,7 @@ describe('UsersController', () => { const updateDto: UpdateUserDto = { name: 'Updated Name' }; const updatedUser = { ...mockUser, name: 'Updated Name' }; - mockSyncUserFromToken.mockResolvedValue(mockUser); + mockEnsureUserExists.mockResolvedValue(mockUser); mockUpdate.mockResolvedValue(updatedUser); const result = await controller.updateCurrentUser( @@ -99,7 +102,7 @@ describe('UsersController', () => { ); expect(result).toBe(updatedUser); - expect(mockSyncUserFromToken).toHaveBeenCalledWith(mockAuthUser); + expect(mockEnsureUserExists).toHaveBeenCalledWith(mockAuthUser); expect(mockUpdate).toHaveBeenCalledWith( mockUser.id, updateDto, @@ -185,12 +188,12 @@ describe('UsersController', () => { describe('deleteCurrentUser', () => { it('should delete the current user account', async () => { - mockSyncUserFromToken.mockResolvedValue(mockUser); + mockEnsureUserExists.mockResolvedValue(mockUser); mockDelete.mockResolvedValue(undefined); await controller.deleteCurrentUser(mockAuthUser); - expect(mockSyncUserFromToken).toHaveBeenCalledWith(mockAuthUser); + expect(mockEnsureUserExists).toHaveBeenCalledWith(mockAuthUser); expect(mockDelete).toHaveBeenCalledWith( mockUser.id, mockAuthUser.keycloakSub, diff --git a/src/users/users.controller.ts b/src/users/users.controller.ts index 7a0ac28..24dd40e 100644 --- a/src/users/users.controller.ts +++ b/src/users/users.controller.ts @@ -51,7 +51,8 @@ export class UsersController { /** * Get current authenticated user's profile. - * This endpoint syncs the user from Keycloak token on each request. + * This endpoint syncs the user from Keycloak token to ensure profile data + * is up-to-date when explicitly requested by the user. */ @Get('me') @ApiOperation({ @@ -72,7 +73,8 @@ export class UsersController { ): Promise { this.logger.debug(`Get current user: ${authUser.keycloakSub}`); - // Sync user from token (creates if doesn't exist, updates if exists) + // Sync user from token - this is one of the few endpoints that should + // actively sync profile data, as it's an explicit request for user info const user = await this.authService.syncUserFromToken(authUser); return user; @@ -106,7 +108,7 @@ export class UsersController { this.logger.log(`Update current user: ${authUser.keycloakSub}`); // First ensure user exists in our system - const user = await this.authService.syncUserFromToken(authUser); + const user = await this.authService.ensureUserExists(authUser); // Update the user's profile return this.usersService.update( @@ -221,7 +223,7 @@ export class UsersController { this.logger.log(`Delete current user: ${authUser.keycloakSub}`); // First ensure user exists in our system - const user = await this.authService.syncUserFromToken(authUser); + const user = await this.authService.ensureUserExists(authUser); // Delete the user's account await this.usersService.delete(user.id, authUser.keycloakSub);