Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion backend/src/main/java/com/accord/config/WebSocketConfig.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,9 @@ public class WebSocketConfig implements WebSocketMessageBrokerConfigurer {

@Override
public void configureMessageBroker(MessageBrokerRegistry config) {
config.enableSimpleBroker("/topic");
config.enableSimpleBroker("/topic", "/user");
config.setApplicationDestinationPrefixes("/app");
config.setUserDestinationPrefix("/user");
Copy link

Copilot AI Feb 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The WebSocket configuration sets the user destination prefix to "/user" explicitly with setUserDestinationPrefix("/user"), but this is actually the default value in Spring. While this is not incorrect, it's redundant. The configuration could be simplified by removing line 27, as Spring WebSocket already defaults to "/user" for user destinations.

This is a minor code cleanliness issue and does not affect functionality.

Suggested change
config.setUserDestinationPrefix("/user");

Copilot uses AI. Check for mistakes.
}

@Override
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,162 @@
package com.accord.controller;

import com.accord.model.DirectMessage;
import com.accord.model.User;
import com.accord.service.DirectMessageService;
import com.accord.service.UserService;
import com.accord.util.ValidationUtils;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.http.ResponseEntity;
import org.springframework.messaging.simp.SimpMessagingTemplate;
import org.springframework.web.bind.annotation.*;

import java.util.List;
import java.util.Map;
import java.util.Optional;

@RestController
@RequestMapping("/api/dm")
@CrossOrigin(origins = "${app.cors.allowed-origins}")
public class DirectMessageController {

private static final int MAX_LIMIT = 500;

@Value("${app.message.max-length}")
private int maxMessageLength;

@Autowired
private DirectMessageService directMessageService;

@Autowired
private UserService userService;

@Autowired
private SimpMessagingTemplate messagingTemplate;

@PostMapping("/send")
public ResponseEntity<?> sendDirectMessage(@RequestBody Map<String, Object> payload) {
String senderUsername = (String) payload.get("senderUsername");
String recipientUsername = (String) payload.get("recipientUsername");
String content = (String) payload.get("content");

if (senderUsername == null || senderUsername.trim().isEmpty()) {
return ResponseEntity.badRequest().body(Map.of("error", "Sender username is required"));
}
if (recipientUsername == null || recipientUsername.trim().isEmpty()) {
return ResponseEntity.badRequest().body(Map.of("error", "Recipient username is required"));
}
if (!ValidationUtils.isValidContent(content, maxMessageLength)) {
return ResponseEntity.badRequest().body(Map.of("error", "Invalid message content"));
}
if (senderUsername.trim().equals(recipientUsername.trim())) {
return ResponseEntity.badRequest().body(Map.of("error", "Cannot send a message to yourself"));
}

Optional<User> sender = userService.findByUsername(senderUsername.trim());
Optional<User> recipient = userService.findByUsername(recipientUsername.trim());

if (sender.isEmpty()) {
return ResponseEntity.badRequest().body(Map.of("error", "Sender not found"));
}
if (recipient.isEmpty()) {
return ResponseEntity.badRequest().body(Map.of("error", "Recipient not found"));
}

DirectMessage message = directMessageService.sendMessage(
sender.get().getId(), recipient.get().getId(), content.trim());

// Send real-time notification to recipient via WebSocket
messagingTemplate.convertAndSend(
"/user/" + recipient.get().getId() + "/queue/messages", message);
Copy link

Copilot AI Feb 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Critical bug: The WebSocket destination path uses user IDs, but Spring's user destination resolver matches against the authenticated principal name, which is the username (not user ID).

When messagingTemplate.convertAndSend("/user/2/queue/messages", message) is called, Spring looks for WebSocket sessions with principal name "2", but the actual principal names are usernames like "bob" (set in WebSocketAuthInterceptor line 46 using the username from JWT).

This means direct messages will never be delivered via WebSocket - they will be silently dropped. The system appears to work only because the frontend calls the REST API to send messages and immediately displays them, but real-time notifications to recipients will fail.

The fix requires using usernames instead of user IDs in the destination path, e.g., messagingTemplate.convertAndSend("/user/" + recipient.get().getUsername() + "/queue/messages", message), and the frontend should subscribe using their username rather than userId.

Suggested change
"/user/" + recipient.get().getId() + "/queue/messages", message);
"/user/" + recipient.get().getUsername() + "/queue/messages", message);

Copilot uses AI. Check for mistakes.

return ResponseEntity.ok(message);
}

@GetMapping("/conversation")
public ResponseEntity<?> getConversation(
@RequestParam String user1,
@RequestParam String user2,
@RequestParam(defaultValue = "50") int limit) {

if (user1 == null || user1.trim().isEmpty() || user2 == null || user2.trim().isEmpty()) {
return ResponseEntity.badRequest().body(Map.of("error", "Both usernames are required"));
}

Optional<User> userOne = userService.findByUsername(user1.trim());
Optional<User> userTwo = userService.findByUsername(user2.trim());

if (userOne.isEmpty() || userTwo.isEmpty()) {
return ResponseEntity.badRequest().body(Map.of("error", "User not found"));
}

if (limit < 1) {
limit = 1;
} else if (limit > MAX_LIMIT) {
limit = MAX_LIMIT;
}

List<DirectMessage> messages = directMessageService.getConversation(
userOne.get().getId(), userTwo.get().getId(), limit);
return ResponseEntity.ok(messages);
}

@PostMapping("/read/{messageId}")
public ResponseEntity<?> markAsRead(@PathVariable Long messageId,
@RequestParam String username) {
Optional<User> user = userService.findByUsername(username.trim());
if (user.isEmpty()) {
return ResponseEntity.badRequest().body(Map.of("error", "User not found"));
}

directMessageService.markAsRead(messageId, user.get().getId());
return ResponseEntity.ok(Map.of("status", "marked as read"));
}

@PostMapping("/read/conversation")
public ResponseEntity<?> markConversationAsRead(@RequestBody Map<String, String> payload) {
String recipientUsername = payload.get("recipientUsername");
String senderUsername = payload.get("senderUsername");

if (recipientUsername == null || senderUsername == null) {
return ResponseEntity.badRequest().body(Map.of("error", "Both usernames are required"));
}

Optional<User> recipient = userService.findByUsername(recipientUsername.trim());
Optional<User> sender = userService.findByUsername(senderUsername.trim());

if (recipient.isEmpty() || sender.isEmpty()) {
return ResponseEntity.badRequest().body(Map.of("error", "User not found"));
}

directMessageService.markConversationAsRead(recipient.get().getId(), sender.get().getId());
return ResponseEntity.ok(Map.of("status", "conversation marked as read"));
}

@GetMapping("/unread")
public ResponseEntity<?> getUnreadCount(@RequestParam String username) {
Optional<User> user = userService.findByUsername(username.trim());
if (user.isEmpty()) {
return ResponseEntity.badRequest().body(Map.of("error", "User not found"));
}

long count = directMessageService.getUnreadCount(user.get().getId());
return ResponseEntity.ok(Map.of("unreadCount", count));
}

@GetMapping("/unread/from")
public ResponseEntity<?> getUnreadCountFromSender(
@RequestParam String username,
@RequestParam String senderUsername) {
Optional<User> user = userService.findByUsername(username.trim());
Optional<User> sender = userService.findByUsername(senderUsername.trim());

if (user.isEmpty() || sender.isEmpty()) {
return ResponseEntity.badRequest().body(Map.of("error", "User not found"));
}

long count = directMessageService.getUnreadCountFromSender(
user.get().getId(), sender.get().getId());
return ResponseEntity.ok(Map.of("unreadCount", count));
}
}
16 changes: 16 additions & 0 deletions backend/src/main/java/com/accord/controller/UserController.java
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,10 @@
import org.springframework.security.authentication.UsernamePasswordAuthenticationToken;
import org.springframework.web.bind.annotation.*;

import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;

/**
* User authentication controller handling registration and login.
Expand Down Expand Up @@ -125,4 +128,17 @@ public ResponseEntity<Boolean> checkUsername(@PathVariable String username) {
boolean exists = userService.userExists(username.trim());
return ResponseEntity.ok(exists);
}

@GetMapping
public ResponseEntity<?> listUsers() {
List<Map<String, Object>> users = userService.findAllUsers().stream()
.map(user -> {
Map<String, Object> userMap = new HashMap<>();
userMap.put("id", user.getId());
userMap.put("username", user.getUsername());
return userMap;
})
.collect(Collectors.toList());
return ResponseEntity.ok(users);
}
Comment on lines +132 to +143
Copy link

Copilot AI Feb 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The /api/users endpoint returns all users' IDs and usernames without any filtering or pagination. This could become a privacy and performance issue as the user base grows. Consider:

  1. Implementing pagination to handle large user lists
  2. Adding filtering capabilities (e.g., search by username prefix)
  3. Evaluating whether all users should be visible to everyone, or if there should be privacy controls
  4. For very large user bases, consider lazy-loading users as needed rather than fetching all at once

Copilot uses AI. Check for mistakes.
}
90 changes: 90 additions & 0 deletions backend/src/main/java/com/accord/model/DirectMessage.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
package com.accord.model;

import jakarta.persistence.*;
import java.time.LocalDateTime;

@Entity
@Table(name = "direct_messages")
public class DirectMessage {

@Id
@GeneratedValue(strategy = GenerationType.IDENTITY)
private Long id;

@Column(nullable = false)
private Long senderId;

@Column(nullable = false)
private Long recipientId;

Copy link

Copilot AI Feb 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The DirectMessage entity does not have any validation constraints on the senderId and recipientId fields to ensure they reference valid users. While the controller validates that users exist before saving messages, the entity could benefit from referential integrity if these were @manytoone relationships to User entities, or at minimum @column(nullable = false) constraints (which are present).

Consider whether foreign key constraints should be added at the database level to ensure data integrity, preventing orphaned messages if users are deleted. This would require a decision on the deletion cascade behavior (e.g., should messages be deleted when a user is deleted, or should the user deletion be prevented if they have messages?).

Suggested change
@ManyToOne(fetch = FetchType.LAZY)
@JoinColumn(name = "sender_id", insertable = false, updatable = false,
foreignKey = @ForeignKey(name = "fk_direct_message_sender"))
private User sender;
@ManyToOne(fetch = FetchType.LAZY)
@JoinColumn(name = "recipient_id", insertable = false, updatable = false,
foreignKey = @ForeignKey(name = "fk_direct_message_recipient"))
private User recipient;

Copilot uses AI. Check for mistakes.
@Column(nullable = false, length = 1000)
private String content;

@Column(nullable = false)
private LocalDateTime timestamp;

@Column(nullable = false)
private boolean read;
Comment on lines +6 to +27
Copy link

Copilot AI Feb 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The DirectMessage entity lacks database indexes for frequently queried fields. The queries in DirectMessageRepository filter by senderId, recipientId, and read status, but these columns are not indexed. This will cause performance degradation as the number of direct messages grows.

Consider adding composite indexes:

  1. Index on (recipientId, senderId, timestamp) for conversation queries
  2. Index on (recipientId, read) for unread count queries
  3. Index on (recipientId, senderId, read) for the markConversationAsRead query

Add these using JPA annotations like @table(indexes = {@Index(columnList = "recipientId,senderId,timestamp")}) or create them via migration scripts.

Copilot uses AI. Check for mistakes.

public DirectMessage() {
this.timestamp = LocalDateTime.now();
this.read = false;
}

public DirectMessage(Long senderId, Long recipientId, String content) {
this.senderId = senderId;
this.recipientId = recipientId;
this.content = content;
this.timestamp = LocalDateTime.now();
this.read = false;
}

// Getters and Setters
public Long getId() {
return id;
}

public void setId(Long id) {
this.id = id;
}

public Long getSenderId() {
return senderId;
}

public void setSenderId(Long senderId) {
this.senderId = senderId;
}

public Long getRecipientId() {
return recipientId;
}

public void setRecipientId(Long recipientId) {
this.recipientId = recipientId;
}

public String getContent() {
return content;
}

public void setContent(String content) {
this.content = content;
}

public LocalDateTime getTimestamp() {
return timestamp;
}

public void setTimestamp(LocalDateTime timestamp) {
this.timestamp = timestamp;
}

public boolean isRead() {
return read;
}

public void setRead(boolean read) {
this.read = read;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
package com.accord.repository;

import com.accord.model.DirectMessage;
import org.springframework.data.domain.Pageable;
import org.springframework.data.jpa.repository.JpaRepository;
import org.springframework.data.jpa.repository.Modifying;
import org.springframework.data.jpa.repository.Query;
import org.springframework.data.repository.query.Param;
import org.springframework.stereotype.Repository;

import java.util.List;

@Repository
public interface DirectMessageRepository extends JpaRepository<DirectMessage, Long> {

@Query("SELECT dm FROM DirectMessage dm WHERE " +
"(dm.senderId = :userId1 AND dm.recipientId = :userId2) OR " +
"(dm.senderId = :userId2 AND dm.recipientId = :userId1) " +
"ORDER BY dm.timestamp DESC")
List<DirectMessage> findConversation(@Param("userId1") Long userId1,
@Param("userId2") Long userId2,
Pageable pageable);

@Modifying
@Query("UPDATE DirectMessage dm SET dm.read = true WHERE dm.recipientId = :recipientId AND dm.senderId = :senderId AND dm.read = false")
int markConversationAsRead(@Param("recipientId") Long recipientId, @Param("senderId") Long senderId);

@Query("SELECT COUNT(dm) FROM DirectMessage dm WHERE dm.recipientId = :recipientId AND dm.senderId = :senderId AND dm.read = false")
long countUnreadFromSender(@Param("recipientId") Long recipientId, @Param("senderId") Long senderId);
Comment on lines +26 to +29
Copy link

Copilot AI Feb 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The markConversationAsRead query updates messages where dm.recipientId = :recipientId AND dm.senderId = :senderId. However, the method signature has parameters (recipientId, senderId) which could be confusing. When marking a conversation as read, the current user is the recipient and wants to mark messages from the sender as read.

The parameter names match the query correctly, but the method would be clearer if it was named markMessagesFromSenderAsRead or if the parameters were renamed to (currentUserId, otherUserId) to better reflect the intent. This improves code maintainability and reduces the chance of parameter order mistakes.

Suggested change
int markConversationAsRead(@Param("recipientId") Long recipientId, @Param("senderId") Long senderId);
@Query("SELECT COUNT(dm) FROM DirectMessage dm WHERE dm.recipientId = :recipientId AND dm.senderId = :senderId AND dm.read = false")
long countUnreadFromSender(@Param("recipientId") Long recipientId, @Param("senderId") Long senderId);
int markConversationAsRead(@Param("recipientId") Long currentUserId, @Param("senderId") Long otherUserId);
@Query("SELECT COUNT(dm) FROM DirectMessage dm WHERE dm.recipientId = :recipientId AND dm.senderId = :senderId AND dm.read = false")
long countUnreadFromSender(@Param("recipientId") Long currentUserId, @Param("senderId") Long otherUserId);

Copilot uses AI. Check for mistakes.

@Query("SELECT COUNT(dm) FROM DirectMessage dm WHERE dm.recipientId = :recipientId AND dm.read = false")
long countUnreadForUser(@Param("recipientId") Long recipientId);

@Query("SELECT DISTINCT CASE WHEN dm.senderId = :userId THEN dm.recipientId ELSE dm.senderId END " +
"FROM DirectMessage dm WHERE dm.senderId = :userId OR dm.recipientId = :userId")
List<Long> findConversationPartnerIds(@Param("userId") Long userId);
}
57 changes: 57 additions & 0 deletions backend/src/main/java/com/accord/service/DirectMessageService.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
package com.accord.service;

import com.accord.model.DirectMessage;
import com.accord.repository.DirectMessageRepository;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.data.domain.PageRequest;
import org.springframework.data.domain.Pageable;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

import java.util.Collections;
import java.util.List;

@Service
public class DirectMessageService {

@Autowired
private DirectMessageRepository directMessageRepository;

public DirectMessage sendMessage(Long senderId, Long recipientId, String content) {
Copy link

Copilot AI Feb 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The DirectMessageService.sendMessage method does not validate that the senderId and recipientId are different. While the controller does check this (line 52-54 in DirectMessageController), the service layer should also enforce this business rule for defense in depth. If the service is called from another context in the future, self-messaging could be allowed unintentionally.

Add a check in the service method: if (senderId.equals(recipientId)) throw new IllegalArgumentException("Cannot send message to self");

Suggested change
public DirectMessage sendMessage(Long senderId, Long recipientId, String content) {
public DirectMessage sendMessage(Long senderId, Long recipientId, String content) {
if (senderId != null && senderId.equals(recipientId)) {
throw new IllegalArgumentException("Cannot send message to self");
}

Copilot uses AI. Check for mistakes.
DirectMessage message = new DirectMessage(senderId, recipientId, content);
return directMessageRepository.save(message);
}

public List<DirectMessage> getConversation(Long userId1, Long userId2, int limit) {
Pageable pageable = PageRequest.of(0, limit);
List<DirectMessage> messages = directMessageRepository.findConversation(userId1, userId2, pageable);
Collections.reverse(messages); // Show oldest first
Comment on lines +25 to +28
Copy link

Copilot AI Feb 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The findConversation query orders by dm.timestamp DESC to get the most recent messages, then the service reverses the list to show oldest-first. This approach is correct for pagination (getting the N most recent messages). However, the JPQL query could be optimized by adding a subquery or using a native query with ORDER BY timestamp DESC LIMIT :limit then reversing in SQL, depending on the database.

This is not a critical issue for the current implementation but could be optimized if performance becomes a concern with large conversation histories. The current approach is acceptable and readable.

Copilot uses AI. Check for mistakes.
return messages;
}

public void markAsRead(Long messageId, Long recipientId) {
directMessageRepository.findById(messageId).ifPresent(message -> {
if (message.getRecipientId().equals(recipientId)) {
message.setRead(true);
directMessageRepository.save(message);
}
});
Comment on lines +32 to +38
Copy link

Copilot AI Feb 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The markAsRead method silently does nothing if the message is not found or if the recipientId doesn't match. While this prevents errors, it makes it difficult to debug issues where read receipts aren't working. Consider logging a warning or debug message when a mark-as-read operation is skipped due to these conditions.

Example: logger.debug("Message {} not found or user {} is not the recipient", messageId, recipientId);

Copilot uses AI. Check for mistakes.
}

@Transactional
public void markConversationAsRead(Long recipientId, Long senderId) {
directMessageRepository.markConversationAsRead(recipientId, senderId);
}

public long getUnreadCount(Long recipientId) {
return directMessageRepository.countUnreadForUser(recipientId);
}

public long getUnreadCountFromSender(Long recipientId, Long senderId) {
return directMessageRepository.countUnreadFromSender(recipientId, senderId);
}

public List<Long> getConversationPartnerIds(Long userId) {
return directMessageRepository.findConversationPartnerIds(userId);
}
}
4 changes: 4 additions & 0 deletions backend/src/main/java/com/accord/service/UserService.java
Original file line number Diff line number Diff line change
Expand Up @@ -48,4 +48,8 @@ public Optional<User> findByUsername(String username) {
public boolean verifyPassword(String rawPassword, String encodedPassword) {
return passwordEncoder.matches(rawPassword, encodedPassword);
}

public java.util.List<User> findAllUsers() {
return userRepository.findAll();
}
}
Loading