743 Add transaction support to the Chat Module #762
Conversation
CapoMK25
requested review from
MikhailDeriabin,
PlayJeri and
hoan301298
as code owners
hoan301298
left a comment
hoan301298
left a comment
There was a problem hiding this comment.
I just reviewed your PR quickly but unfortunately, I did not see you implement the transaction methods. Please use the transaction methods in Common/functions as we worked on it before.
hoan301298
left a comment
hoan301298
left a comment
There was a problem hiding this comment.
Since those changes still lacked of many requirements and need to be checked carefully with options type, documentation comments. Please check again every methods, workflow and documentation comments to match the requirements.
| @WebSocketGateway(apiPort) | ||
| @UseFilters(GlobalWsExceptionFilter) | ||
| export class ChatGateway implements OnGatewayConnection, OnGatewayDisconnect { | ||
| private readonly logger = new Logger(ChatGateway.name); |
There was a problem hiding this comment.
What is your purpose to import Logger in here? The logger is declared but never in use.
| } | ||
|
|
||
| @SubscribeMessage('clanMessage') | ||
| @WsLog() |
There was a problem hiding this comment.
Please do not remove @WsLog() if you don't understand what is it used for. Also check for functions below.
| @ConnectedSocket() client: WebSocketUser, | ||
| ) { | ||
| await this.clanChatService.handleNewClanMessage(client, message); | ||
| async handleNewClanMessage( |
There was a problem hiding this comment.
The name of this function is the same with clanChatService.handleNewClanMessage. Please be aware of changing those things.
| const [session, sessionError] = await initializeSession(this.connection); | ||
| if (sessionError) return [null, sessionError]; | ||
|
|
||
| try { |
There was a problem hiding this comment.
Again, remove the try-catch block that we handle to catch the errors manually!
| @SubscribeMessage('clanMessageReaction') | ||
| @WsLog() | ||
| async handleClanMessageReaction( | ||
| async handleNewClanReaction( |
There was a problem hiding this comment.
like above, please rename it as before
| async handleNewGlobalReaction( | ||
| client: WebSocketUser, | ||
| reaction: AddReactionDto, | ||
| options?: any, |
There was a problem hiding this comment.
please follow the type of option by each service they are using, any is very bad option in here.
| */ | ||
| async handleNewClanMessage(client: WebSocketUser, message: WsMessageBodyDto) { | ||
| const chatMessage = new CreateChatMessageDto({ | ||
| async handleNewClanMessage( |
There was a problem hiding this comment.
add Documentation comments for options param
| async handleNewClanReaction( | ||
| client: WebSocketUser, | ||
| reaction: AddReactionDto, | ||
| options?: TIServiceUpdateManyOptions, |
There was a problem hiding this comment.
options: TIServiceUpdateByIdOptions following the ChatService.addReaction
| * @param chatMessage - The DTO containing the new chat message data. | ||
| * @param client - The WebSocket user who sent the message. | ||
| * @param chatType - The type of chat (e.g., group, private). | ||
| * @param recipients - The set of WebSocket users to broadcast the message to. |
| client: WebSocketUser, | ||
| reaction: AddReactionDto, | ||
| recipients: Set<WebSocketUser>, | ||
| options?: any, |
There was a problem hiding this comment.
add the correct type for options
Brief description
This PR implements MongoDB Transaction support for both the Global and Clan chat modules. The goal is to ensure all or nothing behavior, message persistence and any secondary operations (e.g., player stats updates or daily task progress), preventing "ghost" messages if a downstream operation fails.
Key changes
Commit 1: Updated the tests to handle the new transaction support instead of the other way around
Commit 2: Implemented the actual transaction support
Updated
GlobalChatGatewayandClanChatGatewayto initiate aClientSessionvia@InjectConnection()Modified
handleNewGlobalMessageandhandleNewClanMessageto accept and pass thesessiondown to the service and repository layers.Implemented
try/catch/finallyblocks to ensureabortTransaction()is called on failure and thesessionis always closed.Verification results
Since standard MQTT testing is almost impossible in Postman, the following verification was performed:
Change list
src/__tests__/chat/...Updated 4 test suites to support new method signatures and verify DB call counts.src/chat/chat.gateway.tsImplemented transaction lifecycle (session management) for all chat-related WebSocket events.ChatService.createOne,readOneById, andupdateOneByIdto utilize provided database sessions.src/chat/service/clanChat.service.tsUpdated Clan message/reaction handlers to support transactional all-or-nothing behavior.src/chat/service/globalChat.service.tsUpdated Global message/reaction handlers to support transactional all-or-nothing behavior.