-
Notifications
You must be signed in to change notification settings - Fork 0
[Feat] 세모피드 이모지 클릭 시 FCM 알림 추가 #142
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,47 @@ | ||
| package com.semosan.api.domain.semofeed.notification.service; | ||
|
|
||
| import com.semosan.api.domain.notification.enums.NotificationType; | ||
| import com.semosan.api.domain.notification.service.NotificationService; | ||
| import com.semosan.api.domain.semofeed.entity.SemoFeed; | ||
| import com.semosan.api.domain.semofeed.enums.SemoFeedEmojiType; | ||
| import com.semosan.api.domain.user.entity.User; | ||
| import com.semosan.api.domain.user.repository.UserRepository; | ||
| import lombok.RequiredArgsConstructor; | ||
| import org.springframework.stereotype.Service; | ||
|
|
||
| import java.util.Map; | ||
|
|
||
| @Service | ||
| @RequiredArgsConstructor | ||
| public class SemoFeedNotificationService { | ||
|
|
||
| private final NotificationService notificationService; | ||
| private final UserRepository userRepository; | ||
|
|
||
| // 세모피드에 새 이모지 반응이 등록되면 작성자에게 알림을 보냅니다. | ||
| public void sendEmojiNotification(SemoFeed semoFeed, User reactor, SemoFeedEmojiType emojiType) { | ||
| User receiver = semoFeed.getUser(); | ||
| sendIfEligible( | ||
| receiver, | ||
| reactor, | ||
| Map.of( | ||
| "actorId", reactor.getId(), | ||
| "actorName", reactor.displayName(), | ||
| "semoFeedId", semoFeed.getId(), | ||
| "emojiType", emojiType.name() | ||
| ) | ||
| ); | ||
| } | ||
|
|
||
| // 본인 반응과 탈퇴한 작성자는 알림 발송 대상에서 제외합니다. | ||
| private void sendIfEligible(User receiver, User reactor, Map<String, Object> params) { | ||
| if (receiver.getId().equals(reactor.getId())) { | ||
| return; | ||
| } | ||
| if (!userRepository.existsByIdAndDeletedFalse(receiver.getId())) { | ||
| return; | ||
| } | ||
|
|
||
| notificationService.send(receiver.getId(), NotificationType.SEMOFEED_EMOJI, params); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,6 +6,7 @@ | |
| import com.semosan.api.domain.semofeed.entity.SemoFeed; | ||
| import com.semosan.api.domain.semofeed.entity.SemoFeedEmoji; | ||
| import com.semosan.api.domain.semofeed.enums.SemoFeedEmojiType; | ||
| import com.semosan.api.domain.semofeed.notification.service.SemoFeedNotificationService; | ||
| import com.semosan.api.domain.semofeed.repository.SemoFeedEmojiRepository; | ||
| import com.semosan.api.domain.semofeed.repository.SemoFeedRepository; | ||
| import com.semosan.api.domain.user.entity.User; | ||
|
|
@@ -23,6 +24,7 @@ public class SemoFeedEmojiService { | |
|
|
||
| private final SemoFeedEmojiRepository semoFeedEmojiRepository; | ||
| private final SemoFeedRepository semoFeedRepository; | ||
| private final SemoFeedNotificationService semoFeedNotificationService; | ||
| private final UserReader userReader; | ||
|
|
||
| @Transactional | ||
|
|
@@ -36,6 +38,10 @@ public SemoFeedEmojiToggleResponse toggleWithCount( | |
| User user = userReader.findActiveUserById(userId); | ||
|
|
||
| boolean reacted = toggle(semoFeed, user, emojiType); | ||
| if (reacted) { | ||
| // 새 반응 등록일 때만 작성자 알림을 발송합니다. | ||
| semoFeedNotificationService.sendEmojiNotification(semoFeed, user, emojiType); | ||
| } | ||
|
Comment on lines
+41
to
+44
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 현재 이 구조는 다음과 같은 문제를 야기할 수 있습니다:
개선 제안: References
|
||
| long count = semoFeedEmojiRepository.countBySemoFeedAndEmojiType(semoFeed, emojiType); | ||
|
|
||
| return new SemoFeedEmojiToggleResponse(emojiType, reacted, count); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,111 @@ | ||
| package com.semosan.api.domain.semofeed.notification.service; | ||
|
|
||
| import com.semosan.api.domain.notification.enums.NotificationType; | ||
| import com.semosan.api.domain.notification.service.NotificationService; | ||
| import com.semosan.api.domain.semofeed.entity.SemoFeed; | ||
| import com.semosan.api.domain.semofeed.enums.SemoFeedEmojiType; | ||
| import com.semosan.api.domain.user.entity.User; | ||
| import com.semosan.api.domain.user.enums.user.DeviceType; | ||
| import com.semosan.api.domain.user.repository.UserRepository; | ||
| import org.junit.jupiter.api.Test; | ||
| import org.junit.jupiter.api.extension.ExtendWith; | ||
| import org.mockito.InjectMocks; | ||
| import org.mockito.Mock; | ||
| import org.mockito.junit.jupiter.MockitoExtension; | ||
| import org.springframework.test.util.ReflectionTestUtils; | ||
|
|
||
| import java.util.Map; | ||
|
|
||
| import static org.assertj.core.api.Assertions.assertThatCode; | ||
| import static org.mockito.ArgumentMatchers.argThat; | ||
| import static org.mockito.ArgumentMatchers.eq; | ||
| import static org.mockito.Mockito.never; | ||
| import static org.mockito.Mockito.verify; | ||
| import static org.mockito.Mockito.when; | ||
|
|
||
| @ExtendWith(MockitoExtension.class) | ||
| class SemoFeedNotificationServiceTest { | ||
|
|
||
| @Mock | ||
| private NotificationService notificationService; | ||
|
|
||
| @Mock | ||
| private UserRepository userRepository; | ||
|
|
||
| @InjectMocks | ||
| private SemoFeedNotificationService semoFeedNotificationService; | ||
|
|
||
| @Test | ||
| void semoFeedEmojiNotificationTypeRequiresNavigationPayload() { | ||
| assertThatCode(() -> NotificationType.SEMOFEED_EMOJI.validate(Map.of( | ||
| "actorId", 2L, | ||
| "actorName", "reactor", | ||
| "semoFeedId", 10L, | ||
| "emojiType", "FIRE" | ||
| ))).doesNotThrowAnyException(); | ||
| } | ||
|
|
||
| @Test | ||
| void sendEmojiNotificationSendsToSemoFeedAuthor() { | ||
| User author = user(1L, "author"); | ||
| User reactor = user(2L, "reactor"); | ||
| SemoFeed semoFeed = semoFeed(10L, author); | ||
|
|
||
| when(userRepository.existsByIdAndDeletedFalse(1L)).thenReturn(true); | ||
|
|
||
| semoFeedNotificationService.sendEmojiNotification(semoFeed, reactor, SemoFeedEmojiType.FIRE); | ||
|
|
||
| verify(notificationService).send( | ||
| eq(1L), | ||
| eq(NotificationType.SEMOFEED_EMOJI), | ||
| argThat(params -> params.get("actorId").equals(2L) | ||
| && params.get("actorName").equals("reactor") | ||
| && params.get("semoFeedId").equals(10L) | ||
| && params.get("emojiType").equals("FIRE")) | ||
| ); | ||
| } | ||
|
|
||
| @Test | ||
| void sendEmojiNotificationSkipsSelfReaction() { | ||
| User author = user(1L, "author"); | ||
| SemoFeed semoFeed = semoFeed(10L, author); | ||
|
|
||
| semoFeedNotificationService.sendEmojiNotification(semoFeed, author, SemoFeedEmojiType.HEART); | ||
|
|
||
| verify(notificationService, never()).send( | ||
| org.mockito.ArgumentMatchers.any(), | ||
| org.mockito.ArgumentMatchers.any(), | ||
| org.mockito.ArgumentMatchers.any() | ||
| ); | ||
| } | ||
|
|
||
| @Test | ||
| void sendEmojiNotificationSkipsInactiveAuthor() { | ||
| User author = user(1L, "author"); | ||
| User reactor = user(2L, "reactor"); | ||
| SemoFeed semoFeed = semoFeed(10L, author); | ||
|
|
||
| when(userRepository.existsByIdAndDeletedFalse(1L)).thenReturn(false); | ||
|
|
||
| semoFeedNotificationService.sendEmojiNotification(semoFeed, reactor, SemoFeedEmojiType.LAUGH); | ||
|
|
||
| verify(notificationService, never()).send( | ||
| org.mockito.ArgumentMatchers.any(), | ||
| org.mockito.ArgumentMatchers.any(), | ||
| org.mockito.ArgumentMatchers.any() | ||
| ); | ||
| } | ||
|
|
||
| private User user(Long id, String nickname) { | ||
| User user = User.createTestUser(nickname, DeviceType.IOS); | ||
| ReflectionTestUtils.setField(user, "id", id); | ||
| ReflectionTestUtils.setField(user, "nickname", nickname); | ||
| return user; | ||
| } | ||
|
|
||
| private SemoFeed semoFeed(Long id, User user) { | ||
| SemoFeed semoFeed = SemoFeed.create(user, "https://example.com/semofeed.png"); | ||
| ReflectionTestUtils.setField(semoFeed, "id", id); | ||
| return semoFeed; | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
userRepository.existsByIdAndDeletedFalse(receiver.getId())호출로 인해 중복 쿼리가 발생합니다.NotificationService.send메서드 내부(46라인)에서도 동일하게userRepository.existsByIdAndDeletedFalse(receiverId)를 호출하여 수신자의 존재 여부 및 탈퇴 여부를 검증하고 있습니다. 이로 인해 동일한receiverId에 대해exists쿼리가 두 번 연속으로 실행되는 비효율이 존재합니다.개선 제안:
알림 발송 실패(탈퇴한 사용자 등)가 주 비즈니스 트랜잭션에 영향을 주지 않도록 이벤트를 통해 비동기/분리 처리하거나,
NotificationService에 예외를 던지지 않고 무시하는 안전한 발송 메서드(예:sendIfPresent)를 추가하여 중복 쿼리를 제거하는 것을 고려해 주세요.