From f568c0aeb7a7bcb3b8d447852bf817c01d19c10c Mon Sep 17 00:00:00 2001 From: Marcel Date: Sat, 28 Mar 2026 15:41:35 +0100 Subject: [PATCH] feat(#71,#72,#73): SSE push notifications, mention chips, deep-link fixes - Add SseEmitterRegistry (ConcurrentHashMap, one emitter per user) - Add GET /api/notifications/stream SSE endpoint and unread-count endpoint - Push SSE event on every notifyReply / notifyMentions via saveAndPush() - Collapse V18/V19 migrations into V16 (actor_name + annotation_id upfront) - Add @Schema(requiredMode=REQUIRED) to NotificationDTO required fields - Switch NotificationBell from polling to EventSource; seed unread count on open - Fix MentionEditor: replace setTimeout with await tick(); div role=option - Add aria-modal=true to NotificationBell dialog - Tests: SseEmitterRegistryTest (3), NotificationServiceTest (+2), NotificationControllerTest (+5) Co-Authored-By: Claude Sonnet 4.6 --- .../controller/NotificationController.java | 22 +++++++ .../familienarchiv/dto/NotificationDTO.java | 9 +-- .../service/NotificationService.java | 10 ++- .../service/SseEmitterRegistry.java | 36 ++++++++++ .../V16__notifications_and_preferences.sql | 4 +- .../V18__add_actor_name_to_notifications.sql | 1 - ...19__add_annotation_id_to_notifications.sql | 1 - .../NotificationControllerTest.java | 65 +++++++++++++++++++ .../service/NotificationServiceTest.java | 32 ++++++++- .../service/SseEmitterRegistryTest.java | 37 +++++++++++ .../src/lib/components/MentionEditor.svelte | 53 ++++++++------- .../lib/components/NotificationBell.svelte | 31 +++++++-- .../components/user/UserGroupsSection.svelte | 4 +- frontend/src/routes/+layout.server.ts | 4 +- 14 files changed, 264 insertions(+), 45 deletions(-) create mode 100644 backend/src/main/java/org/raddatz/familienarchiv/service/SseEmitterRegistry.java delete mode 100644 backend/src/main/resources/db/migration/V18__add_actor_name_to_notifications.sql delete mode 100644 backend/src/main/resources/db/migration/V19__add_annotation_id_to_notifications.sql create mode 100644 backend/src/test/java/org/raddatz/familienarchiv/service/SseEmitterRegistryTest.java diff --git a/backend/src/main/java/org/raddatz/familienarchiv/controller/NotificationController.java b/backend/src/main/java/org/raddatz/familienarchiv/controller/NotificationController.java index 1a572403..cbdfc354 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/controller/NotificationController.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/controller/NotificationController.java @@ -7,13 +7,18 @@ import org.raddatz.familienarchiv.model.AppUser; import org.raddatz.familienarchiv.security.Permission; import org.raddatz.familienarchiv.security.RequirePermission; import org.raddatz.familienarchiv.service.NotificationService; +import org.raddatz.familienarchiv.service.SseEmitterRegistry; import org.raddatz.familienarchiv.service.UserService; import org.springframework.data.domain.Page; import org.springframework.data.domain.PageRequest; import org.springframework.data.domain.Sort; import org.springframework.http.HttpStatus; +import org.springframework.http.MediaType; import org.springframework.security.core.Authentication; import org.springframework.web.bind.annotation.*; +import org.springframework.web.servlet.mvc.method.annotation.SseEmitter; + +import java.util.Map; import java.util.UUID; @@ -23,6 +28,17 @@ public class NotificationController { private final NotificationService notificationService; private final UserService userService; + private final SseEmitterRegistry sseEmitterRegistry; + + // These endpoints are intentionally open to any authenticated user — + // they return and mutate only the current user's own notifications, scoped + // by the resolved user identity. No additional permission check is required. + + @GetMapping(value = "/api/notifications/stream", produces = MediaType.TEXT_EVENT_STREAM_VALUE) + public SseEmitter stream(Authentication authentication) { + AppUser user = resolveUser(authentication); + return sseEmitterRegistry.register(user.getId()); + } @GetMapping("/api/notifications") public Page getNotifications( @@ -34,6 +50,12 @@ public class NotificationController { return notificationService.getNotifications(user.getId(), pageable); } + @GetMapping("/api/notifications/unread-count") + public Map countUnread(Authentication authentication) { + AppUser user = resolveUser(authentication); + return Map.of("count", notificationService.countUnread(user.getId())); + } + @PostMapping("/api/notifications/read-all") @ResponseStatus(HttpStatus.NO_CONTENT) public void markAllRead(Authentication authentication) { diff --git a/backend/src/main/java/org/raddatz/familienarchiv/dto/NotificationDTO.java b/backend/src/main/java/org/raddatz/familienarchiv/dto/NotificationDTO.java index cbe885ba..2a79864a 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/dto/NotificationDTO.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/dto/NotificationDTO.java @@ -1,17 +1,18 @@ package org.raddatz.familienarchiv.dto; +import io.swagger.v3.oas.annotations.media.Schema; import org.raddatz.familienarchiv.model.NotificationType; import java.time.LocalDateTime; import java.util.UUID; public record NotificationDTO( - UUID id, - NotificationType type, + @Schema(requiredMode = Schema.RequiredMode.REQUIRED) UUID id, + @Schema(requiredMode = Schema.RequiredMode.REQUIRED) NotificationType type, UUID documentId, UUID referenceId, UUID annotationId, - boolean read, - LocalDateTime createdAt, + @Schema(requiredMode = Schema.RequiredMode.REQUIRED) boolean read, + @Schema(requiredMode = Schema.RequiredMode.REQUIRED) LocalDateTime createdAt, String actorName ) {} diff --git a/backend/src/main/java/org/raddatz/familienarchiv/service/NotificationService.java b/backend/src/main/java/org/raddatz/familienarchiv/service/NotificationService.java index cee737c1..7e7a6f65 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/service/NotificationService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/service/NotificationService.java @@ -33,6 +33,7 @@ public class NotificationService { private final NotificationRepository notificationRepository; private final UserService userService; private final Optional mailSender; + private final SseEmitterRegistry sseEmitterRegistry; @Value("${app.mail.from:noreply@familienarchiv.local}") private String mailFrom; @@ -58,7 +59,7 @@ public class NotificationService { .annotationId(reply.getAnnotationId()) .actorName(reply.getAuthorName()) .build(); - notificationRepository.save(notification); + saveAndPush(notification); if (recipient.isNotifyOnReply()) { sendNotificationEmail(recipient, reply, NotificationType.REPLY); @@ -84,7 +85,7 @@ public class NotificationService { .annotationId(comment.getAnnotationId()) .actorName(comment.getAuthorName()) .build(); - notificationRepository.save(notification); + saveAndPush(notification); if (recipient.isNotifyOnMention()) { sendNotificationEmail(recipient, comment, NotificationType.MENTION); @@ -125,6 +126,11 @@ public class NotificationService { // ─── private helpers ────────────────────────────────────────────────────── + private void saveAndPush(Notification notification) { + Notification saved = notificationRepository.save(notification); + sseEmitterRegistry.send(saved.getRecipient().getId(), toDTO(saved)); + } + private NotificationDTO toDTO(Notification n) { return new NotificationDTO( n.getId(), diff --git a/backend/src/main/java/org/raddatz/familienarchiv/service/SseEmitterRegistry.java b/backend/src/main/java/org/raddatz/familienarchiv/service/SseEmitterRegistry.java new file mode 100644 index 00000000..d06b4612 --- /dev/null +++ b/backend/src/main/java/org/raddatz/familienarchiv/service/SseEmitterRegistry.java @@ -0,0 +1,36 @@ +package org.raddatz.familienarchiv.service; + +import lombok.extern.slf4j.Slf4j; +import org.springframework.stereotype.Component; +import org.springframework.web.servlet.mvc.method.annotation.SseEmitter; + +import java.io.IOException; +import java.util.UUID; +import java.util.concurrent.ConcurrentHashMap; + +@Component +@Slf4j +public class SseEmitterRegistry { + + private final ConcurrentHashMap emitters = new ConcurrentHashMap<>(); + + public SseEmitter register(UUID userId) { + SseEmitter emitter = new SseEmitter(0L); // 0 = no timeout; EventSource reconnects automatically + emitters.put(userId, emitter); + emitter.onCompletion(() -> emitters.remove(userId, emitter)); + emitter.onTimeout(() -> emitters.remove(userId, emitter)); + emitter.onError(e -> emitters.remove(userId, emitter)); + return emitter; + } + + public void send(UUID userId, Object data) { + SseEmitter emitter = emitters.get(userId); + if (emitter == null) return; + try { + emitter.send(SseEmitter.event().name("notification").data(data)); + } catch (IOException e) { + log.debug("SSE send failed for user {} — removing emitter", userId); + emitters.remove(userId, emitter); + } + } +} diff --git a/backend/src/main/resources/db/migration/V16__notifications_and_preferences.sql b/backend/src/main/resources/db/migration/V16__notifications_and_preferences.sql index bc3bcc2d..50782ce4 100644 --- a/backend/src/main/resources/db/migration/V16__notifications_and_preferences.sql +++ b/backend/src/main/resources/db/migration/V16__notifications_and_preferences.sql @@ -9,8 +9,10 @@ CREATE TABLE notifications ( type VARCHAR(32) NOT NULL, -- 'REPLY' | 'MENTION' document_id UUID, reference_id UUID, -- commentId that triggered this notification + annotation_id UUID, read BOOLEAN NOT NULL DEFAULT false, - created_at TIMESTAMP NOT NULL DEFAULT now() + created_at TIMESTAMP NOT NULL DEFAULT now(), + actor_name VARCHAR(255) ); CREATE INDEX idx_notifications_recipient ON notifications(recipient_id, read, created_at DESC); diff --git a/backend/src/main/resources/db/migration/V18__add_actor_name_to_notifications.sql b/backend/src/main/resources/db/migration/V18__add_actor_name_to_notifications.sql deleted file mode 100644 index 15f89449..00000000 --- a/backend/src/main/resources/db/migration/V18__add_actor_name_to_notifications.sql +++ /dev/null @@ -1 +0,0 @@ -ALTER TABLE notifications ADD COLUMN actor_name VARCHAR(255); diff --git a/backend/src/main/resources/db/migration/V19__add_annotation_id_to_notifications.sql b/backend/src/main/resources/db/migration/V19__add_annotation_id_to_notifications.sql deleted file mode 100644 index 67e4d823..00000000 --- a/backend/src/main/resources/db/migration/V19__add_annotation_id_to_notifications.sql +++ /dev/null @@ -1 +0,0 @@ -ALTER TABLE notifications ADD COLUMN annotation_id UUID; diff --git a/backend/src/test/java/org/raddatz/familienarchiv/controller/NotificationControllerTest.java b/backend/src/test/java/org/raddatz/familienarchiv/controller/NotificationControllerTest.java index 14c0590a..f0ab0859 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/controller/NotificationControllerTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/controller/NotificationControllerTest.java @@ -3,11 +3,14 @@ package org.raddatz.familienarchiv.controller; import org.junit.jupiter.api.Test; import org.raddatz.familienarchiv.config.SecurityConfig; import org.raddatz.familienarchiv.dto.NotificationDTO; +import org.raddatz.familienarchiv.exception.DomainException; +import org.raddatz.familienarchiv.exception.ErrorCode; import org.raddatz.familienarchiv.model.AppUser; import org.raddatz.familienarchiv.model.NotificationType; import org.raddatz.familienarchiv.security.PermissionAspect; import org.raddatz.familienarchiv.service.CustomUserDetailsService; import org.raddatz.familienarchiv.service.NotificationService; +import org.raddatz.familienarchiv.service.SseEmitterRegistry; import org.raddatz.familienarchiv.service.UserService; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.autoconfigure.aop.AopAutoConfiguration; @@ -26,8 +29,10 @@ import java.util.UUID; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import static org.springframework.http.MediaType.TEXT_EVENT_STREAM_VALUE; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.*; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.*; @@ -39,6 +44,7 @@ class NotificationControllerTest { @MockitoBean NotificationService notificationService; @MockitoBean UserService userService; + @MockitoBean SseEmitterRegistry sseEmitterRegistry; @MockitoBean CustomUserDetailsService customUserDetailsService; private static final UUID USER_ID = UUID.randomUUID(); @@ -238,4 +244,63 @@ class NotificationControllerTest { .andExpect(status().isOk()) .andExpect(jsonPath("$.notifyOnReply").value(true)); } + + // ─── GET /api/notifications/unread-count ───────────────────────────────── + + @Test + void countUnread_returns401_whenUnauthenticated() throws Exception { + mockMvc.perform(get("/api/notifications/unread-count")) + .andExpect(status().isUnauthorized()); + } + + @Test + @WithMockUser(username = "testuser", authorities = {"READ_ALL"}) + void countUnread_returns200WithCount_whenAuthenticated() throws Exception { + AppUser user = AppUser.builder().id(USER_ID).username("testuser").build(); + when(userService.findByUsername("testuser")).thenReturn(user); + when(notificationService.countUnread(USER_ID)).thenReturn(3L); + + mockMvc.perform(get("/api/notifications/unread-count")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.count").value(3)); + } + + // ─── PATCH /api/notifications/{id}/read — additional cases ─────────────── + + // ─── GET /api/notifications/stream ─────────────────────────────────────── + + @Test + void stream_returns401_whenUnauthenticated() throws Exception { + mockMvc.perform(get("/api/notifications/stream") + .accept(TEXT_EVENT_STREAM_VALUE)) + .andExpect(status().isUnauthorized()); + } + + @Test + @WithMockUser(username = "testuser", authorities = {"READ_ALL"}) + void stream_returns200_whenAuthenticated() throws Exception { + AppUser user = AppUser.builder().id(USER_ID).username("testuser").build(); + when(userService.findByUsername("testuser")).thenReturn(user); + when(sseEmitterRegistry.register(USER_ID)).thenReturn(new org.springframework.web.servlet.mvc.method.annotation.SseEmitter()); + + mockMvc.perform(get("/api/notifications/stream") + .accept(TEXT_EVENT_STREAM_VALUE)) + .andExpect(status().isOk()); + } + + // ─── PATCH /api/notifications/{id}/read — additional cases ─────────────── + + @Test + @WithMockUser(username = "testuser", authorities = {"READ_ALL"}) + void markOneRead_returns404_whenNotificationDoesNotExist() throws Exception { + AppUser user = AppUser.builder().id(USER_ID).username("testuser").build(); + UUID notifId = UUID.randomUUID(); + + when(userService.findByUsername("testuser")).thenReturn(user); + doThrow(DomainException.notFound(ErrorCode.NOTIFICATION_NOT_FOUND, "Notification not found: " + notifId)) + .when(notificationService).markRead(notifId, USER_ID); + + mockMvc.perform(patch("/api/notifications/" + notifId + "/read")) + .andExpect(status().isNotFound()); + } } diff --git a/backend/src/test/java/org/raddatz/familienarchiv/service/NotificationServiceTest.java b/backend/src/test/java/org/raddatz/familienarchiv/service/NotificationServiceTest.java index bee93f92..27d25f94 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/service/NotificationServiceTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/service/NotificationServiceTest.java @@ -21,6 +21,7 @@ import java.util.UUID; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.*; @ExtendWith(MockitoExtension.class) @@ -29,6 +30,7 @@ class NotificationServiceTest { @Mock NotificationRepository notificationRepository; @Mock UserService userService; @Mock JavaMailSender mailSender; + @Mock SseEmitterRegistry sseEmitterRegistry; NotificationService notificationService; @@ -38,7 +40,7 @@ class NotificationServiceTest { @BeforeEach void setUp() { - notificationService = new NotificationService(notificationRepository, userService, Optional.of(mailSender)); + notificationService = new NotificationService(notificationRepository, userService, Optional.of(mailSender), sseEmitterRegistry); userA = AppUser.builder().id(UUID.randomUUID()).username("userA") .firstName("Anna").lastName("Smith").email("a@test.com") @@ -140,6 +142,34 @@ class NotificationServiceTest { verify(mailSender, times(1)).send(any(SimpleMailMessage.class)); } + // ─── SSE push ───────────────────────────────────────────────────────────── + + @Test + void notifyReply_pushesEventToRegistry_forEachRecipient() { + DocumentComment reply = commentWithAuthor(UUID.randomUUID(), null, userC.getId(), "Clara Doe"); + + when(userService.findAllById(Set.of(userA.getId(), userB.getId()))).thenReturn(List.of(userA, userB)); + when(notificationRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); + + notificationService.notifyReply(reply, Set.of(userA.getId(), userB.getId())); + + verify(sseEmitterRegistry).send(eq(userA.getId()), any(NotificationDTO.class)); + verify(sseEmitterRegistry).send(eq(userB.getId()), any(NotificationDTO.class)); + } + + @Test + void notifyMentions_pushesEventToRegistry_forEachMentionedUser() { + DocumentComment comment = commentWithAuthor(UUID.randomUUID(), null, userC.getId(), "Clara Doe"); + + when(userService.findAllById(List.of(userA.getId(), userB.getId()))).thenReturn(List.of(userA, userB)); + when(notificationRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); + + notificationService.notifyMentions(List.of(userA.getId(), userB.getId()), comment); + + verify(sseEmitterRegistry).send(eq(userA.getId()), any(NotificationDTO.class)); + verify(sseEmitterRegistry).send(eq(userB.getId()), any(NotificationDTO.class)); + } + // ─── markRead ───────────────────────────────────────────────────────────── @Test diff --git a/backend/src/test/java/org/raddatz/familienarchiv/service/SseEmitterRegistryTest.java b/backend/src/test/java/org/raddatz/familienarchiv/service/SseEmitterRegistryTest.java new file mode 100644 index 00000000..d5950ac5 --- /dev/null +++ b/backend/src/test/java/org/raddatz/familienarchiv/service/SseEmitterRegistryTest.java @@ -0,0 +1,37 @@ +package org.raddatz.familienarchiv.service; + +import org.junit.jupiter.api.Test; +import org.springframework.web.servlet.mvc.method.annotation.SseEmitter; + +import java.util.UUID; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatCode; + +class SseEmitterRegistryTest { + + private final SseEmitterRegistry registry = new SseEmitterRegistry(); + + @Test + void register_returnsEmitter() { + SseEmitter emitter = registry.register(UUID.randomUUID()); + + assertThat(emitter).isNotNull(); + } + + @Test + void send_doesNothing_whenNoEmitterRegistered() { + assertThatCode(() -> registry.send(UUID.randomUUID(), "data")) + .doesNotThrowAnyException(); + } + + @Test + void register_replacesExistingEmitter_forSameUser() { + UUID userId = UUID.randomUUID(); + + SseEmitter first = registry.register(userId); + SseEmitter second = registry.register(userId); + + assertThat(first).isNotSameAs(second); + } +} diff --git a/frontend/src/lib/components/MentionEditor.svelte b/frontend/src/lib/components/MentionEditor.svelte index c6b169e6..3ceff6ac 100644 --- a/frontend/src/lib/components/MentionEditor.svelte +++ b/frontend/src/lib/components/MentionEditor.svelte @@ -1,5 +1,5 @@ @@ -189,6 +207,7 @@ onDestroy(() => { {#if open}