From 225456535d4443d52cea10dddac146e0df7841b4 Mon Sep 17 00:00:00 2001 From: jjoonleo Date: Sun, 28 Jun 2026 14:59:32 +0900 Subject: [PATCH] Fix duplicate schedule notifications --- .../NotificationScheduleRepository.java | 3 +- .../ontime_back/service/ScheduleService.java | 34 ++++- ...ication_schedule_add_unique_constraint.sql | 9 ++ .../service/PreparationUserServiceTest.java | 39 +++++ ...cheduleServiceNotificationRefreshTest.java | 135 ++++++++++++++++++ .../service/ScheduleServiceTest.java | 53 +++++++ 6 files changed, 267 insertions(+), 6 deletions(-) create mode 100644 ontime-back/src/main/resources/db/migration/V19__deduplicate_notification_schedule_add_unique_constraint.sql create mode 100644 ontime-back/src/test/java/devkor/ontime_back/service/ScheduleServiceNotificationRefreshTest.java 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 0e88702d..26885bf5 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 @@ -7,7 +7,6 @@ import java.time.LocalDateTime; import java.util.List; -import java.util.Optional; import java.util.UUID; @Repository @@ -18,5 +17,5 @@ public interface NotificationScheduleRepository extends JpaRepository :now AND n.isSent = false") List findAllWithScheduleAndUser(LocalDateTime now); - Optional findByScheduleScheduleId(UUID scheduleId); + List findAllByScheduleScheduleIdOrderByIdAsc(UUID scheduleId); } diff --git a/ontime-back/src/main/java/devkor/ontime_back/service/ScheduleService.java b/ontime-back/src/main/java/devkor/ontime_back/service/ScheduleService.java index 8dc89dc0..4c71f8f6 100644 --- a/ontime-back/src/main/java/devkor/ontime_back/service/ScheduleService.java +++ b/ontime-back/src/main/java/devkor/ontime_back/service/ScheduleService.java @@ -123,8 +123,11 @@ public ScheduleDto showScheduleByScheduleId(Long userId, UUID scheduleId) { public void deleteSchedule(UUID scheduleId, Long userId) { Schedule schedule = getLockedScheduleWithAuthorization(scheduleId, userId); assertScheduleNotFinished(schedule); - NotificationSchedule notification = notificationScheduleRepository.findByScheduleScheduleId(scheduleId) - .orElseThrow(() -> new GeneralException(NOTIFICATION_NOT_FOUND)); + List notifications = notificationScheduleRepository.findAllByScheduleScheduleIdOrderByIdAsc(scheduleId); + if (notifications.isEmpty()) { + throw new GeneralException(NOTIFICATION_NOT_FOUND); + } + notifications.forEach(notification -> notificationService.cancelScheduledNotification(notification.getId())); scheduleRepository.deleteByScheduleId(scheduleId); } @@ -160,6 +163,9 @@ public void updateAndRescheduleNotification(LocalDateTime newNotificationTime, N // schedule 추가 @Transactional public void addSchedule(ScheduleAddDto scheduleAddDto, Long userId) { + if (scheduleRepository.existsById(scheduleAddDto.getScheduleId())) { + throw new GeneralException(RESOURCE_ALREADY_EXISTS); + } User user = userRepository.findById(userId) .orElseThrow(() -> new GeneralException(USER_NOT_FOUND)); Place place = placeRepository.findByPlaceName(scheduleAddDto.getPlaceName()) @@ -559,9 +565,8 @@ public void refreshNotStartedDefaultModeSchedules(Long userId) { } private void refreshScheduleNotification(Schedule schedule) { - NotificationSchedule notification = notificationScheduleRepository.findByScheduleScheduleId(schedule.getScheduleId()) - .orElseThrow(() -> new GeneralException(NOTIFICATION_NOT_FOUND)); LocalDateTime newNotificationTime = getNotificationTime(schedule, schedule.getUser()); + NotificationSchedule notification = resolveNotificationForRefresh(schedule, newNotificationTime); if (newNotificationTime.equals(notification.getNotificationTime())) { notificationService.cancelScheduledNotification(notification.getId()); notification.markAsUnsent(); @@ -572,6 +577,27 @@ private void refreshScheduleNotification(Schedule schedule) { updateAndRescheduleNotification(newNotificationTime, notification); } + private NotificationSchedule resolveNotificationForRefresh(Schedule schedule, LocalDateTime notificationTime) { + List notifications = notificationScheduleRepository + .findAllByScheduleScheduleIdOrderByIdAsc(schedule.getScheduleId()); + if (notifications.isEmpty()) { + NotificationSchedule notification = NotificationSchedule.builder() + .notificationTime(notificationTime) + .isSent(false) + .schedule(schedule) + .build(); + return notificationScheduleRepository.save(notification); + } + + NotificationSchedule notification = notifications.get(0); + for (int i = 1; i < notifications.size(); i++) { + NotificationSchedule duplicate = notifications.get(i); + notificationService.cancelScheduledNotification(duplicate.getId()); + notificationScheduleRepository.delete(duplicate); + } + return notification; + } + private PreparationDto mapPreparationUserToDto(PreparationUser preparationUser) { return new PreparationDto( preparationUser.getPreparationUserId(), diff --git a/ontime-back/src/main/resources/db/migration/V19__deduplicate_notification_schedule_add_unique_constraint.sql b/ontime-back/src/main/resources/db/migration/V19__deduplicate_notification_schedule_add_unique_constraint.sql new file mode 100644 index 00000000..d2c29b3e --- /dev/null +++ b/ontime-back/src/main/resources/db/migration/V19__deduplicate_notification_schedule_add_unique_constraint.sql @@ -0,0 +1,9 @@ +DELETE ns_duplicate +FROM notification_schedule ns_duplicate +JOIN notification_schedule ns_keep + ON ns_duplicate.schedule_id = ns_keep.schedule_id + AND ns_duplicate.id > ns_keep.id +WHERE ns_duplicate.schedule_id IS NOT NULL; + +CREATE UNIQUE INDEX uk_notification_schedule_schedule + ON notification_schedule (schedule_id); diff --git a/ontime-back/src/test/java/devkor/ontime_back/service/PreparationUserServiceTest.java b/ontime-back/src/test/java/devkor/ontime_back/service/PreparationUserServiceTest.java index 33ad9864..81a8a105 100644 --- a/ontime-back/src/test/java/devkor/ontime_back/service/PreparationUserServiceTest.java +++ b/ontime-back/src/test/java/devkor/ontime_back/service/PreparationUserServiceTest.java @@ -299,4 +299,43 @@ void handlePreparationUsers_withDeletingExisting() { .containsExactlyInAnyOrder("세면", "옷입기"); } + @Test + @DisplayName("기존 준비과정 ID를 유지하며 기본 준비과정을 수정한다.") + void updatePreparationUsers_reusesExistingStepIds() { + // given + User newUser = User.builder() + .email("reuse@example.com") + .password(passwordEncoder.encode("password1234")) + .name("reuse") + .punctualityScore(-1f) + .scheduleCountAfterReset(0) + .latenessCountAfterReset(0) + .build(); + userRepository.save(newUser); + + UUID preparationUser1Id = UUID.randomUUID(); + UUID preparationUser2Id = UUID.randomUUID(); + + PreparationUser preparationUser2 = preparationUserRepository.save(new PreparationUser( + preparationUser2Id, newUser, "화장실 가기", 3, 1, null)); + preparationUserRepository.save(new PreparationUser( + preparationUser1Id, newUser, "메이크업", 38, 0, preparationUser2)); + + List preparationDtoList = List.of( + new PreparationDto(preparationUser1Id, "메이크업", 38, preparationUser2Id), + new PreparationDto(preparationUser2Id, "화장실 가기", 3, null) + ); + + // when + preparationUserService.updatePreparationUsers(newUser.getId(), preparationDtoList); + + // then + List result = preparationUserService.showAllPreparationUsers(newUser.getId()); + assertThat(result).hasSize(2); + assertThat(result.get(0).getPreparationId()).isEqualTo(preparationUser1Id); + assertThat(result.get(0).getNextPreparationId()).isEqualTo(preparationUser2Id); + assertThat(result.get(1).getPreparationId()).isEqualTo(preparationUser2Id); + assertThat(result.get(1).getNextPreparationId()).isNull(); + } + } diff --git a/ontime-back/src/test/java/devkor/ontime_back/service/ScheduleServiceNotificationRefreshTest.java b/ontime-back/src/test/java/devkor/ontime_back/service/ScheduleServiceNotificationRefreshTest.java new file mode 100644 index 00000000..bb0418ee --- /dev/null +++ b/ontime-back/src/test/java/devkor/ontime_back/service/ScheduleServiceNotificationRefreshTest.java @@ -0,0 +1,135 @@ +package devkor.ontime_back.service; + +import devkor.ontime_back.dto.PreparationDto; +import devkor.ontime_back.entity.DoneStatus; +import devkor.ontime_back.entity.NotificationSchedule; +import devkor.ontime_back.entity.PreparationMode; +import devkor.ontime_back.entity.Schedule; +import devkor.ontime_back.entity.User; +import devkor.ontime_back.repository.NotificationScheduleRepository; +import devkor.ontime_back.repository.PlaceRepository; +import devkor.ontime_back.repository.PreparationScheduleRepository; +import devkor.ontime_back.repository.PreparationTemplateRepository; +import devkor.ontime_back.repository.PreparationTemplateStepRepository; +import devkor.ontime_back.repository.PreparationUserRepository; +import devkor.ontime_back.repository.ScheduleRepository; +import devkor.ontime_back.repository.UserRepository; +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 org.springframework.test.util.ReflectionTestUtils; + +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.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +@ExtendWith(MockitoExtension.class) +class ScheduleServiceNotificationRefreshTest { + + @Mock + private UserService userService; + @Mock + private NotificationService notificationService; + @Mock + private AlarmService alarmService; + @Mock + private ScheduleRepository scheduleRepository; + @Mock + private UserRepository userRepository; + @Mock + private PlaceRepository placeRepository; + @Mock + private PreparationScheduleRepository preparationScheduleRepository; + @Mock + private PreparationUserRepository preparationUserRepository; + @Mock + private PreparationTemplateRepository preparationTemplateRepository; + @Mock + private PreparationTemplateStepRepository preparationTemplateStepRepository; + @Mock + private NotificationScheduleRepository notificationScheduleRepository; + @Mock + private PreparationStepService preparationStepService; + + private ScheduleService scheduleService; + + @BeforeEach + void setUp() { + scheduleService = new ScheduleService( + userService, + notificationService, + alarmService, + scheduleRepository, + userRepository, + placeRepository, + preparationScheduleRepository, + preparationUserRepository, + preparationTemplateRepository, + preparationTemplateStepRepository, + notificationScheduleRepository, + preparationStepService + ); + } + + @Test + void refreshDefaultModeScheduleDeduplicatesNotificationRows() { + UUID scheduleId = UUID.randomUUID(); + User user = User.builder() + .id(1L) + .spareTime(10) + .build(); + Schedule schedule = Schedule.builder() + .scheduleId(scheduleId) + .scheduleName("학교") + .scheduleTime(LocalDateTime.of(2026, 6, 29, 14, 30)) + .moveTime(10) + .doneStatus(DoneStatus.NOT_ENDED) + .preparationMode(PreparationMode.DEFAULT) + .user(user) + .build(); + NotificationSchedule canonical = notification(1L, schedule, LocalDateTime.of(2026, 6, 29, 13, 0)); + NotificationSchedule duplicate1 = notification(2L, schedule, LocalDateTime.of(2026, 6, 29, 13, 5)); + NotificationSchedule duplicate2 = notification(3L, schedule, LocalDateTime.of(2026, 6, 29, 13, 10)); + NotificationSchedule duplicate3 = notification(4L, schedule, LocalDateTime.of(2026, 6, 29, 13, 15)); + + when(scheduleRepository.findNotStartedDefaultModeSchedules(1L)).thenReturn(List.of(schedule)); + when(scheduleRepository.findByIdWithUser(scheduleId)).thenReturn(Optional.of(schedule)); + when(preparationUserRepository.findByUserIdWithNextPreparation(1L)).thenReturn(List.of()); + when(preparationStepService.toLinkedDtoFromUser(List.of())).thenReturn(List.of( + new PreparationDto(UUID.randomUUID(), "메이크업", 38, null), + new PreparationDto(UUID.randomUUID(), "화장실 가기", 3, null) + )); + when(alarmService.getDefaultAlarmOffsetMinutes(1L)).thenReturn(0); + when(notificationScheduleRepository.findAllByScheduleScheduleIdOrderByIdAsc(scheduleId)) + .thenReturn(List.of(canonical, duplicate1, duplicate2, duplicate3)); + + scheduleService.refreshNotStartedDefaultModeSchedules(1L); + + assertThat(canonical.getNotificationTime()).isEqualTo(LocalDateTime.of(2026, 6, 29, 13, 29)); + verify(notificationService).cancelScheduledNotification(2L); + verify(notificationService).cancelScheduledNotification(3L); + verify(notificationService).cancelScheduledNotification(4L); + verify(notificationScheduleRepository).delete(duplicate1); + verify(notificationScheduleRepository).delete(duplicate2); + verify(notificationScheduleRepository).delete(duplicate3); + verify(notificationScheduleRepository).save(canonical); + verify(notificationService).scheduleReminder(canonical); + } + + private NotificationSchedule notification(Long id, Schedule schedule, LocalDateTime notificationTime) { + NotificationSchedule notification = NotificationSchedule.builder() + .schedule(schedule) + .notificationTime(notificationTime) + .isSent(false) + .build(); + ReflectionTestUtils.setField(notification, "id", id); + return notification; + } +} diff --git a/ontime-back/src/test/java/devkor/ontime_back/service/ScheduleServiceTest.java b/ontime-back/src/test/java/devkor/ontime_back/service/ScheduleServiceTest.java index 934caf88..64e6a310 100644 --- a/ontime-back/src/test/java/devkor/ontime_back/service/ScheduleServiceTest.java +++ b/ontime-back/src/test/java/devkor/ontime_back/service/ScheduleServiceTest.java @@ -1109,6 +1109,59 @@ void addSchedule_withNewPlace() { assertThat(savedSchedule.get().getPlace().getPlaceName()).isEqualTo("고려대학교"); } + @Test + @DisplayName("이미 존재하는 scheduleId로 약속 추가 시 실패한다.") + void addSchedule_failByDuplicateScheduleId() { + // given + User newUser = User.builder() + .email("user@example.com") + .password(passwordEncoder.encode("password1234")) + .name("jinsuh") + .punctualityScore(-1f) + .scheduleCountAfterReset(0) + .latenessCountAfterReset(0) + .build(); + userRepository.save(newUser); + + Place place = Place.builder() + .placeId(UUID.fromString("70d460da-6a82-4c57-a285-567cdeda5601")) + .placeName("과학도서관") + .build(); + placeRepository.save(place); + + UUID duplicateScheduleId = UUID.fromString("023e4567-e89b-12d3-a456-426614170000"); + scheduleRepository.save(Schedule.builder() + .scheduleId(duplicateScheduleId) + .scheduleName("기존 약속") + .scheduleTime(LocalDateTime.of(2025, 2, 10, 14, 0)) + .moveTime(30) + .latenessTime(-1) + .doneStatus(DoneStatus.NOT_ENDED) + .place(place) + .user(newUser) + .build()); + + ScheduleAddDto scheduleAddDto = ScheduleAddDto.builder() + .scheduleId(duplicateScheduleId) + .scheduleName("중복 약속") + .scheduleTime(LocalDateTime.of(2025, 2, 11, 14, 0)) + .moveTime(30) + .scheduleNote("늦으면 안됨") + .placeId(place.getPlaceId()) + .placeName(place.getPlaceName()) + .scheduleSpareTime(5) + .isChange(false) + .isStarted(false) + .build(); + + // when & then + assertThatThrownBy(() -> scheduleService.addSchedule(scheduleAddDto, newUser.getId())) + .isInstanceOf(GeneralException.class) + .hasMessage(ErrorCode.RESOURCE_ALREADY_EXISTS.getMessage()) + .extracting("errorCode") + .isEqualTo(ErrorCode.RESOURCE_ALREADY_EXISTS); + } + @Test @DisplayName("다른 사용자가 약속 추가 시 실패한다.") void addSchedule_failByNonExistentUser() {