From e3b56781e1cf4755a210fa9b95d957e2381950aa Mon Sep 17 00:00:00 2001 From: Wind-Explorer Date: Thu, 18 Dec 2025 14:24:00 +0800 Subject: [PATCH] efficiency & performance fine tuning --- package.json | 2 + pnpm-lock.yaml | 36 ++++++ prisma/schema.prisma | 1 + src/app.module.ts | 15 ++- src/auth/auth.service.spec.ts | 34 ++---- src/auth/auth.service.ts | 22 +--- src/friends/events/friend.events.ts | 33 ++++++ src/friends/friends.controller.spec.ts | 43 +++---- src/friends/friends.controller.ts | 22 +--- src/friends/friends.service.spec.ts | 41 +++++++ src/friends/friends.service.ts | 152 ++++++++++++++++--------- src/users/users.service.ts | 52 +++++++++ src/ws/state/state.gateway.spec.ts | 27 +++-- src/ws/state/state.gateway.ts | 136 +++++++++++----------- 14 files changed, 392 insertions(+), 224 deletions(-) create mode 100644 src/friends/events/friend.events.ts diff --git a/package.json b/package.json index 4699fe7..9b184ab 100644 --- a/package.json +++ b/package.json @@ -30,10 +30,12 @@ "@nestjs/common": "^11.0.1", "@nestjs/config": "^4.0.2", "@nestjs/core": "^11.0.1", + "@nestjs/event-emitter": "^3.0.1", "@nestjs/passport": "^11.0.5", "@nestjs/platform-express": "^11.0.1", "@nestjs/platform-socket.io": "^11.1.9", "@nestjs/swagger": "^11.2.3", + "@nestjs/throttler": "^6.5.0", "@nestjs/websockets": "^11.1.9", "@prisma/adapter-pg": "^7.0.0", "@prisma/client": "^7.0.0", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 2612bc6..647a182 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -17,6 +17,9 @@ importers: '@nestjs/core': specifier: ^11.0.1 version: 11.1.9(@nestjs/common@11.1.9(class-transformer@0.5.1)(class-validator@0.14.2)(reflect-metadata@0.2.2)(rxjs@7.8.2))(@nestjs/platform-express@11.1.9)(@nestjs/websockets@11.1.9)(reflect-metadata@0.2.2)(rxjs@7.8.2) + '@nestjs/event-emitter': + specifier: ^3.0.1 + version: 3.0.1(@nestjs/common@11.1.9(class-transformer@0.5.1)(class-validator@0.14.2)(reflect-metadata@0.2.2)(rxjs@7.8.2))(@nestjs/core@11.1.9(@nestjs/common@11.1.9(class-transformer@0.5.1)(class-validator@0.14.2)(reflect-metadata@0.2.2)(rxjs@7.8.2))(@nestjs/platform-express@11.1.9)(@nestjs/websockets@11.1.9)(reflect-metadata@0.2.2)(rxjs@7.8.2)) '@nestjs/passport': specifier: ^11.0.5 version: 11.0.5(@nestjs/common@11.1.9(class-transformer@0.5.1)(class-validator@0.14.2)(reflect-metadata@0.2.2)(rxjs@7.8.2))(passport@0.7.0) @@ -29,6 +32,9 @@ importers: '@nestjs/swagger': specifier: ^11.2.3 version: 11.2.3(@nestjs/common@11.1.9(class-transformer@0.5.1)(class-validator@0.14.2)(reflect-metadata@0.2.2)(rxjs@7.8.2))(@nestjs/core@11.1.9(@nestjs/common@11.1.9(class-transformer@0.5.1)(class-validator@0.14.2)(reflect-metadata@0.2.2)(rxjs@7.8.2))(@nestjs/platform-express@11.1.9)(@nestjs/websockets@11.1.9)(reflect-metadata@0.2.2)(rxjs@7.8.2))(class-transformer@0.5.1)(class-validator@0.14.2)(reflect-metadata@0.2.2) + '@nestjs/throttler': + specifier: ^6.5.0 + version: 6.5.0(@nestjs/common@11.1.9(class-transformer@0.5.1)(class-validator@0.14.2)(reflect-metadata@0.2.2)(rxjs@7.8.2))(@nestjs/core@11.1.9(@nestjs/common@11.1.9(class-transformer@0.5.1)(class-validator@0.14.2)(reflect-metadata@0.2.2)(rxjs@7.8.2))(@nestjs/platform-express@11.1.9)(@nestjs/websockets@11.1.9)(reflect-metadata@0.2.2)(rxjs@7.8.2))(reflect-metadata@0.2.2) '@nestjs/websockets': specifier: ^11.1.9 version: 11.1.9(@nestjs/common@11.1.9(class-transformer@0.5.1)(class-validator@0.14.2)(reflect-metadata@0.2.2)(rxjs@7.8.2))(@nestjs/core@11.1.9)(@nestjs/platform-socket.io@11.1.9)(reflect-metadata@0.2.2)(rxjs@7.8.2) @@ -789,6 +795,12 @@ packages: '@nestjs/websockets': optional: true + '@nestjs/event-emitter@3.0.1': + resolution: {integrity: sha512-0Ln/x+7xkU6AJFOcQI9tIhUMXVF7D5itiaQGOyJbXtlAfAIt8gzDdJm+Im7cFzKoWkiW5nCXCPh6GSvdQd/3Dw==} + peerDependencies: + '@nestjs/common': ^10.0.0 || ^11.0.0 + '@nestjs/core': ^10.0.0 || ^11.0.0 + '@nestjs/mapped-types@2.1.0': resolution: {integrity: sha512-W+n+rM69XsFdwORF11UqJahn4J3xi4g/ZEOlJNL6KoW5ygWSmBB2p0S2BZ4FQeS/NDH72e6xIcu35SfJnE8bXw==} peerDependencies: @@ -856,6 +868,13 @@ packages: '@nestjs/platform-express': optional: true + '@nestjs/throttler@6.5.0': + resolution: {integrity: sha512-9j0ZRfH0QE1qyrj9JjIRDz5gQLPqq9yVC2nHsrosDVAfI5HHw08/aUAWx9DZLSdQf4HDkmhTTEGLrRFHENvchQ==} + peerDependencies: + '@nestjs/common': ^7.0.0 || ^8.0.0 || ^9.0.0 || ^10.0.0 || ^11.0.0 + '@nestjs/core': ^7.0.0 || ^8.0.0 || ^9.0.0 || ^10.0.0 || ^11.0.0 + reflect-metadata: ^0.1.13 || ^0.2.0 + '@nestjs/websockets@11.1.9': resolution: {integrity: sha512-kkkdeTVcc3X7ZzvVqUVpOAJoh49kTRUjWNUXo5jmG+27OvZoHfs/vuSiqxidrrbIgydSqN15HUsf1wZwQUrxCQ==} peerDependencies: @@ -1975,6 +1994,9 @@ packages: resolution: {integrity: sha512-aIL5Fx7mawVa300al2BnEE4iNvo1qETxLrPI/o05L7z6go7fCw1J6EQmbK4FmJ2AS7kgVF/KEZWufBfdClMcPg==} engines: {node: '>= 0.6'} + eventemitter2@6.4.9: + resolution: {integrity: sha512-JEPTiaOt9f04oa6NOkc4aH+nVp5I3wEjpHbIPqfgCdD5v5bUzy7xQqwcVO2aDQgOWhI28da57HksMrzK9HlRxg==} + events@3.3.0: resolution: {integrity: sha512-mQw+2fkQbALzQ7V0MY0IqdnXNOeTtP4r0lN9z7AAawCXgqea7bDii20AYrIBrFd/Hx0M2Ocz6S111CaFkUcb0Q==} engines: {node: '>=0.8.x'} @@ -4490,6 +4512,12 @@ snapshots: '@nestjs/platform-express': 11.1.9(@nestjs/common@11.1.9(class-transformer@0.5.1)(class-validator@0.14.2)(reflect-metadata@0.2.2)(rxjs@7.8.2))(@nestjs/core@11.1.9) '@nestjs/websockets': 11.1.9(@nestjs/common@11.1.9(class-transformer@0.5.1)(class-validator@0.14.2)(reflect-metadata@0.2.2)(rxjs@7.8.2))(@nestjs/core@11.1.9)(@nestjs/platform-socket.io@11.1.9)(reflect-metadata@0.2.2)(rxjs@7.8.2) + '@nestjs/event-emitter@3.0.1(@nestjs/common@11.1.9(class-transformer@0.5.1)(class-validator@0.14.2)(reflect-metadata@0.2.2)(rxjs@7.8.2))(@nestjs/core@11.1.9(@nestjs/common@11.1.9(class-transformer@0.5.1)(class-validator@0.14.2)(reflect-metadata@0.2.2)(rxjs@7.8.2))(@nestjs/platform-express@11.1.9)(@nestjs/websockets@11.1.9)(reflect-metadata@0.2.2)(rxjs@7.8.2))': + dependencies: + '@nestjs/common': 11.1.9(class-transformer@0.5.1)(class-validator@0.14.2)(reflect-metadata@0.2.2)(rxjs@7.8.2) + '@nestjs/core': 11.1.9(@nestjs/common@11.1.9(class-transformer@0.5.1)(class-validator@0.14.2)(reflect-metadata@0.2.2)(rxjs@7.8.2))(@nestjs/platform-express@11.1.9)(@nestjs/websockets@11.1.9)(reflect-metadata@0.2.2)(rxjs@7.8.2) + eventemitter2: 6.4.9 + '@nestjs/mapped-types@2.1.0(@nestjs/common@11.1.9(class-transformer@0.5.1)(class-validator@0.14.2)(reflect-metadata@0.2.2)(rxjs@7.8.2))(class-transformer@0.5.1)(class-validator@0.14.2)(reflect-metadata@0.2.2)': dependencies: '@nestjs/common': 11.1.9(class-transformer@0.5.1)(class-validator@0.14.2)(reflect-metadata@0.2.2)(rxjs@7.8.2) @@ -4561,6 +4589,12 @@ snapshots: optionalDependencies: '@nestjs/platform-express': 11.1.9(@nestjs/common@11.1.9(class-transformer@0.5.1)(class-validator@0.14.2)(reflect-metadata@0.2.2)(rxjs@7.8.2))(@nestjs/core@11.1.9) + '@nestjs/throttler@6.5.0(@nestjs/common@11.1.9(class-transformer@0.5.1)(class-validator@0.14.2)(reflect-metadata@0.2.2)(rxjs@7.8.2))(@nestjs/core@11.1.9(@nestjs/common@11.1.9(class-transformer@0.5.1)(class-validator@0.14.2)(reflect-metadata@0.2.2)(rxjs@7.8.2))(@nestjs/platform-express@11.1.9)(@nestjs/websockets@11.1.9)(reflect-metadata@0.2.2)(rxjs@7.8.2))(reflect-metadata@0.2.2)': + dependencies: + '@nestjs/common': 11.1.9(class-transformer@0.5.1)(class-validator@0.14.2)(reflect-metadata@0.2.2)(rxjs@7.8.2) + '@nestjs/core': 11.1.9(@nestjs/common@11.1.9(class-transformer@0.5.1)(class-validator@0.14.2)(reflect-metadata@0.2.2)(rxjs@7.8.2))(@nestjs/platform-express@11.1.9)(@nestjs/websockets@11.1.9)(reflect-metadata@0.2.2)(rxjs@7.8.2) + reflect-metadata: 0.2.2 + '@nestjs/websockets@11.1.9(@nestjs/common@11.1.9(class-transformer@0.5.1)(class-validator@0.14.2)(reflect-metadata@0.2.2)(rxjs@7.8.2))(@nestjs/core@11.1.9)(@nestjs/platform-socket.io@11.1.9)(reflect-metadata@0.2.2)(rxjs@7.8.2)': dependencies: '@nestjs/common': 11.1.9(class-transformer@0.5.1)(class-validator@0.14.2)(reflect-metadata@0.2.2)(rxjs@7.8.2) @@ -5753,6 +5787,8 @@ snapshots: etag@1.8.1: {} + eventemitter2@6.4.9: {} + events@3.3.0: {} execa@5.1.1: diff --git a/prisma/schema.prisma b/prisma/schema.prisma index 83440e2..802ca45 100644 --- a/prisma/schema.prisma +++ b/prisma/schema.prisma @@ -3,6 +3,7 @@ generator client { provider = "prisma-client-js" + previewFeatures = ["driverAdapters"] } datasource db { diff --git a/src/app.module.ts b/src/app.module.ts index c99288b..7178ee4 100644 --- a/src/app.module.ts +++ b/src/app.module.ts @@ -1,5 +1,7 @@ import { Module } from '@nestjs/common'; -import { ConfigModule } from '@nestjs/config'; +import { ConfigModule, ConfigService } from '@nestjs/config'; +import { EventEmitterModule } from '@nestjs/event-emitter'; +import { ThrottlerModule } from '@nestjs/throttler'; import { AppController } from './app.controller'; import { AppService } from './app.service'; import { UsersModule } from './users/users.module'; @@ -49,6 +51,17 @@ function validateEnvironment(config: Record): Record { envFilePath: '.env', validate: validateEnvironment, }), + ThrottlerModule.forRootAsync({ + imports: [ConfigModule], + inject: [ConfigService], + useFactory: (config: ConfigService) => [ + { + ttl: config.get('THROTTLE_TTL', 60000), + limit: config.get('THROTTLE_LIMIT', 10), + }, + ], + }), + EventEmitterModule.forRoot(), DatabaseModule, UsersModule, AuthModule, diff --git a/src/auth/auth.service.spec.ts b/src/auth/auth.service.spec.ts index 41aaaf4..dd7278e 100644 --- a/src/auth/auth.service.spec.ts +++ b/src/auth/auth.service.spec.ts @@ -9,10 +9,12 @@ describe('AuthService', () => { const mockCreateFromToken = jest.fn(); const mockFindByKeycloakSub = jest.fn(); + const mockFindOrCreate = jest.fn(); const mockUsersService = { createFromToken: mockCreateFromToken, findByKeycloakSub: mockFindByKeycloakSub, + findOrCreate: mockFindOrCreate, }; const mockAuthUser: AuthenticatedUser = { @@ -205,25 +207,13 @@ describe('AuthService', () => { }); describe('ensureUserExists', () => { - it('should return existing user without updating', async () => { - mockFindByKeycloakSub.mockResolvedValue(mockUser); + it('should call findOrCreate with correct params', async () => { + mockFindOrCreate.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({ + expect(mockFindOrCreate).toHaveBeenCalledWith({ keycloakSub: 'f:realm:user123', email: 'test@example.com', name: 'Test User', @@ -233,14 +223,13 @@ describe('AuthService', () => { }); }); - it('should handle user with no email when creating new user', async () => { + it('should handle user with no email', async () => { const authUserNoEmail: AuthenticatedUser = { keycloakSub: 'f:realm:user456', name: 'No Email User', }; - mockFindByKeycloakSub.mockResolvedValue(null); - mockCreateFromToken.mockResolvedValue({ + mockFindOrCreate.mockResolvedValue({ ...mockUser, email: '', name: 'No Email User', @@ -248,8 +237,7 @@ describe('AuthService', () => { await service.ensureUserExists(authUserNoEmail); - expect(mockFindByKeycloakSub).toHaveBeenCalledWith('f:realm:user456'); - expect(mockCreateFromToken).toHaveBeenCalledWith( + expect(mockFindOrCreate).toHaveBeenCalledWith( expect.objectContaining({ email: '', name: 'No Email User', @@ -262,16 +250,14 @@ describe('AuthService', () => { keycloakSub: 'f:realm:minimal', }; - mockFindByKeycloakSub.mockResolvedValue(null); - mockCreateFromToken.mockResolvedValue({ + mockFindOrCreate.mockResolvedValue({ ...mockUser, name: 'Unknown User', }); await service.ensureUserExists(authUserMinimal); - expect(mockFindByKeycloakSub).toHaveBeenCalledWith('f:realm:minimal'); - expect(mockCreateFromToken).toHaveBeenCalledWith( + expect(mockFindOrCreate).toHaveBeenCalledWith( expect.objectContaining({ name: 'Unknown User', }), diff --git a/src/auth/auth.service.ts b/src/auth/auth.service.ts index d1f9cb3..6842884 100644 --- a/src/auth/auth.service.ts +++ b/src/auth/auth.service.ts @@ -72,7 +72,7 @@ export class AuthService { * 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 + * - For existing users: Returns existing record WITHOUT updating (read-only) * * Use this for most API endpoints. Only use syncUserFromToken() for actual * login events (WebSocket connections, /users/me endpoint). @@ -84,20 +84,10 @@ export class AuthService { 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({ + // Use optimized findOrCreate method + // This returns existing users immediately (1 read) + // And creates new users if needed (1 read + 1 write) + return await this.usersService.findOrCreate({ keycloakSub, email: email || '', name: name || username || 'Unknown User', @@ -105,8 +95,6 @@ export class AuthService { picture, roles, }); - - return user; } /** diff --git a/src/friends/events/friend.events.ts b/src/friends/events/friend.events.ts new file mode 100644 index 0000000..2d65f97 --- /dev/null +++ b/src/friends/events/friend.events.ts @@ -0,0 +1,33 @@ +import { FriendRequest, User } from '@prisma/client'; + +export enum FriendEvents { + REQUEST_RECEIVED = 'friend.request.received', + REQUEST_ACCEPTED = 'friend.request.accepted', + REQUEST_DENIED = 'friend.request.denied', + UNFRIENDED = 'friend.unfriended', +} + +export type FriendRequestWithRelations = FriendRequest & { + sender: User; + receiver: User; +}; + +export interface FriendRequestReceivedEvent { + userId: string; + friendRequest: FriendRequestWithRelations; +} + +export interface FriendRequestAcceptedEvent { + userId: string; + friendRequest: FriendRequestWithRelations; +} + +export interface FriendRequestDeniedEvent { + userId: string; + friendRequest: FriendRequestWithRelations; +} + +export interface UnfriendedEvent { + userId: string; + friendId: string; +} diff --git a/src/friends/friends.controller.spec.ts b/src/friends/friends.controller.spec.ts index 7d0cf87..704c28e 100644 --- a/src/friends/friends.controller.spec.ts +++ b/src/friends/friends.controller.spec.ts @@ -1,9 +1,10 @@ import { Test, TestingModule } from '@nestjs/testing'; +import { ThrottlerModule } from '@nestjs/throttler'; import { FriendsController } from './friends.controller'; import { FriendsService } from './friends.service'; import { UsersService } from '../users/users.service'; import { AuthService } from '../auth/auth.service'; -import { StateGateway } from '../ws/state/state.gateway'; +// StateGateway removed enum FriendRequestStatus { PENDING = 'PENDING', @@ -85,21 +86,21 @@ describe('FriendsController', () => { ensureUserExists: jest.fn(), }; - const mockStateGateway = { - emitFriendRequestReceived: jest.fn(), - emitFriendRequestAccepted: jest.fn(), - emitFriendRequestDenied: jest.fn(), - emitUnfriended: jest.fn(), - }; - beforeEach(async () => { const module: TestingModule = await Test.createTestingModule({ + imports: [ + ThrottlerModule.forRoot([ + { + ttl: 60000, + limit: 10, + }, + ]), + ], controllers: [FriendsController], providers: [ { provide: FriendsService, useValue: mockFriendsService }, { provide: UsersService, useValue: mockUsersService }, { provide: AuthService, useValue: mockAuthService }, - { provide: StateGateway, useValue: mockStateGateway }, ], }).compile(); @@ -140,7 +141,7 @@ describe('FriendsController', () => { }); describe('sendFriendRequest', () => { - it('should send friend request and emit WebSocket event', async () => { + it('should send friend request', async () => { mockFriendsService.sendFriendRequest.mockResolvedValue(mockFriendRequest); const result = await controller.sendFriendRequest( @@ -170,10 +171,6 @@ describe('FriendsController', () => { 'user-1', 'user-2', ); - expect(mockStateGateway.emitFriendRequestReceived).toHaveBeenCalledWith( - 'user-2', - mockFriendRequest, - ); }); }); @@ -210,7 +207,7 @@ describe('FriendsController', () => { }); describe('acceptFriendRequest', () => { - it('should accept friend request and emit WebSocket event', async () => { + it('should accept friend request', async () => { const acceptedRequest = { ...mockFriendRequest, status: FriendRequestStatus.ACCEPTED, @@ -227,15 +224,11 @@ describe('FriendsController', () => { 'request-1', 'user-1', ); - expect(mockStateGateway.emitFriendRequestAccepted).toHaveBeenCalledWith( - 'user-1', - acceptedRequest, - ); }); }); describe('denyFriendRequest', () => { - it('should deny friend request and emit WebSocket event', async () => { + it('should deny friend request', async () => { const deniedRequest = { ...mockFriendRequest, status: FriendRequestStatus.DENIED, @@ -252,10 +245,6 @@ describe('FriendsController', () => { 'request-1', 'user-1', ); - expect(mockStateGateway.emitFriendRequestDenied).toHaveBeenCalledWith( - 'user-1', - deniedRequest, - ); }); }); @@ -282,7 +271,7 @@ describe('FriendsController', () => { }); describe('unfriend', () => { - it('should unfriend user and emit WebSocket event', async () => { + it('should unfriend user', async () => { mockFriendsService.unfriend.mockResolvedValue(undefined); await controller.unfriend('user-2', mockAuthUser); @@ -291,10 +280,6 @@ describe('FriendsController', () => { 'user-1', 'user-2', ); - expect(mockStateGateway.emitUnfriended).toHaveBeenCalledWith( - 'user-2', - 'user-1', - ); }); }); }); diff --git a/src/friends/friends.controller.ts b/src/friends/friends.controller.ts index b8f5b1e..03002fc 100644 --- a/src/friends/friends.controller.ts +++ b/src/friends/friends.controller.ts @@ -19,6 +19,7 @@ import { ApiUnauthorizedResponse, ApiQuery, } from '@nestjs/swagger'; +import { ThrottlerGuard, Throttle } from '@nestjs/throttler'; import { User, FriendRequest } from '@prisma/client'; import { FriendsService } from './friends.service'; import { JwtAuthGuard } from '../auth/guards/jwt-auth.guard'; @@ -40,7 +41,6 @@ type FriendRequestWithRelations = FriendRequest & { receiver: User; }; import { UsersService } from '../users/users.service'; -import { StateGateway } from '../ws/state/state.gateway'; @ApiTags('friends') @Controller('friends') @@ -53,10 +53,11 @@ export class FriendsController { private readonly friendsService: FriendsService, private readonly usersService: UsersService, private readonly authService: AuthService, - private readonly stateGateway: StateGateway, ) {} @Get('search') + @UseGuards(ThrottlerGuard) + @Throttle({ default: { limit: 10, ttl: 60000 } }) @ApiOperation({ summary: 'Search users by username', description: 'Search for users by username to send friend requests', @@ -137,11 +138,6 @@ export class FriendsController { sendRequestDto.receiverId, ); - this.stateGateway.emitFriendRequestReceived( - sendRequestDto.receiverId, - friendRequest, - ); - return this.mapFriendRequestToDto(friendRequest); } @@ -236,11 +232,6 @@ export class FriendsController { user.id, ); - this.stateGateway.emitFriendRequestAccepted( - friendRequest.senderId, - friendRequest, - ); - return this.mapFriendRequestToDto(friendRequest); } @@ -283,11 +274,6 @@ export class FriendsController { user.id, ); - this.stateGateway.emitFriendRequestDenied( - friendRequest.senderId, - friendRequest, - ); - return this.mapFriendRequestToDto(friendRequest); } @@ -360,8 +346,6 @@ export class FriendsController { this.logger.log(`User ${user.id} unfriending user ${friendId}`); await this.friendsService.unfriend(user.id, friendId); - - this.stateGateway.emitUnfriended(friendId, user.id); } private mapFriendRequestToDto( diff --git a/src/friends/friends.service.spec.ts b/src/friends/friends.service.spec.ts index 87702f8..4ba6e74 100644 --- a/src/friends/friends.service.spec.ts +++ b/src/friends/friends.service.spec.ts @@ -1,4 +1,5 @@ import { Test, TestingModule } from '@nestjs/testing'; +import { EventEmitter2 } from '@nestjs/event-emitter'; import { FriendsService } from './friends.service'; import { PrismaService } from '../database/prisma.service'; import { @@ -15,6 +16,7 @@ enum FriendRequestStatus { describe('FriendsService', () => { let service: FriendsService; + let eventEmitter: EventEmitter2; const mockUser1 = { id: 'user-1', @@ -82,6 +84,10 @@ describe('FriendsService', () => { $transaction: jest.fn(), }; + const mockEventEmitter = { + emit: jest.fn(), + }; + beforeEach(async () => { const module: TestingModule = await Test.createTestingModule({ providers: [ @@ -90,10 +96,15 @@ describe('FriendsService', () => { provide: PrismaService, useValue: mockPrismaService, }, + { + provide: EventEmitter2, + useValue: mockEventEmitter, + }, ], }).compile(); service = module.get(FriendsService); + eventEmitter = module.get(EventEmitter2); jest.clearAllMocks(); }); @@ -110,6 +121,12 @@ describe('FriendsService', () => { mockPrismaService.friendRequest.create.mockResolvedValue( mockFriendRequest, ); + // Mock transaction implementation + mockPrismaService.$transaction.mockImplementation( + async (callback: (prisma: any) => Promise) => { + return (await callback(mockPrismaService)) as unknown; + }, + ); const result = await service.sendFriendRequest('user-1', 'user-2'); @@ -128,6 +145,7 @@ describe('FriendsService', () => { receiver: true, }, }); + expect(mockEventEmitter.emit).toHaveBeenCalled(); }); it('should throw BadRequestException when trying to send request to self', async () => { @@ -141,6 +159,11 @@ describe('FriendsService', () => { it('should throw NotFoundException when receiver does not exist', async () => { mockPrismaService.user.findUnique.mockResolvedValue(null); + mockPrismaService.$transaction.mockImplementation( + async (callback: (prisma: any) => Promise) => { + return (await callback(mockPrismaService)) as unknown; + }, + ); await expect( service.sendFriendRequest('user-1', 'nonexistent'), @@ -153,6 +176,11 @@ describe('FriendsService', () => { it('should throw ConflictException when users are already friends', async () => { mockPrismaService.user.findUnique.mockResolvedValue(mockUser2); mockPrismaService.friendship.findFirst.mockResolvedValue(mockFriendship); + mockPrismaService.$transaction.mockImplementation( + async (callback: (prisma: any) => Promise) => { + return (await callback(mockPrismaService)) as unknown; + }, + ); await expect( service.sendFriendRequest('user-1', 'user-2'), @@ -168,6 +196,11 @@ describe('FriendsService', () => { mockPrismaService.friendRequest.findFirst.mockResolvedValue( mockFriendRequest, ); + mockPrismaService.$transaction.mockImplementation( + async (callback: (prisma: any) => Promise) => { + return (await callback(mockPrismaService)) as unknown; + }, + ); await expect( service.sendFriendRequest('user-1', 'user-2'), @@ -185,6 +218,11 @@ describe('FriendsService', () => { senderId: 'user-2', receiverId: 'user-1', }); + mockPrismaService.$transaction.mockImplementation( + async (callback: (prisma: any) => Promise) => { + return (await callback(mockPrismaService)) as unknown; + }, + ); await expect( service.sendFriendRequest('user-1', 'user-2'), @@ -259,6 +297,7 @@ describe('FriendsService', () => { expect(result).toEqual(acceptedRequest); expect(mockPrismaService.$transaction).toHaveBeenCalled(); + expect(mockEventEmitter.emit).toHaveBeenCalled(); }); it('should throw NotFoundException when request does not exist', async () => { @@ -320,6 +359,7 @@ describe('FriendsService', () => { expect(mockPrismaService.friendRequest.delete).toHaveBeenCalledWith({ where: { id: 'request-1' }, }); + expect(mockEventEmitter.emit).toHaveBeenCalled(); }); it('should throw NotFoundException when request does not exist', async () => { @@ -392,6 +432,7 @@ describe('FriendsService', () => { ], }, }); + expect(mockEventEmitter.emit).toHaveBeenCalled(); }); it('should throw BadRequestException when trying to unfriend self', async () => { diff --git a/src/friends/friends.service.ts b/src/friends/friends.service.ts index b4b7e8f..73c5e7f 100644 --- a/src/friends/friends.service.ts +++ b/src/friends/friends.service.ts @@ -5,8 +5,16 @@ import { Logger, ConflictException, } from '@nestjs/common'; +import { EventEmitter2 } from '@nestjs/event-emitter'; import { PrismaService } from '../database/prisma.service'; import { User, FriendRequest, FriendRequestStatus } from '@prisma/client'; +import { + FriendEvents, + FriendRequestReceivedEvent, + FriendRequestAcceptedEvent, + FriendRequestDeniedEvent, + UnfriendedEvent, +} from './events/friend.events'; export type FriendRequestWithRelations = FriendRequest & { sender: User; @@ -17,7 +25,10 @@ export type FriendRequestWithRelations = FriendRequest & { export class FriendsService { private readonly logger = new Logger(FriendsService.name); - constructor(private readonly prisma: PrismaService) {} + constructor( + private readonly prisma: PrismaService, + private readonly eventEmitter: EventEmitter2, + ) {} async sendFriendRequest( senderId: string, @@ -27,66 +38,82 @@ export class FriendsService { throw new BadRequestException('Cannot send friend request to yourself'); } - const receiver = await this.prisma.user.findUnique({ - where: { id: receiverId }, - }); + const friendRequest = await this.prisma.$transaction(async (tx) => { + const receiver = await tx.user.findUnique({ + where: { id: receiverId }, + }); - if (!receiver) { - throw new NotFoundException('User not found'); - } - - const existingFriendship = await this.areFriends(senderId, receiverId); - if (existingFriendship) { - throw new ConflictException('You are already friends with this user'); - } - - const existingRequest = await this.prisma.friendRequest.findFirst({ - where: { - OR: [ - { senderId, receiverId }, - { - senderId: receiverId, - receiverId: senderId, - }, - ], - }, - }); - - if (existingRequest) { - if (existingRequest.status === FriendRequestStatus.PENDING) { - if (existingRequest.senderId === senderId) { - throw new ConflictException( - 'You already sent a friend request to this user', - ); - } else { - throw new ConflictException( - 'This user already sent you a friend request', - ); - } - } else { - // If there's an existing request that is not pending (accepted or denied), delete it so a new one can be created - await this.prisma.friendRequest.delete({ - where: { id: existingRequest.id }, - }); + if (!receiver) { + throw new NotFoundException('User not found'); } - } - const friendRequest = await this.prisma.friendRequest.create({ - data: { - senderId, - receiverId, - status: FriendRequestStatus.PENDING, - }, - include: { - sender: true, - receiver: true, - }, + // Check for existing friendship using the transaction client + const existingFriendship = await tx.friendship.findFirst({ + where: { + userId: senderId, + friendId: receiverId, + }, + }); + + if (existingFriendship) { + throw new ConflictException('You are already friends with this user'); + } + + const existingRequest = await tx.friendRequest.findFirst({ + where: { + OR: [ + { senderId, receiverId }, + { + senderId: receiverId, + receiverId: senderId, + }, + ], + }, + }); + + if (existingRequest) { + if (existingRequest.status === FriendRequestStatus.PENDING) { + if (existingRequest.senderId === senderId) { + throw new ConflictException( + 'You already sent a friend request to this user', + ); + } else { + throw new ConflictException( + 'This user already sent you a friend request', + ); + } + } else { + // If there's an existing request that is not pending (accepted or denied), delete it so a new one can be created + await tx.friendRequest.delete({ + where: { id: existingRequest.id }, + }); + } + } + + return await tx.friendRequest.create({ + data: { + senderId, + receiverId, + status: FriendRequestStatus.PENDING, + }, + include: { + sender: true, + receiver: true, + }, + }); }); this.logger.log( `Friend request sent from ${senderId} to ${receiverId} (ID: ${friendRequest.id})`, ); + // Emit event + const event: FriendRequestReceivedEvent = { + userId: receiverId, + friendRequest, + }; + this.eventEmitter.emit(FriendEvents.REQUEST_RECEIVED, event); + return friendRequest; } @@ -183,6 +210,13 @@ export class FriendsService { `Friend request ${requestId} accepted. Users ${friendRequest.senderId} and ${friendRequest.receiverId} are now friends`, ); + // Emit event + const event: FriendRequestAcceptedEvent = { + userId: friendRequest.senderId, + friendRequest: result, + }; + this.eventEmitter.emit(FriendEvents.REQUEST_ACCEPTED, event); + return result; } @@ -227,6 +261,13 @@ export class FriendsService { this.logger.log(`Friend request ${requestId} denied by user ${userId}`); + // Emit event + const event: FriendRequestDeniedEvent = { + userId: friendRequest.senderId, + friendRequest: result, + }; + this.eventEmitter.emit(FriendEvents.REQUEST_DENIED, event); + return result; } @@ -268,6 +309,13 @@ export class FriendsService { }); this.logger.log(`User ${userId} unfriended user ${friendId}`); + + // Emit event + const event: UnfriendedEvent = { + userId: friendId, + friendId: userId, + }; + this.eventEmitter.emit(FriendEvents.UNFRIENDED, event); } async areFriends(userId: string, friendId: string): Promise { diff --git a/src/users/users.service.ts b/src/users/users.service.ts index 59af3b4..bb19281 100644 --- a/src/users/users.service.ts +++ b/src/users/users.service.ts @@ -58,6 +58,58 @@ export class UsersService { constructor(private readonly prisma: PrismaService) {} + /** + * Finds a user or creates one if they don't exist. + * Optimized to minimize database operations: + * - If user exists: Returns immediately (1 read, 0 writes) + * - If user missing: Creates user (1 read, 1 write) + * + * Unlike createFromToken, this method does NOT update lastLoginAt for existing users. + * + * @param createDto - User data + * @returns The user entity + */ + async findOrCreate(createDto: CreateUserFromTokenDto): Promise { + // 1. Try to find the user (Read) + const existingUser = await this.prisma.user.findUnique({ + where: { keycloakSub: createDto.keycloakSub }, + }); + + // 2. If found, return immediately without update + if (existingUser) { + return existingUser; + } + + // 3. If not found, create (Write) + // We handle creation directly here to avoid a second read that createFromToken would do + const roles = createDto.roles || []; + const now = new Date(); + + this.logger.log(`Creating new user from token: ${createDto.keycloakSub}`); + + // Use upsert to handle race conditions safely + return await this.prisma.user.upsert({ + where: { keycloakSub: createDto.keycloakSub }, + update: { + email: createDto.email, + name: createDto.name, + username: createDto.username, + picture: createDto.picture, + roles, + lastLoginAt: now, + }, + create: { + keycloakSub: createDto.keycloakSub, + email: createDto.email, + name: createDto.name, + username: createDto.username, + picture: createDto.picture, + roles, + lastLoginAt: now, + }, + }); + } + /** * Creates a new user or syncs/tracks login for existing users. * This method is called automatically during authentication flow. diff --git a/src/ws/state/state.gateway.spec.ts b/src/ws/state/state.gateway.spec.ts index eba3b2b..3ec713e 100644 --- a/src/ws/state/state.gateway.spec.ts +++ b/src/ws/state/state.gateway.spec.ts @@ -1,10 +1,10 @@ +import { CursorPositionDto } from '../dto/cursor-position.dto'; import { Test, TestingModule } from '@nestjs/testing'; import { StateGateway } from './state.gateway'; import { AuthenticatedSocket } from '../../types/socket'; import { AuthService } from '../../auth/auth.service'; import { JwtVerificationService } from '../../auth/services/jwt-verification.service'; - -import { FriendsService } from '../../friends/friends.service'; +import { PrismaService } from '../../database/prisma.service'; interface MockSocket extends Partial { id: string; @@ -25,24 +25,25 @@ describe('StateGateway', () => { let mockLoggerDebug: jest.SpyInstance; let mockLoggerWarn: jest.SpyInstance; let mockServer: { - sockets: { sockets: { size: number } }; + sockets: { sockets: { size: number; get: jest.Mock } }; to: jest.Mock; }; let mockAuthService: Partial; let mockJwtVerificationService: Partial; - let mockFriendsService: Partial; + let mockPrismaService: Partial; beforeEach(async () => { mockServer = { sockets: { sockets: { size: 5, + get: jest.fn(), }, }, to: jest.fn().mockReturnValue({ emit: jest.fn(), }), - } as any; + }; mockAuthService = { syncUserFromToken: jest.fn().mockResolvedValue({ @@ -59,8 +60,10 @@ describe('StateGateway', () => { }), }; - mockFriendsService = { - getFriends: jest.fn().mockResolvedValue([]), + mockPrismaService = { + friendship: { + findMany: jest.fn().mockResolvedValue([]), + }, }; const module: TestingModule = await Test.createTestingModule({ @@ -71,7 +74,7 @@ describe('StateGateway', () => { provide: JwtVerificationService, useValue: mockJwtVerificationService, }, - { provide: FriendsService, useValue: mockFriendsService }, + { provide: PrismaService, useValue: mockPrismaService }, ], }).compile(); @@ -204,7 +207,7 @@ describe('StateGateway', () => { // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access (gateway as any).userSocketMap.set('friend-1', 'friend-socket-id'); - const data = { x: 100, y: 200, isDrawing: false }; + const data: CursorPositionDto = { x: 100, y: 200 }; gateway.handleCursorReportPosition( mockClient as unknown as AuthenticatedSocket, @@ -232,7 +235,7 @@ describe('StateGateway', () => { }; // Don't set up userSocketMap - friend is not online - const data = { x: 100, y: 200, isDrawing: false }; + const data: CursorPositionDto = { x: 100, y: 200 }; gateway.handleCursorReportPosition( mockClient as unknown as AuthenticatedSocket, @@ -253,7 +256,7 @@ describe('StateGateway', () => { }, }; - const data = { x: 100, y: 200, isDrawing: false }; + const data: CursorPositionDto = { x: 100, y: 200 }; gateway.handleCursorReportPosition( mockClient as unknown as AuthenticatedSocket, @@ -273,7 +276,7 @@ describe('StateGateway', () => { id: 'client1', data: {}, }; - const data = { x: 100, y: 200 }; + const data: CursorPositionDto = { x: 100, y: 200 }; expect(() => { gateway.handleCursorReportPosition( diff --git a/src/ws/state/state.gateway.ts b/src/ws/state/state.gateway.ts index 1095f32..9a7b93b 100644 --- a/src/ws/state/state.gateway.ts +++ b/src/ws/state/state.gateway.ts @@ -1,4 +1,4 @@ -import { Logger, Inject, forwardRef } from '@nestjs/common'; +import { Logger } from '@nestjs/common'; import { OnGatewayConnection, OnGatewayDisconnect, @@ -8,16 +8,22 @@ import { WebSocketServer, WsException, } from '@nestjs/websockets'; +import { OnEvent } from '@nestjs/event-emitter'; import type { Server } from 'socket.io'; import type { AuthenticatedSocket } from '../../types/socket'; import { AuthService } from '../../auth/auth.service'; import { JwtVerificationService } from '../../auth/services/jwt-verification.service'; import { CursorPositionDto } from '../dto/cursor-position.dto'; -import { - FriendRequestWithRelations, - FriendsService, -} from '../../friends/friends.service'; +import { PrismaService } from '../../database/prisma.service'; + +import { FriendEvents } from '../../friends/events/friend.events'; +import type { + FriendRequestReceivedEvent, + FriendRequestAcceptedEvent, + FriendRequestDeniedEvent, + UnfriendedEvent, +} from '../../friends/events/friend.events'; const WS_EVENT = { CURSOR_REPORT_POSITION: 'cursor-report-position', @@ -40,14 +46,14 @@ export class StateGateway { private readonly logger = new Logger(StateGateway.name); private userSocketMap: Map = new Map(); + private lastBroadcastMap: Map = new Map(); @WebSocketServer() io: Server; constructor( private readonly authService: AuthService, private readonly jwtVerificationService: JwtVerificationService, - @Inject(forwardRef(() => FriendsService)) - private readonly friendsService: FriendsService, + private readonly prisma: PrismaService, ) {} afterInit() { @@ -91,8 +97,11 @@ export class StateGateway this.userSocketMap.set(user.id, client.id); client.data.userId = user.id; - // Initialize friends cache - const friends = await this.friendsService.getFriends(user.id); + // Initialize friends cache using Prisma directly + const friends = await this.prisma.friendship.findMany({ + where: { userId: user.id }, + select: { friendId: true }, + }); client.data.friends = new Set(friends.map((f) => f.friendId)); const { sockets } = this.io.sockets; @@ -119,6 +128,7 @@ export class StateGateway const currentSocketId = this.userSocketMap.get(userId); if (currentSocketId === client.id) { this.userSocketMap.delete(userId); + this.lastBroadcastMap.delete(userId); // Notify friends that this user has disconnected const friends = client.data.friends; @@ -138,6 +148,7 @@ export class StateGateway for (const [uid, socketId] of this.userSocketMap.entries()) { if (socketId === client.id) { this.userSocketMap.delete(uid); + this.lastBroadcastMap.delete(uid); break; } } @@ -171,6 +182,13 @@ export class StateGateway return; } + const now = Date.now(); + const lastBroadcast = this.lastBroadcastMap.get(currentUserId) || 0; + if (now - lastBroadcast < 100) { + return; + } + this.lastBroadcastMap.set(currentUserId, now); + // Broadcast to online friends const friends = client.data.friends; if (friends) { @@ -189,10 +207,9 @@ export class StateGateway } } - emitFriendRequestReceived( - userId: string, - friendRequest: FriendRequestWithRelations, - ) { + @OnEvent(FriendEvents.REQUEST_RECEIVED) + handleFriendRequestReceived(payload: FriendRequestReceivedEvent) { + const { userId, friendRequest } = payload; const socketId = this.userSocketMap.get(userId); if (socketId) { this.io.to(socketId).emit(WS_EVENT.FRIEND_REQUEST_RECEIVED, { @@ -211,20 +228,14 @@ export class StateGateway } } - emitFriendRequestAccepted( - userId: string, - friendRequest: FriendRequestWithRelations, - ) { + @OnEvent(FriendEvents.REQUEST_ACCEPTED) + handleFriendRequestAccepted(payload: FriendRequestAcceptedEvent) { + const { userId, friendRequest } = payload; + const socketId = this.userSocketMap.get(userId); + + // 1. Update cache for the user who sent the request (userId / friendRequest.senderId) if (socketId) { - // Update cache for the user accepting (userId here is the sender of the original request) - // Wait, in friends.controller: acceptFriendRequest returns the request. - // emitFriendRequestAccepted is called with friendRequest.senderId (the one who sent the request). - // The one who accepted is friendRequest.receiverId. - - // We need to update cache for BOTH users if they are online. - - // 1. Update cache for the user who sent the request (userId / friendRequest.senderId) const senderSocket = this.io.sockets.sockets.get( socketId, ) as AuthenticatedSocket; @@ -232,17 +243,6 @@ export class StateGateway senderSocket.data.friends.add(friendRequest.receiverId); } - // 2. Update cache for the user who accepted the request (friendRequest.receiverId) - const receiverSocketId = this.userSocketMap.get(friendRequest.receiverId); - if (receiverSocketId) { - const receiverSocket = this.io.sockets.sockets.get( - receiverSocketId, - ) as AuthenticatedSocket; - if (receiverSocket && receiverSocket.data.friends) { - receiverSocket.data.friends.add(friendRequest.senderId); - } - } - this.io.to(socketId).emit(WS_EVENT.FRIEND_REQUEST_ACCEPTED, { id: friendRequest.id, friend: { @@ -257,12 +257,22 @@ export class StateGateway `Emitted friend request accepted notification to user ${userId}`, ); } + + // 2. Update cache for the user who accepted the request (friendRequest.receiverId) + const receiverSocketId = this.userSocketMap.get(friendRequest.receiverId); + if (receiverSocketId) { + const receiverSocket = this.io.sockets.sockets.get( + receiverSocketId, + ) as AuthenticatedSocket; + if (receiverSocket && receiverSocket.data.friends) { + receiverSocket.data.friends.add(friendRequest.senderId); + } + } } - emitFriendRequestDenied( - userId: string, - friendRequest: FriendRequestWithRelations, - ) { + @OnEvent(FriendEvents.REQUEST_DENIED) + handleFriendRequestDenied(payload: FriendRequestDeniedEvent) { + const { userId, friendRequest } = payload; const socketId = this.userSocketMap.get(userId); if (socketId) { this.io.to(socketId).emit(WS_EVENT.FRIEND_REQUEST_DENIED, { @@ -281,17 +291,14 @@ export class StateGateway } } - emitUnfriended(userId: string, friendId: string) { + @OnEvent(FriendEvents.UNFRIENDED) + handleUnfriended(payload: UnfriendedEvent) { + const { userId, friendId } = payload; + const socketId = this.userSocketMap.get(userId); + + // 1. Update cache for the user receiving the notification (userId) if (socketId) { - // Update cache for the user being unfriended (userId) - // Wait, emitUnfriended is called with (friendId, user.id) in controller. - // So userId here is the friendId (the one being removed from friend list of the initiator). - // friendId here is the initiator (user.id). - - // We need to update cache for BOTH users. - - // 1. Update cache for the user receiving the notification (userId) const socket = this.io.sockets.sockets.get( socketId, ) as AuthenticatedSocket; @@ -299,31 +306,20 @@ export class StateGateway socket.data.friends.delete(friendId); } - // 2. Update cache for the user initiating the unfriend (friendId) - const initiatorSocketId = this.userSocketMap.get(friendId); - if (initiatorSocketId) { - const initiatorSocket = this.io.sockets.sockets.get( - initiatorSocketId, - ) as AuthenticatedSocket; - if (initiatorSocket && initiatorSocket.data.friends) { - initiatorSocket.data.friends.delete(userId); - } - } - this.io.to(socketId).emit(WS_EVENT.UNFRIENDED, { friendId, }); this.logger.debug(`Emitted unfriended notification to user ${userId}`); - } else { - // If the notified user is offline, we still need to update the initiator's cache if they are online - const initiatorSocketId = this.userSocketMap.get(friendId); - if (initiatorSocketId) { - const initiatorSocket = this.io.sockets.sockets.get( - initiatorSocketId, - ) as AuthenticatedSocket; - if (initiatorSocket && initiatorSocket.data.friends) { - initiatorSocket.data.friends.delete(userId); - } + } + + // 2. Update cache for the user initiating the unfriend (friendId) + const initiatorSocketId = this.userSocketMap.get(friendId); + if (initiatorSocketId) { + const initiatorSocket = this.io.sockets.sockets.get( + initiatorSocketId, + ) as AuthenticatedSocket; + if (initiatorSocket && initiatorSocket.data.friends) { + initiatorSocket.data.friends.delete(userId); } } }