From 8144200e4001f90cec80e8f46eba3eecdb50acde Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 1 Dec 2025 12:33:28 +0000 Subject: [PATCH 1/2] Initial plan From 590fdaa2dc3785249bca2c4b1c4ef487c7eae007 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 1 Dec 2025 12:41:41 +0000 Subject: [PATCH 2/2] Refactor: Remove automatic "Mark as Read" from getChatHistoryForAnonymous and improve MARK_AS_READ WebSocket handler Co-authored-by: fred-maina <150168105+fred-maina@users.noreply.github.com> --- .../Controllers/ChatWebSocketHandler.java | 9 +- .../Repositories/ChatMessageRepository.java | 5 - .../chatapp/core/Services/ChatService.java | 4 +- .../core/Services/MessagingService.java | 8 +- .../core/Services/ChatServiceTest.java | 123 ++++++++++++++++++ .../core/Services/MessagingServiceTest.java | 91 +++++++++++++ 6 files changed, 227 insertions(+), 13 deletions(-) create mode 100644 src/test/java/com/fredmaina/chatapp/core/Services/ChatServiceTest.java create mode 100644 src/test/java/com/fredmaina/chatapp/core/Services/MessagingServiceTest.java diff --git a/src/main/java/com/fredmaina/chatapp/core/Controllers/ChatWebSocketHandler.java b/src/main/java/com/fredmaina/chatapp/core/Controllers/ChatWebSocketHandler.java index 2770ece..81a6e50 100644 --- a/src/main/java/com/fredmaina/chatapp/core/Controllers/ChatWebSocketHandler.java +++ b/src/main/java/com/fredmaina/chatapp/core/Controllers/ChatWebSocketHandler.java @@ -101,8 +101,13 @@ protected void handleTextMessage(WebSocketSession session, TextMessage message) } } case MARK_AS_READ -> { - messagingService.setMessageAsRead(payload.getChatId()); - log.info(payload.toString()); + String email = extractUsernameFromJWT(session); + if (email != null && payload.getChatId() != null) { + messagingService.setMessageAsRead(email, payload.getChatId()); + log.info(payload.toString()); + } else { + log.warn("MARK_AS_READ failed: email={}, chatId={}", email, payload.getChatId()); + } } default -> log.warn("Unsupported message type: {}", payload.getType()); } diff --git a/src/main/java/com/fredmaina/chatapp/core/Repositories/ChatMessageRepository.java b/src/main/java/com/fredmaina/chatapp/core/Repositories/ChatMessageRepository.java index 9e5979d..4ba0603 100644 --- a/src/main/java/com/fredmaina/chatapp/core/Repositories/ChatMessageRepository.java +++ b/src/main/java/com/fredmaina/chatapp/core/Repositories/ChatMessageRepository.java @@ -56,11 +56,6 @@ List findFullChatHistory(@Param("sessionId") String sessionId, "WHERE m.toUser.id = :userId AND m.fromSessionId = :fromSessionId AND m.isRead = false") void markMessagesAsRead(@Param("userId") UUID userId, @Param("fromSessionId") String fromSessionId); - @Modifying - @Query("UPDATE ChatMessage m SET m.isRead =true " + - "WHERE m.fromSessionId=:sessionId AND m.isRead=false") - void markMessagesAsRead(String sessionId); - void deleteByFromSessionIdAndToUserId(String fromSessionId, UUID toUserId); void deleteByToSessionIdAndFromUserId(String toSessionId, UUID fromUserId); diff --git a/src/main/java/com/fredmaina/chatapp/core/Services/ChatService.java b/src/main/java/com/fredmaina/chatapp/core/Services/ChatService.java index 89d7db3..0f6a110 100644 --- a/src/main/java/com/fredmaina/chatapp/core/Services/ChatService.java +++ b/src/main/java/com/fredmaina/chatapp/core/Services/ChatService.java @@ -72,15 +72,13 @@ public List getUserChatSessions(UUID userId) { return sessions; } - @Transactional + @Transactional(readOnly = true) public List getChatHistoryForAnonymous(String sessionId, String recipientUsername) { User recipient = userRepository.findByUsername(recipientUsername) .orElseThrow(() -> new RuntimeException("Recipient user not found: " + recipientUsername)); List messages = chatMessageRepository.findFullChatHistory(sessionId,recipient.getId()); - chatMessageRepository.markMessagesAsRead(recipient.getId(), sessionId); - return messages.stream() .map(msg -> ChatMessageMapper.toDto(msg, recipient.getId())) .collect(Collectors.toList()); diff --git a/src/main/java/com/fredmaina/chatapp/core/Services/MessagingService.java b/src/main/java/com/fredmaina/chatapp/core/Services/MessagingService.java index 32b1c7e..68553cc 100644 --- a/src/main/java/com/fredmaina/chatapp/core/Services/MessagingService.java +++ b/src/main/java/com/fredmaina/chatapp/core/Services/MessagingService.java @@ -130,9 +130,11 @@ public void sendMessageFromUser(String username, String targetSessionId, String } @Transactional - public void setMessageAsRead(String sessionId) { - log.info("Marking messages as read for session {}", sessionId); - chatMessageRepository.markMessagesAsRead(sessionId); + public void setMessageAsRead(String email, String sessionId) { + User user = userRepository.findByEmail(email) + .orElseThrow(() -> new RuntimeException("User not found: " + email)); + log.info("Marking messages as read for user {} from session {}", email, sessionId); + chatMessageRepository.markMessagesAsRead(user.getId(), sessionId); } // For RedisSubscriber to call diff --git a/src/test/java/com/fredmaina/chatapp/core/Services/ChatServiceTest.java b/src/test/java/com/fredmaina/chatapp/core/Services/ChatServiceTest.java new file mode 100644 index 0000000..afec841 --- /dev/null +++ b/src/test/java/com/fredmaina/chatapp/core/Services/ChatServiceTest.java @@ -0,0 +1,123 @@ +package com.fredmaina.chatapp.core.Services; + +import com.fredmaina.chatapp.Auth.Models.User; +import com.fredmaina.chatapp.Auth.Repositories.UserRepository; +import com.fredmaina.chatapp.core.DTOs.ChatMessageDto; +import com.fredmaina.chatapp.core.Repositories.ChatMessageRepository; +import com.fredmaina.chatapp.core.models.ChatMessage; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.mockito.*; + +import java.time.Instant; +import java.util.List; +import java.util.Optional; +import java.util.UUID; + +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.Mockito.*; + +class ChatServiceTest { + + @Mock + private ChatMessageRepository chatMessageRepository; + + @Mock + private UserRepository userRepository; + + @InjectMocks + private ChatService chatService; + + private User testUser; + private UUID testUserId; + private String testSessionId; + + @BeforeEach + void setup() { + MockitoAnnotations.openMocks(this); + + testUserId = UUID.randomUUID(); + testSessionId = "test-session-123"; + + testUser = new User(); + testUser.setId(testUserId); + testUser.setUsername("testuser"); + testUser.setEmail("test@example.com"); + } + + @Test + void getChatHistoryForAnonymous_shouldNotMarkMessagesAsRead() { + // Arrange + ChatMessage message = ChatMessage.builder() + .messageId(UUID.randomUUID()) + .content("Hello") + .fromSessionId(testSessionId) + .toUser(testUser) + .nickname("Anonymous") + .isRead(false) + .timestamp(Instant.now()) + .build(); + + when(userRepository.findByUsername("testuser")).thenReturn(Optional.of(testUser)); + when(chatMessageRepository.findFullChatHistory(testSessionId, testUserId)).thenReturn(List.of(message)); + + // Act + List result = chatService.getChatHistoryForAnonymous(testSessionId, "testuser"); + + // Assert + assertNotNull(result); + assertEquals(1, result.size()); + + // Verify that markMessagesAsRead was NOT called - this is the key assertion + verify(chatMessageRepository, never()).markMessagesAsRead(any(UUID.class), anyString()); + } + + @Test + void getChatHistoryForAnonymous_shouldReturnMessagesUnchanged() { + // Arrange + ChatMessage message1 = ChatMessage.builder() + .messageId(UUID.randomUUID()) + .content("Hello") + .fromSessionId(testSessionId) + .toUser(testUser) + .nickname("Anonymous") + .isRead(false) + .timestamp(Instant.now().minusSeconds(60)) + .build(); + + ChatMessage message2 = ChatMessage.builder() + .messageId(UUID.randomUUID()) + .content("How are you?") + .fromSessionId(testSessionId) + .toUser(testUser) + .nickname("Anonymous") + .isRead(false) + .timestamp(Instant.now()) + .build(); + + when(userRepository.findByUsername("testuser")).thenReturn(Optional.of(testUser)); + when(chatMessageRepository.findFullChatHistory(testSessionId, testUserId)).thenReturn(List.of(message1, message2)); + + // Act + List result = chatService.getChatHistoryForAnonymous(testSessionId, "testuser"); + + // Assert + assertNotNull(result); + assertEquals(2, result.size()); + assertEquals("Hello", result.get(0).getText()); + assertEquals("How are you?", result.get(1).getText()); + } + + @Test + void getChatHistoryForAnonymous_shouldThrowExceptionForUnknownUser() { + // Arrange + when(userRepository.findByUsername("unknownuser")).thenReturn(Optional.empty()); + + // Act & Assert + RuntimeException exception = assertThrows(RuntimeException.class, () -> + chatService.getChatHistoryForAnonymous(testSessionId, "unknownuser") + ); + + assertEquals("Recipient user not found: unknownuser", exception.getMessage()); + } +} diff --git a/src/test/java/com/fredmaina/chatapp/core/Services/MessagingServiceTest.java b/src/test/java/com/fredmaina/chatapp/core/Services/MessagingServiceTest.java new file mode 100644 index 0000000..4410985 --- /dev/null +++ b/src/test/java/com/fredmaina/chatapp/core/Services/MessagingServiceTest.java @@ -0,0 +1,91 @@ +package com.fredmaina.chatapp.core.Services; + +import com.fredmaina.chatapp.Auth.Models.User; +import com.fredmaina.chatapp.Auth.Repositories.UserRepository; +import com.fredmaina.chatapp.core.Repositories.ChatMessageRepository; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.mockito.*; +import org.springframework.cache.CacheManager; +import org.springframework.data.redis.core.RedisTemplate; + +import java.util.Optional; +import java.util.UUID; + +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.Mockito.*; + +class MessagingServiceTest { + + @Mock + private UserRepository userRepository; + + @Mock + private ChatMessageRepository chatMessageRepository; + + @Mock + private CacheManager cacheManager; + + @Mock + private RedisTemplate redisTemplate; + + @InjectMocks + private MessagingService messagingService; + + private User testUser; + private UUID testUserId; + private String testSessionId; + + @BeforeEach + void setup() { + MockitoAnnotations.openMocks(this); + + testUserId = UUID.randomUUID(); + testSessionId = "test-session-123"; + + testUser = new User(); + testUser.setId(testUserId); + testUser.setUsername("testuser"); + testUser.setEmail("test@example.com"); + } + + @Test + void setMessageAsRead_shouldMarkMessagesForUserAndSession() { + // Arrange + when(userRepository.findByEmail("test@example.com")).thenReturn(Optional.of(testUser)); + + // Act + messagingService.setMessageAsRead("test@example.com", testSessionId); + + // Assert + verify(userRepository).findByEmail("test@example.com"); + verify(chatMessageRepository).markMessagesAsRead(testUserId, testSessionId); + } + + @Test + void setMessageAsRead_shouldThrowExceptionForUnknownUser() { + // Arrange + when(userRepository.findByEmail("unknown@example.com")).thenReturn(Optional.empty()); + + // Act & Assert + RuntimeException exception = assertThrows(RuntimeException.class, () -> + messagingService.setMessageAsRead("unknown@example.com", testSessionId) + ); + + assertEquals("User not found: unknown@example.com", exception.getMessage()); + verify(chatMessageRepository, never()).markMessagesAsRead(any(UUID.class), anyString()); + } + + @Test + void setMessageAsRead_shouldUseCorrectSessionId() { + // Arrange + String differentSessionId = "different-session-456"; + when(userRepository.findByEmail("test@example.com")).thenReturn(Optional.of(testUser)); + + // Act + messagingService.setMessageAsRead("test@example.com", differentSessionId); + + // Assert + verify(chatMessageRepository).markMessagesAsRead(testUserId, differentSessionId); + } +}