From cfb8e23e71a760e9b41b049d4f414699a4e6f9bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=EB=B0=95=ED=83=9C=EC=A7=84?= <140797244+taejinn@users.noreply.github.com> Date: Tue, 1 Sep 2026 16:14:36 +0900 Subject: [PATCH] fix: make team recruitment notification actions idempotent --- ...TeamRecruitmentNotificationRepository.java | 30 ++++- .../TeamRecruitmentNotificationApi.java | 12 +- .../TeamRecruitmentNotificationService.java | 13 +- .../TeamRecruitmentArticleFlowApiTest.java | 113 ++++++++++++++++++ ...eamRecruitmentNotificationServiceTest.java | 82 +++++++++---- 5 files changed, 207 insertions(+), 43 deletions(-) diff --git a/src/main/java/in/koreatech/koin/domain/team/recruitment/repository/TeamRecruitmentNotificationRepository.java b/src/main/java/in/koreatech/koin/domain/team/recruitment/repository/TeamRecruitmentNotificationRepository.java index fbe53937c6..5ae58f0542 100644 --- a/src/main/java/in/koreatech/koin/domain/team/recruitment/repository/TeamRecruitmentNotificationRepository.java +++ b/src/main/java/in/koreatech/koin/domain/team/recruitment/repository/TeamRecruitmentNotificationRepository.java @@ -3,8 +3,10 @@ import in.koreatech.koin.domain.team.recruitment.enums.TeamRecruitmentNotificationType; import in.koreatech.koin.domain.team.recruitment.enums.TeamRecruitmentNotificationTargetType; import in.koreatech.koin.domain.team.recruitment.model.TeamRecruitmentNotification; +import java.time.LocalDateTime; import org.springframework.data.domain.Page; import org.springframework.data.domain.Pageable; +import org.springframework.data.jpa.repository.Modifying; import org.springframework.data.jpa.repository.Query; import org.springframework.data.repository.Repository; import org.springframework.data.repository.query.Param; @@ -29,7 +31,33 @@ Page findAllByRecipient_IdAndIsDeletedFalseOrderByI long countByRecipient_IdAndReadAtIsNullAndIsDeletedFalse(Integer recipientId); - Optional findByIdAndRecipient_Id(Integer id, Integer recipientId); + @Modifying(clearAutomatically = true) + @Query(""" + UPDATE TeamRecruitmentNotification notification + SET notification.readAt = :readAt + WHERE notification.recipient.id = :recipientId + AND notification.id = :notificationId + AND notification.readAt IS NULL + AND notification.isDeleted = false + """) + void updateReadAtByRecipientIdAndNotificationId( + @Param("recipientId") Integer recipientId, + @Param("notificationId") Integer notificationId, + @Param("readAt") LocalDateTime readAt + ); + + @Modifying(clearAutomatically = true) + @Query(""" + UPDATE TeamRecruitmentNotification notification + SET notification.isDeleted = true + WHERE notification.recipient.id = :recipientId + AND notification.id = :notificationId + AND notification.isDeleted = false + """) + void updateIsDeletedByRecipientIdAndNotificationId( + @Param("recipientId") Integer recipientId, + @Param("notificationId") Integer notificationId + ); List findAllByRecipient_IdAndIsDeletedFalse(Integer recipientId); diff --git a/src/main/java/in/koreatech/koin/domain/teamrecruitment/controller/TeamRecruitmentNotificationApi.java b/src/main/java/in/koreatech/koin/domain/teamrecruitment/controller/TeamRecruitmentNotificationApi.java index 611e170ff9..8e653d1d7b 100644 --- a/src/main/java/in/koreatech/koin/domain/teamrecruitment/controller/TeamRecruitmentNotificationApi.java +++ b/src/main/java/in/koreatech/koin/domain/teamrecruitment/controller/TeamRecruitmentNotificationApi.java @@ -3,7 +3,6 @@ import static in.koreatech.koin.global.code.ApiResponseCode.FORBIDDEN_USER_TYPE; import static in.koreatech.koin.global.code.ApiResponseCode.NO_CONTENT; import static in.koreatech.koin.global.code.ApiResponseCode.OK; -import static in.koreatech.koin.global.code.ApiResponseCode.TEAM_RECRUITMENT_NOTIFICATION_NOT_FOUND; import static in.koreatech.koin.global.code.ApiResponseCode.UNAUTHORIZED_USER; import org.springframework.http.ResponseEntity; @@ -32,11 +31,14 @@ ResponseEntity getNotifications( @ApiResponseCodes({ NO_CONTENT, - TEAM_RECRUITMENT_NOTIFICATION_NOT_FOUND, UNAUTHORIZED_USER, FORBIDDEN_USER_TYPE, }) - @Operation(summary = "알림 읽음 처리") + @Operation(summary = "알림 읽음 처리", description = """ + ### 알림 읽음 처리 + - 본인에게 온 삭제되지 않은 알림만 읽음 처리하며, 타 사용자 알림은 변경하지 않습니다. + - 이미 읽었거나 삭제되었거나 대상이 없어도 204를 반환합니다. + """) ResponseEntity markAsRead( Integer userId, @PathVariable Integer notificationId @@ -52,14 +54,14 @@ ResponseEntity markAsRead( @ApiResponseCodes({ NO_CONTENT, - TEAM_RECRUITMENT_NOTIFICATION_NOT_FOUND, UNAUTHORIZED_USER, FORBIDDEN_USER_TYPE, }) @Operation(summary = "알림 개별 삭제", description = """ ### 알림 개별 삭제 - 알림을 삭제 처리하여 목록에서 제외합니다. - - 본인에게 온 알림만 삭제할 수 있으며, 그 외에는 404를 반환합니다. + - 본인에게 온 삭제되지 않은 알림만 삭제 처리하며, 타 사용자 알림은 변경하지 않습니다. + - 이미 삭제되었거나 대상이 없어도 204를 반환합니다. """) ResponseEntity delete( Integer userId, diff --git a/src/main/java/in/koreatech/koin/domain/teamrecruitment/service/TeamRecruitmentNotificationService.java b/src/main/java/in/koreatech/koin/domain/teamrecruitment/service/TeamRecruitmentNotificationService.java index 8831efba30..6c0e7790eb 100644 --- a/src/main/java/in/koreatech/koin/domain/teamrecruitment/service/TeamRecruitmentNotificationService.java +++ b/src/main/java/in/koreatech/koin/domain/teamrecruitment/service/TeamRecruitmentNotificationService.java @@ -13,11 +13,8 @@ import in.koreatech.koin.domain.team.recruitment.repository.TeamRecruitmentNotificationRepository; import in.koreatech.koin.domain.teamrecruitment.dto.TeamRecruitmentNotificationResponse; import in.koreatech.koin.domain.teamrecruitment.dto.TeamRecruitmentNotificationsResponse; -import in.koreatech.koin.global.exception.CustomException; import lombok.RequiredArgsConstructor; -import static in.koreatech.koin.global.code.ApiResponseCode.TEAM_RECRUITMENT_NOTIFICATION_NOT_FOUND; - @Service @RequiredArgsConstructor @Transactional(readOnly = true) @@ -50,18 +47,12 @@ public TeamRecruitmentNotificationsResponse getNotifications(Integer userId, int @Transactional public void markAsRead(Integer userId, Integer notificationId) { - TeamRecruitmentNotification notification = notificationRepository - .findByIdAndRecipient_Id(notificationId, userId) - .orElseThrow(() -> CustomException.of(TEAM_RECRUITMENT_NOTIFICATION_NOT_FOUND)); - notification.markAsRead(LocalDateTime.now()); + notificationRepository.updateReadAtByRecipientIdAndNotificationId(userId, notificationId, LocalDateTime.now()); } @Transactional public void delete(Integer userId, Integer notificationId) { - TeamRecruitmentNotification notification = notificationRepository - .findByIdAndRecipient_Id(notificationId, userId) - .orElseThrow(() -> CustomException.of(TEAM_RECRUITMENT_NOTIFICATION_NOT_FOUND)); - notification.delete(); + notificationRepository.updateIsDeletedByRecipientIdAndNotificationId(userId, notificationId); } @Transactional diff --git a/src/test/java/in/koreatech/koin/acceptance/domain/TeamRecruitmentArticleFlowApiTest.java b/src/test/java/in/koreatech/koin/acceptance/domain/TeamRecruitmentArticleFlowApiTest.java index 04e75af30c..8a10705eb8 100644 --- a/src/test/java/in/koreatech/koin/acceptance/domain/TeamRecruitmentArticleFlowApiTest.java +++ b/src/test/java/in/koreatech/koin/acceptance/domain/TeamRecruitmentArticleFlowApiTest.java @@ -8,6 +8,7 @@ import static in.koreatech.koin.domain.team.recruitment.enums.TeamRecruitmentChatRoomStatus.READ_ONLY; import static in.koreatech.koin.domain.team.recruitment.enums.TeamRecruitmentChatRoomType.TEAM; import static in.koreatech.koin.domain.team.recruitment.enums.TeamRecruitmentMeetingType.ONLINE; +import static in.koreatech.koin.domain.team.recruitment.enums.TeamRecruitmentNotificationTargetType.MY_APPLICATIONS; import static in.koreatech.koin.domain.team.recruitment.enums.TeamRecruitmentNotificationType.APPLICATION_REJECTED; import static in.koreatech.koin.domain.team.recruitment.enums.TeamRecruitmentStatus.CLOSED; import static in.koreatech.koin.domain.team.recruitment.enums.TeamRecruitmentStatus.DELETED; @@ -18,11 +19,13 @@ import static org.assertj.core.api.Assertions.tuple; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.delete; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.put; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; import java.time.LocalDate; +import java.time.LocalDateTime; import java.util.List; import org.junit.jupiter.api.BeforeEach; @@ -37,6 +40,7 @@ import in.koreatech.koin.acceptance.fixture.UserAcceptanceFixture; import in.koreatech.koin.domain.student.model.Department; import in.koreatech.koin.domain.student.model.Student; +import in.koreatech.koin.domain.user.model.User; import in.koreatech.koin.domain.team.recruitment.enums.TeamRecruitmentApplicationStatus; import in.koreatech.koin.domain.team.recruitment.model.TeamRecruitment; import in.koreatech.koin.domain.team.recruitment.model.TeamRecruitmentApplication; @@ -101,6 +105,7 @@ class TeamRecruitmentArticleFlowApiTest extends AcceptanceTest { private Student author; private Student applicant; private String authorToken; + private String applicantToken; @BeforeEach void setUp() { @@ -109,6 +114,7 @@ void setUp() { author = userFixture.준호_학생(department, null); applicant = userFixture.성빈_학생(department); authorToken = userFixture.getToken(author.getUser()); + applicantToken = userFixture.getToken(applicant.getUser()); profileRepository.save(TeamRecruitmentProfile.builder() .user(applicant.getUser()) .profileNickname("지원자") @@ -161,6 +167,96 @@ void setUp() { .anyMatch(message -> message.contains("삭제되어")); } + @Test + @DisplayName("알림 단건 읽음 처리는 수신자와 미삭제 최초 읽음만 변경하고 반복 호출에도 읽은 시각을 보존한다") + void 알림_단건_읽음_처리_멱등성() throws Exception { + TeamRecruitment recruitment = saveGeneralRecruitment("알림 읽음", 3, 0); + TeamRecruitmentNotification unread = saveNotification(recruitment, applicant.getUser(), null, false); + TeamRecruitmentNotification deletedUnread = saveNotification(recruitment, applicant.getUser(), null, true); + TeamRecruitmentNotification otherUserUnread = saveNotification(recruitment, author.getUser(), null, false); + entityManager.flush(); + + mockMvc.perform(post("/team-recruitments/notifications/{notificationId}/read", unread.getId()) + .header("Authorization", "Bearer " + applicantToken)) + .andExpect(status().isNoContent()); + + entityManager.clear(); + LocalDateTime firstReadAt = notificationRepository.findById(unread.getId()).orElseThrow().getReadAt(); + assertThat(firstReadAt).isNotNull(); + + mockMvc.perform(post("/team-recruitments/notifications/{notificationId}/read", unread.getId()) + .header("Authorization", "Bearer " + applicantToken)) + .andExpect(status().isNoContent()); + mockMvc.perform(post("/team-recruitments/notifications/{notificationId}/read", deletedUnread.getId()) + .header("Authorization", "Bearer " + applicantToken)) + .andExpect(status().isNoContent()); + mockMvc.perform(post("/team-recruitments/notifications/{notificationId}/read", otherUserUnread.getId()) + .header("Authorization", "Bearer " + applicantToken)) + .andExpect(status().isNoContent()); + mockMvc.perform(post("/team-recruitments/notifications/{notificationId}/read", 999999) + .header("Authorization", "Bearer " + applicantToken)) + .andExpect(status().isNoContent()); + + entityManager.clear(); + assertThat(notificationRepository.findById(unread.getId()).orElseThrow().getReadAt()) + .isEqualTo(firstReadAt); + assertThat(notificationRepository.findById(deletedUnread.getId()).orElseThrow().getReadAt()) + .isNull(); + assertThat(notificationRepository.findById(otherUserUnread.getId()).orElseThrow().getReadAt()) + .isNull(); + assertThat(notificationRepository.countByRecipient_IdAndIsDeletedFalse(applicant.getUser().getId())) + .isEqualTo(1L); + mockMvc.perform(get("/team-recruitments/notifications") + .header("Authorization", "Bearer " + applicantToken)) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.total_count").value(1)) + .andExpect(jsonPath("$.unread_count").value(0)) + .andExpect(jsonPath("$.notifications.length()").value(1)) + .andExpect(jsonPath("$.notifications[0].id").value(unread.getId())); + } + + @Test + @DisplayName("알림 단건 삭제는 수신자와 미삭제 알림만 변경하고 반복·대상 없음에도 204를 반환한다") + void 알림_단건_삭제_멱등성() throws Exception { + TeamRecruitment recruitment = saveGeneralRecruitment("알림 삭제", 3, 0); + TeamRecruitmentNotification unread = saveNotification(recruitment, applicant.getUser(), null, false); + TeamRecruitmentNotification deleted = saveNotification(recruitment, applicant.getUser(), null, true); + TeamRecruitmentNotification otherUser = saveNotification(recruitment, author.getUser(), null, false); + entityManager.flush(); + + mockMvc.perform(delete("/team-recruitments/notifications/{notificationId}", unread.getId()) + .header("Authorization", "Bearer " + applicantToken)) + .andExpect(status().isNoContent()); + mockMvc.perform(delete("/team-recruitments/notifications/{notificationId}", unread.getId()) + .header("Authorization", "Bearer " + applicantToken)) + .andExpect(status().isNoContent()); + mockMvc.perform(delete("/team-recruitments/notifications/{notificationId}", deleted.getId()) + .header("Authorization", "Bearer " + applicantToken)) + .andExpect(status().isNoContent()); + mockMvc.perform(delete("/team-recruitments/notifications/{notificationId}", otherUser.getId()) + .header("Authorization", "Bearer " + applicantToken)) + .andExpect(status().isNoContent()); + mockMvc.perform(delete("/team-recruitments/notifications/{notificationId}", 999999) + .header("Authorization", "Bearer " + applicantToken)) + .andExpect(status().isNoContent()); + + entityManager.clear(); + assertThat(notificationRepository.findById(unread.getId()).orElseThrow().getIsDeleted()) + .isTrue(); + assertThat(notificationRepository.findById(deleted.getId()).orElseThrow().getIsDeleted()) + .isTrue(); + assertThat(notificationRepository.findById(otherUser.getId()).orElseThrow().getIsDeleted()) + .isFalse(); + assertThat(notificationRepository.countByRecipient_IdAndIsDeletedFalse(applicant.getUser().getId())) + .isZero(); + mockMvc.perform(get("/team-recruitments/notifications") + .header("Authorization", "Bearer " + applicantToken)) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.total_count").value(0)) + .andExpect(jsonPath("$.unread_count").value(0)) + .andExpect(jsonPath("$.notifications.length()").value(0)); + } + @Test @DisplayName("정원을 승인 인원과 같게 줄이면 마감되지만 TEAM 채팅방은 ACTIVE 를 유지한다") void 정원_충족_자동_마감() throws Exception { @@ -406,6 +502,23 @@ private TeamRecruitmentApplication saveApplication( .build()); } + private TeamRecruitmentNotification saveNotification( + TeamRecruitment recruitment, + User recipient, + LocalDateTime readAt, + boolean isDeleted + ) { + return notificationRepository.save(TeamRecruitmentNotification.builder() + .recipient(recipient) + .type(APPLICATION_REJECTED) + .targetType(MY_APPLICATIONS) + .messagePreview("알림") + .recruitment(recruitment) + .readAt(readAt) + .isDeleted(isDeleted) + .build()); + } + private String generalBody(int maxParticipants) { return """ { diff --git a/src/test/java/in/koreatech/koin/unit/domain/teamrecruitment/service/TeamRecruitmentNotificationServiceTest.java b/src/test/java/in/koreatech/koin/unit/domain/teamrecruitment/service/TeamRecruitmentNotificationServiceTest.java index 4b8916a28f..0139f79aed 100644 --- a/src/test/java/in/koreatech/koin/unit/domain/teamrecruitment/service/TeamRecruitmentNotificationServiceTest.java +++ b/src/test/java/in/koreatech/koin/unit/domain/teamrecruitment/service/TeamRecruitmentNotificationServiceTest.java @@ -1,15 +1,14 @@ package in.koreatech.koin.unit.domain.teamrecruitment.service; 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.mock; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import java.util.List; -import java.util.Optional; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -19,9 +18,6 @@ import org.springframework.data.domain.PageImpl; import org.springframework.data.domain.Pageable; -import in.koreatech.koin.global.code.ApiResponseCode; -import in.koreatech.koin.global.exception.CustomException; - import in.koreatech.koin.domain.team.recruitment.model.TeamRecruitmentNotification; import in.koreatech.koin.domain.team.recruitment.repository.TeamRecruitmentNotificationRepository; import in.koreatech.koin.domain.teamrecruitment.dto.TeamRecruitmentNotificationsResponse; @@ -64,14 +60,20 @@ class TeamRecruitmentNotificationServiceTest { } @Test - void 존재하지_않는_알림을_읽음_처리하면_404를_반환한다() { - when(notificationRepository.findByIdAndRecipient_Id(NOTIFICATION_ID, USER_ID)) - .thenReturn(Optional.empty()); - - assertThatThrownBy(() -> notificationService.markAsRead(USER_ID, NOTIFICATION_ID)) - .isInstanceOf(CustomException.class) - .satisfies(e -> assertThat(((CustomException) e).getErrorCode()) - .isEqualTo(ApiResponseCode.TEAM_RECRUITMENT_NOTIFICATION_NOT_FOUND)); + void 개별_읽음_처리는_수신자_범위의_원자적_업데이트를_호출한다() { + notificationService.markAsRead(USER_ID, NOTIFICATION_ID); + + verify(notificationRepository).updateReadAtByRecipientIdAndNotificationId( + eq(USER_ID), eq(NOTIFICATION_ID), any()); + } + + @Test + void 개별_읽음_처리를_반복해도_예외가_발생하지_않는다() { + notificationService.markAsRead(USER_ID, NOTIFICATION_ID); + notificationService.markAsRead(USER_ID, NOTIFICATION_ID); + + verify(notificationRepository, times(2)).updateReadAtByRecipientIdAndNotificationId( + eq(USER_ID), eq(NOTIFICATION_ID), any()); } @Test @@ -89,25 +91,53 @@ class TeamRecruitmentNotificationServiceTest { } @Test - void 개별_삭제시_해당_알림만_삭제한다() { - TeamRecruitmentNotification notification = mock(TeamRecruitmentNotification.class); - when(notificationRepository.findByIdAndRecipient_Id(NOTIFICATION_ID, USER_ID)) - .thenReturn(Optional.of(notification)); + void 개별_삭제는_수신자_범위의_원자적_업데이트를_호출한다() { + notificationService.delete(USER_ID, NOTIFICATION_ID); + + verify(notificationRepository).updateIsDeletedByRecipientIdAndNotificationId(USER_ID, NOTIFICATION_ID); + } + + @Test + void 개별_삭제를_반복해도_예외가_발생하지_않는다() { + notificationService.delete(USER_ID, NOTIFICATION_ID); + notificationService.delete(USER_ID, NOTIFICATION_ID); + verify(notificationRepository, times(2)) + .updateIsDeletedByRecipientIdAndNotificationId(USER_ID, NOTIFICATION_ID); + } + + @Test + void 존재하지_않는_알림을_읽음_처리해도_예외가_발생하지_않는다() { + notificationService.markAsRead(USER_ID, NOTIFICATION_ID); + + verify(notificationRepository).updateReadAtByRecipientIdAndNotificationId( + eq(USER_ID), eq(NOTIFICATION_ID), any()); + } + + @Test + void 타_사용자_알림을_읽음_처리해도_예외가_발생하지_않는다() { + Integer otherUserId = 2; + + notificationService.markAsRead(otherUserId, NOTIFICATION_ID); + + verify(notificationRepository).updateReadAtByRecipientIdAndNotificationId( + eq(otherUserId), eq(NOTIFICATION_ID), any()); + } + + @Test + void 존재하지_않는_알림을_삭제해도_예외가_발생하지_않는다() { notificationService.delete(USER_ID, NOTIFICATION_ID); - verify(notification).delete(); + verify(notificationRepository).updateIsDeletedByRecipientIdAndNotificationId(USER_ID, NOTIFICATION_ID); } @Test - void 존재하지_않는_알림을_삭제하면_404를_반환한다() { - when(notificationRepository.findByIdAndRecipient_Id(NOTIFICATION_ID, USER_ID)) - .thenReturn(Optional.empty()); - - assertThatThrownBy(() -> notificationService.delete(USER_ID, NOTIFICATION_ID)) - .isInstanceOf(CustomException.class) - .satisfies(e -> assertThat(((CustomException) e).getErrorCode()) - .isEqualTo(ApiResponseCode.TEAM_RECRUITMENT_NOTIFICATION_NOT_FOUND)); + void 타_사용자_알림을_삭제해도_예외가_발생하지_않는다() { + Integer otherUserId = 2; + + notificationService.delete(otherUserId, NOTIFICATION_ID); + + verify(notificationRepository).updateIsDeletedByRecipientIdAndNotificationId(otherUserId, NOTIFICATION_ID); } @Test