diff --git a/ontime-back/src/main/java/devkor/ontime_back/config/SchedulerConfig.java b/ontime-back/src/main/java/devkor/ontime_back/config/SchedulerConfig.java index 924d6d1..dbe71fc 100644 --- a/ontime-back/src/main/java/devkor/ontime_back/config/SchedulerConfig.java +++ b/ontime-back/src/main/java/devkor/ontime_back/config/SchedulerConfig.java @@ -3,8 +3,11 @@ import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.scheduling.TaskScheduler; +import org.springframework.scheduling.concurrent.ThreadPoolTaskExecutor; import org.springframework.scheduling.concurrent.ThreadPoolTaskScheduler; +import java.util.concurrent.Executor; + @Configuration public class SchedulerConfig { @@ -16,4 +19,15 @@ public TaskScheduler taskScheduler() { scheduler.initialize(); return scheduler; } -} \ No newline at end of file + + @Bean(name = "notificationAsyncExecutor") + public Executor notificationAsyncExecutor() { + ThreadPoolTaskExecutor executor = new ThreadPoolTaskExecutor(); + executor.setCorePoolSize(4); + executor.setMaxPoolSize(8); + executor.setQueueCapacity(200); + executor.setThreadNamePrefix("notification-"); + executor.initialize(); + return executor; + } +} diff --git a/ontime-back/src/main/java/devkor/ontime_back/repository/NotificationScheduleRepository.java b/ontime-back/src/main/java/devkor/ontime_back/repository/NotificationScheduleRepository.java index 26885bf..716ade0 100644 --- a/ontime-back/src/main/java/devkor/ontime_back/repository/NotificationScheduleRepository.java +++ b/ontime-back/src/main/java/devkor/ontime_back/repository/NotificationScheduleRepository.java @@ -3,10 +3,12 @@ import devkor.ontime_back.entity.NotificationSchedule; import org.springframework.data.jpa.repository.JpaRepository; import org.springframework.data.jpa.repository.Query; +import org.springframework.data.repository.query.Param; import org.springframework.stereotype.Repository; import java.time.LocalDateTime; import java.util.List; +import java.util.Optional; import java.util.UUID; @Repository @@ -17,5 +19,11 @@ public interface NotificationScheduleRepository extends JpaRepository :now AND n.isSent = false") List findAllWithScheduleAndUser(LocalDateTime now); + @Query("SELECT n FROM NotificationSchedule n " + + "JOIN FETCH n.schedule s " + + "JOIN FETCH s.user " + + "WHERE n.id = :notificationId") + Optional findByIdWithScheduleAndUser(@Param("notificationId") Long notificationId); + List findAllByScheduleScheduleIdOrderByIdAsc(UUID scheduleId); } diff --git a/ontime-back/src/main/java/devkor/ontime_back/service/NotificationDeliveryService.java b/ontime-back/src/main/java/devkor/ontime_back/service/NotificationDeliveryService.java new file mode 100644 index 0000000..dc5f467 --- /dev/null +++ b/ontime-back/src/main/java/devkor/ontime_back/service/NotificationDeliveryService.java @@ -0,0 +1,86 @@ +package devkor.ontime_back.service; + +import com.google.firebase.messaging.FirebaseMessaging; +import com.google.firebase.messaging.Message; +import devkor.ontime_back.entity.NotificationSchedule; +import devkor.ontime_back.entity.Schedule; +import devkor.ontime_back.entity.User; +import devkor.ontime_back.entity.UserSetting; +import devkor.ontime_back.repository.NotificationScheduleRepository; +import devkor.ontime_back.repository.UserSettingRepository; +import lombok.RequiredArgsConstructor; +import lombok.extern.slf4j.Slf4j; +import org.springframework.stereotype.Service; +import org.springframework.transaction.annotation.Transactional; + +import java.util.List; + +@Slf4j +@Service +@RequiredArgsConstructor +@Transactional(readOnly = true) +public class NotificationDeliveryService { + + private final UserSettingRepository userSettingRepository; + private final AlarmService alarmService; + private final NotificationScheduleRepository notificationScheduleRepository; + + @Transactional + public void sendReminder(NotificationSchedule notificationSchedule, String message) { + Long userId = notificationSchedule.getSchedule().getUser().getId(); + + if (userId != null) { + UserSetting userSetting = userSettingRepository.findByUserId(userId) + .orElseThrow(() -> new IllegalArgumentException("No UserSetting found in schedule's user")); + log.debug("사용자 알림 전송 설정 여부: " + userSetting.getIsNotificationsEnabled()); + + if (Boolean.TRUE.equals(userSetting.getIsNotificationsEnabled())) { + if (alarmService.shouldSuppressLegacyReminder( + userId, + notificationSchedule.getSchedule().getScheduleId(), + notificationSchedule.getNotificationTime())) { + log.info("현재 기기 로컬 알람 커버리지로 인해 레거시 푸시 알림을 생략합니다. scheduleId={}", + notificationSchedule.getSchedule().getScheduleId()); + return; + } + sendNotificationToUser(notificationSchedule.getSchedule(), message); + notificationSchedule.changeStatusToSent(); + notificationScheduleRepository.save(notificationSchedule); + } + } + } + + public void sendReminder(List schedules, String message) { + for (Schedule schedule : schedules) { + User user = schedule.getUser(); + Long userId = user.getId(); + + if (userId != null) { + UserSetting userSetting = userSettingRepository.findByUserId(userId) + .orElseThrow(() -> new IllegalArgumentException("No UserSetting found in schedule's user")); + + if (userSetting != null && userSetting.getIsNotificationsEnabled()) { + sendNotificationToUser(schedule, message); + } + } + } + } + + public void sendNotificationToUser(Schedule schedule, String message) { + User user = schedule.getUser(); + String firebaseToken = user.getFirebaseToken(); + + Message firebaseMessage = Message.builder() + .putData("title", "약속 알림") + .putData("content", user.getName() + "님 " + message + "\n약속명: " + schedule.getScheduleName()) + .setToken(firebaseToken) + .build(); + + try { + FirebaseMessaging.getInstance().send(firebaseMessage); + log.info("Firebase에 성공적으로 push notification 요청을 보냈으며, Firebase로부터 적절한 응답을 받았습니다 \n알림 푸시한 약속:" + schedule.getScheduleName()); + } catch (Exception e) { + e.printStackTrace(); + } + } +} diff --git a/ontime-back/src/main/java/devkor/ontime_back/service/NotificationDispatchService.java b/ontime-back/src/main/java/devkor/ontime_back/service/NotificationDispatchService.java new file mode 100644 index 0000000..4f8aa0e --- /dev/null +++ b/ontime-back/src/main/java/devkor/ontime_back/service/NotificationDispatchService.java @@ -0,0 +1,31 @@ +package devkor.ontime_back.service; + +import devkor.ontime_back.entity.NotificationSchedule; +import devkor.ontime_back.repository.NotificationScheduleRepository; +import lombok.RequiredArgsConstructor; +import lombok.extern.slf4j.Slf4j; +import org.springframework.scheduling.annotation.Async; +import org.springframework.stereotype.Service; +import org.springframework.transaction.annotation.Transactional; + +@Slf4j +@Service +@RequiredArgsConstructor +public class NotificationDispatchService { + + private final NotificationScheduleRepository notificationScheduleRepository; + private final NotificationDeliveryService notificationDeliveryService; + + @Async("notificationAsyncExecutor") + @Transactional + public void dispatchReminder(Long notificationId, String message) { + NotificationSchedule notificationSchedule = notificationScheduleRepository.findByIdWithScheduleAndUser(notificationId) + .orElse(null); + if (notificationSchedule == null) { + log.warn("예약된 알림을 찾을 수 없어 푸시 알림을 건너뜁니다. notificationId={}", notificationId); + return; + } + + notificationDeliveryService.sendReminder(notificationSchedule, message); + } +} diff --git a/ontime-back/src/main/java/devkor/ontime_back/service/NotificationService.java b/ontime-back/src/main/java/devkor/ontime_back/service/NotificationService.java index 442ae98..7f0cadf 100644 --- a/ontime-back/src/main/java/devkor/ontime_back/service/NotificationService.java +++ b/ontime-back/src/main/java/devkor/ontime_back/service/NotificationService.java @@ -1,20 +1,11 @@ package devkor.ontime_back.service; -import com.google.firebase.messaging.FirebaseMessaging; -import com.google.firebase.messaging.Message; import devkor.ontime_back.entity.NotificationSchedule; import devkor.ontime_back.entity.Schedule; -import devkor.ontime_back.entity.User; -import devkor.ontime_back.entity.UserSetting; -import devkor.ontime_back.repository.NotificationScheduleRepository; -import devkor.ontime_back.repository.UserSettingRepository; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; import org.springframework.scheduling.TaskScheduler; -import org.springframework.scheduling.annotation.Async; -import org.springframework.scheduling.annotation.EnableAsync; import org.springframework.stereotype.Service; -import org.springframework.transaction.annotation.Transactional; import java.time.LocalDateTime; import java.time.ZoneId; @@ -26,13 +17,11 @@ @Slf4j @Service @RequiredArgsConstructor -@Transactional(readOnly = true) public class NotificationService { - private final UserSettingRepository userSettingRepository; - private final AlarmService alarmService; private final TaskScheduler taskScheduler; - private final NotificationScheduleRepository notificationScheduleRepository; + private final NotificationDispatchService notificationDispatchService; + private final NotificationDeliveryService notificationDeliveryService; private final ConcurrentHashMap> scheduledTasks = new ConcurrentHashMap<>(); public void scheduleReminder(NotificationSchedule notificationSchedule) { @@ -43,12 +32,19 @@ public void scheduleReminder(NotificationSchedule notificationSchedule) { return; } + Long notificationId = notificationSchedule.getId(); + if (notificationId == null) { + throw new IllegalArgumentException("NotificationSchedule must be persisted before scheduling"); + } + ScheduledFuture future = taskScheduler.schedule( - () -> sendReminder(notificationSchedule, "준비 시작해야 합니다.(현재 시각: 약속시각 - (여유시간 + 이동시간 + 총준비시간) )"), + () -> notificationDispatchService.dispatchReminder( + notificationId, + "준비 시작해야 합니다.(현재 시각: 약속시각 - (여유시간 + 이동시간 + 총준비시간) )"), Date.from(reminderTime.atZone(ZoneId.systemDefault()).toInstant()) ); - scheduledTasks.put(notificationSchedule.getId(), future); + scheduledTasks.put(notificationId, future); log.info("스케줄 등록 완료 {} ({})", notificationSchedule.getSchedule().getScheduleName(), reminderTime); } @@ -62,64 +58,15 @@ public void cancelScheduledNotification(Long notificationId) { } } - @Async - @Transactional public void sendReminder(NotificationSchedule notificationSchedule, String message) { - Long userId = notificationSchedule.getSchedule().getUser().getId(); - - if (userId != null) { - UserSetting userSetting = userSettingRepository.findByUserId(userId) - .orElseThrow(() -> new IllegalArgumentException("No UserSetting found in schedule's user")); - log.debug("사용자 알림 전송 설정 여부: " + userSetting.getIsNotificationsEnabled()); - - if (Boolean.TRUE.equals(userSetting.getIsNotificationsEnabled())) { - if (alarmService.shouldSuppressLegacyReminder( - userId, - notificationSchedule.getSchedule().getScheduleId(), - notificationSchedule.getNotificationTime())) { - log.info("현재 기기 로컬 알람 커버리지로 인해 레거시 푸시 알림을 생략합니다. scheduleId={}", - notificationSchedule.getSchedule().getScheduleId()); - return; - } - sendNotificationToUser(notificationSchedule.getSchedule(), message); - notificationSchedule.changeStatusToSent(); - notificationScheduleRepository.save(notificationSchedule); - } - } + notificationDeliveryService.sendReminder(notificationSchedule, message); } public void sendReminder(List schedules, String message) { - for (Schedule schedule : schedules) { - User user = schedule.getUser(); - Long userId = user.getId(); - - if (userId != null) { - UserSetting userSetting = userSettingRepository.findByUserId(userId) - .orElseThrow(() -> new IllegalArgumentException("No UserSetting found in schedule's user"));// Repository 메서드 가정 - - if (userSetting != null && userSetting.getIsNotificationsEnabled()) { - sendNotificationToUser(schedule, message); - } - } - } + notificationDeliveryService.sendReminder(schedules, message); } - @Transactional public void sendNotificationToUser(Schedule schedule, String message) { - User user = schedule.getUser(); - String firebaseToken = user.getFirebaseToken(); - - Message firebaseMessage = Message.builder() - .putData("title", "약속 알림") - .putData("content", user.getName() + "님 " + message + "\n약속명: " + schedule.getScheduleName()) - .setToken(firebaseToken) - .build(); - - try { - FirebaseMessaging.getInstance().send(firebaseMessage); - log.info("Firebase에 성공적으로 push notification 요청을 보냈으며, Firebase로부터 적절한 응답을 받았습니다 \n알림 푸시한 약속:" + schedule.getScheduleName()); - } catch (Exception e) { - e.printStackTrace(); - } + notificationDeliveryService.sendNotificationToUser(schedule, message); } } diff --git a/ontime-back/src/test/java/devkor/ontime_back/service/NotificationDeliveryServiceTest.java b/ontime-back/src/test/java/devkor/ontime_back/service/NotificationDeliveryServiceTest.java new file mode 100644 index 0000000..e73b255 --- /dev/null +++ b/ontime-back/src/test/java/devkor/ontime_back/service/NotificationDeliveryServiceTest.java @@ -0,0 +1,208 @@ +package devkor.ontime_back.service; + +import devkor.ontime_back.entity.NotificationSchedule; +import devkor.ontime_back.entity.Role; +import devkor.ontime_back.entity.Schedule; +import devkor.ontime_back.entity.User; +import devkor.ontime_back.entity.UserSetting; +import devkor.ontime_back.repository.NotificationScheduleRepository; +import devkor.ontime_back.repository.UserSettingRepository; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +import java.time.LocalDateTime; +import java.util.List; +import java.util.Optional; +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.doNothing; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; +import static org.mockito.Mockito.when; + +@ExtendWith(MockitoExtension.class) +class NotificationDeliveryServiceTest { + + @Mock + private UserSettingRepository userSettingRepository; + + @Mock + private AlarmService alarmService; + + @Mock + private NotificationScheduleRepository notificationScheduleRepository; + + private NotificationDeliveryService notificationDeliveryService; + + @BeforeEach + void setUp() { + notificationDeliveryService = new NotificationDeliveryService( + userSettingRepository, + alarmService, + notificationScheduleRepository + ); + } + + @Test + void sendReminderDoesNothingWhenUserIdIsMissing() { + NotificationSchedule notification = NotificationSchedule.builder() + .notificationTime(LocalDateTime.now()) + .isSent(false) + .schedule(Schedule.builder() + .scheduleId(UUID.randomUUID()) + .scheduleName("No owner") + .user(User.builder().build()) + .build()) + .build(); + + notificationDeliveryService.sendReminder(notification, "message"); + + verifyNoInteractions(userSettingRepository, alarmService, notificationScheduleRepository); + assertThat(notification.getIsSent()).isFalse(); + } + + @Test + void sendReminderDoesNotSendWhenUserDisabledNotifications() { + NotificationSchedule notification = notificationSchedule(LocalDateTime.now()); + when(userSettingRepository.findByUserId(1L)).thenReturn(Optional.of(userSetting(false))); + + notificationDeliveryService.sendReminder(notification, "message"); + + verifyNoInteractions(alarmService, notificationScheduleRepository); + assertThat(notification.getIsSent()).isFalse(); + } + + @Test + void sendReminderDoesNotSendWhenNativeAlarmAlreadyCoversSchedule() { + NotificationSchedule notification = notificationSchedule(LocalDateTime.now()); + UUID scheduleId = notification.getSchedule().getScheduleId(); + when(userSettingRepository.findByUserId(1L)).thenReturn(Optional.of(userSetting(true))); + when(alarmService.shouldSuppressLegacyReminder(1L, scheduleId, notification.getNotificationTime())) + .thenReturn(true); + + notificationDeliveryService.sendReminder(notification, "message"); + + verify(notificationScheduleRepository, never()).save(notification); + assertThat(notification.getIsSent()).isFalse(); + } + + @Test + void sendReminderMarksNotificationSentWhenUserEnabledNotificationsAndNativeAlarmDoesNotCoverIt() { + NotificationSchedule notification = notificationSchedule(LocalDateTime.now()); + UUID scheduleId = notification.getSchedule().getScheduleId(); + NotificationDeliveryService spyService = spy(notificationDeliveryService); + doNothing().when(spyService).sendNotificationToUser(notification.getSchedule(), "message"); + when(userSettingRepository.findByUserId(1L)).thenReturn(Optional.of(userSetting(true))); + when(alarmService.shouldSuppressLegacyReminder(1L, scheduleId, notification.getNotificationTime())) + .thenReturn(false); + + spyService.sendReminder(notification, "message"); + + verify(spyService).sendNotificationToUser(notification.getSchedule(), "message"); + verify(notificationScheduleRepository).save(notification); + assertThat(notification.getIsSent()).isTrue(); + } + + @Test + void sendReminderFailsClearlyWhenUserSettingsAreMissing() { + NotificationSchedule notification = notificationSchedule(LocalDateTime.now()); + when(userSettingRepository.findByUserId(1L)).thenReturn(Optional.empty()); + + assertThatThrownBy(() -> notificationDeliveryService.sendReminder(notification, "message")) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("No UserSetting found in schedule's user"); + } + + @Test + void listReminderSkipsSchedulesWithoutPersistedUsers() { + Schedule schedule = Schedule.builder() + .scheduleId(UUID.randomUUID()) + .scheduleName("Unsaved user") + .user(User.builder().build()) + .build(); + + notificationDeliveryService.sendReminder(List.of(schedule), "message"); + + verifyNoInteractions(userSettingRepository, alarmService, notificationScheduleRepository); + } + + @Test + void listReminderSendsOnlyForSchedulesWhoseUsersAllowNotifications() { + Schedule enabledSchedule = scheduleForListReminder(1L, "Enabled"); + Schedule disabledSchedule = scheduleForListReminder(2L, "Disabled"); + NotificationDeliveryService spyService = spy(notificationDeliveryService); + doNothing().when(spyService).sendNotificationToUser(any(Schedule.class), eq("message")); + when(userSettingRepository.findByUserId(1L)).thenReturn(Optional.of(userSetting(true))); + when(userSettingRepository.findByUserId(2L)).thenReturn(Optional.of(userSetting(false))); + + spyService.sendReminder(List.of(enabledSchedule, disabledSchedule), "message"); + + verify(spyService).sendNotificationToUser(enabledSchedule, "message"); + verify(spyService, never()).sendNotificationToUser(disabledSchedule, "message"); + } + + @Test + void listReminderFailsClearlyWhenPersistedUserHasNoSettings() { + Schedule schedule = scheduleForListReminder(1L, "Missing setting"); + when(userSettingRepository.findByUserId(1L)).thenReturn(Optional.empty()); + + assertThatThrownBy(() -> notificationDeliveryService.sendReminder(List.of(schedule), "message")) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("No UserSetting found in schedule's user"); + } + + @Test + void sendNotificationToUserDoesNotPropagateFirebaseClientFailures() { + Schedule schedule = scheduleForListReminder(1L, "Firebase smoke test"); + + notificationDeliveryService.sendNotificationToUser(schedule, "message"); + + assertThat(schedule.getUser().getFirebaseToken()).isEqualTo("firebase-token-1"); + } + + private NotificationSchedule notificationSchedule(LocalDateTime notificationTime) { + return NotificationSchedule.builder() + .notificationTime(notificationTime) + .isSent(false) + .schedule(Schedule.builder() + .scheduleId(UUID.randomUUID()) + .scheduleName("Morning meeting") + .user(User.builder() + .id(1L) + .name("User") + .firebaseToken("firebase-token") + .role(Role.USER) + .build()) + .build()) + .build(); + } + + private UserSetting userSetting(boolean notificationsEnabled) { + return UserSetting.builder() + .userSettingId(UUID.randomUUID()) + .isNotificationsEnabled(notificationsEnabled) + .build(); + } + + private Schedule scheduleForListReminder(Long userId, String scheduleName) { + return Schedule.builder() + .scheduleId(UUID.randomUUID()) + .scheduleName(scheduleName) + .user(User.builder() + .id(userId) + .name("User " + userId) + .firebaseToken("firebase-token-" + userId) + .role(Role.USER) + .build()) + .build(); + } +} diff --git a/ontime-back/src/test/java/devkor/ontime_back/service/NotificationDispatchServiceAsyncIntegrationTest.java b/ontime-back/src/test/java/devkor/ontime_back/service/NotificationDispatchServiceAsyncIntegrationTest.java new file mode 100644 index 0000000..1906fc9 --- /dev/null +++ b/ontime-back/src/test/java/devkor/ontime_back/service/NotificationDispatchServiceAsyncIntegrationTest.java @@ -0,0 +1,146 @@ +package devkor.ontime_back.service; + +import devkor.ontime_back.config.SchedulerConfig; +import devkor.ontime_back.entity.NotificationSchedule; +import devkor.ontime_back.entity.Schedule; +import devkor.ontime_back.entity.User; +import devkor.ontime_back.repository.NotificationScheduleRepository; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.Import; +import org.springframework.scheduling.annotation.EnableAsync; +import org.springframework.test.context.junit.jupiter.SpringJUnitConfig; +import org.springframework.transaction.TransactionDefinition; +import org.springframework.transaction.annotation.EnableTransactionManagement; +import org.springframework.transaction.support.AbstractPlatformTransactionManager; +import org.springframework.transaction.support.DefaultTransactionStatus; +import org.springframework.transaction.support.TransactionSynchronizationManager; + +import java.time.Duration; +import java.time.LocalDateTime; +import java.util.Optional; +import java.util.UUID; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicReference; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.reset; +import static org.mockito.Mockito.when; + +@SpringJUnitConfig(NotificationDispatchServiceAsyncIntegrationTest.Config.class) +class NotificationDispatchServiceAsyncIntegrationTest { + private static final long BLOCKING_DELIVERY_MS = 300; + private static final long MAX_DISPATCH_RETURN_MS = 100; + + @Autowired + private NotificationDispatchService notificationDispatchService; + + @Autowired + private NotificationScheduleRepository notificationScheduleRepository; + + @Autowired + private NotificationDeliveryService notificationDeliveryService; + + @BeforeEach + void setUp() { + reset(notificationScheduleRepository, notificationDeliveryService); + } + + @Test + void dispatchReminderReturnsQuicklyWhileDeliveryRunsOnNotificationExecutorInTransaction() throws Exception { + NotificationSchedule notification = notificationSchedule(); + CountDownLatch deliveryStarted = new CountDownLatch(1); + CountDownLatch deliveryFinished = new CountDownLatch(1); + AtomicReference deliveryThreadName = new AtomicReference<>(); + AtomicBoolean transactionActive = new AtomicBoolean(false); + + when(notificationScheduleRepository.findByIdWithScheduleAndUser(10L)) + .thenReturn(Optional.of(notification)); + doAnswer(invocation -> { + deliveryThreadName.set(Thread.currentThread().getName()); + transactionActive.set(TransactionSynchronizationManager.isActualTransactionActive()); + deliveryStarted.countDown(); + try { + Thread.sleep(BLOCKING_DELIVERY_MS); + } finally { + deliveryFinished.countDown(); + } + return null; + }).when(notificationDeliveryService).sendReminder(notification, "message"); + + long startedAt = System.nanoTime(); + + notificationDispatchService.dispatchReminder(10L, "message"); + + long dispatchReturnMs = Duration.ofNanos(System.nanoTime() - startedAt).toMillis(); + + assertThat(dispatchReturnMs).isLessThan(MAX_DISPATCH_RETURN_MS); + assertThat(deliveryStarted.await(1, TimeUnit.SECONDS)).isTrue(); + assertThat(deliveryFinished.await(1, TimeUnit.SECONDS)).isTrue(); + assertThat(deliveryThreadName.get()).startsWith("notification-"); + assertThat(transactionActive.get()).isTrue(); + } + + private NotificationSchedule notificationSchedule() { + return NotificationSchedule.builder() + .notificationTime(LocalDateTime.now()) + .isSent(false) + .schedule(Schedule.builder() + .scheduleId(UUID.randomUUID()) + .scheduleName("Morning meeting") + .user(User.builder() + .id(1L) + .name("User") + .firebaseToken("firebase-token") + .build()) + .build()) + .build(); + } + + @Configuration + @EnableAsync + @EnableTransactionManagement + @Import({SchedulerConfig.class, NotificationDispatchService.class}) + static class Config { + @Bean + NotificationScheduleRepository notificationScheduleRepository() { + return mock(NotificationScheduleRepository.class); + } + + @Bean + NotificationDeliveryService notificationDeliveryService() { + return mock(NotificationDeliveryService.class); + } + + @Bean + TestTransactionManager transactionManager() { + return new TestTransactionManager(); + } + } + + static class TestTransactionManager extends AbstractPlatformTransactionManager { + @Override + protected Object doGetTransaction() { + return new Object(); + } + + @Override + protected void doBegin(Object transaction, TransactionDefinition definition) { + } + + @Override + protected void doCommit(DefaultTransactionStatus status) { + } + + @Override + protected void doRollback(DefaultTransactionStatus status) { + } + } +} diff --git a/ontime-back/src/test/java/devkor/ontime_back/service/NotificationDispatchServiceTest.java b/ontime-back/src/test/java/devkor/ontime_back/service/NotificationDispatchServiceTest.java new file mode 100644 index 0000000..93e3ac2 --- /dev/null +++ b/ontime-back/src/test/java/devkor/ontime_back/service/NotificationDispatchServiceTest.java @@ -0,0 +1,78 @@ +package devkor.ontime_back.service; + +import devkor.ontime_back.entity.NotificationSchedule; +import devkor.ontime_back.entity.Schedule; +import devkor.ontime_back.entity.User; +import devkor.ontime_back.repository.NotificationScheduleRepository; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +import java.time.LocalDateTime; +import java.util.Optional; +import java.util.UUID; + +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; +import static org.mockito.Mockito.when; + +@ExtendWith(MockitoExtension.class) +class NotificationDispatchServiceTest { + + @Mock + private NotificationScheduleRepository notificationScheduleRepository; + + @Mock + private NotificationDeliveryService notificationDeliveryService; + + private NotificationDispatchService notificationDispatchService; + + @BeforeEach + void setUp() { + notificationDispatchService = new NotificationDispatchService( + notificationScheduleRepository, + notificationDeliveryService + ); + } + + @Test + void dispatchReminderReloadsNotificationByIdBeforeDelivery() { + NotificationSchedule notification = notificationSchedule(); + when(notificationScheduleRepository.findByIdWithScheduleAndUser(10L)) + .thenReturn(Optional.of(notification)); + + notificationDispatchService.dispatchReminder(10L, "message"); + + verify(notificationScheduleRepository).findByIdWithScheduleAndUser(10L); + verify(notificationDeliveryService).sendReminder(notification, "message"); + } + + @Test + void dispatchReminderSkipsMissingNotification() { + when(notificationScheduleRepository.findByIdWithScheduleAndUser(10L)) + .thenReturn(Optional.empty()); + + notificationDispatchService.dispatchReminder(10L, "message"); + + verify(notificationScheduleRepository).findByIdWithScheduleAndUser(10L); + verifyNoInteractions(notificationDeliveryService); + } + + private NotificationSchedule notificationSchedule() { + return NotificationSchedule.builder() + .notificationTime(LocalDateTime.now()) + .isSent(false) + .schedule(Schedule.builder() + .scheduleId(UUID.randomUUID()) + .scheduleName("Morning meeting") + .user(User.builder() + .id(1L) + .name("User") + .firebaseToken("firebase-token") + .build()) + .build()) + .build(); + } +} diff --git a/ontime-back/src/test/java/devkor/ontime_back/service/NotificationServiceTest.java b/ontime-back/src/test/java/devkor/ontime_back/service/NotificationServiceTest.java index a6b311b..980c1e3 100644 --- a/ontime-back/src/test/java/devkor/ontime_back/service/NotificationServiceTest.java +++ b/ontime-back/src/test/java/devkor/ontime_back/service/NotificationServiceTest.java @@ -1,15 +1,12 @@ package devkor.ontime_back.service; import devkor.ontime_back.entity.NotificationSchedule; -import devkor.ontime_back.entity.Role; import devkor.ontime_back.entity.Schedule; import devkor.ontime_back.entity.User; -import devkor.ontime_back.entity.UserSetting; -import devkor.ontime_back.repository.NotificationScheduleRepository; -import devkor.ontime_back.repository.UserSettingRepository; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.ArgumentCaptor; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; import org.springframework.scheduling.TaskScheduler; @@ -18,39 +15,38 @@ import java.time.LocalDateTime; import java.util.Date; import java.util.List; -import java.util.Optional; import java.util.UUID; import java.util.concurrent.ScheduledFuture; 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.Mockito.*; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; +import static org.mockito.Mockito.when; @ExtendWith(MockitoExtension.class) class NotificationServiceTest { @Mock - private UserSettingRepository userSettingRepository; - - @Mock - private AlarmService alarmService; + private TaskScheduler taskScheduler; @Mock - private TaskScheduler taskScheduler; + private NotificationDispatchService notificationDispatchService; @Mock - private NotificationScheduleRepository notificationScheduleRepository; + private NotificationDeliveryService notificationDeliveryService; private NotificationService notificationService; @BeforeEach void setUp() { notificationService = new NotificationService( - userSettingRepository, - alarmService, taskScheduler, - notificationScheduleRepository + notificationDispatchService, + notificationDeliveryService ); } @@ -60,11 +56,22 @@ void scheduleReminderIgnoresPastReminderTimes() { notificationService.scheduleReminder(notification); - verifyNoInteractions(taskScheduler); + verifyNoInteractions(taskScheduler, notificationDispatchService); } @Test - void scheduleReminderRegistersFutureTaskAndCanCancelIt() { + void scheduleReminderRequiresPersistedNotificationId() { + NotificationSchedule notification = notificationSchedule(LocalDateTime.now().plusHours(1)); + + assertThatThrownBy(() -> notificationService.scheduleReminder(notification)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("NotificationSchedule must be persisted before scheduling"); + + verifyNoInteractions(taskScheduler, notificationDispatchService); + } + + @Test + void scheduleReminderRegistersFutureTaskWithNotificationIdAndCanCancelIt() { NotificationSchedule notification = notificationSchedule(LocalDateTime.now().plusHours(1)); ReflectionTestUtils.setField(notification, "id", 10L); ScheduledFuture future = mock(ScheduledFuture.class); @@ -74,125 +81,51 @@ void scheduleReminderRegistersFutureTaskAndCanCancelIt() { notificationService.scheduleReminder(notification); notificationService.cancelScheduledNotification(10L); - verify(taskScheduler).schedule(any(Runnable.class), any(Date.class)); + ArgumentCaptor runnableCaptor = ArgumentCaptor.forClass(Runnable.class); + verify(taskScheduler).schedule(runnableCaptor.capture(), any(Date.class)); verify(future).cancel(true); - } - @Test - void sendReminderDoesNothingWhenUserIdIsMissing() { - NotificationSchedule notification = NotificationSchedule.builder() - .notificationTime(LocalDateTime.now()) - .isSent(false) - .schedule(Schedule.builder() - .scheduleId(UUID.randomUUID()) - .scheduleName("No owner") - .user(User.builder().build()) - .build()) - .build(); - - notificationService.sendReminder(notification, "message"); - - verifyNoInteractions(userSettingRepository, alarmService, notificationScheduleRepository); - assertThat(notification.getIsSent()).isFalse(); - } + runnableCaptor.getValue().run(); - @Test - void sendReminderDoesNotSendWhenUserDisabledNotifications() { - NotificationSchedule notification = notificationSchedule(LocalDateTime.now()); - when(userSettingRepository.findByUserId(1L)).thenReturn(Optional.of(userSetting(false))); - - notificationService.sendReminder(notification, "message"); - - verifyNoInteractions(alarmService, notificationScheduleRepository); - assertThat(notification.getIsSent()).isFalse(); + verify(notificationDispatchService).dispatchReminder( + 10L, + "준비 시작해야 합니다.(현재 시각: 약속시각 - (여유시간 + 이동시간 + 총준비시간) )" + ); } @Test - void sendReminderDoesNotSendWhenNativeAlarmAlreadyCoversSchedule() { + void sendReminderDelegatesSingleNotificationDelivery() { NotificationSchedule notification = notificationSchedule(LocalDateTime.now()); - UUID scheduleId = notification.getSchedule().getScheduleId(); - when(userSettingRepository.findByUserId(1L)).thenReturn(Optional.of(userSetting(true))); - when(alarmService.shouldSuppressLegacyReminder(1L, scheduleId, notification.getNotificationTime())) - .thenReturn(true); notificationService.sendReminder(notification, "message"); - verify(notificationScheduleRepository, never()).save(notification); - assertThat(notification.getIsSent()).isFalse(); + verify(notificationDeliveryService).sendReminder(notification, "message"); } @Test - void sendReminderMarksNotificationSentWhenUserEnabledNotificationsAndNativeAlarmDoesNotCoverIt() { - NotificationSchedule notification = notificationSchedule(LocalDateTime.now()); - UUID scheduleId = notification.getSchedule().getScheduleId(); - NotificationService spyService = spy(notificationService); - doNothing().when(spyService).sendNotificationToUser(notification.getSchedule(), "message"); - when(userSettingRepository.findByUserId(1L)).thenReturn(Optional.of(userSetting(true))); - when(alarmService.shouldSuppressLegacyReminder(1L, scheduleId, notification.getNotificationTime())) - .thenReturn(false); - - spyService.sendReminder(notification, "message"); - - verify(spyService).sendNotificationToUser(notification.getSchedule(), "message"); - verify(notificationScheduleRepository).save(notification); - assertThat(notification.getIsSent()).isTrue(); - } - - @Test - void sendReminderFailsClearlyWhenUserSettingsAreMissing() { - NotificationSchedule notification = notificationSchedule(LocalDateTime.now()); - when(userSettingRepository.findByUserId(1L)).thenReturn(Optional.empty()); - - assertThatThrownBy(() -> notificationService.sendReminder(notification, "message")) - .isInstanceOf(IllegalArgumentException.class) - .hasMessage("No UserSetting found in schedule's user"); - } - - @Test - void listReminderSkipsSchedulesWithoutPersistedUsers() { + void sendReminderDelegatesListNotificationDelivery() { Schedule schedule = Schedule.builder() .scheduleId(UUID.randomUUID()) - .scheduleName("Unsaved user") - .user(User.builder().build()) + .scheduleName("Meeting") + .user(User.builder().id(1L).build()) .build(); notificationService.sendReminder(List.of(schedule), "message"); - verifyNoInteractions(userSettingRepository, alarmService, notificationScheduleRepository); - } - - @Test - void listReminderSendsOnlyForSchedulesWhoseUsersAllowNotifications() { - Schedule enabledSchedule = scheduleForListReminder(1L, "Enabled"); - Schedule disabledSchedule = scheduleForListReminder(2L, "Disabled"); - NotificationService spyService = spy(notificationService); - doNothing().when(spyService).sendNotificationToUser(any(Schedule.class), eq("message")); - when(userSettingRepository.findByUserId(1L)).thenReturn(Optional.of(userSetting(true))); - when(userSettingRepository.findByUserId(2L)).thenReturn(Optional.of(userSetting(false))); - - spyService.sendReminder(List.of(enabledSchedule, disabledSchedule), "message"); - - verify(spyService).sendNotificationToUser(enabledSchedule, "message"); - verify(spyService, never()).sendNotificationToUser(disabledSchedule, "message"); - } - - @Test - void listReminderFailsClearlyWhenPersistedUserHasNoSettings() { - Schedule schedule = scheduleForListReminder(1L, "Missing setting"); - when(userSettingRepository.findByUserId(1L)).thenReturn(Optional.empty()); - - assertThatThrownBy(() -> notificationService.sendReminder(List.of(schedule), "message")) - .isInstanceOf(IllegalArgumentException.class) - .hasMessage("No UserSetting found in schedule's user"); + verify(notificationDeliveryService).sendReminder(List.of(schedule), "message"); } @Test - void sendNotificationToUserDoesNotPropagateFirebaseClientFailures() { - Schedule schedule = scheduleForListReminder(1L, "Firebase smoke test"); + void sendNotificationToUserDelegatesDelivery() { + Schedule schedule = Schedule.builder() + .scheduleId(UUID.randomUUID()) + .scheduleName("Meeting") + .user(User.builder().id(1L).build()) + .build(); notificationService.sendNotificationToUser(schedule, "message"); - assertThat(schedule.getUser().getFirebaseToken()).isEqualTo("firebase-token-1"); + verify(notificationDeliveryService).sendNotificationToUser(schedule, "message"); } private NotificationSchedule notificationSchedule(LocalDateTime notificationTime) { @@ -206,29 +139,8 @@ private NotificationSchedule notificationSchedule(LocalDateTime notificationTime .id(1L) .name("User") .firebaseToken("firebase-token") - .role(Role.USER) .build()) .build()) .build(); } - - private UserSetting userSetting(boolean notificationsEnabled) { - return UserSetting.builder() - .userSettingId(UUID.randomUUID()) - .isNotificationsEnabled(notificationsEnabled) - .build(); - } - - private Schedule scheduleForListReminder(Long userId, String scheduleName) { - return Schedule.builder() - .scheduleId(UUID.randomUUID()) - .scheduleName(scheduleName) - .user(User.builder() - .id(userId) - .name("User " + userId) - .firebaseToken("firebase-token-" + userId) - .role(Role.USER) - .build()) - .build(); - } }