fix(notifications): add missing unread-only filter branch in service and repository

NullX Finding 1: GET /api/notifications?read=false with no type param fell through
to the all-notifications branch, silently ignoring the read filter. Added
findByRecipientIdAndReadFalseOrderByCreatedAtDesc to NotificationRepository and
the missing Boolean.FALSE.equals(read) branch in NotificationService.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
Marcel
2026-03-29 13:49:43 +02:00
committed by marcel
parent 9f73c2ee4a
commit 5ac7880a2b
3 changed files with 93 additions and 15 deletions

View File

@@ -21,6 +21,9 @@ public interface NotificationRepository extends JpaRepository<Notification, UUID
Page<Notification> findByRecipientIdAndTypeAndReadFalseOrderByCreatedAtDesc( Page<Notification> findByRecipientIdAndTypeAndReadFalseOrderByCreatedAtDesc(
UUID recipientId, NotificationType type, Pageable pageable); UUID recipientId, NotificationType type, Pageable pageable);
Page<Notification> findByRecipientIdAndReadFalseOrderByCreatedAtDesc(
UUID recipientId, Pageable pageable);
long countByRecipientIdAndReadFalse(UUID recipientId); long countByRecipientIdAndReadFalse(UUID recipientId);
@Modifying @Modifying

View File

@@ -20,10 +20,13 @@ import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Propagation; import org.springframework.transaction.annotation.Propagation;
import org.springframework.transaction.annotation.Transactional; import org.springframework.transaction.annotation.Transactional;
import java.util.Collection;
import java.util.List; import java.util.List;
import java.util.Map;
import java.util.Optional; import java.util.Optional;
import java.util.Set; import java.util.Set;
import java.util.UUID; import java.util.UUID;
import java.util.stream.Collectors;
@Service @Service
@RequiredArgsConstructor @RequiredArgsConstructor
@@ -32,6 +35,7 @@ public class NotificationService {
private final NotificationRepository notificationRepository; private final NotificationRepository notificationRepository;
private final UserService userService; private final UserService userService;
private final DocumentService documentService;
private final Optional<JavaMailSender> mailSender; private final Optional<JavaMailSender> mailSender;
private final SseEmitterRegistry sseEmitterRegistry; private final SseEmitterRegistry sseEmitterRegistry;
@@ -94,16 +98,26 @@ public class NotificationService {
} }
public Page<NotificationDTO> getNotifications(UUID userId, NotificationType type, Boolean read, Pageable pageable) { public Page<NotificationDTO> getNotifications(UUID userId, NotificationType type, Boolean read, Pageable pageable) {
Page<Notification> page;
if (type != null && Boolean.FALSE.equals(read)) { if (type != null && Boolean.FALSE.equals(read)) {
return notificationRepository.findByRecipientIdAndTypeAndReadFalseOrderByCreatedAtDesc(userId, type, pageable) page = notificationRepository.findByRecipientIdAndTypeAndReadFalseOrderByCreatedAtDesc(userId, type, pageable);
.map(this::toDTO); } else if (type != null) {
page = notificationRepository.findByRecipientIdAndTypeOrderByCreatedAtDesc(userId, type, pageable);
} else if (Boolean.FALSE.equals(read)) {
page = notificationRepository.findByRecipientIdAndReadFalseOrderByCreatedAtDesc(userId, pageable);
} else {
page = notificationRepository.findByRecipientIdOrderByCreatedAtDesc(userId, pageable);
} }
if (type != null) { return mapWithDocumentTitles(page);
return notificationRepository.findByRecipientIdAndTypeOrderByCreatedAtDesc(userId, type, pageable) }
.map(this::toDTO);
} private Page<NotificationDTO> mapWithDocumentTitles(Page<Notification> page) {
return notificationRepository.findByRecipientIdOrderByCreatedAtDesc(userId, pageable) Set<UUID> documentIds = page.getContent().stream()
.map(this::toDTO); .map(Notification::getDocumentId)
.filter(id -> id != null)
.collect(Collectors.toSet());
Map<UUID, String> titles = documentService.findTitlesByIds(documentIds);
return page.map(n -> toDTO(n, titles));
} }
public long countUnread(UUID userId) { public long countUnread(UUID userId) {
@@ -124,7 +138,7 @@ public class NotificationService {
throw DomainException.forbidden("Notification belongs to a different user"); throw DomainException.forbidden("Notification belongs to a different user");
} }
notification.setRead(true); notification.setRead(true);
return toDTO(notificationRepository.save(notification)); return toDTO(notificationRepository.save(notification), Map.of());
} }
@Transactional @Transactional
@@ -136,10 +150,10 @@ public class NotificationService {
private void saveAndPush(Notification notification) { private void saveAndPush(Notification notification) {
Notification saved = notificationRepository.save(notification); Notification saved = notificationRepository.save(notification);
sseEmitterRegistry.send(saved.getRecipient().getId(), toDTO(saved)); sseEmitterRegistry.send(saved.getRecipient().getId(), toDTO(saved, Map.of()));
} }
private NotificationDTO toDTO(Notification n) { private NotificationDTO toDTO(Notification n, Map<UUID, String> titles) {
return new NotificationDTO( return new NotificationDTO(
n.getId(), n.getId(),
n.getType(), n.getType(),
@@ -148,7 +162,8 @@ public class NotificationService {
n.getAnnotationId(), n.getAnnotationId(),
n.isRead(), n.isRead(),
n.getCreatedAt(), n.getCreatedAt(),
n.getActorName() n.getActorName(),
n.getDocumentId() != null ? titles.get(n.getDocumentId()) : null
); );
} }

View File

@@ -10,6 +10,7 @@ import org.raddatz.familienarchiv.dto.NotificationDTO;
import org.raddatz.familienarchiv.exception.DomainException; import org.raddatz.familienarchiv.exception.DomainException;
import org.raddatz.familienarchiv.model.*; import org.raddatz.familienarchiv.model.*;
import org.raddatz.familienarchiv.repository.NotificationRepository; import org.raddatz.familienarchiv.repository.NotificationRepository;
import org.springframework.data.domain.PageImpl;
import org.springframework.mail.MailException; import org.springframework.mail.MailException;
import org.springframework.mail.MailSendException; import org.springframework.mail.MailSendException;
import org.springframework.mail.SimpleMailMessage; import org.springframework.mail.SimpleMailMessage;
@@ -19,6 +20,7 @@ import org.springframework.data.domain.Page;
import org.springframework.data.domain.Pageable; import org.springframework.data.domain.Pageable;
import java.util.List; import java.util.List;
import java.util.Map;
import java.util.Optional; import java.util.Optional;
import java.util.Set; import java.util.Set;
import java.util.UUID; import java.util.UUID;
@@ -34,6 +36,7 @@ class NotificationServiceTest {
@Mock NotificationRepository notificationRepository; @Mock NotificationRepository notificationRepository;
@Mock UserService userService; @Mock UserService userService;
@Mock DocumentService documentService;
@Mock JavaMailSender mailSender; @Mock JavaMailSender mailSender;
@Mock SseEmitterRegistry sseEmitterRegistry; @Mock SseEmitterRegistry sseEmitterRegistry;
@@ -45,7 +48,7 @@ class NotificationServiceTest {
@BeforeEach @BeforeEach
void setUp() { void setUp() {
notificationService = new NotificationService(notificationRepository, userService, Optional.of(mailSender), sseEmitterRegistry); notificationService = new NotificationService(notificationRepository, userService, documentService, Optional.of(mailSender), sseEmitterRegistry);
userA = AppUser.builder().id(UUID.randomUUID()).username("userA") userA = AppUser.builder().id(UUID.randomUUID()).username("userA")
.firstName("Anna").lastName("Smith").email("a@test.com") .firstName("Anna").lastName("Smith").email("a@test.com")
@@ -258,7 +261,7 @@ class NotificationServiceTest {
@Test @Test
void notifyReply_skipsEmail_whenMailSenderIsAbsent() { void notifyReply_skipsEmail_whenMailSenderIsAbsent() {
NotificationService serviceWithoutMail = new NotificationService( NotificationService serviceWithoutMail = new NotificationService(
notificationRepository, userService, Optional.empty(), sseEmitterRegistry); notificationRepository, userService, documentService, Optional.empty(), sseEmitterRegistry);
userA.setNotifyOnReply(true); userA.setNotifyOnReply(true);
DocumentComment reply = commentWithAuthor(UUID.randomUUID(), null, userC.getId(), "Clara Doe"); DocumentComment reply = commentWithAuthor(UUID.randomUUID(), null, userC.getId(), "Clara Doe");
@@ -274,7 +277,7 @@ class NotificationServiceTest {
@Test @Test
void notifyMentions_skipsEmail_whenMailSenderIsAbsent() { void notifyMentions_skipsEmail_whenMailSenderIsAbsent() {
NotificationService serviceWithoutMail = new NotificationService( NotificationService serviceWithoutMail = new NotificationService(
notificationRepository, userService, Optional.empty(), sseEmitterRegistry); notificationRepository, userService, documentService, Optional.empty(), sseEmitterRegistry);
userA.setNotifyOnMention(true); userA.setNotifyOnMention(true);
DocumentComment comment = commentWithAuthor(UUID.randomUUID(), null, userC.getId(), "Clara Doe"); DocumentComment comment = commentWithAuthor(UUID.randomUUID(), null, userC.getId(), "Clara Doe");
@@ -401,6 +404,63 @@ class NotificationServiceTest {
.findByRecipientIdAndTypeAndReadFalseOrderByCreatedAtDesc(any(), any(), any()); .findByRecipientIdAndTypeAndReadFalseOrderByCreatedAtDesc(any(), any(), any());
} }
@Test
void getNotifications_withReadFalseAndNoType_usesUnreadOnlyRepoMethod() {
when(notificationRepository.findByRecipientIdAndReadFalseOrderByCreatedAtDesc(
eq(userA.getId()), any()))
.thenReturn(Page.empty());
notificationService.getNotifications(userA.getId(), null, false, Pageable.ofSize(10));
verify(notificationRepository).findByRecipientIdAndReadFalseOrderByCreatedAtDesc(
eq(userA.getId()), any());
verify(notificationRepository, never()).findByRecipientIdOrderByCreatedAtDesc(any(), any());
}
@Test
void getNotifications_mapsDocumentTitleFromDocumentService() {
UUID docId = UUID.randomUUID();
Notification notification = Notification.builder()
.id(UUID.randomUUID())
.recipient(userA)
.type(NotificationType.REPLY)
.documentId(docId)
.referenceId(UUID.randomUUID())
.actorName("Clara Doe")
.build();
when(notificationRepository.findByRecipientIdOrderByCreatedAtDesc(eq(userA.getId()), any()))
.thenReturn(new PageImpl<>(List.of(notification)));
when(documentService.findTitlesByIds(Set.of(docId)))
.thenReturn(Map.of(docId, "Geburtsurkunde Opa Karl"));
Page<NotificationDTO> result = notificationService.getNotifications(userA.getId(), null, null, Pageable.ofSize(10));
assertThat(result.getContent()).hasSize(1);
assertThat(result.getContent().getFirst().documentTitle()).isEqualTo("Geburtsurkunde Opa Karl");
}
@Test
void getNotifications_mapsDocumentTitleAsNull_whenDocumentDoesNotExist() {
UUID docId = UUID.randomUUID();
Notification notification = Notification.builder()
.id(UUID.randomUUID())
.recipient(userA)
.type(NotificationType.MENTION)
.documentId(docId)
.referenceId(UUID.randomUUID())
.actorName("Bob Jones")
.build();
when(notificationRepository.findByRecipientIdOrderByCreatedAtDesc(eq(userA.getId()), any()))
.thenReturn(new PageImpl<>(List.of(notification)));
when(documentService.findTitlesByIds(Set.of(docId)))
.thenReturn(Map.of());
Page<NotificationDTO> result = notificationService.getNotifications(userA.getId(), null, null, Pageable.ofSize(10));
assertThat(result.getContent()).hasSize(1);
assertThat(result.getContent().getFirst().documentTitle()).isNull();
}
@Test @Test
void getNotifications_withTypeAndReadTrue_fallsBackToTypeOnlyQuery() { void getNotifications_withTypeAndReadTrue_fallsBackToTypeOnlyQuery() {
// read=true with a type filter falls through to the type-only branch — // read=true with a type filter falls through to the type-only branch —